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