Hi Peter,

Thank you for the patch.

On Monday 15 May 2017 12:03:12 Peter Ujfalusi wrote:
> Use interrupt handler for hpd GPIO to react to HPD changes.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfal...@ti.com>
> ---
>  .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c index
> 58276a48112e..529b8509dd99 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -15,12 +15,17 @@
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/spinlock.h>

Did you mean linux/mutex.h ?

> 
>  #include "../dss/omapdss.h"
> 
>  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 gpio_desc *ct_cp_hpd_gpio;
>       struct gpio_desc *ls_oe_gpio;
> @@ -162,6 +167,49 @@ static bool tpd_detect(struct omap_dss_device *dssdev)
>       return gpiod_get_value_cansleep(ddata->hpd_gpio);
>  }
> 
> +static int tpd_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);
> +
> +     mutex_lock(&ddata->hpd_lock);
> +     ddata->hpd_cb = cb;
> +     ddata->hpd_cb_data = cb_data;
> +     mutex_unlock(&ddata->hpd_lock);
> +
> +     return 0;
> +}
> +
> +static void tpd_unregister_hpd_cb(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> +     mutex_lock(&ddata->hpd_lock);
> +     ddata->hpd_cb = NULL;
> +     ddata->hpd_cb_data = NULL;
> +     mutex_unlock(&ddata->hpd_lock);
> +}
> +
> +static void tpd_enable_hpd(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> +     mutex_lock(&ddata->hpd_lock);
> +     ddata->hpd_enabled = true;
> +     mutex_unlock(&ddata->hpd_lock);
> +}
> +
> +static void tpd_disable_hpd(struct omap_dss_device *dssdev)
> +{
> +     struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> +     mutex_lock(&ddata->hpd_lock);
> +     ddata->hpd_enabled = false;
> +     mutex_unlock(&ddata->hpd_lock);
> +}
> +
>  static int tpd_set_infoframe(struct omap_dss_device *dssdev,
>               const struct hdmi_avi_infoframe *avi)
>  {
> @@ -193,10 +241,34 @@ static const struct omapdss_hdmi_ops tpd_hdmi_ops = {
> 
>       .read_edid              = tpd_read_edid,
>       .detect                 = tpd_detect,
> +     .register_hpd_cb        = tpd_register_hpd_cb,
> +     .unregister_hpd_cb      = tpd_unregister_hpd_cb,
> +     .enable_hpd             = tpd_enable_hpd,
> +     .disable_hpd            = tpd_disable_hpd,
>       .set_infoframe          = tpd_set_infoframe,
>       .set_hdmi_mode          = tpd_set_hdmi_mode,
>  };
> 
> +static irqreturn_t tpd_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 (tpd_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);
> +
> +     return IRQ_HANDLED;
> +}
> +
>  static int tpd_probe_of(struct platform_device *pdev)
>  {
>       struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> @@ -261,6 +333,15 @@ static int tpd_probe(struct platform_device *pdev)
> 
>       ddata->hpd_gpio = gpio;
> 
> +     mutex_init(&ddata->hpd_lock);
> +
> +     r = devm_request_threaded_irq(&pdev->dev, gpiod_to_irq(ddata-
>hpd_gpio),

As the connector code can handle GPIO HPD thanks to patch 2/3, why does it 
have to be duplicated here ? I agree that encoders should support reporting of 
hotplug events when the HPD signal is connected to an encoder, but if it's 
connected to a GPIO, it seems to me that it should be the sole responsibility 
of the connector code to handle it.

> +             NULL, tpd_hpd_isr,
> +             IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +             "tpd12s015 hpd", ddata);
> +     if (r)
> +             goto err_gpio;
> +
>       dssdev = &ddata->dssdev;
>       dssdev->ops.hdmi = &tpd_hdmi_ops;
>       dssdev->dev = &pdev->dev;

-- 
Regards,

Laurent Pinchart

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

Reply via email to