Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomi Valkeinen @ 2013-04-30 11:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1547536.Us4pVspuI2@avalon>

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On 04/30/2013 02:42 PM, Laurent Pinchart wrote:
> On Tuesday 30 April 2013 12:28:42 Arnd Bergmann wrote:
>> On Tuesday 30 April 2013, Tomi Valkeinen wrote:
>>> The bootloader would init the display hardware, the bootfb would give an
>>> early /dev/fb0 for the kernel and userspace, and when the real display
>>> driver is loaded, the bootfb would be unbound and the real driver would
>>> take over.
> 
> This could be done with a KMS driver as well ;-)

Right, I forgot to mention that. I had it in mind also =).

DRM has the same problem as fbdev with multi-arch and lots of display
and panel drivers.

Probably the same bootfb could be used for DRM also. Or do you see any
benefit of having a "bootkms" driver, or such?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomi Valkeinen @ 2013-04-30 11:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201304301228.42245.arnd@arndb.de>

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On 04/30/2013 01:28 PM, Arnd Bergmann wrote:
> On Tuesday 30 April 2013, Tomi Valkeinen wrote:
> 
>> The bootloader would init the display hardware, the bootfb would give an
>> early /dev/fb0 for the kernel and userspace, and when the real display
>> driver is loaded, the bootfb would be unbound and the real driver would
>> take over.
> 
> I think that's a great idea. What I'm not sure about is how that
> infrastructure for switching frame buffers would work and how hard
> it's to write.

I don't have any clear ideas either, just some vague ones:

I think the simplest option would be for the userspace to just unbind
the fbcon, then the bootfb device/driver, and then load the real driver.
I don't see why that would not work, but it's far from optimal. The fb
memory would become unallocated for a while, and the user could see
garbage on the display.

A proper hand-over would be more complex. So, I don't know... Maybe the
bootfb driver could have custom API for this. When the real driver is
loaded, it'd call the bootfb to get the fb memory, and to unbind the bootfb.

Would there be issues with passing the fb memory? If one uses dma_alloc,
the device is linked to the allocated memory, so I presume you can't
just pass that around and remove the original device.

Then again, dma_alloc would not be used here, as bootfb needs the fb
memory from a particular location, so I guess bootmem is needed here.

I think it would be reasonable to have a restriction of only a single
bootfb instance, so one could just use static global variables/funcs to
manage the hand-over.

All this, of course, presumes that nobody else than fbcon is using the
fb. But I think it's also a reasonable restriction that the fb device is
not used (mmapped) by anyone.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-30 11:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201304301228.42245.arnd@arndb.de>

On Tuesday 30 April 2013 12:28:42 Arnd Bergmann wrote:
> On Tuesday 30 April 2013, Tomi Valkeinen wrote:
> > The bootloader would init the display hardware, the bootfb would give an
> > early /dev/fb0 for the kernel and userspace, and when the real display
> > driver is loaded, the bootfb would be unbound and the real driver would
> > take over.

This could be done with a KMS driver as well ;-)

> I think that's a great idea. What I'm not sure about is how that
> infrastructure for switching frame buffers would work and how hard it's to
> write.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Arnd Bergmann @ 2013-04-30 10:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <517F753F.5090909@ti.com>

On Tuesday 30 April 2013, Tomi Valkeinen wrote:
> > +     framebuffer {
> > +             compatible = "simple-framebuffer";
> > +             reg = <0x1d385000 (1600 * 1200 * 2)>;
> > +             width = <1600>;
> > +             height = <1200>;
> > +             stride = <(1600 * 2)>;
> > +             format = "r5g6b5";
> > +     };
> 
> I'm not an expert on DT, but I think the point of DT is to describe the
> hardware. This doesn't describe the hardware at all.

That's ok.

It's not uncommon to have settings in the device tree that describe how
hardware is set up. Other similar properties would be the line rate
of a serial port, or a keymap describing what each button is labeled.
They are not physical properties, but they are necessary platform specific
pieces of information that are not available otherwise.

	Arnd

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Arnd Bergmann @ 2013-04-30 10:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <517F7245.2050606@iki.fi>

On Tuesday 30 April 2013, Tomi Valkeinen wrote:

> The bootloader would init the display hardware, the bootfb would give an
> early /dev/fb0 for the kernel and userspace, and when the real display
> driver is loaded, the bootfb would be unbound and the real driver would
> take over.

I think that's a great idea. What I'm not sure about is how that
infrastructure for switching frame buffers would work and how hard
it's to write.

	Arnd

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-30  9:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOesGMiCvBwck0Z6Nyo9zW=1=J=wjhJW68qNGMrN_8iim=8Srg@mail.gmail.com>

Hi Olof,

On Monday 29 April 2013 15:40:44 Olof Johansson wrote:
> On Mon, Apr 29, 2013 at 3:23 PM, Laurent Pinchart wrote:
> > On Tuesday 30 April 2013 00:04:20 Arnd Bergmann wrote:
> >> On Monday 29 April 2013, Laurent Pinchart wrote:
> >> > On Monday 29 April 2013 23:31:30 Tomasz Figa wrote:
> >> > > Good point. Stephen, would it be a problem to make this a KMS driver
> >> > > instead? Old fbdev API could be emulated on top of it, until it goes
> >> > > out of use, couldn't it?
> >> > 
> >> > There's already an fbdev emulation layer in KMS, for such a simple use
> >> > case it will work fine.
> >> 
> >> I suggested the same to Stephen when he first brought up this driver.
> >> Unfortunately his attempt to create a simple KMS driver resulted in a
> >> significantly larger driver than the one he did for the framebuffer
> >> interface. This means that either Stephen did something really wrong in
> >> his attempt, or the KMS interface isn't as good as it should be if we
> >> want to move people away from frame buffer drivers.
> > 
> > Implementing a KMS driver requires a fair amount of code. The DRM/KMS
> > subsystem provides helpers that significantly reduce the driver size.
> > Those helpers have been designed around common use cases found in existing
> > KMS drivers.
> > 
> > I'm pretty sure we will be able to add useful helpers for existing fbdev
> > drivers that will make porting them to KMS pretty easy, but that first
> > requires starting to port drivers to find out what common code could be
> > factored out.
> 
> Meanwhile this tiny driver will allow us to use hardware that can't
> otherwise be used (because the proper graphics drivers for said hardware is
> _also_ stuck behind large reworks, i.e. common display framework, which has
> uncertain progress at this time).

Point taken :-)

> As Stephen mentioned in the original patch, this particular driver is very
> useful for Raspberry Pi. But it is also the best way forward (right now) for
> getting basic upstream usability out of the Samsung ARM-based Chromebook. As
> a matter of fact, as of this weekend linux-next boots on that platform with
> keyboard, trackpad, framebuffer and USB2.0 without a single out-of-tree
> patch. Getting the same functionality out of 3.10 would be very desirable.
> 
> So, I clearly understand the desire to move over to a more modern framework.
> At the same time, there's definite value in merging this small driver while
> that's happening. Holding useful code hostage isn't always very productive.

I want to be clear about this, even though I believe we should put as much 
effort as possible behind KMS drivers, I won't nack this patch. It has real 
use cases and is useful, I'm just concerned it will get abused (but it's far 
from being the only piece of code that could be subject to abuse).

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomi Valkeinen @ 2013-04-30  7:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365043183-28905-1-git-send-email-swarren@wwwdotorg.org>

[-- Attachment #1: Type: text/plain, Size: 2694 bytes --]

On 04/04/2013 05:39 AM, Stephen Warren wrote:
> A simple frame-buffer describes a raw memory region that may be rendered
> to, with the assumption that the display hardware has already been set
> up to scan out from that buffer.
> 
> This is useful in cases where a bootloader exists and has set up the
> display hardware, but a Linux driver doesn't yet exist for the display
> hardware.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> v2: s/dumb/simple/ throughout. Provide more details on pixel format.
> 
> I ended up going with a separate FB driver:
> * DRM/KMS look much more complex, and don't provide any benefit that I can
>   tell for this simple driver.
> * Creating a separate driver rather than adjusting offb.c to work allows a
>   new clean binding to be defined, and doesn't require removing or ifdefing
>   PPC-isms in offb.c.
> ---
>  .../bindings/video/simple-framebuffer.txt          |   25 +++
>  drivers/video/Kconfig                              |   17 ++
>  drivers/video/Makefile                             |    1 +
>  drivers/video/simplefb.c                           |  234 ++++++++++++++++++++
>  4 files changed, 277 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer.txt
>  create mode 100644 drivers/video/simplefb.c
> 
> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> new file mode 100644
> index 0000000..3ea4605
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> @@ -0,0 +1,25 @@
> +Simple Framebuffer
> +
> +A simple frame-buffer describes a raw memory region that may be rendered to,
> +with the assumption that the display hardware has already been set up to scan
> +out from that buffer.
> +
> +Required properties:
> +- compatible: "simple-framebuffer"
> +- reg: Should contain the location and size of the framebuffer memory.
> +- width: The width of the framebuffer in pixels.
> +- height: The height of the framebuffer in pixels.
> +- stride: The number of bytes in each line of the framebuffer.
> +- format: The format of the framebuffer surface. Valid values are:
> +  - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
> +
> +Example:
> +
> +	framebuffer {
> +		compatible = "simple-framebuffer";
> +		reg = <0x1d385000 (1600 * 1200 * 2)>;
> +		width = <1600>;
> +		height = <1200>;
> +		stride = <(1600 * 2)>;
> +		format = "r5g6b5";
> +	};

I'm not an expert on DT, but I think the point of DT is to describe the
hardware. This doesn't describe the hardware at all.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomi Valkeinen @ 2013-04-30  7:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2863648.BDvrAG5uPk@flatron>

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On 04/30/2013 12:15 AM, Tomasz Figa wrote:

> Well, there is also at least one legitimate use case for this driver.
> 
> I believe there exist embedded devices on which there is no need to 
> dynamically control the framebuffer. It needs one time initialization, 
> usually in bootloader, and then it is used as is, using constant 
> parameters as long as the system is running.
> 
> I doubt there is a need for any KMS (or any other control) driver for such 
> devices - dumb framebuffer driver would be everything needed in such case.

Isn't there usually the need to power off the hardware when
blanking/suspending?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomi Valkeinen @ 2013-04-30  7:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1365043183-28905-1-git-send-email-swarren@wwwdotorg.org>

Hi,

On 04/04/2013 05:39 AM, Stephen Warren wrote:
> A simple frame-buffer describes a raw memory region that may be rendered
> to, with the assumption that the display hardware has already been set
> up to scan out from that buffer.
> 
> This is useful in cases where a bootloader exists and has set up the
> display hardware, but a Linux driver doesn't yet exist for the display
> hardware.

I had an idea related to this driver. There are two requirements that I
believe are (or will be) quite common:

On embedded devices quite often the bootloader initializes the display,
with the purpose of having a valid image on the screen for the whole
boot up process (company logo, or whatever).

With multi-arch kernels, we'll have display drivers for all platforms
and panels compiled with the kernel. If all these are built-in, they
will possibly increase the kernel size quite a bit, so it would be good
to have the display drivers as modules.

Now, if we have the display drivers as modules, the point when the
kernel/userspace can update the screen will be pretty late. Also,
there's no chance to get any early kernel boot messages on the screen.

So how about if we had this kind of simple fb, built-in to the kernel,
used only for the boot time until the proper driver is loaded. A "bootfb".

The bootloader would init the display hardware, the bootfb would give an
early /dev/fb0 for the kernel and userspace, and when the real display
driver is loaded, the bootfb would be unbound and the real driver would
take over.

 Tomi


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Olof Johansson @ 2013-04-29 22:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2040099.REU8D4ds1g@avalon>

On Mon, Apr 29, 2013 at 3:23 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Arnd,
>
> On Tuesday 30 April 2013 00:04:20 Arnd Bergmann wrote:
>> On Monday 29 April 2013, Laurent Pinchart wrote:
>> > On Monday 29 April 2013 23:31:30 Tomasz Figa wrote:
>> > > Good point. Stephen, would it be a problem to make this a KMS driver
>> > > instead? Old fbdev API could be emulated on top of it, until it goes out
>> > > of use, couldn't it?
>> >
>> > There's already an fbdev emulation layer in KMS, for such a simple use
>> > case it will work fine.
>>
>> I suggested the same to Stephen when he first brought up this driver.
>> Unfortunately his attempt to create a simple KMS driver resulted in a
>> significantly larger driver than the one he did for the framebuffer
>> interface. This means that either Stephen did something really wrong in his
>> attempt, or the KMS interface isn't as good as it should be if we want to
>> move people away from frame buffer drivers.
>
> Implementing a KMS driver requires a fair amount of code. The DRM/KMS
> subsystem provides helpers that significantly reduce the driver size. Those
> helpers have been designed around common use cases found in existing KMS
> drivers.
>
> I'm pretty sure we will be able to add useful helpers for existing fbdev
> drivers that will make porting them to KMS pretty easy, but that first
> requires starting to port drivers to find out what common code could be
> factored out.

Meanwhile this tiny driver will allow us to use hardware that can't
otherwise be used (because the proper graphics drivers for said
hardware is _also_ stuck behind large reworks, i.e. common display
framework, which has uncertain progress at this time).

As Stephen mentioned in the original patch, this particular driver is
very useful for Raspberry Pi. But it is also the best way forward
(right now) for getting basic upstream usability out of the Samsung
ARM-based Chromebook. As a matter of fact, as of this weekend
linux-next boots on that platform with keyboard, trackpad, framebuffer
and USB2.0 without a single out-of-tree patch. Getting the same
functionality out of 3.10 would be very desirable.

So, I clearly understand the desire to move over to a more modern
framework. At the same time, there's definite value in merging this
small driver while that's happening. Holding useful code hostage isn't
always very productive.


-Olof

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-29 22:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201304300004.20320.arnd@arndb.de>

Hi Arnd,

On Tuesday 30 April 2013 00:04:20 Arnd Bergmann wrote:
> On Monday 29 April 2013, Laurent Pinchart wrote:
> > On Monday 29 April 2013 23:31:30 Tomasz Figa wrote:
> > > Good point. Stephen, would it be a problem to make this a KMS driver
> > > instead? Old fbdev API could be emulated on top of it, until it goes out
> > > of use, couldn't it?
> > 
> > There's already an fbdev emulation layer in KMS, for such a simple use
> > case it will work fine.
> 
> I suggested the same to Stephen when he first brought up this driver.
> Unfortunately his attempt to create a simple KMS driver resulted in a
> significantly larger driver than the one he did for the framebuffer
> interface. This means that either Stephen did something really wrong in his
> attempt, or the KMS interface isn't as good as it should be if we want to
> move people away from frame buffer drivers.

Implementing a KMS driver requires a fair amount of code. The DRM/KMS 
subsystem provides helpers that significantly reduce the driver size. Those 
helpers have been designed around common use cases found in existing KMS 
drivers.

I'm pretty sure we will be able to add useful helpers for existing fbdev 
drivers that will make porting them to KMS pretty easy, but that first 
requires starting to port drivers to find out what common code could be 
factored out.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Arnd Bergmann @ 2013-04-29 22:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6607610.omg016OAvj@avalon>

On Monday 29 April 2013, Laurent Pinchart wrote:
> On Monday 29 April 2013 23:31:30 Tomasz Figa wrote:
>
> > Good point. Stephen, would it be a problem to make this a KMS driver
> > instead? Old fbdev API could be emulated on top of it, until it goes out
> > of use, couldn't it?
> 
> There's already an fbdev emulation layer in KMS, for such a simple use case it 
> will work fine.

I suggested the same to Stephen when he first brought up this driver.
Unfortunately his attempt to create a simple KMS driver resulted in a
significantly larger driver than the one he did for the framebuffer
interface. This means that either Stephen did something really wrong
in his attempt, or the KMS interface isn't as good as it should be
if we want to move people away from frame buffer drivers.

	Arnd

^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-29 21:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1556968.7tR6KuEIWb@flatron>

On Monday 29 April 2013 23:31:30 Tomasz Figa wrote:
> On Monday 29 of April 2013 23:20:43 Laurent Pinchart wrote:
> > On Monday 29 April 2013 23:15:13 Tomasz Figa wrote:
> > > On Thursday 11 of April 2013 11:56:31 Laurent Pinchart wrote:
> > > > On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> > > > > On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> > > > > > A simple frame-buffer describes a raw memory region that may be
> > > > > > rendered to, with the assumption that the display hardware has
> > > > > > already been set up to scan out from that buffer.
> > > > > > 
> > > > > > This is useful in cases where a bootloader exists and has set up
> > > > > > the display hardware, but a Linux driver doesn't yet exist for the
> > > > > > display hardware.
> > > > > > 
> > > > > > ...
> > > > > > 
> > > > > > +config FB_SIMPLE
> > > > > > +	bool "Simple framebuffer support"
> > > > > > +	depends on (FB = y) && OF
> > > > > 
> > > > > It's sad that this simple little thing requires Open Firmware.
> > > > > Could it be generalised in some way so that the small amount of
> > > > > setup info could be provided by other means (eg, module_param) or
> > > > > does the dependency go deeper than that?
> > > > 
> > > > I second that request. I like the idea of a simple framebuffer
> > > > driver if it helps deprecating fbdev in the long term, but I don't
> > > > want it to offer an excuse not to implement a DRM/KMS driver. In
> > > > particular adding DT bindings would force us to keep supporting the
> > > > ABI for a (too) long time.
> > > 
> > > Well, there is also at least one legitimate use case for this driver.
> > > 
> > > I believe there exist embedded devices on which there is no need to
> > > dynamically control the framebuffer. It needs one time initialization,
> > > usually in bootloader, and then it is used as is, using constant
> > > parameters as long as the system is running.
> > > 
> > > I doubt there is a need for any KMS (or any other control) driver for
> > > such devices - dumb framebuffer driver would be everything needed in
> > > such case.
> > 
> > As we want to deprecate the fbdev API we would need to move to KMS at
> > some point anyway, even if the device can't be controlled. I don't
> > think writting such a KMS driver would be that difficult.
> 
> Good point. Stephen, would it be a problem to make this a KMS driver
> instead? Old fbdev API could be emulated on top of it, until it goes out
> of use, couldn't it?

There's already an fbdev emulation layer in KMS, for such a simple use case it 
will work fine.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomasz Figa @ 2013-04-29 21:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1461633.kfcPFxva7x@avalon>

On Monday 29 of April 2013 23:20:43 Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Monday 29 April 2013 23:15:13 Tomasz Figa wrote:
> > On Thursday 11 of April 2013 11:56:31 Laurent Pinchart wrote:
> > > On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> > > > On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> > > > > A simple frame-buffer describes a raw memory region that may be
> > > > > rendered to, with the assumption that the display hardware has
> > > > > already been set up to scan out from that buffer.
> > > > > 
> > > > > This is useful in cases where a bootloader exists and has set up
> > > > > the
> > > > > display hardware, but a Linux driver doesn't yet exist for the
> > > > > display hardware.
> > > > > 
> > > > > ...
> > > > > 
> > > > > +config FB_SIMPLE
> > > > > +	bool "Simple framebuffer support"
> > > > > +	depends on (FB = y) && OF
> > > > 
> > > > It's sad that this simple little thing requires Open Firmware. 
> > > > Could
> > > > it be generalised in some way so that the small amount of setup
> > > > info
> > > > could be provided by other means (eg, module_param) or does the
> > > > dependency go deeper than that?
> > > 
> > > I second that request. I like the idea of a simple framebuffer
> > > driver if it helps deprecating fbdev in the long term, but I don't
> > > want it to offer an excuse not to implement a DRM/KMS driver. In
> > > particular adding DT bindings would force us to keep supporting the
> > > ABI for a (too) long time.
> > 
> > Well, there is also at least one legitimate use case for this driver.
> > 
> > I believe there exist embedded devices on which there is no need to
> > dynamically control the framebuffer. It needs one time initialization,
> > usually in bootloader, and then it is used as is, using constant
> > parameters as long as the system is running.
> > 
> > I doubt there is a need for any KMS (or any other control) driver for
> > such devices - dumb framebuffer driver would be everything needed in
> > such case.
> As we want to deprecate the fbdev API we would need to move to KMS at
> some point anyway, even if the device can't be controlled. I don't
> think writting such a KMS driver would be that difficult.

Good point. Stephen, would it be a problem to make this a KMS driver 
instead? Old fbdev API could be emulated on top of it, until it goes out 
of use, couldn't it?

Best regards,
Tomasz


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-29 21:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2863648.BDvrAG5uPk@flatron>

Hi Tomasz,

On Monday 29 April 2013 23:15:13 Tomasz Figa wrote:
> On Thursday 11 of April 2013 11:56:31 Laurent Pinchart wrote:
> > On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> > > On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> > > > A simple frame-buffer describes a raw memory region that may be
> > > > rendered to, with the assumption that the display hardware has
> > > > already been set up to scan out from that buffer.
> > > > 
> > > > This is useful in cases where a bootloader exists and has set up the
> > > > display hardware, but a Linux driver doesn't yet exist for the
> > > > display hardware.
> > > > 
> > > > ...
> > > > 
> > > > +config FB_SIMPLE
> > > > +	bool "Simple framebuffer support"
> > > > +	depends on (FB = y) && OF
> > > 
> > > It's sad that this simple little thing requires Open Firmware.  Could
> > > it be generalised in some way so that the small amount of setup info
> > > could be provided by other means (eg, module_param) or does the
> > > dependency go deeper than that?
> > 
> > I second that request. I like the idea of a simple framebuffer driver if
> > it helps deprecating fbdev in the long term, but I don't want it to
> > offer an excuse not to implement a DRM/KMS driver. In particular adding
> > DT bindings would force us to keep supporting the ABI for a (too) long
> > time.
> 
> Well, there is also at least one legitimate use case for this driver.
> 
> I believe there exist embedded devices on which there is no need to
> dynamically control the framebuffer. It needs one time initialization,
> usually in bootloader, and then it is used as is, using constant
> parameters as long as the system is running.
> 
> I doubt there is a need for any KMS (or any other control) driver for such
> devices - dumb framebuffer driver would be everything needed in such case.

As we want to deprecate the fbdev API we would need to move to KMS at some 
point anyway, even if the device can't be controlled. I don't think writting 
such a KMS driver would be that difficult.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Tomasz Figa @ 2013-04-29 21:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2569832.XQDrhUGNaI@avalon>

Hi Laurent,

On Thursday 11 of April 2013 11:56:31 Laurent Pinchart wrote:
> On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> > On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> > > A simple frame-buffer describes a raw memory region that may be
> > > rendered to, with the assumption that the display hardware has
> > > already been set up to scan out from that buffer.
> > > 
> > > This is useful in cases where a bootloader exists and has set up the
> > > display hardware, but a Linux driver doesn't yet exist for the
> > > display
> > > hardware.
> > > 
> > > ...
> > > 
> > > +config FB_SIMPLE
> > > +	bool "Simple framebuffer support"
> > > +	depends on (FB = y) && OF
> > 
> > It's sad that this simple little thing requires Open Firmware.  Could
> > it be generalised in some way so that the small amount of setup info
> > could be provided by other means (eg, module_param) or does the
> > dependency go deeper than that?
> 
> I second that request. I like the idea of a simple framebuffer driver if
> it helps deprecating fbdev in the long term, but I don't want it to
> offer an excuse not to implement a DRM/KMS driver. In particular adding
> DT bindings would force us to keep supporting the ABI for a (too) long
> time.

Well, there is also at least one legitimate use case for this driver.

I believe there exist embedded devices on which there is no need to 
dynamically control the framebuffer. It needs one time initialization, 
usually in bootloader, and then it is used as is, using constant 
parameters as long as the system is running.

I doubt there is a need for any KMS (or any other control) driver for such 
devices - dumb framebuffer driver would be everything needed in such case.

Best regards,
Tomasz


^ permalink raw reply

* Re: [PATCH V2] video: implement a simple framebuffer driver
From: Laurent Pinchart @ 2013-04-29 20:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51671F3D.5040501@wwwdotorg.org>

Hi Stephen,

Sorry for the late reply.

On Thursday 11 April 2013 14:38:21 Stephen Warren wrote:
> On 04/11/2013 02:06 PM, Laurent Pinchart wrote:
> > On Thursday 11 April 2013 10:06:49 Stephen Warren wrote:
> >> On 04/11/2013 03:56 AM, Laurent Pinchart wrote:
> >>> On Monday 08 April 2013 17:16:37 Andrew Morton wrote:
> >>>> On Wed,  3 Apr 2013 20:39:43 -0600 Stephen Warren wrote:
> >>>>> A simple frame-buffer describes a raw memory region that may be
> >>>>> rendered to, with the assumption that the display hardware has already
> >>>>> been set up to scan out from that buffer.
> >>>>> 
> >>>>> This is useful in cases where a bootloader exists and has set up the
> >>>>> display hardware, but a Linux driver doesn't yet exist for the display
> >>>>> hardware.
> >>>>> 
> >>>>> ...
> >>>>> 
> >>>>> +config FB_SIMPLE
> >>>>> +	bool "Simple framebuffer support"
> >>>>> +	depends on (FB = y) && OF
> >>>> 
> >>>> It's sad that this simple little thing requires Open Firmware.  Could
> >>>> it be generalised in some way so that the small amount of setup info
> >>>> could be provided by other means (eg, module_param) or does the
> >>>> dependency go deeper than that?
> >>> 
> >>> I second that request. I like the idea of a simple framebuffer driver if
> >>> it helps deprecating fbdev in the long term, but I don't want it to
> >>> offer an excuse not to implement a DRM/KMS driver. In particular adding
> >>> DT bindings would force us to keep supporting the ABI for a (too) long
> >>> time.
> >> 
> >> The platforms I intend to use this with only support device tree. Adding
> >> support for platform data right now would just be dead code. If somebody
> >> wants to use this driver with a board file based system rather than a DT
> >> based system, it would be trivial do modify the driver to add a platform
> >> data structure at that time.
> > 
> > What about using module parameters instead of DT bindings and platform
> > data ? As this is a hack designed to work around the lack of a proper
> > display driver the frame buffer address and size could just be passed by
> > the boot loader through the kernel command line, and the device would
> > then be instantiated by the driver.
> 
> I'm afraid I strongly dislike the idea of using module parameters. One
> of the things that I dislike the most about NVIDIA's downstream Android
> kernels is the huge number of random parameters that are required on the
> kernel command-line just to get it to boot.
> 
> I'd specifically prefer the configuration for this driver to be a device
> tree binding so that it /is/ an ABI. That way, arbitrary software that
> boots the Linux kernel will be able to target a common standard for how
> to pass this information to the kernel. If we move to module parameters
> instead, especially with the specific intent that the format of those
> parameters is no longer an ABI, we run in to issues where a new kernel
> requires a bootloader update in order to generate the new set of module
> parameters that are required by the driver. If we say the module
> parameters will never change except in a backwards-compatible fashion,
> and hence that issue won't arise, then why not just make it a DT binding?
> 
> Do module parameters handle the case of multiple device instances?

Not well. It can be handled manually by using parameter arrays, but that's a 
bit hackish.

> Also, I really don't see this driver as a hack. It's a perfectly reasonable
> situation to have some other SW set up the frame-buffer, and Linux simply
> continue to use it. Perhaps that other SW even ran on a difference CPU, or
> the parameters are hard-coded in HW design, and so there's no HW that Linux
> ever could drive, so it just isn't possible to create any more advanced
> driver.

I totally agree that this driver would be very useful during board bring-up. 
My concern is that it would also give an incentive to vendors not to develop a 
proper KMS driver (or rather remove an incentive to develop one).

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* [GIT PULL] fbdev changes for 3.10
From: Tomi Valkeinen @ 2013-04-29 11:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fbdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 6642 bytes --]

Hi Linus,

The following changes since commit 60d509fa6a9c4653a86ad830e4c4b30360b23f0e:

  Linux 3.9-rc8 (2013-04-21 14:38:45 -0700)

are available in the git repository at:

  git://gitorious.org/linux-omap-dss2/linux.git tags/fbdev-for-3.10

for you to fetch changes up to 9bf9d47a29afbf7a43eae74a988a4aefe88ccbfd:

  Merge branch '3.10/fb-mmap' into for-next (2013-04-26 09:14:47 +0300)

----------------------------------------------------------------

fbdev for 3.10

* use vm_iomap_memory() in various fb drivers to map the fb memory to userspace
* Cleanups for the videomode and display_timing features
* Updates to vt8500, wm8505 and auo-k190x fb drivers

----------------------------------------------------------------
Arnd Bergmann (2):
      video/exynos: remove unnecessary header inclusions
      video/s3c: move platform_data out of arch/arm

Fabio Porcedda (1):
      drivers: video: use module_platform_driver_probe()

Heiko Stübner (9):
      AUO-K190x: Use correct line length
      AUO-K190x: add runtime-pm calls to controller init functions
      AUO-K190x: set the correct runtime-pm state in recover
      AUO-K190x: make memory check in check_var more flexible
      AUO-K190x: move var resolution-handling into check_var
      AUO-K190x: make color handling more flexible
      AUO-K190x: add a 16bit truecolor mode
      AUO-K190x: add framebuffer rotation support
      AUO-K190x: Add resolutions for portrait displays

Julia Lawall (1):
      drivers/video/wm8505fb.c: use devm_ functions

Paul Bolle (1):
      ARM: OMAP: remove "config FB_OMAP_CONSISTENT_DMA_SIZE"

Sachin Kamat (1):
      video: wm8505fb: Convert to devm_ioremap_resource()

Timur Tabi (1):
      drivers/video: fsl-diu-fb: add hardware cursor support

Tomi Valkeinen (17):
      videomode: simplify videomode Kconfig and Makefile
      videomode: combine videomode dmt_flags and data_flags
      videomode: create enum for videomode's display flags
      videomode: remove timing_entry_index
      videomode: videomode_from_timing work
      fbdev: Merge fbdev topic branches
      video: vt8500: fix Kconfig for videomode
      fbdev/omapfb: use vm_iomap_memory()
      fbdev/controlfb: use vm_iomap_memory()
      fbdev/fb-puv3: use vm_iomap_memory()
      fbdev/sa1100fb: use vm_iomap_memory()
      fbdev/vermillion: use vm_iomap_memory()
      fbdev/sgivwfb: use vm_iomap_memory()
      fbdev/ps3fb: use vm_iomap_memory()
      fbdev: improve fb_mmap bounds checks
      fbdev: fix check for fb_mmap's mmio availability
      Merge branch '3.10/fb-mmap' into for-next

Tony Prisk (5):
      video: vt8500: Make wmt_ge_rops optional
      video: vt8500: Remove unused platform_data/video-vt8500lcdfb.h
      video: vt8500: Correct descriptions in video/Kconfig
      video: vt8500: Adjust contrast in wm8505 framebuffer driver.
      video: fb: vt8500: Convert framebuffer drivers to standardized binding

 .../devicetree/bindings/video/via,vt8500-fb.txt    |   48 +---
 .../devicetree/bindings/video/wm,wm8505-fb.txt     |   32 ++-
 arch/arm/boot/dts/vt8500-bv07.dts                  |   34 ++-
 arch/arm/boot/dts/vt8500.dtsi                      |    4 +-
 arch/arm/boot/dts/wm8505-ref.dts                   |   34 ++-
 arch/arm/boot/dts/wm8505.dtsi                      |    4 +-
 arch/arm/boot/dts/wm8650-mid.dts                   |   36 ++-
 arch/arm/boot/dts/wm8650.dtsi                      |    4 +-
 arch/arm/boot/dts/wm8850-w70v2.dts                 |   40 ++--
 arch/arm/boot/dts/wm8850.dtsi                      |    4 +-
 arch/arm/plat-samsung/include/plat/fb.h            |   50 +----
 drivers/gpu/drm/drm_modes.c                        |   20 +-
 drivers/gpu/drm/tilcdc/Kconfig                     |    3 +-
 drivers/gpu/drm/tilcdc/tilcdc_panel.c              |    2 +-
 drivers/video/Kconfig                              |   57 ++---
 drivers/video/Makefile                             |    8 +-
 drivers/video/amifb.c                              |   14 +-
 drivers/video/atmel_lcdfb.c                        |   13 +-
 drivers/video/auo_k1900fb.c                        |   11 +-
 drivers/video/auo_k1901fb.c                        |   11 +-
 drivers/video/auo_k190x.c                          |  235 ++++++++++++++++----
 drivers/video/controlfb.c                          |   50 ++---
 drivers/video/exynos/exynos_mipi_dsi.c             |    2 -
 drivers/video/exynos/exynos_mipi_dsi_common.c      |    2 -
 drivers/video/exynos/exynos_mipi_dsi_lowlevel.c    |    2 -
 drivers/video/fb-puv3.c                            |   14 +-
 drivers/video/fbmem.c                              |    5 +
 drivers/video/fbmon.c                              |   16 +-
 drivers/video/fsl-diu-fb.c                         |  157 ++++++++++++-
 drivers/video/gbefb.c                              |    4 +-
 drivers/video/of_display_timing.c                  |   19 +-
 drivers/video/of_videomode.c                       |    2 +-
 drivers/video/omap/Kconfig                         |   11 -
 drivers/video/omap2/omapfb/omapfb-main.c           |   30 +--
 drivers/video/omap2/vrfb.c                         |   13 +-
 drivers/video/ps3fb.c                              |   18 +-
 drivers/video/s3c-fb.c                             |    3 +-
 drivers/video/sa1100fb.c                           |   16 +-
 drivers/video/sgivwfb.c                            |   20 +-
 drivers/video/sh_mipi_dsi.c                        |   12 +-
 drivers/video/sh_mobile_hdmi.c                     |   12 +-
 drivers/video/smscufx.c                            |    6 +-
 drivers/video/udlfb.c                              |    6 +-
 drivers/video/vermilion/vermilion.c                |   14 +-
 drivers/video/vfb.c                                |    7 +-
 drivers/video/videomode.c                          |   36 +--
 drivers/video/vt8500lcdfb.c                        |   55 ++---
 drivers/video/wm8505fb.c                           |  145 ++++--------
 drivers/video/wmt_ge_rops.h                        |   23 ++
 include/linux/platform_data/video-vt8500lcdfb.h    |   31 ---
 include/linux/platform_data/video_s3c.h            |   54 +++++
 include/video/auo_k190xfb.h                        |    3 +-
 include/video/display_timing.h                     |   57 ++---
 include/video/videomode.h                          |   18 +-
 54 files changed, 799 insertions(+), 728 deletions(-)
 delete mode 100644 include/linux/platform_data/video-vt8500lcdfb.h
 create mode 100644 include/linux/platform_data/video_s3c.h


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* [PATCH 10/10] OMAPDSS: TFP410: return EPROBE_DEFER if the i2c adapter not found
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

If the I2C adapter needed by the TFP410 device is not available yet,
return EPROBE_DEFER so that the device will get probed again.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/displays/panel-tfp410.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/omap2/displays/panel-tfp410.c b/drivers/video/omap2/displays/panel-tfp410.c
index a1dba868..46039c4 100644
--- a/drivers/video/omap2/displays/panel-tfp410.c
+++ b/drivers/video/omap2/displays/panel-tfp410.c
@@ -135,7 +135,7 @@ static int tfp410_probe(struct omap_dss_device *dssdev)
 		if (!adapter) {
 			dev_err(&dssdev->dev, "Failed to get I2C adapter, bus %d\n",
 					i2c_bus_num);
-			return -EINVAL;
+			return -EPROBE_DEFER;
 		}
 
 		ddata->i2c_adapter = adapter;
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 09/10] OMAPDSS: HDMI: Add error handling for hdmi_probe_pdata
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Add proper error handling for hdmi_probe_pdata(). This will cause
EPROBE_DEFER to be properly passed upwards, causing the HDMI driver to be
probed again later if a resource was missing.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/hdmi.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 9aefe60..17f4d55 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -982,7 +982,7 @@ static struct omap_dss_device *hdmi_find_dssdev(struct platform_device *pdev)
 	return def_dssdev;
 }
 
-static void hdmi_probe_pdata(struct platform_device *pdev)
+static int hdmi_probe_pdata(struct platform_device *pdev)
 {
 	struct omap_dss_device *plat_dssdev;
 	struct omap_dss_device *dssdev;
@@ -992,11 +992,11 @@ static void hdmi_probe_pdata(struct platform_device *pdev)
 	plat_dssdev = hdmi_find_dssdev(pdev);
 
 	if (!plat_dssdev)
-		return;
+		return 0;
 
 	dssdev = dss_alloc_and_init_device(&pdev->dev);
 	if (!dssdev)
-		return;
+		return -ENOMEM;
 
 	dss_copy_device_pdata(dssdev, plat_dssdev);
 
@@ -1010,7 +1010,7 @@ static void hdmi_probe_pdata(struct platform_device *pdev)
 	if (r) {
 		DSSERR("device %s init failed: %d\n", dssdev->name, r);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = omapdss_output_set_device(&hdmi.output, dssdev);
@@ -1018,7 +1018,7 @@ static void hdmi_probe_pdata(struct platform_device *pdev)
 		DSSERR("failed to connect output to new device: %s\n",
 				dssdev->name);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = dss_add_device(dssdev);
@@ -1027,8 +1027,10 @@ static void hdmi_probe_pdata(struct platform_device *pdev)
 		omapdss_output_unset_device(&hdmi.output);
 		hdmi_uninit_display(dssdev);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
+
+	return 0;
 }
 
 static void hdmi_init_output(struct platform_device *pdev)
@@ -1096,7 +1098,13 @@ static int omapdss_hdmihw_probe(struct platform_device *pdev)
 
 	dss_debugfs_create_file("hdmi", hdmi_dump_regs);
 
-	hdmi_probe_pdata(pdev);
+	r = hdmi_probe_pdata(pdev);
+	if (r) {
+		hdmi_panel_exit();
+		hdmi_uninit_output(pdev);
+		pm_runtime_disable(&pdev->dev);
+		return r;
+	}
 
 	return 0;
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 08/10] OMAPDSS: HDMI: use platform_driver_register()
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Use platform_driver_register() instead of platform_driver_probe() so
that we can support EPROBE_DEFER.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/hdmi.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 7939309..9aefe60 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -328,7 +328,7 @@ static void hdmi_runtime_put(void)
 	WARN_ON(r < 0 && r != -ENOSYS);
 }
 
-static int __init hdmi_init_display(struct omap_dss_device *dssdev)
+static int hdmi_init_display(struct omap_dss_device *dssdev)
 {
 	int r;
 
@@ -954,7 +954,7 @@ int hdmi_audio_config(struct omap_dss_audio *audio)
 
 #endif
 
-static struct omap_dss_device * __init hdmi_find_dssdev(struct platform_device *pdev)
+static struct omap_dss_device *hdmi_find_dssdev(struct platform_device *pdev)
 {
 	struct omap_dss_board_info *pdata = pdev->dev.platform_data;
 	const char *def_disp_name = omapdss_get_default_display_name();
@@ -982,7 +982,7 @@ static struct omap_dss_device * __init hdmi_find_dssdev(struct platform_device *
 	return def_dssdev;
 }
 
-static void __init hdmi_probe_pdata(struct platform_device *pdev)
+static void hdmi_probe_pdata(struct platform_device *pdev)
 {
 	struct omap_dss_device *plat_dssdev;
 	struct omap_dss_device *dssdev;
@@ -1031,7 +1031,7 @@ static void __init hdmi_probe_pdata(struct platform_device *pdev)
 	}
 }
 
-static void __init hdmi_init_output(struct platform_device *pdev)
+static void hdmi_init_output(struct platform_device *pdev)
 {
 	struct omap_dss_output *out = &hdmi.output;
 
@@ -1052,7 +1052,7 @@ static void __exit hdmi_uninit_output(struct platform_device *pdev)
 }
 
 /* HDMI HW IP initialisation */
-static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
+static int omapdss_hdmihw_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int r;
@@ -1151,6 +1151,7 @@ static const struct dev_pm_ops hdmi_pm_ops = {
 };
 
 static struct platform_driver omapdss_hdmihw_driver = {
+	.probe		= omapdss_hdmihw_probe,
 	.remove         = __exit_p(omapdss_hdmihw_remove),
 	.driver         = {
 		.name   = "omapdss_hdmi",
@@ -1161,7 +1162,7 @@ static struct platform_driver omapdss_hdmihw_driver = {
 
 int __init hdmi_init_platform_driver(void)
 {
-	return platform_driver_probe(&omapdss_hdmihw_driver, omapdss_hdmihw_probe);
+	return platform_driver_register(&omapdss_hdmihw_driver);
 }
 
 void __exit hdmi_uninit_platform_driver(void)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 07/10] OMAPDSS: DPI: Add error handling for dpi_probe_pdata
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Add proper error handling for dpi_probe_pdata(). This will cause
EPROBE_DEFER to be properly passed upwards, causing the DPI driver to be
probed again later if a resource was missing.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dpi.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 2024787..757b57f 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -635,7 +635,7 @@ static struct omap_dss_device *dpi_find_dssdev(struct platform_device *pdev)
 	return def_dssdev;
 }
 
-static void dpi_probe_pdata(struct platform_device *dpidev)
+static int dpi_probe_pdata(struct platform_device *dpidev)
 {
 	struct omap_dss_device *plat_dssdev;
 	struct omap_dss_device *dssdev;
@@ -644,11 +644,11 @@ static void dpi_probe_pdata(struct platform_device *dpidev)
 	plat_dssdev = dpi_find_dssdev(dpidev);
 
 	if (!plat_dssdev)
-		return;
+		return 0;
 
 	dssdev = dss_alloc_and_init_device(&dpidev->dev);
 	if (!dssdev)
-		return;
+		return -ENOMEM;
 
 	dss_copy_device_pdata(dssdev, plat_dssdev);
 
@@ -656,7 +656,7 @@ static void dpi_probe_pdata(struct platform_device *dpidev)
 	if (r) {
 		DSSERR("device %s init failed: %d\n", dssdev->name, r);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = omapdss_output_set_device(&dpi.output, dssdev);
@@ -664,7 +664,7 @@ static void dpi_probe_pdata(struct platform_device *dpidev)
 		DSSERR("failed to connect output to new device: %s\n",
 				dssdev->name);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = dss_add_device(dssdev);
@@ -672,8 +672,10 @@ static void dpi_probe_pdata(struct platform_device *dpidev)
 		DSSERR("device %s register failed: %d\n", dssdev->name, r);
 		omapdss_output_unset_device(&dpi.output);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
+
+	return 0;
 }
 
 static void dpi_init_output(struct platform_device *pdev)
@@ -698,11 +700,17 @@ static void __exit dpi_uninit_output(struct platform_device *pdev)
 
 static int omap_dpi_probe(struct platform_device *pdev)
 {
+	int r;
+
 	mutex_init(&dpi.lock);
 
 	dpi_init_output(pdev);
 
-	dpi_probe_pdata(pdev);
+	r = dpi_probe_pdata(pdev);
+	if (r) {
+		dpi_uninit_output(pdev);
+		return r;
+	}
 
 	return 0;
 }
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 06/10] OMAPDSS: DPI: use platform_driver_register()
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Use platform_driver_register() instead of platform_driver_probe() so
that we can support EPROBE_DEFER.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dpi.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index e93c4de..2024787 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -520,7 +520,7 @@ void omapdss_dpi_set_data_lines(struct omap_dss_device *dssdev, int data_lines)
 }
 EXPORT_SYMBOL(omapdss_dpi_set_data_lines);
 
-static int __init dpi_verify_dsi_pll(struct platform_device *dsidev)
+static int dpi_verify_dsi_pll(struct platform_device *dsidev)
 {
 	int r;
 
@@ -572,7 +572,7 @@ static enum omap_channel dpi_get_channel(void)
 	}
 }
 
-static int __init dpi_init_display(struct omap_dss_device *dssdev)
+static int dpi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev;
 
@@ -607,7 +607,7 @@ static int __init dpi_init_display(struct omap_dss_device *dssdev)
 	return 0;
 }
 
-static struct omap_dss_device * __init dpi_find_dssdev(struct platform_device *pdev)
+static struct omap_dss_device *dpi_find_dssdev(struct platform_device *pdev)
 {
 	struct omap_dss_board_info *pdata = pdev->dev.platform_data;
 	const char *def_disp_name = omapdss_get_default_display_name();
@@ -635,7 +635,7 @@ static struct omap_dss_device * __init dpi_find_dssdev(struct platform_device *p
 	return def_dssdev;
 }
 
-static void __init dpi_probe_pdata(struct platform_device *dpidev)
+static void dpi_probe_pdata(struct platform_device *dpidev)
 {
 	struct omap_dss_device *plat_dssdev;
 	struct omap_dss_device *dssdev;
@@ -676,7 +676,7 @@ static void __init dpi_probe_pdata(struct platform_device *dpidev)
 	}
 }
 
-static void __init dpi_init_output(struct platform_device *pdev)
+static void dpi_init_output(struct platform_device *pdev)
 {
 	struct omap_dss_output *out = &dpi.output;
 
@@ -696,7 +696,7 @@ static void __exit dpi_uninit_output(struct platform_device *pdev)
 	dss_unregister_output(out);
 }
 
-static int __init omap_dpi_probe(struct platform_device *pdev)
+static int omap_dpi_probe(struct platform_device *pdev)
 {
 	mutex_init(&dpi.lock);
 
@@ -717,6 +717,7 @@ static int __exit omap_dpi_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver omap_dpi_driver = {
+	.probe		= omap_dpi_probe,
 	.remove         = __exit_p(omap_dpi_remove),
 	.driver         = {
 		.name   = "omapdss_dpi",
@@ -726,7 +727,7 @@ static struct platform_driver omap_dpi_driver = {
 
 int __init dpi_init_platform_driver(void)
 {
-	return platform_driver_probe(&omap_dpi_driver, omap_dpi_probe);
+	return platform_driver_register(&omap_dpi_driver);
 }
 
 void __exit dpi_uninit_platform_driver(void)
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 05/10] OMAPDSS: DSI: Add error handling for dsi_probe_pdata
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Add proper error handling for dsi_probe_pdata(). This will cause
EPROBE_DEFER to be properly passed upwards, causing the DSI driver to be
probed again later if a resource was missing.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 55fc0d4..a73dedc 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -5398,7 +5398,7 @@ static struct omap_dss_device *dsi_find_dssdev(struct platform_device *pdev)
 	return def_dssdev;
 }
 
-static void dsi_probe_pdata(struct platform_device *dsidev)
+static int dsi_probe_pdata(struct platform_device *dsidev)
 {
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	struct omap_dss_device *plat_dssdev;
@@ -5408,11 +5408,11 @@ static void dsi_probe_pdata(struct platform_device *dsidev)
 	plat_dssdev = dsi_find_dssdev(dsidev);
 
 	if (!plat_dssdev)
-		return;
+		return 0;
 
 	dssdev = dss_alloc_and_init_device(&dsidev->dev);
 	if (!dssdev)
-		return;
+		return -ENOMEM;
 
 	dss_copy_device_pdata(dssdev, plat_dssdev);
 
@@ -5420,7 +5420,7 @@ static void dsi_probe_pdata(struct platform_device *dsidev)
 	if (r) {
 		DSSERR("device %s init failed: %d\n", dssdev->name, r);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = omapdss_output_set_device(&dsi->output, dssdev);
@@ -5428,7 +5428,7 @@ static void dsi_probe_pdata(struct platform_device *dsidev)
 		DSSERR("failed to connect output to new device: %s\n",
 				dssdev->name);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
 
 	r = dss_add_device(dssdev);
@@ -5458,7 +5458,7 @@ static void dsi_init_output(struct platform_device *dsidev)
 	dss_register_output(out);
 }
 
-static void __exit dsi_uninit_output(struct platform_device *dsidev)
+static void dsi_uninit_output(struct platform_device *dsidev)
 {
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	struct omap_dss_output *out = &dsi->output;
@@ -5563,7 +5563,13 @@ static int omap_dsihw_probe(struct platform_device *dsidev)
 
 	dsi_init_output(dsidev);
 
-	dsi_probe_pdata(dsidev);
+	r = dsi_probe_pdata(dsidev);
+	if (r) {
+		dsi_runtime_put(dsidev);
+		dsi_uninit_output(dsidev);
+		pm_runtime_disable(&dsidev->dev);
+		return r;
+	}
 
 	dsi_runtime_put(dsidev);
 
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 04/10] OMAPDSS: DSI: use platform_driver_register()
From: Tomi Valkeinen @ 2013-04-29 10:22 UTC (permalink / raw)
  To: linux-fbdev, linux-omap
  Cc: Tony Lindgren, grygorii.strashko, chf.fritz, Archit Taneja,
	Tomi Valkeinen
In-Reply-To: <1367230957-32337-1-git-send-email-tomi.valkeinen@ti.com>

Use platform_driver_register() instead of platform_driver_probe() so
that we can support EPROBE_DEFER.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/dss/dsi.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 9b1c5ec..55fc0d4 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -5225,7 +5225,7 @@ static enum omap_channel dsi_get_channel(int module_id)
 	}
 }
 
-static int __init dsi_init_display(struct omap_dss_device *dssdev)
+static int dsi_init_display(struct omap_dss_device *dssdev)
 {
 	struct platform_device *dsidev  			dsi_get_dsidev_from_id(dssdev->phy.dsi.module);
@@ -5366,7 +5366,7 @@ static int dsi_get_clocks(struct platform_device *dsidev)
 	return 0;
 }
 
-static struct omap_dss_device * __init dsi_find_dssdev(struct platform_device *pdev)
+static struct omap_dss_device *dsi_find_dssdev(struct platform_device *pdev)
 {
 	struct omap_dss_board_info *pdata = pdev->dev.platform_data;
 	struct dsi_data *dsi = dsi_get_dsidrv_data(pdev);
@@ -5398,7 +5398,7 @@ static struct omap_dss_device * __init dsi_find_dssdev(struct platform_device *p
 	return def_dssdev;
 }
 
-static void __init dsi_probe_pdata(struct platform_device *dsidev)
+static void dsi_probe_pdata(struct platform_device *dsidev)
 {
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	struct omap_dss_device *plat_dssdev;
@@ -5436,11 +5436,13 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev)
 		DSSERR("device %s register failed: %d\n", dssdev->name, r);
 		omapdss_output_unset_device(&dsi->output);
 		dss_put_device(dssdev);
-		return;
+		return r;
 	}
+
+	return 0;
 }
 
-static void __init dsi_init_output(struct platform_device *dsidev)
+static void dsi_init_output(struct platform_device *dsidev)
 {
 	struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 	struct omap_dss_output *out = &dsi->output;
@@ -5465,7 +5467,7 @@ static void __exit dsi_uninit_output(struct platform_device *dsidev)
 }
 
 /* DSI1 HW IP initialisation */
-static int __init omap_dsihw_probe(struct platform_device *dsidev)
+static int omap_dsihw_probe(struct platform_device *dsidev)
 {
 	u32 rev;
 	int r, i;
@@ -5632,6 +5634,7 @@ static const struct dev_pm_ops dsi_pm_ops = {
 };
 
 static struct platform_driver omap_dsihw_driver = {
+	.probe		= omap_dsihw_probe,
 	.remove         = __exit_p(omap_dsihw_remove),
 	.driver         = {
 		.name   = "omapdss_dsi",
@@ -5642,7 +5645,7 @@ static struct platform_driver omap_dsihw_driver = {
 
 int __init dsi_init_platform_driver(void)
 {
-	return platform_driver_probe(&omap_dsihw_driver, omap_dsihw_probe);
+	return platform_driver_register(&omap_dsihw_driver);
 }
 
 void __exit dsi_uninit_platform_driver(void)
-- 
1.7.10.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox