Hi,
Thanks for your test.
On Wed, Sep 04, 2024 at 08:45:51PM +0000, Edmund Raile wrote:
> Hello Sakamoto-San, I very much appreciate the idea and effort to take on the tasklet conversion in small steps instead of all-at-once!
>
> I also thank you for the CC, I'd like to be the testing canary for the coal mine of firewire ALSA with RME FireFace!
> The ALSA mailing list is a bit overwhelming and I'll likely unsubscribe so a direct CC for anything I can test is a good idea.
>
> Trying to apply patch 1 of 5 to mainline, your kernel tree appears to be out of sync with mainline!
> It was missing b171e20 from 2009 and a7ecbe9 from 2022!
> I hope nothing else important is missing!
Yes. The series of changes is prepared for the next merge window to
v6.12 kernel. It is on the top of for-next branch in linux1394 tree.
You can see some patches on v6.12-rc2 tag.
https://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git/log/?h=for-next
> Since in fw_card_initialize, ret is tested to be 0 we'd need an else instead, is this correct?
>
> I edited these functions of patch 1, now everything applies just fine:
>
> @@ -571,11 +571,28 @@ void fw_card_initialize(struct fw_card *card,
> }
> EXPORT_SYMBOL(fw_card_initialize);
>
> -int fw_card_add(struct fw_card *card,
> - u32 max_receive, u32 link_speed, u64 guid)
> +int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
> + unsigned int supported_isoc_contexts)
> {
> + struct workqueue_struct *isoc_wq;
> int ret;
>
> + // This workqueue should be:
> + // * != WQ_BH Sleepable.
> + // * == WQ_UNBOUND Any core can process data for isoc context. The
> + // implementation of unit protocol could consumes the core
> + // longer somehow.
> + // * != WQ_MEM_RECLAIM Not used for any backend of block device.
> + // * == WQ_HIGHPRI High priority to process semi-realtime timestamped data.
> + // * == WQ_SYSFS Parameters are available via sysfs.
> + // * max_active == n_it + n_ir A hardIRQ could notify events for multiple isochronous
> + // contexts if they are scheduled to the same cycle.
> + isoc_wq = alloc_workqueue("firewire-isoc-card%u",
> + WQ_UNBOUND | WQ_HIGHPRI | WQ_SYSFS,
> + supported_isoc_contexts, card->index);
> + if (!isoc_wq)
> + return -ENOMEM;
> +
> card->max_receive = max_receive;
> card->link_speed = link_speed;
> card->guid = guid;
> @@ -584,9 +601,13 @@ int fw_card_add(struct fw_card *card,
>
> generate_config_rom(card, tmp_config_rom);
> ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
> if (ret == 0)
> list_add_tail(&card->link, &card_list);
> + else
> + destroy_workqueue(isoc_wq);
> +
> + card->isoc_wq = isoc_wq;
>
> mutex_unlock(&card_mutex);
>
> return ret;
> @@ -709,7 +729,9 @@ void fw_core_remove_card(struct fw_card *card)
> {
> struct fw_card_driver dummy_driver = dummy_driver_template;
> unsigned long flags;
>
> + might_sleep();
> +
> card->driver->update_phy_reg(card, 4,
> PHY_LINK_ACTIVE | PHY_CONTENDER, 0);
> fw_schedule_bus_reset(card, false, true);
> @@ -719,6 +741,7 @@ void fw_core_remove_card(struct fw_card *card)
> dummy_driver.free_iso_context = card->driver->free_iso_context;
> dummy_driver.stop_iso = card->driver->stop_iso;
> card->driver = &dummy_driver;
> + drain_workqueue(card->isoc_wq);
>
> spin_lock_irqsave(&card->lock, flags);
> fw_destroy_nodes(card);
>
> Building a kernel with the patch produced 6.11.0-rc6-1-mainline-00019-g67784a74e258-dirty.
> Testing it with TI XIO2213B and RME Fireface 800 so far > 1 hour reveals no issues at all.
> ALSA streaming works fine:
> mpv --audio-device=alsa/sysdefault:CARD=Fireface800 Spor-Ignition.flac
>
> Though I haven't the faintest clue how to measure CPU usage impact of the patch, it looks like it would be neglible.
>
> As of finishing this, I noticed you released [2] https://lore.kernel.org/lkml/20240904125155.461886-1-o-takashi@xxxxxxxxxxxxx/T/
> I'll get around to testing that one too, but tomorrow at the earliest.
>
> Kind regards,
> Edmund Raile.
>
> Signed-off-by: Edmund Raile <edmund.raile@xxxxxxxxxxxxxx>
> Tested-by: Edmund Raile <edmund.raile@xxxxxxxxxxxxxx>
If using v6.11 kernel, it is convenient to use my remote repository to
backport changes for v6.12. But let you be careful to the history of
changes anyway.
* https://github.com/takaswie/linux-firewire-dkms/
Thanks
Takashi Sakamoto
[Index of Archives]
[Pulseaudio]
[Linux Audio Users]
[ALSA Devel]
[Fedora Desktop]
[Fedora SELinux]
[Big List of Linux Books]
[Yosemite News]
[KDE Users]