* [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode
@ 2023-07-06 16:14 Nícolas F. R. A. Prado
2023-07-07 7:53 ` Chen-Yu Tsai
2023-07-07 8:32 ` AngeloGioacchino Del Regno
0 siblings, 2 replies; 3+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-07-06 16:14 UTC (permalink / raw)
To: Daniel Lezcano
Cc: kernel, AngeloGioacchino Del Regno, Chen-Yu Tsai,
Bernhard Rosenkränzer, Nícolas F. R. A. Prado,
Amit Kucheria, Balsam CHIHI, Matthias Brugger, Rafael J. Wysocki,
Zhang Rui, linux-arm-kernel, linux-kernel, linux-mediatek,
linux-pm
Currently, when a controller is configured to use filtered mode, thermal
readings are valid only about 30% of the time.
Upon testing, it was noticed that lowering any of the interval settings
resulted in an improved rate of valid data. The same was observed when
decreasing the number of samples for each sensor (which also results in
quicker measurements).
Retrying the read with a timeout longer than the time it takes to
resample (about 344us with these settings and 4 sensors) also improves
the rate.
Lower all timing settings to the minimum, configure the filtering to
single sample, and poll the measurement register for at least one period
to improve the data validity on filtered mode. With these changes in
place, out of 100000 reads, a single one failed, ie 99.999% of the data
was valid.
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 1e11defe4f35..b5fb1d8bc3d8 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -58,11 +58,11 @@
#define LVTS_PROTTC(__base) (__base + 0x00CC)
#define LVTS_CLKEN(__base) (__base + 0x00E4)
-#define LVTS_PERIOD_UNIT ((118 * 1000) / (256 * 38))
-#define LVTS_GROUP_INTERVAL 1
-#define LVTS_FILTER_INTERVAL 1
-#define LVTS_SENSOR_INTERVAL 1
-#define LVTS_HW_FILTER 0x2
+#define LVTS_PERIOD_UNIT 0
+#define LVTS_GROUP_INTERVAL 0
+#define LVTS_FILTER_INTERVAL 0
+#define LVTS_SENSOR_INTERVAL 0
+#define LVTS_HW_FILTER 0x0
#define LVTS_TSSEL_CONF 0x13121110
#define LVTS_CALSCALE_CONF 0x300
#define LVTS_MONINT_CONF 0x9FBF7BDE
@@ -257,6 +257,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
struct lvts_sensor *lvts_sensor = thermal_zone_device_priv(tz);
void __iomem *msr = lvts_sensor->msr;
u32 value;
+ int rc;
/*
* Measurement registers:
@@ -269,7 +270,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
* 16 : Valid temperature
* 15-0 : Raw temperature
*/
- value = readl(msr);
+ rc = readl_poll_timeout(msr, value, value & BIT(16), 240, 400);
/*
* As the thermal zone temperature will read before the
@@ -282,7 +283,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
* functionning temperature and directly jump to a system
* shutdown.
*/
- if (!(value & BIT(16)))
+ if (rc)
return -EAGAIN;
*temp = lvts_raw_to_temp(value & 0xFFFF);
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode
2023-07-06 16:14 [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode Nícolas F. R. A. Prado
@ 2023-07-07 7:53 ` Chen-Yu Tsai
2023-07-07 8:32 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 3+ messages in thread
From: Chen-Yu Tsai @ 2023-07-07 7:53 UTC (permalink / raw)
To: Nícolas F. R. A. Prado
Cc: Daniel Lezcano, kernel, AngeloGioacchino Del Regno,
Bernhard Rosenkränzer, Amit Kucheria, Balsam CHIHI,
Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-pm
On Thu, Jul 06, 2023 at 12:14:33PM -0400, Nícolas F. R. A. Prado wrote:
> Currently, when a controller is configured to use filtered mode, thermal
> readings are valid only about 30% of the time.
>
> Upon testing, it was noticed that lowering any of the interval settings
> resulted in an improved rate of valid data. The same was observed when
> decreasing the number of samples for each sensor (which also results in
> quicker measurements).
>
> Retrying the read with a timeout longer than the time it takes to
> resample (about 344us with these settings and 4 sensors) also improves
> the rate.
>
> Lower all timing settings to the minimum, configure the filtering to
> single sample, and poll the measurement register for at least one period
> to improve the data validity on filtered mode. With these changes in
> place, out of 100000 reads, a single one failed, ie 99.999% of the data
> was valid.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
on Hayato. Reading out the temperature is now quite reliable.
>
> ---
>
> drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 1e11defe4f35..b5fb1d8bc3d8 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -58,11 +58,11 @@
> #define LVTS_PROTTC(__base) (__base + 0x00CC)
> #define LVTS_CLKEN(__base) (__base + 0x00E4)
>
> -#define LVTS_PERIOD_UNIT ((118 * 1000) / (256 * 38))
> -#define LVTS_GROUP_INTERVAL 1
> -#define LVTS_FILTER_INTERVAL 1
> -#define LVTS_SENSOR_INTERVAL 1
> -#define LVTS_HW_FILTER 0x2
> +#define LVTS_PERIOD_UNIT 0
> +#define LVTS_GROUP_INTERVAL 0
> +#define LVTS_FILTER_INTERVAL 0
> +#define LVTS_SENSOR_INTERVAL 0
> +#define LVTS_HW_FILTER 0x0
This hunk conflicts with
thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts
from your other series
> #define LVTS_TSSEL_CONF 0x13121110
> #define LVTS_CALSCALE_CONF 0x300
> #define LVTS_MONINT_CONF 0x9FBF7BDE
on this line.
> @@ -257,6 +257,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
> struct lvts_sensor *lvts_sensor = thermal_zone_device_priv(tz);
> void __iomem *msr = lvts_sensor->msr;
> u32 value;
> + int rc;
>
> /*
> * Measurement registers:
> @@ -269,7 +270,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
> * 16 : Valid temperature
> * 15-0 : Raw temperature
> */
> - value = readl(msr);
> + rc = readl_poll_timeout(msr, value, value & BIT(16), 240, 400);
>
> /*
> * As the thermal zone temperature will read before the
> @@ -282,7 +283,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
> * functionning temperature and directly jump to a system
> * shutdown.
> */
> - if (!(value & BIT(16)))
> + if (rc)
> return -EAGAIN;
>
> *temp = lvts_raw_to_temp(value & 0xFFFF);
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode
2023-07-06 16:14 [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode Nícolas F. R. A. Prado
2023-07-07 7:53 ` Chen-Yu Tsai
@ 2023-07-07 8:32 ` AngeloGioacchino Del Regno
1 sibling, 0 replies; 3+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-07-07 8:32 UTC (permalink / raw)
To: Nícolas F. R. A. Prado, Daniel Lezcano
Cc: kernel, Chen-Yu Tsai, Bernhard Rosenkränzer, Amit Kucheria,
Balsam CHIHI, Matthias Brugger, Rafael J. Wysocki, Zhang Rui,
linux-arm-kernel, linux-kernel, linux-mediatek, linux-pm
Il 06/07/23 18:14, Nícolas F. R. A. Prado ha scritto:
> Currently, when a controller is configured to use filtered mode, thermal
> readings are valid only about 30% of the time.
>
> Upon testing, it was noticed that lowering any of the interval settings
> resulted in an improved rate of valid data. The same was observed when
> decreasing the number of samples for each sensor (which also results in
> quicker measurements).
>
> Retrying the read with a timeout longer than the time it takes to
> resample (about 344us with these settings and 4 sensors) also improves
> the rate.
>
> Lower all timing settings to the minimum, configure the filtering to
> single sample, and poll the measurement register for at least one period
> to improve the data validity on filtered mode. With these changes in
> place, out of 100000 reads, a single one failed, ie 99.999% of the data
> was valid.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
>
> drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 1e11defe4f35..b5fb1d8bc3d8 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -58,11 +58,11 @@
> #define LVTS_PROTTC(__base) (__base + 0x00CC)
> #define LVTS_CLKEN(__base) (__base + 0x00E4)
>
> -#define LVTS_PERIOD_UNIT ((118 * 1000) / (256 * 38))
> -#define LVTS_GROUP_INTERVAL 1
> -#define LVTS_FILTER_INTERVAL 1
> -#define LVTS_SENSOR_INTERVAL 1
> -#define LVTS_HW_FILTER 0x2
> +#define LVTS_PERIOD_UNIT 0
> +#define LVTS_GROUP_INTERVAL 0
> +#define LVTS_FILTER_INTERVAL 0
> +#define LVTS_SENSOR_INTERVAL 0
> +#define LVTS_HW_FILTER 0x0
> #define LVTS_TSSEL_CONF 0x13121110
> #define LVTS_CALSCALE_CONF 0x300
> #define LVTS_MONINT_CONF 0x9FBF7BDE
> @@ -257,6 +257,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
> struct lvts_sensor *lvts_sensor = thermal_zone_device_priv(tz);
> void __iomem *msr = lvts_sensor->msr;
> u32 value;
> + int rc;
>
> /*
> * Measurement registers:
> @@ -269,7 +270,7 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
> * 16 : Valid temperature
> * 15-0 : Raw temperature
> */
> - value = readl(msr);
#define LVTS_MSR_READ_TIMEOUT_US 400
then, either 240 like this...
#define LVTS_MSR_READ_WAIT_US ((LVTS_MSR_READ_TIMEOUT_US / 2) - 10)
..or 220 (if valid) like this..
#define LVTS_MSR_READ_WAIT_US ((LVTS_MSR_READ_TIMEOUT_US / 20) + \
(LVTS_MSR_READ_TIMEOUT_US / 2))
..or just "240-and-that's-it"
#define LVTS_MSR_READ_WAIT_US 240
rc = readl_poll_timeout(msr, value, value & BIT(16),
LVTS_MSR_READ_WAIT_US, LVTS_MSR_READ_TIMEOUT_US);
Cheers,
Angelo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-07 8:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-06 16:14 [PATCH] thermal/drivers/mediatek/lvts_thermal: Make readings valid in filtered mode Nícolas F. R. A. Prado
2023-07-07 7:53 ` Chen-Yu Tsai
2023-07-07 8:32 ` AngeloGioacchino Del Regno
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).