[3/5] drm/i915: Implement the intel_dp_autotest_edid function for DP EDID complaince tests

Submitted by Todd Previte on April 18, 2015, 7:04 a.m.

Details

Message ID 1429340659-14369-4-git-send-email-tprevite@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Todd Previte April 18, 2015, 7:04 a.m.
Updates the EDID compliance test function to perform the analyze and react to
the EDID data read as a result of a hot plug event. The results of this
analysis are handed off to userspace so that the userspace app can set the
display mode appropriately for the test result/response.

The compliance_test_active flag now appears at the end of the individual
test handling functions. This is so that the kernel-side operations can
be completed without the risk of interruption from the userspace app
that is polling on that flag.

V2:
- Addressed mailing list feedback
- Removed excess debug messages
- Removed extraneous comments
- Fixed formatting issues (line length > 80)
- Updated the debug message in compute_edid_checksum to output hex values
  instead of decimal
V3:
- Addressed more list feedback
- Added the test_active flag to the autotest function
- Removed test_active flag from handler
- Added failsafe check on the compliance test active flag
  at the end of the test handler
- Fixed checkpatch.pl issues
V4:
- Removed the checksum computation function and its use as it has been
  rendered superfluous by changes to the core DRM EDID functions
- Updated to use the raw header corruption detection mechanism
- Moved the declaration of the test_data variable here
V5:
- Update test active flag variable name to match the change in the
  first patch of the series.
- Relocated the test active flag declaration and initialization
  to this patch
V6:
- Updated to use the new flag for raw EDID header corruption
- Removed the extra EDID read from the autotest function
- Added the edid_checksum variable to struct intel_dp so that the
  autotest function can write it to the sink device
- Moved the update to the hpd_pulse function to another patch
- Removed extraneous constants
V7:
- Fixed erroneous placement of the checksum assignment. In some cases
  such as when the EDID read fails and is NULL, this causes a NULL ptr
  dereference in the kernel. Bad news. Fixed now.
V8:
- Updated to support the kfree() on the EDID data added previously
V9:
- Updated for the long_hpd flag propagation
V10:
- Updated to use actual checksum from the EDID read that occurs during
  normal hot plug path execution
- Removed variables from intel_dp struct that are no longer needed
- Updated the patch subject to more closely match the nature and contents
  of the patch

Signed-off-by: Todd Previte <tprevite@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 43 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 55d1f5f..4c1f6a9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -41,6 +41,12 @@ 
 
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
+/* Compliance test status bits  */
+#define INTEL_DP_RESOLUTION_SHIFT_MASK	4
+#define INTEL_DP_RESOLUTION_PREFERRED	(1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+#define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
+
 struct dp_link_dpll {
 	int link_bw;
 	struct dpll dpll;
@@ -3994,6 +4000,38 @@  static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
 static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
 {
 	uint8_t test_result = DP_TEST_NAK;
+	uint32_t ret = 0;
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct drm_connector *connector = &intel_connector->base;
+
+	if (intel_connector->detect_edid == NULL ||
+	    connector->edid_corrupt == 1 ||
+	    intel_dp->aux.i2c_defer_count > 6) {
+		/* Check for NACKs/DEFERs, use failsafe if detected
+		 *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
+		 */
+		if (intel_dp->aux.i2c_nack_count > 0 ||
+			intel_dp->aux.i2c_defer_count > 0)
+			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;
+	} else {
+		ret = drm_dp_dpcd_write(&intel_dp->aux,
+					DP_TEST_EDID_CHECKSUM,
+					&intel_connector->detect_edid->checksum, 1);
+		if (ret <= 0)
+			DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
+		else
+			DRM_DEBUG_DRIVER("EDID checksum written to sink\n");
+
+		test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
+		intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
+	}
+
+	/* Set test active flag here so userspace doesn't interrupt things */
+	intel_dp->compliance_test_active = 1;
+
 	return test_result;
 }
 
@@ -4009,7 +4047,10 @@  static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	uint8_t rxdata = 0;
 	int status = 0;
 
+	intel_dp->compliance_test_active = 0;
 	intel_dp->compliance_test_type = 0;
+	intel_dp->compliance_test_data = 0;
+
 	intel_dp->aux.i2c_nack_count = 0;
 	intel_dp->aux.i2c_defer_count = 0;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a4675fa..6c71be9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -742,6 +742,8 @@  struct intel_dp {
 
 	/* Displayport compliance testing */
 	unsigned long compliance_test_type;
+	unsigned long compliance_test_data;
+	bool compliance_test_active;
 };
 
 struct intel_digital_port {

Comments

2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> Updates the EDID compliance test function to perform the analyze and react to
> the EDID data read as a result of a hot plug event. The results of this
> analysis are handed off to userspace so that the userspace app can set the
> display mode appropriately for the test result/response.
>
> The compliance_test_active flag now appears at the end of the individual
> test handling functions. This is so that the kernel-side operations can
> be completed without the risk of interruption from the userspace app
> that is polling on that flag.
>
> V2:
> - Addressed mailing list feedback
> - Removed excess debug messages
> - Removed extraneous comments
> - Fixed formatting issues (line length > 80)
> - Updated the debug message in compute_edid_checksum to output hex values
>   instead of decimal
> V3:
> - Addressed more list feedback
> - Added the test_active flag to the autotest function
> - Removed test_active flag from handler
> - Added failsafe check on the compliance test active flag
>   at the end of the test handler
> - Fixed checkpatch.pl issues
> V4:
> - Removed the checksum computation function and its use as it has been
>   rendered superfluous by changes to the core DRM EDID functions
> - Updated to use the raw header corruption detection mechanism
> - Moved the declaration of the test_data variable here
> V5:
> - Update test active flag variable name to match the change in the
>   first patch of the series.
> - Relocated the test active flag declaration and initialization
>   to this patch
> V6:
> - Updated to use the new flag for raw EDID header corruption
> - Removed the extra EDID read from the autotest function
> - Added the edid_checksum variable to struct intel_dp so that the
>   autotest function can write it to the sink device
> - Moved the update to the hpd_pulse function to another patch
> - Removed extraneous constants
> V7:
> - Fixed erroneous placement of the checksum assignment. In some cases
>   such as when the EDID read fails and is NULL, this causes a NULL ptr
>   dereference in the kernel. Bad news. Fixed now.
> V8:
> - Updated to support the kfree() on the EDID data added previously
> V9:
> - Updated for the long_hpd flag propagation
> V10:
> - Updated to use actual checksum from the EDID read that occurs during
>   normal hot plug path execution
> - Removed variables from intel_dp struct that are no longer needed
> - Updated the patch subject to more closely match the nature and contents
>   of the patch
>
> Signed-off-by: Todd Previte <tprevite@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 55d1f5f..4c1f6a9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -41,6 +41,12 @@
>
>  #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)
>
> +/* Compliance test status bits  */
> +#define INTEL_DP_RESOLUTION_SHIFT_MASK 4

Now that you cleaned all the other stuff, can't we start at 0 instead
of 4? (of course, this would require a change to the user space tool)


> +#define INTEL_DP_RESOLUTION_PREFERRED  (1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> +#define INTEL_DP_RESOLUTION_STANDARD   (2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> +#define INTEL_DP_RESOLUTION_FAILSAFE   (3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> +
>  struct dp_link_dpll {
>         int link_bw;
>         struct dpll dpll;
> @@ -3994,6 +4000,38 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>  static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
>  {
>         uint8_t test_result = DP_TEST_NAK;
> +       uint32_t ret = 0;
> +       struct intel_connector *intel_connector = intel_dp->attached_connector;
> +       struct drm_connector *connector = &intel_connector->base;
> +
> +       if (intel_connector->detect_edid == NULL ||
> +           connector->edid_corrupt == 1 ||
> +           intel_dp->aux.i2c_defer_count > 6) {
> +               /* Check for NACKs/DEFERs, use failsafe if detected
> +                *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
> +                */
> +               if (intel_dp->aux.i2c_nack_count > 0 ||
> +                       intel_dp->aux.i2c_defer_count > 0)
> +                       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;
> +       } else {
> +               ret = drm_dp_dpcd_write(&intel_dp->aux,
> +                                       DP_TEST_EDID_CHECKSUM,
> +                                       &intel_connector->detect_edid->checksum, 1);
> +               if (ret <= 0)
> +                       DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
> +               else
> +                       DRM_DEBUG_DRIVER("EDID checksum written to sink\n");

You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
one, and I'd probably pick _KMS.

With or without the above bikesheds: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>

> +
> +               test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> +               intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
> +       }
> +
> +       /* Set test active flag here so userspace doesn't interrupt things */
> +       intel_dp->compliance_test_active = 1;
> +
>         return test_result;
>  }
>
> @@ -4009,7 +4047,10 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>         uint8_t rxdata = 0;
>         int status = 0;
>
> +       intel_dp->compliance_test_active = 0;
>         intel_dp->compliance_test_type = 0;
> +       intel_dp->compliance_test_data = 0;
> +
>         intel_dp->aux.i2c_nack_count = 0;
>         intel_dp->aux.i2c_defer_count = 0;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a4675fa..6c71be9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -742,6 +742,8 @@ struct intel_dp {
>
>         /* Displayport compliance testing */
>         unsigned long compliance_test_type;
> +       unsigned long compliance_test_data;
> +       bool compliance_test_active;
>  };
>
>  struct intel_digital_port {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Apr 30, 2015 at 03:30:10PM -0300, Paulo Zanoni wrote:
> 2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
> > Updates the EDID compliance test function to perform the analyze and react to
> > the EDID data read as a result of a hot plug event. The results of this
> > analysis are handed off to userspace so that the userspace app can set the
> > display mode appropriately for the test result/response.
> >
> > The compliance_test_active flag now appears at the end of the individual
> > test handling functions. This is so that the kernel-side operations can
> > be completed without the risk of interruption from the userspace app
> > that is polling on that flag.
> >
> > V2:
> > - Addressed mailing list feedback
> > - Removed excess debug messages
> > - Removed extraneous comments
> > - Fixed formatting issues (line length > 80)
> > - Updated the debug message in compute_edid_checksum to output hex values
> >   instead of decimal
> > V3:
> > - Addressed more list feedback
> > - Added the test_active flag to the autotest function
> > - Removed test_active flag from handler
> > - Added failsafe check on the compliance test active flag
> >   at the end of the test handler
> > - Fixed checkpatch.pl issues
> > V4:
> > - Removed the checksum computation function and its use as it has been
> >   rendered superfluous by changes to the core DRM EDID functions
> > - Updated to use the raw header corruption detection mechanism
> > - Moved the declaration of the test_data variable here
> > V5:
> > - Update test active flag variable name to match the change in the
> >   first patch of the series.
> > - Relocated the test active flag declaration and initialization
> >   to this patch
> > V6:
> > - Updated to use the new flag for raw EDID header corruption
> > - Removed the extra EDID read from the autotest function
> > - Added the edid_checksum variable to struct intel_dp so that the
> >   autotest function can write it to the sink device
> > - Moved the update to the hpd_pulse function to another patch
> > - Removed extraneous constants
> > V7:
> > - Fixed erroneous placement of the checksum assignment. In some cases
> >   such as when the EDID read fails and is NULL, this causes a NULL ptr
> >   dereference in the kernel. Bad news. Fixed now.
> > V8:
> > - Updated to support the kfree() on the EDID data added previously
> > V9:
> > - Updated for the long_hpd flag propagation
> > V10:
> > - Updated to use actual checksum from the EDID read that occurs during
> >   normal hot plug path execution
> > - Removed variables from intel_dp struct that are no longer needed
> > - Updated the patch subject to more closely match the nature and contents
> >   of the patch
> >
> > Signed-off-by: Todd Previte <tprevite@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 55d1f5f..4c1f6a9 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -41,6 +41,12 @@
> >
> >  #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)
> >
> > +/* Compliance test status bits  */
> > +#define INTEL_DP_RESOLUTION_SHIFT_MASK 4
> 
> Now that you cleaned all the other stuff, can't we start at 0 instead
> of 4? (of course, this would require a change to the user space tool)
> 
> 
> > +#define INTEL_DP_RESOLUTION_PREFERRED  (1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> > +#define INTEL_DP_RESOLUTION_STANDARD   (2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> > +#define INTEL_DP_RESOLUTION_FAILSAFE   (3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
> > +
> >  struct dp_link_dpll {
> >         int link_bw;
> >         struct dpll dpll;
> > @@ -3994,6 +4000,38 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
> >  static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
> >  {
> >         uint8_t test_result = DP_TEST_NAK;
> > +       uint32_t ret = 0;
> > +       struct intel_connector *intel_connector = intel_dp->attached_connector;
> > +       struct drm_connector *connector = &intel_connector->base;
> > +
> > +       if (intel_connector->detect_edid == NULL ||
> > +           connector->edid_corrupt == 1 ||
> > +           intel_dp->aux.i2c_defer_count > 6) {
> > +               /* Check for NACKs/DEFERs, use failsafe if detected
> > +                *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
> > +                */
> > +               if (intel_dp->aux.i2c_nack_count > 0 ||
> > +                       intel_dp->aux.i2c_defer_count > 0)
> > +                       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;
> > +       } else {
> > +               ret = drm_dp_dpcd_write(&intel_dp->aux,
> > +                                       DP_TEST_EDID_CHECKSUM,
> > +                                       &intel_connector->detect_edid->checksum, 1);
> > +               if (ret <= 0)
> > +                       DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
> > +               else
> > +                       DRM_DEBUG_DRIVER("EDID checksum written to sink\n");
> 
> You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
> one, and I'd probably pick _KMS.
> 
> With or without the above bikesheds: Reviewed-by: Paulo Zanoni
> <paulo.r.zanoni@intel.com>

Yeah I second the two polish requests from Paulo. Merged the previous
patch meanwhile.
-Daniel

> 
> > +
> > +               test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
> > +               intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
> > +       }
> > +
> > +       /* Set test active flag here so userspace doesn't interrupt things */
> > +       intel_dp->compliance_test_active = 1;
> > +
> >         return test_result;
> >  }
> >
> > @@ -4009,7 +4047,10 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >         uint8_t rxdata = 0;
> >         int status = 0;
> >
> > +       intel_dp->compliance_test_active = 0;
> >         intel_dp->compliance_test_type = 0;
> > +       intel_dp->compliance_test_data = 0;
> > +
> >         intel_dp->aux.i2c_nack_count = 0;
> >         intel_dp->aux.i2c_defer_count = 0;
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a4675fa..6c71be9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -742,6 +742,8 @@ struct intel_dp {
> >
> >         /* Displayport compliance testing */
> >         unsigned long compliance_test_type;
> > +       unsigned long compliance_test_data;
> > +       bool compliance_test_active;
> >  };
> >
> >  struct intel_digital_port {
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 04 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 30, 2015 at 03:30:10PM -0300, Paulo Zanoni wrote:
>> 2015-04-18 4:04 GMT-03:00 Todd Previte <tprevite@gmail.com>:
>> > Updates the EDID compliance test function to perform the analyze and react to
>> > the EDID data read as a result of a hot plug event. The results of this
>> > analysis are handed off to userspace so that the userspace app can set the
>> > display mode appropriately for the test result/response.
>> >
>> > The compliance_test_active flag now appears at the end of the individual
>> > test handling functions. This is so that the kernel-side operations can
>> > be completed without the risk of interruption from the userspace app
>> > that is polling on that flag.
>> >
>> > V2:
>> > - Addressed mailing list feedback
>> > - Removed excess debug messages
>> > - Removed extraneous comments
>> > - Fixed formatting issues (line length > 80)
>> > - Updated the debug message in compute_edid_checksum to output hex values
>> >   instead of decimal
>> > V3:
>> > - Addressed more list feedback
>> > - Added the test_active flag to the autotest function
>> > - Removed test_active flag from handler
>> > - Added failsafe check on the compliance test active flag
>> >   at the end of the test handler
>> > - Fixed checkpatch.pl issues
>> > V4:
>> > - Removed the checksum computation function and its use as it has been
>> >   rendered superfluous by changes to the core DRM EDID functions
>> > - Updated to use the raw header corruption detection mechanism
>> > - Moved the declaration of the test_data variable here
>> > V5:
>> > - Update test active flag variable name to match the change in the
>> >   first patch of the series.
>> > - Relocated the test active flag declaration and initialization
>> >   to this patch
>> > V6:
>> > - Updated to use the new flag for raw EDID header corruption
>> > - Removed the extra EDID read from the autotest function
>> > - Added the edid_checksum variable to struct intel_dp so that the
>> >   autotest function can write it to the sink device
>> > - Moved the update to the hpd_pulse function to another patch
>> > - Removed extraneous constants
>> > V7:
>> > - Fixed erroneous placement of the checksum assignment. In some cases
>> >   such as when the EDID read fails and is NULL, this causes a NULL ptr
>> >   dereference in the kernel. Bad news. Fixed now.
>> > V8:
>> > - Updated to support the kfree() on the EDID data added previously
>> > V9:
>> > - Updated for the long_hpd flag propagation
>> > V10:
>> > - Updated to use actual checksum from the EDID read that occurs during
>> >   normal hot plug path execution
>> > - Removed variables from intel_dp struct that are no longer needed
>> > - Updated the patch subject to more closely match the nature and contents
>> >   of the patch
>> >
>> > Signed-off-by: Todd Previte <tprevite@gmail.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp.c  | 41 ++++++++++++++++++++++++++++++++++++++++
>> >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>> >  2 files changed, 43 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 55d1f5f..4c1f6a9 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -41,6 +41,12 @@
>> >
>> >  #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)
>> >
>> > +/* Compliance test status bits  */
>> > +#define INTEL_DP_RESOLUTION_SHIFT_MASK 4
>> 
>> Now that you cleaned all the other stuff, can't we start at 0 instead
>> of 4? (of course, this would require a change to the user space tool)
>> 
>> 
>> > +#define INTEL_DP_RESOLUTION_PREFERRED  (1 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>> > +#define INTEL_DP_RESOLUTION_STANDARD   (2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>> > +#define INTEL_DP_RESOLUTION_FAILSAFE   (3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>> > +
>> >  struct dp_link_dpll {
>> >         int link_bw;
>> >         struct dpll dpll;
>> > @@ -3994,6 +4000,38 @@ static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp)
>> >  static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp)
>> >  {
>> >         uint8_t test_result = DP_TEST_NAK;
>> > +       uint32_t ret = 0;
>> > +       struct intel_connector *intel_connector = intel_dp->attached_connector;
>> > +       struct drm_connector *connector = &intel_connector->base;
>> > +
>> > +       if (intel_connector->detect_edid == NULL ||
>> > +           connector->edid_corrupt == 1 ||
>> > +           intel_dp->aux.i2c_defer_count > 6) {
>> > +               /* Check for NACKs/DEFERs, use failsafe if detected
>> > +                *  (DP CTS 1.2 Core Rev 1.1, 4.2.2.4, 4.2.2.5)
>> > +                */
>> > +               if (intel_dp->aux.i2c_nack_count > 0 ||
>> > +                       intel_dp->aux.i2c_defer_count > 0)
>> > +                       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;
>> > +       } else {
>> > +               ret = drm_dp_dpcd_write(&intel_dp->aux,
>> > +                                       DP_TEST_EDID_CHECKSUM,
>> > +                                       &intel_connector->detect_edid->checksum, 1);
>> > +               if (ret <= 0)
>> > +                       DRM_DEBUG_DRIVER("Failed to write EDID checksum\n");
>> > +               else
>> > +                       DRM_DEBUG_DRIVER("EDID checksum written to sink\n");
>> 
>> You're mixing DRM_DEBUG_DRIVER and DRM_DEBUG_KMS. I'd stick with only
>> one, and I'd probably pick _KMS.
>> 
>> With or without the above bikesheds: Reviewed-by: Paulo Zanoni
>> <paulo.r.zanoni@intel.com>
>
> Yeah I second the two polish requests from Paulo. Merged the previous
> patch meanwhile.

I reverted this to unblock people because it breaks the build due to the
dependency on patch 2/5 which was apparently not merged yet. Sorry
Daniel, didn't get hold of you fast enough...

BR,
Jani.




> -Daniel
>
>> 
>> > +
>> > +               test_result = DP_TEST_ACK | DP_TEST_EDID_CHECKSUM_WRITE;
>> > +               intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_STANDARD;
>> > +       }
>> > +
>> > +       /* Set test active flag here so userspace doesn't interrupt things */
>> > +       intel_dp->compliance_test_active = 1;
>> > +
>> >         return test_result;
>> >  }
>> >
>> > @@ -4009,7 +4047,10 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>> >         uint8_t rxdata = 0;
>> >         int status = 0;
>> >
>> > +       intel_dp->compliance_test_active = 0;
>> >         intel_dp->compliance_test_type = 0;
>> > +       intel_dp->compliance_test_data = 0;
>> > +
>> >         intel_dp->aux.i2c_nack_count = 0;
>> >         intel_dp->aux.i2c_defer_count = 0;
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index a4675fa..6c71be9 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -742,6 +742,8 @@ struct intel_dp {
>> >
>> >         /* Displayport compliance testing */
>> >         unsigned long compliance_test_type;
>> > +       unsigned long compliance_test_data;
>> > +       bool compliance_test_active;
>> >  };
>> >
>> >  struct intel_digital_port {
>> > --
>> > 1.9.1
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> 
>> 
>> -- 
>> Paulo Zanoni
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx