| Message ID | 20170109143158.21917-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > > 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
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
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) { + /* + * 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)
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 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(-)