Re: [PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
Hi Jean,
On 08/23/12 14:58, Jean Pihet wrote:
Hi Timo,
On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen timo.t.kokko...@iki.fi wrote:
That is correct. The API to use is the PM QoS API which cpuidle uses
to determine the next MPU state based on the allowed latency.
A more appropriate fix for the problem would be to modify the idle
layer so that it does not allow MPU going to too deep sleep modes when
it is expected that the timers need to wake up MPU.
The idle layer already uses the PM QoS framework to decide the next
MPU state. I think the right solution is to convert from
omap_pm_set_max_mpu_wakeup_lat to the PM QoS API.
Cf. http://marc.info/?l=linux-omapm=133968658305580w=2 for an
example of the conversion.
Thanks. It looks like really easy and straightforward conversion.
However, I couldn't find the patch you were referring to from any trees
I could find. So, I take that this API does not really have omap2
support in it yet? I tried git grepping through the source and to me it
appears there is nothing in place yet that actually restricts the MPU
sleep states on omap2 when requested.
Which puzzles me.. The patch you are referring to transfers the omap I2C
from the old omap PM API to the new QOS API is not applied yet in
mainline. The I2C is definitely working with the old API too, I'm just
wondering why I can't make it working with either of the APIs.. Am I
missing something here?
Therefore, it makes sense to actually remove this call entirely from
the ir-rx51 driver as it is both wrong and does nothing useful at the
moment.
Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
Regards,
Jean
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 9 -
include/media/ir-rx51.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 7eed541..ac7d3f0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if (count WBUF_LEN)
lirc_rx51-wbuf[count] = -1; /* Insert termination mark */
- /*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
-*/
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, 50);
-
lirc_rx51_on(lirc_rx51);
lirc_rx51-wbuf_index = 1;
pulse_timer_set_timeout(lirc_rx51, lirc_rx51-wbuf[0]);
@@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
-
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
Hi Timo, On Fri, Aug 24, 2012 at 10:14 AM, Timo Kokkonen timo.t.kokko...@iki.fi wrote: Hi Jean, On 08/23/12 14:58, Jean Pihet wrote: Hi Timo, On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen timo.t.kokko...@iki.fi wrote: That is correct. The API to use is the PM QoS API which cpuidle uses to determine the next MPU state based on the allowed latency. A more appropriate fix for the problem would be to modify the idle layer so that it does not allow MPU going to too deep sleep modes when it is expected that the timers need to wake up MPU. The idle layer already uses the PM QoS framework to decide the next MPU state. I think the right solution is to convert from omap_pm_set_max_mpu_wakeup_lat to the PM QoS API. Cf. http://marc.info/?l=linux-omapm=133968658305580w=2 for an example of the conversion. Thanks. It looks like really easy and straightforward conversion. However, I couldn't find the patch you were referring to from any trees Correct, this patch is not applied to the mainline code yet, it is provided as an example of the conversion. I could find. So, I take that this API does not really have omap2 support in it yet? I tried git grepping through the source and to me it appears there is nothing in place yet that actually restricts the MPU sleep states on omap2 when requested. The MPU state is controlled from the cpuidle framework, which retrieves the MPU allowed latency from the PM QoS framework. This is supported on OMAP2. Cf. the table of states and the associated latency in arch/arm/mach-omap2/cpuidle34xx.c. Which puzzles me.. The patch you are referring to transfers the omap I2C from the old omap PM API to the new QOS API is not applied yet in mainline. The I2C is definitely working with the old API too, I'm just wondering why I can't make it working with either of the APIs.. Am I missing something here? AFAIK the old API is a noop in mainline. Using the PM QoS API defnitely is supported, that is why I think the conversion needs to be performed. Therefore, it makes sense to actually remove this call entirely from the ir-rx51 driver as it is both wrong and does nothing useful at the moment. Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi Regards, Jean Thanks, Jean -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
On 08/24/12 12:04, Jean Pihet wrote: Hi Timo, On Fri, Aug 24, 2012 at 10:14 AM, Timo Kokkonen timo.t.kokko...@iki.fi wrote: Hi Jean, On 08/23/12 14:58, Jean Pihet wrote: Hi Timo, On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen timo.t.kokko...@iki.fi wrote: That is correct. The API to use is the PM QoS API which cpuidle uses to determine the next MPU state based on the allowed latency. A more appropriate fix for the problem would be to modify the idle layer so that it does not allow MPU going to too deep sleep modes when it is expected that the timers need to wake up MPU. The idle layer already uses the PM QoS framework to decide the next MPU state. I think the right solution is to convert from omap_pm_set_max_mpu_wakeup_lat to the PM QoS API. Cf. http://marc.info/?l=linux-omapm=133968658305580w=2 for an example of the conversion. Thanks. It looks like really easy and straightforward conversion. However, I couldn't find the patch you were referring to from any trees Correct, this patch is not applied to the mainline code yet, it is provided as an example of the conversion. I could find. So, I take that this API does not really have omap2 support in it yet? I tried git grepping through the source and to me it appears there is nothing in place yet that actually restricts the MPU sleep states on omap2 when requested. The MPU state is controlled from the cpuidle framework, which retrieves the MPU allowed latency from the PM QoS framework. This is supported on OMAP2. Cf. the table of states and the associated latency in arch/arm/mach-omap2/cpuidle34xx.c. Thanks for the pointer. I took a look at the state table and adjusted the latency requirements in my code. If I lower the latency from 50us to 10us, the timers are then waking up as they should be. I'll replace this patch with one where I convert it using the new API. Thanks! -Timo -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
Hi Timo,
On Wed, Aug 22, 2012 at 9:50 PM, Timo Kokkonen timo.t.kokko...@iki.fi wrote:
The ir-rx51 driver calls omap_pm_set_max_mpu_wakeup_lat() in order to
avoid problems that occur when MPU goes to sleep in the middle of
sending an IR code. Without such calls it takes ridiculously long for
the MPU to wake up from a sleep, which distorts the IR signal
completely.
However, the actual problem is that probably the GP timers are not
able to wake up the MPU at all. That is, adjusting the latency
requirements is not the correct way to solve the issue either. The
reason why this used to work with the original 2.6.28 based N900
kernel that is shipped with the product is that placing strict latency
requirements prevents the MPU from going to sleep at all. Furthermore,
the only PM layer imlementation available at the moment for OMAP3
doesn't do anything with the latency requirement placed with
omap_pm_set_max_mpu_wakeup_lat() calls.
That is correct. The API to use is the PM QoS API which cpuidle uses
to determine the next MPU state based on the allowed latency.
A more appropriate fix for the problem would be to modify the idle
layer so that it does not allow MPU going to too deep sleep modes when
it is expected that the timers need to wake up MPU.
The idle layer already uses the PM QoS framework to decide the next
MPU state. I think the right solution is to convert from
omap_pm_set_max_mpu_wakeup_lat to the PM QoS API.
Cf. http://marc.info/?l=linux-omapm=133968658305580w=2 for an
example of the conversion.
Therefore, it makes sense to actually remove this call entirely from
the ir-rx51 driver as it is both wrong and does nothing useful at the
moment.
Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
Regards,
Jean
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 9 -
include/media/ir-rx51.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 7eed541..ac7d3f0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if (count WBUF_LEN)
lirc_rx51-wbuf[count] = -1; /* Insert termination mark */
- /*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
-*/
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, 50);
-
lirc_rx51_on(lirc_rx51);
lirc_rx51-wbuf_index = 1;
pulse_timer_set_timeout(lirc_rx51, lirc_rx51-wbuf[0]);
@@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
-
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] ir-rx51: Remove MPU wakeup latency adjustments
The ir-rx51 driver calls omap_pm_set_max_mpu_wakeup_lat() in order to
avoid problems that occur when MPU goes to sleep in the middle of
sending an IR code. Without such calls it takes ridiculously long for
the MPU to wake up from a sleep, which distorts the IR signal
completely.
However, the actual problem is that probably the GP timers are not
able to wake up the MPU at all. That is, adjusting the latency
requirements is not the correct way to solve the issue either. The
reason why this used to work with the original 2.6.28 based N900
kernel that is shipped with the product is that placing strict latency
requirements prevents the MPU from going to sleep at all. Furthermore,
the only PM layer imlementation available at the moment for OMAP3
doesn't do anything with the latency requirement placed with
omap_pm_set_max_mpu_wakeup_lat() calls.
A more appropriate fix for the problem would be to modify the idle
layer so that it does not allow MPU going to too deep sleep modes when
it is expected that the timers need to wake up MPU.
Therefore, it makes sense to actually remove this call entirely from
the ir-rx51 driver as it is both wrong and does nothing useful at the
moment.
Signed-off-by: Timo Kokkonen timo.t.kokko...@iki.fi
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 2 --
drivers/media/rc/ir-rx51.c | 9 -
include/media/ir-rx51.h | 2 --
3 files changed, 13 deletions(-)
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c
b/arch/arm/mach-omap2/board-rx51-peripherals.c
index ca07264..e0750cb 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -34,7 +34,6 @@
#include plat/gpmc.h
#include plat/onenand.h
#include plat/gpmc-smc91x.h
-#include plat/omap-pm.h
#include mach/board-rx51.h
@@ -1227,7 +1226,6 @@ static void __init rx51_init_tsc2005(void)
#if defined(CONFIG_IR_RX51) || defined(CONFIG_IR_RX51_MODULE)
static struct lirc_rx51_platform_data rx51_lirc_data = {
- .set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
.pwm_timer = 9, /* Use GPT 9 for CIR */
};
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 7eed541..ac7d3f0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -267,12 +267,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
if (count WBUF_LEN)
lirc_rx51-wbuf[count] = -1; /* Insert termination mark */
- /*
-* Adjust latency requirements so the device doesn't go in too
-* deep sleep states
-*/
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, 50);
-
lirc_rx51_on(lirc_rx51);
lirc_rx51-wbuf_index = 1;
pulse_timer_set_timeout(lirc_rx51, lirc_rx51-wbuf[0]);
@@ -292,9 +286,6 @@ static ssize_t lirc_rx51_write(struct file *file, const
char *buf,
*/
lirc_rx51_stop_tx(lirc_rx51);
- /* We can sleep again */
- lirc_rx51-pdata-set_max_mpu_wakeup_lat(lirc_rx51-dev, -1);
-
return n;
}
diff --git a/include/media/ir-rx51.h b/include/media/ir-rx51.h
index 104aa89..57523f2 100644
--- a/include/media/ir-rx51.h
+++ b/include/media/ir-rx51.h
@@ -3,8 +3,6 @@
struct lirc_rx51_platform_data {
int pwm_timer;
-
- int(*set_max_mpu_wakeup_lat)(struct device *dev, long t);
};
#endif
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
