[3/4] drm/i915: Avoid TP3 on CHV

Message ID	1439876279-1546-4-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> To: ville.syrjala@linux.intel.com, jani.nikula@linux.intel.com, intel-gfx@lists.freedesktop.org Date: Tue, 18 Aug 2015 11:07:58 +0530 Message-Id: <1439876279-1546-4-git-send-email-sivakumar.thulasimani@intel.com> In-Reply-To: <1439876279-1546-1-git-send-email-sivakumar.thulasimani@intel.com> References: <1439876279-1546-1-git-send-email-sivakumar.thulasimani@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Avoid TP3 on CHV Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Message ID

1439876279-1546-4-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive)

State

New, archived

Headers

From: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
To: ville.syrjala@linux.intel.com, jani.nikula@linux.intel.com,
	intel-gfx@lists.freedesktop.org
Date: Tue, 18 Aug 2015 11:07:58 +0530
Message-Id: <1439876279-1546-4-git-send-email-sivakumar.thulasimani@intel.com>
In-Reply-To: <1439876279-1546-1-git-send-email-sivakumar.thulasimani@intel.com>
References: <1439876279-1546-1-git-send-email-sivakumar.thulasimani@intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH 3/4] drm/i915: Avoid TP3 on CHV
Precedence: list
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Commit Message

Sivakumar Thulasimani Aug. 18, 2015, 5:37 a.m. UTC

From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>

This patch removes TP3 support on CHV since there is no support
for HBR2 on this platform.

v2: rename the function to indicate it checks source rates (Jani)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

Comments

Jani Nikula Aug. 18, 2015, 6:44 a.m. UTC | #1

On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>
> This patch removes TP3 support on CHV since there is no support
> for HBR2 on this platform.
>
> v2: rename the function to indicate it checks source rates (Jani)
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 475d8cb..8bc6361 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>  	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
>  }
>  
> +static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
> +{
> +	/* WaDisableHBR2:skl */
> +	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
> +		return false;
> +
> +	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
> +	    (INTEL_INFO(dev)->gen >= 9))
> +		return true;
> +	else
> +		return false;
> +}
> +
> +
>  static int
>  intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>  {
> @@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>  
>  	*source_rates = default_rates;
>  
> -	/* WaDisableHBR2:skl */
> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
> -		return (DP_LINK_BW_2_7 >> 3) + 1;
> -
> -	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
> -	    (INTEL_INFO(dev)->gen >= 9))
> +	/* This depends on the fact that 5.4 is last value in the array */
> +	if (intel_dp_source_supports_hbr2(dev))
>  		return (DP_LINK_BW_5_4 >> 3) + 1;
>  	else
>  		return (DP_LINK_BW_2_7 >> 3) + 1;
> @@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	/* Training Pattern 3 support, both source and sink */
>  	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
>  	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
> -	    (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
> +	    intel_dp_source_supports_hbr2(dev)) {

hbr2 is not the same as tps3, is it? It's possible to use tps3 without
using hbr2, right?

BR,
Jani.


>  		intel_dp->use_tps3 = true;
>  		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
>  	} else
> -- 
> 1.7.9.5
>

Sivakumar Thulasimani Aug. 18, 2015, 6:56 a.m. UTC | #2

On 8/18/2015 12:14 PM, Jani Nikula wrote:
> On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>
>> This patch removes TP3 support on CHV since there is no support
>> for HBR2 on this platform.
>>
>> v2: rename the function to indicate it checks source rates (Jani)
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c |   24 +++++++++++++++++-------
>>   1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 475d8cb..8bc6361 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>>   	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
>>   }
>>   
>> +static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
>> +{
>> +	/* WaDisableHBR2:skl */
>> +	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>> +		return false;
>> +
>> +	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>> +	    (INTEL_INFO(dev)->gen >= 9))
>> +		return true;
>> +	else
>> +		return false;
>> +}
>> +
>> +
>>   static int
>>   intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>   {
>> @@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>   
>>   	*source_rates = default_rates;
>>   
>> -	/* WaDisableHBR2:skl */
>> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>> -		return (DP_LINK_BW_2_7 >> 3) + 1;
>> -
>> -	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>> -	    (INTEL_INFO(dev)->gen >= 9))
>> +	/* This depends on the fact that 5.4 is last value in the array */
>> +	if (intel_dp_source_supports_hbr2(dev))
>>   		return (DP_LINK_BW_5_4 >> 3) + 1;
>>   	else
>>   		return (DP_LINK_BW_2_7 >> 3) + 1;
>> @@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>   	/* Training Pattern 3 support, both source and sink */
>>   	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
>>   	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
>> -	    (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
>> +	    intel_dp_source_supports_hbr2(dev)) {
> hbr2 is not the same as tps3, is it? It's possible to use tps3 without
> using hbr2, right?
>
> BR,
> Jani.
Yes, TP3 can be supported on panels that does not support HBR2 as well, but
the check here is for hardware capability. Intel platforms that does not 
support
HBR2 cannot support TP3 as well, so we should treat them both the same.
(the only exception here seems to be SKL <B0 where WA is in place)

>
>>   		intel_dp->use_tps3 = true;
>>   		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
>>   	} else
>> -- 
>> 1.7.9.5
>>

Jani Nikula Aug. 18, 2015, 7:12 a.m. UTC | #3

On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> On 8/18/2015 12:14 PM, Jani Nikula wrote:
>> On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>>
>>> This patch removes TP3 support on CHV since there is no support
>>> for HBR2 on this platform.
>>>
>>> v2: rename the function to indicate it checks source rates (Jani)
>>>
>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dp.c |   24 +++++++++++++++++-------
>>>   1 file changed, 17 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 475d8cb..8bc6361 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>>>   	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
>>>   }
>>>   
>>> +static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
>>> +{
>>> +	/* WaDisableHBR2:skl */
>>> +	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>> +		return false;
>>> +
>>> +	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>> +	    (INTEL_INFO(dev)->gen >= 9))
>>> +		return true;
>>> +	else
>>> +		return false;
>>> +}
>>> +
>>> +
>>>   static int
>>>   intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>>   {
>>> @@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>>   
>>>   	*source_rates = default_rates;
>>>   
>>> -	/* WaDisableHBR2:skl */
>>> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>> -		return (DP_LINK_BW_2_7 >> 3) + 1;
>>> -
>>> -	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>> -	    (INTEL_INFO(dev)->gen >= 9))
>>> +	/* This depends on the fact that 5.4 is last value in the array */
>>> +	if (intel_dp_source_supports_hbr2(dev))
>>>   		return (DP_LINK_BW_5_4 >> 3) + 1;
>>>   	else
>>>   		return (DP_LINK_BW_2_7 >> 3) + 1;
>>> @@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>>   	/* Training Pattern 3 support, both source and sink */
>>>   	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
>>>   	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
>>> -	    (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
>>> +	    intel_dp_source_supports_hbr2(dev)) {
>> hbr2 is not the same as tps3, is it? It's possible to use tps3 without
>> using hbr2, right?
>>
>> BR,
>> Jani.
> Yes, TP3 can be supported on panels that does not support HBR2 as well, but
> the check here is for hardware capability. Intel platforms that does not 
> support
> HBR2 cannot support TP3 as well, so we should treat them both the same.
> (the only exception here seems to be SKL <B0 where WA is in place)

Okay, please at least add a comment to that effect.

Alternatively you add a separate intel_dp_source_supports_tps3, and
modify intel_dp_source_supports_hbr2 to be
(intel_dp_source_supports_tps3 && !WaDisableHBR2:skl). *shrug*.

BR,
Jani.



>
>>
>>>   		intel_dp->use_tps3 = true;
>>>   		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
>>>   	} else
>>> -- 
>>> 1.7.9.5
>>>
>
> -- 
> regards,
> Sivakumar
>

Sivakumar Thulasimani Aug. 18, 2015, 11:58 a.m. UTC | #4

On 8/18/2015 12:42 PM, Jani Nikula wrote:
> On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>> On 8/18/2015 12:14 PM, Jani Nikula wrote:
>>> On Tue, 18 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
>>>> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com>
>>>>
>>>> This patch removes TP3 support on CHV since there is no support
>>>> for HBR2 on this platform.
>>>>
>>>> v2: rename the function to indicate it checks source rates (Jani)
>>>>
>>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_dp.c |   24 +++++++++++++++++-------
>>>>    1 file changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>> index 475d8cb..8bc6361 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>> @@ -1207,6 +1207,20 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>>>>    	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
>>>>    }
>>>>    
>>>> +static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
>>>> +{
>>>> +	/* WaDisableHBR2:skl */
>>>> +	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>>> +		return false;
>>>> +
>>>> +	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>>> +	    (INTEL_INFO(dev)->gen >= 9))
>>>> +		return true;
>>>> +	else
>>>> +		return false;
>>>> +}
>>>> +
>>>> +
>>>>    static int
>>>>    intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>>>    {
>>>> @@ -1220,12 +1234,8 @@ intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
>>>>    
>>>>    	*source_rates = default_rates;
>>>>    
>>>> -	/* WaDisableHBR2:skl */
>>>> -	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
>>>> -		return (DP_LINK_BW_2_7 >> 3) + 1;
>>>> -
>>>> -	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
>>>> -	    (INTEL_INFO(dev)->gen >= 9))
>>>> +	/* This depends on the fact that 5.4 is last value in the array */
>>>> +	if (intel_dp_source_supports_hbr2(dev))
>>>>    		return (DP_LINK_BW_5_4 >> 3) + 1;
>>>>    	else
>>>>    		return (DP_LINK_BW_2_7 >> 3) + 1;
>>>> @@ -3926,7 +3936,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>>>    	/* Training Pattern 3 support, both source and sink */
>>>>    	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
>>>>    	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
>>>> -	    (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
>>>> +	    intel_dp_source_supports_hbr2(dev)) {
>>> hbr2 is not the same as tps3, is it? It's possible to use tps3 without
>>> using hbr2, right?
>>>
>>> BR,
>>> Jani.
>> Yes, TP3 can be supported on panels that does not support HBR2 as well, but
>> the check here is for hardware capability. Intel platforms that does not
>> support
>> HBR2 cannot support TP3 as well, so we should treat them both the same.
>> (the only exception here seems to be SKL <B0 where WA is in place)
> Okay, please at least add a comment to that effect.
>
> Alternatively you add a separate intel_dp_source_supports_tps3, and
> modify intel_dp_source_supports_hbr2 to be
> (intel_dp_source_supports_tps3 && !WaDisableHBR2:skl). *shrug*.
>
> BR,
> Jani.
>
updated the patch with comments and shared again.
>
>>>>    		intel_dp->use_tps3 = true;
>>>>    		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
>>>>    	} else
>>>> -- 
>>>> 1.7.9.5
>>>>
>> -- 
>> regards,
>> Sivakumar
>>

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 475d8cb..8bc6361 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1207,6 +1207,20 @@  intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
 	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
 }
 
+static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
+{
+	/* WaDisableHBR2:skl */
+	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
+		return false;
+
+	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
+	    (INTEL_INFO(dev)->gen >= 9))
+		return true;
+	else
+		return false;
+}
+
+
 static int
 intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 {
@@ -1220,12 +1234,8 @@  intel_dp_source_rates(struct drm_device *dev, const int **source_rates)
 
 	*source_rates = default_rates;
 
-	/* WaDisableHBR2:skl */
-	if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0)
-		return (DP_LINK_BW_2_7 >> 3) + 1;
-
-	if ((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || IS_BROADWELL(dev) ||
-	    (INTEL_INFO(dev)->gen >= 9))
+	/* This depends on the fact that 5.4 is last value in the array */
+	if (intel_dp_source_supports_hbr2(dev))
 		return (DP_LINK_BW_5_4 >> 3) + 1;
 	else
 		return (DP_LINK_BW_2_7 >> 3) + 1;
@@ -3926,7 +3936,7 @@  intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	/* Training Pattern 3 support, both source and sink */
 	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
 	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
-	    (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)) {
+	    intel_dp_source_supports_hbr2(dev)) {
 		intel_dp->use_tps3 = true;
 		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
 	} else

[3/4] drm/i915: Avoid TP3 on CHV

Commit Message

Comments

Patch