[PATCH v2 1/7] drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs

Sam Ravnborg sam at ravnborg.org
Fri Oct 22 12:32:08 PDT 2021


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



More information about the Linux-mediatek mailing list