[PATCH 2/8] drm/amdkfd: Correct SDMA ring buffer size

Christian König ckoenig.leichtzumerken at gmail.com
Thu Nov 2 12:03:44 UTC 2017


Am 02.11.2017 um 00:21 schrieb Felix Kuehling:
> From: shaoyunl <Shaoyun.Liu at amd.com>
>
> ffs function return the position of the first bit set on 1 based.
> (bit zero returns 1).
>
> Signed-off-by: shaoyun liu <shaoyun.liu at amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> index 4859d26..4728fad 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -202,8 +202,8 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>   	struct cik_sdma_rlc_registers *m;
>   
>   	m = get_sdma_mqd(mqd);
> -	m->sdma_rlc_rb_cntl = ffs(q->queue_size / sizeof(unsigned int)) <<
> -			SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
> +	m->sdma_rlc_rb_cntl = (ffs(q->queue_size / sizeof(unsigned int)) - 1)
> +			<< SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
>   			q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT |

Usually we use order_base_2 for this. See cik_sdma.c for example:

rb_bufsz = order_base_2(ring->ring_size / 4);

Additional to that I won't expect that sizeof(unsigned int) is ever 
something else than 4 on any CPU that Linux supports, but that we need 
to divide the value by 4 is a programming detail of our hardware and not 
something dependent on the CPU/Compiler in use.

So I think using 4 over sizeof(unsigned int) is cleaner and easier to 
understand.

Regards,
Christian.

>   			1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
>   			6 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_TIMER__SHIFT;




More information about the amd-gfx mailing list