On 06/29/2015 10: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_CRTDC_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

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


The bits are not a bit mask where a specific bit position corresponds to a specific pipe, but those bits encode a 5 bits wide pipe number between 0 and (2^5)-1 ie. 0-31. The comment in the code snippet you show about bits 1-6 is a bit inaccurate, as it is only bits 1-5, looking at the DRM_VBLANK_HIGH_CRTC_MASK.

You could address output->pipe 0 and 1 also with that code. The reason for the special case of pipe 0 and 1 is backwards compatibility with older kernels which didn't support passing in a pipe number, but instead had "no special flags" for pipe 0 and the DRM_VBLANK_SECONDARY flag for pipe 1. A leftover from the days when there were only single/dual-head graphis cards.

But apart from that this is just code transplanted from the x-org video drivers which i tested at some point both on NVidia quad-head (nouveau) and AMD six-head gpu's (radeon).

-mario
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to