drm/vmwgfx: switch from ioremap_cache to memremap

Commit Message

Dan Williams Oct. 19, 2015, 9:34 p.m. UTC

On Tue, 2015-10-13 at 20:52 +0200, Thomas Hellstrom wrote:
> > Ok, I'll make local read_fifo() and write_fifo() macros to make this
> > explicit.  Are these names ok with you?
> 
> Sure.
> 

So I ended up just leaving the __iomem annotation on mmio_virt for now
until the implementation can be converted to use explicit barriers where
necessary.

8<-----
Subject: drm/vmwgfx: switch from ioremap_cache to memremap

From: Dan Williams <[e-mail]>

Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver
expects the fifo registers to be cacheable.  In preparation for
deprecating ioremap_cache() convert its usage in vmwgfx to memremap().

Cc: David Airlie <[e-mail]>
Cc: Thomas Hellstrom <[e-mail]>
Cc: Sinclair Yeh <[e-mail]>
Cc: [e-mail]
Signed-off-by: Dan Williams <[e-mail]>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Thomas Hellstrom Oct. 28, 2015, 7:05 a.m. UTC | #1

Dan,

On 10/19/2015 11:34 PM, Williams, Dan J wrote:
> On Tue, 2015-10-13 at 20:52 +0200, Thomas Hellstrom wrote:
>>> Ok, I'll make local read_fifo() and write_fifo() macros to make this
>>> explicit.  Are these names ok with you?
>> Sure.
>>
> So I ended up just leaving the __iomem annotation on mmio_virt for now
> until the implementation can be converted to use explicit barriers where
> necessary.

Thanks, I'll queue this on vmwgfx-next for 4.4 and carry on from there.

Reviewed-by: Thomas Hellstrom <[e-mail]>


>
> 8<-----
> Subject: drm/vmwgfx: switch from ioremap_cache to memremap
>
> From: Dan Williams <[e-mail]>
>
> Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver
> expects the fifo registers to be cacheable.  In preparation for
> deprecating ioremap_cache() convert its usage in vmwgfx to memremap().
>
> Cc: David Airlie <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: [e-mail]
> Signed-off-by: Dan Williams <[e-mail]>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |   14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 2c7a25c71af2..33e9eda77bad 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -752,8 +752,14 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
>  	ttm_lock_set_kill(&dev_priv->fbdev_master.lock, false, SIGTERM);
>  	dev_priv->active_master = &dev_priv->fbdev_master;
>  
> -	dev_priv->mmio_virt = ioremap_cache(dev_priv->mmio_start,
> -					    dev_priv->mmio_size);
> +	/*
> +	 * Force __iomem for this mapping until the implied compiler
> +	 * barriers and {READ|WRITE}_ONCE semantics from the
> +	 * io{read|write}32() accessors can be replaced with explicit
> +	 * barriers.
> +	 */
> +	dev_priv->mmio_virt = (void __iomem *) memremap(dev_priv->mmio_start,
> +					    dev_priv->mmio_size, MEMREMAP_WB);
>  
>  	if (unlikely(dev_priv->mmio_virt == NULL)) {
>  		ret = -ENOMEM;
> @@ -907,7 +913,7 @@ out_no_irq:
>  out_no_device:
>  	ttm_object_device_release(&dev_priv->tdev);
>  out_err4:
> -	iounmap(dev_priv->mmio_virt);
> +	memunmap((void __force *) dev_priv->mmio_virt);
>  out_err3:
>  	vmw_ttm_global_release(dev_priv);
>  out_err0:
> @@ -958,7 +964,7 @@ static int vmw_driver_unload(struct drm_device *dev)
>  		pci_release_regions(dev->pdev);
>  
>  	ttm_object_device_release(&dev_priv->tdev);
> -	iounmap(dev_priv->mmio_virt);
> +	memunmap((void __force *) dev_priv->mmio_virt);
>  	if (dev_priv->ctx.staged_bindings)
>  		vmw_binding_state_free(dev_priv->ctx.staged_bindings);
>  	vmw_ttm_global_release(dev_priv);
>

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 2c7a25c71af2..33e9eda77bad 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -752,8 +752,14 @@  static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	ttm_lock_set_kill(&dev_priv->fbdev_master.lock, false, SIGTERM);
 	dev_priv->active_master = &dev_priv->fbdev_master;
 
-	dev_priv->mmio_virt = ioremap_cache(dev_priv->mmio_start,
-					    dev_priv->mmio_size);
+	/*
+	 * Force __iomem for this mapping until the implied compiler
+	 * barriers and {READ|WRITE}_ONCE semantics from the
+	 * io{read|write}32() accessors can be replaced with explicit
+	 * barriers.
+	 */
+	dev_priv->mmio_virt = (void __iomem *) memremap(dev_priv->mmio_start,
+					    dev_priv->mmio_size, MEMREMAP_WB);
 
 	if (unlikely(dev_priv->mmio_virt == NULL)) {
 		ret = -ENOMEM;
@@ -907,7 +913,7 @@  out_no_irq:
 out_no_device:
 	ttm_object_device_release(&dev_priv->tdev);
 out_err4:
-	iounmap(dev_priv->mmio_virt);
+	memunmap((void __force *) dev_priv->mmio_virt);
 out_err3:
 	vmw_ttm_global_release(dev_priv);
 out_err0:
@@ -958,7 +964,7 @@  static int vmw_driver_unload(struct drm_device *dev)
 		pci_release_regions(dev->pdev);
 
 	ttm_object_device_release(&dev_priv->tdev);
-	iounmap(dev_priv->mmio_virt);
+	memunmap((void __force *) dev_priv->mmio_virt);
 	if (dev_priv->ctx.staged_bindings)
 		vmw_binding_state_free(dev_priv->ctx.staged_bindings);
 	vmw_ttm_global_release(dev_priv);