Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	"Zhang, Rui" <rui.zhang@intel.com>,
	Bastien Nocera <hadess@hadess.net>,
	Mark Pearson <mpearson@lenovo.com>,
	"Limonciello, Mario" <Mario.Limonciello@dell.com>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>,
	Mark Gross <mgross@linux.intel.com>,
	Elia Devito <eliadevito@gmail.com>,
	Benjamin Berg <bberg@redhat.com>,
	"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>
Subject: Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework)
Date: Tue, 20 Oct 2020 14:34:59 +0200
Message-ID: <CAJZ5v0js7ujhzJF3UdO4+M2c5=N1kTP=8JRbdvxM7DuN4RqVUA@mail.gmail.com> (raw)
In-Reply-To: <64de7cf0-5d52-f8b3-426a-431fb3a6a6ec@redhat.com>

On Mon, Oct 19, 2020 at 8:43 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 10/18/20 2:31 PM, Rafael J. Wysocki wrote:
> > On Sun, Oct 18, 2020 at 11:41 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 10/16/20 4:51 PM, Rafael J. Wysocki wrote:
> >>> On Fri, Oct 16, 2020 at 1:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> <note folding the 2 threads we are having on this into one, adding every one from both threads to the Cc>
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote:
> >>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote:
> >>>>
> >>>> <snip>
> >>>>
> >>>>>>> First, a common place to register a DPTF system profile seems to be
> >>>>>>> needed and, as I said above, I wouldn't expect more than one such
> >>>>>>> thing to be present in the system at any given time, so it may be
> >>>>>>> registered along with the list of supported profiles and user space
> >>>>>>> will have to understand what they mean.
> >>>>>>
> >>>>>> Mostly Ack, I would still like to have an enum for DPTF system
> >>>>>> profiles in the kernel and have a single piece of code map that
> >>>>>> enum to profile names. This enum can then be extended as
> >>>>>> necessary, but I want to avoid having one driver use
> >>>>>> "Performance" and the other "performance" or one using
> >>>>>> "performance-balanced" and the other "balanced-performance", etc.
> >>>>>>
> >>>>>> With the goal being that new drivers use existing values from
> >>>>>> the enum as much as possible, but we extend it where necessary.
> >>>>>
> >>>>> IOW, just a table of known profile names with specific indices assigned to them.
> >>>>
> >>>> Yes.
> >>>>
> >>>>> This sounds reasonable.
> >>>>>
> >>>>>>> Second, irrespective of the above, it may be useful to have a
> >>>>>>> consistent way to pass performance-vs-power preference information
> >>>>>>> from user space to different parts of the kernel so as to allow them
> >>>>>>> to adjust their operation and this could be done with a system-wide
> >>>>>>> power profile attribute IMO.
> >>>>>>
> >>>>>> I agree, which is why I tried to tackle both things in one go,
> >>>>>> but as you said doing both in 1 API is probably not the best idea.
> >>>>>> So I believe we should park this second issue for now and revisit it
> >>>>>> when we find a need for it.
> >>>>>
> >>>>> Agreed.
> >>>>>
> >>>>>> Do you have any specific userspace API in mind for the
> >>>>>> DPTF system profile selection?
> >>>>>
> >>>>> Not really.
> >>>>
> >>>> So before /sys/power/profile was mentioned, but that seems more like
> >>>> a thing which should have a set of fixed possible values, iow that is
> >>>> out of scope for this discussion.
> >>>
> >>> Yes.
> >>>
> >>>> Since we all seem to agree that this is something which we need
> >>>> specifically for DPTF profiles maybe just add:
> >>>>
> >>>> /sys/power/dptf_current_profile    (rw)
> >>>> /sys/power/dptf_available_profiles (ro)
> >>>>
> >>>> (which will only be visible if a dptf-profile handler
> >>>>  has been registered) ?
> >>>>
> >>>> Or more generic and thus better (in case other platforms
> >>>> later need something similar) I think, mirror the:
> >>>>
> >>>> /sys/bus/cpu/devices/cpu#/cpufreq/energy_performance_* bits
> >>>> for a system-wide energy-performance setting, so we get:
> >>>>
> >>>> /sys/power/energy_performance_preference
> >>>> /sys/power/energy_performance_available_preferences
> >>>
> >>> But this is not about energy vs performance only in general, is it?
> >>>
> >>>> (again only visible when applicable) ?
> >>>>
> >>>> I personally like the second option best.
> >>>
> >>> But I would put it under /sys/firmware/ instead of /sys/power/ and I
> >>> would call it something like platform_profile (and
> >>> platform_profile_choices or similar).
> >>
> >> Currently we only have dirs under /sys/firmware:
> >>
> >> [hans@x1 ~]$ ls /sys/firmware
> >> acpi  dmi  efi  memmap
> >>
> >> But we do have /sys/firmware/apci/pm_profile:
> >>
> >> Documentation/ABI/stable/sysfs-acpi-pmprofile
> >>
> >> What:           /sys/firmware/acpi/pm_profile
> >> Date:           03-Nov-2011
> >> KernelVersion:  v3.2
> >> Contact:        linux-acpi@vger.kernel.org
> >> Description:    The ACPI pm_profile sysfs interface exports the platform
> >>                 power management (and performance) requirement expectations
> >>                 as provided by BIOS. The integer value is directly passed as
> >>                 retrieved from the FADT ACPI table.
> >> Values:         For possible values see ACPI specification:
> >>                 5.2.9 Fixed ACPI Description Table (FADT)
> >>                 Field: Preferred_PM_Profile
> >>
> >>                 Currently these values are defined by spec:
> >>                 0 Unspecified
> >>                 1 Desktop
> >>                 2 Mobile
> >>                 3 Workstation
> >>                 4 Enterprise Server
> >>                 ...
> >>
> >> Since all platforms which we need this for are ACPI based
> >> (and the involved interfaces are also all ACPI interfaces)
> >> how about:
> >>
> >> /sys/firmware/acpi/platform_profile
> >> /sys/firmware/acpi/platform_profile_choices
> >>
> >> ?
> >>
> >> I think this goes nice together with /sys/firmware/acpi/pm_profile
> >> although that is read-only and this is a read/write setting.
> >>
> >> Rafel, would:
> >>
> >> /sys/firmware/acpi/platform_profile
> >> /sys/firmware/acpi/platform_profile_choices
> >>
> >> work for you ?
> >
> > Yes, it would.
>
> Great. So I think hat means that we have the most important part
> for moving forward with this.
>
> So I guess the plan for this now looks something like this.
>
> 1. Rewrite my API docs RFC to update it for the new /sys/firmware/acpi/platform_profile[_choices]
>    plan (should be easy and a bunch of stuff like the "type" bit can just be dropped)
>
> 2. Add code somewhere under drivers/acpi which allows code from else where
>    to register itself as platform_profile handler/provider.

Sounds good to me.

> Rafael, any suggestions / preference for where this should be added under
> drivers/acpi ?  In a new .c file perhaps ?

Yes, that would be most suitable IMV.

> 3.1 Use the code from 2 to add support for platform-profile selection in
>     thinkpad_acpi (something for me or Mark Pearson) to do
> 3.2 Use the code from 2 to add support for platform-profile selection
>     to hp-wmi
> 3.3 (and to other drivers in the future).

Right.

      parent reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20201006122024.14539-1-daniel.lezcano@linaro.org>
     [not found] ` <eb26a00d-eee0-a4d1-ed25-61a661ad5683@redhat.com>
     [not found]   ` <8be66efd-7833-2c8a-427d-b0055c2f6ec1@linaro.org>
     [not found]     ` <97e5368b-228d-eca1-85a5-b918dfcfd336@redhat.com>
     [not found]       ` <CAJZ5v0gwc_d1vnwDVWXY+i4f0T2r0tAz8xuWV7oS_afsy7OocQ@mail.gmail.com>
     [not found]         ` <63dfa6a1-0424-7985-7803-756c0c5cc4a5@redhat.com>
     [not found]           ` <CAJZ5v0jpYpu3Tk7qq_MCVs0wUr-Dw0rY5EZELrVbQta0NZaoVA@mail.gmail.com>
     [not found]             ` <87d9a808-39d6-4949-c4f9-6a80d14a3768@redhat.com>
     [not found]               ` <CAJZ5v0iWmmu5WV7cX7uNb61NMYQ7s0dnhg1K+T0x90b3sBfU9w@mail.gmail.com>
2020-10-16 11:10                 ` Hans de Goede
2020-10-16 14:26                   ` Elia Devito
     [not found]                     ` <HK2PR0302MB2449214B28438ADC1790D468BD030@HK2PR0302MB2449.apcprd03.prod.outlook.com>
2020-10-16 14:43                       ` Fw: [External] " Mark Pearson
2020-10-16 15:16                         ` Elia Devito
2020-10-16 14:51                   ` Rafael J. Wysocki
2020-10-18  9:41                     ` Hans de Goede
2020-10-18 12:31                       ` Rafael J. Wysocki
2020-10-19 18:43                         ` Hans de Goede
     [not found]                           ` <HK2PR0302MB24494037019FBC7720976735BD1E0@HK2PR0302MB2449.apcprd03.prod.outlook.com>
2020-10-19 18:49                             ` Fw: [External] " Mark Pearson
2020-10-25 10:13                               ` Hans de Goede
2020-10-20 12:34                           ` Rafael J. Wysocki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJZ5v0js7ujhzJF3UdO4+M2c5=N1kTP=8JRbdvxM7DuN4RqVUA@mail.gmail.com' \
    --to=rafael@kernel.org \
    --cc=Mario.Limonciello@dell.com \
    --cc=andy@infradead.org \
    --cc=bberg@redhat.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=dvhart@infradead.org \
    --cc=eliadevito@gmail.com \
    --cc=hadess@hadess.net \
    --cc=hdegoede@redhat.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=mgross@linux.intel.com \
    --cc=mpearson@lenovo.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git