On Wed, Mar 09, 2022 at 06:49:47PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Use separate bitmasks for QGV vs. PSF GV points during > the computation. Makes the whole thing a lot less confusing. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_bw.c | 35 ++++++++++++------------- > drivers/gpu/drm/i915/i915_reg.h | 3 ++- > 2 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c > index adf58c58513b..b794545ff81d 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -820,7 +820,7 @@ static u16 icl_qgv_points_mask(struct drm_i915_private *i915) > { > unsigned int num_psf_gv_points = i915->max_bw[0].num_psf_gv_points; > unsigned int num_qgv_points = i915->max_bw[0].num_qgv_points; > - u16 mask = 0; > + u16 qgv_points = 0, psf_points = 0; > > /* > * We can _not_ use the whole ADLS_QGV_PT_MASK here, as PCode rejects > @@ -828,12 +828,12 @@ static u16 icl_qgv_points_mask(struct drm_i915_private *i915) > * So need to operate only with those returned from PCode. > */ > if (num_qgv_points > 0) > - mask |= REG_GENMASK(num_qgv_points - 1, 0); > + qgv_points = GENMASK(num_qgv_points - 1, 0); > > if (num_psf_gv_points > 0) > - mask |= REG_GENMASK(num_psf_gv_points - 1, 0) << ADLS_PSF_PT_SHIFT; > + psf_points = GENMASK(num_psf_gv_points - 1, 0); > > - return mask; > + return ADLS_QGV_PT(qgv_points) | ADLS_PSF_PT(psf_points); > } > > static int intel_bw_check_data_rate(struct intel_atomic_state *state, bool *changed) > @@ -890,7 +890,7 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) > unsigned int data_rate; > unsigned int num_active_planes; > int i, ret; > - u32 allowed_points = 0; > + u16 qgv_points = 0, psf_points = 0; > unsigned int max_bw_point = 0, max_bw = 0; > unsigned int num_qgv_points = dev_priv->max_bw[0].num_qgv_points; > unsigned int num_psf_gv_points = dev_priv->max_bw[0].num_psf_gv_points; > @@ -948,7 +948,7 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) > max_bw = max_data_rate; > } > if (max_data_rate >= data_rate) > - allowed_points |= REG_FIELD_PREP(ADLS_QGV_PT_MASK, BIT(i)); > + qgv_points |= BIT(i); > > drm_dbg_kms(&dev_priv->drm, "QGV point %d: max bw %d required %d\n", > i, max_data_rate, data_rate); > @@ -958,7 +958,7 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) > unsigned int max_data_rate = adl_psf_bw(dev_priv, i); > > if (max_data_rate >= data_rate) > - allowed_points |= REG_FIELD_PREP(ADLS_PSF_PT_MASK, BIT(i)); > + psf_points |= BIT(i); > > drm_dbg_kms(&dev_priv->drm, "PSF GV point %d: max bw %d" > " required %d\n", > @@ -970,20 +970,18 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) > * left, so if we couldn't - simply reject the configuration for obvious > * reasons. > */ > - if ((allowed_points & ADLS_QGV_PT_MASK) == 0) { > + if (qgv_points == 0) { > drm_dbg_kms(&dev_priv->drm, "No QGV points provide sufficient memory" > " bandwidth %d for display configuration(%d active planes).\n", > data_rate, num_active_planes); > return -EINVAL; > } > > - if (num_psf_gv_points > 0) { > - if ((allowed_points & ADLS_PSF_PT_MASK) == 0) { > - drm_dbg_kms(&dev_priv->drm, "No PSF GV points provide sufficient memory" > - " bandwidth %d for display configuration(%d active planes).\n", > - data_rate, num_active_planes); > - return -EINVAL; > - } > + if (num_psf_gv_points > 0 && psf_points == 0) { > + drm_dbg_kms(&dev_priv->drm, "No PSF GV points provide sufficient memory" > + " bandwidth %d for display configuration(%d active planes).\n", > + data_rate, num_active_planes); > + return -EINVAL; > } > > /* > @@ -992,16 +990,17 @@ int intel_bw_atomic_check(struct intel_atomic_state *state) > * cause. > */ > if (!intel_can_enable_sagv(dev_priv, new_bw_state)) { > - allowed_points &= ADLS_PSF_PT_MASK; > - allowed_points |= BIT(max_bw_point); > + qgv_points = BIT(max_bw_point); > drm_dbg_kms(&dev_priv->drm, "No SAGV, using single QGV point %d\n", > max_bw_point); > } > + > /* > * We store the ones which need to be masked as that is what PCode > * actually accepts as a parameter. > */ > - new_bw_state->qgv_points_mask = ~allowed_points & > + new_bw_state->qgv_points_mask = > + ~(ADLS_QGV_PT(qgv_points) | ADLS_PSF_PT(psf_points)) & > icl_qgv_points_mask(dev_priv); > > /* > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 70484f6f2b8b..48a12f6c19b4 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6722,9 +6722,10 @@ > #define ICL_PCODE_SAGV_DE_MEM_SS_CONFIG 0xe > #define ICL_PCODE_POINTS_RESTRICTED 0x0 > #define ICL_PCODE_POINTS_RESTRICTED_MASK 0xf > -#define ADLS_PSF_PT_SHIFT 8 > #define ADLS_QGV_PT_MASK REG_GENMASK(7, 0) > +#define ADLS_QGV_PT(x) REG_FIELD_PREP(ADLS_QGV_PT_MASK, (x)) > #define ADLS_PSF_PT_MASK REG_GENMASK(10, 8) > +#define ADLS_PSF_PT(x) REG_FIELD_PREP(ADLS_PSF_PT_MASK, (x)) > #define GEN6_PCODE_READ_D_COMP 0x10 > #define GEN6_PCODE_WRITE_D_COMP 0x11 > #define ICL_PCODE_EXIT_TCCOLD 0x12 > -- > 2.34.1 >