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@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.
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() .
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.
Yeah, agree totally but didn't had time for this yet (it has a rather
low priority). Might be a good task for someone to get his hands dirty
on the kernel code for the first time as well.
I think turning interrupts on and off at each fence wait is the reason
why inlining r100_mm_rreg and r100_mm_wreg again turned out to improve
performance for some people quite a bit.
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.
The issue is still that as far as I understand it when the vblank
interrupt fires the flip is actually not completed yet.
So we only don't see the pending bit high any more because of the
minimal time between vblank fires and we check the bit.
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.
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?
Good question, essentially we need to test with a TV capable of the 24Hz
modes. I don't have the necessary hardware here and last time was only
able to test this by torturing my poor HP monitor with a 24Hz modeline
it actually couldn't handle.
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.
Good to know, that narrows down the scenario how this problem is triggered.
[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?
It seemed to be the right thing for the job, but at this time I planned
to use it exclusively and stop bothering with the vblank interrupt at all.
But then Alex noted that the pflip interrupt might be unreliable on
older system and so I wanted to use the exiting vblank handling as
backup. Now I'm not sure any more what's the best approach here.
Regards,
Christian.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
