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.syrj...@linux.intel.com>

Thanks! I've applied it

Maxime

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to