Re: [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"  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 
---
   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ä 


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.




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Wed, Apr 18, 2018 at 01:49:23PM +0300, Jani Nikula wrote:
> On Tue, 17 Apr 2018, "Kumar, Abhay"  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 
> >> ---
> >>   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ä 

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

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

[Jani Nikula]

On Tue, 17 Apr 2018, "Kumar, Abhay"  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 
>> ---
>>   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.

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


BR,
Jani.


[1] 
http://patchwork.freedesktop.org/patch/msgid/20180418103707.14645-1-jani.nik...@intel.com


>
> Regards,
> Abhay
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Du,Wenkai]

On 4/17/2018 12:17 PM, Kumar, Abhay 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 

Reviewed-by: Wenkai Du 
Tested-by: Wenkai Du 

Regards,
Wenkai

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


Regards,
Abhay

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kumar, Abhay]

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


Regards,
Abhay
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Abhay Kumar]

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 
---
 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);
 
/*
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx