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

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

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.


-- 
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