[2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled
diff mbox series

Message ID 20190125130548.3266-2-thellstrom@vmware.com
State New
Headers show
Series
  • [1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
Related show

Commit Message

Thomas Hellstrom Jan. 25, 2019, 1:05 p.m. UTC
Instead of guessing whether TTM has the dma page allocator enabled,
ask TTM.

Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Sam Ravnborg Jan. 25, 2019, 6:22 p.m. UTC | #1
Hi Thomas.

One little nit, and an improvment proposal (for another patch/day).

On Fri, Jan 25, 2019 at 02:05:48PM +0100, Thomas Hellstrom wrote:
> Instead of guessing whether TTM has the dma page allocator enabled,
> ask TTM.
> 
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 77f422cd18ab..125a2b423847 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -35,6 +35,7 @@
>  #include <drm/ttm/ttm_placement.h>
>  #include <drm/ttm/ttm_bo_driver.h>
>  #include <drm/ttm/ttm_module.h>
> +#include <drm/ttm/ttm_page_alloc.h>
>  
>  #define VMWGFX_DRIVER_DESC "Linux drm driver for VMware graphics devices"
>  #define VMWGFX_CHIP_SVGAII 0
> @@ -594,8 +595,7 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
>  	if (dev_priv->map_mode == vmw_dma_map_populate && vmw_restrict_iommu)
>  		dev_priv->map_mode = vmw_dma_map_bind;
>  
> -	/* No TTM coherent page pool? FIXME: Ask TTM instead! */
> -        if (!(IS_ENABLED(CONFIG_SWIOTLB) || IS_ENABLED(CONFIG_INTEL_IOMMU)) &&
> +        if (!ttm_dma_page_alloc_enabled() &&
The line uses spaces for indent, tabs?

>  	    (dev_priv->map_mode == vmw_dma_alloc_coherent))

The code could benefit from a:
static bool is_map_coherent(const struct vmw_private *dev_priv)
{
	return dev_priv->map_mode == vmw_dma_alloc_coherent;
}

Then the above would become:

	if (!ttm_dma_page_alloc_enabled() && is_map_coherent(dev_priv))

And the other places that test for vmw_dma_alloc_coherent
would be a bit simpler too.
Same goes for the other map types obviously.

	Sam
Thomas Hellström (VMware) Jan. 30, 2019, 8:37 a.m. UTC | #2
Hi, Sam,

On 01/25/2019 07:22 PM, Sam Ravnborg wrote:
> Hi Thomas.
>
> One little nit, and an improvment proposal (for another patch/day).
>
> On Fri, Jan 25, 2019 at 02:05:48PM +0100, Thomas Hellstrom wrote:
>> Instead of guessing whether TTM has the dma page allocator enabled,
>> ask TTM.
>>
>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>> ---
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> index 77f422cd18ab..125a2b423847 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>> @@ -35,6 +35,7 @@
>>   #include <drm/ttm/ttm_placement.h>
>>   #include <drm/ttm/ttm_bo_driver.h>
>>   #include <drm/ttm/ttm_module.h>
>> +#include <drm/ttm/ttm_page_alloc.h>
>>   
>>   #define VMWGFX_DRIVER_DESC "Linux drm driver for VMware graphics devices"
>>   #define VMWGFX_CHIP_SVGAII 0
>> @@ -594,8 +595,7 @@ static int vmw_dma_select_mode(struct vmw_private *dev_priv)
>>   	if (dev_priv->map_mode == vmw_dma_map_populate && vmw_restrict_iommu)
>>   		dev_priv->map_mode = vmw_dma_map_bind;
>>   
>> -	/* No TTM coherent page pool? FIXME: Ask TTM instead! */
>> -        if (!(IS_ENABLED(CONFIG_SWIOTLB) || IS_ENABLED(CONFIG_INTEL_IOMMU)) &&
>> +        if (!ttm_dma_page_alloc_enabled() &&
> The line uses spaces for indent, tabs?

Ugh, I thought I ran this through checkpatch. Anyway that will get 
corrected. Thanks.


>
>>   	    (dev_priv->map_mode == vmw_dma_alloc_coherent))
> The code could benefit from a:
> static bool is_map_coherent(const struct vmw_private *dev_priv)
> {
> 	return dev_priv->map_mode == vmw_dma_alloc_coherent;
> }
>
> Then the above would become:
>
> 	if (!ttm_dma_page_alloc_enabled() && is_map_coherent(dev_priv))
>
> And the other places that test for vmw_dma_alloc_coherent
> would be a bit simpler too.
> Same goes for the other map types obviously.
Sure. I'll add that to the backlog for consideration, although if we get 
to it we'll use other names because all map modes are presumed to be 
coherent...

Thanks,
/Thomas

Patch
diff mbox series

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 77f422cd18ab..125a2b423847 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -35,6 +35,7 @@ 
 #include <drm/ttm/ttm_placement.h>
 #include <drm/ttm/ttm_bo_driver.h>
 #include <drm/ttm/ttm_module.h>
+#include <drm/ttm/ttm_page_alloc.h>
 
 #define VMWGFX_DRIVER_DESC "Linux drm driver for VMware graphics devices"
 #define VMWGFX_CHIP_SVGAII 0
@@ -594,8 +595,7 @@  static int vmw_dma_select_mode(struct vmw_private *dev_priv)
 	if (dev_priv->map_mode == vmw_dma_map_populate && vmw_restrict_iommu)
 		dev_priv->map_mode = vmw_dma_map_bind;
 
-	/* No TTM coherent page pool? FIXME: Ask TTM instead! */
-        if (!(IS_ENABLED(CONFIG_SWIOTLB) || IS_ENABLED(CONFIG_INTEL_IOMMU)) &&
+        if (!ttm_dma_page_alloc_enabled() &&
 	    (dev_priv->map_mode == vmw_dma_alloc_coherent))
 		return -EINVAL;