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@xxxxxxxxxx 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.
--
Earthling Michel DÃnzer | http://www.vmware.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
