On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote: [...] > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index d08d799..2a23b18 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL > This framework adds support for low-level control of the video > output switch. > > +config DISPLAY_TIMING
DISPLAY_TIMINGS?
> #video output switch sysfs driver
> obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o
> +obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o
display_timings.o?
> +obj-$(CONFIG_VIDEOMODE) += videomode.o
> diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
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?
> diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
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
pgp1ObrkbKrOq.pgp
Description: PGP signature
_______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
