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 <[email protected]>
> >
> > >
> > > Fixes: 0c9f9ca99238 ("drm/amd/display: Add common modes to analog
> > > displays without EDID")
> > > Cc: Timur Kristóf <[email protected]>
> > > Signed-off-by: Harry Wentland <[email protected]>
> > > ---
> > > 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.
> > > */
