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