From: Daniel Vetter <daniel@ffwll.ch>
To: Sean Paul <seanpaul@chromium.org>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>,
freedesktop-bugs@paulsd.com, gleb@fastmail.com,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Daniel Vetter <daniel.vetter@intel.com>,
stable@vger.kernel.org
Subject: Re: [PATCH v2] drm: Schedule the output_poll_work with 1s delay if we have delayed event
Date: Tue, 10 Jan 2017 11:43:01 +0100 [thread overview]
Message-ID: <20170110104301.rr2tc5p2kajbglsz@phenom.ffwll.local> (raw)
In-Reply-To: <20170110104059.drkmxkldy2fcb7mb@phenom.ffwll.local>
On Tue, Jan 10, 2017 at 11:40:59AM +0100, Daniel Vetter wrote:
> On Mon, Jan 09, 2017 at 11:50:59AM -0500, Sean Paul wrote:
> > On Mon, Jan 9, 2017 at 9:31 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> > > Instead of scheduling the work to handle the initial delayed event, use 1s
> > > delay.
> > >
> > > This delay should not be needed, but Optimus/nouveau will fail in a
> > > mysterious way if the delayed event is handled as soon as possible like it
> >
> > Has anyone tried to demystify the failure? It seems like fixing the
> > root problem would be better than this.
>
> Peter is on it, but fixing the regression meanwhile has priority imo.
>
> > Perhaps we should just revert 339fd36238dd to fix stable.
>
> That will make people unhappy about the delay again, so I think 1s delay
> is the better option.
For the future: If you add an in-patch changelog then it'd would have been
clear that we discussed all this already. Also good to cc all previous
commenters when submitting a new version.
-Daniel
>
> >
> > Sean
> >
> > > is done in drm_helper_probe_single_connector_modes() in case the poll
> > > was enabled before.
> > >
> > > Reverting 339fd36238dd would give back the 10 sec (!) delay to handle the
> > > delayed event. Adding 1sec delay to the poll_work is enough to work around
> > > the issue in Optimus setups and gives shorter response on handling the
> > > initial delayed event.
> > >
> > > Fixes: 339fd36238dd ("drm: drm_probe_helper: Fix output_poll_work scheduling")
> > > Cc: stable@vger.kernel.org # v4.9
> > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > > ---
> > > drivers/gpu/drm/drm_probe_helper.c | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > > index 06a62e37fbdc..258abed43e38 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -146,8 +146,16 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
> > > drm_connector_list_iter_put(&conn_iter);
> > >
> > > if (dev->mode_config.delayed_event) {
> > > + /*
>
> I added a FIXME: heading here to make it stick out more, and then applied
> the patch.
>
> Thanks, Daniel
>
> > > + * Use short (1s) delay to handle the initial delayed event.
> > > + * This delay should not be needed, but Optimus/nouveau will
> > > + * fail in a mysterious way if the delayed event is handled as
> > > + * soon as possible like it is done in
> > > + * drm_helper_probe_single_connector_modes() in case the poll
> > > + * was enabled before.
> > > + */
> > > poll = true;
> > > - delay = 0;
> > > + delay = HZ;
> > > }
> > >
> > > if (poll)
> > > --
> > > 2.11.0
> > >
> >
> >
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
prev parent reply other threads:[~2017-01-10 10:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-09 14:31 [PATCH v2] drm: Schedule the output_poll_work with 1s delay if we have delayed event Peter Ujfalusi
2017-01-09 16:50 ` Sean Paul
2017-01-10 10:40 ` Daniel Vetter
2017-01-10 10:43 ` Daniel Vetter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170110104301.rr2tc5p2kajbglsz@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedesktop-bugs@paulsd.com \
--cc=gleb@fastmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peter.ujfalusi@ti.com \
--cc=seanpaul@chromium.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).