[PATCH v2 6/9] drm/i915/dmc: Reload pipe DMC MMIO registers for pipe C/D on various platforms

[Shankar, Uma]

> -----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 17, 2025 10:38 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: intel-xe at lists.freedesktop.org
> Subject: [PATCH v2 6/9] drm/i915/dmc: Reload pipe DMC MMIO registers for pipe
> C/D on various platforms
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> On ADL/MTL pipe DMC MMIO state evidently lives in PG0. The main DMC
> saves/restores it for pipes A/B, but for pipes C/D we have to do it in the driver.
> 
> On PTL the situation is mostly the same, except the main DMC firmware doesn't
> seem to have the PG0 save/restore code anymore, and instead the hardware (or
> maybe Punit?) seems to take care of this job now. Pipes C/D still need a manual
> restore by the driver.
> 
> On LNL I've been unable to lose any pipe DMC state, despite the main DMC
> firmware still implementing the PG0 save/restore for pipes A/B.
> Not sure what's going on here.
> 
> On DG2 I've also not been able to lose the pipe DMC state. DG2 doesn't support
> DC6, so that might explain part of it. But even
> DC9 doesn't make a difference here. Perhaps PG0 is just always on for DG2?
> 
> BMG I've not tested at all. The main DMC firmware does appaer to implement the
> PG0 pipe A/B save/restore logic.

Nice to get the behaviour on all the platforms, I think we can even ask DMC team to get
this properly documented in spec as well. 

Changes Look Good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dmc.c | 67 +++++++++++++++++++++---
>  1 file changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> b/drivers/gpu/drm/i915/display/intel_dmc.c
> index fd99c4645260..dd15d35fbae8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -575,8 +575,18 @@ static u32 dmc_mmiodata(struct intel_display *display,
>  		return dmc->dmc_info[dmc_id].mmiodata[i];
>  }
> 
> -static void dmc_load_program(struct intel_display *display,
> -			     enum intel_dmc_id dmc_id)
> +static void dmc_load_mmio(struct intel_display *display, enum
> +intel_dmc_id dmc_id) {
> +	struct intel_dmc *dmc = display_to_dmc(display);
> +	int i;
> +
> +	for (i = 0; i < dmc->dmc_info[dmc_id].mmio_count; i++) {
> +		intel_de_write(display, dmc->dmc_info[dmc_id].mmioaddr[i],
> +			       dmc_mmiodata(display, dmc, dmc_id, i));
> +	}
> +}
> +
> +static void dmc_load_program(struct intel_display *display, enum
> +intel_dmc_id dmc_id)
>  {
>  	struct intel_dmc *dmc = display_to_dmc(display);
>  	int i;
> @@ -593,10 +603,7 @@ static void dmc_load_program(struct intel_display
> *display,
> 
>  	preempt_enable();
> 
> -	for (i = 0; i < dmc->dmc_info[dmc_id].mmio_count; i++) {
> -		intel_de_write(display, dmc->dmc_info[dmc_id].mmioaddr[i],
> -			       dmc_mmiodata(display, dmc, dmc_id, i));
> -	}
> +	dmc_load_mmio(display, dmc_id);
>  }
> 
>  static bool need_pipedmc_load_program(struct intel_display *display) @@ -605,6
> +612,52 @@ static bool need_pipedmc_load_program(struct intel_display
> *display)
>  	return DISPLAY_VER(display) == 12;
>  }
> 
> +static bool need_pipedmc_load_mmio(struct intel_display *display, enum
> +pipe pipe) {
> +	/*
> +	 * PTL:
> +	 * - pipe A/B DMC doesn't need save/restore
> +	 * - pipe C/D DMC is in PG0, needs manual save/restore
> +	 */
> +	if (DISPLAY_VER(display) == 30)
> +		return pipe >= PIPE_C;
> +
> +	/*
> +	 * FIXME LNL unclear, main DMC firmware has the pipe DMC A/B PG0
> +	 * save/restore, but so far unable to see the loss of pipe DMC state
> +	 * in action. Are we just failing to turn off PG0 due to some other
> +	 * SoC level stuff?
> +	 */
> +	if (DISPLAY_VER(display) == 20)
> +		return false;
> +
> +	/*
> +	 * FIXME BMG untested, main DMC firmware has the
> +	 * pipe DMC A/B PG0 save/restore...
> +	 */
> +	if (display->platform.battlemage)
> +		return false;
> +
> +	/*
> +	 * DG2:
> +	 * - Pipe DMCs presumably in PG0?
> +	 * - No DC6, and even DC9 doesn't seem to result
> +	 *   in loss of DMC state for whatever reason
> +	 */
> +	if (display->platform.dg2)
> +		return false;
> +
> +	/*
> +	 * ADL/MTL:
> +	 * - pipe A/B DMC is in PG0, saved/restored by the main DMC
> +	 * - pipe C/D DMC is in PG0, needs manual save/restore
> +	 */
> +	if (IS_DISPLAY_VER(display, 13, 14))
> +		return pipe >= PIPE_C;
> +
> +	return false;
> +}
> +
>  void intel_dmc_enable_pipe(struct intel_display *display, enum pipe pipe)  {
>  	enum intel_dmc_id dmc_id = PIPE_TO_DMC_ID(pipe); @@ -614,6 +667,8
> @@ void intel_dmc_enable_pipe(struct intel_display *display, enum pipe pipe)
> 
>  	if (need_pipedmc_load_program(display))
>  		dmc_load_program(display, dmc_id);
> +	else if (need_pipedmc_load_mmio(display, pipe))
> +		dmc_load_mmio(display, dmc_id);
> 
>  	if (DISPLAY_VER(display) >= 20) {
>  		intel_de_write(display, PIPEDMC_INTERRUPT(pipe),
> pipedmc_interrupt_mask(display));
> --
> 2.49.0