Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unconditional call to drmm_connector_dp_init() breaks bridge chains with downstream format converters. - [Low] Kernel-doc for DRM_BRIDGE_OP_DP incorrectly references a non-existent structure member. --
commit 95b54bb01e2f24e6a00ddf825b25aaa6985fa864 Author: Kory Maincent <[email protected]> drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag Introduce DRM_BRIDGE_OP_DP to indicate bridges that provide DP connector operations. Update drm_bridge_connector_init() to use drmm_connector_dp_init() when this flag is present to expose link training state properties. > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c > b/drivers/gpu/drm/display/drm_bridge_connector.c > index 39cc18f78eda1..5238b1ec738ce 100644 > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c [ ... ] > @@ -986,6 +1003,13 @@ struct drm_connector *drm_bridge_connector_init(struct > drm_device *drm, > max_bpc); > if (ret) > return ERR_PTR(ret); > + } else if (bridge_connector->bridge_dp) { > + ret = drmm_connector_dp_init(drm, connector, > + &drm_bridge_connector_funcs, > + > bridge_connector->bridge_dp->dp_link_caps, > + connector_type, ddc); > + if (ret) > + return ERR_PTR(ret); [Severity: High] If a DRM bridge chain consists of a DisplayPort transmitter followed by a format-converting bridge (e.g., DP-to-VGA or DP-to-LVDS), the final connector_type of the chain will be the output format (like DRM_MODE_CONNECTOR_VGA). Since drmm_connector_dp_init() explicitly enforces that connector_type must be DRM_MODE_CONNECTOR_DisplayPort or DRM_MODE_CONNECTOR_eDP, will this call immediately return -EINVAL and break display initialization for legitimate format-converted hardware combinations? Does the branch condition need to verify the final connector_type is actually DP/eDP before forcing the DP initialization path? > } else { > ret = drmm_connector_init(drm, connector, [ ... ] > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index a8d67bd9ee505..63db754d21146 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -1092,6 +1092,14 @@ enum drm_bridge_ops { > * &drm_bridge_funcs->hdmi_clear_spd_infoframe callbacks. > */ > DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME = BIT(10), > + /** > + * @DRM_BRIDGE_OP_DP: The bridge provides DisplayPort connector > + * operations, including link training support. Bridges that set > + * this flag must provide DisplayPort-related information and > + * fill the &drm_bridge->dp_link_train_caps link training [Severity: Low] This isn't a bug, but there's a typo in the documentation. It references &drm_bridge->dp_link_train_caps, but the actual structure member added in this patch is named dp_link_caps. > + * capabilities. > + */ > + DRM_BRIDGE_OP_DP = BIT(11), > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
