Hi Daniel,
On Wed, May 03, 2017 at 12:47:18PM +0100, Daniel Stone wrote:
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.
This is the same in both approaches.
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;
I concede the tricky code is all confined to the single implementation
in the kernel (encoding is far harder than decoding) - I just think
create_in_format_blob() is cumbersome, ugly and error-prone.
That's shorter than fetching separate blobs, and doesn't have multiple
error paths either. What am I missing?
Yes, this is a convincing argument.
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.
Hey I'm just saying it's an option - not that it should be implemented
in V1.
Where both the formats and the modifiers are the same, Ben's approach
lets the blob be shared anyway.
Thanks,
Brian
Cheers, Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
