Jani, This patch adds a fix for CRC mismatch errors as well. The reason I have squashed this fix with this patch is because these CRC failures are seen for a Video Pattern test. It uses the approach that we discussed where a piep_config->dither_force_disable is used to disable dithering for compliance 18bpp case in intel_modeset_pipe_config.
Regards Manasi On Thu, Jan 19, 2017 at 10:23:38PM -0800, Manasi Navare wrote: > The intel_dp_autotest_video_pattern() 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. > When the test is requested with specific BPC value, we read the BPC > value from the DPCD register. If this BPC value in intel_dp structure > has a non-zero value and we're on a display port connector, then we use > the value to calculate the bpp for the pipe. Also in this case if its > a 18bpp video pattern request, then we force the dithering on pipe to be > disabled since it causes CRC mismatches. > > 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. > > v3: > * Use the updated properly shifted bit definitions (Jani Nikula) > * Force dithering to be disabled on 18bpp compliance > test request (Manasi Navare) > v2: > * Updated the DPCD Register reads based on proper defines in header (Jani > Nikula) > * Squahsed the patch that forced the pipe bpp to compliance test bpp (Jani > Nikula) > Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com> > Cc: Jani Nikula <jani.nik...@linux.intel.com> > Cc: Daniel Vetter <daniel.vet...@intel.com> > Cc: Ville Syrjala <ville.syrj...@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++ > drivers/gpu/drm/i915/intel_display.c | 8 +++-- > drivers/gpu/drm/i915/intel_dp.c | 66 > ++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_dp_mst.c | 7 +++- > drivers/gpu/drm/i915/intel_drv.h | 11 ++++++ > 5 files changed, 98 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 8ec7edf..2eb8e50 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3776,6 +3776,15 @@ static int i915_displayport_test_data_show(struct > seq_file *m, void *data) > DP_TEST_LINK_EDID_READ) > seq_printf(m, "%lx", > intel_dp->compliance.test_data.edid); > + else if (intel_dp->compliance.test_type == > + DP_TEST_LINK_VIDEO_PATTERN) { > + seq_printf(m, "hdisplay: %d\n", > + > intel_dp->compliance.test_data.hdisplay); > + seq_printf(m, "vdisplay: %d\n", > + > intel_dp->compliance.test_data.vdisplay); > + seq_printf(m, "bpc: %u\n", > + intel_dp->compliance.test_data.bpc); > + } > } else > seq_puts(m, "0"); > } > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 0f4272f..c593789 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13052,8 +13052,12 @@ static bool check_digital_port_conflicts(struct > drm_atomic_state *state) > } > > /* Dithering seems to not pass-through bits correctly when it should, so > - * only enable it on 6bpc panels. */ > - pipe_config->dither = pipe_config->pipe_bpp == 6*3; > + * only enable it on 6bpc panels and when its not a compliance > + * test requesting 6bpc video pattern. > + */ > + pipe_config->dither = (pipe_config->pipe_bpp == 6*3) && > + !pipe_config->dither_force_disable; > + > DRM_DEBUG_KMS("hw max bpp: %i, pipe bpp: %i, dithering: %i\n", > base_bpp, pipe_config->pipe_bpp, pipe_config->dither); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 7ca45e0..044e36a 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -28,8 +28,10 @@ > #include <linux/i2c.h> > #include <linux/slab.h> > #include <linux/export.h> > +#include <linux/types.h> > #include <linux/notifier.h> > #include <linux/reboot.h> > +#include <asm/byteorder.h> > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_crtc.h> > @@ -1593,6 +1595,13 @@ static int intel_dp_compute_bpp(struct intel_dp > *intel_dp, > if (bpc > 0) > bpp = min(bpp, 3*bpc); > > + /* For DP Compliance we override the computed bpp for the pipe */ > + if (intel_dp->compliance.test_data.bpc != 0) { > + pipe_config->pipe_bpp = 3*intel_dp->compliance.test_data.bpc; > + pipe_config->dither_force_disable = pipe_config->pipe_bpp == > 6*3; > + DRM_DEBUG_KMS("Setting pipe_bpp to %d\n", > + pipe_config->pipe_bpp); > + } > return bpp; > } > > @@ -3972,6 +3981,63 @@ 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 test_misc; > + __be16 h_width, v_height; > + int status = 0; > + > + /* 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("Test pattern read failed\n"); > + return 0; > + } > + if (test_pattern != DP_COLOR_RAMP) > + return test_result; > + intel_dp->compliance.test_data.video_pattern = test_pattern; > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_H_WIDTH_HI, > + &h_width, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("H Width read failed\n"); > + return 0; > + } > + intel_dp->compliance.test_data.hdisplay = be16_to_cpu(h_width); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_V_HEIGHT_HI, > + &v_height, 2); > + if (status <= 0) { > + DRM_DEBUG_KMS("V Height read failed\n"); > + return 0; > + } > + intel_dp->compliance.test_data.vdisplay = be16_to_cpu(v_height); > + > + status = drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_MISC0, > + &test_misc, 1); > + if (status <= 0) { > + DRM_DEBUG_KMS("TEST MISC read failed\n"); > + return 0; > + } > + if ((test_misc & DP_TEST_COLOR_FORMAT_MASK) != DP_COLOR_FORMAT_RGB) > + return test_result; > + if ((test_misc & DP_TEST_DYNAMIC_RANGE) != DP_VESA_RANGE) > + return test_result; > + switch (test_misc & DP_TEST_BIT_DEPTH_MASK) { > + case DP_TEST_BIT_DEPTH_6: > + intel_dp->compliance.test_data.bpc = 6; > + break; > + case DP_TEST_BIT_DEPTH_8: > + intel_dp->compliance.test_data.bpc = 8; > + 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; > + > return test_result; > } > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c > b/drivers/gpu/drm/i915/intel_dp_mst.c > index 205fe47..29a9af1 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -47,6 +47,11 @@ static bool intel_dp_mst_compute_config(struct > intel_encoder *encoder, > > pipe_config->has_pch_encoder = false; > bpp = 24; > + if (intel_dp->compliance.test_data.bpc) { > + bpp = intel_dp->compliance.test_data.bpc * 3; > + DRM_DEBUG_KMS("Setting pipe bpp to %d\n", > + bpp); > + } > /* > * for MST we always configure max link bw - the spec doesn't > * seem to suggest we should do otherwise. > @@ -55,7 +60,7 @@ static bool intel_dp_mst_compute_config(struct > intel_encoder *encoder, > > pipe_config->lane_count = lane_count; > > - pipe_config->pipe_bpp = 24; > + pipe_config->pipe_bpp = bpp; > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp); > > state = pipe_config->base.state; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 1586a02..503e1b5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -578,6 +578,14 @@ struct intel_crtc_state { > */ > bool dither; > > + /* > + * Dither gets enabled for 18bpp which causes CRC mismatch errors for > + * compliance video pattern tests. > + * Disable dither only if it is a compliance test request for > + * 18bpp. > + */ > + bool dither_force_disable; > + > /* Controls for the clock computation, to override various stages. */ > bool clock_set; > > @@ -888,6 +896,9 @@ struct intel_dp_desc { > > struct intel_dp_compliance_data { > unsigned long edid; > + uint8_t video_pattern; > + uint16_t hdisplay, vdisplay; > + uint8_t bpc; > }; > > struct intel_dp_compliance { > -- > 1.9.1 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel