Hi Tomi,
On Friday 13 December 2013 12:05:20 Tomi Valkeinen wrote:
> 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.
That sounds good to me.
> >> 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.
OK.
> > - 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.
Another option would be to add a phandle that references the default display.
I don't have a strong preference at the moment.
> > - 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.
> > - 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
Attachment:
signature.asc
Description: This is a digitally signed message part.
