On 24.06.2014 05:32, Dieter Nützel wrote: > Am 23.06.2014 21:46, schrieb Dieter Nützel: >> Am 23.06.2014 11:34, schrieb Michel Dänzer: >>> On 18.06.2014 18:14, Christian König wrote: >>>> Am 18.06.2014 07:53, schrieb Michel Dänzer: >>>>> >>>>> (WW) RADEON(0): radeon_dri2_flip_event_handler: Pageflip completion >>>>> event has impossible msc [x-1] < target_msc [x] >>>>> >>>>> messages in the X log file which have been popping up in bug reports >>>>> lately. >>>>> This also results in 0s being returned to the client for the MSC and >>>>> timestamp of the swap completion, which could cause all kinds of bad >>>>> behaviour. >>>> First of all thanks for looking into it. Are you getting this on >>>> 3.16 or >>>> 3.15? >>> >>> I haven't actually run into this myself yet. I thought I'd seen it in >>> several bug reports, but right now I can only find >>> https://bugs.freedesktop.org/show_bug.cgi?id=80029#c17 , which seems to >>> include the page flipping changes from 3.16. >> >> With 3.16-rc2 I get it now on my RV730 AGP as in the above bug report. >> But only the lines in Xorg.0.log. >> NO signs of any damage/error in use. >> >> Since 3.15 and 3.16 (rc2 only) my system is rock solid. >> >> I've tried 3.15-rc7 + Christian's pflip rework (did some little >> handwork), too. >> It was solid but I saw the reported flip/black distortion in the below >> part during Kwin 4.13 cube screen effect (rotation). Your fix for >> 3.16-rc1 fixed that. That's good to hear. > I can reliable generate such lines in Xorg.0.log with KWin cube desktop > effect. > > Rotate screens with mouse wheel or screen switcher => new entry in > Xorg.0.log. If it happens I notice ('see') flip delay. I was only able to reproduce it a couple of times even with that, but not at all yet with the patch below. Does it help for you as well? >>>> I don't think that the pflip irq is thrown earlier than the vblank, but >>>> on 3.16 it might actually be that we program the flip so fast into the >>>> hardware that we do it one frame earlier than planned. >>> >>> So userspace is notified of the previous vertical blank period and calls >>> the page flip ioctl in response, which then manages to program the >>> scanout address update into the hardware before the scanout address >>> update is latched during the previous vertical blank period? I think there's another possible scenario: 1. Userspace submits page flip intended for MSC x 2. The vertical blank interrupt is triggered for MSC x => radeon_crtc_handle_vblank() => radeon_crtc_handle_flip() 3. Userspace submits page flip intended for MSC (x + 1) 4. The page flip interrupt is triggered for the previous flip => radeon_crtc_handle_flip() => drm_send_vblank_event(). The second flip hasn't actually executed yet, and the event has MSC x instead of (x + 1) as expected by userspace. If that is the case, only actually enabling and handling the page flip interrupt when a flip is pending might also avoid it. I can hack that up tomorrow, if Christian doesn't beat me to it. diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 8b575a4..8350f8c 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -336,14 +336,19 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; struct radeon_flip_work *work; unsigned long flags; + struct timeval vblank_time; + u32 vblank_seq; /* this can happen at init */ if (radeon_crtc == NULL) return; + vblank_seq = drm_vblank_count_and_time(rdev->ddev, crtc_id, &vblank_time); + spin_lock_irqsave(&rdev->ddev->event_lock, flags); work = radeon_crtc->flip_work; - if (work == NULL) { + if (work == NULL || + (vblank_seq - work->event->event.sequence) > (1<<23)) { spin_unlock_irqrestore(&rdev->ddev->event_lock, flags); return; } @@ -379,6 +384,7 @@ static void radeon_flip_work_func(struct work_struct *__work) struct drm_crtc *crtc = &radeon_crtc->base; struct drm_framebuffer *fb = work->fb; + struct timeval vblank_time; uint32_t tiling_flags, pitch_pixels; uint64_t base; @@ -466,6 +472,10 @@ static void radeon_flip_work_func(struct work_struct *__work) goto pflip_cleanup; } + work->event->event.sequence = + drm_vblank_count_and_time(crtc->dev, radeon_crtc->crtc_id, + &vblank_time) + 1; + /* We borrow the event spin lock for protecting flip_work */ spin_lock_irqsave(&crtc->dev->event_lock, flags); -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
