Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] The newly introduced bus_flags assignment bypasses DRM bridge state negotiation by reading flags directly from the final connector instead of the adjacent bridge. Pre-existing issues: - [High] The driver unconditionally overwrites the function pointers and private data of external bridge objects, causing type confusion and completely breaking external bridge support. -- --- Patch [1]: [PATCH 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state --- commit e03a59004874598201254d64c0f6e0e88de3ff1a Author: Rok Markovic <[email protected]> drm/rockchip: lvds: propagate bus_flags to the CRTC state This commit updates the Rockchip LVDS encoder's atomic_check function to propagate bus_flags from the connector's display_info to the CRTC state. This ensures that the VOP can correctly set the pixel clock polarity based on the panel's requirements. > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c > b/drivers/gpu/drm/rockchip/rockchip_lvds.c > index 7a0c4fa29f2f0..150299c12298a 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c > @@ -129,7 +129,15 @@ rockchip_lvds_encoder_atomic_check(struct drm_encoder > *encoder, > struct drm_connector_state *conn_state) > { > struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state); > - > + struct drm_connector *connector = conn_state->connector; > + > + /* > + * The VOP derives the pixel clock polarity from this. Without it a > + * panel that declares pixelclk-active = <0> is clocked on the wrong > + * edge. panel-lvds fills the connector's bus_flags in from the DT > + * display timing. > + */ > + s->bus_flags = connector->display_info.bus_flags; [Severity: Medium] Does this bypass DRM bridge state negotiation by reading flags directly from the final connector instead of the adjacent bridge? By reading from connector->display_info.bus_flags in rockchip_lvds_encoder_atomic_check(), the VOP pixel clock polarity is configured based on the final sink (e.g., an HDMI monitor) rather than the LVDS bridge's input requirements. This causes incorrect pixel clock polarity for external bridge setups, as it violates DRM bridge chaining contracts which expect the state to be retrieved via drm_atomic_get_new_bridge_state(). > s->output_mode = ROCKCHIP_OUT_MODE_P888; > s->output_type = DRM_MODE_CONNECTOR_LVDS; [Severity: High] This isn't a bug introduced by this patch, but does the driver unconditionally overwrite the function pointers and private data of external bridge objects in rockchip_lvds_bind()? When a valid external bridge is present, the initialization path overwrites the bridge data: rockchip_lvds_bind() { ... if (lvds->bridge) { lvds->bridge->driver_private = lvds; lvds->bridge->ops = DRM_BRIDGE_OP_MODES; lvds->bridge->funcs = &rockchip_lvds_bridge_funcs; ... } Does this hijack the external bridge by replacing its callbacks and private data with LVDS-specific values? This causes type confusion and prevents external bridges from ever being enabled, since their callbacks are replaced by the LVDS functions. It also seems likely to cause panics if the external bridge driver attempts to access its original driver_private data. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
