drm/i915: set minimum CD clock to twice the BCLK.

Submitted by Kumar, Abhay on Oct. 25, 2017, 10:02 p.m.

Details

Message ID 1508968932-32208-1-git-send-email-abhay.kumar@intel.com
State New
Series "drm/i915: set minimum CD clock to twice the BCLK."
Headers show

Commit Message

Kumar, Abhay Oct. 25, 2017, 10:02 p.m.
From: Abhay Kumar <abhay.kumar@intel.com>

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 chagne will ensure CD clock to be twice of  BCLK.

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(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index e8884c2ade98..185a70f0921c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1920,7 +1920,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);
 
 	if (min_cdclk > dev_priv->max_cdclk_freq) {

Comments

Dhinakaran Pandiyan Oct. 26, 2017, 4:03 a.m.
On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> 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 chagne will ensure CD clock to be twice of  BCLK.
> 
> 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 e8884c2ade98..185a70f0921c
> 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1920,7 +1920,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)

Why should cdclk be increased when audio is not being enabled?

>  		min_cdclk = max(2 * 96000, min_cdclk);
> 
>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
Jani Nikula Oct. 26, 2017, 8:45 a.m.
On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:
> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:
>> From: Abhay Kumar <abhay.kumar@intel.com>
>> 
>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
>> This result in no audio forever as cdclk is < 96Mhz.

Forever... or until next modeset with audio enabled?

>> This chagne will ensure CD clock to be twice of  BCLK.
>> 
>> 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 e8884c2ade98..185a70f0921c
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>> @@ -1920,7 +1920,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)
>
> Why should cdclk be increased when audio is not being enabled?

Indeed. I can easily imagine a counter-bug reporting excessive cdclk
when audio is not enabled.

BR,
Jani.

>
>>  		min_cdclk = max(2 * 96000, min_cdclk);
>> 
>>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kumar, Abhay Oct. 26, 2017, 7:10 p.m.
On 10/26/2017 1:45 AM, Jani Nikula wrote:
> On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:
>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:
>>> From: Abhay Kumar <abhay.kumar@intel.com>
>>>
>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
>>> This result in no audio forever as cdclk is < 96Mhz.
> Forever... or until next modeset with audio enabled?
Soundcard probing/detection and creation happens only during bootup.  So 
even though we do modeset later there is no soundcard driver to handle 
the event.
>
>>> This chagne will ensure CD clock to be twice of  BCLK.
>>>
>>> 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 e8884c2ade98..185a70f0921c
>>> 100644
>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>>> @@ -1920,7 +1920,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)
>> Why should cdclk be increased when audio is not being enabled?
> Indeed. I can easily imagine a counter-bug reporting excessive cdclk
> when audio is not enabled.
During bootup time audio driver is trying to acquire HDA audio power 
well inside i915 and then it will send HDA verb commands.
since cdclk is lower than 96Mhz  HDA will not comeup resulting in 
timeout.  This was working fine  before SKL/APL since there was no 2 PPC .

Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to 
needed CDCLK?
wondering if this approach can cause any issue to subsequent HDA verb 
commands ..


>
> BR,
> Jani.
>
>>>   		min_cdclk = max(2 * 96000, min_cdclk);
>>>
>>>   	if (min_cdclk > dev_priv->max_cdclk_freq) {
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kumar, Abhay Oct. 29, 2017, 3:04 a.m.
+ Subhransu

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Kumar, Abhay

Sent: Thursday, October 26, 2017 12:10 PM
To: Jani Nikula <jani.nikula@linux.intel.com>; Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>; subransu.s.prusty@intel.com
Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.



On 10/26/2017 1:45 AM, Jani Nikula wrote:
> On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:

>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:

>>> From: Abhay Kumar <abhay.kumar@intel.com>

>>>

>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.

>>> This result in no audio forever as cdclk is < 96Mhz.

> Forever... or until next modeset with audio enabled?


Soundcard probing/detection and creation happens only during bootup.  So even though we do modeset later there is no soundcard driver to handle the event.
>

>>> This chagne will ensure CD clock to be twice of  BCLK.

>>>

>>> 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 

>>> e8884c2ade98..185a70f0921c

>>> 100644

>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c

>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c

>>> @@ -1920,7 +1920,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)

>> Why should cdclk be increased when audio is not being enabled?

> Indeed. I can easily imagine a counter-bug reporting excessive cdclk 

> when audio is not enabled.

During bootup time audio driver is trying to acquire HDA audio power well inside i915 and then it will send HDA verb commands.
since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.  This was working fine  before SKL/APL since there was no 2 PPC .

Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to needed CDCLK?
wondering if this approach can cause any issue to subsequent HDA verb commands ..


>

> BR,

> Jani.

>

>>>   		min_cdclk = max(2 * 96000, min_cdclk);

>>>

>>>   	if (min_cdclk > dev_priv->max_cdclk_freq) {

>>

>> _______________________________________________

>> Intel-gfx mailing list

>> Intel-gfx@lists.freedesktop.org

>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Oct. 31, 2017, 12:21 a.m.
On Sun, 2017-10-29 at 03:04 +0000, Kumar, Abhay wrote:
> + Subhransu

> 

> -----Original Message-----

> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Kumar, Abhay

> Sent: Thursday, October 26, 2017 12:10 PM

> To: Jani Nikula <jani.nikula@linux.intel.com>; Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>; subransu.s.prusty@intel.com

> Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>

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

> 

> 

> 

> On 10/26/2017 1:45 AM, Jani Nikula wrote:

> > On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:

> >> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:

> >>> From: Abhay Kumar <abhay.kumar@intel.com>

> >>>

> >>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.

> >>> This result in no audio forever as cdclk is < 96Mhz.

> > Forever... or until next modeset with audio enabled?

> 

> Soundcard probing/detection and creation happens only during bootup.  So even though we do modeset later there is no soundcard driver to handle the event.

> >

> >>> This chagne will ensure CD clock to be twice of  BCLK.

> >>>

> >>> 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 

> >>> e8884c2ade98..185a70f0921c

> >>> 100644

> >>> --- a/drivers/gpu/drm/i915/intel_cdclk.c

> >>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c

> >>> @@ -1920,7 +1920,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)

> >> Why should cdclk be increased when audio is not being enabled?

> > Indeed. I can easily imagine a counter-bug reporting excessive cdclk 

> > when audio is not enabled.

> During bootup time audio driver is trying to acquire HDA audio power well inside i915 and then it will send HDA verb commands.

> since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.  This was working fine  before SKL/APL since there was no 2 PPC .

> 

> Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to needed CDCLK?


I think it is worth exploring, do you have code to test whether it
solves this particular issue?

> wondering if this approach can cause any issue to subsequent HDA verb commands ..

> 

> 

> >

> > BR,

> > Jani.

> >

> >>>   		min_cdclk = max(2 * 96000, min_cdclk);

> >>>

> >>>   	if (min_cdclk > dev_priv->max_cdclk_freq) {

> >>

> >> _______________________________________________

> >> Intel-gfx mailing list

> >> Intel-gfx@lists.freedesktop.org

> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kumar, Abhay Oct. 31, 2017, 11:43 p.m.
On 10/30/2017 5:21 PM, Pandiyan, Dhinakaran wrote:
> On Sun, 2017-10-29 at 03:04 +0000, Kumar, Abhay wrote:
>> + Subhransu
>>
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Kumar, Abhay
>> Sent: Thursday, October 26, 2017 12:10 PM
>> To: Jani Nikula <jani.nikula@linux.intel.com>; Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>; subransu.s.prusty@intel.com
>> Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
>>
>>
>>
>> On 10/26/2017 1:45 AM, Jani Nikula wrote:
>>> On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:
>>>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:
>>>>> From: Abhay Kumar <abhay.kumar@intel.com>
>>>>>
>>>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
>>>>> This result in no audio forever as cdclk is < 96Mhz.
>>> Forever... or until next modeset with audio enabled?
>> Soundcard probing/detection and creation happens only during bootup.  So even though we do modeset later there is no soundcard driver to handle the event.
>>>>> This chagne will ensure CD clock to be twice of  BCLK.
>>>>>
>>>>> 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
>>>>> e8884c2ade98..185a70f0921c
>>>>> 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>>>>> @@ -1920,7 +1920,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)
>>>> Why should cdclk be increased when audio is not being enabled?
>>> Indeed. I can easily imagine a counter-bug reporting excessive cdclk
>>> when audio is not enabled.
>> During bootup time audio driver is trying to acquire HDA audio power well inside i915 and then it will send HDA verb commands.
>> since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.  This was working fine  before SKL/APL since there was no 2 PPC .
>>
>> Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to needed CDCLK?
> I think it is worth exploring, do you have code to test whether it
> solves this particular issue?
No i don't have test code for this but what i learned from other OS that 
glk run at 148000 and cnl 96000*2 due to this limitation all the time.

@Shubhransu : can you please answer this doubt which we all have. This 
we should be able to get from HDA specs.

>
>> wondering if this approach can cause any issue to subsequent HDA verb commands ..
>>
>>
>>> BR,
>>> Jani.
>>>
>>>>>    		min_cdclk = max(2 * 96000, min_cdclk);
>>>>>
>>>>>    	if (min_cdclk > dev_priv->max_cdclk_freq) {
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Nov. 1, 2017, 9:40 a.m.
On Tue, 31 Oct 2017, "Kumar, Abhay" <abhay.kumar@intel.com> wrote:
> On 10/30/2017 5:21 PM, Pandiyan, Dhinakaran wrote:
>> On Sun, 2017-10-29 at 03:04 +0000, Kumar, Abhay wrote:
>>> + Subhransu
>>>
>>> -----Original Message-----
>>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Kumar, Abhay
>>> Sent: Thursday, October 26, 2017 12:10 PM
>>> To: Jani Nikula <jani.nikula@linux.intel.com>; Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com>; subransu.s.prusty@intel.com
>>> Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana <sathyanarayana.nujella@intel.com>
>>> Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
>>>
>>>
>>>
>>> On 10/26/2017 1:45 AM, Jani Nikula wrote:
>>>> On Wed, 25 Oct 2017, Dhinakaran Pandiyan <dhinakaran.pandiyan@gmail.com> wrote:
>>>>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com wrote:
>>>>>> From: Abhay Kumar <abhay.kumar@intel.com>
>>>>>>
>>>>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
>>>>>> This result in no audio forever as cdclk is < 96Mhz.
>>>> Forever... or until next modeset with audio enabled?
>>> Soundcard probing/detection and creation happens only during bootup.  So even though we do modeset later there is no soundcard driver to handle the event.
>>>>>> This chagne will ensure CD clock to be twice of  BCLK.
>>>>>>
>>>>>> 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
>>>>>> e8884c2ade98..185a70f0921c
>>>>>> 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>>>>>> @@ -1920,7 +1920,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)
>>>>> Why should cdclk be increased when audio is not being enabled?
>>>> Indeed. I can easily imagine a counter-bug reporting excessive cdclk
>>>> when audio is not enabled.
>>> During bootup time audio driver is trying to acquire HDA audio power well inside i915 and then it will send HDA verb commands.
>>> since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.  This was working fine  before SKL/APL since there was no 2 PPC .
>>>
>>> Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to needed CDCLK?
>> I think it is worth exploring, do you have code to test whether it
>> solves this particular issue?
> No i don't have test code for this but what i learned from other OS that 
> glk run at 148000 and cnl 96000*2 due to this limitation all the time.

Is there an HSD for this? It's a bit surprising you can't even probe the
driver without a higher cdclk.

BR,
Jani.

>
> @Shubhransu : can you please answer this doubt which we all have. This 
> we should be able to get from HDA specs.
>
>>
>>> wondering if this approach can cause any issue to subsequent HDA verb commands ..
>>>
>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>>>    		min_cdclk = max(2 * 96000, min_cdclk);
>>>>>>
>>>>>>    	if (min_cdclk > dev_priv->max_cdclk_freq) {
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Prusty, Subhransu S Nov. 3, 2017, 3:54 a.m.
> >>>>>> b/drivers/gpu/drm/i915/intel_cdclk.c index
> >>>>>> e8884c2ade98..185a70f0921c
> >>>>>> 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> >>>>>> @@ -1920,7 +1920,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)
> >>>>> Why should cdclk be increased when audio is not being enabled?
> >>>> Indeed. I can easily imagine a counter-bug reporting excessive cdclk
> >>>> when audio is not enabled.
> >>> During bootup time audio driver is trying to acquire HDA audio power well
> inside i915 and then it will send HDA verb commands.
> >>> since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.
> This was working fine  before SKL/APL since there was no 2 PPC .
> >>>
> >>> Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to
> needed CDCLK?
> >> I think it is worth exploring, do you have code to test whether it
> >> solves this particular issue?
> > No i don't have test code for this but what i learned from other OS that
> > glk run at 148000 and cnl 96000*2 due to this limitation all the time.
> 
> Is there an HSD for this? It's a bit surprising you can't even probe the
> driver without a higher cdclk.

Hi Jani,

The driver probe happens based on vendor id and revision id read from the codec, and
the vendor/revision are read from the codec over HDA bus. 

Since codecs are enumerable, without successful match of vendor/revision id  the driver
probe will not happen.

Regards,
Subhransu

> 
> BR,
> Jani.
> 
> >
> > @Shubhransu : can you please answer this doubt which we all have. This
> > we should be able to get from HDA specs.
> >
> >>
> >>> wondering if this approach can cause any issue to subsequent HDA verb
> commands ..
> >>>
> >>>
> >>>> BR,
> >>>> Jani.
> >>>>
> >>>>>>    		min_cdclk = max(2 * 96000, min_cdclk);
> >>>>>>
> >>>>>>    	if (min_cdclk > dev_priv->max_cdclk_freq) {
> >>>>> _______________________________________________
> >>>>> Intel-gfx mailing list
> >>>>> Intel-gfx@lists.freedesktop.org
> >>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>> _______________________________________________
> >>> Intel-gfx mailing list
> >>> Intel-gfx@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 
> --
> Jani Nikula, Intel Open Source Technology Center
Prusty, Subhransu S Nov. 3, 2017, 4:13 a.m.
> 

> On 10/30/2017 5:21 PM, Pandiyan, Dhinakaran wrote:

> > On Sun, 2017-10-29 at 03:04 +0000, Kumar, Abhay wrote:

> >> + Subhransu

> >>

> >> -----Original Message-----

> >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of

> Kumar, Abhay

> >> Sent: Thursday, October 26, 2017 12:10 PM

> >> To: Jani Nikula <jani.nikula@linux.intel.com>; Dhinakaran Pandiyan

> <dhinakaran.pandiyan@gmail.com>; subransu.s.prusty@intel.com

> >> Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana

> <sathyanarayana.nujella@intel.com>

> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the

> BCLK.

> >>

> >>

> >>

> >> On 10/26/2017 1:45 AM, Jani Nikula wrote:

> >>> On Wed, 25 Oct 2017, Dhinakaran Pandiyan

> <dhinakaran.pandiyan@gmail.com> wrote:

> >>>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.kumar@intel.com

> wrote:

> >>>>> From: Abhay Kumar <abhay.kumar@intel.com>

> >>>>>

> >>>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't

> comeup.

> >>>>> This result in no audio forever as cdclk is < 96Mhz.

> >>> Forever... or until next modeset with audio enabled?

> >> Soundcard probing/detection and creation happens only during bootup.  So

> even though we do modeset later there is no soundcard driver to handle the

> event.

> >>>>> This chagne will ensure CD clock to be twice of  BCLK.

> >>>>>

> >>>>> 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

> >>>>> e8884c2ade98..185a70f0921c

> >>>>> 100644

> >>>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c

> >>>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c

> >>>>> @@ -1920,7 +1920,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)

> >>>> Why should cdclk be increased when audio is not being enabled?

> >>> Indeed. I can easily imagine a counter-bug reporting excessive cdclk

> >>> when audio is not enabled.

> >> During bootup time audio driver is trying to acquire HDA audio power well

> inside i915 and then it will send HDA verb commands.

> >> since cdclk is lower than 96Mhz  HDA will not comeup resulting in timeout.

> This was working fine  before SKL/APL since there was no 2 PPC .

> >>

> >> Is it ok to bump  up cdclk while bootup of system/HDA and then reduce to

> needed CDCLK?

> > I think it is worth exploring, do you have code to test whether it

> > solves this particular issue?

> No i don't have test code for this but what i learned from other OS that

> glk run at 148000 and cnl 96000*2 due to this limitation all the time.

> 

> @Shubhransu : can you please answer this doubt which we all have. This

> we should be able to get from HDA specs.


This clock setting is specific to idisp codec and HDA spec may not have details on this.
Yes we can test the changes with skylake audio driver. I believe Sathya has already
tested with the changes.

Regards,
Subhransu

> 

> >

> >> wondering if this approach can cause any issue to subsequent HDA verb

> commands ..

> >>

> >>

> >>> BR,

> >>> Jani.

> >>>

> >>>>>    		min_cdclk = max(2 * 96000, min_cdclk);

> >>>>>

> >>>>>    	if (min_cdclk > dev_priv->max_cdclk_freq) {

> >>>> _______________________________________________

> >>>> Intel-gfx mailing list

> >>>> Intel-gfx@lists.freedesktop.org

> >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

> >> _______________________________________________

> >> Intel-gfx mailing list

> >> Intel-gfx@lists.freedesktop.org

> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Feb. 14, 2018, 5:59 p.m.
On Wed, 25 Oct 2017, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
>
> 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 chagne will ensure CD clock to be twice of  BCLK.

So this issue was never resolved was it?

Summing up, I think the ideas for solution were in order of preference:

1) Tie higher cdclk requirement to audio component get/put power
   callbacks, and bump up cdclk when audio requests power

2) Bump up cdclk during i915 probe, after that require higher cdclk only
   when has_audio is true

3) Require higher cdclk whenever there are display outputs that are
   capable of hda

4) Always require higher cdclk (this patch)

BR,
Jani.


>
> 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 e8884c2ade98..185a70f0921c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1920,7 +1920,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);
>  
>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
Kumar, Abhay March 23, 2018, 7:33 p.m.
On 3/23/2018 12:21 PM, Kumar, Abhay wrote:
>
> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
> Sent: Wednesday, February 14, 2018 10:00 AM
> To: Kumar, Abhay <abhay.kumar@intel.com>; Intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
>
> On Wed, 25 Oct 2017, abhay.kumar@intel.com wrote:
>> From: Abhay Kumar <abhay.kumar@intel.com>
>>
>> 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 chagne will ensure CD clock to be twice of  BCLK.
> So this issue was never resolved was it?
>
> Summing up, I think the ideas for solution were in order of preference:
>
> 1) Tie higher cdclk requirement to audio component get/put power
>     callbacks, and bump up cdclk when audio requests power
>
> 2) Bump up cdclk during i915 probe, after that require higher cdclk only
>     when has_audio is true
>
> 3) Require higher cdclk whenever there are display outputs that are
>     capable of hda
>
> 4) Always require higher cdclk (this patch)
>
> BR,
> Jani.

First of all sorry for addressing this patch late as i got tied up in 
something else.

Yes this bug is still there and never resolved.

we can also honour VBT field which says enable or disable Dynamic cdclk 
and control this from BIOS.

Let me figure out best way and comeup with patch.
>
>> 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 e8884c2ade98..185a70f0921c 100644
>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>> @@ -1920,7 +1920,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);
>>   
>>   	if (min_cdclk > dev_priv->max_cdclk_freq) {