Re: [PATCH 00/16] Move chipset specific stuff to struct intel_chipset

From: "Kristian Høgsberg" <krh@bitplanet.net>
To: Kenneth Graunke <kenneth@whitecape.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 00/16] Move chipset specific stuff to struct intel_chipset
Date: Wed, 8 Jun 2011 20:36:46 +0200	[thread overview]
Message-ID: <BANLkTi=sgk4k=myfmmCk=HzBtky_8KRhTg@mail.gmail.com> (raw)
In-Reply-To: <4DEF622D.90001@whitecape.org>

2011/6/8 Kenneth Graunke <kenneth@whitecape.org>:
> On 06/07/2011 12:34 PM, Kristian Høgsberg wrote:
>>
>> Hi,
>>
>> Here's a handful of patches that try to replace most of our chipset
>> feature checking with data in a new struct intel_chipset.  It uses the
>> new PCI ID list infrastructure and eliminates all IS_FOO macros in
>> favor of a per-family chipset info struct.  Actually, I was surprised
>> how much in the driver is really just a gen check, but there are a few
>> cases where we have to check a certain feature, as well as all the
>> gen4+ urb and thread limits (includes the recent fix for swapped VS
>> entry counts).
>>
>> The series compiles and passes casual testing for me, but I've not run
>> piglit on it yet.
>>
>> Kristian
>
> This patchset is fantastic.  Way better than what I'd been doing.  I'm
> tidying it up a bit and working on a bunch of follow-on cleanups...I'll send
> out a proposed replacement series tomorrow.

It was a Tuesday morning distraction for me, and now I'm in Denmark
for a few days vacation.  If you want to take over the patch set, I'd
be very happy :)  One thing I was thinking about changing was to just
pull the chipset inteo the chipset_map array and set that in
intelScreen in intel_screen.c, instead including the chipsets again in
the big pci id switch.

> The only concern I have is with the intel_decode changes to use gen.  I like
> the idea, but I'm afraid it may get us into trouble: What if we need G45
> specific decoding?  We'll -certainly- need Gen 7.5 specific dumping (over
> and above Gen7), eventually.
>
> ickle's idea to use gen 70 and 75 would solve that...though, in general I
> agree with Eric in preferring chipset->gen >= 7 and chipset->is_name.

Chris also suggested sharing the chipset structs so maybe we could
just move it all to intel-gpu-tools (and make decode use the chipset
struct perhaps) as the canonical location for these chipset
definitions.  We also have a similar setup in the kernel, but I'm not
sure it's practical to consolidate all that.  I'd rather get this
first step done and then maybe think about further improvements.

Kristian
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx