diff mbox series

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

Message ID 20250212075742.995022-15-jouni.hogander@intel.com (mailing list archive)
State New
Headers show
Series PSR DSB support | expand

Commit Message

Hogander, Jouni Feb. 12, 2025, 7:57 a.m. UTC 
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(-)

Comments

Ville Syrjälä Feb. 12, 2025, 2:52 p.m. UTC | #1 
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?

>  		!new_crtc_state->scaler_state.scaler_users &&
>  		!old_crtc_state->scaler_state.scaler_users &&
>  		!intel_crtc_needs_modeset(new_crtc_state) &&
> -- 
> 2.43.0
Hogander, Jouni Feb. 12, 2025, 6:22 p.m. UTC | #2 
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.

BR,

Jouni Högander
> 
> >  		!new_crtc_state->scaler_state.scaler_users &&
> >  		!old_crtc_state->scaler_state.scaler_users &&
> >  		!intel_crtc_needs_modeset(new_crtc_state) &&
> > -- 
> > 2.43.0
>
Ville Syrjälä Feb. 12, 2025, 6:52 p.m. UTC | #3 
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.
diff mbox series

Patch

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) &&
 		!new_crtc_state->scaler_state.scaler_users &&
 		!old_crtc_state->scaler_state.scaler_users &&
 		!intel_crtc_needs_modeset(new_crtc_state) &&