[PATCH 1/2] drm/ttm: Implement and export ttm_dma_page_alloc_enabled

[Koenig, Christian]

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 at amd.com>
>>>> Signed-off-by: Thomas Hellstrom <thellstrom at 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
>