[PATCH v4] media: video-i2c: add video-i2c driver

2016-11-25 Thread Matt Ranostay
There are several thermal sensors that only have a low-speed bus
interface but output valid video data. This patchset enables support
for the AMG88xx "Grid-Eye" sensor family.

Cc: Attila Kinali 
Cc: Marek Vasut 
Cc: Luca Barbato 
Cc: Laurent Pinchart 
Signed-off-by: Matt Ranostay 
---
Changes from v1:
* correct i2c_polling_remove() operations
* fixed delay calcuation in buffer_queue()
* add include linux/slab.h

Changes from v2:
* fix build error due to typo in include of slab.h

Changes form v3:
* switch data transport to a kthread to avoid to .buf_queue that can't sleep
* change naming from i2c-polling to video-i2c
* make the driver for single chipset under another uses the driver

 drivers/media/i2c/Kconfig |   9 +
 drivers/media/i2c/Makefile|   1 +
 drivers/media/i2c/video-i2c.c | 553 ++
 3 files changed, 563 insertions(+)
 create mode 100644 drivers/media/i2c/video-i2c.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index b31fa6fae009..ef84715293a9 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -768,6 +768,15 @@ config VIDEO_M52790
 
 To compile this driver as a module, choose M here: the
 module will be called m52790.
+
+config VIDEO_I2C
+   tristate "I2C transport video support"
+   depends on VIDEO_V4L2 && I2C
+   select VIDEOBUF2_VMALLOC
+   ---help---
+ Enable the I2C transport video support which supports the
+ following:
+  * Panasonic AMG88xx Grid-Eye Sensors
 endmenu
 
 menu "Sensors used on soc_camera driver"
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 92773b2e6225..7c8eeb213c3b 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.o
 obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
+obj-$(CONFIG_VIDEO_I2C)+= video-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
new file mode 100644
index ..59b6f68daed6
--- /dev/null
+++ b/drivers/media/i2c/video-i2c.c
@@ -0,0 +1,553 @@
+/*
+ * video-i2c.c - Support for I2C transport video devices
+ *
+ * Copyright (C) 2016 Matt Ranostay 
+ *
+ * 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.
+ *
+ * Supported:
+ * - Panasonic AMG88xx Grid-Eye Sensors
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIDEO_I2C_DRIVER   "video-i2c"
+#define MAX_BUFFER_SIZE128
+
+struct video_i2c_chip;
+
+struct video_i2c_buffer {
+   struct vb2_v4l2_buffer vb;
+   struct list_head list;
+};
+
+struct video_i2c_data {
+   struct i2c_client *client;
+   const struct video_i2c_chip *chip;
+   struct mutex lock;
+   spinlock_t slock;
+   struct mutex queue_lock;
+
+   struct v4l2_device v4l2_dev;
+   struct video_device vdev;
+   struct vb2_queue vb_vidq;
+
+   struct task_struct *kthread_vid_cap;
+   struct list_head vid_cap_active;
+};
+
+static struct v4l2_fmtdesc amg88xx_format = {
+   .description = "12-bit Greyscale",
+   .pixelformat = V4L2_PIX_FMT_Y12,
+};
+
+static struct v4l2_frmsize_discrete amg88xx_size = {
+   .width = 8,
+   .height = 8,
+};
+
+struct video_i2c_chip {
+   /* video dimensions */
+   struct v4l2_fmtdesc *format;
+   struct v4l2_frmsize_discrete *size;
+
+   /* max frames per second */
+   unsigned int max_fps;
+
+   /* pixel buffer size */
+   unsigned int buffer_size;
+
+   /* pixel size in bits */
+   unsigned int bpp;
+
+   /* xfer function */
+   int (*xfer)(struct video_i2c_data *data, char *buf);
+};
+
+static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
+{
+   struct i2c_client *client = data->client;
+   struct i2c_msg msg[2];
+   u8 reg = 0x80;
+   int ret;
+
+   msg[0].addr = client->addr;
+   msg[0].flags = 0;
+   msg[0].len = 1;
+   msg[0].buf  = (char *) ®
+
+   msg[1].addr = client->addr;
+   msg[1].flags = I2C_M_RD;
+   msg[1].len = data->chip->buffer_size;
+   msg[1].buf = (char *) buf;
+
+   ret = i2c_transfer(c

cron job: media_tree daily build: ERRORS

2016-11-25 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sat Nov 26 05:00:20 CET 2016
media-tree git hash:d3d83ee20afda16ad0133ba00f63c11a8d842a35
media_build git hash:   1606032398b1d79149c1507be2029e1a00d8dff0
v4l-utils git hash: dab9bf5687eddea2f4cb8cdb38b3bbc5b079a037
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.8.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: 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: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.12.67-i686: OK
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-rc5-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: OK
linux-3.12.67-x86_64: OK
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: OK
linux-4.9-rc5-x86_64: OK
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Alex Deucher
On Fri, Nov 25, 2016 at 2:34 PM, Jason Gunthorpe
 wrote:
> On Fri, Nov 25, 2016 at 12:16:30PM -0500, Serguei Sagalovitch wrote:
>
>> b) Allocation may not  have CPU address  at all - only GPU one.
>
> But you don't expect RDMA to work in the case, right?
>
> GPU people need to stop doing this windowed memory stuff :)
>

Blame 32 bit systems and GPUs with tons of vram :)

I think resizable bars are finally coming in a useful way so this
should go away soon.

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


[patch] [media] media: ti-vpe: vpdma: fix a timeout loop

2016-11-25 Thread Dan Carpenter
The check assumes that we end on zero but actually we end on -1.  Change
the post-op to a pre-op so that we do end on zero.  Techinically now we
only loop 499 times instead of 500 but that's fine.

Fixes: dc12b124353b ("[media] media: ti-vpe: vpdma: Add abort channel desc and 
cleanup APIs")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/platform/ti-vpe/vpdma.c 
b/drivers/media/platform/ti-vpe/vpdma.c
index 13bfd71..23472e3 100644
--- a/drivers/media/platform/ti-vpe/vpdma.c
+++ b/drivers/media/platform/ti-vpe/vpdma.c
@@ -453,7 +453,7 @@ int vpdma_list_cleanup(struct vpdma_data *vpdma, int 
list_num,
if (ret)
return ret;
 
-   while (vpdma_list_busy(vpdma, list_num) && timeout--)
+   while (vpdma_list_busy(vpdma, list_num) && --timeout)
;
 
if (timeout == 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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Jason Gunthorpe
On Fri, Nov 25, 2016 at 09:40:10PM +0100, Christian König wrote:

> We call this "userptr" and it's just a combination of get_user_pages() on
> command submission and making sure the returned list of pages stays valid
> using a MMU notifier.

Doesn't that still pin the page?

> The "big" problem with this approach is that it is horrible slow. I mean
> seriously horrible slow so that we actually can't use it for some of the
> purposes we wanted to use it.
> 
> >The code moving the page will move it and the next GPU command that
> >needs it will refault it in the usual way, just like the CPU would.
> 
> And here comes the problem. CPU do this on a page by page basis, so they
> fault only what needed and everything else gets filled in on demand. This
> results that faulting a page is relatively light weight operation.
>
> But for GPU command submission we don't know which pages might be accessed
> beforehand, so what we do is walking all possible pages and make sure all of
> them are present.

Little confused why this is slow? So you fault the entire user MM into
your page tables at start of day and keep track of it with mmu
notifiers?

> >This might be much more efficient since it optimizes for the common
> >case of unchanging translation tables.
> 
> Yeah, completely agree. It works perfectly fine as long as you don't have
> two drivers trying to mess with the same page.

Well, the idea would be to not have the GPU block the other driver
beyond hinting that the page shouldn't be swapped out.

> >This assumes the commands are fairly short lived of course, the
> >expectation of the mmu notifiers is that a flush is reasonably prompt
> 
> Correct, this is another problem. GFX command submissions usually don't take
> longer than a few milliseconds, but compute command submission can easily
> take multiple hours.

So, that won't work - you have the same issue as RDMA with work loads
like that.

If you can't somehow fence the hardware then pinning is the only
solution. Felix has the right kind of suggestion for what is needed -
globally stop the GPU, fence the DMA, fix the page tables, and start
it up again. :\

> I can easily imagine what would happen when kswapd is blocked by a GPU
> command submission for an hour or so while the system is under memory
> pressure :)

Right. The advantage of pinning is it tells the other stuff not to
touch the page and doesn't block it, MMU notifiers have to be able to
block&fence quickly.

> I'm thinking on this problem for about a year now and going in circles for
> quite a while. So if you have ideas on this even if they sound totally
> crazy, feel free to come up.

Well, it isn't a software problem. From what I've seen in this thread
the GPU application requires coherent page table mirroring, so the
only full & complete solution is going to be to actually implement
that somehow in GPU hardware.

Everything else is going to be deeply flawed somehow. Linux just
doesn't have the support for this kind of stuff - and I'm honestly not
sure something better is even possible considering the hardware
constraints

This doesn't have to be faulting, but really anything that lets you
pause the GPU DMA and reload the page tables.

You might look at trying to use the IOMMU and/or PCI ATS in very new
hardware. IIRC the physical IOMMU hardware can do the fault and fence
and block stuff, but I'm not sure about software support for using the
IOMMU to create coherent user page table mirrors - that is something
Linux doesn't do today. But there is demand for this kind of capability..

Jason
--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Felix Kuehling
On 16-11-25 12:20 PM, Serguei Sagalovitch wrote:
>
>> A white list may end up being rather complicated if it has to cover
>> different CPU generations and system architectures. I feel this is a
>> decision user space could easily make.
>>
>> Logan
> I agreed that it is better to leave up to user space to check what is
> working
> and what is not. I found that write is practically always working but
> read very
> often not. Also sometimes system BIOS update could fix the issue.
>
But is user mode always aware that P2P is going on or even possible? For
example you may have a library reading a buffer from a file, but it
doesn't necessarily know where that buffer is located (system memory,
VRAM, ...) and it may not know what kind of the device the file is on
(SATA drive, NVMe SSD, ...). The library will never know if all it gets
is a pointer and a file descriptor.

The library ends up calling a read system call. Then it would be up to
the kernel to figure out the most efficient way to read the buffer from
the file. If supported, it could use P2P between a GPU and NVMe where
the NVMe device performs a DMA write to VRAM.

If you put the burden of figuring out the P2P details on user mode code,
I think it will severely limit the use cases that actually take
advantage of it. You also risk a bunch of different implementations that
get it wrong half the time on half the systems out there.

Regards,
  Felix


--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Felix Kuehling

On 16-11-25 03:40 PM, Christian König wrote:
> Am 25.11.2016 um 20:32 schrieb Jason Gunthorpe:
>> This assumes the commands are fairly short lived of course, the
>> expectation of the mmu notifiers is that a flush is reasonably prompt
>
> Correct, this is another problem. GFX command submissions usually
> don't take longer than a few milliseconds, but compute command
> submission can easily take multiple hours.
>
> I can easily imagine what would happen when kswapd is blocked by a GPU
> command submission for an hour or so while the system is under memory
> pressure :)
>
> I'm thinking on this problem for about a year now and going in circles
> for quite a while. So if you have ideas on this even if they sound
> totally crazy, feel free to come up.

Our GPUs (at least starting with VI) support compute-wave-save-restore
and can swap out compute queues with fairly low latency. Yes, there is
some overhead (both memory usage and time), but it's a fairly regular
thing with our hardware scheduler (firmware, actually) when we need to
preempt running compute queues to update runlists or we overcommit the
hardware queue resources.

Regards,
  Felix

--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Serguei Sagalovitch


On 2016-11-25 03:26 PM, Felix Kuehling wrote:

On 16-11-25 12:20 PM, Serguei Sagalovitch wrote:

A white list may end up being rather complicated if it has to cover
different CPU generations and system architectures. I feel this is a
decision user space could easily make.

Logan

I agreed that it is better to leave up to user space to check what is
working
and what is not. I found that write is practically always working but
read very
often not. Also sometimes system BIOS update could fix the issue.


But is user mode always aware that P2P is going on or even possible? For
example you may have a library reading a buffer from a file, but it
doesn't necessarily know where that buffer is located (system memory,
VRAM, ...) and it may not know what kind of the device the file is on
(SATA drive, NVMe SSD, ...). The library will never know if all it gets
is a pointer and a file descriptor.

The library ends up calling a read system call. Then it would be up to
the kernel to figure out the most efficient way to read the buffer from
the file. If supported, it could use P2P between a GPU and NVMe where
the NVMe device performs a DMA write to VRAM.

If you put the burden of figuring out the P2P details on user mode code,
I think it will severely limit the use cases that actually take
advantage of it. You also risk a bunch of different implementations that
get it wrong half the time on half the systems out there.

Regards,
   Felix



I agreed in theory with you but  I must admit that I do not know how
kernel could effectively collect all informations without running
pretty complicated tests each time on boot-up (if any configuration
changed including BIOS settings)  and on pnp events. Also for efficient
way kernel needs to know performance results (and it could also
depends on clock / power mode) for read/write of each pair devices, for
double-buffering it needs to know / detect on which NUMA node
to allocate, etc. etc.  Also  device could be fully configured only
on the first request for access so it may be needed to change initialization
sequences.

--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Christian König

Am 25.11.2016 um 20:32 schrieb Jason Gunthorpe:

On Fri, Nov 25, 2016 at 02:22:17PM +0100, Christian König wrote:


Like you say below we have to handle short lived in the usual way, and
that covers basically every device except IB MRs, including the
command queue on a NVMe drive.

Well a problem which wasn't mentioned so far is that while GPUs do have a
page table to mirror the CPU page table, they usually can't recover from
page faults.
So what we do is making sure that all memory accessed by the GPU Jobs stays
in place while those jobs run (pretty much the same pinning you do for the
DMA).

Yes, it is DMA, so this is a valid approach.

But, you don't need page faults from the GPU to do proper coherent
page table mirroring. Basically when the driver submits the work to
the GPU it 'faults' the pages into the CPU and mirror translation
table (instead of pinning).

Like in ODP, MMU notifiers/HMM are used to monitor for translation
changes. If a change comes in the GPU driver checks if an executing
command is touching those pages and blocks the MMU notifier until the
command flushes, then unfaults the page (blocking future commands) and
unblocks the mmu notifier.


Yeah, we have a function to "import" anonymous pages from a CPU pointer 
which works exactly that way as well.


We call this "userptr" and it's just a combination of get_user_pages() 
on command submission and making sure the returned list of pages stays 
valid using a MMU notifier.


The "big" problem with this approach is that it is horrible slow. I mean 
seriously horrible slow so that we actually can't use it for some of the 
purposes we wanted to use it.



The code moving the page will move it and the next GPU command that
needs it will refault it in the usual way, just like the CPU would.


And here comes the problem. CPU do this on a page by page basis, so they 
fault only what needed and everything else gets filled in on demand. 
This results that faulting a page is relatively light weight operation.


But for GPU command submission we don't know which pages might be 
accessed beforehand, so what we do is walking all possible pages and 
make sure all of them are present.


Now as far as I understand it the I/O subsystem for example assumes that 
it can easily change the CPU page tables without much overhead. So for 
example when a page can't modified it is temporary marked as readonly 
AFAIK (you are probably way deeper into this than me, so please confirm).


That absolutely kills any performance for GPU command submissions. We 
have use cases where we practically ended up playing ping/pong between 
the GPU driver trying to grab the page with get_user_pages() and sombody 
else in the kernel marking it readonly.



This might be much more efficient since it optimizes for the common
case of unchanging translation tables.


Yeah, completely agree. It works perfectly fine as long as you don't 
have two drivers trying to mess with the same page.



This assumes the commands are fairly short lived of course, the
expectation of the mmu notifiers is that a flush is reasonably prompt


Correct, this is another problem. GFX command submissions usually don't 
take longer than a few milliseconds, but compute command submission can 
easily take multiple hours.


I can easily imagine what would happen when kswapd is blocked by a GPU 
command submission for an hour or so while the system is under memory 
pressure :)


I'm thinking on this problem for about a year now and going in circles 
for quite a while. So if you have ideas on this even if they sound 
totally crazy, feel free to come up.


Cheers,
Christian.

--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Jason Gunthorpe
On Fri, Nov 25, 2016 at 02:49:50PM -0500, Serguei Sagalovitch wrote:

> GPU could perfectly access all VRAM.  It is only issue for p2p without
> special interconnect and CPU access. Strictly speaking as long as we
> have "bus address"  we could have RDMA but  I agreed that for
> RDMA we could/should(?) always "request"  CPU address (I hope that we
> could forget about 32-bit application :-)).

At least on x86 if you have a bus address you have a CPU address. All
RDMAable VRAM has to be visible in the BAR.

> BTW/FYI: About CPU access: Some user-level API is mainly handle based
> so there is no need for CPU access by default.

You mean no need for the memory to be virtually mapped into the
process?

Do you expect to RDMA from this kind of API? How will that work?

Jason
--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Serguei Sagalovitch

On 2016-11-25 02:34 PM, Jason Gunthorpe wrote:

On Fri, Nov 25, 2016 at 12:16:30PM -0500, Serguei Sagalovitch wrote:


b) Allocation may not  have CPU address  at all - only GPU one.

But you don't expect RDMA to work in the case, right?

GPU people need to stop doing this windowed memory stuff :)

GPU could perfectly access all VRAM.  It is only issue for p2p without
special interconnect and CPU access. Strictly speaking as long as we
have "bus address"  we could have RDMA but  I agreed that for
RDMA we could/should(?) always "request"  CPU address (I hope that we
could forget about 32-bit application :-)).

BTW/FYI: About CPU access: Some user-level API is mainly handle based
so there is no need for CPU access by default.

About "visible" / non-visible VRAM parts: I assume  that going
forward we will be able to get rid from it completely as soon as support
for resizable PCI BAR will be implemented and/or old/current h/w
will become obsolete.
--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Jason Gunthorpe
On Thu, Nov 24, 2016 at 11:58:17PM -0800, Christoph Hellwig wrote:
> On Thu, Nov 24, 2016 at 11:11:34AM -0700, Logan Gunthorpe wrote:
> > * Regular DAX in the FS doesn't work at this time because the FS can
> > move the file you think your transfer to out from under you. Though I
> > understand there's been some work with XFS to solve that issue.
> 
> The file system will never move anything under locked down pages,
> locking down pages is used exactly to protect against that.

.. And ODP style mmu notifiers work correctly as well, I'd assume.

So this should work with ZONE_DEVICE, if it doesn't it is a filesystem
bug?

> really want a notification to the consumer if the file systems wants
> to remove the mapping.  We have implemented that using FL_LAYOUTS locks
> for NFSD, but only XFS supports it so far.  Without that a long term
> locked down region of memory (e.g. a kernel MR) would prevent various
> file operations that would simply hang.

So you imagine a signal back to user space asking user space to drop
any RDMA MRS so the FS can relocate things?

Do we need that, or should we encourage people to use either short
lived MRs or ODP MRs when working with scenarios that need FS
relocation?

Jason
--
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 1/1] smiapp: Implement power-on and power-off sequences without runtime PM

2016-11-25 Thread Laurent Pinchart
Hi Alan,

On Friday 25 Nov 2016 10:21:21 Alan Stern wrote:
> On Fri, 25 Nov 2016, Sakari Ailus wrote:
> > On Thu, Nov 24, 2016 at 09:15:39PM -0500, Alan Stern wrote:
> >> On Fri, 25 Nov 2016, Laurent Pinchart wrote:
> >>> Dear linux-pm developers, what's the suggested way to ensure that a
> >>> runtime- pm-enabled driver can run fine on a system with CONFIG_PM
> >>> disabled ?
> >>
> >> The exact point of your question isn't entirely clear.  In the most
> >> literal sense, the best ways to ensure this are (1) audit the code, and
> >> (2) actually try it.
> >> 
> >> I have a feeling this doesn't quite answer your question, however.  :-)
> > 
> > The question is related to devices that require certain power-up and
> > power-down sequences that are now implemented as PM runtime hooks that,
> > without CONFIG_PM defined, will not be executed. Is there a better way
> > than to handle this than have an implementation in the driver for the PM
> > runtime and non-PM runtime case separately?
> 
> Yes, there is a better way.  For the initial power-up and final
> power-down sequences, don't rely on the PM core to invoke the
> callbacks.  Just call them directly, yourself.
> 
> For example, as part of the probe routine, instead of doing this:
> 
>   pm_runtime_set_suspended(dev);
>   pm_runtime_enable(dev);
>   pm_runtime_get_sync(dev);
> 
> Do this:
> 
>   pm_runtime_set_active(dev);
>   pm_runtime_get_noresume(dev);
>   pm_runtime_enable(dev);
>   /*
>* In case CONFIG_PM is disabled, invoke the runtime-resume
>* callback directly.
>*/
>   my_runtime_resume(dev);

Wouldn't it be cleaner for drivers not to have to handle this manually (which 
gives an opportunity to get it wrong) but instead have pm_runtime_enable() 
call the runtime resume callback when CONFIG_PM is disabled ?

-- 
Regards,

Laurent Pinchart

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


Re: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Jason Gunthorpe
On Fri, Nov 25, 2016 at 12:16:30PM -0500, Serguei Sagalovitch wrote:

> b) Allocation may not  have CPU address  at all - only GPU one.

But you don't expect RDMA to work in the case, right?

GPU people need to stop doing this windowed memory stuff :)

Jason
--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Jason Gunthorpe
On Fri, Nov 25, 2016 at 02:22:17PM +0100, Christian König wrote:

> >Like you say below we have to handle short lived in the usual way, and
> >that covers basically every device except IB MRs, including the
> >command queue on a NVMe drive.
> 
> Well a problem which wasn't mentioned so far is that while GPUs do have a
> page table to mirror the CPU page table, they usually can't recover from
> page faults.

> So what we do is making sure that all memory accessed by the GPU Jobs stays
> in place while those jobs run (pretty much the same pinning you do for the
> DMA).

Yes, it is DMA, so this is a valid approach.

But, you don't need page faults from the GPU to do proper coherent
page table mirroring. Basically when the driver submits the work to
the GPU it 'faults' the pages into the CPU and mirror translation
table (instead of pinning).

Like in ODP, MMU notifiers/HMM are used to monitor for translation
changes. If a change comes in the GPU driver checks if an executing
command is touching those pages and blocks the MMU notifier until the
command flushes, then unfaults the page (blocking future commands) and
unblocks the mmu notifier.

The code moving the page will move it and the next GPU command that
needs it will refault it in the usual way, just like the CPU would.

This might be much more efficient since it optimizes for the common
case of unchanging translation tables.

This assumes the commands are fairly short lived of course, the
expectation of the mmu notifiers is that a flush is reasonably prompt
..

> >Serguei, what is your plan in GPU land for migration? Ie if I have a
> >CPU mapped page and the GPU moves it to VRAM, it becomes non-cachable
> >- do you still allow the CPU to access it? Or do you swap it back to
> >cachable memory if the CPU touches it?
> 
> Depends on the policy in command, but currently it's the other way around
> most of the time.
> 
> E.g. we allocate memory in VRAM, the CPU writes to it WC and avoids reading
> because that is slow, the GPU in turn can access it with full speed.
> 
> When we run out of VRAM we move those allocations to system memory and
> update both the CPU as well as the GPU page tables.
> 
> So that move is transparent for both userspace as well as shaders running on
> the GPU.

That makes sense to me, but the objection that came back for
non-cachable CPU mappings is that it basically breaks too much stuff
subtly, eg atomics, unaligned accesses, the CPU threading memory
model, all change on various architectures and break when caching is
disabled.

IMHO that is OK for specialty things like the GPU where the mmap comes
in via drm or something and apps know to handle that buffer specially.

But it is certainly not OK for DAX where the application is coded for
normal file open()/mmap() is not prepared for a mmap where (eg)
unaligned read accesses or atomics don't work depending on how the
filesystem is setup.

Which is why I think iopmem is still problematic..

At the very least I think a mmap flag or open flag should be needed to
opt into this behavior and by default non-cachebale DAX mmaps should
be paged into system ram when the CPU accesses them.

I'm hearing most people say ZONE_DEVICE is the way to handle this,
which means the missing remaing piece for RDMA is some kind of DMA
core support for p2p address translation..

Jason
--
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] uvcvideo: freeing an error pointer

2016-11-25 Thread Dan Carpenter
On Fri, Nov 25, 2016 at 06:02:45PM +0200, Laurent Pinchart wrote:
> Sakari Ailus (CC'ed) has expressed the opinion that we might want to go one 
> step further and treat error pointers the same way we treat NULL or ZERO 
> pointers today, by just returning without logging anything. The reasoning is 
> that accepting a NULL pointer in kfree() was decided before we made extensive 
> use of allocation APIs returning error pointers, so it could be time to 
> update 
> kfree() based on the current allocation usage patterns.

Just don't free things that haven't been allocated.  That honestly seems
like a simple rule to me, whenever I touch error handling code it feels
better and simpler after I fix the bugs.  Error handling doesn't have to
be complicated if you just follow the rules.

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] uvcvideo: freeing an error pointer

2016-11-25 Thread Dan Carpenter
On Fri, Nov 25, 2016 at 03:57:51PM +0200, Laurent Pinchart wrote:
> diff --git a/mm/slab.c b/mm/slab.c
> index 0b0550ca85b4..a7eb830c6684 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3819,6 +3819,8 @@ void kfree(const void *objp)
>  
>   if (unlikely(ZERO_OR_NULL_PTR(objp)))
>   return;
> + if (WARN_ON(IS_ERR(objp)))
> + return;
>   local_irq_save(flags);
>   kfree_debugcheck(objp);
>   c = virt_to_cache(objp);
> 

A bunch of people have proposed that.  You're the first person to
actually write up a patch.  Feel free to send it.  ;)

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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Serguei Sagalovitch

On 2016-11-25 08:22 AM, Christian König wrote:



Serguei, what is your plan in GPU land for migration? Ie if I have a
CPU mapped page and the GPU moves it to VRAM, it becomes non-cachable
- do you still allow the CPU to access it? Or do you swap it back to
cachable memory if the CPU touches it?


Depends on the policy in command, but currently it's the other way 
around most of the time.


E.g. we allocate memory in VRAM, the CPU writes to it WC and avoids 
reading because that is slow, the GPU in turn can access it with full 
speed.


When we run out of VRAM we move those allocations to system memory and 
update both the CPU as well as the GPU page tables.


So that move is transparent for both userspace as well as shaders 
running on the GPU.

I would like to add more in relation to  CPU access :

a) we could have CPU-accessible part of VRAM ("inside" of PCIe BAR register)
and non-CPU  accessible part.  As the result if user needs to have
CPU access than memory should be located in CPU-accessible part
of VRAM or in system memory.

Application/user mode driver could specify preference/hints of
locations based on their assumption / knowledge about access
patterns requirements, game resolution,  knowledge
about size of VRAM memory, etc.  So if CPU access performance
is critical then such memory should be allocated in system memory
as  the first (and may be only) choice.

b) Allocation may not  have CPU address  at all - only GPU one.
Also we may not be able to have CPU address/accesses for all VRAM
memory but memory may still  be migrated in any case unrelated
if we have CPU address or not.

c) " VRAM, it becomes non-cachable "
Strictly speaking VRAM is configured as WC (write-combined memory) to
provide fast CPU write access. Also it was found that sometimes if CPU
access is not critical from performance perspective it may be useful
to allocate/program system memory also as WC to  avoid needs for
extra "snooping" to synchronize with CPU caches during GPU access.
So potentially system memory could be WC too.


--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Serguei Sagalovitch



Well, I guess there's some consensus building to do. The existing
options are:

* Device DAX: which could work but the problem I see with it is that it
only allows one application to do these transfers. Or there would have
to be some user-space coordination to figure which application gets what
memeroy.
About one application restriction: so it is per memory mapping? I assume 
that
it should not be problem for one application to do transfer to the 
several devices

simultaneously? Am I right?

May be we should follow RDMA MR design and register memory for p2p 
transfer from user

space?

What about the following:

a)  Device DAX is created
b) "Normal" (movable, etc.) allocation will be done for PCIe memory and 
CPU pointer/access will

be requested.
c)  p2p_mr_register() will be called and CPU pointer (mmap( on DAX 
Device)) will be returned.
Accordingly such memory will be marked as "unmovable" by e.g. graphics 
driver.

d) When p2p is not needed then p2p_mr_unregister() will be called.

What do you think? Will it work?


--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Serguei Sagalovitch



A white list may end up being rather complicated if it has to cover
different CPU generations and system architectures. I feel this is a
decision user space could easily make.

Logan
I agreed that it is better to leave up to user space to check what is 
working
and what is not. I found that write is practically always working but 
read very

often not. Also sometimes system BIOS update could fix the issue.

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


[PATCH 2/6] drm: writeback: Add out-fences for writeback connectors

2016-11-25 Thread Brian Starkey
Add the OUT_FENCE_PTR property to writeback connectors, to enable
userspace to get a fence which will signal once the writeback is
complete. It is not allowed to request an out-fence without a
framebuffer attached to the connector.

A timeline is added to drm_writeback_connector for use by the writeback
out-fences.

In the case of a commit failure or DRM_MODE_ATOMIC_TEST_ONLY, the fence
is set to -1.

Changes from v2:
 - Rebase onto Gustavo Padovan's v9 explicit sync series
 - Change out_fence_ptr type to s32 __user *
 - Set *out_fence_ptr to -1 in drm_atomic_connector_set_property
 - Store fence in drm_writeback_job
 Gustavo Padovan:
 - Move out_fence_ptr out of connector_state
 - Signal fence from drm_writeback_signal_completion instead of
   in driver directly

Signed-off-by: Brian Starkey 
---
 drivers/gpu/drm/drm_atomic.c|   96 +
 drivers/gpu/drm/drm_writeback.c |   99 ++-
 include/drm/drm_atomic.h|8 
 include/drm/drm_connector.h |8 ++--
 include/drm/drm_writeback.h |   39 ++-
 5 files changed, 234 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 343e2b7..0e3c900 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -308,6 +308,32 @@ static s64 __user *get_out_fence_for_crtc(struct 
drm_atomic_state *state,
return fence_ptr;
 }
 
+static int set_out_fence_for_connector(struct drm_atomic_state *state,
+   struct drm_connector *connector,
+   s64 __user *fence_ptr)
+{
+   unsigned int index = drm_connector_index(connector);
+
+   if (fence_ptr && put_user(-1, fence_ptr))
+   return -EFAULT;
+
+   state->connectors[index].out_fence_ptr = fence_ptr;
+
+   return 0;
+}
+
+static s64 __user *get_out_fence_for_connector(struct drm_atomic_state *state,
+  struct drm_connector *connector)
+{
+   unsigned int index = drm_connector_index(connector);
+   s64 __user *fence_ptr;
+
+   fence_ptr = state->connectors[index].out_fence_ptr;
+   state->connectors[index].out_fence_ptr = NULL;
+
+   return fence_ptr;
+}
+
 /**
  * drm_atomic_set_mode_for_crtc - set mode for CRTC
  * @state: the CRTC whose incoming state to update
@@ -696,6 +722,12 @@ static int drm_atomic_connector_check(struct drm_connector 
*connector,
return -EINVAL;
}
 
+   if (writeback_job->out_fence && !writeback_job->fb) {
+   DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence 
without framebuffer\n",
+connector->base.id, connector->name);
+   return -EINVAL;
+   }
+
return 0;
 }
 
@@ -1134,6 +1166,11 @@ int drm_atomic_connector_set_property(struct 
drm_connector *connector,
if (fb)
drm_framebuffer_unreference(fb);
return ret;
+   } else if (property == config->prop_out_fence_ptr) {
+   s64 __user *fence_ptr = u64_to_user_ptr(val);
+
+   return set_out_fence_for_connector(state->state, connector,
+  fence_ptr);
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1185,6 +1222,8 @@ static void drm_atomic_connector_print_state(struct 
drm_printer *p,
} else if (property == config->writeback_fb_id_property) {
/* Writeback framebuffer is one-shot, write and forget */
*val = 0;
+   } else if (property == config->prop_out_fence_ptr) {
+   *val = 0;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
@@ -1976,7 +2015,7 @@ static int setup_out_fence(struct drm_out_fence_state 
*fence_state,
return 0;
 }
 
-static int prepare_crtc_signaling(struct drm_device *dev,
+static int prepare_signaling(struct drm_device *dev,
  struct drm_atomic_state *state,
  struct drm_mode_atomic *arg,
  struct drm_file *file_priv,
@@ -1985,6 +2024,8 @@ static int prepare_crtc_signaling(struct drm_device *dev,
 {
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
+   struct drm_connector *conn;
+   struct drm_connector_state *conn_state;
int i, ret;
 
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
@@ -2048,14 +2089,51 @@ static int prepare_crtc_signaling(struct drm_device 
*dev,
}
}
 
+   for_each_connector_in_state(state, conn, conn_state, i) {
+   struct drm_writeback_job

[PATCH 1/6] drm: Add writeback connector type

2016-11-25 Thread Brian Starkey
Writeback connectors represent writeback engines which can write the
CRTC output to a memory framebuffer. Add a writeback connector type and
related support functions.

Drivers should initialize a writeback connector with
drm_writeback_connector_init() which takes care of setting up all the
writeback-specific details on top of the normal functionality of
drm_connector_init().

Writeback connectors have a WRITEBACK_FB_ID property, used to set the
output framebuffer, and a PIXEL_FORMATS blob used to expose the
supported writeback formats to userspace.

When a framebuffer is attached to a writeback connector with the
WRITEBACK_FB_ID property, it is used only once (for the commit in which
it was included), and userspace can never read back the value of
WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is
attached to a CRTC.

Changes since v1:
 - Added drm_writeback.c + documentation
 - Added helper to initialize writeback connector in one go
 - Added core checks
 - Squashed into a single commit
 - Dropped the client cap
 - Writeback framebuffers are no longer persistent

Changes since v2:
 Daniel Vetter:
 - Subclass drm_connector to drm_writeback_connector
 - Relax check to allow CRTC to be set without an FB
 - Add some writeback_ prefixes
 - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary
 Gustavo Padovan:
 - Add drm_writeback_job to handle writeback signalling centrally

Signed-off-by: Brian Starkey 
---
 Documentation/gpu/drm-kms.rst   |9 ++
 drivers/gpu/drm/Makefile|2 +-
 drivers/gpu/drm/drm_atomic.c|  130 
 drivers/gpu/drm/drm_atomic_helper.c |6 +
 drivers/gpu/drm/drm_connector.c |4 +-
 drivers/gpu/drm/drm_writeback.c |  230 +++
 include/drm/drm_atomic.h|3 +
 include/drm/drm_connector.h |   13 ++
 include/drm/drm_mode_config.h   |   14 +++
 include/drm/drm_writeback.h |   78 
 include/uapi/drm/drm_mode.h |1 +
 11 files changed, 488 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_writeback.c
 create mode 100644 include/drm/drm_writeback.h

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 568f3c2..3a4f35b 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -122,6 +122,15 @@ Connector Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
:export:
 
+Writeback Connectors
+
+
+.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
+  :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_writeback.c
+  :export:
+
 Encoder Abstraction
 ===
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 883f3e7..3209aa4 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_writeback.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b476ec5..343e2b7 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "drm_crtc_internal.h"
@@ -659,6 +660,46 @@ static void drm_atomic_crtc_print_state(struct drm_printer 
*p,
 }
 
 /**
+ * drm_atomic_connector_check - check connector state
+ * @connector: connector to check
+ * @state: connector state to check
+ *
+ * Provides core sanity checks for connector state.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int drm_atomic_connector_check(struct drm_connector *connector,
+   struct drm_connector_state *state)
+{
+   struct drm_crtc_state *crtc_state;
+   struct drm_writeback_job *writeback_job = state->writeback_job;
+
+   if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) ||
+   !writeback_job)
+   return 0;
+
+   if (writeback_job->fb && !state->crtc) {
+   DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] framebuffer without CRTC\n",
+connector->base.id, connector->name);
+   return -EINVAL;
+   }
+
+   if (state->crtc)
+   crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+   state->crtc);
+
+   if (writeback_job->fb && !crtc_state->active) {
+   DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] has framebuffer, but 
[CRTC:%d] is off\n",
+connector->base.id, connector->name,
+ 

[PATCH 3/6] drm: mali-dp: Rename malidp_input_format

2016-11-25 Thread Brian Starkey
We're going to use the same format list for output formats, so rename
everything related to input formats to avoid confusion.

Signed-off-by: Brian Starkey 
Reviewed-by: Liviu Dudau 
---
 drivers/gpu/drm/arm/malidp_hw.c |   24 
 drivers/gpu/drm/arm/malidp_hw.h |8 
 drivers/gpu/drm/arm/malidp_planes.c |8 
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 4bdf531..9ec6d69 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -21,7 +21,7 @@
 #include "malidp_drv.h"
 #include "malidp_hw.h"
 
-static const struct malidp_input_format malidp500_de_formats[] = {
+static const struct malidp_format_id malidp500_de_formats[] = {
/*fourcc,   layers supporting the format, internal id  */
{ DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_GRAPHICS2,  0 },
{ DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_GRAPHICS2,  1 },
@@ -69,7 +69,7 @@
{ DRM_FORMAT_NV12, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 6) },\
{ DRM_FORMAT_YUV420, DE_VIDEO1 | DE_VIDEO2, MALIDP_ID(5, 7) }
 
-static const struct malidp_input_format malidp550_de_formats[] = {
+static const struct malidp_format_id malidp550_de_formats[] = {
MALIDP_COMMON_FORMATS,
 };
 
@@ -436,8 +436,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
.irq_mask = MALIDP500_DE_IRQ_CONF_VALID,
.vsync_irq = MALIDP500_DE_IRQ_CONF_VALID,
},
-   .input_formats = malidp500_de_formats,
-   .n_input_formats = ARRAY_SIZE(malidp500_de_formats),
+   .pixel_formats = malidp500_de_formats,
+   .n_pixel_formats = ARRAY_SIZE(malidp500_de_formats),
.bus_align_bytes = 8,
},
.query_hw = malidp500_query_hw,
@@ -469,8 +469,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
},
-   .input_formats = malidp550_de_formats,
-   .n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+   .pixel_formats = malidp550_de_formats,
+   .n_pixel_formats = ARRAY_SIZE(malidp550_de_formats),
.bus_align_bytes = 8,
},
.query_hw = malidp550_query_hw,
@@ -503,8 +503,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
.irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
},
-   .input_formats = malidp550_de_formats,
-   .n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+   .pixel_formats = malidp550_de_formats,
+   .n_pixel_formats = ARRAY_SIZE(malidp550_de_formats),
.bus_align_bytes = 16,
},
.query_hw = malidp650_query_hw,
@@ -522,10 +522,10 @@ u8 malidp_hw_get_format_id(const struct malidp_hw_regmap 
*map,
 {
unsigned int i;
 
-   for (i = 0; i < map->n_input_formats; i++) {
-   if (((map->input_formats[i].layer & layer_id) == layer_id) &&
-   (map->input_formats[i].format == format))
-   return map->input_formats[i].id;
+   for (i = 0; i < map->n_pixel_formats; i++) {
+   if (((map->pixel_formats[i].layer & layer_id) == layer_id) &&
+   (map->pixel_formats[i].format == format))
+   return map->pixel_formats[i].id;
}
 
return MALIDP_INVALID_FORMAT_ID;
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 087e1202..4f8c884 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -35,7 +35,7 @@ enum {
DE_SMART = BIT(4),
 };
 
-struct malidp_input_format {
+struct malidp_format_id {
u32 format; /* DRM fourcc */
u8 layer;   /* bitmask of layers supporting it */
u8 id;  /* used internally */
@@ -85,9 +85,9 @@ struct malidp_hw_regmap {
const struct malidp_irq_map se_irq_map;
const struct malidp_irq_map dc_irq_map;
 
-   /* list of supported input formats for each layer */
-   const struct malidp_input_format *input_formats;
-   const u8 n_input_formats;
+   /* list of supported pixel formats for each layer */
+   const struct malidp_format_id *pixel_formats;
+   const u8 n_pixel_formats;
 
/* pitch alignment requirement in bytes */
const u8 bus_align_bytes;
diff

[PATCH 4/6] drm: mali-dp: Add support for writeback on DP550/DP650

2016-11-25 Thread Brian Starkey
From: Liviu Dudau 

Mali-DP display processors are able to write the composition result to a
memory buffer via the SE.

Add entry points in the HAL for enabling/disabling this feature, and
implement support for it on DP650 and DP550. DP500 acts differently and
so is omitted from this change.

Signed-off-by: Liviu Dudau 
Signed-off-by: Brian Starkey 
---
 drivers/gpu/drm/arm/malidp_hw.c   |   52 +++--
 drivers/gpu/drm/arm/malidp_hw.h   |   18 +
 drivers/gpu/drm/arm/malidp_regs.h |   15 +++
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 9ec6d69..c696e67 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -384,6 +384,48 @@ static int malidp550_rotmem_required(struct 
malidp_hw_device *hwdev, u16 w, u16
return w * bytes_per_col;
 }
 
+static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
+dma_addr_t *addrs, s32 *pitches,
+int num_planes, u16 w, u16 h, u32 fmt_id)
+{
+   u32 base = MALIDP550_SE_MEMWRITE_BASE;
+   u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+
+   /* enable the scaling engine block */
+   malidp_hw_setbits(hwdev, MALIDP_SCALE_ENGINE_EN, de_base + 
MALIDP_DE_DISPLAY_FUNC);
+
+   malidp_hw_write(hwdev, fmt_id, base + MALIDP_MW_FORMAT);
+   switch (num_planes) {
+   case 2:
+   malidp_hw_write(hwdev, lower_32_bits(addrs[1]), base + 
MALIDP_MW_P2_PTR_LOW);
+   malidp_hw_write(hwdev, upper_32_bits(addrs[1]), base + 
MALIDP_MW_P2_PTR_HIGH);
+   malidp_hw_write(hwdev, pitches[1], base + MALIDP_MW_P2_STRIDE);
+   /* fall through */
+   case 1:
+   malidp_hw_write(hwdev, lower_32_bits(addrs[0]), base + 
MALIDP_MW_P1_PTR_LOW);
+   malidp_hw_write(hwdev, upper_32_bits(addrs[0]), base + 
MALIDP_MW_P1_PTR_HIGH);
+   malidp_hw_write(hwdev, pitches[0], base + MALIDP_MW_P1_STRIDE);
+   break;
+   default:
+   WARN(1, "Invalid number of planes");
+   }
+
+   malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h),
+   MALIDP550_SE_MEMWRITE_OUT_SIZE);
+   malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | 
MALIDP_SE_MEMWRITE_EN,
+ MALIDP550_SE_CONTROL);
+
+   return 0;
+}
+
+static void malidp550_disable_memwrite(struct malidp_hw_device *hwdev)
+{
+   u32 base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+   malidp_hw_clearbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | 
MALIDP_SE_MEMWRITE_EN,
+   MALIDP550_SE_CONTROL);
+   malidp_hw_clearbits(hwdev, MALIDP_SCALE_ENGINE_EN, base + 
MALIDP_DE_DISPLAY_FUNC);
+}
+
 static int malidp650_query_hw(struct malidp_hw_device *hwdev)
 {
u32 conf = malidp_hw_read(hwdev, MALIDP550_CONFIG_ID);
@@ -466,7 +508,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
MALIDP550_SE_IRQ_AXI_ERR,
},
.dc_irq_map = {
-   .irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
+   .irq_mask = MALIDP550_DC_IRQ_CONF_VALID |
+   MALIDP550_DC_IRQ_SE,
.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
},
.pixel_formats = malidp550_de_formats,
@@ -480,6 +523,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
.set_config_valid = malidp550_set_config_valid,
.modeset = malidp550_modeset,
.rotmem_required = malidp550_rotmem_required,
+   .enable_memwrite = malidp550_enable_memwrite,
+   .disable_memwrite = malidp550_disable_memwrite,
},
[MALIDP_650] = {
.map = {
@@ -500,7 +545,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
MALIDP550_SE_IRQ_AXI_ERR,
},
.dc_irq_map = {
-   .irq_mask = MALIDP550_DC_IRQ_CONF_VALID,
+   .irq_mask = MALIDP550_DC_IRQ_CONF_VALID |
+   MALIDP550_DC_IRQ_SE,
.vsync_irq = MALIDP550_DC_IRQ_CONF_VALID,
},
.pixel_formats = malidp550_de_formats,
@@ -514,6 +560,8 @@ static int malidp650_query_hw(struct malidp_hw_device 
*hwdev)
.set_config_valid = malidp550_set_config_valid,
.modeset = malidp550_modeset,
.rotmem_required = malidp550_rotmem_required,
+   .enable_memwrite = malidp550_enable_memwrite,
+   .disable_me

[PATCH 5/6] drm: mali-dp: Add RGB writeback formats for DP550/DP650

2016-11-25 Thread Brian Starkey
Add a layer bit for the SE memory-write, and add it to the pixel format
matrix for DP550/DP650.

Signed-off-by: Brian Starkey 
---
 drivers/gpu/drm/arm/malidp_hw.c |   28 ++--
 drivers/gpu/drm/arm/malidp_hw.h |1 +
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index c696e67..be17631 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -46,20 +46,20 @@
 
 #define MALIDP_COMMON_FORMATS \
/*fourcc,   layers supporting the format,  internal id   */ \
-   { DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(0, 0) }, \
-   { DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(0, 1) }, \
-   { DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(0, 2) }, \
-   { DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(0, 3) }, \
-   { DRM_FORMAT_ARGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(1, 0) }, \
-   { DRM_FORMAT_ABGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(1, 1) }, \
-   { DRM_FORMAT_RGBA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(1, 2) }, \
-   { DRM_FORMAT_BGRA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(1, 3) }, \
-   { DRM_FORMAT_XRGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(2, 0) }, \
-   { DRM_FORMAT_XBGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(2, 1) }, \
-   { DRM_FORMAT_RGBX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(2, 2) }, \
-   { DRM_FORMAT_BGRX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, 
MALIDP_ID(2, 3) }, \
-   { DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 
0) }, \
-   { DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 
1) }, \
+   { DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(0, 0) }, \
+   { DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(0, 1) }, \
+   { DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(0, 2) }, \
+   { DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(0, 3) }, \
+   { DRM_FORMAT_ARGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(1, 0) }, \
+   { DRM_FORMAT_ABGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(1, 1) }, \
+   { DRM_FORMAT_RGBA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(1, 2) }, \
+   { DRM_FORMAT_BGRA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(1, 3) }, \
+   { DRM_FORMAT_XRGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(2, 0) }, \
+   { DRM_FORMAT_XBGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(2, 1) }, \
+   { DRM_FORMAT_RGBX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(2, 2) }, \
+   { DRM_FORMAT_BGRX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART 
| SE_MEMWRITE, MALIDP_ID(2, 3) }, \
+   { DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(3, 0) }, \
+   { DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | 
SE_MEMWRITE, MALIDP_ID(3, 1) }, \
{ DRM_FORMAT_RGBA5551, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(4, 0) }, \
{ DRM_FORMAT_ABGR1555, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, 
MALIDP_ID(4, 1) }, \
{ DRM_FORMAT_RGB565, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 
2) }, \
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 091171d..8056efa 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -33,6 +33,7 @@ enum {
DE_GRAPHICS2 = BIT(2), /* used only in DP500 */
DE_VIDEO2 = BIT(3),
DE_SMART = BIT(4),
+   SE_MEMWRITE = BIT(5),
 };
 
 struct malidp_format_id {
-- 
1.7.9.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


[RFC PATCH v3 0/6] Introduce writeback connectors

2016-11-25 Thread Brian Starkey
Hi,

This is v3 of my series introducing a new connector type:
 DRM_MODE_CONNECTOR_WRITEBACK
See v1 and v2 here: [1] [2]

Writeback connectors are used to expose the memory writeback engines
found in some display controllers, which can write a CRTC's
composition result to a memory buffer.
This is useful e.g. for testing, screen-recording, screenshots,
wireless display, display cloning, memory-to-memory composition.

Writeback connectors are given a WRITEBACK_FB_ID property (which acts
slightly differently to FB_ID, so gets a new name), as well as
a PIXEL_FORMATS blob to list the supported writeback formats, and
OUT_FENCE_PTR to be used for out-fences.

The changes since v2 are in the commit messages of each commit.

The main differences are:
 - Subclass drm_connector as drm_writeback_connector
 - Slight relaxation of core checks, to allow
   (connector->crtc && !connector->fb)
 - Dropped PIXEL_FORMATS_SIZE, which was redundant
 - Reworked the event interface, drivers don't need to deal with the
   fence directly
 - Re-ordered the commits to introduce writeback out-fences up-front.

I've kept RFC on this series because the event reporting (introduction
of drm_writeback_job) is probably up for debate.

v4 will be accompanied by igt tests.

As always, I look forward to any comments.

Thanks,
Brian

[1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1247574.html
[2] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1258017.html

---

Brian Starkey (5):
  drm: Add writeback connector type
  drm: writeback: Add out-fences for writeback connectors
  drm: mali-dp: Rename malidp_input_format
  drm: mali-dp: Add RGB writeback formats for DP550/DP650
  drm: mali-dp: Add writeback connector

Liviu Dudau (1):
  drm: mali-dp: Add support for writeback on DP550/DP650

 Documentation/gpu/drm-kms.rst   |9 +
 drivers/gpu/drm/Makefile|2 +-
 drivers/gpu/drm/arm/Makefile|1 +
 drivers/gpu/drm/arm/malidp_crtc.c   |   21 +++
 drivers/gpu/drm/arm/malidp_drv.c|   25 ++-
 drivers/gpu/drm/arm/malidp_drv.h|3 +
 drivers/gpu/drm/arm/malidp_hw.c |  111 
 drivers/gpu/drm/arm/malidp_hw.h |   27 ++-
 drivers/gpu/drm/arm/malidp_mw.c |  278 ++
 drivers/gpu/drm/arm/malidp_mw.h |   18 ++
 drivers/gpu/drm/arm/malidp_planes.c |8 +-
 drivers/gpu/drm/arm/malidp_regs.h   |   15 ++
 drivers/gpu/drm/drm_atomic.c|  226 +++-
 drivers/gpu/drm/drm_atomic_helper.c |6 +
 drivers/gpu/drm/drm_connector.c |4 +-
 drivers/gpu/drm/drm_writeback.c |  325 +++
 include/drm/drm_atomic.h|   11 ++
 include/drm/drm_connector.h |   13 ++
 include/drm/drm_mode_config.h   |   14 ++
 include/drm/drm_writeback.h |  115 +
 include/uapi/drm/drm_mode.h |1 +
 21 files changed, 1181 insertions(+), 52 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.c
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.h
 create mode 100644 drivers/gpu/drm/drm_writeback.c
 create mode 100644 include/drm/drm_writeback.h

-- 
1.7.9.5

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


[PATCH 6/6] drm: mali-dp: Add writeback connector

2016-11-25 Thread Brian Starkey
Mali-DP has a memory writeback engine which can be used to write the
composition result to a memory buffer. Expose this functionality as a
DRM writeback connector on supported hardware.

Changes since v1:
 Daniel Vetter:
 - Don't require a modeset when writeback routing changes
 - Make writeback connector always disconnected

Changes since v2:
 - Rebase onto new drm_writeback_connector
 - Add reset callback, allocating subclassed state
 Daniel Vetter:
 - Squash out-fence support into this commit
 Gustavo Padovan:
 - Don't signal fence directly from driver (and drop malidp_mw_job)

Signed-off-by: Brian Starkey 
---
 drivers/gpu/drm/arm/Makefile  |1 +
 drivers/gpu/drm/arm/malidp_crtc.c |   21 +++
 drivers/gpu/drm/arm/malidp_drv.c  |   25 +++-
 drivers/gpu/drm/arm/malidp_drv.h  |3 +
 drivers/gpu/drm/arm/malidp_hw.c   |7 +-
 drivers/gpu/drm/arm/malidp_mw.c   |  278 +
 drivers/gpu/drm/arm/malidp_mw.h   |   18 +++
 7 files changed, 348 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.c
 create mode 100644 drivers/gpu/drm/arm/malidp_mw.h

diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
index bb8b158..3bf31d1 100644
--- a/drivers/gpu/drm/arm/Makefile
+++ b/drivers/gpu/drm/arm/Makefile
@@ -1,4 +1,5 @@
 hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
 obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o
 mali-dp-y := malidp_drv.o malidp_hw.o malidp_planes.o malidp_crtc.o
+mali-dp-y += malidp_mw.o
 obj-$(CONFIG_DRM_MALI_DISPLAY) += mali-dp.o
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
b/drivers/gpu/drm/arm/malidp_crtc.c
index 08e6a71..5413a5a 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -68,6 +68,18 @@ static void malidp_crtc_enable(struct drm_crtc *crtc)
clk_set_rate(hwdev->pxlclk, crtc->state->adjusted_mode.crtc_clock * 
1000);
 
hwdev->modeset(hwdev, &vm);
+   /*
+* We should always disable the memory write when leaving config mode,
+* otherwise the hardware will start writing right away - possibly with
+* a stale config, and definitely before we've had a chance to configure
+* the planes.
+* If the memory write needs to be enabled, that will get taken care
+* of later during the atomic commit
+*/
+   if (hwdev->disable_memwrite) {
+   DRM_DEV_DEBUG_DRIVER(crtc->dev->dev, "Disable memwrite\n");
+   hwdev->disable_memwrite(hwdev);
+   }
hwdev->leave_config_mode(hwdev);
drm_crtc_vblank_on(crtc);
 }
@@ -157,6 +169,15 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
}
}
 
+   /* If only the writeback routing has changed, we don't need a modeset */
+   if (state->connectors_changed) {
+   u32 old_mask = crtc->state->connector_mask;
+   u32 new_mask = state->connector_mask;
+   if ((old_mask ^ new_mask) ==
+   (1 << drm_connector_index(&malidp->mw_connector.base)))
+   state->connectors_changed = false;
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 32f746e..2d0465b 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -28,6 +28,7 @@
 #include 
 
 #include "malidp_drv.h"
+#include "malidp_mw.h"
 #include "malidp_regs.h"
 #include "malidp_hw.h"
 
@@ -92,6 +93,14 @@ static void malidp_atomic_commit_tail(struct 
drm_atomic_state *state)
 
drm_atomic_helper_commit_modeset_disables(drm, state);
drm_atomic_helper_commit_modeset_enables(drm, state);
+
+   /*
+* The order here is important. We must configure memory-write after
+* the CRTC is already enabled, so that its configuration update is
+* gated on the next CVAL.
+*/
+   malidp_mw_atomic_commit(drm, state);
+
drm_atomic_helper_commit_planes(drm, state, 0);
 
malidp_atomic_commit_hw_done(state);
@@ -147,12 +156,20 @@ static int malidp_init(struct drm_device *drm)
drm->mode_config.helper_private = &malidp_mode_config_helpers;
 
ret = malidp_crtc_init(drm);
-   if (ret) {
-   drm_mode_config_cleanup(drm);
-   return ret;
-   }
+   if (ret)
+   goto crtc_fail;
+
+   ret = malidp_mw_connector_init(drm);
+   if (ret)
+   goto mw_fail;
 
return 0;
+
+mw_fail:
+   malidp_de_planes_destroy(drm);
+crtc_fail:
+   drm_mode_config_cleanup(drm);
+   return ret;
 }
 
 static void malidp_fini(struct drm_device *drm)
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 9fc8a2e..8abfa8a 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -13,6 +13,7 @@
 #ifndef __MALIDP_DRV_H__
 #define __MALIDP_DRV_H__
 
+#include 
 #include 
 #include 
 #include "malidp_hw.h"
@@ -22,6 +2

Re: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Logan Gunthorpe


On 25/11/16 06:06 AM, Christian König wrote:
> Well Serguei send me a couple of documents about QPI when we started to
> discuss this internally as well and that's exactly one of the cases I
> had in mind when writing this.
> 
> If I understood it correctly for such systems P2P is technical possible,
> but not necessary a good idea. Usually it is faster to just use a
> bouncing buffer when the peers are a bit "father" apart.
> 
> That this problem is solved on newer hardware is good, but doesn't helps
> us at all if we at want to support at least systems from the last five
> years or so.

Well we have been testing with Sandy Bridge, I think the problem was
supposed to be fixed in Ivy but we never tested it so I can't say what
the performance turned out to be. Ivy is nearly 5 years old. I expect
this is something that will be improved more and more with subsequent
generations.

A white list may end up being rather complicated if it has to cover
different CPU generations and system architectures. I feel this is a
decision user space could easily make.

Logan
--
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] uvcvideo: freeing an error pointer

2016-11-25 Thread Laurent Pinchart
Hi Walter,

On Friday 25 Nov 2016 15:47:49 walter harms wrote:
> Am 25.11.2016 14:57, schrieb Laurent Pinchart:
> > On Friday 25 Nov 2016 13:28:35 Dan Carpenter wrote:
> >> A recent cleanup introduced a potential dereference of -EFAULT when we
> >> call kfree(map->menu_info).
> > 
> > I should have caught that, my apologies :-(
> > 
> > Thinking a bit more about this class of problems, would the following
> > patch make sense ?
> > 
> > commit 034b71306510643f9f059249a0c14418099eb436
> > Author: Laurent Pinchart 
> > Date:   Fri Nov 25 15:54:22 2016 +0200
> > 
> > mm/slab: WARN_ON error pointers passed to kfree()
> > 
> > Passing an error pointer to kfree() is invalid and can lead to crashes
> > or memory corruption. Reject those pointers and WARN in order to catch
> > the problems and fix them in the callers.
> > 
> > Signed-off-by: Laurent Pinchart 
> > 
> > diff --git a/mm/slab.c b/mm/slab.c
> > index 0b0550ca85b4..a7eb830c6684 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -3819,6 +3819,8 @@ void kfree(const void *objp)
> > if (unlikely(ZERO_OR_NULL_PTR(objp)))
> > return;
> > 
> > +   if (WARN_ON(IS_ERR(objp)))
> > +   return;
> > local_irq_save(flags);
> > kfree_debugcheck(objp);
> > c = virt_to_cache(objp);
> 
> I this is better in kfree_debugcheck().
> 1. it has the right name
> 2. is contains already a check

Sakari Ailus (CC'ed) has expressed the opinion that we might want to go one 
step further and treat error pointers the same way we treat NULL or ZERO 
pointers today, by just returning without logging anything. The reasoning is 
that accepting a NULL pointer in kfree() was decided before we made extensive 
use of allocation APIs returning error pointers, so it could be time to update 
kfree() based on the current allocation usage patterns.

> static void kfree_debugcheck(const void *objp)
>  {
>  if (!virt_addr_valid(objp)) {
>  pr_err("kfree_debugcheck: out of range ptr %lxh\n",
> (unsigned long)objp);
>  BUG();
>  }
>   }
> 
> btw: should this read %p ?

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH 1/1] smiapp: Implement power-on and power-off sequences without runtime PM

2016-11-25 Thread Alan Stern
On Fri, 25 Nov 2016, Sakari Ailus wrote:

> Hi Alan and others,
> 
> On Thu, Nov 24, 2016 at 09:15:39PM -0500, Alan Stern wrote:
> > On Fri, 25 Nov 2016, Laurent Pinchart wrote:
> > 
> > > Dear linux-pm developers, what's the suggested way to ensure that a 
> > > runtime-
> > > pm-enabled driver can run fine on a system with CONFIG_PM disabled ?
> > 
> > The exact point of your question isn't entirely clear.  In the most 
> > literal sense, the best ways to ensure this are (1) audit the code, and 
> > (2) actually try it.
> > 
> > I have a feeling this doesn't quite answer your question, however.  :-)
> 
> The question is related to devices that require certain power-up and
> power-down sequences that are now implemented as PM runtime hooks that,
> without CONFIG_PM defined, will not be executed. Is there a better way than
> to handle this than have an implementation in the driver for the PM runtime
> and non-PM runtime case separately?

Yes, there is a better way.  For the initial power-up and final 
power-down sequences, don't rely on the PM core to invoke the 
callbacks.  Just call them directly, yourself.

For example, as part of the probe routine, instead of doing this:

pm_runtime_set_suspended(dev);
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);

Do this:

pm_runtime_set_active(dev);
pm_runtime_get_noresume(dev);
pm_runtime_enable(dev);
/*
 * In case CONFIG_PM is disabled, invoke the runtime-resume 
 * callback directly.
 */
my_runtime_resume(dev);

Alan Stern

--
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 10/10] media: camss: Add Makefiles and Kconfig files

2016-11-25 Thread Todor Tomov
Add Makefiles and Kconfig files to build the camss driver.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/Kconfig |  5 +
 drivers/media/platform/qcom/Makefile|  1 +
 drivers/media/platform/qcom/camss-8x16/Makefile | 12 
 3 files changed, 18 insertions(+)
 create mode 100644 drivers/media/platform/qcom/Kconfig
 create mode 100644 drivers/media/platform/qcom/Makefile
 create mode 100644 drivers/media/platform/qcom/camss-8x16/Makefile

diff --git a/drivers/media/platform/qcom/Kconfig 
b/drivers/media/platform/qcom/Kconfig
new file mode 100644
index 000..743ab88
--- /dev/null
+++ b/drivers/media/platform/qcom/Kconfig
@@ -0,0 +1,5 @@
+
+menuconfig VIDEO_QCOM_CAMSS
+   tristate "Qualcomm 8x16 V4L2 Camera Subsystem driver"
+   depends on ARCH_QCOM && VIDEO_V4L2
+   select VIDEOBUF2_DMA_CONTIG
diff --git a/drivers/media/platform/qcom/Makefile 
b/drivers/media/platform/qcom/Makefile
new file mode 100644
index 000..2d73819
--- /dev/null
+++ b/drivers/media/platform/qcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= camss-8x16/
diff --git a/drivers/media/platform/qcom/camss-8x16/Makefile 
b/drivers/media/platform/qcom/camss-8x16/Makefile
new file mode 100644
index 000..839e5f6
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/Makefile
@@ -0,0 +1,12 @@
+# Makefile for Qualcomm CAMSS driver
+
+ccflags-y += -Idrivers/media/platform/qcom/camss
+qcom-camss-objs += \
+   camss.o \
+   csid.o \
+   csiphy.o \
+   ispif.o \
+   vfe.o \
+   video.o \
+
+obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
-- 
1.9.1

--
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 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver

2016-11-25 Thread Todor Tomov
Add DT binding document for Qualcomm Camera subsystem driver.

Signed-off-by: Todor Tomov 
---
 .../devicetree/bindings/media/qcom,camss.txt   | 196 +
 1 file changed, 196 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt

diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt 
b/Documentation/devicetree/bindings/media/qcom,camss.txt
new file mode 100644
index 000..76ad89a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
@@ -0,0 +1,196 @@
+Qualcomm Camera Subsystem
+
+* Properties
+
+- compatible:
+   Usage: required
+   Value type: 
+   Definition: Should contain:
+   - "qcom,8x16-camss"
+- reg:
+   Usage: required
+   Value type: 
+   Definition: Register ranges as listed in the reg-names property.
+- reg-names:
+   Usage: required
+   Value type: 
+   Definition: Should contain the following entries:
+   - "csiphy0"
+   - "csiphy0_clk_mux"
+   - "csiphy1"
+   - "csiphy1_clk_mux"
+   - "csid0"
+   - "csid1"
+   - "ispif"
+   - "csi_clk_mux"
+   - "vfe0"
+- interrupts:
+   Usage: required
+   Value type: 
+   Definition: Interrupts as listed in the interrupt-names property.
+- interrupt-names:
+   Usage: required
+   Value type: 
+   Definition: Should contain the following entries:
+   - "csiphy0"
+   - "csiphy1"
+   - "csid0"
+   - "csid1"
+   - "ispif"
+   - "vfe0"
+- power-domains:
+   Usage: required
+   Value type: 
+   Definition: A phandle and power domain specifier pairs to the
+   power domain which is responsible for collapsing
+   and restoring power to the peripheral.
+- clocks:
+   Usage: required
+   Value type: 
+   Definition: A list of phandle and clock specifier pairs as listed
+   in clock-names property.
+- clock-names:
+   Usage: required
+   Value type: 
+   Definition: Should contain the following entries:
+   - "camss_top_ahb_clk"
+   - "ispif_ahb_clk"
+   - "csiphy0_timer_clk"
+   - "csiphy1_timer_clk"
+   - "csi0_ahb_clk"
+   - "csi0_clk"
+   - "csi0_phy_clk"
+   - "csi0_pix_clk"
+   - "csi0_rdi_clk"
+   - "csi1_ahb_clk"
+   - "csi1_clk"
+   - "csi1_phy_clk"
+   - "csi1_pix_clk"
+   - "csi1_rdi_clk"
+   - "camss_ahb_clk"
+   - "camss_vfe_vfe_clk"
+   - "camss_csi_vfe_clk"
+   - "iface_clk"
+   - "bus_clk"
+- vdda-supply:
+   Usage: required
+   Value type: 
+   Definition: A phandle to voltage supply for CSI2.
+- iommus:
+   Usage: required
+   Value type: 
+   Definition: A list of phandle and IOMMU specifier pairs.
+
+* Nodes
+
+- ports:
+   Usage: required
+   Definition: As described in video-interfaces.txt in same directory.
+   Properties:
+   - reg:
+   Usage: required
+   Value type: 
+   Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
+   Endpoint node properties:
+   - clock-lanes:
+   Usage: required
+   Value type: 
+   Definition: The clock lane.
+   - data-lanes:
+   Usage: required
+   Value type: 
+   Definition: An array of data lanes.
+   - qcom,settle-cnt:
+   Usage: required
+   Value type: 
+   Definition: The settle count parameter for CSI PHY.
+
+* An Example
+
+   camss: camss@1b0 {
+   compatible = "qcom,8x16-camss";
+   reg = <0x1b0ac00 0x200>,
+   <0x1b00030 0x4>,
+   <0x1b0b000 0x200>,
+   <0x1b00038 0x4>,
+   <0x1b08000 0x100>,
+   <0x1b08400 0x100>,
+   <0x1b0a000 0x500>,
+   <0x1b00020 0x10>,
+   <0x1b1 0x1000>;
+   reg-names = "csiphy0",
+   "csiphy0_clk_mux",
+   "csiphy1",
+   "csiphy1_clk_mux",
+   "csid0",
+   "csid1",
+   "ispif",
+   "csi_clk_mux",
+   "vfe0";
+   interrupts = ,
+   ,
+   ,
+   ,
+   ,
+   ;
+   interrupt-names = "csiphy0",
+   "csiphy1"

[PATCH 05/10] media: camss: Add CSID files

2016-11-25 Thread Todor Tomov
These files control the CSID modules which handle the protocol and application
layer of the CSI2 receivers.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/csid.c | 1071 +
 drivers/media/platform/qcom/camss-8x16/csid.h |   82 ++
 2 files changed, 1153 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/csid.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/csid.h

diff --git a/drivers/media/platform/qcom/camss-8x16/csid.c 
b/drivers/media/platform/qcom/camss-8x16/csid.c
new file mode 100644
index 000..c7b8b88
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/csid.c
@@ -0,0 +1,1071 @@
+/*
+ * csid.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSID Module
+ *
+ * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2015-2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "csid.h"
+#include "camss.h"
+
+#define MSM_CSID_NAME "msm_csid"
+
+#define CAMSS_CSID_HW_VERSION  0x0
+#define CAMSS_CSID_CORE_CTRL_0 0x004
+#define CAMSS_CSID_CORE_CTRL_1 0x008
+#define CAMSS_CSID_RST_CMD 0x00c
+#define CAMSS_CSID_CID_LUT_VC_n(n) (0x010 + 0x4 * (n))
+#define CAMSS_CSID_CID_n_CFG(n)(0x020 + 0x4 * (n))
+#define CAMSS_CSID_IRQ_CLEAR_CMD   0x060
+#define CAMSS_CSID_IRQ_MASK0x064
+#define CAMSS_CSID_IRQ_STATUS  0x068
+#define CAMSS_CSID_TG_CTRL 0x0a0
+#define CAMSS_CSID_TG_VC_CFG   0x0a4
+#define CAMSS_CSID_TG_VC_CFG_H_BLANKING0x3ff
+#define CAMSS_CSID_TG_VC_CFG_V_BLANKING0x7f
+#define CAMSS_CSID_TG_DT_n_CGG_0(n)(0x0ac + 0xc * (n))
+#define CAMSS_CSID_TG_DT_n_CGG_1(n)(0x0b0 + 0xc * (n))
+#define CAMSS_CSID_TG_DT_n_CGG_2(n)(0x0b4 + 0xc * (n))
+
+#define DATA_TYPE_EMBEDDED_DATA_8BIT   0x12
+#define DATA_TYPE_YUV422_8BIT  0x1e
+#define DATA_TYPE_RAW_6BIT 0x28
+#define DATA_TYPE_RAW_8BIT 0x2a
+#define DATA_TYPE_RAW_10BIT0x2b
+#define DATA_TYPE_RAW_12BIT0x2c
+
+#define DECODE_FORMAT_UNCOMPRESSED_6_BIT   0x0
+#define DECODE_FORMAT_UNCOMPRESSED_8_BIT   0x1
+#define DECODE_FORMAT_UNCOMPRESSED_10_BIT  0x2
+#define DECODE_FORMAT_UNCOMPRESSED_12_BIT  0x3
+
+#define CSID_RESET_TIMEOUT_MS 500
+
+static const struct {
+   u32 code;
+   u32 uncompressed;
+   u8 data_type;
+   u8 decode_format;
+   u8 uncompr_bpp;
+} csid_input_fmts[] = {
+   {
+   MEDIA_BUS_FMT_UYVY8_2X8,
+   MEDIA_BUS_FMT_UYVY8_2X8,
+   DATA_TYPE_YUV422_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   16
+   },
+   {
+   MEDIA_BUS_FMT_VYUY8_2X8,
+   MEDIA_BUS_FMT_VYUY8_2X8,
+   DATA_TYPE_YUV422_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   16
+   },
+   {
+   MEDIA_BUS_FMT_YUYV8_2X8,
+   MEDIA_BUS_FMT_YUYV8_2X8,
+   DATA_TYPE_YUV422_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   16
+   },
+   {
+   MEDIA_BUS_FMT_YVYU8_2X8,
+   MEDIA_BUS_FMT_YVYU8_2X8,
+   DATA_TYPE_YUV422_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   16
+   },
+   {
+   MEDIA_BUS_FMT_SBGGR8_1X8,
+   MEDIA_BUS_FMT_SBGGR8_1X8,
+   DATA_TYPE_RAW_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   8
+   },
+   {
+   MEDIA_BUS_FMT_SGBRG8_1X8,
+   MEDIA_BUS_FMT_SGBRG8_1X8,
+   DATA_TYPE_RAW_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   8
+   },
+   {
+   MEDIA_BUS_FMT_SGRBG8_1X8,
+   MEDIA_BUS_FMT_SGRBG8_1X8,
+   DATA_TYPE_RAW_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   8
+   },
+   {
+   MEDIA_BUS_FMT_SRGGB8_1X8,
+   MEDIA_BUS_FMT_SRGGB8_1X8,
+   DATA_TYPE_RAW_8BIT,
+   DECODE_FORMAT_UNCOMPRESSED_8_BIT,
+   8
+   },
+   {
+   MEDIA_BUS_FMT_SBGGR10_1X10,
+   MEDIA_BUS_FMT_SBGGR10_1X10,
+   DATA_TYPE_RAW_10BIT,
+   DECODE_FORMAT_UNCOMPRESSED_10_BIT,
+   10
+   },
+   {
+   MEDIA_BUS_FMT_SGBRG1

[PATCH 03/10] doc: media/v4l-drivers: Add Qualcomm Camera Subsystem driver document

2016-11-25 Thread Todor Tomov
Add a document to describe Qualcomm Camera Subsystem driver.

Signed-off-by: Todor Tomov 
---
 Documentation/media/v4l-drivers/index.rst  |   1 +
 Documentation/media/v4l-drivers/qcom_camss.rst | 124 +
 2 files changed, 125 insertions(+)
 create mode 100644 Documentation/media/v4l-drivers/qcom_camss.rst

diff --git a/Documentation/media/v4l-drivers/index.rst 
b/Documentation/media/v4l-drivers/index.rst
index aac566f..ba2aaeb 100644
--- a/Documentation/media/v4l-drivers/index.rst
+++ b/Documentation/media/v4l-drivers/index.rst
@@ -45,6 +45,7 @@ For more details see the file COPYING in the source 
distribution of Linux.
omap4_camera
pvrusb2
pxa_camera
+   qcom_camss
radiotrack
saa7134
sh_mobile_ceu_camera
diff --git a/Documentation/media/v4l-drivers/qcom_camss.rst 
b/Documentation/media/v4l-drivers/qcom_camss.rst
new file mode 100644
index 000..4707ea7
--- /dev/null
+++ b/Documentation/media/v4l-drivers/qcom_camss.rst
@@ -0,0 +1,124 @@
+.. include:: 
+
+Qualcomm Camera Subsystem driver
+
+
+Introduction
+
+
+This file documents the Qualcomm Camera Subsystem driver located under
+drivers/media/platform/qcom/camss-8x16.
+
+The current version of the driver supports the Camera Subsystem found on
+Qualcomm MSM8916 and APQ8016 processors.
+
+The driver implements V4L2, Media controller and V4L2 subdev interfaces.
+Camera sensor using V4L2 subdev interface in the kernel is supported.
+
+The driver is implemented using as a reference the Qualcomm Camera Subsystem
+driver for Android as found in Code Aurora [#f1]_.
+
+
+Qualcomm Camera Subsystem hardware
+--
+
+The Camera Subsystem hardware found on 8x16 processors and supported by the
+driver consists of:
+
+- 2 CSIPHY modules. They handle the Physical layer of the CSI2 receivers.
+  A separate camera sensor can be connected to each of the CSIPHY module;
+- 2 CSID (CSI Decoder) modules. They handle the Protocol and Application layer
+  of the CSI2 receivers. A CSID can decode data stream from any of the CSIPHY.
+  Each CSID also contains a TG (Test Generator) block which can generate
+  artificial input data for test purposes;
+- ISPIF (ISP Interface) module. Handles the routing of the data streams from
+  the CSIDs to the inputs of the VFE;
+- VFE (Video Front End) module. Contains a pipeline of image processing 
hardware
+  blocks. The VFE has different input interfaces. The PIX input interface feeds
+  the input data to the image processing pipeline. Three RDI input interfaces
+  bypass the image processing pipeline. The VFE also contains the AXI bus
+  interface which writes the output data to memory.
+
+
+Supported functionality
+---
+
+The current version of the driver supports:
+
+- input from camera sensor via CSIPHY;
+- generation of test input data by the TG in CSID;
+- raw dump of the input data to memory. RDI interface of VFE is supported.
+  PIX interface (ISP processing, statistics engines, resize/crop, format
+  conversion) is not supported in the current version;
+- concurrent and independent usage of two data inputs - could be camera sensors
+  and/or TG.
+
+
+Driver Architecture and Design
+--
+
+The driver implements the V4L2 subdev interface. With the goal to model the
+hardware links between the modules and to expose a clean, logical and usable
+interface, the driver is split into V4L2 sub-devices as follows:
+
+- 2 CSIPHY sub-devices - each CSIPHY is represented by a single sub-device;
+- 2 CSID sub-devices - each CSID is represented by a single sub-device;
+- 2 ISPIF sub-devices - ISPIF is represented by a number of sub-devices equal
+  to the number of CSID sub-devices;
+- 3 VFE sub-devices - VFE is represented by a number of sub-devices equal to
+  the number of RDI input interfaces.
+
+The considerations to split the driver in this particular way are as follows:
+
+- representing CSIPHY and CSID modules by a separate sub-device for each module
+  allows to model the hardware links between these modules;
+- representing VFE by a separate sub-devices for each RDI input interface 
allows
+  to use the three RDI interfaces concurently and independently as this is
+  supported by the hardware;
+- representing ISPIF by a number of sub-devices equal to the number of CSID
+  sub-devices allows to create linear media controller pipelines when using two
+  cameras simultaneously. This avoids branches in the pipelines which otherwise
+  will require a) userspace and b) media framework (e.g. power on/off
+  operations) to  make assumptions about the data flow from a sink pad to a
+  source pad on a single media entity.
+
+Each VFE sub-device is linked to a separate video device node.
+
+The complete list of the media entities (V4L2 sub-devices and video device
+nodes) is as follows:
+
+- msm_csiphy0
+- msm_csiphy1
+- msm_csid0
+- msm_csid1
+- msm_

[PATCH 07/10] media: camss: Add VFE files

2016-11-25 Thread Todor Tomov
These files control the VFE module. The VFE has different input interfaces.
The PIX input interface feeds the input data to an image processing pipeline.
Three RDI input interfaces bypass the image processing pipeline. The VFE also
contains the AXI bus interface which writes the output data to memory.

RDI interfaces are supported in this version. PIX interface is not supported.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/vfe.c | 1877 ++
 drivers/media/platform/qcom/camss-8x16/vfe.h |  112 ++
 2 files changed, 1989 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/vfe.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/vfe.h

diff --git a/drivers/media/platform/qcom/camss-8x16/vfe.c 
b/drivers/media/platform/qcom/camss-8x16/vfe.c
new file mode 100644
index 000..86fdf47
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/vfe.c
@@ -0,0 +1,1877 @@
+/*
+ * vfe.c
+ *
+ * Qualcomm MSM Camera Subsystem - VFE Module
+ *
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2015-2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vfe.h"
+#include "camss.h"
+
+#define MSM_VFE_NAME "msm_vfe"
+
+#define vfe_line_array(ptr_line)   \
+   ((const struct vfe_line (*)[]) &(ptr_line[-(ptr_line->id)]))
+
+#define to_vfe(ptr_line)   \
+   container_of(vfe_line_array(ptr_line), struct vfe_device, ptr_line)
+
+#define VFE_0_HW_VERSION   0x000
+
+#define VFE_0_GLOBAL_RESET_CMD 0x00c
+#define VFE_0_GLOBAL_RESET_CMD_CORE(1 << 0)
+#define VFE_0_GLOBAL_RESET_CMD_CAMIF   (1 << 1)
+#define VFE_0_GLOBAL_RESET_CMD_BUS (1 << 2)
+#define VFE_0_GLOBAL_RESET_CMD_BUS_BDG (1 << 3)
+#define VFE_0_GLOBAL_RESET_CMD_REGISTER(1 << 4)
+#define VFE_0_GLOBAL_RESET_CMD_TIMER   (1 << 5)
+#define VFE_0_GLOBAL_RESET_CMD_PM  (1 << 6)
+#define VFE_0_GLOBAL_RESET_CMD_BUS_MISR(1 << 7)
+#define VFE_0_GLOBAL_RESET_CMD_TESTGEN (1 << 8)
+
+#define VFE_0_IRQ_CMD  0x024
+#define VFE_0_IRQ_CMD_GLOBAL_CLEAR (1 << 0)
+
+#define VFE_0_IRQ_MASK_0   0x028
+#define VFE_0_IRQ_MASK_0_RDIn_REG_UPDATE(n)(1 << ((n) + 5))
+#define VFE_0_IRQ_MASK_0_IMAGE_MASTER_n_PING_PONG(n)   (1 << ((n) + 8))
+#define VFE_0_IRQ_MASK_0_RESET_ACK (1 << 31)
+#define VFE_0_IRQ_MASK_1   0x02c
+#define VFE_0_IRQ_MASK_1_VIOLATION (1 << 7)
+#define VFE_0_IRQ_MASK_1_BUS_BDG_HALT_ACK  (1 << 8)
+#define VFE_0_IRQ_MASK_1_IMAGE_MASTER_n_BUS_OVERFLOW(n)(1 << ((n) + 9))
+
+#define VFE_0_IRQ_CLEAR_0  0x030
+#define VFE_0_IRQ_CLEAR_1  0x034
+
+#define VFE_0_IRQ_STATUS_0 0x038
+#define VFE_0_IRQ_STATUS_0_RDIn_REG_UPDATE(n)  (1 << ((n) + 5))
+#define VFE_0_IRQ_STATUS_0_IMAGE_MASTER_n_PING_PONG(n) (1 << ((n) + 8))
+#define VFE_0_IRQ_STATUS_0_RESET_ACK   (1 << 31)
+#define VFE_0_IRQ_STATUS_1 0x03c
+#define VFE_0_IRQ_STATUS_1_BUS_BDG_HALT_ACK(1 << 8)
+
+#define VFE_0_BUS_CMD  0x4c
+#define VFE_0_BUS_CMD_Mx_RLD_CMD(x)(1 << (x))
+
+#define VFE_0_BUS_CFG  0x050
+
+#define VFE_0_BUS_XBAR_CFG_x(x)(0x58 + 0x4 * ((x) / 2))
+#define VFE_0_BUS_XBAR_CFG_x_M0_SINGLE_STREAM_SEL_SHIFT8
+#define VFE_0_BUS_XBAR_CFG_x_M_SINGLE_STREAM_SEL_VAL_RDI0  5
+#define VFE_0_BUS_XBAR_CFG_x_M_SINGLE_STREAM_SEL_VAL_RDI1  6
+#define VFE_0_BUS_XBAR_CFG_x_M_SINGLE_STREAM_SEL_VAL_RDI2  7
+
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_CFG(n) (0x06c + 0x24 * (n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_WR_PATH_SHIFT  0
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_CFG_FRM_BASED_SHIFT1
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_PING_ADDR(n)   (0x070 + 0x24 * (n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_PONG_ADDR(n)   (0x074 + 0x24 * (n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_ADDR_CFG(n)(0x078 + 0x24 * 
(n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_ADDR_CFG_FRM_DROP_PER_SHIFT2
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_ADDR_CFG_FRM_DROP_PER_MASK (0x1F << 2)
+
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_UB_CFG(n)  (0x07c + 0x24 * (n))
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_UB_CFG_OFFSET_SHIFT16
+#define VFE_0_BUS_IMAGE_MASTER_n_WR_FRAMEDROP_PATTERN(n)   \
+   

[PATCH 08/10] media: camss: Add files which handle the video device nodes

2016-11-25 Thread Todor Tomov
These files handle the video device nodes of the camss driver.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/video.c | 597 +
 drivers/media/platform/qcom/camss-8x16/video.h |  67 +++
 2 files changed, 664 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h

diff --git a/drivers/media/platform/qcom/camss-8x16/video.c 
b/drivers/media/platform/qcom/camss-8x16/video.c
new file mode 100644
index 000..0bf8ea9
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/video.c
@@ -0,0 +1,597 @@
+/*
+ * video.c
+ *
+ * Qualcomm MSM Camera Subsystem - V4L2 device node
+ *
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2015-2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+
+#include "video.h"
+#include "camss.h"
+
+/*
+ * struct format_info - ISP media bus format information
+ * @code: V4L2 media bus format code
+ * @pixelformat: V4L2 pixel format FCC identifier
+ * @bpp: Bits per pixel when stored in memory
+ */
+static const struct format_info {
+   u32 code;
+   u32 pixelformat;
+   unsigned int bpp;
+} formats[] = {
+   { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_PIX_FMT_UYVY, 16 },
+   { MEDIA_BUS_FMT_VYUY8_2X8, V4L2_PIX_FMT_VYUY, 16 },
+   { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_PIX_FMT_YUYV, 16 },
+   { MEDIA_BUS_FMT_YVYU8_2X8, V4L2_PIX_FMT_YVYU, 16 },
+   { MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8 },
+   { MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8 },
+   { MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8 },
+   { MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8 },
+   { MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_PIX_FMT_SBGGR10P, 10 },
+   { MEDIA_BUS_FMT_SGBRG10_1X10, V4L2_PIX_FMT_SGBRG10P, 10 },
+   { MEDIA_BUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10P, 10 },
+   { MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_PIX_FMT_SRGGB10P, 10 },
+   { MEDIA_BUS_FMT_SBGGR12_1X12, V4L2_PIX_FMT_SRGGB12P, 12 },
+   { MEDIA_BUS_FMT_SGBRG12_1X12, V4L2_PIX_FMT_SGBRG12P, 12 },
+   { MEDIA_BUS_FMT_SGRBG12_1X12, V4L2_PIX_FMT_SGRBG12P, 12 },
+   { MEDIA_BUS_FMT_SRGGB12_1X12, V4L2_PIX_FMT_SRGGB12P, 12 }
+};
+
+/* 
-
+ * Helper functions
+ */
+
+/*
+ * video_mbus_to_pix - Convert v4l2_mbus_framefmt to v4l2_pix_format
+ * @mbus: v4l2_mbus_framefmt format (input)
+ * @pix: v4l2_pix_format format (output)
+ *
+ * Fill the output pix structure with information from the input mbus format.
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static unsigned int video_mbus_to_pix(const struct v4l2_mbus_framefmt *mbus,
+ struct v4l2_pix_format *pix)
+{
+   unsigned int i;
+
+   memset(pix, 0, sizeof(*pix));
+   pix->width = mbus->width;
+   pix->height = mbus->height;
+
+   for (i = 0; i < ARRAY_SIZE(formats); ++i) {
+   if (formats[i].code == mbus->code)
+   break;
+   }
+
+   if (WARN_ON(i == ARRAY_SIZE(formats)))
+   return -EINVAL;
+
+   pix->pixelformat = formats[i].pixelformat;
+   pix->bytesperline = pix->width * formats[i].bpp / 8;
+   pix->bytesperline = ALIGN(pix->bytesperline, 8);
+   pix->sizeimage = pix->bytesperline * pix->height;
+   pix->colorspace = mbus->colorspace;
+   pix->field = mbus->field;
+
+   return 0;
+}
+
+static struct v4l2_subdev *video_remote_subdev(struct camss_video *video,
+  u32 *pad)
+{
+   struct media_pad *remote;
+
+   remote = media_entity_remote_pad(&video->pad);
+
+   if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
+   return NULL;
+
+   if (pad)
+   *pad = remote->index;
+
+   return media_entity_to_v4l2_subdev(remote->entity);
+}
+
+static int video_get_subdev_format(struct camss_video *video,
+  struct v4l2_format *format)
+{
+   struct v4l2_subdev_format fmt;
+   struct v4l2_subdev *subdev;
+   u32 pad;
+   int ret;
+
+   subdev = video_remote_subdev(video, &pad);
+   if (subdev == NULL)
+   return -EINVAL;
+
+   fmt.pad = pad;
+   fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+
+   ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
+   if (ret

[PATCH 09/10] media: camms: Add core files

2016-11-25 Thread Todor Tomov
These files implement the platform driver code.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/camss.c | 603 +
 drivers/media/platform/qcom/camss-8x16/camss.h |  93 
 2 files changed, 696 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/camss.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/camss.h

diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c 
b/drivers/media/platform/qcom/camss-8x16/camss.c
new file mode 100644
index 000..6d9ef1f
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/camss.c
@@ -0,0 +1,603 @@
+/*
+ * camss.c
+ *
+ * Qualcomm MSM Camera Subsystem - Core
+ *
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2015-2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "camss.h"
+
+static struct resources csiphy_res[] = {
+   /* CSIPHY0 */
+   {
+   .regulator = { NULL },
+   .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
+  "camss_ahb_clk", "csiphy0_timer_clk" },
+   .clock_rate = { 0, 0, 0, 2 },
+   .reg = { "csiphy0", "csiphy0_clk_mux" },
+   .interrupt = { "csiphy0" }
+   },
+
+   /* CSIPHY1 */
+   {
+   .regulator = { NULL },
+   .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
+  "camss_ahb_clk", "csiphy1_timer_clk" },
+   .clock_rate = { 0, 0, 0, 2 },
+   .reg = { "csiphy1", "csiphy1_clk_mux" },
+   .interrupt = { "csiphy1" }
+   }
+};
+
+static struct resources csid_res[] = {
+   /* CSID0 */
+   {
+   .regulator = { "vdda" },
+   .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
+  "csi0_ahb_clk", "camss_ahb_clk",
+  "csi0_clk", "csi0_phy_clk",
+  "csi0_pix_clk", "csi0_rdi_clk" },
+   .clock_rate = { 0, 0, 0, 0, 2, 0, 0, 0 },
+   .reg = { "csid0" },
+   .interrupt = { "csid0" }
+   },
+
+   /* CSID1 */
+   {
+   .regulator = { "vdda" },
+   .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
+  "csi1_ahb_clk", "camss_ahb_clk",
+  "csi1_clk", "csi1_phy_clk",
+  "csi1_pix_clk", "csi1_rdi_clk" },
+   .clock_rate = { 0, 0, 0, 0, 2, 0, 0, 0 },
+   .reg = { "csid1" },
+   .interrupt = { "csid1" }
+   },
+};
+
+static struct resources_ispif ispif_res = {
+   /* ISPIF */
+   .clock = { "camss_top_ahb_clk", "camss_ahb_clk", "ispif_ahb_clk",
+  "csi0_clk", "csi0_pix_clk", "csi0_rdi_clk",
+  "csi1_clk", "csi1_pix_clk", "csi1_rdi_clk" },
+   .clock_for_reset = { "camss_vfe_vfe_clk", "camss_csi_vfe_clk" },
+   .reg = { "ispif", "csi_clk_mux" },
+   .interrupt = "ispif"
+
+};
+
+static struct resources vfe_res = {
+   /* VFE0 */
+   .regulator = { NULL },
+   .clock = { "camss_top_ahb_clk", "camss_vfe_vfe_clk",
+  "camss_csi_vfe_clk", "iface_clk",
+  "bus_clk", "camss_ahb_clk" },
+   .clock_rate = { 0, 32000, 0, 0, 0, 0, 0, 0 },
+   .reg = { "vfe0" },
+   .interrupt = { "vfe0" }
+};
+
+/*
+ * camss_enable_clocks - Enable multiple clocks
+ * @nclocks: Number of clocks in clock array
+ * @clock: Clock array
+ * @dev: Device
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+int camss_enable_clocks(int nclocks, struct clk **clock, struct device *dev)
+{
+   int ret;
+   int i;
+
+   for (i = 0; i < nclocks; i++) {
+   ret = clk_prepare_enable(clock[i]);
+   if (ret) {
+   dev_err(dev, "clock enable failed\n");
+   goto error;
+   }
+   }
+
+   return 0;
+
+error:
+   for (i--; i >= 0; i--)
+   clk_disable_unprepare(clock[i]);
+
+   return ret;
+}
+
+/*
+ * camss_disable_clocks - Disable multiple clocks
+ * @nclocks: Number of clocks in clock array
+ * @clock: Clock array
+ */
+void camss_disable_clocks(int nclocks, struct clk **clock)
+{
+   int i;
+
+   for (i = nclocks - 1; i >= 0; i--)
+   clk_disable_unprepare(clock[i]);
+}
+
+/*
+ * camss_of_p

[PATCH 06/10] media: camss: Add ISPIF files

2016-11-25 Thread Todor Tomov
These files control the ISPIF module which handles the routing of the data
streams from the CSIDs to the inputs of the VFE.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/ispif.c | 1105 
 drivers/media/platform/qcom/camss-8x16/ispif.h |   85 ++
 2 files changed, 1190 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/ispif.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/ispif.h

diff --git a/drivers/media/platform/qcom/camss-8x16/ispif.c 
b/drivers/media/platform/qcom/camss-8x16/ispif.c
new file mode 100644
index 000..7d56b7d
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/ispif.c
@@ -0,0 +1,1105 @@
+/*
+ * ispif.c
+ *
+ * Qualcomm MSM Camera Subsystem - ISPIF Module
+ *
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2015-2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ispif.h"
+#include "camss.h"
+
+#define MSM_ISPIF_NAME "msm_ispif"
+
+#define ispif_line_array(ptr_line) \
+   ((const struct ispif_line (*)[]) &(ptr_line[-(ptr_line->id)]))
+
+#define to_ispif(ptr_line) \
+   container_of(ispif_line_array(ptr_line), struct ispif_device, ptr_line)
+
+#define ISPIF_RST_CMD_00x008
+#define ISPIF_IRQ_GLOBAL_CLEAR_CMD 0x01c
+#define ISPIF_VFE_m_CTRL_0(m)  (0x200 + 0x200 * (m))
+#define ISPIF_VFE_m_CTRL_0_PIX0_LINE_BUF_EN(1 << 6)
+#define ISPIF_VFE_m_IRQ_MASK_0(m)  (0x208 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_MASK_0_PIX0_ENABLE 0x1249
+#define ISPIF_VFE_m_IRQ_MASK_0_PIX0_MASK   0x1fff
+#define ISPIF_VFE_m_IRQ_MASK_0_RDI0_ENABLE 0x02492000
+#define ISPIF_VFE_m_IRQ_MASK_0_RDI0_MASK   0x03ffe000
+#define ISPIF_VFE_m_IRQ_MASK_1(m)  (0x20c + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_MASK_1_PIX1_ENABLE 0x1249
+#define ISPIF_VFE_m_IRQ_MASK_1_PIX1_MASK   0x1fff
+#define ISPIF_VFE_m_IRQ_MASK_1_RDI1_ENABLE 0x02492000
+#define ISPIF_VFE_m_IRQ_MASK_1_RDI1_MASK   0x03ffe000
+#define ISPIF_VFE_m_IRQ_MASK_2(m)  (0x210 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_MASK_2_RDI2_ENABLE 0x1249
+#define ISPIF_VFE_m_IRQ_MASK_2_RDI2_MASK   0x1fff
+#define ISPIF_VFE_m_IRQ_STATUS_0(m)(0x21c + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_STATUS_1(m)(0x220 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_STATUS_2(m)(0x224 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_CLEAR_0(m) (0x230 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_CLEAR_1(m) (0x234 + 0x200 * (m))
+#define ISPIF_VFE_m_IRQ_CLEAR_2(m) (0x238 + 0x200 * (m))
+#define ISPIF_VFE_m_INTF_INPUT_SEL(m)  (0x244 + 0x200 * (m))
+#define ISPIF_VFE_m_INTF_CMD_0(m)  (0x248 + 0x200 * (m))
+#define ISPIF_VFE_m_INTF_CMD_1(m)  (0x24c + 0x200 * (m))
+#define ISPIF_VFE_m_PIX_INTF_n_CID_MASK(m, n)  \
+   (0x254 + 0x200 * (m) + 0x4 * (n))
+#define ISPIF_VFE_m_RDI_INTF_n_CID_MASK(m, n)  \
+   (0x264 + 0x200 * (m) + 0x4 * (n))
+#define ISPIF_VFE_m_PIX_INTF_n_STATUS(m, n)\
+   (0x2c0 + 0x200 * (m) + 0x4 * (n))
+#define ISPIF_VFE_m_RDI_INTF_n_STATUS(m, n)\
+   (0x2d0 + 0x200 * (m) + 0x4 * (n))
+
+#define CSI_PIX_CLK_MUX_SEL0x000
+#define CSI_RDI_CLK_MUX_SEL0x008
+
+#define ISPIF_TIMEOUT_SLEEP_US 1000
+#define ISPIF_TIMEOUT_ALL_US   100
+#define ISPIF_RESET_TIMEOUT_MS 500
+
+enum ispif_intf_cmd {
+   CMD_DISABLE_FRAME_BOUNDARY = 0x0,
+   CMD_ENABLE_FRAME_BOUNDARY = 0x1,
+   CMD_DISABLE_IMMEDIATELY = 0x2,
+   CMD_ALL_DISABLE_IMMEDIATELY = 0x,
+   CMD_ALL_NO_CHANGE = 0x,
+};
+
+static const u32 ispif_formats[] = {
+   MEDIA_BUS_FMT_UYVY8_2X8,
+   MEDIA_BUS_FMT_VYUY8_2X8,
+   MEDIA_BUS_FMT_YUYV8_2X8,
+   MEDIA_BUS_FMT_YVYU8_2X8,
+   MEDIA_BUS_FMT_SBGGR8_1X8,
+   MEDIA_BUS_FMT_SGBRG8_1X8,
+   MEDIA_BUS_FMT_SGRBG8_1X8,
+   MEDIA_BUS_FMT_SRGGB8_1X8,
+   MEDIA_BUS_FMT_SBGGR10_1X10,
+   MEDIA_BUS_FMT_SGBRG10_1X10,
+   MEDIA_BUS_FMT_SGRBG10_1X10,
+   MEDIA_BUS_FMT_SRGGB10_1X10,
+   MEDIA_BUS_FMT_SBGGR12_1X12,
+   MEDIA_BUS_FMT_SGBRG12_1X12,
+   MEDIA_BUS_FMT_SGRBG12_1X12,
+   MEDIA_BUS_FMT_SRGGB12_1X12,
+};
+
+/*
+ * ispif_isr - ISPIF module interrupt handler
+ * @irq: Interrupt line
+ * @dev: I

[PATCH 02/10] MAINTAINERS: Add Qualcomm Camera subsystem driver

2016-11-25 Thread Todor Tomov
Add an entry for Qualcomm Camera subsystem driver.

Signed-off-by: Todor Tomov 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b8..0740aee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9971,6 +9971,14 @@ T:   git 
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
 S: Supported
 F: drivers/net/wireless/ath/ath10k/
 
+QUALCOMM CAMERA SUBSYSTEM DRIVER
+M: Todor Tomov 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/media/qcom,camss.txt
+F: Documentation/media/v4l-drivers/qcom_camss.rst
+F: drivers/media/platform/qcom/camss/
+
 QUALCOMM EMAC GIGABIT ETHERNET DRIVER
 M: Timur Tabi 
 L: net...@vger.kernel.org
-- 
1.9.1

--
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 04/10] media: camss: Add CSIPHY files

2016-11-25 Thread Todor Tomov
These files control the CSIPHY modules which are responsible for the physical
layer of the CSI2 receivers.

Signed-off-by: Todor Tomov 
---
 drivers/media/platform/qcom/camss-8x16/csiphy.c | 685 
 drivers/media/platform/qcom/camss-8x16/csiphy.h |  77 +++
 2 files changed, 762 insertions(+)
 create mode 100644 drivers/media/platform/qcom/camss-8x16/csiphy.c
 create mode 100644 drivers/media/platform/qcom/camss-8x16/csiphy.h

diff --git a/drivers/media/platform/qcom/camss-8x16/csiphy.c 
b/drivers/media/platform/qcom/camss-8x16/csiphy.c
new file mode 100644
index 000..42ec7da
--- /dev/null
+++ b/drivers/media/platform/qcom/camss-8x16/csiphy.c
@@ -0,0 +1,685 @@
+/*
+ * csiphy.c
+ *
+ * Qualcomm MSM Camera Subsystem - CSIPHY Module
+ *
+ * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "csiphy.h"
+#include "camss.h"
+
+#define MSM_CSIPHY_NAME "msm_csiphy"
+
+#define CAMSS_CSI_PHY_LNn_CFG2(n)  (0x004 + 0x40 * (n))
+#define CAMSS_CSI_PHY_LNn_CFG3(n)  (0x008 + 0x40 * (n))
+#define CAMSS_CSI_PHY_GLBL_RESET   0x140
+#define CAMSS_CSI_PHY_GLBL_PWR_CFG 0x144
+#define CAMSS_CSI_PHY_GLBL_IRQ_CMD 0x164
+#define CAMSS_CSI_PHY_HW_VERSION   0x188
+#define CAMSS_CSI_PHY_INTERRUPT_STATUSn(n) (0x18c + 0x4 * (n))
+#define CAMSS_CSI_PHY_INTERRUPT_MASKn(n)   (0x1ac + 0x4 * (n))
+#define CAMSS_CSI_PHY_INTERRUPT_CLEARn(n)  (0x1cc + 0x4 * (n))
+#define CAMSS_CSI_PHY_GLBL_T_INIT_CFG0 0x1ec
+#define CAMSS_CSI_PHY_T_WAKEUP_CFG00x1f4
+
+static const u32 csiphy_formats[] = {
+   MEDIA_BUS_FMT_UYVY8_2X8,
+   MEDIA_BUS_FMT_VYUY8_2X8,
+   MEDIA_BUS_FMT_YUYV8_2X8,
+   MEDIA_BUS_FMT_YVYU8_2X8,
+   MEDIA_BUS_FMT_SBGGR8_1X8,
+   MEDIA_BUS_FMT_SGBRG8_1X8,
+   MEDIA_BUS_FMT_SGRBG8_1X8,
+   MEDIA_BUS_FMT_SRGGB8_1X8,
+   MEDIA_BUS_FMT_SBGGR10_1X10,
+   MEDIA_BUS_FMT_SGBRG10_1X10,
+   MEDIA_BUS_FMT_SGRBG10_1X10,
+   MEDIA_BUS_FMT_SRGGB10_1X10,
+   MEDIA_BUS_FMT_SBGGR12_1X12,
+   MEDIA_BUS_FMT_SGBRG12_1X12,
+   MEDIA_BUS_FMT_SGRBG12_1X12,
+   MEDIA_BUS_FMT_SRGGB12_1X12,
+};
+
+/*
+ * csiphy_isr - CSIPHY module interrupt handler
+ * @irq: Interrupt line
+ * @dev: CSIPHY device
+ *
+ * Return IRQ_HANDLED on success
+ */
+static irqreturn_t csiphy_isr(int irq, void *dev)
+{
+   struct csiphy_device *csiphy = dev;
+   u8 i;
+
+   for (i = 0; i < 8; i++) {
+   u8 val = readl_relaxed(csiphy->base +
+  CAMSS_CSI_PHY_INTERRUPT_STATUSn(i));
+   writel_relaxed(val, csiphy->base +
+  CAMSS_CSI_PHY_INTERRUPT_CLEARn(i));
+   writel_relaxed(0x1, csiphy->base + CAMSS_CSI_PHY_GLBL_IRQ_CMD);
+   writel_relaxed(0x0, csiphy->base + CAMSS_CSI_PHY_GLBL_IRQ_CMD);
+   writel_relaxed(0x0, csiphy->base +
+  CAMSS_CSI_PHY_INTERRUPT_CLEARn(i));
+   }
+
+   return IRQ_HANDLED;
+}
+
+/*
+ * csiphy_reset - Perform software reset on CSIPHY module
+ * @csiphy: CSIPHY device
+ */
+static void csiphy_reset(struct csiphy_device *csiphy)
+{
+   writel_relaxed(0x1, csiphy->base + CAMSS_CSI_PHY_GLBL_RESET);
+   usleep_range(5000, 8000);
+   writel_relaxed(0x0, csiphy->base + CAMSS_CSI_PHY_GLBL_RESET);
+}
+
+/*
+ * csiphy_set_power - Power on/off CSIPHY module
+ * @sd: CSIPHY V4L2 subdevice
+ * @on: Requested power state
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+static int csiphy_set_power(struct v4l2_subdev *sd, int on)
+{
+   struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
+   struct device *dev = to_device_index(csiphy, csiphy->id);
+   int ret;
+
+   if (on) {
+   u8 hw_version;
+
+   ret = camss_enable_clocks(csiphy->nclocks, csiphy->clock, dev);
+   if (ret < 0)
+   return ret;
+
+   enable_irq(csiphy->irq);
+
+   csiphy_reset(csiphy);
+
+   hw_version = readl_relaxed(csiphy->base +
+  CAMSS_CSI_PHY_HW_VERSION);
+   dev_dbg(dev, "CSIPHY HW Version = 0x%02x\n", hw_version);
+   } else {
+   disable_irq(csiphy->irq);
+
+   camss_disable_clocks(csiphy->nclocks, csiphy

[PATCH 00/10] Qualcomm 8x16 Camera Subsystem driver

2016-11-25 Thread Todor Tomov
This patchset adds basic support for the Qualcomm Camera Subsystem found
on Qualcomm MSM8916 and APQ8016 processors.

The driver implements V4L2, Media controller and V4L2 subdev interfaces.
Camera sensor using V4L2 subdev interface in the kernel is supported.

The driver is implemented using as a reference the Qualcomm Camera
Subsystem driver for Android as found in Code Aurora [1].

The driver supports raw dump of the input data to memory. ISP processing
is not supported.

The driver is tested on Dragonboard 410C (APQ8016) with one and two
OV5645 camera sensors. media-ctl [2] and yavta [3] applications were
used for testing. Also Gstreamer 1.4.4 with v4l2src plugin is supported.

More information is present in the document added by the third patch.


The patchset depends on:
v4l: Add packed Bayer raw12 pixel formats [4]
media: venus: enable building of Venus video codec driver [5]


V4L2 compliance test result:

root@linaro-alip:~/v4l-utils/utils/v4l2-compliance# ./v4l2-compliance -s -d 
/dev/video0
v4l2-compliance SHA   : 6a760145f1a6809591a1cb17ee1b06913e4fddd1

Driver Info:
Driver name   : qcom-camss
Card type : Qualcomm Camera Subsystem
Bus info  : platform:qcom-camss
Driver version: 4.9.0
Capabilities  : 0x8421
Video Capture
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0421
Video Capture
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK (Not Supported)

Test input 0:

Streaming ioctls:
test read/write: OK (Not Supported)
test MMAP: OK 
test USERPTR: OK (Not Supported)
test DMABUF: OK (Not Supported)


Total: 47, Succeeded: 47, Failed: 0, Warnings: 0


[1] https://source.codeaurora.org/quic/la/kernel/msm-3.10/
[2] https://git.linuxtv.org//v4l-utils.git
[3] http://git.ideasonboard.org/yavta.git
[4] http://www.spinics.net/lists/linux-media/msg107494.html
[5] http://www.spinics.net/lists/linux-media/msg104013.html


Todor Tomov (10):
  doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
  MAINTAINERS: Add Qualcomm Camera subsystem driver
  doc: media/v4l-drivers: 

Re: [patch] [media] uvcvideo: freeing an error pointer

2016-11-25 Thread walter harms


Am 25.11.2016 14:57, schrieb Laurent Pinchart:
> Hi Dan,
> 
> Thank you for the patch.
> 
> On Friday 25 Nov 2016 13:28:35 Dan Carpenter wrote:
>> A recent cleanup introduced a potential dereference of -EFAULT when we
>> call kfree(map->menu_info).
> 
> I should have caught that, my apologies :-(
> 
> Thinking a bit more about this class of problems, would the following patch 
> make sense ?
> 
> commit 034b71306510643f9f059249a0c14418099eb436
> Author: Laurent Pinchart 
> Date:   Fri Nov 25 15:54:22 2016 +0200
> 
> mm/slab: WARN_ON error pointers passed to kfree()
> 
> Passing an error pointer to kfree() is invalid and can lead to crashes
> or memory corruption. Reject those pointers and WARN in order to catch
> the problems and fix them in the callers.
> 
> Signed-off-by: Laurent Pinchart 
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 0b0550ca85b4..a7eb830c6684 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3819,6 +3819,8 @@ void kfree(const void *objp)
>  
>   if (unlikely(ZERO_OR_NULL_PTR(objp)))
>   return;
> + if (WARN_ON(IS_ERR(objp)))
> + return;
>   local_irq_save(flags);
>   kfree_debugcheck(objp);
>   c = virt_to_cache(objp);


I this is better in kfree_debugcheck().
1. it has the right name
2. is contains already a check

static void kfree_debugcheck(const void *objp)
 {
 if (!virt_addr_valid(objp)) {
 pr_err("kfree_debugcheck: out of range ptr %lxh\n",
(unsigned long)objp);
 BUG();
 }
  }

btw: should this read %p ?

re,
 wh



>> Fixes: 4cc5bed1caeb ("[media] uvcvideo: Use memdup_user() rather than
>> duplicating its implementation")
>> Signed-off-by: Dan Carpenter 
> 
> Reviewed-by: Laurent Pinchart 
> 
> Mauro, the bug is present in your branch only at the moment and queued for 
> v4.10. Could you please pick this patch up as well ?
> 
>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> b/drivers/media/usb/uvc/uvc_v4l2.c index a7e12fd..3e7e283 100644
>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -66,14 +66,14 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
>> *chain, if (xmap->menu_count == 0 ||
>>  xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) {
>>  ret = -EINVAL;
>> -goto done;
>> +goto free_map;
>>  }
>>
>>  size = xmap->menu_count * sizeof(*map->menu_info);
>>  map->menu_info = memdup_user(xmap->menu_info, size);
>>  if (IS_ERR(map->menu_info)) {
>>  ret = PTR_ERR(map->menu_info);
>> -goto done;
>> +goto free_map;
>>  }
>>
>>  map->menu_count = xmap->menu_count;
>> @@ -83,13 +83,13 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
>> *chain, uvc_trace(UVC_TRACE_CONTROL, "Unsupported V4L2 control type "
>>"%u.\n", xmap->v4l2_type);
>>  ret = -ENOTTY;
>> -goto done;
>> +goto free_map;
>>  }
>>
>>  ret = uvc_ctrl_add_mapping(chain, map);
>>
>> -done:
>>  kfree(map->menu_info);
>> +free_map:
>>  kfree(map);
>>
>>  return ret;
> 
--
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: Platform: Omap3isp: Do not forget to call

2016-11-25 Thread Laurent Pinchart
Hi Shailendra,

Thank you for the patch.

On Friday 25 Nov 2016 10:14:32 Shailendra Verma wrote:
> v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition
> before returing from the function.
> 
> Signed-off-by: Shailendra Verma 
> ---
>  drivers/media/platform/omap3isp/ispvideo.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index 7354469..2822e2f 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1350,6 +1350,7 @@ static int isp_video_open(struct file *file)
>  done:
>   if (ret < 0) {
>   v4l2_fh_del(&handle->vfh);
> + v4l2_fh_exit(&handle->vfh);

While at it you should call v4l2_fh_exit() in the isp_video_release() function 
as well. I propose updating the commit message to

v4l: omap3isp: Clean up file handle in open() and release()

Both functions initialize the file handle with v4l2_fh_init() and thus
need to call clean up with v4l2_fh_exit() as appropriate. Fix it.

Same comment for the OMAP4 ISS patches you've submitted.

>   kfree(handle);
>   }

-- 
Regards,

Laurent Pinchart

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


Re: [patch] [media] uvcvideo: freeing an error pointer

2016-11-25 Thread Laurent Pinchart
Hi Dan,

Thank you for the patch.

On Friday 25 Nov 2016 13:28:35 Dan Carpenter wrote:
> A recent cleanup introduced a potential dereference of -EFAULT when we
> call kfree(map->menu_info).

I should have caught that, my apologies :-(

Thinking a bit more about this class of problems, would the following patch 
make sense ?

commit 034b71306510643f9f059249a0c14418099eb436
Author: Laurent Pinchart 
Date:   Fri Nov 25 15:54:22 2016 +0200

mm/slab: WARN_ON error pointers passed to kfree()

Passing an error pointer to kfree() is invalid and can lead to crashes
or memory corruption. Reject those pointers and WARN in order to catch
the problems and fix them in the callers.

Signed-off-by: Laurent Pinchart 

diff --git a/mm/slab.c b/mm/slab.c
index 0b0550ca85b4..a7eb830c6684 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3819,6 +3819,8 @@ void kfree(const void *objp)
 
if (unlikely(ZERO_OR_NULL_PTR(objp)))
return;
+   if (WARN_ON(IS_ERR(objp)))
+   return;
local_irq_save(flags);
kfree_debugcheck(objp);
c = virt_to_cache(objp);

> Fixes: 4cc5bed1caeb ("[media] uvcvideo: Use memdup_user() rather than
> duplicating its implementation")
> Signed-off-by: Dan Carpenter 

Reviewed-by: Laurent Pinchart 

Mauro, the bug is present in your branch only at the moment and queued for 
v4.10. Could you please pick this patch up as well ?

> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index a7e12fd..3e7e283 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -66,14 +66,14 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
> *chain, if (xmap->menu_count == 0 ||
>   xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) {
>   ret = -EINVAL;
> - goto done;
> + goto free_map;
>   }
> 
>   size = xmap->menu_count * sizeof(*map->menu_info);
>   map->menu_info = memdup_user(xmap->menu_info, size);
>   if (IS_ERR(map->menu_info)) {
>   ret = PTR_ERR(map->menu_info);
> - goto done;
> + goto free_map;
>   }
> 
>   map->menu_count = xmap->menu_count;
> @@ -83,13 +83,13 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain
> *chain, uvc_trace(UVC_TRACE_CONTROL, "Unsupported V4L2 control type "
> "%u.\n", xmap->v4l2_type);
>   ret = -ENOTTY;
> - goto done;
> + goto free_map;
>   }
> 
>   ret = uvc_ctrl_add_mapping(chain, map);
> 
> -done:
>   kfree(map->menu_info);
> +free_map:
>   kfree(map);
> 
>   return ret;

-- 
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 5/5] media: entity: Add debug information to graph walk

2016-11-25 Thread Sakari Ailus
Use dev_dbg() to tell about the progress of the graph traversal algorithm.
This is intended to make debugging of the algorithm easier.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-entity.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e242ead..186906b 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -335,6 +335,8 @@ void media_graph_walk_start(struct media_graph *graph,
graph->top = 0;
graph->stack[graph->top].entity = NULL;
stack_push(graph, entity);
+   dev_dbg(entity->graph_obj.mdev->dev,
+   "begin graph walk at \"%s\"\n", entity->name);
 }
 EXPORT_SYMBOL_GPL(media_graph_walk_start);
 
@@ -349,6 +351,10 @@ static void graph_walk_iter(struct media_graph *graph)
/* The link is not enabled so we do not follow. */
if (!(link->flags & MEDIA_LNK_FL_ENABLED)) {
link_top(graph) = link_top(graph)->next;
+   dev_dbg(entity->graph_obj.mdev->dev,
+   "walk: skipping disabled link \"%s\":%u -> \"%s\":%u\n",
+   link->source->entity->name, link->source->index,
+   link->sink->entity->name, link->sink->index);
return;
}
 
@@ -358,16 +364,23 @@ static void graph_walk_iter(struct media_graph *graph)
/* Has the entity already been visited? */
if (media_entity_enum_test_and_set(&graph->ent_enum, next)) {
link_top(graph) = link_top(graph)->next;
+   dev_dbg(entity->graph_obj.mdev->dev,
+   "walk: skipping entity \"%s\" (already seen)\n",
+   next->name);
return;
}
 
/* Push the new entity to stack and start over. */
link_top(graph) = link_top(graph)->next;
stack_push(graph, next);
+   dev_dbg(entity->graph_obj.mdev->dev, "walk: pushing \"%s\" on stack\n",
+   next->name);
 }
 
 struct media_entity *media_graph_walk_next(struct media_graph *graph)
 {
+   struct media_entity *entity;
+
if (stack_top(graph) == NULL)
return NULL;
 
@@ -379,7 +392,11 @@ struct media_entity *media_graph_walk_next(struct 
media_graph *graph)
while (link_top(graph) != &stack_top(graph)->links)
graph_walk_iter(graph);
 
-   return stack_pop(graph);
+   entity = stack_pop(graph);
+   dev_dbg(entity->graph_obj.mdev->dev,
+   "walk: returning entity \"%s\"\n", entity->name);
+
+   return entity;
 }
 EXPORT_SYMBOL_GPL(media_graph_walk_next);
 
-- 
2.1.4

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


[PATCH 2/5] media: entity: Be vocal about failing sanity checks

2016-11-25 Thread Sakari Ailus
Commit 3801bc7d1b8d ("[media] media: Media Controller fix to not let
stream_count go negative") added a sanity check for negative stream_count,
but a failure of the check remained silent. Make sure the failure is
noticed.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-entity.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index da46706..82dd0bc 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -483,8 +483,8 @@ __must_check int __media_entity_pipeline_start(struct 
media_entity *entity,
media_entity_graph_walk_start(graph, entity_err);
 
while ((entity_err = media_entity_graph_walk_next(graph))) {
-   /* don't let the stream_count go negative */
-   if (entity_err->stream_count > 0) {
+   /* Sanity check for negative stream_count */
+   if (!WARN_ON_ONCE(entity_err->stream_count <= 0)) {
entity_err->stream_count--;
if (entity_err->stream_count == 0)
entity_err->pipe = NULL;
@@ -529,8 +529,8 @@ void __media_entity_pipeline_stop(struct media_entity 
*entity)
media_entity_graph_walk_start(graph, entity);
 
while ((entity = media_entity_graph_walk_next(graph))) {
-   /* don't let the stream_count go negative */
-   if (entity->stream_count > 0) {
+   /* Sanity check for negative stream_count */
+   if (!WARN_ON_ONCE(entity->stream_count <= 0)) {
entity->stream_count--;
if (entity->stream_count == 0)
entity->pipe = NULL;
-- 
2.1.4

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


[PATCH 3/5] media: Rename graph and pipeline structs and functions

2016-11-25 Thread Sakari Ailus
The media_entity_pipeline_start() and media_entity_pipeline_stop()
functions are renamed as media_pipeline_start() and media_pipeline_stop(),
respectively. The reason is two-fold: the pipeline struct is, rightly,
already called media_pipeline (rather than media_entity_pipeline) and what
this really is about is a pipeline. A pipeline consists of entities ---
and, well, other objects embedded in these entities.

As the pipeline object will be in the future moved from entities to pads
in order to support multiple pipelines through a single entity, do the
renaming now.

Similarly, functions operating on struct media_entity_graph as well as the
struct itself are renamed by dropping the "entity_" part from the prefix
of the function family and the data structure. The graph traversal which
is what the functions are about is not specifically about entities only
and will operate on pads for the same reason as the media pipeline.

The patch has been generated using the following command:

git grep -l media_entity |xargs perl -i -pe '
s/media_entity_pipeline/media_pipeline/g;
s/media_entity_graph/media_graph/g'

And a few manual edits related to line start alignment and line wrapping.

Signed-off-by: Sakari Ailus 
---
 Documentation/media/kapi/mc-core.rst   | 18 ++---
 drivers/media/media-device.c   |  8 +--
 drivers/media/media-entity.c   | 77 +++---
 drivers/media/platform/exynos4-is/fimc-capture.c   |  8 +--
 drivers/media/platform/exynos4-is/fimc-isp-video.c |  8 +--
 drivers/media/platform/exynos4-is/fimc-lite.c  |  8 +--
 drivers/media/platform/exynos4-is/media-dev.c  | 16 ++---
 drivers/media/platform/exynos4-is/media-dev.h  |  2 +-
 drivers/media/platform/omap3isp/ispvideo.c | 16 ++---
 drivers/media/platform/s3c-camif/camif-capture.c   |  6 +-
 drivers/media/platform/vsp1/vsp1_drm.c |  4 +-
 drivers/media/platform/vsp1/vsp1_video.c   | 16 ++---
 drivers/media/platform/xilinx/xilinx-dma.c | 16 ++---
 drivers/media/usb/au0828/au0828-core.c |  4 +-
 drivers/media/v4l2-core/v4l2-mc.c  | 18 ++---
 drivers/staging/media/davinci_vpfe/vpfe_video.c| 25 ---
 drivers/staging/media/davinci_vpfe/vpfe_video.h|  2 +-
 drivers/staging/media/omap4iss/iss_video.c | 32 -
 include/media/media-device.h   |  2 +-
 include/media/media-entity.h   | 65 +-
 20 files changed, 174 insertions(+), 177 deletions(-)

diff --git a/Documentation/media/kapi/mc-core.rst 
b/Documentation/media/kapi/mc-core.rst
index 1a738e5..0c05503 100644
--- a/Documentation/media/kapi/mc-core.rst
+++ b/Documentation/media/kapi/mc-core.rst
@@ -162,13 +162,13 @@ framework provides a depth-first graph traversal API for 
that purpose.
currently defined as 16.
 
 Drivers initiate a graph traversal by calling
-:c:func:`media_entity_graph_walk_start()`
+:c:func:`media_graph_walk_start()`
 
 The graph structure, provided by the caller, is initialized to start graph
 traversal at the given entity.
 
 Drivers can then retrieve the next entity by calling
-:c:func:`media_entity_graph_walk_next()`
+:c:func:`media_graph_walk_next()`
 
 When the graph traversal is complete the function will return ``NULL``.
 
@@ -206,7 +206,7 @@ Pipelines and media streams
 
 When starting streaming, drivers must notify all entities in the pipeline to
 prevent link states from being modified during streaming by calling
-:c:func:`media_entity_pipeline_start()`.
+:c:func:`media_pipeline_start()`.
 
 The function will mark all entities connected to the given entity through
 enabled links, either directly or indirectly, as streaming.
@@ -218,17 +218,17 @@ in higher-level pipeline structures and can then access 
the
 pipeline through the struct :c:type:`media_entity`
 pipe field.
 
-Calls to :c:func:`media_entity_pipeline_start()` can be nested.
+Calls to :c:func:`media_pipeline_start()` can be nested.
 The pipeline pointer must be identical for all nested calls to the function.
 
-:c:func:`media_entity_pipeline_start()` may return an error. In that case,
+:c:func:`media_pipeline_start()` may return an error. In that case,
 it will clean up any of the changes it did by itself.
 
 When stopping the stream, drivers must notify the entities with
-:c:func:`media_entity_pipeline_stop()`.
+:c:func:`media_pipeline_stop()`.
 
-If multiple calls to :c:func:`media_entity_pipeline_start()` have been
-made the same number of :c:func:`media_entity_pipeline_stop()` calls
+If multiple calls to :c:func:`media_pipeline_start()` have been
+made the same number of :c:func:`media_pipeline_stop()` calls
 are required to stop streaming.
 The :c:type:`media_entity`.\ ``pipe`` field is reset to ``NULL`` on the last
 nested stop call.
@@ -245,7 +245,7 @@ operation must be done with the media_device graph_mutex 
held.
 Link validation
 ^^^
 
-Link validation is perf

[PATCH 4/5] media: entity: Split graph walk iteration into two functions

2016-11-25 Thread Sakari Ailus
With media_entity_graph_walk_next() getting more and more complicated (and
especially so with has_routing() support added), split the function into
two.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-entity.c | 56 
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2bddebb..e242ead 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -338,6 +338,34 @@ void media_graph_walk_start(struct media_graph *graph,
 }
 EXPORT_SYMBOL_GPL(media_graph_walk_start);
 
+static void graph_walk_iter(struct media_graph *graph)
+{
+   struct media_entity *entity = stack_top(graph);
+   struct media_link *link;
+   struct media_entity *next;
+
+   link = list_entry(link_top(graph), typeof(*link), list);
+
+   /* The link is not enabled so we do not follow. */
+   if (!(link->flags & MEDIA_LNK_FL_ENABLED)) {
+   link_top(graph) = link_top(graph)->next;
+   return;
+   }
+
+   /* Get the entity in the other end of the link . */
+   next = media_entity_other(entity, link);
+
+   /* Has the entity already been visited? */
+   if (media_entity_enum_test_and_set(&graph->ent_enum, next)) {
+   link_top(graph) = link_top(graph)->next;
+   return;
+   }
+
+   /* Push the new entity to stack and start over. */
+   link_top(graph) = link_top(graph)->next;
+   stack_push(graph, next);
+}
+
 struct media_entity *media_graph_walk_next(struct media_graph *graph)
 {
if (stack_top(graph) == NULL)
@@ -348,32 +376,8 @@ struct media_entity *media_graph_walk_next(struct 
media_graph *graph)
 * top of the stack until no more entities on the level can be
 * found.
 */
-   while (link_top(graph) != &stack_top(graph)->links) {
-   struct media_entity *entity = stack_top(graph);
-   struct media_link *link;
-   struct media_entity *next;
-
-   link = list_entry(link_top(graph), typeof(*link), list);
-
-   /* The link is not enabled so we do not follow. */
-   if (!(link->flags & MEDIA_LNK_FL_ENABLED)) {
-   link_top(graph) = link_top(graph)->next;
-   continue;
-   }
-
-   /* Get the entity in the other end of the link . */
-   next = media_entity_other(entity, link);
-
-   /* Has the entity already been visited? */
-   if (media_entity_enum_test_and_set(&graph->ent_enum, next)) {
-   link_top(graph) = link_top(graph)->next;
-   continue;
-   }
-
-   /* Push the new entity to stack and start over. */
-   link_top(graph) = link_top(graph)->next;
-   stack_push(graph, next);
-   }
+   while (link_top(graph) != &stack_top(graph)->links)
+   graph_walk_iter(graph);
 
return stack_pop(graph);
 }
-- 
2.1.4

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


[PATCH 0/5] Media pipeline and graph walk cleanups and fixes

2016-11-25 Thread Sakari Ailus
Hi folks,

This patchset contains a few cleanups and fixes for graph traversal and
pipeline starting and stopping.

The set prepares for further routing changes without still making any of
them. I'll post further patches on routing in the near future.

Niklas: these go on top of your two patches in your series. Some of the
later patches in the series will conflict with the graph walk / pipeline
interface rename. I think it'd be ideal to have a single pull request to
contain them all when it's been all reviewed. What do you think?

-- 
Kind regards,
Sakari

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


[PATCH 1/5] media: entity: Fix stream count check

2016-11-25 Thread Sakari Ailus
There's a sanity check for the stream count remaining positive or zero on
error path, but instead of performing the check on the traversed entity it
is performed on the entity where traversal ends. Fix this.

Fixes: commit 3801bc7d1b8d ("[media] media: Media Controller fix to not let 
stream_count go negative")
Signed-off-by: Sakari Ailus 
---
 drivers/media/media-entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 58d9fa6..da46706 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -484,7 +484,7 @@ __must_check int __media_entity_pipeline_start(struct 
media_entity *entity,
 
while ((entity_err = media_entity_graph_walk_next(graph))) {
/* don't let the stream_count go negative */
-   if (entity->stream_count > 0) {
+   if (entity_err->stream_count > 0) {
entity_err->stream_count--;
if (entity_err->stream_count == 0)
entity_err->pipe = NULL;
-- 
2.1.4

--
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] uvcvideo: freeing an error pointer

2016-11-25 Thread SF Markus Elfring
> A recent cleanup introduced a potential dereference of -EFAULT when we
> call kfree(map->menu_info).
> 
> Fixes: 4cc5bed1caeb ("[media] uvcvideo: Use memdup_user() rather than 
> duplicating its implementation")
> Signed-off-by: Dan Carpenter 

Thanks for your information.


> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c 
> b/drivers/media/usb/uvc/uvc_v4l2.c
> index a7e12fd..3e7e283 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -66,14 +66,14 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain 
> *chain,
>   if (xmap->menu_count == 0 ||
>   xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) {
>   ret = -EINVAL;
> - goto done;
> + goto free_map;
>   }
>  
>   size = xmap->menu_count * sizeof(*map->menu_info);
>   map->menu_info = memdup_user(xmap->menu_info, size);
>   if (IS_ERR(map->menu_info)) {
>   ret = PTR_ERR(map->menu_info);
> - goto done;
> + goto free_map;
>   }
>  
>   map->menu_count = xmap->menu_count;
> @@ -83,13 +83,13 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain 
> *chain,
>   uvc_trace(UVC_TRACE_CONTROL, "Unsupported V4L2 control type "
> "%u.\n", xmap->v4l2_type);
>   ret = -ENOTTY;
> - goto done;
> + goto free_map;
>   }
>  
>   ret = uvc_ctrl_add_mapping(chain, map);
>  
> -done:
>   kfree(map->menu_info);
> +free_map:
>   kfree(map);
>  
>   return ret;
> 

Did your update suggestion become also relevant just because the corresponding
update step “[PATCH 2/2] uvc_v4l2: One function call less in 
uvc_ioctl_ctrl_map()
after error detection” which I offered as another change possibility on 
2016-08-19
was rejected on 2016-11-22?

https://patchwork.linuxtv.org/patch/36528/
https://patchwork.kernel.org/patch/9289897/
https://lkml.kernel.org/r/<8f89ec37-1556-4c09-f0b7-df87b4169...@users.sourceforge.net>

Regards,
Markus
--
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: platform: vsp1: - Do not forget to call

2016-11-25 Thread Laurent Pinchart
Hi Shailendra,

Thank you for the patch.

The subject line is missing something (and has an extra -), I would phrase it 
as

"v4l: vsp1: Clean up file handle in open() error path"

(Mauro's scripts will add the "[media]" prefix when applying, so there's no 
need to add it manually)

The same comment holds for all other patches in this series.

On Friday 25 Nov 2016 10:37:57 Shailendra Verma wrote:
> v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition
> before returing from the function.
> 
> Signed-off-by: Shailendra Verma 
> ---
>  drivers/media/platform/vsp1/vsp1_video.c |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index d351b9c..cc58163 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -1044,6 +1044,7 @@ static int vsp1_video_open(struct file *file)
>   ret = vsp1_device_get(video->vsp1);
>   if (ret < 0) {
>   v4l2_fh_del(vfh);
> + v4l2_fh_exit(vfh);
>   kfree(vfh);
>   }

-- 
Regards,

Laurent Pinchart

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


Re: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Christian König

Am 24.11.2016 um 18:55 schrieb Logan Gunthorpe:

Hey,

On 24/11/16 02:45 AM, Christian König wrote:

E.g. it can happen that PCI device A exports it's BAR using ZONE_DEVICE.
Not PCI device B (a SATA device) can directly read/write to it because
it is on the same bus segment, but PCI device C (a network card for
example) can't because it is on a different bus segment and the bridge
can't handle P2P transactions.

Yeah, that could be an issue but in our experience we have yet to see
it. We've tested with two separate PCI buses on different CPUs connected
through QPI links and it works fine. (It is rather slow but I understand
Intel has improved the bottleneck in newer CPUs than the ones we tested.)


Well Serguei send me a couple of documents about QPI when we started to 
discuss this internally as well and that's exactly one of the cases I 
had in mind when writing this.


If I understood it correctly for such systems P2P is technical possible, 
but not necessary a good idea. Usually it is faster to just use a 
bouncing buffer when the peers are a bit "father" apart.


That this problem is solved on newer hardware is good, but doesn't helps 
us at all if we at want to support at least systems from the last five 
years or so.



It may just be older hardware that has this issue. I expect that as long
as a failed transfer can be handled gracefully by the initiator I don't
see a need to predetermine whether a device can see another devices memory.


I don't want to predetermine whether a device can see another devices 
memory at get_user_pages() time.


My thinking was more going into the direction of a whitelist to figure 
out during dma_map_single()/dma_map_sg() time if we should use a 
bouncing buffer or not.


Christian.




Logan



--
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: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Christian König

Am 24.11.2016 um 17:42 schrieb Jason Gunthorpe:

On Wed, Nov 23, 2016 at 06:25:21PM -0700, Logan Gunthorpe wrote:


On 23/11/16 02:55 PM, Jason Gunthorpe wrote:

Only ODP hardware allows changing the DMA address on the fly, and it
works at the page table level. We do not need special handling for
RDMA.

I am aware of ODP but, noted by others, it doesn't provide a general
solution to the points above.

How do you mean?

I was only saying it wasn't general in that it wouldn't work for IB
hardware that doesn't support ODP or other hardware  that doesn't do
similar things (like an NVMe drive).

There are three cases to worry about:
  - Coherent long lived page table mirroring (RDMA ODP MR)
  - Non-coherent long lived page table mirroring (RDMA MR)
  - Short lived DMA mapping (everything else)

Like you say below we have to handle short lived in the usual way, and
that covers basically every device except IB MRs, including the
command queue on a NVMe drive.


Well a problem which wasn't mentioned so far is that while GPUs do have 
a page table to mirror the CPU page table, they usually can't recover 
from page faults.


So what we do is making sure that all memory accessed by the GPU Jobs 
stays in place while those jobs run (pretty much the same pinning you do 
for the DMA).


But since this can lock down huge amounts of memory the whole command 
submission to GPUs is bound to the memory management. So when to much 
memory would get blocked by the GPU we block further command submissions 
until the situation resolves.



any complex allocators (GPU or otherwise) should respect that. And that
seems like it should be the default way most of this works -- and I
think it wouldn't actually take too much effort to make it all work now
as is. (Our iopmem work is actually quite small and simple.)

Yes, absolutely, some kind of page pinning like locking is a hard
requirement.


Yeah, we've had RDMA and O_DIRECT transfers to PCIe backed ZONE_DEVICE
memory working for some time. I'd say it's a good fit. The main question
we've had is how to expose PCIe bars to userspace to be used as MRs and
such.

Is there any progress on that?

I still don't quite get what iopmem was about.. I thought the
objection to uncachable ZONE_DEVICE & DAX made sense, so running DAX
over iopmem and still ending up with uncacheable mmaps still seems
like a non-starter to me...

Serguei, what is your plan in GPU land for migration? Ie if I have a
CPU mapped page and the GPU moves it to VRAM, it becomes non-cachable
- do you still allow the CPU to access it? Or do you swap it back to
cachable memory if the CPU touches it?


Depends on the policy in command, but currently it's the other way 
around most of the time.


E.g. we allocate memory in VRAM, the CPU writes to it WC and avoids 
reading because that is slow, the GPU in turn can access it with full speed.


When we run out of VRAM we move those allocations to system memory and 
update both the CPU as well as the GPU page tables.


So that move is transparent for both userspace as well as shaders 
running on the GPU.



One approach might be to mmap the uncachable ZONE_DEVICE memory and
mark it inaccessible to the CPU - DMA could still translate. If the
CPU needs it then the kernel migrates it to system memory so it
becomes cachable. ??


The whole purpose of this effort is that we can do I/O on VRAM directly 
without migrating everything back to system memory.


Allowing this, but then doing the migration by the first touch of the 
CPU is clearly not a good idea.


Regards,
Christian.



Jason



--
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


[GIT PULL FOR v4.10] HSV fixes for 4.10

2016-11-25 Thread Hans Verkuil
Two fixes to make vivid work correctly with HSV.

Regards,

Hans

The following changes since commit d3d83ee20afda16ad0133ba00f63c11a8d842a35:

  [media] DaVinci-VPFE-Capture: fix error handling (2016-11-25 07:57:07 -0200)

are available in the git repository at:

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

for you to fetch changes up to d59b74b430d90fc1421c74e225ab76d80568124e:

  vivid: Set color_enc on HSV formats (2016-11-25 13:15:59 +0100)


Ricardo Ribalda Delgado (2):
  v4l2-tpg: Init hv_enc field with a valid value
  vivid: Set color_enc on HSV formats

 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c   | 1 +
 drivers/media/platform/vivid/vivid-vid-common.c | 2 ++
 2 files changed, 3 insertions(+)
--
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


[GIT PULL FOR v4.10] cec: pass parent device in register(), not allocate()

2016-11-25 Thread Hans Verkuil
Hi Mauro,

It's just a single patch, but I'd like to get that into 4.10. Now that the
cec framework moves out of staging I prefer to get this kernel API change
in before more CEC drivers are added.

Regards,

Hans

The following changes since commit 4cc5bed1caeb6d40f2f41c4c5eb83368691fbffb:

  [media] uvcvideo: Use memdup_user() rather than duplicating its 
implementation (2016-11-23 20:06:41 -0200)

are available in the git repository at:

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

for you to fetch changes up to f2684fe2abba97b9ca2c5966c4113644afca181b:

  cec: pass parent device in register(), not allocate() (2016-11-25 09:23:34 
+0100)


Hans Verkuil (1):
  cec: pass parent device in register(), not allocate()

 Documentation/media/kapi/cec-core.rst | 14 ++
 drivers/media/cec/cec-api.c   |  2 +-
 drivers/media/cec/cec-core.c  | 18 ++
 drivers/media/i2c/adv7511.c   |  5 +++--
 drivers/media/i2c/adv7604.c   |  6 +++---
 drivers/media/i2c/adv7842.c   |  6 +++---
 drivers/media/platform/vivid/vivid-cec.c  |  3 +--
 drivers/media/platform/vivid/vivid-cec.h  |  1 -
 drivers/media/platform/vivid/vivid-core.c |  9 -
 drivers/media/usb/pulse8-cec/pulse8-cec.c |  4 ++--
 drivers/staging/media/s5p-cec/s5p_cec.c   |  5 ++---
 drivers/staging/media/st-cec/stih-cec.c   |  5 ++---
 include/media/cec.h   | 10 --
 13 files changed, 41 insertions(+), 47 deletions(-)
--
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] uvcvideo: freeing an error pointer

2016-11-25 Thread Dan Carpenter
A recent cleanup introduced a potential dereference of -EFAULT when we
call kfree(map->menu_info).

Fixes: 4cc5bed1caeb ("[media] uvcvideo: Use memdup_user() rather than 
duplicating its implementation")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index a7e12fd..3e7e283 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -66,14 +66,14 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain,
if (xmap->menu_count == 0 ||
xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) {
ret = -EINVAL;
-   goto done;
+   goto free_map;
}
 
size = xmap->menu_count * sizeof(*map->menu_info);
map->menu_info = memdup_user(xmap->menu_info, size);
if (IS_ERR(map->menu_info)) {
ret = PTR_ERR(map->menu_info);
-   goto done;
+   goto free_map;
}
 
map->menu_count = xmap->menu_count;
@@ -83,13 +83,13 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain,
uvc_trace(UVC_TRACE_CONTROL, "Unsupported V4L2 control type "
  "%u.\n", xmap->v4l2_type);
ret = -ENOTTY;
-   goto done;
+   goto free_map;
}
 
ret = uvc_ctrl_add_mapping(chain, map);
 
-done:
kfree(map->menu_info);
+free_map:
kfree(map);
 
return ret;
--
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


[GIT PULL for v4.9] media fix for xc2028 driver

2016-11-25 Thread Mauro Carvalho Chehab
Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.9-4

For a fix at the firmware load logic at the tuner-xc2028 driver.

Regards,
Mauro

---

The following changes since commit 9c763584b7c8911106bb77af7e648bef09af9d80:

  Linux 4.9-rc6 (2016-11-20 13:52:19 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.9-4

for you to fetch changes up to 22a1e7783e173ab3d86018eb590107d68df46c11:

  xc2028: Fix use-after-free bug properly (2016-11-23 21:04:26 -0200)


media fixes for v4.9-rc7


Takashi Iwai (1):
  xc2028: Fix use-after-free bug properly

 drivers/media/tuners/tuner-xc2028.c | 37 -
 1 file changed, 16 insertions(+), 21 deletions(-)

--
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: Problem with media_build install

2016-11-25 Thread Mauro Carvalho Chehab
Em Fri, 25 Nov 2016 20:02:57 +1100
Vincent McIntyre  escreveu:

> Hi list,
> 
> I sent a patch for this issue, could someone take a look?
> 
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg105340.html

Patch applied. Next time, please add your Signed-off-by:


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: rc: refactor raw handler kthread

2016-11-25 Thread Sean Young
On Tue, Aug 02, 2016 at 07:44:07AM +0200, Heiner Kallweit wrote:
> I think we can get rid of the spinlock protecting the kthread from being
> interrupted by a wakeup in certain parts.
> Even with the current implementation of the kthread the only lost wakeup
> scenario could happen if the wakeup occurs between the kfifo_len check
> and setting the state to TASK_INTERRUPTIBLE.
> 
> In the changed version we could lose a wakeup if it occurs between
> processing the fifo content and setting the state to TASK_INTERRUPTIBLE.
> This scenario is covered by an additional check for available events in
> the fifo and setting the state to TASK_RUNNING in this case.
> 
> In addition the changed version flushes the kfifo before ending
> when the kthread is stopped.
> 
> With this patch we gain:
> - Get rid of the spinlock
> - Simplify code
> - Don't grep / release the mutex for each individual event but just once
>   for the complete fifo content. This reduces overhead if a driver e.g.
>   triggers processing after writing the content of a hw fifo to the kfifo.
> 
> Signed-off-by: Heiner Kallweit 

Looks good to me and I've also tested it.

Tested-by: Sean Young 

> ---
>  drivers/media/rc/rc-core-priv.h |  2 --
>  drivers/media/rc/rc-ir-raw.c| 46 
> +++--
>  2 files changed, 17 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
> index 585d5e5..577128a 100644
> --- a/drivers/media/rc/rc-core-priv.h
> +++ b/drivers/media/rc/rc-core-priv.h
> @@ -20,7 +20,6 @@
>  #define  MAX_IR_EVENT_SIZE   512
>  
>  #include 
> -#include 
>  #include 
>  
>  struct ir_raw_handler {
> @@ -37,7 +36,6 @@ struct ir_raw_handler {
>  struct ir_raw_event_ctrl {
>   struct list_headlist;   /* to keep track of raw 
> clients */
>   struct task_struct  *thread;
> - spinlock_t  lock;
>   /* fifo for the pulse/space durations */
>   DECLARE_KFIFO(kfifo, struct ir_raw_event, MAX_IR_EVENT_SIZE);
>   ktime_t last_event; /* when last event 
> occurred */
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index 205ecc6..71a3e17 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -17,7 +17,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include "rc-core-priv.h"
>  
>  /* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */
> @@ -35,32 +34,26 @@ static int ir_raw_event_thread(void *data)
>   struct ir_raw_handler *handler;
>   struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data;
>  
> - while (!kthread_should_stop()) {
> -
> - spin_lock_irq(&raw->lock);
> -
> - if (!kfifo_len(&raw->kfifo)) {
> - set_current_state(TASK_INTERRUPTIBLE);
> -
> - if (kthread_should_stop())
> - set_current_state(TASK_RUNNING);
> -
> - spin_unlock_irq(&raw->lock);
> - schedule();
> - continue;
> + while (1) {
> + mutex_lock(&ir_raw_handler_lock);
> + while (kfifo_out(&raw->kfifo, &ev, 1)) {
> + list_for_each_entry(handler, &ir_raw_handler_list, list)
> + if (raw->dev->enabled_protocols &
> + handler->protocols || !handler->protocols)
> + handler->decode(raw->dev, ev);
> + raw->prev_ev = ev;
>   }
> + mutex_unlock(&ir_raw_handler_lock);
>  
> - if(!kfifo_out(&raw->kfifo, &ev, 1))
> - dev_err(&raw->dev->dev, "IR event FIFO is empty!\n");
> - spin_unlock_irq(&raw->lock);
> + set_current_state(TASK_INTERRUPTIBLE);
>  
> - mutex_lock(&ir_raw_handler_lock);
> - list_for_each_entry(handler, &ir_raw_handler_list, list)
> - if (raw->dev->enabled_protocols & handler->protocols ||
> - !handler->protocols)
> - handler->decode(raw->dev, ev);
> - raw->prev_ev = ev;
> - mutex_unlock(&ir_raw_handler_lock);
> + if (kthread_should_stop()) {
> + __set_current_state(TASK_RUNNING);
> + break;
> + } else if (!kfifo_is_empty(&raw->kfifo))
> + set_current_state(TASK_RUNNING);
> +
> + schedule();
>   }
>  
>   return 0;
> @@ -219,14 +212,10 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle);
>   */
>  void ir_raw_event_handle(struct rc_dev *dev)
>  {
> - unsigned long flags;
> -
>   if (!dev->raw)
>   return;
>  
> - spin_lock_irqsave(&dev->raw->lock, flags);
>   wake_up_process(dev->raw->thread);
> - spin_unlock_irqrestore(&dev->raw->l

Re: [PATCH v4 3/3] sound/usb: Use Media Controller API to share media resources

2016-11-25 Thread Mauro Carvalho Chehab
Em Wed, 16 Nov 2016 07:29:11 -0700
Shuah Khan  escreveu:

> Change ALSA driver to use Media Controller API to share media resources
> with DVB, and V4L2 drivers on a AU0828 media device.
> 
> Media Controller specific initialization is done after sound card is
> registered.  ALSA creates Media interface and entity function graph
> nodes for Control, Mixer, PCM Playback, and PCM Capture devices.
> 
> snd_usb_hw_params() will call Media Controller enable source handler
> interface to request the media resource. If resource request is granted,
> it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is
> returned.
> 
> Media specific cleanup is done in usb_audio_disconnect().
> 
> Signed-off-by: Shuah Khan 
> ---
> Changes to patch 0003 since the last version in 4.7-rc1:
> - Included fixes to bugs found during testing. 
> - Updated to use the Media Allocator API.
> 
>  sound/usb/Kconfig|   4 +
>  sound/usb/Makefile   |   2 +
>  sound/usb/card.c |  14 +++
>  sound/usb/card.h |   3 +
>  sound/usb/media.c| 314 
> +++
>  sound/usb/media.h|  73 +++
>  sound/usb/mixer.h|   3 +
>  sound/usb/pcm.c  |  28 -
>  sound/usb/quirks-table.h |   1 +
>  sound/usb/stream.c   |   2 +
>  sound/usb/usbaudio.h |   6 +
>  11 files changed, 445 insertions(+), 5 deletions(-)
>  create mode 100644 sound/usb/media.c
>  create mode 100644 sound/usb/media.h
> 
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index a452ad7..d14bf41 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>   select SND_RAWMIDI
>   select SND_PCM
>   select BITREVERSE
> + select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && 
> (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
>   help
> Say Y here to include support for USB audio and USB MIDI
> devices.
> @@ -22,6 +23,9 @@ config SND_USB_AUDIO
> To compile this driver as a module, choose M here: the module
> will be called snd-usb-audio.
>  
> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> + bool
> +
>  config SND_USB_UA101
>   tristate "Edirol UA-101/UA-1000 driver"
>   select SND_PCM
> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
> index 2d2d122..8dca3c4 100644
> --- a/sound/usb/Makefile
> +++ b/sound/usb/Makefile
> @@ -15,6 +15,8 @@ snd-usb-audio-objs :=   card.o \
>   quirks.o \
>   stream.o
>  
> +snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o
> +
>  snd-usbmidi-lib-objs := midi.o
>  
>  # Toplevel Module Dependency
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 9e5276d..2750e56 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -66,6 +66,7 @@
>  #include "format.h"
>  #include "power.h"
>  #include "stream.h"
> +#include "media.h"
>  
>  MODULE_AUTHOR("Takashi Iwai ");
>  MODULE_DESCRIPTION("USB Audio");
> @@ -615,6 +616,11 @@ static int usb_audio_probe(struct usb_interface *intf,
>   if (err < 0)
>   goto __error;
>  
> + if (quirk && quirk->media_device) {
> + /* don't want to fail when media_snd_device_create() fails */
> + media_snd_device_create(chip, intf);
> + }
> +
>   usb_chip[chip->index] = chip;
>   chip->num_interfaces++;
>   usb_set_intfdata(intf, chip);
> @@ -671,6 +677,14 @@ static void usb_audio_disconnect(struct usb_interface 
> *intf)
>   list_for_each(p, &chip->midi_list) {
>   snd_usbmidi_disconnect(p);
>   }
> + /*
> +  * Nice to check quirk && quirk->media_device and
> +  * then call media_snd_device_delete(). Don't have
> +  * access to quirk here. media_snd_device_delete()
> +  * acceses mixer_list
> +  */
> + media_snd_device_delete(chip);
> +
>   /* release mixer resources */
>   list_for_each_entry(mixer, &chip->mixer_list, list) {
>   snd_usb_mixer_disconnect(mixer);
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index 111b0f0..6ef8e88 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -105,6 +105,8 @@ struct snd_usb_endpoint {
>   struct list_head list;
>  };
>  
> +struct media_ctl;
> +
>  struct snd_usb_substream {
>   struct snd_usb_stream *stream;
>   struct usb_device *dev;
> @@ -156,6 +158,7 @@ struct snd_usb_substream {
>   } dsd_dop;
>  
>   bool trigger_tstamp_pending_update; /* trigger timestamp being updated 
> from initial estimate */
> + struct media_ctl *media_ctl;
>  };
>  
>  struct snd_usb_stream {
> diff --git a/sound/usb/media.c b/sound/usb/media.c
> new file mode 100644
> index 000..ccdc4d9
> --- /dev/null
> +++ b/sound/usb/media.c
> @@ -0,0 +1,314 @@
> +/*
> + * media.c - Media Controller specific ALSA driver code
> + *
> + * 

Re: [PATCH v4 2/3] media: change au0828 to use Media Device Allocator API

2016-11-25 Thread Mauro Carvalho Chehab
Em Wed, 16 Nov 2016 07:29:10 -0700
Shuah Khan  escreveu:

> Change au0828 to use Media Device Allocator API to allocate media device
> with the parent usb struct device as the key, so it can be shared with the
> snd_usb_audio driver.
> 
> Signed-off-by: Shuah Khan 

I missed a v5 for this patch. This one looks OK.

If you don't change this anymore, please add on the v6:

Reviewed-by: Mauro Carvalho Chehab 

> ---
> Changes since v2:
> - Updated media_device_delete() to pass in module name.
> 
>  drivers/media/usb/au0828/au0828-core.c | 12 
>  drivers/media/usb/au0828/au0828.h  |  1 +
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/usb/au0828/au0828-core.c 
> b/drivers/media/usb/au0828/au0828-core.c
> index bf53553..582f31f 100644
> --- a/drivers/media/usb/au0828/au0828-core.c
> +++ b/drivers/media/usb/au0828/au0828-core.c
> @@ -157,9 +157,7 @@ static void au0828_unregister_media_device(struct 
> au0828_dev *dev)
>   dev->media_dev->enable_source = NULL;
>   dev->media_dev->disable_source = NULL;
>  
> - media_device_unregister(dev->media_dev);
> - media_device_cleanup(dev->media_dev);
> - kfree(dev->media_dev);
> + media_device_delete(dev->media_dev, KBUILD_MODNAME);
>   dev->media_dev = NULL;
>  #endif
>  }
> @@ -212,14 +210,10 @@ static int au0828_media_device_init(struct au0828_dev 
> *dev,
>  #ifdef CONFIG_MEDIA_CONTROLLER
>   struct media_device *mdev;
>  
> - mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> + mdev = media_device_usb_allocate(udev, KBUILD_MODNAME);
>   if (!mdev)
>   return -ENOMEM;
>  
> - /* check if media device is already initialized */
> - if (!mdev->dev)
> - media_device_usb_init(mdev, udev, udev->product);
> -
>   dev->media_dev = mdev;
>  #endif
>   return 0;
> @@ -487,6 +481,8 @@ static int au0828_media_device_register(struct au0828_dev 
> *dev,
>   /* register media device */
>   ret = media_device_register(dev->media_dev);
>   if (ret) {
> + media_device_delete(dev->media_dev, KBUILD_MODNAME);
> + dev->media_dev = NULL;
>   dev_err(&udev->dev,
>   "Media Device Register Error: %d\n", ret);
>   return ret;
> diff --git a/drivers/media/usb/au0828/au0828.h 
> b/drivers/media/usb/au0828/au0828.h
> index dd7b378..4bf1b0c 100644
> --- a/drivers/media/usb/au0828/au0828.h
> +++ b/drivers/media/usb/au0828/au0828.h
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /* DVB */
>  #include "demux.h"



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 v5 1/3] media: Media Device Allocator API

2016-11-25 Thread Mauro Carvalho Chehab
Em Fri, 18 Nov 2016 14:45:10 -0700
Shuah Khan  escreveu:

> Media Device Allocator API to allows multiple drivers share a media device.
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.
> 
> Signed-off-by: Shuah Khan 
> Reviewed-by: Hans Verkuil 
> ---
> Changes to patch 0001 since v4:
> - Addressed Sakari's review comments with the exception of
>   opting to not introduce media_device_usb_allocate() macro,
>   and to not add a new routine to find media device instance
>   to avoid a one line check.
> 
>  Documentation/media/kapi/mc-core.rst |   2 +
>  drivers/media/Makefile   |   3 +-
>  drivers/media/media-dev-allocator.c  | 135 
> +++
>  include/media/media-dev-allocator.h  |  88 +++
>  4 files changed, 227 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/media-dev-allocator.c
>  create mode 100644 include/media/media-dev-allocator.h
> 
> diff --git a/Documentation/media/kapi/mc-core.rst 
> b/Documentation/media/kapi/mc-core.rst
> index 1a738e5..0b9090d 100644
> --- a/Documentation/media/kapi/mc-core.rst
> +++ b/Documentation/media/kapi/mc-core.rst
> @@ -262,3 +262,5 @@ in the end provide a way to use driver-specific callbacks.
>  .. kernel-doc:: include/media/media-devnode.h
>  
>  .. kernel-doc:: include/media/media-entity.h
> +
> +.. kernel-doc:: include/media/media-dev-allocator.h
> diff --git a/drivers/media/Makefile b/drivers/media/Makefile
> index 0deaa93..7c0701d 100644
> --- a/drivers/media/Makefile
> +++ b/drivers/media/Makefile
> @@ -6,7 +6,8 @@ ifeq ($(CONFIG_MEDIA_CEC_EDID),y)
>obj-$(CONFIG_MEDIA_SUPPORT) += cec-edid.o
>  endif
>  
> -media-objs   := media-device.o media-devnode.o media-entity.o
> +media-objs   := media-device.o media-devnode.o media-entity.o \
> +media-dev-allocator.o
>  
>  #
>  # I2C drivers should come before other drivers, otherwise they'll fail
> diff --git a/drivers/media/media-dev-allocator.c 
> b/drivers/media/media-dev-allocator.c
> new file mode 100644
> index 000..581e50e
> --- /dev/null
> +++ b/drivers/media/media-dev-allocator.c
> @@ -0,0 +1,135 @@
> +/*
> + * media-dev-allocator.c - Media Controller Device Allocator API
> + *
> + * Copyright (c) 2016 Shuah Khan 
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + *
> + * This file is released under the GPLv2.
> + * Credits: Suggested by Laurent Pinchart 
> + */
> +
> +/*
> + * This file adds a global refcounted Media Controller Device Instance API.
> + * A system wide global media device list is managed and each media device
> + * includes a kref count. The last put on the media device releases the media
> + * device instance.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +static LIST_HEAD(media_device_list);
> +static DEFINE_MUTEX(media_device_lock);
> +
> +struct media_device_instance {
> + struct media_device mdev;
> + struct module *owner;
> + struct list_head list;
> + struct device *dev;

I agree with Sakari here: mdev is embedded. So, it will always have
a mdev.dev inside it. So, if dev == mdev.dev for all cases, we
can just get rid of it. If you want to simplify the code, just add
a to_dev() macro like:

#define to_dev(mdi) (mdi)->mdev.dev

> + struct kref refcount;
> +};
> +
> +static inline struct media_device_instance *
> +to_media_device_instance(struct media_device *mdev)
> +{
> + return container_of(mdev, struct media_device_instance, mdev);
> +}
> +
> +static void media_device_instance_release(struct kref *kref)
> +{
> + struct media_device_instance *mdi =
> + container_of(kref, struct media_device_instance, refcount);
> +
> + dev_dbg(mdi->mdev.dev, "%s: mdev=%p\n", __func__, &mdi->mdev);
> +
> + mutex_lock(&media_device_lock);
> +
> + media_device_unregister(&mdi->mdev);
> + media_device_cleanup(&mdi->mdev);
> +
> + list_del(&mdi->list);
> + mutex_unlock(&media_device_lock);
> +
> + kfree(mdi);
> +}
> +
> +/* Callers should hold media_device_lock when calling this function */
> +static struct media_device *__media_device_get(struct device *dev,
> +char *module_name)
> +{
> + struct media_device_instance *mdi;
> +
> + list_for_each_entry(mdi, &media_device_list, list) {
> + if (mdi->dev != dev)
> + continue;
> +
> + kref_get(&mdi->refcount);
> + /* get module reference for the media_device owner */
> + if (find_module(module_name) != mdi->owner &&
> + !try_module_get(mdi->owner))
> + dev_err(dev, "%s: try_module_get() error\n", __func__);
> + dev_dbg(dev, "%s: get mdev=%p module_name %s\n",
> +   

Re: Problem with media_build install

2016-11-25 Thread Vincent McIntyre
Hi list,

I sent a patch for this issue, could someone take a look?

http://www.mail-archive.com/linux-media@vger.kernel.org/msg105340.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: ir-keytable: infinite loops, segfaults

2016-11-25 Thread Vincent McIntyre
On 11/25/16, Sean Young  wrote:
>
> So if I understand you correctly, if you change the keymap, like you
> changed 0xfe47 to KEY_PAUSE, then "ir-keytable -s rc1 -t" show you the
> correct (new) key? So as far as ir-keytable is concerned, everything
> works?
>
> However when you try to use the new mapping in some application then
> it does not work?

That's correct. ir-keytable seems to be doing the right thing, mapping
the scancode to the input-event-codes.h key code I asked it to.

The application I am trying to use it with is the mythtv frontend.  I
am doing the keycode munging from an SSH session while myth is still
running on the main screen. I didn't think this would matter (since it
worked for KEY_OK->KEY_ENTER) but perhaps it does. Obviously
ir-keytable -t intercepts the scancodes when it is running, but when I
kill it myth responds normally to some keys, but not all.



I wanted to mention that the IR protocol is still showing as unknown.
Is there anything that can be done to sort that out?

Vince
--
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 0/3] Avoid warnings about using unitialized dest_dir

2016-11-25 Thread Mauro Carvalho Chehab
Em Mon, 21 Nov 2016 17:35:42 +0100
Arnd Bergmann  escreveu:

> On Saturday, November 19, 2016 12:56:57 PM CET Mauro Carvalho Chehab wrote:
> > As Arnd reported:
> > 
> > With gcc-5 or higher on x86, we can get a bogus warning in the
> > dvb-net code:
> > 
> > drivers/media/dvb-core/dvb_net.c: In function ‘dvb_net_ule’:
> > arch/x86/include/asm/string_32.h:77:14: error: ‘dest_addr’ may be used 
> > uninitialized in this function [-Werror=maybe-uninitialized]
> > drivers/media/dvb-core/dvb_net.c:633:8: note: ‘dest_addr’ was declared 
> > here
> > 
> > Inspecting the code is really hard, as the function Arnd patched is really
> > complex.
> > 
> > IMHO, the best is to first simplify the logic, by breaking parts of it into
> > sub-routines, and then apply a proper fix.
> > 
> > This patch series does that.
> > 
> > Arnd,
> > 
> > After splitting the function, I think that the GCC 5 warning is not bogus,
> > as this code:
> > skb_copy_from_linear_data(h->priv->ule_skb, dest_addr,
> >   ETH_ALEN);
> >
> > is called before initializing dest_dir, but, even if I'm wrong, it is not a 
> > bad
> > idea to zero the dest_addr before handing the logic.  
> 
> My conclusion after looking at it for a while was that it is correct, the
> relevant code is roughtly:
> 
>   if (!priv->ule_dbit) {
>   drop = ...;
>   if (drop)
>   goto done;
>   else
>   skb_copy_from_linear_data(priv->ule_skb, dest_addr, 
> ETH_ALEN);
>   }
> 
>   ...
> 
>   if (!priv->ule_dbit) {
>   memcpy(ethh->h_dest, dest_addr, ETH_ALEN)
>   }
> 
> done:
>   ...
> 
> 
> So it is always copied from the skb data before it gets used.
> 
> > PS.: I took a lot of care to avoid breaking something on this series, as I 
> > don't
> > have any means here to test DVB net.  So, I'd appreciate if you could take
> > a look and see if everything looks fine.  
> 
> I have replaced my patch with your series in my randconfig builds and see no
> new warnings so far.
> 
> The first patch looks correct to me, but I can't really verify the
> second one by inspection. It looks like a nice cleanup and I'd assume
> you did it correctly too. The third patch is probably not needed now,
> I think with the 'goto' removed, gcc will be able to figure it out
> already. It probably adds a few extra cycles to copy the zero data,
> which shouldn't be too bad either.

Thanks for looking into it. I'll apply only the first two
patches for now. If gcc starts complaining again, we can apply
it later.


> [btw, your mche...@s-opensource.com keeps bouncing for me, I had to
>  remove that from Cc to get my reply to make it out]

Gah! I guess I'll need to switch to use some other e-mail instead.
Maybe it is time to use mche...@kernel.org for everything.

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


[PATCHv2] cec: pass parent device in register(), not allocate()

2016-11-25 Thread Hans Verkuil
The cec_allocate_adapter function doesn't need the parent device, only the
cec_register_adapter function needs it.

Drop the cec_devnode parent field, since devnode.dev.parent can be used
instead.

This change makes the framework consistent with other frameworks where the
parent device is not used until the device is registered.

Signed-off-by: Hans Verkuil 
---
The previous v1 patch got mangled and didn't apply, hopefully this time it is
without linewrapping.
---
 Documentation/media/kapi/cec-core.rst | 14 ++
 drivers/media/cec/cec-api.c   |  2 +-
 drivers/media/cec/cec-core.c  | 18 ++
 drivers/media/i2c/adv7511.c   |  5 +++--
 drivers/media/i2c/adv7604.c   |  6 +++---
 drivers/media/i2c/adv7842.c   |  6 +++---
 drivers/media/platform/vivid/vivid-cec.c  |  3 +--
 drivers/media/platform/vivid/vivid-cec.h  |  1 -
 drivers/media/platform/vivid/vivid-core.c |  9 -
 drivers/media/usb/pulse8-cec/pulse8-cec.c |  4 ++--
 drivers/staging/media/s5p-cec/s5p_cec.c   |  5 ++---
 drivers/staging/media/st-cec/stih-cec.c   |  5 ++---
 include/media/cec.h   | 10 --
 13 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/Documentation/media/kapi/cec-core.rst 
b/Documentation/media/kapi/cec-core.rst
index 8a88dd4..81c6d8e 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -37,9 +37,8 @@ The struct cec_adapter represents the CEC adapter hardware. 
It is created by
 calling cec_allocate_adapter() and deleted by calling cec_delete_adapter():

 .. c:function::
-   struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
-  void *priv, const char *name, u32 caps, u8 available_las,
-  struct device *parent);
+   struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops, 
void *priv,
+   const char *name, u32 caps, u8 available_las);

 .. c:function::
void cec_delete_adapter(struct cec_adapter *adap);
@@ -66,20 +65,19 @@ available_las:
the number of simultaneous logical addresses that this
adapter can handle. Must be 1 <= available_las <= CEC_MAX_LOG_ADDRS.

-parent:
-   the parent device.
-

 To register the /dev/cecX device node and the remote control device (if
 CEC_CAP_RC is set) you call:

 .. c:function::
-   int cec_register_adapter(struct cec_adapter \*adap);
+   int cec_register_adapter(struct cec_adapter *adap, struct device 
*parent);
+
+where parent is the parent device.

 To unregister the devices call:

 .. c:function::
-   void cec_unregister_adapter(struct cec_adapter \*adap);
+   void cec_unregister_adapter(struct cec_adapter *adap);

 Note: if cec_register_adapter() fails, then call cec_delete_adapter() to
 clean up. But if cec_register_adapter() succeeded, then only call
diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
index 597fbb6..8950b6c 100644
--- a/drivers/media/cec/cec-api.c
+++ b/drivers/media/cec/cec-api.c
@@ -88,7 +88,7 @@ static long cec_adap_g_caps(struct cec_adapter *adap,
 {
struct cec_caps caps = {};

-   strlcpy(caps.driver, adap->devnode.parent->driver->name,
+   strlcpy(caps.driver, adap->devnode.dev.parent->driver->name,
sizeof(caps.driver));
strlcpy(caps.name, adap->name, sizeof(caps.name));
caps.available_log_addrs = adap->available_log_addrs;
diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index b0137e2..aca3ab8 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -132,7 +132,6 @@ static int __must_check cec_devnode_register(struct 
cec_devnode *devnode,
devnode->dev.bus = &cec_bus_type;
devnode->dev.devt = MKDEV(MAJOR(cec_dev_t), minor);
devnode->dev.release = cec_devnode_release;
-   devnode->dev.parent = devnode->parent;
dev_set_name(&devnode->dev, "cec%d", devnode->minor);
device_initialize(&devnode->dev);

@@ -198,13 +197,11 @@ static void cec_devnode_unregister(struct cec_devnode 
*devnode)

 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 void *priv, const char *name, u32 caps,
-u8 available_las, struct device 
*parent)
+u8 available_las)
 {
struct cec_adapter *adap;
int res;

-   if (WARN_ON(!parent))
-   return ERR_PTR(-EINVAL);
if (WARN_ON(!caps))
return ERR_PTR(-EINVAL);
if (WARN_ON(!ops))
@@ -214,8 +211,6 @@ struct cec_adapter *cec_allocate_adapter(const struct 
cec_adap_ops *ops,
adap = kzalloc(sizeof(*adap), GFP_KERNEL);
if (!adap)
return ERR_PTR(-ENOMEM);
-   adap->owner = parent->driver->owner;
-   adap->devnode.parent = parent;
strlcpy(adap->name, name, sizeof(adap->name));
a

Re: Enabling peer to peer device transactions for PCIe devices

2016-11-25 Thread Christoph Hellwig
On Thu, Nov 24, 2016 at 11:11:34AM -0700, Logan Gunthorpe wrote:
> * Regular DAX in the FS doesn't work at this time because the FS can
> move the file you think your transfer to out from under you. Though I
> understand there's been some work with XFS to solve that issue.

The file system will never move anything under locked down pages,
locking down pages is used exactly to protect against that.  So as long
as we page structures available RDMA to/from device memory _from kernel
space_ is trivial, although for file systems to work properly you
really want a notification to the consumer if the file systems wants
to remove the mapping.  We have implemented that using FL_LAYOUTS locks
for NFSD, but only XFS supports it so far.  Without that a long term
locked down region of memory (e.g. a kernel MR) would prevent various
file operations that would simply hang.
--
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] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay

2016-11-25 Thread Sakari Ailus
Hi Javi,

On Wed, Nov 23, 2016 at 04:15:11PM +, Javi Merino wrote:
> On Wed, Nov 23, 2016 at 05:10:42PM +0200, Sakari Ailus wrote:
> > Hi Javi,
> 
> Hi Sakari,
> 
> > On Wed, Nov 23, 2016 at 10:09:57AM +, Javi Merino wrote:
> > > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > > a devicetree overlay, its of_node pointer will be different each time
> > > the overlay is applied.  We are not interested in matching the
> > > pointer, what we want to match is that the path is the one we are
> > > expecting.  Change to use of_node_cmp() so that we continue matching
> > > after the overlay has been removed and reapplied.
> > > 
> > > Cc: Mauro Carvalho Chehab 
> > > Cc: Javier Martinez Canillas 
> > > Cc: Sakari Ailus 
> > > Signed-off-by: Javi Merino 
> > > ---
> > > Hi,
> > > 
> > > I feel it is a bit of a hack, but I couldn't think of anything better.
> > > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > > solution.
> > 
> > First I have to admit that I'm not an expert when it comes to DT overlays.
> > 
> > That said, my understanding is that the sub-device and the async sub-device
> > are supposed to point to the exactly same DT node. I wonder if there's
> > actually anything wrong in the current code.
> > 
> > If the overlay has changed between probing the driver for the async notifier
> > and the async sub-device, there should be no match here, should there? The
> > two nodes actually point to a node in a different overlay in that case.
> 
> Overlays are parts of the devicetree that can be added and removed.
> When the overlay is applied, the camera driver is probed and does
> v4l2_async_register_subdev().  However, v4l2_async_belongs() fails.
> The problem is with comparing pointers.  I haven't looked at the
> implementation of overlays in detail, but what I see is that the
> of_node pointer changes when you remove and reapply an overlay (I
> guess it's dynamically allocated and when you remove the overlay, it's
> freed).

The concern here which we were discussing was whether the overlay should be
relied on having compliant configuration compared to the part which was not
part of the overlay.

As external components are involved, quite possibly also the ISP DT node
will require changes, not just the image source (TV tuner, camera sensor
etc.). This could be because of number of CSI-2 lanes or parallel bus width,
for instance.

I'd also be interested in having an actual driver implement support for
removing and adding a DT overlay first so we'd see how this would actually
work. We need both in order to be able to actually remove and add DT
overlays _without_ unbinding the ISP driver. Otherwise it should already
work in the current codebase.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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