lkml.org 
[lkml]   [2018]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 10/11] media: imx: Allow interweave with top/bottom lines swapped
From
Date
Hi Philipp,


On 10/05/2018 03:43 AM, Philipp Zabel wrote:
> Hi Steve,
>
> On Thu, 2018-10-04 at 11:54 -0700, Steve Longerbeam wrote:
>> Allow sequential->interlaced interweaving but with top/bottom
>> lines swapped to the output buffer.
>>
>> This can be accomplished by adding one line length to IDMAC output
>> channel address, with a negative line length for the interlace offset.
>>
>> This is to allow the seq-bt -> interlaced-bt transformation, where
>> bottom lines are still dominant (older in time) but with top lines
>> first in the interweaved output buffer.
>>
>> With this support, the CSI can now allow seq-bt at its source pads,
>> e.g. the following transformations are allowed in CSI from sink to
>> source:
>>
>> seq-tb -> seq-bt
>> seq-bt -> seq-bt
>> alternate -> seq-bt
>>
>> Suggested-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> drivers/staging/media/imx/imx-ic-prpencvf.c | 17 +++++++-
>> drivers/staging/media/imx/imx-media-csi.c | 46 +++++++++++++++++----
>> 2 files changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> index cf76b0432371..1499b0c62d74 100644
>> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
>> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> @@ -106,6 +106,8 @@ struct prp_priv {
>> u32 frame_sequence; /* frame sequence counter */
>> bool last_eof; /* waiting for last EOF at stream off */
>> bool nfb4eof; /* NFB4EOF encountered during streaming */
>> + u32 interweave_offset; /* interweave line offset to swap
>> + top/bottom lines */
> We have to store this instead of using vdev->fmt.fmt.bytesperline
> because this potentially is the pre-rotation stride instead?

interweave_offset was used by prp_vb2_buf_done() below, but in fact
that function is passed the non-rotation IDMAC channel (priv->out_ch)
_only_ if rotation is not enabled, so it is actually safe to use
vdev->fmt.fmt.bytesperline for the interweave offset in
prp_vb2_buf_done().

So I've gotten rid of interweave_offset in both imx-ic-prpencvf.c and
imx-media-csi.c, and replaced with a boolean interweave_swap as you
suggested. I agree it is much cleaner.


>>
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
>> index 679295da5dde..592f7d6edec1 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -114,6 +114,8 @@ struct csi_priv {
>> u32 frame_sequence; /* frame sequence counter */
>> bool last_eof; /* waiting for last EOF at stream off */
>> bool nfb4eof; /* NFB4EOF encountered during streaming */
>> + u32 interweave_offset; /* interweave line offset to swap
>> + top/bottom lines */
> This doesn't seem necessary. Since there is no rotation here, the offset
> is just vdev->fmt.fmt.pix.bytesperline if interweave_swap is enabled.
> Maybe turn this into a bool interweave_swap?

Agreed.

>
>> struct completion last_eof_comp;
>> };
>>
>> @@ -286,7 +288,8 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
>> if (ipu_idmac_buffer_is_ready(priv->idmac_ch, priv->ipu_buf_num))
>> ipu_idmac_clear_buffer(priv->idmac_ch, priv->ipu_buf_num);
>>
>> - ipu_cpmem_set_buffer(priv->idmac_ch, priv->ipu_buf_num, phys);
>> + ipu_cpmem_set_buffer(priv->idmac_ch, priv->ipu_buf_num,
>> + phys + priv->interweave_offset);
>> }
>>
>> static irqreturn_t csi_idmac_eof_interrupt(int irq, void *dev_id)
>> @@ -396,10 +399,10 @@ static void csi_idmac_unsetup_vb2_buf(struct csi_priv *priv,
>> static int csi_idmac_setup_channel(struct csi_priv *priv)
>> {
>> struct imx_media_video_dev *vdev = priv->vdev;
>> + bool passthrough, interweave, interweave_swap;
>> const struct imx_media_pixfmt *incc;
>> struct v4l2_mbus_framefmt *infmt;
>> struct v4l2_mbus_framefmt *outfmt;
>> - bool passthrough, interweave;
>> struct ipu_image image;
>> u32 passthrough_bits;
>> u32 passthrough_cycles;
>> @@ -433,6 +436,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>> */
>> interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
>> V4L2_FIELD_IS_SEQUENTIAL(outfmt->field);
>> + interweave_swap = interweave &&
>> + image.pix.field == V4L2_FIELD_INTERLACED_BT;
> Although this could just as well be recalculated in csi_vb2_buf_done.

In the future yes, when we add support for alternate mode (I assume
that's what you are getting at?).


> Apart from that,
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks.

Steve

\
 
 \ /
  Last update: 2018-10-09 03:08    [W:0.158 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site