On Fri, Feb 04, 2022 at 04:18:16PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Reintroduce the !join_mbus single pipe cases for adlp+.
>
> Due to the mbus relative dbuf offsets in PLANE_BUF_CFG we
> need to know the actual slices used by the pipe when doing
> readout, even when mbus joining isn't enabled. Accurate
> readout will be needed to properly sanitize invalid BIOS
> dbuf configurations.
>
> This will also make it much easier to play around with the
> !join_mbus configs for testin/workaround purposes
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 66 +++++++++++++++++++++++----------
> 1 file changed, 46 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 859be750fb22..2eb70ec38f6e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4695,6 +4695,10 @@ static const struct dbuf_slice_conf_entry dg2_allowed_dbufs[] = {
> };
>
> static const struct dbuf_slice_conf_entry adlp_allowed_dbufs[] = {
> + /*
> + * Keep the join_mbus cases first so check_mbus_joined()
> + * will prefer them over the !join_mbus cases.
> + */
> {
> .active_pipes = BIT(PIPE_A),
> .dbuf_mask = {
> @@ -4709,6 +4713,20 @@ static const struct dbuf_slice_conf_entry adlp_allowed_dbufs[] = {
> },
> .join_mbus = true,
> },
> + {
> + .active_pipes = BIT(PIPE_A),
> + .dbuf_mask = {
> + [PIPE_A] = BIT(DBUF_S1) | BIT(DBUF_S2),
> + },
> + .join_mbus = false,
> + },
> + {
> + .active_pipes = BIT(PIPE_B),
> + .dbuf_mask = {
> + [PIPE_B] = BIT(DBUF_S3) | BIT(DBUF_S4),
> + },
> + .join_mbus = false,
> + },
> {
> .active_pipes = BIT(PIPE_A) | BIT(PIPE_B),
> .dbuf_mask = {
> @@ -4825,13 +4843,14 @@ static bool adlp_check_mbus_joined(u8 active_pipes)
> return check_mbus_joined(active_pipes, adlp_allowed_dbufs);
> }
>
> -static u8 compute_dbuf_slices(enum pipe pipe, u8 active_pipes,
> +static u8 compute_dbuf_slices(enum pipe pipe, u8 active_pipes, bool join_mbus,
> const struct dbuf_slice_conf_entry *dbuf_slices)
> {
> int i;
>
> for (i = 0; i < dbuf_slices[i].active_pipes; i++) {
> - if (dbuf_slices[i].active_pipes == active_pipes)
> + if (dbuf_slices[i].active_pipes == active_pipes &&
> + dbuf_slices[i].join_mbus == join_mbus)
We had similar blocker issue in May I remember for ADL, I remember I've sent
a fix, which was however just about checking also join_mbus state besides, the active pipes.
Nice to see we are finally handling this now even more properly
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> return dbuf_slices[i].dbuf_mask[pipe];
> }
> return 0;
> @@ -4842,7 +4861,7 @@ static u8 compute_dbuf_slices(enum pipe pipe, u8 active_pipes,
> * returns correspondent DBuf slice mask as stated in BSpec for particular
> * platform.
> */
> -static u8 icl_compute_dbuf_slices(enum pipe pipe, u8 active_pipes)
> +static u8 icl_compute_dbuf_slices(enum pipe pipe, u8 active_pipes, bool join_mbus)
> {
> /*
> * FIXME: For ICL this is still a bit unclear as prev BSpec revision
> @@ -4856,37 +4875,41 @@ static u8 icl_compute_dbuf_slices(enum pipe pipe, u8 active_pipes)
> * still here - we will need it once those additional constraints
> * pop up.
> */
> - return compute_dbuf_slices(pipe, active_pipes, icl_allowed_dbufs);
> + return compute_dbuf_slices(pipe, active_pipes, join_mbus,
> + icl_allowed_dbufs);
> }
>
> -static u8 tgl_compute_dbuf_slices(enum pipe pipe, u8 active_pipes)
> +static u8 tgl_compute_dbuf_slices(enum pipe pipe, u8 active_pipes, bool join_mbus)
> {
> - return compute_dbuf_slices(pipe, active_pipes, tgl_allowed_dbufs);
> + return compute_dbuf_slices(pipe, active_pipes, join_mbus,
> + tgl_allowed_dbufs);
> }
>
> -static u32 adlp_compute_dbuf_slices(enum pipe pipe, u32 active_pipes)
> +static u8 adlp_compute_dbuf_slices(enum pipe pipe, u8 active_pipes, bool join_mbus)
> {
> - return compute_dbuf_slices(pipe, active_pipes, adlp_allowed_dbufs);
> + return compute_dbuf_slices(pipe, active_pipes, join_mbus,
> + adlp_allowed_dbufs);
> }
>
> -static u32 dg2_compute_dbuf_slices(enum pipe pipe, u32 active_pipes)
> +static u8 dg2_compute_dbuf_slices(enum pipe pipe, u8 active_pipes, bool join_mbus)
> {
> - return compute_dbuf_slices(pipe, active_pipes, dg2_allowed_dbufs);
> + return compute_dbuf_slices(pipe, active_pipes, join_mbus,
> + dg2_allowed_dbufs);
> }
>
> -static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes)
> +static u8 skl_compute_dbuf_slices(struct intel_crtc *crtc, u8 active_pipes, bool join_mbus)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum pipe pipe = crtc->pipe;
>
> if (IS_DG2(dev_priv))
> - return dg2_compute_dbuf_slices(pipe, active_pipes);
> + return dg2_compute_dbuf_slices(pipe, active_pipes, join_mbus);
> else if (IS_ALDERLAKE_P(dev_priv))
> - return adlp_compute_dbuf_slices(pipe, active_pipes);
> + return adlp_compute_dbuf_slices(pipe, active_pipes, join_mbus);
> else if (DISPLAY_VER(dev_priv) == 12)
> - return tgl_compute_dbuf_slices(pipe, active_pipes);
> + return tgl_compute_dbuf_slices(pipe, active_pipes, join_mbus);
> else if (DISPLAY_VER(dev_priv) == 11)
> - return icl_compute_dbuf_slices(pipe, active_pipes);
> + return icl_compute_dbuf_slices(pipe, active_pipes, join_mbus);
> /*
> * For anything else just return one slice yet.
> * Should be extended for other platforms.
> @@ -6139,11 +6162,16 @@ skl_compute_ddb(struct intel_atomic_state *state)
> return ret;
> }
>
> + if (IS_ALDERLAKE_P(dev_priv))
> + new_dbuf_state->joined_mbus =
> + adlp_check_mbus_joined(new_dbuf_state->active_pipes);
> +
> for_each_intel_crtc(&dev_priv->drm, crtc) {
> enum pipe pipe = crtc->pipe;
>
> new_dbuf_state->slices[pipe] =
> - skl_compute_dbuf_slices(crtc, new_dbuf_state->active_pipes);
> + skl_compute_dbuf_slices(crtc, new_dbuf_state->active_pipes,
> + new_dbuf_state->joined_mbus);
>
> if (old_dbuf_state->slices[pipe] == new_dbuf_state->slices[pipe])
> continue;
> @@ -6155,9 +6183,6 @@ skl_compute_ddb(struct intel_atomic_state *state)
>
> new_dbuf_state->enabled_slices = intel_dbuf_enabled_slices(new_dbuf_state);
>
> - if (IS_ALDERLAKE_P(dev_priv))
> - new_dbuf_state->joined_mbus = adlp_check_mbus_joined(new_dbuf_state->active_pipes);
> -
> if (old_dbuf_state->enabled_slices != new_dbuf_state->enabled_slices ||
> old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> ret = intel_atomic_serialize_global_state(&new_dbuf_state->base);
> @@ -6658,7 +6683,8 @@ void skl_wm_get_hw_state(struct drm_i915_private *dev_priv)
> }
>
> dbuf_state->slices[pipe] =
> - skl_compute_dbuf_slices(crtc, dbuf_state->active_pipes);
> + skl_compute_dbuf_slices(crtc, dbuf_state->active_pipes,
> + dbuf_state->joined_mbus);
>
> dbuf_state->weight[pipe] = intel_crtc_ddb_weight(crtc_state);
>
> --
> 2.34.1
>
