On 29/06/15 03:20 PM, Bill Spitzak wrote:
> On Fri, Jun 26, 2015 at 11:34 AM, Derek Foreman <[email protected]
> <mailto:[email protected]>> wrote:
> 
> 
>     > +static unsigned int drm_waitvblank_pipe(struct drm_output *output)
>     > +{
>     > +     if (output->pipe > 1)
>     > +             return (output->pipe << DRM_VBLANK_HIGH_CRTC_SHIFT) &
>     > +                             DRM_VBLANK_HIGH_CRTC_MASK;
> 
> 
> This does not seem right as the individual bits in the pipe number
> cannot really change things in any useful way. From the header file I
> see this:
> 
> +    /* bits 1-6 are reserved for high crtcs */
> +    DRM_VBLANK_HIGH_CRTC_MASK = 0x0000003e,
> +#define DRM_VBLANK_HIGH_CRTC_SHIFT 1
> 
> My best guess is that the intended code was something like:
> 
>   (1 << output->pipe - 2 + DRM_VBLANK_HIGH_CRTC_SHIFT) & 
> DRM_VBLANK_HIGH_CRTC_MASK

The idea isn't to have a single set bit in that range, no.

> Or is output->pipe always a power of 2?

It may seem a little funky, but a google search for
DRM_VBLANK_HIGH_CRTC_SHIFT will show this as the common idiom.

It's not a single set bit, it's a 5 bit wide number shifted.  The kernel
code masks it and shifts it back down symmetrically to what you see here.

The code here will break if the output number doesn't fit in the mask,
but that'd be a pretty serious number of pipes.  :)
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to