diff mbox series

[15/21] drm: Remove transitional helpers

Message ID 20181004202446.22905-16-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series atomic helper cleanup, take 2 | expand

Commit Message

Daniel Vetter Oct. 4, 2018, 8:24 p.m. UTC
With armada the last bigger driver that realistically needed these to
convert from legacy kms to atomic is converted. These helpers have
been broken more often than not the past 2 years, and as this little
patch series shows, tricked a bunch of people into using the wrong
helpers for their functions.

Aside: I think a lot more drivers should be using the device-level
drm_atomic_helper_shutdown/suspend/resume helpers and related
functions. In almost all the cases they get things exactly right.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c  | 115 -----------------
 drivers/gpu/drm/drm_plane_helper.c | 197 -----------------------------
 include/drm/drm_crtc_helper.h      |   6 -
 include/drm/drm_plane_helper.h     |  14 --
 4 files changed, 332 deletions(-)

Comments

Sam Ravnborg Oct. 4, 2018, 9:04 p.m. UTC | #1
Hi Daniel.

> Aside: I think a lot more drivers should be using the device-level
> drm_atomic_helper_shutdown/suspend/resume helpers and related
> functions. In almost all the cases they get things exactly right.
One more point to todo.rst then?

	Sam
Daniel Vetter Oct. 5, 2018, 6:10 a.m. UTC | #2
On Thu, Oct 4, 2018 at 11:04 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel.
>
> > Aside: I think a lot more drivers should be using the device-level
> > drm_atomic_helper_shutdown/suspend/resume helpers and related
> > functions. In almost all the cases they get things exactly right.
> One more point to todo.rst then?

We have:

https://dri.freedesktop.org/docs/drm/gpu/todo.html#convert-drivers-to-use-simple-modeset-suspend-resume

Except this doesn't mention _shutdown. Feel free to type up a quick patch.
-Daniel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index ce75e9506e85..a3c81850e755 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -984,118 +984,3 @@  void drm_helper_resume_force_mode(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
-
-/**
- * drm_helper_crtc_mode_set - mode_set implementation for atomic plane helpers
- * @crtc: DRM CRTC
- * @mode: DRM display mode which userspace requested
- * @adjusted_mode: DRM display mode adjusted by ->mode_fixup callbacks
- * @x: x offset of the CRTC scanout area on the underlying framebuffer
- * @y: y offset of the CRTC scanout area on the underlying framebuffer
- * @old_fb: previous framebuffer
- *
- * This function implements a callback useable as the ->mode_set callback
- * required by the CRTC helpers. Besides the atomic plane helper functions for
- * the primary plane the driver must also provide the ->mode_set_nofb callback
- * to set up the CRTC.
- *
- * This is a transitional helper useful for converting drivers to the atomic
- * interfaces.
- */
-int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			     struct drm_display_mode *adjusted_mode, int x, int y,
-			     struct drm_framebuffer *old_fb)
-{
-	struct drm_crtc_state *crtc_state;
-	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
-	int ret;
-
-	if (crtc->funcs->atomic_duplicate_state)
-		crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
-	else {
-		if (!crtc->state)
-			drm_atomic_helper_crtc_reset(crtc);
-
-		crtc_state = drm_atomic_helper_crtc_duplicate_state(crtc);
-	}
-
-	if (!crtc_state)
-		return -ENOMEM;
-
-	crtc_state->planes_changed = true;
-	crtc_state->mode_changed = true;
-	ret = drm_atomic_set_mode_for_crtc(crtc_state, mode);
-	if (ret)
-		goto out;
-	drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);
-
-	if (crtc_funcs->atomic_check) {
-		ret = crtc_funcs->atomic_check(crtc, crtc_state);
-		if (ret)
-			goto out;
-	}
-
-	swap(crtc->state, crtc_state);
-
-	crtc_funcs->mode_set_nofb(crtc);
-
-	ret = drm_helper_crtc_mode_set_base(crtc, x, y, old_fb);
-
-out:
-	if (crtc_state) {
-		if (crtc->funcs->atomic_destroy_state)
-			crtc->funcs->atomic_destroy_state(crtc, crtc_state);
-		else
-			drm_atomic_helper_crtc_destroy_state(crtc, crtc_state);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL(drm_helper_crtc_mode_set);
-
-/**
- * drm_helper_crtc_mode_set_base - mode_set_base implementation for atomic plane helpers
- * @crtc: DRM CRTC
- * @x: x offset of the CRTC scanout area on the underlying framebuffer
- * @y: y offset of the CRTC scanout area on the underlying framebuffer
- * @old_fb: previous framebuffer
- *
- * This function implements a callback useable as the ->mode_set_base used
- * required by the CRTC helpers. The driver must provide the atomic plane helper
- * functions for the primary plane.
- *
- * This is a transitional helper useful for converting drivers to the atomic
- * interfaces.
- */
-int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-				  struct drm_framebuffer *old_fb)
-{
-	struct drm_plane_state *plane_state;
-	struct drm_plane *plane = crtc->primary;
-
-	if (plane->funcs->atomic_duplicate_state)
-		plane_state = plane->funcs->atomic_duplicate_state(plane);
-	else {
-		if (!plane->state)
-			drm_atomic_helper_plane_reset(plane);
-
-		plane_state = drm_atomic_helper_plane_duplicate_state(plane);
-	}
-	if (!plane_state)
-		return -ENOMEM;
-	plane_state->plane = plane;
-
-	plane_state->crtc = crtc;
-	drm_atomic_set_fb_for_plane(plane_state, crtc->primary->fb);
-	plane_state->crtc_x = 0;
-	plane_state->crtc_y = 0;
-	plane_state->crtc_h = crtc->mode.vdisplay;
-	plane_state->crtc_w = crtc->mode.hdisplay;
-	plane_state->src_x = x << 16;
-	plane_state->src_y = y << 16;
-	plane_state->src_h = crtc->mode.vdisplay << 16;
-	plane_state->src_w = crtc->mode.hdisplay << 16;
-
-	return drm_plane_helper_commit(plane, plane_state, old_fb);
-}
-EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index a393756b664e..965286231600 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -336,200 +336,3 @@  const struct drm_plane_funcs drm_primary_helper_funcs = {
 	.destroy = drm_primary_helper_destroy,
 };
 EXPORT_SYMBOL(drm_primary_helper_funcs);
-
-int drm_plane_helper_commit(struct drm_plane *plane,
-			    struct drm_plane_state *plane_state,
-			    struct drm_framebuffer *old_fb)
-{
-	const struct drm_plane_helper_funcs *plane_funcs;
-	struct drm_crtc *crtc[2];
-	const struct drm_crtc_helper_funcs *crtc_funcs[2];
-	int i, ret = 0;
-
-	plane_funcs = plane->helper_private;
-
-	/* Since this is a transitional helper we can't assume that plane->state
-	 * is always valid. Hence we need to use plane->crtc instead of
-	 * plane->state->crtc as the old crtc. */
-	crtc[0] = plane->crtc;
-	crtc[1] = crtc[0] != plane_state->crtc ? plane_state->crtc : NULL;
-
-	for (i = 0; i < 2; i++)
-		crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL;
-
-	if (plane_funcs->atomic_check) {
-		ret = plane_funcs->atomic_check(plane, plane_state);
-		if (ret)
-			goto out;
-	}
-
-	if (plane_funcs->prepare_fb && plane_state->fb != old_fb) {
-		ret = plane_funcs->prepare_fb(plane,
-					      plane_state);
-		if (ret)
-			goto out;
-	}
-
-	/* Point of no return, commit sw state. */
-	swap(plane->state, plane_state);
-
-	for (i = 0; i < 2; i++) {
-		if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin)
-			crtc_funcs[i]->atomic_begin(crtc[i], crtc[i]->state);
-	}
-
-	/*
-	 * Drivers may optionally implement the ->atomic_disable callback, so
-	 * special-case that here.
-	 */
-	if (drm_atomic_plane_disabling(plane_state, plane->state) &&
-	    plane_funcs->atomic_disable)
-		plane_funcs->atomic_disable(plane, plane_state);
-	else
-		plane_funcs->atomic_update(plane, plane_state);
-
-	for (i = 0; i < 2; i++) {
-		if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush)
-			crtc_funcs[i]->atomic_flush(crtc[i], crtc[i]->state);
-	}
-
-	/*
-	 * If we only moved the plane and didn't change fb's, there's no need to
-	 * wait for vblank.
-	 */
-	if (plane->state->fb == old_fb)
-		goto out;
-
-	for (i = 0; i < 2; i++) {
-		if (!crtc[i])
-			continue;
-
-		if (crtc[i]->cursor == plane)
-			continue;
-
-		/* There's no other way to figure out whether the crtc is running. */
-		ret = drm_crtc_vblank_get(crtc[i]);
-		if (ret == 0) {
-			drm_crtc_wait_one_vblank(crtc[i]);
-			drm_crtc_vblank_put(crtc[i]);
-		}
-
-		ret = 0;
-	}
-
-	if (plane_funcs->cleanup_fb)
-		plane_funcs->cleanup_fb(plane, plane_state);
-out:
-	if (plane->funcs->atomic_destroy_state)
-		plane->funcs->atomic_destroy_state(plane, plane_state);
-	else
-		drm_atomic_helper_plane_destroy_state(plane, plane_state);
-
-	return ret;
-}
-
-/**
- * drm_plane_helper_update() - Transitional helper for plane update
- * @plane: plane object to update
- * @crtc: owning CRTC of owning plane
- * @fb: framebuffer to flip onto plane
- * @crtc_x: x offset of primary plane on crtc
- * @crtc_y: y offset of primary plane on crtc
- * @crtc_w: width of primary plane rectangle on crtc
- * @crtc_h: height of primary plane rectangle on crtc
- * @src_x: x offset of @fb for panning
- * @src_y: y offset of @fb for panning
- * @src_w: width of source rectangle in @fb
- * @src_h: height of source rectangle in @fb
- * @ctx: lock acquire context, not used here
- *
- * Provides a default plane update handler using the atomic plane update
- * functions. It is fully left to the driver to check plane constraints and
- * handle corner-cases like a fully occluded or otherwise invisible plane.
- *
- * This is useful for piecewise transitioning of a driver to the atomic helpers.
- *
- * RETURNS:
- * Zero on success, error code on failure
- */
-int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
-			    struct drm_framebuffer *fb,
-			    int crtc_x, int crtc_y,
-			    unsigned int crtc_w, unsigned int crtc_h,
-			    uint32_t src_x, uint32_t src_y,
-			    uint32_t src_w, uint32_t src_h,
-			    struct drm_modeset_acquire_ctx *ctx)
-{
-	struct drm_plane_state *plane_state;
-
-	if (plane->funcs->atomic_duplicate_state)
-		plane_state = plane->funcs->atomic_duplicate_state(plane);
-	else {
-		if (!plane->state)
-			drm_atomic_helper_plane_reset(plane);
-
-		plane_state = drm_atomic_helper_plane_duplicate_state(plane);
-	}
-	if (!plane_state)
-		return -ENOMEM;
-	plane_state->plane = plane;
-
-	plane_state->crtc = crtc;
-	drm_atomic_set_fb_for_plane(plane_state, fb);
-	plane_state->crtc_x = crtc_x;
-	plane_state->crtc_y = crtc_y;
-	plane_state->crtc_h = crtc_h;
-	plane_state->crtc_w = crtc_w;
-	plane_state->src_x = src_x;
-	plane_state->src_y = src_y;
-	plane_state->src_h = src_h;
-	plane_state->src_w = src_w;
-
-	return drm_plane_helper_commit(plane, plane_state, plane->fb);
-}
-EXPORT_SYMBOL(drm_plane_helper_update);
-
-/**
- * drm_plane_helper_disable() - Transitional helper for plane disable
- * @plane: plane to disable
- * @ctx: lock acquire context, not used here
- *
- * Provides a default plane disable handler using the atomic plane update
- * functions. It is fully left to the driver to check plane constraints and
- * handle corner-cases like a fully occluded or otherwise invisible plane.
- *
- * This is useful for piecewise transitioning of a driver to the atomic helpers.
- *
- * RETURNS:
- * Zero on success, error code on failure
- */
-int drm_plane_helper_disable(struct drm_plane *plane,
-			     struct drm_modeset_acquire_ctx *ctx)
-{
-	struct drm_plane_state *plane_state;
-	struct drm_framebuffer *old_fb;
-
-	/* crtc helpers love to call disable functions for already disabled hw
-	 * functions. So cope with that. */
-	if (!plane->crtc)
-		return 0;
-
-	if (plane->funcs->atomic_duplicate_state)
-		plane_state = plane->funcs->atomic_duplicate_state(plane);
-	else {
-		if (!plane->state)
-			drm_atomic_helper_plane_reset(plane);
-
-		plane_state = drm_atomic_helper_plane_duplicate_state(plane);
-	}
-	if (!plane_state)
-		return -ENOMEM;
-	plane_state->plane = plane;
-
-	plane_state->crtc = NULL;
-	old_fb = plane_state->fb;
-	drm_atomic_set_fb_for_plane(plane_state, NULL);
-
-	return drm_plane_helper_commit(plane, plane_state, old_fb);
-}
-EXPORT_SYMBOL(drm_plane_helper_disable);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 6914633037a5..d65f034843ce 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -57,12 +57,6 @@  int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
 
 void drm_helper_resume_force_mode(struct drm_device *dev);
 
-int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			     struct drm_display_mode *adjusted_mode, int x, int y,
-			     struct drm_framebuffer *old_fb);
-int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-				  struct drm_framebuffer *old_fb);
-
 /* drm_probe_helper.c */
 int drm_helper_probe_single_connector_modes(struct drm_connector
 					    *connector, uint32_t maxX,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 26cee2934781..1999781781c7 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -62,18 +62,4 @@  int drm_primary_helper_disable(struct drm_plane *plane,
 void drm_primary_helper_destroy(struct drm_plane *plane);
 extern const struct drm_plane_funcs drm_primary_helper_funcs;
 
-int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
-			    struct drm_framebuffer *fb,
-			    int crtc_x, int crtc_y,
-			    unsigned int crtc_w, unsigned int crtc_h,
-			    uint32_t src_x, uint32_t src_y,
-			    uint32_t src_w, uint32_t src_h,
-			    struct drm_modeset_acquire_ctx *ctx);
-int drm_plane_helper_disable(struct drm_plane *plane,
-			     struct drm_modeset_acquire_ctx *ctx);
-
-/* For use by drm_crtc_helper.c */
-int drm_plane_helper_commit(struct drm_plane *plane,
-			    struct drm_plane_state *plane_state,
-			    struct drm_framebuffer *old_fb);
 #endif