| Message ID | 20240425071837.529039-3-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panthor: Collection of tiler heap related fixes | expand |
On 25/04/2024 08:18, Boris Brezillon wrote: > It doesn't make sense to have a maximum number of chunks smaller than > the initial number of chunks attached to the context. > > Fix the uAPI header to reflect the new constraint, and mention the > undocumented "initial_chunk_count > 0" constraint while at it. > > Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panthor/panthor_heap.c | 3 +++ > include/uapi/drm/panthor_drm.h | 8 ++++++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c > index 143fa35f2e74..8728c9bb76e4 100644 > --- a/drivers/gpu/drm/panthor/panthor_heap.c > +++ b/drivers/gpu/drm/panthor/panthor_heap.c > @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool, > if (initial_chunk_count == 0) > return -EINVAL; > > + if (initial_chunk_count < max_chunks) > + return -EINVAL; > + > if (hweight32(chunk_size) != 1 || > chunk_size < SZ_256K || chunk_size > SZ_2M) > return -EINVAL; > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > index dadb05ab1235..5db80a0682d5 100644 > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h > @@ -895,13 +895,17 @@ struct drm_panthor_tiler_heap_create { > /** @vm_id: VM ID the tiler heap should be mapped to */ > __u32 vm_id; > > - /** @initial_chunk_count: Initial number of chunks to allocate. */ > + /** @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. */ > __u32 chunk_size; > > - /** @max_chunks: Maximum number of chunks that can be allocated. */ > + /** > + * @max_chunks: Maximum number of chunks that can be allocated. > + * > + * Must be at least @initial_chunk_count. > + */ > __u32 max_chunks; > > /**
On 25/04/2024 10:28, Steven Price wrote: > On 25/04/2024 08:18, Boris Brezillon wrote: >> It doesn't make sense to have a maximum number of chunks smaller than >> the initial number of chunks attached to the context. >> >> Fix the uAPI header to reflect the new constraint, and mention the >> undocumented "initial_chunk_count > 0" constraint while at it. >> >> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") >> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > Reviewed-by: Steven Price <steven.price@arm.com> Ok, I'll take that back... I've rebased (and fixed up all the out of tree patches) and this doesn't work when I actually test it! > >> --- >> drivers/gpu/drm/panthor/panthor_heap.c | 3 +++ >> include/uapi/drm/panthor_drm.h | 8 ++++++-- >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c >> index 143fa35f2e74..8728c9bb76e4 100644 >> --- a/drivers/gpu/drm/panthor/panthor_heap.c >> +++ b/drivers/gpu/drm/panthor/panthor_heap.c >> @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool, >> if (initial_chunk_count == 0) >> return -EINVAL; >> >> + if (initial_chunk_count < max_chunks) This should be initial_chunk_count > max_chunks. Otherwise you're requiring the initial chunk count to be equal *or greater* than the max chunks which makes no sense! Steve >> + return -EINVAL; >> + >> if (hweight32(chunk_size) != 1 || >> chunk_size < SZ_256K || chunk_size > SZ_2M) >> return -EINVAL; >> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h >> index dadb05ab1235..5db80a0682d5 100644 >> --- a/include/uapi/drm/panthor_drm.h >> +++ b/include/uapi/drm/panthor_drm.h >> @@ -895,13 +895,17 @@ struct drm_panthor_tiler_heap_create { >> /** @vm_id: VM ID the tiler heap should be mapped to */ >> __u32 vm_id; >> >> - /** @initial_chunk_count: Initial number of chunks to allocate. */ >> + /** @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. */ >> __u32 chunk_size; >> >> - /** @max_chunks: Maximum number of chunks that can be allocated. */ >> + /** >> + * @max_chunks: Maximum number of chunks that can be allocated. >> + * >> + * Must be at least @initial_chunk_count. >> + */ >> __u32 max_chunks; >> >> /** >
On Thu, 25 Apr 2024 11:43:39 +0100 Steven Price <steven.price@arm.com> wrote: > On 25/04/2024 10:28, Steven Price wrote: > > On 25/04/2024 08:18, Boris Brezillon wrote: > >> It doesn't make sense to have a maximum number of chunks smaller than > >> the initial number of chunks attached to the context. > >> > >> Fix the uAPI header to reflect the new constraint, and mention the > >> undocumented "initial_chunk_count > 0" constraint while at it. > >> > >> Fixes: 9cca48fa4f89 ("drm/panthor: Add the heap logical block") > >> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > > > Reviewed-by: Steven Price <steven.price@arm.com> > > Ok, I'll take that back... I've rebased (and fixed up all the out of > tree patches) and this doesn't work when I actually test it! > > > > >> --- > >> drivers/gpu/drm/panthor/panthor_heap.c | 3 +++ > >> include/uapi/drm/panthor_drm.h | 8 ++++++-- > >> 2 files changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c > >> index 143fa35f2e74..8728c9bb76e4 100644 > >> --- a/drivers/gpu/drm/panthor/panthor_heap.c > >> +++ b/drivers/gpu/drm/panthor/panthor_heap.c > >> @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool, > >> if (initial_chunk_count == 0) > >> return -EINVAL; > >> > >> + if (initial_chunk_count < max_chunks) > > This should be initial_chunk_count > max_chunks. Otherwise you're > requiring the initial chunk count to be equal *or greater* than the max > chunks which makes no sense! Damn it, here's what happens when you think your changes are too trivial to be wrong... But I swear I would have tested the whole thing before pushing to drm-misc. :P
diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c index 143fa35f2e74..8728c9bb76e4 100644 --- a/drivers/gpu/drm/panthor/panthor_heap.c +++ b/drivers/gpu/drm/panthor/panthor_heap.c @@ -281,6 +281,9 @@ int panthor_heap_create(struct panthor_heap_pool *pool, if (initial_chunk_count == 0) return -EINVAL; + if (initial_chunk_count < max_chunks) + return -EINVAL; + if (hweight32(chunk_size) != 1 || chunk_size < SZ_256K || chunk_size > SZ_2M) return -EINVAL; diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h index dadb05ab1235..5db80a0682d5 100644 --- a/include/uapi/drm/panthor_drm.h +++ b/include/uapi/drm/panthor_drm.h @@ -895,13 +895,17 @@ struct drm_panthor_tiler_heap_create { /** @vm_id: VM ID the tiler heap should be mapped to */ __u32 vm_id; - /** @initial_chunk_count: Initial number of chunks to allocate. */ + /** @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. */ __u32 chunk_size; - /** @max_chunks: Maximum number of chunks that can be allocated. */ + /** + * @max_chunks: Maximum number of chunks that can be allocated. + * + * Must be at least @initial_chunk_count. + */ __u32 max_chunks; /**
It doesn't make sense to have a maximum number of chunks smaller than the initial number of chunks attached to the context. Fix the uAPI header to reflect the new constraint, and mention the undocumented "initial_chunk_count > 0" constraint while at it. 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 | 3 +++ include/uapi/drm/panthor_drm.h | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-)