On 10/24/2025 6:09 PM, Ville Syrjälä wrote:
On Thu, Oct 23, 2025 at 06:08:47PM +0300, 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.
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.
Ah, apparently we have to always put t1 into EMP_AS_SDP_TL, and
there's some other logic for panel replay to use t2 when possible.
But since it's all dynamic we have to use t1 when calculating the
guardband/SCL.
Yeah t2 will be taken up by HW based on PR_ALPM_CTL adaptive sync SDP
position (to have either t1/t2 or t1 always or t2 always) and also what
we send in AS SDP payload wrt to Fixed Vtotal, Fixed AVG VTotal mode.
I agree with your suggestion above. For PCON, VRR support still needs
work so perhaps we can skip that. Makes sense to always enable AS SDP
when Panel replay with Auxless-ALPM is supported.
With that we can use Jouni's patch to increase the guardband such that
Vsync falls in the required region [1], effectively
`crtc_state->vrr.vsync_start + 1`.
I was trying with the above changes suggested, sent also to trybot [2].
But now I see that we have a problem with LRR on such panels.
When switching to lower mode with LRR, the `vsync_start` changes, which
results in guardband being changed.
So seamless switch to the required lower mode cannot take place. :-(
[1] https://patchwork.freedesktop.org/patch/682984/?series=156341&rev=2
[2] https://patchwork.freedesktop.org/series/156569/
Regards,
Ankit
