| Message ID | 20180606093650.26034-15-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Jun 06, 2018 at 12:36:43PM +0300, Laurent Pinchart wrote: > Instead of calling the hot-plug detection callback registration > operations (.register_hpd_cb() and .unregister_hpd_cb()) recursively > from the display device back to the first device that provides hot plug > detection support, iterate over the devices manually in the DRM > connector code. This moves the complexity to a single central location > and simplifies the logic in omap_dss_device drivers. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk> -- Sebastian > drivers/gpu/drm/omapdrm/displays/connector-dvi.c | 8 ++- > drivers/gpu/drm/omapdrm/displays/connector-hdmi.c | 67 ++++++++---------- > .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 3 +- > drivers/gpu/drm/omapdrm/omap_connector.c | 79 ++++++++++++++-------- > 4 files changed, 88 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > index f1674b3eee50..e9353e4cd297 100644 > --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c > @@ -372,8 +372,12 @@ static int dvic_probe(struct platform_device *pdev) > dssdev->type = OMAP_DISPLAY_TYPE_DVI; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(0); > - dssdev->ops_flags = ddata->hpd_gpio || ddata->i2c_adapter > - ? OMAP_DSS_DEVICE_OP_DETECT : 0; > + > + if (ddata->hpd_gpio) > + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT > + | OMAP_DSS_DEVICE_OP_HPD; > + else if (ddata->i2c_adapter) > + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT; > > omapdss_display_init(dssdev); > omapdss_device_register(dssdev); > diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > index 0d22d7004c98..8eae973474dd 100644 > --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c > @@ -153,62 +153,53 @@ static int hdmic_register_hpd_cb(struct omap_dss_device *dssdev, > void *cb_data) > { > struct panel_drv_data *ddata = to_panel_data(dssdev); > - struct omap_dss_device *src = dssdev->src; > > - if (ddata->hpd_gpio) { > - mutex_lock(&ddata->hpd_lock); > - ddata->hpd_cb = cb; > - ddata->hpd_cb_data = cb_data; > - mutex_unlock(&ddata->hpd_lock); > - return 0; > - } else if (src->ops->register_hpd_cb) { > - return src->ops->register_hpd_cb(src, cb, cb_data); > - } > + if (!ddata->hpd_gpio) > + return -ENOTSUPP; > > - return -ENOTSUPP; > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_cb = cb; > + ddata->hpd_cb_data = cb_data; > + mutex_unlock(&ddata->hpd_lock); > + > + return 0; > } > > static void hdmic_unregister_hpd_cb(struct omap_dss_device *dssdev) > { > struct panel_drv_data *ddata = to_panel_data(dssdev); > - struct omap_dss_device *src = dssdev->src; > > - if (ddata->hpd_gpio) { > - mutex_lock(&ddata->hpd_lock); > - ddata->hpd_cb = NULL; > - ddata->hpd_cb_data = NULL; > - mutex_unlock(&ddata->hpd_lock); > - } else if (src->ops->unregister_hpd_cb) { > - src->ops->unregister_hpd_cb(src); > - } > + if (!ddata->hpd_gpio) > + return; > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_cb = NULL; > + ddata->hpd_cb_data = NULL; > + mutex_unlock(&ddata->hpd_lock); > } > > static void hdmic_enable_hpd(struct omap_dss_device *dssdev) > { > struct panel_drv_data *ddata = to_panel_data(dssdev); > - struct omap_dss_device *src = dssdev->src; > > - if (ddata->hpd_gpio) { > - mutex_lock(&ddata->hpd_lock); > - ddata->hpd_enabled = true; > - mutex_unlock(&ddata->hpd_lock); > - } else if (src->ops->enable_hpd) { > - src->ops->enable_hpd(src); > - } > + if (!ddata->hpd_gpio) > + return; > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_enabled = true; > + mutex_unlock(&ddata->hpd_lock); > } > > static void hdmic_disable_hpd(struct omap_dss_device *dssdev) > { > struct panel_drv_data *ddata = to_panel_data(dssdev); > - struct omap_dss_device *src = dssdev->src; > > - if (ddata->hpd_gpio) { > - mutex_lock(&ddata->hpd_lock); > - ddata->hpd_enabled = false; > - mutex_unlock(&ddata->hpd_lock); > - } else if (src->ops->disable_hpd) { > - src->ops->disable_hpd(src); > - } > + if (!ddata->hpd_gpio) > + return; > + > + mutex_lock(&ddata->hpd_lock); > + ddata->hpd_enabled = false; > + mutex_unlock(&ddata->hpd_lock); > } > > static int hdmic_set_hdmi_mode(struct omap_dss_device *dssdev, bool hdmi_mode) > @@ -314,7 +305,9 @@ static int hdmic_probe(struct platform_device *pdev) > dssdev->type = OMAP_DISPLAY_TYPE_HDMI; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(0); > - dssdev->ops_flags = ddata->hpd_gpio ? OMAP_DSS_DEVICE_OP_DETECT : 0; > + dssdev->ops_flags = ddata->hpd_gpio > + ? OMAP_DSS_DEVICE_OP_DETECT | OMAP_DSS_DEVICE_OP_HPD > + : 0; > > omapdss_display_init(dssdev); > omapdss_device_register(dssdev); > diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > index f37878ca6077..e5a25baa0364 100644 > --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c > @@ -289,7 +289,8 @@ static int tpd_probe(struct platform_device *pdev) > dssdev->output_type = OMAP_DISPLAY_TYPE_HDMI; > dssdev->owner = THIS_MODULE; > dssdev->of_ports = BIT(1) | BIT(0); > - dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT; > + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT > + | OMAP_DSS_DEVICE_OP_HPD; > > dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1); > if (IS_ERR(dssdev->next)) { > diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c > index 6c02c2492d47..578b0b105755 100644 > --- a/drivers/gpu/drm/omapdrm/omap_connector.c > +++ b/drivers/gpu/drm/omapdrm/omap_connector.c > @@ -57,6 +57,21 @@ bool omap_connector_get_hdmi_mode(struct drm_connector *connector) > return omap_connector->hdmi_mode; > } > > +static struct omap_dss_device * > +omap_connector_find_device(struct drm_connector *connector, > + enum omap_dss_device_ops_flag op) > +{ > + struct omap_connector *omap_connector = to_omap_connector(connector); > + struct omap_dss_device *dssdev; > + > + for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) { > + if (dssdev->ops_flags & op) > + return dssdev; > + } > + > + return NULL; > +} > + > static enum drm_connector_status omap_connector_detect( > struct drm_connector *connector, bool force) > { > @@ -64,10 +79,8 @@ static enum drm_connector_status omap_connector_detect( > struct omap_dss_device *dssdev; > enum drm_connector_status status; > > - for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) { > - if (dssdev->ops_flags & OMAP_DSS_DEVICE_OP_DETECT) > - break; > - } > + dssdev = omap_connector_find_device(connector, > + OMAP_DSS_DEVICE_OP_DETECT); > > if (dssdev) { > if (dssdev->ops->detect(dssdev)) > @@ -96,18 +109,21 @@ static enum drm_connector_status omap_connector_detect( > static void omap_connector_destroy(struct drm_connector *connector) > { > struct omap_connector *omap_connector = to_omap_connector(connector); > - struct omap_dss_device *dssdev = omap_connector->dssdev; > + struct omap_dss_device *dssdev; > > DBG("%s", omap_connector->dssdev->name); > - if (connector->polled == DRM_CONNECTOR_POLL_HPD && > - dssdev->ops->unregister_hpd_cb) { > + > + if (connector->polled == DRM_CONNECTOR_POLL_HPD) { > + dssdev = omap_connector_find_device(connector, > + OMAP_DSS_DEVICE_OP_HPD); > dssdev->ops->unregister_hpd_cb(dssdev); > } > + > drm_connector_unregister(connector); > drm_connector_cleanup(connector); > kfree(omap_connector); > > - omapdss_device_put(dssdev); > + omapdss_device_put(omap_connector->dssdev); > } > > #define MAX_EDID 512 > @@ -258,45 +274,50 @@ struct drm_connector *omap_connector_init(struct drm_device *dev, > { > struct drm_connector *connector = NULL; > struct omap_connector *omap_connector; > - bool hpd_supported = false; > > DBG("%s", dssdev->name); > > - omapdss_device_get(dssdev); > - > omap_connector = kzalloc(sizeof(*omap_connector), GFP_KERNEL); > if (!omap_connector) > goto fail; > > - omap_connector->dssdev = dssdev; > + omap_connector->dssdev = omapdss_device_get(dssdev); > > connector = &omap_connector->base; > + connector->interlace_allowed = 1; > + connector->doublescan_allowed = 0; > > drm_connector_init(dev, connector, &omap_connector_funcs, > connector_type); > drm_connector_helper_add(connector, &omap_connector_helper_funcs); > > - if (dssdev->ops->register_hpd_cb) { > - int ret = dssdev->ops->register_hpd_cb(dssdev, > - omap_connector_hpd_cb, > - omap_connector); > - if (!ret) > - hpd_supported = true; > - else if (ret != -ENOTSUPP) > + /* > + * Initialize connector status handling. First try to find a device that > + * supports hot-plug reporting. If it fails, fall back to a device that > + * support polling. If that fails too, we don't support hot-plug > + * detection at all. > + */ > + dssdev = omap_connector_find_device(connector, OMAP_DSS_DEVICE_OP_HPD); > + if (dssdev) { > + int ret; > + > + ret = dssdev->ops->register_hpd_cb(dssdev, > + omap_connector_hpd_cb, > + omap_connector); > + if (ret < 0) > DBG("%s: Failed to register HPD callback (%d).", > dssdev->name, ret); > + else > + connector->polled = DRM_CONNECTOR_POLL_HPD; > } > > - if (hpd_supported) > - connector->polled = DRM_CONNECTOR_POLL_HPD; > - else if (dssdev->ops->detect) > - connector->polled = DRM_CONNECTOR_POLL_CONNECT | > - DRM_CONNECTOR_POLL_DISCONNECT; > - else > - connector->polled = 0; > - > - connector->interlace_allowed = 1; > - connector->doublescan_allowed = 0; > + if (!connector->polled) { > + dssdev = omap_connector_find_device(connector, > + OMAP_DSS_DEVICE_OP_DETECT); > + if (dssdev) > + connector->polled = DRM_CONNECTOR_POLL_CONNECT | > + DRM_CONNECTOR_POLL_DISCONNECT; > + } > > return connector; > > -- > 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/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c index f1674b3eee50..e9353e4cd297 100644 --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c @@ -372,8 +372,12 @@ static int dvic_probe(struct platform_device *pdev) dssdev->type = OMAP_DISPLAY_TYPE_DVI; dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(0); - dssdev->ops_flags = ddata->hpd_gpio || ddata->i2c_adapter - ? OMAP_DSS_DEVICE_OP_DETECT : 0; + + if (ddata->hpd_gpio) + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT + | OMAP_DSS_DEVICE_OP_HPD; + else if (ddata->i2c_adapter) + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c index 0d22d7004c98..8eae973474dd 100644 --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c @@ -153,62 +153,53 @@ static int hdmic_register_hpd_cb(struct omap_dss_device *dssdev, void *cb_data) { struct panel_drv_data *ddata = to_panel_data(dssdev); - struct omap_dss_device *src = dssdev->src; - if (ddata->hpd_gpio) { - mutex_lock(&ddata->hpd_lock); - ddata->hpd_cb = cb; - ddata->hpd_cb_data = cb_data; - mutex_unlock(&ddata->hpd_lock); - return 0; - } else if (src->ops->register_hpd_cb) { - return src->ops->register_hpd_cb(src, cb, cb_data); - } + if (!ddata->hpd_gpio) + return -ENOTSUPP; - return -ENOTSUPP; + mutex_lock(&ddata->hpd_lock); + ddata->hpd_cb = cb; + ddata->hpd_cb_data = cb_data; + mutex_unlock(&ddata->hpd_lock); + + return 0; } static void hdmic_unregister_hpd_cb(struct omap_dss_device *dssdev) { struct panel_drv_data *ddata = to_panel_data(dssdev); - struct omap_dss_device *src = dssdev->src; - if (ddata->hpd_gpio) { - mutex_lock(&ddata->hpd_lock); - ddata->hpd_cb = NULL; - ddata->hpd_cb_data = NULL; - mutex_unlock(&ddata->hpd_lock); - } else if (src->ops->unregister_hpd_cb) { - src->ops->unregister_hpd_cb(src); - } + if (!ddata->hpd_gpio) + return; + + mutex_lock(&ddata->hpd_lock); + ddata->hpd_cb = NULL; + ddata->hpd_cb_data = NULL; + mutex_unlock(&ddata->hpd_lock); } static void hdmic_enable_hpd(struct omap_dss_device *dssdev) { struct panel_drv_data *ddata = to_panel_data(dssdev); - struct omap_dss_device *src = dssdev->src; - if (ddata->hpd_gpio) { - mutex_lock(&ddata->hpd_lock); - ddata->hpd_enabled = true; - mutex_unlock(&ddata->hpd_lock); - } else if (src->ops->enable_hpd) { - src->ops->enable_hpd(src); - } + if (!ddata->hpd_gpio) + return; + + mutex_lock(&ddata->hpd_lock); + ddata->hpd_enabled = true; + mutex_unlock(&ddata->hpd_lock); } static void hdmic_disable_hpd(struct omap_dss_device *dssdev) { struct panel_drv_data *ddata = to_panel_data(dssdev); - struct omap_dss_device *src = dssdev->src; - if (ddata->hpd_gpio) { - mutex_lock(&ddata->hpd_lock); - ddata->hpd_enabled = false; - mutex_unlock(&ddata->hpd_lock); - } else if (src->ops->disable_hpd) { - src->ops->disable_hpd(src); - } + if (!ddata->hpd_gpio) + return; + + mutex_lock(&ddata->hpd_lock); + ddata->hpd_enabled = false; + mutex_unlock(&ddata->hpd_lock); } static int hdmic_set_hdmi_mode(struct omap_dss_device *dssdev, bool hdmi_mode) @@ -314,7 +305,9 @@ static int hdmic_probe(struct platform_device *pdev) dssdev->type = OMAP_DISPLAY_TYPE_HDMI; dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(0); - dssdev->ops_flags = ddata->hpd_gpio ? OMAP_DSS_DEVICE_OP_DETECT : 0; + dssdev->ops_flags = ddata->hpd_gpio + ? OMAP_DSS_DEVICE_OP_DETECT | OMAP_DSS_DEVICE_OP_HPD + : 0; omapdss_display_init(dssdev); omapdss_device_register(dssdev); diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c index f37878ca6077..e5a25baa0364 100644 --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c @@ -289,7 +289,8 @@ static int tpd_probe(struct platform_device *pdev) dssdev->output_type = OMAP_DISPLAY_TYPE_HDMI; dssdev->owner = THIS_MODULE; dssdev->of_ports = BIT(1) | BIT(0); - dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT; + dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT + | OMAP_DSS_DEVICE_OP_HPD; dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1); if (IS_ERR(dssdev->next)) { diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c index 6c02c2492d47..578b0b105755 100644 --- a/drivers/gpu/drm/omapdrm/omap_connector.c +++ b/drivers/gpu/drm/omapdrm/omap_connector.c @@ -57,6 +57,21 @@ bool omap_connector_get_hdmi_mode(struct drm_connector *connector) return omap_connector->hdmi_mode; } +static struct omap_dss_device * +omap_connector_find_device(struct drm_connector *connector, + enum omap_dss_device_ops_flag op) +{ + struct omap_connector *omap_connector = to_omap_connector(connector); + struct omap_dss_device *dssdev; + + for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) { + if (dssdev->ops_flags & op) + return dssdev; + } + + return NULL; +} + static enum drm_connector_status omap_connector_detect( struct drm_connector *connector, bool force) { @@ -64,10 +79,8 @@ static enum drm_connector_status omap_connector_detect( struct omap_dss_device *dssdev; enum drm_connector_status status; - for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) { - if (dssdev->ops_flags & OMAP_DSS_DEVICE_OP_DETECT) - break; - } + dssdev = omap_connector_find_device(connector, + OMAP_DSS_DEVICE_OP_DETECT); if (dssdev) { if (dssdev->ops->detect(dssdev)) @@ -96,18 +109,21 @@ static enum drm_connector_status omap_connector_detect( static void omap_connector_destroy(struct drm_connector *connector) { struct omap_connector *omap_connector = to_omap_connector(connector); - struct omap_dss_device *dssdev = omap_connector->dssdev; + struct omap_dss_device *dssdev; DBG("%s", omap_connector->dssdev->name); - if (connector->polled == DRM_CONNECTOR_POLL_HPD && - dssdev->ops->unregister_hpd_cb) { + + if (connector->polled == DRM_CONNECTOR_POLL_HPD) { + dssdev = omap_connector_find_device(connector, + OMAP_DSS_DEVICE_OP_HPD); dssdev->ops->unregister_hpd_cb(dssdev); } + drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(omap_connector); - omapdss_device_put(dssdev); + omapdss_device_put(omap_connector->dssdev); } #define MAX_EDID 512 @@ -258,45 +274,50 @@ struct drm_connector *omap_connector_init(struct drm_device *dev, { struct drm_connector *connector = NULL; struct omap_connector *omap_connector; - bool hpd_supported = false; DBG("%s", dssdev->name); - omapdss_device_get(dssdev); - omap_connector = kzalloc(sizeof(*omap_connector), GFP_KERNEL); if (!omap_connector) goto fail; - omap_connector->dssdev = dssdev; + omap_connector->dssdev = omapdss_device_get(dssdev); connector = &omap_connector->base; + connector->interlace_allowed = 1; + connector->doublescan_allowed = 0; drm_connector_init(dev, connector, &omap_connector_funcs, connector_type); drm_connector_helper_add(connector, &omap_connector_helper_funcs); - if (dssdev->ops->register_hpd_cb) { - int ret = dssdev->ops->register_hpd_cb(dssdev, - omap_connector_hpd_cb, - omap_connector); - if (!ret) - hpd_supported = true; - else if (ret != -ENOTSUPP) + /* + * Initialize connector status handling. First try to find a device that + * supports hot-plug reporting. If it fails, fall back to a device that + * support polling. If that fails too, we don't support hot-plug + * detection at all. + */ + dssdev = omap_connector_find_device(connector, OMAP_DSS_DEVICE_OP_HPD); + if (dssdev) { + int ret; + + ret = dssdev->ops->register_hpd_cb(dssdev, + omap_connector_hpd_cb, + omap_connector); + if (ret < 0) DBG("%s: Failed to register HPD callback (%d).", dssdev->name, ret); + else + connector->polled = DRM_CONNECTOR_POLL_HPD; } - if (hpd_supported) - connector->polled = DRM_CONNECTOR_POLL_HPD; - else if (dssdev->ops->detect) - connector->polled = DRM_CONNECTOR_POLL_CONNECT | - DRM_CONNECTOR_POLL_DISCONNECT; - else - connector->polled = 0; - - connector->interlace_allowed = 1; - connector->doublescan_allowed = 0; + if (!connector->polled) { + dssdev = omap_connector_find_device(connector, + OMAP_DSS_DEVICE_OP_DETECT); + if (dssdev) + connector->polled = DRM_CONNECTOR_POLL_CONNECT | + DRM_CONNECTOR_POLL_DISCONNECT; + } return connector;
Instead of calling the hot-plug detection callback registration operations (.register_hpd_cb() and .unregister_hpd_cb()) recursively from the display device back to the first device that provides hot plug detection support, iterate over the devices manually in the DRM connector code. This moves the complexity to a single central location and simplifies the logic in omap_dss_device drivers. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/omapdrm/displays/connector-dvi.c | 8 ++- drivers/gpu/drm/omapdrm/displays/connector-hdmi.c | 67 ++++++++---------- .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 3 +- drivers/gpu/drm/omapdrm/omap_connector.c | 79 ++++++++++++++-------- 4 files changed, 88 insertions(+), 69 deletions(-)