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

[Kristian Høgsberg]

On Wed, Nov 18, 2015 at 1:54 PM, Sarah Sharp
<sarah.a.sharp at linux.intel.com> wrote:
> 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?

We don't usually resend for adding Reviewed-by.

Kristian

>
> Sarah Sharp
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev