subject:"\[PATCH v7 8\/8\] media\: vsp1\: Move video configuration to a cached dlb"

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-05-17 Thread Laurent Pinchart

Hi Kieran,

On Thursday, 17 May 2018 20:06:46 EEST Kieran Bingham wrote:
> On 17/05/18 15:35, Laurent Pinchart wrote:
> > On Monday, 30 April 2018 20:48:03 EEST Kieran Bingham wrote:
> >> On 07/04/18 01:23, Laurent Pinchart wrote:
> >>> On Thursday, 8 March 2018 02:05:31 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. This ensures that when the hardware is reset - our cached
>  configuration will be re-attached to the next committed DL.
>  
>  Signed-off-by: Kieran Bingham 
>  ---
>  
>  v3:
>   - 's/fragment/body/', 's/fragments/bodies/'
>   - video dlb cache allocation increased from 2 to 3 dlbs
>  
>  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
> >>> 
> >>> This might be useful to capture in the main part of the commit message.
> >>> 
>  v4:
>   - Adjust pipe configured flag to be reset on resume rather than
>   suspend
>   - rename dl_child, dl_next
>   
>   drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
>   drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>   drivers/media/platform/vsp1/vsp1_video.c | 67 ++-
>   drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>   4 files changed, 54 insertions(+), 26 deletions(-)
>  
>  diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>  b/drivers/media/platform/vsp1/vsp1_pipe.c index
>  5012643583b6..fa445b1a2e38
>  100644
>  --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>  +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>  @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>   vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>  VI6_CMD_STRCMD);
>   pipe->state = VSP1_PIPELINE_RUNNING;
>  +pipe->configured = true;
>   }
>   
>   pipe->buffers_ready = 0;
>  @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device
>  *vsp1)
>   continue;
>   
>   spin_lock_irqsave(>irqlock, flags);
>  +/*
>  + * The hardware may have been reset during a suspend 
>  and will
>  + * need a full reconfiguration
>  + */
> >>> 
> >>> s/reconfiguration/reconfiguration./
> >>> 
>  +

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-05-17 Thread Kieran Bingham

Hi Laurent,

On 17/05/18 15:35, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Monday, 30 April 2018 20:48:03 EEST Kieran Bingham wrote:
>> On 07/04/18 01:23, Laurent Pinchart wrote:
>>> On Thursday, 8 March 2018 02:05:31 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.
 This ensures that when the hardware is reset - our cached configuration
 will be re-attached to the next committed DL.

 Signed-off-by: Kieran Bingham 
 ---

 v3:
  - 's/fragment/body/', 's/fragments/bodies/'
  - video dlb cache allocation increased from 2 to 3 dlbs

 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
>>>
>>> This might be useful to capture in the main part of the commit message.
>>>
 v4:
  - Adjust pipe configured flag to be reset on resume rather than suspend
  - rename dl_child, dl_next
  
  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
  drivers/media/platform/vsp1/vsp1_video.c | 67 -
  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
  4 files changed, 54 insertions(+), 26 deletions(-)

 diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
 b/drivers/media/platform/vsp1/vsp1_pipe.c index
 5012643583b6..fa445b1a2e38
 100644
 --- a/drivers/media/platform/vsp1/vsp1_pipe.c
 +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
 @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
   VI6_CMD_STRCMD);
pipe->state = VSP1_PIPELINE_RUNNING;
 +  pipe->configured = true;
}

pipe->buffers_ready = 0;
 @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
continue;

spin_lock_irqsave(>irqlock, flags);
 +  /*
 +   * The hardware may have been reset during a suspend and will
 +   * need a full reconfiguration
 +   */
>>>
>>> s/reconfiguration/reconfiguration./
>>>
 +  pipe->configured = false;
 +
>>>
>>> Where does that full reconfiguration occur, given that the
>>> vsp1_pipeline_run() right below sets pipe->configured to true without
>>> performing reconfiguration ?
> Q 
>> It's magic isn't it :D
>>
>> If the pipe->configured flag gets set to false, the next execution of
>> vsp1_pipeline_run() attaches the video->pipe_config (the cached
>>

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-05-17 Thread Laurent Pinchart

Hi Kieran,

On Monday, 30 April 2018 20:48:03 EEST Kieran Bingham wrote:
> On 07/04/18 01:23, Laurent Pinchart wrote:
> > On Thursday, 8 March 2018 02:05:31 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.
> >> This ensures that when the hardware is reset - our cached configuration
> >> will be re-attached to the next committed DL.
> >> 
> >> Signed-off-by: Kieran Bingham 
> >> ---
> >> 
> >> v3:
> >>  - 's/fragment/body/', 's/fragments/bodies/'
> >>  - video dlb cache allocation increased from 2 to 3 dlbs
> >> 
> >> 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
> > 
> > This might be useful to capture in the main part of the commit message.
> > 
> >> v4:
> >>  - Adjust pipe configured flag to be reset on resume rather than suspend
> >>  - rename dl_child, dl_next
> >>  
> >>  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
> >>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
> >>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
> >>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
> >>  4 files changed, 54 insertions(+), 26 deletions(-)
> >> 
> >> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
> >> b/drivers/media/platform/vsp1/vsp1_pipe.c index
> >> 5012643583b6..fa445b1a2e38
> >> 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
> >> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
> >>vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
> >>   VI6_CMD_STRCMD);
> >>pipe->state = VSP1_PIPELINE_RUNNING;
> >> +  pipe->configured = true;
> >>}
> >>
> >>pipe->buffers_ready = 0;
> >> @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
> >>continue;
> >>
> >>spin_lock_irqsave(>irqlock, flags);
> >> +  /*
> >> +   * The hardware may have been reset during a suspend and will
> >> +   * need a full reconfiguration
> >> +   */
> > 
> > s/reconfiguration/reconfiguration./
> > 
> >> +  pipe->configured = false;
> >> +
> > 
> > Where does that full reconfiguration occur, given that the
> > vsp1_pipeline_run() right below sets pipe->configured to true without
> > performing reconfiguration ?
Q 
> It's magic isn't it :D
> 
> If the pipe->configured flag gets set to false, the next execution of
> vsp1_pipeline_run() attaches the video->pipe_config (the cached
> configuration, containing the route_setup() and the

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-05-01 Thread Kieran Bingham

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 01/05/18 09:28, Kieran Bingham wrote:
> Hi Laurent,
> 
> New plan ... (from the .. why didn't I think of this earlier department)

Nope ... My suggestion in the previous mail (regarding dropping the configured
flag in place of using the pipe->state s) doesn't work because the pipeline is
set to be running before the vsp1_video_pipeline_run() gets the opportunity to
check it.

We could extend the states to include a 'VSP1_PIPELINE_STARTING' in between
_STOPPED, and _RUNNING, but I think that's more complicated as the state
machine across the whole code will be affected.

(which is why I chose to add a flag in the first place)


> On 30/04/18 18:48, Kieran Bingham wrote:
>> Hi Laurent,
>> 
>> On 07/04/18 01:23, Laurent Pinchart wrote:
>>> Hi Kieran,
>>> 
>>> Thank you for the patch.
>>> 
>>> On Thursday, 8 March 2018 02:05:31 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. This ensures that when the hardware is reset - our cached
 configuration will be re-attached to the next committed DL.
 
 Signed-off-by: Kieran Bingham
  ---
 
 v3: - 's/fragment/body/', 's/fragments/bodies/' - video dlb cache
 allocation increased from 2 to 3 dlbs
 
 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
>>> 
>>> This might be useful to capture in the main part of the commit
>>> message.
>>> 
 v4: - Adjust pipe configured flag to be reset on resume rather than
 suspend - rename dl_child, dl_next
 
 drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++- 
 drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +- 
 drivers/media/platform/vsp1/vsp1_video.c | 67
 - drivers/media/platform/vsp1/vsp1_video.h |
 2 +- 4 files changed, 54 insertions(+), 26 deletions(-)
 
 diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
 b/drivers/media/platform/vsp1/vsp1_pipe.c index
 5012643583b6..fa445b1a2e38 100644 ---
 a/drivers/media/platform/vsp1/vsp1_pipe.c +++
 b/drivers/media/platform/vsp1/vsp1_pipe.c @@ -249,6 +249,7 @@ void
 vsp1_pipeline_run(struct vsp1_pipeline *pipe) vsp1_write(vsp1,
 VI6_CMD(pipe->output->entity.index), VI6_CMD_STRCMD); pipe->state =
 VSP1_PIPELINE_RUNNING; +   pipe->configured = true;
> 
> Look at that lovely pipe->state flag update right above the
> pipe->configured update...
> 
 }
 
 pipe->buffers_ready = 0; @@ -470,6 +471,12 @@ void
 vsp1_pipelines_resume(struct vsp1_device *vsp1) continue;
 
 spin_lock_irqsave(>irqlock, flags); +/* +
  * The hardware
 may have been reset

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-05-01 Thread Kieran Bingham

Hi Laurent,

New plan ... (from the .. why didn't I think of this earlier department)



On 30/04/18 18:48, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 07/04/18 01:23, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> Thank you for the patch.
>>
>> On Thursday, 8 March 2018 02:05:31 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.
>>> This ensures that when the hardware is reset - our cached configuration
>>> will be re-attached to the next committed DL.
>>>
>>> Signed-off-by: Kieran Bingham 
>>> ---
>>>
>>> v3:
>>>  - 's/fragment/body/', 's/fragments/bodies/'
>>>  - video dlb cache allocation increased from 2 to 3 dlbs
>>>
>>> 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
>>
>> This might be useful to capture in the main part of the commit message.
>>
>>> v4:
>>>  - Adjust pipe configured flag to be reset on resume rather than suspend
>>>  - rename dl_child, dl_next
>>>
>>>  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
>>>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>>>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
>>>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>>>  4 files changed, 54 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>>> b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..fa445b1a2e38
>>> 100644
>>> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>>> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>>> vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>>>VI6_CMD_STRCMD);
>>> pipe->state = VSP1_PIPELINE_RUNNING;
>>> +   pipe->configured = true;

Look at that lovely pipe->state flag update right above the pipe->configured
update...

>>> }
>>>
>>> pipe->buffers_ready = 0;
>>> @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
>>> continue;
>>>
>>> spin_lock_irqsave(>irqlock, flags);
>>> +   /*
>>> +* The hardware may have been reset during a suspend and will
>>> +* need a full reconfiguration
>>> +*/
>>
>> s/reconfiguration/reconfiguration./
>>
>>> +   pipe->configured = false;

If we have 'suspended' then pipe->state == STOPPED


>>> +
>>
>> Where does that full reconfiguration occur, given that the 
>> vsp1_pipeline_run() 
>> right below sets pipe->configured to true without performing reconfiguration 
>> ?
> 
> It's magic isn't it :D
> 
> If the pipe->configured flag gets set to false, the next execution of
>

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-04-30 Thread Kieran Bingham

Hi Laurent,

On 07/04/18 01:23, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 8 March 2018 02:05:31 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.
>> This ensures that when the hardware is reset - our cached configuration
>> will be re-attached to the next committed DL.
>>
>> Signed-off-by: Kieran Bingham 
>> ---
>>
>> v3:
>>  - 's/fragment/body/', 's/fragments/bodies/'
>>  - video dlb cache allocation increased from 2 to 3 dlbs
>>
>> 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
> 
> This might be useful to capture in the main part of the commit message.
> 
>> v4:
>>  - Adjust pipe configured flag to be reset on resume rather than suspend
>>  - rename dl_child, dl_next
>>
>>  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
>>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
>>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>>  4 files changed, 54 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
>> b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..fa445b1a2e38
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
>> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>>  vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>> VI6_CMD_STRCMD);
>>  pipe->state = VSP1_PIPELINE_RUNNING;
>> +pipe->configured = true;
>>  }
>>
>>  pipe->buffers_ready = 0;
>> @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
>>  continue;
>>
>>  spin_lock_irqsave(>irqlock, flags);
>> +/*
>> + * The hardware may have been reset during a suspend and will
>> + * need a full reconfiguration
>> + */
> 
> s/reconfiguration/reconfiguration./
> 
>> +pipe->configured = false;
>> +
> 
> Where does that full reconfiguration occur, given that the 
> vsp1_pipeline_run() 
> right below sets pipe->configured to true without performing reconfiguration ?

It's magic isn't it :D

If the pipe->configured flag gets set to false, the next execution of
vsp1_pipeline_run() attaches the video->pipe_config (the cached configuration,
containing the route_setup() and the configure_stream() entries) to the display
list before configuring for the next frame.

This means that the hardware gets a full configuration written to it after a
suspend/resume action.

Perhaps the comment should say "The video object will write out it's cached pipe

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-04-06 Thread Laurent Pinchart

Hi Kieran,

Thank you for the patch.

On Thursday, 8 March 2018 02:05:31 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.
> This ensures that when the hardware is reset - our cached configuration
> will be re-attached to the next committed DL.
> 
> Signed-off-by: Kieran Bingham 
> ---
> 
> v3:
>  - 's/fragment/body/', 's/fragments/bodies/'
>  - video dlb cache allocation increased from 2 to 3 dlbs
> 
> 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

This might be useful to capture in the main part of the commit message.

> v4:
>  - Adjust pipe configured flag to be reset on resume rather than suspend
>  - rename dl_child, dl_next
> 
>  drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
>  drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
>  drivers/media/platform/vsp1/vsp1_video.c | 67 -
>  drivers/media/platform/vsp1/vsp1_video.h |  2 +-
>  4 files changed, 54 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c
> b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..fa445b1a2e38
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.c
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.c
> @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
>   vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
>  VI6_CMD_STRCMD);
>   pipe->state = VSP1_PIPELINE_RUNNING;
> + pipe->configured = true;
>   }
> 
>   pipe->buffers_ready = 0;
> @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
>   continue;
> 
>   spin_lock_irqsave(>irqlock, flags);
> + /*
> +  * The hardware may have been reset during a suspend and will
> +  * need a full reconfiguration
> +  */

s/reconfiguration/reconfiguration./

> + pipe->configured = false;
> +

Where does that full reconfiguration occur, given that the vsp1_pipeline_run() 
right below sets pipe->configured to true without performing reconfiguration ?

>   if (vsp1_pipeline_ready(pipe))
>   vsp1_pipeline_run(pipe);
>   spin_unlock_irqrestore(>irqlock, flags);
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index 90d29492b9b9..e7ad6211b4d0
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -90,6 +90,7 @@ struct vsp1_partition {
>   * @irqlock: protects the pipeline state
>   * @state: current state
>   * @wq: wait queue to wait for state change completion
> + * @configured: flag determining if the hardware has run since reset
>

[PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

2018-03-07 Thread Kieran Bingham

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.

Signed-off-by: Kieran Bingham 
---

v3:
 - 's/fragment/body/', 's/fragments/bodies/'
 - video dlb cache allocation increased from 2 to 3 dlbs

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

v4:
 - Adjust pipe configured flag to be reset on resume rather than suspend
 - rename dl_child, dl_next

 drivers/media/platform/vsp1/vsp1_pipe.c  |  7 +++-
 drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
 drivers/media/platform/vsp1/vsp1_video.c | 67 -
 drivers/media/platform/vsp1/vsp1_video.h |  2 +-
 4 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 5012643583b6..fa445b1a2e38 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
   VI6_CMD_STRCMD);
pipe->state = VSP1_PIPELINE_RUNNING;
+   pipe->configured = true;
}
 
pipe->buffers_ready = 0;
@@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1)
continue;
 
spin_lock_irqsave(>irqlock, flags);
+   /*
+* The hardware may have been reset during a suspend and will
+* need a full reconfiguration
+*/
+   pipe->configured = false;
+
if (vsp1_pipeline_ready(pipe))
vsp1_pipeline_run(pipe);
spin_unlock_irqrestore(>irqlock, flags);
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h 
b/drivers/media/platform/vsp1/vsp1_pipe.h
index 90d29492b9b9..e7ad6211b4d0 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -90,6 +90,7 @@ struct vsp1_partition {
  * @irqlock: protects the pipeline state
  * @state: current state
  * @wq: wait queue to wait for state change completion
+ * @configured: flag determining if the hardware has run since reset
  * @frame_end: frame end interrupt handler
  * @lock: protects the pipeline use count and stream count
  * @kref: pipeline reference count
@@ -117,6 +118,7 @@ struct vsp1_pipeline {
spinlock_t irqlock;
enum vsp1_pipeline_state state;
wait_queue_head_t wq;
+   bool configured;
 
void (*frame_end)(struct vsp1_pipeline *pipe, bool completed);
 
@@ -143,8 +145,6 @@ struct vsp1_pipeline {
 */
struct list_head entities;
 
-   struct vsp1_dl_list *dl;
-
unsigned int partitions;
struct

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

[PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb

8 matches

Site Navigation

Mail list logo

Footer information