Thanks a lot -----é?®ä»¶å??件----- å??件人: Deucher, Alexander å??é??æ?¶é?´: 2017å¹´7æ??26æ?¥ 12:48 æ?¶ä»¶äºº: Min, Frank <Frank.Min at amd.com>; Alex Deucher <alexdeucher at gmail.com>; Yu, Xiangliang <Xiangliang.Yu at amd.com> æ??é??: amd-gfx list <amd-gfx at lists.freedesktop.org> 主é¢?: RE: [PATCH 5/8] drm/amdgpu: According hardware design revert vce and uvd doorbell assignment > -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Min, Frank > Sent: Tuesday, July 25, 2017 10:17 PM > To: Alex Deucher; Yu, Xiangliang > Cc: amd-gfx list > Subject: RE: [PATCH 5/8] drm/amdgpu: According hardware design revert > vce and uvd doorbell assignment > > Hi Alexï¼? > In sriov currently we only use only one encoding ring for uvd and vce. > the logic here is to leave the unused doorbell location(for vce it is > AMDGPU_DOORBELL64_VCE_RING2_3 * 2 + 1 and for uvd it is > AMDGPU_DOORBELL64_UVD_RING2_3 * 2 + 1) for all the unused ring index. > > How about to change the code like this: > > if (amdgpu_sriov_vf(adev)) { > /* DOORBELL only works under SRIOV */ > ring->use_doorbell = true; > if (i == 0) > ring->doorbell_index = > AMDGPU_DOORBELL64_VCE_RING0_1 * 2; > else > ring->doorbell_index = > AMDGPU_DOORBELL64_VCE_RING2_3 * 2 + 1; > } > > if (amdgpu_sriov_vf(adev)) { > ring->use_doorbell = true; > if (i == 0) > ring->doorbell_index = > AMDGPU_DOORBELL64_UVD_RING0_1 * 2; > else > ring->doorbell_index = > AMDGPU_DOORBELL64_UVD_RING2_3 * 2 + 1; > } That's fine. Just add a comment similar to your explanation above where you set the doorbells so it's clear why they are set that way. With that fixed: Acked-by: Alex Deucher <alexander.deucher at amd.com> Alex > > Best Regards, > Frank > > -----é?®ä»¶å??件----- > å??件人: Alex Deucher [mailto:alexdeucher at gmail.com] > å??é??æ?¶é?´: 2017å¹´7æ??26æ?¥ 0:00 > æ?¶ä»¶äºº: Yu, Xiangliang <Xiangliang.Yu at amd.com> > æ??é??: amd-gfx list <amd-gfx at lists.freedesktop.org>; Min, Frank > <Frank.Min at amd.com> > 主é¢?: Re: [PATCH 5/8] drm/amdgpu: According hardware design revert vce > and uvd doorbell assignment > > On Tue, Jul 25, 2017 at 5:17 AM, Xiangliang.Yu <Xiangliang.Yu at amd.com> > wrote: > > From: Frank Min <Frank.Min at amd.com> > > > > Now uvd doorbell is from 0xf8-0xfb and vce doorbell is from > > 0xfc-0xff > > > > Signed-off-by: Frank Min <Frank.Min at amd.com> > > Signed-off-by: Xiangliang.Yu <Xiangliang.Yu at amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 18 +++++++++--------- > > drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 6 ++++-- > > drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 6 +++--- > > 3 files changed, 16 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > index fe96236..d287621 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > @@ -680,15 +680,15 @@ typedef enum > _AMDGPU_DOORBELL64_ASSIGNMENT > > /* overlap the doorbell assignment with VCN as they are > > mutually > exclusive > > * VCE engine's doorbell is 32 bit and two VCE ring share one QWORD > > */ > > - AMDGPU_DOORBELL64_RING0_1 = 0xF8, > > - AMDGPU_DOORBELL64_RING2_3 = 0xF9, > > - AMDGPU_DOORBELL64_RING4_5 = 0xFA, > > - AMDGPU_DOORBELL64_RING6_7 = 0xFB, > > - > > - AMDGPU_DOORBELL64_UVD_RING0_1 = 0xFC, > > - AMDGPU_DOORBELL64_UVD_RING2_3 = 0xFD, > > - AMDGPU_DOORBELL64_UVD_RING4_5 = 0xFE, > > - AMDGPU_DOORBELL64_UVD_RING6_7 = 0xFF, > > + AMDGPU_DOORBELL64_UVD_RING0_1 = 0xF8, > > + AMDGPU_DOORBELL64_UVD_RING2_3 = 0xF9, > > + AMDGPU_DOORBELL64_UVD_RING4_5 = 0xFA, > > + AMDGPU_DOORBELL64_UVD_RING6_7 = 0xFB, > > + > > + AMDGPU_DOORBELL64_VCE_RING0_1 = 0xFC, > > + AMDGPU_DOORBELL64_VCE_RING2_3 = 0xFD, > > + AMDGPU_DOORBELL64_VCE_RING4_5 = 0xFE, > > + AMDGPU_DOORBELL64_VCE_RING6_7 = 0xFF, > > > > AMDGPU_DOORBELL64_MAX_ASSIGNMENT = 0xFF, > > AMDGPU_DOORBELL64_INVALID = 0xFFFF > > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c > > b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c > > index ab447e8..590c3f0 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c > > @@ -435,13 +435,15 @@ static int uvd_v7_0_sw_init(void *handle) > > return r; > > } > > > > - > > for (i = 0; i < adev->uvd.num_enc_rings; ++i) { > > ring = &adev->uvd.ring_enc[i]; > > sprintf(ring->name, "uvd_enc%d", i); > > if (amdgpu_sriov_vf(adev)) { > > ring->use_doorbell = true; > > - ring->doorbell_index = > AMDGPU_DOORBELL64_UVD_RING0_1 * 2; > > + if (i == 0) > > + ring->doorbell_index = > AMDGPU_DOORBELL64_UVD_RING0_1 * 2; > > + else > > + ring->doorbell_index = > > + AMDGPU_DOORBELL64_UVD_RING2_3 * 2 + 1; > > Can you clarify the requirements? This logic doesn't seem right. > > > } > > r = amdgpu_ring_init(adev, ring, 512, &adev->uvd.irq, 0); > > if (r) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c > > b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c > > index 9e0050d..34c2281 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c > > @@ -446,11 +446,11 @@ static int vce_v4_0_sw_init(void *handle) > > /* DOORBELL only works under SRIOV */ > > ring->use_doorbell = true; > > if (i == 0) > > - ring->doorbell_index = AMDGPU_DOORBELL64_RING0_1 * > 2; > > + ring->doorbell_index = > > + AMDGPU_DOORBELL64_VCE_RING0_1 * 2; > > else if (i == 1) > > - ring->doorbell_index = AMDGPU_DOORBELL64_RING2_3 * > 2; > > + ring->doorbell_index = > > + AMDGPU_DOORBELL64_VCE_RING2_3 * 2; > > else > > - ring->doorbell_index = AMDGPU_DOORBELL64_RING2_3 * > 2 + 1; > > + ring->doorbell_index = > > + AMDGPU_DOORBELL64_VCE_RING2_3 * 2 + 1; > > Same here. The one is even weirder. > > Alex > > > } > > r = amdgpu_ring_init(adev, ring, 512, &adev->vce.irq, 0); > > if (r) > > -- > > 2.7.4 > > > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
