[usb][audio] Fix a bug, improve volume hack.
Fix an issue where we were attempting to set the sample rate of an
interface which only supported a single sample rate, which cause the
device to stall the interface's endpoint. Now, if an interface
supports only one sample rate, we do not attempt to set it to anything
in particular.
Also, add some functionality to the set_volume hack. Volume in USB
audio was more complicated than the previous hack was expecting.
Volume controls (if they exist) will exist in the 'Feature' unit (if
it exists) in the great big bucket of audio units which make up the
audio path. The feature unit has a bitmask which indicates which
control exist for every channel which has features. There tends to be
at least one channel (index 0, the 'master' channel), and sometimes
more channels (one for each speaker, indexed [1, N]). There are at
least 10 different controls in the feature unit descriptor for each
channel including volume and mute. Attempting to either set or query
a control which does not exist for a given channel is supposed to
cause the device to stall the endpoint (which some devices do and some
devices don't).
So, with this CL, we now pay attention to the feature unit descriptor
when attempting to set the initial volume, and we don't attempt to
touch controls which are not supposed to exist. When setting the
volume, we attempt to set the volume and mute controls for each
channel in the feature unit based on support.
Finally, there was a bug in the commands being issued when setting
volumes which has been fixed. Specifically, there are 4 numbers which
make up the address of a control in a feature unit. They are the...
1) Interface ID
2) Feature Unit ID
3) Control Selector
4) Channel Number
Each is 1 byte long. 1 and 2 are supposed to be combined and sent as
the wIndex field of the control request, while 3 and 4 are supposed to
be combined and sent as the wValue field of the request. Prior to
this change, 1 was being combined with 3, not with 2 as it should be.
Change-Id: If36839d86a6eb3b5f8c26287019c92eb8726a3e1
diff --git a/system/dev/audio/usb-audio/audio-util.c b/system/dev/audio/usb-audio/audio-util.c
index 07a75b1..44bc5e3 100644
--- a/system/dev/audio/usb-audio/audio-util.c
+++ b/system/dev/audio/usb-audio/audio-util.c
@@ -44,31 +44,136 @@
return result;
}
+zx_status_t get_feature_unit_ch_count(const usb_audio_ac_feature_unit_desc* fu_desc,
+ uint8_t* ch_count_out) {
+ if (!fu_desc->bControlSize) {
+ printf("[USB_AUD] Invalid ControlSize (%u) while computing feature unit channel count\n",
+ fu_desc->bControlSize);
+ return ZX_ERR_INVALID_ARGS;
+ }
+
+ // In addition to the fields listed in the feature_unit_desc struct, there
+ // is and additional single byte field (iFeature) which comes after the
+ // variable length control bitmaps field. Account for this when we sanity
+ // check our length.
+ const uint8_t overhead = sizeof(*fu_desc) + 1;
+
+ if (((fu_desc->bLength < (overhead + fu_desc->bControlSize)) ||
+ ((fu_desc->bLength - overhead) % fu_desc->bControlSize))) {
+ printf("[USB_AUD] Invalid Length (%u) while computing feature unit channel count\n",
+ fu_desc->bLength);
+ return ZX_ERR_INVALID_ARGS;
+ }
+
+ *ch_count_out = (fu_desc->bLength - overhead) / fu_desc->bControlSize;
+ return ZX_OK;
+}
+
+void usb_audio_dump_feature_unit_caps(usb_protocol_t* usb,
+ uint8_t interface_number,
+ const usb_audio_ac_feature_unit_desc* fu_desc) {
+ printf("Feature unit dump for interface number %u\n", interface_number);
+ printf("Length : 0x%02x (%u)\n", fu_desc->bLength, fu_desc->bLength);
+ printf("DType : 0x%02x (%u)\n", fu_desc->bDescriptorType, fu_desc->bDescriptorType);
+ printf("DSubtype : 0x%02x (%u)\n", fu_desc->bDescriptorSubtype, fu_desc->bDescriptorSubtype);
+ printf("UnitID : 0x%02x (%u)\n", fu_desc->bUnitID, fu_desc->bUnitID);
+ printf("SrcID : 0x%02x (%u)\n", fu_desc->bSourceID, fu_desc->bSourceID);
+ printf("CtrlSz : 0x%02x (%u)\n", fu_desc->bControlSize, fu_desc->bControlSize);
+
+ uint8_t ch_count;
+ if (get_feature_unit_ch_count(fu_desc, &ch_count) != ZX_OK) {
+ return;
+ }
+
+ const uint8_t* bma = fu_desc->bmaControls;
+ for (uint8_t i = 0; i < ch_count; ++i) {
+ printf("CBma[%3u] : 0x", i);
+ for (uint8_t j = 0; j < fu_desc->bControlSize; ++j) {
+ printf("%02x", bma[fu_desc->bControlSize - j - 1]);
+ }
+ printf("\n");
+ bma += fu_desc->bControlSize;
+ }
+}
+
// volume is in 0 - 100 range
-zx_status_t usb_audio_set_volume(usb_protocol_t* usb, uint8_t interface_number, int fu_id,
+zx_status_t usb_audio_set_volume(usb_protocol_t* usb,
+ uint8_t interface_number,
+ const usb_audio_ac_feature_unit_desc* fu_desc,
int volume) {
- uint16_t volume_min;
- uint16_t volume_max;
zx_status_t status;
size_t out_length;
+ uint8_t ch_count;
- status = usb_control(usb, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- USB_AUDIO_GET_MIN, USB_AUDIO_VOLUME_CONTROL << 8 | interface_number,
- fu_id << 8, &volume_min, sizeof(volume_min), ZX_TIME_INFINITE,
- &out_length);
- if (status != ZX_OK || out_length != sizeof(volume_min)) goto out;
- status = usb_control(usb, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- USB_AUDIO_GET_MAX, USB_AUDIO_VOLUME_CONTROL << 8 | interface_number,
- fu_id << 8, &volume_max, sizeof(volume_max), ZX_TIME_INFINITE,
- &out_length);
- if (status != ZX_OK || status != sizeof(volume_min)) goto out;
- if (volume_min >= volume_max) return ZX_ERR_INTERNAL;
+ if ((volume < 0) || (volume > 100)) {
+ printf("[USB_AUD] Bad volume (%d)\n", volume);
+ status = ZX_ERR_INVALID_ARGS;
+ goto out;
+ }
- // TODO (voydanoff) - maybe this should be logarithmic?
- uint16_t volume16 = volume_min + ((volume_max - volume_min) * volume) / 100;
- status = usb_control(usb, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- USB_AUDIO_SET_CUR, USB_AUDIO_VOLUME_CONTROL << 8 | interface_number,
- fu_id << 8, &volume16, sizeof(volume16), ZX_TIME_INFINITE, NULL);
+ status = get_feature_unit_ch_count(fu_desc, &ch_count);
+ if (status != ZX_OK) {
+ printf("[USB_AUD] Failed to parse feature unit descriptor\n");
+ goto out;
+ }
+
+ const uint16_t unit_addr = (((uint16_t)fu_desc->bUnitID) << 8) | interface_number;
+ for (uint8_t ch = 0; ch < ch_count; ++ch) {
+ uint8_t caps_bma = fu_desc->bmaControls[ch * fu_desc->bControlSize];
+
+ if (caps_bma & USB_AUDIO_FU_BMA_MUTE) {
+ uint8_t val = (volume == 0) ? 1 : 0;
+ const uint16_t ctrl_addr = (USB_AUDIO_MUTE_CONTROL << 8) | ch;
+ status = usb_control(usb, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_AUDIO_SET_CUR, ctrl_addr, unit_addr,
+ &val, sizeof(val), ZX_TIME_INFINITE,
+ &out_length);
+ if ((status != ZX_OK) || (out_length != sizeof(val))) {
+ printf("[USB_AUD] Failed to set mute; IID %u FeatUnitID %u Ch %u\n",
+ interface_number, fu_desc->bUnitID, ch);
+ goto out;
+ }
+ }
+
+ if (caps_bma & USB_AUDIO_FU_BMA_VOLUME) {
+ int16_t min, max, val;
+ const uint16_t ctrl_addr = (USB_AUDIO_VOLUME_CONTROL << 8) | ch;
+
+ status = usb_control(usb, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_AUDIO_GET_MIN, ctrl_addr, unit_addr,
+ &min, sizeof(min), ZX_TIME_INFINITE,
+ &out_length);
+
+ if ((status != ZX_OK) || (out_length != sizeof(val))) {
+ printf("[USB_AUD] Failed to to fetch min vol; IID %u FeatUnitID %u Ch %u\n",
+ interface_number, fu_desc->bUnitID, ch);
+ goto out;
+ }
+
+ status = usb_control(usb, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_AUDIO_GET_MAX, ctrl_addr, unit_addr,
+ &max, sizeof(max), ZX_TIME_INFINITE,
+ &out_length);
+
+ if ((status != ZX_OK) || (out_length != sizeof(val))) {
+ printf("[USB_AUD] Failed to to fetch max vol; IID %u FeatUnitID %u Ch %u\n",
+ interface_number, fu_desc->bUnitID, ch);
+ goto out;
+ }
+
+ val = (int16_t)(((((int32_t)(max - min)) * volume) / 100) + min);
+
+ status = usb_control(usb, USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ USB_AUDIO_SET_CUR, ctrl_addr, unit_addr,
+ &val, sizeof(val), ZX_TIME_INFINITE,
+ &out_length);
+ if ((status != ZX_OK) || (out_length != sizeof(val))) {
+ printf("[USB_AUD] Failed to to set vol; IID %u FeatUnitID %u Ch %u\n",
+ interface_number, fu_desc->bUnitID, ch);
+ goto out;
+ }
+ }
+ }
out:
if (status == ZX_ERR_IO_REFUSED) {
diff --git a/system/dev/audio/usb-audio/usb-audio-stream.cpp b/system/dev/audio/usb-audio/usb-audio-stream.cpp
index 723155d..6202a42 100644
--- a/system/dev/audio/usb-audio/usb-audio-stream.cpp
+++ b/system/dev/audio/usb-audio/usb-audio-stream.cpp
@@ -241,6 +241,7 @@
supported_formats->push_back(range);
}
+ fixed_sample_rate_ = supported_formats->size() <= 1;
} else {
fbl::AllocChecker ac;
supported_formats->reserve(1, &ac);
@@ -254,6 +255,7 @@
range.flags = ASF_RANGE_FLAG_FPS_CONTINUOUS;
supported_formats->push_back(range);
+ fixed_sample_rate_ = false;
}
return ZX_OK;
@@ -576,14 +578,22 @@
}
}
- // Send the commands required to set up the new format.
+ // Send the commands required to set up the new format. Do not attempt to
+ // set the sample rate if the endpoint supports only one. In theory,
+ // devices should ignore this request, but in practice, some devices will
+ // refuse the command entirely, and we will get ZX_ERR_IO_REFUSED back from
+ // the bus driver.
//
// TODO(johngro): more work is needed if we are changing sample format or
// channel count. Right now, we only support the one format/count provided
// to us by the outer layer, but eventually we need to support them all.
- ZX_DEBUG_ASSERT(parent_ != nullptr);
- resp.result = usb_audio_set_sample_rate(&usb_, usb_ep_addr_, req.frames_per_second);
- if (resp.result != ZX_OK) goto finished;
+ if (!fixed_sample_rate_) {
+ ZX_DEBUG_ASSERT(parent_ != nullptr);
+ resp.result = usb_audio_set_sample_rate(&usb_, usb_ep_addr_, req.frames_per_second);
+ if (resp.result != ZX_OK) {
+ goto finished;
+ }
+ }
// Create a new ring buffer channel which can be used to move bulk data and
// bind it to us.
diff --git a/system/dev/audio/usb-audio/usb-audio-stream.h b/system/dev/audio/usb-audio/usb-audio-stream.h
index e31749c..237ae94 100644
--- a/system/dev/audio/usb-audio/usb-audio-stream.h
+++ b/system/dev/audio/usb-audio/usb-audio-stream.h
@@ -138,6 +138,7 @@
// descriptors present for a stream, not just a single format (with multiple
// sample rates).
fbl::Vector<audio_stream_format_range_t> supported_formats_;
+ bool fixed_sample_rate_ = false;
uint32_t frame_size_;
uint32_t iso_packet_rate_;
diff --git a/system/dev/audio/usb-audio/usb-audio.c b/system/dev/audio/usb-audio/usb-audio.c
index 77229cd..0851847 100644
--- a/system/dev/audio/usb-audio/usb-audio.c
+++ b/system/dev/audio/usb-audio/usb-audio.c
@@ -114,13 +114,13 @@
usb_audio_source_create(device, &usb, audio_source_index++, intf, endp,
format_desc);
}
- // this is a quick and dirty hack to set volume to 75%
+ // this is a quick and dirty hack to set volume to 100%
// otherwise, audio might default to 0%
// TODO - properly support getting and setting stream volumes via ioctls
feature_unit_node_t* fu_node;
list_for_every_entry(&fu_descs, fu_node, feature_unit_node_t, node) {
// this may fail, but we are taking shotgun approach here
- usb_audio_set_volume(&usb, intf->bInterfaceNumber, fu_node->desc->bUnitID, 75);
+ usb_audio_set_volume(&usb, intf->bInterfaceNumber, fu_node->desc, 100);
}
} else if (intf->bInterfaceSubClass == USB_SUBCLASS_MIDI_STREAMING &&
usb_ep_type(endp) == USB_ENDPOINT_BULK) {
@@ -177,6 +177,11 @@
if (fu_node) {
fu_node->desc = (usb_audio_ac_feature_unit_desc *)header;
list_add_tail(&fu_descs, &fu_node->node);
+#if TRACE
+ usb_audio_dump_feature_unit_caps(&usb,
+ intf->bInterfaceNumber,
+ fu_node->desc);
+#endif
}
break;
}
diff --git a/system/dev/audio/usb-audio/usb-audio.h b/system/dev/audio/usb-audio/usb-audio.h
index 115ed84..9e9547c 100644
--- a/system/dev/audio/usb-audio/usb-audio.h
+++ b/system/dev/audio/usb-audio/usb-audio.h
@@ -32,8 +32,14 @@
zx_status_t usb_audio_set_sample_rate(usb_protocol_t* usb, uint8_t ep_addr,
uint32_t sample_rate);
+void usb_audio_dump_feature_unit_caps(usb_protocol_t* usb,
+ uint8_t interface_number,
+ const usb_audio_ac_feature_unit_desc* fu_desc);
+
// volume is in 0 - 100 range
-zx_status_t usb_audio_set_volume(usb_protocol_t* usb, uint8_t interface_number, int fu_id,
+zx_status_t usb_audio_set_volume(usb_protocol_t* usb,
+ uint8_t interface_number,
+ const usb_audio_ac_feature_unit_desc* fu_desc,
int volume);
__END_CDECLS
diff --git a/system/public/zircon/hw/usb-audio.h b/system/public/zircon/hw/usb-audio.h
index a7b33ff..cd6d655 100644
--- a/system/public/zircon/hw/usb-audio.h
+++ b/system/public/zircon/hw/usb-audio.h
@@ -79,6 +79,18 @@
#define USB_AUDIO_BASS_BOOST_CONTROL 0x09
#define USB_AUDIO_LOUDNESS_CONTROL 0x0A
+// feature unit control support bitmasks
+#define USB_AUDIO_FU_BMA_MUTE (1u << 0u)
+#define USB_AUDIO_FU_BMA_VOLUME (1u << 1u)
+#define USB_AUDIO_FU_BMA_BASS (1u << 2u)
+#define USB_AUDIO_FU_BMA_MID (1u << 3u)
+#define USB_AUDIO_FU_BMA_TREBLE (1u << 4u)
+#define USB_AUDIO_FU_BMA_GRAPHIC_EQUALIZER (1u << 5u)
+#define USB_AUDIO_FU_BMA_AUTOMATIC_GAIN (1u << 6u)
+#define USB_AUDIO_FU_BMA_DELAY (1u << 7u)
+#define USB_AUDIO_FU_BMA_BASS_BOOST (1u << 8u)
+#define USB_AUDIO_FU_BMA_LOUDNESS (1u << 9u)
+
// up/down mix processing unit control selectors
#define USB_AUDIO_UD_ENABLE_CONTROL 0x01
#define USB_AUDIO_UD_MODE_SELECT_CONTROL 0x02
