drivers/soc/qcom/pmic_glink_altmode.c | 30 +++++++++++++++++---------- 1 file changed, 19 insertions(+), 11 deletions(-)
The PMIC GLINK driver is currently generating DisplayPort hotplug
notifications whenever something is connected to (or disconnected from)
a port regardless of the type of notification sent by the firmware.
These notifications are forwarded to user space by the DRM subsystem as
connector "change" uevents:
KERNEL[1556.223776] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
ACTION=change
DEVPATH=/devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0
SUBSYSTEM=drm
HOTPLUG=1
CONNECTOR=36
DEVNAME=/dev/dri/card0
DEVTYPE=drm_minor
SEQNUM=4176
MAJOR=226
MINOR=0
On the Lenovo ThinkPad X13s and T14s, the PMIC GLINK firmware sends two
identical notifications with orientation information when connecting a
charger, each generating a bogus DRM hotplug event. On the X13s, two
such notification are also sent every 90 seconds while a charger remains
connected, which again are forwarded to user space:
port = 1, svid = ff00, mode = 255, hpd_state = 0
payload = 01 00 00 00 00 00 00 ff 00 00 00 00 00 00 00 00
Note that the firmware only sends on of these when connecting an
ethernet adapter.
Fix the spurious hotplug events by only forwarding hotplug notifications
for the Type-C DisplayPort service id. This also reduces the number of
uevents from four to two when an actual DisplayPort altmode device is
connected:
port = 0, svid = ff01, mode = 2, hpd_state = 0
payload = 00 01 02 00 f2 0c 01 ff 03 00 00 00 00 00 00 00
port = 0, svid = ff01, mode = 2, hpd_state = 1
payload = 00 01 02 00 f2 0c 01 ff 43 00 00 00 00 00 00 00
Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
Cc: stable@vger.kernel.org # 6.3
Cc: Bjorn Andersson <andersson@kernel.org>
Reported-by: Clayton Craft <clayton@craftyguy.net>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
Clayton reported seeing display flickering with recent RC kernels, which
may possibly be related to these spurious events being generated with
even greater frequency.
That still remains to be fully understood, but the spurious events, that
on the X13s are generated every 90 seconds, should be fixed either way.
Johan
drivers/soc/qcom/pmic_glink_altmode.c | 30 +++++++++++++++++----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
index bd06ce161804..7f11acd33323 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -218,21 +218,29 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
{
struct pmic_glink_altmode_port *alt_port = work_to_altmode_port(work);
struct pmic_glink_altmode *altmode = alt_port->altmode;
+ enum drm_connector_status conn_status;
typec_switch_set(alt_port->typec_switch, alt_port->orientation);
- if (alt_port->svid == USB_TYPEC_DP_SID && alt_port->mode == 0xff)
- pmic_glink_altmode_safe(altmode, alt_port);
- else if (alt_port->svid == USB_TYPEC_DP_SID)
- pmic_glink_altmode_enable_dp(altmode, alt_port, alt_port->mode,
- alt_port->hpd_state, alt_port->hpd_irq);
- else
- pmic_glink_altmode_enable_usb(altmode, alt_port);
+ if (alt_port->svid == USB_TYPEC_DP_SID) {
+ if (alt_port->mode == 0xff) {
+ pmic_glink_altmode_safe(altmode, alt_port);
+ } else {
+ pmic_glink_altmode_enable_dp(altmode, alt_port,
+ alt_port->mode,
+ alt_port->hpd_state,
+ alt_port->hpd_irq);
+ }
- drm_aux_hpd_bridge_notify(&alt_port->bridge->dev,
- alt_port->hpd_state ?
- connector_status_connected :
- connector_status_disconnected);
+ if (alt_port->hpd_state)
+ conn_status = connector_status_connected;
+ else
+ conn_status = connector_status_disconnected;
+
+ drm_aux_hpd_bridge_notify(&alt_port->bridge->dev, conn_status);
+ } else {
+ pmic_glink_altmode_enable_usb(altmode, alt_port);
+ }
pmic_glink_altmode_request(altmode, ALTMODE_PAN_ACK, alt_port->index);
}
--
2.48.1
On Mon, 24 Mar 2025 14:24:48 +0100, Johan Hovold wrote:
> The PMIC GLINK driver is currently generating DisplayPort hotplug
> notifications whenever something is connected to (or disconnected from)
> a port regardless of the type of notification sent by the firmware.
>
> These notifications are forwarded to user space by the DRM subsystem as
> connector "change" uevents:
>
> [...]
Applied, thanks!
[1/1] soc: qcom: pmic_glink_altmode: fix spurious DP hotplug events
commit: 5090ac9191a19c61beeade60d3d839e509fab640
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
On 3/24/25 2:24 PM, Johan Hovold wrote: > The PMIC GLINK driver is currently generating DisplayPort hotplug > notifications whenever something is connected to (or disconnected from) > a port regardless of the type of notification sent by the firmware. Yikes! Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> That said, I'm hoping there isn't any sort of "port is full of water, emergency" messages that we should treat as "unplug" though.. Konrad
On Mon, Mar 24, 2025 at 08:21:10PM +0100, Konrad Dybcio wrote: > On 3/24/25 2:24 PM, Johan Hovold wrote: > > The PMIC GLINK driver is currently generating DisplayPort hotplug > > notifications whenever something is connected to (or disconnected from) > > a port regardless of the type of notification sent by the firmware. > > Yikes! > > Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > > That said, I'm hoping there isn't any sort of "port is full of water, > emergency" messages that we should treat as "unplug" though.. Seems a bit far fetched, but I guess only you guys inside Qualcomm can try to figure that out. An alternative could be to cache the hpd_state regardless of the svid and only forward changes. But perhaps the hpd_state bit is only valid for DP notifications. Johan
On 3/25/25 9:40 AM, Johan Hovold wrote: > On Mon, Mar 24, 2025 at 08:21:10PM +0100, Konrad Dybcio wrote: >> On 3/24/25 2:24 PM, Johan Hovold wrote: >>> The PMIC GLINK driver is currently generating DisplayPort hotplug >>> notifications whenever something is connected to (or disconnected from) >>> a port regardless of the type of notification sent by the firmware. >> >> Yikes! >> >> Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> >> >> That said, I'm hoping there isn't any sort of "port is full of water, >> emergency" messages that we should treat as "unplug" though.. > > Seems a bit far fetched, but I guess only you guys inside Qualcomm can > try to figure that out. I tried looking around, but couldn't find anything like that > An alternative could be to cache the hpd_state regardless of the svid > and only forward changes. But perhaps the hpd_state bit is only valid > for DP notifications. The current state of your patch seems to be a good approach, I think. Konrad
On 3/24/25 06:24, Johan Hovold wrote:
> The PMIC GLINK driver is currently generating DisplayPort hotplug
> notifications whenever something is connected to (or disconnected from)
> a port regardless of the type of notification sent by the firmware.
>
> These notifications are forwarded to user space by the DRM subsystem as
> connector "change" uevents:
>
> KERNEL[1556.223776] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> ACTION=change
> DEVPATH=/devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0
> SUBSYSTEM=drm
> HOTPLUG=1
> CONNECTOR=36
> DEVNAME=/dev/dri/card0
> DEVTYPE=drm_minor
> SEQNUM=4176
> MAJOR=226
> MINOR=0
>
> On the Lenovo ThinkPad X13s and T14s, the PMIC GLINK firmware sends two
> identical notifications with orientation information when connecting a
> charger, each generating a bogus DRM hotplug event. On the X13s, two
> such notification are also sent every 90 seconds while a charger remains
> connected, which again are forwarded to user space:
>
> port = 1, svid = ff00, mode = 255, hpd_state = 0
> payload = 01 00 00 00 00 00 00 ff 00 00 00 00 00 00 00 00
>
> Note that the firmware only sends on of these when connecting an
> ethernet adapter.
>
> Fix the spurious hotplug events by only forwarding hotplug notifications
> for the Type-C DisplayPort service id. This also reduces the number of
> uevents from four to two when an actual DisplayPort altmode device is
> connected:
>
> port = 0, svid = ff01, mode = 2, hpd_state = 0
> payload = 00 01 02 00 f2 0c 01 ff 03 00 00 00 00 00 00 00
> port = 0, svid = ff01, mode = 2, hpd_state = 1
> payload = 00 01 02 00 f2 0c 01 ff 43 00 00 00 00 00 00 00
>
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Cc: stable@vger.kernel.org # 6.3
> Cc: Bjorn Andersson <andersson@kernel.org>
> Reported-by: Clayton Craft <clayton@craftyguy.net>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>
> Clayton reported seeing display flickering with recent RC kernels, which
> may possibly be related to these spurious events being generated with
> even greater frequency.
>
> That still remains to be fully understood, but the spurious events, that
> on the X13s are generated every 90 seconds, should be fixed either way.
When a display/dock (which has ethernet) is connected, I see this
hotplug change event 2 times (every 30 seconds) which I think you said
this is expected now?
> UDEV [236.150574] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [236.588696] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [266.208175] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [266.644710] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [296.243187] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [296.678177] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [326.276256] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
> UDEV [326.712248] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm)
Not sure about you seeing it every 90s vs my 30s... anyways, I no longer
see these events when a PD charger is connected though, so this patch
seems to help with that!
Tested-by: Clayton Craft <clayton@craftyguy.net>
On Mon, Mar 24, 2025 at 10:05:44AM -0700, Clayton Craft wrote: > On 3/24/25 06:24, Johan Hovold wrote: > > The PMIC GLINK driver is currently generating DisplayPort hotplug > > notifications whenever something is connected to (or disconnected from) > > a port regardless of the type of notification sent by the firmware. > > > > These notifications are forwarded to user space by the DRM subsystem as > > connector "change" uevents: > > --- > > > > Clayton reported seeing display flickering with recent RC kernels, which > > may possibly be related to these spurious events being generated with > > even greater frequency. > > > > That still remains to be fully understood, but the spurious events, that > > on the X13s are generated every 90 seconds, should be fixed either way. > > When a display/dock (which has ethernet) is connected, I see this > hotplug change event 2 times (every 30 seconds) which I think you said > this is expected now? I didn't realise you were also using a display/dock. Bjorn mentioned that he has noticed issues with one of his monitors (e.g. built-in hub reenumerating repeatedly iirc) which may be related. I see these pairs of identical notification when connecting the stock charger to one of the ports directly, and I noticed that they repeat every 90 seconds here. After plugging and unplugging a bunch of devices I think they stopped at one point, but they were there again after a reboot. So there's something going on with the PMIC GLINK firmware or driver on the X13s. I did not see these repeated messages on the T14s with just a charger (and I don't have a dock to test with). > > UDEV [236.150574] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [236.588696] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [266.208175] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [266.644710] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [296.243187] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [296.678177] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [326.276256] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > UDEV [326.712248] change /devices/platform/soc@0/ae00000.display-subsystem/ae01000.display-controller/drm/card0 (drm) > > Not sure about you seeing it every 90s vs my 30s... anyways, I no longer > see these events when a PD charger is connected though, so this patch > seems to help with that! Just so I understand you correctly here, you're no longer seeing the repeated uevents with this patch? Both when using a dock and when using a charger directly? Did it help with the display flickering too? Was that only on the external display? > Tested-by: Clayton Craft <clayton@craftyguy.net> Thanks for testing. Johan
On Tue, Mar 25, 2025 at 09:32:10AM +0100, Johan Hovold wrote: > On Mon, Mar 24, 2025 at 10:05:44AM -0700, Clayton Craft wrote: > > On 3/24/25 06:24, Johan Hovold wrote: > > > The PMIC GLINK driver is currently generating DisplayPort hotplug > > > notifications whenever something is connected to (or disconnected from) > > > a port regardless of the type of notification sent by the firmware. > > > > > > These notifications are forwarded to user space by the DRM subsystem as > > > connector "change" uevents: > > > > --- > > > > > > Clayton reported seeing display flickering with recent RC kernels, which > > > may possibly be related to these spurious events being generated with > > > even greater frequency. > > > > > > That still remains to be fully understood, but the spurious events, that > > > on the X13s are generated every 90 seconds, should be fixed either way. > > > > When a display/dock (which has ethernet) is connected, I see this > > hotplug change event 2 times (every 30 seconds) which I think you said > > this is expected now? > > I didn't realise you were also using a display/dock. Bjorn mentioned > that he has noticed issues with one of his monitors (e.g. built-in hub > reenumerating repeatedly iirc) which may be related. > > I see these pairs of identical notification when connecting the stock > charger to one of the ports directly, and I noticed that they repeat > every 90 seconds here. After plugging and unplugging a bunch of devices > I think they stopped at one point, but they were there again after a > reboot. > > So there's something going on with the PMIC GLINK firmware or driver on > the X13s. I did not see these repeated messages on the T14s with just a > charger (and I don't have a dock to test with). With this patch enabling UCSI on sc8280xp: https://lore.kernel.org/lkml/20250326124944.6338-1-johan+linaro@kernel.org/ most of the periodic orientation notifications for the port with the charger connected appears to be gone on the X13s (note that the T14s already has UCSI enabled). I get one to three notification 90 seconds after boot with the charger connected (and two notifications when reconnecting it) but that appears to be it. Perhaps you can give that one a try with your docks and monitors as well, Clayton and Bjorn. > > Not sure about you seeing it every 90s vs my 30s... anyways, I no longer > > see these events when a PD charger is connected though, so this patch > > seems to help with that! > > Just so I understand you correctly here, you're no longer seeing the > repeated uevents with this patch? Both when using a dock and when using > a charger directly? > > Did it help with the display flickering too? Was that only on the > external display? Johan
On 3/26/25 07:43, Johan Hovold wrote: > On Tue, Mar 25, 2025 at 09:32:10AM +0100, Johan Hovold wrote: >> On Mon, Mar 24, 2025 at 10:05:44AM -0700, Clayton Craft wrote: >>> On 3/24/25 06:24, Johan Hovold wrote: >>>> The PMIC GLINK driver is currently generating DisplayPort hotplug >>>> notifications whenever something is connected to (or disconnected from) >>>> a port regardless of the type of notification sent by the firmware. >>>> >>>> These notifications are forwarded to user space by the DRM subsystem as >>>> connector "change" uevents: >> >>>> --- >>>> >>>> Clayton reported seeing display flickering with recent RC kernels, which >>>> may possibly be related to these spurious events being generated with >>>> even greater frequency. >>>> >>>> That still remains to be fully understood, but the spurious events, that >>>> on the X13s are generated every 90 seconds, should be fixed either way. >>> >>> When a display/dock (which has ethernet) is connected, I see this >>> hotplug change event 2 times (every 30 seconds) which I think you said >>> this is expected now? >> >> I didn't realise you were also using a display/dock. Bjorn mentioned >> that he has noticed issues with one of his monitors (e.g. built-in hub >> reenumerating repeatedly iirc) which may be related. Sorry for the confusion, let me clarify: The original issue I reported to you on IRC was *without* a dock/external display attached, only a PD adapter was attached. With your patch, I no longer see these drm hotplug events in this scenario. After confirming that your patch resolved the spurious hotplug events when using a PD charger, I connected a dock+external display to see if the patch caused any regressions there for me. It was here that I noticed a periodic hotplug event firing 2 times every 30 seconds was still showing up. I don't know if this is expected or not, I've never noticed it before because I wasn't monitoring udev events. >> Just so I understand you correctly here, you're no longer seeing the >> repeated uevents with this patch? Both when using a dock and when using >> a charger directly? With PD charger only: no more spurious hotplug events firing With dock+external displauy: 2x hotplug events every 30s. >> >> Did it help with the display flickering too? Was that only on the >> external display? The flickering was only on the internal display, and your patch here seems to have resolved that. -Clayton
On Tue, Apr 01, 2025 at 11:18:49AM -0700, Clayton Craft wrote: > On 3/26/25 07:43, Johan Hovold wrote: > > On Tue, Mar 25, 2025 at 09:32:10AM +0100, Johan Hovold wrote: > >> I didn't realise you were also using a display/dock. Bjorn mentioned > >> that he has noticed issues with one of his monitors (e.g. built-in hub > >> reenumerating repeatedly iirc) which may be related. > > Sorry for the confusion, let me clarify: > > The original issue I reported to you on IRC was *without* a > dock/external display attached, only a PD adapter was attached. With > your patch, I no longer see these drm hotplug events in this scenario. > > After confirming that your patch resolved the spurious hotplug events > when using a PD charger, I connected a dock+external display to see if > the patch caused any regressions there for me. It was here that I > noticed a periodic hotplug event firing 2 times every 30 seconds was > still showing up. I don't know if this is expected or not, I've never > noticed it before because I wasn't monitoring udev events. Got it, thanks for clarifying. Did you get a chance to try the patch enabling UCSI and USB-PD? That one may possibly help with the dock issue: https://lore.kernel.org/lkml/20250326124944.6338-1-johan+linaro@kernel.org/ > >> Did it help with the display flickering too? Was that only on the > >> external display? > > The flickering was only on the internal display, and your patch here > seems to have resolved that. Good. Is there any flickering on either screen when you see the spurious HP events with the dock? Johan
© 2016 - 2025 Red Hat, Inc.