9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] USB isochronous endpoint with feedback
@ 2021-02-08  2:02 Michael Forney
  2021-02-09  2:31 ` cinap_lenrek
  2021-02-10 19:10 ` cinap_lenrek
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Forney @ 2021-02-08  2:02 UTC (permalink / raw)
  To: 9front

[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]

Hi,

I'm working on getting my USB audio DAC to work in 9front.  The device
implements USB audio 2.0 which is a bit different than the 1.0
supported by nusb/audio, but I made some local changes to deal with
that (still WIP though).  The main difference is that sampling rate is
controlled through a clock source entity connected to the terminal
associated with the AS interface rather than being selected through an
alternate setting of the AS interface and sampling rate endpoint
control.

The issue I'm encountering is that the output iso endpoint is
asynchronous, and this is not supported by usb(3) or nusb(2).  An
async iso endpoint is not synchronized to USB (μ)frames, but to some
other clock.  Instead, sinks use an associated feedback endpoint which
provides a fixed-point number of samples to write each frame.  This is
updated by the device to maintain an optimal buffer level.  Luckily,
it seems that if I just ignore the feedback endpoint and use the
existing pollival/hz/samplesz mechanism, it works well enough.

However, the existence of the feedback endpoint breaks several
assumptions made in nusb(2).  The feedback endpoint has the same
number as the data endpoint, which causes usbd to create an unusable
rw iso endpoint.  Attached patch 1 prevents usbd from creating rw iso
endpoints (the feedback endpoint gets mapped to the 0x10|* range, just
as for endpoints with different types).

The other problem is that nusb(2) assumes that there is a single
endpoint per Altc; the settings for the last endpoint in the alt
setting take precedence.  There is a bit of overlap between fields in
Ep and Altc.  Both have maxpkt and ntds, but only Altc has pollival.
Altc contains the settings of the last endpoint in the alternate
setting, and Ep contains the last alt setting of the endpoint, but
neither of these is what I want (which is the alt settings of the data
endpoint).  nusb/audio uses the Altc to prepare the endpoint, so it
ends up using the settings for the feedback endpoint to configure the
data endpoint.

It seems to me that the hierarchy of structures in nusb(2) is not
quite right.  My best idea about how to fix it is to move Altc from
Iface to Ep, since it only seems to be used for endpoint settings.
That way, alternate settings for *each* endpoint in the interface are
available.  We could then drop the maxpkt and ntds fields from Ep,
using the ones from altc[0] instead.  This would probably be a fairly
involved change, and isn't something I want to tackle right now.  My
current solution is attached patch 2 to keep track of the iso usage in
the Ep, and patch 3 to only update the fields in the Altc for *data*
endpoints.  I believe this should be sufficient for USB audio devices.

Anyway, sorry for the long email.  Any advice or comments on my
patches would be appreciated!

-Michael

[-- Attachment #2.1: Type: text/plain, Size: 360 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Disposition: attachment; filename=0001-nusb-don-t-create-rw-iso-endpoints.patch
	Content-Type: text/plain; charset="US-ASCII"
	Content-Transfer-Encoding: 7bit

[-- Attachment #2.2: 0001-nusb-don-t-create-rw-iso-endpoints.patch.suspect --]
[-- Type: application/octet-stream, Size: 785 bytes --]

[-- Attachment #3.1: Type: text/plain, Size: 375 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Disposition: attachment; filename=0003-nusb-only-save-settings-for-data-endpoint-in-Altc.patch
	Content-Type: text/plain; charset="US-ASCII"
	Content-Transfer-Encoding: 7bit

[-- Attachment #3.2: 0003-nusb-only-save-settings-for-data-endpoint-in-Altc.patch.suspect --]
[-- Type: application/octet-stream, Size: 1417 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] USB isochronous endpoint with feedback
  2021-02-08  2:02 [9front] USB isochronous endpoint with feedback Michael Forney
@ 2021-02-09  2:31 ` cinap_lenrek
  2021-02-10 19:10 ` cinap_lenrek
  1 sibling, 0 replies; 4+ messages in thread
From: cinap_lenrek @ 2021-02-09  2:31 UTC (permalink / raw)
  To: 9front

thank you!

patch looks good to me...

will apply shortly...

--
cinap

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] USB isochronous endpoint with feedback
  2021-02-08  2:02 [9front] USB isochronous endpoint with feedback Michael Forney
  2021-02-09  2:31 ` cinap_lenrek
@ 2021-02-10 19:10 ` cinap_lenrek
  2021-02-10 20:55   ` Michael Forney
  1 sibling, 1 reply; 4+ messages in thread
From: cinap_lenrek @ 2021-02-10 19:10 UTC (permalink / raw)
  To: 9front

applied. you forgot to submit the patches for the header
with the addition of isousage attribute and the constants.

approximated it.

thank you very mutch :-)

changeset:   8331:09bb715f9ccb
tag:         tip
user:        cinap_lenrek@felloff.net
date:        Wed Feb 10 20:08:13 2021 +0100
summary:     nusb: don't create rw iso endpoints (by Michael Forney)

diff -r ce38dd881b90 -r 09bb715f9ccb sys/src/cmd/nusb/lib/parse.c
--- a/sys/src/cmd/nusb/lib/parse.c	Wed Feb 10 19:52:00 2021 +0100
+++ b/sys/src/cmd/nusb/lib/parse.c	Wed Feb 10 20:08:13 2021 +0100
@@ -111,8 +111,6 @@
 		return -1;
 	}
 	dep = (DEp *)b;
-	altc->attrib = dep->bmAttributes;	/* here? */
-	altc->interval = dep->bInterval;
 
 	type = dep->bmAttributes & 0x03;
 	addr = dep->bEndpointAddress;
@@ -145,13 +143,18 @@
 	ep->maxpkt = GET2(dep->wMaxPacketSize);
 	ep->ntds = 1 + ((ep->maxpkt >> 11) & 3);
 	ep->maxpkt &= 0x7FF;
-	altc->maxpkt = ep->maxpkt;
-	altc->ntds = ep->ntds;
 	ep->addr = addr;
 	ep->type = type;
 	ep->isotype = (dep->bmAttributes>>2) & 0x03;
+ 	ep->isousage = (dep->bmAttributes>>4) & 0x03;
 	ep->conf = c;
 	ep->iface = ip;
+	if(ep->type != Eiso || ep->isousage == Edata || ep->isousage == Eimplicit){
+		altc->attrib = dep->bmAttributes;
+		altc->interval = dep->bInterval;
+		altc->maxpkt = ep->maxpkt;
+		altc->ntds = ep->ntds;
+	}
 	for(i = 0; i < nelem(ip->ep); i++)
 		if(ip->ep[i] == nil)
 			break;
diff -r ce38dd881b90 -r 09bb715f9ccb sys/src/cmd/nusb/lib/usb.h
--- a/sys/src/cmd/nusb/lib/usb.h	Wed Feb 10 19:52:00 2021 +0100
+++ b/sys/src/cmd/nusb/lib/usb.h	Wed Feb 10 20:08:13 2021 +0100
@@ -116,6 +116,11 @@
 	Eadapt = 2,
 	Esync = 3,
 
+	/* endpoint isousage */
+	Edata = 0,
+	Efeedback = 1,
+	Eimplicit = 2,
+
 	/* config attrib */
 	Cbuspowered = 1<<7,
 	Cselfpowered = 1<<6,
@@ -209,7 +214,9 @@
 	uchar	addr;		/* endpt address, 0-15 (|0x80 if Ein) */
 	uchar	dir;		/* direction, Ein/Eout */
 	uchar	type;		/* Econtrol, Eiso, Ebulk, Eintr */
-	uchar	isotype;		/* Eunknown, Easync, Eadapt, Esync */
+	uchar	isotype;	/* Eunknown, Easync, Eadapt, Esync */
+	uchar	isousage;	/* Edata, Efeedback, Eimplicit */
+	
 	int	id;
 	int	maxpkt;		/* max. packet size */
 	int	ntds;		/* nb. of Tds per µframe */

changeset:   8330:ce38dd881b90
user:        cinap_lenrek@felloff.net
date:        Wed Feb 10 19:52:00 2021 +0100
summary:     nusb: don't create rw iso endpoints (by Michael Forney)

diff -r 2363b5489377 -r ce38dd881b90 sys/src/cmd/nusb/lib/parse.c
--- a/sys/src/cmd/nusb/lib/parse.c	Wed Feb 10 10:21:06 2021 -0800
+++ b/sys/src/cmd/nusb/lib/parse.c	Wed Feb 10 19:52:00 2021 +0100
@@ -127,7 +127,7 @@
 		ep = mkep(d, epid);
 		ep->dir = dir;
 	}else if((ep->addr & 0x80) != (addr & 0x80)){
-		if(ep->type == type)
+		if(ep->type == type && type != Eiso)
 			ep->dir = Eboth;
 		else {
 			/*



--
cinap

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [9front] USB isochronous endpoint with feedback
  2021-02-10 19:10 ` cinap_lenrek
@ 2021-02-10 20:55   ` Michael Forney
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Forney @ 2021-02-10 20:55 UTC (permalink / raw)
  To: 9front

On 2021-02-10, cinap_lenrek@felloff.net <cinap_lenrek@felloff.net> wrote:
> applied. you forgot to submit the patches for the header
> with the addition of isousage attribute and the constants.
>
> approximated it.

Hmm, I don't know what you mean. The patches look fine to me, and they
match what you committed except for some whitespace differences.
0002-nusb-record-iso-usage-in-endpoint.patch includes the changes to
lib/usb.h.

Anyway, thank you for applying!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-10 22:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08  2:02 [9front] USB isochronous endpoint with feedback Michael Forney
2021-02-09  2:31 ` cinap_lenrek
2021-02-10 19:10 ` cinap_lenrek
2021-02-10 20:55   ` Michael Forney

9front - general discussion about 9front

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/9front

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 9front 9front/ http://inbox.vuxu.org/9front \
		9front@9front.org
	public-inbox-index 9front

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.9front


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git