Hi Brian, On 3 May 2017 at 12:00, Brian Starkey <brian.starkey@xxxxxxx> wrote: > Having just caught up on IRC I'm sure I'm far too late for this party, > but... > > Wouldn't this whole thing be a whole lot simpler if "IN_FORMATS" > stored "pointers" to separate blobs for the format and modifier lists? > > I've used/written a few interfaces with offsets to > variable-length-arrays and IMO the code to handle them is always > hideous. I don't agree ... The idea is that the header can grow because the offsets allow it to; adding a new member slots in between the end of static data and the start of variable data, and since all the variable data is accessed by an array, userspace doesn't have to be aware of the new members. The code for that is very clean (modulo the casts for pointer maths), cf. formats_ptr and modifiers_ptr; I'd expect userspace to copy and use those with version guards: uint32_t *formats = formats_ptr(blob); struct drm_format_modifier *modifiers = modifiers_ptr(blob); struct drm_format_unifier *unifiiers = (blob->version >= 2) ? unifiers_ptr(blob) : NULL; That's shorter than fetching separate blobs, and doesn't have multiple error paths either. What am I missing? > Added bonus: With smart enough helper code the FourCC and modifiers > blobs can be shared across planes. I think this is a serious case of premature optimisation; the memory savings might be outweighed by the additional number of objects to track, and userspace would have to jump through serious hoops with a blob ID cache - something which is a very bad idea regardless - to take any advantage of it. Cheers, Daniel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx