| Message ID | 1437058071-4035-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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
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);
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(-)