diff mbox series

[07/14] drm/i915/dsb: Account for VRR properly in DSB scanline stuff

Message ID 20240624191032.27333-8-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dsb: Use chained DSBs for LUT programming | expand

Commit Message

Ville Syrjala June 24, 2024, 7:10 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When determining various scanlines for DSB use we should take into
account whether VRR is active at the time when the DSB uses said
scanline information. For now all DSB scanline usage occurs prior
to the actual commit, so we only need to care about the state of
VRR at that time.

I've decided to move intel_crtc_scanline_to_hw() in its entirety
to the DSB code as it will also need to know the actual state
of VRR in order to do its job 100% correctly.

TODO: figure out how much of this could be moved to some
      more generic place and perhaps be shared with the CPU
      vblank evasion code/etc...

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  4 +-
 drivers/gpu/drm/i915/display/intel_display.h |  3 +
 drivers/gpu/drm/i915/display/intel_dsb.c     | 65 ++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_vblank.c  | 10 +--
 drivers/gpu/drm/i915/display/intel_vblank.h  |  3 +-
 5 files changed, 67 insertions(+), 18 deletions(-)

Comments

Manna, Animesh July 3, 2024, 8:23 a.m. UTC | #1 
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 25, 2024 12:40 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH 07/14] drm/i915/dsb: Account for VRR properly in DSB
> scanline stuff
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When determining various scanlines for DSB use we should take into account
> whether VRR is active at the time when the DSB uses said scanline
> information. For now all DSB scanline usage occurs prior to the actual
> commit, so we only need to care about the state of VRR at that time.
> 
> I've decided to move intel_crtc_scanline_to_hw() in its entirety to the DSB
> code as it will also need to know the actual state of VRR in order to do its job
> 100% correctly.
> 
> TODO: figure out how much of this could be moved to some
>       more generic place and perhaps be shared with the CPU
>       vblank evasion code/etc...
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Animesh Manna <animesh.manna@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  4 +-
> drivers/gpu/drm/i915/display/intel_display.h |  3 +
>  drivers/gpu/drm/i915/display/intel_dsb.c     | 65 ++++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_vblank.c  | 10 +--
> drivers/gpu/drm/i915/display/intel_vblank.h  |  3 +-
>  5 files changed, 67 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 01a5faa3fea5..592483651b3c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1031,8 +1031,8 @@ static bool intel_crtc_vrr_enabling(struct
> intel_atomic_state *state,
>  		  vrr_params_changed(old_crtc_state, new_crtc_state)));  }
> 
> -static bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
> -				     struct intel_crtc *crtc)
> +bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
> +			      struct intel_crtc *crtc)
>  {
>  	const struct intel_crtc_state *old_crtc_state =
>  		intel_atomic_get_old_crtc_state(state, crtc); diff --git
> a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index b0cf6ca70952..b21d9578d5db 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -532,6 +532,9 @@ void intel_plane_fixup_bitmasks(struct
> intel_crtc_state *crtc_state);
> 
>  void intel_update_watermarks(struct drm_i915_private *i915);
> 
> +bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
> +			      struct intel_crtc *crtc);
> +
>  /* modesetting */
>  int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
>  				      const char *reason, u8 pipe_mask); diff --
> git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index e871af5517b5..b362a3050c7f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -83,15 +83,72 @@ struct intel_dsb {
>  #define DSB_OPCODE_POLL			0xA
>  /* see DSB_REG_VALUE_MASK */
> 
> -static int dsb_dewake_scanline(const struct intel_crtc_state *crtc_state)
> +static bool pre_commit_is_vrr_active(struct intel_atomic_state *state,
> +				     struct intel_crtc *crtc)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, crtc);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	/* VRR will be enabled afterwards, if necessary */
> +	if (intel_crtc_needs_modeset(new_crtc_state))
> +		return false;
> +
> +	/* VRR will have been disabled during intel_pre_plane_update() */
> +	return old_crtc_state->vrr.enable && !intel_crtc_vrr_disabling(state,
> +crtc); }
> +
> +static const struct intel_crtc_state *
> +pre_commit_crtc_state(struct intel_atomic_state *state,
> +		      struct intel_crtc *crtc)
> +{
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, crtc);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
> +
> +	/*
> +	 * During fastsets/etc. the transcoder is still
> +	 * running with the old timings at this point.
> +	 */
> +	if (intel_crtc_needs_modeset(new_crtc_state))
> +		return new_crtc_state;
> +	else
> +		return old_crtc_state;
> +}
> +
> +static int dsb_vtotal(struct intel_atomic_state *state,
> +		      struct intel_crtc *crtc)
> +{
> +	const struct intel_crtc_state *crtc_state =
> +pre_commit_crtc_state(state, crtc);
> +
> +	if (pre_commit_is_vrr_active(state, crtc))
> +		return crtc_state->vrr.vmax;
> +	else
> +		return intel_mode_vtotal(&crtc_state->hw.adjusted_mode);
> +}
> +
> +static int dsb_dewake_scanline(struct intel_atomic_state *state,
> +			       struct intel_crtc *crtc)
> +{
> +	const struct intel_crtc_state *crtc_state =
> pre_commit_crtc_state(state, crtc);
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  	unsigned int latency = skl_watermark_max_latency(i915, 0);
> 
>  	return intel_mode_vdisplay(&crtc_state->hw.adjusted_mode) -
>  		intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode,
> latency);  }
> 
> +static int dsb_scanline_to_hw(struct intel_atomic_state *state,
> +			      struct intel_crtc *crtc, int scanline) {
> +	const struct intel_crtc_state *crtc_state =
> pre_commit_crtc_state(state, crtc);
> +	int vtotal = dsb_vtotal(state, crtc);
> +
> +	return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) %
> +vtotal; }
> +
>  static u32 dsb_chicken(struct intel_crtc *crtc)  {
>  	if (crtc->mode_flags & I915_MODE_FLAG_VRR) @@ -489,8 +546,6
> @@ struct intel_dsb *intel_dsb_prepare(struct intel_atomic_state *state,
>  				    unsigned int max_cmds)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	const struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
>  	intel_wakeref_t wakeref;
>  	struct intel_dsb *dsb;
>  	unsigned int size;
> @@ -526,7 +581,7 @@ struct intel_dsb *intel_dsb_prepare(struct
> intel_atomic_state *state,
>  	dsb->ins_start_offset = 0;
> 
>  	dsb->hw_dewake_scanline =
> -		intel_crtc_scanline_to_hw(crtc_state,
> dsb_dewake_scanline(crtc_state));
> +		dsb_scanline_to_hw(state, crtc, dsb_dewake_scanline(state,
> crtc));
> 
>  	return dsb;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
> b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 56c8033eec4c..71abc7354c3b 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -190,7 +190,7 @@ static u32
> __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
>  	return scanline;
>  }
> 
> -static int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
> +int intel_crtc_scanline_offset(const struct intel_crtc_state
> +*crtc_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> 
> @@ -284,14 +284,6 @@ static int __intel_get_crtc_scanline(struct intel_crtc
> *crtc)
>  	return (position + vtotal + crtc->scanline_offset) % vtotal;  }
> 
> -int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state,
> -			      int scanline)
> -{
> -	int vtotal = intel_mode_vtotal(&crtc_state->hw.adjusted_mode);
> -
> -	return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) %
> vtotal;
> -}
> -
>  /*
>   * The uncore version of the spin lock functions is used to decide
>   * whether we need to lock the uncore lock or not.  This is only diff --git
> a/drivers/gpu/drm/i915/display/intel_vblank.h
> b/drivers/gpu/drm/i915/display/intel_vblank.h
> index 45a4a961aaab..6d7336256982 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.h
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.h
> @@ -40,7 +40,6 @@ void intel_wait_for_pipe_scanline_stopped(struct
> intel_crtc *crtc);  void intel_wait_for_pipe_scanline_moving(struct intel_crtc
> *crtc);  void intel_crtc_update_active_timings(const struct intel_crtc_state
> *crtc_state,
>  				      bool vrr_enable);
> -int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state,
> -			      int scanline);
> +int intel_crtc_scanline_offset(const struct intel_crtc_state
> +*crtc_state);
> 
>  #endif /* __INTEL_VBLANK_H__ */
> --
> 2.44.2
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 01a5faa3fea5..592483651b3c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1031,8 +1031,8 @@  static bool intel_crtc_vrr_enabling(struct intel_atomic_state *state,
 		  vrr_params_changed(old_crtc_state, new_crtc_state)));
 }
 
-static bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
-				     struct intel_crtc *crtc)
+bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
+			      struct intel_crtc *crtc)
 {
 	const struct intel_crtc_state *old_crtc_state =
 		intel_atomic_get_old_crtc_state(state, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index b0cf6ca70952..b21d9578d5db 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -532,6 +532,9 @@  void intel_plane_fixup_bitmasks(struct intel_crtc_state *crtc_state);
 
 void intel_update_watermarks(struct drm_i915_private *i915);
 
+bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
+			      struct intel_crtc *crtc);
+
 /* modesetting */
 int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
 				      const char *reason, u8 pipe_mask);
diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index e871af5517b5..b362a3050c7f 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -83,15 +83,72 @@  struct intel_dsb {
 #define DSB_OPCODE_POLL			0xA
 /* see DSB_REG_VALUE_MASK */
 
-static int dsb_dewake_scanline(const struct intel_crtc_state *crtc_state)
+static bool pre_commit_is_vrr_active(struct intel_atomic_state *state,
+				     struct intel_crtc *crtc)
 {
-	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(state, crtc);
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	/* VRR will be enabled afterwards, if necessary */
+	if (intel_crtc_needs_modeset(new_crtc_state))
+		return false;
+
+	/* VRR will have been disabled during intel_pre_plane_update() */
+	return old_crtc_state->vrr.enable && !intel_crtc_vrr_disabling(state, crtc);
+}
+
+static const struct intel_crtc_state *
+pre_commit_crtc_state(struct intel_atomic_state *state,
+		      struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(state, crtc);
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	/*
+	 * During fastsets/etc. the transcoder is still
+	 * running with the old timings at this point.
+	 */
+	if (intel_crtc_needs_modeset(new_crtc_state))
+		return new_crtc_state;
+	else
+		return old_crtc_state;
+}
+
+static int dsb_vtotal(struct intel_atomic_state *state,
+		      struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
+
+	if (pre_commit_is_vrr_active(state, crtc))
+		return crtc_state->vrr.vmax;
+	else
+		return intel_mode_vtotal(&crtc_state->hw.adjusted_mode);
+}
+
+static int dsb_dewake_scanline(struct intel_atomic_state *state,
+			       struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	unsigned int latency = skl_watermark_max_latency(i915, 0);
 
 	return intel_mode_vdisplay(&crtc_state->hw.adjusted_mode) -
 		intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, latency);
 }
 
+static int dsb_scanline_to_hw(struct intel_atomic_state *state,
+			      struct intel_crtc *crtc, int scanline)
+{
+	const struct intel_crtc_state *crtc_state = pre_commit_crtc_state(state, crtc);
+	int vtotal = dsb_vtotal(state, crtc);
+
+	return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) % vtotal;
+}
+
 static u32 dsb_chicken(struct intel_crtc *crtc)
 {
 	if (crtc->mode_flags & I915_MODE_FLAG_VRR)
@@ -489,8 +546,6 @@  struct intel_dsb *intel_dsb_prepare(struct intel_atomic_state *state,
 				    unsigned int max_cmds)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
-	const struct intel_crtc_state *crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
 	intel_wakeref_t wakeref;
 	struct intel_dsb *dsb;
 	unsigned int size;
@@ -526,7 +581,7 @@  struct intel_dsb *intel_dsb_prepare(struct intel_atomic_state *state,
 	dsb->ins_start_offset = 0;
 
 	dsb->hw_dewake_scanline =
-		intel_crtc_scanline_to_hw(crtc_state, dsb_dewake_scanline(crtc_state));
+		dsb_scanline_to_hw(state, crtc, dsb_dewake_scanline(state, crtc));
 
 	return dsb;
 
diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
index 56c8033eec4c..71abc7354c3b 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -190,7 +190,7 @@  static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
 	return scanline;
 }
 
-static int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
+int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
@@ -284,14 +284,6 @@  static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 	return (position + vtotal + crtc->scanline_offset) % vtotal;
 }
 
-int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state,
-			      int scanline)
-{
-	int vtotal = intel_mode_vtotal(&crtc_state->hw.adjusted_mode);
-
-	return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) % vtotal;
-}
-
 /*
  * The uncore version of the spin lock functions is used to decide
  * whether we need to lock the uncore lock or not.  This is only
diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h b/drivers/gpu/drm/i915/display/intel_vblank.h
index 45a4a961aaab..6d7336256982 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.h
+++ b/drivers/gpu/drm/i915/display/intel_vblank.h
@@ -40,7 +40,6 @@  void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc);
 void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc);
 void intel_crtc_update_active_timings(const struct intel_crtc_state *crtc_state,
 				      bool vrr_enable);
-int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state,
-			      int scanline);
+int intel_crtc_scanline_offset(const struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_VBLANK_H__ */