On Mit, 2011-03-09 at 11:33 -0600, Ilija Hadzic wrote:
>
> So what I did is to actually probe the kernel at screen init time.
> When the screen is initialized, the DDX checks if the GPU has more than
> two CRTCs and tries a dummy vblank wait on all crtcs > 1. If any
> of them fails, it falls back to the old method of using only
> primary/secondary flag.
I'm afraid I don't really like this. Apart from the ugly bogus error
message, the probes could fail for other reasons, e.g. at some point we
might want to return an error when the ioctl is called for a CRTC which
is currently disabled, to avoid the hang you were getting for CRTC 1.
Dave's drm-next tree actually has a new DRM_IOCTL_GET_CAP ioctl, which
could be used for detecting this capability. It might even be possible
to handle old kernels transparently in drmWaitVBlank(), but I'm not sure
offhand if that would be better overall than doing it in the driver
code.
Some detailed technical comments:
> diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
> index 66df03c..c45abe6 100644
> --- a/src/radeon_dri2.c
> +++ b/src/radeon_dri2.c
> @@ -1145,8 +1193,15 @@ static int radeon_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
> vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
> if (flip == 0)
> vbl.request.type |= DRM_VBLANK_NEXTONMISS;
> - if (crtc > 0)
> + if (crtc == 1)
> vbl.request.type |= DRM_VBLANK_SECONDARY;
> + else if (crtc > 1) {
> + if (info->high_crtc_works) {
> + high_crtc = (crtc << DRM_VBLANK_HIGH_CRTC_SHIFT) &
> + DRM_VBLANK_HIGH_CRTC_MASK;
> + } else vbl.request.type |= DRM_VBLANK_SECONDARY;
Waiting on a random other CRTC makes no sense. If you know you can't
wait on the given CRTC, don't wait at all.
> @@ -1261,6 +1319,22 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
> xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "You need a newer kernel for sync extension\n");
> }
>
> + if (info->drmmode.mode_res->count_crtcs > 2) {
> + xf86DrvMsg(pScrn->scrnIndex, X_INFO, "GPU with %d CRTCs found, probing VBLANKs on CRTC>1\n",
> + info->drmmode.mode_res->count_crtcs);
> + info->high_crtc_works = TRUE;
> + for (c=2; c<info->drmmode.mode_res->count_crtcs; c++) {
> + vbl.request.type = DRM_VBLANK_RELATIVE;
> + vbl.request.type |= (c << DRM_VBLANK_HIGH_CRTC_SHIFT) & DRM_VBLANK_HIGH_CRTC_MASK;
> + vbl.request.sequence = 0;
> + if (drmWaitVBlank(info->dri2.drm_fd, &vbl)) {
> + xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Kernel rejects VBLANK request on CRTC %d\n", c);
> + info->high_crtc_works = FALSE;
Should break out of the for loop at this point.
> + }
> + }
> + if (info->high_crtc_works) xf86DrvMsg(pScrn->scrnIndex, X_INFO, "VBLANK request accepted on all CRTCs\n");
The statement guarded by the if condition needs to go on a new line, and
please don't add trailing whitespace. Also, again, please match the
indentation of the surrounding code.
--
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
