Re: [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register

2019-08-08 Thread Hans Verkuil 
On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
> Use the new cec_notifier_conn_(un)register() functions to
> (un)register the notifier for the HDMI connector, and fill
> in the cec_connector_info.
> 
> Changes since v2:
>   - cec_notifier_phys_addr_invalidate where appropriate,
>   - don't check for NULL notifier before calling
>   cec_notifier_conn_unregister.
> Changes since v1:
>   Add memory barrier to make sure that the notifier
>   becomes visible to the irq thread once it is
>   fully constructed.
> 
> Signed-off-by: Dariusz Marcinkiewicz 

Tested-by: Hans Verkuil 

Tested with a BeagleBone Black board.

Regards,

Hans

> ---
>  drivers/gpu/drm/i2c/tda998x_drv.c | 33 +--
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
> b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 7f34601bb5155..31214663d6fdd 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -794,9 +794,14 @@ static irqreturn_t tda998x_irq_thread(int irq, void 
> *data)
>   if (lvl & CEC_RXSHPDLEV_HPD) {
>   tda998x_edid_delay_start(priv);
>   } else {
> + struct cec_notifier *notify;
> +
>   schedule_work(&priv->detect_work);
> - cec_notifier_set_phys_addr(priv->cec_notify,
> -CEC_PHYS_ADDR_INVALID);
> +
> + notify = READ_ONCE(priv->cec_notify);
> + if (notify)
> + cec_notifier_phys_addr_invalidate(
> + notify);
>   }
>  
>   handled = true;
> @@ -1253,6 +1258,8 @@ static int tda998x_connector_init(struct tda998x_priv 
> *priv,
> struct drm_device *drm)
>  {
>   struct drm_connector *connector = &priv->connector;
> + struct cec_connector_info conn_info;
> + struct cec_notifier *notifier;
>   int ret;
>  
>   connector->interlace_allowed = 1;
> @@ -1269,6 +1276,19 @@ static int tda998x_connector_init(struct tda998x_priv 
> *priv,
>   if (ret)
>   return ret;
>  
> + cec_fill_conn_info_from_drm(&conn_info, connector);
> +
> + notifier = cec_notifier_conn_register(priv->cec_glue.parent,
> +   NULL, &conn_info);
> + if (!notifier)
> + return -ENOMEM;
> + /*
> +  * Make sure that tda998x_irq_thread does see the notifier
> +  * when it fully constructed.
> +  */
> + smp_wmb();
> + priv->cec_notify = notifier;
> +
>   drm_connector_attach_encoder(&priv->connector,
>priv->bridge.encoder);
>  
> @@ -1650,8 +1670,7 @@ static void tda998x_destroy(struct device *dev)
>  
>   i2c_unregister_device(priv->cec);
>  
> - if (priv->cec_notify)
> - cec_notifier_put(priv->cec_notify);
> + cec_notifier_conn_unregister(priv->cec_notify);
>  }
>  
>  static int tda998x_create(struct device *dev)
> @@ -1776,12 +1795,6 @@ static int tda998x_create(struct device *dev)
>   cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
>   }
>  
> - priv->cec_notify = cec_notifier_get(dev);
> - if (!priv->cec_notify) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
>   priv->cec_glue.parent = dev;
>   priv->cec_glue.data = priv;
>   priv->cec_glue.init = tda998x_cec_hook_init;
>

[PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register

2019-08-07 Thread Dariusz Marcinkiewicz 
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.

Changes since v2:
- cec_notifier_phys_addr_invalidate where appropriate,
- don't check for NULL notifier before calling
cec_notifier_conn_unregister.
Changes since v1:
Add memory barrier to make sure that the notifier
becomes visible to the irq thread once it is
fully constructed.

Signed-off-by: Dariusz Marcinkiewicz 
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 33 +--
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c 
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..31214663d6fdd 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -794,9 +794,14 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
if (lvl & CEC_RXSHPDLEV_HPD) {
tda998x_edid_delay_start(priv);
} else {
+   struct cec_notifier *notify;
+
schedule_work(&priv->detect_work);
-   cec_notifier_set_phys_addr(priv->cec_notify,
-  CEC_PHYS_ADDR_INVALID);
+
+   notify = READ_ONCE(priv->cec_notify);
+   if (notify)
+   cec_notifier_phys_addr_invalidate(
+   notify);
}
 
handled = true;
@@ -1253,6 +1258,8 @@ static int tda998x_connector_init(struct tda998x_priv 
*priv,
  struct drm_device *drm)
 {
struct drm_connector *connector = &priv->connector;
+   struct cec_connector_info conn_info;
+   struct cec_notifier *notifier;
int ret;
 
connector->interlace_allowed = 1;
@@ -1269,6 +1276,19 @@ static int tda998x_connector_init(struct tda998x_priv 
*priv,
if (ret)
return ret;
 
+   cec_fill_conn_info_from_drm(&conn_info, connector);
+
+   notifier = cec_notifier_conn_register(priv->cec_glue.parent,
+ NULL, &conn_info);
+   if (!notifier)
+   return -ENOMEM;
+   /*
+* Make sure that tda998x_irq_thread does see the notifier
+* when it fully constructed.
+*/
+   smp_wmb();
+   priv->cec_notify = notifier;
+
drm_connector_attach_encoder(&priv->connector,
 priv->bridge.encoder);
 
@@ -1650,8 +1670,7 @@ static void tda998x_destroy(struct device *dev)
 
i2c_unregister_device(priv->cec);
 
-   if (priv->cec_notify)
-   cec_notifier_put(priv->cec_notify);
+   cec_notifier_conn_unregister(priv->cec_notify);
 }
 
 static int tda998x_create(struct device *dev)
@@ -1776,12 +1795,6 @@ static int tda998x_create(struct device *dev)
cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
}
 
-   priv->cec_notify = cec_notifier_get(dev);
-   if (!priv->cec_notify) {
-   ret = -ENOMEM;
-   goto fail;
-   }
-
priv->cec_glue.parent = dev;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
-- 
2.22.0.770.g0f2c4a37fd-goog