diff mbox series

hwmon: (k10temp) Add AMD family 17h model 60h probe

Message ID 20200617013255.391975-1-hengqing.hu@gmail.com (mailing list archive)
State New, archived
Headers show
Series hwmon: (k10temp) Add AMD family 17h model 60h probe | expand

Commit Message

Jacky Hu June 17, 2020, 1:32 a.m. UTC
With this patch applied, output from 4800H (idle) looks as follows:

k10temp-pci-00c3
Adapter: PCI adapter
Vcore:         1.55 V
Vsoc:          1.55 V
Tctl:         +49.6°C
Tdie:         +49.6°C
Icore:         0.00 A
Isoc:          0.00 A

Signed-off-by: Jacky Hu <hengqing.hu@gmail.com>
---
 drivers/hwmon/k10temp.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Guenter Roeck June 17, 2020, 3:40 a.m. UTC | #1
On Wed, Jun 17, 2020 at 09:32:55AM +0800, Jacky Hu wrote:
> With this patch applied, output from 4800H (idle) looks as follows:
> 
> k10temp-pci-00c3
> Adapter: PCI adapter
> Vcore:         1.55 V
> Vsoc:          1.55 V
> Tctl:         +49.6°C
> Tdie:         +49.6°C
> Icore:         0.00 A
> Isoc:          0.00 A
> 
> Signed-off-by: Jacky Hu <hengqing.hu@gmail.com>
> ---
>  drivers/hwmon/k10temp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index 8f12995ec133..287e9cf2aab9 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -583,6 +583,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  			k10temp_get_ccd_support(pdev, data, 4);
>  			break;
>  		case 0x31:	/* Zen2 Threadripper */
> +		case 0x60:	/* Zen2 APU */

Unfortunately it is not that simple. Output above and the little data I have
available suggests that current and voltage measurements are different on the
APU. That means that show_current must remain false.
This will require a separate case statement which doesn't set any flags.

Guenter

>  		case 0x71:	/* Zen2 */
>  			data->show_current = !is_threadripper() && !is_epyc();
>  			data->cfactor[0] = CFACTOR_ICORE;
> -- 
> 2.27.0
>
Jacky Hu June 17, 2020, 7:19 a.m. UTC | #2
Hi Guenter,

By increasing the regs count from 32 to 256 and looking into the output of `cat /sys/kernel/debug/k10temp-0000\:00\:18.3/svi`
There is some data from 0x05a300 - 0x05a330
Do you have any idea how we can guess the offset for this model?

0x05a000: 00000000 00000000 00000000 00000000
0x05a010: 00000000 00000000 00000000 00000000
0x05a020: 00000000 00000000 00000000 00000000
0x05a030: 00000000 00000000 00000000 00000000
0x05a040: 00000000 00000000 00000000 00000000
0x05a050: 00000000 00000000 00000000 00000000
0x05a060: 00000000 00000000 00000000 00000000
0x05a070: 00000000 00000000 00000000 00000000
0x05a080: 00000000 00000000 00000000 00000000
0x05a090: 00000000 00000000 00000000 00000000
0x05a0a0: 00000000 00000000 00000000 00000000
0x05a0b0: 00000000 00000000 00000000 00000000
0x05a0c0: 00000000 00000000 00000000 00000000
0x05a0d0: 00000000 00000000 00000000 00000000
0x05a0e0: 00000000 00000000 00000000 00000000
0x05a0f0: 00000000 00000000 00000000 00000000
0x05a100: 00000000 00000000 00000000 00000000
0x05a110: 00000000 00000000 00000000 00000000
0x05a120: 00000000 00000000 00000000 00000000
0x05a130: 00000000 00000000 00000000 00000000
0x05a140: 00000000 00000000 00000000 00000000
0x05a150: 00000000 00000000 00000000 00000000
0x05a160: 00000000 00000000 00000000 00000000
0x05a170: 00000000 00000000 00000000 00000000
0x05a180: 00000000 00000000 00000000 00000000
0x05a190: 00000000 00000000 00000000 00000000
0x05a1a0: 00000000 00000000 00000000 00000000
0x05a1b0: 00000000 00000000 00000000 00000000
0x05a1c0: 00000000 00000000 00000000 00000000
0x05a1d0: 00000000 00000000 00000000 00000000
0x05a1e0: 00000000 00000000 00000000 00000000
0x05a1f0: 00000000 00000000 00000000 00000000
0x05a200: 00000000 00000000 00000000 00000000
0x05a210: 00000000 00000000 00000000 00000000
0x05a220: 00000000 00000000 00000000 00000000
0x05a230: 00000000 00000000 00000000 00000000
0x05a240: 00000000 00000000 00000000 00000000
0x05a250: 00000000 00000000 00000000 00000000
0x05a260: 00000000 00000000 00000000 00000000
0x05a270: 00000000 00000000 00000000 00000000
0x05a280: 00000000 00000000 00000000 00000000
0x05a290: 00000000 00000000 00000000 00000000
0x05a2a0: 00000000 00000000 00000000 00000000
0x05a2b0: 00000000 00000000 00000000 00000000
0x05a2c0: 00000000 00000000 00000000 00000000
0x05a2d0: 00000000 00000000 00000000 00000000
0x05a2e0: 00000000 00000000 00000000 00000000
0x05a2f0: 00000000 00000000 00000000 00000000
0x05a300: 00000000 00000001 00000000 00002710
0x05a310: 00000000 00000008 0000000e 00000000
0x05a320: 00000001 0000c000 00000000 0000000b
0x05a330: 00000001 00000000 00000000 00000000
0x05a340: 00000000 00000000 00000000 00000000
0x05a350: 00000000 00000000 00000000 00000000
0x05a360: 00000000 00000000 00000000 00000000
0x05a370: 00000000 00000000 00000000 00000000
0x05a380: 00000000 00000000 00000000 00000000
0x05a390: 00000000 00000000 00000000 00000000
0x05a3a0: 00000000 00000000 00000000 00000000
0x05a3b0: 00000000 00000000 00000000 00000000
0x05a3c0: 00000000 00000000 00000000 00000000
0x05a3d0: 00000000 00000000 00000000 00000000
0x05a3e0: 00000000 00000000 00000000 00000000
0x05a3f0: 00000000 00000000 00000000 00000000 

Thanks.
Jacky
On Tue, Jun 16, 2020 at 08:40:28PM -0700, Guenter Roeck wrote:
> On Wed, Jun 17, 2020 at 09:32:55AM +0800, Jacky Hu wrote:
> > With this patch applied, output from 4800H (idle) looks as follows:
> > 
> > k10temp-pci-00c3
> > Adapter: PCI adapter
> > Vcore:         1.55 V
> > Vsoc:          1.55 V
> > Tctl:         +49.6°C
> > Tdie:         +49.6°C
> > Icore:         0.00 A
> > Isoc:          0.00 A
> > 
> > Signed-off-by: Jacky Hu <hengqing.hu@gmail.com>
> > ---
> >  drivers/hwmon/k10temp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> > index 8f12995ec133..287e9cf2aab9 100644
> > --- a/drivers/hwmon/k10temp.c
> > +++ b/drivers/hwmon/k10temp.c
> > @@ -583,6 +583,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >  			k10temp_get_ccd_support(pdev, data, 4);
> >  			break;
> >  		case 0x31:	/* Zen2 Threadripper */
> > +		case 0x60:	/* Zen2 APU */
> 
> Unfortunately it is not that simple. Output above and the little data I have
> available suggests that current and voltage measurements are different on the
> APU. That means that show_current must remain false.
> This will require a separate case statement which doesn't set any flags.
> 
> Guenter
> 
> >  		case 0x71:	/* Zen2 */
> >  			data->show_current = !is_threadripper() && !is_epyc();
> >  			data->cfactor[0] = CFACTOR_ICORE;
> > -- 
> > 2.27.0
> >
Guenter Roeck June 17, 2020, 2:33 p.m. UTC | #3
On Wed, Jun 17, 2020 at 03:19:27PM +0800, Jacky Hu wrote:
> Hi Guenter,
> 
> By increasing the regs count from 32 to 256 and looking into the output of `cat /sys/kernel/debug/k10temp-0000\:00\:18.3/svi`
> There is some data from 0x05a300 - 0x05a330
> Do you have any idea how we can guess the offset for this model?
> 

For other chips, the upper 16 bits of the register reported the voltage
and the lower 16 bit reported the current. It might possibly be that the
data is now split into multiple registers, but that is impossible to
determine without datasheet and/or additional information. So, sorry,
no, I have no idea. We'll have to wait for someone to reverse engineer
it.

Can you send the contents of the "thm" file ? Maybe we can at least
find the new location of the ccd temperature registers.

Guenter
Alexander Monakov June 17, 2020, 2:54 p.m. UTC | #4
Hi,

I've already said in my patch submission (which was Cc'ed to LKML,
linux-edac and linux-hwmon so you should have been able to find it):

>> It appears SMU offsets for reading current/voltage and CCD temperature
>> have changed for this generation (reads from currently used offsets
>> yield zeros), so those features cannot be enabled so trivially.

In https://github.com/FlyGoat/ryzen_nb_smu/issues/3 there some
reverse-engineered info that indicates that for Renoir, SMU region has been
moved to 0x6F000. Its layout also changed, as far as I can tell.

(also, please ask yourself if you want to offer the maintainers an apology)

Alexander
Jacky Hu June 17, 2020, 2:55 p.m. UTC | #5
Yep, here it is

~> cat thm.idle
0x059800: 29a00fef 017f1201 00012921 000f4240
0x059810: 800000f9 00000000 00000000 00000000
0x059820: 00000000 00000000 00000000 0fff0078
0x059830: 00000000 002cecf9 002cecf9 002d2cfb
0x059840: 002cccf8 002cacf7 002d2cfb 002cccf8
0x059850: 002d2cfb 002c8cf6 002cccf8 002cacf7
0x059860: 002cacf7 002cecf9 002c8cf6 002d0cfa
0x059870: 002d2cfb 002d0cfa 002d0cfa 002cacf7
0x059880: 002cecf9 002d0cfa 002cecf9 002cacf7
0x059890: 002cccf8 002c6cf5 002d2cfb 002c8cf6
0x0598a0: 002c8cf6 002cccf8 002cacf7 002cccf8
0x0598b0: 002c8cf6 00000000 00002100 ffffffff
0x0598c0: 00000000 00000000 00000000 00000000
0x0598d0: 00000000 00000000 00000000 00000000
0x0598e0: 00000000 00000000 00000000 00000000
0x0598f0: 00000000 00000000 00000000 00000000
0x059900: 00000000 00000000 00000000 00000000
0x059910: 00000000 00000000 00000000 00000000
0x059920: 00000000 00000000 00000000 00000000
0x059930: 00000000 00000000 00000000 00000000
0x059940: 00000000 00000000 00000000 00000000
0x059950: 00000000 00000000 00000000 00000000
0x059960: 00000000 00000000 00000000 00000000
0x059970: 00000000 00000000 08400001 00005229
0x059980: 00000053 a7800005 3118680e 00000000
0x059990: 00000000 00000000 00000000 00000000
0x0599a0: 00000000 00000000 00000000 00000000
0x0599b0: 00000000 00000000 00000000 00000000
0x0599c0: 00000000 00000000 00000000 00000000
0x0599d0: 00000000 00003060 000002d4 00000019
0x0599e0: 000002c6 00000008 00000000 00000000
0x0599f0: 000002d4 00000019 00000000 00000000
0x059a00: 00000001 0001000d 00000000 00000000
0x059a10: 00000000 00000000 00000000 00000000
0x059a20: 00000000 00000000 00000000 00000000
0x059a30: 00000000 00000000 00000000 00000000
0x059a40: 00000000 00000000 00000000 00000000
0x059a50: 00000000 00000000 00000000 00000000
0x059a60: 00000000 00000000 00000000 00000000
0x059a70: 00000000 00000000 00000000 00000000
0x059a80: 00000000 00000000 00000000 00000000
0x059a90: 00000000 00000000 00000000 00000000
0x059aa0: 00000000 00000000 00000000 00000000
0x059ab0: 00000000 00000000 00000000 00000000
0x059ac0: 00000000 00000000 00000000 00000000
0x059ad0: 00000000 00000000 00000000 00000000
0x059ae0: 00000000 00000000 00000000 00000000
0x059af0: 00000000 00000000 00000000 00000000
0x059b00: 00000000 00000000 00000000 00000000
0x059b10: 0000000e 00000000 00000003 00000000
0x059b20: 901f001a 00050003 00000000 00000000
0x059b30: 00480001 00000000 00000000 00000000
0x059b40: 00000000 00000000 00000010 0000ffff
0x059b50: 00000000 00000000 00000000 00000000
0x059b60: 00000000 00000000 00000000 00000000
0x059b70: 00000000 00000000 00000000 00130082
0x059b80: 0000067f 12110201 0003045a 00001303
0x059b90: 00000000 028a4f5c 08036927 0021e548
0x059ba0: 00000000 7fffffff 00000000 00000043
0x059bb0: c00001c0 000000f9 00000000 00000000
0x059bc0: 00000000 00000000 00000000 00000000
0x059bd0: 00000000 00000000 00000000 00000000
0x059be0: 00000000 00000000 00000000 00000000
0x059bf0: 00000000 00000000 00000000 00000000

~> cat thm.busy
0x059800: 57800fef 017f1201 00012921 000f4240
0x059810: 800000f9 00000000 00000000 00000000
0x059820: 00000000 00000000 00000000 0fff0078
0x059830: 00000000 00372d4d 0042edad 003f6d90
0x059840: 00416da1 003e0d85 00392d5d 003bad72
0x059850: 00394d5e 003acd6b 00382d55 00348d37
0x059860: 0033ad30 0033cd31 0033cd31 00336d2e
0x059870: 00338d2f 0043edb5 00402d97 00440db6
0x059880: 0040ed9d 0039ed63 003c4d77 0039ed63
0x059890: 003c2d76 003a0d65 00338d2f 00334d2d
0x0598a0: 0033ad30 00342d34 00344d35 0037cd52
0x0598b0: 0035ed43 00000000 00002100 ffffffff
0x0598c0: 00000000 00000000 00000000 00000000
0x0598d0: 00000000 00000000 00000000 00000000
0x0598e0: 00000000 00000000 00000000 00000000
0x0598f0: 00000000 00000000 00000000 00000000
0x059900: 00000000 00000000 00000000 00000000
0x059910: 00000000 00000000 00000000 00000000
0x059920: 00000000 00000000 00000000 00000000
0x059930: 00000000 00000000 00000000 00000000
0x059940: 00000000 00000000 00000000 00000000
0x059950: 00000000 00000000 00000000 00000000
0x059960: 00000000 00000000 00000000 00000000
0x059970: 00000000 00000000 08400001 0000ae57
0x059980: 0000005b a7800005 3118680e 00000000
0x059990: 00000000 00000000 00000000 00000000
0x0599a0: 00000000 00000000 00000000 00000000
0x0599b0: 00000000 00000000 00000000 00000000
0x0599c0: 00000000 00000000 00000000 00000000
0x0599d0: 00000000 00003060 00000440 00000012
0x0599e0: 00000334 0000001a 00000000 00000000
0x0599f0: 00000440 00000012 00000000 00000000
0x059a00: 00000001 0001000d 00000000 00000000
0x059a10: 00000000 00000000 00000000 00000000
0x059a20: 00000000 00000000 00000000 00000000
0x059a30: 00000000 00000000 00000000 00000000
0x059a40: 00000000 00000000 00000000 00000000
0x059a50: 00000000 00000000 00000000 00000000
0x059a60: 00000000 00000000 00000000 00000000
0x059a70: 00000000 00000000 00000000 00000000
0x059a80: 00000000 00000000 00000000 00000000
0x059a90: 00000000 00000000 00000000 00000000
0x059aa0: 00000000 00000000 00000000 00000000
0x059ab0: 00000000 00000000 00000000 00000000
0x059ac0: 00000000 00000000 00000000 00000000
0x059ad0: 00000000 00000000 00000000 00000000
0x059ae0: 00000000 00000000 00000000 00000000
0x059af0: 00000000 00000000 00000000 00000000
0x059b00: 00000000 00000000 00000000 00000000
0x059b10: 0000000e 00000000 00000003 00000000
0x059b20: 901f001a 00050003 00000000 00000000
0x059b30: 00480001 00000000 00000000 00000000
0x059b40: 00000000 00000000 00000010 0000ffff
0x059b50: 00000000 00000000 00000000 00000000
0x059b60: 00000000 00000000 00000000 00000000
0x059b70: 00000000 00000000 00000000 00130082
0x059b80: 0000067f 12110201 0003045a 00001303
0x059b90: 00000000 028a4f5c 08036927 0021e548
0x059ba0: 00000000 7fffffff 00000000 00000043
0x059bb0: c00001c0 000000f9 00000000 00000000
0x059bc0: 00000000 00000000 00000000 00000000
0x059bd0: 00000000 00000000 00000000 00000000
0x059be0: 00000000 00000000 00000000 00000000
0x059bf0: 00000000 00000000 00000000 00000000

Jacky
On Wed, Jun 17, 2020 at 07:33:42AM -0700, Guenter Roeck wrote:
> On Wed, Jun 17, 2020 at 03:19:27PM +0800, Jacky Hu wrote:
> > Hi Guenter,
> > 
> > By increasing the regs count from 32 to 256 and looking into the output of `cat /sys/kernel/debug/k10temp-0000\:00\:18.3/svi`
> > There is some data from 0x05a300 - 0x05a330
> > Do you have any idea how we can guess the offset for this model?
> > 
> 
> For other chips, the upper 16 bits of the register reported the voltage
> and the lower 16 bit reported the current. It might possibly be that the
> data is now split into multiple registers, but that is impossible to
> determine without datasheet and/or additional information. So, sorry,
> no, I have no idea. We'll have to wait for someone to reverse engineer
> it.
> 
> Can you send the contents of the "thm" file ? Maybe we can at least
> find the new location of the ccd temperature registers.
> 
> Guenter
Jacky Hu June 17, 2020, 3:07 p.m. UTC | #6
Hi,

Sorry, I apologize for didn't do much lookup that you already did the patch
submission before I submitted the patch.
I have to say we are all programmed by the programs.
Also I didn't submit to either of the lists.
A few places I did looked at are below before I did the submission.
https://pci-ids.ucw.cz/v2.2/pci.ids
https://lore.kernel.org/patchwork/project/lkml/list/

Jacky
On Wed, Jun 17, 2020 at 05:54:36PM +0300, Alexander Monakov wrote:
> Hi,
> 
> I've already said in my patch submission (which was Cc'ed to LKML,
> linux-edac and linux-hwmon so you should have been able to find it):
> 
> >> It appears SMU offsets for reading current/voltage and CCD temperature
> >> have changed for this generation (reads from currently used offsets
> >> yield zeros), so those features cannot be enabled so trivially.
> 
> In https://github.com/FlyGoat/ryzen_nb_smu/issues/3 there some
> reverse-engineered info that indicates that for Renoir, SMU region has been
> moved to 0x6F000. Its layout also changed, as far as I can tell.
> 
> (also, please ask yourself if you want to offer the maintainers an apology)
> 
> Alexander
Borislav Petkov June 17, 2020, 3:22 p.m. UTC | #7
Ok, both of you:

On Wed, Jun 17, 2020 at 11:07:35PM +0800, Jacky Hu wrote:
> Hi,
> 
> Sorry, I apologize for didn't do much lookup that you already did the patch
> submission before I submitted the patch.
> I have to say we are all programmed by the programs.
> Also I didn't submit to either of the lists.
> A few places I did looked at are below before I did the submission.
> https://pci-ids.ucw.cz/v2.2/pci.ids
> https://lore.kernel.org/patchwork/project/lkml/list/

Jacky, please do not top-post. Please adhere to the etiquette on public
mailing lists.

Alexander, things like that can happen and they pretty much do happen
everytime new hw comes out. Kernel development has exploded so much in
recent years so that it is absolutely normal to miss stuff. Hell, *we*
miss stuff too, from time to time.

So let's concentrate on the work pls.

Thank you both!
diff mbox series

Patch

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 8f12995ec133..287e9cf2aab9 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -583,6 +583,7 @@  static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 			k10temp_get_ccd_support(pdev, data, 4);
 			break;
 		case 0x31:	/* Zen2 Threadripper */
+		case 0x60:	/* Zen2 APU */
 		case 0x71:	/* Zen2 */
 			data->show_current = !is_threadripper() && !is_epyc();
 			data->cfactor[0] = CFACTOR_ICORE;