[PATCH] drm/amd/display: Fix null pointer on analog detection

[PATCH] drm/amd/display: Fix null pointer on analog detection

[Harry Wentland]

Check if we have an amdgpu_dm_connector->dc_sink first before
adding common modes for analog outputs. If we don't have a
sink yet we can safely skip this.

Fixes: 0c9f9ca99238 ("drm/amd/display: Add common modes to analog displays without EDID")
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 c621e50f636a..bc5a8b8442a2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8669,7 +8669,7 @@ static int amdgpu_dm_connector_get_modes(struct drm_connector *connector)
 			amdgpu_dm_connector->num_modes +=
 				drm_add_modes_noedid(connector, 1920, 1080);
 
-		if (amdgpu_dm_connector->dc_sink->edid_caps.analog) {
+		if (amdgpu_dm_connector->dc_sink && amdgpu_dm_connector->dc_sink->edid_caps.analog) {
 			/* Analog monitor connected by DAC load detection.
 			 * Add common modes. It will be up to the user to select one that works.
 			 */
-- 
2.51.2

Re: [PATCH] drm/amd/display: Fix null pointer on analog detection

[Timur Kristóf]

On Wed, 2025-10-29 at 12:16 -0400, Harry Wentland wrote:
> Check if we have an amdgpu_dm_connector->dc_sink first before
> adding common modes for analog outputs. If we don't have a
> sink yet we can safely skip this.

Hi Harry, nice catch!

Out of curiosity, how is it possible to reach this code when the
dc_sink is NULL?

The patch is:
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>

> 
> Fixes: 0c9f9ca99238 ("drm/amd/display: Add common modes to analog
> displays without EDID")
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 c621e50f636a..bc5a8b8442a2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8669,7 +8669,7 @@ static int amdgpu_dm_connector_get_modes(struct
> drm_connector *connector)
>  			amdgpu_dm_connector->num_modes +=
>  				drm_add_modes_noedid(connector,
> 1920, 1080);
>  
> -		if (amdgpu_dm_connector->dc_sink->edid_caps.analog)
> {
> +		if (amdgpu_dm_connector->dc_sink &&
> amdgpu_dm_connector->dc_sink->edid_caps.analog) {
>  			/* Analog monitor connected by DAC load
> detection.
>  			 * Add common modes. It will be up to the
> user to select one that works.
>  			 */

Re: [PATCH] drm/amd/display: Fix null pointer on analog detection

[Harry Wentland]

On 2025-10-29 12:54, Timur Kristóf wrote:
> On Wed, 2025-10-29 at 12:16 -0400, Harry Wentland wrote:
>> Check if we have an amdgpu_dm_connector->dc_sink first before
>> adding common modes for analog outputs. If we don't have a
>> sink yet we can safely skip this.
> 
> Hi Harry, nice catch!
> 
> Out of curiosity, how is it possible to reach this code when the
> dc_sink is NULL?
> 

Huh, that was a bit of a goose chase.

amdgpu_dm_init
> amdgpu_dm_initialize_drm_device
> amdgpu_set_panel_orientation
> amdgpu_dm_connector_get_modes

This will run even on disconnected (i.e., without dc_sink)
connectors.

Maybe amdgpu_set_panel_orientation had better move into
this hunk after the amdgpu_dm_update_connector_after_detect:

	ret = dc_link_detect(link, DETECT_REASON_BOOT);
	mutex_unlock(&dm->dc_lock);
	printk("hwhw: %s %d\n", __func__, __LINE__);

	if (ret) {
		amdgpu_dm_update_connector_after_detect(aconnector);
		printk("hwhw: %s %d\n", __func__, __LINE__);

Either way, the NULL check is still prudent.

Harry

> The patch is:
> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> 
>>
>> Fixes: 0c9f9ca99238 ("drm/amd/display: Add common modes to analog
>> displays without EDID")
>> Cc: Timur Kristóf <timur.kristof@gmail.com>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> 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 c621e50f636a..bc5a8b8442a2 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -8669,7 +8669,7 @@ static int amdgpu_dm_connector_get_modes(struct
>> drm_connector *connector)
>>  			amdgpu_dm_connector->num_modes +=
>>  				drm_add_modes_noedid(connector,
>> 1920, 1080);
>>  
>> -		if (amdgpu_dm_connector->dc_sink->edid_caps.analog)
>> {
>> +		if (amdgpu_dm_connector->dc_sink &&
>> amdgpu_dm_connector->dc_sink->edid_caps.analog) {
>>  			/* Analog monitor connected by DAC load
>> detection.
>>  			 * Add common modes. It will be up to the
>> user to select one that works.
>>  			 */

Re: [PATCH] drm/amd/display: Fix null pointer on analog detection

[Timur Kristóf]

On Wed, 2025-10-29 at 14:29 -0400, Harry Wentland wrote:
> 
> 
> On 2025-10-29 12:54, Timur Kristóf wrote:
> > On Wed, 2025-10-29 at 12:16 -0400, Harry Wentland wrote:
> > > Check if we have an amdgpu_dm_connector->dc_sink first before
> > > adding common modes for analog outputs. If we don't have a
> > > sink yet we can safely skip this.
> > 
> > Hi Harry, nice catch!
> > 
> > Out of curiosity, how is it possible to reach this code when the
> > dc_sink is NULL?
> > 
> 
> Huh, that was a bit of a goose chase.
> 
> amdgpu_dm_init
> > amdgpu_dm_initialize_drm_device
> > amdgpu_set_panel_orientation
> > amdgpu_dm_connector_get_modes
> 
> This will run even on disconnected (i.e., without dc_sink)
> connectors.

Interesting. I hadn't encountered this code path during my testing, so
I'm a bit baffled how you could trigger that.

I'm sorry for the goose chase, but glad you found a solution.

> 
> Maybe amdgpu_set_panel_orientation had better move into
> this hunk after the amdgpu_dm_update_connector_after_detect:
> 
> 	ret = dc_link_detect(link, DETECT_REASON_BOOT);
> 	mutex_unlock(&dm->dc_lock);
> 	printk("hwhw: %s %d\n", __func__, __LINE__);
> 
> 	if (ret) {
> 		amdgpu_dm_update_connector_after_detect(aconnector);
> 		printk("hwhw: %s %d\n", __func__, __LINE__);
> 
> Either way, the NULL check is still prudent.

Yes, I agree with the NULL check. No argument there.
Also agree with the suggestion above.

Thanks & best regards,
Timur



> > The patch is:
> > Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> > 
> > > 
> > > Fixes: 0c9f9ca99238 ("drm/amd/display: Add common modes to analog
> > > displays without EDID")
> > > Cc: Timur Kristóf <timur.kristof@gmail.com>
> > > Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > 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 c621e50f636a..bc5a8b8442a2 100644
> > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > @@ -8669,7 +8669,7 @@ static int
> > > amdgpu_dm_connector_get_modes(struct
> > > drm_connector *connector)
> > >  			amdgpu_dm_connector->num_modes +=
> > >  				drm_add_modes_noedid(connector,
> > > 1920, 1080);
> > >  
> > > -		if (amdgpu_dm_connector->dc_sink-
> > > >edid_caps.analog)
> > > {
> > > +		if (amdgpu_dm_connector->dc_sink &&
> > > amdgpu_dm_connector->dc_sink->edid_caps.analog) {
> > >  			/* Analog monitor connected by DAC load
> > > detection.
> > >  			 * Add common modes. It will be up to
> > > the
> > > user to select one that works.
> > >  			 */