> -----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 >