On Thu, 2024-03-28 at 10:13 +0530, Animesh Manna wrote:
> 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.
>
> 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]
>
> Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 17 +++++++++++++++++
> drivers/gpu/drm/i915/display/intel_display.h | 1 +
> drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++
> 3 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 00ac65a14029..7f5c42a14196 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3840,6 +3840,23 @@ bool intel_crtc_get_pipe_config(struct
> intel_crtc_state *crtc_state)
> return true;
> }
>
> +void intel_crtc_vblank_delay(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> +
> + /*
> + * wa_14015401596 for display versions >= 13.
> + * Program Set Context Latency in TRANS_SET_CONTEXT_LATENCY
> register
> + * to at least a value of 1 when 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 (crtc_state->vrr.enable && crtc_state->has_panel_replay &&
> + adjusted_mode->crtc_vblank_start == adjusted_mode-
> >crtc_vdisplay)
> + adjusted_mode->crtc_vblank_start += 1;
> +}
> +
Do you have some specific reason to have this in intel_display.c? How
about move it to intel_psr.c? You could also use more descriptive name.
Current name somehow made me think it is some generic function to
calculate vblank delay. It is actually only for this workaround.
BR,
Jouni Högander
> 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 f4a0773f0fca..23315eced41e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -413,6 +413,7 @@ bool intel_crtc_is_bigjoiner_master(const struct
> intel_crtc_state *crtc_state);
> u8 intel_crtc_bigjoiner_slave_pipes(const struct intel_crtc_state
> *crtc_state);
> struct intel_crtc *intel_master_crtc(const struct intel_crtc_state
> *crtc_state);
> bool intel_crtc_get_pipe_config(struct intel_crtc_state
> *crtc_state);
> +void intel_crtc_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);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 72cadad09db5..fccef5434e9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1430,6 +1430,10 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
> if (!(crtc_state->has_panel_replay || crtc_state->has_psr))
> return;
>
> + /* wa_14015401596: display versions 13, 14 */
> + if (DISPLAY_VER(dev_priv) >= 13)
> + intel_crtc_vblank_delay(crtc_state);
> +
> crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp,
> crtc_state);
> }
>
