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

[Animesh Manna <animesh.manna@intel.com>]

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.

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@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
>>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx