On Wed, 2012-03-14 at 18:33 +0200, Grazvydas Ignotas wrote: > On Wed, Mar 14, 2012 at 1:22 PM, Tomi Valkeinen <tomi.valkei...@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
signature.asc
Description: This is a digitally signed message part
