[Mesa-dev] [PATCH v2 1/2] mesa: Add KBL PCI IDs and platform information.

[Sarah Sharp]

On Tue, Nov 17, 2015 at 02:20:28PM -0800, Ben Widawsky wrote:
> On Tue, Nov 17, 2015 at 11:40:53AM -0800, Sarah Sharp wrote:
> > Add PCI IDs for the Intel Kabylake platforms.  The IDs are taken
> > directly from the Linux kernel patches, which are under review:
> > 
> > http://lists.freedesktop.org/archives/intel-gfx/2015-October/078967.html
> > http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=kbl-upstream-v2
> > 
> > The Kabylake PCI IDs taken from the kernel are rearranged to be in order
> > of GT type, then PCI ID.
> > 
> > Please note that if this patch is backported, the following fixes will
> > need to be added before this patch:
> > 
> > commit 28ed1e08e8ba98e "i965/skl: Remove early platform support"
> > commit c1e38ad37042b0e "i965/skl: Use larger URB size where available."
> > 
> > Thanks to Ben for fixing a bug around setting urb.size, and being
> > patient with my questions about what the various fields mean.
> > 
> > Signed-off-by: Sarah Sharp <sarah.a.sharp at linux.intel.com>
> > Suggested-by: Ben Widawsky <benjamin.widawsky at intel.com>
> > Tested-by: Rodrigo Vivi <rodrigo.vivi at intel.com> (KBL-GT2)
> > ---
> > 
> > v2:
> >  - reorder the PCI IDs
> >  - rebase on latest mesa master
> 
> There's not really a consensus I guess, but most people do leave the version
> information in the final commit message.

I personally feel like that's leaving boredom doodles on a final
architectural drawing. If people want to know the back-and-forth
history, the mailing list archive will always be there. So, no, I don't
really want to leave version info in the commit message.

> Also FWIW, I too was tempted to ask for the PCI IDs to be in order, but, that
> makes the patch harder to review :-( - so I am assuming it's just the sorted
> list from the v1, and I am not going through it again.

Yep, I ran the two patches through grep and sort, so I can confirm
they're the same.

[snip]

> > +#define  KBL_MAX_THREADS_PER_PSD 64
> > +
> > +static const struct brw_device_info brw_device_info_kbl_gt1 = {
> > +   GEN9_FEATURES,
> > +   .gt = 1,
> > +
> > +   .max_cs_threads = 7 * 6,
> > +   .max_wm_threads = KBL_MAX_THREADS_PER_PSD * 2,
> > +   .urb.size = 192,
> > +};
> > +
> > +static const struct brw_device_info brw_device_info_kbl_gt1_5 = {
> > +   GEN9_FEATURES,
> > +   .gt = 1,
> > +
> > +   .max_cs_threads = 7 * 6,
> > +   .max_wm_threads = KBL_MAX_THREADS_PER_PSD * 3,
> > +};
> > +
> > +static const struct brw_device_info brw_device_info_kbl_gt2 = {
> > +   GEN9_FEATURES,
> > +   .gt = 2,
> > +
> > +   .max_wm_threads = KBL_MAX_THREADS_PER_PSD * 3,
> > +};
> > +
> > +static const struct brw_device_info brw_device_info_kbl_gt3 = {
> > +   GEN9_FEATURES,
> > +   .gt = 3,
> > +
> > +   .max_wm_threads = KBL_MAX_THREADS_PER_PSD * 6,
> > +};
> > +
> > +static const struct brw_device_info brw_device_info_kbl_gt4 = {
> > +   GEN9_FEATURES,
> > +   .gt = 4,
> > +
> > +   .max_wm_threads = KBL_MAX_THREADS_PER_PSD * 9,
> > +   /*
> > +    * From the "L3 Allocation and Programming" documentation:
> > +    *
> > +    * "URB is limited to 1008KB due to programming restrictions.  This
> > +    *  is not a restriction of the L3 implementation, but of the FF and
> > +    *  other clients.  Therefore, in a GT4 implementation it is
> > +    *  possible for the programmed allocation of the L3 data array to
> > +    *  provide 3*384KB=1152KB for URB, but only 1008KB of this
> > +    *  will be used."
> > +    */
> > +   .urb.size = 1008 / 3,
> > +};
> > +
> >  const struct brw_device_info *
> >  brw_get_device_info(int devid)
> >  {
> 
> I commented on the internal list about the potential of making defines for the
> sublices per slice, and #slices (until we're getting the info from libdrm). I
> also mentioned we might not want to call it "KBL_MAX_THREADS" since that 64
> number goes back many generations. Those are very minor details IMO.
> 
> Reviewed-by: Ben Widawsky <benjamin.widawsky at intel.com>

Do people normally resend patches with Reviewed-by tags, or will the
person merging it pick those up?

Sarah Sharp