Re: [PATCHv15 2/7] video: add display_timing and videomode
On 2012-12-06 12:07, Grant Likely wrote:
On Mon, 26 Nov 2012 16:39:58 +0100, Steffen Trumtrar
s.trumt...@pengutronix.de wrote:
On Mon, Nov 26, 2012 at 02:37:26PM +0200, Tomi Valkeinen wrote:
On 2012-11-26 11:07, Steffen Trumtrar wrote:
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+ u32 pixelclock; /* pixelclock in Hz */
I don't know if this is of any importance, but the linux clock framework
manages clock rates with unsigned long. Would it be better to use the
same type here?
Hm, I don't know. Anyone? u32 should be large enough for a pixelclock.
4GHz is a pretty large pixel clock. I have no idea how conceivable it is
that hardware will get to that speed. However, if it will ever be
larger, then you'll need to account for that in the DT binding so that
the pixel clock can be specified using 2 cells.
I didn't mention the type because of the size of the field, but only
because to me it makes sense to use the same type for clock rates all
around the kernel. In many cases the value will be passed to clk_set_rate().
I can't see any real issues with u32, though.
Tomi
signature.asc
Description: OpenPGP digital signature
Re: [PATCHv15 2/7] video: add display_timing and videomode
On Mon, 26 Nov 2012 16:39:58 +0100, Steffen Trumtrar
s.trumt...@pengutronix.de wrote:
On Mon, Nov 26, 2012 at 02:37:26PM +0200, Tomi Valkeinen wrote:
On 2012-11-26 11:07, Steffen Trumtrar wrote:
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+ u32 pixelclock; /* pixelclock in Hz */
I don't know if this is of any importance, but the linux clock framework
manages clock rates with unsigned long. Would it be better to use the
same type here?
Hm, I don't know. Anyone? u32 should be large enough for a pixelclock.
4GHz is a pretty large pixel clock. I have no idea how conceivable it is
that hardware will get to that speed. However, if it will ever be
larger, then you'll need to account for that in the DT binding so that
the pixel clock can be specified using 2 cells.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv15 2/7] video: add display_timing and videomode
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.trumt...@pengutronix.de
Reviewed-by: Thierry Reding thierry.red...@avionic-design.de
Acked-by: Thierry Reding thierry.red...@avionic-design.de
Tested-by: Thierry Reding thierry.red...@avionic-design.de
Tested-by: Philipp Zabel p.za...@pengutronix.de
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/video/Kconfig |6 +++
drivers/video/Makefile |2 +
drivers/video/display_timing.c | 24 ++
drivers/video/videomode.c | 44 +
include/linux/display_timing.h | 104
include/linux/videomode.h | 54 +
6 files changed, 234 insertions(+)
create mode 100644 drivers/video/display_timing.c
create mode 100644 drivers/video/videomode.c
create mode 100644 include/linux/display_timing.h
create mode 100644 include/linux/videomode.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 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 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,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 000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/display_timing.h
+#include linux/export.h
+#include linux/slab.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 000..784eaad
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,44 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/display_timing.h
+#include linux/errno.h
+#include linux/export.h
+#include linux/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);
+
Re: [PATCHv15 2/7] video: add display_timing and videomode
On 2012-11-26 11:07, Steffen Trumtrar wrote:
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+ u32 pixelclock; /* pixelclock in Hz */
I don't know if this is of any importance, but the linux clock framework
manages clock rates with unsigned long. Would it be better to use the
same type here?
+ u32 hactive;
+ u32 hfront_porch;
+ u32 hback_porch;
+ u32 hsync_len;
+
+ u32 vactive;
+ u32 vfront_porch;
+ u32 vback_porch;
+ u32 vsync_len;
+
+ u32 hah;/* hsync active high */
+ u32 vah;/* vsync active high */
+ u32 de; /* data enable */
+ u32 pixelclk_pol;
What values will these 4 have? Aren't these booleans?
The data enable comment should be data enable active high.
The next patch says in the devtree binding documentation that the values
may be on/off/ignored. Is that represented here somehow, i.e. values are
0/1/2 or so? If not, what does it mean if the value is left out from
devtree data, meaning not used on hardware?
There's also a doubleclk value mentioned in the devtree bindings doc,
but not shown here.
I think this videomode struct is the one that display drivers are going
to use (?), in addition to the display_timing, so it would be good to
document it well.
Tomi
signature.asc
Description: OpenPGP digital signature
Re: [PATCHv15 2/7] video: add display_timing and videomode
On Mon, Nov 26, 2012 at 02:37:26PM +0200, Tomi Valkeinen wrote:
On 2012-11-26 11:07, Steffen Trumtrar wrote:
+/*
+ * Subsystem independent description of a videomode.
+ * Can be generated from struct display_timing.
+ */
+struct videomode {
+ u32 pixelclock; /* pixelclock in Hz */
I don't know if this is of any importance, but the linux clock framework
manages clock rates with unsigned long. Would it be better to use the
same type here?
Hm, I don't know. Anyone? u32 should be large enough for a pixelclock.
+ u32 hactive;
+ u32 hfront_porch;
+ u32 hback_porch;
+ u32 hsync_len;
+
+ u32 vactive;
+ u32 vfront_porch;
+ u32 vback_porch;
+ u32 vsync_len;
+
+ u32 hah;/* hsync active high */
+ u32 vah;/* vsync active high */
+ u32 de; /* data enable */
+ u32 pixelclk_pol;
What values will these 4 have? Aren't these booleans?
The data enable comment should be data enable active high.
The next patch says in the devtree binding documentation that the values
may be on/off/ignored. Is that represented here somehow, i.e. values are
0/1/2 or so? If not, what does it mean if the value is left out from
devtree data, meaning not used on hardware?
Hm, I think you might be right here. It is no problem in the DT part of the
code.
The properties are optional, left out means not used on hardware.
But this does not propagate correctly to the videomode. I should set the fields
to -EINVAL in case they are omitted, than everything should work nicely.
There's also a doubleclk value mentioned in the devtree bindings doc,
but not shown here.
Argh. That slipped through. I have patches that add this property to all
structs and the devicetree binding. But it is not supposed to be in this
series, because I want to at least have the binding stable for now.
I think this videomode struct is the one that display drivers are going
to use (?), in addition to the display_timing, so it would be good to
document it well.
Yes. Maybe I have to put more of the devicetree documentation in the code.
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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
