On 12/01/18 12:01, Tvrtko Ursulin wrote:
On 12/01/2018 10:58, Lionel Landwerlin wrote:
On 12/01/18 10:15, Tvrtko Ursulin wrote:
[snip]
+static inline int sseu_eu_idx(const struct sseu_dev_info *sseu,
+ int slice, int subslice, int eu_group)
What is eu_group for? Will it be used at some point?
In case we ever have more than 8 EUs per subslice.
I am thinking if we could hide that from the call sites, to avoid it
being passed as zeros, and to avoid having to write loops in other
patches which reference eu_groups, when it is not immediately obvious
what that means.
Could we for instance have a helper which would clear/set numbered EUs
in sseu_dev_info, and so hide all the implementation details?
sseu_enable_eus(sseu, slice, subslice, start, end);
Then when you have code like:
sseu->eu_mask[sseu_eu_idx(sseu, s, ss, 0)] = ~eu_disabled_mask;
You would write it as:
/* On this slice/subslice mark EUs 0 to N as enabled. */
sseu_enable_eus(sseu, s, ss, 0, fls(~eu_disabled_mask));
Hmm... I don't think that works if you have gaps, right?
Like a BXT 2x6 where a row of EUs has been fused off. It would be
something like 0b01110111 or 0b10111011.
Helper would internally know the size of the underlying storage and
dtrt. There would be no need to manually manage eu_groups. In the
initial implementation you could simply GEM_BUG_ON if the EU range
does not fit into the current storage. Later u8 could be turned into
u16 or similar. You also wouldn't have any iteration over eu_groups in
this version.
I think that would be cleaner and easier to extend in the future.
Unless I overlooked some important detail?
Or even simplify it by passing bitmask instead of start/end, and just
have no support for more than 8 EUs in this version? No eu_group etc.
When the need arises to have more, bump the eu_mask type to u16. That
would require you to put back the stride parameter in the uAPI I think.
I'm not really a fan of having the data field in userspace be
reinterpreted (as u16 or u32) based on one of the other field.
It might be easier on the kernel side, but complicates userspace.
I would prefer to stick to u8 and have everybody think of
slice/subslice/eus availability as array of u8 bit fields which you
might need to iterate more than one if there are more than 8 elements.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
