On Mon, 03 Oct 2022 13:56:05 -0700, Andi Shyti wrote: Hi Andi, Badal is out for a bit so I am posting this version of the patches. > > Hi Badal, > > [...] > > > static void > > hwm_get_preregistration_info(struct drm_i915_private *i915) > > { > > + struct i915_hwmon *hwmon = i915->hwmon; > > + > > + if (IS_DG1(i915) || IS_DG2(i915)) > > why not GRAPHICS_VER(i915) >= 12 here? Thanks for catching this, because GEN12_RPSTAT1 is indeed available for all Gen12+. It was done this way because the voltage bits of GEN12_RPSTAT1 are only available for DG1/DG2. Anyway in v9 I have changed this to just: /* Available for all Gen12+/dGfx */ hwmon->rg.gt_perf_status = GEN12_RPSTAT1; That is because hwmon is only availbable for dGfx (there's a check in Patch 1). Also, because of this change the 'IS_DG1(i915) || IS_DG2(i915)' check has been moved to hwm_in_is_visible. Thanks. -- Ashutosh > > + hwmon->rg.gt_perf_status = GEN12_RPSTAT1; > > + else > > + hwmon->rg.gt_perf_status = INVALID_MMIO_REG; > > } > > > > void i915_hwmon_register(struct drm_i915_private *i915) > > -- > > 2.25.1