On Fri, 2016-04-29 at 18:28 -0700, Manasi Navare wrote: > This video pattern test function gets invoked through the > compliance test handler on a HPD short pulse if the test type is > set to DP_TEST_VIDEO_PATTERN. This performs the DPCD registers > reads to read the requested test pattern, video pattern resolution, > frame rate and bits per color value. The results of this analysis > are handed off to userspace so that the userspace app can set the > video pattern mode appropriately for the test result/response. > > The compliance_test_active flag is set 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. > > Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++++- > drivers/gpu/drm/i915/intel_dp.c | 76 > +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 9 +++++ > include/drm/drm_dp_helper.h | 14 ++++++- > 4 files changed, 111 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 6ee69b1..c8d0805 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -4458,7 +4458,19 @@ 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); > + else if (intel_dp->compliance_test_type == > + DP_TEST_LINK_VIDEO_PATTERN) { > + seq_printf(m, "hdisplay: %lu\n", > + intel_dp->test_data.hdisplay); > + seq_printf(m, "vdisplay: %lu\n", > + intel_dp->test_data.vdisplay); > + seq_printf(m, "bpc: %u\n", > + intel_dp->test_data.bpc); > + } > } else > seq_puts(m, "0"); > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 456fc17..134cff8 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4018,6 +4018,82 @@ static uint8_t intel_dp_autotest_link_training(struct > intel_dp *intel_dp) > static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) > { > uint8_t test_result = DP_TEST_NAK; > + uint8_t test_pattern; > + uint16_t h_width, v_height, test_misc; > + int status = 0; > + > + /* Automation support for Video Pattern Tests has a dependency > + * on Link training Automation support (CTS Test 4.3.1.11) > + * Hence it returns a TEST NAK until the Link Training automation > + * support is added to the kernel > + */ > + return test_result;
We shouldn't merge this patch until this is resolved. There's no point in adding dead code. > + > + /* Read the TEST_PATTERN (DP CTS 3.1.5) */ > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_PATTERN, > + &test_pattern, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read test pattern from" > + "refernce sink\n"); > + return 0; > + } > + if (test_pattern != DP_COLOR_RAMP) > + return test_result; > + intel_dp->test_data.video_pattern = test_pattern; > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_H_WIDTH, > + &h_width, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read H Width from" > + "refernce sink\n"); > + return 0; > + } > + intel_dp->test_data.hdisplay = (h_width & DP_TEST_MSB_MASK) >> 8 | > + (h_width << 8); Just use the kernel endianness helpers, i.e.: __le16 h_width; drm_dp_dpcd_read(..., &h_width, 2) hdisplay = le16_to_cpu(h_width); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_V_HEIGHT, > + &v_height, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read V Height from" > + "refernce sink\n"); reference > + return 0; > + } > + intel_dp->test_data.vdisplay = (v_height & DP_TEST_MSB_MASK) >> 8 | > + (v_height << 8); Same here. > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC, > + &test_misc, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("Could not read TEST MISC from" > + "refernce sink\n"); reference > + return 0; > + } > + if (((test_misc & DP_TEST_COLOR_FORMAT_MASK) >> 1) != > + DP_VIDEO_PATTERN_RGB_FORMAT) > + return test_result; > + if (((test_misc & DP_TEST_DYNAMIC_RANGE_MASK) >> 3) != > + DP_VIDEO_PATTERN_VESA) > + return test_result; > + switch ((test_misc & DP_TEST_BIT_DEPTH_MASK) >> 5) { Maybe define *_SHIFT to avoid the hardcoded values. A couple of helpers to make the code more readable woudn't be bad either. > + case 0: > + intel_dp->compliance_force_bpc = 6; > + intel_dp->test_data.bpc = 6; > + break; > + case 1: > + intel_dp->compliance_force_bpc = 8; > + intel_dp->test_data.bpc = 8; So here's where compliance_force_bpc from patch 3 is set. Would be better to squash that patch with this one. > + break; > + default: > + return test_result; > + } > + /* Set test active flag here so userspace doesn't interrupt things */ > + intel_dp->compliance_test_active = 1; > + > + test_result = DP_TEST_ACK; > + DRM_DEBUG_KMS("Hdisplay = %lu\n Vdisplay = %lu\n BPC = %u\n ", > + intel_dp->test_data.hdisplay, > + intel_dp->test_data.vdisplay, > + intel_dp->test_data.bpc); > return test_result; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 10eaff8..6ba8a75 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -794,6 +794,12 @@ enum link_m_n_set { > M2_N2 > }; > > +struct compliance_test_data { > + uint8_t video_pattern; > + uint16_t hdisplay, vdisplay; > + uint8_t bpc; > +}; > + > struct intel_dp { > i915_reg_t output_reg; > i915_reg_t aux_ch_ctl_reg; > @@ -866,6 +872,9 @@ struct intel_dp { > unsigned long compliance_test_data; > bool compliance_test_active; > unsigned long compliance_force_bpc; /* 0 for default or bpc to use */ > + struct compliance_test_data test_data; /* a structure to hold all > dp > + * compliance test data > + */ > }; > > struct intel_digital_port { > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index 92d9a52..7422dc5 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -412,7 +412,19 @@ > > #define DP_TEST_LANE_COUNT 0x220 > > -#define DP_TEST_PATTERN 0x221 > +#define DP_TEST_PATTERN 0x221 > +#define DP_COLOR_RAMP (1 << 0) > +#define DP_TEST_H_WIDTH 0x22E > +#define DP_TEST_V_HEIGHT 0x230 > +#define DP_TEST_MISC 0x232 > +#define DP_TEST_MSB_MASK 0xFF00 > +#define DP_VIDEO_PATTERN_RGB_FORMAT 0 > +#define DP_TEST_COLOR_FORMAT_MASK 0x6 > +#define DP_TEST_DYNAMIC_RANGE_MASK (1 << 3) > +#define DP_VIDEO_PATTERN_VESA 0 > +#define DP_TEST_BIT_DEPTH_MASK 0x00E0 > +#define DP_TEST_BIT_DEPTH_6 0 > +#define DP_TEST_BIT_DEPTH_8 1 This should be in a separate patch with Cc: dri-de...@lists.freedesktop.org > > #define DP_TEST_CRC_R_CR 0x240 > #define DP_TEST_CRC_G_Y 0x242 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx