From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work
 for direct setup
Date: Fri, 8 Jun 2012 14:39:10 +0200
Message-ID: <20120608123910.GA5761@phenom.ffwll.local>
References: <1339077364-21032-1-git-send-email-daniel.vetter@ffwll.ch>
	<1339077364-21032-9-git-send-email-daniel.vetter@ffwll.ch>
	<87oboup12x.fsf@intel.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 mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49])
	by gabe.freedesktop.org (Postfix) with ESMTP id 205F69E7A0
	for <intel-gfx@lists.freedesktop.org>;
	Fri,  8 Jun 2012 05:37:44 -0700 (PDT)
Received: by eekd17 with SMTP id d17so1378209eek.36
	for <intel-gfx@lists.freedesktop.org>;
	Fri, 08 Jun 2012 05:37:44 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <87oboup12x.fsf@intel.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: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>, DRI Development <dri-devel@lists.freedesktop.org>
List-Id: intel-gfx@lists.freedesktop.org

On Fri, Jun 08, 2012 at 01:09:10PM +0300, Jani Nikula wrote:
> On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >  #define INTEL_GTT_GEN	intel_private.driver->gen
> > @@ -1522,14 +1523,32 @@ static int find_gmch(u16 device)
> >  	return 1;
> >  }
> >  
> > -int intel_gmch_probe(struct pci_dev *pdev,
> > -				      struct agp_bridge_data *bridge)
> > +int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> > +		     struct agp_bridge_data *bridge)
> >  {
> >  	int i, mask;
> > -	intel_private.driver = NULL;
> 
> A possibly clueless question: should intel_gmch_remove() do this now,
> since intel_private.driver and intel_private.refcount are linked
> together below?
> 
> > +
> > +	/*
> > +	 * Can be called from the fake agp driver but also directly from
> > +	 * drm/i915.ko. Hence we need to check whether everything is set up
> > +	 * already.
> > +	 */
> > +	if (intel_private.driver) {
> > +		intel_private.refcount++;
> > +		return 1;
> > +	}
> 
> Judging by the commit message, is it intentional that the first call to
> probe does not increase the refcount? The cleanup will now happen only
> if probe and remove are called twice or more. Should this perhaps be
> clarified in a code comment in addition to the commit message?

Yeah, things went a bit wrong here. I'll also see whether I can make the
cleanup path a bit more robust. Thanks for taking a look at this patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48