4.18 regression: dvb-usb-v2: General Protection Fault shortly after boot
I reported this on bugzilla also a few days ago, but I'm not sure if that is actually the right place to report, so copying to the mailing list... Starting with the first 4.18 RC kernel, my system experiences general protection faults leading to kernel panic shortly after the login prompt appears on most boots. Occasionally that doesn't happen and instead numerous other seemingly random stack traces are printed (bad page map, scheduling while atomic, null pointer deref, etc), but either way the system is unusable. This bug remains up through the latest mainline kernel 4.19-rc2. Booting with my USB ATSC tv tuner disconnected prevents the bug from happening. Kernel bisection between v4.17 and 4.18-rc1 shows problem is caused by: 1a0c10ed7bb1 media: dvb-usb-v2: stop using coherent memory for URBs Building both 4.18.6 and 4.19-rc2 with that commit reverted resolves the bug for me. My DVB hardware uses driver mxl111sf: Bus 002 Device 003: ID 2040:c61b Hauppauge Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x2040 Hauppauge idProduct 0xc61b bcdDevice0.00 iManufacturer 1 Hauppauge iProduct2 WinTV Aero-M Other system info: Arch Linux x86_64 Intel i7-3770 16 GB ram Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201055 Arch bug: https://bugs.archlinux.org/task/59990 Thanks, Dan Ziemba
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Sep 20 04:00:17 CEST 2018 media-tree git hash:985cdcb08a0488558d1005139596b64d73bee267 media_build git hash: 44385b9c61ecc27059a651885895c8ea09cd4179 v4l-utils git hash: 0ed116049b9365658d858d07279779685455fd70 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.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-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.119-i686: OK linux-3.18.119-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.152-i686: OK linux-4.4.152-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.124-i686: OK linux-4.9.124-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.67-i686: OK linux-4.14.67-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.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 Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
RE: [PATCH v6 06/12] intel-ipu3: css: Add support for firmware management
Hi, Tomasz,
> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Tomasz Figa
> Sent: Monday, July 2, 2018 2:05 AM
> To: Zhi, Yong
> Cc: Linux Media Mailing List ; Sakari Ailus
> ; Mani, Rajmohan
> ; Toivonen, Tuukka
> ; Hu, Jerry W ; Zheng,
> Jian Xu
> Subject: Re: [PATCH v6 06/12] intel-ipu3: css: Add support for firmware
> management
>
> Hi Yong,
>
> Continuing my review. Sorry for the delay.
>
> On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi wrote:
> >
> > Introduce functions to load and install ImgU FW blobs.
> >
> > Signed-off-by: Yong Zhi
> > ---
> > drivers/media/pci/intel/ipu3/ipu3-abi.h| 1888
>
> > drivers/media/pci/intel/ipu3/ipu3-css-fw.c | 261
> > drivers/media/pci/intel/ipu3/ipu3-css-fw.h | 198 +++
> > 3 files changed, 2347 insertions(+)
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c
> > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-abi.h
> > b/drivers/media/pci/intel/ipu3/ipu3-abi.h
> > new file mode 100644
> > index ..24102647a89e
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-abi.h
> [snip]
> > +/* SYSTEM_REQ_0_5_0_IMGHMMADR */
> > +#define IMGU_REG_SYSTEM_REQ0x18
> > +#define IMGU_SYSTEM_REQ_FREQ_MASK 0x3f
> > +#define IMGU_SYSTEM_REQ_FREQ_DIVIDER 25
> > +#define IMGU_REG_INT_STATUS0x30
> > +#define IMGU_REG_INT_ENABLE0x34
> > +#define IMGU_REG_INT_CSS_IRQ (1 << 31)
>
> BIT(31)
>
Ack.
> [snip]
> > + IMGU_ABI_FRAME_FORMAT_CSI_MIPI_LEGACY_YUV420_8, /*
> Legacy YUV420.
> > +* UY odd line;
> > +* VY even line
> > +*/
> > + IMGU_ABI_FRAME_FORMAT_CSI_MIPI_YUV420_10,/* 10 bit per
> Y/U/V. Y odd
> > + * line; UYVY interleaved
> > + * even line
> > + */
> > + IMGU_ABI_FRAME_FORMAT_YCgCo444_16, /* Internal format for
> > + ISP2.7,
>
> Macros and enums should be uppercase.
>
Ack.
> [snip]
> > +struct imgu_abi_shd_intra_frame_operations_data {
> > + struct imgu_abi_acc_operation
> > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> IPU3_ALIGN;
> > + struct imgu_abi_acc_process_lines_cmd_data
> > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> IPU3_ALIGN;
> > + struct imgu_abi_shd_transfer_luts_set_data
> > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS] IPU3_ALIGN;
> > +} __packed;
> > +
> > +struct imgu_abi_shd_config {
> > + struct ipu3_uapi_shd_config_static shd IMGU_ABI_PAD;
> > + struct imgu_abi_shd_intra_frame_operations_data shd_ops
> IMGU_ABI_PAD;
> > + struct ipu3_uapi_shd_lut shd_lut IMGU_ABI_PAD;
>
> Definitions of both IPU3_ALIGN and IMGU_ABI_PAD seem to be equivalent.
> Could we remove one and use the other everywhere?
>
Agree, will remove IMGU_ABI_PAD.
> [snip]
> > +struct imgu_abi_osys_scaler_params {
> > + __u32 inp_buf_y_st_addr;
> > + __u32 inp_buf_y_line_stride;
> > + __u32 inp_buf_y_buffer_stride;
> > + __u32 inp_buf_u_st_addr;
> > + __u32 inp_buf_v_st_addr;
> > + __u32 inp_buf_uv_line_stride;
> > + __u32 inp_buf_uv_buffer_stride;
> > + __u32 inp_buf_chunk_width;
> > + __u32 inp_buf_nr_buffers;
> > + /* Output buffers */
> > + __u32 out_buf_y_st_addr;
> > + __u32 out_buf_y_line_stride;
> > + __u32 out_buf_y_buffer_stride;
> > + __u32 out_buf_u_st_addr;
> > + __u32 out_buf_v_st_addr;
> > + __u32 out_buf_uv_line_stride;
> > + __u32 out_buf_uv_buffer_stride;
> > + __u32 out_buf_nr_buffers;
> > + /* Intermediate buffers */
> > + __u32 int_buf_y_st_addr;
> > + __u32 int_buf_y_line_stride;
> > + __u32 int_buf_u_st_addr;
> > + __u32 int_buf_v_st_addr;
> > + __u32 int_buf_uv_line_stride;
> > + __u32 int_buf_height;
> > + __u32 int_buf_chunk_width;
> > + __u32 int_buf_chunk_height;
> > + /* Context buffers */
> > + __u32 ctx_buf_hor_y_st_addr;
> > + __u32 ctx_buf_hor_u_st_addr;
> > + __u32 ctx_buf_hor_v_st_addr;
> > + __u32 ctx_buf_ver_y_st_addr;
> > + __u32 ctx_buf_ver_u_st_addr;
> > + __u32 ctx_buf_ver_v_st_addr;
> > + /* Addresses for release-input and process-output tokens */
> > + __u32 release_inp_buf_addr;
> > + __u32 release_inp_buf_en;
> > + __u32 release_out_buf_en;
> > + __u32 process_out_buf_addr;
> > + /* Settings dimensions,
Re: [PATCH 3/5] media: v4l2-common: add v4l2_find_closest_fract()
2018年9月19日(水) 20:18 Sakari Ailus :
>
> Hi Mita-san,
>
> On Tue, Sep 18, 2018 at 01:03:09AM +0900, Akinobu Mita wrote:
> > Add a function to locate the closest element in a sorted v4l2_fract array.
> >
> > The implementation is based on find_closest() macro in linux/util_macros.h
> > and the way to compare two v4l2_fract in vivid_vid_cap_s_parm in
> > drivers/media/platform/vivid/vivid-vid-cap.c.
> >
> > Cc: Matt Ranostay
> > Cc: Sakari Ailus
> > Cc: Hans Verkuil
> > Cc: Mauro Carvalho Chehab
> > Signed-off-by: Akinobu Mita
> > ---
> > drivers/media/v4l2-core/v4l2-common.c | 26 ++
> > include/media/v4l2-common.h | 12
> > 2 files changed, 38 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c
> > b/drivers/media/v4l2-core/v4l2-common.c
> > index b518b92..91bd460 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -387,6 +387,32 @@ __v4l2_find_nearest_size(const void *array, size_t
> > array_size,
> > }
> > EXPORT_SYMBOL_GPL(__v4l2_find_nearest_size);
> >
> > +#define FRACT_CMP(a, OP, b) \
> > + ((u64)(a).numerator * (b).denominator OP\
> > + (u64)(b).numerator * (a).denominator)
> > +
> > +int v4l2_find_closest_fract(struct v4l2_fract x, const struct v4l2_fract
> > *array,
>
> unsigned int ?
As you noticed below, this function may lead to an overflow. So I planned
to make it return -EOVERFLOW with help of linux/overflow.h.
But now I'm start thinking that finding closest (rounding) value is
overkill. Instead finding smallest (ceiling) or largest (floor) value is
enough just like in vivid_vid_cap_s_parm() in vivid-vid-cap.c and we don't
need to be bothered with overflows.
> > + size_t num)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < num - 1; i++) {
> > + struct v4l2_fract a = array[i];
> > + struct v4l2_fract b = array[i + 1];
> > + struct v4l2_fract midpoint = {
> > + .numerator = a.numerator * b.denominator +
> > + b.numerator * a.denominator,
>
> Assuming the entire range could be in use, this may lead to an overflow.
> Same on the line below.
>
> I also wonder if e.g. a binary search would be more effective than going
> through the entire list.
The video-i2c driver will use this function with an array of 2 objects
and the vivid driver may also use this function with an array of 5 objects.
So simple linear search is enough for now, but it can be changed to
bsearch without changing external interface if needed sometime.
> > + .denominator = 2 * a.denominator * b.denominator,
> > + };
> > +
> > + if (FRACT_CMP(x, <=, midpoint))
> > + break;
> > + }
> > +
> > + return i;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_find_closest_fract);
> > +
> > void v4l2_get_timestamp(struct timeval *tv)
> > {
> > struct timespec ts;
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > index cdc87ec..e388f4e 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -350,6 +350,18 @@ __v4l2_find_nearest_size(const void *array, size_t
> > array_size,
> >size_t height_offset, s32 width, s32 height);
> >
> > /**
> > + * v4l2_find_closest_fract - locate the closest element in a sorted array
> > + * @x: The reference value.
> > + * @array: The array in which to look for the closest element. Must be
> > sorted
> > + * in ascending order.
> > + * @num: number of elements in 'array'.
> > + *
> > + * Returns the index of the element closest to 'x'.
> > + */
> > +int v4l2_find_closest_fract(struct v4l2_fract x, const struct v4l2_fract
> > *array,
> > + size_t num);
> > +
> > +/**
> > * v4l2_get_timestamp - helper routine to get a timestamp to be used when
> > * filling streaming metadata. Internally, it uses ktime_get_ts(),
> > * which is the recommended way to get it.
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
Re: [PATCH 1/5] media: video-i2c: avoid accessing released memory area when removing driver
2018年9月19日(水) 19:35 Sakari Ailus :
>
> Hi Mita-san,
>
> On Tue, Sep 18, 2018 at 01:03:07AM +0900, Akinobu Mita wrote:
> > The struct video_i2c_data is released when video_unregister_device() is
> > called, but it will still be accessed after calling
> > video_unregister_device().
> >
> > Use devm_kzalloc() and let the memory be automatically released on driver
> > detach.
> >
> > Fixes: 5cebaac60974 ("media: video-i2c: add video-i2c driver")
> > Cc: Matt Ranostay
> > Cc: Sakari Ailus
> > Cc: Hans Verkuil
> > Cc: Mauro Carvalho Chehab
> > Signed-off-by: Akinobu Mita
> > ---
> > drivers/media/i2c/video-i2c.c | 18 +-
> > 1 file changed, 5 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> > index 06d29d8..b7a2af9 100644
> > --- a/drivers/media/i2c/video-i2c.c
> > +++ b/drivers/media/i2c/video-i2c.c
> > @@ -508,20 +508,15 @@ static const struct v4l2_ioctl_ops
> > video_i2c_ioctl_ops = {
> > .vidioc_streamoff = vb2_ioctl_streamoff,
> > };
> >
> > -static void video_i2c_release(struct video_device *vdev)
> > -{
> > - kfree(video_get_drvdata(vdev));
>
> This is actually correct: it ensures that that the device data stays in
> place as long as the device is being accessed. Allocating device data with
> devm_kzalloc() no longer guarantees that, and is not the right thing to do
> for that reason.
I have actually inserted printk() each line in video_i2_remove(). When
rmmod this driver, video_i2c_release() (and also kfree) is called while
executing video_unregister_device(). Because video_unregister_device()
releases the last reference to data->vdev.dev, then v4l2_device_release()
callback executes data->vdev.release.
So use after freeing video_i2c_data actually happened.
In this patch, devm_kzalloc() is called with client->dev (not with vdev->dev).
So the allocated memory is released when the last user of client->dev
is gone (maybe just after video_i2_remove() is finished).
> > -}
> > -
> > static int video_i2c_probe(struct i2c_client *client,
> >const struct i2c_device_id *id)
> > {
> > struct video_i2c_data *data;
> > struct v4l2_device *v4l2_dev;
> > struct vb2_queue *queue;
> > - int ret = -ENODEV;
> > + int ret;
> >
> > - data = kzalloc(sizeof(*data), GFP_KERNEL);
> > + data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> > if (!data)
> > return -ENOMEM;
> >
> > @@ -530,7 +525,7 @@ static int video_i2c_probe(struct i2c_client *client,
> > else if (id)
> > data->chip = _i2c_chip[id->driver_data];
> > else
> > - goto error_free_device;
> > + return -ENODEV;
> >
> > data->client = client;
> > v4l2_dev = >v4l2_dev;
> > @@ -538,7 +533,7 @@ static int video_i2c_probe(struct i2c_client *client,
> >
> > ret = v4l2_device_register(>dev, v4l2_dev);
> > if (ret < 0)
> > - goto error_free_device;
> > + return ret;
> >
> > mutex_init(>lock);
> > mutex_init(>queue_lock);
> > @@ -568,7 +563,7 @@ static int video_i2c_probe(struct i2c_client *client,
> > data->vdev.fops = _i2c_fops;
> > data->vdev.lock = >lock;
> > data->vdev.ioctl_ops = _i2c_ioctl_ops;
> > - data->vdev.release = video_i2c_release;
> > + data->vdev.release = video_device_release_empty;
> > data->vdev.device_caps = V4L2_CAP_VIDEO_CAPTURE |
> >V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
> >
> > @@ -597,9 +592,6 @@ static int video_i2c_probe(struct i2c_client *client,
> > mutex_destroy(>lock);
> > mutex_destroy(>queue_lock);
> >
> > -error_free_device:
> > - kfree(data);
> > -
> > return ret;
> > }
> >
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
PAL60 not working
Hello, I am using v4l2 with my Hauppauge USB-Live2 and it works with NTSC or PAL but not with PAL60. I have a few NTSC tapes I need to digitalize but my european VCR only output PAL60 with them. As described here: https://github.com/b-rad-NDi/Ubuntu-media-tree-kernel-builder/issues/48, I don't get any signal when switching the driver to PAL60 even if my Hauppauge device seems to support it. Is it a bug from the driver ? Thank you
Re: [PATCH v3 6/9] media: v4l2-subdev: fix v4l2_subdev_get_try_* dependency
Hi Sakari,
On 18-09-19 13:45, Sakari Ailus wrote:
> Hi Marco,
>
> On Tue, Sep 18, 2018 at 03:14:50PM +0200, Marco Felsch wrote:
> > 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
> > ---
> > Changelog:
> >
> > v3:
> > - add CONFIG_MEDIA_CONTROLLER switch instead of moving the
> > v4l2_subdev_get_try_* APIs into the existing one.
> >
> > v2:
> > - Initial commit
> >
> > include/media/v4l2-subdev.h | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index ce48f1fcf295..d2479d5ebca8 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -912,6 +912,8 @@ struct v4l2_subdev_fh {
> > #define to_v4l2_subdev_fh(fh) \
> > container_of(fh, struct v4l2_subdev_fh, vfh)
> >
> > +#ifdef CONFIG_MEDIA_CONTROLLER
>
> VIDEO_V4L2_SUBDEV_API (used below) depends on MEDIA_CONTROLLER. Either this
> or the previous patch would be meaningful but not both.
>
> Considering a driver wouldn't use the functions below if it did not need or
> could use VIDEO_V4L2_SUBDEV_API, I'd suggest retaining the other patch.
Oh sorry didn't checked the Kconfig.
Mauro can you drop that patch and use only the patch ("media: v4l2-subdev:
add stubs for v4l2_subdev_get_try_*")?
Regards,
Marco
> > +
> > /**
> > * v4l2_subdev_get_try_format - ancillary routine to call
> > * v4l2_subdev_pad_config->try_fmt
> > @@ -978,6 +980,8 @@ static inline struct v4l2_rect
> > #endif
> > }
> >
> > +#endif
> > +
> > extern const struct v4l2_file_operations v4l2_subdev_fops;
> >
> > /**
>
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
MIPI-CSI on Tegra124
Hello, I would like to use a baseline kernel on a TegraK1 board with a MIPI CSI video camera. I have seen this patch regarding a tegra-vi driver https://patchwork.kernel.org/patch/7246881/ Cqn someone tell me if there is a chance (or not) to make it work with tegra124 chip on kernel 4.17 ? (maybe with some minor modifications). Thank you in advance. -- Philippe GISLARD
Re: [PATCH 3/5] media: v4l2-common: add v4l2_find_closest_fract()
Hi Mita-san,
On Tue, Sep 18, 2018 at 01:03:09AM +0900, Akinobu Mita wrote:
> Add a function to locate the closest element in a sorted v4l2_fract array.
>
> The implementation is based on find_closest() macro in linux/util_macros.h
> and the way to compare two v4l2_fract in vivid_vid_cap_s_parm in
> drivers/media/platform/vivid/vivid-vid-cap.c.
>
> Cc: Matt Ranostay
> Cc: Sakari Ailus
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> Signed-off-by: Akinobu Mita
> ---
> drivers/media/v4l2-core/v4l2-common.c | 26 ++
> include/media/v4l2-common.h | 12
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c
> b/drivers/media/v4l2-core/v4l2-common.c
> index b518b92..91bd460 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -387,6 +387,32 @@ __v4l2_find_nearest_size(const void *array, size_t
> array_size,
> }
> EXPORT_SYMBOL_GPL(__v4l2_find_nearest_size);
>
> +#define FRACT_CMP(a, OP, b) \
> + ((u64)(a).numerator * (b).denominator OP\
> + (u64)(b).numerator * (a).denominator)
> +
> +int v4l2_find_closest_fract(struct v4l2_fract x, const struct v4l2_fract
> *array,
unsigned int ?
> + size_t num)
> +{
> + int i;
> +
> + for (i = 0; i < num - 1; i++) {
> + struct v4l2_fract a = array[i];
> + struct v4l2_fract b = array[i + 1];
> + struct v4l2_fract midpoint = {
> + .numerator = a.numerator * b.denominator +
> + b.numerator * a.denominator,
Assuming the entire range could be in use, this may lead to an overflow.
Same on the line below.
I also wonder if e.g. a binary search would be more effective than going
through the entire list.
> + .denominator = 2 * a.denominator * b.denominator,
> + };
> +
> + if (FRACT_CMP(x, <=, midpoint))
> + break;
> + }
> +
> + return i;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_find_closest_fract);
> +
> void v4l2_get_timestamp(struct timeval *tv)
> {
> struct timespec ts;
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index cdc87ec..e388f4e 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -350,6 +350,18 @@ __v4l2_find_nearest_size(const void *array, size_t
> array_size,
>size_t height_offset, s32 width, s32 height);
>
> /**
> + * v4l2_find_closest_fract - locate the closest element in a sorted array
> + * @x: The reference value.
> + * @array: The array in which to look for the closest element. Must be sorted
> + * in ascending order.
> + * @num: number of elements in 'array'.
> + *
> + * Returns the index of the element closest to 'x'.
> + */
> +int v4l2_find_closest_fract(struct v4l2_fract x, const struct v4l2_fract
> *array,
> + size_t num);
> +
> +/**
> * v4l2_get_timestamp - helper routine to get a timestamp to be used when
> * filling streaming metadata. Internally, it uses ktime_get_ts(),
> * which is the recommended way to get it.
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
[RFP] Key signing party at the media summit
As we're now using signed tags for media pull request, let's reserve some space for a key signing party. Regards, Mauro
Re: [PATCH v3 6/9] media: v4l2-subdev: fix v4l2_subdev_get_try_* dependency
Hi Marco,
On Tue, Sep 18, 2018 at 03:14:50PM +0200, Marco Felsch wrote:
> 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
> ---
> Changelog:
>
> v3:
> - add CONFIG_MEDIA_CONTROLLER switch instead of moving the
> v4l2_subdev_get_try_* APIs into the existing one.
>
> v2:
> - Initial commit
>
> include/media/v4l2-subdev.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index ce48f1fcf295..d2479d5ebca8 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -912,6 +912,8 @@ struct v4l2_subdev_fh {
> #define to_v4l2_subdev_fh(fh)\
> container_of(fh, struct v4l2_subdev_fh, vfh)
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
VIDEO_V4L2_SUBDEV_API (used below) depends on MEDIA_CONTROLLER. Either this
or the previous patch would be meaningful but not both.
Considering a driver wouldn't use the functions below if it did not need or
could use VIDEO_V4L2_SUBDEV_API, I'd suggest retaining the other patch.
> +
> /**
> * v4l2_subdev_get_try_format - ancillary routine to call
> *v4l2_subdev_pad_config->try_fmt
> @@ -978,6 +980,8 @@ static inline struct v4l2_rect
> #endif
> }
>
> +#endif
> +
> extern const struct v4l2_file_operations v4l2_subdev_fops;
>
> /**
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH 1/5] media: video-i2c: avoid accessing released memory area when removing driver
Hi Mita-san,
On Tue, Sep 18, 2018 at 01:03:07AM +0900, Akinobu Mita wrote:
> The struct video_i2c_data is released when video_unregister_device() is
> called, but it will still be accessed after calling
> video_unregister_device().
>
> Use devm_kzalloc() and let the memory be automatically released on driver
> detach.
>
> Fixes: 5cebaac60974 ("media: video-i2c: add video-i2c driver")
> Cc: Matt Ranostay
> Cc: Sakari Ailus
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> Signed-off-by: Akinobu Mita
> ---
> drivers/media/i2c/video-i2c.c | 18 +-
> 1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> index 06d29d8..b7a2af9 100644
> --- a/drivers/media/i2c/video-i2c.c
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -508,20 +508,15 @@ static const struct v4l2_ioctl_ops video_i2c_ioctl_ops
> = {
> .vidioc_streamoff = vb2_ioctl_streamoff,
> };
>
> -static void video_i2c_release(struct video_device *vdev)
> -{
> - kfree(video_get_drvdata(vdev));
This is actually correct: it ensures that that the device data stays in
place as long as the device is being accessed. Allocating device data with
devm_kzalloc() no longer guarantees that, and is not the right thing to do
for that reason.
> -}
> -
> static int video_i2c_probe(struct i2c_client *client,
>const struct i2c_device_id *id)
> {
> struct video_i2c_data *data;
> struct v4l2_device *v4l2_dev;
> struct vb2_queue *queue;
> - int ret = -ENODEV;
> + int ret;
>
> - data = kzalloc(sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> @@ -530,7 +525,7 @@ static int video_i2c_probe(struct i2c_client *client,
> else if (id)
> data->chip = _i2c_chip[id->driver_data];
> else
> - goto error_free_device;
> + return -ENODEV;
>
> data->client = client;
> v4l2_dev = >v4l2_dev;
> @@ -538,7 +533,7 @@ static int video_i2c_probe(struct i2c_client *client,
>
> ret = v4l2_device_register(>dev, v4l2_dev);
> if (ret < 0)
> - goto error_free_device;
> + return ret;
>
> mutex_init(>lock);
> mutex_init(>queue_lock);
> @@ -568,7 +563,7 @@ static int video_i2c_probe(struct i2c_client *client,
> data->vdev.fops = _i2c_fops;
> data->vdev.lock = >lock;
> data->vdev.ioctl_ops = _i2c_ioctl_ops;
> - data->vdev.release = video_i2c_release;
> + data->vdev.release = video_device_release_empty;
> data->vdev.device_caps = V4L2_CAP_VIDEO_CAPTURE |
>V4L2_CAP_READWRITE | V4L2_CAP_STREAMING;
>
> @@ -597,9 +592,6 @@ static int video_i2c_probe(struct i2c_client *client,
> mutex_destroy(>lock);
> mutex_destroy(>queue_lock);
>
> -error_free_device:
> - kfree(data);
> -
> return ret;
> }
>
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
Re: RFC: stop support for 2.6 kernel in the daily build
On 09/13/2018 08:49 AM, Hans Verkuil wrote: > SUSE Linux Enterprise Server 12 is on kernel 3.12, and version 11 SP2 or up > is on kernel 3.0. > > Red Hat's RHEL 7 is on kernel 3.10. > > I'm inclined to drop support for 2.6 altogether. If nothing else it > simplifies the kernel version handling in media_build. > > Whether we should also drop support for 3.0-3.9 is another matter. > I wouldn't mind, 3.10 seems to be a reasonable minimum to me, but > I might be too optimistic here. Final reminder: unless someone objects I will drop support for kernels 3.0-3.9 from the daily build tomorrow. Pre-3.0 kernels are already dropped. Regards, hans
