From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:59724 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750881AbeDSJMG (ORCPT ); Thu, 19 Apr 2018 05:12:06 -0400 From: Jani Nikula To: intel-gfx@lists.freedesktop.org Cc: Abhay Kumar , stable@vger.kernel.org, Ville =?utf-8?B?U3lyasOkbMOk?= , Dhinakaran Pandiyan , Wenkai Du Subject: Re: [PATCH] drm/i915/audio: set minimum CD clock to twice the BCLK In-Reply-To: <20180418103707.14645-1-jani.nikula@intel.com> References: <20180418103707.14645-1-jani.nikula@intel.com> Date: Thu, 19 Apr 2018 12:14:48 +0300 Message-ID: <87d0yvfth3.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Wed, 18 Apr 2018, Jani Nikula wrote: > From: Abhay Kumar > > In GLK when the device boots with only 1366x768 panel without audio, HDA > codec doesn't come up. In this case, the CDCLK is less than twice the > BCLK. Even though audio isn't being enabled, having a too low CDCLK > leads to audio probe failing altogether. > > Require CDCLK to be at least twice the BLCK regardless of audio. This is > a minimal fix to improve things. Unfortunately, this a) leads to too > high CDCLK being used when audio is not used, and b) is still not enough > to fix audio probe when no outputs are connected at probe time. > > The proper fix would be to increase CDCLK dynamically from the audio > component hooks. > > v2: > - Address comment (Jani) > - New design approach > v3: - Typo fix on top of v1 > > v4 by Jani: rewrite commit message, add comment in code > > Cc: stable@vger.kernel.org > Cc: Ville Syrjälä > Cc: Dhinakaran Pandiyan > Cc: Wenkai Du > Reviewed-by: Wenkai Du > Tested-by: Wenkai Du > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 > Signed-off-by: Abhay Kumar > Signed-off-by: Jani Nikula Pushed to dinq with Ville's Ack on another thread. BR, Jani. > --- > drivers/gpu/drm/i915/intel_cdclk.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index fc8b2c6e3508..32d24c69da3c 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -2140,10 +2140,22 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) > } > } > > - /* According to BSpec, "The CD clock frequency must be at least twice > + /* > + * According to BSpec, "The CD clock frequency must be at least twice > * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default. > + * > + * FIXME: Check the actual, not default, BCLK being used. > + * > + * FIXME: This does not depend on ->has_audio because the higher CDCLK > + * is required for audio probe, also when there are no audio capable > + * displays connected at probe time. This leads to unnecessarily high > + * CDCLK when audio is not required. > + * > + * FIXME: This limit is only applied when there are displays connected > + * at probe time. If we probe without displays, we'll still end up using > + * the platform minimum CDCLK, failing audio probe. > */ > - if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) > + if (INTEL_GEN(dev_priv) >= 9) > min_cdclk = max(2 * 96000, min_cdclk); > > /* -- Jani Nikula, Intel Open Source Technology Center