Re: [RFC v2 0/5] Common Display Framework

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Friday 28 December 2012 01:04:04 Sascha Hauer wrote:
> On Thu, Dec 27, 2012 at 01:57:56PM -0600, Rob Clark wrote:
> > On Thu, Dec 27, 2012 at 1:18 PM, Sascha Hauer wrote:
> > > On Thu, Dec 27, 2012 at 09:54:55AM -0600, Rob Clark wrote:
> > >> On Mon, Dec 24, 2012 at 7:37 AM, Laurent Pinchart
> > > 
> > > This implies that the master driver knows all potential subdevices,
> > > something which is not true for SoCs which have external i2c encoders
> > > attached to unrelated i2c controllers.
> > 
> > well, it can be brute-forced..  ie. drm driver calls common
> > register_all_panels() fxn, which, well, registers all the
> > panel/display subdev's based on their corresponding CONFIG_FOO_PANEL
> > defines.  If you anyways aren't building the panels as separate
> > modules, that would work.  Maybe not the most *elegant* approach, but
> > simple and functional.
> > 
> > I guess it partly depends on the structure in devicetree.  If you are
> > 
> > assuming that the i2c encoder belongs inside the i2c bus, like:
> >   &i2cN {
> >   
> >     foo-i2c-encoder {
> >     
> >       ....
> >     
> >     };
> >   
> >   };
> > 
> > and you are letting devicetree create the devices, then it doesn't
> > quite work.  I'm not entirely convinced you should do it that way.
> > Really any device like that is going to be hooked up to at least a
> > couple busses..  i2c, some sort of bus carrying pixel data, maybe some
> > gpio's, etc.  So maybe makes more sense for a virtual drm/kms bus, and
> > then use phandle stuff to link it to the various other busses it
> > 
> > needs:
> >   mydrmdev {
> >     foo-i2c-encoder {
> >        i2c = <&i2cN>;
> >        gpio = <&gpioM 2 3>
> >        ...
> >     };
> >   };
> 
> This seems to shift initialization order problem to another place. Here we
> have to make sure the controller is initialized before the drm driver. Same
> with suspend/resume.
> 
> It's not only i2c devices, also platform devices. On i.MX for example we
> have a hdmi transmitter which is somewhere on the physical address space.
> 
> I think grouping the different units together in a devicetree blob because
> we think they might form a logical virtual device is not going to work. It
> might make it easier from a drm perspective, but I think doing this will
> make for a lot of special cases. What will happen for example if you have
> two encoder devices in a row to configure? The foo-i2c-encoder would then
> get another child node.
> 
> Right now the devicetree is strictly ordered by (control-, not data-) bus
> topology. Linux has great helper code to support this model. Giving up this
> help to brute force a different topology and then trying to fit the result
> back into the Linux Bus hierarchy doesn't sound like a good idea to me.

I agree. The Linux device model is architectured around a control bus based 
tree, I don't want to change that. With devices hooked up on several busses we 
will have dependency issues anyway, regardless of how we describe them in DT. 
If we hook up the nodes from a data bus perspective we will run into control 
bus dependency issues. It's thus better in my opinion to keep the classic 
control bus based model and solve the data bus dependency issues.

> > ok, admittedly that is a bit different from other proposals about how
> > this all fits in devicetree.. but otoh, I'm not a huge believer in
> > letting something that is supposed to make life easier (DT), actually
> > make things harder or more complicated.  Plus this CDF stuff all needs
> > to also work on platforms not using OF/DT.
> 
> Right, but every other platform I know of is also described by its bus
> topology, be it platform device based or PCI or maybe even USB based.
> 
> CDF has to solve the same problem as ASoC and soc-camera: subdevices for
> a virtual device can come from many different corners of the system. BTW
> one example for a i2c encoder would be the SiI9022 which could not only
> be part of a drm device, but also of an ASoC device.

-- 
Regards,

Laurent Pinchart