[PATCH] drm/i915/dip: Write the DIP header last with SDP transmission lines

[PATCH] drm/i915/dip: Write the DIP header last with SDP transmission lines

[Chaitanya Kumar Borah]

On platforms with SDP transmission line registers, the packets anchored
to them are double buffered in the transcoder (TC) clock domain. The packet
registers are in the core display (CD) clock domain and the whole packet is
clock crossed on the write of the header (dword 0), so Bspec requires the
data to be programmed first and the header last.

hsw_write_infoframe() writes the header first, so the packet is crossed
before its payload is in place and the previous payload keeps being sent.
This hits the AS SDP, and the VSC SDP whenever an AS SDP is enabled, which
is whenever VRR is possible since commit 6a1712052859 ("drm/i915/dp: Enable
AS SDP whenever VRR is possible or PR !async"). With CMN_SDP_TL, enabled by
commit bfa597238073 ("drm/i915/dip: Enable Common SDP Transmission line"),
it also hits the GMP and VSC SDPs: on NVL, entering HDR on an eDP panel
leaves the image discoloured and scaled until the headers are rewritten.

Write the payload, including the ECC zero fill, first and the header last
on platforms with EMP_AS_SDP_TL. CD domain packets have no ordering
requirement and Bspec recommends the same order for all packets, so use it
for all of them. Older platforms keep the current order.

Bspec: 75640, 71197, 74384
Fixes: e1123e617e51 ("drm/i915/vrr: Program EMP_AS_SDP_TL for DP AS SDP")
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17732
Cc: <stable@vger.kernel.org> # v6.17+
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
---

Notes:
    Fixes tag rationale
    
    The Fixes tag points at commit e1123e617e51 ("drm/i915/vrr: Program
    EMP_AS_SDP_TL for DP AS SDP"). The AS SDP has been written header first
    since it was added by commit 8845021b7331 ("drm/i915/display: Read/Write
    Adaptive Sync SDP"), but EMP_AS_SDP_TL, which holds the double buffer
    point and the transmission line of the AS SDP on BMG and LNL+, was only
    programmed from that commit on. From then on the driver relies on that
    register to anchor the AS SDP, and the VSC SDP whenever an AS SDP is
    enabled, and the programming note of the register asks for the packet
    data first and the header last. The two later commits cited in the commit
    message only widened the exposure: the first enabled the AS SDP whenever
    VRR is possible and the second added CMN_SDP_TL for the GMP and VSC SDPs.
    
    Alternatives considered:
     - 6a1712052859 (v7.2): AS SDP on whenever VRR is possible. The PTL
       report is fixed by reverting it. Narrower than the spec argument, as
       it only covers v7.2 and later.
     - bfa597238073 (7.4): CMN_SDP_TL. The NVL regression, verified on
       hardware, but covered by the older tag as every kernel with it also
       has e1123e617e51.
     - 8845021b7331 (v6.10): first header first AS SDP write, but
       EMP_AS_SDP_TL was not programmed until e1123e617e51, so exposure
       before it could not be shown.
    
    Not verified:
     - The failure was observed on NVL with CMN_SDP_TL (rewriting the GMP and
       VSC headers makes the panel recover). The AS SDP and the VSC SDP with
       an AS SDP on BMG, LNL and PTL follow from the Bspec requirement and
       were not reproduced here.
     - No reproduction on kernels older than v7.2.
     - Stable backports need the HAS_EMP_AS_SDP_TL gate adapted, as it is
       defined in intel_dip.h, which is new in 7.4.

 drivers/gpu/drm/i915/display/intel_hdmi.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 4ec0db8d93fb..3ecf017b3176 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -51,6 +51,7 @@
 #include "intel_cx0_phy.h"
 #include "intel_ddi.h"
 #include "intel_de.h"
+#include "intel_dip.h"
 #include "intel_display_driver.h"
 #include "intel_display_regs.h"
 #include "intel_display_types.h"
@@ -524,6 +525,15 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
 	const u32 *data = frame;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	intel_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(display, cpu_transcoder);
+	/*
+	 * On platforms with the SDP transmission line registers some of the
+	 * packets are double buffered in the transcoder clock domain, and the
+	 * packet is clock crossed on the write of the header (dword 0). Program
+	 * the payload first and the header last, so that the whole packet gets
+	 * crossed. Packets double buffered in the core display clock domain
+	 * have no ordering requirement, so use the same order for all of them.
+	 */
+	bool header_last = HAS_EMP_AS_SDP_TL(display);
 	int data_size;
 	int i;
 	u32 val = intel_de_read(display, ctl_reg);
@@ -535,18 +545,21 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
 	val &= ~hsw_infoframe_enable(type);
 	intel_de_write(display, ctl_reg, val);
 
-	for (i = 0; i < len; i += 4) {
+	for (i = header_last ? 4 : 0; i < len; i += 4)
 		intel_de_write(display,
 			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
-			       *data);
-		data++;
-	}
+			       data[i >> 2]);
 	/* Write every possible data byte to force correct ECC calculation. */
 	for (; i < data_size; i += 4)
 		intel_de_write(display,
 			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
 			       0);
 
+	if (header_last)
+		intel_de_write(display,
+			       hsw_dip_data_reg(display, cpu_transcoder, type, 0),
+			       data[0]);
+
 	/* Wa_14013475917 */
 	if (!(IS_DISPLAY_VER(display, 13, 14) && crtc_state->has_psr &&
 	      !crtc_state->has_panel_replay && type == DP_SDP_VSC))
-- 
2.50.1

Re: [PATCH] drm/i915/dip: Write the DIP header last with SDP transmission lines

[Ville Syrjälä]

On Wed, Nov 11, 2026 at 04:05:42PM +0530, Chaitanya Kumar Borah wrote:
> On platforms with SDP transmission line registers, the packets anchored
> to them are double buffered in the transcoder (TC) clock domain. The packet
> registers are in the core display (CD) clock domain and the whole packet is
> clock crossed on the write of the header (dword 0), so Bspec requires the
> data to be programmed first and the header last.
> 
> hsw_write_infoframe() writes the header first, so the packet is crossed
> before its payload is in place and the previous payload keeps being sent.
> This hits the AS SDP, and the VSC SDP whenever an AS SDP is enabled, which
> is whenever VRR is possible since commit 6a1712052859 ("drm/i915/dp: Enable
> AS SDP whenever VRR is possible or PR !async"). With CMN_SDP_TL, enabled by
> commit bfa597238073 ("drm/i915/dip: Enable Common SDP Transmission line"),
> it also hits the GMP and VSC SDPs: on NVL, entering HDR on an eDP panel
> leaves the image discoloured and scaled until the headers are rewritten.
> 
> Write the payload, including the ECC zero fill, first and the header last
> on platforms with EMP_AS_SDP_TL. CD domain packets have no ordering
> requirement and Bspec recommends the same order for all packets, so use it
> for all of them. Older platforms keep the current order.
> 
> Bspec: 75640, 71197, 74384
> Fixes: e1123e617e51 ("drm/i915/vrr: Program EMP_AS_SDP_TL for DP AS SDP")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17732
> Cc: <stable@vger.kernel.org> # v6.17+
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
> ---
> 
> Notes:
>     Fixes tag rationale
>     
>     The Fixes tag points at commit e1123e617e51 ("drm/i915/vrr: Program
>     EMP_AS_SDP_TL for DP AS SDP"). The AS SDP has been written header first
>     since it was added by commit 8845021b7331 ("drm/i915/display: Read/Write
>     Adaptive Sync SDP"), but EMP_AS_SDP_TL, which holds the double buffer
>     point and the transmission line of the AS SDP on BMG and LNL+, was only
>     programmed from that commit on. From then on the driver relies on that
>     register to anchor the AS SDP, and the VSC SDP whenever an AS SDP is
>     enabled, and the programming note of the register asks for the packet
>     data first and the header last. The two later commits cited in the commit
>     message only widened the exposure: the first enabled the AS SDP whenever
>     VRR is possible and the second added CMN_SDP_TL for the GMP and VSC SDPs.
>     
>     Alternatives considered:
>      - 6a1712052859 (v7.2): AS SDP on whenever VRR is possible. The PTL
>        report is fixed by reverting it. Narrower than the spec argument, as
>        it only covers v7.2 and later.
>      - bfa597238073 (7.4): CMN_SDP_TL. The NVL regression, verified on
>        hardware, but covered by the older tag as every kernel with it also
>        has e1123e617e51.
>      - 8845021b7331 (v6.10): first header first AS SDP write, but
>        EMP_AS_SDP_TL was not programmed until e1123e617e51, so exposure
>        before it could not be shown.
>     
>     Not verified:
>      - The failure was observed on NVL with CMN_SDP_TL (rewriting the GMP and
>        VSC headers makes the panel recover). The AS SDP and the VSC SDP with
>        an AS SDP on BMG, LNL and PTL follow from the Bspec requirement and
>        were not reproduced here.
>      - No reproduction on kernels older than v7.2.
>      - Stable backports need the HAS_EMP_AS_SDP_TL gate adapted, as it is
>        defined in intel_dip.h, which is new in 7.4.
> 
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 4ec0db8d93fb..3ecf017b3176 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -51,6 +51,7 @@
>  #include "intel_cx0_phy.h"
>  #include "intel_ddi.h"
>  #include "intel_de.h"
> +#include "intel_dip.h"
>  #include "intel_display_driver.h"
>  #include "intel_display_regs.h"
>  #include "intel_display_types.h"
> @@ -524,6 +525,15 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
>  	const u32 *data = frame;
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	intel_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(display, cpu_transcoder);
> +	/*
> +	 * On platforms with the SDP transmission line registers some of the
> +	 * packets are double buffered in the transcoder clock domain, and the
> +	 * packet is clock crossed on the write of the header (dword 0). Program
> +	 * the payload first and the header last, so that the whole packet gets
> +	 * crossed. Packets double buffered in the core display clock domain
> +	 * have no ordering requirement, so use the same order for all of them.
> +	 */
> +	bool header_last = HAS_EMP_AS_SDP_TL(display);
>  	int data_size;
>  	int i;
>  	u32 val = intel_de_read(display, ctl_reg);
> @@ -535,18 +545,21 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
>  	val &= ~hsw_infoframe_enable(type);
>  	intel_de_write(display, ctl_reg, val);
>  
> -	for (i = 0; i < len; i += 4) {
> +	for (i = header_last ? 4 : 0; i < len; i += 4)

IMO just always use the "write header last" approach. 

>  		intel_de_write(display,
>  			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
> -			       *data);
> -		data++;
> -	}
> +			       data[i >> 2]);

Making 'i' the dword index rather than the byte index might make this
code a bit nicer. The only slight concern is whether len%4!=0 might
be true. Maybe just have the caller always align it for good measure.
But let's stick to the byte based thing for now to avoid adding more
noise to this patch, and clean it up later.

>  	/* Write every possible data byte to force correct ECC calculation. */
>  	for (; i < data_size; i += 4)
>  		intel_de_write(display,
>  			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
>  			       0);
>  
> +	if (header_last)
> +		intel_de_write(display,
> +			       hsw_dip_data_reg(display, cpu_transcoder, type, 0),
> +			       data[0]);
> +
>  	/* Wa_14013475917 */
>  	if (!(IS_DISPLAY_VER(display, 13, 14) && crtc_state->has_psr &&
>  	      !crtc_state->has_panel_replay && type == DP_SDP_VSC))
> -- 
> 2.50.1

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915/dip: Write the DIP header last with SDP transmission lines

[Borah, Chaitanya Kumar]

On 11/11/2026 5:06 PM, Ville Syrjälä wrote:
> On Wed, Nov 11, 2026 at 04:05:42PM +0530, Chaitanya Kumar Borah wrote:
>> On platforms with SDP transmission line registers, the packets anchored
>> to them are double buffered in the transcoder (TC) clock domain. The packet
>> registers are in the core display (CD) clock domain and the whole packet is
>> clock crossed on the write of the header (dword 0), so Bspec requires the
>> data to be programmed first and the header last.
>>
>> hsw_write_infoframe() writes the header first, so the packet is crossed
>> before its payload is in place and the previous payload keeps being sent.
>> This hits the AS SDP, and the VSC SDP whenever an AS SDP is enabled, which
>> is whenever VRR is possible since commit 6a1712052859 ("drm/i915/dp: Enable
>> AS SDP whenever VRR is possible or PR !async"). With CMN_SDP_TL, enabled by
>> commit bfa597238073 ("drm/i915/dip: Enable Common SDP Transmission line"),
>> it also hits the GMP and VSC SDPs: on NVL, entering HDR on an eDP panel
>> leaves the image discoloured and scaled until the headers are rewritten.
>>
>> Write the payload, including the ECC zero fill, first and the header last
>> on platforms with EMP_AS_SDP_TL. CD domain packets have no ordering
>> requirement and Bspec recommends the same order for all packets, so use it
>> for all of them. Older platforms keep the current order.
>>
>> Bspec: 75640, 71197, 74384
>> Fixes: e1123e617e51 ("drm/i915/vrr: Program EMP_AS_SDP_TL for DP AS SDP")
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17732
>> Cc: <stable@vger.kernel.org> # v6.17+
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>
>> Notes:
>>      Fixes tag rationale
>>      
>>      The Fixes tag points at commit e1123e617e51 ("drm/i915/vrr: Program
>>      EMP_AS_SDP_TL for DP AS SDP"). The AS SDP has been written header first
>>      since it was added by commit 8845021b7331 ("drm/i915/display: Read/Write
>>      Adaptive Sync SDP"), but EMP_AS_SDP_TL, which holds the double buffer
>>      point and the transmission line of the AS SDP on BMG and LNL+, was only
>>      programmed from that commit on. From then on the driver relies on that
>>      register to anchor the AS SDP, and the VSC SDP whenever an AS SDP is
>>      enabled, and the programming note of the register asks for the packet
>>      data first and the header last. The two later commits cited in the commit
>>      message only widened the exposure: the first enabled the AS SDP whenever
>>      VRR is possible and the second added CMN_SDP_TL for the GMP and VSC SDPs.
>>      
>>      Alternatives considered:
>>       - 6a1712052859 (v7.2): AS SDP on whenever VRR is possible. The PTL
>>         report is fixed by reverting it. Narrower than the spec argument, as
>>         it only covers v7.2 and later.
>>       - bfa597238073 (7.4): CMN_SDP_TL. The NVL regression, verified on
>>         hardware, but covered by the older tag as every kernel with it also
>>         has e1123e617e51.
>>       - 8845021b7331 (v6.10): first header first AS SDP write, but
>>         EMP_AS_SDP_TL was not programmed until e1123e617e51, so exposure
>>         before it could not be shown.
>>      
>>      Not verified:
>>       - The failure was observed on NVL with CMN_SDP_TL (rewriting the GMP and
>>         VSC headers makes the panel recover). The AS SDP and the VSC SDP with
>>         an AS SDP on BMG, LNL and PTL follow from the Bspec requirement and
>>         were not reproduced here.
>>       - No reproduction on kernels older than v7.2.
>>       - Stable backports need the HAS_EMP_AS_SDP_TL gate adapted, as it is
>>         defined in intel_dip.h, which is new in 7.4.
>>
>>   drivers/gpu/drm/i915/display/intel_hdmi.c | 21 +++++++++++++++++----
>>   1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 4ec0db8d93fb..3ecf017b3176 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -51,6 +51,7 @@
>>   #include "intel_cx0_phy.h"
>>   #include "intel_ddi.h"
>>   #include "intel_de.h"
>> +#include "intel_dip.h"
>>   #include "intel_display_driver.h"
>>   #include "intel_display_regs.h"
>>   #include "intel_display_types.h"
>> @@ -524,6 +525,15 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
>>   	const u32 *data = frame;
>>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>   	intel_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(display, cpu_transcoder);
>> +	/*
>> +	 * On platforms with the SDP transmission line registers some of the
>> +	 * packets are double buffered in the transcoder clock domain, and the
>> +	 * packet is clock crossed on the write of the header (dword 0). Program
>> +	 * the payload first and the header last, so that the whole packet gets
>> +	 * crossed. Packets double buffered in the core display clock domain
>> +	 * have no ordering requirement, so use the same order for all of them.
>> +	 */
>> +	bool header_last = HAS_EMP_AS_SDP_TL(display);
>>   	int data_size;
>>   	int i;
>>   	u32 val = intel_de_read(display, ctl_reg);
>> @@ -535,18 +545,21 @@ void hsw_write_infoframe(struct intel_encoder *encoder,
>>   	val &= ~hsw_infoframe_enable(type);
>>   	intel_de_write(display, ctl_reg, val);
>>   
>> -	for (i = 0; i < len; i += 4) {
>> +	for (i = header_last ? 4 : 0; i < len; i += 4)
> 
> IMO just always use the "write header last" approach.
> 

Agreed. I was trying to keep the exposure minimal but this should be 
harmless given the registers are either not double-buffered or double 
buffered without an arming register in the older platforms.

>>   		intel_de_write(display,
>>   			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
>> -			       *data);
>> -		data++;
>> -	}
>> +			       data[i >> 2]);
> 
> Making 'i' the dword index rather than the byte index might make this
> code a bit nicer. The only slight concern is whether len%4!=0 might
> be true. Maybe just have the caller always align it for good measure.
> But let's stick to the byte based thing for now to avoid adding more
> noise to this patch, and clean it up later.
> 

Noted for later.

>>   	/* Write every possible data byte to force correct ECC calculation. */
>>   	for (; i < data_size; i += 4)
>>   		intel_de_write(display,
>>   			       hsw_dip_data_reg(display, cpu_transcoder, type, i >> 2),
>>   			       0);
>>   
>> +	if (header_last)
>> +		intel_de_write(display,
>> +			       hsw_dip_data_reg(display, cpu_transcoder, type, 0),
>> +			       data[0]);
>> +
>>   	/* Wa_14013475917 */
>>   	if (!(IS_DISPLAY_VER(display, 13, 14) && crtc_state->has_psr &&
>>   	      !crtc_state->has_panel_replay && type == DP_SDP_VSC))
>> -- 
>> 2.50.1
>