[PATCH 2/2] tty: serial: OMAP: transmit FIFO threshold interrupts don't wake the chip

[Govindraj]

On Mon, Jan 23, 2012 at 2:20 PM, Govindraj <govindraj.ti at gmail.com> wrote:
> On Sat, Jan 21, 2012 at 12:57 PM, Paul Walmsley <paul at pwsan.com> wrote:
>> It seems that when the transmit FIFO threshold is reached on OMAP
>> UARTs, it does not result in a PRCM wakeup.  This appears to be a
>> silicon bug.  This means that if the MPU powerdomain is in a low-power
>> state, the MPU will not be awakened to refill the FIFO until the next
>> interrupt from another device.
>>
>> The best solution, at least for the short term, would be for the OMAP
>> serial driver to call a OMAP subarchitecture function to prevent the
>> MPU powerdomain from entering a low power state while the FIFO has
>> data to transmit.  However, we no longer have a clean way to do this,
>> since patches that add platform_data function pointers have been
>> deprecated by the OMAP maintainer.  So we attempt to work around this
>> as well.  The workarounds depend on the setting of CONFIG_CPU_IDLE.
>>
>> When CONFIG_CPU_IDLE=n, the driver will now only transmit one byte at
>> a time.  This causes the transmit FIFO threshold interrupt to stay
>> active until there is no more data to be sent.  Thus, the MPU
>> powerdomain stays on during transmits.  Aside from that energy
>> consumption penalty, each transmitted byte results in a huge number of
>> UART interrupts -- about five per byte.  This wastes CPU time and is
>> quite inefficient, but is probably the most expedient workaround in
>> this case.
>>
>> When CONFIG_CPU_IDLE=y, there is a slightly more direct workaround:
>> the PM QoS constraint can be abused to keep the MPU powerdomain on.
>> This results in a normal number of interrupts, but, similar to the
>> above workaround, wastes power by preventing the MPU from entering
>> WFI.
>>
>> Future patches are planned for the 3.4 merge window to implement more
>> efficient, but also more disruptive, workarounds to these problems.
>>
>
> With these two patches number of interrupts seems to increase by 24x
> after boot up.
>
> [...]
>
> / # cat /proc/interrupts | grep "UART2"
>  74:       3902      INTC  OMAP UART2
> / #
> [...]
>
> without these two patches + cpu_idle enabled
>
> [...]
>
> / #
> / # cat /proc/interrupts | grep "UART2"
>  74:        158      INTC  OMAP UART2
> / #
>
> [...]
>
>
> I am using beagle xm and 3.3rc1
>
> Looks like there are far two many uart irqs which
> keeps the mpu busy and thus preventing uart sluggishness.
>

after looking in closely,
looks like every byte is getting transferred in case
of non cpu_idle cases causing 1 irq for each tx.


> --
> Thanks,
> Govindraj.R
>
>
>> DMA operation is unaffected by this patch.
>>
>> Signed-off-by: Paul Walmsley <paul at pwsan.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen at ti.com>
>> Cc: Govindraj Raja <govindraj.r at ti.com>
>> Cc: Kevin Hilman <khilman at ti.com>
>> ---
>>  arch/arm/plat-omap/include/plat/omap-serial.h |    1
>>  drivers/tty/serial/omap-serial.c              |   51 +++++++++++++++++++++++++
>>  2 files changed, 51 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
>> index 9ff4444..12a64eb 100644
>> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
>> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
>> @@ -131,6 +131,7 @@ struct uart_omap_port {
>>        u32                     context_loss_cnt;
>>        u32                     errata;
>>        u8                      wakeups_enabled;
>> +       u8                      max_tx_count;
>>
>>        struct pm_qos_request   pm_qos_request;
>>        u32                     latency;
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 9de7d71..621fde5 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -88,6 +88,49 @@ static inline void serial_omap_clear_fifos(struct uart_omap_port *up)
>>        serial_out(up, UART_FCR, 0);
>>  }
>>
>> +/**
>> + * serial_omap_block_cpu_low_power_state - prevent MPU pwrdm from leaving ON
>> + * @up: struct uart_omap_port *
>> + *
>> + * Prevent the MPU powerdomain from entering a power state lower than
>> + * ON.  (It should be sufficient to prevent it from entering INACTIVE,
>> + * but there is presently no easy way to do this.)  This works around
>> + * a suspected silicon bug in the OMAP UART IP blocks.  The UARTs should
>> + * wake the PRCM when the transmit FIFO threshold interrupt is raised, but
>> + * they do not.   See also serial_omap_allow_cpu_low_power_state().  No
>> + * return value.
>> + */
>> +static void serial_omap_block_cpu_low_power_state(struct uart_omap_port *up)
>> +{
>> +#ifdef CONFIG_CPU_IDLE
>> +       up->latency = 1;
>> +       schedule_work(&up->qos_work);
>> +#else
>> +       up->max_tx_count = 1;
>> +#endif
>> +}
>> +
>> +/**
>> + * serial_omap_allow_cpu_low_power_state - remove power state restriction on MPU
>> + * @up: struct uart_omap_port *
>> + *
>> + * Cancel the effects of serial_omap_block_cpu_low_power_state().
>> + * This should allow the MPU powerdomain to enter a power state lower
>> + * than ON, assuming the rest of the kernel is not restricting it.
>> + * This works around a suspected silicon bug in the OMAP UART IP
>> + * blocks.  The UARTs should wake the PRCM when the transmit FIFO
>> + * threshold interrupt is raised, but they do not.  No return value.
>> + */
>> +static void serial_omap_allow_cpu_low_power_state(struct uart_omap_port *up)
>> +{
>> +#ifdef CONFIG_CPU_IDLE
>> +       up->latency = up->calc_latency;
>> +       schedule_work(&up->qos_work);
>> +#else
>> +       up->max_tx_count = up->port.fifosize / 4;
>> +#endif
>> +}
>> +
>>  /*
>>  * serial_omap_get_divisor - calculate divisor value
>>  * @port: uart port info
>> @@ -163,6 +206,9 @@ static void serial_omap_stop_tx(struct uart_port *port)
>>                serial_out(up, UART_IER, up->ier);
>>        }
>>
>> +       if (!up->use_dma)
>> +               serial_omap_allow_cpu_low_power_state(up);
>> +
>>        pm_runtime_mark_last_busy(&up->pdev->dev);
>>        pm_runtime_put_autosuspend(&up->pdev->dev);
>>  }
>> @@ -264,7 +310,7 @@ static void transmit_chars(struct uart_omap_port *up)
>>                serial_omap_stop_tx(&up->port);
>>                return;
>>        }
>> -       count = up->port.fifosize / 4;
>> +       count = up->max_tx_count;
>>        do {
>>                serial_out(up, UART_TX, xmit->buf[xmit->tail]);
>>                xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>> @@ -297,6 +343,7 @@ static void serial_omap_start_tx(struct uart_port *port)
>>
>>        if (!up->use_dma) {
>>                pm_runtime_get_sync(&up->pdev->dev);
>> +               serial_omap_block_cpu_low_power_state(up);
>>                serial_omap_enable_ier_thri(up);
>>                pm_runtime_mark_last_busy(&up->pdev->dev);
>>                pm_runtime_put_autosuspend(&up->pdev->dev);
>> @@ -1421,6 +1468,8 @@ static int serial_omap_probe(struct platform_device *pdev)
>>        up->port.fifosize = 64;
>>        up->port.ops = &serial_omap_pops;
>>
>> +       up->max_tx_count = up->port.fifosize / 4;
>> +
>>        if (pdev->dev.of_node)
>>                up->port.line = of_alias_get_id(pdev->dev.of_node, "serial");
>>        else
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html