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@amd.com
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@nuetzel-hh.de Signed-off-by: Michel Dänzer michel.daenzer@amd.com
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?