Re: [PATCH] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

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

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

On Tue, 2012-07-31 at 14:27 +0530, Jassi Brar wrote:
> On 31 July 2012 14:12, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Tue, 2012-07-31 at 13:57 +0530, Jassi Brar wrote:
> >> On 31 July 2012 13:44, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > On Tue, 2012-07-31 at 13:33 +0530, Jassi Brar wrote:
> >> >> On 31 July 2012 13:21, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> >>
> >> >> > 2) Have the configuration for countless panels specified in the DT data
> >> >> >
> >> >> Why should a DT blob for a board contain more than 1 panel configuration?
> >> >
> >> > I meant the DT data generally, for all boards.
> >> >
> >> If you mean : Why have the configuration (those 15 integers) of the
> >> panel on a board specified in board.dtb?
> >> Well, that is an important purpose of DT - moving board specific
> >> parameters, on which a generic code works, out of kernel (I am
> >> refraining from preaching the goodness of that).
> >
> > Sure. But panel's unconfigurable properties are not board specific
> > parameters, they are panel's internal stuff. It doesn't matter to which
> > board I attach Acme Foo-123 panel, the panel timings are still the same.
> >
> It's not about the panel, it's about the board. For the generic driver
> in the kernel , the 'panel' is just a set of 15 integer values.
> There's no "Acme Foo-123" or "Acme Bar-123".  In fact, the _only_
> purpose of the panel's name string in the driver is to pick the
> correct set of "15 integers". With DT, the name string would be
> unnecessary.

Yes, the panel's name is used to "probe" the correct config. If we had
panels that could be asked "which panel are you" we could use that, but
with dummy panels we need to manually give the identifier (name) so that
the driver can do the probe.

> Consider two panels "ABC_123" and "XYZ_321" having identical
> parameters but different internals.
> Would you have duplicate elements in the generic_dpi_panels[] array ?
> Because the 'panels' are different.
> Or would you simply assume the XYZ_Board has the panel 'ABC_123'?
> Because after all it's the parameters that matter.

I would duplicate the elements. Or, if we have lots of panels having the
exact same parameters, we could have an array of names instead of just a
name.

> In short, we should see a 'panel' simply as a set of 15 integers.

Ok, I see. You mean that the 15 integers define the panel, so, in a
sense, the 15 integers is the name/identifier for the panel.

It would technically work, of course. But I do disagree with it:

1) I still don't see why you say it's board related. The properties in
question are properties of the panel, told in the panel specs, and
programmed when using the panel. No matter where the panel is used, the
same properties should be used.

2) As I see it, describing non-configurable device hardware properties
in the DT data is the wrong way. The driver should either probe the
properties or an ID from the device, or the ID of the device should be
given to the driver (a bit like what can be done with i2c).

3) Moving the data to DT would make any future changes more difficult.
Say, we could (probably should) add some regulator handling to the
driver, because usually panels need power to operate. Currently we just
presume the powers are always on.

Adding this is easy with the current approach. Adding it if the data is
in DT would be difficult, if not impossible, as all the board out there
could already be using the old DT format which doesn't have the
regulator data. Even in the best case all the boards out there would not
be able to use the regulator stuff.


Academic issues aside, what is the issue with the current approach in
practice? How would the DT approach make it better? Both approaches work
just fine, afaik. The current approach requires some maintenance from
me, but that's rather minor.

Anyway, even if I don't see the point, I'm not strictly against your
approach. If everybody thinks it's much better, it's fine for me. We're
currently discussing a platform agnostic approach to panel drivers with
some other SoC guys, I'll raise this question.

However, I don't want to apply the patch to move the properties to board
files. We're gonna move to DT anyway, and that patch would be just extra
stuff.

 Tomi


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