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.pinch...@ideasonboard.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reic...@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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to