| Message ID | 20230223-topic-gmuwrapper-v8-7-69c68206609e@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GMU-less A6xx support (A610, A619_holi) | expand |
On Mon, May 29, 2023 at 03:52:26PM +0200, Konrad Dybcio wrote: > > Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper > GPUs and reuse it in a6xx_gmu_force_off(). > > This helper, contrary to the original usage in GMU code paths, adds > a write memory barrier which together with the necessary delay should > ensure that the reset is never deasserted too quickly due to e.g. OoO > execution going crazy. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ > drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > 3 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index b86be123ecd0..5ba8cba69383 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) > a6xx_bus_clear_pending_transactions(adreno_gpu, true); > > /* Reset GPU core blocks */ > - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); > - udelay(100); > + a6xx_gpu_sw_reset(gpu, true); > } > > static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index e3ac3f045665..083ccb5bcb4e 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ > gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); > } > > +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) > +{ > + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); > + /* Add a barrier to avoid bad surprises */ Can you please make this comment a bit more clear? Highlight that we should ensure the register is posted at hw before polling. I think this barrier is required only during assert. -Akhil. > + mb(); > + > + /* The reset line needs to be asserted for at least 100 us */ > + if (assert) > + udelay(100); > +} > + > static int a6xx_pm_resume(struct msm_gpu *gpu) > { > struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > index 9580def06d45..aa70390ee1c6 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); > int a6xx_gpu_state_put(struct msm_gpu_state *state); > > void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); > +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); > > #endif /* __A6XX_GPU_H__ */ > > -- > 2.40.1 >
On 6.06.2023 19:18, Akhil P Oommen wrote: > On Mon, May 29, 2023 at 03:52:26PM +0200, Konrad Dybcio wrote: >> >> Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper >> GPUs and reuse it in a6xx_gmu_force_off(). >> >> This helper, contrary to the original usage in GMU code paths, adds >> a write memory barrier which together with the necessary delay should >> ensure that the reset is never deasserted too quickly due to e.g. OoO >> execution going crazy. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ >> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + >> 3 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> index b86be123ecd0..5ba8cba69383 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) >> a6xx_bus_clear_pending_transactions(adreno_gpu, true); >> >> /* Reset GPU core blocks */ >> - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); >> - udelay(100); >> + a6xx_gpu_sw_reset(gpu, true); >> } >> >> static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> index e3ac3f045665..083ccb5bcb4e 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >> @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ >> gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); >> } >> >> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) >> +{ >> + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); >> + /* Add a barrier to avoid bad surprises */ > Can you please make this comment a bit more clear? Highlight that we > should ensure the register is posted at hw before polling. > > I think this barrier is required only during assert. Generally it should not be strictly required at all, but I'm thinking that it'd be good to keep it in both cases, so that: if (assert) we don't keep writing things to the GPU if it's in reset else we don't start writing things to the GPU becomes it comes out of reset Also, if you squint hard enough at the commit message, you'll notice I intended for this so only be a wmb, but for some reason generalized it.. Perhaps that's another thing I should fix! for v9.. Konrad > > -Akhil. >> + mb(); >> + >> + /* The reset line needs to be asserted for at least 100 us */ >> + if (assert) >> + udelay(100); >> +} >> + >> static int a6xx_pm_resume(struct msm_gpu *gpu) >> { >> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> index 9580def06d45..aa70390ee1c6 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); >> int a6xx_gpu_state_put(struct msm_gpu_state *state); >> >> void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); >> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); >> >> #endif /* __A6XX_GPU_H__ */ >> >> -- >> 2.40.1 >>
On Thu, Jun 15, 2023 at 12:34:06PM +0200, Konrad Dybcio wrote: > > On 6.06.2023 19:18, Akhil P Oommen wrote: > > On Mon, May 29, 2023 at 03:52:26PM +0200, Konrad Dybcio wrote: > >> > >> Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper > >> GPUs and reuse it in a6xx_gmu_force_off(). > >> > >> This helper, contrary to the original usage in GMU code paths, adds > >> a write memory barrier which together with the necessary delay should > >> ensure that the reset is never deasserted too quickly due to e.g. OoO > >> execution going crazy. > >> > >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >> --- > >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- > >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ > >> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > >> 3 files changed, 13 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> index b86be123ecd0..5ba8cba69383 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) > >> a6xx_bus_clear_pending_transactions(adreno_gpu, true); > >> > >> /* Reset GPU core blocks */ > >> - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); > >> - udelay(100); > >> + a6xx_gpu_sw_reset(gpu, true); > >> } > >> > >> static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> index e3ac3f045665..083ccb5bcb4e 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >> @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ > >> gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); > >> } > >> > >> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) > >> +{ > >> + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); > >> + /* Add a barrier to avoid bad surprises */ > > Can you please make this comment a bit more clear? Highlight that we > > should ensure the register is posted at hw before polling. > > > > I think this barrier is required only during assert. > Generally it should not be strictly required at all, but I'm thinking > that it'd be good to keep it in both cases, so that: > > if (assert) > we don't keep writing things to the GPU if it's in reset > else > we don't start writing things to the GPU becomes it comes > out of reset > > Also, if you squint hard enough at the commit message, you'll notice > I intended for this so only be a wmb, but for some reason generalized > it.. Perhaps that's another thing I should fix! > for v9.. wmb() doesn't provide any ordering guarantee with the delay loop. A common practice is to just read back the same register before the loop because a readl followed by delay() is guaranteed to be ordered. -Akhil. > > Konrad > > > > -Akhil. > >> + mb(); > >> + > >> + /* The reset line needs to be asserted for at least 100 us */ > >> + if (assert) > >> + udelay(100); > >> +} > >> + > >> static int a6xx_pm_resume(struct msm_gpu *gpu) > >> { > >> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >> index 9580def06d45..aa70390ee1c6 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >> @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); > >> int a6xx_gpu_state_put(struct msm_gpu_state *state); > >> > >> void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); > >> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); > >> > >> #endif /* __A6XX_GPU_H__ */ > >> > >> -- > >> 2.40.1 > >>
On 15.06.2023 22:11, Akhil P Oommen wrote: > On Thu, Jun 15, 2023 at 12:34:06PM +0200, Konrad Dybcio wrote: >> >> On 6.06.2023 19:18, Akhil P Oommen wrote: >>> On Mon, May 29, 2023 at 03:52:26PM +0200, Konrad Dybcio wrote: >>>> >>>> Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper >>>> GPUs and reuse it in a6xx_gmu_force_off(). >>>> >>>> This helper, contrary to the original usage in GMU code paths, adds >>>> a write memory barrier which together with the necessary delay should >>>> ensure that the reset is never deasserted too quickly due to e.g. OoO >>>> execution going crazy. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>> --- >>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + >>>> 3 files changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >>>> index b86be123ecd0..5ba8cba69383 100644 >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >>>> @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) >>>> a6xx_bus_clear_pending_transactions(adreno_gpu, true); >>>> >>>> /* Reset GPU core blocks */ >>>> - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); >>>> - udelay(100); >>>> + a6xx_gpu_sw_reset(gpu, true); >>>> } >>>> >>>> static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>> index e3ac3f045665..083ccb5bcb4e 100644 >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c >>>> @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ >>>> gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); >>>> } >>>> >>>> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) >>>> +{ >>>> + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); >>>> + /* Add a barrier to avoid bad surprises */ >>> Can you please make this comment a bit more clear? Highlight that we >>> should ensure the register is posted at hw before polling. >>> >>> I think this barrier is required only during assert. >> Generally it should not be strictly required at all, but I'm thinking >> that it'd be good to keep it in both cases, so that: >> >> if (assert) >> we don't keep writing things to the GPU if it's in reset >> else >> we don't start writing things to the GPU becomes it comes >> out of reset >> >> Also, if you squint hard enough at the commit message, you'll notice >> I intended for this so only be a wmb, but for some reason generalized >> it.. Perhaps that's another thing I should fix! >> for v9.. > > wmb() doesn't provide any ordering guarantee with the delay loop. Hm, fair.. I'm still not as fluent with memory access knowledge as I'd like to be.. > A common practice is to just read back the same register before > the loop because a readl followed by delay() is guaranteed to be ordered. So, how should I proceed? Keep the r/w barrier, or add a readback and a tiiiny (perhaps even using ndelay instead of udelay?) delay on de-assert? Konrad > > -Akhil. >> >> Konrad >>> >>> -Akhil. >>>> + mb(); >>>> + >>>> + /* The reset line needs to be asserted for at least 100 us */ >>>> + if (assert) >>>> + udelay(100); >>>> +} >>>> + >>>> static int a6xx_pm_resume(struct msm_gpu *gpu) >>>> { >>>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >>>> index 9580def06d45..aa70390ee1c6 100644 >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >>>> @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); >>>> int a6xx_gpu_state_put(struct msm_gpu_state *state); >>>> >>>> void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); >>>> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); >>>> >>>> #endif /* __A6XX_GPU_H__ */ >>>> >>>> -- >>>> 2.40.1 >>>>
On Thu, Jun 15, 2023 at 10:59:23PM +0200, Konrad Dybcio wrote: > > On 15.06.2023 22:11, Akhil P Oommen wrote: > > On Thu, Jun 15, 2023 at 12:34:06PM +0200, Konrad Dybcio wrote: > >> > >> On 6.06.2023 19:18, Akhil P Oommen wrote: > >>> On Mon, May 29, 2023 at 03:52:26PM +0200, Konrad Dybcio wrote: > >>>> > >>>> Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper > >>>> GPUs and reuse it in a6xx_gmu_force_off(). > >>>> > >>>> This helper, contrary to the original usage in GMU code paths, adds > >>>> a write memory barrier which together with the necessary delay should > >>>> ensure that the reset is never deasserted too quickly due to e.g. OoO > >>>> execution going crazy. > >>>> > >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > >>>> --- > >>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- > >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ > >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > >>>> 3 files changed, 13 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>>> index b86be123ecd0..5ba8cba69383 100644 > >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>>> @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) > >>>> a6xx_bus_clear_pending_transactions(adreno_gpu, true); > >>>> > >>>> /* Reset GPU core blocks */ > >>>> - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); > >>>> - udelay(100); > >>>> + a6xx_gpu_sw_reset(gpu, true); > >>>> } > >>>> > >>>> static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) > >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> index e3ac3f045665..083ccb5bcb4e 100644 > >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > >>>> @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ > >>>> gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); > >>>> } > >>>> > >>>> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) > >>>> +{ > >>>> + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); > >>>> + /* Add a barrier to avoid bad surprises */ > >>> Can you please make this comment a bit more clear? Highlight that we > >>> should ensure the register is posted at hw before polling. > >>> > >>> I think this barrier is required only during assert. > >> Generally it should not be strictly required at all, but I'm thinking > >> that it'd be good to keep it in both cases, so that: > >> > >> if (assert) > >> we don't keep writing things to the GPU if it's in reset > >> else > >> we don't start writing things to the GPU becomes it comes > >> out of reset > >> > >> Also, if you squint hard enough at the commit message, you'll notice > >> I intended for this so only be a wmb, but for some reason generalized > >> it.. Perhaps that's another thing I should fix! > >> for v9.. > > > > wmb() doesn't provide any ordering guarantee with the delay loop. > Hm, fair.. I'm still not as fluent with memory access knowledge as I'd > like to be.. > > > A common practice is to just read back the same register before > > the loop because a readl followed by delay() is guaranteed to be ordered. > So, how should I proceed? Keep the r/w barrier, or add a readback and > a tiiiny (perhaps even using ndelay instead of udelay?) delay on de-assert? readback + delay (similar value as downstream). This path is exercised rarely. -Akhil. > > Konrad > > > > -Akhil. > >> > >> Konrad > >>> > >>> -Akhil. > >>>> + mb(); > >>>> + > >>>> + /* The reset line needs to be asserted for at least 100 us */ > >>>> + if (assert) > >>>> + udelay(100); > >>>> +} > >>>> + > >>>> static int a6xx_pm_resume(struct msm_gpu *gpu) > >>>> { > >>>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >>>> index 9580def06d45..aa70390ee1c6 100644 > >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h > >>>> @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); > >>>> int a6xx_gpu_state_put(struct msm_gpu_state *state); > >>>> > >>>> void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); > >>>> +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); > >>>> > >>>> #endif /* __A6XX_GPU_H__ */ > >>>> > >>>> -- > >>>> 2.40.1 > >>>>
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index b86be123ecd0..5ba8cba69383 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -899,8 +899,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu) a6xx_bus_clear_pending_transactions(adreno_gpu, true); /* Reset GPU core blocks */ - gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, 1); - udelay(100); + a6xx_gpu_sw_reset(gpu, true); } static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index e3ac3f045665..083ccb5bcb4e 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -1634,6 +1634,17 @@ void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_ gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0); } +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert) +{ + gpu_write(gpu, REG_A6XX_RBBM_SW_RESET_CMD, assert); + /* Add a barrier to avoid bad surprises */ + mb(); + + /* The reset line needs to be asserted for at least 100 us */ + if (assert) + udelay(100); +} + static int a6xx_pm_resume(struct msm_gpu *gpu) { struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h index 9580def06d45..aa70390ee1c6 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h @@ -89,5 +89,6 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu); int a6xx_gpu_state_put(struct msm_gpu_state *state); void a6xx_bus_clear_pending_transactions(struct adreno_gpu *adreno_gpu, bool gx_off); +void a6xx_gpu_sw_reset(struct msm_gpu *gpu, bool assert); #endif /* __A6XX_GPU_H__ */
Introduce a6xx_gpu_sw_reset() in preparation for adding GMU wrapper GPUs and reuse it in a6xx_gmu_force_off(). This helper, contrary to the original usage in GMU code paths, adds a write memory barrier which together with the necessary delay should ensure that the reset is never deasserted too quickly due to e.g. OoO execution going crazy. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 3 +-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++++++++++ drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + 3 files changed, 13 insertions(+), 2 deletions(-)