[ros-diffs] [janderwald] 39954: - Use Interlocked*List function to improve stability as a few of the timing issues have been resolved - Queue a dpc for each completed buffer - Complete the irp as soon as possible

Author: janderwald
Date: Wed Mar 11 21:00:11 2009
New Revision: 39954

URL: http://svn.reactos.org/svn/reactos?rev=39954&view=rev
Log:
- Use Interlocked*List function to improve stability as a few of the timing issues have
been resolved
- Queue a dpc for each completed buffer
- Complete the irp as soon as possible

Modified:
    trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.h
    trunk/reactos/drivers/wdm/audio/backpln/portcls/irpstream.c
    trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.c

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/…
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.h [iso-8859-1] Wed Mar 11
21:00:11 2009
@@ -216,6 +216,10 @@
 
     STDMETHOD_(BOOL, CancelBuffers)(THIS);
 
+    STDMETHOD_(VOID, UpdateFormat)(THIS_
+        IN PKSDATAFORMAT DataFormat);
+
+
 };
 
 

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/irpstream.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/…
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/irpstream.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/irpstream.c [iso-8859-1] Wed Mar 11
21:00:11 2009
@@ -13,9 +13,11 @@
 
 typedef struct _IRP_MAPPING_
 {
+    LIST_ENTRY Entry;
     KSSTREAM_HEADER *Header;
     PIRP Irp;
-    struct _IRP_MAPPING_ * Next;
+    KDPC Dpc;
+
 }IRP_MAPPING, *PIRP_MAPPING;
 
 typedef struct
@@ -30,15 +32,11 @@
     KSPIN_CONNECT *ConnectDetails;
     PKSDATAFORMAT_WAVEFORMATEX DataFormat;
 
-    KDPC Dpc;
     PIRP_MAPPING FirstMap;
     PIRP_MAPPING LastMap;
 
-    ULONG DpcActive;
-    PIRP_MAPPING FreeMapHead;
-    PIRP_MAPPING FreeMapTail;
-    ULONG FreeDataSize;
-    LONG FreeCount;
+    KSPIN_LOCK Lock;
+    LIST_ENTRY ListHead;
 
 }IIrpQueueImpl;
 
@@ -50,39 +48,22 @@
     IN PVOID  SystemArgument1,
     IN PVOID  SystemArgument2)
 {
-    PIRP_MAPPING CurMapping, NextMapping = NULL;
-    ULONG Count;
-    IIrpQueueImpl * This = (IIrpQueueImpl*)DeferredContext;
+    PIRP_MAPPING CurMapping;
 
     CurMapping = (PIRP_MAPPING)SystemArgument1;
     ASSERT(CurMapping);
 
-    Count = 0;
-    while(CurMapping)
-    {
-        NextMapping = CurMapping->Next;
-
-        This->FreeDataSize -= CurMapping->Header->DataUsed;
-
-        if (CurMapping->Irp)
-        {
-            CurMapping->Irp->IoStatus.Information =
CurMapping->Header->FrameExtent;
-            CurMapping->Irp->IoStatus.Status = STATUS_SUCCESS;
-            IoCompleteRequest(CurMapping->Irp, IO_SOUND_INCREMENT);
-        }
-
-        ExFreePool(CurMapping->Header->Data);
-        ExFreePool(CurMapping->Header);
-
-        ExFreePool(CurMapping);
-
-        CurMapping = NextMapping;
-        InterlockedDecrement(&This->FreeCount);
-
-        Count++;
-    }
-    This->DpcActive = FALSE;
-    DPRINT1("Freed %u Buffers / IRP Available Mappings %u\n", Count,
This->NumMappings);
+    if (CurMapping->Irp)
+    {
+        CurMapping->Irp->IoStatus.Information =
CurMapping->Header->FrameExtent;
+        CurMapping->Irp->IoStatus.Status = STATUS_SUCCESS;
+        IoCompleteRequest(CurMapping->Irp, IO_SOUND_INCREMENT);
+    }
+
+    ExFreePool(CurMapping->Header->Data);
+    ExFreePool(CurMapping->Header);
+
+    ExFreePool(CurMapping);
 }
 
 NTSTATUS
@@ -144,8 +125,8 @@
 
     This->ConnectDetails = ConnectDetails;
     This->DataFormat = (PKSDATAFORMAT_WAVEFORMATEX)DataFormat;
-
-    KeInitializeDpc(&This->Dpc, DpcRoutine, (PVOID)This);
+    InitializeListHead(&This->ListHead);
+    KeInitializeSpinLock(&This->Lock);
 
     return STATUS_SUCCESS;
 }
@@ -167,18 +148,17 @@
 
     Mapping->Header = (KSSTREAM_HEADER*)Buffer;
     Mapping->Irp = Irp;
-    Mapping->Next = NULL;
-
-    //DPRINT1("FirstMap %p LastMap %p NumMappings %u\n", This->FirstMap,
This->LastMap, This->NumMappings);
-
-    if (!This->FirstMap)
-        This->FirstMap = Mapping;
-    else
-        This->LastMap->Next = Mapping;
-
-    This->LastMap = Mapping;
-
-    InterlockedIncrement(&This->NumMappings);
+    KeInitializeDpc(&Mapping->Dpc, DpcRoutine, (PVOID)Mapping);
+
+    This->NumDataAvailable += Mapping->Header->DataUsed;
+
+    DPRINT1("IIrpQueue_fnAddMapping NumMappings %u SizeOfMapping %lu
NumDataAvailable %lu Irp %p\n", This->NumMappings,
Mapping->Header->DataUsed, This->NumDataAvailable, Irp);
+
+    /* FIXME use InterlockedCompareExchangePointer */
+    if (InterlockedCompareExchange((volatile long *)&This->FirstMap,
(LONG)Mapping, (LONG)0) != 0)
+        ExInterlockedInsertTailList(&This->ListHead, &Mapping->Entry,
&This->Lock);
+
+    (void)InterlockedIncrement((volatile long*)&This->NumMappings);
 
     if (Irp)
     {
@@ -187,9 +167,6 @@
         IoMarkIrpPending(Irp);
     }
 
-    This->NumDataAvailable += Mapping->Header->DataUsed;
-
-    DPRINT1("IIrpQueue_fnAddMapping NumMappings %u SizeOfMapping %lu
NumDataAvailable %lu Irp %p\n", This->NumMappings,
Mapping->Header->DataUsed, This->NumDataAvailable, Irp);
     return STATUS_SUCCESS;
 }
 
@@ -227,35 +204,16 @@
     if (This->FirstMap->Header->DataUsed <=This->CurrentOffset)
     {
         This->CurrentOffset = 0;
-        Mapping = This->FirstMap;
-        This->FirstMap = This->FirstMap->Next;
-
-        if (!This->FirstMap)
-            This->LastMap = NULL;
-
-        This->FreeCount++;
-
-        if (!This->FreeMapHead)
-           This->FreeMapHead = Mapping;
-        else
-           This->FreeMapTail->Next = Mapping;
-
-        This->FreeMapTail = Mapping;
-        Mapping->Next = NULL;
-        This->FreeDataSize += Mapping->Header->DataUsed;
+        Mapping = (PIRP_MAPPING)ExInterlockedRemoveHeadList(&This->ListHead,
&This->Lock);
+
         InterlockedDecrement(&This->NumMappings);
-        This->NumDataAvailable -= Mapping->Header->DataUsed;
-
-
-        if (This->FreeCount > 5 && This->DpcActive == FALSE)
-        {
-            Mapping = This->FreeMapHead;
-            This->FreeMapHead = NULL;
-            This->FreeMapTail = NULL;
-            This->DpcActive = TRUE;
-            KeInsertQueueDpc(&This->Dpc, (PVOID)Mapping, NULL);
-        }
-    }
+        This->NumDataAvailable -= This->FirstMap->Header->DataUsed;
+
+        KeInsertQueueDpc(&This->FirstMap->Dpc, (PVOID)This->FirstMap,
NULL);
+        (void)InterlockedExchangePointer((PVOID volatile*)&This->FirstMap,
(PVOID)Mapping);
+
+    }
+
 }
 
 ULONG
@@ -298,29 +256,20 @@
 IIrpQueue_fnCancelBuffers(
     IN IIrpQueue *iface)
 {
-    PIRP_MAPPING Mapping;
-    IIrpQueueImpl * This = (IIrpQueueImpl*)iface;
-
-    if (This->DpcActive)
-        return FALSE;
-
-    ASSERT(This->FirstMap == NULL);
-    ASSERT(This->LastMap == NULL);
-
-    if (This->FreeMapHead == NULL)
-    {
-        ASSERT(This->FreeMapTail == NULL);
-        This->FreeMapTail = NULL;
-        return TRUE;
-    }
-
-    Mapping = This->FreeMapHead;
-    This->FreeMapHead = NULL;
-    This->FreeMapTail = NULL;
-    This->DpcActive = TRUE;
-    KeInsertQueueDpc(&This->Dpc, (PVOID)Mapping, NULL);
-    return FALSE;
-}
+    return TRUE;
+}
+
+VOID
+NTAPI
+IIrpQueue_fnUpdateFormat(
+    IN IIrpQueue *iface,
+    PKSDATAFORMAT DataFormat)
+{
+    IIrpQueueImpl * This = (IIrpQueueImpl*)iface;
+    This->DataFormat = (PKSDATAFORMAT_WAVEFORMATEX)DataFormat;
+
+}
+
 
 static IIrpQueueVtbl vt_IIrpQueue =
 {
@@ -334,7 +283,8 @@
     IIrpQueue_fnNumMappings,
     IIrpQueue_fnMinMappings,
     IIrpQueue_fnMinimumDataAvailable,
-    IIrpQueue_fnCancelBuffers
+    IIrpQueue_fnCancelBuffers,
+    IIrpQueue_fnUpdateFormat
 };
 
 

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/…
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.c [iso-8859-1]
(original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.c [iso-8859-1] Wed Mar
11 21:00:11 2009
@@ -334,8 +334,9 @@
 
             if (Property->Flags & KSPROPERTY_TYPE_SET)
             {
+                Status = STATUS_UNSUCCESSFUL;
                 Irp->IoStatus.Information = 0;
-                Irp->IoStatus.Status = STATUS_UNSUCCESSFUL;
+
                 if (This->Stream)
                 {
                     Status = This->Stream->lpVtbl->SetState(This->Stream,
*State);
@@ -344,14 +345,11 @@
                     if (NT_SUCCESS(Status))
                     {
                         This->State = *State;
-                        Irp->IoStatus.Information = sizeof(KSSTATE);
-                        Irp->IoStatus.Status = Status;
-                        IoCompleteRequest(Irp, IO_NO_INCREMENT);
-                        return Status;
                     }
-                    Irp->IoStatus.Status = Status;
                 }
-                return Irp->IoStatus.Status;
+                Irp->IoStatus.Status = Status;
+                IoCompleteRequest(Irp, IO_NO_INCREMENT);
+                return Status;
             }
             else if (Property->Flags & KSPROPERTY_TYPE_GET)
             {
@@ -379,24 +377,29 @@
 
                 if (This->Stream)
                 {
-                   
while(!This->IrpQueue->lpVtbl->CancelBuffers(This->IrpQueue))
-                        KeStallExecutionProcessor(10);
-
-                    This->Stream->lpVtbl->SetState(This->Stream,
KSSTATE_STOP);
+                    Status = This->Stream->lpVtbl->SetState(This->Stream,
KSSTATE_STOP);
                     This->State = KSSTATE_STOP;
+                    DPRINT1("NewDataFormat: Channels %u Bits %u Samples %u\n",
((PKSDATAFORMAT_WAVEFORMATEX)DataFormat)->WaveFormatEx.nChannels,
+                                                                                
((PKSDATAFORMAT_WAVEFORMATEX)DataFormat)->WaveFormatEx.wBitsPerSample,
+                                                                                
((PKSDATAFORMAT_WAVEFORMATEX)DataFormat)->WaveFormatEx.nSamplesPerSec);
 
                     Status = This->Stream->lpVtbl->SetFormat(This->Stream,
NewDataFormat);
                     if (NT_SUCCESS(Status))
                     {
                         if (This->Format)
                             ExFreePoolWithTag(This->Format, TAG_PORTCLASS);
+
+                        This->IrpQueue->lpVtbl->UpdateFormat(This->IrpQueue,
(PKSDATAFORMAT)NewDataFormat);
                         This->Format = NewDataFormat;
                         Irp->IoStatus.Information = DataFormat->FormatSize;
                         Irp->IoStatus.Status = STATUS_SUCCESS;
                         IoCompleteRequest(Irp, IO_NO_INCREMENT);
                         return STATUS_SUCCESS;
                     }
+
                 }
+                DPRINT1("Failed to set format\n");
+DbgBreakPoint();
                 Irp->IoStatus.Information = 0;
                 Irp->IoStatus.Status = STATUS_UNSUCCESSFUL;
                 IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -658,37 +661,33 @@
 
     Packet = (PCONTEXT_WRITE)Buffer;
 
+
     if (This->IrpQueue->lpVtbl->MinimumDataAvailable(This->IrpQueue))
+    {
         Irp = Packet->Irp;
+        StatusBlock->Status = STATUS_PENDING;
+    }
     else
+    {
         Irp = NULL;
-
-    Status = This->IrpQueue->lpVtbl->AddMapping(This->IrpQueue, Buffer,
Length, Irp);
-
-    if (!NT_SUCCESS(Status))
-        return FALSE;
-
-    if (This->IrpQueue->lpVtbl->MinimumDataAvailable(This->IrpQueue) == TRUE
&& This->State != KSSTATE_RUN)
-    {
-        /* some should initiate a state request but didnt do it */
-        DPRINT1("Starting stream with %lu mappings\n",
This->IrpQueue->lpVtbl->NumMappings(This->IrpQueue));
-
-        This->Stream->lpVtbl->SetState(This->Stream, KSSTATE_RUN);
-        This->State = KSSTATE_RUN;
-    }
-
-    if (!Irp)
-    {
-        //DPRINT1("Completing Irp %p\n", Packet->Irp);
-
         Packet->Irp->IoStatus.Status = STATUS_SUCCESS;
         Packet->Irp->IoStatus.Information = Packet->Header.FrameExtent;
         IoCompleteRequest(Packet->Irp, IO_SOUND_INCREMENT);
         StatusBlock->Status = STATUS_SUCCESS;
     }
-    else
-    {
-        StatusBlock->Status = STATUS_PENDING;
+
+    Status = This->IrpQueue->lpVtbl->AddMapping(This->IrpQueue, Buffer,
Length, Irp);
+
+    if (!NT_SUCCESS(Status))
+        return FALSE;
+
+    if (This->IrpQueue->lpVtbl->MinimumDataAvailable(This->IrpQueue) == TRUE
&& This->State != KSSTATE_RUN)
+    {
+        /* some should initiate a state request but didnt do it */
+        DPRINT1("Starting stream with %lu mappings\n",
This->IrpQueue->lpVtbl->NumMappings(This->IrpQueue));
+
+        This->Stream->lpVtbl->SetState(This->Stream, KSSTATE_RUN);
+        This->State = KSSTATE_RUN;
     }
 
     return TRUE;