Re: [PATCH 2/2] drm/i915: Introduce intel_set_rps()

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 29, 2015 at 01:57:16PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 28, 2015 at 10:29:06AM +0000, Chris Wilson wrote:
> > On Tue, Jan 27, 2015 at 04:36:16PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Replace the valleyview_set_rps() and gen6_set_rps() calls with
> > > intel_set_rps() which itself does the IS_VALLEYVIEW() check. The
> > > code becomes simpler since the callers don't have to do this check
> > > themselves.
> > > 
> > > Most of the change was performe with the following semantic patch:
> > > @@
> > > expression E1, E2;
> > > @@
> > > (
> > > - valleyview_set_rps(E1, E2)
> > > + intel_set_rps(E1, E2)
> > > |
> > > - gen6_set_rps(E1, E2)
> > > + intel_set_rps(E1, E2)
> > > )
> > > 
> > > @@
> > > expression E1, E2, E3;
> > > @@
> > > - if (IS_VALLEYVIEW(E1)) {
> > > -  intel_set_rps(E2, E3);
> > > - } else {
> > > -  intel_set_rps(E2, E3);
> > > - }
> > > + intel_set_rps(E2, E3);
> > > 
> > > Adding intel_set_rps() and making valleyview_set_rps() and gen6_set_rps()
> > > static was done manually.
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Hmm, I like half of them :|
> > 
> > The external callers from i915_debugfs.c, i915_irq.c are good. But
> > inside intel_pm.c, it is more mixed. gen6_rps_boost() clearly wants
> > intel_set_rps(), but from within the gen specific lowlevel functions, it
> > looks odder to callback into intel_set_rps.
> 
> The semantic patch was greedy, and I figured I'd leave it that way to
> avoid someone accidentally copy-pasting the wrong thing. But I can
> change them back if that's the preferred way.

Yeah in case of doubt I prefer less abstraction and just direct calls of
the matching low-level functions. Merged the first patch meanwhile,
thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx