On Die, 2011-03-22 at 06:16 -0500, Ilija Hadzic wrote:
Unless I oversaw something nothing was silently ignored. I believe I responded to each of your comments (and comments by others), those I agreed with I implemented, those I didn't agree with I didn't implement.
I haven't seen any response to the below excerpts from 1299251679.14068.83.camel@thor.local and the post you replied to:
------------------------- kernel patch ----------------------------------------
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 16d5155..3b0abae 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -677,16 +677,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data, return -EINVAL;
if (vblwait->request.type &
~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK)) {
~(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK |_DRM_VBLANK_HIGH_CRTC_MASK)) { DRM_ERROR("Unsupported type value 0x%x, supported mask 0x%x\n", vblwait->request.type,
(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK));
(_DRM_VBLANK_TYPES_MASK | _DRM_VBLANK_FLAGS_MASK |_DRM_VBLANK_HIGH_CRTC_MASK)); return -EINVAL; }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.
diff --git a/include/drm/drm.h b/include/drm/drm.h index e5f7061..d950581 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -469,6 +469,8 @@ enum drm_vblank_seq_type { _DRM_VBLANK_SECONDARY = 0x20000000, /**< Secondary display controller */ _DRM_VBLANK_SIGNAL = 0x40000000 /**< Send signal instead of blocking, unsupported */ }; +#define _DRM_VBLANK_HIGH_CRTC_SHIFT 16 +#define _DRM_VBLANK_HIGH_CRTC_MASK 0x001F0000
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.
Also,
#define _DRM_VBLANK_HIGH_CRTC_MASK (0x1F << _DRM_VBLANK_HIGH_CRTC_SHIFT)
would make it obvious how these values are related and decrease the likelihood of divergence during development of the patch.
[...]
@@ -753,6 +755,7 @@ struct drm_event_vblank { };
#define DRM_CAP_DUMB_BUFFER 0x1 +#define DRM_CAP_HIGH_CRTC 0x2
Seems like a rather generic name, something like DRM_CAP_VBLANK_HIGH_CRTC might be better.