Make connector registration automatic

Make connector registration automatic

[Chris Wilson]

The current recommendation is for new drivers to call:
	drm_dev_register();
	drm_connector_register_all();
because of a limitation in the API for old drivers using driver->load()
and being unable to call drm_connector_register() multiple times. Now
that drm_connector_register() is safe for multiple uses, we can combine
the two steps into one (drm_dev_register()).
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm: Automatically register/unregister all connectors

[Chris Wilson]

As the drm_connector is now safe for multiple calls to
register/unregister, automatically perform a registration on all known
connectors drm drv_register (and unregister from drm_drv_unregister).
Drivers can still call drm_connector_register() and
drm_connector_unregister() individually, or defer as required.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_crtc.c |  6 +++---
 drivers/gpu/drm/drm_drv.c  | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 316dea9bea08..e7c862bd2f19 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1047,9 +1047,9 @@ EXPORT_SYMBOL(drm_connector_unregister);
  * @dev: drm device
  *
  * This function registers all connectors in sysfs and other places so that
- * userspace can start to access them. Drivers can call it after calling
- * drm_dev_register() to complete the device registration, if they don't call
- * drm_connector_register() on each connector individually.
+ * userspace can start to access them. drm_connector_register_all() is called
+ * automatically from drm_dev_register() to complete the device registration,
+ * if they don't call drm_connector_register() on each connector individually.
  *
  * When a device is unplugged and should be removed from userspace access,
  * call drm_connector_unregister_all(), which is the inverse of this
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 40fb4352432c..2067ff089380 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -650,11 +650,7 @@ EXPORT_SYMBOL(drm_dev_unref);
  *
  * Register the DRM device @dev with the system, advertise device to user-space
  * and start normal device operation. @dev must be allocated via drm_dev_alloc()
- * previously. Right after drm_dev_register() the driver should call
- * drm_connector_register_all() to register all connectors in sysfs. This is
- * a separate call for backward compatibility with drivers still using
- * the deprecated ->load() callback, where connectors are registered from within
- * the ->load() callback.
+ * previously.
  *
  * Never call this twice on any device!
  *
@@ -691,6 +687,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 			goto err_minors;
 	}
 
+	drm_connector_register_all(dev);
+
 	ret = 0;
 	goto out_unlock;
 
@@ -721,6 +719,8 @@ void drm_dev_unregister(struct drm_device *dev)
 
 	drm_lastclose(dev);
 
+	drm_connector_unregister_all(dev);
+
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/7] drm/arc: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/arc/arcpgu_drv.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 381c5fcbf903..ccbdadb108dc 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -211,15 +211,8 @@ static int arcpgu_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unload;
 
-	ret = drm_connector_register_all(drm);
-	if (ret)
-		goto err_unregister;
-
 	return 0;
 
-err_unregister:
-	drm_dev_unregister(drm);
-
 err_unload:
 	arcpgu_unload(drm);
 
@@ -233,7 +226,6 @@ static int arcpgu_remove(struct platform_device *pdev)
 {
 	struct drm_device *drm = platform_get_drvdata(pdev);
 
-	drm_connector_unregister_all(drm);
 	drm_dev_unregister(drm);
 	arcpgu_unload(drm);
 	drm_dev_unref(drm);
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/atmel-hlcdc: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 9ecf16c7911d..99c4af697c8a 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -815,15 +815,8 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unload;
 
-	ret = drm_connector_register_all(ddev);
-	if (ret)
-		goto err_unregister;
-
 	return 0;
 
-err_unregister:
-	drm_dev_unregister(ddev);
-
 err_unload:
 	atmel_hlcdc_dc_unload(ddev);
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/hisilicon: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 193657259ee9..ef2c32ec1616 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -221,19 +221,12 @@ static int kirin_drm_bind(struct device *dev)
 	if (ret)
 		goto err_kms_cleanup;
 
-	/* connectors should be registered after drm device register */
-	ret = drm_connector_register_all(drm_dev);
-	if (ret)
-		goto err_drm_dev_unregister;
-
 	DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, drm_dev->primary->index);
 
 	return 0;
 
-err_drm_dev_unregister:
-	drm_dev_unregister(drm_dev);
 err_kms_cleanup:
 	kirin_drm_kms_cleanup(drm_dev);
 err_drm_dev_unref:
@@ -246,7 +239,6 @@ static void kirin_drm_unbind(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
 
-	drm_connector_unregister_all(drm_dev);
 	drm_dev_unregister(drm_dev);
 	kirin_drm_kms_cleanup(drm_dev);
 	drm_dev_unref(drm_dev);
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/mediatek: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c33bf98c5d5e..7ab91f4a200f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -293,14 +293,8 @@ static int mtk_drm_bind(struct device *dev)
 	if (ret < 0)
 		goto err_deinit;
 
-	ret = drm_connector_register_all(drm);
-	if (ret < 0)
-		goto err_unregister;
-
 	return 0;
 
-err_unregister:
-	drm_dev_unregister(drm);
 err_deinit:
 	mtk_drm_kms_deinit(drm);
 err_free:
@@ -455,7 +449,6 @@ static int mtk_drm_remove(struct platform_device *pdev)
 	struct drm_device *drm = private->drm;
 	int i;
 
-	drm_connector_unregister_all(drm);
 	drm_dev_unregister(drm);
 	mtk_drm_kms_deinit(drm);
 	drm_dev_unref(drm);
-- 
2.8.1

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

[PATCH 5/7] drm/mediatek: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-mediatek at lists.infradead.org
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c33bf98c5d5e..7ab91f4a200f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -293,14 +293,8 @@ static int mtk_drm_bind(struct device *dev)
 	if (ret < 0)
 		goto err_deinit;
 
-	ret = drm_connector_register_all(drm);
-	if (ret < 0)
-		goto err_unregister;
-
 	return 0;
 
-err_unregister:
-	drm_dev_unregister(drm);
 err_deinit:
 	mtk_drm_kms_deinit(drm);
 err_free:
@@ -455,7 +449,6 @@ static int mtk_drm_remove(struct platform_device *pdev)
 	struct drm_device *drm = private->drm;
 	int i;
 
-	drm_connector_unregister_all(drm);
 	drm_dev_unregister(drm);
 	mtk_drm_kms_deinit(drm);
 	drm_dev_unref(drm);
-- 
2.8.1

[PATCH 6/7] drm/msm: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rob Clark <robdclark@gmail.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/msm_drv.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9c654092ef78..568fcc328f27 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -197,8 +197,6 @@ static int msm_drm_uninit(struct device *dev)
 
 	drm_kms_helper_poll_fini(ddev);
 
-	drm_connector_unregister_all(ddev);
-
 	drm_dev_unregister(ddev);
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
@@ -431,12 +429,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
 	if (ret)
 		goto fail;
 
-	ret = drm_connector_register_all(ddev);
-	if (ret) {
-		dev_err(dev, "failed to register connectors\n");
-		goto fail;
-	}
-
 	drm_mode_config_reset(ddev);
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
-- 
2.8.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 48ec4b6e8b26..36d390093c92 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev)
 	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
 	struct drm_device *ddev = rcdu->ddev;
 
-	drm_connector_unregister_all(ddev);
 	drm_dev_unregister(ddev);
 
 	if (rcdu->fbdev)
@@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev)
 	if (ret)
 		goto error;
 
-	ret = drm_connector_register_all(ddev);
-	if (ret < 0)
-		goto error;
-
 	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
 
 	return 0;
-- 
2.8.1

[PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()

[Chris Wilson]

Up to now, the recommendation was for drivers to call drm_dev_register()
followed by drm_connector_register_all(). Now that
drm_connector_register() is safe against multiple invocations, we can
move drm_connector_register_all() to drm_dev_register() and not suffer
from any backwards compatibility issues with drivers not following the
more rigorous init ordering.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 48ec4b6e8b26..36d390093c92 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev)
 	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
 	struct drm_device *ddev = rcdu->ddev;
 
-	drm_connector_unregister_all(ddev);
 	drm_dev_unregister(ddev);
 
 	if (rcdu->fbdev)
@@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev)
 	if (ret)
 		goto error;
 
-	ret = drm_connector_register_all(ddev);
-	if (ret < 0)
-		goto error;
-
 	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
 
 	return 0;
-- 
2.8.1

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

✗ Ro.CI.BAT: warning for series starting with [1/7] drm: Automatically register/unregister all connectors

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm: Automatically register/unregister all connectors
URL   : https://patchwork.freedesktop.org/series/8814/
State : warning

== Summary ==

Series 8814v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8814/revisions/1/mbox

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-ilk1-i5-650)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (ro-snb-i7-2620M)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-ivb-i7-3770)
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)
                pass       -> DMESG-WARN (ro-byt-n2820)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)

fi-skl-i7-6700k  total:213  pass:187  dwarn:1   dfail:0   fail:0   skip:25 
ro-bdw-i5-5250u  total:213  pass:196  dwarn:2   dfail:0   fail:0   skip:15 
ro-bdw-i7-5557U  total:213  pass:197  dwarn:1   dfail:0   fail:0   skip:15 
ro-bdw-i7-5600u  total:213  pass:184  dwarn:1   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:171  dwarn:1   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:172  dwarn:1   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:213  pass:189  dwarn:1   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:189  dwarn:1   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:149  dwarn:1   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:208  pass:149  dwarn:1   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:180  dwarn:1   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:184  dwarn:1   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:173  dwarn:1   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1203/

1ed16fe drm-intel-nightly: 2016y-06m-17d-07h-46m-19s UTC integration manifest
3461c4a drm/rcar-du: Remove redundant calls to drm_connector_register_all()
b53ed5d drm/msm: Remove redundant calls to drm_connector_register_all()
b6e260e drm/mediatek: Remove redundant calls to drm_connector_register_all()
81f49e6 drm/hisilicon: Remove redundant calls to drm_connector_register_all()
0a63e9e drm/atmel-hlcdc: Remove redundant calls to drm_connector_register_all()
aef238b drm/arc: Remove redundant calls to drm_connector_register_all()
234b384 drm: Automatically register/unregister all connectors

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: warning for series starting with [1/7] drm: Automatically register/unregister all connectors

[Chris Wilson]

On Fri, Jun 17, 2016 at 08:47:04AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/7] drm: Automatically register/unregister all connectors
> URL   : https://patchwork.freedesktop.org/series/8814/
> State : warning
> 
> == Summary ==
> 
> Series 8814v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/8814/revisions/1/mbox
> 
> Test drv_module_reload_basic:
>                 pass       -> DMESG-WARN (ro-bdw-i7-5557U)
>                 pass       -> DMESG-WARN (ro-ilk1-i5-650)
>                 pass       -> DMESG-WARN (ro-hsw-i3-4010u)
>                 pass       -> DMESG-WARN (ro-bdw-i7-5600u)
>                 pass       -> DMESG-WARN (ro-snb-i7-2620M)
>                 pass       -> DMESG-WARN (ro-bdw-i5-5250u)
>                 pass       -> DMESG-WARN (ro-hsw-i7-4770r)
>                 pass       -> DMESG-WARN (ro-ivb-i7-3770)
>                 pass       -> DMESG-WARN (ro-ivb2-i7-3770)
>                 pass       -> DMESG-WARN (ro-byt-n2820)
>                 pass       -> DMESG-WARN (fi-skl-i7-6700k)

So this is because i915 is one of the bad drivers still using
driver->unload and not updating its init ordering:

[  160.838919] ------------[ cut here ]------------
[  160.838933] WARNING: CPU: 2 PID: 5784 at fs/sysfs/group.c:237 sysfs_remove_group+0x86/0x90
[  160.838938] sysfs group ffffffff81cc8380 not found for kobject 'intel_backlight'
[  160.838941] Modules linked in: i915(-) i2c_algo_bit drm_kms_helper snd_hda_codec_hdmi snd_hda_codec_generic syscopyarea sysfillrect sysimgblt fb_sys_fops x86_pkg_temp_thermal intel_powerclamp drm snd_hda_codec snd_hwdep snd_hda_core coretemp snd_pcm crct10dif_pclmul crc32_pclmul mei_me ghash_clmulni_intel lpc_ich mei sdhci_pci sdhci mmc_core e1000e ptp pps_core [last unloaded: snd_hda_intel]
[  160.838998] CPU: 2 PID: 5784 Comm: rmmod Tainted: G     U          4.7.0-rc2-gfxbench-RO_Patchwork_1203+ #1
[  160.839003] Hardware name: LENOVO 4287CTO/4287CTO, BIOS 8DET47WW (1.17 ) 05/31/2011
[  160.839006]  0000000000000000 ffff880212bd3bc0 ffffffff8140e085 ffff880212bd3c10
[  160.839016]  0000000000000000 ffff880212bd3c00 ffffffff8107a996 000000ed81c797e0
[  160.839024]  0000000000000000 ffffffff81cc8380 ffff88020bc6c290 ffff88020bed0000
[  160.839032] Call Trace:
[  160.839040]  [<ffffffff8140e085>] dump_stack+0x67/0x92
[  160.839047]  [<ffffffff8107a996>] __warn+0xc6/0xe0
[  160.839052]  [<ffffffff8107a9fa>] warn_slowpath_fmt+0x4a/0x50
[  160.839060]  [<ffffffff817a92e9>] ? mutex_unlock+0x9/0x10
[  160.839065]  [<ffffffff8125b375>] ? kernfs_find_and_get_ns+0x45/0x50
[  160.839070]  [<ffffffff8125ec66>] sysfs_remove_group+0x86/0x90
[  160.839076]  [<ffffffff81523e22>] dpm_sysfs_remove+0x52/0x60
[  160.839083]  [<ffffffff81517834>] device_del+0x44/0x240
[  160.839088]  [<ffffffff810cbd7a>] ? up_write+0x1a/0x40
[  160.839093]  [<ffffffff81517a49>] device_unregister+0x19/0x60
[  160.839099]  [<ffffffff81475fb9>] backlight_device_unregister+0x89/0x90
[  160.839155]  [<ffffffffa039a25f>] intel_backlight_unregister+0x2f/0x50 [i915]
[  160.839192]  [<ffffffffa0367ad9>] intel_modeset_cleanup+0x29/0xb0 [i915]
[  160.839225]  [<ffffffffa03a257c>] i915_driver_unload+0x6c/0x1b0 [i915]
[  160.839241]  [<ffffffffa018799c>] drm_dev_unregister+0x2c/0xb0 [drm]
[  160.839253]  [<ffffffffa0187f2e>] drm_put_dev+0x1e/0x50 [drm]
[  160.839274]  [<ffffffffa02d9300>] i915_pci_remove+0x10/0x20 [i915]
[  160.839278]  [<ffffffff81457cd4>] pci_device_remove+0x34/0xb0
[  160.839281]  [<ffffffff8151ba9c>] __device_release_driver+0x9c/0x150
[  160.839285]  [<ffffffff8151c666>] driver_detach+0xb6/0xc0
[  160.839288]  [<ffffffff8151b4b3>] bus_remove_driver+0x53/0xd0
[  160.839291]  [<ffffffff8151d127>] driver_unregister+0x27/0x50
[  160.839294]  [<ffffffff81456d55>] pci_unregister_driver+0x25/0x70
[  160.839307]  [<ffffffffa0189292>] drm_pci_exit+0x72/0x90 [drm]
[  160.839339]  [<ffffffffa03a2d06>] i915_exit+0x20/0x209 [i915]
[  160.839343]  [<ffffffff8110ba39>] SyS_delete_module+0x199/0x1f0
[  160.839347]  [<ffffffff817acae9>] entry_SYSCALL_64_fastpath+0x1c/0xac
[  160.839351] ---[ end trace d3039df93d0212ed ]---

So perhaps we have to wait until more drivers are de-midlayered?
Or do we apply and fix up only reports (not many are going to be quite as
complex as i915, as evidenced by the lack of connector unregister
callbacks elsewhere)? At any rate, we will have an ordering conflict
between drm-misc and next-queue (i.e. bisecting through the drm-misc branch
will generate warns for i915).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/msm: Remove redundant calls to drm_connector_register_all()

[Archit Taneja]

On 06/17/2016 01:55 PM, Chris Wilson wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
>

Tested-by: Archit Taneja <architt@codeaurora.org>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> ---
>   drivers/gpu/drm/msm/msm_drv.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 9c654092ef78..568fcc328f27 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -197,8 +197,6 @@ static int msm_drm_uninit(struct device *dev)
>
>   	drm_kms_helper_poll_fini(ddev);
>
> -	drm_connector_unregister_all(ddev);
> -
>   	drm_dev_unregister(ddev);
>
>   #ifdef CONFIG_DRM_FBDEV_EMULATION
> @@ -431,12 +429,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>   	if (ret)
>   		goto fail;
>
> -	ret = drm_connector_register_all(ddev);
> -	if (ret) {
> -		dev_err(dev, "failed to register connectors\n");
> -		goto fail;
> -	}
> -
>   	drm_mode_config_reset(ddev);
>
>   #ifdef CONFIG_DRM_FBDEV_EMULATION
>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

Re: ✗ Ro.CI.BAT: warning for series starting with [1/7] drm: Automatically register/unregister all connectors

[Chris Wilson]

On Fri, Jun 17, 2016 at 10:12:00AM +0100, Chris Wilson wrote:
> On Fri, Jun 17, 2016 at 08:47:04AM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/7] drm: Automatically register/unregister all connectors
> > URL   : https://patchwork.freedesktop.org/series/8814/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 8814v1 Series without cover letter
> > http://patchwork.freedesktop.org/api/1.0/series/8814/revisions/1/mbox
> > 
> > Test drv_module_reload_basic:
> >                 pass       -> DMESG-WARN (ro-bdw-i7-5557U)
> >                 pass       -> DMESG-WARN (ro-ilk1-i5-650)
> >                 pass       -> DMESG-WARN (ro-hsw-i3-4010u)
> >                 pass       -> DMESG-WARN (ro-bdw-i7-5600u)
> >                 pass       -> DMESG-WARN (ro-snb-i7-2620M)
> >                 pass       -> DMESG-WARN (ro-bdw-i5-5250u)
> >                 pass       -> DMESG-WARN (ro-hsw-i7-4770r)
> >                 pass       -> DMESG-WARN (ro-ivb-i7-3770)
> >                 pass       -> DMESG-WARN (ro-ivb2-i7-3770)
> >                 pass       -> DMESG-WARN (ro-byt-n2820)
> >                 pass       -> DMESG-WARN (fi-skl-i7-6700k)
> 
> So this is because i915 is one of the bad drivers still using
> driver->unload and not updating its init ordering:
> 
> [  160.838919] ------------[ cut here ]------------
> [  160.838933] WARNING: CPU: 2 PID: 5784 at fs/sysfs/group.c:237 sysfs_remove_group+0x86/0x90
> [  160.838938] sysfs group ffffffff81cc8380 not found for kobject 'intel_backlight'
> [  160.838941] Modules linked in: i915(-) i2c_algo_bit drm_kms_helper snd_hda_codec_hdmi snd_hda_codec_generic syscopyarea sysfillrect sysimgblt fb_sys_fops x86_pkg_temp_thermal intel_powerclamp drm snd_hda_codec snd_hwdep snd_hda_core coretemp snd_pcm crct10dif_pclmul crc32_pclmul mei_me ghash_clmulni_intel lpc_ich mei sdhci_pci sdhci mmc_core e1000e ptp pps_core [last unloaded: snd_hda_intel]
> [  160.838998] CPU: 2 PID: 5784 Comm: rmmod Tainted: G     U          4.7.0-rc2-gfxbench-RO_Patchwork_1203+ #1
> [  160.839003] Hardware name: LENOVO 4287CTO/4287CTO, BIOS 8DET47WW (1.17 ) 05/31/2011
> [  160.839006]  0000000000000000 ffff880212bd3bc0 ffffffff8140e085 ffff880212bd3c10
> [  160.839016]  0000000000000000 ffff880212bd3c00 ffffffff8107a996 000000ed81c797e0
> [  160.839024]  0000000000000000 ffffffff81cc8380 ffff88020bc6c290 ffff88020bed0000
> [  160.839032] Call Trace:
> [  160.839040]  [<ffffffff8140e085>] dump_stack+0x67/0x92
> [  160.839047]  [<ffffffff8107a996>] __warn+0xc6/0xe0
> [  160.839052]  [<ffffffff8107a9fa>] warn_slowpath_fmt+0x4a/0x50
> [  160.839060]  [<ffffffff817a92e9>] ? mutex_unlock+0x9/0x10
> [  160.839065]  [<ffffffff8125b375>] ? kernfs_find_and_get_ns+0x45/0x50
> [  160.839070]  [<ffffffff8125ec66>] sysfs_remove_group+0x86/0x90
> [  160.839076]  [<ffffffff81523e22>] dpm_sysfs_remove+0x52/0x60
> [  160.839083]  [<ffffffff81517834>] device_del+0x44/0x240
> [  160.839088]  [<ffffffff810cbd7a>] ? up_write+0x1a/0x40
> [  160.839093]  [<ffffffff81517a49>] device_unregister+0x19/0x60
> [  160.839099]  [<ffffffff81475fb9>] backlight_device_unregister+0x89/0x90
> [  160.839155]  [<ffffffffa039a25f>] intel_backlight_unregister+0x2f/0x50 [i915]
> [  160.839192]  [<ffffffffa0367ad9>] intel_modeset_cleanup+0x29/0xb0 [i915]
> [  160.839225]  [<ffffffffa03a257c>] i915_driver_unload+0x6c/0x1b0 [i915]
> [  160.839241]  [<ffffffffa018799c>] drm_dev_unregister+0x2c/0xb0 [drm]
> [  160.839253]  [<ffffffffa0187f2e>] drm_put_dev+0x1e/0x50 [drm]
> [  160.839274]  [<ffffffffa02d9300>] i915_pci_remove+0x10/0x20 [i915]
> [  160.839278]  [<ffffffff81457cd4>] pci_device_remove+0x34/0xb0
> [  160.839281]  [<ffffffff8151ba9c>] __device_release_driver+0x9c/0x150
> [  160.839285]  [<ffffffff8151c666>] driver_detach+0xb6/0xc0
> [  160.839288]  [<ffffffff8151b4b3>] bus_remove_driver+0x53/0xd0
> [  160.839291]  [<ffffffff8151d127>] driver_unregister+0x27/0x50
> [  160.839294]  [<ffffffff81456d55>] pci_unregister_driver+0x25/0x70
> [  160.839307]  [<ffffffffa0189292>] drm_pci_exit+0x72/0x90 [drm]
> [  160.839339]  [<ffffffffa03a2d06>] i915_exit+0x20/0x209 [i915]
> [  160.839343]  [<ffffffff8110ba39>] SyS_delete_module+0x199/0x1f0
> [  160.839347]  [<ffffffff817acae9>] entry_SYSCALL_64_fastpath+0x1c/0xac
> [  160.839351] ---[ end trace d3039df93d0212ed ]---
> 
> So perhaps we have to wait until more drivers are de-midlayered?
> Or do we apply and fix up only reports (not many are going to be quite as
> complex as i915, as evidenced by the lack of connector unregister
> callbacks elsewhere)? At any rate, we will have an ordering conflict
> between drm-misc and next-queue (i.e. bisecting through the drm-misc branch
> will generate warns for i915).

To shut i915 up, we need the two patches from

https://patchwork.freedesktop.org/series/8825/

The tricky part is ordering the branches to avoid WARNs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm: Automatically register/unregister all connectors

[Daniel Vetter]

On Fri, Jun 17, 2016 at 09:25:17AM +0100, Chris Wilson wrote:
> As the drm_connector is now safe for multiple calls to
> register/unregister, automatically perform a registration on all known
> connectors drm drv_register (and unregister from drm_drv_unregister).
> Drivers can still call drm_connector_register() and
> drm_connector_unregister() individually, or defer as required.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org

Applied to drm-misc. I'll let driver maintainers have a little bit more
time to ack or test the patches before I vacuum them up.

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c |  6 +++---
>  drivers/gpu/drm/drm_drv.c  | 10 +++++-----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 316dea9bea08..e7c862bd2f19 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1047,9 +1047,9 @@ EXPORT_SYMBOL(drm_connector_unregister);
>   * @dev: drm device
>   *
>   * This function registers all connectors in sysfs and other places so that
> - * userspace can start to access them. Drivers can call it after calling
> - * drm_dev_register() to complete the device registration, if they don't call
> - * drm_connector_register() on each connector individually.
> + * userspace can start to access them. drm_connector_register_all() is called
> + * automatically from drm_dev_register() to complete the device registration,
> + * if they don't call drm_connector_register() on each connector individually.
>   *
>   * When a device is unplugged and should be removed from userspace access,
>   * call drm_connector_unregister_all(), which is the inverse of this
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 40fb4352432c..2067ff089380 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -650,11 +650,7 @@ EXPORT_SYMBOL(drm_dev_unref);
>   *
>   * Register the DRM device @dev with the system, advertise device to user-space
>   * and start normal device operation. @dev must be allocated via drm_dev_alloc()
> - * previously. Right after drm_dev_register() the driver should call
> - * drm_connector_register_all() to register all connectors in sysfs. This is
> - * a separate call for backward compatibility with drivers still using
> - * the deprecated ->load() callback, where connectors are registered from within
> - * the ->load() callback.
> + * previously.
>   *
>   * Never call this twice on any device!
>   *
> @@ -691,6 +687,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  			goto err_minors;
>  	}
>  
> +	drm_connector_register_all(dev);
> +
>  	ret = 0;
>  	goto out_unlock;
>  
> @@ -721,6 +719,8 @@ void drm_dev_unregister(struct drm_device *dev)
>  
>  	drm_lastclose(dev);
>  
> +	drm_connector_unregister_all(dev);
> +
>  	if (dev->driver->unload)
>  		dev->driver->unload(dev);
>  
> -- 
> 2.8.1
> 

-- 
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: ✗ Ro.CI.BAT: warning for series starting with [1/7] drm: Automatically register/unregister all connectors

[Daniel Vetter]

On Fri, Jun 17, 2016 at 11:44:48AM +0100, Chris Wilson wrote:
> On Fri, Jun 17, 2016 at 10:12:00AM +0100, Chris Wilson wrote:
> > On Fri, Jun 17, 2016 at 08:47:04AM -0000, Patchwork wrote:
> > > == Series Details ==
> > > 
> > > Series: series starting with [1/7] drm: Automatically register/unregister all connectors
> > > URL   : https://patchwork.freedesktop.org/series/8814/
> > > State : warning
> > > 
> > > == Summary ==
> > > 
> > > Series 8814v1 Series without cover letter
> > > http://patchwork.freedesktop.org/api/1.0/series/8814/revisions/1/mbox
> > > 
> > > Test drv_module_reload_basic:
> > >                 pass       -> DMESG-WARN (ro-bdw-i7-5557U)
> > >                 pass       -> DMESG-WARN (ro-ilk1-i5-650)
> > >                 pass       -> DMESG-WARN (ro-hsw-i3-4010u)
> > >                 pass       -> DMESG-WARN (ro-bdw-i7-5600u)
> > >                 pass       -> DMESG-WARN (ro-snb-i7-2620M)
> > >                 pass       -> DMESG-WARN (ro-bdw-i5-5250u)
> > >                 pass       -> DMESG-WARN (ro-hsw-i7-4770r)
> > >                 pass       -> DMESG-WARN (ro-ivb-i7-3770)
> > >                 pass       -> DMESG-WARN (ro-ivb2-i7-3770)
> > >                 pass       -> DMESG-WARN (ro-byt-n2820)
> > >                 pass       -> DMESG-WARN (fi-skl-i7-6700k)
> > 
> > So this is because i915 is one of the bad drivers still using
> > driver->unload and not updating its init ordering:
> > 
> > [  160.838919] ------------[ cut here ]------------
> > [  160.838933] WARNING: CPU: 2 PID: 5784 at fs/sysfs/group.c:237 sysfs_remove_group+0x86/0x90
> > [  160.838938] sysfs group ffffffff81cc8380 not found for kobject 'intel_backlight'
> > [  160.838941] Modules linked in: i915(-) i2c_algo_bit drm_kms_helper snd_hda_codec_hdmi snd_hda_codec_generic syscopyarea sysfillrect sysimgblt fb_sys_fops x86_pkg_temp_thermal intel_powerclamp drm snd_hda_codec snd_hwdep snd_hda_core coretemp snd_pcm crct10dif_pclmul crc32_pclmul mei_me ghash_clmulni_intel lpc_ich mei sdhci_pci sdhci mmc_core e1000e ptp pps_core [last unloaded: snd_hda_intel]
> > [  160.838998] CPU: 2 PID: 5784 Comm: rmmod Tainted: G     U          4.7.0-rc2-gfxbench-RO_Patchwork_1203+ #1
> > [  160.839003] Hardware name: LENOVO 4287CTO/4287CTO, BIOS 8DET47WW (1.17 ) 05/31/2011
> > [  160.839006]  0000000000000000 ffff880212bd3bc0 ffffffff8140e085 ffff880212bd3c10
> > [  160.839016]  0000000000000000 ffff880212bd3c00 ffffffff8107a996 000000ed81c797e0
> > [  160.839024]  0000000000000000 ffffffff81cc8380 ffff88020bc6c290 ffff88020bed0000
> > [  160.839032] Call Trace:
> > [  160.839040]  [<ffffffff8140e085>] dump_stack+0x67/0x92
> > [  160.839047]  [<ffffffff8107a996>] __warn+0xc6/0xe0
> > [  160.839052]  [<ffffffff8107a9fa>] warn_slowpath_fmt+0x4a/0x50
> > [  160.839060]  [<ffffffff817a92e9>] ? mutex_unlock+0x9/0x10
> > [  160.839065]  [<ffffffff8125b375>] ? kernfs_find_and_get_ns+0x45/0x50
> > [  160.839070]  [<ffffffff8125ec66>] sysfs_remove_group+0x86/0x90
> > [  160.839076]  [<ffffffff81523e22>] dpm_sysfs_remove+0x52/0x60
> > [  160.839083]  [<ffffffff81517834>] device_del+0x44/0x240
> > [  160.839088]  [<ffffffff810cbd7a>] ? up_write+0x1a/0x40
> > [  160.839093]  [<ffffffff81517a49>] device_unregister+0x19/0x60
> > [  160.839099]  [<ffffffff81475fb9>] backlight_device_unregister+0x89/0x90
> > [  160.839155]  [<ffffffffa039a25f>] intel_backlight_unregister+0x2f/0x50 [i915]
> > [  160.839192]  [<ffffffffa0367ad9>] intel_modeset_cleanup+0x29/0xb0 [i915]
> > [  160.839225]  [<ffffffffa03a257c>] i915_driver_unload+0x6c/0x1b0 [i915]
> > [  160.839241]  [<ffffffffa018799c>] drm_dev_unregister+0x2c/0xb0 [drm]
> > [  160.839253]  [<ffffffffa0187f2e>] drm_put_dev+0x1e/0x50 [drm]
> > [  160.839274]  [<ffffffffa02d9300>] i915_pci_remove+0x10/0x20 [i915]
> > [  160.839278]  [<ffffffff81457cd4>] pci_device_remove+0x34/0xb0
> > [  160.839281]  [<ffffffff8151ba9c>] __device_release_driver+0x9c/0x150
> > [  160.839285]  [<ffffffff8151c666>] driver_detach+0xb6/0xc0
> > [  160.839288]  [<ffffffff8151b4b3>] bus_remove_driver+0x53/0xd0
> > [  160.839291]  [<ffffffff8151d127>] driver_unregister+0x27/0x50
> > [  160.839294]  [<ffffffff81456d55>] pci_unregister_driver+0x25/0x70
> > [  160.839307]  [<ffffffffa0189292>] drm_pci_exit+0x72/0x90 [drm]
> > [  160.839339]  [<ffffffffa03a2d06>] i915_exit+0x20/0x209 [i915]
> > [  160.839343]  [<ffffffff8110ba39>] SyS_delete_module+0x199/0x1f0
> > [  160.839347]  [<ffffffff817acae9>] entry_SYSCALL_64_fastpath+0x1c/0xac
> > [  160.839351] ---[ end trace d3039df93d0212ed ]---
> > 
> > So perhaps we have to wait until more drivers are de-midlayered?
> > Or do we apply and fix up only reports (not many are going to be quite as
> > complex as i915, as evidenced by the lack of connector unregister
> > callbacks elsewhere)? At any rate, we will have an ordering conflict
> > between drm-misc and next-queue (i.e. bisecting through the drm-misc branch
> > will generate warns for i915).
> 
> To shut i915 up, we need the two patches from
> 
> https://patchwork.freedesktop.org/series/8825/
> 
> The tricky part is ordering the branches to avoid WARNs.

I'd say apply the 2 i915 fixups to drm-misc. Shame on me for not
consulting BAT before merging ... This time around I'll wait for the
results to come in first!
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/atmel-hlcdc: Remove redundant calls to drm_connector_register_all()

[Emil Velikov]

On 17 June 2016 at 09:25, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 9ecf16c7911d..99c4af697c8a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -815,15 +815,8 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_unload;
>
> -       ret = drm_connector_register_all(ddev);
> -       if (ret)
> -               goto err_unregister;
> -
>         return 0;
>
> -err_unregister:
> -       drm_dev_unregister(ddev);
> -
>  err_unload:
>         atmel_hlcdc_dc_unload(ddev);
>
This should also kill off atmel_hlcdc_dc_connector_unplug_all() ? The
later of which has calls drm_connector_unregister_all() guarded by
mode_config.mutex :-\

-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/hisilicon: Remove redundant calls to drm_connector_register_all()

[Xinliang Liu]

Hi,

On 17 June 2016 at 16:25, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> Cc: Chen Feng <puck.chen@hisilicon.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org

This patch looks fine to me. Thank you for your patch.

Acked-by: Xinliang Liu <z.liuxinliang@hisilicon.com>

Thanks,
-xinliang

> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> index 193657259ee9..ef2c32ec1616 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
> @@ -221,19 +221,12 @@ static int kirin_drm_bind(struct device *dev)
>         if (ret)
>                 goto err_kms_cleanup;
>
> -       /* connectors should be registered after drm device register */
> -       ret = drm_connector_register_all(drm_dev);
> -       if (ret)
> -               goto err_drm_dev_unregister;
> -
>         DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
>                  driver->name, driver->major, driver->minor, driver->patchlevel,
>                  driver->date, drm_dev->primary->index);
>
>         return 0;
>
> -err_drm_dev_unregister:
> -       drm_dev_unregister(drm_dev);
>  err_kms_cleanup:
>         kirin_drm_kms_cleanup(drm_dev);
>  err_drm_dev_unref:
> @@ -246,7 +239,6 @@ static void kirin_drm_unbind(struct device *dev)
>  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
>
> -       drm_connector_unregister_all(drm_dev);
>         drm_dev_unregister(drm_dev);
>         kirin_drm_kms_cleanup(drm_dev);
>         drm_dev_unref(drm_dev);
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()

[Daniel Vetter]

On Fri, Jun 17, 2016 at 09:25:23AM +0100, Chris Wilson wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org

Ok, merged the driver patches now. Would be good to unexport/static
drm_connector_register_all, and I think there's a few drivers (like vc4)
which open-coded it.
-Daniel

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 48ec4b6e8b26..36d390093c92 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev)
>  	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
>  	struct drm_device *ddev = rcdu->ddev;
>  
> -	drm_connector_unregister_all(ddev);
>  	drm_dev_unregister(ddev);
>  
>  	if (rcdu->fbdev)
> @@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto error;
>  
> -	ret = drm_connector_register_all(ddev);
> -	if (ret < 0)
> -		goto error;
> -
>  	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
>  
>  	return 0;
> -- 
> 2.8.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()

[Laurent Pinchart]

Hi Chris,

Thank you for the patch.

On Friday 17 Jun 2016 09:25:23 Chris Wilson wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48ec4b6e8b26..36d390093c92
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev)
>  	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
>  	struct drm_device *ddev = rcdu->ddev;
> 
> -	drm_connector_unregister_all(ddev);
>  	drm_dev_unregister(ddev);
> 
>  	if (rcdu->fbdev)
> @@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto error;
> 
> -	ret = drm_connector_register_all(ddev);
> -	if (ret < 0)
> -		goto error;
> -
>  	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
> 
>  	return 0;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] drm/rcar-du: Remove redundant calls to drm_connector_register_all()

[Laurent Pinchart]

Hi Chris,

Thank you for the patch.

On Friday 17 Jun 2016 09:25:23 Chris Wilson wrote:
> Up to now, the recommendation was for drivers to call drm_dev_register()
> followed by drm_connector_register_all(). Now that
> drm_connector_register() is safe against multiple invocations, we can
> move drm_connector_register_all() to drm_dev_register() and not suffer
> from any backwards compatibility issues with drivers not following the
> more rigorous init ordering.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48ec4b6e8b26..36d390093c92
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -278,7 +278,6 @@ static int rcar_du_remove(struct platform_device *pdev)
>  	struct rcar_du_device *rcdu = platform_get_drvdata(pdev);
>  	struct drm_device *ddev = rcdu->ddev;
> 
> -	drm_connector_unregister_all(ddev);
>  	drm_dev_unregister(ddev);
> 
>  	if (rcdu->fbdev)
> @@ -360,10 +359,6 @@ static int rcar_du_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto error;
> 
> -	ret = drm_connector_register_all(ddev);
> -	if (ret < 0)
> -		goto error;
> -
>  	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
> 
>  	return 0;

-- 
Regards,

Laurent Pinchart

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx