[PATCH 14/21] drm/omap: Don't call HPD registration operations recursively
Sebastian Reichel
sre at kernel.org
Mon Jun 11 22:17:07 UTC 2018
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 at ideasonboard.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180612/43b5a659/attachment.sig>
More information about the dri-devel
mailing list