From patchwork Wed Oct 30 01:24:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lucas De Marchi X-Patchwork-Id: 11218869 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A82E613BD for ; Wed, 30 Oct 2019 01:25:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8F5F320866 for ; Wed, 30 Oct 2019 01:25:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8F5F320866 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0249F6EC6D; Wed, 30 Oct 2019 01:25:12 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5CC486E868 for ; Wed, 30 Oct 2019 01:25:12 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Oct 2019 18:25:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,245,1569308400"; d="scan'208";a="211945566" Received: from ldmartin-desk1.jf.intel.com ([10.7.200.70]) by orsmga002.jf.intel.com with ESMTP; 29 Oct 2019 18:25:10 -0700 From: Lucas De Marchi To: intel-gfx@lists.freedesktop.org Date: Tue, 29 Oct 2019 18:24:48 -0700 Message-Id: <20191030012448.14937-6-lucas.demarchi@intel.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191030012448.14937-1-lucas.demarchi@intel.com> References: <20191030012448.14937-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 5/5] drm/i915/tgl: Select master transcoder in DP MST X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" From: José Roberto de Souza On TGL the blending of all the streams have moved from DDI to transcoder, so now every transcoder working over the same MST port must send its stream to a master transcoder and master will send to DDI respecting the time slots. So here it is picking the lowest pipe/transcoder as it will be enabled first and disabled last. BSpec: 50493 BSpec: 49190 v2: Missed set mst_master_trans to TRANSCODER_INVALID when computing HSW encoder config. HSW CRT hw state readout calls hsw_crt_get_config()->intel_ddi_get_config() that will set mst_master_trans to TRANSCODER_INVALID causing the mismatch when verifying CRTC state after a modeset. (José) v3: Add WARN_ON() requested by Jani. Add FIXME. From Jani: double check PIPE_CONF_CHECK_I(mst_master_trans) - it's now checking for all platforms and MST and non-MST alike. Perhaps in general I'd like the approach of only doing the readout when it's relevant, and only checking the value when it's relevant. v4 (Lucas): Revamp previous implementation: - Make sure we only compute master once; the compute function will be called for each crtc, so we don't need to recompute during check. Now the check simply compares the master from before and after. - Don't loop on all crtcs and connectors, we can just loop over the connectors and find the lowest crtc - Move the setting of INVALID_TRANSCODER around to make it similar to the places where we got it added for master transport sync. - Move all added code to use intel types - Use REG_FIELD_GET() Cc: Ville Syrjälä Cc: Manasi Navare Cc: Rodrigo Vivi Signed-off-by: José Roberto de Souza Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/i915/display/intel_ddi.c | 14 +- drivers/gpu/drm/i915/display/intel_display.c | 16 ++ .../drm/i915/display/intel_display_types.h | 3 + drivers/gpu/drm/i915/display/intel_dp_mst.c | 143 ++++++++++++++++++ drivers/gpu/drm/i915/display/intel_dp_mst.h | 2 + 5 files changed, 176 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 41b9b9a6772a..2c0da46ac8e2 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -1906,8 +1906,13 @@ intel_ddi_transcoder_func_reg_val_get(const struct intel_crtc_state *crtc_state) temp |= TRANS_DDI_MODE_SELECT_DP_MST; temp |= DDI_PORT_WIDTH(crtc_state->lane_count); - if (INTEL_GEN(dev_priv) >= 12) - temp |= TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder); + if (INTEL_GEN(dev_priv) >= 12) { + enum transcoder master = + crtc_state->mst_master_transcoder; + + WARN_ON(master == INVALID_TRANSCODER); + temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master); + } } else { temp |= TRANS_DDI_MODE_SELECT_DP_SST; temp |= DDI_PORT_WIDTH(crtc_state->lane_count); @@ -4304,6 +4309,11 @@ void intel_ddi_get_config(struct intel_encoder *encoder, pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST); pipe_config->lane_count = ((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1; + + if (INTEL_GEN(dev_priv) >= 12) + pipe_config->mst_master_transcoder = + REG_FIELD_GET(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, temp); + intel_dp_get_m_n(intel_crtc, pipe_config); break; default: diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e56a75c07043..4dd1ec1300f1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -46,6 +46,7 @@ #include "display/intel_crt.h" #include "display/intel_ddi.h" #include "display/intel_dp.h" +#include "display/intel_dp_mst.h" #include "display/intel_dsi.h" #include "display/intel_dvo.h" #include "display/intel_gmbus.h" @@ -8896,6 +8897,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc, pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; pipe_config->shared_dpll = NULL; pipe_config->master_transcoder = INVALID_TRANSCODER; + pipe_config->mst_master_transcoder = INVALID_TRANSCODER; ret = false; @@ -10087,6 +10089,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe; pipe_config->shared_dpll = NULL; pipe_config->master_transcoder = INVALID_TRANSCODER; + pipe_config->mst_master_transcoder = INVALID_TRANSCODER; ret = false; tmp = I915_READ(PIPECONF(crtc->pipe)); @@ -10602,6 +10605,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, intel_crtc_init_scalers(crtc, pipe_config); pipe_config->master_transcoder = INVALID_TRANSCODER; + pipe_config->mst_master_transcoder = INVALID_TRANSCODER; power_domain = POWER_DOMAIN_PIPE(crtc->pipe); wf = intel_display_power_get_if_enabled(dev_priv, power_domain); @@ -12478,6 +12482,11 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config, pipe_config->csc_mode, pipe_config->gamma_mode, pipe_config->gamma_enable, pipe_config->csc_enable); + if (INTEL_GEN(dev_priv) >= 12 && + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST)) + DRM_DEBUG_KMS("MST master transcoder: %s\n", + transcoder_name(pipe_config->mst_master_transcoder)); + dump_planes: if (!state) return; @@ -12694,6 +12703,8 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config) return ret; } + pipe_config->mst_master_transcoder = INVALID_TRANSCODER; + /* Pass our mode to the connectors and the CRTC to give them a chance to * adjust it according to limitations or connector properties, and also * a chance to reject the mode entirely. @@ -13215,6 +13226,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, PIPE_CONF_CHECK_I(sync_mode_slaves_mask); PIPE_CONF_CHECK_I(master_transcoder); + PIPE_CONF_CHECK_I(mst_master_transcoder); #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I @@ -14003,6 +14015,10 @@ static int intel_atomic_check(struct drm_device *dev, if (ret) goto fail; + ret = intel_dp_mst_atomic_add_affected_crtcs(state); + if (ret) + return ret; + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { if (!needs_modeset(new_crtc_state)) diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index a550abb48b3c..40753d1a29e5 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1002,6 +1002,9 @@ struct intel_crtc_state { /* Forward Error correction State */ bool fec_enable; + /* MST master transcoder for all streams, only used on TGL+ */ + enum transcoder mst_master_transcoder; + /* Pointer to master transcoder in case of tiled displays */ enum transcoder master_transcoder; diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index ad54618f6142..1c040de3a396 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -87,6 +87,50 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, return 0; } +/* + * Iterate over all the CRTCs and return the transcoder of the lowest CRTC that + * shares the same MST connector. + */ +void +intel_dp_mst_compute_master_transcoder(struct intel_connector *mst_connector, + struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *i915 = to_i915(crtc_state->base.crtc->dev); + struct intel_atomic_state *state = + to_intel_atomic_state(crtc_state->base.state); + enum transcoder mst_master_transcoder = crtc_state->cpu_transcoder; + struct intel_crtc_state *other_crtc_state; + struct intel_digital_connector_state *conn_state; + struct intel_connector *conn; + int i; + + if (INTEL_GEN(i915) < 12) + return; + + for_each_new_intel_connector_in_state(state, conn, conn_state, i) { + struct intel_crtc *crtc = to_intel_crtc(conn_state->base.crtc); + + if (conn == mst_connector || + conn->mst_port != mst_connector->mst_port || + !crtc) + continue; + + other_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + if (!other_crtc_state->base.active) + continue; + + if (other_crtc_state->cpu_transcoder < mst_master_transcoder) + mst_master_transcoder = other_crtc_state->cpu_transcoder; + } + + crtc_state->mst_master_transcoder = mst_master_transcoder; + + DRM_DEBUG_KMS("[CRTC:%d:%s] MST master transcoder: %s\n", + crtc_state->base.crtc->base.id, + crtc_state->base.crtc->name, + transcoder_name(mst_master_transcoder)); +} + static int intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) @@ -154,6 +198,42 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder, intel_ddi_compute_min_voltage_level(dev_priv, pipe_config); + intel_dp_mst_compute_master_transcoder(connector, pipe_config); + + return 0; +} + +static int +intel_dp_mst_master_transcoder_check(struct intel_connector *connector, + struct intel_atomic_state *state) +{ + struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct intel_digital_connector_state *new_conn_state = + intel_atomic_get_new_connector_state(state, connector); + struct intel_digital_connector_state *old_conn_state = + intel_atomic_get_old_connector_state(state, connector); + struct intel_crtc_state *new_crtc_state, *old_crtc_state; + struct intel_crtc *crtc; + + if (INTEL_GEN(i915) < 12) + return 0; + + /* + * A modeset will be triggered when checking other affected crtcs if + * this was connected to a master transcoder + */ + if (!new_conn_state->base.crtc) + return 0; + + crtc = to_intel_crtc(new_conn_state->base.crtc); + new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc); + + if (!old_conn_state || + new_crtc_state->mst_master_transcoder != + old_crtc_state->mst_master_transcoder) + new_crtc_state->base.mode_changed = true; + return 0; } @@ -176,6 +256,11 @@ intel_dp_mst_atomic_check(struct drm_connector *connector, if (ret) return ret; + ret = intel_dp_mst_master_transcoder_check(intel_connector, + to_intel_atomic_state(state)); + if (ret) + return ret; + if (!old_conn_state->crtc) return 0; @@ -706,3 +791,61 @@ intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port) drm_dp_mst_topology_mgr_destroy(&intel_dp->mst_mgr); /* encoders will get killed by normal cleanup */ } + +/** + * intel_dp_mst_atomic_add_affected_crtcs - Add all CRTCs that share the MST + * port with the CRTCs in the current atomic state. + * @state: state to add CRTCs + * + * We need to make the CRTCs trigger a call to atomic_check() to every connector + * attached to the CRTC in case a new master transcoder is needed. + */ +int intel_dp_mst_atomic_add_affected_crtcs(struct intel_atomic_state *state) +{ + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_digital_connector_state *intel_conn_state; + struct drm_device *dev = state->base.dev; + struct intel_connector *intel_conn; + int i; + + if (INTEL_GEN(dev_priv) < 12) + return 0; + + for_each_new_intel_connector_in_state(state, intel_conn, intel_conn_state, i) { + struct drm_connector_list_iter conn_list_iter; + struct drm_connector *conn_iter; + + if (!intel_conn->mst_port) + continue; + + drm_connector_list_iter_begin(dev, &conn_list_iter); + drm_for_each_connector_iter(conn_iter, &conn_list_iter) { + struct drm_connector_state *conn_iter_state; + struct intel_connector *intel_conn_iter; + struct drm_crtc_state *crtc_state; + + intel_conn_iter = to_intel_connector(conn_iter); + + if (intel_conn_iter->mst_port != intel_conn->mst_port) + continue; + + conn_iter_state = drm_atomic_get_connector_state(&state->base, conn_iter); + if (IS_ERR(conn_iter_state)) { + drm_connector_list_iter_end(&conn_list_iter); + return PTR_ERR(conn_iter_state); + } + if (!conn_iter_state->crtc) + continue; + + crtc_state = drm_atomic_get_crtc_state(&state->base, + conn_iter_state->crtc); + if (IS_ERR(crtc_state)) { + drm_connector_list_iter_end(&conn_list_iter); + return PTR_ERR(crtc_state); + } + } + drm_connector_list_iter_end(&conn_list_iter); + } + + return 0; +} diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h index f660ad80db04..9e654f77a2d2 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.h +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h @@ -7,9 +7,11 @@ #define __INTEL_DP_MST_H__ struct intel_digital_port; +struct intel_atomic_state; int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id); void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port); int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port); +int intel_dp_mst_atomic_add_affected_crtcs(struct intel_atomic_state *state); #endif /* __INTEL_DP_MST_H__ */