Re: [PATCH] Increase AMDGPU_MAX_UVD_INSTANCES to 3

From: James Zhu <jamesz-5C7GfCeVMHo@public.gmane.org>
To: "Deucher,
	Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>,
	Timothy Pearson
	<tpearson-z0qzliK6Om0mgXJStvpl+vpXobYPEAuW@public.gmane.org>,
	"Zhu, James" <James.Zhu-5C7GfCeVMHo@public.gmane.org>
Cc: "Zhou,
	David(ChunMing)" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>,
	"Koenig,
	Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH] Increase AMDGPU_MAX_UVD_INSTANCES to 3
Date: Mon, 25 Jun 2018 14:37:46 -0400	[thread overview]
Message-ID: <2b12b893-e2a0-9362-cb8f-15b57b12fe11@amd.com> (raw)
In-Reply-To: <BN6PR12MB1809E207A784F6EE6D79239AF74A0-/b2+HYfkarSEx6ez0IUAagdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

[-- Attachment #1.1: Type: text/plain, Size: 2634 bytes --]

For one UVD instance case,:

/In function/ amdgpu_driver_load_kms, all ring->me should be set to zero.
*  adev = kzalloc(sizeof(struct amdgpu_device), GFP_KERNEL);*

For two UVD instances cases:

static void uvd_v7_0_set_ring_funcs(struct amdgpu_device *adev)
..
     for (i = 0; i < adev->uvd.num_uvd_inst; i++) {
*adev->uvd.inst[i].ring.me = i;*

static void uvd_v7_0_set_enc_ring_funcs(struct amdgpu_device *adev)

     for (j = 0; j < adev->uvd.num_uvd_inst; j++) {
*adev->uvd.inst[j].ring_enc[i].me = j;*

uvd_v4_2_early_init in uvd_v4_2.c  adev->uvd.num_uvd_inst = 1;
uvd_v5_0_early_init in uvd_v5_0.c  adev->uvd.num_uvd_inst = 1;
uvd_v6_0_early_init in uvd_v6_0.c  adev->uvd.num_uvd_inst = 1;
uvd_v7_0_early_init in uvd_v7_0.c
     if (adev->asic_type == CHIP_VEGA20)
*adev->uvd.num_uvd_inst = UVD7_MAX_HW_INSTANCES_VEGA20;/*2*/*
     else
*adev->uvd.num_uvd_inst = 1;*

I didn't know when ring->me is set to 2. Maybe there is some leakage 
somewhere.

Best regards!

James zhu

On 2018-06-25 01:29 PM, Deucher, Alexander wrote:
>
> Odd. The structure should be 0 initialized.  Does this patch help?
>
>
> Alex
>
> ------------------------------------------------------------------------
> *From:* Timothy Pearson <tpearson-z0qzliK6Om0mgXJStvpl+vpXobYPEAuW@public.gmane.org>
> *Sent:* Monday, June 25, 2018 11:53:12 AM
> *To:* Zhu, James
> *Cc:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; Deucher, Alexander; Zhou, 
> David(ChunMing); Koenig, Christian
> *Subject:* Re: [PATCH] Increase AMDGPU_MAX_UVD_INSTANCES to 3
> n 06/25/2018 09:46 AM, James Zhu wrote:
> >
> >
> > On 2018-06-23 08:02 PM, Timothy Pearson wrote:
> >> amdgpu_fence_driver_start_ring() attempts to access
> >> UVD instance 2 during setup, while the existing UVD
> >> instance count only allows instances 0 and 1.
> >>
> >> Increase AMDGPU_MAX_UVD_INSTANCES by one to avoid the
> >> invalid array access.
> >>
> >> Caught by UBSAN.
> > Hi Timothy,
> >
> > From design of view, it is not right to just change
> > AMDGPU_MAX_UVD_INSTANCES to 3.
> >
> > Could you tell me some detail of UBSAN test and attach the dmesg also?
>
> Definitely, was looking for some feedback from anyone knowing more about
> the internals of the UVD system.
>
> What's happening is that "ring->me" in amdgpu_fence_driver_start_ring()
> (drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c:379) is set to a value of
> "2".  The overall dmesg is otherwise uninteresting, but I can try to
> grab the UBSAN output if needed.
>
> -- 
> Timothy Pearson
> Raptor Engineering
> +1 (415) 727-8645 (direct line)
> +1 (512) 690-0200 (switchboard)
> https://www.raptorengineering.com

[-- Attachment #1.2: Type: text/html, Size: 5530 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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