| Message ID | 20220210110458.152494-1-jacopo@jmondi.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: ov5640: Rework the clock tree programming for MIPI | expand |
Hi, On 10/02/2022 13:04, Jacopo Mondi wrote: > v1: > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > > A branch for testing based on the most recent media-master is available at > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > > If anyone with a DVP setup could verify I have not broken their use case > I would very much appreciate that :) What is this based on? I'm not able to apply. "error: sha1 information is lacking or useless" Tomi
On 10/02/2022 14:03, Tomi Valkeinen wrote: > Hi, > > On 10/02/2022 13:04, Jacopo Mondi wrote: >> v1: >> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >> >> A branch for testing based on the most recent media-master is >> available at >> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >> >> If anyone with a DVP setup could verify I have not broken their use case >> I would very much appreciate that :) > > What is this based on? I'm not able to apply. "error: sha1 information > is lacking or useless" Answering to myself: The branch url above had an extra #, so my git fetch didn't get the right branch. And in that branch there's an extra commit that this series was based on. Tomi
On 10/02/2022 13:04, Jacopo Mondi wrote: > v1: > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 You could rather point to lore.kernel.org, so that the intro letter and the discussions are also visible. > A branch for testing based on the most recent media-master is available at > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > > If anyone with a DVP setup could verify I have not broken their use case > I would very much appreciate that :) > > v1 -> v2: > - rework the modes definition to process the full pixel array > - rework get_selection to report the correct BOUND and DEFAULT targets > - implement init_cfg > - minor style changes as suggested by Laurent > - test with 1 data lane Very nice! I tested this on TI's DRA76 EVM (CSI-2). UYVY and RGB565, with the following resolutions: 160 120, 176 144, 320 240, 640 480, 720 480, 720 576, 1024 768, 1280 720, 1920 1080. All work. The only issue I saw was that with 1024x768 and 1280x720 there's a thin vertical purple column on the right edge. Tomi
Hi Tomi On Thu, Feb 10, 2022 at 03:00:21PM +0200, Tomi Valkeinen wrote: > On 10/02/2022 13:04, Jacopo Mondi wrote: > > v1: > > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > > You could rather point to lore.kernel.org, so that the intro letter and the > discussions are also visible. Sure, here you go! https://lore.kernel.org/linux-media/20220131143245.128089-1-jacopo@jmondi.org/ > > > A branch for testing based on the most recent media-master is available at > > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > > > > If anyone with a DVP setup could verify I have not broken their use case > > I would very much appreciate that :) > > > > v1 -> v2: > > - rework the modes definition to process the full pixel array > > - rework get_selection to report the correct BOUND and DEFAULT targets > > - implement init_cfg > > - minor style changes as suggested by Laurent > > - test with 1 data lane > > Very nice! I tested this on TI's DRA76 EVM (CSI-2). UYVY and RGB565, with > the following resolutions: 160 120, 176 144, 320 240, 640 480, 720 480, 720 > 576, 1024 768, 1280 720, 1920 1080. Great! A 2 data lanes setup I assume ? Have you been able to test the framerate as well ? > > All work. The only issue I saw was that with 1024x768 and 1280x720 there's a > thin vertical pueple column on the right edge. Argh, you're right. I'll see if I can fix them! Thanks j > > Tomi
On 10/02/2022 19:11, Jacopo Mondi wrote: > Hi Tomi > > On Thu, Feb 10, 2022 at 03:00:21PM +0200, Tomi Valkeinen wrote: >> On 10/02/2022 13:04, Jacopo Mondi wrote: >>> v1: >>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >> >> You could rather point to lore.kernel.org, so that the intro letter and the >> discussions are also visible. > > Sure, here you go! > https://lore.kernel.org/linux-media/20220131143245.128089-1-jacopo@jmondi.org/ > >> >>> A branch for testing based on the most recent media-master is available at >>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>> >>> If anyone with a DVP setup could verify I have not broken their use case >>> I would very much appreciate that :) >>> >>> v1 -> v2: >>> - rework the modes definition to process the full pixel array >>> - rework get_selection to report the correct BOUND and DEFAULT targets >>> - implement init_cfg >>> - minor style changes as suggested by Laurent >>> - test with 1 data lane >> >> Very nice! I tested this on TI's DRA76 EVM (CSI-2). UYVY and RGB565, with >> the following resolutions: 160 120, 176 144, 320 240, 640 480, 720 480, 720 >> 576, 1024 768, 1280 720, 1920 1080. > > Great! A 2 data lanes setup I assume ? Have you been able to test the > framerate as well ? Yes, 2 data lanes. I can see that with 640x480 and 1024x768 I get ~30fps. Why does ov5640_enum_frame_interval() return an error if ov5640 is a csi sensor? Tomi
On 11/02/2022 09:55, Tomi Valkeinen wrote: > On 10/02/2022 19:11, Jacopo Mondi wrote: >> Hi Tomi >> >> On Thu, Feb 10, 2022 at 03:00:21PM +0200, Tomi Valkeinen wrote: >>> On 10/02/2022 13:04, Jacopo Mondi wrote: >>>> v1: >>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >>> >>> You could rather point to lore.kernel.org, so that the intro letter >>> and the >>> discussions are also visible. >> >> Sure, here you go! >> https://lore.kernel.org/linux-media/20220131143245.128089-1-jacopo@jmondi.org/ >> >> >>> >>>> A branch for testing based on the most recent media-master is >>>> available at >>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>>> >>>> If anyone with a DVP setup could verify I have not broken their use >>>> case >>>> I would very much appreciate that :) >>>> >>>> v1 -> v2: >>>> - rework the modes definition to process the full pixel array >>>> - rework get_selection to report the correct BOUND and DEFAULT targets >>>> - implement init_cfg >>>> - minor style changes as suggested by Laurent >>>> - test with 1 data lane >>> >>> Very nice! I tested this on TI's DRA76 EVM (CSI-2). UYVY and RGB565, >>> with >>> the following resolutions: 160 120, 176 144, 320 240, 640 480, 720 >>> 480, 720 >>> 576, 1024 768, 1280 720, 1920 1080. >> >> Great! A 2 data lanes setup I assume ? Have you been able to test the >> framerate as well ? > > Yes, 2 data lanes. I can see that with 640x480 and 1024x768 I get ~30fps. > > Why does ov5640_enum_frame_interval() return an error if ov5640 is a csi > sensor? Ah, never mind. I see the patch 18. Tomi
Hi Tomi, On Thu, Feb 10, 2022 at 03:00:21PM +0200, Tomi Valkeinen wrote: > On 10/02/2022 13:04, Jacopo Mondi wrote: > > v1: > > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > > You could rather point to lore.kernel.org, so that the intro letter and the > discussions are also visible. > > > A branch for testing based on the most recent media-master is available at > > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > > > > If anyone with a DVP setup could verify I have not broken their use case > > I would very much appreciate that :) > > > > v1 -> v2: > > - rework the modes definition to process the full pixel array > > - rework get_selection to report the correct BOUND and DEFAULT targets > > - implement init_cfg > > - minor style changes as suggested by Laurent > > - test with 1 data lane > > Very nice! I tested this on TI's DRA76 EVM (CSI-2). UYVY and RGB565, with > the following resolutions: 160 120, 176 144, 320 240, 640 480, 720 480, 720 > 576, 1024 768, 1280 720, 1920 1080. > > All work. The only issue I saw was that with 1024x768 and 1280x720 there's a > thin vertical purple column on the right edge. Thanks for spotting. I've sent a v2.1 version of "media: ov5640: Rework analog crop rectangles" where I have re-introduced the crop rectangles as they were defined in the register tables for 1024x768 and 1280x720, which are closer to what the datasheet suggests. The vertical purple column seems to be gone now. Thanks j > > Tomi
On 2/10/22 1:04 PM, Jacopo Mondi wrote: Hello Jacopo, > v1: > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > > A branch for testing based on the most recent media-master is available at > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > > If anyone with a DVP setup could verify I have not broken their use case > I would very much appreciate that :) I started testing this on my bench. So far things look good. To be able to test this, I have to revert this patch : "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" Otherwise the sensor will not power up when starting streaming. I have tested several formats, as you worked more on this sensor, could you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or 1024x768 ? I managed to get it working fine at 640x480 . The sensor looks to report valid framesizes for this mbus code : # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) 0x4001: MEDIA_BUS_FMT_JPEG_1X8 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE 0x1017: MEDIA_BUS_FMT_RGB565_1X16 0x100a: MEDIA_BUS_FMT_RGB888_1X24 0x1013: MEDIA_BUS_FMT_BGR888_1X24 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) Size Range: 160x120 - 160x120 Size Range: 176x144 - 176x144 Size Range: 320x240 - 320x240 Size Range: 640x480 - 640x480 Size Range: 720x480 - 720x480 Size Range: 720x576 - 720x576 Size Range: 1024x768 - 1024x768 Size Range: 1280x720 - 1280x720 Size Range: 1920x1080 - 1920x1080 Size Range: 2592x1944 - 2592x1944 # but the ISC does not receive any frames at 1024x768 and 1920x1080. What I can say is that the raw bayer format works at 1920x1080 , frames are received correctly. Thanks, Eugen > > v1 -> v2: > - rework the modes definition to process the full pixel array > - rework get_selection to report the correct BOUND and DEFAULT targets > - implement init_cfg > - minor style changes as suggested by Laurent > - test with 1 data lane > > Thanks > j > > Jacopo Mondi (23): > media: ov5640: Add pixel rate to modes > media: ov5604: Re-arrange modes definition > media: ov5640: Add ov5640_is_csi2() function > media: ov5640: Associate bpp with formats > media: ov5640: Add LINK_FREQ control > media: ov5640: Update pixel_rate and link_freq > media: ov5640: Rework CSI-2 clock tree > media: ov5640: Rework timings programming > media: ov5640: Fix 720x480 in RGB888 mode > media: ov5640: Rework analog crop rectangles > media: ov5640: Re-sort per-mode register tables > media: ov5640: Remove ov5640_mode_init_data > media: ov5640: Add HBLANK control > media: ov5640: Add VBLANK control > media: ov5640: Fix durations to comply with FPS > media: ov5640: Implement init_cfg > media: ov5640: Implement get_selection > media: ov5640: Limit frame_interval to DVP mode only > media: ov5640: Register device properties > media: ov5640: Add RGB565_1X16 format > media: ov5640: Add RGB888/BGR888 formats > media: ov5640: Restrict sizes to mbus code > media: ov5640: Adjust format to bpp in s_fmt > > drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > 1 file changed, 830 insertions(+), 313 deletions(-) > > -- > 2.35.0 >
Hi Eugen
thanks very much for testing
On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote:
> On 2/10/22 1:04 PM, Jacopo Mondi wrote:
>
> Hello Jacopo,
>
> > v1:
> > https://patchwork.linuxtv.org/project/linux-media/list/?series=7249
> >
> > A branch for testing based on the most recent media-master is available at
> > https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2
> >
> > If anyone with a DVP setup could verify I have not broken their use case
> > I would very much appreciate that :)
>
> I started testing this on my bench.
> So far things look good.
>
\o/
> To be able to test this, I have to revert this patch :
> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming"
>
> Otherwise the sensor will not power up when starting streaming.
>
>
> I have tested several formats, as you worked more on this sensor, could
> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or
> 1024x768 ?
I never tested the sensor driver with a parallel setup I'm afraid.
The idea behind this series is that DVP shouldn't be affected and
continue working like it did.
> I managed to get it working fine at 640x480 .
>
> The sensor looks to report valid framesizes for this mbus code :
>
> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes
> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0)
> 0x4001: MEDIA_BUS_FMT_JPEG_1X8
> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8
> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16
> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8
> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16
> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE
> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE
> 0x1017: MEDIA_BUS_FMT_RGB565_1X16
> 0x100a: MEDIA_BUS_FMT_RGB888_1X24
> 0x1013: MEDIA_BUS_FMT_BGR888_1X24
> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8
> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8
> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8
> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8
> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008
> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0)
> Size Range: 160x120 - 160x120
> Size Range: 176x144 - 176x144
> Size Range: 320x240 - 320x240
> Size Range: 640x480 - 640x480
> Size Range: 720x480 - 720x480
> Size Range: 720x576 - 720x576
> Size Range: 1024x768 - 1024x768
> Size Range: 1280x720 - 1280x720
> Size Range: 1920x1080 - 1920x1080
> Size Range: 2592x1944 - 2592x1944
> #
>
> but the ISC does not receive any frames at 1024x768 and 1920x1080.
Are 1080p and 1024x768 working without this series applied on your
setup ?
Thanks again for testin!
>
>
> What I can say is that the raw bayer format works at 1920x1080 , frames
> are received correctly.
>
> Thanks,
> Eugen
>
> >
> > v1 -> v2:
> > - rework the modes definition to process the full pixel array
> > - rework get_selection to report the correct BOUND and DEFAULT targets
> > - implement init_cfg
> > - minor style changes as suggested by Laurent
> > - test with 1 data lane
> >
> > Thanks
> > j
> >
> > Jacopo Mondi (23):
> > media: ov5640: Add pixel rate to modes
> > media: ov5604: Re-arrange modes definition
> > media: ov5640: Add ov5640_is_csi2() function
> > media: ov5640: Associate bpp with formats
> > media: ov5640: Add LINK_FREQ control
> > media: ov5640: Update pixel_rate and link_freq
> > media: ov5640: Rework CSI-2 clock tree
> > media: ov5640: Rework timings programming
> > media: ov5640: Fix 720x480 in RGB888 mode
> > media: ov5640: Rework analog crop rectangles
> > media: ov5640: Re-sort per-mode register tables
> > media: ov5640: Remove ov5640_mode_init_data
> > media: ov5640: Add HBLANK control
> > media: ov5640: Add VBLANK control
> > media: ov5640: Fix durations to comply with FPS
> > media: ov5640: Implement init_cfg
> > media: ov5640: Implement get_selection
> > media: ov5640: Limit frame_interval to DVP mode only
> > media: ov5640: Register device properties
> > media: ov5640: Add RGB565_1X16 format
> > media: ov5640: Add RGB888/BGR888 formats
> > media: ov5640: Restrict sizes to mbus code
> > media: ov5640: Adjust format to bpp in s_fmt
> >
> > drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++----------
> > 1 file changed, 830 insertions(+), 313 deletions(-)
> >
> > --
> > 2.35.0
> >
>
On 2/11/22 1:25 PM, Jacopo Mondi wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Eugen > > thanks very much for testing > > On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: >> On 2/10/22 1:04 PM, Jacopo Mondi wrote: >> >> Hello Jacopo, >> >>> v1: >>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >>> >>> A branch for testing based on the most recent media-master is available at >>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>> >>> If anyone with a DVP setup could verify I have not broken their use case >>> I would very much appreciate that :) >> >> I started testing this on my bench. >> So far things look good. >> > > \o/ > >> To be able to test this, I have to revert this patch : >> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" >> >> Otherwise the sensor will not power up when starting streaming. >> >> >> I have tested several formats, as you worked more on this sensor, could >> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or >> 1024x768 ? > > I never tested the sensor driver with a parallel setup I'm afraid. > The idea behind this series is that DVP shouldn't be affected and > continue working like it did. Hi Jacopo, I was hoping that you had more information about the driver than myself. I can tell that the parallel mode is not affected by your series from what I've seen so far. > >> I managed to get it working fine at 640x480 . >> >> The sensor looks to report valid framesizes for this mbus code : >> >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes >> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 >> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE >> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 >> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 >> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >> Size Range: 160x120 - 160x120 >> Size Range: 176x144 - 176x144 >> Size Range: 320x240 - 320x240 >> Size Range: 640x480 - 640x480 >> Size Range: 720x480 - 720x480 >> Size Range: 720x576 - 720x576 >> Size Range: 1024x768 - 1024x768 >> Size Range: 1280x720 - 1280x720 >> Size Range: 1920x1080 - 1920x1080 >> Size Range: 2592x1944 - 2592x1944 >> # >> >> but the ISC does not receive any frames at 1024x768 and 1920x1080. > > Are 1080p and 1024x768 working without this series applied on your > setup ? I remember they weren't working before either. I don't know exactly to which patches to add my Tested-by , as I have not tested the explicit patch behavior for each patch (e.g. where you add HBLANK control, I have not tested that ). Is there something particular you would like me to try , except my usual image captures ? Eugen > > Thanks again for testin! > >> >> >> What I can say is that the raw bayer format works at 1920x1080 , frames >> are received correctly. >> >> Thanks, >> Eugen >> >>> >>> v1 -> v2: >>> - rework the modes definition to process the full pixel array >>> - rework get_selection to report the correct BOUND and DEFAULT targets >>> - implement init_cfg >>> - minor style changes as suggested by Laurent >>> - test with 1 data lane >>> >>> Thanks >>> j >>> >>> Jacopo Mondi (23): >>> media: ov5640: Add pixel rate to modes >>> media: ov5604: Re-arrange modes definition >>> media: ov5640: Add ov5640_is_csi2() function >>> media: ov5640: Associate bpp with formats >>> media: ov5640: Add LINK_FREQ control >>> media: ov5640: Update pixel_rate and link_freq >>> media: ov5640: Rework CSI-2 clock tree >>> media: ov5640: Rework timings programming >>> media: ov5640: Fix 720x480 in RGB888 mode >>> media: ov5640: Rework analog crop rectangles >>> media: ov5640: Re-sort per-mode register tables >>> media: ov5640: Remove ov5640_mode_init_data >>> media: ov5640: Add HBLANK control >>> media: ov5640: Add VBLANK control >>> media: ov5640: Fix durations to comply with FPS >>> media: ov5640: Implement init_cfg >>> media: ov5640: Implement get_selection >>> media: ov5640: Limit frame_interval to DVP mode only >>> media: ov5640: Register device properties >>> media: ov5640: Add RGB565_1X16 format >>> media: ov5640: Add RGB888/BGR888 formats >>> media: ov5640: Restrict sizes to mbus code >>> media: ov5640: Adjust format to bpp in s_fmt >>> >>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- >>> 1 file changed, 830 insertions(+), 313 deletions(-) >>> >>> -- >>> 2.35.0 >>> >>
Hi Eugen, On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: > On 2/11/22 1:25 PM, Jacopo Mondi wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hi Eugen > > > > thanks very much for testing > > > > On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: > >> On 2/10/22 1:04 PM, Jacopo Mondi wrote: > >> > >> Hello Jacopo, > >> > >>> v1: > >>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > >>> > >>> A branch for testing based on the most recent media-master is available at > >>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > >>> > >>> If anyone with a DVP setup could verify I have not broken their use case > >>> I would very much appreciate that :) > >> > >> I started testing this on my bench. > >> So far things look good. > >> > > > > \o/ > > > >> To be able to test this, I have to revert this patch : > >> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" > >> > >> Otherwise the sensor will not power up when starting streaming. > >> > >> > >> I have tested several formats, as you worked more on this sensor, could > >> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or > >> 1024x768 ? > > > > I never tested the sensor driver with a parallel setup I'm afraid. > > The idea behind this series is that DVP shouldn't be affected and > > continue working like it did. > > Hi Jacopo, > > I was hoping that you had more information about the driver than myself. Not on DVP mode I'm sorry :( > I can tell that the parallel mode is not affected by your series from > what I've seen so far. That's great, thanks for testing. > > > > >> I managed to get it working fine at 640x480 . > >> > >> The sensor looks to report valid framesizes for this mbus code : > >> > >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes > >> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > >> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 > >> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > >> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > >> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > >> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > >> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > >> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE > >> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 > >> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > >> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 > >> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > >> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > >> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > >> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 > >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >> Size Range: 160x120 - 160x120 > >> Size Range: 176x144 - 176x144 > >> Size Range: 320x240 - 320x240 > >> Size Range: 640x480 - 640x480 > >> Size Range: 720x480 - 720x480 > >> Size Range: 720x576 - 720x576 > >> Size Range: 1024x768 - 1024x768 > >> Size Range: 1280x720 - 1280x720 > >> Size Range: 1920x1080 - 1920x1080 > >> Size Range: 2592x1944 - 2592x1944 > >> # > >> > >> but the ISC does not receive any frames at 1024x768 and 1920x1080. > > > > Are 1080p and 1024x768 working without this series applied on your > > setup ? > > I remember they weren't working before either. > > I don't know exactly to which patches to add my Tested-by , as I have > not tested the explicit patch behavior for each patch (e.g. where you > add HBLANK control, I have not tested that ). > I think it's good enough making sure I have not broke DVP. As you can see the driver now behaves in two different ways in DVP and MIPI mode. The DVP works as it used to, and the framerate is controlled by s_frame_interval, the frame size is fixed and the PCLK is computed dyanically to accomodate the required FPS. In MIPI mode the framerate is controlled by changing the vertical blankings. To each mode a pixel rate is assigned and userspace changes the frame duration by changing blankings. The most obvious gain is that the frame rate is controllable in a continuous interval instead of being limited to a few fixed values. It is my understanding that all drivers should be moved to this model and s_frame_intervall should be dropped, but unfortunately some bridge drivers in mainline still deman that. If you are interested, I think the DVP mode should be moved to use the same mecahnism as MIPI mode. I can't test so I haven't done so in this series. For now I think it's enough to make sure there are no regressions in DVP mode. For the tag, I think it would be appropriate to add it to the following one: 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > Is there something particular you would like me to try , except my usual > image captures ? RGB888 is weird on both the platforms I've tested on and I cannot get colors right. Does your platform support RGB888 ? Thanks j > > > Eugen > > > > > Thanks again for testin! > > > >> > >> > >> What I can say is that the raw bayer format works at 1920x1080 , frames > >> are received correctly. > >> > >> Thanks, > >> Eugen > >> > >>> > >>> v1 -> v2: > >>> - rework the modes definition to process the full pixel array > >>> - rework get_selection to report the correct BOUND and DEFAULT targets > >>> - implement init_cfg > >>> - minor style changes as suggested by Laurent > >>> - test with 1 data lane > >>> > >>> Thanks > >>> j > >>> > >>> Jacopo Mondi (23): > >>> media: ov5640: Add pixel rate to modes > >>> media: ov5604: Re-arrange modes definition > >>> media: ov5640: Add ov5640_is_csi2() function > >>> media: ov5640: Associate bpp with formats > >>> media: ov5640: Add LINK_FREQ control > >>> media: ov5640: Update pixel_rate and link_freq > >>> media: ov5640: Rework CSI-2 clock tree > >>> media: ov5640: Rework timings programming > >>> media: ov5640: Fix 720x480 in RGB888 mode > >>> media: ov5640: Rework analog crop rectangles > >>> media: ov5640: Re-sort per-mode register tables > >>> media: ov5640: Remove ov5640_mode_init_data > >>> media: ov5640: Add HBLANK control > >>> media: ov5640: Add VBLANK control > >>> media: ov5640: Fix durations to comply with FPS > >>> media: ov5640: Implement init_cfg > >>> media: ov5640: Implement get_selection > >>> media: ov5640: Limit frame_interval to DVP mode only > >>> media: ov5640: Register device properties > >>> media: ov5640: Add RGB565_1X16 format > >>> media: ov5640: Add RGB888/BGR888 formats > >>> media: ov5640: Restrict sizes to mbus code > >>> media: ov5640: Adjust format to bpp in s_fmt > >>> > >>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > >>> 1 file changed, 830 insertions(+), 313 deletions(-) > >>> > >>> -- > >>> 2.35.0 > >>> > >> >
On 2/14/22 4:38 PM, Jacopo Mondi wrote: > Hi Eugen, > > On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: >> On 2/11/22 1:25 PM, Jacopo Mondi wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Hi Eugen >>> >>> thanks very much for testing >>> >>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: >>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: >>>> >>>> Hello Jacopo, >>>> >>>>> v1: >>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >>>>> >>>>> A branch for testing based on the most recent media-master is available at >>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>>>> >>>>> If anyone with a DVP setup could verify I have not broken their use case >>>>> I would very much appreciate that :) >>>> >>>> I started testing this on my bench. >>>> So far things look good. >>>> >>> >>> \o/ >>> >>>> To be able to test this, I have to revert this patch : >>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" >>>> >>>> Otherwise the sensor will not power up when starting streaming. >>>> >>>> >>>> I have tested several formats, as you worked more on this sensor, could >>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or >>>> 1024x768 ? >>> >>> I never tested the sensor driver with a parallel setup I'm afraid. >>> The idea behind this series is that DVP shouldn't be affected and >>> continue working like it did. >> >> Hi Jacopo, >> >> I was hoping that you had more information about the driver than myself. > > Not on DVP mode I'm sorry :( > >> I can tell that the parallel mode is not affected by your series from >> what I've seen so far. > > That's great, thanks for testing. I found one change, before your series, I could attempt to capture BGGR @ 640x480, now it looks to be gone : Before: # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) Size Range: 160x120 - 160x120 Size Range: 176x144 - 176x144 Size Range: 320x240 - 320x240 Size Range: 640x480 - 640x480 Size Range: 720x480 - 720x480 Size Range: 720x576 - 720x576 Size Range: 1024x768 - 1024x768 Size Range: 1280x720 - 1280x720 Size Range: 1920x1080 - 1920x1080 Size Range: 2592x1944 - 2592x1944 After: # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) Size Range: 1280x720 - 1280x720 Size Range: 1920x1080 - 1920x1080 Size Range: 2592x1944 - 2592x1944 However trying it without your patches, it doesn't work , I don't receive frames, and I can't even set 1280x720 at all, I get -EINVAL So I assume you have been reworking the frame sizes. With your series, I have tested RGGB at 1280x720 , 1920x1080 . I also tested YUYV at 640x480 and RGB565_LE at 640x480. I can't go higher with the YUYV/RGB565, I don't receive frames anymore. I can't go higher with the bayer to 2592x1944 . But this did not work before your patches either. > >> >>> >>>> I managed to get it working fine at 640x480 . >>>> >>>> The sensor looks to report valid framesizes for this mbus code : >>>> >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes >>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 >>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE >>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 >>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 >>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >>>> Size Range: 160x120 - 160x120 >>>> Size Range: 176x144 - 176x144 >>>> Size Range: 320x240 - 320x240 >>>> Size Range: 640x480 - 640x480 >>>> Size Range: 720x480 - 720x480 >>>> Size Range: 720x576 - 720x576 >>>> Size Range: 1024x768 - 1024x768 >>>> Size Range: 1280x720 - 1280x720 >>>> Size Range: 1920x1080 - 1920x1080 >>>> Size Range: 2592x1944 - 2592x1944 >>>> # >>>> >>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. >>> >>> Are 1080p and 1024x768 working without this series applied on your >>> setup ? >> >> I remember they weren't working before either. >> >> I don't know exactly to which patches to add my Tested-by , as I have >> not tested the explicit patch behavior for each patch (e.g. where you >> add HBLANK control, I have not tested that ). >> > > I think it's good enough making sure I have not broke DVP. > > As you can see the driver now behaves in two different ways in DVP and > MIPI mode. The DVP works as it used to, and the framerate is > controlled by s_frame_interval, the frame size is fixed and the PCLK > is computed dyanically to accomodate the required FPS. In MIPI mode the > framerate is controlled by changing the vertical blankings. To each > mode a pixel rate is assigned and userspace changes the frame duration > by changing blankings. The most obvious gain is that the frame rate is > controllable in a continuous interval instead of being limited to a > few fixed values. It is my understanding that all drivers should be > moved to this model and s_frame_intervall should be dropped, but > unfortunately some bridge drivers in mainline still deman that. > > If you are interested, I think the DVP mode should be moved to use the > same mecahnism as MIPI mode. I can't test so I haven't done so in this > series. > > For now I think it's enough to make sure there are no regressions in > DVP mode. > > For the tag, I think it would be appropriate to add it to the > following one: > > 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > >> Is there something particular you would like me to try , except my usual >> image captures ? > > RGB888 is weird on both the platforms I've tested on and I cannot get > colors right. Does your platform support RGB888 ? I can't test this one unfortunately. It's a 1X24 . I have only 8 bits connected, so unless you can make this a 3x8 , there isn't anything I can do. > > Thanks > j > > >> >> >> Eugen >> >>> >>> Thanks again for testin! >>> >>>> >>>> >>>> What I can say is that the raw bayer format works at 1920x1080 , frames >>>> are received correctly. >>>> >>>> Thanks, >>>> Eugen >>>> >>>>> >>>>> v1 -> v2: >>>>> - rework the modes definition to process the full pixel array >>>>> - rework get_selection to report the correct BOUND and DEFAULT targets >>>>> - implement init_cfg >>>>> - minor style changes as suggested by Laurent >>>>> - test with 1 data lane >>>>> >>>>> Thanks >>>>> j >>>>> >>>>> Jacopo Mondi (23): >>>>> media: ov5640: Add pixel rate to modes >>>>> media: ov5604: Re-arrange modes definition >>>>> media: ov5640: Add ov5640_is_csi2() function >>>>> media: ov5640: Associate bpp with formats >>>>> media: ov5640: Add LINK_FREQ control >>>>> media: ov5640: Update pixel_rate and link_freq >>>>> media: ov5640: Rework CSI-2 clock tree >>>>> media: ov5640: Rework timings programming >>>>> media: ov5640: Fix 720x480 in RGB888 mode >>>>> media: ov5640: Rework analog crop rectangles >>>>> media: ov5640: Re-sort per-mode register tables >>>>> media: ov5640: Remove ov5640_mode_init_data >>>>> media: ov5640: Add HBLANK control >>>>> media: ov5640: Add VBLANK control >>>>> media: ov5640: Fix durations to comply with FPS >>>>> media: ov5640: Implement init_cfg >>>>> media: ov5640: Implement get_selection >>>>> media: ov5640: Limit frame_interval to DVP mode only >>>>> media: ov5640: Register device properties >>>>> media: ov5640: Add RGB565_1X16 format >>>>> media: ov5640: Add RGB888/BGR888 formats >>>>> media: ov5640: Restrict sizes to mbus code >>>>> media: ov5640: Adjust format to bpp in s_fmt >>>>> >>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- >>>>> 1 file changed, 830 insertions(+), 313 deletions(-) >>>>> >>>>> -- >>>>> 2.35.0 >>>>> >>>> >>
Hi Eugen On Mon, Feb 14, 2022 at 03:08:56PM +0000, Eugen.Hristev@microchip.com wrote: > On 2/14/22 4:38 PM, Jacopo Mondi wrote: > > Hi Eugen, > > > > On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: > >> On 2/11/22 1:25 PM, Jacopo Mondi wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> Hi Eugen > >>> > >>> thanks very much for testing > >>> > >>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: > >>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: > >>>> > >>>> Hello Jacopo, > >>>> > >>>>> v1: > >>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > >>>>> > >>>>> A branch for testing based on the most recent media-master is available at > >>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > >>>>> > >>>>> If anyone with a DVP setup could verify I have not broken their use case > >>>>> I would very much appreciate that :) > >>>> > >>>> I started testing this on my bench. > >>>> So far things look good. > >>>> > >>> > >>> \o/ > >>> > >>>> To be able to test this, I have to revert this patch : > >>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" > >>>> > >>>> Otherwise the sensor will not power up when starting streaming. > >>>> > >>>> > >>>> I have tested several formats, as you worked more on this sensor, could > >>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or > >>>> 1024x768 ? > >>> > >>> I never tested the sensor driver with a parallel setup I'm afraid. > >>> The idea behind this series is that DVP shouldn't be affected and > >>> continue working like it did. > >> > >> Hi Jacopo, > >> > >> I was hoping that you had more information about the driver than myself. > > > > Not on DVP mode I'm sorry :( > > > >> I can tell that the parallel mode is not affected by your series from > >> what I've seen so far. > > > > That's great, thanks for testing. > > > I found one change, before your series, I could attempt to capture BGGR > @ 640x480, now it looks to be gone : > > Before: > > # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > Size Range: 160x120 - 160x120 > Size Range: 176x144 - 176x144 > Size Range: 320x240 - 320x240 > Size Range: 640x480 - 640x480 > Size Range: 720x480 - 720x480 > Size Range: 720x576 - 720x576 > Size Range: 1024x768 - 1024x768 > Size Range: 1280x720 - 1280x720 > Size Range: 1920x1080 - 1920x1080 > Size Range: 2592x1944 - 2592x1944 > > > After: > > # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > Size Range: 1280x720 - 1280x720 > Size Range: 1920x1080 - 1920x1080 > Size Range: 2592x1944 - 2592x1944 > Right... I'm limiting SRGGB formats to only resolutions > 1280x720 as.. > > However trying it without your patches, it doesn't work , I don't ... they do not work for me either. So if they were not working before, maybe it's right not to enumerate them ? > receive frames, and I can't even set 1280x720 at all, I get -EINVAL Be aware that DVP mode prevents you from setting a format if the currently selected framerate is said to be "not supported" for that format > > So I assume you have been reworking the frame sizes. > > With your series, I have tested RGGB at 1280x720 , 1920x1080 . > I also tested YUYV at 640x480 and RGB565_LE at 640x480. > > I can't go higher with the YUYV/RGB565, I don't receive frames anymore. Ah, I wonder if 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS 82aebf4b7314 media: ov5640: Rework analog crop rectangles Might have impacted DVP... Should I keep separate fields for MIPI mode and leave the totals for DVP untouched ? Thanks again for your very useful testing > I can't go higher with the bayer to 2592x1944 . But this did not work > before your patches either. > > > >> > >>> > >>>> I managed to get it working fine at 640x480 . > >>>> > >>>> The sensor looks to report valid framesizes for this mbus code : > >>>> > >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes > >>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > >>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 > >>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > >>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > >>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > >>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > >>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > >>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE > >>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 > >>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > >>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 > >>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > >>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > >>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > >>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 > >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>> Size Range: 160x120 - 160x120 > >>>> Size Range: 176x144 - 176x144 > >>>> Size Range: 320x240 - 320x240 > >>>> Size Range: 640x480 - 640x480 > >>>> Size Range: 720x480 - 720x480 > >>>> Size Range: 720x576 - 720x576 > >>>> Size Range: 1024x768 - 1024x768 > >>>> Size Range: 1280x720 - 1280x720 > >>>> Size Range: 1920x1080 - 1920x1080 > >>>> Size Range: 2592x1944 - 2592x1944 > >>>> # > >>>> > >>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. > >>> > >>> Are 1080p and 1024x768 working without this series applied on your > >>> setup ? > >> > >> I remember they weren't working before either. > >> > >> I don't know exactly to which patches to add my Tested-by , as I have > >> not tested the explicit patch behavior for each patch (e.g. where you > >> add HBLANK control, I have not tested that ). > >> > > > > I think it's good enough making sure I have not broke DVP. > > > > As you can see the driver now behaves in two different ways in DVP and > > MIPI mode. The DVP works as it used to, and the framerate is > > controlled by s_frame_interval, the frame size is fixed and the PCLK > > is computed dyanically to accomodate the required FPS. In MIPI mode the > > framerate is controlled by changing the vertical blankings. To each > > mode a pixel rate is assigned and userspace changes the frame duration > > by changing blankings. The most obvious gain is that the frame rate is > > controllable in a continuous interval instead of being limited to a > > few fixed values. It is my understanding that all drivers should be > > moved to this model and s_frame_intervall should be dropped, but > > unfortunately some bridge drivers in mainline still deman that. > > > > If you are interested, I think the DVP mode should be moved to use the > > same mecahnism as MIPI mode. I can't test so I haven't done so in this > > series. > > > > For now I think it's enough to make sure there are no regressions in > > DVP mode. > > > > For the tag, I think it would be appropriate to add it to the > > following one: > > > > 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > > > >> Is there something particular you would like me to try , except my usual > >> image captures ? > > > > RGB888 is weird on both the platforms I've tested on and I cannot get > > colors right. Does your platform support RGB888 ? > > I can't test this one unfortunately. It's a 1X24 . I have only 8 bits > connected, so unless you can make this a 3x8 , there isn't anything I > can do. > > > > > Thanks > > j > > > > > >> > >> > >> Eugen > >> > >>> > >>> Thanks again for testin! > >>> > >>>> > >>>> > >>>> What I can say is that the raw bayer format works at 1920x1080 , frames > >>>> are received correctly. > >>>> > >>>> Thanks, > >>>> Eugen > >>>> > >>>>> > >>>>> v1 -> v2: > >>>>> - rework the modes definition to process the full pixel array > >>>>> - rework get_selection to report the correct BOUND and DEFAULT targets > >>>>> - implement init_cfg > >>>>> - minor style changes as suggested by Laurent > >>>>> - test with 1 data lane > >>>>> > >>>>> Thanks > >>>>> j > >>>>> > >>>>> Jacopo Mondi (23): > >>>>> media: ov5640: Add pixel rate to modes > >>>>> media: ov5604: Re-arrange modes definition > >>>>> media: ov5640: Add ov5640_is_csi2() function > >>>>> media: ov5640: Associate bpp with formats > >>>>> media: ov5640: Add LINK_FREQ control > >>>>> media: ov5640: Update pixel_rate and link_freq > >>>>> media: ov5640: Rework CSI-2 clock tree > >>>>> media: ov5640: Rework timings programming > >>>>> media: ov5640: Fix 720x480 in RGB888 mode > >>>>> media: ov5640: Rework analog crop rectangles > >>>>> media: ov5640: Re-sort per-mode register tables > >>>>> media: ov5640: Remove ov5640_mode_init_data > >>>>> media: ov5640: Add HBLANK control > >>>>> media: ov5640: Add VBLANK control > >>>>> media: ov5640: Fix durations to comply with FPS > >>>>> media: ov5640: Implement init_cfg > >>>>> media: ov5640: Implement get_selection > >>>>> media: ov5640: Limit frame_interval to DVP mode only > >>>>> media: ov5640: Register device properties > >>>>> media: ov5640: Add RGB565_1X16 format > >>>>> media: ov5640: Add RGB888/BGR888 formats > >>>>> media: ov5640: Restrict sizes to mbus code > >>>>> media: ov5640: Adjust format to bpp in s_fmt > >>>>> > >>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > >>>>> 1 file changed, 830 insertions(+), 313 deletions(-) > >>>>> > >>>>> -- > >>>>> 2.35.0 > >>>>> > >>>> > >> >
On 2/14/22 8:56 PM, Jacopo Mondi wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Eugen > > On Mon, Feb 14, 2022 at 03:08:56PM +0000, Eugen.Hristev@microchip.com wrote: >> On 2/14/22 4:38 PM, Jacopo Mondi wrote: >>> Hi Eugen, >>> >>> On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: >>>> On 2/11/22 1:25 PM, Jacopo Mondi wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>> >>>>> Hi Eugen >>>>> >>>>> thanks very much for testing >>>>> >>>>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: >>>>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: >>>>>> >>>>>> Hello Jacopo, >>>>>> >>>>>>> v1: >>>>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >>>>>>> >>>>>>> A branch for testing based on the most recent media-master is available at >>>>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>>>>>> >>>>>>> If anyone with a DVP setup could verify I have not broken their use case >>>>>>> I would very much appreciate that :) >>>>>> >>>>>> I started testing this on my bench. >>>>>> So far things look good. >>>>>> >>>>> >>>>> \o/ >>>>> >>>>>> To be able to test this, I have to revert this patch : >>>>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" >>>>>> >>>>>> Otherwise the sensor will not power up when starting streaming. >>>>>> >>>>>> >>>>>> I have tested several formats, as you worked more on this sensor, could >>>>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or >>>>>> 1024x768 ? >>>>> >>>>> I never tested the sensor driver with a parallel setup I'm afraid. >>>>> The idea behind this series is that DVP shouldn't be affected and >>>>> continue working like it did. >>>> >>>> Hi Jacopo, >>>> >>>> I was hoping that you had more information about the driver than myself. >>> >>> Not on DVP mode I'm sorry :( >>> >>>> I can tell that the parallel mode is not affected by your series from >>>> what I've seen so far. >>> >>> That's great, thanks for testing. >> >> >> I found one change, before your series, I could attempt to capture BGGR >> @ 640x480, now it looks to be gone : >> >> Before: >> >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >> Size Range: 160x120 - 160x120 >> Size Range: 176x144 - 176x144 >> Size Range: 320x240 - 320x240 >> Size Range: 640x480 - 640x480 >> Size Range: 720x480 - 720x480 >> Size Range: 720x576 - 720x576 >> Size Range: 1024x768 - 1024x768 >> Size Range: 1280x720 - 1280x720 >> Size Range: 1920x1080 - 1920x1080 >> Size Range: 2592x1944 - 2592x1944 >> >> >> After: >> >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >> Size Range: 1280x720 - 1280x720 >> Size Range: 1920x1080 - 1920x1080 >> Size Range: 2592x1944 - 2592x1944 >> > > Right... I'm limiting SRGGB formats to only resolutions > 1280x720 > as.. > >> >> However trying it without your patches, it doesn't work , I don't > > ... they do not work for me either. > > So if they were not working before, maybe it's right not to enumerate > them ? > >> receive frames, and I can't even set 1280x720 at all, I get -EINVAL > > Be aware that DVP mode prevents you from setting a format if the > currently selected framerate is said to be "not supported" for that > format > >> >> So I assume you have been reworking the frame sizes. >> >> With your series, I have tested RGGB at 1280x720 , 1920x1080 . >> I also tested YUYV at 640x480 and RGB565_LE at 640x480. >> >> I can't go higher with the YUYV/RGB565, I don't receive frames anymore. > > Ah, I wonder if > 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS > 82aebf4b7314 media: ov5640: Rework analog crop rectangles > > Might have impacted DVP... > > Should I keep separate fields for MIPI mode and leave the totals for > DVP untouched ? Hi Jacopo, I tested again without your series, and I can capture YUYV directly @1280x720 , which does not work anymore with your patches. The image has some bad pixels in it, but still, capture is pretty good. I am not sure whether it's a timing problem on capturing pixels, I uploaded it here so you can have a look : https://ibb.co/Yt8c0dJ Eugen > > Thanks again for your very useful testing > > >> I can't go higher with the bayer to 2592x1944 . But this did not work >> before your patches either. >>> >>>> >>>>> >>>>>> I managed to get it working fine at 640x480 . >>>>>> >>>>>> The sensor looks to report valid framesizes for this mbus code : >>>>>> >>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes >>>>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >>>>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 >>>>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >>>>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >>>>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >>>>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >>>>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >>>>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE >>>>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 >>>>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >>>>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 >>>>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >>>>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >>>>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >>>>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 >>>>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >>>>>> Size Range: 160x120 - 160x120 >>>>>> Size Range: 176x144 - 176x144 >>>>>> Size Range: 320x240 - 320x240 >>>>>> Size Range: 640x480 - 640x480 >>>>>> Size Range: 720x480 - 720x480 >>>>>> Size Range: 720x576 - 720x576 >>>>>> Size Range: 1024x768 - 1024x768 >>>>>> Size Range: 1280x720 - 1280x720 >>>>>> Size Range: 1920x1080 - 1920x1080 >>>>>> Size Range: 2592x1944 - 2592x1944 >>>>>> # >>>>>> >>>>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. >>>>> >>>>> Are 1080p and 1024x768 working without this series applied on your >>>>> setup ? >>>> >>>> I remember they weren't working before either. >>>> >>>> I don't know exactly to which patches to add my Tested-by , as I have >>>> not tested the explicit patch behavior for each patch (e.g. where you >>>> add HBLANK control, I have not tested that ). >>>> >>> >>> I think it's good enough making sure I have not broke DVP. >>> >>> As you can see the driver now behaves in two different ways in DVP and >>> MIPI mode. The DVP works as it used to, and the framerate is >>> controlled by s_frame_interval, the frame size is fixed and the PCLK >>> is computed dyanically to accomodate the required FPS. In MIPI mode the >>> framerate is controlled by changing the vertical blankings. To each >>> mode a pixel rate is assigned and userspace changes the frame duration >>> by changing blankings. The most obvious gain is that the frame rate is >>> controllable in a continuous interval instead of being limited to a >>> few fixed values. It is my understanding that all drivers should be >>> moved to this model and s_frame_intervall should be dropped, but >>> unfortunately some bridge drivers in mainline still deman that. >>> >>> If you are interested, I think the DVP mode should be moved to use the >>> same mecahnism as MIPI mode. I can't test so I haven't done so in this >>> series. >>> >>> For now I think it's enough to make sure there are no regressions in >>> DVP mode. >>> >>> For the tag, I think it would be appropriate to add it to the >>> following one: >>> >>> 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only >>> >>>> Is there something particular you would like me to try , except my usual >>>> image captures ? >>> >>> RGB888 is weird on both the platforms I've tested on and I cannot get >>> colors right. Does your platform support RGB888 ? >> >> I can't test this one unfortunately. It's a 1X24 . I have only 8 bits >> connected, so unless you can make this a 3x8 , there isn't anything I >> can do. >> >>> >>> Thanks >>> j >>> >>> >>>> >>>> >>>> Eugen >>>> >>>>> >>>>> Thanks again for testin! >>>>> >>>>>> >>>>>> >>>>>> What I can say is that the raw bayer format works at 1920x1080 , frames >>>>>> are received correctly. >>>>>> >>>>>> Thanks, >>>>>> Eugen >>>>>> >>>>>>> >>>>>>> v1 -> v2: >>>>>>> - rework the modes definition to process the full pixel array >>>>>>> - rework get_selection to report the correct BOUND and DEFAULT targets >>>>>>> - implement init_cfg >>>>>>> - minor style changes as suggested by Laurent >>>>>>> - test with 1 data lane >>>>>>> >>>>>>> Thanks >>>>>>> j >>>>>>> >>>>>>> Jacopo Mondi (23): >>>>>>> media: ov5640: Add pixel rate to modes >>>>>>> media: ov5604: Re-arrange modes definition >>>>>>> media: ov5640: Add ov5640_is_csi2() function >>>>>>> media: ov5640: Associate bpp with formats >>>>>>> media: ov5640: Add LINK_FREQ control >>>>>>> media: ov5640: Update pixel_rate and link_freq >>>>>>> media: ov5640: Rework CSI-2 clock tree >>>>>>> media: ov5640: Rework timings programming >>>>>>> media: ov5640: Fix 720x480 in RGB888 mode >>>>>>> media: ov5640: Rework analog crop rectangles >>>>>>> media: ov5640: Re-sort per-mode register tables >>>>>>> media: ov5640: Remove ov5640_mode_init_data >>>>>>> media: ov5640: Add HBLANK control >>>>>>> media: ov5640: Add VBLANK control >>>>>>> media: ov5640: Fix durations to comply with FPS >>>>>>> media: ov5640: Implement init_cfg >>>>>>> media: ov5640: Implement get_selection >>>>>>> media: ov5640: Limit frame_interval to DVP mode only >>>>>>> media: ov5640: Register device properties >>>>>>> media: ov5640: Add RGB565_1X16 format >>>>>>> media: ov5640: Add RGB888/BGR888 formats >>>>>>> media: ov5640: Restrict sizes to mbus code >>>>>>> media: ov5640: Adjust format to bpp in s_fmt >>>>>>> >>>>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- >>>>>>> 1 file changed, 830 insertions(+), 313 deletions(-) >>>>>>> >>>>>>> -- >>>>>>> 2.35.0 >>>>>>> >>>>>> >>>> >>
Hi Eugen On Thu, Feb 17, 2022 at 02:25:37PM +0000, Eugen.Hristev@microchip.com wrote: > On 2/14/22 8:56 PM, Jacopo Mondi wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hi Eugen > > > > On Mon, Feb 14, 2022 at 03:08:56PM +0000, Eugen.Hristev@microchip.com wrote: > >> On 2/14/22 4:38 PM, Jacopo Mondi wrote: > >>> Hi Eugen, > >>> > >>> On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: > >>>> On 2/11/22 1:25 PM, Jacopo Mondi wrote: > >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>>>> > >>>>> Hi Eugen > >>>>> > >>>>> thanks very much for testing > >>>>> > >>>>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: > >>>>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: > >>>>>> > >>>>>> Hello Jacopo, > >>>>>> > >>>>>>> v1: > >>>>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > >>>>>>> > >>>>>>> A branch for testing based on the most recent media-master is available at > >>>>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > >>>>>>> > >>>>>>> If anyone with a DVP setup could verify I have not broken their use case > >>>>>>> I would very much appreciate that :) > >>>>>> > >>>>>> I started testing this on my bench. > >>>>>> So far things look good. > >>>>>> > >>>>> > >>>>> \o/ > >>>>> > >>>>>> To be able to test this, I have to revert this patch : > >>>>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" > >>>>>> > >>>>>> Otherwise the sensor will not power up when starting streaming. > >>>>>> > >>>>>> > >>>>>> I have tested several formats, as you worked more on this sensor, could > >>>>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or > >>>>>> 1024x768 ? > >>>>> > >>>>> I never tested the sensor driver with a parallel setup I'm afraid. > >>>>> The idea behind this series is that DVP shouldn't be affected and > >>>>> continue working like it did. > >>>> > >>>> Hi Jacopo, > >>>> > >>>> I was hoping that you had more information about the driver than myself. > >>> > >>> Not on DVP mode I'm sorry :( > >>> > >>>> I can tell that the parallel mode is not affected by your series from > >>>> what I've seen so far. > >>> > >>> That's great, thanks for testing. > >> > >> > >> I found one change, before your series, I could attempt to capture BGGR > >> @ 640x480, now it looks to be gone : > >> > >> Before: > >> > >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >> Size Range: 160x120 - 160x120 > >> Size Range: 176x144 - 176x144 > >> Size Range: 320x240 - 320x240 > >> Size Range: 640x480 - 640x480 > >> Size Range: 720x480 - 720x480 > >> Size Range: 720x576 - 720x576 > >> Size Range: 1024x768 - 1024x768 > >> Size Range: 1280x720 - 1280x720 > >> Size Range: 1920x1080 - 1920x1080 > >> Size Range: 2592x1944 - 2592x1944 > >> > >> > >> After: > >> > >> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > >> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >> Size Range: 1280x720 - 1280x720 > >> Size Range: 1920x1080 - 1920x1080 > >> Size Range: 2592x1944 - 2592x1944 > >> > > > > Right... I'm limiting SRGGB formats to only resolutions > 1280x720 > > as.. > > > >> > >> However trying it without your patches, it doesn't work , I don't > > > > ... they do not work for me either. > > > > So if they were not working before, maybe it's right not to enumerate > > them ? > > > >> receive frames, and I can't even set 1280x720 at all, I get -EINVAL > > > > Be aware that DVP mode prevents you from setting a format if the > > currently selected framerate is said to be "not supported" for that > > format > > > >> > >> So I assume you have been reworking the frame sizes. > >> > >> With your series, I have tested RGGB at 1280x720 , 1920x1080 . > >> I also tested YUYV at 640x480 and RGB565_LE at 640x480. > >> > >> I can't go higher with the YUYV/RGB565, I don't receive frames anymore. > > > > Ah, I wonder if > > 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS > > 82aebf4b7314 media: ov5640: Rework analog crop rectangles > > > > Might have impacted DVP... > > > > Should I keep separate fields for MIPI mode and leave the totals for > > DVP untouched ? > > Hi Jacopo, > > I tested again without your series, and I can capture YUYV directly > @1280x720 , which does not work anymore with your patches. > > The image has some bad pixels in it, but still, capture is pretty good. > I am not sure whether it's a timing problem on capturing pixels, I > uploaded it here so you can have a look : > > https://ibb.co/Yt8c0dJ > This is without my series ? Or with it applied ? Sorry for the dumb question :) > Eugen > > > > > Thanks again for your very useful testing > > > > > >> I can't go higher with the bayer to 2592x1944 . But this did not work > >> before your patches either. > >>> > >>>> > >>>>> > >>>>>> I managed to get it working fine at 640x480 . > >>>>>> > >>>>>> The sensor looks to report valid framesizes for this mbus code : > >>>>>> > >>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes > >>>>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > >>>>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 > >>>>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > >>>>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > >>>>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > >>>>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > >>>>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > >>>>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE > >>>>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 > >>>>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > >>>>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 > >>>>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > >>>>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > >>>>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > >>>>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > >>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 > >>>>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>>>> Size Range: 160x120 - 160x120 > >>>>>> Size Range: 176x144 - 176x144 > >>>>>> Size Range: 320x240 - 320x240 > >>>>>> Size Range: 640x480 - 640x480 > >>>>>> Size Range: 720x480 - 720x480 > >>>>>> Size Range: 720x576 - 720x576 > >>>>>> Size Range: 1024x768 - 1024x768 > >>>>>> Size Range: 1280x720 - 1280x720 > >>>>>> Size Range: 1920x1080 - 1920x1080 > >>>>>> Size Range: 2592x1944 - 2592x1944 > >>>>>> # > >>>>>> > >>>>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. > >>>>> > >>>>> Are 1080p and 1024x768 working without this series applied on your > >>>>> setup ? > >>>> > >>>> I remember they weren't working before either. > >>>> > >>>> I don't know exactly to which patches to add my Tested-by , as I have > >>>> not tested the explicit patch behavior for each patch (e.g. where you > >>>> add HBLANK control, I have not tested that ). > >>>> > >>> > >>> I think it's good enough making sure I have not broke DVP. > >>> > >>> As you can see the driver now behaves in two different ways in DVP and > >>> MIPI mode. The DVP works as it used to, and the framerate is > >>> controlled by s_frame_interval, the frame size is fixed and the PCLK > >>> is computed dyanically to accomodate the required FPS. In MIPI mode the > >>> framerate is controlled by changing the vertical blankings. To each > >>> mode a pixel rate is assigned and userspace changes the frame duration > >>> by changing blankings. The most obvious gain is that the frame rate is > >>> controllable in a continuous interval instead of being limited to a > >>> few fixed values. It is my understanding that all drivers should be > >>> moved to this model and s_frame_intervall should be dropped, but > >>> unfortunately some bridge drivers in mainline still deman that. > >>> > >>> If you are interested, I think the DVP mode should be moved to use the > >>> same mecahnism as MIPI mode. I can't test so I haven't done so in this > >>> series. > >>> > >>> For now I think it's enough to make sure there are no regressions in > >>> DVP mode. > >>> > >>> For the tag, I think it would be appropriate to add it to the > >>> following one: > >>> > >>> 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > >>> > >>>> Is there something particular you would like me to try , except my usual > >>>> image captures ? > >>> > >>> RGB888 is weird on both the platforms I've tested on and I cannot get > >>> colors right. Does your platform support RGB888 ? > >> > >> I can't test this one unfortunately. It's a 1X24 . I have only 8 bits > >> connected, so unless you can make this a 3x8 , there isn't anything I > >> can do. > >> > >>> > >>> Thanks > >>> j > >>> > >>> > >>>> > >>>> > >>>> Eugen > >>>> > >>>>> > >>>>> Thanks again for testin! > >>>>> > >>>>>> > >>>>>> > >>>>>> What I can say is that the raw bayer format works at 1920x1080 , frames > >>>>>> are received correctly. > >>>>>> > >>>>>> Thanks, > >>>>>> Eugen > >>>>>> > >>>>>>> > >>>>>>> v1 -> v2: > >>>>>>> - rework the modes definition to process the full pixel array > >>>>>>> - rework get_selection to report the correct BOUND and DEFAULT targets > >>>>>>> - implement init_cfg > >>>>>>> - minor style changes as suggested by Laurent > >>>>>>> - test with 1 data lane > >>>>>>> > >>>>>>> Thanks > >>>>>>> j > >>>>>>> > >>>>>>> Jacopo Mondi (23): > >>>>>>> media: ov5640: Add pixel rate to modes > >>>>>>> media: ov5604: Re-arrange modes definition > >>>>>>> media: ov5640: Add ov5640_is_csi2() function > >>>>>>> media: ov5640: Associate bpp with formats > >>>>>>> media: ov5640: Add LINK_FREQ control > >>>>>>> media: ov5640: Update pixel_rate and link_freq > >>>>>>> media: ov5640: Rework CSI-2 clock tree > >>>>>>> media: ov5640: Rework timings programming > >>>>>>> media: ov5640: Fix 720x480 in RGB888 mode > >>>>>>> media: ov5640: Rework analog crop rectangles > >>>>>>> media: ov5640: Re-sort per-mode register tables > >>>>>>> media: ov5640: Remove ov5640_mode_init_data > >>>>>>> media: ov5640: Add HBLANK control > >>>>>>> media: ov5640: Add VBLANK control > >>>>>>> media: ov5640: Fix durations to comply with FPS > >>>>>>> media: ov5640: Implement init_cfg > >>>>>>> media: ov5640: Implement get_selection > >>>>>>> media: ov5640: Limit frame_interval to DVP mode only > >>>>>>> media: ov5640: Register device properties > >>>>>>> media: ov5640: Add RGB565_1X16 format > >>>>>>> media: ov5640: Add RGB888/BGR888 formats > >>>>>>> media: ov5640: Restrict sizes to mbus code > >>>>>>> media: ov5640: Adjust format to bpp in s_fmt > >>>>>>> > >>>>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > >>>>>>> 1 file changed, 830 insertions(+), 313 deletions(-) > >>>>>>> > >>>>>>> -- > >>>>>>> 2.35.0 > >>>>>>> > >>>>>> > >>>> > >> >
On 2/21/22 10:51 AM, Jacopo Mondi wrote: > Hi Eugen > > On Thu, Feb 17, 2022 at 02:25:37PM +0000, Eugen.Hristev@microchip.com wrote: >> On 2/14/22 8:56 PM, Jacopo Mondi wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Hi Eugen >>> >>> On Mon, Feb 14, 2022 at 03:08:56PM +0000, Eugen.Hristev@microchip.com wrote: >>>> On 2/14/22 4:38 PM, Jacopo Mondi wrote: >>>>> Hi Eugen, >>>>> >>>>> On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: >>>>>> On 2/11/22 1:25 PM, Jacopo Mondi wrote: >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>>>> >>>>>>> Hi Eugen >>>>>>> >>>>>>> thanks very much for testing >>>>>>> >>>>>>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: >>>>>>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: >>>>>>>> >>>>>>>> Hello Jacopo, >>>>>>>> >>>>>>>>> v1: >>>>>>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 >>>>>>>>> >>>>>>>>> A branch for testing based on the most recent media-master is available at >>>>>>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 >>>>>>>>> >>>>>>>>> If anyone with a DVP setup could verify I have not broken their use case >>>>>>>>> I would very much appreciate that :) >>>>>>>> >>>>>>>> I started testing this on my bench. >>>>>>>> So far things look good. >>>>>>>> >>>>>>> >>>>>>> \o/ >>>>>>> >>>>>>>> To be able to test this, I have to revert this patch : >>>>>>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" >>>>>>>> >>>>>>>> Otherwise the sensor will not power up when starting streaming. >>>>>>>> >>>>>>>> >>>>>>>> I have tested several formats, as you worked more on this sensor, could >>>>>>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or >>>>>>>> 1024x768 ? >>>>>>> >>>>>>> I never tested the sensor driver with a parallel setup I'm afraid. >>>>>>> The idea behind this series is that DVP shouldn't be affected and >>>>>>> continue working like it did. >>>>>> >>>>>> Hi Jacopo, >>>>>> >>>>>> I was hoping that you had more information about the driver than myself. >>>>> >>>>> Not on DVP mode I'm sorry :( >>>>> >>>>>> I can tell that the parallel mode is not affected by your series from >>>>>> what I've seen so far. >>>>> >>>>> That's great, thanks for testing. >>>> >>>> >>>> I found one change, before your series, I could attempt to capture BGGR >>>> @ 640x480, now it looks to be gone : >>>> >>>> Before: >>>> >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >>>> Size Range: 160x120 - 160x120 >>>> Size Range: 176x144 - 176x144 >>>> Size Range: 320x240 - 320x240 >>>> Size Range: 640x480 - 640x480 >>>> Size Range: 720x480 - 720x480 >>>> Size Range: 720x576 - 720x576 >>>> Size Range: 1024x768 - 1024x768 >>>> Size Range: 1280x720 - 1280x720 >>>> Size Range: 1920x1080 - 1920x1080 >>>> Size Range: 2592x1944 - 2592x1944 >>>> >>>> >>>> After: >>>> >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >>>> Size Range: 1280x720 - 1280x720 >>>> Size Range: 1920x1080 - 1920x1080 >>>> Size Range: 2592x1944 - 2592x1944 >>>> >>> >>> Right... I'm limiting SRGGB formats to only resolutions > 1280x720 >>> as.. >>> >>>> >>>> However trying it without your patches, it doesn't work , I don't >>> >>> ... they do not work for me either. >>> >>> So if they were not working before, maybe it's right not to enumerate >>> them ? >>> >>>> receive frames, and I can't even set 1280x720 at all, I get -EINVAL >>> >>> Be aware that DVP mode prevents you from setting a format if the >>> currently selected framerate is said to be "not supported" for that >>> format >>> >>>> >>>> So I assume you have been reworking the frame sizes. >>>> >>>> With your series, I have tested RGGB at 1280x720 , 1920x1080 . >>>> I also tested YUYV at 640x480 and RGB565_LE at 640x480. >>>> >>>> I can't go higher with the YUYV/RGB565, I don't receive frames anymore. >>> >>> Ah, I wonder if >>> 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS >>> 82aebf4b7314 media: ov5640: Rework analog crop rectangles >>> >>> Might have impacted DVP... >>> >>> Should I keep separate fields for MIPI mode and leave the totals for >>> DVP untouched ? >> >> Hi Jacopo, >> >> I tested again without your series, and I can capture YUYV directly >> @1280x720 , which does not work anymore with your patches. >> >> The image has some bad pixels in it, but still, capture is pretty good. >> I am not sure whether it's a timing problem on capturing pixels, I >> uploaded it here so you can have a look : >> >> https://ibb.co/Yt8c0dJ >> > > This is without my series ? Or with it applied ? Sorry for the dumb > question :) This photo is *without* your series. *With* your series, I cannot capture YUYV at this resolution at all. No frames received. > >> Eugen >> >>> >>> Thanks again for your very useful testing >>> >>> >>>> I can't go higher with the bayer to 2592x1944 . But this did not work >>>> before your patches either. >>>>> >>>>>> >>>>>>> >>>>>>>> I managed to get it working fine at 640x480 . >>>>>>>> >>>>>>>> The sensor looks to report valid framesizes for this mbus code : >>>>>>>> >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes >>>>>>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) >>>>>>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 >>>>>>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 >>>>>>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 >>>>>>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 >>>>>>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 >>>>>>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE >>>>>>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE >>>>>>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 >>>>>>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 >>>>>>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 >>>>>>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 >>>>>>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 >>>>>>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 >>>>>>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 >>>>>>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) >>>>>>>> Size Range: 160x120 - 160x120 >>>>>>>> Size Range: 176x144 - 176x144 >>>>>>>> Size Range: 320x240 - 320x240 >>>>>>>> Size Range: 640x480 - 640x480 >>>>>>>> Size Range: 720x480 - 720x480 >>>>>>>> Size Range: 720x576 - 720x576 >>>>>>>> Size Range: 1024x768 - 1024x768 >>>>>>>> Size Range: 1280x720 - 1280x720 >>>>>>>> Size Range: 1920x1080 - 1920x1080 >>>>>>>> Size Range: 2592x1944 - 2592x1944 >>>>>>>> # >>>>>>>> >>>>>>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. >>>>>>> >>>>>>> Are 1080p and 1024x768 working without this series applied on your >>>>>>> setup ? >>>>>> >>>>>> I remember they weren't working before either. >>>>>> >>>>>> I don't know exactly to which patches to add my Tested-by , as I have >>>>>> not tested the explicit patch behavior for each patch (e.g. where you >>>>>> add HBLANK control, I have not tested that ). >>>>>> >>>>> >>>>> I think it's good enough making sure I have not broke DVP. >>>>> >>>>> As you can see the driver now behaves in two different ways in DVP and >>>>> MIPI mode. The DVP works as it used to, and the framerate is >>>>> controlled by s_frame_interval, the frame size is fixed and the PCLK >>>>> is computed dyanically to accomodate the required FPS. In MIPI mode the >>>>> framerate is controlled by changing the vertical blankings. To each >>>>> mode a pixel rate is assigned and userspace changes the frame duration >>>>> by changing blankings. The most obvious gain is that the frame rate is >>>>> controllable in a continuous interval instead of being limited to a >>>>> few fixed values. It is my understanding that all drivers should be >>>>> moved to this model and s_frame_intervall should be dropped, but >>>>> unfortunately some bridge drivers in mainline still deman that. >>>>> >>>>> If you are interested, I think the DVP mode should be moved to use the >>>>> same mecahnism as MIPI mode. I can't test so I haven't done so in this >>>>> series. >>>>> >>>>> For now I think it's enough to make sure there are no regressions in >>>>> DVP mode. >>>>> >>>>> For the tag, I think it would be appropriate to add it to the >>>>> following one: >>>>> >>>>> 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only >>>>> >>>>>> Is there something particular you would like me to try , except my usual >>>>>> image captures ? >>>>> >>>>> RGB888 is weird on both the platforms I've tested on and I cannot get >>>>> colors right. Does your platform support RGB888 ? >>>> >>>> I can't test this one unfortunately. It's a 1X24 . I have only 8 bits >>>> connected, so unless you can make this a 3x8 , there isn't anything I >>>> can do. >>>> >>>>> >>>>> Thanks >>>>> j >>>>> >>>>> >>>>>> >>>>>> >>>>>> Eugen >>>>>> >>>>>>> >>>>>>> Thanks again for testin! >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> What I can say is that the raw bayer format works at 1920x1080 , frames >>>>>>>> are received correctly. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Eugen >>>>>>>> >>>>>>>>> >>>>>>>>> v1 -> v2: >>>>>>>>> - rework the modes definition to process the full pixel array >>>>>>>>> - rework get_selection to report the correct BOUND and DEFAULT targets >>>>>>>>> - implement init_cfg >>>>>>>>> - minor style changes as suggested by Laurent >>>>>>>>> - test with 1 data lane >>>>>>>>> >>>>>>>>> Thanks >>>>>>>>> j >>>>>>>>> >>>>>>>>> Jacopo Mondi (23): >>>>>>>>> media: ov5640: Add pixel rate to modes >>>>>>>>> media: ov5604: Re-arrange modes definition >>>>>>>>> media: ov5640: Add ov5640_is_csi2() function >>>>>>>>> media: ov5640: Associate bpp with formats >>>>>>>>> media: ov5640: Add LINK_FREQ control >>>>>>>>> media: ov5640: Update pixel_rate and link_freq >>>>>>>>> media: ov5640: Rework CSI-2 clock tree >>>>>>>>> media: ov5640: Rework timings programming >>>>>>>>> media: ov5640: Fix 720x480 in RGB888 mode >>>>>>>>> media: ov5640: Rework analog crop rectangles >>>>>>>>> media: ov5640: Re-sort per-mode register tables >>>>>>>>> media: ov5640: Remove ov5640_mode_init_data >>>>>>>>> media: ov5640: Add HBLANK control >>>>>>>>> media: ov5640: Add VBLANK control >>>>>>>>> media: ov5640: Fix durations to comply with FPS >>>>>>>>> media: ov5640: Implement init_cfg >>>>>>>>> media: ov5640: Implement get_selection >>>>>>>>> media: ov5640: Limit frame_interval to DVP mode only >>>>>>>>> media: ov5640: Register device properties >>>>>>>>> media: ov5640: Add RGB565_1X16 format >>>>>>>>> media: ov5640: Add RGB888/BGR888 formats >>>>>>>>> media: ov5640: Restrict sizes to mbus code >>>>>>>>> media: ov5640: Adjust format to bpp in s_fmt >>>>>>>>> >>>>>>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- >>>>>>>>> 1 file changed, 830 insertions(+), 313 deletions(-) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> 2.35.0 >>>>>>>>> >>>>>>>> >>>>>> >>>> >>
Hi Eugen On Mon, Feb 21, 2022 at 09:04:39AM +0000, Eugen.Hristev@microchip.com wrote: > On 2/21/22 10:51 AM, Jacopo Mondi wrote: > > Hi Eugen > > > > On Thu, Feb 17, 2022 at 02:25:37PM +0000, Eugen.Hristev@microchip.com wrote: > >> On 2/14/22 8:56 PM, Jacopo Mondi wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> Hi Eugen > >>> > >>> On Mon, Feb 14, 2022 at 03:08:56PM +0000, Eugen.Hristev@microchip.com wrote: > >>>> On 2/14/22 4:38 PM, Jacopo Mondi wrote: > >>>>> Hi Eugen, > >>>>> > >>>>> On Mon, Feb 14, 2022 at 02:06:02PM +0000, Eugen.Hristev@microchip.com wrote: > >>>>>> On 2/11/22 1:25 PM, Jacopo Mondi wrote: > >>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>>>>>> > >>>>>>> Hi Eugen > >>>>>>> > >>>>>>> thanks very much for testing > >>>>>>> > >>>>>>> On Fri, Feb 11, 2022 at 10:09:04AM +0000, Eugen.Hristev@microchip.com wrote: > >>>>>>>> On 2/10/22 1:04 PM, Jacopo Mondi wrote: > >>>>>>>> > >>>>>>>> Hello Jacopo, > >>>>>>>> > >>>>>>>>> v1: > >>>>>>>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=7249 > >>>>>>>>> > >>>>>>>>> A branch for testing based on the most recent media-master is available at > >>>>>>>>> https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v2 > >>>>>>>>> > >>>>>>>>> If anyone with a DVP setup could verify I have not broken their use case > >>>>>>>>> I would very much appreciate that :) > >>>>>>>> > >>>>>>>> I started testing this on my bench. > >>>>>>>> So far things look good. > >>>>>>>> > >>>>>>> > >>>>>>> \o/ > >>>>>>> > >>>>>>>> To be able to test this, I have to revert this patch : > >>>>>>>> "media: i2c: ov5640: Remain in power down for DVP mode unless streaming" > >>>>>>>> > >>>>>>>> Otherwise the sensor will not power up when starting streaming. > >>>>>>>> > >>>>>>>> > >>>>>>>> I have tested several formats, as you worked more on this sensor, could > >>>>>>>> you tell me, does format YUYV_2x8 work in parallel mode at 1920x1080 or > >>>>>>>> 1024x768 ? > >>>>>>> > >>>>>>> I never tested the sensor driver with a parallel setup I'm afraid. > >>>>>>> The idea behind this series is that DVP shouldn't be affected and > >>>>>>> continue working like it did. > >>>>>> > >>>>>> Hi Jacopo, > >>>>>> > >>>>>> I was hoping that you had more information about the driver than myself. > >>>>> > >>>>> Not on DVP mode I'm sorry :( > >>>>> > >>>>>> I can tell that the parallel mode is not affected by your series from > >>>>>> what I've seen so far. > >>>>> > >>>>> That's great, thanks for testing. > >>>> > >>>> > >>>> I found one change, before your series, I could attempt to capture BGGR > >>>> @ 640x480, now it looks to be gone : > >>>> > >>>> Before: > >>>> > >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>> Size Range: 160x120 - 160x120 > >>>> Size Range: 176x144 - 176x144 > >>>> Size Range: 320x240 - 320x240 > >>>> Size Range: 640x480 - 640x480 > >>>> Size Range: 720x480 - 720x480 > >>>> Size Range: 720x576 - 720x576 > >>>> Size Range: 1024x768 - 1024x768 > >>>> Size Range: 1280x720 - 1280x720 > >>>> Size Range: 1920x1080 - 1920x1080 > >>>> Size Range: 2592x1944 - 2592x1944 > >>>> > >>>> > >>>> After: > >>>> > >>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x3001 > >>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>> Size Range: 1280x720 - 1280x720 > >>>> Size Range: 1920x1080 - 1920x1080 > >>>> Size Range: 2592x1944 - 2592x1944 > >>>> > >>> > >>> Right... I'm limiting SRGGB formats to only resolutions > 1280x720 > >>> as.. > >>> > >>>> > >>>> However trying it without your patches, it doesn't work , I don't > >>> > >>> ... they do not work for me either. > >>> > >>> So if they were not working before, maybe it's right not to enumerate > >>> them ? > >>> > >>>> receive frames, and I can't even set 1280x720 at all, I get -EINVAL > >>> > >>> Be aware that DVP mode prevents you from setting a format if the > >>> currently selected framerate is said to be "not supported" for that > >>> format > >>> > >>>> > >>>> So I assume you have been reworking the frame sizes. > >>>> > >>>> With your series, I have tested RGGB at 1280x720 , 1920x1080 . > >>>> I also tested YUYV at 640x480 and RGB565_LE at 640x480. > >>>> > >>>> I can't go higher with the YUYV/RGB565, I don't receive frames anymore. > >>> > >>> Ah, I wonder if > >>> 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS > >>> 82aebf4b7314 media: ov5640: Rework analog crop rectangles > >>> > >>> Might have impacted DVP... > >>> > >>> Should I keep separate fields for MIPI mode and leave the totals for > >>> DVP untouched ? > >> > >> Hi Jacopo, > >> > >> I tested again without your series, and I can capture YUYV directly > >> @1280x720 , which does not work anymore with your patches. > >> > >> The image has some bad pixels in it, but still, capture is pretty good. > >> I am not sure whether it's a timing problem on capturing pixels, I > >> uploaded it here so you can have a look : > >> > >> https://ibb.co/Yt8c0dJ > >> > > > > This is without my series ? Or with it applied ? Sorry for the dumb > > question :) > > This photo is *without* your series. *With* your series, I cannot > capture YUYV at this resolution at all. No frames received. > Ok, thanks for clarifying. I'll send a new version with the DVP timings and the CSI-2 ones separated, to make sure parallel mode is less affected as possible. Thanks j > > > >> Eugen > >> > >>> > >>> Thanks again for your very useful testing > >>> > >>> > >>>> I can't go higher with the bayer to 2592x1944 . But this did not work > >>>> before your patches either. > >>>>> > >>>>>> > >>>>>>> > >>>>>>>> I managed to get it working fine at 640x480 . > >>>>>>>> > >>>>>>>> The sensor looks to report valid framesizes for this mbus code : > >>>>>>>> > >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes > >>>>>>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > >>>>>>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 > >>>>>>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > >>>>>>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > >>>>>>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > >>>>>>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > >>>>>>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > >>>>>>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE > >>>>>>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 > >>>>>>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > >>>>>>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 > >>>>>>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > >>>>>>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > >>>>>>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > >>>>>>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 > >>>>>>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>>>>>> Size Range: 160x120 - 160x120 > >>>>>>>> Size Range: 176x144 - 176x144 > >>>>>>>> Size Range: 320x240 - 320x240 > >>>>>>>> Size Range: 640x480 - 640x480 > >>>>>>>> Size Range: 720x480 - 720x480 > >>>>>>>> Size Range: 720x576 - 720x576 > >>>>>>>> Size Range: 1024x768 - 1024x768 > >>>>>>>> Size Range: 1280x720 - 1280x720 > >>>>>>>> Size Range: 1920x1080 - 1920x1080 > >>>>>>>> Size Range: 2592x1944 - 2592x1944 > >>>>>>>> # > >>>>>>>> > >>>>>>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. > >>>>>>> > >>>>>>> Are 1080p and 1024x768 working without this series applied on your > >>>>>>> setup ? > >>>>>> > >>>>>> I remember they weren't working before either. > >>>>>> > >>>>>> I don't know exactly to which patches to add my Tested-by , as I have > >>>>>> not tested the explicit patch behavior for each patch (e.g. where you > >>>>>> add HBLANK control, I have not tested that ). > >>>>>> > >>>>> > >>>>> I think it's good enough making sure I have not broke DVP. > >>>>> > >>>>> As you can see the driver now behaves in two different ways in DVP and > >>>>> MIPI mode. The DVP works as it used to, and the framerate is > >>>>> controlled by s_frame_interval, the frame size is fixed and the PCLK > >>>>> is computed dyanically to accomodate the required FPS. In MIPI mode the > >>>>> framerate is controlled by changing the vertical blankings. To each > >>>>> mode a pixel rate is assigned and userspace changes the frame duration > >>>>> by changing blankings. The most obvious gain is that the frame rate is > >>>>> controllable in a continuous interval instead of being limited to a > >>>>> few fixed values. It is my understanding that all drivers should be > >>>>> moved to this model and s_frame_intervall should be dropped, but > >>>>> unfortunately some bridge drivers in mainline still deman that. > >>>>> > >>>>> If you are interested, I think the DVP mode should be moved to use the > >>>>> same mecahnism as MIPI mode. I can't test so I haven't done so in this > >>>>> series. > >>>>> > >>>>> For now I think it's enough to make sure there are no regressions in > >>>>> DVP mode. > >>>>> > >>>>> For the tag, I think it would be appropriate to add it to the > >>>>> following one: > >>>>> > >>>>> 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > >>>>> > >>>>>> Is there something particular you would like me to try , except my usual > >>>>>> image captures ? > >>>>> > >>>>> RGB888 is weird on both the platforms I've tested on and I cannot get > >>>>> colors right. Does your platform support RGB888 ? > >>>> > >>>> I can't test this one unfortunately. It's a 1X24 . I have only 8 bits > >>>> connected, so unless you can make this a 3x8 , there isn't anything I > >>>> can do. > >>>> > >>>>> > >>>>> Thanks > >>>>> j > >>>>> > >>>>> > >>>>>> > >>>>>> > >>>>>> Eugen > >>>>>> > >>>>>>> > >>>>>>> Thanks again for testin! > >>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> What I can say is that the raw bayer format works at 1920x1080 , frames > >>>>>>>> are received correctly. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Eugen > >>>>>>>> > >>>>>>>>> > >>>>>>>>> v1 -> v2: > >>>>>>>>> - rework the modes definition to process the full pixel array > >>>>>>>>> - rework get_selection to report the correct BOUND and DEFAULT targets > >>>>>>>>> - implement init_cfg > >>>>>>>>> - minor style changes as suggested by Laurent > >>>>>>>>> - test with 1 data lane > >>>>>>>>> > >>>>>>>>> Thanks > >>>>>>>>> j > >>>>>>>>> > >>>>>>>>> Jacopo Mondi (23): > >>>>>>>>> media: ov5640: Add pixel rate to modes > >>>>>>>>> media: ov5604: Re-arrange modes definition > >>>>>>>>> media: ov5640: Add ov5640_is_csi2() function > >>>>>>>>> media: ov5640: Associate bpp with formats > >>>>>>>>> media: ov5640: Add LINK_FREQ control > >>>>>>>>> media: ov5640: Update pixel_rate and link_freq > >>>>>>>>> media: ov5640: Rework CSI-2 clock tree > >>>>>>>>> media: ov5640: Rework timings programming > >>>>>>>>> media: ov5640: Fix 720x480 in RGB888 mode > >>>>>>>>> media: ov5640: Rework analog crop rectangles > >>>>>>>>> media: ov5640: Re-sort per-mode register tables > >>>>>>>>> media: ov5640: Remove ov5640_mode_init_data > >>>>>>>>> media: ov5640: Add HBLANK control > >>>>>>>>> media: ov5640: Add VBLANK control > >>>>>>>>> media: ov5640: Fix durations to comply with FPS > >>>>>>>>> media: ov5640: Implement init_cfg > >>>>>>>>> media: ov5640: Implement get_selection > >>>>>>>>> media: ov5640: Limit frame_interval to DVP mode only > >>>>>>>>> media: ov5640: Register device properties > >>>>>>>>> media: ov5640: Add RGB565_1X16 format > >>>>>>>>> media: ov5640: Add RGB888/BGR888 formats > >>>>>>>>> media: ov5640: Restrict sizes to mbus code > >>>>>>>>> media: ov5640: Adjust format to bpp in s_fmt > >>>>>>>>> > >>>>>>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > >>>>>>>>> 1 file changed, 830 insertions(+), 313 deletions(-) > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> 2.35.0 > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >> >
Hi again Eugen one more last favour :) On Mon, Feb 21, 2022 at 09:04:39AM +0000, Eugen.Hristev@microchip.com wrote: > On 2/21/22 10:51 AM, Jacopo Mondi wrote: > > Hi Eugen > > > >>>> > >>>> So I assume you have been reworking the frame sizes. > >>>> > >>>> With your series, I have tested RGGB at 1280x720 , 1920x1080 . > >>>> I also tested YUYV at 640x480 and RGB565_LE at 640x480. > >>>> > >>>> I can't go higher with the YUYV/RGB565, I don't receive frames anymore. > >>> > >>> Ah, I wonder if > >>> 07b49ac59fd9 media: ov5640: Fix durations to comply with FPS > >>> 82aebf4b7314 media: ov5640: Rework analog crop rectangles > >>> > >>> Might have impacted DVP... > >>> > >>> Should I keep separate fields for MIPI mode and leave the totals for > >>> DVP untouched ? Could you try to revert these two ? I'm reworking the series and knowing where I break DVP might be useful. In example, I think if: 82aebf4b7314 media: ov5640: Rework analog crop rectangles is harmless it should be kept for DVP as well. Thanks j > >> > >> Hi Jacopo, > >> > >> I tested again without your series, and I can capture YUYV directly > >> @1280x720 , which does not work anymore with your patches. > >> > >> The image has some bad pixels in it, but still, capture is pretty good. > >> I am not sure whether it's a timing problem on capturing pixels, I > >> uploaded it here so you can have a look : > >> > >> https://ibb.co/Yt8c0dJ > >> > > > > This is without my series ? Or with it applied ? Sorry for the dumb > > question :) > > This photo is *without* your series. *With* your series, I cannot > capture YUYV at this resolution at all. No frames received. > > > > >> Eugen > >> > >>> > >>> Thanks again for your very useful testing > >>> > >>> > >>>> I can't go higher with the bayer to 2592x1944 . But this did not work > >>>> before your patches either. > >>>>> > >>>>>> > >>>>>>> > >>>>>>>> I managed to get it working fine at 640x480 . > >>>>>>>> > >>>>>>>> The sensor looks to report valid framesizes for this mbus code : > >>>>>>>> > >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-mbus-codes > >>>>>>>> \ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0) > >>>>>>>> 0x4001: MEDIA_BUS_FMT_JPEG_1X8 > >>>>>>>> 0x2006: MEDIA_BUS_FMT_UYVY8_2X8 > >>>>>>>> 0x200f: MEDIA_BUS_FMT_UYVY8_1X16 > >>>>>>>> 0x2008: MEDIA_BUS_FMT_YUYV8_2X8 > >>>>>>>> 0x2011: MEDIA_BUS_FMT_YUYV8_1X16 > >>>>>>>> 0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE > >>>>>>>> 0x1007: MEDIA_BUS_FMT_RGB565_2X8_BE > >>>>>>>> 0x1017: MEDIA_BUS_FMT_RGB565_1X16 > >>>>>>>> 0x100a: MEDIA_BUS_FMT_RGB888_1X24 > >>>>>>>> 0x1013: MEDIA_BUS_FMT_BGR888_1X24 > >>>>>>>> 0x3001: MEDIA_BUS_FMT_SBGGR8_1X8 > >>>>>>>> 0x3013: MEDIA_BUS_FMT_SGBRG8_1X8 > >>>>>>>> 0x3002: MEDIA_BUS_FMT_SGRBG8_1X8 > >>>>>>>> 0x3014: MEDIA_BUS_FMT_SRGGB8_1X8 > >>>>>>>> # v4l2-ctl -d /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x2008 > >>>>>>>> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0) > >>>>>>>> Size Range: 160x120 - 160x120 > >>>>>>>> Size Range: 176x144 - 176x144 > >>>>>>>> Size Range: 320x240 - 320x240 > >>>>>>>> Size Range: 640x480 - 640x480 > >>>>>>>> Size Range: 720x480 - 720x480 > >>>>>>>> Size Range: 720x576 - 720x576 > >>>>>>>> Size Range: 1024x768 - 1024x768 > >>>>>>>> Size Range: 1280x720 - 1280x720 > >>>>>>>> Size Range: 1920x1080 - 1920x1080 > >>>>>>>> Size Range: 2592x1944 - 2592x1944 > >>>>>>>> # > >>>>>>>> > >>>>>>>> but the ISC does not receive any frames at 1024x768 and 1920x1080. > >>>>>>> > >>>>>>> Are 1080p and 1024x768 working without this series applied on your > >>>>>>> setup ? > >>>>>> > >>>>>> I remember they weren't working before either. > >>>>>> > >>>>>> I don't know exactly to which patches to add my Tested-by , as I have > >>>>>> not tested the explicit patch behavior for each patch (e.g. where you > >>>>>> add HBLANK control, I have not tested that ). > >>>>>> > >>>>> > >>>>> I think it's good enough making sure I have not broke DVP. > >>>>> > >>>>> As you can see the driver now behaves in two different ways in DVP and > >>>>> MIPI mode. The DVP works as it used to, and the framerate is > >>>>> controlled by s_frame_interval, the frame size is fixed and the PCLK > >>>>> is computed dyanically to accomodate the required FPS. In MIPI mode the > >>>>> framerate is controlled by changing the vertical blankings. To each > >>>>> mode a pixel rate is assigned and userspace changes the frame duration > >>>>> by changing blankings. The most obvious gain is that the frame rate is > >>>>> controllable in a continuous interval instead of being limited to a > >>>>> few fixed values. It is my understanding that all drivers should be > >>>>> moved to this model and s_frame_intervall should be dropped, but > >>>>> unfortunately some bridge drivers in mainline still deman that. > >>>>> > >>>>> If you are interested, I think the DVP mode should be moved to use the > >>>>> same mecahnism as MIPI mode. I can't test so I haven't done so in this > >>>>> series. > >>>>> > >>>>> For now I think it's enough to make sure there are no regressions in > >>>>> DVP mode. > >>>>> > >>>>> For the tag, I think it would be appropriate to add it to the > >>>>> following one: > >>>>> > >>>>> 91ae667b0d47 media: ov5640: Limit frame_interval to DVP mode only > >>>>> > >>>>>> Is there something particular you would like me to try , except my usual > >>>>>> image captures ? > >>>>> > >>>>> RGB888 is weird on both the platforms I've tested on and I cannot get > >>>>> colors right. Does your platform support RGB888 ? > >>>> > >>>> I can't test this one unfortunately. It's a 1X24 . I have only 8 bits > >>>> connected, so unless you can make this a 3x8 , there isn't anything I > >>>> can do. > >>>> > >>>>> > >>>>> Thanks > >>>>> j > >>>>> > >>>>> > >>>>>> > >>>>>> > >>>>>> Eugen > >>>>>> > >>>>>>> > >>>>>>> Thanks again for testin! > >>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> What I can say is that the raw bayer format works at 1920x1080 , frames > >>>>>>>> are received correctly. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Eugen > >>>>>>>> > >>>>>>>>> > >>>>>>>>> v1 -> v2: > >>>>>>>>> - rework the modes definition to process the full pixel array > >>>>>>>>> - rework get_selection to report the correct BOUND and DEFAULT targets > >>>>>>>>> - implement init_cfg > >>>>>>>>> - minor style changes as suggested by Laurent > >>>>>>>>> - test with 1 data lane > >>>>>>>>> > >>>>>>>>> Thanks > >>>>>>>>> j > >>>>>>>>> > >>>>>>>>> Jacopo Mondi (23): > >>>>>>>>> media: ov5640: Add pixel rate to modes > >>>>>>>>> media: ov5604: Re-arrange modes definition > >>>>>>>>> media: ov5640: Add ov5640_is_csi2() function > >>>>>>>>> media: ov5640: Associate bpp with formats > >>>>>>>>> media: ov5640: Add LINK_FREQ control > >>>>>>>>> media: ov5640: Update pixel_rate and link_freq > >>>>>>>>> media: ov5640: Rework CSI-2 clock tree > >>>>>>>>> media: ov5640: Rework timings programming > >>>>>>>>> media: ov5640: Fix 720x480 in RGB888 mode > >>>>>>>>> media: ov5640: Rework analog crop rectangles > >>>>>>>>> media: ov5640: Re-sort per-mode register tables > >>>>>>>>> media: ov5640: Remove ov5640_mode_init_data > >>>>>>>>> media: ov5640: Add HBLANK control > >>>>>>>>> media: ov5640: Add VBLANK control > >>>>>>>>> media: ov5640: Fix durations to comply with FPS > >>>>>>>>> media: ov5640: Implement init_cfg > >>>>>>>>> media: ov5640: Implement get_selection > >>>>>>>>> media: ov5640: Limit frame_interval to DVP mode only > >>>>>>>>> media: ov5640: Register device properties > >>>>>>>>> media: ov5640: Add RGB565_1X16 format > >>>>>>>>> media: ov5640: Add RGB888/BGR888 formats > >>>>>>>>> media: ov5640: Restrict sizes to mbus code > >>>>>>>>> media: ov5640: Adjust format to bpp in s_fmt > >>>>>>>>> > >>>>>>>>> drivers/media/i2c/ov5640.c | 1143 ++++++++++++++++++++++++++---------- > >>>>>>>>> 1 file changed, 830 insertions(+), 313 deletions(-) > >>>>>>>>> > >>>>>>>>> -- > >>>>>>>>> 2.35.0 > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>> > >> >