On Wed, Jul 25, 2012 at 12:25:00PM -0700, Ben Widawsky wrote: > On Tue, 24 Jul 2012 23:33:45 +0200 > Daniel Vetter <[email protected]> wrote: > > > This way it's easier so see what belongs together, and what is used > > by the ilk ips code. Also add some comments that explain the locking. > > > > v2: Missed one place that the dev_priv->ips change caught ... > > > > Signed-off-by: Daniel Vetter <[email protected]> > Reviewed-by: Ben Widawsky <[email protected]> > With a few comments below > > > + /* gen6+ rps state */ > > + struct { > > + struct work_struct work; > > + u32 pm_iir; > > + /* lock - irqsave spinlock that protectects the work_struct and > > + * pm_iir. */ > > + spinlock_t lock; > > + > > + /* The below variables an all the rps hw state are protected by > > + * dev->struct mutext. */ > > + u8 cur_delay; > > + u8 min_delay; > > + u8 max_delay; > > + } rps; > > + > > > > u8 cur_delay; > > u8 min_delay; > > Could you add the reason for adding new cur/min/max delays to the commit > message? From just this hunk it would seem we'd want to remove the old > cur/min/max.
I'll add a comment that (cur|min|max)_delay need to be duplicated, because they're also used by the ips code when applying the patch. -Daniel -- Daniel Vetter Mail: [email protected] Mobile: +41 (0)79 365 57 48 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
