* 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
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tony Lindgren @ 2013-12-14 14:09 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: <52AC0A18.30405@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [131213 23:36]:
> 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.
Hmm OK sorry looks like I removed some non-legacy mux framework code
by accident. The omap_mux_* parts clearly are dead code for omap4 as
it's been DT only since v3.10, but the omap4_dsi_mux_pads() is not
using omap_mux_* functions.
Yes, let's revert e30b06f4d5f0 except for the dead code parts, which
seems to be omap4_tpd12s015_mux_pads(), right?
> > 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.
Yes sorry omap4_dsi_mux_pads() should not have been removed.
> 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.
Oops right, those are DPI.
Regards,
Tony
^ permalink raw reply
* [PATCH] fbmem: really support wildcard video=options for all fbdev drivers
From: Olaf Hering @ 2013-12-15 20:40 UTC (permalink / raw)
To: plagnioj, tomi.valkeinen; +Cc: linux-fbdev, linux-kernel, Olaf Hering
Documentation/fb/modedb.txt states that video=option should be
considered a global option. But video_setup and fb_get_options are not
coded that way. Instead its required to boot with video=driver:option to
set a given option in drvier. This is cumbersome because it requires to
know in advance which driver will be active for a given board/kernel.
The following patch implements the documented catchall for the fbdev
drivers. It is now possible to boot with video=XxY without the need to
know the active driver in advance. The specific case it tries to fix is
syslinux in the SUSE installer which offers a menu to set a display
resolution. Right now this just appends the vga= option the kernel. But
in addition to vga= it should be possible to pass a generic video=XxY
for all framebuffer/drm drivers. With this change forcing a certain
window size of VM displays is now much easier.
Today the video= option is stored in a global fb_mode_option. But
unfortunately only drm uses it.
Note: this change introduces a small memleak if video=option is actually
used because fb_mode_option is const. Most drivers use strsep to get to
individual options. This could be fixed in a followup patch which always
releases the option string in every caller of fb_get_options.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
drivers/video/fbmem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 010d191..cde4619 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1930,6 +1930,9 @@ int fb_get_options(const char *name, char **option)
options = opt + name_len + 1;
}
}
+ /* No match, pass global option */
+ if (!options && option && fb_mode_option)
+ options = kstrdup(fb_mode_option, GFP_KERNEL);
if (options && !strncmp(options, "off", 3))
retval = 1;
^ permalink raw reply related
* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
From: Jingoo Han @ 2013-12-16 4:52 UTC (permalink / raw)
To: 'Levente Kurusa', 'Andrew Morton'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han'
In-Reply-To: <1386959996-7958-4-git-send-email-levex@linux.com>
On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>
> 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>
(+cc Andrew Morton)
Acked-by: Jingoo Han <jg1.han@samsung.com>
It looks good.
According to the comment of device_register, put_device()
should be used, instead of directly freeing.
./drivers/base/core.c
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
*/
int device_register(struct device *dev)
{
device_initialize(dev);
return device_add(dev);
}
EXPORT_SYMBOL_GPL(device_register);
Levente Kurusa,
By the way, don't send the same mails three times, without any
reason. It is the waste of traffic. :-(
Best regards,
Jingoo Han
> ---
> 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
* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-16 7:24 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: <20131214140913.GA12324@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 2946 bytes --]
On 2013-12-14 16:09, Tony Lindgren wrote:
>> Why was e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 merged into -rc2? It's
>> not a fix, just a cleanup.
>
> Hmm OK sorry looks like I removed some non-legacy mux framework code
> by accident. The omap_mux_* parts clearly are dead code for omap4 as
> it's been DT only since v3.10, but the omap4_dsi_mux_pads() is not
> using omap_mux_* functions.
>
> Yes, let's revert e30b06f4d5f0 except for the dead code parts, which
> seems to be omap4_tpd12s015_mux_pads(), right?
Yes. I tested the below patch on 4430SDP, both DSI and HDMI works.
Tomi
From cc38cdb5c77c82f526ebce855711a6b4d4c51180 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
Date: Mon, 16 Dec 2013 09:14:48 +0200
Subject: [PATCH] Revert "ARM: OMAP2+: Remove legacy mux code for display.c"
Commit e30b06f4d5f000c31a7747a7e7ada78a5fd419a1 (ARM: OMAP2+: Remove
legacy mux code for display.c) removed non-DT DSI and HDMI pinmuxing.
However, DSI pinmuxing is still needed, and removing that caused DSI
displays not to work.
This reverts the DSI parts of the commit.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
arch/arm/mach-omap2/display.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 58347bb874a0..4cf165502b35 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -101,13 +101,51 @@ static const struct omap_dss_hwmod_data omap4_dss_hwmod_data[] __initconst = {
{ "dss_hdmi", "omapdss_hdmi", -1 },
};
+static int omap4_dsi_mux_pads(int dsi_id, unsigned lanes)
+{
+ u32 enable_mask, enable_shift;
+ u32 pipd_mask, pipd_shift;
+ u32 reg;
+
+ if (dsi_id == 0) {
+ enable_mask = OMAP4_DSI1_LANEENABLE_MASK;
+ enable_shift = OMAP4_DSI1_LANEENABLE_SHIFT;
+ pipd_mask = OMAP4_DSI1_PIPD_MASK;
+ pipd_shift = OMAP4_DSI1_PIPD_SHIFT;
+ } else if (dsi_id == 1) {
+ enable_mask = OMAP4_DSI2_LANEENABLE_MASK;
+ enable_shift = OMAP4_DSI2_LANEENABLE_SHIFT;
+ pipd_mask = OMAP4_DSI2_PIPD_MASK;
+ pipd_shift = OMAP4_DSI2_PIPD_SHIFT;
+ } else {
+ return -ENODEV;
+ }
+
+ reg = omap4_ctrl_pad_readl(OMAP4_CTRL_MODULE_PAD_CORE_CONTROL_DSIPHY);
+
+ reg &= ~enable_mask;
+ reg &= ~pipd_mask;
+
+ reg |= (lanes << enable_shift) & enable_mask;
+ reg |= (lanes << pipd_shift) & pipd_mask;
+
+ omap4_ctrl_pad_writel(reg, OMAP4_CTRL_MODULE_PAD_CORE_CONTROL_DSIPHY);
+
+ return 0;
+}
+
static int omap_dsi_enable_pads(int dsi_id, unsigned lane_mask)
{
+ if (cpu_is_omap44xx())
+ return omap4_dsi_mux_pads(dsi_id, lane_mask);
+
return 0;
}
static void omap_dsi_disable_pads(int dsi_id, unsigned lane_mask)
{
+ if (cpu_is_omap44xx())
+ omap4_dsi_mux_pads(dsi_id, 0);
}
static int omap_dss_set_min_bus_tput(struct device *dev, unsigned long tput)
--
1.8.3.2
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox