Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:09PM +0200, Sam Ravnborg wrote: > Factor out connector creation to a small helper function. > > Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx> > Cc: Peter Senna Tschudin <peter.senna@xxxxxxxxx> > Cc: Martin Donnelly <martin.donnelly@xxxxxx> > Cc: Martyn Welch <martyn.welch@xxxxxxxxxxxxxxx> > Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > Cc: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx> > Cc: Jonas Karlman <jonas@xxxxxxxxx> > Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxx> > --- > .../bridge/megachips-stdpxxxx-ge-b850v3-fw.c | 47 +++++++++++-------- > 1 file changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c > index 6200f12a37e6..258e0525cdcc 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c > @@ -191,6 +191,32 @@ static const struct drm_connector_funcs ge_b850v3_lvds_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > +static int ge_b850v3_lvds_create_connector(struct drm_bridge *bridge) > +{ > + struct drm_connector *connector = &ge_b850v3_lvds_ptr->connector; Wow, storing device state in a global variable... :-( How did this go past review ? Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > + int ret; > + > + if (!bridge->encoder) { > + DRM_ERROR("Parent encoder object not found"); > + return -ENODEV; > + } > + > + connector->polled = DRM_CONNECTOR_POLL_HPD; > + > + drm_connector_helper_add(connector, > + &ge_b850v3_lvds_connector_helper_funcs); > + > + ret = drm_connector_init(bridge->dev, connector, > + &ge_b850v3_lvds_connector_funcs, > + DRM_MODE_CONNECTOR_DisplayPort); > + if (ret) { > + DRM_ERROR("Failed to initialize connector with drm\n"); > + return ret; > + } > + > + return drm_connector_attach_encoder(connector, bridge->encoder); > +} > + > static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, void *dev_id) > { > struct i2c_client *stdp4028_i2c > @@ -209,7 +235,6 @@ static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, void *dev_id) > static int ge_b850v3_lvds_attach(struct drm_bridge *bridge, > enum drm_bridge_attach_flags flags) > { > - struct drm_connector *connector = &ge_b850v3_lvds_ptr->connector; > struct i2c_client *stdp4028_i2c > = ge_b850v3_lvds_ptr->stdp4028_i2c; > int ret; > @@ -219,25 +244,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge *bridge, > return -EINVAL; > } > > - if (!bridge->encoder) { > - DRM_ERROR("Parent encoder object not found"); > - return -ENODEV; > - } > - > - connector->polled = DRM_CONNECTOR_POLL_HPD; > - > - drm_connector_helper_add(connector, > - &ge_b850v3_lvds_connector_helper_funcs); > - > - ret = drm_connector_init(bridge->dev, connector, > - &ge_b850v3_lvds_connector_funcs, > - DRM_MODE_CONNECTOR_DisplayPort); > - if (ret) { > - DRM_ERROR("Failed to initialize connector with drm\n"); > - return ret; > - } > - > - ret = drm_connector_attach_encoder(connector, bridge->encoder); > + ret = ge_b850v3_lvds_create_connector(bridge); > if (ret) > return ret; > -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel