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

Takashi Iwai
2023-11-27T13:18:08Z

On Mon, 27 Nov 2023 14:02:44 +0100,
Peter Ujfalusi wrote:
>
> The snd_hda_device_is_hdmi() can be used in drivers to check if the hdev
> belongs to a display audio device.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
> include/sound/hdaudio.h | 10 ++++++++++
> sound/pci/hda/patch_hdmi.c | 13 +++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index dd7c87bbc613..cf5483d6b5b7 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -158,6 +158,16 @@ bool snd_hdac_check_power_state(struct hdac_device *hdac,
> hda_nid_t nid, unsigned int target_state);
> unsigned int snd_hdac_sync_power_state(struct hdac_device *hdac,
> hda_nid_t nid, unsigned int target_state);
> +
> +#if IS_ENABLED(CONFIG_SND_HDA_CODEC_HDMI)
> +bool snd_hda_device_is_hdmi(struct hdac_device *hdev);
> +#else
> +static inline bool snd_hda_device_is_hdmi(struct hdac_device *hdev)
> +{
> + return false;
> +}
> +#endif
> +
> /**
> * snd_hdac_read_parm - read a codec parameter
> * @codec: the codec object
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 1cde2a69bdb4..d6943575c8c7 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -4645,6 +4645,19 @@ HDA_CODEC_ENTRY(HDA_CODEC_ID_GENERIC_HDMI, "Generic HDMI", patch_generic_hdmi),
> };
> MODULE_DEVICE_TABLE(hdaudio, snd_hda_id_hdmi);
>
> +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.

IMO, it's better to add a bool flag is_hdmi to struct hdac_device, and
let the HDMI codec driver setting it at the probe, instead. Then we
can save an extra exported symbol, too.

thanks,

Takashi