[PATCH 5/8] drm/amdgpu: According hardware design revert vce and uvd doorbell assignment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux