<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 02/11/2013 10:13 AM, Thierry Reding wrote: </div> <blockquote cite="mid:20130211091301.GC3423@avionic-0098.mockup.avionic-design.de" type="cite"> <pre wrap="">On Tue, Jan 22, 2013 at 06:37:39PM +0100, Mario Kleiner wrote: </pre> <blockquote type="cite"> <pre wrap="">On 14.01.13 17:05, Thierry Reding wrote: </pre> <blockquote type="cite"> <pre wrap="">Implement support for the VBLANK IOCTL. Note that Tegra is somewhat special in this case because it doesn't use the generic IRQ support provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one interrupt handler for each display controller. While at it, clean up the way that interrupts are enabled to ensure that the VBLANK interrupt only gets enabled when required. Signed-off-by: Thierry Reding <a class="moz-txt-link-rfc2396E" href="mailto:thierry.reding@avionic-design.de"><thierry.reding@avionic-design.de></a> </pre> </blockquote> <pre wrap=""> ... snip ... </pre> <blockquote type="cite"> <pre wrap=""> struct drm_driver tegra_drm_driver = { .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM, .load = tegra_drm_load, @@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = { .open = tegra_drm_open, .lastclose = tegra_drm_lastclose, + .get_vblank_counter = drm_vblank_count, </pre> </blockquote> <pre wrap=""> -> .get_vblank_counter = drm_vblank_count is a no-op. .get_vblank_counter() is supposed to return some real hardware vblank counter value to reinitialize the software vblank counter at vbl irq enable time. That software counter gets queried via drm_vblank_count(). If you hook this up to drm_vblank_count() it essentially returns a constant, frozen vblank count, it has the same effect as returning zero or any other constant value -- You lose all vblank counter increments during vblank irq off time. The same problem is present in nouveau-kms. I think it would be better to either implement a real hw counter query, or some function with a /* TODO: Implement me properly */ comment which returns zero, so it is clear something is missing here. </pre> </blockquote> <pre wrap=""> I've finally managed to find some time to look into this some more. The comment atop the drm_driver.get_vblank_counter() field actually suggests that drivers should set this to drm_vblank_count if no real hardware counter is supported. </pre> </blockquote> It certainly works fine that way. I just think it hides that some implementation is missing there, whereas an extra no-op function makes that clear to the reader. <blockquote cite="mid:20130211091301.GC3423@avionic-0098.mockup.avionic-design.de" type="cite"> <pre wrap=""> Now it seems like we may get functionality to obtain the real VBLANK counter once the syncpoint support is merged, so maybe we can leave this as-is until then and replace it with a proper implementation at that point in time? </pre> </blockquote> Perfectly fine with me. ciao, -mario <blockquote cite="mid:20130211091301.GC3423@avionic-0098.mockup.avionic-design.de" type="cite"> <pre wrap=""> Alternatively I could use a small wrapper with an explicit comment that this should be implemented using the upcoming syncpoint support. Thierry </pre> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ dri-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/dri-devel">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a> </pre> </blockquote> </body> </html>