> -----Original Message----- > From: Manasi Navare <navaremanasi@xxxxxxxxxxxx> > Sent: Tuesday, February 27, 2024 11:37 PM > To: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Cc: Lisovskiy, Stanislav <stanislav.lisovskiy@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx; Saarinen, Jani <jani.saarinen@xxxxxxxxx>; > ville.syrjala@xxxxxxxxxxxxxxx; Srinivas, Vidya <vidya.srinivas@xxxxxxxxx> > Subject: Re: [PATCH 3/3] drm/i915: Fix bigjoiner case for DP2.0 > > Thanks Jani for your review. > Thanks @Lisovskiy, Stanislav and @vidya.srinivas@xxxxxxxxx for taking this > patch forward. > > @Jani Nikula , @Ville Syrjälä : MST bigjoiner as a feature needs to be enabled > upstream and this patch enables that feature. > If you agree that bigjoiner refactoring patches 1 and 2 have no impact on > enabling bigjoiner on MST, could we decouple this patch from bigjoiner > refactoring and land this separately? Hello Manasi Thank you. I have submitted this series as suggested after addressing comments from Jani Nikula about the commit message errors. https://patchwork.freedesktop.org/series/130449/ Regards Vidya > > We need the Bigjoiner to be enabled on MST feature landed asap and > bigjoiner refactoring can follow. > > Regards > Manasi > > On Tue, Feb 27, 2024 at 1:15 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > wrote: > > > > On Tue, 27 Feb 2024, "Lisovskiy, Stanislav" <stanislav.lisovskiy@xxxxxxxxx> > wrote: > > > On Mon, Feb 26, 2024 at 09:56:10PM +0200, Jani Nikula wrote: > > >> On Wed, 21 Feb 2024, Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > wrote: > > >> > Patch calculates bigjoiner pipes in mst compute. > > >> > Patch also passes bigjoiner bool to validate plane max size. > > >> > > >> Please use the imperative mood in commit messages, e.g. "calculate" > > >> intead of "calculates". > > >> > > >> Please do not refer to "patch". We know it's a patch, until it > > >> isn't, and then it's a commit. > > >> > > >> Please explain *why* the changes are being done, not just *what* is > > >> being done. > > >> > > >> In the subject, what is "bigjoiner case for DP2.0"? DP 2.0 is a > > >> spec version, and as such irrelevant for the changes being done. > > >> > > >> > Signed-off-by: vsrini4 <vidya.srinivas@xxxxxxxxx> > > >> > > >> ? > > > > > > Hi Jani, I just added that patch from Vidya to my series, to be > > > honest, didn't have time at all to look much into it. > > > Looks like its me who is going to fix that. > > > > Should the original authorship be preserved? If not, please add > > Co-developed-by. Just having the Signed-off-by is not enough. > > > > BR, > > Jani. > > > > > > > > > >> > > >> > Signed-off-by: Stanislav Lisovskiy > > >> > <stanislav.lisovskiy@xxxxxxxxx> > > >> > --- > > >> > drivers/gpu/drm/i915/display/intel_dp_mst.c | 19 > > >> > ++++++++++++------- > > >> > 1 file changed, 12 insertions(+), 7 deletions(-) > > >> > > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >> > index 5307ddd4edcf5..fd27d9976c050 100644 > > >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > >> > @@ -523,6 +523,7 @@ static int intel_dp_mst_compute_config(struct > intel_encoder *encoder, > > >> > struct drm_connector_state > > >> > *conn_state) { > > >> > struct drm_i915_private *dev_priv = > > >> > to_i915(encoder->base.dev); > > >> > + struct intel_crtc *crtc = > > >> > + to_intel_crtc(pipe_config->uapi.crtc); > > >> > struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder); > > >> > struct intel_dp *intel_dp = &intel_mst->primary->dp; > > >> > const struct intel_connector *connector = @@ -540,6 +541,10 @@ > > >> > static int intel_dp_mst_compute_config(struct intel_encoder *encoder, > > >> > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN) > > >> > return -EINVAL; > > >> > > > >> > + if (intel_dp_need_bigjoiner(intel_dp, adjusted_mode->crtc_hdisplay, > > >> > + adjusted_mode->crtc_clock)) > > >> > + pipe_config->bigjoiner_pipes = GENMASK(crtc->pipe + 1, > > >> > + crtc->pipe); > > >> > + > > >> > pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB; > > >> > pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB; > > >> > pipe_config->has_pch_encoder = false; @@ -1318,12 +1323,6 @@ > > >> > intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, > > >> > * corresponding link capabilities of the sink) in case the > > >> > * stream is uncompressed for it by the last branch device. > > >> > */ > > >> > - if (mode_rate > max_rate || mode->clock > max_dotclk || > > >> > - drm_dp_calc_pbn_mode(mode->clock, min_bpp << 4) > port- > >full_pbn) { > > >> > - *status = MODE_CLOCK_HIGH; > > >> > - return 0; > > >> > - } > > >> > - > > >> > if (mode->clock < 10000) { > > >> > *status = MODE_CLOCK_LOW; > > >> > return 0; > > >> > @@ -1343,6 +1342,12 @@ intel_dp_mst_mode_valid_ctx(struct > drm_connector *connector, > > >> > return 0; > > >> > } > > >> > > > >> > + if (mode_rate > max_rate || mode->clock > max_dotclk || > > >> > + drm_dp_calc_pbn_mode(mode->clock, min_bpp << 4) > port- > >full_pbn) { > > >> > + *status = MODE_CLOCK_HIGH; > > >> > + return 0; > > >> > + } > > >> > + > > >> > if (DISPLAY_VER(dev_priv) >= 10 && > > >> > drm_dp_sink_supports_dsc(intel_connector->dp.dsc_dpcd)) { > > >> > /* > > >> > @@ -1385,7 +1390,7 @@ intel_dp_mst_mode_valid_ctx(struct > drm_connector *connector, > > >> > return 0; > > >> > } > > >> > > > >> > - *status = intel_mode_valid_max_plane_size(dev_priv, mode, > > >> > false); > > >> > + *status = intel_mode_valid_max_plane_size(dev_priv, mode, > > >> > + bigjoiner); > > >> > return 0; > > >> > } > > >> > > >> -- > > >> Jani Nikula, Intel > > > > -- > > Jani Nikula, Intel
