From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rodrigo Vivi Subject: Re: [PATCH] drm/i915/psr: Chase psr.enabled only under the psr.lock Date: Mon, 9 Apr 2018 12:14:32 -0700 Message-ID: <20180409191432.GN8964@intel.com> References: <20180405114915.29609-1-chris@chris-wilson.co.uk> <7e7b4336aefd6e6cf993150c1a83d214c161a210.camel@intel.com> <20180406221816.GG8964@intel.com> <152309192577.26482.17726023366871782940@mail.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 68D296E14D for ; Mon, 9 Apr 2018 19:14:31 +0000 (UTC) Content-Disposition: inline In-Reply-To: <152309192577.26482.17726023366871782940@mail.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: "intel-gfx@lists.freedesktop.org" , "R, Durgadoss" , "stable@vger.kernel.org" List-Id: intel-gfx@lists.freedesktop.org T24gU2F0LCBBcHIgMDcsIDIwMTggYXQgMTA6MDU6MjVBTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IFF1b3RpbmcgUm9kcmlnbyBWaXZpICgyMDE4LTA0LTA2IDIzOjE4OjE2KQo+ID4gT24g RnJpLCBBcHIgMDYsIDIwMTggYXQgMTE6MTI6MjdBTSAtMDcwMCwgU291emEsIEpvc2Ugd3JvdGU6 Cj4gPiA+IE9uIFRodSwgMjAxOC0wNC0wNSBhdCAxMjo0OSArMDEwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+ID4gPiA+ICsgICAgICAgICAgIHN0cnVjdCBkcm1fY3J0YyAqY3J0YyA9Cj4gPiA+ID4g KyAgICAgICAgICAgICAgICAgICBkcF90b19kaWdfcG9ydChpbnRlbF9kcCktPmJhc2UuYmFzZS5j cnRjOwo+ID4gCj4gPiBJJ20gYWZyYWlkIHRoYXQgdGhlIGlzc3VlIGlzIHRoaXMgcG9pbnRlciBo ZXJlLiBTbyB0aGlzIHdpbGwgb25seSBtYXNrCj4gPiB0aGUgaXNzdWUuCj4gPiAKPiA+IFNob3Vs ZCB3ZSBtYXliZSBzdGFzaCB0aGUgcGlwZT8gOi8KPiAKPiBJdCdzIG5vdCB0aGF0IGJhZC4gcGlw ZSBjYW5ub3QgY2hhbmdlIHVudGlsIGFmdGVyIHBzcl9kaXNhYmxlIGlzIGNhbGxlZCwKPiByaWdo dD8gQW5kIHBzcl9kaXNhYmxlIGVuc3VyZXMgdGhhdCB0aGlzIHdvcmtlciBpcyBmbHVzaGVkLiBU aGUgY3VycmVudAo+IHByb2JsZW0gaXMganVzdCB0aGUgY29vcmRpbmF0aW9uIG9mIGNhbmNlbGxp bmcgdGhlIHdvcmtlciwgd2hlcmUgd2UgbWF5Cj4gc2V0IHBzci5lbmFibGVkIHRvIE5VTEwgcmln aHQgYmVmb3JlIHRoZSB3b3JrZXIgZ3JhYnMgaXQgYW5kCj4gZGVyZWZlcmVuY2VzIGl0Lgo+IAo+ IFNvIGlmIHdlIGxvY2sgdW50aWwgd2UgaGF2ZSB0aGUgcGlwZSwgd2Uga25vdyB0aGF0IGRlcmVm ZXJlbmNlIGNoYWluIGlzCj4gdmFsaWQsIGFuZCB3ZSBrbm93IHRoYXQgcHNyX2Rpc2FibGUoKSBj YW5ub3QgY29tcGxldGUgdW50aWwgd2UgY29tcGxldGUKPiB0aGUgd2FpdC4gU28gdGhlIHBpcGUg cmVtYWlucyB2YWxpZCB1bnRpbCB3ZSByZXR1cm4gKHNvIGxvbmcgYXMgdGhlIHBpcGUKPiBleGlz dHMgd2hlbiB3ZSBzdGFydCkuCgpobW0uLi4gaXQgbWFrZXMgc2Vuc2UgYW5kIEkgaGF2ZSBubyBi ZXR0ZXIgc3VnZ2VzdGlvbiBhY3R1YWxseS4KU28sIGFzIGxvbmcgaXQgcmVhbGx5IGZpeGVzIHRo ZSByZWdyZXNzaW9uIHdlIGludHJvZHVjZWQ6CgpBY2tlZC1ieTogUm9kcmlnbyBWaXZpIDxyb2Ry aWdvLnZpdmlAaW50ZWwuY29tPgoKPiAtQ2hyaXMKPiBfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwo+IEludGVsLWdmeCBtYWlsaW5nIGxpc3QKPiBJbnRlbC1n ZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlz dGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:49082 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753861AbeDITOb (ORCPT ); Mon, 9 Apr 2018 15:14:31 -0400 Date: Mon, 9 Apr 2018 12:14:32 -0700 From: Rodrigo Vivi To: Chris Wilson Cc: "Souza, Jose" , "intel-gfx@lists.freedesktop.org" , "R, Durgadoss" , "stable@vger.kernel.org" Subject: Re: [Intel-gfx] [PATCH] drm/i915/psr: Chase psr.enabled only under the psr.lock Message-ID: <20180409191432.GN8964@intel.com> References: <20180405114915.29609-1-chris@chris-wilson.co.uk> <7e7b4336aefd6e6cf993150c1a83d214c161a210.camel@intel.com> <20180406221816.GG8964@intel.com> <152309192577.26482.17726023366871782940@mail.alporthouse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <152309192577.26482.17726023366871782940@mail.alporthouse.com> Sender: stable-owner@vger.kernel.org List-ID: On Sat, Apr 07, 2018 at 10:05:25AM +0100, Chris Wilson wrote: > Quoting Rodrigo Vivi (2018-04-06 23:18:16) > > On Fri, Apr 06, 2018 at 11:12:27AM -0700, Souza, Jose wrote: > > > On Thu, 2018-04-05 at 12:49 +0100, Chris Wilson wrote: > > > > + struct drm_crtc *crtc = > > > > + dp_to_dig_port(intel_dp)->base.base.crtc; > > > > I'm afraid that the issue is this pointer here. So this will only mask > > the issue. > > > > Should we maybe stash the pipe? :/ > > It's not that bad. pipe cannot change until after psr_disable is called, > right? And psr_disable ensures that this worker is flushed. The current > problem is just the coordination of cancelling the worker, where we may > set psr.enabled to NULL right before the worker grabs it and > dereferences it. > > So if we lock until we have the pipe, we know that dereference chain is > valid, and we know that psr_disable() cannot complete until we complete > the wait. So the pipe remains valid until we return (so long as the pipe > exists when we start). hmm... it makes sense and I have no better suggestion actually. So, as long it really fixes the regression we introduced: Acked-by: Rodrigo Vivi > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx