On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
[...]

DISPLAY_TIMINGS?

 #video output switch sysfs driver
 obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
+obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o

display_timings.o?

display_timings.c?

+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+			  unsigned int index)

I find the indexing API a bit confusing. But that's perhaps just a
matter of personal preference.

Also the ordering of arguments seems a little off. I find it more
natural to have the destination pointer in the first argument, similar
to the memcpy() function, so this would be:

int videomode_from_timing(struct videomode *vm, struct display_timings *disp,
			  unsigned int index);

Actually, when reading videomode_from_timing() I'd expect the argument
list to be:

int videomode_from_timing(struct videomode *vm, struct display_timing *timing);

Am I the only one confused by this?

display_timings.h?

+/* placeholder function until ranges are really needed 

The above line has trailing whitespace. Also the block comment should
have the opening /* on a separate line.

+ * the index parameter should then be used to select one of [min typ max]

If index is supposed to select min, typ or max, then maybe an enum would
be a better candidate? Or alternatively provide separate accessors, like
display_timing_get_{minimum,typical,maximum}().

+ */
+static inline u32 display_timing_get_value(struct timing_entry *te,
+					   unsigned int index)
+{
+	return te->typ;
+}
+
+static inline struct display_timing *display_timings_get(struct display_timings *disp,
+							 unsigned int index)
+{
+	if (disp->num_timings > index)
+		return disp->timings[index];
+	else
+		return NULL;
+}
+
+void timings_release(struct display_timings *disp);

This function no longer exists.

Thierry