<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 16, 2017 at 5:54 PM, 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"><div class="HOEnZb"><div class="h5">><br>
> In case the driver supports VSync control feature, it<br>
> maintains timer for VSync interrupt indication.<br>
> In further commits this timer will be started upon<br>
> class driver request. The interrupt notification and<br>
> related DPC do not access device registers.<br>
><br>
> Signed-off-by: Yuri Benditovich <<a href="mailto:yuri.benditovich@daynix.com" target="_blank">yuri.benditovich@daynix.com</a>><br>
> ---<br>
> qxldod/QxlDod.cpp | 82<br>
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++<br>
> qxldod/QxlDod.h | 14 ++++++++++<br>
> 2 files changed, 96 insertions(+)<br>
><br>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp<br>
> index cd22446..9175300 100755<br>
> --- a/qxldod/QxlDod.cpp<br>
> +++ b/qxldod/QxlDod.cpp<br>
> @@ -82,6 +82,12 @@ QxlDod::QxlDod(_In_ DEVICE_OBJECT* pPhysicalDeviceObject)<br>
> : m_pPhysicalDevice(pP<br>
> RtlZeroMemory(m_CurrentModes, sizeof(m_CurrentModes));<br>
> RtlZeroMemory(&m_PointerShape, sizeof(m_PointerShape));<br>
> m_pHWDevice = NULL;<br>
> +<br>
> + KeInitializeDpc(&m_VsyncTimerDpc, VsyncTimerProcGate, this);<br>
> + KeInitializeTimer(&m_VsyncTimer);<br>
> + m_VsyncFiredCounter = 0;<br>
> + m_bVsyncEnabled = FALSE;<br>
> +<br>
> DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));<br>
> }<br>
><br>
> @@ -198,6 +204,7 @@ NTSTATUS QxlDod::StopDevice(VOID)<br>
> {<br>
> PAGED_CODE();<br>
> m_Flags.DriverStarted = FALSE;<br>
> + EnableVsync(FALSE);<br>
> return STATUS_SUCCESS;<br>
> }<br>
><br>
> @@ -518,6 +525,7 @@ NTSTATUS QxlDod::QueryAdapterInfo(_In_ CONST<br>
> DXGKARG_QUERYADAPTERINFO* pQueryAda<br>
> pDriverCaps->PointerCaps.Color = 1;<br>
><br>
> pDriverCaps->SupportNonVGA = m_pHWDevice->IsBIOSCompatible();<br>
> + pDriverCaps->SchedulingCaps.VSyncPowerSaveAware =<br>
> g_bSupportVSync;<br>
><br><br></div></div>I think we could set this to TRUE. If OS supports VSync will deactivate<br>
if not should do nothing or try to deactivate.<br>
Could be if set to FALSE that OS still expect VSync coming or don't<br>
enable some energy saving policy.<br></blockquote><div><br></div><div>Setting it to TRUE when VSync is not supported may create some side-effect in Windows,<br></div><div>as this combination is not expected. Setting it to FALSE when VSync is supported also does</div><div>not make too much sense as then the driver will send indications also when the system</div><div>is completely inactive. So, only meaningful choice for us is to keep it equal to VSync control.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><br>
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s 1\n", __FUNCTION__));<br>
> return STATUS_SUCCESS;<br>
> @@ -4813,6 +4821,14 @@ BOOLEAN QxlDevice::InterruptRoutine(_In_<br>
> PDXGKRNL_INTERFACE pDxgkInterface, _In_<br>
> }<br>
><br>
> QXL_NON_PAGED<br>
> +VOID QxlDevice::VSyncInterruptPostProcess(_In_ PDXGKRNL_INTERFACE<br>
> pDxgkInterface)<br>
> +{<br>
> + if (!pDxgkInterface->DxgkCbQueueDpc(pDxgkInterface->DeviceHandle)) {<br>
> + DbgPrint(TRACE_LEVEL_WARNING, ("---> %s can't enqueue DPC, pending<br>
> interrupts %X\n", __FUNCTION__, m_Pending));<br>
> + }<br>
> +}<br>
> +<br>
> +QXL_NON_PAGED<br>
> VOID QxlDevice::DpcRoutine(PVOID)<br>
> {<br>
> LONG intStatus = InterlockedExchange(&m_Pending, 0);<br>
> @@ -4914,3 +4930,69 @@ NTSTATUS<br>
> HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInf<br>
> }<br>
> return Status;<br>
> }<br>
> +<br>
> +// Vga device does not generate interrupts<br>
> +QXL_NON_PAGED VOID VgaDevice::VSyncInterruptPostProcess(_In_<br>
> PDXGKRNL_INTERFACE pxface)<br>
> +{<br>
> + pxface->DxgkCbQueueDpc(pxface->DeviceHandle);<br>
> +}<br>
> +<br>
> +QXL_NON_PAGED VOID QxlDod::IndicateVSyncInterrupt()<br>
> +{<br><br></div></div>Maybe is more flexible if this function returns BOOLEAN.<br><span class=""><br>
> + DXGKARGCB_NOTIFY_INTERRUPT_DATA data = {};<br>
> + data.InterruptType = DXGK_INTERRUPT_DISPLAYONLY_VSYNC;<br>
> + m_DxgkInterface.DxgkCbNotifyInterrupt(m_DxgkInterface.DeviceHandle,<br>
> &data);<br>
> + m_pHWDevice->VSyncInterruptPostProcess(&m_DxgkInterface);<br>
> +}<br>
> +<br>
> +QXL_NON_PAGED BOOLEAN QxlDod::VsyncTimerSynchRoutine(PVOID context)<br>
> +{<br>
> + QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);<br>
> + pQxl->IndicateVSyncInterrupt();<br>
> + return FALSE;<br>
<br>
</span>Here would be<br><br>
return pQxl->IndicateVSyncInterrupt();<br><br>
so we could handle the return value from the member function (if needed<br>
in the future).<br><div><div class="h5"><br></div></div></blockquote><div><br></div><div>This procedure currently returns boolean just because Windows</div><div>synch routine prototype defined such a way without any reason.</div><div>There is no meaningful information that we can pass back on this boolean. </div><div>When synch procedure needs to return something little bigger than boolean,</div><div>local context passed to the routine does the job.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">> +}<br>
> +<br>
> +QXL_NON_PAGED VOID QxlDod::VsyncTimerProc()<br>
> +{<br>
> + BOOLEAN bDummy;<br>
> + DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));<br>
> + if (m_bVsyncEnabled && m_AdapterPowerState == PowerDeviceD0)<br>
> + {<br>
> + m_DxgkInterface.DxgkCbSynchronizeExecution(<br>
> + m_DxgkInterface.DeviceHandle,<br>
> + VsyncTimerSynchRoutine,<br>
> + this,<br>
> + 0,<br>
> + &bDummy<br>
> + );<br>
> + INCREMENT_VSYNC_COUNTER(&m_VsyncFiredCounter);<br>
> + }<br>
> +}<br>
> +<br>
> +VOID QxlDod::EnableVsync(BOOLEAN bEnable)<br>
> +{<br>
> + PAGED_CODE();<br>
> + if (g_bSupportVSync)<br>
> + {<br>
> + m_bVsyncEnabled = bEnable;<br>
> + if (!m_bVsyncEnabled)<br>
> + {<br>
> + DbgPrint(TRACE_LEVEL_WARNING, ("Disabled VSync(fired %d)\n",<br>
> InterlockedExchange(&m_VsyncFiredCounter, 0)));<br>
> + KeCancelTimer(&m_VsyncTimer);<br>
> + }<br>
> + else<br>
> + {<br>
> + LARGE_INTEGER li;<br>
> + LONG period = 1000 / VSYNC_RATE;<br>
> + DbgPrint(TRACE_LEVEL_WARNING, ("Enabled VSync(fired %d)\n",<br>
> m_VsyncFiredCounter));<br>
> + li.QuadPart = -10000000 / VSYNC_RATE;<br>
> + KeSetTimerEx(&m_VsyncTimer, li, period, &m_VsyncTimerDpc);<br>
> + }<br>
> + }<br>
> +}<br>
> +<br>
> +QXL_NON_PAGED VOID QxlDod::VsyncTimerProcGate(_In_ _KDPC *dpc, _In_ PVOID<br>
> context, _In_ PVOID arg1, _In_ PVOID arg2)<br>
> +{<br>
> + QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);<br>
> + pQxl->VsyncTimerProc();<br>
> +}<br>
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h<br>
> index 53724e8..9cfb6de 100755<br>
> --- a/qxldod/QxlDod.h<br>
> +++ b/qxldod/QxlDod.h<br>
> @@ -239,6 +239,7 @@ public:<br>
> QXL_NON_PAGED virtual BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE<br>
> pDxgkInterface, _In_ ULONG MessageNumber) = 0;<br>
> QXL_NON_PAGED virtual VOID DpcRoutine(PVOID) = 0;<br>
> QXL_NON_PAGED virtual VOID ResetDevice(void) = 0;<br>
> + QXL_NON_PAGED virtual VOID VSyncInterruptPostProcess(_In_<br>
> PDXGKRNL_INTERFACE) = 0;<br>
> virtual NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {<br>
> return STATUS_SUCCESS; }<br>
> virtual NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode) {<br>
> return STATUS_SUCCESS; }<br>
><br>
> @@ -306,6 +307,7 @@ public:<br>
> QXL_NON_PAGED BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE<br>
> pDxgkInterface, _In_ ULONG MessageNumber);<br>
> QXL_NON_PAGED VOID DpcRoutine(PVOID);<br>
> QXL_NON_PAGED VOID ResetDevice(VOID);<br>
> + QXL_NON_PAGED VOID VSyncInterruptPostProcess(_In_ PDXGKRNL_INTERFACE);<br>
> NTSTATUS AcquireFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode);<br>
> NTSTATUS ReleaseFrameBuffer(CURRENT_BDD_MODE* pCurrentBddMode);<br>
> NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*<br>
> pSetPointerShape);<br>
> @@ -358,8 +360,10 @@ enum {<br>
><br>
> #ifdef DBG<br>
> #define RESOURCE_TYPE(res, val) do { res->type = val; } while (0)<br>
> +#define INCREMENT_VSYNC_COUNTER(counter) InterlockedIncrement(counter)<br>
> #else<br>
> #define RESOURCE_TYPE(res, val)<br>
> +#define INCREMENT_VSYNC_COUNTER(counter)<br>
> #endif<br>
><br>
> typedef struct Resource Resource;<br>
> @@ -480,6 +484,7 @@ public:<br>
> QXL_NON_PAGED BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE<br>
> pDxgkInterface, _In_ ULONG MessageNumber);<br>
> QXL_NON_PAGED VOID DpcRoutine(PVOID);<br>
> QXL_NON_PAGED VOID ResetDevice(VOID);<br>
> + QXL_NON_PAGED VOID VSyncInterruptPostProcess(_In_ PDXGKRNL_INTERFACE);<br>
> NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE*<br>
> pSetPointerShape);<br>
> NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION*<br>
> pSetPointerPosition);<br>
> NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);<br>
> @@ -622,6 +627,10 @@ private:<br>
> DXGKARG_SETPOINTERSHAPE m_PointerShape;<br>
><br>
> HwDeviceInterface* m_pHWDevice;<br>
> + KTIMER m_VsyncTimer;<br>
> + KDPC m_VsyncTimerDpc;<br>
> + BOOLEAN m_bVsyncEnabled;<br>
> + LONG m_VsyncFiredCounter;<br>
> public:<br>
> QxlDod(_In_ DEVICE_OBJECT* pPhysicalDeviceObject);<br>
> ~QxlDod(void);<br>
> @@ -719,6 +728,7 @@ public:<br>
> {<br>
> return<br>
> m_DxgkInterface.DxgkCbAcquirePostDisplayOwnership(m_DxgkInterface.DeviceHandle,<br>
> &DispInfo);<br>
> }<br>
> + VOID EnableVsync(BOOLEAN bEnable);<br>
> private:<br>
> VOID CleanUp(VOID);<br>
> NTSTATUS CheckHardware();<br>
> @@ -744,6 +754,10 @@ private:<br>
> NTSTATUS IsVidPnSourceModeFieldsValid(CONST D3DKMDT_VIDPN_SOURCE_MODE*<br>
> pSourceMode) const;<br>
> NTSTATUS IsVidPnPathFieldsValid(CONST D3DKMDT_VIDPN_PRESENT_PATH* pPath)<br>
> const;<br>
> NTSTATUS RegisterHWInfo(_In_ ULONG Id);<br>
> + QXL_NON_PAGED VOID VsyncTimerProc();<br>
> + static QXL_NON_PAGED VOID VsyncTimerProcGate(_In_ _KDPC *dpc, _In_ PVOID<br>
> context, _In_ PVOID arg1, _In_ PVOID arg2);<br>
> + QXL_NON_PAGED VOID IndicateVSyncInterrupt();<br>
> + static QXL_NON_PAGED BOOLEAN VsyncTimerSynchRoutine(PVOID context);<br>
> };<br>
><br>
> NTSTATUS<br><br></div></div>Otherwise looks good.<br></blockquote></div></div></div></blockquote><div>Acked-by: Frediano Ziglio <fziglio@redhat.com><br></div><div><br></div><div>Frediano<br></div><div><br></div></div></body></html>