[PATCH] drm/i915: remove indirection in the PCI ID macros

[PATCH] drm/i915: remove indirection in the PCI ID macros

[Jani Nikula]

Spell all the PCI IDs out to be able to quickly grep for the IDs. No
functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

I tested this by comparing the results of

$ make drivers/gpu/drm/i915/i915_drv.s
$ make arch/x86/kernel/early-quirks.s

before and after the patch. No change.
---
 include/drm/i915_pciids.h | 49 ++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 180ad0e6de21..eb79a64947da 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -208,40 +208,41 @@
 #define INTEL_VLV_D_IDS(info) \
 	INTEL_VGA_DEVICE(0x0155, info)
 
-#define _INTEL_BDW_M(gt, id, info) \
-	INTEL_VGA_DEVICE((((gt) - 1) << 4) | (id), info)
-#define _INTEL_BDW_D(gt, id, info) \
-	INTEL_VGA_DEVICE((((gt) - 1) << 4) | (id), info)
-
-#define _INTEL_BDW_M_IDS(gt, info) \
-	_INTEL_BDW_M(gt, 0x1602, info), /* ULT */ \
-	_INTEL_BDW_M(gt, 0x1606, info), /* ULT */ \
-	_INTEL_BDW_M(gt, 0x160B, info), /* Iris */ \
-	_INTEL_BDW_M(gt, 0x160E, info) /* ULX */
-
-#define _INTEL_BDW_D_IDS(gt, info) \
-	_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
-	_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
-
-#define INTEL_BDW_GT12M_IDS(info) \
-	_INTEL_BDW_M_IDS(1, info), \
-	_INTEL_BDW_M_IDS(2, info)
+#define INTEL_BDW_GT12M_IDS(info)  \
+	INTEL_VGA_DEVICE(0x1602, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x1606, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x160B, info), /* Iris */ \
+	INTEL_VGA_DEVICE(0x160E, info), /* ULX */ \
+	INTEL_VGA_DEVICE(0x1612, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x1616, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x161B, info), /* Iris */ \
+	INTEL_VGA_DEVICE(0x161E, info)  /* ULX */
 
 #define INTEL_BDW_GT12D_IDS(info) \
-	_INTEL_BDW_D_IDS(1, info), \
-	_INTEL_BDW_D_IDS(2, info)
+	INTEL_VGA_DEVICE(0x160A, info), /* Server */ \
+	INTEL_VGA_DEVICE(0x160D, info), /* Workstation */ \
+	INTEL_VGA_DEVICE(0x161A, info), /* Server */ \
+	INTEL_VGA_DEVICE(0x161D, info)  /* Workstation */
 
 #define INTEL_BDW_GT3M_IDS(info) \
-	_INTEL_BDW_M_IDS(3, info)
+	INTEL_VGA_DEVICE(0x1622, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x1626, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x162B, info), /* Iris */ \
+	INTEL_VGA_DEVICE(0x162E, info)  /* ULX */
 
 #define INTEL_BDW_GT3D_IDS(info) \
-	_INTEL_BDW_D_IDS(3, info)
+	INTEL_VGA_DEVICE(0x162A, info), /* Server */ \
+	INTEL_VGA_DEVICE(0x162D, info)  /* Workstation */
 
 #define INTEL_BDW_RSVDM_IDS(info) \
-	_INTEL_BDW_M_IDS(4, info)
+	INTEL_VGA_DEVICE(0x1632, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x1636, info), /* ULT */ \
+	INTEL_VGA_DEVICE(0x163B, info), /* Iris */ \
+	INTEL_VGA_DEVICE(0x163E, info)  /* ULX */
 
 #define INTEL_BDW_RSVDD_IDS(info) \
-	_INTEL_BDW_D_IDS(4, info)
+	INTEL_VGA_DEVICE(0x163A, info), /* Server */ \
+	INTEL_VGA_DEVICE(0x163D, info)  /* Workstation */
 
 #define INTEL_BDW_M_IDS(info) \
 	INTEL_BDW_GT12M_IDS(info), \
-- 
2.1.4

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

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Chris Wilson]

On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
> Spell all the PCI IDs out to be able to quickly grep for the IDs. No
> functional changes.

On the other hand this is a loss of contextual information in the header
file.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Jani Nikula]

On Tue, 03 Feb 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
>> Spell all the PCI IDs out to be able to quickly grep for the IDs. No
>> functional changes.
>
> On the other hand this is a loss of contextual information in the header
> file.

I assume the GT count is what you're referring to. I contemplated that,
and decided that I, personally, really value the file as the canonical
place to look at all the supported PCI IDs above all.

Also, I'm pretty sure there's nothing in the specs that make guarantees
about digits in PCI IDs mapping to GT count. My impression is that it's
just an inference on our part, valid for the set of PCI IDs currently in
the file. But this isn't that important.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Chris Wilson]

On Tue, Feb 03, 2015 at 03:05:05PM +0200, Jani Nikula wrote:
> On Tue, 03 Feb 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
> >> Spell all the PCI IDs out to be able to quickly grep for the IDs. No
> >> functional changes.
> >
> > On the other hand this is a loss of contextual information in the header
> > file.
> 
> I assume the GT count is what you're referring to. I contemplated that,
> and decided that I, personally, really value the file as the canonical
> place to look at all the supported PCI IDs above all.

Exactly, which is why keeping the extra contextual information we have
is useful and why we want to keep changes here concise.
 
> Also, I'm pretty sure there's nothing in the specs that make guarantees
> about digits in PCI IDs mapping to GT count. My impression is that it's
> just an inference on our part, valid for the set of PCI IDs currently in
> the file. But this isn't that important.

It has been deliberate since SNB, since the introduction of varying GT
parts, much to the annoyance of people who rightly state that is not
what PCI ID was meant for. It is also how we detect which GT the chipset
is.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Damien Lespiau]

On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
> Spell all the PCI IDs out to be able to quickly grep for the IDs. No
> functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> I tested this by comparing the results of
> 
> $ make drivers/gpu/drm/i915/i915_drv.s
> $ make arch/x86/kernel/early-quirks.s
> 
> before and after the patch. No change.

I'm sold by the test so:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

I did double check with the specs though and found:

  - 0x160B is marked as reserved
  - 0x161B is marked as reserved
  - A few more ids are reserved than the ones in the reserved define

The reserved ones are non existing SKUs, so meh.

-- 
Damien

> ---
>  include/drm/i915_pciids.h | 49 ++++++++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 180ad0e6de21..eb79a64947da 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -208,40 +208,41 @@
>  #define INTEL_VLV_D_IDS(info) \
>  	INTEL_VGA_DEVICE(0x0155, info)
>  
> -#define _INTEL_BDW_M(gt, id, info) \
> -	INTEL_VGA_DEVICE((((gt) - 1) << 4) | (id), info)
> -#define _INTEL_BDW_D(gt, id, info) \
> -	INTEL_VGA_DEVICE((((gt) - 1) << 4) | (id), info)
> -
> -#define _INTEL_BDW_M_IDS(gt, info) \
> -	_INTEL_BDW_M(gt, 0x1602, info), /* ULT */ \
> -	_INTEL_BDW_M(gt, 0x1606, info), /* ULT */ \
> -	_INTEL_BDW_M(gt, 0x160B, info), /* Iris */ \
> -	_INTEL_BDW_M(gt, 0x160E, info) /* ULX */
> -
> -#define _INTEL_BDW_D_IDS(gt, info) \
> -	_INTEL_BDW_D(gt, 0x160A, info), /* Server */ \
> -	_INTEL_BDW_D(gt, 0x160D, info) /* Workstation */
> -
> -#define INTEL_BDW_GT12M_IDS(info) \
> -	_INTEL_BDW_M_IDS(1, info), \
> -	_INTEL_BDW_M_IDS(2, info)
> +#define INTEL_BDW_GT12M_IDS(info)  \
> +	INTEL_VGA_DEVICE(0x1602, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x1606, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x160B, info), /* Iris */ \
> +	INTEL_VGA_DEVICE(0x160E, info), /* ULX */ \
> +	INTEL_VGA_DEVICE(0x1612, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x1616, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x161B, info), /* Iris */ \
> +	INTEL_VGA_DEVICE(0x161E, info)  /* ULX */
>  
>  #define INTEL_BDW_GT12D_IDS(info) \
> -	_INTEL_BDW_D_IDS(1, info), \
> -	_INTEL_BDW_D_IDS(2, info)
> +	INTEL_VGA_DEVICE(0x160A, info), /* Server */ \
> +	INTEL_VGA_DEVICE(0x160D, info), /* Workstation */ \
> +	INTEL_VGA_DEVICE(0x161A, info), /* Server */ \
> +	INTEL_VGA_DEVICE(0x161D, info)  /* Workstation */
>  
>  #define INTEL_BDW_GT3M_IDS(info) \
> -	_INTEL_BDW_M_IDS(3, info)
> +	INTEL_VGA_DEVICE(0x1622, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x1626, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x162B, info), /* Iris */ \
> +	INTEL_VGA_DEVICE(0x162E, info)  /* ULX */
>  
>  #define INTEL_BDW_GT3D_IDS(info) \
> -	_INTEL_BDW_D_IDS(3, info)
> +	INTEL_VGA_DEVICE(0x162A, info), /* Server */ \
> +	INTEL_VGA_DEVICE(0x162D, info)  /* Workstation */
>  
>  #define INTEL_BDW_RSVDM_IDS(info) \
> -	_INTEL_BDW_M_IDS(4, info)
> +	INTEL_VGA_DEVICE(0x1632, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x1636, info), /* ULT */ \
> +	INTEL_VGA_DEVICE(0x163B, info), /* Iris */ \
> +	INTEL_VGA_DEVICE(0x163E, info)  /* ULX */
>  
>  #define INTEL_BDW_RSVDD_IDS(info) \
> -	_INTEL_BDW_D_IDS(4, info)
> +	INTEL_VGA_DEVICE(0x163A, info), /* Server */ \
> +	INTEL_VGA_DEVICE(0x163D, info)  /* Workstation */
>  
>  #define INTEL_BDW_M_IDS(info) \
>  	INTEL_BDW_GT12M_IDS(info), \
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Chris Wilson]

On Tue, Feb 03, 2015 at 01:36:00PM +0000, Damien Lespiau wrote:
> On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
> > Spell all the PCI IDs out to be able to quickly grep for the IDs. No
> > functional changes.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > ---
> > 
> > I tested this by comparing the results of
> > 
> > $ make drivers/gpu/drm/i915/i915_drv.s
> > $ make arch/x86/kernel/early-quirks.s
> > 
> > before and after the patch. No change.
> 
> I'm sold by the test so:
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> I did double check with the specs though and found:
> 
>   - 0x160B is marked as reserved
>   - 0x161B is marked as reserved
>   - A few more ids are reserved than the ones in the reserved define
> 
> The reserved ones are non existing SKUs, so meh.

Though we have been caught out in the past when products started
shipping. Userspace depends upon the list being complete (and for not
new PCI IDs being released for old hardware).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[Daniel Vetter]

On Tue, Feb 03, 2015 at 01:36:00PM +0000, Damien Lespiau wrote:
> On Tue, Feb 03, 2015 at 02:34:05PM +0200, Jani Nikula wrote:
> > Spell all the PCI IDs out to be able to quickly grep for the IDs. No
> > functional changes.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > ---
> > 
> > I tested this by comparing the results of
> > 
> > $ make drivers/gpu/drm/i915/i915_drv.s
> > $ make arch/x86/kernel/early-quirks.s
> > 
> > before and after the patch. No change.
> 
> I'm sold by the test so:
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Jani just pinged me about this and I agree grepability is nice and bdw is
indeed the odd one out right now. I've added GT1/2 comments so that we
don't just drop that information on the floor and merged it to dinq. Minor
conflict but I think wiggle did the right thing ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: remove indirection in the PCI ID macros

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5706
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -5              283/283              278/283
ILK                 -1              319/319              318/319
SNB                                  322/346              322/346
IVB              +1                 382/384              383/384
BYT                                  296/296              296/296
HSW              +1                 425/428              426/428
BDW                 -1              319/333              318/333
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_coherency-sync      PASS(3, M25M7)      CRASH(1, M7)
*PNV  igt_gem_userptr_blits_coherency-unsync      PASS(3, M25M7)      CRASH(1, M7)
*PNV  igt_gen3_render_linear_blits      PASS(7, M25M23M7)      FAIL(1, M7)
*PNV  igt_gen3_render_tiledx_blits      PASS(3, M25M7)      FAIL(1, M7)
 PNV  igt_gen3_render_tiledy_blits      FAIL(1, M7)PASS(2, M25M7)      FAIL(1, M7)
 ILK  igt_gem_unfence_active_buffers      DMESG_WARN(1, M37)PASS(4, M26M37)      DMESG_WARN(1, M37)
 IVB  igt_gem_storedw_batches_loop_normal      DMESG_WARN(7, M34M4M21)PASS(20, M34M4M21)      PASS(1, M34)
 HSW  igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance      DMESG_WARN(3, M40)PASS(25, M40M20)      PASS(1, M40)
*BDW  igt_gem_fence_thrash_bo-write-verify-threaded-none      PASS(4, M30)      DMESG_WARN(1, M30)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx