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

[Felix Kuehling]

On 2017-11-02 08:03 AM, Christian König wrote:
> 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.

Thanks for pointing those out. I'd address that with a separate cleanup
commit, because we're using ffs like this and dividing by sizeof(various
32-bit integer types) in a bunch more places.

Regards,
  Felix

>
> Regards,
> Christian.
>
>>               1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
>>               6 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_TIMER__SHIFT;
>
>