[PATCH 8/8] drm/amdgpu: move align_mask and nop into ring funcs as well

Yu, Xiangliang Xiangliang.Yu at amd.com
Fri Oct 28 11:22:01 UTC 2016


> -----Original Message-----
> From: Christian König [mailto:deathsimple at vodafone.de]
> Sent: Friday, October 28, 2016 6:13 PM
> To: Yu, Xiangliang <Xiangliang.Yu at amd.com>; Alex Deucher
> <alexdeucher at gmail.com>
> Cc: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 8/8] drm/amdgpu: move align_mask and nop into ring
> funcs as well
> 
> Am 28.10.2016 um 04:06 schrieb Yu, Xiangliang:
> >>>>> NAK, we agreed upon moving the constant data into the functions
> >>>>> structure as well.
> >>>>>
> >>>>> How about renaming the amdgpu_ring_funcs structure to make clear
> >>>>> that it isn't only functions any more? Something
> >>>>> amdgpu_ring_params or something like that?
> >>> What benefit to put the constant data into the structure?
> >>> If no, I'll create a new patch that create new structure for these
> >>> constant
> >> data.
> >>
> >> The functions and the parameters should be const.  I don't care how
> >> we arrange the structure(s).  Feel free to re-arrange.
> > Yes, I know. I'll replace the pointer address of function structure not the
> member's content.
> > I think this is best choice for making SRIOV code independent and easy to
> maintain.
> 
> I'm actually not sure if it is a good idea to make the SRIOV code independent,
> but anyway I clearly don't see the advantage of putting the constant
> members into a separate structure even with SRIOV.

For you, there is not any advantage, but for me, I can use the structure for my code.
And the constant structure can make the code very clear, and easy to be re-used 
or changed.

I have spent two weeks to design SRIOV, I think you can see the advantage after
Reviewing my SRIOV patches.

> 
> On the other hand as Alex said feel free to rearrange the members in the
> structure or rename the structure to better match it's purpose.

I still think new structure is better, which benefit code reusing. 

> 
> >
> >>>> Why not create a new structure for the constant data? It can make
> >>>> code more flexible.
> >>>> Actually, I'm working on SRIOV patches, I need the
> >>>> amdgpu_ring_funcs structure so That easy to inherint all of function.
> >>>>
> >>>>> Regards,
> >>>>> Christian.
> >>>>>
> >>>>> Am 26.10.2016 um 09:34 schrieb Yu, Xiangliang:
> >>>>>> Hi Christian,
> >>>>>>
> >>>>>> Could you help put  type, aligan_mask, nop of amdgpu_ring_funcs
> >>>>>> struct
> >>>>> field into amdgpu_ring struct?
> >>>>>> I think amdgpu_ring_funcs should include function pointer, not
> >>>>>> attribute. Your patches is not follow the struct design,  and the
> >>>>> amdgpu_ring will lost the meaning.
> >>>>>>
> >>>>>> Thanks!
> >>>>>> Xiangliang Yu
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org]
> On
> >>>>>>> Behalf Of Christian K?nig
> >>>>>>> Sent: Wednesday, October 05, 2016 10:13 PM
> >>>>>>> To: amd-gfx at lists.freedesktop.org
> >>>>>>> Subject: [PATCH 8/8] drm/amdgpu: move align_mask and nop into
> >>>>>>> ring funcs as well
> >>>>>>>
> >>>>>>> From: Christian König <christian.koenig at amd.com>
> >>>>>>>
> >>>>>>> They are constant as well.
> >>>>>>>
> >>>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
> >>>>>>> ---
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 19
> >>>>>>> +++++++++--------
> >>>> --
> >>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  8 ++++----
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/cik_sdma.c    |  7 ++++---
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c    |  6 ++++--
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c    |  6 ++++--
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c    | 14 ++++++++------
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  7 ++++---
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  7 ++++---
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/si_dma.c      |  3 ++-
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c    |  5 +++--
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c    |  5 +++--
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c    |  5 +++--
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/vce_v2_0.c    |  4 +++-
> >>>>>>>    drivers/gpu/drm/amd/amdgpu/vce_v3_0.c    |  5 +++--
> >>>>>>>    14 files changed, 58 insertions(+), 43 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >>>>>>> index b2df735..a141b46 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> >>>>>>> @@ -65,7 +65,7 @@ int amdgpu_ring_alloc(struct amdgpu_ring
> >>>>>>> *ring, unsigned ndw)  {
> >>>>>>>           /* Align requested size with padding so unlock_commit can
> >>>>>>>            * pad safely */
> >>>>>>> -        ndw = (ndw + ring->align_mask) & ~ring->align_mask;
> >>>>>>> +        ndw = (ndw + ring->funcs->align_mask) &
> >>>>>>> + ~ring->funcs->align_mask;
> >>>>>>>
> >>>>>>>           /* Make sure we aren't trying to allocate more space
> >>>>>>>            * than the maximum for one submission @@ -94,7 +94,7
> >>>>>>> @@ void amdgpu_ring_insert_nop(struct amdgpu_ring *ring,
> >> uint32_t count)
> >>>>>>>           int i;
> >>>>>>>
> >>>>>>>           for (i = 0; i < count; i++)
> >>>>>>> -                amdgpu_ring_write(ring, ring->nop);
> >>>>>>> +                amdgpu_ring_write(ring, ring->funcs->nop);
> >>>>>>>    }
> >>>>>>>
> >>>>>>>    /** amdgpu_ring_generic_pad_ib - pad IB with NOP packets @@
> >>>>>>> -106,8
> >>>>>>> +106,8 @@ void amdgpu_ring_insert_nop(struct amdgpu_ring
> *ring,
> >>>>>>> uint32_t count)
> >>>>>>>     */
> >>>>>>>    void amdgpu_ring_generic_pad_ib(struct amdgpu_ring *ring,
> >>>>>>> struct amdgpu_ib *ib)  {
> >>>>>>> -        while (ib->length_dw & ring->align_mask)
> >>>>>>> -                ib->ptr[ib->length_dw++] = ring->nop;
> >>>>>>> +        while (ib->length_dw & ring->funcs->align_mask)
> >>>>>>> +                ib->ptr[ib->length_dw++] = ring->funcs->nop;
> >>>>>>>    }
> >>>>>>>
> >>>>>>>    /**
> >>>>>>> @@ -125,8 +125,9 @@ void amdgpu_ring_commit(struct
> >> amdgpu_ring
> >>>>> *ring)
> >>>>>>>           uint32_t count;
> >>>>>>>
> >>>>>>>           /* We pad to match fetch size */
> >>>>>>> -        count = ring->align_mask + 1 - (ring->wptr & ring->align_mask);
> >>>>>>> -        count %= ring->align_mask + 1;
> >>>>>>> +        count = ring->funcs->align_mask + 1 -
> >>>>>>> +                (ring->wptr & ring->funcs->align_mask);
> >>>>>>> +        count %= ring->funcs->align_mask + 1;
> >>>>>>>           ring->funcs->insert_nop(ring, count);
> >>>>>>>
> >>>>>>>           mb();
> >>>>>>> @@ -163,8 +164,8 @@ void amdgpu_ring_undo(struct
> amdgpu_ring
> >>>> *ring)
> >>>>>>>     * Returns 0 on success, error on failure.
> >>>>>>>     */
> >>>>>>>    int amdgpu_ring_init(struct amdgpu_device *adev, struct
> >>>>>>> amdgpu_ring *ring,
> >>>>>>> -                     unsigned max_dw, u32 nop, u32 align_mask,
> >>>>>>> -                     struct amdgpu_irq_src *irq_src, unsigned irq_type)
> >>>>>>> +                     unsigned max_dw, struct amdgpu_irq_src *irq_src,
> >>>>>>> +                     unsigned irq_type)
> >>>>>>>    {
> >>>>>>>           int r;
> >>>>>>>
> >>>>>>> @@ -215,8 +216,6 @@ int amdgpu_ring_init(struct amdgpu_device
> >>>>>>> *adev, struct amdgpu_ring *ring,
> >>>>>>>
> >>>>>>>           ring->ring_size = roundup_pow_of_two(max_dw * 4 *
> >>>>>>>                                                amdgpu_sched_hw_submission);
> >>>>>>> -        ring->align_mask = align_mask;
> >>>>>>> -        ring->nop = nop;
> >>>>>>>
> >>>>>>>           /* Allocate ring buffer */
> >>>>>>>           if (ring->ring_obj == NULL) { diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> >>>>>>> index 6cf89c9..1ee1b65 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> >>>>>>> @@ -93,6 +93,8 @@ unsigned
> amdgpu_fence_count_emitted(struct
> >>>>>>> amdgpu_ring *ring);
> >>>>>>>    /* provided by hw blocks that expose a ring buffer for
> >>>>>>> commands */ struct amdgpu_ring_funcs {
> >>>>>>>           enum amdgpu_ring_type   type;
> >>>>>>> +        uint32_t                align_mask;
> >>>>>>> +        u32                     nop;
> >>>>>>>
> >>>>>>>           /* ring read/write ptr handling */
> >>>>>>>           u32 (*get_rptr)(struct amdgpu_ring *ring); @@ -149,10
> >>>>>>> +151,8 @@ struct amdgpu_ring {
> >>>>>>>           unsigned                max_dw;
> >>>>>>>           int                     count_dw;
> >>>>>>>           uint64_t                gpu_addr;
> >>>>>>> -        uint32_t                align_mask;
> >>>>>>>           uint32_t                ptr_mask;
> >>>>>>>           bool                    ready;
> >>>>>>> -        u32                     nop;
> >>>>>>>           u32                     idx;
> >>>>>>>           u32                     me;
> >>>>>>>           u32                     pipe;
> >>>>>>> @@ -178,8 +178,8 @@ void amdgpu_ring_generic_pad_ib(struct
> >>>>>>> amdgpu_ring *ring, struct amdgpu_ib *ib);  void
> >>>>>>> amdgpu_ring_commit(struct amdgpu_ring *ring);  void
> >>>>>>> amdgpu_ring_undo(struct amdgpu_ring *ring);  int
> >>>>> amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring
> >>>>> *ring,
> >>>>>>> -                     unsigned ring_size, u32 nop, u32 align_mask,
> >>>>>>> -                     struct amdgpu_irq_src *irq_src, unsigned irq_type);
> >>>>>>> +                     unsigned ring_size, struct amdgpu_irq_src *irq_src,
> >>>>>>> +                     unsigned irq_type);
> >>>>>>>    void amdgpu_ring_fini(struct amdgpu_ring *ring);
> >>>>>>>
> >>>>>>>    #endif
> >>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> >>>>>>> index f91f02f..664f894 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
> >>>>>>> @@ -205,10 +205,10 @@ static void
> >>>>>>> cik_sdma_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
> >>>>>>> count)
> >>>>>>>
> >>>>>>>           for (i = 0; i < count; i++)
> >>>>>>>                   if (sdma && sdma->burst_nop && (i == 0))
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop |
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop |
> >>>>>>>                                             SDMA_NOP_COUNT(count - 1));
> >>>>>>>                   else
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop);
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop);
> >>>>>>>    }
> >>>>>>>
> >>>>>>>    /**
> >>>>>>> @@ -942,7 +942,6 @@ static int cik_sdma_sw_init(void *handle)
> >>>>>>>                   ring->ring_obj = NULL;
> >>>>>>>                   sprintf(ring->name, "sdma%d", i);
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     SDMA_PACKET(SDMA_OPCODE_NOP, 0,
> >>>>>>> 0), 0xf,
> >>>>>>>                                        &adev->sdma.trap_irq,
> >>>>>>>                                        (i == 0) ?
> >>>>>>>                                        AMDGPU_SDMA_IRQ_TRAP0 :
> >>>>>>> @@ -1207,6 +1206,8 @@ const struct amd_ip_funcs
> >>>>>>> cik_sdma_ip_funcs
> >>>> =
> >>>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs cik_sdma_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_SDMA,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = SDMA_PACKET(SDMA_OPCODE_NOP, 0, 0),
> >>>>>>>           .get_rptr = cik_sdma_ring_get_rptr,
> >>>>>>>           .get_wptr = cik_sdma_ring_get_wptr,
> >>>>>>>           .set_wptr = cik_sdma_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>>>>>> index 1f8687fd..367b14e 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>>>>>> @@ -2869,7 +2869,6 @@ static int gfx_v6_0_sw_init(void *handle)
> >>>>>>>                   ring->ring_obj = NULL;
> >>>>>>>                   sprintf(ring->name, "gfx");
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     0x80000000, 0xff,
> >>>>>>>                                        &adev->gfx.eop_irq,
> >>>>>>> AMDGPU_CP_IRQ_GFX_EOP);
> >>>>>>>                   if (r)
> >>>>>>>                           return r; @@ -2892,7 +2891,6 @@ static
> >>>>>>> int gfx_v6_0_sw_init(void *handle)
> >>>>>>>                   sprintf(ring->name, "comp %d.%d.%d", ring->me,
> >>>>>>> ring->pipe,
> >>>>>>> ring->queue);
> >>>>>>>                   irq_type =
> >>>>>>> AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
> >>>>>>> + ring->pipe;
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     0x80000000, 0xff,
> >>>>>>>                                        &adev->gfx.eop_irq, irq_type);
> >>>>>>>                   if (r)
> >>>>>>>                           return r; @@ -3227,6 +3225,8 @@ const
> >>>>>>> struct amd_ip_funcs gfx_v6_0_ip_funcs
> >>>> =
> >>>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs gfx_v6_0_ring_funcs_gfx =
> {
> >>>>>>>           .type = AMDGPU_RING_TYPE_GFX,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = 0x80000000,
> >>>>>>>           .get_rptr = gfx_v6_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v6_0_ring_get_wptr,
> >>>>>>>           .set_wptr = gfx_v6_0_ring_set_wptr_gfx, @@ -3252,6
> >>>>>>> +3252,8 @@ static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v6_0_ring_funcs_gfx = {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v6_0_ring_funcs_compute
> >>>> = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_COMPUTE,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = 0x80000000,
> >>>>>>>           .get_rptr = gfx_v6_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v6_0_ring_get_wptr,
> >>>>>>>           .set_wptr = gfx_v6_0_ring_set_wptr_compute, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >>>>>>> index be3881e..15f24bd 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >>>>>>> @@ -4605,7 +4605,6 @@ static int gfx_v7_0_sw_init(void *handle)
> >>>>>>>                   ring->ring_obj = NULL;
> >>>>>>>                   sprintf(ring->name, "gfx");
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     PACKET3(PACKET3_NOP, 0x3FFF), 0xff,
> >>>>>>>                                        &adev->gfx.eop_irq,
> >>>>>>> AMDGPU_CP_IRQ_GFX_EOP);
> >>>>>>>                   if (r)
> >>>>>>>                           return r; @@ -4631,7 +4630,6 @@ static
> >>>>>>> int gfx_v7_0_sw_init(void *handle)
> >>>>>>>                   irq_type =
> >>>>>>> AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
> >>>>>>> + ring->pipe;
> >>>>>>>                   /* type-2 packets are deprecated on MEC, use
> >>>>>>> type-3 instead
> >>>>> */
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     PACKET3(PACKET3_NOP, 0x3FFF), 0xff,
> >>>>>>>                                        &adev->gfx.eop_irq, irq_type);
> >>>>>>>                   if (r)
> >>>>>>>                           return r; @@ -5102,6 +5100,8 @@ const
> >>>>>>> struct amd_ip_funcs gfx_v7_0_ip_funcs
> >>>> =
> >>>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_gfx =
> {
> >>>>>>>           .type = AMDGPU_RING_TYPE_GFX,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = PACKET3(PACKET3_NOP, 0x3FFF),
> >>>>>>>           .get_rptr = gfx_v7_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v7_0_ring_get_wptr_gfx,
> >>>>>>>           .set_wptr = gfx_v7_0_ring_set_wptr_gfx, @@ -5130,6
> >>>>>>> +5130,8 @@ static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v7_0_ring_funcs_gfx = {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v7_0_ring_funcs_compute
> >>>> = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_COMPUTE,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = PACKET3(PACKET3_NOP, 0x3FFF),
> >>>>>>>           .get_rptr = gfx_v7_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v7_0_ring_get_wptr_compute,
> >>>>>>>           .set_wptr = gfx_v7_0_ring_set_wptr_compute, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >>>>>>> index 0d66f49..51f1139 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >>>>>>> @@ -2061,9 +2061,8 @@ static int gfx_v8_0_sw_init(void *handle)
> >>>>>>>                           ring->doorbell_index =
> >>>>>>> AMDGPU_DOORBELL_GFX_RING0;
> >>>>>>>                   }
> >>>>>>>
> >>>>>>> -                r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     PACKET3(PACKET3_NOP, 0x3FFF), 0xff,
> >>>>>>> -                                     &adev->gfx.eop_irq,
> >>>>>>> AMDGPU_CP_IRQ_GFX_EOP);
> >>>>>>> +                r = amdgpu_ring_init(adev, ring, 1024, &adev-
> >gfx.eop_irq,
> >>>>>>> +                                     AMDGPU_CP_IRQ_GFX_EOP);
> >>>>>>>                   if (r)
> >>>>>>>                           return r;
> >>>>>>>           }
> >>>>>>> @@ -2087,9 +2086,8 @@ static int gfx_v8_0_sw_init(void *handle)
> >>>>>>>                   sprintf(ring->name, "comp_%d.%d.%d", ring->me,
> >>>>>>> ring-
> >>>>>>>> pipe, ring->queue);
> >>>>>>>                   irq_type =
> >>>>>>> AMDGPU_CP_IRQ_COMPUTE_MEC1_PIPE0_EOP
> >>>>>>> + ring->pipe;
> >>>>>>>                   /* type-2 packets are deprecated on MEC, use
> >>>>>>> type-3 instead
> >>>>> */
> >>>>>>> -                r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     PACKET3(PACKET3_NOP, 0x3FFF), 0xff,
> >>>>>>> -                                     &adev->gfx.eop_irq, irq_type);
> >>>>>>> +                r = amdgpu_ring_init(adev, ring, 1024, &adev-
> >gfx.eop_irq,
> >>>>>>> +                                     irq_type);
> >>>>>>>                   if (r)
> >>>>>>>                           return r;
> >>>>>>>           }
> >>>>>>> @@ -6532,6 +6530,8 @@ const struct amd_ip_funcs
> >>>>>>> gfx_v8_0_ip_funcs
> >>>> =
> >>>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_gfx =
> {
> >>>>>>>           .type = AMDGPU_RING_TYPE_GFX,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = PACKET3(PACKET3_NOP, 0x3FFF),
> >>>>>>>           .get_rptr = gfx_v8_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v8_0_ring_get_wptr_gfx,
> >>>>>>>           .set_wptr = gfx_v8_0_ring_set_wptr_gfx, @@ -6562,6
> >>>>>>> +6562,8 @@ static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v8_0_ring_funcs_gfx = {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs
> >>>>>>> gfx_v8_0_ring_funcs_compute
> >>>> = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_COMPUTE,
> >>>>>>> +        .align_mask = 0xff,
> >>>>>>> +        .nop = PACKET3(PACKET3_NOP, 0x3FFF),
> >>>>>>>           .get_rptr = gfx_v8_0_ring_get_rptr,
> >>>>>>>           .get_wptr = gfx_v8_0_ring_get_wptr_compute,
> >>>>>>>           .set_wptr = gfx_v8_0_ring_set_wptr_compute, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> >>>>>>> index b11a81e..49c47d6 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> >>>>>>> @@ -236,10 +236,10 @@ static void
> >>>>>>> sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
> >>>>>>> count)
> >>>>>>>
> >>>>>>>           for (i = 0; i < count; i++)
> >>>>>>>                   if (sdma && sdma->burst_nop && (i == 0))
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop |
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop |
> >>>>>>>
> >>>>>>> SDMA_PKT_NOP_HEADER_COUNT(count
> >>>>>>> -
> >>>>> 1));
> >>>>>>>                   else
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop);
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop);
> >>>>>>>    }
> >>>>>>>
> >>>>>>>    /**
> >>>>>>> @@ -953,7 +953,6 @@ static int sdma_v2_4_sw_init(void *handle)
> >>>>>>>                   ring->use_doorbell = false;
> >>>>>>>                   sprintf(ring->name, "sdma%d", i);
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -
> >>>>>>> SDMA_PKT_NOP_HEADER_OP(SDMA_OP_NOP), 0xf,
> >>>>>>>                                        &adev->sdma.trap_irq,
> >>>>>>>                                        (i == 0) ?
> >>>>>>>                                        AMDGPU_SDMA_IRQ_TRAP0 :
> >>>>>>> @@ -1211,6 +1210,8 @@ const struct amd_ip_funcs
> >>>> sdma_v2_4_ip_funcs
> >>>>> =
> >>>>>>> {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs sdma_v2_4_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_SDMA,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = SDMA_PKT_NOP_HEADER_OP(SDMA_OP_NOP),
> >>>>>>>           .get_rptr = sdma_v2_4_ring_get_rptr,
> >>>>>>>           .get_wptr = sdma_v2_4_ring_get_wptr,
> >>>>>>>           .set_wptr = sdma_v2_4_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> >>>>>>> index accf4d1..f8ec370 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> >>>>>>> @@ -396,10 +396,10 @@ static void
> >>>>>>> sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring, uint32_t
> >>>>>>> count)
> >>>>>>>
> >>>>>>>           for (i = 0; i < count; i++)
> >>>>>>>                   if (sdma && sdma->burst_nop && (i == 0))
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop |
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop |
> >>>>>>>
> >>>>>>> SDMA_PKT_NOP_HEADER_COUNT(count
> >>>>>>> -
> >>>>> 1));
> >>>>>>>                   else
> >>>>>>> -                        amdgpu_ring_write(ring, ring->nop);
> >>>>>>> +                        amdgpu_ring_write(ring,
> >>>>>>> + ring->funcs->nop);
> >>>>>>>    }
> >>>>>>>
> >>>>>>>    /**
> >>>>>>> @@ -1165,7 +1165,6 @@ static int sdma_v3_0_sw_init(void
> *handle)
> >>>>>>>
> >>>>>>>                   sprintf(ring->name, "sdma%d", i);
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -
> >>>>>>> SDMA_PKT_NOP_HEADER_OP(SDMA_OP_NOP), 0xf,
> >>>>>>>                                        &adev->sdma.trap_irq,
> >>>>>>>                                        (i == 0) ?
> >>>>>>>                                        AMDGPU_SDMA_IRQ_TRAP0 :
> >>>>>>> @@ -1556,6 +1555,8 @@ const struct amd_ip_funcs
> >>>> sdma_v3_0_ip_funcs
> >>>>> =
> >>>>>>> {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs sdma_v3_0_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_SDMA,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = SDMA_PKT_NOP_HEADER_OP(SDMA_OP_NOP),
> >>>>>>>           .get_rptr = sdma_v3_0_ring_get_rptr,
> >>>>>>>           .get_wptr = sdma_v3_0_ring_get_wptr,
> >>>>>>>           .set_wptr = sdma_v3_0_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> >>>>>>> index 1aee45b..7fece1f 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> >>>>>>> @@ -531,7 +531,6 @@ static int si_dma_sw_init(void *handle)
> >>>>>>>                   ring->use_doorbell = false;
> >>>>>>>                   sprintf(ring->name, "sdma%d", i);
> >>>>>>>                   r = amdgpu_ring_init(adev, ring, 1024,
> >>>>>>> -                                     DMA_PACKET(DMA_PACKET_NOP, 0, 0, 0,
> >>>>>>> 0), 0xf,
> >>>>>>>                                        &adev->sdma.trap_irq,
> >>>>>>>                                        (i == 0) ?
> >>>>>>>                                        AMDGPU_SDMA_IRQ_TRAP0 :
> >>>>>>> @@ -765,6 +764,8 @@ const struct amd_ip_funcs si_dma_ip_funcs
> =
> >>>>>>> {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs si_dma_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_SDMA,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = DMA_PACKET(DMA_PACKET_NOP, 0, 0, 0, 0),
> >>>>>>>           .get_rptr = si_dma_ring_get_rptr,
> >>>>>>>           .get_wptr = si_dma_ring_get_wptr,
> >>>>>>>           .set_wptr = si_dma_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >>>>>>> index 55af8ac..1bab75a 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >>>>>>> @@ -116,8 +116,7 @@ static int uvd_v4_2_sw_init(void *handle)
> >>>>>>>
> >>>>>>>           ring = &adev->uvd.ring;
> >>>>>>>           sprintf(ring->name, "uvd");
> >>>>>>> -        r = amdgpu_ring_init(adev, ring, 512,
> PACKET0(mmUVD_NO_OP,
> >> 0),
> >>>>>>> 0xf,
> >>>>>>> -                             &adev->uvd.irq, 0);
> >>>>>>> +        r = amdgpu_ring_init(adev, ring, 512, &adev->uvd.irq,
> >>>>>>> + 0);
> >>>>>>>
> >>>>>>>           return r;
> >>>>>>>    }
> >>>>>>> @@ -743,6 +742,8 @@ const struct amd_ip_funcs
> >> uvd_v4_2_ip_funcs
> >>>>>>> =
> >>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs uvd_v4_2_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_UVD,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = PACKET0(mmUVD_NO_OP, 0),
> >>>>>>>           .get_rptr = uvd_v4_2_ring_get_rptr,
> >>>>>>>           .get_wptr = uvd_v4_2_ring_get_wptr,
> >>>>>>>           .set_wptr = uvd_v4_2_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >>>>>>> index 21e725b..ec848fc 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >>>>>>> @@ -112,8 +112,7 @@ static int uvd_v5_0_sw_init(void *handle)
> >>>>>>>
> >>>>>>>           ring = &adev->uvd.ring;
> >>>>>>>           sprintf(ring->name, "uvd");
> >>>>>>> -        r = amdgpu_ring_init(adev, ring, 512,
> PACKET0(mmUVD_NO_OP,
> >> 0),
> >>>>>>> 0xf,
> >>>>>>> -                             &adev->uvd.irq, 0);
> >>>>>>> +        r = amdgpu_ring_init(adev, ring, 512, &adev->uvd.irq,
> >>>>>>> + 0);
> >>>>>>>
> >>>>>>>           return r;
> >>>>>>>    }
> >>>>>>> @@ -794,6 +793,8 @@ const struct amd_ip_funcs
> >> uvd_v5_0_ip_funcs
> >>>>>>> =
> >>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_UVD,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = PACKET0(mmUVD_NO_OP, 0),
> >>>>>>>           .get_rptr = uvd_v5_0_ring_get_rptr,
> >>>>>>>           .get_wptr = uvd_v5_0_ring_get_wptr,
> >>>>>>>           .set_wptr = uvd_v5_0_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >>>>>>> index 2ce1818..15708f8 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >>>>>>> @@ -116,8 +116,7 @@ static int uvd_v6_0_sw_init(void *handle)
> >>>>>>>
> >>>>>>>           ring = &adev->uvd.ring;
> >>>>>>>           sprintf(ring->name, "uvd");
> >>>>>>> -        r = amdgpu_ring_init(adev, ring, 512,
> PACKET0(mmUVD_NO_OP,
> >> 0),
> >>>>>>> 0xf,
> >>>>>>> -                             &adev->uvd.irq, 0);
> >>>>>>> +        r = amdgpu_ring_init(adev, ring, 512, &adev->uvd.irq,
> >>>>>>> + 0);
> >>>>>>>
> >>>>>>>           return r;
> >>>>>>>    }
> >>>>>>> @@ -1047,6 +1046,8 @@ static const struct amdgpu_ring_funcs
> >>>>>>> uvd_v6_0_ring_phys_funcs = {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs uvd_v6_0_ring_vm_funcs
> = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_UVD,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = PACKET0(mmUVD_NO_OP, 0),
> >>>>>>>           .get_rptr = uvd_v6_0_ring_get_rptr,
> >>>>>>>           .get_wptr = uvd_v6_0_ring_get_wptr,
> >>>>>>>           .set_wptr = uvd_v6_0_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >>>>>>> index cf0c68f..d585839 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >>>>>>> @@ -224,7 +224,7 @@ static int vce_v2_0_sw_init(void *handle)
> >>>>>>>           for (i = 0; i < adev->vce.num_rings; i++) {
> >>>>>>>                   ring = &adev->vce.ring[i];
> >>>>>>>                   sprintf(ring->name, "vce%d", i);
> >>>>>>> -                r = amdgpu_ring_init(adev, ring, 512, VCE_CMD_NO_OP,
> >> 0xf,
> >>>>>>> +                r = amdgpu_ring_init(adev, ring, 512,
> >>>>>>>                                        &adev->vce.irq, 0);
> >>>>>>>                   if (r)
> >>>>>>>                           return r; @@ -611,6 +611,8 @@ const
> >>>>>>> struct amd_ip_funcs vce_v2_0_ip_funcs =
> >>>> {
> >>>>>>>    static const struct amdgpu_ring_funcs vce_v2_0_ring_funcs = {
> >>>>>>>           .type = AMDGPU_RING_TYPE_VCE,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = VCE_CMD_NO_OP,
> >>>>>>>           .get_rptr = vce_v2_0_ring_get_rptr,
> >>>>>>>           .get_wptr = vce_v2_0_ring_get_wptr,
> >>>>>>>           .set_wptr = vce_v2_0_ring_set_wptr, diff --git
> >>>>>>> a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >>>>>>> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >>>>>>> index 56a3feb..f7dbd0d 100644
> >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >>>>>>> @@ -389,8 +389,7 @@ static int vce_v3_0_sw_init(void *handle)
> >>>>>>>           for (i = 0; i < adev->vce.num_rings; i++) {
> >>>>>>>                   ring = &adev->vce.ring[i];
> >>>>>>>                   sprintf(ring->name, "vce%d", i);
> >>>>>>> -                r = amdgpu_ring_init(adev, ring, 512, VCE_CMD_NO_OP,
> >> 0xf,
> >>>>>>> -                                     &adev->vce.irq, 0);
> >>>>>>> +                r = amdgpu_ring_init(adev, ring, 512,
> >>>>>>> + &adev->vce.irq, 0);
> >>>>>>>                   if (r)
> >>>>>>>                           return r;
> >>>>>>>           }
> >>>>>>> @@ -850,6 +849,8 @@ static const struct amdgpu_ring_funcs
> >>>>>>> vce_v3_0_ring_phys_funcs = {
> >>>>>>>
> >>>>>>>    static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs =
> {
> >>>>>>>           .type = AMDGPU_RING_TYPE_VCE,
> >>>>>>> +        .align_mask = 0xf,
> >>>>>>> +        .nop = VCE_CMD_NO_OP,
> >>>>>>>           .get_rptr = vce_v3_0_ring_get_rptr,
> >>>>>>>           .get_wptr = vce_v3_0_ring_get_wptr,
> >>>>>>>           .set_wptr = vce_v3_0_ring_set_wptr,
> >>>>>>> --
> >>>>>>> 2.5.0
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> 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
> >>>> _______________________________________________
> >>>> 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