| Message ID | 20240815-preemption-a750-t-v1-1-7bda26c34037@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Preemption support for A7XX | expand |
On Thu, Aug 15, 2024 at 08:26:11PM +0200, 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@gmail.com> Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com> -Akhil. > --- > 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; > volatile uint32_t bv_fence; > > volatile struct msm_gpu_submit_stats stats[MSM_GPU_SUBMIT_STATS_COUNT]; > > -- > 2.46.0 > >
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@gmail.com> > --- > 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
On Tue, Aug 20, 2024 at 11:15 AM Konrad Dybcio <konradybcio@kernel.org> 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@gmail.com> > > --- > > 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. Connor
On 20.08.2024 12:45 PM, Connor Abbott wrote: > On Tue, Aug 20, 2024 at 11:15 AM Konrad Dybcio <konradybcio@kernel.org> 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@gmail.com> >>> --- >>> 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
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; volatile uint32_t bv_fence; volatile struct msm_gpu_submit_stats stats[MSM_GPU_SUBMIT_STATS_COUNT];
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@gmail.com> --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- drivers/gpu/drm/msm/msm_ringbuffer.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)