Hi, On 16/01/2025 13:16, Jayesh Choudhary wrote:
For the cases we have DRM_BRIDGE_ATTACH_NO_CONNECTOR flag set,
Any idea if any other platform than K3 is using this driver? tidss supports DRM_BRIDGE_ATTACH_NO_CONNECTOR, so if K3 is the only user, we could drop the legacy !DRM_BRIDGE_ATTACH_NO_CONNECTOR case. Which would remove quite a bit of code, I think, and make the driver a bit more easy to understand (although I think it could use a major cleanup...).
the connector structure is not initialised in the bridge. That's done by encoder. So in case of some failure in cdns_mhdp_atomic_enable, when we schedule work for modeset_retry_work, we will use the mutex of connector which will result in NULL pointer dereference. Handle it by adding condition for the connector. Otherwise, since the modeset_retry_work tries to set the connector status as bad, set the mhdp->plugged as false which would give the connector status as disconnected in detect hook.
I'm not quite sure if this whole system makes sense (no one else is doing it), but I think you can find the connector from the current state. Then setting the property could be done for DRM_BRIDGE_ATTACH_NO_CONNECTOR case too.
Tomi
Fixes: fb43aa0acdfd ("drm: bridge: Add support for Cadence MHDP8546 DPI/DP bridge") Signed-off-by: Jayesh Choudhary <j-choudhary@xxxxxx> --- NOTE: Found this issue in one particular board where edid read failed. Issue log: <https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429> Adding conditional fixes the null pointer issue but there is still flooding of these logs (128 times): "cdns-mhdp8546 a000000.bridge: Failed to read DPCD addr 0" Sending RFC as I am still not sure about how to handle this flooding. Is it okay to decrease the log level for DPCD read and DPCD write in the cdns_mhdp_transfer to debug? .../drm/bridge/cadence/cdns-mhdp8546-core.c | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d081850e3c03..6a121a2700d2 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -2363,18 +2363,20 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)mhdp = container_of(work, typeof(*mhdp), modeset_retry_work); - conn = &mhdp->connector;- - /* Grab the locks before changing connector property */ - mutex_lock(&conn->dev->mode_config.mutex); - - /* - * Set connector link status to BAD and send a Uevent to notify - * userspace to do a modeset. - */ - drm_connector_set_link_status_property(conn, DRM_MODE_LINK_STATUS_BAD); - mutex_unlock(&conn->dev->mode_config.mutex); + if (mhdp->connector.dev) { + conn = &mhdp->connector; + /* Grab the locks before changing connector property */ + mutex_lock(&conn->dev->mode_config.mutex);+ /*+ * Set connector link status to BAD and send a Uevent to notify + * userspace to do a modeset. + */ + drm_connector_set_link_status_property(conn, DRM_MODE_LINK_STATUS_BAD); + mutex_unlock(&conn->dev->mode_config.mutex); + } else { + mhdp->plugged = false; + } /* Send Hotplug uevent so userspace can reprobe */ drm_kms_helper_hotplug_event(mhdp->bridge.dev); }
