cron job: media_tree daily build: OK

[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 Apr 28 04:00:21 CEST 2016
git branch: test
git hash:   45c175c4ae9695d6d2f30a45ab7f3866cfac184b
gcc version:i686-linux-gcc (GCC) 5.3.0
sparse version: v0.5.0-56-g7647c77
smatch version: v0.5.0-3413-g618cd5c
host hardware:  x86_64
host os:4.5.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-4.5-i686: OK
linux-4.6-rc1-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
linux-4.5-x86_64: OK
linux-4.6-rc1-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 01/13] dt-bindings: Add Renesas R-Car FCP DT bindings

[Rob Herring]

On Tue, Apr 26, 2016 at 12:36:26AM +0300, Laurent Pinchart wrote:
> The FCP is a companion module of video processing modules in the Renesas
> R-Car Gen3 SoCs. It provides data compression and decompression, data
> caching, and conversion of AXI transactions in order to reduce the
> memory bandwidth.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  .../devicetree/bindings/media/renesas,fcp.txt  | 31 
> ++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,fcp.txt
> 
> Cc: devicet...@vger.kernel.org
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,fcp.txt 
> b/Documentation/devicetree/bindings/media/renesas,fcp.txt
> new file mode 100644
> index ..0c72ca24379f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,fcp.txt
> @@ -0,0 +1,31 @@
> +Renesas R-Car Frame Compression Processor (FCP)
> +---
> +
> +The FCP is a companion module of video processing modules in the Renesas 
> R-Car
> +Gen3 SoCs. It provides data compression and decompression, data caching, and
> +conversion of AXI transactions in order to reduce the memory bandwidth.
> +
> +There are three types of FCP whose configuration and behaviour highly depend
> +on the module they are paired with.

3 types?, but I only see one compatible and no relationship with said 
module described.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tw686x: use a formula instead of two tables for div

[Mauro Carvalho Chehab]

Em Wed, 27 Apr 2016 09:00:49 -0300
Mauro Carvalho Chehab  escreveu:

> Hmm
> 
> Em Wed, 27 Apr 2016 08:01:19 -0300
> Mauro Carvalho Chehab  escreveu:
> 
> > Instead of using two tables to estimate the temporal decimation
> > factor, use a formula. This allows to get the closest fps, with
> > sounds better than the current tables.
> > 
> > Compile-tested only.  
> 
> Please discard this patch. It is wrong.
> 
> I found the datasheet for this device at:
>   http://www.starterkit.ru/html/doc/tw6869-ds.pdf
> 
> Based on what it is said on page 50, it seems that it doesn't use a
> decimation filter, but, instead, it just discards some fields in
> a way that the average fps will be reduced.
> 
> So, the actual frame rate is given by the number of enabled bits
> that are written to VIDEO_FIELD_CTRL[vc->ch]

Ok, I think I got this right this time. See the enclosed code.

It produces the fps register map, with each fps associated to
both 60Hz and 50Hz standard, plus it replaces the tables by a
calculus.

If my code is right, there are some values on the current tables
that are wrong.

See below. I'll submit an updated patch soon.

---
Program results:

FPS map:
index  0, map = 0x, 30 fps (60Hz), 25 fps (50Hz), output all 
fields
index  1, map = 0x8006, 2 fps (60Hz), 2 fps (50Hz)
index  2, map = 0x80018006, 4 fps (60Hz), 4 fps (50Hz)
index  3, map = 0x80618006, 6 fps (60Hz), 6 fps (50Hz)
index  4, map = 0x81818186, 8 fps (60Hz), 8 fps (50Hz)
index  5, map = 0x86186186, 10 fps (60Hz), 8 fps (50Hz)
index  6, map = 0x86619866, 12 fps (60Hz), 10 fps (50Hz)
index  7, map = 0x8666, 14 fps (60Hz), 12 fps (50Hz)
index  8, map = 0x999e, 16 fps (60Hz), 14 fps (50Hz)
index  9, map = 0x99e6799e, 18 fps (60Hz), 16 fps (50Hz)
index 10, map = 0x9e79e79e, 20 fps (60Hz), 16 fps (50Hz)
index 11, map = 0x9e7e7e7e, 22 fps (60Hz), 18 fps (50Hz)
index 12, map = 0x9fe7f9fe, 24 fps (60Hz), 20 fps (50Hz)
index 13, map = 0x9ffe7ffe, 26 fps (60Hz), 22 fps (50Hz)
index 14, map = 0x9ffe, 28 fps (60Hz), 24 fps (50Hz)

60 Hz
Requested fps 0, table 0 (30 fps, delta 30), calculus 1 (2 fps, delta 
2) DIFFERENT!
Requested fps 1, table 1 (2 fps, delta 1), calculus 1 (2 fps, delta 1)
Requested fps 2, table 1 (2 fps, delta 0), calculus 1 (2 fps, delta 0)
Requested fps 3, table 1 (2 fps, delta -1), calculus 1 (2 fps, delta -1)
Requested fps 4, table 2 (4 fps, delta 0), calculus 2 (4 fps, delta 0)
Requested fps 5, table 2 (4 fps, delta -1), calculus 2 (4 fps, delta -1)
Requested fps 6, table 3 (6 fps, delta 0), calculus 3 (6 fps, delta 0)
Requested fps 7, table 3 (6 fps, delta -1), calculus 3 (6 fps, delta -1)
Requested fps 8, table 4 (8 fps, delta 0), calculus 4 (8 fps, delta 0)
Requested fps 9, table 4 (8 fps, delta -1), calculus 4 (8 fps, delta -1)
Requested fps 10, table 5 (10 fps, delta 0), calculus 5 (10 fps, delta 
0)
Requested fps 11, table 5 (10 fps, delta -1), calculus 5 (10 fps, delta 
-1)
Requested fps 12, table 6 (12 fps, delta 0), calculus 6 (12 fps, delta 
0)
Requested fps 13, table 6 (12 fps, delta -1), calculus 6 (12 fps, delta 
-1)
Requested fps 14, table 7 (14 fps, delta 0), calculus 7 (14 fps, delta 
0)
Requested fps 15, table 7 (14 fps, delta -1), calculus 7 (14 fps, delta 
-1)
Requested fps 16, table 8 (16 fps, delta 0), calculus 8 (16 fps, delta 
0)
Requested fps 17, table 8 (16 fps, delta -1), calculus 8 (16 fps, delta 
-1)
Requested fps 18, table 9 (18 fps, delta 0), calculus 9 (18 fps, delta 
0)
Requested fps 19, table 9 (18 fps, delta -1), calculus 9 (18 fps, delta 
-1)
Requested fps 20, table 10 (20 fps, delta 0), calculus 10 (20 fps, 
delta 0)
Requested fps 21, table 10 (20 fps, delta -1), calculus 10 (20 fps, 
delta -1)
Requested fps 22, table 11 (22 fps, delta 0), calculus 11 (22 fps, 
delta 0)
Requested fps 23, table 11 (22 fps, delta -1), calculus 11 (22 fps, 
delta -1)
Requested fps 24, table 12 (24 fps, delta 0), calculus 12 (24 fps, 
delta 0)
Requested fps 25, table 12 (24 fps, delta -1), calculus 12 (24 fps, 
delta -1)
Requested fps 26, table 13 (26 fps, delta 0), calculus 13 (26 fps, 
delta 0)
Requested fps 27, table 13 (26 fps, delta -1), calculus 13 (26 fps, 
delta -1)
Requested fps 28, table 14 (28 fps, delta 0), calculus 14 (28 fps, 
delta 0)
Requested fps 29, table 0 (30 fps, delta 1), calculus 14 (28 fps, delta 
-1) DIFFERENT!
Requested fps 30, table 0 (30 fps, delta 0), calculus 0 (30 fps, delta 
0)

50 Hz
Requested fps 0, table 0 (25 fps, delta 25), calculus 1 (2 fps, delta 
2) DIFFERENT!
Requested fps 1, table 1 (2 fps, delta 1), calculus 1 (2 fps, 

Re: [PATCH 4/4] [meida] media-device: dynamically allocate struct media_devnode

[Shuah Khan]

On 03/24/2016 05:37 AM, Mauro Carvalho Chehab wrote:
> Em Thu, 24 Mar 2016 10:24:44 +0200
> Laurent Pinchart  escreveu:
> 
>> On Wednesday 23 Mar 2016 16:27:46 Mauro Carvalho Chehab wrote:
>>> struct media_devnode is currently embedded at struct media_device.
>>>
>>> While this works fine during normal usage, it leads to a race
>>> condition during devnode unregister. the problem is that drivers
>>> assume that, after calling media_device_unregister(), the struct
>>> that contains media_device can be freed. This is not true, as it
>>> can't be freed until userspace closes all opened /dev/media devnodes.
>>>
>>> In other words, if the media devnode is still open, and media_device
>>> gets freed, any call to an ioctl will make the core to try to access
>>> struct media_device, with will cause an use-after-free and even GPF.
>>>
>>> Fix this by dynamically allocating the struct media_devnode and only
>>> freeing it when it is safe.  

Hi Mauro,

I think this is the patch you were referring to in response to the patch
I sent out. Looks like this is still under review and some outstanding
issues. This patch itself doesn't ensure media_devnode sticks around
until the last app. closes the cdev. More work is needed such as adding
cdev parent and providing kobject release function that can be called
from cdev-core which will free media_devnode when the last cdev ref
is gone.

Anyway, since you asked me to do the fix on top of your patch, I am asking
to see if this patch is in a good shape for me to apply. As such, we no
longer have sound/us/media.c in the mix. Hence this patch needs work before
I can base my work on it.

Lars gave a few comments on the patch I sent out in the code that makes
devnode dynamic which are relevant to be folded into your patch. Added
Lars to this thread.

P.S: removed alsa folks and alsa list and added linux-media

thanks,
-- Shuah
>>
>> We have the exact same issue with video_device, and there we've solved the 
>> problem by passing the release call all the way up to the driver. I'm open 
>> to 
>> discuss what the best solution is, but I don't think we should special-case 
>> media_device unless there are compelling arguments regarding why different 
>> solutions are needed for media_device and video_device.
> 
> The relationship between a video driver and  video_device/v4l2_dev is
> different. On V4L2 we have:
>   - One driver using video_device resources;
>   - multiple video_device devnodes.
> 
> For media devices, the relationship is the opposite:
>   - multiple independent drivers using media_devnode.
>   - One media device node;
> 
> On media devices, when multiple drivers are sharing the same devnode, the
> .probe() order can be different than the .release() order.
> 
> So, we don't need to use the same solution as we did for video_device
> on media controller. Actually, the V4L2 solution won't work.
> 
> On V4L2, a video device is typically initialized with:
> 
> video-dev->release = video_device_release;
> err = video_register_device(video_dev,VFL_TYPE_GRABBER,
> video_nr[dev->nr]);
> 
> And video_device_release is simply a kfree:
> 
> void video_device_release(struct video_device *vdev)
> {
> kfree(vdev);
> }
> 
> The caller driver may opt to use its own code to free the resources
> instead of the core one, but it needs to free vdev in the end
> (or some struct that embedds it).
> 
> In the specific case of media, drivers don't need to touch or even
> be aware of media_devnode, as the creation of the media devnode is
> handled internally by the core. Also, there's no good reason to
> make the caller drivers to be aware of that.
> 
> So, the approach taken by this patch is actually simpler, as the
> kfree() is internal to the core, and it doesn't require
> any callbacks. This patch provides all that it is needed to make devnode
> destroy safe. 
> 
> On the common case where one driver allocates one /dev/media devnode,
> using the standard media_device_register()/media_device_unregister(),
> grants that a media_devnode instance will only be freed after all uses
> have gone, including open() descriptors. It also grants that the caller
> can free its own resources after media_device_unregister(), because
> media_devnode won't use media_device anymore.
> 
> This happens because media_devnode_is_registered() will return
> false after media_device_unregister(), and the media_ioctl logic
> will return an error in this case:
> __media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
> long (*ioctl_func)(struct file *filp, unsigned int cmd,
>unsigned long arg))
> {
>   struct media_devnode *devnode = media_devnode_data(filp);
> 
>   if (!ioctl_func)
>   return -ENOTTY;
> 
>   if (!media_devnode_is_registered(devnode))
>   return -EIO;
>   /* IMHO, it should be -ENODEV here 

Re: [PATCH] media: fix media_ioctl use-after-free when driver unbinds

[Shuah Khan]

On 04/27/2016 10:43 AM, Lars-Peter Clausen wrote:
> Looks mostly good, a few comments.
> 
> On 04/27/2016 05:08 AM, Shuah Khan wrote:
> [...]
>> @@ -428,7 +428,7 @@ static long media_device_ioctl(struct file *filp, 
>> unsigned int cmd,
>> unsigned long arg)
>>  {
>>  struct media_devnode *devnode = media_devnode_data(filp);
>> -struct media_device *dev = to_media_device(devnode);
> 
> Can we keep the helper macro, means we don't need to touch this code.

Yeah. I have been thinking about that as well. It avoids changes
and abstracts it.

> 
>> +struct media_device *dev = devnode->media_dev;
> 
> You need a lock to protect this from running concurrently with
> media_device_unregister() otherwise the struct might be freed while still in
> use.
> 

Right. This needs to be protected.

>>  long ret;
>>  
>>  switch (cmd) {
> [...]
>> @@ -725,21 +726,26 @@ int __must_check __media_device_register(struct 
>> media_device *mdev,
>>  {
>>  int ret;
>>  
>> +mdev->devnode = kzalloc(sizeof(struct media_devnode), GFP_KERNEL);
> 
> sizeof(*mdev->devnode) is preferred kernel style,

Yeah. Force of habit, I keep forgetting it.

> 
>> +if (!mdev->devnode)
>> +return -ENOMEM;
>> +
>>  /* Register the device node. */
>> -mdev->devnode.fops = _device_fops;
>> -mdev->devnode.parent = mdev->dev;
>> -mdev->devnode.release = media_device_release;
>> +mdev->devnode->fops = _device_fops;
>> +mdev->devnode->parent = mdev->dev;
>> +mdev->devnode->media_dev = mdev;
>> +mdev->devnode->release = media_device_release;
> 
> This should no longer be necessary. Just drop the release callback altogether.

It does nothing at the moment. I believe the intent is for this routine
to invoke any driver hooks if any at media_device level. It gets called
from media_devnode_release() which is the media_devnode->dev.release.
I will look into if it can be removed.

> 
>>  
>>  /* Set version 0 to indicate user-space that the graph is static */
>>  mdev->topology_version = 0;
>>  
> [...]
>> @@ -813,8 +819,10 @@ void media_device_unregister(struct media_device *mdev)
>>  
>>  spin_unlock(>lock);
>>  
>> -device_remove_file(>devnode.dev, _attr_model);
>> -media_devnode_unregister(>devnode);
>> +device_remove_file(>devnode->dev, _attr_model);
>> +media_devnode_unregister(mdev->devnode);
>> +/* kfree devnode is done via kobject_put() handler */
>> +mdev->devnode = NULL;
> 
> mdev->devnode->media_dev needs to be set to NULL.

Yes. Thanks for catching it.

> 
>>  
>>  dev_dbg(mdev->dev, "Media device unregistered\n");
>>  }
>> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
>> index 29409f4..9af9ba1 100644
>> --- a/drivers/media/media-devnode.c
>> +++ b/drivers/media/media-devnode.c
>> @@ -171,6 +171,9 @@ static int media_open(struct inode *inode, struct file 
>> *filp)
>>  mutex_unlock(_devnode_lock);
>>  return -ENXIO;
>>  }
>> +
>> +kobject_get(>kobj);
> 
> This is not necessary, and if it was it would be prone to race condition as
> the last reference could be dropped before this line. But assigning the cdev
> parent makes sure that we always have a reference to the object while the
> open() callback is running.

I don't see cdev parent kobj get in cdev_get() which does kobject_get()
on cdev->kobj. Is that enough to get the reference?

cdev_add() gets the cdev parent kobj and cdev_del() puts it back. That is
the reason why I added a get here and put in media_release().

I can remove the get and put and test. Looks like I am not checking
kobject_get() return value which isn't good?

> 
>> +
>>  /* and increase the device refcount */
>>  get_device(>dev);
>>  mutex_unlock(_devnode_lock);
>>  /*
> [...]
>> diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
>> index fe42f08..ba4bdaa 100644
>> --- a/include/media/media-devnode.h
>> +++ b/include/media/media-devnode.h
>> @@ -70,7 +70,9 @@ struct media_file_operations {
>>   * @fops:   pointer to struct _file_operations with media device ops
>>   * @dev:struct device pointer for the media controller device
>>   * @cdev:   struct cdev pointer character device
>> + * @kobj:   struct kobject
>>   * @parent: parent device
>> + * @media_dev:  media device
>>   * @minor:  device node minor number
>>   * @flags:  flags, combination of the MEDIA_FLAG_* constants
>>   * @release:release callback called at the end of 
>> media_devnode_release()
>> @@ -87,7 +89,9 @@ struct media_devnode {
>>  /* sysfs */
>>  struct device dev;  /* media device */
>>  struct cdev cdev;   /* character device */
>> +struct kobject kobj;/* set as cdev parent kobj */
> 
> You don't need a extra kobj. Just use the struct dev kobj.

Yeah I can use that as long as I can override the default release
function with media_devnode_free(). 

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> It's part of the v4l-utils git repo:
> 
> https://git.linuxtv.org/v4l-utils.git/
...

> > > Anyway, does anyone know where to get the media-ctl tool?
> > 
> > Looks like it is part of v4l-utils package. At least in git:
> > https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl
> > 
> > > It does not seem to be in debian 7 or debian 8...
> > 
> > I do not see it in debian too, but there is some version in ubuntu:
> > http://packages.ubuntu.com/trusty/media-ctl
> > 
> > So you can compile ubuntu dsc package, should work on debian.
> 
> Finally, it is also in debian, see:
> 
> https://packages.debian.org/search?suite=sid=any=path=contents=media-ctl
> https://packages.debian.org/sid/amd64/v4l-utils/filelist

Thanks for the pointers. It seems that new debian contains media-ctl,
but I'm using older one, so I compiled it from source. Could not find
yavta, either, but that was very easy to pull from git and compile.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> >>Try with:
> >>
> >>media-ctl -r
> >>media-ctl -l '"et8ek8 3-003e":0 -> "video-bus-switch":1 [1]'
> >>media-ctl -l '"video-bus-switch":0 -> "OMAP3 ISP CCP2":0 [1]'
> >>media-ctl -l '"OMAP3 ISP CCP2":1 -> "OMAP3 ISP CCDC":0 [1]'
> >>media-ctl -l '"OMAP3 ISP CCDC":2 -> "OMAP3 ISP preview":0 [1]'
> >>media-ctl -l '"OMAP3 ISP preview":1 -> "OMAP3 ISP resizer":0 [1]'
> >>media-ctl -l '"OMAP3 ISP resizer":1 -> "OMAP3 ISP resizer output":0 [1]'
> >>
> >>media-ctl -V '"et8ek8 3-003e":0 [SGRBG10 864x656]'
> >>media-ctl -V '"OMAP3 ISP CCP2":0 [SGRBG10 864x656]'
> >>media-ctl -V '"OMAP3 ISP CCP2":1 [SGRBG10 864x656]'
> >>media-ctl -V '"OMAP3 ISP CCDC":2 [SGRBG10 864x656]'
> >>media-ctl -V '"OMAP3 ISP preview":1 [UYVY 864x656]'
> >>media-ctl -V '"OMAP3 ISP resizer":1 [UYVY 800x600]'
> >>
> >>
> >>mplayer -tv driver=v4l2:width=800:height=600:outfmt=uyvy:device=/dev/video6
> >>-vo xv -vf screenshot tv://
> >
> >It fails with:
> >
> >pavel@n900:/my/tui/ofone/camera$ sudo ./back.sh
> >Unable to parse link: Device or resource busy (16)
> 
> That shouldn't happen, there is something else wrong.
> 
> >MPlayer svn r34540 (Debian), built with gcc-4.6 (C) 2000-2012 MPlayer
> >Team
> >
> >...but as I'm using the original dts, it is expected...?
> >
> >Would you have dts suitable for the 5MPx camera?
> 
> Just change from strobe = <0>; to strobe = <1>; in isp node.

Thanks, that got the trick. (With  drivers being compiled as modules).

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> On Wed, Apr 27, 2016 at 09:57:51AM +0300, Ivaylo Dimitrov wrote:
> > >>https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
> > >
> > >Ok, going to diff with my tree to see what I have missed to send in the
> > >patchset
> > 
> > Now, that's getting weird.
> 
> [...]
> 
> > I you want to try it, zImage and initrd are on
> > http://46.249.74.23/linux/camera-n900/
> 
> The zImage + initrd works with the steps you described below. I
> received a completly black image, but at least there are interrupts
> and yavta is happy (=> it does not hang).

Ok, thanks for all the help. I switched from =y to =m, and it started
to work.

sudo insmod videobuf2-core.ko
sudo insmod videobuf2-v4l2.ko
sudo insmod videobuf2-memops.ko
sudo insmod video-bus-switch.ko
sudo insmod smiaregs.ko
sudo insmod smiapp-pll.ko
sudo insmod smiapp.ko
sudo insmod /my/modules/videobuf2-dma-contig.ko
sudo insmod /my/modules/omap3-isp.ko

So far I tested front camera only, and used a rather bright light to
get something... but that's a start :-).

Ok, and these seem to get some image that is dark, but not completely
dark:

YA=/my/tui/yavta/yavta
sudo $YA --set-control '0x009e0903 240'  /dev/v4l-subdev8
sudo $YA --set-control '0x00980911 485'  /dev/v4l-subdev8

Thanks!
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] af9035: correct eeprom offsets

[Antti Palosaari]

Used memory mapped eeprom offsets were off-by 8 bytes.

Signed-off-by: Antti Palosaari 
---
 drivers/media/usb/dvb-usb-v2/af9035.h | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.h 
b/drivers/media/usb/dvb-usb-v2/af9035.h
index df22001..89e629a 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.h
+++ b/drivers/media/usb/dvb-usb-v2/af9035.h
@@ -118,20 +118,20 @@ static const u32 clock_lut_it9135[] = {
  * Values 0, 3 and 5 are seen to this day. 0 for single TS and 3/5 for dual TS.
  */
 
-#define EEPROM_BASE_AF90350x42fd
-#define EEPROM_BASE_IT91350x499c
+#define EEPROM_BASE_AF90350x42f5
+#define EEPROM_BASE_IT91350x4994
 #define EEPROM_SHIFT0x10
 
-#define EEPROM_IR_MODE  0x10
-#define EEPROM_TS_MODE  0x29
-#define EEPROM_2ND_DEMOD_ADDR   0x2a
-#define EEPROM_IR_TYPE  0x2c
-#define EEPROM_1_IF_L   0x30
-#define EEPROM_1_IF_H   0x31
-#define EEPROM_1_TUNER_ID   0x34
-#define EEPROM_2_IF_L   0x40
-#define EEPROM_2_IF_H   0x41
-#define EEPROM_2_TUNER_ID   0x44
+#define EEPROM_IR_MODE  0x18
+#define EEPROM_TS_MODE  0x31
+#define EEPROM_2ND_DEMOD_ADDR   0x32
+#define EEPROM_IR_TYPE  0x34
+#define EEPROM_1_IF_L   0x38
+#define EEPROM_1_IF_H   0x39
+#define EEPROM_1_TUNER_ID   0x3c
+#define EEPROM_2_IF_L   0x48
+#define EEPROM_2_IF_H   0x49
+#define EEPROM_2_TUNER_ID   0x4c
 
 /* USB commands */
 #define CMD_MEM_RD  0x00
-- 
http://palosaari.fi/

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mceusb xhci issue?

[Sean Young]

On Mon, Apr 25, 2016 at 09:16:51PM -0600, Wade Berrier wrote:
> On Apr 25 18:15, Sean Young wrote:
> > On Sun, Apr 24, 2016 at 10:06:33PM -0600, Wade Berrier wrote:
> > > Hello,
> > > 
> > > I have a mceusb compatible transceiver that only seems to work with
> > > certain computers.  I'm testing this on centos7 (3.10.0) and fedora23
> > > (4.4.7).
> > > 
> > > The only difference I can see is that the working computer shows
> > > "using uhci_hcd" and the non working shows "using xhci_hcd".
> > > 
> > > Here's the dmesg output of the non-working version:
> > > 
> > > -
> > > 
> > > [  217.951079] usb 1-5: new full-speed USB device number 10 using xhci_hcd
> > > [  218.104087] usb 1-5: device descriptor read/64, error -71
> > > [  218.371010] usb 1-5: config 1 interface 0 altsetting 0 endpoint 0x1 
> > > has an invalid bInterval 0, changing to 32
> > > [  218.371019] usb 1-5: config 1 interface 0 altsetting 0 endpoint 0x81 
> > > has an invalid bInterval 0, changing to 32
> > 
> > That's odd. Can you post a "lsusb -vvv" of the device please?
> > 
> 
> Sure.
> 
> ---
> 
> Bus 002 Device 009: ID 1784:0006 TopSeed Technology Corp. eHome Infrared 
> Transceiver
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   2.00
>   bDeviceClass0 
>   bDeviceSubClass 0 
>   bDeviceProtocol 0 
>   bMaxPacketSize0 8
>   idVendor   0x1784 TopSeed Technology Corp.
>   idProduct  0x0006 eHome Infrared Transceiver
>   bcdDevice1.02
>   iManufacturer   1 TopSeed Technology Corp.
>   iProduct2 eHome Infrared Transceiver
>   iSerial 3 TS004RrP
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength   32
> bNumInterfaces  1
> bConfigurationValue 1
> iConfiguration  0 
> bmAttributes 0xa0
>   (Bus Powered)
>   Remote Wakeup
> MaxPower  100mA
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber0
>   bAlternateSetting   0
>   bNumEndpoints   2
>   bInterfaceClass   255 Vendor Specific Class
>   bInterfaceSubClass255 Vendor Specific Subclass
>   bInterfaceProtocol255 Vendor Specific Protocol
>   iInterface  0 
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x01  EP 1 OUT
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0020  1x 32 bytes
> bInterval   0

That's wrong indeed. It might be interesting to see if there is anything
in the xhci debug messages with (in Fedora 23):

echo "file xhci*.c +p" > /sys/kernel/debug/dynamic_debug/control
echo "file mceusb.c +p" > /sys/kernel/debug/dynamic_debug/control

And then plug in the receiver, and try to send IR to it with a remote.
You should have quite a few kernel messages in the journal.

>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x81  EP 1 IN
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0020  1x 32 bytes
> bInterval   0
> Device Status: 0x0001
>   Self Powered
> 
> ---
> 
> Also, here's a link to a response on the lirc list:
> 
> https://sourceforge.net/p/lirc/mailman/message/35039126/

That seems suggest that mode2 works but lirc does not. It would be nice
if that could be narrowed down a bit.


Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dvb-s2 card "TeVii S472" (cx23885)

[Hendrik Oenings]

Hi together,

I tried to recompile with the newest media_tree, so I just copied your
modifications to the new media_tree. (I also refetched the media_build.)
It wasn't very helpful, but I've seen that there is a kernel
error message (which was there before too, but I hadn't seen it because
I worked on X).

$ su -c "modprobe cx23885"
modprobe: ERROR: could not insert 'cx23885': Exec format error
$ dmesg
...
[XX.XX] frame_vector: exports duplicate symbol
frame_vector_create (owned by kernel)

I've looked up the reason for the re-export and I have seen that
frame_vector was built as an module, but is compiled into the kernel.

Changing this in the ./media/v4l/.config from m to y worked, so I can
now load the module.

The kernel message is gone away, but there is another problem now,
dmesg says and w_scan doesn't work either:

[XXX] m88ds3103 7-0068: found a 'Montage Technology M88DS3103' in cold
state

[XXX] m88ds3103 7-0068: downloading firmware from file
'dvb-demod-m88ds3103.fw'

[XXX] m88ds3103 7-0068: firmware did not run

I've copied the firmware as suggested to /lib/firmware.

Regards,
Hendrik

Am Tue, 26 Apr 2016 09:03:24 +0300
schrieb Olli Salonen :

> Hi Hendrik, Hans,
> 
> My media_tree Hendrik is pulling from Github is just a rather recent
> copy of the media_tree master branch on git.linuxtv.org (it's missing
> like the last 10 patches that have been added within the last 24
> hours) with 2 simple patches on top that should not break anything.
> 
> Also, the media_build is recent, since you just cloned it.
> 
> Hans, do you know if there are any known issues with the current
> media_build/media_tree on certain kernels?
> 
> Thanks.
> 
> Cheers,
> -olli
> 
> On 25 April 2016 at 19:14, Hendrik Oenings  wrote:
> > Hi Olli,
> >
> > I've tested the driver, it compiles well and I've installed it on my
> > system.
> > But there's a problem: Everytime I try to load the driver (exact:
> > the module cx23885), modprobe (or insmod) is giving me the
> > following: # modprobe cx23885
> > modprobe: ERROR: could not insert 'cx23885': Exec format error
> >
> > I've also tried to compile it with the current 4.6er kernel, but it
> > stays the same.
> > $ uname -r
> > 4.6.0-rc5
> >
> > I've also tried to recompile the driver, but it didn't help.
> >
> > Maybe it is important to mention that some patches fail at the
> > beginning of the build process (pr_fmt, debug).
> >
> > The installed module seems to be correct file format (my arch is
> > x86_64):
> > $ file \
> >  /lib/modules/4.6.0-rc5/kernel/drivers/media/pci/cx23885/cx23885.ko
> >
> > /lib/modules/4.6.0-rc5/kernel/drivers/media/pci/cx23885/cx23885.ko:
> > ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV),
> > BuildID[sha1]=07a3f2f2fe383ab691b0022568dcd4d8315dc4b8, not stripped
> >
> > Regards,
> > Hendrik
> >
> > On Mo, 25 Apr 2016 11:56:37 +0300, Olli Salonen
> >  wrote:
> >  
> >> Hello Hendrik,
> >>
> >> I've created an initial version of the driver.
> >> https://github.com/trsqr/media_tree/commit/b59f25b18bbe84e009618eefeaf646f5939bdd53
> >>
> >> To build, do the following:
> >>
> >> git clone git://linuxtv.org/media_build.git
> >> git clone --depth=1 https://github.com/trsqr/media_tree.git -b
> >> s472 ./media cd media_build
> >> make dir DIR=../media
> >> make distclean
> >> make
> >>
> >> To install after a successful build:
> >>
> >> sudo make install
> >>
> >> Download also the following firmware and place it in /lib/firmware:
> >> http://palosaari.fi/linux/v4l-dvb/firmware/M88DS3103/3.B/
> >>
> >> The thing is, I had to guess the following parameters in
> >> drivers/media/pci/cx23885/cx23885-dvb.c file:
> >>
> >> +static const struct m88ds3103_config tevii_s472_m88ds3103_config
> >> = {
> >> +   .i2c_addr = 0x68,
> >> +   .clock = 2700,
> >> +   .i2c_wr_max = 33,
> >> +   .clock_out = 0,
> >> +   .ts_mode = M88DS3103_TS_PARALLEL,
> >> +   .ts_clk = 16000,
> >> +   .ts_clk_pol = 0,
> >> +   .lnb_en_pol = 0,
> >> +   .lnb_hv_pol = 1,
> >> +   .agc = 0x99,
> >> +};
> >>
> >> If the driver does not work (it loads and appears to tune, but does
> >> not find channels), try altering ts_clk_pol, lnb_en_pol and
> >> lnb_hv_pol. The possible values are 1 and 0 so there should not be
> >> that many iterations needed.. Current values are based on best
> >> guess.
> >>
> >> Cheers,
> >> -olli
> >>
> >>
> >>
> >> On 24 April 2016 at 19:00, Hendrik Oenings 
> >> wrote:  
> >> > Hi Olli,
> >> >
> >> > I'm glad that there is someone trying to help me and of course
> >> > I'm able to test a driver.
> >> >
> >> > Because of the attached photos, I think it is better not to send
> >> > this mail to the mailing list.
> >> >
> >> > cx23885, m88ds3103 and m88ts2022 are the values I also think
> >> > they're correct. cx23885 is also mentioned by lspci, the other
> >> > values I've also seen often while searching for a solution.
> >> >
> >> 

Re: [RFC PATCH v2 1/2] [media] tvp5150: Add input connectors DT bindings

[Javier Martinez Canillas]

Hello Laurent,

Thanks a lot for your feedback.

On 04/27/2016 10:29 AM, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thank you for the patch.
> 
> On Tuesday 12 Apr 2016 18:42:52 Javier Martinez Canillas wrote:
>> The tvp5150 and tvp5151 decoders support different video input source
>> connections to their AIP1A and AIP1B pins. Either two Composite or a
>> S-Video input signals are supported.
>>
>> The possible configurations are as follows:
>>
>> - Analog Composite signal connected to AIP1A.
>> - Analog Composite signal connected to AIP1B.
>> - Analog S-Video Y (luminance) and C (chrominance)
>>   signals connected to AIP1A and AIP1B respectively.
>>
>> This patch extends the Device Tree binding documentation to describe
>> how the input connectors for these devices should be defined in a DT.
>>
>> Suggested-by: Laurent Pinchart 
>> Signed-off-by: Javier Martinez Canillas 
>>
>> ---
>> Hello,
>>
>> The DT binding assumes that there is a 1:1 map between physical connectors
>> and connections, so there will be a connector described in the DT for each
>> connection.
>>
>> There is also the question about how the DT bindings will be extended to
>> support other attributes (color/position/group) using the properties API.
> 
> I foresee lots of bikeshedding on that particular topic, but I don't think it 
> will be a blocker. We need a volunteer to quickstart a discussion on the 
> devicetree (or possible devicetree-spec) mailing list :-)
>

Yes, I plan to extend this binding once we have the properties API in mainline
but that can be done as a follow-up since it should just be more properties on
top of compatible, label and port that will be supported in the meantime.
 
>> But I believe that can be done as a follow-up, once the properties API is
>> in mainline.
>>
>> Best regards,
>> Javier
>>
>> Changes in v2:
>> - Remove from the changelog a mention of devices that multiplex the
>>   physical RCA connectors to be used for the S-Video Y and C signals
>>   since it's a special case and it doesn't really work on the IGEPv2.
>>
>>  .../devicetree/bindings/media/i2c/tvp5150.txt  | 59 +++
>>  1 file changed, 59 insertions(+)
>> :
>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
>> b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt index
>> 8c0fc1a26bf0..df555650b0b4 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
>> @@ -26,8 +26,46 @@ Required Endpoint Properties for parallel
>> synchronization: If none of hsync-active, vsync-active and
>> field-even-active is specified, the endpoint is assumed to use embedded
>> BT.656 synchronization.
>>
>> +-Optional nodes:
>> +- connectors: The list of tvp5150 input connectors available on a given
>> +  board. The node should contain a child 'port' node for each connector.
> 
> I had understood this as meaning that connectors should be fully described in 
> the connectors subnode, until I read through the whole patch and saw that 
> dedicated DT nodes are needed for the connectors. I thus believe the 
> paragraph 
> should be reworded to avoid the ambiguity.
>

I see what you mean, OK I'll make it clear that this only is the list of ports
and that connectors should be described somewhere else (i.e: the root node).

> This being said, why do you need a connectors subnode ? Can't we just add the 
> port nodes for the input ports directly in the tvp5150 node (or possibly in a 
> ports subnode, as defined in the OF graph bindings).
>

Yes we could, I went with a "connectors" subnode because the video decoders
will have another port node to point to the bridge device node endpoint. So
I thought it could be more clear to make a distinction between those ports.

We can go with the "ports" subnode instead of "connectors" but then again it
could be confusing to differentiate between bridge and connectors ports both
for users writing/reading DTS and the drivers parsing the DT.

I used as an inspiration the regulators binding where regulators are usually
described under a "regulators" subnode.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> > On Wed, Apr 27, 2016 at 09:57:51AM +0300, Ivaylo Dimitrov wrote:
> > > > > https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
> > > > 
> > > > Ok, going to diff with my tree to see what I have missed to send in
> > > > the patchset
> > > 
> > > Now, that's getting weird.
> > 
> > [...]
> > The zImage + initrd works with the steps you described below. I
> 
> Great!
> 
> > received a completly black image, but at least there are interrupts
> > and yavta is happy (=> it does not hang).
> >
> 
> The black image is because by default exposure and gain are set to 0 :). 
> Use yavta to set the appropriate controls. You can
> also enable test patterns from there.

Uff.. .there's a lot of controls to play with ;-).

So I did this...

while true; do ./yavta --capture=1 --skip 0 --format UYVY --size
656x488 /dev/video6 --file=/tmp/delme# && cat /tmp/delme01 >
/dev/fb0; done

...and kernel certainly did not like that. After a while:

[ 3468.118774] ---[ end trace 70aa4a6442fc6916 ]---
[ 3468.137084] Address Hole seen by CAM  at address 0
[ 3468.137084] [ cut here ]
[ 3468.137084] WARNING: CPU: 0 PID: 4974 at
drivers/bus/omap_l3_smx.c:166 omap3_l3_app_irq+0xdc/0x124
[ 3468.137207] Modules linked in: smiapp smiapp_pll ipv6
isp1704_charger omap3_isp videobuf2_v4l2 videobuf2_dma_contig
videobuf2_memops videobuf2_core et8ek8 smiaregs lis3lv02d_i2c
lis3lv02d input_polldev ti_soc_thermal arc4 wl1251_spi wl1251 crc7
mac80211 cfg80211 omap_ssi hsi bq2415x_charger si4713
bq27xxx_battery_i2c bq27xxx_battery leds_lp5523 leds_lp55xx_common
adp1653 v4l2_common tsl2563 smc91x mii rtc_twl twl4030_vibra
ff_memless twl4030_wdt tsc2005 tsc200x_core omap_sham omap_wdt
gpio_keys rx51_battery video_bus_switch videodev media
[ 3468.137207] CPU: 0 PID: 4974 Comm: init Tainted: GW
4.6.0-rc4+ #1
[ 3468.137207] Hardware name: Nokia RX-51 board
[ 3468.137237] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 3468.137237] [] (show_stack) from []
(__warn+0xcc/0xf8)
[ 3468.137268] [] (__warn) from []
(warn_slowpath_null+0x1c/0x20)
[ 3468.137298] [] (warn_slowpath_null) from []
(omap3_l3_app_irq+0xdc/0x124)
[ 3468.137298] [] (omap3_l3_app_irq) from []
(handle_irq_event_percpu+0x34/0x138)
[ 3468.137329] [] (handle_irq_event_percpu) from
[] (handle_irq_event+0x5c/0x88)
[ 3468.137329] [] (handle_irq_event) from []
(handle_level_irq+0xcc/0x130)
[ 3468.137359] [] (handle_level_irq) from []
(generic_handle_irq+0x18/0x28)
[ 3468.137359] [] (generic_handle_irq) from []
(__handle_domain_irq+0x84/0xa8)
[ 3468.137390] [] (__handle_domain_irq) from []
(__irq_svc+0x54/0x90)
[ 3468.137390] [] (__irq_svc) from []
(__do_softirq+0x5c/0x208)
[ 3468.137420] [] (__do_softirq) from []
(irq_exit+0x80/0xe4)
[ 3468.137451] [] (irq_exit) from []
(__handle_domain_irq+0x88/0xa8)
[ 3468.137451] [] (__handle_domain_irq) from []
(__irq_svc+0x54/0x90)
[ 3468.137481] [] (__irq_svc) from []
(console_unlock+0x3f4/0x4fc)
[ 3468.137481] [] (console_unlock) from []
(do_con_write.part.10+0x1d80/0x1dac)
[ 3468.137512] [] (do_con_write.part.10) from []
(con_write+0x30/0x48)
[ 3468.137512] [] (con_write) from []
(do_output_char+0x9c/0x1e4)
[ 3468.137542] [] (do_output_char) from []
(n_tty_write+0x2ac/0x430)
[ 3468.137573] [] (n_tty_write) from []
(tty_write+0x1b0/0x240)
[ 3468.137573] [] (tty_write) from []
(__vfs_write+0x2c/0xd4)
[ 3468.137603] [] (__vfs_write) from []
(vfs_write+0xa0/0x18c)
[ 3468.137603] [] (vfs_write) from []
(SyS_write+0x3c/0x78)
[ 3468.137603] [] (SyS_write) from []
(ret_fast_syscall+0x0/0x3c)
[ 3468.137634] ---[ end trace 70aa4a6442fc6917 ]---
[ 3468.144317] omap3isp 480bc000.isp: CCP2 err:8010817
[ 3468.144317] Address Hole seen by CAM  at address b00
[ 3468.144317] [ cut here ]
[ 3468.144348] WARNING: CPU: 0 PID: 4973 at
drivers/bus/omap_l3_smx.c:166 omap3_l3_app_irq+0xdc/0x124
[ 3468.144439] Modules linked in: smiapp smiapp_pll ipv6
isp1704_charger omap3_isp videobuf2_v4l2 videobuf2_dma_contig
videobuf2_memops videobuf2_core et8ek8 smiaregs lis3lv02d_i2c
lis3lv02d input_polldev ti_soc_thermal arc4 wl1251_spi wl1251 crc7
mac80211 cfg80211 omap_ssi hsi bq2415x_charger si4713
bq27xxx_battery_i2c bq27xxx_battery leds_lp5523 leds_lp55xx_common
adp1653 v4l2_common tsl2563 smc91x mii rtc_twl twl4030_vibra
ff_memless twl4030_wdt tsc2005 tsc200x_core omap_sham omap_wdt
gpio_keys rx51_battery video_bus_switch videodev media
[ 3468.144439] CPU: 0 PID: 4973 Comm: yavta Tainted: GW
4.6.0-rc4+ #1
[ 3468.144470] Hardware name: Nokia RX-51 board
[ 3468.144470] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 3468.144500] [] (show_stack) from []
(__warn+0xcc/0xf8)
[ 3468.144500] [] (__warn) from []
(warn_slowpath_null+0x1c/0x20)
[ 3468.144531] [] (warn_slowpath_null) from []
(omap3_l3_app_irq+0xdc/0x124)
[ 3468.144561] [] (omap3_l3_app_irq) from []
(handle_irq_event_percpu+0x34/0x138)
[ 3468.144561] [] (handle_irq_event_percpu) from
[] 

[GIT PULL] two patches: pipeline validation error code

[Helen Koike]

Hi Mauro,

Please pull the following patches correcting the returned error codes 
and respective docs in the pipeline validation.


Regards,
Helen

The following changes since commit 45c175c4ae9695d6d2f30a45ab7f3866cfac184b:

  [media] tw686x: avoid going past array (2016-04-26 06:38:53 -0300)

are available in the git repository at:

  https://github.com/helen-fornazier/opw-staging.git media/devel

for you to fetch changes up to 957f69645eae5faae6daa205e85471ef82752abc:

  [media] DocBook: update error code in videoc-streamon (2016-04-27 
14:14:11 -0300)



Helen Mae Koike Fornazier (2):
  [media] media: change pipeline validation return error
  [media] DocBook: update error code in videoc-streamon

 Documentation/DocBook/media/v4l/vidioc-streamon.xml | 8 
 drivers/media/media-entity.c| 2 +-
 drivers/media/v4l2-core/v4l2-subdev.c   | 4 ++--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-streamon.xml 
b/Documentation/DocBook/media/v4l/vidioc-streamon.xml

index df2c63d..89fd7ce 100644
--- a/Documentation/DocBook/media/v4l/vidioc-streamon.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-streamon.xml
@@ -123,6 +123,14 @@ synchronize with other events.
   
 
   
+  
+  ENOLINK
+
+  The driver implements Media Controller interface and
+  the pipeline link configuration is invalid.
+  
+
+  
 
   
 
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index c53c1d5..d8a2299 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -445,7 +445,7 @@ __must_check int 
__media_entity_pipeline_start(struct media_entity *entity,

 bitmap_or(active, active, has_no_links, entity->num_pads);

 if (!bitmap_full(active, entity->num_pads)) {
-ret = -EPIPE;
+ret = -ENOLINK;
 dev_dbg(entity->graph_obj.mdev->dev,
 "\"%s\":%u must be connected by an enabled link\n",
 entity->name,
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c

index 224ea60..953eab0 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -510,7 +510,7 @@ int v4l2_subdev_link_validate_default(struct 
v4l2_subdev *sd,

 if (source_fmt->format.width != sink_fmt->format.width
 || source_fmt->format.height != sink_fmt->format.height
 || source_fmt->format.code != sink_fmt->format.code)
-return -EINVAL;
+return -EPIPE;

 /* The field order must match, or the sink field order must be NONE
  * to support interlaced hardware connected to bridges that support
@@ -518,7 +518,7 @@ int v4l2_subdev_link_validate_default(struct 
v4l2_subdev *sd,

  */
 if (source_fmt->format.field != sink_fmt->format.field &&
 sink_fmt->format.field != V4L2_FIELD_NONE)
-return -EINVAL;
+return -EPIPE;

 return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Ивайло Димитров]

Hi,

On Wed Apr 27 19:42:56 2016 Sebastian Reichel  wrote:
> Hi,
> 
> On Wed, Apr 27, 2016 at 09:57:51AM +0300, Ivaylo Dimitrov wrote:
> > > > https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
> > > 
> > > Ok, going to diff with my tree to see what I have missed to send in
> > > the patchset
> > 
> > Now, that's getting weird.
> 
> [...]
> The zImage + initrd works with the steps you described below. I

Great!

> received a completly black image, but at least there are interrupts
> and yavta is happy (=> it does not hang).
>

The black image is because by default exposure and gain are set to 0 :). 
Use yavta to set the appropriate controls. You can
also enable test patterns from there.

> 
> Can you try if your config still works if you configure
> CONFIG_VIDEO_OMAP3=y, but leaving the sensors configured
> as modules? I will try the reverse process (using my config
> and moving config options to =m).
>

Will try to find time later today.
 
> > ~$ modprobe smiapp
> 
> modprobing smiapp resulted in a kernel message about a missing
> symbol btw. I currently don't remember which one and it's no
> longer in dmesg due to ISP debug messages.
>

Never seen such missing symbols.

> > Please, Sebastian and Pavel, make sure you're not using some
> > development devices, old board versions need VAUX3 enabled as well,
> > and this is not supported in the $subject patchset. I guess you may
> > try to make VAUX3 always-on in board DTS if that's the case, but I've
> > never tested that, my device is a production one.
> 
> I don't have pre-production N900s. The phone I use for development
> is HW revision 2101 with Finish keyboard layout. Apart from that
> I have my productive phone, which is rev 2204 with German layout.
> 

The one I use for testing is 2204, but I guess it is
irrelevant now you have it finally working.

Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Sebastian Reichel]

Hi,

On Wed, Apr 27, 2016 at 06:45:29PM +0200, Pavel Machek wrote:
> > I don't have pre-production N900s. The phone I use for development
> > is HW revision 2101 with Finish keyboard layout. Apart from that
> > I have my productive phone, which is rev 2204 with German layout.
> 
> How do you check hw revision?

0x tells you the HW revision (e.g. when executed with -I, but
also during normal kernel loading operation). Apart from that there
is /proc/cpuinfo.

-- Sebastian


signature.asc
Description: PGP signature

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> I don't have pre-production N900s. The phone I use for development
> is HW revision 2101 with Finish keyboard layout. Apart from that
> I have my productive phone, which is rev 2204 with German layout.

How do you check hw revision?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: fix media_ioctl use-after-free when driver unbinds

[Lars-Peter Clausen]

Looks mostly good, a few comments.

On 04/27/2016 05:08 AM, Shuah Khan wrote:
[...]
> @@ -428,7 +428,7 @@ static long media_device_ioctl(struct file *filp, 
> unsigned int cmd,
>  unsigned long arg)
>  {
>   struct media_devnode *devnode = media_devnode_data(filp);
> - struct media_device *dev = to_media_device(devnode);

Can we keep the helper macro, means we don't need to touch this code.

> + struct media_device *dev = devnode->media_dev;

You need a lock to protect this from running concurrently with
media_device_unregister() otherwise the struct might be freed while still in
use.

>   long ret;
>  
>   switch (cmd) {
[...]
> @@ -725,21 +726,26 @@ int __must_check __media_device_register(struct 
> media_device *mdev,
>  {
>   int ret;
>  
> + mdev->devnode = kzalloc(sizeof(struct media_devnode), GFP_KERNEL);

sizeof(*mdev->devnode) is preferred kernel style,

> + if (!mdev->devnode)
> + return -ENOMEM;
> +
>   /* Register the device node. */
> - mdev->devnode.fops = _device_fops;
> - mdev->devnode.parent = mdev->dev;
> - mdev->devnode.release = media_device_release;
> + mdev->devnode->fops = _device_fops;
> + mdev->devnode->parent = mdev->dev;
> + mdev->devnode->media_dev = mdev;
> + mdev->devnode->release = media_device_release;

This should no longer be necessary. Just drop the release callback altogether.

>  
>   /* Set version 0 to indicate user-space that the graph is static */
>   mdev->topology_version = 0;
>  
[...]
> @@ -813,8 +819,10 @@ void media_device_unregister(struct media_device *mdev)
>  
>   spin_unlock(>lock);
>  
> - device_remove_file(>devnode.dev, _attr_model);
> - media_devnode_unregister(>devnode);
> + device_remove_file(>devnode->dev, _attr_model);
> + media_devnode_unregister(mdev->devnode);
> + /* kfree devnode is done via kobject_put() handler */
> + mdev->devnode = NULL;

mdev->devnode->media_dev needs to be set to NULL.

>  
>   dev_dbg(mdev->dev, "Media device unregistered\n");
>  }
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index 29409f4..9af9ba1 100644
> --- a/drivers/media/media-devnode.c
> +++ b/drivers/media/media-devnode.c
> @@ -171,6 +171,9 @@ static int media_open(struct inode *inode, struct file 
> *filp)
>   mutex_unlock(_devnode_lock);
>   return -ENXIO;
>   }
> +
> + kobject_get(>kobj);

This is not necessary, and if it was it would be prone to race condition as
the last reference could be dropped before this line. But assigning the cdev
parent makes sure that we always have a reference to the object while the
open() callback is running.

> +
>   /* and increase the device refcount */
>   get_device(>dev);
>   mutex_unlock(_devnode_lock);
>  /*
[...]
> diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
> index fe42f08..ba4bdaa 100644
> --- a/include/media/media-devnode.h
> +++ b/include/media/media-devnode.h
> @@ -70,7 +70,9 @@ struct media_file_operations {
>   * @fops:pointer to struct _file_operations with media device ops
>   * @dev: struct device pointer for the media controller device
>   * @cdev:struct cdev pointer character device
> + * @kobj:struct kobject
>   * @parent:  parent device
> + * @media_dev:   media device
>   * @minor:   device node minor number
>   * @flags:   flags, combination of the MEDIA_FLAG_* constants
>   * @release: release callback called at the end of media_devnode_release()
> @@ -87,7 +89,9 @@ struct media_devnode {
>   /* sysfs */
>   struct device dev;  /* media device */
>   struct cdev cdev;   /* character device */
> + struct kobject kobj;/* set as cdev parent kobj */

You don't need a extra kobj. Just use the struct dev kobj.

>   struct device *parent;  /* device parent */
> + struct media_device *media_dev; /* media device for the devnode */
>  
>   /* device info */
>   int minor;

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Sebastian Reichel]

Hi,

On Wed, Apr 27, 2016 at 09:57:51AM +0300, Ivaylo Dimitrov wrote:
> >>https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
> >
> >Ok, going to diff with my tree to see what I have missed to send in the
> >patchset
> 
> Now, that's getting weird.

[...]

> I you want to try it, zImage and initrd are on
> http://46.249.74.23/linux/camera-n900/

The zImage + initrd works with the steps you described below. I
received a completly black image, but at least there are interrupts
and yavta is happy (=> it does not hang).

For reference I configured the pipeline using media-ctl and used
this to acquire the image:

./yavta --capture=1 --format UYVY --file="/tmp/frame.uyvy" --size 656x488 
/dev/video6

Then I copied the file to my notebook using the FTP server coming
with your initrd and displayed it using "display" from imagemagick:

display -size 656x488 -colorspace rgb frame.uyvy

Before analysing why there is only a black image let's start
with the no image at all problem, though.

> I cloned n900-camera-ivo, copied rx51_defconfig from my tree, added:
> 
> CONFIG_VIDEO_SMIAREGS=m
> CONFIG_VIDEO_ET8EK8=m
> CONFIG_VIDEO_BUS_SWITCH=m
> 
> to it, make mrproper, built the kernel using rx51_defconfig and made initrd
> for rescueos, so to be sure that maemo5 did not influence cameras somehow.

Ok, so there is probably a problem when some things are not built as
modules.

> I cloned n900-camera-ivo, copied rx51_defconfig from my tree, added:
> 
> CONFIG_VIDEO_SMIAREGS=m
> CONFIG_VIDEO_ET8EK8=m
> CONFIG_VIDEO_BUS_SWITCH=m
> 
> to it, make mrproper, built the kernel using rx51_defconfig and made initrd
> for rescueos, so to be sure that maemo5 did not influence cameras somehow.

I will test your kernel + your modules with my userspace, so
that I know for sure, that my userland behaves correctly.

Can you try if your config still works if you configure
CONFIG_VIDEO_OMAP3=y, but leaving the sensors configured
as modules? I will try the reverse process (using my config
and moving config options to =m).

> ~$ modprobe smiapp

modprobing smiapp resulted in a kernel message about a missing
symbol btw. I currently don't remember which one and it's no
longer in dmesg due to ISP debug messages.

> Please, Sebastian and Pavel, make sure you're not using some development
> devices, old board versions need VAUX3 enabled as well, and this is not
> supported in the $subject patchset. I guess you may try to make VAUX3
> always-on in board DTS if that's the case, but I've never tested that, my
> device is a production one.

I don't have pre-production N900s. The phone I use for development
is HW revision 2101 with Finish keyboard layout. Apart from that
I have my productive phone, which is rev 2204 with German layout.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v3] [media] vimc: Virtual Media Controller core, capture and sensor

[Helen Fornazier]

Hi,

On Wed, Apr 6, 2016 at 5:20 PM, Helen Mae Koike Fornazier
 wrote:
> From: Helen Fornazier 
>
> First version of the Virtual Media Controller.
> Add a simple version of the core of the driver, the capture and
> sensor nodes in the topology, generating a grey image in a hardcoded
> format.
>
> Signed-off-by: Helen Fornazier 
> ---
>
> Changes since v2: update with current media master tree
> - Add struct media_pipeline in vimc_cap_device
> - Use vb2_v4l2_buffer instead of vb2_buffer
> - Typos
> - Remove usage of entity->type and use entity->function instead
> - Remove fmt argument from queue setup
> - Use ktime_get_ns instead of v4l2_get_timestamp
> - Iterate over link's list using list_for_each_entry
> - Use media_device_{init, cleanup}
> - Use entity->use_count to keep track of entities instead of the old
> entity->id
> - Replace media_entity_init by media_entity_pads_init
>
>  drivers/media/platform/Kconfig |   2 +
>  drivers/media/platform/Makefile|   1 +
>  drivers/media/platform/vimc/Kconfig|   6 +
>  drivers/media/platform/vimc/Makefile   |   3 +
>  drivers/media/platform/vimc/vimc-capture.c | 534 ++
>  drivers/media/platform/vimc/vimc-capture.h |  28 ++
>  drivers/media/platform/vimc/vimc-core.c| 595 
> +
>  drivers/media/platform/vimc/vimc-core.h|  55 +++
>  drivers/media/platform/vimc/vimc-sensor.c  | 277 ++
>  drivers/media/platform/vimc/vimc-sensor.h  |  28 ++
>  10 files changed, 1529 insertions(+)
>  create mode 100644 drivers/media/platform/vimc/Kconfig
>  create mode 100644 drivers/media/platform/vimc/Makefile
>  create mode 100644 drivers/media/platform/vimc/vimc-capture.c
>  create mode 100644 drivers/media/platform/vimc/vimc-capture.h
>  create mode 100644 drivers/media/platform/vimc/vimc-core.c
>  create mode 100644 drivers/media/platform/vimc/vimc-core.h
>  create mode 100644 drivers/media/platform/vimc/vimc-sensor.c
>  create mode 100644 drivers/media/platform/vimc/vimc-sensor.h
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 201f5c2..14ed03f 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -284,6 +284,8 @@ menuconfig V4L_TEST_DRIVERS
>
>  if V4L_TEST_DRIVERS
>
> +source "drivers/media/platform/vimc/Kconfig"
> +
>  source "drivers/media/platform/vivid/Kconfig"
>
>  config VIDEO_VIM2M
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index bbb7bd1..e4508fe 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/
>
>  obj-$(CONFIG_VIDEO_VIU) += fsl-viu.o
>
> +obj-$(CONFIG_VIDEO_VIMC)   += vimc/
>  obj-$(CONFIG_VIDEO_VIVID)  += vivid/
>  obj-$(CONFIG_VIDEO_VIM2M)  += vim2m.o
>
> diff --git a/drivers/media/platform/vimc/Kconfig 
> b/drivers/media/platform/vimc/Kconfig
> new file mode 100644
> index 000..81279f4
> --- /dev/null
> +++ b/drivers/media/platform/vimc/Kconfig
> @@ -0,0 +1,6 @@
> +config VIDEO_VIMC
> +   tristate "Virtual Media Controller Driver (VIMC)"
> +   select VIDEO_V4L2_SUBDEV_API
> +   default n
> +   ---help---
> + Skeleton driver for Virtual Media Controller
> diff --git a/drivers/media/platform/vimc/Makefile 
> b/drivers/media/platform/vimc/Makefile
> new file mode 100644
> index 000..c45195e
> --- /dev/null
> +++ b/drivers/media/platform/vimc/Makefile
> @@ -0,0 +1,3 @@
> +vimc-objs := vimc-core.o vimc-capture.o vimc-sensor.o
> +
> +obj-$(CONFIG_VIDEO_VIMC) += vimc.o
> diff --git a/drivers/media/platform/vimc/vimc-capture.c 
> b/drivers/media/platform/vimc/vimc-capture.c
> new file mode 100644
> index 000..3fb8bfe
> --- /dev/null
> +++ b/drivers/media/platform/vimc/vimc-capture.c
> @@ -0,0 +1,534 @@
> +/*
> + * vimc-capture.c Virtual Media Controller Driver
> + *
> + * Copyright (C) 2015 Helen Fornazier 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "vimc-capture.h"
> +
> +struct vimc_cap_device {
> +   struct vimc_ent_device ved;
> +   struct video_device vdev;
> +   struct v4l2_device *v4l2_dev;
> +   struct device *dev;
> +   struct v4l2_pix_format format;
> +   struct vb2_queue queue;
> +   

Re: [RFC PATCH v2 1/2] [media] tvp5150: Add input connectors DT bindings

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Tuesday 12 Apr 2016 18:42:52 Javier Martinez Canillas wrote:
> The tvp5150 and tvp5151 decoders support different video input source
> connections to their AIP1A and AIP1B pins. Either two Composite or a
> S-Video input signals are supported.
> 
> The possible configurations are as follows:
> 
> - Analog Composite signal connected to AIP1A.
> - Analog Composite signal connected to AIP1B.
> - Analog S-Video Y (luminance) and C (chrominance)
>   signals connected to AIP1A and AIP1B respectively.
> 
> This patch extends the Device Tree binding documentation to describe
> how the input connectors for these devices should be defined in a DT.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Javier Martinez Canillas 
> 
> ---
> Hello,
> 
> The DT binding assumes that there is a 1:1 map between physical connectors
> and connections, so there will be a connector described in the DT for each
> connection.
> 
> There is also the question about how the DT bindings will be extended to
> support other attributes (color/position/group) using the properties API.

I foresee lots of bikeshedding on that particular topic, but I don't think it 
will be a blocker. We need a volunteer to quickstart a discussion on the 
devicetree (or possible devicetree-spec) mailing list :-)

> But I believe that can be done as a follow-up, once the properties API is
> in mainline.
> 
> Best regards,
> Javier
> 
> Changes in v2:
> - Remove from the changelog a mention of devices that multiplex the
>   physical RCA connectors to be used for the S-Video Y and C signals
>   since it's a special case and it doesn't really work on the IGEPv2.
> 
>  .../devicetree/bindings/media/i2c/tvp5150.txt  | 59 +++
>  1 file changed, 59 insertions(+)
> :
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt index
> 8c0fc1a26bf0..df555650b0b4 100644
> --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> @@ -26,8 +26,46 @@ Required Endpoint Properties for parallel
> synchronization: If none of hsync-active, vsync-active and
> field-even-active is specified, the endpoint is assumed to use embedded
> BT.656 synchronization.
> 
> +-Optional nodes:
> +- connectors: The list of tvp5150 input connectors available on a given
> +  board. The node should contain a child 'port' node for each connector.

I had understood this as meaning that connectors should be fully described in 
the connectors subnode, until I read through the whole patch and saw that 
dedicated DT nodes are needed for the connectors. I thus believe the paragraph 
should be reworded to avoid the ambiguity.

This being said, why do you need a connectors subnode ? Can't we just add the 
port nodes for the input ports directly in the tvp5150 node (or possibly in a 
ports subnode, as defined in the OF graph bindings).

> +  The tvp5150 has support for three possible connectors: 2 Composite and
> +  1 S-video. The "reg" property is used to specify which input connector
> +  is associated with each 'port', using the following possible values:
> +
> +  0: Composite0
> +  1: Composite1
> +  2: S-Video
> +
> +  The ports should have an endpoint subnode that is linked to a connector
> +  node defined in Documentation/devicetree/bindings/display/connector/.
> +  The linked connector compatible string should match the connector type.
> +
>  Example:
> 
> +composite0: connector@0 {
> + compatible = "composite-video-connector";
> + label = "Composite0";
> +
> + port {
> + comp0_out: endpoint {
> + remote-endpoint = <_comp0_in>;
> + };
> + };
> +};
> +
> +svideo: connector@1 {
> + compatible = "composite-video-connector";
> + label = "S-Video";
> +
> + port {
> + svideo_out: endpoint {
> + remote-endpoint = <_svideo_in>;
> + };
> + };
> +};
> +
>   {
>   ...
>   tvp5150@5c {
> @@ -36,6 +74,27 @@ Example:
>   pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
>   reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> 
> + connectors {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* Composite0 input */
> + port@0 {
> + reg = <0>;
> + tvp5150_comp0_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> +
> + /* S-Video input */
> + port@2 {
> + reg = <2>;
> + tvp5150_svideo_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> +  

Re: [PATCH v2 08/13] v4l: vsp1: Make vsp1_entity_get_pad_compose() more generic

[Laurent Pinchart]

Hi Sergei,

On Tuesday 26 Apr 2016 21:09:10 Sergei Shtylyov wrote:
> On 04/26/2016 12:36 AM, Laurent Pinchart wrote:
> > Turn the helper into a function that can retrieve crop and compose
> > selection rectangles.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >   drivers/media/platform/vsp1/vsp1_entity.c | 24 
> >   drivers/media/platform/vsp1/vsp1_entity.h |  6 +++---
> >   drivers/media/platform/vsp1/vsp1_rpf.c|  7 ---
> >   3 files changed, 27 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_entity.c
> > b/drivers/media/platform/vsp1/vsp1_entity.c index
> > f60d7926d53f..8c49a74381a1 100644
> > --- a/drivers/media/platform/vsp1/vsp1_entity.c
> > +++ b/drivers/media/platform/vsp1/vsp1_entity.c
> > @@ -87,12 +87,28 @@ vsp1_entity_get_pad_format(struct vsp1_entity *entity,
> > 
> > return v4l2_subdev_get_try_format(>subdev, cfg, pad);
> >   
> >   }
> > 
> > +/**
> > + * vsp1_entity_get_pad_selection - Get a pad selection from storage for
> > entity + * @entity: the entity
> > + * @cfg: the configuration storage
> > + * @pad: the pad number
> > + * @target: the selection target
> > + *
> > + * Return the selection rectangle stored in the given configuration for
> > an
> > + * entity's pad. The configuration can be an ACTIVE or TRY configuration.
> > The + * selection target can be COMPOSE or CROP.
> > + */
> > 
> >   struct v4l2_rect *
> > 
> > -vsp1_entity_get_pad_compose(struct vsp1_entity *entity,
> > -   struct v4l2_subdev_pad_config *cfg,
> > -   unsigned int pad)
> > +vsp1_entity_get_pad_selection(struct vsp1_entity *entity,
> > + struct v4l2_subdev_pad_config *cfg,
> > + unsigned int pad, unsigned int target)
> > 
> >   {
> > 
> > -   return v4l2_subdev_get_try_compose(>subdev, cfg, pad);
> > +   if (target == V4L2_SEL_TGT_COMPOSE)
> > +   return v4l2_subdev_get_try_compose(>subdev, cfg, pad);
> > +   else if (target == V4L2_SEL_TGT_CROP)
> > +   return v4l2_subdev_get_try_crop(>subdev, cfg, pad);
> > +   else
> > +   return NULL;
> 
> How about *switch* instead?

That's certainly an option. It don't think it would make a big difference, but 
I'll change it nonetheless. I won't post a new of the series now just for that 
change though, but I'll include it when I'll do.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] tw686x: use a formula instead of two tables for div

[Mauro Carvalho Chehab]

Instead of using two tables to estimate the temporal decimation
factor, use a formula. This allows to get the closest fps, with
sounds better than the current tables.

Compile-tested only.

Signed-off-by: Mauro Carvalho Chehab 

[media] tw686x: cleanup the fps estimation code

There are some issues with the old code:
1) it uses two static tables;
2) some values for 50Hz standards are wrong;
3) it doesn't store the real framerate.

This patch fixes the above issues.

Compile-tested only.

Signed-off-by: Mauro Carvalho Chehab 

-

v3: Patch v2 were actually a diff patch against PATCH v1. Fold the two patches 
in one.

PS.: With this patch, it should be easy to add support for
VIDIOC_G_PARM and VIDIOC_S_PARM, as vc->fps will now store the
real frame rate, with should be used when returning from those
functions.

---
 drivers/media/pci/tw686x/tw686x-video.c | 110 +---
 1 file changed, 73 insertions(+), 37 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c
index 253e10823ba3..b247a7b4ddd8 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -43,53 +43,89 @@ static const struct tw686x_format formats[] = {
}
 };
 
-static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
+static const unsigned int fps_map[15] = {
+   /*
+* bit 31 enables selecting the field control register
+* bits 0-29 are a bitmask with fields that will be output.
+* For NTSC (and PAL-M, PAL-60), all 30 bits are used.
+* For other PAL standards, only the first 25 bits are used.
+*/
+   0x, /* output all fields */
+   0x8006, /* 2 fps (60Hz), 2 fps (50Hz) */
+   0x80018006, /* 4 fps (60Hz), 4 fps (50Hz) */
+   0x80618006, /* 6 fps (60Hz), 6 fps (50Hz) */
+   0x81818186, /* 8 fps (60Hz), 8 fps (50Hz) */
+   0x86186186, /* 10 fps (60Hz), 8 fps (50Hz) */
+   0x86619866, /* 12 fps (60Hz), 10 fps (50Hz) */
+   0x8666, /* 14 fps (60Hz), 12 fps (50Hz) */
+   0x999e, /* 16 fps (60Hz), 14 fps (50Hz) */
+   0x99e6799e, /* 18 fps (60Hz), 16 fps (50Hz) */
+   0x9e79e79e, /* 20 fps (60Hz), 16 fps (50Hz) */
+   0x9e7e7e7e, /* 22 fps (60Hz), 18 fps (50Hz) */
+   0x9fe7f9fe, /* 24 fps (60Hz), 20 fps (50Hz) */
+   0x9ffe7ffe, /* 26 fps (60Hz), 22 fps (50Hz) */
+   0x9ffe, /* 28 fps (60Hz), 24 fps (50Hz) */
+};
+
+static unsigned int tw686x_real_fps(unsigned int index, unsigned int max_fps)
+{
+   unsigned int i, bits, c = 0;
+
+   if (!index || index >= ARRAY_SIZE(fps_map))
+   return max_fps;
+
+   bits = fps_map[index];
+   for (i = 0; i < max_fps; i++)
+   if ((1 << i) & bits)
+   c++;
+
+   return c;
+}
+
+static unsigned int tw686x_fps_idx(unsigned int fps, unsigned int max_fps)
 {
-   static const unsigned int map[15] = {
-   0x, 0x0001, 0x4001, 0x00104001, 0x00404041,
-   0x01041041, 0x01104411, 0x0111, 0x0445, 0x04511445,
-   0x05145145, 0x05151515, 0x05515455, 0x05551555, 0x0555
-   };
-
-   static const unsigned int std_625_50[26] = {
-   0, 1, 1, 2,  3,  3,  4,  4,  5,  5,  6,  7,  7,
-  8, 8, 9, 10, 10, 11, 11, 12, 13, 13, 14, 14, 0
-   };
-
-   static const unsigned int std_525_60[31] = {
-   0, 1, 1, 1, 2,  2,  3,  3,  4,  4,  5,  5,  6,  6, 7, 7,
-  8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 0, 0
-   };
-
-   unsigned int i;
-
-   if (std & V4L2_STD_525_60) {
-   if (fps >= ARRAY_SIZE(std_525_60))
-   fps = 30;
-   i = std_525_60[fps];
-   } else {
-   if (fps >= ARRAY_SIZE(std_625_50))
-   fps = 25;
-   i = std_625_50[fps];
-   }
-
-   return map[i];
+   unsigned int idx, real_fps;
+   int delta;
+
+   /* First guess */
+   idx = (12 + 15 * fps) / max_fps;
+
+   /* Minimal possible framerate is 2 frames per second */
+   if (!idx)
+   return 1;
+
+   /* Check if the difference is bigger than abs(1) and adjust */
+   real_fps = tw686x_real_fps(idx, max_fps);
+   delta = real_fps - fps;
+   if (delta < -1)
+   idx++;
+   else if (delta > 1)
+   idx--;
+
+   /* Max framerate */
+   if (idx >= 15)
+   return 0;
+
+   return idx;
 }
 
 static void tw686x_set_framerate(struct tw686x_video_channel *vc,
 unsigned int fps)
 {
-   unsigned int map;
+   unsigned int i, max_fps;
 
if (vc->fps == fps)
return;
 
-   map = tw686x_fields_map(vc->video_standard, fps) << 1;
-   map |= map << 1;
-   if (map > 0)
- 

[PATCH v2] [media] tw686x: cleanup the fps estimation code

[Mauro Carvalho Chehab]

There are some issues with the old code:
1) it uses two static tables;
2) some values for 50Hz standards are wrong;
3) it doesn't store the real framerate.

This patch fixes the above issues.

Compile-tested only.

Signed-off-by: Mauro Carvalho Chehab 

---

PS.: With this patch, it should be easy to add support for
VIDIOC_G_PARM and VIDIOC_S_PARM, as vc->fps will now store the
real frame rate, with should be used when returning from those
functions.

 drivers/media/pci/tw686x/tw686x-video.c | 100 +++-
 1 file changed, 73 insertions(+), 27 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c
index 0210fa304e4c..b247a7b4ddd8 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -43,43 +43,89 @@ static const struct tw686x_format formats[] = {
}
 };
 
-static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
+static const unsigned int fps_map[15] = {
+   /*
+* bit 31 enables selecting the field control register
+* bits 0-29 are a bitmask with fields that will be output.
+* For NTSC (and PAL-M, PAL-60), all 30 bits are used.
+* For other PAL standards, only the first 25 bits are used.
+*/
+   0x, /* output all fields */
+   0x8006, /* 2 fps (60Hz), 2 fps (50Hz) */
+   0x80018006, /* 4 fps (60Hz), 4 fps (50Hz) */
+   0x80618006, /* 6 fps (60Hz), 6 fps (50Hz) */
+   0x81818186, /* 8 fps (60Hz), 8 fps (50Hz) */
+   0x86186186, /* 10 fps (60Hz), 8 fps (50Hz) */
+   0x86619866, /* 12 fps (60Hz), 10 fps (50Hz) */
+   0x8666, /* 14 fps (60Hz), 12 fps (50Hz) */
+   0x999e, /* 16 fps (60Hz), 14 fps (50Hz) */
+   0x99e6799e, /* 18 fps (60Hz), 16 fps (50Hz) */
+   0x9e79e79e, /* 20 fps (60Hz), 16 fps (50Hz) */
+   0x9e7e7e7e, /* 22 fps (60Hz), 18 fps (50Hz) */
+   0x9fe7f9fe, /* 24 fps (60Hz), 20 fps (50Hz) */
+   0x9ffe7ffe, /* 26 fps (60Hz), 22 fps (50Hz) */
+   0x9ffe, /* 28 fps (60Hz), 24 fps (50Hz) */
+};
+
+static unsigned int tw686x_real_fps(unsigned int index, unsigned int max_fps)
+{
+   unsigned int i, bits, c = 0;
+
+   if (!index || index >= ARRAY_SIZE(fps_map))
+   return max_fps;
+
+   bits = fps_map[index];
+   for (i = 0; i < max_fps; i++)
+   if ((1 << i) & bits)
+   c++;
+
+   return c;
+}
+
+static unsigned int tw686x_fps_idx(unsigned int fps, unsigned int max_fps)
 {
-   static const unsigned int map[15] = {
-   0x, 0x0001, 0x4001, 0x00104001, 0x00404041,
-   0x01041041, 0x01104411, 0x0111, 0x0445, 0x04511445,
-   0x05145145, 0x05151515, 0x05515455, 0x05551555, 0x0555
-   };
-   unsigned int i, max_fps;
-
-   if (std & V4L2_STD_525_60)
-   max_fps = 30;
-   else
-   max_fps = 25;
-
-   i = DIV_ROUND_CLOSEST(15 * fps, max_fps);
-   if (!i)
-   i = 1;  /* Min possible fps */
-   else if (i > 14)
-   i = 0;  /* fps = max_fps */
-
-   return map[i];
+   unsigned int idx, real_fps;
+   int delta;
+
+   /* First guess */
+   idx = (12 + 15 * fps) / max_fps;
+
+   /* Minimal possible framerate is 2 frames per second */
+   if (!idx)
+   return 1;
+
+   /* Check if the difference is bigger than abs(1) and adjust */
+   real_fps = tw686x_real_fps(idx, max_fps);
+   delta = real_fps - fps;
+   if (delta < -1)
+   idx++;
+   else if (delta > 1)
+   idx--;
+
+   /* Max framerate */
+   if (idx >= 15)
+   return 0;
+
+   return idx;
 }
 
 static void tw686x_set_framerate(struct tw686x_video_channel *vc,
 unsigned int fps)
 {
-   unsigned int map;
+   unsigned int i, max_fps;
 
if (vc->fps == fps)
return;
 
-   map = tw686x_fields_map(vc->video_standard, fps) << 1;
-   map |= map << 1;
-   if (map > 0)
-   map |= BIT(31);
-   reg_write(vc->dev, VIDEO_FIELD_CTRL[vc->ch], map);
-   vc->fps = fps;
+   if (vc->video_standard & V4L2_STD_525_60)
+   max_fps = 30;
+   else
+   max_fps = 25;
+
+   i = tw686x_fps_idx(fps, max_fps);
+
+   reg_write(vc->dev, VIDEO_FIELD_CTRL[vc->ch], fps_map[i]);
+   vc->fps = tw686x_real_fps(i, max_fps);
 }
 
 static const struct tw686x_format *format_by_fourcc(unsigned int fourcc)
-- 
2.5.5


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Kernel docs: muddying the waters a bit

[Grant Likely]

On Tue, Apr 12, 2016 at 4:46 PM, Jonathan Corbet  wrote:
> On Fri, 8 Apr 2016 17:12:27 +0200
> Markus Heiser  wrote:
>
>> motivated by this MT, I implemented a toolchain to migrate the kernel’s
>> DocBook XML documentation to reST markup.
>>
>> It converts 99% of the docs well ... to gain an impression how
>> kernel-docs could benefit from, visit my sphkerneldoc project page
>> on github:
>>
>>   http://return42.github.io/sphkerneldoc/
>
> So I've obviously been pretty quiet on this recently.  Apologies...I've
> been dealing with an extended death-in-the-family experience, and there is
> still a fair amount of cleanup to be done.
>
> Looking quickly at this work, it seems similar to the results I got.  But
> there's a lot of code there that came from somewhere?  I'd put together a
> fairly simple conversion using pandoc and a couple of short sed scripts;
> is there a reason for a more complex solution?
>
> Thanks for looking into this, anyway; I hope to be able to focus more on
> it shortly.

Hi Jon,

Thanks for digging into this. FWIW, here is my $0.02. I've been
working on restarting the devicetree specification, and after looking
at both reStructuredText and Asciidoc(tor) I thought I liked the
Asciidoc markup better, so chose that. I then proceeded to spend weeks
trying to get reasonable output from the toolchain. When I got fed up
and gave Sphinx a try, I was up and running with reasonable PDF and
HTML output in a day and a half.

Honestly, in the end I think we could make either tool do what is
needed of it. However, my impression after trying to do a document
that needs to have nice publishable output with both tools is that
Sphinx is easier to work with, simpler to extend, better supported. My
vote is firmly behind Sphinx/reStructuredText.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: fix media_ioctl use-after-free when driver unbinds

[Shuah Khan]

Hi Mauro,

On 04/27/2016 03:55 AM, Mauro Carvalho Chehab wrote:
> Hi Shuah,
> 
> Good work! I have a few notes below.
> 
> Em Tue, 26 Apr 2016 21:08:32 -0600
> Shuah Khan  escreveu:
> 
>> When driver unbind is run while media_ioctl is in progress, media_ioctl()
>> fails with use-after-free. This first use-after-free is followed by more
>> user-after-free errors in media_release(), kobject_put(), and cdev_put()
>> as driver unbind continues. This problem is found on uvcvideo, em28xx, and
>> au0828 drivers and fix has been tested on all three.
>>
>> This fix allocates media devnode and manages its lifetime separate from the
>> struct media_device. Adds kobject to the media_devnode structure and this
>> kobject is set as the cdev parent kobject. This allows cdev_add() to hold
>> a reference to it and release the reference in cdev_del() ensuring that the
>> media_devnode is not deallocated as long as the application has the cdev
>> open.
>>
>> The first error is below:
>>
>> [  472.424302] 
>> ==
>> [  472.424333] BUG: KASAN: use-after-free in media_ioctl+0xf0/0x130 [media] 
>> at addr 880027b72330
>> [  472.424341] Read of size 8 by task media_device_te/1794
>> [  472.424348] 
>> =
>> [  472.424356] BUG kmalloc-4096 (Not tainted): kasan: bad access detected
>> [  472.424361] 
>> -
>> [  472.431973] CPU: 1 PID: 1794 Comm: media_device_te Tainted: GB
>>4.6.0-rc5 #2
>> [  472.431988] Hardware name: Hewlett-Packard HP ProBook 6475b/180F, BIOS 
>> 68TTU Ver. F.04 08/03/2012
>> [  472.431996]  ea9edc00 88009ddffc78 81aecac3 
>> 8801fa403200
>> [  472.432016]  880027b72260 88009ddffca8 815359b2 
>> 8801fa403200
>> [  472.432040]  ea9edc00 880027b72260 a0c9cc60 
>> 88009ddffcd0
>> [  472.432059] Call Trace:
>> [  472.432079]  [] dump_stack+0x67/0x94
>> [  472.432092]  [] print_trailer+0x112/0x1a0
>> [  472.432108]  [] object_err+0x34/0x40
>> [  472.432125]  [] kasan_report_error+0x224/0x530
>> [  472.432148]  [] ? 
>> __media_device_get_topology+0x1850/0x1850 [media]
>> [  472.432167]  [] __asan_report_load8_noabort+0x43/0x50
>> [  472.432190]  [] ? media_ioctl+0x120/0x130 [media]
>> [  472.432209]  [] media_ioctl+0x120/0x130 [media]
>> [  472.432229]  [] do_vfs_ioctl+0x184/0xe80
>> [  472.432243]  [] ? ioctl_preallocate+0x1a0/0x1a0
>> [  472.432256]  [] ? __hrtimer_init+0x170/0x170
>> [  472.432272]  [] ? do_nanosleep+0x161/0x480
>> [  472.432298]  [] ? sigprocmask+0x290/0x290
>> [  472.432323]  [] ? __fget_light+0x139/0x200
>> [  472.432358]  [] SyS_ioctl+0x79/0x90
>> [  472.432381]  [] entry_SYSCALL_64_fastpath+0x18/0xa8
> 
> This patch is almost identical to my patch, as you took the same
> approach as I did:
>   https://patchwork.linuxtv.org/patch/33577/
> 
> So, I compared both to identify the differences. What I noticed is
> that:
> 
> 1) your patch is setting cdev->parent; in my case, I fixed on a
>separate patch. 
> 
> IMHO, this should be on a separate patch, as cdev is a separate bug.
> 
> 2) On my patch, I also fixed the error conditions at 
>__media_device_register(): currently, it has a few issues, and,
>after the patch, if an error occurs, mdev->devnode should be
>set to NULL;
> 
> 3) you added a kref. I would prefer to see this on a separate patch
>too, as this is not related with moving from an embedded struct
>to a dynamically allocated one. One logical change per patch,
>please.
> 
> There's also the point that you're using my patch, but removing
> my credits. In this specific case, as a developer, I don't mind
> much about that, as it is just one patch and didn't take much
> of my time to produce. Yet, maintainers should not allow stripping
> off credits, as this could cause troubles later.

I don't do things like using another developer's work and strip
credits.

I didn't user your patch. I started from scratch to solve the problem.
There is a good reason for that.

For one thing, your patch came out when we were dealing with lots of
problems with having au0828 and snd-us-audio in the mix. I also wanted
to start from a clean slate and not use any code that was done while we
were debugging two driver problems. As such, there a flood of patches
from you at that time, and I didn't know which ones are applicable
for a single driver case, and which ones aren't. So I just went the
route of clean slate.

That said, I am fine with you want to not take my patch and use yours.

> 
> So, IMHO, the best would be to split this patch in 3 patches:
> 
> - my patch (fixing the context changes);

I will leave it up to you to fix the context changes if any.
I can apply you patch as is and then add the cdev and kref
patch.

> - cdev patch;
> - kref patch.
> 
> 

Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Hans Verkuil]

On 04/27/2016 03:14 PM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-04-27 14:58, Hans Verkuil wrote:
>> On 04/27/2016 02:23 PM, Marek Szyprowski wrote:
>>> Hello,
>>>
>>>
>>> On 2016-04-27 14:10, Hans Verkuil wrote:
 On 04/27/2016 02:00 PM, Marek Szyprowski wrote:
> Add a helper function for device drivers to set DMA's max_seg_size.
> Setting it to largest possible value lets DMA-mapping API always create
> contiguous mappings in DMA address space. This is essential for all
> devices, which use dma-contig videobuf2 memory allocator and shared
> buffers.
>
> Signed-off-by: Marek Szyprowski 
> ---
> This patch was posted earlier as a part of
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
> thread, but applying it is really needed to get all Exynos multimedia
> drivers working with IOMMU enabled.
>
> Best regards,
> Marek Szyprowski
> ---
>drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
>include/media/videobuf2-dma-contig.h   |  1 +
>2 files changed, 16 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index 5361197..f611456 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>}
>EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
>+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int 
> size)
> +{
> +if (!dev->dma_parms) {
> +dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
> +  GFP_KERNEL);
 The v3 patch from December uses kzalloc here. Is this perhaps on old 
 version?
>>> Right, my fault. I will do another resend (and fix the typo in the second 
>>> patch).
>>>
> +if (!dev->dma_parms)
> +return -ENOMEM;
> +}
> +if (dma_get_max_seg_size(dev) < size)
> +return dma_set_max_seg_size(dev, size);
> +
> +return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
 Admittedly I haven't looked closely at this, but is this something that you
 want for all dma-contig devices? Or to rephrase this question: what type of
 devices will need this?
>>> This is needed for all devices using vb2-dc, iommu and user-ptr mode, 
>>> however
>>> in the previous discussions (see https://patchwork.linuxtv.org/patch/30870/
>>> ) it has been suggested to make it via common helper instead of forcing it
>>> in vb2-dc.
>> This certainly will need to be carefully documented in 
>> videobuf2-dma-contig.h.
>>
>> What happens if it is called when you don't have an iommu? Does something 
>> break?
> 
> Nope, nothing breaks in such case. When no iommu is available this parameter 
> is
> ignored by dma-mapping layer. Due to some other issues, it cannot be set by
> generic platform init code:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html

OK.

Is there any reason this cannot be done in vb2-dma-contig.c itself? If there is
no iommu, then this does nothing, and if there is one, don't you always need it?

Requiring drivers to set this fairly obscure setting is asking for problems.
This is guaranteed to be forgotten.

As Lars-Peter said, ideally this is done by the DMA provider, but apparently
that is (for now at least) a no-go. But the next best thing is vb2 IMHO.

I gather that this is specific to userptr mode, so placing this code in
vb2_dc_get_userptr() would make sense. You can even check the size here and
do nothing if it is < 64 kB. Note sure if that is useful, though.

Don't put it in vb2_dma_contig_init_ctx though as I am trying to get rid of
that: http://www.mail-archive.com/linux-media@vger.kernel.org/msg96741.html

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Marek Szyprowski]

Hello,


On 2016-04-27 14:58, Hans Verkuil wrote:

On 04/27/2016 02:23 PM, Marek Szyprowski wrote:

Hello,


On 2016-04-27 14:10, Hans Verkuil wrote:

On 04/27/2016 02:00 PM, Marek Szyprowski wrote:

Add a helper function for device drivers to set DMA's max_seg_size.
Setting it to largest possible value lets DMA-mapping API always create
contiguous mappings in DMA address space. This is essential for all
devices, which use dma-contig videobuf2 memory allocator and shared
buffers.

Signed-off-by: Marek Szyprowski 
---
This patch was posted earlier as a part of
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
thread, but applying it is really needed to get all Exynos multimedia
drivers working with IOMMU enabled.

Best regards,
Marek Szyprowski
---
   drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
   include/media/videobuf2-dma-contig.h   |  1 +
   2 files changed, 16 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 5361197..f611456 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
   }
   EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
   +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
+{
+if (!dev->dma_parms) {
+dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
+  GFP_KERNEL);

The v3 patch from December uses kzalloc here. Is this perhaps on old version?

Right, my fault. I will do another resend (and fix the typo in the second 
patch).


+if (!dev->dma_parms)
+return -ENOMEM;
+}
+if (dma_get_max_seg_size(dev) < size)
+return dma_set_max_seg_size(dev, size);
+
+return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);

Admittedly I haven't looked closely at this, but is this something that you
want for all dma-contig devices? Or to rephrase this question: what type of
devices will need this?

This is needed for all devices using vb2-dc, iommu and user-ptr mode, however
in the previous discussions (see https://patchwork.linuxtv.org/patch/30870/
) it has been suggested to make it via common helper instead of forcing it
in vb2-dc.

This certainly will need to be carefully documented in videobuf2-dma-contig.h.

What happens if it is called when you don't have an iommu? Does something break?


Nope, nothing breaks in such case. When no iommu is available this 
parameter is

ignored by dma-mapping layer. Due to some other issues, it cannot be set by
generic platform init code:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html 



Best regards

--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Hans Verkuil]

On 04/27/2016 02:23 PM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-04-27 14:10, Hans Verkuil wrote:
>> On 04/27/2016 02:00 PM, Marek Szyprowski wrote:
>>> Add a helper function for device drivers to set DMA's max_seg_size.
>>> Setting it to largest possible value lets DMA-mapping API always create
>>> contiguous mappings in DMA address space. This is essential for all
>>> devices, which use dma-contig videobuf2 memory allocator and shared
>>> buffers.
>>>
>>> Signed-off-by: Marek Szyprowski 
>>> ---
>>> This patch was posted earlier as a part of
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
>>> thread, but applying it is really needed to get all Exynos multimedia
>>> drivers working with IOMMU enabled.
>>>
>>> Best regards,
>>> Marek Szyprowski
>>> ---
>>>   drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
>>>   include/media/videobuf2-dma-contig.h   |  1 +
>>>   2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
>>> b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> index 5361197..f611456 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> @@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>>>   }
>>>   EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
>>>   +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int 
>>> size)
>>> +{
>>> +if (!dev->dma_parms) {
>>> +dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
>>> +  GFP_KERNEL);
>> The v3 patch from December uses kzalloc here. Is this perhaps on old version?
> 
> Right, my fault. I will do another resend (and fix the typo in the second 
> patch).
> 
>>> +if (!dev->dma_parms)
>>> +return -ENOMEM;
>>> +}
>>> +if (dma_get_max_seg_size(dev) < size)
>>> +return dma_set_max_seg_size(dev, size);
>>> +
>>> +return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
>> Admittedly I haven't looked closely at this, but is this something that you
>> want for all dma-contig devices? Or to rephrase this question: what type of
>> devices will need this?
> 
> This is needed for all devices using vb2-dc, iommu and user-ptr mode, however
> in the previous discussions (see https://patchwork.linuxtv.org/patch/30870/
> ) it has been suggested to make it via common helper instead of forcing it
> in vb2-dc.

This certainly will need to be carefully documented in videobuf2-dma-contig.h.

What happens if it is called when you don't have an iommu? Does something break?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Marek Szyprowski]

Hello,


On 2016-04-27 14:10, Hans Verkuil wrote:

On 04/27/2016 02:00 PM, Marek Szyprowski wrote:

Add a helper function for device drivers to set DMA's max_seg_size.
Setting it to largest possible value lets DMA-mapping API always create
contiguous mappings in DMA address space. This is essential for all
devices, which use dma-contig videobuf2 memory allocator and shared
buffers.

Signed-off-by: Marek Szyprowski 
---
This patch was posted earlier as a part of
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
thread, but applying it is really needed to get all Exynos multimedia
drivers working with IOMMU enabled.

Best regards,
Marek Szyprowski
---
  drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
  include/media/videobuf2-dma-contig.h   |  1 +
  2 files changed, 16 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 5361197..f611456 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
  }
  EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
  
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)

+{
+   if (!dev->dma_parms) {
+   dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
+ GFP_KERNEL);

The v3 patch from December uses kzalloc here. Is this perhaps on old version?


Right, my fault. I will do another resend (and fix the typo in the 
second patch).



+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+   if (dma_get_max_seg_size(dev) < size)
+   return dma_set_max_seg_size(dev, size);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);

Admittedly I haven't looked closely at this, but is this something that you
want for all dma-contig devices? Or to rephrase this question: what type of
devices will need this?


This is needed for all devices using vb2-dc, iommu and user-ptr mode, 
however

in the previous discussions (see https://patchwork.linuxtv.org/patch/30870/
) it has been suggested to make it via common helper instead of forcing it
in vb2-dc.

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Hans Verkuil]

On 04/27/2016 02:00 PM, Marek Szyprowski wrote:
> Add a helper function for device drivers to set DMA's max_seg_size.
> Setting it to largest possible value lets DMA-mapping API always create
> contiguous mappings in DMA address space. This is essential for all
> devices, which use dma-contig videobuf2 memory allocator and shared
> buffers.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> This patch was posted earlier as a part of
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
> thread, but applying it is really needed to get all Exynos multimedia
> drivers working with IOMMU enabled.
> 
> Best regards,
> Marek Szyprowski
> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
>  include/media/videobuf2-dma-contig.h   |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index 5361197..f611456 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>  }
>  EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
>  
> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
> +{
> + if (!dev->dma_parms) {
> + dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
> +   GFP_KERNEL);

The v3 patch from December uses kzalloc here. Is this perhaps on old version?

> + if (!dev->dma_parms)
> + return -ENOMEM;
> + }
> + if (dma_get_max_seg_size(dev) < size)
> + return dma_set_max_seg_size(dev, size);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);

Admittedly I haven't looked closely at this, but is this something that you
want for all dma-contig devices? Or to rephrase this question: what type of
devices will need this?

Regards,

Hans

> +
>  MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
>  MODULE_AUTHOR("Pawel Osciak ");
>  MODULE_LICENSE("GPL");
> diff --git a/include/media/videobuf2-dma-contig.h 
> b/include/media/videobuf2-dma-contig.h
> index 2087c9a..98857b5 100644
> --- a/include/media/videobuf2-dma-contig.h
> +++ b/include/media/videobuf2-dma-contig.h
> @@ -35,6 +35,7 @@ static inline void *vb2_dma_contig_init_ctx(struct device 
> *dev)
>  }
>  
>  void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size);
>  
>  extern const struct vb2_mem_ops vb2_dma_contig_memops;
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs

[Hans Verkuil]

On 04/27/2016 02:00 PM, Marek Szyprowski wrote:
> All multimedia devices found on Exynos SoCs support only contiguous
> buffers, so set DMA max segment size to DMA_BIT_MASK(32) to let memory
> allocator to correctly create contiguous memory mappings.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> This patch was posted earlier as a part of
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
> thread, but applying it is really needed to get all Exynos multimedia
> drivers working with IOMMU enabled.
> 
> Best regards,
> Marek Szyprowski
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.c  | 1 +
>  drivers/media/platform/exynos4-is/fimc-core.c | 1 +
>  drivers/media/platform/exynos4-is/fimc-is.c   | 1 +
>  drivers/media/platform/exynos4-is/fimc-lite.c | 1 +
>  drivers/media/platform/s5p-g2d/g2d.c  | 1 +
>  drivers/media/platform/s5p-jpeg/jpeg-core.c   | 1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc.c  | 2 ++
>  drivers/media/platform/s5p-tv/mixer_video.c   | 1 +
>  8 files changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 9b9e423..4f90be4 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -1140,6 +1140,7 @@ static int gsc_probe(struct platform_device *pdev)
>   goto err_m2m;
>  
>   /* Initialize continious memory allocator */

Typo: continious -> contiguous

Regards,

Hans


> + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
>   gsc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
>   if (IS_ERR(gsc->alloc_ctx)) {
>   ret = PTR_ERR(gsc->alloc_ctx);
> diff --git a/drivers/media/platform/exynos4-is/fimc-core.c 
> b/drivers/media/platform/exynos4-is/fimc-core.c
> index cef2a7f..368e19b 100644
> --- a/drivers/media/platform/exynos4-is/fimc-core.c
> +++ b/drivers/media/platform/exynos4-is/fimc-core.c
> @@ -1019,6 +1019,7 @@ static int fimc_probe(struct platform_device *pdev)
>   }
>  
>   /* Initialize contiguous memory allocator */
> + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
>   fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
>   if (IS_ERR(fimc->alloc_ctx)) {
>   ret = PTR_ERR(fimc->alloc_ctx);
> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c 
> b/drivers/media/platform/exynos4-is/fimc-is.c
> index 979c388..3f50856 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is.c
> @@ -847,6 +847,7 @@ static int fimc_is_probe(struct platform_device *pdev)
>   if (ret < 0)
>   goto err_pm;
>  
> + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
>   is->alloc_ctx = vb2_dma_contig_init_ctx(dev);
>   if (IS_ERR(is->alloc_ctx)) {
>   ret = PTR_ERR(is->alloc_ctx);
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c 
> b/drivers/media/platform/exynos4-is/fimc-lite.c
> index dc1b929..95841c8 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> @@ -1551,6 +1551,7 @@ static int fimc_lite_probe(struct platform_device *pdev)
>   goto err_sd;
>   }
>  
> + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
>   fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
>   if (IS_ERR(fimc->alloc_ctx)) {
>   ret = PTR_ERR(fimc->alloc_ctx);
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c 
> b/drivers/media/platform/s5p-g2d/g2d.c
> index 74bd46c..5048b68 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -681,6 +681,7 @@ static int g2d_probe(struct platform_device *pdev)
>   goto put_clk_gate;
>   }
>  
> + vb2_dma_contig_set_max_seg_size(>dev, DMA_BIT_MASK(32));
>   dev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
>   if (IS_ERR(dev->alloc_ctx)) {
>   ret = PTR_ERR(dev->alloc_ctx);
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index c3b13a6..e535ccf 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -2838,6 +2838,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
>   goto device_register_rollback;
>   }
>  
> + vb2_dma_contig_set_max_seg_size(>dev, DMA_BIT_MASK(32));
>   jpeg->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
>   if (IS_ERR(jpeg->alloc_ctx)) {
>   v4l2_err(>v4l2_dev, "Failed to init memory allocator\n");
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 927ab49..ae0bf26 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1164,11 +1164,13 @@ static int s5p_mfc_probe(struct platform_device *pdev)
> 

[PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs

[Marek Szyprowski]

All multimedia devices found on Exynos SoCs support only contiguous
buffers, so set DMA max segment size to DMA_BIT_MASK(32) to let memory
allocator to correctly create contiguous memory mappings.

Signed-off-by: Marek Szyprowski 
---
This patch was posted earlier as a part of
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
thread, but applying it is really needed to get all Exynos multimedia
drivers working with IOMMU enabled.

Best regards,
Marek Szyprowski
---
 drivers/media/platform/exynos-gsc/gsc-core.c  | 1 +
 drivers/media/platform/exynos4-is/fimc-core.c | 1 +
 drivers/media/platform/exynos4-is/fimc-is.c   | 1 +
 drivers/media/platform/exynos4-is/fimc-lite.c | 1 +
 drivers/media/platform/s5p-g2d/g2d.c  | 1 +
 drivers/media/platform/s5p-jpeg/jpeg-core.c   | 1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c  | 2 ++
 drivers/media/platform/s5p-tv/mixer_video.c   | 1 +
 8 files changed, 9 insertions(+)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 9b9e423..4f90be4 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1140,6 +1140,7 @@ static int gsc_probe(struct platform_device *pdev)
goto err_m2m;
 
/* Initialize continious memory allocator */
+   vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
gsc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(gsc->alloc_ctx)) {
ret = PTR_ERR(gsc->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-core.c 
b/drivers/media/platform/exynos4-is/fimc-core.c
index cef2a7f..368e19b 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.c
+++ b/drivers/media/platform/exynos4-is/fimc-core.c
@@ -1019,6 +1019,7 @@ static int fimc_probe(struct platform_device *pdev)
}
 
/* Initialize contiguous memory allocator */
+   vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(fimc->alloc_ctx)) {
ret = PTR_ERR(fimc->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c 
b/drivers/media/platform/exynos4-is/fimc-is.c
index 979c388..3f50856 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -847,6 +847,7 @@ static int fimc_is_probe(struct platform_device *pdev)
if (ret < 0)
goto err_pm;
 
+   vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
is->alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(is->alloc_ctx)) {
ret = PTR_ERR(is->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c 
b/drivers/media/platform/exynos4-is/fimc-lite.c
index dc1b929..95841c8 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1551,6 +1551,7 @@ static int fimc_lite_probe(struct platform_device *pdev)
goto err_sd;
}
 
+   vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
if (IS_ERR(fimc->alloc_ctx)) {
ret = PTR_ERR(fimc->alloc_ctx);
diff --git a/drivers/media/platform/s5p-g2d/g2d.c 
b/drivers/media/platform/s5p-g2d/g2d.c
index 74bd46c..5048b68 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -681,6 +681,7 @@ static int g2d_probe(struct platform_device *pdev)
goto put_clk_gate;
}
 
+   vb2_dma_contig_set_max_seg_size(>dev, DMA_BIT_MASK(32));
dev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
if (IS_ERR(dev->alloc_ctx)) {
ret = PTR_ERR(dev->alloc_ctx);
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index c3b13a6..e535ccf 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2838,6 +2838,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
goto device_register_rollback;
}
 
+   vb2_dma_contig_set_max_seg_size(>dev, DMA_BIT_MASK(32));
jpeg->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
if (IS_ERR(jpeg->alloc_ctx)) {
v4l2_err(>v4l2_dev, "Failed to init memory allocator\n");
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 927ab49..ae0bf26 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1164,11 +1164,13 @@ static int s5p_mfc_probe(struct platform_device *pdev)
}
}
 
+   vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
dev->alloc_ctx[0] = vb2_dma_contig_init_ctx(dev->mem_dev_l);
if (IS_ERR(dev->alloc_ctx[0])) {
ret = 

[PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Marek Szyprowski]

Add a helper function for device drivers to set DMA's max_seg_size.
Setting it to largest possible value lets DMA-mapping API always create
contiguous mappings in DMA address space. This is essential for all
devices, which use dma-contig videobuf2 memory allocator and shared
buffers.

Signed-off-by: Marek Szyprowski 
---
This patch was posted earlier as a part of
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
thread, but applying it is really needed to get all Exynos multimedia
drivers working with IOMMU enabled.

Best regards,
Marek Szyprowski
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
 include/media/videobuf2-dma-contig.h   |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 5361197..f611456 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
 }
 EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
 
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
+{
+   if (!dev->dma_parms) {
+   dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
+ GFP_KERNEL);
+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+   if (dma_get_max_seg_size(dev) < size)
+   return dma_set_max_seg_size(dev, size);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
+
 MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
 MODULE_AUTHOR("Pawel Osciak ");
 MODULE_LICENSE("GPL");
diff --git a/include/media/videobuf2-dma-contig.h 
b/include/media/videobuf2-dma-contig.h
index 2087c9a..98857b5 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -35,6 +35,7 @@ static inline void *vb2_dma_contig_init_ctx(struct device 
*dev)
 }
 
 void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size);
 
 extern const struct vb2_mem_ops vb2_dma_contig_memops;
 
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tw686x: use a formula instead of two tables for div

[Mauro Carvalho Chehab]

Hmm

Em Wed, 27 Apr 2016 08:01:19 -0300
Mauro Carvalho Chehab  escreveu:

> Instead of using two tables to estimate the temporal decimation
> factor, use a formula. This allows to get the closest fps, with
> sounds better than the current tables.
> 
> Compile-tested only.

Please discard this patch. It is wrong.

I found the datasheet for this device at:
http://www.starterkit.ru/html/doc/tw6869-ds.pdf

Based on what it is said on page 50, it seems that it doesn't use a
decimation filter, but, instead, it just discards some fields in
a way that the average fps will be reduced.

So, the actual frame rate is given by the number of enabled bits
that are written to VIDEO_FIELD_CTRL[vc->ch] and are at the
valid bitmask range, e. g:

index  0, map = 0x, 30 fps (60Hz), 25 fps (50Hz), output all fields
index  1, map = 0x8006, 2 fps (60Hz), 2 fps (50Hz)
index  2, map = 0x80018006, 4 fps (60Hz), 4 fps (50Hz)
index  3, map = 0x80618006, 6 fps (60Hz), 6 fps (50Hz)
index  4, map = 0x81818186, 8 fps (60Hz), 8 fps (50Hz)
index  5, map = 0x86186186, 10 fps (60Hz), 8 fps (50Hz)
index  6, map = 0x86619866, 12 fps (60Hz), 10 fps (50Hz)
index  7, map = 0x8666, 14 fps (60Hz), 12 fps (50Hz)
index  8, map = 0x999e, 16 fps (60Hz), 14 fps (50Hz)
index  9, map = 0x99e6799e, 18 fps (60Hz), 16 fps (50Hz)
index 10, map = 0x9e79e79e, 20 fps (60Hz), 16 fps (50Hz)
index 11, map = 0x9e7e7e7e, 22 fps (60Hz), 18 fps (50Hz)
index 12, map = 0x9fe7f9fe, 24 fps (60Hz), 20 fps (50Hz)
index 13, map = 0x9ffe7ffe, 26 fps (60Hz), 22 fps (50Hz)
index 14, map = 0x9ffe, 28 fps (60Hz), 24 fps (50Hz)

This was done by using the enclosed program.

---

#include 

static const unsigned int map[15] = {
0x, 0x0001, 0x4001, 0x00104001, 0x00404041,
0x01041041, 0x01104411, 0x0111, 0x0445, 0x04511445,
0x05145145, 0x05151515, 0x05515455, 0x05551555, 0x0555
};

unsigned int count_bits(unsigned int bits, unsigned int max)
{
unsigned int i, c= 0;

for (i = 0; i < max; i++)
if ((1 << i) & bits)
c++;

return c;
}

void calc_map(unsigned int i)
{
unsigned int m, fps_30, fps_25;

m = map[i] << 1;
m |= m << 1;

fps_30 = count_bits(m, 30);
fps_25 = count_bits(m, 25);

if (m)
m |= 1 << 31;
else {
fps_30 = 30;
fps_25 = 25;
}

printf("index %2i, map = 0x%08x, %d fps (60Hz), %d fps (50Hz)%s\n",
i, m, fps_30, fps_25,
i == 0 ? ", output all fields" : ""
);
}

int main(void)
{
unsigned int i;

printf ("\nmap:\n");
for (i = 0; i < 15; i++)
calc_map(i);

return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [media] ir-rx51: Fix build after multiarch changes broke it

[Pavel Machek]

Hi!

> 
> Can you guys please test this still works? I've only been able
> to test that it compiles/loads/unloads as my n900 in in a rack.
> 

I guess Pali (To:) is the right person to test this? My n900 is in my
pocket, but I've never used this functionality before (and would have
to set up the userspace support).

Best regards,
Pavel

> 
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index bd4d685..370e16e 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -336,7 +336,7 @@ config IR_TTUSBIR
>  
>  config IR_RX51
>   tristate "Nokia N900 IR transmitter diode"
> - depends on OMAP_DM_TIMER && ARCH_OMAP2PLUS && LIRC && 
> !ARCH_MULTIPLATFORM
> + depends on OMAP_DM_TIMER && PWM_OMAP_DMTIMER && ARCH_OMAP2PLUS && LIRC
>   ---help---
>  Say Y or M here if you want to enable support for the IR
>  transmitter diode built in the Nokia N900 (RX51) device.
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> index 4e1711a..da839c3 100644
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -19,6 +19,7 @@
>   *
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -26,11 +27,9 @@
>  #include 
>  #include 
>  
> -#include 
> -#include 
> -
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define LIRC_RX51_DRIVER_FEATURES (LIRC_CAN_SET_SEND_DUTY_CYCLE |\
> @@ -44,8 +43,9 @@
>  #define TIMER_MAX_VALUE 0x
>  
>  struct lirc_rx51 {
> - struct omap_dm_timer *pwm_timer;
> - struct omap_dm_timer *pulse_timer;
> + pwm_omap_dmtimer *pwm_timer;
> + pwm_omap_dmtimer *pulse_timer;
> + struct pwm_omap_dmtimer_pdata *dmtimer;
>   struct device*dev;
>   struct lirc_rx51_platform_data *pdata;
>   wait_queue_head_t wqueue;
> @@ -63,14 +63,14 @@ struct lirc_rx51 {
>  
>  static void lirc_rx51_on(struct lirc_rx51 *lirc_rx51)
>  {
> - omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
> -   OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE);
> + lirc_rx51->dmtimer->set_pwm(lirc_rx51->pwm_timer, 0, 1,
> + PWM_OMAP_DMTIMER_TRIGGER_OVERFLOW_AND_COMPARE);
>  }
>  
>  static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
>  {
> - omap_dm_timer_set_pwm(lirc_rx51->pwm_timer, 0, 1,
> -   OMAP_TIMER_TRIGGER_NONE);
> + lirc_rx51->dmtimer->set_pwm(lirc_rx51->pwm_timer, 0, 1,
> + PWM_OMAP_DMTIMER_TRIGGER_NONE);
>  }
>  
>  static int init_timing_params(struct lirc_rx51 *lirc_rx51)
> @@ -79,12 +79,12 @@ static int init_timing_params(struct lirc_rx51 *lirc_rx51)
>  
>   load = -(lirc_rx51->fclk_khz * 1000 / lirc_rx51->freq);
>   match = -(lirc_rx51->duty_cycle * -load / 100);
> - omap_dm_timer_set_load(lirc_rx51->pwm_timer, 1, load);
> - omap_dm_timer_set_match(lirc_rx51->pwm_timer, 1, match);
> - omap_dm_timer_write_counter(lirc_rx51->pwm_timer, TIMER_MAX_VALUE - 2);
> - omap_dm_timer_start(lirc_rx51->pwm_timer);
> - omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> - omap_dm_timer_start(lirc_rx51->pulse_timer);
> + lirc_rx51->dmtimer->set_load(lirc_rx51->pwm_timer, 1, load);
> + lirc_rx51->dmtimer->set_match(lirc_rx51->pwm_timer, 1, match);
> + lirc_rx51->dmtimer->write_counter(lirc_rx51->pwm_timer, TIMER_MAX_VALUE 
> - 2);
> + lirc_rx51->dmtimer->start(lirc_rx51->pwm_timer);
> + lirc_rx51->dmtimer->set_int_enable(lirc_rx51->pulse_timer, 0);
> + lirc_rx51->dmtimer->start(lirc_rx51->pulse_timer);
>  
>   lirc_rx51->match = 0;
>  
> @@ -100,15 +100,15 @@ static int pulse_timer_set_timeout(struct lirc_rx51 
> *lirc_rx51, int usec)
>   BUG_ON(usec < 0);
>  
>   if (lirc_rx51->match == 0)
> - counter = omap_dm_timer_read_counter(lirc_rx51->pulse_timer);
> + counter = 
> lirc_rx51->dmtimer->read_counter(lirc_rx51->pulse_timer);
>   else
>   counter = lirc_rx51->match;
>  
>   counter += (u32)(lirc_rx51->fclk_khz * usec / (1000));
> - omap_dm_timer_set_match(lirc_rx51->pulse_timer, 1, counter);
> - omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer,
> -  OMAP_TIMER_INT_MATCH);
> - if (tics_after(omap_dm_timer_read_counter(lirc_rx51->pulse_timer),
> + lirc_rx51->dmtimer->set_match(lirc_rx51->pulse_timer, 1, counter);
> + lirc_rx51->dmtimer->set_int_enable(lirc_rx51->pulse_timer,
> +PWM_OMAP_DMTIMER_INT_MATCH);
> + if (tics_after(lirc_rx51->dmtimer->read_counter(lirc_rx51->pulse_timer),
>  counter)) {
>   return 1;
>   }
> @@ -120,18 +120,18 @@ static irqreturn_t lirc_rx51_interrupt_handler(int irq, 
> void *ptr)
>   unsigned int retval;
>   struct lirc_rx51 

[PATCH] tw686x: use a formula instead of two tables for div

[Mauro Carvalho Chehab]

Instead of using two tables to estimate the temporal decimation
factor, use a formula. This allows to get the closest fps, with
sounds better than the current tables.

Compile-tested only.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/tw686x/tw686x-video.c | 34 -
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c
index 253e10823ba3..0210fa304e4c 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -50,28 +50,18 @@ static unsigned int tw686x_fields_map(v4l2_std_id std, 
unsigned int fps)
0x01041041, 0x01104411, 0x0111, 0x0445, 0x04511445,
0x05145145, 0x05151515, 0x05515455, 0x05551555, 0x0555
};
-
-   static const unsigned int std_625_50[26] = {
-   0, 1, 1, 2,  3,  3,  4,  4,  5,  5,  6,  7,  7,
-  8, 8, 9, 10, 10, 11, 11, 12, 13, 13, 14, 14, 0
-   };
-
-   static const unsigned int std_525_60[31] = {
-   0, 1, 1, 1, 2,  2,  3,  3,  4,  4,  5,  5,  6,  6, 7, 7,
-  8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 0, 0
-   };
-
-   unsigned int i;
-
-   if (std & V4L2_STD_525_60) {
-   if (fps >= ARRAY_SIZE(std_525_60))
-   fps = 30;
-   i = std_525_60[fps];
-   } else {
-   if (fps >= ARRAY_SIZE(std_625_50))
-   fps = 25;
-   i = std_625_50[fps];
-   }
+   unsigned int i, max_fps;
+
+   if (std & V4L2_STD_525_60)
+   max_fps = 30;
+   else
+   max_fps = 25;
+
+   i = DIV_ROUND_CLOSEST(15 * fps, max_fps);
+   if (!i)
+   i = 1;  /* Min possible fps */
+   else if (i > 14)
+   i = 0;  /* fps = max_fps */
 
return map[i];
 }
-- 
2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] [media] tw686x: off by one bugs in tw686x_fields_map()

[Mauro Carvalho Chehab]

Em Wed, 27 Apr 2016 07:31:59 -0300
Mauro Carvalho Chehab  escreveu:

> Hi Dan,
> 
> Em Wed, 27 Apr 2016 11:09:28 +0300
> Dan Carpenter  escreveu:
> 
> > The > ARRAY_SIZE() should be >= ARRAY_SIZE(). 
> 
> I actually did this fix when I produced the patch, just I forgot to fold
> it when merging. Anyway, this was fixed upstream by this patch:
>   
> https://git.linuxtv.org/media_tree.git/commit/?id=45c175c4ae9695d6d2f30a45ab7f3866cfac184b
> 
> > Also this is a slightly
> > unrelated cleanup but I replaced the magic numbers 30 and 25 with
> > ARRAY_SIZE() - 1.
> 
> I don't like magic numbers, but, in this very specific case, setting
> frames per second (fps) var to 25 or 30 makes much more sense. The
> rationale is that:
> 
> The V4L2_STD_525_60 macro is for the Countries where the power line 
> uses 60Hz, and V4L2_STD_625_50 for the Countries where the power line
> is 50Hz.
> 
> The broadcast TV sends frames in half of this frequency, so, for
> V4L2_STD_525_60, fps = 30, while, for V4L2_STD_625_50, fps = 25.
> 
> So, in this very specific case, IMHO, it is better to see 25 or 30 there,
> instead of ARRAY_SIZE().
> 
> That's said, I guess one improvement would be to get rid of those two
> arrays and replacing them by a formula, like:
> 
>   i = (max_fps / 2 + 15 * fps) / max_fps;
> if (i > 14)
> i = 0;
> 
> I'll propose such patch for evaluation.

I did some tests to check how that array was determined, using 
the enclosed program.

It seems that the table was generated using this formula:

i2 = (adjust + 15 * fps) / max_fps;
if (fps && !i2)
i2 = 1;
if (i2 > 14)
i2 = 0;

Where adjust is given by:
adjust = 12; /* actually, any value between 11 and 14 */

Using it, I got these results:


60 Hz
fps 0, i1=0, i2=0
fps 1, i1=1, i2=1
fps 2, i1=1, i2=1
fps 3, i1=1, i2=1
fps 4, i1=2, i2=2
fps 5, i1=2, i2=2
fps 6, i1=3, i2=3
fps 7, i1=3, i2=3
fps 8, i1=4, i2=4
fps 9, i1=4, i2=4
fps 10, i1=5, i2=5
fps 11, i1=5, i2=5
fps 12, i1=6, i2=6
fps 13, i1=6, i2=6
fps 14, i1=7, i2=7
fps 15, i1=7, i2=7
fps 16, i1=8, i2=8
fps 17, i1=8, i2=8
fps 18, i1=9, i2=9
fps 19, i1=9, i2=9
fps 20, i1=10, i2=10
fps 21, i1=10, i2=10
fps 22, i1=11, i2=11
fps 23, i1=11, i2=11
fps 24, i1=12, i2=12
fps 25, i1=12, i2=12
fps 26, i1=13, i2=13
fps 27, i1=13, i2=13
fps 28, i1=14, i2=14
NOK fps 29, i1=0, i2=14
fps 30, i1=0, i2=0

50 Hz
fps 0, i1=0, i2=0
fps 1, i1=1, i2=1
fps 2, i1=1, i2=1
fps 3, i1=2, i2=2
NOK fps 4, i1=3, i2=2
fps 5, i1=3, i2=3
fps 6, i1=4, i2=4
fps 7, i1=4, i2=4
fps 8, i1=5, i2=5
fps 9, i1=5, i2=5
fps 10, i1=6, i2=6
fps 11, i1=7, i2=7
fps 12, i1=7, i2=7
fps 13, i1=8, i2=8
fps 14, i1=8, i2=8
fps 15, i1=9, i2=9
fps 16, i1=10, i2=10
fps 17, i1=10, i2=10
fps 18, i1=11, i2=11
fps 19, i1=11, i2=11
fps 20, i1=12, i2=12
fps 21, i1=13, i2=13
fps 22, i1=13, i2=13
fps 23, i1=14, i2=14
fps 24, i1=14, i2=14
fps 25, i1=0, i2=0

IMHO, the two values that are different at the table are wrong ;)

I would also round to the closest number, with probably makes more
sense, and fits well at the API requirements.

The small program to test itis enclosed below. I'll send a patch
getting rid of those tables.

Regards,
Mauro

===

#include 

static const unsigned int std_625_50[26] = {
0, 1, 1, 2,  3,  3,  4,  4,  5,  5,  6,  7,  7,
   8, 8, 9, 10, 10, 11, 11, 12, 13, 13, 14, 14, 0
};

static const unsigned int std_525_60[31] = {
0, 1, 1, 1, 2,  2,  3,  3,  4,  4,  5,  5,  6,  6, 7, 7,
   8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 0, 0
};

void calc_fps(unsigned int max_fps)
{
unsigned int i1, i2, fps, adjust;

//  adjust = max_fps / 2;
adjust = 12; /* 11 a 14 */
for (fps = 0; fps <= max_fps; fps++) {
if (max_fps == 30)
i1 = std_525_60[fps];
else
i1 = std_625_50[fps];

i2 = (adjust + 15 * fps) / max_fps;
if (fps && !i2)
i2 = 1;
if (i2 > 14)
i2 = 0;

//if (i1 != i2)
printf("\t%s fps %d, i1=%d, 

Re: [patch] [media] tw686x: off by one bugs in tw686x_fields_map()

[Dan Carpenter]

On Wed, Apr 27, 2016 at 07:31:59AM -0300, Mauro Carvalho Chehab wrote:
> I don't like magic numbers, but, in this very specific case, setting
> frames per second (fps) var to 25 or 30 makes much more sense.

Fine fine.  I buy that rationale.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] [media] tw686x: off by one bugs in tw686x_fields_map()

[Mauro Carvalho Chehab]

Hi Dan,

Em Wed, 27 Apr 2016 11:09:28 +0300
Dan Carpenter  escreveu:

> The > ARRAY_SIZE() should be >= ARRAY_SIZE(). 

I actually did this fix when I produced the patch, just I forgot to fold
it when merging. Anyway, this was fixed upstream by this patch:

https://git.linuxtv.org/media_tree.git/commit/?id=45c175c4ae9695d6d2f30a45ab7f3866cfac184b

> Also this is a slightly
> unrelated cleanup but I replaced the magic numbers 30 and 25 with
> ARRAY_SIZE() - 1.

I don't like magic numbers, but, in this very specific case, setting
frames per second (fps) var to 25 or 30 makes much more sense. The
rationale is that:

The V4L2_STD_525_60 macro is for the Countries where the power line 
uses 60Hz, and V4L2_STD_625_50 for the Countries where the power line
is 50Hz.

The broadcast TV sends frames in half of this frequency, so, for
V4L2_STD_525_60, fps = 30, while, for V4L2_STD_625_50, fps = 25.

So, in this very specific case, IMHO, it is better to see 25 or 30 there,
instead of ARRAY_SIZE().

That's said, I guess one improvement would be to get rid of those two
arrays and replacing them by a formula, like:

i = (max_fps / 2 + 15 * fps) / max_fps;
if (i > 14)
i = 0;

I'll propose such patch for evaluation.

Regards,
Mauro

> 
> Fixes: 363d79f1d5bd ('[media] tw686x: Don't go past array')
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
> b/drivers/media/pci/tw686x/tw686x-video.c
> index d2a0147..7b87f27 100644
> --- a/drivers/media/pci/tw686x/tw686x-video.c
> +++ b/drivers/media/pci/tw686x/tw686x-video.c
> @@ -64,12 +64,12 @@ static unsigned int tw686x_fields_map(v4l2_std_id std, 
> unsigned int fps)
>   unsigned int i;
>  
>   if (std & V4L2_STD_525_60) {
> - if (fps > ARRAY_SIZE(std_525_60))
> - fps = 30;
> + if (fps >= ARRAY_SIZE(std_525_60))
> + fps = ARRAY_SIZE(std_525_60) - 1;
>   i = std_525_60[fps];
>   } else {
> - if (fps > ARRAY_SIZE(std_625_50))
> - fps = 25;
> + if (fps >= ARRAY_SIZE(std_625_50))
> + fps = ARRAY_SIZE(std_625_50) - 1;
>   i = std_625_50[fps];
>   }
>  


-- 
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: fix media_ioctl use-after-free when driver unbinds

[Mauro Carvalho Chehab]

Hi Shuah,

Good work! I have a few notes below.

Em Tue, 26 Apr 2016 21:08:32 -0600
Shuah Khan  escreveu:

> When driver unbind is run while media_ioctl is in progress, media_ioctl()
> fails with use-after-free. This first use-after-free is followed by more
> user-after-free errors in media_release(), kobject_put(), and cdev_put()
> as driver unbind continues. This problem is found on uvcvideo, em28xx, and
> au0828 drivers and fix has been tested on all three.
> 
> This fix allocates media devnode and manages its lifetime separate from the
> struct media_device. Adds kobject to the media_devnode structure and this
> kobject is set as the cdev parent kobject. This allows cdev_add() to hold
> a reference to it and release the reference in cdev_del() ensuring that the
> media_devnode is not deallocated as long as the application has the cdev
> open.
> 
> The first error is below:
> 
> [  472.424302] 
> ==
> [  472.424333] BUG: KASAN: use-after-free in media_ioctl+0xf0/0x130 [media] 
> at addr 880027b72330
> [  472.424341] Read of size 8 by task media_device_te/1794
> [  472.424348] 
> =
> [  472.424356] BUG kmalloc-4096 (Not tainted): kasan: bad access detected
> [  472.424361] 
> -
> [  472.431973] CPU: 1 PID: 1794 Comm: media_device_te Tainted: GB 
>   4.6.0-rc5 #2
> [  472.431988] Hardware name: Hewlett-Packard HP ProBook 6475b/180F, BIOS 
> 68TTU Ver. F.04 08/03/2012
> [  472.431996]  ea9edc00 88009ddffc78 81aecac3 
> 8801fa403200
> [  472.432016]  880027b72260 88009ddffca8 815359b2 
> 8801fa403200
> [  472.432040]  ea9edc00 880027b72260 a0c9cc60 
> 88009ddffcd0
> [  472.432059] Call Trace:
> [  472.432079]  [] dump_stack+0x67/0x94
> [  472.432092]  [] print_trailer+0x112/0x1a0
> [  472.432108]  [] object_err+0x34/0x40
> [  472.432125]  [] kasan_report_error+0x224/0x530
> [  472.432148]  [] ? 
> __media_device_get_topology+0x1850/0x1850 [media]
> [  472.432167]  [] __asan_report_load8_noabort+0x43/0x50
> [  472.432190]  [] ? media_ioctl+0x120/0x130 [media]
> [  472.432209]  [] media_ioctl+0x120/0x130 [media]
> [  472.432229]  [] do_vfs_ioctl+0x184/0xe80
> [  472.432243]  [] ? ioctl_preallocate+0x1a0/0x1a0
> [  472.432256]  [] ? __hrtimer_init+0x170/0x170
> [  472.432272]  [] ? do_nanosleep+0x161/0x480
> [  472.432298]  [] ? sigprocmask+0x290/0x290
> [  472.432323]  [] ? __fget_light+0x139/0x200
> [  472.432358]  [] SyS_ioctl+0x79/0x90
> [  472.432381]  [] entry_SYSCALL_64_fastpath+0x18/0xa8

This patch is almost identical to my patch, as you took the same
approach as I did:
https://patchwork.linuxtv.org/patch/33577/

So, I compared both to identify the differences. What I noticed is
that:

1) your patch is setting cdev->parent; in my case, I fixed on a
   separate patch. 

IMHO, this should be on a separate patch, as cdev is a separate bug.

2) On my patch, I also fixed the error conditions at 
   __media_device_register(): currently, it has a few issues, and,
   after the patch, if an error occurs, mdev->devnode should be
   set to NULL;

3) you added a kref. I would prefer to see this on a separate patch
   too, as this is not related with moving from an embedded struct
   to a dynamically allocated one. One logical change per patch,
   please.

There's also the point that you're using my patch, but removing
my credits. In this specific case, as a developer, I don't mind
much about that, as it is just one patch and didn't take much
of my time to produce. Yet, maintainers should not allow stripping
off credits, as this could cause troubles later.

So, IMHO, the best would be to split this patch in 3 patches:

- my patch (fixing the context changes);
- cdev patch;
- kref patch.

As a bonus side, by breaking into that, it helps to identify what
fixes are needed if we found similar issues at the other parts of
the subsystems.

If I remember well, I ended by having some cdev troubles with the
V4L2 core on one of my stress test. So, this is something that
we want to double check at RC, DVB and V4L parts that handle
cdev, and eventually porting the changes to the core of those
subsystems.

PS.: I did just a code review. I intend to test this along the
week.

Regards,
Mauro


> 
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/media-device.c   | 32 
>  drivers/media/media-devnode.c  | 23 +++
>  drivers/media/usb/au0828/au0828-core.c |  4 ++--
>  drivers/media/usb/uvc/uvc_driver.c |  2 +-
>  include/media/media-device.h   |  7 ++-
>  include/media/media-devnode.h  |  8 +++-
>  6 files changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git 

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Pavel Machek]

Hi!

> After taking a nap, a question came to my mind - what is that device you're
> using? As some early board versions use VAUX3 for cameras as well.

I got used N900 from a friend. It had czech keyboard, originally, so I
believe it should be regular version, not early one...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch] [media] tw686x: off by one bugs in tw686x_fields_map()

[Dan Carpenter]

The > ARRAY_SIZE() should be >= ARRAY_SIZE().  Also this is a slightly
unrelated cleanup but I replaced the magic numbers 30 and 25 with
ARRAY_SIZE() - 1.

Fixes: 363d79f1d5bd ('[media] tw686x: Don't go past array')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/pci/tw686x/tw686x-video.c 
b/drivers/media/pci/tw686x/tw686x-video.c
index d2a0147..7b87f27 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -64,12 +64,12 @@ static unsigned int tw686x_fields_map(v4l2_std_id std, 
unsigned int fps)
unsigned int i;
 
if (std & V4L2_STD_525_60) {
-   if (fps > ARRAY_SIZE(std_525_60))
-   fps = 30;
+   if (fps >= ARRAY_SIZE(std_525_60))
+   fps = ARRAY_SIZE(std_525_60) - 1;
i = std_525_60[fps];
} else {
-   if (fps > ARRAY_SIZE(std_625_50))
-   fps = 25;
+   if (fps >= ARRAY_SIZE(std_625_50))
+   fps = ARRAY_SIZE(std_625_50) - 1;
i = std_625_50[fps];
}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: tuner: mt2063: use lib gcd

[zengzhaoxiu]

From: Zhaoxiu Zeng 

This patch removes the local MT2063_gcd function, uses lib gcd instead

Signed-off-by: Zhaoxiu Zeng 
---
 drivers/media/tuners/mt2063.c | 30 +-
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/media/tuners/mt2063.c b/drivers/media/tuners/mt2063.c
index 6457ac9..7f0b9d5 100644
--- a/drivers/media/tuners/mt2063.c
+++ b/drivers/media/tuners/mt2063.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mt2063.h"
 
@@ -665,27 +666,6 @@ static u32 MT2063_ChooseFirstIF(struct 
MT2063_AvoidSpursData_t *pAS_Info)
 }
 
 /**
- * gcd() - Uses Euclid's algorithm
- *
- * @u, @v: Unsigned values whose GCD is desired.
- *
- * Returns THE greatest common divisor of u and v, if either value is 0,
- * the other value is returned as the result.
- */
-static u32 MT2063_gcd(u32 u, u32 v)
-{
-   u32 r;
-
-   while (v != 0) {
-   r = u % v;
-   u = v;
-   v = r;
-   }
-
-   return u;
-}
-
-/**
  * IsSpurInBand() - Checks to see if a spur will be present within the IF's
  *  bandwidth. (fIFOut +/- fIFBW, -fIFOut +/- fIFBW)
  *
@@ -731,12 +711,12 @@ static u32 IsSpurInBand(struct MT2063_AvoidSpursData_t 
*pAS_Info,
 ** of f_LO1, f_LO2 and the edge value.  Use the larger of this
 ** gcd-based scale factor or f_Scale.
 */
-   lo_gcd = MT2063_gcd(f_LO1, f_LO2);
-   gd_Scale = max((u32) MT2063_gcd(lo_gcd, d), f_Scale);
+   lo_gcd = gcd(f_LO1, f_LO2);
+   gd_Scale = max((u32) gcd(lo_gcd, d), f_Scale);
hgds = gd_Scale / 2;
-   gc_Scale = max((u32) MT2063_gcd(lo_gcd, c), f_Scale);
+   gc_Scale = max((u32) gcd(lo_gcd, c), f_Scale);
hgcs = gc_Scale / 2;
-   gf_Scale = max((u32) MT2063_gcd(lo_gcd, f), f_Scale);
+   gf_Scale = max((u32) gcd(lo_gcd, f), f_Scale);
hgfs = gf_Scale / 2;
 
n0 = DIV_ROUND_UP(f_LO2 - d, f_LO1 - f_LO2);
-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 00/24] Make Nokia N900 cameras working

[Ivaylo Dimitrov]

Hi,

On 27.04.2016 08:05, Ivaylo Dimitrov wrote:

Hi,

On 27.04.2016 06:08, Sebastian Reichel wrote:

Hi,

On Mon, Apr 25, 2016 at 12:08:00AM +0300, Ivaylo Dimitrov wrote:

Those patch series make cameras on Nokia N900 partially working.
Some more patches are needed, but I've already sent them for
upstreaming so they are not part of the series:

https://lkml.org/lkml/2016/4/16/14
https://lkml.org/lkml/2016/4/16/33

As omap3isp driver supports only one endpoint on ccp2 interface,
but cameras on N900 require different strobe settings, so far
it is not possible to have both cameras correctly working with
the same board DTS. DTS patch in the series has the correct
settings for the front camera. This is a problem still to be
solved.

The needed pipeline could be made with:

media-ctl -r
media-ctl -l '"vs6555 binner 2-0010":1 -> "video-bus-switch":2 [1]'
media-ctl -l '"video-bus-switch":0 -> "OMAP3 ISP CCP2":0 [1]'
media-ctl -l '"OMAP3 ISP CCP2":1 -> "OMAP3 ISP CCDC":0 [1]'
media-ctl -l '"OMAP3 ISP CCDC":2 -> "OMAP3 ISP preview":0 [1]'
media-ctl -l '"OMAP3 ISP preview":1 -> "OMAP3 ISP resizer":0 [1]'
media-ctl -l '"OMAP3 ISP resizer":1 -> "OMAP3 ISP resizer output":0 [1]'
media-ctl -V '"vs6555 pixel array 2-0010":0 [SGRBG10/648x488
(0,0)/648x488 (0,0)/648x488]'
media-ctl -V '"vs6555 binner 2-0010":1 [SGRBG10/648x488 (0,0)/648x488
(0,0)/648x488]'
media-ctl -V '"OMAP3 ISP CCP2":0 [SGRBG10 648x488]'
media-ctl -V '"OMAP3 ISP CCP2":1 [SGRBG10 648x488]'
media-ctl -V '"OMAP3 ISP CCDC":2 [SGRBG10 648x488]'
media-ctl -V '"OMAP3 ISP preview":1 [UYVY 648x488]'
media-ctl -V '"OMAP3 ISP resizer":1 [UYVY 656x488]'

and tested with:

mplayer -tv
driver=v4l2:width=656:height=488:outfmt=uyvy:device=/dev/video6 -vo
xv -vf screenshot tv://


4.6-rc4 + twl regulator patch + the patches mentioned above + this
patchset (I put everything together here [0]) do _not_ work for me.
The error matches what I have seen when I was working on it: No
image data seems to be received by the ISP. For example there are
no related IRQs:

root@n900:~# cat /proc/interrupts  | grep ISP
  40:  0  INTC  24 Edge  480bd400.mmu, OMAP3 ISP

I tested with mpv and yavta (yavta --capture=8 --pause --skip 0
--format UYVY --size 656x488 /dev/video6)

[0]
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo




Ok, going to diff with my tree to see what I have missed to send in the
patchset



Now, that's getting weird.

I cloned n900-camera-ivo, copied rx51_defconfig from my tree, added:

CONFIG_VIDEO_SMIAREGS=m
CONFIG_VIDEO_ET8EK8=m
CONFIG_VIDEO_BUS_SWITCH=m

to it, make mrproper, built the kernel using rx51_defconfig and made 
initrd for rescueos, so to be sure that maemo5 did not influence cameras 
somehow.


Booted the device with flasher3.5:

sudo flasher-3.5 -k zImage -n ramfs -l -b"rootdelay root=/dev/ram0 
mtdoops.mtddev=log log_buf_len=1M"


ivo@ivo-H81M-S2PV:~/maemo/rescueos$ sudo ifconfig usb0 192.168.2.14
ivo@ivo-H81M-S2PV:~/maemo/rescueos$ telnet 192.168.2.15
Trying 192.168.2.15...
Connected to 192.168.2.15.
Escape character is '^]'.

rescueos login: root
Password:
~$ modprobe smiapp
~$ cd /camera/
/camera$ export LD_LIBRARY_PATH=./
/camera$ ./media-ctl -r
/camera$ ./media-ctl -l '"vs6555 binner 2-0010":1 -> 
"video-bus-switch":2 [1]'

/camera$ ./media-ctl -l '"video-bus-switch":0 -> "OMAP3 ISP CCP2":0 [1]'
/camera$ ./media-ctl -l '"OMAP3 ISP CCP2":1 -> "OMAP3 ISP CCDC":0 [1]'
/camera$ ./media-ctl -l '"OMAP3 ISP CCDC":2 -> "OMAP3 ISP preview":0 [1]'
/camera$ ./media-ctl -l '"OMAP3 ISP preview":1 -> "OMAP3 ISP resizer":0 [1]'
/camera$ ./media-ctl -l '"OMAP3 ISP resizer":1 -> "OMAP3 ISP resizer 
output":0 [1]'
/camera$ ./media-ctl -V '"vs6555 pixel array 2-0010":0 [SGRBG10/648x488 
(0,0)/648x488 (0,0)/648x488]'
/camera$ ./media-ctl -V '"vs6555 binner 2-0010":1 [SGRBG10/648x488 
(0,0)/648x488 (0,0)/648x488]'

/camera$ ./media-ctl -V '"OMAP3 ISP CCP2":0 [SGRBG10 648x488]'
/camera$ ./media-ctl -V '"OMAP3 ISP CCP2":1 [SGRBG10 648x488]'
/camera$ ./media-ctl -V '"OMAP3 ISP CCDC":2 [SGRBG10 648x488]'
/camera$ ./media-ctl -V '"OMAP3 ISP preview":1 [UYVY 648x488]'
/camera$ ./media-ctl -V '"OMAP3 ISP resizer":1 [UYVY 656x488]'
/camera$ ./yavta --capture=8 --pause --skip 0 --format UYVY --size 
656x488 /dev/video6

Device /dev/video6 opened.
Device `OMAP3 ISP resizer output' on `media' is a video capture device.
Video format set: UYVY (59565955) 656x488 (stride 1312) buffer size 640256
Video format: UYVY (59565955) 656x488 (stride 1312) buffer size 640256
8 buffers requested.
length: 640256 offset: 0 timestamp type: monotonic
Buffer 0 mapped at address 0xb6de5000.
length: 640256 offset: 643072 timestamp type: monotonic
Buffer 1 mapped at address 0xb6d48000.
length: 640256 offset: 1286144 timestamp type: monotonic
Buffer 2 mapped at address 0xb6cab000.
length: 640256 offset: 1929216 timestamp type: monotonic
Buffer 3 mapped at address 0xb6c0e000.
length: 640256 offset: 2572288 timestamp type: monotonic
Buffer 4