On Thu, Apr 14, 2022 at 10:00:35PM +0200, Mark Kettenis wrote:
> > Date: Thu, 14 Apr 2022 20:16:13 +0200
> > From: Marcus Glocker <mar...@nazgul.ch>
> >
> > I did hit this panic when trying to stream audio through
> > uaudio(4) / dwctwo(4):
> >
> > panic: _dmamap_sync: ran off map!
> > Stopped at panic+0x160: cmp w21, #0x0
> > TID PID UID PRFLAGS PFLAGS CPU COMMAND
> > *421282 69103 0 0x2 0x4000000 0K ld
> > db_enter() at panic+0x15c
> > panic() at _dmamap_sync+0x10c
> > _dmamap_sync() at dwc2_xfercomp_isoc_split_in+0xe0
> > dwc2_xfercomp_isoc_split_in() at dwc2_hc_xfercomp_intr+0xf4
> > dwc2_hc_xfercomp_intr() at dwc2_hc_n_intr+0x1d4
> > dwc2_hc_n_intr() at dwc2_handle_hcd_intr+0x1e4
> > dwc2_handle_hcd_intr() at dwc2_intr+0xb8
> >
> > Obviously usb_syncmem() expects an integer offset as second argument,
> > not a memory address. Looks like a typo to me ...
> >
> > OK?
>
> Hmm, that doesn't look right.
>
> >
> >
> > Index: sys/dev/usb/dwc2/dwc2_hcdintr.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2_hcdintr.c,v
> > retrieving revision 1.13
> > diff -u -p -u -p -r1.13 dwc2_hcdintr.c
> > --- sys/dev/usb/dwc2/dwc2_hcdintr.c 4 Sep 2021 10:19:28 -0000 1.13
> > +++ sys/dev/usb/dwc2/dwc2_hcdintr.c 14 Apr 2022 17:48:08 -0000
> > @@ -975,11 +975,11 @@ STATIC int dwc2_xfercomp_isoc_split_in(s
> >
> > if (chan->align_buf) {
> > dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> > - usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> > + usb_syncmem(qtd->urb->usbdma, 0,
> > chan->qh->dw_align_buf_size, BUS_DMASYNC_POSTREAD);
> > memcpy(qtd->urb->buf + frame_desc->offset +
> > qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
> > - usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> > + usb_syncmem(qtd->urb->usbdma, 0,
> > chan->qh->dw_align_buf_size, BUS_DMASYNC_PREREAD);
> > }
>
> When chan->align_buf is set we're doing DMA into dw_align_buf, and I
> think that means we need to use dw_align_buf_usbdma as the first
> argument to usb_syncmem since we need to make sure that the memory
> we're doing DMA into is coherent with the cache. So I think this
> should be something like:
>
> if (chan->align_buf) {
> struct usb_dma *ud = &chan->qh->dw_align_buf_usbdma;
>
> dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
> usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
> BUS_DMASYNC_POSTREAD);
> memcpy(qtd->urb->buf + frame_desc->offset +
> qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
> usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
> usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
> BUS_DMASYNC_PREREAD);
>
> I think that means the code in dwc2_update_uwb_state() is wrong too.
> But then I never fully understood this code...
I guess you're right. Also looking at the other usb_syncmem() calls
when dw_align_buf is involved. As well the comment for struct dwc2_qh
says:
* @dw_align_buf_size: Size of dw_align_buf
* @dw_align_buf_dma: DMA address for dw_align_buf
I've tested the following diff. Panic is gone, sound still crackles,
but still partially plays. I think there are other issues related to
the ISOC transfers which need to be investigated further.
Index: sys/dev/usb/dwc2/dwc2_hcdintr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2_hcdintr.c,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 dwc2_hcdintr.c
--- sys/dev/usb/dwc2/dwc2_hcdintr.c 4 Sep 2021 10:19:28 -0000 1.13
+++ sys/dev/usb/dwc2/dwc2_hcdintr.c 15 Apr 2022 07:25:42 -0000
@@ -493,13 +493,15 @@ STATIC int dwc2_update_urb_state(struct
/* Non DWORD-aligned buffer case handling */
if (chan->align_buf && xfer_length) {
dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
- usb_syncmem(urb->usbdma, 0, chan->qh->dw_align_buf_size,
+ struct usb_dma *ud = &chan->qh->dw_align_buf_usbdma;
+
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
chan->ep_is_in ?
BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
if (chan->ep_is_in)
memcpy(urb->buf + urb->actual_length,
chan->qh->dw_align_buf, xfer_length);
- usb_syncmem(urb->usbdma, 0, chan->qh->dw_align_buf_size,
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
chan->ep_is_in ?
BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE);
}
@@ -975,12 +977,14 @@ STATIC int dwc2_xfercomp_isoc_split_in(s
if (chan->align_buf) {
dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
- usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
- chan->qh->dw_align_buf_size, BUS_DMASYNC_POSTREAD);
+ struct usb_dma *ud = &chan->qh->dw_align_buf_usbdma;
+
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
+ BUS_DMASYNC_POSTREAD);
memcpy(qtd->urb->buf + frame_desc->offset +
qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
- usb_syncmem(qtd->urb->usbdma, chan->qh->dw_align_buf_dma,
- chan->qh->dw_align_buf_size, BUS_DMASYNC_PREREAD);
+ usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
+ BUS_DMASYNC_PREREAD);
}
qtd->isoc_split_offset += len;
@@ -1207,7 +1211,6 @@ STATIC void dwc2_update_urb_state_abn(st
/* Non DWORD-aligned buffer case handling */
if (chan->align_buf && xfer_length && chan->ep_is_in) {
dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
-
struct usb_dma *ud = &chan->qh->dw_align_buf_usbdma;
usb_syncmem(ud, 0, chan->qh->dw_align_buf_size,
