[PATCH 0/8] remove unsafe drm_for_each_connector()

[PATCH 0/8] remove unsafe drm_for_each_connector()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Hi all,

Something ligth in the middle of all my other tasks. I removed
drm_for_each_connector() in favor of drm_for_each_connector_iter().

The i915 patch was tested, but I only *built-tested* the others!

Gustavo

---
Gustavo Padovan (8):
  drm/exynos: use drm_for_each_connector_iter()
  drm/rockchip: use drm_for_each_connector_iter()
  drm/i915: use drm_for_each_connector_iter()
  drm/mediatek: use drm_for_each_connector_iter()
  drm/nouveau: use drm_for_each_connector_iter()
  drm/vc4: use drm_for_each_connector_iter()
  drm: remove unsafe drm_for_each_connector()
  drm: todo: remove task about switch to drm_connector_list_iter

 Documentation/gpu/todo.rst                  | 13 -------------
 drivers/gpu/drm/exynos/exynos_drm_drv.c     | 14 ++++++++------
 drivers/gpu/drm/i915/intel_display.c        |  5 ++++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  5 ++++-
 drivers/gpu/drm/nouveau/nv50_display.c      |  5 ++++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
 drivers/gpu/drm/vc4/vc4_crtc.c              |  5 ++++-
 include/drm/drm_connector.h                 | 15 ---------------
 8 files changed, 29 insertions(+), 43 deletions(-)

-- 
2.9.3

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

[PATCH 1/8] drm/exynos: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter()

Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 09d3c4c..db6ef2d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -177,12 +177,13 @@ static int exynos_drm_suspend(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
 	if (pm_runtime_suspended(dev) || !drm_dev)
 		return 0;
 
-	drm_modeset_lock_all(drm_dev);
-	drm_for_each_connector(connector, drm_dev) {
+	drm_connector_list_iter_begin(drm_dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		int old_dpms = connector->dpms;
 
 		if (connector->funcs->dpms)
@@ -191,7 +192,7 @@ static int exynos_drm_suspend(struct device *dev)
 		/* Set the old mode back to the connector for resume */
 		connector->dpms = old_dpms;
 	}
-	drm_modeset_unlock_all(drm_dev);
+	drm_connector_list_iter_end(&conn_iter);
 
 	return 0;
 }
@@ -200,12 +201,13 @@ static int exynos_drm_resume(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
 	if (pm_runtime_suspended(dev) || !drm_dev)
 		return 0;
 
-	drm_modeset_lock_all(drm_dev);
-	drm_for_each_connector(connector, drm_dev) {
+	drm_connector_list_iter_begin(drm_dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->funcs->dpms) {
 			int dpms = connector->dpms;
 
@@ -213,7 +215,7 @@ static int exynos_drm_resume(struct device *dev)
 			connector->funcs->dpms(connector, dpms);
 		}
 	}
-	drm_modeset_unlock_all(drm_dev);
+	drm_connector_list_iter_end(&conn_iter);
 
 	return 0;
 }
-- 
2.9.3

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

[PATCH 2/8] drm/rockchip: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter()

Cc: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 40a5e6e..4f042ab 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1118,16 +1118,16 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc,
 #ifdef CONFIG_DRM_ANALOGIX_DP
 static struct drm_connector *vop_get_edp_connector(struct vop *vop)
 {
-	struct drm_crtc *crtc = &vop->crtc;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
-	mutex_lock(&crtc->dev->mode_config.mutex);
-	drm_for_each_connector(connector, crtc->dev)
+	drm_connector_list_iter_begin(vop->drm_dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
-			mutex_unlock(&crtc->dev->mode_config.mutex);
+			drm_connector_list_iter_end(&conn_iter);
 			return connector;
 		}
-	mutex_unlock(&crtc->dev->mode_config.mutex);
+	drm_connector_list_iter_end(&conn_iter);
 
 	return NULL;
 }
-- 
2.9.3

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

[PATCH 3/8] drm/i915: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter().

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3617927..207f144 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11216,6 +11216,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 	unsigned int used_ports = 0;
 	unsigned int used_mst_ports = 0;
 
@@ -11224,7 +11225,8 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 	 * list to detect the problem on ddi platforms
 	 * where there's just one encoder per digital port.
 	 */
-	drm_for_each_connector(connector, dev) {
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		struct drm_connector_state *connector_state;
 		struct intel_encoder *encoder;
 
@@ -11263,6 +11265,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 			break;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	/* can't mix MST and SST/HDMI on the same port */
 	if (used_ports & used_mst_ports)
-- 
2.9.3

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

[PATCH 4/8] drm/mediatek: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter().

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 6b08774..6582e1f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -221,6 +221,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
 	struct drm_crtc *crtc = &mtk_crtc->base;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
+	struct drm_connector_list_iter conn_iter;
 	unsigned int width, height, vrefresh, bpc = MTK_MAX_BPC;
 	int ret;
 	int i;
@@ -237,13 +238,15 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
 		if (encoder->crtc != crtc)
 			continue;
 
-		drm_for_each_connector(connector, crtc->dev) {
+		drm_connector_list_iter_begin(crtc->dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
 			if (connector->encoder != encoder)
 				continue;
 			if (connector->display_info.bpc != 0 &&
 			    bpc > connector->display_info.bpc)
 				bpc = connector->display_info.bpc;
 		}
+		drm_connector_list_iter_end(&conn_iter);
 	}
 
 	ret = pm_runtime_get_sync(crtc->dev->dev);
-- 
2.9.3

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

[PATCH 5/8] drm/nouveau: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter().

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/nouveau/nv50_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 0e58537..8790bdc 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2871,17 +2871,20 @@ nv50_msto_enable(struct drm_encoder *encoder)
 	struct nv50_mstc *mstc = NULL;
 	struct nv50_mstm *mstm = NULL;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 	u8 proto, depth;
 	int slots;
 	bool r;
 
-	drm_for_each_connector(connector, encoder->dev) {
+	drm_connector_list_iter_begin(encoder->dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->state->best_encoder == &msto->encoder) {
 			mstc = nv50_mstc(connector);
 			mstm = mstc->mstm;
 			break;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	if (WARN_ON(!mstc))
 		return;
-- 
2.9.3

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

[PATCH 6/8] drm/vc4: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter().

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index d86c8cc..feee83a 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -359,12 +359,15 @@ static u32 vc4_get_fifo_full_level(u32 format)
 static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
 {
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
-	drm_for_each_connector(connector, crtc->dev) {
+	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->state->crtc == crtc) {
 			return connector->encoder;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	return NULL;
 }
-- 
2.9.3

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

[PATCH 7/8] drm: remove unsafe drm_for_each_connector()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

After converting all users to drm_for_each_connector_iter() we no
longer need drm_for_each_connector() so we can go ahead and remove it.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
---
 include/drm/drm_connector.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4eeda12..ffb8ec1 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -989,21 +989,6 @@ void drm_mode_put_tile_group(struct drm_device *dev,
 			     struct drm_tile_group *tg);
 
 /**
- * drm_for_each_connector - iterate over all connectors
- * @connector: the loop cursor
- * @dev: the DRM device
- *
- * Iterate over all connectors of @dev.
- *
- * WARNING:
- *
- * This iterator is not safe against hotadd/removal of connectors and is
- * deprecated. Use drm_for_each_connector_iter() instead.
- */
-#define drm_for_each_connector(connector, dev) \
-	list_for_each_entry(connector, &(dev)->mode_config.connector_list, head)
-
-/**
  * struct drm_connector_list_iter - connector_list iterator
  *
  * This iterator tracks state needed to be able to walk the connector_list
-- 
2.9.3

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

[PATCH 8/8] drm: todo: remove task about switch to drm_connector_list_iter

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

This is now completed.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
---
 Documentation/gpu/todo.rst | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 1bdb735..95a5170 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -177,19 +177,6 @@ following drivers still use ``struct_mutex``: ``msm``, ``omapdrm`` and
 
 Contact: Daniel Vetter, respective driver maintainers
 
-Switch to drm_connector_list_iter for any connector_list walking
-----------------------------------------------------------------
-
-Connectors can be hotplugged, and we now have a special list of helpers to walk
-the connector_list in a race-free fashion, without incurring deadlocks on
-mutexes and other fun stuff.
-
-Unfortunately most drivers are not converted yet. At least all those supporting
-DP MST hotplug should be converted, since for those drivers the difference
-matters. See drm_for_each_connector_iter() vs. drm_for_each_connector().
-
-Contact: Daniel Vetter
-
 Core refactorings
 =================
 
-- 
2.9.3

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

Re: [PATCH 1/8] drm/exynos: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:42PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter()
> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!
> ---

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 09d3c4c..db6ef2d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -177,12 +177,13 @@ static int exynos_drm_suspend(struct device *dev)
>  {
>  	struct drm_device *drm_dev = dev_get_drvdata(dev);
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
>  	if (pm_runtime_suspended(dev) || !drm_dev)
>  		return 0;
>  
> -	drm_modeset_lock_all(drm_dev);
> -	drm_for_each_connector(connector, drm_dev) {
> +	drm_connector_list_iter_begin(drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		int old_dpms = connector->dpms;
>  
>  		if (connector->funcs->dpms)
> @@ -191,7 +192,7 @@ static int exynos_drm_suspend(struct device *dev)
>  		/* Set the old mode back to the connector for resume */
>  		connector->dpms = old_dpms;
>  	}
> -	drm_modeset_unlock_all(drm_dev);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return 0;
>  }
> @@ -200,12 +201,13 @@ static int exynos_drm_resume(struct device *dev)
>  {
>  	struct drm_device *drm_dev = dev_get_drvdata(dev);
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
>  	if (pm_runtime_suspended(dev) || !drm_dev)
>  		return 0;
>  
> -	drm_modeset_lock_all(drm_dev);
> -	drm_for_each_connector(connector, drm_dev) {
> +	drm_connector_list_iter_begin(drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->funcs->dpms) {
>  			int dpms = connector->dpms;
>  
> @@ -213,7 +215,7 @@ static int exynos_drm_resume(struct device *dev)
>  			connector->funcs->dpms(connector, dpms);
>  		}
>  	}
> -	drm_modeset_unlock_all(drm_dev);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return 0;
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/8] drm/rockchip: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:43PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter()
> 
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 40a5e6e..4f042ab 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1118,16 +1118,16 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc,
>  #ifdef CONFIG_DRM_ANALOGIX_DP
>  static struct drm_connector *vop_get_edp_connector(struct vop *vop)
>  {
> -	struct drm_crtc *crtc = &vop->crtc;
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
> -	mutex_lock(&crtc->dev->mode_config.mutex);
> -	drm_for_each_connector(connector, crtc->dev)
> +	drm_connector_list_iter_begin(vop->drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> -			mutex_unlock(&crtc->dev->mode_config.mutex);
> +			drm_connector_list_iter_end(&conn_iter);
>  			return connector;
>  		}
> -	mutex_unlock(&crtc->dev->mode_config.mutex);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return NULL;
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/8] drm/i915: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:44PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter().
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3617927..207f144 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11216,6 +11216,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  	unsigned int used_ports = 0;
>  	unsigned int used_mst_ports = 0;
>  
> @@ -11224,7 +11225,8 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  	 * list to detect the problem on ddi platforms
>  	 * where there's just one encoder per digital port.
>  	 */
> -	drm_for_each_connector(connector, dev) {
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {

Since this checks an atomic state it'd probably be better to use
for_each_new_connector_in_state (to make sure we check the same connectors
as in the atomic state, and not something else due to hotplug changes
meanwhile). But this is ok as the mechanical change, so

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  		struct drm_connector_state *connector_state;
>  		struct intel_encoder *encoder;
>  
> @@ -11263,6 +11265,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  			break;
>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	/* can't mix MST and SST/HDMI on the same port */
>  	if (used_ports & used_mst_ports)
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/8] drm/mediatek: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:45PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter().
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 6b08774..6582e1f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -221,6 +221,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
>  	struct drm_crtc *crtc = &mtk_crtc->base;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> +	struct drm_connector_list_iter conn_iter;
>  	unsigned int width, height, vrefresh, bpc = MTK_MAX_BPC;
>  	int ret;
>  	int i;
> @@ -237,13 +238,15 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
>  		if (encoder->crtc != crtc)
>  			continue;
>  
> -		drm_for_each_connector(connector, crtc->dev) {
> +		drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> +		drm_for_each_connector_iter(connector, &conn_iter) {
>  			if (connector->encoder != encoder)
>  				continue;
>  			if (connector->display_info.bpc != 0 &&
>  			    bpc > connector->display_info.bpc)
>  				bpc = connector->display_info.bpc;

Computing bpc here is probably a bug, should be done in atomic_check most
likely. But there's already a todo about this.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  		}
> +		drm_connector_list_iter_end(&conn_iter);
>  	}
>  
>  	ret = pm_runtime_get_sync(crtc->dev->dev);
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/8] drm/nouveau: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:46PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter().
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!
> ---
>  drivers/gpu/drm/nouveau/nv50_display.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 0e58537..8790bdc 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -2871,17 +2871,20 @@ nv50_msto_enable(struct drm_encoder *encoder)
>  	struct nv50_mstc *mstc = NULL;
>  	struct nv50_mstm *mstm = NULL;
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  	u8 proto, depth;
>  	int slots;
>  	bool r;
>  
> -	drm_for_each_connector(connector, encoder->dev) {
> +	drm_connector_list_iter_begin(encoder->dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->state->best_encoder == &msto->encoder) {
>  			mstc = nv50_mstc(connector);
>  			mstm = mstc->mstm;
>  			break;

We probably want an encoder->atomic_enable which passes both states
(crtc&connector) instead of fun like this. But that's a different storey.
See i915 for how we've done this for our intel-specific encoder callbacks.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	if (WARN_ON(!mstc))
>  		return;
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/8] drm/vc4: use drm_for_each_connector_iter()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:47PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter().
> 
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!
> ---
>  drivers/gpu/drm/vc4/vc4_crtc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index d86c8cc..feee83a 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -359,12 +359,15 @@ static u32 vc4_get_fifo_full_level(u32 format)
>  static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
>  {
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
> -	drm_for_each_connector(connector, crtc->dev) {
> +	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->state->crtc == crtc) {

Needs an iter_end here. With that fixed,

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  			return connector->encoder;
>  		}
>  	}
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return NULL;
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/8] drm: remove unsafe drm_for_each_connector()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:48PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> After converting all users to drm_for_each_connector_iter() we no
> longer need drm_for_each_connector() so we can go ahead and remove it.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  include/drm/drm_connector.h | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4eeda12..ffb8ec1 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -989,21 +989,6 @@ void drm_mode_put_tile_group(struct drm_device *dev,
>  			     struct drm_tile_group *tg);
>  
>  /**
> - * drm_for_each_connector - iterate over all connectors
> - * @connector: the loop cursor
> - * @dev: the DRM device
> - *
> - * Iterate over all connectors of @dev.
> - *
> - * WARNING:
> - *
> - * This iterator is not safe against hotadd/removal of connectors and is
> - * deprecated. Use drm_for_each_connector_iter() instead.
> - */
> -#define drm_for_each_connector(connector, dev) \
> -	list_for_each_entry(connector, &(dev)->mode_config.connector_list, head)
> -
> -/**
>   * struct drm_connector_list_iter - connector_list iterator
>   *
>   * This iterator tracks state needed to be able to walk the connector_list
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/8] drm: todo: remove task about switch to drm_connector_list_iter

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:49PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> This is now completed.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  Documentation/gpu/todo.rst | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 1bdb735..95a5170 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -177,19 +177,6 @@ following drivers still use ``struct_mutex``: ``msm``, ``omapdrm`` and
>  
>  Contact: Daniel Vetter, respective driver maintainers
>  
> -Switch to drm_connector_list_iter for any connector_list walking
> -----------------------------------------------------------------
> -
> -Connectors can be hotplugged, and we now have a special list of helpers to walk
> -the connector_list in a race-free fashion, without incurring deadlocks on
> -mutexes and other fun stuff.
> -
> -Unfortunately most drivers are not converted yet. At least all those supporting
> -DP MST hotplug should be converted, since for those drivers the difference
> -matters. See drm_for_each_connector_iter() vs. drm_for_each_connector().
> -
> -Contact: Daniel Vetter
> -
>  Core refactorings
>  =================
>  
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/8] remove unsafe drm_for_each_connector()

[Daniel Vetter]

On Thu, May 11, 2017 at 04:10:41PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Hi all,
> 
> Something ligth in the middle of all my other tasks. I removed
> drm_for_each_connector() in favor of drm_for_each_connector_iter().
> 
> The i915 patch was tested, but I only *built-tested* the others!

Thanks for doing this. Dropped a few comments just for more context,
spotted 1 bug in the vc4 patch, but otherwise looks all good.

I'd say respin the vc4 one, then wait 1-2 weeks for driver maintainer
acks/t-b, and then push it all through drm-misc.
-Daniel

> 
> Gustavo
> 
> ---
> Gustavo Padovan (8):
>   drm/exynos: use drm_for_each_connector_iter()
>   drm/rockchip: use drm_for_each_connector_iter()
>   drm/i915: use drm_for_each_connector_iter()
>   drm/mediatek: use drm_for_each_connector_iter()
>   drm/nouveau: use drm_for_each_connector_iter()
>   drm/vc4: use drm_for_each_connector_iter()
>   drm: remove unsafe drm_for_each_connector()
>   drm: todo: remove task about switch to drm_connector_list_iter
> 
>  Documentation/gpu/todo.rst                  | 13 -------------
>  drivers/gpu/drm/exynos/exynos_drm_drv.c     | 14 ++++++++------
>  drivers/gpu/drm/i915/intel_display.c        |  5 ++++-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  5 ++++-
>  drivers/gpu/drm/nouveau/nv50_display.c      |  5 ++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
>  drivers/gpu/drm/vc4/vc4_crtc.c              |  5 ++++-
>  include/drm/drm_connector.h                 | 15 ---------------
>  8 files changed, 29 insertions(+), 43 deletions(-)
> 
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/vc4: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter().

v2: add missing drm_connector_list_iter_end(Daniel Vetter)

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

---
only built-tested!
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index d86c8cc..d66345f 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -359,12 +359,16 @@ static u32 vc4_get_fifo_full_level(u32 format)
 static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
 {
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
-	drm_for_each_connector(connector, crtc->dev) {
+	drm_connector_list_iter_begin(crtc->dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->state->crtc == crtc) {
+			drm_connector_list_iter_end(&conn_iter);
 			return connector->encoder;
 		}
 	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	return NULL;
 }
-- 
2.9.3

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

Re: [PATCH 3/8] drm/i915: use drm_for_each_connector_iter()

[Ville Syrjälä]

On Fri, May 12, 2017 at 09:42:20AM +0200, Daniel Vetter wrote:
> On Thu, May 11, 2017 at 04:10:44PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.com>
> > 
> > Drop legacy drm_for_each_connector() in favor of the race-free
> > drm_for_each_connector_iter().
> > 
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3617927..207f144 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11216,6 +11216,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  {
> >  	struct drm_device *dev = state->dev;
> >  	struct drm_connector *connector;
> > +	struct drm_connector_list_iter conn_iter;
> >  	unsigned int used_ports = 0;
> >  	unsigned int used_mst_ports = 0;
> >  
> > @@ -11224,7 +11225,8 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  	 * list to detect the problem on ddi platforms
> >  	 * where there's just one encoder per digital port.
> >  	 */
> > -	drm_for_each_connector(connector, dev) {
> > +	drm_connector_list_iter_begin(dev, &conn_iter);
> > +	drm_for_each_connector_iter(connector, &conn_iter) {
> 
> Since this checks an atomic state it'd probably be better to use
> for_each_new_connector_in_state (to make sure we check the same connectors
> as in the atomic state,

We need to check all ports here, not just the ones part of the state.

> and not something else due to hotplug changes
> meanwhile). But this is ok as the mechanical change, so
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> >  		struct drm_connector_state *connector_state;
> >  		struct intel_encoder *encoder;
> >  
> > @@ -11263,6 +11265,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  			break;
> >  		}
> >  	}
> > +	drm_connector_list_iter_end(&conn_iter);
> >  
> >  	/* can't mix MST and SST/HDMI on the same port */
> >  	if (used_ports & used_mst_ports)
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/vc4: use drm_for_each_connector_iter()

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]

Gustavo Padovan <gustavo@padovan.org> writes:

> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter().
>
> v2: add missing drm_connector_list_iter_end(Daniel Vetter)
>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Acked-by: Eric Anholt <eric@anholt.net>

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/8] drm/rockchip: use drm_for_each_connector_iter()

[Sean Paul]

On Thu, May 11, 2017 at 04:10:43PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter()
> 
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> ---
> only built-tested!
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 40a5e6e..4f042ab 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1118,16 +1118,16 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc,
>  #ifdef CONFIG_DRM_ANALOGIX_DP
>  static struct drm_connector *vop_get_edp_connector(struct vop *vop)
>  {
> -	struct drm_crtc *crtc = &vop->crtc;
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
> -	mutex_lock(&crtc->dev->mode_config.mutex);
> -	drm_for_each_connector(connector, crtc->dev)
> +	drm_connector_list_iter_begin(vop->drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {

Unmatched brace here. This code is protected by CONFIG_DRM_ANALOGIX_DP, so I'm
guessing that's why your build test didn't pick it up.

Sean

>  		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> -			mutex_unlock(&crtc->dev->mode_config.mutex);
> +			drm_connector_list_iter_end(&conn_iter);
>  			return connector;
>  		}
> -	mutex_unlock(&crtc->dev->mode_config.mutex);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return NULL;
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/8] remove unsafe drm_for_each_connector()

[Sean Paul]

On Thu, May 11, 2017 at 04:10:41PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Hi all,
> 
> Something ligth in the middle of all my other tasks. I removed
> drm_for_each_connector() in favor of drm_for_each_connector_iter().
> 
> The i915 patch was tested, but I only *built-tested* the others!
> 
> Gustavo
> 
> ---

Hi Gustavo,
Aside from the rockchip splat, consider the rest of the series:

Reviewed-by: Sean Paul <seanpaul@chromium.org>

Once you have a v2 rockchip, I'll test and ack.

Sean

> Gustavo Padovan (8):
>   drm/exynos: use drm_for_each_connector_iter()
>   drm/rockchip: use drm_for_each_connector_iter()
>   drm/i915: use drm_for_each_connector_iter()
>   drm/mediatek: use drm_for_each_connector_iter()
>   drm/nouveau: use drm_for_each_connector_iter()
>   drm/vc4: use drm_for_each_connector_iter()
>   drm: remove unsafe drm_for_each_connector()
>   drm: todo: remove task about switch to drm_connector_list_iter
> 
>  Documentation/gpu/todo.rst                  | 13 -------------
>  drivers/gpu/drm/exynos/exynos_drm_drv.c     | 14 ++++++++------
>  drivers/gpu/drm/i915/intel_display.c        |  5 ++++-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  5 ++++-
>  drivers/gpu/drm/nouveau/nv50_display.c      |  5 ++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++++-----
>  drivers/gpu/drm/vc4/vc4_crtc.c              |  5 ++++-
>  include/drm/drm_connector.h                 | 15 ---------------
>  8 files changed, 29 insertions(+), 43 deletions(-)
> 
> -- 
> 2.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/rockchip: use drm_for_each_connector_iter()

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Drop legacy drm_for_each_connector() in favor of the race-free
drm_for_each_connector_iter()

Cc: Mark Yao <mark.yao@rock-chips.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

---
only built-tested!
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 40a5e6e..9b3525a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1118,16 +1118,17 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc,
 #ifdef CONFIG_DRM_ANALOGIX_DP
 static struct drm_connector *vop_get_edp_connector(struct vop *vop)
 {
-	struct drm_crtc *crtc = &vop->crtc;
 	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
 
-	mutex_lock(&crtc->dev->mode_config.mutex);
-	drm_for_each_connector(connector, crtc->dev)
+	drm_connector_list_iter_begin(vop->drm_dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
 		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
-			mutex_unlock(&crtc->dev->mode_config.mutex);
+			drm_connector_list_iter_end(&conn_iter);
 			return connector;
 		}
-	mutex_unlock(&crtc->dev->mode_config.mutex);
+	}
+	drm_connector_list_iter_end(&conn_iter);
 
 	return NULL;
 }
-- 
2.9.3

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

Re: [PATCH 8/8] drm: todo: remove task about switch to drm_connector_list_iter

[Gustavo Padovan]

2017-05-11 Gustavo Padovan <gustavo@padovan.org>:

> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> This is now completed.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> ---
>  Documentation/gpu/todo.rst | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 1bdb735..95a5170 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -177,19 +177,6 @@ following drivers still use ``struct_mutex``: ``msm``, ``omapdrm`` and
>  
>  Contact: Daniel Vetter, respective driver maintainers
>  
> -Switch to drm_connector_list_iter for any connector_list walking
> -----------------------------------------------------------------
> -
> -Connectors can be hotplugged, and we now have a special list of helpers to walk
> -the connector_list in a race-free fashion, without incurring deadlocks on
> -mutexes and other fun stuff.
> -
> -Unfortunately most drivers are not converted yet. At least all those supporting
> -DP MST hotplug should be converted, since for those drivers the difference
> -matters. See drm_for_each_connector_iter() vs. drm_for_each_connector().
> -
> -Contact: Daniel Vetter
> -
>  Core refactorings
>  =================
>  
> -- 
> 2.9.3
> 

Pushed all patches to drm-misc-next. Thanks all for the reviews.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm/exynos: use drm_for_each_connector_iter()

[Inki Dae]

2017년 05월 12일 04:10에 Gustavo Padovan 이(가) 쓴 글:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Drop legacy drm_for_each_connector() in favor of the race-free
> drm_for_each_connector_iter()
> 
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>

Seems this patch with other could go to drm-misc.

Signed-off-by : Inki Dae <inki.dae@samsung.com>

Thanks,
Inki Dae

> 
> ---
> only built-tested!
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 09d3c4c..db6ef2d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -177,12 +177,13 @@ static int exynos_drm_suspend(struct device *dev)
>  {
>  	struct drm_device *drm_dev = dev_get_drvdata(dev);
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
>  	if (pm_runtime_suspended(dev) || !drm_dev)
>  		return 0;
>  
> -	drm_modeset_lock_all(drm_dev);
> -	drm_for_each_connector(connector, drm_dev) {
> +	drm_connector_list_iter_begin(drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		int old_dpms = connector->dpms;
>  
>  		if (connector->funcs->dpms)
> @@ -191,7 +192,7 @@ static int exynos_drm_suspend(struct device *dev)
>  		/* Set the old mode back to the connector for resume */
>  		connector->dpms = old_dpms;
>  	}
> -	drm_modeset_unlock_all(drm_dev);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return 0;
>  }
> @@ -200,12 +201,13 @@ static int exynos_drm_resume(struct device *dev)
>  {
>  	struct drm_device *drm_dev = dev_get_drvdata(dev);
>  	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
>  
>  	if (pm_runtime_suspended(dev) || !drm_dev)
>  		return 0;
>  
> -	drm_modeset_lock_all(drm_dev);
> -	drm_for_each_connector(connector, drm_dev) {
> +	drm_connector_list_iter_begin(drm_dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
>  		if (connector->funcs->dpms) {
>  			int dpms = connector->dpms;
>  
> @@ -213,7 +215,7 @@ static int exynos_drm_resume(struct device *dev)
>  			connector->funcs->dpms(connector, dpms);
>  		}
>  	}
> -	drm_modeset_unlock_all(drm_dev);
> +	drm_connector_list_iter_end(&conn_iter);
>  
>  	return 0;
>  }
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel