diff mbox

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

Message ID fa078611769415d7adbad208f1299d05bee3bda8.1502723341.git-series.kieran.bingham+renesas@ideasonboard.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Kieran Bingham Aug. 14, 2017, 3:13 p.m. UTC
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.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart Aug. 17, 2017, 4:11 p.m. UTC | #1
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 <kieran.bingham+renesas@ideasonboard.com>
> ---
>  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)
Kieran Bingham Sept. 11, 2017, 9:15 p.m. UTC | #2
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 <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  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)
>
diff mbox

Patch

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)