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;
Date:   Mon, 6 Aug 2018 21:43:03 +0200
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Lukas Wunner <lukas@wunner.de>
Cc:     Lyude Paul <lyude@redhat.com>, Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>, nouveau@lists.freedesktop.org,
        Karol Herbst <karolherbst@gmail.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
Subject: Re: [PATCH v3 3/8] drm/fb_helper: Introduce hotplug_suspend/resume()
Message-ID: <20180806194303.GT3008@phenom.ffwll.local>
Mail-Followup-To: Lukas Wunner <lukas@wunner.de>,
        Lyude Paul <lyude@redhat.com>, David Airlie <airlied@linux.ie>,
        nouveau@lists.freedesktop.org, Karol Herbst <karolherbst@gmail.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
References: <20180731003954.19962-1-lyude@redhat.com>
 <20180731003954.19962-4-lyude@redhat.com>
 <20180806084322.GK3008@phenom.ffwll.local>
 <4e72a10062166d646ebd3eb28b6c186d4c00a63f.camel@redhat.com>
 <20180806193457.GA7028@wunner.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180806193457.GA7028@wunner.de>
User-Agent: Mutt/1.10.0 (2018-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Aug 06, 2018 at 09:34:57PM +0200, Lukas Wunner wrote:
> On Mon, Aug 06, 2018 at 03:15:31PM -0400, Lyude Paul wrote:
> > You did mention in the review of one of my other patches that we should avoid
> > disabling polling during runtime suspend, and you're definitely right. I feel
> > a bit silly for not remembering that since I was the one who made it so that
> > i915 does polling in runtime suspend for chips without RPM HPD detection in
> > the first place because it was causing people's displays not to come up on
> > vlv...
> > Anyway: I think if we just leave output polling enabled during runtime suspend
> > that might actually fix all of the fb_helper locking issues since we won't
> > need to wait on any of the output poll workers to finish, at least I think it
> > should: I'll confirm this when I get into the office
> 
> Quoth Imre Deak:
> 
>    "In i915 polling is on during runtime suspend only if there are outputs
>     without hotplug interrupt support. A special case is when an output has
>     working HPD interrupts when in D0, but no interrupts when runtime
>     suspended. For these we start polling (from a scheduled work) in the
>     runtime suspend hook and stop it in the runtime resume hook (again from
>     a scheduled work)."
>     https://lkml.org/lkml/2018/2/12/330
> 
> nouveau only uses runtime PM on discrete GPUs in dual GPU laptops.
> Resuming the GPU from D3cold to D0 every few seconds to poll the
> outputs would waste too much power on such machines.
> 
> The question is, why is polling running at all, since all modern
> laptops have HPD-capable ports such as DP?

Note we don't fully sync with the poll worker here, we just update the
polling flags and let the poll worker lazily notice the change (and stop
itsefl). That avoids the deadlock. It also means that the poll worker must
call pm_runtime_get() in all the right places though.

On resume we need to kick it again, but that doesn't have a deadlock
potential.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch