[PATCH 1/2] drm: Stop resetting connector state to unknown

[PATCH 1/2] drm: Stop resetting connector state to unknown

[Daniel Vetter]

It's causing piles of issues since we've stopped forcing full detect
cycles in the sysfs interfaces with

commit c484f02d0f02fbbfc6decc945a69aae011041a27
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Mar 6 12:36:42 2015 +0000

    drm: Lighten sysfs connector 'status'

The original justification for this was that the hpd handlers could
use the unknown state as a hint to force a full detection. But current
i915 code isn't doing that any more, and no one else really uses reset
on resume. So instead just keep the old state around.

References: http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/62584
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100641
Cc: Rui Matos <tiagomatos@gmail.com>
Cc: Julien Wajsberg <felash@gmail.com>
Cc: kuddel.mail@gmx.de
Cc: Lennart Poettering <mzxreary@0pointer.de>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1f0da41ae2a1..c91c18b2b1d4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5273,12 +5273,9 @@ void drm_mode_config_reset(struct drm_device *dev)
 		if (encoder->funcs->reset)
 			encoder->funcs->reset(encoder);
 
-	drm_for_each_connector(connector, dev) {
-		connector->status = connector_status_unknown;
-
+	drm_for_each_connector(connector, dev)
 		if (connector->funcs->reset)
 			connector->funcs->reset(connector);
-	}
 }
 EXPORT_SYMBOL(drm_mode_config_reset);
 
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/i915: Don't reprobe on resume

[Daniel Vetter]

If we don't force the connector state to unknown there's no reason any
more to force a reprobe. Also no other driver bothers with this, so
probably it's not required - userspace handles lid/resume events
through other channels already.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6f6fdb44e17a..8e86da63cf5d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -733,8 +733,6 @@ static int i915_drm_resume(struct drm_device *dev)
 	 * notifications.
 	 * */
 	intel_hpd_init(dev_priv);
-	/* Config may have changed between suspend and resume */
-	drm_helper_hpd_irq_event(dev);
 
 	intel_opregion_init(dev);
 
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Chris Wilson]

On Thu, Jul 16, 2015 at 04:47:51PM +0200, Daniel Vetter wrote:
> If we don't force the connector state to unknown there's no reason any
> more to force a reprobe. Also no other driver bothers with this, so
> probably it's not required - userspace handles lid/resume events
> through other channels already.

No, we don't. We don't synthesize any events at all for changing
connectors whilst suspended and userspace doesn't know about being
suspended.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Rui Tiago Cação Matos]

On Thu, Jul 16, 2015 at 5:32 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Jul 16, 2015 at 04:47:51PM +0200, Daniel Vetter wrote:
>> If we don't force the connector state to unknown there's no reason any
>> more to force a reprobe. Also no other driver bothers with this, so
>> probably it's not required - userspace handles lid/resume events
>> through other channels already.
>
> No, we don't. We don't synthesize any events at all for changing
> connectors whilst suspended

I haven't tried this patch yet but note that even the current kernel
(4.1.2) isn't sending HOTPLUG uevents out in precisely that case. It
would be nice if that got fixed.

> userspace doesn't know about being
> suspended.

For GNOME, we just rely on systemd to suspend so we do know when we
resume and we can easily trigger a reprobe then. It's up to you to
decide if this should be left to userspace but please
advertise/document it clearly.

Thanks,
Rui
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Daniel Vetter]

On Thu, Jul 16, 2015 at 07:41:14PM +0200, Rui Tiago Cação Matos wrote:
> On Thu, Jul 16, 2015 at 5:32 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, Jul 16, 2015 at 04:47:51PM +0200, Daniel Vetter wrote:
> >> If we don't force the connector state to unknown there's no reason any
> >> more to force a reprobe. Also no other driver bothers with this, so
> >> probably it's not required - userspace handles lid/resume events
> >> through other channels already.
> >
> > No, we don't. We don't synthesize any events at all for changing
> > connectors whilst suspended
> 
> I haven't tried this patch yet but note that even the current kernel
> (4.1.2) isn't sending HOTPLUG uevents out in precisely that case. It
> would be nice if that got fixed.

You only get a hotplug event when something has actually changed, not for
every suspend/resume.

> > userspace doesn't know about being
> > suspended.
> 
> For GNOME, we just rely on systemd to suspend so we do know when we
> resume and we can easily trigger a reprobe then. It's up to you to
> decide if this should be left to userspace but please
> advertise/document it clearly.

I'm not fully sure, but I think all other drivers don't trigger a probe
over suspend/resume either.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Rui Tiago Cação Matos]

On 7/16/15, Daniel Vetter <daniel@ffwll.ch> wrote:
> You only get a hotplug event when something has actually changed, not for
> every suspend/resume.

Sure. But what about the case where something did get
plugged/unplugged while suspended. That's what I was referring to,
sorry for not being clear.

Rui
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Daniel Vetter]

On Thu, Jul 16, 2015 at 04:32:44PM +0100, Chris Wilson wrote:
> On Thu, Jul 16, 2015 at 04:47:51PM +0200, Daniel Vetter wrote:
> > If we don't force the connector state to unknown there's no reason any
> > more to force a reprobe. Also no other driver bothers with this, so
> > probably it's not required - userspace handles lid/resume events
> > through other channels already.
> 
> No, we don't. We don't synthesize any events at all for changing
> connectors whilst suspended and userspace doesn't know about being
> suspended.

The problem is that since 

commit 816da85a0990c2b52cfffa77637d1c770d6790e9
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Oct 23 18:23:33 2012 +0000

    drm: handle HPD and polled connectors separately

this has partially been broken (we only checked hpd connectors and not all
of them), and apparently no one noticed or complained. Also none of the
other drivers check this either. If we really need this then we need to
fix it correctly, but right now I don't see much evidence for this really.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Don't reprobe on resume

[Daniel Vetter]

On Thu, Jul 16, 2015 at 04:32:44PM +0100, Chris Wilson wrote:
> On Thu, Jul 16, 2015 at 04:47:51PM +0200, Daniel Vetter wrote:
> > If we don't force the connector state to unknown there's no reason any
> > more to force a reprobe. Also no other driver bothers with this, so
> > probably it's not required - userspace handles lid/resume events
> > through other channels already.
> 
> No, we don't. We don't synthesize any events at all for changing
> connectors whilst suspended and userspace doesn't know about being
> suspended.

One night of sleep does wonders ;-) I agree the patch is crap and my
thinking that it's been broken since ages is also: We start the poll
helper right away and that will take care of all the non-hpd ports. It's
all fine as-is.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm: Stop resetting connector state to unknown

[Rui Tiago Cação Matos]

On Thu, Jul 16, 2015 at 4:47 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> It's causing piles of issues since we've stopped forcing full detect
> cycles in the sysfs interfaces with

I tested this one on top of 4.1.2 and it does what it says on the tin,
i.e. after resuming I get the connected/disconnected strings on the
/sys/class/drm/*/status files.

Rui