| Message ID | 1498840428-23176-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 30, 2017 at 09:33:48AM -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 set to 800ms which is 300msecs more than > the minimum value specified in the eDP spec. So a quirk is > implemented for that specific PCI device. > > v4: > * Add Bugzilla links for FDO bugs in the commit message (Ville, Jani) > v3: > * Change some comments, specify the delay as 800 * 10 (Ville) > v2: > * Change the function and variable names to from PPS_T12_ > to _T12 since it is a T12 delay (Clint) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101154 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101167 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101515 > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com> > Acked-by: Ville Syrjala <ville.syrjala@linux.intel.com> I don't recall ever stating that explicitly. Please don't invent acks/rbs. I also fixed up a checkpatch warning about an empty line being present where none was supposed to be. Please review all checkpatch warnings when submitting patches. Anyways this is now pushed to dinq. Thanks for the patch. > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++ > 3 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index effbe4f..4bef5d3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1167,6 +1167,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..87dfde9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14765,6 +14765,17 @@ static void quirk_backlight_present(struct drm_device *dev) > DRM_INFO("applying backlight present quirk\n"); > } > > +/* Toshiba Satellite P50-C-18C requires T12 delay to be min 800ms > + * which is 300 ms greater than eDP spec T12 min. > + */ > +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 +14859,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..538950c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5230,6 +5230,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > intel_pps_dump_state("cur", &cur); > > vbt = dev_priv->vbt.edp.pps; > + /* On Toshiba Satellite P50-C-18C system the VBT T12 delay > + * of 500ms appears to be too short. Ocassionally the panel > + * just fails to power back on. Increasing the delay to 800ms > + * seems sufficient to avoid this problem. > + */ > + if (dev_priv->quirks & QUIRK_INCREASE_T12_DELAY) { > + > + vbt.t11_t12 = max_t(u16, vbt.t11_t12, 800 * 10); > + 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
On Fri, Jun 30, 2017 at 09:33:48AM -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 set to 800ms which is 300msecs more than the minimum value > specified in the eDP spec. So a quirk is implemented for that specific > PCI device. > > v4: > * Add Bugzilla links for FDO bugs in the commit message (Ville, Jani) > v3: > * Change some comments, specify the delay as 800 * 10 (Ville) > v2: > * Change the function and variable names to from PPS_T12_ to _T12 > since it is a T12 delay (Clint) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101154 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101167 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101515 > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com> > Acked-by: Ville Syrjala <ville.syrjala@linux.intel.com> I don't recall ever stating that explicitly. Please don't invent acks/rbs. You had said "Looks good to me" on the previous version. Doesn't that mean its Acked By you? I also fixed up a checkpatch warning about an empty line being present where none was supposed to be. Please review all checkpatch warnings when submitting patches. Thanks for fixing this, sorry must have missed this while respinning. Else I always have been checking for checkpatch Warnings. Manasi Anyways this is now pushed to dinq. Thanks for the patch. > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++ > 3 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h index effbe4f..4bef5d3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1167,6 +1167,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..87dfde9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14765,6 +14765,17 @@ static void quirk_backlight_present(struct drm_device *dev) > DRM_INFO("applying backlight present quirk\n"); } > > +/* Toshiba Satellite P50-C-18C requires T12 delay to be min 800ms > + * which is 300 ms greater than eDP spec T12 min. > + */ > +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 +14859,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..538950c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5230,6 +5230,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > intel_pps_dump_state("cur", &cur); > > vbt = dev_priv->vbt.edp.pps; > + /* On Toshiba Satellite P50-C-18C system the VBT T12 delay > + * of 500ms appears to be too short. Ocassionally the panel > + * just fails to power back on. Increasing the delay to 800ms > + * seems sufficient to avoid this problem. > + */ > + if (dev_priv->quirks & QUIRK_INCREASE_T12_DELAY) { > + > + vbt.t11_t12 = max_t(u16, vbt.t11_t12, 800 * 10); > + 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
On Wed, Jul 05, 2017 at 06:25:30PM +0000, Navare, Manasi D wrote: > On Fri, Jun 30, 2017 at 09:33:48AM -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 set to 800ms which is 300msecs more than the minimum value > > specified in the eDP spec. So a quirk is implemented for that specific > > PCI device. > > > > v4: > > * Add Bugzilla links for FDO bugs in the commit message (Ville, Jani) > > v3: > > * Change some comments, specify the delay as 800 * 10 (Ville) > > v2: > > * Change the function and variable names to from PPS_T12_ to _T12 > > since it is a T12 delay (Clint) > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101144 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101154 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101167 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101515 > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com> > > Acked-by: Ville Syrjala <ville.syrjala@linux.intel.com> > > I don't recall ever stating that explicitly. Please don't invent acks/rbs. > > You had said "Looks good to me" on the previous version. Doesn't that mean its Acked By you? I don't generally expect acks/rbs to appear on a patch unless explicitly stated. I guess sometimes it might be an irc 'ack' or something like that, but that's as far as I would go in interpreting things. > > > I also fixed up a checkpatch warning about an empty line being present where none was supposed to be. Please review all checkpatch warnings when submitting patches. > > Thanks for fixing this, sorry must have missed this while respinning. Else I always have been checking for checkpatch > Warnings. > > Manasi > > Anyways this is now pushed to dinq. Thanks for the patch. > > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++ > > drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++ > > 3 files changed, 26 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index effbe4f..4bef5d3 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1167,6 +1167,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..87dfde9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -14765,6 +14765,17 @@ static void quirk_backlight_present(struct drm_device *dev) > > DRM_INFO("applying backlight present quirk\n"); } > > > > +/* Toshiba Satellite P50-C-18C requires T12 delay to be min 800ms > > + * which is 300 ms greater than eDP spec T12 min. > > + */ > > +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 +14859,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..538950c 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -5230,6 +5230,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > intel_pps_dump_state("cur", &cur); > > > > vbt = dev_priv->vbt.edp.pps; > > + /* On Toshiba Satellite P50-C-18C system the VBT T12 delay > > + * of 500ms appears to be too short. Ocassionally the panel > > + * just fails to power back on. Increasing the delay to 800ms > > + * seems sufficient to avoid this problem. > > + */ > > + if (dev_priv->quirks & QUIRK_INCREASE_T12_DELAY) { > > + > > + vbt.t11_t12 = max_t(u16, vbt.t11_t12, 800 * 10); > > + 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
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index effbe4f..4bef5d3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1167,6 +1167,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..87dfde9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14765,6 +14765,17 @@ static void quirk_backlight_present(struct drm_device *dev) DRM_INFO("applying backlight present quirk\n"); } +/* Toshiba Satellite P50-C-18C requires T12 delay to be min 800ms + * which is 300 ms greater than eDP spec T12 min. + */ +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 +14859,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..538950c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5230,6 +5230,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, intel_pps_dump_state("cur", &cur); vbt = dev_priv->vbt.edp.pps; + /* On Toshiba Satellite P50-C-18C system the VBT T12 delay + * of 500ms appears to be too short. Ocassionally the panel + * just fails to power back on. Increasing the delay to 800ms + * seems sufficient to avoid this problem. + */ + if (dev_priv->quirks & QUIRK_INCREASE_T12_DELAY) { + + vbt.t11_t12 = max_t(u16, vbt.t11_t12, 800 * 10); + 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,