> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Sent: Thursday, June 20, 2024 11:06 PM
> To: Manna, Animesh <animesh.manna@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx>;
> Hogander, Jouni <jouni.hogander@xxxxxxxxx>; Murthy, Arun R
> <arun.r.murthy@xxxxxxxxx>; Golani, Mitulkumar Ajitkumar
> <mitulkumar.ajitkumar.golani@xxxxxxxxx>
> Subject: Re: [PATCH v7] drm/i915/panelreplay: Panel replay workaround with
> VRR
>
> On Wed, Jun 19, 2024 at 04:01:01PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2024 at 04:52:15PM +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]
> > > 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]
> > >
> > > Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@xxxxxxxxx>
> > > Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx>
> > > ---
> > > 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 7bc4f3de691e..c3ff3a5c5fa3 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -2515,6 +2515,10 @@ static int intel_crtc_compute_config(struct
> intel_atomic_state *state,
> > > intel_atomic_get_new_crtc_state(state, crtc);
> > > int ret;
> > >
> > > + /* wa_14015401596: display versions 13, 14 */
> > > + if (IS_DISPLAY_VER(to_i915(crtc->base.dev), 13, 14))
> > > + intel_crtc_vblank_delay(crtc_state);
> > > +
> > > ret = intel_dpll_crtc_compute_clock(state, crtc);
> > > if (ret)
> > > return ret;
> > > @@ -3924,6 +3928,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, 14.
> > > + * 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; }
> >
> > This is probably too late actually. We already used the previous value
> > to calculate the VRR guardband/pipeline full values, which may or may
> > not now be incorrect. So NAK for now until someone actually checks how
> > it all works (I don't recall the details right now).
>
> I double checked this and the guardband/pipeline full values do indeed need
> to be calculated based on the delayed vblank. So unfortunately this needs to
> be done before VRR computation, which is a bit annoying if we'd need to
> tweak this also for HDMI or DSI.
> But for now we shouldn't actually need other adjustements as I'm going to
> be doing the DSB stuff without relying on delayed vblank.
Sure, I will add a change for recalculating guardband.
No need to change pipeline full value as this workaround is for display version >= 13.
Currently this workaround is only for panel replay, so HDMI and DSI is out of scope.
As I understood, DSB stuff will be taken care separately, is it ok if I move the adjustment in encoder-compute-config, more specifically in psr-compute-config where we will know about vrr and panel replay is enabled or not and recalculate the guardband ?
Regards,
Animesh
>
> >
> > > +
> > > 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 b0cf6ca70952..f99a24e76608 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > @@ -428,6 +428,7 @@ bool intel_crtc_is_joiner_primary(const struct
> > > intel_crtc_state *crtc_state);
> > > u8 intel_crtc_joiner_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_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.29.0
> >
> > --
> > Ville Syrjälä
> > Intel
>
> --
> Ville Syrjälä
> Intel
