| Message ID | 20241021121138.422-1-zhengshaobo1@xiaomi.com (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | thermal: gov_power_allocator: Granted power set to max when nobody request power | expand |
Hello ZhengShaobo, On 10/21/24 13:11, ZhengShaobo wrote: > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > and cdev will be limited to the lowest performance. If our polling delay > is set to 200ms, it means that cdev cannot perform better within 200ms > even if cdev has a sudden load. This will affect the performance of cdev > and is not as expected. > > For this reason, if nobody requests power, then set the granted power to > the max_power. > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > --- > drivers/thermal/gov_power_allocator.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c > index 1b2345a697c5..4301516c0938 100644 > --- a/drivers/thermal/gov_power_allocator.c > +++ b/drivers/thermal/gov_power_allocator.c > @@ -356,11 +356,19 @@ static void divvy_up_power(struct power_actor *power, int num_actors, > u32 extra_power = 0; > int i; > > - /* > - * Prevent division by 0 if none of the actors request power. > - */ > - if (!total_req_power) > - total_req_power = 1; > + if (!total_req_power) { > + /* > + * Nobody requested anything, just give everybody > + * the maximum power > + */ > + for (i = 0; i < num_actors; i++) { > + struct power_actor *pa = &power[i]; > + > + pa->granted_power = pa->max_power; > + } > + > + return; > + } > > for (i = 0; i < num_actors; i++) { > struct power_actor *pa = &power[i]; Thank you for the patch. Good catch of that corner case. If there is no load on those devices, then the temperature should be low, lower than the 1st trip point. In that case we should reset the PID and call allow_maximum_power()... Although, what if the temperature is higher, e.g. due to ambient temperature and no load on the devices. Then we hit that corner case and your code would help. LGTM, Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > and cdev will be limited to the lowest performance. If our polling delay > is set to 200ms, it means that cdev cannot perform better within 200ms > even if cdev has a sudden load. This will affect the performance of cdev > and is not as expected. > > For this reason, if nobody requests power, then set the granted power to > the max_power. > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> I would have applied this, but your S-o-b above needs to be fixed. Why don't you use your real name there? If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", please let me know, and I will fix it for you when applying the patch. > --- > drivers/thermal/gov_power_allocator.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c > index 1b2345a697c5..4301516c0938 100644 > --- a/drivers/thermal/gov_power_allocator.c > +++ b/drivers/thermal/gov_power_allocator.c > @@ -356,11 +356,19 @@ static void divvy_up_power(struct power_actor *power, int num_actors, > u32 extra_power = 0; > int i; > > - /* > - * Prevent division by 0 if none of the actors request power. > - */ > - if (!total_req_power) > - total_req_power = 1; > + if (!total_req_power) { > + /* > + * Nobody requested anything, just give everybody > + * the maximum power > + */ > + for (i = 0; i < num_actors; i++) { > + struct power_actor *pa = &power[i]; > + > + pa->granted_power = pa->max_power; > + } > + > + return; > + } > > for (i = 0; i < num_actors; i++) { > struct power_actor *pa = &power[i]; > -- > 2.43.0 >
On Wed, Oct 23, 2024 at 12:09:44PM +0200, Rafael J. Wysocki wrote: > On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > > and cdev will be limited to the lowest performance. If our polling delay > > is set to 200ms, it means that cdev cannot perform better within 200ms > > even if cdev has a sudden load. This will affect the performance of cdev > > and is not as expected. > > > > For this reason, if nobody requests power, then set the granted power to > > the max_power. > > > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > I would have applied this, but your S-o-b above needs to be fixed. > Why don't you use your real name there? > > If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", > please let me know, and I will fix it for you when applying the patch. > Yes, it should be "ZhengShaobo <zhengshaobo1@xiaomi.com>". I would really appreciate your help in solving this problem. Thanks, ZhengShaobo
On Wed, Oct 23, 2024 at 12:09:44PM +0200, Rafael J. Wysocki wrote: > On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > > and cdev will be limited to the lowest performance. If our polling delay > > is set to 200ms, it means that cdev cannot perform better within 200ms > > even if cdev has a sudden load. This will affect the performance of cdev > > and is not as expected. > > > > For this reason, if nobody requests power, then set the granted power to > > the max_power. > > > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > I would have applied this, but your S-o-b above needs to be fixed. > Why don't you use your real name there? > > If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", > please let me know, and I will fix it for you when applying the patch. > Yes, it should be "ZhengShaobo <zhengshaobo1@xiaomi.com>". I would really appreciate your help in solving this problem. Thanks, ZhengShaobo
On Wed, Oct 23, 2024 at 12:09:44PM +0200, Rafael J. Wysocki wrote: > On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > > and cdev will be limited to the lowest performance. If our polling delay > > is set to 200ms, it means that cdev cannot perform better within 200ms > > even if cdev has a sudden load. This will affect the performance of cdev > > and is not as expected. > > > > For this reason, if nobody requests power, then set the granted power to > > the max_power. > > > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > I would have applied this, but your S-o-b above needs to be fixed. > Why don't you use your real name there? > > If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", > please let me know, and I will fix it for you when applying the patch. > Yes, it should be "ZhengShaobo <zhengshaobo1@xiaomi.com>". I would really appreciate your help in solving this problem. Thanks, ZhengShaobo
On Thu, Oct 24, 2024 at 10:01 AM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > On Wed, Oct 23, 2024 at 12:09:44PM +0200, Rafael J. Wysocki wrote: > > On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > > > and cdev will be limited to the lowest performance. If our polling delay > > > is set to 200ms, it means that cdev cannot perform better within 200ms > > > even if cdev has a sudden load. This will affect the performance of cdev > > > and is not as expected. > > > > > > For this reason, if nobody requests power, then set the granted power to > > > the max_power. > > > > > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > I would have applied this, but your S-o-b above needs to be fixed. > > Why don't you use your real name there? > > > > If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", > > please let me know, and I will fix it for you when applying the patch. > > > Yes, it should be "ZhengShaobo <zhengshaobo1@xiaomi.com>". > I would really appreciate your help in solving this problem. OK, applied as 6.13 material with the S-o-b tag as per the above, thanks!
On Thu, Oct 24, 2024 at 05:26:49PM +0200, Rafael J. Wysocki wrote: > On Thu, Oct 24, 2024 at 10:01 AM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > On Wed, Oct 23, 2024 at 12:09:44PM +0200, Rafael J. Wysocki wrote: > > > On Mon, Oct 21, 2024 at 2:12 PM ZhengShaobo <zhengshaobo1@xiaomi.com> wrote: > > > > > > > > From: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > > > > > When total_req_power is 0, divvy_up_power() will set granted_power to 0, > > > > and cdev will be limited to the lowest performance. If our polling delay > > > > is set to 200ms, it means that cdev cannot perform better within 200ms > > > > even if cdev has a sudden load. This will affect the performance of cdev > > > > and is not as expected. > > > > > > > > For this reason, if nobody requests power, then set the granted power to > > > > the max_power. > > > > > > > > Signed-off-by: zhengshaobo1 <zhengshaobo1@xiaomi.com> > > > > > > I would have applied this, but your S-o-b above needs to be fixed. > > > Why don't you use your real name there? > > > > > > If it can be changed to "ZhengShaobo <zhengshaobo1@xiaomi.com>", > > > please let me know, and I will fix it for you when applying the patch. > > > > > Yes, it should be "ZhengShaobo <zhengshaobo1@xiaomi.com>". > > I would really appreciate your help in solving this problem. > > OK, applied as 6.13 material with the S-o-b tag as per the above, thanks! I would like to backport the patch to android15-6.6, but this needs to be merged into the maintainer tree. I am wondering when this patch will be merged into your maintainer tree, and I hope you can let me know. Thanks, ZhengShaobo
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 1b2345a697c5..4301516c0938 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -356,11 +356,19 @@ static void divvy_up_power(struct power_actor *power, int num_actors, u32 extra_power = 0; int i; - /* - * Prevent division by 0 if none of the actors request power. - */ - if (!total_req_power) - total_req_power = 1; + if (!total_req_power) { + /* + * Nobody requested anything, just give everybody + * the maximum power + */ + for (i = 0; i < num_actors; i++) { + struct power_actor *pa = &power[i]; + + pa->granted_power = pa->max_power; + } + + return; + } for (i = 0; i < num_actors; i++) { struct power_actor *pa = &power[i];