[alsa-devel] [PATCH 0/3] snd-firewire-lib: add handling CMP output connection
Current implementation of snd-firewire-lib can handle only CMP input connection. This series of patch enable it to handle both CMP output and input connection according to IEC 61883-1.
Note that this series of patch still supports only a point-to-point connection without overlaying. The overlaying and any broadcast connection are not supported. Each connections gain its own isochronous resource.
Takashi Sakamoto (3): rename macros, variables and functions related to CMP plug Add "direction" member to cmp_connection structure Add handling CMP output connection
sound/firewire/cmp.c | 192 +++++++++++++++++++++++++++++++++------------ sound/firewire/cmp.h | 13 ++- sound/firewire/speakers.c | 2 +- 3 files changed, 152 insertions(+), 55 deletions(-)
Referring to IEC 61883-1, oMPR and iMPR, oPCR and iPCR have some fields with the same role in the same position. This patch renames some local macros, local variables and function arguments which has "i" in its prefix to reuse them between oMPR and iMPR, oPCR and iPCR.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 91 +++++++++++++++++++++++++------------------------- sound/firewire/cmp.h | 6 ++-- 2 files changed, 49 insertions(+), 48 deletions(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index 645cb0b..ec6d341 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -14,18 +14,20 @@ #include "iso-resources.h" #include "cmp.h"
-#define IMPR_SPEED_MASK 0xc0000000 -#define IMPR_SPEED_SHIFT 30 -#define IMPR_XSPEED_MASK 0x00000060 -#define IMPR_XSPEED_SHIFT 5 -#define IMPR_PLUGS_MASK 0x0000001f - -#define IPCR_ONLINE 0x80000000 -#define IPCR_BCAST_CONN 0x40000000 -#define IPCR_P2P_CONN_MASK 0x3f000000 -#define IPCR_P2P_CONN_SHIFT 24 -#define IPCR_CHANNEL_MASK 0x003f0000 -#define IPCR_CHANNEL_SHIFT 16 +/* MPR common fields */ +#define MPR_SPEED_MASK 0xc0000000 +#define MPR_SPEED_SHIFT 30 +#define MPR_XSPEED_MASK 0x00000060 +#define MPR_XSPEED_SHIFT 5 +#define MPR_PLUGS_MASK 0x0000001f + +/* PCR common fields */ +#define PCR_ONLINE 0x80000000 +#define PCR_BCAST_CONN 0x40000000 +#define PCR_P2P_CONN_MASK 0x3f000000 +#define PCR_P2P_CONN_SHIFT 24 +#define PCR_CHANNEL_MASK 0x003f0000 +#define PCR_CHANNEL_SHIFT 16
enum bus_reset_handling { ABORT_ON_BUS_RESET, @@ -96,24 +98,24 @@ bus_reset: * cmp_connection_init - initializes a connection manager * @c: the connection manager to initialize * @unit: a unit of the target device - * @ipcr_index: the index of the iPCR on the target device + * @pcr_index: the index of the iPCR/oPCR on the target device */ int cmp_connection_init(struct cmp_connection *c, struct fw_unit *unit, - unsigned int ipcr_index) + unsigned int pcr_index) { - __be32 impr_be; - u32 impr; + __be32 mpr_be; + u32 mpr; int err;
err = snd_fw_transaction(unit, TCODE_READ_QUADLET_REQUEST, CSR_REGISTER_BASE + CSR_IMPR, - &impr_be, 4); + &mpr_be, 4); if (err < 0) return err; - impr = be32_to_cpu(impr_be); + mpr = be32_to_cpu(mpr_be);
- if (ipcr_index >= (impr & IMPR_PLUGS_MASK)) + if (pcr_index >= (mpr & MPR_PLUGS_MASK)) return -EINVAL;
err = fw_iso_resources_init(&c->resources, unit); @@ -123,10 +125,10 @@ int cmp_connection_init(struct cmp_connection *c, c->connected = false; mutex_init(&c->mutex); c->last_pcr_value = cpu_to_be32(0x80000000); - c->pcr_index = ipcr_index; - c->max_speed = (impr & IMPR_SPEED_MASK) >> IMPR_SPEED_SHIFT; + c->pcr_index = pcr_index; + c->max_speed = (mpr & MPR_SPEED_MASK) >> MPR_SPEED_SHIFT; if (c->max_speed == SCODE_BETA) - c->max_speed += (impr & IMPR_XSPEED_MASK) >> IMPR_XSPEED_SHIFT; + c->max_speed += (mpr & MPR_XSPEED_MASK) >> MPR_XSPEED_SHIFT;
return 0; } @@ -147,23 +149,23 @@ EXPORT_SYMBOL(cmp_connection_destroy);
static __be32 ipcr_set_modify(struct cmp_connection *c, __be32 ipcr) { - ipcr &= ~cpu_to_be32(IPCR_BCAST_CONN | - IPCR_P2P_CONN_MASK | - IPCR_CHANNEL_MASK); - ipcr |= cpu_to_be32(1 << IPCR_P2P_CONN_SHIFT); - ipcr |= cpu_to_be32(c->resources.channel << IPCR_CHANNEL_SHIFT); + ipcr &= ~cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK | + PCR_CHANNEL_MASK); + ipcr |= cpu_to_be32(1 << PCR_P2P_CONN_SHIFT); + ipcr |= cpu_to_be32(c->resources.channel << PCR_CHANNEL_SHIFT);
return ipcr; }
-static int ipcr_set_check(struct cmp_connection *c, __be32 ipcr) +static int pcr_set_check(struct cmp_connection *c, __be32 pcr) { - if (ipcr & cpu_to_be32(IPCR_BCAST_CONN | - IPCR_P2P_CONN_MASK)) { + if (pcr & cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK)) { cmp_error(c, "plug is already in use\n"); return -EBUSY; } - if (!(ipcr & cpu_to_be32(IPCR_ONLINE))) { + if (!(pcr & cpu_to_be32(PCR_ONLINE))) { cmp_error(c, "plug is not on-line\n"); return -ECONNREFUSED; } @@ -178,9 +180,9 @@ static int ipcr_set_check(struct cmp_connection *c, __be32 ipcr) * * This function establishes a point-to-point connection from the local * computer to the target by allocating isochronous resources (channel and - * bandwidth) and setting the target's input plug control register. When this - * function succeeds, the caller is responsible for starting transmitting - * packets. + * bandwidth) and setting the target's input/output plug control register. + * When this function succeeds, the caller is responsible for starting + * transmitting packets. */ int cmp_connection_establish(struct cmp_connection *c, unsigned int max_payload_bytes) @@ -201,7 +203,7 @@ retry_after_bus_reset: if (err < 0) goto err_mutex;
- err = pcr_modify(c, ipcr_set_modify, ipcr_set_check, + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, ABORT_ON_BUS_RESET); if (err == -EAGAIN) { fw_iso_resources_free(&c->resources); @@ -229,8 +231,8 @@ EXPORT_SYMBOL(cmp_connection_establish); * cmp_connection_update - update the connection after a bus reset * @c: the connection manager * - * This function must be called from the driver's .update handler to reestablish - * any connection that might have been active. + * This function must be called from the driver's .update handler to + * reestablish any connection that might have been active. * * Returns zero on success, or a negative error code. On an error, the * connection is broken and the caller must stop transmitting iso packets. @@ -250,7 +252,7 @@ int cmp_connection_update(struct cmp_connection *c) if (err < 0) goto err_unconnect;
- err = pcr_modify(c, ipcr_set_modify, ipcr_set_check, + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, SUCCEED_ON_BUS_RESET); if (err < 0) goto err_resources; @@ -269,19 +271,18 @@ err_unconnect: } EXPORT_SYMBOL(cmp_connection_update);
- -static __be32 ipcr_break_modify(struct cmp_connection *c, __be32 ipcr) +static __be32 pcr_break_modify(struct cmp_connection *c, __be32 pcr) { - return ipcr & ~cpu_to_be32(IPCR_BCAST_CONN | IPCR_P2P_CONN_MASK); + return pcr & ~cpu_to_be32(PCR_BCAST_CONN | PCR_P2P_CONN_MASK); }
/** * cmp_connection_break - break the connection to the target * @c: the connection manager * - * This function deactives the connection in the target's input plug control - * register, and frees the isochronous resources of the connection. Before - * calling this function, the caller should cease transmitting packets. + * This function deactives the connection in the target's input/output plug + * control register, and frees the isochronous resources of the connection. + * Before calling this function, the caller should cease transmitting packets. */ void cmp_connection_break(struct cmp_connection *c) { @@ -294,7 +295,7 @@ void cmp_connection_break(struct cmp_connection *c) return; }
- err = pcr_modify(c, ipcr_break_modify, NULL, SUCCEED_ON_BUS_RESET); + err = pcr_modify(c, pcr_break_modify, NULL, SUCCEED_ON_BUS_RESET); if (err < 0) cmp_error(c, "plug is still connected\n");
diff --git a/sound/firewire/cmp.h b/sound/firewire/cmp.h index f47de08..2320cd4 100644 --- a/sound/firewire/cmp.h +++ b/sound/firewire/cmp.h @@ -11,8 +11,8 @@ struct fw_unit; * struct cmp_connection - manages an isochronous connection to a device * @speed: the connection's actual speed * - * This structure manages (using CMP) an isochronous stream from the local - * computer to a device's input plug (iPCR). + * This structure manages (using CMP) an isochronous stream between the local + * computer and a device's input plug (iPCR) and output plug (oPCR). * * There is no corresponding oPCR created on the local computer, so it is not * possible to overlay connections on top of this one. @@ -30,7 +30,7 @@ struct cmp_connection {
int cmp_connection_init(struct cmp_connection *connection, struct fw_unit *unit, - unsigned int ipcr_index); + unsigned int pcr_index); void cmp_connection_destroy(struct cmp_connection *connection);
int cmp_connection_establish(struct cmp_connection *connection,
To indicate the direction of connection, this patch adds "direction" member to cmp_connection structure. To determine the direction, this patch also adds "direction" argument to cmp_connection_init() function. The cmp_connection_init() function is exported and used in snd-firewire-speakers so this patch also affect it.
This patch just add them. Actual implementation will be done by followed patches.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 1 + sound/firewire/cmp.h | 7 +++++++ sound/firewire/speakers.c | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index ec6d341..a8d3d5f 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -102,6 +102,7 @@ bus_reset: */ int cmp_connection_init(struct cmp_connection *c, struct fw_unit *unit, + enum cmp_direction direction, unsigned int pcr_index) { __be32 mpr_be; diff --git a/sound/firewire/cmp.h b/sound/firewire/cmp.h index 2320cd4..4d26cc9 100644 --- a/sound/firewire/cmp.h +++ b/sound/firewire/cmp.h @@ -7,6 +7,11 @@
struct fw_unit;
+enum cmp_direction { + CMP_OUTPUT = 0, + CMP_INPUT +}; + /** * struct cmp_connection - manages an isochronous connection to a device * @speed: the connection's actual speed @@ -26,10 +31,12 @@ struct cmp_connection { __be32 last_pcr_value; unsigned int pcr_index; unsigned int max_speed; + enum cmp_direction direction; };
int cmp_connection_init(struct cmp_connection *connection, struct fw_unit *unit, + enum cmp_direction direction, unsigned int pcr_index); void cmp_connection_destroy(struct cmp_connection *connection);
diff --git a/sound/firewire/speakers.c b/sound/firewire/speakers.c index d684655..1e1f003 100644 --- a/sound/firewire/speakers.c +++ b/sound/firewire/speakers.c @@ -723,7 +723,7 @@ static int fwspk_probe(struct device *unit_dev) goto err_unit; }
- err = cmp_connection_init(&fwspk->connection, unit, 0); + err = cmp_connection_init(&fwspk->connection, unit, CMP_INPUT, 0); if (err < 0) goto err_unit;
To handle CMP output connection, this patch adds some macros, codes with condition of direction and new functions. Once cmp_connection_init() is executed with its direction, CMP input and output connection can be handled by the same way.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 106 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 9 deletions(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index a8d3d5f..334744d 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -29,6 +29,16 @@ #define PCR_CHANNEL_MASK 0x003f0000 #define PCR_CHANNEL_SHIFT 16
+/* oPCR specific fields */ +#define OPCR_XSPEED_MASK 0x00C00000 +#define OPCR_XSPEED_SHIFT 22 +#define OPCR_SPEED_MASK 0x0000C000 +#define OPCR_SPEED_SHIFT 14 +#define OPCR_OVERHEAD_ID_MASK 0x00003C00 +#define OPCR_OVERHEAD_ID_SHIFT 10 +#define OPCR_PAYLOAD_MASK 0x000003FF +#define OPCR_PAYLOAD_SHIFT 0 + enum bus_reset_handling { ABORT_ON_BUS_RESET, SUCCEED_ON_BUS_RESET, @@ -41,7 +51,8 @@ void cmp_error(struct cmp_connection *c, const char *fmt, ...)
va_start(va, fmt); dev_err(&c->resources.unit->device, "%cPCR%u: %pV", - 'i', c->pcr_index, &(struct va_format){ fmt, &va }); + (c->direction == CMP_INPUT) ? 'i' : 'o', + c->pcr_index, &(struct va_format){ fmt, &va }); va_end(va); }
@@ -54,8 +65,14 @@ static int pcr_modify(struct cmp_connection *c, int generation = c->resources.generation; int rcode, errors = 0; __be32 old_arg, buffer[2]; + unsigned long long offset; int err;
+ if (c->direction == CMP_INPUT) + offset = CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index); + else + offset = CSR_REGISTER_BASE + CSR_OPCR(c->pcr_index); + buffer[0] = c->last_pcr_value; for (;;) { old_arg = buffer[0]; @@ -64,8 +81,7 @@ static int pcr_modify(struct cmp_connection *c, rcode = fw_run_transaction( device->card, TCODE_LOCK_COMPARE_SWAP, device->node_id, generation, device->max_speed, - CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index), - buffer, 8); + offset, buffer, 8);
if (rcode == RCODE_COMPLETE) { if (buffer[0] == old_arg) /* success? */ @@ -107,11 +123,16 @@ int cmp_connection_init(struct cmp_connection *c, { __be32 mpr_be; u32 mpr; + unsigned long long offset; int err;
+ if (c->direction == CMP_INPUT) + offset = CSR_REGISTER_BASE + CSR_IMPR; + else + offset = CSR_REGISTER_BASE + CSR_OMPR; + err = snd_fw_transaction(unit, TCODE_READ_QUADLET_REQUEST, - CSR_REGISTER_BASE + CSR_IMPR, - &mpr_be, 4); + offset, &mpr_be, 4); if (err < 0) return err; mpr = be32_to_cpu(mpr_be); @@ -130,6 +151,7 @@ int cmp_connection_init(struct cmp_connection *c, c->max_speed = (mpr & MPR_SPEED_MASK) >> MPR_SPEED_SHIFT; if (c->max_speed == SCODE_BETA) c->max_speed += (mpr & MPR_XSPEED_MASK) >> MPR_XSPEED_SHIFT; + c->direction = direction;
return 0; } @@ -159,6 +181,62 @@ static __be32 ipcr_set_modify(struct cmp_connection *c, __be32 ipcr) return ipcr; }
+static int get_overhead_id(struct cmp_connection *c) +{ + int id; + + /* + * apply "oPCR overhead ID encoding" + * the encoding table can convert up to 512. + * here the value over 512 is converted as the same way as 512. + */ + for (id = 1; id < 16; id += 1) { + if (c->resources.bandwidth_overhead < (id << 5)) + break; + } + if (id == 16) + id = 0; + + return id; +} + +static __be32 opcr_set_modify(struct cmp_connection *c, __be32 opcr) +{ + unsigned int spd, xspd; + + /* generate speed and extended speed field value */ + if (c->speed > SCODE_400) { + spd = SCODE_800; + xspd = c->speed - SCODE_800; + } else { + spd = c->speed; + xspd = 0; + } + + opcr &= ~cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK | + OPCR_XSPEED_MASK | + PCR_CHANNEL_MASK | + OPCR_SPEED_MASK | + OPCR_OVERHEAD_ID_MASK | + OPCR_PAYLOAD_MASK); + opcr |= cpu_to_be32(1 << PCR_P2P_CONN_SHIFT); + opcr |= cpu_to_be32(xspd << OPCR_XSPEED_SHIFT); + opcr |= cpu_to_be32(c->resources.channel << PCR_CHANNEL_SHIFT); + opcr |= cpu_to_be32(spd << OPCR_SPEED_SHIFT); + opcr |= cpu_to_be32(get_overhead_id(c) << OPCR_OVERHEAD_ID_SHIFT); + /* + * here zero is applied to payload field. + * it means the maximum number of quadlets in an isochronous packet is + * 1024 when spd is less than three, 1024 * 2 * xspd + 1 when spd is + * equal to three. An arbitrary value can be set here but 0 is enough + * for our purpose. + */ + opcr |= cpu_to_be32(0 << OPCR_PAYLOAD_SHIFT); + + return opcr; +} + static int pcr_set_check(struct cmp_connection *c, __be32 pcr) { if (pcr & cpu_to_be32(PCR_BCAST_CONN | @@ -204,8 +282,13 @@ retry_after_bus_reset: if (err < 0) goto err_mutex;
- err = pcr_modify(c, ipcr_set_modify, pcr_set_check, - ABORT_ON_BUS_RESET); + if (c->direction == CMP_OUTPUT) + err = pcr_modify(c, opcr_set_modify, pcr_set_check, + ABORT_ON_BUS_RESET); + else + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, + ABORT_ON_BUS_RESET); + if (err == -EAGAIN) { fw_iso_resources_free(&c->resources); goto retry_after_bus_reset; @@ -253,8 +336,13 @@ int cmp_connection_update(struct cmp_connection *c) if (err < 0) goto err_unconnect;
- err = pcr_modify(c, ipcr_set_modify, pcr_set_check, - SUCCEED_ON_BUS_RESET); + if (c->direction == CMP_OUTPUT) + err = pcr_modify(c, opcr_set_modify, pcr_set_check, + SUCCEED_ON_BUS_RESET); + else + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, + SUCCEED_ON_BUS_RESET); + if (err < 0) goto err_resources;
Takashi Sakamoto wrote:
To handle CMP output connection, this patch adds some macros, codes with condition of direction and new functions. Once cmp_connection_init() is executed with its direction, CMP input and output connection can be handled by the same way.
+++ b/sound/firewire/cmp.c
- if (c->direction == CMP_INPUT)
offset = CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index);- else
offset = CSR_REGISTER_BASE + CSR_OPCR(c->pcr_index);
This code is used twice and could be moved into a helper function.
+static int get_overhead_id(struct cmp_connection *c)
- /*
* apply "oPCR overhead ID encoding"* the encoding table can convert up to 512.* here the value over 512 is converted as the same way as 512.*/
/* * Apply "oPCR overhead ID encoding": * The encoding table can convert up to 512. * Here any value over 512 is converted in the same way as 512. */
- for (id = 1; id < 16; id += 1) {
if (c->resources.bandwidth_overhead < (id << 5))break;- }
- if (id == 16)
id = 0;
id = DIV_ROUND_UP(c->resources.bandwidth_overhead, 32); if (id >= 16) id = 0;
+static __be32 opcr_set_modify(struct cmp_connection *c, __be32 opcr)
- /* generate speed and extended speed field value */
This comment is superfluous; it does not tell anything non-obvious about the code.
- /*
* here zero is applied to payload field.* it means the maximum number of quadlets in an isochronous packet is* 1024 when spd is less than three, 1024 * 2 * xspd + 1 when spd is* equal to three. An arbitrary value can be set here but 0 is enough* for our purpose.*/- opcr |= cpu_to_be32(0 << OPCR_PAYLOAD_SHIFT);
In an oPCR, the payload field is changed only by the device.
Regards, Clemens
Clemens,
Thanks for your review. I arrange these issues to 5 items below.
cmp[PATCH 3/3]: 1. Calculating offset should be moved into a helper function because they are used twice. OK. I push them into a helper function.
2. Comments should be start with capital letter in each sentenses. OK. I rewrite it.
3. Use DIV_ROUND_UP macro instead of "for" loop. OK. I rewrite with the macro.
4. This superfluous comment should say anything to be obvious about the code. OK. I remove the comment.
5. In an oPCR, the payload field is changed only by the device. OK. I misunderstand the specification. I check IEC 611883-1:2008 and I should follow the rules in "7.9 Plug control register modification rules". It menthions the fields which should be modified in the same compare_swap lock transaction and payload field is not included in it.
Then I have a question about handling payload field. In specification, "The payload field shall specify the maximum number of quadlets that may be transmitted in a single isochronous packet for this plug" but actually cheking this field seems to make no sense because there is no checking process in the procedure. I think I can do nothing for payload field.
Regards
Takashi Sakamoto o-takashi@sakamocchi.jp
(Apr 28 2013 21:52), Clemens Ladisch wrote:
Takashi Sakamoto wrote:
To handle CMP output connection, this patch adds some macros, codes with condition of direction and new functions. Once cmp_connection_init() is executed with its direction, CMP input and output connection can be handled by the same way.
+++ b/sound/firewire/cmp.c
- if (c->direction == CMP_INPUT)
offset = CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index);- else
offset = CSR_REGISTER_BASE + CSR_OPCR(c->pcr_index);This code is used twice and could be moved into a helper function.
+static int get_overhead_id(struct cmp_connection *c)
- /*
* apply "oPCR overhead ID encoding"* the encoding table can convert up to 512.* here the value over 512 is converted as the same way as 512.*//* * Apply "oPCR overhead ID encoding": * The encoding table can convert up to 512. * Here any value over 512 is converted in the same way as 512. */
- for (id = 1; id < 16; id += 1) {
if (c->resources.bandwidth_overhead < (id << 5))break;- }
- if (id == 16)
id = 0;id = DIV_ROUND_UP(c->resources.bandwidth_overhead, 32); if (id >= 16) id = 0;
+static __be32 opcr_set_modify(struct cmp_connection *c, __be32 opcr)
- /* generate speed and extended speed field value */
This comment is superfluous; it does not tell anything non-obvious about the code.
- /*
* here zero is applied to payload field.* it means the maximum number of quadlets in an isochronous packet is* 1024 when spd is less than three, 1024 * 2 * xspd + 1 when spd is* equal to three. An arbitrary value can be set here but 0 is enough* for our purpose.*/- opcr |= cpu_to_be32(0 << OPCR_PAYLOAD_SHIFT);
In an oPCR, the payload field is changed only by the device.
Regards, Clemens
Takashi Sakamoto wrote:
Then I have a question about handling payload field. In specification, "The payload field shall specify the maximum number of quadlets that may be transmitted in a single isochronous packet for this plug" but actually cheking this field seems to make no sense because there is no checking process in the procedure. I think I can do nothing for payload field.
The specification was written for the general case where the device that creates the connection might be different from the transmitting and receiving devices. In that case, the payload size is needed for correct bandwidth allocation.
In our case, the Linux PC does not have plugs that could be accessed by anybody else. (And I'm not sure if the value set by the Fireworks firmware is correct enough so that we'd want to use it.)
Regards, Clemens
Clemens,
The specification was written for the general case where the device that creates the connection might be different from the transmitting and receiving devices. In that case, the payload size is needed for correct bandwidth allocation.
I'm sure that the general case.
In our case, the Linux PC does not have plugs that could be accessed by anybody else.
I'm sure that host system (Linux PC) doesn't have no plugs.
(And I'm not sure if the value set by the Fireworks firmware is correct enough so that we'd want to use it.)
With my Fireworks device, it always report 0, it means 1024 bytes for max payload size for output plug. But I'm not sure that it always report the same value. Especially at 192.0 kHz, the payload size is over 1024 bytes but my device supports up to 96.0 kHz...
Actually the target device seems to update the max payload size after host connects to CMP output plug because its initial value is zero. Then we already allocate isochronous resources and isochronous packet to host system. I don't think it better to reallocate them after the connecting.
It's reasonable to ignore the payload field in any processing.
Regards
Takashi Sakamoto
(Apr 29 2013 15:30), Clemens Ladisch wrote:
Takashi Sakamoto wrote:
Then I have a question about handling payload field. In specification, "The payload field shall specify the maximum number of quadlets that may be transmitted in a single isochronous packet for this plug" but actually cheking this field seems to make no sense because there is no checking process in the procedure. I think I can do nothing for payload field.
The specification was written for the general case where the device that creates the connection might be different from the transmitting and receiving devices. In that case, the payload size is needed for correct bandwidth allocation.
In our case, the Linux PC does not have plugs that could be accessed by anybody else. (And I'm not sure if the value set by the Fireworks firmware is correct enough so that we'd want to use it.)
Regards, Clemens
I'm sorry but I correct what I said:
Actually the target device seems to update the max payload size after host connects to CMP output plug because its initial value is zero. Then we already allocate isochronous resources and isochronous packet to host system. I don't think it better to reallocate them after the connecting.
We don't allocate buffer for isochronous packet yet when connecting CMP output plug. Then we just gain isochronous resource.
Anyway I don't think it better to reallocate isochronous resource.
Regards
Takashi Sakamoto
(Apr 29 2013 18:25), Takashi Sakamoto wrote:
Clemens,
The specification was written for the general case where the device that creates the connection might be different from the transmitting and receiving devices. In that case, the payload size is needed for correct bandwidth allocation.
I'm sure that the general case.
In our case, the Linux PC does not have plugs that could be accessed by anybody else.
I'm sure that host system (Linux PC) doesn't have no plugs.
(And I'm not sure if the value set by the Fireworks firmware is correct enough so that we'd want to use it.)
With my Fireworks device, it always report 0, it means 1024 bytes for max payload size for output plug. But I'm not sure that it always report the same value. Especially at 192.0 kHz, the payload size is over 1024 bytes but my device supports up to 96.0 kHz...
Actually the target device seems to update the max payload size after host connects to CMP output plug because its initial value is zero. Then we already allocate isochronous resources and isochronous packet to host system. I don't think it better to reallocate them after the connecting.
It's reasonable to ignore the payload field in any processing.
Regards
Takashi Sakamoto
(Apr 29 2013 15:30), Clemens Ladisch wrote:
Takashi Sakamoto wrote:
Then I have a question about handling payload field. In specification, "The payload field shall specify the maximum number of quadlets that may be transmitted in a single isochronous packet for this plug" but actually cheking this field seems to make no sense because there is no checking process in the procedure. I think I can do nothing for payload field.
The specification was written for the general case where the device that creates the connection might be different from the transmitting and receiving devices. In that case, the payload size is needed for correct bandwidth allocation.
In our case, the Linux PC does not have plugs that could be accessed by anybody else. (And I'm not sure if the value set by the Fireworks firmware is correct enough so that we'd want to use it.)
Regards, Clemens
Current implementation of snd-firewire-lib can handle only CMP input connection. This series of patch enable it to handle both CMP output and input connection according to IEC 61883-1.
Note that this series of patch still supports only a point-to-point connection without overlaying. The overlaying and any broadcast connection are not supported. Each connections gain its own isochronous resource.
Takashi Sakamoto (3): rename macros, variables and functions related to CMP plug Add "direction" member to cmp_connection structure Add handling CMP output connection
sound/firewire/cmp.c | 188 +++++++++++++++++++++++++++++++++------------ sound/firewire/cmp.h | 13 +++- sound/firewire/speakers.c | 2 +- 3 files changed, 148 insertions(+), 55 deletions(-)
Referring to IEC 61883-1, oMPR and iMPR, oPCR and iPCR have some fields with the same role in the same position. This patch renames some local macros, local variables and function arguments which has "i" in its prefix to reuse them between oMPR and iMPR, oPCR and iPCR.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 91 +++++++++++++++++++++++++------------------------- sound/firewire/cmp.h | 6 ++-- 2 files changed, 49 insertions(+), 48 deletions(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index 645cb0b..ec6d341 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -14,18 +14,20 @@ #include "iso-resources.h" #include "cmp.h"
-#define IMPR_SPEED_MASK 0xc0000000 -#define IMPR_SPEED_SHIFT 30 -#define IMPR_XSPEED_MASK 0x00000060 -#define IMPR_XSPEED_SHIFT 5 -#define IMPR_PLUGS_MASK 0x0000001f - -#define IPCR_ONLINE 0x80000000 -#define IPCR_BCAST_CONN 0x40000000 -#define IPCR_P2P_CONN_MASK 0x3f000000 -#define IPCR_P2P_CONN_SHIFT 24 -#define IPCR_CHANNEL_MASK 0x003f0000 -#define IPCR_CHANNEL_SHIFT 16 +/* MPR common fields */ +#define MPR_SPEED_MASK 0xc0000000 +#define MPR_SPEED_SHIFT 30 +#define MPR_XSPEED_MASK 0x00000060 +#define MPR_XSPEED_SHIFT 5 +#define MPR_PLUGS_MASK 0x0000001f + +/* PCR common fields */ +#define PCR_ONLINE 0x80000000 +#define PCR_BCAST_CONN 0x40000000 +#define PCR_P2P_CONN_MASK 0x3f000000 +#define PCR_P2P_CONN_SHIFT 24 +#define PCR_CHANNEL_MASK 0x003f0000 +#define PCR_CHANNEL_SHIFT 16
enum bus_reset_handling { ABORT_ON_BUS_RESET, @@ -96,24 +98,24 @@ bus_reset: * cmp_connection_init - initializes a connection manager * @c: the connection manager to initialize * @unit: a unit of the target device - * @ipcr_index: the index of the iPCR on the target device + * @pcr_index: the index of the iPCR/oPCR on the target device */ int cmp_connection_init(struct cmp_connection *c, struct fw_unit *unit, - unsigned int ipcr_index) + unsigned int pcr_index) { - __be32 impr_be; - u32 impr; + __be32 mpr_be; + u32 mpr; int err;
err = snd_fw_transaction(unit, TCODE_READ_QUADLET_REQUEST, CSR_REGISTER_BASE + CSR_IMPR, - &impr_be, 4); + &mpr_be, 4); if (err < 0) return err; - impr = be32_to_cpu(impr_be); + mpr = be32_to_cpu(mpr_be);
- if (ipcr_index >= (impr & IMPR_PLUGS_MASK)) + if (pcr_index >= (mpr & MPR_PLUGS_MASK)) return -EINVAL;
err = fw_iso_resources_init(&c->resources, unit); @@ -123,10 +125,10 @@ int cmp_connection_init(struct cmp_connection *c, c->connected = false; mutex_init(&c->mutex); c->last_pcr_value = cpu_to_be32(0x80000000); - c->pcr_index = ipcr_index; - c->max_speed = (impr & IMPR_SPEED_MASK) >> IMPR_SPEED_SHIFT; + c->pcr_index = pcr_index; + c->max_speed = (mpr & MPR_SPEED_MASK) >> MPR_SPEED_SHIFT; if (c->max_speed == SCODE_BETA) - c->max_speed += (impr & IMPR_XSPEED_MASK) >> IMPR_XSPEED_SHIFT; + c->max_speed += (mpr & MPR_XSPEED_MASK) >> MPR_XSPEED_SHIFT;
return 0; } @@ -147,23 +149,23 @@ EXPORT_SYMBOL(cmp_connection_destroy);
static __be32 ipcr_set_modify(struct cmp_connection *c, __be32 ipcr) { - ipcr &= ~cpu_to_be32(IPCR_BCAST_CONN | - IPCR_P2P_CONN_MASK | - IPCR_CHANNEL_MASK); - ipcr |= cpu_to_be32(1 << IPCR_P2P_CONN_SHIFT); - ipcr |= cpu_to_be32(c->resources.channel << IPCR_CHANNEL_SHIFT); + ipcr &= ~cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK | + PCR_CHANNEL_MASK); + ipcr |= cpu_to_be32(1 << PCR_P2P_CONN_SHIFT); + ipcr |= cpu_to_be32(c->resources.channel << PCR_CHANNEL_SHIFT);
return ipcr; }
-static int ipcr_set_check(struct cmp_connection *c, __be32 ipcr) +static int pcr_set_check(struct cmp_connection *c, __be32 pcr) { - if (ipcr & cpu_to_be32(IPCR_BCAST_CONN | - IPCR_P2P_CONN_MASK)) { + if (pcr & cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK)) { cmp_error(c, "plug is already in use\n"); return -EBUSY; } - if (!(ipcr & cpu_to_be32(IPCR_ONLINE))) { + if (!(pcr & cpu_to_be32(PCR_ONLINE))) { cmp_error(c, "plug is not on-line\n"); return -ECONNREFUSED; } @@ -178,9 +180,9 @@ static int ipcr_set_check(struct cmp_connection *c, __be32 ipcr) * * This function establishes a point-to-point connection from the local * computer to the target by allocating isochronous resources (channel and - * bandwidth) and setting the target's input plug control register. When this - * function succeeds, the caller is responsible for starting transmitting - * packets. + * bandwidth) and setting the target's input/output plug control register. + * When this function succeeds, the caller is responsible for starting + * transmitting packets. */ int cmp_connection_establish(struct cmp_connection *c, unsigned int max_payload_bytes) @@ -201,7 +203,7 @@ retry_after_bus_reset: if (err < 0) goto err_mutex;
- err = pcr_modify(c, ipcr_set_modify, ipcr_set_check, + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, ABORT_ON_BUS_RESET); if (err == -EAGAIN) { fw_iso_resources_free(&c->resources); @@ -229,8 +231,8 @@ EXPORT_SYMBOL(cmp_connection_establish); * cmp_connection_update - update the connection after a bus reset * @c: the connection manager * - * This function must be called from the driver's .update handler to reestablish - * any connection that might have been active. + * This function must be called from the driver's .update handler to + * reestablish any connection that might have been active. * * Returns zero on success, or a negative error code. On an error, the * connection is broken and the caller must stop transmitting iso packets. @@ -250,7 +252,7 @@ int cmp_connection_update(struct cmp_connection *c) if (err < 0) goto err_unconnect;
- err = pcr_modify(c, ipcr_set_modify, ipcr_set_check, + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, SUCCEED_ON_BUS_RESET); if (err < 0) goto err_resources; @@ -269,19 +271,18 @@ err_unconnect: } EXPORT_SYMBOL(cmp_connection_update);
- -static __be32 ipcr_break_modify(struct cmp_connection *c, __be32 ipcr) +static __be32 pcr_break_modify(struct cmp_connection *c, __be32 pcr) { - return ipcr & ~cpu_to_be32(IPCR_BCAST_CONN | IPCR_P2P_CONN_MASK); + return pcr & ~cpu_to_be32(PCR_BCAST_CONN | PCR_P2P_CONN_MASK); }
/** * cmp_connection_break - break the connection to the target * @c: the connection manager * - * This function deactives the connection in the target's input plug control - * register, and frees the isochronous resources of the connection. Before - * calling this function, the caller should cease transmitting packets. + * This function deactives the connection in the target's input/output plug + * control register, and frees the isochronous resources of the connection. + * Before calling this function, the caller should cease transmitting packets. */ void cmp_connection_break(struct cmp_connection *c) { @@ -294,7 +295,7 @@ void cmp_connection_break(struct cmp_connection *c) return; }
- err = pcr_modify(c, ipcr_break_modify, NULL, SUCCEED_ON_BUS_RESET); + err = pcr_modify(c, pcr_break_modify, NULL, SUCCEED_ON_BUS_RESET); if (err < 0) cmp_error(c, "plug is still connected\n");
diff --git a/sound/firewire/cmp.h b/sound/firewire/cmp.h index f47de08..2320cd4 100644 --- a/sound/firewire/cmp.h +++ b/sound/firewire/cmp.h @@ -11,8 +11,8 @@ struct fw_unit; * struct cmp_connection - manages an isochronous connection to a device * @speed: the connection's actual speed * - * This structure manages (using CMP) an isochronous stream from the local - * computer to a device's input plug (iPCR). + * This structure manages (using CMP) an isochronous stream between the local + * computer and a device's input plug (iPCR) and output plug (oPCR). * * There is no corresponding oPCR created on the local computer, so it is not * possible to overlay connections on top of this one. @@ -30,7 +30,7 @@ struct cmp_connection {
int cmp_connection_init(struct cmp_connection *connection, struct fw_unit *unit, - unsigned int ipcr_index); + unsigned int pcr_index); void cmp_connection_destroy(struct cmp_connection *connection);
int cmp_connection_establish(struct cmp_connection *connection,
To indicate the direction of connection, this patch adds "direction" member to cmp_connection structure. To determine the direction, this patch also adds "direction" argument to cmp_connection_init() function. The cmp_connection_init() function is exported and used in snd-firewire-speakers so this patch also affect it.
This patch just add them. Actual implementation will be done by followed patches.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 1 + sound/firewire/cmp.h | 7 +++++++ sound/firewire/speakers.c | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index ec6d341..a8d3d5f 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -102,6 +102,7 @@ bus_reset: */ int cmp_connection_init(struct cmp_connection *c, struct fw_unit *unit, + enum cmp_direction direction, unsigned int pcr_index) { __be32 mpr_be; diff --git a/sound/firewire/cmp.h b/sound/firewire/cmp.h index 2320cd4..4d26cc9 100644 --- a/sound/firewire/cmp.h +++ b/sound/firewire/cmp.h @@ -7,6 +7,11 @@
struct fw_unit;
+enum cmp_direction { + CMP_OUTPUT = 0, + CMP_INPUT +}; + /** * struct cmp_connection - manages an isochronous connection to a device * @speed: the connection's actual speed @@ -26,10 +31,12 @@ struct cmp_connection { __be32 last_pcr_value; unsigned int pcr_index; unsigned int max_speed; + enum cmp_direction direction; };
int cmp_connection_init(struct cmp_connection *connection, struct fw_unit *unit, + enum cmp_direction direction, unsigned int pcr_index); void cmp_connection_destroy(struct cmp_connection *connection);
diff --git a/sound/firewire/speakers.c b/sound/firewire/speakers.c index d684655..1e1f003 100644 --- a/sound/firewire/speakers.c +++ b/sound/firewire/speakers.c @@ -723,7 +723,7 @@ static int fwspk_probe(struct device *unit_dev) goto err_unit; }
- err = cmp_connection_init(&fwspk->connection, unit, 0); + err = cmp_connection_init(&fwspk->connection, unit, CMP_INPUT, 0); if (err < 0) goto err_unit;
To handle CMP output connection, this patch adds some macros, codes with condition of direction and new functions. Once cmp_connection_init() is executed with its direction, CMP input and output connection can be handled by the same way.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/cmp.c | 102 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 9 deletions(-)
diff --git a/sound/firewire/cmp.c b/sound/firewire/cmp.c index a8d3d5f..9e70077 100644 --- a/sound/firewire/cmp.c +++ b/sound/firewire/cmp.c @@ -29,6 +29,16 @@ #define PCR_CHANNEL_MASK 0x003f0000 #define PCR_CHANNEL_SHIFT 16
+/* oPCR specific fields */ +#define OPCR_XSPEED_MASK 0x00C00000 +#define OPCR_XSPEED_SHIFT 22 +#define OPCR_SPEED_MASK 0x0000C000 +#define OPCR_SPEED_SHIFT 14 +#define OPCR_OVERHEAD_ID_MASK 0x00003C00 +#define OPCR_OVERHEAD_ID_SHIFT 10 +#define OPCR_PAYLOAD_MASK 0x000003FF +#define OPCR_PAYLOAD_SHIFT 0 + enum bus_reset_handling { ABORT_ON_BUS_RESET, SUCCEED_ON_BUS_RESET, @@ -41,10 +51,30 @@ void cmp_error(struct cmp_connection *c, const char *fmt, ...)
va_start(va, fmt); dev_err(&c->resources.unit->device, "%cPCR%u: %pV", - 'i', c->pcr_index, &(struct va_format){ fmt, &va }); + (c->direction == CMP_INPUT) ? 'i' : 'o', + c->pcr_index, &(struct va_format){ fmt, &va }); va_end(va); }
+static unsigned long long get_offset(struct cmp_connection *c, bool master) +{ + unsigned long long offset = CSR_REGISTER_BASE; + + if (!master) { + if (c->direction == CMP_INPUT) + offset += CSR_IPCR(c->pcr_index); + else + offset += CSR_OPCR(c->pcr_index); + } else { + if (c->direction == CMP_INPUT) + offset += CSR_IMPR; + else + offset += CSR_OMPR; + } + + return offset; +} + static int pcr_modify(struct cmp_connection *c, __be32 (*modify)(struct cmp_connection *c, __be32 old), int (*check)(struct cmp_connection *c, __be32 pcr), @@ -64,8 +94,7 @@ static int pcr_modify(struct cmp_connection *c, rcode = fw_run_transaction( device->card, TCODE_LOCK_COMPARE_SWAP, device->node_id, generation, device->max_speed, - CSR_REGISTER_BASE + CSR_IPCR(c->pcr_index), - buffer, 8); + get_offset(c, false), buffer, 8);
if (rcode == RCODE_COMPLETE) { if (buffer[0] == old_arg) /* success? */ @@ -110,8 +139,7 @@ int cmp_connection_init(struct cmp_connection *c, int err;
err = snd_fw_transaction(unit, TCODE_READ_QUADLET_REQUEST, - CSR_REGISTER_BASE + CSR_IMPR, - &mpr_be, 4); + get_offset(c, true), &mpr_be, 4); if (err < 0) return err; mpr = be32_to_cpu(mpr_be); @@ -130,6 +158,7 @@ int cmp_connection_init(struct cmp_connection *c, c->max_speed = (mpr & MPR_SPEED_MASK) >> MPR_SPEED_SHIFT; if (c->max_speed == SCODE_BETA) c->max_speed += (mpr & MPR_XSPEED_MASK) >> MPR_XSPEED_SHIFT; + c->direction = direction;
return 0; } @@ -159,6 +188,51 @@ static __be32 ipcr_set_modify(struct cmp_connection *c, __be32 ipcr) return ipcr; }
+static int get_overhead_id(struct cmp_connection *c) +{ + int id; + + /* + * Apply "oPCR overhead ID encoding" + * The encoding table can convert up to 512. + * Here the value over 512 is converted as the same way as 512. + */ + id = DIV_ROUND_UP(c->resources.bandwidth_overhead, 32); + if (id >= 16) + id = 0; + + return id; +} + +static __be32 opcr_set_modify(struct cmp_connection *c, __be32 opcr) +{ + unsigned int spd, xspd; + + if (c->speed > SCODE_400) { + spd = SCODE_800; + xspd = c->speed - SCODE_800; + } else { + spd = c->speed; + xspd = 0; + } + + opcr &= ~cpu_to_be32(PCR_BCAST_CONN | + PCR_P2P_CONN_MASK | + OPCR_XSPEED_MASK | + PCR_CHANNEL_MASK | + OPCR_SPEED_MASK | + OPCR_OVERHEAD_ID_MASK); + opcr |= cpu_to_be32(1 << PCR_P2P_CONN_SHIFT); + opcr |= cpu_to_be32(xspd << OPCR_XSPEED_SHIFT); + opcr |= cpu_to_be32(c->resources.channel << PCR_CHANNEL_SHIFT); + opcr |= cpu_to_be32(spd << OPCR_SPEED_SHIFT); + opcr |= cpu_to_be32(get_overhead_id(c) << OPCR_OVERHEAD_ID_SHIFT); + + /* NOTE: payload field is set by target device */ + + return opcr; +} + static int pcr_set_check(struct cmp_connection *c, __be32 pcr) { if (pcr & cpu_to_be32(PCR_BCAST_CONN | @@ -204,8 +278,13 @@ retry_after_bus_reset: if (err < 0) goto err_mutex;
- err = pcr_modify(c, ipcr_set_modify, pcr_set_check, - ABORT_ON_BUS_RESET); + if (c->direction == CMP_OUTPUT) + err = pcr_modify(c, opcr_set_modify, pcr_set_check, + ABORT_ON_BUS_RESET); + else + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, + ABORT_ON_BUS_RESET); + if (err == -EAGAIN) { fw_iso_resources_free(&c->resources); goto retry_after_bus_reset; @@ -253,8 +332,13 @@ int cmp_connection_update(struct cmp_connection *c) if (err < 0) goto err_unconnect;
- err = pcr_modify(c, ipcr_set_modify, pcr_set_check, - SUCCEED_ON_BUS_RESET); + if (c->direction == CMP_OUTPUT) + err = pcr_modify(c, opcr_set_modify, pcr_set_check, + SUCCEED_ON_BUS_RESET); + else + err = pcr_modify(c, ipcr_set_modify, pcr_set_check, + SUCCEED_ON_BUS_RESET); + if (err < 0) goto err_resources;
participants (2)
-
Clemens Ladisch -
Takashi Sakamoto