diff mbox

[08/28] ARM: OMAP: CM-T35: Kconfig option for the display options

Message ID 1364474962-20439-9-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen March 28, 2013, 12:49 p.m. UTC
Boards with multiple display options for the same video bus have all the
possible linux display devices present at the same time. Only one of
those devices should be used at a time, as the video bus cannot be
shared.

This model is hacky, and will be changed in the forthcoming DSS patches
to a model where only one display device can be present on a single
video bus.

This patch creates Kconfig options to select which of the display
devices is present on the board. While this model is also somewhat
hacky, and prevents us from using a single kernel image for all the
display options, it allows us to make the DSS driver changes for DT
adaptation. And with DT, the information about display devices can be
passed from the bootloader, solving the mess.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Mike Rapoport <mike@compulab.co.il>
Cc: Igor Grinberg <grinberg@compulab.co.il>
---
 arch/arm/mach-omap2/Kconfig        |   13 ++++++
 arch/arm/mach-omap2/board-cm-t35.c |   84 +++++++++++++++++++++---------------
 2 files changed, 62 insertions(+), 35 deletions(-)

Comments

Igor Grinberg March 28, 2013, 4:09 p.m. UTC | #1
On 03/28/13 14:49, Tomi Valkeinen wrote:
> Boards with multiple display options for the same video bus have all the
> possible linux display devices present at the same time. Only one of
> those devices should be used at a time, as the video bus cannot be
> shared.

Yes, only one can be used at a time, but you can switch at runtime...

> 
> This model is hacky, and will be changed in the forthcoming DSS patches
> to a model where only one display device can be present on a single
> video bus.

What do you mean by "present"?
Is it active? or registered? or compiled in?

> 
> This patch creates Kconfig options to select which of the display
> devices is present on the board. While this model is also somewhat
> hacky, and prevents us from using a single kernel image for all the
> display options, it allows us to make the DSS driver changes for DT
> adaptation. And with DT, the information about display devices can be
> passed from the bootloader, solving the mess.

Hmmm, the fact that we cannot use the same binary for multiple displays...
This does not sound right and good at all...
While we try to move to support multiple platforms in the same binary, we
cannot support multiple displays? I agree that the multiplatform stuff is
not really related, but you know what I mean...

I bet there must be a much better solution for DT support.
Tony Lindgren March 28, 2013, 4:53 p.m. UTC | #2
* Igor Grinberg <grinberg@compulab.co.il> [130328 09:13]:
> On 03/28/13 14:49, Tomi Valkeinen wrote:
> > Boards with multiple display options for the same video bus have all the
> > possible linux display devices present at the same time. Only one of
> > those devices should be used at a time, as the video bus cannot be
> > shared.
> 
> Yes, only one can be used at a time, but you can switch at runtime...
> 
> > 
> > This model is hacky, and will be changed in the forthcoming DSS patches
> > to a model where only one display device can be present on a single
> > video bus.
> 
> What do you mean by "present"?
> Is it active? or registered? or compiled in?
> 
> > 
> > This patch creates Kconfig options to select which of the display
> > devices is present on the board. While this model is also somewhat
> > hacky, and prevents us from using a single kernel image for all the
> > display options, it allows us to make the DSS driver changes for DT
> > adaptation. And with DT, the information about display devices can be
> > passed from the bootloader, solving the mess.
> 
> Hmmm, the fact that we cannot use the same binary for multiple displays...
> This does not sound right and good at all...
> While we try to move to support multiple platforms in the same binary, we
> cannot support multiple displays? I agree that the multiplatform stuff is
> not really related, but you know what I mean...

Yes that's not nice from usability point of view. Can we still switch
between LCD and DVI during the runtime? I thought the issue was registering
multiple LCD panels where only one can exist at a time?
 
> I bet there must be a much better solution for DT support.

Well the best legacy support option would be some fbdev/panel generic cmdline
option to specify which one to use. Maybe there is already something like
that available that the board-*.c files can parse and can be used also
later on with DT support to specify the preferred output?

What we don't want to do is add a board specific cmdline option as the
board-*.c files will be going away as soon as DT is usable for us and we
don't want to support a board specific cmdline after that. But if it's
a generic option then parsing it in the board-*.c file or the driver later
on can be supported.

Of course for some cases it might be possible to detect the configured
panel based on what's plugged in over i2c.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen March 28, 2013, 5:02 p.m. UTC | #3
On 2013-03-28 18:09, Igor Grinberg wrote:
> On 03/28/13 14:49, Tomi Valkeinen wrote:
>> Boards with multiple display options for the same video bus have all the
>> possible linux display devices present at the same time. Only one of
>> those devices should be used at a time, as the video bus cannot be
>> shared.
> 
> Yes, only one can be used at a time, but you can switch at runtime...

Yep. But the panel drivers need to know about this, and they cannot do
more or less anything in the driver probe function, which I think should
be the place to reserve resources and do initialization. With the
current model that would lead to multiple drivers trying to acquire the
same resources.

>> This model is hacky, and will be changed in the forthcoming DSS patches
>> to a model where only one display device can be present on a single
>> video bus.
> 
> What do you mean by "present"?
> Is it active? or registered? or compiled in?

I mean that only one device can be registered. Well, nothing strictly
prevents having multiple devices registered, but if the devices have
matching drivers, the drivers would acquire the same resources. Possibly
the same gpios, but at least the same video bus.

>> This patch creates Kconfig options to select which of the display
>> devices is present on the board. While this model is also somewhat
>> hacky, and prevents us from using a single kernel image for all the
>> display options, it allows us to make the DSS driver changes for DT
>> adaptation. And with DT, the information about display devices can be
>> passed from the bootloader, solving the mess.
> 
> Hmmm, the fact that we cannot use the same binary for multiple displays...
> This does not sound right and good at all...
> While we try to move to support multiple platforms in the same binary, we
> cannot support multiple displays? I agree that the multiplatform stuff is
> not really related, but you know what I mean...

Yes, I quite agree that this sucks =). There are a few reasons I chose
to try this approach:

- The whole multi-display feature is hacky

- DT support for DSS has been in development too long time. We really
need it, preferably yesterday. This change helps getting DT support ready.

- Common Display Framework won't (most likely) support this kind of
feature, as the feature itself is rather hacky, so this would anyway
disappear.

- DT support should solve this (except for runtime switching), and the
board files are on the way out (as far as I understand). So in that
sense this is temporary.

- Keeping this feature functional consumes considerable amount of
man-hours, which are in short supply.

I still feel quite bad about this, though. Any ideas how to manage this
better are appreciated.

> I bet there must be a much better solution for DT support.

Yes, I think I covered that in the last email. DT will solve the issue,
except for runtime switching, which is still open. I'm not sure how
these board specific drivers would be implemented.

 Tomi
Tomi Valkeinen March 28, 2013, 5:12 p.m. UTC | #4
On 2013-03-28 18:53, Tony Lindgren wrote:
> * Igor Grinberg <grinberg@compulab.co.il> [130328 09:13]:
>> On 03/28/13 14:49, Tomi Valkeinen wrote:
>>> Boards with multiple display options for the same video bus have all the
>>> possible linux display devices present at the same time. Only one of
>>> those devices should be used at a time, as the video bus cannot be
>>> shared.
>>
>> Yes, only one can be used at a time, but you can switch at runtime...
>>
>>>
>>> This model is hacky, and will be changed in the forthcoming DSS patches
>>> to a model where only one display device can be present on a single
>>> video bus.
>>
>> What do you mean by "present"?
>> Is it active? or registered? or compiled in?
>>
>>>
>>> This patch creates Kconfig options to select which of the display
>>> devices is present on the board. While this model is also somewhat
>>> hacky, and prevents us from using a single kernel image for all the
>>> display options, it allows us to make the DSS driver changes for DT
>>> adaptation. And with DT, the information about display devices can be
>>> passed from the bootloader, solving the mess.
>>
>> Hmmm, the fact that we cannot use the same binary for multiple displays...
>> This does not sound right and good at all...
>> While we try to move to support multiple platforms in the same binary, we
>> cannot support multiple displays? I agree that the multiplatform stuff is
>> not really related, but you know what I mean...
> 
> Yes that's not nice from usability point of view. Can we still switch
> between LCD and DVI during the runtime? I thought the issue was registering
> multiple LCD panels where only one can exist at a time?

I guess I could've been more verbose in my descriptions. And I agree
this is not a nice change.

No, we can't switch between the LCD and DVI. But that's not strictly DSS
issue. The selection between LCD and DVI (or any other displays on the
same bus) are board specific things, sometimes requiring board specific
gpios or even things like i2c commands to muxes.

It works now because we have custom callbacks in the board files to do
those things, but it'll break with DT.

>> I bet there must be a much better solution for DT support.
> 
> Well the best legacy support option would be some fbdev/panel generic cmdline
> option to specify which one to use. Maybe there is already something like
> that available that the board-*.c files can parse and can be used also
> later on with DT support to specify the preferred output?
> 
> What we don't want to do is add a board specific cmdline option as the
> board-*.c files will be going away as soon as DT is usable for us and we
> don't want to support a board specific cmdline after that. But if it's
> a generic option then parsing it in the board-*.c file or the driver later
> on can be supported.
> 
> Of course for some cases it might be possible to detect the configured
> panel based on what's plugged in over i2c.

I could look at using omapdss's module parameter for this. It has
default display parameter. Perhaps the board files could parse this, and
use it to decide which display to register. Perhaps that'd cause less
inconvenience.

It's still quite hacky, as you can only select one display. If there's a
board with two video outputs, and, say, 2 displays connected to each.
You can only select the display for one of the busses. Anyway, we don't
have that kind of board in the mainline, so it's not an issue.

 Tomi
Tony Lindgren March 28, 2013, 9:28 p.m. UTC | #5
* Tomi Valkeinen <tomi.valkeinen@ti.com> [130328 10:16]:
> On 2013-03-28 18:53, Tony Lindgren wrote:
> > 
> > Yes that's not nice from usability point of view. Can we still switch
> > between LCD and DVI during the runtime? I thought the issue was registering
> > multiple LCD panels where only one can exist at a time?
> 
> I guess I could've been more verbose in my descriptions. And I agree
> this is not a nice change.
> 
> No, we can't switch between the LCD and DVI. But that's not strictly DSS
> issue. The selection between LCD and DVI (or any other displays on the
> same bus) are board specific things, sometimes requiring board specific
> gpios or even things like i2c commands to muxes.
> 
> It works now because we have custom callbacks in the board files to do
> those things, but it'll break with DT.

Well people are already used to changing between LCD and DVI.. So we
need some way of doing it with DT also.

How do you plan to change between LCD and DVI with DT? By using some
custom driver modules?

There's the capebus coming that can be used for that too, but in most
cases all the hardware is permanently connected with LCD and DVI. So
sounds like capebus should only be needed for the add on boards.
 
> >> I bet there must be a much better solution for DT support.
> > 
> > Well the best legacy support option would be some fbdev/panel generic cmdline
> > option to specify which one to use. Maybe there is already something like
> > that available that the board-*.c files can parse and can be used also
> > later on with DT support to specify the preferred output?
> > 
> > What we don't want to do is add a board specific cmdline option as the
> > board-*.c files will be going away as soon as DT is usable for us and we
> > don't want to support a board specific cmdline after that. But if it's
> > a generic option then parsing it in the board-*.c file or the driver later
> > on can be supported.
> > 
> > Of course for some cases it might be possible to detect the configured
> > panel based on what's plugged in over i2c.
> 
> I could look at using omapdss's module parameter for this. It has
> default display parameter. Perhaps the board files could parse this, and
> use it to decide which display to register. Perhaps that'd cause less
> inconvenience.
> 
> It's still quite hacky, as you can only select one display. If there's a
> board with two video outputs, and, say, 2 displays connected to each.
> You can only select the display for one of the busses. Anyway, we don't
> have that kind of board in the mainline, so it's not an issue.

If we can use omapdss cmdline params for initializing the selected display,
that allows leaving out the kconfig selection, so that's much nicer for
distros etc.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen March 29, 2013, 4:27 a.m. UTC | #6
On 2013-03-28 23:28, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [130328 10:16]:
>> On 2013-03-28 18:53, Tony Lindgren wrote:
>>>
>>> Yes that's not nice from usability point of view. Can we still switch
>>> between LCD and DVI during the runtime? I thought the issue was registering
>>> multiple LCD panels where only one can exist at a time?
>>
>> I guess I could've been more verbose in my descriptions. And I agree
>> this is not a nice change.
>>
>> No, we can't switch between the LCD and DVI. But that's not strictly DSS
>> issue. The selection between LCD and DVI (or any other displays on the
>> same bus) are board specific things, sometimes requiring board specific
>> gpios or even things like i2c commands to muxes.
>>
>> It works now because we have custom callbacks in the board files to do
>> those things, but it'll break with DT.
> 
> Well people are already used to changing between LCD and DVI.. So we
> need some way of doing it with DT also.
> 
> How do you plan to change between LCD and DVI with DT? By using some
> custom driver modules?

I don't know, but I guess we need board specific drivers.

Do you know if there are other similar cases for other busses? I mean, I
think this is a similar case than, say, having two i2c devices with the
same i2c-id on the same bus. Only one of the devices can be enabled, the
other must be disconnected. And how the connect/disconnect is made is
board specific.

> There's the capebus coming that can be used for that too, but in most
> cases all the hardware is permanently connected with LCD and DVI. So
> sounds like capebus should only be needed for the add on boards.

True.

Well, depends on how you look at it. From one point of view there's no
real difference whether the disabled device is physically on the board
or not, as it has to be disconnected. The details depend on the bus, but
for example for DSI I think the disabled device has to be totally
removed from the DSI bus with some mux or such.

But, from the other point of view, the devices are there, on the board,
and in some cases the HW has been designed to support switching the
displays during runtime.

So what I wish is that I could make the linux device for the display to
be removed when it's disabled. I believe that can be done, but I guess
it requires a board specific driver, with some custom user interface to
do that.

And it would be different user interface than currently, which is again
not so nice. Currently the devices are always there, with their sysfs
files, and those sysfs files are used to enable/disable the devices. If
the device is removed, the sysfs files get removed also...

But I would really like to get rid of the current model, which I think
was a very bad design decision.

 Tomi
Tony Lindgren March 29, 2013, 3:47 p.m. UTC | #7
* Tomi Valkeinen <tomi.valkeinen@ti.com> [130328 21:32]:
> On 2013-03-28 23:28, Tony Lindgren wrote:
> > 
> > How do you plan to change between LCD and DVI with DT? By using some
> > custom driver modules?
> 
> I don't know, but I guess we need board specific drivers.
> 
> Do you know if there are other similar cases for other busses? I mean, I
> think this is a similar case than, say, having two i2c devices with the
> same i2c-id on the same bus. Only one of the devices can be enabled, the
> other must be disconnected. And how the connect/disconnect is made is
> board specific.

No good ideas, unless it all can be controlled via pinctrl framework?

With pinctrl framework the dss driver could request named modes like
"dvi" and "lcd" for sets of pins. And the panel driver could implement
pinctrl functions, so when a named mode "lcd" is requested by the dss
driver, the data lines configured and GPIO and regulators get set.
If we are really muxing the dss data lines, then using that might
make sense.

Note that there may be issues related to waiting sleeping regulators etc.

> > There's the capebus coming that can be used for that too, but in most
> > cases all the hardware is permanently connected with LCD and DVI. So
> > sounds like capebus should only be needed for the add on boards.
> 
> True.
> 
> Well, depends on how you look at it. From one point of view there's no
> real difference whether the disabled device is physically on the board
> or not, as it has to be disconnected. The details depend on the bus, but
> for example for DSI I think the disabled device has to be totally
> removed from the DSI bus with some mux or such.

Yes, so if we're really muxing lines, then maybe using the pinctrl
framework makes sense until we have the capebus available?
 
> But, from the other point of view, the devices are there, on the board,
> and in some cases the HW has been designed to support switching the
> displays during runtime.
> 
> So what I wish is that I could make the linux device for the display to
> be removed when it's disabled. I believe that can be done, but I guess
> it requires a board specific driver, with some custom user interface to
> do that.

Sounds like capebus will do that when it's available.
 
> And it would be different user interface than currently, which is again
> not so nice. Currently the devices are always there, with their sysfs
> files, and those sysfs files are used to enable/disable the devices. If
> the device is removed, the sysfs files get removed also...

You can probably keep the user interface the same if the dss driver
requests changing the panels, and can probably support both pinctrl
muxing and capebus pretty easily. The biggest change would be that when
capebus is available there's more than one struct device for the panels,
so the .dts files would have to be updated for that.

Or maybe the pinctrl handler could already easily add and remove devices
depending which named mode is requested by the dss driver, I don't know.
 
> But I would really like to get rid of the current model, which I think
> was a very bad design decision.

Yes agreed, we need to get rid of the board-*.c file callback functions.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Igor Grinberg March 31, 2013, 9:17 a.m. UTC | #8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 03/28/13 19:02, Tomi Valkeinen wrote:
> On 2013-03-28 18:09, Igor Grinberg wrote:
>> On 03/28/13 14:49, Tomi Valkeinen wrote:
>>> Boards with multiple display options for the same video bus have all the
>>> possible linux display devices present at the same time. Only one of
>>> those devices should be used at a time, as the video bus cannot be
>>> shared.
>>
>> Yes, only one can be used at a time, but you can switch at runtime...
> 
> Yep. But the panel drivers need to know about this, and they cannot do
> more or less anything in the driver probe function, which I think should
> be the place to reserve resources and do initialization. With the
> current model that would lead to multiple drivers trying to acquire the
> same resources.

Yep, this is a problem, but it only means that probably
the platform device framework is not suitable for this.

What about having some kind of display manager which will have a private
list of registered devices and will instantiate only those that are marked
active?
This also might be useful with DT.

> 
>>> This model is hacky, and will be changed in the forthcoming DSS patches
>>> to a model where only one display device can be present on a single
>>> video bus.
>>
>> What do you mean by "present"?
>> Is it active? or registered? or compiled in?
> 
> I mean that only one device can be registered. Well, nothing strictly
> prevents having multiple devices registered, but if the devices have
> matching drivers, the drivers would acquire the same resources. Possibly
> the same gpios, but at least the same video bus.

Well, I think, we should follow/extend the already existing EDID concept...
Instantiate a display device only when one has been "plugged in".
By "plugged in" I mean enabled - made active.

> 
>>> This patch creates Kconfig options to select which of the display
>>> devices is present on the board. While this model is also somewhat
>>> hacky, and prevents us from using a single kernel image for all the
>>> display options, it allows us to make the DSS driver changes for DT
>>> adaptation. And with DT, the information about display devices can be
>>> passed from the bootloader, solving the mess.
>>
>> Hmmm, the fact that we cannot use the same binary for multiple displays...
>> This does not sound right and good at all...
>> While we try to move to support multiple platforms in the same binary, we
>> cannot support multiple displays? I agree that the multiplatform stuff is
>> not really related, but you know what I mean...
> 
> Yes, I quite agree that this sucks =). There are a few reasons I chose
> to try this approach:
> 
> - The whole multi-display feature is hacky
> 
> - DT support for DSS has been in development too long time. We really
> need it, preferably yesterday. This change helps getting DT support ready.

Yes, but I don't think this should involve removing the existing
functionality..
Let me exaggerate a bit: this is like removing ARM support from mainline,
so it will make less noise and headache to Linus...

> 
> - Common Display Framework won't (most likely) support this kind of
> feature, as the feature itself is rather hacky, so this would anyway
> disappear.

Hmmm, I don't think it will disappear, but instead you will keep
receiving hacky patches to bring that support in...
So, IMO, it would be better designed to support this or at least
keep it in mind, so there will no necessity for hacky patches...

> 
> - DT support should solve this (except for runtime switching), and the
> board files are on the way out (as far as I understand). So in that
> sense this is temporary.

But not the runtime switching...
The runtime switching should be done in the framework.

> 
> - Keeping this feature functional consumes considerable amount of
> man-hours, which are in short supply.

This is where you should request some help from the interested sides...
And mailing lists are quite handy.

> 
> I still feel quite bad about this, though. Any ideas how to manage this
> better are appreciated.
> 
>> I bet there must be a much better solution for DT support.
> 
> Yes, I think I covered that in the last email. DT will solve the issue,
> except for runtime switching, which is still open. I'm not sure how
> these board specific drivers would be implemented.

Well, at least the generic kernel display parameter will do more than
half of the job.
We can discuss the runtime switch stuff, but it is really a non-sense
to compile a kernel for specific display...


- -- 
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRV/8gAAoJEBDE8YO64EfaGOQQAIEnXwrHiZFlsMkCo5n8uUok
qBBvEoFh5s1h2aaDNna3Q7tvKF42UlaGtZPqwl5bDUHimcEaLnNPy14MQJ/w/dHN
AvDaI2ZoqGCYDVjVa0KUCWqL6Chm7ZpAoU9NeY3T3+U8S7kIeizz0oBJKoZ4bfpk
g6Kq5vnbi36pCWFMqQe+7fEn5CHJNHTp+miPpq3Dsed2zkVgIg9re0nTxpQjlDHO
+kLwJjrHbCqZoBMzE0MacaibROiHhccQ9Xtf25iVemyHQ6CP6g+ibJduFF7S4pOk
kOwH1NY2QiFGDyA0Tj+kuYsbUYe3E1ds7FCu0/9g2KpiGRqSAqrFDghJzj1PojL+
SxL47jgMkDUehEnBC2Eq3DD5jq4YRd2Sn989FH5ov9GWcyFhzBl72LHHhI78YRpu
3QKGAnAcSVOyNcvkDNYhsJRqs9qUpnn9Z89jrx430ckK+SI4T4qiczn4aFEXFPxC
Zqp8Rmvn95X5D9kMBgMScy1cHzkWRZ32GCKSRkOqnaFim2knxrflnX1Riu11jFCL
k1AK+rKhqUB9FaME3Uyn07s+cTKCVX1TWmnP6Er9ObZkOKR2fR7ZwEmTbhA/m0pe
iefjKf6gxuZV/FX+OnTvivorw2CMfGyweolu2JHj9cTuEGvddeq/LhKz+cEzdMVL
Vpj9krR7L6Hq8dFqz8Dk
=sr0U
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen April 2, 2013, 8:07 a.m. UTC | #9
On 2013-03-31 12:17, Igor Grinberg wrote:
> 
> 
> On 03/28/13 19:02, Tomi Valkeinen wrote:
>> On 2013-03-28 18:09, Igor Grinberg wrote:
>>> On 03/28/13 14:49, Tomi Valkeinen wrote:
>>>> Boards with multiple display options for the same video bus have all the
>>>> possible linux display devices present at the same time. Only one of
>>>> those devices should be used at a time, as the video bus cannot be
>>>> shared.
>>>
>>> Yes, only one can be used at a time, but you can switch at runtime...
> 
>> Yep. But the panel drivers need to know about this, and they cannot do
>> more or less anything in the driver probe function, which I think should
>> be the place to reserve resources and do initialization. With the
>> current model that would lead to multiple drivers trying to acquire the
>> same resources.
> 
> Yep, this is a problem, but it only means that probably
> the platform device framework is not suitable for this.
> 
> What about having some kind of display manager which will have a private
> list of registered devices and will instantiate only those that are marked
> active?
> This also might be useful with DT.

We can't have a generic manager that handles instantiating the devices,
as we don't know what device they are. They could be platform devices,
i2c, anything. There could even be a single device that creates multiple
displays.

>>>> This model is hacky, and will be changed in the forthcoming DSS patches
>>>> to a model where only one display device can be present on a single
>>>> video bus.
>>>
>>> What do you mean by "present"?
>>> Is it active? or registered? or compiled in?
> 
>> I mean that only one device can be registered. Well, nothing strictly
>> prevents having multiple devices registered, but if the devices have
>> matching drivers, the drivers would acquire the same resources. Possibly
>> the same gpios, but at least the same video bus.
> 
> Well, I think, we should follow/extend the already existing EDID concept...
> Instantiate a display device only when one has been "plugged in".
> By "plugged in" I mean enabled - made active.

This brings the complication that we need a way to make the display
active even if the display device doesn't exist. So we need to know
about the display even if it's not there.

>>>> This patch creates Kconfig options to select which of the display
>>>> devices is present on the board. While this model is also somewhat
>>>> hacky, and prevents us from using a single kernel image for all the
>>>> display options, it allows us to make the DSS driver changes for DT
>>>> adaptation. And with DT, the information about display devices can be
>>>> passed from the bootloader, solving the mess.
>>>
>>> Hmmm, the fact that we cannot use the same binary for multiple displays...
>>> This does not sound right and good at all...
>>> While we try to move to support multiple platforms in the same binary, we
>>> cannot support multiple displays? I agree that the multiplatform stuff is
>>> not really related, but you know what I mean...
> 
>> Yes, I quite agree that this sucks =). There are a few reasons I chose
>> to try this approach:
> 
>> - The whole multi-display feature is hacky
> 
>> - DT support for DSS has been in development too long time. We really
>> need it, preferably yesterday. This change helps getting DT support ready.
> 
> Yes, but I don't think this should involve removing the existing
> functionality..
> Let me exaggerate a bit: this is like removing ARM support from mainline,
> so it will make less noise and headache to Linus...

That is exaggerating quite a bit ;). But yes, I agree that we should not
remove the features. I just couldn't find a manageable way to have them
while still moving forward to DT and CDF.

 Tomi
Igor Grinberg April 2, 2013, 11:20 a.m. UTC | #10
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/02/13 11:07, Tomi Valkeinen wrote:
> On 2013-03-31 12:17, Igor Grinberg wrote:
>>
>>
>> On 03/28/13 19:02, Tomi Valkeinen wrote:
>>> On 2013-03-28 18:09, Igor Grinberg wrote:
>>>> On 03/28/13 14:49, Tomi Valkeinen wrote:
>>>>> Boards with multiple display options for the same video bus have all the
>>>>> possible linux display devices present at the same time. Only one of
>>>>> those devices should be used at a time, as the video bus cannot be
>>>>> shared.
>>>>
>>>> Yes, only one can be used at a time, but you can switch at runtime...
>>
>>> Yep. But the panel drivers need to know about this, and they cannot do
>>> more or less anything in the driver probe function, which I think should
>>> be the place to reserve resources and do initialization. With the
>>> current model that would lead to multiple drivers trying to acquire the
>>> same resources.
>>
>> Yep, this is a problem, but it only means that probably
>> the platform device framework is not suitable for this.
>>
>> What about having some kind of display manager which will have a private
>> list of registered devices and will instantiate only those that are marked
>> active?
>> This also might be useful with DT.
> 
> We can't have a generic manager that handles instantiating the devices,
> as we don't know what device they are. They could be platform devices,
> i2c, anything. There could even be a single device that creates multiple
> displays.

Unless, the registered device node is descriptive enough,
or the device node can "instantiate itself".
We use the "ops" structures all over the kernel.

What about the capebus? Can it help with abstracting such things?

> 
>>>>> This model is hacky, and will be changed in the forthcoming DSS patches
>>>>> to a model where only one display device can be present on a single
>>>>> video bus.
>>>>
>>>> What do you mean by "present"?
>>>> Is it active? or registered? or compiled in?
>>
>>> I mean that only one device can be registered. Well, nothing strictly
>>> prevents having multiple devices registered, but if the devices have
>>> matching drivers, the drivers would acquire the same resources. Possibly
>>> the same gpios, but at least the same video bus.
>>
>> Well, I think, we should follow/extend the already existing EDID concept...
>> Instantiate a display device only when one has been "plugged in".
>> By "plugged in" I mean enabled - made active.
> 
> This brings the complication that we need a way to make the display
> active even if the display device doesn't exist. So we need to know
> about the display even if it's not there.

Yes, well, I mean that the process of making the display active can
involve registering the display device, no?
And yes, it does bring the complication, but I don't really see a way we
can avoid such complications and yet support the modern hardware.


- -- 
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRWr8VAAoJEBDE8YO64Efa5sgP/ArPjYtGakOnWkbFah5ClnPH
sOMDwnTEW76rLpXzPE63dTBIImiBhKpaKNKK3pxdPH7uhZ5qr1usi/s/W59MPrYA
7lsZnkdsJLk/piFKk1IdB9Q+ke7qftCEmVTrv9fXtYrojZ9NHH1yGqmjQIINyzUm
hGTbplKUldfMSh9z+XszUcGgBkkZQgMd435eJKOYw9Bny56u0ekOfPIo5pHTJioc
F4gQybWQ41hpBVWe8OkYH8tN9hOnujbdgQ+K4K5JYizJV0NYnGgxESvescieAINn
0INEYhiyD97Fcr6sFFjOSqkLTZoBaXFLDl0EPWqasImf5sCLapmtkqfpMRlUjoiW
MiVv6XACE5AmFicivjPowDn01HurH0Q8aXXhvhtSEu8oLePN3gH5PW9P6VIjAUbM
85OT+VkPfWIUupw2aeygy+ePoBpjj4NtZWzhpxzdDi/k5HkrZvSh3uGernCbAsYj
/JV4wZRQQVml7j65uYRLmnKx+tMbBnd/QU96OLdSOdGaNor+bY0x0zDcy4DAKuw3
/W0HPGMr6dctBOb26ofYn97jxjLAcULKTWffykxktNyB1ODzN2NdEUnUv6rtq4Bv
6EA7EaK1dehM2TV4eVNiCxxvv88Kk58uJqzuxvx2BHGtDkbygJGxkBZ41/MNWQOT
3UMYsUEKecOie5cT+JtG
=IrRQ
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 4108d07..6e73cb4 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -387,6 +387,19 @@  config MACH_CM_T35
 	select MACH_CM_T3730
 	select OMAP_PACKAGE_CUS
 
+choice
+	depends on MACH_CM_T35
+	default MACH_CM_T35_EXPANSION_LCD
+	prompt "CM-T35/CM-T3730 video output"
+
+config MACH_CM_T35_EXPANSION_LCD
+	bool "CM-T35/CM-T3730 with LCD"
+
+config MACH_CM_T35_EXPANSION_DVI
+	bool "CM-T35/CM-T3730 with DVI"
+
+endchoice
+
 config MACH_CM_T3517
 	bool "CompuLab CM-T3517 module"
 	depends on ARCH_OMAP3
diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
index 9c1895a..1b34c96 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -187,32 +187,28 @@  static void __init cm_t35_init_nand(void)
 static inline void cm_t35_init_nand(void) {}
 #endif
 
+static struct omap_dss_device cm_t35_tv_device = {
+	.name			= "tv",
+	.driver_name		= "venc",
+	.type			= OMAP_DISPLAY_TYPE_VENC,
+	.phy.venc.type		= OMAP_DSS_VENC_TYPE_SVIDEO,
+};
+
+#if defined(CONFIG_MACH_CM_T35_EXPANSION_LCD)
+
 #define CM_T35_LCD_EN_GPIO 157
 #define CM_T35_LCD_BL_GPIO 58
-#define CM_T35_DVI_EN_GPIO 54
-
-static int lcd_enabled;
-static int dvi_enabled;
 
 static int cm_t35_panel_enable_lcd(struct omap_dss_device *dssdev)
 {
-	if (dvi_enabled) {
-		printk(KERN_ERR "cannot enable LCD, DVI is enabled\n");
-		return -EINVAL;
-	}
-
 	gpio_set_value(CM_T35_LCD_EN_GPIO, 1);
 	gpio_set_value(CM_T35_LCD_BL_GPIO, 1);
 
-	lcd_enabled = 1;
-
 	return 0;
 }
 
 static void cm_t35_panel_disable_lcd(struct omap_dss_device *dssdev)
 {
-	lcd_enabled = 0;
-
 	gpio_set_value(CM_T35_LCD_BL_GPIO, 0);
 	gpio_set_value(CM_T35_LCD_EN_GPIO, 0);
 }
@@ -231,36 +227,15 @@  static struct omap_dss_device cm_t35_lcd_device = {
 	.phy.dpi.data_lines	= 18,
 };
 
-static struct tfp410_platform_data dvi_panel = {
-	.power_down_gpio	= CM_T35_DVI_EN_GPIO,
-	.i2c_bus_num		= -1,
-};
-
-static struct omap_dss_device cm_t35_dvi_device = {
-	.name			= "dvi",
-	.type			= OMAP_DISPLAY_TYPE_DPI,
-	.driver_name		= "tfp410",
-	.data			= &dvi_panel,
-	.phy.dpi.data_lines	= 24,
-};
-
-static struct omap_dss_device cm_t35_tv_device = {
-	.name			= "tv",
-	.driver_name		= "venc",
-	.type			= OMAP_DISPLAY_TYPE_VENC,
-	.phy.venc.type		= OMAP_DSS_VENC_TYPE_SVIDEO,
-};
-
 static struct omap_dss_device *cm_t35_dss_devices[] = {
 	&cm_t35_lcd_device,
-	&cm_t35_dvi_device,
 	&cm_t35_tv_device,
 };
 
 static struct omap_dss_board_info cm_t35_dss_data = {
 	.num_devices	= ARRAY_SIZE(cm_t35_dss_devices),
 	.devices	= cm_t35_dss_devices,
-	.default_device	= &cm_t35_dvi_device,
+	.default_display_name = "lcd",
 };
 
 static struct omap2_mcspi_device_config tdo24m_mcspi_config = {
@@ -314,6 +289,45 @@  static void __init cm_t35_init_display(void)
 	}
 }
 
+#elif defined(CONFIG_MACH_CM_T35_EXPANSION_DVI)
+
+#define CM_T35_DVI_EN_GPIO 54
+
+static struct tfp410_platform_data dvi_panel = {
+	.power_down_gpio	= CM_T35_DVI_EN_GPIO,
+	.i2c_bus_num		= -1,
+};
+
+static struct omap_dss_device cm_t35_dvi_device = {
+	.name			= "dvi",
+	.type			= OMAP_DISPLAY_TYPE_DPI,
+	.driver_name		= "tfp410",
+	.data			= &dvi_panel,
+	.phy.dpi.data_lines	= 24,
+};
+
+static struct omap_dss_device *cm_t35_dss_devices[] = {
+	&cm_t35_dvi_device,
+	&cm_t35_tv_device,
+};
+
+static struct omap_dss_board_info cm_t35_dss_data = {
+	.num_devices	= ARRAY_SIZE(cm_t35_dss_devices),
+	.devices	= cm_t35_dss_devices,
+	.default_display_name = "dvi",
+};
+
+static void __init cm_t35_init_display(void)
+{
+	int err;
+
+	err = omap_display_init(&cm_t35_dss_data);
+	if (err)
+		pr_err("CM-T35: failed to register DSS device\n");
+}
+
+#endif
+
 static struct regulator_consumer_supply cm_t35_vmmc1_supply[] = {
 	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0"),
 };