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.
Thanks,
Archit
We can see this with what happened to the DW-HDMI driver - that still
needs the component helper, and it provides a DRM bridge, DRM encoder
and DRM connector parts.
To be precise, the DW-HDMI driver core doesn't create a DRM encoder, it's the
glue layers implemented as part of the Rockchip and iMX display drivers that
do. Nonetheless, that's a mistake, the encoder should be created by the
display drivers.
The only reason it made sense to split the DW-HDMI driver was due to the
differences between the Rockchip and Freescale implementations. Such
differences do not exist for TDA998x. So even here, your idea that "DRM
bridge" vs "non-DRM bridge component based" doesn't work - we have "DRM
bridge component based" because of the problem that I'm illustrating here.
So, again, I ask: what's the point of needlessly splitting the code
between an encoder/connector and a bridge?
We need to standardize on one model. I don't care much about how the end
result is named, as long as it fulfils the task at hand. For the reasons
explained above, it should not create a DRM encoder or DRM connector. A step
by step approach is obviously needed to get there, one option being moving all
external encoders to DRM bridge as a first step without instantiating the DRM
encoder in the bridge driver, and then moving connector instantiation out as a
second step.
The current situation is a huge mess, we simply can't pretend to ignore the
problem.
You seem to be forcing the DRM bridge model onto drivers with no
regard for its appropriateness for those drivers. If it pushes
more complexity into drivers, the model is wrong.
I expect several (many? most?) display drivers to handle bridges, encoders and
connectors in the same way, so we should obviously share common code in the
form of helper functions to keep display drivers simple.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
