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 <[email protected]>
Signed-off-by: Steve Longerbeam <[email protected]>
---
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 */
struct completion last_eof_comp;
};
@@ -235,6 +237,9 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch)
if (ipu_idmac_buffer_is_ready(ch, priv->ipu_buf_num))
ipu_idmac_clear_buffer(ch, priv->ipu_buf_num);
+ if (ch == priv->out_ch)
+ phys += priv->interweave_offset;
+
ipu_cpmem_set_buffer(ch, priv->ipu_buf_num, phys);
}
@@ -388,6 +393,13 @@ static int prp_setup_channel(struct prp_priv *priv,
(image.pix.width * outcc->bpp) >> 3;
}
+ priv->interweave_offset = 0;
+
+ if (interweave && image.pix.field == V4L2_FIELD_INTERLACED_BT) {
+ image.rect.top = 1;
+ priv->interweave_offset = image.pix.bytesperline;
+ }
+
image.phys0 = addr0;
image.phys1 = addr1;
@@ -425,7 +437,10 @@ static int prp_setup_channel(struct prp_priv *priv,
ipu_cpmem_set_rotation(channel, rot_mode);
if (interweave)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ ipu_cpmem_interlaced_scan(channel,
+ priv->interweave_offset ?
+ -image.pix.bytesperline :
+ image.pix.bytesperline,
image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
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 */
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;
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_SBGGR8:
@@ -486,15 +491,25 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
}
if (passthrough) {
+ priv->interweave_offset = interweave_swap ?
+ image.pix.bytesperline : 0;
ipu_cpmem_set_resolution(priv->idmac_ch,
image.rect.width * passthrough_cycles,
image.rect.height);
ipu_cpmem_set_stride(priv->idmac_ch, image.pix.bytesperline);
- ipu_cpmem_set_buffer(priv->idmac_ch, 0, image.phys0);
- ipu_cpmem_set_buffer(priv->idmac_ch, 1, image.phys1);
+ ipu_cpmem_set_buffer(priv->idmac_ch, 0,
+ image.phys0 + priv->interweave_offset);
+ ipu_cpmem_set_buffer(priv->idmac_ch, 1,
+ image.phys1 + priv->interweave_offset);
ipu_cpmem_set_format_passthrough(priv->idmac_ch,
passthrough_bits);
} else {
+ priv->interweave_offset = 0;
+ if (interweave_swap) {
+ image.rect.top = 1;
+ priv->interweave_offset = image.pix.bytesperline;
+ }
+
ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
if (ret)
goto unsetup_vb2;
@@ -526,6 +541,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (interweave)
ipu_cpmem_interlaced_scan(priv->idmac_ch,
+ priv->interweave_offset ?
+ -image.pix.bytesperline :
image.pix.bytesperline,
image.pix.pixelformat);
@@ -1334,16 +1351,27 @@ static void csi_try_field(struct csi_priv *priv,
switch (infmt->field) {
case V4L2_FIELD_SEQ_TB:
case V4L2_FIELD_SEQ_BT:
+ /*
+ * If the user requests sequential at the source pad,
+ * allow it (along with possibly inverting field order).
+ * Otherwise passthrough the field type.
+ */
+ if (!V4L2_FIELD_IS_SEQUENTIAL(sdformat->format.field))
+ sdformat->format.field = infmt->field;
+ break;
case V4L2_FIELD_ALTERNATE:
/*
- * If the sink is sequential or alternating fields,
- * allow only SEQ_TB at the source.
- *
* This driver does not support alternate field mode, and
* the CSI captures a whole frame, so the CSI never presents
- * alternate mode at its source pads.
+ * alternate mode at its source pads. If user has not
+ * already requested sequential, translate ALTERNATE at
+ * sink pad to SEQ_TB or SEQ_BT at the source pad depending
+ * on input height (assume NTSC BT order if 480 total active
+ * frame lines, otherwise PAL TB order).
*/
- sdformat->format.field = V4L2_FIELD_SEQ_TB;
+ if (!V4L2_FIELD_IS_SEQUENTIAL(sdformat->format.field))
+ sdformat->format.field = (infmt->height == 480 / 2) ?
+ V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB;
break;
default:
/* Passthrough for all other input field types */
--
2.17.1
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 <[email protected]>
> Signed-off-by: Steve Longerbeam <[email protected]>
> ---
> 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?
> struct completion last_eof_comp;
> };
>
> @@ -235,6 +237,9 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct ipuv3_channel *ch)
> if (ipu_idmac_buffer_is_ready(ch, priv->ipu_buf_num))
> ipu_idmac_clear_buffer(ch, priv->ipu_buf_num);
>
> + if (ch == priv->out_ch)
> + phys += priv->interweave_offset;
> +
> ipu_cpmem_set_buffer(ch, priv->ipu_buf_num, phys);
> }
>
> @@ -388,6 +393,13 @@ static int prp_setup_channel(struct prp_priv *priv,
> (image.pix.width * outcc->bpp) >> 3;
> }
>
> + priv->interweave_offset = 0;
> +
> + if (interweave && image.pix.field == V4L2_FIELD_INTERLACED_BT) {
> + image.rect.top = 1;
> + priv->interweave_offset = image.pix.bytesperline;
> + }
> +
> image.phys0 = addr0;
> image.phys1 = addr1;
>
> @@ -425,7 +437,10 @@ static int prp_setup_channel(struct prp_priv *priv,
> ipu_cpmem_set_rotation(channel, rot_mode);
>
> if (interweave)
> - ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
> + ipu_cpmem_interlaced_scan(channel,
> + priv->interweave_offset ?
> + -image.pix.bytesperline :
> + image.pix.bytesperline,
> image.pix.pixelformat);
>
> ret = ipu_ic_task_idma_init(priv->ic, channel,
> 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?
> 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.
Apart from that,
Reviewed-by: Philipp Zabel <[email protected]>
regards
Philipp
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 <[email protected]>
>> Signed-off-by: Steve Longerbeam <[email protected]>
>> ---
>> 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 <[email protected]>
Thanks.
Steve