[PATCH 06/16] drm/msm/a6xx: Use barriers while updating HFI Q headers

Akhil P Oommen posted 16 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 06/16] drm/msm/a6xx: Use barriers while updating HFI Q headers
Posted by Akhil P Oommen 1 week, 5 days ago
To avoid harmful compiler optimizations and IO reordering in the HW, use
barriers and READ/WRITE_ONCE helpers as necessary while accessing the HFI
queue index variables.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
index 2daaa340366d..aef00c2dd137 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -34,7 +34,7 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
 	struct a6xx_hfi_queue_header *header = queue->header;
 	u32 i, hdr, index = header->read_index;
 
-	if (header->read_index == header->write_index) {
+	if (header->read_index == READ_ONCE(header->write_index)) {
 		header->rx_request = 1;
 		return 0;
 	}
@@ -62,7 +62,10 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
 	if (!gmu->legacy)
 		index = ALIGN(index, 4) % header->size;
 
-	header->read_index = index;
+	/*  Ensure all memory operations are complete before updating the read index */
+	dma_mb();
+
+	WRITE_ONCE(header->read_index, index);
 	return HFI_HEADER_SIZE(hdr);
 }
 
@@ -74,7 +77,7 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
 
 	spin_lock(&queue->lock);
 
-	space = CIRC_SPACE(header->write_index, header->read_index,
+	space = CIRC_SPACE(header->write_index, READ_ONCE(header->read_index),
 		header->size);
 	if (space < dwords) {
 		header->dropped++;
@@ -95,7 +98,10 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
 			queue->data[index] = 0xfafafafa;
 	}
 
-	header->write_index = index;
+	/*  Ensure all memory operations are complete before updating the write index */
+	dma_mb();
+
+	WRITE_ONCE(header->write_index, index);
 	spin_unlock(&queue->lock);
 
 	gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, 0x01);

-- 
2.51.0
Re: [PATCH 06/16] drm/msm/a6xx: Use barriers while updating HFI Q headers
Posted by Rob Clark 1 week, 5 days ago
On Mon, Mar 23, 2026 at 1:13 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> wrote:
>
> To avoid harmful compiler optimizations and IO reordering in the HW, use
> barriers and READ/WRITE_ONCE helpers as necessary while accessing the HFI
> queue index variables.

This feels a bit like it should have Fixes: ?

> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index 2daaa340366d..aef00c2dd137 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -34,7 +34,7 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
>         struct a6xx_hfi_queue_header *header = queue->header;
>         u32 i, hdr, index = header->read_index;
>
> -       if (header->read_index == header->write_index) {
> +       if (header->read_index == READ_ONCE(header->write_index)) {
>                 header->rx_request = 1;
>                 return 0;
>         }
> @@ -62,7 +62,10 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
>         if (!gmu->legacy)
>                 index = ALIGN(index, 4) % header->size;
>
> -       header->read_index = index;
> +       /*  Ensure all memory operations are complete before updating the read index */
> +       dma_mb();
> +
> +       WRITE_ONCE(header->read_index, index);
>         return HFI_HEADER_SIZE(hdr);
>  }
>
> @@ -74,7 +77,7 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
>
>         spin_lock(&queue->lock);
>
> -       space = CIRC_SPACE(header->write_index, header->read_index,
> +       space = CIRC_SPACE(header->write_index, READ_ONCE(header->read_index),
>                 header->size);
>         if (space < dwords) {
>                 header->dropped++;
> @@ -95,7 +98,10 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
>                         queue->data[index] = 0xfafafafa;
>         }
>
> -       header->write_index = index;
> +       /*  Ensure all memory operations are complete before updating the write index */
> +       dma_mb();
> +
> +       WRITE_ONCE(header->write_index, index);
>         spin_unlock(&queue->lock);
>
>         gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, 0x01);
>
> --
> 2.51.0
>
Re: [PATCH 06/16] drm/msm/a6xx: Use barriers while updating HFI Q headers
Posted by Akhil P Oommen 1 week, 5 days ago
On 3/24/2026 2:15 AM, Rob Clark wrote:
> On Mon, Mar 23, 2026 at 1:13 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> wrote:
>>
>> To avoid harmful compiler optimizations and IO reordering in the HW, use
>> barriers and READ/WRITE_ONCE helpers as necessary while accessing the HFI
>> queue index variables.
> 
> This feels a bit like it should have Fixes: ?

Ack. Will update in v2.

-Akhil

> 
>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>> ---
>>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
>> index 2daaa340366d..aef00c2dd137 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
>> @@ -34,7 +34,7 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
>>         struct a6xx_hfi_queue_header *header = queue->header;
>>         u32 i, hdr, index = header->read_index;
>>
>> -       if (header->read_index == header->write_index) {
>> +       if (header->read_index == READ_ONCE(header->write_index)) {
>>                 header->rx_request = 1;
>>                 return 0;
>>         }
>> @@ -62,7 +62,10 @@ static int a6xx_hfi_queue_read(struct a6xx_gmu *gmu,
>>         if (!gmu->legacy)
>>                 index = ALIGN(index, 4) % header->size;
>>
>> -       header->read_index = index;
>> +       /*  Ensure all memory operations are complete before updating the read index */
>> +       dma_mb();
>> +
>> +       WRITE_ONCE(header->read_index, index);
>>         return HFI_HEADER_SIZE(hdr);
>>  }
>>
>> @@ -74,7 +77,7 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
>>
>>         spin_lock(&queue->lock);
>>
>> -       space = CIRC_SPACE(header->write_index, header->read_index,
>> +       space = CIRC_SPACE(header->write_index, READ_ONCE(header->read_index),
>>                 header->size);
>>         if (space < dwords) {
>>                 header->dropped++;
>> @@ -95,7 +98,10 @@ static int a6xx_hfi_queue_write(struct a6xx_gmu *gmu,
>>                         queue->data[index] = 0xfafafafa;
>>         }
>>
>> -       header->write_index = index;
>> +       /*  Ensure all memory operations are complete before updating the write index */
>> +       dma_mb();
>> +
>> +       WRITE_ONCE(header->write_index, index);
>>         spin_unlock(&queue->lock);
>>
>>         gmu_write(gmu, REG_A6XX_GMU_HOST2GMU_INTR_SET, 0x01);
>>
>> --
>> 2.51.0
>>

Re: [PATCH 06/16] drm/msm/a6xx: Use barriers while updating HFI Q headers
Posted by Dmitry Baryshkov 1 week, 5 days ago
On Mon, Mar 23, 2026 at 01:45:16PM -0700, Rob Clark wrote:
> On Mon, Mar 23, 2026 at 1:13 PM Akhil P Oommen <akhilpo@oss.qualcomm.com> wrote:
> >
> > To avoid harmful compiler optimizations and IO reordering in the HW, use
> > barriers and READ/WRITE_ONCE helpers as necessary while accessing the HFI
> > queue index variables.
> 
> This feels a bit like it should have Fixes: ?

And as a friendly reminder, if it is a Fix, please move it towards the
start of the series.

-- 
With best wishes
Dmitry