Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Gabriel C <nix.or.die@googlemail.com>
Cc: linux-hwmon@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Wei Huang <wei.huang2@amd.com>
Subject: Re: k10temp: ZEN3 readings are broken
Date: Tue, 22 Dec 2020 07:51:13 -0800	[thread overview]
Message-ID: <5d8a2b4b-dfa0-d8c3-e827-0dc9971caa0b@roeck-us.net> (raw)
In-Reply-To: <CAEJqkggMJMKS5E2n26nvoCfq1tnokQG+WQi+WH4J7gNNz+0o5A@mail.gmail.com>

On 12/22/20 7:26 AM, Gabriel C wrote:
> Am Di., 22. Dez. 2020 um 07:16 Uhr schrieb Guenter Roeck <linux@roeck-us.net>:
>>
>> On Tue, Dec 22, 2020 at 05:33:17AM +0100, Gabriel C wrote:
>> [ ... ]
>>> At least is what the weird amd_energy driver added and since is only supporting
>>> fam 17h model 0x31 which is TR 3000 & SP3 Rome, I guess fam 19h 0x1 is
>>> TR/SP3 ZEN3.
>>
>> The limited model support is because people nowadays are not willing to
>> accept that reported values may not always be perfect ... and the reported
>> energy for non-server parts is known to be not always perfect. Kind of an
>> odd situation: If we support non-server parts, we have people complain
>> that values are not perfect. If we only support server parts, we have
>> people complain that only server parts are supported. For us, that is
>> a lose-lose situation. I used to think that is is better to report
>> _something_, but the (sometimes loud) complaints about lack of perfection
>> teached me a lesson. So now my reaction is to drop support if I get
>> complaints about lack of perfection.
>>
> 
> I agree it is an odd situation with these modules, but having
> something is better than nothing.

That is your opinion, and it used to be mine as well. As I said, I have
learned from the feedback.

> As for the amd_energy driver, yes it is off on some platforms by 2%-5%
> or alike but without having
> that support in the kernel, regardless of the module, we cannot ever
> come to perfection or near it.
> 
> For both k10temp & amd_energy driver I suggest to not drop the support
>  but add kernel modules
> options disabled by default, much like a lot laptop platform drivers
> have for different reasons.
> 

That would just add complexity for little gain. The code would still have
to be maintained, and as experience (and the out-of-tree driver) has shown
this is a never ending story. Plus, it would still be inaccurate, leading
to complaints, module parameter or not.

> The amd_energy driver could have some any_ryzen option which turned
> off by default.
> That way people may decide if they want to use it even when not 100%
> perfect and can report
> back on platforms the reporting is accurate.
> Waiting for AMD to give us ID of what may be in their eyes accurate is
> like waiting for pigs to fly.
> 
> The k10temp module much like the same, some experimental_voltage_report module
> option will be fine for now, I think.
> 
> I'm also sure owner of AMD HW will help out optimizing and maintaining the code.
> 

Not really. My experience is that almost everyone will just complain.
It was a bad idea to add voltage/current reporting to the k10temp driver,
and it is time to revert it. If someone else wants to write (and maintain)
a separate amd_voltage or similar driver, I am all open to accept it.

Note that even you suggested to _drop_ the amd energy driver instead of
fixing it. I'll take that as a qed.

Guenter

  reply	other threads:[~2020-12-22 15:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-22  1:45 k10temp: ZEN3 readings are broken Gabriel C
2020-12-22  3:58 ` Guenter Roeck
2020-12-22  4:33   ` Wei Huang
2020-12-22  5:09     ` Gabriel C
2020-12-22  6:08       ` Wei Huang
2020-12-22  4:33   ` Gabriel C
2020-12-22  6:07     ` Guenter Roeck
2020-12-22  6:16     ` Guenter Roeck
2020-12-22 15:26       ` Gabriel C
2020-12-22 15:51         ` Guenter Roeck [this message]
2020-12-23 10:41   ` Jan Engelhardt
2020-12-23 11:22     ` Gabriel C
2020-12-23 11:27     ` Gabriel C
2020-12-23 14:25     ` 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=5d8a2b4b-dfa0-d8c3-e827-0dc9971caa0b@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nix.or.die@googlemail.com \
    --cc=wei.huang2@amd.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