Re: [PATCH 1/3] drm/i915/skl: Split the SKL PCI ids by GT

Damien Lespiau
2015-02-04T13:10:10Z

On Tue, Feb 03, 2015 at 05:51:29PM -0800, Rodrigo Vivi wrote:
> On Thu, Jan 29, 2015 at 6:13 AM, Damien Lespiau
> <damien.lespiau@intel.com> wrote:
> > We need to have a separate GT3 struct intel_device_info to declare they
> > have a second VCS. Let's start by splitting the PCI ids per-GT.
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> > include/drm/i915_pciids.h | 28 +++++++++++++++++++---------
> > 1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > index 180ad0e..38a7c80 100644
> > --- a/include/drm/i915_pciids.h
> > +++ b/include/drm/i915_pciids.h
> > @@ -259,21 +259,31 @@
> > INTEL_VGA_DEVICE(0x22b2, info), \
> > INTEL_VGA_DEVICE(0x22b3, info)
> >
> > -#define INTEL_SKL_IDS(info) \
> > - INTEL_VGA_DEVICE(0x1916, info), /* ULT GT2 */ \
> > +#define INTEL_SKL_GT1_IDS(info) \
> > INTEL_VGA_DEVICE(0x1906, info), /* ULT GT1 */ \
> > - INTEL_VGA_DEVICE(0x1926, info), /* ULT GT3 */ \
> > - INTEL_VGA_DEVICE(0x1921, info), /* ULT GT2F */ \
> > INTEL_VGA_DEVICE(0x190E, info), /* ULX GT1 */ \
> > + INTEL_VGA_DEVICE(0x1902, info), /* DT GT1 */ \
>
> spec shows this id as GT2 DT

That is weird, for the other ids, 0 << 4 means GT1, while GT2 are 1 << 4.

Those ids have gone through review once, so 0x1902 was clearly marked as
GT1 then. Could be an error in BSpec. will ask.

>
> > + INTEL_VGA_DEVICE(0x190B, info), /* Halo GT1 */ \
> > + INTEL_VGA_DEVICE(0x190A, info) /* SRV GT1 */
>
> couldn't find those 2 on spec

For these and the rest of those, I'd rather keep them in tree as they
may stil be pre-production/early-adopters parts.

> Also I've seem some ids there that aren't here...

This is a known thing and on "purpose".

> I know this patch doesn't introduce the those IDs I couldn't fine
> so with 0x1902 fixed on v2 or on follow-up or explained consider this one here:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Considering the above I think we should go ahead with this patch.

--
Damien