Rob Clark robdclark@gmail.com writes:
On Tue, Dec 20, 2016 at 7:12 PM, Kristian H. Kristensen hoegsberg@gmail.com wrote:
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index ce7efe2..cea3de3 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -209,6 +209,33 @@ struct drm_mode_get_plane { __u64 format_type_ptr; };
+struct drm_format_modifier {
/* Bitmask of formats in get_plane format list this info* applies to. */uint64_t formats;Hmm, this seems a bit clunky/brittle when you have a lot of supported formats (esp. if format order changes or new formats are not added at end). I guess fine when you support a four or five different formats, but I think you'll start to hate maintaining those tables on hw that supports more.
I can't disagree, it was pretty finicky getting the bit masks right for the Intel patch. On the other hand, in userspace, the interface is easier to work with as you create the bitmask programatically, and there's precedence for bitmask already (possible_crtcs).
Also I guess it limits you to modifiers only with the first 64 formats.. maybe not a problem right away, but a quick look and drm/msm is already at 23 formats (and there are probably some more it could do.. without even starting to get into "exotic" float/etc formats and whatever else might come in the future.
Not that I really have a better idea.. maybe just instead of getplane2 we just add a querymodifiers ioctl (ie. fourcc in, list of modifiers out), with the idea that userspace probably knows what format/fourcc it wants to use, and only just cares about modifiers for that particular fourcc..
I'll reworking this with your base idea and Ville's suggestion and see how it comes out. Thanks for the feedback.
Kristian
BR, -R
/* This modifier can be used with the format for this plane. */uint64_t modifier;+};