From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-owner@vger.kernel.org>
Received: from mga05.intel.com ([192.55.52.43]:23096 "EHLO mga05.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751304AbdKHRk0 (ORCPT <rfc822;stable@vger.kernel.org>);
        Wed, 8 Nov 2017 12:40:26 -0500
Date: Wed, 8 Nov 2017 19:38:10 +0200
From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org,
        Mark Janes <mark.a.janes@intel.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Oscar Mateo <oscar.mateo@intel.com>,
        Mika Kuoppala <mika.kuoppala@linux.intel.com>
Subject: Re: [PATCH v2] drm/i915: Move init_clock_gating() back to where it
 was
Message-ID: <20171108173810.GE10981@intel.com>
References: <20171103130337.23652-1-ville.syrjala@linux.intel.com>
 <20171108133555.14091-1-ville.syrjala@linux.intel.com>
 <151015790756.20729.4841631926962116708@mail.alporthouse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <151015790756.20729.4841631926962116708@mail.alporthouse.com>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>

On Wed, Nov 08, 2017 at 04:18:27PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-11-08 13:35:55)
> > 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
> > 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>
> 
> CTS remains fixed,
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I know of no reason why this should work in this order,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> (but I didn't know about the v2 requirement either!)

It's somewhat baffling how this stuff managed to work before I
moved .init_clock_gating() to happen earlier. AFAICS that plane
disabling vs. the chicken bit should have bitten us back then as
well.

Patch pushed to dinq. Thanks for doing all the heavy lifting on
this one.

-- 
Ville Syrj�l�
Intel OTC