cron job: media_tree daily build: OK

2018-10-16 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Wed Oct 17 05:00:10 CEST 2018
media-tree git hash:490d84f6d73c12f4204241cff8651eed60aae914
media_build git hash:   9f419c414672676f63e85a61ea99df0ddcd6e9a7
v4l-utils git hash: e84c8bdb0af5b79efa26aabc7b17d1ddead56c70
edid-decode git hash:   5eeb151a748788666534d6ea3da07f90400d24c2
gcc version:i686-linux-gcc (GCC) 8.2.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.18.11-marune

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.57-i686: OK
linux-3.16.57-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.123-i686: OK
linux-3.18.123-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.52-i686: OK
linux-4.1.52-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.159-i686: OK
linux-4.4.159-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.131-i686: OK
linux-4.9.131-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.74-i686: OK
linux-4.14.74-x86_64: OK
linux-4.15.18-i686: OK
linux-4.15.18-x86_64: OK
linux-4.16.18-i686: OK
linux-4.16.18-x86_64: OK
linux-4.17.19-i686: OK
linux-4.17.19-x86_64: OK
linux-4.18.12-i686: OK
linux-4.18.12-x86_64: OK
linux-4.19-rc6-i686: OK
linux-4.19-rc6-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


[PATCH v5 00/12] imx-media: Fixes for interlaced capture

2018-10-16 Thread Steve Longerbeam
A set of patches that fixes some bugs with capturing from an
interlaced source, and incompatibilites between IDMAC interlace
interweaving and 4:2:0 data write reduction.

History:
v5:
- Added a regression fix to allow empty endpoints to CSI (fix for imx6q
  SabreAuto).
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
  by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
- Removed interweave_offset and replace with boolean interweave_swap,
  suggested by Philipp.
- Make clear that it is IDMAC channel that does pixel reordering and
  interweave, not the CSI, in the imx.rst doc, caught by Philipp.

v4:
- rebased to latest media-tree master branch.
- Make patch author and SoB email addresses the same.

v3:
- add support for/fix interweaved scan with YUV planar output.
- fix bug in 4:2:0 U/V offset macros.
- add patch that generalizes behavior of field swap in
  ipu_csi_init_interface().
- add support for interweaved scan with field order swap.
  Suggested by Philipp Zabel.
- in v2, inteweave scan was determined using field types of
  CSI (and PRPENCVF) at the sink and source pads. In v3, this
  has been moved one hop downstream: interweave is now determined
  using field type at source pad, and field type selected at
  capture interface. Suggested by Philipp.
- make sure to double CSI crop target height when input field
  type in alternate.
- more updates to media driver doc to reflect above.

v2:
- update media driver doc.
- enable idmac interweave only if input field is sequential/alternate,
  and output field is 'interlaced*'.
- move field try logic out of *try_fmt and into separate function.
- fix bug with resetting crop/compose rectangles.
- add a patch that fixes a field order bug in VDIC indirect mode.
- remove alternate field type from V4L2_FIELD_IS_SEQUENTIAL() macro
  Suggested-by: Nicolas Dufresne .
- add macro V4L2_FIELD_IS_INTERLACED().


Steve Longerbeam (12):
  media: videodev2.h: Add more field helper macros
  gpu: ipu-csi: Swap fields according to input/output field types
  gpu: ipu-v3: Add planar support to interlaced scan
  media: imx: Fix field negotiation
  media: imx-csi: Input connections to CSI should be optional
  media: imx-csi: Double crop height for alternate fields at sink
  media: imx: interweave and odd-chroma-row skip are incompatible
  media: imx-csi: Allow skipping odd chroma rows for YVU420
  media: imx: vdic: rely on VDIC for correct field order
  media: imx-csi: Move crop/compose reset after filling default mbus
fields
  media: imx: Allow interweave with top/bottom lines swapped
  media: imx.rst: Update doc to reflect fixes to interlaced capture

 Documentation/media/v4l-drivers/imx.rst   | 103 +++
 drivers/gpu/ipu-v3/ipu-cpmem.c|  26 ++-
 drivers/gpu/ipu-v3/ipu-csi.c  | 119 +
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  46 +++--
 drivers/staging/media/imx/imx-media-capture.c |  14 ++
 drivers/staging/media/imx/imx-media-csi.c | 168 +-
 drivers/staging/media/imx/imx-media-vdic.c|  12 +-
 include/uapi/linux/videodev2.h|   7 +
 include/video/imx-ipu-v3.h|   6 +-
 9 files changed, 354 insertions(+), 147 deletions(-)

-- 
2.17.1



[PATCH v5 01/12] media: videodev2.h: Add more field helper macros

2018-10-16 Thread Steve Longerbeam
Adds two helper macros:

V4L2_FIELD_IS_SEQUENTIAL: returns true if the given field type is
'sequential', that is a full frame is transmitted, or exists in
memory, as all top field lines followed by all bottom field lines,
or vice-versa.

V4L2_FIELD_IS_INTERLACED: returns true if the given field type is
'interlaced', that is a full frame is transmitted, or exists in
memory, as top field lines interlaced with bottom field lines.

Signed-off-by: Steve Longerbeam 
---
Changes since v3:
- none
Changes since v2:
- none
Changes since v1:
- add the complement macro V4L2_FIELD_IS_INTERLACED
- remove V4L2_FIELD_ALTERNATE from V4L2_FIELD_IS_SEQUENTIAL macro.
- moved new macros past end of existing V4L2_FIELD_HAS_* macros.
---
 include/uapi/linux/videodev2.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 29729d580452..f7f031736d91 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -130,6 +130,13 @@ enum v4l2_field {
((field) == V4L2_FIELD_BOTTOM ||\
 (field) == V4L2_FIELD_TOP ||\
 (field) == V4L2_FIELD_ALTERNATE)
+#define V4L2_FIELD_IS_INTERLACED(field) \
+   ((field) == V4L2_FIELD_INTERLACED ||\
+(field) == V4L2_FIELD_INTERLACED_TB ||\
+(field) == V4L2_FIELD_INTERLACED_BT)
+#define V4L2_FIELD_IS_SEQUENTIAL(field) \
+   ((field) == V4L2_FIELD_SEQ_TB ||\
+(field) == V4L2_FIELD_SEQ_BT)
 
 enum v4l2_buf_type {
V4L2_BUF_TYPE_VIDEO_CAPTURE= 1,
-- 
2.17.1



Re: [PATCH 1/4] media: ov5640: fix resolution update

2018-10-16 Thread Samuel Bobrowicz
Glad this is spurring a lot of conversation, and I’m happy to see this many 
contributors too. I think we have all solved many of these problems (and the 
many others) offline, and now it’s the hard part to try to glue them all 
together. I decided to jump back in the mix with these patches because they 
seemed that they would be the easiest to rebase and were obvious bug fixes. I 
was hoping we could get this one applied and have a better starting place to 
work from before we start what is a very necessary restructure of the mode 
setting, but if everybody is ready to go now, we can just do a larger overhaul 
and I can withdraw this patch from this series. I expect the larger overhaul to 
warrant its own patch series.

I’m not going to comment yet on some of the proposed changes below, because I 
think it is better if we just move the discussion to its own series.

Another note, I also agree that prior to changing the rest of mode setting, the 
next step is to introduce maximes critical pll setting patch. It is required 
for my platform to work at all (yes my platform has never worked with main 
line) because my CSI2 receiver needs the PCLK PERIOD register to be set 
properly. I actually have tweaked Maximes original patch so that it doesn’t 
introduce a regression for MIPI platforms, but it will need to be tested on DVP 
platforms. I’ll send it out RFC tonight, so hopefully that will get us closer 
to a single patch that doesn’t break things in either mode.

IMO I would like to see this patch broken out of Maximes larger series, because 
I think it is getting bogged down by the many other changes there.

With this many interested parties, someone would really be helping if they put 
out a organized to do list that we could schedule out :)

> On Oct 16, 2018, at 5:15 AM, Laurent Pinchart 
>  wrote:
> 
> Hi Hugues,
> 
>> On Monday, 15 October 2018 18:13:12 EEST Hugues FRUCHET wrote:
>> Hi Laurent, Jacopo, Sam,
>> 
>> I'm also OK to change to a simpler alternative;
>> - drop the "restore" step
>> - send the whole init register sequence + mode changes + format changes 
>> at streamon
>> 
>> is this what you have in mind Laurent ?
> 
> Yes, that's pretty much the idea. The init sequence could be sent when 
> powering the sensor on to save time at streamon. Everything else can be 
> programmed at streamon time to simplify the implementation.
> 
>>> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
 On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
 
 Hi Sam,
 
 thanks for the patch, I see the same issue you reported, but I
 think this patch can be improved.
 
 (expanding the Cc list to all people involved in recent ov5640
 developemts, not just for this patch, but for the whole series to look
 at. Copying names from another series cover letter, hope it is
 complete.)
 
> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> 
> set_fmt was not properly triggering a mode change when
> a new mode was set that happened to have the same format
> as the previous mode (for example, when only changing the
> frame dimensions). Fix this.
> 
> Signed-off-by: Sam Bobrowicz 
> ---
> 
>  drivers/media/i2c/ov5640.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..5031aab 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev
> *sd,
>  goto out;
>  }
> 
> -if (new_mode != sensor->current_mode) {
> +
> +if (new_mode != sensor->current_mode ||
> +mbus_fmt->code != sensor->fmt.code) {
> +sensor->fmt = *mbus_fmt;
>  sensor->current_mode = new_mode;
>  sensor->pending_mode_change = true;
> -}
> -if (mbus_fmt->code != sensor->fmt.code) {
> -sensor->fmt = *mbus_fmt;
>  sensor->pending_fmt_change = true;
>  }
 
 How I did reproduce the issue:
 
 # Set 1024x768 on ov5640 without changing the image format
 # (default image size at startup is 640x480)
 $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768
 field:none]"
   sensor->pending_mode_change = true; //verified this flag gets set
 
 # Start streaming, after having configured the whole pipeline to work
 # with 1024x768
 $  yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
Unable to start streaming: Broken pipe (32).
 
 # Inspect which part of pipeline validation went wrong
 # Turns out the sensor->fmt field is not updated, and when get_fmt()
 # is called, the old one is returned.
 $ media-ctl -e "ov5640 2-003c" -p
   ...
   [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
   ycbcr:601 

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

2018-10-16 Thread jacopo mondi
Hello Maxime,
   a few comments I have collected while testing the series.

Please see below.

On Thu, Oct 11, 2018 at 11:20:56AM +0200, 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 
> ---
>  drivers/media/i2c/ov5640.c | 289 -
>  1 file changed, 288 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 30b15e91d8be..88fb16341466 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,
>ov5640_setting_30fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
>   {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> +  5600,
>ov5640_setting_30fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
>   {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> +  5600,
>ov5640_setting_30fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
>   {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>

Re: [PATCH v4 00/12] media: ov5640: Misc cleanup and improvements

2018-10-16 Thread jacopo mondi
Hello Maxime,

On Thu, Oct 11, 2018 at 11:20:55AM +0200, Maxime Ripard wrote:
> Hi,
>
> Here is a "small" series that mostly cleans up the ov5640 driver code,
> slowly getting rid of the big data array for more understandable code
> (hopefully).
>
> The biggest addition would be the clock rate computation at runtime,
> instead of relying on those arrays to setup the clock tree
> properly. As a side effect, it fixes the framerate that was off by
> around 10% on the smaller resolutions, and we now support 60fps.
>
> This also introduces a bunch of new features.
>
> Let me know what you think,

I'm sorry to report this breaks CSI-2 capture on my i.MX6 testing
platform.

I have been testing the whole afternoon with different configurations,
but I have not been able yet to find out the root of the problem.

In the meantime, I have some comments on the patches, and I'll reply
to them singularly.

Thanks
   j

> Maxime
>
> Changes from v3:
>   - Rebased on current Sakari tree
>   - Fixed an error when changing only the framerate
>
> Changes from v2:
>   - Rebased on latest Sakari PR
>   - Fixed the issues reported by Hugues: improper FPS returned for
> formats, improper rounding of the FPS, some with his suggestions,
> some by simplifying the logic.
>   - Expanded the clock tree comments based on the feedback from Samuel
> Bobrowicz and Loic Poulain
>   - Merged some of the changes made by Samuel Bobrowicz to fix the
> MIPI rate computation, fix the call sites of the
> ov5640_set_timings function, the auto-exposure calculation call,
> etc.
>   - Split the patches into smaller ones in order to make it more
> readable (hopefully)
>
> Changes from v1:
>   - Integrated Hugues' suggestions to fix v4l2-compliance
>   - Fixed the bus width with JPEG
>   - Dropped the clock rate calculation loops for something simpler as
> suggested by Sakari
>   - Cache the exposure value instead of using the control value
>   - Rebased on top of 4.17
>
> Maxime Ripard (12):
>   media: ov5640: Adjust the clock based on the expected rate
>   media: ov5640: Remove the clocks registers initialization
>   media: ov5640: Remove redundant defines
>   media: ov5640: Remove redundant register setup
>   media: ov5640: Compute the clock rate at runtime
>   media: ov5640: Remove pixel clock rates
>   media: ov5640: Enhance FPS handling
>   media: ov5640: Make the return rate type more explicit
>   media: ov5640: Make the FPS clamping / rounding more extendable
>   media: ov5640: Add 60 fps support
>   media: ov5640: Remove duplicate auto-exposure setup
>   ov5640: Enforce a mode change when changing the framerate
>
>  drivers/media/i2c/ov5640.c | 679 -
>  1 file changed, 374 insertions(+), 305 deletions(-)
>
> --
> 2.19.1
>


signature.asc
Description: PGP signature


Re: [PATCH v3 6/6] media: video-i2c: support runtime PM

2018-10-16 Thread Akinobu Mita
2018年10月16日(火) 0:31 Sakari Ailus :
>
> Hi Mita-san,
>
> On Sun, Oct 14, 2018 at 03:02:39AM +0900, Akinobu Mita wrote:
> > AMG88xx has a register for setting operating mode.  This adds support
> > runtime PM by changing the operating mode.
> >
> > The instruction for changing sleep mode to normal mode is from the
> > reference specifications.
> >
> > https://docid81hrs3j1.cloudfront.net/medialibrary/2017/11/PANA-S-A0002141979-1.pdf
> >
> > Cc: Matt Ranostay 
> > Cc: Sakari Ailus 
> > Cc: Hans Verkuil 
> > Cc: Mauro Carvalho Chehab 
> > Reviewed-by: Matt Ranostay 
> > Acked-by: Sakari Ailus 
> > Signed-off-by: Akinobu Mita 
> > ---
> > * v3
> > - Move chip->set_power() call to the video device release() callback.
> > - Add Acked-by line
> >
> >  drivers/media/i2c/video-i2c.c | 141 
> > +-
> >  1 file changed, 139 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> > index 3334fc2..22fdc43 100644
> > --- a/drivers/media/i2c/video-i2c.c
> > +++ b/drivers/media/i2c/video-i2c.c
> > @@ -17,6 +17,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -94,10 +95,23 @@ struct video_i2c_chip {
> >   /* xfer function */
> >   int (*xfer)(struct video_i2c_data *data, char *buf);
> >
> > + /* power control function */
> > + int (*set_power)(struct video_i2c_data *data, bool on);
> > +
> >   /* hwmon init function */
> >   int (*hwmon_init)(struct video_i2c_data *data);
> >  };
> >
> > +/* Power control register */
> > +#define AMG88XX_REG_PCTL 0x00
> > +#define AMG88XX_PCTL_NORMAL  0x00
> > +#define AMG88XX_PCTL_SLEEP   0x10
> > +
> > +/* Reset register */
> > +#define AMG88XX_REG_RST  0x01
> > +#define AMG88XX_RST_FLAG 0x30
> > +#define AMG88XX_RST_INIT 0x3f
> > +
> >  /* Frame rate register */
> >  #define AMG88XX_REG_FPSC 0x02
> >  #define AMG88XX_FPSC_1FPSBIT(0)
> > @@ -127,6 +141,59 @@ static int amg88xx_setup(struct video_i2c_data *data)
> >   return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> >  }
> >
> > +static int amg88xx_set_power_on(struct video_i2c_data *data)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, 
> > AMG88XX_PCTL_NORMAL);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(50);
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_INIT);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(2);
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_FLAG);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > +  * Wait two frames before reading thermistor and temperature registers
> > +  */
> > + msleep(200);
> > +
> > + return 0;
> > +}
> > +
> > +static int amg88xx_set_power_off(struct video_i2c_data *data)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL, 
> > AMG88XX_PCTL_SLEEP);
> > + if (ret)
> > + return ret;
> > + /*
> > +  * Wait for a while to avoid resuming normal mode immediately after
> > +  * entering sleep mode, otherwise the device occasionally goes wrong
> > +  * (thermistor and temperature registers are not updated at all)
> > +  */
> > + msleep(100);
> > +
> > + return 0;
> > +}
> > +
> > +static int amg88xx_set_power(struct video_i2c_data *data, bool on)
> > +{
> > + if (on)
> > + return amg88xx_set_power_on(data);
> > +
> > + return amg88xx_set_power_off(data);
> > +}
> > +
> >  #if IS_ENABLED(CONFIG_HWMON)
> >
> >  static const u32 amg88xx_temp_config[] = {
> > @@ -158,7 +225,15 @@ static int amg88xx_read(struct device *dev, enum 
> > hwmon_sensor_types type,
> >   __le16 buf;
> >   int tmp;
> >
> > + tmp = pm_runtime_get_sync(regmap_get_device(data->regmap));
> > + if (tmp < 0) {
> > + pm_runtime_put_noidle(regmap_get_device(data->regmap));
> > + return tmp;
> > + }
> > +
> >   tmp = regmap_bulk_read(data->regmap, AMG88XX_REG_TTHL, , 2);
> > + pm_runtime_mark_last_busy(regmap_get_device(data->regmap));
> > + pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
> >   if (tmp)
> >   return tmp;
> >
> > @@ -217,6 +292,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> >   .regmap_config  = _regmap_config,
> >   .setup  = _setup,
> >   .xfer   = _xfer,
> > + .set_power  = amg88xx_set_power,
> >   .hwmon_init = amg88xx_hwmon_init,
> >   },
> >  };
> > @@ -343,14 +419,21 @@ static void video_i2c_del_list(struct vb2_queue *vq, 
> > enum vb2_buffer_state state
> >  static int start_streaming(struct vb2_queue *vq, unsigned int count)
> >  {
> >   struct 

[PATCH] media: rc: self test for IR encoders and decoders

2018-10-16 Thread Sean Young
ir-loopback can transmit IR on one rc device and check the correct
scancode and protocol is decoded on a different rc device. This can be
used to check IR transmission between two rc devices. Using rc-loopback,
we use it to check the IR encoders and decoders themselves.

No hardware is required for this test.

Signed-off-by: Sean Young 
Cc: Shuah Khan 
---
 tools/testing/selftests/Makefile  |   1 +
 tools/testing/selftests/ir/.gitignore |   1 +
 tools/testing/selftests/ir/Makefile   |   5 +
 tools/testing/selftests/ir/ir_loopback.c  | 199 ++
 tools/testing/selftests/ir/ir_loopback.sh |  20 +++
 5 files changed, 226 insertions(+)
 create mode 100644 tools/testing/selftests/ir/.gitignore
 create mode 100644 tools/testing/selftests/ir/Makefile
 create mode 100644 tools/testing/selftests/ir/ir_loopback.c
 create mode 100755 tools/testing/selftests/ir/ir_loopback.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f1fe492c8e17..995034ea5546 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -15,6 +15,7 @@ TARGETS += futex
 TARGETS += gpio
 TARGETS += intel_pstate
 TARGETS += ipc
+TARGETS += ir
 TARGETS += kcmp
 TARGETS += kvm
 TARGETS += lib
diff --git a/tools/testing/selftests/ir/.gitignore 
b/tools/testing/selftests/ir/.gitignore
new file mode 100644
index ..070ea0c75fb8
--- /dev/null
+++ b/tools/testing/selftests/ir/.gitignore
@@ -0,0 +1 @@
+ir_loopback
diff --git a/tools/testing/selftests/ir/Makefile 
b/tools/testing/selftests/ir/Makefile
new file mode 100644
index ..f4ba8eb84b95
--- /dev/null
+++ b/tools/testing/selftests/ir/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_PROGS := ir_loopback.sh
+TEST_GEN_PROGS_EXTENDED := ir_loopback
+
+include ../lib.mk
diff --git a/tools/testing/selftests/ir/ir_loopback.c 
b/tools/testing/selftests/ir/ir_loopback.c
new file mode 100644
index ..858c19caf224
--- /dev/null
+++ b/tools/testing/selftests/ir/ir_loopback.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+// test ir decoder
+//
+// Copyright (C) 2018 Sean Young 
+
+// When sending LIRC_MODE_SCANCODE, the IR will be encoded. rc-loopback
+// will send this IR to the receiver side, where we try to read the decoded
+// IR. Decoding happens in a separate kernel thread, so we will need to
+// wait until that is scheduled, hence we use poll to check for read
+// readiness.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../kselftest.h"
+
+#define TEST_SCANCODES 10
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+static const struct {
+   enum rc_proto proto;
+   const char *name;
+   unsigned int mask;
+   const char *decoder;
+} protocols[] = {
+   { RC_PROTO_RC5, "rc-5", 0x1f7f, "rc-5" },
+   { RC_PROTO_RC5X_20, "rc-5x-20", 0x1f7f3f, "rc-5" },
+   { RC_PROTO_RC5_SZ, "rc-5-sz", 0x2fff, "rc-5-sz" },
+   { RC_PROTO_JVC, "jvc", 0x, "jvc" },
+   { RC_PROTO_SONY12, "sony-12", 0x1f007f, "sony" },
+   { RC_PROTO_SONY15, "sony-15", 0xff007f, "sony" },
+   { RC_PROTO_SONY20, "sony-20", 0x1fff7f, "sony" },
+   { RC_PROTO_NEC, "nec", 0x, "nec" },
+   { RC_PROTO_NECX, "nec-x", 0xff, "nec" },
+   { RC_PROTO_NEC32, "nec-32", 0x, "nec" },
+   { RC_PROTO_SANYO, "sanyo", 0x1f, "sanyo" },
+   { RC_PROTO_RC6_0, "rc-6-0", 0x, "rc-6" },
+   { RC_PROTO_RC6_6A_20, "rc-6-6a-20", 0xf, "rc-6" },
+   { RC_PROTO_RC6_6A_24, "rc-6-6a-24", 0xff, "rc-6" },
+   { RC_PROTO_RC6_6A_32, "rc-6-6a-32", 0x, "rc-6" },
+   { RC_PROTO_RC6_MCE, "rc-6-mce", 0x7fff, "rc-6" },
+   { RC_PROTO_SHARP, "sharp", 0x1fff, "sharp" },
+};
+
+int lirc_open(const char *rc)
+{
+   struct dirent *dent;
+   char buf[100];
+   DIR *d;
+   int fd;
+
+   snprintf(buf, sizeof(buf), "/sys/class/rc/%s", rc);
+
+   d = opendir(buf);
+   if (!d)
+   ksft_exit_fail_msg("cannot open %s: %m\n", buf);
+
+   while ((dent = readdir(d)) != NULL) {
+   if (!strncmp(dent->d_name, "lirc", 4)) {
+   snprintf(buf, sizeof(buf), "/dev/%s", dent->d_name);
+   break;
+   }
+   }
+
+   if (!dent)
+   ksft_exit_fail_msg("cannot find lirc device for %s\n", rc);
+
+   closedir(d);
+
+   fd = open(buf, O_RDWR | O_NONBLOCK);
+   if (fd == -1)
+   ksft_exit_fail_msg("cannot open: %s: %m\n", buf);
+
+   return fd;
+}
+
+int main(int argc, char **argv)
+{
+   unsigned int mode;
+   char buf[100];
+   int rlircfd, wlircfd, protocolfd, i, n;
+
+   srand(time(NULL));
+
+   if (argc != 3)
+   ksft_exit_fail_msg("Usage: %s  \n",
+  argv[0]);
+
+   rlircfd = 

Re: [RFP] Which V4L2 ioctls could be replaced by better versions?

2018-10-16 Thread Hans Verkuil
On 10/03/18 10:24, Tomasz Figa wrote:
> On Fri, Sep 21, 2018 at 3:14 AM Nicolas Dufresne  wrote:
>>
>> Le jeudi 20 septembre 2018 à 16:42 +0200, Hans Verkuil a écrit :
>>> Some parts of the V4L2 API are awkward to use and I think it would be
>>> a good idea to look at possible candidates for that.
>>>
>>> Examples are the ioctls that use struct v4l2_buffer: the multiplanar 
>>> support is
>>> really horrible, and writing code to support both single and multiplanar is 
>>> hard.
>>> We are also running out of fields and the timeval isn't y2038 compliant.
>>>
>>> A proof-of-concept is here:
>>>
>>> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=v4l2-buffer=a95549df06d9900f3559afdbb9da06bd4b22d1f3
>>>
>>> It's a bit old, but it gives a good impression of what I have in mind.
>>>
>>> Another candidate is 
>>> VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL/VIDIOC_ENUM_FRAMEINTERVALS:
>>> expressing frame intervals as a fraction is really awkward and so is the 
>>> fact
>>> that the subdev and 'normal' ioctls are not the same.
>>>
>>> Would using nanoseconds or something along those lines for intervals be 
>>> better?
>>
>> This one is not a good idea, because you cannot represent well known
>> rates used a lot in the field. Like 6/1001 also known as 59.94 Hz.
>> You could endup with drift issues.
>>
>> For me, what is the most difficult with this API is the fact that it
>> uses frame internal (duration) instead of frame rate.
>>
>>>
>>> I have similar concerns with VIDIOC_SUBDEV_ENUM_FRAME_SIZE where there is no
>>> stepwise option, making it different from VIDIOC_ENUM_FRAMESIZES. But it 
>>> should
>>> be possible to extend VIDIOC_SUBDEV_ENUM_FRAME_SIZE with stepwise support, I
>>> think.
>>
>> One of the thing to fix, maybe it's doable now, is the differentiation
>> between allocation size and display size. Pretty much all video capture
>> code assumes this is display size and ignores the selection API. This
>> should be documented explicitly.
> 
> Technically, there is already a distinction between allocation and
> display size at format level - width and height could be interpreted
> as the rectangle containing the captured video, while bytesperline and
> sizeimage would match to the allocation size. The behavior between
> drivers seems to be extremely variable - some would just enforce
> bytesperline = bpp*width and sizeimage = bytesperline*height, while
> others would actually give control over them to the user space.
> 
> It's a bit more complicated with video decoders, because the stream,
> in addition to the 2 sizes mentioned before, is also characterized by
> coded size, which corresponds to the actual number of pixels encoded
> in the stream, even if not all contain pixels to be displayed. This is
> something that needs to be specified explicitly (and is, in my
> documentation patches) in the Codec Interface.
> 
>>
>> In fact, the display/allocation dimension isn't very nice, as both
>> information overlaps in structures. As an example, you call S_FMT with
>> a display size you want, and it will return you an allocation size
>> (which yes, can be smaller, because we always round to the middle).
>>
>>>
>>> Do we have more ioctls that could use a refresh? S/G/TRY_FMT perhaps, again 
>>> in
>>> order to improve single vs multiplanar handling.
>>
>> Yes, but that would fall in a complete redesign I guess. The buffer
>> allocation scheme is very inflexible. You can't have buffers of two
>> dimensions allocated at the same time for the same queue. Worst, you
>> cannot leave even 1 buffer as your scannout buffer while reallocating
>> new buffers, this is not permitted by the framework (in software). As a
>> side effect, there is no way to optimize the resolution changes, you
>> even have to copy your scannout buffer on the CPU, to free it in order
>> to proceed. Resolution changes are thus painfully slow, by design.
> 
> Hmm? There is VIDIOC_CREATEBUFS, which can allows you to allocate
> buffers for explicitly specified format, with other buffers already
> existing in the queue.

Of course, we are missing the VIDIOC_DELETEBUFS ioctl. Also, CREATEBUFS
is rather awful. Using v4l2_format in the struct was a major mistake.

> 
> Also, I fail to understand the scanout issue. If one exports a vb2
> buffer to a DMA-buf and import it to the scanout engine, it can keep
> scanning out from it as long as it want, because the DMA-buf will hold
> a reference on the buffer, even if it's removed from the vb2 queue.
> 
>>
>> You also cannot switch from internal buffers to importing buffers
>> easily (in some case you, like encoder, you cannot do that without
>> flushing the encoder state).
> 
> Hmm, technically VIDIOC_CREATEBUFS accepts the "memory" type value,
> but I'm not sure what happens if the queue already has buffers
> requested with different one.

It is not allowed to mix memory types, that will return -EINVAL.

I have to say that this is the first time I had this request.

It is probably doable, but the 

HI

2018-10-16 Thread Lisa



 I am sgt lisa. am sending this brief letter to solicit
your
partnership to transfer $3.5 M million US Dollars. I shall send you
more information and procedures when I receive positive response from
you.

sgt lisa. (sgtlis...@gmail.com)





Re: [PATCH 1/4] media: ov5640: fix resolution update

2018-10-16 Thread Laurent Pinchart
Hi Hugues,

On Monday, 15 October 2018 18:13:12 EEST Hugues FRUCHET wrote:
> Hi Laurent, Jacopo, Sam,
> 
> I'm also OK to change to a simpler alternative;
> - drop the "restore" step
> - send the whole init register sequence + mode changes + format changes 
> at streamon
> 
> is this what you have in mind Laurent ?

Yes, that's pretty much the idea. The init sequence could be sent when 
powering the sensor on to save time at streamon. Everything else can be 
programmed at streamon time to simplify the implementation.

> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
> > On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
> > 
> >> Hi Sam,
> >> 
> >> thanks for the patch, I see the same issue you reported, but I
> >> think this patch can be improved.
> >>
> >> (expanding the Cc list to all people involved in recent ov5640
> >> developemts, not just for this patch, but for the whole series to look
> >> at. Copying names from another series cover letter, hope it is
> >> complete.)
> >>
> >> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> >> 
> >>> set_fmt was not properly triggering a mode change when
> >>> a new mode was set that happened to have the same format
> >>> as the previous mode (for example, when only changing the
> >>> frame dimensions). Fix this.
> >>>
> >>> Signed-off-by: Sam Bobrowicz 
> >>> ---
> >>>
> >>>   drivers/media/i2c/ov5640.c | 8 
> >>>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >>> index eaefdb5..5031aab 100644
> >>> --- a/drivers/media/i2c/ov5640.c
> >>> +++ b/drivers/media/i2c/ov5640.c
> >>> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev
> >>> *sd,
> >>>   goto out;
> >>>   }
> >>>
> >>> - if (new_mode != sensor->current_mode) {
> >>> +
> >>> + if (new_mode != sensor->current_mode ||
> >>> + mbus_fmt->code != sensor->fmt.code) {
> >>> + sensor->fmt = *mbus_fmt;
> >>>   sensor->current_mode = new_mode;
> >>>   sensor->pending_mode_change = true;
> >>> - }
> >>> - if (mbus_fmt->code != sensor->fmt.code) {
> >>> - sensor->fmt = *mbus_fmt;
> >>>   sensor->pending_fmt_change = true;
> >>>   }
> >>
> >> How I did reproduce the issue:
> >>
> >> # Set 1024x768 on ov5640 without changing the image format
> >> # (default image size at startup is 640x480)
> >> $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768
> >> field:none]"
> >>sensor->pending_mode_change = true; //verified this flag gets set
> >>
> >> # Start streaming, after having configured the whole pipeline to work
> >> # with 1024x768
> >> $  yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
> >> Unable to start streaming: Broken pipe (32).
> >>
> >> # Inspect which part of pipeline validation went wrong
> >> # Turns out the sensor->fmt field is not updated, and when get_fmt()
> >> # is called, the old one is returned.
> >> $ media-ctl -e "ov5640 2-003c" -p
> >>...
> >>[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
> >>ycbcr:601 quantization:full-range] ^^^ ^^^
> >>
> >> So yes, sensor->fmt is not udapted as it should be when only image
> >> resolution is changed.
> >>
> >> Although I still see value in having two separate flags for the
> >> 'mode_change' (which in ov5640 lingo is resolution) and 'fmt_change'
> >> (which in ov5640 lingo is the image format), and write their
> >> configuration to registers only when they get actually changed.
> >>
> >> For this reasons I would like to propse the following patch which I
> >> have tested by:
> >> 1) changing resolution only
> >> 2) changing format only
> >> 3) change both
> >>
> >> What do you and others think?
> > 
> > I think that the format setting code should be completely rewritten, it's
> > pretty much unmaintainable as-is.
> > 
> >> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >> index eaefdb5..e392b9d 100644
> >> --- a/drivers/media/i2c/ov5640.c
> >> +++ b/drivers/media/i2c/ov5640.c
> >> @@ -2020,6 +2020,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)
> >> @@ -2037,22 +2038,19 @@ 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 

Re: [PATCH v7] media: add imx319 camera sensor driver

2018-10-16 Thread Sakari Ailus
Hi Tomasz,

On Fri, Oct 12, 2018 at 05:06:56PM +0900, Tomasz Figa wrote:
> On Fri, Oct 12, 2018 at 4:58 PM Sakari Ailus
>  wrote:
> >
> > Hi Tomasz,
> >
> > On Fri, Oct 12, 2018 at 01:51:10PM +0900, Tomasz Figa wrote:
> > > Hi Sakari,
> > >
> > > On Wed, Sep 26, 2018 at 11:38 AM  wrote:
> > > >
> > > > From: Bingbu Cao 
> > > >
> > > > Add a v4l2 sub-device driver for the Sony imx319 image sensor.
> > > > This is a camera sensor using the i2c bus for control and the
> > > > csi-2 bus for data.
> > > >
> > > > This driver supports following features:
> > > > - manual exposure and analog/digital gain control support
> > > > - vblank/hblank control support
> > > > -  4 test patterns control support
> > > > - vflip/hflip control support (will impact the output bayer order)
> > > > - support following resolutions:
> > > > - 3264x2448, 3280x2464 @ 30fps
> > > > - 1936x1096, 1920x1080 @ 60fps
> > > > - 1640x1232, 1640x922, 1296x736, 1280x720 @ 120fps
> > > > - support 4 bayer orders output (via change v/hflip)
> > > > - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10
> > > >
> > > > Cc: Tomasz Figa 
> > > > Cc: Sakari Ailus 
> > > > Signed-off-by: Bingbu Cao 
> > > > Signed-off-by: Tianshu Qiu 
> > > >
> > > > ---
> > > >
> > > > This patch is based on sakari's media-tree git:
> > > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-1
> > > >
> > > > Changes from v5:
> > > >  - add some comments for gain calculation
> > > >  - use lock to protect the format
> > > >  - fix some style issues
> > > >
> > > > Changes from v4 to v5:
> > > >  - use single PLL for all internal clocks
> > > >  - change link frequency to 482.4MHz
> > > >  - adjust frame timing for 2x2 binning modes
> > > >and enlarge frame readout time
> > > >  - get CSI-2 link frequencies and external clock
> > > >from firmware
> > >
> > > If I remember correctly, that was suggested by you. Why do we need to
> > > specify link frequency in firmware if it's fully configured by the
> > > driver, with the only external dependency being the external clock?
> >
> > The driver that's now in upstream supports, for now, a very limited set of
> > configurations from what the sensor supports. These are more or less
> > tailored to the particular system where it is being used right now (output
> > image size, external clock frequency, frame rates, link frequencies etc.).
> 
> As a side note, they're tailored to exactly the system I mentioned,
> with different link frequency hardcoded in the firmware, coming from
> earlier stage of development.
> 
> > If the same sensor is needed elsewhere (quite likely), the configuration
> > needed elsewhere is very likely to be different from what you're using now.
> >
> > The link frequency in particular is important as using a different link
> > frequency (which could be fine elsewhere) could cause EMI issues, e.g.
> > rendering your GPS receiver inoperable during the time the camera sensor is
> > streaming images.
> >
> > Should new configurations be added to this driver to support a different
> > system, the link frequencies used by those configurations may be
> > problematic to your system, and after a software update the driver could as
> > well use those new frequencies. That's a big no-no.
> >
> 
> Okay, those are some valid points indeed, thanks for clarifying.
> 
> > >
> > > We're having problems with firmware listing the link frequency from v4
> > > and we can't easily change it anymore to report the new one. I feel
> > > like this dependency on the firmware here is unnecessary, as long as
> > > the external clock frequency matches.
> >
> > This is information you really need to know.
> >
> > A number of older drivers do not use the link frequency information from
> > the firmware but that comes with a risk. Really, it's better to change the
> > frequency now to something you can choose, rather than have it changed
> > later on to something someone else chose for you.
> 
> I guess it means that we have to carry a local downstream patch that
> bypasses this check, since we cannot easily change the firmware
> anymore.

Is there a possibility update the firmware, or carry an SSDT overlay as part
of the software? The options are laid out in
Documentation/acpi/ssdt-overlays.txt . Do remember to pay attention to the
revision field --- also in future Coreboot updates.

> 
> An alternative would be to make the driver try to select a frequency
> that matches what's in the firmware, but issue a warning and fall back
> to a default one if a matching is not found. It might be actually
> better than nothing for some early testing on new systems, since it
> wouldn't require firmware changes.

You don't need firmware changes per se; see above. Allowing that will very,
very easily lead this being unaddressed during developement and changing
later on inadvertly.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


Re: [PATCH 1/4] media: ov5640: fix resolution update

2018-10-16 Thread Hugues FRUCHET
Hi Jacopo,

On 10/15/2018 05:24 PM, jacopo mondi wrote:
> Hi Hugues,
> 
> On Mon, Oct 15, 2018 at 03:13:12PM +, Hugues FRUCHET wrote:
>> Hi Laurent, Jacopo, Sam,
>>
>> I'm also OK to change to a simpler alternative;
>> - drop the "restore" step
> 
> Do you mean the restore step at the end of 'ov5640_restore_mode()' ?

yes

> I agree, I've been carrying this one [1] in my tree for some time now.
> I just didn't send it because of the too many issues in flight on this
> driver.
> 
>> - send the whole init register sequence + mode changes + format changes
>> at streamon
>>
> 
> This might be a first step in my opinion too, yes.
> 
>> is this what you have in mind Laurent ?
> 
> I know Laurent does not fully agree with me on this, but I would like
> to have Maxime's series on clock tree handling merged and tested on
> CSI-2 first before adding more patches to the pile of pending items on
> ov5640. I hope to have time to test them on CSI-2 this week before
> ELC-E.
> 
> Steve, you're the driver maintainer do you have preferences here?
> 
> Also, if this might be useful, I would like to help co-maintaining the
> driver (with possibily other people, possibly with the sensor wired in
> DVP mode), and help establishing priorities, as this driver needs some
> love, but one item at the time to avoid getting lost in too many
> pending changes as it happened recently :)

It's a good problem having too many contributions ;) This means that 
there is some interest on it...
For DVP, me and Maxime have two different setup so we can cover the DVP 
tests.

> 
> Thanks
> j
> 
> [1]
>  media: ov5640: Do not restore format at power up
> 
>  Do not force restoring the last applied capture format during sensor 
> power up
>  as it will be re-set at s_stream time.
> 
>  Signed-off-by: Jacopo Mondi 
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index b226946..17ee55b 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1737,12 +1737,10 @@ static int ov5640_restore_mode(struct ov5640_dev 
> *sensor)
>  if (ret)
>  return ret;
> 
> -   /* now restore the last capture mode */
> -   ret = ov5640_set_mode(sensor, _mode_init_data);
> -   if (ret < 0)
> -   return ret;
> +   sensor->pending_mode_change = true;
> +   sensor->pending_fmt_change = true;
> 
> -   return ov5640_set_framefmt(sensor, >fmt);
> +   return 0;
>   }
> 
>   static void ov5640_power(struct ov5640_dev *sensor, bool enable)
> 
>>
>> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
>>> Hi Jacopo,
>>>
>>> On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
 Hi Sam,
  thanks for the patch, I see the same issue you reported, but I
 think this patch can be improved.

 (expanding the Cc list to all people involved in recent ov5640
 developemts, not just for this patch, but for the whole series to look
 at. Copying names from another series cover letter, hope it is
 complete.)

 On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> set_fmt was not properly triggering a mode change when
> a new mode was set that happened to have the same format
> as the previous mode (for example, when only changing the
> frame dimensions). Fix this.
>
> Signed-off-by: Sam Bobrowicz 
> ---
>
>drivers/media/i2c/ov5640.c | 8 
>1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..5031aab 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>
>   goto out;
>
>   }
>
> - if (new_mode != sensor->current_mode) {
> +
> + if (new_mode != sensor->current_mode ||
> + mbus_fmt->code != sensor->fmt.code) {
> + sensor->fmt = *mbus_fmt;
>
>   sensor->current_mode = new_mode;
>   sensor->pending_mode_change = true;
>
> - }
> - if (mbus_fmt->code != sensor->fmt.code) {
> - sensor->fmt = *mbus_fmt;
>
>   sensor->pending_fmt_change = true;
>
>   }

 How I did reproduce the issue:

 # Set 1024x768 on ov5640 without changing the image format
 # (default image size at startup is 640x480)
 $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
 sensor->pending_mode_change = true; //verified this flag gets set

 # Start streaming, after having configured the whole pipeline to work
 # with 1024x768
 $  yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
  Unable to start streaming: Broken pipe (32).

 # Inspect which part of pipeline validation went wrong
 # Turns out the sensor->fmt 

Re: [PATCH v4 12/12] ov5640: Enforce a mode change when changing the framerate

2018-10-16 Thread Hugues FRUCHET
You're welcome ;)

On 10/16/2018 09:10 AM, Maxime Ripard wrote:
> Hi Hugues,
> 
> On Mon, Oct 15, 2018 at 01:57:40PM +, Hugues FRUCHET wrote:
>> This is already fixed in media tree:
>> 0929983e49c81c1d413702cd9b83bb06c4a2555c media: ov5640: fix framerate update
> 
> My bad then, I missed it, thanks!
> Maxime
> 

[ragnatech:media-tree] BUILD SUCCESS 490d84f6d73c12f4204241cff8651eed60aae914

2018-10-16 Thread kbuild test robot
tree/branch: git://git.ragnatech.se/linux  media-tree
branch HEAD: 490d84f6d73c12f4204241cff8651eed60aae914  media: cec: forgot to 
cancel delayed work

elapsed time: 1093m

configs tested: 210

The following configs have been built successfully.
More configs may be tested in the coming days.

alpha   defconfig
pariscallnoconfig
parisc b180_defconfig
pariscc3000_defconfig
parisc  defconfig
m68k apollo_defconfig
mips   lemote2f_defconfig
x86_64 randconfig-a0-10161216
x86_64 acpi-redef
x86_64   allyesdebian
x86_64nfsroot
shtitan_defconfig
sh  rsk7269_defconfig
sh  sh7785lcr_32bit_defconfig
shallnoconfig
i386   randconfig-c0-10161145
arm s3c6400_defconfig
mips loongson1c_defconfig
i386   randconfig-x019-201841
i386   randconfig-x016-201841
i386   randconfig-x018-201841
i386   randconfig-x014-201841
i386   randconfig-x015-201841
i386   randconfig-x011-201841
i386   randconfig-x012-201841
i386   randconfig-x017-201841
i386   randconfig-x010-201841
i386   randconfig-x013-201841
microblaze  mmu_defconfig
microblazenommu_defconfig
i386 randconfig-n0-201841
i386 randconfig-n1-201841
i386 randconfig-n2-201841
i386 randconfig-n3-201841
x86_64 randconfig-x000-201841
x86_64 randconfig-x005-201841
x86_64 randconfig-x002-201841
x86_64 randconfig-x001-201841
x86_64 randconfig-x008-201841
x86_64 randconfig-x007-201841
x86_64 randconfig-x009-201841
x86_64 randconfig-x003-201841
x86_64 randconfig-x006-201841
x86_64 randconfig-x004-201841
ia64  allnoconfig
ia64defconfig
ia64 alldefconfig
powerpc   allnoconfig
powerpc defconfig
powerpc   ppc64_defconfig
s390default_defconfig
i386 randconfig-i0-201841
i386 randconfig-i1-201841
i386 randconfig-i2-201841
i386 randconfig-i3-201841
arm   allnoconfig
arm at91_dt_defconfig
arm   efm32_defconfig
arm  exynos_defconfig
armmulti_v5_defconfig
armmulti_v7_defconfig
armshmobile_defconfig
arm   sunxi_defconfig
arm64 allnoconfig
arm64   defconfig
mipsnlm_xlr_defconfig
sh   se7343_defconfig
x86_64 randconfig-g0-10161105
openriscor1ksim_defconfig
um i386_defconfig
um   x86_64_defconfig
c6xevmc6678_defconfig
h8300h8300h-sim_defconfig
nios2 10m50_defconfig
xtensa   common_defconfig
xtensa  iss_defconfig
i386 allmodconfig
i386   randconfig-x002-201841
i386   randconfig-x006-201841
i386   randconfig-x005-201841
i386   randconfig-x000-201841
i386   randconfig-x007-201841
i386   randconfig-x004-201841
i386   randconfig-x001-201841
i386   randconfig-x009-201841
i386   randconfig-x008-201841
i386   randconfig-x003-201841
arm  allmodconfig
arm  arm5
arm arm67
arm   imx_v6_v7_defconfig
arm  ixp4xx_defconfig
armmvebu_v7_defconfig
arm   omap2plus_defconfig
armsa1100
arm   samsung
armsh
arm   tegra_defconfig
arm64alldefconfig
arm64allmodconfig
x86_64 randconfig-s0-10161132
x86_64 randconfig-s1-10161132
x86_64  

[PATCH] cec: report Vendor ID after initialization

2018-10-16 Thread Hans Verkuil
The CEC specification requires that the Vendor ID (if any) is reported after
a logical address was claimed.

This was never done, so add support for this.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 31d1f4ab915e..ee4decc1cd64 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -1405,6 +1405,13 @@ static int cec_config_thread_func(void *arg)
las->log_addr[i],
cec_phys_addr_exp(adap->phys_addr));
cec_transmit_msg_fh(adap, , NULL, false);
+
+   /* Report Vendor ID */
+   if (adap->log_addrs.vendor_id != CEC_VENDOR_ID_NONE) {
+   cec_msg_device_vendor_id(,
+adap->log_addrs.vendor_id);
+   cec_transmit_msg_fh(adap, , NULL, false);
+   }
}
adap->kthread_config = NULL;
complete(>config_completion);


Re: [PATCH v4 12/12] ov5640: Enforce a mode change when changing the framerate

2018-10-16 Thread Maxime Ripard
Hi Hugues,

On Mon, Oct 15, 2018 at 01:57:40PM +, Hugues FRUCHET wrote:
> This is already fixed in media tree:
> 0929983e49c81c1d413702cd9b83bb06c4a2555c media: ov5640: fix framerate update

My bad then, I missed it, thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com