Hi Maxime,
Thank you for the patch.
On Fri, Jan 15, 2021 at 01:56:59PM +0100, Maxime Ripard wrote:
> In order to store the new plane state in a subsequent helper, let's move
> the plane->state dereferences into a variable.
>
> This was done using the following coccinelle script, plus some hand
> changes for vmwgfx:
>
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
>
> (
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_disable = func,
> ...,
> };
> |
> static const struct drm_plane_helper_funcs helpers = {
> ...,
> .atomic_update = func,
> ...,
> };
> )
>
> @ has_new_state_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_state)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_old_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_state)
> {
> + struct drm_plane_state *new_state = plane->state;
> <+...
> - plane->state
> + new_state
> ...+>
> }
>
> @ has_new_state_state @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_state @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol state;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *state)
> {
> + struct drm_plane_state *new_plane_state = plane->state;
> <+...
> - plane->state
> + new_plane_state
> ...+>
> }
>
> @ has_new_state_old_s @
> identifier plane_atomic_func.func;
> identifier plane;
> identifier new_state;
> symbol old_s;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_s)
> {
> ...
> struct drm_plane_state *new_state = plane->state;
> ...
> }
>
> @ depends on !has_new_state_old_s @
> identifier plane_atomic_func.func;
> identifier plane;
> symbol old_s;
> @@
>
> func(struct drm_plane *plane, struct drm_plane_state *old_s)
> {
> + struct drm_plane_state *new_s = plane->state;
> <+...
> - plane->state
> + new_s
> ...+>
> }
I may have taken this as an opportunity to align naming conventions for
variables across drivers, but that may just be me.
>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> ---
[snip]
> drivers/gpu/drm/omapdrm/omap_plane.c | 5 ++-
> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 5 ++-
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 7 ++--
For these, with the small issue below addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
[snip]
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 1042e1147e74..de5ad69af4cb 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -88,11 +88,12 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
> static void omap_plane_atomic_disable(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> + struct drm_plane_state *new_state = plane->state;
> struct omap_drm_private *priv = plane->dev->dev_private;
> struct omap_plane *omap_plane = to_omap_plane(plane);
>
> - plane->state->rotation = DRM_MODE_ROTATE_0;
> - plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> + new_state->rotation = DRM_MODE_ROTATE_0;
> + new_state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> ? 0 : omap_plane->id;
Can you fix the indentation ?
> dispc_ovl_enable(priv->dispc, omap_plane->id, false);
[snip]
--
Regards,
Laurent Pinchart
