diff mbox series

[RFC,4/4] drm/i915/mst: enable MST mode for 128b/132b single-stream sideband

Message ID a2c9faf86b5d93013fdd0be9bf3d6ba6228f0a8f.1706882591.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/mst: enable MST mode for 128b/132b single-stream sideband | expand

Commit Message

Jani Nikula Feb. 2, 2024, 2:05 p.m. UTC
If the sink supports 128b/132b and single-stream sideband messaging,
enable MST mode.

With this, the topology manager will still write DP_MSTM_CTRL, which
should be ignored by the sink. In the future,
drm_dp_mst_topology_mgr_set_mst() bool mst_state parameter should
probably be turned into an enum drm_dp_mst_mode mst_mode parameter.

Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Ville Syrjälä Feb. 2, 2024, 3:54 p.m. UTC | #1
On Fri, Feb 02, 2024 at 04:05:34PM +0200, Jani Nikula wrote:
> If the sink supports 128b/132b and single-stream sideband messaging,
> enable MST mode.
> 
> With this, the topology manager will still write DP_MSTM_CTRL, which
> should be ignored by the sink. In the future,
> drm_dp_mst_topology_mgr_set_mst() bool mst_state parameter should
> probably be turned into an enum drm_dp_mst_mode mst_mode parameter.

Rather I'd say the topology manager should stop concerning itself
with the MST enable bit and just frob the sideband enable bit.
The MST enable bit should be configured at modeset time to
reflect whether we're about to transmit in MST or SST mode.

> 
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4dd9c50226d1..16130e87dc23 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4020,7 +4020,9 @@ static bool intel_dp_mst_detect(struct intel_dp *intel_dp)
>  
>  	intel_dp->is_mst = i915->display.params.enable_dp_mst &&
>  		intel_dp_mst_source_support(intel_dp) &&
> -		sink_mst_mode == DP_MST_CAPABLE;
> +		(sink_mst_mode == DP_MST_CAPABLE ||
> +		 (sink_mst_mode == DP_MST_SIDEBAND_MSG &&
> +		  intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_128B132B));
>  
>  	drm_dbg_kms(&i915->drm,
>  		    "[ENCODER:%d:%s] MST support: port: %s, sink: %s, modparam: %s -> enable: %s\n",
> -- 
> 2.39.2
Jani Nikula Feb. 12, 2024, 3:30 p.m. UTC | #2
On Fri, 02 Feb 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Feb 02, 2024 at 04:05:34PM +0200, Jani Nikula wrote:
>> If the sink supports 128b/132b and single-stream sideband messaging,
>> enable MST mode.
>> 
>> With this, the topology manager will still write DP_MSTM_CTRL, which
>> should be ignored by the sink. In the future,
>> drm_dp_mst_topology_mgr_set_mst() bool mst_state parameter should
>> probably be turned into an enum drm_dp_mst_mode mst_mode parameter.
>
> Rather I'd say the topology manager should stop concerning itself
> with the MST enable bit and just frob the sideband enable bit.
> The MST enable bit should be configured at modeset time to
> reflect whether we're about to transmit in MST or SST mode.

Are you suggesting the driver should write the MST vs. SST mode in
DP_MSTM_CTRL?

I worry a bit about the rmw on DPCD regs. The topology manager only does
writes.

BR,
Jani.





>
>> 
>> Cc: Arun R Murthy <arun.r.murthy@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 4dd9c50226d1..16130e87dc23 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4020,7 +4020,9 @@ static bool intel_dp_mst_detect(struct intel_dp *intel_dp)
>>  
>>  	intel_dp->is_mst = i915->display.params.enable_dp_mst &&
>>  		intel_dp_mst_source_support(intel_dp) &&
>> -		sink_mst_mode == DP_MST_CAPABLE;
>> +		(sink_mst_mode == DP_MST_CAPABLE ||
>> +		 (sink_mst_mode == DP_MST_SIDEBAND_MSG &&
>> +		  intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_128B132B));
>>  
>>  	drm_dbg_kms(&i915->drm,
>>  		    "[ENCODER:%d:%s] MST support: port: %s, sink: %s, modparam: %s -> enable: %s\n",
>> -- 
>> 2.39.2
Ville Syrjälä Feb. 12, 2024, 3:36 p.m. UTC | #3
On Mon, Feb 12, 2024 at 05:30:40PM +0200, Jani Nikula wrote:
> On Fri, 02 Feb 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Feb 02, 2024 at 04:05:34PM +0200, Jani Nikula wrote:
> >> If the sink supports 128b/132b and single-stream sideband messaging,
> >> enable MST mode.
> >> 
> >> With this, the topology manager will still write DP_MSTM_CTRL, which
> >> should be ignored by the sink. In the future,
> >> drm_dp_mst_topology_mgr_set_mst() bool mst_state parameter should
> >> probably be turned into an enum drm_dp_mst_mode mst_mode parameter.
> >
> > Rather I'd say the topology manager should stop concerning itself
> > with the MST enable bit and just frob the sideband enable bit.
> > The MST enable bit should be configured at modeset time to
> > reflect whether we're about to transmit in MST or SST mode.
> 
> Are you suggesting the driver should write the MST vs. SST mode in
> DP_MSTM_CTRL?

Either that or there needs to be a function provided by the
topology manager that gets called at modeset time to configure it.
I don't recall when exactly it should be configured according the
DP spec.

> 
> I worry a bit about the rmw on DPCD regs. The topology manager only does
> writes.
> 
> BR,
> Jani.
> 
> 
> 
> 
> 
> >
> >> 
> >> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_dp.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 4dd9c50226d1..16130e87dc23 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -4020,7 +4020,9 @@ static bool intel_dp_mst_detect(struct intel_dp *intel_dp)
> >>  
> >>  	intel_dp->is_mst = i915->display.params.enable_dp_mst &&
> >>  		intel_dp_mst_source_support(intel_dp) &&
> >> -		sink_mst_mode == DP_MST_CAPABLE;
> >> +		(sink_mst_mode == DP_MST_CAPABLE ||
> >> +		 (sink_mst_mode == DP_MST_SIDEBAND_MSG &&
> >> +		  intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_128B132B));
> >>  
> >>  	drm_dbg_kms(&i915->drm,
> >>  		    "[ENCODER:%d:%s] MST support: port: %s, sink: %s, modparam: %s -> enable: %s\n",
> >> -- 
> >> 2.39.2
> 
> -- 
> Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4dd9c50226d1..16130e87dc23 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4020,7 +4020,9 @@  static bool intel_dp_mst_detect(struct intel_dp *intel_dp)
 
 	intel_dp->is_mst = i915->display.params.enable_dp_mst &&
 		intel_dp_mst_source_support(intel_dp) &&
-		sink_mst_mode == DP_MST_CAPABLE;
+		(sink_mst_mode == DP_MST_CAPABLE ||
+		 (sink_mst_mode == DP_MST_SIDEBAND_MSG &&
+		  intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & DP_CAP_ANSI_128B132B));
 
 	drm_dbg_kms(&i915->drm,
 		    "[ENCODER:%d:%s] MST support: port: %s, sink: %s, modparam: %s -> enable: %s\n",