On Mon, 2019-02-04 at 09:19 +0100, Christoph Hellwig wrote:

On Fri, Jan 25, 2019 at 09:12:13AM +0100, Thomas Hellstrom wrote:

...

-#if !defined(CONFIG_SWIOTLB) && !defined(CONFIG_INTEL_IOMMU)

• /*

• * No coherent page pool
  

• */
  

• if (dev_priv->map_mode == vmw_dma_alloc_coherent)

• /* No TTM coherent page pool? FIXME: Ask TTM instead! */
• if (!(IS_ENABLED(CONFIG_SWIOTLB) ||

IS_ENABLED(CONFIG_INTEL_IOMMU)) &&

•    (dev_priv->map_mode == vmw_dma_alloc_coherent))
  

  return -EINVAL;

-#endif

I don't think this edited in change makes any sense. The swiotlb vs dma-direct versions of dma_alloc_coherent are the same, so this check seems very obsfucating.

So this part of code is identical in functionality to the previous version. It checks whether the TTM module has the coherent page pool enabled. (an identical test is present in TTM). What we *really* need to do here instead is to ask TTM whether it has enabled its coherent page pool instead of trying to mimic TTM's test, and I have a changeset under review for that. But as mentioned previously, I don't want to change the TTM interface outside of a merge window, so we either have to live with the above for 5.0 or keep the old defines. I'd prefer the former so I don't have to respin the patch series once more.

Thanks, Thoams