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.brezil...@collabora.com>
>
> Reviewed-by: Steven Price <steven.pr...@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;
>>
>> /**
>
