[Intel-gfx] [RFC 6/6] drm/i915/dp: Program vswing, pre-emphasis, test-pattern

[Manasi Navare]

On Tue, Oct 22, 2019 at 08:37:58PM +0530, Animesh Manna wrote:
> 
> On 10/22/2019 5:17 AM, Manasi Navare wrote:
> >On Thu, Oct 03, 2019 at 08:36:53PM +0530, Animesh Manna wrote:
> >>This patch process phy compliance request by programming requested
> >>vswing, pre-emphasis and test pattern.
> >So the design of this whole patch will need to be changed to work with the whole kms infrastructure
> >Basically what you are doing here is  handling the HPD test request, getting the test patterns.
> >populating the intel_dp_compliance phytest struct and also writing the I915 regs right then and there.
> >
> >This model does not fit the atomic KMS infrastructure, IMO this is how it should look like:
> >
> >1. Handle the test request
> >2. Prepare the PHY patterns where you erad the test patterns from DPCD and populate
> >the intel_dp_compliance phytest struct
> >3. set test_active = true
> >4. Now in atomic_commit_tail somewhere just before enabling pipe, is where
> >based on if intel_dp_compliance.phytest not NULL you will need to call ddi_disable, enable,
> >set_signal_levels, phy_update on the other hand all teh functions that write data to the
> >I915 registers will need to happen here.
> 
> Got the high level view, more brainstorming will help here to finalize the
> design. Hope initial patches will not much change irrespective of the design
> 
> Do we need any drm-property here? The process of auto phy compliance testing
> is general for all driver so a thought came.
>

No need to have any property since the control to run phy test
stays within the kernel. So we can just make use of whether the intel_dp_compliance.phytest struct
is set or not to determine calling phy test related functions in atomic_commit_tail before
enabling the pipe

Manasi
 
> Have an option to put the preparation for phy compliance request in
> atomic_check.
> 
> Will wait for feedback from Jani, Clint and others.
> 
> Regards,
> Animesh
> 
> >
> >Clint, Jani N any thoughts here?
> >
> >Regards
> >Manasi
> >
> >>Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> >>---
> >>  drivers/gpu/drm/i915/display/intel_dp.c | 62 +++++++++++++++++++++++++
> >>  1 file changed, 62 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >>index 93b1ce80c174..dd4c3a81c11d 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >>@@ -4817,14 +4817,76 @@ static inline void intel_dp_phy_pattern_update(struct intel_dp *intel_dp)
> >>  	}
> >>  }
> >>+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);
> >>+	enum port port = intel_dig_port->base.port;
> >>+	u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>+
> >>+	ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>+	dp_tp_ctl_value = I915_READ(DP_TP_CTL(port));
> >>+	trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>+
> >>+	ddi_buf_ctl_value        &= ~(DDI_BUF_CTL_ENABLE | DDI_PORT_WIDTH_MASK);
> >>+	dp_tp_ctl_value          &= ~DP_TP_CTL_ENABLE;
> >>+	trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE |
> >>+				      DDI_PORT_WIDTH_MASK);
> >>+
> >>+	I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>+	I915_WRITE(DP_TP_CTL(port), dp_tp_ctl_value);
> >>+	I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_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;
> >>+	u32 ddi_buf_ctl_value, dp_tp_ctl_value, trans_ddi_func_ctl_value;
> >>+
> >>+	ddi_buf_ctl_value = I915_READ(DDI_BUF_CTL(port));
> >>+	dp_tp_ctl_value = I915_READ(DP_TP_CTL(port));
> >>+	trans_ddi_func_ctl_value = I915_READ(TRANS_DDI_FUNC_CTL(port));
> >>+
> >>+	ddi_buf_ctl_value        |= DDI_BUF_CTL_ENABLE |
> >>+				    DDI_PORT_WIDTH(lane_cnt);
> >>+	dp_tp_ctl_value          |= DP_TP_CTL_ENABLE;
> >>+	trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE |
> >>+				    DDI_PORT_WIDTH(lane_cnt);
> >>+
> >>+	I915_WRITE(TRANS_DDI_FUNC_CTL(port), trans_ddi_func_ctl_value);
> >>+	I915_WRITE(DP_TP_CTL(port), dp_tp_ctl_value);
> >>+	I915_WRITE(DDI_BUF_CTL(port), ddi_buf_ctl_value);
> >>+}
> >>+
> >>  static u8 intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp)
> >>  {
> >>  	u8 test_result = DP_TEST_NAK;
> >>+	struct drm_dp_phy_test_params *data =
> >>+		&intel_dp->compliance.test_data.phytest;
> >>  	test_result = intel_dp_prepare_phytest(intel_dp);
> >>  	if (test_result != DP_TEST_ACK)
> >>  		DRM_ERROR("Phy test preparation failed\n");
> >>+	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->link.num_lanes);
> >>+
> >>+	drm_dp_set_phy_test_pattern(&intel_dp->aux, data);
> >>+
> >>+	/* Set test active flag here so userspace doesn't interrupt things */
> >>+	intel_dp->compliance.test_active = 1;
> >>+
> >>  	return test_result;
> >>  }
> >>-- 
> >>2.22.0
> >>
>