From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH 06/10] vmwgfx: Update register definitions for HWV8 and print out new capabilities Date: Thu, 01 Sep 2011 21:36:27 +0200 Message-ID: <4E5FDEBB.4000106@vmware.com> References: <1314776575-9197-1-git-send-email-thellstrom@vmware.com> <1314776575-9197-5-git-send-email-thellstrom@vmware.com> <20110901180213.GA20440@dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [65.115.85.69]) by gabe.freedesktop.org (Postfix) with ESMTP id 1A33A9E794 for ; Thu, 1 Sep 2011 12:37:55 -0700 (PDT) In-Reply-To: <20110901180213.GA20440@dumpdata.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Konrad Rzeszutek Wilk Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org Thanks for reviewing, please see inline. On 09/01/2011 08:02 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Aug 31, 2011 at 09:42:51AM +0200, Thomas Hellstrom wrote: > = >> Signed-off-by: Thomas Hellstrom >> Reviewed-by: Jos=E9 Fonseca >> --- >> drivers/gpu/drm/vmwgfx/svga_reg.h | 96 +++++++++++++++++++++++++++= +++++++- >> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++ >> 2 files changed, 99 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/svga_reg.h b/drivers/gpu/drm/vmwgfx/= svga_reg.h >> index 1b96c2e..ec5aad9 100644 >> --- a/drivers/gpu/drm/vmwgfx/svga_reg.h >> +++ b/drivers/gpu/drm/vmwgfx/svga_reg.h >> @@ -39,6 +39,15 @@ >> #define PCI_DEVICE_ID_VMWARE_SVGA2 0x0405 >> >> /* >> + * SVGA_REG_ENABLE bit definitions. >> + */ >> +#define SVGA_REG_ENABLE_DISABLE 0 >> +#define SVGA_REG_ENABLE_ENABLE 1 >> +#define SVGA_REG_ENABLE_HIDE 2 >> +#define SVGA_REG_ENABLE_ENABLE_HIDE (SVGA_REG_ENABLE_ENABLE |\ >> + SVGA_REG_ENABLE_HIDE) >> = > That is a mouthfull. "enable enable" sounds repeative. > > What if they were: SVGA_REG_ENABLE_OFF, SVGA_REG_ENABLE_ON ? > = We're trying to keep the open source version of this header as close as = possible to the internal headers. Most likely VMware guys are going to be the = primary maintainers this driver in the future, and having different = names for the register values will be confusing, although I think you = have a point. > = >> + >> +/* >> * Legal values for the SVGA_REG_CURSOR_ON register in old-fashioned >> * cursor bypass mode. This is still supported, but no new guest >> * drivers should use it. >> @@ -158,7 +167,9 @@ enum { >> SVGA_REG_GMR_MAX_DESCRIPTOR_LENGTH =3D 44, >> >> SVGA_REG_TRACES =3D 45, /* Enable trace-based updates ev= en when FIFO is on */ >> - SVGA_REG_TOP =3D 46, /* Must be 1 more than the last r= egister */ >> + SVGA_REG_GMRS_MAX_PAGES =3D 46, /* Maximum number of 4KB pages fo= r all GMRs */ >> + SVGA_REG_MEMORY_SIZE =3D 47, /* Total dedicated device memory = excluding FIFO */ >> + SVGA_REG_TOP =3D 48, /* Must be 1 more than the last r= egister */ >> >> SVGA_PALETTE_BASE =3D 1024, /* Base of SVGA color map */ >> /* Next 768 (=3D=3D 256*3) registers exist for colormap */ >> @@ -370,6 +381,15 @@ struct SVGASignedPoint { >> * Note the holes in the bitfield. Missing bits have been deprecated, >> * and must not be reused. Those capabilities will never be reported >> * by new versions of the SVGA device. >> + * >> + * SVGA_CAP_GMR2 -- >> + * Provides asynchronous commands to define and remap guest memory >> + * regions. Adds device registers SVGA_REG_GMRS_MAX_PAGES and >> + * SVGA_REG_MEMORY_SIZE. >> + * >> + * SVGA_CAP_SCREEN_OBJECT_2 -- >> + * Allow screen object support, and require backing stores from the >> + * guest for each screen object. >> */ >> >> #define SVGA_CAP_NONE 0x00000000 >> @@ -387,6 +407,8 @@ struct SVGASignedPoint { >> #define SVGA_CAP_DISPLAY_TOPOLOGY 0x00080000 // Legacy multi-monit= or support >> #define SVGA_CAP_GMR 0x00100000 >> #define SVGA_CAP_TRACES 0x00200000 >> +#define SVGA_CAP_GMR2 0x00400000 >> +#define SVGA_CAP_SCREEN_OBJECT_2 0x00800000 >> >> >> /* >> @@ -885,6 +907,8 @@ typedef enum { >> SVGA_CMD_BLIT_SCREEN_TO_GMRFB =3D 38, >> SVGA_CMD_ANNOTATION_FILL =3D 39, >> SVGA_CMD_ANNOTATION_COPY =3D 40, >> + SVGA_CMD_DEFINE_GMR2 =3D 41, >> + SVGA_CMD_REMAP_GMR2 =3D 42, >> SVGA_CMD_MAX >> } SVGAFifoCmdId; >> >> @@ -1343,4 +1367,74 @@ struct { >> uint32 srcScreenId; >> } SVGAFifoCmdAnnotationCopy; >> >> + >> +/* >> + * SVGA_CMD_DEFINE_GMR2 -- >> + * >> + * Define guest memory region v2. See the description of GMRs above. >> + * >> + * Availability: >> + * SVGA_CAP_GMR2 >> + */ >> + >> +typedef >> +struct { >> + uint32 gmrId; >> + uint32 numPages; >> +} >> +SVGAFifoCmdDefineGMR2; >> + >> + >> +/* >> + * SVGA_CMD_REMAP_GMR2 -- >> + * >> + * Remap guest memory region v2. See the description of GMRs above. >> = > out of curiosity - what happend to v1? > = V1 is present, but is using IO registers instead of fifo commands. = Please see the "Implement GMR2 patch". > = >> + * >> + * This command allows guest to modify a portion of an existing GMR = by >> + * invalidating it or reassigning it to different guest physical pag= es. >> + * The pages are identified by physical page number (PPN). The pages >> + * are assumed to be pinned and valid for DMA operations. >> + * >> + * Description of command flags: >> + * >> + * SVGA_REMAP_GMR2_VIA_GMR: If enabled, references a PPN list in a G= MR. >> + * The PPN list must not overlap with the remap region (this can = be >> + * handled trivially by referencing a separate GMR). If flag is >> + * disabled, PPN list is appended to SVGARemapGMR command. >> + * >> + * SVGA_REMAP_GMR2_PPN64: If set, PPN list is in PPN64 format, other= wise >> + * it is in PPN32 format. >> + * >> + * SVGA_REMAP_GMR2_SINGLE_PPN: If set, PPN list contains a single en= try. >> + * A single PPN can be used to invalidate a portion of a GMR or >> + * map it to to a single guest scratch page. >> + * >> + * Availability: >> + * SVGA_CAP_GMR2 >> + */ >> + >> +typedef enum { >> + SVGA_REMAP_GMR2_PPN32 =3D 0, >> + SVGA_REMAP_GMR2_VIA_GMR =3D (1<< 0), >> + SVGA_REMAP_GMR2_PPN64 =3D (1<< 1), >> + SVGA_REMAP_GMR2_SINGLE_PPN =3D (1<< 2), >> +} SVGARemapGMR2Flags; >> + >> +typedef >> +struct { >> + uint32 gmrId; >> + SVGARemapGMR2Flags flags; >> + uint32 offsetPages; /* offset in pages to begin remap */ >> + uint32 numPages; /* number of pages to remap */ >> = > That formating looks a bit wierd. Like one space and two spaces - but > it might be just my editor (vim). > = It's probably weird, but we're trying to keep the formatting of the = original file for the register definitions. >> + /* >> + * Followed by additional data depending on SVGARemapGMR2Flags. >> + * >> + * If flag SVGA_REMAP_GMR2_VIA_GMR is set, single SVGAGuestPtr follo= ws. >> + * Otherwise an array of page descriptors in PPN32 or PPN64 format >> + * (according to flag SVGA_REMAP_GMR2_PPN64) follows. If flag >> + * SVGA_REMAP_GMR2_SINGLE_PPN is set, array contains a single entry. >> + */ >> +} >> +SVGAFifoCmdRemapGMR2; >> + >> #endif >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgf= x/vmwgfx_drv.c >> index 96949b9..3a98e56 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c >> @@ -189,6 +189,10 @@ static void vmw_print_capabilities(uint32_t capabil= ities) >> DRM_INFO(" GMR.\n"); >> if (capabilities& SVGA_CAP_TRACES) >> DRM_INFO(" Traces.\n"); >> + if (capabilities& SVGA_CAP_GMR2) >> + DRM_INFO(" GMR2.\n"); >> + if (capabilities& SVGA_CAP_SCREEN_OBJECT_2) >> + DRM_INFO(" Screen Object 2.\n"); >> } >> >> static int vmw_request_device(struct vmw_private *dev_priv) >> -- = >> 1.7.4.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> = Thanks for reviewing. /Thomas