[ros-diffs] [reactos] 01/01: [XBOXVMP] Fix broken pixels and general refactoring (#1896)

[Stanislav Motylkov]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b3301df570948549b9c4c…

commit b3301df570948549b9c4cca2c969384c59a69209
Author:     Stanislav Motylkov <x86corez(a)gmail.com>
AuthorDate: Tue Oct 8 18:08:44 2019 +0300
Commit:     Hermès BÉLUSCA - MAÏTO <hermes.belusca-maito(a)reactos.org>
CommitDate: Tue Oct 8 17:08:44 2019 +0200

    [XBOXVMP] Fix broken pixels and general refactoring (#1896)
    
    - Remove old hacky code based on MmHighestPhysicalPage.
    - Split I2C SMBus code into a separate source file.
    
    CORE-16216 CORE-16357
---
 win32ss/drivers/miniport/xboxvmp/CMakeLists.txt |  10 +-
 win32ss/drivers/miniport/xboxvmp/xboxi2c.c      | 123 +++++++++++++
 win32ss/drivers/miniport/xboxvmp/xboxvmp.c      | 233 +++++++-----------------
 win32ss/drivers/miniport/xboxvmp/xboxvmp.h      |  36 +---
 4 files changed, 203 insertions(+), 199 deletions(-)

diff --git a/win32ss/drivers/miniport/xboxvmp/CMakeLists.txt
b/win32ss/drivers/miniport/xboxvmp/CMakeLists.txt
index d5f9f1332af..59941c74349 100644
--- a/win32ss/drivers/miniport/xboxvmp/CMakeLists.txt
+++ b/win32ss/drivers/miniport/xboxvmp/CMakeLists.txt
@@ -1,5 +1,13 @@
 
-add_library(xboxvmp MODULE xboxvmp.c xboxvmp.rc)
+list(APPEND SOURCE
+    xboxi2c.c
+    xboxvmp.c
+    xboxvmp.h)
+
+add_library(xboxvmp MODULE
+    ${SOURCE}
+    xboxvmp.rc)
+
 set_module_type(xboxvmp kernelmodedriver)
 add_importlibs(xboxvmp ntoskrnl videoprt)
 add_cd_file(TARGET xboxvmp DESTINATION reactos/system32/drivers FOR all)
diff --git a/win32ss/drivers/miniport/xboxvmp/xboxi2c.c
b/win32ss/drivers/miniport/xboxvmp/xboxi2c.c
new file mode 100644
index 00000000000..fc283af7e98
--- /dev/null
+++ b/win32ss/drivers/miniport/xboxvmp/xboxi2c.c
@@ -0,0 +1,123 @@
+/*
+ * PROJECT:     ReactOS Xbox miniport video driver
+ * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
+ * PURPOSE:     I2C SMBus routines
+ * COPYRIGHT:   Copyright 2004 Gé van Geldorp
+ *              Copyright 2004 Filip Navara
+ *              Copyright 2019 Stanislav Motylkov (x86corez(a)gmail.com)
+ */
+
+/* INCLUDES *******************************************************************/
+
+#include "xboxvmp.h"
+
+#include <debug.h>
+#include <dpfilter.h>
+
+/* PUBLIC AND PRIVATE FUNCTIONS ***********************************************/
+
+static
+BOOLEAN
+ReadfromSMBus(
+    UCHAR Address,
+    UCHAR bRegister,
+    UCHAR Size,
+    ULONG *Data_to_smbus)
+{
+    int nRetriesToLive = 50;
+
+    while ((VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE + 0)) & 0x0800) != 0)
+    {
+        ; /* Franz's spin while bus busy with any master traffic */
+    }
+
+    while (nRetriesToLive-- != 0)
+    {
+        UCHAR b;
+        int temp;
+
+        VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 4), (Address << 1) | 1);
+        VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 8), bRegister);
+
+        temp = VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE + 0));
+        VideoPortWritePortUshort((PUSHORT) (I2C_IO_BASE + 0), temp); /* clear down all
preexisting errors */
+
+        switch (Size)
+        {
+            case 4:
+            {
+                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0d); /* DWORD modus
? */
+                break;
+            }
+
+            case 2:
+            {
+                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0b); /* WORD modus
*/
+                break;
+            }
+
+            default:
+            {
+                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0a); /* BYTE */
+            }
+        }
+
+        b = 0;
+
+        while ((b & 0x36) == 0)
+        {
+            b = VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 0));
+        }
+
+        if ((b & 0x24) != 0)
+        {
+            ERR_(IHVVIDEO, "I2CTransmitByteGetReturn error %x\n", b);
+        }
+
+        if ((b & 0x10) == 0)
+        {
+            ERR_(IHVVIDEO, "I2CTransmitByteGetReturn no complete, retry\n");
+        }
+        else
+        {
+            switch (Size)
+            {
+                case 4:
+                {
+                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 6));
+                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
+                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
+                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
+                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
+                    break;
+                }
+
+                case 2:
+                {
+                    *Data_to_smbus = VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE +
6));
+                    break;
+                }
+
+                default:
+                {
+                    *Data_to_smbus = VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 6));
+                }
+            }
+
+            return TRUE;
+        }
+    }
+
+    return FALSE;
+}
+
+BOOLEAN
+I2CTransmitByteGetReturn(
+    UCHAR bPicAddressI2cFormat,
+    UCHAR bDataToWrite,
+    ULONG *Return)
+{
+    return ReadfromSMBus(bPicAddressI2cFormat, bDataToWrite, 1, Return);
+}
+
+/* EOF */
diff --git a/win32ss/drivers/miniport/xboxvmp/xboxvmp.c
b/win32ss/drivers/miniport/xboxvmp/xboxvmp.c
index ee6c7fd2e25..5d6fc275dac 100644
--- a/win32ss/drivers/miniport/xboxvmp/xboxvmp.c
+++ b/win32ss/drivers/miniport/xboxvmp/xboxvmp.c
@@ -389,28 +389,22 @@ XboxVmpMapVideoMemory(
 {
     PHYSICAL_ADDRESS FrameBuffer;
     ULONG inIoSpace = VIDEO_MEMORY_SPACE_MEMORY;
-    SYSTEM_BASIC_INFORMATION BasicInfo;
-    ULONG Length;
 
-    /* FIXME: this should probably be done differently, without native API */
     StatusBlock->Information = sizeof(VIDEO_MEMORY_INFORMATION);
 
-    FrameBuffer.u.HighPart = 0;
-    if (ZwQuerySystemInformation(SystemBasicInformation,
-                                 (PVOID)&BasicInfo,
-                                 sizeof(SYSTEM_BASIC_INFORMATION),
-                                 &Length) == NO_ERROR)
-    {
-        FrameBuffer.u.LowPart = BasicInfo.HighestPhysicalPageNumber * PAGE_SIZE;
-    }
-    else
+    /* Reuse framebuffer that was set up by firmware */
+    FrameBuffer.QuadPart = *((PULONG)((ULONG_PTR)DeviceExtension->VirtControlStart +
NV2A_CONTROL_FRAMEBUFFER_ADDRESS_OFFSET));
+    if (FrameBuffer.QuadPart != 0x3C00000 && FrameBuffer.QuadPart != 0x7C00000)
     {
-        ERR_(IHVVIDEO, "ZwQueryBasicInformation failed, assuming 64MB total
memory\n");
-        FrameBuffer.u.LowPart = 60 * 1024 * 1024;
+        /* Check framebuffer address (high 4 MB of either 64 or 128 MB RAM) */
+        WARN_(IHVVIDEO, "Non-standard framebuffer address 0x%p\n",
FrameBuffer.QuadPart);
     }
+    /* Verify that framebuffer address is page-aligned */
+    ASSERT(FrameBuffer.QuadPart % PAGE_SIZE == 0);
 
     FrameBuffer.QuadPart += DeviceExtension->PhysFrameBufferStart.QuadPart;
     MapInformation->VideoRamBase = RequestedAddress->RequestedVirtualAddress;
+    /* FIXME: obtain fb size from firmware somehow (Cromwell reserves high 4 MB of RAM)
*/
     MapInformation->VideoRamLength = 4 * 1024 * 1024;
 
     VideoPortMapMemory(
@@ -424,7 +418,7 @@ XboxVmpMapVideoMemory(
     MapInformation->FrameBufferLength = MapInformation->VideoRamLength;
 
     /* Tell the nVidia controller about the framebuffer */
-    *((PULONG)((char *)DeviceExtension->VirtControlStart +
NV2A_CONTROL_FRAMEBUFFER_ADDRESS_OFFSET)) = FrameBuffer.u.LowPart;
+    *((PULONG)((ULONG_PTR)DeviceExtension->VirtControlStart +
NV2A_CONTROL_FRAMEBUFFER_ADDRESS_OFFSET)) = FrameBuffer.u.LowPart;
 
     INFO_(IHVVIDEO, "Mapped 0x%x bytes of phys mem at 0x%lx to virt addr
0x%p\n",
         MapInformation->VideoRamLength, FrameBuffer.u.LowPart,
MapInformation->VideoRamBase);
@@ -475,112 +469,6 @@ XboxVmpQueryNumAvailModes(
     return TRUE;
 }
 
-static
-BOOLEAN
-ReadfromSMBus(
-    UCHAR Address,
-    UCHAR bRegister,
-    UCHAR Size,
-    ULONG *Data_to_smbus)
-{
-    int nRetriesToLive = 50;
-
-    while ((VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE + 0)) & 0x0800) != 0)
-    {
-        ; /* Franz's spin while bus busy with any master traffic */
-    }
-
-    while (nRetriesToLive-- != 0)
-    {
-        UCHAR b;
-        int temp;
-
-        VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 4), (Address << 1) | 1);
-        VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 8), bRegister);
-
-        temp = VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE + 0));
-        VideoPortWritePortUshort((PUSHORT) (I2C_IO_BASE + 0), temp); /* clear down all
preexisting errors */
-
-        switch (Size)
-        {
-            case 4:
-            {
-                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0d); /* DWORD modus
? */
-                break;
-            }
-
-            case 2:
-            {
-                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0b); /* WORD modus
*/
-                break;
-            }
-
-            default:
-            {
-                VideoPortWritePortUchar((PUCHAR) (I2C_IO_BASE + 2), 0x0a); /* BYTE */
-            }
-        }
-
-        b = 0;
-
-        while ((b & 0x36) == 0)
-        {
-            b = VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 0));
-        }
-
-        if ((b & 0x24) != 0)
-        {
-            ERR_(IHVVIDEO, "I2CTransmitByteGetReturn error %x\n", b);
-        }
-
-        if ((b & 0x10) == 0)
-        {
-            ERR_(IHVVIDEO, "I2CTransmitByteGetReturn no complete, retry\n");
-        }
-        else
-        {
-            switch (Size)
-            {
-                case 4:
-                {
-                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 6));
-                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
-                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
-                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
-                    VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 9));
-                    break;
-                }
-
-                case 2:
-                {
-                    *Data_to_smbus = VideoPortReadPortUshort((PUSHORT) (I2C_IO_BASE +
6));
-                    break;
-                }
-
-                default:
-                {
-                    *Data_to_smbus = VideoPortReadPortUchar((PUCHAR) (I2C_IO_BASE + 6));
-                }
-            }
-
-            return TRUE;
-        }
-    }
-
-    return FALSE;
-}
-
-
-static
-BOOLEAN
-I2CTransmitByteGetReturn(
-    UCHAR bPicAddressI2cFormat,
-    UCHAR bDataToWrite,
-    ULONG *Return)
-{
-    return ReadfromSMBus(bPicAddressI2cFormat, bDataToWrite, 1, Return);
-}
-
 /*
  * XboxVmpQueryAvailModes
  *
@@ -610,57 +498,58 @@ XboxVmpQueryCurrentMode(
     PVIDEO_MODE_INFORMATION VideoMode,
     PSTATUS_BLOCK StatusBlock)
 {
-	ULONG AvMode = 0;
-
-	VideoMode->Length = sizeof(VIDEO_MODE_INFORMATION);
-	VideoMode->ModeIndex = 0;
-
-	if (I2CTransmitByteGetReturn(0x10, 0x04, &AvMode))
-	{
-		if (AvMode == 1) /* HDTV */
-		{
-			VideoMode->VisScreenWidth = 720;
-		}
-		else
-		{
-			/* FIXME Other possible values of AvMode:
-			 * 0 - AV_SCART_RGB
-			 * 2 - AV_VGA_SOG
-			 * 4 - AV_SVIDEO
-			 * 6 - AV_COMPOSITE
-			 * 7 - AV_VGA
-			 * other AV_COMPOSITE
-			 */
-			VideoMode->VisScreenWidth = 640;
-		}
-	}
-	else
-	{
-		VideoMode->VisScreenWidth = 640;
-	}
-
-	VideoMode->VisScreenHeight = 480;
-	VideoMode->ScreenStride = VideoMode->VisScreenWidth * 4;
-	VideoMode->NumberOfPlanes = 1;
-	VideoMode->BitsPerPlane = 32;
-	VideoMode->Frequency = 1;
-	VideoMode->XMillimeter = 0; /* FIXME */
-	VideoMode->YMillimeter = 0; /* FIXME */
-	VideoMode->NumberRedBits = 8;
-	VideoMode->NumberGreenBits = 8;
-	VideoMode->NumberBlueBits = 8;
-	VideoMode->RedMask = 0xFF0000;
-	VideoMode->GreenMask = 0x00FF00;
-	VideoMode->BlueMask = 0x0000FF;
-	VideoMode->VideoMemoryBitmapWidth = VideoMode->VisScreenWidth;
-	VideoMode->VideoMemoryBitmapHeight = VideoMode->VisScreenHeight;
-	VideoMode->AttributeFlags = VIDEO_MODE_GRAPHICS | VIDEO_MODE_COLOR |
-		VIDEO_MODE_NO_OFF_SCREEN;
-	VideoMode->DriverSpecificAttributeFlags = 0;
-
-	StatusBlock->Information = sizeof(VIDEO_MODE_INFORMATION);
-
-	return TRUE;
+    ULONG AvMode = 0;
+
+    VideoMode->Length = sizeof(VIDEO_MODE_INFORMATION);
+    VideoMode->ModeIndex = 0;
+
+    /* FIXME: don't use SMBus, obtain current video resolution directly from NV2A */
+    if (I2CTransmitByteGetReturn(0x10, 0x04, &AvMode))
+    {
+        if (AvMode == 1) /* HDTV */
+        {
+            VideoMode->VisScreenWidth = 720;
+        }
+        else
+        {
+            /* FIXME Other possible values of AvMode:
+             * 0 - AV_SCART_RGB
+             * 2 - AV_VGA_SOG
+             * 4 - AV_SVIDEO
+             * 6 - AV_COMPOSITE
+             * 7 - AV_VGA
+             * other AV_COMPOSITE
+             */
+            VideoMode->VisScreenWidth = 640;
+        }
+    }
+    else
+    {
+        VideoMode->VisScreenWidth = 640;
+    }
+
+    VideoMode->VisScreenHeight = 480;
+    VideoMode->ScreenStride = VideoMode->VisScreenWidth * 4;
+    VideoMode->NumberOfPlanes = 1;
+    VideoMode->BitsPerPlane = 32;
+    VideoMode->Frequency = 1;
+    VideoMode->XMillimeter = 0; /* FIXME */
+    VideoMode->YMillimeter = 0; /* FIXME */
+    VideoMode->NumberRedBits = 8;
+    VideoMode->NumberGreenBits = 8;
+    VideoMode->NumberBlueBits = 8;
+    VideoMode->RedMask = 0xFF0000;
+    VideoMode->GreenMask = 0x00FF00;
+    VideoMode->BlueMask = 0x0000FF;
+    VideoMode->VideoMemoryBitmapWidth = VideoMode->VisScreenWidth;
+    VideoMode->VideoMemoryBitmapHeight = VideoMode->VisScreenHeight;
+    VideoMode->AttributeFlags = VIDEO_MODE_GRAPHICS | VIDEO_MODE_COLOR |
+        VIDEO_MODE_NO_OFF_SCREEN;
+    VideoMode->DriverSpecificAttributeFlags = 0;
+
+    StatusBlock->Information = sizeof(VIDEO_MODE_INFORMATION);
+
+    return TRUE;
 }
 
 /* EOF */
diff --git a/win32ss/drivers/miniport/xboxvmp/xboxvmp.h
b/win32ss/drivers/miniport/xboxvmp/xboxvmp.h
index f7014cc1166..5742cef3e29 100644
--- a/win32ss/drivers/miniport/xboxvmp/xboxvmp.h
+++ b/win32ss/drivers/miniport/xboxvmp/xboxvmp.h
@@ -11,6 +11,10 @@
 
 /* INCLUDES *******************************************************************/
 
+/*
+ * FIXME: specify headers properly in the triangle brackets and rearrange them
+ * in a way so it would be simpler to add NDK and other headers for debugging.
+ */
 #include "ntdef.h"
 #define PAGE_SIZE 4096
 #include "dderror.h"
@@ -18,35 +22,15 @@
 #include "miniport.h"
 #include "video.h"
 
-/* FIXME: NDK not compatible with miniport drivers */
-#define SystemBasicInformation 0
-typedef struct _SYSTEM_BASIC_INFORMATION
-{
-    ULONG Reserved;
-    ULONG TimerResolution;
-    ULONG PageSize;
-    ULONG NumberOfPhysicalPages;
-    ULONG LowestPhysicalPageNumber;
-    ULONG HighestPhysicalPageNumber;
-    ULONG AllocationGranularity;
-    ULONG MinimumUserModeAddress;
-    ULONG MaximumUserModeAddress;
-    KAFFINITY ActiveProcessorsAffinityMask;
-    CCHAR NumberOfProcessors;
-} SYSTEM_BASIC_INFORMATION, *PSYSTEM_BASIC_INFORMATION;
-
-LONG
-__stdcall
-ZwQuerySystemInformation(
-    IN ULONG SystemInformationClass,
-    OUT PVOID SystemInformation,
-    IN ULONG Length,
-    OUT PULONG ResultLength
-);
-
 #define I2C_IO_BASE 0xC000
 #define NV2A_CONTROL_FRAMEBUFFER_ADDRESS_OFFSET 0x600800
 
+BOOLEAN
+I2CTransmitByteGetReturn(
+    UCHAR bPicAddressI2cFormat,
+    UCHAR bDataToWrite,
+    ULONG *Return);
+
 typedef struct
 {
     PHYSICAL_ADDRESS PhysControlStart;