<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <p style="font-family:Arial;font-size:10pt;color:#317100;margin:15pt;" align="Left"> [AMD Public Use]<br> </p> <br> <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Maybe it would be easier to check if the DCE IP exists? E.g., <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> if (!amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_DCE) ||</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span style="color:black;font-size:12pt">(!amdgpu_device_has_dc_support(adev)</span>)</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> ...</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Also do we even need to skip DCN init for these cards, or will the display code handle it automatically since there will be no displays in the atombios object table. We didn't do anything special for display with the polaris blockchain cards.<br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Alex</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Zhang, Hawking <Hawking.Zhang@amd.com><br> <b>Sent:</b> Friday, October 30, 2020 9:00 AM<br> <b>To:</b> Yin, Tianci (Rico) <Tianci.Yin@amd.com>; amd-gfx@lists.freedesktop.org <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</font> <div> </div> </div> <style>  </style> <div lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <p align="Left" style="font-family:Arial; font-size:10pt; color:#317100; margin:15pt"> [AMD Public Use]<br> </p> <br> <div> <div class="x_WordSection1"> <p class="x_msipheader251902e5" style="margin:0in"><span style="font-size:10.0pt; font-family:"Arial",sans-serif; color:#317100">[AMD Public Use]</span></p> <p class="x_MsoNormal"> </p> <p class="x_MsoNormal">I see, thanks for the clarifying. The patch is</p> <p class="x_MsoNormal"> </p> <p class="x_MsoNormal">Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com></p> <p class="x_MsoNormal"> </p> <p class="x_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. </p> <p class="x_MsoNormal"> </p> <p class="x_MsoNormal">Regards,<br> Hawking</p> <div> <div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0in 0in 0in"> <p class="x_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</p> </div> </div> <p class="x_MsoNormal"> </p> <p style="margin:15.0pt"><span style="font-size:10.0pt; font-family:"Arial",sans-serif; color:#317100">[AMD Public Use]</span></p> <p class="x_MsoNormal"> </p> <div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Hi Hawking,</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">amdgpu_device_asic_has_dc_support() is referrenced by amdgpu_pci_probe(),</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">at that point, adev has not been allocated yet.</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">My change can make it to right code path.</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">int amdgpu_device_resume(struct drm_device *dev, bool fbcon)<br> {</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">...</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">if (!amdgpu_device_has_dc_support(adev)) </span></p> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">drm_helper_hpd_irq_event(dev); //right path for headless SKU</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">else</span></p> </div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">drm_kms_helper_hotplug_event(dev); //wrong path for headless SKU</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">...</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">}</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Thanks!</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">Rico</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div class="x_MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> <p class="x_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> </p> <div> <p class="x_MsoNormal"> </p> </div> </div> <div> <div> <p class="x_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</p> </div> </div> </div> </div> </div> </div> </div> </body> </html>