On Mon, Jan 10, 2022 at 07:14:58AM +0100, Tomasz Moń wrote:
> 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.

Agreed, and if the setup is different it might be better if the
configured waterlevel is lower.

> 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.

I thought a bit about different usecases and the latency penalty for
increasing RXTL from 1 to 8 is at most 8 character frames (i.e. the time
until the aging irq triggers) which happens with a probability of 7/8.
For fast transfers it's a bit less. For a smaller RXTL the penalty is
still 8 character frames but the probablity is a bit lower.
(RXTL - 1 / RXTL, i.e. 0 in the status quo with RXTL = 1)

So the dynamic approach is only a tad better on avarage and the worst
case latency is the same. (Assuming a random message length.)

The worst case is always: There are between 1 and RXTL - 1 chars sitting
in the FIFO and waiting for 8 character frames until they are received.

So I agree to just increasing the RXTL value to 8, and if there is a
problem with that, we can still consider the dynamic approach then.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |