Re: [PATCH] ALSA: dice: add stream format parameters for Weiss devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Mr. Sakamoto,

thank you very much for the prompt feedback, and also for your contributions to
the Linux audio Firewire stack during the last decade.

We at Weiss Engineering would like to improve Linux support for our Firewire
devices in order to extend their lifetimes. We have also occasionally received
requests for Linux compatibility from some customers.

This is also motivated by the fact that the old DICE Apple driver, which was
originally developed by TCAT, is not maintained anymore and does not work on
Apple Silicon computers. Therefore, we would like to give the alternative of
running our devices on Linux if the customers decide to do.

We are also working on a update for our old music archive network player
(MAN301), which uses a DICE chip to interface with its DAC; so it seemed only
fair that owners of all Weiss devices based on DICE benefited from this.

Sorry for mistakes 1, 2, and 3 (you can tell it's my first kernel patch).
And I indeed forgot to include the snd_dice_detect_weiss_formats() prototype
into the patch submittal.

About the 4th issue, yes, the 'category_id' is still correct [1].

About the 5th issue, you are right both about the wrong tx/rx direction and
the absence of isochronous packets from the device. I checked the DICE
firmware code for the INT202 and we are indeed sending isochronous packets from
the device. Therefore, the correct stream formats should look like this:

+/* Weiss INT202: 192kHz unidirectional 2-channel digital Firewire interface
*/
+static const struct dice_weiss_spec int202 = {
+    .tx_pcm_chs = {{2, 2, 2}, {0, 0, 0} },
+    .rx_pcm_chs = {{2, 2, 2}, {0, 0, 0} },
+    .has_midi   = false
+};

I tested this configuration with XLR and RCA outputs, and it works correctly.
Sorry for the mistake, I don't have a deep knowledge about the old DICE
firmware because I've started working at Weiss Engineering only recently.

About the configuration ROMs, that shouldn't be an issue; let me get back to
you as soon as I speak with my colleague, Rolf Anderegg. He has worked on our
DICE devices in the past but he's on vacation right now.

Yet another topic is AVC support. We used to have support for it for the DICE driver in the 3.x kernel, and we are in the process of re-adapting that code.
But this should be probably discussed in a separate e-mail.

[1]: https://github.com/torvalds/linux/commit/a471fcde8c2c4b65f110bb4210af3513ee4deeb8

Kind regards,
Michele Perrone


On 28/07/23 15:13, Takashi Sakamoto wrote:
ei,

On Fri, Jul 28, 2023 at 11:16:11AM +0200, Michele Perrone wrote:
Hard-coded stream format parameters are added for Weiss Engineering
FireWire devices. When the device vendor and model match, the parameters
are copied into the stream format cache. This allows for setting all
supported sampling rates up to 192kHz, and consequently adjusting the
number of available I/O channels.

Signed-off-by: Rolf Anderegg <rolf.anderegg@xxxxxxxx>
Signed-off-by: Michele Perrone <michele.perrone@xxxxxxxx>
---
  sound/firewire/dice/Makefile     |   2 +-
  sound/firewire/dice/dice-weiss.c | 120 +++++++++++++++++++++++++++++++
  sound/firewire/dice/dice.c       |  72 +++++++++++++++++++
  3 files changed, 193 insertions(+), 1 deletion(-)
  create mode 100644 sound/firewire/dice/dice-weiss.c
Thanks for the patch. I've been waiting so long for such patches to
support Weiss models. I welcome them.

As long as reviewing, I found some format and technical issues on the
patches:

Format issues:
1.unexpected line break
2.tab indentations are replaced with spaces

Technical issues:
3.build error due to missing snd_dice_detect_weiss_formats()
   * prototype of snd_dice_detect_weiss_formats() should be in dice.c
4. category_id in GUID
5.stream formats for INT202

I can correct 1st, 2nd, and 3rd issues. You can find the revised patch
in the top-most of my remote repository[1].

Let me confirm the 4th issue. TCAT defines 'category_id' field in GUID
value of devices. As long as I know, Weiss engineers uses 0x00 in the
field[2]. Is it still correct for the devices supported in the patch?

Next, let us discuss about INT202 stream formats.

+
+/* Weiss INT202: 192kHz unidirectional 2-channel digital Firewire interface
*/
+static const struct dice_weiss_spec int202 = {
+    .tx_pcm_chs = {{2, 2, 2}, {0, 0, 0} },
+    .rx_pcm_chs = {{0, 0, 0}, {0, 0, 0} },
+    .has_midi   = false
+};

(tx/rx should be device-oriented, tx = from device, rx = to device,
please correct)

I assume all of the DICE devices transmit isochronous packets to deliver
presentation time stamp and events (= PCM frames). Then driver software
utilizes the presentation time stamp and the amount of events to construct
payload of isochronous packets into the device.

I program ALSA dice driver based on the assumption, thus ALSA dice driver
doesn't work well without receiving any isochronous packet from the
device. However, the stream formats for INT202 device looks to support
uni-directional operation. Weiss engineers really use DICE chipset like
that? If so, I need to integrate the driver to support the case.


As another topic. I make collection of configuration ROMs[3] to make better
support for the devices in Linux system[4]. I'm pleased if you pick them
up from your devices and dedicate them for the collection.

[1] https://github.com/takaswie/sound/tree/topic/dice/weiss-support
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/sound/firewire/dice/dice.c?h=v6.4#n59
[3] https://github.com/takaswie/am-config-roms/
[4] https://github.com/systemd/systemd/blob/main/hwdb.d/80-ieee1394-unit-function.hwdb


Thanks

Takashi Sakamoto



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux