On Fri, 2012-08-17 at 13:10 +0200, Laurent Pinchart wrote:

What kind of directory structure do you have in mind ? Panels are already 
isolated in drivers/video/panel/ so we could already ditch the panel- prefix 
in drivers.

The same directory also contains files for the framework and buses. But
perhaps there's no need for additional directories if the amount of
non-panel files is small. And you can easily see from the name that they
are not panel drivers (e.g. mipi_dbi_bus.c).

Would you also create include/video/panel/ ?

Perhaps that would be good. Well, having all the files prefixed with
panel- is not bad as such, but just feel extra.

quoted

---

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.

Don't forget about non-ARM architectures :-/ We need panel drivers for SH as 
well, which doesn't use DT. I don't think that would be a big issue, a DT-
compliant solution should be easy to use through board code and platform data 
as well.

I didn't forget about them as I didn't even think about them ;). I
somehow had the impression that other architectures would also use DT,
sooner or later. I could be mistaken, though.

And true, it's not a big issue to support both DT and non-DT versions,
but I've been porting omap stuff for DT and keeping the old platform
data stuff also there, and it just feels burdensome. For very simple
panels it's easy, but when you've passing lots of parameters the code
starts to get longer.

quoted

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(..);

I have to admit I have no better solution to propose at the moment, even if I 
don't really like making the panel control the video stream. When several 
devices will be present in the chain all of them might have similar annoying 
requirements, and my feeling is that the resulting code will be quite messy. 
At the end of the day the only way to really find out is to write an 
implementation.

If we have a chain of devices, and each device uses the bus interface
from the previous device in the chain, there shouldn't be a problem. In
that model each device can handle the task however is best for it.

I think the problems come from the divided control we'll have. I mean,
if the panel driver would decide itself what to send to its output, and
it would provide the data (think of an i2c device), this would be very
simple. And it actually is for things like configuration data etc, but
not so for video stream.

quoted

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...

We have two possible ways of calling panel operations, either directly (panel-

quoted

bus->ops->enable(...)) or indirectly (panel_enable(...)).

The former is what V4L2 currently does with subdevs, and requires display 
drivers to hold a reference to the panel. The later can do without a direct 
reference only if we use a global lock, which is something I would like to 

Wouldn't panel_enable() just do the same panel->bus->ops->enable()
anyway, and both require a panel reference? I don't see the difference.

avoid. A panel-wide lock wouldn't work, as the access function would need to 
take the lock on a panel instance that can be removed at any time.

Can't this be handled with some kind of get/put refcounting? If there's
a ref, it can't be removed.

Generally about locks, if we define that panel ops may only be called
exclusively, does it simplify things? I think we can make such
requirements, as there should be only one display framework that handles
the panel. Then we don't need locking for things like enable/disable.

Of course we need to be careful about things where calls come from
"outside" the display framework. I guess one such thing is rmmod, but if
that causes a notification to the display framework, which again handles
locking, it shouldn't be a problem.

Another thing to be careful about is if the panel internally uses irqs,
workqueues, sysfs files or such. In that case it needs to handle
locking.

 Tomi