Laurent Pinchart
2021-Jan-15 20:57 UTC
[PATCH 07/10] drm: Store new plane state in a variable for atomic_update and disable
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 at cerno.tech> > ---[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 at ideasonboard.com> [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