diff mbox

[v7,4/8] video: add videomode helpers

Message ID 1351675689-26814-5-git-send-email-s.trumtrar@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Steffen Trumtrar Oct. 31, 2012, 9:28 a.m. UTC 
Add helper functions to convert from display timings to a generic videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/video/Kconfig     |    6 ++++++
 drivers/video/Makefile    |    1 +
 drivers/video/videomode.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/videomode.h |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+)
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/videomode.h

Comments

Laurent Pinchart Oct. 31, 2012, 5:18 p.m. UTC | #1 
Hi Steffen,

Thanks for the patch.

On Wednesday 31 October 2012 10:28:04 Steffen Trumtrar wrote:
> Add helper functions to convert from display timings to a generic videomode
> structure. This videomode can then be converted to the corresponding
> subsystem mode representation (e.g. fb_videomode).
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/video/Kconfig     |    6 ++++++
>  drivers/video/Makefile    |    1 +
>  drivers/video/videomode.c |   44 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/videomode.h |   36 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 87 insertions(+)
>  create mode 100644 drivers/video/videomode.c
>  create mode 100644 include/linux/videomode.h
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 1421fc8..45dd393 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -38,6 +38,12 @@ config DISPLAY_TIMING
>         help
>           Say Y here, to use the display timing helpers.
> 
> +config VIDEOMODE
> +       bool "Enable videomode helpers"

Shouldn't this option should be automatically selected through a select 
statement in other options that depend on it instead of manually selected ? 
Same for the DISPLAY_TIMING option in 1/8.

There's so little code here, do you think it would be a good idea to merge 
patches 1/8 and 4/8 and have a single Kconfig option ?

> +       help
> +         Say Y here, to use the generic videomode helpers. This allows
> +	 converting from display timings to fb_videomode and drm_display_mode
> +
>  menuconfig FB
>  	tristate "Support for frame buffer devices"
>  	---help---
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 552c045..fc30439 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -168,3 +168,4 @@ 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/videomode.c b/drivers/video/videomode.c
> new file mode 100644
> index 0000000..a9fe010
> --- /dev/null
> +++ b/drivers/video/videomode.c
> @@ -0,0 +1,44 @@
> +/*
> + * generic display timing functions
> + *
> + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>,
> Pengutronix + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/export.h>
> +#include <linux/errno.h>
> +#include <linux/display_timing.h>
> +#include <linux/videomode.h>

As in 1/8, I try to keep #include's sorted alphabetically, but I won't push 
for it here either :-)

> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> +			  int index)

unsigned int for index ?

> +{
> +	struct display_timing *dt = NULL;

No need to initialize dt to NULL.

> +	dt = display_timings_get(disp, index);
> +	if (!dt) {
> +		pr_err("%s: no signal timings found\n", __func__);

I wonder whether this really deserves a pr_err() here. Would this be a caller 
bug, or can there be valid use cases where this function would return an error 
?

> +		return -EINVAL;
> +	}
> +
> +	vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
> +	vm->hactive = display_timing_get_value(&dt->hactive, 0);
> +	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
> +	vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
> +	vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
> +
> +	vm->vactive = display_timing_get_value(&dt->vactive, 0);
> +	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
> +	vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
> +	vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
> +
> +	vm->vah = dt->vsync_pol_active;
> +	vm->hah = dt->hsync_pol_active;
> +	vm->interlaced = dt->interlaced;
> +	vm->doublescan = dt->doublescan;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(videomode_from_timing);
> diff --git a/include/linux/videomode.h b/include/linux/videomode.h
> new file mode 100644
> index 0000000..f932147
> --- /dev/null
> +++ b/include/linux/videomode.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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/display_timing.h>
> +
> +struct videomode {
> +	u32 pixelclock;
> +	u32 refreshrate;
> +
> +	u32 hactive;
> +	u32 hfront_porch;
> +	u32 hback_porch;
> +	u32 hsync_len;
> +
> +	u32 vactive;
> +	u32 vfront_porch;
> +	u32 vback_porch;
> +	u32 vsync_len;
> +
> +	u32 hah;
> +	u32 vah;
> +	bool interlaced;
> +	bool doublescan;
> +};
> +
> +int videomode_from_timing(struct display_timings *disp, struct videomode
> *vm,
> +			  int index);
> +#endif
diff mbox

Patch

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 1421fc8..45dd393 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -38,6 +38,12 @@  config DISPLAY_TIMING
        help
          Say Y here, to use the display timing helpers.
 
+config VIDEOMODE
+       bool "Enable videomode helpers"
+       help
+         Say Y here, to use the generic videomode helpers. This allows
+	 converting from display timings to fb_videomode and drm_display_mode
+
 menuconfig FB
 	tristate "Support for frame buffer devices"
 	---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 552c045..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -168,3 +168,4 @@  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/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 0000000..a9fe010
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,44 @@ 
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/errno.h>
+#include <linux/display_timing.h>
+#include <linux/videomode.h>
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  int index)
+{
+	struct display_timing *dt = NULL;
+
+	dt = display_timings_get(disp, index);
+	if (!dt) {
+		pr_err("%s: no signal timings found\n", __func__);
+		return -EINVAL;
+	}
+
+	vm->pixelclock = display_timing_get_value(&dt->pixelclock, 0);
+	vm->hactive = display_timing_get_value(&dt->hactive, 0);
+	vm->hfront_porch = display_timing_get_value(&dt->hfront_porch, 0);
+	vm->hback_porch = display_timing_get_value(&dt->hback_porch, 0);
+	vm->hsync_len = display_timing_get_value(&dt->hsync_len, 0);
+
+	vm->vactive = display_timing_get_value(&dt->vactive, 0);
+	vm->vfront_porch = display_timing_get_value(&dt->vfront_porch, 0);
+	vm->vback_porch = display_timing_get_value(&dt->vback_porch, 0);
+	vm->vsync_len = display_timing_get_value(&dt->vsync_len, 0);
+
+	vm->vah = dt->vsync_pol_active;
+	vm->hah = dt->hsync_pol_active;
+	vm->interlaced = dt->interlaced;
+	vm->doublescan = dt->doublescan;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/videomode.h b/include/linux/videomode.h
new file mode 100644
index 0000000..f932147
--- /dev/null
+++ b/include/linux/videomode.h
@@ -0,0 +1,36 @@ 
+/*
+ * 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/display_timing.h>
+
+struct videomode {
+	u32 pixelclock;
+	u32 refreshrate;
+
+	u32 hactive;
+	u32 hfront_porch;
+	u32 hback_porch;
+	u32 hsync_len;
+
+	u32 vactive;
+	u32 vfront_porch;
+	u32 vback_porch;
+	u32 vsync_len;
+
+	u32 hah;
+	u32 vah;
+	bool interlaced;
+	bool doublescan;
+};
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  int index);
+#endif