Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <b4c5e0ae5edd09e56077ad0a098ae896571c3731.camel@redhat.com>
Subject: Re: [PATCH v4 2/8] drm/nouveau: Enable polling even if we have
 runtime PM
From:   Lyude Paul <lyude@redhat.com>
To:     Lukas Wunner <lukas@wunner.de>
Cc:     nouveau@lists.freedesktop.org,
        Peter Ujfalusi <peter.ujfalusi@ti.com>, stable@vger.kernel.org,
        Ben Skeggs <bskeggs@redhat.com>,
        David Airlie <airlied@linux.ie>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Date:   Thu, 02 Aug 2018 15:37:12 -0400
In-Reply-To: <20180802191405.GB6180@wunner.de>
References: <20180801211459.7731-1-lyude@redhat.com>
         <20180801211459.7731-3-lyude@redhat.com> <20180802191405.GB6180@wunner.de>
Organization: Red Hat
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

oooh, thanks for pointing that out. I should have grepped for this function
before doing that commit. There also appears to be another duplicate
drm_kms_helper_poll_enable() in nouveau_vga.c which also looks equally
suspecious as being useless. I will fix this up and also remove that other
duplicate call in the next version of this

On Thu, 2018-08-02 at 21:14 +0200, Lukas Wunner wrote:
> On Wed, Aug 01, 2018 at 05:14:52PM -0400, Lyude Paul wrote:
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -592,10 +592,11 @@ 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);
> >  	}
> > +
> > +	/* enable polling for connectors without hpd */
> > +	drm_kms_helper_poll_enable(dev);
> > +
> 
> I'm wondering why drm_kms_helper_poll_enable() is called here at all.
> Does the invocation in nouveau_display_init() not suffice?  Can there
> be a situation when nouveau_display_init() is not called despite there
> being connectors that need to be polled?
> 
> Thanks,
> 
> Lukas
-- 
Cheers,
	Lyude Paul