Re: [PATCH 1/2] ALSA: hda/hdmi: Add helper function to check if a device is HDMI codec

Takashi Iwai
2023-11-27T15:20:08Z

On Mon, 27 Nov 2023 15:45:54 +0100,
Péter Ujfalusi wrote:
>
>
>
> On 27/11/2023 16:31, Takashi Iwai wrote:
> > On Mon, 27 Nov 2023 15:12:51 +0100,
> > Péter Ujfalusi wrote:
> >>
> >>
> >>
> >> On 27/11/2023 15:18, Takashi Iwai wrote:
> >>>> +bool snd_hda_device_is_hdmi(struct hdac_device *hdev)
> >>>> +{
> >>>> + int i;
> >>>> +
> >>>> + for (i = 0; i < ARRAY_SIZE(snd_hda_id_hdmi); i++) {
> >>>> + if (snd_hda_id_hdmi[i].vendor_id == hdev->vendor_id)
> >>>> + return true;
> >>>> + }
> >>>> +
> >>>> + return false;
> >>>> +}
> >>>> +EXPORT_SYMBOL_GPL(snd_hda_device_is_hdmi);
> >>>
> >>> I'm afraid that this will bring unnecessary dependency on HDMI codec
> >>> driver.
> >>
> >> For HDMI support we anyways need HDMI code?
> >
> > But the ASoC hdac-hda driver isn't specifically bound with HDMI, I
> > thought?
> >
> > With your patch, now it becomes a hard-dependency. It'll be even
> > build failure when HDMI codec driver isn't enabled in Kconfig.
>
> The change in hdaudio.h handles the config dependency, if
> CONFIG_SND_HDA_CODEC_HDMI is not enabled in Kconfig then
> snd_hda_device_is_hdmi() will return false.

OK, that's at least good.
But I still find it not ideal to bring the hard dependency there
unnecessarily.

With the introduction of a flag in hdac_device, the necessary change
would be even smaller like below.

Takashi

--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -95,6 +95,7 @@ struct hdac_device {
bool lazy_cache:1; /* don't wake up for writes */
bool caps_overwriting:1; /* caps overwrite being in process */
bool cache_coef:1; /* cache COEF read/write too */
+ bool is_hdmi:1; /* a HDMI/DP codec */
unsigned int registered:1; /* codec was registered */
};

--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2597,6 +2597,7 @@ static int patch_generic_hdmi(struct hda_codec *codec)
}

generic_hdmi_init_per_pins(codec);
+ codec->core.is_hdmi = true;
return 0;
}

@@ -3472,6 +3473,7 @@ static int patch_simple_hdmi(struct hda_codec *codec,
spec->pcm_playback = simple_pcm_playback;

codec->patch_ops = simple_hdmi_patch_ops;
+ codec->core.is_hdmi = true;

return 0;
}