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);