Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Bryan Wu

On 08/24/2015 11:30 PM, Hans Verkuil wrote:

A quick follow-up to Thierry's excellent review:

On 08/25/2015 02:26 AM, Bryan Wu wrote:

On 08/21/2015 06:03 AM, Thierry Reding wrote:

On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:

snip


+static void
+__tegra_channel_try_format(struct tegra_channel *chan, struct v4l2_pix_format 
*pix,
+ const struct tegra_video_format **fmtinfo)
+{
+   const struct tegra_video_format *info;
+   unsigned int min_width;
+   unsigned int max_width;
+   unsigned int min_bpl;
+   unsigned int max_bpl;
+   unsigned int width;
+   unsigned int align;
+   unsigned int bpl;
+
+   /* Retrieve format information and select the default format if the
+* requested format isn't supported.
+*/
+   info = tegra_core_get_format_by_fourcc(pix-pixelformat);
+   if (!info)
+   info = tegra_core_get_format_by_fourcc(TEGRA_VF_DEF_FOURCC);

Should this not be an error? As far as I can tell this is silently
substituting the default format for the requested one if the requested
one isn't supported. Isn't the whole point of this to find out if some
format is supported?


I think it should return some error and escape following code. I will
fix that.

Actually, this code is according to the V4L2 spec: if the given format is
not supported, then VIDIOC_TRY_FMT should replace it with a valid default
format.

The reality is a bit more complex: in many drivers this was never reviewed
correctly and we ended up with some drivers that return an error for this
case and some drivers that follow the spec. Historically TV capture drivers
return an error, webcam drivers don't. Most unfortunate.

Since this driver is much more likely to be used with sensors I would
follow the spec here and substitute an invalid format with a default
format.




Thanks for letting me know this. It's actually quite confusing since I 
looked at several drivers, some of them return error some of them use 
default format.


-Bryan
--
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/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Thierry Reding
On Tue, Aug 25, 2015 at 04:15:58PM +0200, Hans Verkuil wrote:
 On 08/25/15 15:44, Thierry Reding wrote:
  On Mon, Aug 24, 2015 at 05:26:20PM -0700, Bryan Wu wrote:
[...]
   For CMA we need increase the default memory size.
  
  I'd rather not rely on CMA at all, especially since we do have a way
  around it.
 
 For the record, I have no problem with it if we start out with contiguous
 DMA now and enhance it later. I get the impression that getting the IOMMU
 to work is non-trivial, and I don't think it should block merging of this
 driver.
 
 This is all internal to the driver, so changing it later will not affect
 userspace.

Fair enough.

Thierry


signature.asc
Description: PGP signature


Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Hans Verkuil
On 08/25/15 15:44, Thierry Reding wrote:
 On Mon, Aug 24, 2015 at 05:26:20PM -0700, Bryan Wu wrote:
 On 08/21/2015 06:03 AM, Thierry Reding wrote:
 On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:
 +{
 +  if (chan-bypass)
 +  return;
 I don't see this being set anywhere. Is it dead code? Also the only
 description I see is that it's used to bypass register writes, but I
 don't see an explanation why that's necessary.

 We are unifying our downstream VI driver with V4L2 VI driver. And this
 upstream work is the first step to help that.

 We are also backporting this driver back to our internal 3.10 kernel which
 is using nvhost channel to submit register operations from userspace to
 host1x and VI hardware. Then in this case, our driver needs to bypass all
 the register operations otherwise we got conflicts between these 2 paths.

 That's why I put bypass mode here. And bypass mode can be set in device tree
 or from v4l2-ctrls.
 
 I don't think it's fair to burden upstream with code that will only ever
 be used downstream. Let's split these changes into a separate patch that
 can be carried downstream.

I think that's a good idea.

For the record: I haven't really reviewed the bypass mode. I had the impression
that it needed more work anyway (or am I wrong?).

 +static int tegra_channel_capture_frame(struct tegra_channel *chan)
 +{
 +  struct tegra_channel_buffer *buf = chan-active;
 +  struct vb2_buffer *vb = buf-buf;
 +  int err = 0;
 +  u32 thresh, value, frame_start;
 +  int bytes_per_line = chan-format.bytesperline;
 +
 +  if (!vb2_start_streaming_called(chan-queue) || !buf)
 +  return -EINVAL;
 +
 +  if (chan-bypass)
 +  goto bypass_done;
 +
 +  /* Program buffer address */
 +  csi_write(chan,
 +TEGRA_VI_CSI_SURFACE0_OFFSET_MSB + chan-surface * 8,
 +0x0);
 +  csi_write(chan,
 +TEGRA_VI_CSI_SURFACE0_OFFSET_LSB + chan-surface * 8,
 +buf-addr);
 +  csi_write(chan,
 +TEGRA_VI_CSI_SURFACE0_STRIDE + chan-surface * 4,
 +bytes_per_line);
 +
 +  /* Program syncpoint */
 +  frame_start = sp_bit(chan, SP_PP_FRAME_START);
 +  tegra_channel_write(chan, TEGRA_VI_CFG_VI_INCR_SYNCPT,
 +  frame_start | host1x_syncpt_id(chan-sp));
 +
 +  csi_write(chan, TEGRA_VI_CSI_SINGLE_SHOT, 0x1);
 +
 +  /* Use syncpoint to wake up */
 +  thresh = host1x_syncpt_incr_max(chan-sp, 1);
 +
 +  mutex_unlock(chan-lock);
 +  err = host1x_syncpt_wait(chan-sp, thresh,
 +   TEGRA_VI_SYNCPT_WAIT_TIMEOUT, value);
 +  mutex_lock(chan-lock);
 What's the point of taking the lock in the first place if you drop it
 here, even if temporarily? This is a per-channel lock, and it protects
 the channel against concurrent captures. So if you drop the lock here,
 don't you run risk of having two captures run concurrently? And by the
 time you get to the error handling or buffer completion below you can't
 be sure you're actually dealing with the same buffer that you started
 with.

 After some discussion with Hans, I changed to this. Since there won't be a
 second capture start which is prevented by v4l2-core, it won't cause the
 buffer issue.

 Waiting for host1x syncpoint take time, so dropping lock can let other
 non-capture ioctls and operations happen.
 
 If the core already prevents multiple captures for a single channel, do
 we even need the lock in the first place?

While this is running another process might call the driver which then
changes some of these registers. So typically locking is needed. Since this
is going to be rewritten to a kthread I'm postponing reviewing the locking
until I see the new version. I expect this to make much more sense then.

 +static int tegra_channel_buffer_prepare(struct vb2_buffer *vb)
 +{
 +  struct tegra_channel *chan = vb2_get_drv_priv(vb-vb2_queue);
 +  struct tegra_channel_buffer *buf = to_tegra_channel_buffer(vb);
 +
 +  buf-chan = chan;
 +  buf-addr = vb2_dma_contig_plane_dma_addr(vb, 0);
 +
 +  return 0;
 +}
 This seems to use contiguous DMA, which I guess presumes CMA support?
 We're dealing with very large buffers here. Your default frame size
 would yield buffers of roughly 32 MiB each, and you probably need a
 couple of those to ensure smooth playback. That's quite a bit of
 memory to reserve for CMA.
 In vb2 core driver, it's using dma-mapping API which might be CMA or SMMU.
 
 There is no way to use the DMA API with SMMU upstream. You need to set
 up your IOMMU domain yourself and attach the VI device to it manually.
 That means you'll also need to manage your IOVA space manually to make
 use of this. I know it's an unfortunate situation and there's work
 underway to improve it, but we're not quite there yet.
 
 For CMA we need increase the default memory size.
 
 I'd rather not rely on CMA at all, especially since we do have a way
 around it.

For the record, I have no problem with it if we start out with contiguous
DMA now and enhance it later. I get 

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Thierry Reding
On Tue, Aug 25, 2015 at 08:30:41AM +0200, Hans Verkuil wrote:
 A quick follow-up to Thierry's excellent review:
 
 On 08/25/2015 02:26 AM, Bryan Wu wrote:
  On 08/21/2015 06:03 AM, Thierry Reding wrote:
  On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:
 
 snip
 
  +static void
  +__tegra_channel_try_format(struct tegra_channel *chan, struct 
  v4l2_pix_format *pix,
  +   const struct tegra_video_format **fmtinfo)
  +{
  + const struct tegra_video_format *info;
  + unsigned int min_width;
  + unsigned int max_width;
  + unsigned int min_bpl;
  + unsigned int max_bpl;
  + unsigned int width;
  + unsigned int align;
  + unsigned int bpl;
  +
  + /* Retrieve format information and select the default format if the
  +  * requested format isn't supported.
  +  */
  + info = tegra_core_get_format_by_fourcc(pix-pixelformat);
  + if (!info)
  + info = tegra_core_get_format_by_fourcc(TEGRA_VF_DEF_FOURCC);
  Should this not be an error? As far as I can tell this is silently
  substituting the default format for the requested one if the requested
  one isn't supported. Isn't the whole point of this to find out if some
  format is supported?
 
  I think it should return some error and escape following code. I will 
  fix that.
 
 Actually, this code is according to the V4L2 spec: if the given format is
 not supported, then VIDIOC_TRY_FMT should replace it with a valid default
 format.
 
 The reality is a bit more complex: in many drivers this was never reviewed
 correctly and we ended up with some drivers that return an error for this
 case and some drivers that follow the spec. Historically TV capture drivers
 return an error, webcam drivers don't. Most unfortunate.
 
 Since this driver is much more likely to be used with sensors I would
 follow the spec here and substitute an invalid format with a default
 format.

Okay, sounds good to me.

Thierry


signature.asc
Description: PGP signature


Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Hans Verkuil
A quick follow-up to Thierry's excellent review:

On 08/25/2015 02:26 AM, Bryan Wu wrote:
 On 08/21/2015 06:03 AM, Thierry Reding wrote:
 On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:

snip

 +static void
 +__tegra_channel_try_format(struct tegra_channel *chan, struct 
 v4l2_pix_format *pix,
 + const struct tegra_video_format **fmtinfo)
 +{
 +   const struct tegra_video_format *info;
 +   unsigned int min_width;
 +   unsigned int max_width;
 +   unsigned int min_bpl;
 +   unsigned int max_bpl;
 +   unsigned int width;
 +   unsigned int align;
 +   unsigned int bpl;
 +
 +   /* Retrieve format information and select the default format if the
 +* requested format isn't supported.
 +*/
 +   info = tegra_core_get_format_by_fourcc(pix-pixelformat);
 +   if (!info)
 +   info = tegra_core_get_format_by_fourcc(TEGRA_VF_DEF_FOURCC);
 Should this not be an error? As far as I can tell this is silently
 substituting the default format for the requested one if the requested
 one isn't supported. Isn't the whole point of this to find out if some
 format is supported?

 I think it should return some error and escape following code. I will 
 fix that.

Actually, this code is according to the V4L2 spec: if the given format is
not supported, then VIDIOC_TRY_FMT should replace it with a valid default
format.

The reality is a bit more complex: in many drivers this was never reviewed
correctly and we ended up with some drivers that return an error for this
case and some drivers that follow the spec. Historically TV capture drivers
return an error, webcam drivers don't. Most unfortunate.

Since this driver is much more likely to be used with sensors I would
follow the spec here and substitute an invalid format with a default
format.

Regards,

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


Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Bryan Wu

On 08/25/2015 06:44 AM, Thierry Reding wrote:

On Mon, Aug 24, 2015 at 05:26:20PM -0700, Bryan Wu wrote:

On 08/21/2015 06:03 AM, Thierry Reding wrote:

On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:

[...]

+{
+   if (chan-bypass)
+   return;

I don't see this being set anywhere. Is it dead code? Also the only
description I see is that it's used to bypass register writes, but I
don't see an explanation why that's necessary.

We are unifying our downstream VI driver with V4L2 VI driver. And this
upstream work is the first step to help that.

We are also backporting this driver back to our internal 3.10 kernel which
is using nvhost channel to submit register operations from userspace to
host1x and VI hardware. Then in this case, our driver needs to bypass all
the register operations otherwise we got conflicts between these 2 paths.

That's why I put bypass mode here. And bypass mode can be set in device tree
or from v4l2-ctrls.

I don't think it's fair to burden upstream with code that will only ever
be used downstream. Let's split these changes into a separate patch that
can be carried downstream.


OK, I will split out a patch for downstream.



+/* Syncpoint bits of TEGRA_VI_CFG_VI_INCR_SYNCPT */
+static u32 sp_bit(struct tegra_channel *chan, u32 sp)
+{
+   return (sp + chan-port * 4)  8;
+}

Technically this returns a mask, not a bit, so sp_mask() would be more
appropriate.

Actually it returns the syncpoint value for each port not a mask. Probably
sp_bits() is better.

Looking at the TRM, the field that this generates a value for is called
VI_COND (in the VI_CFG_VI_INCR_SYNCPT register), so perhaps this should
really be a macro and named something like:

#define VI_CFG_VI_INCR_SYNCPT_COND(x) (((x)  0xff)  8)

As for the arithmetic, that doesn't seem to match up. Quoting from your
original patch:


+/* VI registers */
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT 0x000
+#defineSP_PP_LINE_START4
+#defineSP_PP_FRAME_START   5
+#defineSP_MW_REQ_DONE  6
+#defineSP_MW_ACK_DONE  7

This doesn't seem to match the TRM, which has the following values:

 0 = IMMEDIATE
 1 = OP_DONE
 2 = RD_DONE
 3 = REG_WR_SAFE
 4 = VI_MWA_REQ_DONE
 5 = VI_MWB_REQ_DONE
 6 = VI_MWA_ACK_DONE
 7 = VI_MWB_ACK_DONE
 8 = VI_ISPA_DONE
 9 = VI_CSI_PPA_FRAME_START
10 = VI_CSI_PPB_FRAME_START
11 = VI_CSI_PPA_LINE_START
12 = VI_CSI_PPB_LINE_START
13 = VI_VGP0_RCVD
14 = VI_VGP1_RCVD
15 = VI_ISPB_DONE

Comparing with the internal register manuals it looks like the TRM is
actually wrong. Can you file an internal bug to rectify this and Cc me
on it, please?


Oh, oops. I will file a bug for that. This list is actually old one in 
Tegra K1 not for Tegra X1.



Irrespective, since this is generating content for a register field it
would seem more consistent to define it as a parameterized macro, like
so:

#define VI_CSI_PP_LINE_START(port)  (4 + (port) * 4)
#define VI_CSI_PP_FRAME_START(port) (5 + (port) * 4)
#define VI_CSI_MWA_REQ_DONE(port)   (6 + (port) * 4)
#define VI_CSI_MWA_ACK_DONE(port)   (7 + (port) * 4)

and then use them together with the above macro:

value = VI_CFG_VI_INCR_SYNCPT_COND(VI_CSI_PP_FRAME_START(port)) |
host1x_syncpt_id(syncpt);
writel(value, ...);


Looks good to me. I will fix this.



+static int tegra_channel_capture_setup(struct tegra_channel *chan)
+{
+   int lanes = 2;

unsigned int? And why is it hardcoded to 2? There are checks below for
lanes == 4, which will effectively never happen. So at the very least I
think this should have a TODO comment of some sort. Preferably can it
not be determined at runtime what number of lanes we need?

Sure, I forget to fix this. lanes should get from DT and for TPG mode I will
choose lanes as 4 by default.

Can the number of lanes required not be determined at runtime? I suspect
it would be a property of whatever camera is attached. Then again, this
is perhaps clarified by the DT binding, so I'll wait and see how that
looks.


Sure, normally lanes number is defined as bus-width in the DT node 
when a real sensor connects.
But since TPG is a virtual sensor which doesn't have any lanes 
requirement. Let's choose 4 for TPG.



+   u32 height = chan-format.height;
+   u32 width = chan-format.width;
+   u32 format = chan-fmtinfo-img_fmt;
+   u32 data_type = chan-fmtinfo-img_dt;
+   u32 word_count = tegra_core_get_word_count(width, chan-fmtinfo);
+   struct chan_regs_config *regs = chan-regs;
+
+   /* CIL PHY register setup */
+   if (port  0x1) {
+   cil_write(chan, TEGRA_CSI_CIL_PAD_CONFIG0 - 0x34, 0x0);
+   

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-25 Thread Thierry Reding
On Mon, Aug 24, 2015 at 05:26:20PM -0700, Bryan Wu wrote:
 On 08/21/2015 06:03 AM, Thierry Reding wrote:
 On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:
[...]
 +#define TEGRA_CSI_PHY_CIL_COMMAND  0x0908
 This doesn't seem to be used at all.
 
 Actually this PHY register just has this one only, I need define it as 0x0
 offset here. Let's keep this since in future we might have more PHY
 registers.

Yes, I had been wondering about the PHY registers. If we make this a
register with offset 0, as I understand it will become used because of
the phy_{readl,writel}() rework.

 +#define TEGRA_CSI_PATTERN_GENERATOR_CTRL   0x000
 +#define TEGRA_CSI_PG_BLANK 0x004
 +#define TEGRA_CSI_PG_PHASE 0x008
 +#define TEGRA_CSI_PG_RED_FREQ  0x00c
 +#define TEGRA_CSI_PG_RED_FREQ_RATE 0x010
 +#define TEGRA_CSI_PG_GREEN_FREQ0x014
 +#define TEGRA_CSI_PG_GREEN_FREQ_RATE   0x018
 +#define TEGRA_CSI_PG_BLUE_FREQ 0x01c
 +#define TEGRA_CSI_PG_BLUE_FREQ_RATE0x020
 +#define TEGRA_CSI_PG_AOHDR 0x024
 +
 +#define TEGRA_CSI_DPCM_CTRL_A  0xad0
 +#define TEGRA_CSI_DPCM_CTRL_B  0xad4
 +#define TEGRA_CSI_STALL_COUNTER0xae8
 +#define TEGRA_CSI_CSI_READONLY_STATUS  0xaec
 +#define TEGRA_CSI_CSI_SW_STATUS_RESET  0xaf0
 +#define TEGRA_CSI_CLKEN_OVERRIDE   0xaf4
 +#define TEGRA_CSI_DEBUG_CONTROL0xaf8
 +#define TEGRA_CSI_DEBUG_COUNTER_0  0xafc
 +#define TEGRA_CSI_DEBUG_COUNTER_1  0xb00
 +#define TEGRA_CSI_DEBUG_COUNTER_2  0xb04
 Some of these are unused. I guess there's an argument to be made to
 include all register definitions rather than just the used ones, if for
 nothing else than completeness. I'll defer to Hans's judgement on this.
 
 These are VI/CSI global registers shared by all the channels. Some of them
 are used in this driver, I suggest we keep them here.

Fine with me.

 +{
 +   if (chan-bypass)
 +   return;
 I don't see this being set anywhere. Is it dead code? Also the only
 description I see is that it's used to bypass register writes, but I
 don't see an explanation why that's necessary.
 
 We are unifying our downstream VI driver with V4L2 VI driver. And this
 upstream work is the first step to help that.
 
 We are also backporting this driver back to our internal 3.10 kernel which
 is using nvhost channel to submit register operations from userspace to
 host1x and VI hardware. Then in this case, our driver needs to bypass all
 the register operations otherwise we got conflicts between these 2 paths.
 
 That's why I put bypass mode here. And bypass mode can be set in device tree
 or from v4l2-ctrls.

I don't think it's fair to burden upstream with code that will only ever
be used downstream. Let's split these changes into a separate patch that
can be carried downstream.

 +/* Syncpoint bits of TEGRA_VI_CFG_VI_INCR_SYNCPT */
 +static u32 sp_bit(struct tegra_channel *chan, u32 sp)
 +{
 +   return (sp + chan-port * 4)  8;
 +}
 Technically this returns a mask, not a bit, so sp_mask() would be more
 appropriate.
 Actually it returns the syncpoint value for each port not a mask. Probably
 sp_bits() is better.

Looking at the TRM, the field that this generates a value for is called
VI_COND (in the VI_CFG_VI_INCR_SYNCPT register), so perhaps this should
really be a macro and named something like:

#define VI_CFG_VI_INCR_SYNCPT_COND(x) (((x)  0xff)  8)

As for the arithmetic, that doesn't seem to match up. Quoting from your
original patch:

   +/* VI registers */
   +#define TEGRA_VI_CFG_VI_INCR_SYNCPT 0x000
   +#define  SP_PP_LINE_START4
   +#define  SP_PP_FRAME_START   5
   +#define  SP_MW_REQ_DONE  6
   +#define  SP_MW_ACK_DONE  7

This doesn't seem to match the TRM, which has the following values:

 0 = IMMEDIATE
 1 = OP_DONE
 2 = RD_DONE
 3 = REG_WR_SAFE
 4 = VI_MWA_REQ_DONE
 5 = VI_MWB_REQ_DONE
 6 = VI_MWA_ACK_DONE
 7 = VI_MWB_ACK_DONE
 8 = VI_ISPA_DONE
 9 = VI_CSI_PPA_FRAME_START
10 = VI_CSI_PPB_FRAME_START
11 = VI_CSI_PPA_LINE_START
12 = VI_CSI_PPB_LINE_START
13 = VI_VGP0_RCVD
14 = VI_VGP1_RCVD
15 = VI_ISPB_DONE

Comparing with the internal register manuals it looks like the TRM is
actually wrong. Can you file an internal bug to rectify this and Cc me
on it, please?

Irrespective, since this is generating content for a register field it
would seem more consistent to define it as a 

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-24 Thread Bryan Wu

On 08/21/2015 02:28 AM, Hans Verkuil wrote:

Hi Bryan,

Thanks for contributing this driver, very much appreciated.

I do have some comments below, basically about the same things we discussed
privately before.

On 08/21/2015 02:51 AM, Bryan Wu wrote:

NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
controller which can support up to 6 MIPI CSI camera sensors.

This patch adds a V4L2 media controller and capture driver to support
Tegra VI hardware. It's verified with Tegra built-in test pattern
generator.

Signed-off-by: Bryan Wu pe...@nvidia.com
Reviewed-by: Hans Verkuil hans.verk...@cisco.com
---
  drivers/media/platform/Kconfig   |1 +
  drivers/media/platform/Makefile  |2 +
  drivers/media/platform/tegra/Kconfig |9 +
  drivers/media/platform/tegra/Makefile|3 +
  drivers/media/platform/tegra/tegra-channel.c | 1074 ++
  drivers/media/platform/tegra/tegra-core.c|  295 +++
  drivers/media/platform/tegra/tegra-core.h|  134 
  drivers/media/platform/tegra/tegra-vi.c  |  585 ++
  drivers/media/platform/tegra/tegra-vi.h  |  224 ++
  include/dt-bindings/media/tegra-vi.h |   35 +
  10 files changed, 2362 insertions(+)
  create mode 100644 drivers/media/platform/tegra/Kconfig
  create mode 100644 drivers/media/platform/tegra/Makefile
  create mode 100644 drivers/media/platform/tegra/tegra-channel.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.h
  create mode 100644 drivers/media/platform/tegra/tegra-vi.c
  create mode 100644 drivers/media/platform/tegra/tegra-vi.h
  create mode 100644 include/dt-bindings/media/tegra-vi.h


snip


+static int tegra_channel_capture_frame(struct tegra_channel *chan)
+{
+   struct tegra_channel_buffer *buf = chan-active;
+   struct vb2_buffer *vb = buf-buf;
+   int err = 0;
+   u32 thresh, value, frame_start;
+   int bytes_per_line = chan-format.bytesperline;
+
+   if (!vb2_start_streaming_called(chan-queue) || !buf)
+   return -EINVAL;
+
+   if (chan-bypass)
+   goto bypass_done;
+
+   /* Program buffer address */
+   csi_write(chan,
+ TEGRA_VI_CSI_SURFACE0_OFFSET_MSB + chan-surface * 8,
+ 0x0);
+   csi_write(chan,
+ TEGRA_VI_CSI_SURFACE0_OFFSET_LSB + chan-surface * 8,
+ buf-addr);
+   csi_write(chan,
+ TEGRA_VI_CSI_SURFACE0_STRIDE + chan-surface * 4,
+ bytes_per_line);
+
+   /* Program syncpoint */
+   frame_start = sp_bit(chan, SP_PP_FRAME_START);
+   tegra_channel_write(chan, TEGRA_VI_CFG_VI_INCR_SYNCPT,
+   frame_start | host1x_syncpt_id(chan-sp));
+
+   csi_write(chan, TEGRA_VI_CSI_SINGLE_SHOT, 0x1);
+
+   /* Use syncpoint to wake up */
+   thresh = host1x_syncpt_incr_max(chan-sp, 1);
+
+   mutex_unlock(chan-lock);
+   err = host1x_syncpt_wait(chan-sp, thresh,
+TEGRA_VI_SYNCPT_WAIT_TIMEOUT, value);
+   mutex_lock(chan-lock);
+
+   if (err) {
+   dev_err(chan-video.dev, frame start syncpt timeout!\n);
+   tegra_channel_capture_error(chan, err);
+   }
+
+bypass_done:
+   /* Captured one frame */
+   spin_lock_irq(chan-queued_lock);
+   vb-v4l2_buf.sequence = chan-sequence++;
+   vb-v4l2_buf.field = V4L2_FIELD_NONE;
+   v4l2_get_timestamp(vb-v4l2_buf.timestamp);
+   vb2_set_plane_payload(vb, 0, chan-format.sizeimage);
+   vb2_buffer_done(vb, err  0 ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
+   spin_unlock_irq(chan-queued_lock);
+
+   return err;
+}
+
+static void tegra_channel_work(struct work_struct *work)
+{
+   struct tegra_channel *chan =
+   container_of(work, struct tegra_channel, work);
+
+   while (1) {
+   spin_lock_irq(chan-queued_lock);
+   if (list_empty(chan-capture)) {
+   chan-active = NULL;
+   spin_unlock_irq(chan-queued_lock);
+   return;
+   }
+   chan-active = list_entry(chan-capture.next,
+   struct tegra_channel_buffer, queue);
+   list_del_init(chan-active-queue);
+   spin_unlock_irq(chan-queued_lock);
+
+   mutex_lock(chan-lock);
+   tegra_channel_capture_frame(chan);
+   mutex_unlock(chan-lock);
+   }
+}
+
+/* 
-
+ * videobuf2 queue operations
+ */
+
+static int
+tegra_channel_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+unsigned int *nbuffers, unsigned int *nplanes,
+unsigned int sizes[], void *alloc_ctxs[])
+{
+   struct tegra_channel *chan = 

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-24 Thread Bryan Wu

On 08/21/2015 06:03 AM, Thierry Reding wrote:

On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:

NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
controller which can support up to 6 MIPI CSI camera sensors.

This patch adds a V4L2 media controller and capture driver to support
Tegra VI hardware. It's verified with Tegra built-in test pattern
generator.

Hi Bryan,

I've been looking forward to seeing this posted. I don't know the VI
hardware in very much detail, nor am I an expert on the media framework,
so I will primarily comment on architectural or SoC-specific things.

By the way, please always Cc linux-te...@vger.kernel.org on all patches
relating to Tegra. That way people not explicitly Cc'ed but still
interested in Tegra will see this code, even if they aren't subscribed
to the linux-media mailing list.

Oops. let me add linux-te...@vger.kernel.org in Cc this time.


Signed-off-by: Bryan Wu pe...@nvidia.com
Reviewed-by: Hans Verkuil hans.verk...@cisco.com
---
  drivers/media/platform/Kconfig   |1 +
  drivers/media/platform/Makefile  |2 +
  drivers/media/platform/tegra/Kconfig |9 +
  drivers/media/platform/tegra/Makefile|3 +
  drivers/media/platform/tegra/tegra-channel.c | 1074 ++
  drivers/media/platform/tegra/tegra-core.c|  295 +++
  drivers/media/platform/tegra/tegra-core.h|  134 
  drivers/media/platform/tegra/tegra-vi.c  |  585 ++
  drivers/media/platform/tegra/tegra-vi.h  |  224 ++
  include/dt-bindings/media/tegra-vi.h |   35 +
  10 files changed, 2362 insertions(+)
  create mode 100644 drivers/media/platform/tegra/Kconfig
  create mode 100644 drivers/media/platform/tegra/Makefile
  create mode 100644 drivers/media/platform/tegra/tegra-channel.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.h
  create mode 100644 drivers/media/platform/tegra/tegra-vi.c
  create mode 100644 drivers/media/platform/tegra/tegra-vi.h
  create mode 100644 include/dt-bindings/media/tegra-vi.h

I can't spot a device tree binding document for this, but we'll need one
to properly review this driver.

Sure, I will add binding document for this.


diff --git a/drivers/media/platform/tegra/Kconfig 
b/drivers/media/platform/tegra/Kconfig
new file mode 100644
index 000..a69d1b2
--- /dev/null
+++ b/drivers/media/platform/tegra/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_TEGRA
+   tristate NVIDIA Tegra Video Input Driver (EXPERIMENTAL)

I don't think the (EXPERIMENTAL) is warranted. Either the driver works
or it doesn't. And I assume you already tested that it works, even if
only using the TPG.


OK, I will remove EXPERIMENTAL.


+   depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF

This seems to be missing a couple of dependencies. For example I would
expect at least TEGRA_HOST1X to be listed here to make sure people can't
select this when the host1x API isn't available. I would also expect
some sort of architecture dependency because it really makes no sense to
build this if Tegra isn't supported.

If you are concerned about compile coverage you can make that explicit
using a COMPILE_TEST alternative such as:

depends on ARCH_TEGRA || (ARM  COMPILE_TEST)

Note that the ARM dependency in there makes sure that HAVE_IOMEM is
selected, so this could also be:

depends on ARCH_TEGRA || (HAVE_IOMEM  COMPILE_TEST)

though that'd still leave open the possibility of build breakage because
of some missing support.

If you add the dependency on TEGRA_HOST1X that I mentioned above you
shouldn't need any architecture dependency because TEGRA_HOST1X implies
those already.
Let me add 'depends on TEGRA_HOST1X' which depends on ARCH_TEGRA. Then I 
don't think I need more Tegra architecture specific rules here, because 
like pmc.c covers IO rails, powergate and reset-controller.



+   select VIDEOBUF2_DMA_CONTIG
+   ---help---
+ Driver for Video Input (VI) device controller in NVIDIA Tegra SoC.

I'd reword this slightly as:

  Driver for the Video Input (VI) controller found on NVIDIA Tegra
  SoCs.


Fixed.


+
+ TO compile this driver as a module, choose M here: the module will be

s/TO/To/.


Fixed.




+ called tegra-video.
diff --git a/drivers/media/platform/tegra/Makefile 
b/drivers/media/platform/tegra/Makefile
new file mode 100644
index 000..c8eff0b
--- /dev/null
+++ b/drivers/media/platform/tegra/Makefile
@@ -0,0 +1,3 @@
+tegra-video-objs += tegra-core.o tegra-vi.o tegra-channel.o

I'd personally leave out the redundant tegra- prefix here, because the
files are in a tegra/ subdirectory already.
Right, some subsystem might don't like those prefix. But I just follow 
the rules in media subsystem here.



+obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o
diff --git a/drivers/media/platform/tegra/tegra-channel.c 

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-21 Thread Thierry Reding
On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:
 NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
 controller which can support up to 6 MIPI CSI camera sensors.
 
 This patch adds a V4L2 media controller and capture driver to support
 Tegra VI hardware. It's verified with Tegra built-in test pattern
 generator.

Hi Bryan,

I've been looking forward to seeing this posted. I don't know the VI
hardware in very much detail, nor am I an expert on the media framework,
so I will primarily comment on architectural or SoC-specific things.

By the way, please always Cc linux-te...@vger.kernel.org on all patches
relating to Tegra. That way people not explicitly Cc'ed but still
interested in Tegra will see this code, even if they aren't subscribed
to the linux-media mailing list.

 Signed-off-by: Bryan Wu pe...@nvidia.com
 Reviewed-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/platform/Kconfig   |1 +
  drivers/media/platform/Makefile  |2 +
  drivers/media/platform/tegra/Kconfig |9 +
  drivers/media/platform/tegra/Makefile|3 +
  drivers/media/platform/tegra/tegra-channel.c | 1074 
 ++
  drivers/media/platform/tegra/tegra-core.c|  295 +++
  drivers/media/platform/tegra/tegra-core.h|  134 
  drivers/media/platform/tegra/tegra-vi.c  |  585 ++
  drivers/media/platform/tegra/tegra-vi.h  |  224 ++
  include/dt-bindings/media/tegra-vi.h |   35 +
  10 files changed, 2362 insertions(+)
  create mode 100644 drivers/media/platform/tegra/Kconfig
  create mode 100644 drivers/media/platform/tegra/Makefile
  create mode 100644 drivers/media/platform/tegra/tegra-channel.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.h
  create mode 100644 drivers/media/platform/tegra/tegra-vi.c
  create mode 100644 drivers/media/platform/tegra/tegra-vi.h
  create mode 100644 include/dt-bindings/media/tegra-vi.h

I can't spot a device tree binding document for this, but we'll need one
to properly review this driver.

 diff --git a/drivers/media/platform/tegra/Kconfig 
 b/drivers/media/platform/tegra/Kconfig
 new file mode 100644
 index 000..a69d1b2
 --- /dev/null
 +++ b/drivers/media/platform/tegra/Kconfig
 @@ -0,0 +1,9 @@
 +config VIDEO_TEGRA
 + tristate NVIDIA Tegra Video Input Driver (EXPERIMENTAL)

I don't think the (EXPERIMENTAL) is warranted. Either the driver works
or it doesn't. And I assume you already tested that it works, even if
only using the TPG.

 + depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF

This seems to be missing a couple of dependencies. For example I would
expect at least TEGRA_HOST1X to be listed here to make sure people can't
select this when the host1x API isn't available. I would also expect
some sort of architecture dependency because it really makes no sense to
build this if Tegra isn't supported.

If you are concerned about compile coverage you can make that explicit
using a COMPILE_TEST alternative such as:

depends on ARCH_TEGRA || (ARM  COMPILE_TEST)

Note that the ARM dependency in there makes sure that HAVE_IOMEM is
selected, so this could also be:

depends on ARCH_TEGRA || (HAVE_IOMEM  COMPILE_TEST)

though that'd still leave open the possibility of build breakage because
of some missing support.

If you add the dependency on TEGRA_HOST1X that I mentioned above you
shouldn't need any architecture dependency because TEGRA_HOST1X implies
those already.

 + select VIDEOBUF2_DMA_CONTIG
 + ---help---
 +   Driver for Video Input (VI) device controller in NVIDIA Tegra SoC.

I'd reword this slightly as:

  Driver for the Video Input (VI) controller found on NVIDIA Tegra
  SoCs.

 +
 +   TO compile this driver as a module, choose M here: the module will be

s/TO/To/.

 +   called tegra-video.

 diff --git a/drivers/media/platform/tegra/Makefile 
 b/drivers/media/platform/tegra/Makefile
 new file mode 100644
 index 000..c8eff0b
 --- /dev/null
 +++ b/drivers/media/platform/tegra/Makefile
 @@ -0,0 +1,3 @@
 +tegra-video-objs += tegra-core.o tegra-vi.o tegra-channel.o

I'd personally leave out the redundant tegra- prefix here, because the
files are in a tegra/ subdirectory already.

 +obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o
 diff --git a/drivers/media/platform/tegra/tegra-channel.c 
 b/drivers/media/platform/tegra/tegra-channel.c
 new file mode 100644
 index 000..b0063d2
 --- /dev/null
 +++ b/drivers/media/platform/tegra/tegra-channel.c
 @@ -0,0 +1,1074 @@
 +/*
 + * NVIDIA Tegra Video Input Device
 + *
 + * Copyright (c) 2015, NVIDIA CORPORATION.  All rights reserved.
 + *
 + * Author: Bryan Wu pe...@nvidia.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-21 Thread Hans Verkuil
On 08/21/2015 03:03 PM, Thierry Reding wrote:
 On Thu, Aug 20, 2015 at 05:51:39PM -0700, Bryan Wu wrote:
 NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
 controller which can support up to 6 MIPI CSI camera sensors.

 This patch adds a V4L2 media controller and capture driver to support
 Tegra VI hardware. It's verified with Tegra built-in test pattern
 generator.
 
 Hi Bryan,
 
 I've been looking forward to seeing this posted. I don't know the VI
 hardware in very much detail, nor am I an expert on the media framework,
 so I will primarily comment on architectural or SoC-specific things.
 
 By the way, please always Cc linux-te...@vger.kernel.org on all patches
 relating to Tegra. That way people not explicitly Cc'ed but still
 interested in Tegra will see this code, even if they aren't subscribed
 to the linux-media mailing list.
 
 Signed-off-by: Bryan Wu pe...@nvidia.com
 Reviewed-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/platform/Kconfig   |1 +
  drivers/media/platform/Makefile  |2 +
  drivers/media/platform/tegra/Kconfig |9 +
  drivers/media/platform/tegra/Makefile|3 +
  drivers/media/platform/tegra/tegra-channel.c | 1074 
 ++
  drivers/media/platform/tegra/tegra-core.c|  295 +++
  drivers/media/platform/tegra/tegra-core.h|  134 
  drivers/media/platform/tegra/tegra-vi.c  |  585 ++
  drivers/media/platform/tegra/tegra-vi.h  |  224 ++
  include/dt-bindings/media/tegra-vi.h |   35 +
  10 files changed, 2362 insertions(+)
  create mode 100644 drivers/media/platform/tegra/Kconfig
  create mode 100644 drivers/media/platform/tegra/Makefile
  create mode 100644 drivers/media/platform/tegra/tegra-channel.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.h
  create mode 100644 drivers/media/platform/tegra/tegra-vi.c
  create mode 100644 drivers/media/platform/tegra/tegra-vi.h
  create mode 100644 include/dt-bindings/media/tegra-vi.h
 
 I can't spot a device tree binding document for this, but we'll need one
 to properly review this driver.
 
 diff --git a/drivers/media/platform/tegra/Kconfig 
 b/drivers/media/platform/tegra/Kconfig
 new file mode 100644
 index 000..a69d1b2
 --- /dev/null
 +++ b/drivers/media/platform/tegra/Kconfig
 @@ -0,0 +1,9 @@
 +config VIDEO_TEGRA
 +tristate NVIDIA Tegra Video Input Driver (EXPERIMENTAL)
 
 I don't think the (EXPERIMENTAL) is warranted. Either the driver works
 or it doesn't. And I assume you already tested that it works, even if
 only using the TPG.
 
 +depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF
 
 This seems to be missing a couple of dependencies. For example I would
 expect at least TEGRA_HOST1X to be listed here to make sure people can't
 select this when the host1x API isn't available. I would also expect
 some sort of architecture dependency because it really makes no sense to
 build this if Tegra isn't supported.
 
 If you are concerned about compile coverage you can make that explicit
 using a COMPILE_TEST alternative such as:
 
   depends on ARCH_TEGRA || (ARM  COMPILE_TEST)
 
 Note that the ARM dependency in there makes sure that HAVE_IOMEM is
 selected, so this could also be:
 
   depends on ARCH_TEGRA || (HAVE_IOMEM  COMPILE_TEST)
 
 though that'd still leave open the possibility of build breakage because
 of some missing support.
 
 If you add the dependency on TEGRA_HOST1X that I mentioned above you
 shouldn't need any architecture dependency because TEGRA_HOST1X implies
 those already.
 
 +select VIDEOBUF2_DMA_CONTIG
 +---help---
 +  Driver for Video Input (VI) device controller in NVIDIA Tegra SoC.
 
 I'd reword this slightly as:
 
 Driver for the Video Input (VI) controller found on NVIDIA Tegra
 SoCs.
 
 +
 +  TO compile this driver as a module, choose M here: the module will be
 
 s/TO/To/.
 
 +  called tegra-video.
 
 diff --git a/drivers/media/platform/tegra/Makefile 
 b/drivers/media/platform/tegra/Makefile
 new file mode 100644
 index 000..c8eff0b
 --- /dev/null
 +++ b/drivers/media/platform/tegra/Makefile
 @@ -0,0 +1,3 @@
 +tegra-video-objs += tegra-core.o tegra-vi.o tegra-channel.o
 
 I'd personally leave out the redundant tegra- prefix here, because the
 files are in a tegra/ subdirectory already.

This is actually consistent with the other media drivers, so please keep the
prefix. One can debate whether the prefix makes sense or not, but changing
that would be a subsystem-wide change.

snip

 +static int tegra_channel_capture_frame(struct tegra_channel *chan)
 +{
 +struct tegra_channel_buffer *buf = chan-active;
 +struct vb2_buffer *vb = buf-buf;
 +int err = 0;
 +u32 thresh, value, frame_start;
 +int bytes_per_line = chan-format.bytesperline;
 +
 +if (!vb2_start_streaming_called(chan-queue) || !buf)
 +

Re: [PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-21 Thread Hans Verkuil
Hi Bryan,

Thanks for contributing this driver, very much appreciated.

I do have some comments below, basically about the same things we discussed
privately before.

On 08/21/2015 02:51 AM, Bryan Wu wrote:
 NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
 controller which can support up to 6 MIPI CSI camera sensors.
 
 This patch adds a V4L2 media controller and capture driver to support
 Tegra VI hardware. It's verified with Tegra built-in test pattern
 generator.
 
 Signed-off-by: Bryan Wu pe...@nvidia.com
 Reviewed-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/platform/Kconfig   |1 +
  drivers/media/platform/Makefile  |2 +
  drivers/media/platform/tegra/Kconfig |9 +
  drivers/media/platform/tegra/Makefile|3 +
  drivers/media/platform/tegra/tegra-channel.c | 1074 
 ++
  drivers/media/platform/tegra/tegra-core.c|  295 +++
  drivers/media/platform/tegra/tegra-core.h|  134 
  drivers/media/platform/tegra/tegra-vi.c  |  585 ++
  drivers/media/platform/tegra/tegra-vi.h  |  224 ++
  include/dt-bindings/media/tegra-vi.h |   35 +
  10 files changed, 2362 insertions(+)
  create mode 100644 drivers/media/platform/tegra/Kconfig
  create mode 100644 drivers/media/platform/tegra/Makefile
  create mode 100644 drivers/media/platform/tegra/tegra-channel.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.c
  create mode 100644 drivers/media/platform/tegra/tegra-core.h
  create mode 100644 drivers/media/platform/tegra/tegra-vi.c
  create mode 100644 drivers/media/platform/tegra/tegra-vi.h
  create mode 100644 include/dt-bindings/media/tegra-vi.h
 

snip

 +static int tegra_channel_capture_frame(struct tegra_channel *chan)
 +{
 + struct tegra_channel_buffer *buf = chan-active;
 + struct vb2_buffer *vb = buf-buf;
 + int err = 0;
 + u32 thresh, value, frame_start;
 + int bytes_per_line = chan-format.bytesperline;
 +
 + if (!vb2_start_streaming_called(chan-queue) || !buf)
 + return -EINVAL;
 +
 + if (chan-bypass)
 + goto bypass_done;
 +
 + /* Program buffer address */
 + csi_write(chan,
 +   TEGRA_VI_CSI_SURFACE0_OFFSET_MSB + chan-surface * 8,
 +   0x0);
 + csi_write(chan,
 +   TEGRA_VI_CSI_SURFACE0_OFFSET_LSB + chan-surface * 8,
 +   buf-addr);
 + csi_write(chan,
 +   TEGRA_VI_CSI_SURFACE0_STRIDE + chan-surface * 4,
 +   bytes_per_line);
 +
 + /* Program syncpoint */
 + frame_start = sp_bit(chan, SP_PP_FRAME_START);
 + tegra_channel_write(chan, TEGRA_VI_CFG_VI_INCR_SYNCPT,
 + frame_start | host1x_syncpt_id(chan-sp));
 +
 + csi_write(chan, TEGRA_VI_CSI_SINGLE_SHOT, 0x1);
 +
 + /* Use syncpoint to wake up */
 + thresh = host1x_syncpt_incr_max(chan-sp, 1);
 +
 + mutex_unlock(chan-lock);
 + err = host1x_syncpt_wait(chan-sp, thresh,
 +  TEGRA_VI_SYNCPT_WAIT_TIMEOUT, value);
 + mutex_lock(chan-lock);
 +
 + if (err) {
 + dev_err(chan-video.dev, frame start syncpt timeout!\n);
 + tegra_channel_capture_error(chan, err);
 + }
 +
 +bypass_done:
 + /* Captured one frame */
 + spin_lock_irq(chan-queued_lock);
 + vb-v4l2_buf.sequence = chan-sequence++;
 + vb-v4l2_buf.field = V4L2_FIELD_NONE;
 + v4l2_get_timestamp(vb-v4l2_buf.timestamp);
 + vb2_set_plane_payload(vb, 0, chan-format.sizeimage);
 + vb2_buffer_done(vb, err  0 ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 + spin_unlock_irq(chan-queued_lock);
 +
 + return err;
 +}
 +
 +static void tegra_channel_work(struct work_struct *work)
 +{
 + struct tegra_channel *chan =
 + container_of(work, struct tegra_channel, work);
 +
 + while (1) {
 + spin_lock_irq(chan-queued_lock);
 + if (list_empty(chan-capture)) {
 + chan-active = NULL;
 + spin_unlock_irq(chan-queued_lock);
 + return;
 + }
 + chan-active = list_entry(chan-capture.next,
 + struct tegra_channel_buffer, queue);
 + list_del_init(chan-active-queue);
 + spin_unlock_irq(chan-queued_lock);
 +
 + mutex_lock(chan-lock);
 + tegra_channel_capture_frame(chan);
 + mutex_unlock(chan-lock);
 + }
 +}
 +
 +/* 
 -
 + * videobuf2 queue operations
 + */
 +
 +static int
 +tegra_channel_queue_setup(struct vb2_queue *vq, const struct v4l2_format 
 *fmt,
 +  unsigned int *nbuffers, unsigned int *nplanes,
 +  unsigned int sizes[], void *alloc_ctxs[])
 +{
 + struct tegra_channel *chan = vb2_get_drv_priv(vq);
 +
 + /* Make sure the image size is large enough. */

[PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-20 Thread Bryan Wu
NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
controller which can support up to 6 MIPI CSI camera sensors.

This patch adds a V4L2 media controller and capture driver to support
Tegra VI hardware. It's verified with Tegra built-in test pattern
generator.

Signed-off-by: Bryan Wu pe...@nvidia.com
Reviewed-by: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/platform/Kconfig   |1 +
 drivers/media/platform/Makefile  |2 +
 drivers/media/platform/tegra/Kconfig |9 +
 drivers/media/platform/tegra/Makefile|3 +
 drivers/media/platform/tegra/tegra-channel.c | 1074 ++
 drivers/media/platform/tegra/tegra-core.c|  295 +++
 drivers/media/platform/tegra/tegra-core.h|  134 
 drivers/media/platform/tegra/tegra-vi.c  |  585 ++
 drivers/media/platform/tegra/tegra-vi.h  |  224 ++
 include/dt-bindings/media/tegra-vi.h |   35 +
 10 files changed, 2362 insertions(+)
 create mode 100644 drivers/media/platform/tegra/Kconfig
 create mode 100644 drivers/media/platform/tegra/Makefile
 create mode 100644 drivers/media/platform/tegra/tegra-channel.c
 create mode 100644 drivers/media/platform/tegra/tegra-core.c
 create mode 100644 drivers/media/platform/tegra/tegra-core.h
 create mode 100644 drivers/media/platform/tegra/tegra-vi.c
 create mode 100644 drivers/media/platform/tegra/tegra-vi.h
 create mode 100644 include/dt-bindings/media/tegra-vi.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f6bed19..553867f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -119,6 +119,7 @@ source drivers/media/platform/exynos4-is/Kconfig
 source drivers/media/platform/s5p-tv/Kconfig
 source drivers/media/platform/am437x/Kconfig
 source drivers/media/platform/xilinx/Kconfig
+source drivers/media/platform/tegra/Kconfig
 
 endif # V4L_PLATFORM_DRIVERS
 
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 114f9ab..426e0e4 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -52,4 +52,6 @@ obj-$(CONFIG_VIDEO_AM437X_VPFE)   += am437x/
 
 obj-$(CONFIG_VIDEO_XILINX) += xilinx/
 
+obj-$(CONFIG_VIDEO_TEGRA)  += tegra/
+
 ccflags-y += -I$(srctree)/drivers/media/i2c
diff --git a/drivers/media/platform/tegra/Kconfig 
b/drivers/media/platform/tegra/Kconfig
new file mode 100644
index 000..a69d1b2
--- /dev/null
+++ b/drivers/media/platform/tegra/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_TEGRA
+   tristate NVIDIA Tegra Video Input Driver (EXPERIMENTAL)
+   depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF
+   select VIDEOBUF2_DMA_CONTIG
+   ---help---
+ Driver for Video Input (VI) device controller in NVIDIA Tegra SoC.
+
+ TO compile this driver as a module, choose M here: the module will be
+ called tegra-video.
diff --git a/drivers/media/platform/tegra/Makefile 
b/drivers/media/platform/tegra/Makefile
new file mode 100644
index 000..c8eff0b
--- /dev/null
+++ b/drivers/media/platform/tegra/Makefile
@@ -0,0 +1,3 @@
+tegra-video-objs += tegra-core.o tegra-vi.o tegra-channel.o
+
+obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o
diff --git a/drivers/media/platform/tegra/tegra-channel.c 
b/drivers/media/platform/tegra/tegra-channel.c
new file mode 100644
index 000..b0063d2
--- /dev/null
+++ b/drivers/media/platform/tegra/tegra-channel.c
@@ -0,0 +1,1074 @@
+/*
+ * NVIDIA Tegra Video Input Device
+ *
+ * Copyright (c) 2015, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author: Bryan Wu pe...@nvidia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/atomic.h
+#include linux/bitmap.h
+#include linux/clk.h
+#include linux/delay.h
+#include linux/host1x.h
+#include linux/lcm.h
+#include linux/list.h
+#include linux/module.h
+#include linux/of.h
+#include linux/slab.h
+
+#include media/v4l2-ctrls.h
+#include media/v4l2-dev.h
+#include media/v4l2-fh.h
+#include media/v4l2-ioctl.h
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-contig.h
+
+#include soc/tegra/pmc.h
+
+#include tegra-vi.h
+
+#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT   200
+
+/* VI registers */
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT 0x000
+#defineSP_PP_LINE_START4
+#defineSP_PP_FRAME_START   5
+#defineSP_MW_REQ_DONE  6
+#defineSP_MW_ACK_DONE  7
+
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT_CNTRL   0x004
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT_ERROR   0x008
+#define TEGRA_VI_CFG_CTXSW  0x020
+#define TEGRA_VI_CFG_INTSTATUS

[PATCH 1/2] [media] v4l: tegra: Add NVIDIA Tegra VI driver

2015-08-20 Thread Bryan Wu
NVIDIA Tegra processor contains a powerful Video Input (VI) hardware
controller which can support up to 6 MIPI CSI camera sensors.

This patch adds a V4L2 media controller and capture driver to support
Tegra VI hardware. It's verified with Tegra built-in test pattern
generator.

Signed-off-by: Bryan Wu pe...@nvidia.com
Reviewed-by: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/platform/Kconfig   |1 +
 drivers/media/platform/Makefile  |2 +
 drivers/media/platform/tegra/Kconfig |9 +
 drivers/media/platform/tegra/Makefile|3 +
 drivers/media/platform/tegra/tegra-channel.c | 1074 ++
 drivers/media/platform/tegra/tegra-core.c|  295 +++
 drivers/media/platform/tegra/tegra-core.h|  134 
 drivers/media/platform/tegra/tegra-vi.c  |  585 ++
 drivers/media/platform/tegra/tegra-vi.h  |  224 ++
 include/dt-bindings/media/tegra-vi.h |   35 +
 10 files changed, 2362 insertions(+)
 create mode 100644 drivers/media/platform/tegra/Kconfig
 create mode 100644 drivers/media/platform/tegra/Makefile
 create mode 100644 drivers/media/platform/tegra/tegra-channel.c
 create mode 100644 drivers/media/platform/tegra/tegra-core.c
 create mode 100644 drivers/media/platform/tegra/tegra-core.h
 create mode 100644 drivers/media/platform/tegra/tegra-vi.c
 create mode 100644 drivers/media/platform/tegra/tegra-vi.h
 create mode 100644 include/dt-bindings/media/tegra-vi.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f6bed19..553867f 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -119,6 +119,7 @@ source drivers/media/platform/exynos4-is/Kconfig
 source drivers/media/platform/s5p-tv/Kconfig
 source drivers/media/platform/am437x/Kconfig
 source drivers/media/platform/xilinx/Kconfig
+source drivers/media/platform/tegra/Kconfig
 
 endif # V4L_PLATFORM_DRIVERS
 
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 114f9ab..426e0e4 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -52,4 +52,6 @@ obj-$(CONFIG_VIDEO_AM437X_VPFE)   += am437x/
 
 obj-$(CONFIG_VIDEO_XILINX) += xilinx/
 
+obj-$(CONFIG_VIDEO_TEGRA)  += tegra/
+
 ccflags-y += -I$(srctree)/drivers/media/i2c
diff --git a/drivers/media/platform/tegra/Kconfig 
b/drivers/media/platform/tegra/Kconfig
new file mode 100644
index 000..a69d1b2
--- /dev/null
+++ b/drivers/media/platform/tegra/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_TEGRA
+   tristate NVIDIA Tegra Video Input Driver (EXPERIMENTAL)
+   depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  OF
+   select VIDEOBUF2_DMA_CONTIG
+   ---help---
+ Driver for Video Input (VI) device controller in NVIDIA Tegra SoC.
+
+ TO compile this driver as a module, choose M here: the module will be
+ called tegra-video.
diff --git a/drivers/media/platform/tegra/Makefile 
b/drivers/media/platform/tegra/Makefile
new file mode 100644
index 000..c8eff0b
--- /dev/null
+++ b/drivers/media/platform/tegra/Makefile
@@ -0,0 +1,3 @@
+tegra-video-objs += tegra-core.o tegra-vi.o tegra-channel.o
+
+obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o
diff --git a/drivers/media/platform/tegra/tegra-channel.c 
b/drivers/media/platform/tegra/tegra-channel.c
new file mode 100644
index 000..b0063d2
--- /dev/null
+++ b/drivers/media/platform/tegra/tegra-channel.c
@@ -0,0 +1,1074 @@
+/*
+ * NVIDIA Tegra Video Input Device
+ *
+ * Copyright (c) 2015, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author: Bryan Wu pe...@nvidia.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/atomic.h
+#include linux/bitmap.h
+#include linux/clk.h
+#include linux/delay.h
+#include linux/host1x.h
+#include linux/lcm.h
+#include linux/list.h
+#include linux/module.h
+#include linux/of.h
+#include linux/slab.h
+
+#include media/v4l2-ctrls.h
+#include media/v4l2-dev.h
+#include media/v4l2-fh.h
+#include media/v4l2-ioctl.h
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-contig.h
+
+#include soc/tegra/pmc.h
+
+#include tegra-vi.h
+
+#define TEGRA_VI_SYNCPT_WAIT_TIMEOUT   200
+
+/* VI registers */
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT 0x000
+#defineSP_PP_LINE_START4
+#defineSP_PP_FRAME_START   5
+#defineSP_MW_REQ_DONE  6
+#defineSP_MW_ACK_DONE  7
+
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT_CNTRL   0x004
+#define TEGRA_VI_CFG_VI_INCR_SYNCPT_ERROR   0x008
+#define TEGRA_VI_CFG_CTXSW  0x020
+#define TEGRA_VI_CFG_INTSTATUS