From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Wunner Subject: Re: [PATCH] drm/i915/fbdev: Serialise early hotplug events with async fbdev config Date: Sun, 26 Nov 2017 12:49:19 +0100 Message-ID: <20171126114919.GA10063@wunner.de> References: <20171125194155.355-1-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from bmailout1.hostsharing.net (bmailout1.hostsharing.net [83.223.95.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7B1346E155 for ; Sun, 26 Nov 2017 11:49:21 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20171125194155.355-1-chris@chris-wilson.co.uk> 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: Daniel Vetter , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gU2F0LCBOb3YgMjUsIDIwMTcgYXQgMDc6NDE6NTVQTSArMDAwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IEBAIC02OTcsMTAgKzY5Nyw4IEBAIHN0YXRpYyB2b2lkIGludGVsX2ZiZGV2X2luaXRp YWxfY29uZmlnKHZvaWQgKmRhdGEsIGFzeW5jX2Nvb2tpZV90IGNvb2tpZSkKPiAgCj4gIAkvKiBE dWUgdG8gcGVjdWxpYXIgaW5pdCBvcmRlciB3cnQgdG8gaHBkIGhhbmRsaW5nIHRoaXMgaXMgc2Vw YXJhdGUuICovCj4gIAlpZiAoZHJtX2ZiX2hlbHBlcl9pbml0aWFsX2NvbmZpZygmaWZiZGV2LT5o ZWxwZXIsCj4gLQkJCQkJIGlmYmRldi0+cHJlZmVycmVkX2JwcCkpIHsKPiArCQkJCQkgaWZiZGV2 LT5wcmVmZXJyZWRfYnBwKSkKPiAgCQlpbnRlbF9mYmRldl91bnJlZ2lzdGVyKHRvX2k5MTUoaWZi ZGV2LT5oZWxwZXIuZGV2KSk7Cj4gLQkJaW50ZWxfZmJkZXZfZmluaSh0b19pOTE1KGlmYmRldi0+ aGVscGVyLmRldikpOwo+IC0JfQo+ICB9CgpIbSwgdGhlIHJhY2UgYXQgaGFuZCB3b3VsZCBiZSBz b2x2ZWQgYnkgdGhlIGludGVsX2ZiZGV2X3N5bmMoKSBiZWxvdywKb3IgYW0gSSBtaXNzaW5nIHNv bWV0aGluZz8gIFN0aWxsIHdvbmRlcmluZyB3aHkgaXQncyBuZWNlc3NhcnkgdG8KbGVhdmUgdGhl IGZiZGV2IGFyb3VuZC4uLgoKCj4gQEAgLTgwMCw3ICs3OTgsMTEgQEAgdm9pZCBpbnRlbF9mYmRl dl9vdXRwdXRfcG9sbF9jaGFuZ2VkKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYpCj4gIHsKPiAgCXN0 cnVjdCBpbnRlbF9mYmRldiAqaWZiZGV2ID0gdG9faTkxNShkZXYpLT5mYmRldjsKPiAgCj4gLQlp ZiAoaWZiZGV2KQo+ICsJaWYgKCFpZmJkZXYpCj4gKwkJcmV0dXJuOwo+ICsKPiArCWludGVsX2Zi ZGV2X3N5bmMoaWZiZGV2KTsKPiArCWlmIChpZmJkZXYtPnZtYSkKPiAgCQlkcm1fZmJfaGVscGVy X2hvdHBsdWdfZXZlbnQoJmlmYmRldi0+aGVscGVyKTsKPiAgfQoKVGhpcyBodW5rIGxvb2tzIGdv b2QsIGFzIHlvdSBub3RlIHRoZSBzeW5jaHJvbml6YXRpb24gd2FzIGFscmVhZHkgdGhlcmUKYnV0 IGhhZCB0byBiZSByZXZlcnRlZCBiZWNhdXNlIEkgZmFpbGVkIHRvIG5vdGljZSB0aGF0IGEgIisg MSIgbmVlZHMgdG8KYmUgYWRkZWQgdG8gdGhlIGNvb2tpZS4gIFlvdSBkaWQgYSBtdWNoIGJldHRl ciBqb2IgdGhhbiBtZSB1bmRlcnN0YW5kaW5nCmhvdyB0aGUgYXN5bmMgQVBJIHdvcmtzIHdpdGgg NDNjZWUzMTQzNDVhLgoKSG93ZXZlciB0aGUgImlmIChpZmJkZXYtPnZtYSkiIGxvb2tzIGEgYml0 IGZpc2h5LCBpZmJkZXYgY291bGQgYmUgTlVMTAooZS5nLiBpZiBCSU9TIGZiIHdhcyB0b28gc21h bGwgYnV0IGludGVsZmJfYWxsb2MoKSBmYWlsZWQpIHNvIEkgdGhpbmsKdGhpcyBtaWdodCBsZWFk IHRvIGEgbnVsbCBwb2ludGVyIGRlcmVmLiAgRG9lcyBpdCBtYWtlIGEgZGlmZmVyZW5jZQppZiB3 ZSBjaGVjayBmb3IgaWZiZGV2IHZlcnN1cyBpZmJkZXYtPnZtYT8gIEkgYWxzbyBub3RpY2UgdGhh dCB5b3UKYWRkZWQgYSBjaGVjayBmb3IgaWZiZGV2LT52bWEgd2l0aCAxNTcyN2VkMGQ5NDQgYnV0 IERhbmllbCBsYXRlcgpyZW1vdmVkIGl0IHdpdGggODhiZTU4YmU4ODZmLgoKSSBndWVzcyBhIGNo ZWNrICppcyogbmVjZXNzYXJ5IGhlcmUgYmVjYXVzZSBmYmRldiBpbml0aWFsaXphdGlvbiBtaWdo dApoYXZlIGZhaWxlZCwgYnV0IEknZCBqdXN0IGNoZWNrIGZvciAiaWYgKGlmYmRldikiLgoKVGhh bmtzICYgaGF2ZSBhIHBsZWFzYW50IFN1bmRheSBhZnRlcm5vb24uCgpMdWthcwpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBs aXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bmailout1.hostsharing.net ([83.223.95.100]:48097 "EHLO bmailout1.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752033AbdKZLtV (ORCPT ); Sun, 26 Nov 2017 06:49:21 -0500 Date: Sun, 26 Nov 2017 12:49:19 +0100 From: Lukas Wunner To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, Joonas Lahtinen , Daniel Vetter , stable@vger.kernel.org Subject: Re: [PATCH] drm/i915/fbdev: Serialise early hotplug events with async fbdev config Message-ID: <20171126114919.GA10063@wunner.de> References: <20171125194155.355-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171125194155.355-1-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org List-ID: On Sat, Nov 25, 2017 at 07:41:55PM +0000, Chris Wilson wrote: > @@ -697,10 +697,8 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) > > /* Due to peculiar init order wrt to hpd handling this is separate. */ > if (drm_fb_helper_initial_config(&ifbdev->helper, > - ifbdev->preferred_bpp)) { > + ifbdev->preferred_bpp)) > intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); > - intel_fbdev_fini(to_i915(ifbdev->helper.dev)); > - } > } Hm, the race at hand would be solved by the intel_fbdev_sync() below, or am I missing something? Still wondering why it's necessary to leave the fbdev around... > @@ -800,7 +798,11 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev) > { > struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; > > - if (ifbdev) > + if (!ifbdev) > + return; > + > + intel_fbdev_sync(ifbdev); > + if (ifbdev->vma) > drm_fb_helper_hotplug_event(&ifbdev->helper); > } This hunk looks good, as you note the synchronization was already there but had to be reverted because I failed to notice that a "+ 1" needs to be added to the cookie. You did a much better job than me understanding how the async API works with 43cee314345a. However the "if (ifbdev->vma)" looks a bit fishy, ifbdev could be NULL (e.g. if BIOS fb was too small but intelfb_alloc() failed) so I think this might lead to a null pointer deref. Does it make a difference if we check for ifbdev versus ifbdev->vma? I also notice that you added a check for ifbdev->vma with 15727ed0d944 but Daniel later removed it with 88be58be886f. I guess a check *is* necessary here because fbdev initialization might have failed, but I'd just check for "if (ifbdev)". Thanks & have a pleasant Sunday afternoon. Lukas