On Fri, Oct 24, 2025 at 09:30:26AM +0530, Nautiyal, Ankit K wrote:
>
> On 10/23/2025 8:38 PM, Ville Syrjälä wrote:
> > On Thu, Oct 23, 2025 at 05:40:09PM +0300, Ville Syrjälä wrote:
> >> On Thu, Oct 23, 2025 at 05:57:02PM +0530, Nautiyal, Ankit K wrote:
> >>> On 10/23/2025 5:34 PM, Ville Syrjälä wrote:
> >>>> On Thu, Oct 23, 2025 at 01:46:14PM +0530, Ankit Nautiyal wrote:
> >>>>> Currently the EMP_AS_SDP_TL is set to vrr.vsync_start which is
> >>>>> incorrect.
> >>>>>
> >>>>> As per Bspec:71197 the transmission line must be within the SCL +
> >>>>> guardband region. Before guardband optimization, guradband was same as
> >>>>> vblank length so EMP_AS_SDP_TL set with vrr.sync_start was falling in
> >>>>> this region and it was not giving an issue.
> >>>>>
> >>>>> Now with optimized guardband, this is falling outside the SCL +
> >>>>> guardband region and since the same transmission line is used by VSC SDP
> >>>>> also, this results in PSR timeout issues.
> >>>>>
> >>>>> Further restrictions on the position of the transmission line:
> >>>>> For DP/eDP, if there is a set context latency (SCL) window, then it
> >>>>> cannot be the first line of SCL
> >>>>> For DP/eDP, if there is no SCL window, then it cannot be the first line
> >>>>> of
> >>>>> the Delayed V. Blank
> >>>>>
> >>>>> Fix the EMP_AS_SDP_TL to VTOTAL - (delayed vblank_start - SCL + 1)
> >>>>> Internally the HW computes the value as VTOTAL - EMP_AS_SDP_TL.
> >>>>>
> >>>>> Fixes: e1123e617e51 ("drm/i915/vrr: Program EMP_AS_SDP_TL for DP AS
> >>>>> SDP")
> >>>>> Cc: Ankit Nautiyal <[email protected]>
> >>>>> Cc: Jouni Högander <[email protected]>
> >>>>> Signed-off-by: Ankit Nautiyal <[email protected]>
> >>>>> ---
> >>>>> drivers/gpu/drm/i915/display/intel_vrr.c | 12 +++++++++---
> >>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> >>>>> b/drivers/gpu/drm/i915/display/intel_vrr.c
> >>>>> index 92fb72b56f16..dd81d2133aba 100644
> >>>>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> >>>>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> >>>>> @@ -655,18 +655,24 @@ void
> >>>>> intel_vrr_set_db_point_and_transmission_line(const struct
> >>>>> intel_crtc_state
> >>>>> {
> >>>>> struct intel_display *display = to_intel_display(crtc_state);
> >>>>> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >>>>> + const struct drm_display_mode *adjusted_mode =
> >>>>> &crtc_state->hw.adjusted_mode;
> >>>>> + int transmission_line;
> >>>>>
> >>>>> /*
> >>>>> * For BMG and LNL+ onwards the EMP_AS_SDP_TL is used for
> >>>>> programming
> >>>>> * double buffering point and transmission line for VRR packets
> >>>>> for
> >>>>> * HDMI2.1/DP/eDP/DP->HDMI2.1 PCON.
> >>>>> * Since currently we support VRR only for DP/eDP, so this is
> >>>>> programmed
> >>>>> - * to for Adaptive Sync SDP to Vsync start.
> >>>>> + * for Adaptive Sync SDP.
> >>>>> */
> >>>>> - if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >=
> >>>>> 20)
> >>>>> + if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >=
> >>>>> 20) {
> >>>>> + transmission_line = adjusted_mode->crtc_vtotal -
> >>>>> (adjusted_mode->crtc_vblank_start -
> >>>>> +
> >>>>> crtc_state->set_context_latency +
> >>>>> + 1);
> >>>>> intel_de_write(display,
> >>>>> EMP_AS_SDP_TL(display, cpu_transcoder),
> >>>>> -
> >>>>> EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
> >>>>> + EMP_AS_SDP_DB_TL(transmission_line));
> >>>>> + }
> >>>> Pretty sure we are expected to send it at vsync_start.
> >>> Hmm.. then do we need to move vsync_start too similar to vblank_start
> >>> for optimized guardband?
> >> The vsync pulse location is dictated by the timings.
> >>
> >>> If we do not move vsync_start, and set the transmission line to
> >>> vsync_start, it will never fall in the region SCL + guardband with a
> >>> reduced guardband.
> >> Only if the vsync pulse is early in the vblank. That's up to the
> >> display.
> > Oh and I think we should get rid of that 'assume_all_enabled' stuff
> > for the AS SDP, and account for it only when actually needed.
> > Which I *think* means PCON or panel replay with AUX-less ALPM.
>
> Currently we are enabling it for VRR and CMRR with AVT and FAVT mode
> respectively.
>
>
> >
> > There's also that t1 vs. t2 setup time thing for the panel replay,
> > which seems to be telling me that we could sometimes transmit the
> > AS SDP later. But if I'm reading that right we have to switch to the
> > t1 (vsync) transmission line whenever we switch to live frame mode,
> > which I presume can happen basically at any time. So maybe we can't
> > actually use that t2 transmission line.
>
> I think it will be prudent to set the min guardband to accommodate the
> vsync_start as done by Jouni in patch:
> https://patchwork.freedesktop.org/patch/682984/?series=156341&rev=2
>
> Need to remove 'assume_all_enabled' as mentioned, and have some check
> for whether AS_SDP is supported (and can be enabled later).
I'm thinking we should perhaps just enable it always if
PCON+vrr.in_range or PR+AUX-less ALPM is possible. Although I'm
not sure if we're actually handling all the VRR details correctly
with PCONs, and not sure we even can due to the disaster that is
HDMI 2.1...
> To have this check is a bit challenging in intel_dp_sdp_min_guardband()
> since its called from
> intel_crtc_compute_config()->intel_vrr_compute_guardband() also, where
> we do not have obvious way to get intel_dp.
.compute_config() should be able to decide whether AS SDP is needed or
not.
--
Ville Syrjälä
Intel
