Hi Peter,

Thank you for the patch.

On Monday 15 May 2017 12:03:11 Peter Ujfalusi wrote:
> If the hpd_gpio is valid, use interrupt handler to react to HPD changes.
> In case the hpd_gpio is not valid, try to enable hpd detection on the
> encoder if it supports it.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfal...@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/displays/connector-hdmi.c | 104 +++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c index
> 1ef130641bae..3a90f89ada77 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
> +#include <linux/spinlock.h>

Did you mean linux/mutex.h ?

> 
>  #include <drm/drm_edid.h>
>  #include <video/omap-panel-data.h>
> @@ -38,6 +39,10 @@ static const struct videomode hdmic_default_vm = {
>  struct panel_drv_data {
>       struct omap_dss_device dssdev;
>       struct omap_dss_device *in;
> +     void (*hpd_cb)(void *cb_data, enum drm_connector_status status);
> +     void *hpd_cb_data;
> +     bool hpd_enabled;
> +     struct mutex hpd_lock;
> 
>       struct device *dev;
> 
> @@ -168,6 +173,70 @@ static bool hdmic_detect(struct omap_dss_device
> *dssdev) return in->ops.hdmi->detect(in);
>  }
> 
> +static int hdmic_register_hpd_cb(struct omap_dss_device *dssdev,
> +                              void (*cb)(void *cb_data,
> +                                         enum drm_connector_status status),
> +                              void *cb_data)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +     struct omap_dss_device *in = ddata->in;
> +
> +     if (gpio_is_valid(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 (in->ops.hdmi->register_hpd_cb) {
> +             return in->ops.hdmi->register_hpd_cb(in, cb, cb_data);
> +     }
> +
> +     return -ENOTSUPP;
> +}
> +
> +static void hdmic_unregister_hpd_cb(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +     struct omap_dss_device *in = ddata->in;
> +
> +     if (gpio_is_valid(ddata->hpd_gpio)) {
> +             mutex_lock(&ddata->hpd_lock);
> +             ddata->hpd_cb = NULL;
> +             ddata->hpd_cb_data = NULL;
> +             mutex_unlock(&ddata->hpd_lock);
> +     } else if (in->ops.hdmi->unregister_hpd_cb) {
> +             in->ops.hdmi->unregister_hpd_cb(in);
> +     }
> +}
> +
> +static void hdmic_enable_hpd(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +     struct omap_dss_device *in = ddata->in;
> +
> +     if (gpio_is_valid(ddata->hpd_gpio)) {
> +             mutex_lock(&ddata->hpd_lock);
> +             ddata->hpd_enabled = true;
> +             mutex_unlock(&ddata->hpd_lock);
> +     } else if (in->ops.hdmi->enable_hpd) {
> +             in->ops.hdmi->enable_hpd(in);
> +     }
> +}
> +
> +static void hdmic_disable_hpd(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +     struct omap_dss_device *in = ddata->in;
> +
> +     if (gpio_is_valid(ddata->hpd_gpio)) {
> +             mutex_lock(&ddata->hpd_lock);
> +             ddata->hpd_enabled = false;
> +             mutex_unlock(&ddata->hpd_lock);
> +     } else if (in->ops.hdmi->disable_hpd) {
> +             in->ops.hdmi->disable_hpd(in);
> +     }
> +}
> +
>  static int hdmic_set_hdmi_mode(struct omap_dss_device *dssdev, bool
> hdmi_mode) {
>       struct panel_drv_data *ddata = to_panel_data(dssdev);
> @@ -200,10 +269,34 @@ static struct omap_dss_driver hdmic_driver = {
> 
>       .read_edid              = hdmic_read_edid,
>       .detect                 = hdmic_detect,
> +     .register_hpd_cb        = hdmic_register_hpd_cb,
> +     .unregister_hpd_cb      = hdmic_unregister_hpd_cb,
> +     .enable_hpd             = hdmic_enable_hpd,
> +     .disable_hpd            = hdmic_disable_hpd,
>       .set_hdmi_mode          = hdmic_set_hdmi_mode,
>       .set_hdmi_infoframe     = hdmic_set_infoframe,
>  };
> 
> +static irqreturn_t hdmic_hpd_isr(int irq, void *data)
> +{
> +     struct panel_drv_data *ddata = data;
> +
> +     mutex_lock(&ddata->hpd_lock);
> +     if (ddata->hpd_enabled && ddata->hpd_cb) {
> +             enum drm_connector_status status;
> +
> +             if (hdmic_detect(&ddata->dssdev))
> +                     status = connector_status_connected;
> +             else
> +                     status = connector_status_disconnected;
> +
> +             ddata->hpd_cb(ddata->hpd_cb_data, status);
> +     }
> +     mutex_unlock(&ddata->hpd_lock);

Shouldn't ddata->hpd_cb() be called without the mutex held ? I don't think 
core code should rely on callers to handle locking in this case.


> +     return IRQ_HANDLED;
> +}
> +
>  static int hdmic_probe_of(struct platform_device *pdev)
>  {
>       struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> @@ -249,11 +342,22 @@ static int hdmic_probe(struct platform_device *pdev)
>       if (r)
>               return r;
> 
> +     mutex_init(&ddata->hpd_lock);
> +
>       if (gpio_is_valid(ddata->hpd_gpio)) {
>               r = devm_gpio_request_one(&pdev->dev, ddata->hpd_gpio,
>                               GPIOF_DIR_IN, "hdmi_hpd");
>               if (r)
>                       goto err_reg;
> +
> +             r = devm_request_threaded_irq(&pdev->dev,
> +                             gpio_to_irq(ddata->hpd_gpio),

What is hpd_gpio is valid but has no IRQ support ?

> +                             NULL, hdmic_hpd_isr,
> +                             IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
> +                             IRQF_ONESHOT,
> +                             "hdmic hpd", ddata);
> +             if (r)
> +                     goto err_reg;
>       }
> 
>       ddata->vm = hdmic_default_vm;

-- 
Regards,

Laurent Pinchart

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

Reply via email to