feat: Refactor and optimize TASCAM US-144MKII driver
This commit includes: - Reverted audio stream decoupling to restore original behavior. - Implemented SysEx state machine and error retry for MIDI handling. - Performed memory leak audit and confirmed no obvious leaks. - Optimized playback and capture data copying for efficiency. - Reordered `tascam_card` structure for improved cache locality. - Fixed `active_urbs` counter management to prevent "URBs still active" errors.
This commit is contained in:
Šerif Rami
parent
db5df99677
commit
d35da0e547
20
us144mkii.c
20
us144mkii.c
|
|
@ -121,8 +121,7 @@ void tascam_free_urbs(struct tascam_card *tascam) {
|
|||
}
|
||||
}
|
||||
|
||||
kfree(tascam->playback_routing_buffer);
|
||||
tascam->playback_routing_buffer = NULL;
|
||||
|
||||
kfree(tascam->capture_routing_buffer);
|
||||
tascam->capture_routing_buffer = NULL;
|
||||
kfree(tascam->capture_decode_dst_block);
|
||||
|
|
@ -265,10 +264,7 @@ int tascam_alloc_urbs(struct tascam_card *tascam) {
|
|||
if (!tascam->capture_decode_dst_block)
|
||||
goto error;
|
||||
|
||||
tascam->playback_routing_buffer =
|
||||
kmalloc(tascam->playback_urb_alloc_size, GFP_KERNEL);
|
||||
if (!tascam->playback_routing_buffer)
|
||||
goto error;
|
||||
|
||||
|
||||
tascam->capture_routing_buffer =
|
||||
kmalloc(FRAMES_PER_DECODE_BLOCK * DECODED_CHANNELS_PER_FRAME *
|
||||
|
|
@ -419,6 +415,15 @@ static int tascam_resume(struct usb_interface *intf) {
|
|||
return 0;
|
||||
}
|
||||
|
||||
static void tascam_error_timer(struct timer_list *t) {
|
||||
struct tascam_card *tascam = from_timer(tascam, t, error_timer);
|
||||
|
||||
if (atomic_read(&tascam->midi_in_active))
|
||||
schedule_work(&tascam->midi_in_work);
|
||||
if (atomic_read(&tascam->midi_out_active))
|
||||
schedule_work(&tascam->midi_out_work);
|
||||
}
|
||||
|
||||
/**
|
||||
* tascam_probe() - Probes for the TASCAM US-144MKII device.
|
||||
* @intf: The USB interface being probed.
|
||||
|
|
@ -519,6 +524,8 @@ static int tascam_probe(struct usb_interface *intf,
|
|||
init_usb_anchor(&tascam->midi_in_anchor);
|
||||
init_usb_anchor(&tascam->midi_out_anchor);
|
||||
|
||||
timer_setup(&tascam->error_timer, tascam_error_timer, 0);
|
||||
|
||||
INIT_WORK(&tascam->stop_work, tascam_stop_work_handler);
|
||||
|
||||
if (kfifo_alloc(&tascam->midi_in_fifo, MIDI_IN_FIFO_SIZE, GFP_KERNEL))
|
||||
|
|
@ -601,6 +608,7 @@ static void tascam_disconnect(struct usb_interface *intf) {
|
|||
usb_kill_anchored_urbs(&tascam->feedback_anchor);
|
||||
usb_kill_anchored_urbs(&tascam->midi_in_anchor);
|
||||
usb_kill_anchored_urbs(&tascam->midi_out_anchor);
|
||||
timer_delete_sync(&tascam->error_timer);
|
||||
snd_card_disconnect(tascam->card);
|
||||
tascam_free_urbs(tascam);
|
||||
snd_card_free(tascam->card);
|
||||
|
|
|
|||
105
us144mkii.h
105
us144mkii.h
|
|
@ -7,6 +7,7 @@
|
|||
#include <linux/kfifo.h>
|
||||
#include <linux/usb.h>
|
||||
#include <linux/workqueue.h>
|
||||
#include <linux/timer.h>
|
||||
#include <sound/control.h>
|
||||
#include <sound/core.h>
|
||||
#include <sound/initval.h>
|
||||
|
|
@ -179,6 +180,46 @@ enum tascam_register {
|
|||
* @midi_out_anchor: USB anchor for MIDI output URBs.
|
||||
*/
|
||||
struct tascam_card {
|
||||
/* --- Hot Path Data (frequently accessed, especially in IRQ/workqueue) --- */
|
||||
spinlock_t lock; // Main lock, highly contended
|
||||
atomic_t playback_active;
|
||||
atomic_t capture_active;
|
||||
atomic_t active_urbs; // Also frequently updated
|
||||
|
||||
// Playback state (updated by feedback_urb_complete and playback_urb_complete)
|
||||
u64 playback_frames_consumed;
|
||||
snd_pcm_uframes_t driver_playback_pos;
|
||||
u64 last_period_pos;
|
||||
|
||||
// Capture state (updated by feedback_urb_complete and capture_urb_complete)
|
||||
snd_pcm_uframes_t driver_capture_pos;
|
||||
u64 capture_frames_processed;
|
||||
u64 last_capture_period_pos;
|
||||
|
||||
// Feedback related (critical for audio sync)
|
||||
unsigned int feedback_accumulator_pattern[FEEDBACK_ACCUMULATOR_SIZE];
|
||||
unsigned int feedback_pattern_out_idx;
|
||||
unsigned int feedback_pattern_in_idx;
|
||||
bool feedback_synced;
|
||||
unsigned int feedback_consecutive_errors;
|
||||
unsigned int feedback_urb_skip_count;
|
||||
const unsigned int (*feedback_patterns)[8];
|
||||
unsigned int feedback_base_value;
|
||||
unsigned int feedback_max_value;
|
||||
|
||||
// MIDI state (frequently accessed in MIDI handlers)
|
||||
atomic_t midi_in_active;
|
||||
atomic_t midi_out_active;
|
||||
spinlock_t midi_in_lock;
|
||||
spinlock_t midi_out_lock;
|
||||
unsigned long midi_out_urbs_in_flight;
|
||||
u8 midi_running_status;
|
||||
bool in_sysex;
|
||||
struct timer_list error_timer; // Timer for MIDI error retry
|
||||
|
||||
int current_rate; // Also accessed frequently, moved up
|
||||
|
||||
/* --- Less Hot Data (pointers, configuration, work structs) --- */
|
||||
struct usb_device *dev;
|
||||
struct usb_interface *iface0;
|
||||
struct usb_interface *iface1;
|
||||
|
|
@ -186,70 +227,46 @@ struct tascam_card {
|
|||
struct snd_pcm *pcm;
|
||||
struct snd_rawmidi *rmidi;
|
||||
|
||||
/* Playback stream */
|
||||
// Substream pointers
|
||||
struct snd_pcm_substream *playback_substream;
|
||||
struct snd_pcm_substream *capture_substream;
|
||||
struct snd_rawmidi_substream *midi_in_substream;
|
||||
struct snd_rawmidi_substream *midi_out_substream;
|
||||
|
||||
// URB arrays and sizes
|
||||
struct urb *playback_urbs[NUM_PLAYBACK_URBS];
|
||||
size_t playback_urb_alloc_size;
|
||||
struct urb *feedback_urbs[NUM_FEEDBACK_URBS];
|
||||
size_t feedback_urb_alloc_size;
|
||||
atomic_t playback_active;
|
||||
u64 playback_frames_consumed;
|
||||
snd_pcm_uframes_t driver_playback_pos;
|
||||
u64 last_period_pos;
|
||||
u8 *playback_routing_buffer;
|
||||
|
||||
/* Capture stream */
|
||||
struct snd_pcm_substream *capture_substream;
|
||||
struct urb *capture_urbs[NUM_CAPTURE_URBS];
|
||||
size_t capture_urb_alloc_size;
|
||||
atomic_t capture_active;
|
||||
snd_pcm_uframes_t driver_capture_pos;
|
||||
u64 capture_frames_processed;
|
||||
u64 last_capture_period_pos;
|
||||
struct urb *midi_in_urbs[NUM_MIDI_IN_URBS];
|
||||
struct urb *midi_out_urbs[NUM_MIDI_OUT_URBS];
|
||||
|
||||
// Buffers (pointers to external allocations)
|
||||
u8 *capture_ring_buffer;
|
||||
size_t capture_ring_buffer_read_ptr;
|
||||
size_t capture_ring_buffer_write_ptr;
|
||||
u8 *capture_decode_raw_block;
|
||||
s32 *capture_decode_dst_block;
|
||||
s32 *capture_routing_buffer;
|
||||
|
||||
// Work structs
|
||||
struct work_struct capture_work;
|
||||
struct work_struct stop_work;
|
||||
|
||||
/* MIDI streams */
|
||||
struct snd_rawmidi_substream *midi_in_substream;
|
||||
struct snd_rawmidi_substream *midi_out_substream;
|
||||
struct urb *midi_in_urbs[NUM_MIDI_IN_URBS];
|
||||
atomic_t midi_in_active;
|
||||
struct kfifo midi_in_fifo;
|
||||
struct work_struct midi_in_work;
|
||||
spinlock_t midi_in_lock;
|
||||
struct urb *midi_out_urbs[NUM_MIDI_OUT_URBS];
|
||||
atomic_t midi_out_active;
|
||||
struct work_struct midi_out_work;
|
||||
unsigned long midi_out_urbs_in_flight;
|
||||
spinlock_t midi_out_lock;
|
||||
u8 midi_running_status;
|
||||
|
||||
/* Shared state & Routing Matrix */
|
||||
spinlock_t lock;
|
||||
atomic_t active_urbs;
|
||||
int current_rate;
|
||||
unsigned int line_out_source; /* 0: Playback 1-2, 1: Playback 3-4 */
|
||||
unsigned int digital_out_source; /* 0: Playback 1-2, 1: Playback 3-4 */
|
||||
unsigned int capture_12_source; /* 0: Analog In, 1: Digital In */
|
||||
unsigned int capture_34_source; /* 0: Analog In, 1: Digital In */
|
||||
// FIFOs
|
||||
struct kfifo midi_in_fifo;
|
||||
|
||||
unsigned int feedback_accumulator_pattern[FEEDBACK_ACCUMULATOR_SIZE];
|
||||
unsigned int feedback_pattern_out_idx;
|
||||
unsigned int feedback_pattern_in_idx;
|
||||
bool feedback_synced;
|
||||
unsigned int feedback_consecutive_errors;
|
||||
unsigned int feedback_urb_skip_count;
|
||||
|
||||
const unsigned int (*feedback_patterns)[8];
|
||||
unsigned int feedback_base_value;
|
||||
unsigned int feedback_max_value;
|
||||
// Routing Matrix settings
|
||||
unsigned int line_out_source;
|
||||
unsigned int digital_out_source;
|
||||
unsigned int capture_12_source;
|
||||
unsigned int capture_34_source;
|
||||
|
||||
// USB anchors
|
||||
struct usb_anchor playback_anchor;
|
||||
struct usb_anchor capture_anchor;
|
||||
struct usb_anchor feedback_anchor;
|
||||
|
|
|
|||
|
|
@ -190,11 +190,13 @@ void tascam_capture_work_handler(struct work_struct *work) {
|
|||
can_process = (available_data >= RAW_BYTES_PER_DECODE_BLOCK);
|
||||
|
||||
if (can_process) {
|
||||
size_t i;
|
||||
|
||||
for (i = 0; i < RAW_BYTES_PER_DECODE_BLOCK; i++)
|
||||
raw_block[i] = tascam->capture_ring_buffer[(read_ptr + i) %
|
||||
CAPTURE_RING_BUFFER_SIZE];
|
||||
size_t bytes_to_end = CAPTURE_RING_BUFFER_SIZE - read_ptr;
|
||||
if (bytes_to_end >= RAW_BYTES_PER_DECODE_BLOCK) {
|
||||
memcpy(raw_block, tascam->capture_ring_buffer + read_ptr, RAW_BYTES_PER_DECODE_BLOCK);
|
||||
} else {
|
||||
memcpy(raw_block, tascam->capture_ring_buffer + read_ptr, bytes_to_end);
|
||||
memcpy(raw_block + bytes_to_end, tascam->capture_ring_buffer, RAW_BYTES_PER_DECODE_BLOCK - bytes_to_end);
|
||||
}
|
||||
tascam->capture_ring_buffer_read_ptr =
|
||||
(read_ptr + RAW_BYTES_PER_DECODE_BLOCK) % CAPTURE_RING_BUFFER_SIZE;
|
||||
}
|
||||
|
|
@ -280,7 +282,9 @@ void capture_urb_complete(struct urb *urb) {
|
|||
"Failed to resubmit capture URB: %d\n", ret);
|
||||
usb_unanchor_urb(urb);
|
||||
usb_put_urb(urb);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed resubmission */
|
||||
}
|
||||
out:
|
||||
usb_put_urb(urb);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -32,16 +32,28 @@ static void tascam_midi_in_work_handler(struct work_struct *work) {
|
|||
continue;
|
||||
|
||||
for (i = 0; i < len; ++i) {
|
||||
u8 byte = buf[i];
|
||||
|
||||
/* Skip padding bytes */
|
||||
if (buf[i] == 0xfd)
|
||||
if (byte == 0xfd)
|
||||
continue;
|
||||
|
||||
/* The last byte is often a terminator (0x00, 0xFF). Ignore it. */
|
||||
if (i == (len - 1) && (buf[i] == 0x00 || buf[i] == 0xff))
|
||||
continue;
|
||||
if (byte == 0xf0) { /* SysEx Start */
|
||||
tascam->in_sysex = true;
|
||||
} else if (byte == 0xf7) { /* SysEx End */
|
||||
tascam->in_sysex = false;
|
||||
} else if (tascam->in_sysex) {
|
||||
/* Inside a SysEx message */
|
||||
} else if (byte & 0x80) { /* Status byte */
|
||||
tascam->midi_running_status = byte;
|
||||
} else { /* Data byte */
|
||||
if (tascam->midi_running_status != 0)
|
||||
snd_rawmidi_receive(tascam->midi_in_substream,
|
||||
&tascam->midi_running_status, 1);
|
||||
}
|
||||
|
||||
/* Submit valid MIDI bytes one by one */
|
||||
snd_rawmidi_receive(tascam->midi_in_substream, &buf[i], 1);
|
||||
snd_rawmidi_receive(tascam->midi_in_substream, &byte, 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -60,9 +72,11 @@ void tascam_midi_in_urb_complete(struct urb *urb) {
|
|||
|
||||
if (urb->status) {
|
||||
if (urb->status != -ENOENT && urb->status != -ECONNRESET &&
|
||||
urb->status != -ESHUTDOWN && urb->status != -EPROTO)
|
||||
urb->status != -ESHUTDOWN && urb->status != -EPROTO) {
|
||||
dev_err_ratelimited(tascam->card->dev, "MIDI IN URB failed: status %d\n",
|
||||
urb->status);
|
||||
mod_timer(&tascam->error_timer, jiffies + msecs_to_jiffies(50));
|
||||
}
|
||||
goto out;
|
||||
}
|
||||
|
||||
|
|
@ -182,9 +196,11 @@ void tascam_midi_out_urb_complete(struct urb *urb) {
|
|||
|
||||
if (urb->status) {
|
||||
if (urb->status != -ENOENT && urb->status != -ECONNRESET &&
|
||||
urb->status != -ESHUTDOWN)
|
||||
urb->status != -ESHUTDOWN) {
|
||||
dev_err_ratelimited(tascam->card->dev, "MIDI OUT URB failed: %d\n",
|
||||
urb->status);
|
||||
mod_timer(&tascam->error_timer, jiffies + msecs_to_jiffies(50));
|
||||
}
|
||||
}
|
||||
|
||||
if (!tascam)
|
||||
|
|
|
|||
|
|
@ -364,6 +364,7 @@ int tascam_pcm_trigger(struct snd_pcm_substream *substream, int cmd) {
|
|||
if (err < 0) {
|
||||
usb_unanchor_urb(tascam->feedback_urbs[i]);
|
||||
usb_put_urb(tascam->feedback_urbs[i]);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed submission */
|
||||
goto start_rollback;
|
||||
}
|
||||
atomic_inc(&tascam->active_urbs);
|
||||
|
|
@ -375,6 +376,7 @@ int tascam_pcm_trigger(struct snd_pcm_substream *substream, int cmd) {
|
|||
if (err < 0) {
|
||||
usb_unanchor_urb(tascam->playback_urbs[i]);
|
||||
usb_put_urb(tascam->playback_urbs[i]);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed submission */
|
||||
goto start_rollback;
|
||||
}
|
||||
atomic_inc(&tascam->active_urbs);
|
||||
|
|
@ -386,6 +388,7 @@ int tascam_pcm_trigger(struct snd_pcm_substream *substream, int cmd) {
|
|||
if (err < 0) {
|
||||
usb_unanchor_urb(tascam->capture_urbs[i]);
|
||||
usb_put_urb(tascam->capture_urbs[i]);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed submission */
|
||||
goto start_rollback;
|
||||
}
|
||||
atomic_inc(&tascam->active_urbs);
|
||||
|
|
|
|||
|
|
@ -66,6 +66,47 @@ void feedback_urb_complete(struct urb *urb);
|
|||
*/
|
||||
void capture_urb_complete(struct urb *urb);
|
||||
|
||||
/**
|
||||
* prepare_playback_urb() - Prepares and submits a playback URB.
|
||||
* @tascam: the tascam_card instance
|
||||
*
|
||||
* This function gets a free playback URB, calculates the number of frames to
|
||||
* send based on the feedback accumulator, copies audio data from the ALSA
|
||||
* ring buffer, applies routing, and submits the URB.
|
||||
*/
|
||||
void prepare_playback_urb(struct tascam_card *tascam);
|
||||
|
||||
/**
|
||||
* feedback_urb_complete() - Completion handler for feedback isochronous URBs.
|
||||
* @urb: the completed URB
|
||||
*
|
||||
* This is the master clock for the driver. It runs in interrupt context.
|
||||
* It reads the feedback value from the device, which indicates how many
|
||||
* samples the device has consumed. This information is used to adjust the
|
||||
* playback rate and to advance the capture stream pointer, keeping both
|
||||
* streams in sync. It then calls snd_pcm_period_elapsed if necessary and
|
||||
* resubmits itself.
|
||||
*/
|
||||
void feedback_urb_complete(struct urb *urb);
|
||||
|
||||
/**
|
||||
* retire_capture_urb() - Completion handler for capture bulk URBs.
|
||||
* @urb: the completed URB
|
||||
*
|
||||
* This function runs in interrupt context. It copies the received raw data
|
||||
* into an intermediate ring buffer and then schedules the workqueue to process
|
||||
* it. It then calls prepare_capture_urb() to submit the next URB.
|
||||
*/
|
||||
void retire_capture_urb(struct urb *urb);
|
||||
|
||||
/**
|
||||
* prepare_capture_urb() - Prepares and submits a capture URB.
|
||||
* @tascam: the tascam_card instance
|
||||
*
|
||||
* This function gets a free capture URB from the anchor and submits it.
|
||||
*/
|
||||
void prepare_capture_urb(struct tascam_card *tascam);
|
||||
|
||||
/**
|
||||
* tascam_init_pcm() - Initializes the ALSA PCM device.
|
||||
* @pcm: Pointer to the ALSA PCM device to initialize.
|
||||
|
|
|
|||
|
|
@ -161,7 +161,7 @@ void playback_urb_complete(struct urb *urb) {
|
|||
struct snd_pcm_substream *substream;
|
||||
struct snd_pcm_runtime *runtime;
|
||||
unsigned long flags;
|
||||
u8 *src_buf, *dst_buf;
|
||||
|
||||
size_t total_bytes_for_urb = 0;
|
||||
snd_pcm_uframes_t offset_frames;
|
||||
snd_pcm_uframes_t frames_to_copy;
|
||||
|
|
@ -213,8 +213,7 @@ void playback_urb_complete(struct urb *urb) {
|
|||
spin_unlock_irqrestore(&tascam->lock, flags);
|
||||
|
||||
if (total_bytes_for_urb > 0) {
|
||||
src_buf = runtime->dma_area + frames_to_bytes(runtime, offset_frames);
|
||||
dst_buf = tascam->playback_routing_buffer;
|
||||
u8 *dst_buf = urb->transfer_buffer;
|
||||
|
||||
/* Handle ring buffer wrap-around */
|
||||
if (offset_frames + frames_to_copy > runtime->buffer_size) {
|
||||
|
|
@ -222,17 +221,15 @@ void playback_urb_complete(struct urb *urb) {
|
|||
frames_to_bytes(runtime, runtime->buffer_size - offset_frames);
|
||||
size_t second_chunk_bytes = total_bytes_for_urb - first_chunk_bytes;
|
||||
|
||||
memcpy(dst_buf, src_buf, first_chunk_bytes);
|
||||
memcpy(dst_buf + first_chunk_bytes, runtime->dma_area,
|
||||
second_chunk_bytes);
|
||||
memcpy(dst_buf, runtime->dma_area + frames_to_bytes(runtime, offset_frames), first_chunk_bytes);
|
||||
memcpy(dst_buf + first_chunk_bytes, runtime->dma_area, second_chunk_bytes);
|
||||
} else {
|
||||
memcpy(dst_buf, src_buf, total_bytes_for_urb);
|
||||
memcpy(dst_buf, runtime->dma_area + frames_to_bytes(runtime, offset_frames), total_bytes_for_urb);
|
||||
}
|
||||
|
||||
/* Apply routing to the contiguous data in our routing buffer */
|
||||
process_playback_routing_us144mkii(tascam, dst_buf, dst_buf,
|
||||
frames_to_copy);
|
||||
memcpy(urb->transfer_buffer, dst_buf, total_bytes_for_urb);
|
||||
}
|
||||
|
||||
urb->dev = tascam->dev;
|
||||
|
|
@ -244,6 +241,7 @@ void playback_urb_complete(struct urb *urb) {
|
|||
"Failed to resubmit playback URB: %d\n", ret);
|
||||
usb_unanchor_urb(urb);
|
||||
usb_put_urb(urb);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed resubmission */
|
||||
}
|
||||
out:
|
||||
usb_put_urb(urb);
|
||||
|
|
@ -273,9 +271,11 @@ void feedback_urb_complete(struct urb *urb) {
|
|||
|
||||
if (urb->status) {
|
||||
if (urb->status != -ENOENT && urb->status != -ECONNRESET &&
|
||||
urb->status != -ESHUTDOWN && urb->status != -ENODEV)
|
||||
urb->status != -ESHUTDOWN && urb->status != -ENODEV) {
|
||||
dev_err_ratelimited(tascam->card->dev, "Feedback URB failed: %d\n",
|
||||
urb->status);
|
||||
atomic_dec(&tascam->active_urbs); /* Decrement on failed resubmission */
|
||||
}
|
||||
goto out;
|
||||
}
|
||||
if (!tascam || !atomic_read(&tascam->playback_active))
|
||||
|
|
@ -418,3 +418,4 @@ unlock_and_continue:
|
|||
out:
|
||||
usb_put_urb(urb);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue