Re: [PATCH 3/5] drm/i915/dp: Add helper for AS SDP TL and fix documentation

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

On 3/11/2026 5:34 PM, Ville Syrjälä wrote:
> On Wed, Mar 11, 2026 at 05:24:22PM +0530, Nautiyal, Ankit K wrote:
>> On 3/10/2026 2:38 PM, Ville Syrjälä wrote:
>>> On Fri, Mar 06, 2026 at 02:02:09PM +0200, Ville Syrjälä wrote:
>>>> On Thu, Mar 05, 2026 at 09:31:16AM +0530, Ankit Nautiyal wrote:
>>>>> Add a helper, intel_dp_emp_as_sdp_tl(), to compute the EMP_AS_SDP_TL
>>>>> value used when programming the double‑buffering point and transmission
>>>>> line for VRR packets.
>>>>> Also improve the documentation: the AS SDP transmission line corresponds
>>>>> to the T1 position, which maps to the start of the Vsync pulse.
>>>>>
>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++++++
>>>>>    drivers/gpu/drm/i915/display/intel_dp.h  | 1 +
>>>>>    drivers/gpu/drm/i915/display/intel_vrr.c | 4 ++--
>>>>>    3 files changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> index 86390553800d..9204a813639a 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> @@ -7288,6 +7288,15 @@ bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
>>>>>    	return true;
>>>>>    }
>>>>>    
>>>>> +int intel_dp_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
>>>>> +{
>>>>> +	/*
>>>>> +	 * EMP_AS_SDP_TL defines the T1 position : The default AS SDP position
>>>>> +	 * that corresponds to the start of the Vsync pulse.
>>>>> +	 */
>>>>> +	return crtc_state->vrr.vsync_start;
>>>>> +}
>>>> Other parts of the code (eg. ALPM) still just directly use the
>>>> adjusted_mode timings to calculate the same stuff. So this doesn't
>>>> really seem to help us.
>>>>
>>>> Feels like all of our abstractions around this SDP transmission line
>>>> stuff are way too low level, and thus the same information is
>>>> calculated in different ways in different parts of the code. There
>>>> should be a single place that defines the transmission line(s),
>>>> and everyone should just consult that stuff (regardless of whether
>>>> the platform uses implicit transmission lines, EMP_AS_SDP_TL, or
>>>> the new stuff).
>>> I think instead of tracking the low level stagger values directly, what
>>> we want to track is just the transmission line itself for each type of
>>> SDP. That seems like a form that is easier to use elsewhere in the code.
>> Agreed storing TL will be easier, stagger values can be derived from that.
>>
>> So we set the transmission line for the packets only if we plan to send
>> them right?
> I don't see why we wouldn't just always set them. And if we set one we
> have to anyway set all of them since they're all derived from that one
> pair of registers.

Hmm alright, I will take care of this in the next revision.

(I will be Off for a few days, will come back to it in couple of weeks).

Thanks Ville for all the inputs.


Regards,

Ankit

>
>>
>>> And to accommodate VRR I suppose we should use the "transmission line
>>> is specified relative to the end of vblank/vtotal" convention for all
>>> of these.
>> Makes sense.
>>
>>
>> Regards,
>>
>> Ankit
>>