Re: [Intel-gfx] [PATCH] drm/i915/edp: fix eDP MSO pipe sanity checks for ADL-P

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

On Fri, 13 Aug 2021, "Sharma, Swati2" <swati2.sharma@intel.com> wrote:
> Please check CI results once; if regressions are unrelated please report.
> With those fixed.
>
> Reviewed-by:
> Swati Sharma <swati2.sharma@intel.com>
> Tested-by:
> Swati Sharma <swati2.sharma@intel.com>

Thanks, pushed. The CI failures are unrelated.

BR,
Jani.

>
> On 12-Aug-21 6:53 PM, Jani Nikula wrote:
>> ADL-P supports stream splitter on pipe B in addition to pipe A. Update
>> the sanity check in intel_ddi_mso_get_config() to reflect this, and
>> remove the check in intel_ddi_mso_configure() as redundant with
>> encoder->pipe_mask. Abstract the splitter pipe mask to a single point of
>> truth while at it to avoid similar mistakes in the future.
>> 
>> Fixes: 7bc188cc2c8c ("drm/i915/adl_p: enable MSO on pipe B")
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Swati Sharma <swati2.sharma@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c | 24 ++++++++++++------------
>>   1 file changed, 12 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index e932fd0fe7e2..eb9aa871f87b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2251,6 +2251,15 @@ static void intel_ddi_power_up_lanes(struct intel_encoder *encoder,
>>   	}
>>   }
>>   
>> +/* Splitter enable for eDP MSO is limited to certain pipes. */
>> +static u8 intel_ddi_splitter_pipe_mask(struct drm_i915_private *i915)
>> +{
>> +	if (IS_ALDERLAKE_P(i915))
>> +		return BIT(PIPE_A) | BIT(PIPE_B);
>> +	else
>> +		return BIT(PIPE_A);
>> +}
>> +
>>   static void intel_ddi_mso_get_config(struct intel_encoder *encoder,
>>   				     struct intel_crtc_state *pipe_config)
>>   {
>> @@ -2268,8 +2277,7 @@ static void intel_ddi_mso_get_config(struct intel_encoder *encoder,
>>   	if (!pipe_config->splitter.enable)
>>   		return;
>>   
>> -	/* Splitter enable is supported for pipe A only. */
>> -	if (drm_WARN_ON(&i915->drm, pipe != PIPE_A)) {
>> +	if (drm_WARN_ON(&i915->drm, !(intel_ddi_splitter_pipe_mask(i915) & BIT(pipe)))) {
>>   		pipe_config->splitter.enable = false;
>>   		return;
>>   	}
>> @@ -2301,10 +2309,6 @@ static void intel_ddi_mso_configure(const struct intel_crtc_state *crtc_state)
>>   		return;
>>   
>>   	if (crtc_state->splitter.enable) {
>> -		/* Splitter enable is supported for pipe A only. */
>> -		if (drm_WARN_ON(&i915->drm, pipe != PIPE_A))
>> -			return;
>> -
>>   		dss1 |= SPLITTER_ENABLE;
>>   		dss1 |= OVERLAP_PIXELS(crtc_state->splitter.pixel_overlap);
>>   		if (crtc_state->splitter.link_count == 2)
>> @@ -4666,12 +4670,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   
>>   		dig_port->hpd_pulse = intel_dp_hpd_pulse;
>>   
>> -		/* Splitter enable for eDP MSO is limited to certain pipes. */
>> -		if (dig_port->dp.mso_link_count) {
>> -			encoder->pipe_mask = BIT(PIPE_A);
>> -			if (IS_ALDERLAKE_P(dev_priv))
>> -				encoder->pipe_mask |= BIT(PIPE_B);
>> -		}
>> +		if (dig_port->dp.mso_link_count)
>> +			encoder->pipe_mask = intel_ddi_splitter_pipe_mask(dev_priv);
>>   	}
>>   
>>   	/* In theory we don't need the encoder->type check, but leave it just in
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center