Hello Sebastian, At 2026-08-07 01:02:08, "Sebastian Reichel" <[email protected]> wrote: >Currently the Synopsys DesignWare DP controller driver's bind function >requests lots of resources using device managed functions. These are >free'd on driver removal instead of at unbind time. Fix this discrepancy >by introducing a new probe helper function and moving over the whole >bind function. This results in a fully functional DRM bridge once probe >succeeded. The only thing still happening when the component is bound >is the bridge attachment, which requires the encoder. > >The interrupt is kept disabled while the bridge is detached to ensure no >spurious interrupts can arrive as the interrupt handler triggers a >worker, which accesses the DRM device. > >Fixes: 86eecc3a9c2e ("drm/bridge: synopsys: Add DW DPTX Controller support >library") >Reported-by: Sashiko <[email protected]> >Signed-off-by: Sebastian Reichel <[email protected]>
Acked-by: Andy Yan <[email protected]> >--- > drivers/gpu/drm/bridge/synopsys/dw-dp.c | 73 ++++++++++++++++--------------- > drivers/gpu/drm/rockchip/dw_dp-rockchip.c | 53 ++++++++++++---------- > include/drm/bridge/dw_dp.h | 5 ++- > 3 files changed, 72 insertions(+), 59 deletions(-) > >diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c >b/drivers/gpu/drm/bridge/synopsys/dw-dp.c >index 60feb3d1e14b..d7945f7fe9f0 100644 >--- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c >+++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c >@@ -1827,16 +1827,22 @@ static int dw_dp_bridge_attach(struct drm_bridge >*bridge, > dp->aux.transfer = dw_dp_aux_transfer; > > ret = drm_dp_aux_register(&dp->aux); >- if (ret) >+ if (ret) { > dev_err(dev, "Aux register failed: %d\n", ret); >+ return ret; >+ } > >- return ret; >+ enable_irq(dp->irq); >+ >+ return 0; > } > > static void dw_dp_bridge_detach(struct drm_bridge *bridge) > { > struct dw_dp *dp = bridge_to_dp(bridge); > >+ disable_irq(dp->irq); >+ cancel_work_sync(&dp->hpd_work); > drm_dp_aux_unregister(&dp->aux); > } > >@@ -1982,6 +1988,18 @@ static const struct regmap_config dw_dp_regmap_config = >{ > .rd_table = &dw_dp_readable_table, > }; > >+int dw_dp_bind(struct dw_dp *dp, struct drm_encoder *encoder) >+{ >+ return drm_bridge_attach(encoder, &dp->bridge, NULL, >DRM_BRIDGE_ATTACH_NO_CONNECTOR); >+} >+EXPORT_SYMBOL_GPL(dw_dp_bind); >+ >+void dw_dp_unbind(struct dw_dp *dp) >+{ >+ /* nothing to do as bridge is detached automatically */ >+} >+EXPORT_SYMBOL_GPL(dw_dp_unbind); >+ > static void dw_dp_phy_exit(void *data) > { > struct dw_dp *dp = data; >@@ -1989,13 +2007,12 @@ static void dw_dp_phy_exit(void *data) > phy_exit(dp->phy); > } > >-struct dw_dp *dw_dp_bind(struct device *dev, struct drm_encoder *encoder, >- const struct dw_dp_plat_data *plat_data) >+struct dw_dp *dw_dp_probe(struct platform_device *pdev, const struct >dw_dp_plat_data *plat_data) > { >- struct platform_device *pdev = to_platform_device(dev); >- struct dw_dp *dp; >+ struct device *dev = &pdev->dev; > struct drm_bridge *bridge; > void __iomem *res; >+ struct dw_dp *dp; > int ret; > > dp = devm_drm_bridge_alloc(dev, struct dw_dp, bridge, > &dw_dp_bridge_funcs); >@@ -2004,9 +2021,8 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct >drm_encoder *encoder, > > dp->dev = dev; > dp->pixel_mode = plat_data->pixel_mode; >- > dp->plat_data.max_link_rate = plat_data->max_link_rate; >- bridge = &dp->bridge; >+ > mutex_init(&dp->irq_lock); > INIT_WORK(&dp->hpd_work, dw_dp_hpd_work); > init_completion(&dp->complete); >@@ -2063,18 +2079,14 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct >drm_encoder *encoder, > return ERR_CAST(dp->rstc); > } > >- bridge->of_node = dev->of_node; >- bridge->ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | >DRM_BRIDGE_OP_HPD; >- bridge->type = DRM_MODE_CONNECTOR_DisplayPort; >- bridge->ycbcr_420_allowed = true; >- >- ret = devm_drm_bridge_add(dev, bridge); >- if (ret) >- return ERR_PTR(ret); >+ dp->irq = platform_get_irq(pdev, 0); >+ if (dp->irq < 0) >+ return ERR_PTR(dp->irq); > >- ret = drm_bridge_attach(encoder, bridge, NULL, >DRM_BRIDGE_ATTACH_NO_CONNECTOR); >+ ret = devm_request_threaded_irq(dev, dp->irq, NULL, dw_dp_irq, >+ IRQF_ONESHOT | IRQF_NO_AUTOEN, >dev_name(dev), dp); > if (ret) { >- dev_err_probe(dev, ret, "Failed to attach bridge\n"); >+ dev_err_probe(dev, ret, "failed to request irq\n"); > return ERR_PTR(ret); > } > >@@ -2090,28 +2102,19 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct >drm_encoder *encoder, > if (ret) > return ERR_PTR(ret); > >- dp->irq = platform_get_irq(pdev, 0); >- if (dp->irq < 0) { >- ret = dp->irq; >- return ERR_PTR(ret); >- } >+ bridge = &dp->bridge; >+ bridge->of_node = dev->of_node; >+ bridge->ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | >DRM_BRIDGE_OP_HPD; >+ bridge->type = DRM_MODE_CONNECTOR_DisplayPort; >+ bridge->ycbcr_420_allowed = true; > >- ret = devm_request_threaded_irq(dev, dp->irq, NULL, dw_dp_irq, >- IRQF_ONESHOT, dev_name(dev), dp); >- if (ret) { >- dev_err_probe(dev, ret, "failed to request irq\n"); >+ ret = devm_drm_bridge_add(dev, bridge); >+ if (ret) > return ERR_PTR(ret); >- } > > return dp; > } >-EXPORT_SYMBOL_GPL(dw_dp_bind); >- >-void dw_dp_unbind(struct dw_dp *dp) >-{ >- /* nothing to do */ >-} >-EXPORT_SYMBOL_GPL(dw_dp_unbind); >+EXPORT_SYMBOL_GPL(dw_dp_probe); > > MODULE_AUTHOR("Andy Yan <[email protected]>"); > MODULE_DESCRIPTION("DW DP Core Library"); >diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c >b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c >index b23efb153c9e..38e8fe75718e 100644 >--- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c >+++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c >@@ -26,7 +26,7 @@ > struct rockchip_dw_dp { > struct dw_dp *base; > struct device *dev; >- struct rockchip_encoder encoder; >+ struct rockchip_encoder *encoder; > }; > > static int dw_dp_encoder_atomic_check(struct drm_encoder *encoder, >@@ -73,37 +73,28 @@ static const struct drm_encoder_helper_funcs >dw_dp_encoder_helper_funcs = { > > static int dw_dp_rockchip_bind(struct device *dev, struct device *master, > void *data) > { >- struct platform_device *pdev = to_platform_device(dev); >- const struct dw_dp_plat_data *plat_data; >+ struct rockchip_dw_dp *dp = dev_get_drvdata(dev); > struct drm_device *drm_dev = data; >- struct rockchip_dw_dp *dp; > struct drm_encoder *encoder; > struct drm_connector *connector; > int ret; > >- dp = drmm_kzalloc(drm_dev, sizeof(*dp), GFP_KERNEL); >- if (!dp) >+ dp->encoder = drmm_kzalloc(drm_dev, sizeof(*dp->encoder), GFP_KERNEL); >+ if (!dp->encoder) > return -ENOMEM; > >- dp->dev = dev; >- platform_set_drvdata(pdev, dp); >- >- plat_data = of_device_get_match_data(dev); >- if (!plat_data) >- return -ENODEV; >- >- encoder = &dp->encoder.encoder; >+ encoder = &dp->encoder->encoder; > encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev, > dev->of_node); >- rockchip_drm_encoder_set_crtc_endpoint_id(&dp->encoder, dev->of_node, >0, 0); >+ rockchip_drm_encoder_set_crtc_endpoint_id(dp->encoder, dev->of_node, 0, >0); > > ret = drmm_encoder_init(drm_dev, encoder, NULL, DRM_MODE_ENCODER_TMDS, > NULL); > if (ret) > return ret; > drm_encoder_helper_add(encoder, &dw_dp_encoder_helper_funcs); > >- dp->base = dw_dp_bind(dev, encoder, plat_data); >- if (IS_ERR(dp->base)) >- return PTR_ERR(dp->base); >+ ret = dw_dp_bind(dp->base, encoder); >+ if (ret) >+ return dev_err_probe(dev, ret, "failed to bind DW-DP bridge\n"); > > connector = drm_bridge_connector_init(drm_dev, encoder); > if (IS_ERR(connector)) { >@@ -128,12 +119,30 @@ static const struct component_ops >dw_dp_rockchip_component_ops = { > .unbind = dw_dp_rockchip_unbind, > }; > >-static int dw_dp_probe(struct platform_device *pdev) >+static int dw_dp_rockchip_probe(struct platform_device *pdev) > { >+ const struct dw_dp_plat_data *plat_data; >+ struct device *dev = &pdev->dev; >+ struct rockchip_dw_dp *dp; >+ >+ plat_data = of_device_get_match_data(dev); >+ if (!plat_data) >+ return -ENODEV; >+ >+ dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL); >+ if (!dp) >+ return -ENOMEM; >+ platform_set_drvdata(pdev, dp); >+ dp->dev = dev; >+ >+ dp->base = dw_dp_probe(pdev, plat_data); >+ if (IS_ERR(dp->base)) >+ return PTR_ERR(dp->base); >+ > return component_add(&pdev->dev, &dw_dp_rockchip_component_ops); > } > >-static void dw_dp_remove(struct platform_device *pdev) >+static void dw_dp_rockchip_remove(struct platform_device *pdev) > { > component_del(&pdev->dev, &dw_dp_rockchip_component_ops); > } >@@ -161,8 +170,8 @@ static const struct of_device_id dw_dp_of_match[] = { > MODULE_DEVICE_TABLE(of, dw_dp_of_match); > > struct platform_driver dw_dp_driver = { >- .probe = dw_dp_probe, >- .remove = dw_dp_remove, >+ .probe = dw_dp_rockchip_probe, >+ .remove = dw_dp_rockchip_remove, > .driver = { > .name = "dw-dp", > .of_match_table = dw_dp_of_match, >diff --git a/include/drm/bridge/dw_dp.h b/include/drm/bridge/dw_dp.h >index 22105c3e8e4d..a82412a9e769 100644 >--- a/include/drm/bridge/dw_dp.h >+++ b/include/drm/bridge/dw_dp.h >@@ -22,7 +22,8 @@ struct dw_dp_plat_data { > u8 pixel_mode; > }; > >-struct dw_dp *dw_dp_bind(struct device *dev, struct drm_encoder *encoder, >- const struct dw_dp_plat_data *plat_data); >+int dw_dp_bind(struct dw_dp *dp, struct drm_encoder *encoder); > void dw_dp_unbind(struct dw_dp *dp); >+ >+struct dw_dp *dw_dp_probe(struct platform_device *pdev, const struct >dw_dp_plat_data *plat_data); > #endif /* __DW_DP__ */ > >-- >2.53.0 >
