Re: [PATCH] ALSA: hda/tegra: Fix hda Jack detection

[Mohan Kumar D <mkumard@nvidia.com>]

On 4/5/2022 12:03 PM, Takashi Iwai wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, 05 Apr 2022 08:05:03 +0200,
> Mohan Kumar D wrote:
>> On 4/5/2022 11:27 AM, Takashi Iwai wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Tue, 05 Apr 2022 05:26:07 +0200,
>>> Mohan Kumar wrote:
>>>> Tegra HDA Jack detection logic doesn't work when the HDACODEC
>>>> in runtime suspended state as unsol event won't be triggered
>>>> during D3 state. As pulseaudio server in userspace rely on the
>>>> jack mixer control status to show the audio devices in gui and
>>>> any display sink device hotplug event during D3 state will never
>>>> updates the jack status which will result in no audio device option
>>>> available in userspace settings.
>>>>
>>>> The possible option available to resolve this issue for multiple
>>>> tegra platforms is to use Jack polling method for every 5 seconds.
>>>> Also to make Jack detection work seamlessly the Jack worker thread
>>>> needs to run continuously after HDA sound card registered
>>>> irrespective of whether HDMI sink device connected or not, but the
>>>> Jack state update call happens only when Codec is not powered on.
>>>>
>>>> Signed-off-by: Mohan Kumar<mkumard@nvidia.com>
>>> Hmm, any reason not to use the standard jackpoll stuff that is already
>>> implemented in HD-audio controller side?  That is, doesn't the
>>> following oneliner work instead?
>> The reason is, the Jack poll thread implemented in hda_codec.c runs
>> only when HDACODEC is in runtime resume state. But the problem trying
>> resolve here is something opposite, bcaz when hdacodec is in runtime
>> resume state unsol event would work but not during suspend state. So
>> either need to make some changes on hda_codec.c specific to tegra or
>> make it on tegra specific driver. So I went with second option.
> Well, the current behavior of jackpoll is intentional, so that it
> avoids the unnecessary power up at the runtime PM suspend.  And your
> requirement is rather opposite, and it's not Tegra-specific at all --
> you just prefer the jack notification over the power saving.
>
> So, implementing the feature in HD-audio core side would make more
> sense (and it's simpler), something like below.
>
> BTW, which codec needs this?  If it's about HDMI, doesn't the audio
> component work?  At least nouveau has it, and I thought Nvidia binary
> driver does poke the driver at hotplug by opening the proc file or
> such.
Thanks for the feedback. Will implement the next patch in HD-audio core 
side as suggested below.
Yes, it is about HDMI.
> Takashi
>
> -- 8< --
> --- a/include/sound/hda_codec.h
> +++ b/include/sound/hda_codec.h
> @@ -59,6 +59,7 @@ struct hda_bus {
>          unsigned int no_response_fallback:1; /* don't fallback at RIRB error */
>          unsigned int bus_probing :1;    /* during probing process */
>          unsigned int keep_power:1;      /* keep power up for notification */
> +       unsigned int jackpoll_in_suspend:1; /* keep jack polling during runtime suspend */
>
>          int primary_dig_out_type;       /* primary digital out PCM type */
>          unsigned int mixer_assigned;    /* codec addr for mixer name */
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -2935,7 +2935,9 @@ static int hda_codec_runtime_suspend(struct device *dev)
>          if (!codec->card)
>                  return 0;
>
> -       cancel_delayed_work_sync(&codec->jackpoll_work);
> +       if (!codec->bus->jackpoll_in_suspend &&
> +           dev->power.power_state != PMSG_ON)
> +               cancel_delayed_work_sync(&codec->jackpoll_work);
>          state = hda_call_codec_suspend(codec);
>          if (codec->link_down_at_suspend ||
>              (codec_has_clkstop(codec) && codec_has_epss(codec) &&
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -421,6 +421,8 @@ static int hda_tegra_create(struct snd_card *card,
>          chip->driver_type = driver_caps & 0xff;
>          chip->dev_index = 0;
>          INIT_LIST_HEAD(&chip->pcm_list);
> +       chip->jackpoll_interval = msecs_to_jiffies(5000);
> +       chip->jackpoll_in_resume = 1;
>
>          chip->codec_probe_mask = -1;
>