cron job: media_tree daily build: WARNINGS
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: Tue Aug 14 05:00:13 CEST 2018 media-tree git hash:da2048b7348a0be92f706ac019e022139e29495e media_build git hash: baf45935ffad914f33faf751ad9f4d0dd276c021 v4l-utils git hash: 4e160e6dfc8705fbc6867c880f445e69fcedcada edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.102-i686: OK linux-3.2.102-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.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.115-i686: OK linux-3.18.115-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.18-i686: OK linux-4.18-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v2 2/7] [media] dt-bindings: tvp5150: Add input port connectors DT bindings
On Mon, Aug 13, 2018 at 11:25:03AM +0200, Marco Felsch wrote:
> The TVP5150/1 decoders support different video input sources to their
> AIP1A/B pins.
>
> Possible configurations are as follows:
> - Analog Composite signal connected to AIP1A.
> - Analog Composite signal connected to AIP1B.
> - Analog S-Video Y (luminance) and C (chrominance)
> signals connected to AIP1A and AIP1B respectively.
>
> This patch extends the device tree bindings documentation to describe
> how the input connectors for these devices should be defined in a DT.
>
> Signed-off-by: Marco Felsch
>
> ---
> Changelog:
> v2:
> - adapt port layout in accordance with
> https://www.spinics.net/lists/linux-media/msg138546.html with the
> svideo-connector deviation (use only one endpoint)
> ---
> .../devicetree/bindings/media/i2c/tvp5150.txt | 191 +-
> 1 file changed, 185 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> index 8c0fc1a26bf0..d647d671f14a 100644
> --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> @@ -12,11 +12,31 @@ Optional Properties:
> - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any.
> - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any.
>
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> +The device node must contain one 'port' child node per device physical input
> +and output port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
> +are numbered as follows
>
> -Required Endpoint Properties for parallel synchronization:
> + Name TypePort
> + --
> + AIP1A sink0
> + AIP1B sink1
> + Y-OUT src 2
> +
> +The device node must contain at least one sink port and the src port. Each
> input
> +port must be linked to an endpoint defined in
> +Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt.
> The
> +port/connector layout is as follows
> +
> +tvp-5150 port@0 (AIP1A)
> + endpoint@0 ---> Comp0-Con port
> + endpoint@1 ---> Svideo-Con port
> +tvp-5150 port@1 (AIP1B)
> + endpoint ---> Comp1-Con port
> +tvp-5150 port@2
> + endpoint (video bitstream output at YOUT[0-7] parallel bus)
> +
> +Required Endpoint Properties for parallel synchronization on output port:
>
> - hsync-active: active state of the HSYNC signal. Must be <1> (HIGH).
> - vsync-active: active state of the VSYNC signal. Must be <1> (HIGH).
> @@ -26,7 +46,140 @@ Required Endpoint Properties for parallel synchronization:
> If none of hsync-active, vsync-active and field-even-active is specified,
> the endpoint is assumed to use embedded BT.656 synchronization.
>
> -Example:
> +Examples:
Is it really necessary to enumerate every possibility? Just show the
most complicated case which is a superset of the rest.
> +
> +One Input:
> +
> +connector {
> + compatible = "composite-video-connector";
> + label = "Composite0";
> +
> + port {
> + composite0_to_tvp5150: endpoint {
> + remote-endpoint = <_to_composite0>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> + pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
> + reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> +
> + port@0 {
> + reg = <0>;
> +
> + tvp5150_to_composite0: endpoint {
> + remote-endpoint = <_to_tvp5150>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> +
> + tvp5150_1: endpoint {
> + remote-endpoint = <_ep>;
> + };
> + };
> + };
> +};
> +
> +Two Inputs:
> +
> +comp_connector_1 {
> + compatible = "composite-video-connector";
> + label = "Composite1";
> +
> + port {
> + composite1_to_tvp5150: endpoint {
> + remote-endpoint = <_to_composite1>;
> + };
> + };
> +};
> +
> +svid_connector {
> + compatible = "svideo-connector";
> + label = "S-Video";
> +
> + port {
> + svideo_to_tvp5150: endpoint {
> + remote-endpoint = <_to_svideo>;
> + };
> + };
> +};
> +
> + {
> + ...
> + tvp5150@5c {
> + compatible = "ti,tvp5150";
> + reg = <0x5c>;
> +
Re: [PATCH v1 2/2] v4l: Document Intel IPU3 meta data uAPI
Em Mon, 13 Aug 2018 15:42:34 +0200
Hans Verkuil escreveu:
> On 15/06/18 05:29, Yong Zhi wrote:
> > These meta formats are used on Intel IPU3 ImgU video queues
> > to carry 3A statistics and ISP pipeline parameters.
> >
> > V4L2_META_FMT_IPU3_3A
> > V4L2_META_FMT_IPU3_PARAMS
> >
> > Signed-off-by: Yong Zhi
> > Signed-off-by: Chao C Li
> > Signed-off-by: Rajmohan Mani
> > ---
> > Documentation/media/uapi/v4l/meta-formats.rst |1 +
> > .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst | 174 ++
> > include/uapi/linux/intel-ipu3.h| 2816
> >
> > 3 files changed, 2991 insertions(+)
> > create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > create mode 100644 include/uapi/linux/intel-ipu3.h
> >
> > diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> > b/Documentation/media/uapi/v4l/meta-formats.rst
> > index 0c4e1ec..b887fca 100644
> > --- a/Documentation/media/uapi/v4l/meta-formats.rst
> > +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> > @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface
> > only.
> > .. toctree::
> > :maxdepth: 1
> >
> > +pixfmt-meta-intel-ipu3
> > pixfmt-meta-uvc
> > pixfmt-meta-vsp1-hgo
> > pixfmt-meta-vsp1-hgt
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > new file mode 100644
> > index 000..5c050e6
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > @@ -0,0 +1,174 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _intel-ipu3:
> > +
> > +**
> > +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> > +**
> > +
> > +.. c:type:: ipu3_uapi_stats_3a
> > +
> > +3A statistics
> > +=
> > +
> > +For IPU3 ImgU, the 3A statistics accelerators collect different statistics
> > over
> > +an input bayer frame. Those statistics, defined in data struct
> > +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a
> > stat"
> > +video node, which are then passed to user space for statistics analysis
> > +using :c:type:`v4l2_meta_format` interface.
> > +
> > +The statistics collected are AWB (Auto-white balance) RGBS cells, AWB
> > filter
Just like you did with AWB, AF and AE, please place the full name in parenthesis
for RGBS and AWB.
> > +response, AF (Auto-focus) filter response, and AE (Auto-exposure)
> > histogram.
> > +
> > +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all
> > above.
> > +
> > +
> > +.. code-block:: c
> > +
> > +
> > + struct ipu3_uapi_stats_3a {
> > + IPU3_ALIGN struct ipu3_uapi_awb_raw_buffer awb_raw_buffer;
>
> IPU3_ALIGN? What's that?
>
> OK, after reading the header I see what it does, but I think you should
> drop it in the documentation since it doesn't help the reader.
Yeah, that IPU3_ALIGN is confusing.
Yet, instead of just dropping, I would replace it by a comment
to explain that the struct is 32-bytes aligned.
On a separate (but related) comment, you're declaring it as:
#define IPU3_ALIGN
__attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES)))
This is a gcc-specific dialect. Better to use, instead, __aligned(x)
which is defined as:
#define __aligned(x)__attribute__((aligned(x)))
>
> > + struct ipu3_uapi_ae_raw_buffer_aligned
> > + ae_raw_buffer[IPU3_UAPI_MAX_STRIPES];
> > + struct ipu3_uapi_af_raw_buffer af_raw_buffer;
> > + struct ipu3_uapi_awb_fr_raw_buffer awb_fr_raw_buffer;
> > + struct ipu3_uapi_4a_config stats_4a_config;
> > + __u32 ae_join_buffers;
> > + __u8 padding[28];
> > + struct ipu3_uapi_stats_3a_bubble_info_per_stripe
> > + stats_3a_bubble_per_stripe;
> > + struct ipu3_uapi_ff_status stats_3a_status;
> > + } __packed;
> > +
> > +
> > +.. c:type:: ipu3_uapi_params
> > +
> > +Pipeline parameters
> > +===
> > +
> > +IPU3 pipeline has a number of image processing stages, each of which takes
> > a
> > +set of parameters as input. The major stages of pipelines are shown here:
> > +
> > +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> > +
> > +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> > +
> > +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> > +
> > +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> > +
> > +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> > +
> > +Correction -> XNR3 -> TNR -> DDR
> > +
> > +The table below presents a description of the above algorithms.
> > +
> > +
> > ===
> > +NameDescription
> > +
> >
Re: [PATCH v2 1/6] dt-bindings: Document the Rockchip VPU bindings
On Thu, Aug 02, 2018 at 05:00:05PM -0300, Ezequiel Garcia wrote: > Add devicetree binding documentation for Rockchip Video Processing > Unit IP. > > Signed-off-by: Ezequiel Garcia > --- > .../bindings/media/rockchip-vpu.txt | 30 +++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/rockchip-vpu.txt Reviewed-by: Rob Herring
[BUG, RFC] media: Wrong module gets acquired
From: Petr Cvek When transferring a media sensor driver from the soc_camera I've found the controller module can get removed (which will cause a stack dump because the sensor driver depends on resources from the controller driver). When I've tried to remove the driver module of the sensor it said the resource was busy (without a reference name) though is should be possible to remove the sensor driver because it is at the end of the dependency list and not to remove the controller driver. I've dig into the called functions and I've found this in drivers/media/v4l2-core/v4l2-device.c: /* * The reason to acquire the module here is to avoid unloading * a module of sub-device which is registered to a media * device. To make it possible to unload modules for media * devices that also register sub-devices, do not * try_module_get() such sub-device owners. */ sd->owner_v4l2_dev = v4l2_dev->dev && v4l2_dev->dev->driver && sd->owner == v4l2_dev->dev->driver->owner; if (!sd->owner_v4l2_dev && !try_module_get(sd->owner)) return -ENODEV; It basicaly checks if subdevice (=sensor) is a same module as the media device (=controller) and if they are different it acquires the module. The acquired module is the one in sd->owner, which is the same module from which the function is called (-> sensor aquires itself). Is this functionality valid (should the subdevice really be unloadable)? When I've patched the module to aquire the controller instead the module, the removal worked as expected (sensor free to go, controller not). If this is really a bug (= there isn't a sensor which cannot be unloaded from a controller?) then I send a new patch with reworded commentary. Signed-off-by: Petr Cvek --- drivers/media/v4l2-core/v4l2-device.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c index 3940e55c72f1..1dec61cd560c 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -173,7 +173,8 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, sd->owner_v4l2_dev = v4l2_dev->dev && v4l2_dev->dev->driver && sd->owner == v4l2_dev->dev->driver->owner; - if (!sd->owner_v4l2_dev && !try_module_get(sd->owner)) + if (!sd->owner_v4l2_dev && + !try_module_get(v4l2_dev->dev->driver->owner)) return -ENODEV; sd->v4l2_dev = v4l2_dev; @@ -209,7 +210,7 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, #endif error_module: if (!sd->owner_v4l2_dev) - module_put(sd->owner); + module_put(v4l2_dev->dev->driver->owner); sd->v4l2_dev = NULL; return err; } @@ -318,6 +319,6 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd) #endif video_unregister_device(sd->devnode); if (!sd->owner_v4l2_dev) - module_put(sd->owner); + module_put(v4l2_dev->dev->driver->owner); } EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev); -- 2.18.0
Re: [PATCHv17 34/34] RFC: media-requests: add debugfs node
Em Sat, 4 Aug 2018 14:45:26 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Keep track of the number of requests and request objects of a media
> device. Helps to verify that all request-related memory is freed.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 41 +++
> drivers/media/media-devnode.c | 17 +++
> drivers/media/media-request.c | 5 +
> include/media/media-device.h | 11 ++
> include/media/media-devnode.h | 4
> include/media/media-request.h | 2 ++
> 6 files changed, 80 insertions(+)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 4b9a8de05562..28a891b53886 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -691,6 +691,23 @@ void media_device_unregister_entity(struct media_entity
> *entity)
> }
> EXPORT_SYMBOL_GPL(media_device_unregister_entity);
>
> +#ifdef CONFIG_DEBUG_FS
Patch itself looks good. Yet, perhaps we could request both
CONFIG_DEBUG_FS and CONFIG_VIDEO_ADV_DEBUG.
Also, instead of ifdef, please use IS_ENABLED for DEBUG_FS. That tends
to be safer long term.
With that:
Reviewed-by: Mauro Carvalho Chehab
> +/*
> + * Log the state of media requests.
> + * Very useful for debugging.
> + */
> +static int media_device_requests(struct seq_file *file, void *priv)
> +{
> + struct media_device *dev = dev_get_drvdata(file->private);
> +
> + seq_printf(file, "number of requests: %d\n",
> +atomic_read(>num_requests));
> + seq_printf(file, "number of request objects: %d\n",
> +atomic_read(>num_request_objects));
> + return 0;
> +}
> +#endif
> +
> /**
> * media_device_init() - initialize a media device
> * @mdev:The media device
> @@ -713,6 +730,9 @@ void media_device_init(struct media_device *mdev)
> mutex_init(>graph_mutex);
> ida_init(>entity_internal_idx);
>
> + atomic_set(>num_requests, 0);
> + atomic_set(>num_request_objects, 0);
> +
> dev_dbg(mdev->dev, "Media device initialized\n");
> }
> EXPORT_SYMBOL_GPL(media_device_init);
> @@ -764,6 +784,26 @@ int __must_check __media_device_register(struct
> media_device *mdev,
>
> dev_dbg(mdev->dev, "Media device registered\n");
>
> +#ifdef CONFIG_DEBUG_FS
> + if (!media_top_dir)
> + return 0;
> +
> + mdev->media_dir = debugfs_create_dir(dev_name(>dev),
> + media_top_dir);
> + if (IS_ERR_OR_NULL(mdev->media_dir)) {
> + dev_warn(mdev->dev, "Failed to create debugfs dir\n");
> + return 0;
> + }
> + mdev->requests_file = debugfs_create_devm_seqfile(>dev,
> + "requests", mdev->media_dir, media_device_requests);
> + if (IS_ERR_OR_NULL(mdev->requests_file)) {
> + dev_warn(mdev->dev, "Failed to create requests file\n");
> + debugfs_remove_recursive(mdev->media_dir);
> + mdev->media_dir = NULL;
> + return 0;
> + }
> +#endif
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(__media_device_register);
> @@ -841,6 +881,7 @@ void media_device_unregister(struct media_device *mdev)
>
> dev_dbg(mdev->dev, "Media device unregistered\n");
>
> + debugfs_remove_recursive(mdev->media_dir);
> device_remove_file(>devnode->dev, _attr_model);
> media_devnode_unregister(mdev->devnode);
> /* devnode free is handled in media_devnode_*() */
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index 6b87a721dc49..4358ed22f208 100644
> --- a/drivers/media/media-devnode.c
> +++ b/drivers/media/media-devnode.c
> @@ -53,6 +53,12 @@ static dev_t media_dev_t;
> static DEFINE_MUTEX(media_devnode_lock);
> static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
>
> +/*
> + * debugfs
> + */
> +struct dentry *media_top_dir;
> +
> +
> /* Called when the last user of the media device exits. */
> static void media_devnode_release(struct device *cd)
> {
> @@ -259,6 +265,8 @@ int __must_check media_devnode_register(struct
> media_device *mdev,
> goto cdev_add_error;
> }
>
> + dev_set_drvdata(>dev, mdev);
> +
> /* Part 4: Activate this minor. The char device can now be used. */
> set_bit(MEDIA_FLAG_REGISTERED, >flags);
>
> @@ -310,6 +318,14 @@ static int __init media_devnode_init(void)
> return ret;
> }
>
> +#ifdef CONFIG_DEBUG_FS
> + media_top_dir = debugfs_create_dir("media", NULL);
> + if (IS_ERR_OR_NULL(media_top_dir)) {
> + pr_warn("Failed to create debugfs media dir\n");
> + media_top_dir = NULL;
> + }
> +#endif
> +
> ret = bus_register(_bus_type);
> if (ret < 0) {
> unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
> @@ -322,6 +338,7 @@ static int __init media_devnode_init(void)
>
> static void __exit media_devnode_exit(void)
> {
> +
Re: [PATCHv17 33/34] vivid: add request support
Em Sat, 4 Aug 2018 14:45:25 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add support for requests to vivid.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/platform/vivid/vivid-core.c| 8
> drivers/media/platform/vivid/vivid-kthread-cap.c | 12
> drivers/media/platform/vivid/vivid-kthread-out.c | 12
> drivers/media/platform/vivid/vivid-sdr-cap.c | 16
> drivers/media/platform/vivid/vivid-vbi-cap.c | 10 ++
> drivers/media/platform/vivid/vivid-vbi-out.c | 10 ++
> drivers/media/platform/vivid/vivid-vid-cap.c | 10 ++
> drivers/media/platform/vivid/vivid-vid-out.c | 10 ++
> 8 files changed, 88 insertions(+)
>
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c
> index 1c448529be04..3f6f5cbe1b60 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -627,6 +627,13 @@ static void vivid_dev_release(struct v4l2_device
> *v4l2_dev)
> kfree(dev);
> }
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +static const struct media_device_ops vivid_media_ops = {
> + .req_validate = vb2_request_validate,
> + .req_queue = vb2_request_queue,
> +};
> +#endif
> +
> static int vivid_create_instance(struct platform_device *pdev, int inst)
> {
> static const struct v4l2_dv_timings def_dv_timings =
> @@ -664,6 +671,7 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
> strlcpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model));
> dev->mdev.dev = >dev;
> media_device_init(>mdev);
> + dev->mdev.ops = _media_ops;
> #endif
>
> /* register v4l2_device */
> diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c
> b/drivers/media/platform/vivid/vivid-kthread-cap.c
> index f06003bb8e42..eebfff2126be 100644
> --- a/drivers/media/platform/vivid/vivid-kthread-cap.c
> +++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
> @@ -703,6 +703,8 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev
> *dev, int dropped_bufs)
> goto update_mv;
>
> if (vid_cap_buf) {
> + v4l2_ctrl_request_setup(vid_cap_buf->vb.vb2_buf.req_obj.req,
> + >ctrl_hdl_vid_cap);
> /* Fill buffer */
> vivid_fillbuff(dev, vid_cap_buf);
> dprintk(dev, 1, "filled buffer %d\n",
> @@ -713,6 +715,8 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev
> *dev, int dropped_bufs)
> dev->fb_cap.fmt.pixelformat == dev->fmt_cap->fourcc)
> vivid_overlay(dev, vid_cap_buf);
>
> + v4l2_ctrl_request_complete(vid_cap_buf->vb.vb2_buf.req_obj.req,
> +>ctrl_hdl_vid_cap);
> vb2_buffer_done(_cap_buf->vb.vb2_buf, dev->dqbuf_error ?
> VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> dprintk(dev, 2, "vid_cap buffer %d done\n",
> @@ -720,10 +724,14 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev
> *dev, int dropped_bufs)
> }
>
> if (vbi_cap_buf) {
> + v4l2_ctrl_request_setup(vbi_cap_buf->vb.vb2_buf.req_obj.req,
> + >ctrl_hdl_vbi_cap);
> if (dev->stream_sliced_vbi_cap)
> vivid_sliced_vbi_cap_process(dev, vbi_cap_buf);
> else
> vivid_raw_vbi_cap_process(dev, vbi_cap_buf);
> + v4l2_ctrl_request_complete(vbi_cap_buf->vb.vb2_buf.req_obj.req,
> +>ctrl_hdl_vbi_cap);
> vb2_buffer_done(_cap_buf->vb.vb2_buf, dev->dqbuf_error ?
> VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> dprintk(dev, 2, "vbi_cap %d done\n",
> @@ -891,6 +899,8 @@ void vivid_stop_generating_vid_cap(struct vivid_dev *dev,
> bool *pstreaming)
> buf = list_entry(dev->vid_cap_active.next,
>struct vivid_buffer, list);
> list_del(>list);
> + v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
> +>ctrl_hdl_vid_cap);
> vb2_buffer_done(>vb.vb2_buf, VB2_BUF_STATE_ERROR);
> dprintk(dev, 2, "vid_cap buffer %d done\n",
> buf->vb.vb2_buf.index);
> @@ -904,6 +914,8 @@ void vivid_stop_generating_vid_cap(struct vivid_dev *dev,
> bool *pstreaming)
> buf = list_entry(dev->vbi_cap_active.next,
>struct vivid_buffer, list);
> list_del(>list);
> + v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
> +
Re: [PATCH 01/14] staging: media: tegra-vde: Support BSEV clock and reset
On Monday, 13 August 2018 17:50:14 MSK Thierry Reding wrote: > From: Thierry Reding > > The BSEV clock has a separate gate bit and can not be assumed to be > always enabled. Add explicit handling for the BSEV clock and reset. > > This fixes an issue on Tegra124 where the BSEV clock is not enabled > by default and therefore accessing the BSEV registers will hang the > CPU if the BSEV clock is not enabled and the reset not deasserted. > > Signed-off-by: Thierry Reding > --- Are you sure that BSEV clock is really needed for T20/30? I've tried already to disable the clock explicitly and everything kept working, though I'll try again. The device-tree changes should be reflected in the binding documentation.
Re: [PATCHv17 32/34] vivid: add mc
Em Sat, 4 Aug 2018 14:45:24 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add support for the media_device to vivid. This is a prerequisite
> for request support.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/platform/vivid/vivid-core.c | 61 +++
> drivers/media/platform/vivid/vivid-core.h | 8 +++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c
> index 31db363602e5..1c448529be04 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -657,6 +657,15 @@ static int vivid_create_instance(struct platform_device
> *pdev, int inst)
>
> dev->inst = inst;
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->v4l2_dev.mdev = >mdev;
> +
> + /* Initialize media device */
> + strlcpy(dev->mdev.model, VIVID_MODULE_NAME, sizeof(dev->mdev.model));
> + dev->mdev.dev = >dev;
> + media_device_init(>mdev);
> +#endif
> +
> /* register v4l2_device */
> snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name),
> "%s-%03d", VIVID_MODULE_NAME, inst);
> @@ -1174,6 +1183,13 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> vfd->lock = >mutex;
> video_set_drvdata(vfd, dev);
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->vid_cap_pad.flags = MEDIA_PAD_FL_SINK;
> + ret = media_entity_pads_init(>entity, 1,
> >vid_cap_pad);
> + if (ret)
> + goto unreg_dev;
> +#endif
> +
> #ifdef CONFIG_VIDEO_VIVID_CEC
> if (in_type_counter[HDMI]) {
> struct cec_adapter *adap;
> @@ -1226,6 +1242,13 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> vfd->lock = >mutex;
> video_set_drvdata(vfd, dev);
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->vid_out_pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(>entity, 1,
> >vid_out_pad);
> + if (ret)
> + goto unreg_dev;
> +#endif
> +
> #ifdef CONFIG_VIDEO_VIVID_CEC
> for (i = 0; i < dev->num_outputs; i++) {
> struct cec_adapter *adap;
> @@ -1275,6 +1298,13 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> vfd->tvnorms = tvnorms_cap;
> video_set_drvdata(vfd, dev);
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->vbi_cap_pad.flags = MEDIA_PAD_FL_SINK;
> + ret = media_entity_pads_init(>entity, 1,
> >vbi_cap_pad);
> + if (ret)
> + goto unreg_dev;
> +#endif
> +
> ret = video_register_device(vfd, VFL_TYPE_VBI,
> vbi_cap_nr[inst]);
> if (ret < 0)
> goto unreg_dev;
> @@ -1300,6 +1330,13 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> vfd->tvnorms = tvnorms_out;
> video_set_drvdata(vfd, dev);
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->vbi_out_pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_pads_init(>entity, 1,
> >vbi_out_pad);
> + if (ret)
> + goto unreg_dev;
> +#endif
> +
> ret = video_register_device(vfd, VFL_TYPE_VBI,
> vbi_out_nr[inst]);
> if (ret < 0)
> goto unreg_dev;
> @@ -1323,6 +1360,13 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> vfd->lock = >mutex;
> video_set_drvdata(vfd, dev);
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + dev->sdr_cap_pad.flags = MEDIA_PAD_FL_SINK;
> + ret = media_entity_pads_init(>entity, 1,
> >sdr_cap_pad);
> + if (ret)
> + goto unreg_dev;
> +#endif
> +
> ret = video_register_device(vfd, VFL_TYPE_SDR,
> sdr_cap_nr[inst]);
> if (ret < 0)
> goto unreg_dev;
> @@ -1369,12 +1413,25 @@ static int vivid_create_instance(struct
> platform_device *pdev, int inst)
> video_device_node_name(vfd));
> }
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + /* Register the media device */
> + ret = media_device_register(>mdev);
> + if (ret) {
> + dev_err(dev->mdev.dev,
> + "media device register failed (err=%d)\n", ret);
> + goto unreg_dev;
> + }
> +#endif
> +
> /* Now that everything is fine, let's add it to device list */
> vivid_devs[inst] = dev;
>
> return 0;
>
> unreg_dev:
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + media_device_unregister(>mdev);
> +#endif
> video_unregister_device(>radio_tx_dev);
> video_unregister_device(>radio_rx_dev);
>
Re: [PATCHv17 31/34] vim2m: support requests
Em Sat, 4 Aug 2018 14:45:23 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add support for requests to vim2m.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/platform/vim2m.c | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> index 6f87ef025ff1..3b8df2c9d24a 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -379,8 +379,18 @@ static void device_run(void *priv)
> src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> + /* Apply request controls if needed */
> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
> + >hdl);
> +
> device_process(ctx, src_buf, dst_buf);
>
> + /* Complete request controls if needed */
> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
> + >hdl);
> +
> /* Run delayed work, which simulates a hardware irq */
> schedule_delayed_work(>work_run, msecs_to_jiffies(ctx->transtime));
> }
> @@ -808,12 +818,21 @@ static void vim2m_stop_streaming(struct vb2_queue *q)
> vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> if (vbuf == NULL)
> return;
> + v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
> +>hdl);
> spin_lock_irqsave(>dev->irqlock, flags);
> v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
> spin_unlock_irqrestore(>dev->irqlock, flags);
> }
> }
>
> +static void vim2m_buf_request_complete(struct vb2_buffer *vb)
> +{
> + struct vim2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> + v4l2_ctrl_request_complete(vb->req_obj.req, >hdl);
> +}
> +
> static const struct vb2_ops vim2m_qops = {
> .queue_setup = vim2m_queue_setup,
> .buf_prepare = vim2m_buf_prepare,
> @@ -822,6 +841,7 @@ static const struct vb2_ops vim2m_qops = {
> .stop_streaming = vim2m_stop_streaming,
> .wait_prepare= vb2_ops_wait_prepare,
> .wait_finish = vb2_ops_wait_finish,
> + .buf_request_complete = vim2m_buf_request_complete,
> };
>
> static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue
> *dst_vq)
> @@ -988,6 +1008,11 @@ static const struct v4l2_m2m_ops m2m_ops = {
> .job_abort = job_abort,
> };
>
> +static const struct media_device_ops m2m_media_ops = {
> + .req_validate = vb2_request_validate,
> + .req_queue = vb2_m2m_request_queue,
> +};
> +
> static int vim2m_probe(struct platform_device *pdev)
> {
> struct vim2m_dev *dev;
> @@ -1036,6 +1061,7 @@ static int vim2m_probe(struct platform_device *pdev)
> dev->mdev.dev = >dev;
> strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
> media_device_init(>mdev);
> + dev->mdev.ops = _media_ops;
> dev->v4l2_dev.mdev = >mdev;
>
> ret = v4l2_m2m_register_media_controller(dev->m2m_dev,
Thanks,
Mauro
Re: [PATCHv17 30/34] vim2m: use workqueue
Em Sat, 4 Aug 2018 14:45:22 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> v4l2_ctrl uses mutexes, so we can't setup a ctrl_handler in
> interrupt context. Switch to a workqueue instead and drop the timer.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
Shouldn't this come earlier at the series (before adding request API
support to m2m) in order to avoid regressions?
> ---
> drivers/media/platform/vim2m.c | 25 ++---
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> index 462099a141e4..6f87ef025ff1 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -3,7 +3,8 @@
> *
> * This is a virtual device driver for testing mem-to-mem videobuf framework.
> * It simulates a device that uses memory buffers for both source and
> - * destination, processes the data and issues an "irq" (simulated by a
> timer).
> + * destination, processes the data and issues an "irq" (simulated by a
> delayed
> + * workqueue).
> * The device is capable of multi-instance, multi-buffer-per-transaction
> * operation (via the mem2mem framework).
> *
> @@ -19,7 +20,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
>
> @@ -148,7 +148,7 @@ struct vim2m_dev {
> struct mutexdev_mutex;
> spinlock_t irqlock;
>
> - struct timer_list timer;
> + struct delayed_work work_run;
>
> struct v4l2_m2m_dev *m2m_dev;
> };
> @@ -336,12 +336,6 @@ static int device_process(struct vim2m_ctx *ctx,
> return 0;
> }
>
> -static void schedule_irq(struct vim2m_dev *dev, int msec_timeout)
> -{
> - dprintk(dev, "Scheduling a simulated irq\n");
> - mod_timer(>timer, jiffies + msecs_to_jiffies(msec_timeout));
> -}
> -
> /*
> * mem2mem callbacks
> */
> @@ -387,13 +381,14 @@ static void device_run(void *priv)
>
> device_process(ctx, src_buf, dst_buf);
>
> - /* Run a timer, which simulates a hardware irq */
> - schedule_irq(dev, ctx->transtime);
> + /* Run delayed work, which simulates a hardware irq */
> + schedule_delayed_work(>work_run, msecs_to_jiffies(ctx->transtime));
> }
>
> -static void device_isr(struct timer_list *t)
> +static void device_work(struct work_struct *w)
> {
> - struct vim2m_dev *vim2m_dev = from_timer(vim2m_dev, t, timer);
> + struct vim2m_dev *vim2m_dev =
> + container_of(w, struct vim2m_dev, work_run.work);
> struct vim2m_ctx *curr_ctx;
> struct vb2_v4l2_buffer *src_vb, *dst_vb;
> unsigned long flags;
> @@ -805,6 +800,7 @@ static void vim2m_stop_streaming(struct vb2_queue *q)
> struct vb2_v4l2_buffer *vbuf;
> unsigned long flags;
>
> + flush_scheduled_work();
> for (;;) {
> if (V4L2_TYPE_IS_OUTPUT(q->type))
> vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> @@ -1015,6 +1011,7 @@ static int vim2m_probe(struct platform_device *pdev)
> vfd = >vfd;
> vfd->lock = >dev_mutex;
> vfd->v4l2_dev = >v4l2_dev;
> + INIT_DELAYED_WORK(>work_run, device_work);
>
> ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> if (ret) {
> @@ -1026,7 +1023,6 @@ static int vim2m_probe(struct platform_device *pdev)
> v4l2_info(>v4l2_dev,
> "Device registered as /dev/video%d\n", vfd->num);
>
> - timer_setup(>timer, device_isr, 0);
> platform_set_drvdata(pdev, dev);
>
> dev->m2m_dev = v4l2_m2m_init(_ops);
> @@ -1083,7 +1079,6 @@ static int vim2m_remove(struct platform_device *pdev)
> media_device_cleanup(>mdev);
> #endif
> v4l2_m2m_release(dev->m2m_dev);
> - del_timer_sync(>timer);
> video_unregister_device(>vfd);
> v4l2_device_unregister(>v4l2_dev);
>
Thanks,
Mauro
Re: [PATCHv17 29/34] v4l2-mem2mem: add vb2_m2m_request_queue
Em Sat, 4 Aug 2018 14:45:21 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> For mem2mem devices we have to make sure that v4l2_m2m_try_schedule()
> is called whenever a request is queued.
>
> We do that by creating a vb2_m2m_request_queue() helper that should
> be used instead of the 'normal' vb2_request_queue() helper. The m2m
> helper function will call v4l2_m2m_try_schedule() as needed.
>
> In addition we also avoid calling v4l2_m2m_try_schedule() when preparing
> or queueing a buffer for a request since that is no longer needed.
> Instead this helper function will do that when the request is actually
> queued.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
But please notice below that there's another change on patch 1/34 due
to this patchset.
> ---
> drivers/media/v4l2-core/v4l2-mem2mem.c | 59 ++
> include/media/v4l2-mem2mem.h | 4 ++
> 2 files changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index b09494174eb4..56a16cfef6f8 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -369,7 +369,7 @@ static void v4l2_m2m_cancel_job(struct v4l2_m2m_ctx
> *m2m_ctx)
> spin_unlock_irqrestore(_dev->job_spinlock, flags);
> if (m2m_dev->m2m_ops->job_abort)
> m2m_dev->m2m_ops->job_abort(m2m_ctx->priv);
> - dprintk("m2m_ctx %p running, will wait to complete", m2m_ctx);
> + dprintk("m2m_ctx %p running, will wait to complete\n", m2m_ctx);
> wait_event(m2m_ctx->finished,
> !(m2m_ctx->job_flags & TRANS_RUNNING));
> } else if (m2m_ctx->job_flags & TRANS_QUEUED) {
> @@ -460,8 +460,14 @@ int v4l2_m2m_qbuf(struct file *file, struct v4l2_m2m_ctx
> *m2m_ctx,
> int ret;
>
> vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
> + if (!V4L2_TYPE_IS_OUTPUT(vq->type) &&
> + (buf->flags & V4L2_BUF_FLAG_REQUEST_FD)) {
> + dprintk("%s: requests cannot be used with capture buffers\n",
> + __func__);
I had to double-check the documentation at patch 01/34. While on one
part is says the same, saying that -EPERM is the error code, on
another part, it says:
+Note that there is typically no need to use the Request API for
CAPTURE buffers
+since there are no per-frame settings to report there.
"typically" means that the normal usecase is to now allow, but gives
it open to implementations doing it.
Please fix that paragraph on patch 01/34 to reflect that no CAPTURE
buffers should be used with request API for m2m, in order to
reflect the code's implementation.
> + return -EPERM;
> + }
> ret = vb2_qbuf(vq, vdev->v4l2_dev->mdev, buf);
> - if (!ret)
> + if (!ret && !(buf->flags & V4L2_BUF_FLAG_IN_REQUEST))
> v4l2_m2m_try_schedule(m2m_ctx);
>
> return ret;
> @@ -483,14 +489,9 @@ int v4l2_m2m_prepare_buf(struct file *file, struct
> v4l2_m2m_ctx *m2m_ctx,
> {
> struct video_device *vdev = video_devdata(file);
> struct vb2_queue *vq;
> - int ret;
>
> vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
> - ret = vb2_prepare_buf(vq, vdev->v4l2_dev->mdev, buf);
> - if (!ret)
> - v4l2_m2m_try_schedule(m2m_ctx);
> -
> - return ret;
> + return vb2_prepare_buf(vq, vdev->v4l2_dev->mdev, buf);
> }
> EXPORT_SYMBOL_GPL(v4l2_m2m_prepare_buf);
>
> @@ -934,6 +935,48 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx,
> }
> EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
>
> +void vb2_m2m_request_queue(struct media_request *req)
> +{
> + struct media_request_object *obj, *obj_safe;
> + struct v4l2_m2m_ctx *m2m_ctx = NULL;
> +
> + /* Queue all non-buffer objects */
> + list_for_each_entry_safe(obj, obj_safe, >objects, list)
> + if (obj->ops->queue && !vb2_request_object_is_buffer(obj))
> + obj->ops->queue(obj);
> +
> + /* Queue all buffer objects */
> + list_for_each_entry_safe(obj, obj_safe, >objects, list) {
> + struct v4l2_m2m_ctx *m2m_ctx_obj;
> + struct vb2_buffer *vb;
> +
> + if (!obj->ops->queue || !vb2_request_object_is_buffer(obj))
> + continue;
> +
> + /* Sanity checks */
> + vb = container_of(obj, struct vb2_buffer, req_obj);
> + WARN_ON(!V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type));
> + m2m_ctx_obj = container_of(vb->vb2_queue,
> +struct v4l2_m2m_ctx,
> +out_q_ctx.q);
> + WARN_ON(m2m_ctx && m2m_ctx_obj != m2m_ctx);
> + m2m_ctx = m2m_ctx_obj;
> +
> + /*
> + * The buffer we queue here can in theory be immediately
> + * unbound, hence the use of
Re: [PATCHv17 28/34] videobuf2-v4l2: refuse qbuf if queue uses requests or vv.
Em Sat, 4 Aug 2018 14:45:20 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Check if the vb2 queue uses requests, and if so refuse to
> add buffers that are not part of a request. Also check for
> the reverse: a vb2 queue did not use requests, and an attempt
> was made to queue a buffer to a request.
>
> We might relax this in the future, but for now just return
> -EPERM in that case.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 88d8f60c742b..1b2351986230 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -378,8 +378,16 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q,
> struct media_device *md
> return ret;
> }
>
> - if (!(b->flags & V4L2_BUF_FLAG_REQUEST_FD))
> + if (!(b->flags & V4L2_BUF_FLAG_REQUEST_FD)) {
> + if (q->uses_requests) {
> + dprintk(1, "%s: queue uses requests\n", opname);
> + return -EPERM;
> + }
> return 0;
> + } else if (q->uses_qbuf) {
> + dprintk(1, "%s: queue does not use requests\n", opname);
> + return -EPERM;
> + }
>
> /*
>* For proper locking when queueing a request you need to be able
Thanks,
Mauro
Re: [PATCHv17 27/34] videobuf2-core: add uses_requests/qbuf flags
Em Sat, 4 Aug 2018 14:45:19 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Set the first time a buffer from a request is queued to vb2
> (uses_requests) or directly queued (uses_qbuf).
> Cleared when the queue is canceled.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 13 +
> include/media/videobuf2-core.h | 8
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index f8af7add35ab..5d7946ec80d8 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1491,9 +1491,17 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int
> index, void *pb,
>
> vb = q->bufs[index];
>
> + if ((req && q->uses_qbuf) ||
> + (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
> + q->uses_requests)) {
> + dprintk(1, "queue in wrong mode (qbuf vs requests)\n");
> + return -EPERM;
> + }
> +
> if (req) {
> int ret;
>
> + q->uses_requests = 1;
> if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> dprintk(1, "buffer %d not in dequeued state\n",
> vb->index);
> @@ -1523,6 +1531,9 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int
> index, void *pb,
> return 0;
> }
>
> + if (vb->state != VB2_BUF_STATE_IN_REQUEST)
> + q->uses_qbuf = 1;
> +
> switch (vb->state) {
> case VB2_BUF_STATE_DEQUEUED:
> case VB2_BUF_STATE_IN_REQUEST:
> @@ -1825,6 +1836,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> q->start_streaming_called = 0;
> q->queued_count = 0;
> q->error = 0;
> + q->uses_requests = 0;
> + q->uses_qbuf = 0;
>
> /*
>* Remove all buffers from videobuf's list...
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index cad712403d14..daffdf259fce 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -468,6 +468,12 @@ struct vb2_buf_ops {
> * @quirk_poll_must_check_waiting_for_buffers: Return %EPOLLERR at poll when
> QBUF
> * has not been called. This is a vb1 idiom that has been
> adopted
> * also by vb2.
> + * @uses_qbuf: qbuf was used directly for this queue. Set to 1 the
> first
> + * time this is called. Set to 0 when the queue is canceled.
> + * If this is 1, then you cannot queue buffers from a request.
> + * @uses_requests: requests are used for this queue. Set to 1 the first time
> + * a request is queued. Set to 0 when the queue is canceled.
> + * If this is 1, then you cannot queue buffers directly.
> * @lock:pointer to a mutex that protects the vb2_queue. The
> * driver can set this to a mutex to let the v4l2 core serialize
> * the queuing ioctls. If the driver wants to handle locking
> @@ -535,6 +541,8 @@ struct vb2_queue {
> unsignedfileio_write_immediately:1;
> unsignedallow_zero_bytesused:1;
> unsigned quirk_poll_must_check_waiting_for_buffers:1;
> + unsigneduses_qbuf:1;
> + unsigneduses_requests:1;
>
> struct mutex*lock;
> void*owner;
Thanks,
Mauro
Re: [PATCHv17 26/34] videobuf2-v4l2: add vb2_request_queue/validate helpers
Em Sat, 4 Aug 2018 14:45:18 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The generic vb2_request_validate helper function checks if
> there are buffers in the request and if so, prepares (validates)
> all objects in the request.
>
> The generic vb2_request_queue helper function queues all buffer
> objects in the validated request.
>
> Signed-off-by: Hans Verkuil
> ---
> .../media/common/videobuf2/videobuf2-v4l2.c | 44 +++
> include/media/videobuf2-v4l2.h| 4 ++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 9c652afa62ab..88d8f60c742b 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -1100,6 +1100,50 @@ void vb2_ops_wait_finish(struct vb2_queue *vq)
> }
> EXPORT_SYMBOL_GPL(vb2_ops_wait_finish);
>
> +int vb2_request_validate(struct media_request *req)
> +{
> + struct media_request_object *obj;
> + int ret = 0;
> +
> + if (!vb2_request_has_buffers(req))
> + return -ENOENT;
This holds the spinlock...
> +
> + list_for_each_entry(obj, >objects, list) {
> + if (!obj->ops->prepare)
> + continue;
> +
> + ret = obj->ops->prepare(obj);
> + if (ret)
> + break;
> + }
> +
Shouldn't this logic hold it too?
> + if (ret) {
> + list_for_each_entry_continue_reverse(obj, >objects, list)
> + if (obj->ops->unprepare)
> + obj->ops->unprepare(obj);
> + return ret;
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_request_validate);
> +
> +void vb2_request_queue(struct media_request *req)
> +{
> + struct media_request_object *obj, *obj_safe;
> +
> + /* Queue all non-buffer objects */
> + list_for_each_entry_safe(obj, obj_safe, >objects, list)
> + if (obj->ops->queue && !vb2_request_object_is_buffer(obj))
> + obj->ops->queue(obj);
> +
> + /* Queue all buffer objects */
> + list_for_each_entry_safe(obj, obj_safe, >objects, list) {
> + if (obj->ops->queue && vb2_request_object_is_buffer(obj))
> + obj->ops->queue(obj);
> + }
> +}
> +EXPORT_SYMBOL_GPL(vb2_request_queue);
> +
> MODULE_DESCRIPTION("Driver helper framework for Video for Linux 2");
> MODULE_AUTHOR("Pawel Osciak , Marek Szyprowski");
> MODULE_LICENSE("GPL");
> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> index 91a2b3e1a642..727855463838 100644
> --- a/include/media/videobuf2-v4l2.h
> +++ b/include/media/videobuf2-v4l2.h
> @@ -303,4 +303,8 @@ void vb2_ops_wait_prepare(struct vb2_queue *vq);
> */
> void vb2_ops_wait_finish(struct vb2_queue *vq);
>
> +struct media_request;
> +int vb2_request_validate(struct media_request *req);
> +void vb2_request_queue(struct media_request *req);
> +
> #endif /* _MEDIA_VIDEOBUF2_V4L2_H */
Thanks,
Mauro
[PATCH 14/14] ARM: tegra: Enable SMMU for VDE on Tegra124
From: Thierry Reding
The video decode engine can use the SMMU to use buffers that are not
physically contiguous in memory. This allows better memory usage for
video decoding, since fragmentation may cause contiguous allocations
to fail.
Signed-off-by: Thierry Reding
---
arch/arm/boot/dts/tegra124.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 8fdca4723205..0713e0ed5fef 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -321,6 +321,8 @@
resets = <_car 61>,
<_car 63>;
reset-names = "vde", "bsev";
+
+ iommus = < TEGRA_SWGROUP_VDE>;
};
apbdma: dma@6002 {
--
2.17.0
Re: [PATCHv17 25/34] videobuf2-core: add request helper functions
Em Sat, 4 Aug 2018 14:45:17 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add a new helper function to tell if a request object is a buffer.
>
> Add a new helper function that returns true if a media_request
> contains at least one buffer.
>
> Signed-off-by: Hans Verkuil
> ---
> .../media/common/videobuf2/videobuf2-core.c | 24 +++
> include/media/videobuf2-core.h| 15
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 3e6db7d30989..f8af7add35ab 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1362,6 +1362,30 @@ static const struct media_request_object_ops
> vb2_core_req_ops = {
> .release = vb2_req_release,
> };
>
> +bool vb2_request_object_is_buffer(struct media_request_object *obj)
> +{
> + return obj->ops == _core_req_ops;
> +}
> +EXPORT_SYMBOL_GPL(vb2_request_object_is_buffer);
> +
> +bool vb2_request_has_buffers(struct media_request *req)
> +{
> + struct media_request_object *obj;
> + unsigned long flags;
> + bool has_buffers = false;
> +
> + spin_lock_irqsave(>lock, flags);
> + list_for_each_entry(obj, >objects, list) {
> + if (vb2_request_object_is_buffer(obj)) {
> + has_buffers = true;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(>lock, flags);
> + return has_buffers;
> +}
> +EXPORT_SYMBOL_GPL(vb2_request_has_buffers);
> +
> int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
> {
> struct vb2_buffer *vb;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 8a8d7732d182..cad712403d14 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -1168,4 +1168,19 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct
> vb2_buffer *vb);
> */
> int vb2_verify_memory_type(struct vb2_queue *q,
> enum vb2_memory memory, unsigned int type);
> +
> +/**
> + * vb2_request_object_is_buffer() - return true if the object is a buffer
> + *
> + * @obj: the request object.
It should be mentioned that it should be called with req->lock locked.
With such change:
Reviewed-by: Mauro Carvalho Chehab
> + */
> +bool vb2_request_object_is_buffer(struct media_request_object *obj);
> +
> +/**
> + * vb2_request_has_buffers() - return true if the request contains buffers
> + *
> + * @req: the request.
> + */
> +bool vb2_request_has_buffers(struct media_request *req);
> +
> #endif /* _MEDIA_VIDEOBUF2_CORE_H */
Thanks,
Mauro
[PATCH 12/14] ARM: tegra: Add BSEV clock and reset for VDE on Tegra20
From: Thierry Reding
Signed-off-by: Thierry Reding
---
arch/arm/boot/dts/tegra20.dtsi | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 15b73bd377f0..abb5738a0705 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -287,9 +287,13 @@
, /* BSE-V
interrupt */
; /* SXE interrupt
*/
interrupt-names = "sync-token", "bsev", "sxe";
- clocks = <_car TEGRA20_CLK_VDE>;
- reset-names = "vde", "mc";
- resets = <_car 61>, < TEGRA20_MC_RESET_VDE>;
+ clocks = <_car TEGRA20_CLK_VDE>,
+<_car TEGRA20_CLK_BSEV>;
+ clock-names = "vde", "bsev";
+ resets = <_car 61>,
+<_car 63>,
+< TEGRA20_MC_RESET_VDE>;
+ reset-names = "vde", "bsev", "mc";
};
apbmisc@7800 {
--
2.17.0
[PATCH 13/14] ARM: tegra: Add BSEV clock and reset for VDE on Tegra30
From: Thierry Reding
Signed-off-by: Thierry Reding
---
arch/arm/boot/dts/tegra30.dtsi | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index a6781f653310..492917d61bab 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -408,9 +408,13 @@
, /* BSE-V
interrupt */
; /* SXE interrupt
*/
interrupt-names = "sync-token", "bsev", "sxe";
- clocks = <_car TEGRA30_CLK_VDE>;
- reset-names = "vde", "mc";
- resets = <_car 61>, < TEGRA30_MC_RESET_VDE>;
+ clocks = <_car TEGRA30_CLK_VDE>,
+<_car TEGRA30_CLK_BSEV>;
+ clock-names = "vde", "bsev";
+ resets = <_car 61>,
+<_car 63>,
+< TEGRA30_MC_RESET_VDE>;
+ reset-names = "vde", "bsev", "mc";
};
apbmisc@7800 {
--
2.17.0
[PATCH 08/14] staging: media: tegra-vde: Track struct device *
From: Thierry Reding
The pointer to the struct device is frequently used, so store it in
struct tegra_vde. Also, pass around a pointer to a struct tegra_vde
instead of struct device in some cases to prepare for subsequent
patches referencing additional data from that structure.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 63 -
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 41cf86dc5dbd..2496a03fd158 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -71,6 +71,7 @@ struct tegra_vde_soc {
};
struct tegra_vde {
+ struct device *dev;
const struct tegra_vde_soc *soc;
void __iomem *sxe;
void __iomem *bsev;
@@ -644,7 +645,7 @@ static void tegra_vde_detach_and_put_dmabuf(struct
dma_buf_attachment *a,
dma_buf_put(dmabuf);
}
-static int tegra_vde_attach_dmabuf(struct device *dev,
+static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
int fd,
unsigned long offset,
size_t min_size,
@@ -662,38 +663,40 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
dmabuf = dma_buf_get(fd);
if (IS_ERR(dmabuf)) {
- dev_err(dev, "Invalid dmabuf FD: %d\n", fd);
+ dev_err(vde->dev, "Invalid dmabuf FD: %d\n", fd);
return PTR_ERR(dmabuf);
}
if (dmabuf->size & (align_size - 1)) {
- dev_err(dev, "Unaligned dmabuf 0x%zX, should be aligned to
0x%zX\n",
+ dev_err(vde->dev,
+ "Unaligned dmabuf 0x%zX, should be aligned to 0x%zX\n",
dmabuf->size, align_size);
return -EINVAL;
}
if ((u64)offset + min_size > dmabuf->size) {
- dev_err(dev, "Too small dmabuf size %zu @0x%lX, should be at
least %zu\n",
+ dev_err(vde->dev,
+ "Too small dmabuf size %zu @0x%lX, should be at least
%zu\n",
dmabuf->size, offset, min_size);
return -EINVAL;
}
- attachment = dma_buf_attach(dmabuf, dev);
+ attachment = dma_buf_attach(dmabuf, vde->dev);
if (IS_ERR(attachment)) {
- dev_err(dev, "Failed to attach dmabuf\n");
+ dev_err(vde->dev, "Failed to attach dmabuf\n");
err = PTR_ERR(attachment);
goto err_put;
}
sgt = dma_buf_map_attachment(attachment, dma_dir);
if (IS_ERR(sgt)) {
- dev_err(dev, "Failed to get dmabufs sg_table\n");
+ dev_err(vde->dev, "Failed to get dmabufs sg_table\n");
err = PTR_ERR(sgt);
goto err_detach;
}
if (sgt->nents != 1) {
- dev_err(dev, "Sparse DMA region is unsupported\n");
+ dev_err(vde->dev, "Sparse DMA region is unsupported\n");
err = -EINVAL;
goto err_unmap;
}
@@ -717,7 +720,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
return err;
}
-static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
+static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
struct video_frame *frame,
struct tegra_vde_h264_frame *src,
enum dma_data_direction dma_dir,
@@ -726,7 +729,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device
*dev,
{
int err;
- err = tegra_vde_attach_dmabuf(dev, src->y_fd,
+ err = tegra_vde_attach_dmabuf(vde, src->y_fd,
src->y_offset, lsize, SZ_256,
>y_dmabuf_attachment,
>y_addr,
@@ -735,7 +738,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device
*dev,
if (err)
return err;
- err = tegra_vde_attach_dmabuf(dev, src->cb_fd,
+ err = tegra_vde_attach_dmabuf(vde, src->cb_fd,
src->cb_offset, csize, SZ_256,
>cb_dmabuf_attachment,
>cb_addr,
@@ -744,7 +747,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device
*dev,
if (err)
goto err_release_y;
- err = tegra_vde_attach_dmabuf(dev, src->cr_fd,
+ err = tegra_vde_attach_dmabuf(vde, src->cr_fd,
src->cr_offset, csize, SZ_256,
>cr_dmabuf_attachment,
>cr_addr,
@@ -758,7 +761,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device
*dev,
[PATCH 10/14] staging: media: tegra-vde: Keep VDE in reset when unused
From: Thierry Reding There is no point in keeping the VDE module out of reset when it is not in use. Reset it on runtime suspend. Signed-off-by: Thierry Reding --- drivers/staging/media/tegra-vde/tegra-vde.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c index 3bc0bfcfe34e..4b3c6ab3c77e 100644 --- a/drivers/staging/media/tegra-vde/tegra-vde.c +++ b/drivers/staging/media/tegra-vde/tegra-vde.c @@ -1226,6 +1226,7 @@ static int tegra_vde_runtime_suspend(struct device *dev) } reset_control_assert(vde->rst_bsev); + reset_control_assert(vde->rst); usleep_range(2000, 4000); -- 2.17.0
[PATCH 11/14] ARM: tegra: Enable VDE on Tegra124
From: Thierry Reding
Signed-off-by: Thierry Reding
---
arch/arm/boot/dts/tegra124.dtsi | 40 +
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index b113e47b2b2a..8fdca4723205 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -83,6 +83,19 @@
};
};
+ iram@4000 {
+ compatible = "mmio-sram";
+ reg = <0x0 0x4000 0x0 0x4>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x0 0x4000 0x4>;
+
+ vde_pool: pool@400 {
+ reg = <0x400 0x3fc00>;
+ pool;
+ };
+ };
+
host1x@5000 {
compatible = "nvidia,tegra124-host1x", "simple-bus";
reg = <0x0 0x5000 0x0 0x00034000>;
@@ -283,6 +296,33 @@
*/
};
+ vde@6003 {
+ compatible = "nvidia,tegra124-vde", "nvidia,tegra30-vde",
+"nvidia,tegra20-vde";
+ reg = <0x0 0x6003 0x0 0x1000 /* Syntax Engine */
+ 0x0 0x60031000 0x0 0x1000 /* Video Bitstream Engine */
+ 0x0 0x60032000 0x0 0x0100 /* Macroblock Engine */
+ 0x0 0x60032200 0x0 0x0100 /* Post-processing Engine */
+ 0x0 0x60032400 0x0 0x0100 /* Motion Compensation
Engine */
+ 0x0 0x60032600 0x0 0x0100 /* Transform Engine */
+ 0x0 0x60032800 0x0 0x0100 /* Pixel prediction block */
+ 0x0 0x60032a00 0x0 0x0100 /* Video DMA */
+ 0x0 0x60033800 0x0 0x0400>; /* Video frame controls */
+ reg-names = "sxe", "bsev", "mbe", "ppe", "mce",
+ "tfe", "ppb", "vdma", "frameid";
+ iram = <_pool>; /* IRAM region */
+ interrupts = , /* Sync token
interrupt */
+, /* BSE-V
interrupt */
+; /* SXE interrupt
*/
+ interrupt-names = "sync-token", "bsev", "sxe";
+ clocks = <_car TEGRA124_CLK_VDE>,
+<_car TEGRA124_CLK_BSEV>;
+ clock-names = "vde", "bsev";
+ resets = <_car 61>,
+<_car 63>;
+ reset-names = "vde", "bsev";
+ };
+
apbdma: dma@6002 {
compatible = "nvidia,tegra124-apbdma", "nvidia,tegra148-apbdma";
reg = <0x0 0x6002 0x0 0x1400>;
--
2.17.0
[PATCH 09/14] staging: media: tegra-vde: Add IOMMU support
From: Thierry Reding
Implement support for using an IOMMU to map physically discontiguous
buffers into contiguous I/O virtual mappings that the VDE can use. This
allows importing arbitrary DMA-BUFs for use by the VDE.
While at it, make sure that the device is detached from any DMA/IOMMU
mapping that it might have automatically been attached to at boot. If
using the IOMMU API explicitly, detaching from any existing mapping is
required to avoid double mapping of buffers.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 171 +---
1 file changed, 153 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 2496a03fd158..3bc0bfcfe34e 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -13,7 +13,9 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
@@ -22,6 +24,10 @@
#include
#include
+#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
+#include
+#endif
+
#include
#include
@@ -61,6 +67,11 @@ struct video_frame {
u32 frame_num;
u32 flags;
u64 modifier;
+
+ struct iova *y_iova;
+ struct iova *cb_iova;
+ struct iova *cr_iova;
+ struct iova *aux_iova;
};
struct tegra_vde_soc {
@@ -93,6 +104,12 @@ struct tegra_vde {
struct clk *clk_bsev;
dma_addr_t iram_lists_addr;
u32 *iram;
+
+ struct iommu_domain *domain;
+ struct iommu_group *group;
+ struct iova_domain iova;
+ unsigned long limit;
+ unsigned int shift;
};
static void tegra_vde_set_bits(struct tegra_vde *vde,
@@ -634,12 +651,22 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
VDE_WR(0x2000 | (macroblocks_nb - 1), vde->sxe + 0x00);
}
-static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
+static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
+ struct dma_buf_attachment *a,
struct sg_table *sgt,
+ struct iova *iova,
enum dma_data_direction dma_dir)
{
struct dma_buf *dmabuf = a->dmabuf;
+ if (vde->domain) {
+ unsigned long size = iova_size(iova) << vde->shift;
+ dma_addr_t addr = iova_dma_addr(>iova, iova);
+
+ iommu_unmap(vde->domain, addr, size);
+ __free_iova(>iova, iova);
+ }
+
dma_buf_unmap_attachment(a, sgt, dma_dir);
dma_buf_detach(dmabuf, a);
dma_buf_put(dmabuf);
@@ -651,14 +678,16 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
size_t min_size,
size_t align_size,
struct dma_buf_attachment **a,
- dma_addr_t *addr,
+ dma_addr_t *addrp,
struct sg_table **s,
- size_t *size,
+ struct iova **iovap,
+ size_t *sizep,
enum dma_data_direction dma_dir)
{
struct dma_buf_attachment *attachment;
struct dma_buf *dmabuf;
struct sg_table *sgt;
+ size_t size;
int err;
dmabuf = dma_buf_get(fd);
@@ -695,18 +724,47 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
goto err_detach;
}
- if (sgt->nents != 1) {
+ if (sgt->nents > 1 && !vde->domain) {
dev_err(vde->dev, "Sparse DMA region is unsupported\n");
err = -EINVAL;
goto err_unmap;
}
- *addr = sg_dma_address(sgt->sgl) + offset;
+ if (vde->domain) {
+ int prot = IOMMU_READ | IOMMU_WRITE;
+ struct iova *iova;
+ dma_addr_t addr;
+
+ size = (dmabuf->size - offset) >> vde->shift;
+
+ iova = alloc_iova(>iova, size, vde->limit - 1, true);
+ if (!iova) {
+ err = -ENOMEM;
+ goto err_unmap;
+ }
+
+ addr = iova_dma_addr(>iova, iova);
+
+ size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
+ prot);
+ if (!size) {
+ __free_iova(>iova, iova);
+ err = -ENXIO;
+ goto err_unmap;
+ }
+
+ *addrp = addr;
+ *iovap = iova;
+ } else {
+ *addrp = sg_dma_address(sgt->sgl) + offset;
+ size = dmabuf->size - offset;
+ }
+
*a = attachment;
*s = sgt;
- if (size)
-
[PATCH 01/14] staging: media: tegra-vde: Support BSEV clock and reset
From: Thierry Reding
The BSEV clock has a separate gate bit and can not be assumed to be
always enabled. Add explicit handling for the BSEV clock and reset.
This fixes an issue on Tegra124 where the BSEV clock is not enabled
by default and therefore accessing the BSEV registers will hang the
CPU if the BSEV clock is not enabled and the reset not deasserted.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 35 +++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 6f06061a40d9..9d8f833744db 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -74,9 +74,11 @@ struct tegra_vde {
struct miscdevice miscdev;
struct reset_control *rst;
struct reset_control *rst_mc;
+ struct reset_control *rst_bsev;
struct gen_pool *iram_pool;
struct completion decode_completion;
struct clk *clk;
+ struct clk *clk_bsev;
dma_addr_t iram_lists_addr;
u32 *iram;
};
@@ -979,6 +981,11 @@ static int tegra_vde_runtime_suspend(struct device *dev)
return err;
}
+ reset_control_assert(vde->rst_bsev);
+
+ usleep_range(2000, 4000);
+
+ clk_disable_unprepare(vde->clk_bsev);
clk_disable_unprepare(vde->clk);
return 0;
@@ -996,6 +1003,16 @@ static int tegra_vde_runtime_resume(struct device *dev)
return err;
}
+ err = clk_prepare_enable(vde->clk_bsev);
+ if (err < 0)
+ return err;
+
+ err = reset_control_deassert(vde->rst_bsev);
+ if (err < 0)
+ return err;
+
+ usleep_range(2000, 4000);
+
return 0;
}
@@ -1084,14 +1101,21 @@ static int tegra_vde_probe(struct platform_device *pdev)
if (IS_ERR(vde->frameid))
return PTR_ERR(vde->frameid);
- vde->clk = devm_clk_get(dev, NULL);
+ vde->clk = devm_clk_get(dev, "vde");
if (IS_ERR(vde->clk)) {
err = PTR_ERR(vde->clk);
dev_err(dev, "Could not get VDE clk %d\n", err);
return err;
}
- vde->rst = devm_reset_control_get(dev, NULL);
+ vde->clk_bsev = devm_clk_get(dev, "bsev");
+ if (IS_ERR(vde->clk_bsev)) {
+ err = PTR_ERR(vde->clk_bsev);
+ dev_err(dev, "failed to get BSEV clock: %d\n", err);
+ return err;
+ }
+
+ vde->rst = devm_reset_control_get(dev, "vde");
if (IS_ERR(vde->rst)) {
err = PTR_ERR(vde->rst);
dev_err(dev, "Could not get VDE reset %d\n", err);
@@ -1105,6 +1129,13 @@ static int tegra_vde_probe(struct platform_device *pdev)
return err;
}
+ vde->rst_bsev = devm_reset_control_get(dev, "bsev");
+ if (IS_ERR(vde->rst_bsev)) {
+ err = PTR_ERR(vde->rst_bsev);
+ dev_err(dev, "failed to get BSEV reset: %d\n", err);
+ return err;
+ }
+
irq = platform_get_irq_byname(pdev, "sync-token");
if (irq < 0)
return irq;
--
2.17.0
[PATCH 06/14] staging: media: tegra-vde: Print out invalid FD
From: Thierry Reding
Include the invalid file descriptor when reporting an error message to
help diagnosing why importing the buffer failed.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 0ce30c7ccb75..0adc603fa437 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -643,7 +643,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
dmabuf = dma_buf_get(fd);
if (IS_ERR(dmabuf)) {
- dev_err(dev, "Invalid dmabuf FD\n");
+ dev_err(dev, "Invalid dmabuf FD: %d\n", fd);
return PTR_ERR(dmabuf);
}
--
2.17.0
[PATCH 02/14] staging: media: tegra-vde: Support reference picture marking
From: Thierry Reding
Tegra114 and Tegra124 support reference picture marking, which will
cause BSEV to write picture marking data to SDRAM. Make sure there is
a valid destination address for that data to avoid error messages from
the memory controller.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 54 -
drivers/staging/media/tegra-vde/uapi.h | 3 ++
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 9d8f833744db..3027b11b11ae 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -60,7 +60,12 @@ struct video_frame {
u32 flags;
};
+struct tegra_vde_soc {
+ bool supports_ref_pic_marking;
+};
+
struct tegra_vde {
+ const struct tegra_vde_soc *soc;
void __iomem *sxe;
void __iomem *bsev;
void __iomem *mbe;
@@ -330,6 +335,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
struct video_frame *dpb_frames,
dma_addr_t bitstream_data_addr,
size_t bitstream_data_size,
+ dma_addr_t secure_addr,
unsigned int macroblocks_nb)
{
struct device *dev = vde->miscdev.parent;
@@ -454,6 +460,9 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
VDE_WR(bitstream_data_addr, vde->sxe + 0x6C);
+ if (vde->soc->supports_ref_pic_marking)
+ VDE_WR(secure_addr, vde->sxe + 0x7c);
+
value = 0x1005;
value |= ctx->pic_width_in_mbs << 11;
value |= ctx->pic_height_in_mbs << 3;
@@ -772,12 +781,15 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
*vde,
struct tegra_vde_h264_frame __user *frames_user;
struct video_frame *dpb_frames;
struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
- struct sg_table *bitstream_sgt;
+ struct dma_buf_attachment *secure_attachment = NULL;
+ struct sg_table *bitstream_sgt, *secure_sgt;
enum dma_data_direction dma_dir;
dma_addr_t bitstream_data_addr;
+ dma_addr_t secure_addr;
dma_addr_t bsev_ptr;
size_t lsize, csize;
size_t bitstream_data_size;
+ size_t secure_size;
unsigned int macroblocks_nb;
unsigned int read_bytes;
unsigned int cstride;
@@ -803,6 +815,18 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
*vde,
if (ret)
return ret;
+ if (vde->soc->supports_ref_pic_marking) {
+ ret = tegra_vde_attach_dmabuf(dev, ctx.secure_fd,
+ ctx.secure_offset, 0, SZ_256,
+ _attachment,
+ _addr,
+ _sgt,
+ _size,
+ DMA_TO_DEVICE);
+ if (ret)
+ goto release_bitstream_dmabuf;
+ }
+
dpb_frames = kcalloc(ctx.dpb_frames_nb, sizeof(*dpb_frames),
GFP_KERNEL);
if (!dpb_frames) {
@@ -876,6 +900,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
*vde,
ret = tegra_vde_setup_hw_context(vde, , dpb_frames,
bitstream_data_addr,
bitstream_data_size,
+secure_addr,
macroblocks_nb);
if (ret)
goto put_runtime_pm;
@@ -929,6 +954,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
*vde,
kfree(dpb_frames);
release_bitstream_dmabuf:
+ if (secure_attachment)
+ tegra_vde_detach_and_put_dmabuf(secure_attachment, secure_sgt,
+ DMA_TO_DEVICE);
+
tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
bitstream_sgt, DMA_TO_DEVICE);
@@ -1029,6 +1058,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, vde);
+ vde->soc = of_device_get_match_data(>dev);
+
regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sxe");
if (!regs)
return -ENODEV;
@@ -1258,8 +1289,27 @@ static const struct dev_pm_ops tegra_vde_pm_ops = {
tegra_vde_pm_resume)
};
+static const struct tegra_vde_soc tegra20_vde_soc = {
+ .supports_ref_pic_marking = false,
+};
+
+static const struct tegra_vde_soc tegra30_vde_soc = {
+ .supports_ref_pic_marking = false,
+};
+
+static const struct tegra_vde_soc tegra114_vde_soc = {
+
[PATCH 03/14] staging: media: tegra-vde: Prepare for interlacing support
From: Thierry Reding
The number of frames doubles when decoding interlaced content and the
structures describing the frames double in size. Take that into account
to prepare for interlacing support.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 73 -
1 file changed, 58 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 3027b11b11ae..1a40f6dff7c8 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -61,7 +61,9 @@ struct video_frame {
};
struct tegra_vde_soc {
+ unsigned int num_ref_pics;
bool supports_ref_pic_marking;
+ bool supports_interlacing;
};
struct tegra_vde {
@@ -205,8 +207,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
+ u32 value = y_addr >> 8;
- VDE_WR(y_addr >> 8, vde->frameid + 0x000 + frameid * 4);
+ if (vde->soc->supports_interlacing)
+ value |= BIT(31);
+
+ VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
VDE_WR(cr_addr >> 8, vde->frameid + 0x180 + frameid * 4);
VDE_WR(value1, vde->frameid + 0x080 + frameid * 4);
@@ -229,20 +235,23 @@ static void tegra_setup_frameidx(struct tegra_vde *vde,
}
static void tegra_vde_setup_iram_entry(struct tegra_vde *vde,
+ unsigned int num_ref_pics,
unsigned int table,
unsigned int row,
u32 value1, u32 value2)
{
+ unsigned int entries = num_ref_pics * 2;
u32 *iram_tables = vde->iram;
dev_dbg(vde->miscdev.parent, "IRAM table %u: row %u: 0x%08X 0x%08X\n",
table, row, value1, value2);
- iram_tables[0x20 * table + row * 2] = value1;
- iram_tables[0x20 * table + row * 2 + 1] = value2;
+ iram_tables[entries * table + row * 2] = value1;
+ iram_tables[entries * table + row * 2 + 1] = value2;
}
static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
+ unsigned int num_ref_pics,
struct video_frame *dpb_frames,
unsigned int ref_frames_nb,
unsigned int with_earlier_poc_nb)
@@ -251,13 +260,17 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
u32 value, aux_addr;
int with_later_poc_nb;
unsigned int i, k;
+ size_t size;
+
+ size = num_ref_pics * 4 * 8;
+ memset(vde->iram, 0, size);
dev_dbg(vde->miscdev.parent, "DPB: Frame 0: frame_num = %d\n",
dpb_frames[0].frame_num);
dev_dbg(vde->miscdev.parent, "REF L0:\n");
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < num_ref_pics; i++) {
if (i < ref_frames_nb) {
frame = _frames[i + 1];
@@ -277,10 +290,14 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
value = 0;
}
- tegra_vde_setup_iram_entry(vde, 0, i, value, aux_addr);
- tegra_vde_setup_iram_entry(vde, 1, i, value, aux_addr);
- tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
- tegra_vde_setup_iram_entry(vde, 3, i, value, aux_addr);
+ tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
+ aux_addr);
+ tegra_vde_setup_iram_entry(vde, num_ref_pics, 1, i, value,
+ aux_addr);
+ tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+ aux_addr);
+ tegra_vde_setup_iram_entry(vde, num_ref_pics, 3, i, value,
+ aux_addr);
}
if (!(dpb_frames[0].flags & FLAG_B_FRAME))
@@ -309,7 +326,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
- tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
+ tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+ aux_addr);
}
for (k = 0; i < ref_frames_nb; i++, k++) {
@@ -326,7 +344,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
-
[PATCH 07/14] staging: media: tegra-vde: Add some clarifying comments
From: Thierry Reding Add some comments specifying what tables are being set up in VRAM. Signed-off-by: Thierry Reding --- drivers/staging/media/tegra-vde/tegra-vde.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c index 0adc603fa437..41cf86dc5dbd 100644 --- a/drivers/staging/media/tegra-vde/tegra-vde.c +++ b/drivers/staging/media/tegra-vde/tegra-vde.c @@ -271,6 +271,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde *vde, unsigned int i, k; size_t size; + /* clear H256RefPicList */ size = num_ref_pics * 4 * 8; memset(vde->iram, 0, size); @@ -453,6 +454,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde, VDE_WR(0x, vde->bsev + 0x98); VDE_WR(0x0060, vde->bsev + 0x9C); + /* clear H264MB2SliceGroupMap, assuming no FMO */ memset(vde->iram + 1024, 0, macroblocks_nb / 2); tegra_setup_frameidx(vde, dpb_frames, ctx->dpb_frames_nb, @@ -480,6 +482,8 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde, if (err) return err; + /* upload H264MB2SliceGroupMap */ + /* XXX don't hardcode map size? */ value = (0x20 << 26) | (0 << 25) | ((4096 >> 2) & 0x1fff); err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false); if (err) @@ -492,6 +496,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde, if (err) return err; + /* clear H264MBInfo XXX don't hardcode size */ value = (0x21 << 26) | ((240 & 0x1fff) << 12) | (0x54c & 0xfff); err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x840F054C, false); if (err) @@ -499,6 +504,16 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde, size = num_ref_pics * 4 * 8; + /* clear H264RefPicList */ + /* + value = (0x21 << 26) | (((size >> 2) & 0x1fff) << 12) | 0xE34; + + err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false); + if (err) + return err; + */ + + /* upload H264RefPicList */ value = (0x20 << 26) | (0x0 << 25) | ((size >> 2) & 0x1fff); err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false); if (err) @@ -584,7 +599,11 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde, tegra_vde_mbe_set_0xa_reg(vde, 0, 0x09FC); tegra_vde_mbe_set_0xa_reg(vde, 2, 0x61DEAD00); +#if 0 + tegra_vde_mbe_set_0xa_reg(vde, 4, dpb_frames[0].aux_addr); /* 0x62DEAD00 */ +#else tegra_vde_mbe_set_0xa_reg(vde, 4, 0x62DEAD00); +#endif tegra_vde_mbe_set_0xa_reg(vde, 6, 0x63DEAD00); tegra_vde_mbe_set_0xa_reg(vde, 8, dpb_frames[0].aux_addr); -- 2.17.0
[PATCH 05/14] staging: media: tegra-vde: Properly mark invalid entries
From: Thierry Reding
Entries in the reference picture list are marked as invalid by setting
the frame ID to 0x3f.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 275884e745df..0ce30c7ccb75 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -296,7 +296,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
(frame->flags & FLAG_B_FRAME));
} else {
aux_addr = 0x6ADEAD00;
- value = 0;
+ value = 0x3f;
}
tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
--
2.17.0
[PATCH 00/14] staging: media: tegra-vdea: Add Tegra124 support
From: Thierry Reding Hi, this set of patches perform a bit of cleanup and extend support to the VDE implementation found on Tegra114 and Tegra124. This requires adding handling for a clock and a reset for the BSEV block that is separate from the main VDE block. The new VDE revision also supports reference picture marking, which requires that the BSEV writes out some related data to a memory location. Since the supported tiling layouts have been changed in Tegra124, which supports only block-linear and no pitch- linear layouts, a new way is added to request a specific layout for the decoded frames. Both of the above changes require breaking the ABI to accomodate for the new data in the custom IOCTL. Finally this set also adds support for dealing with an IOMMU, which makes it more convenient to deal with imported buffers since they no longer need to be physically contiguous. Userspace changes for the updated ABI are available here: https://cgit.freedesktop.org/~tagr/libvdpau-tegra/commit/ Mauro, I'm sending the device tree changes as part of the series for completeness, but I expect to pick those up into the Tegra tree once this has been reviewed and you've applied the driver changes. Thanks, Thierry Thierry Reding (14): staging: media: tegra-vde: Support BSEV clock and reset staging: media: tegra-vde: Support reference picture marking staging: media: tegra-vde: Prepare for interlacing support staging: media: tegra-vde: Use DRM/KMS framebuffer modifiers staging: media: tegra-vde: Properly mark invalid entries staging: media: tegra-vde: Print out invalid FD staging: media: tegra-vde: Add some clarifying comments staging: media: tegra-vde: Track struct device * staging: media: tegra-vde: Add IOMMU support staging: media: tegra-vde: Keep VDE in reset when unused ARM: tegra: Enable VDE on Tegra124 ARM: tegra: Add BSEV clock and reset for VDE on Tegra20 ARM: tegra: Add BSEV clock and reset for VDE on Tegra30 ARM: tegra: Enable SMMU for VDE on Tegra124 arch/arm/boot/dts/tegra124.dtsi | 42 ++ arch/arm/boot/dts/tegra20.dtsi | 10 +- arch/arm/boot/dts/tegra30.dtsi | 10 +- drivers/staging/media/tegra-vde/tegra-vde.c | 528 +--- drivers/staging/media/tegra-vde/uapi.h | 6 +- 5 files changed, 511 insertions(+), 85 deletions(-) -- 2.17.0
[PATCH 04/14] staging: media: tegra-vde: Use DRM/KMS framebuffer modifiers
From: Thierry Reding
VDE on Tegra20 through Tegra114 supports reading and writing frames in
16x16 tiled layout. Similarily, the various block-linear layouts that
are supported by the GPU on Tegra124 can also be read from and written
to by the Tegra124 VDE.
Enable userspace to specify the desired layout using the existing DRM
framebuffer modifiers.
Signed-off-by: Thierry Reding
---
drivers/staging/media/tegra-vde/tegra-vde.c | 112 +---
drivers/staging/media/tegra-vde/uapi.h | 3 +-
2 files changed, 100 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 1a40f6dff7c8..275884e745df 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -24,6 +24,8 @@
#include
+#include
+
#include "uapi.h"
#define ICMDQUE_WR 0x00
@@ -58,12 +60,14 @@ struct video_frame {
dma_addr_t aux_addr;
u32 frame_num;
u32 flags;
+ u64 modifier;
};
struct tegra_vde_soc {
unsigned int num_ref_pics;
bool supports_ref_pic_marking;
bool supports_interlacing;
+ bool supports_block_linear;
};
struct tegra_vde {
@@ -202,6 +206,7 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
unsigned int frameid,
u32 mbs_width, u32 mbs_height)
{
+ u64 modifier = frame ? frame->modifier : DRM_FORMAT_MOD_LINEAR;
u32 y_addr = frame ? frame->y_addr : 0x6CDEAD00;
u32 cb_addr = frame ? frame->cb_addr : 0x6CDEAD00;
u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
@@ -209,8 +214,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
u32 value = y_addr >> 8;
- if (vde->soc->supports_interlacing)
+ if (!vde->soc->supports_interlacing) {
+ if (modifier == DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED)
+ value |= BIT(31);
+ } else {
value |= BIT(31);
+ }
VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
@@ -349,6 +358,37 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
}
}
+static int tegra_vde_get_block_height(u64 modifier, unsigned int *block_height)
+{
+ switch (modifier) {
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB:
+ *block_height = 0;
+ return 0;
+
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB:
+ *block_height = 1;
+ return 0;
+
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB:
+ *block_height = 2;
+ return 0;
+
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_EIGHT_GOB:
+ *block_height = 3;
+ return 0;
+
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_SIXTEEN_GOB:
+ *block_height = 4;
+ return 0;
+
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB:
+ *block_height = 5;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
struct tegra_vde_h264_decoder_ctx *ctx,
struct video_frame *dpb_frames,
@@ -383,7 +423,21 @@ static int tegra_vde_setup_hw_context(struct tegra_vde
*vde,
tegra_vde_set_bits(vde, 0x0005, vde->vdma + 0x04);
VDE_WR(0x, vde->vdma + 0x1C);
- VDE_WR(0x, vde->vdma + 0x00);
+
+ value = 0x;
+
+ if (vde->soc->supports_block_linear) {
+ unsigned int block_height;
+
+ err = tegra_vde_get_block_height(dpb_frames[0].modifier,
+_height);
+ if (err < 0)
+ return err;
+
+ value |= block_height << 10;
+ }
+
+ VDE_WR(value, vde->vdma + 0x00);
VDE_WR(0x0007, vde->vdma + 0x04);
VDE_WR(0x0007, vde->frameid + 0x200);
VDE_WR(0x0005, vde->tfe + 0x04);
@@ -730,11 +784,37 @@ static void tegra_vde_release_frame_dmabufs(struct
video_frame *frame,
static int tegra_vde_validate_frame(struct device *dev,
struct tegra_vde_h264_frame *frame)
{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+
if (frame->frame_num > 0x7F) {
dev_err(dev, "Bad frame_num %u\n", frame->frame_num);
return -EINVAL;
}
+ if (vde->soc->supports_block_linear) {
+ switch (frame->modifier) {
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB:
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB:
+ case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB:
+
Re: [PATCHv17 24/34] videobuf2-v4l2: integrate with media requests
Em Sat, 4 Aug 2018 14:45:16 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> This implements the V4L2 part of the request support. The main
> change is that vb2_qbuf and vb2_prepare_buf now have a new
> media_device pointer. This required changes to several drivers
> that did not use the vb2_ioctl_qbuf/prepare_buf helper functions.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> .../media/common/videobuf2/videobuf2-core.c | 13 +-
> .../media/common/videobuf2/videobuf2-v4l2.c | 112 --
> drivers/media/platform/omap3isp/ispvideo.c| 2 +-
> .../media/platform/s3c-camif/camif-capture.c | 4 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 4 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 4 +-
> .../media/platform/soc_camera/soc_camera.c| 4 +-
> drivers/media/usb/uvc/uvc_queue.c | 5 +-
> drivers/media/usb/uvc/uvc_v4l2.c | 3 +-
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> drivers/media/v4l2-core/v4l2-mem2mem.c| 6 +-
> .../staging/media/davinci_vpfe/vpfe_video.c | 3 +-
> drivers/staging/media/omap4iss/iss_video.c| 3 +-
> drivers/usb/gadget/function/uvc_queue.c | 2 +-
> include/media/videobuf2-v4l2.h| 14 ++-
> 15 files changed, 148 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 1efb9c8b359d..3e6db7d30989 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1338,6 +1338,14 @@ static void vb2_req_queue(struct media_request_object
> *obj)
> mutex_unlock(vb->vb2_queue->lock);
> }
>
> +static void vb2_req_unbind(struct media_request_object *obj)
> +{
> + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> +
> + if (vb->state == VB2_BUF_STATE_IN_REQUEST)
> + call_void_bufop(vb->vb2_queue, init_buffer, vb);
> +}
> +
> static void vb2_req_release(struct media_request_object *obj)
> {
> struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> @@ -1350,6 +1358,7 @@ static const struct media_request_object_ops
> vb2_core_req_ops = {
> .prepare = vb2_req_prepare,
> .unprepare = vb2_req_unprepare,
> .queue = vb2_req_queue,
> + .unbind = vb2_req_unbind,
> .release = vb2_req_release,
> };
>
> @@ -1481,8 +1490,10 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int
> index, void *pb,
>
> vb->state = VB2_BUF_STATE_IN_REQUEST;
> /* Fill buffer information for the userspace */
> - if (pb)
> + if (pb) {
> + call_void_bufop(q, copy_timestamp, vb, pb);
> call_void_bufop(q, fill_user_buffer, vb, pb);
> + }
>
> dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
> return 0;
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index ea9db4b3f59a..9c652afa62ab 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -25,6 +25,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -40,10 +41,12 @@ module_param(debug, int, 0644);
> pr_info("vb2-v4l2: %s: " fmt, __func__, ## arg); \
> } while (0)
>
> -/* Flags that are set by the vb2 core */
> +/* Flags that are set by us */
> #define V4L2_BUFFER_MASK_FLAGS (V4L2_BUF_FLAG_MAPPED |
> V4L2_BUF_FLAG_QUEUED | \
>V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \
>V4L2_BUF_FLAG_PREPARED | \
> + V4L2_BUF_FLAG_IN_REQUEST | \
> + V4L2_BUF_FLAG_REQUEST_FD | \
>V4L2_BUF_FLAG_TIMESTAMP_MASK)
> /* Output buffer flags that should be passed on to the driver */
> #define V4L2_BUFFER_OUT_FLAGS(V4L2_BUF_FLAG_PFRAME |
> V4L2_BUF_FLAG_BFRAME | \
> @@ -118,6 +121,16 @@ static int __verify_length(struct vb2_buffer *vb, const
> struct v4l2_buffer *b)
> return 0;
> }
>
> +/*
> + * __init_v4l2_vb2_buffer() - initialize the v4l2_vb2_buffer struct
> + */
> +static void __init_v4l2_vb2_buffer(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> + vbuf->request_fd = -1;
> +}
> +
> static void __copy_timestamp(struct vb2_buffer *vb, const void *pb)
> {
> const struct v4l2_buffer *b = pb;
> @@ -181,6 +194,7 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer
> *vb, struct v4l2_buffer *b
> return -EINVAL;
> }
> vbuf->sequence = 0;
> + vbuf->request_fd = -1;
>
> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> switch (b->memory) {
>
Re: [PATCH v1 4/5] [media] i2c: drop soc_camera code from ov9640 and switch to v4l2_async
Hi Petr,
On Sun, Aug 12, 2018 at 03:13:39PM +0200, Petr Cvek wrote:
> Dne 10.8.2018 v 09:51 jacopo mondi napsal(a):
> > Hi Petr,
> >
> > On Thu, Aug 09, 2018 at 03:39:48AM +0200, petrcve...@gmail.com wrote:
> >> From: Petr Cvek
> >>
> >> This patch removes the dependency on an obsoleted soc_camera from ov9640
> >> driver and changes the code to be a standalone v4l2 async subdevice.
> >> It also adds GPIO allocations for power and reset signals (as they are not
> >> handled by soc_camera now).
> >>
> >> The patch should make ov9640 again compatible with the pxa_camera driver.
> >
> > Are there board files using this driverin mainline ? (git grep says so)
> > Care to port them to use the new driver if necessary? You can have a
> > look at the SH4 Migo-R board, which recently underwent the same
> > process (arch/sh/boards/mach-migor/setup.c)
> >
>
> Yes there are Magician and Palm Zire72 which are directly using ov9640
> (and few others which are using pxa_camera with a different sensor). I'm
> working on HTC magician (pxa_camera is not a soc_camera subdev anymore,
> ov9640 still is).
>
> > I also suggest to adjust the build system in a single patch with this
> > changes, but that's not a big deal...
> >
>
> OK (at the end of the patchset I suppose?)
>
When I did the same soc_camera removal process on other drivers, I
adjusted the build system in the same patch that removed soc_camera
dependencies in the driver.
The process looked like:
01: copy the driver from drivers/media/i2c/soc_camera to
drivers/media/i2c/
02: Remove soc_camera dependencies from the driver and adjust
drivers/media/i2c/Kconfg drivers/media/i2c/Makefile accordingly
03: Port existing users of the soc_camera driver to use the new one
I guess patch ordering is not a big deal though ;)
Thanks
j
> >
> >> + ret = v4l2_clk_enable(priv->clk);
> >
> > Is this required by the pxa camera driver using v4l2_clk_ APIs?
> > Otherwise you should use the clk API directly.
> >
>
> Yes the clock is registered by pxa camera with v4l2_clk_register(). I
> will probably get to that in the future, but there is stuff (bugs, dead
> code from soc_camera removal, ...) with more priority in the driver for now.
>
>
> >> + mdelay(1);
> >> + gpiod_set_value(priv->gpio_reset, 0);
> >> + } else {
> >> + gpiod_set_value(priv->gpio_reset, 1);
> >> + mdelay(1);
> >> + v4l2_clk_disable(priv->clk);
> >> + mdelay(1);
> >> + gpiod_set_value(priv->gpio_power, 0);
> >> + }
> >> + return ret;
> >> }
> >>
> >> /* select nearest higher resolution for capture */
> >> @@ -631,14 +648,10 @@ static const struct v4l2_subdev_core_ops
> >> ov9640_core_ops = {
> >> static int ov9640_g_mbus_config(struct v4l2_subdev *sd,
> >>struct v4l2_mbus_config *cfg)
> >
> > g_mbus/s_mbus are deprecated. Unless the pxa camera driver wants them
> > all format negotiation should go through s_fmt/g_fmt pad operations
> >
>
> Yeah it does:
>
> ret = sensor_call(pcdev, video, g_mbus_config, );
>
>
> >> {
> >> - struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> >> -
> >>cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> >>V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> >>V4L2_MBUS_DATA_ACTIVE_HIGH;
> >>cfg->type = V4L2_MBUS_PARALLEL;
> >> - cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> >>
> >>return 0;
> >> }
> >> @@ -667,18 +680,27 @@ static int ov9640_probe(struct i2c_client *client,
> >>const struct i2c_device_id *did)
> >> {
> >>struct ov9640_priv *priv;
> >> - struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> >>int ret;
> >>
> >> - if (!ssdd) {
> >> - dev_err(>dev, "Missing platform_data for driver\n");
> >> - return -EINVAL;
> >> - }
> >> -
> >> - priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> >> + priv = devm_kzalloc(>dev, sizeof(*priv),
> >> + GFP_KERNEL);
> >>if (!priv)
> >>return -ENOMEM;
> >>
> >> + priv->gpio_power = devm_gpiod_get(>dev, "Camera power",
> >> +GPIOD_OUT_LOW);
> >> + if (IS_ERR_OR_NULL(priv->gpio_power)) {
> >> + ret = PTR_ERR(priv->gpio_power);
> >> + return ret;
> >> + }
> >> +
> >> + priv->gpio_reset = devm_gpiod_get(>dev, "Camera reset",
> >> +GPIOD_OUT_HIGH);
> >> + if (IS_ERR_OR_NULL(priv->gpio_reset)) {
> >> + ret = PTR_ERR(priv->gpio_reset);
> >> + return ret;
> >> + }
> >> +
> >>v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
> >>
> >>v4l2_ctrl_handler_init(>hdl, 2);
> >> @@ -692,17 +714,25 @@ static int ov9640_probe(struct i2c_client *client,
> >>
> >>priv->clk = v4l2_clk_get(>dev, "mclk");
> >>if (IS_ERR(priv->clk)) {
> >> -
Re: [PATCH v1 2/2] v4l: Document Intel IPU3 meta data uAPI
On 15/06/18 05:29, Yong Zhi wrote:
> These meta formats are used on Intel IPU3 ImgU video queues
> to carry 3A statistics and ISP pipeline parameters.
>
> V4L2_META_FMT_IPU3_3A
> V4L2_META_FMT_IPU3_PARAMS
>
> Signed-off-by: Yong Zhi
> Signed-off-by: Chao C Li
> Signed-off-by: Rajmohan Mani
> ---
> Documentation/media/uapi/v4l/meta-formats.rst |1 +
> .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst | 174 ++
> include/uapi/linux/intel-ipu3.h| 2816
>
> 3 files changed, 2991 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> create mode 100644 include/uapi/linux/intel-ipu3.h
>
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index 0c4e1ec..b887fca 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface
> only.
> .. toctree::
> :maxdepth: 1
>
> +pixfmt-meta-intel-ipu3
> pixfmt-meta-uvc
> pixfmt-meta-vsp1-hgo
> pixfmt-meta-vsp1-hgt
> diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> new file mode 100644
> index 000..5c050e6
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> @@ -0,0 +1,174 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _intel-ipu3:
> +
> +**
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +**
> +
> +.. c:type:: ipu3_uapi_stats_3a
> +
> +3A statistics
> +=
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics
> over
> +an input bayer frame. Those statistics, defined in data struct
> +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a
> stat"
> +video node, which are then passed to user space for statistics analysis
> +using :c:type:`v4l2_meta_format` interface.
> +
> +The statistics collected are AWB (Auto-white balance) RGBS cells, AWB filter
> +response, AF (Auto-focus) filter response, and AE (Auto-exposure) histogram.
> +
> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all
> above.
> +
> +
> +.. code-block:: c
> +
> +
> + struct ipu3_uapi_stats_3a {
> + IPU3_ALIGN struct ipu3_uapi_awb_raw_buffer awb_raw_buffer;
IPU3_ALIGN? What's that?
OK, after reading the header I see what it does, but I think you should
drop it in the documentation since it doesn't help the reader.
> + struct ipu3_uapi_ae_raw_buffer_aligned
> + ae_raw_buffer[IPU3_UAPI_MAX_STRIPES];
> + struct ipu3_uapi_af_raw_buffer af_raw_buffer;
> + struct ipu3_uapi_awb_fr_raw_buffer awb_fr_raw_buffer;
> + struct ipu3_uapi_4a_config stats_4a_config;
> + __u32 ae_join_buffers;
> + __u8 padding[28];
> + struct ipu3_uapi_stats_3a_bubble_info_per_stripe
> + stats_3a_bubble_per_stripe;
> + struct ipu3_uapi_ff_status stats_3a_status;
> + } __packed;
> +
> +
> +.. c:type:: ipu3_uapi_params
> +
> +Pipeline parameters
> +===
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes a
> +set of parameters as input. The major stages of pipelines are shown here:
> +
> +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> +
> +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> +
> +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> +
> +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> +
> +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> +
> +Correction -> XNR3 -> TNR -> DDR
> +
> +The table below presents a description of the above algorithms.
> +
> +
> ===
> +Name Description
> +
> ===
> +Optical Black Correction Optical Black Correction block subtracts a
> pre-defined
> + value from the respective pixel values to obtain better
> + image quality.
> + Defined in :c:type:`ipu3_uapi_obgrid_param`.
> +Linearization This algo block uses linearization parameters
> to
> + address non-linearity sensor effects. The Lookup table
> + table is defined in
> + :c:type:`ipu3_uapi_isp_lin_vmem_params`.
> +SHD Lens shading correction is used to correct spatial
> + non-uniformity of the pixel response due to optical
> + lens shading. This is done by applying a different gain
> +
Re: [PATCHv17 23/34] videobuf2-core: integrate with media requests
Em Sat, 4 Aug 2018 14:45:15 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Buffers can now be prepared or queued for a request.
>
> A buffer is unbound from the request at vb2_buffer_done time or
> when the queue is cancelled.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> .../media/common/videobuf2/videobuf2-core.c | 133 +-
> .../media/common/videobuf2/videobuf2-v4l2.c | 4 +-
> drivers/media/dvb-core/dvb_vb2.c | 2 +-
> include/media/videobuf2-core.h| 18 ++-
> 4 files changed, 147 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 230f83d6d094..1efb9c8b359d 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -499,8 +499,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned
> int buffers)
> pr_info(" buf_init: %u buf_cleanup: %u buf_prepare:
> %u buf_finish: %u\n",
> vb->cnt_buf_init, vb->cnt_buf_cleanup,
> vb->cnt_buf_prepare, vb->cnt_buf_finish);
> - pr_info(" buf_queue: %u buf_done: %u\n",
> - vb->cnt_buf_queue, vb->cnt_buf_done);
> + pr_info(" buf_queue: %u buf_done: %u
> buf_request_complete: %u\n",
> + vb->cnt_buf_queue, vb->cnt_buf_done,
> + vb->cnt_buf_request_complete);
> pr_info(" alloc: %u put: %u prepare: %u finish: %u
> mmap: %u\n",
> vb->cnt_mem_alloc, vb->cnt_mem_put,
> vb->cnt_mem_prepare, vb->cnt_mem_finish,
> @@ -936,6 +937,14 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
> vb2_buffer_state state)
> vb->state = state;
> }
> atomic_dec(>owned_by_drv_count);
> +
> + if (vb->req_obj.req) {
> + /* This is not supported at the moment */
> + WARN_ON(state == VB2_BUF_STATE_REQUEUEING);
> + media_request_object_unbind(>req_obj);
> + media_request_object_put(>req_obj);
> + }
> +
> spin_unlock_irqrestore(>done_lock, flags);
>
> trace_vb2_buf_done(q, vb);
> @@ -1290,6 +1299,60 @@ static int __buf_prepare(struct vb2_buffer *vb)
> return 0;
> }
>
> +static int vb2_req_prepare(struct media_request_object *obj)
> +{
> + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> + int ret;
> +
> + if (WARN_ON(vb->state != VB2_BUF_STATE_IN_REQUEST))
> + return -EINVAL;
> +
> + mutex_lock(vb->vb2_queue->lock);
> + ret = __buf_prepare(vb);
> + mutex_unlock(vb->vb2_queue->lock);
> + return ret;
> +}
> +
> +static void __vb2_dqbuf(struct vb2_buffer *vb);
> +
> +static void vb2_req_unprepare(struct media_request_object *obj)
> +{
> + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> +
> + mutex_lock(vb->vb2_queue->lock);
> + __vb2_dqbuf(vb);
> + vb->state = VB2_BUF_STATE_IN_REQUEST;
> + mutex_unlock(vb->vb2_queue->lock);
> + WARN_ON(!vb->req_obj.req);
> +}
> +
> +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
> + struct media_request *req);
> +
> +static void vb2_req_queue(struct media_request_object *obj)
> +{
> + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> +
> + mutex_lock(vb->vb2_queue->lock);
> + vb2_core_qbuf(vb->vb2_queue, vb->index, NULL, NULL);
> + mutex_unlock(vb->vb2_queue->lock);
> +}
> +
> +static void vb2_req_release(struct media_request_object *obj)
> +{
> + struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
> +
> + if (vb->state == VB2_BUF_STATE_IN_REQUEST)
> + vb->state = VB2_BUF_STATE_DEQUEUED;
Shouldn't it be protected by the lock?
> +}
> +
> +static const struct media_request_object_ops vb2_core_req_ops = {
> + .prepare = vb2_req_prepare,
> + .unprepare = vb2_req_unprepare,
> + .queue = vb2_req_queue,
> + .release = vb2_req_release,
> +};
> +
> int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
> {
> struct vb2_buffer *vb;
> @@ -1315,7 +1378,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned
> int index, void *pb)
>
> dprintk(2, "prepare of buffer %d succeeded\n", vb->index);
>
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
>
> @@ -1382,7 +1445,8 @@ static int vb2_start_streaming(struct vb2_queue *q)
> return ret;
> }
>
> -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
> +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
> + struct media_request *req)
> {
> struct vb2_buffer *vb;
> int
Re: [PATCHv17 22/34] videobuf2-core: embed media_request_object
Em Sat, 4 Aug 2018 14:45:14 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Make vb2_buffer a request object.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> include/media/videobuf2-core.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index cbda3968d018..df92dcdeabb3 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
>
> #define VB2_MAX_FRAME(32)
> #define VB2_MAX_PLANES (8)
> @@ -236,6 +237,8 @@ struct vb2_queue;
> * @num_planes: number of planes in the buffer
> * on an internal driver queue.
> * @timestamp: frame timestamp in ns.
> + * @req_obj: used to bind this buffer to a request. This
> + * request object has a refcount.
> */
> struct vb2_buffer {
> struct vb2_queue*vb2_queue;
> @@ -244,6 +247,7 @@ struct vb2_buffer {
> unsigned intmemory;
> unsigned intnum_planes;
> u64 timestamp;
> + struct media_request_object req_obj;
>
> /* private: internal use only
>*
Thanks,
Mauro
Re: [PATCHv17 21/34] vb2: add init_buffer buffer op
Em Sat, 4 Aug 2018 14:45:13 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> We need to initialize the request_fd field in struct vb2_v4l2_buffer
> to -1 instead of the default of 0. So we need to add a new op that
> is called when struct vb2_v4l2_buffer is allocated.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 2 ++
> include/media/videobuf2-core.h | 4
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index eead693ba619..230f83d6d094 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -356,6 +356,8 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
> vb2_memory memory,
> vb->planes[plane].length = plane_sizes[plane];
> vb->planes[plane].min_length = plane_sizes[plane];
> }
> + call_void_bufop(q, init_buffer, vb);
> +
> q->bufs[vb->index] = vb;
>
> /* Allocate video buffer memory for the MMAP type */
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 5e760d5f280a..cbda3968d018 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -408,6 +408,9 @@ struct vb2_ops {
> * @verify_planes_array: Verify that a given user space structure contains
> * enough planes for the buffer. This is called
> * for each dequeued buffer.
> + * @init_buffer: given a _buffer initialize the extra data after
> + * struct vb2_buffer.
> + * For V4L2 this is a vb2_v4l2_buffer.
> * @fill_user_buffer:given a _buffer fill in the userspace
> structure.
> * For V4L2 this is a v4l2_buffer.
> * @fill_vb2_buffer: given a userspace structure, fill in the _buffer.
> @@ -418,6 +421,7 @@ struct vb2_ops {
> */
> struct vb2_buf_ops {
> int (*verify_planes_array)(struct vb2_buffer *vb, const void *pb);
> + void (*init_buffer)(struct vb2_buffer *vb);
> void (*fill_user_buffer)(struct vb2_buffer *vb, void *pb);
> int (*fill_vb2_buffer)(struct vb2_buffer *vb, struct vb2_plane *planes);
> void (*copy_timestamp)(struct vb2_buffer *vb, const void *pb);
Thanks,
Mauro
Re: [PATCHv17 20/34] videodev2.h: Add request_fd field to v4l2_buffer
Em Sat, 4 Aug 2018 14:45:12 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> When queuing buffers allow for passing the request that should
> be associated with this buffer.
>
> If V4L2_BUF_FLAG_REQUEST_FD is set, then request_fd is used as
> the file descriptor.
>
> If a buffer is stored in a request, but not yet queued to the
> driver, then V4L2_BUF_FLAG_IN_REQUEST is set.
>
> Signed-off-by: Hans Verkuil
Reviewed-by: Mauro Carvalho Chehab
> ---
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 +-
> drivers/media/usb/cpia2/cpia2_v4l.c | 2 +-
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 ++---
> drivers/media/v4l2-core/v4l2-ioctl.c| 4 ++--
> include/uapi/linux/videodev2.h | 10 +-
> 5 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index a677e2c26247..64905d87465c 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -384,7 +384,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb,
> void *pb)
> b->timecode = vbuf->timecode;
> b->sequence = vbuf->sequence;
> b->reserved2 = 0;
> - b->reserved = 0;
> + b->request_fd = 0;
>
> if (q->is_multiplanar) {
> /*
> diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c
> b/drivers/media/usb/cpia2/cpia2_v4l.c
> index 99f106b13280..13aee9f67d05 100644
> --- a/drivers/media/usb/cpia2/cpia2_v4l.c
> +++ b/drivers/media/usb/cpia2/cpia2_v4l.c
> @@ -949,7 +949,7 @@ static int cpia2_dqbuf(struct file *file, void *fh,
> struct v4l2_buffer *buf)
> buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
> buf->length = cam->frame_size;
> buf->reserved2 = 0;
> - buf->reserved = 0;
> + buf->request_fd = 0;
> memset(>timecode, 0, sizeof(buf->timecode));
>
> DBG("DQBUF #%d status:%d seq:%d length:%d\n", buf->index,
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index dcce86c1fe40..633465d21d04 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -482,7 +482,7 @@ struct v4l2_buffer32 {
> } m;
> __u32 length;
> __u32 reserved2;
> - __u32 reserved;
> + __s32 request_fd;
> };
>
> static int get_v4l2_plane32(struct v4l2_plane __user *p64,
> @@ -581,6 +581,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user
> *p64,
> {
> u32 type;
> u32 length;
> + s32 request_fd;
> enum v4l2_memory memory;
> struct v4l2_plane32 __user *uplane32;
> struct v4l2_plane __user *uplane;
> @@ -595,7 +596,9 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user
> *p64,
> get_user(memory, >memory) ||
> put_user(memory, >memory) ||
> get_user(length, >length) ||
> - put_user(length, >length))
> + put_user(length, >length) ||
> + get_user(request_fd, >request_fd) ||
> + put_user(request_fd, >request_fd))
> return -EFAULT;
>
> if (V4L2_TYPE_IS_OUTPUT(type))
> @@ -699,7 +702,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user
> *p64,
> copy_in_user(>timecode, >timecode, sizeof(p64->timecode))
> ||
> assign_in_user(>sequence, >sequence) ||
> assign_in_user(>reserved2, >reserved2) ||
> - assign_in_user(>reserved, >reserved) ||
> + assign_in_user(>request_fd, >request_fd) ||
> get_user(length, >length) ||
> put_user(length, >length))
> return -EFAULT;
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 20b5145a5254..2a84ca9e328a 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -474,13 +474,13 @@ static void v4l_print_buffer(const void *arg, bool
> write_only)
> const struct v4l2_plane *plane;
> int i;
>
> - pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, flags=0x%08x,
> field=%s, sequence=%d, memory=%s",
> + pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request_fd=%d,
> flags=0x%08x, field=%s, sequence=%d, memory=%s",
> p->timestamp.tv_sec / 3600,
> (int)(p->timestamp.tv_sec / 60) % 60,
> (int)(p->timestamp.tv_sec % 60),
> (long)p->timestamp.tv_usec,
> p->index,
> - prt_names(p->type, v4l2_type_names),
> + prt_names(p->type, v4l2_type_names), p->request_fd,
> p->flags, prt_names(p->field, v4l2_field_names),
> p->sequence, prt_names(p->memory, v4l2_memory_names));
>
> diff
Re: [PATCHv17 19/34] vb2: drop VB2_BUF_STATE_PREPARED, use bool prepared/synced instead
Em Sat, 4 Aug 2018 14:45:11 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The PREPARED state becomes a problem with the request API: a buffer
> could be PREPARED but dequeued, or PREPARED and in state IN_REQUEST.
>
> PREPARED is really not a state as such, but more a property of the
> buffer. So make new 'prepared' and 'synced' bools instead to remember
> whether the buffer is prepared and/or synced or not.
>
> V4L2_BUF_FLAG_PREPARED is only set if the buffer is both synced and
> prepared and in the DEQUEUED state.
>
> Signed-off-by: Hans Verkuil
> ---
> .../media/common/videobuf2/videobuf2-core.c | 38 +--
> .../media/common/videobuf2/videobuf2-v4l2.c | 16 +---
> include/media/videobuf2-core.h| 6 ++-
> 3 files changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 7401a17c80ca..eead693ba619 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -682,7 +682,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> }
>
> /*
> - * Call queue_cancel to clean up any buffers in the PREPARED or
> + * Call queue_cancel to clean up any buffers in the
>* QUEUED state which is possible if buffers were prepared or
>* queued without ever calling STREAMON.
>*/
> @@ -921,6 +921,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
> vb2_buffer_state state)
> /* sync buffers */
> for (plane = 0; plane < vb->num_planes; ++plane)
> call_void_memop(vb, finish, vb->planes[plane].mem_priv);
> + vb->synced = false;
Shouldn't be prepared cleaned as well on reqbufs?
> }
>
> spin_lock_irqsave(>done_lock, flags);
> @@ -1239,6 +1240,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> static int __buf_prepare(struct vb2_buffer *vb)
> {
> struct vb2_queue *q = vb->vb2_queue;
> + enum vb2_buffer_state orig_state = vb->state;
> unsigned int plane;
> int ret;
>
> @@ -1247,6 +1249,10 @@ static int __buf_prepare(struct vb2_buffer *vb)
> return -EIO;
> }
>
> + if (vb->prepared)
> + return 0;
> + WARN_ON(vb->synced);
> +
> vb->state = VB2_BUF_STATE_PREPARING;
>
> switch (q->memory) {
> @@ -1262,11 +1268,12 @@ static int __buf_prepare(struct vb2_buffer *vb)
> default:
> WARN(1, "Invalid queue type\n");
> ret = -EINVAL;
> + break;
> }
Hmm... is this hunk a bug fix? if so, please split into a separate patch
and add a c/c stable.
>
> if (ret) {
> dprintk(1, "buffer preparation failed: %d\n", ret);
> - vb->state = VB2_BUF_STATE_DEQUEUED;
> + vb->state = orig_state;
> return ret;
> }
>
> @@ -1274,7 +1281,9 @@ static int __buf_prepare(struct vb2_buffer *vb)
> for (plane = 0; plane < vb->num_planes; ++plane)
> call_void_memop(vb, prepare, vb->planes[plane].mem_priv);
>
> - vb->state = VB2_BUF_STATE_PREPARED;
> + vb->synced = true;
> + vb->prepared = true;
> + vb->state = orig_state;
>
> return 0;
> }
> @@ -1290,6 +1299,10 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned
> int index, void *pb)
> vb->state);
> return -EINVAL;
> }
> + if (vb->prepared) {
> + dprintk(1, "buffer already prepared\n");
> + return -EINVAL;
> + }
>
> ret = __buf_prepare(vb);
> if (ret)
> @@ -1381,11 +1394,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int
> index, void *pb)
>
> switch (vb->state) {
> case VB2_BUF_STATE_DEQUEUED:
> - ret = __buf_prepare(vb);
> - if (ret)
> - return ret;
> - break;
> - case VB2_BUF_STATE_PREPARED:
> + if (!vb->prepared) {
> + ret = __buf_prepare(vb);
> + if (ret)
> + return ret;
> + }
> break;
> case VB2_BUF_STATE_PREPARING:
> dprintk(1, "buffer still being prepared\n");
> @@ -1611,6 +1624,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int
> *pindex, void *pb,
> }
>
> call_void_vb_qop(vb, buf_finish, vb);
> + vb->prepared = false;
>
> if (pindex)
> *pindex = vb->index;
> @@ -1699,18 +1713,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> for (i = 0; i < q->num_buffers; ++i) {
> struct vb2_buffer *vb = q->bufs[i];
>
> - if (vb->state == VB2_BUF_STATE_PREPARED ||
> - vb->state == VB2_BUF_STATE_QUEUED) {
> + if (vb->synced) {
>
Re: [PATCHv17 18/34] davinci_vpfe: remove bogus vb2->state check
Em Sat, 4 Aug 2018 14:45:10 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > There is no need to check the vb2 state in the buf_prepare > callback: it can never be wrong. > > Since VB2_BUF_STATE_PREPARED will be removed in the next patch > we'll remove this unnecessary check (and use of that state) first. > > Signed-off-by: Hans Verkuil Reviewed-by: Mauro Carvalho Chehab > --- > drivers/staging/media/davinci_vpfe/vpfe_video.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c > b/drivers/staging/media/davinci_vpfe/vpfe_video.c > index 1269a983455e..4e3ec7fdc90d 100644 > --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c > +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c > @@ -1135,10 +1135,6 @@ static int vpfe_buffer_prepare(struct vb2_buffer *vb) > > v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_buffer_prepare\n"); > > - if (vb->state != VB2_BUF_STATE_ACTIVE && > - vb->state != VB2_BUF_STATE_PREPARED) > - return 0; > - > /* Initialize buffer */ > vb2_set_plane_payload(vb, 0, video->fmt.fmt.pix.sizeimage); > if (vb2_plane_vaddr(vb, 0) && Thanks, Mauro
Re: [PATCHv17 17/34] vb2: store userspace data in vb2_v4l2_buffer
Em Sat, 4 Aug 2018 14:45:09 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The userspace-provided plane data needs to be stored in
> vb2_v4l2_buffer. Currently this information is applied by
> __fill_vb2_buffer() which is called by the core prepare_buf
> and qbuf functions, but when using requests these functions
> aren't called yet since the buffer won't be prepared until
> the media request is actually queued.
>
> In the meantime this information has to be stored somewhere
> and vb2_v4l2_buffer is a good place for it.
>
> The __fill_vb2_buffer callback now just copies the relevant
> information from vb2_v4l2_buffer into the planes array.
Reviewing this patch is very hard, as it does more than one
thing at a time. Also, if something goes bad and we need to revisit,
people will lose precious time trying to understand what are the
actual changes done here.
So, could you please split this patch into a preparation one that
would be just splitting the __fill_vb2_buffer() into two functions
(without any other change), and another one doing the changes required
for req buffers?
>
> Signed-off-by: Hans Verkuil
> ---
> .../media/common/videobuf2/videobuf2-core.c | 43 +--
> .../media/common/videobuf2/videobuf2-v4l2.c | 327 ++
> drivers/media/dvb-core/dvb_vb2.c | 3 +-
> include/media/videobuf2-core.h| 3 +-
> include/media/videobuf2-v4l2.h| 2 +
> 5 files changed, 209 insertions(+), 169 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 5653e8eebe2b..7401a17c80ca 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -967,20 +967,19 @@ EXPORT_SYMBOL_GPL(vb2_discard_done);
> /*
> * __prepare_mmap() - prepare an MMAP buffer
> */
> -static int __prepare_mmap(struct vb2_buffer *vb, const void *pb)
> +static int __prepare_mmap(struct vb2_buffer *vb)
> {
> int ret = 0;
>
> - if (pb)
> - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> - vb, pb, vb->planes);
> + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> + vb, vb->planes);
> return ret ? ret : call_vb_qop(vb, buf_prepare, vb);
> }
>
> /*
> * __prepare_userptr() - prepare a USERPTR buffer
> */
> -static int __prepare_userptr(struct vb2_buffer *vb, const void *pb)
> +static int __prepare_userptr(struct vb2_buffer *vb)
> {
> struct vb2_plane planes[VB2_MAX_PLANES];
> struct vb2_queue *q = vb->vb2_queue;
> @@ -991,12 +990,10 @@ static int __prepare_userptr(struct vb2_buffer *vb,
> const void *pb)
>
> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> /* Copy relevant information provided by the userspace */
> - if (pb) {
> - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> - vb, pb, planes);
> - if (ret)
> - return ret;
> - }
> + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> + vb, planes);
> + if (ret)
> + return ret;
>
> for (plane = 0; plane < vb->num_planes; ++plane) {
> /* Skip the plane if already verified */
> @@ -1096,7 +1093,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
> const void *pb)
> /*
> * __prepare_dmabuf() - prepare a DMABUF buffer
> */
> -static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb)
> +static int __prepare_dmabuf(struct vb2_buffer *vb)
> {
> struct vb2_plane planes[VB2_MAX_PLANES];
> struct vb2_queue *q = vb->vb2_queue;
> @@ -1107,12 +1104,10 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
> const void *pb)
>
> memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> /* Copy relevant information provided by the userspace */
> - if (pb) {
> - ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> - vb, pb, planes);
> - if (ret)
> - return ret;
> - }
> + ret = call_bufop(vb->vb2_queue, fill_vb2_buffer,
> + vb, planes);
> + if (ret)
> + return ret;
>
> for (plane = 0; plane < vb->num_planes; ++plane) {
> struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
> @@ -1241,7 +1236,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> call_void_vb_qop(vb, buf_queue, vb);
> }
>
> -static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
> +static int __buf_prepare(struct vb2_buffer *vb)
> {
> struct vb2_queue *q = vb->vb2_queue;
> unsigned int plane;
> @@ -1256,13 +1251,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const
> void *pb)
>
> switch (q->memory) {
> case VB2_MEMORY_MMAP:
> - ret = __prepare_mmap(vb, pb);
> + ret =
Re: [PATCHv17 16/34] v4l2-ctrls: add v4l2_ctrl_request_hdl_find/put/ctrl_find functions
Em Sat, 4 Aug 2018 14:45:08 +0200
Hans Verkuil escreveu:
> If a driver needs to find/inspect the controls set in a request then
> it can use these functions.
>
> E.g. to check if a required control is set in a request use this in the
> req_validate() implementation:
>
> int res = -EINVAL;
>
> hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
> if (hdl) {
> if (v4l2_ctrl_request_hdl_ctrl_find(hdl, ctrl_id))
> res = 0;
> v4l2_ctrl_request_hdl_put(hdl);
> }
> return res;
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 25
> include/media/v4l2-ctrls.h | 44 +++-
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 86a6ae54ccaa..2a30be824491 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2976,6 +2976,31 @@ static const struct media_request_object_ops req_ops =
> {
> .release = v4l2_ctrl_request_release,
> };
>
> +struct v4l2_ctrl_handler *v4l2_ctrl_request_hdl_find(struct media_request
> *req,
> + struct v4l2_ctrl_handler *parent)
> +{
> + struct media_request_object *obj;
> +
> + if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING &&
> + req->state != MEDIA_REQUEST_STATE_QUEUED))
> + return NULL;
> +
> + obj = media_request_object_find(req, _ops, parent);
> + if (obj)
> + return container_of(obj, struct v4l2_ctrl_handler, req_obj);
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_ctrl_request_hdl_find);
> +
> +struct v4l2_ctrl *
> +v4l2_ctrl_request_hdl_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id)
> +{
> + struct v4l2_ctrl_ref *ref = find_ref_lock(hdl, id);
> +
> + return (ref && ref->req == ref) ? ref->ctrl : NULL;
Doesn't those helper functions (including this one) be serialized?
> +}
> +EXPORT_SYMBOL_GPL(v4l2_ctrl_request_hdl_ctrl_find);
> +
> static int v4l2_ctrl_request_bind(struct media_request *req,
> struct v4l2_ctrl_handler *hdl,
> struct v4l2_ctrl_handler *from)
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 98b1e70a4a46..aeb7f3c24ef7 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -,7 +,49 @@ void v4l2_ctrl_request_setup(struct media_request *req,
> * request object.
> */
> void v4l2_ctrl_request_complete(struct media_request *req,
> - struct v4l2_ctrl_handler *hdl);
> + struct v4l2_ctrl_handler *parent);
> +
> +/**
> + * v4l2_ctrl_request_hdl_find - Find the control handler in the request
> + *
> + * @req: The request
> + * @parent: The parent control handler ('priv' in
> media_request_object_find())
> + *
> + * This function finds the control handler in the request. It may return
> + * NULL if not found. When done, you must call v4l2_ctrl_request_put_hdl()
> + * with the returned handler pointer.
> + *
> + * If the request is not in state VALIDATING or QUEUED, then this function
> + * will always return NULL.
> + */
> +struct v4l2_ctrl_handler *v4l2_ctrl_request_hdl_find(struct media_request
> *req,
> + struct v4l2_ctrl_handler *parent);
> +
> +/**
> + * v4l2_ctrl_request_hdl_put - Put the control handler
> + *
> + * @hdl: Put this control handler
> + *
> + * This function released the control handler previously obtained from'
> + * v4l2_ctrl_request_hdl_find().
> + */
> +static inline void v4l2_ctrl_request_hdl_put(struct v4l2_ctrl_handler *hdl)
> +{
> + if (hdl)
> + media_request_object_put(>req_obj);
> +}
> +
> +/**
> + * v4l2_ctrl_request_ctrl_find() - Find a control with the given ID.
> + *
> + * @hdl: The control handler from the request.
> + * @id: The ID of the control to find.
> + *
> + * This function returns a pointer to the control if this control is
> + * part of the request or NULL otherwise.
> + */
> +struct v4l2_ctrl *
> +v4l2_ctrl_request_hdl_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id);
>
> /* Helpers for ioctl_ops */
>
Thanks,
Mauro
Re: [PATCHv17 15/34] v4l2-ctrls: support g/s_ext_ctrls for requests
Em Sat, 4 Aug 2018 14:45:07 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The v4l2_g/s_ext_ctrls functions now support control handlers that
> represent requests.
>
> The v4l2_ctrls_find_req_obj() function is responsible for finding the
> request from the fd.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/platform/omap3isp/ispvideo.c | 2 +-
> drivers/media/v4l2-core/v4l2-ctrls.c | 138 +++--
> drivers/media/v4l2-core/v4l2-ioctl.c | 12 +-
> drivers/media/v4l2-core/v4l2-subdev.c | 9 +-
> include/media/v4l2-ctrls.h | 7 +-
> 5 files changed, 149 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c
> index 9d228eac24ea..674e7fd3ad99 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1028,7 +1028,7 @@ static int isp_video_check_external_subdevs(struct
> isp_video *video,
> ctrls.count = 1;
> ctrls.controls =
>
> - ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, );
> + ret = v4l2_g_ext_ctrls(pipe->external->ctrl_handler, NULL, );
> if (ret < 0) {
> dev_warn(isp->dev, "no pixel rate control in subdev %s\n",
>pipe->external->name);
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index b8ff6d6b14cd..86a6ae54ccaa 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -3140,7 +3140,8 @@ static int class_check(struct v4l2_ctrl_handler *hdl,
> u32 which)
> }
>
> /* Get extended controls. Allocates the helpers array if needed. */
> -int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls
> *cs)
> +int __v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> +struct v4l2_ext_controls *cs)
I don't like the idea of just adding a __ before the name here. We
generally use __foo() for a non-locked version of foo(). This is not
the case here.
The non-standard convention is even worse here, as this is not a
static function.
Perhaps it could be called something like v4l2_g_ext_ctrls_no_req(),
v4l2_g_ext_ctrls_common() or something similar.
> {
> struct v4l2_ctrl_helper helper[4];
> struct v4l2_ctrl_helper *helpers = helper;
> @@ -3220,6 +3221,83 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> struct v4l2_ext_controls *cs
> kvfree(helpers);
> return ret;
> }
> +
> +static struct media_request_object *
> +v4l2_ctrls_find_req_obj(struct v4l2_ctrl_handler *hdl,
> + struct media_request *req, bool set)
> +{
> + struct media_request_object *obj;
> + struct v4l2_ctrl_handler *new_hdl;
> + int ret;
> +
> + if (IS_ERR(req))
> + return ERR_CAST(req);
> +
> + if (set && WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING))
> + return ERR_PTR(-EBUSY);
> +
> + obj = media_request_object_find(req, _ops, hdl);
> + if (obj)
> + return obj;
> + if (!set)
> + return ERR_PTR(-ENOENT);
> +
> + new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL);
> + if (!new_hdl)
> + return ERR_PTR(-ENOMEM);
> +
> + obj = _hdl->req_obj;
> + ret = v4l2_ctrl_handler_init(new_hdl, (hdl->nr_of_buckets - 1) * 8);
> + if (!ret)
> + ret = v4l2_ctrl_request_bind(req, new_hdl, hdl);
> + if (ret) {
> + kfree(new_hdl);
> +
> + return ERR_PTR(ret);
> + }
> +
> + media_request_object_get(obj);
> + return obj;
> +}
> +
> +int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct media_device
> *mdev,
> + struct v4l2_ext_controls *cs)
> +{
> + struct media_request_object *obj = NULL;
> + int ret;
> +
> + if (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL) {
> + struct media_request *req;
> +
> + if (!mdev || cs->request_fd < 0)
> + return -EINVAL;
> +
> + req = media_request_get_by_fd(mdev, cs->request_fd);
> + if (IS_ERR(req))
> + return PTR_ERR(req);
> +
> + if (req->state != MEDIA_REQUEST_STATE_IDLE &&
> + req->state != MEDIA_REQUEST_STATE_COMPLETE) {
> + media_request_put(req);
> + return -EBUSY;
> + }
> +
> + obj = v4l2_ctrls_find_req_obj(hdl, req, false);
> + /* Reference to the request held through obj */
> + media_request_put(req);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + hdl = container_of(obj, struct v4l2_ctrl_handler,
> +req_obj);
> + }
> +
> + ret = __v4l2_g_ext_ctrls(hdl, cs);
> +
> + if (obj)
> + media_request_object_put(obj);
> + return ret;
> +}
>
Re: [PATCHv17 14/34] v4l2-ctrls: add core request support
Em Sat, 4 Aug 2018 14:45:06 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Integrate the request support. This adds the v4l2_ctrl_request_complete
> and v4l2_ctrl_request_setup functions to complete a request and (as a
> helper function) to apply a request to the hardware.
>
> It takes care of queuing requests and correctly chaining control values
> in the request queue.
>
> Note that when a request is marked completed it will copy control values
> to the internal request state. This can be optimized in the future since
> this is sub-optimal when dealing with large compound and/or array controls.
>
> For the initial 'stateless codec' use-case the current implementation is
> sufficient.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 326 ++-
> include/media/v4l2-ctrls.h | 51 +
> 2 files changed, 371 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 570b6f8ae46a..b8ff6d6b14cd 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1668,6 +1668,13 @@ static int new_to_user(struct v4l2_ext_control *c,
> return ptr_to_user(c, ctrl, ctrl->p_new);
> }
>
> +/* Helper function: copy the request value back to the caller */
> +static int req_to_user(struct v4l2_ext_control *c,
> +struct v4l2_ctrl_ref *ref)
> +{
> + return ptr_to_user(c, ref->ctrl, ref->p_req);
> +}
> +
> /* Helper function: copy the initial control value back to the caller */
> static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
> {
> @@ -1787,6 +1794,26 @@ static void cur_to_new(struct v4l2_ctrl *ctrl)
> ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new);
> }
>
> +/* Copy the new value to the request value */
> +static void new_to_req(struct v4l2_ctrl_ref *ref)
> +{
> + if (!ref)
> + return;
> + ptr_to_ptr(ref->ctrl, ref->ctrl->p_new, ref->p_req);
> + ref->req = ref;
> +}
> +
> +/* Copy the request value to the new value */
> +static void req_to_new(struct v4l2_ctrl_ref *ref)
> +{
> + if (!ref)
> + return;
> + if (ref->req)
> + ptr_to_ptr(ref->ctrl, ref->req->p_req, ref->ctrl->p_new);
> + else
> + ptr_to_ptr(ref->ctrl, ref->ctrl->p_cur, ref->ctrl->p_new);
> +}
> +
> /* Return non-zero if one or more of the controls in the cluster has a new
> value that differs from the current value. */
> static int cluster_changed(struct v4l2_ctrl *master)
> @@ -1896,6 +1923,9 @@ int v4l2_ctrl_handler_init_class(struct
> v4l2_ctrl_handler *hdl,
> lockdep_set_class_and_name(hdl->lock, key, name);
> INIT_LIST_HEAD(>ctrls);
> INIT_LIST_HEAD(>ctrl_refs);
> + INIT_LIST_HEAD(>requests);
> + INIT_LIST_HEAD(>requests_queued);
> + hdl->request_is_queued = false;
> hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8;
> hdl->buckets = kvmalloc_array(hdl->nr_of_buckets,
> sizeof(hdl->buckets[0]),
> @@ -1916,6 +1946,14 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler
> *hdl)
> if (hdl == NULL || hdl->buckets == NULL)
> return;
>
> + if (!hdl->req_obj.req && !list_empty(>requests)) {
> + struct v4l2_ctrl_handler *req, *next_req;
> +
> + list_for_each_entry_safe(req, next_req, >requests,
> requests) {
> + media_request_object_unbind(>req_obj);
> + media_request_object_put(>req_obj);
Hmm... while this would work for the trivial case where object_put()
would just drop the object from the list if nobody else is using it,
nothing prevents that, if v4l2_ctrl_handler_free() is called twice,
it would do the wrong thing... as the only test here is if req_obj.reg
is not NULL, and not if the control framework is already done with the
object.
> + }
> + }
> mutex_lock(hdl->lock);
> /* Free all nodes */
> list_for_each_entry_safe(ref, next_ref, >ctrl_refs, node) {
> @@ -2837,6 +2875,123 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl,
> struct v4l2_querymenu *qm)
> }
> EXPORT_SYMBOL(v4l2_querymenu);
>
> +static int v4l2_ctrl_request_clone(struct v4l2_ctrl_handler *hdl,
> +const struct v4l2_ctrl_handler *from)
> +{
> + struct v4l2_ctrl_ref *ref;
> + int err;
> +
> + if (WARN_ON(!hdl || hdl == from))
> + return -EINVAL;
> +
> + if (hdl->error)
> + return hdl->error;
> +
> + WARN_ON(hdl->lock != >_lock);
> +
> + mutex_lock(from->lock);
> + list_for_each_entry(ref, >ctrl_refs, node) {
> + struct v4l2_ctrl *ctrl = ref->ctrl;
> + struct v4l2_ctrl_ref *new_ref;
> +
> + /* Skip refs inherited from other devices */
> + if (ref->from_other_dev)
> + continue;
> + /* And
mobile apps for you
Do you need mobile apps development? We can do it for you. We are an India base company. Here are the details about us: Years in business: 8 Staffs: 125 App developed: 350 We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients across different kind of industries. Such like retail, media and entertainment, BFSI, hospitality, social media, eCommerce, food and beverages, etc. We do below: Mobile Apps Mobile App UI/UX designing App Maintenance and Support Website or ecommerce portal development Please reply back if you are interested in what we do. We will share our portfolios to you. Regards, Jack
[PATCH v2 1/5] media: ov5640: fix exposure regression
fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at
start time").
Symptom was black image when capturing HD or 5Mp picture
due to manual exposure set to 1 while it was intended to
set autoexposure to "manual", fix this.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..4b9da8b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -938,6 +938,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
return ret;
}
+static int ov5640_set_autoexposure(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(0), on ? 0 : BIT(0));
+}
+
/* read exposure, in number of line periods */
static int ov5640_get_exposure(struct ov5640_dev *sensor)
{
@@ -1593,7 +1599,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
const struct ov5640_mode_info *mode,
- s32 exposure)
+ bool auto_exp)
{
int ret;
@@ -1610,7 +1616,8 @@ static int ov5640_set_mode_direct(struct ov5640_dev
*sensor,
if (ret)
return ret;
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure);
+ return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
+ V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1618,7 +1625,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
- s32 exposure;
+ bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
dn_mode = mode->dn_mode;
@@ -1629,8 +1636,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret)
return ret;
- exposure = sensor->ctrls.auto_exp->val;
- ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
+ ret = ov5640_set_autoexposure(sensor, false);
if (ret)
return ret;
@@ -1646,7 +1652,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, exposure);
+ ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
}
if (ret < 0)
--
2.7.4
[PATCH v2 0/5] Fix OV5640 exposure & gain
This patch serie fixes some problems around exposure & gain in OV5640 driver. The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c: https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html Here is the test procedure used for exposure & gain controls check: 1) Preview in background $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" ! queue ! waylandsink -e & 2) Check gain & exposure values $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=19 flags=inactive, volatile 3) Put finger in front of camera and check that gain/exposure values are changing: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=660 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=37 flags=inactive, volatile 4) switch to manual mode, image exposition must not change $> v4l2-ctl --set-ctrl=gain_automatic=0 $> v4l2-ctl --set-ctrl=auto_exposure=1 Note the "1" for manual exposure. 5) Check current gain/exposure values: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 gain (int): min=0 max=1023 step=1 default=0 value=20 6) Put finger behind camera and check that gain/exposure values are NOT changing: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 gain (int): min=0 max=1023 step=1 default=0 value=20 7) Update exposure, check that it is well changed on display and that same value is returned: $> v4l2-ctl --set-ctrl=exposure=100 $> v4l2-ctl --get-ctrl=exposure exposure: 100 9) Update gain, check that it is well changed on display and that same value is returned: $> v4l2-ctl --set-ctrl=gain=10 $> v4l2-ctl --get-ctrl=gain gain: 10 10) Switch back to auto gain/exposure, verify that image is correct and values returned are correct: $> v4l2-ctl --set-ctrl=gain_automatic=1 $> v4l2-ctl --set-ctrl=auto_exposure=0 $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=22 flags=inactive, volatile Note the "0" for auto exposure. === = history = === version 2: - Fix patch 3/5 commit comment and rename binning function as per jacopo' suggestion: https://www.mail-archive.com/linux-media@vger.kernel.org/msg133272.html Hugues Fruchet (5): media: ov5640: fix exposure regression media: ov5640: fix auto gain & exposure when changing mode media: ov5640: fix wrong binning value in exposure calculation media: ov5640: fix auto controls values when switching to manual mode media: ov5640: fix restore of last mode set drivers/media/i2c/ov5640.c | 124 ++--- 1 file changed, 72 insertions(+), 52 deletions(-) -- 2.7.4
[PATCH v2 3/5] media: ov5640: fix wrong binning value in exposure calculation
ov5640_set_mode_exposure_calc() is checking binning value but
binning value read is buggy, fix this.
Rename ov5640_binning_on() to ov5640_get_binning() as per other
similar functions.
Signed-off-by: Hugues Fruchet
---
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 7c569de..9fb17b5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1349,7 +1349,7 @@ static int ov5640_set_ae_target(struct ov5640_dev
*sensor, int target)
return ov5640_write_reg(sensor, OV5640_REG_AEC_CTRL1F, fast_low);
}
-static int ov5640_binning_on(struct ov5640_dev *sensor)
+static int ov5640_get_binning(struct ov5640_dev *sensor)
{
u8 temp;
int ret;
@@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, );
if (ret)
return ret;
- temp &= 0xfe;
- return temp ? 1 : 0;
+
+ return temp & BIT(0);
}
static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
@@ -1468,7 +1468,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
if (ret < 0)
return ret;
prev_shutter = ret;
- ret = ov5640_binning_on(sensor);
+ ret = ov5640_get_binning(sensor);
if (ret < 0)
return ret;
if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
--
2.7.4
[PATCH v2 4/5] media: ov5640: fix auto controls values when switching to manual mode
When switching from auto to manual mode, V4L2 core is calling
g_volatile_ctrl() in manual mode in order to get the manual initial value.
Remove the manual mode check/return to not break this behaviour.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 9fb17b5..c110a6a 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2277,16 +2277,12 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl
*ctrl)
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
- if (!ctrl->val)
- return 0;
val = ov5640_get_gain(sensor);
if (val < 0)
return val;
sensor->ctrls.gain->val = val;
break;
case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->val == V4L2_EXPOSURE_MANUAL)
- return 0;
val = ov5640_get_exposure(sensor);
if (val < 0)
return val;
--
2.7.4
[PATCH v2 2/5] media: ov5640: fix auto gain & exposure when changing mode
Ensure that auto gain and auto exposure are well restored
when changing mode.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 96 ++
1 file changed, 54 insertions(+), 42 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 4b9da8b..7c569de 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1000,6 +1000,18 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
return gain & 0x3ff;
}
+static int ov5640_set_gain(struct ov5640_dev *sensor, int gain)
+{
+ return ov5640_write_reg16(sensor, OV5640_REG_AEC_PK_REAL_GAIN,
+ (u16)gain & 0x3ff);
+}
+
+static int ov5640_set_autogain(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(1), on ? 0 : BIT(1));
+}
+
static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
{
int ret;
@@ -1577,7 +1589,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
}
/* set capture gain */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.gain, cap_gain16);
+ ret = ov5640_set_gain(sensor, cap_gain16);
if (ret)
return ret;
@@ -1590,7 +1602,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
}
/* set exposure */
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.exposure, cap_shutter);
+ return ov5640_set_exposure(sensor, cap_shutter);
}
/*
@@ -1598,26 +1610,13 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
* change mode directly
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *mode,
- bool auto_exp)
+ const struct ov5640_mode_info *mode)
{
- int ret;
-
if (!mode->reg_data)
return -EINVAL;
/* Write capture setting */
- ret = ov5640_load_regs(sensor, mode);
- if (ret < 0)
- return ret;
-
- /* turn auto gain/exposure back on for direct mode */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
- if (ret)
- return ret;
-
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
- V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
+ return ov5640_load_regs(sensor, mode);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1625,6 +1624,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
+ bool auto_gain = sensor->ctrls.auto_gain->val == 1;
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
@@ -1632,19 +1632,23 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
orig_dn_mode = orig_mode->dn_mode;
/* auto gain and exposure must be turned off when changing modes */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0);
- if (ret)
- return ret;
+ if (auto_gain) {
+ ret = ov5640_set_autogain(sensor, false);
+ if (ret)
+ return ret;
+ }
- ret = ov5640_set_autoexposure(sensor, false);
- if (ret)
- return ret;
+ if (auto_exp) {
+ ret = ov5640_set_autoexposure(sensor, false);
+ if (ret)
+ goto restore_auto_gain;
+ }
if ((dn_mode == SUBSAMPLING && orig_dn_mode == SCALING) ||
(dn_mode == SCALING && orig_dn_mode == SUBSAMPLING)) {
/*
* change between subsampling and scaling
-* go through exposure calucation
+* go through exposure calculation
*/
ret = ov5640_set_mode_exposure_calc(sensor, mode);
} else {
@@ -1652,11 +1656,16 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
+ ret = ov5640_set_mode_direct(sensor, mode);
}
-
if (ret < 0)
- return ret;
+ goto restore_auto_exp_gain;
+
+ /* restore auto gain and exposure */
+ if (auto_gain)
+ ov5640_set_autogain(sensor, true);
+ if (auto_exp)
+ ov5640_set_autoexposure(sensor, true);
ret = ov5640_set_timings(sensor, mode);
if (ret < 0)
@@ -1681,6 +1690,15 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
sensor->pending_mode_change = false;
return 0;
+
+restore_auto_exp_gain:
+ if (auto_exp)
+
[PATCH v2 5/5] media: ov5640: fix restore of last mode set
Mode setting depends on last mode set, in particular
because of exposure calculation when downscale mode
change between subsampling and scaling.
At stream on the last mode was wrongly set to current mode,
so no change was detected and exposure calculation
was not made, fix this.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c110a6a..923cc30 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -225,6 +225,7 @@ struct ov5640_dev {
struct v4l2_mbus_framefmt fmt;
const struct ov5640_mode_info *current_mode;
+ const struct ov5640_mode_info *last_mode;
enum ov5640_frame_rate current_fr;
struct v4l2_fract frame_interval;
@@ -1628,6 +1629,9 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
+ if (!orig_mode)
+ orig_mode = mode;
+
dn_mode = mode->dn_mode;
orig_dn_mode = orig_mode->dn_mode;
@@ -1688,6 +1692,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
return ret;
sensor->pending_mode_change = false;
+ sensor->last_mode = mode;
return 0;
@@ -2551,7 +2556,8 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int
enable)
if (sensor->streaming == !enable) {
if (enable && sensor->pending_mode_change) {
- ret = ov5640_set_mode(sensor, sensor->current_mode);
+ ret = ov5640_set_mode(sensor, sensor->last_mode);
+
if (ret)
goto out;
--
2.7.4
mobile apps design
Do you need mobile apps development? We can do it for you. We are an India base company. Here are the details about us: Years in business: 8 Staffs: 125 App developed: 350 We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients across different kind of industries. Such like retail, media and entertainment, BFSI, hospitality, social media, eCommerce, food and beverages, etc. We do below: Mobile Apps Mobile App UI/UX designing App Maintenance and Support Website or ecommerce portal development Please reply back if you are interested in what we do. We will share our portfolios to you. Regards, Jack
[PATCH v2 6/7] [media] tvp5150: initialize subdev before parsing device tree
From: Michael Tretter
There are several debug prints in the tvp5150_parse_dt() function, which
do not print the prefix, because the v4l2_subdev is not initialized, yet.
Initialize the v4l2_subdev before parsing the device tree to fix the
debug messages.
Signed-off-by: Michael Tretter
Signed-off-by: Marco Felsch
---
drivers/media/i2c/tvp5150.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 825dcf3a9d83..e736f609fecd 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -2032,6 +2032,9 @@ static int tvp5150_probe(struct i2c_client *c,
core->regmap = map;
sd = >sd;
+ v4l2_i2c_subdev_init(sd, c, _ops);
+ sd->internal_ops = _internal_ops;
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
if (IS_ENABLED(CONFIG_OF) && np) {
res = tvp5150_parse_dt(core, np);
@@ -2044,10 +2047,6 @@ static int tvp5150_probe(struct i2c_client *c,
core->mbus_type = V4L2_MBUS_BT656;
}
- v4l2_i2c_subdev_init(sd, c, _ops);
- sd->internal_ops = _internal_ops;
- sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-
res = tvp5150_mc_init(sd);
if (res)
goto err_cleanup_dt;
--
2.18.0
we do mobile apps
Do you need mobile apps development? We can do it for you. We are an India base company. Here are the details about us: Years in business: 8 Staffs: 125 App developed: 350 We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients across different kind of industries. Such like retail, media and entertainment, BFSI, hospitality, social media, eCommerce, food and beverages, etc. We do below: Mobile Apps Mobile App UI/UX designing App Maintenance and Support Website or ecommerce portal development Please reply back if you are interested in what we do. We will share our portfolios to you. Regards, Jack
[PATCH v2 7/7] [media] tvp5150: add s_power callback
Don't en-/disable the interrupts during s_stream because someone can
disable the stream but wants to get informed if the stream is locked
again. So keep the interrupts enabled the whole time the pipeline is
opened.
Signed-off-by: Marco Felsch
---
drivers/media/i2c/tvp5150.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index e736f609fecd..e296f5bfae21 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -1389,11 +1389,26 @@ static const struct media_entity_operations
tvp5150_sd_media_ops = {
/
I2C Command
/
+static int tvp5150_s_power(struct v4l2_subdev *sd, int on)
+{
+ struct tvp5150 *decoder = to_tvp5150(sd);
+ unsigned int val = 0;
+
+ if (on)
+ val = TVP5150_INT_A_LOCK;
+
+ if (decoder->irq)
+ /* Enable / Disable lock interrupt */
+ regmap_update_bits(decoder->regmap, TVP5150_INT_ENABLE_REG_A,
+ TVP5150_INT_A_LOCK, val);
+
+ return 0;
+}
static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
{
struct tvp5150 *decoder = to_tvp5150(sd);
- unsigned int mask, val = 0, int_val = 0;
+ unsigned int mask, val = 0;
mask = TVP5150_MISC_CTL_YCBCR_OE | TVP5150_MISC_CTL_SYNC_OE |
TVP5150_MISC_CTL_CLOCK_OE;
@@ -1406,15 +1421,10 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int
enable)
val = decoder->lock ? decoder->oe : 0;
else
val = decoder->oe;
- int_val = TVP5150_INT_A_LOCK;
v4l2_subdev_notify_event(>sd, _ev_fmt);
}
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, mask, val);
- if (decoder->irq)
- /* Enable / Disable lock interrupt */
- regmap_update_bits(decoder->regmap, TVP5150_INT_ENABLE_REG_A,
- TVP5150_INT_A_LOCK, int_val);
return 0;
}
@@ -1616,6 +1626,7 @@ static const struct v4l2_subdev_core_ops tvp5150_core_ops
= {
.g_register = tvp5150_g_register,
.s_register = tvp5150_s_register,
#endif
+ .s_power = tvp5150_s_power,
};
static const struct v4l2_subdev_tuner_ops tvp5150_tuner_ops = {
--
2.18.0
[PATCH v2 4/7] [media] v4l2-subdev: fix v4l2_subdev_get_try_* dependency
These helpers make us of the media-controller entity which is only
available if the CONFIG_MEDIA_CONTROLLER is enabled.
Signed-off-by: Marco Felsch
---
include/media/v4l2-subdev.h | 100 ++--
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index ce48f1fcf295..79c066934ad2 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -912,6 +912,56 @@ struct v4l2_subdev_fh {
#define to_v4l2_subdev_fh(fh) \
container_of(fh, struct v4l2_subdev_fh, vfh)
+extern const struct v4l2_file_operations v4l2_subdev_fops;
+
+/**
+ * v4l2_set_subdevdata - Sets V4L2 dev private device data
+ *
+ * @sd: pointer to v4l2_subdev
+ * @p: pointer to the private device data to be stored.
+ */
+static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
+{
+ sd->dev_priv = p;
+}
+
+/**
+ * v4l2_get_subdevdata - Gets V4L2 dev private device data
+ *
+ * @sd: pointer to v4l2_subdev
+ *
+ * Returns the pointer to the private device data to be stored.
+ */
+static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
+{
+ return sd->dev_priv;
+}
+
+/**
+ * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
+ *
+ * @sd: pointer to v4l2_subdev
+ * @p: pointer to the private data to be stored.
+ */
+static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
+{
+ sd->host_priv = p;
+}
+
+/**
+ * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
+ *
+ * @sd: pointer to v4l2_subdev
+ *
+ * Returns the pointer to the private host data to be stored.
+ */
+static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
+{
+ return sd->host_priv;
+}
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+
/**
* v4l2_subdev_get_try_format - ancillary routine to call
* v4l2_subdev_pad_config->try_fmt
@@ -978,56 +1028,6 @@ static inline struct v4l2_rect
#endif
}
-extern const struct v4l2_file_operations v4l2_subdev_fops;
-
-/**
- * v4l2_set_subdevdata - Sets V4L2 dev private device data
- *
- * @sd: pointer to v4l2_subdev
- * @p: pointer to the private device data to be stored.
- */
-static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
-{
- sd->dev_priv = p;
-}
-
-/**
- * v4l2_get_subdevdata - Gets V4L2 dev private device data
- *
- * @sd: pointer to v4l2_subdev
- *
- * Returns the pointer to the private device data to be stored.
- */
-static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
-{
- return sd->dev_priv;
-}
-
-/**
- * v4l2_set_subdev_hostdata - Sets V4L2 dev private host data
- *
- * @sd: pointer to v4l2_subdev
- * @p: pointer to the private data to be stored.
- */
-static inline void v4l2_set_subdev_hostdata(struct v4l2_subdev *sd, void *p)
-{
- sd->host_priv = p;
-}
-
-/**
- * v4l2_get_subdev_hostdata - Gets V4L2 dev private data
- *
- * @sd: pointer to v4l2_subdev
- *
- * Returns the pointer to the private host data to be stored.
- */
-static inline void *v4l2_get_subdev_hostdata(const struct v4l2_subdev *sd)
-{
- return sd->host_priv;
-}
-
-#ifdef CONFIG_MEDIA_CONTROLLER
-
/**
* v4l2_subdev_link_validate_default - validates a media link
*
--
2.18.0
[PATCH v2 0/7] TVP5150 fixes and new features
Hi,
this is my v2 with the integrated reviews from my v1 [1]. Since Mauro
applied the most patches from my v1 to his experimental/tvp5150-3
branch [2], this series only contains those which aren't applied.
Patches I changed contain a changelog, so I will omit these here.
Patch ('[media] tvp5150: add FORMAT_TRY support for get/set selection
handlers') throws a compile error. Therefore I added two v4l2-subdev.h
patches which should fix the error in a common way.
Patch ('[media] tvp5150: add s_power callback') is new too. I forget
them in my v1. This patch address the interrupt enable/disable handling.
Now it is possible to pause streaming and keep the interrupts on.
The changes I made in the ('[media] tvp5150: add input source selection
of_graph support') patch are based on the the RFC [3] and discussion [4].
I dropped patch ('[media] tvp5150: Change default input source selection
behaviour') since the default input source selectopn is setup during the
.registered() callback now.
I've tested this series on a customer dt-based board. Unfortunately I
haven't a device which use the em28xx driver. So other tester a welcome :)
[1] https://www.spinics.net/lists/devicetree/msg236650.html
[2] https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150-3
[3] https://www.spinics.net/lists/devicetree/msg243181.html
[4] https://www.spinics.net/lists/devicetree/msg243840.html
Regards,
Marco
Marco Felsch (6):
[media] tvp5150: add input source selection of_graph support
[media] dt-bindings: tvp5150: Add input port connectors DT bindings
[media] v4l2-subdev: add stubs for v4l2_subdev_get_try_*
[media] v4l2-subdev: fix v4l2_subdev_get_try_* dependency
[media] tvp5150: add FORMAT_TRY support for get/set selection handlers
[media] tvp5150: add s_power callback
Michael Tretter (1):
[media] tvp5150: initialize subdev before parsing device tree
.../devicetree/bindings/media/i2c/tvp5150.txt | 191 +-
drivers/media/i2c/tvp5150.c | 611 +++---
include/media/v4l2-subdev.h | 111 ++--
3 files changed, 776 insertions(+), 137 deletions(-)
--
2.18.0
[PATCH v2 3/7] [media] v4l2-subdev: add stubs for v4l2_subdev_get_try_*
In case of missing CONFIG_VIDEO_V4L2_SUBDEV_API those helpers aren't
available. So each driver have to add ifdefs around those helpers or
add the CONFIG_VIDEO_V4L2_SUBDEV_API as dependcy.
Make these helpers available in case of CONFIG_VIDEO_V4L2_SUBDEV_API
isn't set to avoid ifdefs. This approach is less error prone too.
Signed-off-by: Marco Felsch
---
include/media/v4l2-subdev.h | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 9102d6ca566e..ce48f1fcf295 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -912,8 +912,6 @@ struct v4l2_subdev_fh {
#define to_v4l2_subdev_fh(fh) \
container_of(fh, struct v4l2_subdev_fh, vfh)
-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-
/**
* v4l2_subdev_get_try_format - ancillary routine to call
* v4l2_subdev_pad_config->try_fmt
@@ -927,9 +925,13 @@ static inline struct v4l2_mbus_framefmt
struct v4l2_subdev_pad_config *cfg,
unsigned int pad)
{
+#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
return [pad].try_fmt;
+#else
+ return NULL;
+#endif
}
/**
@@ -945,9 +947,13 @@ static inline struct v4l2_rect
struct v4l2_subdev_pad_config *cfg,
unsigned int pad)
{
+#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
return [pad].try_crop;
+#else
+ return NULL;
+#endif
}
/**
@@ -963,11 +969,14 @@ static inline struct v4l2_rect
struct v4l2_subdev_pad_config *cfg,
unsigned int pad)
{
+#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
if (WARN_ON(pad >= sd->entity.num_pads))
pad = 0;
return [pad].try_compose;
-}
+#else
+ return NULL;
#endif
+}
extern const struct v4l2_file_operations v4l2_subdev_fops;
--
2.18.0
[PATCH v2 5/7] [media] tvp5150: add FORMAT_TRY support for get/set selection handlers
Since commit 10d5509c8d50 ("[media] v4l2: remove g/s_crop from video ops")
the 'which' field for set/get_selection must be FORMAT_ACTIVE. There is
no way to try different selections. The patch adds a helper function to
select the correct selection memory space (sub-device file handle or
driver state) which will be set/returned.
The TVP5150 AVID will be updated if the 'which' field is FORMAT_ACTIVE
and the requested selection rectangle differs from the already set one.
Signed-off-by: Marco Felsch
---
drivers/media/i2c/tvp5150.c | 109 +---
1 file changed, 75 insertions(+), 34 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index a9cd79cbd7b0..825dcf3a9d83 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "tvp5150_reg.h"
@@ -1007,20 +1008,40 @@ static void tvp5150_set_default(v4l2_std_id std, struct
v4l2_rect *crop)
crop->height = TVP5150_V_MAX_OTHERS;
}
+static struct v4l2_rect *
+__tvp5150_get_pad_crop(struct tvp5150 *decoder,
+ struct v4l2_subdev_pad_config *cfg, unsigned int pad,
+ enum v4l2_subdev_format_whence which)
+{
+ switch (which) {
+ case V4L2_SUBDEV_FORMAT_TRY:
+ return v4l2_subdev_get_try_crop(>sd, cfg, pad);
+ case V4L2_SUBDEV_FORMAT_ACTIVE:
+ return >rect;
+ default:
+ return NULL;
+ }
+}
+
static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
{
struct v4l2_mbus_framefmt *f;
+ struct v4l2_rect *__crop;
struct tvp5150 *decoder = to_tvp5150(sd);
if (!format || (format->pad != TVP5150_PAD_VID_OUT))
return -EINVAL;
f = >format;
+ __crop = __tvp5150_get_pad_crop(decoder, cfg, format->pad,
+ format->which);
+ if (!__crop)
+ return -EINVAL;
- f->width = decoder->rect.width;
- f->height = decoder->rect.height / 2;
+ f->width = __crop->width;
+ f->height = __crop->height / 2;
f->code = TVP5150_MBUS_FMT;
f->field = TVP5150_FIELD;
@@ -1031,17 +1052,51 @@ static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
return 0;
}
+unsigned int tvp5150_get_hmax(struct v4l2_subdev *sd)
+{
+ struct tvp5150 *decoder = to_tvp5150(sd);
+ v4l2_std_id std;
+
+ /* Calculate height based on current standard */
+ if (decoder->norm == V4L2_STD_ALL)
+ std = tvp5150_read_std(sd);
+ else
+ std = decoder->norm;
+
+ return (std & V4L2_STD_525_60) ?
+ TVP5150_V_MAX_525_60 : TVP5150_V_MAX_OTHERS;
+}
+
+static inline void
+__tvp5150_set_selection(struct v4l2_subdev *sd, struct v4l2_rect rect)
+{
+ struct tvp5150 *decoder = to_tvp5150(sd);
+ unsigned int hmax = tvp5150_get_hmax(sd);
+
+ regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_START, rect.top);
+ regmap_write(decoder->regmap, TVP5150_VERT_BLANKING_STOP,
+rect.top + rect.height - hmax);
+ regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_MSB,
+rect.left >> TVP5150_CROP_SHIFT);
+ regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_ST_LSB,
+rect.left | (1 << TVP5150_CROP_SHIFT));
+ regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_MSB,
+(rect.left + rect.width - TVP5150_MAX_CROP_LEFT) >>
+TVP5150_CROP_SHIFT);
+ regmap_write(decoder->regmap, TVP5150_ACT_VD_CROP_STP_LSB,
+rect.left + rect.width - TVP5150_MAX_CROP_LEFT);
+}
+
static int tvp5150_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_selection *sel)
{
struct tvp5150 *decoder = to_tvp5150(sd);
struct v4l2_rect rect = sel->r;
- v4l2_std_id std;
- int hmax;
+ struct v4l2_rect *__crop;
+ unsigned int hmax;
- if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
- sel->target != V4L2_SEL_TGT_CROP)
+ if (sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
dev_dbg_lvl(sd->dev, 1, debug, "%s left=%d, top=%d, width=%d,
height=%d\n",
@@ -1050,17 +1105,7 @@ static int tvp5150_set_selection(struct v4l2_subdev *sd,
/* tvp5150 has some special limits */
rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT);
rect.top = clamp(rect.top, 0, TVP5150_MAX_CROP_TOP);
-
- /* Calculate height based on current standard */
- if (decoder->norm == V4L2_STD_ALL)
- std = tvp5150_read_std(sd);
- else
- std = decoder->norm;
-
- if (std &
[PATCH v2 1/7] [media] tvp5150: add input source selection of_graph support
This patch adds the of_graph support to describe the tvp connections.
Physical the TVP5150 has three ports: AIP1A, AIP1B and YOUT. As result
of discussion [1],[2] the device-tree maps these ports 1:1. The svideo
connector must be conneted to port@0/endpoint@1, look at the Documentation
for more information. Since the TVP5150 is a converter the device-tree
must contain at least 1-input and 1-output port. The mc-connectors and
mc-links are only created if the device-tree contains the corresponding
connector nodes. If more than one connector is available the
media_entity_operations.link_setup() callback ensures that only one
connector is active.
[1] https://www.spinics.net/lists/linux-media/msg138545.html
[2] https://www.spinics.net/lists/linux-media/msg138546.html
Signed-off-by: Marco Felsch
---
Changelog:
v2:
- adapt commit message
- unify ifdef switches
- rename tvp5150_valid_input -> tvp5150_of_valid_input, to be more precise
- mc: use 2-input and 1-output pad
- mc: link svideo connector to both input pads
- mc: enable/disable svideo links in one go
- mc: change link_setup() behaviour, switch the input src don't require a
explicite disable before.
- mc: rename 'local' media_pad param to tvp5150_pad to avoid confusion
- mc: enable link to the first available connector and set the
corresponding tvp5150 input src per default during registered()
- mc/of: factor out oftree connector allocation
- of: drop svideo dt port
- of: move svideo connector to port@0/endpoint@1
- of: require at least 1-in and 1-out endpoint
---
drivers/media/i2c/tvp5150.c | 478 +---
1 file changed, 441 insertions(+), 37 deletions(-)
diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index b5d44c25d1da..a9cd79cbd7b0 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -43,16 +43,39 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
#define dprintk0(__dev, __arg...) dev_dbg_lvl(__dev, 0, 0, __arg)
+#if defined(CONFIG_MEDIA_CONTROLLER)
enum tvp5150_pads {
- TVP5150_PAD_IF_INPUT,
- TVP5150_PAD_VID_OUT,
- TVP5150_NUM_PADS
+ TVP5150_PAD_AIP1A = TVP5150_COMPOSITE0,
+ TVP5150_PAD_AIP1B,
+ TVP5150_PAD_VID_OUT,
+ TVP5150_NUM_PADS
+};
+
+enum tvp5150_pads_state {
+ TVP5150_PAD_INACTIVE,
+ TVP5150_PAD_ACTIVE_COMPOSITE,
+ TVP5150_PAD_ACTIVE_SVIDEO,
+};
+
+struct tvp5150_connector {
+ struct media_entity ent;
+ struct media_pad pad;
+ unsigned int port_num;
+ bool is_svideo;
};
+#endif
struct tvp5150 {
struct v4l2_subdev sd;
-#ifdef CONFIG_MEDIA_CONTROLLER
+ /* additional additional endpoint for the svideo connector */
+ struct device_node *endpoints[TVP5150_NUM_PADS + 1];
+ unsigned int endpoints_num;
+#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_pad pads[TVP5150_NUM_PADS];
+ int pads_state[TVP5150_NUM_PADS];
+ struct tvp5150_connector *connectors;
+ int connectors_num;
+ bool modify_second_link;
#endif
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;
@@ -1168,6 +1191,160 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev
*sd,
return 0;
}
+/
+ * Media entity ops
+ /
+#if defined(CONFIG_MEDIA_CONTROLLER)
+static int tvp5150_active_pad_idx(struct tvp5150 *decoder)
+{
+ int *pad_state = >pads_state[0];
+ int i, idx = -1;
+
+ for (i = 0; i < TVP5150_NUM_PADS - 1; i++) {
+ if ((pad_state[i] == TVP5150_PAD_ACTIVE_COMPOSITE) ||
+ (pad_state[i] == TVP5150_PAD_ACTIVE_SVIDEO)) {
+ idx = i;
+ break;
+ }
+ }
+
+ return idx;
+}
+
+static int tvp5150_active_svideo_links(struct tvp5150 *decoder)
+{
+ int *pad_state = >pads_state[0];
+ int i, links = 0;
+
+ for (i = 0; i < TVP5150_NUM_PADS - 1; i++)
+ if (pad_state[i] == TVP5150_PAD_ACTIVE_SVIDEO)
+ links++;
+
+ return links;
+}
+
+static int tvp5150_modify_link(struct tvp5150 *decoder, unsigned int pad_idx,
+ struct media_pad *pad, int flags)
+{
+ struct media_pad *src_pad;
+ struct media_link *link;
+
+ if (pad)
+ src_pad = pad;
+ else
+ src_pad = media_entity_remote_pad(>pads[pad_idx]);
+
+ if (!src_pad)
+ return -1;
+
+ link = media_entity_find_link(src_pad,
+ >pads[pad_idx]);
+
+ /*
+* Don't use locked version, since we are running already within the
+* media_entity_setup_link() context.
+*/
+ return __media_entity_setup_link(link, flags);
+
+}
+
+static int tvp5150_s_routing(struct v4l2_subdev *sd, u32
[PATCH v2 2/7] [media] dt-bindings: tvp5150: Add input port connectors DT bindings
The TVP5150/1 decoders support different video input sources to their
AIP1A/B pins.
Possible configurations are as follows:
- Analog Composite signal connected to AIP1A.
- Analog Composite signal connected to AIP1B.
- Analog S-Video Y (luminance) and C (chrominance)
signals connected to AIP1A and AIP1B respectively.
This patch extends the device tree bindings documentation to describe
how the input connectors for these devices should be defined in a DT.
Signed-off-by: Marco Felsch
---
Changelog:
v2:
- adapt port layout in accordance with
https://www.spinics.net/lists/linux-media/msg138546.html with the
svideo-connector deviation (use only one endpoint)
---
.../devicetree/bindings/media/i2c/tvp5150.txt | 191 +-
1 file changed, 185 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
index 8c0fc1a26bf0..d647d671f14a 100644
--- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
+++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
@@ -12,11 +12,31 @@ Optional Properties:
- pdn-gpios: phandle for the GPIO connected to the PDN pin, if any.
- reset-gpios: phandle for the GPIO connected to the RESETB pin, if any.
-The device node must contain one 'port' child node for its digital output
-video port, in accordance with the video interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
+The device node must contain one 'port' child node per device physical input
+and output port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
+are numbered as follows
-Required Endpoint Properties for parallel synchronization:
+ Name TypePort
+ --
+ AIP1A sink0
+ AIP1B sink1
+ Y-OUT src 2
+
+The device node must contain at least one sink port and the src port. Each
input
+port must be linked to an endpoint defined in
+Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt.
The
+port/connector layout is as follows
+
+tvp-5150 port@0 (AIP1A)
+ endpoint@0 ---> Comp0-Con port
+ endpoint@1 ---> Svideo-Con port
+tvp-5150 port@1 (AIP1B)
+ endpoint ---> Comp1-Con port
+tvp-5150 port@2
+ endpoint (video bitstream output at YOUT[0-7] parallel bus)
+
+Required Endpoint Properties for parallel synchronization on output port:
- hsync-active: active state of the HSYNC signal. Must be <1> (HIGH).
- vsync-active: active state of the VSYNC signal. Must be <1> (HIGH).
@@ -26,7 +46,140 @@ Required Endpoint Properties for parallel synchronization:
If none of hsync-active, vsync-active and field-even-active is specified,
the endpoint is assumed to use embedded BT.656 synchronization.
-Example:
+Examples:
+
+One Input:
+
+connector {
+ compatible = "composite-video-connector";
+ label = "Composite0";
+
+ port {
+ composite0_to_tvp5150: endpoint {
+ remote-endpoint = <_to_composite0>;
+ };
+ };
+};
+
+ {
+ ...
+ tvp5150@5c {
+ compatible = "ti,tvp5150";
+ reg = <0x5c>;
+ pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
+ reset-gpios = < 7 GPIO_ACTIVE_LOW>;
+
+ port@0 {
+ reg = <0>;
+
+ tvp5150_to_composite0: endpoint {
+ remote-endpoint = <_to_tvp5150>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ tvp5150_1: endpoint {
+ remote-endpoint = <_ep>;
+ };
+ };
+ };
+};
+
+Two Inputs:
+
+comp_connector_1 {
+ compatible = "composite-video-connector";
+ label = "Composite1";
+
+ port {
+ composite1_to_tvp5150: endpoint {
+ remote-endpoint = <_to_composite1>;
+ };
+ };
+};
+
+svid_connector {
+ compatible = "svideo-connector";
+ label = "S-Video";
+
+ port {
+ svideo_to_tvp5150: endpoint {
+ remote-endpoint = <_to_svideo>;
+ };
+ };
+};
+
+ {
+ ...
+ tvp5150@5c {
+ compatible = "ti,tvp5150";
+ reg = <0x5c>;
+ pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
+ reset-gpios = < 7 GPIO_ACTIVE_LOW>;
+
+port@0 {
+reg = <0>;
+
+tvp5150_to_svideo: endpoint@1 {
+reg = <1>;
+remote-endpoint = <_to_tvp5150>;
+};
+};
+
+
[PATCH v2] media: ov5640: do not change mode if format or frame interval is unchanged
Save load of mode registers array when V4L2 client sets a format or a
frame interval which selects the same mode than the current one.
Signed-off-by: Hugues Fruchet
---
Version 2:
- Fix fuzzy image because of JPEG default format not being
changed according to new format selected, fix this.
- Init sequence initialises format to YUV422 UYVY but
sensor->fmt initial value was set to JPEG, fix this.
drivers/media/i2c/ov5640.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..2ddd86d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -223,6 +223,7 @@ struct ov5640_dev {
int power_count;
struct v4l2_mbus_framefmt fmt;
+ bool pending_fmt_change;
const struct ov5640_mode_info *current_mode;
enum ov5640_frame_rate current_fr;
@@ -255,7 +256,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct
v4l2_ctrl *ctrl)
* should be identified and removed to speed register load time
* over i2c.
*/
-
+/* YUV422 UYVY VGA@30fps */
static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
@@ -1966,9 +1967,14 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
- sensor->current_mode = new_mode;
- sensor->fmt = *mbus_fmt;
- sensor->pending_mode_change = true;
+ if (new_mode != sensor->current_mode) {
+ 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;
+ }
out:
mutex_unlock(>lock);
return ret;
@@ -2508,8 +2514,10 @@ static int ov5640_s_frame_interval(struct v4l2_subdev
*sd,
goto out;
}
- sensor->current_mode = mode;
- sensor->pending_mode_change = true;
+ if (mode != sensor->current_mode) {
+ sensor->current_mode = mode;
+ sensor->pending_mode_change = true;
+ }
out:
mutex_unlock(>lock);
return ret;
@@ -2540,10 +2548,13 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int
enable)
ret = ov5640_set_mode(sensor, sensor->current_mode);
if (ret)
goto out;
+ }
+ if (enable && sensor->pending_fmt_change) {
ret = ov5640_set_framefmt(sensor, >fmt);
if (ret)
goto out;
+ sensor->pending_fmt_change = false;
}
if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
@@ -2638,9 +2649,14 @@ static int ov5640_probe(struct i2c_client *client,
return -ENOMEM;
sensor->i2c_client = client;
+
+ /*
+* default init sequence initialize sensor to
+* YUV422 UYVY VGA@30fps
+*/
fmt = >fmt;
- fmt->code = ov5640_formats[0].code;
- fmt->colorspace = ov5640_formats[0].colorspace;
+ fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
+ fmt->colorspace = V4L2_COLORSPACE_SRGB;
fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
@@ -2652,7 +2668,6 @@ static int ov5640_probe(struct i2c_client *client,
sensor->current_fr = OV5640_30_FPS;
sensor->current_mode =
_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
- sensor->pending_mode_change = true;
sensor->ae_target = 52;
--
2.7.4
Re: [PATCH v3 2/2] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx
On Fri, Jul 13, 2018 at 05:38:25PM +0300, Ivaylo Dimitrov wrote: > Hi, > > On 13.07.2018 15:22, Sean Young wrote: > > The ir-rx51 is a pwm-based TX driver specific to the N900. This can be > > handled entirely by the generic pwm-ir-tx driver. > > > > Note that the suspend code in the ir-rx51 driver is unnecessary, since > > during transmit, the process is not in interruptable sleep. The process > > is not put to sleep until the transmit completes. > > > > Compile tested only. > > > > I would like to see this being tested on a real HW, however I am on a > holiday for the next week so won't be able to test till I am back. > > @Pali - do you have n900 with fremantle, upstream kernel and pierogi to test > pwm-ir-tx on it? It would be nice to have this verified on real hardware, if possible. If not, I would like to merge this anyway. If there are any problems we can always later patch any problem in pwm-ir-tx for the n900. Sean
Re: [PATCH 00/21] V4L2 fwnode rework; support for default configuration
Hi Jacopo, On Fri, Aug 10, 2018 at 12:38:57PM +0200, jacopo mondi wrote: > Hi Sakari, >thanks for this nice rework > > On Mon, Jul 23, 2018 at 04:46:45PM +0300, Sakari Ailus wrote: > > Hello everyone, > > > > I've long thought the V4L2 fwnode framework requires some work (it's buggy > > and it does not adequately serve common needs). This set should address in > > particular these matters: > > > > - Most devices support a particular media bus type but the V4L2 fwnode > > framework was not able to use such information, but instead tried to > > guess the bus type with varying levels of success while drivers > > generally ignored the results. This patchset makes that possible --- > > setting a bus type enables parsing configuration for only that bus. > > Failing that check results in returning -ENXIO to be returned. > > > > - Support specifying default configuration. If the endpoint has no > > configuration, the defaults set by the driver (as documented in DT > > bindings) will prevail. Any available configuration will still be read > > from the endpoint as one could expect. A common use case for this is > > e.g. the number of CSI-2 lanes. Few devices support lane mapping, and > > default 1:1 mapping is provided in absence of a valid default or > > configuration read OF. > > > > - Debugging information is greatly improved. > > > > - Recognition of the differences between CSI-2 D-PHY and C-PHY. All > > currently supported hardware (or at least drivers) is D-PHY only, so > > this change is still easy. > > > > The smiapp driver is converted to use the new functionality. This patchset > > does not address remaining issues such as supporting setting defaults for > > e.g. bridge drivers with multiple ports, but with Steve Longerbeam's > > patchset we're much closer with that goal. I've rebased this set on top of > > Steve's. Albeit the two deal with the same files, there were only a few > > trivial conflicts. > > > > Note that I've only tested parsing endpoints for the CSI-2 bus (no > > parallel IF hardware). Testing in general would be beneficial: the code > > flows are rather convoluted and different hardware tends to excercise > > different flows while the use of the use of defaults has a similar > > effect. > > > > I have tested on a parallel device, and so far it's all good. I would > like to test default settings next, and see how they behave. Thanks a lot! > > > Comments are welcome. > > > > In the meantine, looking at your (anticipated) v2, and in particular > to this commit > https://git.linuxtv.org/sailus/media_tree.git/commit/?h=v4l2-fwnode-next=077d73a3e1b66f9fbb1227245b1332cc1c7871d4 > I'm wondering if introducing an API to query the bus type from the > firmware wouldn't be beneficial for bridge drivers (and possibly for > sensor drivers too). The current API are basically gives you two options: 1. Try with a given bus type, in which case you'll be able to set default parameters as well. This is preferred as it allows setting defaults. 2. Let the fwnode framework decide. Adding one more API function to tell the bus type to the driver wouldn't change much. To be useful, one of the inputs would need to be a list of possible bus types, and the fwnode framework would need to parse the properties once again. The way I see it is that it'd be likely most practical to simply try: for hardware that supports more than one bus type, the bus-type property is quasi mandatory. If there's an error, i.e. -ENXIO, the driver simply tries a differen bus type --- with default parameters specific to that bus. Usually there are just two possibilities, so there's not that many to choose from anyway. > > I see in that commit most drivers will set the bus type to 0 and rely > on autoguessing, which is fine, but I'm thinking at peripheral that Yes, the patchset at least currently does not try to improve those drivers but just to maintain the functionality unchanged. There are quite few changes already in the patchset and I'm trying to avoid regressions. > can support two different media busses (eg. Parallel and BT.656) or > sensor that can work with parallel or CSI-2 interface, and in that > case would you consider something like: > > static int bridge_driver_parse_of(struct device_node *np): > struct v4l2_fwnode_endpoint bus_cfg ; > > bus_type = v4l2_fwnode_get_bus_type(fwnode_handle(np); > if (bus_type != V4L2_MBUS_PARALLEL && > bus_type != V4L2_MBUS_BT656) > return -ENXIO; Very few drivers actually check the bus type currently, and then proceed with accessing parameters of the bus they support --- whether or not it was parsed by the V4L2 fwnode framework (or not). > > bus_cfg.bus_type = bus_type; > v4l2_fwnode_endpoint_parse(of_fwnode_handle(np), _cfg); Here, you'd set the default parameters for that bus. > > Adding a function to retrieve the bus type specified in firmware would > make easier for drivers to
