Re: [RESEND PATCH 6/7] drm/dp: Add a helper to get the SDP type as a string

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

On 4/27/2026 6:27 PM, Ville Syrjälä wrote:
> On Fri, Apr 24, 2026 at 03:04:23PM +0530, Ankit Nautiyal wrote:
>> Introduce dp_sdp_type_get_name() to get the SDP type as a string.
>> Use this to log the SDP type based on the sdp_type fields of the
>> VSC and AS SDPs instead of the hardcoded strings.
>>
>> While at it, rename "SDP : AS_SDP" to "SDP : Adaptive Sync"
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/display/drm_dp_helper.c | 36 ++++++++++++++++++++++---
>>   1 file changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>> index e29958f8b0b6..102328d9022d 100644
>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>> @@ -3487,10 +3487,38 @@ static const char *dp_content_type_get_name(enum dp_content_type content_type)
>>   	}
>>   }
>>   
>> +static const char *dp_sdp_type_get_name(unsigned char type)
> I dislike this "_get_" stuff in the name. It's redundant, and can be
> confused with some refcounting related get/put. I really wish people
> would stop using it...

Noted.


>
> But looks like all the other similar things here already use the "_get_"
> naming, so I guess we should stick to a consistent style for now.

I will retain the name, but I understand your point. Will use *_get_* 
only for helpers that are in context of refcounting.


>
>> +{
>> +	switch (type) {
>> +	case DP_SDP_AUDIO_TIMESTAMP:
>> +		return "Audio Timestamp";
> "Audio_TimeStamp" to match the spec.
>
>> +	case DP_SDP_AUDIO_STREAM:
>> +		return "Audio Stream";
> "Audio_Stream"
>
>> +	case DP_SDP_EXTENSION:
>> +		return "Extension";
>> +	case DP_SDP_AUDIO_COPYMANAGEMENT:
>> +		return "Audio Copy Management";
> "Audio_CopyManagement"
>
>> +	case DP_SDP_ISRC:
>> +		return "ISRC";
>> +	case DP_SDP_VSC:
>> +		return "VSC";
>> +	case DP_SDP_PPS:
>> +		return "PPS";
>> +	case DP_SDP_VSC_EXT_VESA:
>> +		return "VSC EXT VESA";
>> +	case DP_SDP_VSC_EXT_CEA:
>> +		return "VSC EXT CEA";
> Looks like the spec does favor the acronym form for all of
> those. So I guess that's what we want here as well. The ' '
> should all be '_' to match the spec though.
>
>> +	case DP_SDP_ADAPTIVE_SYNC:
>> +		return "Adaptive Sync";
> "Adaptive-Sync" seems to be the preferred form for this
> in the spec.


Thanks for taking time to fetch these from the spec.

I will change these as suggested.


Regards,

Ankit


>
>> +	default:
>> +		return "Unknown";
>> +	}
>> +}
>> +
>>   void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc)
>>   {
>> -	drm_printf(p, "DP SDP: VSC, revision %u, length %u\n",
>> -		   vsc->revision, vsc->length);
>> +	drm_printf(p, "DP SDP: %s, revision %u, length %u\n",
>> +		   dp_sdp_type_get_name(vsc->sdp_type), vsc->revision, vsc->length);
>>   	drm_printf(p, "    pixelformat: %s\n",
>>   		   dp_pixelformat_get_name(vsc->pixelformat));
>>   	drm_printf(p, "    colorimetry: %s\n",
>> @@ -3505,8 +3533,8 @@ EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>>   
>>   void drm_dp_as_sdp_log(struct drm_printer *p, const struct drm_dp_as_sdp *as_sdp)
>>   {
>> -	drm_printf(p, "DP SDP: AS_SDP, revision %u, length %u\n",
>> -		   as_sdp->revision, as_sdp->length);
>> +	drm_printf(p, "DP SDP: %s, revision %u, length %u\n",
>> +		   dp_sdp_type_get_name(as_sdp->sdp_type), as_sdp->revision, as_sdp->length);
>>   	drm_printf(p, "    vtotal: %d\n", as_sdp->vtotal);
>>   	drm_printf(p, "    target_rr: %d\n", as_sdp->target_rr);
>>   	drm_printf(p, "    duration_incr_ms: %d\n", as_sdp->duration_incr_ms);
>> -- 
>> 2.45.2