Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
@ 2019-08-14 10:44 Dariusz Marcinkiewicz
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
                   ` (9 more replies)
  0 siblings, 10 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:44 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Alex Deucher, Allison Randal, amd-gfx,
	Andrzej Hajda, Chris Wilson, Colin Ian King, Daniel Vetter,
	David Francis, Dhinakaran Pandiyan, Douglas Anderson,
	Enrico Weigelt, Greg Kroah-Hartman, Hans Verkuil, Harry Wentland,
	Imre Deak, intel-gfx, Jani Nikula, Jernej Skrabec,
	Jerry (Fangzhi) Zuo, Jonas Karlman, Kate Stewart,
	Laurent Pinchart, Leo Li, linux-arm-kernel, linux-kernel,
	linux-samsung-soc, linux-tegra, Lyude Paul, Maarten Lankhorst,
	Manasi Navare, Neil Armstrong, nouveau, Ramalingam C,
	Rodrigo Vivi, Russell King, Sam Ravnborg, Sean Paul,
	Shashank Sharma, Thomas Gleixner, Thomas Lim,
	Ville Syrjälä

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

Changes since v6:
	Made CEC notifiers' registration and de-registration symmetric
	in tda998x and dw-hdmi drivers. Also, accidentally dropped one
	patch in v6 (change to drm_dp_cec), brought it back now.
Changes since v5:
        Fixed a warning about a missing comment for a new member of
	drm_dp_aux_cec struct. Sending to a wider audience,
	including maintainers of respective drivers.
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     | 46 +++++++++++++------
 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             | 36 ++++++++++-----
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 drivers/gpu/drm/i915/display/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                   | 17 ++++---
 13 files changed, 155 insertions(+), 94 deletions(-)

-- 
2.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
@ 2019-08-14 10:44 ` Dariusz Marcinkiewicz
  2019-08-15 18:10   ` Lyude Paul
                     ` (2 more replies)
  2019-08-14 10:45 ` [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
                   ` (8 subsequent siblings)
  9 siblings, 3 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:44 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Harry Wentland, Leo Li, Alex Deucher,
	Christian König, David (ChunMing) Zhou, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Ben Skeggs,
	Lyude Paul, Jerry (Fangzhi) Zuo, Anthony Koo, Thomas Lim,
	David Francis, Ville Syrjälä,
	Chris Wilson, Imre Deak, Manasi Navare, Dhinakaran Pandiyan,
	amd-gfx, linux-kernel, intel-gfx, nouveau

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>
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
 drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
 include/drm/drm_dp_helper.h                   | 17 ++++++-------
 5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
@@ -416,7 +416,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/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1092499115760..de2486fe7bf2d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5497,7 +5497,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);
@@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
 
 struct cec_adapter;
 struct edid;
+struct drm_connector;
 
 /**
  * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
  * @lock: mutex protecting this struct
  * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
- * @name: name of the CEC adapter
- * @parent: parent device of the CEC adapter
+ * @connector: the connector this CEC adapter is associated with
  * @unregister_work: unregister the CEC adapter
  */
 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;
 };
 
@@ -1451,8 +1450,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);
@@ -1461,9 +1460,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` Dariusz Marcinkiewicz
  2019-08-22  8:03   ` Hans Verkuil
  2019-08-26 12:08   ` Ville Syrjälä
  2019-08-14 10:45 ` [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, David Airlie, Daniel Vetter,
	Ville Syrjälä,
	Chris Wilson, Maarten Lankhorst, Shashank Sharma, Ramalingam C,
	intel-gfx, linux-kernel

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>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b1ca8e5bdb56d..9fcf2c58c29c5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/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);
 }
@@ -3068,6 +3069,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));
@@ -3120,8 +3122,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
  2019-08-14 10:45 ` [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` Dariusz Marcinkiewicz
  2019-08-19 14:35   ` Neil Armstrong
  2019-08-14 10:45 ` [PATCH v7 4/9] tda9950: " Dariusz Marcinkiewicz
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Andrzej Hajda, Neil Armstrong,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Kate Stewart, Allison Randal, Greg Kroah-Hartman,
	Thomas Gleixner, linux-kernel

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>
---
 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 0f949978d3fcd..ac1e001d08829 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -256,8 +256,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);
@@ -278,13 +278,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;
 	}
 
@@ -294,8 +295,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;
 }
 
@@ -303,8 +302,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 4/9] tda9950: use cec_notifier_cec_adap_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (2 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` " Dariusz Marcinkiewicz
  2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, David Airlie, Daniel Vetter, Russell King,
	Hans Verkuil, Greg Kroah-Hartman, Enrico Weigelt,
	Thomas Gleixner, Colin Ian King, linux-kernel

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>
---
 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 8039fc0d83db4..a5a75bdeb7a5f 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -420,7 +420,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);
@@ -457,13 +458,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;
 	}
 
@@ -473,8 +475,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;
 }
 
@@ -482,8 +482,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (3 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 4/9] tda9950: " Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` Dariusz Marcinkiewicz
  2019-08-19  9:30   ` Hans Verkuil
                     ` (2 more replies)
  2019-08-14 10:45 ` [PATCH v7 6/9] drm: sti: " Dariusz Marcinkiewicz
                   ` (4 subsequent siblings)
  9 siblings, 3 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Russell King, David Airlie, Daniel Vetter,
	linux-kernel

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 v6:
        - move cec_notifier_conn_unregister to tda998x_bridge_detach,
	- add a mutex protecting accesses to a CEC notifier.
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 | 36 +++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 61e042918a7fc..643480415473f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -82,6 +82,8 @@ struct tda998x_priv {
 	u8 audio_port_enable[AUDIO_ROUTE_NUM];
 	struct tda9950_glue cec_glue;
 	struct gpio_desc *calib;
+
+	struct mutex cec_notifiy_mutex;
 	struct cec_notifier *cec_notify;
 };
 
@@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
 				tda998x_edid_delay_start(priv);
 			} else {
 				schedule_work(&priv->detect_work);
-				cec_notifier_set_phys_addr(priv->cec_notify,
-						   CEC_PHYS_ADDR_INVALID);
+
+				mutex_lock(&priv->cec_notifiy_mutex);
+				cec_notifier_phys_addr_invalidate(
+						priv->cec_notify);
+				mutex_unlock(&priv->cec_notifiy_mutex);
 			}
 
 			handled = true;
@@ -1331,6 +1336,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;
@@ -1347,6 +1354,16 @@ 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);
+		return -ENOMEM;
+
+	mutex_lock(&priv->cec_notifiy_mutex);
+	priv->cec_notify = notifier;
+	mutex_unlock(&priv->cec_notifiy_mutex);
+
 	drm_connector_attach_encoder(&priv->connector,
 				     priv->bridge.encoder);
 
@@ -1366,6 +1383,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
+	mutex_lock(&priv->cec_notifiy_mutex);
+	cec_notifier_conn_unregister(priv->cec_notify);
+	priv->cec_notify = NULL;
+	mutex_unlock(&priv->cec_notifiy_mutex);
+
 	drm_connector_cleanup(&priv->connector);
 }
 
@@ -1789,9 +1811,6 @@ static void tda998x_destroy(struct device *dev)
 	cancel_work_sync(&priv->detect_work);
 
 	i2c_unregister_device(priv->cec);
-
-	if (priv->cec_notify)
-		cec_notifier_put(priv->cec_notify);
 }
 
 static int tda998x_create(struct device *dev)
@@ -1812,6 +1831,7 @@ static int tda998x_create(struct device *dev)
 	mutex_init(&priv->mutex);	/* protect the page access */
 	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
 	mutex_init(&priv->edid_mutex);
+	mutex_init(&priv->cec_notifiy_mutex);
 	INIT_LIST_HEAD(&priv->bridge.list);
 	init_waitqueue_head(&priv->edid_delay_waitq);
 	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
@@ -1916,12 +1936,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 6/9] drm: sti: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (4 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` " Dariusz Marcinkiewicz
  2019-08-19  9:34   ` Hans Verkuil
  2019-08-22  8:11   ` Hans Verkuil
  2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Benjamin Gaignard, Vincent Abriou,
	David Airlie, Daniel Vetter, linux-kernel

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 9862c322f0c4a..bd15902b825ad 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1256,6 +1256,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;
@@ -1318,6 +1319,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);
 
@@ -1331,6 +1340,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 = {
@@ -1436,10 +1448,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))
@@ -1459,14 +1467,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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (5 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 6/9] drm: sti: " Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` " Dariusz Marcinkiewicz
  2019-08-19  9:33   ` Hans Verkuil
                     ` (2 more replies)
  2019-08-14 10:45 ` [PATCH v7 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
                   ` (2 subsequent siblings)
  9 siblings, 3 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Thierry Reding, David Airlie,
	Daniel Vetter, Jonathan Hunter, linux-tegra, linux-kernel

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>
---
 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 bdcaa4c7168cf..34373734ff689 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -70,6 +70,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);
 }
@@ -163,18 +168,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 (output->hpd_gpio)
 		free_irq(output->hpd_irq, output);
 
@@ -184,6 +182,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) {
@@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
 	if (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.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (6 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` " Dariusz Marcinkiewicz
  2019-08-19  9:32   ` Hans Verkuil
  2019-08-14 10:45 ` [PATCH v7 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
  2019-08-19  9:38 ` [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Hans Verkuil
  9 siblings, 1 reply; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Andrzej Hajda, Neil Armstrong,
	Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
	Daniel Vetter, Sam Ravnborg, Sean Paul, Douglas Anderson,
	Heiko Stuebner, linux-kernel

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 v6:
        - move cec_notifier_conn_unregister to a bridge detach
	  function,
	- add a mutex protecting a CEC notifier.
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 | 45 +++++++++++++++--------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 83b94b66e464e..55162c9092f71 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -190,6 +190,7 @@ struct dw_hdmi {
 	void (*enable_audio)(struct dw_hdmi *hdmi);
 	void (*disable_audio)(struct dw_hdmi *hdmi);
 
+	struct mutex cec_notifier_mutex;
 	struct cec_notifier *cec_notifier;
 };
 
@@ -2194,6 +2195,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;
@@ -2207,9 +2210,29 @@ 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;
+
+	mutex_lock(&hdmi->cec_notifier_mutex);
+	hdmi->cec_notifier = notifier;
+	mutex_unlock(&hdmi->cec_notifier_mutex);
+
 	return 0;
 }
 
+static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
+{
+	struct dw_hdmi *hdmi = bridge->driver_private;
+
+	mutex_lock(&hdmi->cec_notifier_mutex);
+	cec_notifier_conn_unregister(hdmi->cec_notifier);
+	hdmi->cec_notifier = NULL;
+	mutex_unlock(&hdmi->cec_notifier_mutex);
+}
+
 static enum drm_mode_status
 dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
 			  const struct drm_display_mode *mode)
@@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
 
 static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
 	.attach = dw_hdmi_bridge_attach,
+	.detach = dw_hdmi_bridge_detach,
 	.enable = dw_hdmi_bridge_enable,
 	.disable = dw_hdmi_bridge_disable,
 	.mode_set = dw_hdmi_bridge_mode_set,
@@ -2373,9 +2397,11 @@ 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) {
+			mutex_lock(&hdmi->cec_notifier_mutex);
+			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
+			mutex_unlock(&hdmi->cec_notifier_mutex);
+		}
 	}
 
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
@@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 
 	mutex_init(&hdmi->mutex);
 	mutex_init(&hdmi->audio_mutex);
+	mutex_init(&hdmi->cec_notifier_mutex);
 	spin_lock_init(&hdmi->audio_lock);
 
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
@@ -2693,12 +2720,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
@@ -2796,9 +2817,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);
@@ -2820,9 +2838,6 @@ 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);
-
 	clk_disable_unprepare(hdmi->iahb_clk);
 	clk_disable_unprepare(hdmi->isfr_clk);
 	if (hdmi->cec_clk)
-- 
2.23.0.rc1.153.gdeed80330f-goog


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

* [PATCH v7 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (7 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
@ 2019-08-14 10:45 ` " Dariusz Marcinkiewicz
  2019-08-19  9:32   ` Hans Verkuil
  2019-08-28  8:39   ` Sylwester Nawrocki
  2019-08-19  9:38 ` [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Hans Verkuil
  9 siblings, 2 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-14 10:45 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Inki Dae, Joonyoung Shim, Seung-Woo Kim,
	Kyungmin Park, David Airlie, Daniel Vetter, Kukjin Kim,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-kernel

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>
---
 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 bc1565f1822ab..d532b468d9af5 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -852,6 +852,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);
 }
@@ -935,6 +939,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;
@@ -957,6 +962,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;
 }
 
@@ -1528,8 +1542,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;
 	}
 
@@ -2006,12 +2020,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;
@@ -2023,7 +2031,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)
@@ -2034,8 +2042,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:
@@ -2054,12 +2061,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.23.0.rc1.153.gdeed80330f-goog


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

* Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
@ 2019-08-15 18:10   ` Lyude Paul
  2019-08-26  9:05     ` Ben Skeggs
  2019-08-22  8:08   ` Hans Verkuil
  2019-08-28 15:05   ` Ville Syrjälä
  2 siblings, 1 reply; 49+ messages in thread
From: Lyude Paul @ 2019-08-15 18:10 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media, hverkuil-cisco
  Cc: Harry Wentland, Leo Li, Alex Deucher, Christian König,
	David (ChunMing) Zhou, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Sean Paul, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Ben Skeggs, Jerry (Fangzhi) Zuo,
	Anthony Koo, Thomas Lim, David Francis, Ville Syrjälä,
	Chris Wilson, Imre Deak, Manasi Navare, Dhinakaran Pandiyan,
	amd-gfx, linux-kernel, intel-gfx, nouveau

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Wed, 2019-08-14 at 12:44 +0200, 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>
> ---
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>  include/drm/drm_dp_helper.h                   | 17 ++++++-------
>  5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
> @@ -416,7 +416,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/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1092499115760..de2486fe7bf2d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5497,7 +5497,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);
> @@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
>  
>  struct cec_adapter;
>  struct edid;
> +struct drm_connector;
>  
>  /**
>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
>   * @lock: mutex protecting this struct
>   * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> - * @name: name of the CEC adapter
> - * @parent: parent device of the CEC adapter
> + * @connector: the connector this CEC adapter is associated with
>   * @unregister_work: unregister the CEC adapter
>   */
>  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;
>  };
>  
> @@ -1451,8 +1450,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);
> @@ -1461,9 +1460,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] 49+ messages in thread

* Re: [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-19  9:30   ` Hans Verkuil
  2019-08-19 11:22   ` [PATCH v7.1 " Dariusz Marcinkiewicz
  2019-08-25 13:12   ` [PATCH v7 " Hans Verkuil
  2 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:30 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, Russell King, linux-kernel

On 8/14/19 12:45 PM, 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 v6:
>         - move cec_notifier_conn_unregister to tda998x_bridge_detach,
> 	- add a mutex protecting accesses to a CEC notifier.
> 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 | 36 +++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 61e042918a7fc..643480415473f 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -82,6 +82,8 @@ struct tda998x_priv {
>  	u8 audio_port_enable[AUDIO_ROUTE_NUM];
>  	struct tda9950_glue cec_glue;
>  	struct gpio_desc *calib;
> +
> +	struct mutex cec_notifiy_mutex;

Typo: notifiy -> notify

Regards,

	Hans

>  	struct cec_notifier *cec_notify;
>  };
>  
> @@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
>  				tda998x_edid_delay_start(priv);
>  			} else {
>  				schedule_work(&priv->detect_work);
> -				cec_notifier_set_phys_addr(priv->cec_notify,
> -						   CEC_PHYS_ADDR_INVALID);
> +
> +				mutex_lock(&priv->cec_notifiy_mutex);
> +				cec_notifier_phys_addr_invalidate(
> +						priv->cec_notify);
> +				mutex_unlock(&priv->cec_notifiy_mutex);
>  			}
>  
>  			handled = true;
> @@ -1331,6 +1336,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;
> @@ -1347,6 +1354,16 @@ 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);
> +		return -ENOMEM;
> +
> +	mutex_lock(&priv->cec_notifiy_mutex);
> +	priv->cec_notify = notifier;
> +	mutex_unlock(&priv->cec_notifiy_mutex);
> +
>  	drm_connector_attach_encoder(&priv->connector,
>  				     priv->bridge.encoder);
>  
> @@ -1366,6 +1383,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge)
>  {
>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>  
> +	mutex_lock(&priv->cec_notifiy_mutex);
> +	cec_notifier_conn_unregister(priv->cec_notify);
> +	priv->cec_notify = NULL;
> +	mutex_unlock(&priv->cec_notifiy_mutex);
> +
>  	drm_connector_cleanup(&priv->connector);
>  }
>  
> @@ -1789,9 +1811,6 @@ static void tda998x_destroy(struct device *dev)
>  	cancel_work_sync(&priv->detect_work);
>  
>  	i2c_unregister_device(priv->cec);
> -
> -	if (priv->cec_notify)
> -		cec_notifier_put(priv->cec_notify);
>  }
>  
>  static int tda998x_create(struct device *dev)
> @@ -1812,6 +1831,7 @@ static int tda998x_create(struct device *dev)
>  	mutex_init(&priv->mutex);	/* protect the page access */
>  	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
>  	mutex_init(&priv->edid_mutex);
> +	mutex_init(&priv->cec_notifiy_mutex);
>  	INIT_LIST_HEAD(&priv->bridge.list);
>  	init_waitqueue_head(&priv->edid_delay_waitq);
>  	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
> @@ -1916,12 +1936,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] 49+ messages in thread

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
@ 2019-08-19  9:32   ` Hans Verkuil
  2019-08-19 14:05     ` Hans Verkuil
  0 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:32 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Neil Armstrong,
	Douglas Anderson, linux-kernel, Sean Paul, Laurent Pinchart,
	Sam Ravnborg

On 8/14/19 12:45 PM, 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 v6:
>         - move cec_notifier_conn_unregister to a bridge detach
> 	  function,
> 	- add a mutex protecting a CEC notifier.
> 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>

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

Regards,

	Hans

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 83b94b66e464e..55162c9092f71 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -190,6 +190,7 @@ struct dw_hdmi {
>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>  
> +	struct mutex cec_notifier_mutex;
>  	struct cec_notifier *cec_notifier;
>  };
>  
> @@ -2194,6 +2195,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;
> @@ -2207,9 +2210,29 @@ 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;
> +
> +	mutex_lock(&hdmi->cec_notifier_mutex);
> +	hdmi->cec_notifier = notifier;
> +	mutex_unlock(&hdmi->cec_notifier_mutex);
> +
>  	return 0;
>  }
>  
> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
> +{
> +	struct dw_hdmi *hdmi = bridge->driver_private;
> +
> +	mutex_lock(&hdmi->cec_notifier_mutex);
> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
> +	hdmi->cec_notifier = NULL;
> +	mutex_unlock(&hdmi->cec_notifier_mutex);
> +}
> +
>  static enum drm_mode_status
>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>  			  const struct drm_display_mode *mode)
> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>  
>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>  	.attach = dw_hdmi_bridge_attach,
> +	.detach = dw_hdmi_bridge_detach,
>  	.enable = dw_hdmi_bridge_enable,
>  	.disable = dw_hdmi_bridge_disable,
>  	.mode_set = dw_hdmi_bridge_mode_set,
> @@ -2373,9 +2397,11 @@ 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) {
> +			mutex_lock(&hdmi->cec_notifier_mutex);
> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
> +			mutex_unlock(&hdmi->cec_notifier_mutex);
> +		}
>  	}
>  
>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  
>  	mutex_init(&hdmi->mutex);
>  	mutex_init(&hdmi->audio_mutex);
> +	mutex_init(&hdmi->cec_notifier_mutex);
>  	spin_lock_init(&hdmi->audio_lock);
>  
>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
> @@ -2693,12 +2720,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
> @@ -2796,9 +2817,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);
> @@ -2820,9 +2838,6 @@ 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);
> -
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	clk_disable_unprepare(hdmi->isfr_clk);
>  	if (hdmi->cec_clk)
> 


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

* Re: [PATCH v7 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
@ 2019-08-19  9:32   ` Hans Verkuil
  2019-08-28  8:39   ` Sylwester Nawrocki
  1 sibling, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:32 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: linux-samsung-soc, David Airlie, Seung-Woo Kim, linux-kernel,
	Krzysztof Kozlowski, Kyungmin Park, Kukjin Kim, linux-arm-kernel

On 8/14/19 12:45 PM, 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>

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

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 bc1565f1822ab..d532b468d9af5 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -852,6 +852,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);
>  }
> @@ -935,6 +939,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;
> @@ -957,6 +962,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;
>  }
>  
> @@ -1528,8 +1542,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;
>  	}
>  
> @@ -2006,12 +2020,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;
> @@ -2023,7 +2031,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)
> @@ -2034,8 +2042,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:
> @@ -2054,12 +2061,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] 49+ messages in thread

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
@ 2019-08-19  9:33   ` Hans Verkuil
  2019-08-28  8:09   ` Hans Verkuil
  2019-08-28  9:36   ` Thierry Reding
  2 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:33 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, linux-kernel, Jonathan Hunter, Thierry Reding, linux-tegra

On 8/14/19 12:45 PM, 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>

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

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 bdcaa4c7168cf..34373734ff689 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -70,6 +70,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);
>  }
> @@ -163,18 +168,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 (output->hpd_gpio)
>  		free_irq(output->hpd_irq, output);
>  
> @@ -184,6 +182,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) {
> @@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  	if (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] 49+ messages in thread

* Re: [PATCH v7 6/9] drm: sti: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 6/9] drm: sti: " Dariusz Marcinkiewicz
@ 2019-08-19  9:34   ` Hans Verkuil
  2019-08-22  8:11   ` Hans Verkuil
  1 sibling, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:34 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, linux-kernel, Vincent Abriou

On 8/14/19 12:45 PM, 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:
> 	Don't invalidate physical address before unregistering the
> 	notifier.
> 
> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>

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

Regards,

	Hans

> ---
>  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 9862c322f0c4a..bd15902b825ad 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -1256,6 +1256,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;
> @@ -1318,6 +1319,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);
>  
> @@ -1331,6 +1340,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 = {
> @@ -1436,10 +1448,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))
> @@ -1459,14 +1467,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;
>  }
>  
> 


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

* Re: [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
  2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
                   ` (8 preceding siblings ...)
  2019-08-14 10:45 ` [PATCH v7 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
@ 2019-08-19  9:38 ` Hans Verkuil
  2019-08-19 11:28   ` Dariusz Marcinkiewicz
  2019-08-19 14:48   ` Neil Armstrong
  9 siblings, 2 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19  9:38 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Kate Stewart, Neil Armstrong, Daniel Vetter, Hans Verkuil,
	Dhinakaran Pandiyan, Sam Ravnborg, linux-samsung-soc,
	David Francis, amd-gfx, Leo Li, Jerry (Fangzhi) Zuo,
	linux-arm-kernel, nouveau, Jonas Karlman, Jani Nikula, intel-gfx,
	Russell King, Sean Paul, Rodrigo Vivi, linux-tegra,
	Thomas Gleixner, Allison Randal, Thomas Lim, Jernej Skrabec,
	Greg Kroah-Hartman, Douglas Anderson, linux-kernel,
	Manasi Navare, Alex Deucher, Colin Ian King, Enrico Weigelt,
	Laurent Pinchart

Hi all,

The patches in this series can be applied independently from each other.

If you maintain one of these drivers and you want to merge it for v5.4
yourself, then please do so and let me know. If you prefer I commit it
to drm-misc, then please review and (hopefully) Ack the patch.

I would really like to get this in for v5.4 so I can get the userspace
bits in for v5.4 as well through the media subsystem.

Dariusz, can you post a v7.1 for patch 5/9 fixing the typo?

Thanks!

	Hans

On 8/14/19 12:44 PM, Dariusz Marcinkiewicz wrote:
> This series updates DRM drivers to use new CEC notifier API.
> 
> Changes since v6:
> 	Made CEC notifiers' registration and de-registration symmetric
> 	in tda998x and dw-hdmi drivers. Also, accidentally dropped one
> 	patch in v6 (change to drm_dp_cec), brought it back now.
> Changes since v5:
>         Fixed a warning about a missing comment for a new member of
> 	drm_dp_aux_cec struct. Sending to a wider audience,
> 	including maintainers of respective drivers.
> 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     | 46 +++++++++++++------
>  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             | 36 ++++++++++-----
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
>  drivers/gpu/drm/i915/display/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                   | 17 ++++---
>  13 files changed, 155 insertions(+), 94 deletions(-)
> 


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

* [PATCH v7.1 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
  2019-08-19  9:30   ` Hans Verkuil
@ 2019-08-19 11:22   ` " Dariusz Marcinkiewicz
  2019-08-28  7:15     ` [PATCH v7.2 " Dariusz Marcinkiewicz
  2019-08-25 13:12   ` [PATCH v7 " Hans Verkuil
  2 siblings, 1 reply; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-19 11:22 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Russell King, David Airlie, Daniel Vetter,
	linux-kernel

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 v7:
	- typo fix
Changes since v6:
        - move cec_notifier_conn_unregister to tda998x_bridge_detach,
	- add a mutex protecting accesses to a CEC notifier.
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 | 36 +++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 61e042918a7fc..c6e922cd3c0b5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -82,6 +82,8 @@ struct tda998x_priv {
 	u8 audio_port_enable[AUDIO_ROUTE_NUM];
 	struct tda9950_glue cec_glue;
 	struct gpio_desc *calib;
+
+	struct mutex cec_notify_mutex;
 	struct cec_notifier *cec_notify;
 };
 
@@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
 				tda998x_edid_delay_start(priv);
 			} else {
 				schedule_work(&priv->detect_work);
-				cec_notifier_set_phys_addr(priv->cec_notify,
-						   CEC_PHYS_ADDR_INVALID);
+
+				mutex_lock(&priv->cec_notify_mutex);
+				cec_notifier_phys_addr_invalidate(
+						priv->cec_notify);
+				mutex_unlock(&priv->cec_notify_mutex);
 			}
 
 			handled = true;
@@ -1331,6 +1336,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;
@@ -1347,6 +1354,16 @@ 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);
+		return -ENOMEM;
+
+	mutex_lock(&priv->cec_notify_mutex);
+	priv->cec_notify = notifier;
+	mutex_unlock(&priv->cec_notify_mutex);
+
 	drm_connector_attach_encoder(&priv->connector,
 				     priv->bridge.encoder);
 
@@ -1366,6 +1383,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
+	mutex_lock(&priv->cec_notify_mutex);
+	cec_notifier_conn_unregister(priv->cec_notify);
+	priv->cec_notify = NULL;
+	mutex_unlock(&priv->cec_notify_mutex);
+
 	drm_connector_cleanup(&priv->connector);
 }
 
@@ -1789,9 +1811,6 @@ static void tda998x_destroy(struct device *dev)
 	cancel_work_sync(&priv->detect_work);
 
 	i2c_unregister_device(priv->cec);
-
-	if (priv->cec_notify)
-		cec_notifier_put(priv->cec_notify);
 }
 
 static int tda998x_create(struct device *dev)
@@ -1812,6 +1831,7 @@ static int tda998x_create(struct device *dev)
 	mutex_init(&priv->mutex);	/* protect the page access */
 	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
 	mutex_init(&priv->edid_mutex);
+	mutex_init(&priv->cec_notify_mutex);
 	INIT_LIST_HEAD(&priv->bridge.list);
 	init_waitqueue_head(&priv->edid_delay_waitq);
 	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
@@ -1916,12 +1936,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.23.0.rc1.153.gdeed80330f-goog


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

* Re: [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
  2019-08-19  9:38 ` [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Hans Verkuil
@ 2019-08-19 11:28   ` Dariusz Marcinkiewicz
  2019-08-19 12:00     ` Hans Verkuil
  2019-08-19 14:48   ` Neil Armstrong
  1 sibling, 1 reply; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-19 11:28 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: dri-devel, linux-media, Kate Stewart, Neil Armstrong,
	Daniel Vetter, Hans Verkuil, Dhinakaran Pandiyan, Sam Ravnborg,
	linux-samsung-soc, David Francis, amd-gfx, Leo Li,
	Jerry (Fangzhi) Zuo, linux-arm-kernel, nouveau, Jonas Karlman,
	Jani Nikula, intel-gfx, Russell King, Sean Paul, Rodrigo Vivi,
	linux-tegra, Thomas Gleixner, Allison Randal, Thomas Lim,
	Jernej Skrabec, Greg Kroah-Hartman, Douglas Anderson, open list,
	Manasi Navare, Alex Deucher, Colin Ian King, Enrico Weigelt,
	Laurent Pinchart

On Mon, Aug 19, 2019 at 11:38 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi all,
>
Hi Hans.
> The patches in this series can be applied independently from each other.
>
> If you maintain one of these drivers and you want to merge it for v5.4
> yourself, then please do so and let me know. If you prefer I commit it
> to drm-misc, then please review and (hopefully) Ack the patch.
>
> I would really like to get this in for v5.4 so I can get the userspace
> bits in for v5.4 as well through the media subsystem.
>
> Dariusz, can you post a v7.1 for patch 5/9 fixing the typo?
>
Done.

I think it would be good to test v7 changes to dw-hdmi and tda998x on
a real hardware. Hans, do you think you would be able to test those?

Thank you.

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

* Re: [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
  2019-08-19 11:28   ` Dariusz Marcinkiewicz
@ 2019-08-19 12:00     ` Hans Verkuil
  0 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19 12:00 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz
  Cc: dri-devel, linux-media, Kate Stewart, Neil Armstrong,
	Daniel Vetter, Hans Verkuil, Dhinakaran Pandiyan, Sam Ravnborg,
	linux-samsung-soc, David Francis, amd-gfx, Leo Li,
	Jerry (Fangzhi) Zuo, linux-arm-kernel, nouveau, Jonas Karlman,
	Jani Nikula, intel-gfx, Russell King, Sean Paul, Rodrigo Vivi,
	linux-tegra, Thomas Gleixner, Allison Randal, Thomas Lim,
	Jernej Skrabec, Greg Kroah-Hartman, Douglas Anderson, open list,
	Manasi Navare, Alex Deucher, Colin Ian King, Enrico Weigelt,
	Laurent Pinchart

On 8/19/19 1:28 PM, Dariusz Marcinkiewicz wrote:
> On Mon, Aug 19, 2019 at 11:38 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> Hi all,
>>
> Hi Hans.
>> The patches in this series can be applied independently from each other.
>>
>> If you maintain one of these drivers and you want to merge it for v5.4
>> yourself, then please do so and let me know. If you prefer I commit it
>> to drm-misc, then please review and (hopefully) Ack the patch.
>>
>> I would really like to get this in for v5.4 so I can get the userspace
>> bits in for v5.4 as well through the media subsystem.
>>
>> Dariusz, can you post a v7.1 for patch 5/9 fixing the typo?
>>
> Done.
> 
> I think it would be good to test v7 changes to dw-hdmi and tda998x on
> a real hardware. Hans, do you think you would be able to test those?
> 
> Thank you.
> 

I'll try to do this for dw-hdmi today, but the tda998x testing will have to wait
until next week.

Regards,

	Hans

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

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-19  9:32   ` Hans Verkuil
@ 2019-08-19 14:05     ` Hans Verkuil
  2019-08-19 14:38       ` Neil Armstrong
  0 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19 14:05 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Neil Armstrong,
	Douglas Anderson, linux-kernel, Sean Paul, Laurent Pinchart,
	Sam Ravnborg

On 8/19/19 11:32 AM, Hans Verkuil wrote:
> On 8/14/19 12:45 PM, 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 v6:
>>         - move cec_notifier_conn_unregister to a bridge detach
>> 	  function,
>> 	- add a mutex protecting a CEC notifier.
>> 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>
> 
> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

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

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index 83b94b66e464e..55162c9092f71 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -190,6 +190,7 @@ struct dw_hdmi {
>>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>>  
>> +	struct mutex cec_notifier_mutex;
>>  	struct cec_notifier *cec_notifier;
>>  };
>>  
>> @@ -2194,6 +2195,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;
>> @@ -2207,9 +2210,29 @@ 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;
>> +
>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>> +	hdmi->cec_notifier = notifier;
>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>> +
>>  	return 0;
>>  }
>>  
>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>> +{
>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>> +
>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>> +	hdmi->cec_notifier = NULL;
>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>> +}
>> +
>>  static enum drm_mode_status
>>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>>  			  const struct drm_display_mode *mode)
>> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>>  
>>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>>  	.attach = dw_hdmi_bridge_attach,
>> +	.detach = dw_hdmi_bridge_detach,
>>  	.enable = dw_hdmi_bridge_enable,
>>  	.disable = dw_hdmi_bridge_disable,
>>  	.mode_set = dw_hdmi_bridge_mode_set,
>> @@ -2373,9 +2397,11 @@ 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) {
>> +			mutex_lock(&hdmi->cec_notifier_mutex);
>> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>> +			mutex_unlock(&hdmi->cec_notifier_mutex);
>> +		}
>>  	}
>>  
>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  
>>  	mutex_init(&hdmi->mutex);
>>  	mutex_init(&hdmi->audio_mutex);
>> +	mutex_init(&hdmi->cec_notifier_mutex);
>>  	spin_lock_init(&hdmi->audio_lock);
>>  
>>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>> @@ -2693,12 +2720,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
>> @@ -2796,9 +2817,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);
>> @@ -2820,9 +2838,6 @@ 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);
>> -
>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>  	if (hdmi->cec_clk)
>>
> 


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

* Re: [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  2019-08-14 10:45 ` [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
@ 2019-08-19 14:35   ` Neil Armstrong
  2019-08-20  7:48     ` Neil Armstrong
  0 siblings, 1 reply; 49+ messages in thread
From: Neil Armstrong @ 2019-08-19 14:35 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media, hverkuil-cisco
  Cc: Andrzej Hajda, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, Kate Stewart, Allison Randal,
	Greg Kroah-Hartman, Thomas Gleixner, linux-kernel

On 14/08/2019 12:45, 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>
> ---
>  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 0f949978d3fcd..ac1e001d08829 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> @@ -256,8 +256,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);
> @@ -278,13 +278,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;
>  	}
>  
> @@ -294,8 +295,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;
>  }
>  
> @@ -303,8 +302,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;
>  }
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

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

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-19 14:05     ` Hans Verkuil
@ 2019-08-19 14:38       ` Neil Armstrong
  2019-08-19 14:41         ` Hans Verkuil
  0 siblings, 1 reply; 49+ messages in thread
From: Neil Armstrong @ 2019-08-19 14:38 UTC (permalink / raw)
  To: Hans Verkuil, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Douglas Anderson,
	linux-kernel, Sean Paul, Laurent Pinchart, Sam Ravnborg

Hi Hans,

On 19/08/2019 16:05, Hans Verkuil wrote:
> On 8/19/19 11:32 AM, Hans Verkuil wrote:
>> On 8/14/19 12:45 PM, 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 v6:
>>>         - move cec_notifier_conn_unregister to a bridge detach
>>> 	  function,
>>> 	- add a mutex protecting a CEC notifier.
>>> 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>
>>
>> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Did you test it on an Amlogic platform ? If yes, I don't have to !

Neil

> 
> Regards,
> 
> 	Hans
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>> ---
>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index 83b94b66e464e..55162c9092f71 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -190,6 +190,7 @@ struct dw_hdmi {
>>>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>>>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>>>  
>>> +	struct mutex cec_notifier_mutex;
>>>  	struct cec_notifier *cec_notifier;
>>>  };
>>>  
>>> @@ -2194,6 +2195,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;
>>> @@ -2207,9 +2210,29 @@ 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;
>>> +
>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>> +	hdmi->cec_notifier = notifier;
>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>>> +{
>>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>>> +
>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>>> +	hdmi->cec_notifier = NULL;
>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>> +}
>>> +
>>>  static enum drm_mode_status
>>>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>>>  			  const struct drm_display_mode *mode)
>>> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>>>  
>>>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>>>  	.attach = dw_hdmi_bridge_attach,
>>> +	.detach = dw_hdmi_bridge_detach,
>>>  	.enable = dw_hdmi_bridge_enable,
>>>  	.disable = dw_hdmi_bridge_disable,
>>>  	.mode_set = dw_hdmi_bridge_mode_set,
>>> @@ -2373,9 +2397,11 @@ 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) {
>>> +			mutex_lock(&hdmi->cec_notifier_mutex);
>>> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>>> +			mutex_unlock(&hdmi->cec_notifier_mutex);
>>> +		}
>>>  	}
>>>  
>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>>  
>>>  	mutex_init(&hdmi->mutex);
>>>  	mutex_init(&hdmi->audio_mutex);
>>> +	mutex_init(&hdmi->cec_notifier_mutex);
>>>  	spin_lock_init(&hdmi->audio_lock);
>>>  
>>>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>>> @@ -2693,12 +2720,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
>>> @@ -2796,9 +2817,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);
>>> @@ -2820,9 +2838,6 @@ 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);
>>> -
>>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>>  	if (hdmi->cec_clk)
>>>
>>
> 


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

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-19 14:38       ` Neil Armstrong
@ 2019-08-19 14:41         ` Hans Verkuil
  2019-08-19 14:47           ` Neil Armstrong
  0 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19 14:41 UTC (permalink / raw)
  To: Neil Armstrong, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Douglas Anderson,
	linux-kernel, Sean Paul, Laurent Pinchart, Sam Ravnborg

On 8/19/19 4:38 PM, Neil Armstrong wrote:
> Hi Hans,
> 
> On 19/08/2019 16:05, Hans Verkuil wrote:
>> On 8/19/19 11:32 AM, Hans Verkuil wrote:
>>> On 8/14/19 12:45 PM, 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 v6:
>>>>         - move cec_notifier_conn_unregister to a bridge detach
>>>> 	  function,
>>>> 	- add a mutex protecting a CEC notifier.
>>>> 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>
>>>
>>> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Did you test it on an Amlogic platform ? If yes, I don't have to !

Yes, tested on my khadas VIM2 (modified a bit to fix the issue where
the CEC physical address wasn't invalidated correctly as discussed here
earlier).

Regards,

	Hans

> 
> Neil
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>>> ---
>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>>>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index 83b94b66e464e..55162c9092f71 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -190,6 +190,7 @@ struct dw_hdmi {
>>>>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>>>>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>>>>  
>>>> +	struct mutex cec_notifier_mutex;
>>>>  	struct cec_notifier *cec_notifier;
>>>>  };
>>>>  
>>>> @@ -2194,6 +2195,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;
>>>> @@ -2207,9 +2210,29 @@ 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;
>>>> +
>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>> +	hdmi->cec_notifier = notifier;
>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>>>> +{
>>>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>>>> +
>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>>>> +	hdmi->cec_notifier = NULL;
>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>> +}
>>>> +
>>>>  static enum drm_mode_status
>>>>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>>>>  			  const struct drm_display_mode *mode)
>>>> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>>>>  
>>>>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>>>>  	.attach = dw_hdmi_bridge_attach,
>>>> +	.detach = dw_hdmi_bridge_detach,
>>>>  	.enable = dw_hdmi_bridge_enable,
>>>>  	.disable = dw_hdmi_bridge_disable,
>>>>  	.mode_set = dw_hdmi_bridge_mode_set,
>>>> @@ -2373,9 +2397,11 @@ 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) {
>>>> +			mutex_lock(&hdmi->cec_notifier_mutex);
>>>> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>>>> +			mutex_unlock(&hdmi->cec_notifier_mutex);
>>>> +		}
>>>>  	}
>>>>  
>>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>>>  
>>>>  	mutex_init(&hdmi->mutex);
>>>>  	mutex_init(&hdmi->audio_mutex);
>>>> +	mutex_init(&hdmi->cec_notifier_mutex);
>>>>  	spin_lock_init(&hdmi->audio_lock);
>>>>  
>>>>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>>>> @@ -2693,12 +2720,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
>>>> @@ -2796,9 +2817,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);
>>>> @@ -2820,9 +2838,6 @@ 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);
>>>> -
>>>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>>>  	if (hdmi->cec_clk)
>>>>
>>>
>>
> 


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

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-19 14:41         ` Hans Verkuil
@ 2019-08-19 14:47           ` Neil Armstrong
  2019-08-20  7:48             ` Neil Armstrong
  0 siblings, 1 reply; 49+ messages in thread
From: Neil Armstrong @ 2019-08-19 14:47 UTC (permalink / raw)
  To: Hans Verkuil, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Douglas Anderson,
	linux-kernel, Sean Paul, Laurent Pinchart, Sam Ravnborg

On 19/08/2019 16:41, Hans Verkuil wrote:
> On 8/19/19 4:38 PM, Neil Armstrong wrote:
>> Hi Hans,
>>
>> On 19/08/2019 16:05, Hans Verkuil wrote:
>>> On 8/19/19 11:32 AM, Hans Verkuil wrote:
>>>> On 8/14/19 12:45 PM, 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 v6:
>>>>>         - move cec_notifier_conn_unregister to a bridge detach
>>>>> 	  function,
>>>>> 	- add a mutex protecting a CEC notifier.
>>>>> 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>
>>>>
>>>> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>
>>> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> Did you test it on an Amlogic platform ? If yes, I don't have to !
> 
> Yes, tested on my khadas VIM2 (modified a bit to fix the issue where
> the CEC physical address wasn't invalidated correctly as discussed here
> earlier).

Good, thanks.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

> 
> Regards,
> 
> 	Hans
> 
>>
>> Neil
>>
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>>> ---
>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>>>>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> index 83b94b66e464e..55162c9092f71 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> @@ -190,6 +190,7 @@ struct dw_hdmi {
>>>>>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>>>>>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>>>>>  
>>>>> +	struct mutex cec_notifier_mutex;
>>>>>  	struct cec_notifier *cec_notifier;
>>>>>  };
>>>>>  
>>>>> @@ -2194,6 +2195,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;
>>>>> @@ -2207,9 +2210,29 @@ 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;
>>>>> +
>>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>>> +	hdmi->cec_notifier = notifier;
>>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>> +
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>>>>> +{
>>>>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>>>>> +
>>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>>> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>>>>> +	hdmi->cec_notifier = NULL;
>>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>> +}
>>>>> +
>>>>>  static enum drm_mode_status
>>>>>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>>>>>  			  const struct drm_display_mode *mode)
>>>>> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>>>>>  
>>>>>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>>>>>  	.attach = dw_hdmi_bridge_attach,
>>>>> +	.detach = dw_hdmi_bridge_detach,
>>>>>  	.enable = dw_hdmi_bridge_enable,
>>>>>  	.disable = dw_hdmi_bridge_disable,
>>>>>  	.mode_set = dw_hdmi_bridge_mode_set,
>>>>> @@ -2373,9 +2397,11 @@ 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) {
>>>>> +			mutex_lock(&hdmi->cec_notifier_mutex);
>>>>> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>>>>> +			mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>> +		}
>>>>>  	}
>>>>>  
>>>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>>>>  
>>>>>  	mutex_init(&hdmi->mutex);
>>>>>  	mutex_init(&hdmi->audio_mutex);
>>>>> +	mutex_init(&hdmi->cec_notifier_mutex);
>>>>>  	spin_lock_init(&hdmi->audio_lock);
>>>>>  
>>>>>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>>>>> @@ -2693,12 +2720,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
>>>>> @@ -2796,9 +2817,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);
>>>>> @@ -2820,9 +2838,6 @@ 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);
>>>>> -
>>>>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>>>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>>>>  	if (hdmi->cec_clk)
>>>>>
>>>>
>>>
>>
> 


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

* Re: [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
  2019-08-19  9:38 ` [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Hans Verkuil
  2019-08-19 11:28   ` Dariusz Marcinkiewicz
@ 2019-08-19 14:48   ` Neil Armstrong
  2019-08-19 14:55     ` Hans Verkuil
  1 sibling, 1 reply; 49+ messages in thread
From: Neil Armstrong @ 2019-08-19 14:48 UTC (permalink / raw)
  To: Hans Verkuil, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Kate Stewart, Daniel Vetter, Hans Verkuil, Dhinakaran Pandiyan,
	Sam Ravnborg, linux-samsung-soc, David Francis, amd-gfx, Leo Li,
	Jerry (Fangzhi) Zuo, linux-arm-kernel, nouveau, Jonas Karlman,
	Jani Nikula, intel-gfx, Russell King, Sean Paul, Rodrigo Vivi,
	linux-tegra, Thomas Gleixner, Allison Randal, Thomas Lim,
	Jernej Skrabec, Greg Kroah-Hartman, Douglas Anderson,
	linux-kernel, Manasi Navare, Alex Deucher, Colin Ian King,
	Enrico Weigelt, Laurent Pinchart

Hi Dariusz, Hans,

I can apply the dw-hdmi patches if necessary.

Neil

On 19/08/2019 11:38, Hans Verkuil wrote:
> Hi all,
> 
> The patches in this series can be applied independently from each other.
> 
> If you maintain one of these drivers and you want to merge it for v5.4
> yourself, then please do so and let me know. If you prefer I commit it
> to drm-misc, then please review and (hopefully) Ack the patch.
> 
> I would really like to get this in for v5.4 so I can get the userspace
> bits in for v5.4 as well through the media subsystem.
> 
> Dariusz, can you post a v7.1 for patch 5/9 fixing the typo?
> 
> Thanks!
> 
> 	Hans
> 
> On 8/14/19 12:44 PM, Dariusz Marcinkiewicz wrote:
>> This series updates DRM drivers to use new CEC notifier API.
>>
>> Changes since v6:
>> 	Made CEC notifiers' registration and de-registration symmetric
>> 	in tda998x and dw-hdmi drivers. Also, accidentally dropped one
>> 	patch in v6 (change to drm_dp_cec), brought it back now.
>> Changes since v5:
>>         Fixed a warning about a missing comment for a new member of
>> 	drm_dp_aux_cec struct. Sending to a wider audience,
>> 	including maintainers of respective drivers.
>> 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     | 46 +++++++++++++------
>>  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             | 36 ++++++++++-----
>>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
>>  drivers/gpu/drm/i915/display/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                   | 17 ++++---
>>  13 files changed, 155 insertions(+), 94 deletions(-)
>>
> 


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

* Re: [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
  2019-08-19 14:48   ` Neil Armstrong
@ 2019-08-19 14:55     ` Hans Verkuil
  0 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-19 14:55 UTC (permalink / raw)
  To: Neil Armstrong, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Kate Stewart, Daniel Vetter, Hans Verkuil, Dhinakaran Pandiyan,
	Sam Ravnborg, linux-samsung-soc, David Francis, amd-gfx, Leo Li,
	Jerry (Fangzhi) Zuo, linux-arm-kernel, nouveau, Jonas Karlman,
	Jani Nikula, intel-gfx, Russell King, Sean Paul, Rodrigo Vivi,
	linux-tegra, Thomas Gleixner, Allison Randal, Thomas Lim,
	Jernej Skrabec, Greg Kroah-Hartman, Douglas Anderson,
	linux-kernel, Manasi Navare, Alex Deucher, Colin Ian King,
	Enrico Weigelt, Laurent Pinchart

On 8/19/19 4:48 PM, Neil Armstrong wrote:
> Hi Dariusz, Hans,
> 
> I can apply the dw-hdmi patches if necessary.

I'd appreciate it if you can do that.

Thanks,

	Hans

> 
> Neil
> 
> On 19/08/2019 11:38, Hans Verkuil wrote:
>> Hi all,
>>
>> The patches in this series can be applied independently from each other.
>>
>> If you maintain one of these drivers and you want to merge it for v5.4
>> yourself, then please do so and let me know. If you prefer I commit it
>> to drm-misc, then please review and (hopefully) Ack the patch.
>>
>> I would really like to get this in for v5.4 so I can get the userspace
>> bits in for v5.4 as well through the media subsystem.
>>
>> Dariusz, can you post a v7.1 for patch 5/9 fixing the typo?
>>
>> Thanks!
>>
>> 	Hans
>>
>> On 8/14/19 12:44 PM, Dariusz Marcinkiewicz wrote:
>>> This series updates DRM drivers to use new CEC notifier API.
>>>
>>> Changes since v6:
>>> 	Made CEC notifiers' registration and de-registration symmetric
>>> 	in tda998x and dw-hdmi drivers. Also, accidentally dropped one
>>> 	patch in v6 (change to drm_dp_cec), brought it back now.
>>> Changes since v5:
>>>         Fixed a warning about a missing comment for a new member of
>>> 	drm_dp_aux_cec struct. Sending to a wider audience,
>>> 	including maintainers of respective drivers.
>>> 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     | 46 +++++++++++++------
>>>  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             | 36 ++++++++++-----
>>>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
>>>  drivers/gpu/drm/i915/display/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                   | 17 ++++---
>>>  13 files changed, 155 insertions(+), 94 deletions(-)
>>>
>>
> 


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

* Re: [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register
  2019-08-19 14:35   ` Neil Armstrong
@ 2019-08-20  7:48     ` Neil Armstrong
  0 siblings, 0 replies; 49+ messages in thread
From: Neil Armstrong @ 2019-08-20  7:48 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media, hverkuil-cisco
  Cc: Andrzej Hajda, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, Kate Stewart, Allison Randal,
	Greg Kroah-Hartman, Thomas Gleixner, linux-kernel

On 19/08/2019 16:35, Neil Armstrong wrote:
> On 14/08/2019 12:45, 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>
>> ---
>>  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 0f949978d3fcd..ac1e001d08829 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
>> @@ -256,8 +256,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);
>> @@ -278,13 +278,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;
>>  	}
>>  
>> @@ -294,8 +295,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;
>>  }
>>  
>> @@ -303,8 +302,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;
>>  }
>>
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> 

Applying to drm-misc-next

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

* Re: [PATCH v7 8/9] drm: dw-hdmi: use cec_notifier_conn_(un)register
  2019-08-19 14:47           ` Neil Armstrong
@ 2019-08-20  7:48             ` Neil Armstrong
  0 siblings, 0 replies; 49+ messages in thread
From: Neil Armstrong @ 2019-08-20  7:48 UTC (permalink / raw)
  To: Hans Verkuil, Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Jernej Skrabec, Jonas Karlman, David Airlie, Douglas Anderson,
	linux-kernel, Sean Paul, Laurent Pinchart, Sam Ravnborg

On 19/08/2019 16:47, Neil Armstrong wrote:
> On 19/08/2019 16:41, Hans Verkuil wrote:
>> On 8/19/19 4:38 PM, Neil Armstrong wrote:
>>> Hi Hans,
>>>
>>> On 19/08/2019 16:05, Hans Verkuil wrote:
>>>> On 8/19/19 11:32 AM, Hans Verkuil wrote:
>>>>> On 8/14/19 12:45 PM, 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 v6:
>>>>>>         - move cec_notifier_conn_unregister to a bridge detach
>>>>>> 	  function,
>>>>>> 	- add a mutex protecting a CEC notifier.
>>>>>> 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>
>>>>>
>>>>> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>>
>>>> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>
>>> Did you test it on an Amlogic platform ? If yes, I don't have to !
>>
>> Yes, tested on my khadas VIM2 (modified a bit to fix the issue where
>> the CEC physical address wasn't invalidated correctly as discussed here
>> earlier).
> 
> Good, thanks.
> 
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Neil
>>>
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>>>
>>>>> Regards,
>>>>>
>>>>> 	Hans
>>>>>
>>>>>> ---
>>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 45 +++++++++++++++--------
>>>>>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> index 83b94b66e464e..55162c9092f71 100644
>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> @@ -190,6 +190,7 @@ struct dw_hdmi {
>>>>>>  	void (*enable_audio)(struct dw_hdmi *hdmi);
>>>>>>  	void (*disable_audio)(struct dw_hdmi *hdmi);
>>>>>>  
>>>>>> +	struct mutex cec_notifier_mutex;
>>>>>>  	struct cec_notifier *cec_notifier;
>>>>>>  };
>>>>>>  
>>>>>> @@ -2194,6 +2195,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;
>>>>>> @@ -2207,9 +2210,29 @@ 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;
>>>>>> +
>>>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>>>> +	hdmi->cec_notifier = notifier;
>>>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>>> +
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> +static void dw_hdmi_bridge_detach(struct drm_bridge *bridge)
>>>>>> +{
>>>>>> +	struct dw_hdmi *hdmi = bridge->driver_private;
>>>>>> +
>>>>>> +	mutex_lock(&hdmi->cec_notifier_mutex);
>>>>>> +	cec_notifier_conn_unregister(hdmi->cec_notifier);
>>>>>> +	hdmi->cec_notifier = NULL;
>>>>>> +	mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>>> +}
>>>>>> +
>>>>>>  static enum drm_mode_status
>>>>>>  dw_hdmi_bridge_mode_valid(struct drm_bridge *bridge,
>>>>>>  			  const struct drm_display_mode *mode)
>>>>>> @@ -2266,6 +2289,7 @@ static void dw_hdmi_bridge_enable(struct drm_bridge *bridge)
>>>>>>  
>>>>>>  static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
>>>>>>  	.attach = dw_hdmi_bridge_attach,
>>>>>> +	.detach = dw_hdmi_bridge_detach,
>>>>>>  	.enable = dw_hdmi_bridge_enable,
>>>>>>  	.disable = dw_hdmi_bridge_disable,
>>>>>>  	.mode_set = dw_hdmi_bridge_mode_set,
>>>>>> @@ -2373,9 +2397,11 @@ 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) {
>>>>>> +			mutex_lock(&hdmi->cec_notifier_mutex);
>>>>>> +			cec_notifier_phys_addr_invalidate(hdmi->cec_notifier);
>>>>>> +			mutex_unlock(&hdmi->cec_notifier_mutex);
>>>>>> +		}
>>>>>>  	}
>>>>>>  
>>>>>>  	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
>>>>>> @@ -2561,6 +2587,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>>>>>  
>>>>>>  	mutex_init(&hdmi->mutex);
>>>>>>  	mutex_init(&hdmi->audio_mutex);
>>>>>> +	mutex_init(&hdmi->cec_notifier_mutex);
>>>>>>  	spin_lock_init(&hdmi->audio_lock);
>>>>>>  
>>>>>>  	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
>>>>>> @@ -2693,12 +2720,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
>>>>>> @@ -2796,9 +2817,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);
>>>>>> @@ -2820,9 +2838,6 @@ 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);
>>>>>> -
>>>>>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>>>>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>>>>>  	if (hdmi->cec_clk)
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Applying to drm-misc-next

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

* Re: [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
@ 2019-08-22  8:03   ` Hans Verkuil
  2019-08-26  8:59     ` Hans Verkuil
  2019-08-26 12:08   ` Ville Syrjälä
  1 sibling, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-22  8:03 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, intel-gfx, linux-kernel, Rodrigo Vivi,
	Ville Syrjälä

Ville or Rodrigo,

Can one of you either merge this patch or Ack it so that I can merge this?

Thank you!

Regards,

	Hans

On 8/14/19 12:45 PM, 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>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b1ca8e5bdb56d..9fcf2c58c29c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/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);
>  }
> @@ -3068,6 +3069,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));
> @@ -3120,8 +3122,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] 49+ messages in thread

* Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
  2019-08-15 18:10   ` Lyude Paul
@ 2019-08-22  8:08   ` Hans Verkuil
  2019-08-26  9:00     ` Hans Verkuil
  2019-08-28 15:05   ` Ville Syrjälä
  2 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-22  8:08 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, nouveau, Dhinakaran Pandiyan, Anthony Koo,
	David Francis, amd-gfx, Jerry (Fangzhi) Zuo, Ben Skeggs, Leo Li,
	intel-gfx, Maxime Ripard, Rodrigo Vivi, Sean Paul, Thomas Lim,
	linux-kernel, Manasi Navare, Alex Deucher, Christian König,
	Ville Syrjälä

Alex, Ville/Rodrigo, Ben,

Can you (hopefully) Ack this patch so that I can merge it?

Thank you!

	Hans

On 8/14/19 12:44 PM, 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>
> ---
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>  include/drm/drm_dp_helper.h                   | 17 ++++++-------
>  5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
> @@ -416,7 +416,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/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1092499115760..de2486fe7bf2d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5497,7 +5497,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);
> @@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
>  
>  struct cec_adapter;
>  struct edid;
> +struct drm_connector;
>  
>  /**
>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
>   * @lock: mutex protecting this struct
>   * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> - * @name: name of the CEC adapter
> - * @parent: parent device of the CEC adapter
> + * @connector: the connector this CEC adapter is associated with
>   * @unregister_work: unregister the CEC adapter
>   */
>  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;
>  };
>  
> @@ -1451,8 +1450,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);
> @@ -1461,9 +1460,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] 49+ messages in thread

* Re: [PATCH v7 6/9] drm: sti: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 6/9] drm: sti: " Dariusz Marcinkiewicz
  2019-08-19  9:34   ` Hans Verkuil
@ 2019-08-22  8:11   ` Hans Verkuil
  2019-09-02 13:11     ` Benjamin Gaignard
  1 sibling, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-22  8:11 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, linux-kernel, Vincent Abriou, Benjamin Gaignard

Adding Benjamin Gaignard.

Benjamin, can you take a look at this and Ack it (or merge it if you prefer) and
ideally test it as well. This is the only patch in this series that I could not
test since I don't have any hardware.

Regards,

	Hans

On 8/14/19 12:45 PM, 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:
> 	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 9862c322f0c4a..bd15902b825ad 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -1256,6 +1256,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;
> @@ -1318,6 +1319,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);
>  
> @@ -1331,6 +1340,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 = {
> @@ -1436,10 +1448,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))
> @@ -1459,14 +1467,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;
>  }
>  
> 


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

* Re: [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
  2019-08-19  9:30   ` Hans Verkuil
  2019-08-19 11:22   ` [PATCH v7.1 " Dariusz Marcinkiewicz
@ 2019-08-25 13:12   ` " Hans Verkuil
  2019-08-28  7:18     ` Dariusz Marcinkiewicz
  2 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-25 13:12 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Russell King, David Airlie, Daniel Vetter, linux-kernel

On 8/14/19 12:45 PM, 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 v6:
>         - move cec_notifier_conn_unregister to tda998x_bridge_detach,
> 	- add a mutex protecting accesses to a CEC notifier.
> 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 | 36 +++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 61e042918a7fc..643480415473f 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -82,6 +82,8 @@ struct tda998x_priv {
>  	u8 audio_port_enable[AUDIO_ROUTE_NUM];
>  	struct tda9950_glue cec_glue;
>  	struct gpio_desc *calib;
> +
> +	struct mutex cec_notifiy_mutex;
>  	struct cec_notifier *cec_notify;
>  };
>  
> @@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
>  				tda998x_edid_delay_start(priv);
>  			} else {
>  				schedule_work(&priv->detect_work);
> -				cec_notifier_set_phys_addr(priv->cec_notify,
> -						   CEC_PHYS_ADDR_INVALID);
> +
> +				mutex_lock(&priv->cec_notifiy_mutex);
> +				cec_notifier_phys_addr_invalidate(
> +						priv->cec_notify);
> +				mutex_unlock(&priv->cec_notifiy_mutex);
>  			}
>  
>  			handled = true;
> @@ -1331,6 +1336,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;
> @@ -1347,6 +1354,16 @@ 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);
> +		return -ENOMEM;

You dropped a 'if (!notifier)' before the return!

After adding back this 'if' it worked fine on my BeagleBone Black board,
so after fixing this you can add my:

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

tag.

Regards,

	Hans

> +
> +	mutex_lock(&priv->cec_notifiy_mutex);
> +	priv->cec_notify = notifier;
> +	mutex_unlock(&priv->cec_notifiy_mutex);
> +
>  	drm_connector_attach_encoder(&priv->connector,
>  				     priv->bridge.encoder);
>  
> @@ -1366,6 +1383,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge)
>  {
>  	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
>  
> +	mutex_lock(&priv->cec_notifiy_mutex);
> +	cec_notifier_conn_unregister(priv->cec_notify);
> +	priv->cec_notify = NULL;
> +	mutex_unlock(&priv->cec_notifiy_mutex);
> +
>  	drm_connector_cleanup(&priv->connector);
>  }
>  
> @@ -1789,9 +1811,6 @@ static void tda998x_destroy(struct device *dev)
>  	cancel_work_sync(&priv->detect_work);
>  
>  	i2c_unregister_device(priv->cec);
> -
> -	if (priv->cec_notify)
> -		cec_notifier_put(priv->cec_notify);
>  }
>  
>  static int tda998x_create(struct device *dev)
> @@ -1812,6 +1831,7 @@ static int tda998x_create(struct device *dev)
>  	mutex_init(&priv->mutex);	/* protect the page access */
>  	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
>  	mutex_init(&priv->edid_mutex);
> +	mutex_init(&priv->cec_notifiy_mutex);
>  	INIT_LIST_HEAD(&priv->bridge.list);
>  	init_waitqueue_head(&priv->edid_delay_waitq);
>  	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
> @@ -1916,12 +1936,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] 49+ messages in thread

* Re: [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-22  8:03   ` Hans Verkuil
@ 2019-08-26  8:59     ` Hans Verkuil
  0 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-26  8:59 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: David Airlie, intel-gfx, linux-kernel, Rodrigo Vivi, Daniel Vetter

On 8/22/19 10:03 AM, Hans Verkuil wrote:
> Ville or Rodrigo,
> 
> Can one of you either merge this patch or Ack it so that I can merge this?

Ping!

Regards,

	Hans

> 
> Thank you!
> 
> Regards,
> 
> 	Hans
> 
> On 8/14/19 12:45 PM, 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>
>> ---
>>  drivers/gpu/drm/i915/display/intel_hdmi.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index b1ca8e5bdb56d..9fcf2c58c29c5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/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);
>>  }
>> @@ -3068,6 +3069,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));
>> @@ -3120,8 +3122,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");
>>  }
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


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

* Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-22  8:08   ` Hans Verkuil
@ 2019-08-26  9:00     ` Hans Verkuil
  0 siblings, 0 replies; 49+ messages in thread
From: Hans Verkuil @ 2019-08-26  9:00 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Maxime Ripard, Thomas Lim, David Airlie, nouveau, David Francis,
	linux-kernel, amd-gfx, Christian König, Manasi Navare,
	Leo Li, Jerry (Fangzhi) Zuo, Dhinakaran Pandiyan, Rodrigo Vivi,
	Alex Deucher, Sean Paul, Anthony Koo, intel-gfx, Ben Skeggs,
	Daniel Vetter

On 8/22/19 10:08 AM, Hans Verkuil wrote:
> Alex, Ville/Rodrigo, Ben,
> 
> Can you (hopefully) Ack this patch so that I can merge it?

Ping!

Regards,

	Hans

> 
> Thank you!
> 
> 	Hans
> 
> On 8/14/19 12:44 PM, 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>
>> ---
>>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
>>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>>  include/drm/drm_dp_helper.h                   | 17 ++++++-------
>>  5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
>> @@ -416,7 +416,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/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 1092499115760..de2486fe7bf2d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -5497,7 +5497,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);
>> @@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
>> @@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
>> --- a/include/drm/drm_dp_helper.h
>> +++ b/include/drm/drm_dp_helper.h
>> @@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
>>  
>>  struct cec_adapter;
>>  struct edid;
>> +struct drm_connector;
>>  
>>  /**
>>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
>>   * @lock: mutex protecting this struct
>>   * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
>> - * @name: name of the CEC adapter
>> - * @parent: parent device of the CEC adapter
>> + * @connector: the connector this CEC adapter is associated with
>>   * @unregister_work: unregister the CEC adapter
>>   */
>>  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;
>>  };
>>  
>> @@ -1451,8 +1450,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);
>> @@ -1461,9 +1460,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)
>>  {
>>  }
>>  
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


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

* Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-15 18:10   ` Lyude Paul
@ 2019-08-26  9:05     ` Ben Skeggs
  0 siblings, 0 replies; 49+ messages in thread
From: Ben Skeggs @ 2019-08-26  9:05 UTC (permalink / raw)
  To: Lyude Paul
  Cc: Dariusz Marcinkiewicz, dri-devel, linux-media, hverkuil-cisco,
	Harry Wentland, Leo Li, Alex Deucher, Christian König,
	David (ChunMing) Zhou, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Sean Paul, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, Jerry (Fangzhi) Zuo, Anthony Koo,
	Thomas Lim, David Francis, Ville Syrjälä,
	Chris Wilson, Imre Deak, Manasi Navare, Dhinakaran Pandiyan,
	amd-gfx, LKML, intel-gfx, nouveau

On Fri, Aug 16, 2019 at 4:10 AM Lyude Paul <lyude@redhat.com> wrote:
>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>

>
> On Wed, 2019-08-14 at 12:44 +0200, 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>
> > ---
> >  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
> >  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
> >  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
> >  include/drm/drm_dp_helper.h                   | 17 ++++++-------
> >  5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
> > @@ -416,7 +416,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/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 1092499115760..de2486fe7bf2d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5497,7 +5497,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);
> > @@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> > @@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
> >
> >  struct cec_adapter;
> >  struct edid;
> > +struct drm_connector;
> >
> >  /**
> >   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
> >   * @lock: mutex protecting this struct
> >   * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> > - * @name: name of the CEC adapter
> > - * @parent: parent device of the CEC adapter
> > + * @connector: the connector this CEC adapter is associated with
> >   * @unregister_work: unregister the CEC adapter
> >   */
> >  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;
> >  };
> >
> > @@ -1451,8 +1450,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);
> > @@ -1461,9 +1460,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] 49+ messages in thread

* Re: [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
  2019-08-22  8:03   ` Hans Verkuil
@ 2019-08-26 12:08   ` Ville Syrjälä
  1 sibling, 0 replies; 49+ messages in thread
From: Ville Syrjälä @ 2019-08-26 12:08 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz
  Cc: dri-devel, linux-media, hverkuil-cisco, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter,
	Chris Wilson, Maarten Lankhorst, Shashank Sharma, Ramalingam C,
	intel-gfx, linux-kernel

On Wed, Aug 14, 2019 at 12:45:00PM +0200, 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>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b1ca8e5bdb56d..9fcf2c58c29c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/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);
>  }
> @@ -3068,6 +3069,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));
> @@ -3120,8 +3122,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.23.0.rc1.153.gdeed80330f-goog

-- 
Ville Syrjälä
Intel

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

* [PATCH v7.2 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-19 11:22   ` [PATCH v7.1 " Dariusz Marcinkiewicz
@ 2019-08-28  7:15     ` " Dariusz Marcinkiewicz
  0 siblings, 0 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-28  7:15 UTC (permalink / raw)
  To: dri-devel, linux-media, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Russell King, David Airlie, Daniel Vetter,
	linux-kernel

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 v7.1:
	- re-added if (!notifier)..
Changes since v7:
	- typo fix
Changes since v6:
        - move cec_notifier_conn_unregister to tda998x_bridge_detach,
	- add a mutex protecting accesses to a CEC notifier.
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>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 37 ++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 61e042918a7fc..2bc4f50458137 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -82,6 +82,8 @@ struct tda998x_priv {
 	u8 audio_port_enable[AUDIO_ROUTE_NUM];
 	struct tda9950_glue cec_glue;
 	struct gpio_desc *calib;
+
+	struct mutex cec_notify_mutex;
 	struct cec_notifier *cec_notify;
 };
 
@@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
 				tda998x_edid_delay_start(priv);
 			} else {
 				schedule_work(&priv->detect_work);
-				cec_notifier_set_phys_addr(priv->cec_notify,
-						   CEC_PHYS_ADDR_INVALID);
+
+				mutex_lock(&priv->cec_notify_mutex);
+				cec_notifier_phys_addr_invalidate(
+						priv->cec_notify);
+				mutex_unlock(&priv->cec_notify_mutex);
 			}
 
 			handled = true;
@@ -1331,6 +1336,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;
@@ -1347,6 +1354,17 @@ 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;
+
+	mutex_lock(&priv->cec_notify_mutex);
+	priv->cec_notify = notifier;
+	mutex_unlock(&priv->cec_notify_mutex);
+
 	drm_connector_attach_encoder(&priv->connector,
 				     priv->bridge.encoder);
 
@@ -1366,6 +1384,11 @@ static void tda998x_bridge_detach(struct drm_bridge *bridge)
 {
 	struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
 
+	mutex_lock(&priv->cec_notify_mutex);
+	cec_notifier_conn_unregister(priv->cec_notify);
+	priv->cec_notify = NULL;
+	mutex_unlock(&priv->cec_notify_mutex);
+
 	drm_connector_cleanup(&priv->connector);
 }
 
@@ -1789,9 +1812,6 @@ static void tda998x_destroy(struct device *dev)
 	cancel_work_sync(&priv->detect_work);
 
 	i2c_unregister_device(priv->cec);
-
-	if (priv->cec_notify)
-		cec_notifier_put(priv->cec_notify);
 }
 
 static int tda998x_create(struct device *dev)
@@ -1812,6 +1832,7 @@ static int tda998x_create(struct device *dev)
 	mutex_init(&priv->mutex);	/* protect the page access */
 	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
 	mutex_init(&priv->edid_mutex);
+	mutex_init(&priv->cec_notify_mutex);
 	INIT_LIST_HEAD(&priv->bridge.list);
 	init_waitqueue_head(&priv->edid_delay_waitq);
 	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
@@ -1916,12 +1937,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.23.0.187.g17f5b7556c-goog


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

* Re: [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register
  2019-08-25 13:12   ` [PATCH v7 " Hans Verkuil
@ 2019-08-28  7:18     ` Dariusz Marcinkiewicz
  0 siblings, 0 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-28  7:18 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: dri-devel, linux-media, Russell King, David Airlie,
	Daniel Vetter, open list

On Sun, Aug 25, 2019 at 3:12 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> You dropped a 'if (!notifier)' before the return!
>
> After adding back this 'if' it worked fine on my BeagleBone Black board,
> so after fixing this you can add my:
>
> Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>
Submitted v7.2. Thank you for testing!

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

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
  2019-08-19  9:33   ` Hans Verkuil
@ 2019-08-28  8:09   ` Hans Verkuil
  2019-08-28  9:38     ` Thierry Reding
  2019-08-28  9:36   ` Thierry Reding
  2 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-28  8:09 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media
  Cc: Thierry Reding, David Airlie, Daniel Vetter, Jonathan Hunter,
	linux-tegra, linux-kernel

Thierry,

Can you review this patch?

Thanks!

	Hans

On 8/14/19 12:45 PM, 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>
> ---
>  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 bdcaa4c7168cf..34373734ff689 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -70,6 +70,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);
>  }
> @@ -163,18 +168,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 (output->hpd_gpio)
>  		free_irq(output->hpd_irq, output);
>  
> @@ -184,6 +182,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) {
> @@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  	if (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] 49+ messages in thread

* Re: [PATCH v7 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
  2019-08-19  9:32   ` Hans Verkuil
@ 2019-08-28  8:39   ` Sylwester Nawrocki
  2019-08-28 12:34     ` [PATCH v7.1 " Dariusz Marcinkiewicz
  2019-08-28 12:38     ` [PATCH v7 " Dariusz Marcinkiewicz
  1 sibling, 2 replies; 49+ messages in thread
From: Sylwester Nawrocki @ 2019-08-28  8:39 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz, dri-devel, linux-media, hverkuil-cisco
  Cc: Inki Dae, Joonyoung Shim, Seung-Woo Kim, Kyungmin Park,
	David Airlie, Daniel Vetter, Kukjin Kim, Krzysztof Kozlowski,
	linux-arm-kernel, linux-samsung-soc, linux-kernel

On 8/14/19 12:45, 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>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index bc1565f1822ab..d532b468d9af5 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c

> @@ -2006,12 +2020,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;
> @@ -2023,7 +2031,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;


> -err_notifier_put:
> -	cec_notifier_put(hdata->notifier);
> +err_runtime_disable:
>  	pm_runtime_disable(dev);

nit: I think err_rpm_disable or err_pm_runtime_disable could be better 
     label names.

-- 
Thanks,
Sylwester

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

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
  2019-08-19  9:33   ` Hans Verkuil
  2019-08-28  8:09   ` Hans Verkuil
@ 2019-08-28  9:36   ` Thierry Reding
  2 siblings, 0 replies; 49+ messages in thread
From: Thierry Reding @ 2019-08-28  9:36 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz
  Cc: dri-devel, linux-media, hverkuil-cisco, David Airlie,
	Daniel Vetter, Jonathan Hunter, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2936 bytes --]

On Wed, Aug 14, 2019 at 12:45:05PM +0200, 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>
> ---
>  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 bdcaa4c7168cf..34373734ff689 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -70,6 +70,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);
>  }
> @@ -163,18 +168,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 (output->hpd_gpio)
>  		free_irq(output->hpd_irq, output);
>  
> @@ -184,6 +182,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) {
> @@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>  	if (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;
>  }
>  

It might be slightly cleaner to move this into the HDMI drivers
themselves, although that'd mean a bit of duplication. That could be
mitigated by moving the code into a separate helper that could be called
by the HDMI drivers.

Then again, I don't feel strongly about it, and that could always be
done as part of some later refactoring, so I think this is fine.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-28  8:09   ` Hans Verkuil
@ 2019-08-28  9:38     ` Thierry Reding
  2019-08-28 10:06       ` Hans Verkuil
  0 siblings, 1 reply; 49+ messages in thread
From: Thierry Reding @ 2019-08-28  9:38 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dariusz Marcinkiewicz, dri-devel, linux-media, David Airlie,
	Daniel Vetter, Jonathan Hunter, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3179 bytes --]

On Wed, Aug 28, 2019 at 10:09:30AM +0200, Hans Verkuil wrote:
> Thierry,
> 
> Can you review this patch?
> 
> Thanks!
> 
> 	Hans

Did you want me to pick this up into the drm/tegra tree? Or do you want
to pick it up into your tree?

We're just past the DRM subsystem deadline, so it'll have to wait until
the next cycle if we go that way. If you're in a hurry you may want to
pick it up yourself, in which case:

Acked-by: Thierry Reding <treding@nvidia.com>

> On 8/14/19 12:45 PM, 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>
> > ---
> >  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 bdcaa4c7168cf..34373734ff689 100644
> > --- a/drivers/gpu/drm/tegra/output.c
> > +++ b/drivers/gpu/drm/tegra/output.c
> > @@ -70,6 +70,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);
> >  }
> > @@ -163,18 +168,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 (output->hpd_gpio)
> >  		free_irq(output->hpd_irq, output);
> >  
> > @@ -184,6 +182,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) {
> > @@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
> >  	if (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;
> >  }
> >  
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-28  9:38     ` Thierry Reding
@ 2019-08-28 10:06       ` Hans Verkuil
  2019-08-28 11:54         ` Thierry Reding
  0 siblings, 1 reply; 49+ messages in thread
From: Hans Verkuil @ 2019-08-28 10:06 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dariusz Marcinkiewicz, dri-devel, linux-media, David Airlie,
	Daniel Vetter, Jonathan Hunter, linux-tegra, linux-kernel

On 8/28/19 11:38 AM, Thierry Reding wrote:
> On Wed, Aug 28, 2019 at 10:09:30AM +0200, Hans Verkuil wrote:
>> Thierry,
>>
>> Can you review this patch?
>>
>> Thanks!
>>
>> 	Hans
> 
> Did you want me to pick this up into the drm/tegra tree? Or do you want
> to pick it up into your tree?

Can you pick it up for the next cycle? As you mentioned, we missed the
deadline for 5.4, so this feature won't be enabled in the public CEC API
until 5.5.

Thanks!

	Hans

> 
> We're just past the DRM subsystem deadline, so it'll have to wait until
> the next cycle if we go that way. If you're in a hurry you may want to
> pick it up yourself, in which case:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> 
>> On 8/14/19 12:45 PM, 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>
>>> ---
>>>  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 bdcaa4c7168cf..34373734ff689 100644
>>> --- a/drivers/gpu/drm/tegra/output.c
>>> +++ b/drivers/gpu/drm/tegra/output.c
>>> @@ -70,6 +70,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);
>>>  }
>>> @@ -163,18 +168,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 (output->hpd_gpio)
>>>  		free_irq(output->hpd_irq, output);
>>>  
>>> @@ -184,6 +182,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) {
>>> @@ -199,6 +198,21 @@ int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
>>>  	if (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] 49+ messages in thread

* Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register
  2019-08-28 10:06       ` Hans Verkuil
@ 2019-08-28 11:54         ` Thierry Reding
  0 siblings, 0 replies; 49+ messages in thread
From: Thierry Reding @ 2019-08-28 11:54 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dariusz Marcinkiewicz, dri-devel, linux-media, David Airlie,
	Daniel Vetter, Jonathan Hunter, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On Wed, Aug 28, 2019 at 12:06:34PM +0200, Hans Verkuil wrote:
> On 8/28/19 11:38 AM, Thierry Reding wrote:
> > On Wed, Aug 28, 2019 at 10:09:30AM +0200, Hans Verkuil wrote:
> >> Thierry,
> >>
> >> Can you review this patch?
> >>
> >> Thanks!
> >>
> >> 	Hans
> > 
> > Did you want me to pick this up into the drm/tegra tree? Or do you want
> > to pick it up into your tree?
> 
> Can you pick it up for the next cycle? As you mentioned, we missed the
> deadline for 5.4, so this feature won't be enabled in the public CEC API
> until 5.5.
> 
> Thanks!

Sure, will do.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* [PATCH v7.1 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-28  8:39   ` Sylwester Nawrocki
@ 2019-08-28 12:34     ` " Dariusz Marcinkiewicz
  2019-08-28 12:38     ` [PATCH v7 " Dariusz Marcinkiewicz
  1 sibling, 0 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-28 12:34 UTC (permalink / raw)
  To: dri-devel, linux-media, s.nawrocki, hverkuil-cisco
  Cc: Dariusz Marcinkiewicz, Inki Dae, Joonyoung Shim, Seung-Woo Kim,
	Kyungmin Park, David Airlie, Daniel Vetter, Kukjin Kim,
	Krzysztof Kozlowski, linux-arm-kernel, linux-samsung-soc,
	linux-kernel

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 v7:
	- err_runtime_disable -> err_rpm_disable
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>
---
 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 bc1565f1822ab..799f2db13efe2 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -852,6 +852,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);
 }
@@ -935,6 +939,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;
@@ -957,6 +962,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;
 }
 
@@ -1528,8 +1542,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;
 	}
 
@@ -2006,12 +2020,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;
@@ -2023,7 +2031,7 @@ static int hdmi_probe(struct platform_device *pdev)
 
 	ret = hdmi_register_audio_device(hdata);
 	if (ret)
-		goto err_notifier_put;
+		goto err_rpm_disable;
 
 	ret = component_add(&pdev->dev, &hdmi_component_ops);
 	if (ret)
@@ -2034,8 +2042,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_rpm_disable:
 	pm_runtime_disable(dev);
 
 err_hdmiphy:
@@ -2054,12 +2061,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.23.0.187.g17f5b7556c-goog


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

* Re: [PATCH v7 9/9] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
  2019-08-28  8:39   ` Sylwester Nawrocki
  2019-08-28 12:34     ` [PATCH v7.1 " Dariusz Marcinkiewicz
@ 2019-08-28 12:38     ` " Dariusz Marcinkiewicz
  1 sibling, 0 replies; 49+ messages in thread
From: Dariusz Marcinkiewicz @ 2019-08-28 12:38 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: dri-devel, linux-media, Hans Verkuil, Inki Dae, Joonyoung Shim,
	Seung-Woo Kim, Kyungmin Park, David Airlie, Daniel Vetter,
	Kukjin Kim, Krzysztof Kozlowski, linux-arm-kernel,
	linux-samsung-soc, open list

Hi.

On Wed, Aug 28, 2019 at 10:39 AM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> nit: I think err_rpm_disable or err_pm_runtime_disable could be better
>      label names.
>
Submitted v7.1 which replaces err_runtime_disable with err_rpm_disable.

Thank you.

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

* Re: [PATCH v7 1/9] drm_dp_cec: add connector info support.
  2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
  2019-08-15 18:10   ` Lyude Paul
  2019-08-22  8:08   ` Hans Verkuil
@ 2019-08-28 15:05   ` Ville Syrjälä
  2 siblings, 0 replies; 49+ messages in thread
From: Ville Syrjälä @ 2019-08-28 15:05 UTC (permalink / raw)
  To: Dariusz Marcinkiewicz
  Cc: dri-devel, linux-media, hverkuil-cisco, Harry Wentland, Leo Li,
	Alex Deucher, Christian König, David (ChunMing) Zhou,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Sean Paul, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ben Skeggs, Lyude Paul, Jerry (Fangzhi) Zuo, Anthony Koo,
	Thomas Lim, David Francis, Chris Wilson, Imre Deak,
	Manasi Navare, Dhinakaran Pandiyan, amd-gfx, linux-kernel,
	intel-gfx, nouveau

On Wed, Aug 14, 2019 at 12:44:59PM +0200, 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>
> ---
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  2 +-
>  drivers/gpu/drm/drm_dp_cec.c                  | 25 ++++++++++++-------
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  3 +--
>  include/drm/drm_dp_helper.h                   | 17 ++++++-------
>  5 files changed, 27 insertions(+), 24 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 16218a202b591..5ec14efd4d8cb 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
> @@ -416,7 +416,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/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1092499115760..de2486fe7bf2d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5497,7 +5497,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);
> @@ -5512,8 +5511,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 330d7d29a6e34..8aa703347eb54 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1416,8 +1416,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 8364502f92cfe..7972b925a952b 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1230,20 +1230,19 @@ struct drm_dp_aux_msg {
>  
>  struct cec_adapter;
>  struct edid;
> +struct drm_connector;
>  
>  /**
>   * struct drm_dp_aux_cec - DisplayPort CEC-Tunneling-over-AUX
>   * @lock: mutex protecting this struct
>   * @adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> - * @name: name of the CEC adapter
> - * @parent: parent device of the CEC adapter
> + * @connector: the connector this CEC adapter is associated with
>   * @unregister_work: unregister the CEC adapter
>   */
>  struct drm_dp_aux_cec {
>  	struct mutex lock;
>  	struct cec_adapter *adap;
> -	const char *name;
> -	struct device *parent;
> +	struct drm_connector *connector;

I think all current users could just pass the connector to
cec_set_edid(). So could save a pointer here.

Anyways
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


>  	struct delayed_work unregister_work;
>  };
>  
> @@ -1451,8 +1450,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);
> @@ -1461,9 +1460,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.23.0.rc1.153.gdeed80330f-goog

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH v7 6/9] drm: sti: use cec_notifier_conn_(un)register
  2019-08-22  8:11   ` Hans Verkuil
@ 2019-09-02 13:11     ` Benjamin Gaignard
  0 siblings, 0 replies; 49+ messages in thread
From: Benjamin Gaignard @ 2019-09-02 13:11 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Dariusz Marcinkiewicz, ML dri-devel, linux-media, David Airlie,
	Linux Kernel Mailing List, Vincent Abriou

Le jeu. 22 août 2019 à 10:11, Hans Verkuil <hverkuil-cisco@xs4all.nl> a écrit :
>
> Adding Benjamin Gaignard.
>
> Benjamin, can you take a look at this and Ack it (or merge it if you prefer) and
> ideally test it as well. This is the only patch in this series that I could not
> test since I don't have any hardware.

Looks good for me.

Applied on drm-misc-next,
Thanks,
Benjamin

>
> Regards,
>
>         Hans
>
> On 8/14/19 12:45 PM, 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:
> >       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 9862c322f0c4a..bd15902b825ad 100644
> > --- a/drivers/gpu/drm/sti/sti_hdmi.c
> > +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> > @@ -1256,6 +1256,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;
> > @@ -1318,6 +1319,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);
> >
> > @@ -1331,6 +1340,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 = {
> > @@ -1436,10 +1448,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))
> > @@ -1459,14 +1467,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;
> >  }
> >
> >
>

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

end of thread, back to index

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 10:44 [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Dariusz Marcinkiewicz
2019-08-14 10:44 ` [PATCH v7 1/9] drm_dp_cec: add connector info support Dariusz Marcinkiewicz
2019-08-15 18:10   ` Lyude Paul
2019-08-26  9:05     ` Ben Skeggs
2019-08-22  8:08   ` Hans Verkuil
2019-08-26  9:00     ` Hans Verkuil
2019-08-28 15:05   ` Ville Syrjälä
2019-08-14 10:45 ` [PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
2019-08-22  8:03   ` Hans Verkuil
2019-08-26  8:59     ` Hans Verkuil
2019-08-26 12:08   ` Ville Syrjälä
2019-08-14 10:45 ` [PATCH v7 3/9] dw-hdmi-cec: use cec_notifier_cec_adap_(un)register Dariusz Marcinkiewicz
2019-08-19 14:35   ` Neil Armstrong
2019-08-20  7:48     ` Neil Armstrong
2019-08-14 10:45 ` [PATCH v7 4/9] tda9950: " Dariusz Marcinkiewicz
2019-08-14 10:45 ` [PATCH v7 5/9] drm: tda998x: use cec_notifier_conn_(un)register Dariusz Marcinkiewicz
2019-08-19  9:30   ` Hans Verkuil
2019-08-19 11:22   ` [PATCH v7.1 " Dariusz Marcinkiewicz
2019-08-28  7:15     ` [PATCH v7.2 " Dariusz Marcinkiewicz
2019-08-25 13:12   ` [PATCH v7 " Hans Verkuil
2019-08-28  7:18     ` Dariusz Marcinkiewicz
2019-08-14 10:45 ` [PATCH v7 6/9] drm: sti: " Dariusz Marcinkiewicz
2019-08-19  9:34   ` Hans Verkuil
2019-08-22  8:11   ` Hans Verkuil
2019-09-02 13:11     ` Benjamin Gaignard
2019-08-14 10:45 ` [PATCH v7 7/9] drm: tegra: " Dariusz Marcinkiewicz
2019-08-19  9:33   ` Hans Verkuil
2019-08-28  8:09   ` Hans Verkuil
2019-08-28  9:38     ` Thierry Reding
2019-08-28 10:06       ` Hans Verkuil
2019-08-28 11:54         ` Thierry Reding
2019-08-28  9:36   ` Thierry Reding
2019-08-14 10:45 ` [PATCH v7 8/9] drm: dw-hdmi: " Dariusz Marcinkiewicz
2019-08-19  9:32   ` Hans Verkuil
2019-08-19 14:05     ` Hans Verkuil
2019-08-19 14:38       ` Neil Armstrong
2019-08-19 14:41         ` Hans Verkuil
2019-08-19 14:47           ` Neil Armstrong
2019-08-20  7:48             ` Neil Armstrong
2019-08-14 10:45 ` [PATCH v7 9/9] drm: exynos: exynos_hdmi: " Dariusz Marcinkiewicz
2019-08-19  9:32   ` Hans Verkuil
2019-08-28  8:39   ` Sylwester Nawrocki
2019-08-28 12:34     ` [PATCH v7.1 " Dariusz Marcinkiewicz
2019-08-28 12:38     ` [PATCH v7 " Dariusz Marcinkiewicz
2019-08-19  9:38 ` [PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API Hans Verkuil
2019-08-19 11:28   ` Dariusz Marcinkiewicz
2019-08-19 12:00     ` Hans Verkuil
2019-08-19 14:48   ` Neil Armstrong
2019-08-19 14:55     ` Hans Verkuil

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox