[PATCH] media: coda: don't overwrite h.264 profile_idc on decoder instance

[Lucas Stach]

On a decoder instance, after the profile has been parsed from the stream
__v4l2_ctrl_s_ctrl() is called to notify userspace about changes in the
read-only profile control. This ends up calling back into the CODA driver
where a mssing check on the s_ctrl caused the profile information that has
just been parsed from the stream to be overwritten with the default
baseline profile.

Later on the driver fails to enable frame reordering, based on the wrong
profile information.

Fixes: 347de126d1da (media: coda: add read-only h.264 decoder
 profile/level controls)
Signed-off-by: Lucas Stach 
---
 drivers/media/platform/coda/coda-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index c7631e117dd3..1ae15d4ec5ed 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1719,7 +1719,8 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
/* TODO: switch between baseline and constrained baseline */
-   ctx->params.h264_profile_idc = 66;
+   if (ctx->inst_type == CODA_INST_ENCODER)
+   ctx->params.h264_profile_idc = 66;
break;
case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
/* nothing to do, this is set by the encoder */
-- 
2.18.0

Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error

[Lucas Stach]

Am Mittwoch, den 16.05.2018, 11:42 +0200 schrieb Daniel Vetter:
> On Tue, May 15, 2018 at 01:16:30PM +0100, Chris Wilson wrote:
> > Quoting Ezequiel Garcia (2018-05-14 22:28:31)
> > > On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote:
> > > > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote:
> > > > > Quoting Ezequiel Garcia (2018-05-09 21:14:49)
> > > > > > Change how dma_fence_add_callback() behaves, when the fence
> > > > > > has error-signaled by the time it is being add. After this commit,
> > > > > > dma_fence_add_callback() returns the fence error, if it
> > > > > > has error-signaled before dma_fence_add_callback() is called.
> > > > > 
> > > > > Why? What problem are you trying to solve? fence->error does not imply
> > > > > that the fence has yet been signaled, and the caller wants a callback
> > > > > when it is signaled.
> > > > 
> > > > On top this is incosistent, e.g. we don't do the same for any of the 
> > > > other
> > > > dma_fence interfaces. Plus there's the issue that you might alias errno
> > > > values with fence errno values.
> > > > 
> > > 
> > > Right.
> > > 
> > > > I think keeping the error codes from the functions you're calling 
> > > > distinct
> > > > from the error code of the fence itself makes a lot of sense. The first
> > > > tells you whether your request worked out (or why not), the second tells
> > > > you whether the asynchronous dma operation (gpu rendering, page flip,
> > > > whatever) that the dma_fence represents worked out (or why not). That's 
> > > > 2
> > > > distinct things imo.
> > > > 
> > > > Might be good to show us the driver code that needs this behaviour so we
> > > > can discuss how to best handle your use-case.
> > > > 
> > > 
> > > This change arose while discussing the in-fences support for video4linux.
> > > Here's the patch that calls dma_fence_add_callback 
> > > https://lkml.org/lkml/2018/5/4/766.
> > > 
> > > The code snippet currently looks something like this:
> > > 
> > > if (vb->in_fence) {
> > > ret = dma_fence_add_callback(vb->in_fence, >fence_cb,
> > > 
> > >  vb2_qbuf_fence_cb);
> > > /* is the fence signaled? */
> > > if (ret == -ENOENT) {
> > > 
> > > dma_fence_put(vb->in_fence);
> > > vb->in_fence = NULL;
> > > } else if (ret)
> > > {
> > > goto unlock;
> > > }
> > > }
> > > 
> > > In this use case, if the callback is added successfully,
> > > the video4linux core defers the activation of the buffer
> > > until the fence signals.
> > > 
> > > If the fence is signaled (currently disregarding of errors)
> > > then the buffer is assumed to be ready to be activated,
> > > and so it gets queued for hardware usage.
> > > 
> > > Giving some more thought to this, I'm not so sure what is
> > > the right action if a fence signaled with error. In this case,
> > > it appears to me that we shouldn't be using this buffer
> > > if its in-fence is in error, but perhaps I'm missing
> > > something.
> > 
> > What I have in mind for async errors is to skip the operation and
> > propagate the error onto the next fence. Mostly because those async
> > errors may include fatal errors such as unable to pin the backing
> > storage for the operation, but even "trivial" errors such as an early
> > operation failing means that this request is then subject to garbage-in,
> > garbage-out. However, for trivial errors I would just propagate the
> > error status (so the caller knows something went wrong if they care, but
> > in all likelihood no one will notice) and continue on with the glitchy
> > operation.
> 
> In general, there's not really any hard rule about propagating fence
> errors across devices. It's mostly just used by drivers internally to keep
> track of failed stuff (gpu hangs or anything else async like Chris
> describes here).
> 
> For v4l I'm not sure you want to care much about this, since right now the
> main use of fence errors is gpu hang recovery (whether it's the driver or
> hw that's hung doesn't matter here).

Yes, my understanding is that fence signal and errors are two distinct
things that should not be conflated like it is done in this patch.

In my understanding signaling a fence means the HW or SW component
which added the fence is done with the buffer and will not touch it
anymore. In case of an unrecoverable error the fence will be signaled
with error status set, so we correctly reflect the buffer status as
being free to be used by whoever is waiting for it, but may contain
garbage.

If a fence waiter cares about the buffer content and may wish to skip
its operation if the fence is signaled with an error it should do it by
explicitly checking the fence error status, instead of making this
implicit behavior.

Regards,
Lucas

Re: [Linaro-mm-sig] [PATCH 4/8] dma-buf: add peer2peer flag

[Lucas Stach]

Am Mittwoch, den 25.04.2018, 13:44 -0400 schrieb Alex Deucher:
> On Wed, Apr 25, 2018 at 2:41 AM, Christoph Hellwig  > wrote:
> > On Wed, Apr 25, 2018 at 02:24:36AM -0400, Alex Deucher wrote:
> > > > It has a non-coherent transaction mode (which the chipset can opt to
> > > > not implement and still flush), to make sure the AGP horror show
> > > > doesn't happen again and GPU folks are happy with PCIe. That's at
> > > > least my understanding from digging around in amd the last time we had
> > > > coherency issues between intel and amd gpus. GPUs have some bits
> > > > somewhere (in the pagetables, or in the buffer object description
> > > > table created by userspace) to control that stuff.
> > > 
> > > Right.  We have a bit in the GPU page table entries that determines
> > > whether we snoop the CPU's cache or not.
> > 
> > I can see how that works with the GPU on the same SOC or SOC set as the
> > CPU.  But how is that going to work for a GPU that is a plain old PCIe
> > card?  The cache snooping in that case is happening in the PCIe root
> > complex.
> 
> I'm not a pci expert, but as far as I know, the device sends either a
> snooped or non-snooped transaction on the bus.  I think the
> transaction descriptor supports a no snoop attribute.  Our GPUs have
> supported this feature for probably 20 years if not more, going back
> to PCI.  Using non-snooped transactions have lower latency and faster
> throughput compared to snooped transactions.

PCI-X (as in the thing which as all the rage before PCIe) added a
attribute phase to each transaction where the requestor can enable
relaxed ordering and/or no-snoop on a transaction basis. As those are
strictly performance optimizations the root-complex isn't required to
honor those attributes, but implementations that care about performance
 usually will.

Regards,
Lucas

Re: [Linaro-mm-sig] [PATCH] dma-buf: add some lockdep asserts to the reservation object implementation

[Lucas Stach]

Am Donnerstag, den 11.01.2018, 11:54 +0100 schrieb Christian König:
> Yeah, somehow missed that one.
> 
> The patch looks mostly good, except for reservation_object_get_excl().
> 
> For that one an RCU protection is usually sufficient, so annotating it 
> with reservation_object_assert_held() sounds incorrect to me.

Ah, you are correct. I was confused about this one as
reservation_object_get_excl_rcu() exists and and the doc
above reservation_object_get_excl() states "The obj->lock must be
held.", which is misleading for the read-only case.

I'll send a v2 with that fixed.

Regards,
Lucas

> Regards,
> Christian.
> 
> Am 11.01.2018 um 11:43 schrieb Lucas Stach:
> > Did this fall through the cracks over the holidays? It really has made
> > my work much easier while reworking some of the reservation object
> > handling in etnaviv and I think it might benefit others.
> > 
> > Regards,
> > Lucas
> > 
> > Am Freitag, den 01.12.2017, 12:12 +0100 schrieb Lucas Stach:
> > > This adds lockdep asserts to the reservation functions which state in 
> > > their
> > > documentation that obj->lock must be held. Allows builds with 
> > > PROVE_LOCKING
> > > enabled to check that the locking requirements are met.
> > > 
> > > > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > > 
> > > ---
> > >   drivers/dma-buf/reservation.c | 8 
> > >   include/linux/reservation.h   | 2 ++
> > >   2 files changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> > > index b44d9d7db347..accd398e2ea6 100644
> > > --- a/drivers/dma-buf/reservation.c
> > > +++ b/drivers/dma-buf/reservation.c
> > > @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct 
> > > reservation_object *obj)
> > > > > > > >     struct reservation_object_list *fobj, *old;
> > > >     u32 max;
> > > 
> > >   
> > > > +   reservation_object_assert_held(obj);
> > > 
> > > +
> > > >     old = reservation_object_get_list(obj);
> > > 
> > >   
> > > >     if (old && old->shared_max) {
> > > 
> > > @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct 
> > > reservation_object *obj,
> > >   {
> > > >     struct reservation_object_list *old, *fobj = obj->staged;
> > > 
> > >   
> > > > +   reservation_object_assert_held(obj);
> > > 
> > > +
> > > > > > > >     old = reservation_object_get_list(obj);
> > > >     obj->staged = NULL;
> > > 
> > >   
> > > @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct 
> > > reservation_object *obj,
> > > > > > > >     struct reservation_object_list *old;
> > > >     u32 i = 0;
> > > 
> > >   
> > > > +   reservation_object_assert_held(obj);
> > > 
> > > +
> > > > > > > >     old = reservation_object_get_list(obj);
> > > > > > > >     if (old)
> > > >     i = old->shared_count;
> > > 
> > > @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct 
> > > reservation_object *dst,
> > > > > > > >     size_t size;
> > > >     unsigned i;
> > > 
> > >   
> > > > +   reservation_object_assert_held(dst);
> > > 
> > > +
> > > > > > > >     rcu_read_lock();
> > > >     src_list = rcu_dereference(src->fence);
> > > 
> > >   
> > > diff --git a/include/linux/reservation.h b/include/linux/reservation.h
> > > index 21fc84d82d41..55e7318800fd 100644
> > > --- a/include/linux/reservation.h
> > > +++ b/include/linux/reservation.h
> > > @@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object 
> > > *obj)
> > >   static inline struct dma_fence *
> > >   reservation_object_get_excl(struct reservation_object *obj)
> > >   {
> > > > +   reservation_object_assert_held(obj);
> > > 
> > > +
> > > > > > > >     return rcu_dereference_protected(obj->fence_excl,
> > > >      reservation_object_held(obj));
> > > 
> > >   }
> > 
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 

Re: [Linaro-mm-sig] [PATCH] dma-buf: add some lockdep asserts to the reservation object implementation

[Lucas Stach]

Did this fall through the cracks over the holidays? It really has made
my work much easier while reworking some of the reservation object
handling in etnaviv and I think it might benefit others.

Regards,
Lucas

Am Freitag, den 01.12.2017, 12:12 +0100 schrieb Lucas Stach:
> This adds lockdep asserts to the reservation functions which state in their
> documentation that obj->lock must be held. Allows builds with PROVE_LOCKING
> enabled to check that the locking requirements are met.
> 
> > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> ---
>  drivers/dma-buf/reservation.c | 8 
>  include/linux/reservation.h   | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index b44d9d7db347..accd398e2ea6 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct 
> reservation_object *obj)
> >     struct reservation_object_list *fobj, *old;
> >     u32 max;
>  
> > +   reservation_object_assert_held(obj);
> +
> >     old = reservation_object_get_list(obj);
>  
> >     if (old && old->shared_max) {
> @@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct 
> reservation_object *obj,
>  {
> >     struct reservation_object_list *old, *fobj = obj->staged;
>  
> > +   reservation_object_assert_held(obj);
> +
> >     old = reservation_object_get_list(obj);
> >     obj->staged = NULL;
>  
> @@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct 
> reservation_object *obj,
> >     struct reservation_object_list *old;
> >     u32 i = 0;
>  
> > +   reservation_object_assert_held(obj);
> +
> >     old = reservation_object_get_list(obj);
> >     if (old)
> >     i = old->shared_count;
> @@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct 
> reservation_object *dst,
> >     size_t size;
> >     unsigned i;
>  
> > +   reservation_object_assert_held(dst);
> +
> >     rcu_read_lock();
> >     src_list = rcu_dereference(src->fence);
>  
> diff --git a/include/linux/reservation.h b/include/linux/reservation.h
> index 21fc84d82d41..55e7318800fd 100644
> --- a/include/linux/reservation.h
> +++ b/include/linux/reservation.h
> @@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object *obj)
>  static inline struct dma_fence *
>  reservation_object_get_excl(struct reservation_object *obj)
>  {
> > +   reservation_object_assert_held(obj);
> +
> >     return rcu_dereference_protected(obj->fence_excl,
> >      reservation_object_held(obj));
>  }

[PATCH] dma-buf: add some lockdep asserts to the reservation object implementation

[Lucas Stach]

This adds lockdep asserts to the reservation functions which state in their
documentation that obj->lock must be held. Allows builds with PROVE_LOCKING
enabled to check that the locking requirements are met.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/dma-buf/reservation.c | 8 
 include/linux/reservation.h   | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b44d9d7db347..accd398e2ea6 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct 
reservation_object *obj)
struct reservation_object_list *fobj, *old;
u32 max;
 
+   reservation_object_assert_held(obj);
+
old = reservation_object_get_list(obj);
 
if (old && old->shared_max) {
@@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct 
reservation_object *obj,
 {
struct reservation_object_list *old, *fobj = obj->staged;
 
+   reservation_object_assert_held(obj);
+
old = reservation_object_get_list(obj);
obj->staged = NULL;
 
@@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct 
reservation_object *obj,
struct reservation_object_list *old;
u32 i = 0;
 
+   reservation_object_assert_held(obj);
+
old = reservation_object_get_list(obj);
if (old)
i = old->shared_count;
@@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct 
reservation_object *dst,
size_t size;
unsigned i;
 
+   reservation_object_assert_held(dst);
+
rcu_read_lock();
src_list = rcu_dereference(src->fence);
 
diff --git a/include/linux/reservation.h b/include/linux/reservation.h
index 21fc84d82d41..55e7318800fd 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object *obj)
 static inline struct dma_fence *
 reservation_object_get_excl(struct reservation_object *obj)
 {
+   reservation_object_assert_held(obj);
+
return rcu_dereference_protected(obj->fence_excl,
 reservation_object_held(obj));
 }
-- 
2.11.0

Re: [Linaro-mm-sig] [PATCH] dma-fence: Don't BUG_ON when not absolutely needed

[Lucas Stach]

Am Donnerstag, den 20.07.2017, 14:51 +0200 schrieb Daniel Vetter:
> It makes debugging a massive pain.

It is also considered very bad style to BUG the kernel on anything other
than filesystem eating catastrophic failures.

Reviewed-by: Lucas Stach <l.st...@pengutronix.de>

> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Sumit Semwal <sumit.sem...@linaro.org>
> Cc: Gustavo Padovan <gust...@padovan.org>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-...@lists.linaro.org
> ---
>  drivers/dma-buf/dma-fence.c | 4 ++--
>  include/linux/dma-fence.h   | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 56e0a0e1b600..9a302799040e 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -48,7 +48,7 @@ static atomic64_t dma_fence_context_counter = 
> ATOMIC64_INIT(0);
>   */
>  u64 dma_fence_context_alloc(unsigned num)
>  {
> - BUG_ON(!num);
> + WARN_ON(!num);
>   return atomic64_add_return(num, _fence_context_counter) - num;
>  }
>  EXPORT_SYMBOL(dma_fence_context_alloc);
> @@ -172,7 +172,7 @@ void dma_fence_release(struct kref *kref)
>  
>   trace_dma_fence_destroy(fence);
>  
> - BUG_ON(!list_empty(>cb_list));
> + WARN_ON(!list_empty(>cb_list));
>  
>   if (fence->ops->release)
>   fence->ops->release(fence);
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 9342cf0dada4..171895072435 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -431,8 +431,8 @@ int dma_fence_get_status(struct dma_fence *fence);
>  static inline void dma_fence_set_error(struct dma_fence *fence,
>  int error)
>  {
> - BUG_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags));
> - BUG_ON(error >= 0 || error < -MAX_ERRNO);
> + WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags));
> + WARN_ON(error >= 0 || error < -MAX_ERRNO);
>  
>   fence->error = error;
>  }

Re: TW686x Linux Main Line Driver Issue

[Lucas Stach]

Am Donnerstag, den 20.04.2017, 11:12 -0300 schrieb Ezequiel Garcia:
> On 20 April 2017 at 07:10, Anuradha Ranasinghe  
> wrote:
> > Dear All,
> >
> > This issue is associated to the Linux Mainline Kernel 4.1.15.2 (branch2)
> > tw686x upstream driver and IMX6Q platform.
> >
> > We have an analog camera capture board (a custom one) based around tw6865.
> > We are interfacing it with Nitrogen6_Max board (IMX6Q) . We use the
> > aforementioned kernel with the boundary devices latest patches to the tw686x
> > driver (having 3 DMA buffers) and system running on Ubuntu 16 Xenial Mate
> > version.
> > https://github.com/boundarydevices/linux-imx6/commits/7fcd22da6d731b36e5ab856551c41301fca9881f
> >
> > The driver initialization, device and composite signal detection work well
> > as intended. But when the streaming started, frame rate becomes lower and
> > after few frames, the whole system freezes. To get the camera to work to
> > this level, we had to do :
> >
> 
> What dma-mode are you using? Have you tried other dma-modes?
> 
> How many frames do you manage to obtain? I believe you should
> debug this further and provide more information.
> 
> > 1. Disable PCI interrupts from the kernel (from menuconfig and pci=nomsi
> > kernel command)
> 
> (CCing PCI people) Lucas, Richard: any idea about why is this parameter 
> needed?

Does the device support MSI IRQs?

If it only supports legacy IRQs this might be a known issue, where
legacy IRQs won't work with the designware host controller if MSI IRQs
are configured. I'm working on a patch to get around this issue.

Regards,
Lucas

Re: [PATCH] [media] coda: do not enumerate YUYV if VDOA is not available

[Lucas Stach]

Am Donnerstag, den 06.04.2017, 16:03 +0200 schrieb Philipp Zabel:
> TRY_FMT already disables the YUYV format if the VDOA is not available.
> ENUM_FMT must do the same.
> 
> Fixes: d40e98c13b3e ("[media] coda: support YUYV output if VDOA is used")
> Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>

Reviewed-by: Lucas Stach <l.st...@pengutronix.de>

> ---
>  drivers/media/platform/coda/coda-common.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c 
> b/drivers/media/platform/coda/coda-common.c
> index eb6548f46cbac..1c155b055ea30 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -386,6 +386,7 @@ static int coda_enum_fmt(struct file *file, void *priv,
>  {
>   struct video_device *vdev = video_devdata(file);
>   const struct coda_video_device *cvd = to_coda_video_device(vdev);
> + struct coda_ctx *ctx = fh_to_ctx(priv);
>   const u32 *formats;
>  
>   if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> @@ -398,6 +399,11 @@ static int coda_enum_fmt(struct file *file, void *priv,
>   if (f->index >= CODA_MAX_FORMATS || formats[f->index] == 0)
>   return -EINVAL;
>  
> + /* Skip YUYV if the vdoa is not available */
> + if (!ctx->vdoa && f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> + formats[f->index] == V4L2_PIX_FMT_YUYV)
> + return -EINVAL;
> +
>   f->pixelformat = formats[f->index];
>  
>   return 0;

[PATCH] [media] coda: bump maximum number of internal framebuffers to 17

[Lucas Stach]

The h.264 standard allows up to 16 reference frame for the high profile
and we need one additional internal framebuffer when the VDOA is in use.

Lift the current maximum of 8 internal framebuffers to allow playback
of those video streams.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/platform/coda/coda.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda.h 
b/drivers/media/platform/coda/coda.h
index 799ffca72203..2a5bbe0050bb 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -28,7 +28,7 @@
 
 #include "coda_regs.h"
 
-#define CODA_MAX_FRAMEBUFFERS  8
+#define CODA_MAX_FRAMEBUFFERS  17
 #define FMO_SLICE_SAVE_BUF_SIZE(32)
 
 enum {
-- 
2.11.0

[PATCH 3/3] [media] coda/imx-vdoa: always wait for job completion

[Lucas Stach]

As long as only one CODA context is running we get alternating device_run()
and wait_for_completion() calls, but when more then one CODA context is
active, other VDOA slots can be inserted between those calls for one context.

Make sure to wait on job completion before running a different context and
before destroying the currently active context.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/platform/coda/imx-vdoa.c | 49 +++---
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/coda/imx-vdoa.c 
b/drivers/media/platform/coda/imx-vdoa.c
index 67fd8ffa60a4..ab69a0a9d38b 100644
--- a/drivers/media/platform/coda/imx-vdoa.c
+++ b/drivers/media/platform/coda/imx-vdoa.c
@@ -101,6 +101,8 @@ struct vdoa_ctx {
struct vdoa_data*vdoa;
struct completion   completion;
struct vdoa_q_data  q_data[2];
+   unsigned intsubmitted_job;
+   unsigned intcompleted_job;
 };
 
 static irqreturn_t vdoa_irq_handler(int irq, void *data)
@@ -114,7 +116,7 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
 
curr_ctx = vdoa->curr_ctx;
if (!curr_ctx) {
-   dev_dbg(vdoa->dev,
+   dev_warn(vdoa->dev,
"Instance released before the end of transaction\n");
return IRQ_HANDLED;
}
@@ -127,19 +129,44 @@ static irqreturn_t vdoa_irq_handler(int irq, void *data)
} else if (!(val & VDOAIST_EOT)) {
dev_warn(vdoa->dev, "Spurious interrupt\n");
}
+   curr_ctx->completed_job++;
complete(_ctx->completion);
 
return IRQ_HANDLED;
 }
 
+int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
+{
+   struct vdoa_data *vdoa = ctx->vdoa;
+
+   if (ctx->submitted_job == ctx->completed_job)
+   return 0;
+
+   if (!wait_for_completion_timeout(>completion,
+msecs_to_jiffies(300))) {
+   dev_err(vdoa->dev,
+   "Timeout waiting for transfer result\n");
+   return -ETIMEDOUT;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(vdoa_wait_for_completion);
+
 void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, dma_addr_t src)
 {
struct vdoa_q_data *src_q_data, *dst_q_data;
struct vdoa_data *vdoa = ctx->vdoa;
u32 val;
 
+   if (vdoa->curr_ctx)
+   vdoa_wait_for_completion(vdoa->curr_ctx);
+
vdoa->curr_ctx = ctx;
 
+   reinit_completion(>completion);
+   ctx->submitted_job++;
+
src_q_data = >q_data[V4L2_M2M_SRC];
dst_q_data = >q_data[V4L2_M2M_DST];
 
@@ -177,21 +204,6 @@ void vdoa_device_run(struct vdoa_ctx *ctx, dma_addr_t dst, 
dma_addr_t src)
 }
 EXPORT_SYMBOL(vdoa_device_run);
 
-int vdoa_wait_for_completion(struct vdoa_ctx *ctx)
-{
-   struct vdoa_data *vdoa = ctx->vdoa;
-
-   if (!wait_for_completion_timeout(>completion,
-msecs_to_jiffies(300))) {
-   dev_err(vdoa->dev,
-   "Timeout waiting for transfer result\n");
-   return -ETIMEDOUT;
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(vdoa_wait_for_completion);
-
 struct vdoa_ctx *vdoa_context_create(struct vdoa_data *vdoa)
 {
struct vdoa_ctx *ctx;
@@ -218,6 +230,11 @@ void vdoa_context_destroy(struct vdoa_ctx *ctx)
 {
struct vdoa_data *vdoa = ctx->vdoa;
 
+   if (vdoa->curr_ctx == ctx) {
+   vdoa_wait_for_completion(vdoa->curr_ctx);
+   vdoa->curr_ctx = NULL;
+   }
+
clk_disable_unprepare(vdoa->vdoa_clk);
kfree(ctx);
 }
-- 
2.11.0

[PATCH 1/3] [media] coda: use correct offset for mvcol buffer

[Lucas Stach]

The mvcol buffer needs to be placed behind the chroma plane(s), so
use the real offset including any required rounding.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 466a44e4549e..36062fc494e3 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -387,14 +387,16 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
 
/* Register frame buffers in the parameter buffer */
for (i = 0; i < ctx->num_internal_frames; i++) {
-   u32 y, cb, cr;
+   u32 y, cb, cr, mvcol;
 
/* Start addresses of Y, Cb, Cr planes */
y = ctx->internal_frames[i].paddr;
cb = y + ysize;
cr = y + ysize + ysize/4;
+   mvcol = y + ysize + ysize/4 + ysize/4;
if (ctx->tiled_map_type == GDI_TILED_FRAME_MB_RASTER_MAP) {
cb = round_up(cb, 4096);
+   mvcol = cb + ysize/2;
cr = 0;
/* Packed 20-bit MSB of base addresses */
/* YCCC, CCyc,  */
@@ -408,9 +410,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
/* mvcol buffer for h.264 */
if (ctx->codec->src_fourcc == V4L2_PIX_FMT_H264 &&
dev->devtype->product != CODA_DX6)
-   coda_parabuf_write(ctx, 96 + i,
-  ctx->internal_frames[i].paddr +
-  ysize + ysize/4 + ysize/4);
+   coda_parabuf_write(ctx, 96 + i, mvcol);
}
 
/* mvcol buffer for mpeg4 */
-- 
2.11.0

[PATCH 2/3] [media] coda: first step at error recovery

[Lucas Stach]

This implements a simple handler for the case where decode did not finish
sucessfully. This might be helpful during normal streaming, but for now it
only handles the case where the context would deadlock with userspace,
i.e. userspace issued DEC_CMD_STOP and waits for EOS, but after the failed
decode run we would hold the context and wait for userspace to queue more
buffers.

Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c| 20 
 drivers/media/platform/coda/coda-common.c |  3 +++
 drivers/media/platform/coda/coda.h|  1 +
 3 files changed, 24 insertions(+)

diff --git a/drivers/media/platform/coda/coda-bit.c 
b/drivers/media/platform/coda/coda-bit.c
index 36062fc494e3..6a088f9343bb 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2113,12 +2113,32 @@ static void coda_finish_decode(struct coda_ctx *ctx)
ctx->display_idx = display_idx;
 }
 
+static void coda_error_decode(struct coda_ctx *ctx)
+{
+   struct vb2_v4l2_buffer *dst_buf;
+
+   /*
+* For now this only handles the case where we would deadlock with
+* userspace, i.e. userspace issued DEC_CMD_STOP and waits for EOS,
+* but after a failed decode run we would hold the context and wait for
+* userspace to queue more buffers.
+*/
+   if (!(ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG))
+   return;
+
+   dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+   dst_buf->sequence = ctx->qsequence - 1;
+
+   coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_ERROR);
+}
+
 const struct coda_context_ops coda_bit_decode_ops = {
.queue_init = coda_decoder_queue_init,
.reqbufs = coda_decoder_reqbufs,
.start_streaming = coda_start_decoding,
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
+   .error_run = coda_error_decode,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
 };
diff --git a/drivers/media/platform/coda/coda-common.c 
b/drivers/media/platform/coda/coda-common.c
index eb6548f46cba..0bbf155f9783 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1100,6 +1100,9 @@ static void coda_pic_run_work(struct work_struct *work)
ctx->hold = true;
 
coda_hw_reset(ctx);
+
+   if (ctx->ops->error_run)
+   ctx->ops->error_run(ctx);
} else if (!ctx->aborting) {
ctx->ops->finish_run(ctx);
}
diff --git a/drivers/media/platform/coda/coda.h 
b/drivers/media/platform/coda/coda.h
index 4b831c91ae4a..799ffca72203 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -180,6 +180,7 @@ struct coda_context_ops {
int (*start_streaming)(struct coda_ctx *ctx);
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
+   void (*error_run)(struct coda_ctx *ctx);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
 };
-- 
2.11.0

Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver

[Lucas Stach]

Am Mittwoch, den 05.04.2017, 13:18 +0200 schrieb Pavel Machek:
> Hi!
> 
> > + * video stream multiplexer controlled via gpio or syscon
> > + *
> > + * Copyright (C) 2013 Pengutronix, Sascha Hauer 
> > + * Copyright (C) 2016 Pengutronix, Philipp Zabel 
> 
> This is actually quite interesting. Same email address for two
> people...
> 
> Plus, I believe this wants to say that copyright is with Pengutronix,
> not Sascha and Philipp. In that case you probably want to list
> copyright and authors separately?
> 
Nope, copyright doesn't get transferred to the employer within the rules
of the German "Urheberrecht", but stays at the original author of the
code.
Same email is just to ensure that any requests regarding this code get
routed to the right people, even if someone leaves the company or is
temporarily unavailable. kernel@ is a list for the Pengutronix kernel
team.

Regards,
Lucas

Re: [PATCH v3 3/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Lucas Stach]

Am Montag, den 14.03.2016, 15:19 -0300 schrieb Javier Martinez Canillas:
> Hello Lucas,
> 
> On Mon, Mar 14, 2016 at 12:23 PM, Lucas Stach <l.st...@pengutronix.de> wrote:
> > From: Philipp Zabel <p.za...@pengutronix.de>
> >
> > By looking at the endpoint flags, it can be determined whether the link
> > should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
> > dedicated HSYNC/VSYNC outputs in BT.656 mode.
> >
> > For devices that are not instantiated through DT the current behavior
> > is preserved.
> >
> > Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
> > Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> > ---
> 
> Similar to Mauro's comment on patch 2/9, the current driver already
> supports configuring the interface output format using DT.

Sorry about that. I've lost touch with the current media tree, will look
into what is already there. Sorry for the noise.


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

[PATCH v3 4/9] [media] tvp5150: fix standard autodetection

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

Make sure to not overwrite decoder->norm when setting the standard
in hardware, but only when instructed by V4L2 API calls.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 56 +
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f6720d1d09ea..21d044b564ad 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -703,8 +703,6 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
struct tvp5150 *decoder = to_tvp5150(sd);
int fmt = 0;
 
-   decoder->norm = std;
-
/* First tests should be against specific std */
 
if (std == V4L2_STD_NTSC_443) {
@@ -741,13 +739,37 @@ static int tvp5150_s_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
else
decoder->rect.height = TVP5150_V_MAX_OTHERS;
 
+   decoder->norm = std;
 
return tvp5150_set_std(sd, std);
 }
 
+static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
+{
+   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
+
+   switch (val & 0x0F) {
+   case 0x01:
+   return V4L2_STD_NTSC;
+   case 0x03:
+   return V4L2_STD_PAL;
+   case 0x05:
+   return V4L2_STD_PAL_M;
+   case 0x07:
+   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
+   case 0x09:
+   return V4L2_STD_NTSC_443;
+   case 0xb:
+   return V4L2_STD_SECAM;
+   default:
+   return V4L2_STD_UNKNOWN;
+   }
+}
+
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
 
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
@@ -783,7 +805,13 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
-   tvp5150_set_std(sd, decoder->norm);
+   if (decoder->norm == V4L2_STD_ALL)
+   std = tvp5150_read_std(sd);
+   else
+   std = decoder->norm;
+
+   /* Disable autoswitch mode */
+   tvp5150_set_std(sd, std);
return 0;
 };
 
@@ -808,28 +836,6 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
return -EINVAL;
 }
 
-static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
-{
-   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
-
-   switch (val & 0x0F) {
-   case 0x01:
-   return V4L2_STD_NTSC;
-   case 0x03:
-   return V4L2_STD_PAL;
-   case 0x05:
-   return V4L2_STD_PAL_M;
-   case 0x07:
-   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
-   case 0x09:
-   return V4L2_STD_NTSC_443;
-   case 0xb:
-   return V4L2_STD_SECAM;
-   default:
-   return V4L2_STD_UNKNOWN;
-   }
-}
-
 static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
-- 
2.7.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

[PATCH v3 3/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

By looking at the endpoint flags, it can be determined whether the link
should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
dedicated HSYNC/VSYNC outputs in BT.656 mode.

For devices that are not instantiated through DT the current behavior
is preserved.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 67312c9d83c1..f6720d1d09ea 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -11,10 +11,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "tvp5150_reg.h"
 
@@ -38,6 +40,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   enum v4l2_mbus_type bus_type;
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
@@ -424,8 +427,6 @@ static const struct i2c_reg_value tvp5150_init_enable[] = {
TVP5150_MISC_CTL, 0x6f
},{ /* Activates video std autodetection for all standards */
TVP5150_AUTOSW_MSK, 0x0
-   },{ /* Default format: 0x47. For 4:2:2: 0x40 */
-   TVP5150_DATA_RATE_SEL, 0x47
},{
TVP5150_CHROMA_PROC_CTL_1, 0x0c
},{
@@ -760,6 +761,25 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
+   switch (decoder->bus_type) {
+   case V4L2_MBUS_BT656:
+   /* 8-bit ITU BT.656 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x7);
+   /* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   break;
+   case V4L2_MBUS_PARALLEL:
+   /* 8-bit YUV 4:2:2 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x0);
+   /* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   break;
+   default:
+   return -EINVAL;
+   }
+
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
@@ -1332,6 +1352,8 @@ static struct regmap_config tvp5150_config = {
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
+   struct v4l2_of_endpoint bus_cfg;
+   struct device_node *endpoint;
struct tvp5150 *core;
struct v4l2_subdev *sd;
struct regmap *map;
@@ -1398,6 +1420,14 @@ static int tvp5150_probe(struct i2c_client *c,
}
}
 
+   endpoint = of_graph_get_next_endpoint(c->dev.of_node, NULL);
+   if (endpoint) {
+   v4l2_of_parse_endpoint(endpoint, _cfg);
+   core->bus_type = bus_cfg.bus_type;
+   } else {
+   core->bus_type = V4L2_MBUS_BT656;
+   }
+
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
-- 
2.7.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

[PATCH v3 1/9] [media] tvp5150: convert register access to regmap

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

Regmap provides built in debugging, caching and provides dedicated accessors
for bit manipulations in registers, which make the following changes a lot
simpler.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 194 ++--
 1 file changed, 133 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 6c3769d44b75..6ba93a425640 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,6 +38,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;
+   struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
u32 input;
@@ -56,30 +58,14 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl 
*ctrl)
 
 static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
 {
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
-
-   rc = i2c_smbus_read_byte_data(c, addr);
-   if (rc < 0) {
-   v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
-   return rc;
-   }
-
-   v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, rc);
-
-   return rc;
-}
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   int ret, val;
 
-static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
-unsigned char value)
-{
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
+   ret = regmap_read(decoder->regmap, addr, );
+   if (ret < 0)
+   return ret;
 
-   v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
-   rc = i2c_smbus_write_byte_data(c, addr, value);
-   if (rc < 0)
-   v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
+   return val;
 }
 
 static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
@@ -266,8 +252,8 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
decoder->input, decoder->output,
input, opmode);
 
-   tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
-   tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
+   regmap_write(decoder->regmap, TVP5150_OP_MODE_CTL, opmode);
+   regmap_write(decoder->regmap, TVP5150_VD_IN_SRC_SEL_1, input);
 
/* Svideo should enable YCrCb output and disable GPCL output
 * For Composite and TV, it should be the reverse
@@ -282,7 +268,7 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
val = (val & ~0x40) | 0x10;
else
val = (val & ~0x10) | 0x40;
-   tvp5150_write(sd, TVP5150_MISC_CTL, val);
+   regmap_write(decoder->regmap, TVP5150_MISC_CTL, val);
 };
 
 struct i2c_reg_value {
@@ -553,8 +539,10 @@ static struct i2c_vbi_ram_value vbi_ram_default[] =
 static int tvp5150_write_inittab(struct v4l2_subdev *sd,
const struct i2c_reg_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
while (regs->reg != 0xff) {
-   tvp5150_write(sd, regs->reg, regs->value);
+   regmap_write(decoder->regmap, regs->reg, regs->value);
regs++;
}
return 0;
@@ -563,22 +551,24 @@ static int tvp5150_write_inittab(struct v4l2_subdev *sd,
 static int tvp5150_vdp_init(struct v4l2_subdev *sd,
const struct i2c_vbi_ram_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
unsigned int i;
 
/* Disable Full Field */
-   tvp5150_write(sd, TVP5150_FULL_FIELD_ENA, 0);
+   regmap_write(map, TVP5150_FULL_FIELD_ENA, 0);
 
/* Before programming, Line mode should be at 0xff */
for (i = TVP5150_LINE_MODE_INI; i <= TVP5150_LINE_MODE_END; i++)
-   tvp5150_write(sd, i, 0xff);
+   regmap_write(map, i, 0xff);
 
/* Load Ram Table */
while (regs->reg != (u16)-1) {
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
 
for (i = 0; i < 16; i++)
-   tvp5150_write(sd, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
+   regmap_write(map, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
 
 

[PATCH v3 2/9] [media] tvp5150: add userspace subdev API

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds userspace V4L2 subdevice API support.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 282 +++-
 1 file changed, 223 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 6ba93a425640..67312c9d83c1 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -36,7 +36,9 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 struct tvp5150 {
struct v4l2_subdev sd;
+   struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
 
@@ -819,38 +821,68 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+static void tvp5150_try_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
 {
-   struct v4l2_mbus_framefmt *f;
-   struct tvp5150 *decoder = to_tvp5150(sd);
+   unsigned int hmax;
 
-   if (!format || format->pad)
-   return -EINVAL;
+   /* Clamp the crop rectangle boundaries to tvp5150 limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp(rect->width,
+   TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect->left,
+   TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
 
-   f = >format;
+   /* tvp5150 has some special limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp_t(unsigned int, rect->width,
+ TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - 
rect->left,
+ TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
+
+   /* Calculate height based on current standard */
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   tvp5150_reset(sd, 0);
+   rect->height = clamp(rect->height,
+hmax - TVP5150_MAX_CROP_TOP - rect->top,
+hmax - rect->top);
+}
 
-   f->width = decoder->rect.width;
-   f->height = decoder->rect.height;
+static void tvp5150_set_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
+{
+   struct regmap *map = decoder->regmap;
+   unsigned int hmax;
 
-   f->code = MEDIA_BUS_FMT_UYVY8_2X8;
-   f->field = V4L2_FIELD_SEQ_TB;
-   f->colorspace = V4L2_COLORSPACE_SMPTE170M;
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
-   f->height);
-   return 0;
+   regmap_write(map, TVP5150_VERT_BLANKING_START, rect->top);
+   regmap_write(map, TVP5150_VERT_BLANKING_STOP,
+rect->top + rect->height - hmax);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_MSB,
+rect->left >> TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_LSB,
+rect->left | (1 << TVP5150_CROP_SHIFT));
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_MSB,
+(rect->left + rect->width - TVP5150_MAX_CROP_LEFT) >>
+TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_LSB,
+rect->left + rect->width - TVP5150_MAX_CROP_LEFT);
+
+   decoder->rect = *rect;
 }
 
 static int tvp5150_s_crop(struct v4l2_subdev *sd, const struct v4l2_crop *a)
 {
-   struct v4l2_rect rect = a->c;
struct tvp5150 *decoder = to_tvp5150(sd);
+   struct v4l2_rect rect = a->c;
v4l2_std_id std;
-   unsigned int hmax;
 
v4l2_dbg(1, debug, sd, "%s left=%d, top=%d, width=%d, height=%d\n",
__func__, rect.left, rect.top, rect.width, rect.height);
@@ -858,42 +890,13 @@ static int tvp5150_s_crop(struct v4l2_subdev *sd, const 
struct v4l2_crop *a)
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
-   /* tvp5150 has some special limits */
-   rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT);
-   rect.width = clamp_t(unsigned int, rect.width,
-TVP5150_H_MAX -

[PATCH v3 8/9] [media] tvp5150: Add sync lock interrupt handling

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds an optional interrupt handler to handle the sync
lock interrupt and sync lock status.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 103 ++--
 drivers/media/i2c/tvp5150_reg.h |   2 +
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index e0f5bc219ced..b5140253b648 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -44,12 +45,14 @@ struct tvp5150 {
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
+   int irq;
 
v4l2_std_id norm;   /* Current set standard */
v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
+   bool lock;
 };
 
 static inline struct tvp5150 *to_tvp5150(struct v4l2_subdev *sd)
@@ -716,6 +719,15 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
return 0;
 }
 
+static int tvp5150_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
+   *std = decoder->norm;
+
+   return 0;
+}
+
 static int tvp5150_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -758,14 +770,25 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev 
*sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
+
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
-   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
-   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
-   /* Keep interrupt polarity active low */
-   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
-   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   if (decoder->irq) {
+   /* Configure pins: FID, VSYNC, INTREQ, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x0);
+   /* Set interrupt polarity to active high */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE | 0x1);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x1);
+   } else {
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   }
 
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
@@ -776,6 +799,33 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
return 0;
 }
 
+static irqreturn_t tvp5150_isr(int irq, void *dev_id)
+{
+   struct tvp5150 *decoder = dev_id;
+   struct regmap *map = decoder->regmap;
+   unsigned int active = 0, status = 0;
+
+   regmap_read(map, TVP5150_INT_STATUS_REG_A, );
+   if (status) {
+   regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
+
+   if (status & TVP5150_INT_A_LOCK)
+   decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+
+   return IRQ_HANDLED;
+   }
+
+   regmap_read(map, TVP5150_INT_ACTIVE_REG_B, );
+   if (active) {
+   status = 0;
+   regmap_read(map, TVP5150_INT_STATUS_REG_B, );
+   if (status)
+   regmap_write(map, TVP5150_INT_RESET_REG_B, status);
+   }
+
+   return IRQ_HANDLED;
+}
+
 static int tvp5150_enable(struct v4l2_subdev *sd)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -939,6 +989,35 @@ static int tvp5150_g_crop(struct v4l2_subdev *sd, struct 
v4l2_crop *a)
return 0;
 }
 
+static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+
+   if (enable) {
+   /* Enable YUV(OUT7:0), clock */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
+   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+   if (decoder->irq) {
+   /* Enable lock interrupt */
+   regmap_update_bits(decoder->regmap,
+  TVP5150_INT_ENABLE_REG_A,
+  TVP5150_INT_A_LOCK,
+  TVP5150_INT_A_LOCK);
+   }
+   } else {
+   /* Disable YUV(OUT7:0), SYNC, clock */
+   regmap_up

[PATCH v3 9/9] [media] tvp5150: disable output while signal not locked

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To avoid short frames on stream start, keep output pins at high impedance
while we are not properly locked onto the input signal.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index b5140253b648..13ce826a4093 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -51,6 +51,7 @@ struct tvp5150 {
v4l2_std_id detected_norm;
u32 input;
u32 output;
+   u32 oe;
int enable;
bool lock;
 };
@@ -809,8 +810,11 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id)
if (status) {
regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
 
-   if (status & TVP5150_INT_A_LOCK)
+   if (status & TVP5150_INT_A_LOCK) {
decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL,
+  0xd, decoder->lock ? decoder->oe : 
0);
+   }
 
return IRQ_HANDLED;
}
@@ -841,6 +845,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x7);
/* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   decoder->oe = 0x9;
break;
case V4L2_MBUS_PARALLEL:
/* 8-bit YUV 4:2:2 */
@@ -848,6 +853,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x0);
/* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   decoder->oe = 0xd;
break;
default:
return -EINVAL;
@@ -994,9 +1000,9 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int 
enable)
struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
 
if (enable) {
-   /* Enable YUV(OUT7:0), clock */
+   /* Enable YUV(OUT7:0), (SYNC), clock signal, if locked */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
-   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+  decoder->lock ? decoder->oe : 0);
if (decoder->irq) {
/* Enable lock interrupt */
regmap_update_bits(decoder->regmap,
-- 
2.7.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

[PATCH v3 5/9] [media] tvp5150: split reset/enable routine

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To trigger standard autodetection only the reset part of the routine
is necessary. Split this out to make it callable on its own.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 21d044b564ad..f5e6bfa9dd7f 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -768,9 +768,6 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
-   struct tvp5150 *decoder = to_tvp5150(sd);
-   v4l2_std_id std;
-
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
@@ -780,6 +777,14 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Selects decoder input */
tvp5150_selmux(sd);
 
+   return 0;
+}
+
+static int tvp5150_enable(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
+
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
@@ -844,6 +849,7 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
@@ -1179,8 +1185,10 @@ static int tvp5150_set_format(struct v4l2_subdev *sd,
 
format->format = *mbus_format;
 
-   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
tvp5150_reset(sd, 0);
+   tvp5150_enable(sd);
+   }
 
v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", mbus_format->width,
mbus_format->height);
@@ -1454,6 +1462,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
v4l2_ctrl_handler_setup(>hdl);
 
+   tvp5150_reset(sd, 0);
/* Default is no cropping */
tvp5150_set_default(tvp5150_read_std(sd), >rect, >format);
 
-- 
2.7.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

[PATCH v3 6/9] [media] tvp5150: trigger autodetection on subdev open to reset cropping

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

If cropping isn't set explicitly by userspace, reset it to the maximum
possible rectangle in subdevice open if a standard change is detected.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f5e6bfa9dd7f..d0b5e148dcd8 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -46,6 +46,7 @@ struct tvp5150 {
struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
+   v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
@@ -1220,13 +1221,19 @@ static int tvp5150_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
struct tvp5150 *decoder = to_tvp5150(sd);
v4l2_std_id std;
 
-   if (decoder->norm == V4L2_STD_ALL)
+   if (decoder->norm == V4L2_STD_ALL) {
std = tvp5150_read_std(sd);
-   else
-   std = decoder->norm;
+   if (std != decoder->detected_norm) {
+   decoder->detected_norm = std;
+
+   if (std & V4L2_STD_525_60)
+   decoder->rect.height = TVP5150_V_MAX_525_60;
+   else
+   decoder->rect.height = TVP5150_V_MAX_OTHERS;
+   decoder->format.height = decoder->rect.height;
+   }
+   }
 
-   tvp5150_set_default(std, v4l2_subdev_get_try_crop(fh, 0),
-v4l2_subdev_get_try_format(fh, 0));
return 0;
 }
 #endif
@@ -1443,6 +1450,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
 
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
+   core->detected_norm = V4L2_STD_UNKNOWN;
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
 
-- 
2.7.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

[PATCH v3 7/9] [media] tvp5150: remove pin configuration from initialization tables

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To allow optional interrupt support, we want to configure the pin settings
dynamically. Move those register accesses out of the static initialization
tables.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 19 +++
 drivers/media/i2c/tvp5150_reg.h |  1 +
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index d0b5e148dcd8..e0f5bc219ced 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -323,9 +323,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x0e */
TVP5150_LUMA_PROC_CTL_3,0x00
},
-   { /* 0x0f */
-   TVP5150_CONF_SHARED_PIN,0x08
-   },
{ /* 0x11 */
TVP5150_ACT_VD_CROP_ST_MSB,0x00
},
@@ -362,9 +359,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x1d */
TVP5150_INT_ENABLE_REG_B,0x00
},
-   { /* 0x1e */
-   TVP5150_INTT_CONFIG_REG_B,0x00
-   },
{ /* 0x28 */
TVP5150_VIDEO_STD,0x00
},
@@ -383,9 +377,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0xc1 */
TVP5150_INT_ENABLE_REG_A,0x00
},
-   { /* 0xc2 */
-   TVP5150_INT_CONF,0x04
-   },
{ /* 0xc8 */
TVP5150_FIFO_INT_THRESHOLD,0x80
},
@@ -420,9 +411,7 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
 
 /* Default values as sugested at TVP5150AM1 datasheet */
 static const struct i2c_reg_value tvp5150_init_enable[] = {
-   {
-   TVP5150_CONF_SHARED_PIN, 2
-   },{ /* Automatic offset and AGC enabled */
+   {   /* Automatic offset and AGC enabled */
TVP5150_ANAL_CHL_CTL, 0x15
},{ /* Activate YCrCb output 0x9 or 0xd ? */
TVP5150_MISC_CTL, 0x6f
@@ -772,6 +761,12 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
 
diff --git a/drivers/media/i2c/tvp5150_reg.h b/drivers/media/i2c/tvp5150_reg.h
index 25a994944918..fc3bcb26413a 100644
--- a/drivers/media/i2c/tvp5150_reg.h
+++ b/drivers/media/i2c/tvp5150_reg.h
@@ -117,6 +117,7 @@
 #define TVP5150_INT_STATUS_REG_A0xc0 /* Interrupt status register A */
 #define TVP5150_INT_ENABLE_REG_A0xc1 /* Interrupt enable register A */
 #define TVP5150_INT_CONF0xc2 /* Interrupt configuration */
+#define   TVP5150_VDPOE BIT(2)
 #define TVP5150_VDP_CONF_RAM_DATA   0xc3 /* VDP configuration RAM data */
 #define TVP5150_CONF_RAM_ADDR_LOW   0xc4 /* Configuration RAM address low byte 
*/
 #define TVP5150_CONF_RAM_ADDR_HIGH  0xc5 /* Configuration RAM address high 
byte */
-- 
2.7.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

[PATCH v2 4/9] [media] tvp5150: fix standard autodetection

[Lucas Stach]

From: Philipp Zabel 

Make sure to not overwrite decoder->norm when setting the standard
in hardware, but only when instructed by V4L2 API calls.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 56 +
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f504fc005222..cb9675fbf358 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -703,8 +703,6 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
struct tvp5150 *decoder = to_tvp5150(sd);
int fmt = 0;
 
-   decoder->norm = std;
-
/* First tests should be against specific std */
 
if (std == V4L2_STD_NTSC_443) {
@@ -741,13 +739,37 @@ static int tvp5150_s_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
else
decoder->rect.height = TVP5150_V_MAX_OTHERS;
 
+   decoder->norm = std;
 
return tvp5150_set_std(sd, std);
 }
 
+static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
+{
+   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
+
+   switch (val & 0x0F) {
+   case 0x01:
+   return V4L2_STD_NTSC;
+   case 0x03:
+   return V4L2_STD_PAL;
+   case 0x05:
+   return V4L2_STD_PAL_M;
+   case 0x07:
+   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
+   case 0x09:
+   return V4L2_STD_NTSC_443;
+   case 0xb:
+   return V4L2_STD_SECAM;
+   default:
+   return V4L2_STD_UNKNOWN;
+   }
+}
+
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
 
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
@@ -783,7 +805,13 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
-   tvp5150_set_std(sd, decoder->norm);
+   if (decoder->norm == V4L2_STD_ALL)
+   std = tvp5150_read_std(sd);
+   else
+   std = decoder->norm;
+
+   /* Disable autoswitch mode */
+   tvp5150_set_std(sd, std);
return 0;
 };
 
@@ -808,28 +836,6 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
return -EINVAL;
 }
 
-static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
-{
-   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
-
-   switch (val & 0x0F) {
-   case 0x01:
-   return V4L2_STD_NTSC;
-   case 0x03:
-   return V4L2_STD_PAL;
-   case 0x05:
-   return V4L2_STD_PAL_M;
-   case 0x07:
-   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
-   case 0x09:
-   return V4L2_STD_NTSC_443;
-   case 0xb:
-   return V4L2_STD_SECAM;
-   default:
-   return V4L2_STD_UNKNOWN;
-   }
-}
-
 static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
-- 
2.6.2

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

[PATCH v2 6/9] [media] tvp5150: trigger autodetection on subdev open to reset cropping

[Lucas Stach]

From: Philipp Zabel 

If cropping isn't set explicitly by userspace, reset it to the maximum
possible rectangle in subdevice open if a standard change is detected.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 6d0f2dede866..f107a17650c6 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -46,6 +46,7 @@ struct tvp5150 {
struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
+   v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
@@ -1220,13 +1221,19 @@ static int tvp5150_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
struct tvp5150 *decoder = to_tvp5150(sd);
v4l2_std_id std;
 
-   if (decoder->norm == V4L2_STD_ALL)
+   if (decoder->norm == V4L2_STD_ALL) {
std = tvp5150_read_std(sd);
-   else
-   std = decoder->norm;
+   if (std != decoder->detected_norm) {
+   decoder->detected_norm = std;
+
+   if (std & V4L2_STD_525_60)
+   decoder->rect.height = TVP5150_V_MAX_525_60;
+   else
+   decoder->rect.height = TVP5150_V_MAX_OTHERS;
+   decoder->format.height = decoder->rect.height;
+   }
+   }
 
-   tvp5150_set_default(std, v4l2_subdev_get_try_crop(fh, 0),
-v4l2_subdev_get_try_format(fh, 0));
return 0;
 }
 #endif
@@ -1443,6 +1450,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
 
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
+   core->detected_norm = V4L2_STD_UNKNOWN;
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
 
-- 
2.6.2

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

[PATCH v2 7/9] [media] tvp5150: remove pin configuration from initialization tables

[Lucas Stach]

From: Philipp Zabel 

To allow optional interrupt support, we want to configure the pin settings
dynamically. Move those register accesses out of the static initialization
tables.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 19 +++
 drivers/media/i2c/tvp5150_reg.h |  1 +
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index f107a17650c6..ceda29aa23f9 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -323,9 +323,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x0e */
TVP5150_LUMA_PROC_CTL_3,0x00
},
-   { /* 0x0f */
-   TVP5150_CONF_SHARED_PIN,0x08
-   },
{ /* 0x11 */
TVP5150_ACT_VD_CROP_ST_MSB,0x00
},
@@ -362,9 +359,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x1d */
TVP5150_INT_ENABLE_REG_B,0x00
},
-   { /* 0x1e */
-   TVP5150_INTT_CONFIG_REG_B,0x00
-   },
{ /* 0x28 */
TVP5150_VIDEO_STD,0x00
},
@@ -383,9 +377,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0xc1 */
TVP5150_INT_ENABLE_REG_A,0x00
},
-   { /* 0xc2 */
-   TVP5150_INT_CONF,0x04
-   },
{ /* 0xc8 */
TVP5150_FIFO_INT_THRESHOLD,0x80
},
@@ -420,9 +411,7 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
 
 /* Default values as sugested at TVP5150AM1 datasheet */
 static const struct i2c_reg_value tvp5150_init_enable[] = {
-   {
-   TVP5150_CONF_SHARED_PIN, 2
-   },{ /* Automatic offset and AGC enabled */
+   {   /* Automatic offset and AGC enabled */
TVP5150_ANAL_CHL_CTL, 0x15
},{ /* Activate YCrCb output 0x9 or 0xd ? */
TVP5150_MISC_CTL, 0x6f
@@ -772,6 +761,12 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
 
diff --git a/drivers/media/i2c/tvp5150_reg.h b/drivers/media/i2c/tvp5150_reg.h
index 25a994944918..fc3bcb26413a 100644
--- a/drivers/media/i2c/tvp5150_reg.h
+++ b/drivers/media/i2c/tvp5150_reg.h
@@ -117,6 +117,7 @@
 #define TVP5150_INT_STATUS_REG_A0xc0 /* Interrupt status register A */
 #define TVP5150_INT_ENABLE_REG_A0xc1 /* Interrupt enable register A */
 #define TVP5150_INT_CONF0xc2 /* Interrupt configuration */
+#define   TVP5150_VDPOE BIT(2)
 #define TVP5150_VDP_CONF_RAM_DATA   0xc3 /* VDP configuration RAM data */
 #define TVP5150_CONF_RAM_ADDR_LOW   0xc4 /* Configuration RAM address low byte 
*/
 #define TVP5150_CONF_RAM_ADDR_HIGH  0xc5 /* Configuration RAM address high 
byte */
-- 
2.6.2

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

[PATCH v2 2/9] [media] tvp5150: add userspace subdev API

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds userspace V4L2 subdevice API support.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
v2: Allow the driver to be built without MEDIA_CONTROLLER and
VIDEO_V4L2_SUBDEV_API, to keep it working for devices that
don't want or need the userspace subdev API.
---
 drivers/media/i2c/tvp5150.c | 282 +++-
 1 file changed, 223 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index a7495d2856c3..3eab4d918c54 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -36,7 +36,9 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 struct tvp5150 {
struct v4l2_subdev sd;
+   struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
 
@@ -819,38 +821,68 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+static void tvp5150_try_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
 {
-   struct v4l2_mbus_framefmt *f;
-   struct tvp5150 *decoder = to_tvp5150(sd);
+   unsigned int hmax;
 
-   if (!format || format->pad)
-   return -EINVAL;
+   /* Clamp the crop rectangle boundaries to tvp5150 limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp(rect->width,
+   TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect->left,
+   TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
 
-   f = >format;
+   /* tvp5150 has some special limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp_t(unsigned int, rect->width,
+ TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - 
rect->left,
+ TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
+
+   /* Calculate height based on current standard */
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   tvp5150_reset(sd, 0);
+   rect->height = clamp(rect->height,
+hmax - TVP5150_MAX_CROP_TOP - rect->top,
+hmax - rect->top);
+}
 
-   f->width = decoder->rect.width;
-   f->height = decoder->rect.height;
+static void tvp5150_set_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
+{
+   struct regmap *map = decoder->regmap;
+   unsigned int hmax;
 
-   f->code = MEDIA_BUS_FMT_UYVY8_2X8;
-   f->field = V4L2_FIELD_SEQ_TB;
-   f->colorspace = V4L2_COLORSPACE_SMPTE170M;
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
-   f->height);
-   return 0;
+   regmap_write(map, TVP5150_VERT_BLANKING_START, rect->top);
+   regmap_write(map, TVP5150_VERT_BLANKING_STOP,
+rect->top + rect->height - hmax);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_MSB,
+rect->left >> TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_LSB,
+rect->left | (1 << TVP5150_CROP_SHIFT));
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_MSB,
+(rect->left + rect->width - TVP5150_MAX_CROP_LEFT) >>
+TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_LSB,
+rect->left + rect->width - TVP5150_MAX_CROP_LEFT);
+
+   decoder->rect = *rect;
 }
 
 static int tvp5150_s_crop(struct v4l2_subdev *sd, const struct v4l2_crop *a)
 {
-   struct v4l2_rect rect = a->c;
struct tvp5150 *decoder = to_tvp5150(sd);
+   struct v4l2_rect rect = a->c;
v4l2_std_id std;
-   unsigned int hmax;
 
v4l2_dbg(1, debug, sd, "%s left=%d, top=%d, width=%d, height=%d\n",
__func__, rect.left, rect.top, rect.width, rect.height);
@@ -858,42 +890,13 @@ static int tvp5150_s_crop(struct v4l2_subdev *sd, const 
struct v4l2_crop *a)
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
-   /* tvp5150 has some specia

[PATCH v2 3/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Lucas Stach]

From: Philipp Zabel 

By looking at the endpoint flags, it can be determined whether the link
should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
dedicated HSYNC/VSYNC outputs in BT.656 mode.

For devices that are not instantiated through DT the current behavior
is preserved.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 3eab4d918c54..f504fc005222 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -11,10 +11,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "tvp5150_reg.h"
 
@@ -38,6 +40,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   enum v4l2_mbus_type bus_type;
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
@@ -424,8 +427,6 @@ static const struct i2c_reg_value tvp5150_init_enable[] = {
TVP5150_MISC_CTL, 0x6f
},{ /* Activates video std autodetection for all standards */
TVP5150_AUTOSW_MSK, 0x0
-   },{ /* Default format: 0x47. For 4:2:2: 0x40 */
-   TVP5150_DATA_RATE_SEL, 0x47
},{
TVP5150_CHROMA_PROC_CTL_1, 0x0c
},{
@@ -760,6 +761,25 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
+   switch (decoder->bus_type) {
+   case V4L2_MBUS_BT656:
+   /* 8-bit ITU BT.656 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x7);
+   /* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   break;
+   case V4L2_MBUS_PARALLEL:
+   /* 8-bit YUV 4:2:2 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x0);
+   /* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   break;
+   default:
+   return -EINVAL;
+   }
+
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
@@ -1332,6 +1352,8 @@ static struct regmap_config tvp5150_config = {
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
+   struct v4l2_of_endpoint bus_cfg;
+   struct device_node *endpoint;
struct tvp5150 *core;
struct v4l2_subdev *sd;
struct regmap *map;
@@ -1398,6 +1420,14 @@ static int tvp5150_probe(struct i2c_client *c,
}
}
 
+   endpoint = of_graph_get_next_endpoint(c->dev.of_node, NULL);
+   if (endpoint) {
+   v4l2_of_parse_endpoint(endpoint, _cfg);
+   core->bus_type = bus_cfg.bus_type;
+   } else {
+   core->bus_type = V4L2_MBUS_BT656;
+   }
+
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
-- 
2.6.2

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

[PATCH v2 5/9] [media] tvp5150: split reset/enable routine

[Lucas Stach]

From: Philipp Zabel 

To trigger standard autodetection only the reset part of the routine
is necessary. Split this out to make it callable on its own.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index cb9675fbf358..6d0f2dede866 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -768,9 +768,6 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
-   struct tvp5150 *decoder = to_tvp5150(sd);
-   v4l2_std_id std;
-
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
@@ -780,6 +777,14 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Selects decoder input */
tvp5150_selmux(sd);
 
+   return 0;
+}
+
+static int tvp5150_enable(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
+
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
@@ -844,6 +849,7 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
@@ -1179,8 +1185,10 @@ static int tvp5150_set_format(struct v4l2_subdev *sd,
 
format->format = *mbus_format;
 
-   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
tvp5150_reset(sd, 0);
+   tvp5150_enable(sd);
+   }
 
v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", mbus_format->width,
mbus_format->height);
@@ -1454,6 +1462,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
v4l2_ctrl_handler_setup(>hdl);
 
+   tvp5150_reset(sd, 0);
/* Default is no cropping */
tvp5150_set_default(tvp5150_read_std(sd), >rect, >format);
 
-- 
2.6.2

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

[PATCH v2 9/9] [media] tvp5150: disable output while signal not locked

[Lucas Stach]

From: Philipp Zabel 

To avoid short frames on stream start, keep output pins at high impedance
while we are not properly locked onto the input signal.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 85c5c73098fa..627f8bb58074 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -51,6 +51,7 @@ struct tvp5150 {
v4l2_std_id detected_norm;
u32 input;
u32 output;
+   u32 oe;
int enable;
bool lock;
 };
@@ -809,8 +810,11 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id)
if (status) {
regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
 
-   if (status & TVP5150_INT_A_LOCK)
+   if (status & TVP5150_INT_A_LOCK) {
decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL,
+  0xd, decoder->lock ? decoder->oe : 
0);
+   }
 
return IRQ_HANDLED;
}
@@ -841,6 +845,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x7);
/* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   decoder->oe = 0x9;
break;
case V4L2_MBUS_PARALLEL:
/* 8-bit YUV 4:2:2 */
@@ -848,6 +853,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x0);
/* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   decoder->oe = 0xd;
break;
default:
return -EINVAL;
@@ -994,9 +1000,9 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int 
enable)
struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
 
if (enable) {
-   /* Enable YUV(OUT7:0), clock */
+   /* Enable YUV(OUT7:0), (SYNC), clock signal, if locked */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
-   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+  decoder->lock ? decoder->oe : 0);
if (decoder->irq) {
/* Enable lock interrupt */
regmap_update_bits(decoder->regmap,
-- 
2.6.2

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

[PATCH v2 8/9] [media] tvp5150: Add sync lock interrupt handling

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds an optional interrupt handler to handle the sync
lock interrupt and sync lock status.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 103 ++--
 drivers/media/i2c/tvp5150_reg.h |   2 +
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index ceda29aa23f9..85c5c73098fa 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -44,12 +45,14 @@ struct tvp5150 {
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
+   int irq;
 
v4l2_std_id norm;   /* Current set standard */
v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
+   bool lock;
 };
 
 static inline struct tvp5150 *to_tvp5150(struct v4l2_subdev *sd)
@@ -716,6 +719,15 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
return 0;
 }
 
+static int tvp5150_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
+   *std = decoder->norm;
+
+   return 0;
+}
+
 static int tvp5150_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -758,14 +770,25 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev 
*sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
+
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
-   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
-   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
-   /* Keep interrupt polarity active low */
-   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
-   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   if (decoder->irq) {
+   /* Configure pins: FID, VSYNC, INTREQ, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x0);
+   /* Set interrupt polarity to active high */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE | 0x1);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x1);
+   } else {
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   }
 
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
@@ -776,6 +799,33 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
return 0;
 }
 
+static irqreturn_t tvp5150_isr(int irq, void *dev_id)
+{
+   struct tvp5150 *decoder = dev_id;
+   struct regmap *map = decoder->regmap;
+   unsigned int active = 0, status = 0;
+
+   regmap_read(map, TVP5150_INT_STATUS_REG_A, );
+   if (status) {
+   regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
+
+   if (status & TVP5150_INT_A_LOCK)
+   decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+
+   return IRQ_HANDLED;
+   }
+
+   regmap_read(map, TVP5150_INT_ACTIVE_REG_B, );
+   if (active) {
+   status = 0;
+   regmap_read(map, TVP5150_INT_STATUS_REG_B, );
+   if (status)
+   regmap_write(map, TVP5150_INT_RESET_REG_B, status);
+   }
+
+   return IRQ_HANDLED;
+}
+
 static int tvp5150_enable(struct v4l2_subdev *sd)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -939,6 +989,35 @@ static int tvp5150_g_crop(struct v4l2_subdev *sd, struct 
v4l2_crop *a)
return 0;
 }
 
+static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+
+   if (enable) {
+   /* Enable YUV(OUT7:0), clock */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
+   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+   if (decoder->irq) {
+   /* Enable lock interrupt */
+   regmap_update_bits(decoder->regmap,
+  TVP5150_INT_ENABLE_REG_A,
+  TVP5150_INT_A_LOCK,
+  TVP5150_INT_A_LOCK);
+   }
+   } else {
+   /* Disable YUV(OUT7:0), SYNC, clock */
+   regmap_up

[PATCH v2 1/9] [media] tvp5150: convert register access to regmap

[Lucas Stach]

From: Philipp Zabel 

Regmap provides built in debugging, caching and provides dedicated accessors
for bit manipulations in registers, which make the following changes a lot
simpler.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tvp5150.c | 194 ++--
 1 file changed, 133 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 3c5fb2509c47..a7495d2856c3 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,6 +38,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;
+   struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
u32 input;
@@ -56,30 +58,14 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl 
*ctrl)
 
 static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
 {
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
-
-   rc = i2c_smbus_read_byte_data(c, addr);
-   if (rc < 0) {
-   v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
-   return rc;
-   }
-
-   v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, rc);
-
-   return rc;
-}
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   int ret, val;
 
-static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
-unsigned char value)
-{
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
+   ret = regmap_read(decoder->regmap, addr, );
+   if (ret < 0)
+   return ret;
 
-   v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
-   rc = i2c_smbus_write_byte_data(c, addr, value);
-   if (rc < 0)
-   v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
+   return val;
 }
 
 static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
@@ -266,8 +252,8 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
decoder->input, decoder->output,
input, opmode);
 
-   tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
-   tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
+   regmap_write(decoder->regmap, TVP5150_OP_MODE_CTL, opmode);
+   regmap_write(decoder->regmap, TVP5150_VD_IN_SRC_SEL_1, input);
 
/* Svideo should enable YCrCb output and disable GPCL output
 * For Composite and TV, it should be the reverse
@@ -282,7 +268,7 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
val = (val & ~0x40) | 0x10;
else
val = (val & ~0x10) | 0x40;
-   tvp5150_write(sd, TVP5150_MISC_CTL, val);
+   regmap_write(decoder->regmap, TVP5150_MISC_CTL, val);
 };
 
 struct i2c_reg_value {
@@ -553,8 +539,10 @@ static struct i2c_vbi_ram_value vbi_ram_default[] =
 static int tvp5150_write_inittab(struct v4l2_subdev *sd,
const struct i2c_reg_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
while (regs->reg != 0xff) {
-   tvp5150_write(sd, regs->reg, regs->value);
+   regmap_write(decoder->regmap, regs->reg, regs->value);
regs++;
}
return 0;
@@ -563,22 +551,24 @@ static int tvp5150_write_inittab(struct v4l2_subdev *sd,
 static int tvp5150_vdp_init(struct v4l2_subdev *sd,
const struct i2c_vbi_ram_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
unsigned int i;
 
/* Disable Full Field */
-   tvp5150_write(sd, TVP5150_FULL_FIELD_ENA, 0);
+   regmap_write(map, TVP5150_FULL_FIELD_ENA, 0);
 
/* Before programming, Line mode should be at 0xff */
for (i = TVP5150_LINE_MODE_INI; i <= TVP5150_LINE_MODE_END; i++)
-   tvp5150_write(sd, i, 0xff);
+   regmap_write(map, i, 0xff);
 
/* Load Ram Table */
while (regs->reg != (u16)-1) {
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
 
for (i = 0; i < 16; i++)
-   tvp5150_write(sd, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
+   regmap_write(map, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
 
regs++;
}
@@ -658,11 +648,11 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
reg=((line-6)<<1)+TVP5150_LINE_MODE_INI;
 
if (fields&1) {
-   tvp5150_write(sd, reg, 

Re: [PATCH 2/9] [media] tvp5150: add userspace subdev API

[Lucas Stach]

Am Donnerstag, den 19.11.2015, 01:06 +0800 schrieb kbuild test robot:
> Hi Philipp,
> 
> [auto build test ERROR on: v4.4-rc1]
> [also build test ERROR on: next-20151118]
> [cannot apply to: linuxtv-media/master]
> 
> url:
> https://github.com/0day-ci/linux/commits/Lucas-Stach/tvp5150-convert-register-access-to-regmap/20151119-005732
> config: x86_64-randconfig-x018-11181928 (attached as .config)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All error/warnings (new ones prefixed by >>):
> 
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_get_pad_format':
> >> drivers/media/i2c/tvp5150.c:1062:10: error: implicit declaration of 
> >> function 'v4l2_subdev_get_try_format' 
> >> [-Werror=implicit-function-declaration]
>   return v4l2_subdev_get_try_format(sd, cfg, pad);
>  ^
> >> drivers/media/i2c/tvp5150.c:1062:10: warning: return makes pointer from 
> >> integer without a cast [-Wint-conversion]
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_get_pad_crop':
> >> drivers/media/i2c/tvp5150.c:1077:10: error: implicit declaration of 
> >> function 'v4l2_subdev_get_try_crop' [-Werror=implicit-function-declaration]
>   return v4l2_subdev_get_try_crop(sd, cfg, pad);
>  ^
>drivers/media/i2c/tvp5150.c:1077:10: warning: return makes pointer from 
> integer without a cast [-Wint-conversion]
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_open':
> >> drivers/media/i2c/tvp5150.c:1180:27: warning: passing argument 2 of 
> >> 'tvp5150_set_default' makes pointer from integer without a cast 
> >> [-Wint-conversion]
>  tvp5150_set_default(std, v4l2_subdev_get_try_crop(fh, 0),
>   ^
>drivers/media/i2c/tvp5150.c:1152:13: note: expected 'struct v4l2_rect *' 
> but argument is of type 'int'
> static void tvp5150_set_default(v4l2_std_id std, struct v4l2_rect *crop,
> ^
>drivers/media/i2c/tvp5150.c:1181:6: warning: passing argument 3 of 
> 'tvp5150_set_default' makes pointer from integer without a cast 
> [-Wint-conversion]
>  v4l2_subdev_get_try_format(fh, 0));
>  ^
>drivers/media/i2c/tvp5150.c:1152:13: note: expected 'struct 
> v4l2_mbus_framefmt *' but argument is of type 'int'
> static void tvp5150_set_default(v4l2_std_id std, struct v4l2_rect *crop,
> ^
>drivers/media/i2c/tvp5150.c: In function 'tvp5150_probe':
> >> drivers/media/i2c/tvp5150.c:1340:4: error: 'struct v4l2_subdev' has no 
> >> member named 'entity'
>  sd->entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
>^
>drivers/media/i2c/tvp5150.c:1342:29: error: 'struct v4l2_subdev' has no 
> member named 'entity'
>  res = media_entity_init(>entity, 1, >pad, 0);
>     ^
>cc1: some warnings being treated as errors

Ok, this is just a missing depends on VIDEO_V4L2_SUBDEV_API. I'll wait
for other feedback before resending with that fixed.

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
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/9] [media] tvp5150: determine BT.656 or YUV 4:2:2 mode from device tree

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

By looking at the endpoint flags, it can be determined whether the link
should be of V4L2_MBUS_PARALLEL or V4L2_MBUS_BT656 type. Disable the
dedicated HSYNC/VSYNC outputs in BT.656 mode.

For devices that are not instantiated through DT the current behavior
is preserved.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 8670b478dcd6..21cde350e385 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -11,10 +11,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "tvp5150_reg.h"
 
@@ -38,6 +40,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   enum v4l2_mbus_type bus_type;
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
@@ -424,8 +427,6 @@ static const struct i2c_reg_value tvp5150_init_enable[] = {
TVP5150_MISC_CTL, 0x6f
},{ /* Activates video std autodetection for all standards */
TVP5150_AUTOSW_MSK, 0x0
-   },{ /* Default format: 0x47. For 4:2:2: 0x40 */
-   TVP5150_DATA_RATE_SEL, 0x47
},{
TVP5150_CHROMA_PROC_CTL_1, 0x0c
},{
@@ -760,6 +761,25 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
+   switch (decoder->bus_type) {
+   case V4L2_MBUS_BT656:
+   /* 8-bit ITU BT.656 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x7);
+   /* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   break;
+   case V4L2_MBUS_PARALLEL:
+   /* 8-bit YUV 4:2:2 */
+   regmap_update_bits(decoder->regmap, TVP5150_DATA_RATE_SEL,
+  0x7, 0x0);
+   /* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   break;
+   default:
+   return -EINVAL;
+   }
+
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
@@ -1315,6 +1335,8 @@ static struct regmap_config tvp5150_config = {
 static int tvp5150_probe(struct i2c_client *c,
 const struct i2c_device_id *id)
 {
+   struct v4l2_of_endpoint bus_cfg;
+   struct device_node *endpoint;
struct tvp5150 *core;
struct v4l2_subdev *sd;
struct regmap *map;
@@ -1375,6 +1397,14 @@ static int tvp5150_probe(struct i2c_client *c,
}
}
 
+   endpoint = of_graph_get_next_endpoint(c->dev.of_node, NULL);
+   if (endpoint) {
+   v4l2_of_parse_endpoint(endpoint, _cfg);
+   core->bus_type = bus_cfg.bus_type;
+   } else {
+   core->bus_type = V4L2_MBUS_BT656;
+   }
+
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
-- 
2.6.2

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

[PATCH 6/9] [media] tvp5150: trigger autodetection on subdev open to reset cropping

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

If cropping isn't set explicitly by userspace, reset it to the maximum
possible rectangle in subdevice open if a standard change is detected.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index b6328353404f..791572737ee3 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -46,6 +46,7 @@ struct tvp5150 {
struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
+   v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
@@ -1206,13 +1207,19 @@ static int tvp5150_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
struct tvp5150 *decoder = to_tvp5150(sd);
v4l2_std_id std;
 
-   if (decoder->norm == V4L2_STD_ALL)
+   if (decoder->norm == V4L2_STD_ALL) {
std = tvp5150_read_std(sd);
-   else
-   std = decoder->norm;
+   if (std != decoder->detected_norm) {
+   decoder->detected_norm = std;
+
+   if (std & V4L2_STD_525_60)
+   decoder->rect.height = TVP5150_V_MAX_525_60;
+   else
+   decoder->rect.height = TVP5150_V_MAX_OTHERS;
+   decoder->format.height = decoder->rect.height;
+   }
+   }
 
-   tvp5150_set_default(std, v4l2_subdev_get_try_crop(fh, 0),
-v4l2_subdev_get_try_format(fh, 0));
return 0;
 }
 
@@ -1420,6 +1427,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
 
core->norm = V4L2_STD_ALL;  /* Default is autodetect */
+   core->detected_norm = V4L2_STD_UNKNOWN;
core->input = TVP5150_COMPOSITE1;
core->enable = 1;
 
-- 
2.6.2

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

[PATCH 4/9] [media] tvp5150: fix standard autodetection

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

Make sure to not overwrite decoder->norm when setting the standard
in hardware, but only when instructed by V4L2 API calls.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 56 +
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 21cde350e385..b943b9cc24c8 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -703,8 +703,6 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
struct tvp5150 *decoder = to_tvp5150(sd);
int fmt = 0;
 
-   decoder->norm = std;
-
/* First tests should be against specific std */
 
if (std == V4L2_STD_NTSC_443) {
@@ -741,13 +739,37 @@ static int tvp5150_s_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
else
decoder->rect.height = TVP5150_V_MAX_OTHERS;
 
+   decoder->norm = std;
 
return tvp5150_set_std(sd, std);
 }
 
+static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
+{
+   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
+
+   switch (val & 0x0F) {
+   case 0x01:
+   return V4L2_STD_NTSC;
+   case 0x03:
+   return V4L2_STD_PAL;
+   case 0x05:
+   return V4L2_STD_PAL_M;
+   case 0x07:
+   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
+   case 0x09:
+   return V4L2_STD_NTSC_443;
+   case 0xb:
+   return V4L2_STD_SECAM;
+   default:
+   return V4L2_STD_UNKNOWN;
+   }
+}
+
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
 
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
@@ -783,7 +805,13 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initialize image preferences */
v4l2_ctrl_handler_setup(>hdl);
 
-   tvp5150_set_std(sd, decoder->norm);
+   if (decoder->norm == V4L2_STD_ALL)
+   std = tvp5150_read_std(sd);
+   else
+   std = decoder->norm;
+
+   /* Disable autoswitch mode */
+   tvp5150_set_std(sd, std);
return 0;
 };
 
@@ -808,28 +836,6 @@ static int tvp5150_s_ctrl(struct v4l2_ctrl *ctrl)
return -EINVAL;
 }
 
-static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
-{
-   int val = tvp5150_read(sd, TVP5150_STATUS_REG_5);
-
-   switch (val & 0x0F) {
-   case 0x01:
-   return V4L2_STD_NTSC;
-   case 0x03:
-   return V4L2_STD_PAL;
-   case 0x05:
-   return V4L2_STD_PAL_M;
-   case 0x07:
-   return V4L2_STD_PAL_N | V4L2_STD_PAL_Nc;
-   case 0x09:
-   return V4L2_STD_NTSC_443;
-   case 0xb:
-   return V4L2_STD_SECAM;
-   default:
-   return V4L2_STD_UNKNOWN;
-   }
-}
-
 static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
-- 
2.6.2

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

[PATCH 5/9] [media] tvp5150: split reset/enable routine

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To trigger standard autodetection only the reset part of the routine
is necessary. Split this out to make it callable on its own.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index b943b9cc24c8..b6328353404f 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -768,9 +768,6 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev *sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
-   struct tvp5150 *decoder = to_tvp5150(sd);
-   v4l2_std_id std;
-
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
@@ -780,6 +777,14 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Selects decoder input */
tvp5150_selmux(sd);
 
+   return 0;
+}
+
+static int tvp5150_enable(struct v4l2_subdev *sd)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   v4l2_std_id std;
+
/* Initializes TVP5150 to stream enabled values */
tvp5150_write_inittab(sd, tvp5150_init_enable);
 
@@ -844,6 +849,7 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
@@ -1166,8 +1172,10 @@ static int tvp5150_set_format(struct v4l2_subdev *sd,
 
format->format = *mbus_format;
 
-   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+   if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
tvp5150_reset(sd, 0);
+   tvp5150_enable(sd);
+   }
 
v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", mbus_format->width,
mbus_format->height);
@@ -1431,6 +1439,7 @@ static int tvp5150_probe(struct i2c_client *c,
}
v4l2_ctrl_handler_setup(>hdl);
 
+   tvp5150_reset(sd, 0);
/* Default is no cropping */
tvp5150_set_default(tvp5150_read_std(sd), >rect, >format);
 
-- 
2.6.2

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

[PATCH 8/9] [media] tvp5150: Add sync lock interrupt handling

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds an optional interrupt handler to handle the sync
lock interrupt and sync lock status.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 103 ++--
 drivers/media/i2c/tvp5150_reg.h |   2 +
 2 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index abea26eb6fe0..9e006bf36e67 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -44,12 +45,14 @@ struct tvp5150 {
struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
+   int irq;
 
v4l2_std_id norm;   /* Current set standard */
v4l2_std_id detected_norm;
u32 input;
u32 output;
int enable;
+   bool lock;
 };
 
 static inline struct tvp5150 *to_tvp5150(struct v4l2_subdev *sd)
@@ -716,6 +719,15 @@ static int tvp5150_set_std(struct v4l2_subdev *sd, 
v4l2_std_id std)
return 0;
 }
 
+static int tvp5150_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
+{
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
+   *std = decoder->norm;
+
+   return 0;
+}
+
 static int tvp5150_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -758,14 +770,25 @@ static v4l2_std_id tvp5150_read_std(struct v4l2_subdev 
*sd)
 
 static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
+
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
-   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
-   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
-   /* Keep interrupt polarity active low */
-   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
-   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   if (decoder->irq) {
+   /* Configure pins: FID, VSYNC, INTREQ, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x0);
+   /* Set interrupt polarity to active high */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE | 0x1);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x1);
+   } else {
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+   }
 
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
@@ -776,6 +799,33 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
return 0;
 }
 
+static irqreturn_t tvp5150_isr(int irq, void *dev_id)
+{
+   struct tvp5150 *decoder = dev_id;
+   struct regmap *map = decoder->regmap;
+   unsigned int active = 0, status = 0;
+
+   regmap_read(map, TVP5150_INT_STATUS_REG_A, );
+   if (status) {
+   regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
+
+   if (status & TVP5150_INT_A_LOCK)
+   decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+
+   return IRQ_HANDLED;
+   }
+
+   regmap_read(map, TVP5150_INT_ACTIVE_REG_B, );
+   if (active) {
+   status = 0;
+   regmap_read(map, TVP5150_INT_STATUS_REG_B, );
+   if (status)
+   regmap_write(map, TVP5150_INT_RESET_REG_B, status);
+   }
+
+   return IRQ_HANDLED;
+}
+
 static int tvp5150_enable(struct v4l2_subdev *sd)
 {
struct tvp5150 *decoder = to_tvp5150(sd);
@@ -939,6 +989,35 @@ static int tvp5150_g_crop(struct v4l2_subdev *sd, struct 
v4l2_crop *a)
return 0;
 }
 
+static int tvp5150_s_stream(struct v4l2_subdev *sd, int enable)
+{
+   struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+
+   if (enable) {
+   /* Enable YUV(OUT7:0), clock */
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
+   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+   if (decoder->irq) {
+   /* Enable lock interrupt */
+   regmap_update_bits(decoder->regmap,
+  TVP5150_INT_ENABLE_REG_A,
+  TVP5150_INT_A_LOCK,
+  TVP5150_INT_A_LOCK);
+   }
+   } else {
+   /* Disable YUV(OUT7:0), SYNC, clock */
+   regmap_up

[PATCH 1/9] [media] tvp5150: convert register access to regmap

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

Regmap provides built in debugging, caching and provides dedicated accessors
for bit manipulations in registers, which make the following changes a lot
simpler.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 194 ++--
 1 file changed, 133 insertions(+), 61 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 3c5fb2509c47..a7495d2856c3 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,6 +38,7 @@ struct tvp5150 {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
struct v4l2_rect rect;
+   struct regmap *regmap;
 
v4l2_std_id norm;   /* Current set standard */
u32 input;
@@ -56,30 +58,14 @@ static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl 
*ctrl)
 
 static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
 {
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
-
-   rc = i2c_smbus_read_byte_data(c, addr);
-   if (rc < 0) {
-   v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
-   return rc;
-   }
-
-   v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, rc);
-
-   return rc;
-}
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   int ret, val;
 
-static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
-unsigned char value)
-{
-   struct i2c_client *c = v4l2_get_subdevdata(sd);
-   int rc;
+   ret = regmap_read(decoder->regmap, addr, );
+   if (ret < 0)
+   return ret;
 
-   v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, value);
-   rc = i2c_smbus_write_byte_data(c, addr, value);
-   if (rc < 0)
-   v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
+   return val;
 }
 
 static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
@@ -266,8 +252,8 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
decoder->input, decoder->output,
input, opmode);
 
-   tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
-   tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
+   regmap_write(decoder->regmap, TVP5150_OP_MODE_CTL, opmode);
+   regmap_write(decoder->regmap, TVP5150_VD_IN_SRC_SEL_1, input);
 
/* Svideo should enable YCrCb output and disable GPCL output
 * For Composite and TV, it should be the reverse
@@ -282,7 +268,7 @@ static inline void tvp5150_selmux(struct v4l2_subdev *sd)
val = (val & ~0x40) | 0x10;
else
val = (val & ~0x10) | 0x40;
-   tvp5150_write(sd, TVP5150_MISC_CTL, val);
+   regmap_write(decoder->regmap, TVP5150_MISC_CTL, val);
 };
 
 struct i2c_reg_value {
@@ -553,8 +539,10 @@ static struct i2c_vbi_ram_value vbi_ram_default[] =
 static int tvp5150_write_inittab(struct v4l2_subdev *sd,
const struct i2c_reg_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+
while (regs->reg != 0xff) {
-   tvp5150_write(sd, regs->reg, regs->value);
+   regmap_write(decoder->regmap, regs->reg, regs->value);
regs++;
}
return 0;
@@ -563,22 +551,24 @@ static int tvp5150_write_inittab(struct v4l2_subdev *sd,
 static int tvp5150_vdp_init(struct v4l2_subdev *sd,
const struct i2c_vbi_ram_value *regs)
 {
+   struct tvp5150 *decoder = to_tvp5150(sd);
+   struct regmap *map = decoder->regmap;
unsigned int i;
 
/* Disable Full Field */
-   tvp5150_write(sd, TVP5150_FULL_FIELD_ENA, 0);
+   regmap_write(map, TVP5150_FULL_FIELD_ENA, 0);
 
/* Before programming, Line mode should be at 0xff */
for (i = TVP5150_LINE_MODE_INI; i <= TVP5150_LINE_MODE_END; i++)
-   tvp5150_write(sd, i, 0xff);
+   regmap_write(map, i, 0xff);
 
/* Load Ram Table */
while (regs->reg != (u16)-1) {
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
-   tvp5150_write(sd, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_HIGH, regs->reg >> 8);
+   regmap_write(map, TVP5150_CONF_RAM_ADDR_LOW, regs->reg);
 
for (i = 0; i < 16; i++)
-   tvp5150_write(sd, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
+   regmap_write(map, TVP5150_VDP_CONF_RAM_DATA, 
regs->values[i]);
 
 

[PATCH 9/9] [media] tvp5150: disable output while signal not locked

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To avoid short frames on stream start, keep output pins at high impedance
while we are not properly locked onto the input signal.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 9e006bf36e67..9d03496d0af4 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -51,6 +51,7 @@ struct tvp5150 {
v4l2_std_id detected_norm;
u32 input;
u32 output;
+   u32 oe;
int enable;
bool lock;
 };
@@ -809,8 +810,11 @@ static irqreturn_t tvp5150_isr(int irq, void *dev_id)
if (status) {
regmap_write(map, TVP5150_INT_STATUS_REG_A, status);
 
-   if (status & TVP5150_INT_A_LOCK)
+   if (status & TVP5150_INT_A_LOCK) {
decoder->lock = !!(status & TVP5150_INT_A_LOCK_STATUS);
+   regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL,
+  0xd, decoder->lock ? decoder->oe : 
0);
+   }
 
return IRQ_HANDLED;
}
@@ -841,6 +845,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x7);
/* disable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x0);
+   decoder->oe = 0x9;
break;
case V4L2_MBUS_PARALLEL:
/* 8-bit YUV 4:2:2 */
@@ -848,6 +853,7 @@ static int tvp5150_enable(struct v4l2_subdev *sd)
   0x7, 0x0);
/* enable HSYNC, VSYNC/PALI, AVID, and FID/GLCO */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0x4, 0x4);
+   decoder->oe = 0xd;
break;
default:
return -EINVAL;
@@ -994,9 +1000,9 @@ static int tvp5150_s_stream(struct v4l2_subdev *sd, int 
enable)
struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
 
if (enable) {
-   /* Enable YUV(OUT7:0), clock */
+   /* Enable YUV(OUT7:0), (SYNC), clock signal, if locked */
regmap_update_bits(decoder->regmap, TVP5150_MISC_CTL, 0xd,
-   (decoder->bus_type == V4L2_MBUS_BT656) ? 0x9 : 0xd);
+  decoder->lock ? decoder->oe : 0);
if (decoder->irq) {
/* Enable lock interrupt */
regmap_update_bits(decoder->regmap,
-- 
2.6.2

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

[PATCH 7/9] [media] tvp5150: remove pin configuration from initialization tables

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

To allow optional interrupt support, we want to configure the pin settings
dynamically. Move those register accesses out of the static initialization
tables.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 19 +++
 drivers/media/i2c/tvp5150_reg.h |  1 +
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 791572737ee3..abea26eb6fe0 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -323,9 +323,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x0e */
TVP5150_LUMA_PROC_CTL_3,0x00
},
-   { /* 0x0f */
-   TVP5150_CONF_SHARED_PIN,0x08
-   },
{ /* 0x11 */
TVP5150_ACT_VD_CROP_ST_MSB,0x00
},
@@ -362,9 +359,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0x1d */
TVP5150_INT_ENABLE_REG_B,0x00
},
-   { /* 0x1e */
-   TVP5150_INTT_CONFIG_REG_B,0x00
-   },
{ /* 0x28 */
TVP5150_VIDEO_STD,0x00
},
@@ -383,9 +377,6 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
{ /* 0xc1 */
TVP5150_INT_ENABLE_REG_A,0x00
},
-   { /* 0xc2 */
-   TVP5150_INT_CONF,0x04
-   },
{ /* 0xc8 */
TVP5150_FIFO_INT_THRESHOLD,0x80
},
@@ -420,9 +411,7 @@ static const struct i2c_reg_value tvp5150_init_default[] = {
 
 /* Default values as sugested at TVP5150AM1 datasheet */
 static const struct i2c_reg_value tvp5150_init_enable[] = {
-   {
-   TVP5150_CONF_SHARED_PIN, 2
-   },{ /* Automatic offset and AGC enabled */
+   {   /* Automatic offset and AGC enabled */
TVP5150_ANAL_CHL_CTL, 0x15
},{ /* Activate YCrCb output 0x9 or 0xd ? */
TVP5150_MISC_CTL, 0x6f
@@ -772,6 +761,12 @@ static int tvp5150_reset(struct v4l2_subdev *sd, u32 val)
/* Initializes TVP5150 to its default values */
tvp5150_write_inittab(sd, tvp5150_init_default);
 
+   /* Configure pins: FID, VSYNC, GPCL/VBLK, SCLK */
+   regmap_write(map, TVP5150_CONF_SHARED_PIN, 0x2);
+   /* Keep interrupt polarity active low */
+   regmap_write(map, TVP5150_INT_CONF, TVP5150_VDPOE);
+   regmap_write(map, TVP5150_INTT_CONFIG_REG_B, 0x0);
+
/* Initializes VDP registers */
tvp5150_vdp_init(sd, vbi_ram_default);
 
diff --git a/drivers/media/i2c/tvp5150_reg.h b/drivers/media/i2c/tvp5150_reg.h
index 25a994944918..fc3bcb26413a 100644
--- a/drivers/media/i2c/tvp5150_reg.h
+++ b/drivers/media/i2c/tvp5150_reg.h
@@ -117,6 +117,7 @@
 #define TVP5150_INT_STATUS_REG_A0xc0 /* Interrupt status register A */
 #define TVP5150_INT_ENABLE_REG_A0xc1 /* Interrupt enable register A */
 #define TVP5150_INT_CONF0xc2 /* Interrupt configuration */
+#define   TVP5150_VDPOE BIT(2)
 #define TVP5150_VDP_CONF_RAM_DATA   0xc3 /* VDP configuration RAM data */
 #define TVP5150_CONF_RAM_ADDR_LOW   0xc4 /* Configuration RAM address low byte 
*/
 #define TVP5150_CONF_RAM_ADDR_HIGH  0xc5 /* Configuration RAM address high 
byte */
-- 
2.6.2

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

[PATCH 2/9] [media] tvp5150: add userspace subdev API

[Lucas Stach]

From: Philipp Zabel <p.za...@pengutronix.de>

This patch adds userspace V4L2 subdevice API support.

Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
---
 drivers/media/i2c/tvp5150.c | 259 ++--
 1 file changed, 200 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index a7495d2856c3..8670b478dcd6 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -36,7 +36,9 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 struct tvp5150 {
struct v4l2_subdev sd;
+   struct media_pad pad;
struct v4l2_ctrl_handler hdl;
+   struct v4l2_mbus_framefmt format;
struct v4l2_rect rect;
struct regmap *regmap;
 
@@ -819,38 +821,68 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+static void tvp5150_try_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
 {
-   struct v4l2_mbus_framefmt *f;
-   struct tvp5150 *decoder = to_tvp5150(sd);
+   unsigned int hmax;
 
-   if (!format || format->pad)
-   return -EINVAL;
+   /* Clamp the crop rectangle boundaries to tvp5150 limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp(rect->width,
+   TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect->left,
+   TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
 
-   f = >format;
+   /* tvp5150 has some special limits */
+   rect->left = clamp(rect->left, 0, TVP5150_MAX_CROP_LEFT);
+   rect->width = clamp_t(unsigned int, rect->width,
+ TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - 
rect->left,
+ TVP5150_H_MAX - rect->left);
+   rect->top = clamp(rect->top, 0, TVP5150_MAX_CROP_TOP);
 
-   tvp5150_reset(sd, 0);
+   /* Calculate height based on current standard */
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
 
-   f->width = decoder->rect.width;
-   f->height = decoder->rect.height;
+   rect->height = clamp(rect->height,
+hmax - TVP5150_MAX_CROP_TOP - rect->top,
+hmax - rect->top);
+}
 
-   f->code = MEDIA_BUS_FMT_UYVY8_2X8;
-   f->field = V4L2_FIELD_SEQ_TB;
-   f->colorspace = V4L2_COLORSPACE_SMPTE170M;
+static void tvp5150_set_crop(struct tvp5150 *decoder, struct v4l2_rect *rect,
+  v4l2_std_id std)
+{
+   struct regmap *map = decoder->regmap;
+   unsigned int hmax;
 
-   v4l2_dbg(1, debug, sd, "width = %d, height = %d\n", f->width,
-   f->height);
-   return 0;
+   if (std & V4L2_STD_525_60)
+   hmax = TVP5150_V_MAX_525_60;
+   else
+   hmax = TVP5150_V_MAX_OTHERS;
+
+   regmap_write(map, TVP5150_VERT_BLANKING_START, rect->top);
+   regmap_write(map, TVP5150_VERT_BLANKING_STOP,
+rect->top + rect->height - hmax);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_MSB,
+rect->left >> TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_ST_LSB,
+rect->left | (1 << TVP5150_CROP_SHIFT));
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_MSB,
+(rect->left + rect->width - TVP5150_MAX_CROP_LEFT) >>
+TVP5150_CROP_SHIFT);
+   regmap_write(map, TVP5150_ACT_VD_CROP_STP_LSB,
+rect->left + rect->width - TVP5150_MAX_CROP_LEFT);
+
+   decoder->rect = *rect;
 }
 
 static int tvp5150_s_crop(struct v4l2_subdev *sd, const struct v4l2_crop *a)
 {
-   struct v4l2_rect rect = a->c;
struct tvp5150 *decoder = to_tvp5150(sd);
+   struct v4l2_rect rect = a->c;
v4l2_std_id std;
-   unsigned int hmax;
 
v4l2_dbg(1, debug, sd, "%s left=%d, top=%d, width=%d, height=%d\n",
__func__, rect.left, rect.top, rect.width, rect.height);
@@ -858,42 +890,13 @@ static int tvp5150_s_crop(struct v4l2_subdev *sd, const 
struct v4l2_crop *a)
if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
 
-   /* tvp5150 has some special limits */
-   rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT);
-   rect.width = clamp_t(unsigned int, rect.width,
-TVP5150_H_MAX -

Re: [PATCHv2 4/4] drm: exynos: hdmi: add support for pixel clock limitation

[Lucas Stach]

Am Dienstag, den 15.04.2014, 11:27 +0200 schrieb Tomasz Stanislawski:
 Adds support for limitation of maximal pixel clock of HDMI
 signal. This feature is needed on boards that contains
 lines or bridges with frequency limitations.
 
 Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
 ---
  .../devicetree/bindings/video/exynos_hdmi.txt  |4 
  drivers/gpu/drm/exynos/exynos_hdmi.c   |   12 
  include/media/s5p_hdmi.h   |1 +
  3 files changed, 17 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/video/exynos_hdmi.txt 
 b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
 index f9187a2..8718f8d 100644
 --- a/Documentation/devicetree/bindings/video/exynos_hdmi.txt
 +++ b/Documentation/devicetree/bindings/video/exynos_hdmi.txt
 @@ -28,6 +28,10 @@ Required properties:
  - ddc: phandle to the hdmi ddc node
  - phy: phandle to the hdmi phy node
  
 +Optional properties:
 +- max-pixel-clock: used to limit the maximal pixel clock if a board has 
 lines,
 + connectors or bridges not capable of carring higher frequencies
 +
  Example:
  
   hdmi {
 diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
 b/drivers/gpu/drm/exynos/exynos_hdmi.c
 index 2a18f4e..404f1b7 100644
 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
 +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
 @@ -195,6 +195,7 @@ struct hdmi_context {
   struct hdmi_resources   res;
  
   int hpd_gpio;
 + u32 max_pixel_clock;
  
   enum hdmi_type  type;
  };
 @@ -887,6 +888,9 @@ static int hdmi_mode_valid(struct drm_connector 
 *connector,
   if (ret)
   return MODE_BAD;
  
 + if (mode-clock * 1000  hdata-max_pixel_clock)
 + return MODE_BAD;
 +
This should be MODE_CLOCK_HIGH

Regards,
Lucas
-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
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: [PATCHv5][ 2/8] staging: imx-drm: Add RGB666 support for parallel display.

[Lucas Stach]

Am Freitag, den 06.12.2013, 14:14 +0100 schrieb Thierry Reding:
 On Thu, Dec 05, 2013 at 07:28:06PM +0100, Denis Carikli wrote:
 [...]
  diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c 
  b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
 [...]
  @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
  return IPU_DC_MAP_BGR666;
  case V4L2_PIX_FMT_BGR24:
  return IPU_DC_MAP_BGR24;
  +   case V4L2_PIX_FMT_RGB666:
  +   return IPU_DC_MAP_RGB666;
 
 Why is this DRM driver even using V4L2 pixel formats in the first place?
 
Because imx-drm is actually a misnomer. The i.MX IPU is a multifunction
device, which as one part has the display controllers, but also camera
interfaces and mem-to-mem scaler devices, which are hooked up via the
V4L2 interface.

The generic IPU part, which is used for example for programming the DMA
channels is using V4L2 pixel formats as a common base. We have patches
to split this out and make this fact more visible. (The IPU core will be
placed aside the Tegra host1x driver)

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

--
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: [Linaro-mm-sig] [RFC 0/1] drm/pl111: Initial drm/kms driver for pl111

[Lucas Stach]

 and as such,
 the overhead is more pronounced.
 
 
 
   So in some respects, there is a constraint on how buffers which will
   be drawn to using the GPU are allocated. I don't really like the idea
   of teaching the display controller DRM driver about the GPU buffer
   constraints, even if they are fairly trivial like this. If the same
   display HW IP is being used on several SoCs, it seems wrong somehow
   to enforce those GPU constraints if some of those SoCs don't have a
   GPU.
  
  Well, I suppose you could get min_pitch_alignment from devicetree, or
  something like this..
  
  In the end, the easy solution is just to make the display allocate to
  the worst-case pitch alignment.  In the early days of dma-buf
  discussions, we kicked around the idea of negotiating or
  programatically describing the constraints, but that didn't really
  seem like a bounded problem.
 
 Yeah - I was around for some of those discussions and agree it's not
 really an easy problem to solve.
 
 
 
   We may also then have additional constraints when sharing buffers
   between the display HW and video decode or even camera ISP HW.
   Programmatically describing buffer allocation constraints is very
   difficult and I'm not sure you can actually do it - there's some
   pretty complex constraints out there! E.g. I believe there's a
   platform where Y and UV planes of the reference frame need to be in
   separate DRAM banks for real-time 1080p decode, or something like
   that?
  
  yes, this was discussed.  This is different from pitch/format/size
  constraints.. it is really just a placement constraint (ie. where do
  the physical pages go).  IIRC the conclusion was to use a dummy
  devices with it's own CMA pool for attaching the Y vs UV buffers.
  
   Anyway, I guess my point is that even if we solve how to allocate
   buffers which will be shared between the GPU and display HW such that
   both sets of constraints are satisfied, that may not be the end of
   the story.
  
  
  that was part of the reason to punt this problem to userspace ;-)
 
  In practice, the kernel drivers doesn't usually know too much about
  the dimensions/format/etc.. that is really userspace level knowledge.
  There are a few exceptions when the kernel needs to know how to setup
  GTT/etc for tiled buffers, but normally this sort of information is up
  at the next level up (userspace, and drm_framebuffer in case of
  scanout).  Userspace media frameworks like GStreamer already have a
  concept of format/caps negotiation.  For non-display-gpu sharing, I
  think this is probably where this sort of constraint negotiation
  should be handled.
 
 I agree that user-space will know which devices will access the buffer
 and thus can figure out at least a common pixel format. Though I'm not
 so sure userspace can figure out more low-level details like alignment
 and placement in physical memory, etc.
 
 Anyway, assuming user-space can figure out how a buffer should be 
 stored in memory, how does it indicate this to a kernel driver and 
 actually allocate it? Which ioctl on which device does user-space
 call, with what parameters? Are you suggesting using something like
 ION which exposes the low-level details of how buffers are laid out in
 physical memory to userspace? If not, what?
 

I strongly disagree with exposing low-level hardware details like tiling
to userspace. If we have to do the negotiation of those things in
userspace we will end up with having to pipe those information through
things like the wayland protocol. I don't see how this could ever be
considered a good idea.

I would rather see kernel drivers negotiating those things at dmabuf
attach time in way invisible to userspace. I agree that this negotiation
thing isn't easy to get right for the plethora of different hardware
constraints we see today, but I would rather see this in-kernel, where
we have the chance to fix things up if needed, than in a fixed userspace
interface.

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

--
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: [Linaro-mm-sig] [RFC 0/1] drm/pl111: Initial drm/kms driver for pl111

[Lucas Stach]

Am Dienstag, den 06.08.2013, 10:14 -0400 schrieb Rob Clark:
 On Tue, Aug 6, 2013 at 8:18 AM, Lucas Stach l.st...@pengutronix.de wrote:
  Am Dienstag, den 06.08.2013, 12:31 +0100 schrieb Tom Cooksey:
  Hi Rob,
 
  +lkml
 
On Fri, Jul 26, 2013 at 11:58 AM, Tom Cooksey tom.cook...@arm.com
wrote:
   * It abuses flags parameter of DRM_IOCTL_MODE_CREATE_DUMB to
 also allocate buffers for the GPU. Still not sure how to
 resolve this as we don't use DRM for our GPU driver.

 any thoughts/plans about a DRM GPU driver?  Ideally long term
 (esp. once the dma-fence stuff is in place), we'd have
 gpu-specific drm (gpu-only, no kms) driver, and SoC/display
 specific drm/kms driver, using prime/dmabuf to share between
 the two.

 The extra buffers we were allocating from armsoc DDX were really
 being allocated through DRM/GEM so we could get an flink name
 for them and pass a reference to them back to our GPU driver on
 the client side. If it weren't for our need to access those
 extra off-screen buffers with the GPU we wouldn't need to
 allocate them with DRM at all. So, given they are really GPU
 buffers, it does absolutely make sense to allocate them in a
 different driver to the display driver.

 However, to avoid unnecessary memcpys  related cache
 maintenance ops, we'd also like the GPU to render into buffers
 which are scanned out by the display controller. So let's say
 we continue using DRM_IOCTL_MODE_CREATE_DUMB to allocate scan
 out buffers with the display's DRM driver but a custom ioctl
 on the GPU's DRM driver to allocate non scanout, off-screen
 buffers. Sounds great, but I don't think that really works
 with DRI2. If we used two drivers to allocate buffers, which
 of those drivers do we return in DRI2ConnectReply? Even if we
 solve that somehow, GEM flink names are name-spaced to a
 single device node (AFAIK). So when we do a DRI2GetBuffers,
 how does the EGL in the client know which DRM device owns GEM
 flink name 1234? We'd need some pretty dirty hacks.
   
You would return the name of the display driver allocating the
buffers.  On the client side you can use generic ioctls to go from
flink - handle - dmabuf.  So the client side would end up opening
both the display drm device and the gpu, but without needing to know
too much about the display.
   
I think the bit I was missing was that a GEM bo for a buffer imported
using dma_buf/PRIME can still be flink'd. So the display controller's
DRM driver allocates scan-out buffers via the DUMB buffer allocate
ioctl. Those scan-out buffers than then be exported from the
dispaly's DRM driver and imported into the GPU's DRM driver using
PRIME. Once imported into the GPU's driver, we can use flink to get a
name for that buffer within the GPU DRM driver's name-space to return
to the DRI2 client. That same namespace is also what DRI2 back-
buffers are allocated from, so I think that could work... Except...
  
   (and.. the general direction is that things will move more to just use
   dmabuf directly, ie. wayland or dri3)
 
  I agree, DRI2 is the only reason why we need a system-wide ID. I also
  prefer buffers to be passed around by dma_buf fd, but we still need to
  support DRI2 and will do for some time I expect.
 
 
 
 Anyway, that latter case also gets quite difficult. The GPU
 DRM driver would need to know the constraints of the display
 controller when allocating buffers intended to be scanned out.
 For example, pl111 typically isn't behind an IOMMU and so
 requires physically contiguous memory. We'd have to teach the
 GPU's DRM driver about the constraints of the display HW. Not
 exactly a clean driver model. :-(

 I'm still a little stuck on how to proceed, so any ideas
 would greatly appreciated! My current train of thought is
 having a kind of SoC-specific DRM driver which allocates
 buffers for both display and GPU within a single GEM
 namespace. That SoC-specific DRM driver could then know the
 constraints of both the GPU and the display HW. We could then
 use PRIME to export buffers allocated with the SoC DRM driver
 and import them into the GPU and/or display DRM driver.
   
Usually if the display drm driver is allocating the buffers that
might be scanned out, it just needs to have minimal knowledge of
the GPU (pitch alignment constraints).  I don't think we need a
3rd device just to allocate buffers.
   
While Mali can render to pretty much any buffer, there is a mild
performance improvement to be had if the buffer stride is aligned to
the AXI bus's max burst length when drawing to the buffer.
  
   I suspect the display controllers might frequently benefit if the
   pitch is aligned to AXI burst length too..
 
  If the display controller is going to be reading

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

 even gets scheduled Task A overwrites the
buffer again. Not what you wanted, isn't it?

So to make sure the output of a pipeline of some kind is what you expect
you have to do syncing with IPC. And once you do CPU access it is a
synchronous thing in the stream of events. I see that you might want to
have some kind of bracketed CPU access even for the fallback mmap case
for things like V4L2 that don't provide explicit sync by their own, but
in no way I can see why we would need a user/kernel shared syncpoint for
this.

  A more advanced way to achieve this
  would be using cross-device fences to avoid going through userspace for
  every syncpoint.
  
 
 Ok, maybe there is something I missed. So question. What is the
 cross-device fences? dma fence?. And how we can achieve the
 synchronization mechanism without going through user space for every
 syncpoint; CPU and DMA share a same buffer?. And could you explain it
 in detail as long as possible like I did?
 
Yeah I'm talking about the proposed dma-fences. They would allow you to
just queue things into the kernel without waiting for a device operation
to finish. But you still have to make sure that your commands have the
right order and don't go wild. So for example you could do something
like this:

Userspace   Kernel
-   --
1. build DRM command stream
rendering into buf1
2. queue command stream with execbuf
1. validate command stream
 1.1 reference buf1 for writing
 through dma-buf-mgr
2. kick off GPU processing
3. qbuf buf1 into V4L2
3. reference buf1 for reading
 3.1 wait for fence from GPU to
 signal
4. kick off V4L2 processing

So you don't need to wait in userspace and potentially avoid some
context switches, but you still have to make sure that GPU commands are
queued before you queue the V4L2 operation to make sure things get
operated on in the right order.

Regards,
Lucas

-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

(buf1) as write, and data of the buffer(buf1) isn't
 needed to be shared*.  They just need to use the buffer as *storage*.
 So All they want is to avoid stomping on the buffer in this case.
  
Sorry, but I don't see the point. If no one is interested in the data of
the buffer, why are you sharing it in the first place?

 
 So to make sure the output of a pipeline of some kind is what
 you expect
 you have to do syncing with IPC
  
 So not true.
  
 . And once you do CPU access it is a
 synchronous thing in the stream of events. I see that you
 might want to
 have some kind of bracketed CPU access even for the fallback
 mmap case
 for things like V4L2 that don't provide explicit sync by their
 own, but
 in no way I can see why we would need a user/kernel shared
 syncpoint for
 this.
 
   A more advanced way to achieve this
   would be using cross-device fences to avoid going through
 userspace for
   every syncpoint.
  
 
  Ok, maybe there is something I missed. So question. What is
 the
  cross-device fences? dma fence?. And how we can achieve the
  synchronization mechanism without going through user space
 for every
  syncpoint; CPU and DMA share a same buffer?. And could you
 explain it
  in detail as long as possible like I did?
 
 
 Yeah I'm talking about the proposed dma-fences. They would
 allow you to
 just queue things into the kernel without waiting for a device
 operation
 to finish. But you still have to make sure that your commands
 have the
 right order and don't go wild. So for example you could do
 something
 like this:
 
 Userspace   Kernel
 -   --
 1. build DRM command stream
 rendering into buf1
 2. queue command stream with execbuf
 1. validate
 command stream
  1.1 reference
 buf1 for writing
  through
 dma-buf-mgr
 2. kick off GPU
 processing
 3. qbuf buf1 into V4L2
 3. reference buf1
 for reading
  3.1 wait for
 fence from GPU to
  signal
 4. kick off V4L2
 processing
 
  
 That seems like very specific to Desktop GPU. isn't it?
  
Would you mind explaining what you think is desktop specific about that?

Regards,
Lucas

-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

 the buffer doesn't mean it's
valid for your use-case. Without completion signaling you could easily
end up overwriting your data from task A multiple times before task B
even tries to lock the buffer for processing.

So the valid flow is (and this already works with the current APIs):
Task ATask B
----
CPU access buffer
 --completion signal-
  qbuf (dragging buffer into 
  device domain, flush caches,
  reserve buffer etc.)
|
  wait for device operation to
  complete
|
  dqbuf (dragging buffer back
  into CPU domain, invalidate
  caches, unreserve)
-completion signal
CPU access buffer


Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

Am Donnerstag, den 20.06.2013, 09:17 +0100 schrieb Russell King - ARM
Linux:
 On Thu, Jun 20, 2013 at 09:47:07AM +0200, Lucas Stach wrote:
  Am Donnerstag, den 20.06.2013, 15:43 +0900 schrieb Inki Dae:
   
-Original Message-
From: dri-devel-bounces+inki.dae=samsung@lists.freedesktop.org
[mailto:dri-devel-bounces+inki.dae=samsung@lists.freedesktop.org] On
Behalf Of Russell King - ARM Linux
Sent: Thursday, June 20, 2013 3:29 AM
To: Inki Dae
Cc: linux-fbdev; DRI mailing list; Kyungmin Park; myungjoo.ham; YoungJun
Cho; linux-media@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer 
synchronization
framework

On Thu, Jun 20, 2013 at 12:10:04AM +0900, Inki Dae wrote:
 On the other hand, the below shows how we could enhance the 
 conventional
 way with my approach (just example):

 CPU - DMA,
 ioctl(qbuf command)  ioctl(streamon)
   |   |
   |   |
 qbuf  - dma_buf_sync_get   start streaming - syncpoint

 dma_buf_sync_get just registers a sync buffer(dmabuf) to sync object.
And
 the syncpoint is performed by calling dma_buf_sync_lock(), and then 
 DMA
 accesses the sync buffer.

 And DMA - CPU,
 ioctl(dqbuf command)
   |
   |
 dqbuf - nothing to do

 Actual syncpoint is when DMA operation is completed (in interrupt
handler):
 the syncpoint is performed by calling dma_buf_sync_unlock().
 Hence,  my approach is to move the syncpoints into just before dma
access
 as long as possible.

What you've just described does *not* work on architectures such as
ARMv7 which do speculative cache fetches from memory at any time that
that memory is mapped with a cacheable status, and will lead to data
corruption.
   
   I didn't explain that enough. Sorry about that. 'nothing to do' means 
   that a
   dmabuf sync interface isn't called but existing functions are called. So
   this may be explained again:
   ioctl(dqbuf command)
   |
   |
   dqbuf - 1. dma_unmap_sg
   2. dma_buf_sync_unlock (syncpoint)
   
   The syncpoint I mentioned means lock mechanism; not doing cache operation.
   
   In addition, please see the below more detail examples.
   
   The conventional way (without dmabuf-sync) is:
   Task A 
   
1. CPU accesses buf  
2. Send the buf to Task B  
3. Wait for the buf from Task B
4. go to 1
   
   Task B
   ---
   1. Wait for the buf from Task A
   2. qbuf the buf 
   2.1 insert the buf to incoming queue
   3. stream on
   3.1 dma_map_sg if ready, and move the buf to ready queue
   3.2 get the buf from ready queue, and dma start.
   4. dqbuf
   4.1 dma_unmap_sg after dma operation completion
   4.2 move the buf to outgoing queue
   5. back the buf to Task A
   6. go to 1
   
   In case that two tasks share buffers, and data flow goes from Task A to 
   Task
   B, we would need IPC operation to send and receive buffers properly 
   between
   those two tasks every time CPU or DMA access to buffers is started or
   completed.
   
   
   With dmabuf-sync:
   
   Task A 
   
1. dma_buf_sync_lock - synpoint (call by user side)
2. CPU accesses buf  
3. dma_buf_sync_unlock - syncpoint (call by user side)
4. Send the buf to Task B (just one time)
5. go to 1
   
   
   Task B
   ---
   1. Wait for the buf from Task A (just one time)
   2. qbuf the buf 
   1.1 insert the buf to incoming queue
   3. stream on
   3.1 dma_buf_sync_lock - syncpoint (call by kernel side)
   3.2 dma_map_sg if ready, and move the buf to ready queue
   3.3 get the buf from ready queue, and dma start.
   4. dqbuf
   4.1 dma_buf_sync_unlock - syncpoint (call by kernel side)
   4.2 dma_unmap_sg after dma operation completion
   4.3 move the buf to outgoing queue
   5. go to 1
   
   On the other hand, in case of using dmabuf-sync, as you can see the above
   example, we would need IPC operation just one time. That way, I think we
   could not only reduce performance overhead but also make user application
   simplified. Of course, this approach can be used for all DMA device 
   drivers
   such as DRM. I'm not a specialist in v4l2 world so there may be missing
   point.
   
  
  You already need some kind of IPC between the two tasks, as I suspect
  even in your example it wouldn't make much sense to queue the buffer
  over and over again in task B without task A writing

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

 something to buf1
  3. dma_buf_sync_unlock - syncpoint (call by user side)
  4. copy buf1 to buf2
Random contents here? What's in the buffer, content from the CPU write,
or from V4L2 device write?

  5. go to 1
 
 
  Task B
  ---
  1. dma_buf_sync_lock
  2. CPU writes something to buf3
  3. dma_buf_sync_unlock
  4. qbuf the buf3(src) and buf1(dst)
  4.1 insert buf3,1 to incoming queue
  4.2 dma_buf_sync_lock - syncpoint (call by kernel side)
  5. stream on
  5.1 dma_map_sg if ready, and move the buf to ready queue
  5.2 get the buf from ready queue, and dma start.
  6. dqbuf
  6.1 dma_buf_sync_unlock - syncpoint (call by kernel side)
  6.2 dma_unmap_sg after dma operation completion
  6.3 move the buf3,1 to outgoing queue
 7. go to 1
 

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

Am Mittwoch, den 19.06.2013, 14:45 +0900 schrieb Inki Dae:
 
  -Original Message-
  From: Lucas Stach [mailto:l.st...@pengutronix.de]
  Sent: Tuesday, June 18, 2013 6:47 PM
  To: Inki Dae
  Cc: 'Russell King - ARM Linux'; 'linux-fbdev'; 'Kyungmin Park'; 'DRI
  mailing list'; 'myungjoo.ham'; 'YoungJun Cho'; linux-arm-
  ker...@lists.infradead.org; linux-media@vger.kernel.org
  Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization
  framework
  
  Am Dienstag, den 18.06.2013, 18:04 +0900 schrieb Inki Dae:
  [...]
  
a display device driver.  It shouldn't be used within a single driver
as a means of passing buffers between userspace and kernel space.
  
   What I try to do is not really such ugly thing. What I try to do is to
   notify that, when CPU tries to access a buffer , to kernel side through
   dmabuf interface. So it's not really to send the buffer to kernel.
  
   Thanks,
   Inki Dae
  
  The most basic question about why you are trying to implement this sort
  of thing in the dma_buf framework still stands.
  
  Once you imported a dma_buf into your DRM driver it's a GEM object and
  you can and should use the native DRM ioctls to prepare/end a CPU access
  to this BO. Then internally to your driver you can use the dma_buf
  reservation/fence stuff to provide the necessary cross-device sync.
  
 
 I don't really want that is used only for DRM drivers. We really need
 it for all other DMA devices; i.e., v4l2 based drivers. That is what I
 try to do. And my approach uses reservation to use dma-buf resources
 but not dma fence stuff anymore. However, I'm looking into Radeon DRM
 driver for why we need dma fence stuff, and how we can use it if
 needed.
 

Still I don't see the point why you need syncpoints above dma-buf. In
both the DRM and the V4L2 world we have defined points in the API where
a buffer is allowed to change domain from device to CPU and vice versa.

In DRM if you want to access a buffer with the CPU you do a cpu_prepare.
The buffer changes back to GPU domain once you do the execbuf
validation, queue a pageflip to the buffer or similar things.

In V4L2 the syncpoints for cache operations are the queue/dequeue API
entry points. Those are also the exact points to synchronize with other
hardware thus using dma-buf reserve/fence.

In all this I can't see any need for a new syncpoint primitive slapped
on top of dma-buf.

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

Am Mittwoch, den 19.06.2013, 19:44 +0900 schrieb Inki Dae:
 
  -Original Message-
  From: Lucas Stach [mailto:l.st...@pengutronix.de]
  Sent: Wednesday, June 19, 2013 7:22 PM
  To: Inki Dae
  Cc: 'Russell King - ARM Linux'; 'linux-fbdev'; 'Kyungmin Park'; 'DRI
  mailing list'; 'myungjoo.ham'; 'YoungJun Cho'; linux-arm-
  ker...@lists.infradead.org; linux-media@vger.kernel.org
  Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization
  framework
  
  Am Mittwoch, den 19.06.2013, 14:45 +0900 schrieb Inki Dae:
  
-Original Message-
From: Lucas Stach [mailto:l.st...@pengutronix.de]
Sent: Tuesday, June 18, 2013 6:47 PM
To: Inki Dae
Cc: 'Russell King - ARM Linux'; 'linux-fbdev'; 'Kyungmin Park'; 'DRI
mailing list'; 'myungjoo.ham'; 'YoungJun Cho'; linux-arm-
ker...@lists.infradead.org; linux-media@vger.kernel.org
Subject: Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer
  synchronization
framework
   
Am Dienstag, den 18.06.2013, 18:04 +0900 schrieb Inki Dae:
[...]

  a display device driver.  It shouldn't be used within a single
  driver
  as a means of passing buffers between userspace and kernel space.

 What I try to do is not really such ugly thing. What I try to do is
  to
 notify that, when CPU tries to access a buffer , to kernel side
  through
 dmabuf interface. So it's not really to send the buffer to kernel.

 Thanks,
 Inki Dae

The most basic question about why you are trying to implement this
  sort
of thing in the dma_buf framework still stands.
   
Once you imported a dma_buf into your DRM driver it's a GEM object and
you can and should use the native DRM ioctls to prepare/end a CPU
  access
to this BO. Then internally to your driver you can use the dma_buf
reservation/fence stuff to provide the necessary cross-device sync.
   
  
   I don't really want that is used only for DRM drivers. We really need
   it for all other DMA devices; i.e., v4l2 based drivers. That is what I
   try to do. And my approach uses reservation to use dma-buf resources
   but not dma fence stuff anymore. However, I'm looking into Radeon DRM
   driver for why we need dma fence stuff, and how we can use it if
   needed.
  
  
  Still I don't see the point why you need syncpoints above dma-buf. In
  both the DRM and the V4L2 world we have defined points in the API where
  a buffer is allowed to change domain from device to CPU and vice versa.
  
  In DRM if you want to access a buffer with the CPU you do a cpu_prepare.
  The buffer changes back to GPU domain once you do the execbuf
  validation, queue a pageflip to the buffer or similar things.
  
  In V4L2 the syncpoints for cache operations are the queue/dequeue API
  entry points. Those are also the exact points to synchronize with other
  hardware thus using dma-buf reserve/fence.
 
 
 If so, what if we want to access a buffer with the CPU _in V4L2_? We
 should open a drm device node, and then do a cpu_prepare? 
 
Not at all. As I said the syncpoints are the queue/dequeue operations.
When dequeueing a buffer you are explicitly dragging the buffer domain
back from device into userspace and thus CPU domain.

If you are operating on an mmap of a V4L2 processed buffer it's either
before or after it got processed by the hardware and therefore all DMA
operations on the buffer are bracketed by the V4L2 qbug/dqbuf ioctls.
That is where cache operations and synchronization should happen. The
V4L2 driver shouldn't allow you to dequeue a buffer and thus dragging it
back into CPU domain while there is still DMA ongoing. Equally the queue
ioctrl should make sure caches are properly written back to memory. The
results of reading/writing to the mmap of a V4L2 buffer while it is
enqueued to the hardware is simply undefined and there is nothing
suggesting that this is a valid usecase.

Regards,
Lucas

-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC PATCH v2] dmabuf-sync: Introduce buffer synchronization framework

[Lucas Stach]

Am Dienstag, den 18.06.2013, 18:04 +0900 schrieb Inki Dae:
[...]
 
  a display device driver.  It shouldn't be used within a single driver
  as a means of passing buffers between userspace and kernel space.
 
 What I try to do is not really such ugly thing. What I try to do is to
 notify that, when CPU tries to access a buffer , to kernel side through
 dmabuf interface. So it's not really to send the buffer to kernel.
 
 Thanks,
 Inki Dae
 
The most basic question about why you are trying to implement this sort
of thing in the dma_buf framework still stands.

Once you imported a dma_buf into your DRM driver it's a GEM object and
you can and should use the native DRM ioctls to prepare/end a CPU access
to this BO. Then internally to your driver you can use the dma_buf
reservation/fence stuff to provide the necessary cross-device sync.

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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

Re: [RFC][PATCH 0/2] dma-buf: add importer private data for reimporting

[Lucas Stach]

Am Freitag, den 31.05.2013, 17:29 +0200 schrieb Daniel Vetter:
 On Fri, May 31, 2013 at 07:22:24PM +0900, 김승우 wrote:
  Hello Daniel,
  
  Thanks for your comment.
  
  On 2013년 05월 31일 18:14, Daniel Vetter wrote:
   On Fri, May 31, 2013 at 10:54 AM, Seung-Woo Kim sw0312@samsung.com 
   wrote:
   importer private data in dma-buf attachment can be used by importer to
   reimport same dma-buf.
  
   Seung-Woo Kim (2):
 dma-buf: add importer private data to attachment
 drm/prime: find gem object from the reimported dma-buf
   
   Self-import should already work (at least with the latest refcount
   fixes merged). At least the tests to check both re-import on the same
   drm fd and on a different all work as expected now.
  
  Currently, prime works well for all case including self-importing,
  importing, and reimporting as you describe. Just, importing dma-buf from
  other driver twice with different drm_fd, each import create its own gem
  object even two import is done for same buffer because prime_priv is in
  struct drm_file. This means mapping to the device is done also twice.
  IMHO, these duplicated creations and maps are not necessary if drm can
  find previous import in different prime_priv.
 
 Well, that's imo a bug with the other driver. If it doesn't export
 something really simple (e.g. contiguous memory which doesn't require any
 mmio resources at all) it should have a cache of exported dma_buf fds so
 that it hands out the same dma_buf every time.

I agree with the reasoning here.

Though it would be nice to have this expected driver behavior put down
somewhere in the documentation. Any volunteers?

Regards,
Lucas
-- 
Pengutronix e.K.   | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

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