Re: [PATCHv2 04/17] drm/omap: add HPD support to connector-dvi

2018-06-10 Thread Sebastian Reichel
Hi,

On Wed, Feb 28, 2018 at 01:26:01PM +0200, Tomi Valkeinen wrote:
> Add HPD support to the DVI connector driver. The code is almost
> identical to the HPD code in the HDMI connector driver.
> 
> Signed-off-by: Tomi Valkeinen 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/displays/connector-dvi.c | 118 
> +++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c 
> b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> index 391d80364346..6d8cbd9e2110 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -9,6 +9,7 @@
>   * the Free Software Foundation.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -44,6 +45,14 @@ struct panel_drv_data {
>   struct videomode vm;
>  
>   struct i2c_adapter *i2c_adapter;
> +
> + struct gpio_desc *hpd_gpio;
> +
> + void (*hpd_cb)(void *cb_data, enum drm_connector_status status);
> + void *hpd_cb_data;
> + bool hpd_enabled;
> + /* mutex for hpd fields above */
> + struct mutex hpd_lock;
>  };
>  
>  #define to_panel_data(x) container_of(x, struct panel_drv_data, dssdev)
> @@ -189,6 +198,9 @@ static int dvic_read_edid(struct omap_dss_device *dssdev,
>   struct panel_drv_data *ddata = to_panel_data(dssdev);
>   int r, l, bytes_read;
>  
> + if (ddata->hpd_gpio && !gpiod_get_value_cansleep(ddata->hpd_gpio))
> + return -ENODEV;
> +
>   if (!ddata->i2c_adapter)
>   return -ENODEV;
>  
> @@ -220,6 +232,9 @@ static bool dvic_detect(struct omap_dss_device *dssdev)
>   unsigned char out;
>   int r;
>  
> + if (ddata->hpd_gpio)
> + return gpiod_get_value_cansleep(ddata->hpd_gpio);
> +
>   if (!ddata->i2c_adapter)
>   return true;
>  
> @@ -228,6 +243,60 @@ static bool dvic_detect(struct omap_dss_device *dssdev)
>   return r == 0;
>  }
>  
> +static int dvic_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);
> +
> + if (!ddata->hpd_gpio)
> + 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 dvic_unregister_hpd_cb(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + 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 dvic_enable_hpd(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + if (!ddata->hpd_gpio)
> + return;
> +
> + mutex_lock(&ddata->hpd_lock);
> + ddata->hpd_enabled = true;
> + mutex_unlock(&ddata->hpd_lock);
> +}
> +
> +static void dvic_disable_hpd(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + if (!ddata->hpd_gpio)
> + return;
> +
> + mutex_lock(&ddata->hpd_lock);
> + ddata->hpd_enabled = false;
> + mutex_unlock(&ddata->hpd_lock);
> +}
> +
>  static struct omap_dss_driver dvic_driver = {
>   .connect= dvic_connect,
>   .disconnect = dvic_disconnect,
> @@ -241,14 +310,60 @@ static struct omap_dss_driver dvic_driver = {
>  
>   .read_edid  = dvic_read_edid,
>   .detect = dvic_detect,
> +
> + .register_hpd_cb= dvic_register_hpd_cb,
> + .unregister_hpd_cb  = dvic_unregister_hpd_cb,
> + .enable_hpd = dvic_enable_hpd,
> + .disable_hpd= dvic_disable_hpd,
>  };
>  
> +static irqreturn_t dvic_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 (dvic_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 dvic_probe_of(struct platform_device *pdev)
>  {
>   struct panel_drv_data *ddata = platform_get_drvdata(pdev);
>   struct device_node *node = pdev->dev.of_node;
>   struct device_node *adapter_node;
>   struct i2c_adapter *adapter;
> + struct gpio_desc *gpio;
> + int r;
> +
> + gpio = devm_

Re: [PATCHv2 04/17] drm/omap: add HPD support to connector-dvi

2018-02-28 Thread Laurent Pinchart
Hi Tomi,

Thank you for the patch.

On Wednesday, 28 February 2018 13:26:01 EET Tomi Valkeinen wrote:
> Add HPD support to the DVI connector driver. The code is almost
> identical to the HPD code in the HDMI connector driver.
> 
> Signed-off-by: Tomi Valkeinen 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/displays/connector-dvi.c | 118 
>  1 file changed, 118 insertions(+)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c index
> 391d80364346..6d8cbd9e2110 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -9,6 +9,7 @@
>   * the Free Software Foundation.
>   */
> 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -44,6 +45,14 @@ struct panel_drv_data {
>   struct videomode vm;
> 
>   struct i2c_adapter *i2c_adapter;
> +
> + struct gpio_desc *hpd_gpio;
> +
> + void (*hpd_cb)(void *cb_data, enum drm_connector_status status);
> + void *hpd_cb_data;
> + bool hpd_enabled;
> + /* mutex for hpd fields above */
> + struct mutex hpd_lock;
>  };
> 
>  #define to_panel_data(x) container_of(x, struct panel_drv_data, dssdev)
> @@ -189,6 +198,9 @@ static int dvic_read_edid(struct omap_dss_device
> *dssdev, struct panel_drv_data *ddata = to_panel_data(dssdev);
>   int r, l, bytes_read;
> 
> + if (ddata->hpd_gpio && !gpiod_get_value_cansleep(ddata->hpd_gpio))
> + return -ENODEV;
> +
>   if (!ddata->i2c_adapter)
>   return -ENODEV;
> 
> @@ -220,6 +232,9 @@ static bool dvic_detect(struct omap_dss_device *dssdev)
>   unsigned char out;
>   int r;
> 
> + if (ddata->hpd_gpio)
> + return gpiod_get_value_cansleep(ddata->hpd_gpio);
> +
>   if (!ddata->i2c_adapter)
>   return true;
> 
> @@ -228,6 +243,60 @@ static bool dvic_detect(struct omap_dss_device *dssdev)
> return r == 0;
>  }
> 
> +static int dvic_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);
> +
> + if (!ddata->hpd_gpio)
> + 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 dvic_unregister_hpd_cb(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + 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 dvic_enable_hpd(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + if (!ddata->hpd_gpio)
> + return;
> +
> + mutex_lock(&ddata->hpd_lock);
> + ddata->hpd_enabled = true;
> + mutex_unlock(&ddata->hpd_lock);
> +}
> +
> +static void dvic_disable_hpd(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + if (!ddata->hpd_gpio)
> + return;
> +
> + mutex_lock(&ddata->hpd_lock);
> + ddata->hpd_enabled = false;
> + mutex_unlock(&ddata->hpd_lock);
> +}
> +
>  static struct omap_dss_driver dvic_driver = {
>   .connect= dvic_connect,
>   .disconnect = dvic_disconnect,
> @@ -241,14 +310,60 @@ static struct omap_dss_driver dvic_driver = {
> 
>   .read_edid  = dvic_read_edid,
>   .detect = dvic_detect,
> +
> + .register_hpd_cb= dvic_register_hpd_cb,
> + .unregister_hpd_cb  = dvic_unregister_hpd_cb,
> + .enable_hpd = dvic_enable_hpd,
> + .disable_hpd= dvic_disable_hpd,
>  };
> 
> +static irqreturn_t dvic_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 (dvic_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 dvic_probe_of(struct platform_device *pdev)
>  {
>   struct panel_drv_data *ddata = platform_get_drvdata(pdev);
>   struct device_node *node = pdev->dev.of_node;
>   struct device_node *adapter_node;
>   struct i2c_adapter *adapter;
> + struct gpio_desc *gpio;
> + int r;
> +
> + gpio = devm_gpiod_get_o