[PATCH 0/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[PATCH 0/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Arvind Yadav]

On older chips, the absolute doorbell offset within 
the doorbell page is based on the queue ID.
KFD is using queue ID and doorbell size to get an
absolute doorbell offset in userspace.

This patch is to adjust the absolute doorbell offset
against the doorbell id considering the doorbell
size of 32/64 bit. 

Arvind Yadav (1):
  drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 0/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Arvind Yadav]

On older chips, the absolute doorbell offset within 
the doorbell page is based on the queue ID.
KFD is using queue ID and doorbell size to get an
absolute doorbell offset in userspace.

This patch is to adjust the absolute doorbell offset
against the doorbell id considering the doorbell
size of 32/64 bit. 

Arvind Yadav (1):
  drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Arvind Yadav]

This patch is to adjust the absolute doorbell offset
against the doorbell id considering the doorbell
size of 32/64 bit.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 0d3d538b64eb..c327f7f604d7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
 
 	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
 								  qpd->proc_doorbells,
-								  q->doorbell_id);
+								  0);
+
+	/* Adjust the absolute doorbell offset against the doorbell id considering
+	 * the doorbell size of 32/64 bit.
+	 */
+	if (!KFD_IS_SOC15(dev))
+		q->properties.doorbell_off += q->doorbell_id;
+	else
+		q->properties.doorbell_off += q->doorbell_id * 2;
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Arvind Yadav]

This patch is to adjust the absolute doorbell offset
against the doorbell id considering the doorbell
size of 32/64 bit.

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 0d3d538b64eb..c327f7f604d7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
 
 	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
 								  qpd->proc_doorbells,
-								  q->doorbell_id);
+								  0);
+
+	/* Adjust the absolute doorbell offset against the doorbell id considering
+	 * the doorbell size of 32/64 bit.
+	 */
+	if (!KFD_IS_SOC15(dev))
+		q->properties.doorbell_off += q->doorbell_id;
+	else
+		q->properties.doorbell_off += q->doorbell_id * 2;
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 0/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Yadav, Arvind]

Adding Felix.Kuehling@amd.com for review.

Thanks
~Arvind

On 9/27/2023 9:46 PM, Arvind Yadav wrote:
> On older chips, the absolute doorbell offset within
> the doorbell page is based on the queue ID.
> KFD is using queue ID and doorbell size to get an
> absolute doorbell offset in userspace.
>
> This patch is to adjust the absolute doorbell offset
> against the doorbell id considering the doorbell
> size of 32/64 bit.
>
> Arvind Yadav (1):
>    drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
>
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>

Re: [PATCH 0/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Yadav, Arvind]

Adding Felix.Kuehling@amd.com for review.

Thanks
~Arvind

On 9/27/2023 9:46 PM, Arvind Yadav wrote:
> On older chips, the absolute doorbell offset within
> the doorbell page is based on the queue ID.
> KFD is using queue ID and doorbell size to get an
> absolute doorbell offset in userspace.
>
> This patch is to adjust the absolute doorbell offset
> against the doorbell id considering the doorbell
> size of 32/64 bit.
>
> Arvind Yadav (1):
>    drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8
>
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>

Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Yadav, Arvind]

Adding Felix.Kuehling@amd.com for review.

Thanks
~Arvind

On 9/27/2023 9:46 PM, Arvind Yadav wrote:
> This patch is to adjust the absolute doorbell offset
> against the doorbell id considering the doorbell
> size of 32/64 bit.
>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 0d3d538b64eb..c327f7f604d7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
>   
>   	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
>   								  qpd->proc_doorbells,
> -								  q->doorbell_id);
> +								  0);
> +
> +	/* Adjust the absolute doorbell offset against the doorbell id considering
> +	 * the doorbell size of 32/64 bit.
> +	 */
> +	if (!KFD_IS_SOC15(dev))
> +		q->properties.doorbell_off += q->doorbell_id;
> +	else
> +		q->properties.doorbell_off += q->doorbell_id * 2;
> +
>   	return 0;
>   }
>   

Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Yadav, Arvind]

Adding Felix.Kuehling@amd.com for review.

Thanks
~Arvind

On 9/27/2023 9:46 PM, Arvind Yadav wrote:
> This patch is to adjust the absolute doorbell offset
> against the doorbell id considering the doorbell
> size of 32/64 bit.
>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> Signed-off-by: Arvind Yadav <Arvind.Yadav@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 0d3d538b64eb..c327f7f604d7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
>   
>   	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
>   								  qpd->proc_doorbells,
> -								  q->doorbell_id);
> +								  0);
> +
> +	/* Adjust the absolute doorbell offset against the doorbell id considering
> +	 * the doorbell size of 32/64 bit.
> +	 */
> +	if (!KFD_IS_SOC15(dev))
> +		q->properties.doorbell_off += q->doorbell_id;
> +	else
> +		q->properties.doorbell_off += q->doorbell_id * 2;
> +
>   	return 0;
>   }
>   

Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Felix Kuehling]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

[+Mukul]

On 2023-09-27 12:16, Arvind Yadav wrote:
> This patch is to adjust the absolute doorbell offset
> against the doorbell id considering the doorbell
> size of 32/64 bit.
>
> Cc: Christian Koenig<christian.koenig@amd.com>
> Cc: Alex Deucher<alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma<shashank.sharma@amd.com>
> Signed-off-by: Arvind Yadav<Arvind.Yadav@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 0d3d538b64eb..c327f7f604d7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
>   
>   	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
>   								  qpd->proc_doorbells,
> -								  q->doorbell_id);
> +								  0);

Looks like we're now always calling amdgpu_doorbell_index_on_bar with 
the third parameter = 0. So we could remove that parameter. It doesn't 
do us any good and only causes bugs if we use any non-0 value.


> +
> +	/* Adjust the absolute doorbell offset against the doorbell id considering
> +	 * the doorbell size of 32/64 bit.
> +	 */
> +	if (!KFD_IS_SOC15(dev))
> +		q->properties.doorbell_off += q->doorbell_id;
> +	else
> +		q->properties.doorbell_off += q->doorbell_id * 2;

This could be simplified and generalized as

	q->properties.doorbell_off += q->doorbell_id * dev->kfd->device_info.doorbell_size / 4;

Regards,
   Felix


> +
>   	return 0;
>   }
>   

[-- Attachment #2: Type: text/html, Size: 2780 bytes --]

Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Shashank Sharma]

On 27/09/2023 19:27, Felix Kuehling wrote:
>
> [+Mukul]
>
> On 2023-09-27 12:16, Arvind Yadav wrote:
>> This patch is to adjust the absolute doorbell offset
>> against the doorbell id considering the doorbell
>> size of 32/64 bit.
>>
>> Cc: Christian Koenig<christian.koenig@amd.com>
>> Cc: Alex Deucher<alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma<shashank.sharma@amd.com>
>> Signed-off-by: Arvind Yadav<Arvind.Yadav@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index 0d3d538b64eb..c327f7f604d7 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
>>   
>>   	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
>>   								  qpd->proc_doorbells,
>> -								  q->doorbell_id);
>> +								  0);
>
> Looks like we're now always calling amdgpu_doorbell_index_on_bar with 
> the third parameter = 0. So we could remove that parameter. It doesn't 
> do us any good and only causes bugs if we use any non-0 value.
>
Hey Felix,

Actually this was happening because in usermode KFD library the 
doorbell-size is for non-SOC15() hardware is hard coded to 4 bytes.

We added this fix just so that the library code doesn't need to be 
changed, but can still get aligned to kernel code.

GFX Usermode queue code uses this doorbell-index parameter, and it gives 
us the right offset.


Regards

Shashank

>
>> +
>> +	/* Adjust the absolute doorbell offset against the doorbell id considering
>> +	 * the doorbell size of 32/64 bit.
>> +	 */
>> +	if (!KFD_IS_SOC15(dev))
>> +		q->properties.doorbell_off += q->doorbell_id;
>> +	else
>> +		q->properties.doorbell_off += q->doorbell_id * 2;
>
> This could be simplified and generalized as
>
> 	q->properties.doorbell_off += q->doorbell_id * dev->kfd->device_info.doorbell_size / 4;

Agree, we can do this simplification.

- Shashank

> Regards,
>   Felix
>
>
>> +
>>   	return 0;
>>   }
>>   

Re: [PATCH 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

[Shashank Sharma]

On 27/09/2023 19:27, Felix Kuehling wrote:
>
> [+Mukul]
>
> On 2023-09-27 12:16, Arvind Yadav wrote:
>> This patch is to adjust the absolute doorbell offset
>> against the doorbell id considering the doorbell
>> size of 32/64 bit.
>>
>> Cc: Christian Koenig<christian.koenig@amd.com>
>> Cc: Alex Deucher<alexander.deucher@amd.com>
>> Signed-off-by: Shashank Sharma<shashank.sharma@amd.com>
>> Signed-off-by: Arvind Yadav<Arvind.Yadav@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index 0d3d538b64eb..c327f7f604d7 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -407,7 +407,16 @@ static int allocate_doorbell(struct qcm_process_device *qpd,
>>   
>>   	q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev->adev,
>>   								  qpd->proc_doorbells,
>> -								  q->doorbell_id);
>> +								  0);
>
> Looks like we're now always calling amdgpu_doorbell_index_on_bar with 
> the third parameter = 0. So we could remove that parameter. It doesn't 
> do us any good and only causes bugs if we use any non-0 value.
>
Hey Felix,

Actually this was happening because in usermode KFD library the 
doorbell-size is for non-SOC15() hardware is hard coded to 4 bytes.

We added this fix just so that the library code doesn't need to be 
changed, but can still get aligned to kernel code.

GFX Usermode queue code uses this doorbell-index parameter, and it gives 
us the right offset.


Regards

Shashank

>
>> +
>> +	/* Adjust the absolute doorbell offset against the doorbell id considering
>> +	 * the doorbell size of 32/64 bit.
>> +	 */
>> +	if (!KFD_IS_SOC15(dev))
>> +		q->properties.doorbell_off += q->doorbell_id;
>> +	else
>> +		q->properties.doorbell_off += q->doorbell_id * 2;
>
> This could be simplified and generalized as
>
> 	q->properties.doorbell_off += q->doorbell_id * dev->kfd->device_info.doorbell_size / 4;

Agree, we can do this simplification.

- Shashank

> Regards,
>   Felix
>
>
>> +
>>   	return 0;
>>   }
>>