Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* 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: 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: 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: 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 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: 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: 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: 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: 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: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: [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: 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: Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-15 15:29 UTC (permalink / raw)
  To: Keith Packard
  Cc: linux-fbdev, linaro-dev, linux-kernel, dri-devel, Archit Taneja,
	Clark, Rob
In-Reply-To: <yunboul7sx7.fsf@aiko.keithp.com>

On Thu, 2011-09-15 at 09:59 -0500, Keith Packard wrote:
> On Thu, 15 Sep 2011 15:07:05 +0300, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
> > This was a very rough and quite short proposal, but I'm happy to improve
> > and extend it if it's not totally shot down.
> 
> Jesse Barnes has put together a proposal much like this to work within
> the existing DRM environment. This is pretty much the last piece of
> missing mode-setting functionality that we know of, making DRM capable
> of fully supporting existing (and planned) devices.
> 
> Here's a link to some older discussion on the issue, things have changed
> a bit since then and we had a long talk about this during the X
> Developers' Conference this week in Chicago. Expect an update to his
> proposal in the coming weeks.
> 
> http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html
> 

Thanks for the link.

Right, DRM has already components I described in my proposal, and adding
overlays brings it even closer. However, I think there are two major
differences:

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.

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.

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.

 Tomi



^ permalink raw reply

* Re: Proposal for a low-level Linux display framework
From: Kyungmin Park @ 2011-09-15 15:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, dri-devel, linaro-dev, Archit Taneja,
	대인기
In-Reply-To: <1316088425.11294.78.camel@lappyti>

Hi Tomi,

On Thu, Sep 15, 2011 at 9:07 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> I am the author of OMAP display driver, and while developing it I've
> often felt that there's something missing in Linux's display area. I've
> been planning to write a post about this for a few years already, but I
> never got to it. So here goes at last!
>
> ---
>
> First I want to (try to) describe shortly what we have on OMAP, to give
> a bit of a background for my point of view, and to have an example HW.
>
> The display subsystem (DSS) hardware on OMAP handles only showing pixels
> on a display, so it doesn't contain anything that produces pixels like
> 3D stuff or accelerated copying. All it does is fetch pixels from SDRAM,
> possibly do some modifications for them (color format conversions etc),
> and output them to a display.
>
> The hardware has multiple overlays, which are like hardware windows.
> They fetch pixels from SDRAM, and output them in a certain area on the
> display (possibly with scaling). Multiple overlays can be composited
> into one output.
>
> So we may have something like this, when all overlays read pixels from
> separate areas in the memory, and all overlays are on LCD display:
>
>  .-----.         .------.           .------.
>  | mem |-------->| ovl0 |-----.---->| LCD  |
>  '-----'         '------'     |     '------'
>  .-----.         .------.     |
>  | mem |-------->| ovl1 |-----|
>  '-----'         '------'     |
>  .-----.         .------.     |     .------.
>  | mem |-------->| ovl2 |-----'     |  TV  |
>  '-----'         '------'           '------'
>
Same feature at samsung display subsystem.

> The LCD display can be rather simple one, like a standard monitor or a
> simple panel directly connected to parallel RGB output, or a more
> complex one. A complex panel needs something else than just
> turn-it-on-and-go. This may involve sending and receiving messages
> between OMAP and the panel, but more generally, there's need to have
> custom code that handles the particular panel. And the complex panel is
> not necessarily a panel at all, it may be a buffer chip between OMAP and
> the actual panel.
>
> The software side can be divided into three parts: the lower level
> omapdss driver, the lower level panel drivers, and higher level drivers
> like omapfb, v4l2 and omapdrm.

Current omapdrm codes use the omapfb and omapdss codes even though
omapdrm is located drivers/staging, some time later it should be
drivers/gpu/gem/omap. but it still uses the drivers/video/omap2/dss
codes.
In case of samsung DRM, it has almost similar codes for lowlevel
access from the drivers/video/s3c-fb.c for FIMD and
drivers/media/video/s5p-tv for HDMI.


>
> The omapdss driver handles the OMAP DSS hardware, and offers a kernel
> internal API which the higher level drivers use. The omapdss does not
> know anything about fb or drm, it just offers core display services.
>
> The panel drivers handle particular panels/chips. The panel driver may
> be very simple in case of a conventional display, basically doing pretty
> much nothing, or bigger piece of code, handling communication with the
> panel.
>
> The higher level drivers handle buffers and tell omapdss things like
> where to find the pixels, what size the overlays should be, and use the
> omapdss API to turn displays on/off, etc.
>
> ---
>
> There are two things that I'm proposing to improve the Linux display
> support:
>
> First, there should be a bunch of common video structs and helpers that
> are independent of any higher level framework. Things like video
> timings, mode databases, and EDID seem to be implemented multiple times
> in the kernel. But there shouldn't be anything in those things that
> depend on any particular display framework, so they could be implemented
> just once and all the frameworks could use them.
>
> Second, I think there could be use for a common low level display
> framework. Currently the lower level code (display HW handling, etc.)
> and higher level code (buffer management, policies, etc) seem to be
> usually tied together, like the fb framework or the drm. Granted, the
> frameworks do not force that, and for OMAP we indeed have omapfb and
> omapdrm using the lower level omapdss. But I don't see that it's
> anything OMAP specific as such.

So I suggest the create the drivers/graphics for lowlevel codes and
each framework, DRM, V4L2 and FB uses these lowlevel codes.

Thank you,
Kyungmin Park
>
> I think the lower level framework could have components something like
> this (the naming is OMAP oriented, of course):
>
> overlay - a hardware "window", gets pixels from memory, possibly does
> format conversions, scaling, etc.
>
> overlay compositor - composes multiple overlays into one output,
> possibly doing things like translucency.
>
> output - gets the pixels from overlay compositor, and sends them out
> according to particular video timings when using conventional video
> interface, or via any other mean when using non-conventional video buses
> like DSI command mode.
>
> display - handles an external display. For conventional displays this
> wouldn't do much, but for complex ones it does whatever needed by that
> particular display.
>
> This is something similar to what DRM has, I believe. The biggest
> difference is that the display can be a full blown driver for a complex
> piece of HW.
>
> This kind of low level framework would be good for two purposes: 1) I
> think it's a good division generally, having the low level HW driver
> separate from the higher level buffer/policy management and 2) fb, drm,
> v4l2 or any possible future framework could all use the same low level
> framework.
>
> ---
>
> Now, I'm quite sure the above framework could work quite well with any
> OMAP like hardware, with unified memory (i.e. the video buffers are in
> SDRAM) and 3D chips and similar components are separate. But what I'm
> not sure is how desktop world's gfx cards change things. Most probably
> all the above components can be found from there also in some form, but
> are there some interdependencies between 3D/buffer management/something
> else and the video output side?
>
> This was a very rough and quite short proposal, but I'm happy to improve
> and extend it if it's not totally shot down.
>
>  Tomi
>
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>

^ permalink raw reply

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

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

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

> This was a very rough and quite short proposal, but I'm happy to improve
> and extend it if it's not totally shot down.

Jesse Barnes has put together a proposal much like this to work within
the existing DRM environment. This is pretty much the last piece of
missing mode-setting functionality that we know of, making DRM capable
of fully supporting existing (and planned) devices.

Here's a link to some older discussion on the issue, things have changed
a bit since then and we had a long talk about this during the X
Developers' Conference this week in Chicago. Expect an update to his
proposal in the coming weeks.

http://lists.freedesktop.org/archives/dri-devel/2011-April/010559.html

-- 
keith.packard@intel.com

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

^ permalink raw reply

* Proposal for a low-level Linux display framework
From: Tomi Valkeinen @ 2011-09-15 12:07 UTC (permalink / raw)
  To: linux-fbdev, linux-kernel, dri-devel, linaro-dev
  Cc: Clark, Rob, Archit Taneja

Hi,

I am the author of OMAP display driver, and while developing it I've
often felt that there's something missing in Linux's display area. I've
been planning to write a post about this for a few years already, but I
never got to it. So here goes at last!

---

First I want to (try to) describe shortly what we have on OMAP, to give
a bit of a background for my point of view, and to have an example HW.

The display subsystem (DSS) hardware on OMAP handles only showing pixels
on a display, so it doesn't contain anything that produces pixels like
3D stuff or accelerated copying. All it does is fetch pixels from SDRAM,
possibly do some modifications for them (color format conversions etc),
and output them to a display.

The hardware has multiple overlays, which are like hardware windows.
They fetch pixels from SDRAM, and output them in a certain area on the
display (possibly with scaling). Multiple overlays can be composited
into one output.

So we may have something like this, when all overlays read pixels from
separate areas in the memory, and all overlays are on LCD display:

 .-----.         .------.           .------.
 | mem |-------->| ovl0 |-----.---->| LCD  |
 '-----'         '------'     |     '------'
 .-----.         .------.     |
 | mem |-------->| ovl1 |-----|
 '-----'         '------'     |
 .-----.         .------.     |     .------.
 | mem |-------->| ovl2 |-----'     |  TV  |
 '-----'         '------'           '------'

The LCD display can be rather simple one, like a standard monitor or a
simple panel directly connected to parallel RGB output, or a more
complex one. A complex panel needs something else than just
turn-it-on-and-go. This may involve sending and receiving messages
between OMAP and the panel, but more generally, there's need to have
custom code that handles the particular panel. And the complex panel is
not necessarily a panel at all, it may be a buffer chip between OMAP and
the actual panel.

The software side can be divided into three parts: the lower level
omapdss driver, the lower level panel drivers, and higher level drivers
like omapfb, v4l2 and omapdrm.

The omapdss driver handles the OMAP DSS hardware, and offers a kernel
internal API which the higher level drivers use. The omapdss does not
know anything about fb or drm, it just offers core display services.

The panel drivers handle particular panels/chips. The panel driver may
be very simple in case of a conventional display, basically doing pretty
much nothing, or bigger piece of code, handling communication with the
panel.

The higher level drivers handle buffers and tell omapdss things like
where to find the pixels, what size the overlays should be, and use the
omapdss API to turn displays on/off, etc.

---

There are two things that I'm proposing to improve the Linux display
support:

First, there should be a bunch of common video structs and helpers that
are independent of any higher level framework. Things like video
timings, mode databases, and EDID seem to be implemented multiple times
in the kernel. But there shouldn't be anything in those things that
depend on any particular display framework, so they could be implemented
just once and all the frameworks could use them.

Second, I think there could be use for a common low level display
framework. Currently the lower level code (display HW handling, etc.)
and higher level code (buffer management, policies, etc) seem to be
usually tied together, like the fb framework or the drm. Granted, the
frameworks do not force that, and for OMAP we indeed have omapfb and
omapdrm using the lower level omapdss. But I don't see that it's
anything OMAP specific as such.

I think the lower level framework could have components something like
this (the naming is OMAP oriented, of course):

overlay - a hardware "window", gets pixels from memory, possibly does
format conversions, scaling, etc.

overlay compositor - composes multiple overlays into one output,
possibly doing things like translucency.

output - gets the pixels from overlay compositor, and sends them out
according to particular video timings when using conventional video
interface, or via any other mean when using non-conventional video buses
like DSI command mode.

display - handles an external display. For conventional displays this
wouldn't do much, but for complex ones it does whatever needed by that
particular display.

This is something similar to what DRM has, I believe. The biggest
difference is that the display can be a full blown driver for a complex
piece of HW.

This kind of low level framework would be good for two purposes: 1) I
think it's a good division generally, having the low level HW driver
separate from the higher level buffer/policy management and 2) fb, drm,
v4l2 or any possible future framework could all use the same low level
framework.

---

Now, I'm quite sure the above framework could work quite well with any
OMAP like hardware, with unified memory (i.e. the video buffers are in
SDRAM) and 3D chips and similar components are separate. But what I'm
not sure is how desktop world's gfx cards change things. Most probably
all the above components can be found from there also in some form, but
are there some interdependencies between 3D/buffer management/something
else and the video output side?

This was a very rough and quite short proposal, but I'm happy to improve
and extend it if it's not totally shot down.

 Tomi



^ permalink raw reply

* Re: [PATCH] FB: add early fb blank feature.
From: Lars-Peter Clausen @ 2011-09-15 11:37 UTC (permalink / raw)
  To: Inki Dae
  Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>

Hi

I have a LCD panel with an similar issue, and I think the idea to introduce a
early fb blank event is the right solution. I have some comments and questions
on this particular implementation though.

On 09/09/2011 07:03 AM, Inki Dae wrote:
> this patch adds early fb blank feature that this is a callback of
> lcd panel driver would be called prior to fb driver's one.
> in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> the power off commands should be transferred to lcd panel with display
> and mipi-dsi controller enabled because the commands is set to lcd panel
> at vsync porch period. on the other hand, in opposite case, the callback
> of fb driver should be called prior to lcd panel driver's one because of
> same issue. now we could handle call order to fb blank properly.
> 
> the order is as the following:
> 
> at fb_blank function of fbmem.c
>   -> fb_early_notifier_call_chain()
>      -> lcd panel driver's early_set_power()
>   -> info->fbops->fb_blank()
>      -> fb driver's fb_blank()
>   -> fb_notifier_call_chain()
>      -> lcd panel driver's set_power()
> 

I wonder if we really need the lcd_ops early_set_power callback. I can't really
imagine a situation where you need to power the LCD down only after the LCD
controller has been shutdown.

So I wonder if we couldn't just have the set_power callback, but listen to both
events and call set_power for early blank events with code != FB_BLANK_UNBLANK
and for normal blank events with code = FB_BLANK_UNBLANK?

> note that early fb blank mode is valid only if lcd_ops->early_blank_mode is 1.
> if the value is 0 then early fb blank callback would be ignored.
> 
> this patch is based on git repository below:
> git://github.com/schandinat/linux-2.6.git
> branch: fbdev-next
> commit-id: a67472ad1ae040f073e45048cbc5a01195f2e3f5
> 
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: KyungMin Park <kyungmin.park@samsung.com>
> ---
>  drivers/video/backlight/lcd.c |   77 ++++++++++++++++++++++++++++++++++++++--
>  drivers/video/fb_notify.c     |   31 ++++++++++++++++
>  drivers/video/fbmem.c         |   25 +++++++------
>  include/linux/fb.h            |    4 ++
>  include/linux/lcd.h           |   61 ++++++++++++++++++++++++--------

In my opinion this should be split into two patches, one adding the early blank
event, one adding support for it to the LCD framework.

>  5 files changed, 167 insertions(+), 31 deletions(-)
> 
> [...]
> diff --git a/drivers/video/fb_notify.c b/drivers/video/fb_notify.c
> index 8c02038..3930c7c 100644
> --- a/drivers/video/fb_notify.c
> +++ b/drivers/video/fb_notify.c
> @@ -13,9 +13,20 @@
>  #include <linux/fb.h>
>  #include <linux/notifier.h>
>  
> +static BLOCKING_NOTIFIER_HEAD(fb_early_notifier_list);
>  static BLOCKING_NOTIFIER_HEAD(fb_notifier_list);
>  
>  /**
> + *	fb_register_early_client - register a client early notifier
> + *	@nb: notifier block to callback on events
> + */
> +int fb_register_early_client(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&fb_early_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(fb_register_early_client);
> +

Why do we need a new notifier chain? Can't we introduce a new event for the
existing chain?

> +/**
>   *	fb_register_client - register a client notifier
>   *	@nb: notifier block to callback on events
>   */
> @@ -26,6 +37,16 @@ int fb_register_client(struct notifier_block *nb)
>  EXPORT_SYMBOL(fb_register_client);
>  
>  /**
> + *	fb_unregister_early_client - unregister a client early notifier
> + *	@nb: notifier block to callback on events
> + */
> +int fb_unregister_early_client(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&fb_early_notifier_list, nb);
> +}
> +EXPORT_SYMBOL(fb_unregister_early_client);
> +
> +/**
>   *	fb_unregister_client - unregister a client notifier
>   *	@nb: notifier block to callback on events
>   */
> @@ -36,6 +57,16 @@ int fb_unregister_client(struct notifier_block *nb)
>  EXPORT_SYMBOL(fb_unregister_client);
>  
>  /**
> + * fb_early_notifier_call_chain - early notify clients of fb_events
> + *
> + */
> +int fb_early_notifier_call_chain(unsigned long val, void *v)
> +{
> +	return blocking_notifier_call_chain(&fb_early_notifier_list, val, v);
> +}
> +EXPORT_SYMBOL_GPL(fb_early_notifier_call_chain);
> +
> +/**
>   * fb_notifier_call_chain - notify clients of fb_events
>   *
>   */
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index ad93629..cf22516 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1031,24 +1031,25 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
>  
>  int
>  fb_blank(struct fb_info *info, int blank)
> -{	
> - 	int ret = -EINVAL;
> +{
> +	struct fb_event event;
> +	int ret = -EINVAL;
>  
> - 	if (blank > FB_BLANK_POWERDOWN)
> - 		blank = FB_BLANK_POWERDOWN;
> +	if (blank > FB_BLANK_POWERDOWN)
> +		blank = FB_BLANK_POWERDOWN;
>  
> -	if (info->fbops->fb_blank)
> - 		ret = info->fbops->fb_blank(blank, info);
> +	event.info = info;
> +	event.data = &blank;
>  
> - 	if (!ret) {
> -		struct fb_event event;
> +	fb_early_notifier_call_chain(FB_EVENT_BLANK, &event);
>  
> -		event.info = info;
> -		event.data = &blank;
> +	if (info->fbops->fb_blank)
> +		ret = info->fbops->fb_blank(blank, info);

I think we have to handle the case where the fb_blank callback fails and should
somehow revert the effects of the early blank event.


> +
> +	if (!ret)
>  		fb_notifier_call_chain(FB_EVENT_BLANK, &event);
> -	}
>  



> - 	return ret;
> +	return ret;
>  }
>  
>  static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 1d6836c..1d7d995 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -562,6 +562,10 @@ struct fb_blit_caps {
>  	u32 flags;
>  };
>  
> +extern int fb_register_early_client(struct notifier_block *nb);
> +extern int fb_unregister_early_client(struct notifier_block *nb);
> +extern int fb_early_notifier_call_chain(unsigned long val, void *v);
> +
>  extern int fb_register_client(struct notifier_block *nb);
>  extern int fb_unregister_client(struct notifier_block *nb);
>  extern int fb_notifier_call_chain(unsigned long val, void *v);
> diff --git a/include/linux/lcd.h b/include/linux/lcd.h
> index 8877123..930d1cc 100644
> --- a/include/linux/lcd.h
> +++ b/include/linux/lcd.h
> @@ -37,10 +37,21 @@ struct lcd_properties {
>  };
>  
>  struct lcd_ops {
> -	/* Get the LCD panel power status (0: full on, 1..3: controller
> -	   power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
> +	/*
> +	 * Get the LCD panel power status (0: full on, 1..3: controller
> +	 * power on, flat panel power off, 4: full off), see FB_BLANK_XXX
> +	 */
>  	int (*get_power)(struct lcd_device *);
> -	/* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) */
> +	/*
> +	 * Get the current contrast setting (0-max_contrast) and

???

> +	 * Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX)
> +	 * this callback would be called proir to fb driver's fb_blank callback.
> +	 */
> +	int (*early_set_power)(struct lcd_device *, int power);
> +	/*
> +	 * Get the current contrast setting (0-max_contrast)
> +	 * Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX)
> +	 */
>  	int (*set_power)(struct lcd_device *, int power);
>  	/* Get the current contrast setting (0-max_contrast) */
>  	int (*get_contrast)(struct lcd_device *);
> @@ -48,21 +59,35 @@ struct lcd_ops {
>          int (*set_contrast)(struct lcd_device *, int contrast);
>  	/* Set LCD panel mode (resolutions ...) */
>  	int (*set_mode)(struct lcd_device *, struct fb_videomode *);
> -	/* Check if given framebuffer device is the one LCD is bound to;
> -	   return 0 if not, !=0 if it is. If NULL, lcd always matches the fb. */
> +	/*
> +	 * Check if given framebuffer device is the one LCD is bound to;
> +	 * return 0 if not, !=0 if it is. If NULL, lcd always matches the fb.
> +	 */
>  	int (*check_fb)(struct lcd_device *, struct fb_info *);
> +
> +	/*
> +	 * indicate whether enabling early blank mode or not.
> +	 * (0: disable; 1: enable);
> +	 * if enabled, lcd blank callback would be called prior
> +	 * to fb blank callback.
> +	 */
> +	unsigned int early_blank_mode;

I think it should be sufficient to check early_set_power for NULL instead of
adding this additional flag.

>  };
>  
>  struct lcd_device {
>  	struct lcd_properties props;
> -	/* This protects the 'ops' field. If 'ops' is NULL, the driver that
> -	   registered this device has been unloaded, and if class_get_devdata()
> -	   points to something in the body of that driver, it is also invalid. */
> +	/*
> +	 * This protects the 'ops' field. If 'ops' is NULL, the driver that
> +	 * registered this device has been unloaded, and if class_get_devdata()
> +	 * points to something in the body of that driver, it is also invalid.
> +	 */
>  	struct mutex ops_lock;
>  	/* If this is NULL, the backing module is unloaded */
>  	struct lcd_ops *ops;
>  	/* Serialise access to set_power method */
>  	struct mutex update_lock;
> +	/* The framebuffer early notifier block */
> +	struct notifier_block fb_early_notif;
>  	/* The framebuffer notifier block */
>  	struct notifier_block fb_notif;
>  
> @@ -72,16 +97,22 @@ struct lcd_device {
>  struct lcd_platform_data {
>  	/* reset lcd panel device. */
>  	int (*reset)(struct lcd_device *ld);
> -	/* on or off to lcd panel. if 'enable' is 0 then
> -	   lcd power off and 1, lcd power on. */
> +	/*
> +	 * on or off to lcd panel. if 'enable' is 0 then
> +	 * lcd power off and 1, lcd power on.
> +	 */
>  	int (*power_on)(struct lcd_device *ld, int enable);
>  
> -	/* it indicates whether lcd panel was enabled
> -	   from bootloader or not. */
> +	/*
> +	 * it indicates whether lcd panel was enabled
> +	 * from bootloader or not.
> +	 */
>  	int lcd_enabled;
> -	/* it means delay for stable time when it becomes low to high
> -	   or high to low that is dependent on whether reset gpio is
> -	   low active or high active. */
> +	/*
> +	 * it means delay for stable time when it becomes low to high
> +	 * or high to low that is dependent on whether reset gpio is
> +	 * low active or high active.
> +	 */

The formatting cleanup patches should go into a separate patch.

^ permalink raw reply

* RE: [PATCH] FB: add early fb blank feature.
From: Tomi Valkeinen @ 2011-09-15 10:26 UTC (permalink / raw)
  To: Inki Dae
  Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <000001cc7390$482c00f0$d88402d0$%dae@samsung.com>

On Thu, 2011-09-15 at 19:14 +0900, Inki Dae wrote:
> Hi, Tomi.
> 
> > -----Original Message-----
> > From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> > Sent: Thursday, September 15, 2011 6:53 PM
> > To: Inki Dae
> > Cc: FlorianSchandinat@gmx.de; linux-fbdev@vger.kernel.org; akpm@linux-
> > foundation.org; linux-kernel@vger.kernel.org; kyungmin.park@samsung.com
> > Subject: Re: [PATCH] FB: add early fb blank feature.
> > 
> > Hi,
> > 
> > On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> > > this patch adds early fb blank feature that this is a callback of
> > > lcd panel driver would be called prior to fb driver's one.
> > > in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> > > the power off commands should be transferred to lcd panel with display
> > > and mipi-dsi controller enabled because the commands is set to lcd panel
> > > at vsync porch period. on the other hand, in opposite case, the callback
> > > of fb driver should be called prior to lcd panel driver's one because of
> > > same issue. now we could handle call order to fb blank properly.
> > >
> > > the order is as the following:
> > >
> > > at fb_blank function of fbmem.c
> > >   -> fb_early_notifier_call_chain()
> > >      -> lcd panel driver's early_set_power()
> > >   -> info->fbops->fb_blank()
> > >      -> fb driver's fb_blank()
> > >   -> fb_notifier_call_chain()
> > >      -> lcd panel driver's set_power()
> > 
> > I'm not familiar with the lcd.c, so I may be talking nonsense, but I
> > don't quite understand the need for this patch. If you have some kind of
> > panel driver, shouldn't the panel driver handle power off in just one
> > place?
> > 
> 
> 
> Yes, almost lcd panels are ok. but for command setting, our lcd panel, mipi-dsi based RGB panel, should be power on before it transfers commands to panel. for example, if user requested FB_BLANK_POWERDOWN then first, display controller would be off and then set_power callback of lcd panel driver would be called. at this time, it has a problem. The problem is that display controller already is off so lcd panel can't accept some commands, such as sleep in command. Lcd panel can accept such commands with vsync period. And also there is another case. This is sparkling issue and would have implications for all ones.
> for this, you can refer to a link below:
> < http://adras.com/fixed-sparkling-issue-on-lcd-panel-when-fb-blank-mode-is.t196691-141.html >

Ok, I see. The architecture for omap display is a bit different, so we
don't have similar problems. We don't use the fb notifier at all, but
omapfb handles calling the necessary functions in the panel driver.

 Tomi



^ permalink raw reply

* RE: [PATCH] FB: add early fb blank feature.
From: Inki Dae @ 2011-09-15 10:14 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>

Hi, Tomi.

> -----Original Message-----
> From: Tomi Valkeinen [mailto:tomi.valkeinen@ti.com]
> Sent: Thursday, September 15, 2011 6:53 PM
> To: Inki Dae
> Cc: FlorianSchandinat@gmx.de; linux-fbdev@vger.kernel.org; akpm@linux-
> foundation.org; linux-kernel@vger.kernel.org; kyungmin.park@samsung.com
> Subject: Re: [PATCH] FB: add early fb blank feature.
> 
> Hi,
> 
> On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> > this patch adds early fb blank feature that this is a callback of
> > lcd panel driver would be called prior to fb driver's one.
> > in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> > the power off commands should be transferred to lcd panel with display
> > and mipi-dsi controller enabled because the commands is set to lcd panel
> > at vsync porch period. on the other hand, in opposite case, the callback
> > of fb driver should be called prior to lcd panel driver's one because of
> > same issue. now we could handle call order to fb blank properly.
> >
> > the order is as the following:
> >
> > at fb_blank function of fbmem.c
> >   -> fb_early_notifier_call_chain()
> >      -> lcd panel driver's early_set_power()
> >   -> info->fbops->fb_blank()
> >      -> fb driver's fb_blank()
> >   -> fb_notifier_call_chain()
> >      -> lcd panel driver's set_power()
> 
> I'm not familiar with the lcd.c, so I may be talking nonsense, but I
> don't quite understand the need for this patch. If you have some kind of
> panel driver, shouldn't the panel driver handle power off in just one
> place?
> 


Yes, almost lcd panels are ok. but for command setting, our lcd panel, mipi-dsi based RGB panel, should be power on before it transfers commands to panel. for example, if user requested FB_BLANK_POWERDOWN then first, display controller would be off and then set_power callback of lcd panel driver would be called. at this time, it has a problem. The problem is that display controller already is off so lcd panel can't accept some commands, such as sleep in command. Lcd panel can accept such commands with vsync period. And also there is another case. This is sparkling issue and would have implications for all ones.
for this, you can refer to a link below:
< http://adras.com/fixed-sparkling-issue-on-lcd-panel-when-fb-blank-mode-is.t196691-141.html >


> With omapfb and omapdss, the omapfb's fb_blank function just calls power
> off in the panel driver, which handles all necessary actions. Is your
> model somehow totally different?
> 
>  Tomi

Best Regards,
Inki Dae.


^ permalink raw reply

* Re: [PATCH] FB: add early fb blank feature.
From: Tomi Valkeinen @ 2011-09-15  9:53 UTC (permalink / raw)
  To: Inki Dae
  Cc: FlorianSchandinat, linux-fbdev, akpm, linux-kernel, kyungmin.park
In-Reply-To: <1315544581-16379-1-git-send-email-inki.dae@samsung.com>

Hi,

On Fri, 2011-09-09 at 14:03 +0900, Inki Dae wrote:
> this patch adds early fb blank feature that this is a callback of
> lcd panel driver would be called prior to fb driver's one.
> in case of MIPI-DSI based video mode LCD Panel, for lcd power off,
> the power off commands should be transferred to lcd panel with display
> and mipi-dsi controller enabled because the commands is set to lcd panel
> at vsync porch period. on the other hand, in opposite case, the callback
> of fb driver should be called prior to lcd panel driver's one because of
> same issue. now we could handle call order to fb blank properly.
> 
> the order is as the following:
> 
> at fb_blank function of fbmem.c
>   -> fb_early_notifier_call_chain()
>      -> lcd panel driver's early_set_power()
>   -> info->fbops->fb_blank()
>      -> fb driver's fb_blank()
>   -> fb_notifier_call_chain()
>      -> lcd panel driver's set_power()

I'm not familiar with the lcd.c, so I may be talking nonsense, but I
don't quite understand the need for this patch. If you have some kind of
panel driver, shouldn't the panel driver handle power off in just one
place?

With omapfb and omapdss, the omapfb's fb_blank function just calls power
off in the panel driver, which handles all necessary actions. Is your
model somehow totally different?

 Tomi



^ permalink raw reply

* Re: How to use backlight device with fb device
From: Tomi Valkeinen @ 2011-09-15  9:18 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1316072820.1880.51.camel@deskari>

On Thu, 2011-09-15 at 08:58 +0000, Florian Tobias Schandinat wrote:
> Hi Tomi,
> 
> On 09/15/2011 07:47 AM, Tomi Valkeinen wrote:
> > Hi,
> > 
> > I'm a bit confused how a backlight device and a fb device should be
> > "connected".
> > 
> > In this particular case the OMAP 4430SDP board has an LCD panel
> > controlled by omapfb (and omapdss on the lower level) with a backlight
> > controlled by pwm_backlight (and twl6030-pwm on the lower level).
> > 
> > Both omapfb and pwm_backlight work fine, but they don't know anything
> > about each other, so, for example, blanking the fb via omapfb leaves the
> > backlight on.
> > 
> > Is there something I'm missing, or is the only solution to have some
> > custom code to connect the fb and the bl, and handle it that way?
> 
> I don't know anything about backlight but recently there was a patch that
> affected the same area you mention so it might give an idea
> "[PATCH] FB: add early fb blank feature."
> http://marc.info/?l=linux-fbdev&m\x131554458720940&w=2

Ah, thanks for pointing that out.

I seem to have dropped from linux-fbdev list and hadn't received that
post. For some reason I seem to get dropped from linux mailing lists,
this is getting a bit frustrating...

 Tomi



^ permalink raw reply

* Re: How to use backlight device with fb device
From: Florian Tobias Schandinat @ 2011-09-15  8:58 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1316072820.1880.51.camel@deskari>

Hi Tomi,

On 09/15/2011 07:47 AM, Tomi Valkeinen wrote:
> Hi,
> 
> I'm a bit confused how a backlight device and a fb device should be
> "connected".
> 
> In this particular case the OMAP 4430SDP board has an LCD panel
> controlled by omapfb (and omapdss on the lower level) with a backlight
> controlled by pwm_backlight (and twl6030-pwm on the lower level).
> 
> Both omapfb and pwm_backlight work fine, but they don't know anything
> about each other, so, for example, blanking the fb via omapfb leaves the
> backlight on.
> 
> Is there something I'm missing, or is the only solution to have some
> custom code to connect the fb and the bl, and handle it that way?

I don't know anything about backlight but recently there was a patch that
affected the same area you mention so it might give an idea
"[PATCH] FB: add early fb blank feature."
http://marc.info/?l=linux-fbdev&m\x131554458720940&w=2


Best regards,

Florian Tobias Schandinat

> 
>  Tomi
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* How to use backlight device with fb device
From: Tomi Valkeinen @ 2011-09-15  7:47 UTC (permalink / raw)
  To: linux-fbdev

Hi,

I'm a bit confused how a backlight device and a fb device should be
"connected".

In this particular case the OMAP 4430SDP board has an LCD panel
controlled by omapfb (and omapdss on the lower level) with a backlight
controlled by pwm_backlight (and twl6030-pwm on the lower level).

Both omapfb and pwm_backlight work fine, but they don't know anything
about each other, so, for example, blanking the fb via omapfb leaves the
backlight on.

Is there something I'm missing, or is the only solution to have some
custom code to connect the fb and the bl, and handle it that way?

 Tomi



^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-15  6:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1316066265.1880.6.camel@deskari>

Hi,

On Thu, Sep 15, 2011 at 11:27 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Thu, 2011-09-15 at 11:11 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>> > Yes, you are right, detect() does not "know" if the monitor has changed
>> > between polls, so both notification and polling are needed. I
>> > implemented only polling as there's no HPD event mechanism yet in
>> > omapdss, and also because this was simple and gives DRM basic ability to
>> > detect a monitor.
>> >
>> If it is needed for DRM then it is fine, but with detect renamed to
>> poll. By next week i should have a patch ready for HPD event
>> mechanism.
>
> What is wrong with "detect"? It detects if there's a display connected.
> It can be used in polling manner, trying it every n seconds, but it
> should also be used even if you use HPD event. I think the normal
> sequence would be something like:
>
> 1) register HPD event
> 2) use detect() to see if a monitor is already connected
>
I guess polling ever few seconds to detect would be waste of CPU
cycles when there is already a mechanism in the H/w to detect the
connection.
Current sequence :
Enable display ( Irrespective of whether the cable is connected on not)

Sequence with HPD:
1.Register for HPD connect.
2.Enable display
3.Notify DRM/Audio/Kernel component that wants to listen to this event.

Thanks and regards,
Mythri.

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-15  6:32 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B_WvtNTi9ckyor-+5bo7n6GkeTSAvhDsrGQxhPZWgXO0w@mail.gmail.com>

On Thu, 2011-09-15 at 11:54 +0530, K, Mythri P wrote:
> Hi,
> 
> On Thu, Sep 15, 2011 at 11:27 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Thu, 2011-09-15 at 11:11 +0530, K, Mythri P wrote:
> >> Hi,
> >>
> >> On Wed, Sep 14, 2011 at 7:41 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> >> > Yes, you are right, detect() does not "know" if the monitor has changed
> >> > between polls, so both notification and polling are needed. I
> >> > implemented only polling as there's no HPD event mechanism yet in
> >> > omapdss, and also because this was simple and gives DRM basic ability to
> >> > detect a monitor.
> >> >
> >> If it is needed for DRM then it is fine, but with detect renamed to
> >> poll. By next week i should have a patch ready for HPD event
> >> mechanism.
> >
> > What is wrong with "detect"? It detects if there's a display connected.
> > It can be used in polling manner, trying it every n seconds, but it
> > should also be used even if you use HPD event. I think the normal
> > sequence would be something like:
> >
> > 1) register HPD event
> > 2) use detect() to see if a monitor is already connected
> >
> I guess polling ever few seconds to detect would be waste of CPU
> cycles when there is already a mechanism in the H/w to detect the
> connection.

Obviously. Polling is only used if hot-plug-detect is not available. But
detect function can be used even when HPD is available.

> Current sequence :
> Enable display ( Irrespective of whether the cable is connected on not)
> 
> Sequence with HPD:
> 1.Register for HPD connect.
> 2.Enable display
> 3.Notify DRM/Audio/Kernel component that wants to listen to this event.

Why would you enable the display even if there's no monitor connected?

And when the DRM starts, how does DRM know if the display was already
connected? Would you send a HPD event when DRM registers to the event
even if there's no actual plug-in event done (i.e. user actually
connecting the cable)?

And just to clarify, my sequence example was from DRM's point of view.
The HDMI driver shouldn't do anything before DRM/omapfb asks it to do
something.

 Tomi



^ permalink raw reply


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