Re: [PATCH v7 13/13] drm/i915/psr: Allow DSB usage when PSR is enabled

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

On Wed, Feb 12, 2025 at 06:22:21PM +0000, Hogander, Jouni wrote:
> On Wed, 2025-02-12 at 16:52 +0200, Ville Syrjälä wrote:
> > On Wed, Feb 12, 2025 at 09:57:42AM +0200, Jouni Högander wrote:
> > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can
> > > allow DSB
> > > usage also when PSR is enabled for LunarLake onwards.
> > > 
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 0ba85623835c..a6966a664d87 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7698,6 +7698,7 @@ static void intel_atomic_dsb_prepare(struct
> > > intel_atomic_state *state,
> > >  static void intel_atomic_dsb_finish(struct intel_atomic_state
> > > *state,
> > >  				    struct intel_crtc *crtc)
> > >  {
> > > +	struct intel_display *display = to_intel_display(state);
> > >  	const struct intel_crtc_state *old_crtc_state =
> > >  		intel_atomic_get_old_crtc_state(state, crtc);
> > >  	struct intel_crtc_state *new_crtc_state =
> > > @@ -7713,7 +7714,7 @@ static void intel_atomic_dsb_finish(struct
> > > intel_atomic_state *state,
> > >  	new_crtc_state->use_dsb =
> > >  		new_crtc_state->update_planes &&
> > >  		!new_crtc_state->do_async_flip &&
> > > -		!new_crtc_state->has_psr &&
> > > +		(DISPLAY_VER(display) >= 20 || !new_crtc_state-
> > > >has_psr) &&
> > 
> > Couldn't we also do it for !selective_fetch on earlier platforms?
> 
> I was thinking this as well, but I haven't tested it. I'm concerned are
> of frontbuffer invalidates/flushes. There we are disabling PSR. I'm not
> sure if it would be ok to do that while DSB update is ongoing.

As long as we don't touch any PSR registers via the DSB I don't
really see why it shouldn't work. Whether the PSR exit gets 
triggered by the DSB, some random event, of the PSR code poking
at the control register or CURSURFLIVE it shouldn't really matter.

But better smoke test it first before flipping the switch I guess.

-- 
Ville Syrjälä
Intel