diff mbox series

[v13,3/3] drm/i915/panelreplay: Panel replay workaround with VRR

Message ID 20241001134703.1128487-4-mitulkumar.ajitkumar.golani@intel.com (mailing list archive)
State New, archived
Headers show
Series VRR refactoring and panel replay workaround | expand

Commit Message

Golani, Mitulkumar Ajitkumar Oct. 1, 2024, 1:47 p.m. UTC
From: Animesh Manna <animesh.manna@intel.com>

Panel Replay VSC SDP not getting sent when VRR is enabled
and W1 and W2 are 0. So Program Set Context Latency in
TRANS_SET_CONTEXT_LATENCY register to at least a value of 1.
The same is applicable for PSR1/PSR2 as well.

HSD: 14015406119

v1: Initial version.
v2: Update timings stored in adjusted_mode struct. [Ville]
v3: Add WA in compute_config(). [Ville]
v4:
- Add DISPLAY_VER() check and improve code comment. [Rodrigo]
- Introduce centralized intel_crtc_vblank_delay(). [Ville]
v5: Move to crtc_compute_config(). [Ville]
v6: Restrict DISPLAY_VER till 14. [Mitul]
v7:
- Corrected code-comment. [Mitul]
- dev_priv local variable removed. [Jani]
v8: Introduce late_compute_config() which will take care late
vblank-delay adjustment. [Ville]
v9: Implementation simplified and split into multiple patches.
v10:
- Split vrr changes and use struct intel_display in DISPLAY_VER(). [Ankit]
- Use for_each_new_intel_connector_in_state(). [Jani]
v11: Remove loop and use flipline instead of vrr.enable flag. [Ville]
v12:
- Use intel_Vrr_possible helper.
- Correct flag check for flipline.

Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.h |  1 +
 2 files changed, 22 insertions(+)

Comments

Cavitt, Jonathan Oct. 1, 2024, 3:07 p.m. UTC | #1
-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Mitul Golani
Sent: Tuesday, October 1, 2024 6:47 AM
To: intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma <uma.shankar@intel.com>
Subject: [PATCH v13 3/3] drm/i915/panelreplay: Panel replay workaround with VRR
> 
> From: Animesh Manna <animesh.manna@intel.com>
> 
> Panel Replay VSC SDP not getting sent when VRR is enabled
> and W1 and W2 are 0. So Program Set Context Latency in
> TRANS_SET_CONTEXT_LATENCY register to at least a value of 1.
> The same is applicable for PSR1/PSR2 as well.
> 
> HSD: 14015406119
> 
> v1: Initial version.
> v2: Update timings stored in adjusted_mode struct. [Ville]
> v3: Add WA in compute_config(). [Ville]
> v4:
> - Add DISPLAY_VER() check and improve code comment. [Rodrigo]
> - Introduce centralized intel_crtc_vblank_delay(). [Ville]
> v5: Move to crtc_compute_config(). [Ville]
> v6: Restrict DISPLAY_VER till 14. [Mitul]
> v7:
> - Corrected code-comment. [Mitul]
> - dev_priv local variable removed. [Jani]
> v8: Introduce late_compute_config() which will take care late
> vblank-delay adjustment. [Ville]
> v9: Implementation simplified and split into multiple patches.
> v10:
> - Split vrr changes and use struct intel_display in DISPLAY_VER(). [Ankit]
> - Use for_each_new_intel_connector_in_state(). [Jani]
> v11: Remove loop and use flipline instead of vrr.enable flag. [Ville]
> v12:
> - Use intel_Vrr_possible helper.
> - Correct flag check for flipline.
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>

I left some notes below, but nothing blocking, so:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c59d7bffbf57..a8f846b654e9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2573,6 +2573,8 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	int ret;
>  
> +	intel_crtc_adjust_vblank_delay(crtc_state);


If the purpose of adjusting the vblank delay is for workaround 14015401596,
we might want to consider refactoring this into two steps.  Specifically, we would
first separately check if the workaround is needed:

"""
/*
 * wa_14015401596 for display versions 13, 14.
 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register
 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with VRR.
 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by subtracting
 * crtc_vdisplay from crtc_vblank_start, so this workaround is needed when
 * both are equal.
 */
static bool intel_crtc_needs_wa_14015401596(struct intel_crtc_state *crtc_state)
{
	struct intel_display *display = to_intel_display(crtc_state);
	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;

	return (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
		adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay &&
		IS_DISPLAY_VER(display, 13, 14));
}
"""

Then, here, instead of calling intel_crtc_adjust_vblank_delay, we would do this:

"""
	if (intel_crtc_needs_wa_14015401596(crtc_state)) {
		struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
		adjusted_mode->crtc_vblank_start += 1;
	}
"""

I don't think this refactor is strictly necessary, though, so feel free to ignore this.


> +
>  	ret = intel_dpll_crtc_compute_clock(state, crtc);
>  	if (ret)
>  		return ret;
> @@ -3985,6 +3987,25 @@ bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state)
>  	return true;
>  }
>  
> +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> +	/*
> +	 * wa_14015401596 for display versions 13, 14.
> +	 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register
> +	 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with VRR.
> +	 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by substracting


s/substracting/subtracting


> +	 * crtc_vdisplay from crtc_vblank_start, so incrementing crtc_vblank_start
> +	 * by 1 if both are equal.
> +	 */
> +	if (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
> +	    adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay &&
> +	    IS_DISPLAY_VER(display, 13, 14))
> +		adjusted_mode->crtc_vblank_start += 1;
> +}
> +
>  int intel_dotclock_calculate(int link_freq,
>  			     const struct intel_link_m_n *m_n)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 1f0fed5ea7bc..e6bd03ef104d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -446,6 +446,7 @@ u8 _intel_modeset_primary_pipes(const struct intel_crtc_state *crtc_state);
>  u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state *crtc_state);
>  struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
> +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state);


You may want to make this a static function in intel_display.c.  Doing so would eliminate the need for
the added function declaration in intel_display.h.  The only caveat is that you would need to move the
current definition of this function in intel_display.c from where it is now to above
intel_crtc_compute_config if you wanted to go that route.
-Jonathan Cavitt


>  bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  			       const struct intel_crtc_state *pipe_config,
>  			       bool fastset);
> -- 
> 2.46.0
> 
>
Ville Syrjälä Oct. 9, 2024, 7:30 a.m. UTC | #2
On Tue, Oct 01, 2024 at 07:17:03PM +0530, Mitul Golani wrote:
> From: Animesh Manna <animesh.manna@intel.com>
> 
> Panel Replay VSC SDP not getting sent when VRR is enabled
> and W1 and W2 are 0. So Program Set Context Latency in
> TRANS_SET_CONTEXT_LATENCY register to at least a value of 1.
> The same is applicable for PSR1/PSR2 as well.
> 
> HSD: 14015406119
> 
> v1: Initial version.
> v2: Update timings stored in adjusted_mode struct. [Ville]
> v3: Add WA in compute_config(). [Ville]
> v4:
> - Add DISPLAY_VER() check and improve code comment. [Rodrigo]
> - Introduce centralized intel_crtc_vblank_delay(). [Ville]
> v5: Move to crtc_compute_config(). [Ville]
> v6: Restrict DISPLAY_VER till 14. [Mitul]
> v7:
> - Corrected code-comment. [Mitul]
> - dev_priv local variable removed. [Jani]
> v8: Introduce late_compute_config() which will take care late
> vblank-delay adjustment. [Ville]
> v9: Implementation simplified and split into multiple patches.
> v10:
> - Split vrr changes and use struct intel_display in DISPLAY_VER(). [Ankit]
> - Use for_each_new_intel_connector_in_state(). [Jani]
> v11: Remove loop and use flipline instead of vrr.enable flag. [Ville]
> v12:
> - Use intel_Vrr_possible helper.
> - Correct flag check for flipline.
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index c59d7bffbf57..a8f846b654e9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2573,6 +2573,8 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	int ret;
>  
> +	intel_crtc_adjust_vblank_delay(crtc_state);
> +
>  	ret = intel_dpll_crtc_compute_clock(state, crtc);
>  	if (ret)
>  		return ret;
> @@ -3985,6 +3987,25 @@ bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state)
>  	return true;
>  }
>  
> +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> +
> +	/*
> +	 * wa_14015401596 for display versions 13, 14.
> +	 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register
> +	 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with VRR.
> +	 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by substracting
> +	 * crtc_vdisplay from crtc_vblank_start, so incrementing crtc_vblank_start
> +	 * by 1 if both are equal.

You're just paraphrasing the code in different words here. 
Please don't, and just drop the whole comment (apart from the
w/a number ofc).

> +	 */
> +	if (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
> +	    adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay &&
> +	    IS_DISPLAY_VER(display, 13, 14))
> +		adjusted_mode->crtc_vblank_start += 1;
> +}
> +
>  int intel_dotclock_calculate(int link_freq,
>  			     const struct intel_link_m_n *m_n)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 1f0fed5ea7bc..e6bd03ef104d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -446,6 +446,7 @@ u8 _intel_modeset_primary_pipes(const struct intel_crtc_state *crtc_state);
>  u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state *crtc_state);
>  struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
> +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state);
>  bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  			       const struct intel_crtc_state *pipe_config,
>  			       bool fastset);
> -- 
> 2.46.0
Golani, Mitulkumar Ajitkumar Oct. 9, 2024, 9:18 a.m. UTC | #3
> -----Original Message-----
> From: Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Sent: Tuesday, October 1, 2024 8:37 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>;
> intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
> Subject: RE: [PATCH v13 3/3] drm/i915/panelreplay: Panel replay workaround
> with VRR
> 
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Mitul
> Golani
> Sent: Tuesday, October 1, 2024 6:47 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: [PATCH v13 3/3] drm/i915/panelreplay: Panel replay workaround with
> VRR
> >
> > From: Animesh Manna <animesh.manna@intel.com>
> >
> > Panel Replay VSC SDP not getting sent when VRR is enabled and W1 and
> > W2 are 0. So Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY
> > register to at least a value of 1.
> > The same is applicable for PSR1/PSR2 as well.
> >
> > HSD: 14015406119
> >
> > v1: Initial version.
> > v2: Update timings stored in adjusted_mode struct. [Ville]
> > v3: Add WA in compute_config(). [Ville]
> > v4:
> > - Add DISPLAY_VER() check and improve code comment. [Rodrigo]
> > - Introduce centralized intel_crtc_vblank_delay(). [Ville]
> > v5: Move to crtc_compute_config(). [Ville]
> > v6: Restrict DISPLAY_VER till 14. [Mitul]
> > v7:
> > - Corrected code-comment. [Mitul]
> > - dev_priv local variable removed. [Jani]
> > v8: Introduce late_compute_config() which will take care late
> > vblank-delay adjustment. [Ville]
> > v9: Implementation simplified and split into multiple patches.
> > v10:
> > - Split vrr changes and use struct intel_display in DISPLAY_VER().
> > [Ankit]
> > - Use for_each_new_intel_connector_in_state(). [Jani]
> > v11: Remove loop and use flipline instead of vrr.enable flag. [Ville]
> > v12:
> > - Use intel_Vrr_possible helper.
> > - Correct flag check for flipline.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> 
> I left some notes below, but nothing blocking, so:
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 21
> > ++++++++++++++++++++  drivers/gpu/drm/i915/display/intel_display.h |
> > 1 +
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index c59d7bffbf57..a8f846b654e9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2573,6 +2573,8 @@ static int intel_crtc_compute_config(struct
> intel_atomic_state *state,
> >  		intel_atomic_get_new_crtc_state(state, crtc);
> >  	int ret;
> >
> > +	intel_crtc_adjust_vblank_delay(crtc_state);
> 
> 
> If the purpose of adjusting the vblank delay is for workaround 14015401596, we
> might want to consider refactoring this into two steps.  Specifically, we would
> first separately check if the workaround is needed:
> 
> """
> /*
>  * wa_14015401596 for display versions 13, 14.
>  * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register
>  * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with VRR.
>  * Value for TRANS_SET_CONTEXT_LATENCY is calculated by subtracting
>  * crtc_vdisplay from crtc_vblank_start, so this workaround is needed when
>  * both are equal.
>  */
> static bool intel_crtc_needs_wa_14015401596(struct intel_crtc_state
> *crtc_state) {
> 	struct intel_display *display = to_intel_display(crtc_state);
> 	struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> 
> 	return (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
> 		adjusted_mode->crtc_vblank_start == adjusted_mode-
> >crtc_vdisplay &&
> 		IS_DISPLAY_VER(display, 13, 14));
> }
> """
> 
> Then, here, instead of calling intel_crtc_adjust_vblank_delay, we would do this:
> 
> """
> 	if (intel_crtc_needs_wa_14015401596(crtc_state)) {
> 		struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> 		adjusted_mode->crtc_vblank_start += 1;
> 	}
> """
> 
> I don't think this refactor is strictly necessary, though, so feel free to ignore this.
> 
> 
> > +
> >  	ret = intel_dpll_crtc_compute_clock(state, crtc);
> >  	if (ret)
> >  		return ret;
> > @@ -3985,6 +3987,25 @@ bool intel_crtc_get_pipe_config(struct
> intel_crtc_state *crtc_state)
> >  	return true;
> >  }
> >
> > +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state
> > +*crtc_state) {
> > +	struct intel_display *display = to_intel_display(crtc_state);
> > +	struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > +	/*
> > +	 * wa_14015401596 for display versions 13, 14.
> > +	 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY
> register
> > +	 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with
> VRR.
> > +	 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by
> substracting
> 
> 
> s/substracting/subtracting
> 
> 
> > +	 * crtc_vdisplay from crtc_vblank_start, so incrementing
> crtc_vblank_start
> > +	 * by 1 if both are equal.
> > +	 */
> > +	if (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
> > +	    adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay
> &&
> > +	    IS_DISPLAY_VER(display, 13, 14))
> > +		adjusted_mode->crtc_vblank_start += 1; }
> > +
> >  int intel_dotclock_calculate(int link_freq,
> >  			     const struct intel_link_m_n *m_n)  { diff --git
> > a/drivers/gpu/drm/i915/display/intel_display.h
> > b/drivers/gpu/drm/i915/display/intel_display.h
> > index 1f0fed5ea7bc..e6bd03ef104d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -446,6 +446,7 @@ u8 _intel_modeset_primary_pipes(const struct
> > intel_crtc_state *crtc_state);
> >  u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state
> > *crtc_state);  struct intel_crtc *intel_primary_crtc(const struct
> > intel_crtc_state *crtc_state);  bool intel_crtc_get_pipe_config(struct
> > intel_crtc_state *crtc_state);
> > +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state
> > +*crtc_state);
> 
> 
> You may want to make this a static function in intel_display.c.  Doing so would
> eliminate the need for the added function declaration in intel_display.h.  The
> only caveat is that you would need to move the current definition of this
> function in intel_display.c from where it is now to above
> intel_crtc_compute_config if you wanted to go that route.
> -Jonathan Cavitt
> 

Thanks @Cavitt, Jonathan
I agreed to your suggestion related to needs_wa function.

I will address all suggested comments in next revision.

> 
> >  bool intel_pipe_config_compare(const struct intel_crtc_state
> *current_config,
> >  			       const struct intel_crtc_state *pipe_config,
> >  			       bool fastset);
> > --
> > 2.46.0
> >
> >
Golani, Mitulkumar Ajitkumar Oct. 9, 2024, 9:19 a.m. UTC | #4
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 9, 2024 1:01 PM
> To: Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>;
> Syrjala, Ville <ville.syrjala@intel.com>; Nautiyal, Ankit K
> <ankit.k.nautiyal@intel.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH v13 3/3] drm/i915/panelreplay: Panel replay workaround
> with VRR
> 
> On Tue, Oct 01, 2024 at 07:17:03PM +0530, Mitul Golani wrote:
> > From: Animesh Manna <animesh.manna@intel.com>
> >
> > Panel Replay VSC SDP not getting sent when VRR is enabled and W1 and
> > W2 are 0. So Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY
> > register to at least a value of 1.
> > The same is applicable for PSR1/PSR2 as well.
> >
> > HSD: 14015406119
> >
> > v1: Initial version.
> > v2: Update timings stored in adjusted_mode struct. [Ville]
> > v3: Add WA in compute_config(). [Ville]
> > v4:
> > - Add DISPLAY_VER() check and improve code comment. [Rodrigo]
> > - Introduce centralized intel_crtc_vblank_delay(). [Ville]
> > v5: Move to crtc_compute_config(). [Ville]
> > v6: Restrict DISPLAY_VER till 14. [Mitul]
> > v7:
> > - Corrected code-comment. [Mitul]
> > - dev_priv local variable removed. [Jani]
> > v8: Introduce late_compute_config() which will take care late
> > vblank-delay adjustment. [Ville]
> > v9: Implementation simplified and split into multiple patches.
> > v10:
> > - Split vrr changes and use struct intel_display in DISPLAY_VER().
> > [Ankit]
> > - Use for_each_new_intel_connector_in_state(). [Jani]
> > v11: Remove loop and use flipline instead of vrr.enable flag. [Ville]
> > v12:
> > - Use intel_Vrr_possible helper.
> > - Correct flag check for flipline.
> >
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 21
> > ++++++++++++++++++++  drivers/gpu/drm/i915/display/intel_display.h |
> > 1 +
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index c59d7bffbf57..a8f846b654e9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -2573,6 +2573,8 @@ static int intel_crtc_compute_config(struct
> intel_atomic_state *state,
> >  		intel_atomic_get_new_crtc_state(state, crtc);
> >  	int ret;
> >
> > +	intel_crtc_adjust_vblank_delay(crtc_state);
> > +
> >  	ret = intel_dpll_crtc_compute_clock(state, crtc);
> >  	if (ret)
> >  		return ret;
> > @@ -3985,6 +3987,25 @@ bool intel_crtc_get_pipe_config(struct
> intel_crtc_state *crtc_state)
> >  	return true;
> >  }
> >
> > +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state
> > +*crtc_state) {
> > +	struct intel_display *display = to_intel_display(crtc_state);
> > +	struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > +	/*
> > +	 * wa_14015401596 for display versions 13, 14.
> > +	 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY
> register
> > +	 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with
> VRR.
> > +	 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by
> substracting
> > +	 * crtc_vdisplay from crtc_vblank_start, so incrementing
> crtc_vblank_start
> > +	 * by 1 if both are equal.
> 
> You're just paraphrasing the code in different words here.
> Please don't, and just drop the whole comment (apart from the w/a number
> ofc).

Thanks @Syrjala, Ville for the review,

I will remove comment in next revision.

> 
> > +	 */
> > +	if (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
> > +	    adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay
> &&
> > +	    IS_DISPLAY_VER(display, 13, 14))
> > +		adjusted_mode->crtc_vblank_start += 1; }
> > +
> >  int intel_dotclock_calculate(int link_freq,
> >  			     const struct intel_link_m_n *m_n)  { diff --git
> > a/drivers/gpu/drm/i915/display/intel_display.h
> > b/drivers/gpu/drm/i915/display/intel_display.h
> > index 1f0fed5ea7bc..e6bd03ef104d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -446,6 +446,7 @@ u8 _intel_modeset_primary_pipes(const struct
> > intel_crtc_state *crtc_state);
> >  u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state
> > *crtc_state);  struct intel_crtc *intel_primary_crtc(const struct
> > intel_crtc_state *crtc_state);  bool intel_crtc_get_pipe_config(struct
> > intel_crtc_state *crtc_state);
> > +void intel_crtc_adjust_vblank_delay(struct intel_crtc_state
> > +*crtc_state);
> >  bool intel_pipe_config_compare(const struct intel_crtc_state
> *current_config,
> >  			       const struct intel_crtc_state *pipe_config,
> >  			       bool fastset);
> > --
> > 2.46.0
> 
> --
> Ville Syrjälä
> Intel
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 c59d7bffbf57..a8f846b654e9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2573,6 +2573,8 @@  static int intel_crtc_compute_config(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	int ret;
 
+	intel_crtc_adjust_vblank_delay(crtc_state);
+
 	ret = intel_dpll_crtc_compute_clock(state, crtc);
 	if (ret)
 		return ret;
@@ -3985,6 +3987,25 @@  bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
+void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
+
+	/*
+	 * wa_14015401596 for display versions 13, 14.
+	 * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY register
+	 * to at least a value of 1 when PSR1/PSR2/Panel Replay is enabled with VRR.
+	 * Value for TRANS_SET_CONTEXT_LATENCY is calculated by substracting
+	 * crtc_vdisplay from crtc_vblank_start, so incrementing crtc_vblank_start
+	 * by 1 if both are equal.
+	 */
+	if (intel_vrr_possible(crtc_state) && crtc_state->has_psr &&
+	    adjusted_mode->crtc_vblank_start == adjusted_mode->crtc_vdisplay &&
+	    IS_DISPLAY_VER(display, 13, 14))
+		adjusted_mode->crtc_vblank_start += 1;
+}
+
 int intel_dotclock_calculate(int link_freq,
 			     const struct intel_link_m_n *m_n)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 1f0fed5ea7bc..e6bd03ef104d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -446,6 +446,7 @@  u8 _intel_modeset_primary_pipes(const struct intel_crtc_state *crtc_state);
 u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state *crtc_state);
 struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
+void intel_crtc_adjust_vblank_delay(struct intel_crtc_state *crtc_state);
 bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 			       const struct intel_crtc_state *pipe_config,
 			       bool fastset);