* [PATCH v5 01/12] media: videodev2.h: Add more field helper macros
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` Steve Longerbeam
` (11 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media; +Cc: Steve Longerbeam, Mauro Carvalho Chehab, open list
Adds two helper macros:
V4L2_FIELD_IS_SEQUENTIAL: returns true if the given field type is
'sequential', that is a full frame is transmitted, or exists in
memory, as all top field lines followed by all bottom field lines,
or vice-versa.
V4L2_FIELD_IS_INTERLACED: returns true if the given field type is
'interlaced', that is a full frame is transmitted, or exists in
memory, as top field lines interlaced with bottom field lines.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes since v3:
- none
Changes since v2:
- none
Changes since v1:
- add the complement macro V4L2_FIELD_IS_INTERLACED
- remove V4L2_FIELD_ALTERNATE from V4L2_FIELD_IS_SEQUENTIAL macro.
- moved new macros past end of existing V4L2_FIELD_HAS_* macros.
---
include/uapi/linux/videodev2.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 29729d580452..f7f031736d91 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -130,6 +130,13 @@ enum v4l2_field {
((field) == V4L2_FIELD_BOTTOM ||\
(field) == V4L2_FIELD_TOP ||\
(field) == V4L2_FIELD_ALTERNATE)
+#define V4L2_FIELD_IS_INTERLACED(field) \
+ ((field) == V4L2_FIELD_INTERLACED ||\
+ (field) == V4L2_FIELD_INTERLACED_TB ||\
+ (field) == V4L2_FIELD_INTERLACED_BT)
+#define V4L2_FIELD_IS_SEQUENTIAL(field) \
+ ((field) == V4L2_FIELD_SEQ_TB ||\
+ (field) == V4L2_FIELD_SEQ_BT)
enum v4l2_buf_type {
V4L2_BUF_TYPE_VIDEO_CAPTURE = 1,
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 01/12] media: videodev2.h: Add more field helper macros Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` Steve Longerbeam
` (10 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.
Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().
Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).
When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes since v4:
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
---
drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
drivers/staging/media/imx/imx-media-csi.c | 17 +---
include/video/imx-ipu-v3.h | 3 +-
3 files changed, 88 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index aa0e30a2ba18..4a15e513fa05 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
return 0;
}
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+ return (field != V4L2_FIELD_ALTERNATE) ? field :
+ ((std & V4L2_STD_525_60) ?
+ V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
/*
* Fill a CSI bus config struct from mbus_config and mbus_framefmt.
*/
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
return 0;
}
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt,
+ v4l2_std_id std)
+{
+ enum v4l2_field infield, outfield;
+ bool swap_fields;
+
+ /* get translated field type of input and output */
+ infield = ipu_csi_translate_field(infmt->field, std);
+ outfield = ipu_csi_translate_field(outfmt->field, std);
+
+ /*
+ * Write the H-V-F codes the CSI will match against the
+ * incoming data for start/end of active and blanking
+ * field intervals. If input and output field types are
+ * sequential but not the same (one is SEQ_BT and the other
+ * is SEQ_TB), swap the F-bit so that the CSI will capture
+ * field 1 lines before field 0 lines.
+ */
+ swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+ infield != outfield);
+
+ if (!swap_fields) {
+ /*
+ * Field0BlankEnd = 110, Field0BlankStart = 010
+ * Field0ActiveEnd = 100, Field0ActiveStart = 000
+ * Field1BlankEnd = 111, Field1BlankStart = 011
+ * Field1ActiveEnd = 101, Field1ActiveStart = 001
+ */
+ ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+ } else {
+ dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+ /* same as above but with F-bit inverted */
+ ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+ }
+
+ ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+
+ return 0;
+}
+
+
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt)
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt)
{
struct ipu_csi_bus_config cfg;
unsigned long flags;
u32 width, height, data = 0;
+ v4l2_std_id std;
int ret;
- ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
+ ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
if (ret < 0)
return ret;
/* set default sensor frame width and height */
- width = mbus_fmt->width;
- height = mbus_fmt->height;
+ width = infmt->width;
+ height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ height *= 2;
/* Set the CSI_SENS_CONF register remaining fields */
data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
@@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
break;
case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
- if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
- /*
- * PAL case
- *
- * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
- * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
- * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
- * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
- */
- height = 625; /* framelines for PAL */
-
- ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
- } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
- /*
- * NTSC case
- *
- * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
- * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
- * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
- * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
- */
- height = 525; /* framelines for NTSC */
-
- ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+ if (width == 720 && height == 480) {
+ std = V4L2_STD_NTSC;
+ height = 525;
+ } else if (width == 720 && height == 576) {
+ std = V4L2_STD_PAL;
+ height = 625;
} else {
dev_err(csi->ipu->dev,
- "Unsupported CCIR656 interlaced video mode\n");
- spin_unlock_irqrestore(&csi->lock, flags);
- return -EINVAL;
+ "Unsupported interlaced video mode\n");
+ ret = -EINVAL;
+ goto out_unlock;
}
+
+ ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
+ if (ret)
+ goto out_unlock;
break;
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
@@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
+out_unlock:
spin_unlock_irqrestore(&csi->lock, flags);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..7ecbd4d76d09 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
/* Update the CSI whole sensor and active windows */
static int csi_setup(struct csi_priv *priv)
{
- struct v4l2_mbus_framefmt *infmt, *outfmt;
+ struct v4l2_mbus_framefmt infmt, outfmt;
const struct imx_media_pixfmt *incc;
struct v4l2_mbus_config mbus_cfg;
- struct v4l2_mbus_framefmt if_fmt;
struct v4l2_rect crop;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
+ infmt = priv->format_mbus[CSI_SINK_PAD];
incc = priv->cc[CSI_SINK_PAD];
- outfmt = &priv->format_mbus[priv->active_output_pad];
+ outfmt = priv->format_mbus[priv->active_output_pad];
/* compose mbus_config from the upstream endpoint */
mbus_cfg.type = priv->upstream_ep.bus_type;
@@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
priv->upstream_ep.bus.parallel.flags :
priv->upstream_ep.bus.mipi_csi2.flags;
- /*
- * we need to pass input frame to CSI interface, but
- * with translated field type from output format
- */
- if_fmt = *infmt;
- if_fmt.field = outfmt->field;
crop = priv->crop;
/*
@@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
* generic/bayer data
*/
if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
- if_fmt.width *= incc->cycles;
+ infmt.width *= incc->cycles;
crop.width *= incc->cycles;
}
@@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
priv->crop.width == 2 * priv->compose.width,
priv->crop.height == 2 * priv->compose.height);
- ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+ ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
ipu_csi_set_dest(priv->csi, priv->dest);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index abbad94e14a1..f44a35192313 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
struct ipu_csi;
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt);
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt);
bool ipu_csi_is_interlaced(struct ipu_csi *csi);
void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-10-17 0:00 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Philipp Zabel,
Steve Longerbeam, Mauro Carvalho Chehab
The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.
Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().
Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).
When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes since v4:
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
---
drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
drivers/staging/media/imx/imx-media-csi.c | 17 +---
include/video/imx-ipu-v3.h | 3 +-
3 files changed, 88 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index aa0e30a2ba18..4a15e513fa05 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
return 0;
}
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+ return (field != V4L2_FIELD_ALTERNATE) ? field :
+ ((std & V4L2_STD_525_60) ?
+ V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
/*
* Fill a CSI bus config struct from mbus_config and mbus_framefmt.
*/
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
return 0;
}
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt,
+ v4l2_std_id std)
+{
+ enum v4l2_field infield, outfield;
+ bool swap_fields;
+
+ /* get translated field type of input and output */
+ infield = ipu_csi_translate_field(infmt->field, std);
+ outfield = ipu_csi_translate_field(outfmt->field, std);
+
+ /*
+ * Write the H-V-F codes the CSI will match against the
+ * incoming data for start/end of active and blanking
+ * field intervals. If input and output field types are
+ * sequential but not the same (one is SEQ_BT and the other
+ * is SEQ_TB), swap the F-bit so that the CSI will capture
+ * field 1 lines before field 0 lines.
+ */
+ swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+ infield != outfield);
+
+ if (!swap_fields) {
+ /*
+ * Field0BlankEnd = 110, Field0BlankStart = 010
+ * Field0ActiveEnd = 100, Field0ActiveStart = 000
+ * Field1BlankEnd = 111, Field1BlankStart = 011
+ * Field1ActiveEnd = 101, Field1ActiveStart = 001
+ */
+ ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+ } else {
+ dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+ /* same as above but with F-bit inverted */
+ ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+ }
+
+ ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+
+ return 0;
+}
+
+
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt)
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt)
{
struct ipu_csi_bus_config cfg;
unsigned long flags;
u32 width, height, data = 0;
+ v4l2_std_id std;
int ret;
- ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
+ ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
if (ret < 0)
return ret;
/* set default sensor frame width and height */
- width = mbus_fmt->width;
- height = mbus_fmt->height;
+ width = infmt->width;
+ height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ height *= 2;
/* Set the CSI_SENS_CONF register remaining fields */
data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
@@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
break;
case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
- if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
- /*
- * PAL case
- *
- * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
- * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
- * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
- * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
- */
- height = 625; /* framelines for PAL */
-
- ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
- } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
- /*
- * NTSC case
- *
- * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
- * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
- * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
- * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
- */
- height = 525; /* framelines for NTSC */
-
- ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+ if (width == 720 && height == 480) {
+ std = V4L2_STD_NTSC;
+ height = 525;
+ } else if (width == 720 && height == 576) {
+ std = V4L2_STD_PAL;
+ height = 625;
} else {
dev_err(csi->ipu->dev,
- "Unsupported CCIR656 interlaced video mode\n");
- spin_unlock_irqrestore(&csi->lock, flags);
- return -EINVAL;
+ "Unsupported interlaced video mode\n");
+ ret = -EINVAL;
+ goto out_unlock;
}
+
+ ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
+ if (ret)
+ goto out_unlock;
break;
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
@@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
+out_unlock:
spin_unlock_irqrestore(&csi->lock, flags);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..7ecbd4d76d09 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
/* Update the CSI whole sensor and active windows */
static int csi_setup(struct csi_priv *priv)
{
- struct v4l2_mbus_framefmt *infmt, *outfmt;
+ struct v4l2_mbus_framefmt infmt, outfmt;
const struct imx_media_pixfmt *incc;
struct v4l2_mbus_config mbus_cfg;
- struct v4l2_mbus_framefmt if_fmt;
struct v4l2_rect crop;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
+ infmt = priv->format_mbus[CSI_SINK_PAD];
incc = priv->cc[CSI_SINK_PAD];
- outfmt = &priv->format_mbus[priv->active_output_pad];
+ outfmt = priv->format_mbus[priv->active_output_pad];
/* compose mbus_config from the upstream endpoint */
mbus_cfg.type = priv->upstream_ep.bus_type;
@@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
priv->upstream_ep.bus.parallel.flags :
priv->upstream_ep.bus.mipi_csi2.flags;
- /*
- * we need to pass input frame to CSI interface, but
- * with translated field type from output format
- */
- if_fmt = *infmt;
- if_fmt.field = outfmt->field;
crop = priv->crop;
/*
@@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
* generic/bayer data
*/
if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
- if_fmt.width *= incc->cycles;
+ infmt.width *= incc->cycles;
crop.width *= incc->cycles;
}
@@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
priv->crop.width == 2 * priv->compose.width,
priv->crop.height == 2 * priv->compose.height);
- ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+ ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
ipu_csi_set_dest(priv->csi, priv->dest);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index abbad94e14a1..f44a35192313 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
struct ipu_csi;
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt);
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt);
bool ipu_csi_is_interlaced(struct ipu_csi *csi);
void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-10-17 0:00 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Philipp Zabel,
Steve Longerbeam, Mauro Carvalho Chehab
The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.
Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().
Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).
When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes since v4:
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
---
drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
drivers/staging/media/imx/imx-media-csi.c | 17 +---
include/video/imx-ipu-v3.h | 3 +-
3 files changed, 88 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index aa0e30a2ba18..4a15e513fa05 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
return 0;
}
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+ return (field != V4L2_FIELD_ALTERNATE) ? field :
+ ((std & V4L2_STD_525_60) ?
+ V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
/*
* Fill a CSI bus config struct from mbus_config and mbus_framefmt.
*/
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
return 0;
}
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt,
+ v4l2_std_id std)
+{
+ enum v4l2_field infield, outfield;
+ bool swap_fields;
+
+ /* get translated field type of input and output */
+ infield = ipu_csi_translate_field(infmt->field, std);
+ outfield = ipu_csi_translate_field(outfmt->field, std);
+
+ /*
+ * Write the H-V-F codes the CSI will match against the
+ * incoming data for start/end of active and blanking
+ * field intervals. If input and output field types are
+ * sequential but not the same (one is SEQ_BT and the other
+ * is SEQ_TB), swap the F-bit so that the CSI will capture
+ * field 1 lines before field 0 lines.
+ */
+ swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+ infield != outfield);
+
+ if (!swap_fields) {
+ /*
+ * Field0BlankEnd = 110, Field0BlankStart = 010
+ * Field0ActiveEnd = 100, Field0ActiveStart = 000
+ * Field1BlankEnd = 111, Field1BlankStart = 011
+ * Field1ActiveEnd = 101, Field1ActiveStart = 001
+ */
+ ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+ } else {
+ dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+ /* same as above but with F-bit inverted */
+ ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+ }
+
+ ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+
+ return 0;
+}
+
+
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt)
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt)
{
struct ipu_csi_bus_config cfg;
unsigned long flags;
u32 width, height, data = 0;
+ v4l2_std_id std;
int ret;
- ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
+ ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
if (ret < 0)
return ret;
/* set default sensor frame width and height */
- width = mbus_fmt->width;
- height = mbus_fmt->height;
+ width = infmt->width;
+ height = infmt->height;
+ if (infmt->field = V4L2_FIELD_ALTERNATE)
+ height *= 2;
/* Set the CSI_SENS_CONF register remaining fields */
data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
@@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
break;
case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
- if (mbus_fmt->width = 720 && mbus_fmt->height = 576) {
- /*
- * PAL case
- *
- * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
- * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
- * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
- * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
- */
- height = 625; /* framelines for PAL */
-
- ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
- } else if (mbus_fmt->width = 720 && mbus_fmt->height = 480) {
- /*
- * NTSC case
- *
- * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
- * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
- * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
- * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
- */
- height = 525; /* framelines for NTSC */
-
- ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
- CSI_CCIR_CODE_1);
- ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
- ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
+ if (width = 720 && height = 480) {
+ std = V4L2_STD_NTSC;
+ height = 525;
+ } else if (width = 720 && height = 576) {
+ std = V4L2_STD_PAL;
+ height = 625;
} else {
dev_err(csi->ipu->dev,
- "Unsupported CCIR656 interlaced video mode\n");
- spin_unlock_irqrestore(&csi->lock, flags);
- return -EINVAL;
+ "Unsupported interlaced video mode\n");
+ ret = -EINVAL;
+ goto out_unlock;
}
+
+ ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
+ if (ret)
+ goto out_unlock;
break;
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
@@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
+out_unlock:
spin_unlock_irqrestore(&csi->lock, flags);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..7ecbd4d76d09 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
/* Update the CSI whole sensor and active windows */
static int csi_setup(struct csi_priv *priv)
{
- struct v4l2_mbus_framefmt *infmt, *outfmt;
+ struct v4l2_mbus_framefmt infmt, outfmt;
const struct imx_media_pixfmt *incc;
struct v4l2_mbus_config mbus_cfg;
- struct v4l2_mbus_framefmt if_fmt;
struct v4l2_rect crop;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
+ infmt = priv->format_mbus[CSI_SINK_PAD];
incc = priv->cc[CSI_SINK_PAD];
- outfmt = &priv->format_mbus[priv->active_output_pad];
+ outfmt = priv->format_mbus[priv->active_output_pad];
/* compose mbus_config from the upstream endpoint */
mbus_cfg.type = priv->upstream_ep.bus_type;
@@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
priv->upstream_ep.bus.parallel.flags :
priv->upstream_ep.bus.mipi_csi2.flags;
- /*
- * we need to pass input frame to CSI interface, but
- * with translated field type from output format
- */
- if_fmt = *infmt;
- if_fmt.field = outfmt->field;
crop = priv->crop;
/*
@@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
* generic/bayer data
*/
if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
- if_fmt.width *= incc->cycles;
+ infmt.width *= incc->cycles;
crop.width *= incc->cycles;
}
@@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
priv->crop.width = 2 * priv->compose.width,
priv->crop.height = 2 * priv->compose.height);
- ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+ ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
ipu_csi_set_dest(priv->csi, priv->dest);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index abbad94e14a1..f44a35192313 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
struct ipu_csi;
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt);
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt);
bool ipu_csi_is_interlaced(struct ipu_csi *csi);
void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
2018-10-17 0:00 ` Steve Longerbeam
(?)
@ 2018-12-07 20:02 ` Steve Longerbeam
-1 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-07 20:02 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
Hi Philipp, can you review this patch and give it your ack?
Thanks,
Steve
On 10/16/18 5:00 PM, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field == V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width == 720 && height == 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width == 720 && height == 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width == 2 * priv->compose.width,
> priv->crop.height == 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-07 20:02 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-07 20:02 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Philipp Zabel,
Mauro Carvalho Chehab
Hi Philipp, can you review this patch and give it your ack?
Thanks,
Steve
On 10/16/18 5:00 PM, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field == V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width == 720 && height == 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width == 720 && height == 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width == 2 * priv->compose.width,
> priv->crop.height == 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-07 20:02 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-07 20:02 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Philipp Zabel,
Mauro Carvalho Chehab
Hi Philipp, can you review this patch and give it your ack?
Thanks,
Steve
On 10/16/18 5:00 PM, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field = V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width = 720 && mbus_fmt->height = 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width = 720 && mbus_fmt->height = 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width = 720 && height = 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width = 720 && height = 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width = 2 * priv->compose.width,
> priv->crop.height = 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
2018-10-17 0:00 ` Steve Longerbeam
(?)
@ 2018-12-13 12:59 ` Philipp Zabel
-1 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2018-12-13 12:59 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman,
Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
Hi Steve,
On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
infmt and outfmt parameters could be const.
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field == V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width == 720 && height == 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width == 720 && height == 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
just the userspace visible input format as determined by the pad, but
the modified format we set the interface to? Not a strong preference,
though.
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
Copying the output format onto the stack seems unnecessary to me.
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width == 2 * priv->compose.width,
> priv->crop.height == 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
We can just pass the &priv->format_mbus[priv->active_output_pad] here,
or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
Either way,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-13 12:59 ` Philipp Zabel
0 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2018-12-13 12:59 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Mauro Carvalho Chehab
Hi Steve,
On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
infmt and outfmt parameters could be const.
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field == V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width == 720 && height == 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width == 720 && height == 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
just the userspace visible input format as determined by the pad, but
the modified format we set the interface to? Not a strong preference,
though.
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
Copying the output format onto the stack seems unnecessary to me.
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width == 2 * priv->compose.width,
> priv->crop.height == 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
We can just pass the &priv->format_mbus[priv->active_output_pad] here,
or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
Either way,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-13 12:59 ` Philipp Zabel
0 siblings, 0 replies; 28+ messages in thread
From: Philipp Zabel @ 2018-12-13 12:59 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Cc: open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman, open list,
open list:DRM DRIVERS FOR FREESCALE IMX, Mauro Carvalho Chehab
Hi Steve,
On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
> The function ipu_csi_init_interface() was inverting the F-bit for
> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
> for NTSC bottom-top field order, the CSI would swap fields and
> capture in top-bottom order.
>
> Instead, base field swap on the field order of the input to the CSI,
> and the field order of the requested output. If the input/output
> fields are sequential but different, swap fields, otherwise do
> not swap. This requires passing both the input and output mbus
> frame formats to ipu_csi_init_interface().
>
> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
> possibly interlaced BT.1120 in the future).
>
> When detecting input video standard from the input frame width/height,
> make sure to double height if input field type is alternate, since
> in that case input height only includes lines for one field.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> Changes since v4:
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> ---
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
> drivers/staging/media/imx/imx-media-csi.c | 17 +---
> include/video/imx-ipu-v3.h | 3 +-
> 3 files changed, 88 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index aa0e30a2ba18..4a15e513fa05 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
> return 0;
> }
>
> +/* translate alternate field mode based on given standard */
> +static inline enum v4l2_field
> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
> +{
> + return (field != V4L2_FIELD_ALTERNATE) ? field :
> + ((std & V4L2_STD_525_60) ?
> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
> +}
> +
> /*
> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
> */
> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
> return 0;
> }
>
> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt,
infmt and outfmt parameters could be const.
> + v4l2_std_id std)
> +{
> + enum v4l2_field infield, outfield;
> + bool swap_fields;
> +
> + /* get translated field type of input and output */
> + infield = ipu_csi_translate_field(infmt->field, std);
> + outfield = ipu_csi_translate_field(outfmt->field, std);
> +
> + /*
> + * Write the H-V-F codes the CSI will match against the
> + * incoming data for start/end of active and blanking
> + * field intervals. If input and output field types are
> + * sequential but not the same (one is SEQ_BT and the other
> + * is SEQ_TB), swap the F-bit so that the CSI will capture
> + * field 1 lines before field 0 lines.
> + */
> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
> + infield != outfield);
> +
> + if (!swap_fields) {
> + /*
> + * Field0BlankEnd = 110, Field0BlankStart = 010
> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
> + * Field1BlankEnd = 111, Field1BlankStart = 011
> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
> + */
> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> + } else {
> + dev_dbg(csi->ipu->dev, "capture field swap\n");
> +
> + /* same as above but with F-bit inverted */
> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> + CSI_CCIR_CODE_1);
> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> + }
> +
> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> +
> + return 0;
> +}
> +
> +
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt)
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt)
> {
> struct ipu_csi_bus_config cfg;
> unsigned long flags;
> u32 width, height, data = 0;
> + v4l2_std_id std;
> int ret;
>
> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
> if (ret < 0)
> return ret;
>
> /* set default sensor frame width and height */
> - width = mbus_fmt->width;
> - height = mbus_fmt->height;
> + width = infmt->width;
> + height = infmt->height;
> + if (infmt->field = V4L2_FIELD_ALTERNATE)
> + height *= 2;
>
> /* Set the CSI_SENS_CONF register remaining fields */
> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> break;
> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
> - if (mbus_fmt->width = 720 && mbus_fmt->height = 576) {
> - /*
> - * PAL case
> - *
> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> - */
> - height = 625; /* framelines for PAL */
> -
> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> - } else if (mbus_fmt->width = 720 && mbus_fmt->height = 480) {
> - /*
> - * NTSC case
> - *
> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> - */
> - height = 525; /* framelines for NTSC */
> -
> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> - CSI_CCIR_CODE_1);
> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
> + if (width = 720 && height = 480) {
> + std = V4L2_STD_NTSC;
> + height = 525;
> + } else if (width = 720 && height = 576) {
> + std = V4L2_STD_PAL;
> + height = 625;
> } else {
> dev_err(csi->ipu->dev,
> - "Unsupported CCIR656 interlaced video mode\n");
> - spin_unlock_irqrestore(&csi->lock, flags);
> - return -EINVAL;
> + "Unsupported interlaced video mode\n");
> + ret = -EINVAL;
> + goto out_unlock;
> }
> +
> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
> + if (ret)
> + goto out_unlock;
> break;
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>
> +out_unlock:
> spin_unlock_irqrestore(&csi->lock, flags);
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 4223f8d418ae..7ecbd4d76d09 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
> /* Update the CSI whole sensor and active windows */
> static int csi_setup(struct csi_priv *priv)
> {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt infmt, outfmt;
> const struct imx_media_pixfmt *incc;
> struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
> struct v4l2_rect crop;
>
> - infmt = &priv->format_mbus[CSI_SINK_PAD];
> + infmt = priv->format_mbus[CSI_SINK_PAD];
Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
just the userspace visible input format as determined by the pad, but
the modified format we set the interface to? Not a strong preference,
though.
> incc = priv->cc[CSI_SINK_PAD];
> - outfmt = &priv->format_mbus[priv->active_output_pad];
> + outfmt = priv->format_mbus[priv->active_output_pad];
Copying the output format onto the stack seems unnecessary to me.
>
> /* compose mbus_config from the upstream endpoint */
> mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
> priv->upstream_ep.bus.parallel.flags :
> priv->upstream_ep.bus.mipi_csi2.flags;
>
> - /*
> - * we need to pass input frame to CSI interface, but
> - * with translated field type from output format
> - */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> crop = priv->crop;
>
> /*
> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
> * generic/bayer data
> */
> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
> - if_fmt.width *= incc->cycles;
> + infmt.width *= incc->cycles;
> crop.width *= incc->cycles;
> }
>
> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
> priv->crop.width = 2 * priv->compose.width,
> priv->crop.height = 2 * priv->compose.height);
>
> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
We can just pass the &priv->format_mbus[priv->active_output_pad] here,
or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
>
> ipu_csi_set_dest(priv->csi, priv->dest);
>
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index abbad94e14a1..f44a35192313 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
> struct ipu_csi;
> int ipu_csi_init_interface(struct ipu_csi *csi,
> struct v4l2_mbus_config *mbus_cfg,
> - struct v4l2_mbus_framefmt *mbus_fmt);
> + struct v4l2_mbus_framefmt *infmt,
> + struct v4l2_mbus_framefmt *outfmt);
> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
Either way,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
2018-12-13 12:59 ` Philipp Zabel
(?)
@ 2018-12-14 23:40 ` Steve Longerbeam
-1 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-14 23:40 UTC (permalink / raw)
To: Philipp Zabel, linux-media
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman,
Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
On 12/13/18 4:59 AM, Philipp Zabel wrote:
> Hi Steve,
>
> On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
>> The function ipu_csi_init_interface() was inverting the F-bit for
>> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
>> for NTSC bottom-top field order, the CSI would swap fields and
>> capture in top-bottom order.
>>
>> Instead, base field swap on the field order of the input to the CSI,
>> and the field order of the requested output. If the input/output
>> fields are sequential but different, swap fields, otherwise do
>> not swap. This requires passing both the input and output mbus
>> frame formats to ipu_csi_init_interface().
>>
>> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
>> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
>> possibly interlaced BT.1120 in the future).
>>
>> When detecting input video standard from the input frame width/height,
>> make sure to double height if input field type is alternate, since
>> in that case input height only includes lines for one field.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> Changes since v4:
>> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
>> by Philipp Zabel.
>> - Fixed a regression in csi_setup(), caught by Philipp.
>> ---
>> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
>> drivers/staging/media/imx/imx-media-csi.c | 17 +---
>> include/video/imx-ipu-v3.h | 3 +-
>> 3 files changed, 88 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
>> index aa0e30a2ba18..4a15e513fa05 100644
>> --- a/drivers/gpu/ipu-v3/ipu-csi.c
>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
>> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
>> return 0;
>> }
>>
>> +/* translate alternate field mode based on given standard */
>> +static inline enum v4l2_field
>> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
>> +{
>> + return (field != V4L2_FIELD_ALTERNATE) ? field :
>> + ((std & V4L2_STD_525_60) ?
>> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
>> +}
>> +
>> /*
>> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
>> */
>> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>> return 0;
>> }
>>
>> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt,
> infmt and outfmt parameters could be const.
Agreed, I will convert these pointer args to const. And since we are
changing the API to ipu_csi_init_interface() anyway, I went ahead and
converted the mbus_cfg, infmt, and outfmt pointer args to const there as
well.
>> + v4l2_std_id std)
>> +{
>> + enum v4l2_field infield, outfield;
>> + bool swap_fields;
>> +
>> + /* get translated field type of input and output */
>> + infield = ipu_csi_translate_field(infmt->field, std);
>> + outfield = ipu_csi_translate_field(outfmt->field, std);
>> +
>> + /*
>> + * Write the H-V-F codes the CSI will match against the
>> + * incoming data for start/end of active and blanking
>> + * field intervals. If input and output field types are
>> + * sequential but not the same (one is SEQ_BT and the other
>> + * is SEQ_TB), swap the F-bit so that the CSI will capture
>> + * field 1 lines before field 0 lines.
>> + */
>> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
>> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
>> + infield != outfield);
>> +
>> + if (!swap_fields) {
>> + /*
>> + * Field0BlankEnd = 110, Field0BlankStart = 010
>> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
>> + * Field1BlankEnd = 111, Field1BlankStart = 011
>> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
>> + */
>> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> + } else {
>> + dev_dbg(csi->ipu->dev, "capture field swap\n");
>> +
>> + /* same as above but with F-bit inverted */
>> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> + }
>> +
>> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> +
>> + return 0;
>> +}
>> +
>> +
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt)
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt)
>> {
>> struct ipu_csi_bus_config cfg;
>> unsigned long flags;
>> u32 width, height, data = 0;
>> + v4l2_std_id std;
>> int ret;
>>
>> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
>> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
>> if (ret < 0)
>> return ret;
>>
>> /* set default sensor frame width and height */
>> - width = mbus_fmt->width;
>> - height = mbus_fmt->height;
>> + width = infmt->width;
>> + height = infmt->height;
>> + if (infmt->field == V4L2_FIELD_ALTERNATE)
>> + height *= 2;
>>
>> /* Set the CSI_SENS_CONF register remaining fields */
>> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
>> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> break;
>> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
>> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
>> - /*
>> - * PAL case
>> - *
>> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
>> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
>> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
>> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
>> - */
>> - height = 625; /* framelines for PAL */
>> -
>> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
>> - /*
>> - * NTSC case
>> - *
>> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
>> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
>> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
>> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
>> - */
>> - height = 525; /* framelines for NTSC */
>> -
>> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> + if (width == 720 && height == 480) {
>> + std = V4L2_STD_NTSC;
>> + height = 525;
>> + } else if (width == 720 && height == 576) {
>> + std = V4L2_STD_PAL;
>> + height = 625;
>> } else {
>> dev_err(csi->ipu->dev,
>> - "Unsupported CCIR656 interlaced video mode\n");
>> - spin_unlock_irqrestore(&csi->lock, flags);
>> - return -EINVAL;
>> + "Unsupported interlaced video mode\n");
>> + ret = -EINVAL;
>> + goto out_unlock;
>> }
>> +
>> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
>> + if (ret)
>> + goto out_unlock;
>> break;
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
>> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
>> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>>
>> +out_unlock:
>> spin_unlock_irqrestore(&csi->lock, flags);
>>
>> - return 0;
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>>
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
>> index 4223f8d418ae..7ecbd4d76d09 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
>> /* Update the CSI whole sensor and active windows */
>> static int csi_setup(struct csi_priv *priv)
>> {
>> - struct v4l2_mbus_framefmt *infmt, *outfmt;
>> + struct v4l2_mbus_framefmt infmt, outfmt;
>> const struct imx_media_pixfmt *incc;
>> struct v4l2_mbus_config mbus_cfg;
>> - struct v4l2_mbus_framefmt if_fmt;
>> struct v4l2_rect crop;
>>
>> - infmt = &priv->format_mbus[CSI_SINK_PAD];
>> + infmt = priv->format_mbus[CSI_SINK_PAD];
> Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
> just the userspace visible input format as determined by the pad, but
> the modified format we set the interface to? Not a strong preference,
> though.
Ok, I brought back if_fmt.
>
>> incc = priv->cc[CSI_SINK_PAD];
>> - outfmt = &priv->format_mbus[priv->active_output_pad];
>> + outfmt = priv->format_mbus[priv->active_output_pad];
> Copying the output format onto the stack seems unnecessary to me.
Yes, I did this for consistency with on-stack infmt, but it's not
necessary, I restored outfmt to a pointer.
>
>>
>> /* compose mbus_config from the upstream endpoint */
>> mbus_cfg.type = priv->upstream_ep.bus_type;
>> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
>> priv->upstream_ep.bus.parallel.flags :
>> priv->upstream_ep.bus.mipi_csi2.flags;
>>
>> - /*
>> - * we need to pass input frame to CSI interface, but
>> - * with translated field type from output format
>> - */
>> - if_fmt = *infmt;
>> - if_fmt.field = outfmt->field;
>> crop = priv->crop;
>>
>> /*
>> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
>> * generic/bayer data
>> */
>> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
>> - if_fmt.width *= incc->cycles;
>> + infmt.width *= incc->cycles;
>> crop.width *= incc->cycles;
>> }
>>
>> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
>> priv->crop.width == 2 * priv->compose.width,
>> priv->crop.height == 2 * priv->compose.height);
>>
>> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
>> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
> We can just pass the &priv->format_mbus[priv->active_output_pad] here,
> or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
I just kept it as a short-hand.
Steve
>
>>
>> ipu_csi_set_dest(priv->csi, priv->dest);
>>
>> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
>> index abbad94e14a1..f44a35192313 100644
>> --- a/include/video/imx-ipu-v3.h
>> +++ b/include/video/imx-ipu-v3.h
>> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
>> struct ipu_csi;
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt);
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt);
>> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
>> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
>> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
> Either way,
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-14 23:40 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-14 23:40 UTC (permalink / raw)
To: Philipp Zabel, linux-media
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman,
Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
On 12/13/18 4:59 AM, Philipp Zabel wrote:
> Hi Steve,
>
> On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
>> The function ipu_csi_init_interface() was inverting the F-bit for
>> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
>> for NTSC bottom-top field order, the CSI would swap fields and
>> capture in top-bottom order.
>>
>> Instead, base field swap on the field order of the input to the CSI,
>> and the field order of the requested output. If the input/output
>> fields are sequential but different, swap fields, otherwise do
>> not swap. This requires passing both the input and output mbus
>> frame formats to ipu_csi_init_interface().
>>
>> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
>> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
>> possibly interlaced BT.1120 in the future).
>>
>> When detecting input video standard from the input frame width/height,
>> make sure to double height if input field type is alternate, since
>> in that case input height only includes lines for one field.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> Changes since v4:
>> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
>> by Philipp Zabel.
>> - Fixed a regression in csi_setup(), caught by Philipp.
>> ---
>> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
>> drivers/staging/media/imx/imx-media-csi.c | 17 +---
>> include/video/imx-ipu-v3.h | 3 +-
>> 3 files changed, 88 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
>> index aa0e30a2ba18..4a15e513fa05 100644
>> --- a/drivers/gpu/ipu-v3/ipu-csi.c
>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
>> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
>> return 0;
>> }
>>
>> +/* translate alternate field mode based on given standard */
>> +static inline enum v4l2_field
>> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
>> +{
>> + return (field != V4L2_FIELD_ALTERNATE) ? field :
>> + ((std & V4L2_STD_525_60) ?
>> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
>> +}
>> +
>> /*
>> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
>> */
>> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>> return 0;
>> }
>>
>> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt,
> infmt and outfmt parameters could be const.
Agreed, I will convert these pointer args to const. And since we are
changing the API to ipu_csi_init_interface() anyway, I went ahead and
converted the mbus_cfg, infmt, and outfmt pointer args to const there as
well.
>> + v4l2_std_id std)
>> +{
>> + enum v4l2_field infield, outfield;
>> + bool swap_fields;
>> +
>> + /* get translated field type of input and output */
>> + infield = ipu_csi_translate_field(infmt->field, std);
>> + outfield = ipu_csi_translate_field(outfmt->field, std);
>> +
>> + /*
>> + * Write the H-V-F codes the CSI will match against the
>> + * incoming data for start/end of active and blanking
>> + * field intervals. If input and output field types are
>> + * sequential but not the same (one is SEQ_BT and the other
>> + * is SEQ_TB), swap the F-bit so that the CSI will capture
>> + * field 1 lines before field 0 lines.
>> + */
>> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
>> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
>> + infield != outfield);
>> +
>> + if (!swap_fields) {
>> + /*
>> + * Field0BlankEnd = 110, Field0BlankStart = 010
>> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
>> + * Field1BlankEnd = 111, Field1BlankStart = 011
>> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
>> + */
>> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> + } else {
>> + dev_dbg(csi->ipu->dev, "capture field swap\n");
>> +
>> + /* same as above but with F-bit inverted */
>> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> + }
>> +
>> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> +
>> + return 0;
>> +}
>> +
>> +
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt)
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt)
>> {
>> struct ipu_csi_bus_config cfg;
>> unsigned long flags;
>> u32 width, height, data = 0;
>> + v4l2_std_id std;
>> int ret;
>>
>> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
>> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
>> if (ret < 0)
>> return ret;
>>
>> /* set default sensor frame width and height */
>> - width = mbus_fmt->width;
>> - height = mbus_fmt->height;
>> + width = infmt->width;
>> + height = infmt->height;
>> + if (infmt->field == V4L2_FIELD_ALTERNATE)
>> + height *= 2;
>>
>> /* Set the CSI_SENS_CONF register remaining fields */
>> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
>> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> break;
>> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
>> - if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
>> - /*
>> - * PAL case
>> - *
>> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
>> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
>> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
>> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
>> - */
>> - height = 625; /* framelines for PAL */
>> -
>> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> - } else if (mbus_fmt->width == 720 && mbus_fmt->height == 480) {
>> - /*
>> - * NTSC case
>> - *
>> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
>> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
>> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
>> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
>> - */
>> - height = 525; /* framelines for NTSC */
>> -
>> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> + if (width == 720 && height == 480) {
>> + std = V4L2_STD_NTSC;
>> + height = 525;
>> + } else if (width == 720 && height == 576) {
>> + std = V4L2_STD_PAL;
>> + height = 625;
>> } else {
>> dev_err(csi->ipu->dev,
>> - "Unsupported CCIR656 interlaced video mode\n");
>> - spin_unlock_irqrestore(&csi->lock, flags);
>> - return -EINVAL;
>> + "Unsupported interlaced video mode\n");
>> + ret = -EINVAL;
>> + goto out_unlock;
>> }
>> +
>> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
>> + if (ret)
>> + goto out_unlock;
>> break;
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
>> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
>> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>>
>> +out_unlock:
>> spin_unlock_irqrestore(&csi->lock, flags);
>>
>> - return 0;
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>>
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
>> index 4223f8d418ae..7ecbd4d76d09 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
>> /* Update the CSI whole sensor and active windows */
>> static int csi_setup(struct csi_priv *priv)
>> {
>> - struct v4l2_mbus_framefmt *infmt, *outfmt;
>> + struct v4l2_mbus_framefmt infmt, outfmt;
>> const struct imx_media_pixfmt *incc;
>> struct v4l2_mbus_config mbus_cfg;
>> - struct v4l2_mbus_framefmt if_fmt;
>> struct v4l2_rect crop;
>>
>> - infmt = &priv->format_mbus[CSI_SINK_PAD];
>> + infmt = priv->format_mbus[CSI_SINK_PAD];
> Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
> just the userspace visible input format as determined by the pad, but
> the modified format we set the interface to? Not a strong preference,
> though.
Ok, I brought back if_fmt.
>
>> incc = priv->cc[CSI_SINK_PAD];
>> - outfmt = &priv->format_mbus[priv->active_output_pad];
>> + outfmt = priv->format_mbus[priv->active_output_pad];
> Copying the output format onto the stack seems unnecessary to me.
Yes, I did this for consistency with on-stack infmt, but it's not
necessary, I restored outfmt to a pointer.
>
>>
>> /* compose mbus_config from the upstream endpoint */
>> mbus_cfg.type = priv->upstream_ep.bus_type;
>> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
>> priv->upstream_ep.bus.parallel.flags :
>> priv->upstream_ep.bus.mipi_csi2.flags;
>>
>> - /*
>> - * we need to pass input frame to CSI interface, but
>> - * with translated field type from output format
>> - */
>> - if_fmt = *infmt;
>> - if_fmt.field = outfmt->field;
>> crop = priv->crop;
>>
>> /*
>> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
>> * generic/bayer data
>> */
>> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
>> - if_fmt.width *= incc->cycles;
>> + infmt.width *= incc->cycles;
>> crop.width *= incc->cycles;
>> }
>>
>> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
>> priv->crop.width == 2 * priv->compose.width,
>> priv->crop.height == 2 * priv->compose.height);
>>
>> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
>> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
> We can just pass the &priv->format_mbus[priv->active_output_pad] here,
> or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
I just kept it as a short-hand.
Steve
>
>>
>> ipu_csi_set_dest(priv->csi, priv->dest);
>>
>> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
>> index abbad94e14a1..f44a35192313 100644
>> --- a/include/video/imx-ipu-v3.h
>> +++ b/include/video/imx-ipu-v3.h
>> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
>> struct ipu_csi;
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt);
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt);
>> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
>> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
>> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
> Either way,
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
@ 2018-12-14 23:40 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-14 23:40 UTC (permalink / raw)
To: Philipp Zabel, linux-media
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman,
Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
On 12/13/18 4:59 AM, Philipp Zabel wrote:
> Hi Steve,
>
> On Tue, 2018-10-16 at 17:00 -0700, Steve Longerbeam wrote:
>> The function ipu_csi_init_interface() was inverting the F-bit for
>> NTSC case, in the CCIR_CODE_1/2 registers. The result being that
>> for NTSC bottom-top field order, the CSI would swap fields and
>> capture in top-bottom order.
>>
>> Instead, base field swap on the field order of the input to the CSI,
>> and the field order of the requested output. If the input/output
>> fields are sequential but different, swap fields, otherwise do
>> not swap. This requires passing both the input and output mbus
>> frame formats to ipu_csi_init_interface().
>>
>> Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
>> that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
>> possibly interlaced BT.1120 in the future).
>>
>> When detecting input video standard from the input frame width/height,
>> make sure to double height if input field type is alternate, since
>> in that case input height only includes lines for one field.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>> Changes since v4:
>> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
>> by Philipp Zabel.
>> - Fixed a regression in csi_setup(), caught by Philipp.
>> ---
>> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++++++++-------
>> drivers/staging/media/imx/imx-media-csi.c | 17 +---
>> include/video/imx-ipu-v3.h | 3 +-
>> 3 files changed, 88 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
>> index aa0e30a2ba18..4a15e513fa05 100644
>> --- a/drivers/gpu/ipu-v3/ipu-csi.c
>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
>> @@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config *cfg, u32 mbus_code,
>> return 0;
>> }
>>
>> +/* translate alternate field mode based on given standard */
>> +static inline enum v4l2_field
>> +ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
>> +{
>> + return (field != V4L2_FIELD_ALTERNATE) ? field :
>> + ((std & V4L2_STD_525_60) ?
>> + V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
>> +}
>> +
>> /*
>> * Fill a CSI bus config struct from mbus_config and mbus_framefmt.
>> */
>> @@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config *csicfg,
>> return 0;
>> }
>>
>> +static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt,
> infmt and outfmt parameters could be const.
Agreed, I will convert these pointer args to const. And since we are
changing the API to ipu_csi_init_interface() anyway, I went ahead and
converted the mbus_cfg, infmt, and outfmt pointer args to const there as
well.
>> + v4l2_std_id std)
>> +{
>> + enum v4l2_field infield, outfield;
>> + bool swap_fields;
>> +
>> + /* get translated field type of input and output */
>> + infield = ipu_csi_translate_field(infmt->field, std);
>> + outfield = ipu_csi_translate_field(outfmt->field, std);
>> +
>> + /*
>> + * Write the H-V-F codes the CSI will match against the
>> + * incoming data for start/end of active and blanking
>> + * field intervals. If input and output field types are
>> + * sequential but not the same (one is SEQ_BT and the other
>> + * is SEQ_TB), swap the F-bit so that the CSI will capture
>> + * field 1 lines before field 0 lines.
>> + */
>> + swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
>> + V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
>> + infield != outfield);
>> +
>> + if (!swap_fields) {
>> + /*
>> + * Field0BlankEnd = 110, Field0BlankStart = 010
>> + * Field0ActiveEnd = 100, Field0ActiveStart = 000
>> + * Field1BlankEnd = 111, Field1BlankStart = 011
>> + * Field1ActiveEnd = 101, Field1ActiveStart = 001
>> + */
>> + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> + } else {
>> + dev_dbg(csi->ipu->dev, "capture field swap\n");
>> +
>> + /* same as above but with F-bit inverted */
>> + ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> + CSI_CCIR_CODE_1);
>> + ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> + }
>> +
>> + ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> +
>> + return 0;
>> +}
>> +
>> +
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt)
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt)
>> {
>> struct ipu_csi_bus_config cfg;
>> unsigned long flags;
>> u32 width, height, data = 0;
>> + v4l2_std_id std;
>> int ret;
>>
>> - ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
>> + ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
>> if (ret < 0)
>> return ret;
>>
>> /* set default sensor frame width and height */
>> - width = mbus_fmt->width;
>> - height = mbus_fmt->height;
>> + width = infmt->width;
>> + height = infmt->height;
>> + if (infmt->field = V4L2_FIELD_ALTERNATE)
>> + height *= 2;
>>
>> /* Set the CSI_SENS_CONF register remaining fields */
>> data |= cfg.data_width << CSI_SENS_CONF_DATA_WIDTH_SHIFT |
>> @@ -416,42 +478,22 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> break;
>> case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
>> - if (mbus_fmt->width = 720 && mbus_fmt->height = 576) {
>> - /*
>> - * PAL case
>> - *
>> - * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
>> - * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
>> - * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
>> - * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
>> - */
>> - height = 625; /* framelines for PAL */
>> -
>> - ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> - } else if (mbus_fmt->width = 720 && mbus_fmt->height = 480) {
>> - /*
>> - * NTSC case
>> - *
>> - * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
>> - * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
>> - * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
>> - * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
>> - */
>> - height = 525; /* framelines for NTSC */
>> -
>> - ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
>> - CSI_CCIR_CODE_1);
>> - ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
>> - ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>> + if (width = 720 && height = 480) {
>> + std = V4L2_STD_NTSC;
>> + height = 525;
>> + } else if (width = 720 && height = 576) {
>> + std = V4L2_STD_PAL;
>> + height = 625;
>> } else {
>> dev_err(csi->ipu->dev,
>> - "Unsupported CCIR656 interlaced video mode\n");
>> - spin_unlock_irqrestore(&csi->lock, flags);
>> - return -EINVAL;
>> + "Unsupported interlaced video mode\n");
>> + ret = -EINVAL;
>> + goto out_unlock;
>> }
>> +
>> + ret = ipu_csi_set_bt_interlaced_codes(csi, infmt, outfmt, std);
>> + if (ret)
>> + goto out_unlock;
>> break;
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
>> @@ -476,9 +518,10 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>> dev_dbg(csi->ipu->dev, "CSI_ACT_FRM_SIZE = 0x%08X\n",
>> ipu_csi_read(csi, CSI_ACT_FRM_SIZE));
>>
>> +out_unlock:
>> spin_unlock_irqrestore(&csi->lock, flags);
>>
>> - return 0;
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(ipu_csi_init_interface);
>>
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
>> index 4223f8d418ae..7ecbd4d76d09 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -663,15 +663,14 @@ static void csi_idmac_stop(struct csi_priv *priv)
>> /* Update the CSI whole sensor and active windows */
>> static int csi_setup(struct csi_priv *priv)
>> {
>> - struct v4l2_mbus_framefmt *infmt, *outfmt;
>> + struct v4l2_mbus_framefmt infmt, outfmt;
>> const struct imx_media_pixfmt *incc;
>> struct v4l2_mbus_config mbus_cfg;
>> - struct v4l2_mbus_framefmt if_fmt;
>> struct v4l2_rect crop;
>>
>> - infmt = &priv->format_mbus[CSI_SINK_PAD];
>> + infmt = priv->format_mbus[CSI_SINK_PAD];
> Maybe still call this if_fmt, or maybe csi_fmt, to indicate this is not
> just the userspace visible input format as determined by the pad, but
> the modified format we set the interface to? Not a strong preference,
> though.
Ok, I brought back if_fmt.
>
>> incc = priv->cc[CSI_SINK_PAD];
>> - outfmt = &priv->format_mbus[priv->active_output_pad];
>> + outfmt = priv->format_mbus[priv->active_output_pad];
> Copying the output format onto the stack seems unnecessary to me.
Yes, I did this for consistency with on-stack infmt, but it's not
necessary, I restored outfmt to a pointer.
>
>>
>> /* compose mbus_config from the upstream endpoint */
>> mbus_cfg.type = priv->upstream_ep.bus_type;
>> @@ -679,12 +678,6 @@ static int csi_setup(struct csi_priv *priv)
>> priv->upstream_ep.bus.parallel.flags :
>> priv->upstream_ep.bus.mipi_csi2.flags;
>>
>> - /*
>> - * we need to pass input frame to CSI interface, but
>> - * with translated field type from output format
>> - */
>> - if_fmt = *infmt;
>> - if_fmt.field = outfmt->field;
>> crop = priv->crop;
>>
>> /*
>> @@ -692,7 +685,7 @@ static int csi_setup(struct csi_priv *priv)
>> * generic/bayer data
>> */
>> if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
>> - if_fmt.width *= incc->cycles;
>> + infmt.width *= incc->cycles;
>> crop.width *= incc->cycles;
>> }
>>
>> @@ -702,7 +695,7 @@ static int csi_setup(struct csi_priv *priv)
>> priv->crop.width = 2 * priv->compose.width,
>> priv->crop.height = 2 * priv->compose.height);
>>
>> - ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
>> + ipu_csi_init_interface(priv->csi, &mbus_cfg, &infmt, &outfmt);
> We can just pass the &priv->format_mbus[priv->active_output_pad] here,
> or keep using struct v4l2_mbus_framefmt *outfmt as a shorthand.
I just kept it as a short-hand.
Steve
>
>>
>> ipu_csi_set_dest(priv->csi, priv->dest);
>>
>> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
>> index abbad94e14a1..f44a35192313 100644
>> --- a/include/video/imx-ipu-v3.h
>> +++ b/include/video/imx-ipu-v3.h
>> @@ -352,7 +352,8 @@ int ipu_prg_channel_configure(struct ipuv3_channel *ipu_chan,
>> struct ipu_csi;
>> int ipu_csi_init_interface(struct ipu_csi *csi,
>> struct v4l2_mbus_config *mbus_cfg,
>> - struct v4l2_mbus_framefmt *mbus_fmt);
>> + struct v4l2_mbus_framefmt *infmt,
>> + struct v4l2_mbus_framefmt *outfmt);
>> bool ipu_csi_is_interlaced(struct ipu_csi *csi);
>> void ipu_csi_get_window(struct ipu_csi *csi, struct v4l2_rect *w);
>> void ipu_csi_set_window(struct ipu_csi *csi, struct v4l2_rect *w);
> Either way,
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v5 03/12] gpu: ipu-v3: Add planar support to interlaced scan
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 01/12] media: videodev2.h: Add more field helper macros Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` Steve Longerbeam
` (10 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++++++++++++++++++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++-
drivers/staging/media/imx/imx-media-csi.c | 3 ++-
include/video/imx-ipu-v3.h | 3 ++-
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
}
EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat)
{
- u32 ilo, sly;
+ u32 ilo, sly, sluv;
if (stride < 0) {
stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
sly = (stride * 2) - 1;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ sluv = stride / 2 - 1;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_YUV422P:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ sluv = stride * 2 - 1;
+ break;
+ default:
+ sluv = 0;
+ break;
+ }
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+ if (sluv)
+ ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
};
EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field) &&
channel == priv->out_ch)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+ ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 7ecbd4d76d09..4aa20ae72608 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
- image.pix.bytesperline);
+ image.pix.bytesperline,
+ image.pix.pixelformat);
ipu_idmac_set_double_buffer(priv->idmac_ch, true);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int stride);
void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t buf);
void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat);
void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 03/12] gpu: ipu-v3: Add planar support to interlaced scan
@ 2018-10-17 0:00 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++++++++++++++++++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++-
drivers/staging/media/imx/imx-media-csi.c | 3 ++-
include/video/imx-ipu-v3.h | 3 ++-
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
}
EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat)
{
- u32 ilo, sly;
+ u32 ilo, sly, sluv;
if (stride < 0) {
stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
sly = (stride * 2) - 1;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ sluv = stride / 2 - 1;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_YUV422P:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ sluv = stride * 2 - 1;
+ break;
+ default:
+ sluv = 0;
+ break;
+ }
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+ if (sluv)
+ ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
};
EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field) &&
channel == priv->out_ch)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+ ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 7ecbd4d76d09..4aa20ae72608 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
- image.pix.bytesperline);
+ image.pix.bytesperline,
+ image.pix.pixelformat);
ipu_idmac_set_double_buffer(priv->idmac_ch, true);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int stride);
void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t buf);
void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat);
void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 03/12] gpu: ipu-v3: Add planar support to interlaced scan
@ 2018-10-17 0:00 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
open list:DRM DRIVERS FOR FREESCALE IMX, open list,
open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER
To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++++++++++++++++++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++-
drivers/staging/media/imx/imx-media-csi.c | 3 ++-
include/video/imx-ipu-v3.h | 3 ++-
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off)
}
EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat)
{
- u32 ilo, sly;
+ u32 ilo, sly, sluv;
if (stride < 0) {
stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
sly = (stride * 2) - 1;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ sluv = stride / 2 - 1;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_YUV422P:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ sluv = stride * 2 - 1;
+ break;
+ default:
+ sluv = 0;
+ break;
+ }
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+ if (sluv)
+ ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
};
EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
if (image.pix.field = V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field) &&
channel = priv->out_ch)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+ ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 7ecbd4d76d09..4aa20ae72608 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (image.pix.field = V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
- image.pix.bytesperline);
+ image.pix.bytesperline,
+ image.pix.pixelformat);
ipu_idmac_set_double_buffer(priv->idmac_ch, true);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int stride);
void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t buf);
void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat);
void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v5 04/12] media: imx: Fix field negotiation
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (2 preceding siblings ...)
2018-10-17 0:00 ` Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 05/12] media: imx-csi: Input connections to CSI should be optional Steve Longerbeam
` (8 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
IDMAC interlaced scan, a.k.a. interweave, should be enabled in the
IDMAC output channels only if the IDMAC output pad field type is
'seq-bt' or 'seq-tb', and field type at the capture interface is
'interlaced*'.
V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
enabling interlaced/interweave scan. That macro includes the 'interlaced'
field types, and in those cases the data is already interweaved with
top/bottom field lines.
The CSI will capture whole frames when the source specifies alternate
field mode. So the CSI also enables interweave for alternate input
field type and the field type at capture interface is interlaced.
Fix the logic for setting field type in try_fmt in CSI entity.
The behavior should be:
- No restrictions on field type at sink pad.
- At the output pads, allow sequential fields in TB order, if the sink pad
field type is sequential or alternate. Otherwise passthrough the field
type from sink to source pad.
Move this logic to new function csi_try_field().
These changes result in the following allowed field transformations
from CSI sink -> source pads (all other field types at sink are passed
through to source):
seq-tb -> seq-tb
seq-bt -> seq-tb
alternate -> seq-tb
In a future patch, the CSI sink -> source will allow:
seq-tb -> seq-bt
seq-bt -> seq-bt
alternate -> seq-bt
This will require supporting interweave with top/bottom line swapping.
Until then seq-bt is not allowed at the CSI source pad because there is
no way to swap top/bottom lines when interweaving to INTERLACED_BT --
note that despite the name, INTERLACED_BT is top-bottom order in memory.
The BT in this case refers to field dominance: the bottom lines are
older in time than the top lines.
The capture interface device allows selecting IDMAC interweave by
choosing INTERLACED_TB if the CSI/PRPENCVF source pad is seq-tb and
INTERLACED_BT if the source pad is seq-bt (for future support of seq-bt).
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 21 ++++--
drivers/staging/media/imx/imx-media-capture.c | 14 ++++
drivers/staging/media/imx/imx-media-csi.c | 64 ++++++++++++++-----
3 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index af7224846bd5..1a03d4c9d7b8 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -354,12 +354,13 @@ static int prp_setup_channel(struct prp_priv *priv,
{
struct imx_media_video_dev *vdev = priv->vdev;
const struct imx_media_pixfmt *outcc;
- struct v4l2_mbus_framefmt *infmt;
+ struct v4l2_mbus_framefmt *outfmt;
unsigned int burst_size;
struct ipu_image image;
+ bool interweave;
int ret;
- infmt = &priv->format_mbus[PRPENCVF_SINK_PAD];
+ outfmt = &priv->format_mbus[PRPENCVF_SRC_PAD];
outcc = vdev->cc;
ipu_cpmem_zero(channel);
@@ -369,6 +370,15 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
+ /*
+ * If the field type at capture interface is interlaced, and
+ * the output IDMAC pad is sequential, enable interweave at
+ * the IDMAC output channel.
+ */
+ interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
+ channel == priv->out_ch;
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -409,9 +419,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field) &&
- channel == priv->out_ch)
+ if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
image.pix.pixelformat);
@@ -839,8 +847,7 @@ static void prp_try_fmt(struct prp_priv *priv,
infmt = __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD, sdformat->which);
if (sdformat->pad == PRPENCVF_SRC_PAD) {
- if (sdformat->format.field != V4L2_FIELD_NONE)
- sdformat->format.field = infmt->field;
+ sdformat->format.field = infmt->field;
prp_bound_align_output(&sdformat->format, infmt,
priv->rot_mode);
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index b37e1186eb2f..01ec9443de55 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -239,6 +239,20 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
cc = cc_src;
}
+ /* allow IDMAC interweave but enforce field order from source */
+ if (V4L2_FIELD_IS_INTERLACED(f->fmt.pix.field)) {
+ switch (fmt_src.format.field) {
+ case V4L2_FIELD_SEQ_TB:
+ fmt_src.format.field = V4L2_FIELD_INTERLACED_TB;
+ break;
+ case V4L2_FIELD_SEQ_BT:
+ fmt_src.format.field = V4L2_FIELD_INTERLACED_BT;
+ break;
+ default:
+ break;
+ }
+ }
+
imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src.format, cc);
return 0;
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4aa20ae72608..176978c7dfe7 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -398,16 +398,18 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
struct imx_media_video_dev *vdev = priv->vdev;
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;
dma_addr_t phys[2];
- bool passthrough;
u32 burst_size;
int ret;
infmt = &priv->format_mbus[CSI_SINK_PAD];
incc = priv->cc[CSI_SINK_PAD];
+ outfmt = &priv->format_mbus[CSI_SRC_PAD_IDMAC];
ipu_cpmem_zero(priv->idmac_ch);
@@ -424,6 +426,14 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
passthrough = requires_passthrough(&priv->upstream_ep, infmt, incc);
passthrough_cycles = 1;
+ /*
+ * If the field type at capture interface is interlaced, and
+ * the output IDMAC pad is sequential, enable interweave at
+ * the IDMAC output channel.
+ */
+ interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field);
+
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_SBGGR8:
case V4L2_PIX_FMT_SGBRG8:
@@ -509,8 +519,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
ipu_smfc_set_burstsize(priv->smfc, burst_size);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field))
+ if (interweave)
ipu_cpmem_interlaced_scan(priv->idmac_ch,
image.pix.bytesperline,
image.pix.pixelformat);
@@ -1302,6 +1311,38 @@ static int csi_get_fmt(struct v4l2_subdev *sd,
return ret;
}
+static void csi_try_field(struct csi_priv *priv,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *sdformat)
+{
+ struct v4l2_mbus_framefmt *infmt =
+ __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sdformat->which);
+
+ /* no restrictions on sink pad field type */
+ if (sdformat->pad == CSI_SINK_PAD)
+ return;
+
+ switch (infmt->field) {
+ case V4L2_FIELD_SEQ_TB:
+ case V4L2_FIELD_SEQ_BT:
+ 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.
+ */
+ sdformat->format.field = V4L2_FIELD_SEQ_TB;
+ break;
+ default:
+ /* Passthrough for all other input field types */
+ sdformat->format.field = infmt->field;
+ break;
+ }
+}
+
static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_fwnode_endpoint *upstream_ep,
struct v4l2_subdev_pad_config *cfg,
@@ -1341,25 +1382,14 @@ static void csi_try_fmt(struct csi_priv *priv,
}
}
- if (sdformat->pad == CSI_SRC_PAD_DIRECT ||
- sdformat->format.field != V4L2_FIELD_NONE)
- sdformat->format.field = infmt->field;
-
- /*
- * translate V4L2_FIELD_ALTERNATE to SEQ_TB or SEQ_BT
- * depending on input height (assume NTSC top-bottom
- * order if 480 lines, otherwise PAL bottom-top order).
- */
- if (sdformat->format.field == V4L2_FIELD_ALTERNATE) {
- sdformat->format.field = (infmt->height == 480) ?
- V4L2_FIELD_SEQ_TB : V4L2_FIELD_SEQ_BT;
- }
+ csi_try_field(priv, cfg, sdformat);
/* propagate colorimetry from sink */
sdformat->format.colorspace = infmt->colorspace;
sdformat->format.xfer_func = infmt->xfer_func;
sdformat->format.quantization = infmt->quantization;
sdformat->format.ycbcr_enc = infmt->ycbcr_enc;
+
break;
case CSI_SINK_PAD:
v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W,
@@ -1387,6 +1417,8 @@ static void csi_try_fmt(struct csi_priv *priv,
sdformat->format.code = (*cc)->codes[0];
}
+ csi_try_field(priv, cfg, sdformat);
+
imx_media_fill_default_mbus_fields(
&sdformat->format, infmt,
priv->active_output_pad == CSI_SRC_PAD_DIRECT);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 05/12] media: imx-csi: Input connections to CSI should be optional
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (3 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 04/12] media: imx: Fix field negotiation Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 06/12] media: imx-csi: Double crop height for alternate fields at sink Steve Longerbeam
` (7 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
Some imx platforms do not have fwnode connections to all CSI input
ports, and should not be treated as an error. This includes the
imx6q SabreAuto, which has no connections to ipu1_csi1 and ipu2_csi0.
Return -ENOTCONN in imx_csi_parse_endpoint() so that v4l2-fwnode
endpoint parsing will not treat an unconnected endpoint as an error.
Fixes: c893500a16baf ("media: imx: csi: Register a subdev notifier")
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-csi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 176978c7dfe7..8f52428d2c75 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1813,7 +1813,7 @@ static int imx_csi_parse_endpoint(struct device *dev,
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd)
{
- return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
+ return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
}
static int imx_csi_async_register(struct csi_priv *priv)
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 06/12] media: imx-csi: Double crop height for alternate fields at sink
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (4 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 05/12] media: imx-csi: Input connections to CSI should be optional Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 07/12] media: imx: interweave and odd-chroma-row skip are incompatible Steve Longerbeam
` (6 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
If the incoming sink field type is alternate, the reset crop height
and crop height bounds must be set to twice the incoming height,
because in alternate field mode, upstream will report only the
lines for a single field, and the CSI captures the whole frame.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/staging/media/imx/imx-media-csi.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 8f52428d2c75..d5b0f8a66750 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1140,6 +1140,8 @@ static void csi_try_crop(struct csi_priv *priv,
struct v4l2_mbus_framefmt *infmt,
struct v4l2_fwnode_endpoint *upstream_ep)
{
+ u32 in_height;
+
crop->width = min_t(__u32, infmt->width, crop->width);
if (crop->left + crop->width > infmt->width)
crop->left = infmt->width - crop->width;
@@ -1147,6 +1149,10 @@ static void csi_try_crop(struct csi_priv *priv,
crop->left &= ~0x3;
crop->width &= ~0x7;
+ in_height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ in_height *= 2;
+
/*
* FIXME: not sure why yet, but on interlaced bt.656,
* changing the vertical cropping causes loss of vertical
@@ -1156,12 +1162,12 @@ static void csi_try_crop(struct csi_priv *priv,
if (upstream_ep->bus_type == V4L2_MBUS_BT656 &&
(V4L2_FIELD_HAS_BOTH(infmt->field) ||
infmt->field == V4L2_FIELD_ALTERNATE)) {
- crop->height = infmt->height;
- crop->top = (infmt->height == 480) ? 2 : 0;
+ crop->height = in_height;
+ crop->top = (in_height == 480) ? 2 : 0;
} else {
- crop->height = min_t(__u32, infmt->height, crop->height);
- if (crop->top + crop->height > infmt->height)
- crop->top = infmt->height - crop->height;
+ crop->height = min_t(__u32, in_height, crop->height);
+ if (crop->top + crop->height > in_height)
+ crop->top = in_height - crop->height;
}
}
@@ -1401,6 +1407,8 @@ static void csi_try_fmt(struct csi_priv *priv,
crop->top = 0;
crop->width = sdformat->format.width;
crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
compose->left = 0;
compose->top = 0;
@@ -1528,6 +1536,8 @@ static int csi_get_selection(struct v4l2_subdev *sd,
sel->r.top = 0;
sel->r.width = infmt->width;
sel->r.height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ sel->r.height *= 2;
break;
case V4L2_SEL_TGT_CROP:
sel->r = *crop;
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 07/12] media: imx: interweave and odd-chroma-row skip are incompatible
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (5 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 06/12] media: imx-csi: Double crop height for alternate fields at sink Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 08/12] media: imx-csi: Allow skipping odd chroma rows for YVU420 Steve Longerbeam
` (5 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
If IDMAC interweaving is enabled in a write channel, the channel must
write the odd chroma rows for 4:2:0 formats. Skipping writing the odd
chroma rows produces corrupted captured 4:2:0 images when interweave
is enabled.
Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 9 +++++++--
drivers/staging/media/imx/imx-media-csi.c | 8 ++++++--
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 1a03d4c9d7b8..cf76b0432371 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -391,12 +391,17 @@ static int prp_setup_channel(struct prp_priv *priv,
image.phys0 = addr0;
image.phys1 = addr1;
- if (channel == priv->out_ch || channel == priv->rot_out_ch) {
+ /*
+ * Skip writing U and V components to odd rows in the output
+ * channels for planar 4:2:0 (but not when enabling IDMAC
+ * interweaving, they are incompatible).
+ */
+ if (!interweave && (channel == priv->out_ch ||
+ channel == priv->rot_out_ch)) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
- /* Skip writing U and V components to odd rows */
ipu_cpmem_skip_odd_chroma_rows(channel);
break;
}
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index d5b0f8a66750..7e648fc9626a 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -457,8 +457,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
((image.pix.width & 0x1f) ?
((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
passthrough_bits = 16;
- /* Skip writing U and V components to odd rows */
- ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
+ /*
+ * Skip writing U and V components to odd rows (but not
+ * when enabling IDMAC interweaving, they are incompatible).
+ */
+ if (!interweave)
+ ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
break;
case V4L2_PIX_FMT_YUYV:
case V4L2_PIX_FMT_UYVY:
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 08/12] media: imx-csi: Allow skipping odd chroma rows for YVU420
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (6 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 07/12] media: imx: interweave and odd-chroma-row skip are incompatible Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 09/12] media: imx: vdic: rely on VDIC for correct field order Steve Longerbeam
` (4 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
Skip writing U/V components to odd rows for YVU420 in addition to
YUV420 and NV12.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/staging/media/imx/imx-media-csi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 7e648fc9626a..c9110dd39a49 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -452,6 +452,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
passthrough_bits = 16;
break;
case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
burst_size = (image.pix.width & 0x3f) ?
((image.pix.width & 0x1f) ?
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 09/12] media: imx: vdic: rely on VDIC for correct field order
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (7 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 08/12] media: imx-csi: Allow skipping odd chroma rows for YVU420 Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 10/12] media: imx-csi: Move crop/compose reset after filling default mbus fields Steve Longerbeam
` (3 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
prepare_vdi_in_buffers() was setting up the dma pointers as if the
VDIC is always programmed to receive the fields in bottom-top order,
i.e. as if ipu_vdi_set_field_order() only programs BT order in the VDIC.
But that's not true, ipu_vdi_set_field_order() is working correctly.
So fix prepare_vdi_in_buffers() to give the VDIC the fields in whatever
order they were received by the video source, and rely on the VDIC to
sort out which is top and which is bottom.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
drivers/staging/media/imx/imx-media-vdic.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 482250d47e7c..4a890714193e 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -219,26 +219,18 @@ static void __maybe_unused prepare_vdi_in_buffers(struct vdic_priv *priv,
switch (priv->fieldtype) {
case V4L2_FIELD_SEQ_TB:
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
case V4L2_FIELD_SEQ_BT:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + fs;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
break;
+ case V4L2_FIELD_INTERLACED_TB:
case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + is;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
break;
- default:
- /* assume V4L2_FIELD_INTERLACED_TB */
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
}
ipu_cpmem_set_buffer(priv->vdi_in_ch_p, 0, prev_phys);
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 10/12] media: imx-csi: Move crop/compose reset after filling default mbus fields
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (8 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 09/12] media: imx: vdic: rely on VDIC for correct field order Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 11/12] media: imx: Allow interweave with top/bottom lines swapped Steve Longerbeam
` (2 subsequent siblings)
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
If caller passes un-initialized field type V4L2_FIELD_ANY to CSI
sink pad, the reset CSI crop window would not be correct, because
the crop window depends on a valid input field type. To fix move
the reset of crop and compose windows to after the call to
imx_media_fill_default_mbus_fields().
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/staging/media/imx/imx-media-csi.c | 27 ++++++++++++-----------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index c9110dd39a49..0d494a7db211 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1407,19 +1407,6 @@ static void csi_try_fmt(struct csi_priv *priv,
W_ALIGN, &sdformat->format.height,
MIN_H, MAX_H, H_ALIGN, S_ALIGN);
- /* Reset crop and compose rectangles */
- crop->left = 0;
- crop->top = 0;
- crop->width = sdformat->format.width;
- crop->height = sdformat->format.height;
- if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
- crop->height *= 2;
- csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
- compose->left = 0;
- compose->top = 0;
- compose->width = crop->width;
- compose->height = crop->height;
-
*cc = imx_media_find_mbus_format(sdformat->format.code,
CS_SEL_ANY, true);
if (!*cc) {
@@ -1435,6 +1422,20 @@ static void csi_try_fmt(struct csi_priv *priv,
imx_media_fill_default_mbus_fields(
&sdformat->format, infmt,
priv->active_output_pad == CSI_SRC_PAD_DIRECT);
+
+ /* Reset crop and compose rectangles */
+ crop->left = 0;
+ crop->top = 0;
+ crop->width = sdformat->format.width;
+ crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
+ csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
+ compose->left = 0;
+ compose->top = 0;
+ compose->width = crop->width;
+ compose->height = crop->height;
+
break;
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 11/12] media: imx: Allow interweave with top/bottom lines swapped
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (9 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 10/12] media: imx-csi: Move crop/compose reset after filling default mbus fields Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-10-17 0:00 ` [PATCH v5 12/12] media: imx.rst: Update doc to reflect fixes to interlaced capture Steve Longerbeam
2018-12-07 13:35 ` [PATCH v5 00/12] imx-media: Fixes for " Hans Verkuil
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list
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>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
- Removed interweave_offset and replace with boolean interweave_swap,
suggested by Philipp Zabel.
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +++++++++----
drivers/staging/media/imx/imx-media-csi.c | 40 ++++++++++++++++++---
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index cf76b0432371..33ada6612fee 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -106,6 +106,7 @@ 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 */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -235,6 +236,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 (priv->interweave_swap && ch == priv->out_ch)
+ phys += vdev->fmt.fmt.pix.bytesperline;
+
ipu_cpmem_set_buffer(ch, priv->ipu_buf_num, phys);
}
@@ -376,8 +380,9 @@ static int prp_setup_channel(struct prp_priv *priv,
* the IDMAC output channel.
*/
interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
- V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
- channel == priv->out_ch;
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field);
+ priv->interweave_swap = interweave &&
+ image.pix.field == V4L2_FIELD_INTERLACED_BT;
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
@@ -388,6 +393,11 @@ static int prp_setup_channel(struct prp_priv *priv,
(image.pix.width * outcc->bpp) >> 3;
}
+ if (priv->interweave_swap && channel == priv->out_ch) {
+ /* start interweave scan at 1st top line (2nd line) */
+ image.rect.top = 1;
+ }
+
image.phys0 = addr0;
image.phys1 = addr1;
@@ -396,8 +406,8 @@ static int prp_setup_channel(struct prp_priv *priv,
* channels for planar 4:2:0 (but not when enabling IDMAC
* interweaving, they are incompatible).
*/
- if (!interweave && (channel == priv->out_ch ||
- channel == priv->rot_out_ch)) {
+ if ((channel == priv->out_ch && !interweave) ||
+ channel == priv->rot_out_ch) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
@@ -424,8 +434,11 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (interweave)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ if (interweave && channel == priv->out_ch)
+ ipu_cpmem_interlaced_scan(channel,
+ priv->interweave_swap ?
+ -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 0d494a7db211..73c9f3ae4221 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -114,6 +114,7 @@ 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 */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -286,6 +287,9 @@ 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);
+ if (priv->interweave_swap)
+ phys += vdev->fmt.fmt.pix.bytesperline;
+
ipu_cpmem_set_buffer(priv->idmac_ch, priv->ipu_buf_num, phys);
}
@@ -433,6 +437,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);
+ priv->interweave_swap = interweave &&
+ image.pix.field == V4L2_FIELD_INTERLACED_BT;
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_SBGGR8:
@@ -486,6 +492,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
}
if (passthrough) {
+ if (priv->interweave_swap) {
+ /* start interweave scan at 1st top line (2nd line) */
+ image.phys0 += image.pix.bytesperline;
+ image.phys1 += image.pix.bytesperline;
+ }
+
ipu_cpmem_set_resolution(priv->idmac_ch,
image.rect.width * passthrough_cycles,
image.rect.height);
@@ -495,6 +507,11 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
ipu_cpmem_set_format_passthrough(priv->idmac_ch,
passthrough_bits);
} else {
+ if (priv->interweave_swap) {
+ /* start interweave scan at 1st top line (2nd line) */
+ image.rect.top = 1;
+ }
+
ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
if (ret)
goto unsetup_vb2;
@@ -526,6 +543,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (interweave)
ipu_cpmem_interlaced_scan(priv->idmac_ch,
+ priv->interweave_swap ?
+ -image.pix.bytesperline :
image.pix.bytesperline,
image.pix.pixelformat);
@@ -1336,16 +1355,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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v5 12/12] media: imx.rst: Update doc to reflect fixes to interlaced capture
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (10 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 11/12] media: imx: Allow interweave with top/bottom lines swapped Steve Longerbeam
@ 2018-10-17 0:00 ` Steve Longerbeam
2018-12-07 13:35 ` [PATCH v5 00/12] imx-media: Fixes for " Hans Verkuil
12 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-10-17 0:00 UTC (permalink / raw)
To: linux-media
Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab, open list
Also add an example pipeline for unconverted capture with interweave
on SabreAuto.
Cleanup some language in various places in the process.
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes since v4:
- Make clear that it is IDMAC channel that does pixel reordering and
interweave, not the CSI. Caught by Philipp Zabel.
Changes since v3:
- none.
Changes since v2:
- expand on idmac interweave behavior in CSI subdev.
- switch second SabreAuto pipeline example to PAL to give
both NTSC and PAL examples.
- Cleanup some language in various places.
---
Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
1 file changed, 66 insertions(+), 37 deletions(-)
diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
index 65d3d15eb159..45f3b68865dd 100644
--- a/Documentation/media/v4l-drivers/imx.rst
+++ b/Documentation/media/v4l-drivers/imx.rst
@@ -22,8 +22,8 @@ memory. Various dedicated DMA channels exist for both video capture and
display paths. During transfer, the IDMAC is also capable of vertical
image flip, 8x8 block transfer (see IRT description), pixel component
re-ordering (for example UYVY to YUYV) within the same colorspace, and
-even packed <--> planar conversion. It can also perform a simple
-de-interlacing by interleaving even and odd lines during transfer
+packed <--> planar conversion. The IDMAC can also perform a simple
+de-interlacing by interweaving even and odd lines during transfer
(without motion compensation which requires the VDIC).
The CSI is the backend capture unit that interfaces directly with
@@ -173,15 +173,21 @@ via the SMFC and an IDMAC channel, bypassing IC pre-processing. This
source pad is routed to a capture device node, with a node name of the
format "ipuX_csiY capture".
-Note that since the IDMAC source pad makes use of an IDMAC channel, it
-can do pixel reordering within the same colorspace. For example, the
-sink pad can take UYVY2X8, but the IDMAC source pad can output YUYV2X8.
-If the sink pad is receiving YUV, the output at the capture device can
-also be converted to a planar YUV format such as YUV420.
-
-It will also perform simple de-interlace without motion compensation,
-which is activated if the sink pad's field type is an interlaced type,
-and the IDMAC source pad field type is set to none.
+Note that since the IDMAC source pad makes use of an IDMAC channel,
+pixel reordering within the same colorspace can be carried out by the
+IDMAC channel. For example, if the CSI sink pad is receiving in UYVY
+order, the capture device linked to the IDMAC source pad can capture
+in YUYV order. Also, if the CSI sink pad is receiving a packed YUV
+format, the capture device can capture a planar YUV format such as
+YUV420.
+
+The IDMAC channel at the IDMAC source pad also supports simple
+interweave without motion compensation, which is activated if the source
+pad's field type is sequential top-bottom or bottom-top, and the
+requested capture interface field type is set to interlaced (t-b, b-t,
+or unqualified interlaced). The capture interface will enforce the same
+field order as the source pad field order (interlaced-bt if source pad
+is seq-bt, interlaced-tb if source pad is seq-tb).
This subdev can generate the following event when enabling the second
IDMAC source pad:
@@ -323,14 +329,14 @@ ipuX_vdic
The VDIC carries out motion compensated de-interlacing, with three
motion compensation modes: low, medium, and high motion. The mode is
-specified with the menu control V4L2_CID_DEINTERLACING_MODE. It has
-two sink pads and a single source pad.
+specified with the menu control V4L2_CID_DEINTERLACING_MODE. The VDIC
+has two sink pads and a single source pad.
The direct sink pad receives from an ipuX_csiY direct pad. With this
link the VDIC can only operate in high motion mode.
When the IDMAC sink pad is activated, it receives from an output
-or mem2mem device node. With this pipeline, it can also operate
+or mem2mem device node. With this pipeline, the VDIC can also operate
in low and medium modes, because these modes require receiving
frames from memory buffers. Note that an output or mem2mem device
is not implemented yet, so this sink pad currently has no links.
@@ -343,8 +349,8 @@ ipuX_ic_prp
This is the IC pre-processing entity. It acts as a router, routing
data from its sink pad to one or both of its source pads.
-It has a single sink pad. The sink pad can receive from the ipuX_csiY
-direct pad, or from ipuX_vdic.
+This entity has a single sink pad. The sink pad can receive from the
+ipuX_csiY direct pad, or from ipuX_vdic.
This entity has two source pads. One source pad routes to the
pre-process encode task entity (ipuX_ic_prpenc), the other to the
@@ -367,8 +373,8 @@ color-space conversion, resizing (downscaling and upscaling),
horizontal and vertical flip, and 90/270 degree rotation. Flip
and rotation are provided via standard V4L2 controls.
-Like the ipuX_csiY IDMAC source, it can also perform simple de-interlace
-without motion compensation, and pixel reordering.
+Like the ipuX_csiY IDMAC source, this entity also supports simple
+de-interlace without motion compensation, and pixel reordering.
ipuX_ic_prpvf
-------------
@@ -378,18 +384,18 @@ pad from ipuX_ic_prp, and a single source pad. The source pad is routed
to a capture device node, with a node name of the format
"ipuX_ic_prpvf capture".
-It is identical in operation to ipuX_ic_prpenc, with the same resizing
-and CSC operations and flip/rotation controls. It will receive and
-process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is
+This entity is identical in operation to ipuX_ic_prpenc, with the same
+resizing and CSC operations and flip/rotation controls. It will receive
+and process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is
receiving from ipuX_vdic.
-Like the ipuX_csiY IDMAC source, it can perform simple de-interlace
-without motion compensation. However, note that if the ipuX_vdic is
-included in the pipeline (ipuX_ic_prp is receiving from ipuX_vdic),
-it's not possible to use simple de-interlace in ipuX_ic_prpvf, since
-the ipuX_vdic has already carried out de-interlacing (with motion
-compensation) and therefore the field type output from ipuX_ic_prp can
-only be none.
+Like the ipuX_csiY IDMAC source, this entity supports simple
+interweaving without motion compensation. However, note that if the
+ipuX_vdic is included in the pipeline (ipuX_ic_prp is receiving from
+ipuX_vdic), it's not possible to use interweave in ipuX_ic_prpvf,
+since the ipuX_vdic has already carried out de-interlacing (with
+motion compensation) and therefore the field type output from
+ipuX_vdic can only be none (progressive).
Capture Pipelines
-----------------
@@ -514,10 +520,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
parallel bus input on the internal video mux to IPU1 CSI0.
The following example configures a pipeline to capture from the ADV7180
-video decoder, assuming NTSC 720x480 input signals, with Motion
-Compensated de-interlacing. Pad field types assume the adv7180 outputs
-"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
-entity at its output pad:
+video decoder, assuming NTSC 720x480 input signals, using simple
+interweave (unconverted and without motion compensation). The adv7180
+must output sequential or alternating fields (field type 'seq-bt' for
+NTSC, or 'alternate'):
+
+.. code-block:: none
+
+ # Setup links
+ media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
+ media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
+ media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
+ # Configure pads
+ media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
+ media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
+ media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
+ # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
+ v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
+
+Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
+used to select any supported YUV pixelformat on /dev/video4.
+
+This example configures a pipeline to capture from the ADV7180
+video decoder, assuming PAL 720x576 input signals, with Motion
+Compensated de-interlacing. The adv7180 must output sequential or
+alternating fields (field type 'seq-tb' for PAL, or 'alternate').
+$outputfmt can be any format supported by the ipu1_ic_prpvf entity
+at its output pad:
.. code-block:: none
@@ -529,11 +558,11 @@ entity at its output pad:
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]"
# Configure pads
- media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480]"
- media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]"
- media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x480 field:interlaced]"
- media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x480 field:none]"
- media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x480 field:none]"
+ media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
+ media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
+ media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x576]"
+ media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x576 field:none]"
+ media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x576 field:none]"
media-ctl -V "'ipu1_ic_prpvf':1 [fmt:$outputfmt field:none]"
Streaming can then begin on the capture device node at
--
2.17.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v5 00/12] imx-media: Fixes for interlaced capture
2018-10-17 0:00 [PATCH v5 00/12] imx-media: Fixes for interlaced capture Steve Longerbeam
` (11 preceding siblings ...)
2018-10-17 0:00 ` [PATCH v5 12/12] media: imx.rst: Update doc to reflect fixes to interlaced capture Steve Longerbeam
@ 2018-12-07 13:35 ` Hans Verkuil
2018-12-07 20:07 ` Steve Longerbeam
12 siblings, 1 reply; 28+ messages in thread
From: Hans Verkuil @ 2018-12-07 13:35 UTC (permalink / raw)
To: Steve Longerbeam, linux-media
Hi Steve,
How to proceed with this w.r.t. the two gpu ipu patches? Are those going
in first through the gpu tree? Or do they have to go in through our tree?
In that case I need Acks from whoever maintains that code.
Regards,
Hans
On 10/17/2018 02:00 AM, Steve Longerbeam wrote:
> A set of patches that fixes some bugs with capturing from an
> interlaced source, and incompatibilites between IDMAC interlace
> interweaving and 4:2:0 data write reduction.
>
> History:
> v5:
> - Added a regression fix to allow empty endpoints to CSI (fix for imx6q
> SabreAuto).
> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
> by Philipp Zabel.
> - Fixed a regression in csi_setup(), caught by Philipp.
> - Removed interweave_offset and replace with boolean interweave_swap,
> suggested by Philipp.
> - Make clear that it is IDMAC channel that does pixel reordering and
> interweave, not the CSI, in the imx.rst doc, caught by Philipp.
>
> v4:
> - rebased to latest media-tree master branch.
> - Make patch author and SoB email addresses the same.
>
> v3:
> - add support for/fix interweaved scan with YUV planar output.
> - fix bug in 4:2:0 U/V offset macros.
> - add patch that generalizes behavior of field swap in
> ipu_csi_init_interface().
> - add support for interweaved scan with field order swap.
> Suggested by Philipp Zabel.
> - in v2, inteweave scan was determined using field types of
> CSI (and PRPENCVF) at the sink and source pads. In v3, this
> has been moved one hop downstream: interweave is now determined
> using field type at source pad, and field type selected at
> capture interface. Suggested by Philipp.
> - make sure to double CSI crop target height when input field
> type in alternate.
> - more updates to media driver doc to reflect above.
>
> v2:
> - update media driver doc.
> - enable idmac interweave only if input field is sequential/alternate,
> and output field is 'interlaced*'.
> - move field try logic out of *try_fmt and into separate function.
> - fix bug with resetting crop/compose rectangles.
> - add a patch that fixes a field order bug in VDIC indirect mode.
> - remove alternate field type from V4L2_FIELD_IS_SEQUENTIAL() macro
> Suggested-by: Nicolas Dufresne <nicolas@ndufresne.ca>.
> - add macro V4L2_FIELD_IS_INTERLACED().
>
>
> Steve Longerbeam (12):
> media: videodev2.h: Add more field helper macros
> gpu: ipu-csi: Swap fields according to input/output field types
> gpu: ipu-v3: Add planar support to interlaced scan
> media: imx: Fix field negotiation
> media: imx-csi: Input connections to CSI should be optional
> media: imx-csi: Double crop height for alternate fields at sink
> media: imx: interweave and odd-chroma-row skip are incompatible
> media: imx-csi: Allow skipping odd chroma rows for YVU420
> media: imx: vdic: rely on VDIC for correct field order
> media: imx-csi: Move crop/compose reset after filling default mbus
> fields
> media: imx: Allow interweave with top/bottom lines swapped
> media: imx.rst: Update doc to reflect fixes to interlaced capture
>
> Documentation/media/v4l-drivers/imx.rst | 103 +++++++----
> drivers/gpu/ipu-v3/ipu-cpmem.c | 26 ++-
> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++----
> drivers/staging/media/imx/imx-ic-prpencvf.c | 46 +++--
> drivers/staging/media/imx/imx-media-capture.c | 14 ++
> drivers/staging/media/imx/imx-media-csi.c | 168 +++++++++++++-----
> drivers/staging/media/imx/imx-media-vdic.c | 12 +-
> include/uapi/linux/videodev2.h | 7 +
> include/video/imx-ipu-v3.h | 6 +-
> 9 files changed, 354 insertions(+), 147 deletions(-)
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v5 00/12] imx-media: Fixes for interlaced capture
2018-12-07 13:35 ` [PATCH v5 00/12] imx-media: Fixes for " Hans Verkuil
@ 2018-12-07 20:07 ` Steve Longerbeam
0 siblings, 0 replies; 28+ messages in thread
From: Steve Longerbeam @ 2018-12-07 20:07 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Hi Hans,
On 12/7/18 5:35 AM, Hans Verkuil wrote:
> Hi Steve,
>
> How to proceed with this w.r.t. the two gpu ipu patches? Are those going
> in first through the gpu tree? Or do they have to go in through our tree?
There is only one remaining gpu ipu patch in this series that is
awaiting an ack from Philipp:
"gpu: ipu-csi: Swap fields according to input/output field types"
I pinged him again.
Philipp agreed to allow the two ipu patches in the series to be merged
to media tree.
Steve
>
> In that case I need Acks from whoever maintains that code.
>
> Regards,
>
> Hans
>
> On 10/17/2018 02:00 AM, Steve Longerbeam wrote:
>> A set of patches that fixes some bugs with capturing from an
>> interlaced source, and incompatibilites between IDMAC interlace
>> interweaving and 4:2:0 data write reduction.
>>
>> History:
>> v5:
>> - Added a regression fix to allow empty endpoints to CSI (fix for imx6q
>> SabreAuto).
>> - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
>> by Philipp Zabel.
>> - Fixed a regression in csi_setup(), caught by Philipp.
>> - Removed interweave_offset and replace with boolean interweave_swap,
>> suggested by Philipp.
>> - Make clear that it is IDMAC channel that does pixel reordering and
>> interweave, not the CSI, in the imx.rst doc, caught by Philipp.
>>
>> v4:
>> - rebased to latest media-tree master branch.
>> - Make patch author and SoB email addresses the same.
>>
>> v3:
>> - add support for/fix interweaved scan with YUV planar output.
>> - fix bug in 4:2:0 U/V offset macros.
>> - add patch that generalizes behavior of field swap in
>> ipu_csi_init_interface().
>> - add support for interweaved scan with field order swap.
>> Suggested by Philipp Zabel.
>> - in v2, inteweave scan was determined using field types of
>> CSI (and PRPENCVF) at the sink and source pads. In v3, this
>> has been moved one hop downstream: interweave is now determined
>> using field type at source pad, and field type selected at
>> capture interface. Suggested by Philipp.
>> - make sure to double CSI crop target height when input field
>> type in alternate.
>> - more updates to media driver doc to reflect above.
>>
>> v2:
>> - update media driver doc.
>> - enable idmac interweave only if input field is sequential/alternate,
>> and output field is 'interlaced*'.
>> - move field try logic out of *try_fmt and into separate function.
>> - fix bug with resetting crop/compose rectangles.
>> - add a patch that fixes a field order bug in VDIC indirect mode.
>> - remove alternate field type from V4L2_FIELD_IS_SEQUENTIAL() macro
>> Suggested-by: Nicolas Dufresne <nicolas@ndufresne.ca>.
>> - add macro V4L2_FIELD_IS_INTERLACED().
>>
>>
>> Steve Longerbeam (12):
>> media: videodev2.h: Add more field helper macros
>> gpu: ipu-csi: Swap fields according to input/output field types
>> gpu: ipu-v3: Add planar support to interlaced scan
>> media: imx: Fix field negotiation
>> media: imx-csi: Input connections to CSI should be optional
>> media: imx-csi: Double crop height for alternate fields at sink
>> media: imx: interweave and odd-chroma-row skip are incompatible
>> media: imx-csi: Allow skipping odd chroma rows for YVU420
>> media: imx: vdic: rely on VDIC for correct field order
>> media: imx-csi: Move crop/compose reset after filling default mbus
>> fields
>> media: imx: Allow interweave with top/bottom lines swapped
>> media: imx.rst: Update doc to reflect fixes to interlaced capture
>>
>> Documentation/media/v4l-drivers/imx.rst | 103 +++++++----
>> drivers/gpu/ipu-v3/ipu-cpmem.c | 26 ++-
>> drivers/gpu/ipu-v3/ipu-csi.c | 119 +++++++++----
>> drivers/staging/media/imx/imx-ic-prpencvf.c | 46 +++--
>> drivers/staging/media/imx/imx-media-capture.c | 14 ++
>> drivers/staging/media/imx/imx-media-csi.c | 168 +++++++++++++-----
>> drivers/staging/media/imx/imx-media-vdic.c | 12 +-
>> include/uapi/linux/videodev2.h | 7 +
>> include/video/imx-ipu-v3.h | 6 +-
>> 9 files changed, 354 insertions(+), 147 deletions(-)
>>
^ permalink raw reply [flat|nested] 28+ messages in thread