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

Maarten Lankhorst
2019-07-03T08:03:00Z

Op 03-07-2019 om 02:32 schreef Manasi Navare:
> 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

DSC is only enabled when we don't have enough bandwidth. The only time that function
returns early is when it encounters a failure.

> 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