Hi Laurent,
> > @@ -508,8 +510,8 @@ static const struct drm_bridge_funcs ps8640_bridge_funcs = {
> > .attach = ps8640_bridge_attach,
> > .detach = ps8640_bridge_detach,
> > .get_edid = ps8640_bridge_get_edid,
> > - .post_disable = ps8640_post_disable,
> > - .pre_enable = ps8640_pre_enable,
> > + .atomic_post_disable = ps8640_atomic_post_disable,
> > + .atomic_pre_enable = ps8640_atomic_pre_enable,
>
> Don't you also need to implement .atomic_duplicate_state(),
> .atomic_destroy_state() and .atomic_reset() to use the atomic API ?
What I think happens is that the atomic drivers uses the bridges and
the implementation in drm_bridge has fall-backs to the non-atomic
variants.
So for example drm_atomic_bridge_chain_pre_enable() will call
atomic_pre_enable() if it exists, and if not it will call pre_enable()
if it exists.
With this patch-set I show that the non-atomic versions of several of
the drm_bridge helper functions are almost not used and easy to drop.
So based on this I concluded that the bridge drivers were used
in a way so we only would need to implement the atomic variants
of the functions.
That said I did not consider .atomic_duplicate_state(),
.atomic_destroy_state(), or .atomic_reset().
>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,
And new bridges (or bridges we convert to use the atomic operations)
would not have to specify them.
I would prefer implementing the fallback - rather than adding the above
to this driver.
Sam
