* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Baruch Siach @ 2011-09-20 11:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316532641-2657-2-git-send-email-ajaykumar.rs@samsung.com>
Hi Ajay,
On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
> This patch adds a data structure definiton to hold framebuffer windows/planes.
> An ioctl number is also added to provide user access
> to change window position dynamically.
[snip]
> +/* Window overlaying */
> +struct fb_overlay_win_pos {
> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
> +};
Why not allow negative offsets where the left or upper part of the framebuffer
is hidden?
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{ - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning structure
From: Ajay kumar @ 2011-09-20 14:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316517021.1949.13.camel@deskari>
Hi Tomi,
On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>> An ioctl number is also added to provide user access
>> to change window position dynamically.
>>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
>> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> include/linux/fb.h | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index 1d6836c..2141941 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -39,6 +39,7 @@
>> #define FBIOPUT_MODEINFO 0x4617
>> #define FBIOGET_DISPINFO 0x4618
>> #define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
>> +#define FBIOPOS_OVERLAY_WIN _IOW('F', 0x21, struct fb_overlay_win_pos)
>>
>> #define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
>> #define FB_TYPE_PLANES 1 /* Non interleaved planes */
>> @@ -366,6 +367,12 @@ struct fb_image {
>> struct fb_cmap cmap; /* color map info */
>> };
>>
>> +/* Window overlaying */
>> +struct fb_overlay_win_pos {
>> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
>> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
>> +};
>
> Shouldn't this also include the window size (in case scaling is
> supported)?
The "xres" and "yres" fields in fb_var_screeninfo are being used to
represent the size of the window (visible resolution). So we have,
win_pos_x: x-offset from LCD(0,0) where window starts.
win_pos_y: y-offset from LCD(0,0) where window starts.
(win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
(win_pos_y + yres) : y-offset from LCD(0,0) where window ends.
> This also won't work for setups where the same framebuffer is used by
> multiple overlays. For example, this is the case on OMAP when the same
> content is cloned to, say, LCD and TV, each of which is showing an
> overlay.
These x and y position are used to configure the display controller
(for LCD only) and not to alter the data in physical buffer
(framebuffer). Could you elaborate the above use case you have
mentioned and how adding the x and y offsets would not meet that
requirement.
> Tomi
>
Ajay
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning structure
From: Ajay kumar @ 2011-09-20 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110920112427.GB19695@sapphire.tkos.co.il>
Hi Baruch,
On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Ajay,
>
> On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>> An ioctl number is also added to provide user access
>> to change window position dynamically.
>
> [snip]
>
>> +/* Window overlaying */
>> +struct fb_overlay_win_pos {
>> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
>> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
>> +};
>
> Why not allow negative offsets where the left or upper part of the framebuffer
> is hidden?
Thanks for pointing it out. Are there drivers which place the overlay
windows such that some part of the window is hidden from being
displayed on the screen?
> baruch
>
> --
> ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{> - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Ajay
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Tomi Valkeinen @ 2011-09-20 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEC9eQOTNaMcUWeSFN-DCTYrCJsVaDAO7uOZNQM=efjVdqtU2g@mail.gmail.com>
On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
> Hi Tomi,
>
> On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
> >> This patch adds a data structure definiton to hold framebuffer windows/planes.
> >> An ioctl number is also added to provide user access
> >> to change window position dynamically.
> >>
> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> >> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
> >> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> include/linux/fb.h | 7 +++++++
> >> 1 files changed, 7 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/fb.h b/include/linux/fb.h
> >> index 1d6836c..2141941 100644
> >> --- a/include/linux/fb.h
> >> +++ b/include/linux/fb.h
> >> @@ -39,6 +39,7 @@
> >> #define FBIOPUT_MODEINFO 0x4617
> >> #define FBIOGET_DISPINFO 0x4618
> >> #define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
> >> +#define FBIOPOS_OVERLAY_WIN _IOW('F', 0x21, struct fb_overlay_win_pos)
> >>
> >> #define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
> >> #define FB_TYPE_PLANES 1 /* Non interleaved planes */
> >> @@ -366,6 +367,12 @@ struct fb_image {
> >> struct fb_cmap cmap; /* color map info */
> >> };
> >>
> >> +/* Window overlaying */
> >> +struct fb_overlay_win_pos {
> >> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
> >> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
> >> +};
> >
> > Shouldn't this also include the window size (in case scaling is
> > supported)?
>
> The "xres" and "yres" fields in fb_var_screeninfo are being used to
> represent the size of the window (visible resolution). So we have,
>
> win_pos_x: x-offset from LCD(0,0) where window starts.
> win_pos_y: y-offset from LCD(0,0) where window starts.
> (win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
> (win_pos_y + yres) : y-offset from LCD(0,0) where window ends.
Sure, but the xres and yres tell the _input_ resolution, i.e. how many
pixels are read from the memory. What is missing is the _output_
resolution, which is the size of the window. These are not necessarily
the same, if the system supports scaling.
> > This also won't work for setups where the same framebuffer is used by
> > multiple overlays. For example, this is the case on OMAP when the same
> > content is cloned to, say, LCD and TV, each of which is showing an
> > overlay.
>
> These x and y position are used to configure the display controller
> (for LCD only) and not to alter the data in physical buffer
> (framebuffer). Could you elaborate the above use case you have
> mentioned and how adding the x and y offsets would not meet that
> requirement.
Nothing wrong with adding x/y offsets, but the problem is in configuring
the two overlays. If the framebuffer data is used by two overlays, each
overlay should be configured separately. And your ioctl does not have
any way to define which overlay is being affected.
Of course, if we specify that a single framebuffer will ever go only to
one output, the problem disappears.
However, even if we specify so, this will make the fbdev a bit weird:
what is x/yres after this patch? In the current fbdev x/yres is the size
of the output, and x/yres are part of video timings. After this patch
this is no longer the case: x/yres will be the size of the overlay. But
the old code will still use x/yres as part of video timings, making
things confusing.
And generally I can't really make my mind about adding these more
complex features. On one hand it would be very nice to have fbdev
supporting overlays and whatnot, but on the other hand, I can't figure
out how to add them properly.
Tomi
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-20 15:55 UTC (permalink / raw)
To: Patrik Jakobsson, Tomi Valkeinen
Cc: Alan Cox, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
Clark, Rob, Archit Taneja
In-Reply-To: <CAMeQTsbP-_ut7BA3YQ+qgtqfM1tzM4qn3jxHT8B8_0hdPbP3Jg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
On Tue, 20 Sep 2011 10:29:23 +0200, Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote:
> It would be nice to have a model that fits both DSI and SDVO, and the option
> to configure some of it from userspace.
> I thought the purpose of drm_encoder was to abstract hardware like this?
SDVO is entirely hidden by the drm_encoder interface; some of the
controls (like TV encoder parameters) are exposed through DRM
properties, others are used in the basic configuration of the device.
I'm not sure we need a new abstraction that subsumes both DSI and SDVO,
but we may need a DSI library that can be used by both DRM and other
parts of the kernel.
--
keith.packard@intel.com
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Florian Tobias Schandinat @ 2011-09-20 16:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1316533193.18781.12.camel@deskari>
On 09/20/2011 03:39 PM, Tomi Valkeinen wrote:
> On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
>> Hi Tomi,
>>
>> On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
>>>> This patch adds a data structure definiton to hold framebuffer windows/planes.
>>>> An ioctl number is also added to provide user access
>>>> to change window position dynamically.
Ajay, do you need this urgently or can we delay this one merge window? I don't
think that a week or so is enough to get a consistent API that gets everything
right. So if you have a pressing need to have it within the 3.2 kernel I'd
prefer to do it only for your driver now and adjust it when we get the thing
done, probably in 3.3.
>>>>
>>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>>>> Signed-off-by: Banajit Goswami <banajit.g@samsung.com>
>>>> Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>> include/linux/fb.h | 7 +++++++
>>>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>>>> index 1d6836c..2141941 100644
>>>> --- a/include/linux/fb.h
>>>> +++ b/include/linux/fb.h
>>>> @@ -39,6 +39,7 @@
>>>> #define FBIOPUT_MODEINFO 0x4617
>>>> #define FBIOGET_DISPINFO 0x4618
>>>> #define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
>>>> +#define FBIOPOS_OVERLAY_WIN _IOW('F', 0x21, struct fb_overlay_win_pos)
>>>>
>>>> #define FB_TYPE_PACKED_PIXELS 0 /* Packed Pixels */
>>>> #define FB_TYPE_PLANES 1 /* Non interleaved planes */
>>>> @@ -366,6 +367,12 @@ struct fb_image {
>>>> struct fb_cmap cmap; /* color map info */
>>>> };
>>>>
>>>> +/* Window overlaying */
>>>> +struct fb_overlay_win_pos {
>>>> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
>>>> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
>>>> +};
>>>
>>> Shouldn't this also include the window size (in case scaling is
>>> supported)?
>>
>> The "xres" and "yres" fields in fb_var_screeninfo are being used to
>> represent the size of the window (visible resolution). So we have,
>>
>> win_pos_x: x-offset from LCD(0,0) where window starts.
>> win_pos_y: y-offset from LCD(0,0) where window starts.
>> (win_pos_x + xres) : x-offset from LCD(0,0) where window ends.
>> (win_pos_y + yres) : y-offset from LCD(0,0) where window ends.
>
> Sure, but the xres and yres tell the _input_ resolution, i.e. how many
> pixels are read from the memory. What is missing is the _output_
> resolution, which is the size of the window. These are not necessarily
> the same, if the system supports scaling.
I agree, scaling is an issue that should get solved on the way. So adding
u32 width, height;
with an initial/special value of 0 which means just take what the source
width/height is.
>>> This also won't work for setups where the same framebuffer is used by
>>> multiple overlays. For example, this is the case on OMAP when the same
>>> content is cloned to, say, LCD and TV, each of which is showing an
>>> overlay.
>>
>> These x and y position are used to configure the display controller
>> (for LCD only) and not to alter the data in physical buffer
>> (framebuffer). Could you elaborate the above use case you have
>> mentioned and how adding the x and y offsets would not meet that
>> requirement.
>
> Nothing wrong with adding x/y offsets, but the problem is in configuring
> the two overlays. If the framebuffer data is used by two overlays, each
> overlay should be configured separately. And your ioctl does not have
> any way to define which overlay is being affected.
Did you have a look at the (existing) API [1] Laurent proposed for discovering
the internal connections between the framebuffers (or with any other devices)?
If you agree that it'd be a good idea to use it I feel that we should make the
windowing API more compatible with it. So basically what we want to have as a
window is one or more sunk pads so the pad-index should be also part of the
interface. I'm still confused with how OMAP works when it does not have a "root"
window/framebuffer. Normally I feel that the window position should be a
property of the parent window as this is what the position is relative too. But
if the parent is no framebuffer, should we also include the entity into the
interface to allow configuring things that are nor even framebuffers?
Also I think we need a z-index in case overlays overlap (might happen or?) and
enforcing that all z-indexes are different for the same entity.
> Of course, if we specify that a single framebuffer will ever go only to
> one output, the problem disappears.
>
> However, even if we specify so, this will make the fbdev a bit weird:
> what is x/yres after this patch? In the current fbdev x/yres is the size
> of the output, and x/yres are part of video timings. After this patch
> this is no longer the case: x/yres will be the size of the overlay. But
> the old code will still use x/yres as part of video timings, making
> things confusing.
As I see it xres/yres (together with xoffset/yoffset) is always the visible part
of the framebuffer. Typically that's also part of the timings as they define
what is visible. With the introduction of overlays (and maybe even for some
hardware anyway) it is no longer always true to have any timings at all. So on
all framebuffer that do not have physical timings the timing interpretation is
useless anyway (I'm thinking about adding a FB_CAP_NOTIMING) and what remains is
the interpretation of xres/yres as visible screen region.
> And generally I can't really make my mind about adding these more
> complex features. On one hand it would be very nice to have fbdev
> supporting overlays and whatnot, but on the other hand, I can't figure
> out how to add them properly.
I don't see it as adding new features, rather unifying what is already there for
easier use. Sure it should be done in a consistent way.
Best regards,
Florian Tobias Schandinat
[1] http://linuxtv.org/downloads/v4l-dvb-apis/media_common.html
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Baruch Siach @ 2011-09-20 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAEC9eQOpwfmRfcxHoiA00xX0a=dxXj1xJDpW14DozGXKXPJ3Xg@mail.gmail.com>
Hi Ajay,
On Tue, Sep 20, 2011 at 08:56:57PM +0530, Ajay kumar wrote:
> Hi Baruch,
> On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> > Hi Ajay,
> >
> > On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
> >> This patch adds a data structure definiton to hold framebuffer windows/planes.
> >> An ioctl number is also added to provide user access
> >> to change window position dynamically.
> >
> > [snip]
> >
> >> +/* Window overlaying */
> >> +struct fb_overlay_win_pos {
> >> + __u32 win_pos_x; /* x-offset from LCD(0,0) where window starts */
> >> + __u32 win_pos_y; /* y-offset from LCD(0,0) where window starts */
> >> +};
> >
> > Why not allow negative offsets where the left or upper part of the framebuffer
> > is hidden?
>
> Thanks for pointing it out. Are there drivers which place the overlay
> windows such that some part of the window is hidden from being
> displayed on the screen?
I don't know. However, since this is new userspace ABI which should stay
compatible forever, we should make sure to do it right. Using __s32 instead of
__u32 won't limit us in the future.
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{ - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH 1/3] include: fb: Add definiton for window positioning
From: Tomi Valkeinen @ 2011-09-20 18:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E78C579.9060305@gmx.de>
On Tue, 2011-09-20 at 16:55 +0000, Florian Tobias Schandinat wrote:
> Did you have a look at the (existing) API [1] Laurent proposed for discovering
> the internal connections between the framebuffers (or with any other devices)?
I know the basics of media controller, but I haven't really looked at
the code.
> If you agree that it'd be a good idea to use it I feel that we should make the
> windowing API more compatible with it. So basically what we want to have as a
I can't say if MC should be used for fb or not, but I think something
similar should be used if we want to get overlays etc. supported. But
even with MC (or something else) there, I'm not quite sure how they fit
into the fb model.
I think the core problem here is that in a more complex setup (at least
as I see it for OMAP) the fb should be just a framebuffer in memory,
without any direct relation to hardware. The hardware part (an overlay,
or whichever is the corresponding element) will then use the framebuffer
as the pixel source.
However, the current fb model combines those two things into one. If we
manage to separate them, and add MC or similar, I think it'll work.
> window is one or more sunk pads so the pad-index should be also part of the
> interface. I'm still confused with how OMAP works when it does not have a "root"
Do you mean how the hardware works, or how I've designed omapfb driver?
> window/framebuffer. Normally I feel that the window position should be a
> property of the parent window as this is what the position is relative too. But
> if the parent is no framebuffer, should we also include the entity into the
> interface to allow configuring things that are nor even framebuffers?
Right, I think you're pondering the core problem here =).
On OMAP we have the display (the whole area shown on the panel/tv),
which has video timings and things like that. But no pixel data as such
(a configurable background color is there, though).
And then we have the overlays, which are somewhere on the display, and
the overlays get pixel data from memory (framebuffers).
So in a way we have a contentless root window, but presenting that with
an fb device doesn't feel right. And the fb device would logically be
fb0, and if it couldn't show any content it couldn't be used as default
framebuffer.
> Also I think we need a z-index in case overlays overlap (might happen or?) and
> enforcing that all z-indexes are different for the same entity.
Yes, free z-order is supported in OMAP4. Previous OMAPs had fixed
z-order, although in certain configuration (enable/disable alpha
blending) the fixed z-order does change...
> As I see it xres/yres (together with xoffset/yoffset) is always the visible part
> of the framebuffer. Typically that's also part of the timings as they define
> what is visible. With the introduction of overlays (and maybe even for some
> hardware anyway) it is no longer always true to have any timings at all. So on
> all framebuffer that do not have physical timings the timing interpretation is
> useless anyway (I'm thinking about adding a FB_CAP_NOTIMING) and what remains is
> the interpretation of xres/yres as visible screen region.
For a system where there's always a root window for every output, plus
variable size overlays, FB_CAP_NOTIMING makes sense. But it would still
leave the problem if there's no root window. How to change timings on a
system like OMAP?
Tomi
^ permalink raw reply
* [PATCH v2 0/4] sh_mobile_meram cleanups and fixes
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
To: linux-fbdev
Hi,
Here's the second version of the sh_mobile_meram and cleanup fixes. Compared
to v1, I've incorporated Damian's comments in the register definitions (first
patch).
Laurent Pinchart (4):
fbdev: sh_mobile_meram: Replace hardcoded register values with macros
fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM
suspend
fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h
drivers/video/sh_mobile_lcdcfb.c | 2 +-
drivers/video/sh_mobile_meram.c | 125 ++++++++++++++++++++++++++------------
drivers/video/sh_mobile_meram.h | 33 ----------
3 files changed, 87 insertions(+), 73 deletions(-)
delete mode 100644 drivers/video/sh_mobile_meram.h
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 1/4] fbdev: sh_mobile_meram: Replace hardcoded register values with macros
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
To: linux-fbdev
Instead of hardcoding register values through the driver, define macros
for individual register bits using the register name and the bit name,
and use the macros.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_meram.c | 100 +++++++++++++++++++++++++++++----------
1 files changed, 74 insertions(+), 26 deletions(-)
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 39f28a1..569888c 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -20,22 +20,69 @@
#include "sh_mobile_meram.h"
/* meram registers */
-#define MExxCTL 0x0
-#define MExxBSIZE 0x4
-#define MExxMNCF 0x8
-#define MExxSARA 0x10
-#define MExxSARB 0x14
-#define MExxSBSIZE 0x18
-
-#define MERAM_MExxCTL_VAL(ctl, next_icb, addr) \
- ((ctl) | (((next_icb) & 0x1f) << 11) | (((addr) & 0x7ff) << 16))
-#define MERAM_MExxBSIZE_VAL(a, b, c) \
- (((a) << 28) | ((b) << 16) | (c))
-
-#define MEVCR1 0x4
-#define MEACTS 0x10
-#define MEQSEL1 0x40
-#define MEQSEL2 0x44
+#define MEVCR1 0x0004
+#define MEVCR1_RST (1 << 31)
+#define MEVCR1_WD (1 << 30)
+#define MEVCR1_AMD1 (1 << 29)
+#define MEVCR1_AMD0 (1 << 28)
+#define MEQSEL1 0x0040
+#define MEQSEL2 0x0044
+
+#define MExx_BASE 0x0400
+
+#define MExxCTL 0x0000
+#define MExxCTL_BV (1 << 31)
+#define MExxCTL_BSZ_SHIFT 28
+#define MExxCTL_MSAR_MASK (0x7ff << MExxCTL_MSAR_SHIFT)
+#define MExxCTL_MSAR_SHIFT 16
+#define MExxCTL_NXT_MASK (0x1f << MExxCTL_NXT_SHIFT)
+#define MExxCTL_NXT_SHIFT 11
+#define MExxCTL_WD1 (1 << 10)
+#define MExxCTL_WD0 (1 << 9)
+#define MExxCTL_WS (1 << 8)
+#define MExxCTL_CB (1 << 7)
+#define MExxCTL_WBF (1 << 6)
+#define MExxCTL_WF (1 << 5)
+#define MExxCTL_RF (1 << 4)
+#define MExxCTL_CM (1 << 3)
+#define MExxCTL_MD_READ (1 << 0)
+#define MExxCTL_MD_WRITE (2 << 0)
+#define MExxCTL_MD_ICB_WB (3 << 0)
+#define MExxCTL_MD_ICB (4 << 0)
+#define MExxCTL_MD_FB (7 << 0)
+#define MExxCTL_MD_MASK (7 << 0)
+#define MExxBSIZE 0x0004
+#define MExxBSIZE_RCNT_SHIFT 28
+#define MExxBSIZE_YSZM1_SHIFT 16
+#define MExxBSIZE_XSZM1_SHIFT 0
+#define MExxMNCF 0x0008
+#define MExxMNCF_KWBNM_SHIFT 28
+#define MExxMNCF_KRBNM_SHIFT 24
+#define MExxMNCF_BNM_SHIFT 16
+#define MExxMNCF_XBV (1 << 15)
+#define MExxMNCF_CPL_YCBCR444 (1 << 12)
+#define MExxMNCF_CPL_YCBCR420 (2 << 12)
+#define MExxMNCF_CPL_YCBCR422 (3 << 12)
+#define MExxMNCF_CPL_MSK (3 << 12)
+#define MExxMNCF_BL (1 << 2)
+#define MExxMNCF_LNM_SHIFT 0
+#define MExxSARA 0x0010
+#define MExxSARB 0x0014
+#define MExxSBSIZE 0x0018
+#define MExxSBSIZE_HDV (1 << 31)
+#define MExxSBSIZE_HSZ16 (0 << 28)
+#define MExxSBSIZE_HSZ32 (1 << 28)
+#define MExxSBSIZE_HSZ64 (2 << 28)
+#define MExxSBSIZE_HSZ128 (3 << 28)
+#define MExxSBSIZE_SBSIZZ_SHIFT 0
+
+#define MERAM_MExxCTL_VAL(next, addr) \
+ ((((next) << MExxCTL_NXT_SHIFT) & MExxCTL_NXT_MASK) | \
+ (((addr) << MExxCTL_MSAR_SHIFT) & MExxCTL_MSAR_MASK))
+#define MERAM_MExxBSIZE_VAL(rcnt, yszm1, xszm1) \
+ (((rcnt) << MExxBSIZE_RCNT_SHIFT) | \
+ ((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
+ ((xszm1) << MExxBSIZE_XSZM1_SHIFT))
static unsigned long common_regs[] = {
MEVCR1,
@@ -72,8 +119,8 @@ struct sh_mobile_meram_priv {
* MERAM/ICB access functions
*/
-#define MERAM_ICB_OFFSET(base, idx, off) \
- ((base) + (0x400 + ((idx) * 0x20) + (off)))
+#define MERAM_ICB_OFFSET(base, idx, off) \
+ (MExx_BASE + (base) + (off) + (idx) * 0x20)
static inline void meram_write_icb(void __iomem *base, int idx, int off,
unsigned long val)
@@ -308,17 +355,18 @@ static int meram_init(struct sh_mobile_meram_priv *priv,
/*
* Set MERAM for framebuffer
*
- * 0x70f: WD = 0x3, WS=0x1, CM=0x1, MDû mode
* we also chain the cache_icb and the marker_icb.
* we also split the allocated MERAM buffer between two ICBs.
*/
meram_write_icb(priv->base, icb->cache_icb, MExxCTL,
- MERAM_MExxCTL_VAL(0x70f, icb->marker_icb,
- icb->meram_offset));
+ MERAM_MExxCTL_VAL(icb->marker_icb, icb->meram_offset) |
+ MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+ MExxCTL_MD_FB);
meram_write_icb(priv->base, icb->marker_icb, MExxCTL,
- MERAM_MExxCTL_VAL(0x70f, icb->cache_icb,
- icb->meram_offset +
- icb->meram_size / 2));
+ MERAM_MExxCTL_VAL(icb->cache_icb, icb->meram_offset +
+ icb->meram_size / 2) |
+ MExxCTL_WD1 | MExxCTL_WD0 | MExxCTL_WS | MExxCTL_CM |
+ MExxCTL_MD_FB);
return 0;
}
@@ -507,7 +555,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
/* Reset ICB on resume */
if (icb_regs[k] = MExxCTL)
priv->icb_saved_regs[j * ICB_REGS_SIZE + k] - 0x70;
+ MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
}
}
return 0;
@@ -592,7 +640,7 @@ static int __devinit sh_mobile_meram_probe(struct platform_device *pdev)
/* initialize ICB addressing mode */
if (pdata->addr_mode = SH_MOBILE_MERAM_MODE1)
- meram_write_reg(priv->base, MEVCR1, 1 << 29);
+ meram_write_reg(priv->base, MEVCR1, MEVCR1_AMD1);
pm_runtime_enable(&pdev->dev);
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 2/4] fbdev: sh_mobile_meram: Validate ICB configuration outside mutex
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
To: linux-fbdev
Validate as much of the requested ICB configuration as possible outside
of the mutex-protected region when registering ICBs.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_meram.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 569888c..f7e77f6 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -413,24 +413,22 @@ static int sh_mobile_meram_register(struct sh_mobile_meram_info *pdata,
xres, yres, (!pixelformat) ? "yuv" : "rgb",
base_addr_y, base_addr_c);
- mutex_lock(&priv->lock);
-
/* we can't handle wider than 8192px */
if (xres > 8192) {
dev_err(&pdev->dev, "width exceeding the limit (> 8192).");
- error = -EINVAL;
- goto err;
- }
-
- if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
- dev_err(&pdev->dev, "no more ICB available.");
- error = -EINVAL;
- goto err;
+ return -EINVAL;
}
/* do we have at least one ICB config? */
if (cfg->icb[0].marker_icb < 0 || cfg->icb[0].cache_icb < 0) {
dev_err(&pdev->dev, "at least one ICB is required.");
+ return -EINVAL;
+ }
+
+ mutex_lock(&priv->lock);
+
+ if (priv->used_meram_cache_regions + 2 > SH_MOBILE_MERAM_ICB_NUM) {
+ dev_err(&pdev->dev, "no more ICB available.");
error = -EINVAL;
goto err;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 3/4] fbdev: sh_mobile_meram: Fix MExxCTL register save on runtime PM suspend
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
To: linux-fbdev
To reset the ICB on resume the MExxCTL register needs to be OR'ed with
MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF, no set to that value. Fix this.
This fixes corruption at the bottom of the display when resuming from
runtime PM.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_meram.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index f7e77f6..4ad083c 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -552,7 +552,7 @@ static int sh_mobile_meram_runtime_suspend(struct device *dev)
meram_read_icb(priv->base, j, icb_regs[k]);
/* Reset ICB on resume */
if (icb_regs[k] = MExxCTL)
- priv->icb_saved_regs[j * ICB_REGS_SIZE + k] + priv->icb_saved_regs[j * ICB_REGS_SIZE + k] | MExxCTL_WBF | MExxCTL_WF | MExxCTL_RF;
}
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH v2 4/4] fbdev: sh_mobile_meram: Remove unneeded sh_mobile_meram.h
From: Laurent Pinchart @ 2011-09-20 19:21 UTC (permalink / raw)
To: linux-fbdev
The drivers/video/sh_mobile_meram.h header contains unused definitions
and declarations. Move the only used macro to sh_mobile_meram.c, and
remove the header.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 2 +-
drivers/video/sh_mobile_meram.c | 5 +++--
drivers/video/sh_mobile_meram.h | 33 ---------------------------------
3 files changed, 4 insertions(+), 36 deletions(-)
delete mode 100644 drivers/video/sh_mobile_meram.h
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 0b7b492..088cb17 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -24,10 +24,10 @@
#include <linux/backlight.h>
#include <linux/gpio.h>
#include <video/sh_mobile_lcdc.h>
+#include <video/sh_mobile_meram.h>
#include <linux/atomic.h>
#include "sh_mobile_lcdcfb.h"
-#include "sh_mobile_meram.h"
#define SIDE_B_OFFSET 0x1000
#define MIRROR_OFFSET 0x2000
diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 4ad083c..48c1828 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -16,8 +16,7 @@
#include <linux/io.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
-
-#include "sh_mobile_meram.h"
+#include <video/sh_mobile_meram.h>
/* meram registers */
#define MEVCR1 0x0004
@@ -84,6 +83,8 @@
((yszm1) << MExxBSIZE_YSZM1_SHIFT) | \
((xszm1) << MExxBSIZE_XSZM1_SHIFT))
+#define SH_MOBILE_MERAM_ICB_NUM 32
+
static unsigned long common_regs[] = {
MEVCR1,
MEQSEL1,
diff --git a/drivers/video/sh_mobile_meram.h b/drivers/video/sh_mobile_meram.h
deleted file mode 100644
index 1615204..0000000
--- a/drivers/video/sh_mobile_meram.h
+++ /dev/null
@@ -1,33 +0,0 @@
-#ifndef __sh_mobile_meram_h__
-#define __sh_mobile_meram_h__
-
-#include <linux/mutex.h>
-#include <video/sh_mobile_meram.h>
-
-/*
- * MERAM private
- */
-
-#define MERAM_ICB_Y 0x1
-#define MERAM_ICB_C 0x2
-
-/* MERAM cache size */
-#define SH_MOBILE_MERAM_ICB_NUM 32
-
-#define SH_MOBILE_MERAM_CACHE_OFFSET(p) ((p) >> 16)
-#define SH_MOBILE_MERAM_CACHE_SIZE(p) ((p) & 0xffff)
-
-int sh_mobile_meram_alloc_icb(const struct sh_mobile_meram_cfg *cfg,
- int xres,
- int yres,
- unsigned int base_addr,
- int yuv_mode,
- int *marker_icb,
- int *out_pitch);
-
-void sh_mobile_meram_free_icb(int marker_icb);
-
-#define SH_MOBILE_MERAM_START(ind, ab) \
- (0xC0000000 | ((ab & 0x1) << 23) | ((ind & 0x1F) << 24))
-
-#endif /* !__sh_mobile_meram_h__ */
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 0/3] sh_mobile_lcdc cleanup and fixes
From: Laurent Pinchart @ 2011-09-20 19:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310766753-30314-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Florian,
On Friday 15 July 2011 23:52:30 Laurent Pinchart wrote:
> Hi everybody,
>
> When trying to understand the sh_mobile_lcdc driver, I found it hard to
> read statements that include hardcoded register values. I thus wrote a
> patch that replace them with macros, making the code more readable.
>
> While doing so, I found two potential issues in the driver. See patches 2
> and 3 for detailed explanations and fixes.
>
> Laurent Pinchart (3):
> fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
> fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
> fbdev: sh_mobile_lcdc: Compute clock pattern using divider
> denominator
Could you please pick these patches for v3.2 ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Turn dot clock on before resuming from runtime PM
From: Laurent Pinchart @ 2011-09-20 19:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310387754-23033-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Florian,
On Monday 11 July 2011 14:35:54 Laurent Pinchart wrote:
> Resuming from runtime PM restores all LCDC registers. If the dot clock
> is off at that time display panning information will be corrupted.
>
> Turn the dot clock on before resuming from runtime PM. Similarly,
> turn the clock off after suspending the LCDC.
Could you please pick this patch for v3.2 ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 0/3] sh_mobile_lcdc cleanup and fixes
From: Laurent Pinchart @ 2011-09-20 19:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310766753-30314-1-git-send-email-laurent.pinchart@ideasonboard.com>
On Tuesday 20 September 2011 21:30:15 Laurent Pinchart wrote:
> Hi Florian,
>
> On Friday 15 July 2011 23:52:30 Laurent Pinchart wrote:
> > Hi everybody,
> >
> > When trying to understand the sh_mobile_lcdc driver, I found it hard to
> > read statements that include hardcoded register values. I thus wrote a
> > patch that replace them with macros, making the code more readable.
> >
> > While doing so, I found two potential issues in the driver. See patches 2
> > and 3 for detailed explanations and fixes.
> >
> > Laurent Pinchart (3):
> > fbdev: sh_mobile_lcdc: Replace hardcoded register values with macros
> > fbdev: sh_mobile_lcdc: Don't acknowlege interrupts unintentionally
> > fbdev: sh_mobile_lcdc: Compute clock pattern using divider
> >
> > denominator
>
> Could you please pick these patches for v3.2 ?
Sorry, I meant to answer the next version of this patch set. Please pick v2
instead.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Turn dot clock on before resuming
From: Florian Tobias Schandinat @ 2011-09-20 19:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310387754-23033-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent,
On 09/20/2011 07:30 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> On Monday 11 July 2011 14:35:54 Laurent Pinchart wrote:
>> Resuming from runtime PM restores all LCDC registers. If the dot clock
>> is off at that time display panning information will be corrupted.
>>
>> Turn the dot clock on before resuming from runtime PM. Similarly,
>> turn the clock off after suspending the LCDC.
>
> Could you please pick this patch for v3.2 ?
I assume you meant the currently developed 3.1? (as 3.2 is the next one for
which the patch is lying in my fbdev-next anyway)
Uhm, I originally didn't plan to ask Linus pulling anything before getting rid
of the old stuff, but okay, will go through my patches in the next days and look
what else is appropriate unless Linus beats me and releases 3.1 (although I do
not consider this likely as long as kernel.org is still down)
Best regards,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Turn dot clock on before resuming from runtime PM
From: Laurent Pinchart @ 2011-09-20 19:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310387754-23033-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Florian,
On Tuesday 20 September 2011 21:52:36 Florian Tobias Schandinat wrote:
> On 09/20/2011 07:30 PM, Laurent Pinchart wrote:
> > On Monday 11 July 2011 14:35:54 Laurent Pinchart wrote:
> >> Resuming from runtime PM restores all LCDC registers. If the dot clock
> >> is off at that time display panning information will be corrupted.
> >>
> >> Turn the dot clock on before resuming from runtime PM. Similarly,
> >> turn the clock off after suspending the LCDC.
> >
> > Could you please pick this patch for v3.2 ?
>
> I assume you meant the currently developed 3.1? (as 3.2 is the next one for
> which the patch is lying in my fbdev-next anyway)
No, I meant v3.2. As I haven't received any pull notification in response to
the patch, and as I had no way to check your fbdev-next tree on kernel.org, I
just wanted to make sure the patch would be queued for v3.2.
Please ignore this e-mail (and the other similar ones I've just sent) if the
patches are already in your queue. And thank you for picking them.
> Uhm, I originally didn't plan to ask Linus pulling anything before getting
> rid of the old stuff, but okay, will go through my patches in the next
> days and look what else is appropriate unless Linus beats me and releases
> 3.1 (although I do not consider this likely as long as kernel.org is still
> down)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Turn dot clock on before resuming
From: Florian Tobias Schandinat @ 2011-09-20 20:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310387754-23033-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent,
On 09/20/2011 07:56 PM, Laurent Pinchart wrote:
> Hi Florian,
>
> On Tuesday 20 September 2011 21:52:36 Florian Tobias Schandinat wrote:
>> On 09/20/2011 07:30 PM, Laurent Pinchart wrote:
>>> On Monday 11 July 2011 14:35:54 Laurent Pinchart wrote:
>>>> Resuming from runtime PM restores all LCDC registers. If the dot clock
>>>> is off at that time display panning information will be corrupted.
>>>>
>>>> Turn the dot clock on before resuming from runtime PM. Similarly,
>>>> turn the clock off after suspending the LCDC.
>>>
>>> Could you please pick this patch for v3.2 ?
>>
>> I assume you meant the currently developed 3.1? (as 3.2 is the next one for
>> which the patch is lying in my fbdev-next anyway)
>
> No, I meant v3.2. As I haven't received any pull notification in response to
> the patch, and as I had no way to check your fbdev-next tree on kernel.org, I
> just wanted to make sure the patch would be queued for v3.2.
Just for your info, my tree [1] never was on kernel.org, you can see the patch
in it here [2] (webfrontend). Looks like the patch I got come from your pull
request "SH mobile LCDC cleanups and fixes", hope that's the right version.
Best regards,
Florian Tobias Schandinat
[1] git://github.com/schandinat/linux-2.6.git
[2]
https://github.com/schandinat/linux-2.6/commit/f1ad90da5c0fcb8841cc5e6d66c56f4005d8c960
^ permalink raw reply
* [patch 1/1] fb: fix potential deadlock between lock_fb_info and console_lock
From: akpm @ 2011-09-20 21:11 UTC (permalink / raw)
To: linux-fbdev
From: Andrea Righi <arighi@develer.com>
Subject: fb: fix potential deadlock between lock_fb_info and console_lock
fb_set_suspend() must be called with the console semaphore held, which
means the code path coming in here will first take the console_lock() and
then call lock_fb_info().
However several framebuffer ioctl commands acquire these locks in reverse
order (lock_fb_info() and then console_lock()). This gives rise to
potential AB-BA deadlock.
Fix this by changing the order of acquisition in the ioctl commands that
make use of console_lock().
Signed-off-by: Andrea Righi <arighi@develer.com>
Reported-by: Peter Nordström (Palm GBU) <peter.nordstrom@palm.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@google.com>
---
drivers/video/fbmem.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff -puN drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock drivers/video/fbmem.c
--- a/drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock
+++ a/drivers/video/fbmem.c
@@ -1076,14 +1076,16 @@ static long do_fb_ioctl(struct fb_info *
case FBIOPUT_VSCREENINFO:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_set_var(info, &var);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (!ret && copy_to_user(argp, &var, sizeof(var)))
ret = -EFAULT;
break;
@@ -1112,12 +1114,14 @@ static long do_fb_ioctl(struct fb_info *
case FBIOPAN_DISPLAY:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
ret = fb_pan_display(info, &var);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
return -EFAULT;
break;
@@ -1159,14 +1163,16 @@ static long do_fb_ioctl(struct fb_info *
unlock_fb_info(info);
break;
case FBIOBLANK:
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_blank(info, arg);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
default:
if (!lock_fb_info(info))
_
^ permalink raw reply
* Re: Proposal for a low-level Linux display framework
From: Patrik Jakobsson @ 2011-09-20 21:20 UTC (permalink / raw)
To: Keith Packard
Cc: Tomi Valkeinen, Alan Cox, linux-fbdev, linux-kernel, dri-devel,
linaro-dev, Clark, Rob, Archit Taneja
In-Reply-To: <yunpqivfbtf.fsf@aiko.keithp.com>
On Tue, Sep 20, 2011 at 5:55 PM, Keith Packard wrote:
> I'm not sure we need a new abstraction that subsumes both DSI and SDVO,
Ok. SDVO fits within the current abstraction, but I guess what I'm fishing
for is more code sharing of encoders. For instance, the SDVO code in GMA500
could be shared with i915. I'm currently working on copying the i915 SDVO code
over to GMA500, but sharing it would be even better.
> but we may need a DSI library that can be used by both DRM and other
> parts of the kernel.
Ok, not sure I understand the complexity of DSI. Can overlay composition
occur after/at the DSI stage (through MCS perhaps)? Or is it a matter of
panels requiring special scanout buffer formats that for instance V4L needs
to know about in order to overlay stuff properly? Or am I getting it all wrong?
Thanks
-Patrik
^ permalink raw reply
* Re: [PATCH 1/4] fbdev: sh_mobile_meram: Replace hardcoded register values with macros
From: Laurent Pinchart @ 2011-09-20 21:25 UTC (permalink / raw)
To: linux-fbdev
Hi Damien,
On Tuesday 02 August 2011 10:43:51 Damian Hobson-Garcia wrote:
Oops, late reply :-/
> > -#define MExxCTL 0x0
> > -#define MExxBSIZE 0x4
> > -#define MExxMNCF 0x8
> > -#define MExxSARA 0x10
> > -#define MExxSARB 0x14
> > -#define MExxSBSIZE 0x18
>
> [snip]
>
> > +#define MExxCTL 0x400
> > +#define MExxBSIZE 0x404
> > +#define MExxMNCF 0x408
> > +#define MExxSARA 0x410
> > +#define MExxSARB 0x414
> > +#define MExxSBSIZE 0x418
>
> One small comment on the definition of these register offsets and
> explanation of what I was originally thinking. Since each of these
> actually reprents a series of registers, one per ICB, (i.e. ME00CTL,
> ME01CTL, etc.) it makes sense to me to represent them as an offset from
> the base address (0x400) + the start of each ICB address (0x20 * index).
>
> Other than that, looks great.
Thanks. I've sent a v2.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [patch 1/1] fb: fix potential deadlock between lock_fb_info and
From: Florian Tobias Schandinat @ 2011-09-20 21:46 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <201109202111.p8KLBAJD019239@wpaz13.hot.corp.google.com>
Hi Andrew,
On 09/20/2011 09:11 PM, akpm@google.com wrote:
> From: Andrea Righi <arighi@develer.com>
> Subject: fb: fix potential deadlock between lock_fb_info and console_lock
>
> fb_set_suspend() must be called with the console semaphore held, which
> means the code path coming in here will first take the console_lock() and
> then call lock_fb_info().
>
> However several framebuffer ioctl commands acquire these locks in reverse
> order (lock_fb_info() and then console_lock()). This gives rise to
> potential AB-BA deadlock.
>
> Fix this by changing the order of acquisition in the ioctl commands that
> make use of console_lock().
I already have another patch [1] that fixes the same issue in a different way.
It looks less risky than yours and got more feedback.
Best regards,
Florian Tobias Schandinat
[1] http://marc.info/?l=linux-kernel&m\x130833638508657&w=2
>
> Signed-off-by: Andrea Righi <arighi@develer.com>
> Reported-by: Peter Nordström (Palm GBU) <peter.nordstrom@palm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: <stable@kernel.org>
>
> Signed-off-by: Andrew Morton <akpm@google.com>
> ---
>
> drivers/video/fbmem.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff -puN drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock drivers/video/fbmem.c
> --- a/drivers/video/fbmem.c~fb-fix-potential-deadlock-between-lock_fb_info-and-console_lock
> +++ a/drivers/video/fbmem.c
> @@ -1076,14 +1076,16 @@ static long do_fb_ioctl(struct fb_info *
> case FBIOPUT_VSCREENINFO:
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> info->flags |= FBINFO_MISC_USEREVENT;
> ret = fb_set_var(info, &var);
> info->flags &= ~FBINFO_MISC_USEREVENT;
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> if (!ret && copy_to_user(argp, &var, sizeof(var)))
> ret = -EFAULT;
> break;
> @@ -1112,12 +1114,14 @@ static long do_fb_ioctl(struct fb_info *
> case FBIOPAN_DISPLAY:
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> ret = fb_pan_display(info, &var);
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
> return -EFAULT;
> break;
> @@ -1159,14 +1163,16 @@ static long do_fb_ioctl(struct fb_info *
> unlock_fb_info(info);
> break;
> case FBIOBLANK:
> - if (!lock_fb_info(info))
> - return -ENODEV;
> console_lock();
> + if (!lock_fb_info(info)) {
> + console_unlock();
> + return -ENODEV;
> + }
> info->flags |= FBINFO_MISC_USEREVENT;
> ret = fb_blank(info, arg);
> info->flags &= ~FBINFO_MISC_USEREVENT;
> - console_unlock();
> unlock_fb_info(info);
> + console_unlock();
> break;
> default:
> if (!lock_fb_info(info))
> _
>
^ permalink raw reply
* Re: [patch 1/1] fb: fix potential deadlock between lock_fb_info and
From: Andrea Righi @ 2011-09-20 22:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <201109202111.p8KLBAJD019239@wpaz13.hot.corp.google.com>
On Tue, Sep 20, 2011 at 09:46:08PM +0000, Florian Tobias Schandinat wrote:
> Hi Andrew,
>
> On 09/20/2011 09:11 PM, akpm@google.com wrote:
> > From: Andrea Righi <arighi@develer.com>
> > Subject: fb: fix potential deadlock between lock_fb_info and console_lock
> >
> > fb_set_suspend() must be called with the console semaphore held, which
> > means the code path coming in here will first take the console_lock() and
> > then call lock_fb_info().
> >
> > However several framebuffer ioctl commands acquire these locks in reverse
> > order (lock_fb_info() and then console_lock()). This gives rise to
> > potential AB-BA deadlock.
> >
> > Fix this by changing the order of acquisition in the ioctl commands that
> > make use of console_lock().
>
> I already have another patch [1] that fixes the same issue in a different way.
> It looks less risky than yours and got more feedback.
>
>
> Best regards,
>
> Florian Tobias Schandinat
>
>
> [1] http://marc.info/?l=linux-kernel&m\x130833638508657&w=2
Looks better than my version.
For what it's worth it:
Reviewed-by: Andrea Righi <andrea@betterlinux.com>
Thanks,
-Andrea
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mobile_lcdc: Turn dot clock on before resuming from runtime PM
From: Laurent Pinchart @ 2011-09-20 22:44 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1310387754-23033-1-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Florian,
On Tuesday 20 September 2011 22:19:25 Florian Tobias Schandinat wrote:
> On 09/20/2011 07:56 PM, Laurent Pinchart wrote:
> > On Tuesday 20 September 2011 21:52:36 Florian Tobias Schandinat wrote:
> >> On 09/20/2011 07:30 PM, Laurent Pinchart wrote:
> >>> On Monday 11 July 2011 14:35:54 Laurent Pinchart wrote:
> >>>> Resuming from runtime PM restores all LCDC registers. If the dot clock
> >>>> is off at that time display panning information will be corrupted.
> >>>>
> >>>> Turn the dot clock on before resuming from runtime PM. Similarly,
> >>>> turn the clock off after suspending the LCDC.
> >>>
> >>> Could you please pick this patch for v3.2 ?
> >>
> >> I assume you meant the currently developed 3.1? (as 3.2 is the next one
> >> for which the patch is lying in my fbdev-next anyway)
> >
> > No, I meant v3.2. As I haven't received any pull notification in response
> > to the patch, and as I had no way to check your fbdev-next tree on
> > kernel.org, I just wanted to make sure the patch would be queued for
> > v3.2.
>
> Just for your info, my tree [1] never was on kernel.org, you can see the
> patch in it here [2] (webfrontend). Looks like the patch I got come from
> your pull request "SH mobile LCDC cleanups and fixes", hope that's the
> right version.
Oops, my bad.
I'm rebasing my pending patches on top of your fbdev/fbdev-next tree. There
are a couple of conflicts, I'll resent the rebased patches to the list.
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox