DigiMagic

[zamaudio]

Hi Takashi,

There are 2 models of the 003Rack:

003R: 4 preamps - digimagic works 003R+: 8 preamps - digimagic does not work (yet)

I actually checked out your code from 4 days ago and played with this branch. I manually added the digimagic that Robin and I reverse engineered, but unfortunately the code does not seem to work for 003Rack+ model. We need to reverse engineer new magic bytes for the 003R+.

Then I also tested your latest code and the magic is still not working on 003R+.

I also have added code for mixer control of the clock source and verified it works on my hardware (003R+). I can do a pull request with the new code soon.

Damien

[zamaudio]

I originally reversed the magic bytes with Robin using the 003R model, but I sold this hardware and now I own the 003R+ model. Do you have any 003 hardware Takashi?

[zamaudio]

My first comment is not correct. Digimagic should work on 003R+, but so far it appears there is a bug in your merging of digimagic into the kernel driver.

[takaswie]

Damien,

I use Digi 002 rack for this development and your Digimagic works partially. I suspect that the implement includes some bugs.

[zamaudio]

I have a firewire bus analyser and I have captured the initialisation sequence for 003R+ using ProtoolsLE software. I have implemented the order of the sequence the same as the proprietary driver but there are dropouts with the streams with my version of the code that I don't understand.

[zamaudio]

Takashi,

I believe that the 003R+ does not like the way you used the combining of streams into one command 0100 = 0x00 | (0x01 << 16).

In protools, the 0100 is written sequentially twice, 0100 = 0(tx record) and then 0100 = 1(rx playback).

Do you know what the difference is for the device?

Please see https://github.com/zamaudio/snd-firewire-improve on branch digi003R+magicborked for my latest code that works like protools, (except magic broken) having the dropouts.

[zamaudio]

Takashi,

The original digimagic worked 100% correctly because I tested it on my device, and we saw that the bit patterns were coming out exactly as the bus analyser showed us for a test pattern of a triangle wave form.

I believe the problem with your driver currently, is that the initialisation sequence for the rack is incorrect. You need to address begin_session() because the device needs to be polled for state transitions. I have already implemented the correct state machine step by step in my latest branch above.

[takaswie]

You need to address begin_session() because the device needs to be polled for state transitions.

Post your patch for alsa-devel.

[takaswie]

With enough explaination which every developers can understand.

[takaswie]

And for firewire drivers, ALSA developers basically add no Control interfaces. The reason is that this functionality can be implemented into userspace application. It's unacceptable to add unrequired codes into Linux kernel code base.

[zamaudio]

Hey Takashi, I used your master branch with this slight patch and digimagic is working 100%

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index b86bd1c..933b42d 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -300,9 +300,10 @@ void amdtp_stream_set_pcm_format(struct amdtp_stream *s,
        WARN_ON(1);
        /* fall through */
    case SNDRV_PCM_FORMAT_S32:
-       if (s->direction == AMDTP_OUT_STREAM)
-           s->transfer_samples = amdtp_write_s32;
-       else
+       if (s->direction == AMDTP_OUT_STREAM) {
+           if(!s->transfer_samples)
+               s->transfer_samples = amdtp_write_s32;
+       } else
            s->transfer_samples = amdtp_read_s32;
        break;
    }
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index bc4c88c..e20fd8a 100755
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -170,15 +170,6 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
    if (i == SND_DG00X_RATE_COUNT)
        return -EINVAL;

-   /* Keep resources for out-stream. */
-   amdtp_stream_set_parameters(&dg00x->rx_stream, rate,
-                   snd_dg00x_stream_mbla_data_channels[i], 2);
-   err = fw_iso_resources_allocate(&dg00x->rx_resources,
-               amdtp_stream_get_max_payload(&dg00x->rx_stream),
-               fw_parent_device(dg00x->unit)->max_speed);
-   if (err < 0)
-       return err;
-
    /* Keep resources for in-stream. */
    amdtp_stream_set_parameters(&dg00x->tx_stream, rate,
                    snd_dg00x_stream_mbla_data_channels[i], 1);
@@ -188,6 +179,15 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
    if (err < 0)
        goto error;

+   /* Keep resources for out-stream. */
+   amdtp_stream_set_parameters(&dg00x->rx_stream, rate,
+                   snd_dg00x_stream_mbla_data_channels[i], 2);
+   err = fw_iso_resources_allocate(&dg00x->rx_resources,
+               amdtp_stream_get_max_payload(&dg00x->rx_stream),
+               fw_parent_device(dg00x->unit)->max_speed);
+   if (err < 0)
+       return err;
+
    /* Register isochronous channels for both direction. */
    data = cpu_to_be32((dg00x->tx_resources.channel << 16) |
               dg00x->rx_resources.channel);

[takaswie]

I'm sorry but I cannot find any functional changes in your patch... I think there're some misunderstanding or mis-operation in your side.

[zamaudio]

+           if(!s->transfer_samples)

is very important because otherwise amdtp_stream_set_pcm_format() will override the transfer_samples function pointer with amdtp_write_s32() and digimagic is never called.

[zamaudio]

The order of allocating the streams could be important because the device might be expecting a hardcoded stream number so that the digimagic works on the correct stream (?). It is always the same in ProTools, tx = 0 and rx = 1, verified with bus analyser.

[zamaudio]

It seems we were both correct, there was a bug in the implementation of digimagic, which has now been corrected in 003amdtp, and now we need to integrate it correctly into the digi00x driver.

[zamaudio]

I am closing this issue because the first few comments I made are irrelevant and incorrect.