From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic() Date: Thu, 1 Jun 2017 19:05:41 +0300 Message-ID: <20170601160541.GQ12629@intel.com> References: <20170601143619.27840-1-ville.syrjala@linux.intel.com> <20170601143619.27840-3-ville.syrjala@linux.intel.com> <87lgpbhihb.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8FE926E3C8 for ; Thu, 1 Jun 2017 16:05:55 +0000 (UTC) Content-Disposition: inline In-Reply-To: <87lgpbhihb.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gVGh1LCBKdW4gMDEsIDIwMTcgYXQgMDU6NDg6MTZQTSArMDMwMCwgSmFuaSBOaWt1bGEgd3Jv dGU6Cj4gT24gVGh1LCAwMSBKdW4gMjAxNywgdmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20g d3JvdGU6Cj4gPiBGcm9tOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGludXguaW50 ZWwuY29tPgo+ID4KPiA+IElmIGludGVsX2NydGNfZGlzYWJsZV9ub2F0b21pYygpIHdlcmUgdG8g ZXZlciBnZXQgY2FsbGVkIGR1cmluZyByZXN1bWUKPiA+IGUnZCBlbmQgdXAgZGVhZGxvY2tpbmcg c2luY2UgcmVzdW1lIGhhcyBpdHMgb3duIGFjcWN1aXJlX2N0eCBidXQKPiA+IGludGVsX2NydGNf ZGlzYWJsZV9ub2F0b21pYygpIHN0aWxsIHRyaWVzIHRvIHVzZSB0aGUKPiA+IG1vZGVfY29uZmln LmFjcXVpcmVfY3R4LiBQYXNzIGRvd24gdGhlIGNvcnJlY3QgYWNxdWlyZSBjdHggZnJvbSB0aGUg dG9wLgo+IAo+IFNhbWUgaGVyZSwgaGF2ZSB3ZSBzZWVuIHRoaXMsIG9yICJqdXN0IiB0aGVvcmV0 aWNhbD8KCkhhcHBlbnMgb24gbXkgODMwIGFmdGVyIGFkZGluZyB0aGUgcGlwZXMgcG93ZXJ3ZWxs LiBCZWZvcmUgdGhhdCB0aGUKbWFjaGluZSByZXN1bWVkIHdpdGggYm90aCBwaXBlcyBvZmYgYW5k IHRoZSBwaXBlIEEgcXVpcmsgbGVmdApjcnRjLT5hY3RpdmUgYXMgZmFsc2Ugc28gaW50ZWxfY3J0 Y19kaXNhYmxlX25vYXRvbWljKCkgZGlkbid0IGdldApjYWxsZWQuCgpIbW0uIEFjdHVhbGx5LCBu b3cgdGhhdCBJIHRoaW5rIGFib3V0IGl0IG1vcmUsIEkgYmVsaWV2ZSB0aGlzIHNob3VsZApoYXBw ZW4gZm9yIFM0IGV2ZW4gd2l0aG91dCB0aGUgbmV3IHBvd2Vyd2VsbCBzaW5jZSBib3RoIHBpcGVz IHNob3VsZApoYXZlIGJlZW4gZW5hYmxlZCBieSB0aGUgQklPUyB3aGVuIHJlc3VtaW5nIGZyb20g UzQuIEknbGwgaGF2ZSB0bwpydW4gYSBwcm9wZXIgdG8gdGVzdCB0byB2ZXJpZnkgdGhhdC4KCj4g Cj4gQlIsCj4gSmFuaS4KPiAKPiA+Cj4gPiBDYzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+ID4g Q2M6IE1hYXJ0ZW4gTGFua2hvcnN0IDxtYWFydGVuLmxhbmtob3JzdEBsaW51eC5pbnRlbC5jb20+ Cj4gPiBGaXhlczogZTJjOGI4NzAxZTJkICgiZHJtL2k5MTU6IFVzZSBhdG9taWMgaGVscGVycyBm b3Igc3VzcGVuZCwgdjIuIikKPiA+IFNpZ25lZC1vZmYtYnk6IFZpbGxlIFN5cmrDpGzDpCA8dmls bGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9kaXNwbGF5LmMgfCA5ICsrKysrLS0tLQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCA1 IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCj4gPgo+ID4gZGlmZiAtLWdpdCBhL2RyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXkuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2lu dGVsX2Rpc3BsYXkuYwo+ID4gaW5kZXggMmY3NmE2M2VmZThjLi40ZTNjNjRlZDQxMzEgMTAwNjQ0 Cj4gPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKPiA+ICsrKyBi L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXkuYwo+ID4gQEAgLTU4MzksNyArNTgz OSw4IEBAIHN0YXRpYyB2b2lkIGk5eHhfY3J0Y19kaXNhYmxlKHN0cnVjdCBpbnRlbF9jcnRjX3N0 YXRlICpvbGRfY3J0Y19zdGF0ZSwKPiA+ICAJCWludGVsX3VwZGF0ZV93YXRlcm1hcmtzKGludGVs X2NydGMpOwo+ID4gIH0KPiA+ICAKPiA+IC1zdGF0aWMgdm9pZCBpbnRlbF9jcnRjX2Rpc2FibGVf bm9hdG9taWMoc3RydWN0IGRybV9jcnRjICpjcnRjKQo+ID4gK3N0YXRpYyB2b2lkIGludGVsX2Ny dGNfZGlzYWJsZV9ub2F0b21pYyhzdHJ1Y3QgZHJtX2NydGMgKmNydGMsCj4gPiArCQkJCQlzdHJ1 Y3QgZHJtX21vZGVzZXRfYWNxdWlyZV9jdHggKmN0eCkKPiA+ICB7Cj4gPiAgCXN0cnVjdCBpbnRl bF9lbmNvZGVyICplbmNvZGVyOwo+ID4gIAlzdHJ1Y3QgaW50ZWxfY3J0YyAqaW50ZWxfY3J0YyA9 IHRvX2ludGVsX2NydGMoY3J0Yyk7Cj4gPiBAQCAtNTg2OSw3ICs1ODcwLDcgQEAgc3RhdGljIHZv aWQgaW50ZWxfY3J0Y19kaXNhYmxlX25vYXRvbWljKHN0cnVjdCBkcm1fY3J0YyAqY3J0YykKPiA+ ICAJCXJldHVybjsKPiA+ICAJfQo+ID4gIAo+ID4gLQlzdGF0ZS0+YWNxdWlyZV9jdHggPSBjcnRj LT5kZXYtPm1vZGVfY29uZmlnLmFjcXVpcmVfY3R4Owo+ID4gKwlzdGF0ZS0+YWNxdWlyZV9jdHgg PSBjdHg7Cj4gPiAgCj4gPiAgCS8qIEV2ZXJ5dGhpbmcncyBhbHJlYWR5IGxvY2tlZCwgLUVERUFE TEsgY2FuJ3QgaGFwcGVuLiAqLwo+ID4gIAljcnRjX3N0YXRlID0gaW50ZWxfYXRvbWljX2dldF9j cnRjX3N0YXRlKHN0YXRlLCBpbnRlbF9jcnRjKTsKPiA+IEBAIC0xNTI1Nyw3ICsxNTI1OCw3IEBA IHN0YXRpYyB2b2lkIGludGVsX3Nhbml0aXplX2NydGMoc3RydWN0IGludGVsX2NydGMgKmNydGMs Cj4gPiAgCQlwbGFuZSA9IGNydGMtPnBsYW5lOwo+ID4gIAkJY3J0Yy0+YmFzZS5wcmltYXJ5LT5z dGF0ZS0+dmlzaWJsZSA9IHRydWU7Cj4gPiAgCQljcnRjLT5wbGFuZSA9ICFwbGFuZTsKPiA+IC0J CWludGVsX2NydGNfZGlzYWJsZV9ub2F0b21pYygmY3J0Yy0+YmFzZSk7Cj4gPiArCQlpbnRlbF9j cnRjX2Rpc2FibGVfbm9hdG9taWMoJmNydGMtPmJhc2UsIGN0eCk7Cj4gPiAgCQljcnRjLT5wbGFu ZSA9IHBsYW5lOwo+ID4gIAl9Cj4gPiAgCj4gPiBAQCAtMTUyNzMsNyArMTUyNzQsNyBAQCBzdGF0 aWMgdm9pZCBpbnRlbF9zYW5pdGl6ZV9jcnRjKHN0cnVjdCBpbnRlbF9jcnRjICpjcnRjLAo+ID4g IAkvKiBBZGp1c3QgdGhlIHN0YXRlIG9mIHRoZSBvdXRwdXQgcGlwZSBhY2NvcmRpbmcgdG8gd2hl dGhlciB3ZQo+ID4gIAkgKiBoYXZlIGFjdGl2ZSBjb25uZWN0b3JzL2VuY29kZXJzLiAqLwo+ID4g IAlpZiAoY3J0Yy0+YWN0aXZlICYmICFpbnRlbF9jcnRjX2hhc19lbmNvZGVycyhjcnRjKSkKPiA+ IC0JCWludGVsX2NydGNfZGlzYWJsZV9ub2F0b21pYygmY3J0Yy0+YmFzZSk7Cj4gPiArCQlpbnRl bF9jcnRjX2Rpc2FibGVfbm9hdG9taWMoJmNydGMtPmJhc2UsIGN0eCk7Cj4gPiAgCj4gPiAgCWlm IChjcnRjLT5hY3RpdmUgfHwgSEFTX0dNQ0hfRElTUExBWShkZXZfcHJpdikpIHsKPiA+ICAJCS8q Cj4gCj4gLS0gCj4gSmFuaSBOaWt1bGEsIEludGVsIE9wZW4gU291cmNlIFRlY2hub2xvZ3kgQ2Vu dGVyCgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdm eEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp bG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com ([192.55.52.43]:13228 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751496AbdFAQF7 (ORCPT ); Thu, 1 Jun 2017 12:05:59 -0400 Date: Thu, 1 Jun 2017 19:05:41 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH 2/7] drm/i915: Plumb the correct acquire ctx into intel_crtc_disable_noatomic() Message-ID: <20170601160541.GQ12629@intel.com> References: <20170601143619.27840-1-ville.syrjala@linux.intel.com> <20170601143619.27840-3-ville.syrjala@linux.intel.com> <87lgpbhihb.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87lgpbhihb.fsf@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Jun 01, 2017 at 05:48:16PM +0300, Jani Nikula wrote: > On Thu, 01 Jun 2017, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� > > > > If intel_crtc_disable_noatomic() were to ever get called during resume > > e'd end up deadlocking since resume has its own acqcuire_ctx but > > intel_crtc_disable_noatomic() still tries to use the > > mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. > > Same here, have we seen this, or "just" theoretical? Happens on my 830 after adding the pipes powerwell. Before that the machine resumed with both pipes off and the pipe A quirk left crtc->active as false so intel_crtc_disable_noatomic() didn't get called. Hmm. Actually, now that I think about it more, I believe this should happen for S4 even without the new powerwell since both pipes should have been enabled by the BIOS when resuming from S4. I'll have to run a proper to test to verify that. > > BR, > Jani. > > > > > Cc: stable@vger.kernel.org > > Cc: Maarten Lankhorst > > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > > Signed-off-by: Ville Syrj�l� > > --- > > drivers/gpu/drm/i915/intel_display.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 2f76a63efe8c..4e3c64ed4131 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state, > > intel_update_watermarks(intel_crtc); > > } > > > > -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, > > + struct drm_modeset_acquire_ctx *ctx) > > { > > struct intel_encoder *encoder; > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > return; > > } > > > > - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; > > + state->acquire_ctx = ctx; > > > > /* Everything's already locked, -EDEADLK can't happen. */ > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > > @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > plane = crtc->plane; > > crtc->base.primary->state->visible = true; > > crtc->plane = !plane; > > - intel_crtc_disable_noatomic(&crtc->base); > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > crtc->plane = plane; > > } > > > > @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > /* Adjust the state of the output pipe according to whether we > > * have active connectors/encoders. */ > > if (crtc->active && !intel_crtc_has_encoders(crtc)) > > - intel_crtc_disable_noatomic(&crtc->base); > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > > if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { > > /* > > -- > Jani Nikula, Intel Open Source Technology Center -- Ville Syrj�l� Intel OTC