[PATCH 1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[PATCH 1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Lenovo Thinkpad T431s (ivb) apparently can't handle LP1+ watermarks
being enabled. It underruns severly enough that the screen is
unusable. The latency values and watemarks look as expected.
And sadly updating the BIOS to the latest version (BIOS GHET41WW
(1.26 ) 11/21/2018) did not help.

One glimmer of hope I had was the VBT. It seems to have some sort
of flag for "self refresh = yes/no", but when I looked at a bunch
of VBTs I had lying around most of them had that field set to "no".
So if we used that we'd end up disabling LP1+ on most machines.
That seems a bit harsh since we know LP1+ works just fine on
most machines.

Since I have no better ideas let's just disable LP1+ watermarks
on this particular machine via a quirk.

Cc: stable@vger.kernel.org
Cc: Andrea <andreatclist@gmail.com>
Reported-by: Andrea <andreatclist@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109477
Fixes: a46a7350b1e8 ("drm/i915: Fix ilk+ watermarks when disabling pipes")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c0e43caa5cd..75dceac19950 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3015,6 +3015,29 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv)
 	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
 }
 
+static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
+				 int level)
+{
+	int max_level = ilk_wm_max_level(dev_priv);
+
+	if (dev_priv->wm.pri_latency[level] == 0 &&
+	    dev_priv->wm.spr_latency[level] == 0 &&
+	    dev_priv->wm.cur_latency[level] == 0)
+		return;
+
+	DRM_DEBUG_KMS("LP%d+ watermarks disabled by quirk\n", level);
+
+	for (; level <= max_level; level++) {
+		dev_priv->wm.pri_latency[level] = 0;
+		dev_priv->wm.spr_latency[level] = 0;
+		dev_priv->wm.cur_latency[level] = 0;
+	}
+
+	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
+	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
+	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
+}
+
 static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -3028,23 +3051,18 @@ static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
 	 * interrupts only. To play it safe we disable LP3
 	 * watermarks entirely.
 	 */
-	if (dev_priv->wm.pri_latency[3] == 0 &&
-	    dev_priv->wm.spr_latency[3] == 0 &&
-	    dev_priv->wm.cur_latency[3] == 0)
-		return;
-
-	dev_priv->wm.pri_latency[3] = 0;
-	dev_priv->wm.spr_latency[3] = 0;
-	dev_priv->wm.cur_latency[3] = 0;
+	ilk_wm_disable_quirk(dev_priv, 3);
+}
 
-	DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
-	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
-	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
-	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
+static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
+{
+	ilk_wm_disable_quirk(dev_priv, 1);
 }
 
 static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 {
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+
 	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
 
 	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
@@ -3063,6 +3081,15 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 		snb_wm_latency_quirk(dev_priv);
 		snb_wm_lp3_irq_quirk(dev_priv);
 	}
+
+	/*
+	 * Lenovo Thinkpad T431s (ivb)
+	 * Massive underruns.
+	 */
+	if (pdev->device == 0x0166 &&
+	    pdev->subsystem_vendor == 0x17aa &&
+	    pdev->subsystem_device == 0x2208)
+		ilk_wm_disable_lp1_quirk(dev_priv);
 }
 
 static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
-- 
2.19.2

[PATCH 1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Lenovo Thinkpad T431s (ivb) apparently can't handle LP1+ watermarks
being enabled. It underruns severly enough that the screen is
unusable. The latency values and watemarks look as expected.
And sadly updating the BIOS to the latest version (BIOS GHET41WW
(1.26 ) 11/21/2018) did not help.

One glimmer of hope I had was the VBT. It seems to have some sort
of flag for "self refresh = yes/no", but when I looked at a bunch
of VBTs I had lying around most of them had that field set to "no".
So if we used that we'd end up disabling LP1+ on most machines.
That seems a bit harsh since we know LP1+ works just fine on
most machines.

Since I have no better ideas let's just disable LP1+ watermarks
on this particular machine via a quirk.

Cc: stable@vger.kernel.org
Cc: Andrea <andreatclist@gmail.com>
Reported-by: Andrea <andreatclist@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109477
Fixes: a46a7350b1e8 ("drm/i915: Fix ilk+ watermarks when disabling pipes")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c0e43caa5cd..75dceac19950 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3015,6 +3015,29 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv)
 	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
 }
 
+static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
+				 int level)
+{
+	int max_level = ilk_wm_max_level(dev_priv);
+
+	if (dev_priv->wm.pri_latency[level] == 0 &&
+	    dev_priv->wm.spr_latency[level] == 0 &&
+	    dev_priv->wm.cur_latency[level] == 0)
+		return;
+
+	DRM_DEBUG_KMS("LP%d+ watermarks disabled by quirk\n", level);
+
+	for (; level <= max_level; level++) {
+		dev_priv->wm.pri_latency[level] = 0;
+		dev_priv->wm.spr_latency[level] = 0;
+		dev_priv->wm.cur_latency[level] = 0;
+	}
+
+	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
+	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
+	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
+}
+
 static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -3028,23 +3051,18 @@ static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
 	 * interrupts only. To play it safe we disable LP3
 	 * watermarks entirely.
 	 */
-	if (dev_priv->wm.pri_latency[3] == 0 &&
-	    dev_priv->wm.spr_latency[3] == 0 &&
-	    dev_priv->wm.cur_latency[3] == 0)
-		return;
-
-	dev_priv->wm.pri_latency[3] = 0;
-	dev_priv->wm.spr_latency[3] = 0;
-	dev_priv->wm.cur_latency[3] = 0;
+	ilk_wm_disable_quirk(dev_priv, 3);
+}
 
-	DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
-	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
-	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
-	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
+static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
+{
+	ilk_wm_disable_quirk(dev_priv, 1);
 }
 
 static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 {
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+
 	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
 
 	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
@@ -3063,6 +3081,15 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 		snb_wm_latency_quirk(dev_priv);
 		snb_wm_lp3_irq_quirk(dev_priv);
 	}
+
+	/*
+	 * Lenovo Thinkpad T431s (ivb)
+	 * Massive underruns.
+	 */
+	if (pdev->device == 0x0166 &&
+	    pdev->subsystem_vendor == 0x17aa &&
+	    pdev->subsystem_device == 0x2208)
+		ilk_wm_disable_lp1_quirk(dev_priv);
 }
 
 static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
-- 
2.19.2

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

[PATCH 2/4] drm/i915: Generalize pci quirks

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add support for multiple independent pci quirk tables.
I want to reuse the quirk table approach for some
watermark quirks but I prefer to keep the details in
one place rather than spreading them all over.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  9 +++++++++
 drivers/gpu/drm/i915/intel_quirks.c | 28 +++++++++++++++-------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 81ec73e4a083..8924b43d6e99 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2187,7 +2187,16 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
 bool intel_psr_enabled(struct intel_dp *intel_dp);
 
 /* intel_quirks.c */
+struct intel_pci_quirk {
+	int device;
+	int subsystem_vendor;
+	int subsystem_device;
+	void (*hook)(struct drm_i915_private *i915);
+};
+
 void intel_init_quirks(struct drm_i915_private *dev_priv);
+void intel_apply_pci_quirks(struct drm_i915_private *i915,
+			    const struct intel_pci_quirk *quirks);
 
 /* intel_runtime_pm.c */
 void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_quirks.c b/drivers/gpu/drm/i915/intel_quirks.c
index ec2b0fc92b8b..e073510553a6 100644
--- a/drivers/gpu/drm/i915/intel_quirks.c
+++ b/drivers/gpu/drm/i915/intel_quirks.c
@@ -52,13 +52,6 @@ static void quirk_increase_ddi_disabled_time(struct drm_i915_private *i915)
 	DRM_INFO("Applying Increase DDI Disabled quirk\n");
 }
 
-struct intel_quirk {
-	int device;
-	int subsystem_vendor;
-	int subsystem_device;
-	void (*hook)(struct drm_i915_private *i915);
-};
-
 /* For systems that don't have a meaningful PCI subdevice/subvendor ID */
 struct intel_dmi_quirk {
 	void (*hook)(struct drm_i915_private *i915);
@@ -87,7 +80,7 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
 	},
 };
 
-static struct intel_quirk intel_quirks[] = {
+static const struct intel_pci_quirk intel_pci_quirks[] = {
 	/* Lenovo U160 cannot use SSC on LVDS */
 	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
 
@@ -145,16 +138,17 @@ static struct intel_quirk intel_quirks[] = {
 	/* ASRock ITX*/
 	{ 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
 	{ 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
+
+	{}
 };
 
-void intel_init_quirks(struct drm_i915_private *i915)
+void intel_apply_pci_quirks(struct drm_i915_private *i915,
+			    const struct intel_pci_quirk *quirks)
 {
 	struct pci_dev *d = i915->drm.pdev;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(intel_quirks); i++) {
-		struct intel_quirk *q = &intel_quirks[i];
+	const struct intel_pci_quirk *q;
 
+	for (q = quirks; q->device; q++) {
 		if (d->device == q->device &&
 		    (d->subsystem_vendor == q->subsystem_vendor ||
 		     q->subsystem_vendor == PCI_ANY_ID) &&
@@ -162,6 +156,14 @@ void intel_init_quirks(struct drm_i915_private *i915)
 		     q->subsystem_device == PCI_ANY_ID))
 			q->hook(i915);
 	}
+}
+
+void intel_init_quirks(struct drm_i915_private *i915)
+{
+	int i;
+
+	intel_apply_pci_quirks(i915, intel_pci_quirks);
+
 	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
 		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
 			intel_dmi_quirks[i].hook(i915);
-- 
2.19.2

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

Re: [PATCH 2/4] drm/i915: Generalize pci quirks

[Jani Nikula]

On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add support for multiple independent pci quirk tables.
> I want to reuse the quirk table approach for some
> watermark quirks but I prefer to keep the details in
> one place rather than spreading them all over.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  9 +++++++++
>  drivers/gpu/drm/i915/intel_quirks.c | 28 +++++++++++++++-------------
>  2 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 81ec73e4a083..8924b43d6e99 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2187,7 +2187,16 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
>  bool intel_psr_enabled(struct intel_dp *intel_dp);
>  
>  /* intel_quirks.c */
> +struct intel_pci_quirk {
> +	int device;
> +	int subsystem_vendor;
> +	int subsystem_device;
> +	void (*hook)(struct drm_i915_private *i915);
> +};
> +
>  void intel_init_quirks(struct drm_i915_private *dev_priv);
> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
> +			    const struct intel_pci_quirk *quirks);
>  
>  /* intel_runtime_pm.c */
>  void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_quirks.c b/drivers/gpu/drm/i915/intel_quirks.c
> index ec2b0fc92b8b..e073510553a6 100644
> --- a/drivers/gpu/drm/i915/intel_quirks.c
> +++ b/drivers/gpu/drm/i915/intel_quirks.c
> @@ -52,13 +52,6 @@ static void quirk_increase_ddi_disabled_time(struct drm_i915_private *i915)
>  	DRM_INFO("Applying Increase DDI Disabled quirk\n");
>  }
>  
> -struct intel_quirk {
> -	int device;
> -	int subsystem_vendor;
> -	int subsystem_device;
> -	void (*hook)(struct drm_i915_private *i915);
> -};
> -
>  /* For systems that don't have a meaningful PCI subdevice/subvendor ID */
>  struct intel_dmi_quirk {
>  	void (*hook)(struct drm_i915_private *i915);
> @@ -87,7 +80,7 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>  	},
>  };
>  
> -static struct intel_quirk intel_quirks[] = {
> +static const struct intel_pci_quirk intel_pci_quirks[] = {
>  	/* Lenovo U160 cannot use SSC on LVDS */
>  	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>  
> @@ -145,16 +138,17 @@ static struct intel_quirk intel_quirks[] = {
>  	/* ASRock ITX*/
>  	{ 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
>  	{ 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
> +
> +	{}
>  };
>  
> -void intel_init_quirks(struct drm_i915_private *i915)
> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
> +			    const struct intel_pci_quirk *quirks)
>  {
>  	struct pci_dev *d = i915->drm.pdev;
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(intel_quirks); i++) {
> -		struct intel_quirk *q = &intel_quirks[i];
> +	const struct intel_pci_quirk *q;
>  
> +	for (q = quirks; q->device; q++) {
>  		if (d->device == q->device &&
>  		    (d->subsystem_vendor == q->subsystem_vendor ||
>  		     q->subsystem_vendor == PCI_ANY_ID) &&
> @@ -162,6 +156,14 @@ void intel_init_quirks(struct drm_i915_private *i915)
>  		     q->subsystem_device == PCI_ANY_ID))
>  			q->hook(i915);
>  	}
> +}
> +
> +void intel_init_quirks(struct drm_i915_private *i915)
> +{
> +	int i;
> +
> +	intel_apply_pci_quirks(i915, intel_pci_quirks);
> +
>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
>  			intel_dmi_quirks[i].hook(i915);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Generalize pci quirks

[Jani Nikula]

On Mon, 25 Feb 2019, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Add support for multiple independent pci quirk tables.
>> I want to reuse the quirk table approach for some
>> watermark quirks but I prefer to keep the details in
>> one place rather than spreading them all over.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Hmm. Not to block the series, but this got me thinking. See
e.g. QUIRK_BACKLIGHT_PRESENT or QUIRK_INVERT_BRIGHTNESS. Should we
localize those to their own quirk tables in intel_panel.c now?

What if intel_apply_pci_quirks() returned an OR of the return values of
the hooks. In a few cases we could nuke the use of dev_priv->quirks in
favor of more localized quirks. The former backlight quirk need not be
saved at all, it's a one time check, and the latter could be saved in
the backlight struct.

Something for the future, perhaps.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/intel_drv.h    |  9 +++++++++
>>  drivers/gpu/drm/i915/intel_quirks.c | 28 +++++++++++++++-------------
>>  2 files changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 81ec73e4a083..8924b43d6e99 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -2187,7 +2187,16 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
>>  bool intel_psr_enabled(struct intel_dp *intel_dp);
>>  
>>  /* intel_quirks.c */
>> +struct intel_pci_quirk {
>> +	int device;
>> +	int subsystem_vendor;
>> +	int subsystem_device;
>> +	void (*hook)(struct drm_i915_private *i915);
>> +};
>> +
>>  void intel_init_quirks(struct drm_i915_private *dev_priv);
>> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
>> +			    const struct intel_pci_quirk *quirks);
>>  
>>  /* intel_runtime_pm.c */
>>  void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/intel_quirks.c b/drivers/gpu/drm/i915/intel_quirks.c
>> index ec2b0fc92b8b..e073510553a6 100644
>> --- a/drivers/gpu/drm/i915/intel_quirks.c
>> +++ b/drivers/gpu/drm/i915/intel_quirks.c
>> @@ -52,13 +52,6 @@ static void quirk_increase_ddi_disabled_time(struct drm_i915_private *i915)
>>  	DRM_INFO("Applying Increase DDI Disabled quirk\n");
>>  }
>>  
>> -struct intel_quirk {
>> -	int device;
>> -	int subsystem_vendor;
>> -	int subsystem_device;
>> -	void (*hook)(struct drm_i915_private *i915);
>> -};
>> -
>>  /* For systems that don't have a meaningful PCI subdevice/subvendor ID */
>>  struct intel_dmi_quirk {
>>  	void (*hook)(struct drm_i915_private *i915);
>> @@ -87,7 +80,7 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
>>  	},
>>  };
>>  
>> -static struct intel_quirk intel_quirks[] = {
>> +static const struct intel_pci_quirk intel_pci_quirks[] = {
>>  	/* Lenovo U160 cannot use SSC on LVDS */
>>  	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
>>  
>> @@ -145,16 +138,17 @@ static struct intel_quirk intel_quirks[] = {
>>  	/* ASRock ITX*/
>>  	{ 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
>>  	{ 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
>> +
>> +	{}
>>  };
>>  
>> -void intel_init_quirks(struct drm_i915_private *i915)
>> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
>> +			    const struct intel_pci_quirk *quirks)
>>  {
>>  	struct pci_dev *d = i915->drm.pdev;
>> -	int i;
>> -
>> -	for (i = 0; i < ARRAY_SIZE(intel_quirks); i++) {
>> -		struct intel_quirk *q = &intel_quirks[i];
>> +	const struct intel_pci_quirk *q;
>>  
>> +	for (q = quirks; q->device; q++) {
>>  		if (d->device == q->device &&
>>  		    (d->subsystem_vendor == q->subsystem_vendor ||
>>  		     q->subsystem_vendor == PCI_ANY_ID) &&
>> @@ -162,6 +156,14 @@ void intel_init_quirks(struct drm_i915_private *i915)
>>  		     q->subsystem_device == PCI_ANY_ID))
>>  			q->hook(i915);
>>  	}
>> +}
>> +
>> +void intel_init_quirks(struct drm_i915_private *i915)
>> +{
>> +	int i;
>> +
>> +	intel_apply_pci_quirks(i915, intel_pci_quirks);
>> +
>>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
>>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
>>  			intel_dmi_quirks[i].hook(i915);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Generalize pci quirks

[Ville Syrjälä]

On Mon, Feb 25, 2019 at 03:28:18PM +0200, Jani Nikula wrote:
> On Mon, 25 Feb 2019, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Add support for multiple independent pci quirk tables.
> >> I want to reuse the quirk table approach for some
> >> watermark quirks but I prefer to keep the details in
> >> one place rather than spreading them all over.
> >>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> Hmm. Not to block the series, but this got me thinking. See
> e.g. QUIRK_BACKLIGHT_PRESENT or QUIRK_INVERT_BRIGHTNESS. Should we
> localize those to their own quirk tables in intel_panel.c now?

Might be nice.

However we seem to have a sole DMI quirk for some backlight
stuff. That would need to be handled somehow as well.

> 
> What if intel_apply_pci_quirks() returned an OR of the return values of
> the hooks. In a few cases we could nuke the use of dev_priv->quirks in
> favor of more localized quirks. The former backlight quirk need not be
> saved at all, it's a one time check, and the latter could be saved in
> the backlight struct.
> 
> Something for the future, perhaps.
> 
> BR,
> Jani.
> 
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_drv.h    |  9 +++++++++
> >>  drivers/gpu/drm/i915/intel_quirks.c | 28 +++++++++++++++-------------
> >>  2 files changed, 24 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index 81ec73e4a083..8924b43d6e99 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -2187,7 +2187,16 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state,
> >>  bool intel_psr_enabled(struct intel_dp *intel_dp);
> >>  
> >>  /* intel_quirks.c */
> >> +struct intel_pci_quirk {
> >> +	int device;
> >> +	int subsystem_vendor;
> >> +	int subsystem_device;
> >> +	void (*hook)(struct drm_i915_private *i915);
> >> +};
> >> +
> >>  void intel_init_quirks(struct drm_i915_private *dev_priv);
> >> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
> >> +			    const struct intel_pci_quirk *quirks);
> >>  
> >>  /* intel_runtime_pm.c */
> >>  void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
> >> diff --git a/drivers/gpu/drm/i915/intel_quirks.c b/drivers/gpu/drm/i915/intel_quirks.c
> >> index ec2b0fc92b8b..e073510553a6 100644
> >> --- a/drivers/gpu/drm/i915/intel_quirks.c
> >> +++ b/drivers/gpu/drm/i915/intel_quirks.c
> >> @@ -52,13 +52,6 @@ static void quirk_increase_ddi_disabled_time(struct drm_i915_private *i915)
> >>  	DRM_INFO("Applying Increase DDI Disabled quirk\n");
> >>  }
> >>  
> >> -struct intel_quirk {
> >> -	int device;
> >> -	int subsystem_vendor;
> >> -	int subsystem_device;
> >> -	void (*hook)(struct drm_i915_private *i915);
> >> -};
> >> -
> >>  /* For systems that don't have a meaningful PCI subdevice/subvendor ID */
> >>  struct intel_dmi_quirk {
> >>  	void (*hook)(struct drm_i915_private *i915);
> >> @@ -87,7 +80,7 @@ static const struct intel_dmi_quirk intel_dmi_quirks[] = {
> >>  	},
> >>  };
> >>  
> >> -static struct intel_quirk intel_quirks[] = {
> >> +static const struct intel_pci_quirk intel_pci_quirks[] = {
> >>  	/* Lenovo U160 cannot use SSC on LVDS */
> >>  	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
> >>  
> >> @@ -145,16 +138,17 @@ static struct intel_quirk intel_quirks[] = {
> >>  	/* ASRock ITX*/
> >>  	{ 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
> >>  	{ 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time },
> >> +
> >> +	{}
> >>  };
> >>  
> >> -void intel_init_quirks(struct drm_i915_private *i915)
> >> +void intel_apply_pci_quirks(struct drm_i915_private *i915,
> >> +			    const struct intel_pci_quirk *quirks)
> >>  {
> >>  	struct pci_dev *d = i915->drm.pdev;
> >> -	int i;
> >> -
> >> -	for (i = 0; i < ARRAY_SIZE(intel_quirks); i++) {
> >> -		struct intel_quirk *q = &intel_quirks[i];
> >> +	const struct intel_pci_quirk *q;
> >>  
> >> +	for (q = quirks; q->device; q++) {
> >>  		if (d->device == q->device &&
> >>  		    (d->subsystem_vendor == q->subsystem_vendor ||
> >>  		     q->subsystem_vendor == PCI_ANY_ID) &&
> >> @@ -162,6 +156,14 @@ void intel_init_quirks(struct drm_i915_private *i915)
> >>  		     q->subsystem_device == PCI_ANY_ID))
> >>  			q->hook(i915);
> >>  	}
> >> +}
> >> +
> >> +void intel_init_quirks(struct drm_i915_private *i915)
> >> +{
> >> +	int i;
> >> +
> >> +	intel_apply_pci_quirks(i915, intel_pci_quirks);
> >> +
> >>  	for (i = 0; i < ARRAY_SIZE(intel_dmi_quirks); i++) {
> >>  		if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0)
> >>  			intel_dmi_quirks[i].hook(i915);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

[PATCH 3/4] drm/i915: Use intel_apply_pci_quirks() to apply ILK+ wm quirks

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the newly introduced intel_apply_pci_quirks() to clean up
the way we apply the ilk+ watermark quirks.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 75dceac19950..2a4b5014f56e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3059,10 +3059,17 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
 	ilk_wm_disable_quirk(dev_priv, 1);
 }
 
+static const struct intel_pci_quirk ilk_wm_quirks[] = {
+	/*
+	 * Lenovo Thinkpad T431s (ivb)
+	 * Massive underruns with LP1+.
+	 */
+	{ 0x0166, 0x17aa, 0x2208, ilk_wm_disable_lp1_quirk },
+	{}
+};
+
 static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 {
-	struct pci_dev *pdev = dev_priv->drm.pdev;
-
 	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
 
 	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
@@ -3082,14 +3089,7 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 		snb_wm_lp3_irq_quirk(dev_priv);
 	}
 
-	/*
-	 * Lenovo Thinkpad T431s (ivb)
-	 * Massive underruns.
-	 */
-	if (pdev->device == 0x0166 &&
-	    pdev->subsystem_vendor == 0x17aa &&
-	    pdev->subsystem_device == 0x2208)
-		ilk_wm_disable_lp1_quirk(dev_priv);
+	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
 }
 
 static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
-- 
2.19.2

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

Re: [PATCH 3/4] drm/i915: Use intel_apply_pci_quirks() to apply ILK+ wm quirks

[Jani Nikula]

On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Use the newly introduced intel_apply_pci_quirks() to clean up
> the way we apply the ilk+ watermark quirks.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 75dceac19950..2a4b5014f56e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3059,10 +3059,17 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
>  	ilk_wm_disable_quirk(dev_priv, 1);
>  }
>  
> +static const struct intel_pci_quirk ilk_wm_quirks[] = {

Mmmh, ilk prefix, do you propose to add separate quirk arrays for
different platforms...? *shrug*

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> +	/*
> +	 * Lenovo Thinkpad T431s (ivb)
> +	 * Massive underruns with LP1+.
> +	 */
> +	{ 0x0166, 0x17aa, 0x2208, ilk_wm_disable_lp1_quirk },
> +	{}
> +};
> +
>  static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  {
> -	struct pci_dev *pdev = dev_priv->drm.pdev;
> -
>  	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
>  
>  	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
> @@ -3082,14 +3089,7 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  		snb_wm_lp3_irq_quirk(dev_priv);
>  	}
>  
> -	/*
> -	 * Lenovo Thinkpad T431s (ivb)
> -	 * Massive underruns.
> -	 */
> -	if (pdev->device == 0x0166 &&
> -	    pdev->subsystem_vendor == 0x17aa &&
> -	    pdev->subsystem_device == 0x2208)
> -		ilk_wm_disable_lp1_quirk(dev_priv);
> +	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
>  }
>  
>  static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Use intel_apply_pci_quirks() to apply ILK+ wm quirks

[Ville Syrjälä]

On Mon, Feb 25, 2019 at 03:14:52PM +0200, Jani Nikula wrote:
> On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Use the newly introduced intel_apply_pci_quirks() to clean up
> > the way we apply the ilk+ watermark quirks.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 75dceac19950..2a4b5014f56e 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3059,10 +3059,17 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
> >  	ilk_wm_disable_quirk(dev_priv, 1);
> >  }
> >  
> > +static const struct intel_pci_quirk ilk_wm_quirks[] = {
> 
> Mmmh, ilk prefix, do you propose to add separate quirk arrays for
> different platforms...? *shrug*

Perhaps. So far we just need this on the ilk-bdw codepaths
hence I just went with the ilk_ prefix.

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> > +	/*
> > +	 * Lenovo Thinkpad T431s (ivb)
> > +	 * Massive underruns with LP1+.
> > +	 */
> > +	{ 0x0166, 0x17aa, 0x2208, ilk_wm_disable_lp1_quirk },
> > +	{}
> > +};
> > +
> >  static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
> >  {
> > -	struct pci_dev *pdev = dev_priv->drm.pdev;
> > -
> >  	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
> >  
> >  	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
> > @@ -3082,14 +3089,7 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
> >  		snb_wm_lp3_irq_quirk(dev_priv);
> >  	}
> >  
> > -	/*
> > -	 * Lenovo Thinkpad T431s (ivb)
> > -	 * Massive underruns.
> > -	 */
> > -	if (pdev->device == 0x0166 &&
> > -	    pdev->subsystem_vendor == 0x17aa &&
> > -	    pdev->subsystem_device == 0x2208)
> > -		ilk_wm_disable_lp1_quirk(dev_priv);
> > +	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
> >  }
> >  
> >  static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

[PATCH 4/4] drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The only machine we know for sure to require the LP3+ disable
is the Lenovo Thinkpad X220 tablet. Originally in commit
03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB
machines") I disabled LP3+ watermarks on all SNB machines,
partially for safety, and partially since I didn't want to
add a quirk. But since we now have another watermark quirk
anyway let's reduce the SNB LP3+ disable scope to the x220
tablet only.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2a4b5014f56e..b225461455c2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3038,7 +3038,7 @@ static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
 	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
 }
 
-static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
+static void ilk_wm_disable_lp3(struct drm_i915_private *dev_priv)
 {
 	/*
 	 * On some SNB machines (Thinkpad X220 Tablet at least)
@@ -3060,6 +3060,11 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
 }
 
 static const struct intel_pci_quirk ilk_wm_quirks[] = {
+	/*
+	 * Lenovo ThinkPad X220 Tablet (snb)
+	 * CPU doesn't wake up for vblank interrupts with LP3.
+	 */
+	{ 0x0126, 0x17aa, 0x21db, ilk_wm_disable_lp3 },
 	/*
 	 * Lenovo Thinkpad T431s (ivb)
 	 * Massive underruns with LP1+.
@@ -3084,10 +3089,8 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
 	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
 	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
 
-	if (IS_GEN(dev_priv, 6)) {
+	if (IS_GEN(dev_priv, 6))
 		snb_wm_latency_quirk(dev_priv);
-		snb_wm_lp3_irq_quirk(dev_priv);
-	}
 
 	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
 }
-- 
2.19.2

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

Re: [PATCH 4/4] drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet

[Jani Nikula]

On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The only machine we know for sure to require the LP3+ disable
> is the Lenovo Thinkpad X220 tablet. Originally in commit
> 03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB
> machines") I disabled LP3+ watermarks on all SNB machines,
> partially for safety, and partially since I didn't want to
> add a quirk. But since we now have another watermark quirk
> anyway let's reduce the SNB LP3+ disable scope to the x220
> tablet only.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2a4b5014f56e..b225461455c2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3038,7 +3038,7 @@ static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  }
>  
> -static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
> +static void ilk_wm_disable_lp3(struct drm_i915_private *dev_priv)
>  {
>  	/*
>  	 * On some SNB machines (Thinkpad X220 Tablet at least)
> @@ -3060,6 +3060,11 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
>  }
>  
>  static const struct intel_pci_quirk ilk_wm_quirks[] = {
> +	/*
> +	 * Lenovo ThinkPad X220 Tablet (snb)
> +	 * CPU doesn't wake up for vblank interrupts with LP3.
> +	 */
> +	{ 0x0126, 0x17aa, 0x21db, ilk_wm_disable_lp3 },

The ilk naming keeps bugging me slightly, but no biggie.

Acked-by: Jani Nikula <jani.nikula@intel.com>


>  	/*
>  	 * Lenovo Thinkpad T431s (ivb)
>  	 * Massive underruns with LP1+.
> @@ -3084,10 +3089,8 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  
> -	if (IS_GEN(dev_priv, 6)) {
> +	if (IS_GEN(dev_priv, 6))
>  		snb_wm_latency_quirk(dev_priv);
> -		snb_wm_lp3_irq_quirk(dev_priv);
> -	}
>  
>  	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet

[Ville Syrjälä]

On Fri, Feb 22, 2019 at 05:52:51PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The only machine we know for sure to require the LP3+ disable
> is the Lenovo Thinkpad X220 tablet. Originally in commit
> 03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB
> machines") I disabled LP3+ watermarks on all SNB machines,
> partially for safety, and partially since I didn't want to
> add a quirk. But since we now have another watermark quirk
> anyway let's reduce the SNB LP3+ disable scope to the x220
> tablet only.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2a4b5014f56e..b225461455c2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3038,7 +3038,7 @@ static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  }
>  
> -static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
> +static void ilk_wm_disable_lp3(struct drm_i915_private *dev_priv)
>  {
>  	/*
>  	 * On some SNB machines (Thinkpad X220 Tablet at least)
> @@ -3060,6 +3060,11 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
>  }
>  
>  static const struct intel_pci_quirk ilk_wm_quirks[] = {
> +	/*
> +	 * Lenovo ThinkPad X220 Tablet (snb)
> +	 * CPU doesn't wake up for vblank interrupts with LP3.
> +	 */
> +	{ 0x0126, 0x17aa, 0x21db, ilk_wm_disable_lp3 },

Hmm. Somehow I had convinced myself that this would also cover the second
bugreport (https://bugs.freedesktop.org/show_bug.cgi?id=101269) we had
about this. But looks like we'd need another quirk entry here for that
machine (a Thinkpad T420). I'll respin with. *hopefully* there aren't
many more.

>  	/*
>  	 * Lenovo Thinkpad T431s (ivb)
>  	 * Massive underruns with LP1+.
> @@ -3084,10 +3089,8 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  
> -	if (IS_GEN(dev_priv, 6)) {
> +	if (IS_GEN(dev_priv, 6))
>  		snb_wm_latency_quirk(dev_priv);
> -		snb_wm_lp3_irq_quirk(dev_priv);
> -	}
>  
>  	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
>  }
> -- 
> 2.19.2

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

Re: [PATCH 4/4] drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet

[Ville Syrjälä]

On Mon, Feb 25, 2019 at 05:45:38PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 22, 2019 at 05:52:51PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The only machine we know for sure to require the LP3+ disable
> > is the Lenovo Thinkpad X220 tablet. Originally in commit
> > 03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB
> > machines") I disabled LP3+ watermarks on all SNB machines,
> > partially for safety, and partially since I didn't want to
> > add a quirk. But since we now have another watermark quirk
> > anyway let's reduce the SNB LP3+ disable scope to the x220
> > tablet only.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 2a4b5014f56e..b225461455c2 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3038,7 +3038,7 @@ static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
> >  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> >  }
> >  
> > -static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
> > +static void ilk_wm_disable_lp3(struct drm_i915_private *dev_priv)
> >  {
> >  	/*
> >  	 * On some SNB machines (Thinkpad X220 Tablet at least)
> > @@ -3060,6 +3060,11 @@ static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
> >  }
> >  
> >  static const struct intel_pci_quirk ilk_wm_quirks[] = {
> > +	/*
> > +	 * Lenovo ThinkPad X220 Tablet (snb)
> > +	 * CPU doesn't wake up for vblank interrupts with LP3.
> > +	 */
> > +	{ 0x0126, 0x17aa, 0x21db, ilk_wm_disable_lp3 },
> 
> Hmm. Somehow I had convinced myself that this would also cover the second
> bugreport (https://bugs.freedesktop.org/show_bug.cgi?id=101269) we had
> about this. But looks like we'd need another quirk entry here for that
> machine (a Thinkpad T420). I'll respin with. *hopefully* there aren't
> many more.

https://bugs.freedesktop.org/show_bug.cgi?id=104573#c5 suggests that a
Lifeboot T901 is also affected. I guess I'll just have to drop my
plans to reduce the scope of this quirk :(

> 
> >  	/*
> >  	 * Lenovo Thinkpad T431s (ivb)
> >  	 * Massive underruns with LP1+.
> > @@ -3084,10 +3089,8 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
> >  	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> >  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> >  
> > -	if (IS_GEN(dev_priv, 6)) {
> > +	if (IS_GEN(dev_priv, 6))
> >  		snb_wm_latency_quirk(dev_priv);
> > -		snb_wm_lp3_irq_quirk(dev_priv);
> > -	}
> >  
> >  	intel_apply_pci_quirks(dev_priv, ilk_wm_quirks);
> >  }
> > -- 
> > 2.19.2
> 
> -- 
> Ville Syrjälä
> Intel

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s
URL   : https://patchwork.freedesktop.org/series/57101/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4545b0056a9b drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s
2d4ab5e9f9be drm/i915: Generalize pci quirks
c3643de46878 drm/i915: Use intel_apply_pci_quirks() to apply ILK+ wm quirks
30c90e017a18 drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB machines")'
#11: 
03981c6ebec4 ("drm/i915: Disable LP3 watermarks on all SNB

total: 1 errors, 0 warnings, 0 checks, 30 lines checked

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

✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s
URL   : https://patchwork.freedesktop.org/series/57101/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5654 -> Patchwork_12284
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57101/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12284 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@memory-alloc:
    - fi-ivb-3520m:       NOTRUN -> SKIP [fdo#109271] +48

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +18

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-allowed:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109316] +2

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-skl-6700k2:      NOTRUN -> SKIP [fdo#109271] +41

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-icl-u2:          NOTRUN -> FAIL [fdo#109635 ]

  * igt@kms_chamelium@vga-hpd-fast:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109309] +1

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          NOTRUN -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_pm_rpm@module-reload:
    - {fi-icl-y}:         INCOMPLETE [fdo#108840] -> PASS

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       DMESG-WARN [fdo#107709] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (43 -> 39)
------------------------------

  Additional (3): fi-ivb-3520m fi-icl-u2 fi-skl-6700k2 
  Missing    (7): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5654 -> Patchwork_12284

  CI_DRM_5654: 30c7f283790b433aa311ef7a7d2b6b428886fb9a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4851: 2b7dd10a4e2ea0cabff68421fd15e96c99be3cad @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12284: 30c90e017a183db13e0854ed76b803021c7d960a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

30c90e017a18 drm/i915: Restrict SNB LP3+ disable to Thinkpad X220 tablet
c3643de46878 drm/i915: Use intel_apply_pci_quirks() to apply ILK+ wm quirks
2d4ab5e9f9be drm/i915: Generalize pci quirks
4545b0056a9b drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12284/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s
URL   : https://patchwork.freedesktop.org/series/57101/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5654_full -> Patchwork_12284_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12284_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_lpsp@edp-panel-fitter:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109301]

  * igt@i915_pm_rpm@cursor-dpms:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108840]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_busy@basic-flip-e:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          PASS -> FAIL [fdo#106510] / [fdo#108145] +1

  * igt@kms_chamelium@vga-hpd-fast:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284]

  * igt@kms_chv_cursor_fail@pipe-c-256x256-bottom-edge:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +4

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-glk:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109593]

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713]

  * igt@kms_flip@2x-flip-vs-absolute-wf_vblank:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +31

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +27

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-onoff:
    - shard-iclb:         PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-fullscreen:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +16

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#106978] / [fdo#107713]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@runner@aborted:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109593]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@fences:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +1

  * igt@i915_pm_rpm@modeset-lpsp-stress:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-iclb:         FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +5

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-iclb:         FAIL [fdo#103166] -> PASS +5

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@pipe-b-query-idle-hang:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> FAIL [fdo#103232]

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109301]: https://bugs.freedesktop.org/show_bug.cgi?id=109301
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

    * Linux: CI_DRM_5654 -> Patchwork_12284

  CI_DRM_5654: 30c7f283790b433aa311ef7a7d2b6b428886fb9a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4851: 2b7dd10a4e2ea0cabff68421fd15e96c99be3cad @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12284: 30c90e017a183db13e0854ed76b803021c7d960a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12284/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Jani Nikula]

On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Lenovo Thinkpad T431s (ivb) apparently can't handle LP1+ watermarks
> being enabled. It underruns severly enough that the screen is
> unusable. The latency values and watemarks look as expected.
> And sadly updating the BIOS to the latest version (BIOS GHET41WW
> (1.26 ) 11/21/2018) did not help.
>
> One glimmer of hope I had was the VBT. It seems to have some sort
> of flag for "self refresh = yes/no", but when I looked at a bunch
> of VBTs I had lying around most of them had that field set to "no".
> So if we used that we'd end up disabling LP1+ on most machines.
> That seems a bit harsh since we know LP1+ works just fine on
> most machines.
>
> Since I have no better ideas let's just disable LP1+ watermarks
> on this particular machine via a quirk.
>
> Cc: stable@vger.kernel.org
> Cc: Andrea <andreatclist@gmail.com>
> Reported-by: Andrea <andreatclist@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109477
> Fixes: a46a7350b1e8 ("drm/i915: Fix ilk+ watermarks when disabling pipes")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Does what it says on the box, though I'm saddened we have to resort to a
quirk.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4c0e43caa5cd..75dceac19950 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3015,6 +3015,29 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv)
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  }
>  
> +static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
> +				 int level)
> +{
> +	int max_level = ilk_wm_max_level(dev_priv);
> +
> +	if (dev_priv->wm.pri_latency[level] == 0 &&
> +	    dev_priv->wm.spr_latency[level] == 0 &&
> +	    dev_priv->wm.cur_latency[level] == 0)
> +		return;
> +
> +	DRM_DEBUG_KMS("LP%d+ watermarks disabled by quirk\n", level);
> +
> +	for (; level <= max_level; level++) {
> +		dev_priv->wm.pri_latency[level] = 0;
> +		dev_priv->wm.spr_latency[level] = 0;
> +		dev_priv->wm.cur_latency[level] = 0;
> +	}
> +
> +	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> +	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> +	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> +}
> +
>  static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -3028,23 +3051,18 @@ static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
>  	 * interrupts only. To play it safe we disable LP3
>  	 * watermarks entirely.
>  	 */
> -	if (dev_priv->wm.pri_latency[3] == 0 &&
> -	    dev_priv->wm.spr_latency[3] == 0 &&
> -	    dev_priv->wm.cur_latency[3] == 0)
> -		return;
> -
> -	dev_priv->wm.pri_latency[3] = 0;
> -	dev_priv->wm.spr_latency[3] = 0;
> -	dev_priv->wm.cur_latency[3] = 0;
> +	ilk_wm_disable_quirk(dev_priv, 3);
> +}
>  
> -	DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
> -	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> -	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> -	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> +static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
> +{
> +	ilk_wm_disable_quirk(dev_priv, 1);
>  }
>  
>  static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  {
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
> +
>  	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
>  
>  	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
> @@ -3063,6 +3081,15 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  		snb_wm_latency_quirk(dev_priv);
>  		snb_wm_lp3_irq_quirk(dev_priv);
>  	}
> +
> +	/*
> +	 * Lenovo Thinkpad T431s (ivb)
> +	 * Massive underruns.
> +	 */
> +	if (pdev->device == 0x0166 &&
> +	    pdev->subsystem_vendor == 0x17aa &&
> +	    pdev->subsystem_device == 0x2208)
> +		ilk_wm_disable_lp1_quirk(dev_priv);
>  }
>  
>  static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/4] drm/i915: Disable LP1+ watermarks on Lenovo Thinkpad T431s

[Jani Nikula]

On Fri, 22 Feb 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Lenovo Thinkpad T431s (ivb) apparently can't handle LP1+ watermarks
> being enabled. It underruns severly enough that the screen is
> unusable. The latency values and watemarks look as expected.
> And sadly updating the BIOS to the latest version (BIOS GHET41WW
> (1.26 ) 11/21/2018) did not help.
>
> One glimmer of hope I had was the VBT. It seems to have some sort
> of flag for "self refresh = yes/no", but when I looked at a bunch
> of VBTs I had lying around most of them had that field set to "no".
> So if we used that we'd end up disabling LP1+ on most machines.
> That seems a bit harsh since we know LP1+ works just fine on
> most machines.
>
> Since I have no better ideas let's just disable LP1+ watermarks
> on this particular machine via a quirk.
>
> Cc: stable@vger.kernel.org
> Cc: Andrea <andreatclist@gmail.com>
> Reported-by: Andrea <andreatclist@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109477
> Fixes: a46a7350b1e8 ("drm/i915: Fix ilk+ watermarks when disabling pipes")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Does what it says on the box, though I'm saddened we have to resort to a
quirk.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++--------
>  1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4c0e43caa5cd..75dceac19950 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3015,6 +3015,29 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv)
>  	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
>  }
>  
> +static void ilk_wm_disable_quirk(struct drm_i915_private *dev_priv,
> +				 int level)
> +{
> +	int max_level = ilk_wm_max_level(dev_priv);
> +
> +	if (dev_priv->wm.pri_latency[level] == 0 &&
> +	    dev_priv->wm.spr_latency[level] == 0 &&
> +	    dev_priv->wm.cur_latency[level] == 0)
> +		return;
> +
> +	DRM_DEBUG_KMS("LP%d+ watermarks disabled by quirk\n", level);
> +
> +	for (; level <= max_level; level++) {
> +		dev_priv->wm.pri_latency[level] = 0;
> +		dev_priv->wm.spr_latency[level] = 0;
> +		dev_priv->wm.cur_latency[level] = 0;
> +	}
> +
> +	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> +	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> +	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> +}
> +
>  static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
>  {
>  	/*
> @@ -3028,23 +3051,18 @@ static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv)
>  	 * interrupts only. To play it safe we disable LP3
>  	 * watermarks entirely.
>  	 */
> -	if (dev_priv->wm.pri_latency[3] == 0 &&
> -	    dev_priv->wm.spr_latency[3] == 0 &&
> -	    dev_priv->wm.cur_latency[3] == 0)
> -		return;
> -
> -	dev_priv->wm.pri_latency[3] = 0;
> -	dev_priv->wm.spr_latency[3] = 0;
> -	dev_priv->wm.cur_latency[3] = 0;
> +	ilk_wm_disable_quirk(dev_priv, 3);
> +}
>  
> -	DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n");
> -	intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> -	intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency);
> -	intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency);
> +static void ilk_wm_disable_lp1_quirk(struct drm_i915_private *dev_priv)
> +{
> +	ilk_wm_disable_quirk(dev_priv, 1);
>  }
>  
>  static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  {
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
> +
>  	intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency);
>  
>  	memcpy(dev_priv->wm.spr_latency, dev_priv->wm.pri_latency,
> @@ -3063,6 +3081,15 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv)
>  		snb_wm_latency_quirk(dev_priv);
>  		snb_wm_lp3_irq_quirk(dev_priv);
>  	}
> +
> +	/*
> +	 * Lenovo Thinkpad T431s (ivb)
> +	 * Massive underruns.
> +	 */
> +	if (pdev->device == 0x0166 &&
> +	    pdev->subsystem_vendor == 0x17aa &&
> +	    pdev->subsystem_device == 0x2208)
> +		ilk_wm_disable_lp1_quirk(dev_priv);
>  }
>  
>  static void skl_setup_wm_latency(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx