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(-)
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,
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,
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,
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;
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;
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;
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); - } -}
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); }
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
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);
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);
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); }
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,
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
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.
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... 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
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... 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
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,
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
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
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
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
dri-devel@lists.freedesktop.org
-
Alexey Brodkin -
Archit Taneja -
Benjamin Gaignard -
Boris Brezillon -
Eric Anholt -
kbuild test robot -
Laurent Pinchart -
Liviu Dudau -
Shawn Guo