From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-15?Q?Christian_K=F6nig?= Subject: Re: [PATCH] radeon: Make PM info available to all, not just debug users Date: Mon, 04 Jun 2012 13:30:30 +0200 Message-ID: <4FCC9C56.2000505@vodafone.de> References: <20120602190858.335065e1.cand@gmx.com> <4FCB4266.80907@vodafone.de> <20120604114431.aeeda075.cand@gmx.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 6024A9E77C for ; Mon, 4 Jun 2012 04:30:35 -0700 (PDT) In-Reply-To: <20120604114431.aeeda075.cand@gmx.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Lauri Kasanen Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 04.06.2012 10:44, Lauri Kasanen wrote: > On Sun, 03 Jun 2012 12:54:30 +0200 > Christian K=F6nig wrote: > >>> This moves the pm_info file from debugfs to next to the other two power= files. >>> >>> Requested by several users at Phoronix. >>> >>> PS: Please CC me. Also please be gentle, it's my first step in kernel-l= and ;) >> Hui? What should this be good for? >> >> Sysfs files are for setting driver parameters, like the power management >> method or profile currently in use. One major advantage of sysfs is the >> strict rules for a permanent and machine usable interface, for example >> it is mandatory to only specify one parameter per sysfs file. >> >> Debugfs on the other hand should be used for human readable >> informations, e.g. the printing the current clocks in a human readable >> form. Also you don't need a debug build or turn on any other debugging >> facility to get those information, just take a look under >> "sys/kernel/debug/dri/*". > I have no such dir, /sys/kernel/debug. The fact you have it means you hav= e CONFIG_DEBUGFS enabled and mounted. > >> So the code is actually quite valid as it is. > First, the current location is illogical, and several users have complain= ed about it. This info should be right next to where it is tweaked, ie righ= t next to power_profile and power_method. That is where it's expected to be= by users. > > Secondly, checking the clocks is absolutely not a debug operation. Theref= ore requiring a debug option (CONFIG_DEBUGFS) to see this info is plain wro= ng.. This info needs to be available to all users, including those on produ= ction kernels without such debug options. > > -- > > So the issue is the location of the info, not the format. I'd be more tha= n happy to split it into six files (default_core_clock, current_core_clock.= ..) that each offer just a kHz number, just like the cpufreq scaling_cur_fr= eq do. Would that be preferable? Yeah, that sounds like a start, and also only register those files if = the clock in question is really available, e. g. integrated chipsets = doesn't have a memory clock for example. But I have my doubts that it would be accepted easily, cause for debugfs = we can pretty much pump every information in there we want, while sysfs = files must maintain a more or less stable API for setting system = parameters, see Documentation/sysfs-rules.txt. Christian.