On Tue, Jul 4, 2023 at 10:20 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On Tue, 4 Jul 2023 at 19:36, Rob Clark <robdclark@xxxxxxxxx> wrote:
> >
> > From: Rob Clark <robdclark@xxxxxxxxxxxx>
> >
> > Recently, a WARN_ON() was introduced to ensure that revn is filled before
> > adreno_is_aXYZ is called. This however doesn't work very well when revn is
> > 0 by design (such as for A635).
> >
> > Cc: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> > Fixes: cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before being set")
> > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index 65379e4824d9..ef1bcb6b624e 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -149,7 +149,8 @@ bool adreno_cmp_rev(struct adreno_rev rev1, struct adreno_rev rev2);
> >
> > static inline bool adreno_is_revn(const struct adreno_gpu *gpu, uint32_t revn)
> > {
> > - WARN_ON_ONCE(!gpu->revn);
> > + /* revn can be zero, but if not is set at same time as info */
> > + WARN_ON_ONCE(!gpu->info);
> >
> > return gpu->revn == revn;
> > }
> > @@ -161,14 +162,16 @@ static inline bool adreno_has_gmu_wrapper(const struct adreno_gpu *gpu)
> >
> > static inline bool adreno_is_a2xx(const struct adreno_gpu *gpu)
> > {
> > - WARN_ON_ONCE(!gpu->revn);
> > + /* revn can be zero, but if not is set at same time as info */
> > + WARN_ON_ONCE(!gpu->info);
> >
> > return (gpu->revn < 300);
>
> This is then incorrect for a635 / a690 if they have revn at 0.
Fortunately not any more broken that it has ever been. But as long as
sc7280/sc8280 have GPU OPP tables, you'd never hit this. I'm working
on a better solution for next merge window.
BR,
-R
> > }
> >
> > static inline bool adreno_is_a20x(const struct adreno_gpu *gpu)
> > {
> > - WARN_ON_ONCE(!gpu->revn);
> > + /* revn can be zero, but if not is set at same time as info */
> > + WARN_ON_ONCE(!gpu->info);
> >
> > return (gpu->revn < 210);
>
> And this too.
>
> > }
> > --
> > 2.41.0
> >
>
>
> --
> With best wishes
> Dmitry
