* [PATCH] backlight: pandora_bl: Change TWL4030_MODULE_PWM0 to TWL_MODULE_PWM
From: Peter Ujfalusi @ 2012-11-14 11:16 UTC (permalink / raw)
To: Richard Purdie, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Grazvydas Ignotas
TWL_MODULE_PWM is defined as:
#define TWL_MODULE_PWM TWL4030_MODULE_PWM0
Use the common module ID define here which will facilitate the upcoming
twl-core cleanup.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/video/backlight/pandora_bl.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/pandora_bl.c b/drivers/video/backlight/pandora_bl.c
index 4ec3074..633b0a2 100644
--- a/drivers/video/backlight/pandora_bl.c
+++ b/drivers/video/backlight/pandora_bl.c
@@ -71,8 +71,7 @@ static int pandora_backlight_update_status(struct backlight_device *bl)
* set PWM duty cycle to max. TPS61161 seems to use this
* to calibrate it's PWM sensitivity when it starts.
*/
- twl_i2c_write_u8(TWL4030_MODULE_PWM0, MAX_VALUE,
- TWL_PWM0_OFF);
+ twl_i2c_write_u8(TWL_MODULE_PWM, MAX_VALUE, TWL_PWM0_OFF);
/* first enable clock, then PWM0 out */
twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_GPBR1);
@@ -90,8 +89,7 @@ static int pandora_backlight_update_status(struct backlight_device *bl)
usleep_range(2000, 10000);
}
- twl_i2c_write_u8(TWL4030_MODULE_PWM0, MIN_VALUE + brightness,
- TWL_PWM0_OFF);
+ twl_i2c_write_u8(TWL_MODULE_PWM, MIN_VALUE + brightness, TWL_PWM0_OFF);
done:
if (brightness != 0)
@@ -132,7 +130,7 @@ static int pandora_backlight_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
/* 64 cycle period, ON position 0 */
- twl_i2c_write_u8(TWL4030_MODULE_PWM0, 0x80, TWL_PWM0_ON);
+ twl_i2c_write_u8(TWL_MODULE_PWM, 0x80, TWL_PWM0_ON);
bl->props.state |= PANDORABL_WAS_OFF;
bl->props.brightness = MAX_USER_VALUE;
--
1.8.0
^ permalink raw reply related
* Re: [PATCH v8 1/6] video: add display_timing and videomode
From: Steffen Trumtrar @ 2012-11-14 11:10 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121114110215.GA31999-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On Wed, Nov 14, 2012 at 12:02:15PM +0100, Thierry Reding wrote:
> On Wed, Nov 14, 2012 at 11:59:25AM +0100, Steffen Trumtrar wrote:
> > On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
> > > On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
> > > [...]
> > > > diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
> > > [...]
> > > > +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);
> > > > +}
> > >
> > > I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
> > > be used from modules.
> > >
> > > Thierry
> >
> > Yes. Just in time. I was just starting to type the send-email command ;-)
>
> Great! In that case don't forget to also look at my other email before
> sending. =)
>
Sure.
--
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 |
^ permalink raw reply
* Re: [PATCH v8 1/6] video: add display_timing and videomode
From: Thierry Reding @ 2012-11-14 11:02 UTC (permalink / raw)
To: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <20121114105925.GA18579@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Wed, Nov 14, 2012 at 11:59:25AM +0100, Steffen Trumtrar wrote:
> On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
> > On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
> > [...]
> > > diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
> > [...]
> > > +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);
> > > +}
> >
> > I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
> > be used from modules.
> >
> > Thierry
>
> Yes. Just in time. I was just starting to type the send-email command ;-)
Great! In that case don't forget to also look at my other email before
sending. =)
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-14 10:59 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-3-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote:
[...]
> diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
[...]
> +#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
> +#define __LINUX_OF_DISPLAY_TIMINGS_H
> +
> +#include <linux/display_timing.h>
> +
> +#define OF_USE_NATIVE_MODE -1
> +
> +struct display_timings *of_get_display_timings(struct device_node *np);
> +int of_display_timings_exists(struct device_node *np);
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.
> diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
[...]
> +#ifndef __LINUX_OF_VIDEOMODE_H
> +#define __LINUX_OF_VIDEOMODE_H
> +
> +#include <linux/videomode.h>
> +
> +int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
Same here.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 1/6] video: add display_timing and videomode
From: Steffen Trumtrar @ 2012-11-14 10:59 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121114105634.GA31801-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
> On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
> [...]
> > diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
> [...]
> > +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);
> > +}
>
> I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
> be used from modules.
>
> Thierry
Yes. Just in time. I was just starting to type the send-email command ;-)
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 |
^ permalink raw reply
* Re: [PATCH v8 1/6] video: add display_timing and videomode
From: Thierry Reding @ 2012-11-14 10:56 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-2-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 508 bytes --]
On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
[...]
> diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
[...]
> +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);
> +}
I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
be used from modules.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 2/2] OMAPDSS: Add a dispc_features struct for OMAP5
From: Archit Taneja @ 2012-11-14 8:32 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352881216-7490-1-git-send-email-archit@ti.com>
Add a dispc_features struct for OMAP5. Previously, OMAP5 used the same struct
as OMAP4. The new struct for OMAP5 contains the updated register field offset
and maximum limit for overlay manager width and height.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 29e6aa1..c681460 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -4131,6 +4131,23 @@ static const struct dispc_features omap44xx_dispc_feats __initconst = {
.gfx_fifo_workaround = true,
};
+static const struct dispc_features omap54xx_dispc_feats __initconst = {
+ .sw_start = 7,
+ .fp_start = 19,
+ .bp_start = 31,
+ .sw_max = 256,
+ .vp_max = 4095,
+ .hp_max = 4096,
+ .mgr_width_start = 11,
+ .mgr_height_start = 27,
+ .mgr_width_max = 4096,
+ .mgr_height_max = 4096,
+ .calc_scaling = dispc_ovl_calc_scaling_44xx,
+ .calc_core_clk = calc_core_clk_44xx,
+ .num_fifos = 5,
+ .gfx_fifo_workaround = true,
+};
+
static int __init dispc_init_features(struct platform_device *pdev)
{
const struct dispc_features *src;
@@ -4164,7 +4181,7 @@ static int __init dispc_init_features(struct platform_device *pdev)
break;
case OMAPDSS_VER_OMAP5:
- src = &omap44xx_dispc_feats;
+ src = &omap54xx_dispc_feats;
break;
default:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] OMAPDSS: Add overlay manager width and height limits as a dispc feature
From: Archit Taneja @ 2012-11-14 8:32 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1352881216-7490-1-git-send-email-archit@ti.com>
The overlay manager width and height vary in OMAP5 vary from previous OMAPs in
terms of maximum limit and register field positions. Add parameters in
dispc_features for these. Remove params related to manager width and height from
dss_features. We do this because we want to maintain a feature list for
individual IPs.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 28 +++++++++++++++++++++++++---
drivers/video/omap2/dss/dss_features.c | 8 --------
drivers/video/omap2/dss/dss_features.h | 2 --
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index bab9d6b..29e6aa1 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -87,6 +87,10 @@ struct dispc_features {
u16 sw_max;
u16 vp_max;
u16 hp_max;
+ u8 mgr_width_start;
+ u8 mgr_height_start;
+ u16 mgr_width_max;
+ u16 mgr_height_max;
int (*calc_scaling) (enum omap_plane plane,
const struct omap_video_timings *mgr_timings,
u16 width, u16 height, u16 out_width, u16 out_height,
@@ -1128,7 +1132,9 @@ static void dispc_mgr_set_size(enum omap_channel channel, u16 width,
{
u32 val;
- val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
+ val = FLD_VAL(height - 1, dispc.feat->mgr_height_start, 16) |
+ FLD_VAL(width - 1, dispc.feat->mgr_width_start, 0);
+
dispc_write_reg(DISPC_SIZE_MGR(channel), val);
}
@@ -2985,8 +2991,8 @@ void dispc_mgr_set_lcd_config(enum omap_channel channel,
static bool _dispc_mgr_size_ok(u16 width, u16 height)
{
- return width <= dss_feat_get_param_max(FEAT_PARAM_MGR_WIDTH) &&
- height <= dss_feat_get_param_max(FEAT_PARAM_MGR_HEIGHT);
+ return width <= dispc.feat->mgr_width_max &&
+ height <= dispc.feat->mgr_height_max;
}
static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
@@ -4067,6 +4073,10 @@ static const struct dispc_features omap24xx_dispc_feats __initconst = {
.sw_max = 64,
.vp_max = 255,
.hp_max = 256,
+ .mgr_width_start = 10,
+ .mgr_height_start = 26,
+ .mgr_width_max = 2048,
+ .mgr_height_max = 2048,
.calc_scaling = dispc_ovl_calc_scaling_24xx,
.calc_core_clk = calc_core_clk_24xx,
.num_fifos = 3,
@@ -4079,6 +4089,10 @@ static const struct dispc_features omap34xx_rev1_0_dispc_feats __initconst = {
.sw_max = 64,
.vp_max = 255,
.hp_max = 256,
+ .mgr_width_start = 10,
+ .mgr_height_start = 26,
+ .mgr_width_max = 2048,
+ .mgr_height_max = 2048,
.calc_scaling = dispc_ovl_calc_scaling_34xx,
.calc_core_clk = calc_core_clk_34xx,
.num_fifos = 3,
@@ -4091,6 +4105,10 @@ static const struct dispc_features omap34xx_rev3_0_dispc_feats __initconst = {
.sw_max = 256,
.vp_max = 4095,
.hp_max = 4096,
+ .mgr_width_start = 10,
+ .mgr_height_start = 26,
+ .mgr_width_max = 2048,
+ .mgr_height_max = 2048,
.calc_scaling = dispc_ovl_calc_scaling_34xx,
.calc_core_clk = calc_core_clk_34xx,
.num_fifos = 3,
@@ -4103,6 +4121,10 @@ static const struct dispc_features omap44xx_dispc_feats __initconst = {
.sw_max = 256,
.vp_max = 4095,
.hp_max = 4096,
+ .mgr_width_start = 10,
+ .mgr_height_start = 26,
+ .mgr_width_max = 2048,
+ .mgr_height_max = 2048,
.calc_scaling = dispc_ovl_calc_scaling_44xx,
.calc_core_clk = calc_core_clk_44xx,
.num_fifos = 5,
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 8dcecbc..1d125c6 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -429,8 +429,6 @@ static const struct dss_param_range omap2_dss_param_range[] = {
* scaler cannot scale a image with width more than 768.
*/
[FEAT_PARAM_LINEWIDTH] = { 1, 768 },
- [FEAT_PARAM_MGR_WIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_HEIGHT] = { 1, 2048 },
};
static const struct dss_param_range omap3_dss_param_range[] = {
@@ -445,8 +443,6 @@ static const struct dss_param_range omap3_dss_param_range[] = {
[FEAT_PARAM_DSI_FCK] = { 0, 173000000 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
[FEAT_PARAM_LINEWIDTH] = { 1, 1024 },
- [FEAT_PARAM_MGR_WIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_HEIGHT] = { 1, 2048 },
};
static const struct dss_param_range omap4_dss_param_range[] = {
@@ -461,8 +457,6 @@ static const struct dss_param_range omap4_dss_param_range[] = {
[FEAT_PARAM_DSI_FCK] = { 0, 170000000 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
[FEAT_PARAM_LINEWIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_WIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_HEIGHT] = { 1, 2048 },
};
static const struct dss_param_range omap5_dss_param_range[] = {
@@ -477,8 +471,6 @@ static const struct dss_param_range omap5_dss_param_range[] = {
[FEAT_PARAM_DSI_FCK] = { 0, 170000000 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
[FEAT_PARAM_LINEWIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_WIDTH] = { 1, 2048 },
- [FEAT_PARAM_MGR_HEIGHT] = { 1, 2048 },
};
static const enum dss_feat_id omap2_dss_feat_list[] = {
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index fc492ef..385b0af 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -98,8 +98,6 @@ enum dss_range_param {
FEAT_PARAM_DSI_FCK,
FEAT_PARAM_DOWNSCALE,
FEAT_PARAM_LINEWIDTH,
- FEAT_PARAM_MGR_WIDTH,
- FEAT_PARAM_MGR_HEIGHT,
};
/* DSS Feature Functions */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/2] OMAPDSS: DISPC: Update manager size limits for OMAP5
From: Archit Taneja @ 2012-11-14 8:32 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
Manager width and height can go up to 4K on OMAP5. Make manager width and height
register field offsets and maximum limits as dispc_features. Create a new
dispc_feature struct for OMAP5 which highlights this difference.
Archit Taneja (2):
OMAPDSS: Add overlay manager width and height limits as a dispc
feature
OMAPDSS: Add a dispc_features struct for OMAP5
drivers/video/omap2/dss/dispc.c | 47 +++++++++++++++++++++++++++++---
drivers/video/omap2/dss/dss_features.c | 8 ------
drivers/video/omap2/dss/dss_features.h | 2 --
3 files changed, 43 insertions(+), 14 deletions(-)
--
1.7.9.5
^ permalink raw reply
* RE: [PATCH 6/7] mx3fb: Fix the usage of wait_for_completion_timeout
From: Liu, Chuansheng @ 2012-11-14 4:57 UTC (permalink / raw)
To: linux-fbdev
UGluZy4NClRoYW5rcy4NCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBM
aXUsIENodWFuc2hlbmcNCj4gU2VudDogV2VkbmVzZGF5LCBOb3ZlbWJlciAwNywgMjAxMiAxOjMz
IEFNDQo+IFRvOiBGbG9yaWFuU2NoYW5kaW5hdEBnbXguZGUNCj4gQ2M6IGxpbnV4LWtlcm5lbEB2
Z2VyLmtlcm5lbC5vcmc7IExpdSwgQ2h1YW5zaGVuZw0KPiBTdWJqZWN0OiBbUEFUQ0ggNi83XSBt
eDNmYjogRml4IHRoZSB1c2FnZSBvZiB3YWl0X2Zvcl9jb21wbGV0aW9uX3RpbWVvdXQNCj4gDQo+
IA0KPiBUaGUgcmV0dXJuIHZhbHVlIG9mIHdhaXRfZm9yX2NvbXBsZXRpb25fdGltZW91dCgpIGlz
IGFsd2F5cw0KPiA+PSAwIHdpdGggdW5zaWduZWQgaW50IHR5cGUuDQo+IA0KPiBTbyB0aGUgY29u
ZGl0aW9uICJyZXQgPCAwIiBvciAicmV0ID49IDAiIGlzIHBvaW50bGVzcy4NCj4gDQo+IFNpZ25l
ZC1vZmYtYnk6IGxpdSBjaHVhbnNoZW5nIDxjaHVhbnNoZW5nLmxpdUBpbnRlbC5jb20+DQo+IC0t
LQ0KPiAgZHJpdmVycy92aWRlby9teDNmYi5jIHwgICAgNyArKystLS0tDQo+ICAxIGZpbGVzIGNo
YW5nZWQsIDMgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQg
YS9kcml2ZXJzL3ZpZGVvL214M2ZiLmMgYi9kcml2ZXJzL3ZpZGVvL214M2ZiLmMNCj4gaW5kZXgg
Y2UxZDQ1Mi4uODhiZjM2OCAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy92aWRlby9teDNmYi5jDQo+
ICsrKyBiL2RyaXZlcnMvdmlkZW8vbXgzZmIuYw0KPiBAQCAtMTExMCwxMiArMTExMCwxMSBAQCBz
dGF0aWMgaW50IG14M2ZiX3Bhbl9kaXNwbGF5KHN0cnVjdA0KPiBmYl92YXJfc2NyZWVuaW5mbyAq
dmFyLA0KPiAgCWVuYWJsZV9pcnEobXgzX2ZiaS0+aWRtYWNfY2hhbm5lbC0+ZW9mX2lycSk7DQo+
IA0KPiAgCXJldCA9IHdhaXRfZm9yX2NvbXBsZXRpb25fdGltZW91dCgmbXgzX2ZiaS0+ZmxpcF9j
bXBsLCBIWiAvIDEwKTsNCj4gLQlpZiAocmV0IDw9IDApIHsNCj4gKwlpZiAocmV0ID09IDApIHsN
Cj4gIAkJbXV0ZXhfdW5sb2NrKCZteDNfZmJpLT5tdXRleCk7DQo+IC0JCWRldl9pbmZvKGZiaS0+
ZGV2aWNlLCAiUGFubmluZyBmYWlsZWQgZHVlIHRvICVzXG4iLCByZXQgPCAwID8NCj4gLQkJCSAi
dXNlciBpbnRlcnJ1cHQiIDogInRpbWVvdXQiKTsNCj4gKwkJZGV2X2luZm8oZmJpLT5kZXZpY2Us
ICJQYW5uaW5nIGZhaWxlZCBkdWUgdG8gdGltZW91dFxuIik7DQo+ICAJCWRpc2FibGVfaXJxKG14
M19mYmktPmlkbWFjX2NoYW5uZWwtPmVvZl9pcnEpOw0KPiAtCQlyZXR1cm4gcmV0ID8gOiAtRVRJ
TUVET1VUOw0KPiArCQlyZXR1cm4gLUVUSU1FRE9VVDsNCj4gIAl9DQo+IA0KPiAgCW14M19mYmkt
PmN1cl9pcHVfYnVmID0gIW14M19mYmktPmN1cl9pcHVfYnVmOw0KPiAtLQ0KPiAxLjcuMC40DQo+
IA0KPiANCg0K
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-13 19:17 UTC (permalink / raw)
To: Mitch Bradley
Cc: Stephen Warren, linux-fbdev, kernel, devicetree-discuss,
dri-devel, Tomi Valkeinen, Laurent Pinchart, Philipp Zabel,
Steffen Trumtrar, Guennady Liakhovetski, linux-media
In-Reply-To: <50A28DCB.7050707@firmworks.com>
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
On Tue, Nov 13, 2012 at 08:13:31AM -1000, Mitch Bradley wrote:
> On 11/13/2012 7:51 AM, Thierry Reding wrote:
> > On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote:
> >> On 11/13/2012 04:08 AM, Thierry Reding wrote:
> >>> On Mon, Nov 12, 2012 at 04:37:02PM +0100, 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.
> >>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/video/display-timings.txt
> >>>> b/Documentation/devicetree/bindings/video/display-timings.txt
> >>
> >>>> + - clock-frequency: displayclock in Hz
> >>>
> >>> "display clock"?
> >>
> >> 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.
> >
> > That's not what I meant. I think "displayclock" should be two words in
> > the description of the property. The property name is fine.
>
> Given that modern display engines often have numerous clocks, perhaps it
> would be better to use a more specific name, like for example "pixel-clock".
This binding is only about defining display modes. Are any of the clocks
that you're referring to relevant to the actual display modes?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Mitch Bradley @ 2012-11-13 18:13 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121113175147.GA2597-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On 11/13/2012 7:51 AM, Thierry Reding wrote:
> On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote:
>> On 11/13/2012 04:08 AM, Thierry Reding wrote:
>>> On Mon, Nov 12, 2012 at 04:37:02PM +0100, 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.
>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/video/display-timings.txt
>>>> b/Documentation/devicetree/bindings/video/display-timings.txt
>>
>>>> + - clock-frequency: displayclock in Hz
>>>
>>> "display clock"?
>>
>> 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.
>
> That's not what I meant. I think "displayclock" should be two words in
> the description of the property. The property name is fine.
Given that modern display engines often have numerous clocks, perhaps it
would be better to use a more specific name, like for example "pixel-clock".
>
> Thierry
>
>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-13 17:51 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, Philipp Zabel, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <50A2878D.8020707-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]
On Tue, Nov 13, 2012 at 10:46:53AM -0700, Stephen Warren wrote:
> On 11/13/2012 04:08 AM, Thierry Reding wrote:
> > On Mon, Nov 12, 2012 at 04:37:02PM +0100, 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.
>
> >> diff --git
> >> a/Documentation/devicetree/bindings/video/display-timings.txt
> >> b/Documentation/devicetree/bindings/video/display-timings.txt
>
> >> + - clock-frequency: displayclock in Hz
> >
> > "display clock"?
>
> 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.
That's not what I meant. I think "displayclock" should be two words in
the description of the property. The property name is fine.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Stephen Warren @ 2012-11-13 17:46 UTC (permalink / raw)
To: Thierry Reding
Cc: Steffen Trumtrar, devicetree-discuss, Philipp Zabel, Rob Herring,
linux-fbdev, dri-devel, Laurent Pinchart, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, kernel
In-Reply-To: <20121113110837.GA30049@avionic-0098.mockup.avionic-design.de>
On 11/13/2012 04:08 AM, Thierry Reding wrote:
> On Mon, Nov 12, 2012 at 04:37:02PM +0100, 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.
>> diff --git
>> a/Documentation/devicetree/bindings/video/display-timings.txt
>> b/Documentation/devicetree/bindings/video/display-timings.txt
>> + - clock-frequency: displayclock in Hz
>
> "display clock"?
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.
^ permalink raw reply
* Re: tty, vt: lockdep warnings (Patch v3)
From: Sasha Levin @ 2012-11-13 16:24 UTC (permalink / raw)
To: Sasha Levin
Cc: Hugh Dickins, Alan Cox, Bjørn Mork, Dave Jones,
Daniel Vetter, Greg Kroah-Hartman, Jiri Slaby, linux-kernel,
linux-fbdev, florianSchandinat
In-Reply-To: <509D5C97.2020801@oracle.com>
On Fri, Nov 9, 2012 at 2:42 PM, Sasha Levin <sasha.levin@oracle.com> wrote:
> On 11/09/2012 02:34 PM, Hugh Dickins wrote:
>> On Thu, 8 Nov 2012, Alan Cox wrote:
>>> commit f35b3fbf24c4e4debb6a7a864b09854ccc2a22e7
>>> Author: Alan Cox <alan@linux.intel.com>
>>> Date: Wed Nov 7 16:35:08 2012 +0000
>>>
>>> fb: Rework locking to fix lock ordering on takeover
>>>
>>> Adjust the console layer to allow a take over call where the caller already
>>> holds the locks. Make the fb layer lock in order.
>>>
>>> This s partly a band aid, the fb layer is terminally confused about the
>>> locking rules it uses for its notifiers it seems.
>>>
>>> Signed-off-by: Alan Cox <alan@linux.intel.com>
>>
>> This version works for me too - thanks.
>> Hugh
>
> I was planning to test it last night, but new code in mm/ failed horribly and
> was BUG()ing all over the place, so I didn't get any significant testing
> of this patch done.
>
> Will update...
Nothing complains anywhere, looks great.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v8 6/6] drm_modes: add of_videomode helpers
From: Steffen Trumtrar @ 2012-11-13 13:30 UTC (permalink / raw)
To: Thierry Reding
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <20121113113518.GE30049@avionic-0098.mockup.avionic-design.de>
On Tue, Nov 13, 2012 at 12:35:18PM +0100, Thierry Reding wrote:
> On Mon, Nov 12, 2012 at 04:37:06PM +0100, Steffen Trumtrar wrote:
> [...]
> > +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> > +static void dump_drm_displaymode(struct drm_display_mode *m)
> > +{
> > + pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n",
> > + m->hdisplay, m->hsync_start, m->hsync_end, m->htotal,
> > + m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal,
> > + m->clock);
>
> 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.
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> [...]
> > @@ -1457,6 +1458,10 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
> >
> > extern int videomode_to_display_mode(struct videomode *vm,
> > struct drm_display_mode *dmode);
> > +extern int of_get_drm_display_mode(struct device_node *np,
> > + struct drm_display_mode *dmode,
> > + int index);
>
> Also requires either a dummy or protection.
>
> Thierry
--
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 |
^ permalink raw reply
* Re: [PATCH v8 3/6] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-11-13 13:28 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev, devicetree-discuss, dri-devel, Tomi Valkeinen,
Laurent Pinchart, kernel, Guennady Liakhovetski, linux-media
In-Reply-To: <20121113112257.GB30049@avionic-0098.mockup.avionic-design.de>
On Tue, Nov 13, 2012 at 12:22:58PM +0100, Thierry Reding wrote:
> On Mon, Nov 12, 2012 at 04:37:03PM +0100, Steffen Trumtrar wrote:
> [...]
> > +#if IS_ENABLED(CONFIG_VIDEOMODE)
> > +int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode)
>
> The other helpers are named <destination-type>_from_<source-type>(),
> maybe this should follow that example for consistency?
>
Hm, not a bad idea.
> > +{
> > + fbmode->xres = vm->hactive;
> > + fbmode->left_margin = vm->hback_porch;
> > + fbmode->right_margin = vm->hfront_porch;
> > + fbmode->hsync_len = vm->hsync_len;
> > +
> > + fbmode->yres = vm->vactive;
> > + fbmode->upper_margin = vm->vback_porch;
> > + fbmode->lower_margin = vm->vfront_porch;
> > + fbmode->vsync_len = vm->vsync_len;
> > +
> > + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
> > +
> > + fbmode->sync = 0;
> > + fbmode->vmode = 0;
> > + if (vm->hah)
> > + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> > + if (vm->vah)
> > + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> > + if (vm->interlaced)
> > + fbmode->vmode |= FB_VMODE_INTERLACED;
> > + if (vm->doublescan)
> > + fbmode->vmode |= FB_VMODE_DOUBLE;
> > + if (vm->de)
> > + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
> > + fbmode->refresh = 60;
>
> Can the refresh rate not be computed from the pixel clock and the
> horizontal and vertical timings?
>
Yes. That totally got lost on the way.
> > + fbmode->flag = 0;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
> > +#endif
> > +
> > +
>
> There's a gratuitous blank line here.
>
> > #else
> > int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> > {
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index c7a9571..46c665b 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -714,6 +714,8 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
> > extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
> > extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> >
> > +extern int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode);
> > +
>
> Should you provide a dummy in the !CONFIG_VIDEOMODE case?
>
Okay
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 |
^ permalink raw reply
* Re: [PATCH v8 1/6] video: add display_timing and videomode
From: Steffen Trumtrar @ 2012-11-13 13:14 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20121113104159.GA18645-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
On Tue, Nov 13, 2012 at 11:41:59AM +0100, Thierry Reding wrote:
> On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
> [...]
> > 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
>
> DISPLAY_TIMINGS?
>
> > #video output switch sysfs driver
> > obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
> > +obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
>
> display_timings.o?
>
> > +obj-$(CONFIG_VIDEOMODE) += videomode.o
> > diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
>
> display_timings.c?
>
I originally had that and changed it by request to the singular form.
(Can't find the mail atm). And I think this fits better with all the other drivers.
> > +int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
> > + unsigned int index)
>
> 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?
>
I went with the of_xxx-functions that have fname(from_node, to_property)
and personally prefer it this way. Therefore I'd like to keep it as is.
> > diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
>
> display_timings.h?
>
> > +/* 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.
>
Okay.
> > + * 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}().
>
Hm, I'm not so sure about this one. I'd prefer the enum.
> > + */
> > +static inline u32 display_timing_get_value(struct timing_entry *te,
> > + unsigned int index)
> > +{
> > + return te->typ;
> > +}
> > +
> > +static inline struct display_timing *display_timings_get(struct display_timings *disp,
> > + unsigned int index)
> > +{
> > + if (disp->num_timings > index)
> > + return disp->timings[index];
> > + else
> > + return NULL;
> > +}
> > +
> > +void timings_release(struct display_timings *disp);
>
> This function no longer exists.
>
Right.
Steffen
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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 |
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-13 12:59 UTC (permalink / raw)
To: Stephen Warren
Cc: devicetree-discuss, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media, Tomi Valkeinen, kernel
In-Reply-To: <50A15EAC.9030608@wwwdotorg.org>
On Mon, Nov 12, 2012 at 01:40:12PM -0700, Stephen Warren wrote:
> 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.
>
> > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
>
> The device tree bindings look reasonable to me, so,
>
> Acked-by: Stephen Warren <swarren@nvidia.com>
>
>
Thanks,
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 |
^ permalink raw reply
* Re: [PATCH v8 6/6] drm_modes: add of_videomode helpers
From: Thierry Reding @ 2012-11-13 11:35 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-7-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On Mon, Nov 12, 2012 at 04:37:06PM +0100, Steffen Trumtrar wrote:
[...]
> +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> +static void dump_drm_displaymode(struct drm_display_mode *m)
> +{
> + pr_debug("drm_displaymode = %d %d %d %d %d %d %d %d %d\n",
> + m->hdisplay, m->hsync_start, m->hsync_end, m->htotal,
> + m->vdisplay, m->vsync_start, m->vsync_end, m->vtotal,
> + m->clock);
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().
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> @@ -1457,6 +1458,10 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
>
> extern int videomode_to_display_mode(struct videomode *vm,
> struct drm_display_mode *dmode);
> +extern int of_get_drm_display_mode(struct device_node *np,
> + struct drm_display_mode *dmode,
> + int index);
Also requires either a dummy or protection.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 5/6] drm_modes: add videomode helpers
From: Thierry Reding @ 2012-11-13 11:31 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: linux-fbdev, Stephen Warren, devicetree-discuss, dri-devel,
Tomi Valkeinen, Rob Herring, Laurent Pinchart, kernel,
Guennady Liakhovetski, linux-media
In-Reply-To: <1352734626-27412-6-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 580 bytes --]
On Mon, Nov 12, 2012 at 04:37:05PM +0100, Steffen Trumtrar wrote:
[...]
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
[...]
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode)
This one as well may be more consistently named
drm_display_mode_from_videomode().
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> +extern int videomode_to_display_mode(struct videomode *vm,
> + struct drm_display_mode *dmode);
And this also needs protection or a dummy.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 4/6] fbmon: add of_videomode helpers
From: Thierry Reding @ 2012-11-13 11:28 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-5-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
On Mon, Nov 12, 2012 at 04:37:04PM +0100, Steffen Trumtrar wrote:
[...]
> diff --git a/include/linux/fb.h b/include/linux/fb.h
[...]
> +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
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 3/6] fbmon: add videomode helpers
From: Thierry Reding @ 2012-11-13 11:22 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-4-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2044 bytes --]
On Mon, Nov 12, 2012 at 04:37:03PM +0100, Steffen Trumtrar wrote:
[...]
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode)
The other helpers are named <destination-type>_from_<source-type>(),
maybe this should follow that example for consistency?
> +{
> + fbmode->xres = vm->hactive;
> + fbmode->left_margin = vm->hback_porch;
> + fbmode->right_margin = vm->hfront_porch;
> + fbmode->hsync_len = vm->hsync_len;
> +
> + fbmode->yres = vm->vactive;
> + fbmode->upper_margin = vm->vback_porch;
> + fbmode->lower_margin = vm->vfront_porch;
> + fbmode->vsync_len = vm->vsync_len;
> +
> + fbmode->pixclock = KHZ2PICOS(vm->pixelclock / 1000);
> +
> + fbmode->sync = 0;
> + fbmode->vmode = 0;
> + if (vm->hah)
> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> + if (vm->vah)
> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> + if (vm->interlaced)
> + fbmode->vmode |= FB_VMODE_INTERLACED;
> + if (vm->doublescan)
> + fbmode->vmode |= FB_VMODE_DOUBLE;
> + if (vm->de)
> + fbmode->sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
> + fbmode->refresh = 60;
Can the refresh rate not be computed from the pixel clock and the
horizontal and vertical timings?
> + fbmode->flag = 0;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(videomode_to_fb_videomode);
> +#endif
> +
> +
There's a gratuitous blank line here.
> #else
> int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..46c665b 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -714,6 +714,8 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>
> +extern int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode);
> +
Should you provide a dummy in the !CONFIG_VIDEOMODE case?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Thierry Reding @ 2012-11-13 11:08 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel
In-Reply-To: <1352734626-27412-3-git-send-email-s.trumtrar@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 5800 bytes --]
On Mon, Nov 12, 2012 at 04:37:02PM +0100, 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.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 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
>
> diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
> new file mode 100644
> index 0000000..e22e2fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display-timings.txt
> @@ -0,0 +1,107 @@
> +display-timings bindings
> +========================
> +
> +display-timings-node
> +--------------------
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
> +---------------
Same here: "timing subnodes"?
> +
> +required properties:
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
"display"?
> + 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?
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2a23b18..a324457 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -39,6 +39,19 @@ config DISPLAY_TIMING
> config VIDEOMODE
> bool
>
> +config OF_DISPLAY_TIMING
OF_DISPLAY_TIMINGS?
> diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
of_display_timings.c?
> +/**
> + * 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?
> diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
[...]
> +
> +int of_get_videomode(struct device_node *np, struct videomode *vm, int index);
> +#endif /* __LINUX_OF_VIDEOMODE_H */
There should be a blank line between the last two lines I think.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-13 10:53 UTC (permalink / raw)
To: Alexey Klimov
Cc: devicetree-discuss, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media, Tomi Valkeinen,
Stephen Warren, kernel
In-Reply-To: <CALW4P+JmyEnnxpC8AuJ1-mPG-Rw0XEyZFogeVaNuw5omdH5-CA@mail.gmail.com>
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
> <s.trumtrar@pengutronix.de> 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.
> >
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > 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
> >
> > diff --git a/Documentation/devicetree/bindings/video/display-timings.txt b/Documentation/devicetree/bindings/video/display-timings.txt
> > new file mode 100644
> > index 0000000..e22e2fd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/display-timings.txt
> > @@ -0,0 +1,107 @@
> > +display-timings bindings
> > +============
> > +
> > +display-timings-node
> > +--------------------
> > +
> > +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
> > +---------------
> > +
> > +required properties:
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> > + in pixels
> > + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> > + lines
> > + - clock-frequency: displayclock in Hz
> > +
> > +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
> > +
> > +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
> > +==========> > +
> > + +----------+---------------------------------------------+----------+-------+
> > + | | ↑ | | |
> > + | | |vback_porch | | |
> > + | | ↓ | | |
> > + +----------###############################################----------+-------+
> > + | # ↑ # | |
> > + | # | # | |
> > + | hback # | # hfront | hsync |
> > + | porch # | hactive # porch | len |
> > + |<-------->#<---------------+--------------------------->#<-------->|<----->|
> > + | # | # | |
> > + | # |vactive # | |
> > + | # | # | |
> > + | # ↓ # | |
> > + +----------###############################################----------+-------+
> > + | | ↑ | | |
> > + | | |vfront_porch | | |
> > + | | ↓ | | |
> > + +----------+---------------------------------------------+----------+-------+
> > + | | ↑ | | |
> > + | | |vsync_len | | |
> > + | | ↓ | | |
> > + +----------+---------------------------------------------+----------+-------+
> > +
> > +
> > +Example:
> > +
> > + display-timings {
> > + native-mode = <&timing0>;
> > + timing0: 1920p24 {
> > + /* 1920x1080p24 */
> > + clock-frequency = <52000000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hfront-porch = <25>;
> > + hback-porch = <25>;
> > + hsync-len = <25>;
> > + vback-porch = <2>;
> > + vfront-porch = <2>;
> > + vsync-len = <2>;
> > + hsync-active = <1>;
> > + };
> > + };
> > +
> > +Every required property also supports the use of ranges, so the commonly used
> > +datasheet description with <min typ max>-tuples can be used.
> > +
> > +Example:
> > +
> > + timing1: timing {
> > + /* 1920x1080p24 */
> > + clock-frequency = <148500000>;
> > + hactive = <1920>;
> > + vactive = <1080>;
> > + hsync-len = <0 44 60>;
> > + hfront-porch = <80 88 95>;
> > + hback-porch = <100 148 160>;
> > + vfront-porch = <0 4 6>;
> > + vback-porch = <0 36 50>;
> > + vsync-len = <0 5 6>;
> > + };
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 2a23b18..a324457 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -39,6 +39,19 @@ config DISPLAY_TIMING
> > config VIDEOMODE
> > bool
> >
> > +config OF_DISPLAY_TIMING
> > + def_bool y
> > + select DISPLAY_TIMING
> > + help
> > + helper to parse display timings from the devicetree
> > +
> > +config OF_VIDEOMODE
> > + def_bool y
> > + select VIDEOMODE
> > + select OF_DISPLAY_TIMING
> > + help
> > + helper to get videomodes from the devicetree
> > +
> > menuconfig FB
> > tristate "Support for frame buffer devices"
> > ---help---
> > diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> > index fc30439..b936b00 100644
> > --- a/drivers/video/Makefile
> > +++ b/drivers/video/Makefile
> > @@ -168,4 +168,6 @@ 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_OF_DISPLAY_TIMING) += of_display_timing.o
> > obj-$(CONFIG_VIDEOMODE) += videomode.o
> > +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o
> > diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c
> > new file mode 100644
> > index 0000000..0bd30cc
> > --- /dev/null
> > +++ b/drivers/video/of_display_timing.c
> > @@ -0,0 +1,186 @@
> > +/*
> > + * OF helpers for parsing display timings
> > + *
> > + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
> > + *
> > + * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de>
> > + *
> > + * This file is released under the GPLv2
> > + */
> > +#include <linux/of.h>
> > +#include <linux/slab.h>
> > +#include <linux/export.h>
> > +#include <linux/of_display_timings.h>
> > +
> > +/**
> > + * parse_property - parse timing_entry from device_node
> > + * @np: device_node with the property
> > + * @name: name of the property
> > + * @result: will be set to the return value
> > + *
> > + * DESCRIPTION:
> > + * Every display_timing can be specified with either just the typical value or
> > + * a range consisting of min/typ/max. This function helps handling this
> > + **/
> > +static int parse_property(struct device_node *np, char *name,
> > + struct timing_entry *result)
> > +{
> > + struct property *prop;
> > + int length, 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;
> > + } else if (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;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * of_get_display_timing - parse display_timing entry from device_node
> > + * @np: device_node with the properties
> > + **/
> > +static struct display_timing *of_get_display_timing(struct device_node *np)
> > +{
> > + struct display_timing *dt;
> > + int ret = 0;
> > +
> > + dt = kzalloc(sizeof(*dt), GFP_KERNEL);
> > + if (!dt) {
> > + pr_err("%s: could not allocate display_timing struct\n", __func__);
> > + return NULL;
> > + }
> > +
> > + 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__);
> > + return NULL;
> > + }
> > +
> > + return dt;
> > +}
> > +
> > +/**
> > + * 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)
> > +{
> > + struct device_node *timings_np;
> > + struct device_node *entry;
> > + struct device_node *native_mode;
> > + struct display_timings *disp;
> > +
> > + if (!np) {
> > + pr_err("%s: no devicenode given\n", __func__);
> > + return NULL;
> > + }
> > +
> > + timings_np = of_find_node_by_name(np, "display-timings");
> > + if (!timings_np) {
> > + pr_err("%s: could not find display-timings node\n", __func__);
> > + return NULL;
> > + }
> > +
> > + 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__);
> > + return NULL;
> > + }
> > +
> > + 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(struct display_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 |
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox