Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/analogix/anx7625: wrong eDP check in anx7625_bridge_atomic_disable() --
commit 49aa21f51dedc9ed0b5c807bbb55946e8c40288d Author: Kory Maincent <[email protected]> drm/analogix/anx7625: Use dp_connector helpers to expose link training state This commit adopts the DisplayPort connector helpers in the anx7625 bridge driver by setting DRM_BRIDGE_OP_DP, populating dp_link_caps, and reporting sink capabilities and link training parameters. > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > index c3af88e14f8b2..2b1235efc9fa4 100644 > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c [ ... ] > @@ -2580,6 +2606,17 @@ static void anx7625_bridge_atomic_enable(struct > drm_bridge *bridge, > > anx7625_dp_start(ctx); > > + /* On DisplayPort case, the caps are set in the detect callback. */ > + if (bridge->type == DRM_MODE_CONNECTOR_eDP) { > + ret = drm_dp_sink_sync_caps(connector, &ctx->aux, NULL); > + if (ret) > + DRM_DEV_DEBUG_DRIVER(dev, > + "error sink caps synchronisation > (%d)\n", > + ret); > + } > + > + anx7625_report_link_train(ctx, connector); > + > conn_state = drm_atomic_get_new_connector_state(state, connector); > > if (WARN_ON(!conn_state)) > @@ -2616,6 +2653,12 @@ static void anx7625_bridge_atomic_disable(struct > drm_bridge *bridge, > dev_dbg(dev, "update CP to DESIRE\n"); > } > > + /* On DisplayPort unplug, the caps are already reset in the detect > + * callback. > + */ > + if (bridge->type == DRM_MODE_CONNECTOR_eDP) > + drm_dp_sink_reset_caps(ctx->connector); [Severity: Medium] In anx7625_bridge_atomic_enable() and anx7625_bridge_atomic_disable(), does checking bridge->type == DRM_MODE_CONNECTOR_eDP work as intended on DisplayPort boards that wire port 1 to a downstream dp-connector bridge? In anx7625_parse_dt_panel(), pdata->panel_bridge is populated with whatever bridge is connected on port 1: drivers/gpu/drm/bridge/analogix/anx7625.c:anx7625_parse_dt_panel() { pdata->panel_bridge = devm_drm_of_get_bridge(dev, np, 1, 0); ... } Then in anx7625_link_bridge(), DRM_BRIDGE_OP_DETECT is enabled whenever the panel is not on the aux-bus, while bridge.type is set to DRM_MODE_CONNECTOR_eDP whenever pdata.panel_bridge is non-NULL: drivers/gpu/drm/bridge/analogix/anx7625.c:anx7625_link_bridge() { ... if (!platform->pdata.panel_bridge || !anx7625_of_panel_on_aux_bus(dev)) platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT; platform->bridge.type = platform->pdata.panel_bridge ? DRM_MODE_CONNECTOR_eDP : DRM_MODE_CONNECTOR_DisplayPort; ... } On boards using a downstream dp-connector bridge on port 1 (such as qcs615-ride, lemans-ride, or qcs8300-ride), pdata.panel_bridge is non-NULL, so bridge->type is DRM_MODE_CONNECTOR_eDP even though DRM_BRIDGE_OP_DETECT is set and the connector's connector_type is DRM_MODE_CONNECTOR_DisplayPort. When the display is disabled via anx7625_bridge_atomic_disable() while the DisplayPort monitor remains plugged in (for example during dpms off or a modeset disable), bridge->type == DRM_MODE_CONNECTOR_eDP evaluates to true and calls drm_dp_sink_reset_caps(ctx->connector), clearing connector->dp.sink_link_caps and connector->dp.cur_link_info for the still-connected sink until a future hotplug event occurs. Similarly, anx7625_bridge_atomic_enable() redundantly calls drm_dp_sink_sync_caps() even though anx7625_bridge_detect() already synchronized the caps on hotplug. Should these checks test !(bridge->ops & DRM_BRIDGE_OP_DETECT) (or check ctx->connector->connector_type == DRM_MODE_CONNECTOR_eDP) instead of bridge->type? > + > ctx->connector = NULL; > anx7625_dp_stop(ctx); > [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
