[PATCH v2] drm: Pass the full state to connectors atomic functions

[Maxime Ripard]

Hi,

On Thu, Nov 19, 2020 at 05:21:48PM +0200, Ville Syrjälä wrote:
> On Wed, Nov 18, 2020 at 10:47:58AM +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.
> > 
> > Now that the CRTCs have been converted, let's move forward with the
> > connectors to provide a consistent interface.
> > 
> > The conversion was done using the coccinelle script below, and built tested
> > on all the drivers.
> > 
> > @@
> > identifier connector, connector_state;
> > @@
> > 
> >  struct drm_connector_helper_funcs {
> > 	...
> > 	struct drm_encoder* (*atomic_best_encoder)(struct drm_connector *connector,
> > -						   struct drm_connector_state *connector_state);
> > +						   struct drm_atomic_state *state);
> > 	...
> > }
> > 
> > @@
> > identifier connector, connector_state;
> > @@
> > 
> >  struct drm_connector_helper_funcs {
> > 	...
> > 	void (*atomic_commit)(struct drm_connector *connector,
> > -			      struct drm_connector_state *connector_state);
> > +			      struct drm_atomic_state *state);
> > 	...
> > }
> > 
> > @@
> > struct drm_connector_helper_funcs *FUNCS;
> > identifier state;
> > identifier connector, connector_state;
> > identifier f;
> > @@
> > 
> >  f(..., struct drm_atomic_state *state, ...)
> >  {
> > 	<+...
> > -	FUNCS->atomic_commit(connector, connector_state);
> > +	FUNCS->atomic_commit(connector, state);
> > 	...+>
> >  }
> > 
> > @@
> > struct drm_connector_helper_funcs *FUNCS;
> > identifier state;
> > identifier connector, connector_state;
> > identifier var, f;
> > @@
> > 
> >  f(struct drm_atomic_state *state, ...)
> 
> I think there was some way to deal with these variants using a single
> rule, but maybe that required the use of the parameter list stuff
> which IIRC was a bit ugly. Probably not worth the hassle here.

Do you have any recollection of some patch that used it? I couldn't find
a cleaner way to deal with it, but I'd really like to use it if
available.

> >  {
> > 	<+...
> > -	var = FUNCS->atomic_best_encoder(connector, connector_state);
> > +	var = FUNCS->atomic_best_encoder(connector, state);
> > 	...+>
> >  }
> > 
> > @ connector_atomic_func @
> > identifier helpers;
> > identifier func;
> > @@
> > 
> > (
> > static struct drm_connector_helper_funcs helpers = {
> > 	...,
> > 	.atomic_best_encoder = func,
> > 	...,
> > };
> > |
> > static struct drm_connector_helper_funcs helpers = {
> > 	...,
> > 	.atomic_commit = func,
> > 	...,
> > };
> > )
> > 
> > @@
> > identifier connector_atomic_func.func;
> > identifier connector;
> > symbol state;
> > @@
> > 
> >  func(struct drm_connector *connector,
> > -      struct drm_connector_state *state
> > +      struct drm_connector_state *connector_state
> >       )
> >  {
> > 	...
> > -	state
> > +	connector_state
> >  	...
> >  }
> > 
> > @ ignores_state @
> > identifier connector_atomic_func.func;
> > identifier connector, connector_state;
> > @@
> > 
> >  func(struct drm_connector *connector,
> >       struct drm_connector_state *connector_state)
> > {
> > 	... when != connector_state
> > }
> > 
> > @ adds_state depends on connector_atomic_func && !ignores_state @
> > identifier connector_atomic_func.func;
> > identifier connector, connector_state;
> > @@
> > 
> >  func(struct drm_connector *connector, struct drm_connector_state *connector_state)
> >  {
> > +	struct drm_connector_state *connector_state = drm_atomic_get_new_connector_state(state, connector);
> > 	...
> >  }
> > 
> > @ depends on connector_atomic_func @
> > identifier connector_atomic_func.func;
> > identifier connector_state;
> > identifier connector;
> > @@
> > 
> >  func(struct drm_connector *connector,
> > -     struct drm_connector_state *connector_state
> > +     struct drm_atomic_state *state
> > 	   )
> >  { ... }
> > 
> > @ include depends on adds_state @
> > @@
> > 
> >  #include <drm/drm_atomic.h>
> > 
> > @ no_include depends on !include && adds_state @
> > @@
> > 
> > + #include <drm/drm_atomic.h>
> >   #include <drm/...>
> 
> Nicely done with the depends. Also never used that stuff myself
> so thanks for the tutorial :)

You're welcome :)

> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thanks! I've applied it

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20201120/d5b69a42/attachment-0001.sig>