[PATCH] drm/amd: Fail initialization earlier when DC is disabled

[Alex Deucher]

On Wed, Mar 5, 2025 at 4:53 PM Mario Limonciello
<mario.limonciello at amd.com> wrote:
>
> On 3/5/2025 15:37, Mario Limonciello wrote:
> > Modern APU and dGPU require DC support to be able to light up the
> > display.  If DC support has been disabled either by kernel config
> > or by kernel command line fail init early so that the system won't
> > freeze with a lack of display.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 24 +++++++++++++++----
> >   1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index a4258127083d..c4e1505dcaac 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -2139,10 +2139,6 @@ static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> >               return 0;
> >       }
> >
> > -     if (!amdgpu_device_has_dc_support(adev))
> > -             return 0;
> > -
> > -#if defined(CONFIG_DRM_AMD_DC)
> >       if (amdgpu_ip_version(adev, DCE_HWIP, 0)) {
> >               switch (amdgpu_ip_version(adev, DCE_HWIP, 0)) {
> >               case IP_VERSION(1, 0, 0):
> > @@ -2166,15 +2162,24 @@ static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> >               case IP_VERSION(3, 5, 1):
> >               case IP_VERSION(3, 6, 0):
> >               case IP_VERSION(4, 1, 0):
> > +                     if (!amdgpu_device_has_dc_support(adev)) {
> > +                             dev_err(adev->dev,
> > +                                     "DC support is required for dm ip block(DCE_HWIP:0x%x)\n",
> > +                                     amdgpu_ip_version(adev, DCE_HWIP, 0));
> > +                             return -EINVAL;
> > +                     }
> > +
> >                       /* TODO: Fix IP version. DC code expects version 4.0.1 */
> >                       if (adev->ip_versions[DCE_HWIP][0] == IP_VERSION(4, 1, 0))
> >                               adev->ip_versions[DCE_HWIP][0] = IP_VERSION(4, 0, 1);
> >
> > +#if defined(CONFIG_DRM_AMD_DC)
> >                       if (amdgpu_sriov_vf(adev))
> >                               amdgpu_discovery_set_sriov_display(adev);
> >                       else
> >                               amdgpu_device_ip_block_add(adev, &dm_ip_block);
> >                       break;
> > +#endif
> >               default:
>
> Looking closer at this failure path I *think* this patch will cause
> issues on GPU without DC support.
>
> In the 'default' case now I think it should do something like this:
>
> if (amdgpu_device_has_dc_support(adev))
>         //existing error flow
> else
>         return 0;
>
> Agree?

I think it would be better to just dump a warning in the log if the
board supports displays but CONFIG_DRM_AMD_DC=n rather than failing to
initialize the driver.  The end result is pretty much the same and it
wouldn't break users that want this scenario for some reason.

Alex

>
> >                       dev_err(adev->dev,
> >                               "Failed to add dm ip block(DCE_HWIP:0x%x)\n",
> > @@ -2186,11 +2191,21 @@ static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> >               case IP_VERSION(12, 0, 0):
> >               case IP_VERSION(12, 0, 1):
> >               case IP_VERSION(12, 1, 0):
> > +
> > +             if (!amdgpu_device_has_dc_support(adev)) {
> > +                     dev_err(adev->dev,
> > +                             "DC support is required for dm ip block(DCI_HWIP:0x%x)\n",
> > +                             amdgpu_ip_version(adev, DCI_HWIP, 0));
> > +                     return -EINVAL;
> > +             }
> > +
> > +#if defined(CONFIG_DRM_AMD_DC)
> >                       if (amdgpu_sriov_vf(adev))
> >                               amdgpu_discovery_set_sriov_display(adev);
> >                       else
> >                               amdgpu_device_ip_block_add(adev, &dm_ip_block);
> >                       break;
> > +#endif
> >               default:
> >                       dev_err(adev->dev,
> >                               "Failed to add dm ip block(DCI_HWIP:0x%x)\n",
> > @@ -2198,7 +2213,6 @@ static int amdgpu_discovery_set_display_ip_blocks(struct amdgpu_device *adev)
> >                       return -EINVAL;
> >               }
> >       }
> > -#endif
> >       return 0;
> >   }
> >
>