On Fri, 13 Aug 2021, "Sharma, Swati2" <swati2.sharma@xxxxxxxxx> wrote: > Please check CI results once; if regressions are unrelated please report. > With those fixed. > > Reviewed-by: > Swati Sharma <swati2.sharma@xxxxxxxxx> > Tested-by: > Swati Sharma <swati2.sharma@xxxxxxxxx> 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@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Swati Sharma <swati2.sharma@xxxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> 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