1. Patchew
  2. linux
  3. View series

[PATCH] drm/msm/adreno: Make adreno quirks not overwrite each other

Konrad Dybcio posted 1 patch 3 years, 1 month ago
There is a newer version of this series
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH] drm/msm/adreno: Make adreno quirks not overwrite each other
Posted by Konrad Dybcio 3 years, 1 month ago 
So far the adreno quirks have all been assigned with an OR operator,
which is problematic, because they were assigned consecutive integer
values, which makes checking them with an AND operator kind of no bueno..

Switch to using BIT(n) so that only the quirks that the programmer chose
are taken into account when evaluating info->quirks & ADRENO_QUIRK_...

Fixes: b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index c85857c0a228..5eb254c9832a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -29,11 +29,9 @@ enum {
 	ADRENO_FW_MAX,
 };
 
-enum adreno_quirks {
-	ADRENO_QUIRK_TWO_PASS_USE_WFI = 1,
-	ADRENO_QUIRK_FAULT_DETECT_MASK = 2,
-	ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
-};
+#define ADRENO_QUIRK_TWO_PASS_USE_WFI		BIT(0)
+#define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
+#define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
 
 struct adreno_rev {
 	uint8_t  core;
@@ -65,7 +63,7 @@ struct adreno_info {
 	const char *name;
 	const char *fw[ADRENO_FW_MAX];
 	uint32_t gmem;
-	enum adreno_quirks quirks;
+	u64 quirks;
 	struct msm_gpu *(*init)(struct drm_device *dev);
 	const char *zapfw;
 	u32 inactive_period;
-- 
2.39.0
Re: [PATCH] drm/msm/adreno: Make adreno quirks not overwrite each other
Posted by Dmitry Baryshkov 3 years, 1 month ago 
On 29/12/2022 12:18, Konrad Dybcio wrote:
> So far the adreno quirks have all been assigned with an OR operator,
> which is problematic, because they were assigned consecutive integer
> values, which makes checking them with an AND operator kind of no bueno..
> 
> Switch to using BIT(n) so that only the quirks that the programmer chose
> are taken into account when evaluating info->quirks & ADRENO_QUIRK_...
> 
> Fixes: b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>   drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)-- 
With best wishes
Dmitry
Re: [PATCH] drm/msm/adreno: Make adreno quirks not overwrite each other
Posted by Marijn Suijten 3 years, 1 month ago 
On 2022-12-29 11:18:45, Konrad Dybcio wrote:
> So far the adreno quirks have all been assigned with an OR operator,
> which is problematic, because they were assigned consecutive integer
> values, which makes checking them with an AND operator kind of no bueno..
> 
> Switch to using BIT(n) so that only the quirks that the programmer chose
> are taken into account when evaluating info->quirks & ADRENO_QUIRK_...
> 
> Fixes: b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Nice catch!

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

Not sure if it's the right Fixes commit though, as it would have worked
when ADRENO_QUIRK_LMLOADKILL_DISABLE was added with constant 4 instead
of 3 in 370063ee427a ("drm/msm/adreno: Add A540 support"), but then
using bitflags in an enum value type is invalid anyway, AFAIK.

- Marijn

> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index c85857c0a228..5eb254c9832a 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -29,11 +29,9 @@ enum {
>  	ADRENO_FW_MAX,
>  };
>  
> -enum adreno_quirks {
> -	ADRENO_QUIRK_TWO_PASS_USE_WFI = 1,
> -	ADRENO_QUIRK_FAULT_DETECT_MASK = 2,
> -	ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
> -};
> +#define ADRENO_QUIRK_TWO_PASS_USE_WFI		BIT(0)
> +#define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
> +#define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
>  
>  struct adreno_rev {
>  	uint8_t  core;
> @@ -65,7 +63,7 @@ struct adreno_info {
>  	const char *name;
>  	const char *fw[ADRENO_FW_MAX];
>  	uint32_t gmem;
> -	enum adreno_quirks quirks;
> +	u64 quirks;
>  	struct msm_gpu *(*init)(struct drm_device *dev);
>  	const char *zapfw;
>  	u32 inactive_period;
> -- 
> 2.39.0
>
Re: [PATCH] drm/msm/adreno: Make adreno quirks not overwrite each other
Posted by Rob Clark 3 years, 1 month ago 
On Thu, Dec 29, 2022 at 2:47 AM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2022-12-29 11:18:45, Konrad Dybcio wrote:
> > So far the adreno quirks have all been assigned with an OR operator,
> > which is problematic, because they were assigned consecutive integer
> > values, which makes checking them with an AND operator kind of no bueno..
> >
> > Switch to using BIT(n) so that only the quirks that the programmer chose
> > are taken into account when evaluating info->quirks & ADRENO_QUIRK_...
> >
> > Fixes: b5f103ab98c7 ("drm/msm: gpu: Add A5XX target support")
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>
> Nice catch!
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> Not sure if it's the right Fixes commit though, as it would have worked
> when ADRENO_QUIRK_LMLOADKILL_DISABLE was added with constant 4 instead
> of 3 in 370063ee427a ("drm/msm/adreno: Add A540 support"), but then
> using bitflags in an enum value type is invalid anyway, AFAIK.

It isn't a thing that c++ like so much, but for c code, gdb will
decode enum bitfields in a sensible way (IIRC).  Also, maybe it
doesn't matter at this point, but it would conflict for stable
backports prior to adding LMLOADKILL_DISABLE.

with the fixes msg corrected,

Reviewed-by: Rob Clark <robdclark@gmail.com>

> - Marijn
>
> > ---
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index c85857c0a228..5eb254c9832a 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -29,11 +29,9 @@ enum {
> >       ADRENO_FW_MAX,
> >  };
> >
> > -enum adreno_quirks {
> > -     ADRENO_QUIRK_TWO_PASS_USE_WFI = 1,
> > -     ADRENO_QUIRK_FAULT_DETECT_MASK = 2,
> > -     ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
> > -};
> > +#define ADRENO_QUIRK_TWO_PASS_USE_WFI                BIT(0)
> > +#define ADRENO_QUIRK_FAULT_DETECT_MASK               BIT(1)
> > +#define ADRENO_QUIRK_LMLOADKILL_DISABLE              BIT(2)
> >
> >  struct adreno_rev {
> >       uint8_t  core;
> > @@ -65,7 +63,7 @@ struct adreno_info {
> >       const char *name;
> >       const char *fw[ADRENO_FW_MAX];
> >       uint32_t gmem;
> > -     enum adreno_quirks quirks;
> > +     u64 quirks;
> >       struct msm_gpu *(*init)(struct drm_device *dev);
> >       const char *zapfw;
> >       u32 inactive_period;
> > --
> > 2.39.0
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.