[PATCH] radeon: Make PM info available to all, not just debug users

[Alex Deucher]

On Mon, Jun 4, 2012 at 7:30 AM, Christian König <deathsimple at vodafone.de> wrote:
> On 04.06.2012 10:44, Lauri Kasanen wrote:
>>
>> On Sun, 03 Jun 2012 12:54:30 +0200
>> Christian König<deathsimple at vodafone.de>  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-land ;)
>>>
>>> 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 have
>> 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
>> complained about it. This info should be right next to where it is tweaked,
>> ie right 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.
>> Therefore requiring a debug option (CONFIG_DEBUGFS) to see this info is
>> plain wrong.. This info needs to be available to all users, including those
>> on production kernels without such debug options.
>>
>>
>> --
>>
>> So the issue is the location of the info, not the format. I'd be more than
>> 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_freq
>> 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.

That's my main concern.  I don't want maintain the current debug
interface as a stable one, that's why it's in debugfs.

Alex

>
> Christian.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel