| Message ID | 1579868411-20837-1-git-send-email-akhilpo@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e8e35c62ba517f73cca32bc9925d62f4c4981768 |
| Headers | show |
| Series | drm/msm/a6xx: Correct the highestbank configuration | expand |
On Fri, Jan 24, 2020 at 5:50 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote: > > Highest bank bit configuration is different for a618 gpu. Update > it with the correct configuration which is the reset value incidentally. > > Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org> > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> Thanks, this fixes the UBWC issues on a618 Fixes: e812744c5f95 ("drm: msm: a6xx: Add support for A618") Reviewed-by: Rob Clark <robdclark@gmail.com> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index daf0780..536d196 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -470,10 +470,12 @@ static int a6xx_hw_init(struct msm_gpu *gpu) > /* Select CP0 to always count cycles */ > gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT); > > - gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > + if (adreno_is_a630(adreno_gpu)) { > + gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > + } > > /* Enable fault detection */ > gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, > -- > 2.7.4 > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno
On Fri, Jan 24, 2020 at 05:50:11PM +0530, Akhil P Oommen wrote: > Highest bank bit configuration is different for a618 gpu. Update > it with the correct configuration which is the reset value incidentally. > > Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org> > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index daf0780..536d196 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -470,10 +470,12 @@ static int a6xx_hw_init(struct msm_gpu *gpu) > /* Select CP0 to always count cycles */ > gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT); > > - gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > + if (adreno_is_a630(adreno_gpu)) { > + gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > + } it shouldn't come as a surprise that everything in the a6xx family is going to have a highest bank bit setting. Even though the a618 uses the reset value, I think it would be less confusing to future folks if we explicitly program it: if (adreno_is_a630(adreno_dev)) hbb = 2; else hbb = 0; .... Jordan
On 1/24/2020 11:56 PM, Jordan Crouse wrote: > On Fri, Jan 24, 2020 at 05:50:11PM +0530, Akhil P Oommen wrote: >> Highest bank bit configuration is different for a618 gpu. Update >> it with the correct configuration which is the reset value incidentally. >> >> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org> >> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index daf0780..536d196 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -470,10 +470,12 @@ static int a6xx_hw_init(struct msm_gpu *gpu) >> /* Select CP0 to always count cycles */ >> gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT); >> >> - gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); >> - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); >> - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); >> - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); >> + if (adreno_is_a630(adreno_gpu)) { >> + gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); >> + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); >> + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); >> + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); >> + } > it shouldn't come as a surprise that everything in the a6xx family is going to > have a highest bank bit setting. Even though the a618 uses the reset value, I > think it would be less confusing to future folks if we explicitly program it: > > if (adreno_is_a630(adreno_dev)) > hbb = 2; > else > hbb = 0; I think it would be better if we keep this in the adreno_info. Yes, this would waste a tiny bit of space for other gpu entries in the gpulist. It is also possible to move this to a separate struct and keep a pointer to it in the adreno_info. But that is something we should try when there are more a6xx specific configurations in future. I have a new patch, but testing it is taking longer that I expected. I will share it as soon as possible. > .... > > Jordan > Akhil
On Fri, Jan 31, 2020 at 12:00 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote: > > On 1/24/2020 11:56 PM, Jordan Crouse wrote: > > On Fri, Jan 24, 2020 at 05:50:11PM +0530, Akhil P Oommen wrote: > >> Highest bank bit configuration is different for a618 gpu. Update > >> it with the correct configuration which is the reset value incidentally. > >> > >> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org> > >> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> > >> --- > >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> index daf0780..536d196 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> @@ -470,10 +470,12 @@ static int a6xx_hw_init(struct msm_gpu *gpu) > >> /* Select CP0 to always count cycles */ > >> gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT); > >> > >> - gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > >> - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > >> - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > >> - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > >> + if (adreno_is_a630(adreno_gpu)) { > >> + gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); > >> + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); > >> + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); > >> + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); > >> + } > > it shouldn't come as a surprise that everything in the a6xx family is going to > > have a highest bank bit setting. Even though the a618 uses the reset value, I > > think it would be less confusing to future folks if we explicitly program it: > > > > if (adreno_is_a630(adreno_dev)) > > hbb = 2; > > else > > hbb = 0; > > I think it would be better if we keep this in the adreno_info. Yes, this > would waste a tiny bit of space for other gpu > entries in the gpulist. It is also possible to move this to a separate > struct and keep a pointer to it in the adreno_info. > But that is something we should try when there are more a6xx specific > configurations in future. > > I have a new patch, but testing it is taking longer that I expected. I > will share it as soon as possible. > I'm going to pull this in as-is for msm-fixes. Please rebase the change that you haven't posted yet on top of this patch, and send it as a cleanup for the next cycle. Thanks BR, -R
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index daf0780..536d196 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -470,10 +470,12 @@ static int a6xx_hw_init(struct msm_gpu *gpu) /* Select CP0 to always count cycles */ gpu_write(gpu, REG_A6XX_CP_PERFCTR_CP_SEL_0, PERF_CP_ALWAYS_COUNT); - gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); - gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); - gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); - gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); + if (adreno_is_a630(adreno_gpu)) { + gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL, 2 << 1); + gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, 2 << 1); + gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, 2 << 1); + gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, 2 << 21); + } /* Enable fault detection */ gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL,