* i915 and swiotlb_max_segment @ 2021-05-10 15:25 ` Christoph Hellwig 0 siblings, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2021-05-10 15:25 UTC (permalink / raw) To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi Cc: intel-gfx, iommu, dri-devel, Daniel Vetter, Konrad Rzeszutek Wilk 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. Once that is fixed I'd like to kill off swiotlb_max_segment as it is a horribly confusing API. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] i915 and swiotlb_max_segment @ 2021-05-10 15:25 ` Christoph Hellwig 0 siblings, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2021-05-10 15:25 UTC (permalink / raw) To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi Cc: intel-gfx, iommu, dri-devel, Konrad Rzeszutek Wilk 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. Once that is fixed I'd like to kill off swiotlb_max_segment as it is a horribly confusing API. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: i915 and swiotlb_max_segment 2021-05-10 15:25 ` [Intel-gfx] " Christoph Hellwig (?) @ 2021-05-20 15:12 ` Konrad Rzeszutek Wilk -1 siblings, 0 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2021-05-20 15:12 UTC (permalink / raw) To: Christoph Hellwig Cc: intel-gfx, Joonas Lahtinen, dri-devel, iommu, Jani Nikula, Daniel Vetter, Rodrigo Vivi 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. > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > a horribly confusing API. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-05-20 15:12 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2021-05-20 15:12 UTC (permalink / raw) To: Christoph Hellwig; +Cc: intel-gfx, dri-devel, iommu 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. > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > a horribly confusing API. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: i915 and swiotlb_max_segment @ 2021-05-20 15:12 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2021-05-20 15:12 UTC (permalink / raw) To: Christoph Hellwig; +Cc: intel-gfx, dri-devel, iommu, Rodrigo Vivi 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. > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > a horribly confusing API. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: i915 and swiotlb_max_segment 2021-05-20 15:12 ` Konrad Rzeszutek Wilk (?) @ 2021-06-03 8:40 ` Joonas Lahtinen -1 siblings, 0 replies; 14+ messages in thread From: Joonas Lahtinen @ 2021-06-03 8:40 UTC (permalink / raw) To: Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, dri-devel, iommu, Jani Nikula, Daniel Vetter, Rodrigo Vivi + 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. > > > > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > > a horribly confusing API. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-06-03 8:40 ` Joonas Lahtinen 0 siblings, 0 replies; 14+ messages in thread From: Joonas Lahtinen @ 2021-06-03 8:40 UTC (permalink / raw) To: Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, dri-devel, iommu + 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. > > > > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > > a horribly confusing API. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: i915 and swiotlb_max_segment @ 2021-06-03 8:40 ` Joonas Lahtinen 0 siblings, 0 replies; 14+ messages in thread From: Joonas Lahtinen @ 2021-06-03 8:40 UTC (permalink / raw) To: Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, dri-devel, iommu, Rodrigo Vivi + 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. > > > > > Once that is fixed I'd like to kill off swiotlb_max_segment as it is > > a horribly confusing API. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment 2021-06-03 8:40 ` Joonas Lahtinen (?) @ 2021-06-03 9:17 ` Tvrtko Ursulin -1 siblings, 0 replies; 14+ messages in thread From: Tvrtko Ursulin @ 2021-06-03 9:17 UTC (permalink / raw) To: Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel 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 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-06-03 9:17 ` Tvrtko Ursulin 0 siblings, 0 replies; 14+ messages in thread From: Tvrtko Ursulin @ 2021-06-03 9:17 UTC (permalink / raw) To: Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-06-03 9:17 ` Tvrtko Ursulin 0 siblings, 0 replies; 14+ messages in thread From: Tvrtko Ursulin @ 2021-06-03 9:17 UTC (permalink / raw) To: Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment 2021-06-03 9:17 ` Tvrtko Ursulin (?) @ 2021-06-03 10:54 ` Robin Murphy -1 siblings, 0 replies; 14+ messages in thread From: Robin Murphy @ 2021-06-03 10:54 UTC (permalink / raw) To: Tvrtko Ursulin, Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel On 2021-06-03 10:17, Tvrtko Ursulin wrote: > > 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. But swiotlb_max_segment is also effectively fixed for a given system coinfigration, at either a page (under certain circumstances), or a value considerably larger than what the longest mappable SG segment actually is. Neither seems particularly useful, and to be honest I suspect the forced-bounce cases only set it to a page as a sledgehammer to make things work *because* the "normal" value is nonsense. Robin. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-06-03 10:54 ` Robin Murphy 0 siblings, 0 replies; 14+ messages in thread From: Robin Murphy @ 2021-06-03 10:54 UTC (permalink / raw) To: Tvrtko Ursulin, Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel On 2021-06-03 10:17, Tvrtko Ursulin wrote: > > 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. But swiotlb_max_segment is also effectively fixed for a given system coinfigration, at either a page (under certain circumstances), or a value considerably larger than what the longest mappable SG segment actually is. Neither seems particularly useful, and to be honest I suspect the forced-bounce cases only set it to a page as a sledgehammer to make things work *because* the "normal" value is nonsense. Robin. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] i915 and swiotlb_max_segment @ 2021-06-03 10:54 ` Robin Murphy 0 siblings, 0 replies; 14+ messages in thread From: Robin Murphy @ 2021-06-03 10:54 UTC (permalink / raw) To: Tvrtko Ursulin, Joonas Lahtinen, Christoph Hellwig, Konrad Rzeszutek Wilk, Tvrtko Ursulin Cc: intel-gfx, iommu, dri-devel On 2021-06-03 10:17, Tvrtko Ursulin wrote: > > 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. But swiotlb_max_segment is also effectively fixed for a given system coinfigration, at either a page (under certain circumstances), or a value considerably larger than what the longest mappable SG segment actually is. Neither seems particularly useful, and to be honest I suspect the forced-bounce cases only set it to a page as a sledgehammer to make things work *because* the "normal" value is nonsense. Robin. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-06-03 13:32 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-10 15:25 i915 and swiotlb_max_segment Christoph Hellwig 2021-05-10 15:25 ` [Intel-gfx] " Christoph Hellwig 2021-05-20 15:12 ` Konrad Rzeszutek Wilk 2021-05-20 15:12 ` [Intel-gfx] " Konrad Rzeszutek Wilk 2021-05-20 15:12 ` Konrad Rzeszutek Wilk 2021-06-03 8:40 ` Joonas Lahtinen 2021-06-03 8:40 ` [Intel-gfx] " Joonas Lahtinen 2021-06-03 8:40 ` Joonas Lahtinen 2021-06-03 9:17 ` [Intel-gfx] " Tvrtko Ursulin 2021-06-03 9:17 ` Tvrtko Ursulin 2021-06-03 9:17 ` Tvrtko Ursulin 2021-06-03 10:54 ` Robin Murphy 2021-06-03 10:54 ` Robin Murphy 2021-06-03 10:54 ` Robin Murphy
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.