Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
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 --- 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, 0xFF, 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,
Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
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 > --- > 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, 0xFF, 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(, mbus_cfg, mbus_fmt); > + ret = fill_csi_bus_cfg(, mbus_cfg,
Re: [PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
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 --- 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, 0xFF, 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(, mbus_cfg, mbus_fmt); + ret = fill_csi_bus_cfg(, mbus_cfg, infmt); if (ret < 0) return ret; /* set default sensor frame width and height */ -