Hi Laurent, On Fri, Oct 22, 2021 at 09:42:37PM +0300, Laurent Pinchart wrote: > Hi Sam, > > On Fri, Oct 22, 2021 at 07:13:58PM +0200, Sam Ravnborg wrote: > > Hi Laurent, > > > > > From a quick look only cadence/cdns-mhdp8546 subclass > > > drm_bridge_state and I wonder if the right thing to do would be to > > > implement fallback to the helpers if the bridge driver do not set > > > any of the .atomic_duplicate_state(), .atomic_destroy_state(), or .atomic_reset(). > > > > > > That would drop the following from a few bridges: > > > .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > > > .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > > > .atomic_reset = drm_atomic_helper_bridge_reset, > > > > To answer myself here. This would create a dependency from the core to > > the helpers which is not OK so idea dropped again. > > I agree it would be nicer, but the dependency is likely a problem. That > being said, we have multiple types of helpers. The first set is the > modeset helpers, which were designed as one implementation of KMS > operations, with an opt-in API for drivers. The core should not depend > on those. There are however other helpers that are only default > implementations of some operations, without any dependency on other > components. The atomic state helpers fall in this category, they > implement .atomic_* operations of the drm_*_funcs structures, not > drm_*_helper_funcs. It could make sense to move them to the DRM core. For now I went with a simple macro: +/** + * DRM_BRIDGE_STATE_OPS - Default drm_bridge state funcs + * + * 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 + Thomas Z. is trying to make the core smaller so pulling in these helpers would be counterproductive to that. So I took the simpler approach here which we have already done in several places. It will be part of v3 when I post it. Drop a note if you (or any other reader) have better ideas. Sam
