diff mbox series

[1/6] drm/i915: Move psr unlock out from the pipe update critical section

Message ID 20230828054140.28054-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: VRR and M/N stuff | expand

Commit Message

Ville Syrjala Aug. 28, 2023, 5:41 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do the PSR unlock after the vblank evade critcal section is
fully over, not before.

Cc: Manasi Navare <navaremanasi@chromium.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Manasi Navare Aug. 28, 2023, 6:16 p.m. UTC | #1 
By doing psr_unlock after the vblank evade, are we ensuring that even
when VRR params change during the pipe update, psr unlock will
happen after the actual vblank  based on newly programmed VRR params?

Manasi

On Sun, Aug 27, 2023 at 10:41 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Do the PSR unlock after the vblank evade critcal section is
> fully over, not before.
>
> Cc: Manasi Navare <navaremanasi@chromium.org>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crtc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 182c6dd64f47..5caa928e5ce9 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -646,10 +646,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
>         ktime_t end_vbl_time = ktime_get();
>         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> -       intel_psr_unlock(new_crtc_state);
> -
>         if (new_crtc_state->do_async_flip)
> -               return;
> +               goto out;
>
>         trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
>
> @@ -709,7 +707,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
>         local_irq_enable();
>
>         if (intel_vgpu_active(dev_priv))
> -               return;
> +               goto out;
>
>         if (crtc->debug.start_vbl_count &&
>             crtc->debug.start_vbl_count != end_vbl_count) {
> @@ -724,4 +722,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
>         }
>
>         dbg_vblank_evade(crtc, end_vbl_time);
> +
> +out:
> +       intel_psr_unlock(new_crtc_state);
>  }
> --
> 2.41.0
>
Ville Syrjala Aug. 29, 2023, 8:23 a.m. UTC | #2 
On Mon, Aug 28, 2023 at 11:16:13AM -0700, Manasi Navare wrote:
> By doing psr_unlock after the vblank evade, are we ensuring that even
> when VRR params change during the pipe update, psr unlock will
> happen after the actual vblank  based on newly programmed VRR params?

The unlock will happen as soon as the new register values have been
written. The vblank will happen when it happens, could be asap or could
be much later.

I don't actually even know what this PSR lock is protecting, I suppose
it's trying to prevent muckery with the PSR hw state while the update
is being programmed. Shrug.

> 
> Manasi
> 
> On Sun, Aug 27, 2023 at 10:41 PM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Do the PSR unlock after the vblank evade critcal section is
> > fully over, not before.
> >
> > Cc: Manasi Navare <navaremanasi@chromium.org>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_crtc.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> > index 182c6dd64f47..5caa928e5ce9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> > @@ -646,10 +646,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> >         ktime_t end_vbl_time = ktime_get();
> >         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >
> > -       intel_psr_unlock(new_crtc_state);
> > -
> >         if (new_crtc_state->do_async_flip)
> > -               return;
> > +               goto out;
> >
> >         trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
> >
> > @@ -709,7 +707,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> >         local_irq_enable();
> >
> >         if (intel_vgpu_active(dev_priv))
> > -               return;
> > +               goto out;
> >
> >         if (crtc->debug.start_vbl_count &&
> >             crtc->debug.start_vbl_count != end_vbl_count) {
> > @@ -724,4 +722,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> >         }
> >
> >         dbg_vblank_evade(crtc, end_vbl_time);
> > +
> > +out:
> > +       intel_psr_unlock(new_crtc_state);
> >  }
> > --
> > 2.41.0
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 182c6dd64f47..5caa928e5ce9 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -646,10 +646,8 @@  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	ktime_t end_vbl_time = ktime_get();
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	intel_psr_unlock(new_crtc_state);
-
 	if (new_crtc_state->do_async_flip)
-		return;
+		goto out;
 
 	trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
@@ -709,7 +707,7 @@  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	local_irq_enable();
 
 	if (intel_vgpu_active(dev_priv))
-		return;
+		goto out;
 
 	if (crtc->debug.start_vbl_count &&
 	    crtc->debug.start_vbl_count != end_vbl_count) {
@@ -724,4 +722,7 @@  void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	}
 
 	dbg_vblank_evade(crtc, end_vbl_time);
+
+out:
+	intel_psr_unlock(new_crtc_state);
 }