Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-15 15:50 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Clark, Rob,
	Archit Taneja
In-Reply-To: <1316100594.23214.65.camel@deskari>

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

On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
> the plan is to make DRM the core Linux display framework, upon which
> everything else is built, and fb and v4l2 are changed to use DRM.

I'd like to think we could make DRM the underlying display framework;
it already exposes an fb interface, and with overlays, a bit more of the
v4l2 stuff is done as well. Certainly eliminating three copies of mode
setting infrastructure would be nice...

> But even if it was done like that, I see that it's combining two
> separate things: 1) the lower level HW control, and 2) the upper level
> buffer management, policies and userspace interfaces.

Those are split between the DRM layer and the underlying device driver,
which provides both kernel (via fb) and user space interfaces.

> 2) It's missing the panel driver part. This is rather important on
> embedded systems, as the panels often are not "dummy" panels, but they
> need things like custom initialization, sending commands to adjust
> backlight, etc.

We integrate the panel (and other video output) drivers into the device
drivers. With desktop chips, they're not easily separable. None of the
desktop output drivers are simple; things like DisplayPort require link
training, and everyone needs EDID. We share some of that code in the DRM
layer today, and it would be nice to share even more.

We should figure out if the DRM interfaces are sufficient for your
needs; they're pretty flexible at this point.

Of course, backlight remains a mess in the desktop world; with many
custom backlight drivers along with generic ACPI and then
per-video-device drivers as well.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [REPOST][PATCH 2/2] video: miscellaneous minor changes to the
From: Timur Tabi @ 2011-09-15 16:19 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1315929909-26311-2-git-send-email-timur@freescale.com>

Tormod Volden wrote:
> On Tue, Sep 13, 2011 at 6:05 PM, Timur Tabi <timur@freescale.com> wrote:
>> Make several minor, miscellaneous changes to the Freescale DIU framebuffer
>> driver.  These changes "lighten" the code by removing crud, fixing small
>> bugs, and fixing some coding style problems.  These changes will make it
>> easier to make more substantial fixes in the future.
> 
> It would be much easier to review this if it is split up into several
> commits. At least have the whitespace fixes in a separate commit, and
> also the actual bug fixes. "git add -p" is your friend.

Ok.

> 
>> 1. Fix incorrect indentation and spacing with some code.
>> 2. Remove debug printks (they don't actually help in debugging the code).
>> 3. Clean up some other printks (e.g. use pr_xxx, clean up the text, etc).
>> 4. Remove the "default" videomode object since it's just a dupe of the
>>   first element in the videomode array.
>> 5. Remove some superfluous local variables.
>> 6. Rename ofdev to pdev, since it's a platform device not an OF device.
>> 7. Fix some device tree operations.
>> 8. Fix some build warnings.
>> 9. Removed some unused structures from the header file.
>> 10. Other minor bug fixes and changes.
> 
> I would have found natural to split it up into commits like for
> example: 1, 2+3, 4, 5+8+9, 10.

Ok.

> 
>> @@ -217,59 +201,59 @@ struct mfb_info {
>>        int x_aoi_d;            /* aoi display x offset to physical screen */
>>        int y_aoi_d;            /* aoi display y offset to physical screen */
>>        struct fsl_diu_data *parent;
>> -       u8 *edid_data;
>> +       void *edid_data;
>>  };
> 
> Why do you convert edid_data from pointer to u8 to pointer to void?

Hmm.. Normally I do that to eliminate a typecast, but that didn't happen in this
case.  I guess I'll leave it as a u8*.

-- 
Timur Tabi
Linux kernel developer at Freescale


^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 17:05 UTC (permalink / raw)
  To: Keith Packard
  Cc: linux-fbdev, linaro-dev, linux-kernel, dri-devel, Archit Taneja,
	Clark, Rob
In-Reply-To: <yun8vpp7qk7.fsf@aiko.keithp.com>

On Thu, 15 Sep 2011 10:50:32 -0500
Keith Packard <keithp@keithp.com> wrote:

> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
> > 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
> > the plan is to make DRM the core Linux display framework, upon which
> > everything else is built, and fb and v4l2 are changed to use DRM.
> 
> I'd like to think we could make DRM the underlying display framework;
> it already exposes an fb interface, and with overlays, a bit more of the
> v4l2 stuff is done as well. Certainly eliminating three copies of mode
> setting infrastructure would be nice...

V4L2 needs to interface with the DRM anyway. Lots of current hardware
wants things like shared 1080i/p camera buffers with video in order to do
preview on video and the like.

In my semi-perfect world vision fb would be a legacy layer on top of DRM.
DRM would get the silly recovery fail cases fixed, and a kernel console
would be attachable to a GEM object of your choice.

Alan



^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 17:12 UTC (permalink / raw)
  To: Keith Packard
  Cc: Tomi Valkeinen, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
	Clark, Rob, Archit Taneja
In-Reply-To: <yun8vpp7qk7.fsf@aiko.keithp.com>

On 09/15/2011 03:50 PM, Keith Packard wrote:
> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
>> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>> the plan is to make DRM the core Linux display framework, upon which
>> everything else is built, and fb and v4l2 are changed to use DRM.
> 
> I'd like to think we could make DRM the underlying display framework;
> it already exposes an fb interface, and with overlays, a bit more of the
> v4l2 stuff is done as well. Certainly eliminating three copies of mode
> setting infrastructure would be nice...

Interesting that this comes from the people that pushed the latest mode setting
code into the kernel. But I don't think that this will happen, the exposed user
interfaces will be around for decades and the infrastructure code could be
shared, in theory.
For fb and V4L2 I think we'll develop some level of interoperability, share
concepts and maybe even some code. The FOURCC pixel formats and overlays are
such examples. As Laurent is really interested in it I think we can get some
nice progress here.
For fb and DRM the situation is entirely different. The last proposal I remember
ended in the DRM people stating that only their implementation is acceptable as
is and we could use it. Such attitude is not helpful and as I don't see any
serious intention of the DRM guys to cooperate I think those subsystems are more
likely to diverge. At least I'll never accept any change to the fb
infrastructure that requires DRM.


Regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 17:18 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E72320B.6020000@gmx.de>

> is and we could use it. Such attitude is not helpful and as I don't see any
> serious intention of the DRM guys to cooperate I think those subsystems are more
> likely to diverge. At least I'll never accept any change to the fb
> infrastructure that requires DRM.

There are aspects of the fb code that want changing for DRM (and indeed
modern hardware) but which won't break for other stuff. Given the move to
using main memory for video and the need for the OS to do buffer
management for framebuffers I suspect a move to DRM is pretty much
inevitable, along with having to fix the fb layer to cope with
discontiguous framebuffers.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-15 17:21 UTC (permalink / raw)
  To: Keith Packard
  Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Clark, Rob,
	Archit Taneja
In-Reply-To: <yun8vpp7qk7.fsf@aiko.keithp.com>

On Thu, 2011-09-15 at 10:50 -0500, Keith Packard wrote:
> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
> > 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
> > the plan is to make DRM the core Linux display framework, upon which
> > everything else is built, and fb and v4l2 are changed to use DRM.
> 
> I'd like to think we could make DRM the underlying display framework;
> it already exposes an fb interface, and with overlays, a bit more of the
> v4l2 stuff is done as well. Certainly eliminating three copies of mode
> setting infrastructure would be nice...

Ok, sounds good to me. We (as in OMAP display people) are already
planning to take DRM into use, so no problem there.

> > But even if it was done like that, I see that it's combining two
> > separate things: 1) the lower level HW control, and 2) the upper level
> > buffer management, policies and userspace interfaces.
> 
> Those are split between the DRM layer and the underlying device driver,
> which provides both kernel (via fb) and user space interfaces.

I'm not so familiar with DRM, but with device driver you mean a driver
for the the hardware which handles display output (gfx cards or whatever
it is on that platform)?

If so, it sounds good. That quite well matches what omapdss driver does
currently for us. But we still have semi-complex omapdrm between omapdss
and the standard drm layer.

Rob, would you say omapdrm is more of a DRM wrapper for omapdss than a
real separate entity? If so, then we could possibly in the future (when
nobody else uses omapdss) change omapdss to support DRM natively. (or
make omapdrm support omap HW natively, which ever way =).

> > 2) It's missing the panel driver part. This is rather important on
> > embedded systems, as the panels often are not "dummy" panels, but they
> > need things like custom initialization, sending commands to adjust
> > backlight, etc.
> 
> We integrate the panel (and other video output) drivers into the device
> drivers. With desktop chips, they're not easily separable. None of the
> desktop output drivers are simple; things like DisplayPort require link
> training, and everyone needs EDID. We share some of that code in the DRM
> layer today, and it would be nice to share even more.

I don't think we speak of similar panel drivers. I think there are two
different drivers here:

1) output drivers, handles the output from the SoC / gfx card. For
example DVI, DisplayPort, MIPI DPI/DBI/DSI.

2) panel drivers, handles panel specific things. Each panel may support
custom commands and features, for which we need a dedicated driver. And
this driver is not platform specific, but should work with any platform
which has the output used with the panel.

As an example, DSI command mode displays can be quite complex:

DSI bus is a half-duplex serial bus, and while it's designed for
displays you could use it easily for any communication between the SoC
and the peripheral.

The panel could have a feature like content adaptive backlight control,
and this would be configured via the DSI bus, sending a particular
command to the panel (possibly by first reading something from the
panel). The panel driver would accomplish this more or less the same way
one uses, say, i2c, so it would use the platform's DSI support to send
and receive packets.

Or a more complex scenario (but still a realistic scenario, been there,
done that) is sending the image to the panel in multiple parts, and
between each part sending configuration commands to the panel. (and
still getting it done in time so we avoid tearing).

And to complicate things more, there are DSI bus features like LP mode
(low power, basically low speed mode) and HS mode (high speed), virtual
channel IDs, and whatnot, which each panel may need to be used in
particular manner. Some panels may require initial configuration done in
LP, or configuration commands sent to a certain virtual channel ID.

The point is that we cannot have standard "MIPI DSI command mode panel
driver" which would work for all DSI cmd mode panels, but we need (in
the worst case) separate driver for each panel.

The same goes to lesser extent for other panels also. Some are
configured via i2c or spi.

 Tomi



^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 17:47 UTC (permalink / raw)
  To: Alan Cox
  Cc: Keith Packard, Tomi Valkeinen, linux-fbdev, linux-kernel,
	dri-devel, linaro-dev, Clark, Rob, Archit Taneja
In-Reply-To: <20110915181802.69ef0d56@lxorguk.ukuu.org.uk>

Hi Alan,

On 09/15/2011 05:18 PM, Alan Cox wrote:
>> is and we could use it. Such attitude is not helpful and as I don't see any
>> serious intention of the DRM guys to cooperate I think those subsystems are more
>> likely to diverge. At least I'll never accept any change to the fb
>> infrastructure that requires DRM.
> 
> There are aspects of the fb code that want changing for DRM (and indeed
> modern hardware) but which won't break for other stuff. Given the move to
> using main memory for video and the need for the OS to do buffer
> management for framebuffers I suspect a move to DRM is pretty much
> inevitable, along with having to fix the fb layer to cope with
> discontiguous framebuffers.

What is your problem with discontigous framebuffers? (I assume discontigous
refers to the pages the framebuffer is composed of)
Sounds to me like you should implement your own fb_mmap and either map it
contigous to screen_base or implement your own fb_read/write.
In theory you could even have each pixel at a completely different memory
location although some userspace wouldn't be happy when it could no longer mmap
the framebuffer.


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alex Deucher @ 2011-09-15 17:52 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Keith Packard, linux-fbdev, linaro-dev, linux-kernel, dri-devel,
	Archit Taneja, Clark, Rob
In-Reply-To: <4E72320B.6020000@gmx.de>

On Thu, Sep 15, 2011 at 1:12 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 09/15/2011 03:50 PM, Keith Packard wrote:
>> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>>> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>>> the plan is to make DRM the core Linux display framework, upon which
>>> everything else is built, and fb and v4l2 are changed to use DRM.
>>
>> I'd like to think we could make DRM the underlying display framework;
>> it already exposes an fb interface, and with overlays, a bit more of the
>> v4l2 stuff is done as well. Certainly eliminating three copies of mode
>> setting infrastructure would be nice...
>
> Interesting that this comes from the people that pushed the latest mode setting
> code into the kernel. But I don't think that this will happen, the exposed user
> interfaces will be around for decades and the infrastructure code could be
> shared, in theory.
> For fb and V4L2 I think we'll develop some level of interoperability, share
> concepts and maybe even some code. The FOURCC pixel formats and overlays are
> such examples. As Laurent is really interested in it I think we can get some
> nice progress here.
> For fb and DRM the situation is entirely different. The last proposal I remember
> ended in the DRM people stating that only their implementation is acceptable as
> is and we could use it. Such attitude is not helpful and as I don't see any
> serious intention of the DRM guys to cooperate I think those subsystems are more
> likely to diverge. At least I'll never accept any change to the fb
> infrastructure that requires DRM.

Not exactly.  This point was that the drm modesetting and EDID
handling was derived from X which has had 20+ years of of quirks and
things added to it to deal with tons of wonky monitors and such.  That
information should be preserved.  As mode structs and EDID handling
are pretty self contained, why not use the DRM variants of that code
rather than writing a new version?

While the DRM has historically targeted 3D acceleration, that is not a
requirement to use the DRM KMS modesetting API.  The current fb API
has no concept of display controllers or connectors or overlays, etc.
To match it to modern hardware, it needs a major overhaul.  Why create
a new modern fb interface that's largely the same as DRM KMS?  What if
we just consider the KMS API as the new fb API?  If there are any
inadequacies in the DRM KMS API we would be happy to work out any
changes.

Please don't claim that the DRM developers do not want to cooperate.
I realize that people have strong opinions about existing APIs, put
there has been just as much, if not more obstinacy from the v4l and fb
people.

Alex

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Geert Uytterhoeven @ 2011-09-15 17:56 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Florian Tobias Schandinat, Keith Packard, linux-fbdev, linaro-dev,
	linux-kernel, dri-devel, Archit Taneja, Clark, Rob
In-Reply-To: <CADnq5_NnzLtJmSzOgyppnBtv96DRTJ71u6ZSrs56KJ-F5CC-mw@mail.gmail.com>

On Thu, Sep 15, 2011 at 19:52, Alex Deucher <alexdeucher@gmail.com> wrote:
> While the DRM has historically targeted 3D acceleration, that is not a
> requirement to use the DRM KMS modesetting API.  The current fb API
> has no concept of display controllers or connectors or overlays, etc.
> To match it to modern hardware, it needs a major overhaul.  Why create
> a new modern fb interface that's largely the same as DRM KMS?  What if
> we just consider the KMS API as the new fb API?  If there are any
> inadequacies in the DRM KMS API we would be happy to work out any
> changes.

I admit I didn't look for it, but does there exist a sample DRM KMS driver
for dumb frame buffer hardware with one fixed video mode?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alex Deucher @ 2011-09-15 18:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linaro-dev, Florian Tobias Schandinat, linux-kernel, dri-devel,
	Archit Taneja, linux-fbdev, Clark, Rob
In-Reply-To: <CAMuHMdUoYi5xTQz=v1rCycS23vFgQzeZFbNZGnrRN+qkBCrSRA@mail.gmail.com>

On Thu, Sep 15, 2011 at 1:56 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Sep 15, 2011 at 19:52, Alex Deucher <alexdeucher@gmail.com> wrote:
>> While the DRM has historically targeted 3D acceleration, that is not a
>> requirement to use the DRM KMS modesetting API.  The current fb API
>> has no concept of display controllers or connectors or overlays, etc.
>> To match it to modern hardware, it needs a major overhaul.  Why create
>> a new modern fb interface that's largely the same as DRM KMS?  What if
>> we just consider the KMS API as the new fb API?  If there are any
>> inadequacies in the DRM KMS API we would be happy to work out any
>> changes.
>
> I admit I didn't look for it, but does there exist a sample DRM KMS driver
> for dumb frame buffer hardware with one fixed video mode?

Not at the moment.  However, there drivers for AMD, Intel, and nvidia
chips as well patches for a number of ARM SoCs that are in the process
of moving upstream.  Also, Matt Turner wrote a KMS driver for 3D labs
glint hardware that is pretty simple (single display controller,
single DAC, etc.), however it hasn't been merged upstream yet.
http://code.google.com/p/google-summer-of-code-2010-xorg/downloads/detail?name=Matt_Turner.tar.gz&can=2&qHis kernel git tree was on kernel.org so it's down at the moment,
hence the link to the tarball.

Alex

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Keith Packard @ 2011-09-15 18:12 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Tomi Valkeinen, linux-fbdev, linux-kernel, dri-devel, linaro-dev,
	Clark, Rob, Archit Taneja
In-Reply-To: <4E72320B.6020000@gmx.de>

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

On Thu, 15 Sep 2011 17:12:43 +0000, Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:

> Interesting that this comes from the people that pushed the latest mode setting
> code into the kernel. But I don't think that this will happen, the exposed user
> interfaces will be around for decades and the infrastructure code could be
> shared, in theory.

We moved mode setting code from user space to kernel space -- the DRM
stuff comes directly from X, which has a fairly long history of
complicated display environments.

The DRM code does expose fb interfaces to both kernel and user mode,
figuring out how to integrate v4l2 and drm seems like the remaining
challenge.

> For fb and V4L2 I think we'll develop some level of interoperability, share
> concepts and maybe even some code. The FOURCC pixel formats and overlays are
> such examples. As Laurent is really interested in it I think we can get some
> nice progress here.

Jesse's design for the DRM overlay code will expose the pixel formats as
FOURCC codes so that DRM and v4l2 can interoperate -- we've got a lot of
hardware that has both video decode and 3D acceleration, so those are
going to get integrated somehow. And, we have to figure out how to share
buffers between these APIs to avoid copying data with the CPU.

DRM provides fb interfaces, so you don't need to change fb at all --
hardware that requires the capabilities provided by DRM will use that
and not use any of the other fb code in the kernel.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Rob Clark @ 2011-09-15 18:32 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja
In-Reply-To: <1316107275.23214.99.camel@deskari>

On Thu, Sep 15, 2011 at 12:21 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2011-09-15 at 10:50 -0500, Keith Packard wrote:
>> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> > 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>> > the plan is to make DRM the core Linux display framework, upon which
>> > everything else is built, and fb and v4l2 are changed to use DRM.
>>
>> I'd like to think we could make DRM the underlying display framework;
>> it already exposes an fb interface, and with overlays, a bit more of the
>> v4l2 stuff is done as well. Certainly eliminating three copies of mode
>> setting infrastructure would be nice...
>
> Ok, sounds good to me. We (as in OMAP display people) are already
> planning to take DRM into use, so no problem there.
>
>> > But even if it was done like that, I see that it's combining two
>> > separate things: 1) the lower level HW control, and 2) the upper level
>> > buffer management, policies and userspace interfaces.
>>
>> Those are split between the DRM layer and the underlying device driver,
>> which provides both kernel (via fb) and user space interfaces.
>
> I'm not so familiar with DRM, but with device driver you mean a driver
> for the the hardware which handles display output (gfx cards or whatever
> it is on that platform)?

I think he is more referring to the DRM core and the individual device drivers..

We are (AFAIK) unique in having a two layer driver, where the DRM part
is more of a wrapper (for the KMS parts)... but I see that as more of
a transition thing.. eventually we should be able to merge it all into
the DRM layer.

> If so, it sounds good. That quite well matches what omapdss driver does
> currently for us. But we still have semi-complex omapdrm between omapdss
> and the standard drm layer.
>
> Rob, would you say omapdrm is more of a DRM wrapper for omapdss than a
> real separate entity? If so, then we could possibly in the future (when
> nobody else uses omapdss) change omapdss to support DRM natively. (or
> make omapdrm support omap HW natively, which ever way =).

Yeah, I think eventually it would make sense to merge all into one.
Although I'm not sure about how best to handle various different
custom DSI panels..

BR,
-R


>> > 2) It's missing the panel driver part. This is rather important on
>> > embedded systems, as the panels often are not "dummy" panels, but they
>> > need things like custom initialization, sending commands to adjust
>> > backlight, etc.
>>
>> We integrate the panel (and other video output) drivers into the device
>> drivers. With desktop chips, they're not easily separable. None of the
>> desktop output drivers are simple; things like DisplayPort require link
>> training, and everyone needs EDID. We share some of that code in the DRM
>> layer today, and it would be nice to share even more.
>
> I don't think we speak of similar panel drivers. I think there are two
> different drivers here:
>
> 1) output drivers, handles the output from the SoC / gfx card. For
> example DVI, DisplayPort, MIPI DPI/DBI/DSI.
>
> 2) panel drivers, handles panel specific things. Each panel may support
> custom commands and features, for which we need a dedicated driver. And
> this driver is not platform specific, but should work with any platform
> which has the output used with the panel.
>
> As an example, DSI command mode displays can be quite complex:
>
> DSI bus is a half-duplex serial bus, and while it's designed for
> displays you could use it easily for any communication between the SoC
> and the peripheral.
>
> The panel could have a feature like content adaptive backlight control,
> and this would be configured via the DSI bus, sending a particular
> command to the panel (possibly by first reading something from the
> panel). The panel driver would accomplish this more or less the same way
> one uses, say, i2c, so it would use the platform's DSI support to send
> and receive packets.
>
> Or a more complex scenario (but still a realistic scenario, been there,
> done that) is sending the image to the panel in multiple parts, and
> between each part sending configuration commands to the panel. (and
> still getting it done in time so we avoid tearing).
>
> And to complicate things more, there are DSI bus features like LP mode
> (low power, basically low speed mode) and HS mode (high speed), virtual
> channel IDs, and whatnot, which each panel may need to be used in
> particular manner. Some panels may require initial configuration done in
> LP, or configuration commands sent to a certain virtual channel ID.
>
> The point is that we cannot have standard "MIPI DSI command mode panel
> driver" which would work for all DSI cmd mode panels, but we need (in
> the worst case) separate driver for each panel.
>
> The same goes to lesser extent for other panels also. Some are
> configured via i2c or spi.
>
>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 18:39 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Keith Packard, linux-fbdev, linaro-dev, linux-kernel, dri-devel,
	Archit Taneja, Clark, Rob
In-Reply-To: <CADnq5_NnzLtJmSzOgyppnBtv96DRTJ71u6ZSrs56KJ-F5CC-mw@mail.gmail.com>

On 09/15/2011 05:52 PM, Alex Deucher wrote:
> On Thu, Sep 15, 2011 at 1:12 PM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> On 09/15/2011 03:50 PM, Keith Packard wrote:
>>> On Thu, 15 Sep 2011 18:29:54 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>
>>>> 1) It's part of DRM, so it doesn't help fb or v4l2 drivers. Except if
>>>> the plan is to make DRM the core Linux display framework, upon which
>>>> everything else is built, and fb and v4l2 are changed to use DRM.
>>>
>>> I'd like to think we could make DRM the underlying display framework;
>>> it already exposes an fb interface, and with overlays, a bit more of the
>>> v4l2 stuff is done as well. Certainly eliminating three copies of mode
>>> setting infrastructure would be nice...
>>
>> Interesting that this comes from the people that pushed the latest mode setting
>> code into the kernel. But I don't think that this will happen, the exposed user
>> interfaces will be around for decades and the infrastructure code could be
>> shared, in theory.
>> For fb and V4L2 I think we'll develop some level of interoperability, share
>> concepts and maybe even some code. The FOURCC pixel formats and overlays are
>> such examples. As Laurent is really interested in it I think we can get some
>> nice progress here.
>> For fb and DRM the situation is entirely different. The last proposal I remember
>> ended in the DRM people stating that only their implementation is acceptable as
>> is and we could use it. Such attitude is not helpful and as I don't see any
>> serious intention of the DRM guys to cooperate I think those subsystems are more
>> likely to diverge. At least I'll never accept any change to the fb
>> infrastructure that requires DRM.
> 
> Not exactly.  This point was that the drm modesetting and EDID
> handling was derived from X which has had 20+ years of of quirks and
> things added to it to deal with tons of wonky monitors and such.  That
> information should be preserved.  As mode structs and EDID handling
> are pretty self contained, why not use the DRM variants of that code
> rather than writing a new version?

Well, I'm not against sharing the code and not against taking DRM's current
implementation as a base but the steps required to make it generally acceptable
would be to split it of, probably as a standalone module and strip all DRM
specific things off. Than all things that require EDID can use it, DRM can add
DRM-specific things on top and fb can add fb-specific things.

> While the DRM has historically targeted 3D acceleration, that is not a
> requirement to use the DRM KMS modesetting API.  The current fb API
> has no concept of display controllers or connectors or overlays, etc.
> To match it to modern hardware, it needs a major overhaul.  Why create
> a new modern fb interface that's largely the same as DRM KMS?  What if
> we just consider the KMS API as the new fb API?  If there are any
> inadequacies in the DRM KMS API we would be happy to work out any
> changes.

Well, I rather think that the fb API is more user centric to allow every program
to use it directly in contrast to the KMS/DRM API which aims to support every
feature the hardware has. For this the fb API should not change much, but I
understand some additions were needed for some special users, probably limited
to X and wayland.
One of my biggest problems with KMS is that it has (naturally) a lot more
complexity than the fb API which leads to instability. Basically it's very
difficult to implement a framebuffer in a way that it crashes your machine
during operation which is quite a contrast to my KMS/DRM experience on my toy
(on my work machines I use framebuffer only). And I really hate it when I have
to type my passwords again just because the KMS/DRM thing allowed a program to
crash my machine. Yes, those are driver bugs but the API encourages them and I
did not yet find the feature/config option DOES_NOT_CRASH or SLOW_BUT_STABLE.
And as I said already, I think the fb API is a lot better for direct interaction
with userspace programs and certainly has more direct users at the moment.

> Please don't claim that the DRM developers do not want to cooperate.
> I realize that people have strong opinions about existing APIs, put
> there has been just as much, if not more obstinacy from the v4l and fb
> people.

Well, I think it's too late to really fix this thing. We now have 3 APIs in the
kernel that have to be kept. Probably the best we can do now is figure out how
we can reduce code duplication and do extensions to those APIs in a way that
they are compatible with each other or completely independent and can be used
across the APIs.


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 18:58 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit, Taneja,
	Clark, Rob
In-Reply-To: <4E724659.5080709@gmx.de>

> Well, I rather think that the fb API is more user centric to allow every program
> to use it directly in contrast to the KMS/DRM API which aims to support every
> feature the hardware has. For this the fb API should not change much, but I
> understand some additions were needed for some special users, probably limited
> to X and wayland.

Wayland needs vblank frame buffer switching and the like. Likewise given
you want to composite buffers really any serious accelerated device ends
up needing a full memory manager and that ends up needing a buffer
manager. Wayland needs clients to be doing their own rendering into
objects which means authorisation and management of the render engine
which ends up looking much like DRM.

> One of my biggest problems with KMS is that it has (naturally) a lot more
> complexity than the fb API which leads to instability. Basically it's very

It shouldn't do - and a sample of one (your machine) is not a
statistically valid set. Fb is pretty much ununsable in contrast on my
main box, but that's not a statistically valid sample either.

I'm not that convinced by the complexity either. For a simple video card
setup such as those that the fb layer can kind of cope with (ie linear
buffer, simple mode changes, no client rendering, no vblank flipping,
limited mode management, no serious multi-head) a DRM driver is also
pretty tiny and simple.

> Well, I think it's too late to really fix this thing. We now have 3 APIs in the
> kernel that have to be kept. Probably the best we can do now is figure out how
> we can reduce code duplication and do extensions to those APIs in a way that
> they are compatible with each other or completely independent and can be used
> across the APIs.

I think it comes down to 'when nobody is using the old fb drivers they can
drop into staging and oblivion'. Right now the fb layer is essentially
compatibility glue on most modern x86 platforms.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 19:05 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E723A20.2040002@gmx.de>

> What is your problem with discontigous framebuffers? (I assume discontigous
> refers to the pages the framebuffer is composed of)
> Sounds to me like you should implement your own fb_mmap and either map it
> contigous to screen_base or implement your own fb_read/write.
> In theory you could even have each pixel at a completely different memory
> location although some userspace wouldn't be happy when it could no longer mmap
> the framebuffer.

The mmap side is trivial, the problem is that the fb layer default
implementations of blits, fills etc only work on a kernel linear frame
buffer. And (for example) there is not enough linear stolen memory on
some Intel video for a 1080p console on HDMI even though the hardware is
perfectly capable of using an HDTV as its monitor. Nor - on a 32bit box-
is there enough space to vremap it.

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 19:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Alex Deucher, Keith Packard, linux-fbdev, linaro-dev,
	linux-kernel, dri-devel, Archit Taneja, Clark, Rob
In-Reply-To: <20110915195804.4e6d965b@lxorguk.ukuu.org.uk>

On 09/15/2011 06:58 PM, Alan Cox wrote:
>> Well, I rather think that the fb API is more user centric to allow every program
>> to use it directly in contrast to the KMS/DRM API which aims to support every
>> feature the hardware has. For this the fb API should not change much, but I
>> understand some additions were needed for some special users, probably limited
>> to X and wayland.
> 
> Wayland needs vblank frame buffer switching and the like. Likewise given
> you want to composite buffers really any serious accelerated device ends
> up needing a full memory manager and that ends up needing a buffer
> manager. Wayland needs clients to be doing their own rendering into
> objects which means authorisation and management of the render engine
> which ends up looking much like DRM.

As you have DRM now and as I'm not interested in wayland I won't discuss this,
but I guess it might be a good start for Geert's question what would be needed
to use it on dumb framebuffers.

>> One of my biggest problems with KMS is that it has (naturally) a lot more
>> complexity than the fb API which leads to instability. Basically it's very
> 
> It shouldn't do - and a sample of one (your machine) is not a
> statistically valid set. Fb is pretty much ununsable in contrast on my
> main box, but that's not a statistically valid sample either.
> 
> I'm not that convinced by the complexity either. For a simple video card
> setup such as those that the fb layer can kind of cope with (ie linear
> buffer, simple mode changes, no client rendering, no vblank flipping,
> limited mode management, no serious multi-head) a DRM driver is also
> pretty tiny and simple.

Yes, if you limit DRM to the functionality of the fb API I guess you could reach
the same stability level. But where can I do this? Where is a option to forbid
all acceleration or at least limit to the acceleration that can be done without
any risk?

>> Well, I think it's too late to really fix this thing. We now have 3 APIs in the
>> kernel that have to be kept. Probably the best we can do now is figure out how
>> we can reduce code duplication and do extensions to those APIs in a way that
>> they are compatible with each other or completely independent and can be used
>> across the APIs.
> 
> I think it comes down to 'when nobody is using the old fb drivers they can
> drop into staging and oblivion'. Right now the fb layer is essentially
> compatibility glue on most modern x86 platforms.

That's a really difficult question. Determining the users is difficult and there
are people that use their hardware very long, for example we are about to get a
new driver for i740. For the framebuffer infrastructure I guess you have to at
least wait for my death.


Regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 19:28 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Keith Packard, linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Archit Taneja, Alex Deucher
In-Reply-To: <4E724F93.1050203-Mmb7MZpHnFY@public.gmane.org>

> As you have DRM now and as I'm not interested in wayland I won't discuss this,
> but I guess it might be a good start for Geert's question what would be needed
> to use it on dumb framebuffers.

GMA500 is basically a 2D or dumb frame buffer setup but with a lot of
rather complicated output and memory management required due to the
hardware. With the latest changes to GEM (private objects) it's basically
trivial to use the frame buffer management interfaces.

> Yes, if you limit DRM to the functionality of the fb API I guess you could reach
> the same stability level. But where can I do this? Where is a option to forbid
> all acceleration or at least limit to the acceleration that can be done without
> any risk?

A driver can provide such module options as it wants.

> That's a really difficult question. Determining the users is difficult and there
> are people that use their hardware very long, for example we are about to get a
> new driver for i740. For the framebuffer infrastructure I guess you have to at
> least wait for my death.

I doubt it'll be that long - but you are right it will take time and
there isn't really any need to push or force it. These things take care
of themselves and in time nobody will care about the old fb stuff, either
because DRM covers it all or equally possibly because it doesn't support
3D holographic projection 8)

Alan

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alex Deucher @ 2011-09-15 19:45 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: Alan Cox, Keith Packard, linux-fbdev, linaro-dev, linux-kernel,
	dri-devel, Archit Taneja, Clark, Rob
In-Reply-To: <4E724F93.1050203@gmx.de>

On Thu, Sep 15, 2011 at 3:18 PM, Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> wrote:
> On 09/15/2011 06:58 PM, Alan Cox wrote:
>>> Well, I rather think that the fb API is more user centric to allow every program
>>> to use it directly in contrast to the KMS/DRM API which aims to support every
>>> feature the hardware has. For this the fb API should not change much, but I
>>> understand some additions were needed for some special users, probably limited
>>> to X and wayland.
>>
>> Wayland needs vblank frame buffer switching and the like. Likewise given
>> you want to composite buffers really any serious accelerated device ends
>> up needing a full memory manager and that ends up needing a buffer
>> manager. Wayland needs clients to be doing their own rendering into
>> objects which means authorisation and management of the render engine
>> which ends up looking much like DRM.
>
> As you have DRM now and as I'm not interested in wayland I won't discuss this,
> but I guess it might be a good start for Geert's question what would be needed
> to use it on dumb framebuffers.
>
>>> One of my biggest problems with KMS is that it has (naturally) a lot more
>>> complexity than the fb API which leads to instability. Basically it's very
>>
>> It shouldn't do - and a sample of one (your machine) is not a
>> statistically valid set. Fb is pretty much ununsable in contrast on my
>> main box, but that's not a statistically valid sample either.
>>
>> I'm not that convinced by the complexity either. For a simple video card
>> setup such as those that the fb layer can kind of cope with (ie linear
>> buffer, simple mode changes, no client rendering, no vblank flipping,
>> limited mode management, no serious multi-head) a DRM driver is also
>> pretty tiny and simple.
>
> Yes, if you limit DRM to the functionality of the fb API I guess you could reach
> the same stability level. But where can I do this? Where is a option to forbid
> all acceleration or at least limit to the acceleration that can be done without
> any risk?

Right now most of the KMS DRM drivers do not support accelerated fb,
so as long as you don't run accelerated X or a 3D app, it should be
just as stable as an fb driver.  You may run into modesetting fail in
certain cases due to wonky hardware or driver bugs, but you will hit
that with an fb driver as well.

Alex

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Florian Tobias Schandinat @ 2011-09-15 19:46 UTC (permalink / raw)
  To: Alan Cox
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <20110915200514.74bdcd90@lxorguk.ukuu.org.uk>

On 09/15/2011 07:05 PM, Alan Cox wrote:
>> What is your problem with discontigous framebuffers? (I assume discontigous
>> refers to the pages the framebuffer is composed of)
>> Sounds to me like you should implement your own fb_mmap and either map it
>> contigous to screen_base or implement your own fb_read/write.
>> In theory you could even have each pixel at a completely different memory
>> location although some userspace wouldn't be happy when it could no longer mmap
>> the framebuffer.
> 
> The mmap side is trivial, the problem is that the fb layer default
> implementations of blits, fills etc only work on a kernel linear frame
> buffer. And (for example) there is not enough linear stolen memory on
> some Intel video for a 1080p console on HDMI even though the hardware is
> perfectly capable of using an HDTV as its monitor. Nor - on a 32bit box-
> is there enough space to vremap it.

Okay, I see your problem. It's a bit strange you don't have acceleration. I
guess you need either your own implementation of those or adding function
pointers like fb_read/write just without the __user and use those instead of
direct memory access in the cfb* implementation if screen_base is NULL. Does not
sound like a big problem to me, but pretty inefficient, so probably copying the
existing ones and adjusting it to your needs would be preferred (just like the
sys* implementations exist).


Best regards,

Florian Tobias Schandinat

^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Alan Cox @ 2011-09-15 21:31 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: linaro-dev, linux-fbdev, dri-devel, linux-kernel, Archit Taneja,
	Clark, Rob
In-Reply-To: <4E72561C.7060603@gmx.de>

> Okay, I see your problem. It's a bit strange you don't have acceleration. I

The hardware has 3D acceleration but not open so we can't support it.
There is no 2D acceleration - which seems to be increasingly common.

At some point I'll add hardware scrolling however by using the GTT to
implemnent scroll wrapping.

> sound like a big problem to me, but pretty inefficient, so probably copying the
> existing ones and adjusting it to your needs would be preferred (just like the
> sys* implementations exist).

I did have a look at the current ones but fixing them up given scan lines
can span page boundaries looked pretty horrible so I deferred it until I
feel inspired.

Alan

^ permalink raw reply

* [PATCH 0/13] drivers/video: fsl-diu-fb: several minor improvements
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

This patchset includes several minor improvements to the Freescale DIU
framebuffer driver.


^ permalink raw reply

* [PATCH 01/13] drivers/video: fsl-diu-fb: clean up whitespace and formatting
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Fix various indentation and line length problems in the Freescale
DIU framebuffer driver.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |  104 +++++++++++++++++++++++---------------------
 1 files changed, 55 insertions(+), 49 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 0f1933b..dbb4cb7 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -223,53 +223,53 @@ struct mfb_info {
 
 static struct mfb_info mfb_template[] = {
 	{		/* AOI 0 for plane 0 */
-	.index = 0,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel0",
-	.registered = 0,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 0,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel0",
+		.registered = 0,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 0 for plane 1 */
-	.index = 1,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 1,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 1 */
-	.index = 2,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 480,
+		.index = 2,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 480,
 	},
 	{		/* AOI 0 for plane 2 */
-	.index = 3,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 0,
+		.index = 3,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 2 */
-	.index = 4,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 480,
+		.index = 4,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 480,
 	},
 };
 
@@ -715,8 +715,8 @@ static void update_lcdc(struct fb_info *info)
 	/* Prep for DIU init  - gamma table, cursor table */
 
 	for (i = 0; i <= 2; i++)
-	   for (j = 0; j <= 255; j++)
-	      *gamma_table_base++ = j;
+		for (j = 0; j <= 255; j++)
+			*gamma_table_base++ = j;
 
 	diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);
 
@@ -887,7 +887,7 @@ static int fsl_diu_set_par(struct fb_info *info)
 
 static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
 {
-	return ((val<<width) + 0x7FFF - val)>>16;
+	return ((val << width) + 0x7FFF - val) >> 16;
 }
 
 /*
@@ -899,8 +899,9 @@ static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
  * pseudo_palette in struct fb_info. For pseudocolor mode we have a limited
  * color palette.
  */
-static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
-			   unsigned blue, unsigned transp, struct fb_info *info)
+static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
+			     unsigned int green, unsigned int blue,
+			     unsigned int transp, struct fb_info *info)
 {
 	int ret = 1;
 
@@ -1350,6 +1351,7 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		else if (status & INT_VSYNC) {
 			unsigned int i;
+
 			for (i = 0; i < coherence_data_size;
 				i += d_cache_line_size)
 				__asm__ __volatile__ (
@@ -1381,6 +1383,7 @@ static int request_irq_local(int irq)
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
 		ints |=	INT_VSYNC;
 #endif
+
 		if (dr.mode = MFB_MODE2 || dr.mode = MFB_MODE3)
 			ints |= INT_VSYNC_WB;
 
@@ -1388,6 +1391,7 @@ static int request_irq_local(int irq)
 		status = in_be32(&hw->int_status);
 		out_be32(&hw->int_mask, ints);
 	}
+
 	return ret;
 }
 
@@ -1454,15 +1458,15 @@ static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
 		buf->paddr = (u32)buf->paddr + offset;
 	} else
 		buf->offset = 0;
+
 	return 0;
 }
 
 static void free_buf(struct device *dev, struct diu_addr *buf, u32 size,
 		     u32 bytes_align)
 {
-	dma_free_coherent(dev, size + bytes_align,
-				buf->vaddr, (buf->paddr - buf->offset));
-	return;
+	dma_free_coherent(dev, size + bytes_align, buf->vaddr,
+			  buf->paddr - buf->offset);
 }
 
 static ssize_t store_monitor(struct device *device,
@@ -1798,8 +1802,9 @@ static int __init fsl_diu_init(void)
 		return -ENODEV;
 	}
 
-	/* Freescale PLRU requires 13/8 times the cache size to do a proper
-	   displacement flush
+	/*
+	 * Freescale PLRU requires 13/8 times the cache size to do a proper
+	 * displacement flush
 	 */
 	coherence_data_size = *prop * 13;
 	coherence_data_size /= 8;
@@ -1816,6 +1821,7 @@ static int __init fsl_diu_init(void)
 	if (!coherence_data)
 		return -ENOMEM;
 #endif
+
 	ret = platform_driver_register(&fsl_diu_driver);
 	if (ret) {
 		printk(KERN_ERR
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 02/13] drivers/video: fsl-diu-fb: clean up printk usage
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Remove debug printk messages (they don't help in debugging), replace
printk(KERN_xxx with its pr_xxx or dev_xxx equivalent, and add a couple
missing error messages.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |  105 +++++++------------------------------------
 1 files changed, 17 insertions(+), 88 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index dbb4cb7..10ee411 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -322,14 +322,9 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
 {
 	void *virt;

-	pr_debug("size=%zu\n", size);
-
 	virt = alloc_pages_exact(size, GFP_DMA | __GFP_ZERO);
-	if (virt) {
+	if (virt)
 		*phys = virt_to_phys(virt);
-		pr_debug("virt=%p phys=%llx\n", virt,
-			(unsigned long long)*phys);
-	}

 	return virt;
 }
@@ -343,8 +338,6 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
  */
 static void fsl_diu_free(void *virt, size_t size)
 {
-	pr_debug("virt=%p size=%zu\n", virt, size);
-
 	if (virt && size)
 		free_pages_exact(virt, size);
 }
@@ -368,7 +361,6 @@ static int fsl_diu_enable_panel(struct fb_info *info)
 	struct fsl_diu_data *machine_data = mfbi->parent;
 	int res = 0;

-	pr_debug("enable_panel index %d\n", mfbi->index);
 	if (mfbi->type != MFB_TYPE_OFF) {
 		switch (mfbi->index) {
 		case 0:				/* plane 0 */
@@ -585,9 +577,6 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
 static int fsl_diu_check_var(struct fb_var_screeninfo *var,
 				struct fb_info *info)
 {
-	pr_debug("check_var xres: %d\n", var->xres);
-	pr_debug("check_var yres: %d\n", var->yres);
-
 	if (var->xres_virtual < var->xres)
 		var->xres_virtual = var->xres;
 	if (var->yres_virtual < var->yres)
@@ -720,7 +709,6 @@ static void update_lcdc(struct fb_info *info)

 	diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);

-	pr_debug("update-lcdc: HW - %p\n Disabling DIU\n", hw);
 	disable_lcdc(info);

 	/* Program DIU registers */
@@ -732,9 +720,6 @@ static void update_lcdc(struct fb_info *info)
 	out_be32(&hw->bgnd_wb, 0); 		/* BGND_WB */
 	out_be32(&hw->disp_size, (var->yres << 16 | var->xres));
 						/* DISP SIZE */
-	pr_debug("DIU xres: %d\n", var->xres);
-	pr_debug("DIU yres: %d\n", var->yres);
-
 	out_be32(&hw->wb_size, 0); /* WB SIZE */
 	out_be32(&hw->wb_mem_addr, 0); /* WB MEM ADDR */

@@ -751,15 +736,6 @@ static void update_lcdc(struct fb_info *info)

 	out_be32(&hw->vsyn_para, temp);

-	pr_debug("DIU right_margin - %d\n", var->right_margin);
-	pr_debug("DIU left_margin - %d\n", var->left_margin);
-	pr_debug("DIU hsync_len - %d\n", var->hsync_len);
-	pr_debug("DIU upper_margin - %d\n", var->upper_margin);
-	pr_debug("DIU lower_margin - %d\n", var->lower_margin);
-	pr_debug("DIU vsync_len - %d\n", var->vsync_len);
-	pr_debug("DIU HSYNC - 0x%08x\n", hw->hsyn_para);
-	pr_debug("DIU VSYNC - 0x%08x\n", hw->vsyn_para);
-
 	diu_ops.set_pixel_clock(var->pixclock);

 	out_be32(&hw->syn_pol, 0);	/* SYNC SIGNALS POLARITY */
@@ -776,14 +752,9 @@ static int map_video_memory(struct fb_info *info)
 	phys_addr_t phys;
 	u32 smem_len = info->fix.line_length * info->var.yres_virtual;

-	pr_debug("info->var.xres_virtual = %d\n", info->var.xres_virtual);
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length  = %d\n", info->fix.line_length);
-	pr_debug("MAP_VIDEO_MEMORY: smem_len = %u\n", smem_len);
-
 	info->screen_base = fsl_diu_alloc(smem_len, &phys);
 	if (info->screen_base = NULL) {
-		printk(KERN_ERR "Unable to allocate fb memory\n");
+		dev_err(info->dev, "unable to allocate fb memory\n");
 		return -ENOMEM;
 	}
 	mutex_lock(&info->mm_lock);
@@ -792,10 +763,6 @@ static int map_video_memory(struct fb_info *info)
 	mutex_unlock(&info->mm_lock);
 	info->screen_size = info->fix.smem_len;

-	pr_debug("Allocated fb @ paddr=0x%08lx, size=%d.\n",
-		 info->fix.smem_start, info->fix.smem_len);
-	pr_debug("screen base %p\n", info->screen_base);
-
 	return 0;
 }

@@ -852,11 +819,10 @@ static int fsl_diu_set_par(struct fb_info *info)
 	if (len != info->fix.smem_len) {
 		if (info->fix.smem_start)
 			unmap_video_memory(info);
-		pr_debug("SET PAR: smem_len = %d\n", info->fix.smem_len);

 		/* Memory allocation for framebuffer */
 		if (map_video_memory(info)) {
-			printk(KERN_ERR "Unable to allocate fb memory 1\n");
+			dev_err(info->dev, "unable to allocate fb memory 1\n");
 			return -ENOMEM;
 		}
 	}
@@ -1023,21 +989,17 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_from_user(&pix_fmt, buf, sizeof(pix_fmt)))
 			return -EFAULT;
 		ad->pix_fmt = pix_fmt;
-		pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_GET_PIXFMT:
 		pix_fmt = ad->pix_fmt;
 		if (copy_to_user(buf, &pix_fmt, sizeof(pix_fmt)))
 			return -EFAULT;
-		pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_SET_AOID:
 		if (copy_from_user(&aoi_d, buf, sizeof(aoi_d)))
 			return -EFAULT;
 		mfbi->x_aoi_d = aoi_d.x_aoi_d;
 		mfbi->y_aoi_d = aoi_d.y_aoi_d;
-		pr_debug("set AOI display offset of index %d to (%d,%d)\n",
-				 mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		fsl_diu_check_var(&info->var, info);
 		fsl_diu_set_aoi(info);
 		break;
@@ -1046,14 +1008,11 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		aoi_d.y_aoi_d = mfbi->y_aoi_d;
 		if (copy_to_user(buf, &aoi_d, sizeof(aoi_d)))
 			return -EFAULT;
-		pr_debug("get AOI display offset of index %d (%d,%d)\n",
-				mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		break;
 	case MFB_GET_ALPHA:
 		global_alpha = mfbi->g_alpha;
 		if (copy_to_user(buf, &global_alpha, sizeof(global_alpha)))
 			return -EFAULT;
-		pr_debug("get global alpha of index %d\n", mfbi->index);
 		break;
 	case MFB_SET_ALPHA:
 		/* set panel information */
@@ -1062,7 +1021,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		ad->src_size_g_alpha = (ad->src_size_g_alpha & (~0xff)) |
 							(global_alpha & 0xff);
 		mfbi->g_alpha = global_alpha;
-		pr_debug("set global alpha for index %d\n", mfbi->index);
 		break;
 	case MFB_SET_CHROMA_KEY:
 		/* set panel winformation */
@@ -1090,7 +1048,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 			ad->ckmin_g = ck.green_min;
 			ad->ckmin_b = ck.blue_min;
 		}
-		pr_debug("set chroma key\n");
 		break;
 	case FBIOGET_GWINFO:
 		if (mfbi->type = MFB_TYPE_OFF)
@@ -1110,7 +1067,7 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		break;

 	default:
-		printk(KERN_ERR "Unknown ioctl command (0x%08X)\n", cmd);
+		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
 		return -ENOIOCTLCMD;
 	}

@@ -1131,7 +1088,6 @@ static int fsl_diu_open(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count++;
 	if (mfbi->count = 1) {
-		pr_debug("open plane index %d\n", mfbi->index);
 		fsl_diu_check_var(&info->var, info);
 		res = fsl_diu_set_par(info);
 		if (res < 0)
@@ -1157,7 +1113,6 @@ static int fsl_diu_release(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count--;
 	if (mfbi->count = 0) {
-		pr_debug("release plane index %d\n", mfbi->index);
 		res = fsl_diu_disable_panel(info);
 		if (res < 0)
 			mfbi->count++;
@@ -1222,26 +1177,9 @@ static int __devinit install_fb(struct fb_info *info)
 	} else {
 		aoi_mode = init_aoi_mode;
 	}
-	pr_debug("mode used = %s\n", aoi_mode);
 	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize,
 			  &fsl_diu_default_mode, default_bpp);
-	switch (rc) {
-	case 1:
-		pr_debug("using mode specified in @mode\n");
-		break;
-	case 2:
-		pr_debug("using mode specified in @mode "
-			"with ignored refresh rate\n");
-		break;
-	case 3:
-		pr_debug("using mode default mode\n");
-		break;
-	case 4:
-		pr_debug("using mode from list\n");
-		break;
-	default:
-		pr_debug("rc = %d\n", rc);
-		pr_debug("failed to find mode\n");
+	if (!rc) {
 		/*
 		 * For plane 0 we continue and look into
 		 * driver's internal modedb.
@@ -1250,7 +1188,6 @@ static int __devinit install_fb(struct fb_info *info)
 			has_default_mode = 0;
 		else
 			return -EINVAL;
-		break;
 	}

 	if (!has_default_mode) {
@@ -1286,33 +1223,26 @@ static int __devinit install_fb(struct fb_info *info)
 		fb_videomode_to_var(&info->var, modedb);
 	}

-	pr_debug("xres_virtual %d\n", info->var.xres_virtual);
-	pr_debug("bits_per_pixel %d\n", info->var.bits_per_pixel);
-
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
-
 	if (mfbi->type = MFB_TYPE_OFF)
 		mfbi->blank = FB_BLANK_NORMAL;
 	else
 		mfbi->blank = FB_BLANK_UNBLANK;

 	if (fsl_diu_check_var(&info->var, info)) {
-		printk(KERN_ERR "fb_check_var failed");
+		dev_err(info->dev, "fsl_diu_check_var failed\n");
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}

 	if (register_framebuffer(info) < 0) {
-		printk(KERN_ERR "register_framebuffer failed");
+		dev_err(info->dev, "register_framebuffer failed\n");
 		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}

 	mfbi->registered = 1;
-	printk(KERN_INFO "fb%d: %s fb device registered successfully.\n",
-		 info->node, info->fix.id);
+	dev_info(info->dev, "%s registered successfully\n", mfbi->id);

 	return 0;
 }
@@ -1344,7 +1274,6 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 		/* This is the workaround for underrun */
 		if (status & INT_UNDRUN) {
 			out_be32(&hw->diu_mode, 0);
-			pr_debug("Err: DIU occurs underrun!\n");
 			udelay(1);
 			out_be32(&hw->diu_mode, 1);
 		}
@@ -1376,9 +1305,7 @@ static int request_irq_local(int irq)
 	status = in_be32(&hw->int_status);

 	ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
-	if (ret)
-		pr_info("Request diu IRQ failed.\n");
-	else {
+	if (!ret) {
 		ints = INT_PARERR | INT_LS_BF_VS;
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
 		ints |=	INT_VSYNC;
@@ -1557,7 +1484,6 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		dev_err(&ofdev->dev, "invalid DIU address\n");
 		goto error;
 	}
-	dev_dbg(&ofdev->dev, "%s, res.start: 0x%08x\n", __func__, res.start);

 	dr.diu_reg = ioremap(res.start, sizeof(struct diu));
 	if (!dr.diu_reg) {
@@ -1787,17 +1713,19 @@ static int __init fsl_diu_init(void)
 #else
 	monitor_port = fsl_diu_name_to_port(monitor_string);
 #endif
-	printk(KERN_INFO "Freescale DIU driver\n");
+	pr_info("Freescale Display Interface Unit (DIU) framebuffer driver\n");

 #ifdef CONFIG_NOT_COHERENT_CACHE
 	np = of_find_node_by_type(NULL, "cpu");
 	if (!np) {
-		printk(KERN_ERR "Err: can't find device node 'cpu'\n");
+		pr_err("fsl-diu-fb: can't find 'cpu' device node\n");
 		return -ENODEV;
 	}

 	prop = of_get_property(np, "d-cache-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
@@ -1811,6 +1739,8 @@ static int __init fsl_diu_init(void)

 	prop = of_get_property(np, "d-cache-line-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-line-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
@@ -1824,8 +1754,7 @@ static int __init fsl_diu_init(void)

 	ret = platform_driver_register(&fsl_diu_driver);
 	if (ret) {
-		printk(KERN_ERR
-			"fsl-diu: failed to register platform driver\n");
+		pr_err("fsl-diu-fb: failed to register platform driver\n");
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		vfree(coherence_data);
 #endif
@@ -1853,7 +1782,7 @@ module_param_named(mode, fb_mode, charp, 0);
 MODULE_PARM_DESC(mode,
 	"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
 module_param_named(bpp, default_bpp, ulong, 0);
-MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified mode");
+MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified in 'mode'");
 module_param_named(monitor, monitor_string, charp, 0);
 MODULE_PARM_DESC(monitor, "Specify the monitor port "
 	"(\"dvi\", \"lvds\", or \"dlvds\") if supported by the platform");
--
1.7.3.4



^ permalink raw reply related

* [PATCH 03/13] drivers/video: fsl-diu-fb: remove unused ioctls
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Remove some unused ioctl commands, and treat those commands as unsupported
instead of ignored.

Also remove struct mfb_alpha, which isn't used by any ioctl.  It may have
been once intended for MFB_SET_ALPHA, but that ioctl uses a different
data structure.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |   12 ------------
 include/linux/fsl-diu-fb.h |    5 -----
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 10ee411..226f4bc 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -902,9 +902,6 @@ static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
 			ret = 0;
 		}
 		break;
-	case FB_VISUAL_STATIC_PSEUDOCOLOR:
-	case FB_VISUAL_PSEUDOCOLOR:
-		break;
 	}
 
 	return ret;
@@ -1056,15 +1053,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_to_user(buf, ad, sizeof(*ad)))
 			return -EFAULT;
 		break;
-	case FBIOGET_HWCINFO:
-		pr_debug("FBIOGET_HWCINFO:0x%08x\n", FBIOGET_HWCINFO);
-		break;
-	case FBIOPUT_MODEINFO:
-		pr_debug("FBIOPUT_MODEINFO:0x%08x\n", FBIOPUT_MODEINFO);
-		break;
-	case FBIOGET_DISPINFO:
-		pr_debug("FBIOGET_DISPINFO:0x%08x\n", FBIOGET_DISPINFO);
-		break;
 
 	default:
 		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index daa9952..5ebffa6 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -27,11 +27,6 @@
 
 #include <linux/types.h>
 
-struct mfb_alpha {
-	int enable;
-	int alpha;
-};
-
 struct mfb_chroma_key {
 	int enable;
 	__u8  red_max;
-- 
1.7.3.4



^ permalink raw reply related

* [PATCH 04/13] drivers/video: fsl-diu-fb: fix compilation warning
From: Timur Tabi @ 2011-09-15 21:44 UTC (permalink / raw)
  To: linux-fbdev

Fix this compilation warning in the Freescale DIU framebuffer driver:

warning: 'dummy_ad_addr' may be used uninitialized in this function

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 226f4bc..6c544cf 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1429,7 +1429,7 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
 	struct mfb_info *mfbi;
-	phys_addr_t dummy_ad_addr;
+	phys_addr_t dummy_ad_addr = 0;
 	int ret, i, error = 0;
 	struct resource res;
 	struct fsl_diu_data *machine_data;
-- 
1.7.3.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