From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AC12521939308 for ; Tue, 4 Apr 2017 00:54:29 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 07C4880B22; Tue, 4 Apr 2017 07:54:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 07C4880B22 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 07C4880B22 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-91.phx2.redhat.com [10.3.116.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id BDBA57A240; Tue, 4 Apr 2017 07:54:27 +0000 (UTC) To: Phil Dennis-Jordan , edk2-devel@lists.01.org References: <1491173097-37305-1-git-send-email-lists@philjordan.eu> <1491173097-37305-2-git-send-email-lists@philjordan.eu> Cc: Jordan Justen , Phil Dennis-Jordan From: Laszlo Ersek Message-ID: <81017615-6f68-5acd-43bd-6915ef8c29de@redhat.com> Date: Tue, 4 Apr 2017 09:54:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <1491173097-37305-2-git-send-email-lists@philjordan.eu> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 04 Apr 2017 07:54:29 +0000 (UTC) Subject: Re: [PATCH v2 1/3] OvmfPkg: VMWare SVGA2 display device register definitions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Apr 2017 07:54:29 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 04/03/17 00:44, Phil Dennis-Jordan wrote: > From: Phil Dennis-Jordan > > 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 > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Phil Dennis-Jordan > --- > > 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 > + > + 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 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 Thanks! Laszlo