Re: [RFC 6/7] drm/i915: Try to make bigjoiner work in atomic check.

Manasi Navare
2019-07-03T00:30:47Z

On Tue, Jul 02, 2019 at 09:42:04PM +0200, Maarten Lankhorst wrote:
> When the clock is higher than the dotclock, try with 2 pipes enabled.
> If we can enable 2, then we will go into big joiner mode, and steal
> the adjacent crtc.
>
> This only links the planes in software, no hardware programming is
> done yet.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 145 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_dp.c | 22 ++-
> drivers/gpu/drm/i915/intel_drv.h | 6 +
> 3 files changed, 168 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index d8e63f133a62..ca72058202f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12203,6 +12203,47 @@ static void copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
> crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> }
>
> +static int
> +copy_bigjoiner_crtc_state(struct intel_crtc_state *crtc_state,
> + const struct intel_crtc_state *from_crtc_state)
> +{
> + struct intel_crtc_state *saved_state;
> +
> + saved_state = kmemdup(from_crtc_state, sizeof(*saved_state), GFP_KERNEL);
> + if (!saved_state)
> + return -ENOMEM;
> +
> + saved_state->uapi = crtc_state->uapi;
> + saved_state->scaler_state = crtc_state->scaler_state;
> + saved_state->shared_dpll = crtc_state->shared_dpll;
> + saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> + saved_state->crc_enabled = crtc_state->crc_enabled;
> +
> + intel_crtc_free_hw_state(crtc_state);
> + memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> + kfree(saved_state);
> +
> + /* Re-init hw state */
> + memset(&crtc_state->hw, 0, sizeof(saved_state->hw));
> + crtc_state->hw.enable = from_crtc_state->hw.enable;
> + crtc_state->hw.active = from_crtc_state->hw.active;
> + crtc_state->hw.mode = from_crtc_state->hw.mode;
> + crtc_state->hw.adjusted_mode = from_crtc_state->hw.adjusted_mode;
> +
> + /* Some fixups */
> + crtc_state->uapi.mode_changed = from_crtc_state->uapi.mode_changed;
> + crtc_state->uapi.connectors_changed = from_crtc_state->uapi.connectors_changed;
> + crtc_state->uapi.active_changed = from_crtc_state->uapi.active_changed;
> + crtc_state->nv12_planes = crtc_state->c8_planes = crtc_state->update_planes = 0;
> +
> + crtc_state->bigjoiner_master_crtc = to_intel_crtc(from_crtc_state->uapi.crtc);
> +
> + /* XXX/TODO: Do we need the master's cpu_transcoder here, or reset to default? */
> + crtc_state->cpu_transcoder = (enum transcoder)to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> +
> + return 0;
> +}
> +
> static int
> clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> {
> @@ -13577,6 +13618,96 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta
> new_crtc_state->has_drrs = old_crtc_state->has_drrs;
> }
>
> +static int intel_atomic_check_bigjoiner(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_crtc_state *old_crtc_state, *new_crtc_state, *slave_crtc_state, *master_crtc_state;
> + struct intel_crtc *crtc, *slave, *master;
> + int i, ret = 0;
> +
> + if (INTEL_GEN(dev_priv) < 11)
> + return 0;
> +
> + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> + new_crtc_state, i) {
> + if (!old_crtc_state->bigjoiner_master_crtc)
> + continue;
> +
> + if (crtc->pipe == PIPE_A) {
> + DRM_ERROR("Bigjoiner slave on pipe A?\n");
> + return -EINVAL;
> + }
> +
> + /* crtc staying in slave mode? */
> + if (!new_crtc_state->uapi.enable)
> + continue;
> +
> + if (needs_modeset(new_crtc_state) || new_crtc_state->update_pipe) {
> + master = intel_get_crtc_for_pipe(dev_priv, crtc->pipe - 1);
> + master_crtc_state = intel_atomic_get_crtc_state(&state->base, master);
> + if (IS_ERR(master_crtc_state))
> + return PTR_ERR(master_crtc_state);
> +
> + /*
> + * Force modeset on master, to recalculate bigjoiner
> + * state.
> + *
> + * If master_crtc_state was not part of the atomic commit,
> + * we will fail because the master was not deconfigured,
> + * but at least fail below to unify the checks.
> + */
> + master_crtc_state->uapi.mode_changed = true;
> +
> + ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
> + if (ret)
> + return ret;
> +
> + ret = drm_atomic_add_affected_connectors(&state->base, &crtc->base);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> + new_crtc_state, i) {
> + if (!new_crtc_state->uapi.enable || !new_crtc_state->bigjoiner) {
> + if (!old_crtc_state->bigjoiner)
> + continue;
> + }
> +
> + if (!needs_modeset(new_crtc_state) && !new_crtc_state->update_pipe)
> + continue;
> +
> + if (1 + crtc->pipe >= INTEL_INFO(dev_priv)->num_pipes) {
> + DRM_DEBUG_KMS("Big joiner configuration requires CRTC + 1 to be used, doesn't exist\n");
> + return -EINVAL;
> + }
> +
> + slave = intel_get_crtc_for_pipe(dev_priv, crtc->pipe + 1);
> + slave_crtc_state = intel_atomic_get_crtc_state(&state->base, slave);
> + if (IS_ERR(slave_crtc_state))
> + return PTR_ERR(slave_crtc_state);
> +
> + if (new_crtc_state->bigjoiner && slave_crtc_state->uapi.enable) {
> + DRM_DEBUG_KMS("[CRTC:%d:%s] Big joiner configuration requires this CRTC to be unconfigured\n",
> + slave->base.base.id, slave->base.name);
> + return -EINVAL;
> + } else if (new_crtc_state->bigjoiner) {
> + DRM_DEBUG_KMS("[CRTC:%d:%s] Used as slave for big joiner\n",
> + slave->base.base.id, slave->base.name);
> + ret = copy_bigjoiner_crtc_state(slave_crtc_state, new_crtc_state);
> + } else if (!slave_crtc_state->uapi.enable) {
> + DRM_DEBUG_KMS("[CRTC:%d:%s] Disabling slave from big joiner\n",
> + slave->base.base.id, slave->base.name);
> + ret = clear_intel_crtc_state(slave_crtc_state);
> + }
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /**
> * intel_atomic_check - validate state object
> * @dev: drm device
> @@ -13611,7 +13742,10 @@ static int intel_atomic_check(struct drm_device *dev,
>
> if (!new_crtc_state->uapi.enable) {
> any_ms = true;
> - clear_intel_crtc_state(new_crtc_state);
> +
> + /* big joiner is slave cleared in intel_atomic_check_bigjoiner() */
> + if (old_crtc_state->uapi.enable || !old_crtc_state->bigjoiner)
> + clear_intel_crtc_state(new_crtc_state);
> continue;
> }
>
> @@ -13625,6 +13759,10 @@ static int intel_atomic_check(struct drm_device *dev,
> any_ms = true;
> }
>
> + ret = intel_atomic_check_bigjoiner(state);
> + if (ret)
> + return ret;
> +
> ret = drm_dp_mst_atomic_check(&state->base);
> if (ret)
> goto fail;
> @@ -13729,6 +13867,11 @@ static void intel_update_crtc(struct intel_crtc *crtc,
> else if (new_plane_state)
> intel_fbc_enable(crtc, new_crtc_state, new_plane_state);
>
> + if (new_crtc_state->bigjoiner) {
> + DRM_ERROR("Plane updates not supported in bigjoiner configuration yet\n");
> + return;
> + }
> +
> intel_begin_crtc_commit(state, crtc);
>
> if (INTEL_GEN(dev_priv) >= 9)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index bf28970c01aa..5063c8ee6fd9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2046,6 +2046,15 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> pipe_config->port_clock = intel_dp->common_rates[limits->max_clock];
> pipe_config->lane_count = limits->max_lane_count;
>
> + if (adjusted_mode->crtc_clock > intel_dp_downstream_max_dotclock(intel_dp, false)) {
> + if (adjusted_mode->crtc_clock > intel_dp_downstream_max_dotclock(intel_dp, true)) {
> + DRM_DEBUG_KMS("Clock rate too high for big joiner\n");
> + return -EINVAL;
> + }
> + pipe_config->bigjoiner = true;
> + DRM_DEBUG_KMS("Using bigjoiner configuration\n");
> + }

If we later cannot compute DSC parameters and dsc->compression_en cannot be set then we need to
set pipe_config->big_joiner to false as well as we cannot enable big joiner without DSC

Manasi

> +
> if (intel_dp_is_edp(intel_dp)) {
> pipe_config->dsc_params.compressed_bpp =
> min_t(u16, drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
> @@ -2053,6 +2062,11 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> pipe_config->dsc_params.slice_count =
> drm_dp_dsc_sink_max_slice_count(intel_dp->dsc_dpcd,
> true);
> +
> + if (pipe_config->bigjoiner && pipe_config->dsc_params.slice_count < 4) {
> + DRM_DEBUG_KMS("Cannot split eDP stream in bigjoiner configuration.\n");
> + return -EINVAL;
> + }
> } else {
> u16 dsc_max_output_bpp;
> u8 dsc_dp_slice_count;
> @@ -2080,13 +2094,13 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> * is greater than the maximum Cdclock and if slice count is even
> * then we need to use 2 VDSC instances.
> */
> - if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq) {
> - if (pipe_config->dsc_params.slice_count > 1) {
> - pipe_config->dsc_params.dsc_split = true;
> - } else {
> + if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq || pipe_config->bigjoiner) {
> + if (pipe_config->dsc_params.slice_count < 2) {
> DRM_DEBUG_KMS("Cannot split stream to use 2 VDSC instances\n");
> return -EINVAL;
> }
> +
> + pipe_config->dsc_params.dsc_split = true;
> }
>
> ret = intel_dp_compute_dsc_params(intel_dp, pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8a9f089843bc..d6eb7788ed41 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -976,6 +976,12 @@ struct intel_crtc_state {
> /* enable pipe csc? */
> bool csc_enable;
>
> + /* enable pipe big joiner? */
> + bool bigjoiner;
> +
> + /* big joiner master CRTC */
> + struct intel_crtc *bigjoiner_master_crtc;
> +
> /* Display Stream compression state */
> struct {
> bool compression_enable;
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx