Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? J. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Tue, Sep 02, 2014 at 10:41:05AM +0300, Jani Nikula wrote: On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? I don't know anymore. Just kill the WARN_ON() if it makes things confusing? -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Tue, Sep 02, 2014 at 11:16:12AM +0300, Ville Syrjälä wrote: On Tue, Sep 02, 2014 at 10:41:05AM +0300, Jani Nikula wrote: On Mon, 01 Sep 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) Ville? I don't know anymore. Just kill the WARN_ON() if it makes things confusing? Just drop the WARN_ON. I prefer the if() using the status rather than type, as that seems more idiomatic (when looking at our other detection routines). -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: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
Hi! From: ville.syrj...@linux.intel.com Sent: Monday, September 01, 2014 at 12:07 PM When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I tested the posted patch, applying it on top of 3.17-rc2 did solve my boot problems. Just to make sure I also tried a clean 3.17-rc3, which was unable to boot. With this patch applied, it again booted successfully. I'm not quite sure how tags work, but you may add my following tags: Reported-by: Tibor Billes tbil...@gmx.com Tested-by: Tibor Billes tbil...@gmx.com Thank you all for the quick responses and the patch, nice work! :) Tibor ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_tv.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 32186a6..c6b00f20 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -1311,6 +1311,7 @@ intel_tv_detect(struct drm_connector *connector, bool force) { struct drm_display_mode mode; struct intel_tv *intel_tv = intel_attached_tv(connector); + enum drm_connector_status status; int type; DRM_DEBUG_KMS([CONNECTOR:%d:%s] force=%d\n, @@ -1328,15 +1329,21 @@ intel_tv_detect(struct drm_connector *connector, bool force) if (intel_get_load_detect_pipe(connector, mode, tmp, ctx)) { type = intel_tv_detect_type(intel_tv, connector); intel_release_load_detect_pipe(connector, tmp); + status = type 0 ? + connector_status_disconnected : + connector_status_connected; } else - return connector_status_unknown; + status = connector_status_unknown; drm_modeset_drop_locks(ctx); drm_modeset_acquire_fini(ctx); } else return connector-status; - if (type 0) + if (status != connector_status_connected) + return status; + + if (WARN_ON(type 0)) return connector_status_disconnected; intel_tv-type = type; -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. Otherwise, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chrsi -- 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: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Otherwise, Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chrsi -- Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
On Mon, Sep 01, 2014 at 01:36:37PM +0300, Ville Syrjälä wrote: On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote: On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When intel_tv_detect() fails to do load detection it would forget to drop the locks and clean up the acquire context. Fix it up. This is a regression from: commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Mon Aug 11 13:15:35 2014 +0300 drm/i915: Fix locking for intel_enable_pipe_a() v2: Make the code more readable (Chris) Cc: Tibor Billes tbil...@gmx.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Hmm, if we use WARN_ON() you should init type. type is always set in the branch that sets status=connected. Back to thinking about readability and making sure that the WARN_ON never happens with just a glance. Otherwise, the WARN_ON would be better as WARN_ON(unsigned)type = last_tv_type); Or something. Anway, take your pick and slap my r-b on it. :) -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