Hi,
On 5 April 2016 at 15:04, Rob Clark robdclark@gmail.com wrote:
On Tue, Apr 5, 2016 at 8:57 AM, Daniel Stone daniel@fooishbar.org wrote:
I've been assuming that it would have to be write-only; I don't believe there would be any meaningful usecases for read. Do you have any in mind, or is it just a symmetry/cleanliness thing?
no, don't see a use-case for it.. but this patch didn't seem to be preventing it. And it was storing the fence_fd on the kernel side which is a no-no as well.
Yeah, both should be fixed. :)
The issue is that 'struct file' / 'int fd' have a fundamentally different lifecycle compared to 'kms obj' / 'u32 obj_id'. For the kms objects (like framebuffers) where we can use them with properties, the id is tied to the kernel side object. This is not true for file descriptors. Resulting in a few problems:
Surely the fence FD tied to the kernel-side struct fence, in exactly the same way, no?
well, what I mean is you can't keep around the int fd on the kernel side, like this patch does
A write-only property, which immediately (ie. during the ioctl call) is converted into a fence object, would work. Although given that we need to handle fences differently (ie. not a property) for out-fences, it seems odd to shoehorn them into a property for in-fences.
Depends on how you look at it, I guess. From the point of view of all fence-like things being consistent, yes, it falls down. But from the point of view of in-fences being attached to an FB, and out-fences (like events) being separately attached to a request, it's a lot more consistent.
I think it is really better to pass in an array of 'struct { u32 plane; int fd }' (which the atomic ioctl code converts into 'struct fence's and attaches to the plane state) and an array of 'struct { u32 crtc; int fd }' filled in on the kernel side for the out-fences.
Mmm, it definitely makes ioctl parsing harder, and still you rely on drivers to implement the more-difficult-to-not-screw-up part, which (analagous to crtc_state->event) is the driver managing the lifecycle of that component of the state. We already enforce this with events though, and the difficult part wasn't in the userspace interface, but the backend handling. This doesn't change at all regardless of whether we use a property or an external array, so ...
hmm, I'm assuming that the in/out arrays are handled in drm_mode_atomic_ioctl() and the drivers never see 'em..
True, but it complicates the (already not hugely straightforward) parsing that the ioctl has to do. It also makes extending the ioctl a little harder to do in future, because you're adding in two variable-size elements, and have to do some fairly complicated parsing to even figure out what the size _should_ be. So I'd rather not do it if there was any way out of it; at the very least it'd have to be userptr rather than array.
Cheers, Daniel