Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:54PM +0100, Maxime Ripard wrote:
Most drivers call the argument to the plane atomic_check hook simply state, which is going to conflict with the global atomic state in a later rework. Let's rename it to new_plane_state (or new_state depending on the convention used in the driver).
This was done using the coccinelle script below, and built tested:
@ plane_atomic_func @ identifier helpers; identifier func; @@
static const struct drm_plane_helper_funcs helpers = { .atomic_check = func, };
@ has_old_state @ identifier plane_atomic_func.func; identifier plane; expression e; symbol old_state; symbol state; @@
func(struct drm_plane *plane, struct drm_plane_state *state) { ... struct drm_plane_state *old_state = e; ... }
@ depends on has_old_state @ identifier plane_atomic_func.func; identifier plane; symbol old_state; @@
func(struct drm_plane *plane,
- struct drm_plane_state *state
- struct drm_plane_state *new_state )
{ <+...
- state
- new_state ...+>
}
@ has_state @ identifier plane_atomic_func.func; identifier plane; symbol state; @@
func(struct drm_plane *plane, struct drm_plane_state *state) { ... }
@ depends on has_state @ identifier plane_atomic_func.func; identifier plane; symbol old_state; @@
func(struct drm_plane *plane,
- struct drm_plane_state *state
- struct drm_plane_state *new_plane_state )
{ <+...
- state
- new_plane_state ...+>
}
Signed-off-by: Maxime Ripard maxime@cerno.tech
[...]
drivers/gpu/drm/omapdrm/omap_plane.c | 19 +++++---- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 7 ++-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 7 ++-- drivers/gpu/drm/xlnx/zynqmp_disp.c | 10 +++--
For these, with the comment below addressed,
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
41 files changed, 402 insertions(+), 357 deletions(-)
[snip]
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 51dc24acea73..78d0eb1fd69d 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -99,18 +99,19 @@ static void omap_plane_atomic_disable(struct drm_plane *plane, }
static int omap_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
struct drm_plane_state *new_plane_state){ struct drm_crtc_state *crtc_state;
- if (!state->fb)
if (!new_plane_state->fb) return 0;
/* crtc should only be NULL when disabling (i.e., !state->fb) */
s/state/new_plane_state/ here too ?
- if (WARN_ON(!state->crtc))
- if (WARN_ON(!new_plane_state->crtc)) return 0;
- crtc_state = drm_atomic_get_existing_crtc_state(state->state, state->crtc);
- crtc_state = drm_atomic_get_existing_crtc_state(new_plane_state->state,
new_plane_state->crtc);@@ -118,17 +119,17 @@ static int omap_plane_atomic_check(struct drm_plane *plane, if (!crtc_state->enable) return 0;
- if (state->crtc_x < 0 || state->crtc_y < 0)
- if (new_plane_state->crtc_x < 0 || new_plane_state->crtc_y < 0) return -EINVAL;
- if (state->crtc_x + state->crtc_w > crtc_state->adjusted_mode.hdisplay)
- if (new_plane_state->crtc_x + new_plane_state->crtc_w > crtc_state->adjusted_mode.hdisplay)
I can't help thinking we're using too long variable names... :-(
return -EINVAL;
- if (state->crtc_y + state->crtc_h > crtc_state->adjusted_mode.vdisplay)
- if (new_plane_state->crtc_y + new_plane_state->crtc_h > crtc_state->adjusted_mode.vdisplay) return -EINVAL;
- if (state->rotation != DRM_MODE_ROTATE_0 &&
!omap_framebuffer_supports_rotation(state->fb))
if (new_plane_state->rotation != DRM_MODE_ROTATE_0 &&
!omap_framebuffer_supports_rotation(new_plane_state->fb))return -EINVAL;
return 0;
[...]