Fixed #22

Reworked and improved memory management on sending Output Reports on USB
nefarius · Mar 21, 2021 · 6814645 · 6814645
1 parent 0395cf4
commit 6814645
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 43 deletions.
diff --git a/sys/Ds3.c b/sys/Ds3.c
@@ -7,6 +7,7 @@
 // Default Output Report for LED & Rumble state changes (USB)
 // 
 const UCHAR G_Ds3UsbHidOutputReport[] = {
+	0x01, /* Report ID */
 	0x00, 0xFF, 0x00, 0xFF, 0x00,
 	0x00, 0x00, 0x00, 0x00, 0x00,
 	0xFF, 0x27, 0x10, 0x00, 0x32,
@@ -301,11 +302,17 @@ VOID DS3_GET_UNIFIED_OUTPUT_REPORT_BUFFER(
 	{
 	case DsDeviceConnectionTypeUsb:
 
-		*Buffer = (PUCHAR)WdfMemoryGetBuffer(
+		//
+		// Skip Report ID
+		// 
+
+		*Buffer = &((PUCHAR)WdfMemoryGetBuffer(
 			Context->Connection.Usb.OutputReportMemory,
 			BufferLength
-		);
+		))[1];
 
+		*BufferLength -= 1;
+
 		break;
 
 	case DsDeviceConnectionTypeBth:

diff --git a/sys/Ds3.h b/sys/Ds3.h
@@ -5,7 +5,7 @@
 
 extern const UCHAR G_Ds3UsbHidOutputReport[];
 
-#define DS3_USB_HID_OUTPUT_REPORT_SIZE		0x30
+#define DS3_USB_HID_OUTPUT_REPORT_SIZE		0x31
 
 extern const UCHAR G_Ds3BthHidOutputReport[];
 
@@ -17,13 +17,13 @@ extern const UCHAR G_Ds3BthHidOutputReport[];
 #define DS3_LED_4       0x10
 #define DS3_LED_OFF     0x20
 
-#define DS3_USB_SET_LED(_buf_, _led_)   ((_buf_)[9] = (_led_))
-#define DS3_USB_GET_LED(_buf_)          ((_buf_)[9])
+#define DS3_USB_SET_LED(_buf_, _led_)   ((_buf_)[10] = (_led_))
+#define DS3_USB_GET_LED(_buf_)          ((_buf_)[10])
 
-#define DS3_USB_SET_SMALL_RUMBLE_DURATION(_buf_, _dur_)  ((_buf_)[1] = (_dur_))
-#define DS3_USB_SET_LARGE_RUMBLE_DURATION(_buf_, _dur_)  ((_buf_)[3] = (_dur_))
-#define DS3_USB_SET_SMALL_RUMBLE_STRENGTH(_buf_, _str_)  ((_buf_)[2] = (_str_) > 0 ? 0x01 : 0x00)
-#define DS3_USB_SET_LARGE_RUMBLE_STRENGTH(_buf_, _str_)  ((_buf_)[4] = (_str_))
+#define DS3_USB_SET_SMALL_RUMBLE_DURATION(_buf_, _dur_)  ((_buf_)[2] = (_dur_))
+#define DS3_USB_SET_LARGE_RUMBLE_DURATION(_buf_, _dur_)  ((_buf_)[4] = (_dur_))
+#define DS3_USB_SET_SMALL_RUMBLE_STRENGTH(_buf_, _str_)  ((_buf_)[3] = (_str_) > 0 ? 0x01 : 0x00)
+#define DS3_USB_SET_LARGE_RUMBLE_STRENGTH(_buf_, _str_)  ((_buf_)[5] = (_str_))
 
 #define DS3_BTH_SET_LED(_buf_, _led_)   ((_buf_)[11] = (_led_))
 #define DS3_BTH_GET_LED(_buf_)          ((_buf_)[11])

diff --git a/sys/DsHidMiniDrv.c b/sys/DsHidMiniDrv.c
@@ -1731,8 +1731,6 @@ DMF_OutputReportScheduledTaskCallback(
 	size_t bufferSize = 0;
 	LARGE_INTEGER freq, * t1, t2;
 	LONGLONG ms;
-
-	PUCHAR interruptBuffer = NULL;
 
 	FuncEntry(TRACE_DSHIDMINIDRV);
 
@@ -1770,23 +1768,8 @@ DMF_OutputReportScheduledTaskCallback(
 			break;
 		}
 
-		//
-		// TODO: alloc only once in context to save overhead
-		// 
-		interruptBuffer = (PUCHAR)malloc(0x31);
-
-		if (interruptBuffer == NULL)
-			break;
-
-		interruptBuffer[0] = 0x01; // Report ID
-
-		RtlCopyMemory(&interruptBuffer[1], buffer, bufferSize);
-
-		status = USB_WriteInterruptPipeAsync(
-			WdfUsbTargetDeviceGetIoTarget(pDevCtx->Connection.Usb.UsbDevice),
-			pDevCtx->Connection.Usb.InterruptOutPipe,
-			interruptBuffer,
-			0x31 // TODO: introduce const
+		status = USB_WriteInterruptOutSync(
+			pDevCtx
 		);
 
 		if (NT_SUCCESS(status))
@@ -1798,8 +1781,6 @@ DMF_OutputReportScheduledTaskCallback(
 			);
 		}
 
-		free(interruptBuffer);
-
 		break;
 
 	case DsDeviceConnectionTypeBth:

diff --git a/sys/DsUsb.c b/sys/DsUsb.c
@@ -210,6 +210,39 @@ USB_WriteInterruptPipeAsync(
 	return status;
 }
 
+//
+// Send the Output Report buffer content to the Interrupt OUT endpoint and wait for completion
+// 
+NTSTATUS
+USB_WriteInterruptOutSync(
+	_In_ PDEVICE_CONTEXT Context
+)
+{
+	WDF_MEMORY_DESCRIPTOR  writeBufDesc;
+	ULONG bytesWritten;
+	NTSTATUS status;
+
+	FuncEntry(TRACE_DSUSB);
+
+	WDF_MEMORY_DESCRIPTOR_INIT_HANDLE(
+		&writeBufDesc,
+		Context->Connection.Usb.OutputReportMemory,
+		NULL
+	);
+
+	status = WdfUsbTargetPipeWriteSynchronously(
+		Context->Connection.Usb.InterruptOutPipe,
+		NULL,
+		NULL,
+		&writeBufDesc,
+		&bytesWritten
+	);
+
+	FuncExit(TRACE_DSUSB, "status=%!STATUS!", status);
+
+	return status;
+}
+
 //
 // Reader failed for some reason
 // 

diff --git a/sys/DsUsb.h b/sys/DsUsb.h
@@ -29,4 +29,9 @@ NTSTATUS USB_WriteInterruptPipeAsync(
     _In_ PVOID Buffer,
     _In_ size_t BufferLength);
 
+NTSTATUS
+USB_WriteInterruptOutSync(
+    _In_ PDEVICE_CONTEXT Context
+);
+
 EVT_WDF_REQUEST_COMPLETION_ROUTINE EvtUsbRequestCompletionRoutine;
diff --git a/sys/Power.c b/sys/Power.c
@@ -638,12 +638,16 @@ DsHidMini_EvtDevicePrepareHardware(
 				Ds3FeatureHostAddress,
 				0,
 				controlTransferBuffer,
-				CONTROL_TRANSFER_BUFFER_LENGTH);
+				CONTROL_TRANSFER_BUFFER_LENGTH
+			);
 
 			if (!NT_SUCCESS(status))
 			{
-				TraceError( TRACE_POWER,
-					"Requesting host address failed with %!STATUS!", status);
+				TraceError(
+					TRACE_POWER,
+					"Requesting host address failed with %!STATUS!", 
+					status
+				);
 				return status;
 			}
 
@@ -655,15 +659,12 @@ DsHidMini_EvtDevicePrepareHardware(
 			//
 			// Send initial output report
 			// 
-			status = USB_SendControlRequest(
-				pDevCtx,
-				BmRequestHostToDevice,
-				BmRequestClass,
-				SetReport,
-				USB_SETUP_VALUE(HidReportRequestTypeOutput, HidReportRequestIdOne),
-				0,
+			(void)USB_WriteInterruptPipeAsync(
+				WdfUsbTargetDeviceGetIoTarget(pDevCtx->Connection.Usb.UsbDevice),
+				pDevCtx->Connection.Usb.InterruptOutPipe,
 				(PVOID)G_Ds3UsbHidOutputReport,
-				DS3_USB_HID_OUTPUT_REPORT_SIZE);
+				DS3_USB_HID_OUTPUT_REPORT_SIZE
+			);
 
 			//
 			// Re-create if exists
@@ -688,8 +689,11 @@ DsHidMini_EvtDevicePrepareHardware(
 			);
 			if (!NT_SUCCESS(status))
 			{
-				TraceError(TRACE_POWER,
-					"WdfMemoryCreate failed with %!STATUS!", status);
+				TraceError(
+					TRACE_POWER,
+					"WdfMemoryCreate failed with %!STATUS!", 
+					status
+				);
 				return status;
 			}