Re: [RFC 00/17] OMAPDSS: Change way of passing timings from panel driver to interface

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

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

On Wed, 2012-08-08 at 11:35 +0530, Archit Taneja wrote:
> On Tuesday 07 August 2012 08:02 PM, Tomi Valkeinen wrote:
> > On Wed, 2012-08-01 at 16:01 +0530, Archit Taneja wrote:
> >> This series tries to make interface drivers less dependent on omap_dss_device
> >> which represents a panel/device connected to that interface. The current way of
> >> configuring an interface is to populate the panel's omap_dss_device instance
> >> with parameters common to the panel and the interface, they are either populated
> >> in the board file, or in the panel driver. Panel timings, number of lanes
> >> connected to interface, and pixel format are examples of such parameters, these
> >> are then extracted by the interface driver to configure itself.
> >
> > The series looks good. I had only a few comments to make, but obviously
> > this needs quite a bit of testing. I'll try it out.
> 
> One thing I'm not sure about is whether these new functions should be 
> aware of the state of the output. For example, if we call set_timings() 
> with DSI video mode which is already enabled, the timings won't really 
> take any impact.
> 
> Similar issues would occur when we try to make other ops like 
> set_data_lines() or set_pixel_format(). These need to be called before 
> the output is enabled. I was wondering if we would need to add 
> intelligence here to make panel drivers less likely to make mistakes.

Hmm, true. It'd be nice if the functions returned -EBUSY if the
operation cannot be done while the output is enabled.

We have the dssdev->state, but we should get rid of that (or leave it to
panel drivers). It'd be good if the output drivers know whether the
output is enabled or not. I think this data is already tracked by
apply.c. It's about ovl managers, but I think that's practically the
same as output.

Calling dss_mgr_enable() will set mp->enabled = true, which could be
returned via dss_mgr_is_enabled() or such.

Then again, it wouldn't be many lines of codes to track the enable-state
in each output driver. So if we have any suspicions that mp->enabled
doesn't quite work for, say, dsi, we could just add a private "enabled"
member to dsi. But I don't right away see why dss_mgr_is_enabled()
wouldn't work.

 Tomi


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