From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants Date: Tue, 27 Nov 2018 10:56:22 +0100 Message-ID: <31393f87-a966-4e90-7cdf-e00676124f73@gmail.com> References: <20181123083227.25555-1-Jerry.Zhang@amd.com> <962b81c0-358b-0447-d496-9b8efe817261@amd.com> Reply-To: christian.koenig-5C7GfCeVMHo@public.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0029475978==" Return-path: In-Reply-To: <962b81c0-358b-0447-d496-9b8efe817261-5C7GfCeVMHo@public.gmane.org> Content-Language: en-US List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: "Zhang, Jerry(Junwei)" , christian.koenig-5C7GfCeVMHo@public.gmane.org, "Deucher, Alexander" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" This is a multi-part message in MIME format. --===============0029475978== Content-Type: multipart/alternative; boundary="------------62F98CF52BAA377EA7C7840E" Content-Language: en-US This is a multi-part message in MIME format. --------------62F98CF52BAA377EA7C7840E Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Am 27.11.18 um 02:47 schrieb Zhang, Jerry(Junwei): > On 11/26/18 5:28 PM, Christian König wrote: >> Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei): >>> On 11/24/18 3:32 AM, Deucher, Alexander wrote: >>>> >>>> Is this required? Are the harvesting fuses incorrect?  If the >>>> blocks are harvested, we should bail out of the blocks properly >>>> during init.  Also, please make this more explicit if we still need >>>> it.  E.g., >>>> >>>> >>> >>> The harvest fuse is indeed disabling UVD and VCE, as it's a mining card. >>> Then any command to UVD/VCE causing NULL pointer issue, like >>> amdgpu_test. >> >> In this case we should fix the NULL pointer issue instead. Do you >> have a backtrace for this? > > Sorry to miss the detail. > The NULL pointer is caused by UVD is not initialized as it's disabled > in VBIOS for this kind of card. Yeah, but that should be handled correctly. > > When cs submit, it will check ring->funcs->parse_cs in > amdgpu_cs_ib_fill(). > However, uvd_v6_0_early_init() skip the set ring function, as > CC_HARVEST_FUSES is set UVD/VCE disabled. > Then the access to UVD/VCE ring's funcs will cause NULL pointer issue. > > BTW, Windows driver disables UVD/VCE for it as well. You are approaching this from the wrong side. The fact that UVD/VCE is disabled should already be handled correctly. The problem is rather that in a couple of places (amdgpu_ctx_init for example) we assume that we have at least one UVD/VCE ring. Alex is right that checking the fuses should be sufficient and we rather need to fix the handling here instead of adding another workaround. Regards, Christian. > > Regards, > Jerry > >> >> Regards, >> Christian. >> >>> >>> AFAIW, windows also disable UVD and VCE in initialization. >>> >>>>        if ((adev->pdev->device == 0x67df) && >>>>               (adev->pdev->revision == 0xf7)) { >>>> >>>> /* Some polaris12 variants don't support UVD/VCE */ >>>> >>>>       } else  { >>>> >>>> amdgpu_device_ip_block_add(adev, &uvd_v6_3_ip_block); >>>> >>>> amdgpu_device_ip_block_add(adev, &vce_v3_4_ip_block); >>>> >>>>     } >>>> >>>> >>> >>> OK, will explicit the process. >>> >>> Regards, >>> Jerry >>>> >>>> That way if we re-arrange the order later, it will be easier to track. >>>> >>>> >>>> Alex >>>> >>>> ------------------------------------------------------------------------ >>>> *From:* amd-gfx on behalf >>>> of Junwei Zhang >>>> *Sent:* Friday, November 23, 2018 3:32:27 AM >>>> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>>> *Cc:* Zhang, Jerry >>>> *Subject:* [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 >>>> variants >>>> Some variants don't support UVD and VCE. >>>> >>>> Signed-off-by: Junwei Zhang >>>> --- >>>>  drivers/gpu/drm/amd/amdgpu/vi.c | 4 ++++ >>>>  1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c >>>> b/drivers/gpu/drm/amd/amdgpu/vi.c >>>> index f3a4cf1f013a..3338b013ded4 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c >>>> @@ -1660,6 +1660,10 @@ int vi_set_ip_blocks(struct amdgpu_device *adev) >>>> amdgpu_device_ip_block_add(adev, &dce_v11_2_ip_block); >>>>                  amdgpu_device_ip_block_add(adev, &gfx_v8_0_ip_block); >>>>                  amdgpu_device_ip_block_add(adev, &sdma_v3_1_ip_block); >>>> +               /* Some polaris12 variants don't support UVD/VCE */ >>>> +               if ((adev->pdev->device == 0x67df) && >>>> +                     (adev->pdev->revision == 0xf7)) >>>> +                       break; >>>>                  amdgpu_device_ip_block_add(adev, &uvd_v6_3_ip_block); >>>>                  amdgpu_device_ip_block_add(adev, &vce_v3_4_ip_block); >>>>                  break; >>>> -- >>>> 2.17.1 >>>> >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> >>> >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx --------------62F98CF52BAA377EA7C7840E Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
Am 27.11.18 um 02:47 schrieb Zhang, Jerry(Junwei):
On 11/26/18 5:28 PM, Christian König wrote:
Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei):
On 11/24/18 3:32 AM, Deucher, Alexander wrote:

Is this required?  Are the harvesting fuses incorrect?  If the blocks are harvested, we should bail out of the blocks properly during init.  Also, please make this more explicit if we still need it.  E.g.,



The harvest fuse is indeed disabling UVD and VCE, as it's a mining card.
Then any command to UVD/VCE causing NULL pointer issue, like amdgpu_test.

In this case we should fix the NULL pointer issue instead. Do you have a backtrace for this?

Sorry to miss the detail.
The NULL pointer is caused by UVD is not initialized as it's disabled in VBIOS for this kind of card.

Yeah, but that should be handled correctly.


When cs submit, it will check ring->funcs->parse_cs in amdgpu_cs_ib_fill().
However, uvd_v6_0_early_init() skip the set ring function, as CC_HARVEST_FUSES is set UVD/VCE disabled.
Then the access to UVD/VCE ring's funcs will cause NULL pointer issue.

BTW, Windows driver disables UVD/VCE for it as well.

You are approaching this from the wrong side. The fact that UVD/VCE is disabled should already be handled correctly.

The problem is rather that in a couple of places (amdgpu_ctx_init for example) we assume that we have at least one UVD/VCE ring.

Alex is right that checking the fuses should be sufficient and we rather need to fix the handling here instead of adding another workaround.

Regards,
Christian.


Regards,
Jerry


Regards,
Christian.


AFAIW, windows also disable UVD and VCE in initialization.

       if ((adev->pdev->device == 0x67df) &&
              (adev->pdev->revision == 0xf7)) {

        /* Some polaris12 variants don't support UVD/VCE */

      } else  {

                 amdgpu_device_ip_block_add(adev, &uvd_v6_3_ip_block);

                 amdgpu_device_ip_block_add(adev, &vce_v3_4_ip_block);

    }



OK, will explicit the process.

Regards,
Jerry

That way if we re-arrange the order later, it will be easier to track.


Alex

From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Junwei Zhang <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
Sent: Friday, November 23, 2018 3:32:27 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Zhang, Jerry
Subject: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants
 
Some variants don't support UVD and VCE.

Signed-off-by: Junwei Zhang <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index f3a4cf1f013a..3338b013ded4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1660,6 +1660,10 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
                         amdgpu_device_ip_block_add(adev, &dce_v11_2_ip_block);
                 amdgpu_device_ip_block_add(adev, &gfx_v8_0_ip_block);
                 amdgpu_device_ip_block_add(adev, &sdma_v3_1_ip_block);
+               /* Some polaris12 variants don't support UVD/VCE */
+               if ((adev->pdev->device == 0x67df) &&
+                     (adev->pdev->revision == 0xf7))
+                       break;
                 amdgpu_device_ip_block_add(adev, &uvd_v6_3_ip_block);
                 amdgpu_device_ip_block_add(adev, &vce_v3_4_ip_block);
                 break;
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

--------------62F98CF52BAA377EA7C7840E-- --===============0029475978== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4Cg== --===============0029475978==--