Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp361126pxu; Sun, 25 Oct 2020 03:22:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwSvrkTuN/9x4DFIGBqyOSU+6100NLRxwgQN/VS7huLn3ypuVzXjbUzFfJ13/btD1BT+V/d X-Received: by 2002:a17:906:81c5:: with SMTP id e5mr1028064ejx.10.1603621368751; Sun, 25 Oct 2020 03:22:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1603621368; cv=none; d=google.com; s=arc-20160816; b=bua05oYoCGfR6w025zapSvTmg1Qck6Xbt9jHiXt+2l1R0ywes49CfpseB9Nnz6+/W2 GH7t5cJp9iMX1WbKNGRjdP/rNEIv6IFcm20sgIqHYhSoZvo2SE7pE0T5f7JttBbxFVya vIdCX7N1ynwWXs2imqQCEjFcBP5t7wJTJelqgj+WdawScPU0L+k4unA6Rxx7TftvPk+w oHGw8J5nG9qFGhBRyumQy+4LoDaKPyVE4h5qL1pTxbGiujrTyMtGyfq3jEHvRyXJa6Zn kI6RdTeVNZ3+4WCoeisTSfDnrk3yFcz2YUeP61QXqmLp2pBwI6puKkd8EZ68ErpDF9JS 2IcA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=Pb5hNdlEn/EncaYY/4uh2BYj1mke68pTBE2+lMQ4Oos=; b=tfnvTQdYDb2CLdOPyQd1Zy5L4BRe+yh888957cZcTHDHLKt3V/t3UFIDHbcie7ESsZ co4UT+ueSqrEG6XBIAY1TurLJWD94VAT/FGOsUiNBaBvCbOTV/GvmhaYsvpkrHUItMRw X1N2lRyCy/uq6kSk9WVs77vypuCKWWS9shKY53h+Wm6/CbidS3PL/omIJASfMT6fhzr+ SDj2YanM00cDzuNdE3wHj4TWU2JgLDc4oAgByZr4j882LgiODAVqAQVVZp4S2Rwi7gnt 6V68J6vM73hGS8tROE2xbNkbrTAXyyShg+dwrrqi9reXqjM2BSEJ4i0fVvw+9rKpL/2i gMcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fDlhmuh9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id d18si4898950ejz.660.2020.10.25.03.22.26; Sun, 25 Oct 2020 03:22:48 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fDlhmuh9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1415078AbgJYKNf (ORCPT + 99 others); Sun, 25 Oct 2020 06:13:35 -0400 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:40509 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1415073AbgJYKNe (ORCPT ); Sun, 25 Oct 2020 06:13:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1603620811; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Pb5hNdlEn/EncaYY/4uh2BYj1mke68pTBE2+lMQ4Oos=; b=fDlhmuh9LCTf4wHrObzpDyTmVttN0+Iv2xMknyP+WenV7PfUKMAcKaEmtmHBaHdbDdKIhP BqxPvBxgf6C3valgahKJZY8B0Aw73FgsMJ52SYUQfm+xvXyVBYTbInV1UvcvWAwoXBAt4J UUy5TEj5fEZ3MZxUXB5eX7gq6oB47wI= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-348-C_B_u4pQOjqHZhlFn7URCA-1; Sun, 25 Oct 2020 06:13:29 -0400 X-MC-Unique: C_B_u4pQOjqHZhlFn7URCA-1 Received: by mail-ej1-f72.google.com with SMTP id b19so3319887eju.7 for ; Sun, 25 Oct 2020 03:13:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Pb5hNdlEn/EncaYY/4uh2BYj1mke68pTBE2+lMQ4Oos=; b=drTqBmCg8OBh48zfD0PfHMuKEOXvgoI8MmDjAKEHpmlmfzj8OXBVoke9Pf1fIVRhiE A/ulyQbNQL+5V/GnD5D6Zex6uYfZ7G+dEm/RgtXdN6JqMFlAMbyAbBTDZ5/mbcOdHqKm u/Yt8ehCsboSqgyF7LTTaX80OKGJx+VRgHp9CGgXgcaJlBhmMPA1tCnNCUHwTEcdBGrk X3NvgkYJFHwJZeCfCIGQi3+4IOHpZKWZmyeOpINNJaMSO5g8TdnVuTNCvIdYp0YD/Mv7 8fLvoL0Ulm8OlCAwE1xTzKqSlSDnS/NACuF5A+QsqlDcXsNKYcLFM8o3Jr6K6r1UFPZd KJkQ== X-Gm-Message-State: AOAM530r6CDTIaRlH1JUnE8ggjPm2EjBa5JtFnou43ovuuvwwyuCTz7o 1z9rF/sH6Fc8FmmBiXdjqtHL7kqlS1Yzse3UKSq2nPIucUQpSaem49TemeAHq2x7EJKwMhRvIE8 1Y1wCBTqsjXVhBHVao9IJoGuK X-Received: by 2002:a17:906:cc18:: with SMTP id ml24mr9999220ejb.298.1603620807926; Sun, 25 Oct 2020 03:13:27 -0700 (PDT) X-Received: by 2002:a17:906:cc18:: with SMTP id ml24mr9999191ejb.298.1603620807551; Sun, 25 Oct 2020 03:13:27 -0700 (PDT) Received: from x1.localdomain (2001-1c00-0c0c-fe00-d2ea-f29d-118b-24dc.cable.dynamic.v6.ziggo.nl. [2001:1c00:c0c:fe00:d2ea:f29d:118b:24dc]) by smtp.gmail.com with ESMTPSA id p16sm3706340ejz.103.2020.10.25.03.13.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 25 Oct 2020 03:13:26 -0700 (PDT) Subject: Re: Fw: [External] Re: [RFC] Documentation: Add documentation for new performance_profile sysfs class (Also Re: [PATCH 0/4] powercap/dtpm: Add the DTPM framework) To: Mark Pearson , "Rafael J. Wysocki" Cc: Daniel Lezcano , "Srinivas Pandruvada ,>" , "Lukasz Luba ,>" , "Linux Kernel Mailing List ,>" , Linux PM , "> Zhang, Rui" , Bastien Nocera , "> Mark Pearson" , "Limonciello, Mario ,>" , Darren Hart , "Andy ,> Shevchenko" , Mark Gross , "> Elia Devito" , Benjamin Berg , "> linux-acpi@vger.kernel.org" , "> platform-driver-x86@vger.kernel.org" References: <20201006122024.14539-1-daniel.lezcano@linaro.org> <8be66efd-7833-2c8a-427d-b0055c2f6ec1@linaro.org> <97e5368b-228d-eca1-85a5-b918dfcfd336@redhat.com> <63dfa6a1-0424-7985-7803-756c0c5cc4a5@redhat.com> <87d9a808-39d6-4949-c4f9-6a80d14a3768@redhat.com> <943531a7-74d6-7c7f-67bc-2645b3ba7b8a@redhat.com> <25d000cc-0c00-3b17-50f7-ca8de8b7a65b@redhat.com> <64de7cf0-5d52-f8b3-426a-431fb3a6a6ec@redhat.com> <7189685b-4975-ff09-8ac0-59f4e3204359@lenovo.com> From: Hans de Goede Message-ID: Date: Sun, 25 Oct 2020 11:13:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <7189685b-4975-ff09-8ac0-59f4e3204359@lenovo.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 10/19/20 8:49 PM, Mark Pearson wrote: > Hi > >> On 19/10/2020 14:43, Hans de Goede 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 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 wrote: >>>>>> >>>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 10/14/20 5:42 PM, Rafael J. Wysocki wrote: >>>>>>> On Wed, Oct 14, 2020 at 4:06 PM Hans de Goede wrote: >>>>>>>> On 10/14/20 3:33 PM, Rafael J. Wysocki wrote: >>>>>> >>>>>> >>>>>> >>>>>>>>> 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. >> >> Rafael, any suggestions / preference for where this should be added under >> drivers/acpi ?  In a new .c file perhaps ? >> >> 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). >> >> >> An open question is who will take care of 1. and 2. Mark (Pearson) >> do you feel up to this? or do you want me to take care of this? >> >> Regards, >> >> Hans >> > > Definitely up for (2) and will happily have a go at number (1). > > If there's an example of something similar I can look at for reference that would be helpful :) So what I would do is something like this: 1. Create a new include/acpi/platform_profile_provider.h file and in that file define: 1.1 An enum with possible profile values (as discussed we want the driver API to use an enum (which may be extended) and then use an array with strings inside the shared code to avoid differences like "Performance" vs "performance", etc. The enum should end with something like: ACPI_PLATFORM_PROFILE_COUNT 1.2 An acpi_platform_profile_provider struct in which contains a number of function-pointers for set/get callbacks (these callbacks should get/set the enum type, not strings) a "void *user_data" (to be passed back to the callbacks) and a: "unsigned long profile_choices[BITS_TO_LONGS(ACPI_PLATFORM_PROFILE_COUNT)] member. 1.3 A function to register / unregister a platform_profile_provider 2. Add a new .c file for this under drivers/acpi which should contain the actual implementation of the API (left to you) and at least 2 global variables a "struct acpi_platform_profile_provider *profile_provider;" and a mutex protecting this. The register/unregister function should lock the mutex to protect the pointer and the register function should check that this is the first provider being registered if there already is a provider registered then -EBUSY should be returned. The register/unregister functions should also add / remove the 2 /sys/firmware/acpi/platform_profile[_choices] files. Hint for the read function of the platform_profile_choices function you should use for_each_set_bit(bit, profile_choices, ACPI_PLATFORM_PROFILE_COUNT) {} Note this is more or less what I would do (minus any changes I would come up with when implementing this), feel free to use your own judgement here. Rafael, do you have any comments on this approach ? Regards, Hans