* [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes @ 2018-04-07 13:04 Marek Vasut 2018-05-18 15:51 ` Philipp Zabel 0 siblings, 1 reply; 5+ messages in thread From: Marek Vasut @ 2018-04-07 13:04 UTC (permalink / raw) To: linux-media; +Cc: Marek Vasut, Steve Longerbeam, Philipp Zabel The BT1120 interlaced CCIR codes are the same as BT656 ones and different than BT656 progressive CCIR codes, fix this. 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); + break; case IPU_CSI_CLK_MODE_GATED_CLK: case IPU_CSI_CLK_MODE_NONGATED_CLK: ipu_csi_write(csi, 0, CSI_CCIR_CODE_1); -- 2.16.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes 2018-04-07 13:04 [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes Marek Vasut @ 2018-05-18 15:51 ` Philipp Zabel 2018-05-18 16:21 ` Marek Vasut 0 siblings, 1 reply; 5+ messages in thread From: Philipp Zabel @ 2018-05-18 15:51 UTC (permalink / raw) To: Marek Vasut, linux-media; +Cc: Steve Longerbeam 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? regards Philipp ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes 2018-05-18 15:51 ` Philipp Zabel @ 2018-05-18 16:21 ` Marek Vasut 2018-05-22 11:07 ` Philipp Zabel 0 siblings, 1 reply; 5+ messages in thread From: Marek Vasut @ 2018-05-18 16:21 UTC (permalink / raw) To: Philipp Zabel, linux-media; +Cc: Steve Longerbeam 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. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes 2018-05-18 16:21 ` Marek Vasut @ 2018-05-22 11:07 ` Philipp Zabel 2018-05-23 9:44 ` Marek Vasut 0 siblings, 1 reply; 5+ messages in thread From: Philipp Zabel @ 2018-05-22 11:07 UTC (permalink / raw) To: Marek Vasut, linux-media; +Cc: Steve Longerbeam 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: ----------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? regards Philipp ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes 2018-05-22 11:07 ` Philipp Zabel @ 2018-05-23 9:44 ` Marek Vasut 0 siblings, 0 replies; 5+ messages in thread From: Marek Vasut @ 2018-05-23 9:44 UTC (permalink / raw) To: Philipp Zabel, linux-media; +Cc: Steve Longerbeam 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. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-23 11:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-07 13:04 [PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes Marek Vasut 2018-05-18 15:51 ` Philipp Zabel 2018-05-18 16:21 ` Marek Vasut 2018-05-22 11:07 ` Philipp Zabel 2018-05-23 9:44 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.