| Message ID | 20240410060549.201177-1-ashutosh.dixit@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915: Don't enable hwmon for selftests | expand |
On 10-04-2024 11:35, Ashutosh Dixit wrote: > There are no hwmon selftests so there is no need to enable hwmon for > selftests. So enable hwmon only for real driver load. > > v2: Move the logic inside i915_hwmon.c > v3: Move is_i915_selftest definition to i915_selftest.h (Badal) > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10366 > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > --- > drivers/gpu/drm/i915/i915_hwmon.c | 3 ++- > drivers/gpu/drm/i915/i915_selftest.h | 10 ++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > index 8c3f443c8347..cf1689333ebf 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -10,6 +10,7 @@ > #include "i915_drv.h" > #include "i915_hwmon.h" > #include "i915_reg.h" > +#include "i915_selftest.h" > #include "intel_mchbar_regs.h" > #include "intel_pcode.h" > #include "gt/intel_gt.h" > @@ -789,7 +790,7 @@ void i915_hwmon_register(struct drm_i915_private *i915) > int i; > > /* hwmon is available only for dGfx */ > - if (!IS_DGFX(i915)) > + if (!IS_DGFX(i915) || is_i915_selftest()) > return; > > hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL); > diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h > index bdf3e22c0a34..19e5076823a7 100644 > --- a/drivers/gpu/drm/i915/i915_selftest.h > +++ b/drivers/gpu/drm/i915/i915_selftest.h > @@ -111,6 +111,11 @@ int __i915_subtests(const char *caller, > #define I915_SELFTEST_ONLY(x) unlikely(x) > #define I915_SELFTEST_EXPORT > > +static inline bool is_i915_selftest(void) > +{ > + return i915_selftest.live || i915_selftest.perf || i915_selftest.mock; > +} > + > #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */ > > static inline int i915_mock_selftests(void) { return 0; } > @@ -121,6 +126,11 @@ static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; } > #define I915_SELFTEST_ONLY(x) 0 > #define I915_SELFTEST_EXPORT static > > +static inline bool is_i915_selftest(void) > +{ > + return false; > +} > + Looks good to me. Reviewed-by: Badal Nilawar <badal.nilawar@intel.com> > #endif > > /* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
Hi Ashutosh, please use "git format-patch -v 3 ..." which generates subject [PATCH v3] ...". Otherwise it gets confusing to see the patch that needs to be reviewed. On Tue, Apr 09, 2024 at 11:05:49PM -0700, Ashutosh Dixit wrote: > There are no hwmon selftests so there is no need to enable hwmon for > selftests. So enable hwmon only for real driver load. > > v2: Move the logic inside i915_hwmon.c > v3: Move is_i915_selftest definition to i915_selftest.h (Badal) > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10366 > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > --- > drivers/gpu/drm/i915/i915_hwmon.c | 3 ++- > drivers/gpu/drm/i915/i915_selftest.h | 10 ++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > index 8c3f443c8347..cf1689333ebf 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -10,6 +10,7 @@ > #include "i915_drv.h" > #include "i915_hwmon.h" > #include "i915_reg.h" > +#include "i915_selftest.h" > #include "intel_mchbar_regs.h" > #include "intel_pcode.h" > #include "gt/intel_gt.h" > @@ -789,7 +790,7 @@ void i915_hwmon_register(struct drm_i915_private *i915) > int i; > > /* hwmon is available only for dGfx */ > - if (!IS_DGFX(i915)) > + if (!IS_DGFX(i915) || is_i915_selftest()) > return; I wonder if this is the right place to put it or rather place it in i915_driver.c and avoid calling i915_hwmon_register() at all. In any case, it's good: Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks, Andi
On Wed, 10 Apr 2024 06:53:15 -0700, Andi Shyti wrote: > Hi Andi, > please use "git format-patch -v 3 ..." which generates subject > [PATCH v3] ...". Otherwise it gets confusing to see the patch > that needs to be reviewed. Sure, sorry! > > On Tue, Apr 09, 2024 at 11:05:49PM -0700, Ashutosh Dixit wrote: > > There are no hwmon selftests so there is no need to enable hwmon for > > selftests. So enable hwmon only for real driver load. > > > > v2: Move the logic inside i915_hwmon.c > > v3: Move is_i915_selftest definition to i915_selftest.h (Badal) > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10366 > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > > --- > > drivers/gpu/drm/i915/i915_hwmon.c | 3 ++- > > drivers/gpu/drm/i915/i915_selftest.h | 10 ++++++++++ > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > > index 8c3f443c8347..cf1689333ebf 100644 > > --- a/drivers/gpu/drm/i915/i915_hwmon.c > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > > @@ -10,6 +10,7 @@ > > #include "i915_drv.h" > > #include "i915_hwmon.h" > > #include "i915_reg.h" > > +#include "i915_selftest.h" > > #include "intel_mchbar_regs.h" > > #include "intel_pcode.h" > > #include "gt/intel_gt.h" > > @@ -789,7 +790,7 @@ void i915_hwmon_register(struct drm_i915_private *i915) > > int i; > > > > /* hwmon is available only for dGfx */ > > - if (!IS_DGFX(i915)) > > + if (!IS_DGFX(i915) || is_i915_selftest()) > > return; > > I wonder if this is the right place to put it or rather place it > in i915_driver.c and avoid calling i915_hwmon_register() at all. I thought it was better put it here rather than clutter up common code in i915_driver.c. > > In any case, it's good: > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Thanks. -- Ashutosh
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c index 8c3f443c8347..cf1689333ebf 100644 --- a/drivers/gpu/drm/i915/i915_hwmon.c +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -10,6 +10,7 @@ #include "i915_drv.h" #include "i915_hwmon.h" #include "i915_reg.h" +#include "i915_selftest.h" #include "intel_mchbar_regs.h" #include "intel_pcode.h" #include "gt/intel_gt.h" @@ -789,7 +790,7 @@ void i915_hwmon_register(struct drm_i915_private *i915) int i; /* hwmon is available only for dGfx */ - if (!IS_DGFX(i915)) + if (!IS_DGFX(i915) || is_i915_selftest()) return; hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL); diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h index bdf3e22c0a34..19e5076823a7 100644 --- a/drivers/gpu/drm/i915/i915_selftest.h +++ b/drivers/gpu/drm/i915/i915_selftest.h @@ -111,6 +111,11 @@ int __i915_subtests(const char *caller, #define I915_SELFTEST_ONLY(x) unlikely(x) #define I915_SELFTEST_EXPORT +static inline bool is_i915_selftest(void) +{ + return i915_selftest.live || i915_selftest.perf || i915_selftest.mock; +} + #else /* !IS_ENABLED(CONFIG_DRM_I915_SELFTEST) */ static inline int i915_mock_selftests(void) { return 0; } @@ -121,6 +126,11 @@ static inline int i915_perf_selftests(struct pci_dev *pdev) { return 0; } #define I915_SELFTEST_ONLY(x) 0 #define I915_SELFTEST_EXPORT static +static inline bool is_i915_selftest(void) +{ + return false; +} + #endif /* Using the i915_selftest_ prefix becomes a little unwieldy with the helpers.
There are no hwmon selftests so there is no need to enable hwmon for selftests. So enable hwmon only for real driver load. v2: Move the logic inside i915_hwmon.c v3: Move is_i915_selftest definition to i915_selftest.h (Badal) Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10366 Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> --- drivers/gpu/drm/i915/i915_hwmon.c | 3 ++- drivers/gpu/drm/i915/i915_selftest.h | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)