Re: [PATCH] media: coda: remove definition of CODA_STD_MJPG
On Wed, 2017-11-08 at 19:58 +0100, Martin Kepplinger wrote: > According to i.MX VPU API Reference Manuals the MJPG video codec is > refernced to by number 7, not 3. > > Also Philipp pointed out that this value is only meant to fill in > CMD_ENC_SEQ_COD_STD for encoding, only on i.MX53. It was never written > to any register, and even if defined correctly, wouldn't be needed > for i.MX6. > > So avoid confusion and remove this definition. > > Signed-off-by: Martin Kepplinger > --- > drivers/media/platform/coda/coda_regs.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/media/platform/coda/coda_regs.h > b/drivers/media/platform/coda/coda_regs.h > index 38df5fd9a2fa..35e620c7f1f4 100644 > --- a/drivers/media/platform/coda/coda_regs.h > +++ b/drivers/media/platform/coda/coda_regs.h > @@ -254,7 +254,6 @@ > #define CODA9_STD_H264 0 > #define CODA_STD_H263 1 > #define CODA_STD_H264 2 > -#define CODA_STD_MJPG 3 > #define CODA9_STD_MPEG4 3 > > #define CODA_CMD_ENC_SEQ_SRC_SIZE0x190 Thanks, Acked-by: Philipp Zabel regards Philipp
Re: [PATCH] media: coda: Fix definition of CODA_STD_MJPG
Hi Martin, thank you for the patch. I'd prefer to just drop CODA_STD_MJPG altogether, to avoid confusion. Explanation below: On Wed, 2017-11-08 at 15:12 +0100, Martin Kepplinger wrote: > According to i.MX 6 VPU API Reference Manual Rev. L3.0.35_1.1.0, 01/2013 > chapter 3.2.1.5, the MJPG video codec is refernced to by number 7, not 3. > So change this accordingly. > > This isn't yet being used right now and therefore probably hasn't been > noticed. Fixing this avoids causing trouble in the future. > > Signed-off-by: Martin Kepplinger > --- > drivers/media/platform/coda/coda_regs.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/coda/coda_regs.h > b/drivers/media/platform/coda/coda_regs.h > index 38df5fd9a2fa..8d726faaf86e 100644 > --- a/drivers/media/platform/coda/coda_regs.h > +++ b/drivers/media/platform/coda/coda_regs.h > @@ -254,7 +254,7 @@ > #define CODA9_STD_H264 0 > #define CODA_STD_H263 1 > #define CODA_STD_H264 2 > -#define CODA_STD_MJPG 3 > +#define CODA_STD_MJPG 7 These are only ever used to feed them into the CMD_ENC_SEQ_COD_STD register, and only for MPEG4, H263 (which we don't support), and H264. On i.MX53 the correct value was 3 once, but it was only used in the userspace library, it was never written to any register. On i.MX6 JPEG encoding can be implemented without going through the BIT processor at all. regards Philipp
Re: [PATCH 1/7] media: atomisp: fix ident for assert/return
Hi Mauro,
On Tue, 2017-10-31 at 12:04 -0400, Mauro Carvalho Chehab wrote:
> On lots of places, assert/return are starting at the first
> column, causing indentation issues, as complained by spatch:
>
> drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/irq_private.h:32
> irq_reg_store() warn: inconsistent indenting
>
> Used this small script to fix such occurrences:
>
> for i in $(git grep -l -E "^(assert|return)" drivers/staging/media/); do perl
> -ne 's/^(assert|return)/\t$1/; print $_' <$i >a && mv a $i; done
This also catches labels that start with "return". Adding some
whitespace to the regular expression may avoid these false positives.
> Signed-off-by: Mauro Carvalho Chehab
> ---
> .../pci/atomisp2/css2400/camera/util/src/util.c| 2 +-
> .../hive_isp_css_common/host/event_fifo_private.h | 2 +-
> .../host/fifo_monitor_private.h| 28 +-
> .../css2400/hive_isp_css_common/host/gdc.c | 8 +--
> .../css2400/hive_isp_css_common/host/gp_device.c | 2 +-
> .../hive_isp_css_common/host/gp_device_private.h | 16 +++---
> .../hive_isp_css_common/host/gpio_private.h| 4 +-
> .../hive_isp_css_common/host/hmem_private.h| 4 +-
> .../host/input_formatter_private.h | 16 +++---
> .../hive_isp_css_common/host/input_system.c| 28 +-
> .../host/input_system_private.h| 64
> +++---
> .../css2400/hive_isp_css_common/host/irq.c | 30 +-
> .../css2400/hive_isp_css_common/host/irq_private.h | 12 ++--
> .../css2400/hive_isp_css_common/host/isp.c | 4 +-
> .../css2400/hive_isp_css_common/host/mmu.c | 6 +-
> .../css2400/hive_isp_css_common/host/mmu_private.h | 12 ++--
> .../css2400/hive_isp_css_common/host/sp_private.h | 60 ++--
> .../atomisp/pci/atomisp2/css2400/sh_css_hrt.c | 2 +-
> drivers/staging/media/imx/imx-media-capture.c | 2 +-
[...]
> diff --git a/drivers/staging/media/imx/imx-media-capture.c
> b/drivers/staging/media/imx/imx-media-capture.c
> index ea145bafb880..149f0e1753a1 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -463,7 +463,7 @@ static int capture_start_streaming(struct vb2_queue *vq,
> unsigned int count)
>
> return 0;
>
> -return_bufs:
> + return_bufs:
> spin_lock_irqsave(&priv->q_lock, flags);
> list_for_each_entry_safe(buf, tmp, &priv->ready_q, list) {
> list_del(&buf->list);
This label should stay at the first column.
regards
Philipp
Re: [PATCH] imx-csi: fix burst size
On Fri, 2017-09-29 at 22:41 +0100, Russell King wrote: > Setting a burst size of "8" doesn't work for IMX219 with 8-bit bayer, > but a burst size of "16" does. Fix this. > > Signed-off-by: Russell King Oh, well, this isn't for me to apply after all. Acked-by: Philipp Zabel regards Philipp > --- > drivers/staging/media/imx/imx-media-csi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/imx/imx-media-csi.c > b/drivers/staging/media/imx/imx-media-csi.c > index 6d856118c223..e27bcdb63973 100644 > --- a/drivers/staging/media/imx/imx-media-csi.c > +++ b/drivers/staging/media/imx/imx-media-csi.c > @@ -341,7 +341,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) > case V4L2_PIX_FMT_SGBRG8: > case V4L2_PIX_FMT_SGRBG8: > case V4L2_PIX_FMT_SRGGB8: > - burst_size = 8; > + burst_size = 16; > passthrough = true; > passthrough_bits = 8; > break;
Re: [PATCH][MEDIA] i.MX6: Fix MIPI CSI-2 LP-11 check
On Tue, 2017-10-17 at 08:12 +0200, Krzysztof Hałasa wrote: > Bitmask for the MIPI CSI-2 data PHY status doesn't seem to be correct. > Fix it. > > Signed-off-by: Krzysztof Hałasa > > --- a/drivers/staging/media/imx/imx6-mipi-csi2.c > +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c > @@ -252,8 +252,8 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2) > u32 mask, reg; > int ret; > > - mask = PHY_STOPSTATECLK | > - ((csi2->bus.num_data_lanes - 1) << PHY_STOPSTATEDATA_BIT); > + mask = PHY_STOPSTATECLK | (((1 << csi2->bus.num_data_lanes) - 1) << > +PHY_STOPSTATEDATA_BIT); > > ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg, > (reg & mask) == mask, 0, 50); Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH][MEDIA]i.MX6 CSI: Fix MIPI camera operation in RAW/Bayer mode
On Tue, 2017-10-17 at 14:53 +0100, Russell King - ARM Linux wrote: > On Tue, Oct 17, 2017 at 03:26:19PM +0200, Krzysztof Hałasa wrote: > > Fabio Estevam writes: > > > > > > --- a/drivers/staging/media/imx/imx-media-csi.c > > > > +++ b/drivers/staging/media/imx/imx-media-csi.c > > > > @@ -340,7 +340,7 @@ static int csi_idmac_setup_channel(struct csi_priv > > > > *priv) > > > > case V4L2_PIX_FMT_SGBRG8: > > > > case V4L2_PIX_FMT_SGRBG8: > > > > case V4L2_PIX_FMT_SRGGB8: > > > > - burst_size = 8; > > > > + burst_size = 16; > > > > passthrough = true; > > > > passthrough_bits = 8; > > > > break; > > > > > > Russell has sent the same fix and Philipp made a comment about the > > > possibility of using 32-byte or 64-byte bursts: > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2017-October/111960.html > > > > Great. > > > > Sometimes I wonder how many people are working on exactly the same > > stuff. > > I do wish the patch was merged (which fixes a real problem) rather than > hanging around for optimisation questions. We can always increase it > in the future if it's deemed that a larger burst size is beneficial. I am sorry, that patch should have been part of last week's pull request. I'll send another one for -rc6. regards Philipp
[PATCH v2] [media] tc358743: validate lane count
The TC358743 does not support more than 4 data lanes. Check that the
lane count is valid.
Signed-off-by: Philipp Zabel
---
v2: drop lane order check, as suggested by Sakari
---
drivers/media/i2c/tc358743.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a9355032076f9..8bfb3f56502b7 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1941,6 +1941,11 @@ static int tc358743_probe_of(struct tc358743_state
*state)
goto free_endpoint;
}
+ if (endpoint->bus.mipi_csi2.num_data_lanes > 4) {
+ dev_err(dev, "invalid number of lanes\n");
+ goto free_endpoint;
+ }
+
state->bus = endpoint->bus.mipi_csi2;
ret = clk_prepare_enable(refclk);
--
2.11.0
Re: IMX CSI max pixel rate
On Fri, 2017-10-06 at 10:49 +0300, Sakari Ailus wrote: > On Thu, Oct 05, 2017 at 10:21:16AM -0700, Tim Harvey wrote: > > Greetings, > > > > I'm working on a HDMI receiver driver for the TDA1997x > > (https://lwn.net/Articles/734692/) and wanted to throw an error if > > the > > detected input resolution/vidout-output-bus-format exceeded the max > > pixel rate of the SoC capture bus the chip connects to (in my case > > is > > the IMX6 CSI which has a limit of 180MP/sec). Where does this number come from? I see there are 180 MP/s advertised for burst mode still image capture (with up to 35% blanking overhead) in the i.MX6Q reference manual. For continuous still image capture, only 160 MP/s are advertised. The CSI really supports up to about 240 MHz pixel clock (assuming the IPU is clocked at 264 MHz), so MP/s for video really depends a lot on the blanking. In the IPU display ports chapter, two different numbers are given for CSI-2 input, though: 200 MHz for 4 data lanes, and 250 MHz 2 data lanes. For 1-3 data lanes, the limiting factor is the maximum CSI-2 link frequency, at up to 500 MHz (1000 Mbps/lane). > > Any recommendations on where a dt property should live, its naming, > > and location/naming and functions to validate the pixel rate or is > > there even any interest in this sort of check? > > Why a DT property? > > We do have V4L2_CID_PIXEL_RATE, would that be applicable for this? Isn't this is meant to return the currently set pixel rate at the input of a single subdevice, not the total maximum pixel rate supported by its input? regards Philipp
Re: platform: coda: how to use firmware-imx binary releases?
On Wed, 2017-10-04 at 10:44 +0200, Martin Kepplinger wrote:
> Hi,
>
> Commit
>
> be7f1ab26f42 media: coda: mark CODA960 firmware versions 2.3.10
> and
> 3.1.1 as supported
>
> says firmware version 3.1.1 revision 46072 is contained in
> "firmware-imx-5.4.bin", that's probably
>
> sha1 78a416ae88ff01420260205ce1d567f60af6847e firmware-imx-
> 5.4.bin
>
> How do I use this in order to get a VPU firmware blob that the coda
> platform driver can work with?
>
>
>
> (Maybe it'd be worth adding some short documentation on this. There
> doesn't seem to be a devicetree bindings doc for coda in
> Documentation/devicetree/bindings/media
I was mistaken, Documentation/devicetree/bindings/media/coda.txt exists.
It was added in commit 657eee7d25fb ("media: coda: use genalloc API").
regards
Philipp
Re: i.MX6 CODA warning: vb2: counters for queue xxx, buffer y: UNBALANCED!
On Thu, 2017-10-05 at 12:31 +0200, Krzysztof Hałasa wrote: > Hi, > > I'm using i.MX6 CODA H.264 encoder and found a minor bug somewhere. > Not sure how it should be fixed, though. > The problem manifests itself when I configure (open, qbuf etc) the > encoder device and then close it without any start/stop streaming > calls. > I'm using 2 buffers in this example: > > vb2: counters for queue b699c808, buffer 0: UNBALANCED! > vb2: buf_init: 1 buf_cleanup: 1 buf_prepare: 1 buf_finish: 1 > vb2: buf_queue: 0 buf_done: 0 > vb2: alloc: 1 put: 1 prepare: 1 finish: 0 mmap: 1 > > vb2: get_userptr: 0 put_userptr: 0 > vb2: attach_dmabuf: 0 detach_dmabuf: 0 map_dmabuf: 0 unmap_dmabuf: > 0 > vb2: get_dmabuf: 0 num_users: 0 vaddr: 0 cookie: 0 > > vb2: counters for queue b699c808, buffer 1: UNBALANCED! > vb2: buf_init: 1 buf_cleanup: 1 buf_prepare: 1 buf_finish: 1 > vb2: buf_queue: 0 buf_done: 0 > vb2: alloc: 1 put: 1 prepare: 1 finish: 0 mmap: 1 > > vb2: get_userptr: 0 put_userptr: 0 > vb2: attach_dmabuf: 0 detach_dmabuf: 0 map_dmabuf: 0 unmap_dmabuf: > 0 > vb2: get_dmabuf: 0 num_users: 0 vaddr: 0 cookie: 0 > > These are H.264 (encoder "capture") buffers. Note the alloc > prepare/finish disparity. > > I have investigated a bit and it seems it's some missing *buf_done() > call, probably belonging to coda_release(), but I'm not sure. Or maybe > should my program "finish" the buffers before doing close()? I have to admit, I'm a bit unsure how this should be handled. If the buffers are queued via VIDIOC_QBUF, but VIDIOC_STREAMON is never called, the buffers are (mem_)prepared in __buf_prepare, but they are never enqueued in the driver via __enqueue_in_driver. This is reflected by the balanced buf_queue/done line: vb2: buf_queue: 0 buf_done: 0 I can't call v4l2_m2m_buf_done on buffers that have not been queued in the driver yet: these buffers are in state VB2_BUF_STATE_QUEUED, not VB2_BUF_STATE_ACTIVE. But vb2_buffer_done, which is the only place that (mem_)finishes the buffers, only operates on active buffers. The kerneldoc comments for vb2_mem_ops indicate that prepare/finish are called whenever the buffer is passed from userspace to the driver and back, but they make no mention of buffers being passed from userspace to the videobuf2 core, which doesn't pass them on to the driver until STREAMON. regards Philipp
Re: platform: coda: how to use firmware-imx binary releases?
Hi Martin, On Thu, 2017-10-05 at 09:43 +0200, Martin Kepplinger wrote: > I'm running a little off-topic here, but with the newest firmware too, > my > coda driver says "Video Data Order Adapter: Disabled" when started > by video playback via v4l2. This message is most likely just a result of the VDOA not supporting the selected capture format. In vdoa_context_configure, you can see that the VDOA only writes YUYV or NV12. > (imx6, running linux 4.14-rc3, imx-vdoa is probed and never removed, > a dev_info "probed" would maybe be useful for others too?) > > It supsequently fails with > > cma: cma_alloc: alloc failed, req-size: 178 pages, ret: -12 That is -ENOMEM. Is CMA enabled and sufficiently large? For example, CONFIG_CMA=y CONFIG_CMA_DEBUGFS=y CONFIG_DMA_CMA=y CONFIG_CMA_SIZE_MBYTES=256 CONFIG_CMA_SIZE_SEL_MBYTES=y > which may or may not be related to having the vdoa (is it?), but > shouldn't the VDOA module be active by default? > > # cat /sys/module/coda/parameters/disable_vdoa > 0 I think it is not related to VDOA at all. Yes, by default the VDOA should be activated automatically for any supported format. regards Philipp
Re: platform: coda: how to use firmware-imx binary releases?
Hi Martin, On Wed, 2017-10-04 at 10:44 +0200, Martin Kepplinger wrote: > Hi, > > Commit > > be7f1ab26f42 media: coda: mark CODA960 firmware versions 2.3.10 and > 3.1.1 as supported > > says firmware version 3.1.1 revision 46072 is contained in > "firmware-imx-5.4.bin", that's probably > > sha1 78a416ae88ff01420260205ce1d567f60af6847e firmware-imx-5.4.bin Yes. > How do I use this in order to get a VPU firmware blob that the coda > platform driver can work with? These are self-extracting shell scripts with an attached compressed tar archive. This particular file can be extracted by skipping the first 34087 bytes: dd if=firmware-imx-5.4.bin bs=34087 skip=1 | tar xjv > (Maybe it'd be worth adding some short documentation on this. There > doesn't seem to be a devicetree bindings doc for coda in > Documentation/devicetree/bindings/media which would > be a good place for documenting how to use these binaries too) Thank you for pointing this out, the device tree binding docs for coda are indeed missing. I'm not sure the device tree binding docs are the right place to document driver and firmware though. For that, adding a coda.rst entry to Documentation/media/v4l-drivers would probably be a better place. regards Philipp
Re: [PATCH] imx-csi: fix burst size
Hi Russell, On Fri, 2017-09-29 at 22:41 +0100, Russell King wrote: > Setting a burst size of "8" doesn't work for IMX219 with 8-bit bayer, > but a burst size of "16" does. Fix this. Do larger bursts work as well, if the width is divisible by the burst length? Since the Bayer format can't pass through the IC direct path anyway, 32-byte or 64-byte bursts may be possible. > Signed-off-by: Russell King > --- > drivers/staging/media/imx/imx-media-csi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/imx/imx-media-csi.c > b/drivers/staging/media/imx/imx-media-csi.c > index 6d856118c223..e27bcdb63973 100644 > --- a/drivers/staging/media/imx/imx-media-csi.c > +++ b/drivers/staging/media/imx/imx-media-csi.c > @@ -341,7 +341,7 @@ static int csi_idmac_setup_channel(struct csi_priv > *priv) > case V4L2_PIX_FMT_SGBRG8: > case V4L2_PIX_FMT_SGRBG8: > case V4L2_PIX_FMT_SRGGB8: > - burst_size = 8; > + burst_size = 16; > passthrough = true; > passthrough_bits = 8; > break; regards Philipp
[PATCH] [media] tc358743: set entity function to video interface bridge
The TC358743 is an HDMI to MIPI CSI2-2 bridge. Signed-off-by: Philipp Zabel --- drivers/media/i2c/tc358743.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index b7285e45b908a..82927ef0cd913 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1947,6 +1947,7 @@ static int tc358743_probe(struct i2c_client *client, } state->pad.flags = MEDIA_PAD_FL_SOURCE; + sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; err = media_entity_pads_init(&sd->entity, 1, &state->pad); if (err < 0) goto err_hdl; -- 2.11.0
[PATCH] [media] tc358743: validate lane count and order
The TC358743 does not support reordering lanes, or more than 4 data
lanes.
Signed-off-by: Philipp Zabel
---
drivers/media/i2c/tc358743.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a35043cefe128..b7285e45b908a 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1743,6 +1743,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
struct clk *refclk;
u32 bps_pr_lane;
int ret = -EINVAL;
+ int i;
refclk = devm_clk_get(dev, "refclk");
if (IS_ERR(refclk)) {
@@ -1771,6 +1772,21 @@ static int tc358743_probe_of(struct tc358743_state
*state)
goto free_endpoint;
}
+ if (endpoint->bus.mipi_csi2.num_data_lanes > 4) {
+ dev_err(dev, "invalid number of lanes\n");
+ goto free_endpoint;
+ }
+
+ for (i = 0; i < endpoint->bus.mipi_csi2.num_data_lanes; i++) {
+ if (endpoint->bus.mipi_csi2.data_lanes[i] != i + 1)
+ break;
+ }
+ if (i != endpoint->bus.mipi_csi2.num_data_lanes ||
+ endpoint->bus.mipi_csi2.clock_lane != 0) {
+ dev_err(dev, "invalid lane order\n");
+ goto free_endpoint;
+ }
+
state->bus = endpoint->bus.mipi_csi2;
ret = clk_prepare_enable(refclk);
--
2.11.0
[PATCH v2 2/2] [media] imx: ask source subdevice for number of active data lanes
Temporarily use g_mbus_config() to determine the number of active data
lanes used by the transmitter. If g_mbus_config is not supported or
does not return the number of active lines, default to using all
connected data lines.
Signed-off-by: Philipp Zabel
---
New in v2:
- Use the active lanes reported via g_mbus_config(), if available, to
configure the CSI2_N_LANES register correctly.
---
drivers/staging/media/imx/imx6-mipi-csi2.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 5061f3f524fd5..cd19730d0159c 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -135,10 +135,8 @@ static void csi2_enable(struct csi2_dev *csi2, bool enable)
}
}
-static void csi2_set_lanes(struct csi2_dev *csi2)
+static void csi2_set_lanes(struct csi2_dev *csi2, int lanes)
{
- int lanes = csi2->bus.num_data_lanes;
-
writel(lanes - 1, csi2->base + CSI2_N_LANES);
}
@@ -301,6 +299,9 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
static int csi2_start(struct csi2_dev *csi2)
{
+ const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+ struct v4l2_mbus_config cfg;
+ int lanes = 0;
int ret;
ret = clk_prepare_enable(csi2->pix_clk);
@@ -316,7 +317,10 @@ static int csi2_start(struct csi2_dev *csi2)
goto err_disable_clk;
/* Step 4 */
- csi2_set_lanes(csi2);
+ ret = v4l2_subdev_call(csi2->src_sd, video, g_mbus_config, &cfg);
+ if (ret == 0)
+ lanes = (cfg.flags & mask) >> __ffs(mask);
+ csi2_set_lanes(csi2, lanes ?: csi2->bus.num_data_lanes);
csi2_enable(csi2, true);
/* Step 5 */
--
2.11.0
[PATCH v2 1/2] [media] tc358743: fix connected/active CSI-2 lane reporting
g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the TC358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported in pdata mode.
In device tree mode, do not report lane count and clock mode at all, as
the receiver driver can determine these from the device tree.
To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, to be
used only until a better solution is found.
Signed-off-by: Philipp Zabel
---
Changes since v1:
- Check csi_lanes_in_use <= num_data_lanes before writing to cfg.
- Increase size of lane mask to 4 bits and always explicitly report
number of lanes in use.
- Clear clock and connected lane flags in DT mode.
---
drivers/media/i2c/tc358743.c | 30 --
include/media/v4l2-mediabus.h | 8
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..a35043cefe128 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1458,28 +1458,29 @@ static int tc358743_g_mbus_config(struct v4l2_subdev
*sd,
struct v4l2_mbus_config *cfg)
{
struct tc358743_state *state = to_state(sd);
+ const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+ if (state->csi_lanes_in_use > state->bus.num_data_lanes)
+ return -EINVAL;
cfg->type = V4L2_MBUS_CSI2;
+ cfg->flags = (state->csi_lanes_in_use << __ffs(mask)) & mask;
- /* Support for non-continuous CSI-2 clock is missing in the driver */
- cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ /* In DT mode, only report the number of active lanes */
+ if (sd->dev->of_node)
+ return 0;
- switch (state->csi_lanes_in_use) {
- case 1:
+ /* Support for non-continuous CSI-2 clock is missing in pdata mode */
+ cfg->flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+
+ if (state->bus.num_data_lanes > 0)
cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
- break;
- case 2:
+ if (state->bus.num_data_lanes > 1)
cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
- break;
- case 3:
+ if (state->bus.num_data_lanes > 2)
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
- break;
- case 4:
+ if (state->bus.num_data_lanes > 3)
cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
- break;
- default:
- return -EINVAL;
- }
return 0;
}
@@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 93f8afcb7a220..fc106c902bf47 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -63,6 +63,14 @@
V4L2_MBUS_CSI2_3_LANE |
V4L2_MBUS_CSI2_4_LANE)
#define V4L2_MBUS_CSI2_CHANNELS(V4L2_MBUS_CSI2_CHANNEL_0 |
V4L2_MBUS_CSI2_CHANNEL_1 | \
V4L2_MBUS_CSI2_CHANNEL_2 |
V4L2_MBUS_CSI2_CHANNEL_3)
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK(0xf << 10)
/**
* enum v4l2_mbus_type - media bus type
--
2.11.0
Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting
Hi Ian, On Thu, 2017-09-21 at 12:06 +0100, Ian Arkver wrote: [...] > > My understanding of Hans' comment: > > "I'd also add a comment that all other flags must be 0 if the device > > tree is used. This to avoid mixing the two." > > > > is that all the above should only happen if (!!state->pdata). > > Except that state->pdata is a copy of the pdata, not a pointer, but you > know what I mean. Some other check for DT needed here. Yes, I'll change this to zero all V4L2_MBUS_CSI2_[1-4]_LANE in the DT case. I suppose the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK and V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK bits should be zeroed as well, then? > > I don't know if this would break any existing DT-using bridge drivers. The only current users of g_mbus_config are the pxa_camera and sh_mobile_ceu_camera soc_camera drivers. Neither supports MIPI CSI-2, as far as I can tell. regards Philipp
Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting
On Thu, 2017-09-21 at 12:41 +0100, Dave Stevenson wrote: > Hi Philipp > > On 21 September 2017 at 11:24, Philipp Zabel > wrote: [...] > > + if (state->csi_lanes_in_use > 4) > > One could suggest > if (state->csi_lanes_in_use > state->bus.num_data_lanes) > here. Needing to use more lanes than are configured is surely an > error, although that may be detectable at the other end. See below > too. True. The OF parser could be improved to make sure that num_data_lanes <= 4, and also that all lanes are in the correct order. [...] > > +/* > > + * Number of lanes in use, 0 == use all available lanes (default) > > + * > > + * This is a temporary fix for devices that need to reduce the number of > > active > > + * lanes for certain modes, until g_mbus_config() can be replaced with a > > better > > + * solution. > > + */ > > +#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10) > > I know this was Hans' suggested define, but are we saying 4 lanes is > not a valid value? If it is then the mask needs to be at least (7 << > 10). 0 must map to "all lanes" for backwards compatibility, but I see no reason why we shouldn't add another bit here to support reporting 4 lanes explicitly. > 4 lanes is not necessarily "all available lanes". Correct. > - There are now CSI2 devices supporting up to 8 lanes (although > V4L2_FWNODE_CSI2_MAX_DATA_LANES limits you to 4 at the moment). So how about we just add two bits, then? #define V4L2_MBUS_CSI2_LANE_MASK(0xf << 10) > - Or you could have 2 lanes configured in DT and ask TC358743 for (eg) > 1080P60 UYVY at 594Mbps (needs 4 lanes) which passes the current logic > in the TC358743 driver and would return 0, when it is actually going > to use 4 lanes. That could be classed as a driver bug though. Right, the driver shouldn't try to start streaming at all if it knows that the available CSI-2 bandwidth will be exceeded. > My view is that if a driver is going to report how many lanes to use > then it should always report it explicitly. The default 0 value should > only be used for devices that will never change it from the DT > settings. Perhaps others disagree > > Otherwise the patch works for me. I'm fine with changing this as you suggest. regards Philipp
[PATCH] tc358743: fix connected/active CSI-2 lane reporting
g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the tc358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported.
To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, until
a better solution is found.
Signed-off-by: Philipp Zabel
---
drivers/media/i2c/tc358743.c | 22 --
include/media/v4l2-mediabus.h | 8
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..e2a9e6a18a49d 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1464,21 +1464,22 @@ static int tc358743_g_mbus_config(struct v4l2_subdev
*sd,
/* Support for non-continuous CSI-2 clock is missing in the driver */
cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
- switch (state->csi_lanes_in_use) {
- case 1:
+ if (state->bus.num_data_lanes > 0)
cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
- break;
- case 2:
+ if (state->bus.num_data_lanes > 1)
cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
- break;
- case 3:
+ if (state->bus.num_data_lanes > 2)
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
- break;
- case 4:
+ if (state->bus.num_data_lanes > 3)
cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
- break;
- default:
+
+ if (state->csi_lanes_in_use > 4)
return -EINVAL;
+
+ if (state->csi_lanes_in_use < state->bus.num_data_lanes) {
+ const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+ cfg->flags |= (state->csi_lanes_in_use << __ffs(mask)) & mask;
}
return 0;
@@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 93f8afcb7a220..3467d97be5f5b 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -63,6 +63,14 @@
V4L2_MBUS_CSI2_3_LANE |
V4L2_MBUS_CSI2_4_LANE)
#define V4L2_MBUS_CSI2_CHANNELS(V4L2_MBUS_CSI2_CHANNEL_0 |
V4L2_MBUS_CSI2_CHANNEL_1 | \
V4L2_MBUS_CSI2_CHANNEL_2 |
V4L2_MBUS_CSI2_CHANNEL_3)
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
/**
* enum v4l2_mbus_type - media bus type
--
2.11.0
Re: [PATCH 1/3] [media] tc358743: Correct clock mode reported in g_mbus_config
On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote: > Support for non-continuous clock had previously been added via > device tree, but a comment and the value reported by g_mbus_config > still stated that it wasn't supported. > Remove the comment, and return the correct value in g_mbus_config. > > Signed-off-by: Dave Stevenson > --- > drivers/media/i2c/tc358743.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/tc358743.c > b/drivers/media/i2c/tc358743.c > index e6f5c36..6b0fd07 100644 > --- a/drivers/media/i2c/tc358743.c > +++ b/drivers/media/i2c/tc358743.c > @@ -1461,8 +1461,9 @@ static int tc358743_g_mbus_config(struct > v4l2_subdev *sd, > > cfg->type = V4L2_MBUS_CSI2; > > - /* Support for non-continuous CSI-2 clock is missing in the > driver */ > - cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK; > + cfg->flags = state->bus.flags & > + (V4L2_MBUS_CSI2_CONTINUOUS_CLOCK | > + V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK); Acked-by: Philipp Zabel regards Philipp
Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.
Hi Hans, On Wed, 2017-09-20 at 15:12 +0200, Hans Verkuil wrote: [...] > I don't like it :-) > > Currently g_mbus_config returns (and I quote from v4l2-mediabus.h): "How > many lanes the client can use". I.e. the capabilities of the HW. > > If we are going to use this to communicate how many lines are currently > in use, then I would propose that we add a lane mask, i.e. something like > this: > > /* Number of lanes in use, 0 == use all available lanes (default) */ > #define V4L2_MBUS_CSI2_LANE_MASK(3 << 10) > > And add comments along the lines that this is a temporary fix. > > I would feel a lot happier (or a lot less unhappy) if we'd do it this way. > Rather than re-interpreting bits that are not quite what they should be. > > I'd also add a comment that all other flags must be 0 if the device tree is > used. This to avoid mixing the two. I would like to try this. Currently the driver sets the V4L2_MBUS_CSI2_[1-4]_LANE bits according to csi_lanes_in_use, which is wrong as you say. After moving the csi_lanes_in_use info into a new V4L2_MBUS_CSI2_LANE_MASK bitfield, the V4L2_MBUS_CSI2_[1-4]_LANE bits could be either set to zero or to the really connected lanes as configured in the device tree (csi->bus.num_data_lanes) in the DT case. What would the bits be set to in the pdata case, though? Should a lane count setting be added to tc358743_platform_data with, defaulting to all bits set? regards Philipp
Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.
On Wed, 2017-09-20 at 13:24 +0200, Hans Verkuil wrote: > On 09/20/17 13:00, Dave Stevenson wrote: [...] > > It is communicated over the subdevice API - tc358743_g_mbus_config > > reports back the appropriate number of lanes to the receiver > > subdevice. > > A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call > > as you're starting streaming therefore gives you the correct > > information. That's what I've just done for the BCM283x Unicam > > driver[1], but admittedly I'm not using the media controller API which > > i.MX6 is. > > Shouldn't this information come from the device tree? The g_mbus_config > op is close to being deprecated or even removed. There are currently only > two obscure V4L2 bridge drivers that call it. It dates from pre-DT times > I rather not see it used in a new bridge driver. > > The problem is that contains data that belongs to the DT (hardware > capabilities). Things that can actually change dynamically should be > communicated via another op. We don't have that, so that should be > created. > > I've CC-ed Sakari, he is the specialist for such things. The total number of MIPI CSI-2 lanes (as well as their order) and the list of allowed link frequencies are static and come from the device tree. But the possible combinations of link frequency and number of active lanes out of those for a given resolution and format can vary depending on both transmitter and receiver capabilities. For example, if the DT specifies 4 lanes and both 148.5 MHz and 297 MHz link frequencies, the Toshiba chip could send 720p60 YUYV via 4 lanes at 148.5 MHz, via 2 lanes at 297 MHz, or even via 4 lanes at 297 MHz, with the longer FIFO delay. > > > > [1] http://www.spinics.net/lists/linux-media/msg121813.html, as part > > of the unicam_start_streaming function. > > > > > The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always > > > activates all four lanes that are configured in the device tree. I can > > > work around that with the following patch: > > > > It can't cope running at less than 4 lanes, or it can't cope with a > > change? The hardware can cope with both, although I don't know if there are receivers that can not. In my case this is just about not knowing how many lanes to activate (see below) and which link frequency to choose (currently fixed to the first). [...] > > > [...] 300 is giving a fair safety margin. > > > > > > It does not work for 720p60 on 4 lanes at 594 Mbit/s, as the spreadsheet > > > warns, and testing shows. > > > > If it doesn't work with 720p60, then I guess it has no hope with many > > other resolutions. That would have to be checked on a case by case basis, unfortunately. I have a usecase that only supports 1080p50/60 and 720p50/60. > > It sounds like confirming whether g_mbus_config is a potential > > solution for i.MX6 (sorry I'm not familiar enough with that code to do > > my own quick search), but otherwise cranking it up to 320 is > > reasonable, and I'll see what other numbers fall out of the > > spreadsheet. It seems we need a replacement for g_mbus_config that only returns the dynamic parameters. regards Philipp
Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.
Hi,
On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
> Hi Mauro & Philipp
>
> On 19 September 2017 at 17:49, Mauro Carvalho Chehab
> wrote:
> > Em Tue, 19 Sep 2017 17:24:45 +0200
> > Philipp Zabel escreveu:
> >
> > > Hi Dave,
> > >
> > > On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
> > > > The existing fixed value of 16 worked for UYVY 720P60 over
> > > > 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
> > > > 1080P60 needs 6 lanes at 594MHz).
> > > > It doesn't allow for lower resolutions to work as the FIFO
> > > > underflows.
> > > >
> > > > Using a value of 300 works for all resolutions down to VGA60,
> > > > and the increase in frame delay is <4usecs for 1080P60 UYVY
> > > > (2.55usecs for RGB888).
> > > >
> > > > Signed-off-by: Dave Stevenson
> > >
> > > Can we increase this to 320? This would also allow
> > > 720p60 at 594 Mbps / 4 lanes, according to the xls.
>
> Unless I've missed something then the driver would currently request
> only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
> setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
> on a FIFO setting of 16.
> How/why were you thinking we need to run all four lanes for 720p60
> without other significant driver mods around lane config?
The driver currently silently changes the number of active lanes
depending on required data rate, with no way to communicate it to the
receiver.
The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
activates all four lanes that are configured in the device tree. I can
work around that with the following patch:
--8<--
Subject: [PATCH] [media] tc358743: do not dynamically reduce number of lanes
Dynamic lane number reduction does not work with receivers that
configure a fixed lane number according to the device tree settings.
To allow 720p60 at 594 Mbit/s on 4 lanes, increase the fifo_level
and tclk_trailcnt settings.
Signed-off-by: Philipp Zabel
---
drivers/media/i2c/tc358743.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 64f504542a819..70a9435928cdb 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -683,7 +683,7 @@ static void tc358743_set_csi(struct v4l2_subdev *sd)
{
struct tc358743_state *state = to_state(sd);
struct tc358743_platform_data *pdata = &state->pdata;
- unsigned lanes = tc358743_num_csi_lanes_needed(sd);
+ unsigned lanes = state->bus.num_data_lanes;
v4l2_dbg(3, debug, sd, "%s:\n", __func__);
@@ -1906,7 +1906,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
state->pdata.ddc5v_delay = DDC5V_DELAY_100_MS;
state->pdata.enable_hdcp = false;
/* A FIFO level of 16 should be enough for 2-lane 720p60 at 594 MHz. */
- state->pdata.fifo_level = 16;
+ state->pdata.fifo_level = 320;
/*
* The PLL input clock is obtained by dividing refclk by pll_prd.
* It must be between 6 MHz and 40 MHz, lower frequency is better.
@@ -1948,7 +1948,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
state->pdata.lptxtimecnt = 0x003;
/* tclk-preparecnt: 3, tclk-zerocnt: 20 */
state->pdata.tclk_headercnt = 0x1403;
- state->pdata.tclk_trailcnt = 0x00;
+ state->pdata.tclk_trailcnt = 0x01;
/* ths-preparecnt: 3, ths-zerocnt: 1 */
state->pdata.ths_headercnt = 0x0103;
state->pdata.twakeup = 0x4882;
--
2.11.0
-->8--
Just adding the same heuristic as tc358743_num_csi_lanes_needed in the
imx6-mipi-csi2 driver doesn't work, as the heuristic is specific to the
Toshiba chip. There are MIPI CSI-2 sensors that only support a fixed
number of lanes, for example.
I'd need a way to communicate the number of active MIPI CSI-2 lanes
between transmitting and receiving subdevice driver.
> Once I've got a v3 done on the Unicam driver I'll bash through the
> standard HDMI modes and check what value they need - I can see a big
> spreadsheet coming on.
Oh dear. Unless the point you make below can be resolved, I think we
have no other choice.
> I'll ignore interlaced modes as I can't see any support for it in the
> driver. Receiving the fields on different CSI-2 data types is
> something I know the Unicam hardware won't handle nicely, and I
> suspect it'd be an issue for many other platforms too.
Yes, let's pretend interlacing doesn't exist as long as possible.
> > Hmm... if this is dependent on the resolution and frame rate,
Re: [PATCH v2 1/2] [media] coda: Handle return value of kasprintf
On Wed, 2017-09-20 at 15:10 +0530, Arvind Yadav wrote: > kasprintf() can fail here and we must check its return value. > > Signed-off-by: Arvind Yadav --- > changes in v2 : > Calling coda_free_framebuffers to release already allocated buffers. Thanks, Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 1/2] [media] coda: Handle return value of kasprintf
Hi Arvind, On Wed, 2017-09-20 at 13:07 +0530, Arvind Yadav wrote: > kasprintf() can fail here and we must check its return value. > > Signed-off-by: Arvind Yadav > --- > drivers/media/platform/coda/coda-bit.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/coda/coda-bit.c > b/drivers/media/platform/coda/coda-bit.c > index 291c409..8d78183 100644 > --- a/drivers/media/platform/coda/coda-bit.c > +++ b/drivers/media/platform/coda/coda-bit.c > @@ -417,6 +417,9 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx, > > dev->devtype->product != CODA_DX6) > > size += ysize / 4; > > name = kasprintf(GFP_KERNEL, "fb%d", i); > + if (!name) > + return -ENOMEM; > + Thank you for the patch. Instead of just returning here, this should also call coda_free_framebuffers to release already allocated buffers in earlier iterations of the loop. regards Philipp
Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.
Hi Dave, On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote: > The existing fixed value of 16 worked for UYVY 720P60 over > 2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888 > 1080P60 needs 6 lanes at 594MHz). > It doesn't allow for lower resolutions to work as the FIFO > underflows. > > Using a value of 300 works for all resolutions down to VGA60, > and the increase in frame delay is <4usecs for 1080P60 UYVY > (2.55usecs for RGB888). > > Signed-off-by: Dave Stevenson Can we increase this to 320? This would also allow 720p60 at 594 Mbps / 4 lanes, according to the xls. regards Philipp
Re: Support for direct playback of camera input on imx6q
Hi Stephan, On Wed, 2017-08-23 at 13:25 +0200, Stephan Bauroth wrote: > Dear List, > > I'm trying to directly output a captured video signal to an LVDS > panel > on an imx6q. Capturing frames works fine using the staging imx-media > driver. I can grab JPGs using v4l2grab and I can stream the input to > the > display with gstreamer. However, when streaming, one of the cores is > utilized by ~66%, and I have to use the videoconvert module of > gstreamer. > > The IPU in imx6 can directly stream an captured signal back to the > display processor (DP) according to [1]. While the capturing paths > within the IPU are managed by the staging media-imx driver, its > output > paths are not, so I can not simply set up links using media-ctl. We do not support the direct CSI -> DP path because that requires synchronised clocks between sensor and display. But it is possible to stream buffers through system RAM without much CPU involvement using the dmabuf mechanism. > So, my question is whether it is possible to hook up the captured > signal > to the DP using either the ipuv3 driver or the frame buffer driver > (mxcfb) or something else, and if yes, how? > > Thanks for any help and/or hints > Stephan > > [1] http://www.nxp.com/docs/en/reference-manual/IMX6DQRM.pdf, pg. > 2732 > or chapter 37.1.2.1.4.1, 'Camera Preview' To avoid CPU copies, you'd have to transfer dmabufs from the V4L2 capture device to the DRM output. On current GStreamer this can be achieved with a pipeline like: v4l2src io-mode=dmabuf ! kmssink regards Philipp
Re: [PATCH v2 1/2] docs-rst: media: Document s_stream() video op usage for MC enabled devices
On Mon, 2017-08-21 at 11:01 -0300, Mauro Carvalho Chehab wrote: > Em Mon, 21 Aug 2017 15:52:17 +0200 > Hans Verkuil escreveu: > > > On 08/21/2017 02:07 PM, Mauro Carvalho Chehab wrote: > > > Em Mon, 21 Aug 2017 12:14:18 +0200 > > > Hans Verkuil escreveu: > > > > > > > On 08/21/2017 11:08 AM, Mauro Carvalho Chehab wrote: > > > > > Em Mon, 21 Aug 2017 09:36:49 +0300 > > > > > Sakari Ailus escreveu: > > > > > > > > > > > Hi Mauro, > > > > > > > > > > > > Mauro Carvalho Chehab wrote: > > > > > > > Hi Sakari, > > > > > > > > > > > > > > Em Wed, 16 Aug 2017 15:20:17 +0300 > > > > > > > Sakari Ailus escreveu: > > > > > > > > > > > > > > > As we begin to add support for systems with Media > > > > > > > > controller pipelines > > > > > > > > controlled by more than one device driver, it is > > > > > > > > essential that we > > > > > > > > precisely define the responsibilities of each component > > > > > > > > in the stream > > > > > > > > control and common practices. > > > > > > > > > > > > > > > > Specifically, streaming control is done per sub-device > > > > > > > > and sub-device > > > > > > > > drivers themselves are responsible for streaming setup > > > > > > > > in upstream > > > > > > > > sub-devices. > > > > > > > > > > > > > > IMO, before this patch, we need something like this: > > > > > > > https://patchwork.linuxtv.org/patch/43325/ > > > > > > > > > > > > Thanks. I'll reply separately to that thread. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sakari Ailus > > > > > > > > > > > > > > > > Acked-by: Niklas Söderlund > > > > > > > > > > > > > > > > --- > > > > > > > > Documentation/media/kapi/v4l2-subdev.rst | 29 > > > > > > > > + > > > > > > > > 1 file changed, 29 insertions(+) > > > > > > > > > > > > > > > > diff --git a/Documentation/media/kapi/v4l2-subdev.rst > > > > > > > > b/Documentation/media/kapi/v4l2-subdev.rst > > > > > > > > index e1f0b72..45088ad 100644 > > > > > > > > --- a/Documentation/media/kapi/v4l2-subdev.rst > > > > > > > > +++ b/Documentation/media/kapi/v4l2-subdev.rst > > > > > > > > @@ -262,6 +262,35 @@ is called. After all subdevices > > > > > > > > have been located the .complete() callback is > > > > > > > > called. When a subdevice is removed from the system > > > > > > > > the .unbind() method is > > > > > > > > called. All three callbacks are optional. > > > > > > > > > > > > > > > > +Streaming control on Media controller enabled devices > > > > > > > > +^ > > > > > > > > + > > > > > > > > +Starting and stopping the stream are somewhat complex > > > > > > > > operations that > > > > > > > > +often require walking the media graph to enable > > > > > > > > streaming on > > > > > > > > +sub-devices which the pipeline consists of. This > > > > > > > > involves interaction > > > > > > > > +between multiple drivers, sometimes more than > > > > > > > > two. > > > > > > > > > > > > > > That's still not ok, as it creates a black hole for > > > > > > > devnode-based > > > > > > > devices. > > > > > > > > > > > > > > I would change it to something like: > > > > > > > > > > > > > > Streaming control > > > > > > > ^ > > > > > > > > > > > > > > Starting and stopping the stream are somewhat complex > > > > > > > operations that > > > > > > > often require to enable streaming on sub-devices which > > > > > > > the pipeline > > > > > > > consists of. This involves interaction between multiple > > > > > > > drivers, sometimes > > > > > > > more than two. > > > > > > > > > > > > > > The ``.s_stream()`` op in > > > > > > > :c:type:`v4l2_subdev_video_ops` is responsible > > > > > > > for starting and stopping the stream on the sub-device > > > > > > > it is called > > > > > > > on. > > > > > > > > > > > > > > Streaming control on devnode-centric devices > > > > > > > > > > > > > > > > > > > > > On **devnode-centric** devices, the main driver is > > > > > > > responsible enable > > > > > > > stream all all sub-devices. On most cases, all the main > > > > > > > driver need > > > > > > > to do is to broadcast s_stream to all connected sub- > > > > > > > devices by calling > > > > > > > :c:func:`v4l2_device_call_all`, e. g.:: > > > > > > > > > > > > > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, > > > > > > > s_stream, 1); > > > > > > > > > > > > Looks good to me. > > > > > > > > > > > > > > > > > > > > Streaming control on mc-centric devices > > > > > > > ~~~ > > > > > > > > > > > > > > On **mc-centric** devices, it usually requires walking > > > > > > > the media graph > > > > > > > to enable streaming only at the sub-devices which the > > > > > > > pipeline consists > > > > > > > of. > > > > > > > > > > > > > > (place here the details for such scenario
Re: [PATCH] [media] mx2_emmaprp: Check for platform_get_irq() error
Hi Fabio, On Thu, 2017-08-17 at 18:12 -0300, Fabio Estevam wrote: > From: Fabio Estevam > > platform_get_irq() may fail, so we should better check its return > value and propagate it in the case of error. > > Signed-off-by: Fabio Estevam > --- > drivers/media/platform/mx2_emmaprp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/platform/mx2_emmaprp.c > b/drivers/media/platform/mx2_emmaprp.c > index 03e47e0..f90eaa0 100644 > --- a/drivers/media/platform/mx2_emmaprp.c > +++ b/drivers/media/platform/mx2_emmaprp.c > @@ -942,6 +942,8 @@ static int emmaprp_probe(struct platform_device > *pdev) > platform_set_drvdata(pdev, pcdev); > > irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > ret = devm_request_irq(&pdev->dev, irq, emmaprp_irq, 0, > dev_name(&pdev->dev), pcdev); > if (ret) For IORESOURCE_MEM + devm_ioremap_resource pairs this seems to be a pattern in the kernel, though. Does devm_request_irq reliably return an error when irq is negative? If so, this check would be redundant. regards Philipp
Re: [PATCH] [media] coda/imx-vdoa: Check for platform_get_resource() error
Hi Fabio,
On Wed, 2017-08-16 at 21:14 -0300, Fabio Estevam wrote:
> > From: Fabio Estevam
>
> platform_get_resource() may fail and in this case a NULL dereference
> will occur.
>
> Prevent this from happening by returning an error on
> platform_get_resource() failure.
>
> Fixes: b0444f18e0b18abce ("[media] coda: add i.MX6 VDOA driver")
> > Signed-off-by: Fabio Estevam
> ---
> drivers/media/platform/coda/imx-vdoa.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/coda/imx-vdoa.c
> b/drivers/media/platform/coda/imx-vdoa.c
> index df9b716..8eb3e0c 100644
> --- a/drivers/media/platform/coda/imx-vdoa.c
> +++ b/drivers/media/platform/coda/imx-vdoa.c
> @@ -314,6 +314,8 @@ static int vdoa_probe(struct platform_device *pdev)
> > return PTR_ERR(vdoa->regs);
>
> > res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > + if (!res)
> > + return -EINVAL;
> > vdoa->irq = devm_request_threaded_irq(&pdev->dev, res->start, NULL,
> > vdoa_irq_handler, IRQF_ONESHOT,
> > "vdoa", vdoa);
Thank you for the fix! I think it would be better to replace
platform_get_resource with platform_get_irq. Either way,
Acked-by: Philipp Zabel
regards
Philipp
Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"
Hi,
On Tue, 2016-12-13 at 12:39 -0300, Javier Martinez Canillas wrote:
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
>
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
>
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
>
> > Suggested-by: Laurent Pinchart
> > Signed-off-by: Javier Martinez Canillas
what is the state of this patch? Was it forgotten or was the revert
deemed unnecessary?
regards
Philipp
> ---
>
> Hello Laurent,
>
> I've tested this patch on top of media/master on my IGEPv2 + tvp5150
> with the following:
>
> $ media-ctl -r -l '"tvp5150 1-005c":1->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP
> CCDC":1->"OMAP3 ISP CCDC output":0[1]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":0 [UYVY2X8 720x240
> field:alternate]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":1 [UYVY2X8 720x240
> field:interlaced-tb]'
> $ yavta -f UYVY -s 720x480 -n 1 --field interlaced-tb --capture=1 -F
> /dev/video2
> $ raw2rgbpnm -f UYVY -s 720x480 frame-00.bin frame.pnm
>
> I've also tested the other composite input with the following change:
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 5fe5faefe212..973be68ff78c 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1371,7 +1371,7 @@ static int tvp5150_probe(struct i2c_client *c,
> return res;
>
> core->norm = V4L2_STD_ALL; /* Default is autodetect */
> - core->input = TVP5150_COMPOSITE1;
> + core->input = TVP5150_COMPOSITE0;
> core->enable = true;
>
> v4l2_ctrl_handler_init(&core->hdl, 5);
>
> But as mentioned, it also worked for me without the revert so please let
> me know if the driver works with your omap3 board.
>
> Best regards,
> Javier
>
> drivers/media/i2c/tvp5150.c | 142
>
> 1 file changed, 142 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 48646a7f3fb0..5fe5faefe212 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -42,8 +42,6 @@ struct tvp5150 {
> > struct v4l2_subdev sd;
> #ifdef CONFIG_MEDIA_CONTROLLER
> > struct media_pad pads[DEMOD_NUM_PADS];
> > - struct media_entity input_ent[TVP5150_INPUT_NUM];
> > - struct media_pad input_pad[TVP5150_INPUT_NUM];
> #endif
> > struct v4l2_ctrl_handler hdl;
> > struct v4l2_rect rect;
> @@ -1018,40 +1016,6 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev
> *sd,
> }
>
> /
> > - Media entity ops
> -
> /
> -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -static int tvp5150_link_setup(struct media_entity *entity,
> > - const struct media_pad *local,
> > - const struct media_pad *remote, u32 flags)
> -{
> > - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> > - struct tvp5150 *decoder = to_tvp5150(sd);
> > - int i;
> -
> > - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > - if (remote->entity == &decoder->input_ent[i])
> > - break;
> > - }
> -
> > - /* Do nothing for entities that are not input connectors */
> > - if (i == TVP5150_INPUT_NUM)
> > - return 0;
> -
> > - decoder->input = i;
> -
> > - tvp5150_selmux(sd);
> -
> > - return 0;
> -}
> -
> -static const struct media_entity_operations tvp5150_sd_media_ops = {
> > - .link_setup = tvp5150_link_setup,
> -};
> -#endif
> -
> -/
> > I2C Command
>
> /
>
> @@ -1188,42 +1152,6 @@ static int tvp5150_g_tuner(struct v4l2_subdev *sd,
> struct v4l2_tuner *vt)
> > return 0;
> }
>
> -static int tvp5150_registered(struct v4l2_subdev *sd)
> -{
> -#ifdef CONFIG_MEDIA_CONTROLLER
> > - struct tvp5150 *decoder = to_tvp5150(sd);
> > - int ret = 0;
> > - int i;
> -
> > - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > - struct media_entity *input = &decoder->input_ent[i];
> > - struct media_pad *pad = &decoder->input_pad[i];
> -
> > - if (!input->name)
> > - continue;
> -
> > - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE;
Re: [PATCH 1/3] [media] coda: constify platform_device_id
On Tue, 2017-08-15 at 16:53 +0530, Arvind Yadav wrote: > platform_device_id are not supposed to change at runtime. All > functions > working with platform_device_id provided by > work with const platform_device_id. So mark the non-const structs as > const. > > Signed-off-by: Arvind Yadav Acked-by: Philipp Zabel thanks Philipp
Re: [PATCH] [media] media: imx: use setup_timer
On Sun, 2017-08-13 at 21:39 +0300, Cihangir Akturk wrote: > Use setup_timer function instead of initializing timer with the > function and data fields. > > Generated by: scripts/coccinelle/api/setup_timer.cocci. > > Signed-off-by: Cihangir Akturk > --- > drivers/staging/media/imx/imx-ic-prpencvf.c | 5 ++--- > drivers/staging/media/imx/imx-media-csi.c | 5 ++--- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c > b/drivers/staging/media/imx/imx-ic-prpencvf.c > index ed363fe..65f5729 100644 > --- a/drivers/staging/media/imx/imx-ic-prpencvf.c > +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c > @@ -1278,9 +1278,8 @@ static int prp_init(struct imx_ic_priv > *ic_priv) > priv->ic_priv = ic_priv; > > spin_lock_init(&priv->irqlock); > - init_timer(&priv->eof_timeout_timer); > - priv->eof_timeout_timer.data = (unsigned long)priv; > - priv->eof_timeout_timer.function = prp_eof_timeout; > + setup_timer(&priv->eof_timeout_timer, prp_eof_timeout, > + (unsigned long)priv); > > priv->vdev = imx_media_capture_device_init(&ic_priv->sd, > PRPENCVF_SRC_PAD) > ; > diff --git a/drivers/staging/media/imx/imx-media-csi.c > b/drivers/staging/media/imx/imx-media-csi.c > index a2d2669..8fef5f1 100644 > --- a/drivers/staging/media/imx/imx-media-csi.c > +++ b/drivers/staging/media/imx/imx-media-csi.c > @@ -1731,9 +1731,8 @@ static int imx_csi_probe(struct platform_device > *pdev) > priv->csi_id = pdata->csi; > priv->smfc_id = (priv->csi_id == 0) ? 0 : 2; > > - init_timer(&priv->eof_timeout_timer); > - priv->eof_timeout_timer.data = (unsigned long)priv; > - priv->eof_timeout_timer.function = csi_idmac_eof_timeout; > + setup_timer(&priv->eof_timeout_timer, csi_idmac_eof_timeout, > + (unsigned long)priv); > spin_lock_init(&priv->irqlock); > > v4l2_subdev_init(&priv->sd, &csi_subdev_ops); Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi Jacob,
On Mon, 2017-08-07 at 19:06 +0800, Jacob Chen wrote:
[...]
> >> > --- a/drivers/media/i2c/ov5647.c
> >> > +++ b/drivers/media/i2c/ov5647.c
> >> > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> >> > {
> >> > int ret;
> >> >
> >> > + ret = ov5647_write(sd, 0x4800, 0x04);
> >> > + if (ret < 0)
> >> > + return ret;
> >> > +
So this clears BIT(1) (force clock lane to low power mode) and BIT(5)
(gate clock lane while idle) that were set by ov5647_stream_off() during
__sensor_init() due to the change below.
Is there a reason, btw, that this driver is full of magic register
addresses and values? A few #defines would make this a lot more
readable.
> >> > ret = ov5647_write(sd, 0x4202, 0x00);
> >> > if (ret < 0)
> >> > return ret;
> >> > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
> >> > {
> >> > int ret;
> >> >
> >> > + ret = ov5647_write(sd, 0x4800, 0x25);
> >> > + if (ret < 0)
> >> > + return ret;
> >> > +
> >> > ret = ov5647_write(sd, 0x4202, 0x0f);
> >> > if (ret < 0)
> >> > return ret;
> >> > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd)
> >> > return ret;
> >> > }
> >> >
> >> > - return ov5647_write(sd, 0x4800, 0x04);
> >> > + return ov5647_stream_off(sd);
I see now that BIT(2) (keep bus in LP-11 while idle) is and was always
set. So the change is that initially, additionally to LP-11 mode, the
clock lane is gated and forced into low power mode, as well?
> >> > }
> >> >
> >> > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
> >> > --
> >> > 2.7.4
> >> >
> >>
> >> Can anyone comment on it?
> >>
> >> I saw there is a same discussion in
> >> https://patchwork.kernel.org/patch/9569031/
> >> There is a comment in i.MX CSI2 driver.
> >> "
> >> Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state.
> >> This must be carried out by the MIPI sensor's s_power(ON) subdev
> >> op.
> >> "
> >> That's what this patch do, sensor driver should make sure that clock
> >> lanes are in stop state while not streaming.
> >
> > This is not the same, as far as I can tell. BIT(5) is just clock lane
> > gating, as you describe above. To put the bus into LP-11 state, BIT(2)
> > needs to be set.
> >
>
> Yeah, but i double that clock lane is not in LP11 when continue clock
> mode is enabled.
If indeed LP-11 state is not achieved while the sensor is idle, as long
as BIT(5) is cleared, I think this patch is correct.
regards
Philipp
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi Jacob,
On Mon, 2017-08-07 at 15:11 +0800, Jacob Chen wrote:
> Hi all,
>
> 2017-07-25 10:34 GMT+08:00 Jacob Chen :
> > According to datasheet, BIT5 in reg-0x4800 are used to
> > enable/disable clock lane gate.
> >
> > It's wrong to make clock lane free running before
> > sensor stream on was called, while the mipi phy
> > are not initialized.
> >
> > Signed-off-by: Jacob Chen
>>
> > ---
> > drivers/media/i2c/ov5647.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > index 95ce90f..d3e6fd0 100644
> > --- a/drivers/media/i2c/ov5647.c
> > +++ b/drivers/media/i2c/ov5647.c
> > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> > {
> > int ret;
> >
> > + ret = ov5647_write(sd, 0x4800, 0x04);
> > + if (ret < 0)
> > + return ret;
> > +
> > ret = ov5647_write(sd, 0x4202, 0x00);
> > if (ret < 0)
> > return ret;
> > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
> > {
> > int ret;
> >
> > + ret = ov5647_write(sd, 0x4800, 0x25);
> > + if (ret < 0)
> > + return ret;
> > +
> > ret = ov5647_write(sd, 0x4202, 0x0f);
> > if (ret < 0)
> > return ret;
> > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd)
> > return ret;
> > }
> >
> > - return ov5647_write(sd, 0x4800, 0x04);
> > + return ov5647_stream_off(sd);
> > }
> >
> > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
> > --
> > 2.7.4
> >
>
> Can anyone comment on it?
>
> I saw there is a same discussion in
> https://patchwork.kernel.org/patch/9569031/
> There is a comment in i.MX CSI2 driver.
> "
> Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state.
> This must be carried out by the MIPI sensor's s_power(ON) subdev
> op.
> "
> That's what this patch do, sensor driver should make sure that clock
> lanes are in stop state while not streaming.
This is not the same, as far as I can tell. BIT(5) is just clock lane
gating, as you describe above. To put the bus into LP-11 state, BIT(2)
needs to be set.
regards
Philipp
[PATCH] [media] coda: reduce iram size to leave space for suspend to ram
From: Jan Luebbe
The on-chip SRAM of i.MX6S is only 128 KiB. 4 KiB of that are allocated
for suspend to RAM since commit df595746fa69 ("ARM: imx: add suspend in
ocram support for i.mx6q"). Reduce the requested IRAM size to 124 KiB to
avoid an allocation failure that causes the coda driver to not use the
SRAM at all.
Signed-off-by: Jan Luebbe
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 40f31038249d9..27c613752fbf7 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -2443,7 +2443,7 @@ static const struct coda_devtype coda_devdata[] = {
.num_vdevs= ARRAY_SIZE(coda9_video_devices),
.workbuf_size = 80 * 1024,
.tempbuf_size = 204 * 1024,
- .iram_size= 0x2,
+ .iram_size= 0x1f000, /* leave 4k for suspend code */
},
};
--
2.11.0
[PATCH] [media] coda: fix decoder sequence init escape flag
coda_command_sync calls coda_command_async, which writes the
bit_stream_param context variable into the BIT_STREAM_PARAM register,
overwriting the previously set value during coda_start_decoding. Instead
of writing to the register, set bit_stream_param to ensure that the
decoder sequence init command is executed with the escape flag set.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 95e4b74d5dd01..291c409339357 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1639,9 +1639,6 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
ctx->frm_dis_flg = 0;
coda_write(dev, 0, CODA_REG_BIT_FRM_DIS_FLG(ctx->reg_idx));
- coda_write(dev, CODA_BIT_DEC_SEQ_INIT_ESCAPE,
- CODA_REG_BIT_BIT_STREAM_PARAM);
-
coda_write(dev, bitstream_buf, CODA_CMD_DEC_SEQ_BB_START);
coda_write(dev, bitstream_size / 1024, CODA_CMD_DEC_SEQ_BB_SIZE);
val = 0;
@@ -1676,18 +1673,18 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
if (dev->devtype->product != CODA_960)
coda_write(dev, 0, CODA_CMD_DEC_SEQ_SRC_SIZE);
- if (coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT)) {
+ ctx->bit_stream_param = CODA_BIT_DEC_SEQ_INIT_ESCAPE;
+ ret = coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT);
+ ctx->bit_stream_param = 0;
+ if (ret) {
v4l2_err(&dev->v4l2_dev, "CODA_COMMAND_SEQ_INIT timeout\n");
- coda_write(dev, 0, CODA_REG_BIT_BIT_STREAM_PARAM);
- return -ETIMEDOUT;
+ return ret;
}
ctx->initialized = 1;
/* Update kfifo out pointer from coda bitstream read pointer */
coda_kfifo_sync_from_device(ctx);
- coda_write(dev, 0, CODA_REG_BIT_BIT_STREAM_PARAM);
-
if (coda_read(dev, CODA_RET_DEC_SEQ_SUCCESS) == 0) {
v4l2_err(&dev->v4l2_dev,
"CODA_COMMAND_SEQ_INIT failed, error code = %d\n",
--
2.11.0
Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors
Hi Laurent, Am Dienstag, den 25.07.2017, 02:10 +0300 schrieb Laurent Pinchart: > > > Yes, I think a device-specific driver would make sense, especially if > > > we can implement support for the sensor as a standalone V4L2 subdev > > > driver. The device only fakes UVC compatibility :-( > > > > When you say standalone driver, do you mean I can reuse uvcvideo > > device/stream/chain handling, and just replace the control handling? > > No, I mean a completely separate driver. Given that the driver will be used > for a single device, you can hardcode lots of assumptions and don't have to > parse UVC descriptors. I see, I was hoping I wouldn't have to (re)write all the video transfer and timing parts myself. > > I'll try this, but it isn't a straightforward as I initially thought. > > For example, the mt9v032 subdev driver expects to have control over > > power during probe and s_power. But in this case power is controlled by > > UVC streaming. > > How does that work with the device ? If the sensor is powered off until you > start video streaming, I assume it won't reply to I2C commands. Do you need > to > configure it after stream start ? Yes. The webcam controllers replay the stored initialization sequences to the sensors on startup, like any other usb cameras, and start streaming images. That is why I added them to uvcvideo in the first place. After the stream has started, I'd like to change the controls from the defaults (enable AEC/AGC or increase gain for normal camera use, or reduce gain and exposure time and enable trigger mode for Rift tracking). Unfortunatley those can only be changed via I2C. > > I'd either have to modify the subdev driver to support a passive mode or > > fake the chip id register reads in the i2c adapter driver to make mt9v032 > > probe at all. > > > > Do you have any further comments on the first two patches? > > Just that those patches are not needed if we implement a driver specific to > the Oculus Rift :-) Ok. I'll give that a shot then. regards Philipp
Re: [PATCH] media: imx: prpencvf: enable double write reduction
On Sat, 2017-07-22 at 14:21 -0700, Steve Longerbeam wrote: > For the write channels with 4:2:0 subsampled YUV formats, avoid chroma > overdraw by only writing chroma for even lines. Reduces necessary write > memory bandwidth by at least 25% (more with rotation enabled). > > Signed-off-by: Steve Longerbeam Acked-by: Philipp Zabel regards Philipp
Re: [PATCH] media: imx: prpencvf: enable double write reduction
Hi Steve, On Sat, 2017-07-22 at 15:04 -0700, Steve Longerbeam wrote: > Hi Philipp, > > This is the same as your patch to CSI, applied to ic-prpencvf. > > I'm not really sure what this cpmem bit is doing. The U/V planes > in memory are already subsampled by 2 in both width and height. > This must be referring to what the IDMAC is transferring on the bus, Right, the IDMAC is just receiving AYUV 4:4:4 pixels from the FIFO, the CPMEM settings decide how they are written to the AXI bus. If this bit is not enabled, all pixels are written fully, even though chroma samples of even and odd lines end up in the same place for 4:2:0 chroma subsampled output formats. If the bit is set, the chroma writes for odd lines are skipped, so there is no overdraw. Unfortunately this does not work for reading, unless there is a line buffer (which only the VDIC has), because otherwise odd lines end up without chroma information. This one of the reasons that YUY2 is the most memory efficient format to read, not NV12. > but why would it place duplicate U/V samples on the bus in the first > place? Don't ask me why the hardware was designed this way :) I see no reason to ever disable this bit for YUV420 or NV12 write channels. > Anyway, thanks for the heads-up on this. regards Philipp
Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors
Hi Laurent, Am Montag, den 17.07.2017, 05:25 +0300 schrieb Laurent Pinchart: > Hi Philipp, > > On Saturday 15 Jul 2017 15:13:45 Philipp Zabel wrote: > > Am Samstag, den 15.07.2017, 12:54 +0300 schrieb Laurent Pinchart: > > > On Friday 14 Jul 2017 22:14:24 Philipp Zabel wrote: > > > > The Oculus Rift Sensors (DK2 and CV1) allow to configure their > > > > sensor > > > > chips directly via I2C commands using extension unit controls. > > > > The > > > > processing and camera unit controls do not function at all. > > > > > > Do the processing and camera units they report controls that > > > don't work > > > when exercised ? Who in a sane state of mind could have designed > > > such a > > > terrible product ? > > > > Yes. Without this patch I get a bunch of controls that have no > > effect > > whatsoever. > > > > > If I understand you correctly, this device requires userspace > > > code that > > > knows how to program the sensor (and possibly other chips). If > > > that's > > > the case, is there an open-source implementation of that code > > > publicly > > > available ? > > > > Well, it's all still a bit in the experimentation phase. We have an > > implementation to set up the DK2 camera for synchronised exposure > > triggered by the Rift DK2 HMD and to read the calibration data from > > flash, here: > > > > https://github.com/pH5/ouvrt/blob/master/src/esp570.c > > https://github.com/pH5/ouvrt/blob/master/src/mt9v034.c > > > > And an even more rough version to set up the CV1 camera for > > synchronised exposure triggered by the Rift CV1 HMD here: > > > > https://github.com/OpenHMD/OpenHMD-RiftPlayground/blob/master/src/m > > ain.c > > > > The latter is using libusb, as it needs the variable length SPI > > data > > control. > > > > Do you think adding a pseudo i2c driver for the eSP570/eSP770u > > webcam > > controllers and then exposing the sensor chips as V4L2 subdevices > > could > > be a good idea? We already have a sensor driver for the MT9V034 in > > the > > DK2 USB camera. > > Yes, I think a device-specific driver would make sense, especially if > we can > implement support for the sensor as a standalone V4L2 subdev driver. > The > device only fakes UVC compatibility :-( When you say standalone driver, do you mean I can reuse uvcvideo device/stream/chain handling, and just replace the control handling? I'll try this, but it isn't a straightforward as I initially thought. For example, the mt9v032 subdev driver expects to have control over power during probe and s_power. But in this case power is controlled by UVC streaming. I'd either have to modify the subdev driver to support a passive mode or fake the chip id register reads in the i2c adapter driver to make mt9v032 probe at all. Do you have any further comments on the first two patches? regards Philipp
[PATCH v2] [media] imx: csi: enable double write reduction
For 4:2:0 subsampled YUV formats, avoid chroma overdraw by only writing chroma for even lines. Reduces necessary write memory bandwidth by 25%. Signed-off-by: Philipp Zabel --- Changes since v1: - Move odd row skipping setup into existing switch statement. --- drivers/staging/media/imx/imx-media-csi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index a2d26693912ec..f2d64d1eeff80 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -357,6 +357,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 && sensor_ep->bus.parallel.bus_width >= 16); passthrough_bits = 16; + /* Skip writing U and V components to odd rows */ + ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch); break; case V4L2_PIX_FMT_YUYV: case V4L2_PIX_FMT_UYVY: -- 2.11.0
Re: [PATCH] [media] imx: csi: enable double write reduction
Hi Steve,
On Wed, 2017-07-19 at 09:18 -0700, Steve Longerbeam wrote:
> Hi Philipp,
>
> On 07/19/2017 05:22 AM, Philipp Zabel wrote:
> > For 4:2:0 subsampled YUV formats, avoid chroma overdraw by only writing
> > chroma for even lines. Reduces necessary write memory bandwidth by 25%.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/staging/media/imx/imx-media-csi.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/staging/media/imx/imx-media-csi.c
> > b/drivers/staging/media/imx/imx-media-csi.c
> > index a2d26693912ec..0fb70d5a9e7fe 100644
> > --- a/drivers/staging/media/imx/imx-media-csi.c
> > +++ b/drivers/staging/media/imx/imx-media-csi.c
> > @@ -388,6 +388,12 @@ static int csi_idmac_setup_channel(struct csi_priv
> > *priv)
> > goto unsetup_vb2;
> > }
> >
> > + switch (image.pix.pixelformat) {
> > + case V4L2_PIX_FMT_YUV420:
> > + case V4L2_PIX_FMT_NV12:
> > + ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
> > + }
> > +
>
> Is there any reason why you can't place this call under the
> already existing case statement for YUV420 and NV12 at line
> 352?
Not really. I didn't touch that block of code because it only dealt with
the burst size and generic data / passthrough mode settings. I'll move
the odd row skipping flag up there. Thank you for the suggestion.
regards
Philipp
[PATCH 034/102] coda: explicitly request exclusive reset control
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
reset lines") started to transition the reset control request API calls
to explicitly state whether the driver needs exclusive or shared reset
control behavior. Convert all drivers requesting exclusive resets to the
explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: linux-media@vger.kernel.org
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index f92cc7df58fb8..8e0b1a4e2546b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -2470,7 +2470,8 @@ static int coda_probe(struct platform_device *pdev)
return ret;
}
- dev->rstc = devm_reset_control_get_optional(&pdev->dev, NULL);
+ dev->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev,
+ NULL);
if (IS_ERR(dev->rstc)) {
ret = PTR_ERR(dev->rstc);
dev_err(&pdev->dev, "failed get reset control: %d\n", ret);
--
2.11.0
[PATCH 035/102] st-rc: explicitly request exclusive reset control
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
reset lines") started to transition the reset control request API calls
to explicitly state whether the driver needs exclusive or shared reset
control behavior. Convert all drivers requesting exclusive resets to the
explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Patrice Chotard
Cc: linux-media@vger.kernel.org
Signed-off-by: Philipp Zabel
---
drivers/media/rc/st_rc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index a08e1dd061249..d48304f6c3de0 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -280,7 +280,7 @@ static int st_rc_probe(struct platform_device *pdev)
else
rc_dev->rx_base = rc_dev->base;
- rc_dev->rstc = reset_control_get_optional(dev, NULL);
+ rc_dev->rstc = reset_control_get_optional_exclusive(dev, NULL);
if (IS_ERR(rc_dev->rstc)) {
ret = PTR_ERR(rc_dev->rstc);
goto err;
--
2.11.0
[PATCH 036/102] stm32-dcmi: explicitly request exclusive reset control
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
reset lines") started to transition the reset control request API calls
to explicitly state whether the driver needs exclusive or shared reset
control behavior. Convert all drivers requesting exclusive resets to the
explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Maxime Coquelin
Cc: Alexandre Torgue
Cc: linux-media@vger.kernel.org
Signed-off-by: Philipp Zabel
---
drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
b/drivers/media/platform/stm32/stm32-dcmi.c
index 83d32a5d0f408..a2d883dcf32b9 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1209,7 +1209,7 @@ static int dcmi_probe(struct platform_device *pdev)
if (!dcmi)
return -ENOMEM;
- dcmi->rstc = devm_reset_control_get(&pdev->dev, NULL);
+ dcmi->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
if (IS_ERR(dcmi->rstc)) {
dev_err(&pdev->dev, "Could not get reset control\n");
return -ENODEV;
--
2.11.0
[PATCH 037/102] rc: sunxi-cir: explicitly request exclusive reset control
Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
reset lines") started to transition the reset control request API calls
to explicitly state whether the driver needs exclusive or shared reset
control behavior. Convert all drivers requesting exclusive resets to the
explicit API call so the temporary transition helpers can be removed.
No functional changes.
Cc: Maxime Ripard
Cc: Chen-Yu Tsai
Cc: linux-media@vger.kernel.org
Signed-off-by: Philipp Zabel
---
drivers/media/rc/sunxi-cir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 4b785dd775c11..3e033fb79463a 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -173,7 +173,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
}
/* Reset (optional) */
- ir->rst = devm_reset_control_get_optional(dev, NULL);
+ ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
if (IS_ERR(ir->rst))
return PTR_ERR(ir->rst);
ret = reset_control_deassert(ir->rst);
--
2.11.0
[PATCH] [media] imx: csi: enable double write reduction
For 4:2:0 subsampled YUV formats, avoid chroma overdraw by only writing
chroma for even lines. Reduces necessary write memory bandwidth by 25%.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index a2d26693912ec..0fb70d5a9e7fe 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -388,6 +388,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
goto unsetup_vb2;
}
+ switch (image.pix.pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_NV12:
+ ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
+ }
+
ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
/*
--
2.11.0
[PATCH] [media] coda: disable BWB only while decoding on CODA 960
Disabling the BWB works around hangups observed while decoding. Since no
issues have been observed while encoding, and disabling BWB also reduces
encoding performance, reenable it for encoding.
Cc: Ian Arkver
Reported-by: Ian Arkver
Fixes: 89ed025d5c53 ("[media] coda: disable BWB for all codecs on CODA 960")
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 829c7895a98a2..21d89c411f149 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -73,7 +73,7 @@ MODULE_PARM_DESC(disable_vdoa, "Disable Video Data Order
Adapter tiled to raster
static int enable_bwb = 0;
module_param(enable_bwb, int, 0644);
-MODULE_PARM_DESC(enable_bwb, "Enable BWB unit, may crash on certain streams");
+MODULE_PARM_DESC(enable_bwb, "Enable BWB unit for decoding, may crash on
certain streams");
void coda_write(struct coda_dev *dev, u32 data, u32 reg)
{
@@ -1938,7 +1938,13 @@ static int coda_open(struct file *file)
ctx->idx = idx;
switch (dev->devtype->product) {
case CODA_960:
- if (enable_bwb)
+ /*
+* Enabling the BWB when decoding can hang the firmware with
+* certain streams. The issue was tracked as ENGR00293425 by
+* Freescale. As a workaround, disable BWB for all decoders.
+* The enable_bwb module parameter allows to override this.
+*/
+ if (enable_bwb || ctx->inst_type == CODA_INST_ENCODER)
ctx->frame_mem_ctrl = CODA9_FRAME_ENABLE_BWB;
/* fallthrough */
case CODA_7541:
@@ -2142,7 +2148,8 @@ static int coda_hw_init(struct coda_dev *dev)
CODA_REG_BIT_STREAM_CTRL);
}
if (dev->devtype->product == CODA_960)
- coda_write(dev, 1 << 12, CODA_REG_BIT_FRAME_MEM_CTRL);
+ coda_write(dev, CODA9_FRAME_ENABLE_BWB,
+ CODA_REG_BIT_FRAME_MEM_CTRL);
else
coda_write(dev, 0, CODA_REG_BIT_FRAME_MEM_CTRL);
--
2.11.0
Re: [PATCH] [media] coda: disable BWB for all codecs on CODA 960
On Wed, 2017-07-19 at 10:15 +0100, Ian Arkver wrote: > On 19/07/17 09:09, Philipp Zabel wrote: > > Hi Ian, > > > > On Wed, 2017-07-19 at 08:16 +0100, Ian Arkver wrote: > >> Hi Philipp, > >> > >> On 02/03/17 10:19, Philipp Zabel wrote: > >>> Side effects are reduced burst lengths when writing out decoded frames > >>> to memory, so there is an "enable_bwb" module parameter to turn it back > >>> on. > >> > >> These side effects are dramatically reducing the VPU throughput during > >> H.264 encode as well. Prior to this patch I was just about managing to > >> capture and stream 1080p25 H.264. After this change it fell to about > >> 19fps. Reverting this patch (or presumably using the module param) > >> restores the frame rate. > >> > >> Can we at least make this decode specific? The VPU library patches do it > >> in vpu_DecOpen. I'd guess disabling the BWB any time prior to stream > >> start would be OK. > > > > Yes, since ENGR00293425 only talks about decoders, and I haven't seen > > any BWB related hangups during encoding yet, I'm inclined to agree. > > I took a look at where ctx->frame_mem_ctrl is used, i.e. > coda_start_encoding and __coda_start_decoding. Is it possible to have > one instance of coda open for encode and one for decode at the same > time? Sure, when transcoding with a decoder / encoder pair, for example. > If so, how is the CODA_REG_BIT_FRAME_MEM_CTRL setting > updated between runs, eg. for differing CODA_FRAME_CHROMA_INTERLEAVE? Yes. coda_command_async writes ctx->frame_mem_ctrl to CODA_REG_BIT_FRAME_MEM_CTRL with each PIC_RUN command at from prepare_encode/decode. > This code is in the start_streaming path rather than the prepare_run > path. Does each context switch stop & restart streaming? No, see above. regards Philipp
Re: [PATCH] [media] coda: disable BWB for all codecs on CODA 960
Hi Ian, On Wed, 2017-07-19 at 08:16 +0100, Ian Arkver wrote: > Hi Philipp, > > On 02/03/17 10:19, Philipp Zabel wrote: > > I don't know what the BWB unit is, I guess W is for write and one of the > > Bs is for burst. All I know is that there repeatedly have been issues > > with it hanging on certain streams (ENGR00223231, ENGR00293425), with > > various firmware versions, sometimes blocking something related to the > > GDI bus or the GDI AXI adapter. There are some error cases that we don't > > know how to recover from without a reboot. Apparently this unit can be > > disabled by setting bit 12 in the FRAME_MEM_CTRL mailbox register to > > zero, so do that to avoid crashes. > > Both those FSL ENGR patches to Android and VPU lib are specific to > decode, with the first being specific to VC1 decode only. The first one is older, I suppose VC1 is where the issue has been observed first. I have seen sporadic BWB hangups when decoding h.264 RTP streams. > > Side effects are reduced burst lengths when writing out decoded frames > > to memory, so there is an "enable_bwb" module parameter to turn it back > > on. > > These side effects are dramatically reducing the VPU throughput during > H.264 encode as well. Prior to this patch I was just about managing to > capture and stream 1080p25 H.264. After this change it fell to about > 19fps. Reverting this patch (or presumably using the module param) > restores the frame rate. > > Can we at least make this decode specific? The VPU library patches do it > in vpu_DecOpen. I'd guess disabling the BWB any time prior to stream > start would be OK. Yes, since ENGR00293425 only talks about decoders, and I haven't seen any BWB related hangups during encoding yet, I'm inclined to agree. > It might also be worth adding some sort of /* HACK */ marker, since > disabling the BWB feels very like a hack to me. I don't think HACK in itself is very descriptive, but a reference to the original workaround could be helpful in the future. regards Philipp
[PATCH v2] [media] platform: video-mux: convert to multiplexer framework
Now that the multiplexer framework is merged, drop the temporary
mmio-mux implementation from the video-mux driver and convert it to use
the multiplexer API.
Signed-off-by: Philipp Zabel
---
Changes since v1:
- Select CONFIG_MULTIPLEXER.
---
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/video-mux.c | 53 +-
2 files changed, 8 insertions(+), 46 deletions(-)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 1313cd5334360..9a07e98e5620c 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -76,6 +76,7 @@ config VIDEO_M32R_AR_M64278
config VIDEO_MUX
tristate "Video Multiplexer"
+ select MULTIPLEXER
depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
select REGMAP
help
diff --git a/drivers/media/platform/video-mux.c
b/drivers/media/platform/video-mux.c
index 665744716f73b..ee89ad76bee23 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -17,8 +17,7 @@
#include
#include
#include
-#include
-#include
+#include
#include
#include
#include
@@ -30,7 +29,7 @@ struct video_mux {
struct v4l2_subdev subdev;
struct media_pad *pads;
struct v4l2_mbus_framefmt *format_mbus;
- struct regmap_field *field;
+ struct mux_control *mux;
struct mutex lock;
int active;
};
@@ -71,7 +70,7 @@ static int video_mux_link_setup(struct media_entity *entity,
}
dev_dbg(sd->dev, "setting %d active\n", local->index);
- ret = regmap_field_write(vmux->field, local->index);
+ ret = mux_control_try_select(vmux->mux, local->index);
if (ret < 0)
goto out;
vmux->active = local->index;
@@ -80,6 +79,7 @@ static int video_mux_link_setup(struct media_entity *entity,
goto out;
dev_dbg(sd->dev, "going inactive\n");
+ mux_control_deselect(vmux->mux);
vmux->active = -1;
}
@@ -193,46 +193,6 @@ static const struct v4l2_subdev_ops video_mux_subdev_ops =
{
.video = &video_mux_subdev_video_ops,
};
-static int video_mux_probe_mmio_mux(struct video_mux *vmux)
-{
- struct device *dev = vmux->subdev.dev;
- struct of_phandle_args args;
- struct reg_field field;
- struct regmap *regmap;
- u32 reg, mask;
- int ret;
-
- ret = of_parse_phandle_with_args(dev->of_node, "mux-controls",
-"#mux-control-cells", 0, &args);
- if (ret)
- return ret;
-
- if (!of_device_is_compatible(args.np, "mmio-mux"))
- return -EINVAL;
-
- regmap = syscon_node_to_regmap(args.np->parent);
- if (IS_ERR(regmap))
- return PTR_ERR(regmap);
-
- ret = of_property_read_u32_index(args.np, "mux-reg-masks",
-2 * args.args[0], ®);
- if (!ret)
- ret = of_property_read_u32_index(args.np, "mux-reg-masks",
-2 * args.args[0] + 1, &mask);
- if (ret < 0)
- return ret;
-
- field.reg = reg;
- field.msb = fls(mask) - 1;
- field.lsb = ffs(mask) - 1;
-
- vmux->field = devm_regmap_field_alloc(dev, regmap, field);
- if (IS_ERR(vmux->field))
- return PTR_ERR(vmux->field);
-
- return 0;
-}
-
static int video_mux_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -270,8 +230,9 @@ static int video_mux_probe(struct platform_device *pdev)
return -EINVAL;
}
- ret = video_mux_probe_mmio_mux(vmux);
- if (ret) {
+ vmux->mux = devm_mux_control_get(dev, NULL);
+ if (IS_ERR(vmux->mux)) {
+ ret = PTR_ERR(vmux->mux);
if (ret != -EPROBE_DEFER)
dev_err(dev, "Failed to get mux: %d\n", ret);
return ret;
--
2.11.0
Re: [PATCH] [media] platform: video-mux: convert to multiplexer framework
On Tue, 2017-07-18 at 12:03 +0200, Hans Verkuil wrote: > On 17/07/17 12:55, Philipp Zabel wrote: > > Now that the multiplexer framework is merged, drop the temporary > > mmio-mux implementation from the video-mux driver and convert it to use > > the multiplexer API. > > > > Signed-off-by: Philipp Zabel > > --- > > drivers/media/platform/video-mux.c | 53 > > +- > > 1 file changed, 7 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/media/platform/video-mux.c > > b/drivers/media/platform/video-mux.c > > index 665744716f73b..ee89ad76bee23 100644 > > --- a/drivers/media/platform/video-mux.c > > +++ b/drivers/media/platform/video-mux.c > > @@ -17,8 +17,7 @@ > > #include > > #include > > #include > > -#include > > -#include > > +#include > > Shouldn't Kconfig be modified as well to select the multiplexer? Am I missing > something? The mux framework has stubs, so this compiles fine without MULTIPLEXER enabled. On the other hand this driver is pretty useless without the multiplexer framework, and the i2c and iio muxes select it as well. I'll change it and send a v2. regards Philipp
[PATCH] [media] platform: video-mux: convert to multiplexer framework
Now that the multiplexer framework is merged, drop the temporary
mmio-mux implementation from the video-mux driver and convert it to use
the multiplexer API.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/video-mux.c | 53 +-
1 file changed, 7 insertions(+), 46 deletions(-)
diff --git a/drivers/media/platform/video-mux.c
b/drivers/media/platform/video-mux.c
index 665744716f73b..ee89ad76bee23 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -17,8 +17,7 @@
#include
#include
#include
-#include
-#include
+#include
#include
#include
#include
@@ -30,7 +29,7 @@ struct video_mux {
struct v4l2_subdev subdev;
struct media_pad *pads;
struct v4l2_mbus_framefmt *format_mbus;
- struct regmap_field *field;
+ struct mux_control *mux;
struct mutex lock;
int active;
};
@@ -71,7 +70,7 @@ static int video_mux_link_setup(struct media_entity *entity,
}
dev_dbg(sd->dev, "setting %d active\n", local->index);
- ret = regmap_field_write(vmux->field, local->index);
+ ret = mux_control_try_select(vmux->mux, local->index);
if (ret < 0)
goto out;
vmux->active = local->index;
@@ -80,6 +79,7 @@ static int video_mux_link_setup(struct media_entity *entity,
goto out;
dev_dbg(sd->dev, "going inactive\n");
+ mux_control_deselect(vmux->mux);
vmux->active = -1;
}
@@ -193,46 +193,6 @@ static const struct v4l2_subdev_ops video_mux_subdev_ops =
{
.video = &video_mux_subdev_video_ops,
};
-static int video_mux_probe_mmio_mux(struct video_mux *vmux)
-{
- struct device *dev = vmux->subdev.dev;
- struct of_phandle_args args;
- struct reg_field field;
- struct regmap *regmap;
- u32 reg, mask;
- int ret;
-
- ret = of_parse_phandle_with_args(dev->of_node, "mux-controls",
-"#mux-control-cells", 0, &args);
- if (ret)
- return ret;
-
- if (!of_device_is_compatible(args.np, "mmio-mux"))
- return -EINVAL;
-
- regmap = syscon_node_to_regmap(args.np->parent);
- if (IS_ERR(regmap))
- return PTR_ERR(regmap);
-
- ret = of_property_read_u32_index(args.np, "mux-reg-masks",
-2 * args.args[0], ®);
- if (!ret)
- ret = of_property_read_u32_index(args.np, "mux-reg-masks",
-2 * args.args[0] + 1, &mask);
- if (ret < 0)
- return ret;
-
- field.reg = reg;
- field.msb = fls(mask) - 1;
- field.lsb = ffs(mask) - 1;
-
- vmux->field = devm_regmap_field_alloc(dev, regmap, field);
- if (IS_ERR(vmux->field))
- return PTR_ERR(vmux->field);
-
- return 0;
-}
-
static int video_mux_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -270,8 +230,9 @@ static int video_mux_probe(struct platform_device *pdev)
return -EINVAL;
}
- ret = video_mux_probe_mmio_mux(vmux);
- if (ret) {
+ vmux->mux = devm_mux_control_get(dev, NULL);
+ if (IS_ERR(vmux->mux)) {
+ ret = PTR_ERR(vmux->mux);
if (ret != -EPROBE_DEFER)
dev_err(dev, "Failed to get mux: %d\n", ret);
return ret;
--
2.11.0
[PATCH] [media] coda: wake up capture queue on encoder stop after output streamoff
If an encoder stop command is issued after the output queue has already
stopped streaming, the qsequence counter has been reset to 0. Always
wake up the capture queue if the output queue is not streaming.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index a55e9875e5619..cb378d98d134b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -933,7 +933,7 @@ static int coda_encoder_cmd(struct file *file, void *fh,
ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
/* If there is no buffer in flight, wake up */
- if (ctx->qsequence == ctx->osequence) {
+ if (!ctx->streamon_out || ctx->qsequence == ctx->osequence) {
dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
V4L2_BUF_TYPE_VIDEO_CAPTURE);
dst_vq->last_buffer_dequeued = true;
--
2.11.0
[PATCH v2 1/3] [media] uvcvideo: variable size controls
Some USB webcam controllers have extension unit controls that report
different lengths via GET_LEN, depending on internal state. Add a flag
to mark these controls as variable length and issue GET_LEN before
GET/SET_CUR transfers to verify the current length.
Signed-off-by: Philipp Zabel
---
Changes since v1:
- Split uvc_ctrl_fill_xu_info_size and uvc_ctrl_fill_xu_info_flags out of
uvc_ctrl_fill_xu_info.
- Add uvc_ctrl_update_xu_info_size and reuse uvc_ctrl_fill_xu_info_size.
- Call uvc_ctrl_update_xu_info_size from uvc_xu_ctrl_query instead of open
coding the size update, thereby fixing a double kfree.
---
drivers/media/usb/uvc/uvc_ctrl.c | 98 ++--
include/uapi/linux/uvcvideo.h| 2 +
2 files changed, 87 insertions(+), 13 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c2ee6e39fd0c..43e8851cc381 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
struct usb_device_id id;
u8 entity;
u8 selector;
- u8 flags;
+ u16 flags;
};
static const struct uvc_ctrl_fixup fixups[] = {
@@ -1629,10 +1629,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device
*dev,
}
}
-/*
- * Query control information (size and flags) for XU controls.
- */
-static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
+static int uvc_ctrl_fill_xu_info_size(struct uvc_device *dev,
const struct uvc_control *ctrl, struct uvc_control_info *info)
{
u8 *data;
@@ -1642,11 +1639,6 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
if (data == NULL)
return -ENOMEM;
- memcpy(info->entity, ctrl->entity->extension.guidExtensionCode,
- sizeof(info->entity));
- info->index = ctrl->index;
- info->selector = ctrl->index + 1;
-
/* Query and verify the control length (GET_LEN) */
ret = uvc_query_ctrl(dev, UVC_GET_LEN, ctrl->entity->id, dev->intfnum,
info->selector, data, 2);
@@ -1659,6 +1651,21 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
info->size = le16_to_cpup((__le16 *)data);
+done:
+ kfree(data);
+ return ret;
+}
+
+static int uvc_ctrl_fill_xu_info_flags(struct uvc_device *dev,
+ const struct uvc_control *ctrl, struct uvc_control_info *info)
+{
+ u8 *data;
+ int ret;
+
+ data = kmalloc(1, GFP_KERNEL);
+ if (data == NULL)
+ return -ENOMEM;
+
/* Query the control information (GET_INFO) */
ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
info->selector, data, 1);
@@ -1678,6 +1685,32 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
| (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
UVC_CTRL_FLAG_AUTO_UPDATE : 0);
+done:
+ kfree(data);
+ return ret;
+}
+
+/*
+ * Query control information (size and flags) for XU controls.
+ */
+static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
+ const struct uvc_control *ctrl, struct uvc_control_info *info)
+{
+ int ret;
+
+ memcpy(info->entity, ctrl->entity->extension.guidExtensionCode,
+ sizeof(info->entity));
+ info->index = ctrl->index;
+ info->selector = ctrl->index + 1;
+
+ ret = uvc_ctrl_fill_xu_info_size(dev, ctrl, info);
+ if (ret < 0)
+ return ret;
+
+ ret = uvc_ctrl_fill_xu_info_flags(dev, ctrl, info);
+ if (ret < 0)
+ return ret;
+
uvc_ctrl_fixup_xu_info(dev, ctrl, info);
uvc_trace(UVC_TRACE_CONTROL, "XU control %pUl/%u queried: len %u, "
@@ -1687,9 +1720,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
(info->flags & UVC_CTRL_FLAG_SET_CUR) ? 1 : 0,
(info->flags & UVC_CTRL_FLAG_AUTO_UPDATE) ? 1 : 0);
-done:
- kfree(data);
- return ret;
+ return 0;
}
static int uvc_ctrl_add_info(struct uvc_device *dev, struct uvc_control *ctrl,
@@ -1717,6 +1748,40 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
return ret;
}
+/*
+ * Update control size for variable length XU controls.
+ */
+static int uvc_ctrl_update_xu_info_size(struct uvc_device *dev,
+ struct uvc_control *ctrl)
+{
+ u16 size = ctrl->info.size;
+ int ret;
+
+ if (!(ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN))
+ return 0;
+
+ /* Check if the control size has changed */
+ ret = uvc_ctrl_fill_xu_info_size(dev, ctrl, &ctrl->info);
+ if (ret < 0)
+ return ret;
+
+ if (ctrl->inf
[PATCH v2 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors
The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips
directly via I2C commands using extension unit controls. The processing and
camera unit controls do not function at all.
Signed-off-by: Philipp Zabel
---
drivers/media/usb/uvc/uvc_ctrl.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 1d60321a6777..738edb81bc0a 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2213,6 +2213,10 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
{
struct uvc_entity *entity;
unsigned int i;
+ const struct usb_device_id xu_only[] = {
+ { USB_DEVICE(0x2833, 0x0201) },
+ { USB_DEVICE(0x2833, 0x0211) },
+ };
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, &dev->entities, list) {
@@ -2220,6 +2224,16 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
+ /* Oculus Sensors only handle extension unit controls */
+ if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT) {
+ for (i = 0; i < ARRAY_SIZE(xu_only); i++) {
+ if (usb_match_one_id(dev->intf, &xu_only[i]))
+ break;
+ }
+ if (i != ARRAY_SIZE(xu_only))
+ continue;
+ }
+
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
bmControls = entity->extension.bmControls;
bControlSize = entity->extension.bControlSize;
--
2.13.2
[PATCH v2 2/3] [media] uvcvideo: flag variable length control on Oculus Rift CV1 Sensor
The extension unit controls with selectors 11 and 12 are used to make the
eSP770u webcam controller issue SPI transfers to configure the nRF51288
radio or to read the flash storage. Depending on internal state controlled
by selector 11, selector 12 reports different lengths.
Signed-off-by: Philipp Zabel
---
drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 43e8851cc381..1d60321a6777 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1613,6 +1613,9 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX |
UVC_CTRL_FLAG_GET_DEF | UVC_CTRL_FLAG_SET_CUR |
UVC_CTRL_FLAG_AUTO_UPDATE },
+ { { USB_DEVICE(0x2833, 0x0211) }, 4, 12,
+ UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_SET_CUR |
+ UVC_CTRL_FLAG_VARIABLE_LEN },
};
unsigned int i;
--
2.13.2
Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors
Am Samstag, den 15.07.2017, 12:54 +0300 schrieb Laurent Pinchart: > Hi Philipp, > > Thank you for the patch. > > On Friday 14 Jul 2017 22:14:24 Philipp Zabel wrote: > > The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips > > directly via I2C commands using extension unit controls. The processing and > > camera unit controls do not function at all. > > Do the processing and camera units they report controls that don't work when > exercised ? Who in a sane state of mind could have designed such a terrible > product ? Yes. Without this patch I get a bunch of controls that have no effect whatsoever. > If I understand you correctly, this device requires userspace code that knows > how to program the sensor (and possibly other chips). If that's the case, is > there an open-source implementation of that code publicly available ? Well, it's all still a bit in the experimentation phase. We have an implementation to set up the DK2 camera for synchronised exposure triggered by the Rift DK2 HMD and to read the calibration data from flash, here: https://github.com/pH5/ouvrt/blob/master/src/esp570.c https://github.com/pH5/ouvrt/blob/master/src/mt9v034.c And an even more rough version to set up the CV1 camera for synchronised exposure triggered by the Rift CV1 HMD here: https://github.com/OpenHMD/OpenHMD-RiftPlayground/blob/master/src/main.c The latter is using libusb, as it needs the variable length SPI data control. Do you think adding a pseudo i2c driver for the eSP570/eSP770u webcam controllers and then exposing the sensor chips as V4L2 subdevices could be a good idea? We already have a sensor driver for the MT9V034 in the DK2 USB camera. regards Philipp
Re: [PATCH 1/3] [media] uvcvideo: variable size controls
Hi Laurent,
Am Samstag, den 15.07.2017, 12:49 +0300 schrieb Laurent Pinchart:
> Hi Philipp,
>
> Thank you for the patch.
Thank you for the review.
> On Friday 14 Jul 2017 22:14:22 Philipp Zabel wrote:
> > Some USB webcam controllers have extension unit controls that report
> > different lengths via GET_LEN, depending on internal state.
>
> If I ever need to hire a hardware designer, I'll make sure to reject any
> candidate who thinks that creativity is an asset :-(
:)
> If the size changes, could the flags change as well ? Should you issue a
> GET_INFO too ?
The size-changing eSP770U control always reports the same INFO.
Could this be added as a separate flag in the future if needed, when
the next webcam controller designer feels creative?
> > Add a flag to mark these controls as variable length and issue GET_LEN
> > before GET/SET_CUR transfers to verify the current length.
>
> What happens if the internal state changes between the GET_LEN and the
> GET/SET_CUR ?
At least for the Oculus Sensor, as far as I can tell, the length only
changes in reaction to a SET_CUR on another control:
Control 11 is written to set up an SPI transfer, apparently. That 16-
byte control contains a length field in byte 9. The length written to
that field becomes the LEN of control 12, which can then be used to
read or write data.
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 26 +-
> > include/uapi/linux/uvcvideo.h| 2 ++
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> > b/drivers/media/usb/uvc/uvc_ctrl.c index c2ee6e39fd0c..ce69e2c6937d 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device
> > *dev, struct usb_device_id id;
> > > > u8 entity;
> > > > u8 selector;
> > > > - u8 flags;
> > > > + u16 flags;
> > > > };
> >
> > > > static const struct uvc_ctrl_fixup fixups[] = {
> > @@ -1799,6 +1799,30 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> > > > goto done;
> > > > }
> >
> > > > + if ((ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN) &&
> > > > reqflags) {
> > > > + data = kmalloc(2, GFP_KERNEL);
> > > > + /* Check if the control length has changed */
> > > > + ret = uvc_query_ctrl(chain->dev, UVC_GET_LEN,
> > > > xqry->unit,
> > + chain->dev->intfnum, xqry->selector,
>
> data,
> > > > + 2);
> > > > + size = le16_to_cpup((__le16 *)data);
> > + kfree(data);
>
> Now data is not NULL.
>
> > > > + if (ret < 0) {
> > > > + uvc_trace(UVC_TRACE_CONTROL,
> > > > + "GET_LEN failed on control %pUl/%u
> > > > (%d).\n",
> > > > + entity->extension.guidExtensionCode,
> > > > + xqry->selector, ret);
> > + goto done;
>
> And the kfree(data) at the done label will cause a double free.
Thanks, will fix that.
> > + }
> > > > + if (ctrl->info.size != size) {
> > > > + uvc_trace(UVC_TRACE_CONTROL,
> > + "XU control %pUl/%u queried: len %u ->
>
> %u\n",
> > > > + entity->extension.guidExtensionCode,
> > > > + xqry->selector, ctrl->info.size,
> > > > size);
> > > > + ctrl->info.size = size;
> > > > + }
> > + }
>
> How about moving this code (or part of it at least) to a function that could
> be shared with uvc_ctrl_fill_xu_info() ?
I'll try. This will come at the cost of a bit more boilerplate.
regards
Philipp
[PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors
The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips
directly via I2C commands using extension unit controls. The processing and
camera unit controls do not function at all.
Signed-off-by: Philipp Zabel
---
drivers/media/usb/uvc/uvc_ctrl.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 86cb16a2e7f4..573e1f8735bf 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2165,6 +2165,10 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
{
struct uvc_entity *entity;
unsigned int i;
+ const struct usb_device_id xu_only[] = {
+ { USB_DEVICE(0x2833, 0x0201) },
+ { USB_DEVICE(0x2833, 0x0211) },
+ };
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, &dev->entities, list) {
@@ -2172,6 +2176,16 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
+ /* Oculus Sensors only handle extension unit controls */
+ if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT) {
+ for (i = 0; i < ARRAY_SIZE(xu_only); i++) {
+ if (usb_match_one_id(dev->intf, &xu_only[i]))
+ break;
+ }
+ if (i != ARRAY_SIZE(xu_only))
+ continue;
+ }
+
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
bmControls = entity->extension.bmControls;
bControlSize = entity->extension.bControlSize;
--
2.13.2
[PATCH 1/3] [media] uvcvideo: variable size controls
Some USB webcam controllers have extension unit controls that report
different lengths via GET_LEN, depending on internal state. Add a flag
to mark these controls as variable length and issue GET_LEN before
GET/SET_CUR transfers to verify the current length.
Signed-off-by: Philipp Zabel
---
drivers/media/usb/uvc/uvc_ctrl.c | 26 +-
include/uapi/linux/uvcvideo.h| 2 ++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c2ee6e39fd0c..ce69e2c6937d 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
struct usb_device_id id;
u8 entity;
u8 selector;
- u8 flags;
+ u16 flags;
};
static const struct uvc_ctrl_fixup fixups[] = {
@@ -1799,6 +1799,30 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
goto done;
}
+ if ((ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN) && reqflags) {
+ data = kmalloc(2, GFP_KERNEL);
+ /* Check if the control length has changed */
+ ret = uvc_query_ctrl(chain->dev, UVC_GET_LEN, xqry->unit,
+chain->dev->intfnum, xqry->selector, data,
+2);
+ size = le16_to_cpup((__le16 *)data);
+ kfree(data);
+ if (ret < 0) {
+ uvc_trace(UVC_TRACE_CONTROL,
+ "GET_LEN failed on control %pUl/%u (%d).\n",
+ entity->extension.guidExtensionCode,
+ xqry->selector, ret);
+ goto done;
+ }
+ if (ctrl->info.size != size) {
+ uvc_trace(UVC_TRACE_CONTROL,
+ "XU control %pUl/%u queried: len %u -> %u\n",
+ entity->extension.guidExtensionCode,
+ xqry->selector, ctrl->info.size, size);
+ ctrl->info.size = size;
+ }
+ }
+
if (size != xqry->size) {
ret = -ENOBUFS;
goto done;
diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
index 3b081862b9e8..0f0d63e79045 100644
--- a/include/uapi/linux/uvcvideo.h
+++ b/include/uapi/linux/uvcvideo.h
@@ -27,6 +27,8 @@
#define UVC_CTRL_FLAG_RESTORE (1 << 6)
/* Control can be updated by the camera. */
#define UVC_CTRL_FLAG_AUTO_UPDATE (1 << 7)
+/* Control can change LEN */
+#define UVC_CTRL_FLAG_VARIABLE_LEN (1 << 8)
#define UVC_CTRL_FLAG_GET_RANGE \
(UVC_CTRL_FLAG_GET_CUR | UVC_CTRL_FLAG_GET_MIN | \
--
2.13.2
[PATCH 2/3] [media] uvcvideo: flag variable length control on Oculus Rift CV1 Sensor
The extension unit controls with selectors 11 and 12 are used to make the
eSP770u webcam controller issue SPI transfers to configure the nRF51288
radio or to read the flash storage. Depending on internal state controlled
by selector 11, selector 12 reports different lengths.
Signed-off-by: Philipp Zabel
---
drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index ce69e2c6937d..86cb16a2e7f4 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1613,6 +1613,9 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX |
UVC_CTRL_FLAG_GET_DEF | UVC_CTRL_FLAG_SET_CUR |
UVC_CTRL_FLAG_AUTO_UPDATE },
+ { { USB_DEVICE(0x2833, 0x0211) }, 4, 12,
+ UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_SET_CUR |
+ UVC_CTRL_FLAG_VARIABLE_LEN },
};
unsigned int i;
--
2.13.2
Re: [PATCH v2] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage
On Tue, 2017-07-11 at 15:18 +0200, Arnd Bergmann wrote:
> While looking at a compiler warning, I noticed the use of
> IS_ERR_OR_NULL, which is generally a sign of a bad API design
> and should be avoided.
>
> In this driver, this is fairly easy, we can simply stop storing
> error pointers in persistent structures, and change the two
> functions that might return either a NULL pointer or an error
> code to consistently return error pointers when failing.
>
> of_parse_subdev() now separates the error code and the pointer
> it looks up, to clarify the interface. There are two cases
> where this function originally returns 'NULL', and I have
> changed that to '0' for success to keep the current behavior,
> though returning an error would also make sense there.
>
> Fixes: e130291212df ("[media] media: Add i.MX media core driver")
> Signed-off-by: Arnd Bergmann
> ---
> v2: fix type mismatch
> v3: rework of_parse_subdev() as well.
Thanks!
Reviewed-by: Philipp Zabel
Tested-by: Philipp Zabel
regards
Philipp
Re: coda 2040000.vpu: firmware request failed
Hi Jagan,
On Fri, 2017-07-07 at 18:15 +0530, Jagan Teki wrote:
> Hi,
>
> I'm observing firmware request failure with i.MX6Q board, This is with
> latest linux-next (4.12) with firmware from, [1] and converted
> v4l-coda960-imx6q.bin using [2].
>
> Log:
> --
> coda 204.vpu: Direct firmware load for vpu_fw_imx6q.bin failed with error
> -2
> coda 204.vpu: Direct firmware load for vpu/vpu_fw_imx6q.bin failed
> with error -2
> coda 204.vpu: Direct firmware load for v4l-coda960-imx6q.bin
> failed with error -2
The error code is -ENOENT, so the firmware binary is not found where the
firmware loader code is looking. That could be caused by the coda driver
being probed before the file system containing the firmware binary is
mounted. Have you tried compiling the coda driver as a module
(CONFIG_VIDEO_CODA=m)?
> coda 204.vpu: firmware request failed
>
> I've verified md4sum and VDDPU as well, hope these look OK.
>
> # md5sum /lib/firmware/v4l-coda960-imx6q.bin
> af4971a37c7a3a50c99f7dfd36104c63 /lib/firmware/v4l-coda960-imx6q.bin
> # dmesg | grep regu | grep -i vddpu
> [0.061552] vddpu: supplied by regulator-dummy
>
> Did I missed any, request for help?
>
> [1]
> http://www.freescale.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-3.0.35-4.0.0.bin
> [2]
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181101.html
>
> thanks!
Note that converting the NXP provided firmware is not necessary anymore
since commits a1a87fa3a0cf ("[media] coda: add support for native order
firmware files with Freescale header") and 2ac7f08e3075 ("[media] coda:
add support for firmware files named as distributed by NXP").
Also there are newer firmware binaries available, see
https://patchwork.linuxtv.org/patch/42332/
regards
Philipp
[PATCH 5/5] [media] coda: mark CODA960 firmware versions 2.3.10 and 3.1.1 as supported
Firmware versions 2.3.10 revision 40778 and 3.1.1 revision 46072 are
contained in the i.MX firmware download archives provided by NXP at
http://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-3.5.7-1.0.0.bin
and http://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-5.4.bin,
respectively.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 2a8e13c53f2e1..95e4b74d5dd01 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -703,6 +703,8 @@ static u32 coda_supported_firmwares[] = {
CODA_FIRMWARE_VERNUM(CODA_DX6, 2, 2, 5),
CODA_FIRMWARE_VERNUM(CODA_7541, 1, 4, 50),
CODA_FIRMWARE_VERNUM(CODA_960, 2, 1, 5),
+ CODA_FIRMWARE_VERNUM(CODA_960, 2, 3, 10),
+ CODA_FIRMWARE_VERNUM(CODA_960, 3, 1, 1),
};
static bool coda_firmware_supported(u32 vernum)
--
2.11.0
[PATCH 3/5] [media] coda: align internal mpeg4 framebuffers to 16x16 macroblocks
This fixes visual artifacts in the first macroblock row of encoded
MPEG-4 video output caused by 8 additional lines of luma data leaking
into the chroma planes of the internal reference framebuffers: the
buffer size is rounded up to a multiple of 16x16 macroblock size, same
as for the h.264 encoder.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index a4abeabfa5377..2f31c672aba04 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -394,7 +394,8 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
int i;
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 ||
- ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264) {
+ ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264 ||
+ ctx->codec->dst_fourcc == V4L2_PIX_FMT_MPEG4) {
width = round_up(q_data->width, 16);
height = round_up(q_data->height, 16);
} else {
--
2.11.0
[PATCH 2/5] [media] coda: set field of destination buffers
Set the field of destination buffers properly. Signed-off-by: Philipp Zabel --- drivers/media/platform/coda/coda-bit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c index 50eed636830f8..a4abeabfa5377 100644 --- a/drivers/media/platform/coda/coda-bit.c +++ b/drivers/media/platform/coda/coda-bit.c @@ -1412,6 +1412,7 @@ static void coda_finish_encode(struct coda_ctx *ctx) } dst_buf->vb2_buf.timestamp = src_buf->vb2_buf.timestamp; + dst_buf->field = src_buf->field; dst_buf->flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; dst_buf->flags |= src_buf->flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK; @@ -2154,6 +2155,7 @@ static void coda_finish_decode(struct coda_ctx *ctx) dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); dst_buf->sequence = ctx->osequence++; + dst_buf->field = V4L2_FIELD_NONE; dst_buf->flags &= ~(V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_PFRAME | V4L2_BUF_FLAG_BFRAME); -- 2.11.0
[PATCH 4/5] [media] coda: set MPEG-4 encoder class register
Explicitly set MPEG-4 encoder class register instead of relying on the
default value of 0.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 4
drivers/media/platform/coda/coda_regs.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 2f31c672aba04..2a8e13c53f2e1 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1655,6 +1655,10 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
ctx->params.codec_mode_aux = CODA_MP4_AUX_MPEG4;
else
ctx->params.codec_mode_aux = 0;
+ if (src_fourcc == V4L2_PIX_FMT_MPEG4) {
+ coda_write(dev, CODA_MP4_CLASS_MPEG4,
+ CODA_CMD_DEC_SEQ_MP4_ASP_CLASS);
+ }
if (src_fourcc == V4L2_PIX_FMT_H264) {
if (dev->devtype->product == CODA_7541) {
coda_write(dev, ctx->psbuf.paddr,
diff --git a/drivers/media/platform/coda/coda_regs.h
b/drivers/media/platform/coda/coda_regs.h
index 77ee46a934272..38df5fd9a2fa7 100644
--- a/drivers/media/platform/coda/coda_regs.h
+++ b/drivers/media/platform/coda/coda_regs.h
@@ -158,6 +158,7 @@
#define CODA_CMD_DEC_SEQ_PS_BB_START 0x194
#define CODA_CMD_DEC_SEQ_PS_BB_SIZE0x198
#define CODA_CMD_DEC_SEQ_MP4_ASP_CLASS 0x19c
+#defineCODA_MP4_CLASS_MPEG40
#define CODA_CMD_DEC_SEQ_X264_MV_EN0x19c
#define CODA_CMD_DEC_SEQ_SPP_CHUNK_SIZE0x1a0
--
2.11.0
[PATCH 1/5] [media] coda: extend GOP size range
CodaDx6 only accepts GOP sizes up to 60 frames, but CODA960 can handle
up to 99 frames. If we disable automatic I frame generation altogether
by setting GOP size to 0, we can let an application produce arbitrarily
large I frame intervals using the V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME
control.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 5 +++--
drivers/media/platform/coda/coda-common.c | 4 +++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index bba1eb43b5d83..50eed636830f8 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1006,7 +1006,7 @@ static int coda_start_encoding(struct coda_ctx *ctx)
break;
}
coda_write(dev, value, CODA_CMD_ENC_SEQ_SLICE_MODE);
- value = ctx->params.gop_size & CODA_GOP_SIZE_MASK;
+ value = ctx->params.gop_size;
coda_write(dev, value, CODA_CMD_ENC_SEQ_GOP_SIZE);
}
@@ -1250,7 +1250,8 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
force_ipicture = ctx->params.force_ipicture;
if (force_ipicture)
ctx->params.force_ipicture = false;
- else if ((src_buf->sequence % ctx->params.gop_size) == 0)
+ else if (ctx->params.gop_size != 0 &&
+(src_buf->sequence % ctx->params.gop_size) == 0)
force_ipicture = 1;
/*
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 829c7895a98a2..218d778a9c45a 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1734,10 +1734,12 @@ static const struct v4l2_ctrl_ops coda_ctrl_ops = {
static void coda_encode_ctrls(struct coda_ctx *ctx)
{
+ int max_gop_size = (ctx->dev->devtype->product == CODA_DX6) ? 60 : 99;
+
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
V4L2_CID_MPEG_VIDEO_BITRATE, 0, 32767000, 1000, 0);
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
- V4L2_CID_MPEG_VIDEO_GOP_SIZE, 1, 60, 1, 16);
+ V4L2_CID_MPEG_VIDEO_GOP_SIZE, 0, max_gop_size, 1, 16);
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
V4L2_CID_MPEG_VIDEO_H264_I_FRAME_QP, 0, 51, 1, 25);
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
--
2.11.0
Re: [PATCH] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage
Hi Arnd,
thank you for the cleanup. I see two issues below:
On Wed, 2017-06-28 at 22:13 +0200, Arnd Bergmann wrote:
> While looking at a compiler warning, I noticed the use of
> IS_ERR_OR_NULL, which is generally a sign of a bad API design
> and should be avoided.
>
> In this driver, this is fairly easy, we can simply stop storing
> error pointers in persistent structures, and change the one
> function that might return either a NULL pointer or an error
> code to consistently return error pointers when failing.
>
> Fixes: e130291212df ("[media] media: Add i.MX media core driver")
> Signed-off-by: Arnd Bergmann
> ---
> I can't reproduce the original warning any more, but this
> patch still makes sense by itself.
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 41
> -
> drivers/staging/media/imx/imx-media-csi.c | 29 +++-
> drivers/staging/media/imx/imx-media-dev.c | 2 +-
> drivers/staging/media/imx/imx-media-of.c| 2 +-
> drivers/staging/media/imx/imx-media-vdic.c | 37 ++
> 5 files changed, 61 insertions(+), 50 deletions(-)
>
[...]
> diff --git a/drivers/staging/media/imx/imx-media-csi.c
> b/drivers/staging/media/imx/imx-media-csi.c
> index a2d26693912e..a4b3c305dcc8 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -122,11 +122,11 @@ static inline struct csi_priv *sd_to_dev(struct
> v4l2_subdev *sdev)
>
> static void csi_idmac_put_ipu_resources(struct csi_priv *priv)
> {
> - if (!IS_ERR_OR_NULL(priv->idmac_ch))
> + if (priv->idmac_ch)
> ipu_idmac_put(priv->idmac_ch);
> priv->idmac_ch = NULL;
>
> - if (!IS_ERR_OR_NULL(priv->smfc))
> + if (priv->smfc)
> ipu_smfc_put(priv->smfc);
> priv->smfc = NULL;
> }
> @@ -134,23 +134,26 @@ static void csi_idmac_put_ipu_resources(struct csi_priv
> *priv)
> static int csi_idmac_get_ipu_resources(struct csi_priv *priv)
> {
> int ch_num, ret;
> + struct ipu_smfc *smfc, *idmac_ch;
This should be
+ struct ipuv3_channel *idmac_ch;
+ struct ipu_smfc *smfc;
instead.
[...]
> diff --git a/drivers/staging/media/imx/imx-media-dev.c
> b/drivers/staging/media/imx/imx-media-dev.c
> index 48cbc7716758..c58ff0831890 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -91,7 +91,7 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
> if (imx_media_find_async_subdev(imxmd, np, devname)) {
> dev_dbg(imxmd->md.dev, "%s: already added %s\n",
> __func__, np ? np->name : devname);
> - imxsd = NULL;
> + imxsd = ERR_PTR(-EEXIST);
And since this returns -EEXIST now, ...
> goto out;
> }
>
> diff --git a/drivers/staging/media/imx/imx-media-of.c
> b/drivers/staging/media/imx/imx-media-of.c
> index b026fe66467c..4aac42cb79a4 100644
> --- a/drivers/staging/media/imx/imx-media-of.c
> +++ b/drivers/staging/media/imx/imx-media-of.c
> @@ -115,7 +115,7 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct
> device_node *sd_np,
>
> /* register this subdev with async notifier */
> imxsd = imx_media_add_async_subdev(imxmd, sd_np, NULL);
> - if (IS_ERR_OR_NULL(imxsd))
> + if (IS_ERR(imxsd))
> return imxsd;
... this changes behaviour:
imx-media: imx_media_of_parse failed with -17
imx-media: probe of capture-subsystem failed with error -17
We must continue to return NULL here if imxsd == -EEXIST:
- return imxsd;
+ return PTR_ERR(imxsd) == -EEXIST ? NULL : imxsd;
or change the code where of_parse_subdev is called (from
imx_media_of_parse, and recursively from of_parse_subdev) to not handle
the -EEXIST return value as an error.
With those fixed,
Reviewed-by: Philipp Zabel
Tested-by: Philipp Zabel
regards
Philipp
[PATCH] [media] coda: do not reassign ctx->tiled_map_type in coda_s_fmt
This smatch warning:
coda/coda-common.c:706 coda_s_fmt() warn: missing break? reassigning
'ctx->tiled_map_type'
can be silenced by moving the ctx->tiled_map_type assignment into the
breakout condition. That way the field is not reassigned when falling
through to the next switch statement.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index f92cc7df58fb8..dfceab052a4fa 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -714,9 +714,10 @@ static int coda_s_fmt(struct coda_ctx *ctx, struct
v4l2_format *f,
ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
break;
case V4L2_PIX_FMT_NV12:
- ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
- if (!disable_tiling)
+ if (!disable_tiling) {
+ ctx->tiled_map_type = GDI_TILED_FRAME_MB_RASTER_MAP;
break;
+ }
/* else fall through */
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
--
2.11.0
[PATCH] [media] coda: add h264 and mpeg4 profile and level controls
CODA7541 supports H.264 BP level 3/3.1 and MPEG-4 SP level 5/6.
CODA960 supports H.264 BP level 4.0 and MPEG-4 SP level 5/6.
Implement the necessary profile and level controls to let userspace know
this.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 829c7895a98a2..d119b47773282 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1683,12 +1683,23 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
ctx->params.h264_deblk_enabled = (ctrl->val ==
V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_ENABLED);
break;
+ case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
+ /* TODO: switch between baseline and constrained baseline */
+ ctx->params.h264_profile_idc = 66;
+ break;
+ case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
+ /* nothing to do, this is set by the encoder */
+ break;
case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP:
ctx->params.mpeg4_intra_qp = ctrl->val;
break;
case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP:
ctx->params.mpeg4_inter_qp = ctrl->val;
break;
+ case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+ case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+ /* nothing to do, these are fixed */
+ break;
case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
ctx->params.slice_mode = ctrl->val;
break;
@@ -1756,11 +1767,47 @@ static void coda_encode_ctrls(struct coda_ctx *ctx)
V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_MODE,
V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_DISABLED, 0x0,
V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_ENABLED);
+ v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_H264_PROFILE,
+ V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, 0x0,
+ V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
+ if (ctx->dev->devtype->product == CODA_7541) {
+ v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+ V4L2_MPEG_VIDEO_H264_LEVEL_3_1,
+ ~((1 << V4L2_MPEG_VIDEO_H264_LEVEL_2_0) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_1)),
+ V4L2_MPEG_VIDEO_H264_LEVEL_3_1);
+ }
+ if (ctx->dev->devtype->product == CODA_960) {
+ v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+ V4L2_MPEG_VIDEO_H264_LEVEL_4_0,
+ ~((1 << V4L2_MPEG_VIDEO_H264_LEVEL_2_0) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
+ (1 << V4L2_MPEG_VIDEO_H264_LEVEL_4_0)),
+ V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
+ }
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP, 1, 31, 1, 2);
v4l2_ctrl_new_std(&ctx->ctrls, &coda_ctrl_ops,
V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP, 1, 31, 1, 2);
v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE,
+ V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE, 0x0,
+ V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE);
+ if (ctx->dev->devtype->product == CODA_7541 ||
+ ctx->dev->devtype->product == CODA_960) {
+ v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
+ V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL,
+ V4L2_MPEG_VIDEO_MPEG4_LEVEL_5,
+ ~(1 << V4L2_MPEG_VIDEO_MPEG4_LEVEL_5),
+ V4L2_MPEG_VIDEO_MPEG4_LEVEL_5);
+ }
+ v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE,
V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_BYTES, 0x0,
V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE);
--
2.11.0
[PATCH] [media] coda: rename the picture run timeout error handler
I would have liked the the picture run timeout error handler to be renamed to
something a bit more descriptive in the original commit fb2be08f8cb3 ("[media]
coda: first step at error recovery").
Somehow v1 [1] was merged instead of v2 [2].
[1] https://patchwork.kernel.org/patch/9663965/
[2] https://patchwork.kernel.org/patch/9774239/
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 4 ++--
drivers/media/platform/coda/coda-common.c | 4 ++--
drivers/media/platform/coda/coda.h| 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 25cbf9e5ac5a7..795b6d7584320 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2198,7 +2198,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
ctx->display_idx = display_idx;
}
-static void coda_error_decode(struct coda_ctx *ctx)
+static void coda_decode_timeout(struct coda_ctx *ctx)
{
struct vb2_v4l2_buffer *dst_buf;
@@ -2223,7 +2223,7 @@ const struct coda_context_ops coda_bit_decode_ops = {
.start_streaming = coda_start_decoding,
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
- .error_run = coda_error_decode,
+ .run_timeout = coda_decode_timeout,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
};
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index f92cc7df58fb8..829c7895a98a2 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1164,8 +1164,8 @@ static void coda_pic_run_work(struct work_struct *work)
coda_hw_reset(ctx);
- if (ctx->ops->error_run)
- ctx->ops->error_run(ctx);
+ if (ctx->ops->run_timeout)
+ ctx->ops->run_timeout(ctx);
} else if (!ctx->aborting) {
ctx->ops->finish_run(ctx);
}
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 40fe22f0d7573..c5f504d8cf67f 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -183,7 +183,7 @@ struct coda_context_ops {
int (*start_streaming)(struct coda_ctx *ctx);
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
- void (*error_run)(struct coda_ctx *ctx);
+ void (*run_timeout)(struct coda_ctx *ctx);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
};
--
2.11.0
Re: [PATCH v2 2/3] [media] coda: first step at error recovery
On Fri, 2017-06-23 at 11:29 +0200, Hans Verkuil wrote: > On 06/08/17 10:55, Philipp Zabel wrote: > > From: Lucas Stach > > > > This implements a simple handler for the case where decode did not finish > > successfully. This might be helpful during normal streaming, but for now it > > only handles the case where the context would deadlock with userspace, > > i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed > > decode run we would hold the context and wait for userspace to queue more > > buffers. > > > > Signed-off-by: Lucas Stach > > Reviewed-by: Philipp Zabel > > [p.za...@pengutronix.de: renamed error_decode/run to run/decode_timeout] > > Signed-off-by: Philipp Zabel > > --- > > Changes since v1 [1]: > > - Rename error_run/decode callback to run/decode_timeout, as > > this error handler is called on device_run timeouts only. > > > > [1] https://patchwork.linuxtv.org/patch/40603 > > It appears the v1 version was merged, not the v2. I'm not sure if the v2 > version > was posted after v1 was already merged, or if I missed this v2 series. > > I'm marking this as Obsoleted, and if you want to still get these v2 changes > in, then please post a new patch. > > Sorry for the mix up, Thank you for the heads up, I'll send a patch. regards Philipp
Re: [PATCH] MAINTAINERS: add entry for Freescale i.MX media driver
On Mon, Jun 12, 2017 at 09:36:51AM -0700, Steve Longerbeam wrote: > Add maintainer entry for the imx-media driver. > > Signed-off-by: Steve Longerbeam Acked-by: Philipp Zabel regards Philipp > --- > MAINTAINERS | 12 > 1 file changed, 12 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9c7f663..11adc51 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -8111,6 +8111,18 @@ L: linux-...@vger.kernel.org > S: Maintained > F: drivers/iio/dac/cio-dac.c > > +MEDIA DRIVERS FOR FREESCALE IMX > +M: Steve Longerbeam > +M: Philipp Zabel > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: Documentation/devicetree/bindings/media/imx.txt > +F: Documentation/media/v4l-drivers/imx.rst > +F: drivers/staging/media/imx/ > +F: include/linux/imx-media.h > +F: include/media/imx.h > + > MEDIA DRIVERS FOR RENESAS - FCP > M: Laurent Pinchart > L: linux-media@vger.kernel.org > -- > 2.7.4 > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] MAINTAINERS: add maintainer entry for video multiplexer v4l2 subdevice driver
Add maintainer entry for the video multiplexer v4l2 subdevice driver that will control video bus multiplexers via the multiplexer framework. Signed-off-by: Philip Zabel --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f7d568b8f133..f6b8282efe46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13522,6 +13522,12 @@ S: Maintained F: drivers/media/v4l2-core/videobuf2-* F: include/media/videobuf2-* +VIDEO MULTIPLEXER DRIVER +M: Philipp Zabel +L: linux-media@vger.kernel.org +S: Maintained +F: drivers/media/platform/video-mux.c + VIRTIO AND VHOST VSOCK DRIVER M: Stefan Hajnoczi L: k...@vger.kernel.org -- 2.11.0
Re: [PATCH v8 19/34] media: Add i.MX media core driver
On Wed, 2017-06-07 at 11:33 -0700, Steve Longerbeam wrote:
> Add the core media driver for i.MX SOC.
>
> Signed-off-by: Steve Longerbeam
>
> Switch from the v4l2_of_ APIs to the v4l2_fwnode_ APIs.
>
> Signed-off-by: Philipp Zabel
>
> Add the bayer formats to imx-media's list of supported pixel and bus
> formats.
>
> Signed-off-by: Russell King
> ---
[...]
> diff --git a/drivers/staging/media/imx/imx-media-dev.c
> b/drivers/staging/media/imx/imx-media-dev.c
> new file mode 100644
> index 000..da694f6
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -0,0 +1,666 @@
[...]
> +/*
> + * adds given video device to given imx-media source pad vdev list.
> + * Continues upstream from the pad entity's sink pads.
> + */
> +static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
> + struct imx_media_video_dev *vdev,
> + struct media_pad *srcpad)
> +{
> + struct media_entity *entity = srcpad->entity;
> + struct imx_media_subdev *imxsd;
> + struct imx_media_pad *imxpad;
> + struct media_link *link;
> + struct v4l2_subdev *sd;
> + int i, vdev_idx, ret;
> +
> + if (!is_media_entity_v4l2_subdev(entity))
> + return -EINVAL;
Could we make this return 0, to just skip non-v4l2_subdev entities?
Currently, imx_media_probe_complete silently fails with this -EINVAL if
there is a tvp5150 connected due to the separate media entities that the
tvp5150 driver creates for the input connectors (Composite0, for
example).
regards
Philipp
[PATCH] [media] coda: ctx->codec is not NULL in coda_alloc_framebuffers
This fixes a smatch warning:
drivers/media/platform/coda/coda-bit.c:415 coda_alloc_framebuffers()
error: we previously assumed 'ctx->codec' could be null (see line 396)
coda_alloc_framebuffers() is called from coda_start_encoding() and
__coda_start_decoding(). Both dereference ctx->codec before calling
coda_alloc_framebuffers() in lines 935 and 1649, so ctx->codec can not
be NULL.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 795b6d7584320..bba1eb43b5d83 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -393,8 +393,8 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
int ret;
int i;
- if (ctx->codec && (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 ||
-ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264)) {
+ if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 ||
+ ctx->codec->dst_fourcc == V4L2_PIX_FMT_H264) {
width = round_up(q_data->width, 16);
height = round_up(q_data->height, 16);
} else {
--
2.11.0
[PATCH v2 1/3] [media] coda: use correct offset for mvcol buffer
From: Lucas Stach
The mvcol buffer needs to be placed behind the chroma plane(s), so
use the real offset including any required rounding.
Signed-off-by: Lucas Stach
Reviewed-by: Philipp Zabel
Signed-off-by: Philipp Zabel
---
No changes since v1 [1].
[1] https://patchwork.linuxtv.org/patch/40604
---
drivers/media/platform/coda/coda-bit.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 2ec41375a896f..325035bb0a777 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -427,14 +427,16 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
/* Register frame buffers in the parameter buffer */
for (i = 0; i < ctx->num_internal_frames; i++) {
- u32 y, cb, cr;
+ u32 y, cb, cr, mvcol;
/* Start addresses of Y, Cb, Cr planes */
y = ctx->internal_frames[i].paddr;
cb = y + ysize;
cr = y + ysize + ysize/4;
+ mvcol = y + ysize + ysize/4 + ysize/4;
if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP) {
cb = round_up(cb, 4096);
+ mvcol = cb + ysize/2;
cr = 0;
/* Packed 20-bit MSB of base addresses */
/* YCCC, CCyc, */
@@ -448,9 +450,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
/* mvcol buffer for h.264 */
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 &&
dev->devtype->product != CODA_DX6)
- coda_parabuf_write(ctx, 96 + i,
- ctx->internal_frames[i].paddr +
- ysize + ysize/4 + ysize/4);
+ coda_parabuf_write(ctx, 96 + i, mvcol);
}
/* mvcol buffer for mpeg4 */
--
2.11.0
[PATCH v2 2/3] [media] coda: first step at error recovery
From: Lucas Stach
This implements a simple handler for the case where decode did not finish
successfully. This might be helpful during normal streaming, but for now it
only handles the case where the context would deadlock with userspace,
i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
decode run we would hold the context and wait for userspace to queue more
buffers.
Signed-off-by: Lucas Stach
Reviewed-by: Philipp Zabel
[p.za...@pengutronix.de: renamed error_decode/run to run/decode_timeout]
Signed-off-by: Philipp Zabel
---
Changes since v1 [1]:
- Rename error_run/decode callback to run/decode_timeout, as
this error handler is called on device_run timeouts only.
[1] https://patchwork.linuxtv.org/patch/40603
---
drivers/media/platform/coda/coda-bit.c| 20
drivers/media/platform/coda/coda-common.c | 3 +++
drivers/media/platform/coda/coda.h| 1 +
3 files changed, 24 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 325035bb0a777..795b6d7584320 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2198,12 +2198,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
ctx->display_idx = display_idx;
}
+static void coda_decode_timeout(struct coda_ctx *ctx)
+{
+ struct vb2_v4l2_buffer *dst_buf;
+
+ /*
+* For now this only handles the case where we would deadlock with
+* userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
+* but after a failed decode run we would hold the context and wait for
+* userspace to queue more buffers.
+*/
+ if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
+ return;
+
+ dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+ dst_buf->sequence = ctx->qsequence - 1;
+
+ coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
+}
+
const struct coda_context_ops coda_bit_decode_ops = {
.queue_init = coda_decoder_queue_init,
.reqbufs = coda_decoder_reqbufs,
.start_streaming = coda_start_decoding,
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
+ .run_timeout = coda_decode_timeout,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
};
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 78bd9a4ace0e4..829c7895a98a2 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1163,6 +1163,9 @@ static void coda_pic_run_work(struct work_struct *work)
ctx->hold = true;
coda_hw_reset(ctx);
+
+ if (ctx->ops->run_timeout)
+ ctx->ops->run_timeout(ctx);
} else if (!ctx->aborting) {
ctx->ops->finish_run(ctx);
}
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 76d059431ca13..c5f504d8cf67f 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -183,6 +183,7 @@ struct coda_context_ops {
int (*start_streaming)(struct coda_ctx *ctx);
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
+ void (*run_timeout)(struct coda_ctx *ctx);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
};
--
2.11.0
[PATCH v2 3/3] [media] coda/imx-vdoa: always wait for job completion
From: Lucas Stach
As long as only one CODA context is running we get alternating
device_run() and wait_for_completion() calls, but when more then one
CODA context is active, other VDOA slots can be inserted between those
calls for one context.
Make sure to wait on job completion before running a different context
and before destroying the currently active context.
Signed-off-by: Lucas Stach
Reviewed-by: Philipp Zabel
Signed-off-by: Philipp Zabel
---
No changes since v1 [1].
[1] https://patchwork.linuxtv.org/patch/40605
---
drivers/media/platform/coda/imx-vdoa.c | 49 +++---
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/coda/imx-vdoa.c
b/drivers/media/platform/coda/imx-vdoa.c
index 669a4c82f1ffa..df9b71621420c 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -101,6 +101,8 @@ struct vdoa_ctx {
struct vdoa_data*vdoa;
struct completion completion;
struct vdoa_q_data q_data[2];
+ unsigned intsubmitted_job;
+ unsigned intcompleted_job;
};
static irqreturn_t vdoa_irq_handler(int irq, void *data)
@@ -114,7 +116,7 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
curr_ctx = vdoa->curr_ctx;
if (!curr_ctx) {
- dev_dbg(vdoa->dev,
+ dev_warn(vdoa->dev,
"Instance released before the end of transaction\n");
return IRQ_HANDLED;
}
@@ -127,19 +129,44 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
} else if (!(val & VDOAIST_EOT)) {
dev_warn(vdoa->dev, "Spurious interrupt\n");
}
+ curr_ctx->completed_job++;
complete(&curr_ctx->completion);
return IRQ_HANDLED;
}
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+ struct vdoa_data *vdoa = ctx->vdoa;
+
+ if (ctx->submitted_job == ctx->completed_job)
+ return 0;
+
+ if (!wait_for_completion_timeout(&ctx->completion,
+msecs_to_jiffies(300))) {
+ dev_err(vdoa->dev,
+ "Timeout waiting for transfer result\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(vdoa_wait_for_completion);
+
void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
{
struct vdoa_q_data *src_q_data, *dst_q_data;
struct vdoa_data *vdoa = ctx->vdoa;
u32 val;
+ if (vdoa->curr_ctx)
+ vdoa_wait_for_completion(vdoa->curr_ctx);
+
vdoa->curr_ctx = ctx;
+ reinit_completion(&ctx->completion);
+ ctx->submitted_job++;
+
src_q_data = &ctx->q_data[V4L2_M2M_SRC];
dst_q_data = &ctx->q_data[V4L2_M2M_DST];
@@ -177,21 +204,6 @@ void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst,
dma_addr_t src)
}
EXPORT_SYMBOL(vdoa_device_run);
-int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
-{
- struct vdoa_data *vdoa = ctx->vdoa;
-
- if (!wait_for_completion_timeout(&ctx->completion,
-msecs_to_jiffies(300))) {
- dev_err(vdoa->dev,
- "Timeout waiting for transfer result\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-EXPORT_SYMBOL(vdoa_wait_for_completion);
-
struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
{
struct vdoa_ctx *ctx;
@@ -218,6 +230,11 @@ void vdoa_context_destroy(struct vdoa_ctx *ctx)
{
struct vdoa_data *vdoa = ctx->vdoa;
+ if (vdoa->curr_ctx == ctx) {
+ vdoa_wait_for_completion(vdoa->curr_ctx);
+ vdoa->curr_ctx = NULL;
+ }
+
clk_disable_unprepare(vdoa->vdoa_clk);
kfree(ctx);
}
--
2.11.0
Re: [PATCH] [media] imx: switch from V4L2 OF to V4L2 fwnode API
On Tue, 2017-06-06 at 18:00 -0700, Steve Longerbeam wrote: > Hi Philipp, > > v4l2_fwnode patch has been merged to mediatree, so I've applied this > to my imx-media-staging-md-v16 branch, thanks for the patch! > > However before I can submit version 8 of the patchset, the video-mux > driver also needs conversion. Can you submit a version 8 of your > video-mux patchset (your last was v7) containing the switch to > v4l2-fwnode, and I will incorporate into the imx-media v8 patchset. > > Thanks! > Steve I removed the V4L2 OF API dependency from the video-mux driver, so it works unchanged after the V4L2 fwnode API merge. But I had not yet sent v8 with the documentation changes suggested by Sakari. Fixed now. regards Philipp
[PATCH v8 3/3] [media] platform: add video-multiplexer subdevice driver
This driver can handle SoC internal and external video bus multiplexers,
controlled by mux controllers provided by the mux controller framework,
such as MMIO register bitfields or GPIOs. The subdevice passes through
the mbus configuration of the active input to the output side.
Since the mux framework is not yet merged, this driver contains
temporary mmio-mux support to work without the framework. The driver
should be converted to use the multiplexer API once the "mux: minimal
mux subsystem" and "mux: mmio-based syscon mux controller" patches are
merged.
Signed-off-by: Sascha Hauer
Signed-off-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
Acked-by: Pavel Machek
---
No changes since v7 [1]. No changes necessary for the recently merged V4L2
fwnode API.
[1] https://patchwork.linuxtv.org/patch/41537/
---
drivers/media/platform/Kconfig | 6 +
drivers/media/platform/Makefile| 2 +
drivers/media/platform/video-mux.c | 334 +
3 files changed, 342 insertions(+)
create mode 100644 drivers/media/platform/video-mux.c
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index a4b7cefbde32d..23dfec53dc736 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -74,6 +74,12 @@ config VIDEO_M32R_AR_M64278
To compile this driver as a module, choose M here: the
module will be called arv.
+config VIDEO_MUX
+ tristate "Video Multiplexer"
+ depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
+ help
+ This driver provides support for N:1 video bus multiplexers.
+
config VIDEO_OMAP3
tristate "OMAP 3 Camera support"
depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 231f3c2894c9d..a9ff99d595b86 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_VIDEO_SH_VEU)+= sh_veu.o
obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)+= m2m-deinterlace.o
+obj-$(CONFIG_VIDEO_MUX)+= video-mux.o
+
obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif/
obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
diff --git a/drivers/media/platform/video-mux.c
b/drivers/media/platform/video-mux.c
new file mode 100644
index 0..665744716f73b
--- /dev/null
+++ b/drivers/media/platform/video-mux.c
@@ -0,0 +1,334 @@
+/*
+ * video stream multiplexer controlled via mux control
+ *
+ * Copyright (C) 2013 Pengutronix, Sascha Hauer
+ * Copyright (C) 2016-2017 Pengutronix, Philipp Zabel
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct video_mux {
+ struct v4l2_subdev subdev;
+ struct media_pad *pads;
+ struct v4l2_mbus_framefmt *format_mbus;
+ struct regmap_field *field;
+ struct mutex lock;
+ int active;
+};
+
+static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev
*sd)
+{
+ return container_of(sd, struct video_mux, subdev);
+}
+
+static int video_mux_link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+ int ret = 0;
+
+ /*
+* The mux state is determined by the enabled sink pad link.
+* Enabling or disabling the source pad link has no effect.
+*/
+ if (local->flags & MEDIA_PAD_FL_SOURCE)
+ return 0;
+
+ dev_dbg(sd->dev, "link setup '%s':%d->'%s':%d[%d]",
+ remote->entity->name, remote->index, local->entity->name,
+ local->index, flags & MEDIA_LNK_FL_ENABLED);
+
+ mutex_lock(&vmux->lock);
+
+ if (flags & MEDIA_LNK_FL_ENABLED) {
+ if (vmux->active == local->index)
+ goto out;
+
+ if (vmux->active >= 0) {
+ ret = -EBUSY;
+
[PATCH v8 2/3] [media] add mux and video interface bridge entity functions
Add two new media entity function definitions for video multiplexers
and video interface bridges.
Signed-off-by: Philipp Zabel
- renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX
Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
---
Changes since v7 [1]:
- Drop a redundant sentence.
- Clarify number of video interface bridge source pads.
[1] https://patchwork.linuxtv.org/patch/41536/
---
Documentation/media/uapi/mediactl/media-types.rst | 21 +
include/uapi/linux/media.h| 6 ++
2 files changed, 27 insertions(+)
diff --git a/Documentation/media/uapi/mediactl/media-types.rst
b/Documentation/media/uapi/mediactl/media-types.rst
index 2a5164aea2b40..71078565d6444 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -299,6 +299,27 @@ Types and flags used to represent the media graph elements
received on its sink pad and outputs the statistics data on
its source pad.
+- .. row 29
+
+ .. _MEDIA-ENT-F-VID-MUX:
+
+ - ``MEDIA_ENT_F_VID_MUX``
+
+ - Video multiplexer. An entity capable of multiplexing must have at
+ least two sink pads and one source pad, and must pass the video
+ frame(s) received from the active sink pad to the source pad.
+
+- .. row 30
+
+ .. _MEDIA-ENT-F-VID-IF-BRIDGE:
+
+ - ``MEDIA_ENT_F_VID_IF_BRIDGE``
+
+ - Video interface bridge. A video interface bridge entity must have at
+ least one sink pad and at least one source pad. It receives video
+ frames on its sink pad from an input video bus of one type (HDMI, eDP,
+ MIPI CSI-2, ...), and outputs them on its source pad to an output
+ video bus of another type (eDP, MIPI CSI-2, parallel, ...).
.. tabularcolumns:: |p{5.5cm}|p{12.0cm}|
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4890787731b85..fac96c64fe513 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -105,6 +105,12 @@ struct media_device_info {
#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
/*
+ * Switch and bridge entitites
+ */
+#define MEDIA_ENT_F_VID_MUX(MEDIA_ENT_F_BASE + 0x5001)
+#define MEDIA_ENT_F_VID_IF_BRIDGE (MEDIA_ENT_F_BASE + 0x5002)
+
+/*
* Connectors
*/
/* It is a responsibility of the entity drivers to add connectors and links */
--
2.11.0
[PATCH v8 1/3] dt-bindings: Add bindings for video-multiplexer device
Add bindings documentation for the video multiplexer device.
Signed-off-by: Sascha Hauer
Signed-off-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
Reviewed-by: Sebastian Reichel
Acked-by: Rob Herring
Acked-by: Pavel Machek
---
No changes since v7 [1].
[1] https://patchwork.linuxtv.org/patch/41535/
---
.../devicetree/bindings/media/video-mux.txt| 60 ++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/video-mux.txt
diff --git a/Documentation/devicetree/bindings/media/video-mux.txt
b/Documentation/devicetree/bindings/media/video-mux.txt
new file mode 100644
index 0..63b9dc913e456
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-mux.txt
@@ -0,0 +1,60 @@
+Video Multiplexer
+=
+
+Video multiplexers allow to select between multiple input ports. Video received
+on the active input port is passed through to the output port. Muxes described
+by this binding are controlled by a multiplexer controller that is described by
+the bindings in Documentation/devicetree/bindings/mux/mux-controller.txt
+
+Required properties:
+- compatible : should be "video-mux"
+- mux-controls : mux controller node to use for operating the mux
+- #address-cells: should be <1>
+- #size-cells: should be <0>
+- port@*: at least three port nodes containing endpoints connecting to the
+ source and sink devices according to of_graph bindings. The last port is
+ the output port, all others are inputs.
+
+Optionally, #address-cells, #size-cells, and port nodes can be grouped under a
+ports node as described in Documentation/devicetree/bindings/graph.txt.
+
+Example:
+
+ mux: mux-controller {
+ compatible = "gpio-mux";
+ #mux-control-cells = <0>;
+
+ mux-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>;
+ };
+
+ video-mux {
+ compatible = "video-mux";
+ mux-controls = <&mux>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ mux_in0: endpoint {
+ remote-endpoint = <&video_source0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ mux_in1: endpoint {
+ remote-endpoint = <&video_source1_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ mux_out: endpoint {
+ remote-endpoint = <&capture_interface_in>;
+ };
+ };
+ };
+};
--
2.11.0
[PATCH 2/2] [media] coda: copy headers in front of every I-frame
That way we don't have to rely on userspace to inject the headers on IDR
requests, and there is always enough information to start decoding at an
I-frame.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 22e4630f36711..2ec41375a896f 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1270,10 +1270,10 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
coda_set_gdi_regs(ctx);
/*
-* Copy headers at the beginning of the first frame for H.264 only.
-* In MPEG4 they are already copied by the coda.
+* Copy headers in front of the first frame and forced I frames for
+* H.264 only. In MPEG4 they are already copied by the CODA.
*/
- if (src_buf->sequence == 0) {
+ if (src_buf->sequence == 0 || force_ipicture) {
pic_stream_buffer_addr =
vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0) +
ctx->vpu_header_size[0] +
@@ -1386,7 +1386,8 @@ static void coda_finish_encode(struct coda_ctx *ctx)
wr_ptr = coda_read(dev, CODA_REG_BIT_WR_PTR(ctx->reg_idx));
/* Calculate bytesused field */
- if (dst_buf->sequence == 0) {
+ if (dst_buf->sequence == 0 ||
+ src_buf->flags & V4L2_BUF_FLAG_KEYFRAME) {
vb2_set_plane_payload(&dst_buf->vb2_buf, 0, wr_ptr - start_ptr +
ctx->vpu_header_size[0] +
ctx->vpu_header_size[1] +
--
2.11.0
[PATCH 1/2] [media] coda: implement forced key frames
Implement the V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME control to force IDR
frames. This is useful to implement VFU (Video Fast Update) on RTP
transmissions.
We already force an IDR frame at the beginning of each GOP to work
around a firmware bug on i.MX27, use the same mechanism to service IDR
requests from userspace.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 12
drivers/media/platform/coda/coda-common.c | 3 +++
drivers/media/platform/coda/coda.h| 1 +
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 403214e00e954..22e4630f36711 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1247,12 +1247,18 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
dst_buf->sequence = ctx->osequence;
ctx->osequence++;
+ force_ipicture = ctx->params.force_ipicture;
+ if (force_ipicture)
+ ctx->params.force_ipicture = false;
+ else if ((src_buf->sequence % ctx->params.gop_size) == 0)
+ force_ipicture = 1;
+
/*
* Workaround coda firmware BUG that only marks the first
* frame as IDR. This is a problem for some decoders that can't
* recover when a frame is lost.
*/
- if (src_buf->sequence % ctx->params.gop_size) {
+ if (!force_ipicture) {
src_buf->flags |= V4L2_BUF_FLAG_PFRAME;
src_buf->flags &= ~V4L2_BUF_FLAG_KEYFRAME;
} else {
@@ -1291,8 +1297,7 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
pic_stream_buffer_size = q_data_dst->sizeimage;
}
- if (src_buf->flags & V4L2_BUF_FLAG_KEYFRAME) {
- force_ipicture = 1;
+ if (force_ipicture) {
switch (dst_fourcc) {
case V4L2_PIX_FMT_H264:
quant_param = ctx->params.h264_intra_qp;
@@ -1309,7 +1314,6 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
break;
}
} else {
- force_ipicture = 0;
switch (dst_fourcc) {
case V4L2_PIX_FMT_H264:
quant_param = ctx->params.h264_inter_qp;
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index d523e990d5093..f3fe4adf21a7e 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1680,6 +1680,9 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB:
ctx->params.intra_refresh = ctrl->val;
break;
+ case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
+ ctx->params.force_ipicture = true;
+ break;
case V4L2_CID_JPEG_COMPRESSION_QUALITY:
coda_set_jpeg_compression_quality(ctx, ctrl->val);
break;
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 20222befb9b2f..dccb105a1a384 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -135,6 +135,7 @@ struct coda_params {
u32 vbv_size;
u32 slice_max_bits;
u32 slice_max_mb;
+ boolforce_ipicture;
};
struct coda_buffer_meta {
--
2.11.0
Re: [PATCH 0/3] tc358743: minor driver fixes
Hi Dave, On Fri, 2017-06-02 at 15:36 +0100, Dave Stevenson wrote: [...] > >> > Are you aware of the HDMI modes that the TC358743 driver has been used > >> > with? > >> > The comments mention 720P60 at 594MHz, but I have had to modify the > >> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The > >> > value came out of Toshiba's spreadsheet for computing register > >> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes, > >> > so not a huge change. > >> > Is it worth going to the effort of dynamically computing the delay, or > >> > is increasing the default acceptable? > >> > >> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so > >> I have CC-ed him as I am not sure where those values came from. > > > > I've just chosen a small delay that worked reliably. For 4-lane 1080p60 > > and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet > > believes that it is ok to decrease the FIFO delay all the way down to 0 > > (it is not). I think it should be fine to delay transmission for a few > > microseconds unconditionally, I'll test this next week. > > Thanks Philipp. Were 1080p60 and 720p60 the only modes you were really > testing? Yes. Since 720p60 needs half the bandwidth of 1080p60, it was possible to just use the 1080p60 timings by switching from 4 to 2 lanes. > Did the 594Mbps lane speed come from a specific requirement somewhere? It is what the spreadsheed suggests for 4-lane 1080p60, I assume because it is nice and even to generate from the 27 MHz reference clock, and it fits the 547.28 Mbps/lane requirement (for YCbCr 4:2:2 CSI output) with a bit of headroom. > The standard Pi only has 2 CSI2 lanes exposed, and 1080P30 RGB3 just > tips over into needing 3 lanes with the current link frequency. > When I can find a bit more time I was thinking that an alternate link > frequency would allow us to squeeze it in to 2 lanes. Obviously the > timing values need to be checked carefully, but it should all work and > allow support of multiple link frequencies. See the FIXME comment in tc358743_probe_of, you could calculate and add the correct pdata for the raised link-frequency. > (My calcs say that 1080p50 UYVY can fit down 2 lanes, but that > requires more extensive register mods). With a lane rate of 911.25 Mbps or higher that should be possible, with all the CSI timings are relaxed a bit. regards Philipp
Re: [PATCH 0/3] tc358743: minor driver fixes
On Fri, 2017-06-02 at 15:27 +0200, Hans Verkuil wrote:
> On 06/02/17 15:03, Dave Stevenson wrote:
> > Hi Hans.
> >
> > On 2 June 2017 at 13:35, Hans Verkuil wrote:
> >> On 06/02/17 14:18, Dave Stevenson wrote:
> >>> These 3 patches for TC358743 came out of trying to use the
> >>> existing driver with a new Raspberry Pi CSI-2 receiver driver.
> >>
> >> Nice! Doing that has been on my todo list for ages but I never got
> >> around to it. I have one of these and using the Raspberry Pi with
> >> the tc358743 would allow me to add a CEC driver as well.
> >
> > It's been on my list for a while too! It's working, but just the final
> > clean ups needed.
> > I've got 1 v4l2-compliance failure still outstanding that needs
> > digging into (subscribe_event), rebasing on top of the fwnode tree,
> > and a couple of config things to tidy up. RFC hopefully next week.
> > I'm testing with a demo board designed here at Pi Towers, but there
> > are others successfully testing it using the auvidea.com B101 board.
> >
> > Are you aware of the HDMI modes that the TC358743 driver has been used with?
> > The comments mention 720P60 at 594MHz, but I have had to modify the
> > fifo_level value from 16 to 110 to get VGA60 or 576P50 to work. (The
> > value came out of Toshiba's spreadsheet for computing register
> > settings). It increases the delay by 2.96usecs at 720P60 on 2 lanes,
> > so not a huge change.
> > Is it worth going to the effort of dynamically computing the delay, or
> > is increasing the default acceptable?
>
> I see that the fifo_level value of 16 was supplied by Philipp Zabel, so
> I have CC-ed him as I am not sure where those values came from.
I've just chosen a small delay that worked reliably. For 4-lane 1080p60
and for 2-lane 720p60 at 594 Mbps lane speed, the Toshiba spreadsheet
believes that it is ok to decrease the FIFO delay all the way down to 0
(it is not). I think it should be fine to delay transmission for a few
microseconds unconditionally, I'll test this next week.
> This driver is also used in a Cisco product, but we use platform_data for
> that.
> Here are our settings that we use for reference:
>
> static struct tc358743_platform_data tc358743_pdata = {
> .refclk_hz = 2700,
> .ddc5v_delay = DDC5V_DELAY_100_MS,
> .fifo_level = 300,
> .pll_prd = 4,
> .pll_fbd = 122,
> /* CSI */
> .lineinitcnt = 0x1770,
> .lptxtimecnt = 0x0005,
> .tclk_headercnt = 0x1d04,
> .ths_headercnt = 0x0505,
> .twakeup = 0x4650,
> .ths_trailcnt = 0x0004,
> .hstxvregcnt = 0x0005,
> /* HDMI PHY */
> .hdmi_phy_auto_reset_tmds_detected = true,
> .hdmi_phy_auto_reset_tmds_in_range = true,
> .hdmi_phy_auto_reset_tmds_valid = true,
> .hdmi_phy_auto_reset_hsync_out_of_range = true,
> .hdmi_phy_auto_reset_vsync_out_of_range = true,
> .hdmi_detection_delay = HDMI_MODE_DELAY_25_MS,
> };
>
> I believe these are all calculated from the Toshiba spreadsheet.
>
> Frankly, I have no idea what they mean :-)
>
> I am fine with increasing the default if Philipp is OK as well. Since
> Cisco uses a value of 300 I would expect that 16 is indeed too low.
>
> Regards,
>
> Hans
>
> >
> >>> A couple of the subdevice API calls were not implemented or
> >>> otherwise gave odd results. Those are fixed.
> >>>
> >>> The TC358743 interface board being used didn't have the IRQ
> >>> line wired up to the SoC. "interrupts" is listed as being
> >>> optional in the DT binding, but the driver didn't actually
> >>> function if it wasn't provided.
> >>>
> >>> Dave Stevenson (3):
> >>> [media] tc358743: Add enum_mbus_code
> >>> [media] tc358743: Setup default mbus_fmt before registering
> >>> [media] tc358743: Add support for platforms without IRQ line
> >>
> >> All looks good, I'll take this for 4.12.
> >
> > Thanks.
> >
> >> Regards,
> >>
> >> Hans
> >>
> >>>
> >>> drivers/media/i2c/tc358743.c | 59
> >>> +++-
> >>> 1 file changed, 58 insertions(+), 1 deletion(-)
> >>>
> >>
regards
Philipp
Re: [PATCH v4 1/4] media-ctl: add pad support to set/get_frame_interval
Hi,
On Thu, 2017-03-30 at 18:51 +0200, Philipp Zabel wrote:
> This allows to set and get the frame interval on pads other than pad 0.
>
> Signed-off-by: Philipp Zabel
> Acked-by: Sakari Ailus
any more comments on these?
regards
Philipp
> ---
> utils/media-ctl/libv4l2subdev.c | 24 ++--
> utils/media-ctl/v4l2subdev.h| 4 ++--
> 2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 3dcf943c..2f2ac8ee 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -262,7 +262,8 @@ int v4l2_subdev_set_dv_timings(struct media_entity
> *entity,
> }
>
> int v4l2_subdev_get_frame_interval(struct media_entity *entity,
> -struct v4l2_fract *interval)
> +struct v4l2_fract *interval,
> +unsigned int pad)
> {
> struct v4l2_subdev_frame_interval ival;
> int ret;
> @@ -272,6 +273,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity
> *entity,
> return ret;
>
> memset(&ival, 0, sizeof(ival));
> + ival.pad = pad;
>
> ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
> if (ret < 0)
> @@ -282,7 +284,8 @@ int v4l2_subdev_get_frame_interval(struct media_entity
> *entity,
> }
>
> int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> -struct v4l2_fract *interval)
> +struct v4l2_fract *interval,
> +unsigned int pad)
> {
> struct v4l2_subdev_frame_interval ival;
> int ret;
> @@ -292,6 +295,7 @@ int v4l2_subdev_set_frame_interval(struct media_entity
> *entity,
> return ret;
>
> memset(&ival, 0, sizeof(ival));
> + ival.pad = pad;
> ival.interval = *interval;
>
> ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
> @@ -617,7 +621,7 @@ static int set_selection(struct media_pad *pad, unsigned
> int target,
> return 0;
> }
>
> -static int set_frame_interval(struct media_entity *entity,
> +static int set_frame_interval(struct media_pad *pad,
> struct v4l2_fract *interval)
> {
> int ret;
> @@ -625,20 +629,20 @@ static int set_frame_interval(struct media_entity
> *entity,
> if (interval->numerator == 0)
> return 0;
>
> - media_dbg(entity->media,
> - "Setting up frame interval %u/%u on entity %s\n",
> + media_dbg(pad->entity->media,
> + "Setting up frame interval %u/%u on pad %s/%u\n",
> interval->numerator, interval->denominator,
> - entity->info.name);
> + pad->entity->info.name, pad->index);
>
> - ret = v4l2_subdev_set_frame_interval(entity, interval);
> + ret = v4l2_subdev_set_frame_interval(pad->entity, interval, pad->index);
> if (ret < 0) {
> - media_dbg(entity->media,
> + media_dbg(pad->entity->media,
> "Unable to set frame interval: %s (%d)",
> strerror(-ret), ret);
> return ret;
> }
>
> - media_dbg(entity->media, "Frame interval set: %u/%u\n",
> + media_dbg(pad->entity->media, "Frame interval set: %u/%u\n",
> interval->numerator, interval->denominator);
>
> return 0;
> @@ -685,7 +689,7 @@ static int v4l2_subdev_parse_setup_format(struct
> media_device *media,
> return ret;
> }
>
> - ret = set_frame_interval(pad->entity, &interval);
> + ret = set_frame_interval(pad, &interval);
> if (ret < 0)
> return ret;
>
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index 9c8fee89..413094d5 100644
> --- a/utils/media-ctl/v4l2subdev.h
> +++ b/utils/media-ctl/v4l2subdev.h
> @@ -200,7 +200,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity
> *entity,
> */
>
> int v4l2_subdev_get_frame_interval(struct media_entity *entity,
> - struct v4l2_fract *interval);
> + struct v4l2_fract *interval, unsigned int pad);
>
> /**
> * @brief Set the frame interval on a sub-device.
> @@ -217,7 +217,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity
> *entity,
> * @return 0 on success, or a negative error code on failure.
> */
> int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> - struct v4l2_fract *interval);
> + struct v4l2_fract *interval, unsigned int pad);
>
> /**
> * @brief Parse a string and apply format, crop and frame interval settings.
Re: [PATCH v7 2/3] [media] add mux and video interface bridge entity functions
Hi Sakari, On Mon, 2017-05-29 at 23:38 +0300, Sakari Ailus wrote: [...] > > diff --git a/Documentation/media/uapi/mediactl/media-types.rst > > b/Documentation/media/uapi/mediactl/media-types.rst > > index 2a5164aea2b40..1d15542f447c1 100644 > > --- a/Documentation/media/uapi/mediactl/media-types.rst > > +++ b/Documentation/media/uapi/mediactl/media-types.rst > > @@ -299,6 +299,28 @@ Types and flags used to represent the media graph > > elements > > received on its sink pad and outputs the statistics data on > > its source pad. > > > > +- .. row 29 > > + > > + .. _MEDIA-ENT-F-VID-MUX: > > + > > + - ``MEDIA_ENT_F_VID_MUX`` > > + > > + - Video multiplexer. An entity capable of multiplexing must have at > > + least two sink pads and one source pad, and must pass the video > > + frame(s) received from the active sink pad to the source pad. > > Video > > + frame(s) from the inactive sink pads are discarded. > > I don't think the last sentence is needed, I'd drop it as redundant. Up to > you. Thanks, I'll drop this sentence ... > > + > > +- .. row 30 > > + > > + .. _MEDIA-ENT-F-VID-IF-BRIDGE: > > + > > + - ``MEDIA_ENT_F_VID_IF_BRIDGE`` > > + > > + - Video interface bridge. A video interface bridge entity must have > > at > > + least one sink pad and one source pad. It receives video frames on > > It's not clear whether there must be at least one source pad or one source > pad. How about either: > > "must have at least one sink pad and at least one source pad" or ... and change this to specify "at least one source pad". > "must have at least one sink pad and exactly one source pad"? > > With this considered, > > Acked-by: Sakari Ailus [...] regards Philipp
[PATCH v7 1/3] dt-bindings: Add bindings for video-multiplexer device
Add bindings documentation for the video multiplexer device.
Signed-off-by: Sascha Hauer
Signed-off-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
Reviewed-by: Sebastian Reichel
Acked-by: Rob Herring
Acked-by: Pavel Machek
---
No changes since [1].
[1] https://patchwork.linuxtv.org/patch/41475/
---
.../devicetree/bindings/media/video-mux.txt| 60 ++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/video-mux.txt
diff --git a/Documentation/devicetree/bindings/media/video-mux.txt
b/Documentation/devicetree/bindings/media/video-mux.txt
new file mode 100644
index 0..63b9dc913e456
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/video-mux.txt
@@ -0,0 +1,60 @@
+Video Multiplexer
+=
+
+Video multiplexers allow to select between multiple input ports. Video received
+on the active input port is passed through to the output port. Muxes described
+by this binding are controlled by a multiplexer controller that is described by
+the bindings in Documentation/devicetree/bindings/mux/mux-controller.txt
+
+Required properties:
+- compatible : should be "video-mux"
+- mux-controls : mux controller node to use for operating the mux
+- #address-cells: should be <1>
+- #size-cells: should be <0>
+- port@*: at least three port nodes containing endpoints connecting to the
+ source and sink devices according to of_graph bindings. The last port is
+ the output port, all others are inputs.
+
+Optionally, #address-cells, #size-cells, and port nodes can be grouped under a
+ports node as described in Documentation/devicetree/bindings/graph.txt.
+
+Example:
+
+ mux: mux-controller {
+ compatible = "gpio-mux";
+ #mux-control-cells = <0>;
+
+ mux-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>;
+ };
+
+ video-mux {
+ compatible = "video-mux";
+ mux-controls = <&mux>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ mux_in0: endpoint {
+ remote-endpoint = <&video_source0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ mux_in1: endpoint {
+ remote-endpoint = <&video_source1_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ mux_out: endpoint {
+ remote-endpoint = <&capture_interface_in>;
+ };
+ };
+ };
+};
--
2.11.0
[PATCH v7 3/3] [media] platform: add video-multiplexer subdevice driver
This driver can handle SoC internal and external video bus multiplexers,
controlled by mux controllers provided by the mux controller framework,
such as MMIO register bitfields or GPIOs. The subdevice passes through
the mbus configuration of the active input to the output side.
Since the mux framework is not yet merged, this driver contains
temporary mmio-mux support to work without the framework. The driver
should be converted to use the multiplexer API once the "mux: minimal
mux subsystem" and "mux: mmio-based syscon mux controller" patches are
merged.
Signed-off-by: Sascha Hauer
Signed-off-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Acked-by: Sakari Ailus
Acked-by: Pavel Machek
---
Changes since [1]:
- Merge the temporary mmio-mux code into this patch and remove the
as of now unused mux API code. Another patch will be sent later to
convert this driver back to the mux framework, once it is merged.
[1] https://patchwork.linuxtv.org/patch/41461/
---
drivers/media/platform/Kconfig | 6 +
drivers/media/platform/Makefile| 2 +
drivers/media/platform/video-mux.c | 334 +
3 files changed, 342 insertions(+)
create mode 100644 drivers/media/platform/video-mux.c
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ac026ee1ca074..fea1dc05ea7b7 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -74,6 +74,12 @@ config VIDEO_M32R_AR_M64278
To compile this driver as a module, choose M here: the
module will be called arv.
+config VIDEO_MUX
+ tristate "Video Multiplexer"
+ depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
+ help
+ This driver provides support for N:1 video bus multiplexers.
+
config VIDEO_OMAP3
tristate "OMAP 3 Camera support"
depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 63303d63c64cf..a6363023f981e 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_VIDEO_SH_VEU)+= sh_veu.o
obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)+= m2m-deinterlace.o
+obj-$(CONFIG_VIDEO_MUX)+= video-mux.o
+
obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif/
obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
diff --git a/drivers/media/platform/video-mux.c
b/drivers/media/platform/video-mux.c
new file mode 100644
index 0..665744716f73b
--- /dev/null
+++ b/drivers/media/platform/video-mux.c
@@ -0,0 +1,334 @@
+/*
+ * video stream multiplexer controlled via mux control
+ *
+ * Copyright (C) 2013 Pengutronix, Sascha Hauer
+ * Copyright (C) 2016-2017 Pengutronix, Philipp Zabel
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct video_mux {
+ struct v4l2_subdev subdev;
+ struct media_pad *pads;
+ struct v4l2_mbus_framefmt *format_mbus;
+ struct regmap_field *field;
+ struct mutex lock;
+ int active;
+};
+
+static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev
*sd)
+{
+ return container_of(sd, struct video_mux, subdev);
+}
+
+static int video_mux_link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+ int ret = 0;
+
+ /*
+* The mux state is determined by the enabled sink pad link.
+* Enabling or disabling the source pad link has no effect.
+*/
+ if (local->flags & MEDIA_PAD_FL_SOURCE)
+ return 0;
+
+ dev_dbg(sd->dev, "link setup '%s':%d->'%s':%d[%d]",
+ remote->entity->name, remote->index, local->entity->name,
+ local->index, flags & MEDIA_LNK_FL_ENABLED);
+
+ mutex_lock(&vmux->lock);
+
+ if (flags & MEDIA_LNK_FL_ENABLED) {
+ if (vmux->active == lo
[PATCH v7 2/3] [media] add mux and video interface bridge entity functions
Signed-off-by: Philipp Zabel
- renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX
Signed-off-by: Steve Longerbeam
---
Changes since [1]:
- Reword the video interface bridge description, do not use "bus format" to
avoid confiusion with media bus formats.
[1] https://patchwork.linuxtv.org/patch/41467/
---
Documentation/media/uapi/mediactl/media-types.rst | 22 ++
include/uapi/linux/media.h| 6 ++
2 files changed, 28 insertions(+)
diff --git a/Documentation/media/uapi/mediactl/media-types.rst
b/Documentation/media/uapi/mediactl/media-types.rst
index 2a5164aea2b40..1d15542f447c1 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -299,6 +299,28 @@ Types and flags used to represent the media graph elements
received on its sink pad and outputs the statistics data on
its source pad.
+- .. row 29
+
+ .. _MEDIA-ENT-F-VID-MUX:
+
+ - ``MEDIA_ENT_F_VID_MUX``
+
+ - Video multiplexer. An entity capable of multiplexing must have at
+ least two sink pads and one source pad, and must pass the video
+ frame(s) received from the active sink pad to the source pad. Video
+ frame(s) from the inactive sink pads are discarded.
+
+- .. row 30
+
+ .. _MEDIA-ENT-F-VID-IF-BRIDGE:
+
+ - ``MEDIA_ENT_F_VID_IF_BRIDGE``
+
+ - Video interface bridge. A video interface bridge entity must have at
+ least one sink pad and one source pad. It receives video frames on
+ its sink pad from an input video bus of one type (HDMI, eDP, MIPI
+ CSI-2, ...), and outputs them on its source pad to an output video bus
+ of another type (eDP, MIPI CSI-2, parallel, ...).
.. tabularcolumns:: |p{5.5cm}|p{12.0cm}|
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4890787731b85..fac96c64fe513 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -105,6 +105,12 @@ struct media_device_info {
#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
/*
+ * Switch and bridge entitites
+ */
+#define MEDIA_ENT_F_VID_MUX(MEDIA_ENT_F_BASE + 0x5001)
+#define MEDIA_ENT_F_VID_IF_BRIDGE (MEDIA_ENT_F_BASE + 0x5002)
+
+/*
* Connectors
*/
/* It is a responsibility of the entity drivers to add connectors and links */
--
2.11.0
Re: [PATCH v7 00/34] i.MX Media Driver
On Mon, 2017-05-29 at 15:46 +0200, Hans Verkuil wrote: > Hi Steve, > > On 05/25/2017 02:29 AM, Steve Longerbeam wrote: > > In version 7: > > > > - video-mux: switched to Philipp's latest video-mux driver and updated > >bindings docs, that makes use of the mmio-mux framework. > > > > - mmio-mux: includes Philipp's temporary patch that adds mmio-mux support > >to video-mux driver, until mux framework is merged. > > > > - mmio-mux: updates to device tree from Philipp that define the i.MX6 mux > >devices and modifies the video-mux device to become a consumer of the > >video mmio-mux. > > > > - minor updates to Documentation/media/v4l-drivers/imx.rst. > > > > - ov5640: do nothing if entity stream count is greater than 1 in > >ov5640_s_stream(). > > > > - Previous versions of this driver had not tested the ability to enable > >multiple independent streams, for instance enabling multiple output > >pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and > >prpvf outputs. Marek Vasut tested this support and reported issues > >with it. > > > >v4l2_pipeline_inherit_controls() used the media graph walk APIs, but > >that walks both sink and source pads, so if there are multiple paths > >enabled to video capture devices, controls would be added to the wrong > >video capture device, and no controls added to the other enabled > >capture devices. > > > >These issues have been fixed. Control inheritance works correctly now > >even with multiple enabled capture paths, and (for example) > >simultaneous capture from prpenc and prpvf works also, and each with > >independent scaling, CSC, and controls. For example prpenc can be > >capturing with a 90 degree rotation, while prpvf is capturing with > >vertical flip. > > > >So the v4l2_pipeline_inherit_controls() patch has been dropped. The > >new version of control inheritance could be made generically available, > >but it would be more involved to incorporate it into v4l2-core. > > > > - A new function imx_media_fill_default_mbus_fields() is added to setup > >colorimetry at sink pads, and these are propagated to source pads. > > > > - Ensure that the current sink and source rectangles meet alignment > >restrictions before applying a new rotation control setting in > >prp-enc/vf subdevices. > > > > - Chain the s_stream() subdev calls instead of implementing a custom > >stream on/off function that attempts to call a fixed set of subdevices > >in a pipeline in the correct order. This also simplifies imx6-mipi-csi2 > >subdevice, since the correct MIPI CSI-2 startup sequence can be > >enforced completely in s_stream(), and s_power() is no longer > >required. This also paves the way for more arbitrary OF graphs > >external to the i.MX6. > > > > - Converted the v4l2_subdev and media_entity ops structures to const. > > What is the status as of v7? > > From what I can tell patch 2/34 needs an Ack from Rob Herring, patches > 4-14 are out of scope for the media subsystem, patches 20-25 and 27-34 > are all staging (so fine to be merged from my point of view). > > I'm not sure if patch 26 (defconfig) should be applied while the imx > driver is in staging. I would suggest that this patch is moved to the end > of the series. > > That leaves patches 15-19. I replied to patch 15 with a comment, patches > 16-18 look good to me, although patches 17 and 18 should be combined to one > patch since patch 17 won't compile otherwise. Is this a problem? It won't break any builds as patch 17 depends on CONFIG_MULTIPLEXER, which doesn't exist yet. I'm fine with merging the two patches, though. > Any idea when the multiplexer is expected to be merged? (just curious) I have no idea. v15 of the multiplexer framework patchset was posted on 2017-05-14, is still waiting for comments. > I would really like to get this merged for 4.13, so did I miss anything? Seconded, and not that I can tell. > From what I can tell it is really just an Ack for patch 2/34. regards Philipp
Re: [PATCH v7 15/34] add mux and video interface bridge entity functions
On Mon, 2017-05-29 at 15:37 +0200, Hans Verkuil wrote: > On 05/25/2017 02:29 AM, Steve Longerbeam wrote: > > From: Philipp Zabel > > > > Signed-off-by: Philipp Zabel > > > > - renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX > > > > Signed-off-by: Steve Longerbeam > > --- > > Documentation/media/uapi/mediactl/media-types.rst | 22 > > ++ > > include/uapi/linux/media.h| 6 ++ > > 2 files changed, 28 insertions(+) > > > > diff --git a/Documentation/media/uapi/mediactl/media-types.rst > > b/Documentation/media/uapi/mediactl/media-types.rst > > index 2a5164a..47ee003 100644 > > --- a/Documentation/media/uapi/mediactl/media-types.rst > > +++ b/Documentation/media/uapi/mediactl/media-types.rst > > @@ -299,6 +299,28 @@ Types and flags used to represent the media graph > > elements > > received on its sink pad and outputs the statistics data on > > its source pad. > > > > +- .. row 29 > > + > > + .. _MEDIA-ENT-F-VID-MUX: > > + > > + - ``MEDIA_ENT_F_VID_MUX`` > > + > > + - Video multiplexer. An entity capable of multiplexing must have at > > + least two sink pads and one source pad, and must pass the video > > + frame(s) received from the active sink pad to the source pad. > > Video > > + frame(s) from the inactive sink pads are discarded. > > + > > +- .. row 30 > > + > > + .. _MEDIA-ENT-F-VID-IF-BRIDGE: > > + > > + - ``MEDIA_ENT_F_VID_IF_BRIDGE`` > > + > > + - Video interface bridge. A video interface bridge entity must have > > at > > + least one sink pad and one source pad. It receives video frame(s) > > on > > + its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and > > + converts them and outputs them on its source pad in another bus > > format > > + (eDP, MIPI CSI-2, parallel, ...). > > I'm unhappy with the term 'bus format'. It's too close to 'mediabus format'. > How about calling it "bus protocol"? How about: "It receives video frames on its sink pad from an input video bus of one type (HDMI, eDP, MIPI CSI-2, ...), and outputs them on its source pad to an output video bus of another type (eDP, MIPI CSI-2, parallel, ...)." regards Philipp
[PATCH] [media] coda: improve colorimetry handling
The hardware codec is not colorspace aware. We should trust userspace to
set the correct colorimetry information on the OUTPUT queue and mirror
the exact same setting on the CAPTURE queue.
There is no reason to restrict colorspace to JPEG or REC709 only. Also,
set the default colorspace, as returned by calling VIDIOC_TRY/S_FMT with
V4L2_COLORSPACE_DEFAULT, initially.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 51 ++-
drivers/media/platform/coda/coda.h| 3 ++
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index d523e990d5093..506003eb9a01c 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -430,10 +430,10 @@ static int coda_g_fmt(struct file *file, void *priv,
f->fmt.pix.bytesperline = q_data->bytesperline;
f->fmt.pix.sizeimage= q_data->sizeimage;
- if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_JPEG)
- f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
- else
- f->fmt.pix.colorspace = ctx->colorspace;
+ f->fmt.pix.colorspace = ctx->colorspace;
+ f->fmt.pix.xfer_func= ctx->xfer_func;
+ f->fmt.pix.ycbcr_enc= ctx->ycbcr_enc;
+ f->fmt.pix.quantization = ctx->quantization;
return 0;
}
@@ -599,6 +599,9 @@ static int coda_try_fmt_vid_cap(struct file *file, void
*priv,
}
f->fmt.pix.colorspace = ctx->colorspace;
+ f->fmt.pix.xfer_func = ctx->xfer_func;
+ f->fmt.pix.ycbcr_enc = ctx->ycbcr_enc;
+ f->fmt.pix.quantization = ctx->quantization;
q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
codec = coda_find_codec(ctx->dev, q_data_src->fourcc,
@@ -635,6 +638,23 @@ static int coda_try_fmt_vid_cap(struct file *file, void
*priv,
return 0;
}
+static void coda_set_default_colorspace(struct v4l2_pix_format *fmt)
+{
+ enum v4l2_colorspace colorspace;
+
+ if (fmt->pixelformat == V4L2_PIX_FMT_JPEG)
+ colorspace = V4L2_COLORSPACE_JPEG;
+ else if (fmt->width <= 720 && fmt->height <= 576)
+ colorspace = V4L2_COLORSPACE_SMPTE170M;
+ else
+ colorspace = V4L2_COLORSPACE_REC709;
+
+ fmt->colorspace = colorspace;
+ fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+ fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+}
+
static int coda_try_fmt_vid_out(struct file *file, void *priv,
struct v4l2_format *f)
{
@@ -648,16 +668,8 @@ static int coda_try_fmt_vid_out(struct file *file, void
*priv,
if (ret < 0)
return ret;
- switch (f->fmt.pix.colorspace) {
- case V4L2_COLORSPACE_REC709:
- case V4L2_COLORSPACE_JPEG:
- break;
- default:
- if (f->fmt.pix.pixelformat == V4L2_PIX_FMT_JPEG)
- f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
- else
- f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
- }
+ if (f->fmt.pix.colorspace == V4L2_COLORSPACE_DEFAULT)
+ coda_set_default_colorspace(&f->fmt.pix);
q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
codec = coda_find_codec(dev, f->fmt.pix.pixelformat,
q_data_dst->fourcc);
@@ -772,6 +784,9 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
return ret;
ctx->colorspace = f->fmt.pix.colorspace;
+ ctx->xfer_func = f->fmt.pix.xfer_func;
+ ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
+ ctx->quantization = f->fmt.pix.quantization;
memset(&f_cap, 0, sizeof(f_cap));
f_cap.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
@@ -1282,7 +1297,13 @@ static void set_default_params(struct coda_ctx *ctx)
csize = coda_estimate_sizeimage(ctx, usize, max_w, max_h);
ctx->params.codec_mode = ctx->codec->mode;
- ctx->colorspace = V4L2_COLORSPACE_REC709;
+ if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG)
+ ctx->colorspace = V4L2_COLORSPACE_JPEG;
+ else
+ ctx->colorspace = V4L2_COLORSPACE_REC709;
+ ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+ ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ ctx->quantization = V4L2_QUANTIZATION_DEFAULT;
ctx->params.framerate = 30;
/* Default formats for output and input queues */
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 20222befb9b2f..308116d855e6f 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -206,
[PATCH v5 3/3] [media] platform: video-mux: include temporary mmio-mux support
As long as the mux framework is not merged, add temporary mmio-mux
support to the video-mux driver itself. This patch is to be reverted
once the "mux: minimal mux subsystem" and "mux: mmio-based syscon mux
controller" patches are merged.
Signed-off-by: Philipp Zabel
---
This patch allows the video-mux driver to be built and used on i.MX6 before
the mux framework [1][2] is merged. It can be dropped after that happened,
but until then, it should help to avoid a dependency of the i.MX6 capture
drivers on the mux framework, so that the two can be merged independently.
[1] https://patchwork.kernel.org/patch/9725911/
[2] https://patchwork.kernel.org/patch/9725893/
---
drivers/media/platform/Kconfig | 2 +-
drivers/media/platform/video-mux.c | 62 ++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 259c0ff780937..fea1dc05ea7b7 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -76,7 +76,7 @@ config VIDEO_M32R_AR_M64278
config VIDEO_MUX
tristate "Video Multiplexer"
- depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER &&
MULTIPLEXER
+ depends on OF && VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
help
This driver provides support for N:1 video bus multiplexers.
diff --git a/drivers/media/platform/video-mux.c
b/drivers/media/platform/video-mux.c
index e35ffa18126f3..b997ff881ad24 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -17,7 +17,12 @@
#include
#include
#include
+#ifdef CONFIG_MULTIPLEXER
#include
+#else
+#include
+#include
+#endif
#include
#include
#include
@@ -29,7 +34,11 @@ struct video_mux {
struct v4l2_subdev subdev;
struct media_pad *pads;
struct v4l2_mbus_framefmt *format_mbus;
+#ifdef CONFIG_MULTIPLEXER
struct mux_control *mux;
+#else
+ struct regmap_field *field;
+#endif
struct mutex lock;
int active;
};
@@ -70,7 +79,11 @@ static int video_mux_link_setup(struct media_entity *entity,
}
dev_dbg(sd->dev, "setting %d active\n", local->index);
+#ifdef CONFIG_MULTIPLEXER
ret = mux_control_try_select(vmux->mux, local->index);
+#else
+ ret = regmap_field_write(vmux->field, local->index);
+#endif
if (ret < 0)
goto out;
vmux->active = local->index;
@@ -79,7 +92,9 @@ static int video_mux_link_setup(struct media_entity *entity,
goto out;
dev_dbg(sd->dev, "going inactive\n");
+#ifdef CONFIG_MULTIPLEXER
mux_control_deselect(vmux->mux);
+#endif
vmux->active = -1;
}
@@ -193,6 +208,48 @@ static const struct v4l2_subdev_ops video_mux_subdev_ops =
{
.video = &video_mux_subdev_video_ops,
};
+#ifndef CONFIG_MULTIPLEXER
+static int video_mux_probe_mmio_mux(struct video_mux *vmux)
+{
+ struct device *dev = vmux->subdev.dev;
+ struct of_phandle_args args;
+ struct reg_field field;
+ struct regmap *regmap;
+ u32 reg, mask;
+ int ret;
+
+ ret = of_parse_phandle_with_args(dev->of_node, "mux-controls",
+"#mux-control-cells", 0, &args);
+ if (ret)
+ return ret;
+
+ if (!of_device_is_compatible(args.np, "mmio-mux"))
+ return -EINVAL;
+
+ regmap = syscon_node_to_regmap(args.np->parent);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ ret = of_property_read_u32_index(args.np, "mux-reg-masks",
+2 * args.args[0], ®);
+ if (!ret)
+ ret = of_property_read_u32_index(args.np, "mux-reg-masks",
+2 * args.args[0] + 1, &mask);
+ if (ret < 0)
+ return ret;
+
+ field.reg = reg;
+ field.msb = fls(mask) - 1;
+ field.lsb = ffs(mask) - 1;
+
+ vmux->field = devm_regmap_field_alloc(dev, regmap, field);
+ if (IS_ERR(vmux->field))
+ return PTR_ERR(vmux->field);
+
+ return 0;
+}
+#endif
+
static int video_mux_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -230,9 +287,14 @@ static int video_mux_probe(struct platform_device *pdev)
return -EINVAL;
}
+#ifdef CONFIG_MULTIPLEXER
vmux->mux = devm_mux_control_get(dev, NULL);
if (IS_ERR(vmux->mux)) {
ret = PTR_ERR(vmux->mux);
+#else
+ ret = video_mux_probe_mmio_mux(vmux);
+ if (ret) {
+#endif
if (ret != -EPROBE_DEFER)
dev_err(dev, "Failed to get mux: %d\n", ret);
return ret;
--
2.11.0
