Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] rockchip_lvds_bind overwrites external bridge function pointers, performs invalid frees on error paths, and leaks objects on unbind. -- --- Patch [1]: [PATCH v2 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state --- commit 1b0e3d7f00cf4d61b195bc93e8d5dc954705b92b Author: Rok Markovic <[email protected]> drm/rockchip: lvds: propagate bus_flags to the CRTC state rockchip_lvds_encoder_atomic_check() sets output_mode and output_type but never copies the connector's bus_flags into the Rockchip CRTC state. This prevents the VOP from deriving the pixel clock polarity, causing panels to be clocked on the wrong edge. > 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 [ ... ] While reviewing this patch, I noticed some pre-existing issues in the same file. Looking at 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; [Severity: High] This is a pre-existing issue, but does overwriting the bridge's function pointers and private data here destroy the foreign driver state if this is an external bridge? It seems this would break drm_bridge_is_panel() and prevent the bridge from being enabled or providing modes. Further down in rockchip_lvds_bind() on the error path: err_free_bridge: drm_panel_bridge_remove(lvds->bridge); [Severity: High] This is a pre-existing issue, but could this error path leak the panel bridge? Since the bridge's funcs were overwritten earlier in this function, drm_bridge_is_panel() will return false. This would cause drm_panel_bridge_remove() to early-return without freeing the bridge. Looking at rockchip_lvds_unbind(): static void rockchip_lvds_unbind(struct device *dev, struct device *master, void *data) { struct rockchip_lvds *lvds = dev_get_drvdata(dev); const struct drm_encoder_helper_funcs *encoder_funcs; encoder_funcs = lvds->soc_data->helper_funcs; encoder_funcs->disable(&lvds->encoder.encoder); pm_runtime_disable(dev); } [Severity: High] This isn't a bug introduced by this patch, but is the unbind function missing cleanup for the bridge, encoder, and connector? It appears these objects will leak and corrupt the DRM state list on every unbind. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
