Re: [PATCH 9/9] drm/i915: Expand subslice mask

[Chris Wilson <chris@chris-wilson.co.uk>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx