stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
@ 2017-12-04 12:36 gregkh
  2017-12-04 12:54 ` Ville Syrjälä
  2018-01-11 19:42 ` [PATCH stable-4.14 1/2] drm/i915: Move init_clock_gating() back to where it was Ville Syrjala
  0 siblings, 2 replies; 12+ messages in thread
From: gregkh @ 2017-12-04 12:36 UTC (permalink / raw)
  To: ville.syrjala, chris, jani.nikula, rodrigo.vivi; +Cc: stable

The patch below was submitted to be applied to the 4.14-stable tree.

I fail to see how this patch meets the stable kernel rules as found at
Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to 
<stable@vger.kernel.org> and let me know why this patch should be
applied.  Otherwise, it is now dropped from my patch queues, never to be
seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Thu, 16 Nov 2017 18:02:15 +0200
Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Moving the init_clock_gating() call from intel_modeset_init_hw() to
intel_modeset_gem_init() had an unintended effect of not applying
some workarounds on resume. This, for example, cause some kind of
corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
screen after hibernation. Fix the problem by explicitly calling
init_clock_gating() from the resume path.

I really hope this doesn't break something else again. At least
the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
didn't make a comeback, even after a hibernate cycle.

v2: Reorder the init_clock_gating vs. modeset_init_hw to match
    the display reset path (Rodrigo)

Cc: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171116160215.25715-1-ville.syrjala@linux.intel.com
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
(cherry picked from commit 675f7ff35bd256e65d3d0f52718d8babf5d1002a)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 34191028bbad..7d9b07df32fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1714,6 +1714,7 @@ static int i915_drm_resume(struct drm_device *dev)
 	intel_guc_resume(dev_priv);
 
 	intel_modeset_init_hw(dev);
+	intel_init_clock_gating(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display.hpd_irq_setup)

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 12:36 WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree? gregkh
@ 2017-12-04 12:54 ` Ville Syrjälä
  2017-12-04 13:13   ` Greg KH
  2018-01-11 19:42 ` [PATCH stable-4.14 1/2] drm/i915: Move init_clock_gating() back to where it was Ville Syrjala
  1 sibling, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2017-12-04 12:54 UTC (permalink / raw)
  To: gregkh; +Cc: chris, jani.nikula, rodrigo.vivi, stable

On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> The patch below was submitted to be applied to the 4.14-stable tree.
> 
> I fail to see how this patch meets the stable kernel rules as found at
> Documentation/process/stable-kernel-rules.rst.

It fixes a regression. Why do you think it's not suitable for stable?

> 
> I could be totally wrong, and if so, please respond to 
> <stable@vger.kernel.org> and let me know why this patch should be
> applied.  Otherwise, it is now dropped from my patch queues, never to be
> seen again.
> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> Date: Thu, 16 Nov 2017 18:02:15 +0200
> Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Moving the init_clock_gating() call from intel_modeset_init_hw() to
> intel_modeset_gem_init() had an unintended effect of not applying
> some workarounds on resume. This, for example, cause some kind of
> corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> screen after hibernation. Fix the problem by explicitly calling
> init_clock_gating() from the resume path.
> 
> I really hope this doesn't break something else again. At least
> the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> didn't make a comeback, even after a hibernate cycle.
> 
> v2: Reorder the init_clock_gating vs. modeset_init_hw to match
>     the display reset path (Rodrigo)
> 
> Cc: stable@vger.kernel.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Link: https://patchwork.freedesktop.org/patch/msgid/20171116160215.25715-1-ville.syrjala@linux.intel.com
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> (cherry picked from commit 675f7ff35bd256e65d3d0f52718d8babf5d1002a)
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 34191028bbad..7d9b07df32fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1714,6 +1714,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  	intel_guc_resume(dev_priv);
>  
>  	intel_modeset_init_hw(dev);
> +	intel_init_clock_gating(dev_priv);
>  
>  	spin_lock_irq(&dev_priv->irq_lock);
>  	if (dev_priv->display.hpd_irq_setup)

-- 
Ville Syrj�l�
Intel OTC

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 12:54 ` Ville Syrjälä
@ 2017-12-04 13:13   ` Greg KH
  2017-12-04 13:41     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2017-12-04 13:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: chris, jani.nikula, rodrigo.vivi, stable

On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > The patch below was submitted to be applied to the 4.14-stable tree.
> > 
> > I fail to see how this patch meets the stable kernel rules as found at
> > Documentation/process/stable-kernel-rules.rst.
> 
> It fixes a regression. Why do you think it's not suitable for stable?

Because:

> > I could be totally wrong, and if so, please respond to 
> > <stable@vger.kernel.org> and let me know why this patch should be
> > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > seen again.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> > ------------------ original commit in Linus's tree ------------------
> > 
> > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > 
> > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > intel_modeset_gem_init() had an unintended effect of not applying
> > some workarounds on resume. This, for example, cause some kind of
> > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > screen after hibernation. Fix the problem by explicitly calling
> > init_clock_gating() from the resume path.
> > 
> > I really hope this doesn't break something else again. At least
> > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > didn't make a comeback, even after a hibernate cycle.
> > 
> > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> >     the display reset path (Rodrigo)
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")

$ git describe --contains 6ac43272768c
v4.15-rc1~19^2~13^2~1

How is this a 4.14 regression?

thanks,

greg k-h

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 13:13   ` Greg KH
@ 2017-12-04 13:41     ` Ville Syrjälä
  2017-12-04 13:47       ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2017-12-04 13:41 UTC (permalink / raw)
  To: Greg KH; +Cc: chris, jani.nikula, rodrigo.vivi, stable

On Mon, Dec 04, 2017 at 02:13:00PM +0100, Greg KH wrote:
> On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> > On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > > The patch below was submitted to be applied to the 4.14-stable tree.
> > > 
> > > I fail to see how this patch meets the stable kernel rules as found at
> > > Documentation/process/stable-kernel-rules.rst.
> > 
> > It fixes a regression. Why do you think it's not suitable for stable?
> 
> Because:
> 
> > > I could be totally wrong, and if so, please respond to 
> > > <stable@vger.kernel.org> and let me know why this patch should be
> > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > seen again.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > 
> > > ------------------ original commit in Linus's tree ------------------
> > > 
> > > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > > MIME-Version: 1.0
> > > Content-Type: text/plain; charset=UTF-8
> > > Content-Transfer-Encoding: 8bit
> > > 
> > > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > > intel_modeset_gem_init() had an unintended effect of not applying
> > > some workarounds on resume. This, for example, cause some kind of
> > > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > > screen after hibernation. Fix the problem by explicitly calling
> > > init_clock_gating() from the resume path.
> > > 
> > > I really hope this doesn't break something else again. At least
> > > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > > didn't make a comeback, even after a hibernate cycle.
> > > 
> > > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> > >     the display reset path (Rodrigo)
> > > 
> > > Cc: stable@vger.kernel.org
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> 
> $ git describe --contains 6ac43272768c
> v4.15-rc1~19^2~13^2~1
> 
> How is this a 4.14 regression?

commit 6ac43272768ca901daac4076a66c2c4e3c7b9321
Author: Ville Syrj�l� <ville.syrjala@linux.intel.com>
Date:   Wed Nov 8 15:35:55 2017 +0200

    drm/i915: Move init_clock_gating() back to where it was
...
    Cc: stable@vger.kernel.org


So 4.14 is going to break once that gets backported.

-- 
Ville Syrj�l�
Intel OTC

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 13:41     ` Ville Syrjälä
@ 2017-12-04 13:47       ` Greg KH
  2017-12-04 18:45         ` Rodrigo Vivi
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2017-12-04 13:47 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: chris, jani.nikula, rodrigo.vivi, stable

On Mon, Dec 04, 2017 at 03:41:18PM +0200, Ville Syrj�l� wrote:
> On Mon, Dec 04, 2017 at 02:13:00PM +0100, Greg KH wrote:
> > On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> > > On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > > > The patch below was submitted to be applied to the 4.14-stable tree.
> > > > 
> > > > I fail to see how this patch meets the stable kernel rules as found at
> > > > Documentation/process/stable-kernel-rules.rst.
> > > 
> > > It fixes a regression. Why do you think it's not suitable for stable?
> > 
> > Because:
> > 
> > > > I could be totally wrong, and if so, please respond to 
> > > > <stable@vger.kernel.org> and let me know why this patch should be
> > > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > > seen again.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > > ------------------ original commit in Linus's tree ------------------
> > > > 
> > > > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > > > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > > > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > > > MIME-Version: 1.0
> > > > Content-Type: text/plain; charset=UTF-8
> > > > Content-Transfer-Encoding: 8bit
> > > > 
> > > > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > > > intel_modeset_gem_init() had an unintended effect of not applying
> > > > some workarounds on resume. This, for example, cause some kind of
> > > > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > > > screen after hibernation. Fix the problem by explicitly calling
> > > > init_clock_gating() from the resume path.
> > > > 
> > > > I really hope this doesn't break something else again. At least
> > > > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > > > didn't make a comeback, even after a hibernate cycle.
> > > > 
> > > > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> > > >     the display reset path (Rodrigo)
> > > > 
> > > > Cc: stable@vger.kernel.org
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> > 
> > $ git describe --contains 6ac43272768c
> > v4.15-rc1~19^2~13^2~1
> > 
> > How is this a 4.14 regression?
> 
> commit 6ac43272768ca901daac4076a66c2c4e3c7b9321
> Author: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Date:   Wed Nov 8 15:35:55 2017 +0200
> 
>     drm/i915: Move init_clock_gating() back to where it was
> ...
>     Cc: stable@vger.kernel.org
> 
> 
> So 4.14 is going to break once that gets backported.

Ok, then the backporter should include both of those, as this one did
not apply at all to the tree :(

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 13:47       ` Greg KH
@ 2017-12-04 18:45         ` Rodrigo Vivi
  2017-12-04 19:07           ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Rodrigo Vivi @ 2017-12-04 18:45 UTC (permalink / raw)
  To: Greg KH; +Cc: Ville Syrjälä, chris, jani.nikula, stable,
	daniel.vetter

On Mon, Dec 04, 2017 at 01:47:14PM +0000, Greg KH wrote:
> On Mon, Dec 04, 2017 at 03:41:18PM +0200, Ville Syrj�l� wrote:
> > On Mon, Dec 04, 2017 at 02:13:00PM +0100, Greg KH wrote:
> > > On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> > > > On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > > > > The patch below was submitted to be applied to the 4.14-stable tree.
> > > > >
> > > > > I fail to see how this patch meets the stable kernel rules as found at
> > > > > Documentation/process/stable-kernel-rules.rst.
> > > >
> > > > It fixes a regression. Why do you think it's not suitable for stable?
> > >
> > > Because:
> > >
> > > > > I could be totally wrong, and if so, please respond to
> > > > > <stable@vger.kernel.org> and let me know why this patch should be
> > > > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > > > seen again.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > > >
> > > > > ------------------ original commit in Linus's tree ------------------
> > > > >
> > > > > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > > > > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > > > > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > > > > MIME-Version: 1.0
> > > > > Content-Type: text/plain; charset=UTF-8
> > > > > Content-Transfer-Encoding: 8bit
> > > > >
> > > > > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > > > > intel_modeset_gem_init() had an unintended effect of not applying
> > > > > some workarounds on resume. This, for example, cause some kind of
> > > > > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > > > > screen after hibernation. Fix the problem by explicitly calling
> > > > > init_clock_gating() from the resume path.
> > > > >
> > > > > I really hope this doesn't break something else again. At least
> > > > > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > > > > didn't make a comeback, even after a hibernate cycle.
> > > > >
> > > > > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> > > > >     the display reset path (Rodrigo)
> > > > >
> > > > > Cc: stable@vger.kernel.org
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> > >
> > > $ git describe --contains 6ac43272768c
> > > v4.15-rc1~19^2~13^2~1
> > >
> > > How is this a 4.14 regression?
> >
> > commit 6ac43272768ca901daac4076a66c2c4e3c7b9321
> > Author: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Date:   Wed Nov 8 15:35:55 2017 +0200
> >
> >     drm/i915: Move init_clock_gating() back to where it was
> > ...
> >     Cc: stable@vger.kernel.org
> >
> >
> > So 4.14 is going to break once that gets backported.
>
> Ok, then the backporter should include both of those, as this one did
> not apply at all to the tree :(

Hi Greg,

I have few questions here around this history. Please help me to understand
what is going on so we can improve the process and make sure this doesn't happen
again.

I'm also bringing Daniel because he mentioned you have removed us from a
blacklist and I don't know details about the history of that or on any details
that could make you angry in the past with our fixes.

In summary:

- This patch here 3572f04c69ed ("drm/i915: Fix init_clock_gating\
 for resume")
- Fixes 6ac43272768c (part of 4.15-rc2 now)
  - Which fixes b7048ea12fbb (released first on v4.11)
    - which fixes ed4a6a7ca853 (introduced on 4.7)

My questions:

- What happened with 6ac43272768c that wasn't considered for the 4.14 stable?
  It has the same Cc:stable tag as the last patch. Is there anything we should
  had done differently to make sure this patch was got by you on 4.14 before
  the second one blowing up your scripts?

- I wonder if 4.9 stable should also have all of them as well. Should it? Maybe
  the should part of it is more for Ville to tell us actually. But if so the
  next question would be what process should we follow for that? Just backport
  those 3 to 4.9.66 and test here and send to stable@ ml?

- What was the reason that you used the "WTF - never to be seen again" tone
  instead of the regular "FAILED - if someone wants to apply..."? Or in other
  words, what can we do to improve and not make you angry again?

Thanks,
Rodrigo.

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 18:45         ` Rodrigo Vivi
@ 2017-12-04 19:07           ` Greg KH
  2017-12-04 19:18             ` Daniel Vetter
  2017-12-04 23:17             ` Rodrigo Vivi
  0 siblings, 2 replies; 12+ messages in thread
From: Greg KH @ 2017-12-04 19:07 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: Ville Syrjälä, chris, jani.nikula, stable,
	daniel.vetter

On Mon, Dec 04, 2017 at 10:45:50AM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 04, 2017 at 01:47:14PM +0000, Greg KH wrote:
> > On Mon, Dec 04, 2017 at 03:41:18PM +0200, Ville Syrj�l� wrote:
> > > On Mon, Dec 04, 2017 at 02:13:00PM +0100, Greg KH wrote:
> > > > On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> > > > > On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > > > > > The patch below was submitted to be applied to the 4.14-stable tree.
> > > > > >
> > > > > > I fail to see how this patch meets the stable kernel rules as found at
> > > > > > Documentation/process/stable-kernel-rules.rst.
> > > > >
> > > > > It fixes a regression. Why do you think it's not suitable for stable?
> > > >
> > > > Because:
> > > >
> > > > > > I could be totally wrong, and if so, please respond to
> > > > > > <stable@vger.kernel.org> and let me know why this patch should be
> > > > > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > > > > seen again.
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > > >
> > > > > > ------------------ original commit in Linus's tree ------------------
> > > > > >
> > > > > > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > > > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > > > > > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > > > > > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > > > > > MIME-Version: 1.0
> > > > > > Content-Type: text/plain; charset=UTF-8
> > > > > > Content-Transfer-Encoding: 8bit
> > > > > >
> > > > > > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > > > > > intel_modeset_gem_init() had an unintended effect of not applying
> > > > > > some workarounds on resume. This, for example, cause some kind of
> > > > > > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > > > > > screen after hibernation. Fix the problem by explicitly calling
> > > > > > init_clock_gating() from the resume path.
> > > > > >
> > > > > > I really hope this doesn't break something else again. At least
> > > > > > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > > > > > didn't make a comeback, even after a hibernate cycle.
> > > > > >
> > > > > > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> > > > > >     the display reset path (Rodrigo)
> > > > > >
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> > > >
> > > > $ git describe --contains 6ac43272768c
> > > > v4.15-rc1~19^2~13^2~1
> > > >
> > > > How is this a 4.14 regression?
> > >
> > > commit 6ac43272768ca901daac4076a66c2c4e3c7b9321
> > > Author: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Date:   Wed Nov 8 15:35:55 2017 +0200
> > >
> > >     drm/i915: Move init_clock_gating() back to where it was
> > > ...
> > >     Cc: stable@vger.kernel.org
> > >
> > >
> > > So 4.14 is going to break once that gets backported.
> >
> > Ok, then the backporter should include both of those, as this one did
> > not apply at all to the tree :(
> 
> Hi Greg,
> 
> I have few questions here around this history. Please help me to understand
> what is going on so we can improve the process and make sure this doesn't happen
> again.
> 
> I'm also bringing Daniel because he mentioned you have removed us from a
> blacklist and I don't know details about the history of that or on any details
> that could make you angry in the past with our fixes.
> 
> In summary:
> 
> - This patch here 3572f04c69ed ("drm/i915: Fix init_clock_gating\
>  for resume")
> - Fixes 6ac43272768c (part of 4.15-rc2 now)

This patch got rejected and got a FAILED email


>   - Which fixes b7048ea12fbb (released first on v4.11)
>     - which fixes ed4a6a7ca853 (introduced on 4.7)
> 
> My questions:
> 
> - What happened with 6ac43272768c that wasn't considered for the 4.14 stable?

It did not apply, and got a FAILED email response.

>   It has the same Cc:stable tag as the last patch. Is there anything we should
>   had done differently to make sure this patch was got by you on 4.14 before
>   the second one blowing up your scripts?

Nope, not much you can do about it failing :)

> - I wonder if 4.9 stable should also have all of them as well. Should it?

That's up to you all, not me.

> Maybe
>   the should part of it is more for Ville to tell us actually. But if so the
>   next question would be what process should we follow for that? Just backport
>   those 3 to 4.9.66 and test here and send to stable@ ml?

Yes that would be great.

> - What was the reason that you used the "WTF - never to be seen again" tone
>   instead of the regular "FAILED - if someone wants to apply..."? Or in other
>   words, what can we do to improve and not make you angry again?

First off, the WTF is just an email script, don't take it personally.

Second, I sent it because it referred to a patch that was not in 4.14,
and was not in any stable queue.  I did not catch that I had already
rejected it, as I really don't have a way to do that at all.

So all is fine, just backport the changes and send them to me and I'll
be glad to queue them up.

thanks,

greg k-h

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 19:07           ` Greg KH
@ 2017-12-04 19:18             ` Daniel Vetter
  2017-12-04 19:52               ` Greg KH
  2017-12-04 23:17             ` Rodrigo Vivi
  1 sibling, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2017-12-04 19:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Rodrigo Vivi, Ville Syrjälä, Chris Wilson, Jani Nikula,
	stable

On Mon, Dec 4, 2017 at 8:07 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Dec 04, 2017 at 10:45:50AM -0800, Rodrigo Vivi wrote:
>> - What was the reason that you used the "WTF - never to be seen again" tone
>>   instead of the regular "FAILED - if someone wants to apply..."? Or in other
>>   words, what can we do to improve and not make you angry again?
>
> First off, the WTF is just an email script, don't take it personally.

Jumping in here - tune it down a bit so it's less confusing? I guess
in general it's not all that confusing, but since we did upset you
rather badly a few months ago it's easy to jump to conclusions here
and assume that i915 maintainers once more upset Greg badly :-/

Just an idea in the spirit of the "make the bots friendlier"
discussion from iirc kernel summit.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 19:18             ` Daniel Vetter
@ 2017-12-04 19:52               ` Greg KH
  0 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2017-12-04 19:52 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Rodrigo Vivi, Ville Syrjälä, Chris Wilson, Jani Nikula,
	stable

On Mon, Dec 04, 2017 at 08:18:47PM +0100, Daniel Vetter wrote:
> On Mon, Dec 4, 2017 at 8:07 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Mon, Dec 04, 2017 at 10:45:50AM -0800, Rodrigo Vivi wrote:
> >> - What was the reason that you used the "WTF - never to be seen again" tone
> >>   instead of the regular "FAILED - if someone wants to apply..."? Or in other
> >>   words, what can we do to improve and not make you angry again?
> >
> > First off, the WTF is just an email script, don't take it personally.
> 
> Jumping in here - tune it down a bit so it's less confusing? I guess
> in general it's not all that confusing, but since we did upset you
> rather badly a few months ago it's easy to jump to conclusions here
> and assume that i915 maintainers once more upset Greg badly :-/

Well, I'm ignoring the 10+ patches that I had to drop because I had
duplicates already applied, that did make me grumpy, but I'll live with
it for now...

> Just an idea in the spirit of the "make the bots friendlier"
> discussion from iirc kernel summit.

Normally, the script here is correct, it is rare that this type of
failure happens (dependancy on a patch that failed).  In fact, I think
it's the first time...

thanks,

greg k-h

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

* Re: WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree?
  2017-12-04 19:07           ` Greg KH
  2017-12-04 19:18             ` Daniel Vetter
@ 2017-12-04 23:17             ` Rodrigo Vivi
  1 sibling, 0 replies; 12+ messages in thread
From: Rodrigo Vivi @ 2017-12-04 23:17 UTC (permalink / raw)
  To: Greg KH; +Cc: Ville Syrjälä, chris, jani.nikula, stable,
	daniel.vetter

On Mon, Dec 04, 2017 at 07:07:06PM +0000, Greg KH wrote:
> On Mon, Dec 04, 2017 at 10:45:50AM -0800, Rodrigo Vivi wrote:
> > On Mon, Dec 04, 2017 at 01:47:14PM +0000, Greg KH wrote:
> > > On Mon, Dec 04, 2017 at 03:41:18PM +0200, Ville Syrj�l� wrote:
> > > > On Mon, Dec 04, 2017 at 02:13:00PM +0100, Greg KH wrote:
> > > > > On Mon, Dec 04, 2017 at 02:54:31PM +0200, Ville Syrj�l� wrote:
> > > > > > On Mon, Dec 04, 2017 at 01:36:08PM +0100, gregkh@linuxfoundation.org wrote:
> > > > > > > The patch below was submitted to be applied to the 4.14-stable tree.
> > > > > > >
> > > > > > > I fail to see how this patch meets the stable kernel rules as found at
> > > > > > > Documentation/process/stable-kernel-rules.rst.
> > > > > >
> > > > > > It fixes a regression. Why do you think it's not suitable for stable?
> > > > >
> > > > > Because:
> > > > >
> > > > > > > I could be totally wrong, and if so, please respond to
> > > > > > > <stable@vger.kernel.org> and let me know why this patch should be
> > > > > > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > > > > > seen again.
> > > > > > >
> > > > > > > thanks,
> > > > > > >
> > > > > > > greg k-h
> > > > > > >
> > > > > > > ------------------ original commit in Linus's tree ------------------
> > > > > > >
> > > > > > > >From 3572f04c69ed4369da5d3c65d84fb18774aa60b6 Mon Sep 17 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
> > > > > > > Date: Thu, 16 Nov 2017 18:02:15 +0200
> > > > > > > Subject: [PATCH] drm/i915: Fix init_clock_gating for resume
> > > > > > > MIME-Version: 1.0
> > > > > > > Content-Type: text/plain; charset=UTF-8
> > > > > > > Content-Transfer-Encoding: 8bit
> > > > > > >
> > > > > > > Moving the init_clock_gating() call from intel_modeset_init_hw() to
> > > > > > > intel_modeset_gem_init() had an unintended effect of not applying
> > > > > > > some workarounds on resume. This, for example, cause some kind of
> > > > > > > corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
> > > > > > > screen after hibernation. Fix the problem by explicitly calling
> > > > > > > init_clock_gating() from the resume path.
> > > > > > >
> > > > > > > I really hope this doesn't break something else again. At least
> > > > > > > the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
> > > > > > > didn't make a comeback, even after a hibernate cycle.
> > > > > > >
> > > > > > > v2: Reorder the init_clock_gating vs. modeset_init_hw to match
> > > > > > >     the display reset path (Rodrigo)
> > > > > > >
> > > > > > > Cc: stable@vger.kernel.org
> > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
> > > > >
> > > > > $ git describe --contains 6ac43272768c
> > > > > v4.15-rc1~19^2~13^2~1
> > > > >
> > > > > How is this a 4.14 regression?
> > > >
> > > > commit 6ac43272768ca901daac4076a66c2c4e3c7b9321
> > > > Author: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > > Date:   Wed Nov 8 15:35:55 2017 +0200
> > > >
> > > >     drm/i915: Move init_clock_gating() back to where it was
> > > > ...
> > > >     Cc: stable@vger.kernel.org
> > > >
> > > >
> > > > So 4.14 is going to break once that gets backported.
> > >
> > > Ok, then the backporter should include both of those, as this one did
> > > not apply at all to the tree :(
> > 
> > Hi Greg,
> > 
> > I have few questions here around this history. Please help me to understand
> > what is going on so we can improve the process and make sure this doesn't happen
> > again.
> > 
> > I'm also bringing Daniel because he mentioned you have removed us from a
> > blacklist and I don't know details about the history of that or on any details
> > that could make you angry in the past with our fixes.
> > 
> > In summary:
> > 
> > - This patch here 3572f04c69ed ("drm/i915: Fix init_clock_gating\
> >  for resume")
> > - Fixes 6ac43272768c (part of 4.15-rc2 now)
> 
> This patch got rejected and got a FAILED email

Oh I see.

> 
> 
> >   - Which fixes b7048ea12fbb (released first on v4.11)
> >     - which fixes ed4a6a7ca853 (introduced on 4.7)
> > 
> > My questions:
> > 
> > - What happened with 6ac43272768c that wasn't considered for the 4.14 stable?
> 
> It did not apply, and got a FAILED email response.
> 
> >   It has the same Cc:stable tag as the last patch. Is there anything we should
> >   had done differently to make sure this patch was got by you on 4.14 before
> >   the second one blowing up your scripts?
> 
> Nope, not much you can do about it failing :)
> 
> > - I wonder if 4.9 stable should also have all of them as well. Should it?
> 
> That's up to you all, not me.

yeap!

> 
> > Maybe
> >   the should part of it is more for Ville to tell us actually. But if so the
> >   next question would be what process should we follow for that? Just backport
> >   those 3 to 4.9.66 and test here and send to stable@ ml?
> 
> Yes that would be great.
> 
> > - What was the reason that you used the "WTF - never to be seen again" tone
> >   instead of the regular "FAILED - if someone wants to apply..."? Or in other
> >   words, what can we do to improve and not make you angry again?
> 
> First off, the WTF is just an email script,

I imagined that, but I wasn't sure...

> don't take it personally.

I never take personally ;)

I just jumped in and asked to understand the big picture and see if there
was something we could improve here.

> 
> Second, I sent it because it referred to a patch that was not in 4.14,
> and was not in any stable queue.  I did not catch that I had already
> rejected it, as I really don't have a way to do that at all.

All makes sense now! Also I'm glad this is not a frequent case.

> 
> So all is fine, just backport the changes and send them to me and I'll
> be glad to queue them up.

Cool,

Thanks,
Rodrigo.

> 
> thanks,
> 
> greg k-h

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

* [PATCH stable-4.14 1/2] drm/i915: Move init_clock_gating() back to where it was
  2017-12-04 12:36 WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree? gregkh
  2017-12-04 12:54 ` Ville Syrjälä
@ 2018-01-11 19:42 ` Ville Syrjala
  2018-01-11 19:42   ` [PATCH stable-4.14 2/2] drm/i915: Fix init_clock_gating for resume Ville Syrjala
  1 sibling, 1 reply; 12+ messages in thread
From: Ville Syrjala @ 2018-01-11 19:42 UTC (permalink / raw)
  To: gregkh; +Cc: stable

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

Apparently setting up a bunch of GT registers before we've properly
initialized the rest of the GT hardware leads to these setting being
lost. So looks like I broke HSW with commit b7048ea12fbb ("drm/i915:
Do .init_clock_gating() earlier to avoid it clobbering watermarks")
by doing init_clock_gating() too early. This should actually affect
other platforms as well, but apparently not to such a great degree.

What I was ultimately after in that commit was to move the
ilk_init_lp_watermarks() call earlier. So let's undo the damage and
move init_clock_gating() back to where it was, and call
ilk_init_lp_watermarks() just before the watermark state readout.

This highlights how fragile and messed up our init order really is.
I wonder why we even initialize the display before gem. The opposite
order would make much more sense to me...

v2: Keep WaRsPkgCStateDisplayPMReq:hsw early as it really must
    be done before all planes might get disabled.

Cc: <stable@vger.kernel.org> # 4.14
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mark Janes <mark.a.janes@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reported-by: Mark Janes <mark.a.janes@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103549
Fixes: b7048ea12fbb ("drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks")
References: https://lists.freedesktop.org/archives/intel-gfx/2017-November/145432.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171108133555.14091-1-ville.syrjala@linux.intel.com
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
(cherry picked from commit f72b84c677d61f201b869223a8d6e389c7bb7d3d)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit 6ac43272768ca901daac4076a66c2c4e3c7b9321)
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++--
 drivers/gpu/drm/i915/intel_pm.c      | 44 +++++++++++++++---------------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1c73d5542681..095a2240af4f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3800,6 +3800,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 
 		intel_pps_unlock_regs_wa(dev_priv);
 		intel_modeset_init_hw(dev);
+		intel_init_clock_gating(dev_priv);
 
 		spin_lock_irq(&dev_priv->irq_lock);
 		if (dev_priv->display.hpd_irq_setup)
@@ -14406,8 +14407,6 @@ void intel_modeset_init_hw(struct drm_device *dev)
 
 	intel_update_cdclk(dev_priv);
 	dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw;
-
-	intel_init_clock_gating(dev_priv);
 }
 
 /*
@@ -15124,6 +15123,15 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	struct intel_encoder *encoder;
 	int i;
 
+	if (IS_HASWELL(dev_priv)) {
+		/*
+		 * WaRsPkgCStateDisplayPMReq:hsw
+		 * System hang if this isn't done before disabling all planes!
+		 */
+		I915_WRITE(CHICKEN_PAR1_1,
+			   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
+	}
+
 	intel_modeset_readout_hw_state(dev);
 
 	/* HW state is read out, now we need to sanitize this mess. */
@@ -15220,6 +15228,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
 
 	intel_init_gt_powersave(dev_priv);
 
+	intel_init_clock_gating(dev_priv);
+
 	intel_setup_overlay(dev_priv);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cb950752c346..014e5c08571a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5669,12 +5669,30 @@ void vlv_wm_sanitize(struct drm_i915_private *dev_priv)
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
 
+/*
+ * FIXME should probably kill this and improve
+ * the real watermark readout/sanitation instead
+ */
+static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
+{
+	I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
+	I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
+	I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
+
+	/*
+	 * Don't touch WM1S_LP_EN here.
+	 * Doing so could cause underruns.
+	 */
+}
+
 void ilk_wm_get_hw_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct ilk_wm_values *hw = &dev_priv->wm.hw;
 	struct drm_crtc *crtc;
 
+	ilk_init_lp_watermarks(dev_priv);
+
 	for_each_crtc(dev, crtc)
 		ilk_pipe_wm_get_hw_state(crtc);
 
@@ -7959,18 +7977,6 @@ static void g4x_disable_trickle_feed(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void ilk_init_lp_watermarks(struct drm_i915_private *dev_priv)
-{
-	I915_WRITE(WM3_LP_ILK, I915_READ(WM3_LP_ILK) & ~WM1_LP_SR_EN);
-	I915_WRITE(WM2_LP_ILK, I915_READ(WM2_LP_ILK) & ~WM1_LP_SR_EN);
-	I915_WRITE(WM1_LP_ILK, I915_READ(WM1_LP_ILK) & ~WM1_LP_SR_EN);
-
-	/*
-	 * Don't touch WM1S_LP_EN here.
-	 * Doing so could cause underruns.
-	 */
-}
-
 static void ironlake_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
@@ -8004,8 +8010,6 @@ static void ironlake_init_clock_gating(struct drm_i915_private *dev_priv)
 		   (I915_READ(DISP_ARB_CTL) |
 		    DISP_FBC_WM_DIS));
 
-	ilk_init_lp_watermarks(dev_priv);
-
 	/*
 	 * Based on the document from hardware guys the following bits
 	 * should be set unconditionally in order to enable FBC.
@@ -8118,8 +8122,6 @@ static void gen6_init_clock_gating(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN6_GT_MODE,
 		   _MASKED_FIELD(GEN6_WIZ_HASHING_MASK, GEN6_WIZ_HASHING_16x4));
 
-	ilk_init_lp_watermarks(dev_priv);
-
 	I915_WRITE(CACHE_MODE_0,
 		   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
 
@@ -8293,8 +8295,6 @@ static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	enum pipe pipe;
 
-	ilk_init_lp_watermarks(dev_priv);
-
 	/* WaSwitchSolVfFArbitrationPriority:bdw */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
@@ -8349,8 +8349,6 @@ static void broadwell_init_clock_gating(struct drm_i915_private *dev_priv)
 
 static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
 {
-	ilk_init_lp_watermarks(dev_priv);
-
 	/* L3 caching of data atomics doesn't work -- disable it. */
 	I915_WRITE(HSW_SCRATCH1, HSW_SCRATCH1_L3_DATA_ATOMICS_DISABLE);
 	I915_WRITE(HSW_ROW_CHICKEN3,
@@ -8394,10 +8392,6 @@ static void haswell_init_clock_gating(struct drm_i915_private *dev_priv)
 	/* WaSwitchSolVfFArbitrationPriority:hsw */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
-	/* WaRsPkgCStateDisplayPMReq:hsw */
-	I915_WRITE(CHICKEN_PAR1_1,
-		   I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
-
 	lpt_init_clock_gating(dev_priv);
 }
 
@@ -8405,8 +8399,6 @@ static void ivybridge_init_clock_gating(struct drm_i915_private *dev_priv)
 {
 	uint32_t snpcr;
 
-	ilk_init_lp_watermarks(dev_priv);
-
 	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_VRHUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaDisableEarlyCull:ivb */
-- 
2.13.6

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

* [PATCH stable-4.14 2/2] drm/i915: Fix init_clock_gating for resume
  2018-01-11 19:42 ` [PATCH stable-4.14 1/2] drm/i915: Move init_clock_gating() back to where it was Ville Syrjala
@ 2018-01-11 19:42   ` Ville Syrjala
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjala @ 2018-01-11 19:42 UTC (permalink / raw)
  To: gregkh; +Cc: stable

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

Moving the init_clock_gating() call from intel_modeset_init_hw() to
intel_modeset_gem_init() had an unintended effect of not applying
some workarounds on resume. This, for example, cause some kind of
corruption to appear at the top of my IVB Thinkpad X1 Carbon LVDS
screen after hibernation. Fix the problem by explicitly calling
init_clock_gating() from the resume path.

I really hope this doesn't break something else again. At least
the problems reported at https://bugs.freedesktop.org/show_bug.cgi?id=103549
didn't make a comeback, even after a hibernate cycle.

v2: Reorder the init_clock_gating vs. modeset_init_hw to match
    the display reset path (Rodrigo)

Cc: <stable@vger.kernel.org> # 4.14
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: 6ac43272768c ("drm/i915: Move init_clock_gating() back to where it was")
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171116160215.25715-1-ville.syrjala@linux.intel.com
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
(cherry picked from commit 675f7ff35bd256e65d3d0f52718d8babf5d1002a)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit 3572f04c69ed4369da5d3c65d84fb18774aa60b6)
---
 drivers/gpu/drm/i915/i915_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 82498f8232eb..5c5cb2ceee49 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1693,6 +1693,7 @@ static int i915_drm_resume(struct drm_device *dev)
 	intel_guc_resume(dev_priv);
 
 	intel_modeset_init_hw(dev);
+	intel_init_clock_gating(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display.hpd_irq_setup)
-- 
2.13.6

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

end of thread, other threads:[~2018-01-11 19:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-04 12:36 WTF: patch "[PATCH] drm/i915: Fix init_clock_gating for resume" was seriously submitted to be applied to the 4.14-stable tree? gregkh
2017-12-04 12:54 ` Ville Syrjälä
2017-12-04 13:13   ` Greg KH
2017-12-04 13:41     ` Ville Syrjälä
2017-12-04 13:47       ` Greg KH
2017-12-04 18:45         ` Rodrigo Vivi
2017-12-04 19:07           ` Greg KH
2017-12-04 19:18             ` Daniel Vetter
2017-12-04 19:52               ` Greg KH
2017-12-04 23:17             ` Rodrigo Vivi
2018-01-11 19:42 ` [PATCH stable-4.14 1/2] drm/i915: Move init_clock_gating() back to where it was Ville Syrjala
2018-01-11 19:42   ` [PATCH stable-4.14 2/2] drm/i915: Fix init_clock_gating for resume Ville Syrjala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).