| Message ID | 20220405032607.8489-1-mkumard@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ALSA: hda/tegra: Fix hda Jack detection | expand |
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? thanks, Takashi -- 8< -- --- a/sound/pci/hda/hda_tegra.c +++ b/sound/pci/hda/hda_tegra.c @@ -421,6 +421,7 @@ 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->codec_probe_mask = -1;
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. > > > thanks, > > Takashi > > -- 8< -- > --- a/sound/pci/hda/hda_tegra.c > +++ b/sound/pci/hda/hda_tegra.c > @@ -421,6 +421,7 @@ 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->codec_probe_mask = -1; >
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. 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;
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; >
diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c index 2347d0304f93..c92938a47b65 100644 --- a/sound/pci/hda/hda_tegra.c +++ b/sound/pci/hda/hda_tegra.c @@ -28,6 +28,7 @@ #include <sound/hda_codec.h> #include "hda_controller.h" +#include "hda_jack.h" /* Defines for Nvidia Tegra HDA support */ #define HDA_BAR0 0x8000 @@ -67,6 +68,7 @@ * is used to update the GCAP register to workaround the issue. */ #define TEGRA194_NUM_SDO_LINES 4 +#define JACKPOLL_INTERVAL msecs_to_jiffies(5000) struct hda_tegra_soc { bool has_hda2codec_2x_reset; @@ -82,6 +84,7 @@ struct hda_tegra { unsigned int nclocks; void __iomem *regs; struct work_struct probe_work; + struct delayed_work jack_work; const struct hda_tegra_soc *soc; }; @@ -127,8 +130,11 @@ static void hda_tegra_init(struct hda_tegra *hda) static int __maybe_unused hda_tegra_suspend(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); + struct azx *chip = card->private_data; + struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip); int rc; + cancel_delayed_work_sync(&hda->jack_work); rc = pm_runtime_force_suspend(dev); if (rc < 0) return rc; @@ -140,6 +146,8 @@ static int __maybe_unused hda_tegra_suspend(struct device *dev) static int __maybe_unused hda_tegra_resume(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); + struct azx *chip = card->private_data; + struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip); int rc; rc = pm_runtime_force_resume(dev); @@ -147,6 +155,8 @@ static int __maybe_unused hda_tegra_resume(struct device *dev) return rc; snd_power_change_state(card, SNDRV_CTL_POWER_D0); + schedule_delayed_work(&hda->jack_work, JACKPOLL_INTERVAL); + return 0; } @@ -209,6 +219,29 @@ static const struct dev_pm_ops hda_tegra_pm = { NULL) }; +static void hda_tegra_jack_work(struct work_struct *work) +{ + struct hda_tegra *hda = + container_of(work, struct hda_tegra, jack_work.work); + struct azx *chip = &hda->chip; + struct hda_codec *codec; + + if (!chip->running) + return; + + list_for_each_codec(codec, &chip->bus) { + if (snd_hdac_is_power_on(&codec->core)) + continue; + + snd_hda_power_up_pm(codec); + snd_hda_jack_set_dirty_all(codec); + snd_hda_jack_poll_all(codec); + snd_hda_power_down_pm(codec); + } + + schedule_delayed_work(&hda->jack_work, JACKPOLL_INTERVAL); +} + static int hda_tegra_dev_disconnect(struct snd_device *device) { struct azx *chip = device->device_data; @@ -226,6 +259,7 @@ static int hda_tegra_dev_free(struct snd_device *device) struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip); cancel_work_sync(&hda->probe_work); + cancel_delayed_work_sync(&hda->jack_work); if (azx_bus(chip)->chip_init) { azx_stop_all_streams(chip); azx_stop_chip(chip); @@ -428,6 +462,7 @@ static int hda_tegra_create(struct snd_card *card, chip->snoop = true; INIT_WORK(&hda->probe_work, hda_tegra_probe_work); + INIT_DELAYED_WORK(&hda->jack_work, hda_tegra_jack_work); err = azx_bus_init(chip, NULL); if (err < 0) @@ -574,13 +609,18 @@ static void hda_tegra_probe_work(struct work_struct *work) out_free: pm_runtime_put(hda->dev); + schedule_delayed_work(&hda->jack_work, JACKPOLL_INTERVAL); return; /* no error return from async probe */ } static int hda_tegra_remove(struct platform_device *pdev) { + struct snd_card *card = dev_get_drvdata(&pdev->dev); + struct azx *chip = card->private_data; + struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip); int ret; + cancel_delayed_work_sync(&hda->jack_work); ret = snd_card_free(dev_get_drvdata(&pdev->dev)); pm_runtime_disable(&pdev->dev); @@ -591,10 +631,13 @@ static void hda_tegra_shutdown(struct platform_device *pdev) { struct snd_card *card = dev_get_drvdata(&pdev->dev); struct azx *chip; + struct hda_tegra *hda; if (!card) return; chip = card->private_data; + hda = container_of(chip, struct hda_tegra, chip); + cancel_delayed_work_sync(&hda->jack_work); if (chip && chip->running) azx_stop_chip(chip); }
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> --- sound/pci/hda/hda_tegra.c | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)