Skip to contentSkip to conversation

OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

PATCH In review Grazvydas Ignotas 4 participants 7 messages Jul 2, 2010 – Aug 9, 2010 Framebuffer layer

lore
Share & tools
lore

Grazvydas Ignotas

author
#0 lore raw

PATCH 1/1 OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

FBIO_WAITFORVSYNC is a stardard ioctl for waiting vsync, already used by some userspace, so add it as an alias for OMAPFB_WAITFORVSYNC.

Signed-off-by Grazvydas Ignotas <[email protected]>

---
Tomi,

I know I already sent this earlier, but as now FBIO_WAITFORVSYNC is a standard ioctl, so why don't we support it? I know you might not like that omapfb_ioctl will have one standard ioctl handler between all OMAP specific ones, but that's something plenty of other drivers do.

 drivers/video/omap2/omapfb/omapfb-ioctl.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
drivers/video/omap2/omapfb/omapfb-ioctl.c +12 -0
index 9c73618..32fab5a 100644
@@ -490,6 +490,7 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 		struct omapfb_vram_info		vram_info;
 		struct omapfb_tearsync_info	tearsync_info;
 		struct omapfb_display_info	display_info;
+		u32				crt;
 	} p;
 
 	int r = 0;

@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 			r = -EFAULT;
 		break;
 
+	case FBIO_WAITFORVSYNC:
+		if (get_user(p.crt, (__u32 __user *)arg)) {
+			r = -EFAULT;
+			break;
+		}
+		if (p.crt != 0) {
+			r = -ENODEV;
+			break;
+		}
+		/* FALLTHROUGH */
+
 	case OMAPFB_WAITFORVSYNC:
 		DBG("ioctl WAITFORVSYNC\n");
 		if (!display) {
Signature · 1 line
1.6.3.3
#1 lore raw
Quoted 11 lines

-----Original Message-----
From: Grazvydas Ignotas [mailto:[email protected]]
Sent: Saturday, July 03, 2010 2:25 AM
To: [email protected]
Cc: [email protected]; Tomi Valkeinen; Hiremath, Vaibhav; Grazvydas Ignotas
Subject: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

FBIO_WAITFORVSYNC is a stardard ioctl for waiting vsync, already used by some userspace, so add it as an alias for OMAPFB_WAITFORVSYNC.

Quoted 9 lines
Signed-off-by Grazvydas Ignotas <[email protected]>

---
Tomi,

I know I already sent this earlier, but as now FBIO_WAITFORVSYNC is a standard ioctl, so why don't we support it? I know you might not like that omapfb_ioctl will have one standard ioctl handler between all OMAP specific ones, but that's something plenty of other drivers do.

[Hiremath, Vaibhav] We should always use standard interfaces wherever possible and I think Tomi will also be aligned on this.

Quoted 24 lines
 drivers/video/omap2/omapfb/omapfb-ioctl.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
drivers/video/omap2/omapfb/omapfb-ioctl.c +1 -0
index 9c73618..32fab5a 100644
@@ -490,6 +490,7 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd,
 unsigned long arg)
 		struct omapfb_vram_info		vram_info;
 		struct omapfb_tearsync_info	tearsync_info;
 		struct omapfb_display_info	display_info;
+		u32				crt;
 	} p;
drivers/video/omap2/omapfb/omapfb-ioctl.c +2 -0
 	int r = 0;

@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd,
 unsigned long arg)
 			r = -EFAULT;
 		break;
 
+	case FBIO_WAITFORVSYNC:
+		if (get_user(p.crt, (__u32 __user *)arg)) {

drivers/video/omap2/omapfb/omapfb-ioctl.c int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, · line 664

Quoted 10 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +9 -0
+			r = -EFAULT;
+			break;
+		}
+		if (p.crt != 0) {
+			r = -ENODEV;
+			break;
+		}
+		/* FALLTHROUGH */
+
 	case OMAPFB_WAITFORVSYNC:

[Hiremath, Vaibhav] I don't see any reason why we should still keep old custom IOCTL support here.

Thanks,
Vaibhav

Quoted 4 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +0 -0
 		DBG("ioctl WAITFORVSYNC\n");

if (!display) {
--
1.6.3.3

#2 lore raw
Quoted 5 lines · Vaibhav
Quoted 5 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +0 -0
@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd,
 unsigned long arg)
 			r = -EFAULT;
 		break;
Quoted 13 lines

drivers/video/omap2/omapfb/omapfb-ioctl.c int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg) · line 663

Quoted 12 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +11 -0
+	case FBIO_WAITFORVSYNC:
+		if (get_user(p.crt, (__u32 __user *)arg)) {
+			r = -EFAULT;
+			break;
+		}
+		if (p.crt != 0) {
+			r = -ENODEV;
+			break;
+		}
+		/* FALLTHROUGH */
+
 	case OMAPFB_WAITFORVSYNC:

[Hiremath, Vaibhav] I don't see any reason why we should still keep old custom IOCTL support here.

It can already be used so it should not be removed.

Signature · 1 line
Ville Syrjälä
#3 lore raw
Quoted 29 lines

-----Original Message-----
From: Ville Syrjälä [mailto:[email protected]]
Sent: Tuesday, July 06, 2010 3:36 PM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; [email protected]; linux- [email protected]; Valkeinen Tomi (Nokia-MS/Helsinki)
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

Quoted 1 line · Vaibhav

Review int omapfb_ioctl(struct fb_info *fbi, unsigned int

Quoted 1 line
(quoted diff) +0 -0
@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int

cmd,

Quoted 4 lines
Quoted 4 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +0 -0
 unsigned long arg)
 			r = -EFAULT;
 		break;
Quoted 13 lines

drivers/video/omap2/omapfb/omapfb-ioctl.c int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg) · line 663

Quoted 12 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +11 -0
+	case FBIO_WAITFORVSYNC:
+		if (get_user(p.crt, (__u32 __user *)arg)) {
+			r = -EFAULT;
+			break;
+		}
+		if (p.crt != 0) {
+			r = -ENODEV;
+			break;
+		}
+		/* FALLTHROUGH */
+
 	case OMAPFB_WAITFORVSYNC:

[Hiremath, Vaibhav] I don't see any reason why we should still keep old

Quoted 4 lines

custom IOCTL support here.

It can already be used so it should not be removed.

[Hiremath, Vaibhav] I am not in favor of this, if we have standard interface then we should encourage people to use it. Don't you think we will have different interface for OMAP and different for non-omap device.

Thanks,
Vaibhav

Quoted 2 lines

--
Ville Syrjälä

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 29 lines · Vaibhav
Quoted 29 lines

-----Original Message-----
From: Ville Syrjälä [mailto:[email protected]]
Sent: Tuesday, July 06, 2010 3:36 PM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; [email protected]; linux- [email protected]; Valkeinen Tomi (Nokia-MS/Helsinki)
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

Quoted 1 line · Vaibhav

Review int omapfb_ioctl(struct fb_info *fbi, unsigned int

Quoted 1 line
(quoted diff) +0 -0
@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int

cmd,

Quoted 17 lines
Quoted 6 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +2 -0
 unsigned long arg)
 			r = -EFAULT;
 		break;
 
+	case FBIO_WAITFORVSYNC:
+		if (get_user(p.crt, (__u32 __user *)arg)) {

drivers/video/omap2/omapfb/omapfb-ioctl.c int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg) · line 663

Quoted 10 lines
drivers/video/omap2/omapfb/omapfb-ioctl.c +9 -0
+			r = -EFAULT;
+			break;
+		}
+		if (p.crt != 0) {
+			r = -ENODEV;
+			break;
+		}
+		/* FALLTHROUGH */
+
 	case OMAPFB_WAITFORVSYNC:

[Hiremath, Vaibhav] I don't see any reason why we should still keep old

Quoted 5 lines
Quoted 4 lines

custom IOCTL support here.

It can already be used so it should not be removed.

[Hiremath, Vaibhav] I am not in favor of this, if we have standard interface then we should encourage people to use it. Don't you think we will have different interface for OMAP and different for non-omap device.

What anyone thinks that apps should do doesn't matter. Removing the ioctl will break the ABI and that is not an acceptable thing to do in kernel development usually.

Signature · 1 line
Ville Syrjälä
#5 lore raw
In reply to Grazvydas Ignotas
Quoted 11 lines · ext Grazvydas Ignotas

FBIO_WAITFORVSYNC is a stardard ioctl for waiting vsync, already used by some userspace, so add it as an alias for OMAPFB_WAITFORVSYNC.

Signed-off-by Grazvydas Ignotas <[email protected]>

---
Tomi,

I know I already sent this earlier, but as now FBIO_WAITFORVSYNC is a standard ioctl, so why don't we support it? I know you might not like that omapfb_ioctl will have one standard ioctl handler between all OMAP specific ones, but that's something plenty of other drivers do.

One could ask that if FBIO_WAITFORSYNC is a standard ioctl, why do we need to handle it in omapfb's custom ioctl function =).

But I think this is fine, I'll apply.

Tomi

#6 lore raw
Quoted 21 lines

-----Original Message-----
From: Tomi Valkeinen [mailto:[email protected]]
Sent: Tuesday, August 03, 2010 5:32 PM
To: ext Grazvydas Ignotas
Cc: [email protected]; [email protected]; Hiremath, Vaibhav
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

Quoted 11 lines · ext Grazvydas Ignotas

FBIO_WAITFORVSYNC is a stardard ioctl for waiting vsync, already used by some userspace, so add it as an alias for OMAPFB_WAITFORVSYNC.

Signed-off-by Grazvydas Ignotas <[email protected]>

---
Tomi,

I know I already sent this earlier, but as now FBIO_WAITFORVSYNC is a standard ioctl, so why don't we support it? I know you might not like that omapfb_ioctl will have one standard ioctl handler between all OMAP specific ones, but that's something plenty of other drivers do.

Quoted 4 lines

One could ask that if FBIO_WAITFORSYNC is a standard ioctl, why do we need to handle it in omapfb's custom ioctl function =).

But I think this is fine, I'll apply.

[Hiremath, Vaibhav] I still think we should mark the old (custom) interface as deprecated and add new standard ioctl interface here, so that in the future we can remove custom one.

Thanks,
Vaibhav

Quoted 3 lines

Tomi