On Thu, Jun 20, 2013 at 03:44:56PM +0300, Ville Syrj?l? wrote: > On Thu, Jun 20, 2013 at 01:27:14PM +0100, Chris Wilson wrote: > > On Thu, Jun 20, 2013 at 01:29:10PM +0300, Ville Syrj?l? wrote: > > > On Thu, Jun 20, 2013 at 10:14:36AM +0100, Chris Wilson wrote: > > > > On Thu, Jun 20, 2013 at 11:17:16AM +0300, Ville Syrj?l? wrote: > > > > > We already have a bit of pitch checking in intel_framebuffer_init(). > > > > > In fact there's a FIXME about pre-ilk limits there. > > > > > > > > It looks tidier to fix that check. We still need to double check the > > > > values though as the tiling mode is independent of the fb config and may > > > > be changed by the user. > > > > > > True, some checking needs to be done after pinning. > > > > > > I guess we could have one function that has the checks, and just call it > > > from both places. > > > > I'm thinking we note the tiling (and anything else of significance) > > during framebuffer_init() and reject the set-base if the object changes. > > I don't think any userspace depends upon being able to change the object > > after AddFB, and I don't think we want to allow the fb/obj to so easily > > become inconsistent. > > An alternative idea I've been tossing around is that we'd reject tiling > changes while we have drm_framebuffers pointing at the object. So some > kind of fb_refcount on the object. That would make buggy user space trip > over a bit earlier. But either way seems reasonable to me. That's viable; we need obj->fb tracking for frontbuffer write detection as well. We can prevent such changes - as it is semantically equivalent to EINVAL for pinned objects - once that tracking lands. -Chris -- Chris Wilson, Intel Open Source Technology Centre
