[PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Arun R Murthy]

With a value of '0' read from MSTM_CAP register MST to be enabled.
DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
stream and not supporting single stream sideband MSG.
The underlying protocol will be MST to enable use of MTP.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9ff0cbd9c0df..40d3280f8d98 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 	if (!intel_dp_mst_source_support(intel_dp))
 		return;
 
-	intel_dp->is_mst = sink_can_mst &&
-		i915->display.params.enable_dp_mst;
+	/*
+	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates then
+	 * DP2.1 can be enabled with underlying protocol using MST for MTP
+	 */
+	intel_dp->is_mst = (sink_can_mst ||
+			    drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
+			    && i915->display.params.enable_dp_mst;
 
 	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
 					intel_dp->is_mst);
-- 
2.25.1

✗ Fi.CI.CHECKPATCH: warning for drm/i915/display/dp: 128/132b DP-capable with SST (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/display/dp: 128/132b DP-capable with SST (rev2)
URL   : https://patchwork.freedesktop.org/series/128147/
State : warning

== Summary ==

Error: dim checkpatch failed
3745b837c1be drm/i915/display/dp: 128/132b DP-capable with SST
-:29: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line
#29: FILE: drivers/gpu/drm/i915/display/intel_dp.c:4047:
+			    drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
+			    && i915->display.params.enable_dp_mst;

total: 0 errors, 0 warnings, 1 checks, 15 lines checked

✗ Fi.CI.BAT: failure for drm/i915/display/dp: 128/132b DP-capable with SST (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5982 bytes --]

== Series Details ==

Series: drm/i915/display/dp: 128/132b DP-capable with SST (rev2)
URL   : https://patchwork.freedesktop.org/series/128147/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14076 -> Patchwork_128147v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_128147v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_128147v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/index.html

Participating hosts (38 -> 37)
------------------------------

  Additional (1): bat-kbl-2 
  Missing    (2): fi-snb-2520m fi-pnv-d510 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_128147v2:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_addfb_basic@invalid-set-prop:
    - fi-kbl-7567u:       [PASS][1] -> [DMESG-WARN][2] +37 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/fi-kbl-7567u/igt@kms_addfb_basic@invalid-set-prop.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/fi-kbl-7567u/igt@kms_addfb_basic@invalid-set-prop.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@gt_contexts:
    - {bat-adls-6}:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/bat-adls-6/igt@i915_selftest@live@gt_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/bat-adls-6/igt@i915_selftest@live@gt_contexts.html

  
Known issues
------------

  Here are the changes found in Patchwork_128147v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@fbdev@info:
    - bat-kbl-2:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#1849])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/bat-kbl-2/igt@fbdev@info.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-kbl-2:          NOTRUN -> [SKIP][6] ([fdo#109271]) +36 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/bat-kbl-2/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@i915_module_load@load:
    - bat-adlp-6:         [PASS][7] -> [INCOMPLETE][8] ([i915#8449])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/bat-adlp-6/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/bat-adlp-6/igt@i915_module_load@load.html

  * igt@i915_selftest@live@sanitycheck:
    - fi-kbl-7567u:       [PASS][9] -> [DMESG-WARN][10] ([i915#9730]) +36 other tests dmesg-warn
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-7567u:       [PASS][11] -> [DMESG-WARN][12] ([i915#180]) +29 other tests dmesg-warn
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/fi-kbl-7567u/igt@kms_busy@basic@flip.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/fi-kbl-7567u/igt@kms_busy@basic@flip.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][13] -> [ABORT][14] ([i915#8668])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - fi-kbl-7567u:       [PASS][15] -> [DMESG-WARN][16] ([i915#180] / [i915#8585]) +16 other tests dmesg-warn
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-kbl-7567u:       [PASS][17] -> [DMESG-WARN][18] ([i915#8585]) +3 other tests dmesg-warn
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14076/fi-kbl-7567u/igt@prime_vgem@basic-fence-flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/fi-kbl-7567u/igt@prime_vgem@basic-fence-flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#8449]: https://gitlab.freedesktop.org/drm/intel/issues/8449
  [i915#8585]: https://gitlab.freedesktop.org/drm/intel/issues/8585
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#9730]: https://gitlab.freedesktop.org/drm/intel/issues/9730


Build changes
-------------

  * Linux: CI_DRM_14076 -> Patchwork_128147v2

  CI-20190529: 20190529
  CI_DRM_14076: 6fb23c8c47c3c76c9ea4e62d3e4244eb42a6d081 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7655: ddf7cf40a00caa7d02f3729e1e50f78f102463d9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_128147v2: 6fb23c8c47c3c76c9ea4e62d3e4244eb42a6d081 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

76f8a69e0277 drm/i915/display/dp: 128/132b DP-capable with SST

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128147v2/index.html

[-- Attachment #2: Type: text/html, Size: 7061 bytes --]

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Almahallawy, Khaled]

Thank you for the patch

Tested-by: Khaled Almahallawy <Khaled.almahallawy@intel.com>

-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Arun R Murthy
Sent: Wednesday, January 3, 2024 1:07 AM
To: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>; Deak, Imre <imre.deak@intel.com>
Subject: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

With a value of '0' read from MSTM_CAP register MST to be enabled.
DP2.1 SCR updates the spec for 128/132b DP capable supporting only one stream and not supporting single stream sideband MSG.
The underlying protocol will be MST to enable use of MTP.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9ff0cbd9c0df..40d3280f8d98 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
 	if (!intel_dp_mst_source_support(intel_dp))
 		return;
 
-	intel_dp->is_mst = sink_can_mst &&
-		i915->display.params.enable_dp_mst;
+	/*
+	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates then
+	 * DP2.1 can be enabled with underlying protocol using MST for MTP
+	 */
+	intel_dp->is_mst = (sink_can_mst ||
+			    drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
+			    && i915->display.params.enable_dp_mst;
 
 	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
 					intel_dp->is_mst);
--
2.25.1

Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Nautiyal, Ankit K]

On 1/3/2024 2:37 PM, Arun R Murthy wrote:
> With a value of '0' read from MSTM_CAP register MST to be enabled.
> DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
> stream and not supporting single stream sideband MSG.

I think, we still need to read bit DP_SINGLE_STREAM_SIDEBAND_MSG for 
single stream sideband capability.

Only if the MST_CAP is 1 we can ignore the DP_SINGLE_STREAM_SIDEBAND_MSG.

> The underlying protocol will be MST to enable use of MTP.
>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 9ff0cbd9c0df..40d3280f8d98 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>   	if (!intel_dp_mst_source_support(intel_dp))
>   		return;
>   
> -	intel_dp->is_mst = sink_can_mst &&
> -		i915->display.params.enable_dp_mst;
> +	/*
> +	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates then
> +	 * DP2.1 can be enabled with underlying protocol using MST for MTP
> +	 */

I am not able to find this in the spec, am I missing anything here? If 
MST_CAP [Bit 0]  is 0, for both 8b/10b and 128b/132b, it says it does 
not support Multi stream Transport.

Regards,

Ankit


> +	intel_dp->is_mst = (sink_can_mst ||
> +			    drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> +			    && i915->display.params.enable_dp_mst;
>   
>   	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>   					intel_dp->is_mst);

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Murthy, Arun R]

> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Sent: Monday, January 8, 2024 5:08 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
> Nikula, Jani <jani.nikula@intel.com>; Deak, Imre <imre.deak@intel.com>
> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> 
> 
> On 1/3/2024 2:37 PM, Arun R Murthy wrote:
> > With a value of '0' read from MSTM_CAP register MST to be enabled.
> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
> > stream and not supporting single stream sideband MSG.
> 
> I think, we still need to read bit DP_SINGLE_STREAM_SIDEBAND_MSG for single
> stream sideband capability.

This will be handled separately as part of https://jira.devtools.intel.com/browse/VLK-55112

> 
> Only if the MST_CAP is 1 we can ignore the
> DP_SINGLE_STREAM_SIDEBAND_MSG.
> 
> > The underlying protocol will be MST to enable use of MTP.
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 9ff0cbd9c0df..40d3280f8d98 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
> >   	if (!intel_dp_mst_source_support(intel_dp))
> >   		return;
> >
> > -	intel_dp->is_mst = sink_can_mst &&
> > -		i915->display.params.enable_dp_mst;
> > +	/*
> > +	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
> then
> > +	 * DP2.1 can be enabled with underlying protocol using MST for MTP
> > +	 */
> 
> I am not able to find this in the spec, am I missing anything here? If MST_CAP
> [Bit 0]  is 0, for both 8b/10b and 128b/132b, it says it does not support Multi
> stream Transport.
> 

This is available in the spec under section 2.14.4.1.1

Thanks and Regards,
Arun R Murthy
-------------------

> Regards,
> 
> Ankit
> 
> 
> > +	intel_dp->is_mst = (sink_can_mst ||
> > +
> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> > +			    && i915->display.params.enable_dp_mst;
> >
> >   	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> >   					intel_dp->is_mst);

Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> With a value of '0' read from MSTM_CAP register MST to be enabled.
> DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
> stream and not supporting single stream sideband MSG.
> The underlying protocol will be MST to enable use of MTP.
>
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 9ff0cbd9c0df..40d3280f8d98 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>  	if (!intel_dp_mst_source_support(intel_dp))
>  		return;
>  
> -	intel_dp->is_mst = sink_can_mst &&
> -		i915->display.params.enable_dp_mst;
> +	/*
> +	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates then
> +	 * DP2.1 can be enabled with underlying protocol using MST for MTP
> +	 */
> +	intel_dp->is_mst = (sink_can_mst ||
> +			    drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> +			    && i915->display.params.enable_dp_mst;

We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether
the link rate in the *crtc state* is uhbr, and by proxy whether the link
in the *crtc state* is 128b/132b.

There, we've already decided to use uhbr and 128b/132b.

This one here is different, and I think it's taking us to the wrong
direction. For example, it should be possible to downgrade the link from
uhbr to non-uhbr on link failures. We don't have that yet. But this
makes untangling that even harder.


BR,
Jani.


>  
>  	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>  					intel_dp->is_mst);

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Murthy, Arun R]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Monday, January 8, 2024 7:01 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
> Deak, Imre <imre.deak@intel.com>
> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> 
> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> > With a value of '0' read from MSTM_CAP register MST to be enabled.
> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
> > stream and not supporting single stream sideband MSG.
> > The underlying protocol will be MST to enable use of MTP.
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 9ff0cbd9c0df..40d3280f8d98 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
> >  	if (!intel_dp_mst_source_support(intel_dp))
> >  		return;
> >
> > -	intel_dp->is_mst = sink_can_mst &&
> > -		i915->display.params.enable_dp_mst;
> > +	/*
> > +	 * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
> then
> > +	 * DP2.1 can be enabled with underlying protocol using MST for MTP
> > +	 */
> > +	intel_dp->is_mst = (sink_can_mst ||
> > +
> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> > +			    && i915->display.params.enable_dp_mst;
> 
> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether the
> link rate in the *crtc state* is uhbr, and by proxy whether the link in the *crtc
> state* is 128b/132b.
> 
Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b. Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then 128/132b is not enabled. We need to deviate here and a value of bit[0]=0, bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported then enable 128/132b.

> There, we've already decided to use uhbr and 128b/132b.
> 
> This one here is different, and I think it's taking us to the wrong direction. For
> example, it should be possible to downgrade the link from uhbr to non-uhbr on
> link failures. We don't have that yet. But this makes untangling that even
> harder.

Yes upon having the fallback, I think we will have to handle fallback in this case separately. Change might be required in drm core, drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate this 0x00 value. 

Will be floating the fallback patches very soon.

Thanks and Regards,
Arun R Murthy
--------------------
> 
> 
> BR,
> Jani.
> 
> 
> >
> >  	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> >  					intel_dp->is_mst);
> 
> --
> Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Monday, January 8, 2024 7:01 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
>> Deak, Imre <imre.deak@intel.com>
>> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
>>
>> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
>> > stream and not supporting single stream sideband MSG.
>> > The underlying protocol will be MST to enable use of MTP.
>> >
>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 9ff0cbd9c0df..40d3280f8d98 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>> >     if (!intel_dp_mst_source_support(intel_dp))
>> >             return;
>> >
>> > -   intel_dp->is_mst = sink_can_mst &&
>> > -           i915->display.params.enable_dp_mst;
>> > +   /*
>> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
>> then
>> > +    * DP2.1 can be enabled with underlying protocol using MST for MTP
>> > +    */
>> > +   intel_dp->is_mst = (sink_can_mst ||
>> > +
>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
>> > +                       && i915->display.params.enable_dp_mst;
>>
>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether the
>> link rate in the *crtc state* is uhbr, and by proxy whether the link in the *crtc
>> state* is 128b/132b.
>>
> Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b. Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then 128/132b is not enabled. We need to deviate here and a value of bit[0]=0, bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported then enable 128/132b.

Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
DP_CAP_ANSI_128B132B, why not use that here too?

>
>> There, we've already decided to use uhbr and 128b/132b.
>>
>> This one here is different, and I think it's taking us to the wrong direction. For
>> example, it should be possible to downgrade the link from uhbr to non-uhbr on
>> link failures. We don't have that yet. But this makes untangling that even
>> harder.
>
> Yes upon having the fallback, I think we will have to handle fallback in this case separately. Change might be required in drm core, drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate this 0x00 value.
>
> Will be floating the fallback patches very soon.
>
> Thanks and Regards,
> Arun R Murthy
> --------------------
>>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> >     drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>> >                                     intel_dp->is_mst);
>>
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Tue, 09 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>>> -----Original Message-----
>>> From: Nikula, Jani <jani.nikula@intel.com>
>>> Sent: Monday, January 8, 2024 7:01 PM
>>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
>>> Deak, Imre <imre.deak@intel.com>
>>> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
>>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
>>>
>>> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
>>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
>>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only one
>>> > stream and not supporting single stream sideband MSG.
>>> > The underlying protocol will be MST to enable use of MTP.
>>> >
>>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
>>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>>> > index 9ff0cbd9c0df..40d3280f8d98 100644
>>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp *intel_dp)
>>> >     if (!intel_dp_mst_source_support(intel_dp))
>>> >             return;
>>> >
>>> > -   intel_dp->is_mst = sink_can_mst &&
>>> > -           i915->display.params.enable_dp_mst;
>>> > +   /*
>>> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR rates
>>> then
>>> > +    * DP2.1 can be enabled with underlying protocol using MST for MTP
>>> > +    */
>>> > +   intel_dp->is_mst = (sink_can_mst ||
>>> > +
>>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
>>> > +                       && i915->display.params.enable_dp_mst;
>>>
>>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine whether the
>>> link rate in the *crtc state* is uhbr, and by proxy whether the link in the *crtc
>>> state* is 128b/132b.
>>>
>> Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b. Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then 128/132b is not enabled. We need to deviate here and a value of bit[0]=0, bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported then enable 128/132b.
>
> Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
> DP_CAP_ANSI_128B132B, why not use that here too?

Also, shouldn't we ensure we don't try to do more than one stream?

>
>>
>>> There, we've already decided to use uhbr and 128b/132b.
>>>
>>> This one here is different, and I think it's taking us to the wrong direction. For
>>> example, it should be possible to downgrade the link from uhbr to non-uhbr on
>>> link failures. We don't have that yet. But this makes untangling that even
>>> harder.
>>
>> Yes upon having the fallback, I think we will have to handle fallback in this case separately. Change might be required in drm core, drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate this 0x00 value.
>>
>> Will be floating the fallback patches very soon.
>>
>> Thanks and Regards,
>> Arun R Murthy
>> --------------------
>>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> >
>>> >     drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>>> >                                     intel_dp->is_mst);
>>>
>>> --
>>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Murthy, Arun R]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Tuesday, January 9, 2024 2:59 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
> Deak, Imre <imre.deak@intel.com>
> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> 
> On Tue, 09 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> >>> -----Original Message-----
> >>> From: Nikula, Jani <jani.nikula@intel.com>
> >>> Sent: Monday, January 8, 2024 7:01 PM
> >>> To: Murthy, Arun R <arun.r.murthy@intel.com>;
> >>> intel-gfx@lists.freedesktop.org; Deak, Imre <imre.deak@intel.com>
> >>> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
> >>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with
> >>> SST
> >>>
> >>> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> >>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
> >>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only
> >>> > one stream and not supporting single stream sideband MSG.
> >>> > The underlying protocol will be MST to enable use of MTP.
> >>> >
> >>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> >>> > ---
> >>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
> >>> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >>> >
> >>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >>> > b/drivers/gpu/drm/i915/display/intel_dp.c
> >>> > index 9ff0cbd9c0df..40d3280f8d98 100644
> >>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp
> *intel_dp)
> >>> >     if (!intel_dp_mst_source_support(intel_dp))
> >>> >             return;
> >>> >
> >>> > -   intel_dp->is_mst = sink_can_mst &&
> >>> > -           i915->display.params.enable_dp_mst;
> >>> > +   /*
> >>> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR
> >>> > + rates
> >>> then
> >>> > +    * DP2.1 can be enabled with underlying protocol using MST for MTP
> >>> > +    */
> >>> > +   intel_dp->is_mst = (sink_can_mst ||
> >>> > +
> >>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> >>> > +                       && i915->display.params.enable_dp_mst;
> >>>
> >>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine
> >>> whether the link rate in the *crtc state* is uhbr, and by proxy
> >>> whether the link in the *crtc
> >>> state* is 128b/132b.
> >>>
> >> Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b.
> Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then
> 128/132b is not enabled. We need to deviate here and a value of bit[0]=0,
> bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added
> to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported
> then enable 128/132b.
> >
> > Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
> > DP_CAP_ANSI_128B132B, why not use that here too?
> 
> Also, shouldn't we ensure we don't try to do more than one stream?
> 
Yes, this will be taken care of in a separate patch, tracked as part of separate JIRA. VLK-55112.

Thanks and Regards,
Arun R Murthy
--------------------
> >
> >>
> >>> There, we've already decided to use uhbr and 128b/132b.
> >>>
> >>> This one here is different, and I think it's taking us to the wrong
> >>> direction. For example, it should be possible to downgrade the link
> >>> from uhbr to non-uhbr on link failures. We don't have that yet. But
> >>> this makes untangling that even harder.
> >>
> >> Yes upon having the fallback, I think we will have to handle fallback in this
> case separately. Change might be required in drm core,
> drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate
> this 0x00 value.
> >>
> >> Will be floating the fallback patches very soon.
> >>
> >> Thanks and Regards,
> >> Arun R Murthy
> >> --------------------
> >>>
> >>>
> >>> BR,
> >>> Jani.
> >>>
> >>>
> >>> >
> >>> >     drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> >>> >                                     intel_dp->is_mst);
> >>>
> >>> --
> >>> Jani Nikula, Intel
> 
> --
> Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Jani Nikula]

On Wed, 10 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Tuesday, January 9, 2024 2:59 PM
>> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
>> Deak, Imre <imre.deak@intel.com>
>> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
>>
>> On Tue, 09 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
>> > On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
>> >>> -----Original Message-----
>> >>> From: Nikula, Jani <jani.nikula@intel.com>
>> >>> Sent: Monday, January 8, 2024 7:01 PM
>> >>> To: Murthy, Arun R <arun.r.murthy@intel.com>;
>> >>> intel-gfx@lists.freedesktop.org; Deak, Imre <imre.deak@intel.com>
>> >>> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
>> >>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with
>> >>> SST
>> >>>
>> >>> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
>> >>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
>> >>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only
>> >>> > one stream and not supporting single stream sideband MSG.
>> >>> > The underlying protocol will be MST to enable use of MTP.
>> >>> >
>> >>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
>> >>> > ---
>> >>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
>> >>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >>> >
>> >>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> >>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> >>> > index 9ff0cbd9c0df..40d3280f8d98 100644
>> >>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> >>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> >>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp
>> *intel_dp)
>> >>> >     if (!intel_dp_mst_source_support(intel_dp))
>> >>> >             return;
>> >>> >
>> >>> > -   intel_dp->is_mst = sink_can_mst &&
>> >>> > -           i915->display.params.enable_dp_mst;
>> >>> > +   /*
>> >>> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR
>> >>> > + rates
>> >>> then
>> >>> > +    * DP2.1 can be enabled with underlying protocol using MST for MTP
>> >>> > +    */
>> >>> > +   intel_dp->is_mst = (sink_can_mst ||
>> >>> > +
>> >>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
>> >>> > +                       && i915->display.params.enable_dp_mst;
>> >>>
>> >>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine
>> >>> whether the link rate in the *crtc state* is uhbr, and by proxy
>> >>> whether the link in the *crtc
>> >>> state* is 128b/132b.
>> >>>
>> >> Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b.
>> Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then
>> 128/132b is not enabled. We need to deviate here and a value of bit[0]=0,
>> bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added
>> to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported
>> then enable 128/132b.
>> >
>> > Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
>> > DP_CAP_ANSI_128B132B, why not use that here too?
>>
>> Also, shouldn't we ensure we don't try to do more than one stream?
>>
> Yes, this will be taken care of in a separate patch, tracked as part of separate JIRA. VLK-55112.

Well, you shouldn't really first open the possibility for a broken
config, and then say it'll be taken care of in the future.

BR,
Jani.


-- 
Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Murthy, Arun R]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Wednesday, January 10, 2024 4:24 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
> Deak, Imre <imre.deak@intel.com>
> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> 
> On Wed, 10 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula@intel.com>
> >> Sent: Tuesday, January 9, 2024 2:59 PM
> >> To: Murthy, Arun R <arun.r.murthy@intel.com>;
> >> intel-gfx@lists.freedesktop.org; Deak, Imre <imre.deak@intel.com>
> >> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with
> >> SST
> >>
> >> On Tue, 09 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> >> > On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> >> >>> -----Original Message-----
> >> >>> From: Nikula, Jani <jani.nikula@intel.com>
> >> >>> Sent: Monday, January 8, 2024 7:01 PM
> >> >>> To: Murthy, Arun R <arun.r.murthy@intel.com>;
> >> >>> intel-gfx@lists.freedesktop.org; Deak, Imre <imre.deak@intel.com>
> >> >>> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
> >> >>> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable
> >> >>> with SST
> >> >>>
> >> >>> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> >> >>> > With a value of '0' read from MSTM_CAP register MST to be enabled.
> >> >>> > DP2.1 SCR updates the spec for 128/132b DP capable supporting
> >> >>> > only one stream and not supporting single stream sideband MSG.
> >> >>> > The underlying protocol will be MST to enable use of MTP.
> >> >>> >
> >> >>> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> >> >>> > ---
> >> >>> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
> >> >>> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >> >>> >
> >> >>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> >>> > b/drivers/gpu/drm/i915/display/intel_dp.c
> >> >>> > index 9ff0cbd9c0df..40d3280f8d98 100644
> >> >>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> >>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> >>> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp
> >> *intel_dp)
> >> >>> >     if (!intel_dp_mst_source_support(intel_dp))
> >> >>> >             return;
> >> >>> >
> >> >>> > -   intel_dp->is_mst = sink_can_mst &&
> >> >>> > -           i915->display.params.enable_dp_mst;
> >> >>> > +   /*
> >> >>> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports
> >> >>> > + UHBR rates
> >> >>> then
> >> >>> > +    * DP2.1 can be enabled with underlying protocol using MST for
> MTP
> >> >>> > +    */
> >> >>> > +   intel_dp->is_mst = (sink_can_mst ||
> >> >>> > +
> >> >>> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> >> >>> > +                       && i915->display.params.enable_dp_mst;
> >> >>>
> >> >>> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine
> >> >>> whether the link rate in the *crtc state* is uhbr, and by proxy
> >> >>> whether the link in the *crtc
> >> >>> state* is 128b/132b.
> >> >>>
> >> >> Yes! If the link rate in crtc_state is not uhbr then we dont enable
> 128/132b.
> >> Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not
> >> mst then 128/132b is not enabled. We need to deviate here and a value
> >> of bit[0]=0,
> >> bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack
> >> is added to see if the return from MSTM_CAP is 0x00 and if uhbr rates
> >> are supported then enable 128/132b.
> >> >
> >> > Per spec it depends on intel_dp-
> >dpcd[DP_MAIN_LINK_CHANNEL_CODING]
> >> > & DP_CAP_ANSI_128B132B, why not use that here too?
> >>
> >> Also, shouldn't we ensure we don't try to do more than one stream?
> >>
> > Yes, this will be taken care of in a separate patch, tracked as part of separate
> JIRA. VLK-55112.
> 
> Well, you shouldn't really first open the possibility for a broken config, and then
> say it'll be taken care of in the future.
> 

Sorry, I misread it as single stream sideband msg. As per the transport is considered not more than one stream will be considered(In this case MSTM_CAP=0x00) based on the sink capability.
If a hub is connected inbetween the panel and the source, then source reads MSTM_CAP as 1xb and native MST will be enabled.

Thanks and Regards,
Arun R Murthy
--------------------
> BR,
> Jani.
> 
> 
> --
> Jani Nikula, Intel

RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST

[Murthy, Arun R]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Tuesday, January 9, 2024 2:58 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org;
> Deak, Imre <imre.deak@intel.com>
> Subject: RE: [PATCH] drm/i915/display/dp: 128/132b DP-capable with SST
> 
> On Tue, 09 Jan 2024, "Murthy, Arun R" <arun.r.murthy@intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula@intel.com>
> >> Sent: Monday, January 8, 2024 7:01 PM
> >> To: Murthy, Arun R <arun.r.murthy@intel.com>;
> >> intel-gfx@lists.freedesktop.org; Deak, Imre <imre.deak@intel.com>
> >> Cc: Murthy, Arun R <arun.r.murthy@intel.com>
> >> Subject: Re: [PATCH] drm/i915/display/dp: 128/132b DP-capable with
> >> SST
> >>
> >> On Wed, 03 Jan 2024, Arun R Murthy <arun.r.murthy@intel.com> wrote:
> >> > With a value of '0' read from MSTM_CAP register MST to be enabled.
> >> > DP2.1 SCR updates the spec for 128/132b DP capable supporting only
> >> > one stream and not supporting single stream sideband MSG.
> >> > The underlying protocol will be MST to enable use of MTP.
> >> >
> >> > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_dp.c | 9 +++++++--
> >> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > index 9ff0cbd9c0df..40d3280f8d98 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > @@ -4038,8 +4038,13 @@ intel_dp_configure_mst(struct intel_dp
> *intel_dp)
> >> >     if (!intel_dp_mst_source_support(intel_dp))
> >> >             return;
> >> >
> >> > -   intel_dp->is_mst = sink_can_mst &&
> >> > -           i915->display.params.enable_dp_mst;
> >> > +   /*
> >> > +    * Even if dpcd reg MSTM_CAP is 0, if the sink supports UHBR
> >> > + rates
> >> then
> >> > +    * DP2.1 can be enabled with underlying protocol using MST for MTP
> >> > +    */
> >> > +   intel_dp->is_mst = (sink_can_mst ||
> >> > +
> >> drm_dp_is_uhbr_rate(intel_dp_max_common_rate(intel_dp)))
> >> > +                       && i915->display.params.enable_dp_mst;
> >>
> >> We use drm_dp_is_uhbr_rate() in intel_dp_is_uhbr() to determine
> >> whether the link rate in the *crtc state* is uhbr, and by proxy
> >> whether the link in the *crtc
> >> state* is 128b/132b.
> >>
> > Yes! If the link rate in crtc_state is not uhbr then we dont enable 128/132b.
> Also the return from drm_dp_read_mst_cap() return 0 or 1 and if not mst then
> 128/132b is not enabled. We need to deviate here and a value of bit[0]=0,
> bit[1]=0 in MSTM_CAP register is 128/132b with SST. Hence this hack is added
> to see if the return from MSTM_CAP is 0x00 and if uhbr rates are supported
> then enable 128/132b.
> 
> Per spec it depends on intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
> DP_CAP_ANSI_128B132B, why not use that here too?
> 
Done!

Thanks and Regards,
Arun R Murthy
-------------------
> >
> >> There, we've already decided to use uhbr and 128b/132b.
> >>
> >> This one here is different, and I think it's taking us to the wrong
> >> direction. For example, it should be possible to downgrade the link
> >> from uhbr to non-uhbr on link failures. We don't have that yet. But
> >> this makes untangling that even harder.
> >
> > Yes upon having the fallback, I think we will have to handle fallback in this
> case separately. Change might be required in drm core,
> drm_dp_read_mst_cap() should consider the DP2.1 changes to accommodate
> this 0x00 value.
> >
> > Will be floating the fallback patches very soon.
> >
> > Thanks and Regards,
> > Arun R Murthy
> > --------------------
> >>
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >
> >> >     drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> >> >                                     intel_dp->is_mst);
> >>
> >> --
> >> Jani Nikula, Intel
> 
> --
> Jani Nikula, Intel