Re: [PATCH 1/5] drm/i915: Move all the DP compliance data to a separate struct

On Fri, 09 Dec 2016, Manasi Navare manasi.d.navare@intel.com 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@intel.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwl.ch

---

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@intel.com

} 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 {