Re: [PATCH] drm/panfrost: Split io-pgtable requests properly
On 08/11/2022 17:06, Robin Murphy wrote: > Although we don't use 1GB block mappings, we still need to split > map/unmap requests at 1GB boundaries to match what io-pgtable expects. > Fix that, and add some explanation to make sense of it all. > > Fixes: 3740b081795a ("drm/panfrost: Update io-pgtable API") > Reported-by: Dmitry Osipenko > Signed-off-by: Robin Murphy Reviewed-by: Steven Price I'll push to drm-misc-fixes. Thanks, Steve > --- > The previous diff turned out to be not quite right, so I've not > included Dmitry's Tested-by given for that. > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index e246d914e7f6..4e83a1891f3e 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -250,13 +250,22 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev) > > static size_t get_pgsize(u64 addr, size_t size, size_t *count) > { > + /* > + * io-pgtable only operates on multiple pages within a single table > + * entry, so we need to split at boundaries of the table size, i.e. > + * the next block size up. The distance from address A to the next > + * boundary of block size B is logically B - A % B, but in unsigned > + * two's complement where B is a power of two we get the equivalence > + * B - A % B == (B - A) % B == (n * B - A) % B, and choose n = 0 :) > + */ > size_t blk_offset = -addr % SZ_2M; > > if (blk_offset || size < SZ_2M) { > *count = min_not_zero(blk_offset, size) / SZ_4K; > return SZ_4K; > } > - *count = size / SZ_2M; > + blk_offset = -addr % SZ_1G ?: SZ_1G; > + *count = min(blk_offset, size) / SZ_2M; > return SZ_2M; > } >
Re: [PATCH] drm/panfrost: Split io-pgtable requests properly
On 11/8/22 20:06, Robin Murphy wrote: > Although we don't use 1GB block mappings, we still need to split > map/unmap requests at 1GB boundaries to match what io-pgtable expects. > Fix that, and add some explanation to make sense of it all. > > Fixes: 3740b081795a ("drm/panfrost: Update io-pgtable API") > Reported-by: Dmitry Osipenko > Signed-off-by: Robin Murphy > --- > The previous diff turned out to be not quite right, so I've not > included Dmitry's Tested-by given for that. > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index e246d914e7f6..4e83a1891f3e 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -250,13 +250,22 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev) > > static size_t get_pgsize(u64 addr, size_t size, size_t *count) > { > + /* > + * io-pgtable only operates on multiple pages within a single table > + * entry, so we need to split at boundaries of the table size, i.e. > + * the next block size up. The distance from address A to the next > + * boundary of block size B is logically B - A % B, but in unsigned > + * two's complement where B is a power of two we get the equivalence > + * B - A % B == (B - A) % B == (n * B - A) % B, and choose n = 0 :) > + */ > size_t blk_offset = -addr % SZ_2M; > > if (blk_offset || size < SZ_2M) { > *count = min_not_zero(blk_offset, size) / SZ_4K; > return SZ_4K; > } > - *count = size / SZ_2M; > + blk_offset = -addr % SZ_1G ?: SZ_1G; > + *count = min(blk_offset, size) / SZ_2M; > return SZ_2M; > } > This variant also works fine, thanks! Tested-by: Dmitry Osipenko -- Best regards, Dmitry
[PATCH] drm/panfrost: Split io-pgtable requests properly
Although we don't use 1GB block mappings, we still need to split map/unmap requests at 1GB boundaries to match what io-pgtable expects. Fix that, and add some explanation to make sense of it all. Fixes: 3740b081795a ("drm/panfrost: Update io-pgtable API") Reported-by: Dmitry Osipenko Signed-off-by: Robin Murphy --- The previous diff turned out to be not quite right, so I've not included Dmitry's Tested-by given for that. --- drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index e246d914e7f6..4e83a1891f3e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -250,13 +250,22 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev) static size_t get_pgsize(u64 addr, size_t size, size_t *count) { + /* +* io-pgtable only operates on multiple pages within a single table +* entry, so we need to split at boundaries of the table size, i.e. +* the next block size up. The distance from address A to the next +* boundary of block size B is logically B - A % B, but in unsigned +* two's complement where B is a power of two we get the equivalence +* B - A % B == (B - A) % B == (n * B - A) % B, and choose n = 0 :) +*/ size_t blk_offset = -addr % SZ_2M; if (blk_offset || size < SZ_2M) { *count = min_not_zero(blk_offset, size) / SZ_4K; return SZ_4K; } - *count = size / SZ_2M; + blk_offset = -addr % SZ_1G ?: SZ_1G; + *count = min(blk_offset, size) / SZ_2M; return SZ_2M; } -- 2.36.1.dirty