[PATCHv4,1/6] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*

Commit Message

Denis Carikli Nov. 4, 2013, 4:15 p.m. UTC

Without that fix, drivers using the fb_videomode_from_videomode
  function will not be able to get certain information because
  some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.

Cc: Jean-Christophe Plagniol-Villard <[e-mail]>
Cc: Tomi Valkeinen <[e-mail]>
Cc: [e-mail]
Cc: Rob Herring <[e-mail]>
Cc: Pawel Moll <[e-mail]>
Cc: Mark Rutland <[e-mail]>
Cc: Stephen Warren <[e-mail]>
Cc: Ian Campbell <[e-mail]>
Cc: [e-mail]
Cc: Sascha Hauer <[e-mail]>
Cc: [e-mail]
Cc: Eric Bénard <[e-mail]>
Signed-off-by: Denis Carikli <[e-mail]>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <[e-mail]>
---
ChangeLog v3->v4:
- Fixed the issue with FB_SYNC_PIXDAT_HIGH_ACT value.

ChangeLog v2->v3:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/fbmon.c   |    4 ++++
 include/uapi/linux/fb.h |    2 ++
 2 files changed, 6 insertions(+)

Comments

Geert Uytterhoeven Nov. 4, 2013, 7:01 p.m. UTC | #1

On Mon, Nov 4, 2013 at 5:15 PM, Denis Carikli <[e-mail]> wrote:
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h

> +#define FB_SYNC_DE_HIGH_ACT    64      /* data enable high active */
> +#define FB_SYNC_PIXDAT_HIGH_ACT        128     /* data enable high active */

copy and paste of the comment above?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Russell King - ARM Linux Nov. 5, 2013, 1:47 p.m. UTC | #2

On Mon, Nov 04, 2013 at 05:15:19PM +0100, Denis Carikli wrote:
> Without that fix, drivers using the fb_videomode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.

Please describe how userspace knows how to set these flags.

Flags such as the polarity of the hsync and vsync signals are fairly well
defined based upon the display mode, but aren't things like data enable
and the pixel clock edge a property of the attached LCD panel, which is
something that is configured according to the panel which is attached.

In other words, this information should not be passed from userspace, but
should be part of the DT description about the connected panel.  So should
be the polarity of the line and frame pulses (iow, hsync and vsync pulses)
in the case of a connected panel.

Moreover, in the case of a connected LCD panel, chances are it can only
support a very limited number of settings, so support for userspace
changing the timings is arguably incorrect from a system point of view.

Patch

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@  int fb_videomode_from_videomode(const struct videomode *vm,
 		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
 		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
 		fbmode->vmode |= FB_VMODE_INTERLACED;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..c618cfc 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@  struct fb_bitfield {
 					/* vtotal = 144d/288n/576i => PAL  */
 					/* vtotal = 121d/242n/484i => NTSC */
 #define FB_SYNC_ON_GREEN	32	/* sync on green */
+#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT	128	/* data enable high active */
 
 #define FB_VMODE_NONINTERLACED  0	/* non interlaced */
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/