[alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: Enable jack detection in sof hda driver

[Ranjani Sridharan]

On Wed, 2019-06-26 at 23:03 +0200, Cezary Rojewski wrote:
> On 2019-06-26 22:22, Ranjani Sridharan wrote:
> > From: Rander Wang <rander.wang at linux.intel.com>
> > 
> > In commit 7d4f606c50ff ("ALSA: hda - WAKEEN feature enabling for
> > runtime pm"), legacy HD-A driver sets hda controller in reset mode
> > after
> > entering runtime-suspend. And when resuming from suspend mode, it
> > checks
> > hda controller & codec status to detect headphone hotplug event.
> > Now
> > this patch does the same job in SOF runtime pm functions.
> > 
> > And we need to check all the non-hdmi codecs for some cases like
> > playback
> > with HDMI or capture with DMIC connected to dsp. In these cases,
> > only
> > controller is active and codecs are suspended, so codecs can't send
> > unsolicited event to controller. The jack polling operation will
> > activate
> > codecs and unsolicited event can work even codecs become suspended
> > later.
> > 
> > Tested on whiskylake with hda codecs.
> > 
> > Signed-off-by: Rander Wang <rander.wang at linux.intel.com>
> > Signed-off-by: Pierre-Louis Bossart <
> > pierre-louis.bossart at linux.intel.com>
> > Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.com
> > >
> > ---
> >   sound/soc/sof/intel/hda-codec.c | 45
> > +++++++++++++++++++++++++++++++--
> >   sound/soc/sof/intel/hda-dsp.c   | 29 ++++++++++++++++-----
> >   sound/soc/sof/intel/hda.h       |  4 +++
> >   3 files changed, 70 insertions(+), 8 deletions(-)
> > 
> > diff --git a/sound/soc/sof/intel/hda-codec.c
> > b/sound/soc/sof/intel/hda-codec.c
> > index b8b37f082309..c711792534da 100644
> > --- a/sound/soc/sof/intel/hda-codec.c
> > +++ b/sound/soc/sof/intel/hda-codec.c
> > @@ -10,6 +10,7 @@
> >   
> >   #include <linux/module.h>
> >   #include <sound/hdaudio_ext.h>
> > +#include <sound/hda_register.h>
> >   #include <sound/hda_codec.h>
> >   #include <sound/hda_i915.h>
> >   #include <sound/sof.h>
> > @@ -37,16 +38,55 @@ static void hda_codec_load_module(struct
> > hda_codec *codec)
> >   static void hda_codec_load_module(struct hda_codec *codec) {}
> >   #endif
> >   
> > +/* check jack status after resuming from suspend mode */
> > +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status)
> > +{
> > +	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
> > +	struct hda_bus *hbus = sof_to_hbus(sdev);
> > +	struct hdac_bus *bus = sof_to_bus(sdev);
> > +	struct hda_codec *codec;
> > +	int mask;
> > +
> > +	/*
> > +	 * there are two reasons for runtime resume
> > +	 * (1) waken up by interrupt triggered by WAKEEN feature
> > +	 * (2) waken up by pm get functions for some audio operations
> > +	 * For case (1), the bits in status mean which codec triggers
> > +	 * the interrupt and jacks will be checked on these codecs.
> > +	 * For case (2), we need to check all the non-hdmi codecs for
> > some
> > +	 * cases like playback with HDMI or capture with DMIC. In these
> > +	 * cases, only controller is active and codecs are suspended,
> > so
> > +	 * codecs can't send unsolicited event to controller. The jack
> > polling
> > +	 * operation will activate codecs and unsolicited event can
> > work
> > +	 * even codecs become suspended later.
> > +	 */
> 
> This block is huge. I'd suggest entering some whitespaces to make it 
> more readable.
> 
> On the second thought, I bet this stuff is not SOF specific and if
> so, 
> being more strict may prove more informative than being so explicit
> - 
> reference to HDA spec/ kernel HDA documentation etc.
> 
> > +	mask = status ? status : hda->hda_codec_mask;
> > +
> > +	list_for_each_codec(codec, hbus)
> > +		if (mask & BIT(codec->core.addr))
> > +			schedule_delayed_work(&codec->jackpoll_work,
> > +					      codec-
> > >jackpoll_interval);
> > +
> > +	/* disable controller Wake Up event*/
> > +	snd_hdac_chip_writew(bus, WAKEEN,
> > +			     snd_hdac_chip_readw(bus, WAKEEN) &
> > +			     ~hda->hda_codec_mask);
> 
> Any reason for not using snd_hdac_chip_updatew?
Thanks, Cezary for the feedback. We're working to optimize the jack
detection logic during runtime suspend. Let me get back with a v2 and
address your concerns.

Thanks,
Ranjani