Skip to contentSkip to conversation

FB: Add window positioning support

PATCH New Ajay Kumar 5 participants 15 messages Sep 20, 2011 – Sep 21, 2011

Subsystems (4)
lore
Share & tools
lore

Ajay Kumar

author
#0 lore raw

cover FB: Add window positioning support

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/[email protected]/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(-)
author
#1 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

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.

---
 include/linux/fb.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
include/linux/fb.h +7 -0
index 1d6836c..2141941 100644
@@ -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 */
+};
+
 /*
  * hardware cursor control
  */
Signature · 1 line
1.7.0.4
#2 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 24 lines · Ajay Kumar

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.

---
 include/linux/fb.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
include/linux/fb.h +1 -0
index 1d6836c..2141941 100644
@@ -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 */

include/linux/fb.h struct fb_image { · line 374

Quoted 8 lines
include/linux/fb.h +5 -0
@@ -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)?

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

#3 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Hi Tomi,

Quoted 28 lines · Tomi Valkeinen
Quoted 27 lines · Ajay Kumar

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.

---
?include/linux/fb.h | ? ?7 +++++++
?1 files changed, 7 insertions(+), 0 deletions(-)

include/linux/fb.h +0 -0
index 1d6836c..2141941 100644
@@ -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 */

include/linux/fb.h +0 -0
@@ -366,6 +367,12 @@ struct fb_image {

? ? ? struct fb_cmap cmap; ? ?/* color map info */ ?};

Quoted 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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.

Quoted 4 lines

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.

Quoted 2 lines

?Tomi

Ajay

#4 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 40 lines · Ajay kumar

Hi Tomi,

Quoted 28 lines · Tomi Valkeinen
Quoted 27 lines · Ajay Kumar

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.

---
 include/linux/fb.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
include/linux/fb.h +1 -0
index 1d6836c..2141941 100644
@@ -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 */
 };
 
Quoted 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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)?

Quoted 7 lines

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 10 lines
Quoted 4 lines

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

#5 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 8 lines · Tomi Valkeinen
Quoted 7 lines · Ajay kumar

Hi Tomi,

Quoted 4 lines · Tomi Valkeinen
Quoted 3 lines · Ajay Kumar

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.

Quoted 41 lines
Quoted 33 lines
Quoted 24 lines
Quoted 24 lines
---
 include/linux/fb.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
include/linux/fb.h +1 -0
index 1d6836c..2141941 100644
@@ -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 */
 };
 
Quoted 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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)?

Quoted 7 lines

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.

Quoted 4 lines

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 11 lines
Quoted 10 lines
Quoted 4 lines

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.

Quoted 4 lines

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 9 lines

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 4 lines

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

#6 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 2 lines · Florian Tobias Schandinat

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.

Quoted 2 lines

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.

Quoted 2 lines

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?

Quoted 4 lines

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.

Quoted 2 lines

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...

Quoted 7 lines

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

#7 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

In reply to Florian Tobias Schandinat

Hi Florian,

Quoted 10 lines · Florian Tobias Schandinat
Quoted 8 lines · Tomi Valkeinen
Quoted 7 lines · Ajay kumar

Hi Tomi,

Quoted 4 lines · Tomi Valkeinen
Quoted 3 lines · Ajay Kumar

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.

Quoted 5 lines

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.

Quoted 46 lines
Quoted 41 lines
Quoted 33 lines
Quoted 24 lines
Quoted 24 lines

---
?include/linux/fb.h | ? ?7 +++++++
?1 files changed, 7 insertions(+), 0 deletions(-)

include/linux/fb.h +0 -0
index 1d6836c..2141941 100644
@@ -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 */

include/linux/fb.h +0 -0
@@ -366,6 +367,12 @@ struct fb_image {

? ? ? struct fb_cmap cmap; ? ?/* color map info */ ?};

Quoted 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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)?

Quoted 7 lines

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.

Quoted 4 lines

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 4 lines

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 52 lines
Quoted 11 lines
Quoted 10 lines
Quoted 4 lines

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.

Quoted 4 lines

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 9 lines

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 4 lines

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.

Quoted 8 lines

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

Ajay

Quoted 6 lines

[1] http://linuxtv.org/downloads/v4l-dvb-apis/media_common.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

#8 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

In reply to Ajay Kumar

Hi Ajay,

Quoted 3 lines · Ajay Kumar

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]

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#9 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Hi Baruch,

Quoted 9 lines · Baruch Siach

Hi Ajay,

Quoted 3 lines · Ajay Kumar

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 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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?

Quoted 11 lines

baruch

--
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ~. .~ ? Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= ? - baruch at 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 at vger.kernel.org
More majordomo info at ?http://vger.kernel.org/majordomo-info.html

Ajay

#10 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Hi Ajay,

Quoted 20 lines · Ajay kumar

Hi Baruch,

Quoted 9 lines · Baruch Siach

Hi Ajay,

Quoted 3 lines · Ajay Kumar

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 8 lines

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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?

Quoted 3 lines

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

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#11 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 23 lines · Baruch Siach

Hi Ajay,

Quoted 20 lines · Ajay kumar

Hi Baruch,

Quoted 17 lines · Baruch Siach

Hi Ajay,

Quoted 3 lines · Ajay Kumar

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]

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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?

Quoted 7 lines
Quoted 3 lines

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

#12 lore raw

PATCH 1/3 include: fb: Add definiton for window positioning structure

Quoted 37 lines · Tomi Valkeinen
Quoted 23 lines · Baruch Siach

Hi Ajay,

Quoted 20 lines · Ajay kumar

Hi Baruch,

Quoted 17 lines · Baruch Siach

Hi Ajay,

Quoted 3 lines · Ajay Kumar

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]

include/linux/fb.h struct fb_image { · line 374

Quoted 5 lines
include/linux/fb.h +5 -0
+/* 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?

Quoted 7 lines
Quoted 3 lines

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.

Quoted 7 lines

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

author
#13 lore raw

PATCH 2/3 ARM: SAMSUNG: Embed window positioning data structure in s3c-fb plat data

In reply to Ajay Kumar

This patch adds a new field in s3c_fb_pd_win structure, thereby allowing the user to place window at the desired position.

---
 arch/arm/plat-samsung/include/plat/fb.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/plat-samsung/include/plat/fb.h +3 -0
index 0fedf47..920decd 100644
@@ -27,6 +27,7 @@
  * @win_mode: The display parameters to initialise (not for window 0)
  * @virtual_x: The virtual X size.
  * @virtual_y: The virtual Y size.
+ * @winpos: Position of overlayed window w.r.t the LCD screen.
  */
 struct s3c_fb_pd_win {
 	struct fb_videomode	win_mode;
@@ -35,6 +36,8 @@ struct s3c_fb_pd_win {
 	unsigned short		max_bpp;
 	unsigned short		virtual_x;
 	unsigned short		virtual_y;
+
+	struct fb_overlay_win_pos	winpos;
 };
 
 /**
Signature · 1 line
1.7.0.4
author
#14 lore raw

PATCH 3/3 video: s3c-fb: Modify s3c-fb driver to support window positioning

In reply to Ajay Kumar

Positions the framebuffer window during driver initialization, using the platform data.It also adds an ioctl definition to change window position dynamically.

---
 drivers/video/s3c-fb.c |   37 ++++++++++++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 5 deletions(-)
drivers/video/s3c-fb.c +32 -5
index 0fda252..b087a12 100644
@@ -441,6 +441,7 @@ static int s3c_fb_set_par(struct fb_info *info)
 {
 	struct fb_var_screeninfo *var = &info->var;
 	struct s3c_fb_win *win = info->par;
+	struct fb_overlay_win_pos *winpos = &win->windata->winpos;
 	struct s3c_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,
 			unsigned long arg)
 {
 	struct s3c_fb_win *win = info->par;
+	struct fb_overlay_win_pos *winpos = &win->windata->winpos;
 	struct s3c_fb *sfb = win->parent;
-	int ret;
+	int ret = 0;
 	u32 crtc;
+	u32 data;
 
 	switch (cmd) {
 	case FBIO_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;
+	case FBIOPOS_OVERLAY_WIN:
+		if (copy_from_user(winpos, (u32 __user *)arg,
+					sizeof(struct fb_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;
 	}
Signature · 1 line
1.7.0.4