Re: [PATCH] kernel/drm: vblank wait on crtc > 1

["Michel Dänzer" <michel@daenzer.net>]

On Die, 2011-03-22 at 08:43 -0500, Ilija Hadzic wrote: 
> 
> On Tue, 22 Mar 2011, Michel [ISO-8859-1] Dnzer wrote:
> 
> >> If _DRM_VBLANK_HIGH_CRTC_MASK were included in _DRM_VBLANK_FLAGS_MASK
> >> (or _DRM_VBLANK_TYPES_MASK, but that would make less sense), these
> >> changes shouldn't be necessary.
> 
> HIGH_CRTC does not logically belong either flags nor types, but it's a 
> third thing and hence the separate mask.

I'd say 'logically' it belongs with _DRM_VBLANK_SECONDARY, i.e.
_DRM_VBLANK_FLAGS_MASK .


> >> I'd suggest making _DRM_VBLANK_HIGH_CRTC_SHIFT either 21 (so
> >> _DRM_VBLANK_HIGH_CRTC_MASK is adjacent to _DRM_VBLANK_EVENT) or much
> >> lower, say 8 or even 4, as the flags are more likely to get extended
> >> than the types, if history is any indication.
> 
> I can't have an opinion on that because I wasn't around (at least not in 
> the graphics subsystem world) to see the historic development of this 
> ioctl. However, I'd say that the motivation is rather speculative.
> 
> However, from pragmatic angle, at this point the change is already in (it 
> was in drm-core-next, last time in checked) and it is probably not a good 
> idea to shift things around and thus potentially create multiple 
> incompatible combinations of user spaces and kernels (even if they are 
> just test builds).

It's indeed unfortunate that the patch was applied despite outstanding
issues, but I don't think this is really a problem before it hits
mainline, and the userspace bits haven't been applied anywhere yet.


> Especially that the suggestion is based more on what 
> *may* happen in the future evolution of this ioctl rather than some 
> funamental problem. Furthermore, we have heard on the list that at the end 
> of the day, the "evolution" of this ioctl will be the basis for (later) 
> redesigning the new one and getting it better in many other aspects. If 
> that's indeed so, that's even less of a motivation to split hair.

I don't think it makes sense to replace the ioctl with a new one until
there's something that cannot (reasonably) be done with the existing
one. Migrating userspace to the new ioctl would incur pain of its own,
and more likely than not the new ioctl would at some point later turn
out to have its own issues and limitations as well.


> Unless I hear strong voice from others on the list to change the position 
> of HIGH_CRTC mask, I am reluctant to touch it at this point.

You failed to take simple suggestions to make the patch smaller and more
future-proof at once in several weeks, I'm afraid 'at this point' isn't
a very good argument in light of that.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel