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:
>
> >
> > 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
> >>
>
