[PATCH 4/4] drm: Add getfb2 ioctl

[Daniel Stone]

On 23 March 2018 at 17:31, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Fri, Mar 23, 2018 at 05:00:11PM +0000, Daniel Stone wrote:
>> On 23 March 2018 at 14:49, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
>> > On Fri, Mar 23, 2018 at 01:45:52PM +0000, Daniel Stone wrote:
>> >> +     for (i = 0; i < ARRAY_SIZE(r->handles); i++) {
>> >> +             r->handles[i] = 0;
>> >> +             r->pitches[i] = 0;
>> >> +             r->offsets[i] = 0;
>> >> +             r->modifier[i] = DRM_FORMAT_MOD_INVALID;
>> >
>> > Don't we want to leave this zeroed too? For addfb2 we require any unused
>> > modifier to be 0, so if someone does 'getfb2(&cmd); addfb2(&cmd);' they
>> > would get an error from the addfb2().
>>
>> My thinking is that since the primary userspace for this doesn't have
>> symmetry with add (args for add, struct for get), that it was better
>> to feed in INVALID directly. This is going to change, e.g., X server
>> to:
>>   modifier = (fb->flags ? DRM_MODE_FB_MODIFIERS) ? fb->modifier :
>> DRM_FORMAT_MOD_INVALID;
>>
>> It's a good point about the symmetry though. Do you know of direct
>> non-libdrm users? Apart from igt of course ;)
>
> Nope. Just thought that since both take the same struct it'd make some
> sense. And figured it could serve as a quick sanity check to make sure
> getfb outputs sane data. Or rather, if the driver accepts it back in
> it can't be all bad at least.
>
> But if you think it's not a particularly useful thing to do then I'm
> certainly willing to accept that.

Makes sense. I'm on the fence; let's see if anyone else has any suggestions.

Cheers,
Daniel