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

[Mario Limonciello]

On 3/5/2025 17:04, Alex Deucher wrote:
> 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.

Actually it's a very different result with this patch vs a warning.  By 
failing to initialize then the existing efifb keeps working.

This means you can boot with amdgpu.dc=0 and rather than get a frozen 
display you end up unaccelerated graphics.

> 
> 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;
>>>    }
>>>
>>