linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API
@ 2019-08-07  8:52 Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: Dariusz Marcinkiewicz

This series updates DRM drivers to use new CEC notifier API.

Only first two patches were tested on the actual hardware.

Changes since v4:
	Addressing review comments.
Changes since v3:
        Updated adapter flags in dw-hdmi-cec.
Changes since v2:
	Include all DRM patches from "cec: improve notifier support,
	add connector info connector info" series.
Changes since v1:
	Those patches delay creation of notifiers until respective
	connectors are constructed. It seems that those patches, for a
	couple of drivers, by adding the delay, introduce a race between
	notifiers' creation and the IRQs handling threads - at least I
	don't see anything obvious in there that would explicitly forbid
	such races to occur. v2 adds a write barrier to make sure IRQ
	threads see the notifier once it is created (replacing the
	WRITE_ONCE I put in v1). The best thing to do here, I believe,
	would be not to have any synchronization and make sure that an IRQ
	only gets enabled after the notifier is created.

Dariusz Marcinkiewicz (9):
  drm_dp_cec: add connector info support.
  drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  tda9950: use cec_notifier_cec_adap_(un)register
  drm: tda998x: use cec_notifier_conn_(un)register
  drm: sti: use cec_notifier_conn_(un)register
  drm: tegra: use cec_notifier_conn_(un)register
  drm: dw-hdmi: use cec_notifier_conn_(un)register
  drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register

 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 13 ++++---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 36 +++++++++++--------
 drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++-----
 drivers/gpu/drm/exynos/exynos_hdmi.c          | 31 +++++++++-------
 drivers/gpu/drm/i2c/tda9950.c                 | 12 +++----
 drivers/gpu/drm/i2c/tda998x_drv.c             | 33 +++++++++++------
 drivers/gpu/drm/i915/intel_dp.c               |  4 +--
 drivers/gpu/drm/i915/intel_hdmi.c             | 13 ++++---
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +-
 drivers/gpu/drm/sti/sti_hdmi.c                | 19 ++++++----
 drivers/gpu/drm/tegra/output.c                | 28 +++++++++++----
 include/drm/drm_dp_helper.h                   | 14 ++++----
 13 files changed, 143 insertions(+), 90 deletions(-)

-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v5 1/9] drm_dp_cec: add connector info support.
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08  8:18   ` Hans Verkuil
  2019-08-08  8:44   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: Dariusz Marcinkiewicz

Pass the connector info to the CEC adapter. This makes it possible
to associate the CEC adapter with the corresponding drm connector.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
 drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
 drivers/gpu/drm/i915/intel_dp.c               |  4 +--
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
 include/drm/drm_dp_helper.h                   | 14 +++++------
 5 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 6e205ee36ac3b..7f2eb4eb1035b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -394,7 +394,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
 
 	drm_dp_aux_register(&aconnector->dm_dp_aux.aux);
 	drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
-				      aconnector->base.name, dm->adev->dev);
+				      &aconnector->base);
 	aconnector->mst_mgr.cbs = &dm_mst_cbs;
 	drm_dp_mst_topology_mgr_init(
 		&aconnector->mst_mgr,
diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
index b15cee85b702b..b457c16c3a8bb 100644
--- a/drivers/gpu/drm/drm_dp_cec.c
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -8,7 +8,9 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <drm/drm_connector.h>
 #include <drm/drm_dp_helper.h>
+#include <drm/drmP.h>
 #include <media/cec.h>
 
 /*
@@ -295,7 +297,10 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)
  */
 void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
 {
-	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
+	struct drm_connector *connector = aux->cec.connector;
+	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
+		       CEC_CAP_CONNECTOR_INFO;
+	struct cec_connector_info conn_info;
 	unsigned int num_las = 1;
 	u8 cap;
 
@@ -344,13 +349,17 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
 
 	/* Create a new adapter */
 	aux->cec.adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
-					     aux, aux->cec.name, cec_caps,
+					     aux, connector->name, cec_caps,
 					     num_las);
 	if (IS_ERR(aux->cec.adap)) {
 		aux->cec.adap = NULL;
 		goto unlock;
 	}
-	if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
+
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+	cec_s_conn_info(aux->cec.adap, &conn_info);
+
+	if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
 		cec_delete_adapter(aux->cec.adap);
 		aux->cec.adap = NULL;
 	} else {
@@ -406,22 +415,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
 /**
  * drm_dp_cec_register_connector() - register a new connector
  * @aux: DisplayPort AUX channel
- * @name: name of the CEC device
- * @parent: parent device
+ * @connector: drm connector
  *
  * A new connector was registered with associated CEC adapter name and
  * CEC adapter parent device. After registering the name and parent
  * drm_dp_cec_set_edid() is called to check if the connector supports
  * CEC and to register a CEC adapter if that is the case.
  */
-void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
-				   struct device *parent)
+void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
+				   struct drm_connector *connector)
 {
 	WARN_ON(aux->cec.adap);
 	if (WARN_ON(!aux->transfer))
 		return;
-	aux->cec.name = name;
-	aux->cec.parent = parent;
+	aux->cec.connector = connector;
 	INIT_DELAYED_WORK(&aux->cec.unregister_work,
 			  drm_dp_cec_unregister_work);
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4336df46fe782..79c8afdbc4869 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5693,7 +5693,6 @@ static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct drm_device *dev = connector->dev;
 	int ret;
 
 	ret = intel_connector_register(connector);
@@ -5708,8 +5707,7 @@ intel_dp_connector_register(struct drm_connector *connector)
 	intel_dp->aux.dev = connector->kdev;
 	ret = drm_dp_aux_register(&intel_dp->aux);
 	if (!ret)
-		drm_dp_cec_register_connector(&intel_dp->aux,
-					      connector->name, dev->dev);
+		drm_dp_cec_register_connector(&intel_dp->aux, connector);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 4116ee62adafe..4438824ca88b0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,
 	switch (type) {
 	case DRM_MODE_CONNECTOR_DisplayPort:
 	case DRM_MODE_CONNECTOR_eDP:
-		drm_dp_cec_register_connector(&nv_connector->aux,
-					      connector->name, dev->dev);
+		drm_dp_cec_register_connector(&nv_connector->aux, connector);
 		break;
 	}
 
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 3fc534ee81746..729af0b812909 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1221,6 +1221,7 @@ struct drm_dp_aux_msg {
 
 struct cec_adapter;
 struct edid;
+struct drm_connector;
 
 /**
  * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
@@ -1233,8 +1234,7 @@ struct edid;
 struct drm_dp_aux_cec {
 	struct mutex lock;
 	struct cec_adapter *adap;
-	const char *name;
-	struct device *parent;
+	struct drm_connector *connector;
 	struct delayed_work unregister_work;
 };
 
@@ -1431,8 +1431,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
 
 #ifdef CONFIG_DRM_DP_CEC
 void drm_dp_cec_irq(struct drm_dp_aux *aux);
-void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
-				   struct device *parent);
+void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
+				   struct drm_connector *connector);
 void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
 void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
 void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
@@ -1441,9 +1441,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux)
 {
 }
 
-static inline void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
-						 const char *name,
-						 struct device *parent)
+static inline void
+drm_dp_cec_register_connector(struct drm_dp_aux *aux,
+			      struct drm_connector *connector)
 {
 }
 
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08 11:47   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 187a2b828b975..db7953ce49c8e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2752,8 +2752,9 @@ intel_hdmi_connector_register(struct drm_connector *connector)
 
 static void intel_hdmi_destroy(struct drm_connector *connector)
 {
-	if (intel_attached_hdmi(connector)->cec_notifier)
-		cec_notifier_put(intel_attached_hdmi(connector)->cec_notifier);
+	struct cec_notifier *n = intel_attached_hdmi(connector)->cec_notifier;
+
+	cec_notifier_conn_unregister(n);
 
 	intel_connector_destroy(connector);
 }
@@ -3061,6 +3062,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	struct drm_device *dev = intel_encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum port port = intel_encoder->port;
+	struct cec_connector_info conn_info;
 
 	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
 		      port_name(port));
@@ -3113,8 +3115,11 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
 	}
 
-	intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
-							 port_identifier(port));
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+
+	intel_hdmi->cec_notifier =
+		cec_notifier_conn_register(dev->dev, port_identifier(port),
+					   &conn_info);
 	if (!intel_hdmi->cec_notifier)
 		DRM_DEBUG_KMS("CEC notifier get failed\n");
 }
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-09 10:25   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 4/9] tda9950: " Dariusz Marcinkiewicz
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: Dariusz Marcinkiewicz

Use the new cec_notifier_cec_adap_(un)register() functions to
(un)register the notifier for the CEC adapter.

Also adds CEC_CAP_CONNECTOR_INFO capability to the adapter.

Changes since v3:
	- add CEC_CAP_CONNECTOR_INFO to cec_allocate_adapter,
	- replace CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
	CEC_CAP_RC | CEC_CAP_PASSTHROUGH with CEC_CAP_DEFAULTS.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index 6c323510f1288..361acff2111cf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -259,8 +259,8 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
 
 	cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
-					 CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
-					 CEC_CAP_RC | CEC_CAP_PASSTHROUGH,
+					 CEC_CAP_DEFAULTS |
+					 CEC_CAP_CONNECTOR_INFO,
 					 CEC_MAX_LOG_ADDRS);
 	if (IS_ERR(cec->adap))
 		return PTR_ERR(cec->adap);
@@ -281,13 +281,14 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	cec->notify = cec_notifier_get(pdev->dev.parent);
+	cec->notify = cec_notifier_cec_adap_register(pdev->dev.parent,
+						     NULL, cec->adap);
 	if (!cec->notify)
 		return -ENOMEM;
 
 	ret = cec_register_adapter(cec->adap, pdev->dev.parent);
 	if (ret < 0) {
-		cec_notifier_put(cec->notify);
+		cec_notifier_cec_adap_unregister(cec->notify);
 		return ret;
 	}
 
@@ -297,8 +298,6 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	 */
 	devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec);
 
-	cec_register_cec_notifier(cec->adap, cec->notify);
-
 	return 0;
 }
 
@@ -306,8 +305,8 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev)
 {
 	struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
 
+	cec_notifier_cec_adap_unregister(cec->notify);
 	cec_unregister_adapter(cec->adap);
-	cec_notifier_put(cec->notify);
 
 	return 0;
 }
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 4/9] tda9950: use cec_notifier_cec_adap_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (2 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08  9:11   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: Dariusz Marcinkiewicz

Use the new cec_notifier_cec_adap_(un)register() functions to
(un)register the notifier for the CEC adapter.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/i2c/tda9950.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index 250b5e02a314a..e9f6171c47792 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -423,7 +423,8 @@ static int tda9950_probe(struct i2c_client *client,
 		priv->hdmi = glue->parent;
 
 	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
-					  CEC_CAP_DEFAULTS,
+					  CEC_CAP_DEFAULTS |
+					  CEC_CAP_CONNECTOR_INFO,
 					  CEC_MAX_LOG_ADDRS);
 	if (IS_ERR(priv->adap))
 		return PTR_ERR(priv->adap);
@@ -460,13 +461,14 @@ static int tda9950_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
-	priv->notify = cec_notifier_get(priv->hdmi);
+	priv->notify = cec_notifier_cec_adap_register(priv->hdmi, NULL,
+						      priv->adap);
 	if (!priv->notify)
 		return -ENOMEM;
 
 	ret = cec_register_adapter(priv->adap, priv->hdmi);
 	if (ret < 0) {
-		cec_notifier_put(priv->notify);
+		cec_notifier_cec_adap_unregister(priv->notify);
 		return ret;
 	}
 
@@ -476,8 +478,6 @@ static int tda9950_probe(struct i2c_client *client,
 	 */
 	devm_remove_action(dev, tda9950_cec_del, priv);
 
-	cec_register_cec_notifier(priv->adap, priv->notify);
-
 	return 0;
 }
 
@@ -485,8 +485,8 @@ static int tda9950_remove(struct i2c_client *client)
 {
 	struct tda9950_priv *priv = i2c_get_clientdata(client);
 
+	cec_notifier_cec_adap_unregister(priv->notify);
 	cec_unregister_adapter(priv->adap);
-	cec_notifier_put(priv->notify);
 
 	return 0;
 }
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (3 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 4/9] tda9950: " Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08  9:12   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 6/9] drm: sti: " Dariusz Marcinkiewicz
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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 <darekm@google.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 6/9] drm: sti: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (4 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 7/9] drm: tegra: " Dariusz Marcinkiewicz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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:
	Don't invalidate physical address before unregistering the
	notifier.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
---
 drivers/gpu/drm/sti/sti_hdmi.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 6000df6249807..6de30c0eee9e1 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1250,6 +1250,7 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 	struct drm_device *drm_dev = data;
 	struct drm_encoder *encoder;
 	struct sti_hdmi_connector *connector;
+	struct cec_connector_info conn_info;
 	struct drm_connector *drm_connector;
 	struct drm_bridge *bridge;
 	int err;
@@ -1310,6 +1311,14 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 		goto err_sysfs;
 	}
 
+	cec_fill_conn_info_from_drm(&conn_info, drm_connector);
+	hdmi->notifier = cec_notifier_conn_register(&hdmi->dev, NULL,
+						    &conn_info);
+	if (!hdmi->notifier) {
+		hdmi->drm_connector = NULL;
+		return -ENOMEM;
+	}
+
 	/* Enable default interrupts */
 	hdmi_write(hdmi, HDMI_DEFAULT_INT, HDMI_INT_EN);
 
@@ -1323,6 +1332,9 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 static void sti_hdmi_unbind(struct device *dev,
 		struct device *master, void *data)
 {
+	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+
+	cec_notifier_conn_unregister(hdmi->notifier);
 }
 
 static const struct component_ops sti_hdmi_ops = {
@@ -1428,10 +1440,6 @@ static int sti_hdmi_probe(struct platform_device *pdev)
 		goto release_adapter;
 	}
 
-	hdmi->notifier = cec_notifier_get(&pdev->dev);
-	if (!hdmi->notifier)
-		goto release_adapter;
-
 	hdmi->reset = devm_reset_control_get(dev, "hdmi");
 	/* Take hdmi out of reset */
 	if (!IS_ERR(hdmi->reset))
@@ -1451,14 +1459,11 @@ static int sti_hdmi_remove(struct platform_device *pdev)
 {
 	struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
 
-	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
-
 	i2c_put_adapter(hdmi->ddc_adapt);
 	if (hdmi->audio_pdev)
 		platform_device_unregister(hdmi->audio_pdev);
 	component_del(&pdev->dev, &sti_hdmi_ops);
 
-	cec_notifier_put(hdmi->notifier);
 	return 0;
 }
 
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (5 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 6/9] drm: sti: " Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08 20:42   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
  2019-08-07  8:52 ` [PATCH v5 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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 v4:
	- only create a CEC notifier for HDMI connectors

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
---
 drivers/gpu/drm/tegra/output.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 9c2b9dad55c30..13153b93626fd 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -80,6 +80,11 @@ tegra_output_connector_detect(struct drm_connector *connector, bool force)
 
 void tegra_output_connector_destroy(struct drm_connector *connector)
 {
+	struct tegra_output *output = connector_to_output(connector);
+
+	if (output->cec)
+		cec_notifier_conn_unregister(output->cec);
+
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 }
@@ -174,18 +179,11 @@ int tegra_output_probe(struct tegra_output *output)
 		disable_irq(output->hpd_irq);
 	}
 
-	output->cec = cec_notifier_get(output->dev);
-	if (!output->cec)
-		return -ENOMEM;
-
 	return 0;
 }
 
 void tegra_output_remove(struct tegra_output *output)
 {
-	if (output->cec)
-		cec_notifier_put(output->cec);
-
 	if (gpio_is_valid(output->hpd_gpio)) {
 		free_irq(output->hpd_irq, output);
 		gpio_free(output->hpd_gpio);
@@ -197,6 +195,7 @@ void tegra_output_remove(struct tegra_output *output)
 
 int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
 {
+	int connector_type;
 	int err;
 
 	if (output->panel) {
@@ -212,6 +211,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
 	if (gpio_is_valid(output->hpd_gpio))
 		enable_irq(output->hpd_irq);
 
+	connector_type = output->connector.connector_type;
+	/*
+	 * Create a CEC notifier for HDMI connector.
+	 */
+	if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
+	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
+		struct cec_connector_info conn_info;
+
+		cec_fill_conn_info_from_drm(&conn_info, &output->connector);
+		output->cec = cec_notifier_conn_register(output->dev, NULL,
+							 &conn_info);
+		if (!output->cec)
+			return -ENOMEM;
+	}
+
 	return 0;
 }
 
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (6 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 7/9] drm: tegra: " Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-09 15:12   ` Hans Verkuil
  2019-08-07  8:52 ` [PATCH v5 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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 v4:
	- typo fix
Changes since v2:
	- removed unnecessary NULL check before a call to
	cec_notifier_conn_unregister,
	- use cec_notifier_phys_addr_invalidate to invalidate physical
	address.
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 <darekm@google.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 36 ++++++++++++++---------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a29..b7d0d9ad5f2f7 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2118,6 +2118,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
 	struct dw_hdmi *hdmi = bridge->driver_private;
 	struct drm_encoder *encoder = bridge->encoder;
 	struct drm_connector *connector = &hdmi->connector;
+	struct cec_connector_info conn_info;
+	struct cec_notifier *notifier;
 
 	connector->interlace_allowed = 1;
 	connector->polled = DRM_CONNECTOR_POLL_HPD;
@@ -2129,6 +2131,18 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
 
 	drm_connector_attach_encoder(connector, encoder);
 
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+
+	notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info);
+	if (!notifier)
+		return -ENOMEM;
+	/*
+	 * Make sure that dw_hdmi_irq thread does see the notifier
+	 * when it fully constructed.
+	 */
+	smp_wmb();
+	hdmi->cec_notifier = notifier;
+
 	return 0;
 }
 
@@ -2295,9 +2309,13 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 				       phy_stat & HDMI_PHY_HPD,
 				       phy_stat & HDMI_PHY_RX_SENSE);
 
-		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
-			cec_notifier_set_phys_addr(hdmi->cec_notifier,
-						   CEC_PHYS_ADDR_INVALID);
+		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
+			struct cec_notifier *notifier;
+
+			notifier = READ_ONCE(hdmi->cec_notifier);
+			if (notifier)
+				cec_notifier_phys_addr_invalidate(notifier);
+		}
 	}
 
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
@@ -2600,12 +2618,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	if (ret)
 		goto err_iahb;
 
-	hdmi->cec_notifier = cec_notifier_get(dev);
-	if (!hdmi->cec_notifier) {
-		ret = -ENOMEM;
-		goto err_iahb;
-	}
-
 	/*
 	 * To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
 	 * N and cts values before enabling phy
@@ -2693,9 +2705,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->ddc = NULL;
 	}
 
-	if (hdmi->cec_notifier)
-		cec_notifier_put(hdmi->cec_notifier);
-
 	clk_disable_unprepare(hdmi->iahb_clk);
 	if (hdmi->cec_clk)
 		clk_disable_unprepare(hdmi->cec_clk);
@@ -2717,8 +2726,7 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 	/* Disable all interrupts */
 	hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
 
-	if (hdmi->cec_notifier)
-		cec_notifier_put(hdmi->cec_notifier);
+	cec_notifier_conn_unregister(hdmi->cec_notifier);
 
 	clk_disable_unprepare(hdmi->iahb_clk);
 	clk_disable_unprepare(hdmi->isfr_clk);
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (7 preceding siblings ...)
  2019-08-07  8:52 ` [PATCH v5 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
@ 2019-08-07  8:52 ` Dariusz Marcinkiewicz
  2019-08-08 14:27   ` Hans Verkuil
  8 siblings, 1 reply; 20+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-07  8:52 UTC (permalink / raw)
  To: linux-media, hverkuil-cisco; +Cc: 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:
	- removed unnecessary call to invalidate phys address before
	deregistering the notifier,
	- use cec_notifier_phys_addr_invalidate instead of setting
	invalid address on a notifier.

Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 31 ++++++++++++++++------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 19c252f659dd0..5fd2551a93567 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -858,6 +858,10 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
 
 static void hdmi_connector_destroy(struct drm_connector *connector)
 {
+	struct hdmi_context *hdata = connector_to_hdmi(connector);
+
+	cec_notifier_conn_unregister(hdata->notifier);
+
 	drm_connector_unregister(connector);
 	drm_connector_cleanup(connector);
 }
@@ -941,6 +945,7 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 {
 	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
 	struct drm_connector *connector = &hdata->connector;
+	struct cec_connector_info conn_info;
 	int ret;
 
 	connector->interlace_allowed = true;
@@ -963,6 +968,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 			DRM_DEV_ERROR(hdata->dev, "Failed to attach bridge\n");
 	}
 
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+
+	hdata->notifier = cec_notifier_conn_register(hdata->dev, NULL,
+						     &conn_info);
+	if (hdata->notifier == NULL) {
+		ret = -ENOMEM;
+		DRM_DEV_ERROR(hdata->dev, "Failed to allocate CEC notifier\n");
+	}
+
 	return ret;
 }
 
@@ -1534,8 +1548,8 @@ static void hdmi_disable(struct drm_encoder *encoder)
 		 */
 		mutex_unlock(&hdata->mutex);
 		cancel_delayed_work(&hdata->hotplug_work);
-		cec_notifier_set_phys_addr(hdata->notifier,
-					   CEC_PHYS_ADDR_INVALID);
+		if (hdata->notifier)
+			cec_notifier_phys_addr_invalidate(hdata->notifier);
 		return;
 	}
 
@@ -2012,12 +2026,6 @@ static int hdmi_probe(struct platform_device *pdev)
 		}
 	}
 
-	hdata->notifier = cec_notifier_get(&pdev->dev);
-	if (hdata->notifier == NULL) {
-		ret = -ENOMEM;
-		goto err_hdmiphy;
-	}
-
 	pm_runtime_enable(dev);
 
 	audio_infoframe = &hdata->audio.infoframe;
@@ -2029,7 +2037,7 @@ static int hdmi_probe(struct platform_device *pdev)
 
 	ret = hdmi_register_audio_device(hdata);
 	if (ret)
-		goto err_notifier_put;
+		goto err_runtime_disable;
 
 	ret = component_add(&pdev->dev, &hdmi_component_ops);
 	if (ret)
@@ -2040,8 +2048,7 @@ static int hdmi_probe(struct platform_device *pdev)
 err_unregister_audio:
 	platform_device_unregister(hdata->audio.pdev);
 
-err_notifier_put:
-	cec_notifier_put(hdata->notifier);
+err_runtime_disable:
 	pm_runtime_disable(dev);
 
 err_hdmiphy:
@@ -2060,12 +2067,10 @@ static int hdmi_remove(struct platform_device *pdev)
 	struct hdmi_context *hdata = platform_get_drvdata(pdev);
 
 	cancel_delayed_work_sync(&hdata->hotplug_work);
-	cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
 
 	component_del(&pdev->dev, &hdmi_component_ops);
 	platform_device_unregister(hdata->audio.pdev);
 
-	cec_notifier_put(hdata->notifier);
 	pm_runtime_disable(&pdev->dev);
 
 	if (!IS_ERR(hdata->reg_hdmi_en))
-- 
2.22.0.770.g0f2c4a37fd-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 1/9] drm_dp_cec: add connector info support.
  2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
@ 2019-08-08  8:18   ` Hans Verkuil
  2019-08-09 10:30     ` Hans Verkuil
  2019-08-08  8:44   ` Hans Verkuil
  1 sibling, 1 reply; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08  8:18 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

Hi Dariusz,

On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
> Pass the connector info to the CEC adapter. This makes it possible
> to associate the CEC adapter with the corresponding drm connector.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>  drivers/gpu/drm/i915/intel_dp.c               |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>  include/drm/drm_dp_helper.h                   | 14 +++++------
>  5 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 6e205ee36ac3b..7f2eb4eb1035b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -394,7 +394,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>  
>  	drm_dp_aux_register(&aconnector->dm_dp_aux.aux);
>  	drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
> -				      aconnector->base.name, dm->adev->dev);
> +				      &aconnector->base);
>  	aconnector->mst_mgr.cbs = &dm_mst_cbs;
>  	drm_dp_mst_topology_mgr_init(
>  		&aconnector->mst_mgr,
> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> index b15cee85b702b..b457c16c3a8bb 100644
> --- a/drivers/gpu/drm/drm_dp_cec.c
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -8,7 +8,9 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_dp_helper.h>
> +#include <drm/drmP.h>
>  #include <media/cec.h>
>  
>  /*
> @@ -295,7 +297,10 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)
>   */
>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>  {
> -	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
> +	struct drm_connector *connector = aux->cec.connector;
> +	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
> +		       CEC_CAP_CONNECTOR_INFO;
> +	struct cec_connector_info conn_info;
>  	unsigned int num_las = 1;
>  	u8 cap;
>  
> @@ -344,13 +349,17 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>  
>  	/* Create a new adapter */
>  	aux->cec.adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
> -					     aux, aux->cec.name, cec_caps,
> +					     aux, connector->name, cec_caps,
>  					     num_las);
>  	if (IS_ERR(aux->cec.adap)) {
>  		aux->cec.adap = NULL;
>  		goto unlock;
>  	}
> -	if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
> +
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +	cec_s_conn_info(aux->cec.adap, &conn_info);
> +
> +	if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
>  		cec_delete_adapter(aux->cec.adap);
>  		aux->cec.adap = NULL;
>  	} else {
> @@ -406,22 +415,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
>  /**
>   * drm_dp_cec_register_connector() - register a new connector
>   * @aux: DisplayPort AUX channel
> - * @name: name of the CEC device
> - * @parent: parent device
> + * @connector: drm connector
>   *
>   * A new connector was registered with associated CEC adapter name and
>   * CEC adapter parent device. After registering the name and parent
>   * drm_dp_cec_set_edid() is called to check if the connector supports
>   * CEC and to register a CEC adapter if that is the case.
>   */
> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
> -				   struct device *parent)
> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +				   struct drm_connector *connector)
>  {
>  	WARN_ON(aux->cec.adap);
>  	if (WARN_ON(!aux->transfer))
>  		return;
> -	aux->cec.name = name;
> -	aux->cec.parent = parent;
> +	aux->cec.connector = connector;
>  	INIT_DELAYED_WORK(&aux->cec.unregister_work,
>  			  drm_dp_cec_unregister_work);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4336df46fe782..79c8afdbc4869 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5693,7 +5693,6 @@ static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct drm_device *dev = connector->dev;
>  	int ret;
>  
>  	ret = intel_connector_register(connector);
> @@ -5708,8 +5707,7 @@ intel_dp_connector_register(struct drm_connector *connector)
>  	intel_dp->aux.dev = connector->kdev;
>  	ret = drm_dp_aux_register(&intel_dp->aux);
>  	if (!ret)
> -		drm_dp_cec_register_connector(&intel_dp->aux,
> -					      connector->name, dev->dev);
> +		drm_dp_cec_register_connector(&intel_dp->aux, connector);
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 4116ee62adafe..4438824ca88b0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,
>  	switch (type) {
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  	case DRM_MODE_CONNECTOR_eDP:
> -		drm_dp_cec_register_connector(&nv_connector->aux,
> -					      connector->name, dev->dev);
> +		drm_dp_cec_register_connector(&nv_connector->aux, connector);
>  		break;
>  	}
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 3fc534ee81746..729af0b812909 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1221,6 +1221,7 @@ struct drm_dp_aux_msg {
>  
>  struct cec_adapter;
>  struct edid;
> +struct drm_connector;
>  
>  /**
>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
> @@ -1233,8 +1234,7 @@ struct edid;
>  struct drm_dp_aux_cec {
>  	struct mutex lock;
>  	struct cec_adapter *adap;
> -	const char *name;
> -	struct device *parent;
> +	struct drm_connector *connector;
>  	struct delayed_work unregister_work;
>  };

You probably saw the kbuild test robot mail as well: the comments before this
struct need to be updated.

Also, this series is against an older tree: intel_dp/hdmi.c is now moved to
a i915/display/ directory.

Can you rebase and post a v6 for this series? Make sure to post to dri-devel as
well! The various drm maintainers of these drivers have to review the code as well
and give an Acked-by! I noticed that you only posted it to linux-media, but
that's not enough. In fact, it would be great if you can Cc each patch to the
corresponding maintainer as well (see the MAINTAINERS file).

I've started testing this series on my hardware and I will post Tested-by tags
whenever I finished testing each patch for which I have HW.

Thanks!

	Hans

>  
> @@ -1431,8 +1431,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
>  
>  #ifdef CONFIG_DRM_DP_CEC
>  void drm_dp_cec_irq(struct drm_dp_aux *aux);
> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
> -				   struct device *parent);
> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +				   struct drm_connector *connector);
>  void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
>  void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
> @@ -1441,9 +1441,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux)
>  {
>  }
>  
> -static inline void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> -						 const char *name,
> -						 struct device *parent)
> +static inline void
> +drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +			      struct drm_connector *connector)
>  {
>  }
>  
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 1/9] drm_dp_cec: add connector info support.
  2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
  2019-08-08  8:18   ` Hans Verkuil
@ 2019-08-08  8:44   ` Hans Verkuil
  1 sibling, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08  8:44 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
> Pass the connector info to the CEC adapter. This makes it possible
> to associate the CEC adapter with the corresponding drm connector.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with a USB-C-to-HDMI adapter on an Intel NUC and with a DP-to-HDMI
adapter on an AMD GPU and an NVIDIA GPU.

Regards,

	Hans

> ---
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>  drivers/gpu/drm/i915/intel_dp.c               |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>  include/drm/drm_dp_helper.h                   | 14 +++++------
>  5 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 6e205ee36ac3b..7f2eb4eb1035b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -394,7 +394,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>  
>  	drm_dp_aux_register(&aconnector->dm_dp_aux.aux);
>  	drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
> -				      aconnector->base.name, dm->adev->dev);
> +				      &aconnector->base);
>  	aconnector->mst_mgr.cbs = &dm_mst_cbs;
>  	drm_dp_mst_topology_mgr_init(
>  		&aconnector->mst_mgr,
> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> index b15cee85b702b..b457c16c3a8bb 100644
> --- a/drivers/gpu/drm/drm_dp_cec.c
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -8,7 +8,9 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_dp_helper.h>
> +#include <drm/drmP.h>
>  #include <media/cec.h>
>  
>  /*
> @@ -295,7 +297,10 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)
>   */
>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>  {
> -	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
> +	struct drm_connector *connector = aux->cec.connector;
> +	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
> +		       CEC_CAP_CONNECTOR_INFO;
> +	struct cec_connector_info conn_info;
>  	unsigned int num_las = 1;
>  	u8 cap;
>  
> @@ -344,13 +349,17 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>  
>  	/* Create a new adapter */
>  	aux->cec.adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
> -					     aux, aux->cec.name, cec_caps,
> +					     aux, connector->name, cec_caps,
>  					     num_las);
>  	if (IS_ERR(aux->cec.adap)) {
>  		aux->cec.adap = NULL;
>  		goto unlock;
>  	}
> -	if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
> +
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +	cec_s_conn_info(aux->cec.adap, &conn_info);
> +
> +	if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
>  		cec_delete_adapter(aux->cec.adap);
>  		aux->cec.adap = NULL;
>  	} else {
> @@ -406,22 +415,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
>  /**
>   * drm_dp_cec_register_connector() - register a new connector
>   * @aux: DisplayPort AUX channel
> - * @name: name of the CEC device
> - * @parent: parent device
> + * @connector: drm connector
>   *
>   * A new connector was registered with associated CEC adapter name and
>   * CEC adapter parent device. After registering the name and parent
>   * drm_dp_cec_set_edid() is called to check if the connector supports
>   * CEC and to register a CEC adapter if that is the case.
>   */
> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
> -				   struct device *parent)
> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +				   struct drm_connector *connector)
>  {
>  	WARN_ON(aux->cec.adap);
>  	if (WARN_ON(!aux->transfer))
>  		return;
> -	aux->cec.name = name;
> -	aux->cec.parent = parent;
> +	aux->cec.connector = connector;
>  	INIT_DELAYED_WORK(&aux->cec.unregister_work,
>  			  drm_dp_cec_unregister_work);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4336df46fe782..79c8afdbc4869 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5693,7 +5693,6 @@ static int
>  intel_dp_connector_register(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct drm_device *dev = connector->dev;
>  	int ret;
>  
>  	ret = intel_connector_register(connector);
> @@ -5708,8 +5707,7 @@ intel_dp_connector_register(struct drm_connector *connector)
>  	intel_dp->aux.dev = connector->kdev;
>  	ret = drm_dp_aux_register(&intel_dp->aux);
>  	if (!ret)
> -		drm_dp_cec_register_connector(&intel_dp->aux,
> -					      connector->name, dev->dev);
> +		drm_dp_cec_register_connector(&intel_dp->aux, connector);
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 4116ee62adafe..4438824ca88b0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,
>  	switch (type) {
>  	case DRM_MODE_CONNECTOR_DisplayPort:
>  	case DRM_MODE_CONNECTOR_eDP:
> -		drm_dp_cec_register_connector(&nv_connector->aux,
> -					      connector->name, dev->dev);
> +		drm_dp_cec_register_connector(&nv_connector->aux, connector);
>  		break;
>  	}
>  
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 3fc534ee81746..729af0b812909 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1221,6 +1221,7 @@ struct drm_dp_aux_msg {
>  
>  struct cec_adapter;
>  struct edid;
> +struct drm_connector;
>  
>  /**
>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
> @@ -1233,8 +1234,7 @@ struct edid;
>  struct drm_dp_aux_cec {
>  	struct mutex lock;
>  	struct cec_adapter *adap;
> -	const char *name;
> -	struct device *parent;
> +	struct drm_connector *connector;
>  	struct delayed_work unregister_work;
>  };
>  
> @@ -1431,8 +1431,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
>  
>  #ifdef CONFIG_DRM_DP_CEC
>  void drm_dp_cec_irq(struct drm_dp_aux *aux);
> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
> -				   struct device *parent);
> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +				   struct drm_connector *connector);
>  void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
>  void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
> @@ -1441,9 +1441,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux)
>  {
>  }
>  
> -static inline void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> -						 const char *name,
> -						 struct device *parent)
> +static inline void
> +drm_dp_cec_register_connector(struct drm_dp_aux *aux,
> +			      struct drm_connector *connector)
>  {
>  }
>  
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 4/9] tda9950: use cec_notifier_cec_adap_(un)register
  2019-08-07  8:52 ` [PATCH v5 4/9] tda9950: " Dariusz Marcinkiewicz
@ 2019-08-08  9:11   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08  9:11 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
> Use the new cec_notifier_cec_adap_(un)register() functions to
> (un)register the notifier for the CEC adapter.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with a BeagleBone Black board.

Regards,

	Hans

> ---
>  drivers/gpu/drm/i2c/tda9950.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> index 250b5e02a314a..e9f6171c47792 100644
> --- a/drivers/gpu/drm/i2c/tda9950.c
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -423,7 +423,8 @@ static int tda9950_probe(struct i2c_client *client,
>  		priv->hdmi = glue->parent;
>  
>  	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
> -					  CEC_CAP_DEFAULTS,
> +					  CEC_CAP_DEFAULTS |
> +					  CEC_CAP_CONNECTOR_INFO,
>  					  CEC_MAX_LOG_ADDRS);
>  	if (IS_ERR(priv->adap))
>  		return PTR_ERR(priv->adap);
> @@ -460,13 +461,14 @@ static int tda9950_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		return ret;
>  
> -	priv->notify = cec_notifier_get(priv->hdmi);
> +	priv->notify = cec_notifier_cec_adap_register(priv->hdmi, NULL,
> +						      priv->adap);
>  	if (!priv->notify)
>  		return -ENOMEM;
>  
>  	ret = cec_register_adapter(priv->adap, priv->hdmi);
>  	if (ret < 0) {
> -		cec_notifier_put(priv->notify);
> +		cec_notifier_cec_adap_unregister(priv->notify);
>  		return ret;
>  	}
>  
> @@ -476,8 +478,6 @@ static int tda9950_probe(struct i2c_client *client,
>  	 */
>  	devm_remove_action(dev, tda9950_cec_del, priv);
>  
> -	cec_register_cec_notifier(priv->adap, priv->notify);
> -
>  	return 0;
>  }
>  
> @@ -485,8 +485,8 @@ static int tda9950_remove(struct i2c_client *client)
>  {
>  	struct tda9950_priv *priv = i2c_get_clientdata(client);
>  
> +	cec_notifier_cec_adap_unregister(priv->notify);
>  	cec_unregister_adapter(priv->adap);
> -	cec_notifier_put(priv->notify);
>  
>  	return 0;
>  }
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-07  8:52 ` [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-08  9:12   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08  9:12 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

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 <darekm@google.com>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

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;
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 ` [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-08 11:47   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08 11:47 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

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.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with an Udoo-X86 board.

Regards,

	Hans

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 187a2b828b975..db7953ce49c8e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2752,8 +2752,9 @@ intel_hdmi_connector_register(struct drm_connector *connector)
>  
>  static void intel_hdmi_destroy(struct drm_connector *connector)
>  {
> -	if (intel_attached_hdmi(connector)->cec_notifier)
> -		cec_notifier_put(intel_attached_hdmi(connector)->cec_notifier);
> +	struct cec_notifier *n = intel_attached_hdmi(connector)->cec_notifier;
> +
> +	cec_notifier_conn_unregister(n);
>  
>  	intel_connector_destroy(connector);
>  }
> @@ -3061,6 +3062,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  	struct drm_device *dev = intel_encoder->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum port port = intel_encoder->port;
> +	struct cec_connector_info conn_info;
>  
>  	DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
>  		      port_name(port));
> @@ -3113,8 +3115,11 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>  	}
>  
> -	intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
> -							 port_identifier(port));
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +
> +	intel_hdmi->cec_notifier =
> +		cec_notifier_conn_register(dev->dev, port_identifier(port),
> +					   &conn_info);
>  	if (!intel_hdmi->cec_notifier)
>  		DRM_DEBUG_KMS("CEC notifier get failed\n");
>  }
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 ` [PATCH v5 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
@ 2019-08-08 14:27   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08 14:27 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

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:
> 	- removed unnecessary call to invalidate phys address before
> 	deregistering the notifier,
> 	- use cec_notifier_phys_addr_invalidate instead of setting
> 	invalid address on a notifier.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with an Odroid U3.

Regards,

	Hans

> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 31 ++++++++++++++++------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 19c252f659dd0..5fd2551a93567 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -858,6 +858,10 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
>  
>  static void hdmi_connector_destroy(struct drm_connector *connector)
>  {
> +	struct hdmi_context *hdata = connector_to_hdmi(connector);
> +
> +	cec_notifier_conn_unregister(hdata->notifier);
> +
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);
>  }
> @@ -941,6 +945,7 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  {
>  	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
>  	struct drm_connector *connector = &hdata->connector;
> +	struct cec_connector_info conn_info;
>  	int ret;
>  
>  	connector->interlace_allowed = true;
> @@ -963,6 +968,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  			DRM_DEV_ERROR(hdata->dev, "Failed to attach bridge\n");
>  	}
>  
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +
> +	hdata->notifier = cec_notifier_conn_register(hdata->dev, NULL,
> +						     &conn_info);
> +	if (hdata->notifier == NULL) {
> +		ret = -ENOMEM;
> +		DRM_DEV_ERROR(hdata->dev, "Failed to allocate CEC notifier\n");
> +	}
> +
>  	return ret;
>  }
>  
> @@ -1534,8 +1548,8 @@ static void hdmi_disable(struct drm_encoder *encoder)
>  		 */
>  		mutex_unlock(&hdata->mutex);
>  		cancel_delayed_work(&hdata->hotplug_work);
> -		cec_notifier_set_phys_addr(hdata->notifier,
> -					   CEC_PHYS_ADDR_INVALID);
> +		if (hdata->notifier)
> +			cec_notifier_phys_addr_invalidate(hdata->notifier);
>  		return;
>  	}
>  
> @@ -2012,12 +2026,6 @@ static int hdmi_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	hdata->notifier = cec_notifier_get(&pdev->dev);
> -	if (hdata->notifier == NULL) {
> -		ret = -ENOMEM;
> -		goto err_hdmiphy;
> -	}
> -
>  	pm_runtime_enable(dev);
>  
>  	audio_infoframe = &hdata->audio.infoframe;
> @@ -2029,7 +2037,7 @@ static int hdmi_probe(struct platform_device *pdev)
>  
>  	ret = hdmi_register_audio_device(hdata);
>  	if (ret)
> -		goto err_notifier_put;
> +		goto err_runtime_disable;
>  
>  	ret = component_add(&pdev->dev, &hdmi_component_ops);
>  	if (ret)
> @@ -2040,8 +2048,7 @@ static int hdmi_probe(struct platform_device *pdev)
>  err_unregister_audio:
>  	platform_device_unregister(hdata->audio.pdev);
>  
> -err_notifier_put:
> -	cec_notifier_put(hdata->notifier);
> +err_runtime_disable:
>  	pm_runtime_disable(dev);
>  
>  err_hdmiphy:
> @@ -2060,12 +2067,10 @@ static int hdmi_remove(struct platform_device *pdev)
>  	struct hdmi_context *hdata = platform_get_drvdata(pdev);
>  
>  	cancel_delayed_work_sync(&hdata->hotplug_work);
> -	cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
>  
>  	component_del(&pdev->dev, &hdmi_component_ops);
>  	platform_device_unregister(hdata->audio.pdev);
>  
> -	cec_notifier_put(hdata->notifier);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	if (!IS_ERR(hdata->reg_hdmi_en))
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-07  8:52 ` [PATCH v5 7/9] drm: tegra: " Dariusz Marcinkiewicz
@ 2019-08-08 20:42   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-08 20:42 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

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 v4:
> 	- only create a CEC notifier for HDMI connectors
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with a Jetson TK1 board.

Regards,

	Hans

> ---
>  drivers/gpu/drm/tegra/output.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index 9c2b9dad55c30..13153b93626fd 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -80,6 +80,11 @@ tegra_output_connector_detect(struct drm_connector *connector, bool force)
>  
>  void tegra_output_connector_destroy(struct drm_connector *connector)
>  {
> +	struct tegra_output *output = connector_to_output(connector);
> +
> +	if (output->cec)
> +		cec_notifier_conn_unregister(output->cec);
> +
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);
>  }
> @@ -174,18 +179,11 @@ int tegra_output_probe(struct tegra_output *output)
>  		disable_irq(output->hpd_irq);
>  	}
>  
> -	output->cec = cec_notifier_get(output->dev);
> -	if (!output->cec)
> -		return -ENOMEM;
> -
>  	return 0;
>  }
>  
>  void tegra_output_remove(struct tegra_output *output)
>  {
> -	if (output->cec)
> -		cec_notifier_put(output->cec);
> -
>  	if (gpio_is_valid(output->hpd_gpio)) {
>  		free_irq(output->hpd_irq, output);
>  		gpio_free(output->hpd_gpio);
> @@ -197,6 +195,7 @@ void tegra_output_remove(struct tegra_output *output)
>  
>  int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  {
> +	int connector_type;
>  	int err;
>  
>  	if (output->panel) {
> @@ -212,6 +211,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  	if (gpio_is_valid(output->hpd_gpio))
>  		enable_irq(output->hpd_irq);
>  
> +	connector_type = output->connector.connector_type;
> +	/*
> +	 * Create a CEC notifier for HDMI connector.
> +	 */
> +	if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> +	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> +		struct cec_connector_info conn_info;
> +
> +		cec_fill_conn_info_from_drm(&conn_info, &output->connector);
> +		output->cec = cec_notifier_conn_register(output->dev, NULL,
> +							 &conn_info);
> +		if (!output->cec)
> +			return -ENOMEM;
> +	}
> +
>  	return 0;
>  }
>  
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  2019-08-07  8:52 ` [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
@ 2019-08-09 10:25   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-09 10:25 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
> Use the new cec_notifier_cec_adap_(un)register() functions to
> (un)register the notifier for the CEC adapter.
> 
> Also adds CEC_CAP_CONNECTOR_INFO capability to the adapter.
> 
> Changes since v3:
> 	- add CEC_CAP_CONNECTOR_INFO to cec_allocate_adapter,
> 	- replace CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> 	CEC_CAP_RC | CEC_CAP_PASSTHROUGH with CEC_CAP_DEFAULTS.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested with a Banana Pi M3 Allwinner A83T.

Regards,

	Hans

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> index 6c323510f1288..361acff2111cf 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> @@ -259,8 +259,8 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
>  	dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
>  
>  	cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
> -					 CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> -					 CEC_CAP_RC | CEC_CAP_PASSTHROUGH,
> +					 CEC_CAP_DEFAULTS |
> +					 CEC_CAP_CONNECTOR_INFO,
>  					 CEC_MAX_LOG_ADDRS);
>  	if (IS_ERR(cec->adap))
>  		return PTR_ERR(cec->adap);
> @@ -281,13 +281,14 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	cec->notify = cec_notifier_get(pdev->dev.parent);
> +	cec->notify = cec_notifier_cec_adap_register(pdev->dev.parent,
> +						     NULL, cec->adap);
>  	if (!cec->notify)
>  		return -ENOMEM;
>  
>  	ret = cec_register_adapter(cec->adap, pdev->dev.parent);
>  	if (ret < 0) {
> -		cec_notifier_put(cec->notify);
> +		cec_notifier_cec_adap_unregister(cec->notify);
>  		return ret;
>  	}
>  
> @@ -297,8 +298,6 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
>  	 */
>  	devm_remove_action(&pdev->dev, dw_hdmi_cec_del, cec);
>  
> -	cec_register_cec_notifier(cec->adap, cec->notify);
> -
>  	return 0;
>  }
>  
> @@ -306,8 +305,8 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev)
>  {
>  	struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
>  
> +	cec_notifier_cec_adap_unregister(cec->notify);
>  	cec_unregister_adapter(cec->adap);
> -	cec_notifier_put(cec->notify);
>  
>  	return 0;
>  }
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 1/9] drm_dp_cec: add connector info support.
  2019-08-08  8:18   ` Hans Verkuil
@ 2019-08-09 10:30     ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-09 10:30 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

On 8/8/19 10:18 AM, Hans Verkuil wrote:
> Hi Dariusz,
> 
> On 8/7/19 10:52 AM, Dariusz Marcinkiewicz wrote:
>> Pass the connector info to the CEC adapter. This makes it possible
>> to associate the CEC adapter with the corresponding drm connector.
>>
>> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>>  drivers/gpu/drm/i915/intel_dp.c               |  4 +--
>>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>>  include/drm/drm_dp_helper.h                   | 14 +++++------
>>  5 files changed, 26 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index 6e205ee36ac3b..7f2eb4eb1035b 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -394,7 +394,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>>  
>>  	drm_dp_aux_register(&aconnector->dm_dp_aux.aux);
>>  	drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
>> -				      aconnector->base.name, dm->adev->dev);
>> +				      &aconnector->base);
>>  	aconnector->mst_mgr.cbs = &dm_mst_cbs;
>>  	drm_dp_mst_topology_mgr_init(
>>  		&aconnector->mst_mgr,
>> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
>> index b15cee85b702b..b457c16c3a8bb 100644
>> --- a/drivers/gpu/drm/drm_dp_cec.c
>> +++ b/drivers/gpu/drm/drm_dp_cec.c
>> @@ -8,7 +8,9 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/slab.h>
>> +#include <drm/drm_connector.h>
>>  #include <drm/drm_dp_helper.h>
>> +#include <drm/drmP.h>
>>  #include <media/cec.h>
>>  
>>  /*
>> @@ -295,7 +297,10 @@ static void drm_dp_cec_unregister_work(struct work_struct *work)
>>   */
>>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>>  {
>> -	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
>> +	struct drm_connector *connector = aux->cec.connector;
>> +	u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD |
>> +		       CEC_CAP_CONNECTOR_INFO;
>> +	struct cec_connector_info conn_info;
>>  	unsigned int num_las = 1;
>>  	u8 cap;
>>  
>> @@ -344,13 +349,17 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
>>  
>>  	/* Create a new adapter */
>>  	aux->cec.adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
>> -					     aux, aux->cec.name, cec_caps,
>> +					     aux, connector->name, cec_caps,
>>  					     num_las);
>>  	if (IS_ERR(aux->cec.adap)) {
>>  		aux->cec.adap = NULL;
>>  		goto unlock;
>>  	}
>> -	if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
>> +
>> +	cec_fill_conn_info_from_drm(&conn_info, connector);
>> +	cec_s_conn_info(aux->cec.adap, &conn_info);
>> +
>> +	if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
>>  		cec_delete_adapter(aux->cec.adap);
>>  		aux->cec.adap = NULL;
>>  	} else {
>> @@ -406,22 +415,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
>>  /**
>>   * drm_dp_cec_register_connector() - register a new connector
>>   * @aux: DisplayPort AUX channel
>> - * @name: name of the CEC device
>> - * @parent: parent device
>> + * @connector: drm connector
>>   *
>>   * A new connector was registered with associated CEC adapter name and
>>   * CEC adapter parent device. After registering the name and parent
>>   * drm_dp_cec_set_edid() is called to check if the connector supports
>>   * CEC and to register a CEC adapter if that is the case.
>>   */
>> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
>> -				   struct device *parent)
>> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
>> +				   struct drm_connector *connector)
>>  {
>>  	WARN_ON(aux->cec.adap);
>>  	if (WARN_ON(!aux->transfer))
>>  		return;
>> -	aux->cec.name = name;
>> -	aux->cec.parent = parent;
>> +	aux->cec.connector = connector;
>>  	INIT_DELAYED_WORK(&aux->cec.unregister_work,
>>  			  drm_dp_cec_unregister_work);
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4336df46fe782..79c8afdbc4869 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -5693,7 +5693,6 @@ static int
>>  intel_dp_connector_register(struct drm_connector *connector)
>>  {
>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> -	struct drm_device *dev = connector->dev;
>>  	int ret;
>>  
>>  	ret = intel_connector_register(connector);
>> @@ -5708,8 +5707,7 @@ intel_dp_connector_register(struct drm_connector *connector)
>>  	intel_dp->aux.dev = connector->kdev;
>>  	ret = drm_dp_aux_register(&intel_dp->aux);
>>  	if (!ret)
>> -		drm_dp_cec_register_connector(&intel_dp->aux,
>> -					      connector->name, dev->dev);
>> +		drm_dp_cec_register_connector(&intel_dp->aux, connector);
>>  	return ret;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
>> index 4116ee62adafe..4438824ca88b0 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
>> @@ -1413,8 +1413,7 @@ nouveau_connector_create(struct drm_device *dev,
>>  	switch (type) {
>>  	case DRM_MODE_CONNECTOR_DisplayPort:
>>  	case DRM_MODE_CONNECTOR_eDP:
>> -		drm_dp_cec_register_connector(&nv_connector->aux,
>> -					      connector->name, dev->dev);
>> +		drm_dp_cec_register_connector(&nv_connector->aux, connector);
>>  		break;
>>  	}
>>  
>> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> index 3fc534ee81746..729af0b812909 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1221,6 +1221,7 @@ struct drm_dp_aux_msg {
>>  
>>  struct cec_adapter;
>>  struct edid;
>> +struct drm_connector;
>>  
>>  /**
>>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
>> @@ -1233,8 +1234,7 @@ struct edid;
>>  struct drm_dp_aux_cec {
>>  	struct mutex lock;
>>  	struct cec_adapter *adap;
>> -	const char *name;
>> -	struct device *parent;
>> +	struct drm_connector *connector;
>>  	struct delayed_work unregister_work;
>>  };
> 
> You probably saw the kbuild test robot mail as well: the comments before this
> struct need to be updated.
> 
> Also, this series is against an older tree: intel_dp/hdmi.c is now moved to
> a i915/display/ directory.
> 
> Can you rebase and post a v6 for this series? Make sure to post to dri-devel as
> well! The various drm maintainers of these drivers have to review the code as well
> and give an Acked-by! I noticed that you only posted it to linux-media, but
> that's not enough. In fact, it would be great if you can Cc each patch to the
> corresponding maintainer as well (see the MAINTAINERS file).
> 
> I've started testing this series on my hardware and I will post Tested-by tags
> whenever I finished testing each patch for which I have HW.

When you make a v6, please don't forget to add my Tested-by lines!

There is only one more patch that I need to test (8/9), I'll try to do
that tomorrow.

Patch 6/9 (sti) I cannot test since I don't have the hardware. Still,
being able to test 8 out of 9 patches ain't bad and is a clear sign
I have way too much hardware :-)

Regards,

	Hans

> 
> Thanks!
> 
> 	Hans
> 
>>  
>> @@ -1431,8 +1431,8 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
>>  
>>  #ifdef CONFIG_DRM_DP_CEC
>>  void drm_dp_cec_irq(struct drm_dp_aux *aux);
>> -void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
>> -				   struct device *parent);
>> +void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
>> +				   struct drm_connector *connector);
>>  void drm_dp_cec_unregister_connector(struct drm_dp_aux *aux);
>>  void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid);
>>  void drm_dp_cec_unset_edid(struct drm_dp_aux *aux);
>> @@ -1441,9 +1441,9 @@ static inline void drm_dp_cec_irq(struct drm_dp_aux *aux)
>>  {
>>  }
>>  
>> -static inline void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
>> -						 const char *name,
>> -						 struct device *parent)
>> +static inline void
>> +drm_dp_cec_register_connector(struct drm_dp_aux *aux,
>> +			      struct drm_connector *connector)
>>  {
>>  }
>>  
>>
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-07  8:52 ` [PATCH v5 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
@ 2019-08-09 15:12   ` Hans Verkuil
  0 siblings, 0 replies; 20+ messages in thread
From: Hans Verkuil @ 2019-08-09 15:12 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, linux-media

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 v4:
> 	- typo fix
> Changes since v2:
> 	- removed unnecessary NULL check before a call to
> 	cec_notifier_conn_unregister,
> 	- use cec_notifier_phys_addr_invalidate to invalidate physical
> 	address.
> 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 <darekm@google.com>

Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Tested on a AML_S905X-CC board.

Note: there is a bug (unrelated to this series) that prevents the physical
address from being invalidated in drivers/gpu/drm/meson/meson_dw_hdmi.c.

I have a patch, but it needs a bit more testing before I'll post it.

Regards,

	Hans

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 36 ++++++++++++++---------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ab7968c8f6a29..b7d0d9ad5f2f7 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2118,6 +2118,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
>  	struct dw_hdmi *hdmi = bridge->driver_private;
>  	struct drm_encoder *encoder = bridge->encoder;
>  	struct drm_connector *connector = &hdmi->connector;
> +	struct cec_connector_info conn_info;
> +	struct cec_notifier *notifier;
>  
>  	connector->interlace_allowed = 1;
>  	connector->polled = DRM_CONNECTOR_POLL_HPD;
> @@ -2129,6 +2131,18 @@ static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
>  
>  	drm_connector_attach_encoder(connector, encoder);
>  
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +
> +	notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info);
> +	if (!notifier)
> +		return -ENOMEM;
> +	/*
> +	 * Make sure that dw_hdmi_irq thread does see the notifier
> +	 * when it fully constructed.
> +	 */
> +	smp_wmb();
> +	hdmi->cec_notifier = notifier;
> +
>  	return 0;
>  }
>  
> @@ -2295,9 +2309,13 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>  				       phy_stat & HDMI_PHY_HPD,
>  				       phy_stat & HDMI_PHY_RX_SENSE);
>  
> -		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
> -			cec_notifier_set_phys_addr(hdmi->cec_notifier,
> -						   CEC_PHYS_ADDR_INVALID);
> +		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
> +			struct cec_notifier *notifier;
> +
> +			notifier = READ_ONCE(hdmi->cec_notifier);
> +			if (notifier)
> +				cec_notifier_phys_addr_invalidate(notifier);
> +		}
>  	}
>  
>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> @@ -2600,12 +2618,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  	if (ret)
>  		goto err_iahb;
>  
> -	hdmi->cec_notifier = cec_notifier_get(dev);
> -	if (!hdmi->cec_notifier) {
> -		ret = -ENOMEM;
> -		goto err_iahb;
> -	}
> -
>  	/*
>  	 * To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
>  	 * N and cts values before enabling phy
> @@ -2693,9 +2705,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		hdmi->ddc = NULL;
>  	}
>  
> -	if (hdmi->cec_notifier)
> -		cec_notifier_put(hdmi->cec_notifier);
> -
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	if (hdmi->cec_clk)
>  		clk_disable_unprepare(hdmi->cec_clk);
> @@ -2717,8 +2726,7 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>  	/* Disable all interrupts */
>  	hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
>  
> -	if (hdmi->cec_notifier)
> -		cec_notifier_put(hdmi->cec_notifier);
> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>  
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	clk_disable_unprepare(hdmi->isfr_clk);
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-08-09 15:12 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  8:52 [PATCH v5 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
2019-08-07  8:52 ` [PATCH v5 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
2019-08-08  8:18   ` Hans Verkuil
2019-08-09 10:30     ` Hans Verkuil
2019-08-08  8:44   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
2019-08-08 11:47   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
2019-08-09 10:25   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 4/9] tda9950: " Dariusz Marcinkiewicz
2019-08-08  9:11   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
2019-08-08  9:12   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 6/9] drm: sti: " Dariusz Marcinkiewicz
2019-08-07  8:52 ` [PATCH v5 7/9] drm: tegra: " Dariusz Marcinkiewicz
2019-08-08 20:42   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
2019-08-09 15:12   ` Hans Verkuil
2019-08-07  8:52 ` [PATCH v5 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
2019-08-08 14:27   ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).