cron job: media_tree daily build: OK
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
>> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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