Skip to content

Commit ac57eb3

Browse files
ideakjlahtine-intel
authored andcommitted
drm/i915/dmc: Fix an unlikely NULL pointer deference at probe
intel_dmc_update_dc6_allowed_count() oopses when DMC hasn't been initialized, and dmc is thus NULL. That would be the case when the call path is intel_power_domains_init_hw() -> {skl,bxt,icl}_display_core_init() -> gen9_set_dc_state() -> intel_dmc_update_dc6_allowed_count(), as intel_power_domains_init_hw() is called *before* intel_dmc_init(). However, gen9_set_dc_state() calls intel_dmc_update_dc6_allowed_count() conditionally, depending on the current and target DC states. At probe, the target is disabled, but if DC6 is enabled, the function is called, and an oops follows. Apparently it's quite unlikely that DC6 is enabled at probe, as we haven't seen this failure mode before. It is also strange to have DC6 enabled at boot, since that would require the DMC firmware (loaded by BIOS); the BIOS loading the DMC firmware and the driver stopping / reprogramming the firmware is a poorly specified sequence and as such unlikely an intentional BIOS behaviour. It's more likely that BIOS is leaving an unintentionally enabled DC6 HW state behind (without actually loading the required DMC firmware for this). The tracking of the DC6 allowed counter only works if starting / stopping the counter depends on the _SW_ DC6 state vs. the current _HW_ DC6 state (since stopping the counter requires the DC5 counter captured when the counter was started). Thus, using the HW DC6 state is incorrect and it also leads to the above oops. Fix both issues by using the SW DC6 state for the tracking. This is v2 of the fix originally sent by Jani, updated based on the first Link: discussion below. Link: https://lore.kernel.org/all/[email protected] Link: https://lore.kernel.org/all/[email protected] Fixes: 88c1f9a ("drm/i915/dmc: Create debugfs entry for dc6 counter") Cc: Mohammed Thasleem <[email protected]> Cc: Jani Nikula <[email protected]> Cc: Tao Liu <[email protected]> Cc: <[email protected]> # v6.16+ Tested-by: Tao Liu <[email protected]> Reviewed-by: Jani Nikula <[email protected]> Signed-off-by: Imre Deak <[email protected]> Link: https://patch.msgid.link/[email protected] (cherry picked from commit 2344b93) Signed-off-by: Joonas Lahtinen <[email protected]>
1 parent f338e77 commit ac57eb3

2 files changed

Lines changed: 2 additions & 3 deletions

File tree

drivers/gpu/drm/i915/display/intel_display_power_well.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ void gen9_set_dc_state(struct intel_display *display, u32 state)
806806
power_domains->dc_state, val & mask);
807807

808808
enable_dc6 = state & DC_STATE_EN_UPTO_DC6;
809-
dc6_was_enabled = val & DC_STATE_EN_UPTO_DC6;
809+
dc6_was_enabled = power_domains->dc_state & DC_STATE_EN_UPTO_DC6;
810810
if (!dc6_was_enabled && enable_dc6)
811811
intel_dmc_update_dc6_allowed_count(display, true);
812812

drivers/gpu/drm/i915/display/intel_dmc.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,8 +1599,7 @@ static bool intel_dmc_get_dc6_allowed_count(struct intel_display *display, u32 *
15991599
return false;
16001600

16011601
mutex_lock(&power_domains->lock);
1602-
dc6_enabled = intel_de_read(display, DC_STATE_EN) &
1603-
DC_STATE_EN_UPTO_DC6;
1602+
dc6_enabled = power_domains->dc_state & DC_STATE_EN_UPTO_DC6;
16041603
if (dc6_enabled)
16051604
intel_dmc_update_dc6_allowed_count(display, false);
16061605

0 commit comments

Comments
 (0)