This patchset adds support for positioning the overlay windows on the LCD.
These patches are based on the discussion about adding features to fbdev at:
http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg06292.html
To Florian:
include: fb: Add definiton for window positioning structure
video: s3c-fb: Modify s3c-fb driver to support window positioning
To Kukjin,
ARM: SAMSUNG: Embed window positioning data structure in s3c-fb plat
data
arch/arm/plat-samsung/include/plat/fb.h | 3 ++
drivers/video/s3c-fb.c | 37 ++++++++++++++++++++++++++----
include/linux/fb.h | 7 ++++++
3 files changed, 42 insertions(+), 5 deletions(-)
From: Tomi Valkeinen <hidden> Date: 2011-09-20 11:10:21
On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
quoted hunk
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 <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/fb.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
@@ -366,6 +367,12 @@ struct fb_image {structfb_cmapcmap;/* color map info */};+/* Window overlaying */+structfb_overlay_win_pos{+__u32win_pos_x;/* x-offset from LCD(0,0) where window starts */+__u32win_pos_y;/* y-offset from LCD(0,0) where window starts */+};
Shouldn't this also include the window size (in case scaling is
supported)?
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.
Tomi
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 -
Hi Tomi,
On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen [off-list ref] wrote:
On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
quoted
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 <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/fb.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
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.
Hi Baruch,
On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach [off-list ref] wrote:
Hi Ajay,
On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
quoted
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]
quoted
+/* 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
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 <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/fb.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
@@ -366,6 +367,12 @@ struct fb_image {structfb_cmapcmap;/* color map info */};+/* Window overlaying */+structfb_overlay_win_pos{+__u32win_pos_x;/* x-offset from LCD(0,0) where window starts */+__u32win_pos_y;/* y-offset from LCD(0,0) where window starts */+};+/**hardwarecursorcontrol*/
This patch adds a new field in s3c_fb_pd_win structure,
thereby allowing the user to place window at the desired position.
Signed-off-by: Ajay Kumar <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
arch/arm/plat-samsung/include/plat/fb.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
Positions the framebuffer window during driver initialization,
using the platform data.It also adds an ioctl definition
to change window position dynamically.
Signed-off-by: Ajay Kumar <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/video/s3c-fb.c | 37 ++++++++++++++++++++++++++++++++-----
1 files changed, 32 insertions(+), 5 deletions(-)
@@ -441,6 +441,7 @@ static int s3c_fb_set_par(struct fb_info *info){structfb_var_screeninfo*var=&info->var;structs3c_fb_win*win=info->par;+structfb_overlay_win_pos*winpos=&win->windata->winpos;structs3c_fb*sfb=win->parent;void__iomem*regs=sfb->regs;void__iomem*buf=regs;
@@ -539,12 +540,13 @@ static int s3c_fb_set_par(struct fb_info *info)/* write 'OSD' registers to control position of framebuffer */-data=VIDOSDxA_TOPLEFT_X(0)|VIDOSDxA_TOPLEFT_Y(0);+data=VIDOSDxA_TOPLEFT_X(winpos->win_pos_x)|+VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y);writel(data,regs+VIDOSD_A(win_no,sfb->variant));-data=VIDOSDxB_BOTRIGHT_X(s3c_fb_align_word(var->bits_per_pixel,-var->xres-1))|-VIDOSDxB_BOTRIGHT_Y(var->yres-1);+data=VIDOSDxB_BOTRIGHT_X((s3c_fb_align_word(var->bits_per_pixel,+(winpos->win_pos_x+var->xres-1))))|+VIDOSDxB_BOTRIGHT_Y((winpos->win_pos_y+var->yres-1));writel(data,regs+VIDOSD_B(win_no,sfb->variant));
@@ -998,9 +1000,11 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,unsignedlongarg){structs3c_fb_win*win=info->par;+structfb_overlay_win_pos*winpos=&win->windata->winpos;structs3c_fb*sfb=win->parent;-intret;+intret=0;u32crtc;+u32data;switch(cmd){caseFBIO_WAITFORVSYNC:
@@ -1011,6 +1015,29 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,ret=s3c_fb_wait_for_vsync(sfb,crtc);break;+caseFBIOPOS_OVERLAY_WIN:+if(copy_from_user(winpos,(u32__user*)arg,+sizeof(structfb_overlay_win_pos))){+ret=-EFAULT;+break;+}++shadow_protect_win(win,1);++/* write 'OSD' registers to set position of the window */+data=VIDOSDxA_TOPLEFT_X(winpos->win_pos_x)|+VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y);+writel(data,sfb->regs+VIDOSD_A(win->index,sfb->variant));++data=VIDOSDxB_BOTRIGHT_X(+s3c_fb_align_word(info->var.bits_per_pixel,+(winpos->win_pos_x+info->var.xres-1)));+data|=VIDOSDxB_BOTRIGHT_Y(winpos->win_pos_y++info->var.yres-1);+writel(data,sfb->regs+VIDOSD_B(win->index,sfb->variant));++shadow_protect_win(win,0);+break;default:ret=-ENOTTY;}
From: Tomi Valkeinen <hidden> Date: 2011-09-20 15:39:53
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 [off-list ref] wrote:
quoted
On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
quoted
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 <redacted>
Signed-off-by: Banajit Goswami <redacted>
Suggested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
include/linux/fb.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
@@ -366,6 +367,12 @@ struct fb_image {structfb_cmapcmap;/* color map info */};+/* Window overlaying */+structfb_overlay_win_pos{+__u32win_pos_x;/* x-offset from LCD(0,0) where window starts */+__u32win_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.
quoted
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
On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
quoted
Hi Tomi,
On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen [off-list ref] wrote:
quoted
On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
quoted
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.
@@ -366,6 +367,12 @@ struct fb_image {structfb_cmapcmap;/* color map info */};+/* Window overlaying */+structfb_overlay_win_pos{+__u32win_pos_x;/* x-offset from LCD(0,0) where window starts */+__u32win_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.
quoted
quoted
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
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 [off-list ref] wrote:
quoted
Hi Ajay,
On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
quoted
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]
quoted
+/* 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 -
From: Tomi Valkeinen <hidden> Date: 2011-09-20 18:57:17
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
From: Tomi Valkeinen <hidden> Date: 2011-09-21 06:25:36
On Tue, 2011-09-20 at 20:08 +0300, Baruch Siach wrote:
Hi Ajay,
On Tue, Sep 20, 2011 at 08:56:57PM +0530, Ajay kumar wrote:
quoted
Hi Baruch,
On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach [off-list ref] wrote:
quoted
Hi Ajay,
On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
quoted
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]
quoted
+/* 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.
OMAP HW doesn't allow "funny" things like overlay being outside the
visible area, i.e. negative position or size larger than the display.
And my guess is that hardware rarely allow things like that, as it would
just complicate the hardware without any gain.
Out-of-display-overlays can of course be emulated by the software. But
I'm not sure if it's good to add the complexity in the driver layer, as
it could as well be handled in the userspace.
Then again, a signed value would be future safer ("just in case"), and
if the driver can just reject values it doesn't want to support, there's
no real harm there either.
Tomi
Hi Florian,
On Tue, Sep 20, 2011 at 10:25 PM, Florian Tobias Schandinat
[off-list ref] wrote:
On 09/20/2011 03:39 PM, Tomi Valkeinen wrote:
quoted
On Tue, 2011-09-20 at 20:16 +0530, Ajay kumar wrote:
quoted
Hi Tomi,
On Tue, Sep 20, 2011 at 4:40 PM, Tomi Valkeinen [off-list ref] wrote:
quoted
On Tue, 2011-09-20 at 11:30 -0400, Ajay Kumar wrote:
quoted
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.
No. I am not in a hurry, and I do not have any issue even if it takes
more time to get a consistent API.
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.
Do you mean to say the "width" and the "height" fields which you are
suggesting, will represent the "output resolution" which OMAP needs?
quoted
quoted
quoted
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.
quoted
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.
quoted
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
On Wed, Sep 21, 2011 at 11:55 AM, Tomi Valkeinen [off-list ref] wrote:
On Tue, 2011-09-20 at 20:08 +0300, Baruch Siach wrote:
quoted
Hi Ajay,
On Tue, Sep 20, 2011 at 08:56:57PM +0530, Ajay kumar wrote:
quoted
Hi Baruch,
On Tue, Sep 20, 2011 at 4:54 PM, Baruch Siach [off-list ref] wrote:
quoted
Hi Ajay,
On Tue, Sep 20, 2011 at 11:30:39AM -0400, Ajay Kumar wrote:
quoted
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]
quoted
+/* 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.
OMAP HW doesn't allow "funny" things like overlay being outside the
visible area, i.e. negative position or size larger than the display.
And my guess is that hardware rarely allow things like that, as it would
just complicate the hardware without any gain.
Out-of-display-overlays can of course be emulated by the software. But
I'm not sure if it's good to add the complexity in the driver layer, as
it could as well be handled in the userspace.
Then again, a signed value would be future safer ("just in case"), and
if the driver can just reject values it doesn't want to support, there's
no real harm there either.
OK. I will consider this and modify it in the next version of patches.
Ajay