Re: [PATCH v6 01/12] media: ov5640: Fix set format regression

2018-12-03 Thread Adam Ford
On Mon, Dec 3, 2018 at 2:45 AM Maxime Ripard  wrote:
>
> From: Jacopo Mondi 
>
> The set_fmt operations updates the sensor format only when the image format
> is changed. When only the image sizes gets changed, the format do not get
> updated causing the sensor to always report the one that was previously in
> use.
>
> Without this patch, updating frame size only fails:
>   [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> With this patch applied:
>   [fmt:UYVY8_2X8/1024x768@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> Fixes: 6949d864776e ("media: ov5640: do not change mode if format or frame
> interval is unchanged")
> Signed-off-by: Jacopo Mondi 
> Signed-off-by: Maxime Ripard 

Tested-by: Adam Ford  #imx6 w/ CSI2 interface on
4.19.6 and 4.20-RC5
> ---
>  drivers/media/i2c/ov5640.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index a91d91715d00..807bd0e386a4 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2021,6 +2021,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> const struct ov5640_mode_info *new_mode;
> struct v4l2_mbus_framefmt *mbus_fmt = >format;
> +   struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> if (format->pad != 0)
> @@ -2038,22 +2039,20 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> if (ret)
> goto out;
>
> -   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -   struct v4l2_mbus_framefmt *fmt =
> -   v4l2_subdev_get_try_format(sd, cfg, 0);
> +   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> +   fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +   else
> +   fmt = >fmt;
>
> -   *fmt = *mbus_fmt;
> -   goto out;
> -   }
> +   *fmt = *mbus_fmt;
>
> if (new_mode != sensor->current_mode) {
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> }
> -   if (mbus_fmt->code != sensor->fmt.code) {
> -   sensor->fmt = *mbus_fmt;
> +   if (mbus_fmt->code != sensor->fmt.code)
> sensor->pending_fmt_change = true;
> -   }
> +
>  out:
> mutex_unlock(>lock);
> return ret;
> --
> git-series 0.9.1


Re: [PATCH 2/2] media: ov5640: make MIPI clock depend on mode

2018-12-03 Thread Adam Ford
On Mon, Dec 3, 2018 at 10:28 AM Adam Ford  wrote:
>
> On Thu, Nov 29, 2018 at 8:49 AM Jacopo Mondi  
> wrote:
> >
> > The MIPI clock generation tree uses the MIPI_DIV divider to generate both 
> > the
> > MIPI SCALER CLOCK and the PIXEL_CLOCK. It seems the MIPI BIT CLK frequency 
> > is
> > generated by either the MIPI SCALER or the PIXEL CLOCK, depending if the
> > currently applied image mode uses the scaler or not.
> >
> > Make the MIPI_DIV selection value depend on the subsampling mode used by the
> > currently applied image mode.
> >
> > Tested with:
> > 172x144 320x240, 640x480, 1024x756, 1024x768, 1280x720, 1920x1080 in YUYV 
> > mode
> > at 10, 15, 20, and 30 FPS with MIPI CSI-2 2 lanes mode.
> >
> > Signed-off-by: Jacopo Mondi 
>
> I am not able to apply this patch to 4.19.6, 4.19-RC5, or Linux-media master 
> [1]
>
> Is there a specific branch/repo somewhere I can pull?  I was able to
> apply patch 1/2 just fine, but 2/2 wouldn't apply
>

I also attempted to apply to [2] without success.

> [1] - git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git

[2] - git://linuxtv.org/media_tree.git

>
> adam
> > ---
> > Maxime,
> >this patch is not just a cosmetic updates to the 'set_mipi_pclk()'
> > comment block as I anticipated, but it actually fix the framerate handling
> > compared for CSI-2, which in you v5 results halved for most modes. I have
> > not included any "Fixes:" tag, as I hope this patch will get in with your 
> > v5.
> >
> > That's my bad, as in the patches I sent to be applied on top of your v4 I
> > forgot to add a change, and the requested SCLK rate was always half of what
> > it was actually required.
> >
> > Also, I have left out from this patches most of Sam's improvements (better 
> > SCLK
> > selection policies, and programming of register OV5640_REG_PCLK_PERIOD as 
> > for
> > from testing they're not required, and I don't want to pile more patches 
> > than
> > necessary for the next merge window, not to slow down the clock tree rework
> > inclusion. We can get back to it after it got merged.
> >
> > This are the test result obtained by capturing 100 frames with yavta and
> > inspecting the reported fps results.
> >
> > Results are pretty neat, 2592x1944 mode apart, which runs a bit slow.
> >
> > capturing 176x144 @ 10FPS
> > Captured 100 frames in 10.019398 seconds (9.980640 fps, 505898.657683 B/s).
> > capturing 176x144 @ 15FPS
> > Captured 100 frames in 6.681860 seconds (14.965892 fps, 758591.132803 B/s).
> > capturing 176x144 @ 20FPS
> > Captured 100 frames in 5.056204 seconds (19.777683 fps, 1002491.196755 B/s).
> > capturing 176x144 @ 30FPS
> > Captured 100 frames in 3.357204 seconds (29.786686 fps, 1509827.521040 B/s).
> >
> > capturing 320x240 @ 10FPS
> > Captured 100 frames in 10.015432 seconds (9.984591 fps, 1533633.245951 B/s).
> > capturing 320x240 @ 15FPS
> > Captured 100 frames in 6.684035 seconds (14.961021 fps, 2298012.872049 B/s).
> > capturing 320x240 @ 20FPS
> > Captured 100 frames in 5.019164 seconds (19.923635 fps, 3060270.391218 B/s).
> > capturing 320x240 @ 30FPS
> > Captured 100 frames in 3.352991 seconds (29.824115 fps, 4580984.103432 B/s).
> >
> > capturing 640x480 @ 10FPS
> > Captured 100 frames in 9.990389 seconds (10.009620 fps, 6149910.678538 B/s).
> > capturing 640x480 @ 15FPS
> > Captured 100 frames in 6.856242 seconds (14.585249 fps, 8961176.838123 B/s).
> > capturing 640x480 @ 20FPS
> > Captured 100 frames in 5.030264 seconds (19.879670 fps, 12214069.053476 
> > B/s).
> > capturing 640x480 @ 30FPS
> > Captured 100 frames in 3.364612 seconds (29.721103 fps, 18260645.750580 
> > B/s).
> >
> > capturing 720x480 @ 10FPS
> > Captured 100 frames in 10.022488 seconds (9.977562 fps, 6896491.169279 B/s).
> > capturing 720x480 @ 15FPS
> > Captured 100 frames in 6.891968 seconds (14.509643 fps, 10029065.232208 
> > B/s).
> > capturing 720x480 @ 20FPS
> > Captured 100 frames in 5.234133 seconds (19.105360 fps, 13205624.616211 
> > B/s).
> > capturing 720x480 @ 30FPS
> > Captured 100 frames in 3.602298 seconds (27.760055 fps, 19187750.044688 
> > B/s).
> >
> > capturing 720x576 @ 10FPS
> > Captured 100 frames in 10.197244 seconds (9.806571 fps, 8133961.937805 B/s).
> > capturing 720x576 @ 15FPS
> > Captured 100 frames in 6.925244 seconds (14.439924 fps, 11977050.339261 
> > B/s).
> > capturing 720x576 @ 20FPS
> > Captured 100 frames in 4.68 secon

Re: [PATCH 1/2] media: ov5640: Fix set format regression

2018-12-03 Thread Adam Ford
On Thu, Nov 29, 2018 at 8:48 AM Jacopo Mondi  wrote:
>
> The set_fmt operations updates the sensor format only when the image format
> is changed. When only the image sizes gets changed, the format do not get
> updated causing the sensor to always report the one that was previously in
> use.
>
> Without this patch, updating frame size only fails:
>   [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> With this patch applied:
>   [fmt:UYVY8_2X8/1024x768@1/30 field:none colorspace:srgb xfer:srgb ...]
>
> Fixes: 6949d864776e ("media: ov5640: do not change mode if format or frame
> interval is unchanged")
> Signed-off-by: Jacopo Mondi 

For patch 1 of 2 only,

Tested-by: Adam Ford #imx6d with CSI2
interface on 4.19.6 and 4.20-RC5

It would be great if this could be applied to 4.19+

adam
> ---
>  drivers/media/i2c/ov5640.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 03a031a42b3e..c659efe918a4 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2160,6 +2160,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> const struct ov5640_mode_info *new_mode;
> struct v4l2_mbus_framefmt *mbus_fmt = >format;
> +   struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> if (format->pad != 0)
> @@ -2177,22 +2178,20 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> if (ret)
> goto out;
>
> -   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -   struct v4l2_mbus_framefmt *fmt =
> -   v4l2_subdev_get_try_format(sd, cfg, 0);
> +   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> +   fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +   else
> +   fmt = >fmt;
>
> -   *fmt = *mbus_fmt;
> -   goto out;
> -   }
> +   *fmt = *mbus_fmt;
>
> if (new_mode != sensor->current_mode) {
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> }
> -   if (mbus_fmt->code != sensor->fmt.code) {
> -   sensor->fmt = *mbus_fmt;
> +   if (mbus_fmt->code != sensor->fmt.code)
> sensor->pending_fmt_change = true;
> -   }
> +
>  out:
> mutex_unlock(>lock);
> return ret;
> --
> 2.7.4
>


Re: [PATCH 2/2] media: ov5640: make MIPI clock depend on mode

2018-12-03 Thread Adam Ford
On Thu, Nov 29, 2018 at 8:49 AM Jacopo Mondi  wrote:
>
> The MIPI clock generation tree uses the MIPI_DIV divider to generate both the
> MIPI SCALER CLOCK and the PIXEL_CLOCK. It seems the MIPI BIT CLK frequency is
> generated by either the MIPI SCALER or the PIXEL CLOCK, depending if the
> currently applied image mode uses the scaler or not.
>
> Make the MIPI_DIV selection value depend on the subsampling mode used by the
> currently applied image mode.
>
> Tested with:
> 172x144 320x240, 640x480, 1024x756, 1024x768, 1280x720, 1920x1080 in YUYV mode
> at 10, 15, 20, and 30 FPS with MIPI CSI-2 2 lanes mode.
>
> Signed-off-by: Jacopo Mondi 

I am not able to apply this patch to 4.19.6, 4.19-RC5, or Linux-media master [1]

Is there a specific branch/repo somewhere I can pull?  I was able to
apply patch 1/2 just fine, but 2/2 wouldn't apply

[1] - git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git

adam
> ---
> Maxime,
>this patch is not just a cosmetic updates to the 'set_mipi_pclk()'
> comment block as I anticipated, but it actually fix the framerate handling
> compared for CSI-2, which in you v5 results halved for most modes. I have
> not included any "Fixes:" tag, as I hope this patch will get in with your v5.
>
> That's my bad, as in the patches I sent to be applied on top of your v4 I
> forgot to add a change, and the requested SCLK rate was always half of what
> it was actually required.
>
> Also, I have left out from this patches most of Sam's improvements (better 
> SCLK
> selection policies, and programming of register OV5640_REG_PCLK_PERIOD as for
> from testing they're not required, and I don't want to pile more patches than
> necessary for the next merge window, not to slow down the clock tree rework
> inclusion. We can get back to it after it got merged.
>
> This are the test result obtained by capturing 100 frames with yavta and
> inspecting the reported fps results.
>
> Results are pretty neat, 2592x1944 mode apart, which runs a bit slow.
>
> capturing 176x144 @ 10FPS
> Captured 100 frames in 10.019398 seconds (9.980640 fps, 505898.657683 B/s).
> capturing 176x144 @ 15FPS
> Captured 100 frames in 6.681860 seconds (14.965892 fps, 758591.132803 B/s).
> capturing 176x144 @ 20FPS
> Captured 100 frames in 5.056204 seconds (19.777683 fps, 1002491.196755 B/s).
> capturing 176x144 @ 30FPS
> Captured 100 frames in 3.357204 seconds (29.786686 fps, 1509827.521040 B/s).
>
> capturing 320x240 @ 10FPS
> Captured 100 frames in 10.015432 seconds (9.984591 fps, 1533633.245951 B/s).
> capturing 320x240 @ 15FPS
> Captured 100 frames in 6.684035 seconds (14.961021 fps, 2298012.872049 B/s).
> capturing 320x240 @ 20FPS
> Captured 100 frames in 5.019164 seconds (19.923635 fps, 3060270.391218 B/s).
> capturing 320x240 @ 30FPS
> Captured 100 frames in 3.352991 seconds (29.824115 fps, 4580984.103432 B/s).
>
> capturing 640x480 @ 10FPS
> Captured 100 frames in 9.990389 seconds (10.009620 fps, 6149910.678538 B/s).
> capturing 640x480 @ 15FPS
> Captured 100 frames in 6.856242 seconds (14.585249 fps, 8961176.838123 B/s).
> capturing 640x480 @ 20FPS
> Captured 100 frames in 5.030264 seconds (19.879670 fps, 12214069.053476 B/s).
> capturing 640x480 @ 30FPS
> Captured 100 frames in 3.364612 seconds (29.721103 fps, 18260645.750580 B/s).
>
> capturing 720x480 @ 10FPS
> Captured 100 frames in 10.022488 seconds (9.977562 fps, 6896491.169279 B/s).
> capturing 720x480 @ 15FPS
> Captured 100 frames in 6.891968 seconds (14.509643 fps, 10029065.232208 B/s).
> capturing 720x480 @ 20FPS
> Captured 100 frames in 5.234133 seconds (19.105360 fps, 13205624.616211 B/s).
> capturing 720x480 @ 30FPS
> Captured 100 frames in 3.602298 seconds (27.760055 fps, 19187750.044688 B/s).
>
> capturing 720x576 @ 10FPS
> Captured 100 frames in 10.197244 seconds (9.806571 fps, 8133961.937805 B/s).
> capturing 720x576 @ 15FPS
> Captured 100 frames in 6.925244 seconds (14.439924 fps, 11977050.339261 B/s).
> capturing 720x576 @ 20FPS
> Captured 100 frames in 4.68 seconds (20.000127 fps, 16588905.060854 B/s).
> capturing 720x576 @ 30FPS
> Captured 100 frames in 3.487568 seconds (28.673276 fps, 23782762.085212 B/s).
>
> capturing 1024x768 @ 10FPS
> Captured 100 frames in 10.107128 seconds (9.894007 fps, 15561928.174298 B/s).
> capturing 1024x768 @ 15FPS
> Captured 100 frames in 6.810568 seconds (14.683062 fps, 23094459.169337 B/s).
> capturing 1024x768 @ 20FPS
> Captured 100 frames in 5.012039 seconds (19.951960 fps, 31381719.096759 B/s).
> capturing 1024x768 @ 30FPS
> Captured 100 frames in 3.346338 seconds (29.883407 fps, 47002534.905114 B/s).
>
> capturing 1280x720 @ 10FPS
> Captured 100 frames in 9.957613 seconds (10.042567 fps, 18510459.665283 B/s).
> capturing 1280x720 @ 15FPS
> Captured 100 frames in 6.597751 seconds (15.156679 fps, 27936790.986673 B/s).
> capturing 1280x720 @ 20FPS
> Captured 100 frames in 4.946115 seconds (20.217888 fps, 37265611.495083 B/s).
> capturing 1280x720 @ 30FPS
> Captured 100 frames in 3.301329 

Re: [PATCH 0/2] media: ov5640: MIPI fixes on top of Maxime's v5

2018-11-29 Thread Adam Ford
On Thu, Nov 29, 2018 at 8:48 AM Jacopo Mondi  wrote:
>
> Hello ov5640-ers,
>these two patches should be applied on top of Maxime's clock tree rework v5
> and 'fix' MIPI CSI-2 clock tree configuration.
>
> The first patch is a fix that appeard in various forms on the list several
> times: if the image sizes gets updated but not the image format, the size
> update gets ignored. I had to fix this to run my FPS tests, and thus I'm
> sending the two together. I wish in future a re-work of the image format
> handling part, but for now, let's just fix it for v4.21.
>
> The second patch slightly reworks the MIPI clock tree configuration, based on
> inputs from Sam. The currently submitted v5 in which Maxime squashed my 
> previous
> changes is 'broken'. That's my bad, as explained in the patch change log.
>
> Test results are attacched to patch [2/2].
> Changelog for [2/2] is included in the patch itself.
>
> I wish these patches to go in with Maxime awesome clock tree re-work, pending
> his ack.
>
> Also, I have tested with an i.MX6Q board, with a CSI-2 2 data lanes setup. 
> There
> are still a few things not clear to me in the MIPI clock tree, and I welcome
> more testing, preferibly with 1-lanes setups.
>
> Also, I had to re-apply Maxime's series and latest ov5640 patches on v4.19,
> as my test board is sort of broken with v4.20-rcX (it shouldn't make any
> difference in regard to this series, but I'm pointing it out anyhow).

Greg KH is pulling in some of the first round of fixes to the 4.19.y
branch.  If they're working, we may want to consider having Greg Apply
these there as well.

>
> Recently Adam has been testing quite some ov5640 patches, if you fill like
> testing these as well on your setup (which I understand is a MIPI CSI-2 one)
> please report the results. Same for all other interested ones :)

You are correct, I have an i.MX6 with CSI2 interface.  I'll test
against 4.20-RCx and my 4.19 version with some of the newer patches
pulled down.  I will try to get to these before Monday.

adam

>
> Thanks
>   j
>
> Jacopo Mondi (2):
>   media: i2c: ov5640: Fix set format regression
>   media: ov5640: make MIPI clock depend on mode
>
>  drivers/media/i2c/ov5640.c | 110 
> ++---
>  1 file changed, 54 insertions(+), 56 deletions(-)
>
> --
> 2.7.4
>


Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-19 Thread Adam Ford
On Wed, Nov 14, 2018 at 7:20 AM Adam Ford  wrote:
>
> On Tue, Nov 13, 2018 at 7:04 AM Maxime Ripard  
> wrote:
> >
> > The clock structure for the PCLK is quite obscure in the documentation, and
> > was hardcoded through the bytes array of each and every mode.
> >
> > This is troublesome, since we cannot adjust it at runtime based on other
> > parameters (such as the number of bytes per pixel), and we can't support
> > either framerates that have not been used by the various vendors, since we
> > don't have the needed initialization sequence.
> >
> > We can however understand how the clock tree works, and then implement some
> > functions to derive the various parameters from a given rate. And now that
> > those parameters are calculated at runtime, we can remove them from the
> > initialization sequence.
> >
> > The modes also gained a new parameter which is the clock that they are
> > running at, from the register writes they were doing, so for now the switch
> > to the new algorithm should be transparent.
> >
> > Signed-off-by: Maxime Ripard 
>
> Thanks for the patches! I tested the whole series.  I am stil learning
> the v4l2 stuff, but I'm trying to test what and where I can.
> media-ctl shows the camera is talking at 60fps, but my imx6 is only
> capturing at 30, but I don't think it's the fault of the ov5640
> driver.
>
> Tested-by: Adam Ford  #imx6dq
>

For what it's worth, I would I like to request that this series be
applied to 4.20 fixes (possibly 4.19 if appropriate) as it fixes some
issues where I am trying to capture JPEG images that previously came
up garbled, and now they are clear.  The only change to my kernel is
this series.

adam

> adam
> > ---
> >  drivers/media/i2c/ov5640.c | 366 -
> >  1 file changed, 365 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index eaefdb58653b..8476f85bb8e7 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> > u32 htot;
> > u32 vact;
> > u32 vtot;
> > +   u32 pixel_clock;
> > const struct reg_value *reg_data;
> > u32 reg_data_size;
> >  };
> > @@ -700,6 +701,7 @@ static const struct reg_value 
> > ov5640_setting_15fps_QSXGA_2592_1944[] = {
> >  /* power-on sensor init reg table */
> >  static const struct ov5640_mode_info ov5640_mode_init_data = {
> > 0, SUBSAMPLING, 640, 1896, 480, 984,
> > +   5600,
> > ov5640_init_setting_30fps_VGA,
> > ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> >  };
> > @@ -709,74 +711,91 @@ 
> > ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> > {
> > {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
> >  176, 1896, 144, 984,
> > +2800,
> >  ov5640_setting_15fps_QCIF_176_144,
> >  ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> > {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> >  320, 1896, 240, 984,
> > +2800,
> >  ov5640_setting_15fps_QVGA_320_240,
> >  ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> > {OV5640_MODE_VGA_640_480, SUBSAMPLING,
> >  640, 1896, 480, 1080,
> > +2800,
> >  ov5640_setting_15fps_VGA_640_480,
> >  ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> > {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> >  720, 1896, 480, 984,
> > +2800,
> >  ov5640_setting_15fps_NTSC_720_480,
> >  ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> > {OV5640_MODE_PAL_720_576, SUBSAMPLING,
> >  720, 1896, 576, 984,
> > +2800,
> >  ov5640_setting_15fps_PAL_720_576,
> >  ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> > {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> >  1024, 1896, 768, 1080,
> > +2800,
> >  ov5640_setting_15fps_XGA_1024_768,
> >  ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> > {OV5640_MODE_720P_1280_720, SUBSAMPLING,
> >  1280, 1892, 720, 740,
> > +2100,
> &g

Re: [PATCH v4 00/22] i.MX media mem2mem scaler

2018-11-14 Thread Adam Ford
On Fri, Oct 19, 2018 at 7:16 AM Philipp Zabel  wrote:
>
> Hi,
>
> this is the fourth version of the i.MX mem2mem scaler series.
>
> An alignment issue with 24-bit RGB formats has been corrected in the
> seam position selection patch and a few new fixes by Steve have been
> added. If there are no more issues, I'll pick up the ipu-v3 patches
> via imx-drm/next. The first patch could be merged via the media tree
> independently.

I'd like to test this, but I am not sure how.  Do you have
instructions?  (ideally using gstreamer?)

adam

>
> Changes since v3:
>  - Fix tile_left_align for 24-bit RGB formats and reduce alignment
>restrictions for U/V packed planar YUV formats
>  - Catch unaligned tile offsets in image-convert
>  - Add chroma plane offset overrides to ipu_cpmem_set_image() to
>prevent a false positive warning in some cases
>  - Fix a race between run and unprepare and make abort reentrant.
>
>
> Changes since v2:
>  - Rely on ipu_image_convert_adjust() in mem2mem_try_fmt() for format
>adjustments. This makes the mem2mem driver mostly a V4L2 mem2mem API
>wrapper around the IPU image converter, and independent of the
>internal image converter implementation.
>  - Remove the source and destination buffers on error in device_run().
>Otherwise the conversion is re-attempted apparently over and over
>again (with WARN() backtraces).
>  - Allow subscribing to control changes.
>  - Fix seam position selection for more corner cases:
> - Switch width/height properly and align tile top left positions to 8x8
>   IRT block size when rotating.
> - Align input width to input burst length in case the scaling step
>   flips horizontally.
> - Fix bottom edge calculation.
>
> Changes since v1:
>  - Fix inverted allow_overshoot logic
>  - Correctly switch horizontal / vertical tile alignment when
>determining seam positions with the 90° rotator active.
>  - Fix SPDX-License-Identifier and remove superfluous license
>text.
>  - Fix uninitialized walign in try_fmt
>
> Previous cover letter:
>
> we have image conversion code for scaling and colorspace conversion in
> the IPUv3 base driver for a while. Since the IC hardware can only write
> up to 1024x1024 pixel buffers, it scales to larger output buffers by
> splitting the input and output frame into similarly sized tiles.
>
> This causes the issue that the bilinear interpolation resets at the tile
> boundary: instead of smoothly interpolating across the seam, there is a
> jump in the input sample position that is very apparent for high
> upscaling factors. This can be avoided by slightly changing the scaling
> coefficients to let the left/top tiles overshoot their input sampling
> into the first pixel / line of their right / bottom neighbors. The error
> can be further reduced by letting tiles be differently sized and by
> selecting seam positions that minimize the input sampling position error
> at tile boundaries.
> This is complicated by different DMA start address, burst size, and
> rotator block size alignment requirements, depending on the input and
> output pixel formats, and the fact that flipping happens in different
> places depending on the rotation.
>
> This series implements optimal seam position selection and seam hiding
> with per-tile resizing coefficients and adds a scaling mem2mem device
> to the imx-media driver.
>
> regards
> Philipp
>
> Philipp Zabel (15):
>   media: imx: add mem2mem device
>   gpu: ipu-v3: ipu-ic: allow to manually set resize coefficients
>   gpu: ipu-v3: image-convert: prepare for per-tile configuration
>   gpu: ipu-v3: image-convert: calculate per-tile resize coefficients
>   gpu: ipu-v3: image-convert: reconfigure IC per tile
>   gpu: ipu-v3: image-convert: store tile top/left position
>   gpu: ipu-v3: image-convert: calculate tile dimensions and offsets
> outside fill_image
>   gpu: ipu-v3: image-convert: move tile alignment helpers
>   gpu: ipu-v3: image-convert: select optimal seam positions
>   gpu: ipu-v3: image-convert: fix debug output for varying tile sizes
>   gpu: ipu-v3: image-convert: relax alignment restrictions
>   gpu: ipu-v3: image-convert: fix bytesperline adjustment
>   gpu: ipu-v3: image-convert: add some ASCII art to the exposition
>   gpu: ipu-v3: image-convert: disable double buffering if necessary
>   gpu: ipu-v3: image-convert: allow three rows or columns
>
> Steve Longerbeam (7):
>   gpu: ipu-cpmem: add WARN_ON_ONCE() for unaligned dma buffers
>   gpu: ipu-v3: Add chroma plane offset overrides to
> ipu_cpmem_set_image()
>   gpu: ipu-v3: image-convert: Prevent race between run and unprepare
>   gpu: ipu-v3: image-convert: Only wait for abort completion if active
> run
>   gpu: ipu-v3: image-convert: Allow reentrancy into abort
>   gpu: ipu-v3: image-convert: Remove need_abort flag
>   gpu: ipu-v3: image-convert: Catch unaligned tile offsets
>
>  drivers/gpu/ipu-v3/ipu-cpmem.c|   52 +-
>  drivers/gpu/ipu-v3/ipu-ic.c

Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-14 Thread Adam Ford
On Tue, Nov 13, 2018 at 7:04 AM Maxime Ripard  wrote:
>
> The clock structure for the PCLK is quite obscure in the documentation, and
> was hardcoded through the bytes array of each and every mode.
>
> This is troublesome, since we cannot adjust it at runtime based on other
> parameters (such as the number of bytes per pixel), and we can't support
> either framerates that have not been used by the various vendors, since we
> don't have the needed initialization sequence.
>
> We can however understand how the clock tree works, and then implement some
> functions to derive the various parameters from a given rate. And now that
> those parameters are calculated at runtime, we can remove them from the
> initialization sequence.
>
> The modes also gained a new parameter which is the clock that they are
> running at, from the register writes they were doing, so for now the switch
> to the new algorithm should be transparent.
>
> Signed-off-by: Maxime Ripard 

Thanks for the patches! I tested the whole series.  I am stil learning
the v4l2 stuff, but I'm trying to test what and where I can.
media-ctl shows the camera is talking at 60fps, but my imx6 is only
capturing at 30, but I don't think it's the fault of the ov5640
driver.

Tested-by: Adam Ford  #imx6dq

adam
> ---
>  drivers/media/i2c/ov5640.c | 366 -
>  1 file changed, 365 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb58653b..8476f85bb8e7 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> u32 htot;
> u32 vact;
> u32 vtot;
> +   u32 pixel_clock;
> const struct reg_value *reg_data;
> u32 reg_data_size;
>  };
> @@ -700,6 +701,7 @@ static const struct reg_value 
> ov5640_setting_15fps_QSXGA_2592_1944[] = {
>  /* power-on sensor init reg table */
>  static const struct ov5640_mode_info ov5640_mode_init_data = {
> 0, SUBSAMPLING, 640, 1896, 480, 984,
> +   5600,
> ov5640_init_setting_30fps_VGA,
> ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
>  };
> @@ -709,74 +711,91 @@ 
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>  176, 1896, 144, 984,
> +2800,
>  ov5640_setting_15fps_QCIF_176_144,
>  ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>  320, 1896, 240, 984,
> +2800,
>  ov5640_setting_15fps_QVGA_320_240,
>  ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>  640, 1896, 480, 1080,
> +2800,
>  ov5640_setting_15fps_VGA_640_480,
>  ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>  720, 1896, 480, 984,
> +2800,
>  ov5640_setting_15fps_NTSC_720_480,
>  ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> {OV5640_MODE_PAL_720_576, SUBSAMPLING,
>  720, 1896, 576, 984,
> +2800,
>  ov5640_setting_15fps_PAL_720_576,
>  ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>  1024, 1896, 768, 1080,
> +2800,
>  ov5640_setting_15fps_XGA_1024_768,
>  ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>  1280, 1892, 720, 740,
> +2100,
>  ov5640_setting_15fps_720P_1280_720,
>  ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> {OV5640_MODE_1080P_1920_1080, SCALING,
>  1920, 2500, 1080, 1120,
> +4200,
>  ov5640_setting_15fps_1080P_1920_1080,
>  ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> {OV5640_MODE_QSXGA_2592_1944, SCALING,
>  2592, 2844, 1944, 1968,
> +8400,
>  ov5640_setting_15fps_QSXGA_2592_1944,
>  ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
> }, {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>  176, 1896, 144, 984,
> +5600,
>

[RFC PATCH] media: i2c: mt9p031: Add 8-bit support

2017-07-18 Thread Adam Ford
From: Adam Ford <adam.f...@logicpd.com>

By default the camera driver only supports monochrome or color at 12-bit.
This patch will allow the camera to choose between 12-bit or 8-bit resolution.
Tested on Logic PD DM3730 Torpedo Development Kit.

Signed-off-by: Adam Ford <adam.f...@logicpd.com>

diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt 
b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
index cb60443..77b0dc1 100644
--- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
+++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
@@ -15,6 +15,7 @@ Required Properties:
 
 Optional Properties:
 - reset-gpios: Chip reset GPIO
+- resolution: Empty (12-bit) or 8 bit resolution 
 
 For further reading on port node refer to
 Documentation/devicetree/bindings/media/video-interfaces.txt.
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 91d822f..355791d 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -1024,6 +1024,7 @@ mt9p031_get_pdata(struct i2c_client *client)
 
of_property_read_u32(np, "input-clock-frequency", >ext_freq);
of_property_read_u32(np, "pixel-clock-frequency", >target_freq);
+   of_property_read_u32(np, "resolution", >resolution);
 
 done:
of_node_put(np);
@@ -1058,6 +1059,7 @@ static int mt9p031_probe(struct i2c_client *client,
mt9p031->output_control = MT9P031_OUTPUT_CONTROL_DEF;
mt9p031->mode2 = MT9P031_READ_MODE_2_ROW_BLC;
mt9p031->model = did->driver_data;
+   mt9p031->resolution = pdata->resolution;
 
mt9p031->regulators[0].supply = "vdd";
mt9p031->regulators[1].supply = "vdd_io";
@@ -1123,11 +1125,18 @@ static int mt9p031_probe(struct i2c_client *client,
mt9p031->crop.left = MT9P031_COLUMN_START_DEF;
mt9p031->crop.top = MT9P031_ROW_START_DEF;
 
-   if (mt9p031->model == MT9P031_MODEL_MONOCHROME)
-   mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12;
-   else
-   mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
-
+   if (mt9p031->model == MT9P031_MODEL_MONOCHROME) {
+   if (mt9p031->resolution == 8)
+   mt9p031->format.code = MEDIA_BUS_FMT_Y8_1X8;
+   else
+   mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12;
+   }
+   else {
+   if (mt9p031->resolution == 8)
+   mt9p031->format.code = MEDIA_BUS_FMT_SGRBG8_1X8;
+   else
+   mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
+   }
mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF;
mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
mt9p031->format.field = V4L2_FIELD_NONE;
diff --git a/include/media/i2c/mt9p031.h b/include/media/i2c/mt9p031.h
index 1ba3612..84335cb 100644
--- a/include/media/i2c/mt9p031.h
+++ b/include/media/i2c/mt9p031.h
@@ -11,6 +11,7 @@ struct v4l2_subdev;
 struct mt9p031_platform_data {
int ext_freq;
int target_freq;
+   int resolution;
 };
 
 #endif
-- 
2.7.4



OMAP3630 ISP V4L2 Camera Not Streaming to LCD

2017-06-21 Thread Adam Ford
I have a Leopard Imaging LI-5M03 camera attached in 8-bit mode, and I
am trying to capture an image on  camera and stream it to the LCD
(/dev/fb0) without using the DSP or proprietary codecs.

I was hoping to do it with either gstreamer (preferrably 1.0) or ffpeg.

My board has mainline device tree (logicpd-torpedo-37xx-devkit).
( have played wtih some of the settings, but nothing seems to make any
difference)


Using 
(https://github.com/Alaganraj/omap3isp/blob/master/0001-ARM-omap3-beagle-Add-.dtsi-for-the-LI-5M03-camera-se.patch)
as an example.

Using Linux 4.11.y stable branch, I setup the camera as follows:

media-ctl -v -r -l '"mt9p031 1-0048":0->"OMAP3 ISP CCDC":0[1], "OMAP3
ISP CCDC":2->"OMAP3 ISP preview":0[1], "OMAP3 ISP preview":1->"OMAP3
ISP resizer":0[1], "OMAP3 ISP resizer":1->"OMAP3 ISP resizer
output":0[1]'

media-ctl -v -V '"mt9p031 1-0048":0 [SGRBG8 1298x970
(664,541)/1298x970], "OMAP3 ISP CCDC":2 [SGRBG10 1298x970], "OMAP3 ISP
preview":1 [UYVY 1298x970], "OMAP3 ISP resizer":1 [UYVY 320x240]'


The media controller shows that this part appears to work correctly.
# media-ctl -p
Media controller API version 0.1.0

Media device information

driver  omap3isp
model   TI OMAP3 ISP
serial
bus info
hw revision 0xf0
driver version  0.0.0

Device topology
- entity 1: OMAP3 ISP CCP2 (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[fmt:SGRBG10_1X10/4096x4096 field:none]
<- "OMAP3 ISP CCP2 input":0 []
pad1: Source
[fmt:SGRBG10_1X10/4096x4096 field:none]
-> "OMAP3 ISP CCDC":0 []

- entity 4: OMAP3 ISP CCP2 input (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Source
-> "OMAP3 ISP CCP2":0 []

- entity 8: OMAP3 ISP CSI2a (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev1
pad0: Sink
[fmt:SGRBG10_1X10/4096x4096 field:none]
pad1: Source
[fmt:SGRBG10_1X10/4096x4096 field:none]
-> "OMAP3 ISP CSI2a output":0 []
-> "OMAP3 ISP CCDC":0 []

- entity 11: OMAP3 ISP CSI2a output (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video1
pad0: Sink
<- "OMAP3 ISP CSI2a":1 []

- entity 15: OMAP3 ISP CCDC (3 pads, 9 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev2
pad0: Sink
[fmt:SGRBG12_1X12/1298x970 field:none]
<- "OMAP3 ISP CCP2":1 []
<- "OMAP3 ISP CSI2a":1 []
<- "mt9p031 1-0048":0 [ENABLED]
pad1: Source
[fmt:SGRBG12_1X12/1296x970 field:none
 crop.bounds:(0,0)/1312x970
 crop:(0,0)/1296x970]
-> "OMAP3 ISP CCDC output":0 []
-> "OMAP3 ISP resizer":0 []
pad2: Source
[fmt:SGRBG10_1X10/1298x969 field:none]
-> "OMAP3 ISP preview":0 [ENABLED]
-> "OMAP3 ISP AEWB":0 [ENABLED,IMMUTABLE]
-> "OMAP3 ISP AF":0 [ENABLED,IMMUTABLE]
-> "OMAP3 ISP histogram":0 [ENABLED,IMMUTABLE]

- entity 19: OMAP3 ISP CCDC output (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video2
pad0: Sink
<- "OMAP3 ISP CCDC":1 []

- entity 23: OMAP3 ISP preview (2 pads, 4 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev3
pad0: Sink
[fmt:SGRBG10_1X10/1298x969 field:none
 crop.bounds:(10,4)/1280x961
 crop:(10,4)/1280x961]
<- "OMAP3 ISP CCDC":2 [ENABLED]
<- "OMAP3 ISP preview input":0 []
pad1: Source
[fmt:UYVY8_1X16/1280x961 field:none]
-> "OMAP3 ISP preview output":0 []
-> "OMAP3 ISP resizer":0 [ENABLED]

- entity 26: OMAP3 ISP preview input (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video3
pad0: Source
-> "OMAP3 ISP preview":0 []

- entity 30: OMAP3 ISP preview output (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video4
pad0: Sink
<- "OMAP3 ISP preview":1 []

- entity 34: OMAP3 ISP resizer (2 pads, 4 links)
 type V4L2 subdev subtype Unknown flags 0
 device node name /dev/v4l-subdev4
pad0: Sink
[fmt:UYVY8_1X16/1280x961 field:none
 crop.bounds:(0,0)/1280x961
 crop:(0,0)/1280x961]
<- "OMAP3 ISP CCDC":1 []
<- "OMAP3 ISP preview":1 [ENABLED]