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

Akhil P Oommen
2026-03-23T21:35:22Z

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