* RE: [PATCH v12 3/6] fbmon: add videomode helpers
From: Manjunathappa, Prakash @ 2012-11-21 10:09 UTC (permalink / raw)
To: Steffen Trumtrar, devicetree-discuss@lists.ozlabs.org
Cc: Rob Herring, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media@vger.kernel.org,
Valkeinen, Tomi, Stephen Warren, kernel@pengutronix.de,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353426896-6045-4-git-send-email-s.trumtrar@pengutronix.de>
Hi Steffen,
I am trying to add DT support for da8xx-fb driver on top of your patches.
Encountered below build error. Sorry for reporting it late.
On Tue, Nov 20, 2012 at 21:24:53, Steffen Trumtrar wrote:
> Add a function to convert from the generic videomode to a fb_videomode.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fb.h | 6 ++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..c1939a6 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <video/edid.h>
> +#include <linux/videomode.h>
> #ifdef CONFIG_PPC_OF
> #include <asm/prom.h>
> #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
> kfree(timings);
> return err;
> }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> + struct fb_videomode *fbmode)
> +{
> + unsigned int htotal, vtotal;
> +
> + 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;
"FB_SYNC_DATA_ENABLE_HIGH_ACT" seems to be mxsfb specific flag, I am getting
build error on this. Please let me know if I am missing something.
Thanks,
Prakash
> + fbmode->flag = 0;
> +
> + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> + vm->hsync_len;
> + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> + vm->vsync_len;
> + fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
> +
> #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..920cbe3 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -14,6 +14,7 @@
> #include <linux/backlight.h>
> #include <linux/slab.h>
> #include <asm/io.h>
> +#include <linux/videomode.h>
>
> struct vm_area_struct;
> struct fb_info;
> @@ -714,6 +715,11 @@ 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);
>
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> + struct fb_videomode *fbmode);
> +#endif
> +
> /* drivers/video/modedb.c */
> #define VESA_MODEDB_SIZE 34
> extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21 10:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50AC956D.7020303@ti.com>
On Wednesday 21 November 2012 16:48:45 Tomi Valkeinen wrote:
> If the power-off sequence disables a regulator that was supposed to be
> enabled by the power-on sequence (but wasn't enabled because of an
> error), the regulator_disable is still called when the driver runs the
> power-off sequence, isn't it? Regulator enables and disables are ref
> counted, and the enables should match the disables.
And there collapses my theory.
> > Failures might be better handled if sequences have some "recovery policy"
> > about what to do when they fail, as mentioned in the link above. As you
> > pointed out, the driver might not always know enough about the resources
> > involved to do the right thing.
>
> Yes, I think such recovery policy would be needed.
Indeed, from your last paragraph this makes even more sense now.
Oh, and I noticed I forgot to reply to this:
> This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
> device, no matter where and how many times it's used?
That's true - however when dereferencing the phandle, the underlying framework
will try to acquire the PWM, which will result in failure if the same resource
is referenced several times.
One could compare the phandles to avoid this, but in your example you must
know that for PWMs the "xyz" part is not relevant for comparison.
This makes referencing of resources by name much easier to implement and more
elegant with respect to frameworks leveraging.
Alex.
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: mxs: add oled support for the cfa-10036
From: Maxime Ripard @ 2012-11-21 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1351674774-2891-3-git-send-email-maxime.ripard@free-electrons.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Shawn,
Le 31/10/2012 10:12, Maxime Ripard a écrit :
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> Cc:
> Brian Lilly <brian@crystalfontz.com> ---
> arch/arm/boot/dts/imx28-cfa10036.dts | 19 +++++++++++++++++++ 1
> file changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx28-cfa10036.dts
> b/arch/arm/boot/dts/imx28-cfa10036.dts index c03a577..816cae9
> 100644 --- a/arch/arm/boot/dts/imx28-cfa10036.dts +++
> b/arch/arm/boot/dts/imx28-cfa10036.dts @@ -33,11 +33,30 @@ };
>
> apbx@80040000 { + pwm: pwm@80064000 { + pinctrl-names > "default"; + pinctrl-0 = <&pwm4_pins_a>; + status = "okay"; +
> }; + duart: serial@80074000 { pinctrl-names = "default"; pinctrl-0
> = <&duart_pins_b>; status = "okay"; }; + + i2c0: i2c@80058000 { +
> pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pins_b>; +
> status = "okay"; + + ssd1307: oled@3c { + compatible > "solomon,ssd1307fb-i2c"; + reg = <0x3c>; + pwms = <&pwm 4
> 3000>; + reset-gpios = <&gpio2 7 0>; + }; + }; }; };
Would you consider merging this patch?
Andrew Morton merged the driver into its tree
(http://marc.info/?l=linux-fbdev&m\x135336761530731&w=2)
Thanks,
Maxime
- --
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
iEYEARECAAYFAlCsl7MACgkQGxsu9jQV9nZtJwCfR9c2SnwK1hzAU2F+xptFqg9x
2+0An3YrhNMXSZaPwPcJwc3Bnwy+FSqe
=mjpV
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 8:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1699753.ZQsWMHINxd@percival>
[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]
On 2012-11-21 10:32, Alex Courbot wrote:
>> Ok. I'll need to dig up the conversation
>
> IIRC it was somewhere around here:
>
> https://lkml.org/lkml/2012/9/7/662
>
> See the parent messages too.
Thanks.
>> Did you consider any examples
>> of how some driver could handle the error cases?
>
> For all the (limited) use cases I considered, playing the power-off sequence
> when power-on fails just works. If power-off also fails you are potentially in
> more trouble though. Maybe we could have another "run" function that does not
> stop on errors for handling such cases where you want to "stop everything you
> can".
If the power-off sequence disables a regulator that was supposed to be
enabled by the power-on sequence (but wasn't enabled because of an
error), the regulator_disable is still called when the driver runs the
power-off sequence, isn't it? Regulator enables and disables are ref
counted, and the enables should match the disables.
> Failures might be better handled if sequences have some "recovery policy"
> about what to do when they fail, as mentioned in the link above. As you
> pointed out, the driver might not always know enough about the resources
> involved to do the right thing.
Yes, I think such recovery policy would be needed.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21 8:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50AC8D3B.6040300@ti.com>
On Wednesday 21 November 2012 16:13:47 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On 2012-11-21 03:56, Alex Courbot wrote:
> > Hi Tomi,
> >
> > On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
> >> I guess there's a reason, but the above looks a bit inconsistent. For
> >> gpio you define the gpio resource inside the step. For power and pwm the
> >> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
> >> 5000000>;" work in step2?
> >
> > That's mostly a framework issue. Most frameworks do not export a function
> > that allow to dereference a phandle - they expect resources to be
> > declared right under the device node and accessed by name through
> > foo_get(device, name). So using phandles in power sequences would require
> > to export these additional
> Right, I expected something like that.
>
> > functions and also opens the door to some inconsistencies - for instance,
> > your PWM phandle could be referenced a second time in the sequence with a
> > different period - how do you know that these are actually referring the
> > same PWM device?
>
> This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
> device, no matter where and how many times it's used?
>
> >>> +When a power sequence is run, its steps is executed one after the other
> >>> until +one step fails or the end of the sequence is reached.
> >>
> >> The document doesn't give any hint of what the driver should do if
> >> running the power sequence fails. Run the "opposite" power sequence?
> >> Will that work for all resources? I'm mainly thinking of a case where
> >> each enable of the resource should be matched by a disable, i.e. you
> >> can't call disable if no enable was called.
> >
> > We discussed that issue already (around v5 I think) and the conclusion was
> > that it should be up to the driver. When we simply enable/disable
> > resources it is easy to revert, but in the future non-boolean properties
> > will likely be introduced, and these cannot easily be reverted. Moreover
> > some drivers might have more complex recovery needs. This deserves more
> > discussion I think, as I'd like to have some "generic" recovery mechanism
> > that covers most of the cases.
>
> Ok. I'll need to dig up the conversation
IIRC it was somewhere around here:
https://lkml.org/lkml/2012/9/7/662
See the parent messages too.
> Did you consider any examples
> of how some driver could handle the error cases?
For all the (limited) use cases I considered, playing the power-off sequence
when power-on fails just works. If power-off also fails you are potentially in
more trouble though. Maybe we could have another "run" function that does not
stop on errors for handling such cases where you want to "stop everything you
can".
> What I'm worried about is that, as far as I understand, the power
> sequence is kinda like black box to the driver. The driver just does
> "power-up", without knowing what really goes on in there.
The driver could always inspect the sequence, but you are right in that this
is not how it is intended to be done.
> And if it doesn't know what goes on in there, nor what's in "power-down"
> sequence, how can it do anything when an error happens? The only option
> I see is that the driver doesn't do anything, which will leave some
> resources enabled, or it can run the power-down sequence, which may or
> may not work.
Failures might be better handled if sequences have some "recovery policy"
about what to do when they fail, as mentioned in the link above. As you
pointed out, the driver might not always know enough about the resources
involved to do the right thing.
Alex.
^ permalink raw reply
* Re: [PATCH v12 0/6] of: add display helper
From: Steffen Trumtrar @ 2012-11-21 8:28 UTC (permalink / raw)
To: Thierry Reding
Cc: Robert Schwebel, Laurent Pinchart, devicetree-discuss,
Rob Herring, linux-fbdev, dri-devel, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121120193531.GB27186@avionic-0098.adnet.avionic-design.de>
On Tue, Nov 20, 2012 at 08:35:31PM +0100, Thierry Reding wrote:
> On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote:
> > On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> > > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > > > Hi!
> > > >
> > > > Changes since v11:
> > > > - make pointers const where applicable
> > > > - add reviewed-by Laurent Pinchart
> > >
> > > Looks good to me.
> > >
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Through which tree do you plan to push this ?
> >
> > We have no idea yet, and none of the people on Cc: have volunteered
> > so far... what do you think?
>
> The customary approach would be to take the patches through separate
> trees, but I think this particular series is sufficiently interwoven to
> warrant taking them all through one tree. However the least that we
> should do is collect Acked-by's from the other tree maintainers.
>
> Given that most of the patches modify files in drivers/video, Florian's
> fbdev tree would be the most obvious candidate, right? If Florian agrees
> to take the patches, all we would need is David's Acked-by.
>
> How does that sound?
>
> Thierry
Hi!
That is exactly the way I thought this could happen.
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: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-11-21 8:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4316169.5QXVzv7peZ@percival>
[-- Attachment #1: Type: text/plain, Size: 2745 bytes --]
On 2012-11-21 03:56, Alex Courbot wrote:
> Hi Tomi,
>
> On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
>> I guess there's a reason, but the above looks a bit inconsistent. For
>> gpio you define the gpio resource inside the step. For power and pwm the
>> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
>> 5000000>;" work in step2?
>
> That's mostly a framework issue. Most frameworks do not export a function that
> allow to dereference a phandle - they expect resources to be declared right
> under the device node and accessed by name through foo_get(device, name). So
> using phandles in power sequences would require to export these additional
Right, I expected something like that.
> functions and also opens the door to some inconsistencies - for instance, your
> PWM phandle could be referenced a second time in the sequence with a different
> period - how do you know that these are actually referring the same PWM
> device?
This I didn't understand. Doesn't "<&pwm 2 xyz>" point to a single
device, no matter where and how many times it's used?
>>> +When a power sequence is run, its steps is executed one after the other
>>> until +one step fails or the end of the sequence is reached.
>>
>> The document doesn't give any hint of what the driver should do if
>> running the power sequence fails. Run the "opposite" power sequence?
>> Will that work for all resources? I'm mainly thinking of a case where
>> each enable of the resource should be matched by a disable, i.e. you
>> can't call disable if no enable was called.
>
> We discussed that issue already (around v5 I think) and the conclusion was
> that it should be up to the driver. When we simply enable/disable resources it
> is easy to revert, but in the future non-boolean properties will likely be
> introduced, and these cannot easily be reverted. Moreover some drivers might
> have more complex recovery needs. This deserves more discussion I think, as
> I'd like to have some "generic" recovery mechanism that covers most of the
> cases.
Ok. I'll need to dig up the conversation. Did you consider any examples
of how some driver could handle the error cases?
What I'm worried about is that, as far as I understand, the power
sequence is kinda like black box to the driver. The driver just does
"power-up", without knowing what really goes on in there.
And if it doesn't know what goes on in there, nor what's in "power-down"
sequence, how can it do anything when an error happens? The only option
I see is that the driver doesn't do anything, which will leave some
resources enabled, or it can run the power-down sequence, which may or
may not work.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [PATCH v12 0/6] of: add display helper
From: Steffen Trumtrar @ 2012-11-21 7:41 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Florian Tobias Schandinat,
David Airlie, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1501232.SOApmW1MhU@avalon>
On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > Hi!
> >
> > Changes since v11:
> > - make pointers const where applicable
> > - add reviewed-by Laurent Pinchart
>
> Looks good to me.
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
Thanks.
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: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21 4:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121120215429.B621F3E1821@localhost>
Hi Grant,
On Wednesday 21 November 2012 05:54:29 Grant Likely wrote:
> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but
>
> This isn't strictly true. It is still perfectly fine to have board
> specific code when necessary. However, there is strong encouragement to
> enable that code in device drivers as much as possible and new board
> files need to have very strong justification.
But doesn't introducing board-specific code into the kernel just defeats the
purpose of the DT? If we extend this logic, we are heading straight back to
board-definition files. To a lesser extent than before I agree, but the problem
is fundamentally the same.
> > need a way to implement these sequences in a portable manner. This patch
> > introduces a simple interpreter that can execute such power sequences
> > encoded either as platform data or within the device tree. It also
> > introduces first support for regulator, GPIO and PWM resources.
>
> This is where I start getting nervous. Up to now I've strongly resisted
> adding any kind of interpreted code to the device tree. The model is to
> identify hardware, but require the driver to know how to control it. (as
> compared to ACPI which is entirely designed around executable
> bytecode).
>
> While the power sequences described here certainly cannot be confused
> with a Turing complete bytecode, it is a step in that direction.
Technically speaking power sequences are a step towards an interpreter, but it
is a very small one and it should not go much further than the current state.
I understand the concern of having "code" into the DT but I really think it
should be viewed from a different angle.
Powering sequences are special in that they can be affected by the board design
or the devices variations. For instance hundreds of different panels with
backlights are currently compatible with the pwm-backlight driver. The only
thing that differenciates them is how the backlight is powered on and off. If
you are to build a kernel that is supposed to support all these panels, you
would need to embed all the powering sequences in the kernel even though only
one of them will be used by one specific board. Power sequences in the DT help
preventing that.
With that stated, it is clear that we should not need to define more than the
short, simple sequences of actions that cannot be elegantly handled by the
driver. Anything beyond that should be handled by the driver itself. In
particular, here are a few things I do *not* want to see included in power
seqs:
- conditionals/jumps (or it's not a sequence anymore).
- direct access to hardware. Resources must at least be abstracted in some
way. You shall not e.g. access the address space directly.
- support for non-power related resources - that is out of the special case of
powering sequences and should be done by the driver
That should keep the "grammar" simple, and the sequences short enough to that
we can consider then as data belonging to the device, and not as code that is
interpreted.
> I think this will get very verbose in a hurry. Already this simple
> example is 45 lines long. Using the device tree structure to encode the
> language doesn't look like a very good fit. Not to mention that the
> order of operations is entirely based on the node name. Want to insert
> an operation between step0 and step1? Need to rename step1, step2, and
> step3 to do so.
I don't like that steps numbering thing neither, but it seems to be the best
way to do it so far.
As for the DT structure not being adapted for this - I would agree if we
wanted to implement a complete interpreter, but that's precisely not the case.
More about this later.
> This implementation also isn't very consistent. The gpio is referenced
> with a phandle in step3/step0, but the regulator and pwm are referenced
> by id.
Tomi made the same remark - the reason for using the phandle in GPIO is
because GPIO framework does not support referencing GPIOs by name yet. I
wanted to DT bindings to reflect the underlying framework as much as possible
until we have a function like gpio_get(device, id).
However I agree that this makes things inconsistent at the moment and would
require a bindings change. And in the case of the DT this is actually easy to
implement (I did it in some previous versions). I'll make sure to do it.
> As an alternative, what about something like the following?
>
> backlight {
> compatible = "pwm-backlight";
> ...
>
> /* resources used by the power sequences */
> pwms = <&pwm 2 5000000>;
> pwm-names = "backlight";
> regulators = <&backlight_reg>;
> gpios = <&gpio 28 0>;
>
> power-on-sequence = "r0e;d10000m;p0e;g0s";
> power-off-sequence = "g0c;p0d;d10000m;r0d";
> };
Well, *now* it really looks like bytecode. :)
> I'm thinking about the scripts as trivial-to-parse ascii strings that
> have a very simple set of commands. The commands use resources already
> defined in the node. ie. 'g0' refers to the first entry in the gpios
> property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> no means take this as the format to use, it is just an example off the
> top of my head, but it is already way easier to work with than putting
> each command into a node.
I'd argue that this opens the door wide open towards having a complete
interpreter in the device tree. At least DT nodes restrict what we can do
conveniently.
> The trick is still to define a syntax that doesn't fall apart when it
> needs to be extended. I would also like to get opinions on whether or
> not conditionals or loops should be supported (ie. loop waiting for a
> status to change). If they should then we need to be a lot more careful
> about the design (and due to my aforementioned nervousness, somebody may
> need to get me therapy).
That's IMHO the main argument in favor of using DT nodes here (the syntax
extension, not your therapy). They can be extended simply by adding properties
to them, and I was relying on this to make power seqs extensible while keeping
things consistent (and backward-compatible). For instance, when we want to
support voltage setting in regulators we can just add a "voltage" property for
that.
So to sum up the pros of the current node-base representation:
- give a "data like" representation of powering sequences (what they should
be, actually)
- puts sanity barriers for not turning power seqs into a real code interpreter
- easily extensible
There are probably a few cons, the numbering of steps being one, but at least
we don't need to design a new DSL and care too much about extensibility which
is, in the nodes case, built-in and free.
I also like to make it clear that I don't want to see this going out of
control and that any extension proposal would have to be thoroughly justified
and reviewed - and better not contradict any of the 3 points listed above.
If that makes you feel better, maybe we can try and fix what is wrong with the
current bindings instead of introducing a new language that will be, by
nature, more complex to handle and difficult to extend without breaking things?
Alex.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-21 1:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50AB9832.90709@ti.com>
Hi Tomi,
On Tuesday 20 November 2012 22:48:18 Tomi Valkeinen wrote:
> I guess there's a reason, but the above looks a bit inconsistent. For
> gpio you define the gpio resource inside the step. For power and pwm the
> resource is defined before the steps. Why wouldn't "pwm = <&pwm 2
> 5000000>;" work in step2?
That's mostly a framework issue. Most frameworks do not export a function that
allow to dereference a phandle - they expect resources to be declared right
under the device node and accessed by name through foo_get(device, name). So
using phandles in power sequences would require to export these additional
functions and also opens the door to some inconsistencies - for instance, your
PWM phandle could be referenced a second time in the sequence with a different
period - how do you know that these are actually referring the same PWM
device?
> > +When a power sequence is run, its steps is executed one after the other
> > until +one step fails or the end of the sequence is reached.
>
> The document doesn't give any hint of what the driver should do if
> running the power sequence fails. Run the "opposite" power sequence?
> Will that work for all resources? I'm mainly thinking of a case where
> each enable of the resource should be matched by a disable, i.e. you
> can't call disable if no enable was called.
We discussed that issue already (around v5 I think) and the conclusion was
that it should be up to the driver. When we simply enable/disable resources it
is easy to revert, but in the future non-boolean properties will likely be
introduced, and these cannot easily be reverted. Moreover some drivers might
have more complex recovery needs. This deserves more discussion I think, as
I'd like to have some "generic" recovery mechanism that covers most of the
cases.
Alex.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-11-21 1:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121120215429.B621F3E1821@localhost>
[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]
On Tue, Nov 20, 2012 at 09:54:29PM +0000, Grant Likely wrote:
> On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but
> This isn't strictly true. It is still perfectly fine to have board
> specific code when necessary. However, there is strong encouragement to
> enable that code in device drivers as much as possible and new board
> files need to have very strong justification.
This isn't the message that's gone over, and even for device drivers
everyone seems to be taking the whole device tree thing as a move to
pull all data out of the kernel. In some cases there are some real
practical advantages to doing this but a lot of the people making these
changes seem to view having things in DT as a goal in itself.
> I'm thinking about the scripts as trivial-to-parse ascii strings that
> have a very simple set of commands. The commands use resources already
> defined in the node. ie. 'g0' refers to the first entry in the gpios
> property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
> no means take this as the format to use, it is just an example off the
> top of my head, but it is already way easier to work with than putting
> each command into a node.
It does appear to have some legibility challenges, especially with using
the indexes into the arrays of resources. On the other hand the arrays
should be fairly small.
> > +Platform Data Format
> > +--------------------
> > +All relevant data structures for declaring power sequences are located in
> > +include/linux/power_seq.h.
> Hmm... If sequences are switched to a string instead, then platform_data
> should also use it. The power sequence data structures can be created at
> runtime by parsing the string.
Seems like a step backwards to me - why not let people store the more
parsed version of the data? It's going to be less convenient for users
on non-DT systems.
As I said in my earlier reviews I think this is a useful thing to have
as a utility library for drivers independantly of the DT bindings, it
would allow drivers to become more data driven. Perhaps we can rework
the series so that the DT bindings are added as a final patch? All the
rest of the code seems OK.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* efifb swapping Red/Blue color
From: Bruno Prémont @ 2012-11-20 23:14 UTC (permalink / raw)
To: linux-fbdev
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 749 bytes --]
On a MacBookAir2,1 with 9400M C79 nVidia GPU fbcon on efifb has
red and blue colors swapped.
Extract from kernel log:
efifb: probing for efifb
efifb: framebuffer at 0x80010000, mapped to 0xffffc90004580000, using 6400k, total 6400k
efifb: mode is 1280x800x32, linelength92, pages=1
efifb: scrolling: redraw
efifb: Truecolor: size=8:8:8:8, shift$:0:8:16
On another system with radeon GPU and externally connected DVI monitor
colors are set properly.
Probably efifb should be looking at something to determine if display
is RGB or BGR...
Thanks,
Bruno
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/5] OMAPFB: use dma_alloc instead of omap's vram
From: Tony Lindgren @ 2012-11-20 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50A5E801.3000100@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [121115 23:17]:
>
> I added your acks, and pushed:
>
> git://gitorious.org/linux-omap-dss2/linux.git 3.8/vram-conversion
>
> It's based on -rc4 as my other branches are based on that.
OK thanks!
Tony
^ permalink raw reply
* Re: [PATCHv9 0/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>
On Sat, 17 Nov 2012 19:55:44 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Apologies for sending two patchsets in two days - the main purpose
> of this new revision is to add the linux-arm-kernel list to the
> audience. A few suggestions from v8 have also been added.
Just to be clear from my previous reply, don't merge this series. The DT
binding is not good.
g.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Grant Likely @ 2012-11-20 21:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot@nvidia.com>
On Sat, 17 Nov 2012 19:55:45 +0900, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Some device drivers (e.g. panel or backlights) need to follow precise
> sequences for powering on and off, involving GPIOs, regulators, PWMs
> and other power-related resources, with a defined powering order and
> delays to respect between steps. These sequences are device-specific,
> and do not belong to a particular driver - therefore they have been
> performed by board-specific hook functions so far.
I must be honest, this series really makes me nervous...
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
This isn't strictly true. It is still perfectly fine to have board
specific code when necessary. However, there is strong encouragement to
enable that code in device drivers as much as possible and new board
files need to have very strong justification.
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree. It also
> introduces first support for regulator, GPIO and PWM resources.
This is where I start getting nervous. Up to now I've strongly resisted
adding any kind of interpreted code to the device tree. The model is to
identify hardware, but require the driver to know how to control it. (as
compared to ACPI which is entirely designed around executable
bytecode).
While the power sequences described here certainly cannot be confused
with a Turing complete bytecode, it is a step in that direction. Plus,
there will always be that new use case that needs just a "little new
feature" to make this work for that too. Without thinking about how to
handle that ahead of time is just asking for something to turn into a
maintenance nightmare. It's just as important to specify what the limits
of this approach are and when to punt to real driver code to handle a
device.
> +Power Sequences Steps
> +---------------------
> +Steps of a sequence describe an action to be performed on a resource. They
> +always include a "type" property which indicates what kind of resource this
> +step works on. Depending on the resource type, additional properties are defined
> +to control the action to be performed.
> +
> +"delay" type required properties:
> + - delay: delay to wait (in microseconds)
> +
> +"regulator" type required properties:
> + - id: name of the regulator to use.
> + - enable / disable: one of these two empty properties must be present to
> + enable or disable the resource
> +
> +"pwm" type required properties:
> + - id: name of the PWM to use.
> + - enable / disable: one of these two empty properties must be present to
> + enable or disable the resource
> +
> +"gpio" type required properties:
> + - gpio: phandle of the GPIO to use.
> + - value: value this GPIO should take. Must be 0 or 1.
> +
> +Example
> +-------
> +Here are example sequences declared within a backlight device that use all the
> +supported resources types:
> +
> + backlight {
> + compatible = "pwm-backlight";
> + ...
> +
> + /* resources used by the power sequences */
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> +
> + power-sequences {
> + power-on {
> + step0 {
> + type = "regulator";
> + id = "power";
> + enable;
> + };
> + step1 {
> + type = "delay";
> + delay = <10000>;
> + };
> + step2 {
> + type = "pwm";
> + id = "backlight";
> + enable;
> + };
> + step3 {
> + type = "gpio";
> + gpio = <&gpio 28 0>;
> + value = <1>;
> + };
> + };
> +
> + power-off {
> + step0 {
> + type = "gpio";
> + gpio = <&gpio 28 0>;
> + value = <0>;
> + };
> + step1 {
> + type = "pwm";
> + id = "backlight";
> + disable;
> + };
> + step2 {
> + type = "delay";
> + delay = <10000>;
> + };
> + step3 {
> + type = "regulator";
> + id = "power";
> + disable;
> + };
> + };
> + };
> + };
I think this will get very verbose in a hurry. Already this simple
example is 45 lines long. Using the device tree structure to encode the
language doesn't look like a very good fit. Not to mention that the
order of operations is entirely based on the node name. Want to insert
an operation between step0 and step1? Need to rename step1, step2, and
step3 to do so.
This implementation also isn't very consistent. The gpio is referenced
with a phandle in step3/step0, but the regulator and pwm are referenced
by id.
As an alternative, what about something like the following?
backlight {
compatible = "pwm-backlight";
...
/* resources used by the power sequences */
pwms = <&pwm 2 5000000>;
pwm-names = "backlight";
regulators = <&backlight_reg>;
gpios = <&gpio 28 0>;
power-on-sequence = "r0e;d10000m;p0e;g0s";
power-off-sequence = "g0c;p0d;d10000m;r0d";
};
I'm thinking about the scripts as trivial-to-parse ascii strings that
have a very simple set of commands. The commands use resources already
defined in the node. ie. 'g0' refers to the first entry in the gpios
property. 'r0' for the regulator, 'p0' for the pwms, 'd' means delay. By
no means take this as the format to use, it is just an example off the
top of my head, but it is already way easier to work with than putting
each command into a node.
The trick is still to define a syntax that doesn't fall apart when it
needs to be extended. I would also like to get opinions on whether or
not conditionals or loops should be supported (ie. loop waiting for a
status to change). If they should then we need to be a lot more careful
about the design (and due to my aforementioned nervousness, somebody may
need to get me therapy).
(Mitch, I'll let you make the argument for using Forth here. To be
honest, I'm not keen on defining any kind of new language, however
simple, but neither am I keen to pull in Forth).
> +Platform Data Format
> +--------------------
> +All relevant data structures for declaring power sequences are located in
> +include/linux/power_seq.h.
Hmm... If sequences are switched to a string instead, then platform_data
should also use it. The power sequence data structures can be created at
runtime by parsing the string.
g.
^ permalink raw reply
* Re: [PATCHv7] video: Add support for the Solomon SSD1307 OLED Controller
From: Andrew Morton @ 2012-11-20 19:43 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1353421237-4100-1-git-send-email-maxime.ripard@free-electrons.com>
On Tue, 20 Nov 2012 15:20:37 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> This patch adds support for the Solomon SSD1307 OLED
> controller found on the Crystalfontz CFA10036 board.
>
> This controller can drive a display with a resolution up
> to 128x39 and can operate over I2C or SPI.
>
> The current driver has only been tested on the CFA-10036,
> that is using this controller over I2C to driver a 96x16
> OLED screen.
Looks OK to my little eye. I merged it into -mm. If this
patch (or a variant of it) turns up in linux-next (via Florian's
tree) then I shall autodrop it again.
^ permalink raw reply
* Re: [PATCH v12 0/6] of: add display helper
From: Thierry Reding @ 2012-11-20 19:35 UTC (permalink / raw)
To: Robert Schwebel
Cc: Laurent Pinchart, Steffen Trumtrar, devicetree-discuss,
Rob Herring, linux-fbdev, dri-devel, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <20121120181129.GI23204@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]
On Tue, Nov 20, 2012 at 07:11:29PM +0100, Robert Schwebel wrote:
> On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> > On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > > Hi!
> > >
> > > Changes since v11:
> > > - make pointers const where applicable
> > > - add reviewed-by Laurent Pinchart
> >
> > Looks good to me.
> >
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Through which tree do you plan to push this ?
>
> We have no idea yet, and none of the people on Cc: have volunteered
> so far... what do you think?
The customary approach would be to take the patches through separate
trees, but I think this particular series is sufficiently interwoven to
warrant taking them all through one tree. However the least that we
should do is collect Acked-by's from the other tree maintainers.
Given that most of the patches modify files in drivers/video, Florian's
fbdev tree would be the most obvious candidate, right? If Florian agrees
to take the patches, all we would need is David's Acked-by.
How does that sound?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v12 0/6] of: add display helper
From: Robert Schwebel @ 2012-11-20 18:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Steffen Trumtrar, devicetree-discuss, Rob Herring, linux-fbdev,
dri-devel, Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel, Florian Tobias Schandinat,
David Airlie
In-Reply-To: <1501232.SOApmW1MhU@avalon>
On Tue, Nov 20, 2012 at 05:13:19PM +0100, Laurent Pinchart wrote:
> On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> > Hi!
> >
> > Changes since v11:
> > - make pointers const where applicable
> > - add reviewed-by Laurent Pinchart
>
> Looks good to me.
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Through which tree do you plan to push this ?
We have no idea yet, and none of the people on Cc: have volunteered
so far... what do you think?
rsc
--
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 v12 0/6] of: add display helper
From: Laurent Pinchart @ 2012-11-20 16:13 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: devicetree-discuss, Rob Herring, linux-fbdev, dri-devel,
Thierry Reding, Guennady Liakhovetski, linux-media,
Tomi Valkeinen, Stephen Warren, kernel, Florian Tobias Schandinat,
David Airlie
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar@pengutronix.de>
On Tuesday 20 November 2012 16:54:50 Steffen Trumtrar wrote:
> Hi!
>
> Changes since v11:
> - make pointers const where applicable
> - add reviewed-by Laurent Pinchart
Looks good to me.
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Through which tree do you plan to push this ?
> Regards,
> Steffen
>
>
> Steffen Trumtrar (6):
> video: add display_timing and videomode
> video: add of helper for videomode
> fbmon: add videomode helpers
> fbmon: add of_videomode helpers
> drm_modes: add videomode helpers
> drm_modes: add of_videomode helpers
>
> .../devicetree/bindings/video/display-timings.txt | 107 ++++++++++
> drivers/gpu/drm/drm_modes.c | 70 +++++++
> drivers/video/Kconfig | 19 ++
> drivers/video/Makefile | 4 +
> drivers/video/display_timing.c | 24 +++
> drivers/video/fbmon.c | 86 ++++++++
> drivers/video/of_display_timing.c | 216 +++++++++++++++++
> drivers/video/of_videomode.c | 48 +++++
> drivers/video/videomode.c | 46 +++++
> include/drm/drmP.h | 12 ++
> include/linux/display_timing.h | 70 +++++++
> include/linux/fb.h | 13 ++
> include/linux/of_display_timings.h | 20 ++
> include/linux/of_videomode.h | 18 ++
> include/linux/videomode.h | 40 ++++
> 15 files changed, 793 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/video/display-timings.txt create mode
> 100644 drivers/video/display_timing.c
> create mode 100644 drivers/video/of_display_timing.c
> create mode 100644 drivers/video/of_videomode.c
> create mode 100644 drivers/video/videomode.c
> create mode 100644 include/linux/display_timing.h
> create mode 100644 include/linux/of_display_timings.h
> create mode 100644 include/linux/of_videomode.h
> create mode 100644 include/linux/videomode.h
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] video console: add a driver for lcd2s character display
From: Arnd Bergmann @ 2012-11-20 16:11 UTC (permalink / raw)
To: Lars Poeschel
Cc: Lars Poeschel, FlorianSchandinat, mathieu.poirier, linux-fbdev,
linux-kernel
In-Reply-To: <201211201521.33726.poeschel@lemonage.de>
On Tuesday 20 November 2012, Lars Poeschel wrote:
> >
> > It's better to define the struct consw as a preinitialized
> > 'static const' object, rather than dynamically setting each
> > member.
>
> I could not find a place to store the drivers private data inside the struct
> vc_data. I wanted to have the struct consw inside the drivers private data
> (struct lcd2s_data) to be able to container_of to the drivers private data in
> the consw.con_* functions. This makes it possible to use more than one actual
> lcd2s display with the same driver. Otherwise I have to store the struct
> i2c_client somewhere statically and the driver can control only one display
> and has to forbid to be probed a second time. Or am I wrong somewhere ?
I believe you can only have one device anyway, because of the way you
register using "take_over_console(&data->consw, LCD2S_FIRST, LCD2S_LAST, 1);"
Whichever lcd2s was last registered gets VC 8 and 9. This is not nice, but
I think it's similar to how all the other VC work. They consequently don't
store per-device data at all, but just have global variables for device
specific data. We generally discourage this behaviour for device drivers,
but I woulnd't expect you to change the way that VC works, so you can just
do the same here.
Things would be different if this was a "console" driver rather than a "vc"
driver.
Arnd
^ permalink raw reply
* [PATCH v12 6/6] drm_modes: add of_videomode helpers
From: Steffen Trumtrar @ 2012-11-20 15:54 UTC (permalink / raw)
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Add helper to get drm_display_mode from devicetree.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/gpu/drm/drm_modes.c | 35 ++++++++++++++++++++++++++++++++++-
include/drm/drmP.h | 6 ++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 0073b27..04feef8 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,8 @@
#include <linux/export.h>
#include <drm/drmP.h>
#include <drm/drm_crtc.h>
-#include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
/**
* drm_mode_debug_printmodeline - debug print a mode
@@ -541,6 +542,38 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(const struct device_node *np,
+ struct drm_display_mode *dmode, unsigned int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, &vm, index);
+ if (ret)
+ return ret;
+
+ drm_display_mode_from_videomode(&vm, dmode);
+
+ pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+ vm.vactive, np->name);
+ drm_mode_debug_printmodeline(dmode);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
+
/**
* drm_mode_set_name - set the name on a mode
* @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index de2f6cf..377280f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
#include <linux/cdev.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/of.h>
#include <linux/videomode.h>
#if defined(__alpha__) || defined(__powerpc__)
#include <asm/pgtable.h> /* For pte_wrprotect */
@@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
extern int drm_display_mode_from_videomode(const struct videomode *vm,
struct drm_display_mode *dmode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_drm_display_mode(const struct device_node *np,
+ struct drm_display_mode *dmode,
+ unsigned int index);
+#endif
/* Modesetting support */
extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
--
1.7.10.4
^ permalink raw reply related
* [PATCH v12 5/6] drm_modes: add videomode helpers
From: Steffen Trumtrar @ 2012-11-20 15:54 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar@pengutronix.de>
Add conversion from videomode to drm_display_mode
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/gpu/drm/drm_modes.c | 37 +++++++++++++++++++++++++++++++++++++
include/drm/drmP.h | 6 ++++++
2 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..0073b27 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
#include <linux/export.h>
#include <drm/drmP.h>
#include <drm/drm_crtc.h>
+#include <linux/videomode.h>
/**
* drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,42 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh,
}
EXPORT_SYMBOL(drm_gtf_mode);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int drm_display_mode_from_videomode(const struct videomode *vm,
+ struct drm_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);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
+#endif
+
/**
* drm_mode_set_name - set the name on a mode
* @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..de2f6cf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
#include <linux/cdev.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/videomode.h>
#if defined(__alpha__) || defined(__powerpc__)
#include <asm/pgtable.h> /* For pte_wrprotect */
#endif
@@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
drm_mode_create_from_cmdline_mode(struct drm_device *dev,
struct drm_cmdline_mode *cmd);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int drm_display_mode_from_videomode(const struct videomode *vm,
+ struct drm_display_mode *dmode);
+#endif
+
/* Modesetting support */
extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
--
1.7.10.4
^ permalink raw reply related
* [PATCH v12 4/6] fbmon: add of_videomode helpers
From: Steffen Trumtrar @ 2012-11-20 15:54 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Rob Herring, linux-fbdev, dri-devel,
Laurent Pinchart, Thierry Reding, Guennady Liakhovetski,
linux-media, Tomi Valkeinen, Stephen Warren, kernel,
Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar@pengutronix.de>
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/fbmon.c | 42 +++++++++++++++++++++++++++++++++++++++++-
include/linux/fb.h | 7 +++++++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index c1939a6..16c353c 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,7 +31,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <video/edid.h>
-#include <linux/videomode.h>
+#include <linux/of_videomode.h>
#ifdef CONFIG_PPC_OF
#include <asm/prom.h>
#include <asm/pci-bridge.h>
@@ -1418,6 +1418,46 @@ int fb_videomode_from_videomode(const struct videomode *vm,
EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
#endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+ pr_debug("fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u %u\n",
+ m->xres, m->yres, m->refresh, m->pixclock, m->left_margin,
+ m->right_margin, m->upper_margin, m->lower_margin,
+ m->hsync_len, m->vsync_len, m->sync, m->vmode, m->flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(const struct device_node *np, struct fb_videomode *fb,
+ unsigned int index)
+{
+ struct videomode vm;
+ int ret;
+
+ ret = of_get_videomode(np, &vm, index);
+ if (ret)
+ return ret;
+
+ fb_videomode_from_videomode(&vm, fb);
+
+ pr_info("%s: got %dx%d display mode from %s\n", __func__, vm.hactive,
+ vm.vactive, np->name);
+ dump_fb_videomode(fb);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
#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 920cbe3..41b5e49 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
#include <linux/slab.h>
#include <asm/io.h>
#include <linux/videomode.h>
+#include <linux/of.h>
+#include <linux/of_videomode.h>
struct vm_area_struct;
struct fb_info;
@@ -715,6 +717,11 @@ 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);
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_fb_videomode(const struct device_node *np,
+ struct fb_videomode *fb,
+ unsigned int index);
+#endif
#if IS_ENABLED(CONFIG_VIDEOMODE)
extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);
--
1.7.10.4
^ permalink raw reply related
* [PATCH v12 3/6] fbmon: add videomode helpers
From: Steffen Trumtrar @ 2012-11-20 15:54 UTC (permalink / raw)
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, David Airlie,
Florian Tobias Schandinat,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomi Valkeinen,
Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Steffen Trumtrar,
Guennady Liakhovetski, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Add a function to convert from the generic videomode to a fb_videomode.
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/fbmon.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fb.h | 6 ++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..c1939a6 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <video/edid.h>
+#include <linux/videomode.h>
#ifdef CONFIG_PPC_OF
#include <asm/prom.h>
#include <asm/pci-bridge.h>
@@ -1373,6 +1374,51 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
kfree(timings);
return err;
}
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(const struct videomode *vm,
+ struct fb_videomode *fbmode)
+{
+ unsigned int htotal, vtotal;
+
+ 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->flag = 0;
+
+ htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
+ vm->hsync_len;
+ vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
+ vm->vsync_len;
+ fbmode->refresh = (vm->pixelclock * 1000) / (htotal * vtotal);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
+#endif
+
+
#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..920cbe3 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,7 @@
#include <linux/backlight.h>
#include <linux/slab.h>
#include <asm/io.h>
+#include <linux/videomode.h>
struct vm_area_struct;
struct fb_info;
@@ -714,6 +715,11 @@ 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);
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int fb_videomode_from_videomode(const struct videomode *vm,
+ struct fb_videomode *fbmode);
+#endif
+
/* drivers/video/modedb.c */
#define VESA_MODEDB_SIZE 34
extern void fb_var_to_videomode(struct fb_videomode *mode,
--
1.7.10.4
^ permalink raw reply related
* [PATCH v12 2/6] video: add of helper for videomode
From: Steffen Trumtrar @ 2012-11-20 15:54 UTC (permalink / raw)
To: devicetree-discuss
Cc: Steffen Trumtrar, Philipp Zabel, Rob Herring, linux-fbdev,
dri-devel, Laurent Pinchart, Thierry Reding,
Guennady Liakhovetski, linux-media, Tomi Valkeinen,
Stephen Warren, kernel, Florian Tobias Schandinat, David Airlie
In-Reply-To: <1353426896-6045-1-git-send-email-s.trumtrar@pengutronix.de>
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>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../devicetree/bindings/video/display-timings.txt | 107 ++++++++++
drivers/video/Kconfig | 13 ++
drivers/video/Makefile | 2 +
drivers/video/of_display_timing.c | 216 ++++++++++++++++++++
drivers/video/of_videomode.c | 48 +++++
include/linux/of_display_timings.h | 20 ++
include/linux/of_videomode.h | 18 ++
7 files changed, 424 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..a05cade
--- /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: display clock 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..807fedd 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,19 @@ config DISPLAY_TIMING
config VIDEOMODE
bool
+config OF_DISPLAY_TIMING
+ bool "Enable OF display timing support"
+ select DISPLAY_TIMING
+ help
+ helper to parse display timings from the devicetree
+
+config OF_VIDEOMODE
+ bool "Enable OF videomode support"
+ 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..3eb731f
--- /dev/null
+++ b/drivers/video/of_display_timing.c
@@ -0,0 +1,216 @@
+/*
+ * 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(const struct device_node *np, const 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(const 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__);
+ kfree(dt);
+ 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(const 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) {
+ pr_err("%s: could not allocate struct disp'\n", __func__);
+ goto dispfail;
+ }
+
+ 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 there is no child, it is useless to go on */
+ if (!entry) {
+ pr_err("%s: no timing specifications given\n", __func__);
+ goto entryfail;
+ }
+
+ pr_info("%s: using %s as default timing\n", __func__, entry->name);
+
+ native_mode = entry;
+
+ disp->num_timings = of_get_child_count(timings_np);
+ if (disp->num_timings = 0) {
+ /* should never happen, as entry was already found above */
+ pr_err("%s: no timings specified\n", __func__);
+ goto entryfail;
+ }
+
+ disp->timings = kzalloc(sizeof(struct display_timing *) * disp->num_timings,
+ GFP_KERNEL);
+ if (!disp->timings) {
+ pr_err("%s: could not allocate timings array\n", __func__);
+ goto entryfail;
+ }
+
+ disp->num_timings = 0;
+ disp->native_mode = 0;
+
+ 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);
+ goto timingfail;
+ }
+
+ if (native_mode = entry)
+ disp->native_mode = disp->num_timings;
+
+ disp->timings[disp->num_timings] = dt;
+ disp->num_timings++;
+ }
+ of_node_put(timings_np);
+ of_node_put(native_mode);
+
+ 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__);
+ display_timings_release(disp);
+ return NULL;
+ }
+ return disp;
+
+timingfail:
+ if (native_mode)
+ of_node_put(native_mode);
+ display_timings_release(disp);
+entryfail:
+ if (disp)
+ kfree(disp);
+dispfail:
+ of_node_put(timings_np);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(of_get_display_timings);
+
+/**
+ * of_display_timings_exists - check if a display-timings node is provided
+ * @np: device_node with the timing
+ **/
+int of_display_timings_exists(const 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;
+
+ of_node_put(timings_np);
+ return 1;
+}
+EXPORT_SYMBOL_GPL(of_display_timings_exists);
diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c
new file mode 100644
index 0000000..c573f92
--- /dev/null
+++ b/drivers/video/of_videomode.c
@@ -0,0 +1,48 @@
+/*
+ * generic videomode helper
+ *
+ * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+#include <linux/of.h>
+#include <linux/of_display_timings.h>
+#include <linux/of_videomode.h>
+#include <linux/export.h>
+
+/**
+ * of_get_videomode - get the videomode #<index> from devicetree
+ * @np - devicenode with the display_timings
+ * @vm - set to return value
+ * @index - index into list of display_timings
+ * DESCRIPTION:
+ * Get a list of all display timings and put the one
+ * specified by index into *vm. This function should only be used, if
+ * only one videomode is to be retrieved. A driver that needs to work
+ * with multiple/all videomodes should work with
+ * of_get_display_timings instead.
+ **/
+int of_get_videomode(const struct device_node *np, struct videomode *vm,
+ int index)
+{
+ struct display_timings *disp;
+ int ret;
+
+ disp = of_get_display_timings(np);
+ if (!disp) {
+ pr_err("%s: no timings specified\n", __func__);
+ return -EINVAL;
+ }
+
+ if (index = OF_USE_NATIVE_MODE)
+ index = disp->native_mode;
+
+ ret = videomode_from_timing(disp, vm, index);
+ if (ret)
+ return ret;
+
+ display_timings_release(disp);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_videomode);
diff --git a/include/linux/of_display_timings.h b/include/linux/of_display_timings.h
new file mode 100644
index 0000000..2b4fa0a
--- /dev/null
+++ b/include/linux/of_display_timings.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * display timings of helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
+#define __LINUX_OF_DISPLAY_TIMINGS_H
+
+#include <linux/display_timing.h>
+#include <linux/of.h>
+
+#define OF_USE_NATIVE_MODE -1
+
+struct display_timings *of_get_display_timings(const struct device_node *np);
+int of_display_timings_exists(const struct device_node *np);
+
+#endif
diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
new file mode 100644
index 0000000..4de5fcc
--- /dev/null
+++ b/include/linux/of_videomode.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>
+ *
+ * videomode of-helpers
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_OF_VIDEOMODE_H
+#define __LINUX_OF_VIDEOMODE_H
+
+#include <linux/videomode.h>
+#include <linux/of.h>
+
+int of_get_videomode(const struct device_node *np, struct videomode *vm,
+ int index);
+
+#endif /* __LINUX_OF_VIDEOMODE_H */
--
1.7.10.4
^ permalink raw reply related
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;
as well as URLs for NNTP newsgroup(s).