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

Message ID 1439876279-1546-4-git-send-email-sivakumar.thulasimani@intel.com
State New
Headers show

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

Patch
diff mbox

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