[RFC v2 0/5] Common Display Framework

[Tomi Valkeinen]

Hi,

On 2012-11-22 23:45, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
> 
> Hi everybody,
> 
> Here's the second RFC of what was previously known as the Generic Panel
> Framework.

Nice work! Thanks for working on this.

I was doing some testing with the code, seeing how to use it in omapdss.
Here are some thoughts:

In your model the DSS gets the panel devices connected to it from
platform data. After the DSS and the panel drivers are loaded, DSS gets
a notification and connects DSS and the panel.

I think it's a bit limited way. First of all, it'll make the DT data a
bit more complex (although this is not a major problem). With your
model, you'll need something like:

soc-base.dtsi:

dss {
	dpi0: dpi {
	};
};

board.dts:

&dpi0 {
	panel = &dpi-panel;
};

/ {
	dpi-panel: dpi-panel {
		...panel data...;
	};
};

Second, it'll prevent hotplug, and even if real hotplug would not be
supported, it'll prevent cases where the connected panel must be found
dynamically (like reading ID from eeprom).

Third, it kinda creates a cyclical dependency: the DSS needs to know
about the panel and calls ops in the panel, and the panel calls ops in
the DSS. I'm not sure if this is an actual problem, but I usually find
it simpler if calls are done only in one direction.


What I suggest is take a simpler approach, something alike to how
regulators or gpios are used, even if slightly more complex than those:
the entity that has a video output (SoC's DSS, external chips) offers
that video output as resource. It doesn't know or care who uses it. The
user of the video output (panel, external chips) will find the video
output (to which it is connected in the HW) by some means, and will use
different operations on that output to operate the device.

This would give us something like the following DT data:

soc-base.dtsi:

dss {
	dpi0: dpi {
	};
};

board.dts:

/ {
	dpi-panel: dpi-panel {
		source = <&dpi0>;
		...panel data...;
	};
};

The panel driver would do something like this in its probe:

int dpi_panel_probe()
{
	// Find the video source, increase ref
	src = get_video_source_from_of("source");

	// Reserve the video source for us. others can still get and
	// observe it, but cannot use it as video data source.
	// I think this should cascade upstream, so that after this call
	// each video entity from the panel to the SoC's CRTC is
	// reserved and locked for this video pipeline.
	reserve_video_source(src);

	// set DPI HW configuration, like DPI data lines. The
	// configuration would come from panel's platform data
	set_dpi_config(src, config);

	// register this panel as a display.
	register_display(this);
}


The DSS's dpi driver would do something like:

int dss_dpi_probe()
{
	// register as a DPI video source
	register_video_source(this);
}

A DSI-2-DPI chip would do something like:

int dsi2dpi_probe()
{
	// get, reserve and config the DSI bus from SoC
	src = get_video_source_from_of("source");
	reserve_video_source(src);
	set_dsi_config(src, config);

	// register as a DPI video source
	register_video_source(this);
}


Here we wouldn't have similar display_entity as you have, but video
sources and displays. Video sources are elements in the video pipeline,
and a video source is used only by the next downstream element. The last
element in the pipeline would not be a video source, but a display,
which would be used by the upper layer.

Video source's ops would deal with things related to the video bus in
question, like configuring data lanes, sending DSI packets, etc. The
display ops would be more high level things, like enable, update, etc.
Actually, I guess you could consider the display to represent and deal
with the whole pipeline, while video source deals with the bus between
two display entities.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20121123/fd2895cc/attachment.pgp>