[Intel-gfx] i915 and swiotlb_max_segment

[Tvrtko Ursulin]

Hi,

On 03/06/2021 09:40, Joonas Lahtinen wrote:
> + Tvrtko to take a look
> 
> Quoting Konrad Rzeszutek Wilk (2021-05-20 18:12:58)
>> On Mon, May 10, 2021 at 05:25:25PM +0200, Christoph Hellwig wrote:
>>> Hi all,
>>>
>>> swiotlb_max_segment is a rather strange "API" export by swiotlb.c,
>>> and i915 is the only (remaining) user.
>>>
>>> swiotlb_max_segment returns 0 if swiotlb is not in use, 1 if
>>> SWIOTLB_FORCE is set or swiotlb-zen is set, and the swiotlb segment
>>> size when swiotlb is otherwise enabled.
>>>
>>> i915 then uses it to:
>>>
>>>   a) decided on the max order in i915_gem_object_get_pages_internal
>>>   b) decide on a max segment size in i915_sg_segment_size
>>>
>>> for a) it really seems i915 should switch to dma_alloc_noncoherent
>>> or dma_alloc_noncontigous ASAP instead of using alloc_page and
>>> streaming DMA mappings.  Any chance I could trick one of the i915
>>> maintaines into doing just that given that the callchain is not
>>> exactly trivial?
>>>
>>> For b) I'm not sure swiotlb and i915 really agree on the meaning
>>> of the value.  swiotlb_set_max_segment basically returns the entire
>>> size of the swiotlb buffer, while i915 seems to use it to limit
>>> the size each scatterlist entry.  It seems like dma_max_mapping_size
>>> might be the best value to use here.
>>
>> Yes. The background behind that was SWIOTLB would fail because well, the
>> size of the sg was too large. And some way to limit it to max size
>> was needed - the dma_max_mapping_size "should" be just fine.

Can't say I am 100% at home here but what I remember is that the limiting factor was maximum size of a sg segment and not total size of the mapping.

Looking at the code today, if we would replace usage swiotlb_max_segment() with dma_max_mapping_size(), I don't see that would work when we call dma_map_sg_attrs().

Because AFAICT code can end up in dma_direct_max_mapping_size() (not sure when the ops->map_sg path is active and where to trace that) where we have:

size_t dma_direct_max_mapping_size(struct device *dev)
{
	/* If SWIOTLB is active, use its maximum mapping size */
	if (is_swiotlb_active() &&
	    (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
		return swiotlb_max_mapping_size(dev);
	return SIZE_MAX;
}

So for all swiotlb cases, including force, we get:

size_t swiotlb_max_mapping_size(struct device *dev)
{
	return ((size_t)IO_TLB_SIZE) * IO_TLB_SEGSIZE;
}

Which is fixed and doesn't align with swiotlb_max_segment(). But you guys are the experts here so please feel to correct me.

Regards,

Tvrtko