All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
@ 2017-06-29  0:14 Manasi Navare
  2017-06-29  0:15 ` Clint Taylor
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Manasi Navare @ 2017-06-29  0:14 UTC (permalink / raw)
  To: intel-gfx

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@linux.intel.com>
Cc: Cinton Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@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 */
+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 },
 };
 
 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
+
 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.
+	 */
+	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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
  2017-06-29  0:14 [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts Manasi Navare
@ 2017-06-29  0:15 ` Clint Taylor
  2017-06-29  0:25 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-06-29 20:24 ` [PATCH v2] " Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Clint Taylor @ 2017-06-29  0:15 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx


Looks Good.
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>

-Clint


On 06/28/2017 05:14 PM, 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@linux.intel.com>
> Cc: Cinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@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 */
> +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 },
>   };
>   
>   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
> +
>   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.
> +	 */
> +	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,

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
  2017-06-29  0:14 [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts Manasi Navare
  2017-06-29  0:15 ` Clint Taylor
@ 2017-06-29  0:25 ` Patchwork
  2017-06-29 20:24 ` [PATCH v2] " Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-06-29  0:25 UTC (permalink / raw)
  To: Navare, Manasi D; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
URL   : https://patchwork.freedesktop.org/series/26518/
State : success

== Summary ==

Series 26518v1 drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
https://patchwork.freedesktop.org/api/1.0/series/26518/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_busy:
        Subgroup basic-flip-default-a:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101144 +3
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-skl-6700hq) fdo#101154 +24
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-6700hq)
        Subgroup basic-flip-vs-wf_vblank:
                skip       -> PASS       (fi-skl-6700hq)
        Subgroup basic-plain-flip:
                skip       -> PASS       (fi-skl-6700hq)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                skip       -> PASS       (fi-skl-6700hq)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597
        Subgroup suspend-read-crc-pipe-b:
                fail       -> PASS       (fi-skl-6700hq) fdo#100461 +1
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                warn       -> PASS       (fi-skl-6700hq) fdo#101518
Test kms_sink_crc_basic:
                fail       -> PASS       (fi-skl-6700hq) fdo#101519
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-skl-6700hq)
        Subgroup basic-rte:
                skip       -> PASS       (fi-skl-6700hq)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#101154 https://bugs.freedesktop.org/show_bug.cgi?id=101154
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#100461 https://bugs.freedesktop.org/show_bug.cgi?id=100461
fdo#101518 https://bugs.freedesktop.org/show_bug.cgi?id=101518
fdo#101519 https://bugs.freedesktop.org/show_bug.cgi?id=101519

fi-bdw-gvtdvm    total:279  pass:257  dwarn:8   dfail:0   fail:0   skip:14  time:427s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:353s
fi-bsw-n3050     total:279  pass:242  dwarn:1   dfail:0   fail:0   skip:36  time:529s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:515s
fi-byt-j1900     total:279  pass:253  dwarn:2   dfail:0   fail:0   skip:24  time:488s
fi-byt-n2820     total:279  pass:249  dwarn:2   dfail:0   fail:0   skip:28  time:487s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:465s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:573s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:576s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:549s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:609s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:468s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:481s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:431s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:545s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:409s
fi-bdw-5557u failed to collect. IGT log at Patchwork_5065/fi-bdw-5557u/igt.log

85a692e2c6a7cf93082044d776e838cb9e9b2146 drm-tip: 2017y-06m-28d-14h-24m-59s UTC integration manifest
b9c2fef drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5065/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
  2017-06-29  0:14 [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts Manasi Navare
  2017-06-29  0:15 ` Clint Taylor
  2017-06-29  0:25 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-06-29 20:24 ` Ville Syrjälä
  2017-06-29 20:41   ` Manasi Navare
  2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2017-06-29 20:24 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

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@linux.intel.com>
> Cc: Cinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
  2017-06-29 20:24 ` [PATCH v2] " Ville Syrjälä
@ 2017-06-29 20:41   ` Manasi Navare
  2017-06-30 10:23     ` Ville Syrjälä
  0 siblings, 1 reply; 6+ messages in thread
From: Manasi Navare @ 2017-06-29 20:41 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Thanks for the review comments. Please find my responses below:

On Thu, Jun 29, 2017 at 11:24:48PM +0300, Ville Syrjälä wrote:
> 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@linux.intel.com>
> > Cc: Cinton Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@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.
> 

Agree, I will change it to 800ms in the comment.


> > +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.
>

Yea this quirk should then get applied only to this device.
Do you have any other Toshiba laptop with 1179:f840 combo
to make sure it doesnt get applied on that?

 
> >  };
> >  
> >  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.
> 

Sounds good, will use 800*10

> > +
> >  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."
>

Ok will do

Manasi
 
> > +	 */
> > +	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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts
  2017-06-29 20:41   ` Manasi Navare
@ 2017-06-30 10:23     ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2017-06-30 10:23 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Jun 29, 2017 at 01:41:16PM -0700, Manasi Navare wrote:
> Thanks for the review comments. Please find my responses below:
> 
> On Thu, Jun 29, 2017 at 11:24:48PM +0300, Ville Syrjälä wrote:
> > 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@linux.intel.com>
> > > Cc: Cinton Taylor <clinton.a.taylor@intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@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.
> > 
> 
> Agree, I will change it to 800ms in the comment.
> 
> 
> > > +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.
> >
> 
> Yea this quirk should then get applied only to this device.
> Do you have any other Toshiba laptop with 1179:f840 combo
> to make sure it doesnt get applied on that?

No, but google shows many.

> 
>  
> > >  };
> > >  
> > >  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.
> > 
> 
> Sounds good, will use 800*10
> 
> > > +
> > >  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."
> >
> 
> Ok will do
> 
> Manasi
>  
> > > +	 */
> > > +	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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-30 10:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29  0:14 [PATCH v2] drm/i915/edp: Add a T12 panel delay quirk to fix DP AUX CH timeouts Manasi Navare
2017-06-29  0:15 ` Clint Taylor
2017-06-29  0:25 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-06-29 20:24 ` [PATCH v2] " Ville Syrjälä
2017-06-29 20:41   ` Manasi Navare
2017-06-30 10:23     ` Ville Syrjälä

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.