Re: [PATCH 01/12] drm/i915/vrr: Add HAS_EMP_AS_SDP_TL macro

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 4/13/2026 2:01 PM, Jani Nikula wrote:
> On Mon, 13 Apr 2026, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> 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.
> Okay, EMP AS SDP TL is where I draw the line. I don't understand this
> acronym soup anymore. HAS_EMP_AS_SDL_TL() is meaningless to me.

Hmm I was short of naming it HAS_PROGRAMMABLE_VRR_PACKET_TL() or 
something, but then it become overly long and perhaps TL might also be 
very contextual and not very clear.

So I went ahead with EMP_AS_SDP_TL to align with the register we are 
trying to program.

I also realize that I have not mentioned full forms EMP and SDP, which I 
should have atleast in the first patch, especially for EMP (Extended 
Metadata Packet coming from HDMI2.1).

Thanks for pointing it out. I will try to be bit more clearer with the 
naming and provide the full form atleast in the place where they are 
first used.


>
> The idea with the HAS_*() helpers is to make the code more *readable*.
>
> The absolute minimum is to explain what these acronyms mean in commit
> messages or comments, but you could just make the HAS_*() macro more
> readable on its own.
>
> I'm also not convinced we need to put all the HAS_*() macros in
> intel_display_device.h when we could place some of them inside the
> single .c file that uses them.


Currently this is used in couple of files, but I got your point, if this 
finally used only in intel_vrr.c I will place it in the same file.


Thanks & Regards,

Ankit

>
>
> BR,
> Jani.
>
>
>> Add a macro for this and use it in intel_vrr.c
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
>>   drivers/gpu/drm/i915/display/intel_vrr.c            | 2 +-
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index 1170ac346615..9338ea087e92 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -173,6 +173,7 @@ struct intel_display_platforms {
>>   #define HAS_DSC(__display)		(DISPLAY_RUNTIME_INFO(__display)->has_dsc)
>>   #define HAS_DSC_3ENGINES(__display)	(DISPLAY_VERx100(__display) == 1401 && HAS_DSC(__display))
>>   #define HAS_DSC_MST(__display)		(DISPLAY_VER(__display) >= 12 && HAS_DSC(__display))
>> +#define HAS_EMP_AS_SDP_TL(__display)	(DISPLAY_VERx100(__display) == 1401 || DISPLAY_VER(__display) >= 20)
>>   #define HAS_FBC(__display)		(DISPLAY_RUNTIME_INFO(__display)->fbc_mask != 0)
>>   #define HAS_FBC_DIRTY_RECT(__display)	(DISPLAY_VER(__display) >= 30)
>>   #define HAS_FBC_SYS_CACHE(__display)	(DISPLAY_VER(__display) >= 35 && !(__display)->platform.dgfx)
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index fae1186a90b2..1fed597439b0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -656,7 +656,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>>   	 * Since currently we support VRR only for DP/eDP, so this is programmed
>>   	 * to for Adaptive Sync SDP to Vsync start.
>>   	 */
>> -	if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >= 20)
>> +	if (HAS_EMP_AS_SDP_TL(display))
>>   		intel_de_write(display,
>>   			       EMP_AS_SDP_TL(display, cpu_transcoder),
>>   			       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));