On Sat, Jun 25, 2011 at 10:23:28AM +0100, Chris Wilson wrote: > On Fri, 24 Jun 2011 08:46:53 -0700, Ben Widawsky <[email protected]> wrote: > > On Fri, Jun 24, 2011 at 12:48:22AM +0100, Chris Wilson wrote: > > > On Thu, 23 Jun 2011 15:49:14 -0700, Ben Widawsky <[email protected]> > > > wrote: > > > > Provide a user accessible way to change the hangcheck timer. This is > > > > useful mostly for disabling the timer completely (value <= 0). > > > > > > Having i915.hangcheck_interval as a read/write module parameter was > > > better. :-p > > > -Chris > > > > I considered this, but I wasn't sure how to manage the sysfs parameters, > > and prevent users from doing stupid things. Furthermore, I think to be > > correct we must delete sync the timer if the user requests an interval > > of 0, and we can only do that if we have struct mutex (again the sysfs > > problem). > > You can either register a callback for when the parameter changes, but in > this case it is as easy as deleting the timer in the next hangcheck before > touching any GPU state. In that scenario the timer will only be enabled > again after the next execbuffers, that's a restriction I can live with for > simple code and keeping parameters out of debugfs. > > On the other hand, a debugfs would allow for a per-device parameter. For > that day in the far far future with multiprocessor igfx. Surreal isn't it? > -Chris
So what's the verdict? In term of LOC, your suggestion would probably be smaller, but in terms of complexity I actually think the current patch would be easier to understand, although to be fair, I didn't actually try coding it to see. If you feel strongly about a module parameter being the better solution, I will code it up. Ben _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
