[PATCH 5/8] drm/amdgpu: According hardware design revert vce and uvd doorbell assignment
Deucher, Alexander
Alexander.Deucher at amd.com
Wed Jul 26 04:48:19 UTC 2017
> -----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
More information about the amd-gfx
mailing list