date:20180321

Re: [PATCH v5 0/2] media: video-i2c: add video-i2c driver support

2018-03-21 Thread Matt Ranostay

On Mon, Mar 19, 2018 at 5:31 PM, Hans Verkuil  wrote:
> On 03/16/2018 03:47 AM, Matt Ranostay wrote:
>> On Fri, Mar 9, 2018 at 10:26 AM, Matt Ranostay
>>  wrote:
>>> On Fri, Mar 9, 2018 at 4:45 AM, Hans Verkuil  wrote:
 Hi Matt,

 This is looking good. One request before I merge: please run the
 'v4l2-compliance -s -f' utility and post the result here.

 I don't think I've asked you to do that before (or if I did, I couldn't
 find it in my mail archive).

 It should run without failures.

 Use the latest version from the git repo: 
 https://git.linuxtv.org/v4l-utils.git/

 ./bootstrap.sh; ./configure; make; sudo make install
>>>
>>> Heh so not exactly no failures. Suspect a lot of these are due to the
>>> weird small 8x8 pixel input, and the fact it doesn't
>>> support modes a typical video capture device would.
>>>
>>> v4l2-compliance SHA   : 14ce03c18ef67aa7a3d5781f015be855fd43839c
>>>
>>> Compliance test for device /dev/video0:
>>>
>>> Driver Info:
>>> Driver name  : video-i2c
>>> Card type: I2C 2-105 Transport Video
>>> Bus info : I2C:2-105
>>> Driver version   : 4.14.11
>>> Capabilities : 0x8521
>>> Video Capture
>>> Read/Write
>>> Streaming
>>> Extended Pix Format
>>> Device Capabilities
>>> Device Caps  : 0x0521
>>> Video Capture
>>> Read/Write
>>> Streaming
>>> Extended Pix Format
>>>
>>> Required ioctls:
>>> test VIDIOC_QUERYCAP: OK
>>>
>>> Allow for multiple opens:
>>> test second /dev/video0 open: OK
>>> test VIDIOC_QUERYCAP: OK
>>> test VIDIOC_G/S_PRIORITY: OK
>>> test for unlimited opens: OK
>>>
>>> Debug ioctls:
>>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>>> test VIDIOC_LOG_STATUS: OK (Not Supported)
>>>
>>> Input ioctls:
>>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>>> test VIDIOC_G/S/ENUMINPUT: OK
>>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
>>> Inputs: 1 Audio Inputs: 0 Tuners: 0
>>>
>>> Output ioctls:
>>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>>> Outputs: 0 Audio Outputs: 0 Modulators: 0
>>>
>>> Input/Output configuration ioctls:
>>> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>>> test VIDIOC_G/S_EDID: OK (Not Supported)
>>>
>>> Control ioctls (Input 0):
>>> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
>>> test VIDIOC_QUERYCTRL: OK (Not Supported)
>>> test VIDIOC_G/S_CTRL: OK (Not Supported)
>>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
>>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
>>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>>> Standard Controls: 0 Private Controls: 0
>>>
>>> Format ioctls (Input 0):
>>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>>> test VIDIOC_G/S_PARM: OK
>>> test VIDIOC_G_FBUF: OK (Not Supported)
>>> test VIDIOC_G_FMT: OK
>>> test VIDIOC_TRY_FMT: OK
>>> test VIDIOC_S_FMT: OK
>>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>>> test Cropping: OK (Not Supported)
>>> test Composing: OK (Not Supported)
>>> test Scaling: OK (Not Supported)
>>>
>>> Codec ioctls (Input 0):
>>> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>>> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>>> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>>>
>>> Buffer ioctls (Input 0):
>>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>>> test VIDIOC_EXPBUF: OK (Not Supported)
>>>
>>> Test input 0:
>>>
>>> Streaming ioctls:
>>> test read/write: OK
>>> fail: v4l2-test-buffers.cpp(248): g_field() == V4L2_FIELD_ANY
>>
>> Noticed this seems to be the most worrying of the failures.  Any
>> suggestions where could be requested and still be 0 == V4L2_FIELD_ANY?
>
> struct v4l2_buffers should never return FIELD_ANY. For this driver
> the frames are always progressive, not interlaced, so just set this
> to FIELD_NONE in the driver.
>
> Basically the driver must never give FIELD_ANY back to userspace.
> Userspace can pass in FIELD_ANY when it calls TRY/S_FMT, but it should
> always be replaced with a non-zero field value. In this case FIELD_NONE.


Which callback would that be in? Because currently the driver is only
handling the v4l2_pix_format settings, and not
vb2_v4l2_buffer settings.

- Matt

>
> Regards,
>
> Hans
>
>>
>>> fail: v4l2-test-buffers.cpp(658): buf.check(q, last_seq)
>>> fail: v4l2-test-buffers.cpp(928): captureBufs(node, q, m2m_q,
>>> frame_count, false)
>>> test MMAP: FAIL
>>> fail: v4l2-test-buffers.cpp(1028): can_stream && ret != EINVAL
>>> test USERPTR: FAIL
>>> test DMABUF: Cannot

cron job: media_tree daily build: WARNINGS

2018-03-21 Thread Hans Verkuil

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Thu Mar 22 05:00:14 CET 2018
media-tree git hash:e68854a2588a923b31eebce348f8020374843f8e
media_build git hash:   e95b7e6bfea396f9dfb1ff7d4d6b95ecacd53d3d
v4l-utils git hash: 14ce03c18ef67aa7a3d5781f015be855fd43839c
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: WARNINGS
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: 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: WARNINGS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.101-i686: WARNINGS
linux-3.0.101-x86_64: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.100-i686: WARNINGS
linux-3.2.100-x86_64: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.113-i686: WARNINGS
linux-3.4.113-x86_64: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.10-i686: WARNINGS
linux-3.7.10-x86_64: WARNINGS
linux-3.8.13-i686: WARNINGS
linux-3.8.13-x86_64: WARNINGS
linux-3.9.11-i686: WARNINGS
linux-3.9.11-x86_64: WARNINGS
linux-3.10.108-i686: WARNINGS
linux-3.10.108-x86_64: WARNINGS
linux-3.11.10-i686: WARNINGS
linux-3.11.10-x86_64: WARNINGS
linux-3.12.74-i686: WARNINGS
linux-3.12.74-x86_64: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.79-i686: WARNINGS
linux-3.14.79-x86_64: WARNINGS
linux-3.15.10-i686: WARNINGS
linux-3.15.10-x86_64: WARNINGS
linux-3.16.55-i686: WARNINGS
linux-3.16.55-x86_64: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.100-i686: WARNINGS
linux-3.18.100-x86_64: WARNINGS
linux-3.19.8-i686: WARNINGS
linux-3.19.8-x86_64: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.50-i686: WARNINGS
linux-4.1.50-x86_64: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.99-i686: OK
linux-4.4.99-x86_64: OK
linux-4.5.7-i686: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: WARNINGS
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: WARNINGS
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: WARNINGS
linux-4.9.87-i686: WARNINGS
linux-4.9.87-x86_64: WARNINGS
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: WARNINGS
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: WARNINGS
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.27-i686: OK
linux-4.14.27-x86_64: OK
linux-4.15.10-i686: OK
linux-4.15.10-x86_64: OK
linux-4.16-rc5-i686: OK
linux-4.16-rc5-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Thursday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

[trivial PATCH V2] treewide: Align function definition open/close braces

2018-03-21 Thread Joe Perches

Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Signed-off-by: Joe Perches 
Acked-by: Andy Shevchenko 
Acked-by: Paul Moore 
Acked-by: Alex Deucher 
Acked-by: Dave Chinner 
Reviewed-by: Darrick J. Wong 
Acked-by: Alexandre Belloni 
Acked-by: Martin K. Petersen 
Acked-by: Takashi Iwai 
Acked-by: Mauro Carvalho Chehab 
---

git diff -w still shows no difference.

This patch was sent but December and not applied.

As the trivial maintainer seems not active, it'd be nice if
Andrew Morton picks this up.

V2: Remove fs/xfs/libxfs/xfs_alloc.c as it's updated and remerge the rest

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 19 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 46e1ef17d92d..92212bf0484f 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -123,7 +123,7 @@ static inline long long arch_atomic64_read(const atomic64_t 
*v)
long long r;
alternative_atomic64(read, "=" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * arch_atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index b33fba70ec51..a07fbe999eb6 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -97,7 +97,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 8394d69b963f..e934326a95d3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -588,7 +588,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 439ee9c5f535..231f3a1e27bf 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2967,7 +2967,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(>sas_mgmt.mutex);
 out:

Re: [PATCH] media: uvcvideo: Prevent setting unavailable flags

2018-03-21 Thread Laurent Pinchart

Hi Kieran,

Thank you for the patch.

On Wednesday, 21 March 2018 17:43:08 EET Kieran Bingham wrote:
> The addition of an extra operation to use the GET_INFO command
> overwrites all existing flags from the uvc_ctrls table. This includes
> setting all controls as supporting  GET_MIN, GET_MAX, GET_RES, and
> GET_DEF regardless of whether they do or not.
> 
> Move the initialisation of these control capabilities directly to the
> uvc_ctrl_fill_xu_info() call where they were originally located in that
> use case, and ensure that the new functionality in uvc_ctrl_get_flags()
> will only set flags based on their reported capability from the GET_INFO
> call.
> 
> Fixes: 859086ae3636 ("media: uvcvideo: Apply flags from device to actual
> properties")
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

And applied to my tree.

Mauro, this fixes a regression in your master branch queued for v4.17. Do you 
want a pull request now, or after the merge window for -rc2 ?

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index 1daf444371be..4042cbdb721b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1607,14 +1607,12 @@ static int uvc_ctrl_get_flags(struct uvc_device
> *dev, ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id,
> dev->intfnum, info->selector, data, 1);
>   if (!ret)
> - info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> - | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
> - | (data[0] & UVC_CONTROL_CAP_GET ?
> -UVC_CTRL_FLAG_GET_CUR : 0)
> - | (data[0] & UVC_CONTROL_CAP_SET ?
> -UVC_CTRL_FLAG_SET_CUR : 0)
> - | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
> -UVC_CTRL_FLAG_AUTO_UPDATE : 0);
> + info->flags |= (data[0] & UVC_CONTROL_CAP_GET ?
> + UVC_CTRL_FLAG_GET_CUR : 0)
> + |  (data[0] & UVC_CONTROL_CAP_SET ?
> + UVC_CTRL_FLAG_SET_CUR : 0)
> + |  (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
> + UVC_CTRL_FLAG_AUTO_UPDATE : 0);
> 
>   kfree(data);
>   return ret;
> @@ -1689,6 +1687,9 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device
> *dev,
> 
>   info->size = le16_to_cpup((__le16 *)data);
> 
> + info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> + | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF;
> +
>   ret = uvc_ctrl_get_flags(dev, ctrl, info);
>   if (ret < 0) {
>   uvc_trace(UVC_TRACE_CONTROL,


-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: v4l2-common: fix a compilation breakage

2018-03-21 Thread Sakari Ailus

Hi Mauro,

On Wed, Mar 21, 2018 at 04:08:29PM -0400, Mauro Carvalho Chehab wrote:
> Clearly, changeset 95ce9c28601a ("media: v4l: common: Add a
> function to obtain best size from a list") was never tested, as it
> broke compilation with:
> 
> drivers/media/platform/vivid/vivid-vid-cap.c: In function 
> ‘vivid_try_fmt_vid_cap’:
> drivers/media/platform/vivid/vivid-vid-cap.c:565:34: error: macro 
> "v4l2_find_nearest_size" requer 6 argumentos, mas apenas 5 foram fornecidos
>  mp->width, mp->height);
>   ^
> drivers/media/platform/vivid/vivid-vid-cap.c:564:4: error: 
> ‘v4l2_find_nearest_size’ undeclared (first use in this function); did you 
> mean ‘__v4l2_find_nearest_size’?
> v4l2_find_nearest_size(webcam_sizes, width, height,
> ^~
> __v4l2_find_nearest_size
> drivers/media/platform/vivid/vivid-vid-cap.c:564:4: note: each undeclared 
> identifier is reported only once for each function it appears in
> drivers/media/i2c/ov5670.c: In function ‘ov5670_set_pad_format’:
> drivers/media/i2c/ov5670.c:2233:48: error: macro "v4l2_find_nearest_size" 
> requer 6 argumentos, mas apenas 5 foram fornecidos
>fmt->format.width, fmt->format.height);
> ^
> drivers/media/i2c/ov5670.c:2232:9: error: ‘v4l2_find_nearest_size’ undeclared 
> (first use in this function); did you mean ‘__v4l2_find_nearest_size’?
>   mode = v4l2_find_nearest_size(supported_modes, width, height,
>  ^~
>  __v4l2_find_nearest_size
> drivers/media/i2c/ov13858.c: In function ‘ov13858_set_pad_format’:
> drivers/media/i2c/ov13858.c:1379:48: error: macro "v4l2_find_nearest_size" 
> requer 6 argumentos, mas apenas 5 foram fornecidos
>fmt->format.width, fmt->format.height);
> ^
> drivers/media/i2c/ov13858.c:1378:9: error: ‘v4l2_find_nearest_size’ 
> undeclared (first use in this function); did you mean 
> ‘__v4l2_find_nearest_size’?
>   mode = v4l2_find_nearest_size(supported_modes, width, height,
>  ^~
>  __v4l2_find_nearest_size
> drivers/media/i2c/ov13858.c:1378:9: note: each undeclared identifier is 
> reported only once for each function it appears in
> 
> Basically, v4l2_find_nearest_size() callers pass 5 arguments,
> while its definition require 6 args.
> 
> Unfortunately, my build process was also broken, as it was reporting me that
> the compilation went fine:
> 
>   $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y 
> C=1 W=1 CHECK='compile_checks' M=drivers/staging/media
>   $ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y 
> C=1 W=1 CHECK='compile_checks' M=drivers/media
> 
>   *** ERRORS ***
> 
>   *** WARNINGS ***
>   compilation succeeded
> 
> That was due to a change here to use of linux-log-diff script that
> provides a diffstat between the errors output. Somehow, the logic
> was missing some fatal errors.

Apologies for the above. This isn't still the intended way how things
should be; I'll send you a new patch to properly address this on top of
yours.

What happened was that I had the patches in two different environments and
I ended up picking the last four patches from the wrong one. :-P No errors
from kbuild made me think the patches were the right ones...

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

[PATCH] media: v4l2-common: fix a compilation breakage

2018-03-21 Thread Mauro Carvalho Chehab

Clearly, changeset 95ce9c28601a ("media: v4l: common: Add a
function to obtain best size from a list") was never tested, as it
broke compilation with:

drivers/media/platform/vivid/vivid-vid-cap.c: In function 
‘vivid_try_fmt_vid_cap’:
drivers/media/platform/vivid/vivid-vid-cap.c:565:34: error: macro 
"v4l2_find_nearest_size" requer 6 argumentos, mas apenas 5 foram fornecidos
 mp->width, mp->height);
  ^
drivers/media/platform/vivid/vivid-vid-cap.c:564:4: error: 
‘v4l2_find_nearest_size’ undeclared (first use in this function); did you mean 
‘__v4l2_find_nearest_size’?
v4l2_find_nearest_size(webcam_sizes, width, height,
^~
__v4l2_find_nearest_size
drivers/media/platform/vivid/vivid-vid-cap.c:564:4: note: each undeclared 
identifier is reported only once for each function it appears in
drivers/media/i2c/ov5670.c: In function ‘ov5670_set_pad_format’:
drivers/media/i2c/ov5670.c:2233:48: error: macro "v4l2_find_nearest_size" 
requer 6 argumentos, mas apenas 5 foram fornecidos
   fmt->format.width, fmt->format.height);
^
drivers/media/i2c/ov5670.c:2232:9: error: ‘v4l2_find_nearest_size’ undeclared 
(first use in this function); did you mean ‘__v4l2_find_nearest_size’?
  mode = v4l2_find_nearest_size(supported_modes, width, height,
 ^~
 __v4l2_find_nearest_size
drivers/media/i2c/ov13858.c: In function ‘ov13858_set_pad_format’:
drivers/media/i2c/ov13858.c:1379:48: error: macro "v4l2_find_nearest_size" 
requer 6 argumentos, mas apenas 5 foram fornecidos
   fmt->format.width, fmt->format.height);
^
drivers/media/i2c/ov13858.c:1378:9: error: ‘v4l2_find_nearest_size’ undeclared 
(first use in this function); did you mean ‘__v4l2_find_nearest_size’?
  mode = v4l2_find_nearest_size(supported_modes, width, height,
 ^~
 __v4l2_find_nearest_size
drivers/media/i2c/ov13858.c:1378:9: note: each undeclared identifier is 
reported only once for each function it appears in

Basically, v4l2_find_nearest_size() callers pass 5 arguments,
while its definition require 6 args.

Unfortunately, my build process was also broken, as it was reporting me that
the compilation went fine:

$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y 
C=1 W=1 CHECK='compile_checks' M=drivers/staging/media
$ make ARCH=i386  CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y 
C=1 W=1 CHECK='compile_checks' M=drivers/media

*** ERRORS ***

*** WARNINGS ***
compilation succeeded

That was due to a change here to use of linux-log-diff script that
provides a diffstat between the errors output. Somehow, the logic
was missing some fatal errors.

Fixes: 95ce9c28601a ("media: v4l: common: Add a function to obtain best size 
from a list")

Signed-off-by: Mauro Carvalho Chehab 
---
 include/media/v4l2-common.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 160bca96d524..54b689247937 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -320,7 +320,6 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  * set of resolutions contained in an array of a driver specific struct.
  *
  * @array: a driver specific array of image sizes
- * @array_size: the length of the driver specific array of image sizes
  * @width_field: the name of the width field in the driver specific struct
  * @height_field: the name of the height field in the driver specific struct
  * @width: desired width.
@@ -333,13 +332,13 @@ void v4l_bound_align_image(unsigned int *width, unsigned 
int wmin,
  *
  * Returns the best match or NULL if the length of the array is zero.
  */
-#define v4l2_find_nearest_size(array, array_size, width_field, height_field, \
+#define v4l2_find_nearest_size(array, width_field, height_field, \
   width, height)   \
({  \
BUILD_BUG_ON(sizeof((array)->width_field) != sizeof(u32) || \
 sizeof((array)->height_field) != sizeof(u32)); \
(typeof(&(*(array__v4l2_find_nearest_size(  \
-   (array), array_size, sizeof(*(array)),  \
+   (array), ARRAY_SIZE(array), sizeof(*(array)),   \
offsetof(typeof(*(array)), width_field),\
offsetof(typeof(*(array)), height_field),   \
width, height); \
-- 
2.14.3

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

2018-03-21 Thread Nicolas Dufresne

Le mercredi 21 mars 2018 à 10:55 +0200, Laurent Pinchart a écrit :
> Hi Nicolas,
> 
> On Wednesday, 21 March 2018 05:38:59 EET Nicolas Dufresne wrote:
> > Le mardi 20 mars 2018 à 20:04 +0200, Laurent Pinchart a écrit :
> > > On Tuesday, 20 March 2018 19:45:51 EET Nicolas Dufresne wrote:
> > > > Le mardi 20 mars 2018 à 13:20 +0100, Paul Menzel a écrit :
> > > > > Dear Linux folks,
> > > > > 
> > > > > 
> > > > > On the Dell XPS 13 9370, Linux 4.16-rc6 outputs the messages below.
> > > > > 
> > > > > ```
> > > > > [2.338094] calling  uvc_init+0x0/0x1000 [uvcvideo] @ 295
> > > > > [2.338569] calling  iTCO_wdt_init_module+0x0/0x1000 [iTCO_wdt] @
> > > > > 280
> > > > > [2.338570] iTCO_wdt: Intel TCO WatchDog Timer Driver v1.11
> > > > > [2.338713] iTCO_wdt: Found a Intel PCH TCO device (Version=4,
> > > > > TCOBASE=0x0400)
> > > > > [2.338755] uvcvideo: Found UVC 1.00 device Integrated_Webcam_HD
> > > > > (0bda:58f4)
> > > > > [2.338827] iTCO_wdt: initialized. heartbeat=30 sec (nowayout=0)
> > > > > [2.338851] initcall iTCO_wdt_init_module+0x0/0x1000 [iTCO_wdt]
> > > > > returned 0 after 271 usecs
> > > > > [2.340669] uvcvideo 1-5:1.0: Entity type for entity Extension 4
> > > > > was
> > > > > not initialized!
> > > > > [2.340670] uvcvideo 1-5:1.0: Entity type for entity Extension 7
> > > > > was
> > > > > not initialized!
> > > > > [2.340672] uvcvideo 1-5:1.0: Entity type for entity Processing 2
> > > > > was
> > > > > not initialized!
> > > > > [2.340673] uvcvideo 1-5:1.0: Entity type for entity Camera 1 was
> > > > > not
> > > > > initialized!
> > > > > [2.340736] input: Integrated_Webcam_HD: Integrate as
> > > > > /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/input/input9
> > > > > [2.341447] uvcvideo: Unknown video format
> > > > > 0032-0002-0010-8000-00aa00389b71
> > > > 
> > > > While the 0002 is suspicious, this is pretty close to a color format.
> > > > I've recently come across of similar format using D3DFORMAT instead of
> > > > GUID. According to the vendor*, this camera module includes an infrared
> > > > camera (340x340), so I suspect this is to specify the format it
> > > > outputs. A good guess to start with would be that this is
> > > > D3DFMT_X8L8V8U8 (0x32).
> > > 
> > > Isn't 0x32 D3DFMT_L8, not D3DFMT_X8L8V8U8 ?
> > 
> > You are right, sorry about that, I totally miss-translate. It felt
> > weird. This is much more likely yes. So maybe it's the same mapping
> > (but with the -2- instead) as what I added for the HoloLense
> > Camera.
> > 
> > > > To test it, you could map this
> > > > V4L2_PIX_FMT_YUV32/xRGB and see if the driver is happy with the buffer
> > > > size.
> > > 
> > > VideoStreaming Interface Descriptor:
> > > bLength30
> > > bDescriptorType36
> > > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > > bFrameIndex 1
> > > bmCapabilities   0x00
> > > 
> > >   Still image unsupported
> > > 
> > > wWidth340
> > > wHeight   340
> > > dwMinBitRate 55488000
> > > dwMaxBitRate 55488000
> > > dwMaxVideoFrameBufferSize  115600
> > > dwDefaultFrameInterval 16
> > > bFrameIntervalType  1
> > > dwFrameInterval( 0)16
> > > 
> > > 340*340 is 115600, so this should be a 8-bit format.
> > 
> > Indeed, that matches.
> > 
> > > > Then render it to make sure it looks some image of some sort. A
> > > > new format will need to be defined as this format is in the wrong
> > > > order, and is ambiguous (it may mean AYUV or xYUV). I'm not sure if we
> > > > need specific formats to differentiate infrared data from YUV images,
> > > > need to be discussed.
> > > 
> > > If the format is indeed D3DFMT_L8, it should map to V4L2_PIX_FMT_GREY
> > > (8-bit luminance). I suspect the camera transmits a depth map though.
> > 
> > I wonder if we should think of a way to tell userspace this is fnfrared
> > data rather then black and white ?
> 
> I think we need such a mechanism, yes. Would you like to propose one ? :-)

Ok, meanwhile I've looked over how this camera was used. It seems
that's it's combined with a IR light in order to create a near field
depth. As we already exposes couple of depth sensors as GREY, I think
your patch is ok, and should be merged. It's not really clear in
general how the driver can really figure-out what type of data is
delivered. So I'm not sure where to start.

> 
> I've found https://www.magnumdb.com/search?q=value%3A
> %220032-0002-0010-8000-00aa00389b71%22 and 
> https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/infrared-stream-support-in-uvc
>  that confirm 
> this is a 8-bit IR format.
> 
> > > >

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

2018-03-21 Thread Greg KH

On Wed, Mar 21, 2018 at 06:29:48PM +0100, Daniel Scheller wrote:
> Hi Greg,
> 
> Am Wed, 21 Mar 2018 10:49:32 +0100
> schrieb Greg KH :
> 
> > On Tue, Mar 20, 2018 at 10:01:27PM +0100, Daniel Scheller wrote:
> > > From: Daniel Scheller 
> > > 
> > > This series adds SPDX license identifiers to all source files which are
> > > copyright by either Digital Devices GmbH or Metzlerbros GbR, who are
> > > the original authors of the ddbridge, ngene, cxd2099, mxl5xx, stv0910
> > > and stv6111 bridge/demod/tuner drivers, with the mxl5xx driver being
> > > based on source code released by MaxLinear.
> > > [...]
> > > The original intention was to fully replace all the licensing headers
> > > with only the SPDX License Identifiers as it is done in a lot of other
> > > in-tree drivers nowadays. However, Digital Devices disagreed to do this
> > > and expressed major concerns regarding this, in that a machine readable
> > > license tag instead of a full license boilerplate won't hold up equally,
> > > so we agreed to keep the license boilerplate text as is right now.  
> > 
> > That's really odd, who at that company can I talk to about this?  Or
> > really, what lawyer at that company can I point my lawyer at to talk
> > about this, that's the only way this is going to get resolved.
> 
> I'm not entirely sure, but I guess for a first start it's best to
> contact Ralph (from Metzlerbros) and Manfred (from Digital
> Devices), being the authors and copyright owners of the DDDVB driver
> package where the drivers originate from and thus is the upstream for
> the mainlined copies of the mentioned drivers. Both are in the Cc list
> (rjkm and mvoelkel) of this thread.
> 
> > If it helps, _ALL_ of the major companies that are kernel developers are
> > onboard with the removal of the crazy boiler-plate text, so this tiny
> > holdout should be easy to resolve.
> > 
> > > Greg, I'm Cc'ing you on this due to the last paragraph, as AFAIK you're
> > > one of the initiators of the SPDX tagging initiative, and you even added
> > > tags to 10k+ files all over the tree :-) so we maybe can discuss this
> > > further, also with DD, in the hopes you're fine with this - sorry in
> > > advance if not.  
> > 
> > See my review of your first patch here, this needs to be done a lot
> > differently...
> 
> Check. Thanks for reviewing. The intent was to do a full cleanup of all
> licensing things in one go, per driver. Will do one patch for SPDX and
> eventual boilerplate cleanup for all drivers, one for MODULE_LICENSE
> and one for missing headers in the next iteration. Though I'd wait
> with that for now if you like to contact Ralph and Manfred, and do a v2
> based on the outcome.

You can always just do the "add a SPDX line" patches now, that touch
nothing else.  No one can get upset at that.

thanks,

greg k-h

[GIT PULL v2 FOR v4.17] cec: add error injection support + other improvements

2018-03-21 Thread Hans Verkuil

Same as the previous git pull except for moving and slightly editing the
CEC Pin Error Injection documentation and adding debugfs-cec-error-inj.

Regards,

Hans

The following changes since commit 3f127ce11353fd1071cae9b65bc13add6aec6b90:

  media: em28xx-cards: fix em28xx_duplicate_dev() (2018-03-08 06:06:51 -0500)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git cec

for you to fetch changes up to 6bf7995896c4b428f05825ec0c827c3089cdf97b:

  debugfs-cec-error-inj: document CEC error inj debugfs ABI (2018-03-21 
19:52:09 +0100)


Hans Verkuil (9):
  cec: add core error injection support
  cec-core.rst: document the error injection ops
  cec-pin: create cec_pin_start_timer() function
  cec-pin-error-inj: parse/show error injection
  cec-pin: add error injection support
  cec-pin: improve status log
  cec: improve CEC pin event handling
  cec-pin-error-inj.rst: document CEC Pin Error Injection
  debugfs-cec-error-inj: document CEC error inj debugfs ABI

 Documentation/ABI/testing/debugfs-cec-error-inj|  40 +++
 Documentation/media/kapi/cec-core.rst  |  72 +-
 Documentation/media/uapi/cec/cec-api.rst   |   1 +
 Documentation/media/uapi/cec/cec-pin-error-inj.rst | 325 

 MAINTAINERS|   1 +
 drivers/media/cec/Kconfig  |   6 +
 drivers/media/cec/Makefile |   4 +
 drivers/media/cec/cec-adap.c   |   8 +-
 drivers/media/cec/cec-core.c   |  58 +
 drivers/media/cec/cec-pin-error-inj.c  | 342 
+
 drivers/media/cec/cec-pin-priv.h   | 134 +-
 drivers/media/cec/cec-pin.c| 664 
+++--
 drivers/media/platform/vivid/vivid-cec.c   |   8 +-
 include/media/cec.h|  12 +-
 14 files changed, 1583 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/ABI/testing/debugfs-cec-error-inj
 create mode 100644 Documentation/media/uapi/cec/cec-pin-error-inj.rst
 create mode 100644 drivers/media/cec/cec-pin-error-inj.c

[PATCH] media: doc: fix ReST link syntax

2018-03-21 Thread Luca Ceresoli

There is a ':' i excess, resulting in an unwanted ':' in the rendered
output.

Signed-off-by: Luca Ceresoli 
---
 Documentation/media/kapi/v4l2-dev.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/kapi/v4l2-dev.rst 
b/Documentation/media/kapi/v4l2-dev.rst
index 7bb0505b60f1..eb03ccc41c41 100644
--- a/Documentation/media/kapi/v4l2-dev.rst
+++ b/Documentation/media/kapi/v4l2-dev.rst
@@ -31,7 +31,7 @@ of the video device exits.
 The default :c:func:`video_device_release` callback currently
 just calls ``kfree`` to free the allocated memory.
 
-There is also a ::c:func:`video_device_release_empty` function that does
+There is also a :c:func:`video_device_release_empty` function that does
 nothing (is empty) and should be used if the struct is embedded and there
 is nothing to do when it is released.
 
-- 
2.7.4

[PATCH] debugfs-cec-error-inj: document CEC error inj debugfs ABI

2018-03-21 Thread Hans Verkuil

Document the core of the debugfs CEC error injection ABI.

The driver specific commands are documented elsewhere and
this file points to that documentation.

Signed-off-by: Hans Verkuil 
---
 Documentation/ABI/testing/debugfs-cec-error-inj | 40 +
 MAINTAINERS |  1 +
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-cec-error-inj

diff --git a/Documentation/ABI/testing/debugfs-cec-error-inj 
b/Documentation/ABI/testing/debugfs-cec-error-inj
new file mode 100644
index ..122b65c5fe62
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-cec-error-inj
@@ -0,0 +1,40 @@
+What:  /sys/kernel/debug/cec/*/error-inj
+Date:  March 2018
+Contact:   Hans Verkuil 
+Description:
+
+The CEC Framework allows for CEC error injection commands through
+debugfs. Drivers that support this will create an error-inj file
+through which the error injection commands can be given.
+
+The basic syntax is as follows:
+
+Leading spaces/tabs are ignored. If the next character is a '#' or the
+end of the line was reached, then the whole line is ignored. Otherwise
+a command is expected.
+
+It is up to the driver to decide what commands to implement. The only
+exception is that the command 'clear' without any arguments must be
+implemented and that it will remove all current error injection
+commands.
+
+This ensures that you can always do 'echo clear >error-inj' to clear any
+error injections without having to know the details of the driver-specific
+commands.
+
+Note that the output of 'error-inj' shall be valid as input to 'error-inj'.
+So this must work:
+
+   $ cat error-inj >einj.txt
+   $ cat einj.txt >error-inj
+
+Other than these basic rules described above this ABI is not considered
+stable and may change in the future.
+
+Drivers that implement this functionality must document the commands as
+part of the CEC documentation and must keep that documentation up to date
+when changes are made.
+
+The following CEC error injection implementations exist:
+
+- Documentation/media/uapi/cec/cec-pin-error-inj.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e59769cec0e..55a3c61e9cfb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3307,6 +3307,7 @@ F:include/media/cec-notifier.h
 F: include/uapi/linux/cec.h
 F: include/uapi/linux/cec-funcs.h
 F: Documentation/devicetree/bindings/media/cec.txt
+F: Documentation/ABI/testing/debugfs-cec-error-inj

 CEC GPIO DRIVER
 M: Hans Verkuil 
-- 
2.15.1

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

2018-03-21 Thread Daniel Scheller

Hi Greg,

Am Wed, 21 Mar 2018 10:49:32 +0100
schrieb Greg KH :

> On Tue, Mar 20, 2018 at 10:01:27PM +0100, Daniel Scheller wrote:
> > From: Daniel Scheller 
> > 
> > This series adds SPDX license identifiers to all source files which are
> > copyright by either Digital Devices GmbH or Metzlerbros GbR, who are
> > the original authors of the ddbridge, ngene, cxd2099, mxl5xx, stv0910
> > and stv6111 bridge/demod/tuner drivers, with the mxl5xx driver being
> > based on source code released by MaxLinear.
> > [...]
> > The original intention was to fully replace all the licensing headers
> > with only the SPDX License Identifiers as it is done in a lot of other
> > in-tree drivers nowadays. However, Digital Devices disagreed to do this
> > and expressed major concerns regarding this, in that a machine readable
> > license tag instead of a full license boilerplate won't hold up equally,
> > so we agreed to keep the license boilerplate text as is right now.  
> 
> That's really odd, who at that company can I talk to about this?  Or
> really, what lawyer at that company can I point my lawyer at to talk
> about this, that's the only way this is going to get resolved.

I'm not entirely sure, but I guess for a first start it's best to
contact Ralph (from Metzlerbros) and Manfred (from Digital
Devices), being the authors and copyright owners of the DDDVB driver
package where the drivers originate from and thus is the upstream for
the mainlined copies of the mentioned drivers. Both are in the Cc list
(rjkm and mvoelkel) of this thread.

> If it helps, _ALL_ of the major companies that are kernel developers are
> onboard with the removal of the crazy boiler-plate text, so this tiny
> holdout should be easy to resolve.
> 
> > Greg, I'm Cc'ing you on this due to the last paragraph, as AFAIK you're
> > one of the initiators of the SPDX tagging initiative, and you even added
> > tags to 10k+ files all over the tree :-) so we maybe can discuss this
> > further, also with DD, in the hopes you're fine with this - sorry in
> > advance if not.  
> 
> See my review of your first patch here, this needs to be done a lot
> differently...

Check. Thanks for reviewing. The intent was to do a full cleanup of all
licensing things in one go, per driver. Will do one patch for SPDX and
eventual boilerplate cleanup for all drivers, one for MODULE_LICENSE
and one for missing headers in the next iteration. Though I'd wait
with that for now if you like to contact Ralph and Manfred, and do a v2
based on the outcome.

Thanks & best regards,
Daniel Scheller
-- 
https://github.com/herrnst

[PATCHv3] cec-pin-error-inj.rst: document CEC Pin Error Injection

2018-03-21 Thread Hans Verkuil

The CEC Pin framework adds support for Error Injection.

Document all the error injections commands and how to use it.

Signed-off-by: Hans Verkuil 
---
Changes since v2:
- replace '.. code-block:: none' by ::
- wrap to 80 columns
- move to uapi/cec
---
 Documentation/media/uapi/cec/cec-api.rst   |   1 +
 Documentation/media/uapi/cec/cec-pin-error-inj.rst | 325 +
 2 files changed, 326 insertions(+)
 create mode 100644 Documentation/media/uapi/cec/cec-pin-error-inj.rst

diff --git a/Documentation/media/uapi/cec/cec-api.rst 
b/Documentation/media/uapi/cec/cec-api.rst
index b68ca9c1d2e0..1e2cf498ba30 100644
--- a/Documentation/media/uapi/cec/cec-api.rst
+++ b/Documentation/media/uapi/cec/cec-api.rst
@@ -23,6 +23,7 @@ This part describes the CEC: Consumer Electronics Control

 cec-intro
 cec-funcs
+cec-pin-error-inj
 cec-header


diff --git a/Documentation/media/uapi/cec/cec-pin-error-inj.rst 
b/Documentation/media/uapi/cec/cec-pin-error-inj.rst
new file mode 100644
index ..464b006dbe0a
--- /dev/null
+++ b/Documentation/media/uapi/cec/cec-pin-error-inj.rst
@@ -0,0 +1,325 @@
+CEC Pin Framework Error Injection
+=
+
+The CEC Pin Framework is a core CEC framework for CEC hardware that only
+has low-level support for the CEC bus. Most hardware today will have
+high-level CEC support where the hardware deals with driving the CEC bus,
+but some older devices aren't that fancy. However, this framework also
+allows you to connect the CEC pin to a GPIO on e.g. a Raspberry Pi and
+you have now made a CEC adapter.
+
+What makes doing this so interesting is that since we have full control
+over the bus it is easy to support error injection. This is ideal to
+test how well CEC adapters can handle error conditions.
+
+Currently only the cec-gpio driver (when the CEC line is directly
+connected to a pull-up GPIO line) and the AllWinner A10/A20 drm driver
+support this framework.
+
+If ``CONFIG_CEC_PIN_ERROR_INJ`` is enabled, then error injection is available
+through debugfs. Specifically, in ``/sys/kernel/debug/cec/cecX/`` there is
+now an ``error-inj`` file.
+
+.. note::
+
+The error injection commands are not a stable ABI and may change in the
+future.
+
+With ``cat error-inj`` you can see both the possible commands and the current
+error injection status::
+
+   $ cat /sys/kernel/debug/cec/cec0/error-inj
+   # Clear error injections:
+   #   clear  clear all rx and tx error injections
+   #   rx-clear   clear all rx error injections
+   #   tx-clear   clear all tx error injections
+   #clear clear all rx and tx error injections for 
+   #rx-clear  clear all rx error injections for 
+   #tx-clear  clear all tx error injections for 
+   #
+   # RX error injection:
+   #   [,] rx-nack  NACK the message instead of 
sending an ACK
+   #   [,] rx-low-driveforce a low-drive condition at 
this bit position
+   #   [,] rx-add-byte  add a spurious byte to the 
received CEC message
+   #   [,] rx-remove-byte   remove the last byte from the 
received CEC message
+   #   [,] rx-arb-lostgenerate a POLL message to 
trigger an arbitration lost
+   #
+   # TX error injection settings:
+   #   tx-ignore-nack-until-eom   ignore early NACKs until EOM
+   #   tx-custom-low-usecs define the 'low' time for the 
custom pulse
+   #   tx-custom-high-usecsdefine the 'high' time for the 
custom pulse
+   #   tx-custom-pulsetransmit the custom pulse once 
the bus is idle
+   #
+   # TX error injection:
+   #   [,] tx-no-eomdon't set the EOM bit
+   #   [,] tx-early-eom set the EOM bit one byte too soon
+   #   [,] tx-add-bytesappend  (1-255) spurious 
bytes to the message
+   #   [,] tx-remove-byte   drop the last byte from the 
message
+   #   [,] tx-short-bitmake this bit shorter than 
allowed
+   #   [,] tx-long-bit make this bit longer than allowed
+   #   [,] tx-custom-bit   send the custom pulse instead of 
this bit
+   #   [,] tx-short-start   send a start pulse that's too 
short
+   #   [,] tx-long-startsend a start pulse that's too 
long
+   #   [,] tx-custom-start  send the custom pulse instead of 
the start pulse
+   #   [,] tx-last-bit stop sending after this bit
+   #   [,] tx-low-driveforce a low-drive condition at 
this bit position
+   #
+   #CEC message opcode (0-255) or 'any'
+   #  'once' (default), 'always', 'toggle' or 'off'
+   #   CEC message bit (0-159)
+   #10 bits per 'byte': bits 0-7: data, bit 8: EOM, bit 9: ACK
+   #  CEC poll message used to test arbitration lost (0x00-0xff, 
default 0x0f)
+   # microseconds (0-1000, default 1000)
+

Re: [PATCH 11/12] media: ov5640: Add 60 fps support

2018-03-21 Thread Maxime Ripard

Hi Hugues,

Thanks for all your feedback, I'll merge your suggested changes in the
next iteration.

On Tue, Mar 13, 2018 at 02:32:14PM +, Hugues FRUCHET wrote:
> > if (fi->numerator == 0) {
> > fi->denominator = maxfps;
> > fi->numerator = 1;
> > -   return OV5640_30_FPS;
> > +   return OV5640_60_FPS;
>
> [...]
>
> About 60 fps by default if (fi->numerator == 0): shouldn't we stick to a 
> default value supported by all modes such as 30fps ?

30 fps is not supported by all modes either, so I guess 15 fps would
be a better pick?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver

2018-03-21 Thread sakari.ai...@linux.intel.com

Hi Andy,

On Wed, Mar 21, 2018 at 03:58:42PM +, Yeh, Andy wrote:
> Thanks for the comments. A quick question first. For the reset we need some 
> time to address.
> 
> -Original Message-
> From: jacopo mondi [mailto:jac...@jmondi.org]
> Sent: Tuesday, March 20, 2018 6:28 PM
> To: Yeh, Andy 
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; 
> devicet...@vger.kernel.org; Chiang, AlanX 
> Subject: Re: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver
> 
> Hi Andy,
>a few comments on you patch below...
> 
> On Sat, Mar 17, 2018 at 01:05:26AM +0800, Andy Yeh wrote:
> > From: Alan Chiang  
> > a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c new file 
> > mode 100644 index 000..95626e9
> > --- /dev/null
> > +++ b/drivers/media/i2c/dw9807.c
> > @@ -0,0 +1,320 @@
> > +// Copyright (C) 2018 Intel Corporation // SPDX-License-Identifier: 
> > +GPL-2.0
> > +
> 
> Nit: my understanding is that the SPDX identifier goes first
> 
> https://lwn.net/Articles/739183/
> 
> I checked this site. And it says Copyright should be before SPDX identifier.
> == file01.c ==
> // Copyright (c) 2012-2016 Joe Random Hacker // SPDX-License-Identifier: 
> BSD-2-Clause ...
> == file02.c ==
> // Copyright (c) 2017 Jon Severinsson
> // SPDX-License-Identifier: BSD-2-Clause ...
> == file03.c ==
> // Copyright (c) 2008 The NetBSD Foundation, Inc.
> // SPDX-License-Identifier: BSD-2-Clause-NetBSD

This is an example which is AFAIU purported to show the problem with
various BSD licenses in a comment from someone. The order of the copyright
holder and license lines might be just random. The practice in kernel at
least appears to be SPDX license first.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

RE: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver

2018-03-21 Thread Yeh, Andy

Thanks for the comments. A quick question first. For the reset we need some 
time to address.

-Original Message-
From: jacopo mondi [mailto:jac...@jmondi.org]
Sent: Tuesday, March 20, 2018 6:28 PM
To: Yeh, Andy 
Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; 
devicet...@vger.kernel.org; Chiang, AlanX 
Subject: Re: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver

Hi Andy,
   a few comments on you patch below...

On Sat, Mar 17, 2018 at 01:05:26AM +0800, Andy Yeh wrote:
> From: Alan Chiang  
> a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c new file 
> mode 100644 index 000..95626e9
> --- /dev/null
> +++ b/drivers/media/i2c/dw9807.c
> @@ -0,0 +1,320 @@
> +// Copyright (C) 2018 Intel Corporation // SPDX-License-Identifier: 
> +GPL-2.0
> +

Nit: my understanding is that the SPDX identifier goes first

https://lwn.net/Articles/739183/

I checked this site. And it says Copyright should be before SPDX identifier.
== file01.c ==
// Copyright (c) 2012-2016 Joe Random Hacker // SPDX-License-Identifier: 
BSD-2-Clause ...
== file02.c ==
// Copyright (c) 2017 Jon Severinsson
// SPDX-License-Identifier: BSD-2-Clause ...
== file03.c ==
// Copyright (c) 2008 The NetBSD Foundation, Inc.
// SPDX-License-Identifier: BSD-2-Clause-NetBSD

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DW9807_NAME  "dw9807"
> +#define DW9807_MAX_FOCUS_POS 1023
>

Re: [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection

2018-03-21 Thread Mauro Carvalho Chehab

Em Mon,  5 Mar 2018 14:51:38 +0100
Hans Verkuil  escreveu:

> From: Hans Verkuil 
> 
> The CEC Pin framework adds support for Error Injection.
> 
> Document all the error injections commands and how to use it.

Please notice that all debugfs/sysfs entries should *also* be
documented at the standard way, e. g. by adding the corresponding
documentation at Documentation/ABI.

Please see Documentation/ABI/README.

Additionally, there are a few minor nitpicks on this patch.
See below.

The remaining patches looked ok on my eyes.

I'll wait for a v3 with the debugfs ABI documentation in order to merge
it. Feel free to put it on a separate patch.

Regards,
Mauro

> 
> Signed-off-by: Hans Verkuil 
> ---
>  .../media/cec-drivers/cec-pin-error-inj.rst| 322 
> +
>  Documentation/media/cec-drivers/index.rst  |   1 +
>  MAINTAINERS|   1 +
>  3 files changed, 324 insertions(+)
>  create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst
> 
> diff --git a/Documentation/media/cec-drivers/cec-pin-error-inj.rst 
> b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
> new file mode 100644
> index ..21bda831d3fb
> --- /dev/null
> +++ b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
> @@ -0,0 +1,322 @@
> +CEC Pin Framework Error Injection
> +=
> +
> +The CEC Pin Framework is a core CEC framework for CEC hardware that only
> +has low-level support for the CEC bus. Most hardware today will have
> +high-level CEC support where the hardware deals with driving the CEC bus,
> +but some older devices aren't that fancy. However, this framework also
> +allows you to connect the CEC pin to a GPIO on e.g. a Raspberry Pi and
> +you can become an instant CEC adapter.
> +
> +What makes doing this so interesting is that since we have full control
> +over the bus it is easy to support error injection. This is ideal to
> +test how well CEC adapters can handle error conditions.
> +
> +Currently only the cec-gpio driver (when the CEC line is directly
> +connected to a pull-up GPIO line) and the AllWinner A10/A20 drm driver
> +support this framework.
> +
> +If ``CONFIG_CEC_PIN_ERROR_INJ`` is enabled, then error injection is available
> +through debugfs. Specifically, in ``/sys/kernel/debug/cec/cecX/`` there is
> +now an ``error-inj`` file.
> +
> +With ``cat error-inj`` you can see both the possible commands and the current
> +error injection status:
> +
> +.. code-block:: none

It is usually better to use "::" instead of ".. code-block".

> +
> + $ cat /sys/kernel/debug/cec/cec0/error-inj
> + # Clear error injections:
> + #   clear  clear all rx and tx error injections
> + #   rx-clear   clear all rx error injections
> + #   tx-clear   clear all tx error injections
> + #clear clear all rx and tx error injections for 
> + #rx-clear  clear all rx error injections for 
> + #tx-clear  clear all tx error injections for 
> + #
> + # RX error injection:
> + #   [,] rx-nack  NACK the message instead of 
> sending an ACK
> + #   [,] rx-low-driveforce a low-drive condition at 
> this bit position
> + #   [,] rx-add-byte  add a spurious byte to the 
> received CEC message
> + #   [,] rx-remove-byte   remove the last byte from the 
> received CEC message
> + #   [,] rx-arb-lostgenerate a POLL message to 
> trigger an arbitration lost
> + #
> + # TX error injection settings:
> + #   tx-ignore-nack-until-eom   ignore early NACKs until EOM
> + #   tx-custom-low-usecs define the 'low' time for the 
> custom pulse
> + #   tx-custom-high-usecsdefine the 'high' time for the 
> custom pulse
> + #   tx-custom-pulsetransmit the custom pulse once 
> the bus is idle
> + #
> + # TX error injection:
> + #   [,] tx-no-eomdon't set the EOM bit
> + #   [,] tx-early-eom set the EOM bit one byte too soon
> + #   [,] tx-add-bytesappend  (1-255) spurious 
> bytes to the message
> + #   [,] tx-remove-byte   drop the last byte from the 
> message
> + #   [,] tx-short-bitmake this bit shorter than 
> allowed
> + #   [,] tx-long-bit make this bit longer than allowed
> + #   [,] tx-custom-bit   send the custom pulse instead of 
> this bit
> + #   [,] tx-short-start   send a start pulse that's too 
> short
> + #   [,] tx-long-startsend a start pulse that's too 
> long
> + #   [,] tx-custom-start  send the custom pulse instead of 
> the start pulse
> + #   [,] tx-last-bit stop sending after this bit
> + #   [,] tx-low-driveforce a low-drive condition at 
> this bit position
> + #
> + #CEC message opcode (0-255) or 'any'
> + #  'once' (default),

[PATCH] media: uvcvideo: Prevent setting unavailable flags

2018-03-21 Thread Kieran Bingham

The addition of an extra operation to use the GET_INFO command
overwrites all existing flags from the uvc_ctrls table. This includes
setting all controls as supporting  GET_MIN, GET_MAX, GET_RES, and
GET_DEF regardless of whether they do or not.

Move the initialisation of these control capabilities directly to the
uvc_ctrl_fill_xu_info() call where they were originally located in that
use case, and ensure that the new functionality in uvc_ctrl_get_flags()
will only set flags based on their reported capability from the GET_INFO
call.

Fixes: 859086ae3636 ("media: uvcvideo: Apply flags from device to actual
properties")

Signed-off-by: Kieran Bingham 
---
 drivers/media/usb/uvc/uvc_ctrl.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 1daf444371be..4042cbdb721b 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1607,14 +1607,12 @@ static int uvc_ctrl_get_flags(struct uvc_device *dev,
ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
 info->selector, data, 1);
if (!ret)
-   info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
-   | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF
-   | (data[0] & UVC_CONTROL_CAP_GET ?
-  UVC_CTRL_FLAG_GET_CUR : 0)
-   | (data[0] & UVC_CONTROL_CAP_SET ?
-  UVC_CTRL_FLAG_SET_CUR : 0)
-   | (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
-  UVC_CTRL_FLAG_AUTO_UPDATE : 0);
+   info->flags |= (data[0] & UVC_CONTROL_CAP_GET ?
+   UVC_CTRL_FLAG_GET_CUR : 0)
+   |  (data[0] & UVC_CONTROL_CAP_SET ?
+   UVC_CTRL_FLAG_SET_CUR : 0)
+   |  (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
+   UVC_CTRL_FLAG_AUTO_UPDATE : 0);
 
kfree(data);
return ret;
@@ -1689,6 +1687,9 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
 
info->size = le16_to_cpup((__le16 *)data);
 
+   info->flags = UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
+   | UVC_CTRL_FLAG_GET_RES | UVC_CTRL_FLAG_GET_DEF;
+
ret = uvc_ctrl_get_flags(dev, ctrl, info);
if (ret < 0) {
uvc_trace(UVC_TRACE_CONTROL,
-- 
2.7.4

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

2018-03-21 Thread Mauro Carvalho Chehab

Em Tue, 20 Mar 2018 22:01:27 +0100
Daniel Scheller  escreveu:

> Ralph, Manfred, I'm Cc'ing you on these five patches aswell, so if there
> are any concerns left regarding the changes proposed by these patches,
> please raise them.

It should be the reverse:

Ralph/Manfred,

When Daniel submits a version 2 of this patch series doing the changes
requested by Greg, please reply with "Acked-by: your name "
in order to clearly state that you're OK with such changes.

Thanks,
Mauro

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

2018-03-21 Thread Paul Menzel


Dear Laurent,


On 03/21/2018 10:25 AM, Laurent Pinchart wrote:


On Tuesday, 20 March 2018 18:46:24 EET Paul Menzel wrote:

On 03/20/18 14:30, Laurent Pinchart wrote:

On Tuesday, 20 March 2018 14:20:14 EET Paul Menzel wrote:

On the Dell XPS 13 9370, Linux 4.16-rc6 outputs the messages below.

```


[…]


[2.340736] input: Integrated_Webcam_HD: Integrate as
/devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/input/input9
[2.341447] uvcvideo: Unknown video format
0032-0002-0010-8000-00aa00389b71 >> [2.341450] uvcvideo: Found
UVC 1.00 device Integrated_Webcam_HD (0bda:58f4)


[…]


```

Please tell me, what I can do to improve the situation.


Some vendors routinely implement new formats without bothering to send a
patch for the uvcvideo driver. It would be easy to do so, but it requires
knowing which format is meant by the GUID. Most format GUIDs are of the
form 32595559--0010-8000-00aa00389b71 that starts with a 4CC, but
that's not the case here.


I am adding Mario to the receiver list, though he is currently on vacation.


Could you send me the output of

lsusb -v -d 0bda:58f4

running as root if possible ?


Sure, please find it attached.


Thank you.

Could you please try the following patch ?

commit 7b3dea984b380f5b4b5c1956a9c6c23966af2149
Author: Laurent Pinchart 
Date:   Wed Mar 21 11:16:40 2018 +0200

 media: uvcvideo: Add KSMedia 8-bit IR format support
 
 Add support for the 8-bit IR format GUID defined in the Microsoft Kernel

 Streaming Media API.
 
 Reported-by: Paul Menzel 

 Signed-off-by: Laurent Pinchart 

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 2469b49b2b30..3691d87ef869 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -99,6 +99,11 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid   = UVC_GUID_FORMAT_D3DFMT_L8,
.fcc= V4L2_PIX_FMT_GREY,
},
+   {
+   .name   = "IR 8-bit (L8_IR)",
+   .guid   = UVC_GUID_FORMAT_KSMEDIA_L8_IR,
+   .fcc= V4L2_PIX_FMT_GREY,
+   },
{
.name   = "Greyscale 10-bit (Y10 )",
.guid   = UVC_GUID_FORMAT_Y10,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index be5cf179228b..6b955e0dd956 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -157,6 +157,9 @@
  #define UVC_GUID_FORMAT_D3DFMT_L8 \
{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
+   {0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
+0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
  
  
  /* 


Sure. After fighting how to apply with Mozilla Thunderbird as my mailer 
– hints are welcome –, the warning is gone.


```
[2.569788] calling  uvc_init+0x0/0x1000 [uvcvideo] @ 297
[2.570011] calling  cryptd_init+0x0/0x1000 [cryptd] @ 287
[2.570018] cryptd: max_cpu_qlen set to 1000
[2.570022] initcall cryptd_init+0x0/0x1000 [cryptd] returned 0 after 
7 usecs

[2.570030] calling  init_nls_cp437+0x0/0x1000 [nls_cp437] @ 332
[2.570033] initcall init_nls_cp437+0x0/0x1000 [nls_cp437] returned 0 
after 0 usecs
[2.570502] uvcvideo: Found UVC 1.00 device Integrated_Webcam_HD 
(0bda:58f4)
[2.573583] uvcvideo 1-5:1.0: Entity type for entity Extension 4 was 
not initialized!
[2.573585] uvcvideo 1-5:1.0: Entity type for entity Extension 7 was 
not initialized!
[2.573586] uvcvideo 1-5:1.0: Entity type for entity Processing 2 was 
not initialized!
[2.573587] uvcvideo 1-5:1.0: Entity type for entity Camera 1 was not 
initialized!
[2.573652] input: Integrated_Webcam_HD: Integrate as 
/devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/input/input10
[2.574192] uvcvideo: Found UVC 1.00 device Integrated_Webcam_HD 
(0bda:58f4)

[2.575629] proc_thermal :00:04.0: enabling device ( -> 0002)
[2.576052] uvcvideo: Unable to create debugfs 1-2 directory.
[2.576118] uvcvideo 1-5:1.2: Entity type for entity Extension 10 was 
not initialized!
[2.576119] uvcvideo 1-5:1.2: Entity type for entity Extension 12 was 
not initialized!
[2.576120] uvcvideo 1-5:1.2: Entity type for entity Processing 9 was 
not initialized!
[2.576121] uvcvideo 1-5:1.2: Entity type for entity Camera 11 was 
not initialized!
[2.576184] input: Integrated_Webcam_HD: Integrate as 
/devices/pci:00/:00:14.0/usb1/1-5/1-5:1.2/input/input11

[2.576229] usbcore: registered new interface driver uvcvideo
[2.576230] USB Video Class driver (1.1.1)
[2.576236] initcall uvc_init+0x0/0x1000

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

2018-03-21 Thread Christian König


Am 21.03.2018 um 09:28 schrieb Daniel Vetter:

On Tue, Mar 20, 2018 at 06:47:57PM +0100, Christian König wrote:

Am 20.03.2018 um 15:08 schrieb Daniel Vetter:

[SNIP]
For the in-driver reservation path (CS) having a slow-path that grabs a
temporary reference, drops the vram lock and then locks the reservation
normally (using the acquire context used already for the entire CS) is a
bit tricky, but totally feasible. Ttm doesn't do that though.

That is exactly what we do in amdgpu as well, it's just not very efficient
nor reliable to retry getting the right pages for a submission over and over
again.

Out of curiosity, where's that code? I did read the ttm eviction code way
back, and that one definitely didn't do that. Would be interesting to
update my understanding.


That is in amdgpu_cs.c. amdgpu_cs_parser_bos() does a horrible dance 
with grabbing, releasing and regrabbing locks in a loop.


Then in amdgpu_cs_submit() we grab an lock preventing page table updates 
and check if all pages are still the one we want to have:

    amdgpu_mn_lock(p->mn);
    if (p->bo_list) {
    for (i = p->bo_list->first_userptr;
 i < p->bo_list->num_entries; ++i) {
    struct amdgpu_bo *bo = p->bo_list->array[i].robj;

    if 
(amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {

    amdgpu_mn_unlock(p->mn);
    return -ERESTARTSYS;
    }
    }
    }


If anything changed on the page tables we restart the whole IOCTL using 
-ERESTARTSYS and try again.


Regards,
Christian.


-Daniel

Re: [PATCH v2] media: omap3isp: fix unbalanced dma_iommu_mapping

2018-03-21 Thread Laurent Pinchart

Hi Suman,

Thank you for the patch.

On Wednesday, 14 March 2018 17:41:36 EET Suman Anna wrote:
> The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware
> ARM DMA backend. The current code creates a dma_iommu_mapping and
> attaches this to the ISP device, but never detaches the mapping in
> either the probe failure paths or the driver remove path resulting
> in an unbalanced mapping refcount and a memory leak. Fix this properly.
> 
> Reported-by: Pavel Machek 
> Signed-off-by: Suman Anna 
> Acked-by: Sakari Ailus 

Reviewed-by: Laurent Pinchart 

> ---
> v2 Changes:
>  - Dropped the error_attach label, and returned directly from the
>first error path (comments from Sakari)
>  - Added Sakari's Acked-by
> v1: https://patchwork.kernel.org/patch/10276759/
> 
> Pavel,
> I dropped your Tested-by from v2 since I modified the patch, can you
> recheck the new patch again? Thanks.
> 
> regards
> Suman
> 
>  drivers/media/platform/omap3isp/isp.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..f2db5128d786
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1945,6 +1945,7 @@ static int isp_initialize_modules(struct isp_device
> *isp)
> 
>  static void isp_detach_iommu(struct isp_device *isp)
>  {
> + arm_iommu_detach_device(isp->dev);
>   arm_iommu_release_mapping(isp->mapping);
>   isp->mapping = NULL;
>  }
> @@ -1961,8 +1962,7 @@ static int isp_attach_iommu(struct isp_device *isp)
>   mapping = arm_iommu_create_mapping(_bus_type, SZ_1G, SZ_2G);
>   if (IS_ERR(mapping)) {
>   dev_err(isp->dev, "failed to create ARM IOMMU mapping\n");
> - ret = PTR_ERR(mapping);
> - goto error;
> + return PTR_ERR(mapping);
>   }
> 
>   isp->mapping = mapping;
> @@ -1977,7 +1977,8 @@ static int isp_attach_iommu(struct isp_device *isp)
>   return 0;
> 
>  error:
> - isp_detach_iommu(isp);
> + arm_iommu_release_mapping(isp->mapping);
> + isp->mapping = NULL;
>   return ret;
>  }

-- 
Regards,

Laurent Pinchart

Re: Double-free in /drivers/media/usb/zr364xx driver

2018-03-21 Thread Dan Carpenter

Hi Linux Media Devs,

There is a double free on error in zr364xx_probe().  The bug report
explains it pretty well.  v4l2_device_unregister() calls
zr364xx_release() which frees "cam" but we also to another kfree(cam);
before the "return err;".

Please give reported by credit to:

Reported-by: "Yavuz, Tuba" 

regards,
dan carpenter

On Tue, Mar 20, 2018 at 02:30:45PM +, Yavuz, Tuba wrote:
> Hello,
> 
> 
> It looks like there is a double-free on an error path in the zr364xx_probe 
> function of the zr364xx driver.
> 
> fail:
> v4l2_ctrl_handler_free(hdl);
> v4l2_device_unregister(>v4l2_dev);
> =>
> v4l2_device_disconnect
>=>
>   put_device
>   =>
>   kobject_put
>   =>
>   kref_put
>   =>
>   v4l2_device_release
>   =>
>   zr364xx_release (CALLBACK)
>   =>
>  kfree(cam)
> kfree(cam);
> 
> The vulnerability exists since the initial 
> commit
>  0aa77f6c2954896b132f8b6f2e9f063c52800913 of the driver.
> 
> 
> Best,
> 
> Tuba Yavuz, Ph.D.
> Assistant Professor
> Electrical and Computer Engineering Department
> University of Florida
> Gainesville, FL 32611
> Webpage: http://www.tuba.ece.ufl.edu/
> Email: t...@ece.ufl.edu
> Phone: (352) 846 0202

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

2018-03-21 Thread Greg KH

On Tue, Mar 20, 2018 at 10:01:27PM +0100, Daniel Scheller wrote:
> From: Daniel Scheller 
> 
> This series adds SPDX license identifiers to all source files which are
> copyright by either Digital Devices GmbH or Metzlerbros GbR, who are
> the original authors of the ddbridge, ngene, cxd2099, mxl5xx, stv0910
> and stv6111 bridge/demod/tuner drivers, with the mxl5xx driver being
> based on source code released by MaxLinear.
> 
> All source code either carries the license text "redistribute and/or
> modify it under the terms of the GNU GPL version 2 only as published
> by the FSF", or simply "... GPL version 2" in the case of the mxl5xx
> driver, which all should equal to the SPDX License Identifier
> "GPL-2.0-only" as of SPDX License List Version 3.0 published on
> December the 28th, 2017, which is applied as license tag to all files
> of the mentioned drivers by this series.
> 
> During checking of those modules I noticed that the module info carries
> the "GPL" version tag, which, according to include/linux/module.h, equals
> to "GPL version 2 or later", which (I believe) in turn is a mismatch to
> what is written in the file header's license boilerplates. This series
> corrects this by setting all MODULE_LICENSE() descriptors to "GPL v2",
> which equals to the "GNU GPL version 2 only" phrase.
> 
> Besides that, this fixes some whitespace cosmetics in the headers, and
> removes the link to gnu.org (if existing), which points to the GPLv3
> license anyway.
> 
> The original intention was to fully replace all the licensing headers
> with only the SPDX License Identifiers as it is done in a lot of other
> in-tree drivers nowadays. However, Digital Devices disagreed to do this
> and expressed major concerns regarding this, in that a machine readable
> license tag instead of a full license boilerplate won't hold up equally,
> so we agreed to keep the license boilerplate text as is right now.

That's really odd, who at that company can I talk to about this?  Or
really, what lawyer at that company can I point my lawyer at to talk
about this, that's the only way this is going to get resolved.

If it helps, _ALL_ of the major companies that are kernel developers are
onboard with the removal of the crazy boiler-plate text, so this tiny
holdout should be easy to resolve.

> Greg, I'm Cc'ing you on this due to the last paragraph, as AFAIK you're
> one of the initiators of the SPDX tagging initiative, and you even added
> tags to 10k+ files all over the tree :-) so we maybe can discuss this
> further, also with DD, in the hopes you're fine with this - sorry in
> advance if not.

See my review of your first patch here, this needs to be done a lot
differently...

thanks,

greg k-h

Re: [PATCH 1/5] [media] stv0910/stv6111: add SPDX license headers

2018-03-21 Thread Greg KH

On Tue, Mar 20, 2018 at 10:01:28PM +0100, Daniel Scheller wrote:
> From: Daniel Scheller 
> 
> Add SPDX license headers to the stv0910 and stv6111 DVB frontend
> drivers. Both drivers are licensed as GPL-2.0-only, so fix this in the
> MODULE_LICENSE while at it. Also, the includes were lacking any license
> headers at all, so add them now.
> 
> Signed-off-by: Daniel Scheller 
> ---
>  drivers/media/dvb-frontends/stv0910.c | 5 +++--
>  drivers/media/dvb-frontends/stv0910.h | 9 +
>  drivers/media/dvb-frontends/stv6111.c | 6 +++---
>  drivers/media/dvb-frontends/stv6111.h | 7 +++
>  4 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/stv0910.c 
> b/drivers/media/dvb-frontends/stv0910.c
> index 52355c14fd64..ce82264e99ef 100644
> --- a/drivers/media/dvb-frontends/stv0910.c
> +++ b/drivers/media/dvb-frontends/stv0910.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-only

Please only use the identifiers documented in
Documentation/process/license-rules.rst right now.  We wrote and got
that merged _before_ SPDX bumped the tags to 3.0 which added the (imo
crazy) -only variants.

So please stick with what is already in the kernel tree, if we do decide
to update to a newer version of SPDX, we will hit the tree all at once
with a script to give to Linus to run.


>  /*
>   * Driver for the ST STV0910 DVB-S/S2 demodulator.
>   *
> @@ -11,7 +12,7 @@
>   *
>   * This program is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

Why did you change this in this patch?

Please only do "one" thing per patch.



>   * GNU General Public License for more details.
>   */
>  
> @@ -1836,4 +1837,4 @@ EXPORT_SYMBOL_GPL(stv0910_attach);
>  
>  MODULE_DESCRIPTION("ST STV0910 multistandard frontend driver");
>  MODULE_AUTHOR("Ralph and Marcus Metzler, Manfred Voelkel");
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");

Again, this should be a separate patch.


> diff --git a/drivers/media/dvb-frontends/stv0910.h 
> b/drivers/media/dvb-frontends/stv0910.h
> index fccd8d9b665f..93de08540ce4 100644
> --- a/drivers/media/dvb-frontends/stv0910.h
> +++ b/drivers/media/dvb-frontends/stv0910.h
> @@ -1,3 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Driver for the ST STV0910 DVB-S/S2 demodulator.
> + *
> + * Copyright (C) 2014-2015 Ralph Metzler 
> + * Marcus Metzler 
> + * developed for Digital Devices GmbH
> + */

Where did that copyright notice come from?

This patch is a total mix of different things, please do not do that at
all!

thanks,

greg k-h

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

2018-03-21 Thread Christian König


Am 21.03.2018 um 09:18 schrieb Daniel Vetter:

[SNIP]
They're both in i915_gem_userptr.c, somewhat interleaved. Would be
interesting if you could show what you think is going wrong in there
compared to amdgpu_mn.c.


i915 implements only one callback:

static const struct mmu_notifier_ops i915_gem_userptr_notifier = {
    .invalidate_range_start = 
i915_gem_userptr_mn_invalidate_range_start,

};
For correct operation you always need to implement invalidate_range_end 
as well and add some lock/completion work Otherwise get_user_pages() can 
again grab the reference to the wrong page.


The next problem seems to be that cancel_userptr() doesn't prevent any 
new command submission. E.g.

i915_gem_object_wait(obj, I915_WAIT_ALL, MAX_SCHEDULE_TIMEOUT, NULL);
What prevents new command submissions to use the GEM object directly 
after you finished waiting here?



I get a feeling we're talking past each another here.

Yeah, agree. Additional to that I don't know the i915 code very well.


Can you perhaps explain what exactly the race is you're seeing? The i915 
userptr code is
fairly convoluted and pushes a lot of stuff to workers (but then syncs
with those workers again later on), so easily possible you've overlooked
one of these lines that might guarantee already what you think needs to be
guaranteed. We're definitely not aiming to allow userspace to allow
writing to random pages all over.


You not read/write to random pages, there still is a reference to the 
page. So that the page can't be reused until you are done.


The problem is rather that you can't guarantee that you write to the 
page which is mapped into the process at that location. E.g. the CPU and 
the GPU might see two different things.



Leaking the IOMMU mappings otoh means rogue userspace could do a bunch of
stray writes (I don't see anywhere code in amdgpu_mn.c to unmap at least
the gpu side PTEs to make stuff inaccessible) and wreak the core kernel's
book-keeping.

In i915 we guarantee that we call set_page_dirty/mark_page_accessed only
after all the mappings are really gone (both GPU PTEs and sg mapping),
guaranteeing that any stray writes from either the GPU or IOMMU will
result in faults (except bugs in the IOMMU, but can't have it all, "IOMMU
actually works" is an assumption behind device isolation).

Well exactly that's the point, the handling in i915 looks incorrect to me.
You need to call set_page_dirty/mark_page_accessed way before the mapping is
destroyed.

To be more precise for userptrs it must be called from the
invalidate_range_start, but i915 seems to delegate everything into a
background worker to avoid the locking problems.

Yeah, and at the end of the function there's a flush_work to make sure the
worker has caught up.

Ah, yes haven't seen that.

But then grabbing the obj->base.dev->struct_mutex lock in 
cancel_userptr() is rather evil. You just silenced lockdep because you 
offloaded that into a work item.


So no matter how you put it i915 is clearly doing something wrong here :)


I know. i915 gem has tons of fallbacks and retry loops (we restart the
entire CS if needed), and i915 userptr pushes the entire get_user_pages
dance off into a worker if the fastpath doesn't succeed and we run out of
memory or hit contended locks. We also have obscene amounts of
__GFP_NORETRY and __GFP_NOWARN all over the place to make sure the core mm
code doesn't do something we don't want it do to do in the fastpaths
(because there's really no point in spending lots of time trying to make
memory available if we have a slowpath fallback with much less
constraints).
Well I haven't audited the code, but I'm pretty sure that just mitigates 
the problem and silenced lockdep instead of really fixing the issue.



We're also not limiting ourselves to GFP_NOIO, but instead have a
recursion detection in our own shrinker callback to avoid these
deadlocks.


Which if you ask me is absolutely horrible. I mean the comment in 
linux/mutex.h sums it up pretty well:
 * This function should not be used, _ever_. It is purely for 
hysterical GEM

 * raisins, and once those are gone this will be removed.


Regards,
Christian.

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

2018-03-21 Thread Laurent Pinchart

Hi Paul,

On Tuesday, 20 March 2018 18:46:24 EET Paul Menzel wrote:
> On 03/20/18 14:30, Laurent Pinchart wrote:
> > On Tuesday, 20 March 2018 14:20:14 EET Paul Menzel wrote:
> >> On the Dell XPS 13 9370, Linux 4.16-rc6 outputs the messages below.
> >> 
> >> ```
> 
> […]
> 
> >> [2.340736] input: Integrated_Webcam_HD: Integrate as
> >> /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/input/input9
> >> [2.341447] uvcvideo: Unknown video format
> >> 0032-0002-0010-8000-00aa00389b71 >> [2.341450] uvcvideo: Found
> >> UVC 1.00 device Integrated_Webcam_HD (0bda:58f4)
> 
> […]
> 
> >> ```
> >> 
> >> Please tell me, what I can do to improve the situation.
> > 
> > Some vendors routinely implement new formats without bothering to send a
> > patch for the uvcvideo driver. It would be easy to do so, but it requires
> > knowing which format is meant by the GUID. Most format GUIDs are of the
> > form 32595559--0010-8000-00aa00389b71 that starts with a 4CC, but
> > that's not the case here.
> 
> I am adding Mario to the receiver list, though he is currently on vacation.
> 
> > Could you send me the output of
> > 
> > lsusb -v -d 0bda:58f4
> > 
> > running as root if possible ?
> 
> Sure, please find it attached.

Thank you.

Could you please try the following patch ?

commit 7b3dea984b380f5b4b5c1956a9c6c23966af2149
Author: Laurent Pinchart 
Date:   Wed Mar 21 11:16:40 2018 +0200

media: uvcvideo: Add KSMedia 8-bit IR format support

Add support for the 8-bit IR format GUID defined in the Microsoft Kernel
Streaming Media API.

Reported-by: Paul Menzel 
Signed-off-by: Laurent Pinchart 

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index 2469b49b2b30..3691d87ef869 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -99,6 +99,11 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid   = UVC_GUID_FORMAT_D3DFMT_L8,
.fcc= V4L2_PIX_FMT_GREY,
},
+   {
+   .name   = "IR 8-bit (L8_IR)",
+   .guid   = UVC_GUID_FORMAT_KSMEDIA_L8_IR,
+   .fcc= V4L2_PIX_FMT_GREY,
+   },
{
.name   = "Greyscale 10-bit (Y10 )",
.guid   = UVC_GUID_FORMAT_Y10,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index be5cf179228b..6b955e0dd956 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -157,6 +157,9 @@
 #define UVC_GUID_FORMAT_D3DFMT_L8 \
{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_KSMEDIA_L8_IR \
+   {0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
+0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
 
 
 /* 

-- 
Regards,

Laurent Pinchart

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

2018-03-21 Thread Laurent Pinchart

Hi Nicolas,

On Wednesday, 21 March 2018 05:38:59 EET Nicolas Dufresne wrote:
> Le mardi 20 mars 2018 à 20:04 +0200, Laurent Pinchart a écrit :
> > On Tuesday, 20 March 2018 19:45:51 EET Nicolas Dufresne wrote:
> > > Le mardi 20 mars 2018 à 13:20 +0100, Paul Menzel a écrit :
> > > > Dear Linux folks,
> > > > 
> > > > 
> > > > On the Dell XPS 13 9370, Linux 4.16-rc6 outputs the messages below.
> > > > 
> > > > ```
> > > > [2.338094] calling  uvc_init+0x0/0x1000 [uvcvideo] @ 295
> > > > [2.338569] calling  iTCO_wdt_init_module+0x0/0x1000 [iTCO_wdt] @
> > > > 280
> > > > [2.338570] iTCO_wdt: Intel TCO WatchDog Timer Driver v1.11
> > > > [2.338713] iTCO_wdt: Found a Intel PCH TCO device (Version=4,
> > > > TCOBASE=0x0400)
> > > > [2.338755] uvcvideo: Found UVC 1.00 device Integrated_Webcam_HD
> > > > (0bda:58f4)
> > > > [2.338827] iTCO_wdt: initialized. heartbeat=30 sec (nowayout=0)
> > > > [2.338851] initcall iTCO_wdt_init_module+0x0/0x1000 [iTCO_wdt]
> > > > returned 0 after 271 usecs
> > > > [2.340669] uvcvideo 1-5:1.0: Entity type for entity Extension 4
> > > > was
> > > > not initialized!
> > > > [2.340670] uvcvideo 1-5:1.0: Entity type for entity Extension 7
> > > > was
> > > > not initialized!
> > > > [2.340672] uvcvideo 1-5:1.0: Entity type for entity Processing 2
> > > > was
> > > > not initialized!
> > > > [2.340673] uvcvideo 1-5:1.0: Entity type for entity Camera 1 was
> > > > not
> > > > initialized!
> > > > [2.340736] input: Integrated_Webcam_HD: Integrate as
> > > > /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.0/input/input9
> > > > [2.341447] uvcvideo: Unknown video format
> > > > 0032-0002-0010-8000-00aa00389b71
> > > 
> > > While the 0002 is suspicious, this is pretty close to a color format.
> > > I've recently come across of similar format using D3DFORMAT instead of
> > > GUID. According to the vendor*, this camera module includes an infrared
> > > camera (340x340), so I suspect this is to specify the format it
> > > outputs. A good guess to start with would be that this is
> > > D3DFMT_X8L8V8U8 (0x32).
> > 
> > Isn't 0x32 D3DFMT_L8, not D3DFMT_X8L8V8U8 ?
> 
> You are right, sorry about that, I totally miss-translate. It felt
> weird. This is much more likely yes. So maybe it's the same mapping
> (but with the -2- instead) as what I added for the HoloLense
> Camera.
> 
> > > To test it, you could map this
> > > V4L2_PIX_FMT_YUV32/xRGB and see if the driver is happy with the buffer
> > > size.
> > 
> > VideoStreaming Interface Descriptor:
> > bLength30
> > bDescriptorType36
> > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > bFrameIndex 1
> > bmCapabilities   0x00
> > 
> >   Still image unsupported
> > 
> > wWidth340
> > wHeight   340
> > dwMinBitRate 55488000
> > dwMaxBitRate 55488000
> > dwMaxVideoFrameBufferSize  115600
> > dwDefaultFrameInterval 16
> > bFrameIntervalType  1
> > dwFrameInterval( 0)16
> > 
> > 340*340 is 115600, so this should be a 8-bit format.
> 
> Indeed, that matches.
> 
> > > Then render it to make sure it looks some image of some sort. A
> > > new format will need to be defined as this format is in the wrong
> > > order, and is ambiguous (it may mean AYUV or xYUV). I'm not sure if we
> > > need specific formats to differentiate infrared data from YUV images,
> > > need to be discussed.
> > 
> > If the format is indeed D3DFMT_L8, it should map to V4L2_PIX_FMT_GREY
> > (8-bit luminance). I suspect the camera transmits a depth map though.
> 
> I wonder if we should think of a way to tell userspace this is fnfrared
> data rather then black and white ?

I think we need such a mechanism, yes. Would you like to propose one ? :-)

I've found https://www.magnumdb.com/search?q=value%3A
%220032-0002-0010-8000-00aa00389b71%22 and 
https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/infrared-stream-support-in-uvc
 that confirm 
this is a 8-bit IR format.

> > > *https://dustinweb.azureedge.net/media/338953/xps-13-9370.pdf
> > > 
> > > > [2.341450] uvcvideo: Found UVC 1.00 device Integrated_Webcam_HD
> > > > (0bda:58f4)
> > > > [2.343371] uvcvideo: Unable to create debugfs 1-2 directory.
> > > > [2.343420] uvcvideo 1-5:1.2: Entity type for entity Extension 10
> > > > was
> > > > not initialized!
> > > > [2.343422] uvcvideo 1-5:1.2: Entity type for entity Extension 12
> > > > was
> > > > not initialized!
> > > > [2.343423] uvcvideo 1-5:1.2: Entity type for entity Processing 9
> > > > was
> > > > not initialized!
> > > > [2.343424] uvcvideo 1-5:1.2: Entity type for entity Camera 11 was
> > > > not initialized!
> > > > [

[PATCH] media: ov5645: Use v4l2_find_nearest_size

2018-03-21 Thread Todor Tomov

Use v4l2_find_nearest_size instead of a driver specific function to find
nearest matching size.

Signed-off-by: Todor Tomov 
---

This depends on [1] from Sakari. Thank you, Sakari, this is nice.

[1] 
https://git.linuxtv.org/sailus/media_tree.git/commit/?h=v4l2-common-size=83fdb8a0ab43fc86c329f63f1052e6113871a965

 drivers/media/i2c/ov5645.c | 24 +---
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index d28845f..79db74c 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -959,23 +959,6 @@ __ov5645_get_pad_crop(struct ov5645 *ov5645, struct 
v4l2_subdev_pad_config *cfg,
}
 }
 
-static const struct ov5645_mode_info *
-ov5645_find_nearest_mode(unsigned int width, unsigned int height)
-{
-   int i;
-
-   for (i = ARRAY_SIZE(ov5645_mode_info_data) - 1; i >= 0; i--) {
-   if (ov5645_mode_info_data[i].width <= width &&
-   ov5645_mode_info_data[i].height <= height)
-   break;
-   }
-
-   if (i < 0)
-   i = 0;
-
-   return _mode_info_data[i];
-}
-
 static int ov5645_set_format(struct v4l2_subdev *sd,
 struct v4l2_subdev_pad_config *cfg,
 struct v4l2_subdev_format *format)
@@ -989,8 +972,11 @@ static int ov5645_set_format(struct v4l2_subdev *sd,
__crop = __ov5645_get_pad_crop(ov5645, cfg, format->pad,
format->which);
 
-   new_mode = ov5645_find_nearest_mode(format->format.width,
-   format->format.height);
+   new_mode = v4l2_find_nearest_size(ov5645_mode_info_data,
+  ARRAY_SIZE(ov5645_mode_info_data),
+  width, height,
+  format->format.width, format->format.height);
+
__crop->width = new_mode->width;
__crop->height = new_mode->height;
 
-- 
2.7.4

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

2018-03-21 Thread Daniel Vetter

On Tue, Mar 20, 2018 at 06:47:57PM +0100, Christian König wrote:
> Am 20.03.2018 um 15:08 schrieb Daniel Vetter:
> > [SNIP]
> > For the in-driver reservation path (CS) having a slow-path that grabs a
> > temporary reference, drops the vram lock and then locks the reservation
> > normally (using the acquire context used already for the entire CS) is a
> > bit tricky, but totally feasible. Ttm doesn't do that though.
> 
> That is exactly what we do in amdgpu as well, it's just not very efficient
> nor reliable to retry getting the right pages for a submission over and over
> again.

Out of curiosity, where's that code? I did read the ttm eviction code way
back, and that one definitely didn't do that. Would be interesting to
update my understanding.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

2018-03-21 Thread Daniel Vetter

On Tue, Mar 20, 2018 at 06:47:57PM +0100, Christian König wrote:
> Am 20.03.2018 um 15:08 schrieb Daniel Vetter:
> > [SNIP]
> > For the in-driver reservation path (CS) having a slow-path that grabs a
> > temporary reference, drops the vram lock and then locks the reservation
> > normally (using the acquire context used already for the entire CS) is a
> > bit tricky, but totally feasible. Ttm doesn't do that though.
> 
> That is exactly what we do in amdgpu as well, it's just not very efficient
> nor reliable to retry getting the right pages for a submission over and over
> again.
> 
> > [SNIP]
> > Note that there are 2 paths for i915 userptr. One is the mmu notifier, the
> > other one is the root-only hack we have for dubious reasons (or that I
> > really don't see the point in myself).
> 
> Well I'm referring to i915_gem_userptr.c, if that isn't what you are
> exposing then just feel free to ignore this whole discussion.

They're both in i915_gem_userptr.c, somewhat interleaved. Would be
interesting if you could show what you think is going wrong in there
compared to amdgpu_mn.c.

> > > For coherent usage you need to install some lock to prevent concurrent
> > > get_user_pages(), command submission and
> > > invalidate_range_start/invalidate_range_end from the MMU notifier.
> > > 
> > > Otherwise you can't guarantee that you are actually accessing the right 
> > > page
> > > in the case of a fork() or mprotect().
> > Yeah doing that with a full lock will create endless amounts of issues,
> > but I don't see why we need that. Userspace racing stuff with itself gets
> > to keep all the pieces. This is like racing DIRECT_IO against mprotect and
> > fork.
> 
> First of all I strongly disagree on that. A thread calling fork() because it
> wants to run a command is not something we can forbid just because we have a
> gfx stack loaded. That the video driver is not capable of handling that
> correct is certainly not the problem of userspace.
> 
> Second it's not only userspace racing here, you can get into this kind of
> issues just because of transparent huge page support where the background
> daemon tries to reallocate the page tables into bigger chunks.
> 
> And if I'm not completely mistaken you can also open up quite a bunch of
> security problems if you suddenly access the wrong page.

I get a feeling we're talking past each another here. Can you perhaps
explain what exactly the race is you're seeing? The i915 userptr code is
fairly convoluted and pushes a lot of stuff to workers (but then syncs
with those workers again later on), so easily possible you've overlooked
one of these lines that might guarantee already what you think needs to be
guaranteed. We're definitely not aiming to allow userspace to allow
writing to random pages all over.

> > Leaking the IOMMU mappings otoh means rogue userspace could do a bunch of
> > stray writes (I don't see anywhere code in amdgpu_mn.c to unmap at least
> > the gpu side PTEs to make stuff inaccessible) and wreak the core kernel's
> > book-keeping.
> > 
> > In i915 we guarantee that we call set_page_dirty/mark_page_accessed only
> > after all the mappings are really gone (both GPU PTEs and sg mapping),
> > guaranteeing that any stray writes from either the GPU or IOMMU will
> > result in faults (except bugs in the IOMMU, but can't have it all, "IOMMU
> > actually works" is an assumption behind device isolation).
> Well exactly that's the point, the handling in i915 looks incorrect to me.
> You need to call set_page_dirty/mark_page_accessed way before the mapping is
> destroyed.
> 
> To be more precise for userptrs it must be called from the
> invalidate_range_start, but i915 seems to delegate everything into a
> background worker to avoid the locking problems.

Yeah, and at the end of the function there's a flush_work to make sure the
worker has caught up.

The set_page_dirty is also there, but hidden very deep in the call chain
as part of the vma unmapping and backing storage unpinning. But I do think
we guarantee what you expect needs to happen.

> > > Felix and I hammered for quite some time on amdgpu until all of this was
> > > handled correctly, see drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c.
> > Maybe we should have more shared code in this, it seems to be a source of
> > endless amounts of fun ...
> > 
> > > I can try to gather the lockdep splat from my mail history, but it
> > > essentially took us multiple years to get rid of all of them.
> > I'm very much interested in specifically the splat that makes it
> > impossible for you folks to remove the sg mappings. That one sounds bad.
> > And would essentially make mmu_notifiers useless for their primary use
> > case, which is handling virtual machines where you really have to make
> > sure the IOMMU mapping is gone before you claim it's gone, because there's
> > no 2nd level of device checks (like GPU PTEs, or command checker) catching
> > stray writes.
> 
> Well to be more precise the problem is not that we

Re: [PATCH] media: ov5645: add missing of_node_put() in error path

2018-03-21 Thread Todor Tomov

Thank you Akinobu.

Acked-by: Todor Tomov 

On 20.03.2018 00:14, Akinobu Mita wrote:
> The device node obtained with of_graph_get_next_endpoint() should be
> released by calling of_node_put().  But it was not released when
> v4l2_fwnode_endpoint_parse() failed.
> 
> This change moves the of_node_put() call before the error check and
> fixes the issue.
> 
> Cc: Todor Tomov 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
>  drivers/media/i2c/ov5645.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index d28845f..a31fe18 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -1131,13 +1131,14 @@ static int ov5645_probe(struct i2c_client *client,
>  
>   ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
>>ep);
> +
> + of_node_put(endpoint);
> +
>   if (ret < 0) {
>   dev_err(dev, "parsing endpoint node failed\n");
>   return ret;
>   }
>  
> - of_node_put(endpoint);
> -
>   if (ov5645->ep.bus_type != V4L2_MBUS_CSI2) {
>   dev_err(dev, "invalid bus type, must be CSI2\n");
>   return -EINVAL;
>

Re: [PATCH v3 2/2] staging: media: davinci_vpfe: add kfree() on goto err statement

2018-03-21 Thread Dan Carpenter

Looks good.  Thanks!

regards,
dan carpenter

Re: [PATCH v5 0/2] media: video-i2c: add video-i2c driver support

cron job: media_tree daily build: WARNINGS

[trivial PATCH V2] treewide: Align function definition open/close braces

Re: [PATCH] media: uvcvideo: Prevent setting unavailable flags

Re: [PATCH] media: v4l2-common: fix a compilation breakage

[PATCH] media: v4l2-common: fix a compilation breakage

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

[GIT PULL v2 FOR v4.17] cec: add error injection support + other improvements

[PATCH] media: doc: fix ReST link syntax

[PATCH] debugfs-cec-error-inj: document CEC error inj debugfs ABI

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

[PATCHv3] cec-pin-error-inj.rst: document CEC Pin Error Injection

Re: [PATCH 11/12] media: ov5640: Add 60 fps support

Re: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver

RE: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver

Re: [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection

[PATCH] media: uvcvideo: Prevent setting unavailable flags

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

Re: [PATCH v2] media: omap3isp: fix unbalanced dma_iommu_mapping

Re: Double-free in /drivers/media/usb/zr364xx driver

Re: [PATCH 0/5] SPDX license identifiers in all DD drivers

Re: [PATCH 1/5] [media] stv0910/stv6111: add SPDX license headers

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

Re: uvcvideo: Unknown video format,00000032-0002-0010-8000-00aa00389b71

[PATCH] media: ov5645: Use v4l2_find_nearest_size

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

Re: [Linaro-mm-sig] [PATCH 1/5] dma-buf: add optional invalidate_mappings callback v2

Re: [PATCH] media: ov5645: add missing of_node_put() in error path

Re: [PATCH v3 2/2] staging: media: davinci_vpfe: add kfree() on goto err statement

33 matches

Site Navigation

Mail list logo

Footer information