At Mon, 30 Mar 2015 02:47:45 +0000, Yang, Libin wrote:
Hi Takashi,
-----Original Message----- From: Yang, Libin Sent: Friday, March 27, 2015 4:34 PM To: Takashi Iwai Cc: alsa-devel@alsa-project.org Subject: RE: [alsa-devel] [PATCH] ALSA: hda_intel: add AZX_DCAPS_I915_POWERWELL for skl
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 27, 2015 4:30 PM To: Yang, Libin Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda_intel: add AZX_DCAPS_I915_POWERWELL for skl
At Fri, 27 Mar 2015 08:25:52 +0000, Yang, Libin wrote:
Hi Takashi,
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel- bounces@alsa-project.org] On Behalf Of Takashi Iwai Sent: Friday, March 27, 2015 4:19 PM To: Yang, Libin Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH] ALSA: hda_intel: add AZX_DCAPS_I915_POWERWELL for skl
At Fri, 27 Mar 2015 08:02:54 +0000, Yang, Libin wrote:
Hi Takashi,
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Friday, March 27, 2015 3:57 PM > To: Yang, Libin > Cc: alsa-devel@alsa-project.org > Subject: Re: [PATCH] ALSA: hda_intel: add > AZX_DCAPS_I915_POWERWELL for skl > > At Fri, 27 Mar 2015 15:10:04 +0800, > libin.yang@intel.com wrote: > > > > From: Libin Yang libin.yang@intel.com > > > > HDMI/DP codec on SKL is in the power well. > > The power well must be turned on before probing the > > HDMI/DP codec. > > > > Signed-off-by: Libin Yang libin.yang@intel.com > > So, was the previous question clarified?
Yes, I have confirmed with our silicon team.
> > This certainly sucks. It means that the powerwell is on even
you
> don't use the HDMI/DP at all. If this is intended as a
temporarily
> workaround, it should be mentioned so. Please give more
comments
> and > backgrounds.
Yes, as this is added in the skl audio controller, even there is no
HDMI/DP
codec, we should also add this flag. Otherwise the HDMI/DP
codec
may not be detected correctly.
But it's possible to do it only at probing, not permanently. If so, we'll have another patch in future.
Please write more information in the changelog and resubmit.
Do you mean to add more description in the patch comments?
Yes. The hardware design is different from HSW/BDW, thus applying this isn't straightforward but just a workaround. I don't know whether you think it's a temporary workaround or a permanent fix. Such information must be written there, too.
OK. I see. It seems we need more input from our silicon team for this issue.
From our silicon team's comment, it seems we need power on
the powerwell when detecting and probing the HDMI codec.
For the skl case (HDMI codec is in powerwell, while controller not), my thinking is:
- power on the power well
- read STATESTS to determine the codec_mask
- if codec is not present, power off the power well and remove the flag. (seems we need a new flag)
- if codec is present, we will power on the power well when dealing with HDMI codec.
What do you think?
Yes, it's also similar to my plan. My plan has a bit more code changes for adapting to the new structure:
- Move hda_i915.c code into sound/hda to be used for both legacy and new drivers
- Add a reference counter to get/put, so that it can be called from both controller and codec drivers
- Move the component from struct hda_intel to hdac_bus, but dynamically allocating. The helper code can check it like
if (bus->audio_component) bus->audio_component->ops->get_power(...)
(Or the NULL check can be in a helper function)
- get_power() is called at probing as is now
- put_power() is called at the end of azx_probe*()
- There will be two DCAPS flags, one indicating for the old chips that need get_power() for runtime PM, and one indicating get_power() only for probing.
Now an open question is how to make codec driver to call get_power / put_power. To be more clear, currently there is no solid way to allow the codec driver doing something special before / after hda_set_power_state(). One may implement a tricky set_power_state() ops in the codec driver. Or, we may reimplement pre_resume and post_suspend patch_ops again.
Takashi