> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, June 11, 2025 9:23 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel...@lists.freedesktop.org
> Subject: [PATCH 7/9] drm/i915/dmc: Assert DMC is loaded harder
>
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> Currently we have some asserts to make sure the main DMC has been loaded.
> Add similar assers for the pipe DMCs. And we migth as well just check all the
Nit: Typo in assert and might
> mmio registers the firmware has asked us to initialize. That also covers the
> hardcoded SSP/HTP registers we were checking for the main DMC.
>
> TODO: Maybe always configure DMC_EVT_CTL_ENABLE the way the firmware
> has it set so that we wouldn't need to special case in the assert?
>
> v2: Also assert in intel_dmc_load_program()
Looks Good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> .../i915/display/intel_display_power_well.c | 4 +-
> drivers/gpu/drm/i915/display/intel_dmc.c | 60 ++++++++++++++-----
> drivers/gpu/drm/i915/display/intel_dmc.h | 2 +-
> 3 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index cba96f920fd2..0f1848b970a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -829,7 +829,7 @@ static void assert_can_enable_dc5(struct intel_display
> *display)
>
> assert_display_rpm_held(display);
>
> - assert_dmc_loaded(display);
> + assert_main_dmc_loaded(display);
> }
>
> void gen9_enable_dc5(struct intel_display *display) @@ -860,7 +860,7 @@
> static void assert_can_enable_dc6(struct intel_display *display)
> DC_STATE_EN_UPTO_DC6),
> "DC6 already programmed to be enabled.\n");
>
> - assert_dmc_loaded(display);
> + assert_main_dmc_loaded(display);
> }
>
> void skl_enable_dc6(struct intel_display *display) diff --git
> a/drivers/gpu/drm/i915/display/intel_dmc.c
> b/drivers/gpu/drm/i915/display/intel_dmc.c
> index 76b88c9bea02..37618797d931 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -606,6 +606,46 @@ static void dmc_load_program(struct intel_display
> *display, enum intel_dmc_id dm
> dmc_load_mmio(display, dmc_id);
> }
>
> +static void assert_dmc_loaded(struct intel_display *display,
> + enum intel_dmc_id dmc_id)
> +{
> + struct intel_dmc *dmc = display_to_dmc(display);
> + u32 expected, found;
> + int i;
> +
> + if (!is_valid_dmc_id(dmc_id) || !has_dmc_id_fw(display, dmc_id))
> + return;
> +
> + found = intel_de_read(display, DMC_PROGRAM(dmc-
> >dmc_info[dmc_id].start_mmioaddr, 0));
> + expected = dmc->dmc_info[dmc_id].payload[0];
> +
> + drm_WARN(display->drm, found != expected,
> + "DMC %d program storage start incorrect (expected 0x%x,
> current 0x%x)\n",
> + dmc_id, expected, found);
> +
> + for (i = 0; i < dmc->dmc_info[dmc_id].mmio_count; i++) {
> + i915_reg_t reg = dmc->dmc_info[dmc_id].mmioaddr[i];
> +
> + found = intel_de_read(display, reg);
> + expected = dmc_mmiodata(display, dmc, dmc_id, i);
> +
> + /* once set DMC_EVT_CTL_ENABLE can't be cleared :/ */
> + if (is_dmc_evt_ctl_reg(display, dmc_id, reg)) {
> + found &= ~DMC_EVT_CTL_ENABLE;
> + expected &= ~DMC_EVT_CTL_ENABLE;
> + }
> +
> + drm_WARN(display->drm, found != expected,
> + "DMC %d mmio[%d]/0x%x incorrect (expected 0x%x,
> current 0x%x)\n",
> + dmc_id, i, i915_mmio_reg_offset(reg), expected, found);
> + }
> +}
> +
> +void assert_main_dmc_loaded(struct intel_display *display) {
> + assert_dmc_loaded(display, DMC_FW_MAIN); }
> +
> static bool need_pipedmc_load_program(struct intel_display *display) {
> /* On TGL/derivatives pipe DMC state is lost when PG1 is disabled */ @@
> -635,6 +675,8 @@ void intel_dmc_enable_pipe(struct intel_display *display,
> enum
> pipe pipe)
> else if (need_pipedmc_load_mmio(display, pipe))
> dmc_load_mmio(display, dmc_id);
>
> + assert_dmc_loaded(display, dmc_id);
> +
> if (DISPLAY_VER(display) >= 20) {
> intel_de_write(display, PIPEDMC_INTERRUPT(pipe),
> pipedmc_interrupt_mask(display));
> intel_de_write(display, PIPEDMC_INTERRUPT_MASK(pipe),
> ~pipedmc_interrupt_mask(display));
> @@ -744,8 +786,10 @@ void intel_dmc_load_program(struct intel_display
> *display)
>
> pipedmc_clock_gating_wa(display, true);
>
> - for_each_dmc_id(dmc_id)
> + for_each_dmc_id(dmc_id) {
> dmc_load_program(display, dmc_id);
> + assert_dmc_loaded(display, dmc_id);
> + }
>
> power_domains->dc_state = 0;
>
> @@ -776,20 +820,6 @@ void intel_dmc_disable_program(struct intel_display
> *display)
> pipedmc_clock_gating_wa(display, false); }
>
> -void assert_dmc_loaded(struct intel_display *display) -{
> - struct intel_dmc *dmc = display_to_dmc(display);
> -
> - drm_WARN_ONCE(display->drm, !dmc, "DMC not initialized\n");
> - drm_WARN_ONCE(display->drm, dmc &&
> - !intel_de_read(display, DMC_PROGRAM(dmc-
> >dmc_info[DMC_FW_MAIN].start_mmioaddr, 0)),
> - "DMC program storage start is NULL\n");
> - drm_WARN_ONCE(display->drm, !intel_de_read(display,
> DMC_SSP_BASE),
> - "DMC SSP Base Not fine\n");
> - drm_WARN_ONCE(display->drm, !intel_de_read(display,
> DMC_HTP_SKL),
> - "DMC HTP Not fine\n");
> -}
> -
> static bool fw_info_matches_stepping(const struct intel_fw_info *fw_info,
> const struct stepping_info *si) { diff
> --git
> a/drivers/gpu/drm/i915/display/intel_dmc.h
> b/drivers/gpu/drm/i915/display/intel_dmc.h
> index a98e8deff13a..a3792052078a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> @@ -32,7 +32,7 @@ struct intel_dmc_snapshot
> *intel_dmc_snapshot_capture(struct intel_display *disp void
> intel_dmc_snapshot_print(const struct intel_dmc_snapshot *snapshot, struct
> drm_printer *p); void intel_dmc_update_dc6_allowed_count(struct intel_display
> *display, bool start_tracking);
>
> -void assert_dmc_loaded(struct intel_display *display);
> +void assert_main_dmc_loaded(struct intel_display *display);
>
> void intel_pipedmc_irq_handler(struct intel_display *display, enum pipe
> pipe);
>
> --
> 2.49.0
