* Possible regression in 3.17-rc2 in i915 driver
@ 2014-08-31 13:57 Tibor Billes
2014-09-01 6:25 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Tibor Billes @ 2014-08-31 13:57 UTC (permalink / raw)
To: "Ville Syrjälä",
David Airlie, Daniel Vetter, Jani Nikula
Cc: linux-kernel
Hi!
I tried to upgrade my kernel from 3.16 to 3.17-rc2 and I found that my laptop
was unable to boot. The boot process hangs after 2-3 seconds (according to
timestamps of log messages). The last kernel log line I usually see is the
discovery of my touchpad: "input: SynPS/2 Synaptics TouchPad as
/devices/platform/i8042/serio2/input/input11".
I did a git bisect and it pointed me to the following commit:
commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date: Mon Aug 11 13:15:35 2014 +0300
drm/i915: Fix locking for intel_enable_pipe_a()
intel_enable_pipe_a() gets called with all the modeset locks already
held (by drm_modeset_lock_all()), so trying to grab the same
locks using another drm_modeset_acquire_ctx is going to fail miserably.
Move most of the drm_modeset_acquire_ctx handling (init/drop/fini)
out from intel_{get,release}_load_detect_pipe() into the callers
(intel_{crt,tv}_detect()). Only the actual locking and backoff
handling is left in intel_get_load_detect_pipe(). And in
intel_enable_pipe_a() we just share the mode_config.acquire_ctx from
drm_modeset_lock_all() which is already holding all the relevant locks.
It's perfectly legal to lock the same ww_mutex multiple times using the
same ww_acquire_ctx. drm_modeset_lock() will convert the returned
-EALREADY into 0, so the caller doesn't need to do antyhing special.
Fixes a hang on resume on my 830.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
I tried booting with the above commit reverted on top of 3.17-rc2 and it
booted successfully.
I also did a MagicSyrq-W (Display list of blocked (D state) tasks) after the
boot process stopped (using plain 3.17-rc2, without reverting the above commit)
and it showed that plymouthd was blocked with the following call trace (hand
copied):
irq_exit
common_interrupt
schedule_preemt_disabled
__mutex_lock_slowpath
mutex_lock
drm_modeset_lock
drm_getconnector
drm_mode_getcrttc
drm_ioctl
...
This may have something to do with the hang or it may not, I don't know but is
related to drm locking so I thought it was a good idea to mention it.
My laptop is an old Fujitsu-Siemens Amilo M1450g, running Linux Mint 16.
Let me know if I can help further debug this issue.
Tibor Billes
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Possible regression in 3.17-rc2 in i915 driver
2014-08-31 13:57 Possible regression in 3.17-rc2 in i915 driver Tibor Billes
@ 2014-09-01 6:25 ` Jani Nikula
2014-09-01 8:09 ` [PATCH] drm/i915: Fix lock dropping in intel_tv_detect() ville.syrjala
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2014-09-01 6:25 UTC (permalink / raw)
To: Tibor Billes, Ville Syrjälä, David Airlie, Daniel Vetter
Cc: linux-kernel, intel-gfx
+intel-gfx
Ville, Daniel, any thoughts before we queue a revert?
BR,
Jani.
On Sun, 31 Aug 2014, Tibor Billes <tbilles@gmx.com> wrote:
> Hi!
>
> I tried to upgrade my kernel from 3.16 to 3.17-rc2 and I found that my laptop
> was unable to boot. The boot process hangs after 2-3 seconds (according to
> timestamps of log messages). The last kernel log line I usually see is the
> discovery of my touchpad: "input: SynPS/2 Synaptics TouchPad as
> /devices/platform/i8042/serio2/input/input11".
>
> I did a git bisect and it pointed me to the following commit:
> commit 208bf9fdcd3575aa4a5d48b3e0295f7cdaf6fc44
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date: Mon Aug 11 13:15:35 2014 +0300
>
> drm/i915: Fix locking for intel_enable_pipe_a()
>
> intel_enable_pipe_a() gets called with all the modeset locks already
> held (by drm_modeset_lock_all()), so trying to grab the same
> locks using another drm_modeset_acquire_ctx is going to fail miserably.
>
> Move most of the drm_modeset_acquire_ctx handling (init/drop/fini)
> out from intel_{get,release}_load_detect_pipe() into the callers
> (intel_{crt,tv}_detect()). Only the actual locking and backoff
> handling is left in intel_get_load_detect_pipe(). And in
> intel_enable_pipe_a() we just share the mode_config.acquire_ctx from
> drm_modeset_lock_all() which is already holding all the relevant locks.
>
> It's perfectly legal to lock the same ww_mutex multiple times using the
> same ww_acquire_ctx. drm_modeset_lock() will convert the returned
> -EALREADY into 0, so the caller doesn't need to do antyhing special.
>
> Fixes a hang on resume on my 830.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> I tried booting with the above commit reverted on top of 3.17-rc2 and it
> booted successfully.
>
> I also did a MagicSyrq-W (Display list of blocked (D state) tasks) after the
> boot process stopped (using plain 3.17-rc2, without reverting the above commit)
> and it showed that plymouthd was blocked with the following call trace (hand
> copied):
> irq_exit
> common_interrupt
> schedule_preemt_disabled
> __mutex_lock_slowpath
> mutex_lock
> drm_modeset_lock
> drm_getconnector
> drm_mode_getcrttc
> drm_ioctl
> ...
>
> This may have something to do with the hang or it may not, I don't know but is
> related to drm locking so I thought it was a good idea to mention it.
>
> My laptop is an old Fujitsu-Siemens Amilo M1450g, running Linux Mint 16.
>
> Let me know if I can help further debug this issue.
>
> Tibor Billes
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 6:25 ` Jani Nikula
@ 2014-09-01 8:09 ` ville.syrjala
2014-09-01 8:50 ` [Intel-gfx] " Chris Wilson
0 siblings, 1 reply; 16+ messages in thread
From: ville.syrjala @ 2014-09-01 8:09 UTC (permalink / raw)
To: intel-gfx
Cc: David Airlie, Daniel Vetter, Jani Nikula, linux-kernel, Tibor Billes
From: Ville Syrjälä <ville.syrjala@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.syrjala@linux.intel.com>
Date: Mon Aug 11 13:15:35 2014 +0300
drm/i915: Fix locking for intel_enable_pipe_a()
Cc: Tibor Billes <tbilles@gmx.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_tv.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..abbf0ea 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool force)
{
struct drm_display_mode mode;
struct intel_tv *intel_tv = intel_attached_tv(connector);
- int type;
+ enum drm_connector_status status = connector->status;
+ int type = -1;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
connector->base.id, connector->name,
@@ -1328,21 +1329,23 @@ 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)
- return connector_status_disconnected;
+ return status;
intel_tv->type = type;
intel_tv_find_better_format(connector);
- return connector_status_connected;
+ return status;
}
static const struct input_res {
--
1.8.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 8:09 ` [PATCH] drm/i915: Fix lock dropping in intel_tv_detect() ville.syrjala
@ 2014-09-01 8:50 ` Chris Wilson
2014-09-01 9:45 ` Ville Syrjälä
2014-09-01 10:07 ` [PATCH v2] " ville.syrjala
0 siblings, 2 replies; 16+ messages in thread
From: Chris Wilson @ 2014-09-01 8:50 UTC (permalink / raw)
To: ville.syrjala
Cc: intel-gfx, David Airlie, Daniel Vetter, linux-kernel, Tibor Billes
On Mon, Sep 01, 2014 at 11:09:46AM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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.syrjala@linux.intel.com>
> Date: Mon Aug 11 13:15:35 2014 +0300
>
> drm/i915: Fix locking for intel_enable_pipe_a()
>
> Cc: Tibor Billes <tbilles@gmx.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_tv.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 32186a6..abbf0ea 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool force)
> {
> struct drm_display_mode mode;
> struct intel_tv *intel_tv = intel_attached_tv(connector);
> - int type;
> + enum drm_connector_status status = connector->status;
> + int type = -1;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
> connector->base.id, connector->name,
> @@ -1328,21 +1329,23 @@ 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)
> - return connector_status_disconnected;
> + return status;
>
> intel_tv->type = type;
> intel_tv_find_better_format(connector);
>
> - return connector_status_connected;
> + return status;
Hmm, this makes the code unclear. if type != -1, status should always be
connected.
I think:
if (status != connector_status_connected)
return status;
if (WARN_ON(type < 0))
return connector_status_disconnected;
intel_tv->type = type;
find_better_format();
return connector_status_connected;
And leave type unset to encourage gcc.
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..a109b7b 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 = connector->status;
int type;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
@@ -1327,17 +1328,20 @@ 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);
+ status = type < 0 ?
+ connector_status_disconnected :
+ connector_status_connected;
+
intel_release_load_detect_pipe(connector, &tmp);
} 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)
- return connector_status_disconnected;
+ if (status != connector_status_connected)
+ return status;
intel_tv->type = type;
intel_tv_find_better_format(connector);
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 8:50 ` [Intel-gfx] " Chris Wilson
@ 2014-09-01 9:45 ` Ville Syrjälä
2014-09-01 10:02 ` Chris Wilson
2014-09-01 10:07 ` [PATCH v2] " ville.syrjala
1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2014-09-01 9:45 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, David Airlie, Daniel Vetter,
linux-kernel, Tibor Billes
On Mon, Sep 01, 2014 at 09:50:22AM +0100, Chris Wilson wrote:
> On Mon, Sep 01, 2014 at 11:09:46AM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@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.syrjala@linux.intel.com>
> > Date: Mon Aug 11 13:15:35 2014 +0300
> >
> > drm/i915: Fix locking for intel_enable_pipe_a()
> >
> > Cc: Tibor Billes <tbilles@gmx.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_tv.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index 32186a6..abbf0ea 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -1311,7 +1311,8 @@ intel_tv_detect(struct drm_connector *connector, bool force)
> > {
> > struct drm_display_mode mode;
> > struct intel_tv *intel_tv = intel_attached_tv(connector);
> > - int type;
> > + enum drm_connector_status status = connector->status;
> > + int type = -1;
> >
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
> > connector->base.id, connector->name,
> > @@ -1328,21 +1329,23 @@ 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)
> > - return connector_status_disconnected;
> > + return status;
> >
> > intel_tv->type = type;
> > intel_tv_find_better_format(connector);
> >
> > - return connector_status_connected;
> > + return status;
>
> Hmm, this makes the code unclear. if type != -1, status should always be
> connected.
>
> I think:
>
> if (status != connector_status_connected)
> return status;
>
> if (WARN_ON(type < 0))
> return connector_status_disconnected;
>
> intel_tv->type = type;
> find_better_format();
> return connector_status_connected;
>
> And leave type unset to encourage gcc.
>
>
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 32186a6..a109b7b 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 = connector->status;
> int type;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
> @@ -1327,17 +1328,20 @@ 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);
> + status = type < 0 ?
> + connector_status_disconnected :
> + connector_status_connected;
> +
> intel_release_load_detect_pipe(connector, &tmp);
> } 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)
> - return connector_status_disconnected;
> + if (status != connector_status_connected)
> + return status;
>
> intel_tv->type = type;
> intel_tv_find_better_format(connector);
With this approach we're going to have to keep the !force else branch
to avoid populating intel_tv->type with stack garbage. But I suppose
the resulting code might still be a bit clearer.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 9:45 ` Ville Syrjälä
@ 2014-09-01 10:02 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2014-09-01 10:02 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, David Airlie, Daniel Vetter, linux-kernel, Tibor Billes
On Mon, Sep 01, 2014 at 12:45:56PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 01, 2014 at 09:50:22AM +0100, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index 32186a6..a109b7b 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 = connector->status;
> > int type;
> >
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
> > @@ -1327,17 +1328,20 @@ 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);
> > + status = type < 0 ?
> > + connector_status_disconnected :
> > + connector_status_connected;
> > +
> > intel_release_load_detect_pipe(connector, &tmp);
> > } 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)
> > - return connector_status_disconnected;
> > + if (status != connector_status_connected)
> > + return status;
> >
> > intel_tv->type = type;
> > intel_tv_find_better_format(connector);
>
> With this approach we're going to have to keep the !force else branch
> to avoid populating intel_tv->type with stack garbage. But I suppose
> the resulting code might still be a bit clearer.
Or just set intel_tv->type directly inside
status = intel_tv_detect_type() ?
Hmm. Indeed, we should not be touching them at all for !forced, as type
is only set for forced, and so we can similarly ignore hpd of better
formats.
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a656816..e80eac5e5239 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1169,7 +1169,7 @@ static const struct drm_display_mode reported_modes[] = {
* \return true if TV is connected.
* \return false if TV is disconnected.
*/
-static int
+static enum drm_connector_status
intel_tv_detect_type(struct intel_tv *intel_tv,
struct drm_connector *connector)
{
@@ -1178,10 +1178,10 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ enum drm_connector_status status;
unsigned long irqflags;
u32 tv_ctl, save_tv_ctl;
u32 tv_dac, save_tv_dac;
- int type;
/* Disable TV interrupts around load detect or we'll recurse */
if (connector->polled & DRM_CONNECTOR_POLL_HPD) {
@@ -1229,7 +1229,6 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
intel_wait_for_vblank(intel_tv->base.base.dev,
to_intel_crtc(intel_tv->base.base.crtc)->pipe);
- type = -1;
tv_dac = I915_READ(TV_DAC);
DRM_DEBUG_KMS("TV detected: %x, %x\n", tv_ctl, tv_dac);
/*
@@ -1238,18 +1237,19 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
* 1 0 X svideo
* 0 0 0 Component
*/
+ status = connector_status_connected;
if ((tv_dac & TVDAC_SENSE_MASK) == (TVDAC_B_SENSE | TVDAC_C_SENSE)) {
DRM_DEBUG_KMS("Detected Composite TV connection\n");
- type = DRM_MODE_CONNECTOR_Composite;
+ intel_tv->type = DRM_MODE_CONNECTOR_Composite;
} else if ((tv_dac & (TVDAC_A_SENSE|TVDAC_B_SENSE)) == TVDAC_A_SENSE) {
DRM_DEBUG_KMS("Detected S-Video TV connection\n");
- type = DRM_MODE_CONNECTOR_SVIDEO;
+ intel_tv->type = DRM_MODE_CONNECTOR_SVIDEO;
} else if ((tv_dac & TVDAC_SENSE_MASK) == 0) {
DRM_DEBUG_KMS("Detected Component TV connection\n");
- type = DRM_MODE_CONNECTOR_Component;
+ intel_tv->type = DRM_MODE_CONNECTOR_Component;
} else {
DRM_DEBUG_KMS("Unrecognised TV connection\n");
- type = -1;
+ status = connector_status_disconnected;
}
I915_WRITE(TV_DAC, save_tv_dac & ~TVDAC_STATE_CHG_EN);
@@ -1269,7 +1269,7 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
- return type;
+ return status;
}
/*
@@ -1309,39 +1309,33 @@ static void intel_tv_find_better_format(struct drm_connector *connector)
static enum drm_connector_status
intel_tv_detect(struct drm_connector *connector, bool force)
{
- struct drm_display_mode mode;
struct intel_tv *intel_tv = intel_attached_tv(connector);
- int type;
+ struct drm_display_mode mode = reported_modes[0];
+ struct intel_load_detect_pipe tmp;
+ struct drm_modeset_acquire_ctx ctx;
+ enum drm_connector_status status;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] force=%d\n",
connector->base.id, connector->name,
force);
+ if (!force)
+ return connector->status;
- mode = reported_modes[0];
-
- if (force) {
- struct intel_load_detect_pipe tmp;
- struct drm_modeset_acquire_ctx ctx;
-
- drm_modeset_acquire_init(&ctx, 0);
-
- 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);
- } else
- return connector_status_unknown;
+ drm_modeset_acquire_init(&ctx, 0);
- drm_modeset_drop_locks(&ctx);
- drm_modeset_acquire_fini(&ctx);
+ if (intel_get_load_detect_pipe(connector, &mode, &tmp, &ctx)) {
+ status = intel_tv_detect_type(intel_tv, connector);
+ intel_release_load_detect_pipe(connector, &tmp);
} else
- return connector->status;
+ status = connector_status_unknown;
- if (type < 0)
- return connector_status_disconnected;
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
- intel_tv->type = type;
- intel_tv_find_better_format(connector);
+ if (status != connector
+ return status;
+ intel_tv_find_better_format(connector);
return connector_status_connected;
}
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 8:50 ` [Intel-gfx] " Chris Wilson
2014-09-01 9:45 ` Ville Syrjälä
@ 2014-09-01 10:07 ` ville.syrjala
2014-09-01 10:20 ` Chris Wilson
2014-09-01 20:19 ` [PATCH v2] " Tibor Billes
1 sibling, 2 replies; 16+ messages in thread
From: ville.syrjala @ 2014-09-01 10:07 UTC (permalink / raw)
To: intel-gfx
Cc: David Airlie, Daniel Vetter, Jani Nikula, linux-kernel,
Chris Wilson, Tibor Billes
From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 10:07 ` [PATCH v2] " ville.syrjala
@ 2014-09-01 10:20 ` Chris Wilson
2014-09-01 10:36 ` Ville Syrjälä
2014-09-01 20:19 ` [PATCH v2] " Tibor Billes
1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2014-09-01 10:20 UTC (permalink / raw)
To: ville.syrjala
Cc: intel-gfx, David Airlie, Daniel Vetter, Jani Nikula,
linux-kernel, Tibor Billes
On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Hmm, if we use WARN_ON() you should init type.
Otherwise,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chrsi
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 10:20 ` Chris Wilson
@ 2014-09-01 10:36 ` Ville Syrjälä
2014-09-01 10:43 ` Chris Wilson
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2014-09-01 10:36 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, David Airlie, Daniel Vetter,
Jani Nikula, linux-kernel, Tibor Billes
On Mon, Sep 01, 2014 at 11:20:09AM +0100, Chris Wilson wrote:
> On Mon, Sep 01, 2014 at 01:07:40PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@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 <chris@chris-wilson.co.uk>
> -Chrsi
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 10:36 ` Ville Syrjälä
@ 2014-09-01 10:43 ` Chris Wilson
2014-09-02 7:41 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2014-09-01 10:43 UTC (permalink / raw)
To: Ville Syrjälä
Cc: intel-gfx, David Airlie, Daniel Vetter, Jani Nikula,
linux-kernel, Tibor Billes
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.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 10:07 ` [PATCH v2] " ville.syrjala
2014-09-01 10:20 ` Chris Wilson
@ 2014-09-01 20:19 ` Tibor Billes
1 sibling, 0 replies; 16+ messages in thread
From: Tibor Billes @ 2014-09-01 20:19 UTC (permalink / raw)
To: ville.syrjala
Cc: intel-gfx, David Airlie, Daniel Vetter, Jani Nikula,
linux-kernel, Chris Wilson
Hi!
From: ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@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 <tbilles@gmx.com>
Tested-by: Tibor Billes <tbilles@gmx.com>
Thank you all for the quick responses and the patch, nice work! :)
Tibor
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-01 10:43 ` Chris Wilson
@ 2014-09-02 7:41 ` Jani Nikula
2014-09-02 8:16 ` Ville Syrjälä
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2014-09-02 7:41 UTC (permalink / raw)
To: Chris Wilson, Ville Syrjälä
Cc: intel-gfx, David Airlie, Daniel Vetter, linux-kernel, Tibor Billes
On Mon, 01 Sep 2014, Chris Wilson <chris@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.syrjala@linux.intel.com wrote:
>> > > From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
>> > > Signed-off-by: Ville Syrjälä <ville.syrjala@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-02 7:41 ` Jani Nikula
@ 2014-09-02 8:16 ` Ville Syrjälä
2014-09-02 8:26 ` Chris Wilson
0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2014-09-02 8:16 UTC (permalink / raw)
To: Jani Nikula
Cc: Chris Wilson, intel-gfx, David Airlie, Daniel Vetter,
linux-kernel, Tibor Billes
On Tue, Sep 02, 2014 at 10:41:05AM +0300, Jani Nikula wrote:
> On Mon, 01 Sep 2014, Chris Wilson <chris@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.syrjala@linux.intel.com wrote:
> >> > > From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> >> > > Signed-off-by: Ville Syrjälä <ville.syrjala@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-02 8:16 ` Ville Syrjälä
@ 2014-09-02 8:26 ` Chris Wilson
2014-09-02 9:57 ` [PATCH v3] " ville.syrjala
0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2014-09-02 8:26 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Jani Nikula, intel-gfx, David Airlie, Daniel Vetter,
linux-kernel, Tibor Billes
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 <chris@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.syrjala@linux.intel.com wrote:
> > >> > > From: Ville Syrjälä <ville.syrjala@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.syrjala@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 <tbilles@gmx.com>
> > >> > > Signed-off-by: Ville Syrjälä <ville.syrjala@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-02 8:26 ` Chris Wilson
@ 2014-09-02 9:57 ` ville.syrjala
2014-09-02 11:41 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: ville.syrjala @ 2014-09-02 9:57 UTC (permalink / raw)
To: intel-gfx
Cc: Jani Nikula, Chris Wilson, David Airlie, Daniel Vetter,
linux-kernel, stable, Tibor Billes
From: Ville Syrjälä <ville.syrjala@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.syrjala@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)
v3: Drop WARN_ON(type < 0) (Chris)
Cc: stable@vger.kernel.org
Cc: Tibor Billes <tbilles@gmx.com>
Reported-by: Tibor Billes <tbilles@gmx.com>
Tested-by: Tibor Billes <tbilles@gmx.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_tv.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 32186a6..c69d3ce 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,16 +1329,19 @@ 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)
- return connector_status_disconnected;
+ if (status != connector_status_connected)
+ return status;
intel_tv->type = type;
intel_tv_find_better_format(connector);
--
1.8.5.5
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] drm/i915: Fix lock dropping in intel_tv_detect()
2014-09-02 9:57 ` [PATCH v3] " ville.syrjala
@ 2014-09-02 11:41 ` Jani Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2014-09-02 11:41 UTC (permalink / raw)
To: ville.syrjala, intel-gfx
Cc: Chris Wilson, David Airlie, Daniel Vetter, linux-kernel, stable,
Tibor Billes
On Tue, 02 Sep 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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.syrjala@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)
> v3: Drop WARN_ON(type < 0) (Chris)
>
> Cc: stable@vger.kernel.org
> Cc: Tibor Billes <tbilles@gmx.com>
> Reported-by: Tibor Billes <tbilles@gmx.com>
> Tested-by: Tibor Billes <tbilles@gmx.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-fixes, thanks for the patch, review, and testing.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/intel_tv.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 32186a6..c69d3ce 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,16 +1329,19 @@ 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)
> - return connector_status_disconnected;
> + if (status != connector_status_connected)
> + return status;
>
> intel_tv->type = type;
> intel_tv_find_better_format(connector);
> --
> 1.8.5.5
>
--
Jani Nikula, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-09-02 11:41 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-31 13:57 Possible regression in 3.17-rc2 in i915 driver Tibor Billes
2014-09-01 6:25 ` Jani Nikula
2014-09-01 8:09 ` [PATCH] drm/i915: Fix lock dropping in intel_tv_detect() ville.syrjala
2014-09-01 8:50 ` [Intel-gfx] " Chris Wilson
2014-09-01 9:45 ` Ville Syrjälä
2014-09-01 10:02 ` Chris Wilson
2014-09-01 10:07 ` [PATCH v2] " ville.syrjala
2014-09-01 10:20 ` Chris Wilson
2014-09-01 10:36 ` Ville Syrjälä
2014-09-01 10:43 ` Chris Wilson
2014-09-02 7:41 ` Jani Nikula
2014-09-02 8:16 ` Ville Syrjälä
2014-09-02 8:26 ` Chris Wilson
2014-09-02 9:57 ` [PATCH v3] " ville.syrjala
2014-09-02 11:41 ` Jani Nikula
2014-09-01 20:19 ` [PATCH v2] " Tibor Billes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).