| Message ID | 1526987269.3671.19.camel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/22/2018 01:07 PM, Philipp Zabel wrote: > Hi Marek, > > On Fri, 2018-05-18 at 18:21 +0200, Marek Vasut wrote: >> On 05/18/2018 05:51 PM, Philipp Zabel wrote: >>> Hi Marek, >>> >>> On Sat, 2018-04-07 at 15:04 +0200, Marek Vasut wrote: >>>> The BT1120 interlaced CCIR codes are the same as BT656 ones >>>> and different than BT656 progressive CCIR codes, fix this. >>> >>> thank you for the patch, and sorry for the delay. >>> >>>> Signed-off-by: Marek Vasut <marex@denx.de> >>>> Cc: Steve Longerbeam <steve_longerbeam@mentor.com> >>>> Cc: Philipp Zabel <p.zabel@pengutronix.de> >>>> --- >>>> drivers/gpu/ipu-v3/ipu-csi.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c >>>> index caa05b0702e1..301a729581ce 100644 >>>> --- a/drivers/gpu/ipu-v3/ipu-csi.c >>>> +++ b/drivers/gpu/ipu-v3/ipu-csi.c >>>> @@ -435,12 +435,16 @@ int ipu_csi_init_interface(struct ipu_csi *csi, >>>> break; >>>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: >>>> case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: >>>> - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: >>>> - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: >>>> ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, >>>> CSI_CCIR_CODE_1); >>>> ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3); >>>> break; >>>> + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: >>>> + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: >>>> + 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); >>> >>> If these are the same as BT656 codes (so this case would be for PAL?), >>> could this just be moved up into the IPU_CSI_CLK_MODE_CCIR656_INTERLACED >>> case? Would the NTSC CCIR codes be the same as well? >> >> Dunno, I don't have any NTSC device to test. But the above was tested >> with a PAL device I had. >> >> I think the CCIR codes are different from BT656, although I might be wrong. > > The driver currently has: > > 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; > } > break; > > The PAL codes are exactly the same as in your patch. That's why I wonder > whether we should just move > case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: > case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: > up before > case IPU_CSI_CLK_MODE_CCIR656_INTERLACED: > as follows: Isn't the PAL code doing some sort of resolution check too ? Although this would probably work in my case too. > ----------8<---------- > diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c > index caa05b0702e1..7e96382f9cb1 100644 > --- a/drivers/gpu/ipu-v3/ipu-csi.c > +++ b/drivers/gpu/ipu-v3/ipu-csi.c > @@ -396,6 +396,8 @@ 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: > + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: > + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: > if (mbus_fmt->width == 720 && mbus_fmt->height == 576) { > /* > * PAL case > @@ -435,8 +437,6 @@ int ipu_csi_init_interface(struct ipu_csi *csi, > break; > case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: > case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: > - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: > - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: > ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, > CSI_CCIR_CODE_1); > ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3); > ---------->8---------- > > Does this work for you? I cannot test this anymore, but maybe it'll help someone.
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c index caa05b0702e1..7e96382f9cb1 100644 --- a/drivers/gpu/ipu-v3/ipu-csi.c +++ b/drivers/gpu/ipu-v3/ipu-csi.c @@ -396,6 +396,8 @@ 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: + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: if (mbus_fmt->width == 720 && mbus_fmt->height == 576) { /* * PAL case @@ -435,8 +437,6 @@ int ipu_csi_init_interface(struct ipu_csi *csi, break; case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, CSI_CCIR_CODE_1); ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);