[PATCH RFC 02/19] drm: Support custom encoder/bridge enable/disable sequences officially

[Boris Brezillon]

On Tue, 20 Aug 2019 22:05:05 +0300
Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:

> Hi Boris,
> 
> Thank you for the patch.
> 
> On Thu, Aug 08, 2019 at 05:11:33PM +0200, Boris Brezillon wrote:
> > The VC4 and Exynos DSI encoder drivers need a custom enable/disable
> > sequence and manually set encoder->bridge to NULL to prevent the core
> > from calling the bridge hooks.
> > 
> > Let's provide a more official way to support custom bridge/encoder
> > enable/disable sequences.  
> 
> So, while I'm not opposed to this, I think it's a bit of a hack, and I'd
> like to share my vision of how I'd like to see this being implemented in
> the more distant future (and thus possibly on top of this change).

I agree, it definitely looks like a hack, but I find it less hack-ish
than letting drivers re-initialize the encoder->bridge field to NULL
just after calling bridge_attach() :P.

> 
> It has been established for quite some time now that exposing
> drm_encoder to userspace was likely a mistake, and that it should have
> stayed a kernel-only object. With the generalised usage of drm_bridge, I
> would go one step further: drm_encoder shouldn't matter for DRM/KMS
> drivers at all.

I couldn't agree more.

> 
> drm_bridge has been introduced to model chained encoders, where the
> second (and subsequent) encoders couldn't easily be supported in a
> standard and reusable way with just drm_encoder. A bridge (with the
> exception of the panel bridge) is just an encoder. It shouldn't matter
> whether encoders are internal to the display controller, separate IPs in
> the SoC or external components, they could all be modelled as bridges.
> We do have bridges for DSI encoder IPs, and DSI (and other) bridges
> potentially need similar custom enable/disable sequences. I would thus
> ideally like to see the VC4 and Exynos DSI encoders being implemented as
> bridges, with support for custom enable/disable sequences in bridges,
> and drop support for custom enable/disable sequences in drm_encoder.

Looks like we're on the same page.

> 
> Does this make sense to you ?

Definitely.

> While I would love to see this being
> implemented right away, it may be too much work as a prerequisite for
> this bus format negotiation series, so I don't want to reject this
> patch. I would however like to already make sure we agree on the way
> forward for the next steps.

Actually, I think I have something [1] that implements what you're
proposing here (which is not surprising since we discussed it on
IRC ;)). I'll send a v2 so you can comment on the new approach.

[1]https://github.com/bbrezillon/linux-0day/commits/drm-bridge-busfmt-2