fbdev: move FBIO_WAITFORVSYNC to linux/fb.h

PATCH In review Grazvydas Ignotas 4 participants 6 messages Apr 16, 2010 – Apr 21, 2010 Framebuffer layer

Share & tools

Grazvydas Ignotas

author

2010-04-16 14:57 UTC #0 lore raw

PATCH 1/1 fbdev: move FBIO_WAITFORVSYNC to linux/fb.h

FBIO_WAITFORVSYNC is currently implemented by matroxfb, atyfb, intelfb and more. All of them keep redefining the same FBIO_WAITFORVSYNC macro over and over again, so move it to linux/fb.h and clean up those duplicate defines.

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

---
 drivers/video/aty/atyfb_base.c        |    4 ----
 drivers/video/intelfb/intelfb.h       |    4 ----
 drivers/video/matrox/matroxfb_base.c  |    1 +
 drivers/video/matrox/matroxfb_crtc2.c |    1 +
 include/linux/fb.h                    |    2 ++
 include/linux/ivtvfb.h                |    1 -
 include/linux/matroxfb.h              |    2 --
 include/video/sh_mobile_lcdc.h        |    2 --
 8 files changed, 4 insertions(+), 13 deletions(-)

drivers/video/aty/atyfb_base.c +0 -4

index 29d7285..f8d69ad 100644
@@ -1820,10 +1820,6 @@ struct atyclk {
 #define ATYIO_FEATW		0x41545903	/* ATY\03 */
 #endif
 
-#ifndef FBIO_WAITFORVSYNC
-#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
-#endif
-
 static int atyfb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
 {
 	struct atyfb_par *par = (struct atyfb_par *) info->par;

drivers/video/intelfb/intelfb.h +0 -4

index 4098455..6b51175 100644
@@ -371,10 +371,6 @@ struct intelfb_info {
 			((dinfo)->chipset = INTEL_965G) ||	\
 			((dinfo)->chipset = INTEL_965GM))
 
-#ifndef FBIO_WAITFORVSYNC
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
-#endif
-
 /*** function prototypes ***/
 
 extern int intelfb_var_to_depth(const struct fb_var_screeninfo *var);

drivers/video/matrox/matroxfb_base.c +1 -0

index 052dd9f..3a09978 100644
@@ -112,6 +112,7 @@
 #include "matroxfb_crtc2.h"
 #include "matroxfb_g450.h"
 #include <linux/matroxfb.h>
+#include <linux/fb.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>

drivers/video/matrox/matroxfb_crtc2.c +1 -0

index d7112c3..aeb4e23 100644
@@ -15,6 +15,7 @@
 #include "matroxfb_misc.h"
 #include "matroxfb_DAC1064.h"
 #include <linux/matroxfb.h>
+#include <linux/fb.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>

include/linux/fb.h +2 -0

index c10163b..c99a809 100644
@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+
 /* 0x4607-0x460B are defined below */
 /* #define FBIOGET_MONITORSPEC	0x460C */
 /* #define FBIOPUT_MONITORSPEC	0x460D */

include/linux/ivtvfb.h +0 -1

index 9d88b29..e8b92f6 100644
@@ -33,6 +33,5 @@ struct ivtvfb_dma_frame {
 };
 
 #define IVTVFB_IOC_DMA_FRAME 	_IOW('V', BASE_VIDIOC_PRIVATE+0, struct ivtvfb_dma_frame)
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
 
 #endif

include/linux/matroxfb.h +0 -2

index 2203121..98b1374 100644
@@ -37,7 +37,5 @@ enum matroxfb_ctrl_id {
   MATROXFB_CID_LAST
 };
 
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
-
 #endif

include/video/sh_mobile_lcdc.h +0 -2

index 2cc893f..2882054 100644
@@ -34,8 +34,6 @@ enum { LCDC_CLK_BUS, LCDC_CLK_PERIPHERAL, LCDC_CLK_EXTERNAL };
 #define LCDC_FLAGS_HSCNT (1 << 3) /* Disable HSYNC during VBLANK */
 #define LCDC_FLAGS_DWCNT (1 << 4) /* Disable dotclock during blanking */
 
-#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
-
 struct sh_mobile_lcdc_sys_bus_cfg {
 	unsigned long ldmt2r;
 	unsigned long ldmt3r;

Signature · 1 line

1.6.3.3

Hiremath, Vaibhav

2010-04-20 09:17 UTC #1 lore raw

Quoted 9 lines

-----Original Message-----
From: [email protected] [mailto:linux-fbdev- [email protected]] On Behalf Of Grazvydas Ignotas
Sent: Friday, April 16, 2010 8:27 PM
To: [email protected]
Cc: Andrew Morton; Grazvydas Ignotas
Subject: [PATCH] fbdev: move FBIO_WAITFORVSYNC to linux/fb.h

Quoted 4 lines

FBIO_WAITFORVSYNC is currently implemented by matroxfb, atyfb, intelfb and more. All of them keep redefining the same FBIO_WAITFORVSYNC macro over and over again, so move it to linux/fb.h and clean up those duplicate defines.

[Hiremath, Vaibhav] Thanks Grazvydas,

I was about to submit the patch for this. I think we need this ioctl tobe supported in standard FBDEV.

Minor comments in-lined below -

Quoted 75 lines

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

---
 drivers/video/aty/atyfb_base.c        |    4 ----
 drivers/video/intelfb/intelfb.h       |    4 ----
 drivers/video/matrox/matroxfb_base.c  |    1 +
 drivers/video/matrox/matroxfb_crtc2.c |    1 +
 include/linux/fb.h                    |    2 ++
 include/linux/ivtvfb.h                |    1 -
 include/linux/matroxfb.h              |    2 --
 include/video/sh_mobile_lcdc.h        |    2 --
 8 files changed, 4 insertions(+), 13 deletions(-)

drivers/video/aty/atyfb_base.c +0 -4

index 29d7285..f8d69ad 100644
@@ -1820,10 +1820,6 @@ struct atyclk {
 #define ATYIO_FEATW		0x41545903	/* ATY\03 */
 #endif
 
-#ifndef FBIO_WAITFORVSYNC
-#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
-#endif
-
 static int atyfb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
 {
 	struct atyfb_par *par = (struct atyfb_par *) info->par;

drivers/video/intelfb/intelfb.h +0 -4

index 4098455..6b51175 100644
@@ -371,10 +371,6 @@ struct intelfb_info {
 			((dinfo)->chipset = INTEL_965G) ||	\
 			((dinfo)->chipset = INTEL_965GM))
 
-#ifndef FBIO_WAITFORVSYNC
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
-#endif
-
 /*** function prototypes ***/
 
 extern int intelfb_var_to_depth(const struct fb_var_screeninfo *var);

drivers/video/matrox/matroxfb_base.c +1 -0

index 052dd9f..3a09978 100644
@@ -112,6 +112,7 @@
 #include "matroxfb_crtc2.h"
 #include "matroxfb_g450.h"
 #include <linux/matroxfb.h>
+#include <linux/fb.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>

drivers/video/matrox/matroxfb_crtc2.c +1 -0

index d7112c3..aeb4e23 100644
@@ -15,6 +15,7 @@
 #include "matroxfb_misc.h"
 #include "matroxfb_DAC1064.h"
 #include <linux/matroxfb.h>
+#include <linux/fb.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>

include/linux/fb.h +0 -0

index c10163b..c99a809 100644

include/linux/fb.h struct dentry; · line 27

Quoted 5 lines

include/linux/fb.h +2 -0

@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+

[Hiremath, Vaibhav] Do not insert it in middle, lets follow the sequence and append it at last. Otherwise looks ok to me.

We should merge this patch.

There are some other drivers (like OMAP) which define WAITFORVSYNC ioctl with custom (OMAPFB_WAITFORVSYNC) way needs to be changed for this.

I will change them and submit it to list.

Thanks,
Vaibhav

Quoted 48 lines

include/linux/fb.h +0 -0

 /* 0x4607-0x460B are defined below */
 /* #define FBIOGET_MONITORSPEC	0x460C */
 /* #define FBIOPUT_MONITORSPEC	0x460D */

include/linux/ivtvfb.h +0 -1

index 9d88b29..e8b92f6 100644
@@ -33,6 +33,5 @@ struct ivtvfb_dma_frame {
 };
 
 #define IVTVFB_IOC_DMA_FRAME 	_IOW('V', BASE_VIDIOC_PRIVATE+0, struct
 ivtvfb_dma_frame)
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)

#endif

include/linux/matroxfb.h +0 -2

index 2203121..98b1374 100644
@@ -37,7 +37,5 @@ enum matroxfb_ctrl_id {
   MATROXFB_CID_LAST
 };
 
-#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
-
 #endif

include/video/sh_mobile_lcdc.h +0 -2

index 2cc893f..2882054 100644
@@ -34,8 +34,6 @@ enum { LCDC_CLK_BUS, LCDC_CLK_PERIPHERAL,
 LCDC_CLK_EXTERNAL };
 #define LCDC_FLAGS_HSCNT (1 << 3) /* Disable HSYNC during VBLANK */
 #define LCDC_FLAGS_DWCNT (1 << 4) /* Disable dotclock during blanking */
 
-#define FBIO_WAITFORVSYNC _IOW('F', 0x20, __u32)
-
 struct sh_mobile_lcdc_sys_bus_cfg {
 	unsigned long ldmt2r;

unsigned long ldmt3r;
--
1.6.3.3

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

Andrew Morton

2010-04-20 22:54 UTC #2 lore raw

In reply to Grazvydas Ignotas

Quoted 11 lines · Vaibhav

...
include/linux/fb.h struct dentry; · line 27
Quoted 8 lines
include/linux/fb.h +2 -0
@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+
[Hiremath, Vaibhav] Do not insert it in middle, lets follow the sequence and append it at last. Otherwise looks ok to me.

Confused. That definition appears to be in the appropriate place to me.

Quoted 5 lines

We should merge this patch.
There are some other drivers (like OMAP) which define WAITFORVSYNC ioctl with custom (OMAPFB_WAITFORVSYNC) way needs to be changed for this.
I will change them and submit it to list.

Please cc myself...

Hiremath, Vaibhav

2010-04-21 04:29 UTC #3 lore raw

In reply to Grazvydas Ignotas

Quoted 22 lines

-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Wednesday, April 21, 2010 4:25 AM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; [email protected]
Subject: Re: [PATCH] fbdev: move FBIO_WAITFORVSYNC to linux/fb.h
Quoted 11 lines · Vaibhav
...
include/linux/fb.h struct dentry; · line 27
Quoted 8 lines
include/linux/fb.h +2 -0
@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+
[Hiremath, Vaibhav] Do not insert it in middle, lets follow the sequence

Quoted 2 lines

and append it at last. Otherwise looks ok to me.

[Hiremath, Vaibhav] Andrew,
If I understand correctly, the previous patch was not following the IOCTL numbering sequence.

Logically this IOCTL should get added as per their number sequence that is at the end after FBIOGET_DISPINFO (0x4618).

Thanks,
Vaibhav

Quoted 10 lines

Confused. That definition appears to be in the appropriate place to me.
Quoted 3 lines
We should merge this patch.
There are some other drivers (like OMAP) which define WAITFORVSYNC ioctl
with custom (OMAPFB_WAITFORVSYNC) way needs to be changed for this.
Quoted 2 lines
I will change them and submit it to list.
Please cc myself...

James Simmons

2010-04-21 13:21 UTC #4 lore raw

In reply to Grazvydas Ignotas

Quoted 22 lines

Quoted 22 lines
-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Wednesday, April 21, 2010 4:25 AM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; [email protected]
Subject: Re: [PATCH] fbdev: move FBIO_WAITFORVSYNC to linux/fb.h
Quoted 11 lines · Vaibhav
...
include/linux/fb.h struct dentry; · line 27
Quoted 8 lines
include/linux/fb.h +2 -0
@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+
[Hiremath, Vaibhav] Do not insert it in middle, lets follow the sequence

Quoted 6 lines

Quoted 2 lines
and append it at last. Otherwise looks ok to me.
[Hiremath, Vaibhav] Andrew,
If I understand correctly, the previous patch was not following the IOCTL numbering sequence.
Logically this IOCTL should get added as per their number sequence that is at the end after FBIOGET_DISPINFO (0x4618).

Changing the IOCTL number for WAITFORSYNC would break several apps like egl and directfb. I'm afraid we are stuff with that number.

Hiremath, Vaibhav

2010-04-21 13:36 UTC #5 lore raw

In reply to Grazvydas Ignotas

Quoted 35 lines

-----Original Message-----
From: James Simmons [mailto:[email protected]]
Sent: Wednesday, April 21, 2010 6:51 PM
To: Hiremath, Vaibhav
Cc: Andrew Morton; Grazvydas Ignotas; [email protected]
Subject: RE: [PATCH] fbdev: move FBIO_WAITFORVSYNC to linux/fb.h
Quoted 12 lines
Quoted 12 lines
-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Wednesday, April 21, 2010 4:25 AM
To: Hiremath, Vaibhav
Cc: Grazvydas Ignotas; [email protected]
Subject: Re: [PATCH] fbdev: move FBIO_WAITFORVSYNC to linux/fb.h
Quoted 2 lines · Vaibhav
...
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
include/linux/fb.h struct dentry; · line 27
Quoted 8 lines
include/linux/fb.h +2 -0
@@ -23,6 +23,8 @@ struct dentry;
 #else
 #define FBIO_CURSOR            _IOWR('F', 0x08, struct fb_cursor)
 #endif
+#define FBIO_WAITFORVSYNC	_IOW('F', 0x20, __u32)
+
[Hiremath, Vaibhav] Do not insert it in middle, lets follow the
sequence
Quoted 4 lines
Quoted 2 lines
and append it at last. Otherwise looks ok to me.
[Hiremath, Vaibhav] Andrew,
If I understand correctly, the previous patch was not following the IOCTL

Quoted 7 lines

numbering sequence.
Quoted 2 lines
Logically this IOCTL should get added as per their number sequence that is
at the end after FBIOGET_DISPINFO (0x4618).
Changing the IOCTL number for WAITFORSYNC would break several apps like egl and directfb. I'm afraid we are stuff with that number.

[Hiremath, Vaibhav] Yeah, I understand this, and thinking on the same line, I am not asking to change the ioctl no, it's only placing the declaration/definition in logical sequence.

Thanks,
Vaibhav