Re: [PATCH v2 8/8] v4l: vsp1: Reduce display list body size

[Kieran Bingham]

On 17/08/17 17:11, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Monday 14 Aug 2017 16:13:31 Kieran Bingham wrote:
>> The display list originally allocated a body of 256 entries to store all
>> of the register lists required for each frame.
>>
>> This has now been separated into fragments for constant stream setup, and
>> runtime updates.
>>
>> Empirical testing shows that the body0 now uses a maximum of 41
>> registers for each frame, for both DRM and Video API pipelines thus a
>> rounded 64 entries provides a suitable allocation.
> 
> Didn't you mention in patch 7/8 that one of the fragments uses exactly 64 
> entries ? Which one is it, and is there a risk it could use more ? 

No, that referred to the fragments(bodies) which had been attached. This change
refers only to the body0 allocation which has a maximum of 41 entries written.

The fragment and partition allocations which reach 64 entries, are allocated
with room for 128 currently...

< yes, this can be revisited >

>> Signed-off-by: Kieran Bingham 
>> ---
>>  drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index 176a258146ac..b3f5eb2f9a4f
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -21,7 +21,7 @@
>>  #include "vsp1.h"
>>  #include "vsp1_dl.h"
>>
>> -#define VSP1_DL_NUM_ENTRIES 256
>> +#define VSP1_DL_NUM_ENTRIES 64

This now only defines the size of the body0 which is the defacto list of entries
in a display list.

This too could / should be removed at somepoint I believe, leaving allocations
only where they are needed.
>>  #define VSP1_DLH_INT_ENABLE (1 << 1)
>>  #define VSP1_DLH_AUTO_START (1 << 0)
> 

Re: [PATCH v2 8/8] v4l: vsp1: Reduce display list body size

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:31 Kieran Bingham wrote:
> The display list originally allocated a body of 256 entries to store all
> of the register lists required for each frame.
> 
> This has now been separated into fragments for constant stream setup, and
> runtime updates.
> 
> Empirical testing shows that the body0 now uses a maximum of 41
> registers for each frame, for both DRM and Video API pipelines thus a
> rounded 64 entries provides a suitable allocation.

Didn't you mention in patch 7/8 that one of the fragments uses exactly 64 
entries ? Which one is it, and is there a risk it could use more ? 

> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 176a258146ac..b3f5eb2f9a4f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -21,7 +21,7 @@
>  #include "vsp1.h"
>  #include "vsp1_dl.h"
> 
> -#define VSP1_DL_NUM_ENTRIES  256
> +#define VSP1_DL_NUM_ENTRIES  64
> 
>  #define VSP1_DLH_INT_ENABLE  (1 << 1)
>  #define VSP1_DLH_AUTO_START  (1 << 0)

-- 
Regards,

Laurent Pinchart