| Message ID | 20190125130548.3266-1-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 |
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
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
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
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 >
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
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(+)