[PATCH 01/20] drm/amd/display: Determine DRM connector type more accurately

[Harry Wentland]

On 2025-07-30 13:03, Timur Kristóf wrote:
> On Wed, 2025-07-30 at 10:30 -0400, Harry Wentland wrote:
>>
>>
>> On 2025-07-30 03:40, Timur Kristóf wrote:
>>> On Tue, 2025-07-29 at 14:03 -0400, Harry Wentland wrote:
>>>>
>>>>
>>>> On 2025-07-23 11:57, Timur Kristóf wrote:
>>>>> Previously, DC determined the DRM connector type based on the
>>>>> signal type, which becomes problematic when a connector may
>>>>> support different signal types, such as DVI-I.
>>>>>
>>>>> With this patch, it is now determined according to the actual
>>>>> connector type in DC, meaning it can now distinguish between
>>>>> DVI-D and DVI-I connectors.
>>>>>
>>>>> A subsequent commit will enable polling for these connectors.
>>>>>
>>>>> Signed-off-by: Timur Kristóf <timur.kristof at gmail.com>
>>>>> ---
>>>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28
>>>>> +++++++++++----
>>>>> ----
>>>>>  1 file changed, 16 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> index 096b23ad4845..c347b232ae06 100644
>>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>>> @@ -8038,24 +8038,26 @@ static int
>>>>> dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state
>>>>> *state,
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> -static int to_drm_connector_type(enum signal_type st)
>>>>> +static int to_drm_connector_type(uint32_t connector_id)
>>>>>  {
>>>>> -	switch (st) {
>>>>> -	case SIGNAL_TYPE_HDMI_TYPE_A:
>>>>> +	switch (connector_id) {
>>>>> +	case CONNECTOR_ID_HDMI_TYPE_A:
>>>>>  		return DRM_MODE_CONNECTOR_HDMIA;
>>>>> -	case SIGNAL_TYPE_EDP:
>>>>> +	case CONNECTOR_ID_EDP:
>>>>>  		return DRM_MODE_CONNECTOR_eDP;
>>>>> -	case SIGNAL_TYPE_LVDS:
>>>>> +	case CONNECTOR_ID_LVDS:
>>>>>  		return DRM_MODE_CONNECTOR_LVDS;
>>>>> -	case SIGNAL_TYPE_RGB:
>>>>> +	case CONNECTOR_ID_VGA:
>>>>>  		return DRM_MODE_CONNECTOR_VGA;
>>>>> -	case SIGNAL_TYPE_DISPLAY_PORT:
>>>>> -	case SIGNAL_TYPE_DISPLAY_PORT_MST:
>>>>> +	case CONNECTOR_ID_DISPLAY_PORT:
>>>>>  		return DRM_MODE_CONNECTOR_DisplayPort;
>>>>> -	case SIGNAL_TYPE_DVI_DUAL_LINK:
>>>>> -	case SIGNAL_TYPE_DVI_SINGLE_LINK:
>>>>> +	case CONNECTOR_ID_SINGLE_LINK_DVID:
>>>>> +	case CONNECTOR_ID_DUAL_LINK_DVID:
>>>>>  		return DRM_MODE_CONNECTOR_DVID;
>>>>> -	case SIGNAL_TYPE_VIRTUAL:
>>>>> +	case CONNECTOR_ID_SINGLE_LINK_DVII:
>>>>> +	case CONNECTOR_ID_DUAL_LINK_DVII:
>>>>> +		return DRM_MODE_CONNECTOR_DVII;
>>>>> +	case CONNECTOR_ID_VIRTUAL:
>>>>>  		return DRM_MODE_CONNECTOR_VIRTUAL;
>>>>>  
>>>>>  	default:
>>>>> @@ -8440,6 +8442,8 @@ void
>>>>> amdgpu_dm_connector_init_helper(struct
>>>>> amdgpu_display_manager *dm,
>>>>>  			link->link_enc-
>>>>>> features.dp_ycbcr420_supported ? true : false;
>>>>>  		break;
>>>>>  	case DRM_MODE_CONNECTOR_DVID:
>>>>> +	case DRM_MODE_CONNECTOR_DVII:
>>>>> +	case DRM_MODE_CONNECTOR_VGA:
>>>>
>>>> This seems unrelated and would do better in a separate patch.
>>>>
>>>> Harry
>>>
>>> Keep in mind that currently DC recognizes DVI-I as DVI-D, but after
>>> this patch they will be recognized correctly as DVI-I. So without
>>> this
>>> part, the patch will regress the hotplug capability of those ports.
>>>
>>
>> How will it regress when your patch series introduces analog support?
>> Without that there shouldn't be DVII or VGA connectors.
>>
>> Harry
> 
> DVI-I can carry either an analog or a digital signal.
> 
> Before this patch, the DVI-I ports were recognized by DRM as
> DRM_MODE_CONNECTOR_DVID and their digital part was already working.
> That means you can plug in a digital DVI cable in a DVI-I port and it
> will work today.
> 
> After this patch they will be recognized as DRM_MODE_CONNECTOR_DVII
> instead, so in order to keep the same behaviour as before and avoid
> regressing the digital part, we need to set DRM_CONNECTOR_POLL_HPD, and
> in order to do that we need to add DRM_MODE_CONNECTOR_DVII to this
> switch statement.
> 

Thanks for the explanation. In that case the changes are all related
and it's probably easier to keep them together.

Reviewed-by: Harry Wentland <harry.wentland at amd.com>

Harry

> That being said, I can move this hunk to a separate patch before this
> one. That way there is going to be no regression and this patch stays
> simpler.
> 
> Does that sound ok to you?
> 
> Timur
> 
>>
>>> That said, sure, I can move this part to a separate commit before
>>> this
>>> one, if you prefer.
>>>
>>> Timur
>>>
>>>>
>>>>>  		aconnector->base.polled =
>>>>> DRM_CONNECTOR_POLL_HPD;
>>>>>  		break;
>>>>>  	default:
>>>>> @@ -8631,7 +8635,7 @@ static int
>>>>> amdgpu_dm_connector_init(struct
>>>>> amdgpu_display_manager *dm,
>>>>>  		goto out_free;
>>>>>  	}
>>>>>  
>>>>> -	connector_type = to_drm_connector_type(link-
>>>>>> connector_signal);
>>>>> +	connector_type = to_drm_connector_type(link-
>>>>>> link_id.id);
>>>>>  
>>>>>  	res = drm_connector_init_with_ddc(
>>>>>  			dm->ddev,