On Wed, Aug 23, 2023 at 10:07:33AM -0700, Lucas De Marchi wrote:
> From: Stanislav Lisovskiy <[email protected]>
>
> When we change MDCLK/CDCLK the BSpec now instructs us to write a ratio
> between MDCLK/CDCLK to MBUS CTL and DBUF CTL registers during that
> change.
>
> Previsouly DBuf state and CDCLK were not anyhow coupled together. Now
> at compute stage when we know which CDCLK/MDCLK we are going to use, we
> need to update the DBuf state with that ratio, being properly encoded,
> so that it gets written to those registers, once DBuf state is being
> update. The criteria for updating DBuf state is also a CDCLK/MDCLK ratio
> change now.
>
> Cc: Mika Kahola <[email protected]>
> Signed-off-by: Stanislav Lisovskiy <[email protected]>
> Signed-off-by: Lucas De Marchi <[email protected]>
Bspec: 68864
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 16 +++++++++
> drivers/gpu/drm/i915/display/skl_watermark.c | 35 ++++++++++++++++---
> drivers/gpu/drm/i915/display/skl_watermark.h | 1 +
> .../gpu/drm/i915/display/skl_watermark_regs.h | 2 ++
> 4 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 04937aaabcee..aa1000db3cb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -37,6 +37,7 @@
> #include "intel_pci_config.h"
> #include "intel_pcode.h"
> #include "intel_psr.h"
> +#include "skl_watermark.h"
> #include "vlv_sideband.h"
>
> /**
> @@ -1827,6 +1828,15 @@ static bool cdclk_pll_is_unknown(unsigned int vco)
> return vco == ~0;
> }
>
> +static int get_mdclk_cdclk_ratio(struct drm_i915_private *i915,
> + const struct intel_cdclk_config *cdclk_config)
> +{
> + if (DISPLAY_VER(i915) >= 20)
> + return cdclk_config->mdclk / cdclk_config->cdclk - 1;
Should this be DIV_ROUND_UP? Bspec 69482 and 69445 both say "If mdclk/cdclk is
a
non-integer value, round up the result."
You might want a comment on this function noting that it returns the
register encoding of ratio (i.e., "- 1") rather than the ratio itself.
> + else
> + return 1;
> +}
> +
> static int cdclk_squash_divider(u16 waveform)
> {
> return hweight16(waveform ?: 0xffff);
> @@ -2727,6 +2737,7 @@ static int intel_compute_min_cdclk(struct
> intel_cdclk_state *cdclk_state)
> struct intel_crtc_state *crtc_state;
> int min_cdclk, i;
> enum pipe pipe;
> + struct intel_dbuf_state *dbuf_state;
>
> for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
> int ret;
> @@ -2760,6 +2771,11 @@ static int intel_compute_min_cdclk(struct
> intel_cdclk_state *cdclk_state)
> }
> }
>
> + dbuf_state = intel_atomic_get_new_dbuf_state(state);
> + if (dbuf_state)
> + dbuf_state->mdclk_cdclk_ratio =
> + get_mdclk_cdclk_ratio(dev_priv, &cdclk_state->actual);
> +
> min_cdclk = max(cdclk_state->force_min_cdclk,
> cdclk_state->bw_min_cdclk);
> for_each_pipe(dev_priv, pipe)
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 64a122d3c9c0..79454b4d99e3 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3472,6 +3472,23 @@ int intel_dbuf_init(struct drm_i915_private *i915)
> return 0;
> }
>
> +static int get_mbus_mdclk_cdclk_ratio(struct drm_i915_private *i915,
> + int mdclk_cdclk_ratio,
> + int mbus_joined)
> +{
> + if (DISPLAY_VER(i915) >= 20) {
We can drop this condition. Since mdclk_cdclk_ratio is already "1" for
pre-20 platforms, the calculations here still come out right (returning
either 3 or 1 depending on joining) there too.
Matt
> + if (mbus_joined)
> + return (mdclk_cdclk_ratio << 1) + 1;
> + else
> + return mdclk_cdclk_ratio;
> + }
> +
> + if (mbus_joined)
> + return 3;
> +
> + return 1;
> +}
> +
> /*
> * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state
> before
> * update the request state of all DBUS slices.
> @@ -3483,10 +3500,16 @@ static void update_mbus_pre_enable(struct
> intel_atomic_state *state)
> enum dbuf_slice slice;
> const struct intel_dbuf_state *dbuf_state =
> intel_atomic_get_new_dbuf_state(state);
> + int tracker_state_service;
>
> if (!HAS_MBUS_JOINING(i915))
> return;
>
> + tracker_state_service =
> + get_mbus_mdclk_cdclk_ratio(i915,
> + dbuf_state->mdclk_cdclk_ratio,
> + dbuf_state->joined_mbus);
> +
> /*
> * TODO: Implement vblank synchronized MBUS joining changes.
> * Must be properly coordinated with dbuf reprogramming.
> @@ -3494,13 +3517,15 @@ static void update_mbus_pre_enable(struct
> intel_atomic_state *state)
> if (dbuf_state->joined_mbus) {
> mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
> MBUS_JOIN_PIPE_SELECT_NONE;
> - dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(3);
> } else {
> mbus_ctl = MBUS_HASHING_MODE_2x2 |
> MBUS_JOIN_PIPE_SELECT_NONE;
> - dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(1);
> }
>
> + dbuf_min_tracker_val =
> DBUF_MIN_TRACKER_STATE_SERVICE(tracker_state_service);
> +
> + mbus_ctl |=
> MBUS_TRANS_THROTTLE_MIN_SELECT(dbuf_state->mdclk_cdclk_ratio);
> +
> intel_de_rmw(i915, MBUS_CTL,
> MBUS_HASHING_MODE_MASK | MBUS_JOIN |
> MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
> @@ -3521,7 +3546,8 @@ void intel_dbuf_pre_plane_update(struct
> intel_atomic_state *state)
>
> if (!new_dbuf_state ||
> (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices &&
> - new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus))
> + new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus &&
> + new_dbuf_state->mdclk_cdclk_ratio ==
> old_dbuf_state->mdclk_cdclk_ratio))
> return;
>
> WARN_ON(!new_dbuf_state->base.changed);
> @@ -3542,7 +3568,8 @@ void intel_dbuf_post_plane_update(struct
> intel_atomic_state *state)
>
> if (!new_dbuf_state ||
> (new_dbuf_state->enabled_slices == old_dbuf_state->enabled_slices &&
> - new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus))
> + new_dbuf_state->joined_mbus == old_dbuf_state->joined_mbus &&
> + new_dbuf_state->mdclk_cdclk_ratio ==
> old_dbuf_state->mdclk_cdclk_ratio))
> return;
>
> WARN_ON(!new_dbuf_state->base.changed);
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h
> b/drivers/gpu/drm/i915/display/skl_watermark.h
> index f91a3d4ddc07..54db5c7d517e 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> @@ -56,6 +56,7 @@ struct intel_dbuf_state {
> u8 slices[I915_MAX_PIPES];
> u8 enabled_slices;
> u8 active_pipes;
> + u8 mdclk_cdclk_ratio;
> bool joined_mbus;
> };
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> index 628c5920ad49..4c820f1d351d 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> @@ -38,6 +38,8 @@
> #define MBUS_HASHING_MODE_2x2
> REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
> #define MBUS_HASHING_MODE_1x4
> REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
> #define MBUS_JOIN_PIPE_SELECT_MASK REG_GENMASK(28, 26)
> +#define MBUS_TRANS_THROTTLE_MIN_MASK REG_GENMASK(15, 13)
> +#define MBUS_TRANS_THROTTLE_MIN_SELECT(ratio)
> REG_FIELD_PREP(MBUS_TRANS_THROTTLE_MIN_MASK, ratio)
> #define MBUS_JOIN_PIPE_SELECT(pipe)
> REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
> #define MBUS_JOIN_PIPE_SELECT_NONE MBUS_JOIN_PIPE_SELECT(7)
>
> --
> 2.40.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
