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