On Fri, Dec 18, 2009 at 02:27:57PM +0100, Rafał Miłecki wrote: > W dniu 18 grudnia 2009 13:19 użytkownik Jerome Glisse > <gli...@freedesktop.org> napisał: > > On Thu, Dec 17, 2009 at 12:04:02AM +0100, Rafał Miłecki wrote: > >> In future we will execute AtomBIOS commands from contexts (like power > >> management) so we need to make sure we won't execute two commands at > >> same time to prevent locking up GPU. > >> > >> With this patch applied Sedat Dilek (RV515) was able to finally test > >> my PM patch. Also tested on my RV620. > >> > >> -- > >> Rafał > > > > I am in favor of a different approach, using a r/w lock and > > taking the lock in read in all path, and write in powermanager > > & init path. Doc state that we need exclusive access to GPU > > while doing PM stuff. Thus r/w lock sounds like what we want > > to do. Should be hard to add that to ioctl & modesetting callback. > > > > Never the less this patch might be usefull, as for instance > > we might be doing modesetting at the same time on 2 different > > head of an avivo gpu and this might lead to // execution of > > atombios and it's my understanding that atombios code is not > > safe in this regards. > > Sorry, I'm confused. So finally: do you think we should use read/write > locking? > > I guess we could make atom_execute_table using read-lock and add new > function like atom_execute_table_write_lock using write-lock. Then > convert sensitive places (like power management) to use _write_lock > instead standard atom_execute_table. > > Not sure about that modesetting at same time. If we don't want to do 2 > modesetting operations at same time, is there really sense to > introduce read/write locking? When will we use read lock then? Just > for connectors and powerplay_info reading? Sounds like wasting time > for that. > > Also could you explain: > > Should be hard to add that to ioctl & modesetting callback. > please? > > -- > Rafał >
To summup, this patch is good because it will exclude // execution of atom stuff which i think is good. Yet i don't think it's enough for power management. The idea of r/w lock is that we can have any number of reader so the lock won't block anyone in any path. But once you take the lock in write mode you are the only one messing with the hw which is what we want for PM (according to doc). To be effective reader lock must be taken in all ioctl & modesetting callback, and write lock in pm path. I think this r/w lock needs discussion, i am not fan of adding another lock to the driver. Cheers, Jerome ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
