| Message ID | 20200409004610.12346-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: bridge: adv7511: Enable usage with DRM bridge connector helper | expand |
Hi Laurent. On Thu, Apr 09, 2020 at 03:46:08AM +0300, Laurent Pinchart wrote: > To prepare for making the connector creation optional, move the related > code out of adv7511_bridge_attach() to a separate function. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> On nit below, but otherwise: Acked-by: Sam Ravnborg <sam@ravnborg.org> Sam > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++------- > 1 file changed, 40 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 58d02e92b6b9..e3b62ad95389 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511, > adv7511->f_tmds = mode->clock; > } > > -/* Connector funcs */ > +/* ----------------------------------------------------------------------------- > + * DRM Connector Operations > + */ > + > static struct adv7511 *connector_to_adv7511(struct drm_connector *connector) > { > return container_of(connector, struct adv7511, connector); > @@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > -/* Bridge funcs */ > +static int adv7511_connector_init(struct adv7511 *adv) > +{ > + struct drm_bridge *bridge = &adv->bridge; > + int ret; > + > + if (!bridge->encoder) { > + DRM_ERROR("Parent encoder object not found"); > + return -ENODEV; > + } > + > + if (adv->i2c_main->irq) > + adv->connector.polled = DRM_CONNECTOR_POLL_HPD; > + else > + adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > + DRM_CONNECTOR_POLL_DISCONNECT; > + > + ret = drm_connector_init(bridge->dev, &adv->connector, > + &adv7511_connector_funcs, > + DRM_MODE_CONNECTOR_HDMIA); > + if (ret) { Here we test for ret != 0 > + DRM_ERROR("Failed to initialize connector with drm\n"); > + return ret; > + } > + drm_connector_helper_add(&adv->connector, > + &adv7511_connector_helper_funcs); > + drm_connector_attach_encoder(&adv->connector, bridge->encoder); > + > + return 0; > +} > + > +/* ----------------------------------------------------------------------------- > + * DRM Bridge Operations > + */ > + > static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge) > { > return container_of(bridge, struct adv7511, bridge); > @@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge, > return -EINVAL; > } > > - if (!bridge->encoder) { > - DRM_ERROR("Parent encoder object not found"); > - return -ENODEV; > - } > - > - if (adv->i2c_main->irq) > - adv->connector.polled = DRM_CONNECTOR_POLL_HPD; > - else > - adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT | > - DRM_CONNECTOR_POLL_DISCONNECT; > - > - ret = drm_connector_init(bridge->dev, &adv->connector, > - &adv7511_connector_funcs, > - DRM_MODE_CONNECTOR_HDMIA); > - if (ret) { > - DRM_ERROR("Failed to initialize connector with drm\n"); > + ret = adv7511_connector_init(adv); > + if (ret < 0) Here we test for ret < 0 The code works - but it is inconsistent. drm_connector_init() is documented to: "Zero on success, error code on failure." > return ret; > - } > - drm_connector_helper_add(&adv->connector, > - &adv7511_connector_helper_funcs); > - drm_connector_attach_encoder(&adv->connector, bridge->encoder); > > if (adv->type == ADV7533 || adv->type == ADV7535) > ret = adv7533_attach_dsi(adv); > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 58d02e92b6b9..e3b62ad95389 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -783,7 +783,10 @@ static void adv7511_mode_set(struct adv7511 *adv7511, adv7511->f_tmds = mode->clock; } -/* Connector funcs */ +/* ----------------------------------------------------------------------------- + * DRM Connector Operations + */ + static struct adv7511 *connector_to_adv7511(struct drm_connector *connector) { return container_of(connector, struct adv7511, connector); @@ -827,7 +830,40 @@ static const struct drm_connector_funcs adv7511_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -/* Bridge funcs */ +static int adv7511_connector_init(struct adv7511 *adv) +{ + struct drm_bridge *bridge = &adv->bridge; + int ret; + + if (!bridge->encoder) { + DRM_ERROR("Parent encoder object not found"); + return -ENODEV; + } + + if (adv->i2c_main->irq) + adv->connector.polled = DRM_CONNECTOR_POLL_HPD; + else + adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT | + DRM_CONNECTOR_POLL_DISCONNECT; + + ret = drm_connector_init(bridge->dev, &adv->connector, + &adv7511_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA); + if (ret) { + DRM_ERROR("Failed to initialize connector with drm\n"); + return ret; + } + drm_connector_helper_add(&adv->connector, + &adv7511_connector_helper_funcs); + drm_connector_attach_encoder(&adv->connector, bridge->encoder); + + return 0; +} + +/* ----------------------------------------------------------------------------- + * DRM Bridge Operations + */ + static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge) { return container_of(bridge, struct adv7511, bridge); @@ -867,27 +903,9 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge, return -EINVAL; } - if (!bridge->encoder) { - DRM_ERROR("Parent encoder object not found"); - return -ENODEV; - } - - if (adv->i2c_main->irq) - adv->connector.polled = DRM_CONNECTOR_POLL_HPD; - else - adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT | - DRM_CONNECTOR_POLL_DISCONNECT; - - ret = drm_connector_init(bridge->dev, &adv->connector, - &adv7511_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA); - if (ret) { - DRM_ERROR("Failed to initialize connector with drm\n"); + ret = adv7511_connector_init(adv); + if (ret < 0) return ret; - } - drm_connector_helper_add(&adv->connector, - &adv7511_connector_helper_funcs); - drm_connector_attach_encoder(&adv->connector, bridge->encoder); if (adv->type == ADV7533 || adv->type == ADV7535) ret = adv7533_attach_dsi(adv);
To prepare for making the connector creation optional, move the related code out of adv7511_bridge_attach() to a separate function. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 62 +++++++++++++------- 1 file changed, 40 insertions(+), 22 deletions(-)