Re: [PATCH v7 1/8] video: add display_timing struct and helpers
flat view
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2012-10-31 17:04:15
Also in:
dri-devel, linux-devicetree, linux-media
Hi Steffen, Thanks for the patch. As we'll need a v8 anyway due to the comment on patch 5/8, here are a couple of other small comments. On Wednesday 31 October 2012 10:28:01 Steffen Trumtrar wrote:
quoted hunk ↗ jump to hunk
Add display_timing structure and the according helper functions. This allows the description of a display via its supported timing parameters. Every timing parameter can be specified as a single value or a range <min typ max>. Signed-off-by: Steffen Trumtrar <redacted> --- drivers/video/Kconfig | 5 +++ drivers/video/Makefile | 1 + drivers/video/display_timing.c | 24 ++++++++++++++ include/linux/display_timing.h | 69 +++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+) create mode 100644 drivers/video/display_timing.c create mode 100644 include/linux/display_timing.hdiff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index d08d799..1421fc8 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig@@ -33,6 +33,11 @@ config VIDEO_OUTPUT_CONTROL This framework adds support for low-level control of the video output switch. +config DISPLAY_TIMING + bool "Enable display timings helpers" + help + Say Y here, to use the display timing helpers. + menuconfig FB tristate "Support for frame buffer devices" ---help---diff --git a/drivers/video/Makefile b/drivers/video/Makefile index 23e948e..552c045 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile@@ -167,3 +167,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.odiff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c new file mode 100644 index 0000000..9ccfdb3 --- /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/slab.h> +#include <linux/display_timing.h>
I try to keep #include's sorted alphabetically, but I won't push for that.
quoted hunk ↗ jump to hunk
+void timings_release(struct display_timings *disp) +{ + int i; + + for (i = 0; i < disp->num_timings; i++)
disp->num_timings is an unsigned int, i should be an unsigned int as well to avoid signed vs. unsigned comparisons.
quoted hunk ↗ jump to hunk
+ kfree(disp->timings[i]); +} + +void display_timings_release(struct display_timings *disp) +{ + timings_release(disp); + kfree(disp->timings); +}diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h new file mode 100644 index 0000000..aa02a12 --- /dev/null +++ b/include/linux/display_timing.h@@ -0,0 +1,69 @@ +/* + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de> + * + * description of display timings + * + * This file is released under the GPLv2 + */ + +#ifndef __LINUX_DISPLAY_TIMINGS_H +#define __LINUX_DISPLAY_TIMINGS_H + +#include <linux/types.h> + +struct timing_entry { + u32 min; + u32 typ; + u32 max; +}; + +struct display_timing { + struct timing_entry pixelclock; + + struct timing_entry hactive; + struct timing_entry hfront_porch; + struct timing_entry hback_porch; + struct timing_entry hsync_len; + + struct timing_entry vactive; + struct timing_entry vfront_porch; + struct timing_entry vback_porch; + struct timing_entry vsync_len; + + unsigned int vsync_pol_active; + unsigned int hsync_pol_active; + unsigned int de_pol_active; + unsigned int pixelclk_pol; + bool interlaced; + bool doublescan; +}; + +struct display_timings { + unsigned int num_timings; + unsigned int native_mode; + + struct display_timing **timings; +}; + +/* placeholder function until ranges are really needed */ +static inline u32 display_timing_get_value(struct timing_entry *te, intindex)
What is the index parameter for ?
+{
+ return te->typ;
+}
+
+static inline struct display_timing *display_timings_get(struct
display_timings *disp,
+ int index)
+{
+ struct display_timing *dt;
+
+ if (disp->num_timings > index) {index should be an unsigned int for the same reason as above.
+ dt = disp->timings[index]; + return dt;
Maybe just return disp->timings[index]; ?
quoted hunk ↗ jump to hunk
+ } else + return NULL; +} + +void timings_release(struct display_timings *disp); +void display_timings_release(struct display_timings *disp); + +#endif
-- Regards, Laurent Pinchart