diff mbox

[02/19] drm: Add acquire ctx parameter to ->update_plane

Message ID 20170322215058.8671-3-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 22, 2017, 9:50 p.m. UTC 
Just rolling it out, no code change here.

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/armada/armada_overlay.c    | 3 ++-
 drivers/gpu/drm/drm_atomic_helper.c        | 4 +++-
 drivers/gpu/drm/drm_plane.c                | 2 +-
 drivers/gpu/drm/drm_plane_helper.c         | 4 +++-
 drivers/gpu/drm/i915/intel_display.c       | 5 +++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c  | 8 +++++---
 drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++++--
 drivers/gpu/drm/shmobile/shmob_drm_plane.c | 3 ++-
 drivers/gpu/drm/vc4/vc4_plane.c            | 6 ++++--
 include/drm/drm_atomic_helper.h            | 3 ++-
 include/drm/drm_plane.h                    | 4 +++-
 include/drm/drm_plane_helper.h             | 3 ++-
 12 files changed, 34 insertions(+), 17 deletions(-)

Comments

Russell King (Oracle) March 22, 2017, 11:03 p.m. UTC | #1 
On Wed, Mar 22, 2017 at 10:50:41PM +0100, Daniel Vetter wrote:
> diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
> index 34cb73d0db77..b54fd8cbd3a6 100644
> --- a/drivers/gpu/drm/armada/armada_overlay.c
> +++ b/drivers/gpu/drm/armada/armada_overlay.c
> @@ -94,7 +94,8 @@ static int
>  armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
>  	struct drm_framebuffer *fb,
>  	int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h,
> -	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h)
> +	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h,
> +	struct drm_modeset_acquire_ctx *ctx)

I'm rather unhappy that we're ending up with a function taking soo many
arguments.

Most of these have to be stacked on ARM, and I'm guessing most
architectures end up doing something similar.  Is there a reason why we
don't pass pointers to drm_rect's or maybe even consider passing the
drm_plane_state structure in?

I've found that, when cleaning up these code paths in armada, that
storing all the parameters into a drm_plane_state and then validating
it with drm_plane_helper_check_state() is by way the simplest solution,
and of course, it's forward-compatible with atomic modeset.
Daniel Vetter March 24, 2017, 7:07 a.m. UTC | #2 
On Wed, Mar 22, 2017 at 11:03:41PM +0000, Russell King - ARM Linux wrote:
> On Wed, Mar 22, 2017 at 10:50:41PM +0100, Daniel Vetter wrote:
> > diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
> > index 34cb73d0db77..b54fd8cbd3a6 100644
> > --- a/drivers/gpu/drm/armada/armada_overlay.c
> > +++ b/drivers/gpu/drm/armada/armada_overlay.c
> > @@ -94,7 +94,8 @@ static int
> >  armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> >  	struct drm_framebuffer *fb,
> >  	int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h,
> > -	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h)
> > +	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h,
> > +	struct drm_modeset_acquire_ctx *ctx)
> 
> I'm rather unhappy that we're ending up with a function taking soo many
> arguments.
> 
> Most of these have to be stacked on ARM, and I'm guessing most
> architectures end up doing something similar.  Is there a reason why we
> don't pass pointers to drm_rect's or maybe even consider passing the
> drm_plane_state structure in?
> 
> I've found that, when cleaning up these code paths in armada, that
> storing all the parameters into a drm_plane_state and then validating
> it with drm_plane_helper_check_state() is by way the simplest solution,
> and of course, it's forward-compatible with atomic modeset.

Yeah, we could do that, there's not many plane_update implementations
left. But it wouldn't help for this case here, because acquire context is
in drm_atomic_state, not in the individual per-object state structs.

There's no reason this isn't the case except organically grown and no one
bothered to improve it. I'll be happy to review such patches, but probably
won't ever get around to typing them.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
index 34cb73d0db77..b54fd8cbd3a6 100644
--- a/drivers/gpu/drm/armada/armada_overlay.c
+++ b/drivers/gpu/drm/armada/armada_overlay.c
@@ -94,7 +94,8 @@  static int
 armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 	struct drm_framebuffer *fb,
 	int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h,
-	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h)
+	uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h,
+	struct drm_modeset_acquire_ctx *ctx)
 {
 	struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane);
 	struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 278d72aa012f..8ef7c808670b 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2078,6 +2078,7 @@  EXPORT_SYMBOL(drm_atomic_helper_swap_state);
  * @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
  *
  * Provides a default plane update handler using the atomic driver interface.
  *
@@ -2090,7 +2091,8 @@  int drm_atomic_helper_update_plane(struct drm_plane *plane,
 				   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)
+				   uint32_t src_w, uint32_t src_h,
+				   struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_atomic_state *state;
 	struct drm_plane_state *plane_state;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 0d04888172a7..8a4e75929810 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -510,7 +510,7 @@  static int __setplane_internal(struct drm_plane *plane,
 	plane->old_fb = plane->fb;
 	ret = plane->funcs->update_plane(plane, crtc, fb,
 					 crtc_x, crtc_y, crtc_w, crtc_h,
-					 src_x, src_y, src_w, src_h);
+					 src_x, src_y, src_w, src_h, ctx);
 	if (!ret) {
 		plane->crtc = crtc;
 		plane->fb = fb;
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index de1ac5e08f4d..2339879f775d 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -275,6 +275,7 @@  EXPORT_SYMBOL(drm_plane_helper_check_update);
  * @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 for primary planes.  This is handler
  * is called in response to a userspace SetPlane operation on the plane with a
@@ -303,7 +304,8 @@  int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
 			      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)
+			      uint32_t src_w, uint32_t src_h,
+			      struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_mode_set set = {
 		.crtc = crtc,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 010e5ddb198a..e27ea89efd67 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13426,7 +13426,8 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 			   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)
+			   uint32_t src_w, uint32_t src_h,
+			   struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	int ret;
@@ -13539,7 +13540,7 @@  intel_legacy_cursor_update(struct drm_plane *plane,
 slow:
 	return drm_atomic_helper_update_plane(plane, crtc, fb,
 					      crtc_x, crtc_y, crtc_w, crtc_h,
-					      src_x, src_y, src_w, src_h);
+					      src_x, src_y, src_w, src_h, ctx);
 }
 
 static const struct drm_plane_funcs intel_cursor_plane_funcs = {
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 0ffb8affef35..60a5451ae0b9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -37,7 +37,8 @@  static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
 		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);
+		uint32_t src_w, uint32_t src_h,
+		struct drm_modeset_acquire_ctx *ctx);
 
 static void set_scanout_locked(struct drm_plane *plane,
 		struct drm_framebuffer *fb);
@@ -886,7 +887,8 @@  static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
 			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)
+			uint32_t src_w, uint32_t src_h,
+			struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_plane_state *plane_state, *new_plane_state;
 	struct mdp5_plane_state *mdp5_pstate;
@@ -954,7 +956,7 @@  static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
 slow:
 	return drm_atomic_helper_update_plane(plane, crtc, fb,
 					      crtc_x, crtc_y, crtc_w, crtc_h,
-					      src_x, src_y, src_w, src_h);
+					      src_x, src_y, src_w, src_h, ctx);
 }
 
 enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index 5319f2a7f24d..2d90e7898ec8 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -94,7 +94,8 @@  nv10_update_plane(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)
+		  uint32_t src_w, uint32_t src_h,
+		  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nouveau_drm *drm = nouveau_drm(plane->dev);
 	struct nvif_object *dev = &drm->client.device.object;
@@ -345,7 +346,8 @@  nv04_update_plane(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)
+		  uint32_t src_w, uint32_t src_h,
+		  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nvif_object *dev = &nouveau_drm(plane->dev)->client.device.object;
 	struct nouveau_plane *nv_plane =
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
index 2023a93cee2b..9a3c8ddfe198 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c
@@ -177,7 +177,8 @@  shmob_drm_plane_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)
+		       uint32_t src_w, uint32_t src_h,
+		       struct drm_modeset_acquire_ctx *ctx)
 {
 	struct shmob_drm_plane *splane = to_shmob_plane(plane);
 	struct shmob_drm_device *sdev = plane->dev->dev_private;
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 0f4564beb017..d34cd5393a9b 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -756,7 +756,8 @@  vc4_update_plane(struct drm_plane *plane,
 		 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)
+		 uint32_t src_w, uint32_t src_h,
+		 struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_plane_state *plane_state;
 	struct vc4_plane_state *vc4_state;
@@ -817,7 +818,8 @@  vc4_update_plane(struct drm_plane *plane,
 					      crtc_x, crtc_y,
 					      crtc_w, crtc_h,
 					      src_x, src_y,
-					      src_w, src_h);
+					      src_w, src_h,
+					      ctx);
 }
 
 static const struct drm_plane_funcs vc4_plane_funcs = {
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 969f7237f1fc..62ac6053721d 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -94,7 +94,8 @@  int drm_atomic_helper_update_plane(struct drm_plane *plane,
 				   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);
+				   uint32_t src_w, uint32_t src_h,
+				   struct drm_modeset_acquire_ctx *ctx);
 int drm_atomic_helper_disable_plane(struct drm_plane *plane);
 int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
 		struct drm_plane_state *plane_state);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 31da9f0c4ad2..1076fe150cdf 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -29,6 +29,7 @@ 
 
 struct drm_crtc;
 struct drm_printer;
+struct drm_modeset_acquire_ctx;
 
 /**
  * struct drm_plane_state - mutable plane state
@@ -184,7 +185,8 @@  struct drm_plane_funcs {
 			    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);
+			    uint32_t src_w, uint32_t src_h,
+			    struct drm_modeset_acquire_ctx *ctx);
 
 	/**
 	 * @disable_plane:
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index c18959685c06..ea219423d72b 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -61,7 +61,8 @@  int drm_primary_helper_update(struct drm_plane *plane,
 			      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);
+			      uint32_t src_w, uint32_t src_h,
+			      struct drm_modeset_acquire_ctx *ctx);
 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;