Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)

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

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

On 2013-12-13 05:45, Laurent Pinchart wrote:

>> I know drivers/video is in practice "fbdev", but drivers/video (the
>> words) sound like the best place for things related to video.
> 
> That's an option as well, but I'm not sure I like the idea of mixing fbdev and 
> generic video in a single directory. We could use a subdirectory of 
> drivers/video.

Right, that's what I meant.

>> I also don't supply the same data for both endpoints, when the data is about
>> the link. E.g. I think the V4L2 binding doc suggests to supply things like
>> bus-width for both endpoints. I only supply the data for the endpoint that
>> uses that data. With some encoders/panels the same data could be needed on
>> both ends, but not in these patches.
> 
> How do you handle the situation where the two drivers (for each end of the 
> link) need to know the bus width for instance ?

Then I fill the bus-width for both ends, as shown in the V4L2 bindings.
That's what I mean with "I only supply the data for the endpoint that
uses that data". If both ends use the data, I supply it for both.

>> The most important thing in the DSS DT bindings for now is that they should
>> contain enough information so that any future DT bindings changes can be
>> handled with a boot-time conversion code.
> 
> I'm afraid I can't give you any guarantee here. The bindings will be unstable 
> for some time, and we'll have to deal with that somehow.

I'm not asking for a guarantee. Just "yes, feels fine" =).

> The omapdss platform data structure contains the following fields
> 
> - get_context_loss_count: What is that used for ?

To find out if a HW block has been turned off and it has lost its
registers contents. It's an optimization, without it we need to restore
registers each time when runtime_resume() hook is called.

However, that's on its way out. I've already made new PM code for dispc
which doesn't use that. But I can't merge it before omapdrm has been
fixed to properly set things up when enabling a display.

> - num_devices, devices, default_device: What are those used for ?

Nothing anymore. They can be removed.

> - default_display_name: This should be easy to move to DT.

How? I handled it so that I assign the aliases for displays, and
display0 is always the default display. "default display name" is not
really a hw property =).

I think this should not be used for DT, and just use the display0 as
default (if we use aliases). If we don't use aliases, and instead use
the label properties as discussed in other thread, then there has to be
some other mechanism to tell which display should be used.

> - dsi_enable_pads, dsi_disable_pads: Those don't seem to be used in mainline. 
> What's their purpose, and how are they implemented on platforms that make use 
> of them ? Is the pinmux API an option ?

They are used in mainline, grep again =). They set pinmuxing for DSI.
Pinmux API is an option, but I think it would require a new driver. The
DSI pins for DSI1 and DSI2 are configured in a single register, where
the fields go in seemingly random order with varying widths.
pinmux-single cannot be used.

> - set_min_bus_tput: We have the same problem for the OMAP3 ISP, so a generic 
> solution is needed here.
> 
> - version: Given that we detect the DSS revision based on the SoC revision, 
> I'm tempted to either explicitly encode the DSS revision in the compatible 
> string, or let the DSS driver query the SoC revision somehow. The later is 
> considered as a layering violation, but let's face it, I can't see the DSS 
> being used on a non-OMAP platform.

I also would just use the compatible string, but we need to separate
between different OMAP ES versions. Doing that would mean we'd need
different .dtsi and .dts files for the different OMAP ES versions. It
would be a mess.

I have a TODO item to study this. There are only a few things that are
problematic (changed between ES versions without changing DSS HW
revision). If I can find a way around those, the compatible string
should be enough.

One example is the width of a blanking interval. Newer OMAP3 revisions
have a slightly wider field. I didn't try, but maybe I can write 1-bits
to the register, then read it, and if only part of those 1 bits "stick",
I know it's an old revision.

Anyway, I think this platform data thing is not strictly related. None
of these items affect the DT data (except the pinmuxing, but I think
it's fine to add that later). So while the above issues need to be fixed
at some point, I think they don't affect this series (well, the default
display is there which may affect).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]