> -----Original Message-----
> From: Intel-xe <[email protected]> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 17, 2025 10:38 PM
> To: [email protected]
> Cc: [email protected]
> Subject: [PATCH v2 6/9] drm/i915/dmc: Reload pipe DMC MMIO registers for pipe
> C/D on various platforms
> 
> From: Ville Syrjälä <[email protected]>
> 
> 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 <[email protected]>

> Signed-off-by: Ville Syrjälä <[email protected]>
> ---
>  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

Reply via email to