Re: [PATCH] serial: imx: reduce RX interrupt frequency

From: "Tomasz Moń" <tomasz.mon@camlingroup.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Sergey Organov" <sorganov@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>,
	Jiri Slaby <jirislaby@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	k.drobinski@camlintechnologies.com,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-serial@vger.kernel.org, Shawn Guo <shawnguo@kernel.org>,
	Fabio Estevam <festevam@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>
Subject: Re: [PATCH] serial: imx: reduce RX interrupt frequency
Date: Mon, 10 Jan 2022 07:14:58 +0100	[thread overview]
Message-ID: <a6f0b0dc-da67-2488-16b2-40f5ad049fde@camlingroup.com> (raw)
In-Reply-To: <20220106150552.vdt5qabe2xj4kasg@pengutronix.de>

On 06.01.2022 16:05, Uwe Kleine-König wrote:
> On Wed, Jan 05, 2022 at 04:34:21PM +0300, Sergey Organov wrote:
>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>>> Maybe it's more sensible to make this even more dynamic: e.g. keep it at
>>> 1 as default and increase the water level when a certain irq frequency
>>> is reached?
>>
>> Too complex, and too many questions, I'm afraid. What is "irq
>> frequency", exactly? For this particular driver, or overall system?
>> Measured on what time interval? What is the threshold? Do we drop the
>> water level back to 1 when "irq frequency" is down again? Will we just
>> create re-configure storm at some conditions? Etc.....
> 
> It could be as easy as increasing the waterlevel by one if an RX irq
> happens with USR1.AGTIM = 0 and reset to 1 if USR1.AGTIM = 1.

This is indeed simple, but I fail to see the actual benefit of doing so.

> This makes sure that receiving at a low frequency makes the hardware
> interrupt the CPU early, and a burst doesn't starve the CPU.

If the communication involves multiple characters sent in burst (frame),
and includes some sort of CRC check (so only complete frames are useful,
as no partial processing can be done because checksum has to be checked
first), then it is the latency after the last character (not the first)
that matters.

RXTL upper value has to be capped. If RXTL goes above CTSTL then it
would limit throughput if hardware flow control is enabled. If hardware
flow control is not enabled, then the higher RXTL gets, the higher is
the risk of not reading RxFIFO in time. Setting RXTL to quarter of the
RxFIFO (8) seems to leave enough time for RX interrupt and at the same
time noticeably lowers the interrupt rate.

Best Regards,
Tomasz Mon