Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support

From: Anshuman Gupta <anshuman.gupta@intel.com>
To: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>,
	"Peres, Martin" <martin.peres@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
Date: Mon, 23 Mar 2020 21:03:23 +0530	[thread overview]
Message-ID: <20200323153322.GB7154@intel.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2003231436460.2957@eliteleevi.tm.intel.com>

On 2020-03-23 at 14:48:03 +0200, Kai Vehmanen wrote:
> Hey,
> 
> On Mon, 23 Mar 2020, Anshuman Gupta wrote:
> 
> > On 2020-03-23 at 12:30:08 +0530, Peres, Martin wrote:
> > > On 2020-03-23 08:32, Anshuman Gupta wrote:
> > > > +	/* DP/HDMI panel requires to drive lpsp without audio */
> > > > +	igt_require(unload_snd_hda_core_module());
> > > 
> > > I'm not super happy with having to unload modules that it would be very
> > > easy to forget to re-add and would make audio tests fail :s
> 
> at least this may be painful to maintain. We have three audio drivers in 
> active use (HDA snd-hda-intel, SST DSP and SOF DSP -> drivers is selected 
> at runtime dependsing on hw generation, enabled features and the available 
> DSP FW), and the modules you need to unload are different for all three, 
> and may change over time. So not so nice to maintain if you need to 
> support a large variety of hardware targets.
> 
> > > IMO, if the DUT is not playing sound, we should be able to enter LPSP.
> > > If no, then I would consider this a driver bug for wasting energy
> > > sending 0s to the screen.
> > i915 contorls AUDIO power domain on request of i915_audio_component_ops
> > get_power/put_power, which is an external dependency.
> > Despite unloading the module, it fails to enter lpsp for HDMI panels, 
> > audio driver didn't invoke the put_power to release the i915 power resources.
> > It seems bug with audio driver interface.
> 
> Yes, this sounds like a plain bug. Audio should never keep powers
> on unless it needs it for some actual use-case.
Thanks kai for your comment based upon your comment, i investigated
it and found, while enabling crtc i915 also gets ref count
for AUDIO_POWER_DOMAIN if there is valid ELD.
This was failing the test.
Thanks,
Anshuman Gupta.
> 
> > My plan was to raise a gitlab bug based upon lpsp failure due to
> > AUDIO_POWER_DOMAIN non-zero ref count despite there was no audio 
> > module.
> > > 
> > > Thoughts on this?
> > I agree with you, if there is no audio is being played from DP/hdmi
> > we should be able to enter lpsp, but it seems audio codec requires 
> > power at the time of codec probe itself.
> > @Kai Vehmanen may be the best one to confirm hdmi-codec behavior?
> 
> Yes, audio codec asks for i915 power when it is probed, and also whenever 
> it uses the HDA bus. But when it is idle, it will release i915 power via 
> put_power. One possible problem could be that the audio driver is uilt 
> without runtime pm support. Then it will indeed keep a ref to i915 
> power all the time. Can you provide a bit of data about the 
> case (which hardware, kernel config used, possible dmesg log of error)?
> 
> Br, Kai (Sound Open Firmware (SOF) team)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev