Re: [PATCH] drm/i915/ddi: clarify intel_ddi_connector_get_hw_state() for DP MST

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 25 Nov 2024, Imre Deak <imre.deak@intel.com> wrote:
> On Mon, Nov 25, 2024 at 02:09:59PM +0200, Jani Nikula wrote:
>> encoder->get_hw_state() returns false for DP MST, and currently always
>> interprets 128b/132b as MST. Therefore the DDI MST mode checks in
>> intel_ddi_connector_get_hw_state() are redundant.
>> 
>> Prepare for future, and handle 128b/132b SST and warn on 8b/10b MST.
>> 
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Looks ok to me:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks, pushed to drm-intel-next.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index e25b712bf03b..7d37ddd9ad12 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -731,6 +731,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
>>  	if (!wakeref)
>>  		return false;
>>  
>> +	/* Note: This returns false for DP MST primary encoders. */
>>  	if (!encoder->get_hw_state(encoder, &pipe)) {
>>  		ret = false;
>>  		goto out;
>> @@ -752,12 +753,14 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
>>  	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_SST) {
>>  		ret = type == DRM_MODE_CONNECTOR_eDP ||
>>  			type == DRM_MODE_CONNECTOR_DisplayPort;
>> -	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST ||
>> -		   (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display))) {
>> +	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display)) {
>>  		/*
>> -		 * If the transcoder is in MST state then connector isn't
>> -		 * connected.
>> +		 * encoder->get_hw_state() should have bailed out on MST. This
>> +		 * must be SST and non-eDP.
>>  		 */
>> +		ret = type == DRM_MODE_CONNECTOR_DisplayPort;
>> +	} else if (drm_WARN_ON(display->drm, ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST)) {
>> +		/* encoder->get_hw_state() should have bailed out on MST. */
>>  		ret = false;
>>  	} else {
>>  		ret = false;
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel