[PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Marek Vasut]

In VDIC direct mode, the VDIC applies combing filter during and
doubles the framerate, that is, after the first two half-frames
are received and the first frame is emitted by the VDIC, every
subsequent half-frame is patched into the result and a full frame
is produced. The half-frame order in the full frames is as follows
12 32 34 54 etc.

Drop every second frame to trim the framerate back to the original
one of the signal and skip the odd patched frames.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/staging/media/imx/imx-media-vdic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 482250d47e7c..b538bbebedc5 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv)
 	/* set VDIC to receive from CSI for direct path */
 	ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT,
 		     IPUV3_CHANNEL_CSI_VDI_PREV);
+	ipu_set_vdi_skip(priv->ipu, 0x2);
 
 	return 0;
 }
@@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv)
 	const struct imx_media_pixfmt *incc;
 	int in_size, ret;
 
+	ipu_set_vdi_skip(priv->ipu, 0x0);
+
 	infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC];
 	incc = priv->cc[VDIC_SINK_PAD_IDMAC];
 
-- 
2.16.2

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Philipp Zabel]

On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote:
> In VDIC direct mode, the VDIC applies combing filter during and
> doubles the framerate, that is, after the first two half-frames
> are received and the first frame is emitted by the VDIC, every
> subsequent half-frame is patched into the result and a full frame
> is produced. The half-frame order in the full frames is as follows
> 12 32 34 54 etc.

Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2),
so I was under the impression that only data from the current field
makes it into the full frame. The missing lines should be purely
estimated from the available field using the di_vfilt 4-tap filter.

regards
Philipp

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Marek Vasut]

On 04/12/2018 12:04 PM, Philipp Zabel wrote:
> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote:
>> In VDIC direct mode, the VDIC applies combing filter during and
>> doubles the framerate, that is, after the first two half-frames
>> are received and the first frame is emitted by the VDIC, every
>> subsequent half-frame is patched into the result and a full frame
>> is produced. The half-frame order in the full frames is as follows
>> 12 32 34 54 etc.
> 
> Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2),
> so I was under the impression that only data from the current field
> makes it into the full frame. The missing lines should be purely
> estimated from the available field using the di_vfilt 4-tap filter.

Try using the VDIC within a pipeline directly:

        media-ctl -l "'ipu1_csi0':1->'ipu1_vdic':0[1]"
        media-ctl -l "'ipu1_vdic':2->'ipu1_ic_prp':0[1]"
        media-ctl -l "'ipu1_ic_prp':2->'ipu1_ic_prpvf':0[1]"
        media-ctl -l "'ipu1_ic_prpvf':1->'ipu1_ic_prpvf capture':0[1]"

-- 
Best regards,
Marek Vasut

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Steve Longerbeam]

On 04/12/2018 03:04 AM, Philipp Zabel wrote:
> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote:
>> In VDIC direct mode, the VDIC applies combing filter during and
>> doubles the framerate, that is, after the first two half-frames
>> are received and the first frame is emitted by the VDIC, every
>> subsequent half-frame is patched into the result and a full frame
>> is produced. The half-frame order in the full frames is as follows
>> 12 32 34 54 etc.
> Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2),
> so I was under the impression that only data from the current field
> makes it into the full frame. The missing lines should be purely
> estimated from the available field using the di_vfilt 4-tap filter.

Yes, the reference manual states that the VDI_MOT_SEL field
value 2 is "Full motion, only vertical filter is used". Which is
clearly referring to the di_vfilt 4-tap filter.

There are still some questions about how full motion mode is
supposed to work. For one thing, the di_vfilt only operates on four
consecutive lines of a single field. It makes no sense that the VDIC
can compensate for motion between fields when it is operating
with only one field.

Marek, where did you get the information that full motion mode
applies some kind of combing filter? A combing filter would imply
taking previous and/or future fields back into the result, which is
exactly what the other motion modes do, but as I said the reference
manual is clear that full motion mode uses only the (single field)
vertical filter.

The manual also mentions regarding "real-time mode" which we are
making use of (in which the VDIC FIFO1 receives field F(n-1) directly
from the CSI rather than from DMA):

"In addition IDMAC read the field from FIFO1 and store in external memory.
Then stored frames are used as F(n) and F(n+1).".

It is nowhere explicitly stated, but the assumption is that this is IDMAC
channel 13 that stores the CSI field to memory. But many people have
asked Freescale/NXP how this works in the past, and there has never
been a satisfactory answer. And people have reported no success in
getting this channel to work including myself.

So the approach this driver takes is to use real-time mode to receive
F(n-1) directly from CSI, in concert with full motion mode, so that
the VDIC operates on F(n-1) only. Thus no DMA is necessary.

Finally I have to say that the other modes are supported in this driver,
but they require DMA transfer of all three fields, and there is no
output device node written to make use of those modes yet. But
from experience, the de-interlaced result is of much better quality
than the real-time/full-motion output.


Steve

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Marek Vasut]

On 04/21/2018 11:29 PM, Steve Longerbeam wrote:
> 
> 
> On 04/12/2018 03:04 AM, Philipp Zabel wrote:
>> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote:
>>> In VDIC direct mode, the VDIC applies combing filter during and
>>> doubles the framerate, that is, after the first two half-frames
>>> are received and the first frame is emitted by the VDIC, every
>>> subsequent half-frame is patched into the result and a full frame
>>> is produced. The half-frame order in the full frames is as follows
>>> 12 32 34 54 etc.
>> Is that true? We are only supporting full motion mode (VDI_MOT_SEL=2),
>> so I was under the impression that only data from the current field
>> makes it into the full frame. The missing lines should be purely
>> estimated from the available field using the di_vfilt 4-tap filter.
> 
> Yes, the reference manual states that the VDI_MOT_SEL field
> value 2 is "Full motion, only vertical filter is used". Which is
> clearly referring to the di_vfilt 4-tap filter.
> 
> There are still some questions about how full motion mode is
> supposed to work. For one thing, the di_vfilt only operates on four
> consecutive lines of a single field. It makes no sense that the VDIC
> can compensate for motion between fields when it is operating
> with only one field.
> 
> Marek, where did you get the information that full motion mode
> applies some kind of combing filter?

By observing how the HW behaves. The input from NXP forum was mostly
useless or actually outright wrong. I have to admit it's been a while
since I created the patch, but what I saw was basically the hardware
producing frames as a combination of halfframe 1-2 2-3 3-4 etc , thus
doubling the framerate. Setting the skip normalized the framerate
without any loss of image information.

> A combing filter would imply
> taking previous and/or future fields back into the result, which is
> exactly what the other motion modes do, but as I said the reference
> manual is clear that full motion mode uses only the (single field)
> vertical filter.
> 
> The manual also mentions regarding "real-time mode" which we are
> making use of (in which the VDIC FIFO1 receives field F(n-1) directly
> from the CSI rather than from DMA):
> 
> "In addition IDMAC read the field from FIFO1 and store in external memory.
> Then stored frames are used as F(n) and F(n+1).".
> 
> It is nowhere explicitly stated, but the assumption is that this is IDMAC
> channel 13 that stores the CSI field to memory. But many people have
> asked Freescale/NXP how this works in the past, and there has never
> been a satisfactory answer. And people have reported no success in
> getting this channel to work including myself.
> 
> So the approach this driver takes is to use real-time mode to receive
> F(n-1) directly from CSI, in concert with full motion mode, so that
> the VDIC operates on F(n-1) only. Thus no DMA is necessary.
> 
> Finally I have to say that the other modes are supported in this driver,
> but they require DMA transfer of all three fields, and there is no
> output device node written to make use of those modes yet. But
> from experience, the de-interlaced result is of much better quality
> than the real-time/full-motion output.
> 
> 
> Steve
> 
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Hans Verkuil]

On 07/04/18 15:04, Marek Vasut wrote:
> In VDIC direct mode, the VDIC applies combing filter during and
> doubles the framerate, that is, after the first two half-frames
> are received and the first frame is emitted by the VDIC, every
> subsequent half-frame is patched into the result and a full frame
> is produced. The half-frame order in the full frames is as follows
> 12 32 34 54 etc.
> 
> Drop every second frame to trim the framerate back to the original
> one of the signal and skip the odd patched frames.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>

Steve, Philipp,

I saw there was a discussion about this patch, but no clear answer whether
or not this patch is OK. If it is, then please Ack this patch.

Regards,

	Hans

> ---
>  drivers/staging/media/imx/imx-media-vdic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
> index 482250d47e7c..b538bbebedc5 100644
> --- a/drivers/staging/media/imx/imx-media-vdic.c
> +++ b/drivers/staging/media/imx/imx-media-vdic.c
> @@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv)
>  	/* set VDIC to receive from CSI for direct path */
>  	ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT,
>  		     IPUV3_CHANNEL_CSI_VDI_PREV);
> +	ipu_set_vdi_skip(priv->ipu, 0x2);
>  
>  	return 0;
>  }
> @@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv)
>  	const struct imx_media_pixfmt *incc;
>  	int in_size, ret;
>  
> +	ipu_set_vdi_skip(priv->ipu, 0x0);
> +
>  	infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC];
>  	incc = priv->cc[VDIC_SINK_PAD_IDMAC];
>  
> 

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Hans Verkuil]

On 05/07/2018 11:54 AM, Hans Verkuil wrote:
> On 07/04/18 15:04, Marek Vasut wrote:
>> In VDIC direct mode, the VDIC applies combing filter during and
>> doubles the framerate, that is, after the first two half-frames
>> are received and the first frame is emitted by the VDIC, every
>> subsequent half-frame is patched into the result and a full frame
>> is produced. The half-frame order in the full frames is as follows
>> 12 32 34 54 etc.
>>
>> Drop every second frame to trim the framerate back to the original
>> one of the signal and skip the odd patched frames.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> 
> Steve, Philipp,
> 
> I saw there was a discussion about this patch, but no clear answer whether
> or not this patch is OK. If it is, then please Ack this patch.

Marking this patch as Obsoleted since I have no seen any activity for a long time.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>> ---
>>  drivers/staging/media/imx/imx-media-vdic.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
>> index 482250d47e7c..b538bbebedc5 100644
>> --- a/drivers/staging/media/imx/imx-media-vdic.c
>> +++ b/drivers/staging/media/imx/imx-media-vdic.c
>> @@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv)
>>  	/* set VDIC to receive from CSI for direct path */
>>  	ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT,
>>  		     IPUV3_CHANNEL_CSI_VDI_PREV);
>> +	ipu_set_vdi_skip(priv->ipu, 0x2);
>>  
>>  	return 0;
>>  }
>> @@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv)
>>  	const struct imx_media_pixfmt *incc;
>>  	int in_size, ret;
>>  
>> +	ipu_set_vdi_skip(priv->ipu, 0x0);
>> +
>>  	infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC];
>>  	incc = priv->cc[VDIC_SINK_PAD_IDMAC];
>>  
>>
> 
> 

Re: [PATCH] media: imx: Skip every second frame in VDIC DIRECT mode

[Steve Longerbeam]

On 09/17/2018 03:27 AM, Hans Verkuil wrote:
> On 05/07/2018 11:54 AM, Hans Verkuil wrote:
>> On 07/04/18 15:04, Marek Vasut wrote:
>>> In VDIC direct mode, the VDIC applies combing filter during and
>>> doubles the framerate, that is, after the first two half-frames
>>> are received and the first frame is emitted by the VDIC, every
>>> subsequent half-frame is patched into the result and a full frame
>>> is produced. The half-frame order in the full frames is as follows
>>> 12 32 34 54 etc.
>>>
>>> Drop every second frame to trim the framerate back to the original
>>> one of the signal and skip the odd patched frames.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Steve Longerbeam <steve_longerbeam@mentor.com>
>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Steve, Philipp,
>>
>> I saw there was a discussion about this patch, but no clear answer whether
>> or not this patch is OK. If it is, then please Ack this patch.
> Marking this patch as Obsoleted since I have no seen any activity for a long time.

Hi Hans, yes that's fine.

This needs to be re-worked to allow configuration of input/output 
frame-rates
from the VDIC via [gs]_frame_interval.

Steve

>
>
>>> ---
>>>   drivers/staging/media/imx/imx-media-vdic.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
>>> index 482250d47e7c..b538bbebedc5 100644
>>> --- a/drivers/staging/media/imx/imx-media-vdic.c
>>> +++ b/drivers/staging/media/imx/imx-media-vdic.c
>>> @@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv)
>>>   	/* set VDIC to receive from CSI for direct path */
>>>   	ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT,
>>>   		     IPUV3_CHANNEL_CSI_VDI_PREV);
>>> +	ipu_set_vdi_skip(priv->ipu, 0x2);
>>>   
>>>   	return 0;
>>>   }
>>> @@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv)
>>>   	const struct imx_media_pixfmt *incc;
>>>   	int in_size, ret;
>>>   
>>> +	ipu_set_vdi_skip(priv->ipu, 0x0);
>>> +
>>>   	infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC];
>>>   	incc = priv->cc[VDIC_SINK_PAD_IDMAC];
>>>   
>>>
>>