public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
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>
Subject: Re: [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup
Date: Fri, 8 Jun 2012 14:39:10 +0200	[thread overview]
Message-ID: <20120608123910.GA5761@phenom.ffwll.local> (raw)
In-Reply-To: <87oboup12x.fsf@intel.com>

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

  reply	other threads:[~2012-06-08 12:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
2012-06-07 13:55 ` [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions Daniel Vetter
2012-06-08  9:16   ` Jani Nikula
2012-06-08 15:12     ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 02/12] drm: kill the REQUIRE_AGP driver flag Daniel Vetter
2012-06-07 13:55 ` [PATCH 03/12] drm: kill USE_AGP " Daniel Vetter
2012-06-07 13:55 ` [PATCH 04/12] agp/intel-gtt: remove dead code Daniel Vetter
2012-06-07 13:55 ` [PATCH 05/12] drm/i915: stop using dev->agp->base Daniel Vetter
2012-06-07 13:55 ` [PATCH 06/12] agp/intel-gtt: don't require the agp bridge on setup Daniel Vetter
2012-06-07 13:55 ` [PATCH 07/12] drm/i915: only enable drm agp support when required Daniel Vetter
2012-06-12 11:58   ` [Intel-gfx] " Jani Nikula
2012-06-12 11:19     ` [PATCH] " Daniel Vetter
2012-06-12 12:21     ` [PATCH 07/12] " Daniel Vetter
2012-06-07 13:56 ` [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup Daniel Vetter
2012-06-08 10:09   ` Jani Nikula
2012-06-08 12:39     ` Daniel Vetter [this message]
2012-06-07 13:56 ` [PATCH 09/12] drm/i915: don't check intel_agp_enabled any more Daniel Vetter
2012-06-12 11:24   ` [PATCH] " Daniel Vetter
2012-06-07 13:56 ` [PATCH 10/12] agp/intel-gtt: move gart base addres setup Daniel Vetter
2012-06-07 13:56 ` [PATCH 11/12] drm/i915: call intel_enable_gtt Daniel Vetter
2012-06-07 13:56 ` [PATCH 12/12] agp/intel-agp: remove snb+ host bridge pciids Daniel Vetter
2012-06-12 12:46 ` [PATCH 00/12] clear up drm/agp initialization madness Jani Nikula
2012-06-12 20:22   ` Daniel Vetter

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=20120608123910.GA5761@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox