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

Reply via email to