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