All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
@ 2019-01-25 13:05 Thomas Hellstrom
  2019-01-25 13:05 ` [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled Thomas Hellstrom
  2019-01-28 12:21 ` [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Koenig, Christian
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Hellstrom @ 2019-01-25 13:05 UTC (permalink / raw
  To: linux-graphics-maintainer, dri-devel; +Cc: Thomas Hellstrom, Koenig, Christian

The vmwgfx driver needs to know whether the dma page pool is enabled
to determine whether to refuse loading if the dma mode logic
requests coherent memory from the dma page pool.

Cc: "Koenig, Christian" <Christian.Koenig@amd.com>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 11 +++++++++++
 include/drm/ttm/ttm_page_alloc.h         |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index d594f7520b7b..adf8cc189ecc 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -1235,4 +1235,15 @@ int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
 }
 EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
 
+/**
+ * ttm_dma_page_alloc_enabled - Is the dma page pool enabled?
+ *
+ * Returns true if the dma page pool is enabled. false otherwise.
+ */
+bool ttm_dma_page_alloc_enabled(void)
+{
+	return !!_manager;
+}
+EXPORT_SYMBOL(ttm_dma_page_alloc_enabled);
+
 #endif
diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index 4d9b019d253c..f810d389f5ad 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -94,6 +94,8 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
 			struct ttm_operation_ctx *ctx);
 void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev);
 
+bool ttm_dma_page_alloc_enabled(void);
+
 #else
 static inline int ttm_dma_page_alloc_init(struct ttm_mem_global *glob,
 					  unsigned max_pages)
@@ -117,6 +119,8 @@ static inline void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma,
 				      struct device *dev)
 {
 }
+
+static inline bool ttm_dma_page_alloc_enabled(void) { return false; }
 #endif
 
 #endif
-- 
2.19.0.rc1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled
  2019-01-25 13:05 [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Thomas Hellstrom
@ 2019-01-25 13:05 ` Thomas Hellstrom
  2019-01-25 18:22   ` Sam Ravnborg
  2019-01-28 12:21 ` [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Koenig, Christian
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Hellstrom @ 2019-01-25 13:05 UTC (permalink / raw
  To: linux-graphics-maintainer, dri-devel; +Cc: Thomas Hellstrom

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() &&
 	    (dev_priv->map_mode == vmw_dma_alloc_coherent))
 		return -EINVAL;
 
-- 
2.19.0.rc1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled
  2019-01-25 13:05 ` [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled Thomas Hellstrom
@ 2019-01-25 18:22   ` Sam Ravnborg
  2019-01-30  8:37     ` Thomas Hellstrom
  0 siblings, 1 reply; 8+ messages in thread
From: Sam Ravnborg @ 2019-01-25 18:22 UTC (permalink / raw
  To: Thomas Hellstrom; +Cc: linux-graphics-maintainer, dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
  2019-01-25 13:05 [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Thomas Hellstrom
  2019-01-25 13:05 ` [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled Thomas Hellstrom
@ 2019-01-28 12:21 ` Koenig, Christian
  2019-01-28 13:29   ` Thomas Hellstrom
  1 sibling, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2019-01-28 12:21 UTC (permalink / raw
  To: Thomas Hellstrom, linux-graphics-maintainer@vmware.com,
	dri-devel@lists.freedesktop.org

Am 25.01.19 um 14:05 schrieb Thomas Hellstrom:
> The vmwgfx driver needs to know whether the dma page pool is enabled
> to determine whether to refuse loading if the dma mode logic
> requests coherent memory from the dma page pool.
>
> Cc: "Koenig, Christian" <Christian.Koenig@amd.com>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 11 +++++++++++
>   include/drm/ttm/ttm_page_alloc.h         |  4 ++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index d594f7520b7b..adf8cc189ecc 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -1235,4 +1235,15 @@ int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
>   }
>   EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
>   
> +/**
> + * ttm_dma_page_alloc_enabled - Is the dma page pool enabled?
> + *
> + * Returns true if the dma page pool is enabled. false otherwise.
> + */
> +bool ttm_dma_page_alloc_enabled(void)
> +{
> +	return !!_manager;
> +}
> +EXPORT_SYMBOL(ttm_dma_page_alloc_enabled);
> +

This is completely superfluous cause the manager is enabled as soon as 
it is compiled in.

You could just use "#if defined(CONFIG_SWIOTLB) || 
defined(CONFIG_INTEL_IOMMU)" instead.

Christian.

>   #endif
> diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
> index 4d9b019d253c..f810d389f5ad 100644
> --- a/include/drm/ttm/ttm_page_alloc.h
> +++ b/include/drm/ttm/ttm_page_alloc.h
> @@ -94,6 +94,8 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
>   			struct ttm_operation_ctx *ctx);
>   void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev);
>   
> +bool ttm_dma_page_alloc_enabled(void);
> +
>   #else
>   static inline int ttm_dma_page_alloc_init(struct ttm_mem_global *glob,
>   					  unsigned max_pages)
> @@ -117,6 +119,8 @@ static inline void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma,
>   				      struct device *dev)
>   {
>   }
> +
> +static inline bool ttm_dma_page_alloc_enabled(void) { return false; }
>   #endif
>   
>   #endif

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
  2019-01-28 12:21 ` [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Koenig, Christian
@ 2019-01-28 13:29   ` Thomas Hellstrom
  2019-01-28 14:21     ` Thomas Hellstrom
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Hellstrom @ 2019-01-28 13:29 UTC (permalink / raw
  To: dri-devel@lists.freedesktop.org, Linux-graphics-maintainer,
	Christian.Koenig@amd.com

Hi,

On Mon, 2019-01-28 at 12:21 +0000, Koenig, Christian wrote:
> Am 25.01.19 um 14:05 schrieb Thomas Hellstrom:
> > The vmwgfx driver needs to know whether the dma page pool is
> > enabled
> > to determine whether to refuse loading if the dma mode logic
> > requests coherent memory from the dma page pool.
> > 
> > Cc: "Koenig, Christian" <Christian.Koenig@amd.com>
> > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> > ---
> >   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 11 +++++++++++
> >   include/drm/ttm/ttm_page_alloc.h         |  4 ++++
> >   2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > index d594f7520b7b..adf8cc189ecc 100644
> > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > @@ -1235,4 +1235,15 @@ int ttm_dma_page_alloc_debugfs(struct
> > seq_file *m, void *data)
> >   }
> >   EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
> >   
> > +/**
> > + * ttm_dma_page_alloc_enabled - Is the dma page pool enabled?
> > + *
> > + * Returns true if the dma page pool is enabled. false otherwise.
> > + */
> > +bool ttm_dma_page_alloc_enabled(void)
> > +{
> > +	return !!_manager;
> > +}
> > +EXPORT_SYMBOL(ttm_dma_page_alloc_enabled);
> > +
> 
> This is completely superfluous cause the manager is enabled as soon
> as 
> it is compiled in.
> 
> You could just use "#if defined(CONFIG_SWIOTLB) || 
> defined(CONFIG_INTEL_IOMMU)" instead.
> 
> Christian.
> 

Yes, that's what was done before in vmgwfx, but it's very fragile and
based on assumptions about what various parts of TTM does and doesn't
do. Chances are somebody would modify the enablement and forget to
modify this function. 

But of course I can change it if you think that'd be better.

/Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
  2019-01-28 13:29   ` Thomas Hellstrom
@ 2019-01-28 14:21     ` Thomas Hellstrom
  2019-01-28 15:56       ` Koenig, Christian
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Hellstrom @ 2019-01-28 14:21 UTC (permalink / raw
  To: dri-devel@lists.freedesktop.org, Linux-graphics-maintainer,
	Christian.Koenig@amd.com

On Mon, 2019-01-28 at 14:29 +0100, Thomas Hellström wrote:
> Hi,
> 
> On Mon, 2019-01-28 at 12:21 +0000, Koenig, Christian wrote:
> > Am 25.01.19 um 14:05 schrieb Thomas Hellstrom:
> > > The vmwgfx driver needs to know whether the dma page pool is
> > > enabled
> > > to determine whether to refuse loading if the dma mode logic
> > > requests coherent memory from the dma page pool.
> > > 
> > > Cc: "Koenig, Christian" <Christian.Koenig@amd.com>
> > > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> > > ---
> > >   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 11 +++++++++++
> > >   include/drm/ttm/ttm_page_alloc.h         |  4 ++++
> > >   2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > > b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > > index d594f7520b7b..adf8cc189ecc 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> > > @@ -1235,4 +1235,15 @@ int ttm_dma_page_alloc_debugfs(struct
> > > seq_file *m, void *data)
> > >   }
> > >   EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
> > >   
> > > +/**
> > > + * ttm_dma_page_alloc_enabled - Is the dma page pool enabled?
> > > + *
> > > + * Returns true if the dma page pool is enabled. false
> > > otherwise.
> > > + */
> > > +bool ttm_dma_page_alloc_enabled(void)
> > > +{
> > > +	return !!_manager;
> > > +}
> > > +EXPORT_SYMBOL(ttm_dma_page_alloc_enabled);
> > > +
> > 
> > This is completely superfluous cause the manager is enabled as soon
> > as 
> > it is compiled in.
> > 
> > You could just use "#if defined(CONFIG_SWIOTLB) || 
> > defined(CONFIG_INTEL_IOMMU)" instead.
> > 
> > Christian.
> > 
> 
> Yes, that's what was done before in vmgwfx, but it's very fragile and
> based on assumptions about what various parts of TTM does and doesn't
> do. Chances are somebody would modify the enablement and forget to
> modify this function. 
> 
> But of course I can change it if you think that'd be better.
> 
> /Thomas
> 

What about

static inline bool ttm_dma_page_alloc_enabled(void)
{
	return (IS_ENABLED(CONFIG_INTEL_IOMMU) ||
IS_ENABLED(CONFIG_SWIOTLB)) && _manager;
}

to avoid that layering violation?

/Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled
  2019-01-28 14:21     ` Thomas Hellstrom
@ 2019-01-28 15:56       ` Koenig, Christian
  0 siblings, 0 replies; 8+ messages in thread
From: Koenig, Christian @ 2019-01-28 15:56 UTC (permalink / raw
  To: Thomas Hellstrom, dri-devel@lists.freedesktop.org,
	Linux-graphics-maintainer

Am 28.01.19 um 15:21 schrieb Thomas Hellstrom:
> On Mon, 2019-01-28 at 14:29 +0100, Thomas Hellström wrote:
>> Hi,
>>
>> On Mon, 2019-01-28 at 12:21 +0000, Koenig, Christian wrote:
>>> Am 25.01.19 um 14:05 schrieb Thomas Hellstrom:
>>>> The vmwgfx driver needs to know whether the dma page pool is
>>>> enabled
>>>> to determine whether to refuse loading if the dma mode logic
>>>> requests coherent memory from the dma page pool.
>>>>
>>>> Cc: "Koenig, Christian" <Christian.Koenig@amd.com>
>>>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 11 +++++++++++
>>>>    include/drm/ttm/ttm_page_alloc.h         |  4 ++++
>>>>    2 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>>> b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>>> index d594f7520b7b..adf8cc189ecc 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
>>>> @@ -1235,4 +1235,15 @@ int ttm_dma_page_alloc_debugfs(struct
>>>> seq_file *m, void *data)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
>>>>    
>>>> +/**
>>>> + * ttm_dma_page_alloc_enabled - Is the dma page pool enabled?
>>>> + *
>>>> + * Returns true if the dma page pool is enabled. false
>>>> otherwise.
>>>> + */
>>>> +bool ttm_dma_page_alloc_enabled(void)
>>>> +{
>>>> +	return !!_manager;
>>>> +}
>>>> +EXPORT_SYMBOL(ttm_dma_page_alloc_enabled);
>>>> +
>>> This is completely superfluous cause the manager is enabled as soon
>>> as
>>> it is compiled in.
>>>
>>> You could just use "#if defined(CONFIG_SWIOTLB) ||
>>> defined(CONFIG_INTEL_IOMMU)" instead.
>>>
>>> Christian.
>>>
>> Yes, that's what was done before in vmgwfx, but it's very fragile and
>> based on assumptions about what various parts of TTM does and doesn't
>> do. Chances are somebody would modify the enablement and forget to
>> modify this function.
>>
>> But of course I can change it if you think that'd be better.
>>
>> /Thomas
>>
> What about
>
> static inline bool ttm_dma_page_alloc_enabled(void)
> {
> 	return (IS_ENABLED(CONFIG_INTEL_IOMMU) ||
> IS_ENABLED(CONFIG_SWIOTLB)) && _manager;
> }
>
> to avoid that layering violation?

If you drop the "&& _manager" check and move it into the header then 
that should work.

But we could as well really clean it up and add a hidden 
CONFIG_DRM_TTM_DMA_PAGE_ALLOC and remove all the references to 
CONFIG_INTEL_IOMMU and CONFIG_SWIOTLB from the code.

Christian.

>
> /Thomas
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled
  2019-01-25 18:22   ` Sam Ravnborg
@ 2019-01-30  8:37     ` Thomas Hellstrom
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Hellstrom @ 2019-01-30  8:37 UTC (permalink / raw
  To: Sam Ravnborg, Thomas Hellstrom; +Cc: linux-graphics-maintainer, dri-devel

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-01-30  8:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-25 13:05 [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Thomas Hellstrom
2019-01-25 13:05 ` [PATCH 2/2] drm/vmwgfx: Use ttm_dma_page_alloc_enabled Thomas Hellstrom
2019-01-25 18:22   ` Sam Ravnborg
2019-01-30  8:37     ` Thomas Hellstrom
2019-01-28 12:21 ` [PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled Koenig, Christian
2019-01-28 13:29   ` Thomas Hellstrom
2019-01-28 14:21     ` Thomas Hellstrom
2019-01-28 15:56       ` Koenig, Christian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.