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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 20, 2020 at 12:25:57PM +0100, Maxime Ripard wrote:
> 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.

git log --grep didn't show any commits from me using it at least.
I must have never sent them.

Digging through my pile of old cocci scripts I found a few uses. Eg.:
@r1@
identifier F !~ "_destroy$|_reset$";
identifier E;
parameter list[N] PS;
@@
F(PS,
- struct drm_encoder *E
+ struct intel_encoder *encoder
  ,...)
{ ... }

@@
identifier r1.F;
expression E;
expression list[r1.N] ES;
@@
F(ES,
- E
+ to_intel_encoder(E)
  ,...)

My vague recollection is that it would work for the
N==0 case as well.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel