Quoting Tvrtko Ursulin (2019-09-02 14:42:44)
>
> On 24/07/2019 14:05, Tvrtko Ursulin wrote:
> >
> > On 23/07/2019 16:49, Stuart Summers wrote:
> >> +u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice)
> >> +{
> >> + int i, offset = slice * sseu->ss_stride;
> >> + u32 mask = 0;
> >> +
> >> + if (slice >= sseu->max_slices) {
> >> + DRM_ERROR("%s: invalid slice %d, max: %d\n",
> >> + __func__, slice, sseu->max_slices);
> >> + return 0;
> >> + }
> >> +
> >> + if (sseu->ss_stride > sizeof(mask)) {
> >> + DRM_ERROR("%s: invalid subslice stride %d, max: %lu\n",
> >> + __func__, sseu->ss_stride, sizeof(mask));
> >> + return 0;
> >> + }
> >> +
> >> + for (i = 0; i < sseu->ss_stride; i++)
> >> + mask |= (u32)sseu->subslice_mask[offset + i] <<
> >> + i * BITS_PER_BYTE;
> >> +
> >> + return mask;
> >> +}
> >
> > Why do you actually need these complications when the plan from the
> > start was that the driver and user sseu representation structures can be
> > different?
> >
> > I only gave it a quick look so I might be wrong, but why not just expand
> > the driver representations of subslice mask up from u8? Userspace API
> > should be able to cope with strides already.
>
> I never got an answer to this and the series was merged in the meantime.
>
> Maybe not much harm but I still don't understand why all the
> complications seemingly just to avoid bumping the *internal* ss mask up
> from u8. As long as the internal and abi sseu info struct are well
> separated and access point few and well controlled (I think they are)
> then I don't see why the internal side had to be converted to u8 and
> strides. But maybe I am missing something.
I looked at it and thought it was open-coding bitmap.h as well. I
accepted it in good faith that it improved certain use cases and should
even make tidying up the code without regressing those easier.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
