Re: Resubmit: Intel 965 Express AGP patches

From: Eric Anholt
Date: Wed Sep 06 2006 - 12:57:52 EST


On Wed, 2006-09-06 at 12:02 -0400, Dave Jones wrote:
> On Tue, Sep 05, 2006 at 10:37:32AM -0700, Eric Anholt wrote:
> > The following should be the updated patch series for the Intel 965 Express
> > support, unless I'm making some mistake with git-send-email. I think I've
> > covered Dave's concerns
>
> This chunk seems unrelated to 965 support.
>
> @@ -1469,7 +1570,7 @@ static struct agp_bridge_driver intel_91
> .owner = THIS_MODULE,
> .aperture_sizes = intel_i830_sizes,
> .size_type = FIXED_APER_SIZE,
> - .num_aperture_sizes = 3,
> + .num_aperture_sizes = 4,
> .needs_scratch_page = TRUE,
> .configure = intel_i915_configure,
> .fetch_size = intel_i915_fetch_size,
>
> It seems to be a valid fix (as there are indeed 4 entries in
> intel_i830_sizes), but I wonder if this was intentional ?
> Has this been tested on 915/945?
> I've chopped this bit out and committed the rest, we can
> add this as a separate commit, which may ease future bisecting
> if anything should go awry.
> The intel_830_driver struct also lists the num of sizes as '3' btw.
> It could just be lots of cut-n-paste braindamage, but things like
> this make me nervous in a driver that supports so much hardware
> and is so.. twisted.

It just looks to me like when the 512MB entry was added for the 965, the
count was bumped for 915 too. It wouldn't have mattered for the 915 (or
i830, if it had got the treatment), since it'll never have a 512MB
aperture anyway, and doesn't use the generic gatt creation.

> Also, do we need an entry in agp_intel_resume() for the 965 ?

Yeah, looks like it.

> >, except for making the PCI ID stuff table-driven.
> > You can find a patch for that on the intel-agp-i965 branch at
> > git://anongit.freedesktop.org/~anholt/linux-2.6
>
> I'll take a look at those soon.

Thanks!

--
Eric Anholt anholt@xxxxxxxxxxx
eric@xxxxxxxxxx eric.anholt@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part