* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-25 13:49 UTC (permalink / raw)
To: Thierry Reding
Cc: Stephen Warren, Tomi Valkeinen, Dave Airlie, Laurent Pinchart,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131025122925.GA24720-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]
Hi Thierry,
On Friday 25 October 2013 14:29:26 Thierry Reding wrote:
> On Fri, Oct 25, 2013 at 01:33:11PM +0200, Laurent Pinchart wrote:
> > On Thursday 24 October 2013 23:06:18 Stephen Warren wrote:
> > > On 10/24/2013 12:20 PM, Laurent Pinchart wrote:
> > > > On Sunday 20 October 2013 23:07:36 Stephen Warren wrote:
> > > >> On 10/17/2013 12:07 PM, Laurent Pinchart wrote:
> > > >> ...
> > > >>
> > > >>>> As I said, anything that really needs a CDF binding to work
> > > >>>> likely isn't "simple" anymore, therefore a separate driver can
> > > >>>> easily be justified.
> > > >>>
> > > >>> The system as a whole would be more complex, but the panel could be
> > > >>> the same. We can't have two drivers for the same piece of hardware
> > > >>> in the DT world, as there will be a single compatible string and no
> > > >>> way to choose between the drivers (unlike the board code world that
> > > >>> could set device names to "foo- encoder-v4l2" or "foo-encoder-drm"
> > > >>> and live happily with that ever after).
> > > >>
> > > >> That's not true. We can certainly define two different compatible
> > > >> values for a piece of HW if we have to. We can easily control whether
> > > >> they are handled by the same or different drivers in the OS.
> > > >
> > > > From an implementation point of view, sure. But from a conceptual
> > > > point of view, that would make the DT bindings pretty Linux-specific,
> > > > with a description of what the operating system should do instead of a
> > > > description of what the hardware looks like. My understanding is that
> > > > we've tried pretty hard in the past not to open that Pandora's box.
> > > >
> > > > The case I'm mostly concerned about would be two different
> > > > compatibility strings to select whether the device should be handled
> > > > by a KMS or V4L driver. I don't think that's a good idea.
> > >
> > > I wouldn't think of the two compatible values as selecting different
> > > specific Linux drivers, but rather they simply describe the HW in
> > > different levels of detail. The fact that if we know a certain level of
> > > detail about the HW means that Linux can and does create a KMS driver
> > > rather than a V4L2 driver seems like a detail that's completely hidden
> > > inside the OS.
> >
> > I expect the same level of details to be needed on both the KMS and V4L
> > sides. Taking the example of the ADV7511 HDMI transmitter, the only
> > change in the DT bindings between KMS and V4L would be the compatible
> > string. "adi,adv7511-v4l" and "adi,adv7511-kms" is an option that I don't
> > really like. Renaming -v4l and -kms to different names wouldn't
> > fundamentally change the problem.
>
> I think that we're doing something fundamentally wrong if we use the
> same device (with the same functionality) in two different subsystems.
> If the device is used to display something, shouldn't we be moving the
> driver to DRM?
Let's discuss this in reply to the e-mail from this thread in which I mention
the ADV7511 example.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Thierry Reding @ 2013-10-25 14:10 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Stephen Warren, Tomi Valkeinen, Dave Airlie, Laurent Pinchart,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <1394972.QJSGNM8CJk@avalon>
[-- Attachment #1: Type: text/plain, Size: 5732 bytes --]
On Fri, Oct 25, 2013 at 03:47:21PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
>
> On Friday 25 October 2013 10:13:15 Thierry Reding wrote:
> > On Thu, Oct 24, 2013 at 11:06:18PM +0100, Stephen Warren wrote:
> > > On 10/24/2013 12:20 PM, Laurent Pinchart wrote:
> > > > On Sunday 20 October 2013 23:07:36 Stephen Warren wrote:
> > > >> On 10/17/2013 12:07 PM, Laurent Pinchart wrote:
> > > >> ...
> > > >>
> > > >>>> As I said, anything that really needs a CDF binding to work
> > > >>>> likely isn't "simple" anymore, therefore a separate driver can
> > > >>>> easily be justified.
> > > >>>
> > > >>> The system as a whole would be more complex, but the panel could be
> > > >>> the same. We can't have two drivers for the same piece of hardware
> > > >>> in the DT world, as there will be a single compatible string and no
> > > >>> way to choose between the drivers (unlike the board code world that
> > > >>> could set device names to "foo- encoder-v4l2" or "foo-encoder-drm"
> > > >>> and live happily with that ever after).
> > > >>
> > > >> That's not true. We can certainly define two different compatible
> > > >> values for a piece of HW if we have to. We can easily control whether
> > > >> they are handled by the same or different drivers in the OS.
> > > >
> > > > From an implementation point of view, sure. But from a conceptual point
> > > > of view, that would make the DT bindings pretty Linux-specific, with a
> > > > description of what the operating system should do instead of a
> > > > description of what the hardware looks like. My understanding is that
> > > > we've tried pretty hard in the past not to open that Pandora's box.
> > > >
> > > > The case I'm mostly concerned about would be two different compatibility
> > > > strings to select whether the device should be handled by a KMS or V4L
> > > > driver. I don't think that's a good idea.
> > >
> > > I wouldn't think of the two compatible values as selecting different
> > > specific Linux drivers, but rather they simply describe the HW in
> > > different levels of detail. The fact that if we know a certain level of
> > > detail about the HW means that Linux can and does create a KMS driver
> > > rather than a V4L2 driver seems like a detail that's completely hidden
> > > inside the OS.
> >
> > I've had a somewhat similar idea the other day but couldn't really put
> > it into words. Interestingly someone else mentioned a similar concept in
> > a different thread which I think describes what I had in mind as well.
> >
> > I was wondering if we couldn't use two compatible values to denote two
> > interfaces that the device implements. Something along the lines of:
> >
> > compatible = "vendor,block-name", "encoder";
> >
> > So a driver could primarily match on "vendor,block-name", but at the
> > same time it could use the additional information of being required to
> > implement "encoder" to expose an additonal interface.
>
> Let's take the hardware architecture described in
> http://www.ideasonboard.org/media/meetings/20131024-elce.pdf#39 (page 39) as
> an example. The green blocks are part of the capture pipeline and handled by
> the V4L subsystem. The blue blocks are part of the display pipeline and
> handled by the KMS subsystem. One ADV7511 HDMI transmitter instance need to be
> controlled by a KMS driver and the second one by a V4L driver.
>
> The two instances are identical, so their DT nodes will show no difference if
> we stick to a hardware description only. There would then be no way to bind to
> different drivers.
I don't quite see why some of the green parts couldn't be part of the
display (KMS) pipeline. I thought I remember you say something about
implementing deep pipelining support in one of the more recent talks.
This sounds exactly like a case where this would be useful.
Obviously as long as that work isn't finished that won't help you, but I
think that providing a way to pass a video stream object from V4L2 to
DRM/KMS would be a more proper solution to this problem.
> > I suppose that perhaps something like a device_type property could be
> > used for that as well, and that might even be the more correct thing to
> > do.
> >
> > We already do something similar to make GPIO controllers expose an
> > interrupt chip by adding an interrupt-controller property. We also use
> > the gpio-controller property to mark a device node as exposing the GPIO
> > interface for that matter.
> >
> > So if a HW block can actually implement two different interfaces, each
> > of them being optional, then there should be ways to represent that in
> > DT as well. We already do that for "simpler" HW blocks, so there's no
> > reason we shouldn't be able to do the same with multimedia components.
> >
> > If it's really an encoder, though, the problem might be different,
> > though, since the interface (at a hardware or functional level if you
> > will) remains the same. But I think in that case it's something that
> > needs to be figured out internally by the OS. In my opinion, if we are
> > in a situation where we have two different drivers in two subsystems for
> > the same device, then we're doing something wrong and it should be fixed
> > at that level, not by quirking the DT into making a decision for us.
>
> I tend to agree, and I'd like to be able to share drivers between V4L and KMS.
> This is way down the road though.
My point is that I don't see a need for sharing the drivers if we can
make both V4L2 and DRM/KMS interoperate well enough to cover such use
cases.
Why share the drivers if we can make it work by sharing the data?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Laurent Pinchart @ 2013-10-25 14:22 UTC (permalink / raw)
To: Thierry Reding
Cc: Stephen Warren, Tomi Valkeinen, Dave Airlie, Laurent Pinchart,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <20131025141029.GD1551-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6360 bytes --]
Hi Thierry,
On Friday 25 October 2013 16:10:30 Thierry Reding wrote:
> On Fri, Oct 25, 2013 at 03:47:21PM +0200, Laurent Pinchart wrote:
> > On Friday 25 October 2013 10:13:15 Thierry Reding wrote:
> > > On Thu, Oct 24, 2013 at 11:06:18PM +0100, Stephen Warren wrote:
> > > > On 10/24/2013 12:20 PM, Laurent Pinchart wrote:
> > > > > On Sunday 20 October 2013 23:07:36 Stephen Warren wrote:
> > > > >> On 10/17/2013 12:07 PM, Laurent Pinchart wrote:
> > > > >> ...
> > > > >>
> > > > >>>> As I said, anything that really needs a CDF binding to work
> > > > >>>> likely isn't "simple" anymore, therefore a separate driver can
> > > > >>>> easily be justified.
> > > > >>>
> > > > >>> The system as a whole would be more complex, but the panel could
> > > > >>> be the same. We can't have two drivers for the same piece of
> > > > >>> hardware in the DT world, as there will be a single compatible
> > > > >>> string and no way to choose between the drivers (unlike the board
> > > > >>> code world that could set device names to "foo- encoder-v4l2" or
> > > > >>> "foo-encoder-drm" and live happily with that ever after).
> > > > >>
> > > > >> That's not true. We can certainly define two different compatible
> > > > >> values for a piece of HW if we have to. We can easily control
> > > > >> whether they are handled by the same or different drivers in the
> > > > >> OS.
> > > > >
> > > > > From an implementation point of view, sure. But from a conceptual
> > > > > point of view, that would make the DT bindings pretty Linux-
> > > > > specific, with a description of what the operating system should do
> > > > > instead of a description of what the hardware looks like. My
> > > > > understanding is that we've tried pretty hard in the past not to
> > > > > open that Pandora's box.
> > > > >
> > > > > The case I'm mostly concerned about would be two different
> > > > > compatibility strings to select whether the device should be handled
> > > > > by a KMS or V4L driver. I don't think that's a good idea.
> > > >
> > > > I wouldn't think of the two compatible values as selecting different
> > > > specific Linux drivers, but rather they simply describe the HW in
> > > > different levels of detail. The fact that if we know a certain level
> > > > of detail about the HW means that Linux can and does create a KMS
> > > > driver rather than a V4L2 driver seems like a detail that's completely
> > > > hidden inside the OS.
> > >
> > > I've had a somewhat similar idea the other day but couldn't really put
> > > it into words. Interestingly someone else mentioned a similar concept in
> > > a different thread which I think describes what I had in mind as well.
> > >
> > > I was wondering if we couldn't use two compatible values to denote two
> > >
> > > interfaces that the device implements. Something along the lines of:
> > > compatible = "vendor,block-name", "encoder";
> > >
> > > So a driver could primarily match on "vendor,block-name", but at the
> > > same time it could use the additional information of being required to
> > > implement "encoder" to expose an additonal interface.
> >
> > Let's take the hardware architecture described in
> > http://www.ideasonboard.org/media/meetings/20131024-elce.pdf#39 (page 39)
> > as an example. The green blocks are part of the capture pipeline and
> > handled by the V4L subsystem. The blue blocks are part of the display
> > pipeline and handled by the KMS subsystem. One ADV7511 HDMI transmitter
> > instance need to be controlled by a KMS driver and the second one by a
> > V4L driver.
> >
> > The two instances are identical, so their DT nodes will show no difference
> > if we stick to a hardware description only. There would then be no way to
> > bind to different drivers.
>
> I don't quite see why some of the green parts couldn't be part of the
> display (KMS) pipeline.
Because there might be no blue pipeline in the device, just the green one. The
green pipeline is a video capture pipeline and happens to have an HDMI output
with a deep pipeline only, with no frame buffer. We can't create a KMS driver
for that, as there's no frame buffer and no CRTC.
> I thought I remember you say something about implementing deep pipelining
> support in one of the more recent talks. This sounds exactly like a case
> where this would be useful.
>
> Obviously as long as that work isn't finished that won't help you, but I
> think that providing a way to pass a video stream object from V4L2 to
> DRM/KMS would be a more proper solution to this problem.
We need that as well, but it won't solve the problem when no KMS device is
present.
> > > I suppose that perhaps something like a device_type property could be
> > > used for that as well, and that might even be the more correct thing to
> > > do.
> > >
> > > We already do something similar to make GPIO controllers expose an
> > > interrupt chip by adding an interrupt-controller property. We also use
> > > the gpio-controller property to mark a device node as exposing the GPIO
> > > interface for that matter.
> > >
> > > So if a HW block can actually implement two different interfaces, each
> > > of them being optional, then there should be ways to represent that in
> > > DT as well. We already do that for "simpler" HW blocks, so there's no
> > > reason we shouldn't be able to do the same with multimedia components.
> > >
> > > If it's really an encoder, though, the problem might be different,
> > > though, since the interface (at a hardware or functional level if you
> > > will) remains the same. But I think in that case it's something that
> > > needs to be figured out internally by the OS. In my opinion, if we are
> > > in a situation where we have two different drivers in two subsystems for
> > > the same device, then we're doing something wrong and it should be fixed
> > > at that level, not by quirking the DT into making a decision for us.
> >
> > I tend to agree, and I'd like to be able to share drivers between V4L and
> > KMS. This is way down the road though.
>
> My point is that I don't see a need for sharing the drivers if we can
> make both V4L2 and DRM/KMS interoperate well enough to cover such use
> cases.
>
> Why share the drivers if we can make it work by sharing the data?
Would you like to try and merge the two subsystems ? :-)
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [RFR 2/2] drm/panel: Add simple panel support
From: Stephen Warren @ 2013-10-25 15:29 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Thierry Reding, Tomi Valkeinen, Dave Airlie, Laurent Pinchart,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Fbdev development list
In-Reply-To: <1580941.z2CDo8WmRF@avalon>
On 10/25/2013 12:33 PM, Laurent Pinchart wrote:
> On Thursday 24 October 2013 23:06:18 Stephen Warren wrote:
>> On 10/24/2013 12:20 PM, Laurent Pinchart wrote:
...
>>> The case I'm mostly concerned about would be two different compatibility
>>> strings to select whether the device should be handled by a KMS or V4L
>>> driver. I don't think that's a good idea.
>>
>> I wouldn't think of the two compatible values as selecting different
>> specific Linux drivers, but rather they simply describe the HW in
>> different levels of detail. The fact that if we know a certain level of
>> detail about the HW means that Linux can and does create a KMS driver
>> rather than a V4L2 driver seems like a detail that's completely hidden
>> inside the OS.
>
> I expect the same level of details to be needed on both the KMS and V4L sides.
> Taking the example of the ADV7511 HDMI transmitter, the only change in the DT
> bindings between KMS and V4L would be the compatible string. "adi,adv7511-v4l"
> and "adi,adv7511-kms" is an option that I don't really like. Renaming -v4l and
> -kms to different names wouldn't fundamentally change the problem.
Yes, two compatible values such as "adi,adv7511-v4l" and
"adi,adv7511-kms" sounds like a bad idea.
Rather, shouldn't we have just "adi,adv7511", and the driver for that
should register itself in a way that allows either/both the top-level
DRM or top-level V4L drivers to find it, and make use of its services.
There are plenty of drivers in the kernel already that export services
through two different subsystems, e.g. a GPIO HW module that registers
as both a struct gpio_chip and a struct irq_chip, or both a struct
gpio_chip and a pinctrl device.
^ permalink raw reply
* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
From: Sascha Hauer @ 2013-10-26 0:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20131025195040.0CCC3C404DA@trevor.secretlab.ca>
On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: devicetree@vger.kernel.org
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: Eric Bénard <eric@eukrea.com>
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > ---
> > ChangeLog v2->v3:
> > - The device tree bindings were reworked in order to make it look more like the
> > IPUv3 bindings.
> > - The interface_pix_fmt property now looks like the IPUv3 one.
> > ---
> > .../devicetree/bindings/video/fsl,mx3-fb.txt | 35 ++++++
> > drivers/video/Kconfig | 2 +
> > drivers/video/mx3fb.c | 125 +++++++++++++++++---
> > 3 files changed, 147 insertions(+), 15 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > new file mode 100644
> > index 0000000..0b31374
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > @@ -0,0 +1,35 @@
> > +Freescale MX3 fb
> > +========
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> > + imx35.
> > +- reg: should be register base and length as documented in the datasheet.
> > +- clocks: Handle to the ipu_gate clock.
> > +
> > +Example:
> > +
> > +lcdc: mx3fb@53fc00b4 {
> > + compatible = "fsl,mx3-fb";
> > + reg = <0x53fc00b4 0x0b>;
> > + clocks = <&clks 55>;
> > +};
>
> This (and some of the other bindings) are trivial, and they are all
> associated with a single SoC. I think it would be better to collect all
> the mx3 bindings into a single file rather than distributing them all
> over the bindings tree.
>
> I started thinking about this after some of the DT conversations in
> Edinburgh this week. Unless there is a high likelyhood of components
> being used separately, I think it is far more useful to collect all the
> bindings for an SoC into a single file. It will certainly reduce a lot
> of the boilerplate that we've been collecting in bindings documentation
> files.
>
> A long time ago I took that approach for the mpc5200 documentation[1].
> Take a look at that organization and let me know what you think.
I don't think this is a good idea. When a new SoC comes out we don't
know which components will be reused on the next SoC. This will cause a
lot of bikeshedding when it actually is reused and then has to be moved.
Also I would find it quite inconsistent if I had to lookup some devices
in a SoC file and most bindings in subsystem specific files. So when
searching for a binding I would first have to know if the hardware is
unique to the SoC or not.
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: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
From: Kumar Gala @ 2013-10-26 6:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382532229-32755-3-git-send-email-denis@eukrea.com>
On Oct 23, 2013, at 7:43 AM, Denis Carikli wrote:
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
> IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
> .../devicetree/bindings/video/fsl,mx3-fb.txt | 35 ++++++
> drivers/video/Kconfig | 2 +
> drivers/video/mx3fb.c | 125 +++++++++++++++++---
> 3 files changed, 147 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb
Can you spell out framebuffer here instead of fb
> +========
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> + imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb@53fc00b4 {
> + compatible = "fsl,mx3-fb";
> + reg = <0x53fc00b4 0x0b>;
> + clocks = <&clks 55>;
> +};
> +
> +Display support
> +=======> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> + crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
Why is there no compatible for the display?
> +
> +It can also have an optional timing subnode as described in
> + Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display@di0 {
> + interface-pix-fmt = "rgb666";
> + model = "CMO-QVGA";
> +};
how do you relate the display to the framebuffer?
- k
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> + select VIDEOMODE_HELPERS
> + select FB_MODE_HELPERS
> default y
> help
> This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
> #include <linux/platform_data/dma-imx.h>
> #include <linux/platform_data/video-mx3fb.h>
>
> +#include <video/of_display_timing.h>
> +
> #include <asm/io.h>
> #include <asm/uaccess.h>
>
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
> sig_cfg.Hsync_pol = true;
> if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
> sig_cfg.Vsync_pol = true;
> - if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> + if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> + (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
> sig_cfg.clk_pol = true;
> if (fbi->var.sync & FB_SYNC_DATA_INVERT)
> sig_cfg.data_pol = true;
> - if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> + if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> + (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
> sig_cfg.enable_pol = true;
> if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
> sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
> return fbi;
> }
>
> +static int match_dt_disp_data(const char *property)
> +{
> + if (!strcmp("rgb666", property))
> + return IPU_DISP_DATA_MAPPING_RGB666;
> + else if (!strcmp("rgb565", property))
> + return IPU_DISP_DATA_MAPPING_RGB565;
> + else if (!strcmp("rgb24", property))
> + return IPU_DISP_DATA_MAPPING_RGB888;
> + else
> + return -EINVAL;
> +}
> +
> static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> {
> struct device *dev = mx3fb->dev;
> struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> - const char *name = mx3fb_pdata->name;
> + struct device_node *np = dev->of_node;
> + const char *name;
> + const char *ipu_disp_format;
> unsigned int irq;
> struct fb_info *fbi;
> struct mx3fb_info *mx3fbi;
> const struct fb_videomode *mode;
> int ret, num_modes;
> + struct fb_videomode of_mode;
> + struct device_node *display_np, *root_np;
> +
> + if (np) {
> + root_np = of_find_node_by_path("/");
> + if (!root_np) {
> + dev_err(dev, "Can't get the \"/\" node.\n");
> + return -EINVAL;
> + }
> +
> + display_np = of_find_node_by_name(root_np, "display");
> + if (!display_np) {
> + dev_err(dev, "Can't get the display device node.\n");
> + return -EINVAL;
> + }
> +
> + of_property_read_string(display_np, "interface-pix-fmt",
> + &ipu_disp_format);
> + if (!ipu_disp_format) {
> + mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> + dev_warn(dev,
> + "ipu display data mapping was not defined, using the default rgb666.\n");
> + } else {
> + mx3fb->disp_data_fmt > + match_dt_disp_data(ipu_disp_format);
> + }
> +
> + if (mx3fb->disp_data_fmt = -EINVAL) {
> + dev_err(dev, "Illegal display data format \"%s\"\n",
> + ipu_disp_format);
> + return -EINVAL;
> + }
>
> - if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> - dev_err(dev, "Illegal display data format %d\n",
> + of_property_read_string(display_np, "model", &name);
> + if (ret) {
> + dev_err(dev, "Missing display model name\n");
> + return -EINVAL;
> + }
> + } else {
> + name = mx3fb_pdata->name;
> + if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> + dev_err(dev, "Illegal display data format %d\n",
> mx3fb_pdata->disp_data_fmt);
> - return -EINVAL;
> + return -EINVAL;
> + }
> }
>
> ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> goto emode;
> }
>
> - if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> - mode = mx3fb_pdata->mode;
> - num_modes = mx3fb_pdata->num_modes;
> + if (np) {
> + ret = of_get_fb_videomode(display_np, &of_mode,
> + OF_USE_NATIVE_MODE);
> + if (!ret) {
> + mode = &of_mode;
> + num_modes = 1;
> + } else {
> + mode = mx3fb_modedb;
> + num_modes = ARRAY_SIZE(mx3fb_modedb);
> + }
> } else {
> - mode = mx3fb_modedb;
> - num_modes = ARRAY_SIZE(mx3fb_modedb);
> + if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> + mode = mx3fb_pdata->mode;
> + num_modes = mx3fb_pdata->num_modes;
> + } else {
> + mode = mx3fb_modedb;
> + num_modes = ARRAY_SIZE(mx3fb_modedb);
> + }
> }
>
> if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> mx3fbi->mx3fb = mx3fb;
> mx3fbi->blank = FB_BLANK_NORMAL;
>
> - mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
> + if (!np)
> + mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
>
> init_completion(&mx3fbi->flip_cmpl);
> disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
> return ret;
> }
>
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> + /* Example: 53fc0000.ipu */
> + if (chan && chan->device && chan->device->dev) {
> + char *name = dev_name(chan->device->dev);
> + name = strchr(name, '.');
> + if (name)
> + return !strcmp(name, ".ipu");
> + }
> +
> + return 0;
> +}
> +
> static bool chan_filter(struct dma_chan *chan, void *arg)
> {
> struct dma_chan_request *rq = arg;
> struct device *dev;
> struct mx3fb_platform_data *mx3fb_pdata;
>
> - if (!imx_dma_is_ipu(chan))
> + if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
> return false;
>
> if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
> dev = rq->mx3fb->dev;
> mx3fb_pdata = dev_get_platdata(dev);
>
> - return rq->id = chan->chan_id &&
> - mx3fb_pdata->dma_dev = chan->device->dev;
> + /* When using the devicetree, mx3fb_pdata is NULL */
> + if (imx_dma_is_dt_ipu(chan))
> + return rq->id = chan->chan_id;
> + else
> + return rq->id = chan->chan_id &&
> + mx3fb_pdata->dma_dev = chan->device->dev;
> }
>
> static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
> return 0;
> }
>
> +static struct of_device_id mx3fb_of_dev_id[] = {
> + { .compatible = "fsl,mx3-fb", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
> static struct platform_driver mx3fb_driver = {
> .driver = {
> .name = MX3FB_NAME,
> + .of_match_table = mx3fb_of_dev_id,
> .owner = THIS_MODULE,
> },
> .probe = mx3fb_probe,
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
From: Kumar Gala @ 2013-10-26 6:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20131026001854.GE17135@pengutronix.de>
On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:
> On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
>> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: linux-fbdev@vger.kernel.org
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: Sascha Hauer <kernel@pengutronix.de>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: Eric Bénard <eric@eukrea.com>
>>> Signed-off-by: Denis Carikli <denis@eukrea.com>
>>> ---
>>> ChangeLog v2->v3:
>>> - The device tree bindings were reworked in order to make it look more like the
>>> IPUv3 bindings.
>>> - The interface_pix_fmt property now looks like the IPUv3 one.
>>> ---
>>> .../devicetree/bindings/video/fsl,mx3-fb.txt | 35 ++++++
>>> drivers/video/Kconfig | 2 +
>>> drivers/video/mx3fb.c | 125 +++++++++++++++++---
>>> 3 files changed, 147 insertions(+), 15 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> new file mode 100644
>>> index 0000000..0b31374
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> @@ -0,0 +1,35 @@
>>> +Freescale MX3 fb
>>> +========
>>> +
>>> +Required properties:
>>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
>>> + imx35.
>>> +- reg: should be register base and length as documented in the datasheet.
>>> +- clocks: Handle to the ipu_gate clock.
>>> +
>>> +Example:
>>> +
>>> +lcdc: mx3fb@53fc00b4 {
>>> + compatible = "fsl,mx3-fb";
>>> + reg = <0x53fc00b4 0x0b>;
>>> + clocks = <&clks 55>;
>>> +};
>>
>> This (and some of the other bindings) are trivial, and they are all
>> associated with a single SoC. I think it would be better to collect all
>> the mx3 bindings into a single file rather than distributing them all
>> over the bindings tree.
>>
>> I started thinking about this after some of the DT conversations in
>> Edinburgh this week. Unless there is a high likelyhood of components
>> being used separately, I think it is far more useful to collect all the
>> bindings for an SoC into a single file. It will certainly reduce a lot
>> of the boilerplate that we've been collecting in bindings documentation
>> files.
>>
>> A long time ago I took that approach for the mpc5200 documentation[1].
>> Take a look at that organization and let me know what you think.
>
> I don't think this is a good idea. When a new SoC comes out we don't
> know which components will be reused on the next SoC. This will cause a
> lot of bikeshedding when it actually is reused and then has to be moved.
>
> Also I would find it quite inconsistent if I had to lookup some devices
> in a SoC file and most bindings in subsystem specific files. So when
> searching for a binding I would first have to know if the hardware is
> unique to the SoC or not.
I agree that as new SoC come out its better to have these things split out. Also for IP that is sold by vendors like Synopsys/Designware that get used on a lot of SoCs its makes it easier to ensure consistency by having the binding split out for the IP and not the SoC.
Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future. Far more difficult if things are SoC oriented.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* my subject
From: Mr. X @ 2013-10-26 8:51 UTC (permalink / raw)
To: linux-fbdev
Need a Loan, Loans from $5000 - $10,000,000 00. Get your no obligation FREE quote now! Repayments up to 54 Months. No Collateral, Money paid into your account within 24 hours after approval. For more Info contact us today guaranteeloancompany1@gmail.com
^ permalink raw reply
* [PATCH] drivers: video: metronomefb: avoid out-of-bounds array access
From: Michal Nazarewicz @ 2013-10-26 14:58 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
Cc: linux-fbdev, linux-kernel
From: Michal Nazarewicz <mina86@mina86.com>
load_waveform function checks whether padding bytes in stuff2a
and stuff2b are all zero, but does so by treating those arrays
as a single longer array. Since the structure is packed, and
the size sum matches, it all works, but creates some confusion
in the code.
This commit changes the stuff2a and stuff2b arrays into pad1 and
pad2 fields such that they cover the same bytes as the arrays
covered, and changes the check in the load_waveform function so
that the fields are read instead of iterating over an arary.
It also renames the other “stuff” fields to “ignore*” fields to
give them more semantic meaning.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/video/metronomefb.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
index f30150d..44ef8b0 100644
--- a/drivers/video/metronomefb.c
+++ b/drivers/video/metronomefb.c
@@ -126,7 +126,7 @@ static struct fb_var_screeninfo metronomefb_var = {
/* the waveform structure that is coming from userspace firmware */
struct waveform_hdr {
- u8 stuff[32];
+ u8 ignore1[32];
u8 wmta[3];
u8 fvsn;
@@ -134,13 +134,14 @@ struct waveform_hdr {
u8 luts;
u8 mc;
u8 trc;
- u8 stuff3;
+ u8 ignore2;
u8 endb;
u8 swtb;
- u8 stuff2a[2];
+ u32 pad1; /* u16 halfof(pad1) */
- u8 stuff2b[3];
+ /* u16 halfof(pad1) */
+ u8 pad2;
u8 wfm_cs;
} __attribute__ ((packed));
@@ -210,11 +211,9 @@ static int load_waveform(u8 *mem, size_t size, int m, int t,
}
wfm_hdr->mc += 1;
wfm_hdr->trc += 1;
- for (i = 0; i < 5; i++) {
- if (*(wfm_hdr->stuff2a + i) != 0) {
- dev_err(dev, "Error: unexpected value in padding\n");
- return -EINVAL;
- }
+ if (wfm_hdr->pad1 || wfm_hdr->pad2) {
+ dev_err(dev, "Error: unexpected value in padding\n");
+ return -EINVAL;
}
/* calculating trn. trn is something used to index into
--
1.8.4
^ permalink raw reply related
* Re: [PATCH 1/1] backlight: hx8357: Remove redundant of_match_ptr
From: Maxime Ripard @ 2013-10-27 14:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382702373-19573-1-git-send-email-sachin.kamat@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 467 bytes --]
Hi Sachin,
On Fri, Oct 25, 2013 at 05:29:33PM +0530, Sachin Kamat wrote:
> 'hx8357_dt_ids' is always compiled in. Hence of_match_ptr is
> not needed.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] backlight: hx8357: Remove redundant of_match_ptr
From: Jingoo Han @ 2013-10-27 23:08 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1382702373-19573-1-git-send-email-sachin.kamat@linaro.org>
On Sunday, October 27, 2013 11:22 PM, Maxime Ripard wrote:
> On Fri, Oct 25, 2013 at 05:29:33PM +0530, Sachin Kamat wrote:
> > 'hx8357_dt_ids' is always compiled in. Hence of_match_ptr is
> > not needed.
> >
> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
Best regards,
Jingoo Han
^ permalink raw reply
* [RFC PATCH RESEND] fb: reorder the lock sequence to fix a potential lockdep
From: Gu Zheng @ 2013-10-28 6:01 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen
Cc: Linux Fbdev development list, linux-kernel
Following commits:
50e244cc79 fb: rework locking to fix lock ordering on takeover
e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
054430e773 fbcon: fix locking harder
reworked locking to fix related lock ordering on takeover, and introduced console_lock
into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
a potential deadlock as following:
[ 601.079000] ===========================
[ 601.079000] [ INFO: possible circular locking dependency detected ]
[ 601.079000] 3.11.0 #189 Not tainted
[ 601.079000] -------------------------------------------------------
[ 601.079000] kworker/0:3/619 is trying to acquire lock:
[ 601.079000] (&fb_info->lock){+.+.+.}, at: [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000]
but task is already holding lock:
[ 601.079000] (console_lock){+.+.+.}, at: [<ffffffff8141aae3>] console_callback+0x13/0x160
[ 601.079000]
which lock already depends on the new lock.
[ 601.079000]
the existing dependency chain (in reverse order) is:
[ 601.079000]
-> #1 (console_lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff810c6267>] console_lock+0x77/0x80
[ 601.079000] [<ffffffff81399448>] register_framebuffer+0x1d8/0x320
[ 601.079000] [<ffffffff81cfb4c8>] efifb_probe+0x408/0x48f
[ 601.079000] [<ffffffff8144a963>] platform_drv_probe+0x43/0x80
[ 601.079000] [<ffffffff8144853b>] driver_probe_device+0x8b/0x390
[ 601.079000] [<ffffffff814488eb>] __driver_attach+0xab/0xb0
[ 601.079000] [<ffffffff814463bd>] bus_for_each_dev+0x5d/0xa0
[ 601.079000] [<ffffffff81447e6e>] driver_attach+0x1e/0x20
[ 601.079000] [<ffffffff81447a07>] bus_add_driver+0x117/0x290
[ 601.079000] [<ffffffff81448fea>] driver_register+0x7a/0x170
[ 601.079000] [<ffffffff8144a10a>] __platform_driver_register+0x4a/0x50
[ 601.079000] [<ffffffff8144a12d>] platform_driver_probe+0x1d/0xb0
[ 601.079000] [<ffffffff81cfb0a1>] efifb_init+0x273/0x292
[ 601.079000] [<ffffffff81002132>] do_one_initcall+0x102/0x1c0
[ 601.079000] [<ffffffff81cb80a6>] kernel_init_freeable+0x15d/0x1ef
[ 601.079000] [<ffffffff8166d2de>] kernel_init+0xe/0xf0
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
-> #0 (&fb_info->lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc1d8>] __lock_acquire+0x1e18/0x1f10
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff816835ca>] mutex_lock_nested+0x7a/0x3b0
[ 601.079000] [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000] [<ffffffff813a4aeb>] fbcon_blank+0x29b/0x2e0
[ 601.079000] [<ffffffff81418658>] do_blank_screen+0x1d8/0x280
[ 601.079000] [<ffffffff8141ab34>] console_callback+0x64/0x160
[ 601.079000] [<ffffffff8108d855>] process_one_work+0x1f5/0x540
[ 601.079000] [<ffffffff8108e04c>] worker_thread+0x11c/0x370
[ 601.079000] [<ffffffff81095fbd>] kthread+0xed/0x100
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
other info that might help us debug this:
[ 601.079000] Possible unsafe locking scenario:
[ 601.079000] CPU0 CPU1
[ 601.079000] ---- ----
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000]
*** DEADLOCK ***
so we reorder the lock sequence the same as it in console_callback() to
avoid this issue.
Not very sure this change is suitable, any comments is welcome.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
drivers/video/fbmem.c | 50 +++++++++++++++++++++++++++++++-----------------
1 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index dacaf74..010d191 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1108,14 +1108,16 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPUT_VSCREENINFO:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_set_var(info, &var);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (!ret && copy_to_user(argp, &var, sizeof(var)))
ret = -EFAULT;
break;
@@ -1144,12 +1146,14 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
case FBIOPAN_DISPLAY:
if (copy_from_user(&var, argp, sizeof(var)))
return -EFAULT;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
ret = fb_pan_display(info, &var);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
return -EFAULT;
break;
@@ -1184,23 +1188,27 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
break;
}
event.data = &con2fb;
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
event.info = info;
ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
case FBIOBLANK:
- if (!lock_fb_info(info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(info)) {
+ console_unlock();
+ return -ENODEV;
+ }
info->flags |= FBINFO_MISC_USEREVENT;
ret = fb_blank(info, arg);
info->flags &= ~FBINFO_MISC_USEREVENT;
- console_unlock();
unlock_fb_info(info);
+ console_unlock();
break;
default:
if (!lock_fb_info(info))
@@ -1660,12 +1668,15 @@ static int do_register_framebuffer(struct fb_info *fb_info)
registered_fb[i] = fb_info;
event.info = fb_info;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
return 0;
}
@@ -1678,13 +1689,16 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
+ if (!lock_fb_info(fb_info)) {
+ console_unlock();
+ return -ENODEV;
+ }
+
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- console_unlock();
unlock_fb_info(fb_info);
+ console_unlock();
if (ret)
return -EINVAL;
--
1.7.7
^ permalink raw reply related
* Re: [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
From: Donghwa Lee @ 2013-10-28 6:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <526997BC.8070602@gmail.com>
On Fri, OCT 25, 2013 06:57, Sylwester Nawrocki wrote:
> On 10/24/2013 05:57 PM, Kishon Vijay Abraham I wrote:
>> On Thursday 24 October 2013 09:12 PM, Sachin Kamat wrote:
>>> On 24 October 2013 20:00, Olof Johansson<olof@lixom.net> wrote:
>>>> On Wed, Oct 16, 2013 at 9:28 AM, Kishon Vijay Abraham I<kishon@ti.com> wrote:
>>>>> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
>>>>> index 32e5406..00b3a52 100644
>>>>> --- a/drivers/video/exynos/exynos_mipi_dsi.c
>>>>> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
>>>>> @@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
>>>>> exynos_mipi_regulator_enable(dsim);
>>>>>
>>>>> /* enable MIPI-DSI PHY. */
>>>>> - if (dsim->pd->phy_enable)
>>>>> - dsim->pd->phy_enable(pdev, true);
>>>>> + phy_power_on(dsim->phy);
>>>>>
>>>>> clk_enable(dsim->clock);
>>>>>
>>>>
>>>> This introduces the below with exynos_defconfig:
>>>>
>>>> ../../drivers/video/exynos/exynos_mipi_dsi.c: In function
>>>> 'exynos_mipi_dsi_blank_mode':
>>>> ../../drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning: unused
>>>> variable 'pdev' [-Wunused-variable]
>>>> struct platform_device *pdev = to_platform_device(dsim->dev);
>
> Sorry about missing that, I only noticed this warning recently and didn't
> get around to submit a patch.
>
>>> I have already submitted a patch to fix this [1]
>
> Thanks a lot guys for fixing this.
>
>>> [1] http://marc.info/?l=linux-fbdev&m\x138233359617936&w=2
>>
>> Sorry, missed that :-(
>
> This MIPI DSIM driver is affectively a dead code in the mainline now, once
> Exynos become a dt-only platform. I guess it can be deleted for 3.14, once
> S5P gets converted to the device tree. The new driver using CDF is basically
> a complete rewrite. Or device tree support should be added to that driver,
> but I believe it doesn't make sense without CDF.
>
MIPI DSIM driver is not a dead code. There is a steady trickle of patches.
It's kind of late, but, I will update it as DT based drivers as soon as possible.
And Why do you think that DT support of existing MIPI DSIM is something less
than great?
> --
> Regards,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
From: Tomasz Figa @ 2013-10-28 12:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <526E0038.7050805@samsung.com>
Hi Donghwa,
On Monday 28 of October 2013 15:12:08 Donghwa Lee wrote:
> On Fri, OCT 25, 2013 06:57, Sylwester Nawrocki wrote:
> > On 10/24/2013 05:57 PM, Kishon Vijay Abraham I wrote:
> >> On Thursday 24 October 2013 09:12 PM, Sachin Kamat wrote:
> >>> On 24 October 2013 20:00, Olof Johansson<olof@lixom.net> wrote:
> >>>> On Wed, Oct 16, 2013 at 9:28 AM, Kishon Vijay Abraham I<kishon@ti.com> wrote:
> >>>>> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> >>>>> index 32e5406..00b3a52 100644
> >>>>> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> >>>>> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> >>>>> @@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
> >>>>> exynos_mipi_regulator_enable(dsim);
> >>>>>
> >>>>> /* enable MIPI-DSI PHY. */
> >>>>> - if (dsim->pd->phy_enable)
> >>>>> - dsim->pd->phy_enable(pdev, true);
> >>>>> + phy_power_on(dsim->phy);
> >>>>>
> >>>>> clk_enable(dsim->clock);
> >>>>>
> >>>>
> >>>> This introduces the below with exynos_defconfig:
> >>>>
> >>>> ../../drivers/video/exynos/exynos_mipi_dsi.c: In function
> >>>> 'exynos_mipi_dsi_blank_mode':
> >>>> ../../drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning: unused
> >>>> variable 'pdev' [-Wunused-variable]
> >>>> struct platform_device *pdev = to_platform_device(dsim->dev);
> >
> > Sorry about missing that, I only noticed this warning recently and didn't
> > get around to submit a patch.
> >
> >>> I have already submitted a patch to fix this [1]
> >
> > Thanks a lot guys for fixing this.
> >
> >>> [1] http://marc.info/?l=linux-fbdev&m\x138233359617936&w=2
> >>
> >> Sorry, missed that :-(
> >
> > This MIPI DSIM driver is affectively a dead code in the mainline now, once
> > Exynos become a dt-only platform. I guess it can be deleted for 3.14, once
> > S5P gets converted to the device tree. The new driver using CDF is basically
> > a complete rewrite. Or device tree support should be added to that driver,
> > but I believe it doesn't make sense without CDF.
> >
>
> MIPI DSIM driver is not a dead code. There is a steady trickle of patches.
> It's kind of late, but, I will update it as DT based drivers as soon as possible.
> And Why do you think that DT support of existing MIPI DSIM is something less
> than great?
First of all, the exynos_mipi_dsim driver has currently no users in
mainline kernel, so it is essentially dead code. In addition, on
a platform that is the primary candidate for using it, which is Exynos,
there is no way to use it, due to no DT support.
As for the driver itself, it is not really a great example of good code.
It contains a hacks, like calling msleep() without any clear reason and
also many coding style issues. I'd prefer to replace it with the new
exynos-dsi driver rewritten completely in SRPOL, when CDF is finished.
Best regards,
Tomasz
^ permalink raw reply
* linux-next: build failure after merge of the final tree (fbdev tree related)
From: Stephen Rothwell @ 2013-10-28 14:23 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD, Florian Tobias Schandinat,
Tomi Valkeinen, linux-fbdev
Cc: linux-next, linux-kernel, Michal Simek
In-Reply-To: <20110520163233.abcabd67.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]
Hi all,
After merging the final tree, today's linux-next build (powerpc
ppc44x_defconfig) failed like this:
drivers/video/xilinxfb.c: In function 'xilinxfb_of_probe':
drivers/video/xilinxfb.c:431:30: error: 'op' undeclared (first use in this function)
Caused by commit 353846fb8bb7 ("video: xilinxfb: Use standard variable
name convention") from the fbdev tree.
I have applied the following fix patch for today:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 29 Oct 2013 01:18:22 +1100
Subject: [PATCH] video: xilinxfb: Fix for "Use standard variable name convention"
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
drivers/video/xilinxfb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 9eedf9673b7f..6ff1a91e9dfd 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -428,11 +428,11 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
#ifdef CONFIG_PPC_DCR
else {
int start;
- start = dcr_resource_start(op->dev.of_node, 0);
- drvdata->dcr_len = dcr_resource_len(op->dev.of_node, 0);
- drvdata->dcr_host = dcr_map(op->dev.of_node, start, drvdata->dcr_len);
+ start = dcr_resource_start(pdev->dev.of_node, 0);
+ drvdata->dcr_len = dcr_resource_len(pdev->dev.of_node, 0);
+ drvdata->dcr_host = dcr_map(pdev->dev.of_node, start, drvdata->dcr_len);
if (!DCR_MAP_OK(drvdata->dcr_host)) {
- dev_err(&op->dev, "invalid DCR address\n");
+ dev_err(&pdev->dev, "invalid DCR address\n");
return -ENODEV;
}
}
--
1.8.4.rc3
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: linux-next: build failure after merge of the final tree (fbdev tree related)
From: Michal Simek @ 2013-10-28 14:39 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Jean-Christophe PLAGNIOL-VILLARD, Florian Tobias Schandinat,
Tomi Valkeinen, linux-fbdev, linux-next, linux-kernel,
Michal Simek
In-Reply-To: <20131029012317.b9ba8173dee5bac1a5bd2404@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]
On 10/28/2013 03:23 PM, Stephen Rothwell wrote:
> Hi all,
>
> After merging the final tree, today's linux-next build (powerpc
> ppc44x_defconfig) failed like this:
>
> drivers/video/xilinxfb.c: In function 'xilinxfb_of_probe':
> drivers/video/xilinxfb.c:431:30: error: 'op' undeclared (first use in this function)
>
> Caused by commit 353846fb8bb7 ("video: xilinxfb: Use standard variable
> name convention") from the fbdev tree.
>
> I have applied the following fix patch for today:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 29 Oct 2013 01:18:22 +1100
> Subject: [PATCH] video: xilinxfb: Fix for "Use standard variable name convention"
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/video/xilinxfb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index 9eedf9673b7f..6ff1a91e9dfd 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -428,11 +428,11 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
> #ifdef CONFIG_PPC_DCR
> else {
> int start;
> - start = dcr_resource_start(op->dev.of_node, 0);
> - drvdata->dcr_len = dcr_resource_len(op->dev.of_node, 0);
> - drvdata->dcr_host = dcr_map(op->dev.of_node, start, drvdata->dcr_len);
> + start = dcr_resource_start(pdev->dev.of_node, 0);
> + drvdata->dcr_len = dcr_resource_len(pdev->dev.of_node, 0);
> + drvdata->dcr_host = dcr_map(pdev->dev.of_node, start, drvdata->dcr_len);
> if (!DCR_MAP_OK(drvdata->dcr_host)) {
> - dev_err(&op->dev, "invalid DCR address\n");
> + dev_err(&pdev->dev, "invalid DCR address\n");
> return -ENODEV;
> }
> }
>
Your fix is correct.
Tested-by: Michal Simek <monstr@monstr.eu>
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* [PATCH 00/12] Prepare various SH/R Mobile/Car drivers for CCF migration
From: Laurent Pinchart @ 2013-10-28 22:49 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-sh, Mike Turquette, Daniel Lezcano, linux-kernel, Tejun Heo,
linux-ide, David Airlie, dri-devel, Wolfram Sang, linux-i2c,
Chris Ball, Guennadi Liakhovetski, linux-mmc, Felipe Balbi,
linux-usb, Greg Kroah-Hartman, Yoshihiro Shimoda,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev
Hello,
This patch series, based on v3.12-rc7, prepares various Renesas SH-Mobile,
R-Mobile and R-Car drivers for migration to CCF by adding clock prepare and
unprepare support.
The patches are pretty straightforward. Most of the drivers called
clk_enable and clk_disable in sleepable context, I've thus just converted them
to clk_prepare_enable and clk_disable_unprepare.
The clocksource drivers were slightly more complex to handle as clk_enable and
clk_disable were called in non-sleepable context. As the clocksource framework
architecture doesn't provide any sleepable callback in which clocks could be
prepared and unprepared, I've added clk_prepare and clk_unprepare calls in the
probe and suspend/resume handlers (clocksources can't be removed so the remove
handler doesn't need a clk_unprepare call).
I'd like to get all these patches merged in v3.14. As they will need to go
through their respective subsystems' trees, I would appreciate if all
maintainers involved could notify me when they merge patches from this series
in their tree to help me tracking the merge status. I don't plan to send pull
requests individually for these patches, and I will repost patches
individually if changes are requested during review.
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-kernel@vger.kernel.org
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-ide@vger.kernel.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
Cc: Chris Ball <cjb@laptop.org>
Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Cc: linux-mmc@vger.kernel.org
Cc: Felipe Balbi <balbi@ti.com>
Cc: linux-usb@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Laurent Pinchart (12):
clocksource: sh_cmt: Add clk_prepare/unprepare support
clocksource: sh_mtu2: Add clk_prepare/unprepare support
clocksource: sh_tmu: Add clk_prepare/unprepare support
sata_rcar: Convert to clk_prepare/unprepare
drm: shmob_drm: Convert to clk_prepare/unprepare
i2c: sh_mobile: Convert to clk_prepare/unprepare
mmc: sh_mmcif: Convert to clk_prepare/unprepare
mmc: sh_mobile_sdhi: Convert to clk_prepare/unprepare
usb: gadget: r8a66597-udc: Convert to clk_prepare/unprepare
usb: r8a66597-hcd: Convert to clk_prepare/unprepare
fbdev: shmobile-hdmi: Convert to clk_prepare/unprepare
fbdev: shmobile-lcdcfb: Convert to clk_prepare/unprepare
drivers/ata/sata_rcar.c | 10 +++++-----
drivers/clocksource/sh_cmt.c | 20 ++++++++++++++++----
drivers/clocksource/sh_mtu2.c | 16 ++++++++++++++--
drivers/clocksource/sh_tmu.c | 20 +++++++++++++++++---
drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 4 ++--
drivers/i2c/busses/i2c-sh_mobile.c | 8 ++++----
drivers/mmc/host/sh_mmcif.c | 12 ++++++------
drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
drivers/usb/gadget/r8a66597-udc.c | 6 +++---
drivers/usb/host/r8a66597-hcd.c | 4 ++--
drivers/video/sh_mobile_hdmi.c | 6 +++---
drivers/video/sh_mobile_lcdcfb.c | 4 ++--
12 files changed, 76 insertions(+), 38 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 11/12] fbdev: shmobile-hdmi: Convert to clk_prepare/unprepare
From: Laurent Pinchart @ 2013-10-28 22:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383000569-8916-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
clk_disable_unprepare() to get ready for the migration to the common
clock framework.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/video/sh_mobile_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index bfe4728..190145e 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -1326,7 +1326,7 @@ static int __init sh_hdmi_probe(struct platform_device *pdev)
goto erate;
}
- ret = clk_enable(hdmi->hdmi_clk);
+ ret = clk_prepare_enable(hdmi->hdmi_clk);
if (ret < 0) {
dev_err(hdmi->dev, "Cannot enable clock: %d\n", ret);
goto erate;
@@ -1404,7 +1404,7 @@ emap_htop1:
emap:
release_mem_region(res->start, resource_size(res));
ereqreg:
- clk_disable(hdmi->hdmi_clk);
+ clk_disable_unprepare(hdmi->hdmi_clk);
erate:
clk_put(hdmi->hdmi_clk);
egetclk:
@@ -1427,7 +1427,7 @@ static int __exit sh_hdmi_remove(struct platform_device *pdev)
cancel_delayed_work_sync(&hdmi->edid_work);
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
- clk_disable(hdmi->hdmi_clk);
+ clk_disable_unprepare(hdmi->hdmi_clk);
clk_put(hdmi->hdmi_clk);
if (hdmi->htop1)
iounmap(hdmi->htop1);
--
1.8.1.5
^ permalink raw reply related
* [PATCH 12/12] fbdev: shmobile-lcdcfb: Convert to clk_prepare/unprepare
From: Laurent Pinchart @ 2013-10-28 22:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383000569-8916-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Turn clk_enable() and clk_disable() calls into clk_prepare_enable() and
clk_disable_unprepare() to get ready for the migration to the common
clock framework.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 0264704..eaeae0f 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -344,7 +344,7 @@ static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
{
if (atomic_inc_and_test(&priv->hw_usecnt)) {
if (priv->dot_clk)
- clk_enable(priv->dot_clk);
+ clk_prepare_enable(priv->dot_clk);
pm_runtime_get_sync(priv->dev);
if (priv->meram_dev && priv->meram_dev->pdev)
pm_runtime_get_sync(&priv->meram_dev->pdev->dev);
@@ -358,7 +358,7 @@ static void sh_mobile_lcdc_clk_off(struct sh_mobile_lcdc_priv *priv)
pm_runtime_put_sync(&priv->meram_dev->pdev->dev);
pm_runtime_put(priv->dev);
if (priv->dot_clk)
- clk_disable(priv->dot_clk);
+ clk_disable_unprepare(priv->dot_clk);
}
}
--
1.8.1.5
^ permalink raw reply related
* [PATCH 19/19] fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms
From: Laurent Pinchart @ 2013-10-28 23:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1383004027-25036-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Renesas ARM platforms are transitioning from single-platform to
multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the
driver available on all ARM platforms to enable it on both ARCH_SHMOBILE
and ARCH_SHMOBILE_MULTI and increase build testing coverage.
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/video/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 84b685f..681858e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -10,7 +10,7 @@ config HAVE_FB_ATMEL
config SH_MIPI_DSI
tristate
- depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
+ depends on (SUPERH || ARM) && HAVE_CLK
config SH_LCD_MIPI_DSI
bool
@@ -1995,7 +1995,7 @@ config FB_W100
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
- depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
+ depends on FB && (SUPERH || ARM) && HAVE_CLK
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -2482,7 +2482,7 @@ endif
config FB_SH_MOBILE_MERAM
tristate "SuperH Mobile MERAM read ahead support"
- depends on (SUPERH || ARCH_SHMOBILE)
+ depends on (SUPERH || ARM)
select GENERIC_ALLOCATOR
---help---
Enable MERAM support for the SuperH controller.
--
1.8.1.5
^ permalink raw reply related
* outcome of DRM/KMS DT bindings session
From: Dave Airlie @ 2013-10-29 3:52 UTC (permalink / raw)
To: dri-devel, ksummit-2013-discuss, Linux Fbdev development list
So we had a sessions at kernel summit to discuss the driver model and
DT interactions for a display pipeline,
we had good attendance from a few sides and I hope to summarise the
recommendations below,
a) Device Tree bindings
We should create a top-level virtual device binding that a top level
driver can bind to, like alsa asoc does.
We should separate the CDF device tree model from CDF as a starting
point and refine it outside of CDF, and produce a set of bindings that
cover the current drivers we have, exynos, imx, tegra, msm, armada
etc. This set of bindings should not be tied on CDF being merged or
anything else.
Display pipelines should be modelered in the device tree, but the
level of detail required for links between objects may be left up to
the SoC developer, esp wrt tightly coupled SoCs.
Externally linked devices like bridges and panels should be explicitly linked.
b) Driver Model
The big thing here is that the device tree description we use should
not dictate the driver model we use. This is the biggest thing I
learned, so what does it mean?
We aren't required to write a device driver per device tree object.
We shouldn't be writing device drivers per device tree object.
For tightly-coupled SoCs where the blocks come from one vendor and are
reused a lot, a top level driver should use the DT as configuration
information source for the list of blocks it needs to initialise on
the card, not as a list of separate drivers. There may be some
external drivers required and the code should deal with this, like how
alsa asoc does.
To share code between layers we should refactor it into a helper
library not a separate driver, the kms/v4l/fbdev can use the library.
This should allow us to move forward a bit clearer esp with new
drivers and following these recommendations, and I think porting
current drivers to a sane model, especially exynos and imx.
Now I saw we here but I'm only going to be donating my use of a big
stick and review abilities to making this happen, but I'm quite
willing to enforce some of these rules going forward as I think it
will make life easier.
After looking at some of the ordering issues we've had with x86 GPUs
(which are really just a tightly coupled SoC) I don't want separate
drivers all having their own init, suspend/resume paths in them as I
know we'll have to start making special vtable entry points etc to
solve some random ordering issues that crop up.
Dave.
^ permalink raw reply
* Re: [PATCH 3/7] video: exynos_mipi_dsim: Use the generic PHY driver
From: Donghwa Lee @ 2013-10-29 8:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <23467785.uRr31aFEN8@amdc1227>
Hi Tomasz,
On Tue, OCT 28, 2013 21:24, Tomasz Figa wrote:
> Hi Donghwa,
>
> On Monday 28 of October 2013 15:12:08 Donghwa Lee wrote:
>> On Fri, OCT 25, 2013 06:57, Sylwester Nawrocki wrote:
>>> On 10/24/2013 05:57 PM, Kishon Vijay Abraham I wrote:
>>>> On Thursday 24 October 2013 09:12 PM, Sachin Kamat wrote:
>>>>> On 24 October 2013 20:00, Olof Johansson<olof@lixom.net> wrote:
>>>>>> On Wed, Oct 16, 2013 at 9:28 AM, Kishon Vijay Abraham I<kishon@ti.com> wrote:
>>>>>>> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
>>>>>>> index 32e5406..00b3a52 100644
>>>>>>> --- a/drivers/video/exynos/exynos_mipi_dsi.c
>>>>>>> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
>>>>>>> @@ -156,8 +157,7 @@ static int exynos_mipi_dsi_blank_mode(struct mipi_dsim_device *dsim, int power)
>>>>>>> exynos_mipi_regulator_enable(dsim);
>>>>>>>
>>>>>>> /* enable MIPI-DSI PHY. */
>>>>>>> - if (dsim->pd->phy_enable)
>>>>>>> - dsim->pd->phy_enable(pdev, true);
>>>>>>> + phy_power_on(dsim->phy);
>>>>>>>
>>>>>>> clk_enable(dsim->clock);
>>>>>>>
>>>>>> This introduces the below with exynos_defconfig:
>>>>>>
>>>>>> ../../drivers/video/exynos/exynos_mipi_dsi.c: In function
>>>>>> 'exynos_mipi_dsi_blank_mode':
>>>>>> ../../drivers/video/exynos/exynos_mipi_dsi.c:144:26: warning: unused
>>>>>> variable 'pdev' [-Wunused-variable]
>>>>>> struct platform_device *pdev = to_platform_device(dsim->dev);
>>> Sorry about missing that, I only noticed this warning recently and didn't
>>> get around to submit a patch.
>>>
>>>>> I have already submitted a patch to fix this [1]
>>> Thanks a lot guys for fixing this.
>>>
>>>>> [1] http://marc.info/?l=linux-fbdev&m\x138233359617936&w=2
>>>> Sorry, missed that :-(
>>> This MIPI DSIM driver is affectively a dead code in the mainline now, once
>>> Exynos become a dt-only platform. I guess it can be deleted for 3.14, once
>>> S5P gets converted to the device tree. The new driver using CDF is basically
>>> a complete rewrite. Or device tree support should be added to that driver,
>>> but I believe it doesn't make sense without CDF.
>>>
>> MIPI DSIM driver is not a dead code. There is a steady trickle of patches.
>> It's kind of late, but, I will update it as DT based drivers as soon as possible.
>> And Why do you think that DT support of existing MIPI DSIM is something less
>> than great?
> First of all, the exynos_mipi_dsim driver has currently no users in
> mainline kernel, so it is essentially dead code. In addition, on
> a platform that is the primary candidate for using it, which is Exynos,
> there is no way to use it, due to no DT support.
As I mentioned above, patches are submitted sometimes and I will update
this driver as soon as possible to support DT.
> As for the driver itself, it is not really a great example of good code.
> It contains a hacks, like calling msleep() without any clear reason and
> also many coding style issues. I'd prefer to replace it with the new
> exynos-dsi driver rewritten completely in SRPOL, when CDF is finished.
Yes, I know this drivers had been changed about only minor issues and
it is not really good code style. And CDF is more good and light.
But discussion for CDF is still remaining a kind of requests. If it is merged
into linux kernel and many users use it, existing MIPI DSI drivers will be
replaced with the new drivers naturally, isn't it?
Until that, I and quite a few users will update and code re-factory for
this drivers to be more better.
BR,
Donghwa Lee
> Best regards,
> Tomasz
>
>
^ permalink raw reply
* Re: linux-next: build failure after merge of the final tree (fbdev tree related)
From: Tomi Valkeinen @ 2013-10-29 8:50 UTC (permalink / raw)
To: Stephen Rothwell, Jean-Christophe PLAGNIOL-VILLARD,
Florian Tobias Schandinat, linux-fbdev
Cc: linux-next, linux-kernel, Michal Simek
In-Reply-To: <20131029012317.b9ba8173dee5bac1a5bd2404@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]
On 28/10/13 16:23, Stephen Rothwell wrote:
> Hi all,
>
> After merging the final tree, today's linux-next build (powerpc
> ppc44x_defconfig) failed like this:
>
> drivers/video/xilinxfb.c: In function 'xilinxfb_of_probe':
> drivers/video/xilinxfb.c:431:30: error: 'op' undeclared (first use in this function)
>
> Caused by commit 353846fb8bb7 ("video: xilinxfb: Use standard variable
> name convention") from the fbdev tree.
>
> I have applied the following fix patch for today:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 29 Oct 2013 01:18:22 +1100
> Subject: [PATCH] video: xilinxfb: Fix for "Use standard variable name convention"
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/video/xilinxfb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
> index 9eedf9673b7f..6ff1a91e9dfd 100644
> --- a/drivers/video/xilinxfb.c
> +++ b/drivers/video/xilinxfb.c
> @@ -428,11 +428,11 @@ static int xilinxfb_of_probe(struct platform_device *pdev)
> #ifdef CONFIG_PPC_DCR
> else {
> int start;
> - start = dcr_resource_start(op->dev.of_node, 0);
> - drvdata->dcr_len = dcr_resource_len(op->dev.of_node, 0);
> - drvdata->dcr_host = dcr_map(op->dev.of_node, start, drvdata->dcr_len);
> + start = dcr_resource_start(pdev->dev.of_node, 0);
> + drvdata->dcr_len = dcr_resource_len(pdev->dev.of_node, 0);
> + drvdata->dcr_host = dcr_map(pdev->dev.of_node, start, drvdata->dcr_len);
> if (!DCR_MAP_OK(drvdata->dcr_host)) {
> - dev_err(&op->dev, "invalid DCR address\n");
> + dev_err(&pdev->dev, "invalid DCR address\n");
> return -ENODEV;
> }
> }
>
Thanks, I have applied this to the fbdev tree.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: fix error return code in metronomefb_probe()
From: Tomi Valkeinen @ 2013-10-29 10:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAPgLHd-FgU-512ivq_eRPqefqb8-7qGY8aoOATiFNZYn-P_4rg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 891 bytes --]
On 12/10/13 09:25, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Fix to return a negative error code from the error handling
> case instead of 0, as done elsewhere in this function.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/video/metronomefb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
> index f30150d..0b36160 100644
> --- a/drivers/video/metronomefb.c
> +++ b/drivers/video/metronomefb.c
> @@ -690,7 +690,8 @@ static int metronomefb_probe(struct platform_device *dev)
> goto err_csum_table;
> }
>
> - if (board->setup_irq(info))
> + retval = board->setup_irq(info);
> + if (retval)
> goto err_csum_table;
>
> retval = metronome_init_regs(par);
>
Thanks, queued for 3.13.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
From: Tomi Valkeinen @ 2013-10-29 10:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1382532229-32755-1-git-send-email-denis@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 2393 bytes --]
On 23/10/13 15:43, Denis Carikli wrote:
> Without that fix, drivers using the fb_videomode_from_videomode
> function will not be able to get certain information because
> some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> ChangeLog v2->v3:
> - Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
> ---
> drivers/video/fbmon.c | 4 ++++
> include/uapi/linux/fb.h | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index 6103fa6..29a9ed0 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
> fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
> fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> + fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
> + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
> if (vm->flags & DISPLAY_FLAGS_INTERLACED)
> fbmode->vmode |= FB_VMODE_INTERLACED;
> if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> index fb795c3..30487df 100644
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h
> @@ -215,6 +215,8 @@ struct fb_bitfield {
> /* vtotal = 144d/288n/576i => PAL */
> /* vtotal = 121d/242n/484i => NTSC */
> #define FB_SYNC_ON_GREEN 32 /* sync on green */
> +#define FB_SYNC_DE_HIGH_ACT 64 /* data enable high active */
> +#define FB_SYNC_PIXDAT_HIGH_ACT 64 /* data enable high active */
This can't be right. You map both flags to value 64. And the comment is
the same for both.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox