Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Geert Uytterhoeven @ 2013-12-13  8:16 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomi Valkeinen,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Fbdev development list,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1703598.NbqXAMAFOI@avalon>

On Fri, Dec 13, 2013 at 4:45 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > That's great, and I fully support that. This also calls for refactoring
>> > the V4L2 DT bindings and support code to share them with display devices.
>> > A bikeshedding question is where to put the common code.
>>
>> Thanks very much for review!
>>
>> 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.

Or reshuffle the various graphics/video/fb/console directories (more
bikeshedding). With git it's not that painful.

Frame buffer devices ended up in drivers/video because at that time, graphics
cards were called video cards. Moving video only entered the picture later.

drivers/fb/ (currently most of drivers/video/)
drivers/console/  (currently drivers/video/console/)
...

Or should fb be under gpu?
What about drivers/media? Video and audio are multi-media, too...

Baah, bad idea... too much bikeshedding ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Tomi Valkeinen @ 2013-12-13  8:40 UTC (permalink / raw)
  To: Archit Taneja, Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <52AAC615.504@ti.com>

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

On 2013-12-13 10:32, Archit Taneja wrote:
> On Thursday 12 December 2013 01:00 PM, Tomi Valkeinen wrote:
>> On 2013-12-12 01:10, Laurent Pinchart wrote:
>>> Hi Tomi,
>>>
>>> On Wednesday 04 December 2013 14:28:32 Tomi Valkeinen wrote:
>>>> omapdss driver uses a omapdss platform device to pass platform specific
>>>> function pointers and DSS hardware version from the arch code to the
>>>> driver. This device is needed also when booting with DT.
>>>>
>>>> This patch adds omapdss_init_of() function, called from
>>>> board-generic at
>>>> init time, which creates the omapdss device.
>>>
>>> Is this a temporary solution that you plan to later replace with DT-only
>>> device instantiation ?
>>
>> It's a long term task to remove the "virtual" omapdss device. Removing
>> the platform data that we pass has been very difficult.
> 
> Even if we remove the platform data, what would be the right place to
> register the drm, fb and vout devices? We need to make sure their
> drivers are probed only after omapdss is initialised.

That's the same issue as we have already, so removing omapdss doesn't
affect that.

I don't think we have any good way to ensure those devices are not
probed before omapdss. We just have to manage the case that omapdss has
not been probed yet, by checking if omapdss is there and if not, return
EPROBE_DEFER.

>> For example, we need to get the OMAP revision to know which OMAP DSS
>> hardware we have. We can't get that information from the DSS hardware
>> (thank you, HW designers! ;).
>>
> 
>> Another is DSI pin muxing. I think we need a new pinmuxing driver for
>> that, or maybe change pinmux-single quite a bit. The DSI pinmuxing is
>> very simple, but the register fields are varying lengths and at varying
>> positions, so pinmux-single doesn't work for it.
> 
> I have seen other OMAP drivers passing control module register info to
> their DT node. Instead of having a pinmux driver for a single register,
> we might want to consider just passing the control module register, and
> take care of the reg fields and masks in the OMAP DSI driver itself.

Yes, that's also one option. Quite ugly, though =).

 Tomi



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

^ permalink raw reply

* Re: [PATCH 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Archit Taneja @ 2013-12-13  8:44 UTC (permalink / raw)
  To: Tomi Valkeinen, Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <52A96603.5030709@ti.com>

On Thursday 12 December 2013 01:00 PM, Tomi Valkeinen wrote:
> On 2013-12-12 01:10, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> On Wednesday 04 December 2013 14:28:32 Tomi Valkeinen wrote:
>>> omapdss driver uses a omapdss platform device to pass platform specific
>>> function pointers and DSS hardware version from the arch code to the
>>> driver. This device is needed also when booting with DT.
>>>
>>> This patch adds omapdss_init_of() function, called from board-generic at
>>> init time, which creates the omapdss device.
>>
>> Is this a temporary solution that you plan to later replace with DT-only
>> device instantiation ?
>
> It's a long term task to remove the "virtual" omapdss device. Removing
> the platform data that we pass has been very difficult.

Even if we remove the platform data, what would be the right place to 
register the drm, fb and vout devices? We need to make sure their 
drivers are probed only after omapdss is initialised.

>
> For example, we need to get the OMAP revision to know which OMAP DSS
> hardware we have. We can't get that information from the DSS hardware
> (thank you, HW designers! ;).
>

> Another is DSI pin muxing. I think we need a new pinmuxing driver for
> that, or maybe change pinmux-single quite a bit. The DSI pinmuxing is
> very simple, but the register fields are varying lengths and at varying
> positions, so pinmux-single doesn't work for it.

I have seen other OMAP drivers passing control module register info to 
their DT node. Instead of having a pinmux driver for a single register, 
we might want to consider just passing the control module register, and 
take care of the reg fields and masks in the OMAP DSI driver itself.

The parsing of the DSI pins from DT, however, can be kept generic.

Archit


^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tomi Valkeinen @ 2013-12-13  9:29 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tony Lindgren, linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge
In-Reply-To: <14299863.f0rzOyPfdL@avalon>

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

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

> Right. The real question is whether the DSI or HDMI IPs can be used in a 
> system without the DSS core. If not, it might make sense to just merge the 
> drivers into a single module (of course with a clear interface between the 
> different parts to avoid spaghetti code).

The drivers are already in single kernel module, omapdss.ko.

The HDMI IP is used on another SoC, without DSS. They have their own
display architecture. DSI IP might need some small modifications to work
without DSS, but not much. It doesn't have any strict DSS/DISPC
dependencies.

> Given that the DSS core has a set of registers not dedicated to any of the 
> submodules I believe it should be represented by a device. The omapdss driver 
> thus doesn't look virtual to me, it supports a real piece of hardware.

As noted in another mail, dss_core and omapdss devices are different
things. dss_core is not virtual, omapdss is.

>> But then, I feel that they are quite independent and probably should be
>> separate devices.
> 
> Even if they're separate devices they could be instantiated by DSS core based 
> on DT nodes. I'm not sure whether that's the best idea, but it might be worth 
> thinking about it.

What would be the difference to the one in this series? In this series,
the submodules are instantiated automatically by the driver framework.
The only difference I see is that the submodule devices would
appear/disappear dynamically, but... what would be the benefit?

 Tomi



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

^ permalink raw reply

* Re: [PATCH 16/26] ARM: omap4-sdp.dts: add display information
From: Archit Taneja @ 2013-12-13  9:39 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree
  Cc: Darren Etheridge, Tony Lindgren
In-Reply-To: <1386160133-24026-17-git-send-email-tomi.valkeinen@ti.com>

On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   arch/arm/boot/dts/omap4-sdp.dts | 91 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 91 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 5fc3f43c5a81..e3048f849612 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -550,3 +550,94 @@
>   	mode = <3>;
>   	power = <50>;
>   };
> +
> +&dsi1 {
> +	vdds_dsi-supply = <&vcxio>;
> +
> +	dsi1_out_ep: endpoint {
> +		remote-endpoint = <&lcd0_in>;
> +		lanes = <0 1 2 3 4 5>;

In the previous revision omapdss DT patchset, the lanes node was a 
member of the panel DT node, and not the dsi DT node. Any reason to 
change this? Does it make more sense this way?

I suppose it's more suitable for dsi to hold the property if 2 panels 
are connected on the same bus. Say, one with 4 data lanes, and other 
with 2. It would be tricky for the dsi driver to get lane params from 2 
different places and merge them somehow.

> +	};
> +
> +	lcd0: display@0 {
> +		compatible = "tpo,taal", "panel-dsi-cm";
> +
> +		gpios = <&gpio4 6 0>;	/* 102, reset */
> +
> +		lcd0_in: endpoint {
> +			remote-endpoint = <&dsi1_out_ep>;
> +		};
> +	};

Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2 
respectively? I don't see this for panels on other boards.

Archit


^ permalink raw reply

* Re: [PATCH 16/26] ARM: omap4-sdp.dts: add display information
From: Tomi Valkeinen @ 2013-12-13  9:39 UTC (permalink / raw)
  To: Archit Taneja, linux-omap, linux-fbdev, devicetree
  Cc: Darren Etheridge, Tony Lindgren
In-Reply-To: <52AAD2F1.2040208@ti.com>

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

On 2013-12-13 11:27, Archit Taneja wrote:
> On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   arch/arm/boot/dts/omap4-sdp.dts | 91
>> +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 91 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts
>> b/arch/arm/boot/dts/omap4-sdp.dts
>> index 5fc3f43c5a81..e3048f849612 100644
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -550,3 +550,94 @@
>>       mode = <3>;
>>       power = <50>;
>>   };
>> +
>> +&dsi1 {
>> +    vdds_dsi-supply = <&vcxio>;
>> +
>> +    dsi1_out_ep: endpoint {
>> +        remote-endpoint = <&lcd0_in>;
>> +        lanes = <0 1 2 3 4 5>;
> 
> In the previous revision omapdss DT patchset, the lanes node was a
> member of the panel DT node, and not the dsi DT node. Any reason to
> change this? Does it make more sense this way?

Well, the lane configuration is programmed into the DSI HW. So DSI needs
to know them. Thus the lanes can be considered a property of the DSI.

In some cases, the external encoder or panel also needs to know about
the lanes. In that case, both DSI and the encoder/panel would contain
the same data.

My reasoning where a property belongs to:

If a property is clearly internal to a device, it belongs there. For
example, in this case vdds_dsi-supply is clearly a property of the DSI.

If a property is about the link between two devices, like "lanes" here,
it belongs to both devices. If a device does not need that data for
anything, it can be omitted.

> I suppose it's more suitable for dsi to hold the property if 2 panels
> are connected on the same bus. Say, one with 4 data lanes, and other
> with 2. It would be tricky for the dsi driver to get lane params from 2
> different places and merge them somehow.

It doesn't matter, both would work fine. If the lanes property is in the
DSI node, then the DSI driver finds out the lane config by finding out
which endpoint is used for the panel that's being enabled.

If the lanes property would be in the panel, the panel would pass the
lane config to the DSI when it's enabled.

But I think passing the lane config from panel to DSI (like we do
currently) is not so nice.

>> +    };
>> +
>> +    lcd0: display@0 {
>> +        compatible = "tpo,taal", "panel-dsi-cm";
>> +
>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>> +
>> +        lcd0_in: endpoint {
>> +            remote-endpoint = <&dsi1_out_ep>;
>> +        };
>> +    };
> 
> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
> respectively? I don't see this for panels on other boards.

Yes. The panels are _controlled_ with DSI. We model the child-parent
relationships in DT data based on the control. So an i2c peripheral is
controlled via i2c master, and is thus a child of the i2c master. Same
here. The ports/endpoints are used to define the data path, which is
separate thing from the control path.

DPI panels which don't have any way to control them (except basic things
like gpios) are platform devices without any parent.

If the DPI panel would be controlled with i2c, it'd be a child of an i2c
master.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 16/26] ARM: omap4-sdp.dts: add display information
From: Archit Taneja @ 2013-12-13  9:58 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree
  Cc: Darren Etheridge, Tony Lindgren
In-Reply-To: <52AAD5CE.3010609@ti.com>

On Friday 13 December 2013 03:09 PM, Tomi Valkeinen wrote:
> On 2013-12-13 11:27, Archit Taneja wrote:
>> On Wednesday 04 December 2013 05:58 PM, Tomi Valkeinen wrote:
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>    arch/arm/boot/dts/omap4-sdp.dts | 91
>>> +++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 91 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts
>>> b/arch/arm/boot/dts/omap4-sdp.dts
>>> index 5fc3f43c5a81..e3048f849612 100644
>>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>>> @@ -550,3 +550,94 @@
>>>        mode = <3>;
>>>        power = <50>;
>>>    };
>>> +
>>> +&dsi1 {
>>> +    vdds_dsi-supply = <&vcxio>;
>>> +
>>> +    dsi1_out_ep: endpoint {
>>> +        remote-endpoint = <&lcd0_in>;
>>> +        lanes = <0 1 2 3 4 5>;
>>
>> In the previous revision omapdss DT patchset, the lanes node was a
>> member of the panel DT node, and not the dsi DT node. Any reason to
>> change this? Does it make more sense this way?
>
> Well, the lane configuration is programmed into the DSI HW. So DSI needs
> to know them. Thus the lanes can be considered a property of the DSI.
>
> In some cases, the external encoder or panel also needs to know about
> the lanes. In that case, both DSI and the encoder/panel would contain
> the same data.
>
> My reasoning where a property belongs to:
>
> If a property is clearly internal to a device, it belongs there. For
> example, in this case vdds_dsi-supply is clearly a property of the DSI.
>
> If a property is about the link between two devices, like "lanes" here,
> it belongs to both devices. If a device does not need that data for
> anything, it can be omitted.
>
>> I suppose it's more suitable for dsi to hold the property if 2 panels
>> are connected on the same bus. Say, one with 4 data lanes, and other
>> with 2. It would be tricky for the dsi driver to get lane params from 2
>> different places and merge them somehow.
>
> It doesn't matter, both would work fine. If the lanes property is in the
> DSI node, then the DSI driver finds out the lane config by finding out
> which endpoint is used for the panel that's being enabled.
>
> If the lanes property would be in the panel, the panel would pass the
> lane config to the DSI when it's enabled.
>
> But I think passing the lane config from panel to DSI (like we do
> currently) is not so nice.

Okay, that makes sense.

>
>>> +    };
>>> +
>>> +    lcd0: display@0 {
>>> +        compatible = "tpo,taal", "panel-dsi-cm";
>>> +
>>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>>> +
>>> +        lcd0_in: endpoint {
>>> +            remote-endpoint = <&dsi1_out_ep>;
>>> +        };
>>> +    };
>>
>> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
>> respectively? I don't see this for panels on other boards.
>
> Yes. The panels are _controlled_ with DSI. We model the child-parent
> relationships in DT data based on the control. So an i2c peripheral is
> controlled via i2c master, and is thus a child of the i2c master. Same
> here. The ports/endpoints are used to define the data path, which is
> separate thing from the control path.
>
> DPI panels which don't have any way to control them (except basic things
> like gpios) are platform devices without any parent.
>
> If the DPI panel would be controlled with i2c, it'd be a child of an i2c
> master.

Ah, I thought the port/endpoint stuff had something to do with this. I 
forgot about the parent-child relationship for the control path.

In that case, for the sake of accuracy, the dsi-cm panel could get the 
"in" parameter via the parent node wherever it's used for control, like 
setting a DCS command for sleep out. And via 
omapdss_of_find_source_for_first_ep() when it's used to start data 
transfer, even though both the "in's" are finally the same dsi device?

Archit


^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-13 10:05 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1703598.NbqXAMAFOI@avalon>

[-- 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 --]

^ permalink raw reply

* Re: [PATCH 16/26] ARM: omap4-sdp.dts: add display information
From: Tomi Valkeinen @ 2013-12-13 10:15 UTC (permalink / raw)
  To: Archit Taneja, linux-omap, linux-fbdev, devicetree
  Cc: Darren Etheridge, Tony Lindgren
In-Reply-To: <52AADA55.20309@ti.com>

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

On 2013-12-13 11:58, Archit Taneja wrote:

>>>> +    };
>>>> +
>>>> +    lcd0: display@0 {
>>>> +        compatible = "tpo,taal", "panel-dsi-cm";
>>>> +
>>>> +        gpios = <&gpio4 6 0>;    /* 102, reset */
>>>> +
>>>> +        lcd0_in: endpoint {
>>>> +            remote-endpoint = <&dsi1_out_ep>;
>>>> +        };
>>>> +    };
>>>
>>> Is there a reason why lcd0 and lcd1 are children nodes of dsi1 and dsi2
>>> respectively? I don't see this for panels on other boards.
>>
>> Yes. The panels are _controlled_ with DSI. We model the child-parent
>> relationships in DT data based on the control. So an i2c peripheral is
>> controlled via i2c master, and is thus a child of the i2c master. Same
>> here. The ports/endpoints are used to define the data path, which is
>> separate thing from the control path.
>>
>> DPI panels which don't have any way to control them (except basic things
>> like gpios) are platform devices without any parent.
>>
>> If the DPI panel would be controlled with i2c, it'd be a child of an i2c
>> master.
> 
> Ah, I thought the port/endpoint stuff had something to do with this. I
> forgot about the parent-child relationship for the control path.
> 
> In that case, for the sake of accuracy, the dsi-cm panel could get the
> "in" parameter via the parent node wherever it's used for control, like
> setting a DCS command for sleep out. And via
> omapdss_of_find_source_for_first_ep() when it's used to start data
> transfer, even though both the "in's" are finally the same dsi device?

Don't mix the DT data and the current driver =). The current driver does
not handle things properly. The driver still uses the current model,
where we don't have separate control and data path handling. I.e. both
control and data are handled via the single API, using the "in" field.

The important thing with this DT data is that in the future we can
change the driver model, if we so want, to manage data and control
separately. Or maybe add a DSI bus, as has been proposed elsewhere.

It's true that we could change the driver as you describe, but I don't
think it helps anything, as the current model in the driver only has a
single control-data path.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tomi Valkeinen @ 2013-12-13 10:18 UTC (permalink / raw)
  To: Laurent Pinchart, Tony Lindgren
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge
In-Reply-To: <1933495.zHe4JMYZyB@avalon>

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

On 2013-12-13 05:27, Laurent Pinchart wrote:
> Hi Tony,
> 
> On Thursday 12 December 2013 21:59:13 Tony Lindgren wrote:
>> On Thu, Dec 12, 2013 at 10:38:34AM +0200, Tomi Valkeinen wrote:
>>> On 2013-12-12 01:44, Laurent Pinchart wrote:
>>>
>>> So, are they independent? I don't know =). I think they lean on the
>>> independent side. dss_core is always needed for the submodules to work,
>>> but for example DSI could be used without DISPC, using system DMA to
>>> transfer data from memory to DSI. Not a very useful thing to do, but
>>> still, there are dedicated DMA channels for that.
>>
>> If they have separate hwmod entries, they should be considered separate
>> independent devices for sure.
>>
>> To summarize, here are few reasons why they need to be treated as
>> separate devices:
> 
> Are you talking generally here, or about the DSS modules in particular ?
> 
>> 1. The modules maybe clocked/powered/idled separately and can have their
>>    own idle configuration so they can do the hardware based idling
>>    separately.
> 
> I don't think this applies to the DSS modules.

The DSS submodules have their own SYSCONFIG register, and idle settings
can be set per module. So I think they idle separately, even if they are
in a common power domain.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 06/26] OMAPDSS: if dssdev->name==NULL, use alias
From: Tomi Valkeinen @ 2013-12-13 12:01 UTC (permalink / raw)
  To: Sebastian Reichel, Laurent Pinchart
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Florian Vaussard
In-Reply-To: <52A9C470.2030102@ti.com>

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

On 2013-12-12 16:13, Tomi Valkeinen wrote:
> On 2013-12-12 12:05, Sebastian Reichel wrote:
>> On Thu, Dec 12, 2013 at 09:41:49AM +0200, Tomi Valkeinen wrote:
>>>> A label property is still an option.
>>>
>>> Hmm, what do you mean? Label as in:
>>>
>>> foo : node {
>>> };
>>>
>>> Isn't that 'foo' label only visible in DT itself, as a shortcut?
>>
>> Some driver use a "label" property like this:
>>
>> foo : node {
>>     label = "lcd";
>>
>>     ...
>> };
>>
>> See for example
>>
>> Documentation/devicetree/bindings/leds/common.txt
>> Documentation/devicetree/bindings/mtd/partition.txt
> 
> Ah, I see. That kind of label was actually the first thing I did when
> starting to work on DSS DT. But I removed it, as it didn't describe the
> hardware and I didn't see others using anything similar.
> 
> But I guess one could argue it does describe hardware, not in electrical
> level but in conceptual level.
> 
> The question is, do we need labeling for displays? For backward
> compatibility omapdss would need it, but in general? I'm quite content
> with having just display0, display1 etc. Using the alias node, those can
> be fixed and display0 is always the same display.

I came to the conclusion that it's better to add the label to keep
backward compatibility, especially as it was very easy to add.

So we'll have 'name' and 'alias' for each display (as we have already).

In the current non-DT boot (which is going away):

- 'alias' is created by omapdss dynamically (first display to be
registered is display0, etc.)
- 'name' comes from platform data

In the DT boot:

- 'alias' comes from the DT aliases node, or if there are no display
aliases, it's created the same way as to non-DT. The code presumes that
there either is a DT alias for all displays, or there are none.
- 'name' comes from 'label' property

In both non-DT and DT cases, if 'name' is NULL (i.e. not set in platform
data or no 'label' property), the alias is used as a name.

I think this works fine, and was a trivial change.

 Tomi



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

^ permalink raw reply

* Re: [patch 1/2] video: mmp: delete a stray mutex_unlock()
From: Dan Carpenter @ 2013-12-13 13:11 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20131114081918.GB8150@elgon.mountain>

Ping.

regards,
dan carpenter

On Thu, Nov 14, 2013 at 11:19:18AM +0300, Dan Carpenter wrote:
> We aren't holding the disp_lock so we shouldn't release it.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/video/mmp/core.c b/drivers/video/mmp/core.c
> index 84de263..c8d4265 100644
> --- a/drivers/video/mmp/core.c
> +++ b/drivers/video/mmp/core.c
> @@ -173,7 +173,7 @@ struct mmp_path *mmp_register_path(struct mmp_path_info *info)
>  		+ sizeof(struct mmp_overlay) * info->overlay_num;
>  	path = kzalloc(size, GFP_KERNEL);
>  	if (!path)
> -		goto failed;
> +		return NULL;
>  
>  	/* path set */
>  	mutex_init(&path->access_ok);
> @@ -219,11 +219,6 @@ struct mmp_path *mmp_register_path(struct mmp_path_info *info)
>  
>  	mutex_unlock(&disp_lock);
>  	return path;
> -
> -failed:
> -	kfree(path);
> -	mutex_unlock(&disp_lock);
> -	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(mmp_register_path);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Laurent Pinchart @ 2013-12-13 14:37 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Tony Lindgren, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <52AADBE0.3030203@ti.com>

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

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 #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-13 15:47 UTC (permalink / raw)
  To: Laurent Pinchart, Tony Lindgren
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge, Stefan Roese, Sebastian Reichel, Robert Nelson,
	Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <3426377.keoaFgiZkl@avalon>

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

Hi Laurent, Tony,

On 2013-12-13 16:37, Laurent Pinchart wrote:

>>> - 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)
> {
> }

Yep. It seems Tony removed the muxing for -rc2 in
e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 ARM: OMAP2+: Remove legacy mux
code for display.c

Tony, that patch removes DSI muxing, which is not done via DT. I can't
test right now, but I presume DSI displays don't work at all after -rc2.

 Tomi



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

^ permalink raw reply

* Re: [PATCH 05/26] ARM: OMAP2+: add omapdss_init_of()
From: Tony Lindgren @ 2013-12-13 17:07 UTC (permalink / raw)
  To: Archit Taneja
  Cc: Tomi Valkeinen, Laurent Pinchart, linux-omap, linux-fbdev,
	devicetree, Darren Etheridge
In-Reply-To: <52AAC615.504@ti.com>

* Archit Taneja <archit@ti.com> [131213 00:33]:
> 
> I have seen other OMAP drivers passing control module register info
> to their DT node. Instead of having a pinmux driver for a single
> register, we might want to consider just passing the control module
> register, and take care of the reg fields and masks in the OMAP DSI
> driver itself.

See the PBIAS control module patches and their comments from Balaji.

We can map the misc registers of SCM using drivers/mfd/syscon.c and
that way drivers can access them via syscon.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tony Lindgren @ 2013-12-13 17:10 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Archit Taneja,
	Darren Etheridge
In-Reply-To: <52AADEF3.9040808-l0cyMroinI0@public.gmane.org>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [131213 02:19]:
> On 2013-12-13 05:27, Laurent Pinchart wrote:
> > Hi Tony,
> > 
> > On Thursday 12 December 2013 21:59:13 Tony Lindgren wrote:
> >> On Thu, Dec 12, 2013 at 10:38:34AM +0200, Tomi Valkeinen wrote:
> >>> On 2013-12-12 01:44, Laurent Pinchart wrote:
> >>>
> >>> So, are they independent? I don't know =). I think they lean on the
> >>> independent side. dss_core is always needed for the submodules to work,
> >>> but for example DSI could be used without DISPC, using system DMA to
> >>> transfer data from memory to DSI. Not a very useful thing to do, but
> >>> still, there are dedicated DMA channels for that.
> >>
> >> If they have separate hwmod entries, they should be considered separate
> >> independent devices for sure.
> >>
> >> To summarize, here are few reasons why they need to be treated as
> >> separate devices:
> > 
> > Are you talking generally here, or about the DSS modules in particular ?
> > 
> >> 1. The modules maybe clocked/powered/idled separately and can have their
> >>    own idle configuration so they can do the hardware based idling
> >>    separately.
> > 
> > I don't think this applies to the DSS modules.
> 
> The DSS submodules have their own SYSCONFIG register, and idle settings
> can be set per module. So I think they idle separately, even if they are
> in a common power domain.

Yes. Please see the current omap_hwmod_*_data.c files, if they are separate
entries there, that means we need to treat them as separate devices to
avoid the issues I listed.

We do have some entries still missing from omap_hwmod_*_data.c files, like
the SSI entries as they are undocumented. But for the existing ones there
please follow the same layout for the .dts entries.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tony Lindgren @ 2013-12-13 17:22 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Laurent Pinchart, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <52AB2C25.6000507@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [131213 07:49]:
> Hi Laurent, Tony,
> 
> On 2013-12-13 16:37, Laurent Pinchart wrote:
> 
> >>> - 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)
> > {
> > }
> 
> Yep. It seems Tony removed the muxing for -rc2 in
> e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 ARM: OMAP2+: Remove legacy mux
> code for display.c
> 
> Tony, that patch removes DSI muxing, which is not done via DT. I can't
> test right now, but I presume DSI displays don't work at all after -rc2.

Hmm I suggest you test against commit adfe9361b236 (ARM: dts: Add basic
devices on am3517-evm) as it does not yet remove the legacy data and
that's what's heading to linux next soonish.

With the DT configured displays that muxing needs to be done in the
DSS driver(s) using pinctrl-single.

BTW, I suspect quite a few of the DSI using boards have been broken a
while before 0b2aa8bed3e1 (gpio: twl4030: Fix regression for twl gpio
output) as at least the following have been using TWL GPIO to enable
the panel:

board-3430sdp.c
board-devkit8000.c
board-ldp.c
board-omap3stalker.c

This was the case at least for LDP.

Regards,

Tony

^ permalink raw reply

* [PATCH 0/4] OMAPDSS: DT support for N900 panel
From: Sebastian Reichel @ 2013-12-13 18:17 UTC (permalink / raw)
  To: Sebastian Reichel, Tomi Valkeinen, Benoît Cousson,
	Tony Lindgren
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, linux-omap, linux-fbdev, devicetree,
	Sebastian Reichel

Hi,

This patchset adds DT support for the N900 panel. The patchset is based on
Tomi's work/dss-dt-2 branch [0]. I suggest to send the DT changes through
Benoits queue, since I have more N900 DT changes for 3.14. Also the patch
editing the rx51 boardcode can be dropped, since the file is removed in 3.14.
I included those two with this patchset, since they are needed to test the
other two patches.

I did not include a documentation for the DT API, since the omapdss
documentation is still missing.

I have successfully tested this on the N900.

[0] https://git.kernel.org/cgit/linux/kernel/git/tomba/linux.git/log/?h=work/dss-dt-2

-- Sebastian

Sebastian Reichel (4):
  OMAPDSS: Add DT support to SDI
  OMAPDSS: ACX565AKM: Add DT support
  ARM: OMAP: rx51: DT boot: disable legacy dss init
  ARM: dts: omap3-n900: Add display support

 arch/arm/boot/dts/omap3-n900.dts                   | 18 ++++++++++-
 arch/arm/mach-omap2/board-rx51-video.c             |  2 +-
 .../omap2/displays-new/panel-sony-acx565akm.c      | 35 +++++++++++++++++++++-
 drivers/video/omap2/dss/sdi.c                      | 20 +++++++++++++
 4 files changed, 72 insertions(+), 3 deletions(-)

-- 
1.8.5.1


^ permalink raw reply

* [PATCH 1/4] OMAPDSS: Add DT support to SDI
From: Sebastian Reichel @ 2013-12-13 18:17 UTC (permalink / raw)
  To: Sebastian Reichel, Tomi Valkeinen, Benoît Cousson,
	Tony Lindgren
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, linux-omap, linux-fbdev, devicetree,
	Sebastian Reichel
In-Reply-To: <1386958650-2404-1-git-send-email-sre@debian.org>

Add the code to make the SDI driver work with device tree on OMAP3.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index ccc569a..bdc6a68 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -26,6 +26,8 @@
 #include <linux/export.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 
 #include <video/omapdss.h>
 #include "dss.h"
@@ -333,6 +335,9 @@ static const struct omapdss_sdi_ops sdi_ops = {
 
 static void sdi_init_output(struct platform_device *pdev)
 {
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *ep;
+
 	struct omap_dss_device *out = &sdi.output;
 
 	out->dev = &pdev->dev;
@@ -344,6 +349,15 @@ static void sdi_init_output(struct platform_device *pdev)
 	out->owner = THIS_MODULE;
 
 	omapdss_register_output(out);
+
+	if (!pdev->dev.of_node)
+		return;
+
+	ep = omapdss_of_get_first_endpoint(node);
+	if (!ep)
+		return;
+
+	of_property_read_u32(ep, "data-lines", &sdi.datapairs);
 }
 
 static void __exit sdi_uninit_output(struct platform_device *pdev)
@@ -369,12 +383,18 @@ static int __exit omap_sdi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id sdi_of_match[] = {
+	{ .compatible = "ti,omap3-sdi" },
+	{},
+};
+
 static struct platform_driver omap_sdi_driver = {
 	.probe		= omap_sdi_probe,
 	.remove         = __exit_p(omap_sdi_remove),
 	.driver         = {
 		.name   = "omapdss_sdi",
 		.owner  = THIS_MODULE,
+		.of_match_table = sdi_of_match,
 	},
 };
 
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH 2/4] OMAPDSS: ACX565AKM: Add DT support
From: Sebastian Reichel @ 2013-12-13 18:17 UTC (permalink / raw)
  To: Sebastian Reichel, Tomi Valkeinen, Benoît Cousson,
	Tony Lindgren
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, linux-omap, linux-fbdev, devicetree,
	Sebastian Reichel
In-Reply-To: <1386958650-2404-1-git-send-email-sre@debian.org>

This adds DT support to the ACX565AKM panel driver.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 .../omap2/displays-new/panel-sony-acx565akm.c      | 35 +++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index d94f35d..942b8d4 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -30,6 +30,8 @@
 #include <linux/backlight.h>
 #include <linux/fb.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
@@ -531,7 +533,9 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
 	dev_dbg(&ddata->spi->dev, "%s\n", __func__);
 
 	in->ops.sdi->set_timings(in, &ddata->videomode);
-	in->ops.sdi->set_datapairs(in, ddata->datapairs);
+
+	if (ddata->datapairs >= 0)
+		in->ops.sdi->set_datapairs(in, ddata->datapairs);
 
 	r = in->ops.sdi->enable(in);
 	if (r) {
@@ -710,6 +714,30 @@ static int acx565akm_probe_pdata(struct spi_device *spi)
 	return 0;
 }
 
+static int acx565akm_probe_of(struct spi_device *spi)
+{
+	struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
+	struct device_node *np = spi->dev.of_node;
+	struct omap_dss_device *dssdev;
+	int ret;
+
+	ddata->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+
+	ddata->datapairs = -1;
+	ddata->in = omapdss_of_find_source_for_first_ep(np);
+	if (IS_ERR(ddata->in)) {
+		dev_err(&spi->dev, "failed to find video source\n");
+		return PTR_ERR(ddata->in);
+	}
+
+	dssdev = &ddata->dssdev;
+	ret = of_property_read_string(np, "label", &dssdev->name);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int acx565akm_probe(struct spi_device *spi)
 {
 	struct panel_drv_data *ddata;
@@ -737,7 +765,12 @@ static int acx565akm_probe(struct spi_device *spi)
 		r = acx565akm_probe_pdata(spi);
 		if (r)
 			return r;
+	} else if (spi->dev.of_node) {
+		r = acx565akm_probe_of(spi);
+		if (r)
+			return r;
 	} else {
+		dev_err(&spi->dev, "platform data missing!\n");
 		return -ENODEV;
 	}
 
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH 3/4] ARM: OMAP: rx51: DT boot: disable legacy dss init
From: Sebastian Reichel @ 2013-12-13 18:17 UTC (permalink / raw)
  To: Sebastian Reichel, Tomi Valkeinen, Benoît Cousson,
	Tony Lindgren
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, linux-omap, linux-fbdev, devicetree,
	Sebastian Reichel
In-Reply-To: <1386958650-2404-1-git-send-email-sre@debian.org>

This disables legacy initialization of the
omapdss, if the N900 is booted via DT.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 arch/arm/mach-omap2/board-rx51-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/board-rx51-video.c b/arch/arm/mach-omap2/board-rx51-video.c
index 43a90c8..9cfebc5 100644
--- a/arch/arm/mach-omap2/board-rx51-video.c
+++ b/arch/arm/mach-omap2/board-rx51-video.c
@@ -48,7 +48,7 @@ static struct omap_dss_board_info rx51_dss_board_info = {
 
 static int __init rx51_video_init(void)
 {
-	if (!machine_is_nokia_rx51() && !of_machine_is_compatible("nokia,omap3-n900"))
+	if (!machine_is_nokia_rx51())
 		return 0;
 
 	if (omap_mux_init_gpio(RX51_LCD_RESET_GPIO, OMAP_PIN_OUTPUT)) {
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH 4/4] ARM: dts: omap3-n900: Add display support
From: Sebastian Reichel @ 2013-12-13 18:17 UTC (permalink / raw)
  To: Sebastian Reichel, Tomi Valkeinen, Benoît Cousson,
	Tony Lindgren
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, linux-omap, linux-fbdev, devicetree,
	Sebastian Reichel
In-Reply-To: <1386958650-2404-1-git-send-email-sre@debian.org>

Add support for the Nokia N900's display.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 arch/arm/boot/dts/omap3-n900.dts | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index c2c306d..cf776f3 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -472,12 +472,28 @@
 		reg = <0>;
 	};
 	mipid@2 {
-		compatible = "acx565akm";
+		compatible = "sony,acx565akm";
 		spi-max-frequency = <6000000>;
 		reg = <2>;
 
+		label = "lcd";
+		reset-gpio = <&gpio3 26 GPIO_ACTIVE_HIGH>; /* 90 */
+
 		pinctrl-names = "default";
 		pinctrl-0 = <&display_pins>;
+
+		lcd_in: endpoint {
+			remote-endpoint = <&sdi_out>;
+		};
+	};
+};
+
+&sdi {
+	vdds_sdi-supply = <&vaux1>;
+
+	sdi_out: endpoint {
+		remote-endpoint = <&lcd_in>;
+		data-lines = <2>;
 	};
 };
 
-- 
1.8.5.1


^ permalink raw reply related

* [PATCH 3/4] backlight: lcd: call put_device if device_register fails
From: Levente Kurusa @ 2013-12-13 19:22 UTC (permalink / raw)
  To: LKML
  Cc: Levente Kurusa, Jingoo Han, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev
In-Reply-To: <1386962557-8899-1-git-send-email-levex@linux.com>

Currently we kfree the container of the device which failed to register.
This is wrong as the last reference is not given up with a put_device
call. Also, now that we have put_device() callen, we no longer need
the kfree as the new_ld->dev.release function will take care of kfreeing
the associated memory.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/video/backlight/lcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 93cf15e..7de847d 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
 
 	rc = device_register(&new_ld->dev);
 	if (rc) {
-		kfree(new_ld);
+		put_device(&new_ld->dev);
 		return ERR_PTR(rc);
 	}
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [patch 1/2] video: mmp: delete a stray mutex_unlock()
From: Haojian Zhuang @ 2013-12-14  7:27 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20131114081918.GB8150@elgon.mountain>

On 12/13/2013 09:11 PM, Dan Carpenter wrote:
> Ping.
>
> regards,
> dan carpenter
>
> On Thu, Nov 14, 2013 at 11:19:18AM +0300, Dan Carpenter wrote:
>> We aren't holding the disp_lock so we shouldn't release it.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> diff --git a/drivers/video/mmp/core.c b/drivers/video/mmp/core.c
>> index 84de263..c8d4265 100644
>> --- a/drivers/video/mmp/core.c
>> +++ b/drivers/video/mmp/core.c
>> @@ -173,7 +173,7 @@ struct mmp_path *mmp_register_path(struct mmp_path_info *info)
>>   		+ sizeof(struct mmp_overlay) * info->overlay_num;
>>   	path = kzalloc(size, GFP_KERNEL);
>>   	if (!path)
>> -		goto failed;
>> +		return NULL;
>>
>>   	/* path set */
>>   	mutex_init(&path->access_ok);
>> @@ -219,11 +219,6 @@ struct mmp_path *mmp_register_path(struct mmp_path_info *info)
>>
>>   	mutex_unlock(&disp_lock);
>>   	return path;
>> -
>> -failed:
>> -	kfree(path);
>> -	mutex_unlock(&disp_lock);
>> -	return NULL;
>>   }
>>   EXPORT_SYMBOL_GPL(mmp_register_path);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Jean,
Could you help to merge this fix?

Regards
Haojian

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-14  7:34 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Laurent Pinchart, linux-omap, linux-fbdev, devicetree,
	Archit Taneja, Darren Etheridge, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <20131213172234.GD28184@atomide.com>

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

On 2013-12-13 19:22, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [131213 07:49]:
>> Hi Laurent, Tony,
>>
>> On 2013-12-13 16:37, Laurent Pinchart wrote:
>>
>>>>> - 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)
>>> {
>>> }
>>
>> Yep. It seems Tony removed the muxing for -rc2 in
>> e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 ARM: OMAP2+: Remove legacy mux
>> code for display.c
>>
>> Tony, that patch removes DSI muxing, which is not done via DT. I can't
>> test right now, but I presume DSI displays don't work at all after -rc2.
> 
> Hmm I suggest you test against commit adfe9361b236 (ARM: dts: Add basic
> devices on am3517-evm) as it does not yet remove the legacy data and
> that's what's heading to linux next soonish.

That commit is not in the mainline. I'm talking about mainline.
v3.13-rc3 contains e30b06f4d5f000c31a7747a7e7ada78a5fd419a1, and that
breaks DSI displays (just tested). It needs to be reverted (although the
HDMI parts can probably be removed).

Why was e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 merged into -rc2? It's
not a fix, just a cleanup.

> With the DT configured displays that muxing needs to be done in the
> DSS driver(s) using pinctrl-single.

We don't have any DT configured displays in the mainline.

pinctrl-single doesn't support the kind of register that contains the
DSI muxing. I don't know yet how that should be done. In any case, the
muxing via DT should've been added at the same time as removing the
muxing via platform callback in e30b06f4d5f000c31a77.

> BTW, I suspect quite a few of the DSI using boards have been broken a
> while before 0b2aa8bed3e1 (gpio: twl4030: Fix regression for twl gpio
> output) as at least the following have been using TWL GPIO to enable
> the panel:
> 
> board-3430sdp.c
> board-devkit8000.c
> board-ldp.c
> board-omap3stalker.c
> 
> This was the case at least for LDP.

Only 4430sdp has a DSI display in the mainline. Those boards have DPI
displays.

 Tomi



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

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox