From: "Nilawar, Badal" <badal.nilawar@intel.com>
To: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
Cc: <intel-gfx@lists.freedesktop.org>, <riana.tauro@intel.com>,
<anshuman.gupta@intel.com>, <jon.ewins@intel.com>,
<linux-hwmon@vger.kernel.org>
Subject: Re: [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support
Date: Thu, 15 Sep 2022 20:10:37 +0530 [thread overview]
Message-ID: <9760a03d-a83e-7e27-e4da-faff02c6f11a@intel.com> (raw)
In-Reply-To: <87o7w3yn28.wl-ashutosh.dixit@intel.com>
On 29-08-2022 23:00, Dixit, Ashutosh wrote:
> On Thu, 25 Aug 2022 06:21:13 -0700, Badal Nilawar wrote:
>>
>> From: Riana Tauro <riana.tauro@intel.com>
>>
>> Use i915 HWMON subsystem to display current input voltage.
>
> A couple of suggestions to improve comments in this patch below and after
> addressing those this patch is:
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
>> index 103dd543a214..2192d0fd4c66 100644
>> --- a/drivers/gpu/drm/i915/i915_hwmon.c
>> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
>> @@ -11,8 +11,10 @@
>> #include "i915_hwmon.h"
>> #include "i915_reg.h"
>> #include "intel_mchbar_regs.h"
>> +#include "gt/intel_gt_regs.h"
>
> In later patches we have added units for different quantities here. So I
> think we should add those units for voltage to this patch too. It's in
> Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon but I think it's
> better to add to this file too otherwise if anyone looks at it is seems to
> be missing.
>
> So I would add the following to this patch:
>
> /*
> * SF_* - scale factors for particular quantities according to hwmon spec.
> * - voltage - millivolts
> */
Sure I will add above comment.
> #define SF_VOLTAGE 1000
we are not using above scale factor for voltage. As our scale factor is
2.5 millivolts shall I add like.
#define SF_VOLTAGE_MUL 25
#define SF_VOLTAGE_DIV 10
>
>> +static int
>> +hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
>> +{
>> + struct i915_hwmon *hwmon = ddat->hwmon;
>> + intel_wakeref_t wakeref;
>> + u32 reg_value;
>> +
>> + switch (attr) {
>> + case hwmon_in_input:
>> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
>> + reg_value = intel_uncore_read(ddat->uncore, hwmon->rg.gt_perf_status);
>> + /* In units of 2.5 millivolt */
>> + *val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) * 25, 10);
And use above scale factors here.
*val = DIV_ROUND_CLOSEST(REG_FIELD_GET(GEN12_VOLTAGE_MASK, reg_value) *
SF_VOLTAGE_MUL, SF_VOLTAGE_DIV);
Regards,
Badal
>
> Let's complete this comment to so that it is clear what's happening:
>
> /* HW register value is in units of 2.5 millivolt */
next prev parent reply other threads:[~2022-09-15 14:42 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-25 13:21 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-08-25 13:21 ` [PATCH 1/7] drm/i915/hwmon: Add HWMON infrastructure Badal Nilawar
2022-08-26 13:30 ` Guenter Roeck
2022-08-29 17:26 ` Dixit, Ashutosh
2022-08-25 13:21 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-08-29 17:30 ` Dixit, Ashutosh
2022-09-15 14:40 ` Nilawar, Badal [this message]
2022-09-21 0:02 ` Dixit, Ashutosh
2022-09-12 14:09 ` Gupta, Anshuman
2022-09-12 16:37 ` Dixit, Ashutosh
2022-09-13 8:11 ` Gupta, Anshuman
[not found] ` <878rmn5mks.wl-ashutosh.dixit@intel.com>
2022-09-15 6:29 ` Gupta, Anshuman
2022-08-25 13:21 ` [PATCH 3/7] drm/i915/hwmon: Power PL1 limit and TDP setting Badal Nilawar
2022-08-30 2:33 ` Dixit, Ashutosh
2022-08-25 13:21 ` [PATCH 4/7] drm/i915/hwmon: Show device level energy usage Badal Nilawar
2022-08-30 3:14 ` Dixit, Ashutosh
2022-09-13 8:50 ` [Intel-gfx] " Tvrtko Ursulin
2022-09-21 0:24 ` Dixit, Ashutosh
2022-09-21 7:43 ` Tvrtko Ursulin
2022-08-25 13:21 ` [PATCH 5/7] drm/i915/hwmon: Expose card reactive critical power Badal Nilawar
2022-08-25 13:21 ` [PATCH 6/7] drm/i915/hwmon: Expose power1_max_interval Badal Nilawar
2022-08-25 13:21 ` [PATCH 7/7] drm/i915/hwmon: Extend power/energy for XEHPSDV Badal Nilawar
2022-08-30 5:34 ` Dixit, Ashutosh
-- strict thread matches above, loose matches on Subject: below --
2022-10-13 15:45 [PATCH 0/7] drm/i915: Add HWMON support Ashutosh Dixit
2022-10-13 15:45 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Ashutosh Dixit
2022-09-27 5:50 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-09-27 5:50 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-09-26 17:52 [PATCH 0/7] Add HWMON support Badal Nilawar
2022-09-26 17:52 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-09-23 19:56 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-09-23 19:56 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-09-16 15:00 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-09-16 15:00 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-09-21 11:08 ` Gupta, Anshuman
2022-08-18 19:38 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-08-18 19:38 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-08-12 17:37 [PATCH 0/7] drm/i915: Add HWMON support Badal Nilawar
2022-08-12 17:37 ` [PATCH 2/7] drm/i915/hwmon: Add HWMON current voltage support Badal Nilawar
2022-08-12 18:05 ` Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9760a03d-a83e-7e27-e4da-faff02c6f11a@intel.com \
--to=badal.nilawar@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=ashutosh.dixit@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jon.ewins@intel.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=riana.tauro@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).