Re: [PATCH v2] OMAPDSS: provide default timings functions for panels

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

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

On Wed, 2012-03-14 at 18:33 +0200, Grazvydas Ignotas wrote:
> On Wed, Mar 14, 2012 at 1:22 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Hi,
> >
> > On Mon, 2012-03-12 at 13:27 +0200, Grazvydas Ignotas wrote:
> >> With this we can eliminate some duplicate code in panel drivers.
> >> Also lgphilips-lb035q02, nec-nl8048hl11-01b, picodlp and
> >> tpo-td043mtea1 gain support of timings control over sysfs.
> >
> > I don't like this patch.
> >
> > Panels usually have a single, fixed timing configuration that should be
> > used, like the ones you mention above. There's no need to alter the
> > timings.
> 
> But they often have a range of timings they can tolerate, and that can
> be used to alter refresh rate, for example. We do that on pandora to
> match graphics drawing rate (or multiples of it) to create a feeling
> smoothness.

True. And it's a valid operation anyway, so I guess there's no reason
why not to allow changing of the timings there.

> > But it's true that there's some duplicate code currently in the panel
> > drivers. However, adding just simple funcs like you did in this patch
> > doesn't work quite properly. There should be locking (for example to
> > prevent disabling the panel while timings are being set), and currently
> > the locking is panel driver specific.

Oh, and one more problem with the patch is that currently the panel
informs its inability to change timings by leaving set_timings and
check_timings as NULL, and this tells omapfb etc that the timings cannot
be changed, and the patch changes that behavior.

> ok, what about a version of this with .get_timings only then?
> This should not need a lock unless panel has a set function, but in
> that case panel will be expected to provide safe version of .get and
> .set itself.

I guess there's no harm in having default for get_timings(). It should
be present on all panel drivers anyway.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]