[PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

Subsystems: framebuffer layer, the rest

STALE5911d

7 messages, 4 authors, 2010-08-09 · open the first message on its own page

[PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Grazvydas Ignotas <hidden>
Date: 2010-07-02 20:54:56

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 <redacted>
---
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(-)

diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c b/drivers/video/omap2/omapfb/omapfb-ioctl.c
index 9c73618..32fab5a 100644
--- a/drivers/video/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c

@@ -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) {

-- 
1.6.3.3

RE: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Hiremath, Vaibhav <hidden>
Date: 2010-07-06 06:20:14

-----Original Message-----
From: Grazvydas Ignotas [mailto:notasas@gmail.com]
Sent: Saturday, July 03, 2010 2:25 AM
To: linux-fbdev@vger.kernel.org
Cc: linux-omap@vger.kernel.org; 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.

Signed-off-by: Grazvydas Ignotas <redacted>
---
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 hunk

 drivers/video/omap2/omapfb/omapfb-ioctl.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c

b/drivers/video/omap2/omapfb/omapfb-ioctl.c
index 9c73618..32fab5a 100644

--- a/drivers/video/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c

@@ -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:

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

Thanks,
Vaibhav

 		DBG("ioctl WAITFORVSYNC\n");
 		if (!display) {
--
1.6.3.3

Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Ville Syrjälä <hidden>
Date: 2010-07-06 10:05:33

On Tue, Jul 06, 2010 at 08:08:14AM +0200, ext Hiremath, Vaibhav wrote:

quoted

@@ -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:

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

-- 
Ville Syrjälä

RE: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Hiremath, Vaibhav <hidden>
Date: 2010-07-06 11:38:28

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
Sent: Tuesday, July 06, 2010 3:36 PM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; linux-fbdev@vger.kernel.org; linux-
omap@vger.kernel.org; Valkeinen Tomi (Nokia-MS/Helsinki)
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

On Tue, Jul 06, 2010 at 08:08:14AM +0200, ext Hiremath, Vaibhav wrote:

quoted

@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int

cmd,

quoted

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:

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

[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

--
Ville Syrjälä

Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Ville Syrjälä <hidden>
Date: 2010-07-06 11:42:56

On Tue, Jul 06, 2010 at 01:26:28PM +0200, ext Hiremath, Vaibhav wrote:

quoted

-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@nokia.com]
Sent: Tuesday, July 06, 2010 3:36 PM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; linux-fbdev@vger.kernel.org; linux-
omap@vger.kernel.org; Valkeinen Tomi (Nokia-MS/Helsinki)
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

On Tue, Jul 06, 2010 at 08:08:14AM +0200, ext Hiremath, Vaibhav wrote:

quoted

@@ -648,6 +649,17 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int

cmd,

quoted

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:

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

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

-- 
Ville Syrjälä

Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Tomi Valkeinen <hidden>
Date: 2010-08-03 12:02:24

On Fri, 2010-07-02 at 22:54 +0200, ext Grazvydas Ignotas wrote:

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

RE: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

From: Hiremath, Vaibhav <hidden>
Date: 2010-08-09 13:05:49

-----Original Message-----
From: Tomi Valkeinen [mailto:tomi.valkeinen@nokia.com]
Sent: Tuesday, August 03, 2010 5:32 PM
To: ext Grazvydas Ignotas
Cc: linux-fbdev@vger.kernel.org; linux-omap@vger.kernel.org; Hiremath,
Vaibhav
Subject: Re: [PATCH] OMAP: DSS2: OMAPFB: add support for FBIO_WAITFORVSYNC

On Fri, 2010-07-02 at 22:54 +0200, ext Grazvydas Ignotas wrote:

quoted

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

[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

 Tomi

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help