Re: [19/38] ARM: dts: imx6-sabrelite: add video capture ports and connections
Steve, All, On Tue, Jun 14, 2016 at 03:49:15PM -0700, Steve Longerbeam wrote: > Defines the host video capture device node and an OV5642 camera sensor > node on i2c2. The host capture device connects to the OV5642 via the > parallel-bus mux input on the ipu1_csi0_mux. > > Note there is a pin conflict with GPIO6. This pin functions as a power > input pin to the OV5642, but ENET requires it to wake-up the ARM cores > on normal RX and TX packet done events (see 6261c4c8). So by default, > capture is disabled, enable by uncommenting __OV5642_CAPTURE__ macro. > Ethernet will still work just not quite as well. Actually the following patch fixes this issue and has already been applied on Shawn's tree: https://patchwork.kernel.org/patch/9153523/ Also, this follow-up patch declared the HW workaround for SabreLite: https://patchwork.kernel.org/patch/9153525/ So ideally, once those two patches land on your base tree, you could get rid of the #define and remove the HW workaround declaration. Finally, I'll test the series on Sabre-Lite this week. Regards, Gary -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister()
(Shuah, I did not receive your second reply; I only found it in an email archive.) > Yes media_devnode_create() creates the interfaces links and these > links are deleted by media_devnode_remove(). > media_device_unregister() still needs to delete the interfaces > links. The reason for that is the API dynalic use-case. > > Drivers (other than dvb-core and v4l2-core) can create and delete > media devnode interfaces during run-time My point was that they do not. There are no other media_devnode_create() callers. > So removing kfree() from media_device_unregister() isn't the correct > fix. Then what is? I don't know anything other than the (mostly undocumented) code I read, and my patch implements the design that I interpreted from the code. Apparently my interpretation of the design is wrong after all. > I don't see the stack trace for the double free error you are > seeing? Actually, it didn't crash at the double free; it hung forever because it tried to lock a mutex which was already stale. I don't have a stack trace of that; would it help to produce one? > Could it be that there is a driver problem in the order in which it > is calling media_device_unregister()? Maybe it's due to my patch 1/3 which adds a kref, and it only occurs if one process still has a file handle. In any case, the kernel must decide who's responsible for freeing the object, and how the dvbdev.c library gets to know that its pointer has been invalidated. Please explain how it should be done, and I'll try to adapt my patches to the "grand design". -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/38] i.MX5/6 Video Capture
On 16/06/16 02:37, Steve Longerbeam wrote: Hi Jack, On 06/15/2016 03:43 AM, Jack Mitchell wrote: Trying to use a user pointer rather than mmap also fails and causes a kernel splat. Hmm, I've tested userptr with the mem2mem driver, but maybe never with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked down why (could be a bug in v4l2-ctl). Can you share the splat? On re-checking the splat was the same v4l_cropcap that was mentioned before so I don't think it's related. The error I get back is: VIDIOC_QBUF error 22, Invalid argument I'm using the example program the the v4l2 docs [1]. Cheers, Jack [1] https://linuxtv.org/downloads/v4l-dvb-apis/capture-example.html Apart from that and a few v4l2-compliance tests failing which you already mentioned, it seems to work OK. I'll try and do some more testing and see if I can come back with some more feedback. Thanks! Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Add MT8173 Video Decoder Driver
Em Tue, 14 Jun 2016 19:08:08 +0800 tiffany lin escreveu: > Hi Mauro, > > > On Wed, 2016-06-08 at 07:13 +0900, Hans Verkuil wrote: > > > > On 06/07/2016 11:22 PM, Mauro Carvalho Chehab wrote: > > > Em Mon, 30 May 2016 20:29:14 +0800 > > > Tiffany Lin escreveu: > > > > > >> == > > >> Introduction > > >> == > > >> > > >> The purpose of this series is to add the driver for video codec hw > > >> embedded in the Mediatek's MT8173 SoCs. > > >> Mediatek Video Codec is able to handle video decoding of in a range of > > >> formats. > > >> > > >> This patch series add Mediatek block format V4L2_PIX_FMT_MT21, the > > >> decoder driver will decoded bitstream to > > >> V4L2_PIX_FMT_MT21 format. > > >> > > >> This patch series rely on MTK VPU driver in patch series "Add MT8173 > > >> Video Encoder Driver and VPU Driver"[1] > > >> and patch "CHROMIUM: v4l: Add V4L2_PIX_FMT_VP9 definition"[2] for VP9 > > >> support. > > >> Mediatek Video Decoder driver rely on VPU driver to load, communicate > > >> with VPU. > > >> > > >> Internally the driver uses videobuf2 framework and MTK IOMMU and MTK SMI > > >> both have been merged in v4.6-rc1. > > >> > > >> [1]https://patchwork.linuxtv.org/patch/33734/ > > >> [2]https://chromium-review.googlesource.com/#/c/245241/ > > > > > > Hmm... I'm not seeing the firmware for this driver at the > > > linux-firmware tree: > > > > > > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/log/ > > > > > > Nor I'm seeing any pull request for them. Did you send it? > > > I'll only merge the driver upstream after seeing such pull request. > > > Sorry, I am not familiar with how to upstream firmware. > Do you mean we need to upstream vpu firmware first before merge encoder > driver upstream? Please look at this page: https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches#Firmware_submission The information here can also be useful: https://www.kernel.org/doc/readme/firmware-README.AddingFirmware In summary, you need to provide redistribution rights for the firmware blob. You can either submit it to me or directly to linux-firmware. In the latter, please c/c me on such patch. Thanks! Mauro > > In > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/README, > it mentions that > "To submit firmware to this repository, please send either a git binary > diff or preferably a git pull request to: linux-firmw...@kernel.org and > also cc: to related mailing lists." > > How we made a git pull request to linux-firmw...@kernel.org? > How we find out related mailing lists? > > best regards, > Tiffany > > > Mauro, are you confusing the decoder and encoder driver? I haven't > > thoroughly reviewed the decoder driver > > yet, so there is no pull request for the decoder driver. > > > > The only pull request I made was for the encoder driver. > > > > Regards, > > > > Hans > > -- Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] media-devnode.h: Fix documentation
Two parameters were documented with a wrong name, and a struct
device pointer description was missing.
That caused the following warnings, when building documentation:
include/media/media-devnode.h:102: warning: No description found for parameter
'media_dev'
include/media/media-devnode.h:126: warning: No description found for parameter
'mdev'
include/media/media-devnode.h:126: warning: Excess function parameter
'media_dev' description in 'media_devnode_register'
Rename the description, to match the function parameter and fix
Documentation.
No funcional changes.
Signed-off-by: Mauro Carvalho Chehab
---
include/media/media-devnode.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index f0b7dd79fb92..37d494805944 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -69,8 +69,9 @@ struct media_file_operations {
/**
* struct media_devnode - Media device node
+ * @media_dev: pointer to struct &media_device
* @fops: pointer to struct &media_file_operations with media device ops
- * @dev: struct device pointer for the media controller device
+ * @dev: pointer to struct &device containing the media controller device
* @cdev: struct cdev pointer character device
* @parent:parent device
* @minor: device node minor number
@@ -107,7 +108,7 @@ struct media_devnode {
/**
* media_devnode_register - register a media device node
*
- * @media_dev: struct media_device we want to register a device node
+ * @mdev: struct media_device we want to register a device node
* @devnode: media device node structure we want to register
* @owner: should be filled with %THIS_MODULE
*
--
2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 34/38] media: imx: Add support for ADV7180 Video Decoder
On 06/15/2016 12:49 AM, Steve Longerbeam wrote: > This driver is based on adv7180.c from Freescale imx_3.10.17_1.0.0_beta > branch, modified heavily for code cleanup and converted from int-device > to subdev. We already have a driver for the adv7180 upstream, also using the subdev API. Is there anything that can be done with this new driver that can't be done with the other one. And if it is are there any blockers that would prevent us from adding the missing features to the upstream adv7180? I know that the driver in the Freescale tree used to have bits that made it specially tailored to the iMX6. But these bits seem to be mostly gone in this version of the driver. I'm slightly concerned about the conflicting nature of these drivers. Both attach to the same device ID/DT compatible string and register slightly different userspace ABIs. I'd like to avoid ending up in a situation where we have dependencies on both ABIs and can no longer converge. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister()
On 06/16/2016 03:29 AM, Max Kellermann wrote: > (Shuah, I did not receive your second reply; I only found it in an > email archive.) > >> Yes media_devnode_create() creates the interfaces links and these >> links are deleted by media_devnode_remove(). >> media_device_unregister() still needs to delete the interfaces >> links. The reason for that is the API dynalic use-case. >> >> Drivers (other than dvb-core and v4l2-core) can create and delete >> media devnode interfaces during run-time > > My point was that they do not. There are no other > media_devnode_create() callers. Correct. There are none in the base now. However as I explained the dynamic use-case. There is work in progress that uses this feature in the API. > >> So removing kfree() from media_device_unregister() isn't the correct >> fix. > > Then what is? I don't know anything other than the (mostly > undocumented) code I read, and my patch implements the design that I > interpreted from the code. Apparently my interpretation of the design > is wrong after all. > >> I don't see the stack trace for the double free error you are >> seeing? > > Actually, it didn't crash at the double free; it hung forever because > it tried to lock a mutex which was already stale. I don't have a > stack trace of that; would it help to produce one? I think you are running into another set of problems related to media devnode, cdev, and race between media ioctl/syscall and media unregister sequence. These patches are in git://linuxtv.org/media_tree.git master branch > >> Could it be that there is a driver problem in the order in which it >> is calling media_device_unregister()? > > Maybe it's due to my patch 1/3 which adds a kref, and it only occurs > if one process still has a file handle. So you are adding another refcounted object to the mix, in addition to media_device, media_devnode, and cdev. Now you have three or more objects with varying lifetimes. Not a good situation to be in. > > In any case, the kernel must decide who's responsible for freeing the > object, and how the dvbdev.c library gets to know that its pointer has > been invalidated. Yes it does that. intf links need to be free'd in both cases, one when driver does a devnode_remove() and then when unregister is done. There could be two drivers that are bound to the media hardware and both might own their own sections of the media graph. Media Controller core has to allow the possibility of one driver unbind/rmmod and be able to delete the devnode it created. I don't think the problem you are running into is due to this code path. Without seeing the stack trace, it is hard to debug as you really don't know what the problem is, leave alone being able to fix it. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Mesa-dev] [RFC] New dma_buf -> EGLImage EGL extension - Final spec published!
So, if we wanted to extend this to support the fourcc-modifiers that we have on the kernel side for compressed/tiled/etc formats, what would be the right approach? A new version of the existing extension or a new EGL_EXT_image_dma_buf_import2 extension, or ?? BR, -R On Mon, Feb 25, 2013 at 6:54 AM, Tom Cooksey wrote: > Hi All, > > The final spec has had enum values assigned and been published on Khronos: > > http://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt > > Thanks to all who've provided input. > > > Cheers, > > Tom > > > >> -Original Message- >> From: mesa-dev-bounces+tom.cooksey=arm@lists.freedesktop.org >> [mailto:mesa-dev- >> bounces+tom.cooksey=arm@lists.freedesktop.org] On Behalf Of Tom Cooksey >> Sent: 04 October 2012 13:10 >> To: mesa-...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org; dri- >> de...@lists.freedesktop.org; linux-media@vger.kernel.org >> Subject: [Mesa-dev] [RFC] New dma_buf -> EGLImage EGL extension - New draft! >> >> Hi All, >> >> After receiving a fair bit of feedback (thanks!), I've updated the >> EGL_EXT_image_dma_buf_import spec >> and expanded it to resolve a number of the issues. Please find the latest >> draft below and let >> me >> know any additional feedback you might have, either on the lists or by >> private e-mail - I >> don't mind >> which. >> >> I think the only remaining issue now is if we need a mechanism whereby an >> application can >> query >> which drm_fourcc.h formats EGL supports or if just failing with >> EGL_BAD_MATCH when the >> application >> has use one EGL doesn't support is sufficient. Any thoughts? >> >> >> Cheers, >> >> Tom >> >> >> 8< >> >> >> Name >> >> EXT_image_dma_buf_import >> >> Name Strings >> >> EGL_EXT_image_dma_buf_import >> >> Contributors >> >> Jesse Barker >> Rob Clark >> Tom Cooksey >> >> Contacts >> >> Jesse Barker (jesse 'dot' barker 'at' linaro 'dot' org) >> Tom Cooksey (tom 'dot' cooksey 'at' arm 'dot' com) >> >> Status >> >> DRAFT >> >> Version >> >> Version 4, October 04, 2012 >> >> Number >> >> EGL Extension ??? >> >> Dependencies >> >> EGL 1.2 is required. >> >> EGL_KHR_image_base is required. >> >> The EGL implementation must be running on a Linux kernel supporting the >> dma_buf buffer sharing mechanism. >> >> This extension is written against the wording of the EGL 1.2 >> Specification. >> >> Overview >> >> This extension allows creating an EGLImage from a Linux dma_buf file >> descriptor or multiple file descriptors in the case of multi-plane YUV >> images. >> >> New Types >> >> None >> >> New Procedures and Functions >> >> None >> >> New Tokens >> >> Accepted by the parameter of eglCreateImageKHR: >> >> EGL_LINUX_DMA_BUF_EXT >> >> Accepted as an attribute in the parameter of >> eglCreateImageKHR: >> >> EGL_LINUX_DRM_FOURCC_EXT >> EGL_DMA_BUF_PLANE0_FD_EXT >> EGL_DMA_BUF_PLANE0_OFFSET_EXT >> EGL_DMA_BUF_PLANE0_PITCH_EXT >> EGL_DMA_BUF_PLANE1_FD_EXT >> EGL_DMA_BUF_PLANE1_OFFSET_EXT >> EGL_DMA_BUF_PLANE1_PITCH_EXT >> EGL_DMA_BUF_PLANE2_FD_EXT >> EGL_DMA_BUF_PLANE2_OFFSET_EXT >> EGL_DMA_BUF_PLANE2_PITCH_EXT >> EGL_YUV_COLOR_SPACE_HINT_EXT >> EGL_SAMPLE_RANGE_HINT_EXT >> EGL_YUV_CHROMA_HORIZONTAL_SITING_HINT_EXT >> EGL_YUV_CHROMA_VERTICAL_SITING_HINT_EXT >> >> Accepted as the value for the EGL_YUV_COLOR_SPACE_HINT_EXT attribute: >> >> EGL_ITU_REC601_EXT >> EGL_ITU_REC709_EXT >> EGL_ITU_REC2020_EXT >> >> Accepted as the value for the EGL_SAMPLE_RANGE_HINT_EXT attribute: >> >> EGL_YUV_FULL_RANGE_EXT >> EGL_YUV_NARROW_RANGE_EXT >> >> Accepted as the value for the EGL_YUV_CHROMA_HORIZONTAL_SITING_HINT_EXT & >> EGL_YUV_CHROMA_VERTICAL_SITING_HINT_EXT attributes: >> >> EGL_YUV_CHROMA_SITING_0_EXT >> EGL_YUV_CHROMA_SITING_0_5_EXT >> >> >> Additions to Chapter 2 of the EGL 1.2 Specification (EGL Operation) >> >> Add to section 2.5.1 "EGLImage Specification" (as defined by the >> EGL_KHR_image_base specification), in the description of >> eglCreateImageKHR: >> >>"Values accepted for are listed in Table aaa, below. >> >> >> +-++ >> | | Notes >> | >> >> +-++ >> | EGL_LINUX_DMA_BUF_EXT | Used for EGLImages imported from Linux >> | >> | | dma_buf file descriptors >> | >> >> +-++ >>Table aaa. Legal values for eglCreateImageKHR parameter >> >> ... >> >> If is EGL_LINUX_DMA_BUF_E
Re: [PATCH 1/8] media: rcar-vin: pad-aware driver initialisation
Hello Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:02 Niklas Söderlund wrote:
> From: Ulrich Hecht
>
> Add detection of source pad number for drivers aware of the media controller
> API, so that rcar-vin can create device nodes to support modern drivers
> such as adv7604.c (for HDMI on Lager) and the converted adv7180.c (for
> composite) underneath.
>
> Building rcar_vin gains a dependency on CONFIG_MEDIA_CONTROLLER, in
> line with requirements for building the drivers associated with it.
>
> Signed-off-by: William Towle
> Signed-off-by: Rob Taylor
> [uli: adapted to rcar-vin rewrite]
> Signed-off-by: Ulrich Hecht
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 16
> drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 0bc4487..929816b 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -683,6 +683,9 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> struct v4l2_mbus_framefmt *mf = &fmt.format;
> struct video_device *vdev = &vin->vdev;
> struct v4l2_subdev *sd = vin_to_source(vin);
> +#if defined(CONFIG_MEDIA_CONTROLLER)
I think you can get rid of conditional compilation here and below. Patch 2/8
calls v4l2_subdev_alloc_pad_config() unconditionally, which depends on
CONFIG_MEDIA_CONTROLLER.
> + int pad_idx;
> +#endif
> int ret;
>
> v4l2_set_subdev_hostdata(sd, vin);
> @@ -729,6 +732,19 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> V4L2_CAP_READWRITE;
>
> + vin->src_pad_idx = 0;
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> + if (sd->entity.pads[pad_idx].flags
> + == MEDIA_PAD_FL_SOURCE)
No need for a line break.
> + break;
> + if (pad_idx >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + vin->src_pad_idx = pad_idx;
> +#endif
> + fmt.pad = vin->src_pad_idx;
> +
> /* Try to improve our guess of a reasonable window format */
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> if (ret) {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index 544a3b3..a6dd6db 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -87,6 +87,7 @@ struct rvin_graph_entity {
> *
> * @vdev:V4L2 video device associated with VIN
> * @v4l2_dev:V4L2 device
> + * @src_pad_idx: source pad index for media controller drivers
> * @ctrl_handler:V4L2 control handler
> * @notifier:V4L2 asynchronous subdevs notifier
> * @entity: entity in the DT for subdevice
> @@ -117,6 +118,7 @@ struct rvin_dev {
>
> struct video_device vdev;
> struct v4l2_device v4l2_dev;
> + int src_pad_idx;
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity entity;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] media: rcar_vin: Use correct pad number in try_fmt
Hello Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:03 Niklas Söderlund wrote:
> From: Ulrich Hecht
>
> Fix rcar_vin_try_fmt's use of an inappropriate pad number when calling
> the subdev set_fmt function - for the ADV7612, IDs should be non-zero.
>
> Signed-off-by: William Towle
> Reviewed-by: Rob Taylor
> Acked-by: Hans Verkuil
> [uli: adapted to rcar-vin rewrite]
> Signed-off-by: Ulrich Hecht
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 929816b..3788f8a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -98,7 +98,7 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
> struct rvin_source_fmt *source)
> {
> struct v4l2_subdev *sd;
> - struct v4l2_subdev_pad_config pad_cfg;
> + struct v4l2_subdev_pad_config *pad_cfg;
> struct v4l2_subdev_format format = {
> .which = which,
> };
> @@ -108,10 +108,16 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin,
>
> v4l2_fill_mbus_format(&format.format, pix, vin->source.code);
>
> + pad_cfg = v4l2_subdev_alloc_pad_config(sd);
> + if (pad_cfg == NULL)
> + return -ENOMEM;
> +
> + format.pad = vin->src_pad_idx;
> +
> ret = v4l2_device_call_until_err(sd->v4l2_dev, 0, pad, set_fmt,
> - &pad_cfg, &format);
> + pad_cfg, &format);
pad_cfg is subdev-specific, you can't use v4l2_device_call_until_err(). You
should use v4l2_subdev_call() instead. This will obviously not be enough if we
have more than one subdev in the pipeline, but the code is broken in that case
anyway.
> if (ret < 0)
> - return ret;
> + goto cleanup;
>
> v4l2_fill_pix_format(pix, &format.format);
>
> @@ -121,6 +127,8 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin, vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
> source->height);
>
> +cleanup:
Nitpicking, I'd name the label "done".
> + v4l2_subdev_free_pad_config(pad_cfg);
> return 0;
> }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] media: rcar-vin: add DV timings support
Hello Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:04 Niklas Söderlund wrote:
> From: Ulrich Hecht
>
> Adds ioctls DV_TIMINGS_CAP, ENUM_DV_TIMINGS, G_DV_TIMINGS, S_DV_TIMINGS,
> and QUERY_DV_TIMINGS.
>
> Signed-off-by: Ulrich Hecht
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 82 ++
> 1 file changed, 82 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 3788f8a..10a5c10 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -400,6 +400,10 @@ static int rvin_enum_input(struct file *file, void
> *priv,
>
> i->type = V4L2_INPUT_TYPE_CAMERA;
> i->std = vin->vdev.tvnorms;
> +
> + if (v4l2_subdev_has_op(sd, pad, dv_timings_cap))
> + i->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> +
> strlcpy(i->name, "Camera", sizeof(i->name));
>
> return 0;
> @@ -478,6 +482,78 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
> return v4l2_ctrl_subscribe_event(fh, sub);
> }
>
> +static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
> + struct v4l2_enum_dv_timings *timings)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int pad, ret;
pad can't be negative, you can make it an unsigned int.
unsigned int pad = timings->pad;
int ret;
timings->pad = vin->src_pad_idx;
> +
> + pad = timings->pad;
> + timings->pad = vin->src_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
> +
> + timings->pad = pad;
> +
> + return ret;
> +}
> +
> +static int rvin_s_dv_timings(struct file *file, void *priv_fh,
> + struct v4l2_dv_timings *timings)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int err;
The driver uses ret instead of err, let's keep it that way.
> +
> + err = v4l2_subdev_call(sd,
> + video, s_dv_timings, timings);
No need for a line break.
> + if (!err) {
I'd write this
if (ret)
return ret;
(with a return 0; at the end of the function) to lower the indentation level
of the code below.
> + vin->source.width = timings->bt.width;
> + vin->source.height = timings->bt.height;
> + vin->format.width = timings->bt.width;
> + vin->format.height = timings->bt.height;
> + }
> + return err;
> +}
> +
> +static int rvin_g_dv_timings(struct file *file, void *priv_fh,
> + struct v4l2_dv_timings *timings)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> +
> + return v4l2_subdev_call(sd,
> + video, g_dv_timings, timings);
No need for a line break.
> +}
> +
> +static int rvin_query_dv_timings(struct file *file, void *priv_fh,
> + struct v4l2_dv_timings *timings)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> +
> + return v4l2_subdev_call(sd,
> + video, query_dv_timings, timings);
No need for a line break.
> +}
> +
> +static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> + struct v4l2_dv_timings_cap *cap)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int pad, ret;
Same comment as above about pad not being negative.
> +
> + pad = cap->pad;
> + cap->pad = vin->src_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
> +
> + cap->pad = pad;
> +
> + return ret;
> +}
> +
> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_querycap= rvin_querycap,
> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> @@ -494,6 +570,12 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_g_input = rvin_g_input,
> .vidioc_s_input = rvin_s_input,
>
> + .vidioc_dv_timings_cap = rvin_dv_timings_cap,
> + .vidioc_enum_dv_timings = rvin_enum_dv_timings,
> + .vidioc_g_dv_timings= rvin_g_dv_timings,
> + .vidioc_s_dv_timings= rvin_s_dv_timings,
> + .vidioc_query_dv_timings= rvin_query_dv_timings,
> +
> .vidioc_querystd= rvin_querystd,
> .vidioc_g_std = rvin_g_std,
> .vidioc_s_std = rvin_s_std,
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 31/38] media: imx: Add video switch
For me this fails when I try to enable both video muxes (mx6dl, though mx6q should be the same). I get a sysfs duplicate name failure for 34.videomux. I realise passing the GPR13 register offset and a bitfield mask as a tuple in the reg value of the of_node is handy, but how should we account for multiple devices with the same name and address? A quick and dirty hack would be to have of_get_reg_field do something like field->reg = reg_bit_mask[0] & 0xff; and then use values in the DT that differ in the bits masked off, but there must be a nicer way. Trace below, fyi. This is from the v2 patches posted here, not your v2.1 tree. Regards, IanJ [0.096004] [ cut here ] [0.096035] WARNING: CPU: 0 PID: 1 at /home/ian/tx6/yoctomaster/build/tmp/work-shared/tx6u-vid/kernel-source/fs/sysfs/dir.c:31 sysfs_warn_dup+0x70/0x80 [0.096046] sysfs: cannot create duplicate filename '/devices/soc0/34.videomux' [0.096053] Modules linked in: [0.096071] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc1-yocto-standard #1 [0.096079] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [0.096116] [<80018d8c>] (unwind_backtrace) from [<8001427c>] (show_stack+0x18/0x1c) [0.096138] [<8001427c>] (show_stack) from [<802c85fc>] (dump_stack+0x88/0x9c) [0.096157] [<802c85fc>] (dump_stack) from [<80024d90>] (__warn+0xf4/0x10c) [0.096175] [<80024d90>] (__warn) from [<80024de8>] (warn_slowpath_fmt+0x40/0x50) [0.096194] [<80024de8>] (warn_slowpath_fmt) from [<80176118>] (sysfs_warn_dup+0x70/0x80) [0.096212] [<80176118>] (sysfs_warn_dup) from [<80176204>] (sysfs_create_dir_ns+0x8c/0x9c) [0.096231] [<80176204>] (sysfs_create_dir_ns) from [<802cafd0>] (kobject_add_internal+0xc0/0x360) [0.096249] [<802cafd0>] (kobject_add_internal) from [<802cb2b8>] (kobject_add+0x48/0x98) [0.096269] [<802cb2b8>] (kobject_add) from [<803b90e0>] (device_add+0xf0/0x5a0) [0.096295] [<803b90e0>] (device_add) from [<8051ae20>] (of_platform_device_create_pdata+0x8c/0xc4) [0.096316] [<8051ae20>] (of_platform_device_create_pdata) from [<8051af7c>] (of_platform_bus_create+0x110/0x2a0) [0.096333] [<8051af7c>] (of_platform_bus_create) from [<8051b29c>] (of_platform_populate+0x64/0xb4) [0.096358] [<8051b29c>] (of_platform_populate) from [<808f1a88>] (imx6q_init_machine+0x104/0x2b4) [0.096377] [<808f1a88>] (imx6q_init_machine) from [<808eba7c>] (customize_machine+0x24/0x44) [0.096395] [<808eba7c>] (customize_machine) from [<8000980c>] (do_one_initcall+0x4c/0x174) [0.096414] [<8000980c>] (do_one_initcall) from [<808ead60>] (kernel_init_freeable+0x158/0x1e8) [0.096435] [<808ead60>] (kernel_init_freeable) from [<8062b4b0>] (kernel_init+0x14/0x100) [0.096457] [<8062b4b0>] (kernel_init) from [<800104b8>] (ret_from_fork+0x14/0x3c) [0.096482] ---[ end trace 394e7b4d22c2be44 ]--- [0.096491] [ cut here ] [0.096507] WARNING: CPU: 0 PID: 1 at /home/ian/tx6/yoctomaster/build/tmp/work-shared/tx6u-vid/kernel-source/lib/kobject.c:240 kobject_add_internal+0x2e0/0x360 [0.096518] kobject_add_internal failed for 34.videomux with -EEXIST, don't try to register things with the same name in the same directory. [0.096525] Modules linked in: [0.096539] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.7.0-rc1-yocto-standard #1 [0.096548] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [0.096570] [<80018d8c>] (unwind_backtrace) from [<8001427c>] (show_stack+0x18/0x1c) [0.096585] [<8001427c>] (show_stack) from [<802c85fc>] (dump_stack+0x88/0x9c) [0.096601] [<802c85fc>] (dump_stack) from [<80024d90>] (__warn+0xf4/0x10c) [0.096616] [<80024d90>] (__warn) from [<80024de8>] (warn_slowpath_fmt+0x40/0x50) [0.096632] [<80024de8>] (warn_slowpath_fmt) from [<802cb1f0>] (kobject_add_internal+0x2e0/0x360) [0.096647] [<802cb1f0>] (kobject_add_internal) from [<802cb2b8>] (kobject_add+0x48/0x98) [0.096664] [<802cb2b8>] (kobject_add) from [<803b90e0>] (device_add+0xf0/0x5a0) [0.096681] [<803b90e0>] (device_add) from [<8051ae20>] (of_platform_device_create_pdata+0x8c/0xc4) [0.096700] [<8051ae20>] (of_platform_device_create_pdata) from [<8051af7c>] (of_platform_bus_create+0x110/0x2a0) [0.096716] [<8051af7c>] (of_platform_bus_create) from [<8051b29c>] (of_platform_populate+0x64/0xb4) [0.096735] [<8051b29c>] (of_platform_populate) from [<808f1a88>] (imx6q_init_machine+0x104/0x2b4) [0.096752] [<808f1a88>] (imx6q_init_machine) from [<808eba7c>] (customize_machine+0x24/0x44) [0.096767] [<808eba7c>] (customize_machine) from [<8000980c>] (do_one_initcall+0x4c/0x174) [0.096782] [<8000980c>] (do_one_initcall) from [<808ead60>] (kernel_init_freeable+0x158/0x1e8) [0.096798] [<808ead60>] (kernel_init_freeable) from [<8062b4b0>] (kernel_init+0x14/0x100) [0.096814] [<8062b4b0>] (kernel_init) from [<800104b8>] (r
Re: [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage struct dvb_ca_private
On 06/15/2016 02:15 PM, Max Kellermann wrote: > Don't free the object until the file handle has been closed. Fixes > use-after-free bug which occurs when I disconnect my DVB-S received > while VDR is running. Which file handle? /dev/dvb--- There seems to be a problem in the driver release routine: dvb_ca_en50221_release() routine: kfree(ca->slot_info); dvb_unregister_device(ca->dvbdev); kfree(ca); I think this should be since ioctl references slot info dvb_unregister_device(ca->dvbdev); kfree(ca->slot_info); kfree(ca); I think dvb_ca_en50221_release() and dvb_ca_en50221_io_do_ioctl() should serialize access to ca. dvb_ca_en50221_io_do_ioctl() holds the ioctl_mutex, however, dvb_ca_en50221_release() could happen while ioctl is in progress. Maybe you can try fixing those first. As I mentioned in my review on your 3/3 patch, adding a kref here adds more refcounted objects to the mix. You want to avoid that. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in _destroy()
On 06/15/2016 02:15 PM, Max Kellermann wrote:
> media_gobj_destroy() may be called twice on one instance - once by
> media_device_unregister() and again by dvb_media_device_free(). The
> function media_remove_intf_links() establishes and documents the
> convention that mdev==NULL means that the object is not registered,
> but nobody ever NULLs this variable. So this patch really implements
> this behavior, and adds another mdev==NULL check to
> media_gobj_destroy() to protect against double removal.
Are you seeing null pointer dereference on gobj->mdev? In any case,
we have to look at if there is a missing mutex hold that creates a
race between media_device_unregister() and dvb_media_device_free()
I don't this patch will solve the race condition.
thanks,
-- Shuah
>
> Signed-off-by: Max Kellermann
> ---
> drivers/media/media-entity.c |6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index d8a2299..9526338 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -203,10 +203,16 @@ void media_gobj_destroy(struct media_gobj *gobj)
> {
> dev_dbg_obj(__func__, gobj);
>
> + /* Do nothing if the object is not linked. */
> + if (gobj->mdev == NULL)
> + return;
> +
> gobj->mdev->topology_version++;
>
> /* Remove the object from mdev list */
> list_del(&gobj->list);
> +
> + gobj->mdev = NULL;
> }
>
> int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] [media] rcar-vin: add Gen3 HW registers
Hi Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:06 Niklas Söderlund wrote:
> From: Niklas Söderlund
>
> Add the register needed to work with Gen3 hardware. This patch just adds
> the logic for how to work with the Gen3 hardware. More work is required
> to enable the subdevice structure needed to support capturing.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 99 --
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 -
> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> 3 files changed, 80 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index b3d3c5e..5196395 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -33,21 +33,23 @@
> #define VNELPRC_REG 0x10/* Video n End Line Pre-Clip Register */
> #define VNSPPRC_REG 0x14/* Video n Start Pixel Pre-Clip Register */
> #define VNEPPRC_REG 0x18/* Video n End Pixel Pre-Clip Register */
By the way, that's not directly related to this patch, but hex constants in
the kernel tend to use lowercase letters.
Also, feel free to give a bit more air to the register definitions even if it
makes them cross the 80 characters per line limit, if it helps improving
readability. If it was up to me I'd add one tab before the register address
for instance to align them with the register fields values a bit further
below.
Still from a readability point of view, I would extend the register address to
4 (or just 3 if you prefer) hex digits as we have registers above the first
0x100 bytes.
> -#define VNSLPOC_REG 0x1C/* Video n Start Line Post-Clip Register */
> -#define VNELPOC_REG 0x20/* Video n End Line Post-Clip Register */
> -#define VNSPPOC_REG 0x24/* Video n Start Pixel Post-Clip Register */
> -#define VNEPPOC_REG 0x28/* Video n End Pixel Post-Clip Register */
> #define VNIS_REG 0x2C/* Video n Image Stride Register */
> #define VNMB_REG(m) (0x30 + ((m) << 2)) /* Video n Memory Base m Register
> */
> #define VNIE_REG 0x40/* Video n Interrupt Enable Register */
> #define VNINTS_REG 0x44/* Video n Interrupt Status Register */
> #define VNSI_REG 0x48/* Video n Scanline Interrupt Register */
> #define VNMTC_REG0x4C/* Video n Memory Transfer Control Register */
> -#define VNYS_REG 0x50/* Video n Y Scale Register */
> -#define VNXS_REG 0x54/* Video n X Scale Register */
> #define VNDMR_REG0x58/* Video n Data Mode Register */
> #define VNDMR2_REG 0x5C/* Video n Data Mode Register 2 */
> #define VNUVAOF_REG 0x60/* Video n UV Address Offset Register */
> +
> +/* Register offsets specific for Gen2 */
> +#define VNSLPOC_REG 0x1C/* Video n Start Line Post-Clip Register */
> +#define VNELPOC_REG 0x20/* Video n End Line Post-Clip Register */
> +#define VNSPPOC_REG 0x24/* Video n Start Pixel Post-Clip Register */
> +#define VNEPPOC_REG 0x28/* Video n End Pixel Post-Clip Register */
> +#define VNYS_REG 0x50/* Video n Y Scale Register */
> +#define VNXS_REG 0x54/* Video n X Scale Register */
> #define VNC1A_REG0x80/* Video n Coefficient Set C1A Register */
> #define VNC1B_REG0x84/* Video n Coefficient Set C1B Register */
> #define VNC1C_REG0x88/* Video n Coefficient Set C1C Register */
> @@ -73,9 +75,13 @@
> #define VNC8B_REG0xF4/* Video n Coefficient Set C8B Register */
> #define VNC8C_REG0xF8/* Video n Coefficient Set C8C Register */
>
> +/* Register offsets specific for Gen3 */
> +#define VNCSI_IFMD_REG 0x20 /* Video n CSI2 Interface Mode
> Register */
>
> /* Register bit fields for R-Car VIN */
> /* Video n Main Control Register bits */
> +#define VNMC_DPINE (1 << 27) /* Gen3 specific */
> +#define VNMC_SCLE(1 << 26) /* Gen3 specific */
> #define VNMC_FOC (1 << 21)
> #define VNMC_YCAL(1 << 19)
> #define VNMC_INF_YUV8_BT656 (0 << 16)
> @@ -118,6 +124,12 @@
> #define VNDMR2_FTEV (1 << 17)
> #define VNDMR2_VLV(n)((n & 0xf) << 12)
>
> +/* Video n CSI2 Interface Mode Register (Gen3) */
> +#define VNCSI_IFMD_DES2 (1 << 27)
> +#define VNCSI_IFMD_DES1 (1 << 26)
> +#define VNCSI_IFMD_DES0 (1 << 25)
> +#define VNCSI_IFMD_CSI_CHSEL(n) ((n & 0xf) << 0)
> +
> static void rvin_write(struct rvin_dev *vin, u32 value, u32 offset)
> {
> iowrite32(value, vin->base + offset);
> @@ -196,7 +208,10 @@ static int rvin_setup(struct rvin_dev *vin)
> }
>
> /* Enable VSYNC Field Toogle mode after one VSYNC input */
> - dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
> + if (vin->chip == RCAR_GEN3)
I'm not too fond of scattering this kind of check across the code. My
preferred approach is to add a device info st
Re: [PATCH 7/8] [media] rcar-vin: enable Gen3
Hi Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:08 Niklas Söderlund wrote:
> From: Niklas Söderlund
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/Kconfig | 2 +-
> drivers/media/platform/rcar-vin/rcar-core.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/rcar-vin/Kconfig
> b/drivers/media/platform/rcar-vin/Kconfig index b2ff2d4..ca3ea91 100644
> --- a/drivers/media/platform/rcar-vin/Kconfig
> +++ b/drivers/media/platform/rcar-vin/Kconfig
> @@ -5,7 +5,7 @@ config VIDEO_RCAR_VIN
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Support for Renesas R-Car Video Input (VIN) driver.
> - Supports R-Car Gen2 SoCs.
> + Supports R-Car Gen2 and Gen3 SoCs.
>
> To compile this driver as a module, choose M here: the
> module will be called rcar-vin.
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index d901ad0..520690c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -26,6 +26,7 @@
> #include "rcar-vin.h"
>
> static const struct of_device_id rvin_of_id_table[] = {
> + { .compatible = "renesas,vin-r8a7795", .data = (void *)RCAR_GEN3 },
This isn't needed with patch 8/8. I'd drop this hunk, and merge the previous
one into 8/8.
> { .compatible = "renesas,vin-r8a7794", .data = (void *)RCAR_GEN2 },
> { .compatible = "renesas,vin-r8a7793", .data = (void *)RCAR_GEN2 },
> { .compatible = "renesas,vin-r8a7791", .data = (void *)RCAR_GEN2 },
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] [media] rcar-vin: add Gen2 and Gen3 fallback compatibility strings
Hi Niklas,
Thank you for the patch.
On Wednesday 25 May 2016 21:10:09 Niklas Söderlund wrote:
> From: Niklas Söderlund
>
> These are present in the soc-camera version of this driver and it's time
> to add them to this driver as well.
>
> Signed-off-by: Niklas Söderlund
Acked-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c index 520690c..87041db 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -33,6 +33,8 @@ static const struct of_device_id rvin_of_id_table[] = {
> { .compatible = "renesas,vin-r8a7790", .data = (void *)RCAR_GEN2 },
> { .compatible = "renesas,vin-r8a7779", .data = (void *)RCAR_H1 },
> { .compatible = "renesas,vin-r8a7778", .data = (void *)RCAR_M1 },
> + { .compatible = "renesas,rcar-gen3-vin", .data = (void *)RCAR_GEN3 },
> + { .compatible = "renesas,rcar-gen2-vin", .data = (void *)RCAR_GEN2 },
> { },
> };
> MODULE_DEVICE_TABLE(of, rvin_of_id_table);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/38] i.MX5/6 Video Capture
On 06/16/2016 02:49 AM, Jack Mitchell wrote: > > On 16/06/16 02:37, Steve Longerbeam wrote: >> Hi Jack, >> >> On 06/15/2016 03:43 AM, Jack Mitchell wrote: >>> >>> Trying to use a user pointer rather than mmap also fails and causes a >>> kernel splat. >>> >> >> Hmm, I've tested userptr with the mem2mem driver, but maybe never >> with video capture. I tried "v4l2-ctl -d/dev/video0 --stream-user=8" but >> that returns "VIDIOC_QBUF: failed: Invalid argument", haven't tracked >> down why (could be a bug in v4l2-ctl). Can you share the splat? >> > > On re-checking the splat was the same v4l_cropcap that was mentioned before > so I don't think it's related. The error I get back is: > > VIDIOC_QBUF error 22, Invalid argument > > I'm using the example program the the v4l2 docs [1]. I found the cause at least in my case. After enabling dynamic debug in videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives me [ 468.826046] user data must be aligned to 64 bytes But even getting past that alignment issue, I've only tested userptr (in mem2mem driver) by giving the driver a user address of a mmap'ed kernel contiguous buffer. A true discontiguous user buffer may not work, the IPU DMA does not support scatter-gather. Steve -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] [PATCH 2/3] staging: media: omap1: convert to videobuf2
Created and tested on Amstrad Delta on top of Linux-4.7-rc3 with
"staging: media: omap1: drop videobuf-dma-sg mode" applied.
Signed-off-by: Janusz Krzysztofik
---
drivers/staging/media/omap1/Kconfig| 2 +-
drivers/staging/media/omap1/omap1_camera.c | 363 -
2 files changed, 151 insertions(+), 214 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index e2a39f5..12f1d7a 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -3,7 +3,7 @@ config VIDEO_OMAP1
depends on VIDEO_DEV && SOC_CAMERA
depends on ARCH_OMAP1
depends on HAS_DMA
- select VIDEOBUF_DMA_CONTIG
+ select VIDEOBUF2_DMA_CONTIG
---help---
This is a v4l2 driver for the TI OMAP1 camera interface
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 37ef4da..3761660 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -1,7 +1,7 @@
/*
* V4L2 SoC Camera driver for OMAP1 Camera Interface
*
- * Copyright (C) 2010, Janusz Krzysztofik
+ * Copyright (C) 2010, 2016 Janusz Krzysztofik
*
* Based on V4L2 Driver for i.MXL/i.MXL camera (CSI) host
* Copyright (C) 2008, Paulius Zaleckas
@@ -31,13 +31,13 @@
#include
#include
#include
-#include
+#include
#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.3"
+#define DRIVER_VERSION "0.0.4"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -134,9 +134,8 @@
/* buffer for one video frame */
struct omap1_cam_buf {
- struct videobuf_buffer vb;
- u32 code;
- int inwork;
+ struct vb2_v4l2_buffer vb;
+ struct list_headqueue;
};
struct omap1_cam_dev {
@@ -161,10 +160,18 @@ struct omap1_cam_dev {
struct omap1_cam_buf*active;
struct omap1_cam_buf*ready;
+ struct vb2_alloc_ctx*alloc_ctx;
+ int sequence;
+
u32 reg_cache[0];
};
+static struct omap1_cam_buf *vb2_to_omap1_cam_buf(struct vb2_v4l2_buffer *vbuf)
+{
+ return container_of(vbuf, struct omap1_cam_buf, vb);
+}
+
static void cam_write(struct omap1_cam_dev *pcdev, u16 reg, u32 val)
{
pcdev->reg_cache[reg / sizeof(u32)] = val;
@@ -187,92 +194,59 @@ static u32 cam_read(struct omap1_cam_dev *pcdev, u16 reg,
bool from_cache)
/*
* Videobuf operations
*/
-static int omap1_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
- unsigned int *size)
+
+static int omap1_videobuf_setup(struct vb2_queue *vq, unsigned int *count,
+ unsigned int *num_planes, unsigned int sizes[],
+ void *alloc_ctxs[])
{
- struct soc_camera_device *icd = vq->priv_data;
+ struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ struct omap1_cam_dev *pcdev = ici->priv;
+ unsigned int size = icd->sizeimage;
+
+ pcdev->sequence = 0;
- *size = icd->sizeimage;
+ *num_planes = 1;
+ sizes[0] = size;
+ alloc_ctxs[0] = pcdev->alloc_ctx;
if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT)
*count = OMAP1_CAMERA_MIN_BUF_COUNT;
- if (*size * *count > MAX_VIDEO_MEM * 1024 * 1024)
- *count = (MAX_VIDEO_MEM * 1024 * 1024) / *size;
+ if (size * *count > MAX_VIDEO_MEM * 1024 * 1024)
+ *count = (MAX_VIDEO_MEM * 1024 * 1024) / size;
dev_dbg(icd->parent,
- "%s: count=%d, size=%d\n", __func__, *count, *size);
+ "%s: count=%u, size=%u\n", __func__, *count, size);
return 0;
}
-static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf)
-{
- struct videobuf_buffer *vb = &buf->vb;
-
- BUG_ON(in_interrupt());
-
- videobuf_waiton(vq, vb, 0, 0);
-
- videobuf_dma_contig_free(vq, vb);
-
- vb->state = VIDEOBUF_NEEDS_INIT;
-}
-
-static int omap1_videobuf_prepare(struct videobuf_queue *vq,
- struct videobuf_buffer *vb, enum v4l2_field field)
+static int omap1_videobuf_prepare(struct vb2_buffer *vb)
{
- struct soc_camera_device *icd = vq->priv_data;
- struct omap1_cam_buf *buf = container_of(vb, struct omap1_cam_buf, vb);
- int ret;
-
- WARN_ON(!list_empty(&vb->queue));
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
+ struct omap1_cam_buf *buf = vb2_to_omap1_cam_buf(vbuf);
+ unsigned long size = icd->sizeimage;
- BUG_ON(NULL == icd->current_fmt);
+ W
[RFC] [PATCH 3/3] staging: media: omap1: use dmaengine
Created and tested on Amstrad Delta on top of Linux-4.7-rc3 with
"staging: media: omap1: convert to videobuf2" applied.
Signed-off-by: Janusz Krzysztofik
---
drivers/staging/media/omap1/Kconfig| 2 +-
drivers/staging/media/omap1/omap1_camera.c | 432 +
2 files changed, 135 insertions(+), 299 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index 12f1d7a..0b8456d 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -1,7 +1,7 @@
config VIDEO_OMAP1
tristate "OMAP1 Camera Interface driver"
depends on VIDEO_DEV && SOC_CAMERA
- depends on ARCH_OMAP1
+ depends on ARCH_OMAP1 && DMA_OMAP
depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
---help---
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 3761660..e22ba8a 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -33,11 +33,12 @@
#include
#include
-#include
+#include
+#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.4"
+#define DRIVER_VERSION "0.0.5"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -115,8 +116,8 @@
#define DMA_BURST_SHIFT(1 + OMAP_DMA_DATA_BURST_4)
#define DMA_BURST_SIZE BIT(DMA_BURST_SHIFT)
-#define DMA_ELEMENT_SHIFT OMAP_DMA_DATA_TYPE_S32
-#define DMA_ELEMENT_SIZE BIT(DMA_ELEMENT_SHIFT)
+#define DMA_ELEMENT_SIZE DMA_SLAVE_BUSWIDTH_4_BYTES
+#define DMA_ELEMENT_SHIFT __fls(DMA_ELEMENT_SIZE)
#define DMA_FRAME_SHIFT(FIFO_SHIFT - 1)
#define DMA_FRAME_SIZE BIT(DMA_FRAME_SHIFT)
@@ -124,7 +125,7 @@
#define THRESHOLD_LEVELDMA_FRAME_SIZE
#define OMAP1_CAMERA_MIN_BUF_COUNT \
- 3
+ 2
#define MAX_VIDEO_MEM 4 /* arbitrary video memory limit in MB */
@@ -145,7 +146,8 @@ struct omap1_cam_dev {
unsigned intirq;
void __iomem*base;
- int dma_ch;
+ struct dma_chan *dma_chan;
+ unsigned intdma_rq;
struct omap1_cam_platform_data *pdata;
unsigned long pflags;
@@ -156,10 +158,6 @@ struct omap1_cam_dev {
/* lock used to protect videobuf */
spinlock_t lock;
- /* Pointers to DMA buffers */
- struct omap1_cam_buf*active;
- struct omap1_cam_buf*ready;
-
struct vb2_alloc_ctx*alloc_ctx;
int sequence;
@@ -222,6 +220,16 @@ static int omap1_videobuf_setup(struct vb2_queue *vq,
unsigned int *count,
return 0;
}
+static int omap1_videobuf_init(struct vb2_buffer *vb)
+{
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct omap1_cam_buf *buf = vb2_to_omap1_cam_buf(vbuf);
+
+ INIT_LIST_HEAD(&buf->queue);
+
+ return 0;
+}
+
static int omap1_videobuf_prepare(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -236,96 +244,27 @@ static int omap1_videobuf_prepare(struct vb2_buffer *vb)
vb2_plane_size(vb, 0), size);
return -ENOBUFS;
}
-
vb2_set_plane_payload(vb, 0, size);
return 0;
}
-static void set_dma_dest_params(int dma_ch, struct omap1_cam_buf *buf)
-{
- dma_addr_t dma_addr =
- vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
- unsigned int block_size = vb2_plane_size(&buf->vb.vb2_buf, 0);
-
- omap_set_dma_dest_params(dma_ch,
- OMAP_DMA_PORT_EMIFF, OMAP_DMA_AMODE_POST_INC, dma_addr, 0, 0);
- omap_set_dma_transfer_params(dma_ch,
- OMAP_DMA_DATA_TYPE_S32, DMA_FRAME_SIZE,
- block_size >> (DMA_FRAME_SHIFT + DMA_ELEMENT_SHIFT),
- DMA_SYNC, 0, 0);
-}
-
-static struct omap1_cam_buf *prepare_next_vb(struct omap1_cam_dev *pcdev)
-{
- struct omap1_cam_buf *buf;
-
- /*
-* If there is already a buffer pointed out by the pcdev->ready,
-* (re)use it, otherwise try to fetch and configure a new one.
-*/
- buf = pcdev->ready;
- if (!buf) {
- if (list_empty(&pcdev->capture))
- return buf;
- buf = list_entry(pcdev->capture.next,
- struct omap1_cam_buf, queue);
- pcdev->ready = buf;
- list_del_init(&buf->queue);
- }
-
- /*
-* In CONTIG mode, we can safely enter next buffer parameters
-* into the DMA programming register set after the DMA
-* has already been activated on the previous buffer
-*/
- set_d
[RFC] [PATCH 1/3] staging: media: omap1: drop videobuf-dma-sg mode
For over 20 last kernel versions the driver has been able to allocate
DMA buffers in videobuf-dma-contig mode without any issues. Drop the
no longer needed sg mode in preparation for conversion to videobuf2.
Created and tested on Amstrad Delta against Linux-4.7-rc3 with
omap1_camera and ov6650 fixes applied.
Signed-off-by: Janusz Krzysztofik
---
drivers/staging/media/omap1/Kconfig | 1 -
drivers/staging/media/omap1/omap1_camera.c | 445 ---
include/linux/platform_data/media/omap1_camera.h | 9 -
3 files changed, 77 insertions(+), 378 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index 6cfab3a..e2a39f5 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -4,7 +4,6 @@ config VIDEO_OMAP1
depends on ARCH_OMAP1
depends on HAS_DMA
select VIDEOBUF_DMA_CONTIG
- select VIDEOBUF_DMA_SG
---help---
This is a v4l2 driver for the TI OMAP1 camera interface
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 9b6140a..37ef4da 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -32,13 +32,12 @@
#include
#include
#include
-#include
#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.2"
+#define DRIVER_VERSION "0.0.3"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -114,22 +113,18 @@
#define FIFO_SHIFT __fls(FIFO_SIZE)
#define DMA_BURST_SHIFT(1 + OMAP_DMA_DATA_BURST_4)
-#define DMA_BURST_SIZE (1 << DMA_BURST_SHIFT)
+#define DMA_BURST_SIZE BIT(DMA_BURST_SHIFT)
#define DMA_ELEMENT_SHIFT OMAP_DMA_DATA_TYPE_S32
-#define DMA_ELEMENT_SIZE (1 << DMA_ELEMENT_SHIFT)
+#define DMA_ELEMENT_SIZE BIT(DMA_ELEMENT_SHIFT)
-#define DMA_FRAME_SHIFT_CONTIG (FIFO_SHIFT - 1)
-#define DMA_FRAME_SHIFT_SG DMA_BURST_SHIFT
-
-#define DMA_FRAME_SHIFT(x) ((x) == OMAP1_CAM_DMA_CONTIG ? \
- DMA_FRAME_SHIFT_CONTIG : \
- DMA_FRAME_SHIFT_SG)
-#define DMA_FRAME_SIZE(x) (1 << DMA_FRAME_SHIFT(x))
+#define DMA_FRAME_SHIFT(FIFO_SHIFT - 1)
+#define DMA_FRAME_SIZE BIT(DMA_FRAME_SHIFT)
#define DMA_SYNC OMAP_DMA_SYNC_FRAME
#define THRESHOLD_LEVELDMA_FRAME_SIZE
-
+#define OMAP1_CAMERA_MIN_BUF_COUNT \
+ 3
#define MAX_VIDEO_MEM 4 /* arbitrary video memory limit in MB */
@@ -140,12 +135,8 @@
/* buffer for one video frame */
struct omap1_cam_buf {
struct videobuf_buffer vb;
- u32 code;
+ u32 code;
int inwork;
- struct scatterlist *sgbuf;
- int sgcount;
- int bytes_left;
- enum videobuf_state result;
};
struct omap1_cam_dev {
@@ -170,11 +161,6 @@ struct omap1_cam_dev {
struct omap1_cam_buf*active;
struct omap1_cam_buf*ready;
- enum omap1_cam_vb_mode vb_mode;
- int (*mmap_mapper)(struct videobuf_queue *q,
- struct videobuf_buffer *buf,
- struct vm_area_struct *vma);
-
u32 reg_cache[0];
};
@@ -205,13 +191,11 @@ static int omap1_videobuf_setup(struct videobuf_queue
*vq, unsigned int *count,
unsigned int *size)
{
struct soc_camera_device *icd = vq->priv_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct omap1_cam_dev *pcdev = ici->priv;
*size = icd->sizeimage;
- if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode))
- *count = OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode);
+ if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT)
+ *count = OMAP1_CAMERA_MIN_BUF_COUNT;
if (*size * *count > MAX_VIDEO_MEM * 1024 * 1024)
*count = (MAX_VIDEO_MEM * 1024 * 1024) / *size;
@@ -222,8 +206,7 @@ static int omap1_videobuf_setup(struct videobuf_queue *vq,
unsigned int *count,
return 0;
}
-static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf,
- enum omap1_cam_vb_mode vb_mode)
+static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf)
{
struct videobuf_buffer *vb = &buf->vb;
@@ -231,16 +214,7 @@ static void free_buffer(struct videobuf_queue *vq, struct
omap1_cam_buf *buf,
videobuf_waiton(vq, vb, 0, 0);
- if (vb_mode == OMAP1_CAM_DMA_CONTIG) {
- videobuf_dma_contig_free(
[RFC] [PATCH 0/3] media: an attempt to refresh omap1_camera driver
As requested by media subsystem maintainers, here is an attempt to convert the omap1_camera driver to the vb2 framework. Also, conversion to the dmaengine framework, long awaited by ARM/OMAP maintainers, is done. Next, I'm going to approach removal of soc-camera dependency. Please let me know how much time I have for that, i.e., when the soc-camera framework is going to be depreciated. Thanks, Janusz Janusz Krzysztofik (3): staging: media: omap1: drop videobuf-dma-sg mode staging: media: omap1: convert to videobuf2 staging: media: omap1: use dmaengine drivers/staging/media/omap1/Kconfig | 5 +- drivers/staging/media/omap1/omap1_camera.c | 948 +-- include/linux/platform_data/media/omap1_camera.h | 9 - 3 files changed, 217 insertions(+), 745 deletions(-) -- 2.7.3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/38] i.MX5/6 Video Capture
Le jeudi 16 juin 2016 à 10:02 -0700, Steve Longerbeam a écrit : > I found the cause at least in my case. After enabling dynamic debug in > videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives > me > > [ 468.826046] user data must be aligned to 64 bytes > > > > But even getting past that alignment issue, I've only tested userptr (in > mem2mem > driver) by giving the driver a user address of a mmap'ed kernel contiguous > buffer. A true discontiguous user buffer may not work, the IPU DMA does not > support scatter-gather. If it's dma-contig, you'll need page aligned and contiguous memory. What some test application do when testing their driver with that, is to allocate memory using another device, or m2m device, that uses the same allocator. regards, Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/38] i.MX5/6 Video Capture
Le jeudi 16 juin 2016 à 10:02 -0700, Steve Longerbeam a écrit : > I found the cause at least in my case. After enabling dynamic debug in > videobuf2-dma-contig.c, "v4l2-ctl -d/dev/video0 --stream-user=8" gives > me > > [ 468.826046] user data must be aligned to 64 bytes > > > > But even getting past that alignment issue, I've only tested userptr (in > mem2mem > driver) by giving the driver a user address of a mmap'ed kernel contiguous > buffer. A true discontiguous user buffer may not work, the IPU DMA does not > support scatter-gather. If it's dma-contig, you'll need page aligned and contiguous memory. What some test application do when testing their driver with that, is to allocate memory using another device, or m2m device, that uses the same allocator. regards, Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage struct dvb_ca_private
On 2016/06/16 18:06, Shuah Khan wrote: > On 06/15/2016 02:15 PM, Max Kellermann wrote: > > Don't free the object until the file handle has been closed. Fixes > > use-after-free bug which occurs when I disconnect my DVB-S received > > while VDR is running. > > Which file handle? /dev/dvb--- I don't know which one triggers it. I get crashes with VDR, and VDR opens all of them (ca0, demux0, frontend0), but won't release the file handles even if they become defunct. Only restarting the VDR process leads to recovery (or crash). > I think dvb_ca_en50221_release() and dvb_ca_en50221_io_do_ioctl() > should serialize access to ca. dvb_ca_en50221_io_do_ioctl() holds > the ioctl_mutex, however, dvb_ca_en50221_release() could happen while > ioctl is in progress. Maybe you can try fixing those first. True, there are LOTS of race conditions in the DVB code. I see them everywhere. But that's orthogonal to my patch, isn't it? > As I mentioned in my review on your 3/3 patch, adding a kref here > adds more refcounted objects to the mix. You want to avoid that. Mauro asked me to add the kref. What is your suggestion to fix the use-after-free bug? I have a problem here, as mentioned in my last email: I don't know how all of this is supposed to be, how it was designed; all I see is bugs inside strange code, and I have to guess the previous author's intentions and try to do the best to fix the code. Max -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in _destroy()
On 2016/06/16 18:24, Shuah Khan wrote: > On 06/15/2016 02:15 PM, Max Kellermann wrote: > > media_gobj_destroy() may be called twice on one instance - once by > > media_device_unregister() and again by dvb_media_device_free(). The > > function media_remove_intf_links() establishes and documents the > > convention that mdev==NULL means that the object is not registered, > > but nobody ever NULLs this variable. So this patch really implements > > this behavior, and adds another mdev==NULL check to > > media_gobj_destroy() to protect against double removal. > > Are you seeing null pointer dereference on gobj->mdev? In any case, > we have to look at if there is a missing mutex hold that creates a > race between media_device_unregister() and dvb_media_device_free() > > I don't this patch will solve the race condition. I think we misunderstand. This is not about a race condition. And the problem cannot be a NULL pointer dereference. That's because nobody NULLs the pointer! Pointer NULLing is what my patch adds, and AFTER my patch, there may be NULL pointer dereferences (if there are more previously existing bugs, which we should fix as well). I added this NULL assignment because there are NULL checks - and if nobody NULLs the pointer, that check doesn't make any sense! Max -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in _destroy()
On 06/16/2016 12:43 PM, Max Kellermann wrote: > On 2016/06/16 18:24, Shuah Khan wrote: >> On 06/15/2016 02:15 PM, Max Kellermann wrote: >>> media_gobj_destroy() may be called twice on one instance - once by >>> media_device_unregister() and again by dvb_media_device_free(). The >>> function media_remove_intf_links() establishes and documents the >>> convention that mdev==NULL means that the object is not registered, >>> but nobody ever NULLs this variable. So this patch really implements >>> this behavior, and adds another mdev==NULL check to >>> media_gobj_destroy() to protect against double removal. >> >> Are you seeing null pointer dereference on gobj->mdev? In any case, >> we have to look at if there is a missing mutex hold that creates a >> race between media_device_unregister() and dvb_media_device_free() >> >> I don't this patch will solve the race condition. > > I think we misunderstand. This is not about a race condition. And > the problem cannot be a NULL pointer dereference. > > That's because nobody NULLs the pointer! I see 7 calls to media_gobj_destroy(). In 6 cases, calling routines fee the pointer that contains the graph_obj. __media_device_unregister_entity() sets mdev ot null. entity->graph_obj.mdev = NULL; That is why I am confused when you say it never set to null. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv16 05/13] cec/TODO: add TODO file so we know why this is still in staging
Em Fri, 29 Apr 2016 15:52:20 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > Explain why cec.c is still in staging. Hmm... as this is for staging, even having pointed several things to be improved, I may end merging this series. Will decide after finishing the patch review. > > Signed-off-by: Hans Verkuil > --- > drivers/staging/media/cec/TODO | 13 + > 1 file changed, 13 insertions(+) > create mode 100644 drivers/staging/media/cec/TODO > > diff --git a/drivers/staging/media/cec/TODO b/drivers/staging/media/cec/TODO > new file mode 100644 > index 000..c0751ef > --- /dev/null > +++ b/drivers/staging/media/cec/TODO > @@ -0,0 +1,13 @@ > +The reason why cec.c is still in staging is that I would like > +to have a bit more confidence in the uABI. The kABI is fine, > +no problem there, but I would like to let the public API mature > +a bit. > + > +Once I'm confident that I didn't miss anything then the cec.c source > +can move to drivers/media and the linux/cec.h and linux/cec-funcs.h > +headers can move to uapi/linux and added to uapi/linux/Kbuild to make > +them public. > + > +Hopefully this will happen later in 2016. > + > +Hans Verkuil Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv16 07/13] cec.txt: add CEC framework documentation
Em Fri, 29 Apr 2016 15:52:22 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > Document the new HDMI CEC framework. As we'll be moving documentation to Sphinx/Rst, it would be good if you could make it work fine with sphinx, as this will likely be needed for Kernel 4.9. Right now it most works, although several warnings are produced: /devel/v4l/patchwork/tmp/cec.txt:40: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:40: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:40: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:40: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:40: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:42: WARNING: Definition list ends without a blank line; unexpected unindent. /devel/v4l/patchwork/tmp/cec.txt:42: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:67: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:71: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:92: ERROR: Unexpected indentation. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:87: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:92: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:93: WARNING: Block quote ends without a blank line; unexpected unindent. /devel/v4l/patchwork/tmp/cec.txt:93: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:93: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:95: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:97: WARNING: Definition list ends without a blank line; unexpected unindent. /devel/v4l/patchwork/tmp/cec.txt:105: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:105: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:118: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:118: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:130: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:130: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:144: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:144: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:144: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:161: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:161: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:161: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:173: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:194: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:203: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:203: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:214: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:217: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:217: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:217: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:217: WARNING: Inline emphasis start-string without end-string. /devel/v4l/patchwork/tmp/cec.txt:219: WARNING: Definition list ends without a blank line; unexpected unindent. /devel/v4l/patchwork/tmp/cec.txt:224: WARNING: Inline emphasis start-string without end-s
Re: [PATCHv16 08/13] DocBook/media: add CEC documentation
Em Fri, 29 Apr 2016 15:52:23 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > Add DocBook documentation for the CEC API. Please always send the documentation patch *before* the code, in order to make easier to do the code review. > > Signed-off-by: Hans Verkuil > [k.deb...@samsung.com: add documentation for passthrough mode] > [k.deb...@samsung.com: minor fixes and change of reserved field sizes] > Signed-off-by: Kamil Debski > Signed-off-by: Hans Verkuil > --- > Documentation/DocBook/device-drivers.tmpl | 4 + > Documentation/DocBook/media/Makefile | 2 + > Documentation/DocBook/media/v4l/biblio.xml | 10 + > Documentation/DocBook/media/v4l/cec-api.xml| 72 + > Documentation/DocBook/media/v4l/cec-func-close.xml | 59 > Documentation/DocBook/media/v4l/cec-func-ioctl.xml | 73 + > Documentation/DocBook/media/v4l/cec-func-open.xml | 94 ++ > Documentation/DocBook/media/v4l/cec-func-poll.xml | 89 ++ > .../DocBook/media/v4l/cec-ioc-adap-g-caps.xml | 140 + > .../DocBook/media/v4l/cec-ioc-adap-g-log-addrs.xml | 324 > + > .../DocBook/media/v4l/cec-ioc-adap-g-phys-addr.xml | 82 ++ > .../DocBook/media/v4l/cec-ioc-dqevent.xml | 190 > Documentation/DocBook/media/v4l/cec-ioc-g-mode.xml | 245 > .../DocBook/media/v4l/cec-ioc-receive.xml | 260 + > Documentation/DocBook/media_api.tmpl | 6 +- > 15 files changed, 1649 insertions(+), 1 deletion(-) > create mode 100644 Documentation/DocBook/media/v4l/cec-api.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-func-close.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-func-ioctl.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-func-open.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-func-poll.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-adap-g-caps.xml > create mode 100644 > Documentation/DocBook/media/v4l/cec-ioc-adap-g-log-addrs.xml > create mode 100644 > Documentation/DocBook/media/v4l/cec-ioc-adap-g-phys-addr.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-dqevent.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-mode.xml > create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-receive.xml Hmm... as CEC is at staging, I'm not very comfortable on having it documented there as-is, as the documentation is expected to be for non-staging stuff. Maybe the best would be to add, on every xml above, a note saying that this is a *proposed* documentation for a feature yet to be accepted. > > diff --git a/Documentation/DocBook/device-drivers.tmpl > b/Documentation/DocBook/device-drivers.tmpl > index 893b2ca..31258bf 100644 > --- a/Documentation/DocBook/device-drivers.tmpl > +++ b/Documentation/DocBook/device-drivers.tmpl > @@ -270,6 +270,10 @@ X!Isound/sound_firmware.c > !Iinclude/media/media-devnode.h > !Iinclude/media/media-entity.h > > + Consumer Electronics Control devices > +!Iinclude/media/cec.h > +!Iinclude/media/cec-edid.h > + For the same reason, I would not add those yet. > > > > diff --git a/Documentation/DocBook/media/Makefile > b/Documentation/DocBook/media/Makefile > index 2840ff4..fdc1386 100644 > --- a/Documentation/DocBook/media/Makefile > +++ b/Documentation/DocBook/media/Makefile > @@ -64,6 +64,7 @@ IOCTLS = \ > $(shell perl -ne 'print "$$1 " if /\#define\s+([A-Z][^\s]+)\s+_IO/' > $(srctree)/include/uapi/linux/dvb/net.h) \ > $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' > $(srctree)/include/uapi/linux/dvb/video.h) \ > $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' > $(srctree)/include/uapi/linux/media.h) \ > + $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' > $(srctree)/include/linux/cec.h) \ > $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' > $(srctree)/include/uapi/linux/v4l2-subdev.h) \ > > DEFINES = \ > @@ -100,6 +101,7 @@ STRUCTS = \ > $(shell perl -ne 'print "$$1 " if (/^struct\s+([^\s]+)\s+/ && !/_old/)' > $(srctree)/include/uapi/linux/dvb/net.h) \ > $(shell perl -ne 'print "$$1 " if (/^struct\s+([^\s]+)\s+/)' > $(srctree)/include/uapi/linux/dvb/video.h) \ > $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' > $(srctree)/include/uapi/linux/media.h) \ > + $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' > $(srctree)/include/linux/cec.h) \ > $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' > $(srctree)/include/uapi/linux/v4l2-subdev.h) \ > $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' > $(srctree)/include/uapi/linux/v4l2-mediabus.h) > > diff --git a/Documentation/DocBook/media/v4l/biblio.xml > b/Documentation/DocBook/media/v4l/biblio.xml > index 9beb30f..87f1d24 100644 > --- a/Documentation/DocBook/media/v4l/biblio.xml > +
Re: [PATCHv16 09/13] cec: adv7604: add cec support.
Em Fri, 29 Apr 2016 15:52:24 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add CEC support to the adv7604 driver.
>
> Signed-off-by: Hans Verkuil
> [k.deb...@samsung.com: Merged changes from CEC Updates commit by Hans Verkuil]
> [k.deb...@samsung.com: add missing methods cec/io_write_and_or]
> [k.deb...@samsung.com: change adv7604 to adv76xx in added functions]
> [hansv...@cisco.com: use _clr_set instead of _and_or]
> ---
> drivers/media/i2c/Kconfig | 9 ++
> drivers/media/i2c/adv7604.c | 332
> +++-
> 2 files changed, 305 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 993dc50..cba1fc7 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -209,6 +209,7 @@ config VIDEO_ADV7604
> depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> depends on GPIOLIB || COMPILE_TEST
> select HDMI
> + select MEDIA_CEC_EDID
Same note for all CEC drivers: don't do that! CEC is in staging. We
should not have any code outside staging depending or selecting CEC.
Instead, put all CEC dependent code inside #ifdefs.
> ---help---
> Support for the Analog Devices ADV7604 video decoder.
>
> @@ -218,6 +219,14 @@ config VIDEO_ADV7604
> To compile this driver as a module, choose M here: the
> module will be called adv7604.
>
> +config VIDEO_ADV7604_CEC
> + bool "Enable Analog Devices ADV7604 CEC support"
> + depends on VIDEO_ADV7604 && MEDIA_CEC
> + default y
Remove "default y". CEC is experimental. Users should explicitly
agree adding experimental code that depends on staging stuff.
> + ---help---
> + When selected the adv7604 will support the optional
> + HDMI CEC feature.
Please add a notice that CEC is an experimental feature under
staging and may be changed or even removed in the future.
We don't want any application to use it yet, as we're still
experimenting with it.
For the rest of the code below, and for patches 10-13: all the above
applies to them.
> +
> config VIDEO_ADV7842
> tristate "Analog Devices ADV7842 decoder"
> depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index beb2841..f462585 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -40,6 +40,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -80,6 +81,8 @@ MODULE_LICENSE("GPL");
>
> #define ADV76XX_OP_SWAP_CB_CR(1 << 0)
>
> +#define ADV76XX_MAX_ADDRS (3)
> +
> enum adv76xx_type {
> ADV7604,
> ADV7611,
> @@ -184,6 +187,12 @@ struct adv76xx_state {
> u16 spa_port_a[2];
> struct v4l2_fract aspect_ratio;
> u32 rgb_quantization_range;
> +
> + struct cec_adapter *cec_adap;
> + u8 cec_addr[ADV76XX_MAX_ADDRS];
> + u8 cec_valid_addrs;
> + bool cec_enabled_adap;
> +
> struct workqueue_struct *work_queues;
> struct delayed_work delayed_work_enable_hotplug;
> bool restart_stdi_once;
> @@ -381,7 +390,8 @@ static inline int io_write(struct v4l2_subdev *sd, u8
> reg, u8 val)
> return regmap_write(state->regmap[ADV76XX_PAGE_IO], reg, val);
> }
>
> -static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask,
> u8 val)
> +static inline int io_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask,
> +u8 val)
> {
> return io_write(sd, reg, (io_read(sd, reg) & ~mask) | val);
> }
> @@ -414,6 +424,12 @@ static inline int cec_write(struct v4l2_subdev *sd, u8
> reg, u8 val)
> return regmap_write(state->regmap[ADV76XX_PAGE_CEC], reg, val);
> }
>
> +static inline int cec_write_clr_set(struct v4l2_subdev *sd, u8 reg, u8 mask,
> +u8 val)
> +{
> + return cec_write(sd, reg, (cec_read(sd, reg) & ~mask) | val);
> +}
> +
> static inline int infoframe_read(struct v4l2_subdev *sd, u8 reg)
> {
> struct adv76xx_state *state = to_state(sd);
> @@ -872,9 +888,9 @@ static int adv76xx_s_detect_tx_5v_ctrl(struct v4l2_subdev
> *sd)
> {
> struct adv76xx_state *state = to_state(sd);
> const struct adv76xx_chip_info *info = state->info;
> + u16 cable_det = info->read_cable_det(sd);
>
> - return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl,
> - info->read_cable_det(sd));
> + return v4l2_ctrl_s_ctrl(state->detect_tx_5v_ctrl, cable_det);
> }
>
> static int find_and_set_predefined_video_timings(struct v4l2_subdev *sd,
> @@ -1900,6 +1916,210 @@ static int adv76xx_set_format(struct v4l2_subdev *sd,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_VIDEO_ADV7604_CEC)
> +static void adv76xx_cec_tx_raw_status(struct v4l2_subdev *sd, u8
> tx_raw_status)
> +{
> + struct adv76xx_state *state = to_state(sd);
> +
> + if ((cec_read(sd, 0
Re: [PATCHv16 10/13] cec: adv7842: add cec support
Em Fri, 29 Apr 2016 15:52:25 +0200 Hans Verkuil escreveu: > From: Hans Verkuil > > Add CEC support to the adv7842 driver. > > Signed-off-by: Hans Verkuil Won't review patches 10-13, as the same reviews I made for patch 9 very likely applies. As this series is causing non-staging drivers to be dependent of a staging driver, I'll wait for the next version that should be solving this issue. For the new 9-13 patches, please be sure that checkpatch will be happy. For the staging stuff, the checkpatch issues can be solved later, as I'll re-check against checkpatch when it moves from staging to mainstream. Regards, Mauro Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] [media] s5p-mfc: improve v4l2_capability driver and card fields
According to the V4L2 documentation the driver and card fields should be
used to identify the driver and the device but the s5p-mfc driver fills
those field using the platform device name, which in turn is the name of
the device DT node.
So not only the filled information isn't correct but also the same values
are used in all the fields for both the encoder and decoder video devices.
Before this patch:
Driver Info (not using libv4l2):
Driver name : 1100.codec
Card type : 1100.codec
Bus info : platform:1100.codec
Driver version: 4.7.0
Driver Info (not using libv4l2):
Driver name : 1100.codec
Card type : 1100.codec
Bus info : platform:1100.codec
Driver version: 4.7.0
After this patch:
Driver Info (not using libv4l2):
Driver name : s5p-mfc
Card type : s5p-mfc-dec
Bus info : platform:1100.codec
Driver version: 4.7.0
Driver Info (not using libv4l2):
Driver name : s5p-mfc
Card type : s5p-mfc-enc
Bus info : platform:1100.codec
Driver version: 4.7.0
Signed-off-by: Javier Martinez Canillas
---
drivers/media/platform/s5p-mfc/s5p_mfc.c| 1 -
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 ++
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 4 ++--
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 4 ++--
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 6ee620ee8cd5..a936f89fa54a 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -35,7 +35,6 @@
#include "s5p_mfc_cmd.h"
#include "s5p_mfc_pm.h"
-#define S5P_MFC_NAME "s5p-mfc"
#define S5P_MFC_DEC_NAME "s5p-mfc-dec"
#define S5P_MFC_ENC_NAME "s5p-mfc-enc"
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 9eb2481ec292..a10dcd244ff0 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -25,6 +25,8 @@
#include "regs-mfc.h"
#include "regs-mfc-v8.h"
+#define S5P_MFC_NAME "s5p-mfc"
+
/* Definitions related to MFC memory */
/* Offset base used to differentiate between CAPTURE and OUTPUT
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 4a40df22fd63..5793b0d8ee0c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -265,8 +265,8 @@ static int vidioc_querycap(struct file *file, void *priv,
{
struct s5p_mfc_dev *dev = video_drvdata(file);
- strncpy(cap->driver, dev->plat_dev->name, sizeof(cap->driver) - 1);
- strncpy(cap->card, dev->plat_dev->name, sizeof(cap->card) - 1);
+ strncpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver) - 1);
+ strncpy(cap->card, dev->vfd_dec->name, sizeof(cap->card) - 1);
snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
dev_name(&dev->plat_dev->dev));
/*
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index dd466ea6429e..1220559d4874 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -943,8 +943,8 @@ static int vidioc_querycap(struct file *file, void *priv,
{
struct s5p_mfc_dev *dev = video_drvdata(file);
- strncpy(cap->driver, dev->plat_dev->name, sizeof(cap->driver) - 1);
- strncpy(cap->card, dev->plat_dev->name, sizeof(cap->card) - 1);
+ strncpy(cap->driver, S5P_MFC_NAME, sizeof(cap->driver) - 1);
+ strncpy(cap->card, dev->vfd_enc->name, sizeof(cap->card) - 1);
snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
dev_name(&dev->plat_dev->dev));
/*
--
2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] [media] gsc-m2m: add device name sufix to bus_info capatiliby field
The driver doesn't set the device in the struct v4l2_capability bus_info
field so v4l2-compliance reports the following error for VIDIOC_QUERYCAP:
Required ioctls:
fail: v4l2-compliance.cpp(537): missing bus_info prefix
('platform')
test VIDIOC_QUERYCAP: FAIL
This patch fixes this by filling also the device besides the bus.
Signed-off-by: Javier Martinez Canillas
---
drivers/media/platform/exynos-gsc/gsc-m2m.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c
b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index a600e32e2543..af81383086b8 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -281,7 +281,8 @@ static int gsc_m2m_querycap(struct file *file, void *fh,
strlcpy(cap->driver, gsc->pdev->name, sizeof(cap->driver));
strlcpy(cap->card, gsc->pdev->name, sizeof(cap->card));
- strlcpy(cap->bus_info, "platform", sizeof(cap->bus_info));
+ snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+dev_name(&gsc->pdev->dev));
cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M_MPLANE |
V4L2_CAP_VIDEO_CAPTURE_MPLANE | V4L2_CAP_VIDEO_OUTPUT_MPLANE;
--
2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] [media] s5p-jpeg: set capablity bus_info as required by VIDIOC_QUERYCAP
The driver doesn't set the struct v4l2_capability cap_info field so the v4l2-compliance tool reports the following errors for VIDIOC_QUERYCAP: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) fail: v4l2-compliance.cpp(304): string empty fail: v4l2-compliance.cpp(528): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL This patch fixes by setting the field in VIDIOC_QUERYCAP ioctl handler: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) test VIDIOC_QUERYCAP: OK Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 17bc94092864..e3ff3d4bd72e 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1256,7 +1256,8 @@ static int s5p_jpeg_querycap(struct file *file, void *priv, strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder", sizeof(cap->card)); } - cap->bus_info[0] = 0; + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(ctx->jpeg->dev)); cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M; cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] [media] gsc-m2m: improve v4l2_capability driver and card fields
According to the V4L2 documentation the driver and card fields should be used to identify the driver and the device but the gsc-m2m driver fills those field using the platform device name, which in turn is the name of the device DT node. So not only the filled information isn't correct but also the same values are used in the driver, card and bus_info fields. Before this patch: Driver Info (not using libv4l2): Driver name : 13e0.video- Card type : 13e0.video-scaler Bus info : platform:13e0.video-scaler Driver version: 4.7.0 After this patch: Driver Info (not using libv4l2): Driver name : exynos-gsc Card type : exynos-gsc gscaler Bus info : platform:13e0.video-scaler Driver version: 4.7.0 Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c index af81383086b8..274861c27367 100644 --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c @@ -279,8 +279,8 @@ static int gsc_m2m_querycap(struct file *file, void *fh, struct gsc_ctx *ctx = fh_to_ctx(fh); struct gsc_dev *gsc = ctx->gsc_dev; - strlcpy(cap->driver, gsc->pdev->name, sizeof(cap->driver)); - strlcpy(cap->card, gsc->pdev->name, sizeof(cap->card)); + strlcpy(cap->driver, GSC_MODULE_NAME, sizeof(cap->driver)); + strlcpy(cap->card, GSC_MODULE_NAME " gscaler", sizeof(cap->card)); snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", dev_name(&gsc->pdev->dev)); cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M_MPLANE | -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] [media] s5p-jpeg: only fill driver's name in capabilities driver field
The driver fills in both the struct v4l2_capability driver and card fields
the same values, that is the driver's name plus the information if the dev
is a decoder or an encoder.
But the driver field has a fixed length of 16 bytes so the filled data is
truncated:
Driver Info (not using libv4l2):
Driver name : s5p-jpeg decode
Card type : s5p-jpeg decoder
Bus info : platform:11f5.jpeg
Driver version: 4.7.0
Also, this field should only contain the driver's name so use just that.
The information if the device is a decoder or an encoder is in the card
type field anyways.
Signed-off-by: Javier Martinez Canillas
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index e3ff3d4bd72e..f9fb52a53e79 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1246,12 +1246,12 @@ static int s5p_jpeg_querycap(struct file *file, void
*priv,
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
if (ctx->mode == S5P_JPEG_ENCODE) {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " encoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " encoder",
sizeof(cap->card));
} else {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " decoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder",
sizeof(cap->card));
--
2.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] [media] Fixes and improvements for VIDIOC_QUERYCAP in Samsung media drivers
Hello, This series contains some fixes and improvements for the VIDIOC_QUERYCAP ioctl handler in different media platform drivers for IP blocks found in Exynos SoCs. Some of the issues were reported by the v4l2-compliance tool while others are things I noticed while looking at the Driver name, Card type and Bus info. Best regards, Javier Javier Martinez Canillas (6): [media] s5p-mfc: set capablity bus_info as required by VIDIOC_QUERYCAP [media] s5p-mfc: improve v4l2_capability driver and card fields [media] s5p-jpeg: set capablity bus_info as required by VIDIOC_QUERYCAP [media] s5p-jpeg: only fill driver's name in capabilities driver field [media] gsc-m2m: add device name sufix to bus_info capatiliby field [media] gsc-m2m: improve v4l2_capability driver and card fields drivers/media/platform/exynos-gsc/gsc-m2m.c | 7 --- drivers/media/platform/s5p-jpeg/jpeg-core.c | 7 --- drivers/media/platform/s5p-mfc/s5p_mfc.c| 1 - drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 ++ drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 7 --- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 7 --- 6 files changed, 18 insertions(+), 13 deletions(-) -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] [media] s5p-mfc: set capablity bus_info as required by VIDIOC_QUERYCAP
The driver doesn't set the struct v4l2_capability bus_info field so the v4l2-compliance tool reports the following errors for VIDIOC_QUERYCAP: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) fail: v4l2-compliance.cpp(304): string empty fail: v4l2-compliance.cpp(528): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL This patch fixes by setting the field in VIDIOC_QUERYCAP ioctl handler: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) test VIDIOC_QUERYCAP: OK Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 ++- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c index f2d6376ce618..4a40df22fd63 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c @@ -267,7 +267,8 @@ static int vidioc_querycap(struct file *file, void *priv, strncpy(cap->driver, dev->plat_dev->name, sizeof(cap->driver) - 1); strncpy(cap->card, dev->plat_dev->name, sizeof(cap->card) - 1); - cap->bus_info[0] = 0; + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(&dev->plat_dev->dev)); /* * This is only a mem-to-mem video device. The capture and output * device capability flags are left only for backward compatibility diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c index 034b5c1d35a1..dd466ea6429e 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c @@ -945,7 +945,8 @@ static int vidioc_querycap(struct file *file, void *priv, strncpy(cap->driver, dev->plat_dev->name, sizeof(cap->driver) - 1); strncpy(cap->card, dev->plat_dev->name, sizeof(cap->card) - 1); - cap->bus_info[0] = 0; + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(&dev->plat_dev->dev)); /* * This is only a mem-to-mem video device. The capture and output * device capability flags are left only for backward compatibility -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Need help with ir-keytable imon bug report
HI Gregor, Em Wed, 15 Jun 2016 22:25:06 +0200 Gregor Jasny escreveu: > Hello, > > could someone please help me triaging the following ir-keytable bug? The > reporter complains that the 'other' IR protocol results in double clicks > and we should set the device to RC6 instead: > > https://bugs.launchpad.net/ubuntu/+source/v4l-utils/+bug/1579760 > > This is what we have in v4l-utils: > https://git.linuxtv.org/v4l-utils.git/tree/utils/keytable/rc_keymaps/imon_pad The way it works is that the keymap table comes from the Kernel driver. The scripts at v4l-utils just copies whatever is there. Please notice that the IMON keymap is used by only one Kernel driver: drivers/media/rc/imon.c, with supports two different protocols: RC6 and a proprietary one (the driver calls it iMON protocol). The driver actually supports two types of IR key maps, depending on the protocol: if (ictx->rc_type == RC_BIT_RC6_MCE) rdev->map_name = RC_MAP_IMON_MCE; else rdev->map_name = RC_MAP_IMON_PAD; In other words, it uses either the code at: drivers/media/IR/keymaps/rc-imon-pad.c (for the IMON protocol) or drivers/media/rc/keymaps/rc-imon-mce.c (for RC6) I suspect that the user is selecting the wrong keymap on the BZ you mentioned. It should be using: utils/keytable/rc_keymaps/imon_mce if his device came with a RC6 IR. There's another possibility: maybe some newer devices come with a different keymap than the one available when the driver was originally written. That's said, from his report: $ sudo ir-keytable Found /sys/class/rc/rc0/ (/dev/input/event4) with: Driver imon, table rc-imon-pad Supported protocols: other It should be listing both "other" and "RC6" protocols there. It sounds a Kernel regression. I remember one Kernel patch once broke the list of protocols. Maybe the fix patch were not applied on Ubuntu, or maybe some other regression happened. -- Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Fri Jun 17 04:00:23 CEST 2016 git branch: test git hash: 59f0bc11848f8f3242bc1fefae670e745929cd7b gcc version:i686-linux-gcc (GCC) 5.3.0 sparse version: v0.5.0-56-g7647c77 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.5.0-264 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-i686: OK linux-4.3-i686: OK linux-4.4-i686: OK linux-4.5-i686: OK linux-4.6-i686: OK linux-4.7-rc1-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-x86_64: OK linux-4.3-x86_64: OK linux-4.4-x86_64: OK linux-4.5-x86_64: OK linux-4.6-x86_64: OK linux-4.7-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS smatch: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: fix media devnode ioctl/syscall and unregister race
Hi Shuah,
On Fri, Jun 10, 2016 at 11:37:23AM -0600, Shuah Khan wrote:
> Media devnode open/ioctl could be in progress when media device unregister
> is initiated. System calls and ioctls check media device registered status
> at the beginning, however, there is a window where unregister could be in
> progress without changing the media devnode status to unregistered.
>
> process 1 process 2
> fd = open(/dev/media0)
> media_devnode_is_registered()
> (returns true here)
>
> media_device_unregister()
> (unregister is in progress
> and devnode isn't
> unregistered yet)
> ...
> ioctl(fd, ...)
> __media_ioctl()
> media_devnode_is_registered()
> (returns true here)
> ...
> media_devnode_unregister()
> ...
> (driver releases the media device
> memory)
>
> media_device_ioctl()
> (By this point
> devnode->media_dev does not
> point to allocated memory.
> use-after free in in mutex_lock_nested)
>
> BUG: KASAN: use-after-free in mutex_lock_nested+0x79c/0x800 at addr
> 8801ebe914f0
>
> Fix it by clearing register bit when unregister starts to avoid the race.
Does this patch solve the problem? You'd have to take the mutex for the
duration of the IOCTL which I don't see the patch doing.
Instead of serialising operations using mutexes, I believe a proper fix for
this is to take a reference to the data structures required.
>
> process 1 process 2
> fd = open(/dev/media0)
> media_devnode_is_registered()
> (could return true here)
>
> media_device_unregister()
> (clear the register bit,
>then start unregister.)
> ...
> ioctl(fd, ...)
> __media_ioctl()
> media_devnode_is_registered()
> (return false here, ioctl
>returns I/O error, and
>will not access media
>device memory)
> ...
> media_devnode_unregister()
> ...
> (driver releases the media device
>memory)
>
> Signed-off-by: Shuah Khan
> Suggested-by: Sakari Ailus
> Reported-by: Mauro Carvalho Chehab
> ---
>
> Test Procedure and Results:
>
> https://drive.google.com/file/d/0B0NIL0BQg-AlN1VxT2oyTXBPRHc/view?usp=sharing
>
> drivers/media/media-device.c | 15 ---
> drivers/media/media-devnode.c | 8 +++-
> include/media/media-devnode.h | 16 ++--
> 3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 33a9952..1795abe 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -732,6 +732,7 @@ int __must_check __media_device_register(struct
> media_device *mdev,
> if (ret < 0) {
> /* devnode free is handled in media_devnode_*() */
> mdev->devnode = NULL;
> + media_devnode_unregister_prepare(devnode);
> media_devnode_unregister(devnode);
> return ret;
> }
> @@ -788,6 +789,9 @@ void media_device_unregister(struct media_device *mdev)
> return;
> }
>
> + /* Clear the devnode register bit to avoid races with media dev open */
> + media_devnode_unregister_prepare(mdev->devnode);
> +
> /* Remove all entities from the media device */
> list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> __media_device_unregister_entity(entity);
> @@ -808,13 +812,10 @@ void media_device_unregister(struct media_device *mdev)
>
> dev_dbg(mdev->dev, "Media device unregistered\n");
>
> - /* Check if mdev devnode was registered */
> - if (media_devnode_is_registered(mdev->devnode)) {
> - device_remove_file(&mdev->devnode->dev, &dev_attr_model);
> - media_devnode_unregister(mdev->devnode);
> - /* devnode free is handled in media_devnode_*() */
> - mdev->devnode = NULL;
> - }
> + device_remove_file(&mdev->devnode->dev, &dev_attr_model);
> + media_devnode_unregister(mdev->devnode);
> + /* devnode free is handled in media_devnode_*() */
> + mdev->devnode = NULL;
> }
> EXPORT_SYMBOL_GPL(media_device_unregister);
>
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index 5b605ff..f2772ba
Re: [PATCH 4/6] [media] s5p-jpeg: only fill driver's name in capabilities driver field
Hi Javier,
On 06/16/2016 11:40 PM, Javier Martinez Canillas wrote:
The driver fills in both the struct v4l2_capability driver and card fields
the same values, that is the driver's name plus the information if the dev
is a decoder or an encoder.
But the driver field has a fixed length of 16 bytes so the filled data is
truncated:
Driver Info (not using libv4l2):
Driver name : s5p-jpeg decode
Card type : s5p-jpeg decoder
Bus info : platform:11f5.jpeg
Driver version: 4.7.0
Also, this field should only contain the driver's name so use just that.
The information if the device is a decoder or an encoder is in the card
type field anyways.
Signed-off-by: Javier Martinez Canillas
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index e3ff3d4bd72e..f9fb52a53e79 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1246,12 +1246,12 @@ static int s5p_jpeg_querycap(struct file *file, void
*priv,
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
if (ctx->mode == S5P_JPEG_ENCODE) {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " encoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " encoder",
sizeof(cap->card));
} else {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " decoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder",
sizeof(cap->card));
Acked-by: Jacek Anaszewski
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] [media] s5p-jpeg: set capablity bus_info as required by VIDIOC_QUERYCAP
Hi Javier, On 06/16/2016 11:40 PM, Javier Martinez Canillas wrote: The driver doesn't set the struct v4l2_capability cap_info field so the v4l2-compliance tool reports the following errors for VIDIOC_QUERYCAP: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) fail: v4l2-compliance.cpp(304): string empty fail: v4l2-compliance.cpp(528): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL This patch fixes by setting the field in VIDIOC_QUERYCAP ioctl handler: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) test VIDIOC_QUERYCAP: OK Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 17bc94092864..e3ff3d4bd72e 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1256,7 +1256,8 @@ static int s5p_jpeg_querycap(struct file *file, void *priv, strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder", sizeof(cap->card)); } - cap->bus_info[0] = 0; + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(ctx->jpeg->dev)); cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M; cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; Acked-by: Jacek Anaszewski -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
