Re: [PATCHv4 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer

2023-03-21 Thread Jani Nikula 
On Tue, 07 Mar 2023, Jani Nikula  wrote:
> On Tue, 07 Mar 2023, Jani Nikula  wrote:
>> On Thu, 02 Mar 2023, Arun R Murthy  wrote:
>>> Enable SDP error detection configuration, this will set CRC16 in
>>> 128b/132b link layer.
>>> For Display version 13 a hardware bit31 in register VIDEO_DIP_CTL is
>>> added to enable/disable SDP CRC applicable for DP2.0 only, but the
>>> default value of this bit will enable CRC16 in 128b/132b hence
>>> skipping this write.
>>> Corrective actions on SDP corruption is yet to be defined.
>>>
>>> v2: Moved the CRC enable to link training init(Jani N)
>>> v3: Moved crc enable to ddi pre enable 
>>> v4: Separate function for SDP CRC16 (Jani N)
>>>
>>> Signed-off-by: Arun R Murthy 
>>
>> Reviewed-by: Jani Nikula 
>
> PS. I've queued retest on this one, need to wait for results before
> applying.

And finally pushed to drm-intel-next. Thanks for the patches & patience.

BR,
Jani.

>
>
>>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_ddi.c  |  4 
>>>  .../drm/i915/display/intel_dp_link_training.c | 20 +++
>>>  .../drm/i915/display/intel_dp_link_training.h |  2 ++
>>>  3 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
>>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> index e5979427b38b..127b3035f92d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> @@ -2519,6 +2519,10 @@ static void intel_ddi_pre_enable_dp(struct 
>>> intel_atomic_state *state,
>>>  {
>>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>  
>>> +   if (HAS_DP20(dev_priv))
>>> +   intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
>>> +   crtc_state);
>>> +
>>> if (DISPLAY_VER(dev_priv) >= 12)
>>> tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
>>> else
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
>>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>>> index 3d3efcf02011..35d31e4efab9 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>>> @@ -1454,3 +1454,23 @@ void intel_dp_start_link_train(struct intel_dp 
>>> *intel_dp,
>>> if (!passed)
>>> intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
>>>  }
>>> +
>>> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>>> +const struct intel_crtc_state *crtc_state)
>>> +{
>>> +   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>>> +
>>> +   /*
>>> +* VIDEO_DIP_CTL register bit 31 should be set to '0' to not
>>> +* disable SDP CRC. This is applicable for Display version 13.
>>> +* Default value of bit 31 is '0' hence discarding the write
>>> +* TODO: Corrective actions on SDP corruption yet to be defined
>>> +*/
>>> +   if (intel_dp_is_uhbr(crtc_state))
>>> +   /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */
>>> +   drm_dp_dpcd_writeb(_dp->aux,
>>> +  DP_SDP_ERROR_DETECTION_CONFIGURATION,
>>> +  DP_SDP_CRC16_128B132B_EN);
>>> +
>>> +   drm_dbg_kms(>drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h 
>>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>>> index 7fa1c0833096..2c8f2775891b 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>>> @@ -39,4 +39,6 @@ static inline u8 intel_dp_training_pattern_symbol(u8 
>>> pattern)
>>> return pattern & ~DP_LINK_SCRAMBLING_DISABLE;
>>>  }
>>>  
>>> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>>> +const struct intel_crtc_state *crtc_state);
>>>  #endif /* __INTEL_DP_LINK_TRAINING_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCHv4 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer

2023-03-07 Thread Jani Nikula 
On Tue, 07 Mar 2023, Jani Nikula  wrote:
> On Thu, 02 Mar 2023, Arun R Murthy  wrote:
>> Enable SDP error detection configuration, this will set CRC16 in
>> 128b/132b link layer.
>> For Display version 13 a hardware bit31 in register VIDEO_DIP_CTL is
>> added to enable/disable SDP CRC applicable for DP2.0 only, but the
>> default value of this bit will enable CRC16 in 128b/132b hence
>> skipping this write.
>> Corrective actions on SDP corruption is yet to be defined.
>>
>> v2: Moved the CRC enable to link training init(Jani N)
>> v3: Moved crc enable to ddi pre enable 
>> v4: Separate function for SDP CRC16 (Jani N)
>>
>> Signed-off-by: Arun R Murthy 
>
> Reviewed-by: Jani Nikula 

PS. I've queued retest on this one, need to wait for results before
applying.


>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c  |  4 
>>  .../drm/i915/display/intel_dp_link_training.c | 20 +++
>>  .../drm/i915/display/intel_dp_link_training.h |  2 ++
>>  3 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index e5979427b38b..127b3035f92d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2519,6 +2519,10 @@ static void intel_ddi_pre_enable_dp(struct 
>> intel_atomic_state *state,
>>  {
>>  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  
>> +if (HAS_DP20(dev_priv))
>> +intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
>> +crtc_state);
>> +
>>  if (DISPLAY_VER(dev_priv) >= 12)
>>  tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
>>  else
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> index 3d3efcf02011..35d31e4efab9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> @@ -1454,3 +1454,23 @@ void intel_dp_start_link_train(struct intel_dp 
>> *intel_dp,
>>  if (!passed)
>>  intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
>>  }
>> +
>> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>> + const struct intel_crtc_state *crtc_state)
>> +{
>> +struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +
>> +/*
>> + * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
>> + * disable SDP CRC. This is applicable for Display version 13.
>> + * Default value of bit 31 is '0' hence discarding the write
>> + * TODO: Corrective actions on SDP corruption yet to be defined
>> + */
>> +if (intel_dp_is_uhbr(crtc_state))
>> +/* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */
>> +drm_dp_dpcd_writeb(_dp->aux,
>> +   DP_SDP_ERROR_DETECTION_CONFIGURATION,
>> +   DP_SDP_CRC16_128B132B_EN);
>> +
>> +drm_dbg_kms(>drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h 
>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> index 7fa1c0833096..2c8f2775891b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
>> @@ -39,4 +39,6 @@ static inline u8 intel_dp_training_pattern_symbol(u8 
>> pattern)
>>  return pattern & ~DP_LINK_SCRAMBLING_DISABLE;
>>  }
>>  
>> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
>> + const struct intel_crtc_state *crtc_state);
>>  #endif /* __INTEL_DP_LINK_TRAINING_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCHv4 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer

2023-03-07 Thread Jani Nikula 
On Thu, 02 Mar 2023, Arun R Murthy  wrote:
> Enable SDP error detection configuration, this will set CRC16 in
> 128b/132b link layer.
> For Display version 13 a hardware bit31 in register VIDEO_DIP_CTL is
> added to enable/disable SDP CRC applicable for DP2.0 only, but the
> default value of this bit will enable CRC16 in 128b/132b hence
> skipping this write.
> Corrective actions on SDP corruption is yet to be defined.
>
> v2: Moved the CRC enable to link training init(Jani N)
> v3: Moved crc enable to ddi pre enable 
> v4: Separate function for SDP CRC16 (Jani N)
>
> Signed-off-by: Arun R Murthy 

Reviewed-by: Jani Nikula 

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c  |  4 
>  .../drm/i915/display/intel_dp_link_training.c | 20 +++
>  .../drm/i915/display/intel_dp_link_training.h |  2 ++
>  3 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index e5979427b38b..127b3035f92d 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2519,6 +2519,10 @@ static void intel_ddi_pre_enable_dp(struct 
> intel_atomic_state *state,
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
> + if (HAS_DP20(dev_priv))
> + intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
> + crtc_state);
> +
>   if (DISPLAY_VER(dev_priv) >= 12)
>   tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
>   else
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 3d3efcf02011..35d31e4efab9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1454,3 +1454,23 @@ void intel_dp_start_link_train(struct intel_dp 
> *intel_dp,
>   if (!passed)
>   intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
>  }
> +
> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
> +  const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +
> + /*
> +  * VIDEO_DIP_CTL register bit 31 should be set to '0' to not
> +  * disable SDP CRC. This is applicable for Display version 13.
> +  * Default value of bit 31 is '0' hence discarding the write
> +  * TODO: Corrective actions on SDP corruption yet to be defined
> +  */
> + if (intel_dp_is_uhbr(crtc_state))
> + /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */
> + drm_dp_dpcd_writeb(_dp->aux,
> +DP_SDP_ERROR_DETECTION_CONFIGURATION,
> +DP_SDP_CRC16_128B132B_EN);
> +
> + drm_dbg_kms(>drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h 
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> index 7fa1c0833096..2c8f2775891b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
> @@ -39,4 +39,6 @@ static inline u8 intel_dp_training_pattern_symbol(u8 
> pattern)
>   return pattern & ~DP_LINK_SCRAMBLING_DISABLE;
>  }
>  
> +void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
> +  const struct intel_crtc_state *crtc_state);
>  #endif /* __INTEL_DP_LINK_TRAINING_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

[PATCHv4 2/2] i915/display/dp: SDP CRC16 for 128b132b link layer

2023-03-02 Thread Arun R Murthy 
Enable SDP error detection configuration, this will set CRC16 in
128b/132b link layer.
For Display version 13 a hardware bit31 in register VIDEO_DIP_CTL is
added to enable/disable SDP CRC applicable for DP2.0 only, but the
default value of this bit will enable CRC16 in 128b/132b hence
skipping this write.
Corrective actions on SDP corruption is yet to be defined.

v2: Moved the CRC enable to link training init(Jani N)
v3: Moved crc enable to ddi pre enable 
v4: Separate function for SDP CRC16 (Jani N)

Signed-off-by: Arun R Murthy 
---
 drivers/gpu/drm/i915/display/intel_ddi.c  |  4 
 .../drm/i915/display/intel_dp_link_training.c | 20 +++
 .../drm/i915/display/intel_dp_link_training.h |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index e5979427b38b..127b3035f92d 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2519,6 +2519,10 @@ static void intel_ddi_pre_enable_dp(struct 
intel_atomic_state *state,
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
+   if (HAS_DP20(dev_priv))
+   intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
+   crtc_state);
+
if (DISPLAY_VER(dev_priv) >= 12)
tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
else
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 3d3efcf02011..35d31e4efab9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1454,3 +1454,23 @@ void intel_dp_start_link_train(struct intel_dp *intel_dp,
if (!passed)
intel_dp_schedule_fallback_link_training(intel_dp, crtc_state);
 }
+
+void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
+const struct intel_crtc_state *crtc_state)
+{
+   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+
+   /*
+* VIDEO_DIP_CTL register bit 31 should be set to '0' to not
+* disable SDP CRC. This is applicable for Display version 13.
+* Default value of bit 31 is '0' hence discarding the write
+* TODO: Corrective actions on SDP corruption yet to be defined
+*/
+   if (intel_dp_is_uhbr(crtc_state))
+   /* DP v2.0 SCR on SDP CRC16 for 128b/132b Link Layer */
+   drm_dp_dpcd_writeb(_dp->aux,
+  DP_SDP_ERROR_DETECTION_CONFIGURATION,
+  DP_SDP_CRC16_128B132B_EN);
+
+   drm_dbg_kms(>drm, "DP2.0 SDP CRC16 for 128b/132b enabled\n");
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.h 
b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
index 7fa1c0833096..2c8f2775891b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.h
@@ -39,4 +39,6 @@ static inline u8 intel_dp_training_pattern_symbol(u8 pattern)
return pattern & ~DP_LINK_SCRAMBLING_DISABLE;
 }
 
+void intel_dp_128b132b_sdp_crc16(struct intel_dp *intel_dp,
+const struct intel_crtc_state *crtc_state);
 #endif /* __INTEL_DP_LINK_TRAINING_H__ */
-- 
2.25.1