On 06/01/2018 06:13 AM, Philipp Zabel wrote:
Hi Krzysztof,
On Fri, 2018-06-01 at 12:02 +0200, Krzysztof Hałasa wrote:
Steve Longerbeam <slongerbeam@xxxxxxxxx> writes:
I tend to agree, I've found conflicting info out there regarding
PAL vs. NTSC field order. And I've never liked having to guess
at input analog standard based on input # lines. I will go ahead
and remove the field order override code.
I've merged your current fix-csi-interlaced.2 branch (2018-06-01
00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
"media: adv7180: fix field type" commit and NTSC camera:
media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"
correctly sets:
"adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":1 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":2 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":0 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]
but all 3 cases seem to produce top-first interlaced frames.
The CCIR_CODE_* register dump shows no differences:
2a38014: 010D07DF 00040596 00FF0000
...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
registers depending on the height of the image.
Exactly.
Hovewer, I'm using 480
lines here, so it should be B-T instead.
My understanding is that the CCIR codes for height == 480 (NTSC)
currently capture the second field (top) first, assuming that for NTSC
the EAV/SAV codes are bottom-field-first.
So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
field that is captured first, where F=1 is the field that is marked as
second field on the wire, so top. Which means that the captured frame
has two fields captured across frame boundaries, which might be
problematic if the source data was originally progressive.
I agree, for NTSC the CSI will drop the first B field and start capturing
at the T field, and then capture fields across frame boundaries. At
least, that is if we understand how these CCIR registers work: the
CSI will look for H-S-V codes for the start and end of active and blanking
lines, that match the codes written to CCIR_CODE_1/2 for fields 0/1.
I think this must be legacy code from a Freescale BSP requirement
that the CSI must always capture in T-B order. We should remove this
code, so that the CSI always captures field 0 followed by field 1,
irrespective
of field affinity, as in:
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 5450a2d..b8b9b6d 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -398,41 +398,20 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
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);
} else {
dev_err(csi->ipu->dev,
"Unsupported CCIR656 interlaced video
mode\n");
spin_unlock_irqrestore(&csi->lock, flags);
return -EINVAL;
}
+
+ 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);
break;
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:
