Messages in this thread | | | Date | Wed, 8 Mar 2023 10:58:18 +0100 | From | Johan Hovold <> | Subject | Re: [PATCH] drm/msm: Initialize mode_config earlier |
| |
On Thu, Mar 02, 2023 at 03:17:04PM -0800, Bjorn Andersson wrote: > On Wed, Mar 01, 2023 at 02:58:50PM +0100, Johan Hovold wrote:
> > So after debugging this issue a third time, I can conclude that it is > > still very much present in 6.2. > > > > It appears you looked at the linux-next tree when you concluded that > > this patch was not needed. In 6.2 the bridge->hpd_cb callback is set > > before mode_config.funcs is initialised as part of > > kms->funcs->hw_init(kms). > > > > The hpd DRM changes heading into 6.3 do appear to avoid the NULL-pointer > > dereference by moving the bridge->hpd_cb initialisation to > > drm_kms_helper_poll_init() as you mention above.
I can confirm that as expected my reproducer no longer triggers with 6.3-rc1.
> > The PMIC GLINK altmode driver still happily forwards notifications > > regardless of the DRM driver state though, which can lead to missed > > hotplug events. It seems you need to implement the > > hpd_enable()/disable() callbacks and either cache or not enable events > > in fw until the DRM driver is ready. > > > > It's not clear to me what the expectation from the DRM framework is on > this point. We register a drm_bridge which is only capable of signaling > HPD events (DRM_BRIDGE_OP_HPD), not querying HPD state (DRM_BRIDGE_OP_DETECT).
I think the assumption is that any bridge that can generate hotplug events also has a way of detecting whether it is connected (i.e. DRM_BRIDGE_OP_HPD => DRM_BRIDGE_OP_DETECT).
The pmic_glink_altmode driver appears to be the only driver that sets DRM_BRIDGE_OP_HPD but not DRM_BRIDGE_OP_DETECT.
> Does this imply that any such bridge must ensure that hpd events are > re-delivered once hpd_enable() has been invoked (we can't invoke it from > hpd_enable...)? > > Is it reasonable to do this retriggering in the altmode driver? Or is it > the job of the TCPM (it seems reasonable to not send the PAN_EN message > until we get hpd_enable()...)?
Are you sure there is no way to query the firmware about the connected state?
Otherwise, enabling the notification messages when hpd_enable() is called looks like it should work as the fw currently appears to always send a disconnected event followed by a connect event if connected.
But that's not going to be enough unless you can also disable events in fw on hpd_disable() so that the state can again be updated on the next hpd_enable().
If that's not possible, it seems you need to cache the state in the driver and hope you get a notification after a suspend cycle if the state has changed.
But in any case, the DRM documentation is pretty clear on that a bridge driver should not be calling drm_bridge_hpd_notify() until hpd_enable() is called (and also not after hpd_disable()) as the pmic_glink_altmode driver currently do.
hpd_enable
Enable hot plug detection. From now on the bridge shall call drm_bridge_hpd_notify() each time a change is detected in the output connection status, until hot plug detection gets disabled with hpd_disable.
This callback is optional and shall only be implemented by bridges that support hot-plug notification without polling. Bridges that implement it shall also implement the hpd_disable callback and set the DRM_BRIDGE_OP_HPD flag in their drm_bridge->ops.
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#c.drm_bridge_funcs
Johan
| |