From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH v2] drm/i915: Fix recursive calls to unmap
Date: Sun, 30 Oct 2011 18:52:10 -0700
Message-ID: <20111030185210.6d3a74d4@bwidawsk.net>
References: <1320024054-4732-1-git-send-email-ben@bwidawsk.net>
	<yunehxu2b8o.fsf@aiko.keithp.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com
	[184.106.247.128])
	by gabe.freedesktop.org (Postfix) with ESMTP id 703DA9E8BC
	for <intel-gfx@lists.freedesktop.org>;
	Sun, 30 Oct 2011 18:52:19 -0700 (PDT)
In-Reply-To: <yunehxu2b8o.fsf@aiko.keithp.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Keith Packard <keithp@keithp.com>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Sun, 30 Oct 2011 18:36:23 -0700
Keith Packard <keithp@keithp.com> wrote:

> On Sun, 30 Oct 2011 18:20:54 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > 
> > The solution here is to add a new flag to the call chain which gives the
> > routines the information they need to possibly defer actions which may
> > cause us to recurse.
> 
> This looks a lot nicer; it's shorter than I feared too.
> 
> > @@ -2051,7 +2052,8 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
> >  	 * it.
> >  	 */
> >  	if (obj->active) {
> > -		ret = i915_wait_request(obj->ring, obj->last_rendering_seqno);
> > +		ret = i915_wait_request(obj->ring, obj->last_rendering_seqno,
> > +					true);
> 
> > @@ -2400,7 +2402,8 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj,
> >  		if (!ring_passed_seqno(obj->last_fenced_ring,
> >  				       obj->last_fenced_seqno)) {
> >  			ret = i915_wait_request(obj->last_fenced_ring,
> > -						obj->last_fenced_seqno);
> > +						obj->last_fenced_seqno,
> > +						true);
> >  			if (ret)
> >  				return ret;
> >  		}
> > @@ -2541,7 +2544,8 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
> >  				if (!ring_passed_seqno(obj->last_fenced_ring,
> >  						       reg->setup_seqno)) {
> >  					ret = i915_wait_request(obj->last_fenced_ring,
> > -								reg->setup_seqno);
> > +								reg->setup_seqno,
> > +								true);
> >  					if (ret)
> >  						return ret;
> >  				}
> 
> Any reason you're changing behaviour outside of the area with the actual
> problem? If so, you should stick those in a separate patch instead of
> mixing them into this bug fix.
> 

Well, I had to pick one, and looking at the call chain, it seemed there wasn't
much to gain by doing retiring at this point. If you want to take the
patch, and modify those vales to false - I'm okay with it.

Ben