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

Mario Limonciello <mario.limonciello@xxxxxxx> · Thu, 6 Mar 2025 14:13:30 -0600

On 3/6/2025 14:11, Alex Deucher wrote:
On Thu, Mar 6, 2025 at 2:31 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:

On 3/6/2025 13:19, Alex Deucher wrote:
On Thu, Mar 6, 2025 at 1:58 PM Mario Limonciello
<mario.limonciello@xxxxxxx> 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 the screen will visibly freeze when the
driver finishes early init.

As it's known before early init is done whether DC support is required
detect this during discovery and bail if DC support was disabled
for any reason.  This will ensure that the existing framebuffer
provided by efifb or simpledrm keeps working.

I think there are a couple of corner cases we need to handle:
1. if adev->enable_virtual_display is set.  The user has configured
virtual displays and hence they want to use them rather than the
actual physical displays.  This is useful with GPUs in servers or for
early bring up.
  > 2. If the board supports DCN IP, but all it's been fused off due to>
silicon flaws (e.g., adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK).
In that case, we don't want to fail.

In that case I wonder if it's better to use
amdgpu_device_asic_has_dc_support() instead of
amdgpu_device_has_dc_support() which should cover both of those concerns.

That should work, or maybe just warn once in
amdgpu_device_asic_has_dc_support().  E.g., something like:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1b9b4f8daf531..c986e619dbe99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3988,6 +3988,8 @@ bool amdgpu_device_asic_has_dc_support(enum
amd_asic_type asic_type)
                  */
                 return amdgpu_dc > 0;
         default:
+               if (amdgpu_dc == 0)
+                       DRM_INFO_ONCE("Display Core has been disable
via kernel parameter, No display!\n");
                 return amdgpu_dc != 0;
  #else
         default:


The problem is without a display that message will probably not be seen 
unless someone knows to look for journalctl -k -b-1 or similar.

So my main concern is that people who shoot themselves in the foot at 
least have a display to see the hole in their foot.

I'll have a try with my other idea and follow up with a v3 if I'm happy 
with that.




Alex


Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v2:
   * Update commit message justification
   * Add correct "default" handling
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 46 +++++++++++++------
   1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index a4258127083d..24f532de6322 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,39 +2162,63 @@ 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:
-                       dev_err(adev->dev,
-                               "Failed to add dm ip block(DCE_HWIP:0x%x)\n",
-                               amdgpu_ip_version(adev, DCE_HWIP, 0));
-                       return -EINVAL;
+                       if (amdgpu_device_has_dc_support(adev)) {
+                               dev_err(adev->dev,
+                                       "Failed to add dm ip block(DCE_HWIP:0x%x)\n",
+                                       amdgpu_ip_version(adev, DCE_HWIP, 0));
+                               return -EINVAL;
+                       }
+                       return 0;
                  }
          } else if (amdgpu_ip_version(adev, DCI_HWIP, 0)) {
                  switch (amdgpu_ip_version(adev, DCI_HWIP, 0)) {
                  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",
-                               amdgpu_ip_version(adev, DCI_HWIP, 0));
-                       return -EINVAL;
+                       if (amdgpu_device_has_dc_support(adev)) {
+                               dev_err(adev->dev,
+                                       "Failed to add dm ip block(DCI_HWIP:0x%x)\n",
+                                       amdgpu_ip_version(adev, DCI_HWIP, 0));
+                               return -EINVAL;
+                       }
+                       return 0;
                  }
          }
-#endif
          return 0;
   }

--
2.48.1









