cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Oct 17 05:00:10 CEST 2018 media-tree git hash:490d84f6d73c12f4204241cff8651eed60aae914 media_build git hash: 9f419c414672676f63e85a61ea99df0ddcd6e9a7 v4l-utils git hash: e84c8bdb0af5b79efa26aabc7b17d1ddead56c70 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.11-marune linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.123-i686: OK linux-3.18.123-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.159-i686: OK linux-4.4.159-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.131-i686: OK linux-4.9.131-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19-rc6-i686: OK linux-4.19-rc6-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: i.MX6 MIPI-CSI2 OV5640 Camera testing on Mainline Linux
Hi Adam, On 10/16/18 12:46 PM, Adam Ford wrote: On Thu, Sep 20, 2018 at 9:58 AM jacopo mondi wrote: Hi imx6 people, On Thu, May 31, 2018 at 08:39:20PM +0530, Jagan Teki wrote: Hi All, I'm trying to verify MIPI-CSI2 OV5640 camera on i.MX6 platform with Mainline Linux. Sorry to resurect this, but before diving deep into details, do anyone of you verified JPEG capture with ov5640 and i.MX6 platforms, and has maybe a pipeline configuration to share :) ? I have a 4.14 kernel for my i.MX6D/Q using an ov5640 connected in a similar way as the sabresd and I'm getting similar timeouts. when executing media-ctl -l "'ov5640 2-0010':0 -> 'imx6-mipi-csi2':0[1]" media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]" You're routing through imx6-mipi-csi2 pad 2, which is CSI-2 virtual channel 1, so make sure the ov5640 is transmitting on that channel, see virtual_channel module parameter. media-ctl -l "'ipu1_csi1':1 -> 'ipu1_ic_prp':0[1]" media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]" media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]" media-ctl -V "'ov5640 2-0010':0 [fmt:UYVY2X8/640x480 field:none]" media-ctl -V "'imx6-mipi-csi2':2 [fmt:UYVY2X8/640x480 field:none]" media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/640x480 field:none]" media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480 field:none]" media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/640x480 field:none]" gst-launch-1.0 -v v4l2src num-buffers=1 device=/dev/video0 ! jpegenc ! filesink location=test.jpg [ 72.799015] ipu1_ic_prpenc: EOF timeout [ 73.838985] ipu1_ic_prpenc: wait last EOF timeout When I try to jump directly to 4.19-RC8, I get errors regarding memory allocation, so I think there might be something else there I am missing. Has anyone tried this camera module on a 4.14 kernel? I noticed there are a bunch of driver updates, and I was hoping there might be some patches that could be be backported to the 4.14.y stable branch. I would suggest backporting all the ov5640 commits. You can also backport the imx-media commits, but that shouldn't be the cause of the timeouts you are seeing. Steve thanks for any suggestions to try. adam Thanks j I've followed these[1] instructions to configure MC links and pads based on the probing details from dmesg and trying to capture ipu1_ic_prpenc capture (/dev/video1) but it's not working. Can anyone help me to verify whether I configured all the details properly if not please suggest. I'm pasting full log here, so-that anyone can comment in line and dt changes are at [2] Log: - [1.211866] etnaviv-gpu 2204000.gpu: Ignoring GPU with VG and FE2.0 [1.220211] [drm] Initialized etnaviv 1.2.0 20151214 for etnaviv on minor 0 [1.230344] imx-ipuv3 240.ipu: IPUv3H probed [1.237170] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [1.243920] [drm] No driver support for vblank timestamp query. [1.250831] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops ipu_crtc_ops) [1.258503] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops ipu_crtc_ops) [1.266293] imx-drm display-subsystem: bound imx-ipuv3-crtc.6 (ops ipu_crtc_ops) [1.274027] imx-drm display-subsystem: bound imx-ipuv3-crtc.7 (ops ipu_crtc_ops) [1.282304] dwhdmi-imx 12.hdmi: Detected HDMI TX controller v1.30a with HDCP (DWC HDMI 3D TX PHY) [1.295722] imx-drm display-subsystem: bound 12.hdmi (ops dw_hdmi_imx_ops) [1.373615] Console: switching to colour frame buffer device 128x48 [1.396495] imx-drm display-subsystem: fb0: frame buffer device [1.404620] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem on minor 1 [1.412763] imx-ipuv3 280.ipu: IPUv3H probed [1.439673] brd: module loaded [1.469099] loop: module loaded [1.480324] nand: No NAND device found [1.487768] libphy: Fixed MDIO Bus: probed [1.493034] CAN device driver interface [1.499057] fec 2188000.ethernet: 2188000.ethernet supply phy not found, using dummy regulator [1.511633] pps pps0: new PPS source ptp0 [1.516928] fec 2188000.ethernet (unnamed net_device) (uninitialized): Invalid MAC address: 00:00:00:00:00:00 [1.527177] fec 2188000.ethernet (unnamed net_device) (uninitialized): Using random MAC address: f2:5a:6d:a6:90:74 [1.543567] libphy: fec_enet_mii_bus: probed [1.549138] fec 2188000.ethernet eth0: registered PHC device 0 [1.556499] usbcore: registered new interface driver asix [1.562066] usbcore: registered new interface driver ax88179_178a [1.568259] usbcore: registered new interface driver cdc_ether [1.574276] usbcore: registered new interface driver net1080 [1.580097] usbcore: registered new interface driver cdc_subset [1.586144] usbcore: registered new interface driver zaurus [1.591910] usbcore: registered new interface driver cdc_ncm [1.597589] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [1.604209] ehci-pci: EHCI PCI platform driver [1.6087
[PATCH v5 03/12] gpu: ipu-v3: Add planar support to interlaced scan
To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
Acked-by: Philipp Zabel
---
drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++-
drivers/staging/media/imx/imx-media-csi.c | 3 ++-
include/video/imx-ipu-v3.h | 3 ++-
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32
u_off, u32 v_off)
}
EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat)
{
- u32 ilo, sly;
+ u32 ilo, sly, sluv;
if (stride < 0) {
stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch,
int stride)
sly = (stride * 2) - 1;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ sluv = stride / 2 - 1;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_YUV422P:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ sluv = stride * 2 - 1;
+ break;
+ default:
+ sluv = 0;
+ break;
+ }
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+ if (sluv)
+ ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
};
EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field) &&
channel == priv->out_ch)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+ ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 7ecbd4d76d09..4aa20ae72608 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
- image.pix.bytesperline);
+ image.pix.bytesperline,
+ image.pix.pixelformat);
ipu_idmac_set_double_buffer(priv->idmac_ch, true);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int
stride);
void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t
buf);
void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat);
void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
--
2.17.1
[PATCH v5 12/12] media: imx.rst: Update doc to reflect fixes to interlaced capture
Also add an example pipeline for unconverted capture with interweave on SabreAuto. Cleanup some language in various places in the process. Signed-off-by: Steve Longerbeam --- Changes since v4: - Make clear that it is IDMAC channel that does pixel reordering and interweave, not the CSI. Caught by Philipp Zabel. Changes since v3: - none. Changes since v2: - expand on idmac interweave behavior in CSI subdev. - switch second SabreAuto pipeline example to PAL to give both NTSC and PAL examples. - Cleanup some language in various places. --- Documentation/media/v4l-drivers/imx.rst | 103 +++- 1 file changed, 66 insertions(+), 37 deletions(-) diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst index 65d3d15eb159..45f3b68865dd 100644 --- a/Documentation/media/v4l-drivers/imx.rst +++ b/Documentation/media/v4l-drivers/imx.rst @@ -22,8 +22,8 @@ memory. Various dedicated DMA channels exist for both video capture and display paths. During transfer, the IDMAC is also capable of vertical image flip, 8x8 block transfer (see IRT description), pixel component re-ordering (for example UYVY to YUYV) within the same colorspace, and -even packed <--> planar conversion. It can also perform a simple -de-interlacing by interleaving even and odd lines during transfer +packed <--> planar conversion. The IDMAC can also perform a simple +de-interlacing by interweaving even and odd lines during transfer (without motion compensation which requires the VDIC). The CSI is the backend capture unit that interfaces directly with @@ -173,15 +173,21 @@ via the SMFC and an IDMAC channel, bypassing IC pre-processing. This source pad is routed to a capture device node, with a node name of the format "ipuX_csiY capture". -Note that since the IDMAC source pad makes use of an IDMAC channel, it -can do pixel reordering within the same colorspace. For example, the -sink pad can take UYVY2X8, but the IDMAC source pad can output YUYV2X8. -If the sink pad is receiving YUV, the output at the capture device can -also be converted to a planar YUV format such as YUV420. - -It will also perform simple de-interlace without motion compensation, -which is activated if the sink pad's field type is an interlaced type, -and the IDMAC source pad field type is set to none. +Note that since the IDMAC source pad makes use of an IDMAC channel, +pixel reordering within the same colorspace can be carried out by the +IDMAC channel. For example, if the CSI sink pad is receiving in UYVY +order, the capture device linked to the IDMAC source pad can capture +in YUYV order. Also, if the CSI sink pad is receiving a packed YUV +format, the capture device can capture a planar YUV format such as +YUV420. + +The IDMAC channel at the IDMAC source pad also supports simple +interweave without motion compensation, which is activated if the source +pad's field type is sequential top-bottom or bottom-top, and the +requested capture interface field type is set to interlaced (t-b, b-t, +or unqualified interlaced). The capture interface will enforce the same +field order as the source pad field order (interlaced-bt if source pad +is seq-bt, interlaced-tb if source pad is seq-tb). This subdev can generate the following event when enabling the second IDMAC source pad: @@ -323,14 +329,14 @@ ipuX_vdic The VDIC carries out motion compensated de-interlacing, with three motion compensation modes: low, medium, and high motion. The mode is -specified with the menu control V4L2_CID_DEINTERLACING_MODE. It has -two sink pads and a single source pad. +specified with the menu control V4L2_CID_DEINTERLACING_MODE. The VDIC +has two sink pads and a single source pad. The direct sink pad receives from an ipuX_csiY direct pad. With this link the VDIC can only operate in high motion mode. When the IDMAC sink pad is activated, it receives from an output -or mem2mem device node. With this pipeline, it can also operate +or mem2mem device node. With this pipeline, the VDIC can also operate in low and medium modes, because these modes require receiving frames from memory buffers. Note that an output or mem2mem device is not implemented yet, so this sink pad currently has no links. @@ -343,8 +349,8 @@ ipuX_ic_prp This is the IC pre-processing entity. It acts as a router, routing data from its sink pad to one or both of its source pads. -It has a single sink pad. The sink pad can receive from the ipuX_csiY -direct pad, or from ipuX_vdic. +This entity has a single sink pad. The sink pad can receive from the +ipuX_csiY direct pad, or from ipuX_vdic. This entity has two source pads. One source pad routes to the pre-process encode task entity (ipuX_ic_prpenc), the other to the @@ -367,8 +373,8 @@ color-space conversion, resizing (downscaling and upscaling), horizontal and vertical flip, and 90/270 degree rotation. Flip and rotation are provided via standard V4L2 controls. -Like the ipuX_csiY IDMAC source, it
[PATCH v5 05/12] media: imx-csi: Input connections to CSI should be optional
Some imx platforms do not have fwnode connections to all CSI input
ports, and should not be treated as an error. This includes the
imx6q SabreAuto, which has no connections to ipu1_csi1 and ipu2_csi0.
Return -ENOTCONN in imx_csi_parse_endpoint() so that v4l2-fwnode
endpoint parsing will not treat an unconnected endpoint as an error.
Fixes: c893500a16baf ("media: imx: csi: Register a subdev notifier")
Signed-off-by: Steve Longerbeam
---
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 176978c7dfe7..8f52428d2c75 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1813,7 +1813,7 @@ static int imx_csi_parse_endpoint(struct device *dev,
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd)
{
- return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
+ return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
}
static int imx_csi_async_register(struct csi_priv *priv)
--
2.17.1
[PATCH v5 08/12] media: imx-csi: Allow skipping odd chroma rows for YVU420
Skip writing U/V components to odd rows for YVU420 in addition to YUV420 and NV12. Signed-off-by: Steve Longerbeam Reviewed-by: Philipp Zabel --- drivers/staging/media/imx/imx-media-csi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 7e648fc9626a..c9110dd39a49 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -452,6 +452,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) passthrough_bits = 16; break; case V4L2_PIX_FMT_YUV420: + case V4L2_PIX_FMT_YVU420: case V4L2_PIX_FMT_NV12: burst_size = (image.pix.width & 0x3f) ? ((image.pix.width & 0x1f) ? -- 2.17.1
[PATCH v5 09/12] media: imx: vdic: rely on VDIC for correct field order
prepare_vdi_in_buffers() was setting up the dma pointers as if the
VDIC is always programmed to receive the fields in bottom-top order,
i.e. as if ipu_vdi_set_field_order() only programs BT order in the VDIC.
But that's not true, ipu_vdi_set_field_order() is working correctly.
So fix prepare_vdi_in_buffers() to give the VDIC the fields in whatever
order they were received by the video source, and rely on the VDIC to
sort out which is top and which is bottom.
Signed-off-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-media-vdic.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-vdic.c
b/drivers/staging/media/imx/imx-media-vdic.c
index 482250d47e7c..4a890714193e 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -219,26 +219,18 @@ static void __maybe_unused prepare_vdi_in_buffers(struct
vdic_priv *priv,
switch (priv->fieldtype) {
case V4L2_FIELD_SEQ_TB:
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
case V4L2_FIELD_SEQ_BT:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + fs;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
break;
+ case V4L2_FIELD_INTERLACED_TB:
case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + is;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
break;
- default:
- /* assume V4L2_FIELD_INTERLACED_TB */
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
}
ipu_cpmem_set_buffer(priv->vdi_in_ch_p, 0, prev_phys);
--
2.17.1
[PATCH v5 04/12] media: imx: Fix field negotiation
IDMAC interlaced scan, a.k.a. interweave, should be enabled in the
IDMAC output channels only if the IDMAC output pad field type is
'seq-bt' or 'seq-tb', and field type at the capture interface is
'interlaced*'.
V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
enabling interlaced/interweave scan. That macro includes the 'interlaced'
field types, and in those cases the data is already interweaved with
top/bottom field lines.
The CSI will capture whole frames when the source specifies alternate
field mode. So the CSI also enables interweave for alternate input
field type and the field type at capture interface is interlaced.
Fix the logic for setting field type in try_fmt in CSI entity.
The behavior should be:
- No restrictions on field type at sink pad.
- At the output pads, allow sequential fields in TB order, if the sink pad
field type is sequential or alternate. Otherwise passthrough the field
type from sink to source pad.
Move this logic to new function csi_try_field().
These changes result in the following allowed field transformations
from CSI sink -> source pads (all other field types at sink are passed
through to source):
seq-tb -> seq-tb
seq-bt -> seq-tb
alternate -> seq-tb
In a future patch, the CSI sink -> source will allow:
seq-tb -> seq-bt
seq-bt -> seq-bt
alternate -> seq-bt
This will require supporting interweave with top/bottom line swapping.
Until then seq-bt is not allowed at the CSI source pad because there is
no way to swap top/bottom lines when interweaving to INTERLACED_BT --
note that despite the name, INTERLACED_BT is top-bottom order in memory.
The BT in this case refers to field dominance: the bottom lines are
older in time than the top lines.
The capture interface device allows selecting IDMAC interweave by
choosing INTERLACED_TB if the CSI/PRPENCVF source pad is seq-tb and
INTERLACED_BT if the source pad is seq-bt (for future support of seq-bt).
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 21 --
drivers/staging/media/imx/imx-media-capture.c | 14
drivers/staging/media/imx/imx-media-csi.c | 64 ++-
3 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index af7224846bd5..1a03d4c9d7b8 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -354,12 +354,13 @@ static int prp_setup_channel(struct prp_priv *priv,
{
struct imx_media_video_dev *vdev = priv->vdev;
const struct imx_media_pixfmt *outcc;
- struct v4l2_mbus_framefmt *infmt;
+ struct v4l2_mbus_framefmt *outfmt;
unsigned int burst_size;
struct ipu_image image;
+ bool interweave;
int ret;
- infmt = &priv->format_mbus[PRPENCVF_SINK_PAD];
+ outfmt = &priv->format_mbus[PRPENCVF_SRC_PAD];
outcc = vdev->cc;
ipu_cpmem_zero(channel);
@@ -369,6 +370,15 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
+ /*
+* If the field type at capture interface is interlaced, and
+* the output IDMAC pad is sequential, enable interweave at
+* the IDMAC output channel.
+*/
+ interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
+ channel == priv->out_ch;
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -409,9 +419,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field) &&
- channel == priv->out_ch)
+ if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
image.pix.pixelformat);
@@ -839,8 +847,7 @@ static void prp_try_fmt(struct prp_priv *priv,
infmt = __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD, sdformat->which);
if (sdformat->pad == PRPENCVF_SRC_PAD) {
- if (sdformat->format.field != V4L2_FIELD_NONE)
- sdformat->format.field = infmt->field;
+ sdformat->format.field = infmt->field;
prp_bound_align_output(&sdformat->format, infmt,
priv->rot_mode);
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index b37e1186eb2f..01ec9443de55 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -239,6 +239,20 @@ static int capture_try_fmt_vid_cap(struct file
[PATCH v5 06/12] media: imx-csi: Double crop height for alternate fields at sink
If the incoming sink field type is alternate, the reset crop height
and crop height bounds must be set to twice the incoming height,
because in alternate field mode, upstream will report only the
lines for a single field, and the CSI captures the whole frame.
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 8f52428d2c75..d5b0f8a66750 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1140,6 +1140,8 @@ static void csi_try_crop(struct csi_priv *priv,
struct v4l2_mbus_framefmt *infmt,
struct v4l2_fwnode_endpoint *upstream_ep)
{
+ u32 in_height;
+
crop->width = min_t(__u32, infmt->width, crop->width);
if (crop->left + crop->width > infmt->width)
crop->left = infmt->width - crop->width;
@@ -1147,6 +1149,10 @@ static void csi_try_crop(struct csi_priv *priv,
crop->left &= ~0x3;
crop->width &= ~0x7;
+ in_height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ in_height *= 2;
+
/*
* FIXME: not sure why yet, but on interlaced bt.656,
* changing the vertical cropping causes loss of vertical
@@ -1156,12 +1162,12 @@ static void csi_try_crop(struct csi_priv *priv,
if (upstream_ep->bus_type == V4L2_MBUS_BT656 &&
(V4L2_FIELD_HAS_BOTH(infmt->field) ||
infmt->field == V4L2_FIELD_ALTERNATE)) {
- crop->height = infmt->height;
- crop->top = (infmt->height == 480) ? 2 : 0;
+ crop->height = in_height;
+ crop->top = (in_height == 480) ? 2 : 0;
} else {
- crop->height = min_t(__u32, infmt->height, crop->height);
- if (crop->top + crop->height > infmt->height)
- crop->top = infmt->height - crop->height;
+ crop->height = min_t(__u32, in_height, crop->height);
+ if (crop->top + crop->height > in_height)
+ crop->top = in_height - crop->height;
}
}
@@ -1401,6 +1407,8 @@ static void csi_try_fmt(struct csi_priv *priv,
crop->top = 0;
crop->width = sdformat->format.width;
crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
compose->left = 0;
compose->top = 0;
@@ -1528,6 +1536,8 @@ static int csi_get_selection(struct v4l2_subdev *sd,
sel->r.top = 0;
sel->r.width = infmt->width;
sel->r.height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ sel->r.height *= 2;
break;
case V4L2_SEL_TGT_CROP:
sel->r = *crop;
--
2.17.1
[PATCH v5 10/12] media: imx-csi: Move crop/compose reset after filling default mbus fields
If caller passes un-initialized field type V4L2_FIELD_ANY to CSI
sink pad, the reset CSI crop window would not be correct, because
the crop window depends on a valid input field type. To fix move
the reset of crop and compose windows to after the call to
imx_media_fill_default_mbus_fields().
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 27 ---
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index c9110dd39a49..0d494a7db211 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1407,19 +1407,6 @@ static void csi_try_fmt(struct csi_priv *priv,
W_ALIGN, &sdformat->format.height,
MIN_H, MAX_H, H_ALIGN, S_ALIGN);
- /* Reset crop and compose rectangles */
- crop->left = 0;
- crop->top = 0;
- crop->width = sdformat->format.width;
- crop->height = sdformat->format.height;
- if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
- crop->height *= 2;
- csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
- compose->left = 0;
- compose->top = 0;
- compose->width = crop->width;
- compose->height = crop->height;
-
*cc = imx_media_find_mbus_format(sdformat->format.code,
CS_SEL_ANY, true);
if (!*cc) {
@@ -1435,6 +1422,20 @@ static void csi_try_fmt(struct csi_priv *priv,
imx_media_fill_default_mbus_fields(
&sdformat->format, infmt,
priv->active_output_pad == CSI_SRC_PAD_DIRECT);
+
+ /* Reset crop and compose rectangles */
+ crop->left = 0;
+ crop->top = 0;
+ crop->width = sdformat->format.width;
+ crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
+ csi_try_crop(priv, crop, cfg, &sdformat->format, upstream_ep);
+ compose->left = 0;
+ compose->top = 0;
+ compose->width = crop->width;
+ compose->height = crop->height;
+
break;
}
}
--
2.17.1
[PATCH v5 11/12] media: imx: Allow interweave with top/bottom lines swapped
Allow sequential->interlaced interweaving but with top/bottom
lines swapped to the output buffer.
This can be accomplished by adding one line length to IDMAC output
channel address, with a negative line length for the interlace offset.
This is to allow the seq-bt -> interlaced-bt transformation, where
bottom lines are still dominant (older in time) but with top lines
first in the interweaved output buffer.
With this support, the CSI can now allow seq-bt at its source pads,
e.g. the following transformations are allowed in CSI from sink to
source:
seq-tb -> seq-bt
seq-bt -> seq-bt
alternate -> seq-bt
Suggested-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
Changes since v4:
- Removed interweave_offset and replace with boolean interweave_swap,
suggested by Philipp Zabel.
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +
drivers/staging/media/imx/imx-media-csi.c | 40 ++---
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index cf76b0432371..33ada6612fee 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -106,6 +106,7 @@ struct prp_priv {
u32 frame_sequence; /* frame sequence counter */
bool last_eof; /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -235,6 +236,9 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct
ipuv3_channel *ch)
if (ipu_idmac_buffer_is_ready(ch, priv->ipu_buf_num))
ipu_idmac_clear_buffer(ch, priv->ipu_buf_num);
+ if (priv->interweave_swap && ch == priv->out_ch)
+ phys += vdev->fmt.fmt.pix.bytesperline;
+
ipu_cpmem_set_buffer(ch, priv->ipu_buf_num, phys);
}
@@ -376,8 +380,9 @@ static int prp_setup_channel(struct prp_priv *priv,
* the IDMAC output channel.
*/
interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
- V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
- channel == priv->out_ch;
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field);
+ priv->interweave_swap = interweave &&
+ image.pix.field == V4L2_FIELD_INTERLACED_BT;
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
@@ -388,6 +393,11 @@ static int prp_setup_channel(struct prp_priv *priv,
(image.pix.width * outcc->bpp) >> 3;
}
+ if (priv->interweave_swap && channel == priv->out_ch) {
+ /* start interweave scan at 1st top line (2nd line) */
+ image.rect.top = 1;
+ }
+
image.phys0 = addr0;
image.phys1 = addr1;
@@ -396,8 +406,8 @@ static int prp_setup_channel(struct prp_priv *priv,
* channels for planar 4:2:0 (but not when enabling IDMAC
* interweaving, they are incompatible).
*/
- if (!interweave && (channel == priv->out_ch ||
- channel == priv->rot_out_ch)) {
+ if ((channel == priv->out_ch && !interweave) ||
+ channel == priv->rot_out_ch) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
@@ -424,8 +434,11 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (interweave)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ if (interweave && channel == priv->out_ch)
+ ipu_cpmem_interlaced_scan(channel,
+ priv->interweave_swap ?
+ -image.pix.bytesperline :
+ image.pix.bytesperline,
image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 0d494a7db211..73c9f3ae4221 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -114,6 +114,7 @@ struct csi_priv {
u32 frame_sequence; /* frame sequence counter */
bool last_eof; /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -286,6 +287,9 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
if (ipu_idmac_buffer_is_ready(priv->idmac_ch, priv->ipu_buf_num))
ipu_idmac_clear_buffer(priv->idmac_ch, priv->ipu_buf_num);
+ i
[PATCH v5 07/12] media: imx: interweave and odd-chroma-row skip are incompatible
If IDMAC interweaving is enabled in a write channel, the channel must
write the odd chroma rows for 4:2:0 formats. Skipping writing the odd
chroma rows produces corrupted captured 4:2:0 images when interweave
is enabled.
Reported-by: Krzysztof Hałasa
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 9 +++--
drivers/staging/media/imx/imx-media-csi.c | 8 ++--
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 1a03d4c9d7b8..cf76b0432371 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -391,12 +391,17 @@ static int prp_setup_channel(struct prp_priv *priv,
image.phys0 = addr0;
image.phys1 = addr1;
- if (channel == priv->out_ch || channel == priv->rot_out_ch) {
+ /*
+* Skip writing U and V components to odd rows in the output
+* channels for planar 4:2:0 (but not when enabling IDMAC
+* interweaving, they are incompatible).
+*/
+ if (!interweave && (channel == priv->out_ch ||
+ channel == priv->rot_out_ch)) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
- /* Skip writing U and V components to odd rows */
ipu_cpmem_skip_odd_chroma_rows(channel);
break;
}
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index d5b0f8a66750..7e648fc9626a 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -457,8 +457,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
((image.pix.width & 0x1f) ?
((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
passthrough_bits = 16;
- /* Skip writing U and V components to odd rows */
- ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
+ /*
+* Skip writing U and V components to odd rows (but not
+* when enabling IDMAC interweaving, they are incompatible).
+*/
+ if (!interweave)
+ ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
break;
case V4L2_PIX_FMT_YUYV:
case V4L2_PIX_FMT_UYVY:
--
2.17.1
[PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.
Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().
Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).
When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.
Signed-off-by: Steve Longerbeam
---
Changes since v4:
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
---
drivers/gpu/ipu-v3/ipu-csi.c | 119 +++---
drivers/staging/media/imx/imx-media-csi.c | 17 +---
include/video/imx-ipu-v3.h| 3 +-
3 files changed, 88 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index aa0e30a2ba18..4a15e513fa05 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config
*cfg, u32 mbus_code,
return 0;
}
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+ return (field != V4L2_FIELD_ALTERNATE) ? field :
+ ((std & V4L2_STD_525_60) ?
+V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
/*
* Fill a CSI bus config struct from mbus_config and mbus_framefmt.
*/
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config
*csicfg,
return 0;
}
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt,
+ v4l2_std_id std)
+{
+ enum v4l2_field infield, outfield;
+ bool swap_fields;
+
+ /* get translated field type of input and output */
+ infield = ipu_csi_translate_field(infmt->field, std);
+ outfield = ipu_csi_translate_field(outfmt->field, std);
+
+ /*
+* Write the H-V-F codes the CSI will match against the
+* incoming data for start/end of active and blanking
+* field intervals. If input and output field types are
+* sequential but not the same (one is SEQ_BT and the other
+* is SEQ_TB), swap the F-bit so that the CSI will capture
+* field 1 lines before field 0 lines.
+*/
+ swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+ infield != outfield);
+
+ if (!swap_fields) {
+ /*
+* Field0BlankEnd = 110, Field0BlankStart = 010
+* Field0ActiveEnd = 100, Field0ActiveStart = 000
+* Field1BlankEnd = 111, Field1BlankStart = 011
+* Field1ActiveEnd = 101, Field1ActiveStart = 001
+*/
+ ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+ } else {
+ dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+ /* same as above but with F-bit inverted */
+ ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+ }
+
+ ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
+
+ return 0;
+}
+
+
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt)
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt)
{
struct ipu_csi_bus_config cfg;
unsigned long flags;
u32 width, height, data = 0;
+ v4l2_std_id std;
int ret;
- ret = fill_csi_bus_cfg(&cfg, mbus_cfg, mbus_fmt);
+ ret = fill_csi_bus_cfg(&cfg, mbus_cfg, infmt);
if (ret < 0)
return ret;
/* set default sensor frame width and height */
- width = mbus_fmt->width;
- height = mbus_fmt->height;
+ width = infmt->width;
+ height = infmt->height;
+
[PATCH v5 00/12] imx-media: Fixes for interlaced capture
A set of patches that fixes some bugs with capturing from an interlaced source, and incompatibilites between IDMAC interlace interweaving and 4:2:0 data write reduction. History: v5: - Added a regression fix to allow empty endpoints to CSI (fix for imx6q SabreAuto). - Cleaned up some convoluted code in ipu_csi_init_interface(), suggested by Philipp Zabel. - Fixed a regression in csi_setup(), caught by Philipp. - Removed interweave_offset and replace with boolean interweave_swap, suggested by Philipp. - Make clear that it is IDMAC channel that does pixel reordering and interweave, not the CSI, in the imx.rst doc, caught by Philipp. v4: - rebased to latest media-tree master branch. - Make patch author and SoB email addresses the same. v3: - add support for/fix interweaved scan with YUV planar output. - fix bug in 4:2:0 U/V offset macros. - add patch that generalizes behavior of field swap in ipu_csi_init_interface(). - add support for interweaved scan with field order swap. Suggested by Philipp Zabel. - in v2, inteweave scan was determined using field types of CSI (and PRPENCVF) at the sink and source pads. In v3, this has been moved one hop downstream: interweave is now determined using field type at source pad, and field type selected at capture interface. Suggested by Philipp. - make sure to double CSI crop target height when input field type in alternate. - more updates to media driver doc to reflect above. v2: - update media driver doc. - enable idmac interweave only if input field is sequential/alternate, and output field is 'interlaced*'. - move field try logic out of *try_fmt and into separate function. - fix bug with resetting crop/compose rectangles. - add a patch that fixes a field order bug in VDIC indirect mode. - remove alternate field type from V4L2_FIELD_IS_SEQUENTIAL() macro Suggested-by: Nicolas Dufresne . - add macro V4L2_FIELD_IS_INTERLACED(). Steve Longerbeam (12): media: videodev2.h: Add more field helper macros gpu: ipu-csi: Swap fields according to input/output field types gpu: ipu-v3: Add planar support to interlaced scan media: imx: Fix field negotiation media: imx-csi: Input connections to CSI should be optional media: imx-csi: Double crop height for alternate fields at sink media: imx: interweave and odd-chroma-row skip are incompatible media: imx-csi: Allow skipping odd chroma rows for YVU420 media: imx: vdic: rely on VDIC for correct field order media: imx-csi: Move crop/compose reset after filling default mbus fields media: imx: Allow interweave with top/bottom lines swapped media: imx.rst: Update doc to reflect fixes to interlaced capture Documentation/media/v4l-drivers/imx.rst | 103 +++ drivers/gpu/ipu-v3/ipu-cpmem.c| 26 ++- drivers/gpu/ipu-v3/ipu-csi.c | 119 + drivers/staging/media/imx/imx-ic-prpencvf.c | 46 +++-- drivers/staging/media/imx/imx-media-capture.c | 14 ++ drivers/staging/media/imx/imx-media-csi.c | 168 +- drivers/staging/media/imx/imx-media-vdic.c| 12 +- include/uapi/linux/videodev2.h| 7 + include/video/imx-ipu-v3.h| 6 +- 9 files changed, 354 insertions(+), 147 deletions(-) -- 2.17.1
[PATCH v5 01/12] media: videodev2.h: Add more field helper macros
Adds two helper macros:
V4L2_FIELD_IS_SEQUENTIAL: returns true if the given field type is
'sequential', that is a full frame is transmitted, or exists in
memory, as all top field lines followed by all bottom field lines,
or vice-versa.
V4L2_FIELD_IS_INTERLACED: returns true if the given field type is
'interlaced', that is a full frame is transmitted, or exists in
memory, as top field lines interlaced with bottom field lines.
Signed-off-by: Steve Longerbeam
---
Changes since v3:
- none
Changes since v2:
- none
Changes since v1:
- add the complement macro V4L2_FIELD_IS_INTERLACED
- remove V4L2_FIELD_ALTERNATE from V4L2_FIELD_IS_SEQUENTIAL macro.
- moved new macros past end of existing V4L2_FIELD_HAS_* macros.
---
include/uapi/linux/videodev2.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 29729d580452..f7f031736d91 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -130,6 +130,13 @@ enum v4l2_field {
((field) == V4L2_FIELD_BOTTOM ||\
(field) == V4L2_FIELD_TOP ||\
(field) == V4L2_FIELD_ALTERNATE)
+#define V4L2_FIELD_IS_INTERLACED(field) \
+ ((field) == V4L2_FIELD_INTERLACED ||\
+(field) == V4L2_FIELD_INTERLACED_TB ||\
+(field) == V4L2_FIELD_INTERLACED_BT)
+#define V4L2_FIELD_IS_SEQUENTIAL(field) \
+ ((field) == V4L2_FIELD_SEQ_TB ||\
+(field) == V4L2_FIELD_SEQ_BT)
enum v4l2_buf_type {
V4L2_BUF_TYPE_VIDEO_CAPTURE= 1,
--
2.17.1
Re: i.MX6 MIPI-CSI2 OV5640 Camera testing on Mainline Linux
On Thu, Sep 20, 2018 at 9:58 AM jacopo mondi wrote: > > Hi imx6 people, > > On Thu, May 31, 2018 at 08:39:20PM +0530, Jagan Teki wrote: > > Hi All, > > > > I'm trying to verify MIPI-CSI2 OV5640 camera on i.MX6 platform with > > Mainline Linux. > > Sorry to resurect this, but before diving deep into details, do anyone > of you verified JPEG capture with ov5640 and i.MX6 platforms, and has > maybe a pipeline configuration to share :) ? I have a 4.14 kernel for my i.MX6D/Q using an ov5640 connected in a similar way as the sabresd and I'm getting similar timeouts. when executing media-ctl -l "'ov5640 2-0010':0 -> 'imx6-mipi-csi2':0[1]" media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]" media-ctl -l "'ipu1_csi1':1 -> 'ipu1_ic_prp':0[1]" media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]" media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]" media-ctl -V "'ov5640 2-0010':0 [fmt:UYVY2X8/640x480 field:none]" media-ctl -V "'imx6-mipi-csi2':2 [fmt:UYVY2X8/640x480 field:none]" media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/640x480 field:none]" media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480 field:none]" media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/640x480 field:none]" gst-launch-1.0 -v v4l2src num-buffers=1 device=/dev/video0 ! jpegenc ! filesink location=test.jpg [ 72.799015] ipu1_ic_prpenc: EOF timeout [ 73.838985] ipu1_ic_prpenc: wait last EOF timeout When I try to jump directly to 4.19-RC8, I get errors regarding memory allocation, so I think there might be something else there I am missing. Has anyone tried this camera module on a 4.14 kernel? I noticed there are a bunch of driver updates, and I was hoping there might be some patches that could be be backported to the 4.14.y stable branch. thanks for any suggestions to try. adam > > Thanks >j > > > > > I've followed these[1] instructions to configure MC links and pads > > based on the probing details from dmesg and trying to capture > > ipu1_ic_prpenc capture (/dev/video1) but it's not working. > > > > Can anyone help me to verify whether I configured all the details > > properly if not please suggest. > > > > I'm pasting full log here, so-that anyone can comment in line and dt > > changes are at [2] > > > > Log: > > - > > > > [1.211866] etnaviv-gpu 2204000.gpu: Ignoring GPU with VG and FE2.0 > > [1.220211] [drm] Initialized etnaviv 1.2.0 20151214 for etnaviv on > > minor 0 > > [1.230344] imx-ipuv3 240.ipu: IPUv3H probed > > [1.237170] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > > [1.243920] [drm] No driver support for vblank timestamp query. > > [1.250831] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops > > ipu_crtc_ops) > > [1.258503] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops > > ipu_crtc_ops) > > [1.266293] imx-drm display-subsystem: bound imx-ipuv3-crtc.6 (ops > > ipu_crtc_ops) > > [1.274027] imx-drm display-subsystem: bound imx-ipuv3-crtc.7 (ops > > ipu_crtc_ops) > > [1.282304] dwhdmi-imx 12.hdmi: Detected HDMI TX controller > > v1.30a with HDCP (DWC HDMI 3D TX PHY) > > [1.295722] imx-drm display-subsystem: bound 12.hdmi (ops > > dw_hdmi_imx_ops) > > [1.373615] Console: switching to colour frame buffer device 128x48 > > [1.396495] imx-drm display-subsystem: fb0: frame buffer device > > [1.404620] [drm] Initialized imx-drm 1.0.0 20120507 for > > display-subsystem on minor 1 > > [1.412763] imx-ipuv3 280.ipu: IPUv3H probed > > [1.439673] brd: module loaded > > [1.469099] loop: module loaded > > [1.480324] nand: No NAND device found > > [1.487768] libphy: Fixed MDIO Bus: probed > > [1.493034] CAN device driver interface > > [1.499057] fec 2188000.ethernet: 2188000.ethernet supply phy not > > found, using dummy regulator > > [1.511633] pps pps0: new PPS source ptp0 > > [1.516928] fec 2188000.ethernet (unnamed net_device) > > (uninitialized): Invalid MAC address: 00:00:00:00:00:00 > > [1.527177] fec 2188000.ethernet (unnamed net_device) > > (uninitialized): Using random MAC address: f2:5a:6d:a6:90:74 > > [1.543567] libphy: fec_enet_mii_bus: probed > > [1.549138] fec 2188000.ethernet eth0: registered PHC device 0 > > [1.556499] usbcore: registered new interface driver asix > > [1.562066] usbcore: registered new interface driver ax88179_178a > > [1.568259] usbcore: registered new interface driver cdc_ether > > [1.574276] usbcore: registered new interface driver net1080 > > [1.580097] usbcore: registered new interface driver cdc_subset > > [1.586144] usbcore: registered new interface driver zaurus > > [1.591910] usbcore: registered new interface driver cdc_ncm > > [1.597589] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver > > [1.604209] ehci-pci: EHCI PCI platform driver > > [1.608760] ehci-mxc: Freescale On-Chip EHCI Host driver > > [1.614851] usbcore: registered new interface driver usb-storage > > [1.6299
Re: [PATCH 4/7] mfd: ds90ux9xx: add TI DS90Ux9xx de-/serializer MFD driver
Hi Laurent, On 10/16/2018 04:12 PM, Laurent Pinchart wrote: > Hi Vladimir, > > On Saturday, 13 October 2018 18:10:25 EEST Vladimir Zapolskiy wrote: >> On 10/12/2018 04:01 PM, Laurent Pinchart wrote: >>> On Friday, 12 October 2018 14:47:52 EEST Kieran Bingham wrote: On 12/10/18 11:58, Vladimir Zapolskiy wrote: >> [snip] >> Essentially they are multi purpose buses - which do not yet have a home. We have used media as a home because of our use case. The use case whether they transfer frames from a camera or to a display are of course closely related, but ultimately covered by two separate subsystems at the pixel level (DRM vs V4L, or other for other data) Perhaps as they are buses - on a level with USB or I2C (except they can of course carry I2C or Serial as well as 'bi-directional video' etc ), they are looking for their own subsystem. Except I don't think we don't want to add a new subsystem for just one (or two) devices... >>> >>> I'm not sure a new subsystem is needed. As you've noted there's an overlap >>> between drivers/media/ and drivers/gpu/drm/ in terms of supported >>> hardware. We even have a devices supported by two drivers, one in drivers/ >>> media/ and one in drivers/gpu/drm/ (I'm thinking about the adv7511 in >>> particular). This is a well known issue, and so far nothing has been done >>> in mainline to try and solve it. >> >> I agree that there's an overlap between drivers/media/ and drivers/gpu/drm/, >> formally a hypothetical (sic!) DS90Ux9xx video bridge cell driver should be >> added into both subsystems also, and the actual driver of two should be >> selected in runtime. I call such a driver 'hypothetical', because in fact I >> don't have it, and I'm not so sure that its existence is justified, but >> that's only because DS90Ux9xx video bridge functionality is _transparent_, >> it does not have any controls literally, but it is a pure luck eventually. > > I don't think that's entirely correct, there's at least the video bus width > (18-bit/24-bit) that needs to be selected. You currently do so through a > pinctrl property, but that's not right. if you deal with a complex IC/IP which supports parallel video output routed over multiplexed pins, you have to specify a pinmux configuration, it is unavoidable (for reference see arch/arm/boot/dts/imx6qdl-sabrelite.dtsi and &pinctrl_j15 pin group, why does pinctrl setting exist?), so the property will remain as a pinmux/pinctrl property in one or another form independently on a probable video bus width selection of a DS90Ux9xx video bridge cell. In this particular case the pinmux/pinctrl driver shall be aware of 'ti,video-depth-18bit' property of 'parallel' pin function to resolve pin resource conflicts with GPIO and audio bridging functions of IC, this is a clear hardware pinmux (or pinctrl of "parallel" function) property. Please don't neglect the complexity and necessity of pinmuxing and other IC functions, if all provided functions of DS90Ux9xx ICs are put aside and just video bridging is left, only then you justify the device as a media device, but the IC and its configuration is simply more complex than you describe it. And, as I've said before, the video bridging function is extremely trivial and it has no real controls, but other functions have. -- Best wishes, Vladimir
Re: [PATCH 1/4] media: ov5640: fix resolution update
Glad this is spurring a lot of conversation, and I’m happy to see this many
contributors too. I think we have all solved many of these problems (and the
many others) offline, and now it’s the hard part to try to glue them all
together. I decided to jump back in the mix with these patches because they
seemed that they would be the easiest to rebase and were obvious bug fixes. I
was hoping we could get this one applied and have a better starting place to
work from before we start what is a very necessary restructure of the mode
setting, but if everybody is ready to go now, we can just do a larger overhaul
and I can withdraw this patch from this series. I expect the larger overhaul to
warrant its own patch series.
I’m not going to comment yet on some of the proposed changes below, because I
think it is better if we just move the discussion to its own series.
Another note, I also agree that prior to changing the rest of mode setting, the
next step is to introduce maximes critical pll setting patch. It is required
for my platform to work at all (yes my platform has never worked with main
line) because my CSI2 receiver needs the PCLK PERIOD register to be set
properly. I actually have tweaked Maximes original patch so that it doesn’t
introduce a regression for MIPI platforms, but it will need to be tested on DVP
platforms. I’ll send it out RFC tonight, so hopefully that will get us closer
to a single patch that doesn’t break things in either mode.
IMO I would like to see this patch broken out of Maximes larger series, because
I think it is getting bogged down by the many other changes there.
With this many interested parties, someone would really be helping if they put
out a organized to do list that we could schedule out :)
> On Oct 16, 2018, at 5:15 AM, Laurent Pinchart
> wrote:
>
> Hi Hugues,
>
>> On Monday, 15 October 2018 18:13:12 EEST Hugues FRUCHET wrote:
>> Hi Laurent, Jacopo, Sam,
>>
>> I'm also OK to change to a simpler alternative;
>> - drop the "restore" step
>> - send the whole init register sequence + mode changes + format changes
>> at streamon
>>
>> is this what you have in mind Laurent ?
>
> Yes, that's pretty much the idea. The init sequence could be sent when
> powering the sensor on to save time at streamon. Everything else can be
> programmed at streamon time to simplify the implementation.
>
>>> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
Hi Sam,
thanks for the patch, I see the same issue you reported, but I
think this patch can be improved.
(expanding the Cc list to all people involved in recent ov5640
developemts, not just for this patch, but for the whole series to look
at. Copying names from another series cover letter, hope it is
complete.)
> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
>
> set_fmt was not properly triggering a mode change when
> a new mode was set that happened to have the same format
> as the previous mode (for example, when only changing the
> frame dimensions). Fix this.
>
> Signed-off-by: Sam Bobrowicz
> ---
>
> drivers/media/i2c/ov5640.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..5031aab 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev
> *sd,
> goto out;
> }
>
> -if (new_mode != sensor->current_mode) {
> +
> +if (new_mode != sensor->current_mode ||
> +mbus_fmt->code != sensor->fmt.code) {
> +sensor->fmt = *mbus_fmt;
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
> -}
> -if (mbus_fmt->code != sensor->fmt.code) {
> -sensor->fmt = *mbus_fmt;
> sensor->pending_fmt_change = true;
> }
How I did reproduce the issue:
# Set 1024x768 on ov5640 without changing the image format
# (default image size at startup is 640x480)
$ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768
field:none]"
sensor->pending_mode_change = true; //verified this flag gets set
# Start streaming, after having configured the whole pipeline to work
# with 1024x768
$ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
Unable to start streaming: Broken pipe (32).
# Inspect which part of pipeline validation went wrong
# Turns out the sensor->fmt field is not updated, and when get_fmt()
# is called, the old one is returned.
$ media-ctl -e "ov5640 2-003c" -p
...
[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
ycbcr:601 quantizati
Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Hello Maxime,
a few comments I have collected while testing the series.
Please see below.
On Thu, Oct 11, 2018 at 11:20:56AM +0200, Maxime Ripard wrote:
> The clock structure for the PCLK is quite obscure in the documentation, and
> was hardcoded through the bytes array of each and every mode.
>
> This is troublesome, since we cannot adjust it at runtime based on other
> parameters (such as the number of bytes per pixel), and we can't support
> either framerates that have not been used by the various vendors, since we
> don't have the needed initialization sequence.
>
> We can however understand how the clock tree works, and then implement some
> functions to derive the various parameters from a given rate. And now that
> those parameters are calculated at runtime, we can remove them from the
> initialization sequence.
>
> The modes also gained a new parameter which is the clock that they are
> running at, from the register writes they were doing, so for now the switch
> to the new algorithm should be transparent.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/media/i2c/ov5640.c | 289 -
> 1 file changed, 288 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 30b15e91d8be..88fb16341466 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> u32 htot;
> u32 vact;
> u32 vtot;
> + u32 pixel_clock;
> const struct reg_value *reg_data;
> u32 reg_data_size;
> };
> @@ -700,6 +701,7 @@ static const struct reg_value
> ov5640_setting_15fps_QSXGA_2592_1944[] = {
> /* power-on sensor init reg table */
> static const struct ov5640_mode_info ov5640_mode_init_data = {
> 0, SUBSAMPLING, 640, 1896, 480, 984,
> + 5600,
> ov5640_init_setting_30fps_VGA,
> ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> };
> @@ -709,74 +711,91 @@
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> + 2800,
>ov5640_setting_15fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> + 2800,
>ov5640_setting_15fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> + 2800,
>ov5640_setting_15fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>720, 1896, 480, 984,
> + 2800,
>ov5640_setting_15fps_NTSC_720_480,
>ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> {OV5640_MODE_PAL_720_576, SUBSAMPLING,
>720, 1896, 576, 984,
> + 2800,
>ov5640_setting_15fps_PAL_720_576,
>ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>1024, 1896, 768, 1080,
> + 2800,
>ov5640_setting_15fps_XGA_1024_768,
>ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>1280, 1892, 720, 740,
> + 2100,
>ov5640_setting_15fps_720P_1280_720,
>ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> {OV5640_MODE_1080P_1920_1080, SCALING,
>1920, 2500, 1080, 1120,
> + 4200,
>ov5640_setting_15fps_1080P_1920_1080,
>ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> {OV5640_MODE_QSXGA_2592_1944, SCALING,
>2592, 2844, 1944, 1968,
> + 8400,
>ov5640_setting_15fps_QSXGA_2592_1944,
>ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
> }, {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> + 5600,
>ov5640_setting_30fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> + 5600,
>ov5640_setting_30fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> + 5600,
>ov5640_setting_30fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>
Re: [PATCH v4 00/12] media: ov5640: Misc cleanup and improvements
Hello Maxime, On Thu, Oct 11, 2018 at 11:20:55AM +0200, Maxime Ripard wrote: > Hi, > > Here is a "small" series that mostly cleans up the ov5640 driver code, > slowly getting rid of the big data array for more understandable code > (hopefully). > > The biggest addition would be the clock rate computation at runtime, > instead of relying on those arrays to setup the clock tree > properly. As a side effect, it fixes the framerate that was off by > around 10% on the smaller resolutions, and we now support 60fps. > > This also introduces a bunch of new features. > > Let me know what you think, I'm sorry to report this breaks CSI-2 capture on my i.MX6 testing platform. I have been testing the whole afternoon with different configurations, but I have not been able yet to find out the root of the problem. In the meantime, I have some comments on the patches, and I'll reply to them singularly. Thanks j > Maxime > > Changes from v3: > - Rebased on current Sakari tree > - Fixed an error when changing only the framerate > > Changes from v2: > - Rebased on latest Sakari PR > - Fixed the issues reported by Hugues: improper FPS returned for > formats, improper rounding of the FPS, some with his suggestions, > some by simplifying the logic. > - Expanded the clock tree comments based on the feedback from Samuel > Bobrowicz and Loic Poulain > - Merged some of the changes made by Samuel Bobrowicz to fix the > MIPI rate computation, fix the call sites of the > ov5640_set_timings function, the auto-exposure calculation call, > etc. > - Split the patches into smaller ones in order to make it more > readable (hopefully) > > Changes from v1: > - Integrated Hugues' suggestions to fix v4l2-compliance > - Fixed the bus width with JPEG > - Dropped the clock rate calculation loops for something simpler as > suggested by Sakari > - Cache the exposure value instead of using the control value > - Rebased on top of 4.17 > > Maxime Ripard (12): > media: ov5640: Adjust the clock based on the expected rate > media: ov5640: Remove the clocks registers initialization > media: ov5640: Remove redundant defines > media: ov5640: Remove redundant register setup > media: ov5640: Compute the clock rate at runtime > media: ov5640: Remove pixel clock rates > media: ov5640: Enhance FPS handling > media: ov5640: Make the return rate type more explicit > media: ov5640: Make the FPS clamping / rounding more extendable > media: ov5640: Add 60 fps support > media: ov5640: Remove duplicate auto-exposure setup > ov5640: Enforce a mode change when changing the framerate > > drivers/media/i2c/ov5640.c | 679 - > 1 file changed, 374 insertions(+), 305 deletions(-) > > -- > 2.19.1 > signature.asc Description: PGP signature
Re: [PATCH v11 0/5] Venus updates - PIL
On 2018-10-08 19:02, Vikash Garodia wrote: This version of the series * extends the description of firmware subnode in documentation. * renames the flag suggesting the presence of tz and update code accordingly. Stanimir Varbanov (1): venus: firmware: register separate platform_device for firmware loader Vikash Garodia (4): venus: firmware: add routine to reset ARM9 venus: firmware: move load firmware in a separate function venus: firmware: add no TZ boot and shutdown routine dt-bindings: media: Document bindings for venus firmware device .../devicetree/bindings/media/qcom,venus.txt | 14 +- drivers/media/platform/qcom/venus/core.c | 24 ++- drivers/media/platform/qcom/venus/core.h | 6 + drivers/media/platform/qcom/venus/firmware.c | 235 +++-- drivers/media/platform/qcom/venus/firmware.h | 17 +- drivers/media/platform/qcom/venus/hfi_venus.c | 13 +- drivers/media/platform/qcom/venus/hfi_venus_io.h | 8 + 7 files changed, 274 insertions(+), 43 deletions(-)
Re: [PATCH v3 6/6] media: video-i2c: support runtime PM
2018年10月16日(火) 0:31 Sakari Ailus :
>
> Hi Mita-san,
>
> On Sun, Oct 14, 2018 at 03:02:39AM +0900, Akinobu Mita wrote:
> > AMG88xx has a register for setting operating mode. This adds support
> > runtime PM by changing the operating mode.
> >
> > The instruction for changing sleep mode to normal mode is from the
> > reference specifications.
> >
> > https://docid81hrs3j1.cloudfront.net/medialibrary/2017/11/PANA-S-A0002141979-1.pdf
> >
> > Cc: Matt Ranostay
> > Cc: Sakari Ailus
> > Cc: Hans Verkuil
> > Cc: Mauro Carvalho Chehab
> > Reviewed-by: Matt Ranostay
> > Acked-by: Sakari Ailus
> > Signed-off-by: Akinobu Mita
> > ---
> > * v3
> > - Move chip->set_power() call to the video device release() callback.
> > - Add Acked-by line
> >
> > drivers/media/i2c/video-i2c.c | 141
> > +-
> > 1 file changed, 139 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> > index 3334fc2..22fdc43 100644
> > --- a/drivers/media/i2c/video-i2c.c
> > +++ b/drivers/media/i2c/video-i2c.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -94,10 +95,23 @@ struct video_i2c_chip {
> > /* xfer function */
> > int (*xfer)(struct video_i2c_data *data, char *buf);
> >
> > + /* power control function */
> > + int (*set_power)(struct video_i2c_data *data, bool on);
> > +
> > /* hwmon init function */
> > int (*hwmon_init)(struct video_i2c_data *data);
> > };
> >
> > +/* Power control register */
> > +#define AMG88XX_REG_PCTL 0x00
> > +#define AMG88XX_PCTL_NORMAL 0x00
> > +#define AMG88XX_PCTL_SLEEP 0x10
> > +
> > +/* Reset register */
> > +#define AMG88XX_REG_RST 0x01
> > +#define AMG88XX_RST_FLAG 0x30
> > +#define AMG88XX_RST_INIT 0x3f
> > +
> > /* Frame rate register */
> > #define AMG88XX_REG_FPSC 0x02
> > #define AMG88XX_FPSC_1FPSBIT(0)
> > @@ -127,6 +141,59 @@ static int amg88xx_setup(struct video_i2c_data *data)
> > return regmap_update_bits(data->regmap, AMG88XX_REG_FPSC, mask, val);
> > }
> >
> > +static int amg88xx_set_power_on(struct video_i2c_data *data)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL,
> > AMG88XX_PCTL_NORMAL);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(50);
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_INIT);
> > + if (ret)
> > + return ret;
> > +
> > + msleep(2);
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_RST, AMG88XX_RST_FLAG);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Wait two frames before reading thermistor and temperature registers
> > + */
> > + msleep(200);
> > +
> > + return 0;
> > +}
> > +
> > +static int amg88xx_set_power_off(struct video_i2c_data *data)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(data->regmap, AMG88XX_REG_PCTL,
> > AMG88XX_PCTL_SLEEP);
> > + if (ret)
> > + return ret;
> > + /*
> > + * Wait for a while to avoid resuming normal mode immediately after
> > + * entering sleep mode, otherwise the device occasionally goes wrong
> > + * (thermistor and temperature registers are not updated at all)
> > + */
> > + msleep(100);
> > +
> > + return 0;
> > +}
> > +
> > +static int amg88xx_set_power(struct video_i2c_data *data, bool on)
> > +{
> > + if (on)
> > + return amg88xx_set_power_on(data);
> > +
> > + return amg88xx_set_power_off(data);
> > +}
> > +
> > #if IS_ENABLED(CONFIG_HWMON)
> >
> > static const u32 amg88xx_temp_config[] = {
> > @@ -158,7 +225,15 @@ static int amg88xx_read(struct device *dev, enum
> > hwmon_sensor_types type,
> > __le16 buf;
> > int tmp;
> >
> > + tmp = pm_runtime_get_sync(regmap_get_device(data->regmap));
> > + if (tmp < 0) {
> > + pm_runtime_put_noidle(regmap_get_device(data->regmap));
> > + return tmp;
> > + }
> > +
> > tmp = regmap_bulk_read(data->regmap, AMG88XX_REG_TTHL, &buf, 2);
> > + pm_runtime_mark_last_busy(regmap_get_device(data->regmap));
> > + pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
> > if (tmp)
> > return tmp;
> >
> > @@ -217,6 +292,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> > .regmap_config = &amg88xx_regmap_config,
> > .setup = &amg88xx_setup,
> > .xfer = &amg88xx_xfer,
> > + .set_power = amg88xx_set_power,
> > .hwmon_init = amg88xx_hwmon_init,
> > },
> > };
> > @@ -343,14 +419,21 @@ static void video_i2c_del_list(struct vb2_queue *vq,
> > enum vb2_buffer_state state
> > static int start_streaming(struct vb2_queue *vq, unsigned int count
[PATCH] media: rc: self test for IR encoders and decoders
ir-loopback can transmit IR on one rc device and check the correct
scancode and protocol is decoded on a different rc device. This can be
used to check IR transmission between two rc devices. Using rc-loopback,
we use it to check the IR encoders and decoders themselves.
No hardware is required for this test.
Signed-off-by: Sean Young
Cc: Shuah Khan
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/ir/.gitignore | 1 +
tools/testing/selftests/ir/Makefile | 5 +
tools/testing/selftests/ir/ir_loopback.c | 199 ++
tools/testing/selftests/ir/ir_loopback.sh | 20 +++
5 files changed, 226 insertions(+)
create mode 100644 tools/testing/selftests/ir/.gitignore
create mode 100644 tools/testing/selftests/ir/Makefile
create mode 100644 tools/testing/selftests/ir/ir_loopback.c
create mode 100755 tools/testing/selftests/ir/ir_loopback.sh
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f1fe492c8e17..995034ea5546 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -15,6 +15,7 @@ TARGETS += futex
TARGETS += gpio
TARGETS += intel_pstate
TARGETS += ipc
+TARGETS += ir
TARGETS += kcmp
TARGETS += kvm
TARGETS += lib
diff --git a/tools/testing/selftests/ir/.gitignore
b/tools/testing/selftests/ir/.gitignore
new file mode 100644
index ..070ea0c75fb8
--- /dev/null
+++ b/tools/testing/selftests/ir/.gitignore
@@ -0,0 +1 @@
+ir_loopback
diff --git a/tools/testing/selftests/ir/Makefile
b/tools/testing/selftests/ir/Makefile
new file mode 100644
index ..f4ba8eb84b95
--- /dev/null
+++ b/tools/testing/selftests/ir/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_PROGS := ir_loopback.sh
+TEST_GEN_PROGS_EXTENDED := ir_loopback
+
+include ../lib.mk
diff --git a/tools/testing/selftests/ir/ir_loopback.c
b/tools/testing/selftests/ir/ir_loopback.c
new file mode 100644
index ..858c19caf224
--- /dev/null
+++ b/tools/testing/selftests/ir/ir_loopback.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0
+// test ir decoder
+//
+// Copyright (C) 2018 Sean Young
+
+// When sending LIRC_MODE_SCANCODE, the IR will be encoded. rc-loopback
+// will send this IR to the receiver side, where we try to read the decoded
+// IR. Decoding happens in a separate kernel thread, so we will need to
+// wait until that is scheduled, hence we use poll to check for read
+// readiness.
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "../kselftest.h"
+
+#define TEST_SCANCODES 10
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+static const struct {
+ enum rc_proto proto;
+ const char *name;
+ unsigned int mask;
+ const char *decoder;
+} protocols[] = {
+ { RC_PROTO_RC5, "rc-5", 0x1f7f, "rc-5" },
+ { RC_PROTO_RC5X_20, "rc-5x-20", 0x1f7f3f, "rc-5" },
+ { RC_PROTO_RC5_SZ, "rc-5-sz", 0x2fff, "rc-5-sz" },
+ { RC_PROTO_JVC, "jvc", 0x, "jvc" },
+ { RC_PROTO_SONY12, "sony-12", 0x1f007f, "sony" },
+ { RC_PROTO_SONY15, "sony-15", 0xff007f, "sony" },
+ { RC_PROTO_SONY20, "sony-20", 0x1fff7f, "sony" },
+ { RC_PROTO_NEC, "nec", 0x, "nec" },
+ { RC_PROTO_NECX, "nec-x", 0xff, "nec" },
+ { RC_PROTO_NEC32, "nec-32", 0x, "nec" },
+ { RC_PROTO_SANYO, "sanyo", 0x1f, "sanyo" },
+ { RC_PROTO_RC6_0, "rc-6-0", 0x, "rc-6" },
+ { RC_PROTO_RC6_6A_20, "rc-6-6a-20", 0xf, "rc-6" },
+ { RC_PROTO_RC6_6A_24, "rc-6-6a-24", 0xff, "rc-6" },
+ { RC_PROTO_RC6_6A_32, "rc-6-6a-32", 0x, "rc-6" },
+ { RC_PROTO_RC6_MCE, "rc-6-mce", 0x7fff, "rc-6" },
+ { RC_PROTO_SHARP, "sharp", 0x1fff, "sharp" },
+};
+
+int lirc_open(const char *rc)
+{
+ struct dirent *dent;
+ char buf[100];
+ DIR *d;
+ int fd;
+
+ snprintf(buf, sizeof(buf), "/sys/class/rc/%s", rc);
+
+ d = opendir(buf);
+ if (!d)
+ ksft_exit_fail_msg("cannot open %s: %m\n", buf);
+
+ while ((dent = readdir(d)) != NULL) {
+ if (!strncmp(dent->d_name, "lirc", 4)) {
+ snprintf(buf, sizeof(buf), "/dev/%s", dent->d_name);
+ break;
+ }
+ }
+
+ if (!dent)
+ ksft_exit_fail_msg("cannot find lirc device for %s\n", rc);
+
+ closedir(d);
+
+ fd = open(buf, O_RDWR | O_NONBLOCK);
+ if (fd == -1)
+ ksft_exit_fail_msg("cannot open: %s: %m\n", buf);
+
+ return fd;
+}
+
+int main(int argc, char **argv)
+{
+ unsigned int mode;
+ char buf[100];
+ int rlircfd, wlircfd, protocolfd, i, n;
+
+ srand(time(NULL));
+
+ if (argc != 3)
+ ksft_exit_fail_msg("Usage: %s \n",
+ argv[0]);
+
+ rlircfd = lirc_ope
Re: [PATCH 5/7] mfd: ds90ux9xx: add I2C bridge/alias and link connection driver
Hi Vladimir, On Friday, 12 October 2018 17:36:39 EEST Vladimir Zapolskiy wrote: > On 10/12/2018 04:12 PM, Laurent Pinchart wrote: > > On Friday, 12 October 2018 10:32:32 EEST Vladimir Zapolskiy wrote: > >> On 10/12/2018 09:04 AM, Lee Jones wrote: > >>> On Tue, 09 Oct 2018, Vladimir Zapolskiy wrote: > From: Vladimir Zapolskiy > > The change adds TI DS90Ux9xx I2C bridge/alias subdevice driver and > FPD Link connection handling mechanism. > > Access to I2C devices connected to a remote de-/serializer is done in > a transparent way, on established link detection event such devices > are registered on an I2C bus, which serves a local de-/serializer IC. > > The development of the driver was a collaborative work, the > contribution done by Balasubramani Vivekanandan includes: > * original simplistic implementation of the driver, > * support of implicitly specified devices in device tree, > * support of multiple FPD links for TI DS90Ux9xx, > * other kind of valuable review comments, clean-ups and fixes. > > Also Steve Longerbeam made the following changes: > * clear address maps after linked device removal, > * disable pass-through in disconnection, > * qualify locked status with non-zero remote address. > > Signed-off-by: Vladimir Zapolskiy > --- > > drivers/mfd/Kconfig| 8 + > drivers/mfd/Makefile | 1 + > drivers/mfd/ds90ux9xx-i2c-bridge.c | 764 + > 3 files changed, 773 insertions(+) > create mode 100644 drivers/mfd/ds90ux9xx-i2c-bridge.c > >>> > >>> Shouldn't this live in drivers/i2c? > >> > >> no, the driver is not for an I2C controller of any kind, and the driver > >> does not register itself in the I2C subsystem by calling > >> i2c_add_adapter() or i2c_add_numbered_adapter() or i2c_mux_add_adapter() > >> etc, this topic was discussed with Wolfram also. > > > > (Who is now on CC) > > Wolfram has copies of the drivers and discussion right from the beginning, > hopefully he won't get two copies ;) > > >> Formally the driver converts the managed IC into a multi-address I2C > >> slave device, I understand that it does not sound like a well suited > >> driver for MFD, but ds90ux9xx-core.c and ds90ux9xx-i2c-bridge.c drivers > >> are quite tightly coupled. > > > > As mentioned in other e-mails in this thread I don't think this should be > > split out to a separate driver,> I would move the functionality to the > > ds90ux9xx driver. > > The proposal may have the grounds, but the I2C bridging functionality of ICs > is quite detached from all other ones, thus it found its place in the cell > driver per se. > > > You may want to register an I2C mux, but as you have a single port, that > > could be overkill. I haven't studied in details how to best support this > > chip using the existing I2C subsystems APIs (which we may want to extend > > if it needed), but I believe that (in your use cases) the deserializer > > should be a child of the serializer, and modeled as an I2C device. > > Formally in OF terms to define a link between devices by a phandle should > be sufficient, panels are not the children of LVDS controllers under OF > graph constraints in DT representation, the panels become secondary in > runtime only, I'd like to reuse the concept. Also it adds a better sense of > symmetry of deserializer <-> serializer connections relatively to a > SoC/data source. As I explained, DT models control buses through parent/child relationships. That's why I2C slaves are children of their I2C master. OF graph adds a second topology to describe data connections, which are orthogonal to the control bus relationship. In your case the device at the remote side of the link is controlled over the link, and that control flow goes from the SoC to the device on the local side of the link. That's why the remote device should be a child of the local device. -- Regards, Laurent Pinchart
Re: [PATCH 2/2] media: docs-rst: Document memory-to-memory video encoder interface
On 10/16/18 09:36, Tomasz Figa wrote: > On Tue, Aug 7, 2018 at 3:54 PM Tomasz Figa wrote: + * The driver must expose following selection targets on ``OUTPUT``: + + ``V4L2_SEL_TGT_CROP_BOUNDS`` + maximum crop bounds within the source buffer supported by the + encoder + + ``V4L2_SEL_TGT_CROP_DEFAULT`` + suggested cropping rectangle that covers the whole source picture + + ``V4L2_SEL_TGT_CROP`` + rectangle within the source buffer to be encoded into the + ``CAPTURE`` stream; defaults to ``V4L2_SEL_TGT_CROP_DEFAULT`` + + ``V4L2_SEL_TGT_COMPOSE_BOUNDS`` + maximum rectangle within the coded resolution, which the cropped + source frame can be output into; always equal to (0, 0)x(width of + ``V4L2_SEL_TGT_CROP``, height of ``V4L2_SEL_TGT_CROP``), if the + hardware does not support compose/scaling Re-reading this I would rewrite this a bit: if the hardware does not support composition or scaling, then this is always equal to (0, 0)x(width of ``V4L2_SEL_TGT_CROP``, height of ``V4L2_SEL_TGT_CROP``). + + ``V4L2_SEL_TGT_COMPOSE_DEFAULT`` + equal to ``V4L2_SEL_TGT_CROP`` + + ``V4L2_SEL_TGT_COMPOSE`` + rectangle within the coded frame, which the cropped source frame + is to be output into; defaults to + ``V4L2_SEL_TGT_COMPOSE_DEFAULT``; read-only on hardware without + additional compose/scaling capabilities; resulting stream will + have this rectangle encoded as the visible rectangle in its + metadata + + ``V4L2_SEL_TGT_COMPOSE_PADDED`` + always equal to coded resolution of the stream, as selected by the + encoder based on source resolution and crop/compose rectangles >>> >>> Are there codec drivers that support composition? I can't remember seeing >>> any. >>> >> >> Hmm, I was convinced that MFC could scale and we just lacked support >> in the driver, but I checked the documentation and it doesn't seem to >> be able to do so. I guess we could drop the COMPOSE rectangles for >> now, until we find any hardware that can do scaling or composing on >> the fly. >> > > On the other hand, having them defined already wouldn't complicate > existing drivers too much either, because they would just handle all > of them in the same switch case, i.e. > > case V4L2_SEL_TGT_COMPOSE_BOUNDS: > case V4L2_SEL_TGT_COMPOSE_DEFAULT: > case V4L2_SEL_TGT_COMPOSE: > case V4L2_SEL_TGT_COMPOSE_PADDED: > return visible_rectangle; > > That would need one change, though. We would define > V4L2_SEL_TGT_COMPOSE_DEFAULT to be equal to (0, 0)x(width of > V4L2_SEL_TGT_CROP - 1, height of ``V4L2_SEL_TGT_CROP - 1), which " - 1"? Where does that come from? Usually rectangles are specified as widthxheight@left,top. > makes more sense than current definition, since it would bypass any > compose/scaling by default. I have no problem with drivers optionally implementing these rectangles, even if they don't do scaling or composition. The question is, should it be required for decoders? If there is a good reason, then I'm OK with it. Regards, Hans
Re: [RFP] Which V4L2 ioctls could be replaced by better versions?
On 10/03/18 10:24, Tomasz Figa wrote: > On Fri, Sep 21, 2018 at 3:14 AM Nicolas Dufresne wrote: >> >> Le jeudi 20 septembre 2018 à 16:42 +0200, Hans Verkuil a écrit : >>> Some parts of the V4L2 API are awkward to use and I think it would be >>> a good idea to look at possible candidates for that. >>> >>> Examples are the ioctls that use struct v4l2_buffer: the multiplanar >>> support is >>> really horrible, and writing code to support both single and multiplanar is >>> hard. >>> We are also running out of fields and the timeval isn't y2038 compliant. >>> >>> A proof-of-concept is here: >>> >>> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=v4l2-buffer&id=a95549df06d9900f3559afdbb9da06bd4b22d1f3 >>> >>> It's a bit old, but it gives a good impression of what I have in mind. >>> >>> Another candidate is >>> VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL/VIDIOC_ENUM_FRAMEINTERVALS: >>> expressing frame intervals as a fraction is really awkward and so is the >>> fact >>> that the subdev and 'normal' ioctls are not the same. >>> >>> Would using nanoseconds or something along those lines for intervals be >>> better? >> >> This one is not a good idea, because you cannot represent well known >> rates used a lot in the field. Like 6/1001 also known as 59.94 Hz. >> You could endup with drift issues. >> >> For me, what is the most difficult with this API is the fact that it >> uses frame internal (duration) instead of frame rate. >> >>> >>> I have similar concerns with VIDIOC_SUBDEV_ENUM_FRAME_SIZE where there is no >>> stepwise option, making it different from VIDIOC_ENUM_FRAMESIZES. But it >>> should >>> be possible to extend VIDIOC_SUBDEV_ENUM_FRAME_SIZE with stepwise support, I >>> think. >> >> One of the thing to fix, maybe it's doable now, is the differentiation >> between allocation size and display size. Pretty much all video capture >> code assumes this is display size and ignores the selection API. This >> should be documented explicitly. > > Technically, there is already a distinction between allocation and > display size at format level - width and height could be interpreted > as the rectangle containing the captured video, while bytesperline and > sizeimage would match to the allocation size. The behavior between > drivers seems to be extremely variable - some would just enforce > bytesperline = bpp*width and sizeimage = bytesperline*height, while > others would actually give control over them to the user space. > > It's a bit more complicated with video decoders, because the stream, > in addition to the 2 sizes mentioned before, is also characterized by > coded size, which corresponds to the actual number of pixels encoded > in the stream, even if not all contain pixels to be displayed. This is > something that needs to be specified explicitly (and is, in my > documentation patches) in the Codec Interface. > >> >> In fact, the display/allocation dimension isn't very nice, as both >> information overlaps in structures. As an example, you call S_FMT with >> a display size you want, and it will return you an allocation size >> (which yes, can be smaller, because we always round to the middle). >> >>> >>> Do we have more ioctls that could use a refresh? S/G/TRY_FMT perhaps, again >>> in >>> order to improve single vs multiplanar handling. >> >> Yes, but that would fall in a complete redesign I guess. The buffer >> allocation scheme is very inflexible. You can't have buffers of two >> dimensions allocated at the same time for the same queue. Worst, you >> cannot leave even 1 buffer as your scannout buffer while reallocating >> new buffers, this is not permitted by the framework (in software). As a >> side effect, there is no way to optimize the resolution changes, you >> even have to copy your scannout buffer on the CPU, to free it in order >> to proceed. Resolution changes are thus painfully slow, by design. > > Hmm? There is VIDIOC_CREATEBUFS, which can allows you to allocate > buffers for explicitly specified format, with other buffers already > existing in the queue. Of course, we are missing the VIDIOC_DELETEBUFS ioctl. Also, CREATEBUFS is rather awful. Using v4l2_format in the struct was a major mistake. > > Also, I fail to understand the scanout issue. If one exports a vb2 > buffer to a DMA-buf and import it to the scanout engine, it can keep > scanning out from it as long as it want, because the DMA-buf will hold > a reference on the buffer, even if it's removed from the vb2 queue. > >> >> You also cannot switch from internal buffers to importing buffers >> easily (in some case you, like encoder, you cannot do that without >> flushing the encoder state). > > Hmm, technically VIDIOC_CREATEBUFS accepts the "memory" type value, > but I'm not sure what happens if the queue already has buffers > requested with different one. It is not allowed to mix memory types, that will return -EINVAL. I have to say that this is the first time I had this request. It is probably doable, but the use
Re: [PATCH 4/7] mfd: ds90ux9xx: add TI DS90Ux9xx de-/serializer MFD driver
Hi Vladimir, On Saturday, 13 October 2018 18:10:25 EEST Vladimir Zapolskiy wrote: > On 10/12/2018 04:01 PM, Laurent Pinchart wrote: > > On Friday, 12 October 2018 14:47:52 EEST Kieran Bingham wrote: > >> On 12/10/18 11:58, Vladimir Zapolskiy wrote: > [snip] > > >> Essentially they are multi purpose buses - which do not yet have a home. > >> We have used media as a home because of our use case. > >> > >> The use case whether they transfer frames from a camera or to a display > >> are of course closely related, but ultimately covered by two separate > >> subsystems at the pixel level (DRM vs V4L, or other for other data) > >> > >> Perhaps as they are buses - on a level with USB or I2C (except they can > >> of course carry I2C or Serial as well as 'bi-directional video' etc ), > >> they are looking for their own subsystem. > >> > >> Except I don't think we don't want to add a new subsystem for just one > >> (or two) devices... > > > > I'm not sure a new subsystem is needed. As you've noted there's an overlap > > between drivers/media/ and drivers/gpu/drm/ in terms of supported > > hardware. We even have a devices supported by two drivers, one in drivers/ > > media/ and one in drivers/gpu/drm/ (I'm thinking about the adv7511 in > > particular). This is a well known issue, and so far nothing has been done > > in mainline to try and solve it. > > I agree that there's an overlap between drivers/media/ and drivers/gpu/drm/, > formally a hypothetical (sic!) DS90Ux9xx video bridge cell driver should be > added into both subsystems also, and the actual driver of two should be > selected in runtime. I call such a driver 'hypothetical', because in fact I > don't have it, and I'm not so sure that its existence is justified, but > that's only because DS90Ux9xx video bridge functionality is _transparent_, > it does not have any controls literally, but it is a pure luck eventually. I don't think that's entirely correct, there's at least the video bus width (18-bit/24-bit) that needs to be selected. You currently do so through a pinctrl property, but that's not right. > So, as I've stated in my cover letter, I can misuse yours 'lvds-encoder' > driver only for the purpose of establishing a mediated link between > an LVDS controller and a panel over a serializer-deserializer pair. > > > Trying to find another home in drivers/mfd/ to escape from the problem > > isn't a good solution in my opinion. The best option from a Linux point > > of view would be to unify V4L2 and DRM/KMS when it comes to bridge > > support, but that's a long way down the road (I won't complain if you > > want to give it a go though> > > :-)). > > I return you a wider smile :) > > > As your use cases are display, focused, I would propose to start with > > drivers/gpu/drm/bridge/, and leave the problem of camera support for first > > person who will have such a use case. > > Frankly speaking I would like to start from copy-pasting your 'lvds-encoder' > driver into an 'absolutely-transparent-video-bridge' driver with no LVDS or > 'encoder' specifics, adding just a new compatible may suffice, if the > driver is renamed/redefined. > > PS, I remember I owe you a reference OF snippet of data path to a panel. -- Regards, Laurent Pinchart
Re: [PATCH 4/7] mfd: ds90ux9xx: add TI DS90Ux9xx de-/serializer MFD driver
Hi Vladimir,
On Friday, 12 October 2018 16:59:27 EEST Vladimir Zapolskiy wrote:
> On 10/12/2018 04:01 PM, Laurent Pinchart wrote:
> > On Friday, 12 October 2018 14:47:52 EEST Kieran Bingham wrote:
> >> On 12/10/18 11:58, Vladimir Zapolskiy wrote:
> >>> On 10/12/2018 12:20 PM, Kieran Bingham wrote:
> On 12/10/18 09:39, Lee Jones wrote:
> > On Fri, 12 Oct 2018, Vladimir Zapolskiy wrote:
> >> On 10/12/2018 09:03 AM, Lee Jones wrote:
> >>> On Tue, 09 Oct 2018, Vladimir Zapolskiy wrote:
> From: Vladimir Zapolskiy
>
> The change adds I2C device driver for TI DS90Ux9xx de-/serializers,
> support of subdevice controllers is done in separate drivers,
> because not all IC functionality may be needed in particular
> situations, and this can be fine grained controlled in device tree.
>
> The development of the driver was a collaborative work, the
> contribution done by Balasubramani Vivekanandan includes:
> * original implementation of the driver based on a reference
> driver,
> * regmap powered interrupt controller support on serializers,
> * support of implicitly or improperly specified in device tree ICs,
> * support of device properties and attributes: backward compatible
> mode, low frequency operation mode, spread spectrum clock
> generator.
>
> Contribution by Steve Longerbeam:
> * added ds90ux9xx_read_indirect() function,
> * moved number of links property and added
> ds90ux9xx_num_fpd_links(),
> * moved and updated ds90ux9xx_get_link_status() function to core
> driver,
> * added fpd_link_show device attribute.
>
> Sandeep Jain added support of pixel clock edge configuration.
>
> Signed-off-by: Vladimir Zapolskiy
> ---
>
> drivers/mfd/Kconfig | 14 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/ds90ux9xx-core.c | 879
>
> include/linux/mfd/ds90ux9xx.h | 42 ++
> 4 files changed, 936 insertions(+)
> create mode 100644 drivers/mfd/ds90ux9xx-core.c
> create mode 100644 include/linux/mfd/ds90ux9xx.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8c5dfdce4326..a969fa123f64 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1280,6 +1280,20 @@ config MFD_DM355EVM_MSP
>
> boards. MSP430 firmware manages resets and power sequencing,
> inputs from buttons and the IR remote, LEDs, an RTC, and more.
>
> +config MFD_DS90UX9XX
> +tristate "TI DS90Ux9xx FPD-Link de-/serializer driver"
> +depends on I2C && OF
> +select MFD_CORE
> +select REGMAP_I2C
> +help
> + Say yes here to enable support for TI DS90UX9XX de-/serializer
> ICs.
> +
> + This driver provides basic support for setting up the
> de-/serializer
> + chips. Additional functionalities like connection handling to
> + remote de-/serializers, I2C bridging, pin multiplexing, GPIO
> + controller and so on are provided by separate drivers and
> should
> + enabled individually.
> >>>
> >>> This is not an MFD driver.
> >>
> >> Why do you think so? The representation of the ICs into device tree
> >> format of hardware description shows that this is a truly MFD driver
> >> with multiple IP subcomponents naturally mapped into MFD cells.
> >
> > This driver does too much real work ('stuff') to be an MFD driver.
> > MFD drivers should not need to care of; links, gates, modes, pixels,
> > frequencies maps or properties. Nor should they contain elaborate
> > sysfs structures to control the aforementioned 'stuff'.
> >
> > Granted, there may be some code in there which could be appropriate
> > for an MFD driver. However most of it needs moving out into a
> > function driver (or two).
> >
> >> Basically it is possible to replace explicit of_platform_populate()
> >> by adding a "simple-mfd" compatible, if it is desired.
> >>
> >>> After a 30 second Google of what this device actually does, perhaps
> >>> drivers/media might be a better fit?
> >>
> >> I assume it would be quite unusual to add a driver with NO media
> >> functions and controls into drivers/media.
> >
> > drivers/media may very well not be the correct place for this. In my
> > 30 second Google, I saw that this device has a lot to do with cameras,
> > hence my media association.
> >
> > If *all* else fails, there is always drivers/misc,
HI
I am sgt lisa. am sending this brief letter to solicit your partnership to transfer $3.5 M million US Dollars. I shall send you more information and procedures when I receive positive response from you. sgt lisa. (sgtlis...@gmail.com)
Re: [PATCH 3/7] dt-bindings: pinctrl: ds90ux9xx: add description of TI DS90Ux9xx pinmux
Hi Vladimir,
On Saturday, 13 October 2018 16:47:48 EEST Vladimir Zapolskiy wrote:
> On 10/12/2018 03:01 PM, Laurent Pinchart wrote:
> > On Tuesday, 9 October 2018 00:12:01 EEST Vladimir Zapolskiy wrote:
> >> From: Vladimir Zapolskiy
> >>
> >> TI DS90Ux9xx de-/serializers have a capability to multiplex pin
> >> functions, in particular a pin may have selectable functions of GPIO,
> >> GPIO line transmitter, one of I2S lines, one of RGB24 video signal lines
> >> and so on.
> >>
> >> The change adds a description of DS90Ux9xx pin multiplexers and GPIO
> >> controllers.
> >>
> >> Signed-off-by: Vladimir Zapolskiy
> >> ---
> >>
> >> .../bindings/pinctrl/ti,ds90ux9xx-pinctrl.txt | 83 +++
> >> 1 file changed, 83 insertions(+)
> >> create mode 100644
> >>
> >> Documentation/devicetree/bindings/pinctrl/ti,ds90ux9xx-pinctrl.txt
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/pinctrl/ti,ds90ux9xx-pinctrl.txt
> >> b/Documentation/devicetree/bindings/pinctrl/ti,ds90ux9xx-pinctrl.txt new
> >> file mode 100644
> >> index ..fbfa1a3cdf9f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pinctrl/ti,ds90ux9xx-pinctrl.txt
> >> @@ -0,0 +1,83 @@
> >> +TI DS90Ux9xx de-/serializer pinmux and GPIO subcontroller
> >> +
> >> +Required properties:
> >> +- compatible: Must contain a generic "ti,ds90ux9xx-pinctrl" value and
> >> + may contain one more specific value from the list:
> >> + "ti,ds90ux925-pinctrl",
> >> + "ti,ds90ux926-pinctrl",
> >> + "ti,ds90ux927-pinctrl",
> >> + "ti,ds90ux928-pinctrl",
> >> + "ti,ds90ux940-pinctrl".
> >
> > No need for a subnode, you can mark the main DT node with gpio-controller
> > directly.
>
> If the IC is seen as an MFD, and you guess I highly prefer it and I object
> the "overkill" argument, then the subnode is requred.
>
> Also the more complicated part of the subcontroller and its device driver
> is to provide pinmuxing function to consumers rather than to allow GPIO
> line configuration.
>
> The pinctrl/GPIO driver can not be alloyed with the base driver's code
> to sustain maintainability, so it will reside in drivers/pinctrl as
> a separate cell driver, and by the way that is the reason why it earns
> its own very non-trivial DT binding description documentation.
>
> >> +- gpio-controller: Marks the device node as a GPIO controller.
> >> +
> >> +- #gpio-cells: Must be set to 2,
> >> + - the first cell is the GPIO offset number within the controller,
> >> + - the second cell is used to specify the GPIO line polarity.
> >> +
> >> +- gpio-ranges: Mapping to pin controller pins (as described in
> >> + Documentation/devicetree/bindings/gpio/gpio.txt)
> >> +
> >> +Optional properties:
> >> +- ti,video-depth-18bit: Sets video bridge pins to RGB 18-bit mode.
> >
> > Please use standard properties to configure bus width. There is one
> > defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> Here it is not a bus width description or property, but rather it is a
> custom pinmux control.
>
> It could make sense to reduce the scope of the property to "parallel" pin
> function only though, like in
>
> ds90ux927_0_pins: pinmux {
> parallel {
> groups = "parallel";
> function = "parallel";
> ti,video-depth-18bit;
> };
> };
>
> Alternatively the removal of the property would be almost loseless, it is
> needed just in one very specific case, please reference to the driver code
> for details, there you'll find a comment in ds90ux9xx_parse_dt_properties()
> function.
Based on the information I gathered from the DS90UH92[567] datasheets, the
restriction on GPIO usage related to 18-bit mode is due to the signals being
multiplexed on the same pins on DS90UH925. It could also be that the FPD-Link
protocol itself can't carry both GPIO and the extra 6 colour bits.
In any case, I don't think the property belongs here. The bus width should be
specified in the DT bindings for the video ports, and the drivers should then
use that information to configure other parameters, possibly GPIO-specific, if
needed.
> >> +Available pins, groups and functions (reference to device datasheets):
> >> +
> >> +function: "gpio" ("gpio4" is on DS90Ux925 and DS90Ux926 only,
> >> +"gpio9" is on DS90Ux940 only)
> >> + - pins: "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5", "gpio6",
> >> + "gpio7", "gpio8", "gpio9"
> >> +
> >> +function: "gpio-remote"
> >> + - pins: "gpio0", "gpio1", "gpio2", "gpio3"
> >> +
> >> +function: "pass" (DS90Ux940 specific only)
> >> + - pins: "gpio0", "gpio3"
> >
> > What do those functions mean ?
>
> "gpio" function should be already familiar to you.
I assume this function is only available for the local device, not the remote
one ?
> "gpio-remote" function is the pin function for a GPIO line bridging.
>
> "pass" function sets a pin to a status pin function for detect
Re: [PATCH 6/8] drm: sti: don't pass GFP_DMA32 to dma_alloc_wc
Le lun. 15 oct. 2018 à 11:12, Benjamin Gaignard
a écrit :
>
> Le sam. 13 oct. 2018 à 17:17, Christoph Hellwig a écrit :
> >
> > The DMA API does its own zone decisions based on the coherent_dma_mask.
> >
> > Signed-off-by: Christoph Hellwig
>
> Reviewed-by: Benjamin Gaignard
Christoph do you plan to merge this patch on your own tree ? or would
like I put it directly in drm-misc-next branch ?
Regards,
Benjamin
>
> > ---
> > drivers/gpu/drm/sti/sti_gdp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> > index c32de6cbf061..cdf0a1396e00 100644
> > --- a/drivers/gpu/drm/sti/sti_gdp.c
> > +++ b/drivers/gpu/drm/sti/sti_gdp.c
> > @@ -517,7 +517,7 @@ static void sti_gdp_init(struct sti_gdp *gdp)
> > /* Allocate all the nodes within a single memory page */
> > size = sizeof(struct sti_gdp_node) *
> > GDP_NODE_PER_FIELD * GDP_NODE_NB_BANK;
> > - base = dma_alloc_wc(gdp->dev, size, &dma_addr, GFP_KERNEL |
> > GFP_DMA);
> > + base = dma_alloc_wc(gdp->dev, size, &dma_addr, GFP_KERNEL);
> >
> > if (!base) {
> > DRM_ERROR("Failed to allocate memory for GDP node\n");
> > --
> > 2.19.1
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Benjamin Gaignard
>
> Graphic Study Group
>
> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: Facebook | Twitter | Blog
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH 1/7] dt-bindings: mfd: ds90ux9xx: add description of TI DS90Ux9xx ICs
Hi Vladimir, On Saturday, 13 October 2018 17:28:30 EEST Vladimir Zapolskiy wrote: > On 10/12/2018 02:44 PM, Laurent Pinchart wrote: > > On Tuesday, 9 October 2018 00:11:59 EEST Vladimir Zapolskiy wrote: > >> From: Sandeep Jain > >> > >> The change adds device tree binding description of TI DS90Ux9xx > >> series of serializer and deserializer controllers which support video, > >> audio and control data transmission over FPD-III Link connection. > >> > >> Signed-off-by: Sandeep Jain > >> [vzapolskiy: various updates and corrections of secondary importance] > >> Signed-off-by: Vladimir Zapolskiy > >> --- > >> > >> .../devicetree/bindings/mfd/ti,ds90ux9xx.txt | 66 +++ > >> 1 file changed, 66 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt > >> > >> diff --git a/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt > >> b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt new file mode > >> 100644 > >> index ..0733da88f7ef > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/mfd/ti,ds90ux9xx.txt > >> @@ -0,0 +1,66 @@ > >> +Texas Instruments DS90Ux9xx de-/serializer controllers > >> + > >> +Required properties: > >> +- compatible: Must contain a generic "ti,ds90ux9xx" value and > > > >> + may contain one more specific value from the list: > > If it "may" contain one more specific value, when should that value be > > present, and when can it be absent ? > > Practically you can always omit a specific compatible, because (with a > number of minor exceptions like DS90UH925Q case, see a quirk in the code) > it is possible to read out the IC type in runtime. > > Nevertheless I prefer to have a complete list of all specific compatibles > to avoid problems with maintenance in future, I agree with you. Let's thus word this as Shall contain exactly one value from the list below, and the generic "ti,ds90ux9xx" value, in that order. > recently I had a long > discussion with Jassi Brar about iMX* mailbox compatibles on the DT mailing > list, the arguments remain the same, but I don't feel enough internal power > to start another such an exhaustive discussion right at the moment. > > >> + "ti,ds90ub925q", > >> + "ti,ds90uh925q", > >> + "ti,ds90ub927q", > >> + "ti,ds90uh927q", > >> + "ti,ds90ub926q", > >> + "ti,ds90uh926q", > >> + "ti,ds90ub928q", > >> + "ti,ds90uh928q", > >> + "ti,ds90ub940q", > >> + "ti,ds90uh940q". > >> + > >> +Optional properties: > >> +- reg : Specifies the I2C slave address of a local de-/serializer. > > > > You should explain when the reg property is required and when it isn't. > > This > > Talking about TI DS90Ux9xx IC series, ideally I'd like to shift from > serializer/deserializer concept and promote "remote" and "local" IC, by the > way, and if I'm not mistaken, MOST ICs are truly identical on both ends. > > So, here "reg" property is need only if the IC (serializer or deserializer, > it does not matter) is on the "local" side, i.e. it is a slave I2C device > discovered on an I2C bus, which is under control by an application > processor. > > If IC is on the "remote" side, in other words separated by the serial link > from the "local" IC, then "reg" property is not needed. Let's document this then. Please make sure to document the local and remote concepts in the introduction first. > > will in my opinion require a more detailed explanation of the DT model for > > this device. > > > >> +- power-gpios : GPIO line to control supplied power to the device. > > > > As Marek mentioned, a regulator would be better. I would make it a > > mandatory property, as the device always needs to be powered. > > I get a memory flashback. Did we discuss recently a right property name to > control panel power by a GPIO or was it something else? > > There are quite many properties of exactly the same functionality: > * powerdown-gpios > * pd-gpios > * pdn-gpios > * power-gpios > * powerdn-gpio > * power-down-gpios > * ... > > Probably device tree maintainers should unify the names, but my point is > that your argument should be applicable to all such device tree nodes / > property descriptions and usages. Do I understand you correctly? The general agreement is that we should try to standardize the naming of GPIOs. As powerdown is the inverse of enable, it has been proposed to use enable-gpios instead. > I would prefer to reference to a regulator while dealing with the power > rails, and reference to a GPIO in case of power control only like in the > case above. The patch made me think that the GPIO controls a regulator, which is why I advised using regulators. If that's not the case, if the GPIO is connected to a pin of the device, you should document which pin, and rephrase the description to remove the ambiguity. > >> +- ti,backward-compatible-mode : Overrides backward compatibility mode. > >> + Possible values are "<1>" or "<0>". > >> + If "ti,backward-compatible-mode" is
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Hugues,
On Monday, 15 October 2018 18:13:12 EEST Hugues FRUCHET wrote:
> Hi Laurent, Jacopo, Sam,
>
> I'm also OK to change to a simpler alternative;
> - drop the "restore" step
> - send the whole init register sequence + mode changes + format changes
> at streamon
>
> is this what you have in mind Laurent ?
Yes, that's pretty much the idea. The init sequence could be sent when
powering the sensor on to save time at streamon. Everything else can be
programmed at streamon time to simplify the implementation.
> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
> > On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
> >
> >> Hi Sam,
> >>
> >> thanks for the patch, I see the same issue you reported, but I
> >> think this patch can be improved.
> >>
> >> (expanding the Cc list to all people involved in recent ov5640
> >> developemts, not just for this patch, but for the whole series to look
> >> at. Copying names from another series cover letter, hope it is
> >> complete.)
> >>
> >> On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> >>
> >>> set_fmt was not properly triggering a mode change when
> >>> a new mode was set that happened to have the same format
> >>> as the previous mode (for example, when only changing the
> >>> frame dimensions). Fix this.
> >>>
> >>> Signed-off-by: Sam Bobrowicz
> >>> ---
> >>>
> >>> drivers/media/i2c/ov5640.c | 8
> >>> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >>> index eaefdb5..5031aab 100644
> >>> --- a/drivers/media/i2c/ov5640.c
> >>> +++ b/drivers/media/i2c/ov5640.c
> >>> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev
> >>> *sd,
> >>> goto out;
> >>> }
> >>>
> >>> - if (new_mode != sensor->current_mode) {
> >>> +
> >>> + if (new_mode != sensor->current_mode ||
> >>> + mbus_fmt->code != sensor->fmt.code) {
> >>> + sensor->fmt = *mbus_fmt;
> >>> sensor->current_mode = new_mode;
> >>> sensor->pending_mode_change = true;
> >>> - }
> >>> - if (mbus_fmt->code != sensor->fmt.code) {
> >>> - sensor->fmt = *mbus_fmt;
> >>> sensor->pending_fmt_change = true;
> >>> }
> >>
> >> How I did reproduce the issue:
> >>
> >> # Set 1024x768 on ov5640 without changing the image format
> >> # (default image size at startup is 640x480)
> >> $ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768
> >> field:none]"
> >>sensor->pending_mode_change = true; //verified this flag gets set
> >>
> >> # Start streaming, after having configured the whole pipeline to work
> >> # with 1024x768
> >> $ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
> >> Unable to start streaming: Broken pipe (32).
> >>
> >> # Inspect which part of pipeline validation went wrong
> >> # Turns out the sensor->fmt field is not updated, and when get_fmt()
> >> # is called, the old one is returned.
> >> $ media-ctl -e "ov5640 2-003c" -p
> >>...
> >>[fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb xfer:srgb
> >>ycbcr:601 quantization:full-range] ^^^ ^^^
> >>
> >> So yes, sensor->fmt is not udapted as it should be when only image
> >> resolution is changed.
> >>
> >> Although I still see value in having two separate flags for the
> >> 'mode_change' (which in ov5640 lingo is resolution) and 'fmt_change'
> >> (which in ov5640 lingo is the image format), and write their
> >> configuration to registers only when they get actually changed.
> >>
> >> For this reasons I would like to propse the following patch which I
> >> have tested by:
> >> 1) changing resolution only
> >> 2) changing format only
> >> 3) change both
> >>
> >> What do you and others think?
> >
> > I think that the format setting code should be completely rewritten, it's
> > pretty much unmaintainable as-is.
> >
> >> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >> index eaefdb5..e392b9d 100644
> >> --- a/drivers/media/i2c/ov5640.c
> >> +++ b/drivers/media/i2c/ov5640.c
> >> @@ -2020,6 +2020,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> >> struct ov5640_dev *sensor = to_ov5640_dev(sd);
> >> const struct ov5640_mode_info *new_mode;
> >> struct v4l2_mbus_framefmt *mbus_fmt = &format->format;
> >> + struct v4l2_mbus_framefmt *fmt;
> >> int ret;
> >>
> >> if (format->pad != 0)
> >> @@ -2037,22 +2038,19 @@ static int ov5640_set_fmt(struct v4l2_subdev
> >> *sd,
> >> if (ret)
> >> goto out;
> >>
> >> - if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> >> - struct v4l2_mbus_framefmt *fmt =
> >> - v4l2_subdev_get_try_format(sd, cfg, 0);
> >> + if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> >> + fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> >> + else
> >> + fmt = &sensor->fmt;
> >> -
Re: [PATCH v7] media: add imx319 camera sensor driver
Hi Tomasz, On Fri, Oct 12, 2018 at 05:06:56PM +0900, Tomasz Figa wrote: > On Fri, Oct 12, 2018 at 4:58 PM Sakari Ailus > wrote: > > > > Hi Tomasz, > > > > On Fri, Oct 12, 2018 at 01:51:10PM +0900, Tomasz Figa wrote: > > > Hi Sakari, > > > > > > On Wed, Sep 26, 2018 at 11:38 AM wrote: > > > > > > > > From: Bingbu Cao > > > > > > > > Add a v4l2 sub-device driver for the Sony imx319 image sensor. > > > > This is a camera sensor using the i2c bus for control and the > > > > csi-2 bus for data. > > > > > > > > This driver supports following features: > > > > - manual exposure and analog/digital gain control support > > > > - vblank/hblank control support > > > > - 4 test patterns control support > > > > - vflip/hflip control support (will impact the output bayer order) > > > > - support following resolutions: > > > > - 3264x2448, 3280x2464 @ 30fps > > > > - 1936x1096, 1920x1080 @ 60fps > > > > - 1640x1232, 1640x922, 1296x736, 1280x720 @ 120fps > > > > - support 4 bayer orders output (via change v/hflip) > > > > - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10 > > > > > > > > Cc: Tomasz Figa > > > > Cc: Sakari Ailus > > > > Signed-off-by: Bingbu Cao > > > > Signed-off-by: Tianshu Qiu > > > > > > > > --- > > > > > > > > This patch is based on sakari's media-tree git: > > > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.20-1 > > > > > > > > Changes from v5: > > > > - add some comments for gain calculation > > > > - use lock to protect the format > > > > - fix some style issues > > > > > > > > Changes from v4 to v5: > > > > - use single PLL for all internal clocks > > > > - change link frequency to 482.4MHz > > > > - adjust frame timing for 2x2 binning modes > > > >and enlarge frame readout time > > > > - get CSI-2 link frequencies and external clock > > > >from firmware > > > > > > If I remember correctly, that was suggested by you. Why do we need to > > > specify link frequency in firmware if it's fully configured by the > > > driver, with the only external dependency being the external clock? > > > > The driver that's now in upstream supports, for now, a very limited set of > > configurations from what the sensor supports. These are more or less > > tailored to the particular system where it is being used right now (output > > image size, external clock frequency, frame rates, link frequencies etc.). > > As a side note, they're tailored to exactly the system I mentioned, > with different link frequency hardcoded in the firmware, coming from > earlier stage of development. > > > If the same sensor is needed elsewhere (quite likely), the configuration > > needed elsewhere is very likely to be different from what you're using now. > > > > The link frequency in particular is important as using a different link > > frequency (which could be fine elsewhere) could cause EMI issues, e.g. > > rendering your GPS receiver inoperable during the time the camera sensor is > > streaming images. > > > > Should new configurations be added to this driver to support a different > > system, the link frequencies used by those configurations may be > > problematic to your system, and after a software update the driver could as > > well use those new frequencies. That's a big no-no. > > > > Okay, those are some valid points indeed, thanks for clarifying. > > > > > > > We're having problems with firmware listing the link frequency from v4 > > > and we can't easily change it anymore to report the new one. I feel > > > like this dependency on the firmware here is unnecessary, as long as > > > the external clock frequency matches. > > > > This is information you really need to know. > > > > A number of older drivers do not use the link frequency information from > > the firmware but that comes with a risk. Really, it's better to change the > > frequency now to something you can choose, rather than have it changed > > later on to something someone else chose for you. > > I guess it means that we have to carry a local downstream patch that > bypasses this check, since we cannot easily change the firmware > anymore. Is there a possibility update the firmware, or carry an SSDT overlay as part of the software? The options are laid out in Documentation/acpi/ssdt-overlays.txt . Do remember to pay attention to the revision field --- also in future Coreboot updates. > > An alternative would be to make the driver try to select a frequency > that matches what's in the firmware, but issue a warning and fall back > to a default one if a matching is not found. It might be actually > better than nothing for some early testing on new systems, since it > wouldn't require firmware changes. You don't need firmware changes per se; see above. Allowing that will very, very easily lead this being unaddressed during developement and changing later on inadvertly. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH] media: venus: add support for selection rectangles
Hi Malathi,
On 10/09/2018 10:53 AM, Malathi Gottam wrote:
> Handles target type crop by setting the new active rectangle
> to hardware. The new rectangle should be within YUV size.
>
> Signed-off-by: Malathi Gottam
> ---
> drivers/media/platform/qcom/venus/venc.c | 19 +--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c
> b/drivers/media/platform/qcom/venus/venc.c
> index 3f50cd0..754c19a 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -478,16 +478,31 @@ static int venc_g_fmt(struct file *file, void *fh,
> struct v4l2_format *f)
> venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
> {
> struct venus_inst *inst = to_inst(file);
> + int ret;
> + u32 buftype;
>
> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> return -EINVAL;
>
> switch (s->target) {
> case V4L2_SEL_TGT_CROP:
> - if (s->r.width != inst->out_width ||
> - s->r.height != inst->out_height ||
> + if (s->r.width > inst->out_width ||
> + s->r.height > inst->out_height ||
> s->r.top != 0 || s->r.left != 0)
> return -EINVAL;
> + if (s->r.width != inst->width ||
> + s->r.height != inst->height) {
> + buftype = HFI_BUFFER_OUTPUT;
> + ret = venus_helper_set_output_resolution(inst,
> + s->r.width,
> + s->r.height,
> + buftype);
I'm afraid that set_output_resolution cannot be called at any time. Do
you think we can set it after start_session?
--
regards,
Stan
Re: [PATCH 1/4] media: ov5640: fix resolution update
Hi Jacopo,
On 10/15/2018 05:24 PM, jacopo mondi wrote:
> Hi Hugues,
>
> On Mon, Oct 15, 2018 at 03:13:12PM +, Hugues FRUCHET wrote:
>> Hi Laurent, Jacopo, Sam,
>>
>> I'm also OK to change to a simpler alternative;
>> - drop the "restore" step
>
> Do you mean the restore step at the end of 'ov5640_restore_mode()' ?
yes
> I agree, I've been carrying this one [1] in my tree for some time now.
> I just didn't send it because of the too many issues in flight on this
> driver.
>
>> - send the whole init register sequence + mode changes + format changes
>> at streamon
>>
>
> This might be a first step in my opinion too, yes.
>
>> is this what you have in mind Laurent ?
>
> I know Laurent does not fully agree with me on this, but I would like
> to have Maxime's series on clock tree handling merged and tested on
> CSI-2 first before adding more patches to the pile of pending items on
> ov5640. I hope to have time to test them on CSI-2 this week before
> ELC-E.
>
> Steve, you're the driver maintainer do you have preferences here?
>
> Also, if this might be useful, I would like to help co-maintaining the
> driver (with possibily other people, possibly with the sensor wired in
> DVP mode), and help establishing priorities, as this driver needs some
> love, but one item at the time to avoid getting lost in too many
> pending changes as it happened recently :)
It's a good problem having too many contributions ;) This means that
there is some interest on it...
For DVP, me and Maxime have two different setup so we can cover the DVP
tests.
>
> Thanks
> j
>
> [1]
> media: ov5640: Do not restore format at power up
>
> Do not force restoring the last applied capture format during sensor
> power up
> as it will be re-set at s_stream time.
>
> Signed-off-by: Jacopo Mondi
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index b226946..17ee55b 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1737,12 +1737,10 @@ static int ov5640_restore_mode(struct ov5640_dev
> *sensor)
> if (ret)
> return ret;
>
> - /* now restore the last capture mode */
> - ret = ov5640_set_mode(sensor, &ov5640_mode_init_data);
> - if (ret < 0)
> - return ret;
> + sensor->pending_mode_change = true;
> + sensor->pending_fmt_change = true;
>
> - return ov5640_set_framefmt(sensor, &sensor->fmt);
> + return 0;
> }
>
> static void ov5640_power(struct ov5640_dev *sensor, bool enable)
>
>>
>> On 10/10/2018 02:41 PM, Laurent Pinchart wrote:
>>> Hi Jacopo,
>>>
>>> On Wednesday, 10 October 2018 13:58:04 EEST jacopo mondi wrote:
Hi Sam,
thanks for the patch, I see the same issue you reported, but I
think this patch can be improved.
(expanding the Cc list to all people involved in recent ov5640
developemts, not just for this patch, but for the whole series to look
at. Copying names from another series cover letter, hope it is
complete.)
On Mon, Oct 08, 2018 at 11:47:59PM -0700, Sam Bobrowicz wrote:
> set_fmt was not properly triggering a mode change when
> a new mode was set that happened to have the same format
> as the previous mode (for example, when only changing the
> frame dimensions). Fix this.
>
> Signed-off-by: Sam Bobrowicz
> ---
>
>drivers/media/i2c/ov5640.c | 8
>1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb5..5031aab 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2045,12 +2045,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>
> goto out;
>
> }
>
> - if (new_mode != sensor->current_mode) {
> +
> + if (new_mode != sensor->current_mode ||
> + mbus_fmt->code != sensor->fmt.code) {
> + sensor->fmt = *mbus_fmt;
>
> sensor->current_mode = new_mode;
> sensor->pending_mode_change = true;
>
> - }
> - if (mbus_fmt->code != sensor->fmt.code) {
> - sensor->fmt = *mbus_fmt;
>
> sensor->pending_fmt_change = true;
>
> }
How I did reproduce the issue:
# Set 1024x768 on ov5640 without changing the image format
# (default image size at startup is 640x480)
$ media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/1024x768 field:none]"
sensor->pending_mode_change = true; //verified this flag gets set
# Start streaming, after having configured the whole pipeline to work
# with 1024x768
$ yavta -c10 -n4 -f UYVY -s 1024x768 /dev/video4
Unable to start streaming: Broken pipe (32).
# Inspect which part of pipeline validation went wrong
# Turns out the
Re: [PATCH v4 12/12] ov5640: Enforce a mode change when changing the framerate
You're welcome ;) On 10/16/2018 09:10 AM, Maxime Ripard wrote: > Hi Hugues, > > On Mon, Oct 15, 2018 at 01:57:40PM +, Hugues FRUCHET wrote: >> This is already fixed in media tree: >> 0929983e49c81c1d413702cd9b83bb06c4a2555c media: ov5640: fix framerate update > > My bad then, I missed it, thanks! > Maxime >
[ragnatech:media-tree] BUILD SUCCESS 490d84f6d73c12f4204241cff8651eed60aae914
tree/branch: git://git.ragnatech.se/linux media-tree branch HEAD: 490d84f6d73c12f4204241cff8651eed60aae914 media: cec: forgot to cancel delayed work elapsed time: 1093m configs tested: 210 The following configs have been built successfully. More configs may be tested in the coming days. alpha defconfig pariscallnoconfig parisc b180_defconfig pariscc3000_defconfig parisc defconfig m68k apollo_defconfig mips lemote2f_defconfig x86_64 randconfig-a0-10161216 x86_64 acpi-redef x86_64 allyesdebian x86_64nfsroot shtitan_defconfig sh rsk7269_defconfig sh sh7785lcr_32bit_defconfig shallnoconfig i386 randconfig-c0-10161145 arm s3c6400_defconfig mips loongson1c_defconfig i386 randconfig-x019-201841 i386 randconfig-x016-201841 i386 randconfig-x018-201841 i386 randconfig-x014-201841 i386 randconfig-x015-201841 i386 randconfig-x011-201841 i386 randconfig-x012-201841 i386 randconfig-x017-201841 i386 randconfig-x010-201841 i386 randconfig-x013-201841 microblaze mmu_defconfig microblazenommu_defconfig i386 randconfig-n0-201841 i386 randconfig-n1-201841 i386 randconfig-n2-201841 i386 randconfig-n3-201841 x86_64 randconfig-x000-201841 x86_64 randconfig-x005-201841 x86_64 randconfig-x002-201841 x86_64 randconfig-x001-201841 x86_64 randconfig-x008-201841 x86_64 randconfig-x007-201841 x86_64 randconfig-x009-201841 x86_64 randconfig-x003-201841 x86_64 randconfig-x006-201841 x86_64 randconfig-x004-201841 ia64 allnoconfig ia64defconfig ia64 alldefconfig powerpc allnoconfig powerpc defconfig powerpc ppc64_defconfig s390default_defconfig i386 randconfig-i0-201841 i386 randconfig-i1-201841 i386 randconfig-i2-201841 i386 randconfig-i3-201841 arm allnoconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm sunxi_defconfig arm64 allnoconfig arm64 defconfig mipsnlm_xlr_defconfig sh se7343_defconfig x86_64 randconfig-g0-10161105 openriscor1ksim_defconfig um i386_defconfig um x86_64_defconfig c6xevmc6678_defconfig h8300h8300h-sim_defconfig nios2 10m50_defconfig xtensa common_defconfig xtensa iss_defconfig i386 allmodconfig i386 randconfig-x002-201841 i386 randconfig-x006-201841 i386 randconfig-x005-201841 i386 randconfig-x000-201841 i386 randconfig-x007-201841 i386 randconfig-x004-201841 i386 randconfig-x001-201841 i386 randconfig-x009-201841 i386 randconfig-x008-201841 i386 randconfig-x003-201841 arm allmodconfig arm arm5 arm arm67 arm imx_v6_v7_defconfig arm ixp4xx_defconfig armmvebu_v7_defconfig arm omap2plus_defconfig armsa1100 arm samsung armsh arm tegra_defconfig arm64alldefconfig arm64allmodconfig x86_64 randconfig-s0-10161132 x86_64 randconfig-s1-10161132 x86_64
[PATCH] cec: report Vendor ID after initialization
The CEC specification requires that the Vendor ID (if any) is reported after
a logical address was claimed.
This was never done, so add support for this.
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 31d1f4ab915e..ee4decc1cd64 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -1405,6 +1405,13 @@ static int cec_config_thread_func(void *arg)
las->log_addr[i],
cec_phys_addr_exp(adap->phys_addr));
cec_transmit_msg_fh(adap, &msg, NULL, false);
+
+ /* Report Vendor ID */
+ if (adap->log_addrs.vendor_id != CEC_VENDOR_ID_NONE) {
+ cec_msg_device_vendor_id(&msg,
+adap->log_addrs.vendor_id);
+ cec_transmit_msg_fh(adap, &msg, NULL, false);
+ }
}
adap->kthread_config = NULL;
complete(&adap->config_completion);
Re: [PATCH 2/2] media: docs-rst: Document memory-to-memory video encoder interface
On Tue, Aug 7, 2018 at 3:54 PM Tomasz Figa wrote: > > Hi Hans, > > On Wed, Jul 25, 2018 at 10:57 PM Hans Verkuil wrote: > > > > On 24/07/18 16:06, Tomasz Figa wrote: [snip] > > > +4. The client may set the raw source format on the ``OUTPUT`` queue via > > > + :c:func:`VIDIOC_S_FMT`. > > > + > > > + * **Required fields:** > > > + > > > + ``type`` > > > + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT`` > > > + > > > + ``pixelformat`` > > > + raw format of the source > > > + > > > + ``width``, ``height`` > > > + source resolution > > > + > > > + ``num_planes`` (for _MPLANE) > > > + set to number of planes for pixelformat > > > + > > > + ``sizeimage``, ``bytesperline`` > > > + follow standard semantics > > > + > > > + * **Return fields:** > > > + > > > + ``width``, ``height`` > > > + may be adjusted by driver to match alignment requirements, as > > > + required by the currently selected formats > > > + > > > + ``sizeimage``, ``bytesperline`` > > > + follow standard semantics > > > + > > > + * Setting the source resolution will reset visible resolution to the > > > + adjusted source resolution rounded up to the closest visible > > > + resolution supported by the driver. Similarly, coded resolution will > > > > coded -> the coded > > Ack. > > > > > > + be reset to source resolution rounded up to the closest coded > > > > reset -> set > > source -> the source > > Ack. > Actually, I'd prefer to keep it at "reset", so that it signifies the fact that the driver will actually override whatever was set by the application before. [snip] > > > + * The driver must expose following selection targets on ``OUTPUT``: > > > + > > > + ``V4L2_SEL_TGT_CROP_BOUNDS`` > > > + maximum crop bounds within the source buffer supported by the > > > + encoder > > > + > > > + ``V4L2_SEL_TGT_CROP_DEFAULT`` > > > + suggested cropping rectangle that covers the whole source > > > picture > > > + > > > + ``V4L2_SEL_TGT_CROP`` > > > + rectangle within the source buffer to be encoded into the > > > + ``CAPTURE`` stream; defaults to ``V4L2_SEL_TGT_CROP_DEFAULT`` > > > + > > > + ``V4L2_SEL_TGT_COMPOSE_BOUNDS`` > > > + maximum rectangle within the coded resolution, which the cropped > > > + source frame can be output into; always equal to (0, 0)x(width > > > of > > > + ``V4L2_SEL_TGT_CROP``, height of ``V4L2_SEL_TGT_CROP``), if the > > > + hardware does not support compose/scaling > > > + > > > + ``V4L2_SEL_TGT_COMPOSE_DEFAULT`` > > > + equal to ``V4L2_SEL_TGT_CROP`` > > > + > > > + ``V4L2_SEL_TGT_COMPOSE`` > > > + rectangle within the coded frame, which the cropped source frame > > > + is to be output into; defaults to > > > + ``V4L2_SEL_TGT_COMPOSE_DEFAULT``; read-only on hardware without > > > + additional compose/scaling capabilities; resulting stream will > > > + have this rectangle encoded as the visible rectangle in its > > > + metadata > > > + > > > + ``V4L2_SEL_TGT_COMPOSE_PADDED`` > > > + always equal to coded resolution of the stream, as selected by > > > the > > > + encoder based on source resolution and crop/compose rectangles > > > > Are there codec drivers that support composition? I can't remember seeing > > any. > > > > Hmm, I was convinced that MFC could scale and we just lacked support > in the driver, but I checked the documentation and it doesn't seem to > be able to do so. I guess we could drop the COMPOSE rectangles for > now, until we find any hardware that can do scaling or composing on > the fly. > On the other hand, having them defined already wouldn't complicate existing drivers too much either, because they would just handle all of them in the same switch case, i.e. case V4L2_SEL_TGT_COMPOSE_BOUNDS: case V4L2_SEL_TGT_COMPOSE_DEFAULT: case V4L2_SEL_TGT_COMPOSE: case V4L2_SEL_TGT_COMPOSE_PADDED: return visible_rectangle; That would need one change, though. We would define V4L2_SEL_TGT_COMPOSE_DEFAULT to be equal to (0, 0)x(width of V4L2_SEL_TGT_CROP - 1, height of ``V4L2_SEL_TGT_CROP - 1), which makes more sense than current definition, since it would bypass any compose/scaling by default. > > > + > > > + .. note:: > > > + > > > + The driver may adjust the crop/compose rectangles to the nearest > > > + supported ones to meet codec and hardware requirements. > > > + > > > +6. Allocate buffers for both ``OUTPUT`` and ``CAPTURE`` via > > > + :c:func:`VIDIOC_REQBUFS`. This may be performed in any order. > > > + > > > + * **Required fields:** > > > + > > > + ``count`` > > > + requested number of buffers to allocate; greater than zero > > > + > > > + ``type`` > > > + a ``V4L2_BUF_TYPE_*`` enum appropriate for ``OUTPUT`` or > > > + ``CAPTURE`` > > > + > > > + ``memory`` > > >
Re: [PATCH v4 12/12] ov5640: Enforce a mode change when changing the framerate
Hi Hugues, On Mon, Oct 15, 2018 at 01:57:40PM +, Hugues FRUCHET wrote: > This is already fixed in media tree: > 0929983e49c81c1d413702cd9b83bb06c4a2555c media: ov5640: fix framerate update My bad then, I missed it, thanks! Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
