* [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.