* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 7:03 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120913070012.GC6180-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > >
> > > > However, I fear these board specific things may be quite a bit anything,
> > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > example, there could be an FPGA on the board which requires some
> > > > configuration to accomplish the task at hand. It could be rather
> > > > difficult to handle it with a generic power sequence.
> > >
> > > Right. Note that this framework is supposed to be extended - I would like to
> > > at least add regulator voltage setting, and maybe even support for clocks and
> > > pinmux (but that might be out of place).
> >
> > Yes, that's one concern of mine... I already can imagine someone
> > suggesting adding conditionals to the power sequence data. Perhaps also
> > direct memory read/writes so you can twiddle registers directly. And so
> > on. Where's the limit what it should contain? Can we soon write full
> > drivers with the DT data? =)
>
> I have this concern aswell, that's why I'm sceptical about this patch
> set. But what are the alternatives? Adding power code to the drivers and
> thus adding board specific code to them is backwards.
As was pointed out in earlier posts in this thread, these are almost
always device specific, not board specific.
Do you have examples of board specific power sequences or such?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 7:08 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1347519249.7471.42.camel@lappyti>
On Thursday 13 September 2012 14:54:09 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
>
> > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> >
> >
> >
> > > However, I fear these board specific things may be quite a bit
> > > anything,
> > > so it may well be pwm, gpios and regulators are not enough for them.
> > > For
> > > example, there could be an FPGA on the board which requires some
> > > configuration to accomplish the task at hand. It could be rather
> > > difficult to handle it with a generic power sequence.
> >
> >
> > Right. Note that this framework is supposed to be extended - I would like
> > to at least add regulator voltage setting, and maybe even support for
> > clocks and pinmux (but that might be out of place).
>
>
> Yes, that's one concern of mine... I already can imagine someone
> suggesting adding conditionals to the power sequence data.
I took care of that when naming the feature - it is not a "sequence" anymore
if you have conditionals. :P
> Perhaps also
> direct memory read/writes so you can twiddle registers directly. And so
> on. Where's the limit what it should contain? Can we soon write full
> drivers with the DT data? =)
I shall be satisfied the day the kernel is released as one big DT node along
with the 5KB interpreter that runs it.
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Sascha Hauer @ 2012-09-13 7:18 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Alex Courbot, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1347519807.7471.45.camel@lappyti>
On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > >
> > > > > However, I fear these board specific things may be quite a bit anything,
> > > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > > example, there could be an FPGA on the board which requires some
> > > > > configuration to accomplish the task at hand. It could be rather
> > > > > difficult to handle it with a generic power sequence.
> > > >
> > > > Right. Note that this framework is supposed to be extended - I would like to
> > > > at least add regulator voltage setting, and maybe even support for clocks and
> > > > pinmux (but that might be out of place).
> > >
> > > Yes, that's one concern of mine... I already can imagine someone
> > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > direct memory read/writes so you can twiddle registers directly.
These memory writes can be avoided when these registers are abstracted
as a regular gpio/regulator/pwm driver.
> > > And so
> > > on. Where's the limit what it should contain? Can we soon write full
> > > drivers with the DT data? =)
> >
> > I have this concern aswell, that's why I'm sceptical about this patch
> > set. But what are the alternatives? Adding power code to the drivers and
> > thus adding board specific code to them is backwards.
>
> As was pointed out in earlier posts in this thread, these are almost
> always device specific, not board specific.
>
> Do you have examples of board specific power sequences or such?
Sure, tons of. One board needs a gpio to be set high to enable backlight,
the next one to low, a regulator has to be enabled, and to avoid
flickering a certain timing has to be ensured. This is all highly board
specific.
Sascha
--
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 v6 0/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13 7:19 UTC (permalink / raw)
To: Alex Courbot
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov, Tomi Valkeinen,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <4473898.CeAQBgUhKL@percival>
On Thu, Sep 13, 2012 at 03:42:11PM +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> > It would be sensible to make sure that the framework is done in such a
> > way that drivers can use it - there will be drivers (perhaps not display
> > ones) that have a known power sequence and which could benefit from the
> > ability to use library code to implement it based on the user simply
> > supplying named resources.
> Not sure I understand what you mean, but things should be working this way
> already - regulators and PWMs are acquired by name using the standard
> regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way to be
> referenced by name so their number is used instead.
Right, but the sequencing for enabling them is currently open coded in
each driver.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 7:21 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Sascha Hauer, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
linux-pm@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <1347519807.7471.45.camel@lappyti>
On Thursday 13 September 2012 15:03:27 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
>
> > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> >
> > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > >
> > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > >
> > > >
> > > >
> > > > > However, I fear these board specific things may be quite a bit
> > > > > anything,
> > > > > so it may well be pwm, gpios and regulators are not enough for them.
> > > > > For
> > > > > example, there could be an FPGA on the board which requires some
> > > > > configuration to accomplish the task at hand. It could be rather
> > > > > difficult to handle it with a generic power sequence.
> > > >
> > > >
> > > > Right. Note that this framework is supposed to be extended - I would
> > > > like to
at least add regulator voltage setting, and maybe even
> > > > support for clocks and pinmux (but that might be out of place).
> > >
> > >
> > > Yes, that's one concern of mine... I already can imagine someone
> > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > direct memory read/writes so you can twiddle registers directly. And so
> > > on. Where's the limit what it should contain? Can we soon write full
> > > drivers with the DT data? =)
> >
> >
> > I have this concern aswell, that's why I'm sceptical about this patch
> > set. But what are the alternatives? Adding power code to the drivers and
> > thus adding board specific code to them is backwards.
>
>
> As was pointed out in earlier posts in this thread, these are almost
> always device specific, not board specific.
I think the confusion comes from the fact that in practice, people just wrote
their hooks into the board files instead of writing more "specialized" drivers
(which I agree would have been the correct way of doing). That is why hooks
like those of the pwm_backlight driver were "board specific code" to me too.
Alex.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 7:26 UTC (permalink / raw)
To: Mark Brown
Cc: Tomi Valkeinen, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <20120913071928.GA20959@opensource.wolfsonmicro.com>
On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
> On Thu, Sep 13, 2012 at 03:42:11PM +0900, Alex Courbot wrote:
> > On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> > > It would be sensible to make sure that the framework is done in such a
> > > way that drivers can use it - there will be drivers (perhaps not display
> > > ones) that have a known power sequence and which could benefit from the
> > > ability to use library code to implement it based on the user simply
> > > supplying named resources.
> >
> > Not sure I understand what you mean, but things should be working this way
> > already - regulators and PWMs are acquired by name using the standard
> > regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way
> > to be referenced by name so their number is used instead.
>
> Right, but the sequencing for enabling them is currently open coded in
> each driver.
Mmm then I'm afraid I don't see what you wanted to say initially - could you
elaborate?
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 7:27 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120913071829.GE6180-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]
On Thu, 2012-09-13 at 09:18 +0200, Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > > >
> > > > > > However, I fear these board specific things may be quite a bit anything,
> > > > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > > > example, there could be an FPGA on the board which requires some
> > > > > > configuration to accomplish the task at hand. It could be rather
> > > > > > difficult to handle it with a generic power sequence.
> > > > >
> > > > > Right. Note that this framework is supposed to be extended - I would like to
> > > > > at least add regulator voltage setting, and maybe even support for clocks and
> > > > > pinmux (but that might be out of place).
> > > >
> > > > Yes, that's one concern of mine... I already can imagine someone
> > > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > > direct memory read/writes so you can twiddle registers directly.
>
> These memory writes can be avoided when these registers are abstracted
> as a regular gpio/regulator/pwm driver.
Only if they are gpios/regulators/pwms. Yes, I agree most of the
possible things to configure would be among those (or perhaps
pinmuxing). But there's always the odd one that's not one of those.
> > Do you have examples of board specific power sequences or such?
>
> Sure, tons of. One board needs a gpio to be set high to enable backlight,
> the next one to low, a regulator has to be enabled, and to avoid
> flickering a certain timing has to be ensured. This is all highly board
> specific.
Okay. In my experience these have always been device specific. In the
case of backlight, the backlight device requires one gpio to be set
high, other one low, etc.
Can you share a bit more what kind of HW configuration you have that
requires this? The backlight is not a single piece of HW added to the
board (or embedded into a panel module), but consists of multiple HW
blocks integrated in a custom way to the board?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-09-13 7:29 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala, Sascha Hauer, Mark Brown,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse
In-Reply-To: <1347519807.7471.45.camel@lappyti>
[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]
On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > >
> > > > > However, I fear these board specific things may be quite a bit anything,
> > > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > > example, there could be an FPGA on the board which requires some
> > > > > configuration to accomplish the task at hand. It could be rather
> > > > > difficult to handle it with a generic power sequence.
> > > >
> > > > Right. Note that this framework is supposed to be extended - I would like to
> > > > at least add regulator voltage setting, and maybe even support for clocks and
> > > > pinmux (but that might be out of place).
> > >
> > > Yes, that's one concern of mine... I already can imagine someone
> > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > direct memory read/writes so you can twiddle registers directly. And so
> > > on. Where's the limit what it should contain? Can we soon write full
> > > drivers with the DT data? =)
> >
> > I have this concern aswell, that's why I'm sceptical about this patch
> > set. But what are the alternatives? Adding power code to the drivers and
> > thus adding board specific code to them is backwards.
>
> As was pointed out in earlier posts in this thread, these are almost
> always device specific, not board specific.
>
> Do you have examples of board specific power sequences or such?
It is true that most (perhaps all) power sequences can be associated
with a specific device, but if we go and implement drivers for these
kinds of devices we will probably end up with loads of variations of
the same scheme.
Lets take display panels as an example. One of the devices that we build
has gone through two generations so far and both are slightly different
in how they control the panel backlight: one has an external backlight
controller, the other has the display controller built into the panel.
However, from the board's perspective the control of the backlight
doesn't change, because both devices get the same inputs (an enable pin
and a PWM) that map to the same pins on the SoC.
This may not be a very good example because the timing isn't relevant,
but the basic point is still valid: if we provide a driver for both
panel devices, the code will be exactly the same. So we end up having to
refactor to avoid code duplication and use the same driver for a number
of backlight/panel combinations. Which in itself isn't very bad, but it
also means that we'll probably get to see a large number of "generic"
drivers which aren't very generic after all.
Another problem, which also applies to the case of power-sequences, is
that often the panel and backlight are not the same device. So you could
have the same panel with any number of different backlight controllers
or vice-versa any number of different panels with the same backlight
controller.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13 7:29 UTC (permalink / raw)
To: Alex Courbot
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov, Tomi Valkeinen,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <1378218.yLeQheNFT0@percival>
On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
> > > On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> > > > It would be sensible to make sure that the framework is done in such a
> > > > way that drivers can use it - there will be drivers (perhaps not display
> > > > ones) that have a known power sequence and which could benefit from the
> > > > ability to use library code to implement it based on the user simply
> > > > supplying named resources.
> > > Not sure I understand what you mean, but things should be working this way
> > > already - regulators and PWMs are acquired by name using the standard
> > > regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way
> > > to be referenced by name so their number is used instead.
> > Right, but the sequencing for enabling them is currently open coded in
> > each driver.
> Mmm then I'm afraid I don't see what you wanted to say initially - could you
> elaborate?
The driver knows the power sequence. Having to type the same sequence
into the DT or platform data for each board using the device wouuld be
retarded so we need the drivers to be able to give the sequence to the
library if they're going to be able to reuse it (which is a lot of what
Tomi is talking about).
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Sascha Hauer @ 2012-09-13 7:50 UTC (permalink / raw)
To: Thierry Reding
Cc: Tomi Valkeinen, Alex Courbot, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
linux-pm@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20120913072920.GA11459@avionic-0098.mockup.avionic-design.de>
On Thu, Sep 13, 2012 at 09:29:20AM +0200, Thierry Reding wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > > >
> > > > > > However, I fear these board specific things may be quite a bit anything,
> > > > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > > > example, there could be an FPGA on the board which requires some
> > > > > > configuration to accomplish the task at hand. It could be rather
> > > > > > difficult to handle it with a generic power sequence.
> > > > >
> > > > > Right. Note that this framework is supposed to be extended - I would like to
> > > > > at least add regulator voltage setting, and maybe even support for clocks and
> > > > > pinmux (but that might be out of place).
> > > >
> > > > Yes, that's one concern of mine... I already can imagine someone
> > > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > > direct memory read/writes so you can twiddle registers directly. And so
> > > > on. Where's the limit what it should contain? Can we soon write full
> > > > drivers with the DT data? =)
> > >
> > > I have this concern aswell, that's why I'm sceptical about this patch
> > > set. But what are the alternatives? Adding power code to the drivers and
> > > thus adding board specific code to them is backwards.
> >
> > As was pointed out in earlier posts in this thread, these are almost
> > always device specific, not board specific.
> >
> > Do you have examples of board specific power sequences or such?
>
> It is true that most (perhaps all) power sequences can be associated
> with a specific device, but if we go and implement drivers for these
> kinds of devices we will probably end up with loads of variations of
> the same scheme.
>
> Lets take display panels as an example. One of the devices that we build
> has gone through two generations so far and both are slightly different
> in how they control the panel backlight: one has an external backlight
> controller, the other has the display controller built into the panel.
> However, from the board's perspective the control of the backlight
> doesn't change, because both devices get the same inputs (an enable pin
> and a PWM) that map to the same pins on the SoC.
>
> This may not be a very good example because the timing isn't relevant,
> but the basic point is still valid: if we provide a driver for both
> panel devices, the code will be exactly the same. So we end up having to
> refactor to avoid code duplication and use the same driver for a number
> of backlight/panel combinations. Which in itself isn't very bad, but it
> also means that we'll probably get to see a large number of "generic"
> drivers which aren't very generic after all.
>
> Another problem, which also applies to the case of power-sequences, is
> that often the panel and backlight are not the same device.
Maybe that is the problem that needs to be addressed? They *are* not the
same device, still they are handled in a single platform callback (or
now power sequence). Maybe the amount of combinations dastrically go
down if we really make them two devices.
Most of our panels have:
- A regulator (or gpio) for turning them on
And the backlights have:
- A regulator (or gpio) for turning them on
- A PWM for controlling brightness.
The power sequence for the above is clear: Turn on the panel the panel,
wait until it stabilized and afterwards turn on the backlight.
Sascha
--
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 v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 8:00 UTC (permalink / raw)
To: Thierry Reding
Cc: Sascha Hauer, Alex Courbot, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20120913072920.GA11459-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]
On Thu, 2012-09-13 at 09:29 +0200, Thierry Reding wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > > >
> > > > > > However, I fear these board specific things may be quite a bit anything,
> > > > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > > > example, there could be an FPGA on the board which requires some
> > > > > > configuration to accomplish the task at hand. It could be rather
> > > > > > difficult to handle it with a generic power sequence.
> > > > >
> > > > > Right. Note that this framework is supposed to be extended - I would like to
> > > > > at least add regulator voltage setting, and maybe even support for clocks and
> > > > > pinmux (but that might be out of place).
> > > >
> > > > Yes, that's one concern of mine... I already can imagine someone
> > > > suggesting adding conditionals to the power sequence data. Perhaps also
> > > > direct memory read/writes so you can twiddle registers directly. And so
> > > > on. Where's the limit what it should contain? Can we soon write full
> > > > drivers with the DT data? =)
> > >
> > > I have this concern aswell, that's why I'm sceptical about this patch
> > > set. But what are the alternatives? Adding power code to the drivers and
> > > thus adding board specific code to them is backwards.
> >
> > As was pointed out in earlier posts in this thread, these are almost
> > always device specific, not board specific.
> >
> > Do you have examples of board specific power sequences or such?
>
> It is true that most (perhaps all) power sequences can be associated
> with a specific device, but if we go and implement drivers for these
> kinds of devices we will probably end up with loads of variations of
> the same scheme.
>
> Lets take display panels as an example. One of the devices that we build
> has gone through two generations so far and both are slightly different
> in how they control the panel backlight: one has an external backlight
> controller, the other has the display controller built into the panel.
> However, from the board's perspective the control of the backlight
> doesn't change, because both devices get the same inputs (an enable pin
> and a PWM) that map to the same pins on the SoC.
We had something a bit similar in Nokia. First versions had an
"independent" backlight controlled via pwm. Later versions had a
backlight that is controlled by the panel IP, so it was changed by
sending DSI commands to the panel.
> This may not be a very good example because the timing isn't relevant,
> but the basic point is still valid: if we provide a driver for both
> panel devices, the code will be exactly the same. So we end up having to
> refactor to avoid code duplication and use the same driver for a number
> of backlight/panel combinations. Which in itself isn't very bad, but it
> also means that we'll probably get to see a large number of "generic"
> drivers which aren't very generic after all.
>
> Another problem, which also applies to the case of power-sequences, is
> that often the panel and backlight are not the same device. So you could
> have the same panel with any number of different backlight controllers
> or vice-versa any number of different panels with the same backlight
> controller.
Yes, I think the backlight and the panel should be considered separate
devices. Just like, say, a touch screen and a panel may happen to be in
the same display module, a backlight and a panel can be in the same
display module. They are still separate, independent things, although
they are, of course, used together.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13 8:09 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Anton Vorontsov, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra, linux-kernel, linux-fbdev,
devicetree-discuss, linux-pm, linux-doc
In-Reply-To: <1347443867-18868-2-git-send-email-acourbot@nvidia.com>
On Wed, Sep 12, 2012 at 06:57:44PM +0900, Alexandre Courbot wrote:
> Some device drivers (panel backlights especially) need to follow precise
> sequences for powering on and off, involving gpios, regulators, PWMs
> with a precise powering order and delays to respect between each steps.
> These sequences are board-specific, and do not belong to a particular
> driver - therefore they have been performed by board-specific hook
> functions to far.
It does make me a little sad that the DT bindings need to specify the
number of steps but otherwise this looks good (modulo the minor comments
Stephen had as well):
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
I think regardless of the current discussion about some of the
applications (like pwm-backlight) there are going to be cases where this
is useful even if it ends up being more as library code for drivers than
as something that users work with directly.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 8:21 UTC (permalink / raw)
To: Sascha Hauer
Cc: Thierry Reding, Tomi Valkeinen, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
linux-pm@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20120913075037.GG6180@pengutronix.de>
On Thursday 13 September 2012 15:50:37 Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 09:29:20AM +0200, Thierry Reding wrote:
> > On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > > On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > > > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > > > > > However, I fear these board specific things may be quite a bit
> > > > > > > anything,
> > > > > > > so it may well be pwm, gpios and regulators are not enough for
> > > > > > > them. For
> > > > > > > example, there could be an FPGA on the board which requires some
> > > > > > > configuration to accomplish the task at hand. It could be rather
> > > > > > > difficult to handle it with a generic power sequence.
> > > > > >
> > > > > > Right. Note that this framework is supposed to be extended - I
> > > > > > would like to at least add regulator voltage setting, and maybe
> > > > > > even support for clocks and pinmux (but that might be out of
> > > > > > place).
> > > > >
> > > > > Yes, that's one concern of mine... I already can imagine someone
> > > > > suggesting adding conditionals to the power sequence data. Perhaps
> > > > > also
> > > > > direct memory read/writes so you can twiddle registers directly. And
> > > > > so
> > > > > on. Where's the limit what it should contain? Can we soon write full
> > > > > drivers with the DT data? =)
> > > >
> > > > I have this concern aswell, that's why I'm sceptical about this patch
> > > > set. But what are the alternatives? Adding power code to the drivers
> > > > and
> > > > thus adding board specific code to them is backwards.
> > >
> > > As was pointed out in earlier posts in this thread, these are almost
> > > always device specific, not board specific.
> > >
> > > Do you have examples of board specific power sequences or such?
> >
> > It is true that most (perhaps all) power sequences can be associated
> > with a specific device, but if we go and implement drivers for these
> > kinds of devices we will probably end up with loads of variations of
> > the same scheme.
> >
> > Lets take display panels as an example. One of the devices that we build
> > has gone through two generations so far and both are slightly different
> > in how they control the panel backlight: one has an external backlight
> > controller, the other has the display controller built into the panel.
> > However, from the board's perspective the control of the backlight
> > doesn't change, because both devices get the same inputs (an enable pin
> > and a PWM) that map to the same pins on the SoC.
> >
> > This may not be a very good example because the timing isn't relevant,
> > but the basic point is still valid: if we provide a driver for both
> > panel devices, the code will be exactly the same. So we end up having to
> > refactor to avoid code duplication and use the same driver for a number
> > of backlight/panel combinations. Which in itself isn't very bad, but it
> > also means that we'll probably get to see a large number of "generic"
> > drivers which aren't very generic after all.
> >
> > Another problem, which also applies to the case of power-sequences, is
> > that often the panel and backlight are not the same device.
>
> Maybe that is the problem that needs to be addressed? They *are* not the
> same device, still they are handled in a single platform callback (or
> now power sequence). Maybe the amount of combinations dastrically go
> down if we really make them two devices.
>
> Most of our panels have:
>
> - A regulator (or gpio) for turning them on
>
> And the backlights have:
>
> - A regulator (or gpio) for turning them on
> - A PWM for controlling brightness.
>
> The power sequence for the above is clear: Turn on the panel the panel,
> wait until it stabilized and afterwards turn on the backlight.
Actually the sequence I submitted in this patchset only takes care of the
backlight device (the panel - or LCD - should have its own). The regulator
controls the power supply, the PWM the intensity, and on top of that it also
has an enable GPIO. These 3 resources are exclusively for the LED - the LCD
uses other ones. So as of now it seems that the LCD/backlight separation is
effective and the resources needed are not so uniform across backlights (not
even mentioning the delays).
The LCD's power sequence is even weirder - VDD must take at least 0.5ms for
going from 10% to 90% of its power, you must wait 400ms after switching it off
before switching it on again, and you should also transmit data for 200ms
before switching the backlight's LED on (using its own sequence). That last
point is interesting since it somehow makes the LCD and LED dependent on each
other - on an unrelated note, this might be something to consider in Laurent's
proposal for a panel framework.
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-09-13 8:26 UTC (permalink / raw)
To: Alex Courbot
Cc: Sascha Hauer, Tomi Valkeinen, Stephen Warren, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
linux-pm@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <1469388.zjfDz88nxK@percival>
[-- Attachment #1: Type: text/plain, Size: 5481 bytes --]
On Thu, Sep 13, 2012 at 05:21:10PM +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 15:50:37 Sascha Hauer wrote:
> > On Thu, Sep 13, 2012 at 09:29:20AM +0200, Thierry Reding wrote:
> > > On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > > > On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> > > > > On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > > > > > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > > > > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > > > > > > > However, I fear these board specific things may be quite a bit
> > > > > > > > anything,
> > > > > > > > so it may well be pwm, gpios and regulators are not enough for
> > > > > > > > them. For
> > > > > > > > example, there could be an FPGA on the board which requires some
> > > > > > > > configuration to accomplish the task at hand. It could be rather
> > > > > > > > difficult to handle it with a generic power sequence.
> > > > > > >
> > > > > > > Right. Note that this framework is supposed to be extended - I
> > > > > > > would like to at least add regulator voltage setting, and maybe
> > > > > > > even support for clocks and pinmux (but that might be out of
> > > > > > > place).
> > > > > >
> > > > > > Yes, that's one concern of mine... I already can imagine someone
> > > > > > suggesting adding conditionals to the power sequence data. Perhaps
> > > > > > also
> > > > > > direct memory read/writes so you can twiddle registers directly. And
> > > > > > so
> > > > > > on. Where's the limit what it should contain? Can we soon write full
> > > > > > drivers with the DT data? =)
> > > > >
> > > > > I have this concern aswell, that's why I'm sceptical about this patch
> > > > > set. But what are the alternatives? Adding power code to the drivers
> > > > > and
> > > > > thus adding board specific code to them is backwards.
> > > >
> > > > As was pointed out in earlier posts in this thread, these are almost
> > > > always device specific, not board specific.
> > > >
> > > > Do you have examples of board specific power sequences or such?
> > >
> > > It is true that most (perhaps all) power sequences can be associated
> > > with a specific device, but if we go and implement drivers for these
> > > kinds of devices we will probably end up with loads of variations of
> > > the same scheme.
> > >
> > > Lets take display panels as an example. One of the devices that we build
> > > has gone through two generations so far and both are slightly different
> > > in how they control the panel backlight: one has an external backlight
> > > controller, the other has the display controller built into the panel.
> > > However, from the board's perspective the control of the backlight
> > > doesn't change, because both devices get the same inputs (an enable pin
> > > and a PWM) that map to the same pins on the SoC.
> > >
> > > This may not be a very good example because the timing isn't relevant,
> > > but the basic point is still valid: if we provide a driver for both
> > > panel devices, the code will be exactly the same. So we end up having to
> > > refactor to avoid code duplication and use the same driver for a number
> > > of backlight/panel combinations. Which in itself isn't very bad, but it
> > > also means that we'll probably get to see a large number of "generic"
> > > drivers which aren't very generic after all.
> > >
> > > Another problem, which also applies to the case of power-sequences, is
> > > that often the panel and backlight are not the same device.
> >
> > Maybe that is the problem that needs to be addressed? They *are* not the
> > same device, still they are handled in a single platform callback (or
> > now power sequence). Maybe the amount of combinations dastrically go
> > down if we really make them two devices.
> >
> > Most of our panels have:
> >
> > - A regulator (or gpio) for turning them on
> >
> > And the backlights have:
> >
> > - A regulator (or gpio) for turning them on
> > - A PWM for controlling brightness.
> >
> > The power sequence for the above is clear: Turn on the panel the panel,
> > wait until it stabilized and afterwards turn on the backlight.
>
> Actually the sequence I submitted in this patchset only takes care of the
> backlight device (the panel - or LCD - should have its own). The regulator
> controls the power supply, the PWM the intensity, and on top of that it also
> has an enable GPIO. These 3 resources are exclusively for the LED - the LCD
> uses other ones. So as of now it seems that the LCD/backlight separation is
> effective and the resources needed are not so uniform across backlights (not
> even mentioning the delays).
>
> The LCD's power sequence is even weirder - VDD must take at least 0.5ms for
> going from 10% to 90% of its power, you must wait 400ms after switching it off
> before switching it on again, and you should also transmit data for 200ms
> before switching the backlight's LED on (using its own sequence). That last
> point is interesting since it somehow makes the LCD and LED dependent on each
> other - on an unrelated note, this might be something to consider in Laurent's
> proposal for a panel framework.
Maybe this could be solved by adding a backlight resource type and
embedding a reference to the backlight within the panel's power
sequence?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-09-13 8:32 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala, Sascha Hauer, Mark Brown,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse
In-Reply-To: <1347523218.3014.4.camel@deskari>
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
On Thu, Sep 13, 2012 at 11:00:18AM +0300, Tomi Valkeinen wrote:
> Yes, I think the backlight and the panel should be considered separate
> devices. Just like, say, a touch screen and a panel may happen to be in
> the same display module, a backlight and a panel can be in the same
> display module. They are still separate, independent things, although
> they are, of course, used together.
Still, as Alex mentioned, there may be some dependency between the panel
and the backlight, so we may need to have some kind of connection. I
haven't had much time to look at the panel subsystem, but perhaps it
should provide for this.
I obviously don't know every panel and backlight combination out there,
but isn't the dependency more of a usability nature. What I mean is that
the panel will work even if you switch the backlight on immediately,
just that the panel might take some time to properly display data and
therefore the backlight should remain powered off so the user doesn't
see garbage. Or are there really any functional dependencies?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Tomi Valkeinen @ 2012-09-13 10:54 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1341388595-30672-1-git-send-email-s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
On Wed, 2012-07-04 at 09:56 +0200, Sascha Hauer wrote:
> This patch adds a helper function for parsing videomodes from the devicetree.
> The videomode can be either converted to a struct drm_display_mode or a
> struct fb_videomode.
I have more or less the same generic comment for this as for the power
sequences series discussed: this would add panel specific information
into DT data, instead of the driver handling it. But, as also discussed
in the thread, there are differing opinions on which way is better.
> +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> + struct fb_videomode *fbmode);
From caller's point of view I think it'd make more sense to have
separate functions to get drm mode or fb mode. I don't think there's a
case where the caller would want both.
Then again, even better would be to have a common video mode struct used
by both fb and drm... But I think that's been on the todo list of
Laurent for a long time already =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Sascha Hauer @ 2012-09-13 11:19 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1347533647.3014.10.camel@deskari>
On Thu, Sep 13, 2012 at 01:54:07PM +0300, Tomi Valkeinen wrote:
> On Wed, 2012-07-04 at 09:56 +0200, Sascha Hauer wrote:
> > This patch adds a helper function for parsing videomodes from the devicetree.
> > The videomode can be either converted to a struct drm_display_mode or a
> > struct fb_videomode.
>
> I have more or less the same generic comment for this as for the power
> sequences series discussed: this would add panel specific information
> into DT data, instead of the driver handling it. But, as also discussed
> in the thread, there are differing opinions on which way is better.
With the panel timings I think the approach of moving them into DT is
the best we can do. There are so many displays out there, patching the
kernel each time a customer comes with some new display is no fun.
>
> > +int of_get_video_mode(struct device_node *np, struct drm_display_mode *dmode,
> > + struct fb_videomode *fbmode);
>
> From caller's point of view I think it'd make more sense to have
> separate functions to get drm mode or fb mode. I don't think there's a
> case where the caller would want both.
Ok, that makes sense.
>
> Then again, even better would be to have a common video mode struct used
> by both fb and drm... But I think that's been on the todo list of
> Laurent for a long time already =).
Yes, indeed. We should go into that direction. We already realized that
we want to have ranges instead of fixed values for the timings.
Sascha
--
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: [RFC 1/5] video: Add generic display panel core
From: Sascha Hauer @ 2012-09-13 11:29 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Bryan Wu,
Richard Purdie, Tomi Valkeinen, Marcus Lorentzon, Sumit Semwal,
Archit Taneja, Sebastien Guiriec, Inki Dae, Kyungmin Park
In-Reply-To: <224585745.5E32B1Gv1v@avalon>
On Thu, Sep 13, 2012 at 03:40:40AM +0200, Laurent Pinchart wrote:
> Hi Sascha,
>
> > > +int panel_get_modes(struct panel *panel, const struct fb_videomode
> > > **modes)
> > > +{
> > > + if (!panel->ops || !panel->ops->get_modes)
> > > + return 0;
> > > +
> > > + return panel->ops->get_modes(panel, modes);
> > > +}
> > > +EXPORT_SYMBOL_GPL(panel_get_modes);
> >
> > You have seen my of videomode helper proposal. One result there was that
> > we want to have ranges for the margin/synclen fields. Does it make sense
> > to base this new panel framework on a more sophisticated internal
> > reprentation of the panel parameters?
>
> I think it does, yes. We need a common video mode structure, and the panel
> framework should use it. I'll try to rebase my patches on top of your
> proposal. Have you posted the latest version ?
V2 is the newest version. I'd like to implement ranges for the display
timings which then makes for a new common video mode structure, which
then could be used by drm and fbdev, probably with helper functions to
convert from common videomode to drm/fbdev specific variants.
>
> > This could then be converted to struct fb_videomode / struct
> > drm_display_mode as needed. This would also make it more suitable for drm
> > drivers which are not interested in struct fb_videomode.
> >
> > Related to this I suggest to change the API to be able to iterate over
> > the different modes, like:
> >
> > struct videomode *panel_get_mode(struct panel *panel, int num);
> >
> > This was we do not have to have an array of modes in memory, which may
> > be a burden for some panel drivers.
>
> I currently have mixed feelings about this. Both approaches have pros and
> cons. Iterating over the modes would be more complex for drivers that use
> panels, and would be race-prone if the modes can change at runtime (OK, this
> isn't supported by the current panel API proposal).
I just remember that the array approach was painful when I worked on an
fbdev driver some time ago. There some possible modes came from platform_data,
other modes were default modes in the driver and all had to be merged
into a single array. I don't remember the situation exactly, but it
would have been simpler if it had been a list instead of an array.
Sascha
--
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
* [PATCH 00/21] OMAPDSS: DISPC changes for writeback pipeline
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
DSS HW on OMAP4 onwards supports a new pipeline called writeback. Unlike other
pipelines(called overlays in OMAPDSS), writeback takes pixel data from an
overlay output or a overlay manager output and writes it back into a specified
address in memory.
writeback pipeline allows us to take benefit of the hardware processing
available inside the DISPC like color space conversion, rescaling, compositing
etc and do either a) perform memory-to-memory transfer with data processing,
b) capture a displayed frame. The former is known as memory to memory mode of
the writeback pipeline, and the latter is known as capture mode. More details
about writeback can be found in the Display Subsystem section of the OMAP4/5 TRMs.
witeback has properties of both overlays and overlay managers. It is like an
overlay as it has programmable base addresses and contains blocks like scalar,
color conversion unit, truncation unit, DISPC DMA FIFO. It is like a manager as
enabling it immediately starts transfer to the memory, and it has a GO bit to use
a new writeback configuration.
This series prepares the low level DISPC driver(dispc.c) to configure writeback
registers. The aim is to reuse most of the code as most of its registers are
like overlay or manager registers, and are configured in the same way in most
cases. The first few patches rename dispc_ovl_* functions to dispc_plane_*
functions. The next few patches change how overlay caps are used within the
dispc functions, this helps reusing more functions between overlays and
writeback. The patches in the end add writeback register offsets, and make
changes in the code where writeback behaves differently than
The changes are made only keeping writeback mem to mem support in mind. There
would be a few changes required when capture mode is added, but those are
minimal.
Reference branch:
git://gitorious.org/~boddob/linux-omap-dss2/archit-dss2-clone.git 1-writeback-dispc
Archit Taneja (21):
OMAPDSS: DISPC: Constify omap_overlay_info in dispc_ovl_setup()
OMAPDSS: DISPC: Rename scalar related functions from dispc_ovl_* to
dispc_plane_*
OMAPDSS: DISPC: Rename fifo/burst related functions from dispc_ovl_*
to dispc_plane_*
OMAPDSS: DISPC: Rename misc functions from dispc_ovl_* to
dispc_plane_*
OMAPDSS: DISPC: Simplify function names for setting pipeline input
and output sizes
OMAPDSS: DISPC: Pass overlay caps as a parameter to dispc plane
functions
OMAPDSS: OVERLAY: Add position and replication as overlay caps
OMAPDSS: DISPC: Make dispc_ovl_setup call dispc_plane_setup
OMAPDSS: DISPC: Calculate scaling limits in a more generic way
OMAPDSS: DISPC: Allow both upscaling and downscaling of chroma
OMAPDSS: DISPC: Add writeback register offsets and dss features
structs
OMAPDSS: DISPC: Configure input and output sizes for writeback
OMAPDSS: DISPC: Pass dummy scalar output rates for writeback pipeline
OMAPDSS: DISPC: Downscale chroma if plane is writeback
OMAPDSS: DISPC: Don't set chroma resampling bit for writeback
OMAPDSS: DISPC: Add function to set channel in for writeback
OMAPDSS: DISPC: Configure overlay-like parameters in dispc_wb_setup
OMAPDSS: DISPC: Configure writeback specific parameters in
dispc_wb_setup()
OMAPDSS: DISPC: Configure writeback FIFOs
OMAPDSS: DISPC: Add manager like functions for writeback
OMAPDSS: DISPC: Configure color conversion coefficients for writeback
drivers/video/omap2/dss/apply.c | 4 +-
drivers/video/omap2/dss/dispc.c | 659 +++++++++++++++++++++-----------
drivers/video/omap2/dss/dispc.h | 35 +-
drivers/video/omap2/dss/dispc_coefs.c | 2 +-
drivers/video/omap2/dss/dss.h | 26 +-
drivers/video/omap2/dss/dss_features.c | 57 ++-
drivers/video/omap2/dss/dss_features.h | 1 +
include/video/omapdss.h | 15 +
8 files changed, 564 insertions(+), 235 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH 01/21] OMAPDSS: DISPC: Constify omap_overlay_info in dispc_ovl_setup()
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
The struct omap_overlay_info passed to dispc_ovl_setup() is used to configure
DISPC registers. It shouldn't modify the overlay_info structure. The pos_y field
was being changed in dispc_ovl_setup in the case of interlaced displays. Fix
this and const qualifier to the omap_overlay_info argument.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 9 +++++----
drivers/video/omap2/dss/dss.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 33db882..cd3d532 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2253,7 +2253,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
return 0;
}
-int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
+int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
bool replication, const struct omap_video_timings *mgr_timings)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
@@ -2271,6 +2271,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
enum omap_channel channel;
int x_predecim = 1, y_predecim = 1;
bool ilace = mgr_timings->interlace;
+ u16 pos_y = oi->pos_y;
channel = dispc_ovl_get_channel_out(plane);
@@ -2293,12 +2294,12 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
if (ilace) {
if (fieldmode)
in_height /= 2;
- oi->pos_y /= 2;
+ pos_y /= 2;
out_height /= 2;
DSSDBG("adjusting for ilace: height %d, pos_y %d, "
"out_height %d\n",
- in_height, oi->pos_y, out_height);
+ in_height, pos_y, out_height);
}
if (!dss_feat_color_mode_supported(plane, oi->color_mode))
@@ -2381,7 +2382,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, in_width,
in_height, out_width, out_height);
- dispc_ovl_set_pos(plane, oi->pos_x, oi->pos_y);
+ dispc_ovl_set_pos(plane, oi->pos_x, pos_y);
dispc_ovl_set_pic_size(plane, in_width, in_height);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index c2bc092..43210b8 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -440,7 +440,7 @@ void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high);
void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
u32 *fifo_low, u32 *fifo_high, bool use_fifomerge,
bool manual_update);
-int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
+int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
bool replication, const struct omap_video_timings *mgr_timings);
int dispc_ovl_enable(enum omap_plane plane, bool enable);
void dispc_ovl_set_channel_out(enum omap_plane plane,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 02/21] OMAPDSS: DISPC: Rename scalar related functions from dispc_ovl_* to dispc_plane_*
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
Writeback pipeline has an identical scalar block as in video pipelines. Rename
the scalar related function from dispc_ovl_* to dispc_plane_*. The actual
registers are kept as DISPC_OVL_* only to prevent too much change.
All functions which are common to overlays and writeback are to be named as
dispc_plane_*, functions which are specific to overlays are to be named as
dispc_ovl_*, and writeback as dispc_wb_*.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 92 ++++++++++++++++++---------------
drivers/video/omap2/dss/dispc.h | 2 +-
drivers/video/omap2/dss/dispc_coefs.c | 2 +-
3 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index cd3d532..eae9da4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -560,29 +560,33 @@ void dispc_mgr_go(enum omap_channel channel)
mgr_fld_write(channel, DISPC_MGR_FLD_GO, 1);
}
-static void dispc_ovl_write_firh_reg(enum omap_plane plane, int reg, u32 value)
+static void dispc_plane_write_firh_reg(enum omap_plane plane, int reg,
+ u32 value)
{
dispc_write_reg(DISPC_OVL_FIR_COEF_H(plane, reg), value);
}
-static void dispc_ovl_write_firhv_reg(enum omap_plane plane, int reg, u32 value)
+static void dispc_plane_write_firhv_reg(enum omap_plane plane, int reg,
+ u32 value)
{
dispc_write_reg(DISPC_OVL_FIR_COEF_HV(plane, reg), value);
}
-static void dispc_ovl_write_firv_reg(enum omap_plane plane, int reg, u32 value)
+static void dispc_plane_write_firv_reg(enum omap_plane plane, int reg,
+ u32 value)
{
dispc_write_reg(DISPC_OVL_FIR_COEF_V(plane, reg), value);
}
-static void dispc_ovl_write_firh2_reg(enum omap_plane plane, int reg, u32 value)
+static void dispc_plane_write_firh2_reg(enum omap_plane plane, int reg,
+ u32 value)
{
BUG_ON(plane = OMAP_DSS_GFX);
dispc_write_reg(DISPC_OVL_FIR_COEF_H2(plane, reg), value);
}
-static void dispc_ovl_write_firhv2_reg(enum omap_plane plane, int reg,
+static void dispc_plane_write_firhv2_reg(enum omap_plane plane, int reg,
u32 value)
{
BUG_ON(plane = OMAP_DSS_GFX);
@@ -590,22 +594,23 @@ static void dispc_ovl_write_firhv2_reg(enum omap_plane plane, int reg,
dispc_write_reg(DISPC_OVL_FIR_COEF_HV2(plane, reg), value);
}
-static void dispc_ovl_write_firv2_reg(enum omap_plane plane, int reg, u32 value)
+static void dispc_plane_write_firv2_reg(enum omap_plane plane, int reg,
+ u32 value)
{
BUG_ON(plane = OMAP_DSS_GFX);
dispc_write_reg(DISPC_OVL_FIR_COEF_V2(plane, reg), value);
}
-static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
- int fir_vinc, int five_taps,
- enum omap_color_component color_comp)
+static void dispc_plane_set_scale_coef(enum omap_plane plane, int fir_hinc,
+ int fir_vinc, int five_taps,
+ enum omap_color_component color_comp)
{
const struct dispc_coef *h_coef, *v_coef;
int i;
- h_coef = dispc_ovl_get_scale_coef(fir_hinc, true);
- v_coef = dispc_ovl_get_scale_coef(fir_vinc, five_taps);
+ h_coef = dispc_plane_get_scale_coef(fir_hinc, true);
+ v_coef = dispc_plane_get_scale_coef(fir_vinc, five_taps);
for (i = 0; i < 8; i++) {
u32 h, hv;
@@ -620,11 +625,11 @@ static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
| FLD_VAL(v_coef[i].hc3_vc2, 31, 24);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y) {
- dispc_ovl_write_firh_reg(plane, i, h);
- dispc_ovl_write_firhv_reg(plane, i, hv);
+ dispc_plane_write_firh_reg(plane, i, h);
+ dispc_plane_write_firhv_reg(plane, i, hv);
} else {
- dispc_ovl_write_firh2_reg(plane, i, h);
- dispc_ovl_write_firhv2_reg(plane, i, hv);
+ dispc_plane_write_firh2_reg(plane, i, h);
+ dispc_plane_write_firhv2_reg(plane, i, hv);
}
}
@@ -635,9 +640,9 @@ static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
v = FLD_VAL(v_coef[i].hc0_vc00, 7, 0)
| FLD_VAL(v_coef[i].hc4_vc22, 15, 8);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y)
- dispc_ovl_write_firv_reg(plane, i, v);
+ dispc_plane_write_firv_reg(plane, i, v);
else
- dispc_ovl_write_firv2_reg(plane, i, v);
+ dispc_plane_write_firv2_reg(plane, i, v);
}
}
}
@@ -1208,9 +1213,8 @@ void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
}
}
-static void dispc_ovl_set_fir(enum omap_plane plane,
- int hinc, int vinc,
- enum omap_color_component color_comp)
+static void dispc_plane_set_fir(enum omap_plane plane, int hinc, int vinc,
+ enum omap_color_component color_comp)
{
u32 val;
@@ -1231,7 +1235,8 @@ static void dispc_ovl_set_fir(enum omap_plane plane,
}
}
-static void dispc_ovl_set_vid_accu0(enum omap_plane plane, int haccu, int vaccu)
+static void dispc_plane_set_vid_accu0(enum omap_plane plane, int haccu,
+ int vaccu)
{
u32 val;
u8 hor_start, hor_end, vert_start, vert_end;
@@ -1245,7 +1250,8 @@ static void dispc_ovl_set_vid_accu0(enum omap_plane plane, int haccu, int vaccu)
dispc_write_reg(DISPC_OVL_ACCU0(plane), val);
}
-static void dispc_ovl_set_vid_accu1(enum omap_plane plane, int haccu, int vaccu)
+static void dispc_plane_set_vid_accu1(enum omap_plane plane, int haccu,
+ int vaccu)
{
u32 val;
u8 hor_start, hor_end, vert_start, vert_end;
@@ -1259,7 +1265,7 @@ static void dispc_ovl_set_vid_accu1(enum omap_plane plane, int haccu, int vaccu)
dispc_write_reg(DISPC_OVL_ACCU1(plane), val);
}
-static void dispc_ovl_set_vid_accu2_0(enum omap_plane plane, int haccu,
+static void dispc_plane_set_vid_accu2_0(enum omap_plane plane, int haccu,
int vaccu)
{
u32 val;
@@ -1268,7 +1274,7 @@ static void dispc_ovl_set_vid_accu2_0(enum omap_plane plane, int haccu,
dispc_write_reg(DISPC_OVL_ACCU2_0(plane), val);
}
-static void dispc_ovl_set_vid_accu2_1(enum omap_plane plane, int haccu,
+static void dispc_plane_set_vid_accu2_1(enum omap_plane plane, int haccu,
int vaccu)
{
u32 val;
@@ -1277,7 +1283,7 @@ static void dispc_ovl_set_vid_accu2_1(enum omap_plane plane, int haccu,
dispc_write_reg(DISPC_OVL_ACCU2_1(plane), val);
}
-static void dispc_ovl_set_scale_param(enum omap_plane plane,
+static void dispc_plane_set_scale_param(enum omap_plane plane,
u16 orig_width, u16 orig_height,
u16 out_width, u16 out_height,
bool five_taps, u8 rotation,
@@ -1288,12 +1294,12 @@ static void dispc_ovl_set_scale_param(enum omap_plane plane,
fir_hinc = 1024 * orig_width / out_width;
fir_vinc = 1024 * orig_height / out_height;
- dispc_ovl_set_scale_coef(plane, fir_hinc, fir_vinc, five_taps,
+ dispc_plane_set_scale_coef(plane, fir_hinc, fir_vinc, five_taps,
color_comp);
- dispc_ovl_set_fir(plane, fir_hinc, fir_vinc, color_comp);
+ dispc_plane_set_fir(plane, fir_hinc, fir_vinc, color_comp);
}
-static void dispc_ovl_set_accu_uv(enum omap_plane plane,
+static void dispc_plane_set_accu_uv(enum omap_plane plane,
u16 orig_width, u16 orig_height, u16 out_width, u16 out_height,
bool ilace, enum omap_color_mode color_mode, u8 rotation)
{
@@ -1377,11 +1383,11 @@ static void dispc_ovl_set_accu_uv(enum omap_plane plane,
v_accu2_0 = (accu_val->v0_m * chroma_vinc / accu_val->v0_n) % 1024;
v_accu2_1 = (accu_val->v1_m * chroma_vinc / accu_val->v1_n) % 1024;
- dispc_ovl_set_vid_accu2_0(plane, h_accu2_0, v_accu2_0);
- dispc_ovl_set_vid_accu2_1(plane, h_accu2_1, v_accu2_1);
+ dispc_plane_set_vid_accu2_0(plane, h_accu2_0, v_accu2_0);
+ dispc_plane_set_vid_accu2_1(plane, h_accu2_1, v_accu2_1);
}
-static void dispc_ovl_set_scaling_common(enum omap_plane plane,
+static void dispc_plane_set_scaling_common(enum omap_plane plane,
u16 orig_width, u16 orig_height,
u16 out_width, u16 out_height,
bool ilace, bool five_taps,
@@ -1392,7 +1398,7 @@ static void dispc_ovl_set_scaling_common(enum omap_plane plane,
int accu1 = 0;
u32 l;
- dispc_ovl_set_scale_param(plane, orig_width, orig_height,
+ dispc_plane_set_scale_param(plane, orig_width, orig_height,
out_width, out_height, five_taps,
rotation, DISPC_COLOR_COMPONENT_RGB_Y);
l = dispc_read_reg(DISPC_OVL_ATTRIBUTES(plane));
@@ -1431,11 +1437,11 @@ static void dispc_ovl_set_scaling_common(enum omap_plane plane,
}
}
- dispc_ovl_set_vid_accu0(plane, 0, accu0);
- dispc_ovl_set_vid_accu1(plane, 0, accu1);
+ dispc_plane_set_vid_accu0(plane, 0, accu0);
+ dispc_plane_set_vid_accu1(plane, 0, accu1);
}
-static void dispc_ovl_set_scaling_uv(enum omap_plane plane,
+static void dispc_plane_set_scaling_uv(enum omap_plane plane,
u16 orig_width, u16 orig_height,
u16 out_width, u16 out_height,
bool ilace, bool five_taps,
@@ -1455,7 +1461,7 @@ static void dispc_ovl_set_scaling_uv(enum omap_plane plane,
return;
}
- dispc_ovl_set_accu_uv(plane, orig_width, orig_height, out_width,
+ dispc_plane_set_accu_uv(plane, orig_width, orig_height, out_width,
out_height, ilace, color_mode, rotation);
switch (color_mode) {
@@ -1488,7 +1494,7 @@ static void dispc_ovl_set_scaling_uv(enum omap_plane plane,
if (out_height != orig_height)
scale_y = true;
- dispc_ovl_set_scale_param(plane, orig_width, orig_height,
+ dispc_plane_set_scale_param(plane, orig_width, orig_height,
out_width, out_height, five_taps,
rotation, DISPC_COLOR_COMPONENT_UV);
@@ -1500,7 +1506,7 @@ static void dispc_ovl_set_scaling_uv(enum omap_plane plane,
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), scale_y ? 1 : 0, 6, 6);
}
-static void dispc_ovl_set_scaling(enum omap_plane plane,
+static void dispc_plane_set_scaling(enum omap_plane plane,
u16 orig_width, u16 orig_height,
u16 out_width, u16 out_height,
bool ilace, bool five_taps,
@@ -1509,14 +1515,14 @@ static void dispc_ovl_set_scaling(enum omap_plane plane,
{
BUG_ON(plane = OMAP_DSS_GFX);
- dispc_ovl_set_scaling_common(plane,
+ dispc_plane_set_scaling_common(plane,
orig_width, orig_height,
out_width, out_height,
ilace, five_taps,
fieldmode, color_mode,
rotation);
- dispc_ovl_set_scaling_uv(plane,
+ dispc_plane_set_scaling_uv(plane,
orig_width, orig_height,
out_width, out_height,
ilace, five_taps,
@@ -2190,7 +2196,7 @@ static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
return 0;
}
-static int dispc_ovl_calc_scaling(enum omap_plane plane,
+static int dispc_plane_calc_scaling(enum omap_plane plane,
enum omap_channel channel,
const struct omap_video_timings *mgr_timings,
u16 width, u16 height, u16 out_width, u16 out_height,
@@ -2305,7 +2311,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
if (!dss_feat_color_mode_supported(plane, oi->color_mode))
return -EINVAL;
- r = dispc_ovl_calc_scaling(plane, channel, mgr_timings, in_width,
+ r = dispc_plane_calc_scaling(plane, channel, mgr_timings, in_width,
in_height, out_width, out_height, oi->color_mode,
&five_taps, &x_predecim, &y_predecim, oi->pos_x);
if (r)
@@ -2387,7 +2393,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
dispc_ovl_set_pic_size(plane, in_width, in_height);
if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
- dispc_ovl_set_scaling(plane, in_width, in_height, out_width,
+ dispc_plane_set_scaling(plane, in_width, in_height, out_width,
out_height, ilace, five_taps, fieldmode,
oi->color_mode, oi->rotation);
dispc_ovl_set_vid_size(plane, out_width, out_height);
diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index 42e56cc..84cc472 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -109,7 +109,7 @@ struct dispc_coef {
s8 hc0_vc00;
};
-const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps);
+const struct dispc_coef *dispc_plane_get_scale_coef(int inc, int five_taps);
/* DISPC manager/channel specific registers */
static inline u16 DISPC_DEFAULT_COLOR(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dispc_coefs.c b/drivers/video/omap2/dss/dispc_coefs.c
index 038c15b..21d8c82 100644
--- a/drivers/video/omap2/dss/dispc_coefs.c
+++ b/drivers/video/omap2/dss/dispc_coefs.c
@@ -286,7 +286,7 @@ static const struct dispc_coef coef5_M32[8] = {
{ 5, 29, 48, 36, 10 },
};
-const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
+const struct dispc_coef *dispc_plane_get_scale_coef(int inc, int five_taps)
{
int i;
static const struct {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 03/21] OMAPDSS: DISPC: Rename fifo/burst related functions from dispc_ovl_* to dispc_plane_*
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
Writeback pipeline uses fifo and burst related IP similar to what the graphics
and video pipe have. Rename the related functions from dispc_ovl_* to
dispc_plane_*. The actual registers are kept as DISPC_OVL_* only to prevent too
much change.
All functions which are common to overlays and writeback are to be named as
dispc_plane_*, functions which are specific to overlays are to be named as
dispc_ovl_*, and writeback as dispc_wb_*.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/apply.c | 4 ++--
drivers/video/omap2/dss/dispc.c | 30 +++++++++++++++---------------
drivers/video/omap2/dss/dss.h | 4 ++--
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 2b1fa85..9abeca0 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -618,7 +618,7 @@ static void dss_ovl_write_regs_extra(struct omap_overlay *ovl)
dispc_ovl_enable(ovl->id, op->enabled);
dispc_ovl_set_channel_out(ovl->id, op->channel);
- dispc_ovl_set_fifo_threshold(ovl->id, op->fifo_low, op->fifo_high);
+ dispc_plane_set_fifo_threshold(ovl->id, op->fifo_low, op->fifo_high);
mp = get_mgr_priv(ovl->manager);
@@ -973,7 +973,7 @@ static void dss_ovl_setup_fifo(struct omap_overlay *ovl)
if (!op->enabled && !op->enabling)
return;
- dispc_ovl_compute_fifo_thresholds(ovl->id, &fifo_low, &fifo_high,
+ dispc_plane_compute_fifo_thresholds(ovl->id, &fifo_low, &fifo_high,
use_fifo_merge, ovl_manual_update(ovl));
dss_apply_ovl_fifo_thresholds(ovl, fifo_low, fifo_high);
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index eae9da4..44e86ad 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -866,7 +866,7 @@ static void dispc_ovl_set_color_mode(enum omap_plane plane,
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), m, 4, 1);
}
-static void dispc_ovl_configure_burst_type(enum omap_plane plane,
+static void dispc_plane_configure_burst_type(enum omap_plane plane,
enum omap_dss_rotation_type rotation_type)
{
if (dss_has_feature(FEAT_BURST_2D) = 0)
@@ -976,7 +976,7 @@ static enum omap_channel dispc_ovl_get_channel_out(enum omap_plane plane)
return channel;
}
-static void dispc_ovl_set_burst_size(enum omap_plane plane,
+static void dispc_plane_set_burst_size(enum omap_plane plane,
enum omap_burst_size burst_size)
{
static const unsigned shifts[] = { 6, 14, 14, 14, };
@@ -993,10 +993,10 @@ static void dispc_configure_burst_sizes(void)
/* Configure burst size always to maximum size */
for (i = 0; i < omap_dss_get_num_overlays(); ++i)
- dispc_ovl_set_burst_size(i, burst_size);
+ dispc_plane_set_burst_size(i, burst_size);
}
-static u32 dispc_ovl_get_burst_size(enum omap_plane plane)
+static u32 dispc_plane_get_burst_size(enum omap_plane plane)
{
unsigned unit = dss_feat_get_burst_size_unit();
/* burst multiplier is always x8 (see dispc_configure_burst_sizes()) */
@@ -1121,7 +1121,7 @@ static void dispc_init_fifos(void)
}
}
-static u32 dispc_ovl_get_fifo_size(enum omap_plane plane)
+static u32 dispc_plane_get_fifo_size(enum omap_plane plane)
{
int fifo;
u32 size = 0;
@@ -1134,7 +1134,7 @@ static u32 dispc_ovl_get_fifo_size(enum omap_plane plane)
return size;
}
-void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
+void dispc_plane_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high)
{
u8 hi_start, hi_end, lo_start, lo_end;
u32 unit;
@@ -1174,7 +1174,7 @@ void dispc_enable_fifomerge(bool enable)
REG_FLD_MOD(DISPC_CONFIG, enable ? 1 : 0, 14, 14);
}
-void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
+void dispc_plane_compute_fifo_thresholds(enum omap_plane plane,
u32 *fifo_low, u32 *fifo_high, bool use_fifomerge,
bool manual_update)
{
@@ -1184,18 +1184,18 @@ void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
*/
unsigned buf_unit = dss_feat_get_buffer_size_unit();
- unsigned ovl_fifo_size, total_fifo_size, burst_size;
+ unsigned plane_fifo_size, total_fifo_size, burst_size;
int i;
- burst_size = dispc_ovl_get_burst_size(plane);
- ovl_fifo_size = dispc_ovl_get_fifo_size(plane);
+ burst_size = dispc_plane_get_burst_size(plane);
+ plane_fifo_size = dispc_plane_get_fifo_size(plane);
if (use_fifomerge) {
total_fifo_size = 0;
for (i = 0; i < omap_dss_get_num_overlays(); ++i)
- total_fifo_size += dispc_ovl_get_fifo_size(i);
+ total_fifo_size += dispc_plane_get_fifo_size(i);
} else {
- total_fifo_size = ovl_fifo_size;
+ total_fifo_size = plane_fifo_size;
}
/*
@@ -1205,10 +1205,10 @@ void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
*/
if (manual_update && dss_has_feature(FEAT_OMAP3_DSI_FIFO_BUG)) {
- *fifo_low = ovl_fifo_size - burst_size * 2;
+ *fifo_low = plane_fifo_size - burst_size * 2;
*fifo_high = total_fifo_size - burst_size;
} else {
- *fifo_low = ovl_fifo_size - burst_size;
+ *fifo_low = plane_fifo_size - burst_size;
*fifo_high = total_fifo_size - buf_unit;
}
}
@@ -2371,7 +2371,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
dispc_ovl_set_color_mode(plane, oi->color_mode);
- dispc_ovl_configure_burst_type(plane, oi->rotation_type);
+ dispc_plane_configure_burst_type(plane, oi->rotation_type);
dispc_ovl_set_ba0(plane, oi->paddr + offset0);
dispc_ovl_set_ba1(plane, oi->paddr + offset1);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 43210b8..4b1ca03 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -436,8 +436,8 @@ int dispc_calc_clock_rates(unsigned long dispc_fclk_rate,
struct dispc_clock_info *cinfo);
-void dispc_ovl_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high);
-void dispc_ovl_compute_fifo_thresholds(enum omap_plane plane,
+void dispc_plane_set_fifo_threshold(enum omap_plane plane, u32 low, u32 high);
+void dispc_plane_compute_fifo_thresholds(enum omap_plane plane,
u32 *fifo_low, u32 *fifo_high, bool use_fifomerge,
bool manual_update);
int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 04/21] OMAPDSS: DISPC: Rename misc functions from dispc_ovl_* to dispc_plane_*
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
Writeback pipeline has similar registers compared to graphics and video pipes
for setting base addresses, color conversion, row inc, pix inc etc. Rename these
functions from dispc_ovl_* to dispc_plane_*. The actual registers are kept as
DISPC_OVL_* only to prevent too much change.
All functions which are common to overlays and writeback are to be named as
dispc_plane_*, functions which are specific to overlays are to be named as
dispc_ovl_*, and writeback as dispc_wb_*.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 67 ++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 44e86ad..9ecdd44 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -683,34 +683,35 @@ static void _dispc_setup_color_conv_coef(void)
}
-static void dispc_ovl_set_ba0(enum omap_plane plane, u32 paddr)
+static void dispc_plane_set_ba0(enum omap_plane plane, u32 paddr)
{
dispc_write_reg(DISPC_OVL_BA0(plane), paddr);
}
-static void dispc_ovl_set_ba1(enum omap_plane plane, u32 paddr)
+static void dispc_plane_set_ba1(enum omap_plane plane, u32 paddr)
{
dispc_write_reg(DISPC_OVL_BA1(plane), paddr);
}
-static void dispc_ovl_set_ba0_uv(enum omap_plane plane, u32 paddr)
+static void dispc_plane_set_ba0_uv(enum omap_plane plane, u32 paddr)
{
dispc_write_reg(DISPC_OVL_BA0_UV(plane), paddr);
}
-static void dispc_ovl_set_ba1_uv(enum omap_plane plane, u32 paddr)
+static void dispc_plane_set_ba1_uv(enum omap_plane plane, u32 paddr)
{
dispc_write_reg(DISPC_OVL_BA1_UV(plane), paddr);
}
-static void dispc_ovl_set_pos(enum omap_plane plane, int x, int y)
+static void dispc_plane_set_pos(enum omap_plane plane, int x, int y)
{
u32 val = FLD_VAL(y, 26, 16) | FLD_VAL(x, 10, 0);
dispc_write_reg(DISPC_OVL_POSITION(plane), val);
}
-static void dispc_ovl_set_pic_size(enum omap_plane plane, int width, int height)
+static void dispc_plane_set_pic_size(enum omap_plane plane, int width,
+ int height)
{
u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
@@ -720,7 +721,8 @@ static void dispc_ovl_set_pic_size(enum omap_plane plane, int width, int height)
dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
}
-static void dispc_ovl_set_vid_size(enum omap_plane plane, int width, int height)
+static void dispc_plane_set_vid_size(enum omap_plane plane, int width,
+ int height)
{
u32 val;
@@ -731,7 +733,7 @@ static void dispc_ovl_set_vid_size(enum omap_plane plane, int width, int height)
dispc_write_reg(DISPC_OVL_SIZE(plane), val);
}
-static void dispc_ovl_set_zorder(enum omap_plane plane, u8 zorder)
+static void dispc_plane_set_zorder(enum omap_plane plane, u8 zorder)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
@@ -752,7 +754,7 @@ static void dispc_ovl_enable_zorder_planes(void)
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(i), 1, 25, 25);
}
-static void dispc_ovl_set_pre_mult_alpha(enum omap_plane plane, bool enable)
+static void dispc_plane_set_pre_mult_alpha(enum omap_plane plane, bool enable)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
@@ -762,7 +764,8 @@ static void dispc_ovl_set_pre_mult_alpha(enum omap_plane plane, bool enable)
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), enable ? 1 : 0, 28, 28);
}
-static void dispc_ovl_setup_global_alpha(enum omap_plane plane, u8 global_alpha)
+static void dispc_plane_setup_global_alpha(enum omap_plane plane,
+ u8 global_alpha)
{
static const unsigned shifts[] = { 0, 8, 16, 24, };
int shift;
@@ -775,17 +778,17 @@ static void dispc_ovl_setup_global_alpha(enum omap_plane plane, u8 global_alpha)
REG_FLD_MOD(DISPC_GLOBAL_ALPHA, global_alpha, shift + 7, shift);
}
-static void dispc_ovl_set_pix_inc(enum omap_plane plane, s32 inc)
+static void dispc_plane_set_pix_inc(enum omap_plane plane, s32 inc)
{
dispc_write_reg(DISPC_OVL_PIXEL_INC(plane), inc);
}
-static void dispc_ovl_set_row_inc(enum omap_plane plane, s32 inc)
+static void dispc_plane_set_row_inc(enum omap_plane plane, s32 inc)
{
dispc_write_reg(DISPC_OVL_ROW_INC(plane), inc);
}
-static void dispc_ovl_set_color_mode(enum omap_plane plane,
+static void dispc_plane_set_color_mode(enum omap_plane plane,
enum omap_color_mode color_mode)
{
u32 m = 0;
@@ -1045,7 +1048,7 @@ static void dispc_mgr_set_cpr_coef(enum omap_channel channel,
dispc_write_reg(DISPC_CPR_COEF_B(channel), coef_b);
}
-static void dispc_ovl_set_vid_color_conv(enum omap_plane plane, bool enable)
+static void dispc_plane_set_vid_color_conv(enum omap_plane plane, bool enable)
{
u32 val;
@@ -1056,7 +1059,7 @@ static void dispc_ovl_set_vid_color_conv(enum omap_plane plane, bool enable)
dispc_write_reg(DISPC_OVL_ATTRIBUTES(plane), val);
}
-static void dispc_ovl_enable_replication(enum omap_plane plane, bool enable)
+static void dispc_plane_enable_replication(enum omap_plane plane, bool enable)
{
static const unsigned shifts[] = { 5, 10, 10, 10 };
int shift;
@@ -1530,7 +1533,7 @@ static void dispc_plane_set_scaling(enum omap_plane plane,
rotation);
}
-static void dispc_ovl_set_rotation_attrs(enum omap_plane plane, u8 rotation,
+static void dispc_plane_set_rotation_attrs(enum omap_plane plane, u8 rotation,
bool mirroring, enum omap_color_mode color_mode)
{
bool row_repeat = false;
@@ -2369,45 +2372,45 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
DSSDBG("offset0 %u, offset1 %u, row_inc %d, pix_inc %d\n",
offset0, offset1, row_inc, pix_inc);
- dispc_ovl_set_color_mode(plane, oi->color_mode);
+ dispc_plane_set_color_mode(plane, oi->color_mode);
dispc_plane_configure_burst_type(plane, oi->rotation_type);
- dispc_ovl_set_ba0(plane, oi->paddr + offset0);
- dispc_ovl_set_ba1(plane, oi->paddr + offset1);
+ dispc_plane_set_ba0(plane, oi->paddr + offset0);
+ dispc_plane_set_ba1(plane, oi->paddr + offset1);
if (OMAP_DSS_COLOR_NV12 = oi->color_mode) {
- dispc_ovl_set_ba0_uv(plane, oi->p_uv_addr + offset0);
- dispc_ovl_set_ba1_uv(plane, oi->p_uv_addr + offset1);
+ dispc_plane_set_ba0_uv(plane, oi->p_uv_addr + offset0);
+ dispc_plane_set_ba1_uv(plane, oi->p_uv_addr + offset1);
}
- dispc_ovl_set_row_inc(plane, row_inc);
- dispc_ovl_set_pix_inc(plane, pix_inc);
+ dispc_plane_set_row_inc(plane, row_inc);
+ dispc_plane_set_pix_inc(plane, pix_inc);
DSSDBG("%d,%d %dx%d -> %dx%d\n", oi->pos_x, oi->pos_y, in_width,
in_height, out_width, out_height);
- dispc_ovl_set_pos(plane, oi->pos_x, pos_y);
+ dispc_plane_set_pos(plane, oi->pos_x, pos_y);
- dispc_ovl_set_pic_size(plane, in_width, in_height);
+ dispc_plane_set_pic_size(plane, in_width, in_height);
if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
dispc_plane_set_scaling(plane, in_width, in_height, out_width,
out_height, ilace, five_taps, fieldmode,
oi->color_mode, oi->rotation);
- dispc_ovl_set_vid_size(plane, out_width, out_height);
- dispc_ovl_set_vid_color_conv(plane, cconv);
+ dispc_plane_set_vid_size(plane, out_width, out_height);
+ dispc_plane_set_vid_color_conv(plane, cconv);
}
- dispc_ovl_set_rotation_attrs(plane, oi->rotation, oi->mirror,
+ dispc_plane_set_rotation_attrs(plane, oi->rotation, oi->mirror,
oi->color_mode);
- dispc_ovl_set_zorder(plane, oi->zorder);
- dispc_ovl_set_pre_mult_alpha(plane, oi->pre_mult_alpha);
- dispc_ovl_setup_global_alpha(plane, oi->global_alpha);
+ dispc_plane_set_zorder(plane, oi->zorder);
+ dispc_plane_set_pre_mult_alpha(plane, oi->pre_mult_alpha);
+ dispc_plane_setup_global_alpha(plane, oi->global_alpha);
- dispc_ovl_enable_replication(plane, replication);
+ dispc_plane_enable_replication(plane, replication);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 05/21] OMAPDSS: DISPC: Simplify function names for setting pipeline input and output sizes
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
The DISPC pipeline register names in the TRM for setting the buffer size and
the output size are a bit misleading, for example, there are different register
names for setting the buffer size for VID and GFX pipes. Things get more
confusing when considering writeback pipeline.
Rename the functions so that they tell whether they are configuring the input
to the scalar or the output. These will be extended later to support writeback
registers.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 9ecdd44..f60fcf4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -710,7 +710,7 @@ static void dispc_plane_set_pos(enum omap_plane plane, int x, int y)
dispc_write_reg(DISPC_OVL_POSITION(plane), val);
}
-static void dispc_plane_set_pic_size(enum omap_plane plane, int width,
+static void dispc_plane_set_input_size(enum omap_plane plane, int width,
int height)
{
u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
@@ -721,7 +721,7 @@ static void dispc_plane_set_pic_size(enum omap_plane plane, int width,
dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
}
-static void dispc_plane_set_vid_size(enum omap_plane plane, int width,
+static void dispc_plane_set_output_size(enum omap_plane plane, int width,
int height)
{
u32 val;
@@ -2393,13 +2393,13 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
dispc_plane_set_pos(plane, oi->pos_x, pos_y);
- dispc_plane_set_pic_size(plane, in_width, in_height);
+ dispc_plane_set_input_size(plane, in_width, in_height);
if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
dispc_plane_set_scaling(plane, in_width, in_height, out_width,
out_height, ilace, five_taps, fieldmode,
oi->color_mode, oi->rotation);
- dispc_plane_set_vid_size(plane, out_width, out_height);
+ dispc_plane_set_output_size(plane, out_width, out_height);
dispc_plane_set_vid_color_conv(plane, cconv);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 06/21] OMAPDSS: DISPC: Pass overlay caps as a parameter to dispc plane functions
From: Archit Taneja @ 2012-09-13 12:26 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1347538505-25359-1-git-send-email-archit@ti.com>
Currently, the functions below take the omap_plane parameter and derive the
overlay caps within them. Pass the overlay caps as a parameter to the function
to allow these to be used by writeback too.
- dispc_plane_set_zorder()
- dispc_plane_set_pre_mult_alpha()
- dispc_plane_setup_global_alpha()
- dispc_plane_calc_scaling()
- dispc_ovl_setup()
These functions will be used for writeback later, and the caps will help in
deciding if they are to be used for writeback or not. This allows reuse of
overlay caps for writeback.
Using omap_overlay_caps for writeback seems a bit incorrect, but caps is
something already in use by users of OMAPDSS(omapfb/omap_vout), so we use
overlay caps for overlay like features of writeback too.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/dispc.c | 40 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index f60fcf4..3a0a576 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -733,11 +733,10 @@ static void dispc_plane_set_output_size(enum omap_plane plane, int width,
dispc_write_reg(DISPC_OVL_SIZE(plane), val);
}
-static void dispc_plane_set_zorder(enum omap_plane plane, u8 zorder)
+static void dispc_plane_set_zorder(enum omap_plane plane,
+ enum omap_overlay_caps caps, u8 zorder)
{
- struct omap_overlay *ovl = omap_dss_get_overlay(plane);
-
- if ((ovl->caps & OMAP_DSS_OVL_CAP_ZORDER) = 0)
+ if ((caps & OMAP_DSS_OVL_CAP_ZORDER) = 0)
return;
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), zorder, 27, 26);
@@ -754,24 +753,22 @@ static void dispc_ovl_enable_zorder_planes(void)
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(i), 1, 25, 25);
}
-static void dispc_plane_set_pre_mult_alpha(enum omap_plane plane, bool enable)
+static void dispc_plane_set_pre_mult_alpha(enum omap_plane plane,
+ enum omap_overlay_caps caps, bool enable)
{
- struct omap_overlay *ovl = omap_dss_get_overlay(plane);
-
- if ((ovl->caps & OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA) = 0)
+ if ((caps & OMAP_DSS_OVL_CAP_PRE_MULT_ALPHA) = 0)
return;
REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), enable ? 1 : 0, 28, 28);
}
static void dispc_plane_setup_global_alpha(enum omap_plane plane,
- u8 global_alpha)
+ enum omap_overlay_caps caps, u8 global_alpha)
{
static const unsigned shifts[] = { 0, 8, 16, 24, };
int shift;
- struct omap_overlay *ovl = omap_dss_get_overlay(plane);
- if ((ovl->caps & OMAP_DSS_OVL_CAP_GLOBAL_ALPHA) = 0)
+ if ((caps & OMAP_DSS_OVL_CAP_GLOBAL_ALPHA) = 0)
return;
shift = shifts[plane];
@@ -2200,13 +2197,12 @@ static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
}
static int dispc_plane_calc_scaling(enum omap_plane plane,
- enum omap_channel channel,
+ enum omap_overlay_caps caps, enum omap_channel channel,
const struct omap_video_timings *mgr_timings,
u16 width, u16 height, u16 out_width, u16 out_height,
enum omap_color_mode color_mode, bool *five_taps,
int *x_predecim, int *y_predecim, u16 pos_x)
{
- struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
const int max_decim_limit = 16;
unsigned long core_clk = 0;
@@ -2215,7 +2211,7 @@ static int dispc_plane_calc_scaling(enum omap_plane plane,
if (width = out_width && height = out_height)
return 0;
- if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0)
+ if ((caps & OMAP_DSS_OVL_CAP_SCALE) = 0)
return -EINVAL;
*x_predecim = max_decim_limit;
@@ -2266,6 +2262,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
bool replication, const struct omap_video_timings *mgr_timings)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
+ enum omap_overlay_caps caps = ovl->caps;
bool five_taps = true;
bool fieldmode = 0;
int r, cconv = 0;
@@ -2314,9 +2311,10 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
if (!dss_feat_color_mode_supported(plane, oi->color_mode))
return -EINVAL;
- r = dispc_plane_calc_scaling(plane, channel, mgr_timings, in_width,
- in_height, out_width, out_height, oi->color_mode,
- &five_taps, &x_predecim, &y_predecim, oi->pos_x);
+ r = dispc_plane_calc_scaling(plane, caps, channel, mgr_timings,
+ in_width, in_height, out_width, out_height,
+ oi->color_mode, &five_taps, &x_predecim, &y_predecim,
+ oi->pos_x);
if (r)
return r;
@@ -2395,7 +2393,7 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
dispc_plane_set_input_size(plane, in_width, in_height);
- if (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) {
+ if (caps & OMAP_DSS_OVL_CAP_SCALE) {
dispc_plane_set_scaling(plane, in_width, in_height, out_width,
out_height, ilace, five_taps, fieldmode,
oi->color_mode, oi->rotation);
@@ -2406,9 +2404,9 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
dispc_plane_set_rotation_attrs(plane, oi->rotation, oi->mirror,
oi->color_mode);
- dispc_plane_set_zorder(plane, oi->zorder);
- dispc_plane_set_pre_mult_alpha(plane, oi->pre_mult_alpha);
- dispc_plane_setup_global_alpha(plane, oi->global_alpha);
+ dispc_plane_set_zorder(plane, caps, oi->zorder);
+ dispc_plane_set_pre_mult_alpha(plane, caps, oi->pre_mult_alpha);
+ dispc_plane_setup_global_alpha(plane, caps, oi->global_alpha);
dispc_plane_enable_replication(plane, replication);
--
1.7.9.5
^ 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