Re: [PATCH v10 8/8] media: vsp1: Move video configuration to a cached dlb
Hi Laurent,
On 17/05/18 20:57, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Thursday, 17 May 2018 20:24:01 EEST Kieran Bingham wrote:
>> We are now able to configure a pipeline directly into a local display
>> list body. Take advantage of this fact, and create a cacheable body to
>> store the configuration of the pipeline in the video object.
>>
>> vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
>> Convert this function to use the cached video->config body and obtain a
>> local display list reference.
>>
>> Attach the video->config body to the display list when needed before
>> committing to hardware.
>>
>> The pipe object is marked as un-configured when resuming from a suspend.
>
> Is this comment still valid ?
Nope :D
Updated in latest local patch.
>
>> This ensures that when the hardware is reset - our cached configuration
>> will be re-attached to the next committed DL.
>>
>> Our video DL usage now looks like the below output:
>>
>> dl->body0 contains our disposable runtime configuration. Max 41.
>> dl_child->body0 is our partition specific configuration. Max 12.
>> dl->bodies shows our constant configuration and LUTs.
>>
>> These two are LUT/CLU:
>> * dl->bodies[x]->num_entries 256 / max 256
>> * dl->bodies[x]->num_entries 4914 / max 4914
>>
>> Which shows that our 'constant' configuration cache is currently
>> utilised to a maximum of 64 entries.
>>
>> trace-cmd report | \
>> grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;
>>
>> dl->body0->num_entries 13 / max 128
>> dl->body0->num_entries 14 / max 128
>> dl->body0->num_entries 16 / max 128
>> dl->body0->num_entries 20 / max 128
>> dl->body0->num_entries 27 / max 128
>> dl->body0->num_entries 34 / max 128
>> dl->body0->num_entries 41 / max 128
>> dl_child->body0->num_entries 10 / max 128
>> dl_child->body0->num_entries 12 / max 128
>> dl->bodies[x]->num_entries 15 / max 128
>> dl->bodies[x]->num_entries 16 / max 128
>> dl->bodies[x]->num_entries 17 / max 128
>> dl->bodies[x]->num_entries 18 / max 128
>> dl->bodies[x]->num_entries 20 / max 128
>> dl->bodies[x]->num_entries 21 / max 128
>> dl->bodies[x]->num_entries 256 / max 256
>> dl->bodies[x]->num_entries 31 / max 128
>> dl->bodies[x]->num_entries 32 / max 128
>> dl->bodies[x]->num_entries 39 / max 128
>> dl->bodies[x]->num_entries 40 / max 128
>> dl->bodies[x]->num_entries 47 / max 128
>> dl->bodies[x]->num_entries 48 / max 128
>> dl->bodies[x]->num_entries 4914 / max 4914
>> dl->bodies[x]->num_entries 55 / max 128
>> dl->bodies[x]->num_entries 56 / max 128
>> dl->bodies[x]->num_entries 63 / max 128
>> dl->bodies[x]->num_entries 64 / max 128
>>
>> Signed-off-by: Kieran Bingham
>> ---
>> v10:
>> - Removed pipe->configured flag, and use
>>pipe->state == VSP1_PIPELINE_STOPPED instead.
>>
>> v8:
>> - Fix comments
>> - Rename video->pipe_config -> video->stream_config
>>
>> v4:
>> - Adjust pipe configured flag to be reset on resume rather than suspend
>> - rename dl_child, dl_next
>>
>> v3:
>> - 's/fragment/body/', 's/fragments/bodies/'
>> - video dlb cache allocation increased from 2 to 3 dlbs
>>
>> drivers/media/platform/vsp1/vsp1_pipe.h | 3 +-
>> drivers/media/platform/vsp1/vsp1_video.c | 67 +++--
>> drivers/media/platform/vsp1/vsp1_video.h | 2 +-
>> 3 files changed, 43 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
>> b/drivers/media/platform/vsp1/vsp1_pipe.h index e00010693eef..be6ecab3cbed
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
>> @@ -102,7 +102,6 @@ struct vsp1_partition {
>> * @uds: UDS entity, if present
>> * @uds_input: entity at the input of the UDS, if the UDS is present
>> * @entities: list of entities in the pipeline
>> - * @dl: display list associated with the pipeline
>> * @partitions: The number of partitions used to process one frame
>> * @partition: The current partition for configuration to process
>> * @part_table: The pre-calculated partitions used by the pipeline
>> @@ -139,8 +138,6 @@ struct vsp1_pipeline {
>> */
>> struct list_head entities;
>>
>> -struct vsp1_dl_list *dl;
>> -
>> unsigned int partitions;
>> struct vsp1_partition *partition;
>> struct vsp1_partition *part_table;
>> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
>> b/drivers/media/platform/vsp1/vsp1_video.c index 72f29773eb1c..f2bc26d28396
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_video.c
>> +++ b/drivers/media/platform/vsp1/vsp1_video.c
>> @@ -390,44 +390,48 @@ static void vsp1_video_pipeline_run_partition(struct
>> vsp1_pipeline *pipe, static void vsp1_video_pipeline_run(struct
>> vsp1_pipeline *pipe)
>> {
>> struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
>> +struct vsp1_video *video = pipe->output->video;
>> struct vsp1_entity *entity;
>>
Re: [PATCH v10 8/8] media: vsp1: Move video configuration to a cached dlb
Hi Kieran,
Thank you for the patch.
On Thursday, 17 May 2018 20:24:01 EEST Kieran Bingham wrote:
> We are now able to configure a pipeline directly into a local display
> list body. Take advantage of this fact, and create a cacheable body to
> store the configuration of the pipeline in the video object.
>
> vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
> Convert this function to use the cached video->config body and obtain a
> local display list reference.
>
> Attach the video->config body to the display list when needed before
> committing to hardware.
>
> The pipe object is marked as un-configured when resuming from a suspend.
Is this comment still valid ?
> This ensures that when the hardware is reset - our cached configuration
> will be re-attached to the next committed DL.
>
> Our video DL usage now looks like the below output:
>
> dl->body0 contains our disposable runtime configuration. Max 41.
> dl_child->body0 is our partition specific configuration. Max 12.
> dl->bodies shows our constant configuration and LUTs.
>
> These two are LUT/CLU:
> * dl->bodies[x]->num_entries 256 / max 256
> * dl->bodies[x]->num_entries 4914 / max 4914
>
> Which shows that our 'constant' configuration cache is currently
> utilised to a maximum of 64 entries.
>
> trace-cmd report | \
> grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;
>
> dl->body0->num_entries 13 / max 128
> dl->body0->num_entries 14 / max 128
> dl->body0->num_entries 16 / max 128
> dl->body0->num_entries 20 / max 128
> dl->body0->num_entries 27 / max 128
> dl->body0->num_entries 34 / max 128
> dl->body0->num_entries 41 / max 128
> dl_child->body0->num_entries 10 / max 128
> dl_child->body0->num_entries 12 / max 128
> dl->bodies[x]->num_entries 15 / max 128
> dl->bodies[x]->num_entries 16 / max 128
> dl->bodies[x]->num_entries 17 / max 128
> dl->bodies[x]->num_entries 18 / max 128
> dl->bodies[x]->num_entries 20 / max 128
> dl->bodies[x]->num_entries 21 / max 128
> dl->bodies[x]->num_entries 256 / max 256
> dl->bodies[x]->num_entries 31 / max 128
> dl->bodies[x]->num_entries 32 / max 128
> dl->bodies[x]->num_entries 39 / max 128
> dl->bodies[x]->num_entries 40 / max 128
> dl->bodies[x]->num_entries 47 / max 128
> dl->bodies[x]->num_entries 48 / max 128
> dl->bodies[x]->num_entries 4914 / max 4914
> dl->bodies[x]->num_entries 55 / max 128
> dl->bodies[x]->num_entries 56 / max 128
> dl->bodies[x]->num_entries 63 / max 128
> dl->bodies[x]->num_entries 64 / max 128
>
> Signed-off-by: Kieran Bingham
> ---
> v10:
> - Removed pipe->configured flag, and use
>pipe->state == VSP1_PIPELINE_STOPPED instead.
>
> v8:
> - Fix comments
> - Rename video->pipe_config -> video->stream_config
>
> v4:
> - Adjust pipe configured flag to be reset on resume rather than suspend
> - rename dl_child, dl_next
>
> v3:
> - 's/fragment/body/', 's/fragments/bodies/'
> - video dlb cache allocation increased from 2 to 3 dlbs
>
> drivers/media/platform/vsp1/vsp1_pipe.h | 3 +-
> drivers/media/platform/vsp1/vsp1_video.c | 67 +++--
> drivers/media/platform/vsp1/vsp1_video.h | 2 +-
> 3 files changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index e00010693eef..be6ecab3cbed
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -102,7 +102,6 @@ struct vsp1_partition {
> * @uds: UDS entity, if present
> * @uds_input: entity at the input of the UDS, if the UDS is present
> * @entities: list of entities in the pipeline
> - * @dl: display list associated with the pipeline
> * @partitions: The number of partitions used to process one frame
> * @partition: The current partition for configuration to process
> * @part_table: The pre-calculated partitions used by the pipeline
> @@ -139,8 +138,6 @@ struct vsp1_pipeline {
>*/
> struct list_head entities;
>
> - struct vsp1_dl_list *dl;
> -
> unsigned int partitions;
> struct vsp1_partition *partition;
> struct vsp1_partition *part_table;
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index 72f29773eb1c..f2bc26d28396
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -390,44 +390,48 @@ static void vsp1_video_pipeline_run_partition(struct
> vsp1_pipeline *pipe, static void vsp1_video_pipeline_run(struct
> vsp1_pipeline *pipe)
> {
> struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
> + struct vsp1_video *video = pipe->output->video;
> struct vsp1_entity *entity;
> struct vsp1_dl_body *dlb;
> + struct vsp1_dl_list *dl;
> unsigned int partition;
>
> - if (!pipe->dl)
> - pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
> + dl = vsp1_dl_li
[PATCH v10 8/8] media: vsp1: Move video configuration to a cached dlb
We are now able to configure a pipeline directly into a local display
list body. Take advantage of this fact, and create a cacheable body to
store the configuration of the pipeline in the video object.
vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
Convert this function to use the cached video->config body and obtain a
local display list reference.
Attach the video->config body to the display list when needed before
committing to hardware.
The pipe object is marked as un-configured when resuming from a suspend.
This ensures that when the hardware is reset - our cached configuration
will be re-attached to the next committed DL.
Our video DL usage now looks like the below output:
dl->body0 contains our disposable runtime configuration. Max 41.
dl_child->body0 is our partition specific configuration. Max 12.
dl->bodies shows our constant configuration and LUTs.
These two are LUT/CLU:
* dl->bodies[x]->num_entries 256 / max 256
* dl->bodies[x]->num_entries 4914 / max 4914
Which shows that our 'constant' configuration cache is currently
utilised to a maximum of 64 entries.
trace-cmd report | \
grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;
dl->body0->num_entries 13 / max 128
dl->body0->num_entries 14 / max 128
dl->body0->num_entries 16 / max 128
dl->body0->num_entries 20 / max 128
dl->body0->num_entries 27 / max 128
dl->body0->num_entries 34 / max 128
dl->body0->num_entries 41 / max 128
dl_child->body0->num_entries 10 / max 128
dl_child->body0->num_entries 12 / max 128
dl->bodies[x]->num_entries 15 / max 128
dl->bodies[x]->num_entries 16 / max 128
dl->bodies[x]->num_entries 17 / max 128
dl->bodies[x]->num_entries 18 / max 128
dl->bodies[x]->num_entries 20 / max 128
dl->bodies[x]->num_entries 21 / max 128
dl->bodies[x]->num_entries 256 / max 256
dl->bodies[x]->num_entries 31 / max 128
dl->bodies[x]->num_entries 32 / max 128
dl->bodies[x]->num_entries 39 / max 128
dl->bodies[x]->num_entries 40 / max 128
dl->bodies[x]->num_entries 47 / max 128
dl->bodies[x]->num_entries 48 / max 128
dl->bodies[x]->num_entries 4914 / max 4914
dl->bodies[x]->num_entries 55 / max 128
dl->bodies[x]->num_entries 56 / max 128
dl->bodies[x]->num_entries 63 / max 128
dl->bodies[x]->num_entries 64 / max 128
Signed-off-by: Kieran Bingham
---
v10:
- Removed pipe->configured flag, and use
pipe->state == VSP1_PIPELINE_STOPPED instead.
v8:
- Fix comments
- Rename video->pipe_config -> video->stream_config
v4:
- Adjust pipe configured flag to be reset on resume rather than suspend
- rename dl_child, dl_next
v3:
- 's/fragment/body/', 's/fragments/bodies/'
- video dlb cache allocation increased from 2 to 3 dlbs
drivers/media/platform/vsp1/vsp1_pipe.h | 3 +-
drivers/media/platform/vsp1/vsp1_video.c | 67 +++--
drivers/media/platform/vsp1/vsp1_video.h | 2 +-
3 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
b/drivers/media/platform/vsp1/vsp1_pipe.h
index e00010693eef..be6ecab3cbed 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -102,7 +102,6 @@ struct vsp1_partition {
* @uds: UDS entity, if present
* @uds_input: entity at the input of the UDS, if the UDS is present
* @entities: list of entities in the pipeline
- * @dl: display list associated with the pipeline
* @partitions: The number of partitions used to process one frame
* @partition: The current partition for configuration to process
* @part_table: The pre-calculated partitions used by the pipeline
@@ -139,8 +138,6 @@ struct vsp1_pipeline {
*/
struct list_head entities;
- struct vsp1_dl_list *dl;
-
unsigned int partitions;
struct vsp1_partition *partition;
struct vsp1_partition *part_table;
diff --git a/drivers/media/platform/vsp1/vsp1_video.c
b/drivers/media/platform/vsp1/vsp1_video.c
index 72f29773eb1c..f2bc26d28396 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -390,44 +390,48 @@ static void vsp1_video_pipeline_run_partition(struct
vsp1_pipeline *pipe,
static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
{
struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ struct vsp1_video *video = pipe->output->video;
struct vsp1_entity *entity;
struct vsp1_dl_body *dlb;
+ struct vsp1_dl_list *dl;
unsigned int partition;
- if (!pipe->dl)
- pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
+ dl = vsp1_dl_list_get(pipe->output->dlm);
+
+ /* Attach our pipe configuration to fully initialise the hardware. */
+ if (pipe->state == VSP1_PIPELINE_STOPPED)
+ vsp1_dl_list_add_body(dl, video->stream_config);
- dlb = vsp1_dl_list_get_body0(pipe->dl);
+ dlb = vsp1_dl_list_get_body0(dl);
list_for_each_ent
