| Message ID | 20171114191647.22207-1-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Tue, 14 Nov 2017 11:16:47 -0800 Eric Anholt wrote: > The panel_bridge bridge attaches to the panel's OF node, not the > lvds-encoder's node. Put in a little no-op bridge of our own so that > our consumers can still find a bridge where they expect. > > This also fixes an unintended unregistration and leak of the > panel-bridge on module remove. > > Signed-off-by: Eric Anholt <eric@anholt.net> > Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bri > dge.") > --- > > Note: I haven't actually tested this patch! Hope it helps, though. > > drivers/gpu/drm/bridge/lvds-encoder.c | 48 ++++++++++++++++++++++++++++++----- > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c > index 0903ba574f61..75b0d3f6e4de 100644 > --- a/drivers/gpu/drm/bridge/lvds-encoder.c > +++ b/drivers/gpu/drm/bridge/lvds-encoder.c > @@ -13,13 +13,37 @@ > > #include <linux/of_graph.h> > > +struct lvds_encoder { > + struct drm_bridge bridge; > + struct drm_bridge *panel_bridge; > +}; > + > +static int lvds_encoder_attach(struct drm_bridge *bridge) > +{ > + struct lvds_encoder *lvds_encoder = container_of(bridge, > + struct lvds_encoder, > + bridge); > + > + return drm_bridge_attach(bridge->encoder, lvds_encoder->panel_bridge, > + bridge); > +} > + > +static struct drm_bridge_funcs funcs = { > + .attach = lvds_encoder_attach, > +}; > + > static int lvds_encoder_probe(struct platform_device *pdev) > { > struct device_node *port; > struct device_node *endpoint; > struct device_node *panel_node; > struct drm_panel *panel; > - struct drm_bridge *bridge; > + struct lvds_encoder *lvds_encoder; > + > + lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder), > + GFP_KERNEL); > + if (!lvds_encoder) > + return -ENOMEM; > > /* Locate the panel DT node. */ > port = of_graph_get_port_by_id(pdev->dev.of_node, 1); > @@ -49,20 +73,30 @@ static int lvds_encoder_probe(struct platform_device *pdev) > return -EPROBE_DEFER; > } > > - bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS); > - if (IS_ERR(bridge)) > - return PTR_ERR(bridge); > + lvds_encoder->panel_bridge = > + devm_drm_panel_bridge_add(&pdev->dev, > + panel, DRM_MODE_CONNECTOR_LVDS); > + if (IS_ERR(lvds_encoder->panel_bridge)) > + return PTR_ERR(lvds_encoder->panel_bridge); > + > + /* The panel_bridge bridge is attached to the panel's of_node, > + * but we need a bridge attached to our of_node for our user > + * to look up. > + */ > + lvds_encoder->bridge.of_node = pdev->dev.of_node; > + lvds_encoder->bridge.funcs = &funcs; > + drm_bridge_add(&lvds_encoder->bridge); > > - platform_set_drvdata(pdev, bridge); > + platform_set_drvdata(pdev, lvds_encoder); > > return 0; > } > > static int lvds_encoder_remove(struct platform_device *pdev) > { > - struct drm_bridge *bridge = platform_get_drvdata(pdev); > + struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev); > > - drm_bridge_remove(bridge); > + drm_bridge_remove(&lvds_encoder->bridge); > > return 0; > } > Tested-by: Lothar Waßmann <LW@KARO-electronics.de> Lothar Waßmann
On 11/15/2017 03:29 PM, Lothar Waßmann wrote: > Hi, > > On Tue, 14 Nov 2017 11:16:47 -0800 Eric Anholt wrote: >> The panel_bridge bridge attaches to the panel's OF node, not the >> lvds-encoder's node. Put in a little no-op bridge of our own so that >> our consumers can still find a bridge where they expect. >> >> This also fixes an unintended unregistration and leak of the >> panel-bridge on module remove. >> >> Signed-off-by: Eric Anholt <eric@anholt.net> >> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bri >> dge.") >> --- >> >> Note: I haven't actually tested this patch! Hope it helps, though. >> >> drivers/gpu/drm/bridge/lvds-encoder.c | 48 ++++++++++++++++++++++++++++++----- >> 1 file changed, 41 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c >> index 0903ba574f61..75b0d3f6e4de 100644 >> --- a/drivers/gpu/drm/bridge/lvds-encoder.c >> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c >> @@ -13,13 +13,37 @@ >> >> #include <linux/of_graph.h> >> >> +struct lvds_encoder { >> + struct drm_bridge bridge; >> + struct drm_bridge *panel_bridge; >> +}; >> + >> +static int lvds_encoder_attach(struct drm_bridge *bridge) >> +{ >> + struct lvds_encoder *lvds_encoder = container_of(bridge, >> + struct lvds_encoder, >> + bridge); >> + >> + return drm_bridge_attach(bridge->encoder, lvds_encoder->panel_bridge, >> + bridge); >> +} >> + >> +static struct drm_bridge_funcs funcs = { >> + .attach = lvds_encoder_attach, >> +}; >> + >> static int lvds_encoder_probe(struct platform_device *pdev) >> { >> struct device_node *port; >> struct device_node *endpoint; >> struct device_node *panel_node; >> struct drm_panel *panel; >> - struct drm_bridge *bridge; >> + struct lvds_encoder *lvds_encoder; >> + >> + lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder), >> + GFP_KERNEL); >> + if (!lvds_encoder) >> + return -ENOMEM; >> >> /* Locate the panel DT node. */ >> port = of_graph_get_port_by_id(pdev->dev.of_node, 1); >> @@ -49,20 +73,30 @@ static int lvds_encoder_probe(struct platform_device *pdev) >> return -EPROBE_DEFER; >> } >> >> - bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS); >> - if (IS_ERR(bridge)) >> - return PTR_ERR(bridge); >> + lvds_encoder->panel_bridge = >> + devm_drm_panel_bridge_add(&pdev->dev, >> + panel, DRM_MODE_CONNECTOR_LVDS); >> + if (IS_ERR(lvds_encoder->panel_bridge)) >> + return PTR_ERR(lvds_encoder->panel_bridge); >> + >> + /* The panel_bridge bridge is attached to the panel's of_node, >> + * but we need a bridge attached to our of_node for our user >> + * to look up. >> + */ >> + lvds_encoder->bridge.of_node = pdev->dev.of_node; >> + lvds_encoder->bridge.funcs = &funcs; >> + drm_bridge_add(&lvds_encoder->bridge); >> >> - platform_set_drvdata(pdev, bridge); >> + platform_set_drvdata(pdev, lvds_encoder); >> >> return 0; >> } >> >> static int lvds_encoder_remove(struct platform_device *pdev) >> { >> - struct drm_bridge *bridge = platform_get_drvdata(pdev); >> + struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev); >> >> - drm_bridge_remove(bridge); >> + drm_bridge_remove(&lvds_encoder->bridge); >> >> return 0; >> } >> > Tested-by: Lothar Waßmann <LW@KARO-electronics.de> queued to drm-misc-fixes. I think we should send this patch for the stable kernels (v4.12+) too. Thanks, Archit > > > Lothar Waßmann >
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index 0903ba574f61..75b0d3f6e4de 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -13,13 +13,37 @@ #include <linux/of_graph.h> +struct lvds_encoder { + struct drm_bridge bridge; + struct drm_bridge *panel_bridge; +}; + +static int lvds_encoder_attach(struct drm_bridge *bridge) +{ + struct lvds_encoder *lvds_encoder = container_of(bridge, + struct lvds_encoder, + bridge); + + return drm_bridge_attach(bridge->encoder, lvds_encoder->panel_bridge, + bridge); +} + +static struct drm_bridge_funcs funcs = { + .attach = lvds_encoder_attach, +}; + static int lvds_encoder_probe(struct platform_device *pdev) { struct device_node *port; struct device_node *endpoint; struct device_node *panel_node; struct drm_panel *panel; - struct drm_bridge *bridge; + struct lvds_encoder *lvds_encoder; + + lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder), + GFP_KERNEL); + if (!lvds_encoder) + return -ENOMEM; /* Locate the panel DT node. */ port = of_graph_get_port_by_id(pdev->dev.of_node, 1); @@ -49,20 +73,30 @@ static int lvds_encoder_probe(struct platform_device *pdev) return -EPROBE_DEFER; } - bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS); - if (IS_ERR(bridge)) - return PTR_ERR(bridge); + lvds_encoder->panel_bridge = + devm_drm_panel_bridge_add(&pdev->dev, + panel, DRM_MODE_CONNECTOR_LVDS); + if (IS_ERR(lvds_encoder->panel_bridge)) + return PTR_ERR(lvds_encoder->panel_bridge); + + /* The panel_bridge bridge is attached to the panel's of_node, + * but we need a bridge attached to our of_node for our user + * to look up. + */ + lvds_encoder->bridge.of_node = pdev->dev.of_node; + lvds_encoder->bridge.funcs = &funcs; + drm_bridge_add(&lvds_encoder->bridge); - platform_set_drvdata(pdev, bridge); + platform_set_drvdata(pdev, lvds_encoder); return 0; } static int lvds_encoder_remove(struct platform_device *pdev) { - struct drm_bridge *bridge = platform_get_drvdata(pdev); + struct lvds_encoder *lvds_encoder = platform_get_drvdata(pdev); - drm_bridge_remove(bridge); + drm_bridge_remove(&lvds_encoder->bridge); return 0; }
The panel_bridge bridge attaches to the panel's OF node, not the lvds-encoder's node. Put in a little no-op bridge of our own so that our consumers can still find a bridge where they expect. This also fixes an unintended unregistration and leak of the panel-bridge on module remove. Signed-off-by: Eric Anholt <eric@anholt.net> Fixes: 13dfc0540a57 ("drm/bridge: Refactor out the panel wrapper from the lvds-encoder bri dge.") --- Note: I haven't actually tested this patch! Hope it helps, though. drivers/gpu/drm/bridge/lvds-encoder.c | 48 ++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-)