On 20.08.2024 12:45 PM, Connor Abbott wrote: > On Tue, Aug 20, 2024 at 11:15 AM Konrad Dybcio <konradybcio@xxxxxxxxxx> wrote: >> >> On 15.08.2024 8:26 PM, Antonino Maniscalco wrote: >>> The bv_fence field of rbmemptrs was being used incorrectly as the BV >>> rptr shadow pointer in some places. >>> >>> Add a bv_rptr field and change the code to use that instead. >>> >>> Signed-off-by: Antonino Maniscalco <antomani103@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- >>> drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + >>> 2 files changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> index bcaec86ac67a..32a4faa93d7f 100644 >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>> @@ -1132,7 +1132,7 @@ static int hw_init(struct msm_gpu *gpu) >>> /* ..which means "always" on A7xx, also for BV shadow */ >>> if (adreno_is_a7xx(adreno_gpu)) { >>> gpu_write64(gpu, REG_A7XX_CP_BV_RB_RPTR_ADDR, >>> - rbmemptr(gpu->rb[0], bv_fence)); >>> + rbmemptr(gpu->rb[0], bv_rptr)); >>> } >>> >>> /* Always come up on rb 0 */ >>> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h >>> index 0d6beb8cd39a..40791b2ade46 100644 >>> --- a/drivers/gpu/drm/msm/msm_ringbuffer.h >>> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h >>> @@ -31,6 +31,7 @@ struct msm_rbmemptrs { >>> volatile uint32_t rptr; >>> volatile uint32_t fence; >>> /* Introduced on A7xx */ >>> + volatile uint32_t bv_rptr; >> >> This is never initialized or assigned any value, no? >> >> Konrad > > Neither is the original (retroactively BR) shadow RPTR, except > apparently on suspend (no idea why). It's written by the GPU as it > reads the ringbuffer, because CP_BV_RPTR_ADDR is set to its address. > For the BV shadow RPTR, we aren't really using it for anything (and > neither is kgsl) so we just need to point the register to a valid > "dummy" address that isn't used by anything else. Alright, thanks Konrad