On Fri, 09 Dec 2016, Manasi Navare <manasi.d.navare@xxxxxxxxx> wrote:
> This patch does not change anything functionally, just cleans up
> the DP compliance related variables and stores them all together
> in a separate struct intel_dp_compliance. There is another struct
> intel_dp_compliance_data to store all the test data. This makes it easy to
> reset the compliance variables through a memset instead of
> individual resetting.
>
> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 13 ++++++++-----
> drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++--------------
> drivers/gpu/drm/i915/intel_drv.h | 14 +++++++++++---
> 3 files changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a746130..b2ff532 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4463,9 +4463,9 @@ static ssize_t i915_displayport_test_active_write(struct file *file,
> * testing code, only accept an actual value of 1 here
> */
> if (val == 1)
> - intel_dp->compliance_test_active = 1;
> + intel_dp->compliance.test_active = 1;
> else
> - intel_dp->compliance_test_active = 0;
> + intel_dp->compliance.test_active = 0;
> }
> }
> out:
> @@ -4492,7 +4492,7 @@ static int i915_displayport_test_active_show(struct seq_file *m, void *data)
> if (connector->status == connector_status_connected &&
> connector->encoder != NULL) {
> intel_dp = enc_to_intel_dp(connector->encoder);
> - if (intel_dp->compliance_test_active)
> + if (intel_dp->compliance.test_active)
> seq_puts(m, "1");
> else
> seq_puts(m, "0");
> @@ -4536,7 +4536,10 @@ static int i915_displayport_test_data_show(struct seq_file *m, void *data)
> if (connector->status == connector_status_connected &&
> connector->encoder != NULL) {
> intel_dp = enc_to_intel_dp(connector->encoder);
> - seq_printf(m, "%lx", intel_dp->compliance_test_data);
> + if (intel_dp->compliance.test_type ==
> + DP_TEST_LINK_EDID_READ)
> + seq_printf(m, "%lx",
> + intel_dp->compliance.test_data.edid);
The commit message says no functional changes, but this is one. Probably
better to make this change separately.
With that fixed,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> } else
> seq_puts(m, "0");
> }
> @@ -4575,7 +4578,7 @@ static int i915_displayport_test_type_show(struct seq_file *m, void *data)
> if (connector->status == connector_status_connected &&
> connector->encoder != NULL) {
> intel_dp = enc_to_intel_dp(connector->encoder);
> - seq_printf(m, "%02lx", intel_dp->compliance_test_type);
> + seq_printf(m, "%02lx", intel_dp->compliance.test_type);
> } else
> seq_puts(m, "0");
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index db75bb9..c1e107c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3834,7 +3834,7 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("EDID read had %d NACKs, %d DEFERs\n",
> intel_dp->aux.i2c_nack_count,
> intel_dp->aux.i2c_defer_count);
> - intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
> + intel_dp->compliance.test_data.edid = INTEL_DP_RESOLUTION_FAILSAFE;
> } else {
> struct edid *block = intel_connector->detect_edid;
>
> @@ -3850,11 +3850,11 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("Failed to write EDID checksum\n");
>
> test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> - intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
> + intel_dp->compliance.test_data.edid = INTEL_DP_RESOLUTION_STANDARD;
> }
>
> /* Set test active flag here so userspace doesn't interrupt things */
> - intel_dp->compliance_test_active = 1;
> + intel_dp->compliance.test_active = 1;
>
> return test_result;
> }
> @@ -3880,22 +3880,22 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> switch (rxdata) {
> case DP_TEST_LINK_TRAINING:
> DRM_DEBUG_KMS("LINK_TRAINING test requested\n");
> - intel_dp->compliance_test_type = DP_TEST_LINK_TRAINING;
> + intel_dp->compliance.test_type = DP_TEST_LINK_TRAINING;
> response = intel_dp_autotest_link_training(intel_dp);
> break;
> case DP_TEST_LINK_VIDEO_PATTERN:
> DRM_DEBUG_KMS("TEST_PATTERN test requested\n");
> - intel_dp->compliance_test_type = DP_TEST_LINK_VIDEO_PATTERN;
> + intel_dp->compliance.test_type = DP_TEST_LINK_VIDEO_PATTERN;
> response = intel_dp_autotest_video_pattern(intel_dp);
> break;
> case DP_TEST_LINK_EDID_READ:
> DRM_DEBUG_KMS("EDID test requested\n");
> - intel_dp->compliance_test_type = DP_TEST_LINK_EDID_READ;
> + intel_dp->compliance.test_type = DP_TEST_LINK_EDID_READ;
> response = intel_dp_autotest_edid(intel_dp);
> break;
> case DP_TEST_LINK_PHY_TEST_PATTERN:
> DRM_DEBUG_KMS("PHY_PATTERN test requested\n");
> - intel_dp->compliance_test_type = DP_TEST_LINK_PHY_TEST_PATTERN;
> + intel_dp->compliance.test_type = DP_TEST_LINK_PHY_TEST_PATTERN;
> response = intel_dp_autotest_phy_pattern(intel_dp);
> break;
> default:
> @@ -4019,7 +4019,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> return;
>
> /* if link training is requested we should perform it always */
> - if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
> + if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
> (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> intel_encoder->base.name);
> @@ -4053,9 +4053,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> * Clearing compliance test variables to allow capturing
> * of values for next automated test request.
> */
> - intel_dp->compliance_test_active = 0;
> - intel_dp->compliance_test_type = 0;
> - intel_dp->compliance_test_data = 0;
> + memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
>
> /*
> * Now read the DPCD to see if it's actually running
> @@ -4372,9 +4370,7 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> status = connector_status_disconnected;
>
> if (status == connector_status_disconnected) {
> - intel_dp->compliance_test_active = 0;
> - intel_dp->compliance_test_type = 0;
> - intel_dp->compliance_test_data = 0;
> + memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
>
> if (intel_dp->is_mst) {
> DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8f4ddca..f8c7046 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -884,6 +884,16 @@ struct intel_dp_desc {
> u8 sw_minor_rev;
> } __packed;
>
> +struct intel_dp_compliance_data {
> + unsigned long edid;
> +};
> +
> +struct intel_dp_compliance {
> + unsigned long test_type;
> + struct intel_dp_compliance_data test_data;
> + bool test_active;
> +};
> +
> struct intel_dp {
> i915_reg_t output_reg;
> i915_reg_t aux_ch_ctl_reg;
> @@ -959,9 +969,7 @@ struct intel_dp {
> void (*prepare_link_retrain)(struct intel_dp *intel_dp);
>
> /* Displayport compliance testing */
> - unsigned long compliance_test_type;
> - unsigned long compliance_test_data;
> - bool compliance_test_active;
> + struct intel_dp_compliance compliance;
> };
>
> struct intel_lspcon {
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
