On Wed, 2011-11-23 at 15:18 +0530, Archit Taneja wrote: > Hi, > > On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote: > > The functions in apply.c, called mostly via function pointers in overlay > > and overlay_manager structs, will be divided into two groups. The other > > group will not sleep and can be called from interrupts, and the other > > group may sleep. > > Small sentence issue above, both groups are called the 'other group'.
Thanks, fixed. > > > > The idea is that the non-sleeping functions may only change certain > > settings in overlays and managers, and those settings may only affect > > the particular overlay/manager. For example, set the base address of the > > overlay. > > > > The blocking functions, however, will handle more complex configuration > > changes. For example, when an overlay is enabled and fifo-merge feature > > is used, we need to do the enable in multiple steps, waiting in between, > > and the change affects multiple overlays and managers. > > > > This patch adds the mutex which is used in the blocking functions to > > have exclusive access to overlays and overlay managers. > > Previously, when we changed the links between 'overlay->managers' and > 'manager->devices', it wasn't protected by a lock. Why is it needed now? Previously many places were missing a lock =). > As an example, suppose we are changing a manager's device to some other > display. Is this lock preventing someone else to get the older > 'mgr->device' rather than the new one? Hmm. We need some lock there, that's for sure, as set/unset manager are changing the manager's list of overlays. However, it is also protected by the spinlock, so in that sense mutex is not necessary. I have to say I'm not sure if mutex is needed at this point. However, consider the end result when fifo-merge is used: dss_ovl_enable() will take the mutex, then it does the configuration in multiple steps, doing multiple spin_lock & spin_unlocks, waiting in between. If we had only a spinlock in set/unset_manager, the manager could be changed while dss_ovl_enable is doing the process of enabling the overlay. So I may have added mutexes or spinlocks a bit early in the series to some places, but I don't see any harm in that. It'd be rather difficult to try to find the exact spots where a lock becomes a requirement. Tomi
signature.asc
Description: This is a digitally signed message part
