| Message ID | 20231009184643.129986-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c9ca8de2eb15f9da24113e652980c61f95a47530 |
| Headers | show |
| Series | [v3] usb: typec: ucsi: Use GET_CAPABILITY attributes data to set power supply scope | expand |
On Mon, Oct 09, 2023 at 01:46:43PM -0500, Mario Limonciello wrote: > On some OEM systems, adding a W7900 dGPU triggers RAS errors and hangs > at a black screen on startup. This issue occurs only if `ucsi_acpi` has > loaded before `amdgpu` has loaded. The reason for this failure is that > `amdgpu` uses power_supply_is_system_supplied() to determine if running > on AC or DC power at startup. If this value is reported incorrectly the > dGPU will also be programmed incorrectly and trigger errors. > > power_supply_is_system_supplied() reports the wrong value because UCSI > power supplies provided as part of the system don't properly report the > scope as "DEVICE" scope (not powering the system). > > In order to fix this issue check the capabilities reported from the UCSI > power supply to ensure that it supports charging a battery and that it can > be powered by AC. Mark the scope accordingly. > > Cc: stable@vger.kernel.org > Fixes: a7fbfd44c020 ("usb: typec: ucsi: Mark dGPUs as DEVICE scope") > Link: https://www.intel.com/content/www/us/en/products/docs/io/universal-serial-bus/usb-type-c-ucsi-spec.html p28 > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> > Cc: Alex Deucher <Alexander.Deucher@amd.com>> > Cc: Richard Gong <Richard.Gong@amd.com> > > v2->v3: > * Pick up tag > * Add missing stable tag > * Drop patch 2, we'll just avoid upstreaming it. > --- > drivers/usb/typec/ucsi/psy.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c > index 384b42267f1f..b35c6e07911e 100644 > --- a/drivers/usb/typec/ucsi/psy.c > +++ b/drivers/usb/typec/ucsi/psy.c > @@ -37,6 +37,15 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, > struct device *dev = con->ucsi->dev; > > device_property_read_u8(dev, "scope", &scope); > + if (scope == POWER_SUPPLY_SCOPE_UNKNOWN) { > + u32 mask = UCSI_CAP_ATTR_POWER_AC_SUPPLY | > + UCSI_CAP_ATTR_BATTERY_CHARGING; > + > + if (con->ucsi->cap.attributes & mask) > + scope = POWER_SUPPLY_SCOPE_SYSTEM; > + else > + scope = POWER_SUPPLY_SCOPE_DEVICE; > + } > val->intval = scope; > return 0; > } > -- > 2.34.1
On 10/10/2023 03:42, Heikki Krogerus wrote: > On Mon, Oct 09, 2023 at 01:46:43PM -0500, Mario Limonciello wrote: >> On some OEM systems, adding a W7900 dGPU triggers RAS errors and hangs >> at a black screen on startup. This issue occurs only if `ucsi_acpi` has >> loaded before `amdgpu` has loaded. The reason for this failure is that >> `amdgpu` uses power_supply_is_system_supplied() to determine if running >> on AC or DC power at startup. If this value is reported incorrectly the >> dGPU will also be programmed incorrectly and trigger errors. >> >> power_supply_is_system_supplied() reports the wrong value because UCSI >> power supplies provided as part of the system don't properly report the >> scope as "DEVICE" scope (not powering the system). >> >> In order to fix this issue check the capabilities reported from the UCSI >> power supply to ensure that it supports charging a battery and that it can >> be powered by AC. Mark the scope accordingly. >> >> Cc: stable@vger.kernel.org >> Fixes: a7fbfd44c020 ("usb: typec: ucsi: Mark dGPUs as DEVICE scope") >> Link: https://www.intel.com/content/www/us/en/products/docs/io/universal-serial-bus/usb-type-c-ucsi-spec.html p28 >> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Thanks! Greg, this behavior is pretty awful with the hang at bootup on these OEM systems, do you think this can come into one of the 6.6-rcX or should it wait until 6.7? > >> --- >> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> >> Cc: Alex Deucher <Alexander.Deucher@amd.com>> >> Cc: Richard Gong <Richard.Gong@amd.com> >> >> v2->v3: >> * Pick up tag >> * Add missing stable tag >> * Drop patch 2, we'll just avoid upstreaming it. >> --- >> drivers/usb/typec/ucsi/psy.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c >> index 384b42267f1f..b35c6e07911e 100644 >> --- a/drivers/usb/typec/ucsi/psy.c >> +++ b/drivers/usb/typec/ucsi/psy.c >> @@ -37,6 +37,15 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, >> struct device *dev = con->ucsi->dev; >> >> device_property_read_u8(dev, "scope", &scope); >> + if (scope == POWER_SUPPLY_SCOPE_UNKNOWN) { >> + u32 mask = UCSI_CAP_ATTR_POWER_AC_SUPPLY | >> + UCSI_CAP_ATTR_BATTERY_CHARGING; >> + >> + if (con->ucsi->cap.attributes & mask) >> + scope = POWER_SUPPLY_SCOPE_SYSTEM; >> + else >> + scope = POWER_SUPPLY_SCOPE_DEVICE; >> + } >> val->intval = scope; >> return 0; >> } >> -- >> 2.34.1 >
On Tue, Oct 10, 2023 at 12:49:35PM -0500, Mario Limonciello wrote: > On 10/10/2023 03:42, Heikki Krogerus wrote: > > On Mon, Oct 09, 2023 at 01:46:43PM -0500, Mario Limonciello wrote: > > > On some OEM systems, adding a W7900 dGPU triggers RAS errors and hangs > > > at a black screen on startup. This issue occurs only if `ucsi_acpi` has > > > loaded before `amdgpu` has loaded. The reason for this failure is that > > > `amdgpu` uses power_supply_is_system_supplied() to determine if running > > > on AC or DC power at startup. If this value is reported incorrectly the > > > dGPU will also be programmed incorrectly and trigger errors. > > > > > > power_supply_is_system_supplied() reports the wrong value because UCSI > > > power supplies provided as part of the system don't properly report the > > > scope as "DEVICE" scope (not powering the system). > > > > > > In order to fix this issue check the capabilities reported from the UCSI > > > power supply to ensure that it supports charging a battery and that it can > > > be powered by AC. Mark the scope accordingly. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: a7fbfd44c020 ("usb: typec: ucsi: Mark dGPUs as DEVICE scope") > > > Link: https://www.intel.com/content/www/us/en/products/docs/io/universal-serial-bus/usb-type-c-ucsi-spec.html p28 > > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > > > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Thanks! > > Greg, this behavior is pretty awful with the hang at bootup on these OEM > systems, do you think this can come into one of the 6.6-rcX or should it > wait until 6.7? I'll queue it up for 6.6-final, thanks. greg k-h
diff --git a/drivers/usb/typec/ucsi/psy.c b/drivers/usb/typec/ucsi/psy.c index 384b42267f1f..b35c6e07911e 100644 --- a/drivers/usb/typec/ucsi/psy.c +++ b/drivers/usb/typec/ucsi/psy.c @@ -37,6 +37,15 @@ static int ucsi_psy_get_scope(struct ucsi_connector *con, struct device *dev = con->ucsi->dev; device_property_read_u8(dev, "scope", &scope); + if (scope == POWER_SUPPLY_SCOPE_UNKNOWN) { + u32 mask = UCSI_CAP_ATTR_POWER_AC_SUPPLY | + UCSI_CAP_ATTR_BATTERY_CHARGING; + + if (con->ucsi->cap.attributes & mask) + scope = POWER_SUPPLY_SCOPE_SYSTEM; + else + scope = POWER_SUPPLY_SCOPE_DEVICE; + } val->intval = scope; return 0; }
On some OEM systems, adding a W7900 dGPU triggers RAS errors and hangs at a black screen on startup. This issue occurs only if `ucsi_acpi` has loaded before `amdgpu` has loaded. The reason for this failure is that `amdgpu` uses power_supply_is_system_supplied() to determine if running on AC or DC power at startup. If this value is reported incorrectly the dGPU will also be programmed incorrectly and trigger errors. power_supply_is_system_supplied() reports the wrong value because UCSI power supplies provided as part of the system don't properly report the scope as "DEVICE" scope (not powering the system). In order to fix this issue check the capabilities reported from the UCSI power supply to ensure that it supports charging a battery and that it can be powered by AC. Mark the scope accordingly. Cc: stable@vger.kernel.org Fixes: a7fbfd44c020 ("usb: typec: ucsi: Mark dGPUs as DEVICE scope") Link: https://www.intel.com/content/www/us/en/products/docs/io/universal-serial-bus/usb-type-c-ucsi-spec.html p28 Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> Cc: Alex Deucher <Alexander.Deucher@amd.com>> Cc: Richard Gong <Richard.Gong@amd.com> v2->v3: * Pick up tag * Add missing stable tag * Drop patch 2, we'll just avoid upstreaming it. --- drivers/usb/typec/ucsi/psy.c | 9 +++++++++ 1 file changed, 9 insertions(+)