From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 10/10] vmwgfx: Implement GMR2 Date: Thu, 1 Sep 2011 15:00:25 -0400 Message-ID: <20110901190025.GB20440@dumpdata.com> References: <1314776575-9197-1-git-send-email-thellstrom@vmware.com> <1314776575-9197-9-git-send-email-thellstrom@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from acsinet15.oracle.com (acsinet15.oracle.com [141.146.126.227]) by gabe.freedesktop.org (Postfix) with ESMTP id C6C019EC15 for ; Fri, 2 Sep 2011 05:51:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1314776575-9197-9-git-send-email-thellstrom@vmware.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: Thomas Hellstrom Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, Aug 31, 2011 at 09:42:55AM +0200, Thomas Hellstrom wrote: > Guest Memory Regions 2 is a way to bind pages to the GPU, but using > the FIFO instead of an io-submitted descriptor chain. > = > Signed-off-by: Thomas Hellstrom > Reviewed-by: Jakob Bornecantz > --- > drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c | 81 +++++++++++++++++++++++++++++= +++++- > 1 files changed, 80 insertions(+), 1 deletions(-) > = > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c b/drivers/gpu/drm/vmwgfx= /vmwgfx_gmr.c > index de0c594..f4e7763 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmr.c > @@ -1,6 +1,6 @@ > /***********************************************************************= *** > * > - * Copyright =A9 2009 VMware, Inc., Palo Alto, CA., USA > + * Copyright =A9 2009-2011 VMware, Inc., Palo Alto, CA., USA > * All Rights Reserved. > * > * Permission is hereby granted, free of charge, to any person obtaining= a > @@ -29,6 +29,77 @@ > #include "drmP.h" > #include "ttm/ttm_bo_driver.h" > = > +#define VMW_PPN_SIZE sizeof(unsigned long) > + > +static int vmw_gmr2_bind(struct vmw_private *dev_priv, > + struct page *pages[], > + unsigned long num_pages, > + int gmr_id) > +{ > + SVGAFifoCmdDefineGMR2 define_cmd; > + SVGAFifoCmdRemapGMR2 remap_cmd; > + uint32_t define_size =3D sizeof(define_cmd) + 4; > + uint32_t remap_size =3D VMW_PPN_SIZE * num_pages + sizeof(remap_cmd) + = 4; Why the +4? Is it just as a precaution in case you over-ran? > + uint32_t *cmd; > + uint32_t *cmd_orig; > + uint32_t i; > + > + cmd_orig =3D cmd =3D vmw_fifo_reserve(dev_priv, define_size + remap_siz= e); > + if (unlikely(cmd =3D=3D NULL)) > + return -ENOMEM; > + > + define_cmd.gmrId =3D gmr_id; > + define_cmd.numPages =3D num_pages; > + > + remap_cmd.gmrId =3D gmr_id; > + remap_cmd.flags =3D (VMW_PPN_SIZE > sizeof(*cmd)) ? > + SVGA_REMAP_GMR2_PPN64 : SVGA_REMAP_GMR2_PPN32; > + remap_cmd.offsetPages =3D 0; > + remap_cmd.numPages =3D num_pages; > + > + *cmd++ =3D SVGA_CMD_DEFINE_GMR2; > + memcpy(cmd, &define_cmd, sizeof(define_cmd)); > + cmd +=3D sizeof(define_cmd) / sizeof(uint32); ALIGN macro? Thought I would have thought you would want: cmd +=3D sizeof(define_cmd) - sizeof(uint32); since you stuck a SVGA_CMD_DEFINE_GMR2 and then copied the define_cmd right afterwards? > + > + *cmd++ =3D SVGA_CMD_REMAP_GMR2; > + memcpy(cmd, &remap_cmd, sizeof(remap_cmd)); > + cmd +=3D sizeof(remap_cmd) / sizeof(uint32); > + > + for (i =3D 0; i < num_pages; ++i) { > + if (VMW_PPN_SIZE > 4) > + *cmd =3D page_to_pfn(*pages++); > + else > + *((uint64_t *)cmd) =3D page_to_pfn(*pages++); > + > + cmd +=3D VMW_PPN_SIZE / sizeof(*cmd); > + } > + > + vmw_fifo_commit(dev_priv, define_size + remap_size); > + > + return 0; > +} > + > +static void vmw_gmr2_unbind(struct vmw_private *dev_priv, > + int gmr_id) > +{ > + SVGAFifoCmdDefineGMR2 define_cmd; > + uint32_t define_size =3D sizeof(define_cmd) + 4; > + uint32_t *cmd; > + > + cmd =3D vmw_fifo_reserve(dev_priv, define_size); > + if (unlikely(cmd =3D=3D NULL)) { > + DRM_ERROR("GMR2 unbind failed.\n"); > + return; > + } > + define_cmd.gmrId =3D gmr_id; > + define_cmd.numPages =3D 0; > + > + *cmd++ =3D SVGA_CMD_DEFINE_GMR2; Ah, that is what the +4 is for. Would it be cleaner to just do 'sizeof(SVGA_CMD_DEFINE_GMR2)' ? > + memcpy(cmd, &define_cmd, sizeof(define_cmd)); > + > + vmw_fifo_commit(dev_priv, define_size); > +} > + > /** > * FIXME: Adjust to the ttm lowmem / highmem storage to minimize > * the number of used descriptors. > @@ -170,6 +241,9 @@ int vmw_gmr_bind(struct vmw_private *dev_priv, > struct list_head desc_pages; > int ret; > = > + if (likely(dev_priv->capabilities & SVGA_CAP_GMR2)) > + return vmw_gmr2_bind(dev_priv, pages, num_pages, gmr_id); > + > if (unlikely(!(dev_priv->capabilities & SVGA_CAP_GMR))) > return -EINVAL; > = > @@ -192,6 +266,11 @@ int vmw_gmr_bind(struct vmw_private *dev_priv, > = > void vmw_gmr_unbind(struct vmw_private *dev_priv, int gmr_id) > { > + if (likely(dev_priv->capabilities & SVGA_CAP_GMR2)) { > + vmw_gmr2_unbind(dev_priv, gmr_id); > + return; > + } > + > mutex_lock(&dev_priv->hw_mutex); > vmw_write(dev_priv, SVGA_REG_GMR_ID, gmr_id); > wmb(); > -- = > 1.7.4.4 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel