[RFC 0/5] Generic panel framework

[Tomi Valkeinen]

Hi,

On Fri, 2012-08-17 at 02:49 +0200, Laurent Pinchart wrote:

> I will appreciate all reviews, comments, criticisms, ideas, remarks, ... If

Oookay, where to start... ;)

A few cosmetic/general comments first.

I find the file naming a bit strange. You have panel.c, which is the
core framework, panel-dbi.c, which is the DBI bus, panel-r61517.c, which
is driver for r61517 panel...

Perhaps something in this direction (in order): panel-core.c,
mipi-dbi-bus.c, panel-r61517.c? And we probably end up with quite a lot
of panel drivers, perhaps we should already divide these into separate
directories, and then we wouldn't need to prefix each panel with
"panel-" at all.

---

Should we aim for DT only solution from the start? DT is the direction
we are going, and I feel the older platform data stuff would be
deprecated soon.

---

Something missing from the intro is how this whole thing should be used.
It doesn't help if we know how to turn on the panel, we also need to
display something on it =). So I think some kind of diagram/example of
how, say, drm would use this thing, and also how the SoC specific DBI
bus driver would be done, would clarify things.

---

We have discussed face to face about the different hardware setups and
scenarios that we should support, but I'll list some of them here for
others:

1) We need to support chains of external display chips and panels. A
simple example is a chip that takes DSI in, and outputs DPI. In that
case we'd have a chain of SoC -> DSI2DPI -> DPI panel.

In final products I think two external devices is the maximum (at least
I've never seen three devices in a row), but in theory and in
development environments the chain can be arbitrarily long. Also the
connections are not necessarily 1-to-1, but a device can take one input
while it has two outputs, or a device can take two inputs.

Now, I think two external devices is a must requirement. I'm not sure if
supporting more is an important requirement. However, if we support two
devices, it could be that it's trivial to change the framework to
support n devices.

2) Panels and display chips are all but standard. They very often have
their own sequences how to do things, have bugs, or implement some
feature in slightly different way than some other panel. This is why the
panel driver should be able to control or define the way things happen.

As an example, Sharp LQ043T1DG01 panel
(www.sharpsme.com/download/LQ043T1DG01-SP-072106pdf). It is enabled with
the following sequence:

- Enable VCC and AVDD regulators
- Wait min 50ms
- Enable full video stream (pck, syncs, pixels) from SoC
- Wait min 0.5ms
- Set DISP GPIO, which turns on the display panel

Here we could split the enabling of panel to two parts, prepare (in this
case starts regulators and waits 50ms) and finish (wait 0.5ms and set
DISP GPIO), and the upper layer would start the video stream in between.

I realize this could be done with the PANEL_ENABLE_* levels in your RFC,
but I don't think the concepts quite match:

- PANEL_ENABLE_BLANK level is needed for "smart panels", as we need to
configure them and send the initial frame at that operating level. With
dummy panels there's really no such level, there's just one enable
sequence that is always done right away.

- I find waiting at the beginning of a function very ugly (what are we
waiting for?) and we'd need that when changing the panel to
PANEL_ENABLE_ON level.

- It's still limited if the panel is a stranger one (see following
example).

Consider the following theoretical panel enable example, taken to absurd
level just to show the general problem:

- Enable regulators
- Enable video stream
- Wait 50ms
- Disable video stream
- Set enable GPIO
- Enable video stream

This one would be rather impossible with the upper layer handling the
enabling of the video stream. Thus I see that the panel driver needs to
control the sequences, and the Sharp panel driver's enable would look
something like:

regulator_enable(...);
sleep();
dpi_enable_video();
sleep();
gpip_set(..);

Note that even with this model we still need the PANEL_ENABLE levels you
have.

---

I'm not sure I understand the panel unload problem you mentioned. Nobody
should have direct references to the panel functions, so there shouldn't
be any automatic references that would prevent module unloading. So when
the user does rmmod panel-mypanel, the panel driver's remove will be
called. It'll unregister itself from the panel framework, which causes
notifications and the display driver will stop using the panel. After
that nobody has pointers to the panel, and it can safely be unloaded.

It could cause some locking issues, though. First the panel's remove
could take a lock, but the remove sequence would cause the display
driver to call disable on the panel, which could again try to take the
same lock...

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120817/2fd1cda9/attachment.pgp>