Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct
From: Tomi Valkeinen @ 2011-11-23 10:47 UTC (permalink / raw)
  To: Sergey Kibrik; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCCB07.10702@ti.com>

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

On Wed, 2011-11-23 at 12:29 +0200, Sergey Kibrik wrote:
> On 11/22/2011 11:21 AM, Tomi Valkeinen wrote:
> > dss_cache struct contains a spinlock used to protect the struct. A more
> > logical place for the spinlock is outside the struct that it is
> > protecting. So move it there.
> 
> a small question: isn't it clearer to keep lock inside struct, so it would be easier to read code? Say, if we meet
> 
> > spin_lock_irqsave(&dss_cache.lock, flags);
> 
> in code we already aware of what struct being actually protected, and in case of external lock it's not that obvious

But if you meet code like:

op = get_ovl_priv(ovl);

You don't see that the data is inside the struct protected with the
spinlock. So you still need to understand what it protects, and what the
above function returns.

But I see your point. I'm not sure which way is better. I thought it
like this: the lock protects a struct, but if the lock is inside the
struct, the lock would protect also itself. Which it doesn't.

That said, I'm fine with both ways. It doesn't matter much. I didn't
really look for any established patterns for this in the kernel code,
but if everybody else have their locks inside the structs they protect,
then obviously we should also.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 42/65] OMAPDSS: DSI: call mgr_enable/disable for cmd mode
From: Archit Taneja @ 2011-11-23 11:20 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1322044977.28917.57.camel@deskari>

On Wednesday 23 November 2011 04:12 PM, Tomi Valkeinen wrote:
> On Wed, 2011-11-23 at 15:40 +0530, Archit Taneja wrote:
>> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
>>> The current code uses dsi_video_mode_enable/disable functions to
>>> enable/disable DISPC output for video mode displays. For command mode
>>> displays we have no notion in the DISPC side of whether the panel is
>>> enabled, except when a dss_start_update() call is made.
>>>
>>> However, to properly maintain the DISPC state in apply.c, we need to
>>> know if a manager used for a manual update display is currently in use.
>>>
>>> This patch achieves that by changing dsi_video_mode_enable/disable to
>>> dsi_enable/disable_video_output, which is called by both video and
>>> command mode displays. For video mode displays it starts the actual
>>> pixel stream, as it did before. For command mode displays it doesn't do
>>> anything else than mark that the manager is currently in use.
>>
>> dsi_video_mode_enable() doesn't only enable the DISPC output, it also
>> sends the long packet header to start video mode transfer.
>>
>> I think it would be better if we had 2 separate functions, one which
>> starts/stops DSI video mode, and the other which enables/disables the
>> DISPC video port.
>>
>> This way, a manual update panel would need to call only
>> dsi_enable/disable_video_output(which just enables or disables the
>> manager), whereas a video mode panel will need to call both.
>>
>> This is just a suggestion though. It's probably okay to have both in the
>> same function too. We might have to separate them out later if we were
>> planning to standardise mipi dsi across SoCs.
>
> If you think from the panel driver's point of view, it doesn't know
> about DISPC. It just wants to enable the video stream (on video mode
> displays).
>
> If we had two functions, could only the first be used? I.e. is it
> possible to just enable the video mode transfer, without enabling DISPC?
> If not, I'm not sure what would be the use for two separate functions.
> And even if it can, I'm not sure what use it would be to enable only the
> video mode output without the actual pixel data from DISPC.
>
> It is true that the function in thsi patch is not the best one. For
> command mode display it's more about reserving the ovl manager for use
> than actually enabling it. Then again, how I thought the function's
> purpose was that it enables the DSI video output, but because for
> command mode there's need to trigger the actual frame transfer later,
> the function doesn't start the pixel feed for command mode displays. It
> just "prepares" the output.
>
> But even if we had the functions separated, the function called by video
> mode and command mode displays would be different, as for the former one
> it enables the pixel stream, and for latter one it just reserves the
> output.
>
> So, I'm fine with changing the function, but the reasoning for what
> functions we have and what they do should come from the panel driver's
> perspective, not because of OMAP DSS's HW details.

If you look from the panel driver's perspective, we shouldn't split this.

I think it would be best to stuff the 'video mode enabling and manager 
enabling' functionality in omapdss_dsi_display_enable() itself, the 
panel driver shouldn't need to call a function separately to enable 
video mode for the panel. This way we would be more along the lines of 
the dpi driver, where dpi_display_enable() enables the manager in the end.

I guess we can leave this the way you are doing it currently, and move 
this code into dsi_display_enable() code later on.

Archit

>
>   Tomi
>


^ permalink raw reply

* Re: [PATCH 42/65] OMAPDSS: DSI: call mgr_enable/disable for cmd
From: Tomi Valkeinen @ 2011-11-23 11:27 UTC (permalink / raw)
  To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCD44A.2070008@ti.com>

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

On Wed, 2011-11-23 at 16:38 +0530, Archit Taneja wrote:
> 
> I think it would be best to stuff the 'video mode enabling and
> manager 
> enabling' functionality in omapdss_dsi_display_enable() itself, the 
> panel driver shouldn't need to call a function separately to enable 
> video mode for the panel. This way we would be more along the lines
> of 
> the dpi driver, where dpi_display_enable() enables the manager in the
> end. 

But we need to configure the panel between enabling the DSI interface
and enabling the video output, so we can't combine those two functions.

For DPI things are simpler, as enabling the interface and the video
output are more or less the same thing.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFC] Virtual CRTCs (proposal + experimental code)
From: Dave Airlie @ 2011-11-23 11:48 UTC (permalink / raw)
  To: Ilija Hadzic; +Cc: linux-fbdev, dri-devel
In-Reply-To: <CA+4h6Hk3gE+8ou6NSMhoGJWZJ-5aM1OgC0HFoKs55fjfQ-qN8g@mail.gmail.com>

>
> I would like to bring to the attention of dri-devel and linux-fbdev
> community a set of hopefully useful and interesting patches that
> I (and a few other colleagues) have been working on during the past
> few months. Here, I will provide a short abstract, so that you can
> decide whether this is of interest for you. At the end, I will
> provide the pointers to the code and documentation.
>
> The code is based on Dave Arilie's tree, drm-next branch and it
> allows a GPU driver to have an arbitrary number of CRTCs
> (configurable by user) instead of only those CRTCs that represent
> real hardware.

So another question I have is how you would intend this to work from a
user POV, like how it would integrate with a desktop environment, X or
wayland, i.e. with little or no configuration.

I still foresee problems with tiling, we generally don't encourage
accel code to live in the kernel, and you'll really want a
tiled->untiled blit for this thing, also for Intel GPUs where you have
UMA, would you read from the UMA.

It also doesn't solve the optimus GPU problem in any useful fashion,
since it can't deal with all the use cases, so we still have to write
an alternate solution that can deal with them, so we just end up with
two answers.

Dave.

^ permalink raw reply

* [PATCH] lxfb: add support for LQ057V3DG01 panel
From: Christian Gmeiner @ 2011-11-23 11:52 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-geode, linux-kernel

From 47df48b6fe482a108a0889bf2fdecfbde4aea8ee Mon Sep 17 00:00:00 2001
From: Christian Gmeiner <christian.gmeiner@gmail.com>
Date: Wed, 23 Nov 2011 12:44:14 +0100
Subject: [PATCH] lxfb: add support for LQ057V3DG01 panel

The LQ057V3DG01 panel needs some different timing
values so that the picture is centered.

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 drivers/video/geode/lxfb_core.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/video/geode/lxfb_core.c b/drivers/video/geode/lxfb_core.c
index 416851c..7b75bae 100644
--- a/drivers/video/geode/lxfb_core.c
+++ b/drivers/video/geode/lxfb_core.c
@@ -40,6 +40,10 @@ static struct fb_videomode geode_modedb[] __devinitdata = {
 	{ NULL, 60, 640, 480, 39682, 48, 8, 25, 2, 88, 2,
 	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	  FB_VMODE_NONINTERLACED, 0 },
+	/* 640x480-60 adapted for LQ057V3DG01 panel */
+	{ "LQ057V3DG01", 60, 640, 480, 39682, 48, 16, 34, 10, 88, 2,
+	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
+	  FB_VMODE_NONINTERLACED, 0 },
 	/* 640x400-70 */
 	{ NULL, 70, 640, 400, 39770, 40, 8, 28, 5, 96, 2,
 	  FB_SYNC_HOR_HIGH_ACT,
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH 42/65] OMAPDSS: DSI: call mgr_enable/disable for cmd mode
From: Archit Taneja @ 2011-11-23 12:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1322047652.28917.64.camel@deskari>

On Wednesday 23 November 2011 04:57 PM, Tomi Valkeinen wrote:
> On Wed, 2011-11-23 at 16:38 +0530, Archit Taneja wrote:
>>
>> I think it would be best to stuff the 'video mode enabling and
>> manager
>> enabling' functionality in omapdss_dsi_display_enable() itself, the
>> panel driver shouldn't need to call a function separately to enable
>> video mode for the panel. This way we would be more along the lines
>> of
>> the dpi driver, where dpi_display_enable() enables the manager in the
>> end.
>
> But we need to configure the panel between enabling the DSI interface
> and enabling the video output, so we can't combine those two functions.

Oh okay, that's right, we can't start video mode before preparing the panel.

Archit

>
> For DPI things are simpler, as enabling the interface and the video
> output are more or less the same thing.
>
>   Tomi
>


^ permalink raw reply

* Re: [RFC] Virtual CRTCs (proposal + experimental code)
From: Ilija Hadzic @ 2011-11-24  5:59 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel
In-Reply-To: <CAPM=9tyPOkS7w3593-3ZtjSN6E+cO82125Lpmn2er8r9=PzROQ@mail.gmail.com>



On Wed, 23 Nov 2011, Dave Airlie wrote:

> So another question I have is how you would intend this to work from a
> user POV, like how it would integrate with a desktop environment, X or
> wayland, i.e. with little or no configuration.
>

First thing to understand is that when a virtual CRTC is created, it looks 
to the user like the GPU has an additional DisplayPort connector.
At present I "abuse" DisplayPort, but I have seen that you pushed a patch 
from VMware that adds Virtual connector, so eventually I'll switch to that 
naming. The number of virtual CRTCs is determined when the driver loads 
and that is a static configuration parameter. This does not restrict the 
user because unused virutal CRTCs are just like disconnected connectors on 
the GPU. In extreme case, a user could max out the number of virtual CRTCs 
(i.e. 32 minus #-of-physical-CRTCs), but in general the system needs to be 
booted with maximum number of anticipated CRTCs. Run-time addition and 
removal of CRTCs is not supported at this time and that would be much 
harder to implement and would affect the whole DRM module everywhere.

So now we have a system that booted up and DRM sees all of its real 
connectors as well as virtual ones (as DisplayPorts at present). If there 
is no CTD device attached to virtual CRTCs, these virtual connectors are 
disconnected as far as DRM is concerned. Now the userspace must call 
"attach/fps" ioctl to associate CTDs with CRTCs. I'll explain shortly how 
to automate that and how to eliminate the burden from the user, but for 
now, please assume that "attach/fps" gets called from userland somehow.

When the attach happens, that is a hotplug event (VCRTCM generates it) to 
DRM, just like someone plugged in the monitor. Then when XOrg starts, it
will use the DisplayPort that represents a virtual CRTC just like any 
other connector. How it will use it, will depend on what the xorg.conf 
says, but the key point is that this connector is no different from any 
other connector that the GPU provides and is thus used as an "equal 
citizen". No special configuration is necessary once attached to CTD.

If CTD is detached and new CTD attached, that is just like yanking out 
monitor cable and plugging in the new one. DRM will get all hotplug events 
and windowing system will do the same thing it would normally do with any 
other port. If RANDR is called to resize the desktop it will also work and 
X will have no idea that one of the connectors is on a virtual CRTC. I 
also have another feature, where when CTD is attached, it can ask the 
device it drives for the connection status and propagate that all the way 
back to DRM (this is useful for CTD devices that drive real monitors, like 
DisplayLink).

So now let's go back to the attach/fps ioctl. To attach a CTD device this 
ioctl must happen as a result of some policy. That can be done by having 
CTD device generate UDEV events when it loads for which one can write 
policies to determine which CTD device attaches to which virtual CRTC.
Ultimately that becomes an user configuration, but it's no different from 
what one has to do now with UDEV policies to customize the system.

Having explained this, let's take your hotplug example that you put up on 
your web page and redo it with virtual CRTCs. Here is how it would work: 
Boot up the system and tell the GPU to create a few virtual CRTCs. Bring 
up Xorg with no DisplayLink dongles in it. Now plug in the DisplayLink. 
Once the CTD driver loads as the result of the hotplug (right now UDLCTD 
is a separate driver, but as we discussed before, this is a temporary 
state and at some point its CTD function should be merged wither with 
UDLFB or with your UDL-V2) CTD function in the driver generates an UDEV 
event. The policy directs UDEV to call run the program that issues the 
ioctl to perform attach/fps. Attach/fps of the UDLCTD is now a hotplug 
event and DRM "thinks" that a new connector changed the status from 
disconnected to connected. That causes it to query the modes for the new 
connector and because it's the virtual CRTC, it lands in the virtual CRTC 
helpers in the GPU driver. Virtual CRTC helpers route it to VCRTCM, which 
further routes to it to CTD (UDLCTD in this case). CTD returns the modes 
and DRM gets them ... the rest you know better than me what happens ;-)

So this is your hotplug demo, but the difference is that the new desktop 
can use direct rendering. Also, everything that would work for a normal 
connector works here without having to do any additional tricks. RANDR 
also works seamlessly without having to do anything special. If you move 
away from Xorg, to some other system (Wayland?), it still works for as 
long as the new system knows how to deal with connectors that connect and 
disconnect.

Everything I described above is ready to go except the UDEV event from 
UDLCTD and UDEV rules to automate this. Both are straightforwar and won't 
take long to do. So very shortly, I'll be able to show the hotplug-bis.

From what you wrote in your blog, it sounds like this is exactly what you 
are looking for. I recognize that it disrupts your current views/plans on 
how this should be done, but I do want to work with you to find a suitable 
middle ground that covers most of the possiblities.

In case you are looking at my code to follow the above-described 
scenarios, please make sure you pull the latest stuff from my github 
repository. I have been pushing new stuff since my original annoucement.


> I still foresee problems with tiling, we generally don't encourage
> accel code to live in the kernel, and you'll really want a
> tiled->untiled blit for this thing,

Accel code should not go into the kernel (that I fully agree) and there is 
nothing here that would behove us to do so. Restricting my comments to 
Radeon GPU (which is the only one that I know well enough), shaders for 
blit copy live in the kernel and irrespective of VCRTCM work. I rely on 
them to move the frame buffer out of VRAM to CTD device but I don't add 
any additional features.

Now for detiling, I think that it should be the responsibility of the 
receiving CTD device, not the GPU pushing the data (Alan mentioned that 
during the initial set of comments, and although I didn't say anything to 
it that has been my view as well).

Even if you wanted to use GPU for detiling (which I'll explain shortly why 
you should not), it would not require any new accel code in the kernel. It 
would merely require one bit flip in the setup of blit copy that already 
lives in the kernel.

However, de-tiling in GPU is a bad idea for two reasons. I tried to do 
that just as an experiment on Radeon GPUs and watched with the PCI Express 
analyzer what happens on the bus (yeah, I have some "heavy weapons" in my 
lab). Normally a tile is a continuous array of memory locations in VRAM. 
If blit-copy function is told to assume tiled source and linear 
destination (de-tiling) it will read a continuous set of addresses in 
VRAM, but then scatter 8 rows of 8 pixels each on non-contignuous set of 
addresses of the destination. If the destination is the PCI-Express bus, 
it will result in 8 32-byte write transactions instead of 2 128-byte 
transactions per each tile. That will choke the throughput of the bus 
right there.

BTW, this is the crux of the blit-copy performance improvement that you 
got from me back in October. Since blit-copy deals with copying a linear 
array, playing with tiled/non-tiled bits only affects the order in which 
addresses are accessed, so the trick was to get rid of short PCIe 
transactions and also shape up linear to rectangle mapping to make address 
pattern more friendly for the host.


> also for Intel GPUs where you have
> UMA, would you read from the UMA.
>

Yes the read would be from UMA. I have not yet looked at Intel GPUs in 
detail, so I don't have an answer for you on what problems would pop up 
and how to solve them, but I'll be glad to revisit the Intel discussion 
once I do some homework.

Some initial thoughts is that frame buffer in Intel are at the end of the 
day pages in the system memory, so anyone/anything can get to them if they 
are correctly mapped.


> It also doesn't solve the optimus GPU problem in any useful fashion,
> since it can't deal with all the use cases, so we still have to write
> an alternate solution that can deal with them, so we just end up with
> two answers.
>

Can you elaborate on some specific use cases that are of your concern? I 
have had this case in mind and I think I can make it work. First I would 
have to add CTD functionality to Intel driver. That should be 
straightforward. Once I get there, I'll be ready to experiment and we'll 
probably be in better position to discuss the specifics then (i.e. when we 
have something working to compare with what you did in PRIME experiemnt), 
but it would be good to know your specific concerns early.


thanks,

Ilija


^ permalink raw reply

* Re: [RFC] Virtual CRTCs (proposal + experimental code)
From: Dave Airlie @ 2011-11-24  8:52 UTC (permalink / raw)
  To: Ilija Hadzic; +Cc: linux-fbdev, dri-devel
In-Reply-To: <Pine.GSO.4.62.1111232223110.18154@umail>

On Thu, Nov 24, 2011 at 5:59 AM, Ilija Hadzic
<ihadzic@research.bell-labs.com> wrote:
>
>
> On Wed, 23 Nov 2011, Dave Airlie wrote:
>
>> So another question I have is how you would intend this to work from a
>> user POV, like how it would integrate with a desktop environment, X or
>> wayland, i.e. with little or no configuration.
>>
>
> First thing to understand is that when a virtual CRTC is created, it looks
> to the user like the GPU has an additional DisplayPort connector.
> At present I "abuse" DisplayPort, but I have seen that you pushed a patch
> from VMware that adds Virtual connector, so eventually I'll switch to that
> naming. The number of virtual CRTCs is determined when the driver loads and
> that is a static configuration parameter. This does not restrict the user
> because unused virutal CRTCs are just like disconnected connectors on the
> GPU. In extreme case, a user could max out the number of virtual CRTCs (i.e.
> 32 minus #-of-physical-CRTCs), but in general the system needs to be booted
> with maximum number of anticipated CRTCs. Run-time addition and removal of
> CRTCs is not supported at this time and that would be much harder to
> implement and would affect the whole DRM module everywhere.
>
> So now we have a system that booted up and DRM sees all of its real
> connectors as well as virtual ones (as DisplayPorts at present). If there is
> no CTD device attached to virtual CRTCs, these virtual connectors are
> disconnected as far as DRM is concerned. Now the userspace must call
> "attach/fps" ioctl to associate CTDs with CRTCs. I'll explain shortly how to
> automate that and how to eliminate the burden from the user, but for now,
> please assume that "attach/fps" gets called from userland somehow.
>
> When the attach happens, that is a hotplug event (VCRTCM generates it) to
> DRM, just like someone plugged in the monitor. Then when XOrg starts, it
> will use the DisplayPort that represents a virtual CRTC just like any other
> connector. How it will use it, will depend on what the xorg.conf says, but
> the key point is that this connector is no different from any other
> connector that the GPU provides and is thus used as an "equal citizen". No
> special configuration is necessary once attached to CTD.
>
> If CTD is detached and new CTD attached, that is just like yanking out
> monitor cable and plugging in the new one. DRM will get all hotplug events
> and windowing system will do the same thing it would normally do with any
> other port. If RANDR is called to resize the desktop it will also work and X
> will have no idea that one of the connectors is on a virtual CRTC. I also
> have another feature, where when CTD is attached, it can ask the device it
> drives for the connection status and propagate that all the way back to DRM
> (this is useful for CTD devices that drive real monitors, like DisplayLink).

Okay so thats pretty much how I expected it to work, I don't think
Virtual makes sense for a displaylink attached device though,
again if you were using a real driver you would just re-use whatever
output type it uses, though I'm not sure how well that works,

Do you propogate full EDID information and all the modes or just the
supported modes? we use this in userspace to put monitor names in
GNOME display settings etc.

what does xrandr output looks like for a radeon GPU with 4 vcrtcs? do
you see 4 disconnected connectors? that again isn't a pretty user
experience.

> So this is your hotplug demo, but the difference is that the new desktop can
> use direct rendering. Also, everything that would work for a normal
> connector works here without having to do any additional tricks. RANDR also
> works seamlessly without having to do anything special. If you move away
> from Xorg, to some other system (Wayland?), it still works for as long as
> the new system knows how to deal with connectors that connect and
> disconnect.

My main problem with this is as I'll explain below it only covers some
of the use cases, and I don't want a 50% solution at this point, by
doing something like this you are making it harder to get proper
support into something like wayland as they can ignore some of the
problems, however since this doesn't solve all the other problems it
means getting to a finished solution is actually less likely to
happen.

>> I still foresee problems with tiling, we generally don't encourage
>> accel code to live in the kernel, and you'll really want a
>> tiled->untiled blit for this thing,
>
> Accel code should not go into the kernel (that I fully agree) and there is
> nothing here that would behove us to do so. Restricting my comments to
> Radeon GPU (which is the only one that I know well enough), shaders for blit
> copy live in the kernel and irrespective of VCRTCM work. I rely on them to
> move the frame buffer out of VRAM to CTD device but I don't add any
> additional features.
>
> Now for detiling, I think that it should be the responsibility of the
> receiving CTD device, not the GPU pushing the data (Alan mentioned that
> during the initial set of comments, and although I didn't say anything to it
> that has been my view as well).

That is a pretty much a fundamental problem, there is no way you can
enumerate all the detiling necessary in the CTD device and there is no
way I'd want to merge that code to the kernel. r600 has 16 tiling
modes (we might only see 2 of these on scanout), r300->r500 have
another different set, r100->r200 have another different set, nouveau
has an major amount of modes, and intel has a full set and has crazy
memory configuration dependant swizzling, then gma500, etc. This just
won't be workable or scalable.

> Even if you wanted to use GPU for detiling (which I'll explain shortly why
> you should not), it would not require any new accel code in the kernel. It
> would merely require one bit flip in the setup of blit copy that already
> lives in the kernel.

That is fine for radeon, not so much for intel, nouveau etc.

> However, de-tiling in GPU is a bad idea for two reasons. I tried to do that
> just as an experiment on Radeon GPUs and watched with the PCI Express
> analyzer what happens on the bus (yeah, I have some "heavy weapons" in my
> lab). Normally a tile is a continuous array of memory locations in VRAM. If
> blit-copy function is told to assume tiled source and linear destination
> (de-tiling) it will read a continuous set of addresses in VRAM, but then
> scatter 8 rows of 8 pixels each on non-contignuous set of addresses of the
> destination. If the destination is the PCI-Express bus, it will result in 8
> 32-byte write transactions instead of 2 128-byte transactions per each tile.
> That will choke the throughput of the bus right there.

The thing is this is how optimus works, the nvidia gpus have an engine
that you can program to move data from the nvidia tiled VRAM format to
the intel main memory tiled format, and make if efficent. radeon's
also have some engines that AMD so far haven't told us about, but
someone with no NDA with AMD could easily start REing that sort of
thing.

>
> Yes the read would be from UMA. I have not yet looked at Intel GPUs in
> detail, so I don't have an answer for you on what problems would pop up and
> how to solve them, but I'll be glad to revisit the Intel discussion once I
> do some homework.

Probably a good idea to do some more research on intel/nvidia GPUs.
With intel you can't read back from UMA since it'll be uncached memory
so unuseable, so you'll need to use the GPU to detile and move to some
sort of cached linear area you can readback from.

>> It also doesn't solve the optimus GPU problem in any useful fashion,
>> since it can't deal with all the use cases, so we still have to write
>> an alternate solution that can deal with them, so we just end up with
>> two answers.
>>
>
> Can you elaborate on some specific use cases that are of your concern? I
> have had this case in mind and I think I can make it work. First I would
> have to add CTD functionality to Intel driver. That should be
> straightforward. Once I get there, I'll be ready to experiment and we'll
> probably be in better position to discuss the specifics then (i.e. when we
> have something working to compare with what you did in PRIME experiemnt),
> but it would be good to know your specific concerns early.
>

Switchable/Optimus mode has two modes of operation,

a) nvidia GPU is rendering engine and the intel GPU is just used as a
scanout buffer for the LVDS panel. This mode is used when an external
digital display is plugged in, or in some plugged in configurations.

b) intel GPU is primary rendering engine, and the nvidia gpu is used
as an offload engine. This mode is used when on battery or power
saving, with no external displays plugged in. You can completely turn
on/off the nvidia GPU.

Moving between a and b has to be completely dynamic, userspace apps
need to deal with the whole world changing beneath them.

There is also switchable graphics mode, where there is a MUX used to
switch the outputs between the two GPUs.

So the main problem with taking all this code on-board is it sort of
solves (a), and (b) needs another bunch of work. Now I'd rather not
solve 50% of the issue and have future userspace apps just think they
can ignore the problem. As much as I dislike the whole dual-gpu setups
the fact is they exist and we can't change that, so writing userspace
to ignore the problem because its too hard isn't going to work. So if
I merge this VCRTC stuff I give a lot of people an excuse for not
bothering to fix the harder problems that hotplug and dynamic GPUs put
in front of you.

Dave.
>
> thanks,
>
> Ilija
>
>

^ permalink raw reply

* Re: [PATCH v3 1/3] fbdev: Add FOURCC-based format configuration API
From: Laurent Pinchart @ 2011-11-24 10:50 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-fbdev, linux-media, magnus.damm
In-Reply-To: <201111201155.22948.laurent.pinchart@ideasonboard.com>

Hi Florian,

Gentle ping ?

On Sunday 20 November 2011 11:55:22 Laurent Pinchart wrote:
> On Sunday 20 November 2011 03:00:33 Florian Tobias Schandinat wrote:
> > Hi Laurent,
> > 
> > On 08/31/2011 11:18 AM, Laurent Pinchart wrote:
> > > This API will be used to support YUV frame buffer formats in a standard
> > > way.
> > 
> > looks like the union is causing problems. With this patch applied I get
> > 
> > errors like this:
> >   CC [M]  drivers/auxdisplay/cfag12864bfb.o
> > 
> > drivers/auxdisplay/cfag12864bfb.c:57: error: unknown field ‘red’
> > specified in initializer
> 
> *ouch*
> 
> gcc < 4.6 chokes on anonymous unions initializers :-/
> 
> [snip]
> 
> > > @@ -246,12 +251,23 @@ struct fb_var_screeninfo {
> > > 
> > >  	__u32 yoffset;			/* resolution			*/
> > >  	
> > >  	__u32 bits_per_pixel;		/* guess what			*/
> > > 
> > > -	__u32 grayscale;		/* != 0 Graylevels instead of colors */
> > > 
> > > -	struct fb_bitfield red;		/* bitfield in fb mem if true color, */
> > > -	struct fb_bitfield green;	/* else only length is significant */
> > > -	struct fb_bitfield blue;
> > > -	struct fb_bitfield transp;	/* transparency			*/
> > > +	union {
> > > +		struct {		/* Legacy format API		*/
> > > +			__u32 grayscale; /* 0 = color, 1 = grayscale	*/
> > > +			/* bitfields in fb mem if true color, else only */
> > > +			/* length is significant			*/
> > > +			struct fb_bitfield red;
> > > +			struct fb_bitfield green;
> > > +			struct fb_bitfield blue;
> > > +			struct fb_bitfield transp;	/* transparency	*/
> > > +		};
> > > +		struct {		/* FOURCC-based format API	*/
> > > +			__u32 fourcc;		/* FOURCC format	*/
> > > +			__u32 colorspace;
> > > +			__u32 reserved[11];
> > > +		} fourcc;
> > > +	};
> 
> We can't name the union, otherwise this will change the userspace API.
> 
> We could "fix" the problem on the kernel side with
> 
> #ifdef __KERNEL__
> 	} color;
> #else
> 	};
> #endif

(and the structure that contains the grayscale, red, green, blue and transp 
fields would need to be similarly named, the "rgb" name comes to mind)

> That's quite hackish though... What's your opinion ?
> 
> It would also not handle userspace code that initializes an
> fb_var_screeninfo structure with named initializers, but that shouldn't
> happen, as application should read fb_var_screeninfo , modify it and write
> it back.
> 
> > >  	__u32 nonstd;			/* != 0 Non standard pixel format */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [RFC] Virtual CRTCs (proposal + experimental code)
From: Daniel Vetter @ 2011-11-24 10:52 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel
In-Reply-To: <CAPM=9tx3ue8+PQ6QBxL_0Do2kLB01Q0yN-FOZrJkY2RKiyNm4Q@mail.gmail.com>

On Thu, Nov 24, 2011 at 08:52:45AM +0000, Dave Airlie wrote:
> So the main problem with taking all this code on-board is it sort of
> solves (a), and (b) needs another bunch of work. Now I'd rather not
> solve 50% of the issue and have future userspace apps just think they
> can ignore the problem. As much as I dislike the whole dual-gpu setups
> the fact is they exist and we can't change that, so writing userspace
> to ignore the problem because its too hard isn't going to work. So if
> I merge this VCRTC stuff I give a lot of people an excuse for not
> bothering to fix the harder problems that hotplug and dynamic GPUs put
> in front of you.

My 2 Rappen on this: I agree completely with your point that we should aim
for a full solution. GPU memory management across different devices is
hard, but solveable.

Furthermore I fear that a 50% solution that hides the memory management
and shuffling issues from userspace will end up being a leaky abstraction
(e.g. how and when is stuff transferred to the usb dp port, the kernel
might pin scanout buffers behind userspace's back screwing up the vram
accounting in userspace, random hotplugging of outputs ...). Also
v4l/embedded folks have similar issues (and the same tendency to just go
with a "simple" solution fitting their usecase) and with Intel dead-set on
entering the SoC market I'll have the joy to mess around with this stuff
pretty soon, too.

So I think we do have enough people interested in this and should be able
to cobble together something that does The Right Thing.

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply

* Re: [RFC] Virtual CRTCs (proposal + experimental code)
From: Alan Cox @ 2011-11-24 12:58 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, dri-devel
In-Reply-To: <CAPM=9tx3ue8+PQ6QBxL_0Do2kLB01Q0yN-FOZrJkY2RKiyNm4Q@mail.gmail.com>

> The thing is this is how optimus works, the nvidia gpus have an engine
> that you can program to move data from the nvidia tiled VRAM format to

This is even more of a special case than DisplayLink ;-)

> Probably a good idea to do some more research on intel/nvidia GPUs.
> With intel you can't read back from UMA since it'll be uncached memory
> so unuseable, so you'll need to use the GPU to detile and move to some
> sort of cached linear area you can readback from.

It's main memory so there are various ways to read it or pull it into
cached space.

> I merge this VCRTC stuff I give a lot of people an excuse for not
> bothering to fix the harder problems that hotplug and dynamic GPUs put
> in front of you.

I think both cases are slightly missing the mark, both are specialist
corner cases and once you add things like cameras to the mix that will
become even more painfully obvious.

The underlying need I think is a way to negotiate a shared buffer format
or pipeline between two devices. You also need in some cases to think
about shared fencing, and that is the bit that is really scary.

Figuring out the transform from A to B ('lets both use this buffer
format') or 'I can render then convert' is one thing. Dealing with two
GPUs firing into the same buffer while scanning it out I just pray
doesn't ever need shared fences.

Alan



^ permalink raw reply

* [PATCH 00/13] OMAPDSS: DSI fixes
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen

First a few patches to flush posted writes, and then DSI patches related to the
DSI lane handling. I'm not sure if the lane handling is still as clean as it
could be, but at least it feels cleaner than currently.

Tested entering ULPS on display disable on blaze tablet, and it seems to work.
However, I have no way to ensure that ULPS was actually entered, as I would
need a scope for that.

 Tomi

Tomi Valkeinen (13):
  OMAPDSS: DSI: flush posted write when entering ULPS
  OMAPDSS: DSI: flush posted write in send_bta
  OMAPDSS: DISPC: Flush posted writes when enabling outputs
  OMAPDSS: DSI: count with number of lanes
  OMAPDSS: DSI: Parse lane config
  OMAPDSS: DSI: Use new lane config in dsi_set_lane_config
  OMAPDSS: DSI: use lane config in dsi_get_lane_mask
  OMAPDSS: DSI: use lane config in dsi_cio_wait_tx_clk_esc_reset
  OMAPDSS: DSI: use lane config in dsi_cio_enable_lane_override
  OMAPDSS: DSI: remove dsi_get_num_lanes_used
  OMAPDSS: DSI: fix lane handling when entering ULPS
  OMAPDSS: DSI: improve wait_for_bit_change
  OMAPDSS: DSI: disable DDR_CLK_ALWAYS_ON when entering ULPS

 drivers/video/omap2/dss/dispc.c |   10 +-
 drivers/video/omap2/dss/dsi.c   |  429 +++++++++++++++++++++------------------
 2 files changed, 242 insertions(+), 197 deletions(-)

-- 
1.7.4.1


^ permalink raw reply

* [PATCH 01/13] OMAPDSS: DSI: flush posted write when entering ULPS
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Flush posted write between writing the ULPS enable bits and waiting for
the interrupt.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 5abf8e7..1331f92 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3561,6 +3561,9 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, (1 << 0) | (1 << 1) | (1 << 2),
 		7, 5);
 
+	/* flush posted write and wait for SCP interface to finish the write */
+	dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG2);
+
 	if (wait_for_completion_timeout(&completion,
 				msecs_to_jiffies(1000)) = 0) {
 		DSSERR("ULPS enable timeout\n");
@@ -3575,6 +3578,9 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, (0 << 0) | (0 << 1) | (0 << 2),
 		7, 5);
 
+	/* flush posted write and wait for SCP interface to finish the write */
+	dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG2);
+
 	dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_ULPS);
 
 	dsi_if_enable(dsidev, false);
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 02/13] OMAPDSS: DSI: flush posted write in send_bta
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Flush posted write after setting the bit to send the BTA to ensure the
BTA is sent right away, as the code in dsi_vc_send_bta_sync() waits for
an interrupt caused indirectly by sending the BTA.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 1331f92..4b1c074 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2913,6 +2913,9 @@ static int dsi_vc_send_bta(struct platform_device *dsidev, int channel)
 
 	REG_FLD_MOD(dsidev, DSI_VC_CTRL(channel), 1, 6, 6); /* BTA_EN */
 
+	/* flush posted write */
+	dsi_read_reg(dsidev, DSI_VC_CTRL(channel));
+
 	return 0;
 }
 
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 03/13] OMAPDSS: DISPC: Flush posted writes when enabling outputs
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Flush posted write in _enable_lcd_out() and _enable_digit_out(). This
ensures the the lcd/digit bit is written before the code starts waiting
for interrupts about enabling/disabling the output.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5c81533..5bbd838 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1916,10 +1916,14 @@ static void dispc_disable_isr(void *data, u32 mask)
 
 static void _enable_lcd_out(enum omap_channel channel, bool enable)
 {
-	if (channel = OMAP_DSS_CHANNEL_LCD2)
+	if (channel = OMAP_DSS_CHANNEL_LCD2) {
 		REG_FLD_MOD(DISPC_CONTROL2, enable ? 1 : 0, 0, 0);
-	else
+		/* flush posted write */
+		dispc_read_reg(DISPC_CONTROL2);
+	} else {
 		REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 0, 0);
+		dispc_read_reg(DISPC_CONTROL);
+	}
 }
 
 static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
@@ -1967,6 +1971,8 @@ static void dispc_mgr_enable_lcd_out(enum omap_channel channel, bool enable)
 static void _enable_digit_out(bool enable)
 {
 	REG_FLD_MOD(DISPC_CONTROL, enable ? 1 : 0, 1, 1);
+	/* flush posted write */
+	dispc_read_reg(DISPC_CONTROL);
 }
 
 static void dispc_mgr_enable_digit_out(bool enable)
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 04/13] OMAPDSS: DSI: count with number of lanes
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

DSI driver currently counts used lanes and number of supported lanes by
using the number of data lanes (i.e. excluding clock lane). This patch
changes this to use the number of all lanes so that the following lane
config patches are cleaner.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   49 ++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 4b1c074..6a110b2 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -327,7 +327,7 @@ struct dsi_data {
 	unsigned long  fint_min, fint_max;
 	unsigned long lpdiv_max;
 
-	int num_data_lanes;
+	unsigned num_lanes_supported;
 
 	unsigned scp_clk_refcount;
 };
@@ -2029,19 +2029,8 @@ static int dsi_cio_power(struct platform_device *dsidev,
 	return 0;
 }
 
-/* Number of data lanes present on DSI interface */
-static inline int dsi_get_num_data_lanes(struct platform_device *dsidev)
-{
-	/* DSI on OMAP3 doesn't have register DSI_GNQ, set number
-	 * of data lanes as 2 by default */
-	if (dss_has_feature(FEAT_DSI_GNQ))
-		return REG_GET(dsidev, DSI_GNQ, 11, 9);	/* NB_DATA_LANES */
-	else
-		return 2;
-}
-
-/* Number of data lanes used by the dss device */
-static inline int dsi_get_num_data_lanes_dssdev(struct omap_dss_device *dssdev)
+/* Number of lanes used by the dss device */
+static inline int dsi_get_num_lanes_used(struct omap_dss_device *dssdev)
 {
 	int num_data_lanes = 0;
 
@@ -2054,7 +2043,7 @@ static inline int dsi_get_num_data_lanes_dssdev(struct omap_dss_device *dssdev)
 	if (dssdev->phy.dsi.data4_lane != 0)
 		num_data_lanes++;
 
-	return num_data_lanes;
+	return num_data_lanes + 1;
 }
 
 static unsigned dsi_get_line_buf_size(struct platform_device *dsidev)
@@ -2092,7 +2081,7 @@ static void dsi_set_lane_config(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	u32 r;
-	int num_data_lanes_dssdev = dsi_get_num_data_lanes_dssdev(dssdev);
+	int num_lanes_used = dsi_get_num_lanes_used(dssdev);
 
 	int clk_lane   = dssdev->phy.dsi.clk_lane;
 	int data1_lane = dssdev->phy.dsi.data1_lane;
@@ -2108,14 +2097,14 @@ static void dsi_set_lane_config(struct omap_dss_device *dssdev)
 	r = FLD_MOD(r, data1_pol, 7, 7);
 	r = FLD_MOD(r, data2_lane, 10, 8);
 	r = FLD_MOD(r, data2_pol, 11, 11);
-	if (num_data_lanes_dssdev > 2) {
+	if (num_lanes_used > 3) {
 		int data3_lane  = dssdev->phy.dsi.data3_lane;
 		int data3_pol  = dssdev->phy.dsi.data3_pol;
 
 		r = FLD_MOD(r, data3_lane, 14, 12);
 		r = FLD_MOD(r, data3_pol, 15, 15);
 	}
-	if (num_data_lanes_dssdev > 3) {
+	if (num_lanes_used > 4) {
 		int data4_lane  = dssdev->phy.dsi.data4_lane;
 		int data4_pol  = dssdev->phy.dsi.data4_pol;
 
@@ -2247,7 +2236,7 @@ static void dsi_cio_enable_lane_override(struct omap_dss_device *dssdev,
 	int data4_pol  = dssdev->phy.dsi.data4_pol;
 
 	u32 l = 0;
-	u8 lptxscp_start = dsi->num_data_lanes = 2 ? 22 : 26;
+	u8 lptxscp_start = dsi->num_lanes_supported = 3 ? 22 : 26;
 
 	if (lanes & DSI_CLK_P)
 		l |= 1 << ((clk_lane - 1) * 2 + (clk_pol ? 0 : 1));
@@ -2385,7 +2374,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int r;
-	int num_data_lanes_dssdev = dsi_get_num_data_lanes_dssdev(dssdev);
+	int num_lanes_used = dsi_get_num_lanes_used(dssdev);
 	u32 l;
 
 	DSSDBGF();
@@ -2430,10 +2419,10 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 		 * manually.
 		 */
 
-		if (num_data_lanes_dssdev > 2)
+		if (num_lanes_used > 3)
 			lane_mask |= DSI_DATA3_P;
 
-		if (num_data_lanes_dssdev > 3)
+		if (num_lanes_used > 4)
 			lane_mask |= DSI_DATA4_P;
 
 		dsi_cio_enable_lane_override(dssdev, lane_mask);
@@ -3852,7 +3841,7 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)
 	unsigned ddr_clk_pre, ddr_clk_post;
 	unsigned enter_hs_mode_lat, exit_hs_mode_lat;
 	unsigned ths_eot;
-	int ndl = dsi_get_num_data_lanes_dssdev(dssdev);
+	int ndl = dsi_get_num_lanes_used(dssdev) - 1;
 	u32 r;
 
 	r = dsi_read_reg(dsidev, DSI_DSIPHY_CFG0);
@@ -4552,9 +4541,9 @@ int dsi_init_display(struct omap_dss_device *dssdev)
 		dsi->vdds_dsi_reg = vdds_dsi;
 	}
 
-	if (dsi_get_num_data_lanes_dssdev(dssdev) > dsi->num_data_lanes) {
-		DSSERR("DSI%d can't support more than %d data lanes\n",
-			dsi_module + 1, dsi->num_data_lanes);
+	if (dsi_get_num_lanes_used(dssdev) > dsi->num_lanes_supported) {
+		DSSERR("DSI%d can't support more than %d lanes\n",
+			dsi_module + 1, dsi->num_lanes_supported);
 		return -EINVAL;
 	}
 
@@ -4780,7 +4769,13 @@ static int omap_dsihw_probe(struct platform_device *dsidev)
 	dev_dbg(&dsidev->dev, "OMAP DSI rev %d.%d\n",
 	       FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
 
-	dsi->num_data_lanes = dsi_get_num_data_lanes(dsidev);
+	/* DSI on OMAP3 doesn't have register DSI_GNQ, set number
+	 * of data to 3 by default */
+	if (dss_has_feature(FEAT_DSI_GNQ))
+		/* NB_DATA_LANES */
+		dsi->num_lanes_supported = 1 + REG_GET(dsidev, DSI_GNQ, 11, 9);
+	else
+		dsi->num_lanes_supported = 3;
 
 	dsi_runtime_put(dsidev);
 
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 05/13] OMAPDSS: DSI: Parse lane config
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Parse the lane configuration from the device data into internal lane
config format. This will be used in the following patches to clean up
the lane handling.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   83 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 6a110b2..aea110c 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -203,6 +203,21 @@ struct dsi_reg { u16 idx; };
 typedef void (*omap_dsi_isr_t) (void *arg, u32 mask);
 
 #define DSI_MAX_NR_ISRS                2
+#define DSI_MAX_NR_LANES	5
+
+enum dsi_lane_function {
+	DSI_LANE_UNUSED	= 0,
+	DSI_LANE_CLK,
+	DSI_LANE_DATA1,
+	DSI_LANE_DATA2,
+	DSI_LANE_DATA3,
+	DSI_LANE_DATA4,
+};
+
+struct dsi_lane_config {
+	enum dsi_lane_function function;
+	u8 polarity;
+};
 
 struct dsi_isr_data {
 	omap_dsi_isr_t	isr;
@@ -329,6 +344,9 @@ struct dsi_data {
 
 	unsigned num_lanes_supported;
 
+	struct dsi_lane_config lanes[DSI_MAX_NR_LANES];
+	unsigned num_lanes_used;
+
 	unsigned scp_clk_refcount;
 };
 
@@ -2077,6 +2095,65 @@ static unsigned dsi_get_line_buf_size(struct platform_device *dsidev)
 	}
 }
 
+static int dsi_parse_lane_config(struct omap_dss_device *dssdev)
+{
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	u8 lanes[DSI_MAX_NR_LANES];
+	u8 polarities[DSI_MAX_NR_LANES];
+	int num_lanes, i;
+
+	static const enum dsi_lane_function functions[] = {
+		DSI_LANE_CLK,
+		DSI_LANE_DATA1,
+		DSI_LANE_DATA2,
+		DSI_LANE_DATA3,
+		DSI_LANE_DATA4,
+	};
+
+	lanes[0] = dssdev->phy.dsi.clk_lane;
+	lanes[1] = dssdev->phy.dsi.data1_lane;
+	lanes[2] = dssdev->phy.dsi.data2_lane;
+	lanes[3] = dssdev->phy.dsi.data3_lane;
+	lanes[4] = dssdev->phy.dsi.data4_lane;
+	polarities[0] = dssdev->phy.dsi.clk_pol;
+	polarities[1] = dssdev->phy.dsi.data1_pol;
+	polarities[2] = dssdev->phy.dsi.data2_pol;
+	polarities[3] = dssdev->phy.dsi.data3_pol;
+	polarities[4] = dssdev->phy.dsi.data4_pol;
+
+	num_lanes = 0;
+
+	for (i = 0; i < dsi->num_lanes_supported; ++i)
+		dsi->lanes[i].function = DSI_LANE_UNUSED;
+
+	for (i = 0; i < dsi->num_lanes_supported; ++i) {
+		int num;
+
+		if (lanes[i] = DSI_LANE_UNUSED)
+			break;
+
+		num = lanes[i] - 1;
+
+		if (num >= dsi->num_lanes_supported)
+			return -EINVAL;
+
+		if (dsi->lanes[num].function != DSI_LANE_UNUSED)
+			return -EINVAL;
+
+		dsi->lanes[num].function = functions[i];
+		dsi->lanes[num].polarity = polarities[i];
+		num_lanes++;
+	}
+
+	if (num_lanes < 2 || num_lanes > dsi->num_lanes_supported)
+		return -EINVAL;
+
+	dsi->num_lanes_used = num_lanes;
+
+	return 0;
+}
+
 static void dsi_set_lane_config(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
@@ -4328,6 +4405,12 @@ static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 	int dsi_module = dsi_get_dsidev_id(dsidev);
 	int r;
 
+	r = dsi_parse_lane_config(dssdev);
+	if (r) {
+		DSSERR("illegal lane config");
+		goto err0;
+	}
+
 	r = dsi_pll_init(dsidev, true, true);
 	if (r)
 		goto err0;
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 06/13] OMAPDSS: DSI: Use new lane config in dsi_set_lane_config
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Use the new lane config in dsi_set_lane_config().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   84 +++++++++++++++++++---------------------
 1 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index aea110c..ba8d6b3 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2154,59 +2154,53 @@ static int dsi_parse_lane_config(struct omap_dss_device *dssdev)
 	return 0;
 }
 
-static void dsi_set_lane_config(struct omap_dss_device *dssdev)
+static int dsi_set_lane_config(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	static const u8 offsets[] = { 0, 4, 8, 12, 16 };
+	static const enum dsi_lane_function functions[] = {
+		DSI_LANE_CLK,
+		DSI_LANE_DATA1,
+		DSI_LANE_DATA2,
+		DSI_LANE_DATA3,
+		DSI_LANE_DATA4,
+	};
 	u32 r;
-	int num_lanes_used = dsi_get_num_lanes_used(dssdev);
-
-	int clk_lane   = dssdev->phy.dsi.clk_lane;
-	int data1_lane = dssdev->phy.dsi.data1_lane;
-	int data2_lane = dssdev->phy.dsi.data2_lane;
-	int clk_pol    = dssdev->phy.dsi.clk_pol;
-	int data1_pol  = dssdev->phy.dsi.data1_pol;
-	int data2_pol  = dssdev->phy.dsi.data2_pol;
+	int i;
 
 	r = dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG1);
-	r = FLD_MOD(r, clk_lane, 2, 0);
-	r = FLD_MOD(r, clk_pol, 3, 3);
-	r = FLD_MOD(r, data1_lane, 6, 4);
-	r = FLD_MOD(r, data1_pol, 7, 7);
-	r = FLD_MOD(r, data2_lane, 10, 8);
-	r = FLD_MOD(r, data2_pol, 11, 11);
-	if (num_lanes_used > 3) {
-		int data3_lane  = dssdev->phy.dsi.data3_lane;
-		int data3_pol  = dssdev->phy.dsi.data3_pol;
-
-		r = FLD_MOD(r, data3_lane, 14, 12);
-		r = FLD_MOD(r, data3_pol, 15, 15);
+
+	for (i = 0; i < dsi->num_lanes_used; ++i) {
+		unsigned offset = offsets[i];
+		unsigned polarity, lane_number;
+		unsigned t;
+
+		for (t = 0; t < dsi->num_lanes_supported; ++t)
+			if (dsi->lanes[t].function = functions[i])
+				break;
+
+		if (t = dsi->num_lanes_supported)
+			return -EINVAL;
+
+		lane_number = t;
+		polarity = dsi->lanes[t].polarity;
+
+		r = FLD_MOD(r, lane_number + 1, offset + 2, offset);
+		r = FLD_MOD(r, polarity, offset + 3, offset + 3);
 	}
-	if (num_lanes_used > 4) {
-		int data4_lane  = dssdev->phy.dsi.data4_lane;
-		int data4_pol  = dssdev->phy.dsi.data4_pol;
 
-		r = FLD_MOD(r, data4_lane, 18, 16);
-		r = FLD_MOD(r, data4_pol, 19, 19);
+	/* clear the unused lanes */
+	for (; i < dsi->num_lanes_supported; ++i) {
+		unsigned offset = offsets[i];
+
+		r = FLD_MOD(r, 0, offset + 2, offset);
+		r = FLD_MOD(r, 0, offset + 3, offset + 3);
 	}
-	dsi_write_reg(dsidev, DSI_COMPLEXIO_CFG1, r);
 
-	/* The configuration of the DSI complex I/O (number of data lanes,
-	   position, differential order) should not be changed while
-	   DSS.DSI_CLK_CRTRL[20] LP_CLK_ENABLE bit is set to 1. In order for
-	   the hardware to take into account a new configuration of the complex
-	   I/O (done in DSS.DSI_COMPLEXIO_CFG1 register), it is recommended to
-	   follow this sequence: First set the DSS.DSI_CTRL[0] IF_EN bit to 1,
-	   then reset the DSS.DSI_CTRL[0] IF_EN to 0, then set
-	   DSS.DSI_CLK_CTRL[20] LP_CLK_ENABLE to 1 and finally set again the
-	   DSS.DSI_CTRL[0] IF_EN bit to 1. If the sequence is not followed, the
-	   DSI complex I/O configuration is unknown. */
+	dsi_write_reg(dsidev, DSI_COMPLEXIO_CFG1, r);
 
-	/*
-	REG_FLD_MOD(dsidev, DSI_CTRL, 1, 0, 0);
-	REG_FLD_MOD(dsidev, DSI_CTRL, 0, 0, 0);
-	REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 1, 20, 20);
-	REG_FLD_MOD(dsidev, DSI_CTRL, 1, 0, 0);
-	*/
+	return 0;
 }
 
 static inline unsigned ns2ddr(struct platform_device *dsidev, unsigned ns)
@@ -2473,7 +2467,9 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 		goto err_scp_clk_dom;
 	}
 
-	dsi_set_lane_config(dssdev);
+	r = dsi_set_lane_config(dssdev);
+	if (r)
+		goto err_scp_clk_dom;
 
 	/* set TX STOP MODE timer to maximum for this operation */
 	l = dsi_read_reg(dsidev, DSI_TIMING1);
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 07/13] OMAPDSS: DSI: use lane config in dsi_get_lane_mask
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Use the new lane config in dsi_get_lane_mask().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ba8d6b3..1a209a2 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2422,22 +2422,20 @@ static int dsi_cio_wait_tx_clk_esc_reset(struct omap_dss_device *dssdev)
 	return 0;
 }
 
+/* return bitmask of enabled lanes, lane0 being the lsb */
 static unsigned dsi_get_lane_mask(struct omap_dss_device *dssdev)
 {
-	unsigned lanes = 0;
+	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	unsigned mask = 0;
+	int i;
 
-	if (dssdev->phy.dsi.clk_lane != 0)
-		lanes |= 1 << (dssdev->phy.dsi.clk_lane - 1);
-	if (dssdev->phy.dsi.data1_lane != 0)
-		lanes |= 1 << (dssdev->phy.dsi.data1_lane - 1);
-	if (dssdev->phy.dsi.data2_lane != 0)
-		lanes |= 1 << (dssdev->phy.dsi.data2_lane - 1);
-	if (dssdev->phy.dsi.data3_lane != 0)
-		lanes |= 1 << (dssdev->phy.dsi.data3_lane - 1);
-	if (dssdev->phy.dsi.data4_lane != 0)
-		lanes |= 1 << (dssdev->phy.dsi.data4_lane - 1);
+	for (i = 0; i < dsi->num_lanes_supported; ++i) {
+		if (dsi->lanes[i].function != DSI_LANE_UNUSED)
+			mask |= 1 << i;
+	}
 
-	return lanes;
+	return mask;
 }
 
 static int dsi_cio_init(struct omap_dss_device *dssdev)
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 08/13] OMAPDSS: DSI: use lane config in dsi_cio_wait_tx_clk_esc_reset
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Use the new lane config in dsi_cio_wait_tx_clk_esc_reset(). This also
extends the function to support 5 lanes on OMAP4, instead of 3 lanes.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   47 +++++++++++++++-------------------------
 1 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 1a209a2..52cf97f 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2365,51 +2365,40 @@ static void dsi_cio_disable_lane_override(struct platform_device *dsidev)
 static int dsi_cio_wait_tx_clk_esc_reset(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-	int t;
-	int bits[3];
-	bool in_use[3];
-
-	if (dss_has_feature(FEAT_DSI_REVERSE_TXCLKESC)) {
-		bits[0] = 28;
-		bits[1] = 27;
-		bits[2] = 26;
-	} else {
-		bits[0] = 24;
-		bits[1] = 25;
-		bits[2] = 26;
-	}
-
-	in_use[0] = false;
-	in_use[1] = false;
-	in_use[2] = false;
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
+	int t, i;
+	bool in_use[DSI_MAX_NR_LANES];
+	static const u8 offsets_old[] = { 28, 27, 26 };
+	static const u8 offsets_new[] = { 24, 25, 26, 27, 28 };
+	const u8 *offsets;
+
+	if (dss_has_feature(FEAT_DSI_REVERSE_TXCLKESC))
+		offsets = offsets_old;
+	else
+		offsets = offsets_new;
 
-	if (dssdev->phy.dsi.clk_lane != 0)
-		in_use[dssdev->phy.dsi.clk_lane - 1] = true;
-	if (dssdev->phy.dsi.data1_lane != 0)
-		in_use[dssdev->phy.dsi.data1_lane - 1] = true;
-	if (dssdev->phy.dsi.data2_lane != 0)
-		in_use[dssdev->phy.dsi.data2_lane - 1] = true;
+	for (i = 0; i < dsi->num_lanes_supported; ++i)
+		in_use[i] = dsi->lanes[i].function != DSI_LANE_UNUSED;
 
 	t = 100000;
 	while (true) {
 		u32 l;
-		int i;
 		int ok;
 
 		l = dsi_read_reg(dsidev, DSI_DSIPHY_CFG5);
 
 		ok = 0;
-		for (i = 0; i < 3; ++i) {
-			if (!in_use[i] || (l & (1 << bits[i])))
+		for (i = 0; i < dsi->num_lanes_supported; ++i) {
+			if (!in_use[i] || (l & (1 << offsets[i])))
 				ok++;
 		}
 
-		if (ok = 3)
+		if (ok = dsi->num_lanes_supported)
 			break;
 
 		if (--t = 0) {
-			for (i = 0; i < 3; ++i) {
-				if (!in_use[i] || (l & (1 << bits[i])))
+			for (i = 0; i < dsi->num_lanes_supported; ++i) {
+				if (!in_use[i] || (l & (1 << offsets[i])))
 					continue;
 
 				DSSERR("CIO TXCLKESC%d domain not coming " \
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 09/13] OMAPDSS: DSI: use lane config in dsi_cio_enable_lane_override
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

Use the new lane config in dsi_cio_enable_lane_override(). The function
parameters are also slightly changed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   85 +++++++++++++----------------------------
 1 files changed, 27 insertions(+), 58 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 52cf97f..dee9525 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -238,19 +238,6 @@ enum dsi_vc_source {
 	DSI_VC_SOURCE_VP,
 };
 
-enum dsi_lane {
-	DSI_CLK_P	= 1 << 0,
-	DSI_CLK_N	= 1 << 1,
-	DSI_DATA1_P	= 1 << 2,
-	DSI_DATA1_N	= 1 << 3,
-	DSI_DATA2_P	= 1 << 4,
-	DSI_DATA2_N	= 1 << 5,
-	DSI_DATA3_P	= 1 << 6,
-	DSI_DATA3_N	= 1 << 7,
-	DSI_DATA4_P	= 1 << 8,
-	DSI_DATA4_N	= 1 << 9,
-};
-
 struct dsi_update_region {
 	u16 x, y, w, h;
 	struct omap_dss_device *device;
@@ -2290,49 +2277,28 @@ static void dsi_cio_timings(struct platform_device *dsidev)
 	dsi_write_reg(dsidev, DSI_DSIPHY_CFG2, r);
 }
 
+/* lane masks have lane 0 at lsb. mask_p for positive lines, n for negative */
 static void dsi_cio_enable_lane_override(struct omap_dss_device *dssdev,
-		enum dsi_lane lanes)
+		unsigned mask_p, unsigned mask_n)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
-	int clk_lane   = dssdev->phy.dsi.clk_lane;
-	int data1_lane = dssdev->phy.dsi.data1_lane;
-	int data2_lane = dssdev->phy.dsi.data2_lane;
-	int data3_lane = dssdev->phy.dsi.data3_lane;
-	int data4_lane = dssdev->phy.dsi.data4_lane;
-	int clk_pol    = dssdev->phy.dsi.clk_pol;
-	int data1_pol  = dssdev->phy.dsi.data1_pol;
-	int data2_pol  = dssdev->phy.dsi.data2_pol;
-	int data3_pol  = dssdev->phy.dsi.data3_pol;
-	int data4_pol  = dssdev->phy.dsi.data4_pol;
-
-	u32 l = 0;
+	int i;
+	u32 l;
 	u8 lptxscp_start = dsi->num_lanes_supported = 3 ? 22 : 26;
 
-	if (lanes & DSI_CLK_P)
-		l |= 1 << ((clk_lane - 1) * 2 + (clk_pol ? 0 : 1));
-	if (lanes & DSI_CLK_N)
-		l |= 1 << ((clk_lane - 1) * 2 + (clk_pol ? 1 : 0));
-
-	if (lanes & DSI_DATA1_P)
-		l |= 1 << ((data1_lane - 1) * 2 + (data1_pol ? 0 : 1));
-	if (lanes & DSI_DATA1_N)
-		l |= 1 << ((data1_lane - 1) * 2 + (data1_pol ? 1 : 0));
-
-	if (lanes & DSI_DATA2_P)
-		l |= 1 << ((data2_lane - 1) * 2 + (data2_pol ? 0 : 1));
-	if (lanes & DSI_DATA2_N)
-		l |= 1 << ((data2_lane - 1) * 2 + (data2_pol ? 1 : 0));
-
-	if (lanes & DSI_DATA3_P)
-		l |= 1 << ((data3_lane - 1) * 2 + (data3_pol ? 0 : 1));
-	if (lanes & DSI_DATA3_N)
-		l |= 1 << ((data3_lane - 1) * 2 + (data3_pol ? 1 : 0));
-
-	if (lanes & DSI_DATA4_P)
-		l |= 1 << ((data4_lane - 1) * 2 + (data4_pol ? 0 : 1));
-	if (lanes & DSI_DATA4_N)
-		l |= 1 << ((data4_lane - 1) * 2 + (data4_pol ? 1 : 0));
+	l = 0;
+
+	for (i = 0; i < dsi->num_lanes_supported; ++i) {
+		unsigned p = dsi->lanes[i].polarity;
+
+		if (mask_p & (1 << i))
+			l |= 1 << (i * 2 + (p ? 0 : 1));
+
+		if (mask_n & (1 << i))
+			l |= 1 << (i * 2 + (p ? 1 : 0));
+	}
+
 	/*
 	 * Bits in REGLPTXSCPDAT4TO0DXDY:
 	 * 17: DY0 18: DX0
@@ -2432,7 +2398,6 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	int r;
-	int num_lanes_used = dsi_get_num_lanes_used(dssdev);
 	u32 l;
 
 	DSSDBGF();
@@ -2467,7 +2432,8 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 	dsi_write_reg(dsidev, DSI_TIMING1, l);
 
 	if (dsi->ulps_enabled) {
-		u32 lane_mask = DSI_CLK_P | DSI_DATA1_P | DSI_DATA2_P;
+		unsigned mask_p;
+		int i;
 
 		DSSDBG("manual ulps exit\n");
 
@@ -2476,16 +2442,19 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 		 * ULPS exit sequence, as after reset the DSS HW thinks
 		 * that we are not in ULPS mode, and refuses to send the
 		 * sequence. So we need to send the ULPS exit sequence
-		 * manually.
+		 * manually by setting positive lines high and negative lines
+		 * low for 1ms.
 		 */
 
-		if (num_lanes_used > 3)
-			lane_mask |= DSI_DATA3_P;
+		mask_p = 0;
 
-		if (num_lanes_used > 4)
-			lane_mask |= DSI_DATA4_P;
+		for (i = 0; i < dsi->num_lanes_supported; ++i) {
+			if (dsi->lanes[i].function = DSI_LANE_UNUSED)
+				continue;
+			mask_p |= 1 << i;
+		}
 
-		dsi_cio_enable_lane_override(dssdev, lane_mask);
+		dsi_cio_enable_lane_override(dssdev, mask_p, 0);
 	}
 
 	r = dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_ON);
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 10/13] OMAPDSS: DSI: remove dsi_get_num_lanes_used
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

There's no longer need for the dsi_get_num_lanes_used function, so it
can be removed. The lane check in dsi_init_display() can be removed as
the validity of the config will be verified when parsing it.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   27 ++-------------------------
 1 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index dee9525..2b375c6 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2034,23 +2034,6 @@ static int dsi_cio_power(struct platform_device *dsidev,
 	return 0;
 }
 
-/* Number of lanes used by the dss device */
-static inline int dsi_get_num_lanes_used(struct omap_dss_device *dssdev)
-{
-	int num_data_lanes = 0;
-
-	if (dssdev->phy.dsi.data1_lane != 0)
-		num_data_lanes++;
-	if (dssdev->phy.dsi.data2_lane != 0)
-		num_data_lanes++;
-	if (dssdev->phy.dsi.data3_lane != 0)
-		num_data_lanes++;
-	if (dssdev->phy.dsi.data4_lane != 0)
-		num_data_lanes++;
-
-	return num_data_lanes + 1;
-}
-
 static unsigned dsi_get_line_buf_size(struct platform_device *dsidev)
 {
 	int val;
@@ -3863,6 +3846,7 @@ static int dsi_proto_config(struct omap_dss_device *dssdev)
 static void dsi_proto_timings(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	unsigned tlpx, tclk_zero, tclk_prepare, tclk_trail;
 	unsigned tclk_pre, tclk_post;
 	unsigned ths_prepare, ths_prepare_ths_zero, ths_zero;
@@ -3870,7 +3854,7 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)
 	unsigned ddr_clk_pre, ddr_clk_post;
 	unsigned enter_hs_mode_lat, exit_hs_mode_lat;
 	unsigned ths_eot;
-	int ndl = dsi_get_num_lanes_used(dssdev) - 1;
+	int ndl = dsi->num_lanes_used - 1;
 	u32 r;
 
 	r = dsi_read_reg(dsidev, DSI_DSIPHY_CFG0);
@@ -4554,7 +4538,6 @@ int dsi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
-	int dsi_module = dsi_get_dsidev_id(dsidev);
 
 	DSSDBG("DSI init\n");
 
@@ -4576,12 +4559,6 @@ int dsi_init_display(struct omap_dss_device *dssdev)
 		dsi->vdds_dsi_reg = vdds_dsi;
 	}
 
-	if (dsi_get_num_lanes_used(dssdev) > dsi->num_lanes_supported) {
-		DSSERR("DSI%d can't support more than %d lanes\n",
-			dsi_module + 1, dsi->num_lanes_supported);
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 11/13] OMAPDSS: DSI: fix lane handling when entering ULPS
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

The current code always enters ULPS for 3 lanes. This is not right, as
there could be only 2 lanes used, and on OMAP4 we have 5 lanes.

Fix the code to put all used lanes into ULPS.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 2b375c6..22055db 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3517,7 +3517,8 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 {
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	DECLARE_COMPLETION_ONSTACK(completion);
-	int r;
+	int r, i;
+	unsigned mask;
 
 	DSSDBGF();
 
@@ -3560,10 +3561,16 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 	if (r)
 		return r;
 
+	mask = 0;
+
+	for (i = 0; i < dsi->num_lanes_supported; ++i) {
+		if (dsi->lanes[i].function = DSI_LANE_UNUSED)
+			continue;
+		mask |= 1 << i;
+	}
 	/* Assert TxRequestEsc for data lanes and TxUlpsClk for clk lane */
 	/* LANEx_ULPS_SIG2 */
-	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, (1 << 0) | (1 << 1) | (1 << 2),
-		7, 5);
+	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, mask, 9, 5);
 
 	/* flush posted write and wait for SCP interface to finish the write */
 	dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG2);
@@ -3579,8 +3586,7 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 			DSI_CIO_IRQ_ULPSACTIVENOT_ALL0);
 
 	/* Reset LANEx_ULPS_SIG2 */
-	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, (0 << 0) | (0 << 1) | (0 << 2),
-		7, 5);
+	REG_FLD_MOD(dsidev, DSI_COMPLEXIO_CFG2, 0, 9, 5);
 
 	/* flush posted write and wait for SCP interface to finish the write */
 	dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG2);
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 12/13] OMAPDSS: DSI: improve wait_for_bit_change
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

We sometimes get timeout when disabling the DSI interface with video
mode. It looks like the disable will stall until the current frame has
been finished, and this can take multiple milliseconds.

wait_for_bit_change() currently uses a busyloop to wait for a bit to
change. This is used in multiple places. The problem is, we don't have
clear understanding how long particular operations can take, so the
function needs to support longer waits.

Improve wait_for_bit_change() to first busy loop for 100 times to see if
the bit changes almost right away. If the bit hasn't changed, move to a
loop with a sleep of 1ms, and poll for 500ms.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   25 ++++++++++++++++++++-----
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 22055db..a01a011 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -418,14 +418,29 @@ static void dsi_completion_handler(void *data, u32 mask)
 static inline int wait_for_bit_change(struct platform_device *dsidev,
 		const struct dsi_reg idx, int bitnum, int value)
 {
-	int t = 100000;
+	unsigned long timeout;
+	ktime_t wait;
+	int t;
+
+	/* first busyloop to see if the bit changes right away */
+	t = 100;
+	while (t-- > 0) {
+		if (REG_GET(dsidev, idx, bitnum, bitnum) = value)
+			return value;
+	}
+
+	/* then loop for 500ms, sleeping for 1ms in between */
+	timeout = jiffies + msecs_to_jiffies(500);
+	while (time_before(jiffies, timeout)) {
+		if (REG_GET(dsidev, idx, bitnum, bitnum) = value)
+			return value;
 
-	while (REG_GET(dsidev, idx, bitnum, bitnum) != value) {
-		if (--t = 0)
-			return !value;
+		wait = ns_to_ktime(1000 * 1000);
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		schedule_hrtimeout(&wait, HRTIMER_MODE_REL);
 	}
 
-	return value;
+	return !value;
 }
 
 u8 dsi_get_pixel_size(enum omap_dss_dsi_pixel_format fmt)
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 13/13] OMAPDSS: DSI: disable DDR_CLK_ALWAYS_ON when entering ULPS
From: Tomi Valkeinen @ 2011-11-24 13:29 UTC (permalink / raw)
  To: linux-fbdev, linux-omap; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1322141381-5395-1-git-send-email-tomi.valkeinen@ti.com>

ULPS cannot be entered if the DDR clk is enabled. Thus disable the DDR
clk before entering ULPS.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index a01a011..9bcb5c2 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -3544,9 +3544,11 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 	if (dsi->ulps_enabled)
 		return 0;
 
+	/* DDR_CLK_ALWAYS_ON */
 	if (REG_GET(dsidev, DSI_CLK_CTRL, 13, 13)) {
-		DSSERR("DDR_CLK_ALWAYS_ON enabled when entering ULPS\n");
-		return -EIO;
+		dsi_if_enable(dsidev, 0);
+		REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13);
+		dsi_if_enable(dsidev, 1);
 	}
 
 	dsi_sync_vc(dsidev, 0);
-- 
1.7.4.1


^ permalink raw reply related


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