<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 13, 2017 at 11:50 AM, Frediano Ziglio <span dir="ltr"><<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">><br>
> With default period of VSync interrupt notification<br>
> (1sec/refresh rate) the driver with device rev.4<br>
> has a problem when the system starts running after<br>
> restart. Until the issue is solved we set the notification<br>
> period to 200 ms, with this value both rev.3 and rev.4<br>
> function correctly. Final decision about notification<br>
> period postponed until the investigation is done.<br>
><br>
<br>
</span>Honestly all these VSync patches looks like a workaround<br>
just to make Windows tests happy.<br>
<br></blockquote><div><br></div><div>That's correct. There is no other way to specify the frequency. If I find one this makes our life easier. But till now I do not succeed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
We don't have an h/w monitor so we are providing fake values.<br>
We emulate an interrupt with a timer. I think these stuff should<br>
be avoided just to save CPU power avoiding wake ups not<br>
required.<br></blockquote><div><br></div><div>I also think so, but such a way we do not pass certification tests. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also we tweak interrupts and require additional communication<br>
with the server to handle them. Are we sure we don't introduce<br>
delays if we need to process commands like updates that<br>
require feedback from the server?<br></blockquote><div><br></div><div>When we provide interrupt indication at low rate, we minimize the impact.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
One of the "classic" usages of VSync is to handle double<br>
buffering used mostly on games. Are we sure that with these<br>
patches we don't end up with games running at 5 FPS to keep in<br>
sync with this emulated VSync?<br></blockquote><div><br></div><div>We are not sure as we do not have setup for gaming over QXL/Spice.</div><div>It would be good to create such one.</div><div>For diagnostics/troubleshooting I would allow registry workaround to disable this feature (VSync control) in user environment, this is to be discussed soon.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
As a minor m_VsyncFiredCounter is updated just for debugging<br>
purposes.<br></blockquote><div><br></div><div>Yes, that's correct.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I suspect this problem with restart is due to int_mask<br>
settings (m_RamHdr->int_mask = 0).<br></blockquote><div><br></div><div>If we do not do that (int mask setting) during VSync notification:</div><div>1) this does not solve the problem during restart</div><div>2) this creates a problem for functional interrupts - in case the functional interrupt comes when DPC is queued by VSync notification, the functional interrupt will not be able to queue its DPC and waiting GDI thread will not be notified.</div><div><br></div><div>It looks like the problem with restart happens when responsiveness of client is slow. I'm trying to debug it, but this is not so simple as this happens during system start and logging is currently problematic. WPP can help with this and it is in to-do list.</div><div><br></div><div>If you think this int mask setting is incorrect, please explain why (even offline), this is important.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> Signed-off-by: Yuri Benditovich <<a href="mailto:yuri.benditovich@daynix.com">yuri.benditovich@daynix.com</a>><br>
> ---<br>
> qxldod/QxlDod.cpp | 6 ++++--<br>
> 1 file changed, 4 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp<br>
> index fcca7d1..6a50265 100755<br>
> --- a/qxldod/QxlDod.cpp<br>
> +++ b/qxldod/QxlDod.cpp<br>
> @@ -4972,10 +4972,12 @@ VOID QxlDod::EnableVsync(BOOLEAN bEnable)<br>
> }<br>
> else<br>
> {<br>
> + // set notification period to 200 ms for now, see commit comment<br>
> for details<br>
> + LONG val = 200;<br>
> LARGE_INTEGER li;<br>
> - LONG period = 1000 / VSYNC_RATE;<br>
> + LONG period = val;<br>
> DbgPrint(TRACE_LEVEL_WARNING, ("Enabled VSync(fired %d)\n",<br>
> m_VsyncFiredCounter));<br>
> - li.QuadPart = -10000000 / VSYNC_RATE;<br>
> + li.QuadPart = -10000 * val;<br>
> KeSetTimerEx(&m_VsyncTimer, li, period, &m_VsyncTimerDpc);<br>
> }<br>
> }<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">Frediano<br>
</font></span></blockquote></div><br></div></div>