Re: [RFC 0/5] Generic panel framework

2012-09-19 Thread Tomi Valkeinen
Hi,

On Fri, 2012-08-17 at 02:49 +0200, Laurent Pinchart wrote:
> Hi everybody,
> 
> While working on DT bindings for the Renesas Mobile SoC display controller
> (a.k.a. LCDC) I quickly realized that display panel implementation based on
> board code callbacks would need to be replaced by a driver-based panel
> framework.

I thought I'd try to approach the common panel framework by creating
device tree examples that OMAP would need. I only thought about the
connections and organization, not individual properties, so I have not
filled any "compatible" or such properties.

What I have below is first DT data for OMAP SoC (more or less what omap4
has), then display examples of different board setups. The hardware
examples I have here are all real, so no imaginary use cases =).

We don't need to implement support for all these at once, but I think
the DT data structure should be right from the start, so I'm posting
this to get discussion about the format.


OMAP SoC


So here's first the SoC specific display nodes. OMAP has a DSS (display
subsystem) block, which contains the following elements:

- DISPC (display controller) reads the pixels from memory and outputs
them using specified video timings. DISPC has three outputs, LCD0, LCD1
and TV. These are SoC internal outputs, they do not go outside the SoC.

- DPI gets its data from DISPC's LCD0, and outputs MIPI DPI (parallel
RBG)

- Two independent DSI modules, which get their data from LCD0 or LCD1,
and output MIPI DSI (a serial two-way video bus)

- HDMI, gets data from DISPC's TV output and outputs HDMI

/ {
ocp {
dss {
dispc {
dss-lcd0: output@0 {
};

dss-lcd1: output@1 {
};

dss-tv: output@2 {
};
};

dpi: dpi {
video-source = <&dss-lcd0>;
};

dsi0: dsi@0 {
video-source = <&dss-lcd0>;
};

dsi1: dsi@1 {
video-source = <&dss-lcd1>;
};

hdmi: hdmi {
video-source = <&dss-tv>;
};
};
};
};

I have defined all the relevant nodes, and video-source property is used
to point to the source for video data. I also define aliases for the SoC
outputs so that panels can use them.

One thing to note is that the video sources for some of the blocks, like
DSI, are not hardcoded in the HW, so dsi0 could get its data from LCD0
or LCD1. However, I don't think they are usually changed during runtime,
and the dss driver cannot change them independently for sure (meaning
that some upper layer should tell it how to change the config). Thus I
specify sane defaults here, but the board dts files can of course
override the video sources.

Another thing to note is that we have more outputs from OMAP than we
have outputs from DISPC. This means that the same video source is used
by multiple sinks (LCD0 used by DPI and DSI0). DPI and DSI0 cannot be
used at the same time, obviously.

And third thing to note, DISPC node defines outputs explicitly, as it
has multiple outputs, whereas the external outputs do not as they have
only one output. Thus the node's output is implicitly the node itself.
So, instead of having:

ds0: dsi@0 {
video-source = <&dss-lcd0>;
dsi0-out0: output@0 {
};
};

I have:

dsi0: dsi@0 {
video-source = <&dss-lcd0>;
};

Of this I'm a bit unsure. I believe in most cases there's only one
output, so it'd be nice to have a shorter representation, but I'm not
sure if it's good to handle the cases for single and multiple outputs
differently. Or if it's good to mix control and data busses, as, for
example, dsi0 can be used as both control and data bus. Having the
output defined explicitly would separate the control and data bus nodes.


Simple DPI panel


Here a board has a DPI panel, which is controlled via i2c. Panel nodes
are children of the control bus, so in this case we define the panel
under i2c2.

&i2c2 {
dpi-lcd-panel {
video-source = <&dpi>;

};
};


HDMI


OMAP has a HDMI output, but it cannot be connected directly to an HDMI
cable. TI uses tpd12s015 chip in its board, which provides ESD,
level-shifting and whatnot (I'm an SW guy, google for the chip to read
the details =). tpd12s015 has a few GPIOs and powers that need to be
controlled, so we need a driver for it.

There's no control bus for the tpd12s015, so it's platform device. Then
there's the device for the HDMI monitor, and the DDC lines are connected
to OMAP's i2c4, thus the hdmi monitor device is a child of i2c.

/ {
   

Re: [RFC 0/5] Generic panel framework

2012-10-20 Thread Inki Dae
Hi Laurent. sorry for being late.

2012/8/17 Laurent Pinchart :
> Hi everybody,
>
> While working on DT bindings for the Renesas Mobile SoC display controller
> (a.k.a. LCDC) I quickly realized that display panel implementation based on
> board code callbacks would need to be replaced by a driver-based panel
> framework.
>
> Several driver-based panel support solution already exist in the kernel.
>
> - The LCD device class is implemented in drivers/video/backlight/lcd.c and
>   exposes a kernel API in include/linux/lcd.h. That API is tied to the FBDEV
>   API for historical reason, uses board code callback for reset and power
>   management, and doesn't include support for standard features available in
>   today's "smart panels".
>
> - OMAP2+ based systems use custom panel drivers available in
>   drivers/video/omap2/displays. Those drivers are based on OMAP DSS (display
>   controller) specific APIs.
>
> - Similarly, Exynos based systems use custom panel drivers available in
>   drivers/video/exynos. Only a single driver (s6e8ax0) is currently available.
>   That driver is based on Exynos display controller specific APIs and on the
>   LCD device class API.
>
> I've brought up the issue with Tomi Valkeinen (OMAP DSS maintainer) and Marcus
> Lorentzon (working on panel support for ST/Linaro), and we agreed that a
> generic panel framework for display devices is needed. These patches implement
> a first proof of concept.
>
> One of the main reasons for creating a new panel framework instead of adding
> missing features to the LCD framework is to avoid being tied to the FBDEV
> framework. Panels will be used by DRM drivers as well, and their API should
> thus be subsystem-agnostic. Note that the panel framework used the
> fb_videomode structure in its API, this will be replaced by a common video
> mode structure shared across subsystems (there's only so many hours per day).
>
> Panels, as used in these patches, are defined as physical devices combining a
> matrix of pixels and a controller capable of driving that matrix.
>
> Panel physical devices are registered as children of the control bus the panel
> controller is connected to (depending on the panel type, we can find platform
> devices for dummy panels with no control bus, or I2C, SPI, DBI, DSI, ...
> devices). The generic panel framework matches registered panel devices with
> panel drivers and call the panel drivers probe method, as done by other device
> classes in the kernel. The driver probe() method is responsible for
> instantiating a struct panel instance and registering it with the generic
> panel framework.
>
> Display drivers are panel consumers. They register a panel notifier with the
> framework, which then calls the notifier when a matching panel is registered.
> The reason for this asynchronous mode of operation, compared to how drivers
> acquire regulator or clock resources, is that the panel can use resources
> provided by the display driver. For instance a panel can be a child of the DBI
> or DSI bus controlled by the display device, or use a clock provided by that
> device. We can't defer the display device probe until the panel is registered
> and also defer the panel device probe until the display is registered. As
> most display drivers need to handle output devices hotplug (HDMI monitors for
> instance), handling panel through a notification system seemed to be the
> easiest solution.
>
> Note that this brings a different issue after registration, as display and
> panel drivers would take a reference to each other. Those circular references
> would make driver unloading impossible. I haven't found a good solution for
> that problem yet (hence the RFC state of those patches), and I would
> appreciate your input here. This might also be a hint that the framework
> design is wrong to start with. I guess I can't get everything right on the
> first try ;-)
>
> Getting hold of the panel is the most complex part. Once done, display drivers
> can call abstract operations provided by panel drivers to control the panel
> operation. These patches implement three of those operations (enable, start
> transfer and get modes). More operations will be needed, and those three
> operations will likely get modified during review. Most of the panels on
> devices I own are dumb panels with no control bus, and are thus not the best
> candidates to design a framework that needs to take complex panels' needs into
> account.
>
> In addition to the generic panel core, I've implemented MIPI DBI (Display Bus
> Interface, a parallel bus for panels that supports read/write transfers of
> commands and data) bus support, as well as three panel drivers (dummy panels
> with no control bus, and Renesas R61505- and R61517-based panels, both using
> DBI as their control bus). Only the dummy panel driver has been tested as I
> lack hardware for the two other drivers.
>
> I will appreciate all reviews, comments, criticisms, ideas, remarks, ... If
> you can find a clev

Re: [RFC 0/5] Generic panel framework

2012-10-20 Thread Inki Dae
Hi Tomi,

2012/8/17 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

The smart panel means command mode way(same as cpu mode)? This panel
includes framebuffer internally and needs triggering from Display
controller to update a new frame on that internal framebuffer. I think
we also need this trigger interface.

Thanks,
Inki Dae


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

Re: [RFC 0/5] Generic panel framework

2012-10-20 Thread Inki Dae
correct some typo. Sorry for this.

2012/10/20 Inki Dae :
> Hi Laurent. sorry for being late.
>
> 2012/8/17 Laurent Pinchart :
>> Hi everybody,
>>
>> While working on DT bindings for the Renesas Mobile SoC display controller
>> (a.k.a. LCDC) I quickly realized that display panel implementation based on
>> board code callbacks would need to be replaced by a driver-based panel
>> framework.
>>
>> Several driver-based panel support solution already exist in the kernel.
>>
>> - The LCD device class is implemented in drivers/video/backlight/lcd.c and
>>   exposes a kernel API in include/linux/lcd.h. That API is tied to the FBDEV
>>   API for historical reason, uses board code callback for reset and power
>>   management, and doesn't include support for standard features available in
>>   today's "smart panels".
>>
>> - OMAP2+ based systems use custom panel drivers available in
>>   drivers/video/omap2/displays. Those drivers are based on OMAP DSS (display
>>   controller) specific APIs.
>>
>> - Similarly, Exynos based systems use custom panel drivers available in
>>   drivers/video/exynos. Only a single driver (s6e8ax0) is currently 
>> available.
>>   That driver is based on Exynos display controller specific APIs and on the
>>   LCD device class API.
>>
>> I've brought up the issue with Tomi Valkeinen (OMAP DSS maintainer) and 
>> Marcus
>> Lorentzon (working on panel support for ST/Linaro), and we agreed that a
>> generic panel framework for display devices is needed. These patches 
>> implement
>> a first proof of concept.
>>
>> One of the main reasons for creating a new panel framework instead of adding
>> missing features to the LCD framework is to avoid being tied to the FBDEV
>> framework. Panels will be used by DRM drivers as well, and their API should
>> thus be subsystem-agnostic. Note that the panel framework used the
>> fb_videomode structure in its API, this will be replaced by a common video
>> mode structure shared across subsystems (there's only so many hours per day).
>>
>> Panels, as used in these patches, are defined as physical devices combining a
>> matrix of pixels and a controller capable of driving that matrix.
>>
>> Panel physical devices are registered as children of the control bus the 
>> panel
>> controller is connected to (depending on the panel type, we can find platform
>> devices for dummy panels with no control bus, or I2C, SPI, DBI, DSI, ...
>> devices). The generic panel framework matches registered panel devices with
>> panel drivers and call the panel drivers probe method, as done by other 
>> device
>> classes in the kernel. The driver probe() method is responsible for
>> instantiating a struct panel instance and registering it with the generic
>> panel framework.
>>
>> Display drivers are panel consumers. They register a panel notifier with the
>> framework, which then calls the notifier when a matching panel is registered.
>> The reason for this asynchronous mode of operation, compared to how drivers
>> acquire regulator or clock resources, is that the panel can use resources
>> provided by the display driver. For instance a panel can be a child of the 
>> DBI
>> or DSI bus controlled by the display device, or use a clock provided by that
>> device. We can't defer the display device probe until the panel is registered
>> and also defer the panel device probe until the display is registered. As
>> most display drivers need to handle output devices hotplug (HDMI monitors for
>> instance), handling panel through a notification system seemed to be the
>> easiest solution.
>>
>> Note that this brings a different issue after registration, as display and
>> panel drivers would take a reference to each other. Those circular references
>> would make driver unloading impossible. I haven't found a good solution for
>> that problem yet (hence the RFC state of those patches), and I would
>> appreciate your input here. This might also be a hint that the framework
>> design is wrong to start with. I guess I can't get everything right on the
>> first try ;-)
>>
>> Getting hold of the panel is the most complex part. Once done, display 
>> drivers
>> can call abstract operations provided by panel drivers to control the panel
>> operation. These patches implement three of those operations (enable, start
>> transfer and get modes). More operations will be needed, and those three
>> operations will likely get modified during review. Most of the panels on
>> devices I own are dumb panels with no control bus, and are thus not the best
>> candidates to design a framework that needs to take complex panels' needs 
>> into
>> account.
>>
>> In addition to the generic panel core, I've implemented MIPI DBI (Display Bus
>> Interface, a parallel bus for panels that supports read/write transfers of
>> commands and data) bus support, as well as three panel drivers (dummy panels
>> with no control bus, and Renesas R61505- and R61517-based panels, both using
>> DBI as their control bus). Only the dummy p

Re: [RFC 0/5] Generic panel framework

2012-10-30 Thread Laurent Pinchart
Hi Jun,

I've finally been able to resume my work on the panel framework (I hope to 
post a v2 at the end of the week).

On Thursday 23 August 2012 14:23:01 Jun Nie wrote:
> Hi Laurent,
> Do you plan to add an API to get and parse EDID to mode list?

An API to get the raw EDID data is likely needed. Parsing EDID data in the 
panel driver and providing the modes to the caller isn't enough, as EDID 
contains more than just video modes. I'm not sure whether a driver for an 
EDID-aware panel should parse the EDID data internally and provide both modes 
and raw EDID data, or only raw EDID data.

> video mode is tightly coupled with panel that is capable of hot-plug.
> Or you are busy on modifying EDID parsing code for sharing it amoung
> DRM/FB/etc? I see you mentioned this in Mar.

That's needed as well, but -ENOTIME :-S

> It is great if you are considering add more info into video mode, such as
> pixel repeating, 3D timing related parameter.

Please have a look at "[PATCH 2/2 v6] of: add generic videomode description" 
on dri-devel. There's a proposal for a common video mode structure.

> I have some code for CEA modes filtering and 3D parsing, but still tight
> coupled with FB and with a little hack style.
> 
> My HDMI driver is implemented as lcd device as you mentioned here.
> But more complex than other lcd devices for a kthread is handling
> hot-plug/EDID/HDCP/ASoC etc.
> 
> I also feel a little weird to add code parsing HDMI audio related
> info in fbmod.c in my current implementation, thought it is the only
> place to handle EDID in kernel. Your panel framework provide a better
> place to add panel related audio/HDCP code. panel notifier can also
> trigger hot-plug related feature, such as HDCP start.

That's a good idea. I was wondering whether to put the common EDID parser in 
drivers/gpu/drm, drivers/video or drivers/media. Putting it wherever the panel 
framework will be might be a good option as well.

> Looking forward to your hot-plug panel patch. Or I can help add it
> if you would like me to.

I'll try to post a v2 at the end of the week, but likely without much hot-plug 
support. Patches and enhancement proposals will be welcome.

-- 
Regards,

Laurent Pinchart

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


Re: [RFC 0/5] Generic panel framework

2012-10-30 Thread Laurent Pinchart
Hi Zhou,

On Tuesday 04 September 2012 16:20:38 Zhou Zhu wrote:
> Hi Laurent,
> 
> Basically I agree that we need a common panel framework. I just have
> some questions:
> 1.  I think we should add color format in videomode - if we use such
> common video mode structure shared across subsystems.
> In HDMI, colors are bind with timings tightly. We need a combined
> videomode with timing and color format together.

What kind of color formats do you have in mind ?

> 2. I think we should add "set_videomode" interface. It helps HDMI
> monitors to set EDIDs.

For panels that support several video modes, sure, we need a way to set the 
video mode. I don't have access to any such panel though, that's why the 
operation has been left out. It wouldn't be difficult to add it when a real 
use case will come up.

What do you mean exactly about HDMI monitors setting EDID ?

-- 
Regards,

Laurent Pinchart

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


Re: [RFC 0/5] Generic panel framework

2012-10-31 Thread Laurent Pinchart
Hi Tomi,

On Wednesday 19 September 2012 14:45:29 Tomi Valkeinen wrote:
> On Fri, 2012-08-17 at 02:49 +0200, Laurent Pinchart wrote:
> > Hi everybody,
> > 
> > While working on DT bindings for the Renesas Mobile SoC display controller
> > (a.k.a. LCDC) I quickly realized that display panel implementation based
> > on board code callbacks would need to be replaced by a driver-based panel
> > framework.
> 
> I thought I'd try to approach the common panel framework by creating
> device tree examples that OMAP would need. I only thought about the
> connections and organization, not individual properties, so I have not
> filled any "compatible" or such properties.
> 
> What I have below is first DT data for OMAP SoC (more or less what omap4
> has), then display examples of different board setups. The hardware
> examples I have here are all real, so no imaginary use cases =).
> 
> We don't need to implement support for all these at once, but I think
> the DT data structure should be right from the start, so I'm posting
> this to get discussion about the format.

Thank you for working on this proposal.

> OMAP SoC
> 
> 
> So here's first the SoC specific display nodes. OMAP has a DSS (display
> subsystem) block, which contains the following elements:
> 
> - DISPC (display controller) reads the pixels from memory and outputs
> them using specified video timings. DISPC has three outputs, LCD0, LCD1
> and TV. These are SoC internal outputs, they do not go outside the SoC.
> 
> - DPI gets its data from DISPC's LCD0, and outputs MIPI DPI (parallel
> RBG)
> 
> - Two independent DSI modules, which get their data from LCD0 or LCD1,
> and output MIPI DSI (a serial two-way video bus)
> 
> - HDMI, gets data from DISPC's TV output and outputs HDMI
> 
> / {
>   ocp {
>   dss {
>   dispc {
>   dss-lcd0: output@0 {
>   };
> 
>   dss-lcd1: output@1 {
>   };
> 
>   dss-tv: output@2 {
>   };
>   };
> 
>   dpi: dpi {
>   video-source = <&dss-lcd0>;
>   };
> 
>   dsi0: dsi@0 {
>   video-source = <&dss-lcd0>;
>   };
> 
>   dsi1: dsi@1 {
>   video-source = <&dss-lcd1>;
>   };
> 
>   hdmi: hdmi {
>   video-source = <&dss-tv>;
>   };
>   };
>   };
> };
> 
> I have defined all the relevant nodes, and video-source property is used
> to point to the source for video data. I also define aliases for the SoC
> outputs so that panels can use them.
> 
> One thing to note is that the video sources for some of the blocks, like
> DSI, are not hardcoded in the HW, so dsi0 could get its data from LCD0
> or LCD1.

What about the source that are hardwired in hardware ? Shouldn't those be 
hardcoded in the driver instead ?

> However, I don't think they are usually changed during runtime, and the dss
> driver cannot change them independently for sure (meaning that some upper
> layer should tell it how to change the config). Thus I specify sane defaults
> here, but the board dts files can of course override the video sources.

I'm not sure whether default settings like those really belong to the DT. I'm 
no expert on that topic though.

> Another thing to note is that we have more outputs from OMAP than we have
> outputs from DISPC. This means that the same video source is used by
> multiple sinks (LCD0 used by DPI and DSI0). DPI and DSI0 cannot be used at
> the same time, obviously.

It might not be really obvious, as I don't see what prevents DPI and DSI0 to 
be used at the same time :-) Do they share physical pins ?

> And third thing to note, DISPC node defines outputs explicitly, as it has
> multiple outputs, whereas the external outputs do not as they have only one
> output. Thus the node's output is implicitly the node itself. So, instead of
> having:
> 
> ds0: dsi@0 {
>   video-source = <&dss-lcd0>;
>   dsi0-out0: output@0 {
>   };
> };
> 
> I have:
> 
> dsi0: dsi@0 {
>   video-source = <&dss-lcd0>;
> };

What about defining the data sinks instead of the data sources ? I find it 
more logical for the DSS to get the panel it's connected to than the other way 
around.

> Of this I'm a bit unsure. I believe in most cases there's only one output,
> so it'd be nice to have a shorter representation, but I'm not sure if it's
> good to handle the cases for single and multiple outputs differently. Or if
> it's good to mix control and data busses, as, for example, dsi0 can be used
> as both control and data bus. Having the output defined explicitly would
> separate the control and data bus nodes.
> 
> 
> Simple DPI panel
> ==

Re: [RFC 0/5] Generic panel framework

2012-10-31 Thread Laurent Pinchart
Hi Inki,

On Saturday 20 October 2012 22:10:17 Inki Dae wrote:
> Hi Laurent. sorry for being late.

No worries.

> 2012/8/17 Laurent Pinchart :
> > Hi everybody,
> > 
> > While working on DT bindings for the Renesas Mobile SoC display controller
> > (a.k.a. LCDC) I quickly realized that display panel implementation based
> > on board code callbacks would need to be replaced by a driver-based panel
> > framework.
> > 
> > Several driver-based panel support solution already exist in the kernel.
> > 
> > - The LCD device class is implemented in drivers/video/backlight/lcd.c and
> >   exposes a kernel API in include/linux/lcd.h. That API is tied to the
> >   FBDEV API for historical reason, uses board code callback for reset and
> >   power management, and doesn't include support for standard features
> >   available in today's "smart panels".
> > 
> > - OMAP2+ based systems use custom panel drivers available in
> > 
> >   drivers/video/omap2/displays. Those drivers are based on OMAP DSS
> >   (display controller) specific APIs.
> > 
> > - Similarly, Exynos based systems use custom panel drivers available in
> > 
> >   drivers/video/exynos. Only a single driver (s6e8ax0) is currently
> >   available. That driver is based on Exynos display controller specific
> >   APIs and on the LCD device class API.
> > 
> > I've brought up the issue with Tomi Valkeinen (OMAP DSS maintainer) and
> > Marcus Lorentzon (working on panel support for ST/Linaro), and we agreed
> > that a generic panel framework for display devices is needed. These
> > patches implement a first proof of concept.
> > 
> > One of the main reasons for creating a new panel framework instead of
> > adding missing features to the LCD framework is to avoid being tied to
> > the FBDEV framework. Panels will be used by DRM drivers as well, and
> > their API should thus be subsystem-agnostic. Note that the panel
> > framework used the fb_videomode structure in its API, this will be
> > replaced by a common video mode structure shared across subsystems
> > (there's only so many hours per day).
> > 
> > Panels, as used in these patches, are defined as physical devices
> > combining a matrix of pixels and a controller capable of driving that
> > matrix.
> > 
> > Panel physical devices are registered as children of the control bus the
> > panel controller is connected to (depending on the panel type, we can
> > find platform devices for dummy panels with no control bus, or I2C, SPI,
> > DBI, DSI, ... devices). The generic panel framework matches registered
> > panel devices with panel drivers and call the panel drivers probe method,
> > as done by other device classes in the kernel. The driver probe() method
> > is responsible for instantiating a struct panel instance and registering
> > it with the generic panel framework.
> > 
> > Display drivers are panel consumers. They register a panel notifier with
> > the framework, which then calls the notifier when a matching panel is
> > registered. The reason for this asynchronous mode of operation, compared
> > to how drivers acquire regulator or clock resources, is that the panel
> > can use resources provided by the display driver. For instance a panel
> > can be a child of the DBI or DSI bus controlled by the display device, or
> > use a clock provided by that device. We can't defer the display device
> > probe until the panel is registered and also defer the panel device probe
> > until the display is registered. As most display drivers need to handle
> > output devices hotplug (HDMI monitors for instance), handling panel
> > through a notification system seemed to be the easiest solution.
> > 
> > Note that this brings a different issue after registration, as display and
> > panel drivers would take a reference to each other. Those circular
> > references would make driver unloading impossible. I haven't found a good
> > solution for that problem yet (hence the RFC state of those patches), and
> > I would appreciate your input here. This might also be a hint that the
> > framework design is wrong to start with. I guess I can't get everything
> > right on the first try ;-)
> > 
> > Getting hold of the panel is the most complex part. Once done, display
> > drivers can call abstract operations provided by panel drivers to control
> > the panel operation. These patches implement three of those operations
> > (enable, start transfer and get modes). More operations will be needed,
> > and those three operations will likely get modified during review. Most
> > of the panels on devices I own are dumb panels with no control bus, and
> > are thus not the best candidates to design a framework that needs to take
> > complex panels' needs into account.
> > 
> > In addition to the generic panel core, I've implemented MIPI DBI (Display
> > Bus Interface, a parallel bus for panels that supports read/write
> > transfers of commands and data) bus support, as well as three panel
> > drivers (dummy panels with no control bus, and R

Re: [RFC 0/5] Generic panel framework

2012-10-31 Thread Tomi Valkeinen
On 2012-10-31 15:13, Laurent Pinchart wrote:

>> OMAP SoC
>> 
>>
>> So here's first the SoC specific display nodes. OMAP has a DSS (display
>> subsystem) block, which contains the following elements:
>>
>> - DISPC (display controller) reads the pixels from memory and outputs
>> them using specified video timings. DISPC has three outputs, LCD0, LCD1
>> and TV. These are SoC internal outputs, they do not go outside the SoC.
>>
>> - DPI gets its data from DISPC's LCD0, and outputs MIPI DPI (parallel
>> RBG)
>>
>> - Two independent DSI modules, which get their data from LCD0 or LCD1,
>> and output MIPI DSI (a serial two-way video bus)
>>
>> - HDMI, gets data from DISPC's TV output and outputs HDMI
>>
>> / {
>>  ocp {
>>  dss {
>>  dispc {
>>  dss-lcd0: output@0 {
>>  };
>>
>>  dss-lcd1: output@1 {
>>  };
>>
>>  dss-tv: output@2 {
>>  };
>>  };
>>
>>  dpi: dpi {
>>  video-source = <&dss-lcd0>;
>>  };
>>
>>  dsi0: dsi@0 {
>>  video-source = <&dss-lcd0>;
>>  };
>>
>>  dsi1: dsi@1 {
>>  video-source = <&dss-lcd1>;
>>  };
>>
>>  hdmi: hdmi {
>>  video-source = <&dss-tv>;
>>  };
>>  };
>>  };
>> };
>>
>> I have defined all the relevant nodes, and video-source property is used
>> to point to the source for video data. I also define aliases for the SoC
>> outputs so that panels can use them.
>>
>> One thing to note is that the video sources for some of the blocks, like
>> DSI, are not hardcoded in the HW, so dsi0 could get its data from LCD0
>> or LCD1.
> 
> What about the source that are hardwired in hardware ? Shouldn't those be 
> hardcoded in the driver instead ?

Even if both the DSI and the DISPC are parts of OMAP DSS, and part of
the SoC, they are separate IPs. We should look at them the same way we'd
consider chips that are outside the SoC.

So things that are internal to a device can (and I think should) be
hardcoded in the driver, but integration details, the connections
between the IPs, etc, should be described in the DT data.

Then again, we do have (and need) a driver for the "dss" node in the
above DT data. This dss represents dss_core, a "glue" IP that contains
the rest of the DSS blocks and muxes and such. It could be argued that
this dss_core driver does indeed know the integration details, and thus
there's no need to represent them in the DT data.

However, I do think that we should represent the DISPC outputs with
generic display entities inside CPF, just like DSI and the panels. And
we do need to set the connections between these entities. So the
question is, do we get those connections from the DT data, or are they
hardcoded in the dss_core driver.

I don't currently have strong opinions on either direction. Both make
sense to me. But I think this is SoC driver implementation specific, and
the common panel framework doesn't need to force this to either
direction. Both should be possible from CPF's point of view.

>> However, I don't think they are usually changed during runtime, and the dss
>> driver cannot change them independently for sure (meaning that some upper
>> layer should tell it how to change the config). Thus I specify sane defaults
>> here, but the board dts files can of course override the video sources.
> 
> I'm not sure whether default settings like those really belong to the DT. I'm 
> no expert on that topic though.

I agree. But I also don't have a good solution how the driver would find
good choices for these settings...

>> Another thing to note is that we have more outputs from OMAP than we have
>> outputs from DISPC. This means that the same video source is used by
>> multiple sinks (LCD0 used by DPI and DSI0). DPI and DSI0 cannot be used at
>> the same time, obviously.
> 
> It might not be really obvious, as I don't see what prevents DPI and DSI0 to 
> be used at the same time :-) Do they share physical pins ?

I think they do, but even if they didn't, there's just one source for
two outputs. So if the SoC design is such that the only video source for
DPI is LCD0, and the only video source for DSI0 is LCD0, and presuming
you can't send the video data to both destinations, then only one of DPI
and DSI0 can be enabled at the same time.

Even if the LCD0 could send the pixel stream to both DPI and DSI0, it'd
be "interesting", because the original video timings and pixel clock are
programmed in the LCD0 output, and thus both DPI and DSI0 get the same
timings. If DPI would want to use some other mode, DSI would most likely
go nuts.

So my opinion is that we should only

Re: [RFC 0/5] Generic panel framework

2012-08-16 Thread Jingoo Han
On Friday, August 17, 2012 9:50 AM Laurent Pinchart wrote:
> 
> Hi everybody,
> 
> While working on DT bindings for the Renesas Mobile SoC display controller
> (a.k.a. LCDC) I quickly realized that display panel implementation based on
> board code callbacks would need to be replaced by a driver-based panel
> framework.
> 
> Several driver-based panel support solution already exist in the kernel.
> 
> - The LCD device class is implemented in drivers/video/backlight/lcd.c and
>   exposes a kernel API in include/linux/lcd.h. That API is tied to the FBDEV
>   API for historical reason, uses board code callback for reset and power
>   management, and doesn't include support for standard features available in
>   today's "smart panels".
> 
> - OMAP2+ based systems use custom panel drivers available in
>   drivers/video/omap2/displays. Those drivers are based on OMAP DSS (display
>   controller) specific APIs.
> 
> - Similarly, Exynos based systems use custom panel drivers available in
>   drivers/video/exynos. Only a single driver (s6e8ax0) is currently available.
>   That driver is based on Exynos display controller specific APIs and on the
>   LCD device class API.
> 

Hi Laurent,

I am a Exynos DP maintainer and Samsung FB maintainer.

Actually, eDP (embedded display port) will be faced with this kind of problem.
According to the eDP standard, eDP panel can have their own specific registers
that handle extra operations. In this case, custom panel driver for this eDP 
panel
will be necessary.

In my opinion, the panel framework would be helpful to solve this problem.


Best regards,
Jingoo Han


> I've brought up the issue with Tomi Valkeinen (OMAP DSS maintainer) and Marcus
> Lorentzon (working on panel support for ST/Linaro), and we agreed that a
> generic panel framework for display devices is needed. These patches implement
> a first proof of concept.
> 
> One of the main reasons for creating a new panel framework instead of adding
> missing features to the LCD framework is to avoid being tied to the FBDEV
> framework. Panels will be used by DRM drivers as well, and their API should
> thus be subsystem-agnostic. Note that the panel framework used the
> fb_videomode structure in its API, this will be replaced by a common video
> mode structure shared across subsystems (there's only so many hours per day).
> 
> Panels, as used in these patches, are defined as physical devices combining a
> matrix of pixels and a controller capable of driving that matrix.
> 
> Panel physical devices are registered as children of the control bus the panel
> controller is connected to (depending on the panel type, we can find platform
> devices for dummy panels with no control bus, or I2C, SPI, DBI, DSI, ...
> devices). The generic panel framework matches registered panel devices with
> panel drivers and call the panel drivers probe method, as done by other device
> classes in the kernel. The driver probe() method is responsible for
> instantiating a struct panel instance and registering it with the generic
> panel framework.
> 
> Display drivers are panel consumers. They register a panel notifier with the
> framework, which then calls the notifier when a matching panel is registered.
> The reason for this asynchronous mode of operation, compared to how drivers
> acquire regulator or clock resources, is that the panel can use resources
> provided by the display driver. For instance a panel can be a child of the DBI
> or DSI bus controlled by the display device, or use a clock provided by that
> device. We can't defer the display device probe until the panel is registered
> and also defer the panel device probe until the display is registered. As
> most display drivers need to handle output devices hotplug (HDMI monitors for
> instance), handling panel through a notification system seemed to be the
> easiest solution.
> 
> Note that this brings a different issue after registration, as display and
> panel drivers would take a reference to each other. Those circular references
> would make driver unloading impossible. I haven't found a good solution for
> that problem yet (hence the RFC state of those patches), and I would
> appreciate your input here. This might also be a hint that the framework
> design is wrong to start with. I guess I can't get everything right on the
> first try ;-)
> 
> Getting hold of the panel is the most complex part. Once done, display drivers
> can call abstract operations provided by panel drivers to control the panel
> operation. These patches implement three of those operations (enable, start
> transfer and get modes). More operations will be needed, and those three
> operations will likely get modified during review. Most of the panels on
> devices I own are dumb panels with no control bus, and are thus not the best
> candidates to design a framework that needs to take complex panels' needs into
> account.
> 
> In addition to the generic panel core, I've implemented MIPI DBI (Display Bus
> Interfac

Re: [RFC 0/5] Generic panel framework

2012-08-17 Thread 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



signature.asc
Description: This is a digitally signed message part


Re: [RFC 0/5] Generic panel framework

2012-08-17 Thread Laurent Pinchart
Hi Tomi,

Thanks a lot for the review.

On Friday 17 August 2012 11:38:14 Tomi Valkeinen wrote:
> 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?

That looks good to me. I'll then rename panel_dbi_* to mipi_dbi_*.

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

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.

Would you also create include/video/panel/ ?

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

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

Of course. If I had all that information already I would have shared it :-) 
This is really a first RFC, my goal is to make sure that I'm going in the 
right direction.

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

Re: [RFC 0/5] Generic panel framework

2012-08-17 Thread Tomi Valkeinen
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.

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

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

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



signature.asc
Description: This is a digitally signed message part


Re: [RFC 0/5] Generic panel framework

2012-08-17 Thread Laurent Pinchart
Hi Tomi,

On Friday 17 August 2012 14:42:31 Tomi Valkeinen wrote:
> 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).

I don't expect the directory to contain many non-panel files, so let's keep it 
as-is for now.

mipi-dbi-bus might not belong to include/video/panel/ though, as it can be 
used for non-panel devices (at least in theory). The future mipi-dsi-bus 
certainly will.

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

Would you be able to send incremental patches on top of v2 to implement the 
solution you have in mind ? It would be neat if you could also implement mipi-
dsi-bus for the OMAP DSS and test the code with a real device :-)

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

Indeed, you're right. I'm not sure what I was thinking about.

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

Trouble will come when the display driver will hold a reference to the panel, 
and the panel will hold a reference to the display driver (for instance 
because the display driver provides the DBI/DSI bus, or because it provides a 
clock used by the panel).

> Generally about locks, if we define that panel ops may only be called
> exclusively, does it simplify things? I t

Re: [RFC 0/5] Generic panel framework

2012-08-20 Thread Tomi Valkeinen
On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> Hi Tomi,

> mipi-dbi-bus might not belong to include/video/panel/ though, as it can be 
> used for non-panel devices (at least in theory). The future mipi-dsi-bus 
> certainly will.

They are both display busses. So while they could be used for anything,
I find it quite unlikely as there are much better alternatives for
generic bus needs.

> Would you be able to send incremental patches on top of v2 to implement the 
> solution you have in mind ? It would be neat if you could also implement mipi-
> dsi-bus for the OMAP DSS and test the code with a real device :-)

Yes, I'd like to try this out on OMAP, both DBI and DSI. However, I fear
it'll be quite complex due to the dependencies all around we have in the
current driver. We're working on simplifying things so that it'll be
easier to try thing like the panel framework, though, so we're going in
the right direction.

> > 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.
> 
> Pushing locking to callers would indeed simplify panel drivers, but we need 
> to 
> make sure we won't need to expose a panel to several callers in the future.

I have a feeling that would be a bad idea.

Display related stuff are quite sensitive to any delays, so any extra
transactions over, say, DSI bus could cause a noticeable glitch on the
screen. I'm not sure what are all the possible ops that a panel can
offer, but I think all that affect the display or could cause delays
should be handled by one controlling entity (drm or such). The
controlling entity needs to handle locking anyway, so in that sense I
don't think it's an extra burden for it.

The things that come to my mind that could possibly cause calls to the
panel outside drm: debugfs, sysfs, audio, backlight. Of those, I think
backlight should go through drm. Audio, no idea. debugfs and sysfs
locking needs to be handled by the panel driver, and they are a bit
problematic as I guess having them requires full locking.

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [RFC 0/5] Generic panel framework

2012-08-20 Thread Laurent Pinchart
Hi Tomi,

On Monday 20 August 2012 14:39:30 Tomi Valkeinen wrote:
> On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > mipi-dbi-bus might not belong to include/video/panel/ though, as it can be
> > used for non-panel devices (at least in theory). The future mipi-dsi-bus
> > certainly will.
> 
> They are both display busses. So while they could be used for anything,
> I find it quite unlikely as there are much better alternatives for
> generic bus needs.

My point is that they could be used for display devices other than panels. 
This is especially true for DSI, as there are DSI to HDMI converters. 
Technically speaking that's also true for DBI, as DBI chips convert from DBI 
to DPI, but we can group both the DBI-to-DPI chip and the panel in a single 
panel object.

> > Would you be able to send incremental patches on top of v2 to implement
> > the solution you have in mind ? It would be neat if you could also
> > implement mipi- dsi-bus for the OMAP DSS and test the code with a real
> > device :-)
> 
> Yes, I'd like to try this out on OMAP, both DBI and DSI. However, I fear
> it'll be quite complex due to the dependencies all around we have in the
> current driver. We're working on simplifying things so that it'll be
> easier to try thing like the panel framework, though, so we're going in
> the right direction.

If you want the panel framework to support your use cases I'm afraid you will 
need to work on that ;-)
 
> > > 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.
> > 
> > Pushing locking to callers would indeed simplify panel drivers, but we
> > need to make sure we won't need to expose a panel to several callers in
> > the future.
>
> I have a feeling that would be a bad idea.
> 
> Display related stuff are quite sensitive to any delays, so any extra
> transactions over, say, DSI bus could cause a noticeable glitch on the
> screen. I'm not sure what are all the possible ops that a panel can
> offer, but I think all that affect the display or could cause delays
> should be handled by one controlling entity (drm or such). The
> controlling entity needs to handle locking anyway, so in that sense I
> don't think it's an extra burden for it.
> 
> The things that come to my mind that could possibly cause calls to the
> panel outside drm: debugfs, sysfs, audio, backlight. Of those, I think
> backlight should go through drm. Audio, no idea. debugfs and sysfs
> locking needs to be handled by the panel driver, and they are a bit
> problematic as I guess having them requires full locking.

-- 
Regards,

Laurent Pinchart

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


Re: [RFC 0/5] Generic panel framework

2012-08-20 Thread Tomi Valkeinen
On Tue, 2012-08-21 at 01:29 +0200, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Monday 20 August 2012 14:39:30 Tomi Valkeinen wrote:
> > On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> > > Hi Tomi,
> > > 
> > > mipi-dbi-bus might not belong to include/video/panel/ though, as it can be
> > > used for non-panel devices (at least in theory). The future mipi-dsi-bus
> > > certainly will.
> > 
> > They are both display busses. So while they could be used for anything,
> > I find it quite unlikely as there are much better alternatives for
> > generic bus needs.
> 
> My point is that they could be used for display devices other than panels. 
> This is especially true for DSI, as there are DSI to HDMI converters. 
> Technically speaking that's also true for DBI, as DBI chips convert from DBI 
> to DPI, but we can group both the DBI-to-DPI chip and the panel in a single 
> panel object.

Ah, ok. I thought "panels" would include these buffer/converter chips.

I think we should have one driver for one indivisible hardware entity.
So if you've got a panel module that contains DBI receiver, buffer
memory and a DPI panel, let's just have one "DBI panel" driver for it.

If we get lots of different panel modules containing the same DBI RX IP,
we could have the DBI IP part as a common library, but still have one
panel driver per panel module.

But how do you see the case for separate converter/buffer chips? Are
they part of the generic panel framework? I guess they kinda have to be.
On one side they use the "panel" API control the bus they are connected
to, and on the other they offer an API for the connected panel to use
the bus they provide.

Did you just mean we should have a separate directory for them, while
still part of the same framework, or...?

 Tomi



signature.asc
Description: This is a digitally signed message part


Re: [RFC 0/5] Generic panel framework

2012-08-21 Thread Laurent Pinchart
Hi Tomi,

On Tuesday 21 August 2012 08:49:57 Tomi Valkeinen wrote:
> On Tue, 2012-08-21 at 01:29 +0200, Laurent Pinchart wrote:
> > On Monday 20 August 2012 14:39:30 Tomi Valkeinen wrote:
> > > On Sat, 2012-08-18 at 03:16 +0200, Laurent Pinchart wrote:
> > > > Hi Tomi,
> > > > 
> > > > mipi-dbi-bus might not belong to include/video/panel/ though, as it
> > > > can be used for non-panel devices (at least in theory). The future
> > > > mipi-dsi-bus certainly will.
> > > 
> > > They are both display busses. So while they could be used for anything,
> > > I find it quite unlikely as there are much better alternatives for
> > > generic bus needs.
> > 
> > My point is that they could be used for display devices other than panels.
> > This is especially true for DSI, as there are DSI to HDMI converters.
> > Technically speaking that's also true for DBI, as DBI chips convert from
> > DBI to DPI, but we can group both the DBI-to-DPI chip and the panel in a
> > single panel object.
> 
> Ah, ok. I thought "panels" would include these buffer/converter chips.
> 
> I think we should have one driver for one indivisible hardware entity.
> So if you've got a panel module that contains DBI receiver, buffer
> memory and a DPI panel, let's just have one "DBI panel" driver for it.
> 
> If we get lots of different panel modules containing the same DBI RX IP,
> we could have the DBI IP part as a common library, but still have one
> panel driver per panel module.

Sounds good to me.

> But how do you see the case for separate converter/buffer chips? Are
> they part of the generic panel framework? I guess they kinda have to be.
> On one side they use the "panel" API control the bus they are connected
> to, and on the other they offer an API for the connected panel to use
> the bus they provide.

The DBI/DSI APIs will not be panel-specific (they won't contain any reference 
to "panel") I'm thus thinking of moving them from drivers/video/panel/ to 
drivers/video/.

Furthermore, a DSI-to-HDMI converter chip is not a panel, but needs to expose 
display-related operations in a way similar to panels. I was thus wondering if 
we shouldn't replace the panel structure with some kind of video entity 
structure that would expose operations similar to panels. We could then extend 
that structure with converter-specific operations later. The framework would 
become a bit more generic, while remaining display-specific.

> Did you just mean we should have a separate directory for them, while
> still part of the same framework, or...?

-- 
Regards,

Laurent Pinchart

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


Re: [RFC 0/5] Generic panel framework

2012-08-22 Thread Jun Nie
Hi Laurent,
Do you plan to add an API to get and parse EDID to mode list?
video mode is tightly coupled with panel that is capable of hot-plug.
Or you are busy on modifying EDID parsing code for sharing it amoung
DRM/FB/etc? I see you mentioned this in Mar. It is great if you are
considering add more info into video mode, such as pixel repeating, 3D
timing related parameter. I have some code for CEA modes filtering and
3D parsing, but still tight coupled with FB and with a little hack
style.

My HDMI driver is implemented as lcd device as you mentioned here.
But more complex than other lcd devices for a kthread is handling
hot-plug/EDID/HDCP/ASoC etc.

I also feel a little weird to add code parsing HDMI audio related
info in fbmod.c in my current implementation, thought it is the only
place to handle EDID in kernel. Your panel framework provide a better
place to add panel related audio/HDCP code. panel notifier can also
trigger hot-plug related feature, such as HDCP start.

Looking forward to your hot-plug panel patch. Or I can help add it
if you would like me to.

Thanks!
Jun
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/5] Generic panel framework

2012-09-04 Thread Zhou Zhu
Hi Laurent,

Basically I agree that we need a common panel framework. I just have
some questions:
1.  I think we should add color format in videomode - if we use such
common video mode structure shared across subsystems.
In HDMI, colors are bind with timings tightly. We need a combined
videomode with timing and color format together.
2. I think we should add "set_videomode" interface. It helps HDMI
monitors to set EDIDs.

-- 
Thanks,
-Zhou
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html