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

[Kumar, Abhay]

On 4/18/2018 8:41 AM, Ville Syrjälä wrote:
> On Wed, Apr 18, 2018 at 01:49:23PM +0300, Jani Nikula wrote:
>> On Tue, 17 Apr 2018, "Kumar, Abhay" <abhay.kumar at 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 at 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 at 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.

Yes will work on this.
>