Re: [PATCH v3 10/13] drm/i915/dmc: Add interface to control start of PKG C-state exit

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Mon, 2025-05-12 at 13:57 +0300, Ville Syrjälä wrote:
> On Mon, Apr 14, 2025 at 01:05:05PM +0300, Jouni Högander wrote:
> > Add interface to control if package C exit starts at the start of
> > the
> > undelayed vblank. This is needed to implement workaround for
> > underrun on
> > idle PSR HW issue (Wa_16025596647).
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dmc.c | 31
> > ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dmc.h |  2 ++
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> > b/drivers/gpu/drm/i915/display/intel_dmc.c
> > index c65544e48c42..bd50c98c0ee4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> > @@ -534,6 +534,37 @@ void intel_dmc_block_pkgc(struct intel_display
> > *display, enum pipe pipe,
> >  		     PIPEDMC_BLOCK_PKGC_SW_BLOCK_PKGC_ALWAYS : 0);
> >  }
> >  
> > +/**
> > + * intel_dmc_start_pkgc_exit_at_start_of_undelayed_vblank() -
> > start of PKG
> > + * C-state exit
> > + * @display: display instance
> > + * @pipe: pipe which register use to block
> > + * @enable: enable/disable
> > + *
> > + * This interface is target for Wa_16025596647 usage. I.e. start
> > the package C
> > + * exit at the start of the undelayed vblank
> > + */
> > +void intel_dmc_start_pkgc_exit_at_start_of_undelayed_vblank(struct
> > intel_display *display,
> > +							    enum
> > pipe pipe, bool enable)
> > +{
> > +	u32 val;
> > +
> > +	if (enable)
> > +		val = DMC_EVT_CTL_ENABLE | DMC_EVT_CTL_RECURRING |
> > +			REG_FIELD_PREP(DMC_EVT_CTL_TYPE_MASK,
> > +				       DMC_EVT_CTL_TYPE_EDGE_0_1)
> > |
> > +			REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK,
> > +				      
> > DMC_EVT_CTL_EVENT_ID_VBLANK_A);
> > +	else
> > +		val = REG_FIELD_PREP(DMC_EVT_CTL_EVENT_ID_MASK,
> > +				     DMC_EVT_CTL_EVENT_ID_FALSE) |
> > +			REG_FIELD_PREP(DMC_EVT_CTL_TYPE_MASK,
> > +				       DMC_EVT_CTL_TYPE_EDGE_0_1);
> > +
> > +	intel_de_write(display, MTL_PIPEDMC_EVT_CTL_4(pipe),
> > +		       val);
> > +}
> 
> This thing doesn't seem to be fully thought through:
> 
> - Enabling PIPEDMC events exposes us to the DSB vs. DMC register
>   corruption issues once again. Someone would need to come up with
> some
>   kind of scheme to avoid DMC and DSB executing in parallel.
> Otherwise
>   I don't think we can safely enable this
> - The w/a seems to be only for cases where the pkgc latency exceeds
> the
>   delayed vblank length, but that is never allowed by the
>   skl_is_vblank_too_short() stuff, so looks like this should never be
>   needed currently

Ouch! Do you have suggestion how to take care of this now? Revert whole
thing?

BR,

Jouni Högander

> 
> > +
> >  static bool is_dmc_evt_ctl_reg(struct intel_display *display,
> >  			       enum intel_dmc_id dmc_id,
> > i915_reg_t reg)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_dmc.h
> > b/drivers/gpu/drm/i915/display/intel_dmc.h
> > index 236312fb791c..bd1c459b0075 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dmc.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dmc.h
> > @@ -20,6 +20,8 @@ void intel_dmc_enable_pipe(struct intel_display
> > *display, enum pipe pipe);
> >  void intel_dmc_disable_pipe(struct intel_display *display, enum
> > pipe pipe);
> >  void intel_dmc_block_pkgc(struct intel_display *display, enum pipe
> > pipe,
> >  			  bool block);
> > +void intel_dmc_start_pkgc_exit_at_start_of_undelayed_vblank(struct
> > intel_display *display,
> > +							    enum
> > pipe pipe, bool enable);
> >  void intel_dmc_fini(struct intel_display *display);
> >  void intel_dmc_suspend(struct intel_display *display);
> >  void intel_dmc_resume(struct intel_display *display);
> > -- 
> > 2.43.0
>