linux-kernel - Re: [PATCH v2] drm/i915: Send a hotplug event on resume instead of an IRQ event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20151216083350.GD30437@phenom.ffwll.local>
Date:	Wed, 16 Dec 2015 09:33:50 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Lyude <cpaul@...hat.com>
Cc:	David Airlie <airlied@...ux.ie>,
	"open list:DRM DRIVERS" <dri-devel@...ts.freedesktop.org>,
	open list <linux-kernel@...r.kernel.org>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	Rui Matos <tiagomatos@...il.com>,
	Julien Wajsberg <felash@...il.com>, kuddel.mail@....de,
	Lennart Poettering <mzxreary@...inter.de>,
	Rob Clark <robdclark@...il.com>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>
Subject: Re: [PATCH v2] drm/i915: Send a hotplug event on resume instead of
 an IRQ event

On Tue, Dec 15, 2015 at 05:25:56PM -0500, Lyude wrote:
> We currently call drm_helper_hpd_irq_event() to handle reprobing
> displays on resume, however drm_helper_hpd_irq_event() only checks the
> status of hpd. HPD doesn't update if the displays connected changed
> before resuming the system, and as such causes us not to detect any
> connection changes after resume.
> 
> This originally wasn't the case since drm would reset all of the
> connectors to unknown during resume and cause a reprobe (since the
> connector statuses would change), however this behavior changed in
> 5677d67ae394 ('drm: Stop resetting connector state to unknown').
> 
> Signed-off-by: Lyude <cpaul@...hat.com>
> ---
> I did some research into why setting the connector state to unknown was
> causing a reprobe and discovered that the fact that this reprobing ever
> worked at all was just a coincidence (i915's IRQ handler gets called by the
> irq event, notices the state of all the connectors changed since they were
> set to unknown, causes a reprobe). This seems to fix everything (including
> monitor detection in normal VTs), and I can't figure out any real use now
> for us to set everything to unknown so I'm going to go the route of just
> sending the hotplug event instead.
> 
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 760e0ce..27a6c1b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -792,7 +792,7 @@ static int i915_drm_resume(struct drm_device *dev)
>  	 * */
>  	intel_hpd_init(dev_priv);
>  	/* Config may have changed between suspend and resume */
> -	drm_helper_hpd_irq_event(dev);
> +	drm_kms_helper_hotplug_event(dev);

So there's a bunch of things broken in this area, but just from your
description it's unclear what exactly. Problems I'm aware of:

- hpd_irq_event() only reprobes connectors for which hpd is enabled, which
  are not all of them. We broke this in

  commit 816da85a0990c2b52cfffa77637d1c770d6790e9
  Author: Daniel Vetter <daniel.vetter@...ll.ch>
  Date:   Tue Oct 23 18:23:33 2012 +0000
  
      drm: handle HPD and polled connectors separately

  which is positively ages ago. Also note that any other driver not using
  vt switching also suffers from this (atm radeon, nouveau, rockchip,
  amdgpu). otoh at least from a quick those drivers also don't really
  bother with reprobing on resume really.

- DP MST reprobing is busted in i915 since

  commit e7d6f7d708290da1b7c92f533444b042c79412e0
  Author: Dave Airlie <airlied@...hat.com>
  Date:   Mon Dec 8 13:23:37 2014 +1000
  
      drm/i915: resume MST after reading back hw state

  There's some bug reports iirc from Takashi and Ted Tso about this. Your
  patch won't help for DP MST since we never reprobe MST state from
  userspace, it only ever happens through interrupt handling. Well mostly,
  if you completely unplug something then we detect that and throw all the
  MST stuff over board, but if you e.g. just change a connection in your
  dock or go from one dock to another we won't detect this.

Which one of the above is it? And to what extend do you care about this
working everywhere?

Sorry that I'm opening a can of worms here.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/