Re: [PATCH 5/9] drm/i915/dmc: Reload pipe DMC state on TGL when enabling pipe A

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jun 12, 2025 at 08:32:16PM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Ville
> > Syrjala
> > Sent: Wednesday, June 11, 2025 9:23 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org
> > Subject: [PATCH 5/9] drm/i915/dmc: Reload pipe DMC state on TGL when
> > enabling pipe A
> 
> I guess its applicable for all pipes and not just limited to A.

Only pipe A has a DMC on these platforms.

> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On TGL/derivatives the entire pipe DMC state (program + MMIO) is lost when
> > PG1 is disabled, and the main DMC does not restore any of it. Reload the state
> > when enabling a pipe.
> 
> It is just the TGL or any other platforms affected ?
> Current change looks fine though.
> 
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> > The other option would be to not load the pipe DMC at all since it's only needed for
> > "fast LACE" (which we don't use) on these platforms. But let's keep it around just
> > in case we ever decide that "fast LACE" is something we want.
> 
> I agree, it's good to keep it.
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dmc.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > index 331db28039db..fd99c4645260 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > @@ -599,6 +599,12 @@ static void dmc_load_program(struct intel_display
> > *display,
> >  	}
> >  }
> > 
> > +static bool need_pipedmc_load_program(struct intel_display *display) {
> > +	/* On TGL/derivatives pipe DMC state is lost when PG1 is disabled */
> > +	return DISPLAY_VER(display) == 12;
> > +}
> > +
> >  void intel_dmc_enable_pipe(struct intel_display *display, enum pipe pipe)  {
> >  	enum intel_dmc_id dmc_id = PIPE_TO_DMC_ID(pipe); @@ -606,6 +612,9
> > @@ void intel_dmc_enable_pipe(struct intel_display *display, enum pipe pipe)
> >  	if (!is_valid_dmc_id(dmc_id) || !has_dmc_id_fw(display, dmc_id))
> >  		return;
> > 
> > +	if (need_pipedmc_load_program(display))
> > +		dmc_load_program(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));
> > --
> > 2.49.0
> 

-- 
Ville Syrjälä
Intel