All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v17 0/7] of: add display helper
@ 2013-01-25  9:01 ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Hi!

This is basically just a resend of v16 that was rebased onto v3.8-rc4 and has
two new "tested-by"s from Rob and Leela.

The patches were tested with:

        - v15 on Tegra by Thierry
        - sh-mobile-lcdcfb by Laurent
        - MX53QSB by Marek
        - Exynos: smdk5250 by Leela
        - AM335X EVM & AM335X EVM-SK by Afzal
	- tilcdc lcd-panel by Rob
        - imx6q: sabrelite, sabresd by Philipp and me
        - imx53: tqma53/mba53 by me


Changes since v16:
	- rebased from 3.7 to 3.8-rc4

Changes since v15:
        - move include/linux/{videomode,display_timing}.h to include/video
        - move include/linux/of_{videomode,display_timing}.h to include/video
        - reimplement flags: add VESA flags and data flags
        - let pixelclock in struct videomode be unsigned long
        - rename of_display_timings_exists to of_display_timings_exist
        - revise logging/error messages: replace __func__ with np->full_name
        - rename pixelclk-inverted to pixelclk-active
        - revise comments in code

Changes since v14:
        - fix "const struct *" warning
                (reported by: Leela Krishna Amudala <l.krishna@samsung.com>)
        - return -EINVAL when htotal or vtotal are zero
        - remove unreachable code in of_get_display_timings
        - include headers in .c files and not implicit in .h
        - sort includes alphabetically
        - fix lower/uppercase in binding documentation
        - rebase onto v3.7-rc7

Changes since v13:
        - fix "const struct *" warning
                (reported by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>)
        - prevent division by zero in fb_videomode_from_videomode

Changes since v12:
        - rename struct display_timing to via_display_timing in via subsystem
        - fix refreshrate calculation
        - fix "const struct *" warnings
                (reported by: Manjunathappa, Prakash <prakash.pm@ti.com>)
        - some CodingStyle fixes
        - rewrite parts of commit messages and display-timings.txt
        - let display_timing_get_value get all values instead of just typical

Changes since v11:
        - make pointers const where applicable
        - add reviewed-by Laurent Pinchart

Changes since v10:
        - fix function name (drm_)display_mode_from_videomode
        - add acked-by, reviewed-by, tested-by

Changes since v9:
        - don't leak memory when previous timings were correct
        - CodingStyle fixes
        - move blank lines around

Changes since v8:
        - fix memory leaks
        - change API to be more consistent (foo_from_bar(struct bar, struct foo))
        - include headers were necessary
        - misc minor bugfixes

Changes since v7:
        - move of_xxx to drivers/video
        - remove non-binding documentation from display-timings.txt
        - squash display_timings and videomode in one patch
        - misc minor fixes

Changes since v6:
        - get rid of some empty lines etc.
        - move functions to their subsystems
        - split of_ from non-of_ functions
        - add at least some kerneldoc to some functions

Changes since v5:
        - removed all display stuff and just describe timings

Changes since v4:
        - refactored functions

Changes since v3:
        - print error messages
        - free alloced memory
        - general cleanup

Changes since v2:
        - use hardware-near property-names
        - provide a videomode structure
        - allow ranges for all properties (<min,typ,max>)
        - functions to get display_mode or fb_videomode


Regards
Steffen

Steffen Trumtrar (7):
  viafb: rename display_timing to via_display_timing
  video: add display_timing and videomode
  video: add of helper for display timings/videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 ++++++
 drivers/video/Kconfig                              |   21 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 ++
 drivers/video/fbmon.c                              |   94 ++++++++
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 drivers/video/via/hw.c                             |    6 +-
 drivers/video/via/hw.h                             |    2 +-
 drivers/video/via/lcd.c                            |    2 +-
 drivers/video/via/share.h                          |    2 +-
 drivers/video/via/via_modesetting.c                |    8 +-
 drivers/video/via/via_modesetting.h                |    6 +-
 drivers/video/videomode.c                          |   39 ++++
 include/drm/drmP.h                                 |    9 +
 include/linux/fb.h                                 |    8 +
 include/video/display_timing.h                     |  124 ++++++++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 include/video/videomode.h                          |   48 ++++
 21 files changed, 894 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h
 create mode 100644 include/video/videomode.h

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v17 0/7] of: add display helper
@ 2013-01-25  9:01 ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Hi!

This is basically just a resend of v16 that was rebased onto v3.8-rc4 and has
two new "tested-by"s from Rob and Leela.

The patches were tested with:

        - v15 on Tegra by Thierry
        - sh-mobile-lcdcfb by Laurent
        - MX53QSB by Marek
        - Exynos: smdk5250 by Leela
        - AM335X EVM & AM335X EVM-SK by Afzal
	- tilcdc lcd-panel by Rob
        - imx6q: sabrelite, sabresd by Philipp and me
        - imx53: tqma53/mba53 by me


Changes since v16:
	- rebased from 3.7 to 3.8-rc4

Changes since v15:
        - move include/linux/{videomode,display_timing}.h to include/video
        - move include/linux/of_{videomode,display_timing}.h to include/video
        - reimplement flags: add VESA flags and data flags
        - let pixelclock in struct videomode be unsigned long
        - rename of_display_timings_exists to of_display_timings_exist
        - revise logging/error messages: replace __func__ with np->full_name
        - rename pixelclk-inverted to pixelclk-active
        - revise comments in code

Changes since v14:
        - fix "const struct *" warning
                (reported by: Leela Krishna Amudala <l.krishna@samsung.com>)
        - return -EINVAL when htotal or vtotal are zero
        - remove unreachable code in of_get_display_timings
        - include headers in .c files and not implicit in .h
        - sort includes alphabetically
        - fix lower/uppercase in binding documentation
        - rebase onto v3.7-rc7

Changes since v13:
        - fix "const struct *" warning
                (reported by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>)
        - prevent division by zero in fb_videomode_from_videomode

Changes since v12:
        - rename struct display_timing to via_display_timing in via subsystem
        - fix refreshrate calculation
        - fix "const struct *" warnings
                (reported by: Manjunathappa, Prakash <prakash.pm@ti.com>)
        - some CodingStyle fixes
        - rewrite parts of commit messages and display-timings.txt
        - let display_timing_get_value get all values instead of just typical

Changes since v11:
        - make pointers const where applicable
        - add reviewed-by Laurent Pinchart

Changes since v10:
        - fix function name (drm_)display_mode_from_videomode
        - add acked-by, reviewed-by, tested-by

Changes since v9:
        - don't leak memory when previous timings were correct
        - CodingStyle fixes
        - move blank lines around

Changes since v8:
        - fix memory leaks
        - change API to be more consistent (foo_from_bar(struct bar, struct foo))
        - include headers were necessary
        - misc minor bugfixes

Changes since v7:
        - move of_xxx to drivers/video
        - remove non-binding documentation from display-timings.txt
        - squash display_timings and videomode in one patch
        - misc minor fixes

Changes since v6:
        - get rid of some empty lines etc.
        - move functions to their subsystems
        - split of_ from non-of_ functions
        - add at least some kerneldoc to some functions

Changes since v5:
        - removed all display stuff and just describe timings

Changes since v4:
        - refactored functions

Changes since v3:
        - print error messages
        - free alloced memory
        - general cleanup

Changes since v2:
        - use hardware-near property-names
        - provide a videomode structure
        - allow ranges for all properties (<min,typ,max>)
        - functions to get display_mode or fb_videomode


Regards
Steffen

Steffen Trumtrar (7):
  viafb: rename display_timing to via_display_timing
  video: add display_timing and videomode
  video: add of helper for display timings/videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/gpu/drm/drm_modes.c                        |   70 ++++++
 drivers/video/Kconfig                              |   21 ++
 drivers/video/Makefile                             |    4 +
 drivers/video/display_timing.c                     |   24 ++
 drivers/video/fbmon.c                              |   94 ++++++++
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 drivers/video/via/hw.c                             |    6 +-
 drivers/video/via/hw.h                             |    2 +-
 drivers/video/via/lcd.c                            |    2 +-
 drivers/video/via/share.h                          |    2 +-
 drivers/video/via/via_modesetting.c                |    8 +-
 drivers/video/via/via_modesetting.h                |    6 +-
 drivers/video/videomode.c                          |   39 ++++
 include/drm/drmP.h                                 |    9 +
 include/linux/fb.h                                 |    8 +
 include/video/display_timing.h                     |  124 ++++++++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 include/video/videomode.h                          |   48 ++++
 21 files changed, 894 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h
 create mode 100644 include/video/videomode.h

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v17 1/7] viafb: rename display_timing to via_display_timing
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, which is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/via/hw.c              |    6 +++---
 drivers/video/via/hw.h              |    2 +-
 drivers/video/via/lcd.c             |    2 +-
 drivers/video/via/share.h           |    2 +-
 drivers/video/via/via_modesetting.c |    8 ++++----
 drivers/video/via/via_modesetting.h |    6 +++---
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 80233da..22450908 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
 	via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
 }
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres)
 {
-	struct display_timing timing;
+	struct via_display_timing timing;
 	u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
 
 	timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga)
 {
-	struct display_timing crt_reg = var_to_timing(var,
+	struct via_display_timing crt_reg = var_to_timing(var,
 		cxres ? cxres : var->xres, cyres ? cyres : var->yres);
 
 	if (iga == IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index a820575..3be073c 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
 extern int viafb_DVI_ON;
 extern int viafb_hotplug;
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres);
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 980ee1b..5d21ff4 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo *var, u16 cxres,
 	int panel_hres = plvds_setting_info->lcd_panel_hres;
 	int panel_vres = plvds_setting_info->lcd_panel_vres;
 	u32 clock;
-	struct display_timing timing;
+	struct via_display_timing timing;
 	struct fb_var_screeninfo panel_var;
 	const struct fb_videomode *mode_crt_table, *panel_crt_table;
 
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
 	int refresh_rate;
 	int h_sync_polarity;
 	int v_sync_polarity;
-	struct display_timing crtc;
+	struct via_display_timing crtc;
 };
 
 struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
 #include "debug.h"
 
 
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total / 8 - 5;
 	raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing *timing)
 	via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
 }
 
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total - 1;
 	raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
 #define VIA_PITCH_MAX	0x3FF8
 
 
-struct display_timing {
+struct via_display_timing {
 	u16 hor_total;
 	u16 hor_addr;
 	u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
 };
 
 
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
 void via_set_primary_address(u32 addr);
 void via_set_secondary_address(u32 addr);
 void via_set_primary_pitch(u32 pitch);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 1/7] viafb: rename display_timing to via_display_timing
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

The struct display_timing is specific to the via subsystem. The naming leads to
collisions with the new struct display_timing, which is supposed to be a shared
struct between different subsystems.
To clean this up, prepend the existing struct with the subsystem it is specific
to.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/via/hw.c              |    6 +++---
 drivers/video/via/hw.h              |    2 +-
 drivers/video/via/lcd.c             |    2 +-
 drivers/video/via/share.h           |    2 +-
 drivers/video/via/via_modesetting.c |    8 ++++----
 drivers/video/via/via_modesetting.h |    6 +++---
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 80233da..22450908 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -1467,10 +1467,10 @@ void viafb_set_vclock(u32 clk, int set_iga)
 	via_write_misc_reg_mask(0x0C, 0x0C); /* select external clock */
 }
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres)
 {
-	struct display_timing timing;
+	struct via_display_timing timing;
 	u16 dx = (var->xres - cxres) / 2, dy = (var->yres - cyres) / 2;
 
 	timing.hor_addr = cxres;
@@ -1491,7 +1491,7 @@ struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga)
 {
-	struct display_timing crt_reg = var_to_timing(var,
+	struct via_display_timing crt_reg = var_to_timing(var,
 		cxres ? cxres : var->xres, cyres ? cyres : var->yres);
 
 	if (iga = IGA1)
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index a820575..3be073c 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -637,7 +637,7 @@ extern int viafb_LCD_ON;
 extern int viafb_DVI_ON;
 extern int viafb_hotplug;
 
-struct display_timing var_to_timing(const struct fb_var_screeninfo *var,
+struct via_display_timing var_to_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres);
 void viafb_fill_crtc_timing(const struct fb_var_screeninfo *var,
 	u16 cxres, u16 cyres, int iga);
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 980ee1b..5d21ff4 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -549,7 +549,7 @@ void viafb_lcd_set_mode(const struct fb_var_screeninfo *var, u16 cxres,
 	int panel_hres = plvds_setting_info->lcd_panel_hres;
 	int panel_vres = plvds_setting_info->lcd_panel_vres;
 	u32 clock;
-	struct display_timing timing;
+	struct via_display_timing timing;
 	struct fb_var_screeninfo panel_var;
 	const struct fb_videomode *mode_crt_table, *panel_crt_table;
 
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 3158dfc..65c65c6 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -319,7 +319,7 @@ struct crt_mode_table {
 	int refresh_rate;
 	int h_sync_polarity;
 	int v_sync_polarity;
-	struct display_timing crtc;
+	struct via_display_timing crtc;
 };
 
 struct io_reg {
diff --git a/drivers/video/via/via_modesetting.c b/drivers/video/via/via_modesetting.c
index 0e431ae..0b414b0 100644
--- a/drivers/video/via/via_modesetting.c
+++ b/drivers/video/via/via_modesetting.c
@@ -30,9 +30,9 @@
 #include "debug.h"
 
 
-void via_set_primary_timing(const struct display_timing *timing)
+void via_set_primary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total / 8 - 5;
 	raw.hor_addr = timing->hor_addr / 8 - 1;
@@ -88,9 +88,9 @@ void via_set_primary_timing(const struct display_timing *timing)
 	via_write_reg_mask(VIACR, 0x17, 0x80, 0x80);
 }
 
-void via_set_secondary_timing(const struct display_timing *timing)
+void via_set_secondary_timing(const struct via_display_timing *timing)
 {
-	struct display_timing raw;
+	struct via_display_timing raw;
 
 	raw.hor_total = timing->hor_total - 1;
 	raw.hor_addr = timing->hor_addr - 1;
diff --git a/drivers/video/via/via_modesetting.h b/drivers/video/via/via_modesetting.h
index 06e09fe..f6a6503 100644
--- a/drivers/video/via/via_modesetting.h
+++ b/drivers/video/via/via_modesetting.h
@@ -33,7 +33,7 @@
 #define VIA_PITCH_MAX	0x3FF8
 
 
-struct display_timing {
+struct via_display_timing {
 	u16 hor_total;
 	u16 hor_addr;
 	u16 hor_blank_start;
@@ -49,8 +49,8 @@ struct display_timing {
 };
 
 
-void via_set_primary_timing(const struct display_timing *timing);
-void via_set_secondary_timing(const struct display_timing *timing);
+void via_set_primary_timing(const struct via_display_timing *timing);
+void via_set_secondary_timing(const struct via_display_timing *timing);
 void via_set_primary_address(u32 addr);
 void via_set_secondary_address(u32 addr);
 void via_set_primary_pitch(u32 pitch);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 2/7] video: add display_timing and videomode
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Also, add helper functions to convert from display timings to a generic videomode
structure.

The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
    - ranges for signals that may have min-, max- and typical values
    - single integers for signals that can be on, off or are ignored
    - booleans for signals that are either on or off

As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.

Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.

The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.

This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/Kconfig          |    6 ++
 drivers/video/Makefile         |    2 +
 drivers/video/display_timing.c |   24 ++++++++
 drivers/video/videomode.c      |   39 +++++++++++++
 include/video/display_timing.h |  124 ++++++++++++++++++++++++++++++++++++++++
 include/video/videomode.h      |   48 ++++++++++++++++
 6 files changed, 243 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e7068c5..09a8f0d 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 768a137..e0dd820 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,3 +168,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..5e1822c
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..21c47a2
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,39 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
+	vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+
+	vm->dmt_flags = dt->dmt_flags;
+	vm->data_flags = dt->data_flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
new file mode 100644
index 0000000..71e9a38
--- /dev/null
+++ b/include/video/display_timing.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMING_H
+#define __LINUX_DISPLAY_TIMING_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* VESA display monitor timing parameters */
+#define VESA_DMT_HSYNC_LOW		BIT(0)
+#define VESA_DMT_HSYNC_HIGH		BIT(1)
+#define VESA_DMT_VSYNC_LOW		BIT(2)
+#define VESA_DMT_VSYNC_HIGH		BIT(3)
+
+/* display specific flags */
+#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED	BIT(4)
+#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
+
+/*
+ * A single signal can be specified via a range of minimal and maximal values
+ * with a typical value, that lies somewhere inbetween.
+ */
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+enum timing_entry_index {
+	TE_MIN = 0,
+	TE_TYP = 1,
+	TE_MAX = 2,
+};
+
+/*
+ * Single "mode" entry. This describes one set of signal timings a display can
+ * have in one setting. This struct can later be converted to struct videomode
+ * (see include/video/videomode.h). As each timing_entry can be defined as a
+ * range, one struct display_timing may become multiple struct videomodes.
+ *
+ * Example: hsync active high, vsync active low
+ *
+ *				    Active Video
+ * Video  ______________________XXXXXXXXXXXXXXXXXXXXXX_____________________
+ *	  |<- sync ->|<- back ->|<----- active ----->|<- front ->|<- sync..
+ *	  |	     |	 porch  |		     |	 porch	 |
+ *
+ * HSync _|¯¯¯¯¯¯¯¯¯¯|___________________________________________|¯¯¯¯¯¯¯¯¯
+ *
+ * VSync ¯|__________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|_________
+ */
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;		/* hor. active video */
+	struct timing_entry hfront_porch;	/* hor. front porch */
+	struct timing_entry hback_porch;	/* hor. back porch */
+	struct timing_entry hsync_len;		/* hor. sync len */
+
+	struct timing_entry vactive;		/* ver. active video */
+	struct timing_entry vfront_porch;	/* ver. front porch */
+	struct timing_entry vback_porch;	/* ver. back porch */
+	struct timing_entry vsync_len;		/* ver. sync len */
+
+	unsigned int dmt_flags;			/* VESA DMT flags */
+	unsigned int data_flags;		/* video data flags */
+};
+
+/*
+ * This describes all timing settings a display provides.
+ * The native_mode is the default setting for this display.
+ * Drivers that can handle multiple videomodes should work with this struct and
+ * convert each entry to the desired end result.
+ */
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/* get value specified by index from struct timing_entry */
+static inline u32 display_timing_get_value(const struct timing_entry *te,
+					   enum timing_entry_index index)
+{
+	switch (index) {
+	case TE_MIN:
+		return te->min;
+		break;
+	case TE_TYP:
+		return te->typ;
+		break;
+	case TE_MAX:
+		return te->max;
+		break;
+	default:
+		return te->typ;
+	}
+}
+
+/* get one entry from struct display_timings */
+static inline struct display_timing *display_timings_get(const struct
+							 display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/video/videomode.h b/include/video/videomode.h
new file mode 100644
index 0000000..a421562
--- /dev/null
+++ b/include/video/videomode.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/types.h>
+#include <video/display_timing.h>
+
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+	unsigned long pixelclock;	/* pixelclock in Hz */
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	unsigned int dmt_flags;	/* VESA DMT flags */
+	unsigned int data_flags; /* video data flags */
+};
+
+/**
+ * videomode_from_timing - convert display timing to videomode
+ * @disp: structure with all possible timing entries
+ * @vm: return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function converts a struct display_timing to a struct videomode.
+ */
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index);
+
+#endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Also, add helper functions to convert from display timings to a generic videomode
structure.

The struct display_timing specifies all needed parameters to describe the signal
properties of a display in one mode. This includes
    - ranges for signals that may have min-, max- and typical values
    - single integers for signals that can be on, off or are ignored
    - booleans for signals that are either on or off

As a display may support multiple modes like this, a struct display_timings is
added, that holds all given struct display_timing pointers and declares the
native mode of the display.

Although a display may state that a signal can be in a range, it is driven with
fixed values that indicate a videomode. Therefore graphic drivers don't need all
the information of struct display_timing, but would generate a videomode from
the given set of supported signal timings and work with that.

The video subsystems all define their own structs that describe a mode and work
with that (e.g. fb_videomode or drm_display_mode). To slowly replace all those
various structures and allow code reuse across those subsystems, add struct
videomode as a generic description.

This patch only includes the most basic fields in struct videomode. All missing
fields that are needed to have a really generic video mode description can be
added at a later stage.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/Kconfig          |    6 ++
 drivers/video/Makefile         |    2 +
 drivers/video/display_timing.c |   24 ++++++++
 drivers/video/videomode.c      |   39 +++++++++++++
 include/video/display_timing.h |  124 ++++++++++++++++++++++++++++++++++++++++
 include/video/videomode.h      |   48 ++++++++++++++++
 6 files changed, 243 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/video/display_timing.h
 create mode 100644 include/video/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e7068c5..09a8f0d 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
 	  This framework adds support for low-level control of the video 
 	  output switch.
 
+config DISPLAY_TIMING
+       bool
+
+config VIDEOMODE
+       bool
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 768a137..e0dd820 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,3 +168,5 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 0000000..5e1822c
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+
+void display_timings_release(struct display_timings *disp)
+{
+	if (disp->timings) {
+		unsigned int i;
+
+		for (i = 0; i < disp->num_timings; i++)
+			kfree(disp->timings[i]);
+		kfree(disp->timings);
+	}
+	kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..21c47a2
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,39 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <video/display_timing.h>
+#include <video/videomode.h>
+
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index)
+{
+	struct display_timing *dt;
+
+	dt = display_timings_get(disp, index);
+	if (!dt)
+		return -EINVAL;
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, TE_TYP);
+	vm->hactive = display_timing_get_value(&dt->hactive, TE_TYP);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, TE_TYP);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, TE_TYP);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, TE_TYP);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, TE_TYP);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, TE_TYP);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, TE_TYP);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, TE_TYP);
+
+	vm->dmt_flags = dt->dmt_flags;
+	vm->data_flags = dt->data_flags;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/video/display_timing.h b/include/video/display_timing.h
new file mode 100644
index 0000000..71e9a38
--- /dev/null
+++ b/include/video/display_timing.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMING_H
+#define __LINUX_DISPLAY_TIMING_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* VESA display monitor timing parameters */
+#define VESA_DMT_HSYNC_LOW		BIT(0)
+#define VESA_DMT_HSYNC_HIGH		BIT(1)
+#define VESA_DMT_VSYNC_LOW		BIT(2)
+#define VESA_DMT_VSYNC_HIGH		BIT(3)
+
+/* display specific flags */
+#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
+#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
+#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
+#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
+#define DISPLAY_FLAGS_INTERLACED	BIT(4)
+#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
+
+/*
+ * A single signal can be specified via a range of minimal and maximal values
+ * with a typical value, that lies somewhere inbetween.
+ */
+struct timing_entry {
+	u32 min;
+	u32 typ;
+	u32 max;
+};
+
+enum timing_entry_index {
+	TE_MIN = 0,
+	TE_TYP = 1,
+	TE_MAX = 2,
+};
+
+/*
+ * Single "mode" entry. This describes one set of signal timings a display can
+ * have in one setting. This struct can later be converted to struct videomode
+ * (see include/video/videomode.h). As each timing_entry can be defined as a
+ * range, one struct display_timing may become multiple struct videomodes.
+ *
+ * Example: hsync active high, vsync active low
+ *
+ *				    Active Video
+ * Video  ______________________XXXXXXXXXXXXXXXXXXXXXX_____________________
+ *	  |<- sync ->|<- back ->|<----- active ----->|<- front ->|<- sync..
+ *	  |	     |	 porch  |		     |	 porch	 |
+ *
+ * HSync _|¯¯¯¯¯¯¯¯¯¯|___________________________________________|¯¯¯¯¯¯¯¯¯
+ *
+ * VSync ¯|__________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|_________
+ */
+struct display_timing {
+	struct timing_entry pixelclock;
+
+	struct timing_entry hactive;		/* hor. active video */
+	struct timing_entry hfront_porch;	/* hor. front porch */
+	struct timing_entry hback_porch;	/* hor. back porch */
+	struct timing_entry hsync_len;		/* hor. sync len */
+
+	struct timing_entry vactive;		/* ver. active video */
+	struct timing_entry vfront_porch;	/* ver. front porch */
+	struct timing_entry vback_porch;	/* ver. back porch */
+	struct timing_entry vsync_len;		/* ver. sync len */
+
+	unsigned int dmt_flags;			/* VESA DMT flags */
+	unsigned int data_flags;		/* video data flags */
+};
+
+/*
+ * This describes all timing settings a display provides.
+ * The native_mode is the default setting for this display.
+ * Drivers that can handle multiple videomodes should work with this struct and
+ * convert each entry to the desired end result.
+ */
+struct display_timings {
+	unsigned int num_timings;
+	unsigned int native_mode;
+
+	struct display_timing **timings;
+};
+
+/* get value specified by index from struct timing_entry */
+static inline u32 display_timing_get_value(const struct timing_entry *te,
+					   enum timing_entry_index index)
+{
+	switch (index) {
+	case TE_MIN:
+		return te->min;
+		break;
+	case TE_TYP:
+		return te->typ;
+		break;
+	case TE_MAX:
+		return te->max;
+		break;
+	default:
+		return te->typ;
+	}
+}
+
+/* get one entry from struct display_timings */
+static inline struct display_timing *display_timings_get(const struct
+							 display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void display_timings_release(struct display_timings *disp);
+
+#endif
diff --git a/include/video/videomode.h b/include/video/videomode.h
new file mode 100644
index 0000000..a421562
--- /dev/null
+++ b/include/video/videomode.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * generic videomode description
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_VIDEOMODE_H
+#define __LINUX_VIDEOMODE_H
+
+#include <linux/types.h>
+#include <video/display_timing.h>
+
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+	unsigned long pixelclock;	/* pixelclock in Hz */
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	unsigned int dmt_flags;	/* VESA DMT flags */
+	unsigned int data_flags; /* video data flags */
+};
+
+/**
+ * videomode_from_timing - convert display timing to videomode
+ * @disp: structure with all possible timing entries
+ * @vm: return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function converts a struct display_timing to a struct videomode.
+ */
+int videomode_from_timing(const struct display_timings *disp,
+			  struct videomode *vm, unsigned int index);
+
+#endif
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 3/7] video: add of helper for display timings/videomode
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel

This adds support for reading display timings from DT into a struct
display_timings. The of_display_timing implementation supports multiple
subnodes. All children are read into an array, that can be queried.

If no native mode is specified, the first subnode will be used.

For cases where the graphics driver knows there can be only one
mode description or where the driver only supports one mode, a helper
function of_get_videomode is added, that gets a struct videomode from DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/video/Kconfig                              |   15 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timing.txt b/Documentation/devicetree/bindings/video/display-timing.txt
new file mode 100644
index 0000000..1500385
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timing.txt
@@ -0,0 +1,109 @@
+display-timing bindings
+=======================
+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timing subnode
+--------------
+
+required properties:
+ - hactive, vactive: display resolution
+ - hfront-porch, hback-porch, hsync-len: horizontal display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: vertical display timing parameters in
+   lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: hsync pulse is active low/high/ignored
+ - vsync-active: vsync pulse is active low/high/ignored
+ - de-active: data-enable pulse is active low/high/ignored
+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored
+ - interlaced (bool): boolean to enable interlaced mode
+ - doublescan (bool): boolean to enable doublescan mode
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
+devicetree representation corresponds to the one commonly found in datasheets
+for displays. If a display supports multiple signal timings, the native-mode
+can be specified.
+
+The parameters are defined as:
+
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vback_porch                 |          |       |
+  |          |        ↓                            |          |       |
+  +----------#######################################----------+-------+
+  |          #        ↑                            #          |       |
+  |          #        |                            #          |       |
+  |  hback   #        |                            #  hfront  | hsync |
+  |   porch  #        |       hactive              #  porch   |  len  |
+  |<-------->#<-------+--------------------------->#<-------->|<----->|
+  |          #        |                            #          |       |
+  |          #        |vactive                     #          |       |
+  |          #        |                            #          |       |
+  |          #        ↓                            #          |       |
+  +----------#######################################----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vfront_porch                |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vsync_len                   |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1080p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with minimum, typical and maximum values can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <148500000>;
+		hactive = <1920>;
+		vactive = <1080>;
+		hsync-len = <0 44 60>;
+		hfront-porch = <80 88 95>;
+		hback-porch = <100 148 160>;
+		vfront-porch = <0 4 6>;
+		vback-porch = <0 36 50>;
+		vsync-len = <0 5 6>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 09a8f0d..807c7fa 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,21 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	bool "Enable device tree display timing support"
+	depends on OF
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	bool "Enable device tree videomode support"
+	depends on OF
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e0dd820..f592f3b 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -169,4 +169,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..13ecd98
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,239 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+
+/**
+ * parse_timing_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_timing_property(struct device_node *np, const char *name,
+			  struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells == 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells == 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	u32 val = 0;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_timing_property(np, "hactive", &dt->hactive);
+	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_timing_property(np, "vactive", &dt->vactive);
+	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
+
+	dt->dmt_flags = 0;
+	dt->data_flags = 0;
+	if (!of_property_read_u32(np, "vsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
+				VESA_DMT_VSYNC_LOW;
+	if (!of_property_read_u32(np, "hsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
+				VESA_DMT_HSYNC_LOW;
+	if (!of_property_read_u32(np, "de-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+				DISPLAY_FLAGS_DE_LOW;
+	if (!of_property_read_u32(np, "pixelclk-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+
+	if (of_property_read_bool(np, "interlaced"))
+		dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+	if (of_property_read_bool(np, "doublescan"))
+		dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n",
+			of_node_full_name(np));
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", of_node_full_name(np));
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n",
+			of_node_full_name(np));
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	pr_debug("%s: using %s as default timing\n",
+		of_node_full_name(np), entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings == 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *) *
+				disp->num_timings, GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/*
+			 * to not encourage wrong devicetrees, fail in case of
+			 * an error
+			 */
+			pr_err("%s: error in timing %d\n",
+				of_node_full_name(np), disp->num_timings + 1);
+			goto timingfail;
+		}
+
+		if (native_mode == entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	/*
+	 * native_mode points to the device_node returned by of_parse_phandle
+	 * therefore call of_node_put on it
+	 */
+	of_node_put(native_mode);
+
+	pr_debug("%s: got %d timings. Using timing #%d as default\n",
+		of_node_full_name(np), disp->num_timings,
+		disp->native_mode + 1);
+
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exist - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exist(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exist);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..5b8066c
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,54 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/of.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ *	    (Set this to OF_USE_NATIVE_MODE to use whatever mode is
+ *	     specified as native mode in the DT.)
+ *
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		return -EINVAL;
+	}
+
+	if (index == OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
new file mode 100644
index 0000000..8016eb7
--- /dev/null
+++ b/include/video/of_display_timing.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMING_H
+#define __LINUX_OF_DISPLAY_TIMING_H
+
+struct device_node;
+struct display_timings;
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exist(struct device_node *np);
+
+#endif
diff --git a/include/video/of_videomode.h b/include/video/of_videomode.h
new file mode 100644
index 0000000..a07efcc
--- /dev/null
+++ b/include/video/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct videomode;
+
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* =?UTF-8?q?=5BPATCH=20v17=203/7=5D=20video=3A=20add=20of=20helper=20for=20display=20timings/videomode
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel

This adds support for reading display timings from DT into a struct
display_timings. The of_display_timing implementation supports multiple
subnodes. All children are read into an array, that can be queried.

If no native mode is specified, the first subnode will be used.

For cases where the graphics driver knows there can be only one
mode description or where the driver only supports one mode, a helper
function of_get_videomode is added, that gets a struct videomode from DT.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 .../devicetree/bindings/video/display-timing.txt   |  109 +++++++++
 drivers/video/Kconfig                              |   15 ++
 drivers/video/Makefile                             |    2 +
 drivers/video/of_display_timing.c                  |  239 ++++++++++++++++++++
 drivers/video/of_videomode.c                       |   54 +++++
 include/video/of_display_timing.h                  |   20 ++
 include/video/of_videomode.h                       |   18 ++
 7 files changed, 457 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 include/video/of_display_timing.h
 create mode 100644 include/video/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timing.txt b/Documentation/devicetree/bindings/video/display-timing.txt
new file mode 100644
index 0000000..1500385
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timing.txt
@@ -0,0 +1,109 @@
+display-timing bindings
+===========+
+display-timings node
+--------------------
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: The native mode for the display, in case multiple modes are
+		provided. When omitted, assume the first node is the native.
+
+timing subnode
+--------------
+
+required properties:
+ - hactive, vactive: display resolution
+ - hfront-porch, hback-porch, hsync-len: horizontal display timing parameters
+   in pixels
+   vfront-porch, vback-porch, vsync-len: vertical display timing parameters in
+   lines
+ - clock-frequency: display clock in Hz
+
+optional properties:
+ - hsync-active: hsync pulse is active low/high/ignored
+ - vsync-active: vsync pulse is active low/high/ignored
+ - de-active: data-enable pulse is active low/high/ignored
+ - pixelclk-active: with
+			- active high = drive pixel data on rising edge/
+					sample data on falling edge
+			- active low  = drive pixel data on falling edge/
+					sample data on rising edge
+			- ignored     = ignored
+ - interlaced (bool): boolean to enable interlaced mode
+ - doublescan (bool): boolean to enable doublescan mode
+
+All the optional properties that are not bool follow the following logic:
+    <1>: high active
+    <0>: low active
+    omitted: not used on hardware
+
+There are different ways of describing the capabilities of a display. The
+devicetree representation corresponds to the one commonly found in datasheets
+for displays. If a display supports multiple signal timings, the native-mode
+can be specified.
+
+The parameters are defined as:
+
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vback_porch                 |          |       |
+  |          |        ↓                            |          |       |
+  +----------#######################################----------+-------+
+  |          #        ↑                            #          |       |
+  |          #        |                            #          |       |
+  |  hback   #        |                            #  hfront  | hsync |
+  |   porch  #        |       hactive              #  porch   |  len  |
+  |<-------->#<-------+--------------------------->#<-------->|<----->|
+  |          #        |                            #          |       |
+  |          #        |vactive                     #          |       |
+  |          #        |                            #          |       |
+  |          #        ↓                            #          |       |
+  +----------#######################################----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vfront_porch                |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+  |          |        ↑                            |          |       |
+  |          |        |vsync_len                   |          |       |
+  |          |        ↓                            |          |       |
+  +----------+-------------------------------------+----------+-------+
+
+Example:
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: 1080p24 {
+			/* 1920x1080p24 */
+			clock-frequency = <52000000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hfront-porch = <25>;
+			hback-porch = <25>;
+			hsync-len = <25>;
+			vback-porch = <2>;
+			vfront-porch = <2>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+		};
+	};
+
+Every required property also supports the use of ranges, so the commonly used
+datasheet description with minimum, typical and maximum values can be used.
+
+Example:
+
+	timing1: timing {
+		/* 1920x1080p24 */
+		clock-frequency = <148500000>;
+		hactive = <1920>;
+		vactive = <1080>;
+		hsync-len = <0 44 60>;
+		hfront-porch = <80 88 95>;
+		hback-porch = <100 148 160>;
+		vfront-porch = <0 4 6>;
+		vback-porch = <0 36 50>;
+		vsync-len = <0 5 6>;
+	};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 09a8f0d..807c7fa 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,21 @@ config DISPLAY_TIMING
 config VIDEOMODE
        bool
 
+config OF_DISPLAY_TIMING
+	bool "Enable device tree display timing support"
+	depends on OF
+	select DISPLAY_TIMING
+	help
+	  helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+	bool "Enable device tree videomode support"
+	depends on OF
+	select VIDEOMODE
+	select OF_DISPLAY_TIMING
+	help
+	  helper to get videomodes from the devicetree
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e0dd820..f592f3b 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -169,4 +169,6 @@ obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
 obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
+obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o
 obj-$(CONFIG_VIDEOMODE) += videomode.o
+obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
new file mode 100644
index 0000000..13ecd98
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,239 @@
+/*
+ * OF helpers for parsing display timings
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/export.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+
+/**
+ * parse_timing_property - parse timing_entry from device_node
+ * @np: device_node with the property
+ * @name: name of the property
+ * @result: will be set to the return value
+ *
+ * DESCRIPTION:
+ * Every display_timing can be specified with either just the typical value or
+ * a range consisting of min/typ/max. This function helps handling this
+ **/
+static int parse_timing_property(struct device_node *np, const char *name,
+			  struct timing_entry *result)
+{
+	struct property *prop;
+	int length, cells, ret;
+
+	prop = of_find_property(np, name, &length);
+	if (!prop) {
+		pr_err("%s: could not find property %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	cells = length / sizeof(u32);
+	if (cells = 1) {
+		ret = of_property_read_u32(np, name, &result->typ);
+		result->min = result->typ;
+		result->max = result->typ;
+	} else if (cells = 3) {
+		ret = of_property_read_u32_array(np, name, &result->min, cells);
+	} else {
+		pr_err("%s: illegal timing specification in %s\n",
+			of_node_full_name(np), name);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+/**
+ * of_get_display_timing - parse display_timing entry from device_node
+ * @np: device_node with the properties
+ **/
+static struct display_timing *of_get_display_timing(struct device_node *np)
+{
+	struct display_timing *dt;
+	u32 val = 0;
+	int ret = 0;
+
+	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
+	if (!dt) {
+		pr_err("%s: could not allocate display_timing struct\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	ret |= parse_timing_property(np, "hback-porch", &dt->hback_porch);
+	ret |= parse_timing_property(np, "hfront-porch", &dt->hfront_porch);
+	ret |= parse_timing_property(np, "hactive", &dt->hactive);
+	ret |= parse_timing_property(np, "hsync-len", &dt->hsync_len);
+	ret |= parse_timing_property(np, "vback-porch", &dt->vback_porch);
+	ret |= parse_timing_property(np, "vfront-porch", &dt->vfront_porch);
+	ret |= parse_timing_property(np, "vactive", &dt->vactive);
+	ret |= parse_timing_property(np, "vsync-len", &dt->vsync_len);
+	ret |= parse_timing_property(np, "clock-frequency", &dt->pixelclock);
+
+	dt->dmt_flags = 0;
+	dt->data_flags = 0;
+	if (!of_property_read_u32(np, "vsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_VSYNC_HIGH :
+				VESA_DMT_VSYNC_LOW;
+	if (!of_property_read_u32(np, "hsync-active", &val))
+		dt->dmt_flags |= val ? VESA_DMT_HSYNC_HIGH :
+				VESA_DMT_HSYNC_LOW;
+	if (!of_property_read_u32(np, "de-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_DE_HIGH :
+				DISPLAY_FLAGS_DE_LOW;
+	if (!of_property_read_u32(np, "pixelclk-active", &val))
+		dt->data_flags |= val ? DISPLAY_FLAGS_PIXDATA_POSEDGE :
+				DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+
+	if (of_property_read_bool(np, "interlaced"))
+		dt->data_flags |= DISPLAY_FLAGS_INTERLACED;
+	if (of_property_read_bool(np, "doublescan"))
+		dt->data_flags |= DISPLAY_FLAGS_DOUBLESCAN;
+
+	if (ret) {
+		pr_err("%s: error reading timing properties\n",
+			of_node_full_name(np));
+		kfree(dt);
+		return NULL;
+	}
+
+	return dt;
+}
+
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
+{
+	struct device_node *timings_np;
+	struct device_node *entry;
+	struct device_node *native_mode;
+	struct display_timings *disp;
+
+	if (!np) {
+		pr_err("%s: no devicenode given\n", of_node_full_name(np));
+		return NULL;
+	}
+
+	timings_np = of_find_node_by_name(np, "display-timings");
+	if (!timings_np) {
+		pr_err("%s: could not find display-timings node\n",
+			of_node_full_name(np));
+		return NULL;
+	}
+
+	disp = kzalloc(sizeof(*disp), GFP_KERNEL);
+	if (!disp) {
+		pr_err("%s: could not allocate struct disp'\n",
+			of_node_full_name(np));
+		goto dispfail;
+	}
+
+	entry = of_parse_phandle(timings_np, "native-mode", 0);
+	/* assume first child as native mode if none provided */
+	if (!entry)
+		entry = of_get_next_child(np, NULL);
+	/* if there is no child, it is useless to go on */
+	if (!entry) {
+		pr_err("%s: no timing specifications given\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	pr_debug("%s: using %s as default timing\n",
+		of_node_full_name(np), entry->name);
+
+	native_mode = entry;
+
+	disp->num_timings = of_get_child_count(timings_np);
+	if (disp->num_timings = 0) {
+		/* should never happen, as entry was already found above */
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->timings = kzalloc(sizeof(struct display_timing *) *
+				disp->num_timings, GFP_KERNEL);
+	if (!disp->timings) {
+		pr_err("%s: could not allocate timings array\n",
+			of_node_full_name(np));
+		goto entryfail;
+	}
+
+	disp->num_timings = 0;
+	disp->native_mode = 0;
+
+	for_each_child_of_node(timings_np, entry) {
+		struct display_timing *dt;
+
+		dt = of_get_display_timing(entry);
+		if (!dt) {
+			/*
+			 * to not encourage wrong devicetrees, fail in case of
+			 * an error
+			 */
+			pr_err("%s: error in timing %d\n",
+				of_node_full_name(np), disp->num_timings + 1);
+			goto timingfail;
+		}
+
+		if (native_mode = entry)
+			disp->native_mode = disp->num_timings;
+
+		disp->timings[disp->num_timings] = dt;
+		disp->num_timings++;
+	}
+	of_node_put(timings_np);
+	/*
+	 * native_mode points to the device_node returned by of_parse_phandle
+	 * therefore call of_node_put on it
+	 */
+	of_node_put(native_mode);
+
+	pr_debug("%s: got %d timings. Using timing #%d as default\n",
+		of_node_full_name(np), disp->num_timings,
+		disp->native_mode + 1);
+
+	return disp;
+
+timingfail:
+	if (native_mode)
+		of_node_put(native_mode);
+	display_timings_release(disp);
+entryfail:
+	kfree(disp);
+dispfail:
+	of_node_put(timings_np);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exist - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exist(struct device_node *np)
+{
+	struct device_node *timings_np;
+
+	if (!np)
+		return -EINVAL;
+
+	timings_np = of_parse_phandle(np, "display-timings", 0);
+	if (!timings_np)
+		return -EINVAL;
+
+	of_node_put(timings_np);
+	return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exist);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..5b8066c
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,54 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/of.h>
+#include <video/display_timing.h>
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ *	    (Set this to OF_USE_NATIVE_MODE to use whatever mode is
+ *	     specified as native mode in the DT.)
+ *
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index)
+{
+	struct display_timings *disp;
+	int ret;
+
+	disp = of_get_display_timings(np);
+	if (!disp) {
+		pr_err("%s: no timings specified\n", of_node_full_name(np));
+		return -EINVAL;
+	}
+
+	if (index = OF_USE_NATIVE_MODE)
+		index = disp->native_mode;
+
+	ret = videomode_from_timing(disp, vm, index);
+	if (ret)
+		return ret;
+
+	display_timings_release(disp);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
new file mode 100644
index 0000000..8016eb7
--- /dev/null
+++ b/include/video/of_display_timing.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMING_H
+#define __LINUX_OF_DISPLAY_TIMING_H
+
+struct device_node;
+struct display_timings;
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(struct device_node *np);
+int of_display_timings_exist(struct device_node *np);
+
+#endif
diff --git a/include/video/of_videomode.h b/include/video/of_videomode.h
new file mode 100644
index 0000000..a07efcc
--- /dev/null
+++ b/include/video/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+struct device_node;
+struct videomode;
+
+int of_get_videomode(struct device_node *np, struct videomode *vm,
+		     int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 4/7] fbmon: add videomode helpers
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..17ce135 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	/* prevent division by zero in KHZ2PICOS macro */
+	fbmode->pixclock = vm->pixelclock ?
+			KHZ2PICOS(vm->pixelclock / 1000) : 0;
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	/* prevent division by zero */
+	if (htotal && vtotal) {
+		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+	/* a mode must have htotal and vtotal != 0 or it is invalid */
+	} else {
+		fbmode->refresh = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..100a176 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 struct fb_info;
 struct device;
 struct file;
+struct videomode;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+				       struct fb_videomode *fbmode);
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..17ce135 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
@@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
 	kfree(timings);
 	return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+				struct fb_videomode *fbmode)
+{
+	unsigned int htotal, vtotal;
+
+	fbmode->xres = vm->hactive;
+	fbmode->left_margin = vm->hback_porch;
+	fbmode->right_margin = vm->hfront_porch;
+	fbmode->hsync_len = vm->hsync_len;
+
+	fbmode->yres = vm->vactive;
+	fbmode->upper_margin = vm->vback_porch;
+	fbmode->lower_margin = vm->vfront_porch;
+	fbmode->vsync_len = vm->vsync_len;
+
+	/* prevent division by zero in KHZ2PICOS macro */
+	fbmode->pixclock = vm->pixelclock ?
+			KHZ2PICOS(vm->pixelclock / 1000) : 0;
+
+	fbmode->sync = 0;
+	fbmode->vmode = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		fbmode->vmode |= FB_VMODE_INTERLACED;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		fbmode->vmode |= FB_VMODE_DOUBLE;
+	fbmode->flag = 0;
+
+	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+		 vm->hsync_len;
+	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+		 vm->vsync_len;
+	/* prevent division by zero */
+	if (htotal && vtotal) {
+		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
+	/* a mode must have htotal and vtotal != 0 or it is invalid */
+	} else {
+		fbmode->refresh = 0;
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c7a9571..100a176 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -19,6 +19,7 @@ struct vm_area_struct;
 struct fb_info;
 struct device;
 struct file;
+struct videomode;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+				       struct fb_videomode *fbmode);
+
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 5/7] fbmon: add of_videomode helpers
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/fbmon.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+			       struct fb_videomode *fb,
+			       int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
 				       struct fb_videomode *fbmode);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 5/7] fbmon: add of_videomode helpers
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/video/fbmon.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/fb.h    |    4 ++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <video/edid.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+	pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+		 m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+		 m->right_margin, m->upper_margin, m->lower_margin,
+		 m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+			int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	fb_videomode_from_videomode(&vm, fb);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	dump_fb_videomode(fb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF	1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+			       struct fb_videomode *fb,
+			       int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
 				       struct fb_videomode *fbmode);
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 6/7] drm_modes: add videomode helpers
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    5 +++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d8da30e..9f3f20b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else if (vm->dmt_flags & VESA_DMT_HSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->dmt_flags & VESA_DMT_VSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else if (vm->dmt_flags & VESA_DMT_VSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..d5c06ff 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,8 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct videomode;
+
 #include <drm/drm_os_linux.h>
 #include <drm/drm_hashtab.h>
 #include <drm/drm_mm.h>
@@ -1456,6 +1458,9 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+					   struct drm_display_mode *dmode);
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 6/7] drm_modes: add videomode helpers
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/gpu/drm/drm_modes.c |   37 +++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    5 +++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d8da30e..9f3f20b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/videomode.h>
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+				    struct drm_display_mode *dmode)
+{
+	dmode->hdisplay = vm->hactive;
+	dmode->hsync_start = dmode->hdisplay + vm->hfront_porch;
+	dmode->hsync_end = dmode->hsync_start + vm->hsync_len;
+	dmode->htotal = dmode->hsync_end + vm->hback_porch;
+
+	dmode->vdisplay = vm->vactive;
+	dmode->vsync_start = dmode->vdisplay + vm->vfront_porch;
+	dmode->vsync_end = dmode->vsync_start + vm->vsync_len;
+	dmode->vtotal = dmode->vsync_end + vm->vback_porch;
+
+	dmode->clock = vm->pixelclock / 1000;
+
+	dmode->flags = 0;
+	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PHSYNC;
+	else if (vm->dmt_flags & VESA_DMT_HSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NHSYNC;
+	if (vm->dmt_flags & VESA_DMT_VSYNC_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PVSYNC;
+	else if (vm->dmt_flags & VESA_DMT_VSYNC_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NVSYNC;
+	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
+		dmode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
+		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
+	drm_mode_set_name(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..d5c06ff 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,8 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct videomode;
+
 #include <drm/drm_os_linux.h>
 #include <drm/drm_hashtab.h>
 #include <drm/drm_mm.h>
@@ -1456,6 +1458,9 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 				  struct drm_cmdline_mode *cmd);
 
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+					   struct drm_display_mode *dmode);
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 7/7] drm_modes: add of_videomode helpers
  2013-01-25  9:01 ` Steffen Trumtrar
@ 2013-01-25  9:01   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/gpu/drm/drm_modes.c |   33 +++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 9f3f20b..04fa6f1 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 
 /**
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	drm_display_mode_from_videomode(&vm, dmode);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d5c06ff..fcc9d23 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,7 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct device_node;
 struct videomode;
 
 #include <drm/drm_os_linux.h>
@@ -1460,6 +1461,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 
 extern int drm_display_mode_from_videomode(const struct videomode *vm,
 					   struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   int index);
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v17 7/7] drm_modes: add of_videomode helpers
@ 2013-01-25  9:01   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-01-25  9:01 UTC (permalink / raw)
  To: devicetree-discuss, Dave Airlie
  Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
	Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
	linux-media, Tomi Valkeinen, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Afzal Mohammed <Afzal@ti.com>
Tested-by: Rob Clark <robclark@gmail.com>
Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
---
 drivers/gpu/drm/drm_modes.c |   33 +++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |    4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 9f3f20b..04fa6f1 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include <linux/export.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
+#include <video/of_videomode.h>
 #include <video/videomode.h>
 
 /**
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
 EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np,
+			    struct drm_display_mode *dmode, int index)
+{
+	struct videomode vm;
+	int ret;
+
+	ret = of_get_videomode(np, &vm, index);
+	if (ret)
+		return ret;
+
+	drm_display_mode_from_videomode(&vm, dmode);
+
+	pr_debug("%s: got %dx%d display mode from %s\n",
+		of_node_full_name(np), vm.hactive, vm.vactive, np->name);
+	drm_mode_debug_printmodeline(dmode);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d5c06ff..fcc9d23 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -85,6 +85,7 @@ struct module;
 struct drm_file;
 struct drm_device;
 
+struct device_node;
 struct videomode;
 
 #include <drm/drm_os_linux.h>
@@ -1460,6 +1461,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 
 extern int drm_display_mode_from_videomode(const struct videomode *vm,
 					   struct drm_display_mode *dmode);
+extern int of_get_drm_display_mode(struct device_node *np,
+				   struct drm_display_mode *dmode,
+				   int index);
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* RE: [PATCH v17 4/7] fbmon: add videomode helpers
  2013-01-25  9:01   ` Steffen Trumtrar
  (?)
@ 2013-02-01  9:29     ` Jingoo Han
  -1 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:29 UTC (permalink / raw)
  To: 'Steffen Trumtrar'
  Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
	devicetree-discuss, 'Stephen Warren',
	linux-fbdev, dri-devel, 'Tomi Valkeinen',
	'Rob Herring', 'Laurent Pinchart',
	kernel, 'Guennady Liakhovetski',
	linux-media

On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> 
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Afzal Mohammed <Afzal@ti.com>
> Tested-by: Rob Clark <robclark@gmail.com>
> Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
> ---
>  drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <video/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	/* prevent division by zero in KHZ2PICOS macro */
> +	fbmode->pixclock = vm->pixelclock ?
> +			KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)

Hi Steffen Trumtrar,

Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:

    VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH


Best regards,
Jingoo Han

> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	/* prevent division by zero */
> +	if (htotal && vtotal) {
> +		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> +	/* a mode must have htotal and vtotal != 0 or it is invalid */
> +	} else {
> +		fbmode->refresh = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
>  struct fb_info;
>  struct device;
>  struct file;
> +struct videomode;
> 
>  /* Definitions below are used in the parsed monitor specs */
>  #define FB_DPMS_ACTIVE_OFF	1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> 
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-01  9:29     ` Jingoo Han
  0 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:29 UTC (permalink / raw)
  To: 'Steffen Trumtrar'
  Cc: devicetree-discuss, 'Dave Airlie',
	linux-fbdev, 'Mohammed, Afzal', 'Stephen Warren',
	'Florian Tobias Schandinat',
	dri-devel, 'Tomi Valkeinen', 'Rob Herring',
	'Laurent Pinchart',
	kernel, 'Guennady Liakhovetski',
	linux-media, 'Jingoo Han'

On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> 
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Afzal Mohammed <Afzal@ti.com>
> Tested-by: Rob Clark <robclark@gmail.com>
> Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
> ---
>  drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <video/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	/* prevent division by zero in KHZ2PICOS macro */
> +	fbmode->pixclock = vm->pixelclock ?
> +			KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)

Hi Steffen Trumtrar,

Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:

    VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH


Best regards,
Jingoo Han

> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	/* prevent division by zero */
> +	if (htotal && vtotal) {
> +		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> +	/* a mode must have htotal and vtotal != 0 or it is invalid */
> +	} else {
> +		fbmode->refresh = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
>  struct fb_info;
>  struct device;
>  struct file;
> +struct videomode;
> 
>  /* Definitions below are used in the parsed monitor specs */
>  #define FB_DPMS_ACTIVE_OFF	1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> 
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-01  9:29     ` Jingoo Han
  0 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:29 UTC (permalink / raw)
  To: 'Steffen Trumtrar'
  Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
	devicetree-discuss, 'Stephen Warren',
	linux-fbdev, dri-devel, 'Tomi Valkeinen',
	'Rob Herring', 'Laurent Pinchart',
	kernel, 'Guennady Liakhovetski',
	linux-media

On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> 
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Afzal Mohammed <Afzal@ti.com>
> Tested-by: Rob Clark <robclark@gmail.com>
> Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
> ---
>  drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <video/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	/* prevent division by zero in KHZ2PICOS macro */
> +	fbmode->pixclock = vm->pixelclock ?
> +			KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)

Hi Steffen Trumtrar,

Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:

    VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH


Best regards,
Jingoo Han

> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	/* prevent division by zero */
> +	if (htotal && vtotal) {
> +		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> +	/* a mode must have htotal and vtotal != 0 or it is invalid */
> +	} else {
> +		fbmode->refresh = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
>  struct fb_info;
>  struct device;
>  struct file;
> +struct videomode;
> 
>  /* Definitions below are used in the parsed monitor specs */
>  #define FB_DPMS_ACTIVE_OFF	1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> 
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
  2013-01-25  9:01   ` Steffen Trumtrar
  (?)
@ 2013-02-01  9:33 ` Jingoo Han
  -1 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:33 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: Mohammed, Afzal, Jingoo Han, Florian Tobias Schandinat,
	Dave Airlie, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> 
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Reviewed-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> Acked-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> Tested-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
> Tested-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Tested-by: Afzal Mohammed <Afzal-l0cyMroinI0@public.gmane.org>
> Tested-by: Rob Clark <robclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Tested-by: Leela Krishna Amudala <leelakrishna.a-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <video/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	/* prevent division by zero in KHZ2PICOS macro */
> +	fbmode->pixclock = vm->pixelclock ?
> +			KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)

Hi Steffen Trumtrar,

Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:

    VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH


Best regards,
Jingoo Han

> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	/* prevent division by zero */
> +	if (htotal && vtotal) {
> +		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> +	/* a mode must have htotal and vtotal != 0 or it is invalid */
> +	} else {
> +		fbmode->refresh = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
>  struct fb_info;
>  struct device;
>  struct file;
> +struct videomode;
> 
>  /* Definitions below are used in the parsed monitor specs */
>  #define FB_DPMS_ACTIVE_OFF	1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> 
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-01  9:33 ` Jingoo Han
  0 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:33 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: Mohammed, Afzal, Jingoo Han, Florian Tobias Schandinat,
	Dave Airlie, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

T24gRnJpZGF5LCBKYW51YXJ5IDI1LCAyMDEzIDY6MDIgUE0sIFN0ZWZmZW4gVHJ1bXRyYXIgd3Jv
dGUNCj4gDQo+IEFkZCBhIGZ1bmN0aW9uIHRvIGNvbnZlcnQgZnJvbSB0aGUgZ2VuZXJpYyB2aWRl
b21vZGUgdG8gYSBmYl92aWRlb21vZGUuDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBTdGVmZmVuIFRy
dW10cmFyIDxzLnRydW10cmFyQHBlbmd1dHJvbml4LmRlPg0KPiBSZXZpZXdlZC1ieTogVGhpZXJy
eSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRlPg0KPiBBY2tlZC1ieTog
VGhpZXJyeSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRlPg0KPiBUZXN0
ZWQtYnk6IFRoaWVycnkgUmVkaW5nIDx0aGllcnJ5LnJlZGluZ0BhdmlvbmljLWRlc2lnbi5kZT4N
Cj4gVGVzdGVkLWJ5OiBQaGlsaXBwIFphYmVsIDxwLnphYmVsQHBlbmd1dHJvbml4LmRlPg0KPiBS
ZXZpZXdlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9h
cmQuY29tPg0KPiBBY2tlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBp
ZGVhc29uYm9hcmQuY29tPg0KPiBUZXN0ZWQtYnk6IEFmemFsIE1vaGFtbWVkIDxBZnphbEB0aS5j
b20+DQo+IFRlc3RlZC1ieTogUm9iIENsYXJrIDxyb2JjbGFya0BnbWFpbC5jb20+DQo+IFRlc3Rl
ZC1ieTogTGVlbGEgS3Jpc2huYSBBbXVkYWxhIDxsZWVsYWtyaXNobmEuYUBnbWFpbC5jb20+DQo+
IC0tLQ0KPiAgZHJpdmVycy92aWRlby9mYm1vbi5jIHwgICA1MiArKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrDQo+ICBpbmNsdWRlL2xpbnV4L2ZiLmggICAg
fCAgICA0ICsrKysNCj4gIDIgZmlsZXMgY2hhbmdlZCwgNTYgaW5zZXJ0aW9ucygrKQ0KPiANCj4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vZmJtb24uYyBiL2RyaXZlcnMvdmlkZW8vZmJtb24u
Yw0KPiBpbmRleCBjZWY2NTU3Li4xN2NlMTM1IDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL3ZpZGVv
L2ZibW9uLmMNCj4gKysrIGIvZHJpdmVycy92aWRlby9mYm1vbi5jDQo+IEBAIC0zMSw2ICszMSw3
IEBADQo+ICAjaW5jbHVkZSA8bGludXgvcGNpLmg+DQo+ICAjaW5jbHVkZSA8bGludXgvc2xhYi5o
Pg0KPiAgI2luY2x1ZGUgPHZpZGVvL2VkaWQuaD4NCj4gKyNpbmNsdWRlIDx2aWRlby92aWRlb21v
ZGUuaD4NCj4gICNpZmRlZiBDT05GSUdfUFBDX09GDQo+ICAjaW5jbHVkZSA8YXNtL3Byb20uaD4N
Cj4gICNpbmNsdWRlIDxhc20vcGNpLWJyaWRnZS5oPg0KPiBAQCAtMTM3Myw2ICsxMzc0LDU3IEBA
IGludCBmYl9nZXRfbW9kZShpbnQgZmxhZ3MsIHUzMiB2YWwsIHN0cnVjdCBmYl92YXJfc2NyZWVu
aW5mbyAqdmFyLCBzdHJ1Y3QgZmJfaW5mDQo+ICAJa2ZyZWUodGltaW5ncyk7DQo+ICAJcmV0dXJu
IGVycjsNCj4gIH0NCj4gKw0KPiArI2lmIElTX0VOQUJMRUQoQ09ORklHX1ZJREVPTU9ERSkNCj4g
K2ludCBmYl92aWRlb21vZGVfZnJvbV92aWRlb21vZGUoY29uc3Qgc3RydWN0IHZpZGVvbW9kZSAq
dm0sDQo+ICsJCQkJc3RydWN0IGZiX3ZpZGVvbW9kZSAqZmJtb2RlKQ0KPiArew0KPiArCXVuc2ln
bmVkIGludCBodG90YWwsIHZ0b3RhbDsNCj4gKw0KPiArCWZibW9kZS0+eHJlcyA9IHZtLT5oYWN0
aXZlOw0KPiArCWZibW9kZS0+bGVmdF9tYXJnaW4gPSB2bS0+aGJhY2tfcG9yY2g7DQo+ICsJZmJt
b2RlLT5yaWdodF9tYXJnaW4gPSB2bS0+aGZyb250X3BvcmNoOw0KPiArCWZibW9kZS0+aHN5bmNf
bGVuID0gdm0tPmhzeW5jX2xlbjsNCj4gKw0KPiArCWZibW9kZS0+eXJlcyA9IHZtLT52YWN0aXZl
Ow0KPiArCWZibW9kZS0+dXBwZXJfbWFyZ2luID0gdm0tPnZiYWNrX3BvcmNoOw0KPiArCWZibW9k
ZS0+bG93ZXJfbWFyZ2luID0gdm0tPnZmcm9udF9wb3JjaDsNCj4gKwlmYm1vZGUtPnZzeW5jX2xl
biA9IHZtLT52c3luY19sZW47DQo+ICsNCj4gKwkvKiBwcmV2ZW50IGRpdmlzaW9uIGJ5IHplcm8g
aW4gS0haMlBJQ09TIG1hY3JvICovDQo+ICsJZmJtb2RlLT5waXhjbG9jayA9IHZtLT5waXhlbGNs
b2NrID8NCj4gKwkJCUtIWjJQSUNPUyh2bS0+cGl4ZWxjbG9jayAvIDEwMDApIDogMDsNCj4gKw0K
PiArCWZibW9kZS0+c3luYyA9IDA7DQo+ICsJZmJtb2RlLT52bW9kZSA9IDA7DQo+ICsJaWYgKHZt
LT5kbXRfZmxhZ3MgJiBWRVNBX0RNVF9IU1lOQ19ISUdIKQ0KPiArCQlmYm1vZGUtPnN5bmMgfD0g
RkJfU1lOQ19IT1JfSElHSF9BQ1Q7DQo+ICsJaWYgKHZtLT5kbXRfZmxhZ3MgJiBWRVNBX0RNVF9I
U1lOQ19ISUdIKQ0KDQpIaSBTdGVmZmVuIFRydW10cmFyLA0KDQpVbSwgaXQgc2VlbXMgdG8gYmUg
YSB0eXBlLiAnSCdTWU5DIC0+ICdWJ1NZTkMNClRodXMsIGl0IHdvdWxkIGJlIGNoYW5nZWQgYXMg
YmVsb3c6DQoNCiAgICBWRVNBX0RNVF9IU1lOQ19ISUdIIC0+IFZFU0FfRE1UX1ZTWU5DX0hJR0gN
Cg0KDQpCZXN0IHJlZ2FyZHMsDQpKaW5nb28gSGFuDQoNCj4gKwkJZmJtb2RlLT5zeW5jIHw9IEZC
X1NZTkNfVkVSVF9ISUdIX0FDVDsNCj4gKwlpZiAodm0tPmRhdGFfZmxhZ3MgJiBESVNQTEFZX0ZM
QUdTX0lOVEVSTEFDRUQpDQo+ICsJCWZibW9kZS0+dm1vZGUgfD0gRkJfVk1PREVfSU5URVJMQUNF
RDsNCj4gKwlpZiAodm0tPmRhdGFfZmxhZ3MgJiBESVNQTEFZX0ZMQUdTX0RPVUJMRVNDQU4pDQo+
ICsJCWZibW9kZS0+dm1vZGUgfD0gRkJfVk1PREVfRE9VQkxFOw0KPiArCWZibW9kZS0+ZmxhZyA9
IDA7DQo+ICsNCj4gKwlodG90YWwgPSB2bS0+aGFjdGl2ZSArIHZtLT5oZnJvbnRfcG9yY2ggKyB2
bS0+aGJhY2tfcG9yY2ggKw0KPiArCQkgdm0tPmhzeW5jX2xlbjsNCj4gKwl2dG90YWwgPSB2bS0+
dmFjdGl2ZSArIHZtLT52ZnJvbnRfcG9yY2ggKyB2bS0+dmJhY2tfcG9yY2ggKw0KPiArCQkgdm0t
PnZzeW5jX2xlbjsNCj4gKwkvKiBwcmV2ZW50IGRpdmlzaW9uIGJ5IHplcm8gKi8NCj4gKwlpZiAo
aHRvdGFsICYmIHZ0b3RhbCkgew0KPiArCQlmYm1vZGUtPnJlZnJlc2ggPSB2bS0+cGl4ZWxjbG9j
ayAvIChodG90YWwgKiB2dG90YWwpOw0KPiArCS8qIGEgbW9kZSBtdXN0IGhhdmUgaHRvdGFsIGFu
ZCB2dG90YWwgIT0gMCBvciBpdCBpcyBpbnZhbGlkICovDQo+ICsJfSBlbHNlIHsNCj4gKwkJZmJt
b2RlLT5yZWZyZXNoID0gMDsNCj4gKwkJcmV0dXJuIC1FSU5WQUw7DQo+ICsJfQ0KPiArDQo+ICsJ
cmV0dXJuIDA7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQTChmYl92aWRlb21vZGVfZnJvbV92
aWRlb21vZGUpOw0KPiArI2VuZGlmDQo+ICsNCj4gICNlbHNlDQo+ICBpbnQgZmJfcGFyc2VfZWRp
ZCh1bnNpZ25lZCBjaGFyICplZGlkLCBzdHJ1Y3QgZmJfdmFyX3NjcmVlbmluZm8gKnZhcikNCj4g
IHsNCj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvZmIuaCBiL2luY2x1ZGUvbGludXgvZmIu
aA0KPiBpbmRleCBjN2E5NTcxLi4xMDBhMTc2IDEwMDY0NA0KPiAtLS0gYS9pbmNsdWRlL2xpbnV4
L2ZiLmgNCj4gKysrIGIvaW5jbHVkZS9saW51eC9mYi5oDQo+IEBAIC0xOSw2ICsxOSw3IEBAIHN0
cnVjdCB2bV9hcmVhX3N0cnVjdDsNCj4gIHN0cnVjdCBmYl9pbmZvOw0KPiAgc3RydWN0IGRldmlj
ZTsNCj4gIHN0cnVjdCBmaWxlOw0KPiArc3RydWN0IHZpZGVvbW9kZTsNCj4gDQo+ICAvKiBEZWZp
bml0aW9ucyBiZWxvdyBhcmUgdXNlZCBpbiB0aGUgcGFyc2VkIG1vbml0b3Igc3BlY3MgKi8NCj4g
ICNkZWZpbmUgRkJfRFBNU19BQ1RJVkVfT0ZGCTENCj4gQEAgLTcxNCw2ICs3MTUsOSBAQCBleHRl
cm4gdm9pZCBmYl9kZXN0cm95X21vZGVkYihzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2RlZGIpOw0K
PiAgZXh0ZXJuIGludCBmYl9maW5kX21vZGVfY3Z0KHN0cnVjdCBmYl92aWRlb21vZGUgKm1vZGUs
IGludCBtYXJnaW5zLCBpbnQgcmIpOw0KPiAgZXh0ZXJuIHVuc2lnbmVkIGNoYXIgKmZiX2RkY19y
ZWFkKHN0cnVjdCBpMmNfYWRhcHRlciAqYWRhcHRlcik7DQo+IA0KPiArZXh0ZXJuIGludCBmYl92
aWRlb21vZGVfZnJvbV92aWRlb21vZGUoY29uc3Qgc3RydWN0IHZpZGVvbW9kZSAqdm0sDQo+ICsJ
CQkJICAgICAgIHN0cnVjdCBmYl92aWRlb21vZGUgKmZibW9kZSk7DQo+ICsNCj4gIC8qIGRyaXZl
cnMvdmlkZW8vbW9kZWRiLmMgKi8NCj4gICNkZWZpbmUgVkVTQV9NT0RFREJfU0laRSAzNA0KPiAg
ZXh0ZXJuIHZvaWQgZmJfdmFyX3RvX3ZpZGVvbW9kZShzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2Rl
LA0KPiAtLQ0KPiAxLjcuMTAuNA0KPiANCj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX18NCj4gZHJpLWRldmVsIG1haWxpbmcgbGlzdA0KPiBkcmktZGV2ZWxA
bGlzdHMuZnJlZWRlc2t0b3Aub3JnDQo+IGh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp
bG1hbi9saXN0aW5mby9kcmktZGV2ZWwNCg=



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-01  9:33 ` Jingoo Han
  0 siblings, 0 replies; 38+ messages in thread
From: Jingoo Han @ 2013-02-01  9:33 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Dave Airlie, linux-fbdev, Mohammed, Afzal,
	Stephen Warren, Florian Tobias Schandinat, dri-devel,
	Tomi Valkeinen, Rob Herring, Laurent Pinchart, kernel,
	Guennady Liakhovetski, linux-media, Jingoo Han

On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> 
> Add a function to convert from the generic videomode to a fb_videomode.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Afzal Mohammed <Afzal@ti.com>
> Tested-by: Rob Clark <robclark@gmail.com>
> Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
> ---
>  drivers/video/fbmon.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fb.h    |    4 ++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
>  #include <linux/pci.h>
>  #include <linux/slab.h>
>  #include <video/edid.h>
> +#include <video/videomode.h>
>  #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
>  	kfree(timings);
>  	return err;
>  }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> +				struct fb_videomode *fbmode)
> +{
> +	unsigned int htotal, vtotal;
> +
> +	fbmode->xres = vm->hactive;
> +	fbmode->left_margin = vm->hback_porch;
> +	fbmode->right_margin = vm->hfront_porch;
> +	fbmode->hsync_len = vm->hsync_len;
> +
> +	fbmode->yres = vm->vactive;
> +	fbmode->upper_margin = vm->vback_porch;
> +	fbmode->lower_margin = vm->vfront_porch;
> +	fbmode->vsync_len = vm->vsync_len;
> +
> +	/* prevent division by zero in KHZ2PICOS macro */
> +	fbmode->pixclock = vm->pixelclock ?
> +			KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> +	fbmode->sync = 0;
> +	fbmode->vmode = 0;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)

Hi Steffen Trumtrar,

Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:

    VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH


Best regards,
Jingoo Han

> +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> +		fbmode->vmode |= FB_VMODE_INTERLACED;
> +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> +		fbmode->vmode |= FB_VMODE_DOUBLE;
> +	fbmode->flag = 0;
> +
> +	htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> +		 vm->hsync_len;
> +	vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> +		 vm->vsync_len;
> +	/* prevent division by zero */
> +	if (htotal && vtotal) {
> +		fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> +	/* a mode must have htotal and vtotal != 0 or it is invalid */
> +	} else {
> +		fbmode->refresh = 0;
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
>  #else
>  int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
>  {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
>  struct fb_info;
>  struct device;
>  struct file;
> +struct videomode;
> 
>  /* Definitions below are used in the parsed monitor specs */
>  #define FB_DPMS_ACTIVE_OFF	1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
>  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
>  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> 
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> +				       struct fb_videomode *fbmode);
> +
>  /* drivers/video/modedb.c */
>  #define VESA_MODEDB_SIZE 34
>  extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
  2013-02-01  9:29     ` Jingoo Han
  (?)
@ 2013-02-05 18:29         ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-02-05 18:29 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
	'Dave Airlie',
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	'Tomi Valkeinen', 'Laurent Pinchart',
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, 'Guennady Liakhovetski',
	linux-media-u79uwXL29TY76Z2rM5mHXA

Hi!

On Fri, Feb 01, 2013 at 06:29:50PM +0900, Jingoo Han wrote:
> On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> > 
> > +	fbmode->sync = 0;
> > +	fbmode->vmode = 0;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> > +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> 
> Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
> Thus, it would be changed as below:
> 
>     VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH

Damn. You are right, that is a typo. But I guess some maintainer (Dave) really,
really wants to take the series now and this can wait for an -rc. No?!  ;-)

Thanks,
Steffen

> 
> > +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> > +		fbmode->vmode |= FB_VMODE_INTERLACED;
> > +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> > +		fbmode->vmode |= FB_VMODE_DOUBLE;
> > +	fbmode->flag = 0;
> > +

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-05 18:29         ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-02-05 18:29 UTC (permalink / raw)
  To: Jingoo Han
  Cc: devicetree-discuss, 'Dave Airlie',
	linux-fbdev, 'Mohammed, Afzal', 'Stephen Warren',
	'Florian Tobias Schandinat',
	dri-devel, 'Tomi Valkeinen', 'Rob Herring',
	'Laurent Pinchart',
	kernel, 'Guennady Liakhovetski',
	linux-media

Hi!

On Fri, Feb 01, 2013 at 06:29:50PM +0900, Jingoo Han wrote:
> On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> > 
> > +	fbmode->sync = 0;
> > +	fbmode->vmode = 0;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> > +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> 
> Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
> Thus, it would be changed as below:
> 
>     VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH

Damn. You are right, that is a typo. But I guess some maintainer (Dave) really,
really wants to take the series now and this can wait for an -rc. No?!  ;-)

Thanks,
Steffen

> 
> > +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> > +		fbmode->vmode |= FB_VMODE_INTERLACED;
> > +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> > +		fbmode->vmode |= FB_VMODE_DOUBLE;
> > +	fbmode->flag = 0;
> > +

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 4/7] fbmon: add videomode helpers
@ 2013-02-05 18:29         ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-02-05 18:29 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
	'Dave Airlie',
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	'Tomi Valkeinen', 'Laurent Pinchart',
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, 'Guennady Liakhovetski',
	linux-media-u79uwXL29TY76Z2rM5mHXA

Hi!

On Fri, Feb 01, 2013 at 06:29:50PM +0900, Jingoo Han wrote:
> On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
> > 
> > +	fbmode->sync = 0;
> > +	fbmode->vmode = 0;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> > +		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > +	if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> 
> Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
> Thus, it would be changed as below:
> 
>     VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH

Damn. You are right, that is a typo. But I guess some maintainer (Dave) really,
really wants to take the series now and this can wait for an -rc. No?!  ;-)

Thanks,
Steffen

> 
> > +		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > +	if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> > +		fbmode->vmode |= FB_VMODE_INTERLACED;
> > +	if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> > +		fbmode->vmode |= FB_VMODE_DOUBLE;
> > +	fbmode->flag = 0;
> > +

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
  2013-01-25  9:01   ` Steffen Trumtrar
@ 2013-02-18 14:09     ` Tomi Valkeinen
  -1 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-18 14:09 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel

Hi Steffen,

On 2013-01-25 11:01, Steffen Trumtrar wrote:

> +/* VESA display monitor timing parameters */
> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> +
> +/* display specific flags */
> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)

<snip>

> +	unsigned int dmt_flags;	/* VESA DMT flags */
> +	unsigned int data_flags; /* video data flags */

Why did you go for this approach? To be able to represent
true/false/not-specified?

Would it be simpler to just have "flags" field? What does it give us to
have those two separately?

Should the above say raising edge/falling edge instead of positive
edge/negative edge?

 Tomi

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-18 14:09     ` Tomi Valkeinen
  0 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-18 14:09 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel

Hi Steffen,

On 2013-01-25 11:01, Steffen Trumtrar wrote:

> +/* VESA display monitor timing parameters */
> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> +
> +/* display specific flags */
> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)

<snip>

> +	unsigned int dmt_flags;	/* VESA DMT flags */
> +	unsigned int data_flags; /* video data flags */

Why did you go for this approach? To be able to represent
true/false/not-specified?

Would it be simpler to just have "flags" field? What does it give us to
have those two separately?

Should the above say raising edge/falling edge instead of positive
edge/negative edge?

 Tomi


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
  2013-02-18 14:09     ` Tomi Valkeinen
  (?)
@ 2013-02-27 15:45         ` Tomi Valkeinen
  -1 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 15:45 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 1178 bytes --]

Ping.

On 2013-02-18 16:09, Tomi Valkeinen wrote:
> Hi Steffen,
> 
> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> 
>> +/* VESA display monitor timing parameters */
>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>> +
>> +/* display specific flags */
>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> 
> <snip>
> 
>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>> +	unsigned int data_flags; /* video data flags */
> 
> Why did you go for this approach? To be able to represent
> true/false/not-specified?
> 
> Would it be simpler to just have "flags" field? What does it give us to
> have those two separately?
> 
> Should the above say raising edge/falling edge instead of positive
> edge/negative edge?
> 
>  Tomi
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-27 15:45         ` Tomi Valkeinen
  0 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 15:45 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Tomi Valkeinen,
	Stephen Warren, Florian Tobias Schandinat, Rob Clark,
	Leela Krishna Amudala, Mohammed, Afzal, kernel

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

Ping.

On 2013-02-18 16:09, Tomi Valkeinen wrote:
> Hi Steffen,
> 
> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> 
>> +/* VESA display monitor timing parameters */
>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>> +
>> +/* display specific flags */
>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> 
> <snip>
> 
>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>> +	unsigned int data_flags; /* video data flags */
> 
> Why did you go for this approach? To be able to represent
> true/false/not-specified?
> 
> Would it be simpler to just have "flags" field? What does it give us to
> have those two separately?
> 
> Should the above say raising edge/falling edge instead of positive
> edge/negative edge?
> 
>  Tomi
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-27 15:45         ` Tomi Valkeinen
  0 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 15:45 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

Ping.

On 2013-02-18 16:09, Tomi Valkeinen wrote:
> Hi Steffen,
> 
> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> 
>> +/* VESA display monitor timing parameters */
>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>> +
>> +/* display specific flags */
>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> 
> <snip>
> 
>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>> +	unsigned int data_flags; /* video data flags */
> 
> Why did you go for this approach? To be able to represent
> true/false/not-specified?
> 
> Would it be simpler to just have "flags" field? What does it give us to
> have those two separately?
> 
> Should the above say raising edge/falling edge instead of positive
> edge/negative edge?
> 
>  Tomi
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
  2013-02-27 15:45         ` Tomi Valkeinen
@ 2013-02-27 16:05           ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-02-27 16:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Ah, sorry. Forgot to answer this.

On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
> Ping.
> 
> On 2013-02-18 16:09, Tomi Valkeinen wrote:
> > Hi Steffen,
> > 
> > On 2013-01-25 11:01, Steffen Trumtrar wrote:
> > 
> >> +/* VESA display monitor timing parameters */
> >> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> >> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> >> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> >> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> >> +
> >> +/* display specific flags */
> >> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> >> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> >> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> >> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> >> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> >> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> > 
> > <snip>
> > 
> >> +	unsigned int dmt_flags;	/* VESA DMT flags */
> >> +	unsigned int data_flags; /* video data flags */
> > 
> > Why did you go for this approach? To be able to represent
> > true/false/not-specified?
> > 

We decided somewhere between v3 and v8 (I think), that those flags can be
high/low/ignored.

> > Would it be simpler to just have "flags" field? What does it give us to
> > have those two separately?
> > 

I decided to split them, so it is clear that some flags are VESA defined and
the others are "invented" for the display-timings framework and may be
extended.

> > Should the above say raising edge/falling edge instead of positive
> > edge/negative edge?
> > 

Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
pretty natural to me :-) ). I don't know what others are thinking though.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-27 16:05           ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-02-27 16:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Ah, sorry. Forgot to answer this.

On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
> Ping.
> 
> On 2013-02-18 16:09, Tomi Valkeinen wrote:
> > Hi Steffen,
> > 
> > On 2013-01-25 11:01, Steffen Trumtrar wrote:
> > 
> >> +/* VESA display monitor timing parameters */
> >> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> >> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> >> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> >> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> >> +
> >> +/* display specific flags */
> >> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> >> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> >> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> >> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> >> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> >> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> > 
> > <snip>
> > 
> >> +	unsigned int dmt_flags;	/* VESA DMT flags */
> >> +	unsigned int data_flags; /* video data flags */
> > 
> > Why did you go for this approach? To be able to represent
> > true/false/not-specified?
> > 

We decided somewhere between v3 and v8 (I think), that those flags can be
high/low/ignored.

> > Would it be simpler to just have "flags" field? What does it give us to
> > have those two separately?
> > 

I decided to split them, so it is clear that some flags are VESA defined and
the others are "invented" for the display-timings framework and may be
extended.

> > Should the above say raising edge/falling edge instead of positive
> > edge/negative edge?
> > 

Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
pretty natural to me :-) ). I don't know what others are thinking though.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
  2013-02-27 16:05           ` Steffen Trumtrar
  (?)
@ 2013-02-27 16:13               ` Tomi Valkeinen
  -1 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 16:13 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 2457 bytes --]

On 2013-02-27 18:05, Steffen Trumtrar wrote:
> Ah, sorry. Forgot to answer this.
> 
> On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
>> Ping.
>>
>> On 2013-02-18 16:09, Tomi Valkeinen wrote:
>>> Hi Steffen,
>>>
>>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
>>>
>>>> +/* VESA display monitor timing parameters */
>>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>>>> +
>>>> +/* display specific flags */
>>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
>>>
>>> <snip>
>>>
>>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>>>> +	unsigned int data_flags; /* video data flags */
>>>
>>> Why did you go for this approach? To be able to represent
>>> true/false/not-specified?
>>>
> 
> We decided somewhere between v3 and v8 (I think), that those flags can be
> high/low/ignored.

Okay. Why aren't they enums, though? That always makes more clear which
defines are to be used with which fields.

>>> Would it be simpler to just have "flags" field? What does it give us to
>>> have those two separately?
>>>
> 
> I decided to split them, so it is clear that some flags are VESA defined and
> the others are "invented" for the display-timings framework and may be
> extended.

Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
complicate handling the flags =).

>>> Should the above say raising edge/falling edge instead of positive
>>> edge/negative edge?
>>>
> 
> Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> pretty natural to me :-) ). I don't know what others are thinking though.

I guess it's quite clear, but it's still different terms than used
elsewhere, e.g. documentation for videomodes.

Another thing I noticed while using the new videomode, display_timings.h
has a few names that are quite short and generic. Like "TE_MIN", which
is now a global define. And "timing_entry". Either name could be well
used internally in some .c file, and could easily clash.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-27 16:13               ` Tomi Valkeinen
  0 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 16:13 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

On 2013-02-27 18:05, Steffen Trumtrar wrote:
> Ah, sorry. Forgot to answer this.
> 
> On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
>> Ping.
>>
>> On 2013-02-18 16:09, Tomi Valkeinen wrote:
>>> Hi Steffen,
>>>
>>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
>>>
>>>> +/* VESA display monitor timing parameters */
>>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>>>> +
>>>> +/* display specific flags */
>>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
>>>
>>> <snip>
>>>
>>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>>>> +	unsigned int data_flags; /* video data flags */
>>>
>>> Why did you go for this approach? To be able to represent
>>> true/false/not-specified?
>>>
> 
> We decided somewhere between v3 and v8 (I think), that those flags can be
> high/low/ignored.

Okay. Why aren't they enums, though? That always makes more clear which
defines are to be used with which fields.

>>> Would it be simpler to just have "flags" field? What does it give us to
>>> have those two separately?
>>>
> 
> I decided to split them, so it is clear that some flags are VESA defined and
> the others are "invented" for the display-timings framework and may be
> extended.

Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
complicate handling the flags =).

>>> Should the above say raising edge/falling edge instead of positive
>>> edge/negative edge?
>>>
> 
> Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> pretty natural to me :-) ). I don't know what others are thinking though.

I guess it's quite clear, but it's still different terms than used
elsewhere, e.g. documentation for videomodes.

Another thing I noticed while using the new videomode, display_timings.h
has a few names that are quite short and generic. Like "TE_MIN", which
is now a global define. And "timing_entry". Either name could be well
used internally in some .c file, and could easily clash.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-02-27 16:13               ` Tomi Valkeinen
  0 siblings, 0 replies; 38+ messages in thread
From: Tomi Valkeinen @ 2013-02-27 16:13 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

On 2013-02-27 18:05, Steffen Trumtrar wrote:
> Ah, sorry. Forgot to answer this.
> 
> On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
>> Ping.
>>
>> On 2013-02-18 16:09, Tomi Valkeinen wrote:
>>> Hi Steffen,
>>>
>>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
>>>
>>>> +/* VESA display monitor timing parameters */
>>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
>>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
>>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
>>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
>>>> +
>>>> +/* display specific flags */
>>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
>>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
>>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
>>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
>>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
>>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
>>>
>>> <snip>
>>>
>>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
>>>> +	unsigned int data_flags; /* video data flags */
>>>
>>> Why did you go for this approach? To be able to represent
>>> true/false/not-specified?
>>>
> 
> We decided somewhere between v3 and v8 (I think), that those flags can be
> high/low/ignored.

Okay. Why aren't they enums, though? That always makes more clear which
defines are to be used with which fields.

>>> Would it be simpler to just have "flags" field? What does it give us to
>>> have those two separately?
>>>
> 
> I decided to split them, so it is clear that some flags are VESA defined and
> the others are "invented" for the display-timings framework and may be
> extended.

Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
complicate handling the flags =).

>>> Should the above say raising edge/falling edge instead of positive
>>> edge/negative edge?
>>>
> 
> Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> pretty natural to me :-) ). I don't know what others are thinking though.

I guess it's quite clear, but it's still different terms than used
elsewhere, e.g. documentation for videomodes.

Another thing I noticed while using the new videomode, display_timings.h
has a few names that are quite short and generic. Like "TE_MIN", which
is now a global define. And "timing_entry". Either name could be well
used internally in some .c file, and could easily clash.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
  2013-02-27 16:13               ` Tomi Valkeinen
  (?)
@ 2013-03-05  9:24                   ` Steffen Trumtrar
  -1 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-03-05  9:24 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

Hi!

On Wed, Feb 27, 2013 at 06:13:49PM +0200, Tomi Valkeinen wrote:
> On 2013-02-27 18:05, Steffen Trumtrar wrote:
> > Ah, sorry. Forgot to answer this.
> > 
> > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
> >> Ping.
> >>
> >> On 2013-02-18 16:09, Tomi Valkeinen wrote:
> >>> Hi Steffen,
> >>>
> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> >>>
> >>>> +/* VESA display monitor timing parameters */
> >>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> >>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> >>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> >>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> >>>> +
> >>>> +/* display specific flags */
> >>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> >>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> >>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> >>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> >>>
> >>> <snip>
> >>>
> >>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
> >>>> +	unsigned int data_flags; /* video data flags */
> >>>
> >>> Why did you go for this approach? To be able to represent
> >>> true/false/not-specified?
> >>>
> > 
> > We decided somewhere between v3 and v8 (I think), that those flags can be
> > high/low/ignored.
> 
> Okay. Why aren't they enums, though? That always makes more clear which
> defines are to be used with which fields.
> 

Hm...

> >>> Would it be simpler to just have "flags" field? What does it give us to
> >>> have those two separately?
> >>>
> > 
> > I decided to split them, so it is clear that some flags are VESA defined and
> > the others are "invented" for the display-timings framework and may be
> > extended.
> 
> Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
> complicate handling the flags =).
> 
> >>> Should the above say raising edge/falling edge instead of positive
> >>> edge/negative edge?
> >>>
> > 
> > Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> > pretty natural to me :-) ). I don't know what others are thinking though.
> 
> I guess it's quite clear, but it's still different terms than used
> elsewhere, e.g. documentation for videomodes.
> 
> Another thing I noticed while using the new videomode, display_timings.h
> has a few names that are quite short and generic. Like "TE_MIN", which
> is now a global define. And "timing_entry". Either name could be well
> used internally in some .c file, and could easily clash.
> 

Yes. You are correct.
Everyone using this is welcome to send patches now :-)

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-03-05  9:24                   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-03-05  9:24 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree-discuss, Dave Airlie, Rob Herring, linux-fbdev,
	dri-devel, Laurent Pinchart, Thierry Reding,
	Guennady Liakhovetski, linux-media, Stephen Warren,
	Florian Tobias Schandinat, Rob Clark, Leela Krishna Amudala,
	Mohammed, Afzal, kernel

Hi!

On Wed, Feb 27, 2013 at 06:13:49PM +0200, Tomi Valkeinen wrote:
> On 2013-02-27 18:05, Steffen Trumtrar wrote:
> > Ah, sorry. Forgot to answer this.
> > 
> > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
> >> Ping.
> >>
> >> On 2013-02-18 16:09, Tomi Valkeinen wrote:
> >>> Hi Steffen,
> >>>
> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> >>>
> >>>> +/* VESA display monitor timing parameters */
> >>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> >>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> >>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> >>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> >>>> +
> >>>> +/* display specific flags */
> >>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> >>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> >>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> >>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> >>>
> >>> <snip>
> >>>
> >>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
> >>>> +	unsigned int data_flags; /* video data flags */
> >>>
> >>> Why did you go for this approach? To be able to represent
> >>> true/false/not-specified?
> >>>
> > 
> > We decided somewhere between v3 and v8 (I think), that those flags can be
> > high/low/ignored.
> 
> Okay. Why aren't they enums, though? That always makes more clear which
> defines are to be used with which fields.
> 

Hm...

> >>> Would it be simpler to just have "flags" field? What does it give us to
> >>> have those two separately?
> >>>
> > 
> > I decided to split them, so it is clear that some flags are VESA defined and
> > the others are "invented" for the display-timings framework and may be
> > extended.
> 
> Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
> complicate handling the flags =).
> 
> >>> Should the above say raising edge/falling edge instead of positive
> >>> edge/negative edge?
> >>>
> > 
> > Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> > pretty natural to me :-) ). I don't know what others are thinking though.
> 
> I guess it's quite clear, but it's still different terms than used
> elsewhere, e.g. documentation for videomodes.
> 
> Another thing I noticed while using the new videomode, display_timings.h
> has a few names that are quite short and generic. Like "TE_MIN", which
> is now a global define. And "timing_entry". Either name could be well
> used internally in some .c file, and could easily clash.
> 

Yes. You are correct.
Everyone using this is welcome to send patches now :-)

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v17 2/7] video: add display_timing and videomode
@ 2013-03-05  9:24                   ` Steffen Trumtrar
  0 siblings, 0 replies; 38+ messages in thread
From: Steffen Trumtrar @ 2013-03-05  9:24 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mohammed, Afzal, Dave Airlie,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	Florian Tobias Schandinat,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA

Hi!

On Wed, Feb 27, 2013 at 06:13:49PM +0200, Tomi Valkeinen wrote:
> On 2013-02-27 18:05, Steffen Trumtrar wrote:
> > Ah, sorry. Forgot to answer this.
> > 
> > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote:
> >> Ping.
> >>
> >> On 2013-02-18 16:09, Tomi Valkeinen wrote:
> >>> Hi Steffen,
> >>>
> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote:
> >>>
> >>>> +/* VESA display monitor timing parameters */
> >>>> +#define VESA_DMT_HSYNC_LOW		BIT(0)
> >>>> +#define VESA_DMT_HSYNC_HIGH		BIT(1)
> >>>> +#define VESA_DMT_VSYNC_LOW		BIT(2)
> >>>> +#define VESA_DMT_VSYNC_HIGH		BIT(3)
> >>>> +
> >>>> +/* display specific flags */
> >>>> +#define DISPLAY_FLAGS_DE_LOW		BIT(0)	/* data enable flag */
> >>>> +#define DISPLAY_FLAGS_DE_HIGH		BIT(1)
> >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE	BIT(2)	/* drive data on pos. edge */
> >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE	BIT(3)	/* drive data on neg. edge */
> >>>> +#define DISPLAY_FLAGS_INTERLACED	BIT(4)
> >>>> +#define DISPLAY_FLAGS_DOUBLESCAN	BIT(5)
> >>>
> >>> <snip>
> >>>
> >>>> +	unsigned int dmt_flags;	/* VESA DMT flags */
> >>>> +	unsigned int data_flags; /* video data flags */
> >>>
> >>> Why did you go for this approach? To be able to represent
> >>> true/false/not-specified?
> >>>
> > 
> > We decided somewhere between v3 and v8 (I think), that those flags can be
> > high/low/ignored.
> 
> Okay. Why aren't they enums, though? That always makes more clear which
> defines are to be used with which fields.
> 

Hm...

> >>> Would it be simpler to just have "flags" field? What does it give us to
> >>> have those two separately?
> >>>
> > 
> > I decided to split them, so it is clear that some flags are VESA defined and
> > the others are "invented" for the display-timings framework and may be
> > extended.
> 
> Hmm... Okay. Is it relevant that they are VESA defined? It just feels to
> complicate handling the flags =).
> 
> >>> Should the above say raising edge/falling edge instead of positive
> >>> edge/negative edge?
> >>>
> > 
> > Hm, I used posedge/negedge because it is shorter (and because of my Verilog past
> > pretty natural to me :-) ). I don't know what others are thinking though.
> 
> I guess it's quite clear, but it's still different terms than used
> elsewhere, e.g. documentation for videomodes.
> 
> Another thing I noticed while using the new videomode, display_timings.h
> has a few names that are quite short and generic. Like "TE_MIN", which
> is now a global define. And "timing_entry". Either name could be well
> used internally in some .c file, and could easily clash.
> 

Yes. You are correct.
Everyone using this is welcome to send patches now :-)

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2013-03-05  9:25 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-25  9:01 [PATCH v17 0/7] of: add display helper Steffen Trumtrar
2013-01-25  9:01 ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 1/7] viafb: rename display_timing to via_display_timing Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 2/7] video: add display_timing and videomode Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-02-18 14:09   ` Tomi Valkeinen
2013-02-18 14:09     ` Tomi Valkeinen
     [not found]     ` <51223615.4090709-X3B1VOXEql0@public.gmane.org>
2013-02-27 15:45       ` Tomi Valkeinen
2013-02-27 15:45         ` Tomi Valkeinen
2013-02-27 15:45         ` Tomi Valkeinen
2013-02-27 16:05         ` Steffen Trumtrar
2013-02-27 16:05           ` Steffen Trumtrar
     [not found]           ` <20130227160540.GA10491-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-02-27 16:13             ` Tomi Valkeinen
2013-02-27 16:13               ` Tomi Valkeinen
2013-02-27 16:13               ` Tomi Valkeinen
     [not found]               ` <512E30BD.7010603-l0cyMroinI0@public.gmane.org>
2013-03-05  9:24                 ` Steffen Trumtrar
2013-03-05  9:24                   ` Steffen Trumtrar
2013-03-05  9:24                   ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 3/7] video: add of helper for display timings/videomode Steffen Trumtrar
2013-01-25  9:01   ` =?UTF-8?q?=5BPATCH=20v17=203/7=5D=20video=3A=20add=20of=20helper=20for=20display=20timings/videomode Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 4/7] fbmon: add videomode helpers Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-02-01  9:29   ` Jingoo Han
2013-02-01  9:29     ` Jingoo Han
2013-02-01  9:29     ` Jingoo Han
     [not found]     ` <003401ce005e$af665c50$0e3314f0$%han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-02-05 18:29       ` Steffen Trumtrar
2013-02-05 18:29         ` Steffen Trumtrar
2013-02-05 18:29         ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 5/7] fbmon: add of_videomode helpers Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 6/7] drm_modes: add videomode helpers Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-01-25  9:01 ` [PATCH v17 7/7] drm_modes: add of_videomode helpers Steffen Trumtrar
2013-01-25  9:01   ` Steffen Trumtrar
2013-02-01  9:33 [PATCH v17 4/7] fbmon: add videomode helpers Jingoo Han
2013-02-01  9:33 ` Jingoo Han
2013-02-01  9:33 ` Jingoo Han

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.