[PATCH 00/12] Cargo cult cleanup in atomic drivers

[PATCH 00/12] Cargo cult cleanup in atomic drivers

[Laurent Pinchart]

Hello,

This patch series removes a few cargo-cult constructs from a set of atomic
drivers.

Patches 01/12 and 02/12 remove the unneeded .mode_set() and .mode_set_base()
CRTC handlers from the arc and atmel-hlcdc drivers.

Patches 03/12 to 12/12 then remove the use of drm_plane_helper_disable() from
the plane .destroy() handlers of atomic drivers, replacing them with the use
of drm_atomic_helper_shutdown() at removal time. Interleaved there are patches
04/12 and 06/12 that remove unnecessary cleanups in error paths, and patch
09/12 that adds missing cleanup.

All this has been compile-tested only.

Laurent Pinchart (12):
  drm: arc: Don't set CRTC .mode_set and .mode_set_base handlers
  drm: atmel-hlcdc: Don't set CRTC .mode_set and .mode_set_base handlers
  drm: arc: Use drm_atomic_helper_shutdown() to disable planes on
    removal
  drm: arm: hdlcd: Don't destroy plane manually in hdlcd_setup_crtc()
  drm: arm: hdlcd: Use drm_atomic_helper_shutdown() to disable planes on
    removal
  drm: arm: malidp: Don't destroy planes manually in error handlers
  drm: arm: malidp: Use drm_atomic_helper_shutdown() to disable planes
    on removal
  drm: msm: Use drm_atomic_helper_shutdown() to disable planes on
    removal
  drm: sti: Cleanup KMS objects on removal
  drm: sti: Use drm_atomic_helper_shutdown() to disable planes on
    removal
  drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on
    removal
  drm: zte: Use drm_atomic_helper_shutdown() to disable planes on
    removal

 drivers/gpu/drm/arc/arcpgu_crtc.c              | 12 ++----------
 drivers/gpu/drm/arc/arcpgu_drv.c               |  1 +
 drivers/gpu/drm/arm/hdlcd_crtc.c               | 12 ++----------
 drivers/gpu/drm/arm/hdlcd_drv.c                |  1 +
 drivers/gpu/drm/arm/malidp_crtc.c              | 10 ++--------
 drivers/gpu/drm/arm/malidp_drv.c               |  2 +-
 drivers/gpu/drm/arm/malidp_drv.h               |  1 -
 drivers/gpu/drm/arm/malidp_planes.c            | 17 +----------------
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  2 --
 drivers/gpu/drm/msm/Kconfig                    |  1 -
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c      |  1 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c      |  1 -
 drivers/gpu/drm/msm/msm_drv.c                  |  1 +
 drivers/gpu/drm/sti/sti_cursor.c               | 10 +---------
 drivers/gpu/drm/sti/sti_drv.c                  |  2 ++
 drivers/gpu/drm/sti/sti_gdp.c                  | 10 +---------
 drivers/gpu/drm/sti/sti_hqvdp.c                | 10 +---------
 drivers/gpu/drm/vc4/Kconfig                    |  1 -
 drivers/gpu/drm/vc4/vc4_drv.c                  |  3 +++
 drivers/gpu/drm/vc4/vc4_plane.c                |  8 +-------
 drivers/gpu/drm/zte/Kconfig                    |  2 +-
 drivers/gpu/drm/zte/zx_drm_drv.c               |  1 +
 drivers/gpu/drm/zte/zx_plane.c                 |  8 +-------
 23 files changed, 23 insertions(+), 94 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH 01/12] drm: arc: Don't set CRTC .mode_set and .mode_set_base handlers

[Laurent Pinchart]

The .mode_set() and .mode_set_base() operation handlers are used by
legacy non-atomic helpers only. There's no need to set them for atomic
drivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 16903dc7fe0d..26f360ef7f91 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -161,8 +161,6 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
 	.mode_valid	= arc_pgu_crtc_mode_valid,
-	.mode_set	= drm_helper_crtc_mode_set,
-	.mode_set_base	= drm_helper_crtc_mode_set_base,
 	.mode_set_nofb	= arc_pgu_crtc_mode_set_nofb,
 	.atomic_begin	= arc_pgu_crtc_atomic_begin,
 	.atomic_enable	= arc_pgu_crtc_atomic_enable,
-- 
Regards,

Laurent Pinchart

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

[PATCH 02/12] drm: atmel-hlcdc: Don't set CRTC .mode_set and .mode_set_base handlers

[Laurent Pinchart]

The .mode_set() and .mode_set_base() operation handlers are used by
legacy non-atomic helpers only. There's no need to set them for atomic
drivers.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index d73281095fac..cbe18df06e95 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -318,9 +318,7 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
 	.mode_valid = atmel_hlcdc_crtc_mode_valid,
-	.mode_set = drm_helper_crtc_mode_set,
 	.mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
-	.mode_set_base = drm_helper_crtc_mode_set_base,
 	.atomic_check = atmel_hlcdc_crtc_atomic_check,
 	.atomic_begin = atmel_hlcdc_crtc_atomic_begin,
 	.atomic_flush = atmel_hlcdc_crtc_atomic_flush,
-- 
Regards,

Laurent Pinchart

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

[PATCH 03/12] drm: arc: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time. The plane .destroy()
handler now consisting only of a call to drm_plane_cleanup(), replace it
with direct calls to that function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 10 ++--------
 drivers/gpu/drm/arc/arcpgu_drv.c  |  1 +
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 26f360ef7f91..6e7036a5f6df 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -185,16 +185,10 @@ static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
 	.atomic_update = arc_pgu_plane_atomic_update,
 };
 
-static void arc_pgu_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_helper_disable(plane);
-	drm_plane_cleanup(plane);
-}
-
 static const struct drm_plane_funcs arc_pgu_plane_funcs = {
 	.update_plane		= drm_atomic_helper_update_plane,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= arc_pgu_plane_destroy,
+	.destroy		= drm_plane_cleanup,
 	.reset			= drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
@@ -240,7 +234,7 @@ int arc_pgu_setup_crtc(struct drm_device *drm)
 	ret = drm_crtc_init_with_planes(drm, &arcpgu->crtc, primary, NULL,
 					&arc_pgu_crtc_funcs, NULL);
 	if (ret) {
-		arc_pgu_plane_destroy(primary);
+		drm_plane_cleanup(primary);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index f067de4e1e82..e85e3a349f24 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -134,6 +134,7 @@ static int arcpgu_unload(struct drm_device *drm)
 		arcpgu->fbdev = NULL;
 	}
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 	drm_mode_config_cleanup(drm);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

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

[PATCH 04/12] drm: arm: hdlcd: Don't destroy plane manually in hdlcd_setup_crtc()

[Laurent Pinchart]

The top-level error handler calls drm_mode_config_cleanup() which will
destroy all planes. There's no need to destroy them manually in lower
error handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 630721f429f7..f7e52ab622f3 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -338,10 +338,8 @@ int hdlcd_setup_crtc(struct drm_device *drm)
 
 	ret = drm_crtc_init_with_planes(drm, &hdlcd->crtc, primary, NULL,
 					&hdlcd_crtc_funcs, NULL);
-	if (ret) {
-		hdlcd_plane_destroy(primary);
+	if (ret)
 		return ret;
-	}
 
 	drm_crtc_helper_add(&hdlcd->crtc, &hdlcd_crtc_helper_funcs);
 	return 0;
-- 
Regards,

Laurent Pinchart

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

[PATCH 05/12] drm: arm: hdlcd: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time. The plane .destroy()
handler now consisting only of a call to drm_plane_cleanup(), replace it
with direct calls to that function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 8 +-------
 drivers/gpu/drm/arm/hdlcd_drv.c  | 1 +
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index f7e52ab622f3..6a57f96c696f 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -284,16 +284,10 @@ static const struct drm_plane_helper_funcs hdlcd_plane_helper_funcs = {
 	.atomic_update = hdlcd_plane_atomic_update,
 };
 
-static void hdlcd_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_helper_disable(plane);
-	drm_plane_cleanup(plane);
-}
-
 static const struct drm_plane_funcs hdlcd_plane_funcs = {
 	.update_plane		= drm_atomic_helper_update_plane,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.destroy		= hdlcd_plane_destroy,
+	.destroy		= drm_plane_cleanup,
 	.reset			= drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index feaa8bc3d7b7..e6a6bb119ea8 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -379,6 +379,7 @@ static void hdlcd_drm_unbind(struct device *dev)
 	pm_runtime_put_sync(drm->dev);
 	pm_runtime_disable(drm->dev);
 	of_reserved_mem_device_release(drm->dev);
+	drm_atomic_helper_shutdown(drm);
 	drm_mode_config_cleanup(drm);
 	drm_dev_put(drm);
 	drm->dev_private = NULL;
-- 
Regards,

Laurent Pinchart

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

[PATCH 06/12] drm: arm: malidp: Don't destroy planes manually in error handlers

[Laurent Pinchart]

The top-level error handler calls drm_mode_config_cleanup() which will
destroy all planes. There's no need to destroy them manually in lower
error handlers.

As plane cleanup is now handled entirely by drm_mode_config_cleanup(),
we must ensure that the plane .destroy() handler frees allocated memory
for the plane object that was freed by malidp_de_planes_destroy(). Do so
by replacing the call to devm_kfree() in the .destroy() handler by
kfree(). devm_kfree() is currently a no-op as the plane memory is
allocated with kzalloc(), not devm_kzalloc().

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arm/malidp_crtc.c   | 10 ++--------
 drivers/gpu/drm/arm/malidp_drv.c    |  1 -
 drivers/gpu/drm/arm/malidp_drv.h    |  1 -
 drivers/gpu/drm/arm/malidp_planes.c | 13 +------------
 4 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 904fff80917b..f0fb2aa153a1 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -525,14 +525,13 @@ int malidp_crtc_init(struct drm_device *drm)
 
 	if (!primary) {
 		DRM_ERROR("no primary plane found\n");
-		ret = -EINVAL;
-		goto crtc_cleanup_planes;
+		return -EINVAL;
 	}
 
 	ret = drm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL,
 					&malidp_crtc_funcs, NULL);
 	if (ret)
-		goto crtc_cleanup_planes;
+		return ret;
 
 	drm_crtc_helper_add(&malidp->crtc, &malidp_crtc_helper_funcs);
 	drm_mode_crtc_set_gamma_size(&malidp->crtc, MALIDP_GAMMA_LUT_SIZE);
@@ -542,9 +541,4 @@ int malidp_crtc_init(struct drm_device *drm)
 	malidp_se_set_enh_coeffs(malidp->dev);
 
 	return 0;
-
-crtc_cleanup_planes:
-	malidp_de_planes_destroy(drm);
-
-	return ret;
 }
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 3d82712d8002..0711279e836f 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -276,7 +276,6 @@ static int malidp_init(struct drm_device *drm)
 
 static void malidp_fini(struct drm_device *drm)
 {
-	malidp_de_planes_destroy(drm);
 	drm_mode_config_cleanup(drm);
 }
 
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index e0d12c9fc6b8..7f6ef0a040f7 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -59,7 +59,6 @@ struct malidp_crtc_state {
 #define to_malidp_crtc_state(x) container_of(x, struct malidp_crtc_state, base)
 
 int malidp_de_planes_init(struct drm_device *drm);
-void malidp_de_planes_destroy(struct drm_device *drm);
 int malidp_crtc_init(struct drm_device *drm);
 
 /* often used combination of rotational bits */
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 33c5ef96ced0..d883d28d5d9d 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -61,7 +61,7 @@ static void malidp_de_plane_destroy(struct drm_plane *plane)
 
 	drm_plane_helper_disable(plane);
 	drm_plane_cleanup(plane);
-	devm_kfree(plane->dev->dev, mp);
+	kfree(mp);
 }
 
 /*
@@ -427,18 +427,7 @@ int malidp_de_planes_init(struct drm_device *drm)
 	return 0;
 
 cleanup:
-	malidp_de_planes_destroy(drm);
 	kfree(formats);
 
 	return ret;
 }
-
-void malidp_de_planes_destroy(struct drm_device *drm)
-{
-	struct drm_plane *p, *pt;
-
-	list_for_each_entry_safe(p, pt, &drm->mode_config.plane_list, head) {
-		drm_plane_cleanup(p);
-		kfree(p);
-	}
-}
-- 
Regards,

Laurent Pinchart

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

[PATCH 07/12] drm: arm: malidp: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arm/malidp_drv.c    | 1 +
 drivers/gpu/drm/arm/malidp_planes.c | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 0711279e836f..5d037ea576c7 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -276,6 +276,7 @@ static int malidp_init(struct drm_device *drm)
 
 static void malidp_fini(struct drm_device *drm)
 {
+	drm_atomic_helper_shutdown(drm);
 	drm_mode_config_cleanup(drm);
 }
 
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index d883d28d5d9d..2bc1264ec73a 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -56,10 +56,6 @@ static void malidp_de_plane_destroy(struct drm_plane *plane)
 {
 	struct malidp_plane *mp = to_malidp_plane(plane);
 
-	if (mp->base.fb)
-		drm_framebuffer_put(mp->base.fb);
-
-	drm_plane_helper_disable(plane);
 	drm_plane_cleanup(plane);
 	kfree(mp);
 }
-- 
Regards,

Laurent Pinchart

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

[PATCH 08/12] drm: msm: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 1 -
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 -
 drivers/gpu/drm/msm/msm_drv.c             | 1 +
 3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
index 7a1ad3af08e3..db356d22ef15 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
@@ -68,7 +68,6 @@ static void mdp4_plane_destroy(struct drm_plane *plane)
 {
 	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
 
-	drm_plane_helper_disable(plane);
 	drm_plane_cleanup(plane);
 
 	kfree(mdp4_plane);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 29678876fc09..986684e33a03 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -46,7 +46,6 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 
-	drm_plane_helper_disable(plane);
 	drm_plane_cleanup(plane);
 
 	kfree(mdp5_plane);
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d90ef1d78a1b..4ba48e5acbe9 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -233,6 +233,7 @@ static int msm_drm_uninit(struct device *dev)
 	if (fbdev && priv->fbdev)
 		msm_fbdev_free(ddev);
 #endif
+	drm_atomic_helper_shutdown(ddev);
 	drm_mode_config_cleanup(ddev);
 
 	pm_runtime_get_sync(dev);
-- 
Regards,

Laurent Pinchart

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

[PATCH 09/12] drm: sti: Cleanup KMS objects on removal

[Laurent Pinchart]

Call the drm_mode_config_cleanup() function on removal to ensure that
all KMS objects are properly cleaned up and destroyed.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/sti/sti_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 55b6967d27e1..9c3a000fab5e 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -276,6 +276,7 @@ static void sti_unbind(struct device *dev)
 	struct drm_device *ddev = dev_get_drvdata(dev);
 
 	drm_dev_unregister(ddev);
+	drm_mode_config_cleanup(ddev);
 	sti_cleanup(ddev);
 	drm_dev_unref(ddev);
 }
-- 
Regards,

Laurent Pinchart

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

[PATCH 10/12] drm: sti: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handlers currently call drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time. The plane .destroy()
handlers now consisting only of a call to drm_plane_cleanup(), replace
them with direct calls to that function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/sti/sti_cursor.c | 10 +---------
 drivers/gpu/drm/sti/sti_drv.c    |  1 +
 drivers/gpu/drm/sti/sti_gdp.c    | 10 +---------
 drivers/gpu/drm/sti/sti_hqvdp.c  | 10 +---------
 4 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index df0a282b9615..e1ba253055c7 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -328,14 +328,6 @@ static const struct drm_plane_helper_funcs sti_cursor_helpers_funcs = {
 	.atomic_disable = sti_cursor_atomic_disable,
 };
 
-static void sti_cursor_destroy(struct drm_plane *drm_plane)
-{
-	DRM_DEBUG_DRIVER("\n");
-
-	drm_plane_helper_disable(drm_plane);
-	drm_plane_cleanup(drm_plane);
-}
-
 static int sti_cursor_late_register(struct drm_plane *drm_plane)
 {
 	struct sti_plane *plane = to_sti_plane(drm_plane);
@@ -347,7 +339,7 @@ static int sti_cursor_late_register(struct drm_plane *drm_plane)
 static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = sti_cursor_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 9c3a000fab5e..711ea09f5754 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -276,6 +276,7 @@ static void sti_unbind(struct device *dev)
 	struct drm_device *ddev = dev_get_drvdata(dev);
 
 	drm_dev_unregister(ddev);
+	drm_atomic_helper_shutdown(ddev);
 	drm_mode_config_cleanup(ddev);
 	sti_cleanup(ddev);
 	drm_dev_unref(ddev);
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 9b2c47051b51..73b5c8985998 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -875,14 +875,6 @@ static const struct drm_plane_helper_funcs sti_gdp_helpers_funcs = {
 	.atomic_disable = sti_gdp_atomic_disable,
 };
 
-static void sti_gdp_destroy(struct drm_plane *drm_plane)
-{
-	DRM_DEBUG_DRIVER("\n");
-
-	drm_plane_helper_disable(drm_plane);
-	drm_plane_cleanup(drm_plane);
-}
-
 static int sti_gdp_late_register(struct drm_plane *drm_plane)
 {
 	struct sti_plane *plane = to_sti_plane(drm_plane);
@@ -894,7 +886,7 @@ static int sti_gdp_late_register(struct drm_plane *drm_plane)
 static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = sti_gdp_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 106be8c4e58b..065a5b08a702 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1256,14 +1256,6 @@ static const struct drm_plane_helper_funcs sti_hqvdp_helpers_funcs = {
 	.atomic_disable = sti_hqvdp_atomic_disable,
 };
 
-static void sti_hqvdp_destroy(struct drm_plane *drm_plane)
-{
-	DRM_DEBUG_DRIVER("\n");
-
-	drm_plane_helper_disable(drm_plane);
-	drm_plane_cleanup(drm_plane);
-}
-
 static int sti_hqvdp_late_register(struct drm_plane *drm_plane)
 {
 	struct sti_plane *plane = to_sti_plane(drm_plane);
@@ -1275,7 +1267,7 @@ static int sti_hqvdp_late_register(struct drm_plane *drm_plane)
 static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = sti_hqvdp_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = sti_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
-- 
Regards,

Laurent Pinchart

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

[PATCH 11/12] drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time. The plane .destroy()
handler now consisting only of a call to drm_plane_cleanup(), replace it
with direct calls to that function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/msm/Kconfig     | 1 -
 drivers/gpu/drm/vc4/Kconfig     | 1 -
 drivers/gpu/drm/vc4/vc4_drv.c   | 3 +++
 drivers/gpu/drm/vc4/vc4_plane.c | 8 +-------
 drivers/gpu/drm/zte/Kconfig     | 2 +-
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 99d39b2aefa6..d6b0685ca129 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -2,7 +2,6 @@
 config DRM_MSM
 	tristate "MSM DRM"
 	depends on DRM
-	depends on ARCH_QCOM || (ARM && COMPILE_TEST)
 	depends on OF && COMMON_CLK
 	depends on MMU
 	select QCOM_MDT_LOADER if ARCH_QCOM
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index fdae18aeab4f..412d7a06f0db 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -1,6 +1,5 @@
 config DRM_VC4
 	tristate "Broadcom VC4 Graphics"
-	depends on ARCH_BCM || ARCH_BCM2835 || COMPILE_TEST
 	depends on DRM
 	depends on SND && SND_SOC
 	depends on COMMON_CLK
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index ceb385fd69c5..ed2c1c233c39 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -31,6 +31,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fb_helper.h>
 
@@ -299,6 +300,8 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_fb_cma_fbdev_fini(drm);
 
+	drm_atomic_helper_shutdown(drm);
+
 	drm_mode_config_cleanup(drm);
 
 	drm_dev_unref(drm);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 515f97997624..9fcc55078ff7 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -825,12 +825,6 @@ static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
 	.cleanup_fb = vc4_cleanup_fb,
 };
 
-static void vc4_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_helper_disable(plane);
-	drm_plane_cleanup(plane);
-}
-
 /* Implements immediate (non-vblank-synced) updates of the cursor
  * position, or falls back to the atomic helper otherwise.
  */
@@ -910,7 +904,7 @@ vc4_update_plane(struct drm_plane *plane,
 static const struct drm_plane_funcs vc4_plane_funcs = {
 	.update_plane = vc4_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = vc4_plane_destroy,
+	.destroy = drm_plane_cleanup,
 	.set_property = NULL,
 	.reset = vc4_plane_reset,
 	.atomic_duplicate_state = vc4_plane_duplicate_state,
diff --git a/drivers/gpu/drm/zte/Kconfig b/drivers/gpu/drm/zte/Kconfig
index 5b36421ef3e5..76d63bcc5c3a 100644
--- a/drivers/gpu/drm/zte/Kconfig
+++ b/drivers/gpu/drm/zte/Kconfig
@@ -1,6 +1,6 @@
 config DRM_ZTE
 	tristate "DRM Support for ZTE SoCs"
-	depends on DRM && ARCH_ZX
+	depends on DRM
 	select DRM_KMS_CMA_HELPER
 	select DRM_KMS_FB_HELPER
 	select DRM_KMS_HELPER
-- 
Regards,

Laurent Pinchart

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

[PATCH 12/12] drm: zte: Use drm_atomic_helper_shutdown() to disable planes on removal

[Laurent Pinchart]

The plane cleanup handler currently calls drm_plane_helper_disable(),
which is a legacy helper function. Replace it with a call to
drm_atomic_helper_shutdown() at removal time. The plane .destroy()
handler now consisting only of a call to drm_plane_cleanup(), replace it
with direct calls to that function.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/zte/zx_drm_drv.c | 1 +
 drivers/gpu/drm/zte/zx_plane.c   | 8 +-------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index 6f4205e80378..380c41d7dd9a 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -133,6 +133,7 @@ static void zx_drm_unbind(struct device *dev)
 	drm_dev_unregister(drm);
 	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
+	drm_atomic_helper_shutdown(drm);
 	drm_mode_config_cleanup(drm);
 	component_unbind_all(dev, drm);
 	dev_set_drvdata(dev, NULL);
diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 68fd2e2dc78a..9d2aed9d7e2e 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -457,16 +457,10 @@ static const struct drm_plane_helper_funcs zx_gl_plane_helper_funcs = {
 	.atomic_disable = zx_plane_atomic_disable,
 };
 
-static void zx_plane_destroy(struct drm_plane *plane)
-{
-	drm_plane_helper_disable(plane);
-	drm_plane_cleanup(plane);
-}
-
 static const struct drm_plane_funcs zx_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.destroy = zx_plane_destroy,
+	.destroy = drm_plane_cleanup,
 	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 11/12] drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on removal

[Eric Anholt]

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

Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> writes:

> The plane cleanup handler currently calls drm_plane_helper_disable(),
> which is a legacy helper function. Replace it with a call to
> drm_atomic_helper_shutdown() at removal time. The plane .destroy()
> handler now consisting only of a call to drm_plane_cleanup(), replace it
> with direct calls to that function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Looks like debug changes to Kconfigs slipped in.  Other than that, the
actual change sounds good.

[-- 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 02/12] drm: atmel-hlcdc: Don't set CRTC .mode_set and .mode_set_base handlers

[Boris Brezillon]

On Wed, 17 Jan 2018 23:55:25 +0200
Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote:

> The .mode_set() and .mode_set_base() operation handlers are used by
> legacy non-atomic helpers only. There's no need to set them for atomic
> drivers.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index d73281095fac..cbe18df06e95 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -318,9 +318,7 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
>  
>  static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = {
>  	.mode_valid = atmel_hlcdc_crtc_mode_valid,
> -	.mode_set = drm_helper_crtc_mode_set,
>  	.mode_set_nofb = atmel_hlcdc_crtc_mode_set_nofb,
> -	.mode_set_base = drm_helper_crtc_mode_set_base,
>  	.atomic_check = atmel_hlcdc_crtc_atomic_check,
>  	.atomic_begin = atmel_hlcdc_crtc_atomic_begin,
>  	.atomic_flush = atmel_hlcdc_crtc_atomic_flush,

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

Re: [PATCH 00/12] Cargo cult cleanup in atomic drivers

[Liviu Dudau]

On Wed, Jan 17, 2018 at 11:55:23PM +0200, Laurent Pinchart wrote:
> Hello,

Hi Laurent,

> 
> This patch series removes a few cargo-cult constructs from a set of atomic
> drivers.
> 
> Patches 01/12 and 02/12 remove the unneeded .mode_set() and .mode_set_base()
> CRTC handlers from the arc and atmel-hlcdc drivers.
> 
> Patches 03/12 to 12/12 then remove the use of drm_plane_helper_disable() from
> the plane .destroy() handlers of atomic drivers, replacing them with the use
> of drm_atomic_helper_shutdown() at removal time. Interleaved there are patches
> 04/12 and 06/12 that remove unnecessary cleanups in error paths, and patch
> 09/12 that adds missing cleanup.

Thanks for the cleanup!

> 
> All this has been compile-tested only.

And I was trying to test the patches today but I've made the foolish
decision to update the firmware on my Juno board first :( I hope to
finish the testing tomorrow.

> 
> Laurent Pinchart (12):
>   drm: arc: Don't set CRTC .mode_set and .mode_set_base handlers
>   drm: atmel-hlcdc: Don't set CRTC .mode_set and .mode_set_base handlers
>   drm: arc: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: arm: hdlcd: Don't destroy plane manually in hdlcd_setup_crtc()
>   drm: arm: hdlcd: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: arm: malidp: Don't destroy planes manually in error handlers
>   drm: arm: malidp: Use drm_atomic_helper_shutdown() to disable planes
>     on removal
>   drm: msm: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: sti: Cleanup KMS objects on removal
>   drm: sti: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: zte: Use drm_atomic_helper_shutdown() to disable planes on
>     removal

Anyway, for the hdlcd and malidp patches in the series:

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

If you need me to get the driver patches into my trees let me know.

Best regards,
Liviu

> 
>  drivers/gpu/drm/arc/arcpgu_crtc.c              | 12 ++----------
>  drivers/gpu/drm/arc/arcpgu_drv.c               |  1 +
>  drivers/gpu/drm/arm/hdlcd_crtc.c               | 12 ++----------
>  drivers/gpu/drm/arm/hdlcd_drv.c                |  1 +
>  drivers/gpu/drm/arm/malidp_crtc.c              | 10 ++--------
>  drivers/gpu/drm/arm/malidp_drv.c               |  2 +-
>  drivers/gpu/drm/arm/malidp_drv.h               |  1 -
>  drivers/gpu/drm/arm/malidp_planes.c            | 17 +----------------
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  2 --
>  drivers/gpu/drm/msm/Kconfig                    |  1 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c      |  1 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c      |  1 -
>  drivers/gpu/drm/msm/msm_drv.c                  |  1 +
>  drivers/gpu/drm/sti/sti_cursor.c               | 10 +---------
>  drivers/gpu/drm/sti/sti_drv.c                  |  2 ++
>  drivers/gpu/drm/sti/sti_gdp.c                  | 10 +---------
>  drivers/gpu/drm/sti/sti_hqvdp.c                | 10 +---------
>  drivers/gpu/drm/vc4/Kconfig                    |  1 -
>  drivers/gpu/drm/vc4/vc4_drv.c                  |  3 +++
>  drivers/gpu/drm/vc4/vc4_plane.c                |  8 +-------
>  drivers/gpu/drm/zte/Kconfig                    |  2 +-
>  drivers/gpu/drm/zte/zx_drm_drv.c               |  1 +
>  drivers/gpu/drm/zte/zx_plane.c                 |  8 +-------
>  23 files changed, 23 insertions(+), 94 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/12] Cargo cult cleanup in atomic drivers

[Alexey Brodkin]

Hi Laurent,

On Wed, 2018-01-17 at 23:55 +0200, Laurent Pinchart wrote:
> Hello,
> 
> This patch series removes a few cargo-cult constructs from a set of atomic
> drivers.
> 
> Patches 01/12 and 02/12 remove the unneeded .mode_set() and .mode_set_base()
> CRTC handlers from the arc and atmel-hlcdc drivers.
> 
> Patches 03/12 to 12/12 then remove the use of drm_plane_helper_disable() from
> the plane .destroy() handlers of atomic drivers, replacing them with the use
> of drm_atomic_helper_shutdown() at removal time. Interleaved there are patches
> 04/12 and 06/12 that remove unnecessary cleanups in error paths, and patch
> 09/12 that adds missing cleanup.
> 
> All this has been compile-tested only.

I'd like to test it on ARC boards and so wondering if there's a git tree
with these patches so I'm not missing any prerequisites or maybe you may
specify what should be used as a base for the series?

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

Re: [PATCH 08/12] drm: msm: Use drm_atomic_helper_shutdown() to disable planes on removal

[Archit Taneja]

On 01/18/2018 03:25 AM, Laurent Pinchart wrote:
> The plane cleanup handler currently calls drm_plane_helper_disable(),
> which is a legacy helper function. Replace it with a call to
> drm_atomic_helper_shutdown() at removal time.

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

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 1 -
>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 -
>   drivers/gpu/drm/msm/msm_drv.c             | 1 +
>   3 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> index 7a1ad3af08e3..db356d22ef15 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> @@ -68,7 +68,6 @@ static void mdp4_plane_destroy(struct drm_plane *plane)
>   {
>   	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
>   
> -	drm_plane_helper_disable(plane);
>   	drm_plane_cleanup(plane);
>   
>   	kfree(mdp4_plane);
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 29678876fc09..986684e33a03 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -46,7 +46,6 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
>   {
>   	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
>   
> -	drm_plane_helper_disable(plane);
>   	drm_plane_cleanup(plane);
>   
>   	kfree(mdp5_plane);
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index d90ef1d78a1b..4ba48e5acbe9 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -233,6 +233,7 @@ static int msm_drm_uninit(struct device *dev)
>   	if (fbdev && priv->fbdev)
>   		msm_fbdev_free(ddev);
>   #endif
> +	drm_atomic_helper_shutdown(ddev);
>   	drm_mode_config_cleanup(ddev);
>   
>   	pm_runtime_get_sync(dev);
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/12] Cargo cult cleanup in atomic drivers

[Benjamin Gaignard]

2018-01-17 22:55 GMT+01:00 Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com>:
> Hello,
>
> This patch series removes a few cargo-cult constructs from a set of atomic
> drivers.
>
> Patches 01/12 and 02/12 remove the unneeded .mode_set() and .mode_set_base()
> CRTC handlers from the arc and atmel-hlcdc drivers.
>
> Patches 03/12 to 12/12 then remove the use of drm_plane_helper_disable() from
> the plane .destroy() handlers of atomic drivers, replacing them with the use
> of drm_atomic_helper_shutdown() at removal time. Interleaved there are patches
> 04/12 and 06/12 that remove unnecessary cleanups in error paths, and patch
> 09/12 that adds missing cleanup.
>
> All this has been compile-tested only.

Sorry but sti patches make the platform crash when removing display module.

Benjamin

>
> Laurent Pinchart (12):
>   drm: arc: Don't set CRTC .mode_set and .mode_set_base handlers
>   drm: atmel-hlcdc: Don't set CRTC .mode_set and .mode_set_base handlers
>   drm: arc: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: arm: hdlcd: Don't destroy plane manually in hdlcd_setup_crtc()
>   drm: arm: hdlcd: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: arm: malidp: Don't destroy planes manually in error handlers
>   drm: arm: malidp: Use drm_atomic_helper_shutdown() to disable planes
>     on removal
>   drm: msm: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: sti: Cleanup KMS objects on removal
>   drm: sti: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>   drm: zte: Use drm_atomic_helper_shutdown() to disable planes on
>     removal
>
>  drivers/gpu/drm/arc/arcpgu_crtc.c              | 12 ++----------
>  drivers/gpu/drm/arc/arcpgu_drv.c               |  1 +
>  drivers/gpu/drm/arm/hdlcd_crtc.c               | 12 ++----------
>  drivers/gpu/drm/arm/hdlcd_drv.c                |  1 +
>  drivers/gpu/drm/arm/malidp_crtc.c              | 10 ++--------
>  drivers/gpu/drm/arm/malidp_drv.c               |  2 +-
>  drivers/gpu/drm/arm/malidp_drv.h               |  1 -
>  drivers/gpu/drm/arm/malidp_planes.c            | 17 +----------------
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c |  2 --
>  drivers/gpu/drm/msm/Kconfig                    |  1 -
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c      |  1 -
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c      |  1 -
>  drivers/gpu/drm/msm/msm_drv.c                  |  1 +
>  drivers/gpu/drm/sti/sti_cursor.c               | 10 +---------
>  drivers/gpu/drm/sti/sti_drv.c                  |  2 ++
>  drivers/gpu/drm/sti/sti_gdp.c                  | 10 +---------
>  drivers/gpu/drm/sti/sti_hqvdp.c                | 10 +---------
>  drivers/gpu/drm/vc4/Kconfig                    |  1 -
>  drivers/gpu/drm/vc4/vc4_drv.c                  |  3 +++
>  drivers/gpu/drm/vc4/vc4_plane.c                |  8 +-------
>  drivers/gpu/drm/zte/Kconfig                    |  2 +-
>  drivers/gpu/drm/zte/zx_drm_drv.c               |  1 +
>  drivers/gpu/drm/zte/zx_plane.c                 |  8 +-------
>  23 files changed, 23 insertions(+), 94 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/12] drm: arm: malidp: Use drm_atomic_helper_shutdown() to disable planes on removal

[Liviu Dudau]

Hi Laurent,

On Wed, Jan 17, 2018 at 11:55:30PM +0200, Laurent Pinchart wrote:
> The plane cleanup handler currently calls drm_plane_helper_disable(),
> which is a legacy helper function. Replace it with a call to
> drm_atomic_helper_shutdown() at removal time.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c    | 1 +
>  drivers/gpu/drm/arm/malidp_planes.c | 4 ----
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 0711279e836f..5d037ea576c7 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -276,6 +276,7 @@ static int malidp_init(struct drm_device *drm)
>  
>  static void malidp_fini(struct drm_device *drm)
>  {
> +	drm_atomic_helper_shutdown(drm);
>  	drm_mode_config_cleanup(drm);
>  }
>  
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
> index d883d28d5d9d..2bc1264ec73a 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -56,10 +56,6 @@ static void malidp_de_plane_destroy(struct drm_plane *plane)
>  {
>  	struct malidp_plane *mp = to_malidp_plane(plane);
>  
> -	if (mp->base.fb)
> -		drm_framebuffer_put(mp->base.fb);
> -
> -	drm_plane_helper_disable(plane);
>  	drm_plane_cleanup(plane);
>  	kfree(mp);
>  }

With this change in place I'm getting

[drm:drm_atomic_helper_shutdown [drm_kms_helper]] *ERROR* Disabling all crtc's during unload failed with -22

when trying to remove the mali-dp module.

Echoing 0x3f into /sys/module/drm/parameters/debug I get this:

[  332.072080] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4386, diff=1, hw=0 hw_last=0
[  332.091326] [drm:drm_sysfs_connector_remove [drm]] removing "HDMI-A-1" from sysfs
[  332.101255] Console: switching to colour dummy device 80x25
[  332.105506] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4387, diff=1, hw=0 hw_last=0
[  332.117772] [drm:drm_atomic_state_init [drm]] Allocated atomic state 0000000035c85576
[  332.125710] [drm:drm_mode_object_get [drm]] OBJ ID: 62 (3)
[  332.131302] [drm:drm_atomic_get_plane_state [drm]] Added [PLANE:28:plane-0] 000000002713c919 state to 0000000035c85576
[  332.139069] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4388, diff=1, hw=0 hw_last=0
[  332.152827] [drm:drm_mode_object_get [drm]] OBJ ID: 45 (1)
[  332.158558] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:35:crtc-0] 00000000da10cba0 state to 0000000035c85576
[  332.169169] [drm:drm_atomic_set_fb_for_plane [drm]] Set [NOFB] for plane state 000000002713c919
[  332.178051] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4389, diff=1, hw=0 hw_last=0
[  332.188836] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 62 (4)
[  332.195145] [drm:drm_atomic_set_crtc_for_plane [drm]] Link plane state 000000002713c919 to [NOCRTC]
[  332.204373] [drm:drm_atomic_check_only [drm]] checking 0000000035c85576
[  332.211182] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4390, diff=1, hw=0 hw_last=0
[  332.221968] [drm:drm_atomic_commit [drm]] committing 0000000035c85576
[  332.228617] [drm:drm_calc_timestamping_constants [drm]] crtc 35: hwmode: htotal 2200, vtotal 562, vdisplay 540
[  332.238787] [drm:drm_calc_timestamping_constants [drm]] crtc 35: clock 74250 kHz framedur 8325925 linedur 29629
[  332.249039] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4391, diff=1, hw=0 hw_last=0
[  332.272706] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4392, diff=1, hw=0 hw_last=0
[  332.306160] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4393, diff=1, hw=0 hw_last=0
[  332.317030] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 62 (3)
[  332.323327] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 0000000035c85576
[  332.332041] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 45 (2)
[  332.338331] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 62 (2)
[  332.339680] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4394, diff=1, hw=0 hw_last=0
[  332.355181] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 0000000035c85576
[  332.363204] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 62 (1)
[  332.373126] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4395, diff=1, hw=0 hw_last=0
[  332.395308] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 37 (4)
[  332.401599] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 37 (3)
[  332.406566] [drm:drm_update_vblank_count [drm]] updating vblank count on crtc 0: current=4396, diff=1, hw=0 hw_last=0
[  332.418489] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 0 (2)
[  332.432903] [drm:drm_atomic_state_init [drm]] Allocated atomic state 00000000af12a943
[  332.440934] [drm:drm_mode_object_get [drm]] OBJ ID: 45 (1)
[  332.446635] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:35:crtc-0] 000000006a7e1466 state to 00000000af12a943
[  332.457242] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 45 (2)
[  332.463529] [drm:drm_atomic_set_mode_prop_for_crtc [drm]] Set [NOMODE] for CRTC state 000000006a7e1466
[  332.473014] [drm:drm_atomic_add_affected_connectors [drm]] Adding all current connectors for [CRTC:35:crtc-0] to 00000000af12a943
[  332.484819] [drm:drm_atomic_check_only [drm]] checking 00000000af12a943
[  332.491537] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:35:crtc-0] mode changed
[  332.500391] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:35:crtc-0] enable changed
[  332.509419] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:35:crtc-0] active changed
[  332.518442] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:35:crtc-0] enabled/connectors mismatch
[  332.528700] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 00000000af12a943
[  332.537399] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 00000000af12a943
[  332.551046] [drm:drm_atomic_helper_shutdown [drm_kms_helper]] *ERROR* Disabling all crtc's during unload failed with -22
[  332.562153] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 45 (1)
[  332.568444] [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 63 (1)

It looks like drm_atomic_helper_check_modeset is being tripped by the final drm_atomic_commit()
that happens in the drm_atomic_helper_disable_all() function as called by drm_atomic_helper_shutdown().

Not sure why the new_crtc_state->connector_mask is not zero when drm_atomic_helper_check_modeset()
gets called. Any suggestions?

Best regards,
Liviu


> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/12] drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on removal

[kbuild test robot]

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

Hi Laurent,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.15-rc8 next-20180119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/Cargo-cult-cleanup-in-atomic-drivers/20180120-183018
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-n0-201802 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/msm/msm_gpu.h:24:0,
                    from drivers/gpu/drm/msm/adreno/adreno_gpu.h:25,
                    from drivers/gpu/drm/msm/adreno/adreno_device.c:21:
>> drivers/gpu/drm/msm/msm_drv.h:35:10: fatal error: asm/sizes.h: No such file or directory
    #include <asm/sizes.h>
             ^~~~~~~~~~~~~
   compilation terminated.

vim +35 drivers/gpu/drm/msm/msm_drv.h

c8afe684 Rob Clark     2013-06-26  20  
c8afe684 Rob Clark     2013-06-26  21  #include <linux/kernel.h>
c8afe684 Rob Clark     2013-06-26  22  #include <linux/clk.h>
c8afe684 Rob Clark     2013-06-26  23  #include <linux/cpufreq.h>
c8afe684 Rob Clark     2013-06-26  24  #include <linux/module.h>
060530f1 Rob Clark     2014-03-03  25  #include <linux/component.h>
c8afe684 Rob Clark     2013-06-26  26  #include <linux/platform_device.h>
c8afe684 Rob Clark     2013-06-26  27  #include <linux/pm.h>
c8afe684 Rob Clark     2013-06-26  28  #include <linux/pm_runtime.h>
c8afe684 Rob Clark     2013-06-26  29  #include <linux/slab.h>
c8afe684 Rob Clark     2013-06-26  30  #include <linux/list.h>
c8afe684 Rob Clark     2013-06-26  31  #include <linux/iommu.h>
c8afe684 Rob Clark     2013-06-26  32  #include <linux/types.h>
3d6df062 Archit Taneja 2015-06-09  33  #include <linux/of_graph.h>
e9fbdaf2 Archit Taneja 2015-11-18  34  #include <linux/of_device.h>
c8afe684 Rob Clark     2013-06-26 @35  #include <asm/sizes.h>
c8afe684 Rob Clark     2013-06-26  36  

:::::: The code at line 35 was first introduced by commit
:::::: c8afe684c95cd17cf4f273d81af369a0fdfa5a74 drm/msm: basic KMS driver for snapdragon

:::::: TO: Rob Clark <robdclark@gmail.com>
:::::: CC: Rob Clark <robdclark@gmail.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33577 bytes --]

[-- Attachment #3: 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 11/12] drm: vc4: Use drm_atomic_helper_shutdown() to disable planes on removal

[kbuild test robot]

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

Hi Laurent,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.15-rc8 next-20180119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laurent-Pinchart/Cargo-cult-cleanup-in-atomic-drivers/20180120-183018
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-x002-201802 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

warning: (VIDEO_QCOM_VENUS && DRM_MSM && QCOM_ADSP_PIL && QCOM_Q6V5_PIL && QCOM_WCNSS_PIL && QCOM_MDT_LOADER && QCOM_PM) selects QCOM_SCM which has unmet direct dependencies (ARM || ARM64)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34194 bytes --]

[-- Attachment #3: 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 12/12] drm: zte: Use drm_atomic_helper_shutdown() to disable planes on removal

[Shawn Guo]

On Wed, Jan 17, 2018 at 11:55:35PM +0200, Laurent Pinchart wrote:
> The plane cleanup handler currently calls drm_plane_helper_disable(),
> which is a legacy helper function. Replace it with a call to
> drm_atomic_helper_shutdown() at removal time. The plane .destroy()
> handler now consisting only of a call to drm_plane_cleanup(), replace it
> with direct calls to that function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Acked-by: Shawn Guo <shawnguo@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel