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