On Sun, Oct 07, 2012 at 03:38:33PM +0200, Laurent Pinchart wrote:

Hi Steffen,

On Friday 05 October 2012 17:51:21 Steffen Trumtrar wrote:

quoted

On Thu, Oct 04, 2012 at 12:51:00PM -0600, Stephen Warren wrote:

quoted

On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:

quoted

Get videomode from devicetree in a format appropriate for the
backend. drm_display_mode and fb_videomode are supported atm.
Uses the display signal timings from of_display_timings

+++ b/drivers/of/of_videomode.c

+int videomode_from_timing(struct display_timings *disp, struct

videomode *vm,

+	st = display_timings_get(disp, index);
+
+	if (!st) {

It's a little odd to leave a blank line between those two lines.

Hm, well okay. That can be remedied

quoted

Only half of the code in this file seems OF-related; the routines to
convert a timing to a videomode or drm display mode seem like they'd be
useful outside device tree, so I wonder if putting them into
of_videomode.c is the correct thing to do. Still, it's probably not a
big deal.

I am not sure, what the appropriate way to do this is. I can split it up
(again).

I think it would make sense to move them to their respective subsystems.

I agree. While looking at integrating this for Tegra DRM, I came across
the issue that if I build DRM as a module, linking with this code will
fail. The reason for that was that it was that the code, itself builtin,
uses drm_mode_set_name(), which would be exported by the drm module. So
I had to modifiy the Kconfig entries to be "def_tristate DRM". That
obviously isn't very nice since the code can also be used without DRM.

Moving the subsystem specific conversion routines to the respective
subsystems should solve any of these issues.

Thierry