Re: [PATCH 1/2] Revert "drm/radeon: remove drm_vblank_get|put from pflip handling"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 24.06.2014 12:05, schrieb Michel Dänzer:
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?

Will try in the next run.

My daughter generated kernel crash for us.;-)
See would open up a zoom image in Konqi of a new Waveboard for here girl friends...

But I could only take images with my mobile.
kernel BUG at drivers/gpu/drm/drm_irq.c:976!
Will send one, have two more.

Greetings,
  Dieter

2nd, try.
This time without image.
Let me know where I should add it, please.


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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux