Re: [PATCH] drm/msm: a5xx: Make preemption reset case reentrant

From: Jordan Crouse
Date: Tue Nov 03 2020 - 12:30:23 EST


On Mon, Nov 02, 2020 at 09:02:25PM +0100, Marijn Suijten wrote:
> nr_rings is reset to 1, but when this function is called for a second
> (and third!) time nr_rings > 1 is false, thus the else case is entered
> to set up a buffer for the RPTR shadow and consequently written to
> RB_RPTR_ADDR, hanging platforms without WHERE_AM_I firmware support.
>
> Restructure the condition in such a way that shadow buffer setup only
> ever happens when has_whereami is true; otherwise preemption is only
> finalized when the number of ring buffers has not been reset to 1 yet.
>
> Fixes: 8907afb476ac ("drm/msm: Allow a5xx to mark the RPTR shadow as privileged")
> Signed-off-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>

Way better. Thanks for doing this.

Reviewed-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>

> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index d6804a802355..9a202a7da131 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -755,12 +755,8 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
> gpu_write(gpu, REG_A5XX_CP_RB_CNTL,
> MSM_GPU_RB_CNTL_DEFAULT | AXXX_CP_RB_CNTL_NO_UPDATE);
>
> - /* Disable preemption if WHERE_AM_I isn't available */
> - if (!a5xx_gpu->has_whereami && gpu->nr_rings > 1) {
> - a5xx_preempt_fini(gpu);
> - gpu->nr_rings = 1;
> - } else {
> - /* Create a privileged buffer for the RPTR shadow */
> + /* Create a privileged buffer for the RPTR shadow */
> + if (a5xx_gpu->has_whereami) {
> if (!a5xx_gpu->shadow_bo) {
> a5xx_gpu->shadow = msm_gem_kernel_new(gpu->dev,
> sizeof(u32) * gpu->nr_rings,
> @@ -774,6 +770,10 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
>
> gpu_write64(gpu, REG_A5XX_CP_RB_RPTR_ADDR,
> REG_A5XX_CP_RB_RPTR_ADDR_HI, shadowptr(a5xx_gpu, gpu->rb[0]));
> + } else if (gpu->nr_rings > 1) {
> + /* Disable preemption if WHERE_AM_I isn't available */
> + a5xx_preempt_fini(gpu);
> + gpu->nr_rings = 1;
> }
>
> a5xx_preempt_hw_init(gpu);
> --
> 2.29.2
>

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project