From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [PATCH] drm/nouveau: Fix drm poll_helper handling Date: Mon, 15 May 2017 10:11:36 -0400 Message-ID: <1494857496.14698.0.camel@redhat.com> References: <20170515090431.32325-1-peter.ujfalusi@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170515090431.32325-1-peter.ujfalusi@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Peter Ujfalusi , bskeggs@redhat.com, airlied@linux.ie Cc: nouveau@lists.freedesktop.org, Gleb Nemshilov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Dave Airlie List-Id: nouveau.vger.kernel.org R29vZCBjYXRjaCEKClJldmlld2VkLWJ5OiBMeXVkZSA8bHl1ZGVAcmVkaGF0LmNvbT4KCk9uIE1v biwgMjAxNy0wNS0xNSBhdCAxMjowNCArMDMwMCwgUGV0ZXIgVWpmYWx1c2kgd3JvdGU6Cj4gQ29t bWl0IGNhZTlmZjAzNmVlYSBlZmZlY3RpdmVseSBkaXNhYmxlZCB0aGUgZHJtIHBvbGxfaGVscGVy IGJ5Cj4gY2hlY2tpbmcKPiB0aGUgd3JvbmcgZmxhZyB0byBzZWUgaWYgdGhlIGRyaXZlciBzaG91 bGQgZW5hYmxlIHRoZSBwb2xsIG9yIG5vdDoKPiBtb2RlX2NvbmZpZy5wb2xsX2VuYWJsZWQgaXMg b25seSBzZXQgdG8gdHJ1ZSBieSBwb2xsX2luaXQgYW5kIGl0IGlzCj4gbm90Cj4gaW5kaWNhdGlu ZyBpZiB0aGUgcG9sbCBpcyBlbmFibGVkIG9yIG5vdC4KPiBub3V2ZWF1X2Rpc3BsYXlfY3JlYXRl KCkgd2lsbCBpbml0aWFsaXplIHRoZSBwb2xsIGFuZCBnb2luZyB0bwo+IGRpc2FibGUgaXQKPiBy aWdodCBhd2F5LiBBZnRlciBwb2xsX2luaXQoKSB0aGUgbW9kZV9jb25maWcucG9sbF9lbmFibGVk IHdpbGwgYmUKPiB0cnVlLAo+IGJ1dCB0aGUgcG9sbCBpdHNlbGYgaXMgZGlzYWJsZWQuCj4gCj4g VG8gYXZvaWQgdGhlIHJhY2UgY2F1c2VkIGJ5IGNhbGxpbmcgdGhlIHBvbGxfZW5hYmxlKCkgZnJv bSBkaWZmZXJlbnQKPiBwYXRocywKPiB0aGlzIHBhdGNoIHdpbGwgZW5hYmxlIHRoZSBwb2xsIGZy b20gb25lIHBsYWNlLCBpbiB0aGUKPiBub3V2ZWF1X2Rpc3BsYXlfaHBkX3dvcmsoKS4KPiAKPiBJ biBjYXNlIHRoZSBwbV9ydW50aW1lIGlzIGRpc2FibGVkIHdlIHdpbGwgZW5hYmxlIHRoZSBwb2xs IGluCj4gbm91dmVhdV9kcm1fbG9hZCgpIG9uY2UuCj4gCj4gRml4ZXM6IGNhZTlmZjAzNmVlYSAo ImRybS9ub3V2ZWF1OiBEb24ndCBlbmFibGluZyBwb2xsaW5nIHR3aWNlIG9uCj4gcnVudGltZSBy ZXN1bWUiKQo+IFNpZ25lZC1vZmYtYnk6IFBldGVyIFVqZmFsdXNpIDxwZXRlci51amZhbHVzaUB0 aS5jb20+Cj4gQ2M6IEx5dWRlIFBhdWwgPGx5dWRlQHJlZGhhdC5jb20+Cj4gQ2M6IERhdmUgQWly bGllIDxhaXJsaWVkQHJlZGhhdC5jb20+Cj4gQ2M6IEdsZWIgTmVtc2hpbG92IDxnbGViQGZhc3Rt YWlsLmNvbT4KPiAtLS0KPiBIaSwKPiAKPiB0aGlzIHBhdGNoIHdhcyBjcmVhdGVkIGFuZCB0ZXN0 ZWQgaW4gY29ubmVjdGlvbiBvZjoKPiBodHRwczovL2J1Z3MuZnJlZWRlc2t0b3Aub3JnL3Nob3df YnVnLmNnaT9pZD05ODY5MAo+IAo+IFJlZ2FyZHMsCj4gUGV0ZXIKPiAKPiDCoGRyaXZlcnMvZ3B1 L2RybS9ub3V2ZWF1L25vdXZlYXVfZGlzcGxheS5jIHwgNiArKy0tLS0KPiDCoGRyaXZlcnMvZ3B1 L2RybS9ub3V2ZWF1L25vdXZlYXVfZHJtLmPCoMKgwqDCoMKgfCA2ICsrKy0tLQo+IMKgMiBmaWxl cyBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvZ3B1L2RybS9ub3V2ZWF1L25vdXZlYXVfZGlzcGxheS5jCj4gYi9kcml2ZXJz L2dwdS9kcm0vbm91dmVhdS9ub3V2ZWF1X2Rpc3BsYXkuYwo+IGluZGV4IDIxYjEwZjk4NDBjOS4u NTQ5NzYzZjVlMTdkIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9ub3V2ZWF1L25vdXZl YXVfZGlzcGxheS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL25vdXZlYXUvbm91dmVhdV9kaXNw bGF5LmMKPiBAQCAtMzYwLDYgKzM2MCw4IEBAIG5vdXZlYXVfZGlzcGxheV9ocGRfd29yayhzdHJ1 Y3Qgd29ya19zdHJ1Y3QKPiAqd29yaykKPiDCoAlwbV9ydW50aW1lX2dldF9zeW5jKGRybS0+ZGV2 LT5kZXYpOwo+IMKgCj4gwqAJZHJtX2hlbHBlcl9ocGRfaXJxX2V2ZW50KGRybS0+ZGV2KTsKPiAr CS8qIGVuYWJsZSBwb2xsaW5nIGZvciBleHRlcm5hbCBkaXNwbGF5cyAqLwo+ICsJZHJtX2ttc19o ZWxwZXJfcG9sbF9lbmFibGUoZHJtLT5kZXYpOwo+IMKgCj4gwqAJcG1fcnVudGltZV9tYXJrX2xh c3RfYnVzeShkcm0tPmRldi0+ZGV2KTsKPiDCoAlwbV9ydW50aW1lX3B1dF9zeW5jKGRybS0+ZGV2 LT5kZXYpOwo+IEBAIC00MTMsMTAgKzQxNSw2IEBAIG5vdXZlYXVfZGlzcGxheV9pbml0KHN0cnVj dCBkcm1fZGV2aWNlICpkZXYpCj4gwqAJaWYgKHJldCkKPiDCoAkJcmV0dXJuIHJldDsKPiDCoAo+ IC0JLyogZW5hYmxlIHBvbGxpbmcgZm9yIGV4dGVybmFsIGRpc3BsYXlzICovCj4gLQlpZiAoIWRl di0+bW9kZV9jb25maWcucG9sbF9lbmFibGVkKQo+IC0JCWRybV9rbXNfaGVscGVyX3BvbGxfZW5h YmxlKGRldik7Cj4gLQo+IMKgCS8qIGVuYWJsZSBob3RwbHVnIGludGVycnVwdHMgKi8KPiDCoAls aXN0X2Zvcl9lYWNoX2VudHJ5KGNvbm5lY3RvciwgJmRldi0KPiA+bW9kZV9jb25maWcuY29ubmVj dG9yX2xpc3QsIGhlYWQpIHsKPiDCoAkJc3RydWN0IG5vdXZlYXVfY29ubmVjdG9yICpjb25uID0K PiBub3V2ZWF1X2Nvbm5lY3Rvcihjb25uZWN0b3IpOwo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dw dS9kcm0vbm91dmVhdS9ub3V2ZWF1X2RybS5jCj4gYi9kcml2ZXJzL2dwdS9kcm0vbm91dmVhdS9u b3V2ZWF1X2RybS5jCj4gaW5kZXggMmI2YWMyNGNlNjkwLi4zNjI2OGUxODAyYjUgMTAwNjQ0Cj4g LS0tIGEvZHJpdmVycy9ncHUvZHJtL25vdXZlYXUvbm91dmVhdV9kcm0uYwo+ICsrKyBiL2RyaXZl cnMvZ3B1L2RybS9ub3V2ZWF1L25vdXZlYXVfZHJtLmMKPiBAQCAtNTAyLDYgKzUwMiw5IEBAIG5v dXZlYXVfZHJtX2xvYWQoc3RydWN0IGRybV9kZXZpY2UgKmRldiwgdW5zaWduZWQKPiBsb25nIGZs YWdzKQo+IMKgCQlwbV9ydW50aW1lX2FsbG93KGRldi0+ZGV2KTsKPiDCoAkJcG1fcnVudGltZV9t YXJrX2xhc3RfYnVzeShkZXYtPmRldik7Cj4gwqAJCXBtX3J1bnRpbWVfcHV0KGRldi0+ZGV2KTsK PiArCX0gZWxzZSB7Cj4gKwkJLyogZW5hYmxlIHBvbGxpbmcgZm9yIGV4dGVybmFsIGRpc3BsYXlz ICovCj4gKwkJZHJtX2ttc19oZWxwZXJfcG9sbF9lbmFibGUoZGV2KTsKPiDCoAl9Cj4gwqAJcmV0 dXJuIDA7Cj4gwqAKPiBAQCAtNzc0LDkgKzc3Nyw2IEBAIG5vdXZlYXVfcG1vcHNfcnVudGltZV9y ZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+IMKgCj4gwqAJcmV0ID0gbm91dmVhdV9kb19yZXN1 bWUoZHJtX2RldiwgdHJ1ZSk7Cj4gwqAKPiAtCWlmICghZHJtX2Rldi0+bW9kZV9jb25maWcucG9s bF9lbmFibGVkKQo+IC0JCWRybV9rbXNfaGVscGVyX3BvbGxfZW5hYmxlKGRybV9kZXYpOwo+IC0K PiDCoAkvKiBkbyBtYWdpYyAqLwo+IMKgCW52aWZfbWFzaygmZGV2aWNlLT5vYmplY3QsIDB4MDg4 NDg4LCAoMSA8PCAyNSksICgxIDw8IDI1KSk7Cj4gwqAJdmdhX3N3aXRjaGVyb29fc2V0X2R5bmFt aWNfc3dpdGNoKHBkZXYsIFZHQV9TV0lUQ0hFUk9PX09OKTsKX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2 ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21h aWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934536AbdEOOLq (ORCPT ); Mon, 15 May 2017 10:11:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33746 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933430AbdEOOLo (ORCPT ); Mon, 15 May 2017 10:11:44 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CD5F26438B Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lyude@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com CD5F26438B Message-ID: <1494857496.14698.0.camel@redhat.com> Subject: Re: [PATCH] drm/nouveau: Fix drm poll_helper handling From: Lyude Paul To: Peter Ujfalusi , bskeggs@redhat.com, airlied@linux.ie Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org, linux-kernel@vger.kernel.org, Dave Airlie , Gleb Nemshilov Date: Mon, 15 May 2017 10:11:36 -0400 In-Reply-To: <20170515090431.32325-1-peter.ujfalusi@ti.com> References: <20170515090431.32325-1-peter.ujfalusi@ti.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 15 May 2017 14:11:39 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Good catch! Reviewed-by: Lyude On Mon, 2017-05-15 at 12:04 +0300, Peter Ujfalusi wrote: > Commit cae9ff036eea effectively disabled the drm poll_helper by > checking > the wrong flag to see if the driver should enable the poll or not: > mode_config.poll_enabled is only set to true by poll_init and it is > not > indicating if the poll is enabled or not. > nouveau_display_create() will initialize the poll and going to > disable it > right away. After poll_init() the mode_config.poll_enabled will be > true, > but the poll itself is disabled. > > To avoid the race caused by calling the poll_enable() from different > paths, > this patch will enable the poll from one place, in the > nouveau_display_hpd_work(). > > In case the pm_runtime is disabled we will enable the poll in > nouveau_drm_load() once. > > Fixes: cae9ff036eea ("drm/nouveau: Don't enabling polling twice on > runtime resume") > Signed-off-by: Peter Ujfalusi > Cc: Lyude Paul > Cc: Dave Airlie > Cc: Gleb Nemshilov > --- > Hi, > > this patch was created and tested in connection of: > https://bugs.freedesktop.org/show_bug.cgi?id=98690 > > Regards, > Peter > >  drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++---- >  drivers/gpu/drm/nouveau/nouveau_drm.c     | 6 +++--- >  2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c > b/drivers/gpu/drm/nouveau/nouveau_display.c > index 21b10f9840c9..549763f5e17d 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > @@ -360,6 +360,8 @@ nouveau_display_hpd_work(struct work_struct > *work) >   pm_runtime_get_sync(drm->dev->dev); >   >   drm_helper_hpd_irq_event(drm->dev); > + /* enable polling for external displays */ > + drm_kms_helper_poll_enable(drm->dev); >   >   pm_runtime_mark_last_busy(drm->dev->dev); >   pm_runtime_put_sync(drm->dev->dev); > @@ -413,10 +415,6 @@ nouveau_display_init(struct drm_device *dev) >   if (ret) >   return ret; >   > - /* enable polling for external displays */ > - if (!dev->mode_config.poll_enabled) > - drm_kms_helper_poll_enable(dev); > - >   /* enable hotplug interrupts */ >   list_for_each_entry(connector, &dev- > >mode_config.connector_list, head) { >   struct nouveau_connector *conn = > nouveau_connector(connector); > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 2b6ac24ce690..36268e1802b5 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -502,6 +502,9 @@ nouveau_drm_load(struct drm_device *dev, unsigned > long flags) >   pm_runtime_allow(dev->dev); >   pm_runtime_mark_last_busy(dev->dev); >   pm_runtime_put(dev->dev); > + } else { > + /* enable polling for external displays */ > + drm_kms_helper_poll_enable(dev); >   } >   return 0; >   > @@ -774,9 +777,6 @@ nouveau_pmops_runtime_resume(struct device *dev) >   >   ret = nouveau_do_resume(drm_dev, true); >   > - if (!drm_dev->mode_config.poll_enabled) > - drm_kms_helper_poll_enable(drm_dev); > - >   /* do magic */ >   nvif_mask(&device->object, 0x088488, (1 << 25), (1 << 25)); >   vga_switcheroo_set_dynamic_switch(pdev, VGA_SWITCHEROO_ON);