On Mon, Jan 25, 2021 at 11:52:18AM +0100, Maxime Ripard wrote:

Hi Ville,

On Fri, Jan 22, 2021 at 02:15:07PM +0200, Ville Syrjälä wrote:

quoted

On Thu, Jan 21, 2021 at 05:35:33PM +0100, Maxime Ripard wrote:

quoted

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

quoted

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

Hm. Right. OK, cocci bits look good to me. Variable naming
bikeshed I'll leave to others :)

Reviewed-by: Ville Syrjälä <redacted>

-- 
Ville Syrjälä
Intel