September 2022 - Alsa-devel - mailman.alsa-project.org

[PATCH] soundwire: bus: Fix wrong port number in sdw_handle_slave_alerts()
by Richard Fitzgerald 20 Sep '22

20 Sep '22

for_each_set_bit() gives the bit-number counting from 0 (LSbit==0). When processing INTSTAT2, bit 0 is DP4 so the port number is (bit + 4). Likewise for INTSTAT3 bit 0 is DP11 so port number is (bit + 11). Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- drivers/soundwire/bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 8eded1a55227..df0ae869ee51 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1622,7 +1622,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) port = buf2[0] & SDW_SCP_INTSTAT2_PORT4_10; for_each_set_bit(bit, &port, 8) { /* scp2 ports start from 4 */ - port_num = bit + 3; + port_num = bit + 4; sdw_handle_port_interrupt(slave, port_num, &port_status[port_num]); @@ -1634,7 +1634,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) port = buf2[1] & SDW_SCP_INTSTAT3_PORT11_14; for_each_set_bit(bit, &port, 8) { /* scp3 ports start from 11 */ - port_num = bit + 10; + port_num = bit + 11; sdw_handle_port_interrupt(slave, port_num, &port_status[port_num]); -- 2.30.2

2 2

HDA HDMI PCM device allocation
by Jaroslav Kysela 20 Sep '22

20 Sep '22

Hi, I am trying to fix some issues in UCM for the HDA HDMI devices [1][2]. I would like to summary the current situation at first (correct me, if I miss something): We have two methods to map the PINs in the HDA HDMI driver to the PCM devices (legacy/static - 1:1 mapping, dynamic - used for new devices with the MST capability). There is also set of converters in each HDMI codec and the number of simultaneously used PCM devices cannot go beyond this count of converters in hardware (otherwise -EBUSY error is returned). The count of converters is 3 or 4 depending on the hardware. Things to discuss: It seems quite straight to limit the count of created PCMs to the count of converters. We cannot use more anyway and it does not help, if more PCM devices are allocated (and Jacks reported) to applications when they cannot be used simultaneously. Legacy/static mapping should be converted to dynamic (unless the count of created PCM devices is equal to the count of codec converters). There may be 1:1 mapping between the converter and the PCM device to make things easier. There is a corner case, when more HDMI devices are connected than the count of converters. In this case, an extra method (a module parameter and/or a control element and/or procfs) may be used to filter unwanted HDMI devices. It may be a bit difficult to select the proper filtering key - it may be the PIN/MST device hash or so. The driver may report this key in eld#* files (procfs). Impact to applications: Those days, pulseaudio or pipewire servers are mostly used on the current hardware. Both servers share the legacy probe code for HDMI devices - they are trying to open PCM devices sequentially and check for the error code. There should not be a problem when the connected HDMI devices do not go beyond the count of converters. A minor issue is that the name of the used sink/port may be different (users may be forced to reselect the output path). For other applications, the PCM device assigned to the connected HDMI device may be different (available in a different ALSA device name). I do not think that it's a big issue. It should be easy solvable with an updated software configuration. Let me know about your opinion about this. Thank you, Jaroslav [1] https://github.com/alsa-project/alsa-lib/issues/245 [2] https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2481 -- Jaroslav Kysela <perex(a)perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

3 2

[PATCH 0/8] ALSA: hda: document/simplify hdac_ext handling
by Pierre-Louis Bossart 20 Sep '22

20 Sep '22

This sound/hda/ext library is a confusing magic black box that very few understand. It needn't be that way, we can document/simplify and make the code clearer. Pierre-Louis Bossart (8): ALSA: hda: make snd_hdac_stream_clear() static ALSA: hda: document state machine for hdac_streams ALSA: hda: ext: make snd_hdac_ext_stream_init() static ALSA: hda: Use hdac_ext prefix in snd_hdac_stream_free_all() for clarity ALSA: hda: add snd_hdac_stop_streams() helper ALSA: hda: ext: simplify logic for stream assignment ALSA: hda: ext: fix locking in stream_release ALSA: hda: ext: remove always-true conditions on host and link release include/sound/hdaudio.h | 3 +- include/sound/hdaudio_ext.h | 5 +-- sound/hda/ext/hdac_ext_stream.c | 34 +++++++-------- sound/hda/hdac_stream.c | 74 +++++++++++++++++++++++++++++---- sound/pci/hda/hda_controller.c | 4 +- sound/soc/intel/avs/core.c | 4 +- sound/soc/intel/skylake/skl.c | 2 +- 7 files changed, 87 insertions(+), 39 deletions(-) -- 2.34.1

2 9

[PATCH] ALSA: hda/hdmi: Fix the converter allocation for the silent stream
by Jaroslav Kysela 20 Sep '22

20 Sep '22

Track the converters handling the silent stream using a new variable to avoid mixing of the open/close and silent stream use. This change ensures the proper allocation of the converters. Fixes: 5f80d6bd2b01 ("ALSA: hda/hdmi: Fix the converter reuse for the silent stream") Cc: Kai Vehmanen <kai.vehmanen(a)linux.intel.com> Signed-off-by: Jaroslav Kysela <perex(a)perex.cz> --- sound/pci/hda/patch_hdmi.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 4576af4cff90..dc36af2928b7 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -54,6 +54,7 @@ MODULE_PARM_DESC(enable_all_pins, "Forcibly enable all pins"); struct hdmi_spec_per_cvt { hda_nid_t cvt_nid; bool assigned; /* the stream has been assigned */ + bool silent_stream; /* silent stream activated */ unsigned int channels_min; unsigned int channels_max; u32 rates; @@ -977,7 +978,8 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid, * of the pin. */ static int hdmi_choose_cvt(struct hda_codec *codec, - int pin_idx, int *cvt_id) + int pin_idx, int *cvt_id, + bool silent) { struct hdmi_spec *spec = codec->spec; struct hdmi_spec_per_pin *per_pin; @@ -992,6 +994,9 @@ static int hdmi_choose_cvt(struct hda_codec *codec, if (per_pin && per_pin->silent_stream) { cvt_idx = cvt_nid_to_cvt_index(codec, per_pin->cvt_nid); + per_cvt = get_cvt(spec, cvt_idx); + if (per_cvt->assigned && !silent) + return -EBUSY; if (cvt_id) *cvt_id = cvt_idx; return 0; @@ -1002,7 +1007,7 @@ static int hdmi_choose_cvt(struct hda_codec *codec, per_cvt = get_cvt(spec, cvt_idx); /* Must not already be assigned */ - if (per_cvt->assigned) + if (per_cvt->assigned || per_cvt->silent_stream) continue; if (per_pin == NULL) break; @@ -1188,7 +1193,7 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo, if (pcm_idx < 0) return -EINVAL; - err = hdmi_choose_cvt(codec, -1, &cvt_idx); + err = hdmi_choose_cvt(codec, -1, &cvt_idx, false); if (err) return err; @@ -1256,7 +1261,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, } } - err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx); + err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, false); if (err < 0) goto unlock; @@ -1267,7 +1272,6 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo, set_bit(pcm_idx, &spec->pcm_in_use); per_pin = get_pin(spec, pin_idx); per_pin->cvt_nid = per_cvt->cvt_nid; - per_pin->silent_stream = false; hinfo->nid = per_cvt->cvt_nid; /* flip stripe flag for the assigned stream if supported */ @@ -1749,14 +1753,14 @@ static void silent_stream_enable(struct hda_codec *codec, } pin_idx = pin_id_to_pin_index(codec, per_pin->pin_nid, per_pin->dev_id); - err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx); + err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, true); if (err) { codec_err(codec, "hdmi: no free converter to enable silent mode\n"); goto unlock_out; } per_cvt = get_cvt(spec, cvt_idx); - per_cvt->assigned = true; + per_cvt->silent_stream = true; per_pin->cvt_nid = per_cvt->cvt_nid; per_pin->silent_stream = true; @@ -1816,7 +1820,7 @@ static void silent_stream_disable(struct hda_codec *codec, cvt_idx = cvt_nid_to_cvt_index(codec, per_pin->cvt_nid); if (cvt_idx >= 0 && cvt_idx < spec->num_cvts) { per_cvt = get_cvt(spec, cvt_idx); - per_cvt->assigned = false; + per_cvt->silent_stream = false; } if (spec->silent_stream_type == SILENT_STREAM_I915) { -- 2.35.3

3 2

[PATCH 00/11] soundwire: intel: cleanups/fixes before code reorganization
by Bard Liao 20 Sep '22

20 Sep '22

This is a really big cleanup to introduce well defined helpers, prior to a larger code reorganization. Pierre-Louis Bossart (11): soundwire: intel: fix error handling on dai registration issues soundwire: intel: simplify flow and use devm_ for DAI registration soundwire: intel: move DAI registration and debugfs init earlier soundwire: intel: move all PDI initialization under intel_register_dai() soundwire: intel: remove clock_stop parameter in intel_shim_init() soundwire: intel: move shim initialization before power up/down soundwire: intel: remove intel_init() wrapper soundwire: intel: simplify read ops assignment soundwire: intel: introduce intel_shim_check_wake() helper soundwire: intel: introduce helpers to start bus soundwire: intel: add helper to stop bus drivers/soundwire/intel.c | 775 +++++++++++++++++++------------------- 1 file changed, 380 insertions(+), 395 deletions(-) -- 2.25.1

2 12

[PATCH 1/2] soundwire: cadence: Fix error check in cdns_xfer_msg()
by Richard Fitzgerald 20 Sep '22

20 Sep '22

_cdns_xfer_msg() returns an sdw_command_response value, not a negative error code. Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index ca241bbeadd9..3543a923ee6b 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -708,7 +708,7 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg) for (i = 0; i < msg->len / CDNS_MCP_CMD_LEN; i++) { ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN, CDNS_MCP_CMD_LEN, false); - if (ret < 0) + if (ret != SDW_CMD_OK) goto exit; } -- 2.30.2

3 4

[PATCH] soundwire: cadence: Write to correct address for each FIFO chunk
by Richard Fitzgerald 20 Sep '22

20 Sep '22

_cdns_xfer_msg() must add the fragment offset to msg->addr to get the base target address of each FIFO chunk. Otherwise every chunk will be written to the first 32 register addresses. Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- drivers/soundwire/cadence_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 3ef472049980..ca241bbeadd9 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -569,7 +569,7 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd, } base = CDNS_MCP_CMD_BASE; - addr = msg->addr; + addr = msg->addr + offset; for (i = 0; i < count; i++) { data = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num); -- 2.30.2

3 2

[PATCH v2 1/2] soundwire: qcom: update status from device id 1
by Srinivas Kandagatla 20 Sep '22

20 Sep '22

By default autoenumeration is enabled on QCom SoundWire controller which means the core should not be dealing with device 0 w.r.t enumeration. During Enumeration if SoundWire core sees status[0] as SDW_SLAVE_ATTACHED and start programming the device id, however reading DEVID registers return zeros which does not match to any of the slaves in the list and the core attempts to park this device to Group 13. This results in adding SoundWire device with enumeration address 0:0:0:0 Fix this by not passing device 0 status to SoundWire core. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- Changes since v1: - updated change log and split patch in to two as suggested by Pierre drivers/soundwire/qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index e7f95c41eb70..d3ce580cdeaf 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -448,7 +448,7 @@ static void qcom_swrm_get_device_status(struct qcom_swrm_ctrl *ctrl) ctrl->reg_read(ctrl, SWRM_MCP_SLV_STATUS, &val); ctrl->slave_status = val; - for (i = 0; i <= SDW_MAX_DEVICES; i++) { + for (i = 1; i <= SDW_MAX_DEVICES; i++) { u32 s; s = (val >> (i * 2)); -- 2.21.0

3 4

[PATCH] soundwire: cadence: Don't overwrite msg->buf during write commands
by Richard Fitzgerald 20 Sep '22

20 Sep '22

The buf passed in struct sdw_msg must only be written for a READ, in that case the RDATA part of the response is the data value of the register. For a write command there is no RDATA, and buf should be assumed to be const and unmodifable. The original caller should not expect its data buffer to be corrupted by an sdw_nwrite(). Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- drivers/soundwire/cadence_master.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index be9cd47f31ec..3ef472049980 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -544,9 +544,12 @@ cdns_fill_msg_resp(struct sdw_cdns *cdns, return SDW_CMD_IGNORED; } - /* fill response */ - for (i = 0; i < count; i++) - msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, cdns->response_buf[i]); + if (msg->flags == SDW_MSG_FLAG_READ) { + /* fill response */ + for (i = 0; i < count; i++) + msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, + cdns->response_buf[i]); + } return SDW_CMD_OK; } -- 2.30.2

3 2

[PATCH v4 0/5] soundwire: Fixes for spurious and missing UNATTACH
by Richard Fitzgerald 19 Sep '22

19 Sep '22

The bus and cadence code has several bugs that cause UNATTACH notifications to either be sent spuriously or to be missed. These can be seen occasionally with a single peripheral on the bus, but are much more frequent with multiple peripherals, where several peripherals could change state and report in consecutive PINGs. The root of all of these bugs seems to be a code design flaw that assumed every PING status change would be handled separately. However, PINGs are handled by a workqueue function and there is no guarantee when that function will be scheduled to run or how much CPU time it will receive. PINGs will continue while the work function is handling a snapshot of a previous PING so the code must take account that (a) status could change during the work function and (b) there can be a backlog of changes before the IRQ work function runs again. Tested with 4 peripherals on 1 bus, and 8 peripherals on 2 buses. CHANGES SINCE V3: Fixed minor comment typo in patch #4. Richard Fitzgerald (4): soundwire: bus: Don't lose unattach notifications soundwire: bus: Don't re-enumerate before status is UNATTACHED soundwire: cadence: Fix lost ATTACHED interrupts when enumerating soundwire: bus: Don't exit early if no device IDs were programmed Simon Trimmer (1): soundwire: cadence: fix updating slave status when a bus has multiple peripherals drivers/soundwire/bus.c | 44 +++++++++++++--- drivers/soundwire/cadence_master.c | 80 ++++++++++++++++-------------- 2 files changed, 80 insertions(+), 44 deletions(-) -- 2.30.2

2 6