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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 20, 2016 at 11:20:05AM +0300, Laurent Pinchart wrote:
> Hi Russell,
> 
> On Wednesday 19 Oct 2016 10:35:21 Russell King - ARM Linux wrote:
> > 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.

So, we what you're saying is that we from a component based DRM encoder
plus DRM connector to a component based DRM bridge plus DRM connector
and some nebulous DRM encoder provider.

How does the DRM encoder get associated with the DRM connector?  DRM
requires the presence of at least one DRM encoder for each DRM connector,
and the DRM connector needs to provide a .best_encoder method to pick
the appropriate DRM encoder.

If (as you said elsewhere in your message) that the display driver is
responsible for providing the DRM encoder in your view, how does a
bridge DRM connector get to that DRM encoder, and how does it know
which DRM encoder to pick?

> 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.

You've not said how that could be achieved with TDA998x, so I'm still
opposed to the idea.  Remember, you're the one asking for this, you
must have a view on how to achieve it if you want it to happen.

> > 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.

If we're being precise, the "glue layer" creates the DRM encoder, but
the bridge code is responsible for binding itself to the DRM encoder,
and binding the DRM encoder to its associated DRM connector.

Let's assume it's a mistake.  Please illustrate how you would solve
this mistake.

> > 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.

I don't care about the name either, that's not what I'm asking here.

All I seem to be getting is some hand-waving "we want one model" and
"the current situation is a huge mess" (which is not a sentiment I agree
with) but with no technical response about how to achieve it.  Please do
the technical response bits and stop hand-waving.

-- 
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux