Re: [PATCH v2] sched: Provide USF for the portable equipment.
On 31/07/2020 14:46, Dongdong Yang wrote:
> From: Dongdong Yang
[...]
> + if (unlikely(usf_vdev.enable_debug))
> + trace_printk
> + ("%s: cpu_id=%d non_ux=%d usf_up=%d usf_down=%d util=%lu\n",
> + USF_TAG, cpuid, usf_vdev.usf_non_ux,
> + usf_vdev.usf_up_l0, usf_vdev.usf_down, *util);
trace_printk in code ?
> +static int usf_lcd_notifier(struct notifier_block *nb,
> + unsigned long val, void *data)
> +{
> + struct fb_event *evdata = data;
> + unsigned int blank;
> +
> + if (!evdata)
> + return 0;
> +
> + if (val != FB_EVENT_BLANK)
> + return 0;
> +
> + if (evdata->data && val == FB_EVENT_BLANK) {
> + blank = *(int *)(evdata->data);
> +
> + switch (blank) {
> + case FB_BLANK_POWERDOWN:
> + usf_vdev.is_screen_on = 0;
> + if (usf_vdev.sysctl_sched_usf_non_ux != 0)
> + static_branch_enable(_task_pred_set);
> + else
> + static_branch_disable(_task_pred_set);
> +
> + break;
> +
> + case FB_BLANK_UNBLANK:
> + usf_vdev.is_screen_on = 1;
> + if (usf_vdev.sysctl_sched_usf_up_l0 != 0 ||
> + usf_vdev.sysctl_sched_usf_down != 0)
> + static_branch_enable(_task_pred_set);
> + else
> + static_branch_disable(_task_pred_set);
> + break;
> + default:
> + break;
> + }
> +
> + usf_vdev.is_sched_usf_enabled = 1;
> + if (usf_vdev.enable_debug)
> + trace_printk("%s : usf_vdev.is_screen_on:%d\n",
> + __func__, usf_vdev.is_screen_on);
> + }
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block usf_lcd_nb = {
> + .notifier_call = usf_lcd_notifier,
> + .priority = INT_MAX,
> +};
Looks like those notifications should enable/disable the schedutil
extension adjust_task_pred_demand(). Who's calling them?
The 3 sched_usf_FOO sys files somehow have an influence here too. How
should this work?
I see a fb_register_client() in intera_monitor_init further below.
[...]
> +usf_attr_rw(sched_usf_up_l0_r);
> +usf_attr_rw(sched_usf_down_r);
> +usf_attr_rw(sched_usf_non_ux_r);
What can I do with these three files? What do they stand for?
root@h620:/sys/devices/system/cpu/sched_usf# ls
sched_usf_down_r sched_usf_non_ux_r sched_usf_up_l0_r
[...]
> +static int __init intera_monitor_init(void)
> +{
> + int res = -1;
> + struct attribute_group *attr_group;
> +
> + res = fb_register_client(_lcd_nb);
> + if (res < 0) {
> + pr_err("Failed to register usf_lcd_nb!\n");
> + return res;
[...]
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] sched: Provide USF for the portable equipment.
On Fri, Jul 31, 2020 at 08:46:30PM +0800, Dongdong Yang wrote:
> +static ssize_t store_sched_usf_up_l0_r(struct kobject *kobj,
> +struct kobj_attribute *attr,
> +const char *buf, size_t count)
> +{
> + int val = 0;
> + int ret;
> +
> + ret = kstrtoint(buf, 0, );
> + if (ret) {
> + pr_err(USF_TAG "set state fail ret=%d\n", ret);
> + return ret;
> + }
> +
> + if ((val >= 0) && (val <= BOOST_MAX_V)) {
> + usf_vdev.sysctl_sched_usf_up_l0 = val;
> + usf_vdev.usf_up_l0 = LEVEL_TOP -
> + DIV_ROUND_UP(val, BOOST_MAX_V / 2);
> + } else {
> + pr_err(USF_TAG "%d should fall into [%d %d]",
> +val, 0, BOOST_MAX_V);
> + }
> + if ((usf_vdev.sysctl_sched_usf_up_l0 == 0) &&
> + (usf_vdev.sysctl_sched_usf_down == 0))
> + static_branch_disable(_task_pred_set);
> + else
> + static_branch_enable(_task_pred_set);
> +
> + return count;
> +}
> +
> +static ssize_t store_sched_usf_down_r(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int val = 0;
> + int ret;
> +
> + ret = kstrtoint(buf, 0, );
> + if (ret) {
> + pr_err(USF_TAG "set state fail ret=%d\n", ret);
> + return ret;
> + }
> +
> + if ((val >= BOOST_MIN_V) && (val <= 0)) {
> + usf_vdev.sysctl_sched_usf_down = val;
> + usf_vdev.usf_down = DIV_ROUND_UP(-val, -BOOST_MIN_V / 2);
> + } else {
> + pr_err(USF_TAG "%d should fall into [%d %d]",
> +val, BOOST_MIN_V, 0);
> + }
> + if ((usf_vdev.sysctl_sched_usf_up_l0 == 0) &&
> + (usf_vdev.sysctl_sched_usf_down == 0))
> + static_branch_disable(_task_pred_set);
> + else
> + static_branch_enable(_task_pred_set);
> +
> + return count;
> +}
> +
> +static ssize_t store_sched_usf_non_ux_r(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int val = 0;
> + int ret;
> +
> + ret = kstrtoint(buf, 0, );
> + if (ret) {
> + pr_err(USF_TAG "set state fail ret=%d\n", ret);
> + return ret;
> + }
> +
> + if ((val >= BOOST_MIN_V) && (val <= 0)) {
> + usf_vdev.sysctl_sched_usf_non_ux = val;
> + usf_vdev.usf_non_ux = DIV_ROUND_UP(-val, -BOOST_MIN_V / 2);
> + } else {
> + pr_err(USF_TAG "%d should fall into [%d %d]",
> +val, BOOST_MIN_V, 0);
> + }
> + if (usf_vdev.sysctl_sched_usf_non_ux == 0)
> + static_branch_disable(_task_pred_set);
> + else
> + static_branch_enable(_task_pred_set);
> +
> + return count;
> +}
> +
> +#define usf_attr_rw(_name) \
> +static struct kobj_attribute _name = \
> +__ATTR(_name, 0664, show_##_name, store_##_name)
You have a lot of sysfs files here, but no documentation for them at
all, why not?
> +
> +#define usf_show_node(_name, _value) \
> +static ssize_t show_##_name \
> +(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> \
> +{\
> + unsigned int len = 0; \
> + unsigned int max_len = 8; \
> + len += \
> + snprintf(buf + len, max_len - len, "%d",\
> + usf_vdev.sysctl_##_value); \
> + return len; \
> +}
> +
> +usf_show_node(sched_usf_up_l0_r, sched_usf_up_l0);
> +usf_show_node(sched_usf_down_r, sched_usf_down);
> +usf_show_node(sched_usf_non_ux_r, sched_usf_non_ux);
> +
> +usf_attr_rw(sched_usf_up_l0_r);
> +usf_attr_rw(sched_usf_down_r);
> +usf_attr_rw(sched_usf_non_ux_r);
> +
> +static struct attribute *sched_attrs[] = {
> + _usf_up_l0_r.attr,
> + _usf_down_r.attr,
> + _usf_non_ux_r.attr,
> + NULL,
> +};
> +
> +static struct attribute_group sched_attr_group = {
> + .attrs = sched_attrs,
> +};
> +
> +static int usf_dbg_get(void *data, u64 *val)
> +{
> + *val = (u64)usf_vdev.enable_debug;
> +
> + return 0;
> +}
> +
> +static int usf_dbg_set(void *data, u64 val)
> +{
> + usf_vdev.enable_debug = !!val;
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(usf_dbg_fops, usf_dbg_get,
> + usf_dbg_set, "%llu\n");
> +
> +static int __init intera_monitor_init(void)
> +{
> + int res =
Re: [PATCH v2] sched: Provide USF for the portable equipment.
On Fri, Jul 31, 2020 at 08:46:30PM +0800, Dongdong Yang wrote:
> --- /dev/null
> +++ b/drivers/staging/fbsched/usf.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 XiaoMi Inc.
> + * Author: Yang Dongdong
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + * See http://www.gnu.org/licenses/gpl-2.0.html for more details.
Please remove the license "boilerplate" text as you have the SPDX line
on top.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define BOOST_MIN_V -100
> +#define BOOST_MAX_V 100
> +#define LEVEL_TOP 3
> +
> +#define USF_TAG "[usf_sched]"
> +
> +DEFINE_PER_CPU(unsigned long[PID_MAX_LIMIT], task_hist_nivcsw);
> +
> +static struct {
> + bool is_sched_usf_enabled;
> + int enable_debug;
> + int is_screen_on;
> + struct kobject *kobj;
A raw kobject? For a driver? are you _SURE_???
> + struct dentry *debugfs_entry;
Why do you need this?
> + usf_vdev.enable_debug = 0;
> + usf_vdev.debugfs_entry = debugfs_create_file("usf_dbg",
> + 0660, NULL, NULL,
> + _dbg_fops);
> + if (!usf_vdev.debugfs_entry)
> + pr_err("Failed to create usf_dbg!\n");
How can that value be NULL?
There is no need to check debugfs functions for error values.
But in this case, why are you writing a single file to the root of
debugfs? Why not put it in a directory instead? That would be much
nicer, don't you think?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] sched: Provide USF for the portable equipment.
On Fri, Jul 31, 2020 at 08:46:30PM +0800, Dongdong Yang wrote: > drivers/staging/Kconfig | 2 + > drivers/staging/Makefile | 1 + > drivers/staging/fbsched/Kconfig | 10 ++ > drivers/staging/fbsched/Makefile | 2 + > drivers/staging/fbsched/usf.c| 346 > +++ > kernel/sched/cpufreq_schedutil.c | 3 + > kernel/sched/sched.h | 10 ++ > 7 files changed, 374 insertions(+) > create mode 100644 drivers/staging/fbsched/Kconfig > create mode 100644 drivers/staging/fbsched/Makefile > create mode 100644 drivers/staging/fbsched/usf.c I asked for a TODO file, like all other staging drivers have, why did you not include one here? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
