[Intel-gfx] [PATCH v4 7/7] drm/i915/dp: Program vswing, pre-emphasis, test-pattern
Manasi Navare
manasi.d.navare at intel.com
Thu Mar 12 23:42:13 UTC 2020
Just a few nits below
On Tue, Mar 10, 2020 at 09:07:45PM +0530, Animesh Manna wrote:
> This patch process phy compliance request by programming requested
> vswing, pre-emphasis and test pattern.
>
> v1: Initial patch.
> v2: Fixes added during testing with test-scope. (Khaled/Clint/Manasi)
> - pipe used as argument during registers programming instead of port.
> - TRANS_CONF must be disable/enable as well during ddi disable/enable.
> - harcoded PLTPAT 80 bit custom pattern as the DPR-100 does not set it
> in the sink’s DPCDs
> - TRANS_DDI_FUNC_CTL DDI_Select (Bits 27:30) need to reset/set during
> disable/enable.
>
> Cc: Clinton Taylor <Clinton.A.Taylor at intel.com>
> Cc: Manasi Navare <manasi.d.navare at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 135 ++++++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dp.h | 1 +
> 2 files changed, 136 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 16a4a48c8168..0239a72537ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5020,6 +5020,139 @@ static u8 intel_dp_prepare_phytest(struct intel_dp *intel_dp)
> return DP_TEST_ACK;
> }
>
> +static void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> +{
> + struct drm_i915_private *dev_priv =
> + to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + enum pipe pipe = crtc->pipe;
> + u32 temp;
May be call this a custom_80_val or something not temp
Also you need to add a comment/FIXME explaining why the custom 80 PHY values
are hardcoded to this explaining that DPR 100 current firmware does not
write this in the blah blah DPCD correctly and hence hardcoding
for now
same for CP2520
> +
> + switch (data->phy_pattern) {
> + case DP_PHY_TEST_PATTERN_NONE:
> + DRM_DEBUG_KMS("Disable Phy Test Pattern\n");
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe), 0x0);
> + break;
> + case DP_PHY_TEST_PATTERN_D10_2:
> + DRM_DEBUG_KMS("Set D10.2 Phy Test Pattern\n");
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_D10_2);
> + break;
> + case DP_PHY_TEST_PATTERN_ERROR_COUNT:
> + DRM_DEBUG_KMS("Set Error Count Phy Test Pattern\n");
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
> + DDI_DP_COMP_CTL_ENABLE |
> + DDI_DP_COMP_CTL_SCRAMBLED_0);
> + break;
> + case DP_PHY_TEST_PATTERN_PRBS7:
> + DRM_DEBUG_KMS("Set PRBS7 Phy Test Pattern\n");
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_PRBS7);
> + break;
> + case DP_PHY_TEST_PATTERN_80BIT_CUSTOM:
> + DRM_DEBUG_KMS("Set 80Bit Custom Phy Test Pattern 0x3e0f83e0 0x0f83e0f8 0x0000f83e\n");
> + temp = 0x3e0f83e0;
> + intel_de_write(dev_priv, DDI_DP_COMP_PAT(pipe, 0), temp);
> + temp = 0x0f83e0f8;
> + intel_de_write(dev_priv, DDI_DP_COMP_PAT(pipe, 1), temp);
> + temp = 0x0000f83e;
> + intel_de_write(dev_priv, DDI_DP_COMP_PAT(pipe, 2), temp);
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
> + DDI_DP_COMP_CTL_ENABLE |
> + DDI_DP_COMP_CTL_CUSTOM80);
> + break;
> + case DP_PHY_TEST_PATTERN_CP2520:
> + DRM_DEBUG_KMS("Set HBR2 compliance Phy Test Pattern\n");
> + temp = 0xFB;
> + intel_de_write(dev_priv, DDI_DP_COMP_CTL(pipe),
> + DDI_DP_COMP_CTL_ENABLE | DDI_DP_COMP_CTL_HBR2 |
> + temp);
> + break;
> + default:
> + WARN(1, "Invalid Phy Test PAttern\n");
^^^ Pattern
> + }
> +}
> +
> +static void
> +intel_dp_autotest_phy_ddi_disable(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + enum pipe pipe = crtc->pipe;
> + u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> +
> + trans_ddi_func_ctl_value = intel_de_read(dev_priv,
> + TRANS_DDI_FUNC_CTL(pipe));
> + trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
> + dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
> +
> + trans_ddi_func_ctl_value &= ~(0x1F << 27);
Try and use the already define macro in i915_reg.h for,
Trans_ddi_func_ctl_value &= ~( TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_PORT_MASK)
> + trans_conf_value &= ~PIPECONF_ENABLE;
> + dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE;
> +
> + intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
> + intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
> + trans_ddi_func_ctl_value);
> + intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> +}
> +
> +static void
> +intel_dp_autotest_phy_ddi_enable(struct intel_dp *intel_dp, uint8_t lane_cnt)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + enum port port = intel_dig_port->base.port;
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + enum pipe pipe = crtc->pipe;
> + u32 trans_ddi_func_ctl_value, trans_conf_value, dp_tp_ctl_value;
> +
> + trans_ddi_func_ctl_value = intel_de_read(dev_priv,
> + TRANS_DDI_FUNC_CTL(pipe));
> + trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe));
> + dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe));
> +
> + trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE | ((port + 1) << 27);
Same here please use the macros from i915_reg.h
trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE | TGL_TRANS_DDI_SELECT_PORT(port);
Regards
Manasi
> + trans_conf_value |= PIPECONF_ENABLE;
> + dp_tp_ctl_value |= DP_TP_CTL_ENABLE;
> +
> + intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value);
> + intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value);
> + intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe),
> + trans_ddi_func_ctl_value);
> +}
> +
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp)
> +{
> + struct drm_dp_phy_test_params *data =
> + &intel_dp->compliance.test_data.phytest;
> + u8 link_status[DP_LINK_STATUS_SIZE];
> +
> + if (!intel_dp_get_link_status(intel_dp, link_status)) {
> + DRM_DEBUG_KMS("failed to get link status\n");
> + return;
> + }
> +
> + /* retrieve vswing & pre-emphasis setting */
> + intel_dp_get_adjust_train(intel_dp, link_status);
> +
> + intel_dp_autotest_phy_ddi_disable(intel_dp);
> +
> + intel_dp_set_signal_levels(intel_dp);
> +
> + intel_dp_phy_pattern_update(intel_dp);
> +
> + intel_dp_autotest_phy_ddi_enable(intel_dp, data->num_lanes);
> +
> + drm_dp_set_phy_test_pattern(&intel_dp->aux, data,
> + link_status[DP_DPCD_REV]);
> +}
> +
> static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> {
> u8 test_result = DP_TEST_NAK;
> @@ -5028,6 +5161,8 @@ static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> if (test_result != DP_TEST_ACK)
> DRM_ERROR("Phy test preparation failed\n");
>
> + intel_dp_process_phy_request(intel_dp);
> +
> return test_result;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 0c7be8ed1423..0d22a230b32d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -115,6 +115,7 @@ void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
> bool intel_digital_port_connected(struct intel_encoder *encoder);
> +void intel_dp_process_phy_request(struct intel_dp *intel_dp);
>
> static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> {
> --
> 2.24.0
>
More information about the Intel-gfx
mailing list