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:
signature.asc
Description: This is a digitally signed message part
