Re: [PATCH 1/2] drm/i915: Tune done rc6 enabling output

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Aug 04, 2014 at 04:34:28PM +0100, Chris Wilson wrote:
> On Mon, Aug 04, 2014 at 11:18:27AM +0200, Daniel Vetter wrote:
> > Power users spot this and then get adventurous and try to adjust
> > module driver options. Nothing good ever came out of that, so
> > hide it better.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 338a80b6773e..4f879494e0c5 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3425,10 +3425,10 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
> >  		else
> >  			mode = 0;
> >  	}
> > -	DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> > -		 (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> > -		 (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> > -		 (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
> > +	DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> > +		      (mode & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> > +		      (mode & GEN6_RC_CTL_RC6p_ENABLE) ? "on" : "off",
> > +		      (mode & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
> >  }
> 
> I disagree. This is a good informative message about the capabilities
> enabled for their GPU. If you want to make the statement that the user
> shouldn't touch the values, make that adjustment emit a WARNING and
> implement the kernel tainting you suggested. Also fixing [drm] to be
> more specific would be useful.

Well I'd agree if we'd generally report this, but we don't. This really is
the only feature where we tell the user in dmesg what we've done with it,
for a lot of other stuff (psr, fbc, rps ...) we just silently set stuff
up. rc6 really is the only thing and I don't see why it's special. Imo
users should use tools like powertop to assess the effectivenes or our pm
code anyway, not look at dmesg.

So this is orthogonal to the module param tainting which unfortunately
missed 3.17 since Jani (who took over) went on vacatino before completing
the revised patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch