Am 30.06.2014 14:31, schrieb Christian König:
Am 30.06.2014 11:34, schrieb Michel Dänzer:
On 27.06.2014 19:47, Christian König wrote:
Am 27.06.2014 11:44, schrieb Michel Dänzer:
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've sent a v2 of that patch with an updated commit log. Alex, please get that into 3.16 ASAP to prevent people from running into the panic.
Compiling...
> 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.
We first turn on the vblank interrupt which results in a radeon_irq_set() and then turn on the pflip, which results in another radeon_irq_set() .
The DRM core delays disabling the hardware vblank interrupt, so radeon_irq_set() should only be called once for that.
radeon_irq_kms_pflip_irq_get/put() always call radeon_irq_set() even before my changes.
Anyway, for the issues surrounding the pflip interrupt, I took a step back and considered a fundamentally different approach: It occurred to me that a lot of the issues we've been struggling with are related to programming the flips to the hardware such that they execute during the vertical blank period. We don't know exactly when the hardware update happens or when would be a good time to program the flip. So why bother with that at all?
I've considered this as well, but at this time still hoped that we could completely replace the vblank interrupt with the pflip interrupt.
The patch below (only fleshed out for CIK) moves the programming of the flip to the hardware back to the vertical blank interrupt handler, but changes it to execute during the horizontal blank period. In addition to allowing the pflip interrupt handling, radeon_crtc_handle_vblank() and radeon_flip_pending() to be removed, this should make adding support for asynchronous flips trivial and support for replacing pending flips much easier. What do you think?
It also allows us to stop fiddling with the update pending bit as well, e.g. no more busy waiting for it to become high in evergreen_page_flip.
And as far as I can see it still support all not so trivial use cases like switching to hblank if rendering isn't completed before a certain timeout and dynamic refresh etc...
Yeah, sounds like a really good idea to me.
The delay between vblank start and the flip being executed seemed to be depending on the pixel clock (which makes sense because the CRTC is driven by it), so when it might work ok for a 50Hz mode we can still run into problems with 24Hz modes.
I couldn't see any tearing with this patch at 640x480 at 60 Hz and reduced blanking, which should have a lower pixel clock and shorter vertical blank period than 1920x1080 at 24 Hz?
In theory that should do the trick as well. But let's just make patches for it and then we can make a request to the people who originally reported the problem to test the whole implementation.
That should give us enough confidence that it will work correctly, Christian.
So please flesh something out (for r600 at least) and let me try ;-)
BTW Have someone looked at this (dpm typos): Bug 79071 - Hang with dpm radeon hd 5750, pcie 1.1 motherboard https://bugzilla.kernel.org/show_bug.cgi?id=79071