On Wed, Oct 28, 2020 at 01:32:22PM +0100, Maxime Ripard wrote:
> The current atomic helpers have either their object state being passed as
> an argument or the full atomic state.
> 
> The former is the pattern that was done at first, before switching to the
> latter for new hooks or when it was needed.
> 
> Let's start convert all the remaining helpers to provide a consistent
> interface, starting with the CRTC's atomic_begin and atomic_flush.
> 
> The conversion was done using the coccinelle script below, built tested on
> all the drivers and actually tested on vc4.
> 
> virtual report
> 
> @@
> struct drm_crtc_helper_funcs *FUNCS;
> identifier old_crtc_state, old_state;
> identifier crtc;
> identifier f;
> @@
> 
>  f(struct drm_crtc_state *old_crtc_state)
>  {
> 	...
>  	struct drm_atomic_state *old_state = old_crtc_state->state;
> 	<...
> -	FUNCS->atomic_begin(crtc, old_crtc_state);
> +	FUNCS->atomic_begin(crtc, old_state);
> 	...>
>  }
> 
> @@
> struct drm_crtc_helper_funcs *FUNCS;
> identifier old_crtc_state, old_state;
> identifier crtc;
> identifier f;
> @@
> 
>  f(struct drm_crtc_state *old_crtc_state)
>  {
> 	...
>  	struct drm_atomic_state *old_state = old_crtc_state->state;
> 	<...
> -	FUNCS->atomic_flush(crtc, old_crtc_state);
> +	FUNCS->atomic_flush(crtc, old_state);
> 	...>
>  }
> 
> @@
> struct drm_crtc_helper_funcs *FUNCS;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> identifier dev, state;
> identifier f;
> @@
> 
>  f(struct drm_device *dev, struct drm_atomic_state *state, ...)
>  {
> 	<...
> -	FUNCS->atomic_begin(crtc, crtc_state);
> +	FUNCS->atomic_begin(crtc, state);
> 	...>
>  }
> 
> @@
> struct drm_crtc_helper_funcs *FUNCS;
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> identifier dev, state;
> identifier f;
> @@
> 
>  f(struct drm_device *dev, struct drm_atomic_state *state, ...)
>  {
> 	<...
> -	FUNCS->atomic_flush(crtc, crtc_state);
> +	FUNCS->atomic_flush(crtc, state);
> 	...>
>  }
> 
> @@
> identifier crtc, old_state;
> @@
> 
>  struct drm_crtc_helper_funcs {
> 	...
> -	void (*atomic_begin)(struct drm_crtc *crtc, struct drm_crtc_state *old_state);
> +	void (*atomic_begin)(struct drm_crtc *crtc, struct drm_atomic_state *state);
> 	...
> -	void (*atomic_flush)(struct drm_crtc *crtc, struct drm_crtc_state *old_state);
> +	void (*atomic_flush)(struct drm_crtc *crtc, struct drm_atomic_state *state);
> 	...
> }
> 
> @ crtc_atomic_func @
> identifier helpers;
> identifier func;
> @@
> 
> (
> static struct drm_crtc_helper_funcs helpers = {
> 	...,
> 	.atomic_begin = func,
> 	...,
> };
> |
> static struct drm_crtc_helper_funcs helpers = {
> 	...,
> 	.atomic_flush = func,
> 	...,
> };
> )
> 
> @ ignores_old_state @
> identifier crtc_atomic_func.func;
> identifier crtc, old_state;
> @@
> 
> void func(struct drm_crtc *crtc,
> 		struct drm_crtc_state *old_state)
> {
> 	... when != old_state
> }
> 
> @ adds_old_state depends on crtc_atomic_func && !ignores_old_state @
> identifier crtc_atomic_func.func;
> identifier crtc, old_state;
> @@
> 
> void func(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
> {
> +	struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc);
> 	...
> }
> 
> @ depends on crtc_atomic_func @
> identifier crtc_atomic_func.func;
> expression E;
> type T;
> @@
> 
> void func(...)
> {
> 	...
> -	T state = E;
> +	T crtc_state = E;
> 	<+...
> -	state
> +	crtc_state
> 	...+>
> 
> }
> 
> @ depends on crtc_atomic_func @
> identifier crtc_atomic_func.func;
> type T;
> @@
> 
> void func(...)
> {
> 	...
> -	T state;
> +	T crtc_state;
> 	<+...
> -	state
> +	crtc_state
> 	...+>
> 
> }
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   )
> {
> +	struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state, crtc);
> 	...
> }
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vc4_hvs_atomic_flush(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   );
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vmw_du_crtc_atomic_begin(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   )
> {
> 	...
> }
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vmw_du_crtc_atomic_begin(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   );
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vmw_du_crtc_atomic_flush(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   )
> {
> 	...
> }
> 
> @@
> identifier old_state;
> identifier crtc;
> @@
> 
>  void vmw_du_crtc_atomic_flush(struct drm_crtc *crtc,
> -			   struct drm_crtc_state *old_state
> +			   struct drm_atomic_state *state
> 			   );
> 
> @ depends on crtc_atomic_func @
> identifier crtc_atomic_func.func;
> identifier old_state;
> identifier crtc;
> @@
> 
> void func(struct drm_crtc *crtc,
> -	       struct drm_crtc_state *old_state
> +	       struct drm_atomic_state *state
> 	       )
> 		{ ... }
> 
> @ include depends on adds_old_state @
> @@
> 
>  #include <drm/drm_atomic.h>
> 
> @ no_include depends on !include && adds_old_state @
> @@
> 
> + #include <drm/drm_atomic.h>
>   #include <drm/...>
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Applied both. I'll send a few more patches to fix the issues Ville and
you mentioned.

Thanks!
Maxime