| Message ID | 20240430112852.486424-4-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panthor: Collection of tiler heap related fixes | expand |
Hi Boris, On 30.04.2024 13:28, Boris Brezillon wrote: > The field used to store the chunk size if 12 bits wide, and the encoding > is chunk_size = chunk_header.chunk_size << 12, which gives us a > theoretical [4k:8M] range. This range is further limited by > implementation constraints, and all known implementations seem to > impose a [128k:8M] range, so do the same here. > > We also relax the power-of-two constraint, which doesn't seem to > exist on v10. This will allow userspace to fine-tune initial/max > tiler memory on memory-constrained devices. > > v2: > - Turn the power-of-two constraint into a page-aligned constraint to allow > fine-tune of the initial/max heap memory size > - Fix the panthor_heap_create() kerneldoc > > Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panthor/panthor_heap.c | 8 ++++---- > include/uapi/drm/panthor_drm.h | 6 +++++- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c > index 3be86ec383d6..683bb94761bc 100644 > --- a/drivers/gpu/drm/panthor/panthor_heap.c > +++ b/drivers/gpu/drm/panthor/panthor_heap.c > @@ -253,8 +253,8 @@ int panthor_heap_destroy(struct panthor_heap_pool *pool, u32 handle) > * @pool: Pool to instantiate the heap context from. > * @initial_chunk_count: Number of chunk allocated at initialization time. > * Must be at least 1. > - * @chunk_size: The size of each chunk. Must be a power of two between 256k > - * and 2M. > + * @chunk_size: The size of each chunk. Must be page-aligned and lie in the > + * [128k:2M] range. Probably a typo, but I guess this should be [128k:8M] ? > * @max_chunks: Maximum number of chunks that can be allocated. > * @target_in_flight: Maximum number of in-flight render passes. > * @heap_ctx_gpu_va: Pointer holding the GPU address of the allocated heap > @@ -284,8 +284,8 @@ int panthor_heap_create(struct panthor_heap_pool *pool, > if (initial_chunk_count > max_chunks) > return -EINVAL; > > - if (hweight32(chunk_size) != 1 || > - chunk_size < SZ_256K || chunk_size > SZ_2M) > + if (!IS_ALIGNED(chunk_size, PAGE_SIZE) || > + chunk_size < SZ_128K || chunk_size > SZ_8M) > return -EINVAL; > > down_read(&pool->lock); > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > index 5db80a0682d5..b8220d2e698f 100644 > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h > @@ -898,7 +898,11 @@ struct drm_panthor_tiler_heap_create { > /** @initial_chunk_count: Initial number of chunks to allocate. Must be at least one. */ > __u32 initial_chunk_count; > > - /** @chunk_size: Chunk size. Must be a power of two at least 256KB large. */ > + /** > + * @chunk_size: Chunk size. > + * > + * Must be page-aligned and lie in the [128k:8M] range. > + */ > __u32 chunk_size; > > /** > -- > 2.44.0 Adrian Larumbe
On Tue, Apr 30, 2024 at 01:28:51PM +0200, Boris Brezillon wrote: > The field used to store the chunk size if 12 bits wide, and the encoding > is chunk_size = chunk_header.chunk_size << 12, which gives us a > theoretical [4k:8M] range. This range is further limited by > implementation constraints, and all known implementations seem to > impose a [128k:8M] range, so do the same here. > > We also relax the power-of-two constraint, which doesn't seem to > exist on v10. This will allow userspace to fine-tune initial/max > tiler memory on memory-constrained devices. > > v2: > - Turn the power-of-two constraint into a page-aligned constraint to allow > fine-tune of the initial/max heap memory size > - Fix the panthor_heap_create() kerneldoc > > Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> With the typo that Adrián mentioned fixed, Reviewed-by: Liviu Dudau <liviu.dudau@arm.com> Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_heap.c | 8 ++++---- > include/uapi/drm/panthor_drm.h | 6 +++++- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c > index 3be86ec383d6..683bb94761bc 100644 > --- a/drivers/gpu/drm/panthor/panthor_heap.c > +++ b/drivers/gpu/drm/panthor/panthor_heap.c > @@ -253,8 +253,8 @@ int panthor_heap_destroy(struct panthor_heap_pool *pool, u32 handle) > * @pool: Pool to instantiate the heap context from. > * @initial_chunk_count: Number of chunk allocated at initialization time. > * Must be at least 1. > - * @chunk_size: The size of each chunk. Must be a power of two between 256k > - * and 2M. > + * @chunk_size: The size of each chunk. Must be page-aligned and lie in the > + * [128k:2M] range. > * @max_chunks: Maximum number of chunks that can be allocated. > * @target_in_flight: Maximum number of in-flight render passes. > * @heap_ctx_gpu_va: Pointer holding the GPU address of the allocated heap > @@ -284,8 +284,8 @@ int panthor_heap_create(struct panthor_heap_pool *pool, > if (initial_chunk_count > max_chunks) > return -EINVAL; > > - if (hweight32(chunk_size) != 1 || > - chunk_size < SZ_256K || chunk_size > SZ_2M) > + if (!IS_ALIGNED(chunk_size, PAGE_SIZE) || > + chunk_size < SZ_128K || chunk_size > SZ_8M) > return -EINVAL; > > down_read(&pool->lock); > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > index 5db80a0682d5..b8220d2e698f 100644 > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h > @@ -898,7 +898,11 @@ struct drm_panthor_tiler_heap_create { > /** @initial_chunk_count: Initial number of chunks to allocate. Must be at least one. */ > __u32 initial_chunk_count; > > - /** @chunk_size: Chunk size. Must be a power of two at least 256KB large. */ > + /** > + * @chunk_size: Chunk size. > + * > + * Must be page-aligned and lie in the [128k:8M] range. > + */ > __u32 chunk_size; > > /** > -- > 2.44.0 >
On 30/04/2024 14:08, Adrián Larumbe wrote: > Hi Boris, > > On 30.04.2024 13:28, Boris Brezillon wrote: >> The field used to store the chunk size if 12 bits wide, and the encoding >> is chunk_size = chunk_header.chunk_size << 12, which gives us a >> theoretical [4k:8M] range. This range is further limited by >> implementation constraints, and all known implementations seem to >> impose a [128k:8M] range, so do the same here. >> >> We also relax the power-of-two constraint, which doesn't seem to >> exist on v10. This will allow userspace to fine-tune initial/max >> tiler memory on memory-constrained devices. >> >> v2: >> - Turn the power-of-two constraint into a page-aligned constraint to allow >> fine-tune of the initial/max heap memory size >> - Fix the panthor_heap_create() kerneldoc >> >> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") >> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Other than the typo Adrián pointed out below... Reviewed-by: Steven Price <steven.price@arm.com> >> --- >> drivers/gpu/drm/panthor/panthor_heap.c | 8 ++++---- >> include/uapi/drm/panthor_drm.h | 6 +++++- >> 2 files changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c >> index 3be86ec383d6..683bb94761bc 100644 >> --- a/drivers/gpu/drm/panthor/panthor_heap.c >> +++ b/drivers/gpu/drm/panthor/panthor_heap.c >> @@ -253,8 +253,8 @@ int panthor_heap_destroy(struct panthor_heap_pool *pool, u32 handle) >> * @pool: Pool to instantiate the heap context from. >> * @initial_chunk_count: Number of chunk allocated at initialization time. >> * Must be at least 1. >> - * @chunk_size: The size of each chunk. Must be a power of two between 256k >> - * and 2M. >> + * @chunk_size: The size of each chunk. Must be page-aligned and lie in the >> + * [128k:2M] range. > > Probably a typo, but I guess this should be [128k:8M] ? > >> * @max_chunks: Maximum number of chunks that can be allocated. >> * @target_in_flight: Maximum number of in-flight render passes. >> * @heap_ctx_gpu_va: Pointer holding the GPU address of the allocated heap >> @@ -284,8 +284,8 @@ int panthor_heap_create(struct panthor_heap_pool *pool, >> if (initial_chunk_count > max_chunks) >> return -EINVAL; >> >> - if (hweight32(chunk_size) != 1 || >> - chunk_size < SZ_256K || chunk_size > SZ_2M) >> + if (!IS_ALIGNED(chunk_size, PAGE_SIZE) || >> + chunk_size < SZ_128K || chunk_size > SZ_8M) >> return -EINVAL; >> >> down_read(&pool->lock); >> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h >> index 5db80a0682d5..b8220d2e698f 100644 >> --- a/include/uapi/drm/panthor_drm.h >> +++ b/include/uapi/drm/panthor_drm.h >> @@ -898,7 +898,11 @@ struct drm_panthor_tiler_heap_create { >> /** @initial_chunk_count: Initial number of chunks to allocate. Must be at least one. */ >> __u32 initial_chunk_count; >> >> - /** @chunk_size: Chunk size. Must be a power of two at least 256KB large. */ >> + /** >> + * @chunk_size: Chunk size. >> + * >> + * Must be page-aligned and lie in the [128k:8M] range. >> + */ >> __u32 chunk_size; >> >> /** >> -- >> 2.44.0 > > > Adrian Larumbe
diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c index 3be86ec383d6..683bb94761bc 100644 --- a/drivers/gpu/drm/panthor/panthor_heap.c +++ b/drivers/gpu/drm/panthor/panthor_heap.c @@ -253,8 +253,8 @@ int panthor_heap_destroy(struct panthor_heap_pool *pool, u32 handle) * @pool: Pool to instantiate the heap context from. * @initial_chunk_count: Number of chunk allocated at initialization time. * Must be at least 1. - * @chunk_size: The size of each chunk. Must be a power of two between 256k - * and 2M. + * @chunk_size: The size of each chunk. Must be page-aligned and lie in the + * [128k:2M] range. * @max_chunks: Maximum number of chunks that can be allocated. * @target_in_flight: Maximum number of in-flight render passes. * @heap_ctx_gpu_va: Pointer holding the GPU address of the allocated heap @@ -284,8 +284,8 @@ int panthor_heap_create(struct panthor_heap_pool *pool, if (initial_chunk_count > max_chunks) return -EINVAL; - if (hweight32(chunk_size) != 1 || - chunk_size < SZ_256K || chunk_size > SZ_2M) + if (!IS_ALIGNED(chunk_size, PAGE_SIZE) || + chunk_size < SZ_128K || chunk_size > SZ_8M) return -EINVAL; down_read(&pool->lock); diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h index 5db80a0682d5..b8220d2e698f 100644 --- a/include/uapi/drm/panthor_drm.h +++ b/include/uapi/drm/panthor_drm.h @@ -898,7 +898,11 @@ struct drm_panthor_tiler_heap_create { /** @initial_chunk_count: Initial number of chunks to allocate. Must be at least one. */ __u32 initial_chunk_count; - /** @chunk_size: Chunk size. Must be a power of two at least 256KB large. */ + /** + * @chunk_size: Chunk size. + * + * Must be page-aligned and lie in the [128k:8M] range. + */ __u32 chunk_size; /**
The field used to store the chunk size if 12 bits wide, and the encoding is chunk_size = chunk_header.chunk_size << 12, which gives us a theoretical [4k:8M] range. This range is further limited by implementation constraints, and all known implementations seem to impose a [128k:8M] range, so do the same here. We also relax the power-of-two constraint, which doesn't seem to exist on v10. This will allow userspace to fine-tune initial/max tiler memory on memory-constrained devices. v2: - Turn the power-of-two constraint into a page-aligned constraint to allow fine-tune of the initial/max heap memory size - Fix the panthor_heap_create() kerneldoc Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/gpu/drm/panthor/panthor_heap.c | 8 ++++---- include/uapi/drm/panthor_drm.h | 6 +++++- 2 files changed, 9 insertions(+), 5 deletions(-)