All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Tibor Billes <tbilles@gmx.com>
Subject: Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
Date: Tue, 2 Sep 2014 11:16:12 +0300	[thread overview]
Message-ID: <20140902081612.GH4193@intel.com> (raw)
In-Reply-To: <87egvuii5q.fsf@intel.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	linux-kernel@vger.kernel.org, Tibor Billes <tbilles@gmx.com>
Subject: Re: [PATCH v2] drm/i915: Fix lock dropping in intel_tv_detect()
Date: Tue, 2 Sep 2014 11:16:12 +0300	[thread overview]
Message-ID: <20140902081612.GH4193@intel.com> (raw)
In-Reply-To: <87egvuii5q.fsf@intel.com>

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

  reply	other threads:[~2014-09-02  8:16 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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  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:09     ` ville.syrjala
2014-09-01  8:50     ` Chris Wilson
2014-09-01  8:50       ` [Intel-gfx] " Chris Wilson
2014-09-01  9:45       ` Ville Syrjälä
2014-09-01  9:45         ` [Intel-gfx] " Ville Syrjälä
2014-09-01 10:02         ` Chris Wilson
2014-09-01 10:02           ` [Intel-gfx] " Chris Wilson
2014-09-01 10:07       ` [PATCH v2] " ville.syrjala
2014-09-01 10:07         ` ville.syrjala
2014-09-01 10:20         ` Chris Wilson
2014-09-01 10:20           ` Chris Wilson
2014-09-01 10:36           ` Ville Syrjälä
2014-09-01 10:43             ` Chris Wilson
2014-09-01 10:43               ` Chris Wilson
2014-09-02  7:41               ` Jani Nikula
2014-09-02  7:41                 ` Jani Nikula
2014-09-02  8:16                 ` Ville Syrjälä [this message]
2014-09-02  8:16                   ` Ville Syrjälä
2014-09-02  8:26                   ` Chris Wilson
2014-09-02  8:26                     ` Chris Wilson
2014-09-02  9:57                     ` [PATCH v3] " ville.syrjala
2014-09-02  9:57                       ` ville.syrjala
2014-09-02 11:41                       ` Jani Nikula
2014-09-02 11:41                         ` Jani Nikula
2014-09-01 20:19         ` [PATCH v2] " Tibor Billes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140902081612.GH4193@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tbilles@gmx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.