On 11/29/19 11:20 PM, Leonard Crestez wrote: > On 2019-11-29 1:34 PM, Rafael J. Wysocki wrote: >> On Tuesday, November 26, 2019 4:17:09 PM CET Leonard Crestez wrote: >>> Support for frequency limits in dev_pm_qos was removed when cpufreq was >>> switched to freq_qos, this series attempts to restore it by >>> reimplementing on top of freq_qos. >>> >>> Discussion about removal is here: >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-pm%2FVI1PR04MB7023DF47D046AEADB4E051EBEE680%40VI1PR04MB7023.eurprd04.prod.outlook.com%2FT%2F%23u&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=LYIRtXYe8qPz1G%2F0ADYpPhbhviv1pkk7%2B%2BQ1dX1DQR8%3D&reserved=0 >>> >>> The cpufreq core switched away because it needs contraints at the level >>> of a "cpufreq_policy" which cover multiple cpus so dev_pm_qos coupling >>> to struct device was not useful. Cpufreq could only use dev_pm_qos by >>> implementing an additional layer of aggregation anyway. >>> >>> However in the devfreq subsystem scaling is always performed on a per-device >>> basis so dev_pm_qos is a very good match. Support for dev_pm_qos in devfreq >>> core is here (latest version, no dependencies outside this series): >>> >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11252409%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=YodRx0IRVsjQXYA5X7UEosn%2FatO%2BWREfotwWrley2DQ%3D&reserved=0 >>> >>> That series is RFC mostly because it needs these PM core patches. >>> Earlier versions got entangled in some locking cleanups but those are >>> not strictly necessary to get dev_pm_qos functionality. >>> >>> In theory if freq_qos is extended to handle conflicting min/max values then >>> this sharing would be valuable. Right now freq_qos just ties two unrelated >>> pm_qos aggregations for min and max freq. >>> >>> --- >>> This is implemented by embeding a freq_qos_request inside dev_pm_qos_request: >>> the data field was already an union in order to deal with flag requests. >>> >>> The internal freq_qos_apply is exported so that it can be called from >>> dev_pm_qos apply_constraints. >>> >>> The dev_pm_qos_constraints_destroy function has no obvious equivalent in >>> freq_qos and the whole approach of "removing requests" is somewhat dubios: >>> request objects should be owned by consumers and the list of qos requests >>> will most likely be empty when the target device is deleted. Series follows >>> current pattern for dev_pm_qos. >>> >>> First two patches can be applied separately. >>> >>> Changes since v3: >>> * Fix s/QOS/QoS in patch 2 title >>> * Improves comments in kunit test >>> * Fix assertions after freq_qos_remove_request >>> * Remove (c) from NXP copyright header >>> * Wrap long lines in qos.c to be under 80 chars. This fixes checkpatch but the >>> rule is already broken by code in the files. >>> * Collect reviews >>> Link to v3: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11260627%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=e%2B5SGU%2Bx4UjxlY292ejMO1kDewc3MmFvCpf0SDB0K4U%3D&reserved=0 >>> >>> Changes since v2: >>> * #define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE FREQ_QOS_MAX_DEFAULT_VALUE >>> * #define FREQ_QOS_MAX_DEFAULT_VALUE S32_MAX (in new patch) >>> * Add initial kunit test for freq_qos, validating the MAX_DEFAULT_VALUE found >>> by Matthias and another recent fix. Testing this should be easier! >>> Link to v2: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11250413%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=vyz%2FN2x7OZPCSx4Md8yQkOSjPtfNUvW6%2FHtG0bTG1xU%3D&reserved=0 >>> >>> Changes since v1: >>> * Don't rename or EXPORT_SYMBOL_GPL the freq_qos_apply function; just >>> drop the static marker. >>> Link to v1: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11212887%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=SjcSQmMRZu0z3ATWygBpD8mRToCZT%2FBgQ7U3IRpMUB0%3D&reserved=0 >>> >>> Leonard Crestez (4): >>> PM / QoS: Initial kunit test >>> PM / QoS: Redefine FREQ_QOS_MAX_DEFAULT_VALUE to S32_MAX >>> PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs >>> PM / QoS: Restore DEV_PM_QOS_MIN/MAX_FREQUENCY >>> >>> drivers/base/Kconfig | 4 ++ >>> drivers/base/power/Makefile | 1 + >>> drivers/base/power/qos-test.c | 117 ++++++++++++++++++++++++++++++++++ >>> drivers/base/power/qos.c | 73 +++++++++++++++++++-- >>> include/linux/pm_qos.h | 86 ++++++++++++++----------- >>> kernel/power/qos.c | 4 +- >>> 6 files changed, 242 insertions(+), 43 deletions(-) >>> create mode 100644 drivers/base/power/qos-test.c >>> >>> >> >> I have applied the whole series as 5.5 material, but I have reordered the fix >> (patch [2/4]) before the rest of it and marked it for -stable. > > Thanks! > > Devfreq maintainers, do you think the devfreq parts could be queued for > 5.5 as well? I'm not sure about the mechanics involved in this since > devfreq qos depends at build time on this dev_pm_qos series. > > Latest version is here: https://patchwork.kernel.org/cover/11252415/ Hi Leonard, I agree devfreq's pm-qos patch. [1] https://patchwork.kernel.org/cover/11252415/ But, I think need to discuss about this series[2]. Acutally, I don't want to split out the device_register. [2]https://patchwork.kernel.org/cover/11242865/ > > It's RFC because it didn't compile against unpatched linux-next but it's > otherwise a reduced version of a series that went through review and > testing. > > -- > Regards, > Leonard > > -- Best Regards, Chanwoo Choi Samsung Electronics