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

Manna, Animesh animesh.manna at intel.com
Wed Jul 3 08:23:12 UTC 2024



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Tuesday, June 25, 2024 12:40 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [PATCH 07/14] drm/i915/dsb: Account for VRR properly in DSB
> scanline stuff
> 
> From: Ville Syrjälä <ville.syrjala at 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 at linux.intel.com>

Reviewed-by: Animesh Manna <animesh.manna at 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



More information about the Intel-gfx mailing list