From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] drm/i915: Tune done rc6 enabling output Date: Mon, 4 Aug 2014 17:43:59 +0200 Message-ID: <20140804154359.GC8727@phenom.ffwll.local> References: <1407143908-19196-1-git-send-email-daniel.vetter@ffwll.ch> <20140804153428.GA15200@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f178.google.com (mail-we0-f178.google.com [74.125.82.178]) by gabe.freedesktop.org (Postfix) with ESMTP id 360726E41D for ; Mon, 4 Aug 2014 08:43:49 -0700 (PDT) Received: by mail-we0-f178.google.com with SMTP id w61so7973331wes.9 for ; Mon, 04 Aug 2014 08:43:48 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140804153428.GA15200@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 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 > > --- > > 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