On 3/5/2025 17:04, Alex Deucher wrote:
On Wed, Mar 5, 2025 at 4:53 PM Mario Limonciello
<mario.limonciello@xxxxxxx> 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@xxxxxxx>
---
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;
}
