[PATCH 06/10] vmwgfx: Update register definitions for HWV8 and print out new capabilities
Thomas Hellstrom
thellstrom at vmware.com
Thu Sep 1 12:36:27 PDT 2011
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<thellstrom at vmware.com>
>> Reviewed-by: José Fonseca<jfonseca at vmware.com>
>> ---
>> 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 = 44,
>>
>> SVGA_REG_TRACES = 45, /* Enable trace-based updates even when FIFO is on */
>> - SVGA_REG_TOP = 46, /* Must be 1 more than the last register */
>> + SVGA_REG_GMRS_MAX_PAGES = 46, /* Maximum number of 4KB pages for all GMRs */
>> + SVGA_REG_MEMORY_SIZE = 47, /* Total dedicated device memory excluding FIFO */
>> + SVGA_REG_TOP = 48, /* Must be 1 more than the last register */
>>
>> SVGA_PALETTE_BASE = 1024, /* Base of SVGA color map */
>> /* Next 768 (== 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-monitor 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 = 38,
>> SVGA_CMD_ANNOTATION_FILL = 39,
>> SVGA_CMD_ANNOTATION_COPY = 40,
>> + SVGA_CMD_DEFINE_GMR2 = 41,
>> + SVGA_CMD_REMAP_GMR2 = 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 pages.
>> + * 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 GMR.
>> + * 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, otherwise
>> + * it is in PPN32 format.
>> + *
>> + * SVGA_REMAP_GMR2_SINGLE_PPN: If set, PPN list contains a single entry.
>> + * 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 = 0,
>> + SVGA_REMAP_GMR2_VIA_GMR = (1<< 0),
>> + SVGA_REMAP_GMR2_PPN64 = (1<< 1),
>> + SVGA_REMAP_GMR2_SINGLE_PPN = (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 follows.
>> + * 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/vmwgfx/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 capabilities)
>> 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
Thanks for reviewing.
/Thomas
More information about the dri-devel
mailing list