lkml.org 
[lkml]   [2018]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [sound] hdac-codec runtime suspended at PM:Suspend.
On Mon, 26 Mar 2018 09:03:55 +0200,
Subhransu S. Prusty wrote:
>
> On Thu, Mar 15, 2018 at 09:02:30PM +0530, Anshuman Gupta wrote:
> > On Thu, Mar 15, 2018 at 04:12:44PM +0530, Subhransu S. Prusty wrote:
> > > On Wed, Mar 14, 2018 at 09:07:14PM +0530, Anshuman Gupta wrote:
> > > > On Wed, Mar 14, 2018 at 11:53:58AM +0100, Rafael J. Wysocki wrote:
> > > > > On Wed, Mar 14, 2018 at 11:38 AM, Anshuman Gupta
> > > > > <anshuman.gupta@intel.com> wrote:
> > > > > > On Mon, Mar 12, 2018 at 12:26:53PM +0100, Rafael J. Wysocki wrote:
> > > > > >> On Mon, Mar 12, 2018 at 12:17 PM, Anshuman Gupta
> > > > > >> <anshuman.gupta@intel.com> wrote:
> > > > > >> >
> > > > > >> > + if (pm_runtime_status_suspended(dev))
> > > > > >> > + return;
> > > > > >>
> > > > > >> That, again, is somewhat fragile from the concurrency perspective.
> > > > > >>
> > > > >
> > > > > And here you want to avoid the below if the device is still suspended.
> > > > Yes, if we do not avoid the code below, complete callback takes about
> > > > 3 seconds due to snd_hdac_codec_read timed out because hdac controller
> > > > would be in runtime suspend state.
> > > > >
> > > > > Why is the below code located in the ->complete callback anyway?
> > > > > Shouldn't it be there in the ->resume one?
> > > > >
> > > > Yes even i am also having same doubt, why these power down and power up
> > > > sequences are part of prepare and complete callback.
> > > > Adding driver author "Subhransu S. Prusty" to provide more inputs on this.
> > >
> > > This driver needs a late resume as it receives a jack notification from the
> > > i915 driver and the skl controller driver resume may not have happened and
> > > in turn hda controller may not ready. This ensures a synchronization for
> > > jack event during resume from S3.
> > Let me give you insight of the issue, this driver blocks the direct complete
> > of hda controller PCI 00:1f.3, sometimes it takes 280ms to resume hda controller
> > from S3 and s2idle. So it does not make sense to suspend/resume hda controller
> > when it is already in runtime suspend.
>
> I get it now. I think this patch needs rework to address both the issues.
>
> > >
> > > I think this patch defeats the purpose.
> > Here in this case PCI driver may kick the direct complete for hda controller
> > https://elixir.bootlin.com/linux/latest/source/drivers/pci/pci-driver.c#L691
> > But hdac hdmi codec driver is blocking it.
> > So i think it will be ok to keep this codec and hda controller in runtime
> > suspend while entering S3 or s2idle, both can resume by runtime PM as well,
> > will it brake any audio functionality?
>
> There was some PM and jack detection issues (I don't recollect now) because
> of which this was added. This was due to the gfx display power and hda
> controller synchronization. The rework may be required in both hdac_hdmi
> and skylake drivers.

If it's about the power well issue, it had been fixed in multiple
ways. In i915 side, every relevant component callback is wrapped with
power domain get/put. And, at least HD-audio legacy side, such a
spurious notification is filtered out in the eld notifier:

static void intel_pin_eld_notify(void *audio_ptr, int port, int pipe)
{
....
/* skip notification during system suspend (but not in runtime PM);
* the state will be updated at resume
*/
if (snd_power_get_state(codec->card) != SNDRV_CTL_POWER_D0)
return;
/* ditto during suspend/resume process itself */
if (atomic_read(&(codec)->core.in_pm))
return;

snd_hdac_i915_set_bclk(&codec->bus->core);
check_presence_and_report(codec, pin_nid, dev_id);
}

Last but not least, the jack state is explicitly updated via reading
the ELD at resume callback.

In that way, we can live with the standard suspend/resume procedure.


Takashi

\
 
 \ /
  Last update: 2018-03-26 09:31    [W:0.044 / U:7.924 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site