| Message ID | 1365264303-27813-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 6, 2013 at 12:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > When inlining the actual hpd output probing in > > commit 69787f7da6b2adc4054357a661aaa1701a9ca76f > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Tue Oct 23 18:23:34 2012 +0000 > > drm: run the hpd irq event code directly > > the check for the drm_kms_hlper.poll module option was lost. This > regressed systems where this option is used to work-around output > probing issues (like irq storms). Restore the old behaviour. NAK. It's been a while since I looked at it, but the whole point of this patch set was to be able to disabling polling independently of hpd. If you add this check back, setting poll=0 disables both polling and hpd. I'd suggest we add a separate hpd option to disable hpd. Alex > > Reported-by: George Amanakis <g_amanakis@yahoo.com> > Cc: George Amanakis <g_amanakis@yahoo.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: stable@vger.kernel.org (for 3.8) > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_crtc_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c > index 7b2d378..3260736 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -1067,7 +1067,7 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) > enum drm_connector_status old_status; > bool changed = false; > > - if (!dev->mode_config.poll_enabled) > + if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll) > return; > > mutex_lock(&dev->mode_config.mutex); > -- > 1.7.10.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Apr 8, 2013 at 2:28 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Sat, Apr 6, 2013 at 12:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> When inlining the actual hpd output probing in >> >> commit 69787f7da6b2adc4054357a661aaa1701a9ca76f >> Author: Daniel Vetter <daniel.vetter@ffwll.ch> >> Date: Tue Oct 23 18:23:34 2012 +0000 >> >> drm: run the hpd irq event code directly >> >> the check for the drm_kms_hlper.poll module option was lost. This >> regressed systems where this option is used to work-around output >> probing issues (like irq storms). Restore the old behaviour. > > NAK. It's been a while since I looked at it, but the whole point of > this patch set was to be able to disabling polling independently of > hpd. If you add this check back, setting poll=0 disables both polling > and hpd. I'd suggest we add a separate hpd option to disable hpd. The point for me was that the _driver_ can separate hpd handling from polling. Which it still can with this patch applied. The issue at hand is that the old poll=0 also managed to paper over some hpd handling irq storms and so breaking that is a regression. To fix that we should imo apply this patch. We can revert this again once i915 has the hpd irq storm rate-limiting stuff applied (presuming there's no other bug report for other drivers). Also in 3.9 we have the reworked kms locking, so the usual reason that polling causes cursor/pageflip stalls to set poll=0 evaporated. So imo the only people who should still set poll=0 on 3.9 are those with irq storms. And we've just broken that part ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Mon, Apr 8, 2013 at 8:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > On Mon, Apr 8, 2013 at 2:28 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Sat, Apr 6, 2013 at 12:05 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >>> When inlining the actual hpd output probing in >>> >>> commit 69787f7da6b2adc4054357a661aaa1701a9ca76f >>> Author: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Date: Tue Oct 23 18:23:34 2012 +0000 >>> >>> drm: run the hpd irq event code directly >>> >>> the check for the drm_kms_hlper.poll module option was lost. This >>> regressed systems where this option is used to work-around output >>> probing issues (like irq storms). Restore the old behaviour. >> >> NAK. It's been a while since I looked at it, but the whole point of >> this patch set was to be able to disabling polling independently of >> hpd. If you add this check back, setting poll=0 disables both polling >> and hpd. I'd suggest we add a separate hpd option to disable hpd. > > The point for me was that the _driver_ can separate hpd handling from > polling. Which it still can with this patch applied. > > The issue at hand is that the old poll=0 also managed to paper over > some hpd handling irq storms and so breaking that is a regression. To > fix that we should imo apply this patch. > > We can revert this again once i915 has the hpd irq storm rate-limiting > stuff applied (presuming there's no other bug report for other > drivers). Also in 3.9 we have the reworked kms locking, so the usual > reason that polling causes cursor/pageflip stalls to set poll=0 > evaporated. So imo the only people who should still set poll=0 on 3.9 > are those with irq storms. And we've just broken that part ... There are also a lot of people that have non-HPD capable ports (usually old analog stuff like VGA) that they aren't using and they don't want to waste the CPU cycles polling them, but they still want hpd. I expect we'll get regression reports about the poll option getting broken again (hpd stops working with polling disabled, or monitor detection no longer works) if this patch is applied. Alex
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 7b2d378..3260736 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1067,7 +1067,7 @@ void drm_helper_hpd_irq_event(struct drm_device *dev) enum drm_connector_status old_status; bool changed = false; - if (!dev->mode_config.poll_enabled) + if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll) return; mutex_lock(&dev->mode_config.mutex);
When inlining the actual hpd output probing in commit 69787f7da6b2adc4054357a661aaa1701a9ca76f Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Tue Oct 23 18:23:34 2012 +0000 drm: run the hpd irq event code directly the check for the drm_kms_hlper.poll module option was lost. This regressed systems where this option is used to work-around output probing issues (like irq storms). Restore the old behaviour. Reported-by: George Amanakis <g_amanakis@yahoo.com> Cc: George Amanakis <g_amanakis@yahoo.com> Cc: Dave Airlie <airlied@redhat.com> Cc: stable@vger.kernel.org (for 3.8) Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_crtc_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)