| Message ID | 20190910182916.29693-3-kai.vehmanen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | adapt SOF to use snd-hda-codec-hdmi | expand |
On 9/10/19 1:29 PM, Kai Vehmanen wrote: > Fix the logic in generic_hdmi_build_controls() to identify > unused hda_pcm entries by searching for SNDRV_PCM_INVALID_DEVICE. > This matches with logic in snd_hda_codec_build_pcms(). > > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > --- > sound/pci/hda/patch_hdmi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 59aaee4a40fd..c52726e19f44 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -2183,11 +2183,13 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) > static int generic_hdmi_build_controls(struct hda_codec *codec) > { > struct hdmi_spec *spec = codec->spec; > + struct hda_pcm *hda_pcm; > int dev, err; > int pin_idx, pcm_idx; > > for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) { > - if (!get_pcm_rec(spec, pcm_idx)->pcm) { > + hda_pcm = get_pcm_rec(spec, pcm_idx); > + if (hda_pcm->device == SNDRV_PCM_INVALID_DEVICE) { I find it hard to follow the logic. Before this patch only device 0 would be considered NO_PCM Not it's -1, so wondering if this is a fix unrelated to this series or if there is a dependency on follow-up patches? #define SNDRV_PCM_INVALID_DEVICE (-1) > /* no PCM: mark this for skipping permanently */ > set_bit(pcm_idx, &spec->pcm_bitmap); > continue; >
Hi, On Tue, 10 Sep 2019, Pierre-Louis Bossart wrote: > On 9/10/19 1:29 PM, Kai Vehmanen wrote: > > Fix the logic in generic_hdmi_build_controls() to identify > > unused hda_pcm entries by searching for SNDRV_PCM_INVALID_DEVICE. > > This matches with logic in snd_hda_codec_build_pcms(). [...] > > - if (!get_pcm_rec(spec, pcm_idx)->pcm) { > > + hda_pcm = get_pcm_rec(spec, pcm_idx); > > + if (hda_pcm->device == SNDRV_PCM_INVALID_DEVICE) { > > I find it hard to follow the logic. > Before this patch only device 0 would be considered NO_PCM > Not it's -1, so wondering if this is a fix unrelated to this series or if > there is a dependency on follow-up patches? the intent was to align with logic in hda_codec.c:snd_hda_codec_build_pcms() which is the common function to create the HDA driver PCM state. In that function: » list_for_each_entry(cpcm, &codec->pcm_list_head, list) { [...] » » dev = get_empty_pcm_device(bus, cpcm->pcm_type); » » if (dev < 0) { » » » cpcm->device = SNDRV_PCM_INVALID_DEVICE; » » » continue; /* no fatal error */ » » } ... i.e. if no PCM is attached, it is marked with SNDRV_PCM_INVALID_DEVICE to cpcm->device. The logic in patch_hdmi.c, which deduced the same information by looking at another field (cpcm>->pcm), seems wrong and thus the fix. Now, this used to be critical for this patch series as well, but you are right that is no longer the case (I added passing of the PCM handle so we can reuse all the kcontrol code in patch_hdmi.c and with that this patch is no longer mandatory for SOF usage). I retested the series without the patch and all seems fine, so let's simplify the series by dropping this. Br, Kai
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 59aaee4a40fd..c52726e19f44 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2183,11 +2183,13 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) static int generic_hdmi_build_controls(struct hda_codec *codec) { struct hdmi_spec *spec = codec->spec; + struct hda_pcm *hda_pcm; int dev, err; int pin_idx, pcm_idx; for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) { - if (!get_pcm_rec(spec, pcm_idx)->pcm) { + hda_pcm = get_pcm_rec(spec, pcm_idx); + if (hda_pcm->device == SNDRV_PCM_INVALID_DEVICE) { /* no PCM: mark this for skipping permanently */ set_bit(pcm_idx, &spec->pcm_bitmap); continue;
Fix the logic in generic_hdmi_build_controls() to identify unused hda_pcm entries by searching for SNDRV_PCM_INVALID_DEVICE. This matches with logic in snd_hda_codec_build_pcms(). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> --- sound/pci/hda/patch_hdmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)