linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	rui.zhang@intel.com, amit.kucheria@verdurent.com
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [RFC][PATCH 4/5] thermal: Add support for setting polling interval
Date: Tue, 19 May 2020 12:25:00 +0200	[thread overview]
Message-ID: <3898cf1a-8e4c-def4-73f7-9ef4954e88b8@linaro.org> (raw)
In-Reply-To: <b74767964b028c297840aefc166e2384333afd3b.camel@linux.intel.com>

On 19/05/2020 01:46, Srinivas Pandruvada wrote:
> On Mon, 2020-05-18 at 18:51 +0200, Daniel Lezcano wrote:
>> On 04/05/2020 20:16, Srinivas Pandruvada wrote:
>>> Add new attribute in the thermal syfs for setting temperature
>>> sampling
>>> interval when CONFIG_THERMAL_USER_EVENT_INTERFACE is defined. The
>>> default
>>> value is 0, which means no polling.
>>>
>>> At this interval user space will get an event THERMAL_TEMP_SAMPLE
>>> with
>>> temperature sample. This reuses existing polling mecahnism when
>>> polling
>>> or passive delay is specified during zone registry. To avoid
>>> interference
>>> with passive and polling delay, this new polling attribute can't be
>>> used
>>> for those zones.
>>
>> The userspace can get the temperature whenever it wants via the
>> temperature file. The polling is designed for a specific hardware and
>> the slope of the temperature graphic.
>>
>> The userspace has the alternative of reading the temperature based on
>> its own timer or wait for (and stick to) the thermal framework
>> sampling
>> rate. Adding a notification in the update is enough IMO.
>>
> The problem with this approach is that the user can't change sampling
> interval. Those polling intervals are fixed during thermal-zone
> register. Is there any way to change those defaults from user space?

No, we can't but the userspace can decide when to read the temperature
(via sysfs or netlink) and thus decide its own sampling rate.

Otherwise, we are talking about an userspace governor, so the platform
is setup with the desired sampling rate + userspace governor.

> Kernel can start with some long polling interval and user space can
> change close to some trip.

Ok, let me rephrase it. This (big) comment encompass also patch 3/5.

I understood now the initial need of adding user trip points.

There are platforms where the interrupt mode does not exist so setting
an user trip point does not set the interrupt for the closer
temperature, hence we end up with a kernel sampling rate and in this
case adding a trip point + new user sampling rate is pointless as the
userspace can poll the temperature at its convenient rate.

If we summarize the different combinations we have:

1. monitoring : interrupt mode, mitigation : interrupt mode

There are no thermal zone update until an interrupt fires. The
mitigation is based on trip point crossed.

2. monitoring : interrupt mode, mitigation : polling

There are no thermal zone update until an interrupt fires. The
mitigation happens with a sampling rate specified with the polling rate.

3. monitoring : polling, mitigation : polling

The thermal zone is updated at the polling rate, the mitigation occurs
with an update at the second polling rate.

IIUC, the RFC proposes to add a new type of temperature threshold,
followed a new polling rate to update the userspace.

IMHO, it is not a good thing to delegate to the kernel what the
userspace can handle easily.

I suggest:

 - Not add another polling rate. If the thermal zone has a polling rate
or supports the interrupt mode, then the user trip point setup succeed
otherwise it fails and up to the userspace to read the temperature at
its convenient rate. (Note multiple process may want to get temperature,
so one should not set the rate of others).

 - Not add another temp threshold structure but add a new trip type
"user" and keep using the existing trip structures, so the notification
can happen in the handle_trip_point function. The sysfs only reflects
the setup via the "trip_point_x_hyst", "trip_point_0_temp",
"trip_point_x_type"

 - Do not use sysfs for setup but rely on the genetlink for one message
setup instead of multiple sysfs file writing. Adding a trip point will
be straighforward.


What do you think?


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  reply	other threads:[~2020-05-19 10:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 18:16 [RFC][PATCH 0/5] thermal: Add new mechanism to get thermal notification Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 1/5] thermal: Add support for /dev/thermal_notify Srinivas Pandruvada
2020-05-20  4:45   ` Amit Kucheria
2020-05-04 18:16 ` [RFC][PATCH 2/5] thermal: Add notification for zone creation and deletion Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 3/5] thermal: Add support for setting notification thresholds Srinivas Pandruvada
2020-05-18 16:37   ` Daniel Lezcano
2020-05-18 23:40     ` Srinivas Pandruvada
2020-05-20  4:28       ` Amit Kucheria
2020-05-20 18:16         ` Srinivas Pandruvada
2020-05-21  5:11           ` Amit Kucheria
2020-05-21 19:11             ` Srinivas Pandruvada
2020-05-04 18:16 ` [RFC][PATCH 4/5] thermal: Add support for setting polling interval Srinivas Pandruvada
2020-05-18 16:51   ` Daniel Lezcano
2020-05-18 23:46     ` Srinivas Pandruvada
2020-05-19 10:25       ` Daniel Lezcano [this message]
2020-05-21 22:26         ` Srinivas Pandruvada
2020-05-20  4:38   ` Amit Kucheria
2020-05-04 18:16 ` [RFC][PATCH 5/5] thermal: int340x: Use new device interface Srinivas Pandruvada
2020-05-20  4:49   ` Amit Kucheria

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=3898cf1a-8e4c-def4-73f7-9ef4954e88b8@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=amit.kucheria@verdurent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).