<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <p style="font-family:Arial;font-size:10pt;color:#317100;margin:15pt;" align="Left"> [AMD Public Use]<br> </p> <br> <div> <div class="WordSection1"> <p class="msipheader251902e5" style="margin:0in"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:#317100">[AMD Public Use]</span><o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">I see, thanks for the clarifying. The patch is<o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com><o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">I was thinking to have a new flag like AMD_IS_HEADLESS in amd_chip_flag, but after think it a bit more, that’s just complicated the case, I agree with your current approach. <o:p></o:p></p> <p class="MsoNormal"><o:p> </o:p></p> <p class="MsoNormal">Regards,<br> Hawking<o:p></o:p></p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> <p class="MsoNormal"><b>From:</b> Yin, Tianci (Rico) <Tianci.Yin@amd.com> <br> <b>Sent:</b> Friday, October 30, 2020 20:05<br> <b>To:</b> Zhang, Hawking <Hawking.Zhang@amd.com>; amd-gfx@lists.freedesktop.org<br> <b>Cc:</b> Tuikov, Luben <Luben.Tuikov@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; Cui, Flora <Flora.Cui@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Long, Gang <Gang.Long@amd.com><br> <b>Subject:</b> Re: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU<o:p></o:p></p> </div> </div> <p class="MsoNormal"><o:p> </o:p></p> <p style="margin:15.0pt"><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:#317100">[AMD Public Use]<o:p></o:p></span></p> <p class="MsoNormal"><o:p> </o:p></p> <div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">Hi Hawking,<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">at that point, adev has not been allocated yet.<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">My change can make it to right code path.<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">int amdgpu_device_resume(struct drm_device *dev, bool fbcon)<br> {<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">...<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">if (!amdgpu_device_has_dc_support(adev)) <o:p></o:p></span></p> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">drm_helper_hpd_irq_event(dev); //right path for headless SKU<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">else<o:p></o:p></span></p> </div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">drm_kms_helper_hotplug_event(dev); //wrong path for headless SKU<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">...<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">}<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">Thanks!<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black">Rico<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span style="font-size:12.0pt;color:black"><o:p> </o:p></span></p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> <p class="MsoNormal"><b><span style="color:black">From:</span></b><span style="color:black"> Zhang, Hawking <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>><br> <b>Sent:</b> Friday, October 30, 2020 19:48<br> <b>To:</b> Yin, Tianci (Rico) <<a href="mailto:Tianci.Yin@amd.com">Tianci.Yin@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>><br> <b>Cc:</b> Tuikov, Luben <<a href="mailto:Luben.Tuikov@amd.com">Luben.Tuikov@amd.com</a>>; Deucher, Alexander <<a href="mailto:Alexander.Deucher@amd.com">Alexander.Deucher@amd.com</a>>; Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>>; Cui, Flora <<a href="mailto:Flora.Cui@amd.com">Flora.Cui@amd.com</a>>; Xu, Feifei <<a href="mailto:Feifei.Xu@amd.com">Feifei.Xu@amd.com</a>>; Long, Gang <<a href="mailto:Gang.Long@amd.com">Gang.Long@amd.com</a>>; Yin, Tianci (Rico) <<a href="mailto:Tianci.Yin@amd.com">Tianci.Yin@amd.com</a>><br> <b>Subject:</b> RE: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU</span> <o:p></o:p></p> <div> <p class="MsoNormal"> <o:p></o:p></p> </div> </div> <div> <div> <p class="MsoNormal">[AMD Public Use]<br> <br> I'm not sure I get your point on changing amdgpu_device_has_dc_support() interface by adding new parameter. but it seems to me change input parameter from pdev to adev for nv_is_headless_sku is more straightforward. <br> <br> Regards,<br> Hawking<br> -----Original Message-----<br> From: Tianci Yin <<a href="mailto:tianci.yin@amd.com">tianci.yin@amd.com</a>> <br> Sent: Friday, October 30, 2020 19:32<br> To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br> Cc: Tuikov, Luben <<a href="mailto:Luben.Tuikov@amd.com">Luben.Tuikov@amd.com</a>>; Deucher, Alexander <<a href="mailto:Alexander.Deucher@amd.com">Alexander.Deucher@amd.com</a>>; Zhang, Hawking <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>>; Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>>; Cui, Flora <<a href="mailto:Flora.Cui@amd.com">Flora.Cui@amd.com</a>>; Xu, Feifei <<a href="mailto:Feifei.Xu@amd.com">Feifei.Xu@amd.com</a>>; Long, Gang <<a href="mailto:Gang.Long@amd.com">Gang.Long@amd.com</a>>; Yin, Tianci (Rico) <<a href="mailto:Tianci.Yin@amd.com">Tianci.Yin@amd.com</a>><br> Subject: [PATCH] drm/amdgpu: fix NULL pointer crash on navi10 headless SKU<br> <br> From: "Tianci.Yin" <<a href="mailto:tianci.yin@amd.com">tianci.yin@amd.com</a>><br> <br> The crash caused by the NULL pointer of<br> adev->ddev.mode_config.funcs in drm_kms_helper_hotplug_event(),<br> but this function should not be called on headless SKU.<br> <br> Fix the mismatch between the return value of<br> amdgpu_device_has_dc_support() and the real DCN supporting state to avoid calling to drm_kms_helper_hotplug_event() in amdgpu_device_resume().<br> <br> Change-Id: I3a3d387e6ab5b774abb3911ea1bf6de60797759d<br> Signed-off-by: Tianci.Yin <<a href="mailto:tianci.yin@amd.com">tianci.yin@amd.com</a>><br> ---<br> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-<br> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-<br> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-<br> drivers/gpu/drm/amd/amdgpu/nv.c | 2 +-<br> drivers/gpu/drm/amd/amdgpu/nv.h | 1 +<br> 6 files changed, 13 insertions(+), 6 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br> index ba65d4f2ab67..f0183271456f 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br> @@ -1090,7 +1090,7 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device *adev,<br> u32 pcie_index, u32 pcie_data,<br> u32 reg_addr, u64 reg_data);<br> <br> -bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type);<br> +bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, <br> +struct pci_dev *pdev);<br> bool amdgpu_device_has_dc_support(struct amdgpu_device *adev);<br> <br> int emu_soc_asic_init(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br> index 1fe850e0a94d..323ed69032a7 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br> @@ -2960,11 +2960,12 @@ static void amdgpu_device_detect_sriov_bios(struct amdgpu_device *adev)<br> * amdgpu_device_asic_has_dc_support - determine if DC supports the asic<br> *<br> * @asic_type: AMD asic type<br> + * @pdev: pointer to pci_dev instance<br> *<br> * Check if there is DC (new modesetting infrastructre) support for an asic.<br> * returns true if DC has support, false if not.<br> */<br> -bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)<br> +bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type, <br> +struct pci_dev *pdev)<br> {<br> switch (asic_type) {<br> #if defined(CONFIG_DRM_AMD_DC)<br> @@ -3000,9 +3001,14 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type)<br> case CHIP_VEGA20:<br> #if defined(CONFIG_DRM_AMD_DC_DCN)<br> case CHIP_RAVEN:<br> + return amdgpu_dc != 0;<br> case CHIP_NAVI10:<br> case CHIP_NAVI14:<br> case CHIP_NAVI12:<br> + if (nv_is_headless_sku(pdev))<br> + return false;<br> + else<br> + return amdgpu_dc != 0;<br> case CHIP_RENOIR:<br> #endif<br> #if defined(CONFIG_DRM_AMD_DC_DCN3_0)<br> @@ -3033,7 +3039,7 @@ bool amdgpu_device_has_dc_support(struct amdgpu_device *adev)<br> if (amdgpu_sriov_vf(adev) || adev->enable_virtual_display)<br> return false;<br> <br> - return amdgpu_device_asic_has_dc_support(adev->asic_type);<br> + return amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev);<br> }<br> <br> <br> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c<br> index 9e92d2a070ac..97014458d7de 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c<br> @@ -516,7 +516,7 @@ uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev,<br> */<br> if ((bo_flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&<br> amdgpu_bo_support_uswc(bo_flags) &&<br> - amdgpu_device_asic_has_dc_support(adev->asic_type)) {<br> + amdgpu_device_asic_has_dc_support(adev->asic_type, adev->pdev)) {<br> switch (adev->asic_type) {<br> case CHIP_CARRIZO:<br> case CHIP_STONEY:<br> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br> index 4b78ecfd35f7..b23110241267 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br> @@ -1117,7 +1117,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,<br> bool supports_atomic = false;<br> <br> if (!amdgpu_virtual_display &&<br> - amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK))<br> + amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK, pdev))<br> supports_atomic = true;<br> <br> if ((flags & AMD_EXP_HW_SUPPORT) && !amdgpu_exp_hw_support) { diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c index 026e0a8fd526..97446ae75b0b 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/nv.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c<br> @@ -493,7 +493,7 @@ void nv_set_virt_ops(struct amdgpu_device *adev)<br> adev->virt.ops = &xgpu_nv_virt_ops;<br> }<br> <br> -static bool nv_is_headless_sku(struct pci_dev *pdev)<br> +bool nv_is_headless_sku(struct pci_dev *pdev)<br> {<br> if ((pdev->device == 0x731E &&<br> (pdev->revision == 0xC6 || pdev->revision == 0xC7)) || diff --git a/drivers/gpu/drm/amd/amdgpu/nv.h b/drivers/gpu/drm/amd/amdgpu/nv.h index 515d67bf249f..7880ad0073c9 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/nv.h<br> +++ b/drivers/gpu/drm/amd/amdgpu/nv.h<br> @@ -29,6 +29,7 @@<br> void nv_grbm_select(struct amdgpu_device *adev,<br> u32 me, u32 pipe, u32 queue, u32 vmid); void nv_set_virt_ops(struct amdgpu_device *adev);<br> +bool nv_is_headless_sku(struct pci_dev *pdev);<br> int nv_set_ip_blocks(struct amdgpu_device *adev); int navi10_reg_base_init(struct amdgpu_device *adev); int navi14_reg_base_init(struct amdgpu_device *adev);<br> --<br> 2.17.1<o:p></o:p></p> </div> </div> </div> </div> </div> </body> </html>