| Message ID | 20240103090715.307309-1-arun.r.murthy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display/dp: 128/132b DP-capable with SST | expand |
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
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);
> -----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);
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);
> -----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
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
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
> -----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
> -----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
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.
> -----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
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);
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(-)