Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@intel.com>
Cc: libin.yang@intel.com, tiwai@suse.de,
	intel-gfx@lists.freedesktop.org, daniel.vetter@intel.com,
	han.lu@intel.com
Subject: Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup
Date: Mon, 4 May 2015 17:07:11 +0200	[thread overview]
Message-ID: <20150504150711.GT30184@phenom.ffwll.local> (raw)
In-Reply-To: <878udcbafp.fsf@intel.com>

On Tue, Apr 28, 2015 at 05:21:30PM +0300, Jani Nikula wrote:
> On Tue, 28 Apr 2015, han.lu@intel.com wrote:
> > From: "Lu, Han" <han.lu@intel.com>
> >
> > In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
> > power wells, so it's necessary to reset display audio codecs when
> > power well on, otherwise display audio codecs will disappear when
> > resume from low power state.
> > The reset step when power on is:
> >     enable codec wakeup -> azx_init_chip() -> disable codec wakeup
> >
> > The callback is defined in drivers/gpu/drm/i915/.
> > The caller is in sound/pci/hda/.
> 
> Han Lu, two more things:
> 
> First, is there any chance the chicken bit register is mirrored in the
> audio device mmio bar? If yes, you could use it directly in the audio
> driver, and we could drop patch 1/2...
> 
> Second, looking at the audio driver code for the usage of this callback
> in patch 2/2, I don't think the locking and refcounting add any
> value. Especially since you ignore the refcounting for disable. You're
> the only user, and anything beyond that would be broken anyway, with or
> without refcounting or locking. In fact, I'd like to push back the
> locking and refcounting to audio driver side, where you may realize they
> are not needed at all. Thus, I propose this as patch 1/2:
> 
> From b06bf55b94cdbfad8ec35923b2b657674472cce5 Mon Sep 17 00:00:00 2001
> From: "Lu, Han" <han.lu@intel.com>
> Date: Tue, 28 Apr 2015 17:03:08 +0300
> Subject: [PATCH] drm/i915/audio: add codec wakeup override enabled/disable
>  callback
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>
> 
> Add support for enabling codec wakeup override signal to allow
> re-enumeration of the controller on SKL after resume from low power
> state.

This commit message is missing the explanation and specifically the hw
sequence. Lu's original patch has a nice paragraph explaining all that.
With that re-added this is Acked-by: me for merging through the sound tree
(which seems to be the best option here I think).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx