On 27/11/2023 17:20, Takashi Iwai wrote:
> 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;
> }
I see, so this is what I was not sure how to do ;)
I will rework the series and resend tomorrow.
Thanks for the code snippet, I will make you as author of it, if it is
OK for you.
--
Péter
