Re: [RESEND][PATCH v2] OMAP: DSS: Adding two APIs for panel-taal: check_timings and set_timings

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

Hi,

On Wed, 2011-02-16 at 07:54 -0600, Janorkar, Mayuresh wrote:
> check_timings and set_timings APIS are not present for panel-taal.
> 
> OMAPFB provides a bootarg omapfb.mode for setting mode parameters which include display,
> resolution, bits-per-pixel.
> 
> OMAPFB expects panel driver to have check_timings and set_timings APIs.
> These are checked by omapfb in case we wish to set default mode through bootargs.
> e.g.: omapfb.mode="lcd:864x480-16" (display device:width X height - bits per pixel)
> 
> omapfb_set_def_mode function in omapfb-main.c essentially needs these functions
> otherwise it would return -EINVAL and default mode sent through bootargs
> would be ignored.

I don't like this patch. You cannot change the timings for Taal, so
those added functions look quite hacky.

The reason for this patch isn't clear from the description (it should).
If I guess correctly, the point of the patch is to be able to change the
default color format via boot arguments when using taal panel?

If so, I think the change should be in the omapfb driver. Perhaps the
omapfb driver should:

1. check if check_timings & set_timings exist
2. if they do exist, do the same thing as the code does now
3. if they don't, use get_timings to verify that the given resolution is
correct

That wouldn't be perfect either, but I guess it should do the job. But
this is again something where FB framework and OMAP HW do not quite
match, and we end up with hacky solution, no matter what we do. But we
can try to keep the hacks in the omapfb driver =).

 Tomi