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@xxxxxxxxx>
> >>---
> >> 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
> >>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
