lkml.org 
[lkml]   [2021]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 08/11] drm: Rename plane->state variables in atomic update and disable
Hi Ville,

On Fri, Jan 22, 2021 at 02:15:07PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 21, 2021 at 05:35:33PM +0100, Maxime Ripard wrote:
> > Some drivers are storing the plane->state pointer in atomic_update and
> > atomic_disable in a variable simply called state, while the state passed
> > as an argument is called old_state.
> >
> > In order to ease subsequent reworks and to avoid confusing or
> > inconsistent names, let's rename those variables to new_state.
> >
> > This was done using the following coccinelle script, plus some manual
> > changes for mtk and tegra.
> >
> > @ 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,
> > ...,
> > };
> > )
> >
> > @ moves_new_state_old_state @
> > identifier plane_atomic_func.func;
> > identifier plane;
> > symbol old_state;
> > symbol state;
> > @@
> >
> > func(struct drm_plane *plane, struct drm_plane_state *old_state)
> > {
> > ...
> > - struct drm_plane_state *state = plane->state;
> > + struct drm_plane_state *new_state = plane->state;
> > ...
> > }
> >
> > @ depends on moves_new_state_old_state @
> > identifier plane_atomic_func.func;
> > identifier plane;
> > identifier old_state;
> > symbol state;
> > @@
> >
> > func(struct drm_plane *plane, struct drm_plane_state *old_state)
> > {
> > <...
> > - state
> > + new_state
> > ...>
>
> Was going to say that this migh eat something else, but I guess
> the dependency prevents that?

Yeah, the dependency takes care of this

> Another way to avoid that I suppose would be to declare 'state'
> as
> symbol moves_new_state_old_state.state;
>
> That would probably make the intent a bit more obvious, even with
> the dependency. Or does a dependency somehow automagically imply
> that?

I'm not sure if it does, but it's a symbol here not an identifier or an
expression, so here moves_new_state_old_state.state would always resolve
to state (and only state) anyway

Maxime
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-01-26 06:41    [W:1.085 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site