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

[Sam Ravnborg]

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