On 2/15/2022 01:05, Paul Menzel wrote:
Dear Mario,
Am 14.02.22 um 17:07 schrieb Limonciello, Mario:
+Hans
(For the records, is part of Linux since 5.16-rc2 (commit
1527f69204fe).)
Am 12.11.21 um 21:15 schrieb Mario Limonciello:
AMD requires that the SATA controller be configured for devsleep in
order
for S0i3 entry to work properly.
commit b1a9585cc396 ("ata: ahci: Enable DEVSLP by default on x86 with
SLP_S0") sets up a kernel policy to enable devsleep on Intel mobile
platforms that are using s0ix. Add the PCI ID for the SATA
controller in
Green Sardine platforms to extend this policy by default for AMD based
systems using s0i3 as well.
Cc: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@xxxxxxx>
BugLink:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
illa.kernel.org%2Fshow_bug.cgi%3Fid%3D214091&data=04%7C01%7Cm
ario.limonciello%40amd.com%7Ca32a202d437544cd2cbb08d9ef9112c0%7C3d
d8961fe4884e608e11a82d994e183d%7C0%7C0%7C637804228648002522%7CU
nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=CbfImBnwc8uV1L5QRBuV
PLkP72wpS9yif1UbUwykhNI%3D&reserved=0
You have to love Microsoft Outlook.
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/ata/ahci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index d60f34718b5d..1e1167e725a4 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -438,6 +438,7 @@ static const struct pci_device_id ahci_pci_tbl[]
= {
/* AMD */
{ PCI_VDEVICE(AMD, 0x7800), board_ahci }, /* AMD Hudson-2 */
{ PCI_VDEVICE(AMD, 0x7900), board_ahci }, /* AMD CZ */
+ { PCI_VDEVICE(AMD, 0x7901), board_ahci_mobile }, /* AMD Green
Sardine */
Aren't 0x7900 and 0x7901 the same device only in different modes? I
wonder, why different boards and different comments are used.
No they aren't the same device in different modes.
Reference:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fen%2Fsupport%2Ftech-docs%2Fprocessor-programming-reference-ppr-for-amd-family-19h-model-51h-revision-a1&data=04%7C01%7CMario.Limonciello%40amd.com%7Cf25ec205dd2e46253a2208d9f0519b39%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637805055570772355%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=POXfSI626inWB7k73CqJF3IMp6y31r%2F%2BugdCXOkvydo%3D&reserved=0
Page 33 has a table.
That table misses 0x7900h. (Where can I find it?) coreboot has 0x7900
defined for Cezanne:
src/include/device/pci_ids.h:#define PCI_DEVICE_ID_AMD_CZ_SATA 0x7900
src/soc/amd/stoneyridge/include/soc/pci_devs.h: * SATA_IDE_IDEVID
0x7900
You can see that 0x7900 was introduced in the kernel back in 2013; well
in advance of the controller used in Cezanne.
So I really don't believe that CZ in that context for coreboot means
Cezanne. It's from an earlier product. I haven't referenced any older
documentation to confirm anything but if I were to venture a guess based
on those names you pasted it's probably introduced with Carrizo and
continues to be used up until Stoney Ridge.
The PCI database too [3]:
FCH SATA Controller [IDE mode]
Just FYI the strings in the PCI database is based on empirical user
submissions. You shouldn't rely upon it to tell you what generations
different devices are in.
For example I found recently on some family 19h laptops that they claim
to have a family 17h DMIC according to GNOME Settings which stems back
to a string that was in the PCI database.
When I encountered this I submitted a correction to make it more
generic, but I'm sure there are plenty more like this.
Additionally, the device 0x7901 is also present in desktop systems like
Dell OptiPlex 5055 and MSI B350 MORTAR. Is `board_ahci_mobile` the right
board then? Or should the flag `AHCI_HFLAG_IS_MOBILE` be renamed to
avoid confusion?
Are you having a problem or just want clarity in the enum definition?
It’s more about clarity, and understanding the two different entries.
This was introduced by Hans in commit ebb82e3c79d2a to set LPM policy
properly
for a number of mobile devices.
My opinion here is that the policy being for "mobile" devices only is
short sighted as power
consumption policy on desktops is also relevant as OEMs ship desktops
that need to meet
various power regulations for those too.
So I would be in the camp for renaming the flags.
Why can’t the LPM policy, if available(?), not be set for `board_ahci`
by default, so `board_ahci_mobile` could be removed?
/* AMD is using RAID class only for ahci controllers */
{ PCI_VENDOR_ID_AMD, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_STORAGE_RAID << 8, 0xffffff, board_ahci },
Kind regards,
Paul
[1]:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.coreboot.org%2F10418&data=04%7C01%7CMario.Limonciello%40amd.com%7Cf25ec205dd2e46253a2208d9f0519b39%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637805055570772355%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=vqX1NjUKE0STzvmKSWtJxhrrdQnH%2BmqAiXGuqNMQAt0%3D&reserved=0
[2]:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freview.coreboot.org%2F20200&data=04%7C01%7CMario.Limonciello%40amd.com%7Cf25ec205dd2e46253a2208d9f0519b39%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637805055570772355%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=oz3K76NGXrp41wq%2B5QcPUtJG%2BfqxCGOTVojfYb%2BMIlw%3D&reserved=0
[3]:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpci-ids.ucw.cz%2Fread%2FPC%2F1022%2F7900&data=04%7C01%7CMario.Limonciello%40amd.com%7Cf25ec205dd2e46253a2208d9f0519b39%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637805055570772355%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Y32YS5rIxBXTyNQ8VOuTxBTbfsOipBcIPAvmhBhSz7E%3D&reserved=0
