LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Gabriel C <nix.or.die@gmail.com>
Cc: Clemens Ladisch <clemens@ladisch.de>,
	Jean Delvare <jdelvare@suse.com>,
	linux-hwmon@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Brian Woods <brian.woods@amd.com>
Subject: Re: [PATCH 2/2] hwmon: (k10temp) Display both Tctl and Tdie
Date: Sun, 29 Apr 2018 09:27:13 -0700	[thread overview]
Message-ID: <c4ff12d1-b529-aa71-b9a7-760e2f7d9794@roeck-us.net> (raw)
In-Reply-To: <CAEJqkggcXnTWwNvidc=qG5fWFu8Q-6uGtrapi88e5t3u7wD12Q@mail.gmail.com>

On 04/29/2018 09:13 AM, Gabriel C wrote:
> 2018-04-27 5:13 GMT+02:00 Guenter Roeck <linux@roeck-us.net>:
>> On some AMD CPUs, there is a different between the die temperature
>> (Tdie) and the reported temperature (Tctl). Tdie is the real measured
>> temperature, and Tctl is used for fan control. Lets report both for
>> affected CPUs.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   drivers/hwmon/k10temp.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 48 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
>> index b06bb1f90853..1135b8f1ad0f 100644
>> --- a/drivers/hwmon/k10temp.c
>> +++ b/drivers/hwmon/k10temp.c
>> @@ -80,6 +80,7 @@ struct k10temp_data {
>>          void (*read_tempreg)(struct pci_dev *pdev, u32 *regval);
>>          int temp_offset;
>>          u32 temp_adjust_mask;
>> +       bool show_tdie;
>>   };
>>
>>   struct tctl_offset {
>> @@ -140,17 +141,24 @@ static void read_tempreg_nb_f17(struct pci_dev *pdev, u32 *regval)
>>                            F17H_M01H_REPORTED_TEMP_CTRL_OFFSET, regval);
>>   }
>>
>> -static ssize_t temp1_input_show(struct device *dev,
>> -                               struct device_attribute *attr, char *buf)
>> +unsigned int get_raw_temp(struct k10temp_data *data)
>>   {
>> -       struct k10temp_data *data = dev_get_drvdata(dev);
>> -       u32 regval;
>>          unsigned int temp;
>> +       u32 regval;
>>
>>          data->read_tempreg(data->pdev, &regval);
>>          temp = (regval >> 21) * 125;
>>          if (regval & data->temp_adjust_mask)
>>                  temp -= 49000;
>> +       return temp;
>> +}
>> +
>> +static ssize_t temp1_input_show(struct device *dev,
>> +                               struct device_attribute *attr, char *buf)
>> +{
>> +       struct k10temp_data *data = dev_get_drvdata(dev);
>> +       unsigned int temp = get_raw_temp(data);
>> +
>>          if (temp > data->temp_offset)
>>                  temp -= data->temp_offset;
>>          else
>> @@ -159,6 +167,23 @@ static ssize_t temp1_input_show(struct device *dev,
>>          return sprintf(buf, "%u\n", temp);
>>   }
>>
>> +static ssize_t temp2_input_show(struct device *dev,
>> +                               struct device_attribute *devattr, char *buf)
>> +{
>> +       struct k10temp_data *data = dev_get_drvdata(dev);
>> +       unsigned int temp = get_raw_temp(data);
>> +
>> +       return sprintf(buf, "%u\n", temp);
>> +}
>> +
>> +static ssize_t temp_label_show(struct device *dev,
>> +                              struct device_attribute *devattr, char *buf)
>> +{
>> +       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
>> +
>> +       return sprintf(buf, "%s\n", attr->index ? "Tctl" : "Tdie");
>> +}
>> +
>>   static ssize_t temp1_max_show(struct device *dev,
>>                                struct device_attribute *attr, char *buf)
>>   {
>> @@ -187,16 +212,23 @@ static DEVICE_ATTR_RO(temp1_max);
>>   static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp_crit, NULL, 0);
>>   static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO, show_temp_crit, NULL, 1);
>>
>> +static SENSOR_DEVICE_ATTR(temp1_label, 0444, temp_label_show, NULL, 0);
>> +static DEVICE_ATTR_RO(temp2_input);
>> +static SENSOR_DEVICE_ATTR(temp2_label, 0444, temp_label_show, NULL, 1);
>> +
>>   static umode_t k10temp_is_visible(struct kobject *kobj,
>>                                    struct attribute *attr, int index)
>>   {
>>          struct device *dev = container_of(kobj, struct device, kobj);
>>          struct k10temp_data *data = dev_get_drvdata(dev);
>>          struct pci_dev *pdev = data->pdev;
>> +       u32 reg;
>>
>> -       if (index >= 2) {
>> -               u32 reg;
>> -
>> +       switch (index) {
>> +       case 0 ... 1:   /* temp1_input, temp1_max */
>> +       default:
>> +               break;
>> +       case 2 ... 3:   /* temp1_crit, temp1_crit_hyst */
>>                  if (!data->read_htcreg)
>>                          return 0;
>>
>> @@ -208,6 +240,11 @@ static umode_t k10temp_is_visible(struct kobject *kobj,
>>                  data->read_htcreg(data->pdev, &reg);
>>                  if (!(reg & HTC_ENABLE))
>>                          return 0;
>> +               break;
>> +       case 4 ... 6:   /* temp1_label, temp2_input, temp2_label */
>> +               if (!data->show_tdie)
>> +                       return 0;
>> +               break;
>>          }
>>          return attr->mode;
>>   }
>> @@ -217,6 +254,9 @@ static struct attribute *k10temp_attrs[] = {
>>          &dev_attr_temp1_max.attr,
>>          &sensor_dev_attr_temp1_crit.dev_attr.attr,
>>          &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
>> +       &sensor_dev_attr_temp1_label.dev_attr.attr,
>> +       &dev_attr_temp2_input.attr,
>> +       &sensor_dev_attr_temp2_label.dev_attr.attr,
>>          NULL
>>   };
>>
>> @@ -292,6 +332,7 @@ static int k10temp_probe(struct pci_dev *pdev,
>>          } else if (boot_cpu_data.x86 == 0x17) {
>>                  data->temp_adjust_mask = 0x80000;
>>                  data->read_tempreg = read_tempreg_nb_f17;
>> +               data->show_tdie = true;
>>          } else {
>>                  data->read_htcreg = read_htcreg_pci;
>>                  data->read_tempreg = read_tempreg_pci;
>> --
>> 2.7.4
>>
> 
> On my EPYC box Tdie and Tctl seems to have the same value.
> However the code is working fine and both values are displayed now.
> 

Yes, that is as expected; EPYC CPUs don't have a temperature offset
between Tctl and Tdie (at least not as far as I know). Tctl is the value
reported by the chip; if I recall correctly, it is in the low 20s on
your system if it is idle. If there was an offset, Tdie would be reported
as an even lower temperature, ie in the 10s, which would not make sense.

> Tested-by: Gabriel Craciunescu <nix.or.die@gmail.com>
> 
Thanks a lot for testing!

Guenter

> Regards
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  reply	other threads:[~2018-04-29 16:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-27  3:13 [PATCH 1/2] hwmon: (k10temp) Fix reading critical temperature register Guenter Roeck
2018-04-27  3:13 ` [PATCH 2/2] hwmon: (k10temp) Display both Tctl and Tdie Guenter Roeck
2018-04-29 16:13   ` Gabriel C
2018-04-29 16:27     ` Guenter Roeck [this message]
2018-04-29 14:59 ` [1/2] hwmon: (k10temp) Fix reading critical temperature register 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=c4ff12d1-b529-aa71-b9a7-760e2f7d9794@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=brian.woods@amd.com \
    --cc=clemens@ladisch.de \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nix.or.die@gmail.com \
    --subject='Re: [PATCH 2/2] hwmon: (k10temp) Display both Tctl and Tdie' \
    /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

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).