Re: [PATCH v2 09/23] OMAPDSS: Create links between managers, outputs and devices

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 4671 bytes --]

On Fri, 2012-08-31 at 19:54 +0530, Archit Taneja wrote:
> On Friday 31 August 2012 07:40 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
> >> Links between DSS entities are made in dss_recheck_connections when a new panel
> >> is probed. Rewrite the code in dss_recheck_connections to link managers to
> >> outputs, and outputs to devices.
> >>
> >> The fields in omap_dss_device struct gives information on which output and
> >> manager to connect to. The desired manager and output pointers are retrieved and
> >> prepared(existing outputs/devices unset, if default display)) to form the
> >> desired links. The output is linked to the device, and then the manager to the
> >> output. If a probed device's required manager isn't free, the required output
> >> is still connected to the device so that it's easier to use the panel in the
> >> future.
> >
> > I've been pondering this one, and I can't come to a conclusion. The
> > recheck_connections is just so ugly that I'd like to get rid of it =). I
> > guess this patch doesn't make it any more ugly, so we can include it in
> > the patch series.
> >
> > And as I mentioned earlier, I think we should get rid of the
> > OMAP_DISPLAY_TYPE_* enum, as it's kinda extra now. But doing that would
> > require changing all board files. That's not out of the question, but I
> > think we should first make sure we know what we are doing with the board
> > files so we don't go back and forth there.
> 
> Yes, I didn't remove it for the same reason.
> 
> >
> > Just gathering my thoughts:
> >
> > When we define a panel in DT/board file, we should also define the
> > output instance, because that's part of the hardware design. But there
> 
> It's a part of hardware design if the panel can't be detached. But yes, 
> even for detachable panels, we can assume that the panel would 
> eventually connect to that output.
> 
> > are no hardcoded links between mgrs and outputs, so that doesn't belong
> > to the DT/board file. For example, omap4460's DSI1 can take input from
> > LCD1 or LCD2.
> 
> Right, so we don't need an equivalent of the dssdev->channel field in DT 
> info. As you said, we need the output instance, is that why you were 

What I'm planning for DT is a direct link to the output instance.
Something like this (not correct, just to give the idea):

dss {
	dpi {
	};
};

i2c {
	/* an i2c controlled panel */
	my-panel {
		video-bus = <&dpi>;
	};
};

Then my-panel can get the handle from video-bus, and it'll get the
output device directly, without need for any IDs or such.

> sceptical about it being defined as an enum in previous patches? 
> Probably we could define output instances in DT as a pair of instance 
> number and instance type {number, type}. It would be sort of hard to 
> play around with those within OMAPDSS though.

Well, optimally we wouldn't need to know about display types or output
instances, at least in most of the cases. We'd just have a bunch of
managers, and a bunch of outputs, and rules how these can be connected.
And the panel devices would have a link to the output it's connect in HW
level.

There are probably some cases where we need some kind of ID, so that we
can handle the DSI PLL's and such. And we need to setup the rules above
somewhere, and perhaps that code needs IDs to set it up. I'm not sure.

And, well, if we don't have IDs, the rules above need to be some kind of
lists of pointers. Perhaps having a bit mask is just simpler, as we'll
never have too many output instances.

So I'm not really against having the enum. It just would've been neat to
have the output type and instance number encoded into this enum, so that
it'd be easy to extract either one. But that kinda ruins the possibility
to use it in a bit mask.

> > So who could/should make the decision which mgr to use... Well, I don't
> > know on what basis the decision can be made, but I still think omapdss
> > can't make good decisions on that, so probably the whole "chain" should
> > be configured in the omapfb/omapdrm level.
> 
> Which manager to chose could be simply picking up the next free manager 
> which can connect to that output. omapfb/drm can take care of that.

That's a good default implementation, but not quite perfect. If there
are two displays, often the other display (well, output) can only be
connected to one particular manager, whereas the other one could use two
managers. And if both try to use the same manager, especially if the one
with two options is selected first, the other one is left without a mgr.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]