On 29/10/13 19:06, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> So I took another look at the vblank timestamping code, and got a bit
> excited. The result is this patchset.
>
> Summary of changes:
> - kill crtc->hwmode dependency
> - eliminate a bunch of 64bit math
> - fix timestamps for stereo and interlaced modes (on i915 at least)
> - move the "early vbl irq" hack into radeon code
> - add a similar hack to i915, but make it as finely targeted
> as possibly to minimize the chance of accidentally
> applying it in the wrong place
>
> The s/clock/crtc_clock change could use some radeon people to verify
> whether changing radeon_atom_get_tv_timings() is enough to make
> crtc_clock always populated.
>
> This series applies on top of Mario's
> "Vblank timestamping improvements/fixes for Linux drm." series.
>
> Ville Syrjälä (14):
> drm: Pass the display mode to drm_calc_timestamping_constants()
> drm: Pass the display mode to
drm_calc_vbltimestamp_from_scanoutpos()
> drm/i915: Kill hwmode save/restore
> drm/i915: Call drm_calc_timestamping_constants() earlier
> drm: Improve drm_calc_timestamping_constants() documentation
> drm: Simplify the math in drm_calc_timestamping_constants()
> drm/radeon: Populate crtc_clock in radeon_atom_get_tv_timings()
> drm: Use crtc_clock in drm_calc_timestamping_constants()
> drm: Change {pixel,line,frame}dur_ns from s64 to int
> drm/i915: Fix scanoutpos calculations for interlaced modes
> drm: Fix vblank timestamping constants for interlaced modes
> drm: Pass 'flags' from the caller to .get_scanout_position()
> drm/radeon: Move the early vblank IRQ fixup to
radeon_get_crtc_scanoutpos()
> drm/i915: Add a kludge for DSL incrementing too late and ISR
not working
>
Hi Ville,
sorry this took way longer than expected. I've reviewed all of your
patches. Nice cleanups, nice improvements!
You can add a ...
Reviewed-by: mario.kleiner.de@xxxxxxxxx
... to all of them.
Patches 0 - 11 and 14 are fine as they are. Only tiny formatting/comment
fixes needed so they apply cleanly against the current drm-next.
Patch 12 and 13 need some small fixes, after applying those i'm fine
with them. I'll send separate e-mails for those.
As far as testing goes, i had more encounters with Murphy's law in the
last weeks than ever before, hence the long delay. You can add
Tested-by: mario.kleiner.de@xxxxxxxxx
to the drm core and intel patches with the following restrictions:
I was able to "sort of" test the patchset on Intel GMA-950 (Gen-3 hw).
- I didn't test if your interlaced scanout patches 10 and 11 work as
expected, because i was testing the patches first, then reviewing them,
so i didn't realize at that point testing interlaced mode would be
neccessary. The patches look correct to me though. I no longer have easy
access to that machine.
- My photodiode test equipment, which i need for Intel testing
malfunctioned. Not sure if my testing hardware is dying, or if it is a
bug in the kernels usb or serial/tty stack, or some kernel
misconfiguration wrt. low-latency, but there was so much timing noise in
my equipment that i couldn't test with it.
- As a workaround I ran the kms-timestamping for regular non-interlaced
mode against the original userspace implementation of the same code in
my own toolkit Psychtoolbox, which itself was verified with testing
equipment to do the right thing on that GMA-950 netbook earlier this
year. Difference was less than 40 microseconds and more likely caused
due to userspace noisyness and off-by-one errors in Psychtoolbox than
your code, so i assume that your code is essentially correct at least
for non-interlaced scanout, and that the DRM core changes are therefore
also correct. If you or somebody would want to try this test yourself i
can guide you through the steps. Psychtoolbox is easily apt-get'able for
Debian and at least Ubuntu.
- The next limitation of my testing is wrt. to your "early vbl irq
handling" improvements (patch 14). I currently only have Gen3 hardware
which doesn't exercise those code path at all, so while the patch looks
correct, it's not really tested by me.
As far as Radeon testing goes, i can't test it at all atm. After already
not working very stable at all for the last half year, my last machine
with an AMD card died during bootup for this test, but not without
trying to corrupt the filesystem on my development drive as a little
post-christmas gift to me. If somebody has a AMD card and wants to test
this, it could be tested against the Psychtoolbox userspace reference
implementation, which was verified with very precise external hardware
last time a couple of months ago. However, patch 13 needs some fixes or
it would crash. The now dead PC wasn't mine, but i still have the AMD card.
I will try to hunt for a new PC soon, and hopefully will get your
patches better tested during the -rc phase if they get merged into 3.14.
Apart from a NVidia card, my 2010 MacBookPro also has an integrated
Gen-4 Intel card, connected to the internal panel via hardware mux, but
so far i wasn't successfull at all to make use of it under Ubuntu 13.10.
I can power on the card, make it detected by vgaswitcheroo/kms and
manually switch the mux via some hacks, but it never boots into a
functional desktop :(. I haven't tried very hard though with more recent
kernels.
-mario
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
