From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 0/5] Add power feature debugfs disabling Date: Tue, 4 Feb 2014 12:33:28 +0100 Message-ID: <20140204113328.GP17001@phenom.ffwll.local> References: <1391204572-18888-1-git-send-email-jeff.mcgee@intel.com> <20140201171422.GZ29091@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f169.google.com (mail-ea0-f169.google.com [209.85.215.169]) by gabe.freedesktop.org (Postfix) with ESMTP id 4D8FD1058A4 for ; Tue, 4 Feb 2014 03:33:37 -0800 (PST) Received: by mail-ea0-f169.google.com with SMTP id h10so4380529eak.28 for ; Tue, 04 Feb 2014 03:33:32 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140201171422.GZ29091@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Chris Wilson , jeff.mcgee@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, Feb 01, 2014 at 05:14:22PM +0000, Chris Wilson wrote: > On Fri, Jan 31, 2014 at 03:42:47PM -0600, jeff.mcgee@intel.com wrote: > > From: Jeff McGee > > > > This series has recently been accepted into the Haswell Android kernel and > > helps with debugging and profiling these power features. I would like it > > to be considered for upstream incorporation. The patches here have been > > rebased (minimal changes required) and compile-tested only. > > > > Broad device support is provided, accept for RPS and RC6 with Broadwell > > and Valleyview. Both of these were somewhat of a moving target and I > > didn't have devices to work with. Support can of course be added with > > help from appropriate folks. > > > > The hooks introduce some amount of overhead as an additional check is > > often needed to determine whether the feature is on or off - similar to > > the module parameters that already exist. I felt that the overhead was > > minimal enough and didn't want to ugly up the code with CONFIG_DEBUG_FS > > compile conditionals. But I'm open to the list's thoughts on this. > > > > IGT tests of these new interfaces can certainly be added. I wanted to > > make sure there was sufficient interest in having these interfaces before > > starting on the tests. So please provide feedback. > > I can see the value of adding this for power testing and the code looks > quite neat and self-contained. (The one bikeshed I have is that I would > like the parameter check and debug.disable check combined into a single > function call, similar to intel_enable_rc6() so that all the similar > logic is together, well commented and easy to verify, and hard for > callers to get wrong.) Longer term, should we not consider this for > our /sys/drm/card0/power API? i.e. do we see value beyond debugging and > testing? For fbc/psr/ips I'd even use a connector/crtc property if we really need to frob this. Would be a good excuse for some better kms cmdline utilities ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch