* [PATCH 0/2] v4l: Add media bus polarity flags for HREF signal @ 2011-09-16 17:28 Sylwester Nawrocki 2011-09-16 17:28 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Sylwester Nawrocki 2011-09-16 17:28 ` [PATCH 2/2] s5p-fimc: Convert to use generic bus " Sylwester Nawrocki 0 siblings, 2 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 17:28 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim Hello, The following patche adds support for HREF signal polarity configuration through the parallel media bus flags. The second one just converts s5p-fimc driver to use generic flags. Sylwester Nawrocki (2): v4l2: Add the parallel bus HREF signal polarity flags s5p-fimc: Convert to use generic bus polarity flags drivers/media/video/s5p-fimc/fimc-reg.c | 8 ++++---- include/media/s5p_fimc.h | 7 +------ include/media/v4l2-mediabus.h | 14 ++++++++------ 3 files changed, 13 insertions(+), 16 deletions(-) Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-16 17:28 [PATCH 0/2] v4l: Add media bus polarity flags for HREF signal Sylwester Nawrocki @ 2011-09-16 17:28 ` Sylwester Nawrocki 2011-09-17 10:54 ` Laurent Pinchart 2011-09-16 17:28 ` [PATCH 2/2] s5p-fimc: Convert to use generic bus " Sylwester Nawrocki 1 sibling, 1 reply; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 17:28 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim, Sylwester Nawrocki HREF is a signal indicating valid data during single line transmission. Add corresponding flags for this signal to the set of mediabus signal polarity flags. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- include/media/v4l2-mediabus.h | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 6114007..41d8771 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -26,12 +26,14 @@ /* Note: in BT.656 mode HSYNC and VSYNC are unused */ #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5) -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6) -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) -#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1 << 4) +#define V4L2_MBUS_HREF_ACTIVE_LOW (1 << 5) +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 6) +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 7) +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 8) +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 9) +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 10) +#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 11) /* Serial flags */ /* How many lanes the client can use */ -- 1.7.6 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-16 17:28 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Sylwester Nawrocki @ 2011-09-17 10:54 ` Laurent Pinchart 2011-09-17 12:07 ` Sylwester Nawrocki 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2011-09-17 10:54 UTC (permalink / raw) To: Sylwester Nawrocki Cc: linux-media, m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim Hi Sylwester, On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: > HREF is a signal indicating valid data during single line transmission. > Add corresponding flags for this signal to the set of mediabus signal > polarity flags. So that's a data valid signal that gates the pixel data ? The OMAP3 ISP has a similar signal called WEN, and I've seen other chips using DVAL. Your patch looks good to me, except maybe for the signal name that could be made a bit more explicit (I'm not sure what most chips use though). > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > include/media/v4l2-mediabus.h | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > index 6114007..41d8771 100644 > --- a/include/media/v4l2-mediabus.h > +++ b/include/media/v4l2-mediabus.h > @@ -26,12 +26,14 @@ > /* Note: in BT.656 mode HSYNC and VSYNC are unused */ > #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) > #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) > -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) > -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5) > -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6) > -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) > -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) > -#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) > +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1 << 4) > +#define V4L2_MBUS_HREF_ACTIVE_LOW (1 << 5) > +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 6) > +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 7) > +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 8) > +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 9) > +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 10) > +#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 11) > > /* Serial flags */ > /* How many lanes the client can use */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-17 10:54 ` Laurent Pinchart @ 2011-09-17 12:07 ` Sylwester Nawrocki 2011-09-17 12:34 ` Guennadi Liakhovetski 2011-09-18 23:02 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Laurent Pinchart 0 siblings, 2 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-17 12:07 UTC (permalink / raw) To: Laurent Pinchart Cc: Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim Hi Laurent, thanks for your comments. On 09/17/2011 12:54 PM, Laurent Pinchart wrote: > Hi Sylwester, > > On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: >> HREF is a signal indicating valid data during single line transmission. >> Add corresponding flags for this signal to the set of mediabus signal >> polarity flags. > > So that's a data valid signal that gates the pixel data ? The OMAP3 ISP has a Yes, it's "horizontal window reference" signal, it's well described in this datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf AFAICS there can be also its vertical counterpart - VREF. Many devices seem to use this terminology. However, I realize, not all, as you're pointing out. So perhaps it's time for some naming contest now.. :-) > similar signal called WEN, and I've seen other chips using DVAL. Your patch > looks good to me, except maybe for the signal name that could be made a bit > more explicit (I'm not sure what most chips use though). I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. Maybe V4L2_MBUS_LINE_VALID_ACTIVE_HIGH V4L2_MBUS_LINE_VALID_ACTIVE_LOW V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH V4L2_MBUS_FRAME_VALID_ACTIVE_LOW ? Some of Aptina sensor datasheets describes those signals as LINE_VALID/FRAME_VALID, (www.aptina.com/assets/downloadDocument.do?id=76). > >> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> >> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> >> --- >> include/media/v4l2-mediabus.h | 14 ++++++++------ >> 1 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h >> index 6114007..41d8771 100644 >> --- a/include/media/v4l2-mediabus.h >> +++ b/include/media/v4l2-mediabus.h >> @@ -26,12 +26,14 @@ >> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ I've forgotten to update this: /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ >> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) >> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) >> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) >> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) >> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) >> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) >> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) >> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) >> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) >> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) >> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) >> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) >> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) >> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) >> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) >> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) -- Thanks, Sylwester ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-17 12:07 ` Sylwester Nawrocki @ 2011-09-17 12:34 ` Guennadi Liakhovetski 2011-09-17 16:06 ` Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki 2011-09-18 23:02 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Laurent Pinchart 1 sibling, 2 replies; 18+ messages in thread From: Guennadi Liakhovetski @ 2011-09-17 12:34 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Laurent Pinchart, Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim On Sat, 17 Sep 2011, Sylwester Nawrocki wrote: > Hi Laurent, > > thanks for your comments. > > On 09/17/2011 12:54 PM, Laurent Pinchart wrote: > > Hi Sylwester, > > > > On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: > >> HREF is a signal indicating valid data during single line transmission. > >> Add corresponding flags for this signal to the set of mediabus signal > >> polarity flags. > > > > So that's a data valid signal that gates the pixel data ? The OMAP3 ISP has a > > Yes, it's "horizontal window reference" signal, it's well described in this datasheet: > http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf > > AFAICS there can be also its vertical counterpart - VREF. > > Many devices seem to use this terminology. However, I realize, not all, as you're > pointing out. So perhaps it's time for some naming contest now.. :-) Hi No objections in principle, just one question though: can these signals actually be used simultaneously with respective *SYNC signals or only as an alternative? If the latter, maybe we could reuse same names by just making them more generic? Thanks Guennadi > > similar signal called WEN, and I've seen other chips using DVAL. Your patch > > looks good to me, except maybe for the signal name that could be made a bit > > more explicit (I'm not sure what most chips use though). > > I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. > > Maybe > > V4L2_MBUS_LINE_VALID_ACTIVE_HIGH > V4L2_MBUS_LINE_VALID_ACTIVE_LOW > > V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH > V4L2_MBUS_FRAME_VALID_ACTIVE_LOW > > ? > Some of Aptina sensor datasheets describes those signals as LINE_VALID/FRAME_VALID, > (www.aptina.com/assets/downloadDocument.do?id=76). > > > > >> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> > >> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> > >> --- > >> include/media/v4l2-mediabus.h | 14 ++++++++------ > >> 1 files changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > >> index 6114007..41d8771 100644 > >> --- a/include/media/v4l2-mediabus.h > >> +++ b/include/media/v4l2-mediabus.h > >> @@ -26,12 +26,14 @@ > >> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ > > I've forgotten to update this: > > /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ > > >> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) > >> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) > >> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) > >> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) > >> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) > >> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) > >> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) > >> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) > >> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) > >> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) > >> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) > >> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) > >> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) > >> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) > >> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) > >> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) > > -- > Thanks, > Sylwester > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-17 12:34 ` Guennadi Liakhovetski @ 2011-09-17 16:06 ` Sylwester Nawrocki 2011-09-18 23:05 ` Laurent Pinchart 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki 1 sibling, 1 reply; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-17 16:06 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Laurent Pinchart, Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim Hi Guennadi, On 09/17/2011 02:34 PM, Guennadi Liakhovetski wrote: > On Sat, 17 Sep 2011, Sylwester Nawrocki wrote: > >> Hi Laurent, >> >> thanks for your comments. >> >> On 09/17/2011 12:54 PM, Laurent Pinchart wrote: >>> Hi Sylwester, >>> >>> On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: >>>> HREF is a signal indicating valid data during single line transmission. >>>> Add corresponding flags for this signal to the set of mediabus signal >>>> polarity flags. >>> >>> So that's a data valid signal that gates the pixel data ? The OMAP3 ISP has a >> >> Yes, it's "horizontal window reference" signal, it's well described in this datasheet: >> http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf >> >> AFAICS there can be also its vertical counterpart - VREF. >> >> Many devices seem to use this terminology. However, I realize, not all, as you're >> pointing out. So perhaps it's time for some naming contest now.. :-) > > Hi > > No objections in principle, just one question though: can these signals > actually be used simultaneously with respective *SYNC signals or only as > an alternative? If the latter, maybe we could reuse same names by just > making them more generic? That's actually a good question. In my use cases only HREF is used as horizontal synchronization signal, i.e. physical bus interface has this signals: ->| PCLK ->| VSYNC ->| HREF ->| DATA[0:7] ->| FIELD For interlaced mode FIELD can be connected to the horizontal synchronization signal. For this case there is InvPolHSYNC bit in the host interface registers to indicate the polarity. There are 5 bits actually: InvPolPCLK InvPolVSYNC (vertical sychronization) InvPolHREF (horizontal synchronization) InvPolHSYNC (for interlaced mode only, FIELD port = horizontal sync. signal) InvPolFIELD (interlaced mode, FIELD port = FIELD signal) IMHO keeping different names for synchronization and 'data valid' signals is more clear. > > Thanks > Guennadi > >>> similar signal called WEN, and I've seen other chips using DVAL. Your patch >>> looks good to me, except maybe for the signal name that could be made a bit >>> more explicit (I'm not sure what most chips use though). >> >> I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. >> >> Maybe >> >> V4L2_MBUS_LINE_VALID_ACTIVE_HIGH >> V4L2_MBUS_LINE_VALID_ACTIVE_LOW >> >> V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH >> V4L2_MBUS_FRAME_VALID_ACTIVE_LOW >> >> ? >> Some of Aptina sensor datasheets describes those signals as LINE_VALID/FRAME_VALID, >> (www.aptina.com/assets/downloadDocument.do?id=76). >> >>> >>>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> >>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> >>>> --- >>>> include/media/v4l2-mediabus.h | 14 ++++++++------ >>>> 1 files changed, 8 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h >>>> index 6114007..41d8771 100644 >>>> --- a/include/media/v4l2-mediabus.h >>>> +++ b/include/media/v4l2-mediabus.h >>>> @@ -26,12 +26,14 @@ >>>> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ >> >> I've forgotten to update this: >> >> /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ >> >>>> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) >>>> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) >>>> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) >>>> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) >>>> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) >>>> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) >>>> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) >>>> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) >>>> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) >>>> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) >>>> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) >>>> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) -- Regards, Sylwester ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-17 16:06 ` Sylwester Nawrocki @ 2011-09-18 23:05 ` Laurent Pinchart 2011-09-19 8:48 ` Sylwester Nawrocki 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2011-09-18 23:05 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Guennadi Liakhovetski, Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim Hi Sylwester, On Saturday 17 September 2011 18:06:20 Sylwester Nawrocki wrote: > On 09/17/2011 02:34 PM, Guennadi Liakhovetski wrote: > > On Sat, 17 Sep 2011, Sylwester Nawrocki wrote: > >> On 09/17/2011 12:54 PM, Laurent Pinchart wrote: > >>> On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: > >>>> HREF is a signal indicating valid data during single line > >>>> transmission. Add corresponding flags for this signal to the set of > >>>> mediabus signal polarity flags. > >>> > >>> So that's a data valid signal that gates the pixel data ? The OMAP3 ISP > >>> has a > >> > >> Yes, it's "horizontal window reference" signal, it's well described in > >> this datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf > >> > >> AFAICS there can be also its vertical counterpart - VREF. > >> > >> Many devices seem to use this terminology. However, I realize, not all, > >> as you're pointing out. So perhaps it's time for some naming contest > >> now.. :-) > > > > No objections in principle, just one question though: can these signals > > actually be used simultaneously with respective *SYNC signals or only as > > an alternative? If the latter, maybe we could reuse same names by just > > making them more generic? > > That's actually a good question. In my use cases only HREF is used as > horizontal synchronization signal, i.e. physical bus interface has this > signals: > > ->| PCLK > ->| VSYNC > ->| HREF > ->| DATA[0:7] > ->| FIELD > > For interlaced mode FIELD can be connected to the horizontal > synchronization signal. For this case there is InvPolHSYNC bit in the host > interface registers to indicate the polarity. There are 5 bits actually: > > InvPolPCLK > InvPolVSYNC (vertical sychronization) > InvPolHREF (horizontal synchronization) > InvPolHSYNC (for interlaced mode only, FIELD port = horizontal sync. > signal) InvPolFIELD (interlaced mode, FIELD port = FIELD signal) Shouldn't this be handled through platform data only ? > IMHO keeping different names for synchronization and 'data valid' signals > is more clear. > > >>> similar signal called WEN, and I've seen other chips using DVAL. Your > >>> patch looks good to me, except maybe for the signal name that could be > >>> made a bit more explicit (I'm not sure what most chips use though). > >> > >> I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. > >> > >> Maybe > >> > >> V4L2_MBUS_LINE_VALID_ACTIVE_HIGH > >> V4L2_MBUS_LINE_VALID_ACTIVE_LOW > >> > >> V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH > >> V4L2_MBUS_FRAME_VALID_ACTIVE_LOW > >> > >> ? > >> Some of Aptina sensor datasheets describes those signals as > >> LINE_VALID/FRAME_VALID, > >> (www.aptina.com/assets/downloadDocument.do?id=76). > >> > >>>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> > >>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> > >>>> --- > >>>> > >>>> include/media/v4l2-mediabus.h | 14 ++++++++------ > >>>> 1 files changed, 8 insertions(+), 6 deletions(-) > >>>> > >>>> diff --git a/include/media/v4l2-mediabus.h > >>>> b/include/media/v4l2-mediabus.h index 6114007..41d8771 100644 > >>>> --- a/include/media/v4l2-mediabus.h > >>>> +++ b/include/media/v4l2-mediabus.h > >>>> @@ -26,12 +26,14 @@ > >>>> > >>>> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ > >> > >> I've forgotten to update this: > >> > >> /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ > >> > >>>> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) > >>>> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) > >>>> > >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) > >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) > >>>> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) > >>>> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) > >>>> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) > >>>> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) > >>>> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) > >>>> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) > >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) > >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) > >>>> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) > >>>> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) > >>>> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) > >>>> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-18 23:05 ` Laurent Pinchart @ 2011-09-19 8:48 ` Sylwester Nawrocki 0 siblings, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 8:48 UTC (permalink / raw) To: Laurent Pinchart Cc: Sylwester Nawrocki, Guennadi Liakhovetski, linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim Hi Laurent, On 09/19/2011 01:05 AM, Laurent Pinchart wrote: > On Saturday 17 September 2011 18:06:20 Sylwester Nawrocki wrote: >> On 09/17/2011 02:34 PM, Guennadi Liakhovetski wrote: >>> On Sat, 17 Sep 2011, Sylwester Nawrocki wrote: >>>> On 09/17/2011 12:54 PM, Laurent Pinchart wrote: >>>>> On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: >>>>>> HREF is a signal indicating valid data during single line >>>>>> transmission. Add corresponding flags for this signal to the set of >>>>>> mediabus signal polarity flags. >>>>> >>>>> So that's a data valid signal that gates the pixel data ? The OMAP3 ISP >>>>> has a >>>> >>>> Yes, it's "horizontal window reference" signal, it's well described in >>>> this datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf >>>> >>>> AFAICS there can be also its vertical counterpart - VREF. >>>> >>>> Many devices seem to use this terminology. However, I realize, not all, >>>> as you're pointing out. So perhaps it's time for some naming contest >>>> now.. :-) >>> >>> No objections in principle, just one question though: can these signals >>> actually be used simultaneously with respective *SYNC signals or only as >>> an alternative? If the latter, maybe we could reuse same names by just >>> making them more generic? >> >> That's actually a good question. In my use cases only HREF is used as >> horizontal synchronization signal, i.e. physical bus interface has this >> signals: >> >> ->| PCLK >> ->| VSYNC >> ->| HREF >> ->| DATA[0:7] >> ->| FIELD >> >> For interlaced mode FIELD can be connected to the horizontal >> synchronization signal. For this case there is InvPolHSYNC bit in the host >> interface registers to indicate the polarity. There are 5 bits actually: >> >> InvPolPCLK >> InvPolVSYNC (vertical sychronization) >> InvPolHREF (horizontal synchronization) >> InvPolHSYNC (for interlaced mode only, FIELD port = horizontal sync. >> signal) InvPolFIELD (interlaced mode, FIELD port = FIELD signal) > > Shouldn't this be handled through platform data only ? Indeed, this is how it's done now and I didn't intend to change that. I just wanted to replace driver's private signal polarity flag definitions with the standard ones. Do you think I should rather keep these things in driver's public header? It's of course a way of less resistance :) To make things complete I thought about adding the FIELD flags, i.e. >From 9bd11f9b14dffe877f9c546e068b4b4027c9472a Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki <s.nawrocki@samsung.com> Date: Sun, 18 Sep 2011 11:28:58 +0200 Subject: [PATCH 1/2] v4l2: Add the parallel bus HREF and FIELD signal polarity flags HREF is a signal gating valid data during single line transmission. FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 standard. Add corresponding flags for these signals to the set of media bus signal polarity flags. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- include/media/v4l2-mediabus.h | 20 +++++++++++++------- 1 files changed, 13 insertions(+), 7 deletions(-) diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 6114007..1952d9f 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -23,15 +23,21 @@ #define V4L2_MBUS_MASTER (1 << 0) #define V4L2_MBUS_SLAVE (1 << 1) /* Which signal polarities it supports */ -/* Note: in BT.656 mode HSYNC and VSYNC are unused */ +/* Note: in BT.656 mode HSYNC, HREF, FIELD, and VSYNC are unused */ #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 5) -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 6) -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) -#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) +/* HREF is a horizontal window reference signal gating valid pixel data */ +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1 << 4) +#define V4L2_MBUS_HREF_ACTIVE_LOW (1 << 5) +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 6) +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 << 7) +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 << 8) +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 9) +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 10) +#define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 11) +/* Field selection signal for interlaced scan mode */ +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 12) +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 13) /* Serial flags */ /* How many lanes the client can use */ -- 1.7.4.1 If there is more objection to the above changes then I'll drop the patch and stay with driver's private definitions. Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal 2011-09-17 12:34 ` Guennadi Liakhovetski 2011-09-17 16:06 ` Sylwester Nawrocki @ 2011-09-19 16:41 ` Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 1/2] v4l2: Add the polarity flags for parallel camera bus " Sylwester Nawrocki ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 16:41 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, laurent.pinchart, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim Hello, The following patch adds support for FIELD signal polarity configuration through the parallel media bus flags. The second one just converts s5p-fimc driver to use generic flags. Changes since v2: - dropped V4L2_MBUS_HREF* definitions, added comment on usage of V4L2_MBUS_[HV]SYNC* flags for [HV]REF signals - added V4L2_MBUS_FIELD* - modified the second patch to use HSYNC flags only Sylwester Nawrocki (2): v4l2: Add the polarity flags for parallel camera bus FIELD signal s5p-fimc: Convert to use generic media bus polarity flags drivers/media/video/s5p-fimc/fimc-reg.c | 14 +++++++++----- drivers/media/video/s5p-fimc/regs-fimc.h | 1 + include/media/s5p_fimc.h | 7 +------ include/media/v4l2-mediabus.h | 11 +++++++++-- 4 files changed, 20 insertions(+), 13 deletions(-) Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki @ 2011-09-19 16:41 ` Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 2/2] s5p-fimc: Convert to use generic media bus polarity flags Sylwester Nawrocki 2011-09-19 17:07 ` [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal Sylwester Nawrocki 2 siblings, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 16:41 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, laurent.pinchart, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim, Sylwester Nawrocki FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 standard. Add corresponding flag for configuring the FIELD signal polarity. Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the hardware that uses [HV]REF signals. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- include/media/v4l2-mediabus.h | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 6114007..586fc44 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -22,8 +22,12 @@ */ #define V4L2_MBUS_MASTER (1 << 0) #define V4L2_MBUS_SLAVE (1 << 1) -/* Which signal polarities it supports */ -/* Note: in BT.656 mode HSYNC and VSYNC are unused */ +/* + * Signal polarity flags + * Note: in BT.656 mode HSYNC, FIELD, and VSYNC are unused + * V4L2_MBUS_[HV]SYNC_* flags should be also used for specifying + * configuration of hardware that uses [HV]REF signals + */ #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) #define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) @@ -32,6 +36,9 @@ #define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) #define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) #define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) +/* Field selection signal for interlaced scan mode */ +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 12) +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 13) /* Serial flags */ /* How many lanes the client can use */ -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] s5p-fimc: Convert to use generic media bus polarity flags 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 1/2] v4l2: Add the polarity flags for parallel camera bus " Sylwester Nawrocki @ 2011-09-19 16:41 ` Sylwester Nawrocki 2011-09-19 17:07 ` [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal Sylwester Nawrocki 2 siblings, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 16:41 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, laurent.pinchart, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim, Sylwester Nawrocki Switch to generic media bus signal polarity flags and allow configuring the FIELD signal polarity. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <s.nawrocki@samsung.com> --- drivers/media/video/s5p-fimc/fimc-reg.c | 14 +++++++++----- drivers/media/video/s5p-fimc/regs-fimc.h | 1 + include/media/s5p_fimc.h | 7 +------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c b/drivers/media/video/s5p-fimc/fimc-reg.c index 2a1ae51..678d7d3 100644 --- a/drivers/media/video/s5p-fimc/fimc-reg.c +++ b/drivers/media/video/s5p-fimc/fimc-reg.c @@ -533,20 +533,24 @@ int fimc_hw_set_camera_polarity(struct fimc_dev *fimc, u32 cfg = readl(fimc->regs + S5P_CIGCTRL); cfg &= ~(S5P_CIGCTRL_INVPOLPCLK | S5P_CIGCTRL_INVPOLVSYNC | - S5P_CIGCTRL_INVPOLHREF | S5P_CIGCTRL_INVPOLHSYNC); + S5P_CIGCTRL_INVPOLHREF | S5P_CIGCTRL_INVPOLHSYNC | + S5P_CIGCTRL_INVPOLFIELD); - if (cam->flags & FIMC_CLK_INV_PCLK) + if (cam->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) cfg |= S5P_CIGCTRL_INVPOLPCLK; - if (cam->flags & FIMC_CLK_INV_VSYNC) + if (cam->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLVSYNC; - if (cam->flags & FIMC_CLK_INV_HREF) + if (cam->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLHREF; - if (cam->flags & FIMC_CLK_INV_HSYNC) + if (cam->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLHSYNC; + if (cam->flags & V4L2_MBUS_FIELD_ACTIVE_LOW) + cfg |= S5P_CIGCTRL_INVPOLFIELD; + writel(cfg, fimc->regs + S5P_CIGCTRL); return 0; diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h b/drivers/media/video/s5p-fimc/regs-fimc.h index 94d2302..c8e3b94 100644 --- a/drivers/media/video/s5p-fimc/regs-fimc.h +++ b/drivers/media/video/s5p-fimc/regs-fimc.h @@ -61,6 +61,7 @@ #define S5P_CIGCTRL_CSC_ITU601_709 (1 << 5) #define S5P_CIGCTRL_INVPOLHSYNC (1 << 4) #define S5P_CIGCTRL_SELCAM_MIPI (1 << 3) +#define S5P_CIGCTRL_INVPOLFIELD (1 << 1) #define S5P_CIGCTRL_INTERLACE (1 << 0) /* Window offset 2 */ diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h index 2b58904..688fb3f 100644 --- a/include/media/s5p_fimc.h +++ b/include/media/s5p_fimc.h @@ -19,11 +19,6 @@ enum cam_bus_type { FIMC_LCD_WB, /* FIFO link from LCD mixer */ }; -#define FIMC_CLK_INV_PCLK (1 << 0) -#define FIMC_CLK_INV_VSYNC (1 << 1) -#define FIMC_CLK_INV_HREF (1 << 2) -#define FIMC_CLK_INV_HSYNC (1 << 3) - struct i2c_board_info; /** @@ -37,7 +32,7 @@ struct i2c_board_info; * @i2c_bus_num: i2c control bus id the sensor is attached to * @mux_id: FIMC camera interface multiplexer index (separate for MIPI and ITU) * @clk_id: index of the SoC peripheral clock for sensors - * @flags: flags defining bus signals polarity inversion (High by default) + * @flags: the parallel bus flags defining signals polarity (V4L2_MBUS_*) */ struct s5p_fimc_isp_info { struct i2c_board_info *board_info; -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 1/2] v4l2: Add the polarity flags for parallel camera bus " Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 2/2] s5p-fimc: Convert to use generic media bus polarity flags Sylwester Nawrocki @ 2011-09-19 17:07 ` Sylwester Nawrocki 2011-09-20 23:12 ` Laurent Pinchart 2 siblings, 1 reply; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 17:07 UTC (permalink / raw) To: linux-media Cc: kyungmin.park, m.szyprowski, laurent.pinchart, g.liakhovetski, sw0312.kim, riverful.kim, Sylwester Nawrocki FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 standard. Add corresponding flag for configuring the FIELD signal polarity. Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the hardware that uses [HV]REF signals. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- Resending with proper bit assignment. --- include/media/v4l2-mediabus.h | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 6114007..f3a61ab 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -22,8 +22,12 @@ */ #define V4L2_MBUS_MASTER (1 << 0) #define V4L2_MBUS_SLAVE (1 << 1) -/* Which signal polarities it supports */ -/* Note: in BT.656 mode HSYNC and VSYNC are unused */ +/* + * Signal polarity flags + * Note: in BT.656 mode HSYNC, FIELD, and VSYNC are unused + * V4L2_MBUS_[HV]SYNC_* flags should be also used for specifying + * configuration of hardware that uses [HV]REF signals + */ #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) #define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) @@ -32,6 +36,9 @@ #define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) #define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) #define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) +/* Field selection signal for interlaced scan mode */ +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 10) +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 11) /* Serial flags */ /* How many lanes the client can use */ -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal 2011-09-19 17:07 ` [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal Sylwester Nawrocki @ 2011-09-20 23:12 ` Laurent Pinchart 2011-09-21 13:24 ` Sylwester Nawrocki 0 siblings, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2011-09-20 23:12 UTC (permalink / raw) To: Sylwester Nawrocki Cc: linux-media, kyungmin.park, m.szyprowski, g.liakhovetski, sw0312.kim, riverful.kim Hi Sylwester, Thanks for the patch. On Monday 19 September 2011 19:07:55 Sylwester Nawrocki wrote: > FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 > standard. Add corresponding flag for configuring the FIELD signal polarity. > Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the > hardware that uses [HV]REF signals. I like this approach better. > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Resending with proper bit assignment. > > --- > include/media/v4l2-mediabus.h | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > index 6114007..f3a61ab 100644 > --- a/include/media/v4l2-mediabus.h > +++ b/include/media/v4l2-mediabus.h > @@ -22,8 +22,12 @@ > */ > #define V4L2_MBUS_MASTER (1 << 0) > #define V4L2_MBUS_SLAVE (1 << 1) > -/* Which signal polarities it supports */ > -/* Note: in BT.656 mode HSYNC and VSYNC are unused */ > +/* > + * Signal polarity flags > + * Note: in BT.656 mode HSYNC, FIELD, and VSYNC are unused > + * V4L2_MBUS_[HV]SYNC_* flags should be also used for specifying > + * configuration of hardware that uses [HV]REF signals > + */ > #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 << 2) > #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 << 3) > #define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 << 4) > @@ -32,6 +36,9 @@ > #define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 << 7) > #define V4L2_MBUS_DATA_ACTIVE_HIGH (1 << 8) > #define V4L2_MBUS_DATA_ACTIVE_LOW (1 << 9) > +/* Field selection signal for interlaced scan mode */ > +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 10) > +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 11) What does this mean ? The FIELD signal is used to select between odd and even fields. Does "active high" mean that the field is odd or even when the signal has a high level ? The comment should make it explicit, or we could even rename those two constants to FIELD_ODD_HIGH/FIELD_ODD_LOW (or FIELD_EVEN_HIGH/FIELD_EVEN_LOW). > /* Serial flags */ > /* How many lanes the client can use */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal 2011-09-20 23:12 ` Laurent Pinchart @ 2011-09-21 13:24 ` Sylwester Nawrocki 2011-09-21 14:51 ` Sylwester Nawrocki 0 siblings, 1 reply; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-21 13:24 UTC (permalink / raw) To: Laurent Pinchart Cc: linux-media, kyungmin.park, m.szyprowski, g.liakhovetski, sw0312.kim, riverful.kim Hi Laurent, On 09/21/2011 01:12 AM, Laurent Pinchart wrote: > Hi Sylwester, > > Thanks for the patch. > > On Monday 19 September 2011 19:07:55 Sylwester Nawrocki wrote: >> FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 >> standard. Add corresponding flag for configuring the FIELD signal polarity. >> Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the >> hardware that uses [HV]REF signals. > > I like this approach better. > ... >> +/* Field selection signal for interlaced scan mode */ >> +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 10) >> +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 11) > > What does this mean ? The FIELD signal is used to select between odd and even > fields. Does "active high" mean that the field is odd or even when the signal > has a high level ? The comment should make it explicit, or we could even > rename those two constants to FIELD_ODD_HIGH/FIELD_ODD_LOW (or > FIELD_EVEN_HIGH/FIELD_EVEN_LOW). Yes, certainly I didn't think enough about this. I silently assumed that for V4L2_MBUS_FIELD_ACTIVE_HIGH FIELD = 0 selects Field1 (odd) and FIELD = 1 selects Field2 (even). I think it would be good to construct the macro so it is possibly self-explanatory, rather than requiring often to dig in the documentation. So I would go for V4L2_MBUS_FIELD_ODD_LOW/V4L2_MBUS_FIELD_ODD_HIGH. Unless someone proposes something different/better I'll send an amended version tomorrow. Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal 2011-09-21 13:24 ` Sylwester Nawrocki @ 2011-09-21 14:51 ` Sylwester Nawrocki 0 siblings, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-21 14:51 UTC (permalink / raw) To: linux-media Cc: Laurent Pinchart, kyungmin.park, m.szyprowski, g.liakhovetski, sw0312.kim, riverful.kim On 09/21/2011 03:24 PM, Sylwester Nawrocki wrote: > Hi Laurent, > > On 09/21/2011 01:12 AM, Laurent Pinchart wrote: >> Hi Sylwester, >> >> Thanks for the patch. >> >> On Monday 19 September 2011 19:07:55 Sylwester Nawrocki wrote: >>> FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 >>> standard. Add corresponding flag for configuring the FIELD signal polarity. >>> Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the >>> hardware that uses [HV]REF signals. >> >> I like this approach better. >> > ... >>> +/* Field selection signal for interlaced scan mode */ >>> +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 10) >>> +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 11) >> >> What does this mean ? The FIELD signal is used to select between odd and even >> fields. Does "active high" mean that the field is odd or even when the signal >> has a high level ? The comment should make it explicit, or we could even >> rename those two constants to FIELD_ODD_HIGH/FIELD_ODD_LOW (or >> FIELD_EVEN_HIGH/FIELD_EVEN_LOW). > > Yes, certainly I didn't think enough about this. I silently assumed that for > V4L2_MBUS_FIELD_ACTIVE_HIGH FIELD = 0 selects Field1 (odd) and FIELD = 1 selects > Field2 (even). > I think it would be good to construct the macro so it is possibly self-explanatory, > rather than requiring often to dig in the documentation. > > So I would go for V4L2_MBUS_FIELD_ODD_LOW/V4L2_MBUS_FIELD_ODD_HIGH. > Unless someone proposes something different/better I'll send an amended version > tomorrow. Thinking some more of it, V4L2_MBUS_FIELD_EVEN_HIGH/V4L2_MBUS_FIELD_EVEN_LOW is perhaps more in line with other defines where *HIGH means standard, non-inverted case. So it seems better to me. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-17 12:07 ` Sylwester Nawrocki 2011-09-17 12:34 ` Guennadi Liakhovetski @ 2011-09-18 23:02 ` Laurent Pinchart 2011-09-19 8:37 ` Sylwester Nawrocki 1 sibling, 1 reply; 18+ messages in thread From: Laurent Pinchart @ 2011-09-18 23:02 UTC (permalink / raw) To: Sylwester Nawrocki Cc: Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim Hi Sylwester, On Saturday 17 September 2011 14:07:30 Sylwester Nawrocki wrote: > On 09/17/2011 12:54 PM, Laurent Pinchart wrote: > > On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: > >> HREF is a signal indicating valid data during single line transmission. > >> Add corresponding flags for this signal to the set of mediabus signal > >> polarity flags. > > > > So that's a data valid signal that gates the pixel data ? The OMAP3 ISP > > has a > > Yes, it's "horizontal window reference" signal, it's well described in this > datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf In that specific case I would likely connect to HREF signal to the ISP HSYNC signal and ignore the sensor HSYNC signal completely :-) > AFAICS there can be also its vertical counterpart - VREF. OK, your HREF signal is thus completely unrelated to my DVAL signal. DVAL really qualifies pixel. For instance, if the sensor outputs pixels at half the pixel rate, DVAL would switch at every pixel clock cycle during a line. > Many devices seem to use this terminology. However, I realize, not all, as > you're pointing out. So perhaps it's time for some naming contest now.. > :-) > > > similar signal called WEN, and I've seen other chips using DVAL. Your > > patch looks good to me, except maybe for the signal name that could be > > made a bit more explicit (I'm not sure what most chips use though). > > I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. > > Maybe > > V4L2_MBUS_LINE_VALID_ACTIVE_HIGH > V4L2_MBUS_LINE_VALID_ACTIVE_LOW > > V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH > V4L2_MBUS_FRAME_VALID_ACTIVE_LOW > > ? > Some of Aptina sensor datasheets describes those signals as > LINE_VALID/FRAME_VALID, (www.aptina.com/assets/downloadDocument.do?id=76). LINE_VALID/FRAME_VALID are HSYNC/VSYNC. > >> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> > >> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> > >> --- > >> > >> include/media/v4l2-mediabus.h | 14 ++++++++------ > >> 1 files changed, 8 insertions(+), 6 deletions(-) > >> > >> diff --git a/include/media/v4l2-mediabus.h > >> b/include/media/v4l2-mediabus.h index 6114007..41d8771 100644 > >> --- a/include/media/v4l2-mediabus.h > >> +++ b/include/media/v4l2-mediabus.h > >> @@ -26,12 +26,14 @@ > >> > >> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ > > I've forgotten to update this: > > /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ > > >> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) > >> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) > >> > >> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) > >> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) > >> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) > >> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) > >> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) > >> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) > >> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) > >> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) > >> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) > >> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) > >> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) > >> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) > >> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) > >> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags 2011-09-18 23:02 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Laurent Pinchart @ 2011-09-19 8:37 ` Sylwester Nawrocki 0 siblings, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-19 8:37 UTC (permalink / raw) To: Laurent Pinchart Cc: Sylwester Nawrocki, linux-media, m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim On 09/19/2011 01:02 AM, Laurent Pinchart wrote: > On Saturday 17 September 2011 14:07:30 Sylwester Nawrocki wrote: >> On 09/17/2011 12:54 PM, Laurent Pinchart wrote: >>> On Friday 16 September 2011 19:28:42 Sylwester Nawrocki wrote: >>>> HREF is a signal indicating valid data during single line transmission. >>>> Add corresponding flags for this signal to the set of mediabus signal >>>> polarity flags. >>> >>> So that's a data valid signal that gates the pixel data ? The OMAP3 ISP >>> has a >> >> Yes, it's "horizontal window reference" signal, it's well described in this >> datasheet: http://www.morninghan.com/pdf/OV2640FSL_DS_(1_3).pdf > > In that specific case I would likely connect to HREF signal to the ISP HSYNC > signal and ignore the sensor HSYNC signal completely :-) > >> AFAICS there can be also its vertical counterpart - VREF. > > OK, your HREF signal is thus completely unrelated to my DVAL signal. DVAL > really qualifies pixel. For instance, if the sensor outputs pixels at half the > pixel rate, DVAL would switch at every pixel clock cycle during a line. Yeah, sounds it's entirely different. > >> Many devices seem to use this terminology. However, I realize, not all, as >> you're pointing out. So perhaps it's time for some naming contest now.. >> :-) >> >>> similar signal called WEN, and I've seen other chips using DVAL. Your >>> patch looks good to me, except maybe for the signal name that could be >>> made a bit more explicit (I'm not sure what most chips use though). >> >> I'm pretty OK with HREF/VREF. But I'm open to any better suggestions. >> >> Maybe >> >> V4L2_MBUS_LINE_VALID_ACTIVE_HIGH >> V4L2_MBUS_LINE_VALID_ACTIVE_LOW >> >> V4L2_MBUS_FRAME_VALID_ACTIVE_HIGH >> V4L2_MBUS_FRAME_VALID_ACTIVE_LOW >> >> ? >> Some of Aptina sensor datasheets describes those signals as >> LINE_VALID/FRAME_VALID, (www.aptina.com/assets/downloadDocument.do?id=76). > > LINE_VALID/FRAME_VALID are HSYNC/VSYNC. I would say these are rather inverted horizontal/vertical blanking signal. > >>>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@samsung.com> >>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com> >>>> --- >>>> >>>> include/media/v4l2-mediabus.h | 14 ++++++++------ >>>> 1 files changed, 8 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/include/media/v4l2-mediabus.h >>>> b/include/media/v4l2-mediabus.h index 6114007..41d8771 100644 >>>> --- a/include/media/v4l2-mediabus.h >>>> +++ b/include/media/v4l2-mediabus.h >>>> @@ -26,12 +26,14 @@ >>>> >>>> /* Note: in BT.656 mode HSYNC and VSYNC are unused */ >> >> I've forgotten to update this: >> >> /* Note: in BT.656 mode HSYNC, HREF and VSYNC are unused */ >> >>>> #define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1<< 2) >>>> #define V4L2_MBUS_HSYNC_ACTIVE_LOW (1<< 3) >>>> >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 4) >>>> -#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 5) >>>> -#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 6) >>>> -#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 7) >>>> -#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 8) >>>> -#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 9) >>>> +#define V4L2_MBUS_HREF_ACTIVE_HIGH (1<< 4) >>>> +#define V4L2_MBUS_HREF_ACTIVE_LOW (1<< 5) >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1<< 6) >>>> +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1<< 7) >>>> +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1<< 8) >>>> +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1<< 9) >>>> +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1<< 10) >>>> +#define V4L2_MBUS_DATA_ACTIVE_LOW (1<< 11) > Thanks -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] s5p-fimc: Convert to use generic bus polarity flags 2011-09-16 17:28 [PATCH 0/2] v4l: Add media bus polarity flags for HREF signal Sylwester Nawrocki 2011-09-16 17:28 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Sylwester Nawrocki @ 2011-09-16 17:28 ` Sylwester Nawrocki 1 sibling, 0 replies; 18+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 17:28 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, g.liakhovetski, sw0312.kim, riverful.kim, Sylwester Nawrocki Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <s.nawrocki@samsung.com> --- drivers/media/video/s5p-fimc/fimc-reg.c | 8 ++++---- include/media/s5p_fimc.h | 7 +------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c b/drivers/media/video/s5p-fimc/fimc-reg.c index 2a1ae51..5543b1b 100644 --- a/drivers/media/video/s5p-fimc/fimc-reg.c +++ b/drivers/media/video/s5p-fimc/fimc-reg.c @@ -535,16 +535,16 @@ int fimc_hw_set_camera_polarity(struct fimc_dev *fimc, cfg &= ~(S5P_CIGCTRL_INVPOLPCLK | S5P_CIGCTRL_INVPOLVSYNC | S5P_CIGCTRL_INVPOLHREF | S5P_CIGCTRL_INVPOLHSYNC); - if (cam->flags & FIMC_CLK_INV_PCLK) + if (cam->flags & V4L2_MBUS_PCLK_SAMPLE_FALLING) cfg |= S5P_CIGCTRL_INVPOLPCLK; - if (cam->flags & FIMC_CLK_INV_VSYNC) + if (cam->flags & V4L2_MBUS_VSYNC_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLVSYNC; - if (cam->flags & FIMC_CLK_INV_HREF) + if (cam->flags & V4L2_MBUS_HREF_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLHREF; - if (cam->flags & FIMC_CLK_INV_HSYNC) + if (cam->flags & V4L2_MBUS_HSYNC_ACTIVE_LOW) cfg |= S5P_CIGCTRL_INVPOLHSYNC; writel(cfg, fimc->regs + S5P_CIGCTRL); diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h index 2b58904..688fb3f 100644 --- a/include/media/s5p_fimc.h +++ b/include/media/s5p_fimc.h @@ -19,11 +19,6 @@ enum cam_bus_type { FIMC_LCD_WB, /* FIFO link from LCD mixer */ }; -#define FIMC_CLK_INV_PCLK (1 << 0) -#define FIMC_CLK_INV_VSYNC (1 << 1) -#define FIMC_CLK_INV_HREF (1 << 2) -#define FIMC_CLK_INV_HSYNC (1 << 3) - struct i2c_board_info; /** @@ -37,7 +32,7 @@ struct i2c_board_info; * @i2c_bus_num: i2c control bus id the sensor is attached to * @mux_id: FIMC camera interface multiplexer index (separate for MIPI and ITU) * @clk_id: index of the SoC peripheral clock for sensors - * @flags: flags defining bus signals polarity inversion (High by default) + * @flags: the parallel bus flags defining signals polarity (V4L2_MBUS_*) */ struct s5p_fimc_isp_info { struct i2c_board_info *board_info; -- 1.7.6 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-09-21 14:51 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-16 17:28 [PATCH 0/2] v4l: Add media bus polarity flags for HREF signal Sylwester Nawrocki 2011-09-16 17:28 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Sylwester Nawrocki 2011-09-17 10:54 ` Laurent Pinchart 2011-09-17 12:07 ` Sylwester Nawrocki 2011-09-17 12:34 ` Guennadi Liakhovetski 2011-09-17 16:06 ` Sylwester Nawrocki 2011-09-18 23:05 ` Laurent Pinchart 2011-09-19 8:48 ` Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 0/2] v4l: Add media bus polarity flags for FIELD signal Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 1/2] v4l2: Add the polarity flags for parallel camera bus " Sylwester Nawrocki 2011-09-19 16:41 ` [PATCH v2 2/2] s5p-fimc: Convert to use generic media bus polarity flags Sylwester Nawrocki 2011-09-19 17:07 ` [PATCH v3 1/2] v4l2: Add the polarity flags for parallel camera bus FIELD signal Sylwester Nawrocki 2011-09-20 23:12 ` Laurent Pinchart 2011-09-21 13:24 ` Sylwester Nawrocki 2011-09-21 14:51 ` Sylwester Nawrocki 2011-09-18 23:02 ` [PATCH/RFC 1/2] v4l2: Add the parallel bus HREF signal polarity flags Laurent Pinchart 2011-09-19 8:37 ` Sylwester Nawrocki 2011-09-16 17:28 ` [PATCH 2/2] s5p-fimc: Convert to use generic bus " Sylwester Nawrocki
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.