Re: [PATCH v4 8/8] serial: qcom-geni: Rework TX in FIFO mode to fix hangs/lockups

[Johan Hovold <johan@kernel.org>]

On Tue, Jun 25, 2024 at 07:29:38AM -0700, Doug Anderson wrote:
> On Tue, Jun 25, 2024 at 4:21 AM Johan Hovold <johan@kernel.org> wrote:

> > Right. But with a 16 1-byte word FIFO, we may be able to kick of a
> > really long transfer and just keep it running until it needs to be
> > kicked again (cf. enabling TX). The console code can easily insert
> > characters in the FIFO while the transfer is running (and would only
> > have to wait for 16 characters to drain in the worst case).
> >
> > Effectively, most of the identified issues would just go away, as
> > there's basically never any need to cancel anything except at port
> > shutdown.
> 
> Yeah, though you'd still have to make sure that the corner cases
> worked OK. You'll have to pick _some_ sort of fixed transfer size and
> make sure that all the special cases / console / kdb work if they show
> up right at the end of the transfer.

Yes, there are some details like that would need to be worked out.

> I was also a bit curious if there could be power implications with
> leaving an active TX command always in place. Perhaps geni wouldn't be
> able to drop some resources? Do you happen to know?

Hmm, good point. I'll see if I can ask someone with access to docs.

But I guess we can still continue to stop the command on stop_tx() (as
we are considering anyway) to avoid that.

> > I didn't do an in-depth analysis of the slowdown, but I did rerun the
> > tests now and I'm still seeing a 22-24% slowdown on x1e80100 with rc5.
> > This is a new platform so I compared with sc8280xp, which shows similar
> > numbers even if it's slightly faster to begin with:
> >
> >                                         sc8280xp        x1e80100
> >
> >         rc5 full series                 61 s            67 s
> >         rc5 last patch reverted         50 s            54 s
> >
> > I have a getty running and cat a 10x dmesg file of 543950 bytes to
> > /dev/ttyMSM0 from an ssh session (just catting in a serial console gives
> > similar numbers).
> 
> That's really weird / unexpected. Your hardware should be fancier than
> mine so, if anything, I'd expect it to be faster. Is there something
> causing you really bad interrupt latency or something? ...or is some
> clock misconfigured and "geni" is behaving sub-optimally?

That may be the case. I'm not seeing more interrupts with the last patch
applied, and not more time spent servicing interrupts (based on a quick
look at top), so it may just be geni taking a lot of time to start or
stop commands.

> ...although it wouldn't explain the slowness, I'd at least be a little
> curious if you've confirmed that you're running with a 16-word FIFO
> depth. See the function geni_se_get_tx_fifo_depth() where newer
> hardware can actually have larger FIFO depths.

No, I had confirmed that it is using 16 words (64 bytes).
 
> Just in case it matters, I'd be curious if you have
> `CONFIG_IRQ_TIME_ACCOUNTING=y`

I do, yes.

> Oh: one last thing to confirm: do you have kernel console output
> disabled for your tests? I've been doing tests with the kernel console
> _not_ enabled over the serial port and just an agetty there. I could
> believe things might be different if the kernel console was sending
> messages over the same port.

Yes, there has been no console output during my tests, and I get similar
results with the console disabled.

Johan