| Message ID | 20190125130548.3266-2-thellstrom@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled | expand |
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
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
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;
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(-)