| Message ID | 20201102200227.8876-1-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7cc29fcdfcc8784e97c5151c848e193800ec79ac |
| Headers | show |
| Series | drm/msm: a5xx: Make preemption reset case reentrant | expand |
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@somainline.org> > Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> Way better. Thanks for doing this. Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > 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 >
Hello: This patch was applied to qcom/linux.git (refs/heads/for-next): On Mon, 2 Nov 2020 21:02:25 +0100 you 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. > > [...] Here is the summary with links: - drm/msm: a5xx: Make preemption reset case reentrant https://git.kernel.org/qcom/c/7cc29fcdfcc8 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
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);
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@somainline.org> Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)