From: Phil Dennis-Jordan <phil@philjordan.eu>

This adds a header file defining symbolic constants for the VMWare SVGA2
virtual display device in preparation for supporting it in QemuVideoDXE.

It is an extract of the file lib/vmware/svga_reg.h from commit
329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga
repository at git://git.code.sf.net/p/vmware-svga/git (See also
http://vmware-svga.sourceforge.net/ )

Only the bare essentials necessary for initialisation, modesetting and
framebuffer access have been kept from the original file.

The original file was released by VMWare under the MIT license, this
has been retained.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---

Notes:
    v2:
    - New, custom header file instead of importing VMWare's verbatim. [Laszlo]

 OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h | 102 ++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
new file mode 100644
index 000000000000..9db553155957
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
@@ -0,0 +1,102 @@
+/** @file
+
+  Macro and enum definitions of a subset of port numbers, register identifiers
+  and values required for driving the VMWare SVGA2 virtual display adapter,
+  also implemented by Qemu.
+
+  This file's contents was extracted from file lib/vmware/svga_reg.h in commit
+  329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
+  git://git.code.sf.net/p/vmware-svga/git
+
+
+  Copyright 1998-2009 VMware, Inc.  All rights reserved.
+  Portions Copyright 2017 Phil Dennis-Jordan <phil@philjordan.eu>
+
+  Permission is hereby granted, free of charge, to any person
+  obtaining a copy of this software and associated documentation
+  files (the "Software"), to deal in the Software without
+  restriction, including without limitation the rights to use, copy,
+  modify, merge, publish, distribute, sublicense, and/or sell copies
+  of the Software, and to permit persons to whom the Software is
+  furnished to do so, subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be
+  included in all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+  SOFTWARE.
+
+**/
+
+#ifndef _VMWARE_SVGA2_H_
+#define _VMWARE_SVGA2_H_
+
+//
+// IDs for recognising the device
+//
+#define PCI_VENDOR_ID_VMWARE            0x15AD
+#define PCI_DEVICE_ID_VMWARE_SVGA2      0x0405
+
+//
+// I/O port BAR offsets for register selection and read/write.
+//
+// The register index is written to the 32-bit index port, followed by a 32-bit
+// read or write on the value port to read or set that register's contents.
+//
+#define SVGA_INDEX_PORT         0x0
+#define SVGA_VALUE_PORT         0x1
+
+//
+// Some of the device's register indices for basic framebuffer functionality.
+//
+enum {
+  SVGA_REG_ID = 0,
+  SVGA_REG_ENABLE = 1,
+  SVGA_REG_WIDTH = 2,
+  SVGA_REG_HEIGHT = 3,
+  SVGA_REG_MAX_WIDTH = 4,
+  SVGA_REG_MAX_HEIGHT = 5,
+
+  SVGA_REG_BITS_PER_PIXEL = 7,
+
+  SVGA_REG_RED_MASK = 9,
+  SVGA_REG_GREEN_MASK = 10,
+  SVGA_REG_BLUE_MASK = 11,
+  SVGA_REG_BYTES_PER_LINE = 12,
+
+  SVGA_REG_FB_OFFSET = 14,
+
+  SVGA_REG_FB_SIZE = 16,
+  SVGA_REG_CAPABILITIES = 17,
+
+  SVGA_REG_HOST_BITS_PER_PIXEL = 28,
+};
+
+//
+// Values used with SVGA_REG_ID for sanity-checking the device and getting
+// its version.
+//
+#define SVGA_MAGIC         0x900000UL
+#define SVGA_MAKE_ID(ver)  (SVGA_MAGIC << 8 | (ver))
+
+#define SVGA_VERSION_2     2
+#define SVGA_ID_2          SVGA_MAKE_ID(SVGA_VERSION_2)
+
+#define SVGA_VERSION_1     1
+#define SVGA_ID_1          SVGA_MAKE_ID(SVGA_VERSION_1)
+
+#define SVGA_VERSION_0     0
+#define SVGA_ID_0          SVGA_MAKE_ID(SVGA_VERSION_0)
+
+//
+// One of the capability bits advertised by SVGA_REG_CAPABILITIES.
+//
+#define SVGA_CAP_8BIT_EMULATION     0x00000100
+
+#endif
-- 
2.3.2 (Apple Git-55)

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 2017-04-02 15:44:55, Phil Dennis-Jordan wrote:
> From: Phil Dennis-Jordan <phil@philjordan.eu>
> 
> This adds a header file defining symbolic constants for the VMWare SVGA2
> virtual display device in preparation for supporting it in QemuVideoDXE.
> 
> It is an extract of the file lib/vmware/svga_reg.h from commit
> 329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga
> repository at git://git.code.sf.net/p/vmware-svga/git (See also
> http://vmware-svga.sourceforge.net/ )
> 
> Only the bare essentials necessary for initialisation, modesetting and
> framebuffer access have been kept from the original file.
> 
> The original file was released by VMWare under the MIT license, this
> has been retained.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
> 
> Notes:
>     v2:
>     - New, custom header file instead of importing VMWare's verbatim. [Laszlo]
> 
>  OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h | 102 ++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h

I think EDK II's file naming convention would prefer VmwareSvga2.h, or
possibly VmWareSvga2.h. (But, the former looks better to me.)

Since it covers svga and svga2, should we just drop the '2' from the
filename?

> new file mode 100644
> index 000000000000..9db553155957
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
> @@ -0,0 +1,102 @@
> +/** @file
> +
> +  Macro and enum definitions of a subset of port numbers, register identifiers
> +  and values required for driving the VMWare SVGA2 virtual display adapter,
> +  also implemented by Qemu.
> +
> +  This file's contents was extracted from file lib/vmware/svga_reg.h in commit
> +  329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
> +  git://git.code.sf.net/p/vmware-svga/git
> +
> +
> +  Copyright 1998-2009 VMware, Inc.  All rights reserved.
> +  Portions Copyright 2017 Phil Dennis-Jordan <phil@philjordan.eu>
> +
> +  Permission is hereby granted, free of charge, to any person
> +  obtaining a copy of this software and associated documentation
> +  files (the "Software"), to deal in the Software without
> +  restriction, including without limitation the rights to use, copy,
> +  modify, merge, publish, distribute, sublicense, and/or sell copies
> +  of the Software, and to permit persons to whom the Software is
> +  furnished to do so, subject to the following conditions:
> +
> +  The above copyright notice and this permission notice shall be
> +  included in all copies or substantial portions of the Software.
> +
> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> +  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> +  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> +  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +  SOFTWARE.
> +
> +**/
> +
> +#ifndef _VMWARE_SVGA2_H_
> +#define _VMWARE_SVGA2_H_
> +
> +//
> +// IDs for recognising the device
> +//
> +#define PCI_VENDOR_ID_VMWARE            0x15AD
> +#define PCI_DEVICE_ID_VMWARE_SVGA2      0x0405
> +
> +//
> +// I/O port BAR offsets for register selection and read/write.
> +//
> +// The register index is written to the 32-bit index port, followed by a 32-bit
> +// read or write on the value port to read or set that register's contents.
> +//
> +#define SVGA_INDEX_PORT         0x0
> +#define SVGA_VALUE_PORT         0x1

Thanks for taking Laszlo's advice about pulling out the minimal
definitions and EDK II-ifying it. (I think he may be right about the
Xen contribution.)

One request I have is, how about prefixing all the SVGA items as
VMWARE_SVGA? SVGA seems to generic of a term here.

Thanks,

-Jordan

> +
> +//
> +// Some of the device's register indices for basic framebuffer functionality.
> +//
> +enum {
> +  SVGA_REG_ID = 0,
> +  SVGA_REG_ENABLE = 1,
> +  SVGA_REG_WIDTH = 2,
> +  SVGA_REG_HEIGHT = 3,
> +  SVGA_REG_MAX_WIDTH = 4,
> +  SVGA_REG_MAX_HEIGHT = 5,
> +
> +  SVGA_REG_BITS_PER_PIXEL = 7,
> +
> +  SVGA_REG_RED_MASK = 9,
> +  SVGA_REG_GREEN_MASK = 10,
> +  SVGA_REG_BLUE_MASK = 11,
> +  SVGA_REG_BYTES_PER_LINE = 12,
> +
> +  SVGA_REG_FB_OFFSET = 14,
> +
> +  SVGA_REG_FB_SIZE = 16,
> +  SVGA_REG_CAPABILITIES = 17,
> +
> +  SVGA_REG_HOST_BITS_PER_PIXEL = 28,
> +};
> +
> +//
> +// Values used with SVGA_REG_ID for sanity-checking the device and getting
> +// its version.
> +//
> +#define SVGA_MAGIC         0x900000UL
> +#define SVGA_MAKE_ID(ver)  (SVGA_MAGIC << 8 | (ver))
> +
> +#define SVGA_VERSION_2     2
> +#define SVGA_ID_2          SVGA_MAKE_ID(SVGA_VERSION_2)
> +
> +#define SVGA_VERSION_1     1
> +#define SVGA_ID_1          SVGA_MAKE_ID(SVGA_VERSION_1)
> +
> +#define SVGA_VERSION_0     0
> +#define SVGA_ID_0          SVGA_MAKE_ID(SVGA_VERSION_0)
> +
> +//
> +// One of the capability bits advertised by SVGA_REG_CAPABILITIES.
> +//
> +#define SVGA_CAP_8BIT_EMULATION     0x00000100
> +
> +#endif
> -- 
> 2.3.2 (Apple Git-55)
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 04/04/17 01:17, Jordan Justen wrote:
> On 2017-04-02 15:44:55, Phil Dennis-Jordan wrote:
>> From: Phil Dennis-Jordan <phil@philjordan.eu>
>>
>> This adds a header file defining symbolic constants for the VMWare SVGA2
>> virtual display device in preparation for supporting it in QemuVideoDXE.
>>
>> It is an extract of the file lib/vmware/svga_reg.h from commit
>> 329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga
>> repository at git://git.code.sf.net/p/vmware-svga/git (See also
>> http://vmware-svga.sourceforge.net/ )
>>
>> Only the bare essentials necessary for initialisation, modesetting and
>> framebuffer access have been kept from the original file.
>>
>> The original file was released by VMWare under the MIT license, this
>> has been retained.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
>> ---
>>
>> Notes:
>>     v2:
>>     - New, custom header file instead of importing VMWare's verbatim. [Laszlo]
>>
>>  OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h | 102 ++++++++++++++++++++
>>  1 file changed, 102 insertions(+)
>>
>> diff --git a/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
> 
> I think EDK II's file naming convention would prefer VmwareSvga2.h, or
> possibly VmWareSvga2.h. (But, the former looks better to me.)
> 
> Since it covers svga and svga2, should we just drop the '2' from the
> filename?

Yeah, I suggested sort of the same things, after you -- I reviewed the
patches without reading your (earlier) feedback. I suggested VMW_ and
Vmw as prefixes, but I'm equally fine with the more verbose VMWARE_ and
Vmware.

I also agree with dropping "2" from file names / identifiers / macros
etc. (Or, at least, we should be consistent about the use of "2".)


> 
>> new file mode 100644
>> index 000000000000..9db553155957
>> --- /dev/null
>> +++ b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
>> @@ -0,0 +1,102 @@
>> +/** @file
>> +
>> +  Macro and enum definitions of a subset of port numbers, register identifiers
>> +  and values required for driving the VMWare SVGA2 virtual display adapter,
>> +  also implemented by Qemu.
>> +
>> +  This file's contents was extracted from file lib/vmware/svga_reg.h in commit
>> +  329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
>> +  git://git.code.sf.net/p/vmware-svga/git
>> +
>> +
>> +  Copyright 1998-2009 VMware, Inc.  All rights reserved.
>> +  Portions Copyright 2017 Phil Dennis-Jordan <phil@philjordan.eu>
>> +
>> +  Permission is hereby granted, free of charge, to any person
>> +  obtaining a copy of this software and associated documentation
>> +  files (the "Software"), to deal in the Software without
>> +  restriction, including without limitation the rights to use, copy,
>> +  modify, merge, publish, distribute, sublicense, and/or sell copies
>> +  of the Software, and to permit persons to whom the Software is
>> +  furnished to do so, subject to the following conditions:
>> +
>> +  The above copyright notice and this permission notice shall be
>> +  included in all copies or substantial portions of the Software.
>> +
>> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> +  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> +  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> +  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> +  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> +  SOFTWARE.
>> +
>> +**/
>> +
>> +#ifndef _VMWARE_SVGA2_H_
>> +#define _VMWARE_SVGA2_H_
>> +
>> +//
>> +// IDs for recognising the device
>> +//
>> +#define PCI_VENDOR_ID_VMWARE            0x15AD
>> +#define PCI_DEVICE_ID_VMWARE_SVGA2      0x0405
>> +
>> +//
>> +// I/O port BAR offsets for register selection and read/write.
>> +//
>> +// The register index is written to the 32-bit index port, followed by a 32-bit
>> +// read or write on the value port to read or set that register's contents.
>> +//
>> +#define SVGA_INDEX_PORT         0x0
>> +#define SVGA_VALUE_PORT         0x1
> 
> Thanks for taking Laszlo's advice about pulling out the minimal
> definitions and EDK II-ifying it. (I think he may be right about the
> Xen contribution.)
> 
> One request I have is, how about prefixing all the SVGA items as
> VMWARE_SVGA? SVGA seems to generic of a term here.

+1 (as commented elsewhere)

Thanks!
Laszlo

> 
> Thanks,
> 
> -Jordan
> 
>> +
>> +//
>> +// Some of the device's register indices for basic framebuffer functionality.
>> +//
>> +enum {
>> +  SVGA_REG_ID = 0,
>> +  SVGA_REG_ENABLE = 1,
>> +  SVGA_REG_WIDTH = 2,
>> +  SVGA_REG_HEIGHT = 3,
>> +  SVGA_REG_MAX_WIDTH = 4,
>> +  SVGA_REG_MAX_HEIGHT = 5,
>> +
>> +  SVGA_REG_BITS_PER_PIXEL = 7,
>> +
>> +  SVGA_REG_RED_MASK = 9,
>> +  SVGA_REG_GREEN_MASK = 10,
>> +  SVGA_REG_BLUE_MASK = 11,
>> +  SVGA_REG_BYTES_PER_LINE = 12,
>> +
>> +  SVGA_REG_FB_OFFSET = 14,
>> +
>> +  SVGA_REG_FB_SIZE = 16,
>> +  SVGA_REG_CAPABILITIES = 17,
>> +
>> +  SVGA_REG_HOST_BITS_PER_PIXEL = 28,
>> +};
>> +
>> +//
>> +// Values used with SVGA_REG_ID for sanity-checking the device and getting
>> +// its version.
>> +//
>> +#define SVGA_MAGIC         0x900000UL
>> +#define SVGA_MAKE_ID(ver)  (SVGA_MAGIC << 8 | (ver))
>> +
>> +#define SVGA_VERSION_2     2
>> +#define SVGA_ID_2          SVGA_MAKE_ID(SVGA_VERSION_2)
>> +
>> +#define SVGA_VERSION_1     1
>> +#define SVGA_ID_1          SVGA_MAKE_ID(SVGA_VERSION_1)
>> +
>> +#define SVGA_VERSION_0     0
>> +#define SVGA_ID_0          SVGA_MAKE_ID(SVGA_VERSION_0)
>> +
>> +//
>> +// One of the capability bits advertised by SVGA_REG_CAPABILITIES.
>> +//
>> +#define SVGA_CAP_8BIT_EMULATION     0x00000100
>> +
>> +#endif
>> -- 
>> 2.3.2 (Apple Git-55)
>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 04/03/17 00:44, Phil Dennis-Jordan wrote:
> From: Phil Dennis-Jordan <phil@philjordan.eu>
> 
> This adds a header file defining symbolic constants for the VMWare SVGA2
> virtual display device in preparation for supporting it in QemuVideoDXE.
> 
> It is an extract of the file lib/vmware/svga_reg.h from commit
> 329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga
> repository at git://git.code.sf.net/p/vmware-svga/git (See also
> http://vmware-svga.sourceforge.net/ )
> 
> Only the bare essentials necessary for initialisation, modesetting and
> framebuffer access have been kept from the original file.
> 
> The original file was released by VMWare under the MIT license, this
> has been retained.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
> 
> Notes:
>     v2:
>     - New, custom header file instead of importing VMWare's verbatim. [Laszlo]
> 
>  OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h | 102 ++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
> new file mode 100644
> index 000000000000..9db553155957
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/VMWareSVGA2.h
> @@ -0,0 +1,102 @@
> +/** @file
> +
> +  Macro and enum definitions of a subset of port numbers, register identifiers
> +  and values required for driving the VMWare SVGA2 virtual display adapter,
> +  also implemented by Qemu.
> +
> +  This file's contents was extracted from file lib/vmware/svga_reg.h in commit
> +  329dd537456f93a806841ec8a8213aed11395def of VMWare's vmware-svga repository:
> +  git://git.code.sf.net/p/vmware-svga/git
> +
> +
> +  Copyright 1998-2009 VMware, Inc.  All rights reserved.
> +  Portions Copyright 2017 Phil Dennis-Jordan <phil@philjordan.eu>
> +
> +  Permission is hereby granted, free of charge, to any person
> +  obtaining a copy of this software and associated documentation
> +  files (the "Software"), to deal in the Software without
> +  restriction, including without limitation the rights to use, copy,
> +  modify, merge, publish, distribute, sublicense, and/or sell copies
> +  of the Software, and to permit persons to whom the Software is
> +  furnished to do so, subject to the following conditions:
> +
> +  The above copyright notice and this permission notice shall be
> +  included in all copies or substantial portions of the Software.
> +
> +  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +  EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +  MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +  NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> +  BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> +  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> +  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +  SOFTWARE.
> +
> +**/
> +
> +#ifndef _VMWARE_SVGA2_H_
> +#define _VMWARE_SVGA2_H_
> +
> +//
> +// IDs for recognising the device
> +//
> +#define PCI_VENDOR_ID_VMWARE            0x15AD
> +#define PCI_DEVICE_ID_VMWARE_SVGA2      0x0405
> +
> +//
> +// I/O port BAR offsets for register selection and read/write.
> +//
> +// The register index is written to the 32-bit index port, followed by a 32-bit
> +// read or write on the value port to read or set that register's contents.
> +//
> +#define SVGA_INDEX_PORT         0x0
> +#define SVGA_VALUE_PORT         0x1

Please call these VMW_SVGA_...

> +
> +//
> +// Some of the device's register indices for basic framebuffer functionality.
> +//
> +enum {
> +  SVGA_REG_ID = 0,
> +  SVGA_REG_ENABLE = 1,
> +  SVGA_REG_WIDTH = 2,
> +  SVGA_REG_HEIGHT = 3,
> +  SVGA_REG_MAX_WIDTH = 4,
> +  SVGA_REG_MAX_HEIGHT = 5,
> +
> +  SVGA_REG_BITS_PER_PIXEL = 7,
> +
> +  SVGA_REG_RED_MASK = 9,
> +  SVGA_REG_GREEN_MASK = 10,
> +  SVGA_REG_BLUE_MASK = 11,
> +  SVGA_REG_BYTES_PER_LINE = 12,
> +
> +  SVGA_REG_FB_OFFSET = 14,
> +
> +  SVGA_REG_FB_SIZE = 16,
> +  SVGA_REG_CAPABILITIES = 17,
> +
> +  SVGA_REG_HOST_BITS_PER_PIXEL = 28,
> +};

In edk2, enums are usually defined like this:

typedef enum {
  VmwSvgaRegId = 0,
  VmwSvgaRegEnable = 1,
  ...
} VMW_SVGA_REGISTER;

Upper-case underscore-separated identifiers are used for macros and type
names (typedefs) only; please see 4.3.5 "Type and Macro Names" in the
EDK II C Coding Standards (v2.1 draft).

> +
> +//
> +// Values used with SVGA_REG_ID for sanity-checking the device and getting
> +// its version.
> +//
> +#define SVGA_MAGIC         0x900000UL

The macro names should all start with VMW_ please.

> +#define SVGA_MAKE_ID(ver)  (SVGA_MAGIC << 8 | (ver))

Please define WMV_SVGA_MAGIC only as 0x900000U, not 0x900000UL.

> +
> +#define SVGA_VERSION_2     2
> +#define SVGA_ID_2          SVGA_MAKE_ID(SVGA_VERSION_2)

Please insert a space between VMW_SVGA_MAKE_ID and the opening parenthesis.

> +
> +#define SVGA_VERSION_1     1
> +#define SVGA_ID_1          SVGA_MAKE_ID(SVGA_VERSION_1)
> +
> +#define SVGA_VERSION_0     0
> +#define SVGA_ID_0          SVGA_MAKE_ID(SVGA_VERSION_0)
> +
> +//
> +// One of the capability bits advertised by SVGA_REG_CAPABILITIES.
> +//
> +#define SVGA_CAP_8BIT_EMULATION     0x00000100

I suggest to #include <Base.h> in this header (inside the include
guard), and then defining this macro as BIT8.

> +
> +#endif
> 

With those changes, you can add:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel