>
> Do not clear interrupt mask upon interrupt.
> Instead clear pending interrupt status and write
> QXL_IO_UPDATE_IRQ register (this drops interrupt level).
> There are 3 advantages:
> 1. We do not need to wake the host to enable interrupt
> mask in DPC procedure (1 wake per interrupt instead of 2)
> 2. The driver is not sensitive to failure when queues DPC, as
> already queued DPC will process this interrupt when executed.
> 3. When we implement VSync interrupt simulation, we do not
> need to touch registers neither when notify the OS nor when
> process DPC related to this notification.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> ---
> qxldod/QxlDod.cpp | 54
> ++++++++++++------------------------------------------
> qxldod/QxlDod.h | 4 +---
> 2 files changed, 13 insertions(+), 45 deletions(-)
>
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 2d2a022..cd0a1e5 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -4799,14 +4799,14 @@ BOOLEAN QxlDevice::InterruptRoutine(_In_
> PDXGKRNL_INTERFACE pDxgkInterface, _In_
> if (!(m_RamHdr->int_pending & m_RamHdr->int_mask)) {
> return FALSE;
> }
> - m_RamHdr->int_mask = 0;
> + m_Pending |= InterlockedExchange((LONG *)&m_RamHdr->int_pending, 0);
I was afraid int_pending could be the wrong type or not properly
aligned but luckily it is so this line works fine (and is supposed
to continue to work).
> WRITE_PORT_UCHAR((PUCHAR)(m_IoBase + QXL_IO_UPDATE_IRQ), 0);
> - m_Pending |= m_RamHdr->int_pending;
> - m_RamHdr->int_pending = 0;
> + // QXL_IO_UPDATE_IRQ sets interrupt level to m_RamHdr->int_pending &
> m_RamHdr->int_mask
> + // so it will be dropped if interrupt status is not modified after clear
> +
> if (!pDxgkInterface->DxgkCbQueueDpc(pDxgkInterface->DeviceHandle)) {
> - m_RamHdr->int_mask = WIN_QXL_INT_MASK;
> - WRITE_PORT_UCHAR((PUCHAR)(m_IoBase + QXL_IO_UPDATE_IRQ), 0);
> - DbgPrint(TRACE_LEVEL_FATAL, ("---> %s DxgkCbQueueDpc failed\n",
> __FUNCTION__));
> + // DPC already queued and will process m_Pending when called
> + DbgPrint(TRACE_LEVEL_WARNING, ("---> %s can't queue Dpc for %X\n",
> __FUNCTION__, m_Pending));
> }
> DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> return TRUE;
> @@ -4815,34 +4815,22 @@ BOOLEAN QxlDevice::InterruptRoutine(_In_
> PDXGKRNL_INTERFACE pDxgkInterface, _In_
> QXL_NON_PAGED
> VOID QxlDevice::DpcRoutine(PVOID ptr)
> {
> - PDXGKRNL_INTERFACE pDxgkInterface = (PDXGKRNL_INTERFACE)ptr;
> -
> + LONG intStatus = InterlockedExchange(&m_Pending, 0);
> + UNREFERENCED_PARAMETER(ptr);
I'd use
VOID QxlDevice::DpcRoutine(PVOID)
instead but it's just style
> DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
> - DPC_CB_CONTEXT ctx;
> - BOOLEAN dummy;
> - ctx.ptr = this;
> - NTSTATUS Status = pDxgkInterface->DxgkCbSynchronizeExecution(
> - pDxgkInterface->DeviceHandle,
> - DpcCallbackEx,
> - &ctx,
> - 0,
> - &dummy);
> - ASSERT(Status == STATUS_SUCCESS);
> -
> - if (ctx.data & QXL_INTERRUPT_DISPLAY) {
> +
> + if (intStatus & QXL_INTERRUPT_DISPLAY) {
> DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s m_DisplayEvent\n",
> __FUNCTION__));
> KeSetEvent (&m_DisplayEvent, IO_NO_INCREMENT, FALSE);
> }
> - if (ctx.data & QXL_INTERRUPT_CURSOR) {
> + if (intStatus & QXL_INTERRUPT_CURSOR) {
> DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s m_CursorEvent\n",
> __FUNCTION__));
> KeSetEvent (&m_CursorEvent, IO_NO_INCREMENT, FALSE);
> }
> - if (ctx.data & QXL_INTERRUPT_IO_CMD) {
> + if (intStatus & QXL_INTERRUPT_IO_CMD) {
> DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s m_IoCmdEvent\n",
> __FUNCTION__));
> KeSetEvent (&m_IoCmdEvent, IO_NO_INCREMENT, FALSE);
> }
> - m_RamHdr->int_mask = WIN_QXL_INT_MASK;
> - WRITE_PORT_UCHAR((PUCHAR)(m_IoBase + QXL_IO_UPDATE_IRQ), 0);
>
> DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> }
> @@ -4859,24 +4847,6 @@ VOID QxlDevice::UpdateArea(CONST RECT* area, UINT32
> surface_id)
> }
>
> QXL_NON_PAGED
> -BOOLEAN QxlDevice:: DpcCallbackEx(PVOID ptr)
> -{
> - DbgPrint(TRACE_LEVEL_VERBOSE, ("<--> %s\n", __FUNCTION__));
> - PDPC_CB_CONTEXT ctx = (PDPC_CB_CONTEXT) ptr;
> - QxlDevice* pqxl = (QxlDevice*)ctx->ptr;
> - pqxl->DpcCallback(ctx);
> - return TRUE;
> -}
> -
> -QXL_NON_PAGED
> -VOID QxlDevice::DpcCallback(PDPC_CB_CONTEXT ctx)
> -{
> - ctx->data = m_Pending;
> - m_Pending = 0;
> -
> -}
> -
> -QXL_NON_PAGED
> UINT BPPFromPixelFormat(D3DDDIFORMAT Format)
> {
> switch (Format)
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 4598718..53724e8 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -549,8 +549,6 @@ private:
> void PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> UINT8 **end_ptr, UINT8 *src, int size,
> size_t alloc_size, uint32_t alignment);
> - QXL_NON_PAGED BOOLEAN static DpcCallbackEx(PVOID);
> - QXL_NON_PAGED void DpcCallback(PDPC_CB_CONTEXT);
> void AsyncIo(UCHAR Port, UCHAR Value);
> void SyncIo(UCHAR Port, UCHAR Value);
> NTSTATUS UpdateChildStatus(BOOLEAN connect);
> @@ -602,7 +600,7 @@ private:
> MspaceInfo m_MSInfo[NUM_MSPACES];
>
> UINT64 m_FreeOutputs;
> - UINT32 m_Pending;
> + LONG m_Pending;
>
> QXLMonitorsConfig* m_monitor_config;
> QXLPHYSICAL* m_monitor_config_pa;
Otherwise
Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel