On Tue, 20 Aug 2019 22:05:05 +0300 Laurent Pinchart laurent.pinchart@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