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>.
Also, 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 <redacted>
---
drivers/video/Kconfig | 6 ++++
drivers/video/Makefile | 2 ++
drivers/video/display_timing.c | 22 +++++++++++++
drivers/video/videomode.c | 45 ++++++++++++++++++++++++++
include/linux/display_timing.h | 69 ++++++++++++++++++++++++++++++++++++++++
include/linux/videomode.h | 39 +++++++++++++++++++++++
6 files changed, 183 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
@@ -0,0 +1,69 @@+/*+*Copyright2012SteffenTrumtrar<s.trumtrar@pengutronix.de>+*+*descriptionofdisplaytimings+*+*ThisfileisreleasedundertheGPLv2+*/++#ifndef __LINUX_DISPLAY_TIMINGS_H+#define __LINUX_DISPLAY_TIMINGS_H++#include<linux/types.h>++structtiming_entry{+u32min;+u32typ;+u32max;+};++structdisplay_timing{+structtiming_entrypixelclock;++structtiming_entryhactive;+structtiming_entryhfront_porch;+structtiming_entryhback_porch;+structtiming_entryhsync_len;++structtiming_entryvactive;+structtiming_entryvfront_porch;+structtiming_entryvback_porch;+structtiming_entryvsync_len;++unsignedintvsync_pol_active;+unsignedinthsync_pol_active;+unsignedintde_pol_active;+unsignedintpixelclk_pol;+boolinterlaced;+booldoublescan;+};++structdisplay_timings{+unsignedintnum_timings;+unsignedintnative_mode;++structdisplay_timing**timings;+};++/* placeholder function until ranges are really needed +*theindexparametershouldthenbeusedtoselectoneof[mintypmax]+*/+staticinlineu32display_timing_get_value(structtiming_entry*te,+unsignedintindex)+{+returnte->typ;+}++staticinlinestructdisplay_timing*display_timings_get(structdisplay_timings*disp,+unsignedintindex)+{+if(disp->num_timings>index)+returndisp->timings[index];+else+returnNULL;+}++voidtimings_release(structdisplay_timings*disp);+voiddisplay_timings_release(structdisplay_timings*disp);++#endif
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
Signed-off-by: Steffen Trumtrar <redacted>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
.../devicetree/bindings/video/display-timings.txt | 107 +++++++++++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile | 2 +
drivers/video/of_display_timing.c | 186 ++++++++++++++++++++
drivers/video/of_videomode.c | 47 +++++
include/linux/of_display_timings.h | 19 ++
include/linux/of_videomode.h | 15 ++
7 files changed, 389 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/video/of_display_timing.c
create mode 100644 drivers/video/of_videomode.c
create mode 100644 include/linux/of_display_timings.h
create mode 100644 include/linux/of_videomode.h
@@ -0,0 +1,186 @@+/*+*OFhelpersforparsingdisplaytimings+*+*Copyright(c)2012SteffenTrumtrar<s.trumtrar@pengutronix.de>,Pengutronix+*+*basedonof_videomode.cbySaschaHauer<s.hauer@pengutronix.de>+*+*ThisfileisreleasedundertheGPLv2+*/+#include<linux/of.h>+#include<linux/slab.h>+#include<linux/export.h>+#include<linux/of_display_timings.h>++/**+*parse_property-parsetiming_entryfromdevice_node+*@np:device_nodewiththeproperty+*@name:nameoftheproperty+*@result:willbesettothereturnvalue+*+*DESCRIPTION:+*Everydisplay_timingcanbespecifiedwitheitherjustthetypicalvalueor+*arangeconsistingofmin/typ/max.Thisfunctionhelpshandlingthis+**/+staticintparse_property(structdevice_node*np,char*name,+structtiming_entry*result)+{+structproperty*prop;+intlength,cells,ret;++prop=of_find_property(np,name,&length);+if(!prop){+pr_err("%s: could not find property %s\n",__func__,name);+return-EINVAL;+}++cells=length/sizeof(u32);+if(cells=1){+ret=of_property_read_u32(np,name,&result->typ);+result->min=result->typ;+result->max=result->typ;+}elseif(cells=3){+ret=of_property_read_u32_array(np,name,&result->min,cells);+}else{+pr_err("%s: illegal timing specification in %s\n",__func__,name);+return-EINVAL;+}++returnret;+}++/**+*of_get_display_timing-parsedisplay_timingentryfromdevice_node+*@np:device_nodewiththeproperties+**/+staticstructdisplay_timing*of_get_display_timing(structdevice_node*np)+{+structdisplay_timing*dt;+intret=0;++dt=kzalloc(sizeof(*dt),GFP_KERNEL);+if(!dt){+pr_err("%s: could not allocate display_timing struct\n",__func__);+returnNULL;+}++ret|=parse_property(np,"hback-porch",&dt->hback_porch);+ret|=parse_property(np,"hfront-porch",&dt->hfront_porch);+ret|=parse_property(np,"hactive",&dt->hactive);+ret|=parse_property(np,"hsync-len",&dt->hsync_len);+ret|=parse_property(np,"vback-porch",&dt->vback_porch);+ret|=parse_property(np,"vfront-porch",&dt->vfront_porch);+ret|=parse_property(np,"vactive",&dt->vactive);+ret|=parse_property(np,"vsync-len",&dt->vsync_len);+ret|=parse_property(np,"clock-frequency",&dt->pixelclock);++of_property_read_u32(np,"vsync-active",&dt->vsync_pol_active);+of_property_read_u32(np,"hsync-active",&dt->hsync_pol_active);+of_property_read_u32(np,"de-active",&dt->de_pol_active);+of_property_read_u32(np,"pixelclk-inverted",&dt->pixelclk_pol);+dt->interlaced=of_property_read_bool(np,"interlaced");+dt->doublescan=of_property_read_bool(np,"doublescan");++if(ret){+pr_err("%s: error reading timing properties\n",__func__);+returnNULL;+}++returndt;+}++/**+*of_get_display_timings-parsealldisplay_timingentriesfromadevice_node+*@np:device_nodewiththesubnodes+**/+structdisplay_timings*of_get_display_timings(structdevice_node*np)+{+structdevice_node*timings_np;+structdevice_node*entry;+structdevice_node*native_mode;+structdisplay_timings*disp;++if(!np){+pr_err("%s: no devicenode given\n",__func__);+returnNULL;+}++timings_np=of_find_node_by_name(np,"display-timings");+if(!timings_np){+pr_err("%s: could not find display-timings node\n",__func__);+returnNULL;+}++disp=kzalloc(sizeof(*disp),GFP_KERNEL);+if(!disp)+return-ENOMEM;++entry=of_parse_phandle(timings_np,"native-mode",0);+/* assume first child as native mode if none provided */+if(!entry)+entry=of_get_next_child(np,NULL);+if(!entry){+pr_err("%s: no timing specifications given\n",__func__);+returnNULL;+}++pr_info("%s: using %s as default timing\n",__func__,entry->name);++native_mode=entry;++disp->num_timings=of_get_child_count(timings_np);+disp->timings=kzalloc(sizeof(structdisplay_timing*)*disp->num_timings,+GFP_KERNEL);+if(!disp->timings)+return-ENOMEM;++disp->num_timings=0;+disp->native_mode=0;++for_each_child_of_node(timings_np,entry){+structdisplay_timing*dt;++dt=of_get_display_timing(entry);+if(!dt){+/* to not encourage wrong devicetrees, fail in case of an error */+pr_err("%s: error in timing %d\n",__func__,disp->num_timings+1);+returnNULL;+}++if(native_mode=entry)+disp->native_mode=disp->num_timings;++disp->timings[disp->num_timings]=dt;+disp->num_timings++;+}+of_node_put(timings_np);++if(disp->num_timings>0)+pr_info("%s: got %d timings. Using timing #%d as default\n",__func__,+disp->num_timings,disp->native_mode+1);+else{+pr_err("%s: no valid timings specified\n",__func__);+returnNULL;+}+returndisp;+}+EXPORT_SYMBOL_GPL(of_get_display_timings);++/**+*of_display_timings_exists-checkifadisplay-timingsnodeisprovided+*@np:device_nodewiththetiming+**/+intof_display_timings_exists(structdevice_node*np)+{+structdevice_node*timings_np;++if(!np)+return-EINVAL;++timings_np=of_parse_phandle(np,"display-timings",0);+if(!timings_np)+return-EINVAL;++return1;+}+EXPORT_SYMBOL_GPL(of_display_timings_exists);
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar <redacted>
---
drivers/video/fbmon.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/fb.h | 2 ++
2 files changed, 39 insertions(+)
@@ -504,6 +505,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,}EXPORT_SYMBOL(drm_gtf_mode);+#if IS_ENABLED(CONFIG_VIDEOMODE)+intvideomode_to_display_mode(structvideomode*vm,structdrm_display_mode*dmode)+{+dmode->hdisplay=vm->hactive;+dmode->hsync_start=dmode->hdisplay+vm->hfront_porch;+dmode->hsync_end=dmode->hsync_start+vm->hsync_len;+dmode->htotal=dmode->hsync_end+vm->hback_porch;++dmode->vdisplay=vm->vactive;+dmode->vsync_start=dmode->vdisplay+vm->vfront_porch;+dmode->vsync_end=dmode->vsync_start+vm->vsync_len;+dmode->vtotal=dmode->vsync_end+vm->vback_porch;++dmode->clock=vm->pixelclock/1000;++dmode->flags=0;+if(vm->hah)+dmode->flags|=DRM_MODE_FLAG_PHSYNC;+else+dmode->flags|=DRM_MODE_FLAG_NHSYNC;+if(vm->vah)+dmode->flags|=DRM_MODE_FLAG_PVSYNC;+else+dmode->flags|=DRM_MODE_FLAG_NVSYNC;+if(vm->interlaced)+dmode->flags|=DRM_MODE_FLAG_INTERLACE;+if(vm->doublescan)+dmode->flags|=DRM_MODE_FLAG_DBLSCAN;+drm_mode_set_name(dmode);++return0;+}+EXPORT_SYMBOL_GPL(videomode_to_display_mode);+#endif+/***drm_mode_set_name-setthenameonamode*@mode:namewillbesetinthismode
Hello Steffen,
On Mon, Nov 12, 2012 at 7:37 PM, Steffen Trumtrar
[off-list ref] wrote:
quoted hunk
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
Signed-off-by: Steffen Trumtrar <redacted>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
.../devicetree/bindings/video/display-timings.txt | 107 +++++++++++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile | 2 +
drivers/video/of_display_timing.c | 186 ++++++++++++++++++++
drivers/video/of_videomode.c | 47 +++++
include/linux/of_display_timings.h | 19 ++
include/linux/of_videomode.h | 15 ++
7 files changed, 389 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/video/of_display_timing.c
create mode 100644 drivers/video/of_videomode.c
create mode 100644 include/linux/of_display_timings.h
create mode 100644 include/linux/of_videomode.h
@@ -0,0 +1,186 @@+/*+*OFhelpersforparsingdisplaytimings+*+*Copyright(c)2012SteffenTrumtrar<s.trumtrar@pengutronix.de>,Pengutronix+*+*basedonof_videomode.cbySaschaHauer<s.hauer@pengutronix.de>+*+*ThisfileisreleasedundertheGPLv2+*/+#include<linux/of.h>+#include<linux/slab.h>+#include<linux/export.h>+#include<linux/of_display_timings.h>++/**+*parse_property-parsetiming_entryfromdevice_node+*@np:device_nodewiththeproperty+*@name:nameoftheproperty+*@result:willbesettothereturnvalue+*+*DESCRIPTION:+*Everydisplay_timingcanbespecifiedwitheitherjustthetypicalvalueor+*arangeconsistingofmin/typ/max.Thisfunctionhelpshandlingthis+**/+staticintparse_property(structdevice_node*np,char*name,+structtiming_entry*result)+{+structproperty*prop;+intlength,cells,ret;++prop=of_find_property(np,name,&length);+if(!prop){+pr_err("%s: could not find property %s\n",__func__,name);+return-EINVAL;+}++cells=length/sizeof(u32);+if(cells=1){+ret=of_property_read_u32(np,name,&result->typ);+result->min=result->typ;+result->max=result->typ;+}elseif(cells=3){+ret=of_property_read_u32_array(np,name,&result->min,cells);+}else{+pr_err("%s: illegal timing specification in %s\n",__func__,name);+return-EINVAL;+}++returnret;+}++/**+*of_get_display_timing-parsedisplay_timingentryfromdevice_node+*@np:device_nodewiththeproperties+**/+staticstructdisplay_timing*of_get_display_timing(structdevice_node*np)+{+structdisplay_timing*dt;+intret=0;++dt=kzalloc(sizeof(*dt),GFP_KERNEL);+if(!dt){+pr_err("%s: could not allocate display_timing struct\n",__func__);+returnNULL;+}++ret|=parse_property(np,"hback-porch",&dt->hback_porch);+ret|=parse_property(np,"hfront-porch",&dt->hfront_porch);+ret|=parse_property(np,"hactive",&dt->hactive);+ret|=parse_property(np,"hsync-len",&dt->hsync_len);+ret|=parse_property(np,"vback-porch",&dt->vback_porch);+ret|=parse_property(np,"vfront-porch",&dt->vfront_porch);+ret|=parse_property(np,"vactive",&dt->vactive);+ret|=parse_property(np,"vsync-len",&dt->vsync_len);+ret|=parse_property(np,"clock-frequency",&dt->pixelclock);++of_property_read_u32(np,"vsync-active",&dt->vsync_pol_active);+of_property_read_u32(np,"hsync-active",&dt->hsync_pol_active);+of_property_read_u32(np,"de-active",&dt->de_pol_active);+of_property_read_u32(np,"pixelclk-inverted",&dt->pixelclk_pol);+dt->interlaced=of_property_read_bool(np,"interlaced");+dt->doublescan=of_property_read_bool(np,"doublescan");++if(ret){+pr_err("%s: error reading timing properties\n",__func__);+returnNULL;+}++returndt;+}++/**+*of_get_display_timings-parsealldisplay_timingentriesfromadevice_node+*@np:device_nodewiththesubnodes+**/+structdisplay_timings*of_get_display_timings(structdevice_node*np)+{+structdevice_node*timings_np;+structdevice_node*entry;+structdevice_node*native_mode;+structdisplay_timings*disp;++if(!np){+pr_err("%s: no devicenode given\n",__func__);+returnNULL;+}++timings_np=of_find_node_by_name(np,"display-timings");+if(!timings_np){+pr_err("%s: could not find display-timings node\n",__func__);+returnNULL;+}++disp=kzalloc(sizeof(*disp),GFP_KERNEL);+if(!disp)+return-ENOMEM;++entry=of_parse_phandle(timings_np,"native-mode",0);+/* assume first child as native mode if none provided */+if(!entry)+entry=of_get_next_child(np,NULL);+if(!entry){+pr_err("%s: no timing specifications given\n",__func__);+returnNULL;+}++pr_info("%s: using %s as default timing\n",__func__,entry->name);++native_mode=entry;++disp->num_timings=of_get_child_count(timings_np);+disp->timings=kzalloc(sizeof(structdisplay_timing*)*disp->num_timings,+GFP_KERNEL);+if(!disp->timings)+return-ENOMEM;
Could you please check return values here ^^^ and above "disp kzalloc(sizeof(*disp), GFP_KERNEL);" ?
May be it's better to return NULL instead of -ENOMEM and put error message?
--
Best regards, Klimov Alexey
From: Stephen Warren <hidden> Date: 2012-11-12 20:40:12
On 11/12/2012 08:37 AM, Steffen Trumtrar wrote:
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
I find the indexing API a bit confusing. But that's perhaps just a
matter of personal preference.
Also the ordering of arguments seems a little off. I find it more
natural to have the destination pointer in the first argument, similar
to the memcpy() function, so this would be:
int videomode_from_timing(struct videomode *vm, struct display_timings *disp,
unsigned int index);
Actually, when reading videomode_from_timing() I'd expect the argument
list to be:
int videomode_from_timing(struct videomode *vm, struct display_timing *timing);
Am I the only one confused by this?
+/* placeholder function until ranges are really needed
The above line has trailing whitespace. Also the block comment should
have the opening /* on a separate line.
+ * the index parameter should then be used to select one of [min typ max]
If index is supposed to select min, typ or max, then maybe an enum would
be a better candidate? Or alternatively provide separate accessors, like
display_timing_get_{minimum,typical,maximum}().
On Mon, Nov 12, 2012 at 11:00:37PM +0400, Alexey Klimov wrote:
Hello Steffen,
On Mon, Nov 12, 2012 at 7:37 PM, Steffen Trumtrar
[off-list ref] wrote:
quoted
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
Signed-off-by: Steffen Trumtrar <redacted>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
.../devicetree/bindings/video/display-timings.txt | 107 +++++++++++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile | 2 +
drivers/video/of_display_timing.c | 186 ++++++++++++++++++++
drivers/video/of_videomode.c | 47 +++++
include/linux/of_display_timings.h | 19 ++
include/linux/of_videomode.h | 15 ++
7 files changed, 389 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/video/of_display_timing.c
create mode 100644 drivers/video/of_videomode.c
create mode 100644 include/linux/of_display_timings.h
create mode 100644 include/linux/of_videomode.h
@@ -0,0 +1,186 @@+/*+*OFhelpersforparsingdisplaytimings+*+*Copyright(c)2012SteffenTrumtrar<s.trumtrar@pengutronix.de>,Pengutronix+*+*basedonof_videomode.cbySaschaHauer<s.hauer@pengutronix.de>+*+*ThisfileisreleasedundertheGPLv2+*/+#include<linux/of.h>+#include<linux/slab.h>+#include<linux/export.h>+#include<linux/of_display_timings.h>++/**+*parse_property-parsetiming_entryfromdevice_node+*@np:device_nodewiththeproperty+*@name:nameoftheproperty+*@result:willbesettothereturnvalue+*+*DESCRIPTION:+*Everydisplay_timingcanbespecifiedwitheitherjustthetypicalvalueor+*arangeconsistingofmin/typ/max.Thisfunctionhelpshandlingthis+**/+staticintparse_property(structdevice_node*np,char*name,+structtiming_entry*result)+{+structproperty*prop;+intlength,cells,ret;++prop=of_find_property(np,name,&length);+if(!prop){+pr_err("%s: could not find property %s\n",__func__,name);+return-EINVAL;+}++cells=length/sizeof(u32);+if(cells=1){+ret=of_property_read_u32(np,name,&result->typ);+result->min=result->typ;+result->max=result->typ;+}elseif(cells=3){+ret=of_property_read_u32_array(np,name,&result->min,cells);+}else{+pr_err("%s: illegal timing specification in %s\n",__func__,name);+return-EINVAL;+}++returnret;+}++/**+*of_get_display_timing-parsedisplay_timingentryfromdevice_node+*@np:device_nodewiththeproperties+**/+staticstructdisplay_timing*of_get_display_timing(structdevice_node*np)+{+structdisplay_timing*dt;+intret=0;++dt=kzalloc(sizeof(*dt),GFP_KERNEL);+if(!dt){+pr_err("%s: could not allocate display_timing struct\n",__func__);+returnNULL;+}++ret|=parse_property(np,"hback-porch",&dt->hback_porch);+ret|=parse_property(np,"hfront-porch",&dt->hfront_porch);+ret|=parse_property(np,"hactive",&dt->hactive);+ret|=parse_property(np,"hsync-len",&dt->hsync_len);+ret|=parse_property(np,"vback-porch",&dt->vback_porch);+ret|=parse_property(np,"vfront-porch",&dt->vfront_porch);+ret|=parse_property(np,"vactive",&dt->vactive);+ret|=parse_property(np,"vsync-len",&dt->vsync_len);+ret|=parse_property(np,"clock-frequency",&dt->pixelclock);++of_property_read_u32(np,"vsync-active",&dt->vsync_pol_active);+of_property_read_u32(np,"hsync-active",&dt->hsync_pol_active);+of_property_read_u32(np,"de-active",&dt->de_pol_active);+of_property_read_u32(np,"pixelclk-inverted",&dt->pixelclk_pol);+dt->interlaced=of_property_read_bool(np,"interlaced");+dt->doublescan=of_property_read_bool(np,"doublescan");++if(ret){+pr_err("%s: error reading timing properties\n",__func__);+returnNULL;+}++returndt;+}++/**+*of_get_display_timings-parsealldisplay_timingentriesfromadevice_node+*@np:device_nodewiththesubnodes+**/+structdisplay_timings*of_get_display_timings(structdevice_node*np)+{+structdevice_node*timings_np;+structdevice_node*entry;+structdevice_node*native_mode;+structdisplay_timings*disp;++if(!np){+pr_err("%s: no devicenode given\n",__func__);+returnNULL;+}++timings_np=of_find_node_by_name(np,"display-timings");+if(!timings_np){+pr_err("%s: could not find display-timings node\n",__func__);+returnNULL;+}++disp=kzalloc(sizeof(*disp),GFP_KERNEL);+if(!disp)+return-ENOMEM;++entry=of_parse_phandle(timings_np,"native-mode",0);+/* assume first child as native mode if none provided */+if(!entry)+entry=of_get_next_child(np,NULL);+if(!entry){+pr_err("%s: no timing specifications given\n",__func__);+returnNULL;+}++pr_info("%s: using %s as default timing\n",__func__,entry->name);++native_mode=entry;++disp->num_timings=of_get_child_count(timings_np);+disp->timings=kzalloc(sizeof(structdisplay_timing*)*disp->num_timings,+GFP_KERNEL);+if(!disp->timings)+return-ENOMEM;
Could you please check return values here ^^^ and above "disp > kzalloc(sizeof(*disp), GFP_KERNEL);" ?
May be it's better to return NULL instead of -ENOMEM and put error message?
I will do that along with the memory leak fixes.
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-5555 |
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote:
quoted hunk
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
Signed-off-by: Steffen Trumtrar <redacted>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
.../devicetree/bindings/video/display-timings.txt | 107 +++++++++++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile | 2 +
drivers/video/of_display_timing.c | 186 ++++++++++++++++++++
drivers/video/of_videomode.c | 47 +++++
include/linux/of_display_timings.h | 19 ++
include/linux/of_videomode.h | 15 ++
7 files changed, 389 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
create mode 100644 drivers/video/of_display_timing.c
create mode 100644 drivers/video/of_videomode.c
create mode 100644 include/linux/of_display_timings.h
create mode 100644 include/linux/of_videomode.h
Maybe leave away the last -, so that it reads "display-timings node"? I
think that makes it more obvious that the node is supposed to be called
"display-timings".
+
+required properties:
+ - none
+
+optional properties:
+ - native-mode: the native mode for the display, in case multiple modes are
+ provided. When omitted, assume the first node is the native.
+
+timings-subnode
+---------------
+ in pixels
+ vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
+ lines
+ - clock-frequency: displayclock in Hz
"display clock"?
+
+optional properties:
+ - hsync-active : Hsync pulse is active low/high/ignored
+ - vsync-active : Vsync pulse is active low/high/ignored
+ - de-active : Data-Enable pulse is active low/high/ignored
+ - pixelclk-inverted : pixelclock is inverted/non-inverted/ignored
+ - interlaced (bool)
+ - doublescan (bool)
+
+All the optional properties that are not bool follow the following logic:
+ <1> : high active
+ <0> : low active
+ omitted : not used on hardware
Nitpick: You use space before : in the optional properties, but not in
the required properties above.
+
+There are different ways of describing the capabilities of a display. The devicetree
+representation corresponds to the one commonly found in datasheets for displays.
+If a display supports multiple signal timings, the native-mode can be specified.
+
+The parameters are defined as
+
+struct display_timing
+=====================
struct display_timing has no meaning in device tree documentation. Maybe
this line can just go away?
+/**
+ * of_get_display_timings - parse all display_timing entries from a device_node
+ * @np: device_node with the subnodes
+ **/
+struct display_timings *of_get_display_timings(struct device_node *np)
[...]
+ for_each_child_of_node(timings_np, entry) {
+ struct display_timing *dt;
+
+ dt = of_get_display_timing(entry);
+ if (!dt) {
+ /* to not encourage wrong devicetrees, fail in case of an error */
+ pr_err("%s: error in timing %d\n", __func__, disp->num_timings+1);
+ return NULL;
+ }
In case of a parsing error, of_get_display_timing() already shows an
error message, so I don't think we need another one here.
+/**
+ * of_display_timings_exists - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exists(struct device_node *np)
+{
+ struct device_node *timings_np;
+
+ if (!np)
+ return -EINVAL;
+
+ timings_np = of_parse_phandle(np, "display-timings", 0);
+ if (!timings_np)
+ return -EINVAL;
+
+ return 1;
I think this is missing of_node_put(timings_np) in both failure and
success cases. Also, maybe this should really return a bool instead?
Also, why do you use of_parse_phandle() for this? Aren't the
display-timings nodes expected to be children of some other node like an
output/display device?
+extern int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb, int index);
Similarily this should get a dummy for the !CONFIG_OF_VIDEOMODE case,
right? Either that or the prototype should be protected by
CONFIG_OF_VIDEOMODE as well.
Thierry
I seem to remember a comment to an earlier version of this patch
requesting better formatting of this string. Alternatively you might
want to consider replacing it using drm_mode_debug_printmodeline().
On Mon, Nov 12, 2012 at 01:40:12PM -0700, Stephen Warren wrote:
On 11/12/2012 08:37 AM, Steffen Trumtrar wrote:
quoted
This adds support for reading display timings from DT or/and convert one of those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.
I seem to remember a comment to an earlier version of this patch
requesting better formatting of this string. Alternatively you might
want to consider replacing it using drm_mode_debug_printmodeline().
Ah, yes. I only did that for fb_videomode and forgot about this one.
But the existing function is even better.
From: Stephen Warren <hidden> Date: 2012-11-13 17:46:53
On 11/13/2012 04:08 AM, Thierry Reding wrote:
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote:
quoted
This adds support for reading display timings from DT or/and
convert one of those timings to a videomode. The
of_display_timing implementation supports multiple children where
each property can have up to 3 values. All children are read into
an array, that can be queried. of_get_videomode converts exactly
one of that timings to a struct videomode.
I /think/ I had suggested naming this clock-frequency before so that
the property name would be more standardized; other bindings use that
same name. But I'm not too attached to the name I guess.
This either needs to include linux/of.h or a forward declaration of
struct device_node. Otherwise this will fail to compile if the file
where this is included from doesn't pull linux/of.h in explicitly.