On 27.06.2014 17:18, Christian König wrote: > Am 27.06.2014 04:58, schrieb Michel Dänzer: >> On 26.06.2014 19:39, Christian König wrote: >>> Am 26.06.2014 11:29, schrieb Michel Dänzer: >>>> From: Michel Dänzer <michel.daenzer@xxxxxxx> >>>> >>>> Prevents radeon_crtc_handle_flip() from running before >>>> radeon_flip_work_func(), resulting in a kernel panic due to >>>> the BUG_ON() in drm_vblank_put(). >>>> >>>> Tested-by: Dieter Nützel <Dieter@xxxxxxxxxxxxx> Signed-off-by: >>>> Michel Dänzer <michel.daenzer@xxxxxxx> >>> Does patch #2 alone fixes the problem as well? >> It should avoid the panic as well. >> >> >>> I would rather want to avoid turning the pflip interrupt on and >>> off. >> What's the problem with that? It's not like we're saving any >> register writes by not doing it. > > We don't? As far as I can see we reprogram all interrupt registers > if any of the interrupts changed, Maybe I'm missing something, but: radeon_irq_kms_pflip_irq_get/put() call radeon_irq_set() every time, as there can only be one active page flip per CRTC. And radeon_irq_set() always writes the same registers, only the bits it writes to them change depending on which interrupts the driver is currently interested in. > this has already lead to quite some additional overhead in the fence > waiting code. radeon_irq_set() should probably be split up to reduce the overhead. >> The diagnostic messages Dieter was getting with only patch #2 show >> that the pflip interrupt often triggers unnecessarily, potentially >> wasting power by waking up the CPU from a power saving state >> pointlessly. > That's a really good point, but my question would rather be why does > the pflip interrupt fires if there isn't any pflip? There is a page flip, but it already completes in the vertical blank interrupt handler in a lot of (most?) cases. Which brings me back to the question: Do we really need the pflip interrupt yet? [0] Since flips are no longer programmed to the hardware in the vertical blank handler but in a work queue, is there actually still any problem with handling the flip completion in the vertical blank interrupt handler? FWIW, by disabling the radeon_crtc_handle_flip() call from the pflip interrupt handler, I no longer seem to be able to reproduce the 'impossible msc' lines in the Xorg log file. [0] Of course the pflip interrupt will be needed for asynchronous flips, but that doesn't mean we have to use it for all flips? -- 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
