On Fri, Oct 21, 2016 at 11:55:52AM +0100, Brian Starkey wrote:
On Thu, Oct 20, 2016 at 06:30:17PM -0200, Gustavo Padovan wrote:
2016-10-20 Brian Starkey brian.starkey@arm.com:
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 657a33a..b898604 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -165,6 +165,8 @@ struct drm_crtc_state { struct drm_property_blob *ctm; struct drm_property_blob *gamma_lut;
- u64 __user *out_fence_ptr;
I'm somewhat not convinced about stashing a __user pointer in the CRTC atomic state. I know it gets cleared before the driver sees it, but if anything I think that hints that this isn't the right place to store it.
I've been trying to think of other ways to get from a property to something drm_mode_atomic_ioctl can use - maybe we can store some stuff in drm_atomic_state which only lives for the length of the ioctl call and put this pointer in there.
The drm_atomic_state is still visible by the drivers. Between there and crtc_state, I would keep in the crtc_state for now.
Mm, yeah I suppose they could get to it if they went looking for it in ->atomic_set_property or something, but I was thinking of freeing it before the drm_atomic_commit.
Anyway, this way is certainly simpler, and setting it to NULL should be enough to limit the damage a driver can do :-)
+1 on moving this out of drm_crtc_state. drm_atomic_state already has per-crtc structs, so easy to extend them with this. And yes drivers can still see it, but mostly they're not supposed to touch drm_atomic_state internals - the book-keeping is all done by the core.
The per-object state structs otoh are meant to be massively mangled by drivers. -Daniel