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
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
