Re: [PATCH v3] drm/i915: set minimum CD clock to twice the BCLK.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 18, 2018 at 01:49:23PM +0300, Jani Nikula wrote:
> On Tue, 17 Apr 2018, "Kumar, Abhay" <abhay.kumar@intel.com> wrote:
> > On 4/17/2018 12:06 PM, Abhay Kumar wrote:
> >> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
> >> This result in no audio forever as cdclk is < 96Mhz.
> >> This change will ensure CD clock to be twice of  BCLK.
> >>
> >> v2:
> >>      - Address comment (Jani)
> >>      - New design approach
> >> v3: - Typo fix on top of v1
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
> >> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> >> index dc7db8a2caf8..6e93af4a46ea 100644
> >> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> >> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> >> @@ -2143,7 +2143,7 @@ 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
> >>   	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> >>   	 */
> >> -	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
> >> +	if (INTEL_GEN(dev_priv) >= 9)
> >>   		min_cdclk = max(2 * 96000, min_cdclk);
> >>   
> >>   	/*
> > Hi Ville, Jani,
> >
> >     Since right version of this patch is taking time(doing modeset and 
> > cdclk bump) as we need to poke sound driver. Can we please get this 
> > v3(same as v1 with typo fix in comment) version of patch merged.
> > This works all the time and will unblock Audio and lot of folks. without 
> > this patch audio card is not getting detected at all and blocking basic 
> > audio feature.
> 
> I expanded on the code comment, rewrote the commit message, added cc:
> stable, and resent the patch [1].
> 
> It's not a patch I much like, it's not even a complete fix, and I would
> like this to be addressed properly going forward. However, the proper
> fix is I think too invasive for stable, so here we are. I'm trying to at
> least explain in the comment and commit message what's going on, for
> posterity.
> 
> Ville, I'm not going to push the patch without your ack, but we can't
> sit on this forever either. The patch papers over the most common
> failure case, for now, so perhaps it'll buy us time to find a proper
> solution.

While I don't particularly like this patch I also agree that it's the
least risky way to go for stable. So

Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Abhay, if we merge this, I do expect your support in figuring out and
> testing the proper fix. This is not it.

I also want to see a better solution going forward.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx