* [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
@ 2019-05-20 23:25 Khaled Almahallawy
2019-05-20 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Khaled Almahallawy @ 2019-05-20 23:25 UTC (permalink / raw)
To: Intel-gfx; +Cc: Khaled Almahallawy
According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the Max Pre-Emphasis level for certain Swing Level. This interpretation fails Link Layer compliance test 400.3.1.15 step 17 according to the following Fail condition: TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
drivers/gpu/drm/i915/intel_dp.c | 2 +-
2 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0af47f343faa..6540c979c098 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
DP_TRAIN_VOLTAGE_SWING_MASK;
}
-/*
- * We assume that the full set of pre-emphasis values can be
- * used on all DDI platforms. Should that change we need to
- * rethink this code.
- */
-u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
-{
- switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
- case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
- return DP_TRAIN_PRE_EMPH_LEVEL_3;
- case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
- return DP_TRAIN_PRE_EMPH_LEVEL_2;
- case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
- return DP_TRAIN_PRE_EMPH_LEVEL_1;
- case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
- default:
- return DP_TRAIN_PRE_EMPH_LEVEL_0;
- }
-}
-
static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
int level, enum intel_output_type type)
{
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 77ba4da6b981..f94759e45862 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
enum port port = encoder->port;
if (HAS_DDI(dev_priv)) {
- return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
+ return DP_TRAIN_PRE_EMPH_LEVEL_3;
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
@ 2019-05-20 23:36 ` Patchwork
2019-05-21 13:24 ` [PATCH] " Ville Syrjälä
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-20 23:36 UTC (permalink / raw)
To: Khaled Almahallawy; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
URL : https://patchwork.freedesktop.org/series/60880/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
bc6664a25925 drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7:
According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
total: 0 errors, 1 warnings, 0 checks, 34 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
2019-05-20 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2019-05-21 13:24 ` Ville Syrjälä
2019-05-21 22:16 ` Manasi Navare
2019-05-22 19:25 ` Manasi Navare
2019-05-21 14:24 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2) Patchwork
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Ville Syrjälä @ 2019-05-21 13:24 UTC (permalink / raw)
To: Khaled Almahallawy; +Cc: Intel-gfx
On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
> According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
> Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the Max Pre-Emphasis level for certain Swing Level. This interpretation fails Link Layer compliance test 400.3.1.15 step 17 according to the following Fail condition: TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
Hmm. I guess that's correct. The spec doesn't say anything about
per-vswing pre-emphasis when talking about the 'max reached' bit.
>
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> 2 files changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0af47f343faa..6540c979c098 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> DP_TRAIN_VOLTAGE_SWING_MASK;
> }
>
> -/*
> - * We assume that the full set of pre-emphasis values can be
> - * used on all DDI platforms. Should that change we need to
> - * rethink this code.
> - */
> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
> -{
> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> - default:
> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> - }
> -}
> -
> static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> int level, enum intel_output_type type)
> {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 77ba4da6b981..f94759e45862 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
> enum port port = encoder->port;
>
> if (HAS_DDI(dev_priv)) {
> - return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
We're going to have to change this for all platforms.
Also we need to update the code to pick the correct swing/pre-emphasis
when we can't do what is being requested.
The spec says:
"When the combination of the requested pre-emphasis level and voltage
swing exceeds the capability of a DPTX, the DPTX shall set the pre-emphasis
level according to the request and use the highest voltage swing it can
output with the given pre-emphasis level."
and
"When a DPTX reads a request beyond the limits of this Standard, the
DPTX shall set the pre-emphasis level according to the request and set
the highest voltage swing level it can output with the given pre-emphasis
level. If a DPTX is requested for 9.5dB of pre-emphasis level (may be
supported for a DPTX) and cannot support that level, it shall set the
pre-emphasis level to the next highest level, 6dB."
> } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> --
> 2.17.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
2019-05-20 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-05-21 13:24 ` [PATCH] " Ville Syrjälä
@ 2019-05-21 14:24 ` Patchwork
2019-05-21 14:44 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-22 6:17 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-21 14:24 UTC (permalink / raw)
To: Khaled Almahallawy; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
URL : https://patchwork.freedesktop.org/series/60880/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ee90db0cb06a drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7:
According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
total: 0 errors, 1 warnings, 0 checks, 34 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
` (2 preceding siblings ...)
2019-05-21 14:24 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2) Patchwork
@ 2019-05-21 14:44 ` Patchwork
2019-05-22 6:17 ` ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-21 14:44 UTC (permalink / raw)
To: Khaled Almahallawy; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
URL : https://patchwork.freedesktop.org/series/60880/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6113 -> Patchwork_13055
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/
Known issues
------------
Here are the changes found in Patchwork_13055 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [PASS][1] -> [DMESG-FAIL][2] ([fdo#110235])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
- fi-skl-gvtdvm: [PASS][3] -> [DMESG-FAIL][4] ([fdo#110235])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_hangcheck:
- fi-skl-iommu: [PASS][5] -> [INCOMPLETE][6] ([fdo#108602] / [fdo#108744])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-skl-iommu/igt@i915_selftest@live_hangcheck.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
#### Possible fixes ####
* igt@gem_mmap_gtt@basic-copy:
- {fi-icl-u3}: [DMESG-WARN][9] ([fdo#107724]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html
* igt@kms_frontbuffer_tracking@basic:
- {fi-icl-u2}: [FAIL][11] ([fdo#103167]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
#### Warnings ####
* igt@i915_selftest@live_hangcheck:
- fi-apl-guc: [INCOMPLETE][13] ([fdo#103927] / [fdo#110624]) -> [DMESG-FAIL][14] ([fdo#110620])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
* igt@runner@aborted:
- fi-apl-guc: [FAIL][15] ([fdo#110624]) -> [FAIL][16] ([fdo#110622])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/fi-apl-guc/igt@runner@aborted.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/fi-apl-guc/igt@runner@aborted.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
[fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620
[fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622
[fdo#110624]: https://bugs.freedesktop.org/show_bug.cgi?id=110624
Participating hosts (52 -> 45)
------------------------------
Additional (1): fi-cml-u2
Missing (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6113 -> Patchwork_13055
CI_DRM_6113: ae8881a51dd975344f22da2fbb53b9bf498f4592 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5000: f9961d14d76b3a0fa1296e547f7c065e2f93955c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13055: ee90db0cb06a2d54156db4f8a30660baefb0ed32 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
ee90db0cb06a drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-21 13:24 ` [PATCH] " Ville Syrjälä
@ 2019-05-21 22:16 ` Manasi Navare
2019-05-22 19:25 ` Manasi Navare
1 sibling, 0 replies; 11+ messages in thread
From: Manasi Navare @ 2019-05-21 22:16 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote:
> On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
> > According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
> > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the Max Pre-Emphasis level for certain Swing Level. This interpretation fails Link Layer compliance test 400.3.1.15 step 17 according to the following Fail condition: TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
>
> Hmm. I guess that's correct. The spec doesn't say anything about
> per-vswing pre-emphasis when talking about the 'max reached' bit.
>
> >
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
> > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > 2 files changed, 1 insertion(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0af47f343faa..6540c979c098 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> > DP_TRAIN_VOLTAGE_SWING_MASK;
> > }
> >
> > -/*
> > - * We assume that the full set of pre-emphasis values can be
> > - * used on all DDI platforms. Should that change we need to
> > - * rethink this code.
> > - */
> > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
> > -{
> > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > - default:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > - }
As per the Bspec Voltage swing programming section, the above code makes sense
where the max pre emphasis if vswing level is 0 is 3, max pre emphasis if vswing level 1 is 2
and so on..
So then are you suggesting modifying the entire logic for intel_get_adjust_train() in case we want
to always set the max_pre_emphasis to say 3 and max vswing to 3 and then adjust to the requested values
as long as the combination is as per the above table (as per the bspec)
Regards
Manasi
> > -}
> > -
> > static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > int level, enum intel_output_type type)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 77ba4da6b981..f94759e45862 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
> > enum port port = encoder->port;
> >
> > if (HAS_DDI(dev_priv)) {
> > - return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
> > + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>
> We're going to have to change this for all platforms.
>
> Also we need to update the code to pick the correct swing/pre-emphasis
> when we can't do what is being requested.
>
> The spec says:
> "When the combination of the requested pre-emphasis level and voltage
> swing exceeds the capability of a DPTX, the DPTX shall set the pre-emphasis
> level according to the request and use the highest voltage swing it can
> output with the given pre-emphasis level."
> and
> "When a DPTX reads a request beyond the limits of this Standard, the
> DPTX shall set the pre-emphasis level according to the request and set
> the highest voltage swing level it can output with the given pre-emphasis
> level. If a DPTX is requested for 9.5dB of pre-emphasis level (may be
> supported for a DPTX) and cannot support that level, it shall set the
> pre-emphasis level to the next highest level, 6dB."
>
>
>
> > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
` (3 preceding siblings ...)
2019-05-21 14:44 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-05-22 6:17 ` Patchwork
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-22 6:17 UTC (permalink / raw)
To: Khaled Almahallawy; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2)
URL : https://patchwork.freedesktop.org/series/60880/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6113_full -> Patchwork_13055_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13055_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13055_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_13055_full:
### IGT changes ###
#### Possible regressions ####
* igt@perf@blocking:
- shard-iclb: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-iclb3/igt@perf@blocking.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb8/igt@perf@blocking.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@kms_big_fb@y-tiled-8bpp-rotate-0}:
- shard-iclb: NOTRUN -> [SKIP][3] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb7/igt@kms_big_fb@y-tiled-8bpp-rotate-0.html
Known issues
------------
Here are the changes found in Patchwork_13055_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@in-flight-10ms:
- shard-iclb: [PASS][4] -> [INCOMPLETE][5] ([fdo#107713])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-iclb7/igt@gem_eio@in-flight-10ms.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb7/igt@gem_eio@in-flight-10ms.html
* igt@gem_tiled_swapping@non-threaded:
- shard-hsw: [PASS][6] -> [FAIL][7] ([fdo#108686])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-hsw8/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_workarounds@suspend-resume:
- shard-apl: [PASS][8] -> [DMESG-WARN][9] ([fdo#108566]) +2 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-apl5/igt@gem_workarounds@suspend-resume.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-apl2/igt@gem_workarounds@suspend-resume.html
* igt@kms_cursor_edge_walk@pipe-b-64x64-bottom-edge:
- shard-snb: [PASS][10] -> [SKIP][11] ([fdo#109271] / [fdo#109278])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-snb1/igt@kms_cursor_edge_walk@pipe-b-64x64-bottom-edge.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-snb2/igt@kms_cursor_edge_walk@pipe-b-64x64-bottom-edge.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-skl: [PASS][12] -> [FAIL][13] ([fdo#102670])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: [PASS][14] -> [FAIL][15] ([fdo#105363])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl10/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
- shard-iclb: [PASS][16] -> [FAIL][17] ([fdo#103167]) +3 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [PASS][18] -> [FAIL][19] ([fdo#108145])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
#### Possible fixes ####
* igt@gem_flink_race@flink_close:
- shard-apl: [INCOMPLETE][20] ([fdo#103927]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-apl1/igt@gem_flink_race@flink_close.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-apl5/igt@gem_flink_race@flink_close.html
* igt@gem_softpin@noreloc-s3:
- shard-skl: [INCOMPLETE][22] ([fdo#104108]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl1/igt@gem_softpin@noreloc-s3.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl5/igt@gem_softpin@noreloc-s3.html
* igt@i915_suspend@fence-restore-untiled:
- shard-apl: [DMESG-WARN][24] ([fdo#108566]) -> [PASS][25] +3 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-apl7/igt@i915_suspend@fence-restore-untiled.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-apl3/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
- shard-snb: [SKIP][26] ([fdo#109271] / [fdo#109278]) -> [PASS][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-snb2/igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-snb2/igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b.html
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
- shard-hsw: [FAIL][28] ([fdo#105767]) -> [PASS][29]
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
* igt@kms_cursor_legacy@flip-vs-cursor-varying-size:
- shard-snb: [SKIP][30] ([fdo#109271]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-snb4/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-snb4/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
* igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
- shard-glk: [FAIL][32] ([fdo#100368]) -> [PASS][33]
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-glk2/igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-glk9/igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible.html
* igt@kms_flip@flip-vs-suspend:
- shard-hsw: [INCOMPLETE][34] ([fdo#103540]) -> [PASS][35]
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-hsw8/igt@kms_flip@flip-vs-suspend.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-hsw5/igt@kms_flip@flip-vs-suspend.html
* igt@kms_flip_tiling@flip-to-x-tiled:
- shard-iclb: [FAIL][36] ([fdo#108134]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-iclb8/igt@kms_flip_tiling@flip-to-x-tiled.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb7/igt@kms_flip_tiling@flip-to-x-tiled.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
- shard-iclb: [FAIL][38] ([fdo#103167]) -> [PASS][39] +1 similar issue
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-wc:
- shard-skl: [FAIL][40] ([fdo#103167]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-wc.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@psr-suspend:
- shard-skl: [INCOMPLETE][42] ([fdo#104108] / [fdo#106978]) -> [PASS][43]
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl3/igt@kms_frontbuffer_tracking@psr-suspend.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl9/igt@kms_frontbuffer_tracking@psr-suspend.html
* igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: [FAIL][44] ([fdo#108145]) -> [PASS][45] +1 similar issue
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
* igt@kms_setmode@basic:
- shard-apl: [FAIL][46] ([fdo#99912]) -> [PASS][47]
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-apl5/igt@kms_setmode@basic.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-apl4/igt@kms_setmode@basic.html
#### Warnings ####
* igt@i915_pm_rpm@dpms-non-lpsp:
- shard-skl: [SKIP][48] ([fdo#109271]) -> [INCOMPLETE][49] ([fdo#107807])
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl4/igt@i915_pm_rpm@dpms-non-lpsp.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl6/igt@i915_pm_rpm@dpms-non-lpsp.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite:
- shard-skl: [FAIL][50] ([fdo#103167]) -> [FAIL][51] ([fdo#108040])
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-skl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html
* igt@kms_vblank@pipe-c-query-busy-hang:
- shard-snb: [SKIP][52] ([fdo#109271]) -> [SKIP][53] ([fdo#109271] / [fdo#109278])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6113/shard-snb4/igt@kms_vblank@pipe-c-query-busy-hang.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/shard-snb4/igt@kms_vblank@pipe-c-query-busy-hang.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
[fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_6113 -> Patchwork_13055
CI_DRM_6113: ae8881a51dd975344f22da2fbb53b9bf498f4592 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5000: f9961d14d76b3a0fa1296e547f7c065e2f93955c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13055: ee90db0cb06a2d54156db4f8a30660baefb0ed32 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13055/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-21 13:24 ` [PATCH] " Ville Syrjälä
2019-05-21 22:16 ` Manasi Navare
@ 2019-05-22 19:25 ` Manasi Navare
2019-05-30 19:33 ` Almahallawy, Khaled
1 sibling, 1 reply; 11+ messages in thread
From: Manasi Navare @ 2019-05-22 19:25 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote:
> On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
> > According to DP 1.4 standard, if the source supports four pre-emphasis levels, then the source shall set the bit MAX_PRE-EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis level 3 is the maximum pre-emphasis level that the source supports.
> > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the Max Pre-Emphasis level for certain Swing Level. This interpretation fails Link Layer compliance test 400.3.1.15 step 17 according to the following Fail condition: TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
>
> Hmm. I guess that's correct. The spec doesn't say anything about
> per-vswing pre-emphasis when talking about the 'max reached' bit.
>
> >
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
> > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > 2 files changed, 1 insertion(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 0af47f343faa..6540c979c098 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> > DP_TRAIN_VOLTAGE_SWING_MASK;
> > }
> >
> > -/*
> > - * We assume that the full set of pre-emphasis values can be
> > - * used on all DDI platforms. Should that change we need to
> > - * rethink this code.
> > - */
> > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
> > -{
> > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > - default:
> > - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > - }
> > -}
> > -
> > static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > int level, enum intel_output_type type)
> > {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 77ba4da6b981..f94759e45862 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, u8 voltage_swing)
> > enum port port = encoder->port;
> >
> > if (HAS_DDI(dev_priv)) {
> > - return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
> > + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>
> We're going to have to change this for all platforms.
>
> Also we need to update the code to pick the correct swing/pre-emphasis
> when we can't do what is being requested.
This check will need to be added in adjust_train() function
>
> The spec says:
> "When the combination of the requested pre-emphasis level and voltage
> swing exceeds the capability of a DPTX, the DPTX shall set the pre-emphasis
> level according to the request and use the highest voltage swing it can
> output with the given pre-emphasis level."
> and
> "When a DPTX reads a request beyond the limits of this Standard, the
> DPTX shall set the pre-emphasis level according to the request and set
> the highest voltage swing level it can output with the given pre-emphasis
> level. If a DPTX is requested for 9.5dB of pre-emphasis level (may be
> supported for a DPTX) and cannot support that level, it shall set the
> pre-emphasis level to the next highest level, 6dB."
So my interpretation of this is :
In adjust_train() function:
vswing_max = intel_dp_voltage_max() which is set per platform
pre_emphasis_max = set to level 3
v = get_requested_voltage_swing() - Limit this to vmax
p = get_requested_pre_emphasis() - Limit this to pmax
Now rewrite the intel_ddi_dp_pre_emphasis_max() function to call it intel_ddi_dp_possible_vswing_max()
where we set the possible vswing max based on requested pre emphasis and table in the bspec
Eg: if requested pre emphasis is 3, the vswing is 0 which is the max vswing value
it can ouput with that pre emphasis level based on the bspec vswing programming values
Set v = that value
p = requested
Link train
Ville, is this logic correct?
Regards
Manasi
>
>
>
> > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-22 19:25 ` Manasi Navare
@ 2019-05-30 19:33 ` Almahallawy, Khaled
2019-05-30 21:20 ` Manasi Navare
0 siblings, 1 reply; 11+ messages in thread
From: Almahallawy, Khaled @ 2019-05-30 19:33 UTC (permalink / raw)
To: ville.syrjala, Navare, Manasi D; +Cc: Intel-gfx
On Wed, 2019-05-22 at 12:25 -0700, Manasi Navare wrote:
> On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote:
> > On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
> > > According to DP 1.4 standard, if the source supports four pre-
> > > emphasis levels, then the source shall set the bit MAX_PRE-
> > > EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-
> > > EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis
> > > level 3 is the maximum pre-emphasis level that the source
> > > supports.
> > > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the
> > > Max Pre-Emphasis level for certain Swing Level. This
> > > interpretation fails Link Layer compliance test 400.3.1.15 step
> > > 17 according to the following Fail condition:
> > > TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active
> > > lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
> >
> > Hmm. I guess that's correct. The spec doesn't say anything about
> > per-vswing pre-emphasis when talking about the 'max reached' bit.
> >
> > >
> > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
> > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > 2 files changed, 1 insertion(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 0af47f343faa..6540c979c098 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct
> > > intel_encoder *encoder)
> > > DP_TRAIN_VOLTAGE_SWING_MASK;
> > > }
> > >
> > > -/*
> > > - * We assume that the full set of pre-emphasis values can be
> > > - * used on all DDI platforms. Should that change we need to
> > > - * rethink this code.
> > > - */
> > > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
> > > u8 voltage_swing)
> > > -{
> > > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > - default:
> > > - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > - }
> > > -}
> > > -
> > > static void cnl_ddi_vswing_program(struct intel_encoder
> > > *encoder,
> > > int level, enum intel_output_type
> > > type)
> > > {
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 77ba4da6b981..f94759e45862 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp
> > > *intel_dp, u8 voltage_swing)
> > > enum port port = encoder->port;
> > >
> > > if (HAS_DDI(dev_priv)) {
> > > - return intel_ddi_dp_pre_emphasis_max(encoder,
> > > voltage_swing);
> > > + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> >
> > We're going to have to change this for all platforms.
Yes, I'm going to change for all platforms in intel_dp_pre_emphasis_max
function. I will also add the missing condition:
else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
similar to intel_dp_voltage_max function
> >
> > Also we need to update the code to pick the correct swing/pre-
> > emphasis
> > when we can't do what is being requested.
>
> This check will need to be added in adjust_train() function
sure, I will implement this logic in intel_get_adjust_train
>
> >
> > The spec says:
> > "When the combination of the requested pre-emphasis level and
> > voltage
> > swing exceeds the capability of a DPTX, the DPTX shall set the
> > pre-emphasis
> > level according to the request and use the highest voltage swing
> > it can
> > output with the given pre-emphasis level."
> > and
> > "When a DPTX reads a request beyond the limits of this Standard,
> > the
> > DPTX shall set the pre-emphasis level according to the request and
> > set
> > the highest voltage swing level it can output with the given pre-
> > emphasis
> > level. If a DPTX is requested for 9.5dB of pre-emphasis level (may
> > be
> > supported for a DPTX) and cannot support that level, it shall set
> > the
> > pre-emphasis level to the next highest level, 6dB."
>
> So my interpretation of this is :
>
> In adjust_train() function:
>
> vswing_max = intel_dp_voltage_max() which is set per platform
> pre_emphasis_max = set to level 3
pre_emphasis_max = intel_dp_pre_emphasis_max
because it is set per platfrom as well, similar to vswing_max
>
> v = get_requested_voltage_swing() - Limit this to vmax
> p = get_requested_pre_emphasis() - Limit this to pmax
>
> Now rewrite the intel_ddi_dp_pre_emphasis_max() function to call it
intel_ddi_dp_pre_emphasis_max is needed to determine the max pre-
emphasis level per platform.
> intel_ddi_dp_possible_vswing_max()
I think the logic for choosing the correct vswing/emphasis combination
should be in a different function, as you suggested it can be in
intel_ddi_dp_possible_vswing_max
Thanks
Khaled
> where we set the possible vswing max based on requested pre emphasis
> and table in the bspec
> Eg: if requested pre emphasis is 3, the vswing is 0 which is the max
> vswing value
> it can ouput with that pre emphasis level based on the bspec vswing
> programming values
>
> Set v = that value
> p = requested
>
> Link train
>
> Ville, is this logic correct?
>
> Regards
> Manasi
>
>
> >
> >
> >
> > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > {
> > > switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-30 19:33 ` Almahallawy, Khaled
@ 2019-05-30 21:20 ` Manasi Navare
2019-06-04 0:44 ` Khaled Almahallawy
0 siblings, 1 reply; 11+ messages in thread
From: Manasi Navare @ 2019-05-30 21:20 UTC (permalink / raw)
To: Almahallawy, Khaled; +Cc: Intel-gfx
On Thu, May 30, 2019 at 12:33:40PM -0700, Almahallawy, Khaled wrote:
> On Wed, 2019-05-22 at 12:25 -0700, Manasi Navare wrote:
> > On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote:
> > > On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
> > > > According to DP 1.4 standard, if the source supports four pre-
> > > > emphasis levels, then the source shall set the bit MAX_PRE-
> > > > EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-
> > > > EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis
> > > > level 3 is the maximum pre-emphasis level that the source
> > > > supports.
> > > > Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the
> > > > Max Pre-Emphasis level for certain Swing Level. This
> > > > interpretation fails Link Layer compliance test 400.3.1.15 step
> > > > 17 according to the following Fail condition:
> > > > TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active
> > > > lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
> > >
> > > Hmm. I guess that's correct. The spec doesn't say anything about
> > > per-vswing pre-emphasis when talking about the 'max reached' bit.
> > >
> > > >
> > > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
> > > > drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > > 2 files changed, 1 insertion(+), 21 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index 0af47f343faa..6540c979c098 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct
> > > > intel_encoder *encoder)
> > > > DP_TRAIN_VOLTAGE_SWING_MASK;
> > > > }
> > > >
> > > > -/*
> > > > - * We assume that the full set of pre-emphasis values can be
> > > > - * used on all DDI platforms. Should that change we need to
> > > > - * rethink this code.
> > > > - */
> > > > -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
> > > > u8 voltage_swing)
> > > > -{
> > > > - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > > - return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> > > > - return DP_TRAIN_PRE_EMPH_LEVEL_2;
> > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> > > > - return DP_TRAIN_PRE_EMPH_LEVEL_1;
> > > > - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
> > > > - default:
> > > > - return DP_TRAIN_PRE_EMPH_LEVEL_0;
> > > > - }
> > > > -}
> > > > -
> > > > static void cnl_ddi_vswing_program(struct intel_encoder
> > > > *encoder,
> > > > int level, enum intel_output_type
> > > > type)
> > > > {
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 77ba4da6b981..f94759e45862 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp
> > > > *intel_dp, u8 voltage_swing)
> > > > enum port port = encoder->port;
> > > >
> > > > if (HAS_DDI(dev_priv)) {
> > > > - return intel_ddi_dp_pre_emphasis_max(encoder,
> > > > voltage_swing);
> > > > + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> > >
> > > We're going to have to change this for all platforms.
>
> Yes, I'm going to change for all platforms in intel_dp_pre_emphasis_max
> function. I will also add the missing condition:
> else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
> similar to intel_dp_voltage_max function
>
> > >
> > > Also we need to update the code to pick the correct swing/pre-
> > > emphasis
> > > when we can't do what is being requested.
> >
> > This check will need to be added in adjust_train() function
>
> sure, I will implement this logic in intel_get_adjust_train
> >
> > >
> > > The spec says:
> > > "When the combination of the requested pre-emphasis level and
> > > voltage
> > > swing exceeds the capability of a DPTX, the DPTX shall set the
> > > pre-emphasis
> > > level according to the request and use the highest voltage swing
> > > it can
> > > output with the given pre-emphasis level."
> > > and
> > > "When a DPTX reads a request beyond the limits of this Standard,
> > > the
> > > DPTX shall set the pre-emphasis level according to the request and
> > > set
> > > the highest voltage swing level it can output with the given pre-
> > > emphasis
> > > level. If a DPTX is requested for 9.5dB of pre-emphasis level (may
> > > be
> > > supported for a DPTX) and cannot support that level, it shall set
> > > the
> > > pre-emphasis level to the next highest level, 6dB."
> >
> > So my interpretation of this is :
> >
> > In adjust_train() function:
> >
> > vswing_max = intel_dp_voltage_max() which is set per platform
> > pre_emphasis_max = set to level 3
>
> pre_emphasis_max = intel_dp_pre_emphasis_max
> because it is set per platfrom as well, similar to vswing_max
>
> >
> > v = get_requested_voltage_swing() - Limit this to vmax
> > p = get_requested_pre_emphasis() - Limit this to pmax
> >
> > Now rewrite the intel_ddi_dp_pre_emphasis_max() function to call it
>
> intel_ddi_dp_pre_emphasis_max is needed to determine the max pre-
> emphasis level per platform.
What I meant was that define another function that will return a pre_emphasis_max
per platform but independent of the vswing values.
Makes sense?
Manasi
>
> > intel_ddi_dp_possible_vswing_max()
>
> I think the logic for choosing the correct vswing/emphasis combination
> should be in a different function, as you suggested it can be in
> intel_ddi_dp_possible_vswing_max
>
> Thanks
> Khaled
>
> > where we set the possible vswing max based on requested pre emphasis
> > and table in the bspec
> > Eg: if requested pre emphasis is 3, the vswing is 0 which is the max
> > vswing value
> > it can ouput with that pre emphasis level based on the bspec vswing
> > programming values
> >
> > Set v = that value
> > p = requested
> >
> > Link train
> >
> > Ville, is this logic correct?
> >
> > Regards
> > Manasi
> >
> >
> > >
> > >
> > >
> > > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > > > {
> > > > switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> > > > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> > > > --
> > > > 2.17.1
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15
2019-05-30 21:20 ` Manasi Navare
@ 2019-06-04 0:44 ` Khaled Almahallawy
0 siblings, 0 replies; 11+ messages in thread
From: Khaled Almahallawy @ 2019-06-04 0:44 UTC (permalink / raw)
To: Manasi Navare; +Cc: Intel-gfx
On 5/30/19 2:20 PM, Manasi Navare wrote:
> On Thu, May 30, 2019 at 12:33:40PM -0700, Almahallawy, Khaled wrote:
>> On Wed, 2019-05-22 at 12:25 -0700, Manasi Navare wrote:
>>> On Tue, May 21, 2019 at 04:24:58PM +0300, Ville Syrjälä wrote:
>>>> On Mon, May 20, 2019 at 04:25:41PM -0700, Khaled Almahallawy wrote:
>>>>> According to DP 1.4 standard, if the source supports four pre-
>>>>> emphasis levels, then the source shall set the bit MAX_PRE-
>>>>> EMPHASIS_REACHED = 1 only when trasmitter programmed PRE-
>>>>> EMPHASIS_SET field (bits 4:3) to 11b (Level 3). Pre-emphasis
>>>>> level 3 is the maximum pre-emphasis level that the source
>>>>> supports.
>>>>> Currently the MAX_PRE-EMPHASIS_REACHED bit is interpreted as the
>>>>> Max Pre-Emphasis level for certain Swing Level. This
>>>>> interpretation fails Link Layer compliance test 400.3.1.15 step
>>>>> 17 according to the following Fail condition:
>>>>> TRAINING_LANEx_SET.MAX_PRE-EMPHASIS_REACHED = 1 (check all active
>>>>> lanes) and the Source DUT supports pre-emphasis level 3 (9.5db).
>>>> Hmm. I guess that's correct. The spec doesn't say anything about
>>>> per-vswing pre-emphasis when talking about the 'max reached' bit.
>>>>
>>>>> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>>>>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>>>>> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/intel_ddi.c | 20 --------------------
>>>>> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>>>>> 2 files changed, 1 insertion(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> index 0af47f343faa..6540c979c098 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>>>> @@ -2239,26 +2239,6 @@ u8 intel_ddi_dp_voltage_max(struct
>>>>> intel_encoder *encoder)
>>>>> DP_TRAIN_VOLTAGE_SWING_MASK;
>>>>> }
>>>>>
>>>>> -/*
>>>>> - * We assume that the full set of pre-emphasis values can be
>>>>> - * used on all DDI platforms. Should that change we need to
>>>>> - * rethink this code.
>>>>> - */
>>>>> -u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
>>>>> u8 voltage_swing)
>>>>> -{
>>>>> - switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>>> - return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
>>>>> - return DP_TRAIN_PRE_EMPH_LEVEL_2;
>>>>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
>>>>> - return DP_TRAIN_PRE_EMPH_LEVEL_1;
>>>>> - case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
>>>>> - default:
>>>>> - return DP_TRAIN_PRE_EMPH_LEVEL_0;
>>>>> - }
>>>>> -}
>>>>> -
>>>>> static void cnl_ddi_vswing_program(struct intel_encoder
>>>>> *encoder,
>>>>> int level, enum intel_output_type
>>>>> type)
>>>>> {
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>>> index 77ba4da6b981..f94759e45862 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>> @@ -3541,7 +3541,7 @@ intel_dp_pre_emphasis_max(struct intel_dp
>>>>> *intel_dp, u8 voltage_swing)
>>>>> enum port port = encoder->port;
>>>>>
>>>>> if (HAS_DDI(dev_priv)) {
>>>>> - return intel_ddi_dp_pre_emphasis_max(encoder,
>>>>> voltage_swing);
>>>>> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
>>>> We're going to have to change this for all platforms.
>> Yes, I'm going to change for all platforms in intel_dp_pre_emphasis_max
>> function. I will also add the missing condition:
>> else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
>> similar to intel_dp_voltage_max function
>>
>>>> Also we need to update the code to pick the correct swing/pre-
>>>> emphasis
>>>> when we can't do what is being requested.
>>> This check will need to be added in adjust_train() function
>> sure, I will implement this logic in intel_get_adjust_train
>>>> The spec says:
>>>> "When the combination of the requested pre-emphasis level and
>>>> voltage
>>>> swing exceeds the capability of a DPTX, the DPTX shall set the
>>>> pre-emphasis
>>>> level according to the request and use the highest voltage swing
>>>> it can
>>>> output with the given pre-emphasis level."
>>>> and
>>>> "When a DPTX reads a request beyond the limits of this Standard,
>>>> the
>>>> DPTX shall set the pre-emphasis level according to the request and
>>>> set
>>>> the highest voltage swing level it can output with the given pre-
>>>> emphasis
>>>> level. If a DPTX is requested for 9.5dB of pre-emphasis level (may
>>>> be
>>>> supported for a DPTX) and cannot support that level, it shall set
>>>> the
>>>> pre-emphasis level to the next highest level, 6dB."
>>> So my interpretation of this is :
>>>
>>> In adjust_train() function:
>>>
>>> vswing_max = intel_dp_voltage_max() which is set per platform
>>> pre_emphasis_max = set to level 3
>>
>> pre_emphasis_max = intel_dp_pre_emphasis_max
>> because it is set per platfrom as well, similar to vswing_max
>>
>>>
>>> v = get_requested_voltage_swing() - Limit this to vmax
>>> p = get_requested_pre_emphasis() - Limit this to pmax
>>>
>>> Now rewrite the intel_ddi_dp_pre_emphasis_max() function to call it
>> intel_ddi_dp_pre_emphasis_max is needed to determine the max pre-
>> emphasis level per platform.
> What I meant was that define another function that will return a pre_emphasis_max
> per platform but independent of the vswing values.
> Makes sense?
>
> Manasi
Yes that make sense.
I thought of modifying intel_dp.c/intel_dp_pre_emphasis_max(struct
intel_dp *intel_dp, u8 voltage_swing)
to intel_dp_pre_emphasis_max(struct intel_dp *intel_dp) to return the
max pre-emphasis per platform
independent of vswing values. This function should be similar to
intel_dp.c/intel_dp_voltage_max(struct intel_dp *intel_dp)
And create intel_dp.c/intel_dp_possible_vswing_max(intel_dp *intel_dp,
u8 emphasis) to return the correct vswing/pre-emphasis combination,
given the requested
pre-emphasis value
What do you think?
Thanks
Khaled
>>> intel_ddi_dp_possible_vswing_max()
>> I think the logic for choosing the correct vswing/emphasis combination
>> should be in a different function, as you suggested it can be in
>> intel_ddi_dp_possible_vswing_max
>>
>> Thanks
>> Khaled
>>
>>> where we set the possible vswing max based on requested pre emphasis
>>> and table in the bspec
>>> Eg: if requested pre emphasis is 3, the vswing is 0 which is the max
>>> vswing value
>>> it can ouput with that pre emphasis level based on the bspec vswing
>>> programming values
>>>
>>> Set v = that value
>>> p = requested
>>>
>>> Link train
>>>
>>> Ville, is this logic correct?
>>>
>>> Regards
>>> Manasi
>>>
>>>
>>>>
>>>>
>>>>> } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>>>>> {
>>>>> switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>>>>> case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>>>>> --
>>>>> 2.17.1
>>>>>
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>> --
>>>> Ville Syrjälä
>>>> Intel
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-04 0:44 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20 23:25 [PATCH] drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 Khaled Almahallawy
2019-05-20 23:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-05-21 13:24 ` [PATCH] " Ville Syrjälä
2019-05-21 22:16 ` Manasi Navare
2019-05-22 19:25 ` Manasi Navare
2019-05-30 19:33 ` Almahallawy, Khaled
2019-05-30 21:20 ` Manasi Navare
2019-06-04 0:44 ` Khaled Almahallawy
2019-05-21 14:24 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix the interpretation of MAX_PRE-EMPHASIS_REACHED bit inorder to pass Link Layer compliance test number 400.3.1.15 (rev2) Patchwork
2019-05-21 14:44 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-22 6:17 ` ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.