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

Hi Tomi,

On Friday 13 December 2013 12:05:20 Tomi Valkeinen wrote:

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

quoted

quoted

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.

quoted

quoted

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.

That sounds good to me.

quoted

quoted

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" =).

quoted

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.

OK.

quoted

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

Nothing anymore. They can be removed.

quoted

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

Another option would be to add a phandle that references the default display. 
I don't have a strong preference at the moment.

quoted

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

The only implementations I can find in arch/arm/mach-omap2/display.c are

static int omap_dsi_enable_pads(int dsi_id, unsigned lane_mask)
{
        return 0;
}

static void omap_dsi_disable_pads(int dsi_id, unsigned lane_mask)
{
}

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.

Or, as Archit mentioned, we could pass the SYSCONF registers as resources. 
That might not be very clean though.

quoted

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

-- 
Regards,

Laurent Pinchart