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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxx>,
> 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@xxxxxxxxxxxxxx>
> + *
> + * 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
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
