Re: [RFC 0/5] Generic panel framework
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
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
Re: [RFC 0/5] Generic panel framework
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 laurent.pinch...@ideasonboard.com: 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
Re: [RFC 0/5] Generic panel framework
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 allow 1:1 connections between
sources and sinks. If a component has multiple outputs, and even if
those outputs give the exact same data, it should define
Re: [RFC 0/5] Generic panel framework
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
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
Hi Laurent. sorry for being late. 2012/8/17 Laurent Pinchart laurent.pinch...@ideasonboard.com: 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 clever way to solve the cyclic references issue described
Re: [RFC 0/5] Generic panel framework
Hi Tomi, 2012/8/17 Tomi Valkeinen tomi.valkei...@ti.com: 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 notifications and the display driver will stop using the panel. After that nobody has pointers to the panel,
Re: [RFC 0/5] Generic panel framework
correct some typo. Sorry for this. 2012/10/20 Inki Dae inki@samsung.com: Hi Laurent. sorry for being late. 2012/8/17 Laurent Pinchart laurent.pinch...@ideasonboard.com: 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,
Re: [RFC 0/5] Generic panel framework
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.
/ {
hdmi-connector: tpd12s015 {
Re: [RFC 0/5] Generic panel framework
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
Re: [RFC 0/5] Generic panel framework
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
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
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
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
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
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
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(); sleep(); gpip_set(..); I have to admit I have no better solution to propose at the moment, even if I
Re: [RFC 0/5] Generic panel framework
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
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 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
[RFC 0/5] Generic panel framework
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 clever way to solve the cyclic references issue described above I'll buy you a beer at the next conference we will both attend. If you think the proposed solution is too complex, or too simple, I'm all ears. I personally
Re: [RFC 0/5] Generic panel framework
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 Interface, a parallel bus for panels that supports read/write transfers of commands and
