* [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 16:03 ` Laurent Pinchart
2017-08-04 15:57 ` [PATCH v3 2/7] v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function Kieran Bingham
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
Presently any received buffers are only released back to vb2 if
vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
encounters an error, we will be warned by the vb2 handlers that buffers
have not been returned.
Move the buffer cleanup code to it's own function to prevent duplication
and call from both vsp1_video_stop_streaming() and the error path in
vsp1_video_start_streaming()
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 5af3486afe07..a24033429cd7 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
return 0;
}
+static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
+{
+ struct vsp1_vb2_buffer *buffer;
+ unsigned long flags;
+
+ /* Remove all buffers from the IRQ queue. */
+ spin_lock_irqsave(&video->irqlock, flags);
+ list_for_each_entry(buffer, &video->irqqueue, queue)
+ vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
+ INIT_LIST_HEAD(&video->irqqueue);
+ spin_unlock_irqrestore(&video->irqlock, flags);
+}
+
static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int count)
{
struct vsp1_video *video = vb2_get_drv_priv(vq);
@@ -835,6 +848,7 @@ static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int count)
ret = vsp1_video_setup_pipeline(pipe);
if (ret < 0) {
mutex_unlock(&pipe->lock);
+ vsp1_video_cleanup_pipeline(video);
return ret;
}
@@ -866,7 +880,6 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
{
struct vsp1_video *video = vb2_get_drv_priv(vq);
struct vsp1_pipeline *pipe = video->rwpf->pipe;
- struct vsp1_vb2_buffer *buffer;
unsigned long flags;
int ret;
@@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
media_pipeline_stop(&video->video.entity);
vsp1_video_pipeline_put(pipe);
- /* Remove all buffers from the IRQ queue. */
- spin_lock_irqsave(&video->irqlock, flags);
- list_for_each_entry(buffer, &video->irqqueue, queue)
- vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
- INIT_LIST_HEAD(&video->irqqueue);
- spin_unlock_irqrestore(&video->irqlock, flags);
+ vsp1_video_cleanup_pipeline(video);
}
static const struct vb2_ops vsp1_video_queue_qops = {
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 15:57 ` [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path Kieran Bingham
@ 2017-08-04 16:03 ` Laurent Pinchart
2017-08-04 16:04 ` Laurent Pinchart
2017-08-04 16:08 ` Kieran Bingham
0 siblings, 2 replies; 13+ messages in thread
From: Laurent Pinchart @ 2017-08-04 16:03 UTC (permalink / raw)
To: Kieran Bingham; +Cc: linux-media, linux-renesas-soc, kieran.bingham
Hi Kieran,
Thank you for the patch.
On Friday 04 Aug 2017 16:57:05 Kieran Bingham wrote:
> Presently any received buffers are only released back to vb2 if
> vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
> encounters an error, we will be warned by the vb2 handlers that buffers
> have not been returned.
>
> Move the buffer cleanup code to it's own function to prevent duplication
s/it's/its/
> and call from both vsp1_video_stop_streaming() and the error path in
> vsp1_video_start_streaming()
s/$/./
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index 5af3486afe07..a24033429cd7
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct
> vsp1_pipeline *pipe) return 0;
> }
>
> +static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
Should this function take a pipe pointer instead of a video pointer for
symmetry with vsp1_video_setup_pipeline() ?
> +{
> + struct vsp1_vb2_buffer *buffer;
> + unsigned long flags;
> +
> + /* Remove all buffers from the IRQ queue. */
> + spin_lock_irqsave(&video->irqlock, flags);
> + list_for_each_entry(buffer, &video->irqqueue, queue)
> + vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> + INIT_LIST_HEAD(&video->irqqueue);
> + spin_unlock_irqrestore(&video->irqlock, flags);
> +}
> +
> static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int
> count) {
> struct vsp1_video *video = vb2_get_drv_priv(vq);
> @@ -835,6 +848,7 @@ static int vsp1_video_start_streaming(struct vb2_queue
> *vq, unsigned int count) ret = vsp1_video_setup_pipeline(pipe);
> if (ret < 0) {
> mutex_unlock(&pipe->lock);
> + vsp1_video_cleanup_pipeline(video);
> return ret;
> }
>
> @@ -866,7 +880,6 @@ static void vsp1_video_stop_streaming(struct vb2_queue
> *vq) {
> struct vsp1_video *video = vb2_get_drv_priv(vq);
> struct vsp1_pipeline *pipe = video->rwpf->pipe;
> - struct vsp1_vb2_buffer *buffer;
> unsigned long flags;
> int ret;
>
> @@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct vb2_queue
> *vq) media_pipeline_stop(&video->video.entity);
> vsp1_video_pipeline_put(pipe);
>
> - /* Remove all buffers from the IRQ queue. */
> - spin_lock_irqsave(&video->irqlock, flags);
> - list_for_each_entry(buffer, &video->irqqueue, queue)
> - vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> - INIT_LIST_HEAD(&video->irqqueue);
> - spin_unlock_irqrestore(&video->irqlock, flags);
> + vsp1_video_cleanup_pipeline(video);
The vsp1_video_cleanup_pipeline() call should go before
vsp1_video_pipeline_put(), as you've noticed in patch 7/7.
With all that fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> }
>
> static const struct vb2_ops vsp1_video_queue_qops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 16:03 ` Laurent Pinchart
@ 2017-08-04 16:04 ` Laurent Pinchart
2017-08-04 16:08 ` Kieran Bingham
1 sibling, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2017-08-04 16:04 UTC (permalink / raw)
To: Kieran Bingham; +Cc: linux-media, linux-renesas-soc, kieran.bingham
On Friday 04 Aug 2017 19:03:28 Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Friday 04 Aug 2017 16:57:05 Kieran Bingham wrote:
> > Presently any received buffers are only released back to vb2 if
> > vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
> > encounters an error, we will be warned by the vb2 handlers that buffers
> > have not been returned.
> >
> > Move the buffer cleanup code to it's own function to prevent duplication
>
> s/it's/its/
>
> > and call from both vsp1_video_stop_streaming() and the error path in
> > vsp1_video_start_streaming()
>
> s/$/./
>
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >
> > drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
> > 1 file changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> > b/drivers/media/platform/vsp1/vsp1_video.c index
> > 5af3486afe07..a24033429cd7
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_video.c
> > +++ b/drivers/media/platform/vsp1/vsp1_video.c
> > @@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct
> > vsp1_pipeline *pipe) return 0;
> >
> > }
> >
> > +static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
>
> Should this function take a pipe pointer instead of a video pointer for
> symmetry with vsp1_video_setup_pipeline() ?
>
> > +{
> > + struct vsp1_vb2_buffer *buffer;
> > + unsigned long flags;
> > +
> > + /* Remove all buffers from the IRQ queue. */
> > + spin_lock_irqsave(&video->irqlock, flags);
> > + list_for_each_entry(buffer, &video->irqqueue, queue)
> > + vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> > + INIT_LIST_HEAD(&video->irqqueue);
> > + spin_unlock_irqrestore(&video->irqlock, flags);
> > +}
> > +
> >
> > static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int
> >
> > count) {
> >
> > struct vsp1_video *video = vb2_get_drv_priv(vq);
> >
> > @@ -835,6 +848,7 @@ static int vsp1_video_start_streaming(struct vb2_queue
> > *vq, unsigned int count) ret = vsp1_video_setup_pipeline(pipe);
> >
> > if (ret < 0) {
> >
> > mutex_unlock(&pipe->lock);
> >
> > + vsp1_video_cleanup_pipeline(video);
> >
> > return ret;
> >
> > }
> >
> > @@ -866,7 +880,6 @@ static void vsp1_video_stop_streaming(struct vb2_queue
> > *vq) {
> >
> > struct vsp1_video *video = vb2_get_drv_priv(vq);
> > struct vsp1_pipeline *pipe = video->rwpf->pipe;
> >
> > - struct vsp1_vb2_buffer *buffer;
> >
> > unsigned long flags;
> > int ret;
> >
> > @@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct
> > vb2_queue
> > *vq) media_pipeline_stop(&video->video.entity);
> >
> > vsp1_video_pipeline_put(pipe);
> >
> > - /* Remove all buffers from the IRQ queue. */
> > - spin_lock_irqsave(&video->irqlock, flags);
> > - list_for_each_entry(buffer, &video->irqqueue, queue)
> > - vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> > - INIT_LIST_HEAD(&video->irqqueue);
> > - spin_unlock_irqrestore(&video->irqlock, flags);
> > + vsp1_video_cleanup_pipeline(video);
>
> The vsp1_video_cleanup_pipeline() call should go before
> vsp1_video_pipeline_put(), as you've noticed in patch 7/7.
I meant 3/7.
>
> With all that fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > }
> >
> > static const struct vb2_ops vsp1_video_queue_qops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 16:03 ` Laurent Pinchart
2017-08-04 16:04 ` Laurent Pinchart
@ 2017-08-04 16:08 ` Kieran Bingham
2017-08-04 16:13 ` Laurent Pinchart
1 sibling, 1 reply; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 16:08 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc, kieran.bingham
Hi Laurent,
Thankyou for the speedy review.
On 04/08/17 17:03, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Friday 04 Aug 2017 16:57:05 Kieran Bingham wrote:
>> Presently any received buffers are only released back to vb2 if
>> vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
>> encounters an error, we will be warned by the vb2 handlers that buffers
>> have not been returned.
>>
>> Move the buffer cleanup code to it's own function to prevent duplication
>
> s/it's/its/
Ah yes - I'm always terrible with my its'y bits.
>
>> and call from both vsp1_video_stop_streaming() and the error path in
>> vsp1_video_start_streaming()
>
> s/$/./
:D
>
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>> drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
>> b/drivers/media/platform/vsp1/vsp1_video.c index 5af3486afe07..a24033429cd7
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_video.c
>> +++ b/drivers/media/platform/vsp1/vsp1_video.c
>> @@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct
>> vsp1_pipeline *pipe) return 0;
>> }
>>
>> +static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
>
> Should this function take a pipe pointer instead of a video pointer for
> symmetry with vsp1_video_setup_pipeline() ?
I passed this way because the cleanup needed a *video.
Is it possible to get from a *pipe to a *video?
>> +{
>> + struct vsp1_vb2_buffer *buffer;
>> + unsigned long flags;
>> +
>> + /* Remove all buffers from the IRQ queue. */
>> + spin_lock_irqsave(&video->irqlock, flags);
>> + list_for_each_entry(buffer, &video->irqqueue, queue)
>> + vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
>> + INIT_LIST_HEAD(&video->irqqueue);
>> + spin_unlock_irqrestore(&video->irqlock, flags);
>> +}
>> +
>> static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int
>> count) {
>> struct vsp1_video *video = vb2_get_drv_priv(vq);
>> @@ -835,6 +848,7 @@ static int vsp1_video_start_streaming(struct vb2_queue
>> *vq, unsigned int count) ret = vsp1_video_setup_pipeline(pipe);
>> if (ret < 0) {
>> mutex_unlock(&pipe->lock);
>> + vsp1_video_cleanup_pipeline(video);
>> return ret;
>> }
>>
>> @@ -866,7 +880,6 @@ static void vsp1_video_stop_streaming(struct vb2_queue
>> *vq) {
>> struct vsp1_video *video = vb2_get_drv_priv(vq);
>> struct vsp1_pipeline *pipe = video->rwpf->pipe;
>> - struct vsp1_vb2_buffer *buffer;
>> unsigned long flags;
>> int ret;
>>
>> @@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct vb2_queue
>> *vq) media_pipeline_stop(&video->video.entity);
>> vsp1_video_pipeline_put(pipe);
>>
>> - /* Remove all buffers from the IRQ queue. */
>> - spin_lock_irqsave(&video->irqlock, flags);
>> - list_for_each_entry(buffer, &video->irqqueue, queue)
>> - vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
>> - INIT_LIST_HEAD(&video->irqqueue);
>> - spin_unlock_irqrestore(&video->irqlock, flags);
>> + vsp1_video_cleanup_pipeline(video);
>
> The vsp1_video_cleanup_pipeline() call should go before
> vsp1_video_pipeline_put(), as you've noticed in patch 7/7.
I chose to do the move in 3/7 so that this patch did not change the existing
functionality.
There is no (explicit) need for the cleanup to happen before the pipeline_put()
until the cleanup function references the pipe...
>
> With all that fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>> }
>>
>> static const struct vb2_ops vsp1_video_queue_qops = {
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 16:08 ` Kieran Bingham
@ 2017-08-04 16:13 ` Laurent Pinchart
2017-08-04 16:19 ` Kieran Bingham
0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2017-08-04 16:13 UTC (permalink / raw)
To: kieran.bingham; +Cc: linux-media, linux-renesas-soc
Hi Kieran,
On Friday 04 Aug 2017 17:08:34 Kieran Bingham wrote:
> On 04/08/17 17:03, Laurent Pinchart wrote:
> > On Friday 04 Aug 2017 16:57:05 Kieran Bingham wrote:
> >> Presently any received buffers are only released back to vb2 if
> >> vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
> >> encounters an error, we will be warned by the vb2 handlers that buffers
> >> have not been returned.
> >>
> >> Move the buffer cleanup code to it's own function to prevent duplication
> >
> > s/it's/its/
>
> Ah yes - I'm always terrible with my its'y bits.
>
> >> and call from both vsp1_video_stop_streaming() and the error path in
> >> vsp1_video_start_streaming()
> >
> > s/$/./
>
> :D
>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> ---
> >>
> >> drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
> >> 1 file changed, 15 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> >> b/drivers/media/platform/vsp1/vsp1_video.c index
> >> 5af3486afe07..a24033429cd7
> >> 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_video.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> >> @@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct
> >> vsp1_pipeline *pipe)
> >> return 0;
> >> }
> >>
> >> +static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
> >
> > Should this function take a pipe pointer instead of a video pointer for
> > symmetry with vsp1_video_setup_pipeline() ?
>
> I passed this way because the cleanup needed a *video.
>
> Is it possible to get from a *pipe to a *video?
Yes, pipe->output->video
> >> +{
> >> + struct vsp1_vb2_buffer *buffer;
> >> + unsigned long flags;
> >> +
> >> + /* Remove all buffers from the IRQ queue. */
> >> + spin_lock_irqsave(&video->irqlock, flags);
> >> + list_for_each_entry(buffer, &video->irqqueue, queue)
> >> + vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> >> + INIT_LIST_HEAD(&video->irqqueue);
> >> + spin_unlock_irqrestore(&video->irqlock, flags);
> >> +}
[snip]
> >> @@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct
> >> vb2_queue
> >> *vq) media_pipeline_stop(&video->video.entity);
> >>
> >> vsp1_video_pipeline_put(pipe);
> >>
> >> - /* Remove all buffers from the IRQ queue. */
> >> - spin_lock_irqsave(&video->irqlock, flags);
> >> - list_for_each_entry(buffer, &video->irqqueue, queue)
> >> - vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
> >> - INIT_LIST_HEAD(&video->irqqueue);
> >> - spin_unlock_irqrestore(&video->irqlock, flags);
> >> + vsp1_video_cleanup_pipeline(video);
> >
> > The vsp1_video_cleanup_pipeline() call should go before
> > vsp1_video_pipeline_put(), as you've noticed in patch 7/7.
>
> I chose to do the move in 3/7 so that this patch did not change the existing
> functionality.
>
> There is no (explicit) need for the cleanup to happen before the
> pipeline_put() until the cleanup function references the pipe...
Except if you pass a *pipe to the cleanup function, as
vsp1_video_pipeline_put() can free the pipe.
> > With all that fixed,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> >> }
> >>
> >> static const struct vb2_ops vsp1_video_queue_qops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path
2017-08-04 16:13 ` Laurent Pinchart
@ 2017-08-04 16:19 ` Kieran Bingham
0 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 16:19 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc
Hi Laurent,
On 04/08/17 17:13, Laurent Pinchart wrote:
> On Friday 04 Aug 2017 17:08:34 Kieran Bingham wrote:
>> On 04/08/17 17:03, Laurent Pinchart wrote:
>>> On Friday 04 Aug 2017 16:57:05 Kieran Bingham wrote:
>>>> Presently any received buffers are only released back to vb2 if
>>>> vsp1_video_stop_streaming() is called. If vsp1_video_start_streaming()
>>>> encounters an error, we will be warned by the vb2 handlers that buffers
>>>> have not been returned.
>>>>
>>>> Move the buffer cleanup code to it's own function to prevent duplication
>>>
>>> s/it's/its/
>>
>> Ah yes - I'm always terrible with my its'y bits.
>>
>>>> and call from both vsp1_video_stop_streaming() and the error path in
>>>> vsp1_video_start_streaming()
>>>
>>> s/$/./
>>
>> :D
>>
>>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>> ---
>>>>
>>>> drivers/media/platform/vsp1/vsp1_video.c | 22 +++++++++++++++-------
>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
>>>> b/drivers/media/platform/vsp1/vsp1_video.c index
>>>> 5af3486afe07..a24033429cd7
>>>> 100644
>>>> --- a/drivers/media/platform/vsp1/vsp1_video.c
>>>> +++ b/drivers/media/platform/vsp1/vsp1_video.c
>>>> @@ -822,6 +822,19 @@ static int vsp1_video_setup_pipeline(struct
>>>> vsp1_pipeline *pipe)
>>>> return 0;
>>>> }
>>>>
>>>> +static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
>>>
>>> Should this function take a pipe pointer instead of a video pointer for
>>> symmetry with vsp1_video_setup_pipeline() ?
>>
>> I passed this way because the cleanup needed a *video.
>>
>> Is it possible to get from a *pipe to a *video?
>
> Yes, pipe->output->video
Ah yes of course.
>>>> +{
>>>> + struct vsp1_vb2_buffer *buffer;
>>>> + unsigned long flags;
>>>> +
>>>> + /* Remove all buffers from the IRQ queue. */
>>>> + spin_lock_irqsave(&video->irqlock, flags);
>>>> + list_for_each_entry(buffer, &video->irqqueue, queue)
>>>> + vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
>>>> + INIT_LIST_HEAD(&video->irqqueue);
>>>> + spin_unlock_irqrestore(&video->irqlock, flags);
>>>> +}
>
> [snip]
>
>>>> @@ -893,12 +906,7 @@ static void vsp1_video_stop_streaming(struct
>>>> vb2_queue
>>>> *vq) media_pipeline_stop(&video->video.entity);
>>>>
>>>> vsp1_video_pipeline_put(pipe);
>>>>
>>>> - /* Remove all buffers from the IRQ queue. */
>>>> - spin_lock_irqsave(&video->irqlock, flags);
>>>> - list_for_each_entry(buffer, &video->irqqueue, queue)
>>>> - vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
>>>> - INIT_LIST_HEAD(&video->irqqueue);
>>>> - spin_unlock_irqrestore(&video->irqlock, flags);
>>>> + vsp1_video_cleanup_pipeline(video);
>>>
>>> The vsp1_video_cleanup_pipeline() call should go before
>>> vsp1_video_pipeline_put(), as you've noticed in patch 7/7.
>>
>> I chose to do the move in 3/7 so that this patch did not change the existing
>> functionality.
>>
>> There is no (explicit) need for the cleanup to happen before the
>> pipeline_put() until the cleanup function references the pipe...
>
> Except if you pass a *pipe to the cleanup function, as
> vsp1_video_pipeline_put() can free the pipe.
Indeed!
>
>>> With all that fixed,
Patch update imminent.
Thanks.
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>>> }
>>>>
>>>> static const struct vb2_ops vsp1_video_queue_qops = {
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/7] v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 3/7] v4l: vsp1: Calculate partition sizes at stream start Kieran Bingham
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
Separate the code change from the function move so that code changes can
be clearly identified. This commit has no functional change.
The partition algorithm functions will be changed, and
vsp1_video_pipeline_setup_partitions() will call vsp1_video_partition().
To prepare for that, move the function without any code change.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_video.c | 74 ++++++++++++-------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index a24033429cd7..097e4db572a9 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -182,43 +182,6 @@ static int __vsp1_video_try_format(struct vsp1_video *video,
* VSP1 Partition Algorithm support
*/
-static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
-{
- struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
- const struct v4l2_mbus_framefmt *format;
- struct vsp1_entity *entity;
- unsigned int div_size;
-
- /*
- * Partitions are computed on the size before rotation, use the format
- * at the WPF sink.
- */
- format = vsp1_entity_get_pad_format(&pipe->output->entity,
- pipe->output->entity.config,
- RWPF_PAD_SINK);
- div_size = format->width;
-
- /* Gen2 hardware doesn't require image partitioning. */
- if (vsp1->info->gen == 2) {
- pipe->div_size = div_size;
- pipe->partitions = 1;
- return;
- }
-
- list_for_each_entry(entity, &pipe->entities, list_pipe) {
- unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH;
-
- if (entity->ops->max_width) {
- entity_max = entity->ops->max_width(entity, pipe);
- if (entity_max)
- div_size = min(div_size, entity_max);
- }
- }
-
- pipe->div_size = div_size;
- pipe->partitions = DIV_ROUND_UP(format->width, div_size);
-}
-
/**
* vsp1_video_partition - Calculate the active partition output window
*
@@ -293,6 +256,43 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
return partition;
}
+static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
+{
+ struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
+ const struct v4l2_mbus_framefmt *format;
+ struct vsp1_entity *entity;
+ unsigned int div_size;
+
+ /*
+ * Partitions are computed on the size before rotation, use the format
+ * at the WPF sink.
+ */
+ format = vsp1_entity_get_pad_format(&pipe->output->entity,
+ pipe->output->entity.config,
+ RWPF_PAD_SINK);
+ div_size = format->width;
+
+ /* Gen2 hardware doesn't require image partitioning. */
+ if (vsp1->info->gen == 2) {
+ pipe->div_size = div_size;
+ pipe->partitions = 1;
+ return;
+ }
+
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH;
+
+ if (entity->ops->max_width) {
+ entity_max = entity->ops->max_width(entity, pipe);
+ if (entity_max)
+ div_size = min(div_size, entity_max);
+ }
+ }
+
+ pipe->div_size = div_size;
+ pipe->partitions = DIV_ROUND_UP(format->width, div_size);
+}
+
/* -----------------------------------------------------------------------------
* Pipeline Management
*/
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 3/7] v4l: vsp1: Calculate partition sizes at stream start
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 1/7] v4l: vsp1: Release buffers in start_streaming error path Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 2/7] v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 4/7] v4l: vsp1: Remove redundant context variables Kieran Bingham
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
Previously the active window and partition sizes for each partition were
calculated for each partition every frame. This data is constant and
only needs to be calculated once at the start of the stream.
Extend the vsp1_pipe object to dynamically store the number of partitions
required and pre-calculate the partition sizes into this table.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_pipe.h | 3 ++-
drivers/media/platform/vsp1/vsp1_video.c | 48 +++++++++++++++++--------
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 91a784a13422..0cfd07a187a2 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -82,7 +82,9 @@ enum vsp1_pipeline_state {
* @dl: display list associated with the pipeline
* @div_size: The maximum allowed partition size for the pipeline
* @partitions: The number of partitions used to process one frame
+ * @partition: The current partition for configuration to process
* @current_partition: The partition number currently being configured
+ * @part_table: The pre-calculated partitions used by the pipeline
*/
struct vsp1_pipeline {
struct media_pipeline pipe;
@@ -117,6 +119,7 @@ struct vsp1_pipeline {
unsigned int partitions;
struct v4l2_rect partition;
unsigned int current_partition;
+ struct v4l2_rect *part_table;
};
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 097e4db572a9..447597f1b758 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -256,12 +256,13 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
return partition;
}
-static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
+static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
{
struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
const struct v4l2_mbus_framefmt *format;
struct vsp1_entity *entity;
unsigned int div_size;
+ unsigned int i;
/*
* Partitions are computed on the size before rotation, use the format
@@ -272,17 +273,17 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
RWPF_PAD_SINK);
div_size = format->width;
- /* Gen2 hardware doesn't require image partitioning. */
- if (vsp1->info->gen == 2) {
- pipe->div_size = div_size;
- pipe->partitions = 1;
- return;
- }
+ /*
+ * Only Gen3 hardware requires image partitioning, Gen2 will operate
+ * with a single partition that covers the whole output.
+ */
+ if (vsp1->info->gen == 3) {
+ list_for_each_entry(entity, &pipe->entities, list_pipe) {
+ unsigned int entity_max;
- list_for_each_entry(entity, &pipe->entities, list_pipe) {
- unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH;
+ if (!entity->ops->max_width)
+ continue;
- if (entity->ops->max_width) {
entity_max = entity->ops->max_width(entity, pipe);
if (entity_max)
div_size = min(div_size, entity_max);
@@ -291,6 +292,15 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
pipe->div_size = div_size;
pipe->partitions = DIV_ROUND_UP(format->width, div_size);
+ pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table),
+ GFP_KERNEL);
+ if (!pipe->part_table)
+ return -ENOMEM;
+
+ for (i = 0; i < pipe->partitions; ++i)
+ pipe->part_table[i] = vsp1_video_partition(pipe, div_size, i);
+
+ return 0;
}
/* -----------------------------------------------------------------------------
@@ -373,8 +383,7 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe,
{
struct vsp1_entity *entity;
- pipe->partition = vsp1_video_partition(pipe, pipe->div_size,
- pipe->current_partition);
+ pipe->partition = pipe->part_table[pipe->current_partition];
list_for_each_entry(entity, &pipe->entities, list_pipe) {
if (entity->ops->configure)
@@ -783,9 +792,12 @@ static void vsp1_video_buffer_queue(struct vb2_buffer *vb)
static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
{
struct vsp1_entity *entity;
+ int ret;
/* Determine this pipelines sizes for image partitioning support. */
- vsp1_video_pipeline_setup_partitions(pipe);
+ ret = vsp1_video_pipeline_setup_partitions(pipe);
+ if (ret < 0)
+ return ret;
/* Prepare the display list. */
pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
@@ -824,6 +836,7 @@ static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe)
static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
{
+ struct vsp1_pipeline *pipe = video->rwpf->pipe;
struct vsp1_vb2_buffer *buffer;
unsigned long flags;
@@ -833,6 +846,12 @@ static void vsp1_video_cleanup_pipeline(struct vsp1_video *video)
vb2_buffer_done(&buffer->buf.vb2_buf, VB2_BUF_STATE_ERROR);
INIT_LIST_HEAD(&video->irqqueue);
spin_unlock_irqrestore(&video->irqlock, flags);
+
+ /* Release our partition table allocation */
+ mutex_lock(&pipe->lock);
+ kfree(pipe->part_table);
+ pipe->part_table = NULL;
+ mutex_unlock(&pipe->lock);
}
static int vsp1_video_start_streaming(struct vb2_queue *vq, unsigned int count)
@@ -904,9 +923,8 @@ static void vsp1_video_stop_streaming(struct vb2_queue *vq)
mutex_unlock(&pipe->lock);
media_pipeline_stop(&video->video.entity);
- vsp1_video_pipeline_put(pipe);
-
vsp1_video_cleanup_pipeline(video);
+ vsp1_video_pipeline_put(pipe);
}
static const struct vb2_ops vsp1_video_queue_qops = {
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 4/7] v4l: vsp1: Remove redundant context variables
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
` (2 preceding siblings ...)
2017-08-04 15:57 ` [PATCH v3 3/7] v4l: vsp1: Calculate partition sizes at stream start Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 5/7] v4l: vsp1: Move partition rectangles to struct and operate directly Kieran Bingham
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
The vsp1_pipe object context variables for div_size and
current_partition allowed state to be maintained through processing the
partitions during processing.
Now that the partition tables are calculated during stream on, there is
no requirement to store these variables in the pipe object.
Utilise local variables for the processing as required.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_pipe.h | 4 ----
drivers/media/platform/vsp1/vsp1_video.c | 21 +++++++--------------
2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 0cfd07a187a2..9653ef5cfb0c 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -80,10 +80,8 @@ enum vsp1_pipeline_state {
* @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
- * @div_size: The maximum allowed partition size for the pipeline
* @partitions: The number of partitions used to process one frame
* @partition: The current partition for configuration to process
- * @current_partition: The partition number currently being configured
* @part_table: The pre-calculated partitions used by the pipeline
*/
struct vsp1_pipeline {
@@ -115,10 +113,8 @@ struct vsp1_pipeline {
struct vsp1_dl_list *dl;
- unsigned int div_size;
unsigned int partitions;
struct v4l2_rect partition;
- unsigned int current_partition;
struct v4l2_rect *part_table;
};
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 447597f1b758..f8c3186c0fb6 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -290,7 +290,6 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
}
}
- pipe->div_size = div_size;
pipe->partitions = DIV_ROUND_UP(format->width, div_size);
pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table),
GFP_KERNEL);
@@ -379,11 +378,12 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe,
}
static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl)
+ struct vsp1_dl_list *dl,
+ unsigned int partition)
{
struct vsp1_entity *entity;
- pipe->partition = pipe->part_table[pipe->current_partition];
+ pipe->partition = pipe->part_table[partition];
list_for_each_entry(entity, &pipe->entities, list_pipe) {
if (entity->ops->configure)
@@ -396,6 +396,7 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
{
struct vsp1_device *vsp1 = pipe->output->entity.vsp1;
struct vsp1_entity *entity;
+ unsigned int partition;
if (!pipe->dl)
pipe->dl = vsp1_dl_list_get(pipe->output->dlm);
@@ -412,20 +413,12 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
}
/* Run the first partition */
- pipe->current_partition = 0;
- vsp1_video_pipeline_run_partition(pipe, pipe->dl);
+ vsp1_video_pipeline_run_partition(pipe, pipe->dl, 0);
/* Process consecutive partitions as necessary */
- for (pipe->current_partition = 1;
- pipe->current_partition < pipe->partitions;
- pipe->current_partition++) {
+ for (partition = 1; partition < pipe->partitions; ++partition) {
struct vsp1_dl_list *dl;
- /*
- * Partition configuration operations will utilise
- * the pipe->current_partition variable to determine
- * the work they should complete.
- */
dl = vsp1_dl_list_get(pipe->output->dlm);
/*
@@ -438,7 +431,7 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe)
break;
}
- vsp1_video_pipeline_run_partition(pipe, dl);
+ vsp1_video_pipeline_run_partition(pipe, dl, partition);
vsp1_dl_list_add_chain(pipe->dl, dl);
}
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 5/7] v4l: vsp1: Move partition rectangles to struct and operate directly
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
` (3 preceding siblings ...)
2017-08-04 15:57 ` [PATCH v3 4/7] v4l: vsp1: Remove redundant context variables Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 6/7] v4l: vsp1: Provide UDS register updates Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 7/7] v4l: vsp1: Allow entities to participate in the partition algorithm Kieran Bingham
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
As we develop the partition algorithm, we need to store more information
per partition to describe the phase and other parameters.
To keep this data together, further abstract the existing v4l2_rect
into a partition specific structure. As partitions only have horizontal
coordinates, store the left and width values only.
When generating the partition windows, operate directly on the partition
struct rather than copying and duplicating the processed data
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_pipe.h | 15 +++++++--
drivers/media/platform/vsp1/vsp1_rpf.c | 4 +-
drivers/media/platform/vsp1/vsp1_uds.c | 18 +++++-----
drivers/media/platform/vsp1/vsp1_video.c | 43 +++++++++++--------------
drivers/media/platform/vsp1/vsp1_wpf.c | 14 ++++----
5 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 9653ef5cfb0c..4e9fd96108be 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -58,6 +58,17 @@ enum vsp1_pipeline_state {
};
/*
+ * struct vsp1_partition - A description of a slice for the partition algorithm
+ * @left: horizontal coordinate of the partition start in pixels relative to the
+ * left edge of the image
+ * @width: partition width in pixels
+ */
+struct vsp1_partition {
+ unsigned int left;
+ unsigned int width;
+};
+
+/*
* struct vsp1_pipeline - A VSP1 hardware pipeline
* @pipe: the media pipeline
* @irqlock: protects the pipeline state
@@ -114,8 +125,8 @@ struct vsp1_pipeline {
struct vsp1_dl_list *dl;
unsigned int partitions;
- struct v4l2_rect partition;
- struct v4l2_rect *part_table;
+ struct vsp1_partition *partition;
+ struct vsp1_partition *part_table;
};
void vsp1_pipeline_reset(struct vsp1_pipeline *pipe);
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 8feddd59cf8d..126741f00ae3 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -108,9 +108,9 @@ static void rpf_configure(struct vsp1_entity *entity,
output = vsp1_entity_get_pad_format(wpf, wpf->config,
RWPF_PAD_SINK);
- crop.width = pipe->partition.width * input_width
+ crop.width = pipe->partition->width * input_width
/ output->width;
- crop.left += pipe->partition.left * input_width
+ crop.left += pipe->partition->left * input_width
/ output->width;
}
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index 4226403ad235..4a43e7413b68 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -271,23 +271,25 @@ static void uds_configure(struct vsp1_entity *entity,
unsigned int vscale;
bool multitap;
+ input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SINK);
+ output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SOURCE);
+
if (params == VSP1_ENTITY_PARAMS_PARTITION) {
- const struct v4l2_rect *clip = &pipe->partition;
+ struct vsp1_partition *partition = pipe->partition;
vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE,
- (clip->width << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
- (clip->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
+ (partition->width
+ << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
+ (output->height
+ << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
return;
}
if (params != VSP1_ENTITY_PARAMS_INIT)
return;
- input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
- UDS_PAD_SINK);
- output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
- UDS_PAD_SOURCE);
-
hscale = uds_compute_ratio(input->width, output->width);
vscale = uds_compute_ratio(input->height, output->height);
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index f8c3186c0fb6..5522595e9323 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -183,19 +183,19 @@ static int __vsp1_video_try_format(struct vsp1_video *video,
*/
/**
- * vsp1_video_partition - Calculate the active partition output window
+ * vsp1_video_calculate_partition - Calculate the active partition output window
*
+ * @pipe: the pipeline
+ * @partition: partition that will hold the calculated values
* @div_size: pre-determined maximum partition division size
* @index: partition index
- *
- * Returns a v4l2_rect describing the partition window.
*/
-static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
- unsigned int div_size,
- unsigned int index)
+static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int div_size,
+ unsigned int index)
{
const struct v4l2_mbus_framefmt *format;
- struct v4l2_rect partition;
unsigned int modulus;
/*
@@ -208,18 +208,14 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
/* A single partition simply processes the output size in full. */
if (pipe->partitions <= 1) {
- partition.left = 0;
- partition.top = 0;
- partition.width = format->width;
- partition.height = format->height;
- return partition;
+ partition->left = 0;
+ partition->width = format->width;
+ return;
}
/* Initialise the partition with sane starting conditions. */
- partition.left = index * div_size;
- partition.top = 0;
- partition.width = div_size;
- partition.height = format->height;
+ partition->left = index * div_size;
+ partition->width = div_size;
modulus = format->width % div_size;
@@ -242,18 +238,16 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe,
if (modulus < div_size / 2) {
if (index == partitions - 1) {
/* Halve the penultimate partition. */
- partition.width = div_size / 2;
+ partition->width = div_size / 2;
} else if (index == partitions) {
/* Increase the final partition. */
- partition.width = (div_size / 2) + modulus;
- partition.left -= div_size / 2;
+ partition->width = (div_size / 2) + modulus;
+ partition->left -= div_size / 2;
}
} else if (index == partitions) {
- partition.width = modulus;
+ partition->width = modulus;
}
}
-
- return partition;
}
static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
@@ -297,7 +291,8 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
return -ENOMEM;
for (i = 0; i < pipe->partitions; ++i)
- pipe->part_table[i] = vsp1_video_partition(pipe, div_size, i);
+ vsp1_video_calculate_partition(pipe, &pipe->part_table[i],
+ div_size, i);
return 0;
}
@@ -383,7 +378,7 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe,
{
struct vsp1_entity *entity;
- pipe->partition = pipe->part_table[partition];
+ pipe->partition = &pipe->part_table[partition];
list_for_each_entry(entity, &pipe->entities, list_pipe) {
if (entity->ops->configure)
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 32df109b119f..c8f7cf048841 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -291,7 +291,7 @@ static void wpf_configure(struct vsp1_entity *entity,
* multiple slices.
*/
if (pipe->partitions > 1)
- width = pipe->partition.width;
+ width = pipe->partition->width;
vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
(0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
@@ -320,13 +320,13 @@ static void wpf_configure(struct vsp1_entity *entity,
* is applied horizontally or vertically accordingly.
*/
if (flip & BIT(WPF_CTRL_HFLIP) && !wpf->flip.rotate)
- offset = format->width - pipe->partition.left
- - pipe->partition.width;
+ offset = format->width - pipe->partition->left
+ - pipe->partition->width;
else if (flip & BIT(WPF_CTRL_VFLIP) && wpf->flip.rotate)
- offset = format->height - pipe->partition.left
- - pipe->partition.width;
+ offset = format->height - pipe->partition->left
+ - pipe->partition->width;
else
- offset = pipe->partition.left;
+ offset = pipe->partition->left;
for (i = 0; i < format->num_planes; ++i) {
unsigned int hsub = i > 0 ? fmtinfo->hsub : 1;
@@ -348,7 +348,7 @@ static void wpf_configure(struct vsp1_entity *entity,
* image height.
*/
if (wpf->flip.rotate)
- height = pipe->partition.width;
+ height = pipe->partition->width;
else
height = format->height;
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 6/7] v4l: vsp1: Provide UDS register updates
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
` (4 preceding siblings ...)
2017-08-04 15:57 ` [PATCH v3 5/7] v4l: vsp1: Move partition rectangles to struct and operate directly Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
2017-08-04 15:57 ` [PATCH v3 7/7] v4l: vsp1: Allow entities to participate in the partition algorithm Kieran Bingham
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
Provide register definitions required for UDS phase and partition
algorithm support. The registers and bits defined here are available on
Gen3 hardware only.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_regs.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index cd3e32af6e3b..362f4f8b1148 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -388,6 +388,7 @@
#define VI6_UDS_CTRL_NE_RCR (1 << 18)
#define VI6_UDS_CTRL_NE_GY (1 << 17)
#define VI6_UDS_CTRL_NE_BCB (1 << 16)
+#define VI6_UDS_CTRL_AMDSLH (1 << 2)
#define VI6_UDS_CTRL_TDIPC (1 << 1)
#define VI6_UDS_SCALE 0x2304
@@ -420,11 +421,24 @@
#define VI6_UDS_PASS_BWIDTH_V_MASK (0x7f << 0)
#define VI6_UDS_PASS_BWIDTH_V_SHIFT 0
+#define VI6_UDS_HPHASE 0x2314
+#define VI6_UDS_HPHASE_HSTP_MASK (0xfff << 16)
+#define VI6_UDS_HPHASE_HSTP_SHIFT 16
+#define VI6_UDS_HPHASE_HEDP_MASK (0xfff << 0)
+#define VI6_UDS_HPHASE_HEDP_SHIFT 0
+
#define VI6_UDS_IPC 0x2318
#define VI6_UDS_IPC_FIELD (1 << 27)
#define VI6_UDS_IPC_VEDP_MASK (0xfff << 0)
#define VI6_UDS_IPC_VEDP_SHIFT 0
+#define VI6_UDS_HSZCLIP 0x231c
+#define VI6_UDS_HSZCLIP_HCEN (1 << 28)
+#define VI6_UDS_HSZCLIP_HCL_OFST_MASK (0xff << 16)
+#define VI6_UDS_HSZCLIP_HCL_OFST_SHIFT 16
+#define VI6_UDS_HSZCLIP_HCL_SIZE_MASK (0x1fff << 0)
+#define VI6_UDS_HSZCLIP_HCL_SIZE_SHIFT 0
+
#define VI6_UDS_CLIP_SIZE 0x2324
#define VI6_UDS_CLIP_SIZE_HSIZE_MASK (0x1fff << 16)
#define VI6_UDS_CLIP_SIZE_HSIZE_SHIFT 16
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 7/7] v4l: vsp1: Allow entities to participate in the partition algorithm
2017-08-04 15:57 [PATCH v3 0/7] vsp1 partition algorithm improvements Kieran Bingham
` (5 preceding siblings ...)
2017-08-04 15:57 ` [PATCH v3 6/7] v4l: vsp1: Provide UDS register updates Kieran Bingham
@ 2017-08-04 15:57 ` Kieran Bingham
6 siblings, 0 replies; 13+ messages in thread
From: Kieran Bingham @ 2017-08-04 15:57 UTC (permalink / raw)
To: linux-media, linux-renesas-soc
Cc: laurent.pinchart, kieran.bingham, Kieran Bingham
The configuration of the pipeline and entities directly affects the
inputs required to each entity for the partition algorithm. Thus it
makes sense to involve those entities in the decision making process.
Extend the entity ops API to provide an optional .partition() operation.
This allows entities that affect the partition window to adapt the
window based on their configuration.
Entities implementing this operation must update the window parameter in
place, which will then be passed up the pipeline. This creates a process
whereby each entity describes what is required to satisfy the required
output to its predecessor in the pipeline.
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/vsp1/vsp1_entity.h | 7 ++++-
drivers/media/platform/vsp1/vsp1_pipe.c | 22 +++++++++++++-
drivers/media/platform/vsp1/vsp1_pipe.h | 30 ++++++++++++++++--
drivers/media/platform/vsp1/vsp1_rpf.c | 27 +++++++---------
drivers/media/platform/vsp1/vsp1_sru.c | 26 +++++++++++++++-
drivers/media/platform/vsp1/vsp1_uds.c | 41 +++++++++++++++++++++++-
drivers/media/platform/vsp1/vsp1_video.c | 22 ++++++++-----
drivers/media/platform/vsp1/vsp1_wpf.c | 24 +++++++++-----
8 files changed, 166 insertions(+), 33 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h
index c169a060b6d2..05c616883f4a 100644
--- a/drivers/media/platform/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/vsp1/vsp1_entity.h
@@ -21,6 +21,8 @@
struct vsp1_device;
struct vsp1_dl_list;
struct vsp1_pipeline;
+struct vsp1_partition;
+struct vsp1_partition_window;
enum vsp1_entity_type {
VSP1_ENTITY_BRU,
@@ -81,12 +83,17 @@ struct vsp1_route {
* selection rectangles, ...)
* @max_width: Return the max supported width of data that the entity can
* process in a single operation.
+ * @partition: Process the partition construction based on this entity's
+ * configuration.
*/
struct vsp1_entity_operations {
void (*destroy)(struct vsp1_entity *);
void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *,
struct vsp1_dl_list *, enum vsp1_entity_params);
unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *);
+ void (*partition)(struct vsp1_entity *, struct vsp1_pipeline *,
+ struct vsp1_partition *, unsigned int,
+ struct vsp1_partition_window *);
};
struct vsp1_entity {
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c
index e817623b84e0..eff346727c72 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -382,6 +382,28 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
vsp1_uds_set_alpha(pipe->uds, dl, alpha);
}
+/*
+ * Propagate the partition calculations through the pipeline
+ *
+ * Work backwards through the pipe, allowing each entity to update the partition
+ * parameters based on its configuration, and the entity connected to its
+ * source. Each entity must produce the partition required for the previous
+ * entity in the pipeline.
+ */
+void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int index,
+ struct vsp1_partition_window *window)
+{
+ struct vsp1_entity *entity;
+
+ list_for_each_entry_reverse(entity, &pipe->entities, list_pipe) {
+ if (entity->ops->partition)
+ entity->ops->partition(entity, pipe, partition, index,
+ window);
+ }
+}
+
void vsp1_pipelines_suspend(struct vsp1_device *vsp1)
{
unsigned long flags;
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 4e9fd96108be..424e43fafaa5 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -58,17 +58,33 @@ enum vsp1_pipeline_state {
};
/*
- * struct vsp1_partition - A description of a slice for the partition algorithm
+ * struct vsp1_partition_window - Partition window coordinates
* @left: horizontal coordinate of the partition start in pixels relative to the
* left edge of the image
* @width: partition width in pixels
*/
-struct vsp1_partition {
+struct vsp1_partition_window {
unsigned int left;
unsigned int width;
};
/*
+ * struct vsp1_partition - A description of a slice for the partition algorithm
+ * @rpf: The RPF partition window configuration
+ * @uds_sink: The UDS input partition window configuration
+ * @uds_source: The UDS output partition window configuration
+ * @sru: The SRU partition window configuration
+ * @wpf: The WPF partition window configuration
+ */
+struct vsp1_partition {
+ struct vsp1_partition_window rpf;
+ struct vsp1_partition_window uds_sink;
+ struct vsp1_partition_window uds_source;
+ struct vsp1_partition_window sru;
+ struct vsp1_partition_window wpf;
+};
+
+/*
* struct vsp1_pipeline - A VSP1 hardware pipeline
* @pipe: the media pipeline
* @irqlock: protects the pipeline state
@@ -120,6 +136,11 @@ struct vsp1_pipeline {
struct vsp1_entity *uds;
struct vsp1_entity *uds_input;
+ /*
+ * The order of this list must be identical to the order of the entities
+ * in the pipeline, as it is assumed by the partition algorithm that we
+ * can walk this list in sequence.
+ */
struct list_head entities;
struct vsp1_dl_list *dl;
@@ -142,6 +163,11 @@ void vsp1_pipeline_frame_end(struct vsp1_pipeline *pipe);
void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe,
struct vsp1_dl_list *dl, unsigned int alpha);
+void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int index,
+ struct vsp1_partition_window *window);
+
void vsp1_pipelines_suspend(struct vsp1_device *vsp1);
void vsp1_pipelines_resume(struct vsp1_device *vsp1);
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 126741f00ae3..fe0633da5a5f 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -97,21 +97,8 @@ static void rpf_configure(struct vsp1_entity *entity,
* 'width' need to be adjusted.
*/
if (pipe->partitions > 1) {
- const struct v4l2_mbus_framefmt *output;
- struct vsp1_entity *wpf = &pipe->output->entity;
- unsigned int input_width = crop.width;
-
- /*
- * Scale the partition window based on the configuration
- * of the pipeline.
- */
- output = vsp1_entity_get_pad_format(wpf, wpf->config,
- RWPF_PAD_SINK);
-
- crop.width = pipe->partition->width * input_width
- / output->width;
- crop.left += pipe->partition->left * input_width
- / output->width;
+ crop.width = pipe->partition->rpf.width;
+ crop.left += pipe->partition->rpf.left;
}
vsp1_rpf_write(rpf, dl, VI6_RPF_SRC_BSIZE,
@@ -260,8 +247,18 @@ static void rpf_configure(struct vsp1_entity *entity,
}
+static void rpf_partition(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int partition_idx,
+ struct vsp1_partition_window *window)
+{
+ partition->rpf = *window;
+}
+
static const struct vsp1_entity_operations rpf_entity_ops = {
.configure = rpf_configure,
+ .partition = rpf_partition,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_sru.c b/drivers/media/platform/vsp1/vsp1_sru.c
index 30142793dfcd..51e5691187c3 100644
--- a/drivers/media/platform/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/vsp1/vsp1_sru.c
@@ -18,6 +18,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_pipe.h"
#include "vsp1_sru.h"
#define SRU_MIN_SIZE 4U
@@ -325,9 +326,34 @@ static unsigned int sru_max_width(struct vsp1_entity *entity,
return 256;
}
+static void sru_partition(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int partition_idx,
+ struct vsp1_partition_window *window)
+{
+ struct vsp1_sru *sru = to_sru(&entity->subdev);
+ struct v4l2_mbus_framefmt *input;
+ struct v4l2_mbus_framefmt *output;
+
+ input = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
+ SRU_PAD_SINK);
+ output = vsp1_entity_get_pad_format(&sru->entity, sru->entity.config,
+ SRU_PAD_SOURCE);
+
+ /* Adapt if SRUx2 is enabled */
+ if (input->width != output->width) {
+ window->width /= 2;
+ window->left /= 2;
+ }
+
+ partition->sru = *window;
+}
+
static const struct vsp1_entity_operations sru_entity_ops = {
.configure = sru_configure,
.max_width = sru_max_width,
+ .partition = sru_partition,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c
index 4a43e7413b68..72f72a9d2152 100644
--- a/drivers/media/platform/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/vsp1/vsp1_uds.c
@@ -279,8 +279,15 @@ static void uds_configure(struct vsp1_entity *entity,
if (params == VSP1_ENTITY_PARAMS_PARTITION) {
struct vsp1_partition *partition = pipe->partition;
+ /* Input size clipping */
+ vsp1_uds_write(uds, dl, VI6_UDS_HSZCLIP, VI6_UDS_HSZCLIP_HCEN |
+ (0 << VI6_UDS_HSZCLIP_HCL_OFST_SHIFT) |
+ (partition->uds_sink.width
+ << VI6_UDS_HSZCLIP_HCL_SIZE_SHIFT));
+
+ /* Output size clipping */
vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE,
- (partition->width
+ (partition->uds_source.width
<< VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) |
(output->height
<< VI6_UDS_CLIP_SIZE_VSIZE_SHIFT));
@@ -345,9 +352,41 @@ static unsigned int uds_max_width(struct vsp1_entity *entity,
return 2048;
}
+/* -----------------------------------------------------------------------------
+ * Partition Algorithm Support
+ */
+
+static void uds_partition(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int partition_idx,
+ struct vsp1_partition_window *window)
+{
+ struct vsp1_uds *uds = to_uds(&entity->subdev);
+ const struct v4l2_mbus_framefmt *output;
+ const struct v4l2_mbus_framefmt *input;
+
+ /* Initialise the partition state */
+ partition->uds_sink = *window;
+ partition->uds_source = *window;
+
+ input = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SINK);
+ output = vsp1_entity_get_pad_format(&uds->entity, uds->entity.config,
+ UDS_PAD_SOURCE);
+
+ partition->uds_sink.width = window->width * input->width
+ / output->width;
+ partition->uds_sink.left = window->left * input->width
+ / output->width;
+
+ *window = partition->uds_sink;
+}
+
static const struct vsp1_entity_operations uds_entity_ops = {
.configure = uds_configure,
.max_width = uds_max_width,
+ .partition = uds_partition,
};
/* -----------------------------------------------------------------------------
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index 5522595e9323..0917a42ac5fe 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -196,6 +196,7 @@ static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
unsigned int index)
{
const struct v4l2_mbus_framefmt *format;
+ struct vsp1_partition_window window;
unsigned int modulus;
/*
@@ -208,14 +209,17 @@ static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
/* A single partition simply processes the output size in full. */
if (pipe->partitions <= 1) {
- partition->left = 0;
- partition->width = format->width;
+ window.left = 0;
+ window.width = format->width;
+
+ vsp1_pipeline_propagate_partition(pipe, partition, index,
+ &window);
return;
}
/* Initialise the partition with sane starting conditions. */
- partition->left = index * div_size;
- partition->width = div_size;
+ window.left = index * div_size;
+ window.width = div_size;
modulus = format->width % div_size;
@@ -238,16 +242,18 @@ static void vsp1_video_calculate_partition(struct vsp1_pipeline *pipe,
if (modulus < div_size / 2) {
if (index == partitions - 1) {
/* Halve the penultimate partition. */
- partition->width = div_size / 2;
+ window.width = div_size / 2;
} else if (index == partitions) {
/* Increase the final partition. */
- partition->width = (div_size / 2) + modulus;
- partition->left -= div_size / 2;
+ window.width = (div_size / 2) + modulus;
+ window.left -= div_size / 2;
}
} else if (index == partitions) {
- partition->width = modulus;
+ window.width = modulus;
}
}
+
+ vsp1_pipeline_propagate_partition(pipe, partition, index, &window);
}
static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe)
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index c8f7cf048841..0b882074315e 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -291,7 +291,7 @@ static void wpf_configure(struct vsp1_entity *entity,
* multiple slices.
*/
if (pipe->partitions > 1)
- width = pipe->partition->width;
+ width = pipe->partition->wpf.width;
vsp1_wpf_write(wpf, dl, VI6_WPF_HSZCLIP, VI6_WPF_SZCLIP_EN |
(0 << VI6_WPF_SZCLIP_OFST_SHIFT) |
@@ -320,13 +320,13 @@ static void wpf_configure(struct vsp1_entity *entity,
* is applied horizontally or vertically accordingly.
*/
if (flip & BIT(WPF_CTRL_HFLIP) && !wpf->flip.rotate)
- offset = format->width - pipe->partition->left
- - pipe->partition->width;
+ offset = format->width - pipe->partition->wpf.left
+ - pipe->partition->wpf.width;
else if (flip & BIT(WPF_CTRL_VFLIP) && wpf->flip.rotate)
- offset = format->height - pipe->partition->left
- - pipe->partition->width;
+ offset = format->height - pipe->partition->wpf.left
+ - pipe->partition->wpf.width;
else
- offset = pipe->partition->left;
+ offset = pipe->partition->wpf.left;
for (i = 0; i < format->num_planes; ++i) {
unsigned int hsub = i > 0 ? fmtinfo->hsub : 1;
@@ -348,7 +348,7 @@ static void wpf_configure(struct vsp1_entity *entity,
* image height.
*/
if (wpf->flip.rotate)
- height = pipe->partition->width;
+ height = pipe->partition->wpf.width;
else
height = format->height;
@@ -471,10 +471,20 @@ static unsigned int wpf_max_width(struct vsp1_entity *entity,
return wpf->flip.rotate ? 256 : wpf->max_width;
}
+static void wpf_partition(struct vsp1_entity *entity,
+ struct vsp1_pipeline *pipe,
+ struct vsp1_partition *partition,
+ unsigned int partition_idx,
+ struct vsp1_partition_window *window)
+{
+ partition->wpf = *window;
+}
+
static const struct vsp1_entity_operations wpf_entity_ops = {
.destroy = vsp1_wpf_destroy,
.configure = wpf_configure,
.max_width = wpf_max_width,
+ .partition = wpf_partition,
};
/* -----------------------------------------------------------------------------
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread