| Message ID | 20240320201152.3487892-7-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dp: Few MTL/DSC and a UHBR monitor fix | expand |
On 3/21/2024 1:41 AM, Imre Deak wrote: > Instead of checking each compressed bpp value against the maximum > DSC/DPT bpp, simplify things by calculating the maximum bpp upfront and > limiting the range of bpps looped over using this maximum. > > While at it add a comment about the origin of the DSC/DPT bpp limit. > > Bspec: 49259, 68912 > > Signed-off-by: Imre Deak <imre.deak@intel.com> LGTM. Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 45 +++++++++++---------- > 1 file changed, 23 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 40660dc5edb45..516b00f791420 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -51,27 +51,24 @@ > #include "intel_vdsc.h" > #include "skl_scaler.h" > > -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp, > - const struct drm_display_mode *adjusted_mode, > - struct intel_crtc_state *crtc_state, > - bool dsc) > +static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state *crtc_state, > + bool dsc) > { > - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { > - int output_bpp = bpp; > - int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock); > - int available_bw = mul_u32_u32(symbol_clock * 72, > - drm_dp_bw_channel_coding_efficiency(true)) / > - 1000000; > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > + const struct drm_display_mode *adjusted_mode = > + &crtc_state->hw.adjusted_mode; > > - if (output_bpp * adjusted_mode->crtc_clock > > - available_bw) { > - drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n", > - output_bpp * adjusted_mode->crtc_clock, available_bw); > - return -EINVAL; > - } > - } > + if (!intel_dp_is_uhbr(crtc_state) || DISPLAY_VER(i915) >= 20 || !dsc) > + return INT_MAX; > > - return 0; > + /* > + * DSC->DPT interface width: > + * ICL-MTL: 72 bits (each branch has 72 bits, only left branch is used) > + * LNL+: 144 bits (not a bottleneck in any config) > + */ > + return div64_u64(mul_u32_u32(intel_dp_link_symbol_clock(crtc_state->port_clock) * 72, > + drm_dp_bw_channel_coding_efficiency(true)), > + mul_u32_u32(adjusted_mode->crtc_clock, 1000000)); > } > > static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, > @@ -160,6 +157,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > const struct drm_display_mode *adjusted_mode = > &crtc_state->hw.adjusted_mode; > int bpp, slots = -EINVAL; > + int max_dpt_bpp; > int ret = 0; > > mst_state = drm_atomic_get_mst_topology_state(state, &intel_dp->mst_mgr); > @@ -180,6 +178,13 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > crtc_state->port_clock, > crtc_state->lane_count); > > + max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); > + if (max_bpp > max_dpt_bpp) { > + drm_dbg_kms(&i915->drm, "Limiting bpp to max DPT bpp (%d -> %d)\n", > + max_bpp, max_dpt_bpp); > + max_bpp = max_dpt_bpp; > + } > + > drm_dbg_kms(&i915->drm, "Looking for slots in range min bpp %d max bpp %d\n", > min_bpp, max_bpp); > > @@ -191,10 +196,6 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp); > > - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc); > - if (ret) > - continue; > - > link_bpp_x16 = to_bpp_x16(dsc ? bpp : > intel_dp_output_bpp(crtc_state->output_format, bpp)); >
Hi Imre, Would this impact/fix DSC functionality on ADL based platforms as well or will this change only impact platforms that support UHBR? Manasi On Tue, Mar 26, 2024 at 5:55 AM Nautiyal, Ankit K <ankit.k.nautiyal@intel.com> wrote: > > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > Instead of checking each compressed bpp value against the maximum > > DSC/DPT bpp, simplify things by calculating the maximum bpp upfront and > > limiting the range of bpps looped over using this maximum. > > > > While at it add a comment about the origin of the DSC/DPT bpp limit. > > > > Bspec: 49259, 68912 > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > LGTM. > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > > --- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 45 +++++++++++---------- > > 1 file changed, 23 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > index 40660dc5edb45..516b00f791420 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > @@ -51,27 +51,24 @@ > > #include "intel_vdsc.h" > > #include "skl_scaler.h" > > > > -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp, > > - const struct drm_display_mode *adjusted_mode, > > - struct intel_crtc_state *crtc_state, > > - bool dsc) > > +static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state *crtc_state, > > + bool dsc) > > { > > - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { > > - int output_bpp = bpp; > > - int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock); > > - int available_bw = mul_u32_u32(symbol_clock * 72, > > - drm_dp_bw_channel_coding_efficiency(true)) / > > - 1000000; > > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > > + const struct drm_display_mode *adjusted_mode = > > + &crtc_state->hw.adjusted_mode; > > > > - if (output_bpp * adjusted_mode->crtc_clock > > > - available_bw) { > > - drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n", > > - output_bpp * adjusted_mode->crtc_clock, available_bw); > > - return -EINVAL; > > - } > > - } > > + if (!intel_dp_is_uhbr(crtc_state) || DISPLAY_VER(i915) >= 20 || !dsc) > > + return INT_MAX; > > > > - return 0; > > + /* > > + * DSC->DPT interface width: > > + * ICL-MTL: 72 bits (each branch has 72 bits, only left branch is used) > > + * LNL+: 144 bits (not a bottleneck in any config) > > + */ > > + return div64_u64(mul_u32_u32(intel_dp_link_symbol_clock(crtc_state->port_clock) * 72, > > + drm_dp_bw_channel_coding_efficiency(true)), > > + mul_u32_u32(adjusted_mode->crtc_clock, 1000000)); > > } > > > > static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, > > @@ -160,6 +157,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > const struct drm_display_mode *adjusted_mode = > > &crtc_state->hw.adjusted_mode; > > int bpp, slots = -EINVAL; > > + int max_dpt_bpp; > > int ret = 0; > > > > mst_state = drm_atomic_get_mst_topology_state(state, &intel_dp->mst_mgr); > > @@ -180,6 +178,13 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > crtc_state->port_clock, > > crtc_state->lane_count); > > > > + max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); > > + if (max_bpp > max_dpt_bpp) { > > + drm_dbg_kms(&i915->drm, "Limiting bpp to max DPT bpp (%d -> %d)\n", > > + max_bpp, max_dpt_bpp); > > + max_bpp = max_dpt_bpp; > > + } > > + > > drm_dbg_kms(&i915->drm, "Looking for slots in range min bpp %d max bpp %d\n", > > min_bpp, max_bpp); > > > > @@ -191,10 +196,6 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > > drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp); > > > > - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc); > > - if (ret) > > - continue; > > - > > link_bpp_x16 = to_bpp_x16(dsc ? bpp : > > intel_dp_output_bpp(crtc_state->output_format, bpp)); > >
On Tue, Mar 26, 2024 at 01:13:38PM -0700, Manasi Navare wrote: > Hi Imre, > > Would this impact/fix DSC functionality on ADL based platforms as well > or will this change only impact platforms that support UHBR? The related DPT limit changes in this patchset make a difference only on platforms supporting UHBR, so DG2 and MTL+. > Manasi > > On Tue, Mar 26, 2024 at 5:55 AM Nautiyal, Ankit K > <ankit.k.nautiyal@intel.com> wrote: > > > > > > On 3/21/2024 1:41 AM, Imre Deak wrote: > > > Instead of checking each compressed bpp value against the maximum > > > DSC/DPT bpp, simplify things by calculating the maximum bpp upfront and > > > limiting the range of bpps looped over using this maximum. > > > > > > While at it add a comment about the origin of the DSC/DPT bpp limit. > > > > > > Bspec: 49259, 68912 > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > LGTM. > > > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 45 +++++++++++---------- > > > 1 file changed, 23 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > index 40660dc5edb45..516b00f791420 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > > > @@ -51,27 +51,24 @@ > > > #include "intel_vdsc.h" > > > #include "skl_scaler.h" > > > > > > -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp, > > > - const struct drm_display_mode *adjusted_mode, > > > - struct intel_crtc_state *crtc_state, > > > - bool dsc) > > > +static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state *crtc_state, > > > + bool dsc) > > > { > > > - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { > > > - int output_bpp = bpp; > > > - int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock); > > > - int available_bw = mul_u32_u32(symbol_clock * 72, > > > - drm_dp_bw_channel_coding_efficiency(true)) / > > > - 1000000; > > > + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > > > + const struct drm_display_mode *adjusted_mode = > > > + &crtc_state->hw.adjusted_mode; > > > > > > - if (output_bpp * adjusted_mode->crtc_clock > > > > - available_bw) { > > > - drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n", > > > - output_bpp * adjusted_mode->crtc_clock, available_bw); > > > - return -EINVAL; > > > - } > > > - } > > > + if (!intel_dp_is_uhbr(crtc_state) || DISPLAY_VER(i915) >= 20 || !dsc) > > > + return INT_MAX; > > > > > > - return 0; > > > + /* > > > + * DSC->DPT interface width: > > > + * ICL-MTL: 72 bits (each branch has 72 bits, only left branch is used) > > > + * LNL+: 144 bits (not a bottleneck in any config) > > > + */ > > > + return div64_u64(mul_u32_u32(intel_dp_link_symbol_clock(crtc_state->port_clock) * 72, > > > + drm_dp_bw_channel_coding_efficiency(true)), > > > + mul_u32_u32(adjusted_mode->crtc_clock, 1000000)); > > > } > > > > > > static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, > > > @@ -160,6 +157,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > const struct drm_display_mode *adjusted_mode = > > > &crtc_state->hw.adjusted_mode; > > > int bpp, slots = -EINVAL; > > > + int max_dpt_bpp; > > > int ret = 0; > > > > > > mst_state = drm_atomic_get_mst_topology_state(state, &intel_dp->mst_mgr); > > > @@ -180,6 +178,13 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > crtc_state->port_clock, > > > crtc_state->lane_count); > > > > > > + max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); > > > + if (max_bpp > max_dpt_bpp) { > > > + drm_dbg_kms(&i915->drm, "Limiting bpp to max DPT bpp (%d -> %d)\n", > > > + max_bpp, max_dpt_bpp); > > > + max_bpp = max_dpt_bpp; > > > + } > > > + > > > drm_dbg_kms(&i915->drm, "Looking for slots in range min bpp %d max bpp %d\n", > > > min_bpp, max_bpp); > > > > > > @@ -191,10 +196,6 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, > > > > > > drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp); > > > > > > - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc); > > > - if (ret) > > > - continue; > > > - > > > link_bpp_x16 = to_bpp_x16(dsc ? bpp : > > > intel_dp_output_bpp(crtc_state->output_format, bpp)); > > >
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 40660dc5edb45..516b00f791420 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -51,27 +51,24 @@ #include "intel_vdsc.h" #include "skl_scaler.h" -static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp, - const struct drm_display_mode *adjusted_mode, - struct intel_crtc_state *crtc_state, - bool dsc) +static int intel_dp_mst_max_dpt_bpp(const struct intel_crtc_state *crtc_state, + bool dsc) { - if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 20 && dsc) { - int output_bpp = bpp; - int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock); - int available_bw = mul_u32_u32(symbol_clock * 72, - drm_dp_bw_channel_coding_efficiency(true)) / - 1000000; + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); + const struct drm_display_mode *adjusted_mode = + &crtc_state->hw.adjusted_mode; - if (output_bpp * adjusted_mode->crtc_clock > - available_bw) { - drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n", - output_bpp * adjusted_mode->crtc_clock, available_bw); - return -EINVAL; - } - } + if (!intel_dp_is_uhbr(crtc_state) || DISPLAY_VER(i915) >= 20 || !dsc) + return INT_MAX; - return 0; + /* + * DSC->DPT interface width: + * ICL-MTL: 72 bits (each branch has 72 bits, only left branch is used) + * LNL+: 144 bits (not a bottleneck in any config) + */ + return div64_u64(mul_u32_u32(intel_dp_link_symbol_clock(crtc_state->port_clock) * 72, + drm_dp_bw_channel_coding_efficiency(true)), + mul_u32_u32(adjusted_mode->crtc_clock, 1000000)); } static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state, @@ -160,6 +157,7 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; int bpp, slots = -EINVAL; + int max_dpt_bpp; int ret = 0; mst_state = drm_atomic_get_mst_topology_state(state, &intel_dp->mst_mgr); @@ -180,6 +178,13 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, crtc_state->port_clock, crtc_state->lane_count); + max_dpt_bpp = intel_dp_mst_max_dpt_bpp(crtc_state, dsc); + if (max_bpp > max_dpt_bpp) { + drm_dbg_kms(&i915->drm, "Limiting bpp to max DPT bpp (%d -> %d)\n", + max_bpp, max_dpt_bpp); + max_bpp = max_dpt_bpp; + } + drm_dbg_kms(&i915->drm, "Looking for slots in range min bpp %d max bpp %d\n", min_bpp, max_bpp); @@ -191,10 +196,6 @@ static int intel_dp_mst_find_vcpi_slots_for_bpp(struct intel_encoder *encoder, drm_dbg_kms(&i915->drm, "Trying bpp %d\n", bpp); - ret = intel_dp_mst_check_constraints(i915, bpp, adjusted_mode, crtc_state, dsc); - if (ret) - continue; - link_bpp_x16 = to_bpp_x16(dsc ? bpp : intel_dp_output_bpp(crtc_state->output_format, bpp));
Instead of checking each compressed bpp value against the maximum DSC/DPT bpp, simplify things by calculating the maximum bpp upfront and limiting the range of bpps looped over using this maximum. While at it add a comment about the origin of the DSC/DPT bpp limit. Bspec: 49259, 68912 Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 45 +++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-)