[Intel-gfx] [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts

[Ville Syrjälä]

On Wed, Jun 28, 2017 at 05:14:31PM -0700, Manasi Navare wrote:
> This patch fixes the DP AUX CH timeouts observed during CI IGT
> tests thus fixing the CI failures. This is done by adding a
> quirk for a particular PCI device that requires the panel power
> cycle delay (T12) to be 300msecs more than the minimum value
> specified in the eDP spec. So a quirk is implemented for that
> specific PCI device.
> 
> v2:
> * Change the function and variable names to from PPS_T12_
> to _T12 since it is a T12 delay (Clint)
> 
> Fixes: FDO #101144 #101515 #101154 #101167
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: Cinton Taylor <clinton.a.taylor at intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      | 12 ++++++++++++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 427d10c..4327c8a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1168,6 +1168,7 @@ enum intel_sbi_destination {
>  #define QUIRK_INVERT_BRIGHTNESS (1<<2)
>  #define QUIRK_BACKLIGHT_PRESENT (1<<3)
>  #define QUIRK_PIN_SWIZZLED_PAGES (1<<5)
> +#define QUIRK_INCREASE_T12_DELAY (1<<6)
>  
>  struct intel_fbdev;
>  struct intel_fbc_work;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4e03ca6..37beb62 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14765,6 +14765,15 @@ static void quirk_backlight_present(struct drm_device *dev)
>  	DRM_INFO("applying backlight present quirk\n");
>  }
>  
> +/* Some systems require 300ms extra PPS T12 delay to be added to VBT value */

The comment disagrees with the code. Code uses 800ms explicitly instead of
+300 ms.

> +static void quirk_increase_t12_delay(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	dev_priv->quirks |= QUIRK_INCREASE_T12_DELAY;
> +	DRM_INFO("Applying T12 delay quirk\n");
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> @@ -14848,6 +14857,9 @@ static struct intel_quirk intel_quirks[] = {
>  
>  	/* Dell Chromebook 11 (2015 version) */
>  	{ 0x0a16, 0x1028, 0x0a35, quirk_backlight_present },
> +
> +	/* Toshiba Satellite P50-C-18C */
> +	{ 0x191B, 0x1179, 0xF840, quirk_increase_t12_delay },

Hmm. Looks like the 1179:f840 combo is present on a lot of Toshiba
models. But we do have the device ID here too so the quirk shouldn't
go totally overboard.

>  };
>  
>  static void intel_init_quirks(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 67bc8a7a..db6953e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -49,6 +49,9 @@
>  #define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  
> +/* PPS T12 Delay Quirk value for eDP */
> +#define T11_T12_800MS		8000

The define seems pointless. Just use the raw number in the code. Also
writing it as 800*10 would be more consistent with the rest of the code.

> +
>  struct dp_link_dpll {
>  	int clock;
>  	struct dpll dpll;
> @@ -5230,6 +5233,15 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	intel_pps_dump_state("cur", &cur);
>  
>  	vbt = dev_priv->vbt.edp.pps;
> +	/* Apply the QUIRK_INCREASE_T12_DELAY quirk for a specific
> +	 * type of PCI device to avoid  DP AUX CH Timeouts.

That comment doesn't seem very helpful. I would put in something like:

"On Toshiba <whatever> the VBT t12 delay of 500ms appears to be too
 short. Occasionally the panel just fails to power back on. Increasing
 the delay to 800ms seems sufficient to avoid the problem."

> +	 */
> +	if (dev_priv->quirks & QUIRK_INCREASE_T12_DELAY) {
> +
> +		vbt.t11_t12 = max_t(u16, vbt.t11_t12, T11_T12_800MS);
> +		DRM_DEBUG_KMS("Increasing T12 panel delay as per the quirk to %d\n",
> +			      vbt.t11_t12);
> +	}
>  	/* T11_T12 delay is special and actually in units of 100ms, but zero
>  	 * based in the hw (so we need to add 100 ms). But the sw vbt
>  	 * table multiplies it with 1000 to make it in units of 100usec,
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC