diff mbox series

thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

Message ID 1676035943-115840-1-git-send-email-Sanju.Mehta@amd.com (mailing list archive)
State Superseded
Headers show
Series thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine | expand

Commit Message

Mehta, Sanju Feb. 10, 2023, 1:32 p.m. UTC 
From: Sanjay R Mehta <sanju.mehta@amd.com>

AMD Yellow Carp and Pink Sardine don't support CLx state,
hence disabling it.

Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/thunderbolt/nhi.h    |  6 ++++++
 drivers/thunderbolt/switch.c |  6 ++++++
 drivers/thunderbolt/tb.h     | 24 ++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

Comments

Mika Westerberg Feb. 10, 2023, 1:55 p.m. UTC | #1 
Hi,

On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> From: Sanjay R Mehta <sanju.mehta@amd.com>
> 
> AMD Yellow Carp and Pink Sardine don't support CLx state,
> hence disabling it.

The lane adapters are supposed to announce whether CL-states are
supported or not. Is that not the case with the AMD hardware?
Sanjay R Mehta Feb. 10, 2023, 2:02 p.m. UTC | #2 
On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> Hi,
> 
> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
>> From: Sanjay R Mehta <sanju.mehta@amd.com>
>>
>> AMD Yellow Carp and Pink Sardine don't support CLx state,
>> hence disabling it.
> 
> The lane adapters are supposed to announce whether CL-states are
> supported or not. Is that not the case with the AMD hardware?

Yes Mika. it doesn't work for AMD hardware.

- Sanjay
Mika Westerberg Feb. 10, 2023, 2:17 p.m. UTC | #3 
On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
> 
> 
> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> > Hi,
> > 
> > On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> >> From: Sanjay R Mehta <sanju.mehta@amd.com>
> >>
> >> AMD Yellow Carp and Pink Sardine don't support CLx state,
> >> hence disabling it.
> > 
> > The lane adapters are supposed to announce whether CL-states are
> > supported or not. Is that not the case with the AMD hardware?
> 
> Yes Mika. it doesn't work for AMD hardware.

:-(

Okay can you then add a quirk for this to quirks.c?
Sanjay R Mehta Feb. 10, 2023, 2:46 p.m. UTC | #4 
On 2/10/2023 7:47 PM, Mika Westerberg wrote:
> On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
>>
>>
>> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
>>> Hi,
>>>
>>> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
>>>> From: Sanjay R Mehta <sanju.mehta@amd.com>
>>>>
>>>> AMD Yellow Carp and Pink Sardine don't support CLx state,
>>>> hence disabling it.
>>>
>>> The lane adapters are supposed to announce whether CL-states are
>>> supported or not. Is that not the case with the AMD hardware?
>>
>> Yes Mika. it doesn't work for AMD hardware.
> 
> :-(
> 
> Okay can you then add a quirk for this to quirks.c?

Did you meant like below, is this fine?

diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index b071802..138c649 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -49,6 +49,7 @@ struct tb_nhi_ops {
 };

 extern const struct tb_nhi_ops icl_nhi_ops;
+extern bool clx_enabled;

......
......

diff --git a/drivers/thunderbolt/quirks.c b/drivers/thunderbolt/quirks.c
index b5f2ec7..9ceef7c 100644
--- a/drivers/thunderbolt/quirks.c
+++ b/drivers/thunderbolt/quirks.c
@@ -63,4 +63,10 @@ void tb_check_quirks(struct tb_switch *sw)

                q->hook(sw);
        }
+
+       /*
+        * CLx is not supported on AMD USB4 Yellow Carp and Pink Sardine
platforms.
+        */
+       if (tb_switch_is_yellow_carp(sw->tb->nhi) ||
tb_switch_is_pink_sardine(sw->tb->nhi))
+               clx_enabled = false;
 }
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 363d712..d4492b5 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -26,7 +26,7 @@ struct nvm_auth_status {
        u32 status;
 };

-static bool clx_enabled = true;
+bool clx_enabled = true;
 module_param_named(clx, clx_enabled, bool, 0444);
 MODULE_PARM_DESC(clx, "allow low power states on the high-speed lanes
(default: true)");

........
........
Mika Westerberg Feb. 10, 2023, 2:56 p.m. UTC | #5 
On Fri, Feb 10, 2023 at 08:16:48PM +0530, Sanjay R Mehta wrote:
> 
> 
> On 2/10/2023 7:47 PM, Mika Westerberg wrote:
> > On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
> >>
> >>
> >> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> >>> Hi,
> >>>
> >>> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> >>>> From: Sanjay R Mehta <sanju.mehta@amd.com>
> >>>>
> >>>> AMD Yellow Carp and Pink Sardine don't support CLx state,
> >>>> hence disabling it.
> >>>
> >>> The lane adapters are supposed to announce whether CL-states are
> >>> supported or not. Is that not the case with the AMD hardware?
> >>
> >> Yes Mika. it doesn't work for AMD hardware.
> > 
> > :-(
> > 
> > Okay can you then add a quirk for this to quirks.c?
> 
> Did you meant like below, is this fine?

I mean add the affected routers to quirks.c that then sets:

	sw->quirks |= QUIRK_NO_CLX;

and check this in tb_switch_clx_is_supported(), if set return false or
something like that.

I know we have bunch of Intel specific quirks littered around the
driver but let's not add more. We should move these too into quirks.c at
some point.
diff mbox series

Patch

diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index b071802..3d8cfaf 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -87,6 +87,12 @@  extern const struct tb_nhi_ops icl_nhi_ops;
 #define PCI_DEVICE_ID_INTEL_RPL_NHI0			0xa73e
 #define PCI_DEVICE_ID_INTEL_RPL_NHI1			0xa76d
 
+/* PCI IDs for AMD USB4 controllers */
+#define PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI0		0x162e
+#define PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI1		0x162f
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI0		0x1668
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI1		0x1669
+
 #define PCI_CLASS_SERIAL_USB_USB4			0x0c0340
 
 #endif
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 363d712..93e4788 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3574,6 +3574,12 @@  int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
 	if (root_sw->generation < 4 || tb_switch_is_tiger_lake(root_sw))
 		return 0;
 
+	/*
+	 * Disabling CLx by default on AMD USB4 platforms for Yellow Carp and Pink Sardine.
+	 */
+	if (tb_switch_is_yellow_carp(sw->tb->nhi) || tb_switch_is_pink_sardine(sw->tb->nhi))
+		return 0;
+
 	switch (clx) {
 	case TB_CL1:
 		/* CL0s and CL1 are enabled and supported together */
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index f978697..c63a023 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -905,6 +905,30 @@  static inline bool tb_switch_is_tiger_lake(const struct tb_switch *sw)
 	return false;
 }
 
+static inline bool tb_switch_is_yellow_carp(const struct tb_nhi *nhi)
+{
+	if (nhi->pdev->vendor == PCI_VENDOR_ID_AMD) {
+		switch (nhi->pdev->device) {
+		case PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI0:
+		case PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI1:
+			return true;
+		}
+	}
+	return false;
+}
+
+static inline bool tb_switch_is_pink_sardine(const struct tb_nhi *nhi)
+{
+	if (nhi->pdev->vendor == PCI_VENDOR_ID_AMD) {
+		switch (nhi->pdev->device) {
+		case PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI0:
+		case PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI1:
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * tb_switch_is_usb4() - Is the switch USB4 compliant
  * @sw: Switch to check