Re: [PATCH 2/4] drm/i2c: tda998x: Remove obsolete drm_connector_register() call

[Russell King - ARM Linux <linux@armlinux.org.uk>]

On Thu, Oct 20, 2016 at 02:38:25PM +0530, Archit Taneja wrote:
> 
> 
> On 10/20/2016 01:50 PM, Laurent Pinchart wrote:
> >Hi Russell,
> >
> >On Wednesday 19 Oct 2016 10:35:21 Russell King - ARM Linux wrote:
> >>On Wed, Oct 19, 2016 at 12:19:30PM +0300, Laurent Pinchart wrote:
> >>>On Wednesday 19 Oct 2016 10:11:22 Russell King - ARM Linux wrote:
> >>>>In any case, I don't agree with converting it to a DRM bridge - that
> >>>>will mean that we have to split the driver into two pieces, the bridge
> >>>>part handling the mode set specifics, and a connector/encoder part which
> >>>>handles the detection/edid stuff.
> >>>>
> >>>>We might as well keep the whole thing as the classical connector/encoder
> >>>>rather than introducing this additional layer of complexity.
> >>>
> >>>We have different ways to instantiate external HDMI encoders, and that's
> >>>not good. I believe everybody agrees that drm encoder slave has to go, so
> >>>that's already one problem solved (or rather solvable, it still requires
> >>>someone to do the work). We will then be left with two different methods,
> >>>drm bridge and non-bridge component-based instantiation. We need to
> >>>somehow merge the two, and I'm open to discussions on how the end result
> >>>should look like.
> >>
> >>I think you're idea really doesn't work - and I think your idea that
> >>component-based is somehow separate from other methods is wrong.
> >>
> >>Look at iMX for example - even converting all the code that could be
> >>a bridge to be a bridge will not get rid of its use of the component
> >>instantiation, because you still have other components that need to
> >>come from elsewhere.  The same is true of armada as well.
> >
> >Don't get me wrong, I'm certainly not against the component framework itself.
> >A way to bind multiple independent devices together is needed. We have a
> >similar framework in V4L2 called v4l2-async, and I'd like to see it moved to
> >the component framework at some point to unify code. Some changes to the
> >component framework might be needed to support needs of V4L2 that are
> >currently not applicable to DRM/KMS, but there's nothing major there.
> >
> >>Moreover, as I've already said, converting tda998x to a DRM bridge
> >>will not get rid of the encoder/connector part, because it _is_ a
> >>connector in the DRM sense - it provides the detection and EDID
> >>reading.
> >>
> >>So, what would we achieve by splitting the driver into a DRM bridge
> >>and DRM encoder/connector?
> >
> >Please note that DRM bridge doesn't split the DRM connector out of the bridge,
> >bridges instantiate drm_connector objects. In that sense they don't differ
> >much from the model implemented by the tda998x driver.
> >
> >I however believe that connectors should be split out DRM bridge drivers, for
> >the simple reason that bridges can be chained. The output of a bridge isn't
> >guaranteed to be a connector but can be another bridge. We managed not to have
> >to deal with that in a generic way so far in mainline, but we'll run into the
> >problem one of these days, and a solution will be needed. There's no rush
> >right now, and this is unrelated to converting tda998x to DRM bridge.
> >
> >>We would still need the component helper to manage the binding of
> >>the connector part, which would also then have to register a DRM
> >>bridge in addition to a DRM encoder and providing the DRM connector
> >>as we can't have two device drivers bound to the same i2c device.
> >>What we get is more complexity in the driver.
> >
> >DRM bridges indeed don't create encoders. That task is left to the display
> >driver. The reason is the same as above: bridges can be chained (including
> >with an internal encoder that is not modelled as a bridge, and that's a case
> >we have today), while the KMS model exposes a single encoder to userspace.
> >Exposing DRM encoder objects as part of the KMS UABI was probably a mistake.
> >Better solutions would have been to expose no encoder at all or all encoders
> >in the chain. We are however stuck with this model as we can't break the UABI.
> >For that reason a DRM encoder object doesn't represent an encoder but a chain
> >of encoders. As a DRM bridge models a single encoder, the DRM encoder object
> >must be created at a higher level, in the display driver.
> 
> One more thing is that the TDA998x in its current form won't work
> with KMS drivers that create their own drm_encoder objects to represent
> their hardware's mipi DPI/RGB interfaces. For such drivers, we would
> want the TDA998x to tie itself to the existing encoder created by the
> KMS driver, rather than creating its own.

Please show _technically_ how this would work.  I want to see code or
pseudo-code illustrating how a "foreign" DRM encoder could be used with
either dw-hdmi or tda998x, because right now I can't see any way that
could work.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel