On 2020-09-29 1:33 PM, Andy Shevchenko wrote:
On Sat, Sep 26, 2020 at 10:59:02AM +0200, Cezary Rojewski wrote:
DSP designed for Lynxpoint and Wildcat Point offers no dynamic topology i.e. all pipelines are already defined within firmware and host is relegated to allocing stream for predefined pins. This is represented by 'catpt_topology' member.
Implementation covers all available pin types:
- system playback and capture
- two offload streams
- loopback (reference)
- bluetooth playback and capture
PCM DAI operations differentiate between those pins as some (mainly offload) are to be handled differently - DSP expects wp updates on each notify_position notification.
System playback has no volume control capability as it is routed to mixer stream directly. Other primary streams - capture and two offloads
- offer individual volume controls.
Compared to sound/soc/intel/haswell this configures SSP device format automatically on pcm creation.
Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Some thoughts below, but it shouldn't prevent v10 to appear.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
...
+static u32 catpt_get_channel_map(enum catpt_channel_config config) +{
- switch (config) {
- case CATPT_CHANNEL_CONFIG_MONO:
return GENMASK(31, 4) | CATPT_CHANNEL_CENTER;
- case CATPT_CHANNEL_CONFIG_DUAL_MONO:
return GENMASK(31, 8) | CATPT_CHANNEL_LEFT| (CATPT_CHANNEL_LEFT << 4);Now we know why it used to be at the end of the switch-case. Up to you to leave here or move back.
Reverting to previous relocation in v10.
...
+static int catpt_dai_apply_usettings(struct snd_soc_dai *dai,
struct catpt_stream_runtime *stream)+{
- struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
- struct snd_soc_component *component = dai->component;
- struct snd_kcontrol *pos, *kctl = NULL;
- const char *name;
- int ret;
- u32 id = stream->info.stream_hw_id;
- /* only selected streams have individual controls */
- switch (id) {
- case CATPT_PIN_ID_OFFLOAD1:
name = "Media0 Playback Volume";break;- case CATPT_PIN_ID_OFFLOAD2:
name = "Media1 Playback Volume";break;- case CATPT_PIN_ID_CAPTURE1:
name = "Mic Capture Volume";break;- case CATPT_PIN_ID_REFERENCE:
name = "Loopback Mute";break;- default:
return 0;- };
- list_for_each_entry(pos, &component->card->snd_card->controls, list) {
if (pos->private_data == component &&!strncmp(name, pos->id.name, sizeof(pos->id.name))) {kctl = pos;break;}- }
- if (!kctl)
return -ENOENT;Hmm... Seems we lack of something like
/*
- list_entry_is_head() - Test if the entry points to the head of the list
- ...
*/ #define list_entry_is_head(pos, head, member) (&pos->member == head)
Up to you to consider. In above case we may drop pos from above loop and use kctl directly
struct snd_kcontrol *kctl; ... list_for_each_entry(kctl, &component->card->snd_card->controls, list) { if (kctl->private_data == component && !strncmp(name, kctl->id.name, sizeof(kctl->id.name))) break; } if (list_entry_is_head(kctl, &component->card->snd_card->controls, list)) return -ENOENT;
Note, you usually don't need any special Acks to modify list.h this way, although Andrew's name pops up WRT library stuff.
Since you've already proposed the solution to LKML, once it's added to the kernel, it will be re-used in catpt just like resource_union().
Good ideas all around, Andy. It's rather surprising to me how many good ideas came during the upstream process of this small driver.
Czarek