cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Sep 12 04:00:17 CEST 2018 media-tree git hash:cc1e6315e83db0e517dd9279050b88adc83a7eba media_build git hash: ed1d887e2c18299383c7258615130197c8ce4946 v4l-utils git hash: d26e4941419b05fcb2b6708ee32aef367c2ec4af edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.17.0-3-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: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-i686: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.101-i686: ERRORS linux-3.0.101-x86_64: ERRORS linux-3.1.10-i686: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.102-i686: ERRORS linux-3.2.102-x86_64: ERRORS linux-3.3.8-i686: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.113-i686: ERRORS linux-3.4.113-x86_64: ERRORS linux-3.5.7-i686: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-i686: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.10-i686: ERRORS linux-3.7.10-x86_64: ERRORS linux-3.8.13-i686: ERRORS linux-3.8.13-x86_64: ERRORS linux-3.9.11-i686: ERRORS linux-3.9.11-x86_64: ERRORS linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.57-i686: ERRORS linux-3.16.57-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.119-i686: WARNINGS linux-3.18.119-x86_64: WARNINGS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.52-i686: ERRORS linux-4.1.52-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.152-i686: WARNINGS linux-4.4.152-x86_64: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.10-i686: WARNINGS linux-4.7.10-x86_64: WARNINGS linux-4.8.17-i686: WARNINGS linux-4.8.17-x86_64: WARNINGS linux-4.9.124-i686: OK linux-4.9.124-x86_64: OK linux-4.10.17-i686: WARNINGS linux-4.10.17-x86_64: WARNINGS linux-4.11.12-i686: WARNINGS linux-4.11.12-x86_64: WARNINGS linux-4.12.14-i686: WARNINGS linux-4.12.14-x86_64: WARNINGS linux-4.13.16-i686: WARNINGS linux-4.13.16-x86_64: WARNINGS linux-4.14.67-i686: OK linux-4.14.67-x86_64: OK linux-4.15.18-i686: WARNINGS linux-4.15.18-x86_64: WARNINGS linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.5-i686: OK linux-4.18.5-x86_64: OK linux-4.19-rc1-i686: OK linux-4.19-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Logs weren't copied as they are too large (5404 kB) The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Inquiry
Hello, This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia. We are glad to know about your company from the web and we are interested in your products. Could you kindly send us your Latest catalog and price list for our trial order. Best Regards, Daniel Murray Purchasing Manager
Re: Fund
Hello, My name is ms. Reem Al-Hashimi. The UAE minister of state for international cooparation. I got your contact from a certain email database from your country while i was looking for someone to handle a huge financial transaction for me in confidence. Can you receive and invest on behalf of my only son. Please reply to reemhashimy...@gmail.com, for more details if you are interested. Regards, Ms. Reem Al-Hashimy
[PATCH v3 1/2] media: v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE
add V4L2_FRMIVAL_TYPE_CONTINUOUS and V4L2_FRMIVAL_TYPE_STEPWISE for
subdev's frame intervals in addition to implicit existing
V4L2_FRMIVAL_TYPE_DISCRETE type. This needs three new fields in the
v4l2_subdev_frame_interval_enum struct :
- type
- max_interval
- step_interval
A helper function 'v4l2_fill_frmivalenum_from_subdev' is also added.
Subdevs must fill the 'type' field. If they do not, the default
value (0) is used which is equal to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE.
if type is set to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE, or left untouched,
only the 'interval' field must be filled, just as before.
If type is set to V4L2_FRMIVAL_TYPE_CONTINUOUS, 'interval' must be set
to the minimum frame interval (highest framerate), and 'max_interval'
to the maximum frame interval.
If type is set to V4L2_FRMIVAL_TYPE_STEPWISE, 'step_interval' must be
set to the step between available intervals, in addition to 'interval'
and 'max_interval' which must be set as for V4L2_FRMIVAL_TYPE_CONTINUOUS
Old users which do not check the 'type' field will get the minimum frame
interval (highest framrate) just like before.
Callers who intend to check the 'type' field should zero it themselves,
in case an old subdev driver does not do zero it.
When filled correctly by the sensor driver, the new fields must be
used as follows by the caller :
struct v4l2_frmivalenum * fival;
struct v4l2_subdev_frame_interval_enum fie;
if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete = fie.interval;
} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
fival->stepwise.min = fie.interval;
fival->stepwise.max = fie.max_interval;
} else {
fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
fival->stepwise.min = fie.interval;
fival->stepwise.max = fie.max_interval;
fival->stepwise.step = fie.step_interval;
}
Kernel users should use the new 'v4l2_fill_frmivalenum_from_subdev'
helper function.
Signed-off-by: Philippe De Muyter
---
.../uapi/v4l/vidioc-subdev-enum-frame-interval.rst | 69 +-
drivers/media/v4l2-core/v4l2-common.c | 33 +++
include/media/v4l2-common.h| 12
include/uapi/linux/v4l2-subdev.h | 22 ++-
4 files changed, 133 insertions(+), 3 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
index 1bfe386..e14fa14 100644
--- a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
+++ b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
@@ -51,6 +51,41 @@ EINVAL error code if one of the input fields is invalid. All
frame
intervals are enumerable by beginning at index zero and incrementing by
one until ``EINVAL`` is returned.
+If the sub-device can work only with a fixed set of frame intervals, then
+the driver must enumerate them with increasing indexes, by setting the
+``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE`` and only filling
+the ``interval`` field . If the sub-device can work with a continuous
+range of frame intervals, then the driver must only return success for
+index 0, set the ``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS``,
+fill ``interval`` with the minimum interval and ``max_interval`` with
+the maximum interval. If it is worth mentioning the step in the
+continuous interval, the driver must set the ``type`` field to
+``V4L2_SUBDEV_FRMIVAL_TYPE_STEPWISE`` and fill also the ``step_interval``
+field with the step between the possible intervals.
+
+Callers are expected to use the returned information as follows:
+
+.. code-block:: c
+
+struct v4l2_frmivalenum *fival;
+struct v4l2_subdev_frame_interval_enum fie;
+
+if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
+fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+fival->discrete = fie.interval;
+} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
+fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+fival->stepwise.min = fie.interval;
+fival->stepwise.max = fie.max_interval;
+} else {
+fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
+fival->stepwise.min = fie.interval;
+fival->stepwise.max = fie.max_interval;
+fival->stepwise.step = fie.step_interval;
+}
+
+.. code-block:: c
+
Available frame intervals may depend on the current 'try' formats at
other pads of the sub-device, as well as on the current active links.
See :ref:`VIDIOC_SUBDEV_G_FMT` for more
@@ -93,11 +128,43 @@ multiple pads of the same sub-device is not defined.
[PATCH v3 2/2] media: imx: capture: use 'v4l2_fill_frmivalenum_from_subdev'
---
drivers/staging/media/imx/imx-media-capture.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index 256039c..688dd7a 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -126,28 +126,12 @@ static int capture_enum_frameintervals(struct file *file,
void *fh,
{
struct capture_priv *priv = video_drvdata(file);
const struct imx_media_pixfmt *cc;
- struct v4l2_subdev_frame_interval_enum fie = {
- .index = fival->index,
- .pad = priv->src_sd_pad,
- .width = fival->width,
- .height = fival->height,
- .which = V4L2_SUBDEV_FORMAT_ACTIVE,
- };
- int ret;
cc = imx_media_find_format(fival->pixel_format, CS_SEL_ANY, true);
if (!cc)
return -EINVAL;
- fie.code = cc->codes[0];
-
- ret = v4l2_subdev_call(priv->src_sd, pad, enum_frame_interval,
- NULL, );
- if (ret)
- return ret;
-
- fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
- fival->discrete = fie.interval;
+ return v4l2_fill_frmivalenum_from_subdev(priv->src_sd, fival,
cc->codes[0]);
return 0;
}
--
1.8.4
Editing 3
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
[PATCH v3 0/2] media: v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE
add V4L2_FRMIVAL_TYPE_CONTINUOUS and V4L2_FRMIVAL_TYPE_STEPWISE for subdev's frame intervals in addition to implicit existing V4L2_FRMIVAL_TYPE_DISCRETE type. -- v2: Add a 'type' field and a helper function, as asked by Hans v3: Fix documentation (as asked by Hans) Convert a driver to use the new helper function (asked by Hans) Initialize 'which' to V4L2_SUBDEV_FORMAT_ACTIVE in helper Philippe De Muyter (2): media: v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE media: imx: capture: use 'v4l2_fill_frmivalenum_from_subdev' .../uapi/v4l/vidioc-subdev-enum-frame-interval.rst | 69 +- drivers/media/v4l2-core/v4l2-common.c | 32 ++ drivers/staging/media/imx/imx-media-capture.c | 18 +- include/media/v4l2-common.h| 12 include/uapi/linux/v4l2-subdev.h | 22 ++- 5 files changed, 133 insertions(+), 20 deletions(-) -- 1.8.4
Editing 5
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
Editing 3
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
Editing 4
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
Editing 4
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
Re: [PATCH 1/3] media: use strscpy() instead of strlcpy()
On 09/10/2018 02:19 PM, Mauro Carvalho Chehab wrote: > The implementation of strscpy() is more robust and safer. > > That's now the recommended way to copy NUL terminated strings. > > Signed-off-by: Mauro Carvalho Chehab Acked-by: Hans Verkuil
Re: [PATCH 2/3 v2] media: replace strcpy() by strscpy()
On 09/10/2018 10:20 PM, Mauro Carvalho Chehab wrote: > The strcpy() function is being deprecated upstream. Replace > it by the safer strscpy(). > > Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Hans Verkuil Regards, Hans
Re: [PATCH 5/6] media: isp: fix a warning about a wrong struct initializer
Hi Mauro,
On Friday, 7 September 2018 14:46:34 EEST Laurent Pinchart wrote:
> Hi Mauro,
>
> As maintainers should be held to the same level of obligations as
> developers, and to avoid demotivating reviewers, could you handle comments
> you receive before pushing your own patches to your tree ? There should be
> no maintainer privilege here.
Ping ?
> On Wednesday, 8 August 2018 18:45:49 EEST Laurent Pinchart wrote:
> > Hi Mauro,
> >
> > Thank you for the patch.
> >
> > The subject line should be "media: omap3isp: ...".
> >
> > On Wednesday, 8 August 2018 17:52:55 EEST Mauro Carvalho Chehab wrote:
> >> As sparse complains:
> >>drivers/media/platform/omap3isp/isp.c:303:39: warning: Using plain
> >>integer as NULL pointer
> >>
> >> when a struct is initialized with { 0 }, actually the first
> >> element of the struct is initialized with zeros, initializing the
> >> other elements recursively. That can even generate gcc warnings
> >> on nested structs.
> >>
> > > So, instead, use the gcc-specific syntax for that (with is used
> >> broadly inside the Kernel), initializing it with {};
> >>
> >> Signed-off-by: Mauro Carvalho Chehab
> >> ---
> >>
> >> drivers/media/platform/omap3isp/isp.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/media/platform/omap3isp/isp.c
> >> b/drivers/media/platform/omap3isp/isp.c index 03354d513311..842e2235047d
> >> 100644
> >> --- a/drivers/media/platform/omap3isp/isp.c
> >> +++ b/drivers/media/platform/omap3isp/isp.c
> >> @@ -300,7 +300,7 @@ static struct clk *isp_xclk_src_get(struct
> >> of_phandle_args *clkspec, void *data)
> >> static int isp_xclk_init(struct isp_device *isp)
> >> {
> >>struct device_node *np = isp->dev->of_node;
> >> - struct clk_init_data init = { 0 };
> >> + struct clk_init_data init = {};
> >
> > How about = { NULL }; to avoid a gcc-specific syntax ?
> >
> >>unsigned int i;
> >>
> >>for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i)
--
Regards,
Laurent Pinchart
[PATCH v4] vb2: check for sane values from queue_setup
Warn and return error from the reqbufs ioctl when driver sets 0 number
of planes or 0 as plane sizes, as these values don't make any sense.
Checking this here stops obviously wrong values from propagating
further and causing various problems that are hard to trace back to
either of these values being 0.
v4: check num_planes, not num_buffers
Signed-off-by: Johan Fjeldtvedt
---
drivers/media/common/videobuf2/videobuf2-core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec7342ef0..cf2f93462a54 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
unsigned int num_buffers, allocated_buffers, num_planes = 0;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
int ret;
+ int i;
if (q->streaming) {
dprintk(1, "streaming active\n");
@@ -718,6 +719,14 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
if (ret)
return ret;
+ /* Check that driver has set sane values */
+ if (WARN_ON(!num_planes))
+ return -EINVAL;
+
+ for (i = 0; i < num_planes; i++)
+ if (WARN_ON(!plane_sizes[i]))
+ return -EINVAL;
+
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes,
plane_sizes);
--
2.17.1
Editing 1
Hi, If you have photos for editing, please send email to: hansre...@outlook.com We have 12 in house image editors and we can help you for cutting out your photos, or path the photos. Includes retouching if needed. Used for products photos or portrait photos, catalog photos. You may drop us one photo, we can send you the testing work. Thanks, Aaron Williams Email: hansre...@outlook.com
[PATCH v3 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 v3 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 3: - Change patch 5/5 by removing set_mode() orig_mode parameter as per jacopo' suggestion: https://www.spinics.net/lists/linux-media/msg139457.html 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 | 128 ++--- 1 file changed, 73 insertions(+), 55 deletions(-) -- 2.7.4
[PATCH v3 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 v3 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
Reviewed-by: Laurent Pinchart
---
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 v3 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
Reviewed-by: Laurent Pinchart
---
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 v3 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 | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c110a6a..427c2e7 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;
@@ -1619,10 +1620,10 @@ static int ov5640_set_mode_direct(struct ov5640_dev
*sensor,
return ov5640_load_regs(sensor, mode);
}
-static int ov5640_set_mode(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *orig_mode)
+static int ov5640_set_mode(struct ov5640_dev *sensor)
{
const struct ov5640_mode_info *mode = sensor->current_mode;
+ const struct ov5640_mode_info *orig_mode = sensor->last_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;
@@ -1688,6 +1689,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
return ret;
sensor->pending_mode_change = false;
+ sensor->last_mode = mode;
return 0;
@@ -1713,6 +1715,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
ret = ov5640_load_regs(sensor, _mode_init_data);
if (ret < 0)
return ret;
+ sensor->last_mode = _mode_init_data;
ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
(ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) |
@@ -1721,7 +1724,7 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
return ret;
/* now restore the last capture mode */
- ret = ov5640_set_mode(sensor, _mode_init_data);
+ ret = ov5640_set_mode(sensor);
if (ret < 0)
return ret;
@@ -2551,7 +2554,7 @@ 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);
if (ret)
goto out;
@@ -2667,6 +2670,7 @@ static int ov5640_probe(struct i2c_client *client,
sensor->current_mode =
_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
sensor->pending_mode_change = true;
+ sensor->last_mode = sensor->current_mode;
sensor->ae_target = 52;
--
2.7.4
Re: [PATCH v2] media: v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE
On 09/11/18 10:36, Philippe De Muyter wrote:
> add V4L2_FRMIVAL_TYPE_CONTINUOUS and V4L2_FRMIVAL_TYPE_STEPWISE for
> subdev's frame intervals in addition to implicit existing
> V4L2_FRMIVAL_TYPE_DISCRETE type. This needs three new fields in the
> v4l2_subdev_frame_interval_enum struct :
> - type
> - max_interval
> - step_interval
>
> A helper function 'v4l2_fill_frmivalenum_from_subdev' is also added.
>
> Subdevs must fill the 'type' field. If they do not, the default
> value (0) is used which is equal to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE.
>
> if type is set to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE, or left untouched,
> only the 'interval' field must be filled, just as before.
>
> If type is set to V4L2_FRMIVAL_TYPE_CONTINUOUS, 'interval' must be set
> to the minimum frame interval (highest framerate), and 'max_interval'
> to the maximum frame interval.
>
> If type is set to V4L2_FRMIVAL_TYPE_STEPWISE, 'step_interval' must be
> set to the step between available intervals, in addition to 'interval'
> and 'max_interval' which must be set as for V4L2_FRMIVAL_TYPE_CONTINUOUS
>
> Old users which do not check the 'type' field will get the minimum frame
> interval (highest framrate) just like before.
>
> Callers who intend to check the 'type' field should zero it themselves,
> in case an old subdev driver does not do zero it.
>
> When filled correctly by the sensor driver, the new fields must be
> used as follows by the caller :
>
>struct v4l2_frmivalenum * fival;
>struct v4l2_subdev_frame_interval_enum fie;
>
>if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
>fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
>fival->discrete = fie.interval;
>} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
>fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
>fival->stepwise.min = fie.interval;
>fival->stepwise.max = fie.max_interval;
>} else {
>fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
>fival->stepwise.min = fie.interval;
>fival->stepwise.max = fie.max_interval;
>fival->stepwise.step = fie.step_interval;
>}
>
> Kernel users should use the new 'v4l2_fill_frmivalenum_from_subdev'
> helper function.
>
> Signed-off-by: Philippe De Muyter
> ---
> v2:
> Add a 'type' field and a helper function, as asked by Hans
>
> .../uapi/v4l/vidioc-subdev-enum-frame-interval.rst | 46
> +-
> drivers/media/v4l2-core/v4l2-common.c | 32 +++
> include/media/v4l2-common.h| 12 ++
> include/uapi/linux/v4l2-subdev.h | 22 ++-
> 4 files changed, 109 insertions(+), 3 deletions(-)
>
> diff --git
> a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
> b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
> index 1bfe386..d3144b7 100644
> --- a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
> @@ -51,6 +51,44 @@ EINVAL error code if one of the input fields is invalid.
> All frame
> intervals are enumerable by beginning at index zero and incrementing by
> one until ``EINVAL`` is returned.
>
> +If the sub-device can work only at a fixed set of frame intervals,
at -> with
> +driver must enumerate them with increasing indexes, by setting the
driver -> then the driver
> +``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE`` and only filling
> +the ``interval`` field . If the sub-device can work with a continuous
> +range of frame intervals, driver must only return success for index 0,
> +set the ``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS``,
> +fill ``interval`` with the minimum interval and ``max_interval`` with
> +the maximum interval. If it is worth mentionning the step in the
mentionning -> mentioning
> +continuous interval, the driver must set the ``type`` field to
> +``V4L2_SUBDEV_FRMIVAL_TYPE_STEPWISE`` and fill also the ``step_interval``
> +field with the step between the possible intervals.
> +
> +Callers are expected to use the returned information as follows :
No space before :
> +
> +.. code-block:: c
> +
> +struct v4l2_frmivalenum * fival;
No space after *
> +struct v4l2_subdev_frame_interval_enum fie;
> +
> +if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
> +fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> +fival->discrete = fie.interval;
> +} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
> +fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> +fival->stepwise.min = fie.interval;
> +fival->stepwise.max = fie.max_interval;
> +} else {
> +fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
> +fival->stepwise.min = fie.interval;
> +
[PATCH v3] vb2: check for sane values from queue_setup
Warn and return error from the reqbufs ioctl when driver sets 0 number
of planes or 0 as plane sizes, as these values don't make any sense.
Checking this here stops obviously wrong values from propagating
further and causing various problems that are hard to trace back to
either of these values being 0.
Signed-off-by: Johan Fjeldtvedt
---
drivers/media/common/videobuf2/videobuf2-core.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec7342ef0..5741e95e6af1 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
unsigned int num_buffers, allocated_buffers, num_planes = 0;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
int ret;
+ int i;
if (q->streaming) {
dprintk(1, "streaming active\n");
@@ -718,6 +719,14 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
if (ret)
return ret;
+ /* Check that driver has set sane values */
+ if (WARN_ON(!num_buffers))
+ return -EINVAL;
+
+ for (i = 0; i < num_buffers; i++)
+ if (WARN_ON(!plane_sizes[i]))
+ return -EINVAL;
+
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes,
plane_sizes);
--
2.17.1
Re: [PATCH v2] vb2: check for sane values from queue_setup
A few more comments:
On 09/11/18 13:58, Johan Fjeldtvedt wrote:
> Warn when driver sets 0 number of planes or 0 as plane sizes.
It should also return an error, since there is no point continuing
with garbage values.
Also add *why* this is useful to the commit. I know why, since I
suggested it to you, but others don't :-)
>
> Signed-off-by: Johan Fjeldtvedt
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index f32ec7342ef0..6f903740d813 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> unsigned int num_buffers, allocated_buffers, num_planes = 0;
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> int ret;
> + int i;
>
> if (q->streaming) {
> dprintk(1, "streaming active\n");
> @@ -718,6 +719,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum
> vb2_memory memory,
> if (ret)
> return ret;
>
> + /* Check that driver has set sane values */
> + WARN_ON(!num_buffers);
Just return an error here. EINVAL is not unreasonable. So:
if (WARN_ON(!num_buffers))
return -EINVAL;
> +
> + for (i = 0; i < num_buffers; i++)
> + WARN_ON(!plane_sizes[i]);
Ditto.
> +
> /* Finally, allocate buffers and video memory */
> allocated_buffers =
> __vb2_queue_alloc(q, memory, num_buffers, num_planes,
> plane_sizes);
>
Regards,
Hans
[PATCH v2] vb2: check for sane values from queue_setup
Warn when driver sets 0 number of planes or 0 as plane sizes.
Signed-off-by: Johan Fjeldtvedt
---
drivers/media/common/videobuf2/videobuf2-core.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec7342ef0..6f903740d813 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
unsigned int num_buffers, allocated_buffers, num_planes = 0;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
int ret;
+ int i;
if (q->streaming) {
dprintk(1, "streaming active\n");
@@ -718,6 +719,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
if (ret)
return ret;
+ /* Check that driver has set sane values */
+ WARN_ON(!num_buffers);
+
+ for (i = 0; i < num_buffers; i++)
+ WARN_ON(!plane_sizes[i]);
+
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes,
plane_sizes);
--
2.17.1
Re: [PATCH] vb2: check for sane values from queue_setup
Hi Johan,
Thank you for the patch, but I have some small comments:
On 09/11/18 13:28, Johan Fjeldtvedt wrote:
> Warn when driver sets 0 number of planes or 0 as plane sizes.
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index f32ec7342ef0..d3bc94477e6b 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> unsigned int num_buffers, allocated_buffers, num_planes = 0;
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> int ret;
> + int i;
>
> if (q->streaming) {
> dprintk(1, "streaming active\n");
> @@ -718,6 +719,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum
> vb2_memory memory,
> if (ret)
> return ret;
>
> + /* Check that driver has set sane values */
> + WARN_ON(num_buffers == 0);
> +
> + for (i = 0; i < num_buffers; i++) {
Linux coding style is not to use curly brackets around single statements.
> + WARN_ON(plane_sizes[i] == 0);
There is a general preference in the kernel to use !foo instead of 'foo == 0'
You can (and should) run the patch against checkpatch:
git diff | scripts/checkpatch.pl --strict
> + }
> +
> /* Finally, allocate buffers and video memory */
> allocated_buffers =
> __vb2_queue_alloc(q, memory, num_buffers, num_planes,
> plane_sizes);
>
Regards,
Hans
[PATCH] vb2: check for sane values from queue_setup
Warn when driver sets 0 number of planes or 0 as plane sizes.
---
drivers/media/common/videobuf2/videobuf2-core.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index f32ec7342ef0..d3bc94477e6b 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
unsigned int num_buffers, allocated_buffers, num_planes = 0;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
int ret;
+ int i;
if (q->streaming) {
dprintk(1, "streaming active\n");
@@ -718,6 +719,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
if (ret)
return ret;
+ /* Check that driver has set sane values */
+ WARN_ON(num_buffers == 0);
+
+ for (i = 0; i < num_buffers; i++) {
+ WARN_ON(plane_sizes[i] == 0);
+ }
+
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes,
plane_sizes);
--
2.17.1
Re: [PATCH v6 0/3] Add support for MPEG-2 in DELTA video decoder
Hi Hans, On 09/09/2018 11:38 AM, Hans Verkuil wrote: > Hi Hugues, > > On 04/28/2017 03:25 PM, Hugues Fruchet wrote: >> The patchset implements the MPEG-2 part of V4L2 unified low-level decoder >> API RFC [0] needed by stateless video decoders, ie decoders which requires >> specific parsing metadata in addition to video bitstream chunk in order >> to complete decoding. >> A reference implementation using STMicroelectronics DELTA video decoder >> is provided as initial support in this patchset. >> In addition to this patchset, a libv4l plugin is also provided which convert >> MPEG-2 video bitstream to "parsed MPEG-2" by parsing the user video bitstream >> and filling accordingly the dedicated controls, doing so user code remains >> unchanged whatever decoder is: stateless or not. >> >> The first patch implements the MPEG-2 part of V4L2 unified low-level decoder >> API RFC [0]. A dedicated "parsed MPEG-2" pixel format has been introduced >> with >> its related extended controls in order that user provides both video >> bitstream >> chunk and the associated extra data resulting from this video bitstream chunk >> parsing. >> >> The second patch adds the support of "parsed" pixel format inside DELTA video >> decoder including handling of the dedicated controls and setting of parsing >> metadata required by decoder layer. >> Please note that the current implementation has a restriction regarding >> the atomicity of S_EXT_CTRL/QBUF that must be guaranteed by user. >> This restriction will be removed when V4L2 request API will be implemented >> [1]. >> Please also note the failure in v4l2-compliance in controls section, related >> to complex compound controls handling, to be discussed to find the right way >> to fix it in v4l2-compliance. >> >> The third patch adds the support of DELTA MPEG-2 stateless video decoder >> back-end. > > I've marked this (old) series as obsoleted in patchwork. The Request API > together > with the cedrus stateless MPEG decoder is about to be merged for 4.20, so it > would > be very nice indeed if you can resurrect this driver and base it on the > Request API. Many thanks to you and all contributors for this long and hard work on request API, no doubt that this is a major step towards video codecs support standardisation in Linux. Now I have switched on other activities related to camera on STM32 platform as you can see regularly on mailing list and unfortunately it will be difficult to me to resurrect this now. Anyway I hope that this implementation in kernel associated to the userspace part provided in libv4l will serve its example purpose for any other MPEG2 implementation (idea was that different soc vendor kernel driver but a single userspace implementation...). Best regards, Hugues. > > Thanks! > > Hans > >> >> >> This driver depends on: >>[PATCH v7 00/10] Add support for DELTA video decoder of >> STMicroelectronics STiH4xx SoC series >> https://patchwork.linuxtv.org/patch/39186/ >> >> References: >>[0] [RFC] V4L2 unified low-level decoder API >> https://www.spinics.net/lists/linux-media/msg107150.html >>[1] [ANN] Report of the V4L2 Request API brainstorm meeting >> https://www.spinics.net/lists/linux-media/msg106699.html >> >> === >> = history = >> === >> version 6: >>- patchset 5 review from Hans: >> - revisit 32/64 bit compat in mpeg2 controls struct (using pahole >> utility) >>to avoid padding fields introduction >>- pass latest v4l2-compliance with compound controls fixes >> - fix delta_subscribe_event() adding missing control event >>- fix warnings at documentation generation (add exceptions) >> >> version 5: >>- patchset 4 review from Hans: >> - fix 32/64 bit compat in mpeg2 controls struct (using pahole utility) >> - fix upper case at begining of words in v4l2_ctrl_get_name() >> >> version 4: >>- patchset 3 review from Nicolas Dufresne >> - one attribute per line in structure >>- fix some multilines comments >> >> version 3: >>- fix warning on parisc architecture >> >> version 2: >>- rebase on top of DELTA v7, refer to [0] >>- change VIDEO_STI_DELTA_DRIVER to default=y as per Mauro recommendations >> >> version 1: >>- Initial submission >> >> === >> = v4l2-compliance = >> === >> Below is the v4l2-compliance report, v4l2-compliance has been build from >> SHA1: >> 847bf8d62cd6b11defc1e4c3b30b68d3c66876e0 v4l2/cec-compliance, cec-follower: >> use git -C $(srcdir) rev-parse HEAD >> >> root@sti:~# v4l2-compliance -d /dev/video3 >> v4l2-compliance SHA : 847bf8d62cd6b11defc1e4c3b30b68d3c66876e0 >> >> Driver Info: >> Driver name : st-delta >> Card type : st-delta-21.1-3 >> Bus info : platform:soc:delta0 >> Driver version: 4.10.0 >> Capabilities : 0x84208000 >> Video Memory-to-Memory >> Streaming >>
[PATCH v2] media: v4l2-subdev.h: allow V4L2_FRMIVAL_TYPE_CONTINUOUS & _STEPWISE
add V4L2_FRMIVAL_TYPE_CONTINUOUS and V4L2_FRMIVAL_TYPE_STEPWISE for
subdev's frame intervals in addition to implicit existing
V4L2_FRMIVAL_TYPE_DISCRETE type. This needs three new fields in the
v4l2_subdev_frame_interval_enum struct :
- type
- max_interval
- step_interval
A helper function 'v4l2_fill_frmivalenum_from_subdev' is also added.
Subdevs must fill the 'type' field. If they do not, the default
value (0) is used which is equal to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE.
if type is set to V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE, or left untouched,
only the 'interval' field must be filled, just as before.
If type is set to V4L2_FRMIVAL_TYPE_CONTINUOUS, 'interval' must be set
to the minimum frame interval (highest framerate), and 'max_interval'
to the maximum frame interval.
If type is set to V4L2_FRMIVAL_TYPE_STEPWISE, 'step_interval' must be
set to the step between available intervals, in addition to 'interval'
and 'max_interval' which must be set as for V4L2_FRMIVAL_TYPE_CONTINUOUS
Old users which do not check the 'type' field will get the minimum frame
interval (highest framrate) just like before.
Callers who intend to check the 'type' field should zero it themselves,
in case an old subdev driver does not do zero it.
When filled correctly by the sensor driver, the new fields must be
used as follows by the caller :
struct v4l2_frmivalenum * fival;
struct v4l2_subdev_frame_interval_enum fie;
if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete = fie.interval;
} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
fival->stepwise.min = fie.interval;
fival->stepwise.max = fie.max_interval;
} else {
fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
fival->stepwise.min = fie.interval;
fival->stepwise.max = fie.max_interval;
fival->stepwise.step = fie.step_interval;
}
Kernel users should use the new 'v4l2_fill_frmivalenum_from_subdev'
helper function.
Signed-off-by: Philippe De Muyter
---
v2:
Add a 'type' field and a helper function, as asked by Hans
.../uapi/v4l/vidioc-subdev-enum-frame-interval.rst | 46 +-
drivers/media/v4l2-core/v4l2-common.c | 32 +++
include/media/v4l2-common.h| 12 ++
include/uapi/linux/v4l2-subdev.h | 22 ++-
4 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
index 1bfe386..d3144b7 100644
--- a/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
+++ b/Documentation/media/uapi/v4l/vidioc-subdev-enum-frame-interval.rst
@@ -51,6 +51,44 @@ EINVAL error code if one of the input fields is invalid. All
frame
intervals are enumerable by beginning at index zero and incrementing by
one until ``EINVAL`` is returned.
+If the sub-device can work only at a fixed set of frame intervals,
+driver must enumerate them with increasing indexes, by setting the
+``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE`` and only filling
+the ``interval`` field . If the sub-device can work with a continuous
+range of frame intervals, driver must only return success for index 0,
+set the ``type`` field to ``V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS``,
+fill ``interval`` with the minimum interval and ``max_interval`` with
+the maximum interval. If it is worth mentionning the step in the
+continuous interval, the driver must set the ``type`` field to
+``V4L2_SUBDEV_FRMIVAL_TYPE_STEPWISE`` and fill also the ``step_interval``
+field with the step between the possible intervals.
+
+Callers are expected to use the returned information as follows :
+
+.. code-block:: c
+
+struct v4l2_frmivalenum * fival;
+struct v4l2_subdev_frame_interval_enum fie;
+
+if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_DISCRETE) {
+fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+fival->discrete = fie.interval;
+} else if (fie.type == V4L2_SUBDEV_FRMIVAL_TYPE_CONTINUOUS) {
+fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+fival->stepwise.min = fie.interval;
+fival->stepwise.max = fie.max_interval;
+} else {
+fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
+fival->stepwise.min = fie.interval;
+fival->stepwise.max = fie.max_interval;
+fival->stepwise.step = fie.step_interval;
+}
+
+.. code-block:: c
+
+Kernel users may use the ``v4l2_fill_frmivalenum_from_subdev`` helper
+function instead.
+
Available frame intervals may depend on the current 'try' formats at
other pads of the sub-device, as
Re: [PATCH v2 5/5] media: ov5640: fix restore of last mode set
Hi Laurent,
On 09/10/2018 10:56 PM, Laurent Pinchart wrote:
> Hi Hugues,
>
> On Monday, 10 September 2018 18:14:45 EEST Hugues FRUCHET wrote:
>> On 09/07/2018 04:18 PM, Laurent Pinchart wrote:
>>> On Thursday, 16 August 2018 18:07:54 EEST Hugues FRUCHET wrote:
On 08/16/2018 12:10 PM, jacopo mondi wrote:
> On Mon, Aug 13, 2018 at 12:19:46PM +0200, Hugues Fruchet wrote:
>
>> 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.
>
> I actually see a different issue here...
Which problem do you have exactly, you got a VGA JPEG instead of a QVGA
YUYV ?
> The issue I see here depends on the format programmed through
> set_fmt() never being applied when using the sensor with a media
> controller equipped device (in this case an i.MX6 board) through
> capture sessions, and the not properly calculated exposure you see may
> be a consequence of this.
>
> I'll try to write down what I see, with the help of some debug output.
>
> - At probe time mode 640x460@30 is programmed:
>
> [1.651216] ov5640_probe: Initial mode with id: 2
>
> - I set the format on the sensor's pad and it gets not applied but
> marked as pending as the sensor is powered off:
> >>>
> #media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/320x240
> field:none]"
> [ 65.611983] ov5640_set_fmt: NEW mode with id: 1 - PENDING
So here sensor->current_mode is set to <1>;//QVGA
and sensor->pending_mode_change is set to true;
> - I start streaming with yavta, and the sensor receives a power on;
> this causes the 'initial' format to be re-programmed and the
> pending change to be ignored:
>
> #yavta -c10 -n4 -f YUYV -s $320x240 -F"../frame-#.yuv" /dev/video4
>
> [ 69.395018] ov5640_set_power:1805 - on
> [ 69.431342] ov5640_restore_mode:1711
> [ 69.996882] ov5640_set_mode: Apply mode with id: 0
>
> The 'ov5640_set_mode()' call from 'ov5640_restore_mode()' clears
> the sensor->pending flag, discarding the newly requested format, for
> this reason, at s_stream() time, the pending flag is not set
> anymore.
OK but before clearing sensor->pending_mode_change, set_mode() is
loading registers corresponding to sensor->current_mode:
static int ov5640_set_mode(struct ov5640_dev *sensor,
const struct ov5640_mode_info *orig_mode)
{
==>const struct ov5640_mode_info *mode = sensor->current_mode;
...
ret = ov5640_set_mode_direct(sensor, mode, exposure);
=> so mode <1> is expected to be set now, so I don't understand your
trace:
"> [ 69.996882] ov5640_set_mode: Apply mode with id: 0"
Which variable do you trace that shows "0" ?
> Are you using a media-controller system? I suspect in non-mc cases,
> the set_fmt is applied through a single power_on/power_off session, not
> causing the 'restore_mode()' issue. Is this the case for you or your
> issue is differnt?
>
> Edit:
> Mita-san tried to address the issue of the output pixel format not
> being restored when the image format was restored in
> 19ad26f9e6e1 ("media: ov5640: add missing output pixel format setting")
>
> I understand the issue he tried to fix, but shouldn't the pending
> format (if any) be applied instead of the initial one unconditionally?
This is what does the ov5640_restore_mode(), set the current mode
(sensor->current_mode), that is done through this line:
/* now restore the last capture mode */
ret = ov5640_set_mode(sensor, _mode_init_data);
=> note that the comment above is weird, in fact it is the "current"
mode that is set.
=> note also that the 2nd parameter is not the mode to be set but the
previously applied mode ! (ie loaded in ov5640 registers). This is used
to decide if we have to go to the "set_mode_exposure_calc" or
"set_mode_direct".
the ov5640_restore_mode() also set the current pixel format
(sensor->fmt), that is done through this line:
return ov5640_set_framefmt(sensor, >fmt);
==> This is what have fixed Mita-san, this line was missing previously,
leading to "mode registers" being loaded but not the "pixel format
registers".
>>>
>>> This seems overly complicated to me. Why do we have to set the mode at
>>> power on time at all, why can't we do it at stream on time only, and
>>> simplify all this logic ?
>>
>> I'm not the author of this driver, Steve do you know the
[GIT PULL FOR v4.20 (request_api branch)] Add Allwinner cedrus decoder driver
This supersedes my previous pull request for this since I inadvertently left in the dts patches, but those go through a separate subsystem. Hi Mauro, This is the cedrus Allwinner decoder driver. It is for the request_api topic branch, but it assumes that this pull request is applied first: https://patchwork.linuxtv.org/patch/51889/ The last two patches could optionally be squashed with the main driver patch: they fix COMPILE_TEST issues. I decided not to squash them and leave the choice to you. This won't fully fix the COMPILE_TEST problems, for that another patch is needed: https://lore.kernel.org/patchwork/patch/983848/ But that's going through another subsystem. Many, many thanks go to Paul for working on this, trying to keep up to date with the Request API changes at the same time. It was a pleasure working with you on this! Regards, Hans The following changes since commit 051dfd971de1317626d322581546257b748ebde1: media-request: update documentation (2018-09-04 11:34:57 +0200) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git cedrus for you to fetch changes up to e01a72b552b97e49f4d874fc5c48d3092475c423: media: cedrus: Select the sunxi SRAM driver in Kconfig (2018-09-11 09:53:39 +0200) Paul Kocialkowski (9): media: videobuf2-core: Rework and rename helper for request buffer count media: v4l: Add definitions for MPEG-2 slice format and metadata media: v4l: Add definition for the Sunxi tiled NV12 format dt-bindings: media: Document bindings for the Cedrus VPU driver media: platform: Add Cedrus VPU decoder driver media: cedrus: Fix error reporting in request validation media: cedrus: Add TODO file with tasks to complete before unstaging media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated comment media: cedrus: Select the sunxi SRAM driver in Kconfig Documentation/devicetree/bindings/media/cedrus.txt | 54 ++ Documentation/media/uapi/v4l/extended-controls.rst | 176 +++ Documentation/media/uapi/v4l/pixfmt-compressed.rst | 16 ++ Documentation/media/uapi/v4l/pixfmt-reserved.rst | 15 +- Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 14 +- Documentation/media/videodev2.h.rst.exceptions | 2 + MAINTAINERS| 7 + drivers/media/common/videobuf2/videobuf2-core.c| 18 +- drivers/media/common/videobuf2/videobuf2-v4l2.c| 2 +- drivers/media/v4l2-core/v4l2-ctrls.c | 63 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 2 + drivers/staging/media/Kconfig | 2 + drivers/staging/media/Makefile | 1 + drivers/staging/media/sunxi/Kconfig| 15 ++ drivers/staging/media/sunxi/Makefile | 1 + drivers/staging/media/sunxi/cedrus/Kconfig | 14 ++ drivers/staging/media/sunxi/cedrus/Makefile| 3 + drivers/staging/media/sunxi/cedrus/TODO| 7 + drivers/staging/media/sunxi/cedrus/cedrus.c| 431 + drivers/staging/media/sunxi/cedrus/cedrus.h| 165 + drivers/staging/media/sunxi/cedrus/cedrus_dec.c| 70 drivers/staging/media/sunxi/cedrus/cedrus_dec.h| 27 +++ drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 327 ++ drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 30 drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 237 + drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 233 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 544 + drivers/staging/media/sunxi/cedrus/cedrus_video.h | 30 include/media/v4l2-ctrls.h | 18 +- include/media/videobuf2-core.h | 4 +- include/uapi/linux/v4l2-controls.h | 65 +++ include/uapi/linux/videodev2.h | 6 + 32 files changed, 2576 insertions(+), 23 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/cedrus.txt create mode 100644 drivers/staging/media/sunxi/Kconfig create mode 100644 drivers/staging/media/sunxi/Makefile create mode 100644 drivers/staging/media/sunxi/cedrus/Kconfig create mode 100644 drivers/staging/media/sunxi/cedrus/Makefile create mode 100644 drivers/staging/media/sunxi/cedrus/TODO create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus.c create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus.h create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_dec.c create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_dec.h create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_hw.c create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_hw.h create mode
Re: [GIT PULL FOR v4.20 (request_api branch)] Add Allwinner cedrus decoder driver
On 09/11/18 09:47, Maxime Ripard wrote: > Hi Hans, > > On Mon, Sep 10, 2018 at 10:34:53AM +0200, Hans Verkuil wrote: >> This is the cedrus Allwinner decoder driver. It is for the request_api topic >> branch, but it assumes that this pull request is applied first: >> https://patchwork.linuxtv.org/patch/51889/ >> >> The last two patches could optionally be squashed with the main driver patch: >> they fix COMPILE_TEST issues. I decided not to squash them and leave the >> choice >> to you. >> >> This won't fully fix the COMPILE_TEST problems, for that another patch is >> needed: >> >> https://lore.kernel.org/patchwork/patch/983848/ >> >> But that's going through another subsystem. >> >> Many, many thanks go to Paul for working on this, trying to keep up to date >> with >> the Request API changes at the same time. It was a pleasure working with you >> on >> this! >> >> Regards, >> >> Hans >> >> The following changes since commit 051dfd971de1317626d322581546257b748ebde1: >> >> media-request: update documentation (2018-09-04 11:34:57 +0200) >> >> are available in the Git repository at: >> >> git://linuxtv.org/hverkuil/media_tree.git cedrus >> >> for you to fetch changes up to e035b190fac3735e5f9d3c96cee5afc82aa1a94d: >> >> media: cedrus: Select the sunxi SRAM driver in Kconfig (2018-09-10 >> 10:22:07 +0200) >> >> >> Paul Kocialkowski (13): >> media: videobuf2-core: Rework and rename helper for request buffer >> count >> media: v4l: Add definitions for MPEG-2 slice format and metadata >> media: v4l: Add definition for the Sunxi tiled NV12 format >> dt-bindings: media: Document bindings for the Cedrus VPU driver >> media: platform: Add Cedrus VPU decoder driver >> ARM: dts: sun5i: Add Video Engine and reserved memory nodes >> ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes >> ARM: dts: sun8i-a33: Add Video Engine and reserved memory nodes >> ARM: dts: sun8i-h3: Add Video Engine and reserved memory nodes >> media: cedrus: Fix error reporting in request validation >> media: cedrus: Add TODO file with tasks to complete before unstaging >> media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated >> comment >> media: cedrus: Select the sunxi SRAM driver in Kconfig >> >> Documentation/devicetree/bindings/media/cedrus.txt | 54 + >> Documentation/media/uapi/v4l/extended-controls.rst | 176 >> Documentation/media/uapi/v4l/pixfmt-compressed.rst | 16 ++ >> Documentation/media/uapi/v4l/pixfmt-reserved.rst | 15 +- >> Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 14 +- >> Documentation/media/videodev2.h.rst.exceptions | 2 + >> MAINTAINERS| 7 + >> arch/arm/boot/dts/sun5i.dtsi | 26 +++ >> arch/arm/boot/dts/sun7i-a20.dtsi | 26 +++ >> arch/arm/boot/dts/sun8i-a33.dtsi | 26 +++ >> arch/arm/boot/dts/sun8i-h3.dtsi| 25 +++ > > Sorry for not noticing it earlier, but we'll want to merge the > arch/arm/boot/dts/* changes through arm-soc, to reduce the merge > conflicts. > > I guess we can do it through several ways, depending on what's the > most convenient for you: > > - Drop the patches in your PR, I'll do this. Apologies, completely my mistake. Regards, Hans > - Send a revert patch as an additional patch on top of your current PR > - Or just merge the same patches in our tree and let git figure it out. > > Maxime >
Re: [GIT PULL FOR v4.20 (request_api branch)] Add Allwinner cedrus decoder driver
Hi Hans, On Mon, Sep 10, 2018 at 10:34:53AM +0200, Hans Verkuil wrote: > This is the cedrus Allwinner decoder driver. It is for the request_api topic > branch, but it assumes that this pull request is applied first: > https://patchwork.linuxtv.org/patch/51889/ > > The last two patches could optionally be squashed with the main driver patch: > they fix COMPILE_TEST issues. I decided not to squash them and leave the > choice > to you. > > This won't fully fix the COMPILE_TEST problems, for that another patch is > needed: > > https://lore.kernel.org/patchwork/patch/983848/ > > But that's going through another subsystem. > > Many, many thanks go to Paul for working on this, trying to keep up to date > with > the Request API changes at the same time. It was a pleasure working with you > on > this! > > Regards, > > Hans > > The following changes since commit 051dfd971de1317626d322581546257b748ebde1: > > media-request: update documentation (2018-09-04 11:34:57 +0200) > > are available in the Git repository at: > > git://linuxtv.org/hverkuil/media_tree.git cedrus > > for you to fetch changes up to e035b190fac3735e5f9d3c96cee5afc82aa1a94d: > > media: cedrus: Select the sunxi SRAM driver in Kconfig (2018-09-10 10:22:07 > +0200) > > > Paul Kocialkowski (13): > media: videobuf2-core: Rework and rename helper for request buffer count > media: v4l: Add definitions for MPEG-2 slice format and metadata > media: v4l: Add definition for the Sunxi tiled NV12 format > dt-bindings: media: Document bindings for the Cedrus VPU driver > media: platform: Add Cedrus VPU decoder driver > ARM: dts: sun5i: Add Video Engine and reserved memory nodes > ARM: dts: sun7i-a20: Add Video Engine and reserved memory nodes > ARM: dts: sun8i-a33: Add Video Engine and reserved memory nodes > ARM: dts: sun8i-h3: Add Video Engine and reserved memory nodes > media: cedrus: Fix error reporting in request validation > media: cedrus: Add TODO file with tasks to complete before unstaging > media: cedrus: Wrap PHYS_PFN_OFFSET with ifdef and add dedicated comment > media: cedrus: Select the sunxi SRAM driver in Kconfig > > Documentation/devicetree/bindings/media/cedrus.txt | 54 + > Documentation/media/uapi/v4l/extended-controls.rst | 176 > Documentation/media/uapi/v4l/pixfmt-compressed.rst | 16 ++ > Documentation/media/uapi/v4l/pixfmt-reserved.rst | 15 +- > Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 14 +- > Documentation/media/videodev2.h.rst.exceptions | 2 + > MAINTAINERS| 7 + > arch/arm/boot/dts/sun5i.dtsi | 26 +++ > arch/arm/boot/dts/sun7i-a20.dtsi | 26 +++ > arch/arm/boot/dts/sun8i-a33.dtsi | 26 +++ > arch/arm/boot/dts/sun8i-h3.dtsi| 25 +++ Sorry for not noticing it earlier, but we'll want to merge the arch/arm/boot/dts/* changes through arm-soc, to reduce the merge conflicts. I guess we can do it through several ways, depending on what's the most convenient for you: - Drop the patches in your PR, - Send a revert patch as an additional patch on top of your current PR - Or just merge the same patches in our tree and let git figure it out. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [PATCH v2 5/5] media: ov5640: fix restore of last mode set
Hi Jacopo,
On 08/25/2018 04:53 PM, jacopo mondi wrote:
> Hi Hugues,
> one more comment on this patch..
>
> On Mon, Aug 13, 2018 at 12:19:46PM +0200, Hugues Fruchet wrote:
>> 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;
>> +
>
> Am I wrong or with the introduction of last_mode we could drop the
> 'orig_mode' parameter (which has confused me already :/ ) from the
> set_mode() function?
>
> Just set here 'orig_mode = sensor->last_mode' and make sure last_mode
> is intialized properly at probe time...
>
> Or is there some other value in keeping the orig_mode parameter here?
>
> Thanks
> j
I'm fine with that change, will push it in v3.
>
>> 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
>>
BR Hugues.
hi
Please reply me back I have something to tell you.I am Sgt.Sherri.
Re: [PATCH 2/2] vicodec: set state->info before calling the encode/decode funcs
On 09/10/2018 05:37 PM, Ezequiel Garcia wrote:
> On Mon, 2018-09-10 at 17:00 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> state->info was NULL since I completely forgot to set state->info.
>> Oops.
>>
>> Reported-by: Ezequiel Garcia
>> Signed-off-by: Hans Verkuil
>
> For both patches:
>
> Tested-by: Ezequiel Garcia
>
> With these changes, now this gstreamer pipeline no longer
> crashes:
>
> gst-launch-1.0 -v videotestsrc num-buffers=30 !
> video/x-raw,width=1280,height=720 ! v4l2fwhtenc capture-io-mode=mmap
> output-io-mode=mmap ! v4l2fwhtdec
> capture-io-mode=mmap output-io-mode=mmap ! fakesink
>
> A few things:
>
> * You now need to mark "[PATCH] vicodec: fix sparse warning" as invalid.
I'll wait for that to be merged (it's already in a pending pull request), then
rework this patch and add your other patches for a new pull request.
> * v4l2fwhtenc/v4l2fwhtdec elements are not upstream yet.
> * Gstreamer doesn't end properly; and it seems to negotiate
> different sizes for encoded and decoded unless explicitly set.
As mentioned before, vicodec isn't fully compliant with the upcoming
codec spec, and is also missing certain features (selection support,
support for custom bytesperline values, padding, midstream resolution
changes). Patches are welcome.
If you are working on gstreamer elements for this codec, then it would
be great if you could look at making the driver compliant. I have no
plans to work on vicodec until that codec spec is fully finalized, so
you can go ahead with that if you want to.
Would be really nice, and after all, that's why I wrote vicodec!
Regards,
Hans
>
> Thanks!
>
>> drivers/media/platform/vicodec/vicodec-core.c | 11 +++
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/vicodec/vicodec-core.c
>> b/drivers/media/platform/vicodec/vicodec-core.c
>> index fdd77441a47b..5d42a8414283 100644
>> --- a/drivers/media/platform/vicodec/vicodec-core.c
>> +++ b/drivers/media/platform/vicodec/vicodec-core.c
>> @@ -176,12 +176,15 @@ static int device_process(struct vicodec_ctx *ctx,
>> }
>>
>> if (ctx->is_enc) {
>> -unsigned int size = v4l2_fwht_encode(state, p_in, p_out);
>> -
>> -vb2_set_plane_payload(_vb->vb2_buf, 0, size);
>> +state->info = q_out->info;
>> +ret = v4l2_fwht_encode(state, p_in, p_out);
>> +if (ret < 0)
>> +return ret;
>> +vb2_set_plane_payload(_vb->vb2_buf, 0, ret);
>> } else {
>> +state->info = q_cap->info;
>> ret = v4l2_fwht_decode(state, p_in, p_out);
>> -if (ret)
>> +if (ret < 0)
>> return ret;
>> vb2_set_plane_payload(_vb->vb2_buf, 0, q_cap->sizeimage);
>> }
>
