| Message ID | 1429340659-14369-4-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 {
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(+)