[SRU,D/OEM-B-OSP1,2/2] UBUNTU: SAUCE: drm/i915: extend audio CDCLK>=2*BCLK constraint to more platforms
diff mbox series

Message ID 20191008115552.10791-3-hui.wang@canonical.com
State New
Headers show
Series
  • drm/i915: Fix the issue of "azx_get_response timeout" for hdmi audio on ICL platforms
Related show

Commit Message

Hui Wang Oct. 8, 2019, 11:55 a.m. UTC
From: Kai Vehmanen <kai.vehmanen@linux.intel.com>

BugLink: https://bugs.launchpad.net/bugs/1847192

The CDCLK>=2*BCLK constraint applies to all generations since gen10.
Extend the constraint logic in audio get/put_power().

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191003085531.30990-2-kai.vehmanen@linux.intel.com
(backported from commit f6ec9483091f8e67adab0311a4e2f90aab523310
git://anongit.freedesktop.org/drm-intel)
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stefan Bader Oct. 8, 2019, 12:29 p.m. UTC | #1
On 08.10.19 13:55, Hui Wang wrote:
> From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1847192
> 
> The CDCLK>=2*BCLK constraint applies to all generations since gen10.
> Extend the constraint logic in audio get/put_power().
> 
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20191003085531.30990-2-kai.vehmanen@linux.intel.com
> (backported from commit f6ec9483091f8e67adab0311a4e2f90aab523310
> git://anongit.freedesktop.org/drm-intel)
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 1661a8606017..57346bf1342e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -794,7 +794,7 @@ static void i915_audio_component_get_power(struct device *kdev)
>  
>  	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
>  	if (dev_priv->audio_power_refcount++ == 0) {
> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))

This second patch replaces an IS_CANONLAKE test with a INTEL_GEN test, while the
previous one added and additional section. I have no way to judge whether this
is ok or not, but it looks odd.


>  			glk_force_audio_cdclk(dev_priv, true);
>  
>  		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> @@ -810,7 +810,7 @@ static void i915_audio_component_put_power(struct device *kdev)
>  
>  	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
>  	if (--dev_priv->audio_power_refcount == 0)
> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>  			glk_force_audio_cdclk(dev_priv, false);
>  
>  	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
>
Timo Aaltonen Oct. 9, 2019, 4:39 a.m. UTC | #2
On 8.10.2019 15.29, Stefan Bader wrote:
> On 08.10.19 13:55, Hui Wang wrote:
>> From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1847192
>>
>> The CDCLK>=2*BCLK constraint applies to all generations since gen10.
>> Extend the constraint logic in audio get/put_power().
>>
>> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/20191003085531.30990-2-kai.vehmanen@linux.intel.com
>> (backported from commit f6ec9483091f8e67adab0311a4e2f90aab523310
>> git://anongit.freedesktop.org/drm-intel)
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>  drivers/gpu/drm/i915/intel_audio.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> index 1661a8606017..57346bf1342e 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -794,7 +794,7 @@ static void i915_audio_component_get_power(struct device *kdev)
>>  
>>  	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
>>  	if (dev_priv->audio_power_refcount++ == 0) {
>> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> 
> This second patch replaces an IS_CANONLAKE test with a INTEL_GEN test, while the
> previous one added and additional section. I have no way to judge whether this
> is ok or not, but it looks odd.

Right, these sections could've been combined but this is how it got
merged upstream..

and CANNONLAKE == gen10, so the check still applies to it
Hui Wang Oct. 9, 2019, 7:33 a.m. UTC | #3
On 2019/10/9 下午12:39, Timo Aaltonen wrote:
> On 8.10.2019 15.29, Stefan Bader wrote:
>> On 08.10.19 13:55, Hui Wang wrote:
>>> From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1847192
>>>
>>> The CDCLK>=2*BCLK constraint applies to all generations since gen10.
>>> Extend the constraint logic in audio get/put_power().
>>>
>>> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> Link: https://patchwork.freedesktop.org/patch/msgid/20191003085531.30990-2-kai.vehmanen@linux.intel.com
>>> (backported from commit f6ec9483091f8e67adab0311a4e2f90aab523310
>>> git://anongit.freedesktop.org/drm-intel)
>>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_audio.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>> index 1661a8606017..57346bf1342e 100644
>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> @@ -794,7 +794,7 @@ static void i915_audio_component_get_power(struct device *kdev)
>>>   
>>>   	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
>>>   	if (dev_priv->audio_power_refcount++ == 0) {
>>> -		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>>> +		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
>> This second patch replaces an IS_CANONLAKE test with a INTEL_GEN test, while the
>> previous one added and additional section. I have no way to judge whether this
>> is ok or not, but it looks odd.
> Right, these sections could've been combined but this is how it got
> merged upstream..
>
> and CANNONLAKE == gen10, so the check still applies to it

And I found these 2 patches don't need to be merged to Disco kernel, 
since Disco kernel doesn't support ICL graphic, these two patches are 
useless for Disco kernel. And furthermore, if merging these two patches 
to Disco will introduce building failure, because there is one more 
commit needed to fix this building failure.

commit 09b434d4f6d22e14500569e7e3f951e0eec4d496
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Fri Mar 15 15:56:18 2019 +0200

     drm/i915: introduce REG_BIT() and REG_GENMASK() to define register 
contents


So I will resend a V2, and In the V2, will drop Disco.

Thanks,

Hui.


>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 1661a8606017..57346bf1342e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -794,7 +794,7 @@  static void i915_audio_component_get_power(struct device *kdev)
 
 	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
 	if (dev_priv->audio_power_refcount++ == 0) {
-		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 			glk_force_audio_cdclk(dev_priv, true);
 
 		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
@@ -810,7 +810,7 @@  static void i915_audio_component_put_power(struct device *kdev)
 
 	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
 	if (--dev_priv->audio_power_refcount == 0)
-		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 			glk_force_audio_cdclk(dev_priv, false);
 
 	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);