cron job: media_tree daily build: OK

2018-06-01 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:   Sat Jun  2 05:00:14 CEST 2018
media-tree git hash:a00031c159748f322f771f3c1d5ed944cba4bd30
media_build git hash:   464ef972618cc9f845f07c1a4e8957ce2270cf91
v4l-utils git hash: 034fdb4bc2dd380a3c77c0b82c03c99c222ddef4
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.16.0-1-amd64

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-2.6.36.4-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.101-i686: OK
linux-3.2.101-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: 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.56-i686: OK
linux-3.16.56-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.102-i686: OK
linux-3.18.102-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.51-i686: OK
linux-4.1.51-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.109-i686: OK
linux-4.4.109-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.91-i686: OK
linux-4.9.91-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.42-i686: OK
linux-4.14.42-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
linux-4.16.8-i686: OK
linux-4.16.8-x86_64: OK
linux-4.17-rc4-i686: OK
linux-4.17-rc4-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH v3 03/12] media: ov5640: Remove the clocks registers initialization

2018-06-01 Thread Sam Bobrowicz
>> On May 21, 2018, at 12:39 AM, Maxime Ripard  
>> wrote:
>>
>>> On Fri, May 18, 2018 at 07:42:34PM -0700, Sam Bobrowicz wrote:
 On Fri, May 18, 2018 at 3:35 AM, Daniel Mack  wrote:
 On Thursday, May 17, 2018 10:53 AM, Maxime Ripard wrote:

 Part of the hardcoded initialization sequence is to set up the proper
 clock
 dividers. However, this is now done dynamically through proper code and as
 such, the static one is now redundant.

 Let's remove it.

 Signed-off-by: Maxime Ripard 
 ---
>>>
>>>
>>> [...]
>>>
 @@ -625,8 +623,8 @@ static const struct reg_value
 ov5640_setting_30fps_1080P_1920_1080[] = {
   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
   {0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
   {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
 -   {0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0}, {0x3035, 0x11, 0, 0},
 -   {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
 +   {0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0},
 +   {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
>>>
>>>
>>> This is the mode that I'm testing with. Previously, the hard-coded registers
>>> here were:
>>>
>>> OV5640_REG_SC_PLL_CTRL1 (0x3035) = 0x11
>>> OV5640_REG_SC_PLL_CTRL2 (0x3036) = 0x54
>>> OV5640_REG_SC_PLL_CTRL3 (0x3037) = 0x07
>>>
>>> Your new code that calculates the clock rates dynamically ends up with
>>> different values however:
>>>
>>> OV5640_REG_SC_PLL_CTRL1 (0x3035) = 0x11
>>> OV5640_REG_SC_PLL_CTRL2 (0x3036) = 0xa8
>>> OV5640_REG_SC_PLL_CTRL3 (0x3037) = 0x03
>>>
>>> Interestingly, leaving the hard-coded values in the array *and* letting
>>> ov5640_set_mipi_pclk() do its thing later still works. So again it seems
>>> that writes to registers after 0x3035/0x3036/0x3037 seem to depend on the
>>> values of these timing registers. You might need to leave these values as
>>> dummies in the array. Confusing.
>>>
>>> Any idea?
>>>
>>>
>>> Thanks,
>>> Daniel
>>
>> This set of patches is also not working for my MIPI platform (mine has
>> a 12 MHz external clock). I am pretty sure is isn't working because it
>> does not include the following, which my tests have found to be
>> necessary:
>>
>> 1) Setting pclk period reg in order to correct DPHY timing.
>> 2) Disabling of MIPI lanes when streaming not enabled.
>> 3) setting mipi_div to 1 when the scaler is disabled
>> 4) Doubling ADC clock on faster resolutions.
>
> Yeah, I left them out because I didn't think this was relevant to this
> patchset but should come as future improvements. However, given that
> it works with the parallel bus, maybe the two first are needed when
> adjusting the rate.
>
I agree that 1-4 are separate improvements to MIPI mode that may not
affect all modules. They do break mine, but that has been true since
the driver was released (mainly because of my 12 MHz clock and more
stringent CSI RX requirements). So it makes sense for me to address
them in a follow-up series. But I do think that we should get the
clock generation a little closer to what I know works for MIPI so we
don't break things for people that do have MIPI working.

> The mipi divider however seems to be a bit more complicated than you
> report here. It is indeed set to 1 when the scaler is enabled (all
> resolutions > 1280 * 960), but it's also set to 4 in some cases
> (640x480@30, 320x240@30, 176x144@30). I couldn't really find any
> relationship between the resolution/framerate and whether to use a
> divider of 2 or 4.

I didn't notice the divide by 4, interesting. I have a theory
though... it could be that the constraint of PCLK relative to SCLK is:

SCLK*(cpp/scaler ratio)<=PCLK<= ?
cpp=Components/pixel (1 for JPEG, 2 for YUV, e.g.)

Since the scaler is in auto mode, the scaler ratio might automatically
change depending on the resolution selected, something like (using int
math):

(hTotal/hActive) * (vTotal/vActive) = scaler ratio

If SCLK is responsible for reading the data into the scaler, and PCLK
is responsible for reading data out to the physical interface, this
would make sense. It will require more experiments to verify any of
this, though, and unfortunately I don't have a lot of time to put into
this right now.

On my platform I have also run into an upper bound for PCLK, where it
seems that PCLK must be <= SCLK when the scaler is enabled. I think
this may have to do with some of my platform's idiosyncrasies, so I'm
not ready to say that it needs to be addressed in this series. But if
others run into it while testing MIPI, you should consider
implementing #3 above to address it.

> And the faster resolutions were working already, so I guess the ADC
> clock is already fast enough with a 24MHz oscillator?

That's my theory. It seems to have pretty loose requirements as long
as it is fast enough, which is why I did the simple 2x solution. It
doesn't need to be addressed here though. If anyone runs 

[PATCH 2/2] rockchip/rga: Remove unrequired wait in .job_abort

2018-06-01 Thread Ezequiel Garcia
As per the documentation, job_abort is not required
to wait until the current job finishes. It is redundant
to do so, as the core will perform the wait operation.

Remove the wait infrastructure completely.

Signed-off-by: Ezequiel Garcia 
---
 drivers/media/platform/rockchip/rga/rga.c | 13 +
 drivers/media/platform/rockchip/rga/rga.h |  2 --
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c 
b/drivers/media/platform/rockchip/rga/rga.c
index d508a8ba6f89..5a5a6139e18a 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -41,14 +41,7 @@ module_param(debug, int, 0644);
 
 static void job_abort(void *prv)
 {
-   struct rga_ctx *ctx = prv;
-   struct rockchip_rga *rga = ctx->rga;
-
-   if (!rga->curr) /* No job currently running */
-   return;
-
-   wait_event_timeout(rga->irq_queue,
-  !rga->curr, msecs_to_jiffies(RGA_TIMEOUT));
+   /* Can't do anything rational here */
 }
 
 static void device_run(void *prv)
@@ -104,8 +97,6 @@ static irqreturn_t rga_isr(int irq, void *prv)
v4l2_m2m_buf_done(src, VB2_BUF_STATE_DONE);
v4l2_m2m_buf_done(dst, VB2_BUF_STATE_DONE);
v4l2_m2m_job_finish(rga->m2m_dev, ctx->fh.m2m_ctx);
-
-   wake_up(>irq_queue);
}
 
return IRQ_HANDLED;
@@ -838,8 +829,6 @@ static int rga_probe(struct platform_device *pdev)
spin_lock_init(>ctrl_lock);
mutex_init(>mutex);
 
-   init_waitqueue_head(>irq_queue);
-
ret = rga_parse_dt(rga);
if (ret)
dev_err(>dev, "Unable to parse OF data\n");
diff --git a/drivers/media/platform/rockchip/rga/rga.h 
b/drivers/media/platform/rockchip/rga/rga.h
index 5d43e7ea88af..72d8a159fa7b 100644
--- a/drivers/media/platform/rockchip/rga/rga.h
+++ b/drivers/media/platform/rockchip/rga/rga.h
@@ -86,8 +86,6 @@ struct rockchip_rga {
/* ctrl parm lock */
spinlock_t ctrl_lock;
 
-   wait_queue_head_t irq_queue;
-
struct rga_ctx *curr;
dma_addr_t cmdbuf_phy;
void *cmdbuf_virt;
-- 
2.17.1



[PATCH 1/2] rockchip/rga: Fix broken .start_streaming

2018-06-01 Thread Ezequiel Garcia
Currently, rga_buf_start_streaming() is expecting
pm_runtime_get_sync to return zero on success, which
is wrong.

As per the documentation, pm_runtime_get_sync increments
the device's usage counter and return its result.
This means it will typically return a positive integer
on success and a negative error code.

Therefore, rockchip-rga driver is currently unusable
failing to start_streaming in most cases. Fix it and
while here, cleanup the buffer return-to-core logic.

Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
Signed-off-by: Ezequiel Garcia 
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 44 +--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c 
b/drivers/media/platform/rockchip/rga/rga-buf.c
index fa1ba98c96dc..c7c5a371b11e 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -64,43 +64,43 @@ static void rga_buf_queue(struct vb2_buffer *vb)
v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 }
 
+static void rga_buf_return_buffers(struct vb2_queue *q, enum vb2_buffer_state 
state)
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(q);
+   struct vb2_v4l2_buffer *vbuf;
+
+   for (;;) {
+   if (V4L2_TYPE_IS_OUTPUT(q->type))
+   vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+   else
+   vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+   if (!vbuf)
+   break;
+   v4l2_m2m_buf_done(vbuf, state);
+   }
+}
+
 static int rga_buf_start_streaming(struct vb2_queue *q, unsigned int count)
 {
struct rga_ctx *ctx = vb2_get_drv_priv(q);
struct rockchip_rga *rga = ctx->rga;
-   int ret, i;
+   int ret;
 
ret = pm_runtime_get_sync(rga->dev);
-
-   if (!ret)
-   return 0;
-
-   for (i = 0; i < q->num_buffers; ++i) {
-   if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
-   v4l2_m2m_buf_done(to_vb2_v4l2_buffer(q->bufs[i]),
- VB2_BUF_STATE_QUEUED);
-   }
+   if (ret < 0) {
+   rga_buf_return_buffers(q, VB2_BUF_STATE_QUEUED);
+   return ret;
}
 
-   return ret;
+   return 0;
 }
 
 static void rga_buf_stop_streaming(struct vb2_queue *q)
 {
struct rga_ctx *ctx = vb2_get_drv_priv(q);
struct rockchip_rga *rga = ctx->rga;
-   struct vb2_v4l2_buffer *vbuf;
-
-   for (;;) {
-   if (V4L2_TYPE_IS_OUTPUT(q->type))
-   vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-   else
-   vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
-   if (!vbuf)
-   break;
-   v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
-   }
 
+   rga_buf_return_buffers(q, VB2_BUF_STATE_ERROR);
pm_runtime_put(rga->dev);
 }
 
-- 
2.17.1



[PATCH 0/2] rockchip/rga: A fix and a cleanup

2018-06-01 Thread Ezequiel Garcia
Decided to test v4l2transform filters and found these two
issues.

Without the first commit, start_streaming fails. The second
commit is just a cleanup, removing a seemingly redundant
operation.

Tested on RK3288 Radxa Rock2 with these kind of pipelines:

gst-launch-1.0 videotestsrc ! 
video/x-raw,width=640,height=480,framerate=30/1,format=RGB ! v4l2video0convert 
! video/x-raw,width=1920,height=1080,framerate=30/1,format=NV16 ! fakesink

gst-launch-1.0 v4l2src device=/dev/video1 ! 
video/x-raw,width=640,height=480,framerate=30/1,format=RGB ! v4l2video0convert 
! video/x-raw,width=1920,height=1080,framerate=30/1,format=NV16 ! kmssink

Ezequiel Garcia (2):
  rockchip/rga: Fix broken .start_streaming
  rockchip/rga: Remove unrequired wait in .job_abort

 drivers/media/platform/rockchip/rga/rga-buf.c | 44 +--
 drivers/media/platform/rockchip/rga/rga.c | 13 +-
 drivers/media/platform/rockchip/rga/rga.h |  2 -
 3 files changed, 23 insertions(+), 36 deletions(-)

-- 
2.17.1



Re: [PATCH v6 05/13] clk: imx7d: reset parent for mipi csi root

2018-06-01 Thread Stephen Boyd
Quoting Rui Miguel Silva (2018-05-22 07:52:37)
> To guarantee that we do not get Overflow in image FIFO the outer bandwidth has
> to be faster than inputer bandwidth. For that it must be possible to set a
> faster frequency clock. So set new parent to sys_pfd3 clock for the mipi csi
> block.
> 
> Acked-by: Shawn Guo 
> Signed-off-by: Rui Miguel Silva 
> ---

Applied to clk-next



Re: [PATCH v6 04/13] clk: imx7d: fix mipi dphy div parent

2018-06-01 Thread Stephen Boyd
Quoting Rui Miguel Silva (2018-05-22 07:52:36)
> Fix the mipi dphy root divider to mipi_dphy_pre_div, this would remove a 
> orphan
> clock and set the correct parent.
> 
> before:
> cat clk_orphan_summary
>  enable  prepare  protect
>clock  countcountcountrate   
> accuracy   phase
> 

Applied to clk-next



Re: i.MX6 IPU CSI analog video input on Ventana

2018-06-01 Thread Philipp Zabel
On Fri, 2018-05-25 at 16:21 -0700, Steve Longerbeam wrote:
> Hi Philipp,
> 
> On 05/24/2018 11:32 PM, Philipp Zabel wrote:
> > On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
> > [...]
> > > > The following is required as well. Now the question is why we can't skip
> > > > writing those odd UV rows. Anyway, with these 2 changes, I get a stable
> > > > NTSC (and probably PAL) interlaced video stream.
> > > > 
> > > > The manual says: Reduce Double Read or Writes (RDRW):
> > > > This bit is relevant for YUV4:2:0 formats. For write channels:
> > > > U and V components are not written to odd rows.
> > > > 
> > > > How could it be so? With YUV420, are they normally written?
> > > 
> > > Well, given that this bit exists, and assuming I understand it correctly
> > > (1),
> > > I guess the U and V components for odd rows normally are placed on the
> > > AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> > > the U and V components are the same for odd and even rows.
> > > 
> > > In other words for writing 4:2:0 to memory, this bit should _always_ be 
> > > set.
> > > 
> > > (1) OTOH I don't really understand what this bit is trying to say.
> > > Whether this bit is set or not, the data in memory is correct
> > > for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
> > > (decimated by 2 in width and height), followed by Y plane of 1/4
> > > size.
> > > 
> > > So I assume it is saying that the IPU normally places U/V components
> > > on the AXI bus for odd rows, that are identical to the even row values.
> > 
> > Whether they are identical depends on the input format.
> 
> Right, this is the part I was missing, thanks for clarifying. The
> even and odd chroma rows coming into the IDMAC from the
> CSI (or IC) may not be identical if the CSI has captured 4:4:4
> (or 4:2:2 yeah? 4:2:2 is only decimated in width not height).

Oh right, the MEDIA_BUS_FMT_YUYV variants are pretty common, they have
chroma for all the lines. Actually, that is my default test case (1080p
YUYV from TC358743), usually written to NV12 so it can be encoded.

> But still, when the IDMAC has finished pixel packing/unpacking and
> is writing 4:2:0 to memory, it should always skip overwriting the even
> rows with the odd rows, whether or not it has received identical chroma
> even/odd lines from the CSI.
> 
> Unless interweave is enabled :) See below.

Agreed.

regards
Philipp


Re: [PATCH v2 10/10] media: imx.rst: Update doc to reflect fixes to interlaced capture

2018-06-01 Thread Philipp Zabel
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> Also add an example pipeline for unconverted capture with interweave
> on SabreAuto.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/media/v4l-drivers/imx.rst | 51 
> -
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/media/v4l-drivers/imx.rst 
> b/Documentation/media/v4l-drivers/imx.rst
> index 65d3d15..4149b76 100644
> --- a/Documentation/media/v4l-drivers/imx.rst
> +++ b/Documentation/media/v4l-drivers/imx.rst
> @@ -179,9 +179,10 @@ sink pad can take UYVY2X8, but the IDMAC source pad can 
> output YUYV2X8.
>  If the sink pad is receiving YUV, the output at the capture device can
>  also be converted to a planar YUV format such as YUV420.
>  
> -It will also perform simple de-interlace without motion compensation,
> -which is activated if the sink pad's field type is an interlaced type,
> -and the IDMAC source pad field type is set to none.
> +It will also perform simple interweave without motion compensation,
> +which is activated if the sink pad's field type is sequential top-bottom
> +or bottom-top or alternate, and the IDMAC source pad field type is
> +interlaced (t-b, b-t, or unqualified interlaced).

I think sink pad alternate behaviour should be equal to sink pad top-
bottom for PAL and sink pad bottom-top for NTSC. If we agree on this, we
should mention that here.

>  This subdev can generate the following event when enabling the second
>  IDMAC source pad:
> @@ -383,13 +384,13 @@ and CSC operations and flip/rotation controls. It will 
> receive and
>  process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is
>  receiving from ipuX_vdic.
>  
> -Like the ipuX_csiY IDMAC source, it can perform simple de-interlace
> +Like the ipuX_csiY IDMAC source, it can perform simple interweaving
>  without motion compensation. However, note that if the ipuX_vdic is
>  included in the pipeline (ipuX_ic_prp is receiving from ipuX_vdic),
> -it's not possible to use simple de-interlace in ipuX_ic_prpvf, since
> -the ipuX_vdic has already carried out de-interlacing (with motion
> -compensation) and therefore the field type output from ipuX_ic_prp can
> -only be none.
> +it's not possible to use interweave in ipuX_ic_prpvf, since the
> +ipuX_vdic has already carried out de-interlacing (with motion
> +compensation) and therefore the field type output from ipuX_vdic
> +can only be none (progressive).
>  
>  Capture Pipelines
>  -
> @@ -514,10 +515,32 @@ On the SabreAuto, an on-board ADV7180 SD decoder is 
> connected to the
>  parallel bus input on the internal video mux to IPU1 CSI0.
>  
>  The following example configures a pipeline to capture from the ADV7180
> +video decoder, assuming NTSC 720x480 input signals, using simple
> +interweave (unconverted and without motion compensation). The adv7180
> +must output sequential or alternating fields (field type 'seq-tb',
> +'seq-bt', or 'alternate'):
> +
> +.. code-block:: none
> +
> +   # Setup links
> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> +   # Configure pads
> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480 field:interlaced]"

Could the example suggest using interlaced-bt to be explicit here?
Actually, I don't think we should allow interlaced on the CSI src pads
at all in this case. Technically it always writes either seq-tb or seq-
bt into the smfc, never interlaced (unless the input is already
interlaced).

> +Streaming can then begin on the capture device node at
> +"ipu1_csi0 capture". The v4l2-ctl tool can be used to select any
> +supported YUV pixelformat on the capture device node.
> +
> +This example configures a pipeline to capture from the ADV7180
>  video decoder, assuming NTSC 720x480 input signals, with Motion
> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
> -entity at its output pad:
> +Compensated de-interlacing. The adv7180 must output sequential or
> +alternating fields (field type 'seq-tb', 'seq-bt', or 'alternate').
> +$outputfmt can be any format supported by the ipu1_ic_prpvf entity
> +at its output pad:
>  
>  .. code-block:: none
>  
> @@ -529,9 +552,9 @@ entity at its output pad:
> media-ctl -l "'ipu1_ic_prp':2 -> 'ipu1_ic_prpvf':0[1]"
> media-ctl -l "'ipu1_ic_prpvf':1 -> 'ipu1_ic_prpvf capture':0[1]"
> # Configure pads
> -   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480]"
> -   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]"
> -   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x480 field:interlaced]"
> +   media-ctl -V "'adv7180 3-0021':0 

Re: [PATCH v2 07/10] media: imx-csi: Allow skipping odd chroma rows for YVU420

2018-06-01 Thread Philipp Zabel
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> Skip writing U/V components to odd rows for YVU420 in addition to
> YUV420 and NV12.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-media-csi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 6101e2ed..c878a00 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -430,6 +430,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   passthrough_bits = 16;
>   break;
>   case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
>   case V4L2_PIX_FMT_NV12:
>   burst_size = (image.pix.width & 0x3f) ?
>((image.pix.width & 0x1f) ?

Reviewed-by: Philipp Zabel 

regards
Philipp


Re: [PATCH v2 06/10] media: imx: Fix field setting logic in try_fmt

2018-06-01 Thread Philipp Zabel
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> The logic for setting field type in try_fmt at CSI and PRPENCVF
> entities wasn't quite right. The behavior should be:
> 
> - No restrictions on field type at sink pads (except ANY, which is filled
>   with current sink pad field by imx_media_fill_default_mbus_fields()).
> 
> - At IDMAC output pads, if the caller asks for an interlaced output, and
>   the input is sequential fields, the IDMAC output channel can accommodate
>   by interweaving. The CSI can also interweave if input is alternate
>   fields.
> 
> - If final source pad field type is alternate, translate to seq_bt or
>   seq_tb. But the field order translation was backwards, SD NTSC is BT
>   order, SD PAL is TB.
> 
> Move this logic to new functions csi_try_field() and prp_try_field().
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-ic-prpencvf.c | 22 +++--
>  drivers/staging/media/imx/imx-media-csi.c   | 50 
> +
>  2 files changed, 56 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
> b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 7e1e0c3..1002eb1 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -833,6 +833,21 @@ static int prp_get_fmt(struct v4l2_subdev *sd,
>   return ret;
>  }
>  
> +static void prp_try_field(struct prp_priv *priv,
> +   struct v4l2_subdev_pad_config *cfg,
> +   struct v4l2_subdev_format *sdformat)
> +{
> + struct v4l2_mbus_framefmt *infmt =
> + __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD, sdformat->which);
> +
> + /* no restrictions on sink pad field type */
> + if (sdformat->pad == PRPENCVF_SINK_PAD)
> + return;
> +
> + if (!idmac_interweave(sdformat->format.field, infmt->field))
> + sdformat->format.field = infmt->field;

This is not strict enough. As I wrote in reply to patch 4, we can only
do SEQ_TB -> INTERLACED_TB and SEQ_BT -> INTERLACED_BT interweaving.

regards
Philipp


Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields

2018-06-01 Thread Philipp Zabel
Hi Steve,

On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> IDMAC interlaced scan, a.k.a. interweave, should be enabled at the
> IDMAC output pads only if the input field type is 'seq-bt' or 'seq-tb',
> and the IDMAC output pad field type is 'interlaced*'. Move this
> determination to a new macro idmac_interweave().
> 
> V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
> enabling interlaced/interweave scan. That macro includes the 'interlaced'
> field types, and in those cases the data is already interweaved with
> top/bottom field lines.
> 
> The CSI will capture whole frames when the source specifies alternate
> field mode. So the CSI also enables interweave at the IDMAC output pad
> for alternate input field type.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-ic-prpencvf.c | 22 ++
>  drivers/staging/media/imx/imx-media-csi.c   | 22 ++
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
> b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index ae453fd..894db21 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -132,6 +132,18 @@ static inline struct prp_priv *sd_to_priv(struct 
> v4l2_subdev *sd)
>   return ic_priv->task_priv;
>  }
>  
> +/*
> + * If the field type at IDMAC output pad is interlaced, and
> + * the input is sequential fields, the IDMAC output channel
> + * can accommodate by interweaving.
> + */
> +static inline bool idmac_interweave(enum v4l2_field outfield,
> + enum v4l2_field infield)
> +{
> + return V4L2_FIELD_IS_INTERLACED(outfield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(infield);
> +}

This is ok in this patch, but we can't use this check in the following
TRY_FMT patch as there is no way to interweave
SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T,
but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the
other way around).

> +
>  static void prp_put_ipu_resources(struct prp_priv *priv)
>  {
>   if (priv->ic)
> @@ -353,6 +365,7 @@ static int prp_setup_channel(struct prp_priv *priv,
>   struct v4l2_mbus_framefmt *infmt;
>   unsigned int burst_size;
>   struct ipu_image image;
> + bool interweave;
>   int ret;
>  
>   infmt = >format_mbus[PRPENCVF_SINK_PAD];
> @@ -365,6 +378,9 @@ static int prp_setup_channel(struct prp_priv *priv,
>   image.rect.width = image.pix.width;
>   image.rect.height = image.pix.height;
>  
> + interweave = (idmac_interweave(image.pix.field, infmt->field) &&
> +   channel == priv->out_ch);
> +
>   if (rot_swap_width_height) {
>   swap(image.pix.width, image.pix.height);
>   swap(image.rect.width, image.rect.height);
> @@ -405,9 +421,7 @@ static int prp_setup_channel(struct prp_priv *priv,
>   if (rot_mode)
>   ipu_cpmem_set_rotation(channel, rot_mode);
>  
> - if (image.pix.field == V4L2_FIELD_NONE &&
> - V4L2_FIELD_HAS_BOTH(infmt->field) &&
> - channel == priv->out_ch)
> + if (interweave)
>   ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);

This only works for SEQ_TB -> INTERLACED_TB interweave.
For SEQ_BT -> INTERLACED_BT we have to start at +1 line offset and set
ipu_cpmem_interlaced_scan to -image.pix.bytesperline.

regards
Philipp


Re: [PATCH v2 02/10] gpu: ipu-csi: Check for field type alternate

2018-06-01 Thread Philipp Zabel
Hi Steve,

On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> When the CSI is receiving from a bt.656 bus, include a check for
> field type 'alternate' when determining whether to set CSI clock
> mode to CCIR656_INTERLACED or CCIR656_PROGRESSIVE.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/gpu/ipu-v3/ipu-csi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index caa05b0..5450a2d 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -339,7 +339,8 @@ static void fill_csi_bus_cfg(struct ipu_csi_bus_config 
> *csicfg,
>   break;
>   case V4L2_MBUS_BT656:
>   csicfg->ext_vsync = 0;
> - if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field))
> + if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field) ||
> + mbus_fmt->field == V4L2_FIELD_ALTERNATE)
>   csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
>   else
>   csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;

Thank you, applied to imx-drm/next.

regards
Philipp


Re: [PATCH v2 01/10] media: imx-csi: Pass sink pad field to ipu_csi_init_interface

2018-06-01 Thread Philipp Zabel
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> The output pad's field type was being passed to ipu_csi_init_interface(),
> in order to deal with field type 'alternate' at the sink pad, which
> is not understood by ipu_csi_init_interface().
> 
> Remove that code and pass the sink pad field to ipu_csi_init_interface().
> The latter function will have to explicity deal with field type 'alternate'
> when setting up the CSI interface for BT.656 busses.

I fear this won't be enough. If we want to capture
sink:ALTERNATE/SEQ_TB/SEQ_BT -> src:SEQ_TB we have to configure the CSI
differently than if we want to capture
ALTERNATE/SEQ_TB/SEQ_BT -> src:SEQ_BT. (And differently for NTSC and
PAL). For NTSC sink:ALTERNATE should behave like sink:SEQ_BT, and for
PAL sink:ALTERNATE should behave like sink:SEQ_TB.

Interweaving SEQ_TB to INTERLACED_TB should work right now, but to
interweave SEQ_BT to INTERLACED_BT, we need to add one line offset to
the frame start and use a negative interlaced scanline offset.

regards
Philipp

> Reported-by: Krzysztof Hałasa 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx-media-csi.c | 13 ++---
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index 95d7805..9bc555c 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -629,12 +629,10 @@ static void csi_idmac_stop(struct csi_priv *priv)
>  /* Update the CSI whole sensor and active windows */
>  static int csi_setup(struct csi_priv *priv)
>  {
> - struct v4l2_mbus_framefmt *infmt, *outfmt;
> + struct v4l2_mbus_framefmt *infmt;
>   struct v4l2_mbus_config mbus_cfg;
> - struct v4l2_mbus_framefmt if_fmt;
>  
>   infmt = >format_mbus[CSI_SINK_PAD];
> - outfmt = >format_mbus[priv->active_output_pad];
>  
>   /* compose mbus_config from the upstream endpoint */
>   mbus_cfg.type = priv->upstream_ep.bus_type;
> @@ -642,20 +640,13 @@ static int csi_setup(struct csi_priv *priv)
>   priv->upstream_ep.bus.mipi_csi2.flags :
>   priv->upstream_ep.bus.parallel.flags;
>  
> - /*
> -  * we need to pass input frame to CSI interface, but
> -  * with translated field type from output format
> -  */
> - if_fmt = *infmt;
> - if_fmt.field = outfmt->field;
> -
>   ipu_csi_set_window(priv->csi, >crop);
>  
>   ipu_csi_set_downsize(priv->csi,
>priv->crop.width == 2 * priv->compose.width,
>priv->crop.height == 2 * priv->compose.height);
>  
> - ipu_csi_init_interface(priv->csi, _cfg, _fmt);
> + ipu_csi_init_interface(priv->csi, _cfg, infmt);
>  
>   ipu_csi_set_dest(priv->csi, priv->dest);
>  


[PATCH] gpu: ipu-v3: Allow negative offsets for interlaced scanning

2018-06-01 Thread Philipp Zabel
The IPU also supports interlaced buffers that start with the bottom field.
To achieve this, the the base address EBA has to be increased by a stride
length and the interlace offset ILO has to be set to the negative stride.

Signed-off-by: Philipp Zabel 
---
 drivers/gpu/ipu-v3/ipu-cpmem.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index 9f2d9ec42add..c1028f38c553 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -269,8 +269,14 @@ EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
 
 void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
 {
+   u32 ilo;
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
-   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, stride / 8);
+   if (stride >= 0)
+   ilo = stride / 8;
+   else
+   ilo = 0x10 - (-stride / 8);
+   ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, (stride * 2) - 1);
 };
 EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
-- 
2.17.1



Re: i.MX6 IPU CSI analog video input on Ventana

2018-06-01 Thread Philipp Zabel
Hi Krzysztof,

On Fri, 2018-06-01 at 12:02 +0200, Krzysztof Hałasa wrote:
> Steve Longerbeam  writes:
> 
> > I tend to agree, I've found conflicting info out there regarding
> > PAL vs. NTSC field order. And I've never liked having to guess
> > at input analog standard based on input # lines. I will go ahead
> > and remove the field order override code.
> 
> I've merged your current fix-csi-interlaced.2 branch (2018-06-01
> 00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
> "media: adv7180: fix field type" commit and NTSC camera:
> 
> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"
> 
> correctly sets:
> 
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":0  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":2  [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]
> 
> but all 3 cases seem to produce top-first interlaced frames.
> The CCIR_CODE_* register dump shows no differences:
> 2a38014: 010D07DF 00040596 00FF
> 
> ...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
> registers depending on the height of the image.

Exactly.

>  Hovewer, I'm using 480
> lines here, so it should be B-T instead.

My understanding is that the CCIR codes for height == 480 (NTSC)
currently capture the second field (top) first, assuming that for NTSC
the EAV/SAV codes are bottom-field-first.

So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
field that is captured first, where F=1 is the field that is marked as
second field on the wire, so top. Which means that the captured frame
has two fields captured across frame boundaries, which might be
problematic if the source data was originally progressive.

>  My guess is the CSI is skipping
> the first incomplete line (half-line - the first visible line has full
> length) and BT becomes TB.

That wouldn't make BT TB though, if we'd still capture the bottom field
(minus its first half line) first?

> It seems writing to the CCIR_CODE_[12] registers does the trick, though
> (the captured frames aren't correct and have the lines swapped in pairs
> because t/b field pointers aren't correctly set).

What are you writing exactly? 0x01040596 to CCIR_CODE_1 and 0x000d07df
to CCIR_CODE_2? That is what I would expect to capture SEQ_BT for NTSC
data, and the IPU could interweave this into INTERLACED_BT, correctly if
we fix ipu_cpmem_interlaced_scan to allow negative offsets.

regards
Philipp


Re: i.MX6 IPU CSI analog video input on Ventana

2018-06-01 Thread Krzysztof Hałasa
Steve Longerbeam  writes:

> I tend to agree, I've found conflicting info out there regarding
> PAL vs. NTSC field order. And I've never liked having to guess
> at input analog standard based on input # lines. I will go ahead
> and remove the field order override code.

I've merged your current fix-csi-interlaced.2 branch (2018-06-01
00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
"media: adv7180: fix field type" commit and NTSC camera:

media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"

correctly sets:

"adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":0  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":2  [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]

but all 3 cases seem to produce top-first interlaced frames.
The CCIR_CODE_* register dump shows no differences:
2a38014: 010D07DF 00040596 00FF

...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
registers depending on the height of the image. Hovewer, I'm using 480
lines here, so it should be B-T instead. My guess is the CSI is skipping
the first incomplete line (half-line - the first visible line has full
length) and BT becomes TB.

It seems writing to the CCIR_CODE_[12] registers does the trick, though
(the captured frames aren't correct and have the lines swapped in pairs
because t/b field pointers aren't correctly set).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland


Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-06-01 Thread Sakari Ailus
On Fri, Jun 01, 2018 at 05:54:30PM +0800, Bing Bu Cao wrote:
> > > +AKM AK7375 LENS VOICE COIL DRIVER
> > > +M:   Tianshu Qiu 
> > > +L:   linux-media@vger.kernel.org
> > > +T:   git git://linuxtv.org/media_tree.git
> > > +S:   Maintained
> > > +F:   drivers/media/i2c/ak7375.c
> > > +F:   Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
> > The name of the file also needs to match. Currently it doesn't. How about
> > "asahi-kasei,ak7375.txt"?
> Ack.
> Does it make sense if just change the compatible string to
> "asahi-kasei,ak7375" and keep the file name unchanged?

Most binding files in the directory seem to use the chip name rather than
the full compatible string (including the vendor). I guess both are fine.

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


Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-06-01 Thread Bing Bu Cao




On 2018年06月01日 17:34, Sakari Ailus wrote:

Hi Bingbu,

A few comments below.

On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu@intel.com wrote:

From: Bingbu Cao 

Add a V4L2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the I2C bus to control the
focus position.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
  MAINTAINERS|   8 ++
  drivers/media/i2c/Kconfig  |  10 ++
  drivers/media/i2c/Makefile |   1 +
  drivers/media/i2c/ak7375.c | 278 +
  4 files changed, 297 insertions(+)
  create mode 100644 drivers/media/i2c/ak7375.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea362219c4aa..20379a7baca0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git
  S:Maintained
  F:drivers/media/usb/airspy/
  
+AKM AK7375 LENS VOICE COIL DRIVER

+M: Tianshu Qiu 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ak7375.c
+F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt

The name of the file also needs to match. Currently it doesn't. How about
"asahi-kasei,ak7375.txt"?

Ack.
Does it make sense if just change the compatible string to 
"asahi-kasei,ak7375" and keep the file name unchanged?

I have no clear idea about the DT binding rules.


Could you also move the MAINTAINERS entry to the patch adding the DT
bindings?

Ack.



+
  ALACRITECH GIGABIT ETHERNET DRIVER
  M:Lino Sanfilippo 
  S:Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..ff3cb5afb0e1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,16 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
  
+config VIDEO_AK7375

+   tristate "AK7375 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   help
+ This is a driver for the AK7375 camera lens voice coil.
+ AK7375 is a 12 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
  config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..05b97e319ea9 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
  obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index ..012e673e9ced
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c

...


+static const struct of_device_id ak7375_of_table[] = {
+   { .compatible = "akm,ak7375" },

"asahi-kasei,ak7375"





Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-06-01 Thread Sakari Ailus
On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu@intel.com wrote:
> +static int ak7375_i2c_write(struct ak7375_device *ak7375,
> + u8 addr, u16 data, int size)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>sd);
> + int ret;
> + u8 buf[3];
> +
> + if (size != 1 && size != 2)
> + return -EINVAL;
> + buf[0] = addr;
> + buf[2] = data & 0xff;
> + if (size == 2)
> + buf[1] = data >> 8;
> + ret = i2c_master_send(client, (const char *)buf, size + 1);

I don't have a data datasheet for this thing, but it looks like buf[1] will
be undefined for writes the size of which is 1. And this what appears to be
written to the device as well...

> + if (ret < 0)
> + return ret;
> + if (ret != size + 1)
> + return -EIO;
> + return 0;
> +}

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


Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-06-01 Thread Sakari Ailus
Hi Bingbu,

A few comments below.

On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu@intel.com wrote:
> From: Bingbu Cao 
> 
> Add a V4L2 sub-device driver for the ak7375 lens voice coil.
> This is a voice coil module using the I2C bus to control the
> focus position.
> 
> Signed-off-by: Tianshu Qiu 
> Signed-off-by: Bingbu Cao 
> ---
>  MAINTAINERS|   8 ++
>  drivers/media/i2c/Kconfig  |  10 ++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ak7375.c | 278 
> +
>  4 files changed, 297 insertions(+)
>  create mode 100644 drivers/media/i2c/ak7375.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea362219c4aa..20379a7baca0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -625,6 +625,14 @@ T:   git git://linuxtv.org/anttip/media_tree.git
>  S:   Maintained
>  F:   drivers/media/usb/airspy/
>  
> +AKM AK7375 LENS VOICE COIL DRIVER
> +M:   Tianshu Qiu 
> +L:   linux-media@vger.kernel.org
> +T:   git git://linuxtv.org/media_tree.git
> +S:   Maintained
> +F:   drivers/media/i2c/ak7375.c
> +F:   Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt

The name of the file also needs to match. Currently it doesn't. How about
"asahi-kasei,ak7375.txt"?

Could you also move the MAINTAINERS entry to the patch adding the DT
bindings?

> +
>  ALACRITECH GIGABIT ETHERNET DRIVER
>  M:   Lino Sanfilippo 
>  S:   Maintained
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 341452fe98df..ff3cb5afb0e1 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -326,6 +326,16 @@ config VIDEO_AD5820
> This is a driver for the AD5820 camera lens voice coil.
> It is used for example in Nokia N900 (RX-51).
>  
> +config VIDEO_AK7375
> + tristate "AK7375 lens voice coil support"
> + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> + depends on VIDEO_V4L2_SUBDEV_API
> + help
> +   This is a driver for the AK7375 camera lens voice coil.
> +   AK7375 is a 12 bit DAC with 120mA output current sink
> +   capability. This is designed for linear control of
> +   voice coil motors, controlled via I2C serial interface.
> +
>  config VIDEO_DW9714
>   tristate "DW9714 lens voice coil support"
>   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index d679d57cd3b3..05b97e319ea9 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
>  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
>  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
> +obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
>  obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
>  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> new file mode 100644
> index ..012e673e9ced
> --- /dev/null
> +++ b/drivers/media/i2c/ak7375.c

...

> +static const struct of_device_id ak7375_of_table[] = {
> + { .compatible = "akm,ak7375" },

"asahi-kasei,ak7375"

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


[PATCH v3 2/2] media: dw9807: Add dw9807 vcm driver

2018-06-01 Thread Sakari Ailus
From: Alan Chiang 

DW9807 is a 10 bit DAC from Dongwoon, designed for linear
control of voice coil motor.

This driver creates a V4L2 subdevice and
provides control to set the desired focus.

Signed-off-by: Alan Chiang 
Signed-off-by: Andy Yeh 
Reviewed-by: Jacopo Mondi 
Reviewed-by: Tomasz Figa 
Signed-off-by: Sakari Ailus 
---
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 329 +
 4 files changed, 347 insertions(+)
 create mode 100644 drivers/media/i2c/dw9807.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a38e24a3702e..c4dc20162c8b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4382,6 +4382,13 @@ T:   git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/i2c/dw9714.c
 
+DONGWOON DW9807 LENS VOICE COIL DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/dw9807.c
+
 DOUBLETALK DRIVER
 M: "James R. Van Zandt" 
 L: blinux-l...@redhat.com
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..1f9d7c6aa31a 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -336,6 +336,16 @@ config VIDEO_DW9714
  capability. This is designed for linear control of
  voice coil motors, controlled via I2C serial interface.
 
+config VIDEO_DW9807
+   tristate "DW9807 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   ---help---
+ This is a driver for the DW9807 camera lens voice coil.
+ DW9807 is a 10 bit DAC with 100mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_SAA7110
tristate "Philips SAA7110 video decoder"
depends on VIDEO_V4L2 && I2C
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..16fc34eda5cc 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
+obj-$(CONFIG_VIDEO_DW9807)  += dw9807.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
diff --git a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c
new file mode 100644
index ..8ba3920b6e2f
--- /dev/null
+++ b/drivers/media/i2c/dw9807.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DW9807_MAX_FOCUS_POS   1023
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position.
+ */
+#define DW9807_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define DW9807_CTRL_STEPS  16
+#define DW9807_CTRL_DELAY_US   1000
+
+#define DW9807_CTL_ADDR0x02
+/*
+ * DW9807 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ */
+#define DW9807_MSB_ADDR0x03
+#define DW9807_LSB_ADDR0x04
+#define DW9807_STATUS_ADDR 0x05
+#define DW9807_MODE_ADDR   0x06
+#define DW9807_RESONANCE_ADDR  0x07
+
+#define MAX_RETRY  10
+
+struct dw9807_device {
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   u16 current_val;
+};
+
+static inline struct dw9807_device *sd_to_dw9807_vcm(
+   struct v4l2_subdev *subdev)
+{
+   return container_of(subdev, struct dw9807_device, sd);
+}
+
+static int dw9807_i2c_check(struct i2c_client *client)
+{
+   const char status_addr = DW9807_STATUS_ADDR;
+   char status_result;
+   int ret;
+
+   ret = i2c_master_send(client, _addr, sizeof(status_addr));
+   if (ret < 0) {
+   dev_err(>dev, "I2C write STATUS address fail ret = 
%d\n",
+   ret);
+   return ret;
+   }
+
+   ret = i2c_master_recv(client, _result, sizeof(status_result));
+   if (ret < 0) {
+   dev_err(>dev, "I2C read STATUS value fail ret = %d\n",
+   ret);
+   return ret;
+   }
+
+   return status_result;
+}
+
+static int dw9807_set_dac(struct i2c_client *client, u16 data)
+{
+   const char tx_data[3] = {
+   DW9807_MSB_ADDR, ((data >> 8) & 0x03), (data & 0xff)
+   };
+   int 

[PATCH v3 1/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

2018-06-01 Thread Sakari Ailus
From: Alan Chiang 

Dongwoon DW9807 is a voice coil lens driver.

Signed-off-by: Alan Chiang 
Signed-off-by: Andy Yeh 
Reviewed-by: Rob Herring 
Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +
 1 file changed, 9 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
new file mode 100644
index ..c4701f1eaaf6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
@@ -0,0 +1,9 @@
+Dongwoon Anatech DW9807 voice coil lens driver
+
+DW9807 is a 10-bit DAC with current sink capability. It is intended for
+controlling voice coil lenses.
+
+Mandatory properties:
+
+- compatible: "dongwoon,dw9807-vcm"
+- reg: I2C slave address
-- 
2.11.0



[PATCH v3 0/2] Dongwoon dw9807

2018-06-01 Thread Sakari Ailus
Just posting this after squashing the vcm-only bit to the compatible string.

Alan Chiang (2):
  media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil
  media: dw9807: Add dw9807 vcm driver

 .../bindings/media/i2c/dongwoon,dw9807.txt |   9 +
 MAINTAINERS|   7 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9807.c | 329 +
 5 files changed, 356 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
 create mode 100644 drivers/media/i2c/dw9807.c

-- 
2.11.0



Re: [ANN] Meeting to discuss improvements to support MC-based cameras on generic apps

2018-06-01 Thread Javier Martinez Canillas
On Thu, May 31, 2018 at 4:19 PM, Hans Verkuil  wrote:
> On 05/31/18 15:58, Hans Verkuil wrote:
>> On 05/31/18 15:22, Mauro Carvalho Chehab wrote:
>>> Em Mon, 28 May 2018 10:43:51 -0300
>>> Mauro Carvalho Chehab  escreveu:
>>>
 Em Thu, 17 May 2018 16:07:08 -0300
 Mauro Carvalho Chehab  escreveu:

> Hi all,
>
> The goal of this e-mail is to schedule a meeting in order to discuss
> improvements at the media subsystem in order to support complex camera
> hardware by usual apps.
>
> The main focus here is to allow supporting devices with MC-based
> hardware connected to a camera.
>
> In short, my proposal is to meet with the interested parties on solving
> this issue during the Open Source Summit in Japan, e. g. between
> June, 19-22, in Tokyo.

 Let's schedule the meeting to happen in Tokyo, Japan at June, 19.

 Location yet to be defined, but it will either be together with
 OSS Japan or at Google. I'll confirm the address tomorrow.
>>>
>>> More details about the meeting:
>>>
>>> Date: June, 19
>>> Site: Google
>>> Address: 〒106-6126 Tokyo, Minato, Roppongi, 6 Chome−10−1 Roppongi Hills 
>>> Mori Tower 44F
>>>
>>> Please confirm who will be attending the meeting.
>>
>> I plan to attend the meeting via Google Hangouts.
>
> Well, the afternoon part of the meeting at least :-)
>

I also plan to attend via Google Hangouts. At least the hours that are
reasonable on my timezone.

Best regards,
Javier