Re: [PATCH 02/10] drm: Rename plane atomic_check state names
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2021-01-15 20:30:31
Also in:
amd-gfx, dri-devel, linux-amlogic, linux-arm-msm, linux-mediatek, linux-renesas-soc, linux-samsung-soc, lkml, nouveau, virtualization
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 <redacted>
---[...]
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]
quoted hunk ↗ jump to hunk
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 ?
quoted hunk ↗ jump to hunk
- 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); /* we should have a crtc state if the plane is attached to a crtc */ if (WARN_ON(!crtc_state)) return 0;@@ -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;
[...] -- Regards, Laurent Pinchart