Hi Laurent,

On Tue, Feb 08, 2022 at 02:30:42AM +0200, Laurent Pinchart wrote:
> Hi Sam,
> 
> Thank you for the patch.
> 
> On Sun, Feb 06, 2022 at 04:43:57PM +0100, Sam Ravnborg wrote:
> > The DRM_BRIDGE_STATE_OPS can be used as shortcut for bridge drivers that
> > do not subclass drm_bridge_state to assign the default operations for
> > reset, duplicate and destroy of the state.
> > 
> > Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
> > Cc: Douglas Anderson <diand...@chromium.org>
> > Cc: Andrzej Hajda <a.ha...@samsung.com>
> > Cc: Neil Armstrong <narmstr...@baylibre.com>
> > Cc: Robert Foss <robert.f...@linaro.org>
> > Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > Cc: Jonas Karlman <jo...@kwiboo.se>
> > Cc: Jernej Skrabec <jernej.skra...@gmail.com>
> > Cc: Maxime Ripard <mrip...@kernel.org>
> > Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> > ---
> >  include/drm/drm_bridge.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 061d87313fac..fc00304be643 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -935,4 +935,16 @@ static inline struct drm_bridge 
> > *devm_drm_of_get_bridge(struct device *dev,
> >  }
> >  #endif
> >  
> > +/**
> > + * DRM_BRIDGE_STATE_OPS - Default drm_bridge state funcs
> 
> I'd name the macro DRM_BRIDGE_STATE_DEFAULT_OPS.
OK, done.

> 
> > + *
> > + * Bridge driver that do not subclass &drm_bridge_state can use the helpers
> > + * for reset, duplicate, and destroy. This macro provides a shortcut for
> > + * setting the helpers in the &drm_bridge_funcs structure.
> > + */
> > +#define DRM_BRIDGE_STATE_OPS \
> > +   .atomic_reset = drm_atomic_helper_bridge_reset,                         
> > \
> > +   .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,     
> > \
> > +   .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state
> 
> I'm not a big fan of such macros for a small number of operations, as I
> find that it obfuscates the code a bit, but that could change once I get
> used to the new macro :-)
The use of the macro is a nice signal that all the relevant default
operations are specified - no need to look up if all are included.

I have on my TODO to update all relevant bridge drivers to use it.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> 
> > +
> >  #endif
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Reply via email to