Re: [PATCH] drm/i915/vrr: Program EMP_AS_SDP_TL for DP AS SDP

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 01 May 2025, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 4/30/2025 5:12 PM, Jani Nikula wrote:
>> On Tue, 29 Apr 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>>> The register EMP_AS_SDP_TL (MTL) was introduced for configuring the
>>> double buffering point and transmission line for
>>> HDMI Video Timing Extended Metadata Packet (VTEMP) for VRR.
>>> This was also intended to be configured for DP to HDMI2.1 PCON to
>>> support VRR.
>>>
>>>  From BMG and LNL+ onwards, this register was extended to Display Port
>>> Adaptive Sync SDP to have a common register to configure double
>>> buffering point and transmission line for both HDMI and DP VRR related
>>> packets.
>>>
>>> Currently, we do not support VRR for either native HDMI or via PCON.
>>> However we need to configure this for DP SDP case. As per the spec,
>>> program the register to set Vsync start as the double buffering point
>>> for DP AS SDP.
>>>
>>> Bspec:70984, 71197
>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_vrr.c      | 20 +++++++++++++++++++
>>>   drivers/gpu/drm/i915/display/intel_vrr_regs.h |  6 ++++++
>>>   2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>>> index c6565baf815a..2447bdfde5af 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>>> @@ -576,6 +576,22 @@ bool intel_vrr_always_use_vrr_tg(struct intel_display *display)
>>>   	return false;
>>>   }
>>>   
>>> +static
>>> +void intel_vrr_set_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
>> How do you pronounce that function name?
>
> Haha, good point!. Now that you have pointed it out, this sounds very 
> silly and particularly difficult to read aloud. (^_^;)
>
> Perhaps intel_vrr_set_packet_transmission_line would be better.

Sounds good to me. I didn't check against what the function actually
does. But it sound better. ;D

BR,
Jani.



>
>
> Regards,
>
> Ankit
>
>>
>> BR,
>> Jani.
>>
>>> +{
>>> +	struct intel_display *display = to_intel_display(crtc_state);
>>> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>> +
>>> +	/*
>>> +	 * For BMG and LNL+ onwards the EMP_AS_SDP_TL is used for programming
>>> +	 * double buffering point and transmission line for Adaptive Sync SDP.
>>> +	 */
>>> +	if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >= 20)
>>> +		intel_de_write(display,
>>> +			       EMP_AS_SDP_TL(display, cpu_transcoder),
>>> +			       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
>>> +}
>>> +
>>>   void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>>>   {
>>>   	struct intel_display *display = to_intel_display(crtc_state);
>>> @@ -595,6 +611,8 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>>>   		       TRANS_PUSH_EN);
>>>   
>>>   	if (!intel_vrr_always_use_vrr_tg(display)) {
>>> +		intel_vrr_set_emp_as_sdp_tl(crtc_state);
>>> +
>>>   		if (crtc_state->cmrr.enable) {
>>>   			intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
>>>   				       VRR_CTL_VRR_ENABLE | VRR_CTL_CMRR_ENABLE |
>>> @@ -646,6 +664,8 @@ void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state)
>>>   	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
>>>   		       TRANS_PUSH_EN);
>>>   
>>> +	intel_vrr_set_emp_as_sdp_tl(crtc_state);
>>> +
>>>   	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
>>>   		       VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state));
>>>   }
>>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr_regs.h b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
>>> index 6ed0e0dc97e7..d2af1b6710bf 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_vrr_regs.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
>>> @@ -108,6 +108,12 @@
>>>   #define VRR_VSYNC_START_MASK			REG_GENMASK(12, 0)
>>>   #define VRR_VSYNC_START(vsync_start)		REG_FIELD_PREP(VRR_VSYNC_START_MASK, (vsync_start))
>>>   
>>> +/* Common register for HDMI VTEMP and DP AS SDP */
>>> +#define _EMP_AS_SDP_TL_A			0x60204
>>> +#define EMP_AS_SDP_DB_TL_MASK			REG_GENMASK(12, 0)
>>> +#define EMP_AS_SDP_TL(dev_priv, trans)		_MMIO_TRANS2(dev_priv, trans, _EMP_AS_SDP_TL_A)
>>> +#define EMP_AS_SDP_DB_TL(db_transmit_line)	REG_FIELD_PREP(EMP_AS_SDP_DB_TL_MASK, (db_transmit_line))
>>> +
>>>   /*CMRR Registers*/
>>>   
>>>   #define _TRANS_CMRR_M_LO_A			0x604F0

-- 
Jani Nikula, Intel