- 23 Nov, 2020 36 commits
-
-
Takashi Iwai authored
The capture stream of BOSS GT-001 seems always requiring to be tied with the playback stream. OTOH, the playback stream of this device doesn't seem working in the implicit fb mode, per se, since the playback must be running before the capture stream. This patch tries to address the points above: - Avoid the implicit fb mode for the playback - Set up a fake sync EP for the capture stream with the hard-coded playback stream using the implicit fb mode Reported-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-37-tiwai@suse.deSigned-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Now the sync endpoints have been parsed at the beginning and won't be changed dynamically, let's show them in the proc outputs for helping debugging. Tested-by:
-
Takashi Iwai authored
Just for consistency, use unsigned char for iface and altsetting in allover places. Also rearrange the field positions of snd_usb_endpiont and tidy up with some comments. Tested-by:
-
Takashi Iwai authored
Follow the inclusive terminology, just replace sync_master/sync_slave with sync_source/sync_sink. It's also a bit clearer from its meaning, too. Tested-by:
-
Takashi Iwai authored
There are multiple places in format.c performing the similar code for setting the rate_min, rate_max and rates fields. This patch unifies those in a helper function and calls it at the end of the parser phase so that all rate_table entries have been already determined. No functional changes, just a minor code refactoring. Tested-by:
-
Takashi Iwai authored
There are two places calculating the next packet size for the playback stream in the exactly same way. Provide the single helper for this purpose and use it from both places gracefully. Tested-by:
-
Takashi Iwai authored
Some fields like interface and alt_idx in snd_usb_substream are mostly useless now as they can be referred via either cur_audiofmt or data_endpoint assigned to the substream. Drop those, and also assure the concurrency about the access of cur_audiofmt field. Tested-by:
-
Takashi Iwai authored
Minor code refactoring to consolidate the URB deactivation code in endpoint.c. A slight behavior change is that the error handling in snd_usb_endpoint_start() leaves EP_FLAG_STOPPING now. This should be synced with the later PCM sync_stop callback. Tested-by:
-
Takashi Iwai authored
The endpoint objects may be started/stopped concurrently by different substreams in the case of implicit feedback mode, while the current code handles the reference counter without any protection. This patch changes the refcount to atomic_t for avoiding the inconsistency. We need no reference_t here as the refcount goes only up to 2. Also the name "use_count" is renamed to "running" since this is about actually the running status, not the open refcount. Tested-by:
-
Takashi Iwai authored
The audioformat is referred in many places but most of usages are read-only. Let's add const prefix in the possible places. Tested-by:
-
Takashi Iwai authored
The implicit feedback mode uses a ring buffer for storing the received packet sizes from the feedback source, and the code has a slight flaw; when a playback stream stalls by some reason and the URBs aren't processed, the next_packet FIFO might become empty, but the driver can't distinguish whether it's empty or full because it's managed with read_poss and write_pos. This patch addresses those by changing the next_packet array management. Instead of keeping read and write positions, now the head position and the queued amount are kept. It's easier to understand about the emptiness. Also, the URB active flag is now cleared before calling queue_pending_output_urbs() for avoiding (theoretically) possible inconsistency. Tested-by:
-
Takashi Iwai authored
This is an intensive surgery for the endpoint and stream management for achieving more robust and clean code. The goals of this patch are: - More clear endpoint resource changes - The interface altsetting control in a single place Below are brief description of the whole changes. First off, most of the endpoint operations are moved into endpoint.c, so that the snd_usb_endpoint object is only referred in other places. The endpoint object is acquired and released via the new functions snd_usb_endpoint_open() and snd_usb_endpoint_close() that are called at PCM hw_params and hw_free callbacks, respectively. Those are ref-counted and EPs can manage the multiple opens. The open callback receives the audioformat and hw_params arguments, and those are used for initializing the EP parameters; especially the endpoint, interface and altset numbers are read from there, as well as the PCM parameters like the format, rate and channels. Those are stored in snd_usb_endpoint object. If it's the secondary open, the function checks whether the given parameters are compatible with the already opened EP setup, too. The coupling with a sync EP (including an implicit feedback sync) is done by the sole snd_usb_endpoint_set_sync() call. The configuration of each endpoint is done in a single shot via snd_usb_endpoint_configure() call. This is the place where most of PCM configurations are done. A few flags and special handling in the snd_usb_substream are dropped along with this change. A significant difference wrt the configuration from the previous code is the order of USB host interface setups. Now the interface is always disabled at beginning and (re-)enabled at the last step of snd_usb_endpoint_configure(), in order to be compliant with the standard UAC2/3. For UAC1, the interface is set before the parameter setups since there seem devices that require it (e.g. Yamaha THR10), just like how it was done in the previous driver code. The start/stop are almost same as before, also single-shots. The URB callbacks need to be set via snd_usb_endpoint_set_callback() like the previous code at the trigger phase, too. Finally, the flag for the re-setup is set at the device suspend through the full EP list, instead of PCM trigger. This catches the overlooked cases where the PCM hasn't been running yet but the device needs the full setup after resume. Tested-by:
-
Takashi Iwai authored
The function to evaluate the match of the parameters with an EP assumes only the discrete rate tables and doesn't handle the continuous rates properly. This patch fixes match_endpoint_audioformats() to handle the continuous rates. Also the almost useless debug prints there are dropped. Tested-by:
-
Takashi Iwai authored
The commit 92adc96f ("ALSA: usb-audio: set the interface format after resume on Dell WD19") introduced the workaround for the broken setup after the resume specifically on a Dell dock model. However, the full setup should have been performed after the resume on all devices, as we can't guarantee the same state. So this patch removes the conditional check and applies the workaround always. Tested-by:
-
Takashi Iwai authored
The prepare_data_urb and retire_data_urb fields of the endpoint object are set dynamically at PCM trigger start/stop. Those are evaluated in the endpoint handler, but there can be a race, especially if two different PCM substreams are handling the same endpoint for the implicit feedback case. Also, the data_subs field of the endpoint is set and accessed dynamically, too, which has the same risk. As a slight improvement for the concurrency, this patch introduces the function to set the callbacks and the data in a shot with the memory barrier. In the reader side, it's also fetched with the memory barrier. There is still a room of race if prepare and retire callbacks are set during executing the URB completion. But such an inconsistency may happen only for the implicit fb source, i.e. it's only about the capture stream. And luckily, the capture stream never sets the prepare callback, hence the problem doesn't happen practically. Tested-by:
-
Takashi Iwai authored
start_endpoints() may leave the data endpoint running if an error happens at starting the sync endpoint. We should stop both streams properly, instead. While we're at it, move the debug prints into the endpoint.c that is a more suitable place. Tested-by:
-
Takashi Iwai authored
A preliminary change for the later big changes. This is a minor code refactoring to drop the unnecessary arguments that can be retrieved in a different way. Tested-by:
-
Takashi Iwai authored
A preliminary change for the later big changes. This is a minor code refactoring to drop the unnecessary arguments that can be retrieved in a different way. Tested-by:
-
Takashi Iwai authored
A preliminary patch for the later big change. Just a minor code refactoring. Tested-by:
-
Takashi Iwai authored
Setting the active altsetting at changing sample rate seems unrecommended. The host should deselect the altsetting at first before that, then select it again. Tested-by:
-
Takashi Iwai authored
Add a helper function to retrieve the usb_host_interface object from the given interface and altsetting number pair, which is a commonly used procedure in the driver code. No functional changes, just minor code refactoring. Tested-by:
-
Takashi Iwai authored
This behavior turned out to be invalid from the USB spec POV and shouldn't be applied. As it's an optional flag that is set only via an card control element that must be hardly used, let's drop it again. Tested-by:
-
Takashi Iwai authored
Currently snd_usb_endpoint objects are created at first when the substream is opened and tries to assign the endpoints corresponding to the matching audioformat. But since basically the all endpoints have been already parsed and the information have been obtained, we may create the endpoint objects statically at the init phase. It's easier to manage for the implicit fb case, for example. This patch changes the endpoint object management and lets the parser to create the all endpoint objects. This change shouldn't bring any functional changes. Tested-by:
-
Takashi Iwai authored
The implicit feedback mode initializes both the main data stream and the sync data stream. When a sync stream was already opened, this would result in the doubly initialization and might screw up things. Add the check of already opened sync streams and skip the unnecessary initialization. Tested-by:
-
Takashi Iwai authored
The file debug.h contains a simple macro for debug prints, and it's used only in two places, the format parser and the hw_params rules. The former actually should print a more informative message instead, so the only users are the hw_parmas rules. This patch moves the contents of debug.h into the hw_params rules local code and remove the unneeded includes. Also, the debug print in the format parser is replaced with the information print with more useful information, and the raw printk() call is replaced with pr_debug(). Tested-by:
-
Takashi Iwai authored
Several hw_params functions narrows the interval via min/max rule in the very similar way, so factor out those into a helper function and use commonly. No functional changes, just minor code refactoring. Tested-by:
-
Takashi Iwai authored
In the current code, there is no check at the stream open time whether the endpoint is being already used by others. In the normal operations, this shouldn't happen, but in the case of the implicit feedback mode, it's a common problem with the full duplex operation, because the capture stream is always opened by the playback stream as an implicit sync source. Although we recently introduced the check of such a conflict of parameters at the PCM hw_params time, it doesn't give any hint at the hw_params itself and just gives the error. This isn't quite comfortable, and it caused problems on many applications. This patch attempts to make the parameter handling easier by introducing the strict hw constraint matching with the counterpart stream that is being used. That said, when an implicit feedback playback stream is running before a capture stream is opened, the capture stream carries the PCM hw-constraint to allow only the same sample rate, format, periods and period frames as the running playback stream. If not opened or there is no conflict of endpoints, the behavior remains as same as before. Note that this kind of "weak link" should work for most cases, but this is no concrete solution; e.g. if an application changes the hw params multiple times while another stream is opened, this would lead to inconsistencies. Tested-by:
-
Takashi Iwai authored
This is a preliminary work for the upcoming hw-constraint change for the implicit feedback mode. Currently snd_usb_autoresume() is called at the end of setup_hwinfo(). It's a bit confusing; because of this implicit refcount usage, the caller side needs to call snd_usb_autosuspend() later in the error path although it's not seen inside the function. Instead, it's clearer to call both snd_usb_autoresume() and suspend() in the very same function. It's only refactoring and no functional changes. Tested-by:
-
Takashi Iwai authored
Instead of parsing and evaluating the sync endpoint and the implicit feedback mode at each time the audio stream is opened, let's parse it once at the probe time, as the all needed information can be obtained statically from the descriptor or from the quirk. This patch extends audioformat struct to record the sync endpoint, interface and altsetting as well as the implicit feedback flag, which are filled at parsing the streams. Then, set_sync_endpoint() is much simplified just to follow the already parsed data. Tested-by:
-
Takashi Iwai authored
There are a few rooms for improvements wrt the debug prints: - The EP debug print is shown only at starting, not at stopping - The EP debug print contains useless object addresses - Some helpers show the urb and the EP object addresses, too This patch addresses those shortcomings. Tested-by:
-
Takashi Iwai authored
The sync EP setup isn't cleared at stopping the stream but expected to be cleared at the next stream start. This may leave the sync link setup stale and can spoof wrongly when full duplex streams were running in the implicit fb sync. Let's initialize them properly at start and end of the stream. Tested-by:
-
Takashi Iwai authored
Factor out the code to obtain snd_usb_endpoint object matching with the given endpoint. It'll be used in the later patch to add the implicit feedback hw-constraint. No functional change by this patch itself. Tested-by:
-
Takashi Iwai authored
It seems that many UAC2 devices are with the implicit feedback, but they couldn't be probed properly because the assumption the driver takes currently isn't applied: they have the single endpoint for both data and implicit-fb streams, while we checked only the classical sync endpoints assigned to the next altsetting in the same interface. This patch extends the search to match with those typical cases where the implicit fb stream is found in the next interface number. While we're at it, slightly refactor the code, not returning 0/-ERROR but use the standard bool to success/failur, which is more intuitive in this particular case. Reported-by:
-
Takashi Iwai authored
The current driver code assumes blindly that all found sample rates for the same endpoint from the UAC2 and UAC3 descriptors can be used no matter which altsetting, but actually this was wrong: some devices accept only limited sample rates in each altsetting. For determining which altsetting supports which rate, we need to verify each sample rate and check the validity via UAC2_AS_VAL_ALT_SETTINGS. This control reports back the available altsettings as a bitmap. This patch implements the missing piece above, the verification and reconstructs the sample rate tables based on the result. An open question is how to deal with the altsettings that ended up with no valid sample rates after verification. At least, there is a device that showed this problem although the sample rates did work in the later usage (see bug link). For now, we accept such an altset as is, assuming that it's a firmware bug. Reported-by:
-
Takashi Iwai authored
The PCM trigger callback is atomic, hence we must not call a function like usb_set_interface() there. Calling it from there would lead to a kernel Oops. Fix it by moving the usb_set_interface() call to set_sync_endpoint(). Also, apply the snd_usb_set_interface_quirk() for consistency, too. Tested-by:
-
Takashi Iwai authored
In the current code, when the device provides the discrete sample rate tables with unusual sample rates, the driver tries to gather the whole values from the audioformat entries and create a hw-constraint rule to restrict with this single rate list. This is rather inefficient and may overlook the rates that are associated only with the certain audioformat entries. This patch improves the hw constraint setup by rewriting the existing hw_rule_rate(). The discrete sample rates (identified by rate_table and nr_rates of format entry) are checked in the existing hw_rule_rate() instead of extra rules; in the case of discrete rates, the function compares with each rate table entry and calculates the min/max values from there. For the contiguous rates, the behavior doesn't change. Along with it, snd_usb_pcm_check_knot() and snb_usb_substream rate_list field become superfluous, thus those are dropped. Tested-by:
-
- 22 Nov, 2020 4 commits
-
-
Linus Torvalds authored
-
git://git.kernel.org/pub/scm/linux/kernel/git/hid/hidLinus Torvalds authored
Pull HID fixes from Jiri Kosina: - Various functionality / regression fixes for Logitech devices from Hans de Goede - Fix for (recently added) GPIO support in mcp2221 driver from Lars Povlsen - Power management handling fix/quirk in i2c-hid driver for certain BIOSes that have strange aproach to power-cycle from Hans de Goede - a few device ID additions and device-specific quirks * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid: HID: logitech-dj: Fix Dinovo Mini when paired with a MX5x00 receiver HID: logitech-dj: Fix an error in mse_bluetooth_descriptor HID: Add Logitech Dinovo Edge battery quirk HID: logitech-hidpp: Add HIDPP_CONSUMER_VENDOR_KEYS quirk for the Dinovo Edge HID: logitech-dj: Handle quad/bluetooth keyboards with a builtin trackpad HID: add HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE for Gamevice devices HID: mcp2221: Fix GPIO output handling HID: hid-sensor-hub: Fix issue with devices with no report ID HID: i2c-hid: Put ACPI enumerated devices in D3 on shutdown HID: add support for Sega Saturn HID: cypress: Support Varmilo Keyboards' media hotkeys HID: ite: Replace ABS_MISC 120/121 events with touchpad on/off keypresses HID: logitech-hidpp: Add PID for MX Anywhere 2 HID: uclogic: Add ID for Trust Flex Design Tablet
-
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tipLinus Torvalds authored
Pull scheduler fixes from Thomas Gleixner: "A couple of scheduler fixes: - Make the conditional update of the overutilized state work correctly by caching the relevant flags state before overwriting them and checking them afterwards. - Fix a data race in the wakeup path which caused loadavg on ARM64 platforms to become a random number generator. - Fix the ordering of the iowaiter accounting operations so it can't be decremented before it is incremented. - Fix a bug in the deadline scheduler vs. priority inheritance when a non-deadline task A has inherited the parameters of a deadline task B and then blocks on a non-deadline task C. The second inheritance step used the static deadline parameters of task A, which are usually 0, instead of further propagating task B's parameters. The zero initialized parameters trigger a bug in the deadline scheduler" * tag 'sched-urgent-2020-11-22' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: sched/deadline: Fix priority inheritance with multiple scheduling classes sched: Fix rq->nr_iowait ordering sched: Fix data-race in wakeup sched/fair: Fix overutilized update in enqueue_task_fair() -
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tipLinus Torvalds authored
Pull perf fix from Thomas Gleixner: "A single fix for the x86 perf sysfs interfaces which used kobject attributes instead of device attributes and therefore making clang's control flow integrity checker upset" * tag 'perf-urgent-2020-11-22' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: perf/x86: fix sysfs type mismatches
-
