From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks Date: Thu, 2 Mar 2017 22:22:16 +0200 Message-ID: <20170302202216.GL31595@intel.com> References: <20170220140443.30891-1-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 680126EC2F for ; Thu, 2 Mar 2017 20:22:20 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170220140443.30891-1-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: intel-gfx@lists.freedesktop.org Cc: Gabriele Mazzotta , David Purton , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gTW9uLCBGZWIgMjAsIDIwMTcgYXQgMDQ6MDQ6NDNQTSArMDIwMCwgdmlsbGUuc3lyamFsYUBs aW51eC5pbnRlbC5jb20gd3JvdGU6Cj4gRnJvbTogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJq YWxhQGxpbnV4LmludGVsLmNvbT4KPiAKPiBDdXJyZW50bHkgSUxLLUJEVyBleHBsaWNpdGx5IGRp c2FibGUgTFAxKyB3YXRlcm1hcmtzIGZyb20gdGhlaXIKPiAuaW5pdF9jbG9ja19nYXRpbmcoKSBo b29rcy4gVW5mb3J0dW5hdGVseSB0aGF0IGhvb2sgZ2V0cyBjYWxsZWQgd2F5IHRvbwo+IGxhdGUg c2luY2UgYnkgdGhhdCB0aW1lIHdlJ3ZlIGFscmVhZHkgaW5pdGlhbGl6ZWQgYWxsIHRoZSB3YXRl cm1hcmsKPiBzdGF0ZSB0cmFja2luZyB3aGljaCB0aGVuIGdldHMgb3V0IG9mIHN5bmMgd2l0aCB0 aGUgaGFyZHdhcmUgc3RhdGUuCj4gCj4gV2UgbWF5IGV2ZW50dWFsbHkgd2FudCB0byBjb25zaWRl ciBraWxsaW5nIG9mZiB0aGUgZXhwbGljaXQgTFAxKwo+IGRpc2FibGUgZnJvbSAuaW5pdF9jbG9j a19nYXRpbmcoKS4gSW4gdGhlIG1lYW50aW1lIGhvd2V2ZXIsIHdlIGNhbgo+IGF2b2lkIHRoZSBw cm9ibGVtIGJ5IHJlb3JkZXJpbmcgdGhlIGluaXQgc2VxdWVuY2Ugc3VjaCB0aGF0Cj4gaW50ZWxf bW9kZXNldF9pbml0X2h3KCktPmludGVsX2luaXRfY2xvY2tfZ2F0aW5nKCkgZ2V0cyBjYWxsZWQK PiBwcmlvciB0byB0aGUgaGFyZHdhcmUgc3RhdGUgdGFrZW92ZXIuCj4gCj4gSSBzdXBwb3NlIHBy aW9yIHRvIHRoZSB0d28gc3RhZ2Ugd2F0ZXJtYXJrIHByb2dyYW1taW5nIHdlIHdlcmUKPiBtYWdp Y2FsbHkgc2F2ZWQgYnkgc29tZXRoaW5nIHRoYXQgZm9yY2VkIHRoZSB3YXRlcm1hcmtzIHRvIGJl Cj4gcmVwcm9ncmFtbWVkIGZ1bGx5IGFmdGVyIC5pbml0X2Nsb2NrX2dhdGluZygpIGdvdCBjYWxs ZWQuIEJ1dAo+IG5vdyB0aGF0IG5vIGxvbmdlciBoYXBwZW5zLgo+IAo+IE5vdGUgdGhhdCB0aGUg ZGlmZiBtaWdodCBsb29rIGEgYml0IG9kZCBhcyBpdCBraWxscyBvZmYgb25lCj4gY2FsbCBvZiBp bnRlbF91cGRhdGVfY2RjbGsoKSwgYnV0IHRoYXQncyBmaW5lIGJlY2F1c2UKPiBpbnRlbF9tb2Rl c2V0X2luaXRfaHcoKSBkb2VzIHRoZSBleGFjdCBzYW1lIHRoaW5nLiBQcmV2aW91c2x5Cj4gd2Ug anVzdCBkaWQgaXQgdHdpY2UuCj4gCj4gQWN0dWFsbHkgZXZlbiB0aGlzIG5ldyBpbml0IHNlcXVl bmNlIGlzIHByZXR0eSBib2d1cyBhcwo+IC5pbml0X2Nsb2NrX2dhdGluZygpIHJlYWxseSBzaG91 bGQgYmUgY2FsbGVkIGJlZm9yZSBhbnkgZ2VtCj4gaGFyZHdhcmUgaW5pdCBzaW5jZSBpdCBjYW4g IGNvbmZpZ3VyZSB2YXJpb3VzIGNsb2NrIGdhdGluZwo+IHdvcmthcm91bmRzIGFuZCB3aGF0bm90 IHRoYXQgYWZmZWN0IHRoZSBHVCBzaWRlIGFzIHdlbGwuIEFsc28KPiBpbnRlbF9tb2Rlc2V0X2lu aXQoKSByZWFsbHkgc2hvdWxkIGdldCBzcGxpdCB1cCBpbnRvIGJldHRlcgo+IGRlZmluZWQgaW5p dCBzdGFnZXMuIEFub3RoZXIgImZ1biIgZGV0YWlsIGlzIHRoYXQKPiBpbnRlbF9tb2Rlc2V0X2dl bV9pbml0KCkgaXMgd2hlcmUgUlBTL1JDNiBnZXRzIGNvbmZpZ3VyZWQuCj4gV2h5IHRoYXQgaXMg ZG9uZSBmcm9tIHRoZSBkaXNwbGF5IGNvZGUgaXMgYmV5b25kIG1lLiBJJ3ZlCj4gZGVjaWRlZCB0 byBsZWF2ZSBhbGwgdGhpcyBiZSBmb3Igbm93LCBhbmQganVzdCB0cnkgdG8gZml4Cj4gdGhlIGlu aXQgc2VxdWVuY2UgZW5vdWdoIGZvciB3YXRlcm1hcmtzIHRvIHdvcmsuCj4gCj4gQ2M6IHN0YWJs ZUB2Z2VyLmtlcm5lbC5vcmcKPiBDYzogR2FicmllbGUgTWF6em90dGEgPGdhYnJpZWxlLm16dEBn bWFpbC5jb20+Cj4gQ2M6IERhdmlkIFB1cnRvbiA8ZGNwdXJ0b25AbWFyc2h3aWdnbGUubmV0Pgo+ IENjOiBNYXR0IFJvcGVyIDxtYXR0aGV3LmQucm9wZXJAaW50ZWwuY29tPgo+IENjOiBNYWFydGVu IExhbmtob3JzdCA8bWFhcnRlbi5sYW5raG9yc3RAbGludXguaW50ZWwuY29tPgo+IFJlcG9ydGVk LWJ5OiBHYWJyaWVsZSBNYXp6b3R0YSA8Z2FicmllbGUubXp0QGdtYWlsLmNvbT4KPiBSZXBvcnRl ZC1ieTogRGF2aWQgUHVydG9uIDxkY3B1cnRvbkBtYXJzaHdpZ2dsZS5uZXQ+Cj4gVGVzdGVkLWJ5 OiBHYWJyaWVsZSBNYXp6b3R0YSA8Z2FicmllbGUubXp0QGdtYWlsLmNvbT4KPiBCdWd6aWxsYTog aHR0cHM6Ly9idWdzLmZyZWVkZXNrdG9wLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTY2NDUKPiBGaXhl czogZWQ0YTZhN2NhODUzICgiZHJtL2k5MTU6IEFkZCB0d28tc3RhZ2UgSUxLLXN0eWxlIHdhdGVy bWFyayBwcm9ncmFtbWluZyAodjExKSIpCj4gU2lnbmVkLW9mZi1ieTogVmlsbGUgU3lyasOkbMOk IDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4KClB1c2hlZCB0byBkaW5xIHdpdGggRGFu aWVsJ3MgaXJjIHItYi4gVGhhbmtzLgoKMTk6MjIgPCB2c3lyamFsYT4gYW55b25lIGNhcmUgdG8g cmV2aWV3IGh0dHBzOi8vcGF0Y2h3b3JrLmZyZWVkZXNrdG9wLm9yZy9wYXRjaC8xMzk5NzUvID8g d291bGQgYmUgb25lIGxlc3MgYnVnIHRvIAogICAgICAgICAgICAgICAgICB3b3JyeSBhYm91dC4u LgoxOToyOCA8IGRhbnZldD4gdnN5cmphbGEsIHItYgoKPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJt L2k5MTUvaW50ZWxfZGlzcGxheS5jIHwgOSArKystLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDMg aW5zZXJ0aW9ucygrKSwgNiBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9n cHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf ZGlzcGxheS5jCj4gaW5kZXggNzMwYWVlNzU1YzgwLi4wNDY2ZGIxNmYxOTMgMTAwNjQ0Cj4gLS0t IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jCj4gKysrIGIvZHJpdmVycy9n cHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jCj4gQEAgLTE1MDI4LDEyICsxNTAyOCwxMSBAQCBp bnQgaW50ZWxfbW9kZXNldF9pbml0KHN0cnVjdCBkcm1fZGV2aWNlICpkZXYpCj4gIAkJfQo+ICAJ fQo+ICAKPiAtCWludGVsX3VwZGF0ZV9jemNsayhkZXZfcHJpdik7Cj4gLQlpbnRlbF91cGRhdGVf Y2RjbGsoZGV2X3ByaXYpOwo+IC0JZGV2X3ByaXYtPmNkY2xrLmxvZ2ljYWwgPSBkZXZfcHJpdi0+ Y2RjbGsuYWN0dWFsID0gZGV2X3ByaXYtPmNkY2xrLmh3Owo+IC0KPiAgCWludGVsX3NoYXJlZF9k cGxsX2luaXQoZGV2KTsKPiAgCj4gKwlpbnRlbF91cGRhdGVfY3pjbGsoZGV2X3ByaXYpOwo+ICsJ aW50ZWxfbW9kZXNldF9pbml0X2h3KGRldik7Cj4gKwo+ICAJaWYgKGRldl9wcml2LT5tYXhfY2Rj bGtfZnJlcSA9PSAwKQo+ICAJCWludGVsX3VwZGF0ZV9tYXhfY2RjbGsoZGV2X3ByaXYpOwo+ICAK PiBAQCAtMTU1OTEsOCArMTU1OTAsNiBAQCB2b2lkIGludGVsX21vZGVzZXRfZ2VtX2luaXQoc3Ry dWN0IGRybV9kZXZpY2UgKmRldikKPiAgCj4gIAlpbnRlbF9pbml0X2d0X3Bvd2Vyc2F2ZShkZXZf cHJpdik7Cj4gIAo+IC0JaW50ZWxfbW9kZXNldF9pbml0X2h3KGRldik7Cj4gLQo+ICAJaW50ZWxf c2V0dXBfb3ZlcmxheShkZXZfcHJpdik7Cj4gIH0KPiAgCj4gLS0gCj4gMi4xMC4yCgotLSAKVmls bGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:11560 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbdCBU0o (ORCPT ); Thu, 2 Mar 2017 15:26:44 -0500 Date: Thu, 2 Mar 2017 22:22:16 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org, Gabriele Mazzotta , David Purton , Matt Roper , Maarten Lankhorst Subject: Re: [PATCH] drm/i915: Do .init_clock_gating() earlier to avoid it clobbering watermarks Message-ID: <20170302202216.GL31595@intel.com> References: <20170220140443.30891-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170220140443.30891-1-ville.syrjala@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Feb 20, 2017 at 04:04:43PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� > > Currently ILK-BDW explicitly disable LP1+ watermarks from their > .init_clock_gating() hooks. Unfortunately that hook gets called way too > late since by that time we've already initialized all the watermark > state tracking which then gets out of sync with the hardware state. > > We may eventually want to consider killing off the explicit LP1+ > disable from .init_clock_gating(). In the meantime however, we can > avoid the problem by reordering the init sequence such that > intel_modeset_init_hw()->intel_init_clock_gating() gets called > prior to the hardware state takeover. > > I suppose prior to the two stage watermark programming we were > magically saved by something that forced the watermarks to be > reprogrammed fully after .init_clock_gating() got called. But > now that no longer happens. > > Note that the diff might look a bit odd as it kills off one > call of intel_update_cdclk(), but that's fine because > intel_modeset_init_hw() does the exact same thing. Previously > we just did it twice. > > Actually even this new init sequence is pretty bogus as > .init_clock_gating() really should be called before any gem > hardware init since it can configure various clock gating > workarounds and whatnot that affect the GT side as well. Also > intel_modeset_init() really should get split up into better > defined init stages. Another "fun" detail is that > intel_modeset_gem_init() is where RPS/RC6 gets configured. > Why that is done from the display code is beyond me. I've > decided to leave all this be for now, and just try to fix > the init sequence enough for watermarks to work. > > Cc: stable@vger.kernel.org > Cc: Gabriele Mazzotta > Cc: David Purton > Cc: Matt Roper > Cc: Maarten Lankhorst > Reported-by: Gabriele Mazzotta > Reported-by: David Purton > Tested-by: Gabriele Mazzotta > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96645 > Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)") > Signed-off-by: Ville Syrj�l� Pushed to dinq with Daniel's irc r-b. Thanks. 19:22 < vsyrjala> anyone care to review https://patchwork.freedesktop.org/patch/139975/ ? would be one less bug to worry about... 19:28 < danvet> vsyrjala, r-b > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 730aee755c80..0466db16f193 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15028,12 +15028,11 @@ int intel_modeset_init(struct drm_device *dev) > } > } > > - intel_update_czclk(dev_priv); > - intel_update_cdclk(dev_priv); > - dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw; > - > intel_shared_dpll_init(dev); > > + intel_update_czclk(dev_priv); > + intel_modeset_init_hw(dev); > + > if (dev_priv->max_cdclk_freq == 0) > intel_update_max_cdclk(dev_priv); > > @@ -15591,8 +15590,6 @@ void intel_modeset_gem_init(struct drm_device *dev) > > intel_init_gt_powersave(dev_priv); > > - intel_modeset_init_hw(dev); > - > intel_setup_overlay(dev_priv); > } > > -- > 2.10.2 -- Ville Syrj�l� Intel OTC