cron job: media_tree daily build: ERRORS

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

Results of the daily build of media_tree:

date:   Sat May 26 05:00:17 CEST 2018
media-tree git hash:e646e17713eeb3b6484b6d7a24ce34854123fa39
media_build git hash:   6d922b94bf0fcc9f0a1e52370b7d1d4fc2a59f8d
v4l-utils git hash: 2a12796b5c22cd1a549eb8fa25db873ced811ca5
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.16.0-1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.101-i686: ERRORS
linux-3.0.101-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.101-i686: ERRORS
linux-3.2.101-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.113-i686: ERRORS
linux-3.4.113-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.10-i686: ERRORS
linux-3.7.10-x86_64: ERRORS
linux-3.8.13-i686: ERRORS
linux-3.8.13-x86_64: ERRORS
linux-3.9.11-i686: ERRORS
linux-3.9.11-x86_64: ERRORS
linux-3.10.108-i686: ERRORS
linux-3.10.108-x86_64: ERRORS
linux-3.11.10-i686: ERRORS
linux-3.11.10-x86_64: ERRORS
linux-3.12.74-i686: ERRORS
linux-3.12.74-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.79-i686: ERRORS
linux-3.14.79-x86_64: ERRORS
linux-3.15.10-i686: ERRORS
linux-3.15.10-x86_64: ERRORS
linux-3.16.56-i686: ERRORS
linux-3.16.56-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.102-i686: ERRORS
linux-3.18.102-x86_64: ERRORS
linux-3.19.8-i686: ERRORS
linux-3.19.8-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.51-i686: ERRORS
linux-4.1.51-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.109-i686: ERRORS
linux-4.4.109-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.10-i686: ERRORS
linux-4.7.10-x86_64: ERRORS
linux-4.8.17-i686: ERRORS
linux-4.8.17-x86_64: ERRORS
linux-4.9.91-i686: ERRORS
linux-4.9.91-x86_64: ERRORS
linux-4.10.17-i686: ERRORS
linux-4.10.17-x86_64: ERRORS
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.42-i686: OK
linux-4.14.42-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
linux-4.16.8-i686: OK
linux-4.16.8-x86_64: OK
linux-4.17-rc4-i686: OK
linux-4.17-rc4-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL

2018-05-25 Thread Nicolas Dufresne
Le vendredi 25 mai 2018 à 21:14 -0400, Nicolas Dufresne a écrit :
> Le vendredi 25 mai 2018 à 17:19 -0700, Steve Longerbeam a écrit :
> > 
> > On 05/25/2018 05:10 PM, Nicolas Dufresne wrote:
> > > (in text this time, sorry)
> > > 
> > > Le vendredi 25 mai 2018 à 16:53 -0700, Steve Longerbeam a écrit :
> > > > Add a macro that returns true if the given field type is
> > > > 'sequential',
> > > > that is, the data is transmitted, or exists in memory, as all top
> > > > field
> > > > lines followed by all bottom field lines, or vice-versa.
> > > > 
> > > > Signed-off-by: Steve Longerbeam 
> > > > ---
> > > >   include/uapi/linux/videodev2.h | 4 
> > > >   1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/include/uapi/linux/videodev2.h
> > > > b/include/uapi/linux/videodev2.h
> > > > index 600877b..408ee96 100644
> > > > --- a/include/uapi/linux/videodev2.h
> > > > +++ b/include/uapi/linux/videodev2.h
> > > > @@ -126,6 +126,10 @@ enum v4l2_field {
> > > >  (field) == V4L2_FIELD_INTERLACED_BT ||\
> > > >  (field) == V4L2_FIELD_SEQ_TB ||\
> > > >  (field) == V4L2_FIELD_SEQ_BT)
> > > > +#define V4L2_FIELD_IS_SEQUENTIAL(field) \
> > > > +   ((field) == V4L2_FIELD_SEQ_TB ||\
> > > > +(field) == V4L2_FIELD_SEQ_BT ||\
> > > > +(field) == V4L2_FIELD_ALTERNATE)
> > > 
> > > No, alternate has no place here, in alternate mode each buffers have
> > > only one field.
> > 
> > Then I misunderstand what is meant by "alternate". The name implies
> > to me that a source sends top or bottom field alternately, or top/bottom
> > fields exist in memory buffers alternately, but with no information about
> > which field came first. In other words, "alternate" is either seq-tb or 
> > seq-bt,
> > without any info about field order.
> > 
> > If it is just one field in a memory buffer, why isn't it called
> > V4L2_FIELD_TOP_OR_BOTTOM, e.g. we don't know which?
> 
> I don't see why this could be better then ALTERNATE, were buffers are
> only top or bottom fields alternatively. And even if there was another
> possible name, this is public API.
> 
> V4L2_FIELD_ALTERNATE is a mode, that will only be used with
> v4l2_pix_format or v4l2_pix_format_mplane. I should never bet set on
> the v4l2_buffer.field, instead the driver indicates the parity of the
> field by setting V42_FIELD_TOP/BOTTOM on the v4l2_buffer returned by
> DQBUF. This is a very different mode of operation compared to
> sequential, hence why I believe it is wrong to make it part of the new
> helper. So far, it's the only field value that has this asymmetric
> usage and meaning.

I should have put some references. The explanation of the modes, with a
temporal representation of the fields. Small note, in ALTERNATE mode
bottom and top fields will likely not share the same timestamp, it is a
mode used to achieve lower latency.

https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/field-order.html#c.v4l2_field

And in this section, you'll see a paragraph that explain the field
values when running in ALTERNATE mode.

https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/buffer.html#c.v4l2_buffer

> 
> > 
> > Steve
> > 


Re: [PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL

2018-05-25 Thread Nicolas Dufresne
Le vendredi 25 mai 2018 à 17:19 -0700, Steve Longerbeam a écrit :
> 
> On 05/25/2018 05:10 PM, Nicolas Dufresne wrote:
> > (in text this time, sorry)
> > 
> > Le vendredi 25 mai 2018 à 16:53 -0700, Steve Longerbeam a écrit :
> > > Add a macro that returns true if the given field type is
> > > 'sequential',
> > > that is, the data is transmitted, or exists in memory, as all top
> > > field
> > > lines followed by all bottom field lines, or vice-versa.
> > > 
> > > Signed-off-by: Steve Longerbeam 
> > > ---
> > >   include/uapi/linux/videodev2.h | 4 
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/videodev2.h
> > > b/include/uapi/linux/videodev2.h
> > > index 600877b..408ee96 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -126,6 +126,10 @@ enum v4l2_field {
> > >(field) == V4L2_FIELD_INTERLACED_BT ||\
> > >(field) == V4L2_FIELD_SEQ_TB ||\
> > >(field) == V4L2_FIELD_SEQ_BT)
> > > +#define V4L2_FIELD_IS_SEQUENTIAL(field) \
> > > + ((field) == V4L2_FIELD_SEQ_TB ||\
> > > +  (field) == V4L2_FIELD_SEQ_BT ||\
> > > +  (field) == V4L2_FIELD_ALTERNATE)
> > 
> > No, alternate has no place here, in alternate mode each buffers have
> > only one field.
> 
> Then I misunderstand what is meant by "alternate". The name implies
> to me that a source sends top or bottom field alternately, or top/bottom
> fields exist in memory buffers alternately, but with no information about
> which field came first. In other words, "alternate" is either seq-tb or 
> seq-bt,
> without any info about field order.
> 
> If it is just one field in a memory buffer, why isn't it called
> V4L2_FIELD_TOP_OR_BOTTOM, e.g. we don't know which?

I don't see why this could be better then ALTERNATE, were buffers are
only top or bottom fields alternatively. And even if there was another
possible name, this is public API.

V4L2_FIELD_ALTERNATE is a mode, that will only be used with
v4l2_pix_format or v4l2_pix_format_mplane. I should never bet set on
the v4l2_buffer.field, instead the driver indicates the parity of the
field by setting V42_FIELD_TOP/BOTTOM on the v4l2_buffer returned by
DQBUF. This is a very different mode of operation compared to
sequential, hence why I believe it is wrong to make it part of the new
helper. So far, it's the only field value that has this asymmetric
usage and meaning.

> 
> Steve
> 


Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation

2018-05-25 Thread Laurent Pinchart
Hi Mauro,

On Saturday, 26 May 2018 02:39:16 EEST Laurent Pinchart wrote:
> On Saturday, 26 May 2018 02:10:27 EEST Mauro Carvalho Chehab wrote:
> > Em Sun, 20 May 2018 15:10:50 +0300 Laurent Pinchart escreveu:
> >> Hi Mauro,
> >> 
> >> The following changes since commit
> >> 
> >> 8ed8bba70b4355b1ba029b151ade84475dd12991:
> >>   media: imx274: remove non-indexed pointers from mode_table (2018-05-17
> >> 
> >> 06:22:08 -0400)
> >> 
> >> are available in the Git repository at:
> >>   git://linuxtv.org/pinchartl/media.git v4l2/vsp1/next
> >> 
> >> for you to fetch changes up to 429f256501652c90a4ed82f2416618f82a77d37c:
> >>   media: vsp1: Move video configuration to a cached dlb (2018-05-20
> >>   09:46:51 +0300)
> >> 
> >> The branch passes the VSP and DU test suites, both on its own and when
> >> merged with the drm-next branch.
> > 
> > This series added a new warning:
> > 
> > drivers/media/platform/vsp1/vsp1_dl.c:69: warning: Function parameter or
> > member 'refcnt' not described in 'vsp1_dl_body'
> 
> We'll fix that. Kieran, as you authored the code, would you like to give it
> a go ?
> 
> > To the already existing one:
> > 
> > drivers/media/platform/vsp1/vsp1_drm.c:336 vsp1_du_pipeline_setup_brx()
> > error: we previously assumed 'pipe->brx' could be null (see line 244)
> 
> That's still on my todo list. I tried to give it a go but received plenty of
> SQL errors. How do you run smatch ?

Nevermind, I found out what was wrong (had to specify the data directory 
manually).

I've reproduced the issue and created a minimal test case.

 1. struct vsp1_pipeline;
 2.   
 3. struct vsp1_entity {
 4. struct vsp1_pipeline *pipe;
 5. struct vsp1_entity *sink;
 6. unsigned int source_pad;
 7. };
 8. 
 9. struct vsp1_pipeline {
10. struct vsp1_entity *brx;
11. };
12. 
13. struct vsp1_brx {
14. struct vsp1_entity entity;
15. };
16. 
17. struct vsp1_device {
18. struct vsp1_brx *bru;
19. struct vsp1_brx *brs;
20. };
21. 
22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline *pipe)
23. {
24. struct vsp1_entity *brx;
25. 
26. if (pipe->brx)
27. brx = pipe->brx;
28. else if (!vsp1->bru->entity.pipe)
29. brx = >bru->entity;
30. else
31. brx = >brs->entity;
32. 
33. if (brx != pipe->brx)
34. pipe->brx = brx;
35. 
36. return pipe->brx->source_pad;
37. }

The reason why smatch complains is that it has no guarantee that vsp1->brs is 
not NULL. It's quite tricky:

- On line 26, smatch assumes that pipe->brx can be NULL
- On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL due 
to line 26)
- On line 28, smatch assumes that vsp1->bru is not NULL
- On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL due 
to line 28)
- On line 31, brx is assigned a possibly NULL value (as there's no information 
regarding vsp1->brs)
- On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
- On line 36 pipe->brx is dereferenced

The problem comes from the fact that smatch assumes that vsp1->brs isn't NULL. 
Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the warning 
disappear.

So how do we know that vsp1->brs isn't NULL in the original code ?

if (pipe->num_inputs > 2)
brx = >bru->entity;
else if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
else if (!vsp1->bru->entity.pipe)
brx = >bru->entity;
else
brx = >brs->entity;

A VSP1 instance can have no brs, so in general vsp1->brs can be NULL. However, 
when that's the case, the following conditions are fulfilled.

- drm_pipe->force_brx_release will be false
- either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be NULL

The fourth branch should thus never be taken.

I don't think we could teach smatch to detect this, it's too complicated. One 
possible fix for the warning that wouldn't just silence it artificially could 
be

if (pipe->num_inputs > 2)
brx = >bru->entity;
else if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
else if (!vsp1->bru->entity.pipe)
brx = >bru->entity;
else if (vsp1->brs)
brx = >brs->entity;
else
return -EINVAL;

But running the test case again, this still produces a warning. Now I'm 
getting puzzled, I don't see how smatch can still believe brx could be NULL.

> > (there's also a Spectre warning too, but I'll looking into those
> > in separate).

[snip]

-- 
Regards,

Laurent Pinchart





Re: [PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL

2018-05-25 Thread Steve Longerbeam



On 05/25/2018 05:10 PM, Nicolas Dufresne wrote:

(in text this time, sorry)

Le vendredi 25 mai 2018 à 16:53 -0700, Steve Longerbeam a écrit :

Add a macro that returns true if the given field type is
'sequential',
that is, the data is transmitted, or exists in memory, as all top
field
lines followed by all bottom field lines, or vice-versa.

Signed-off-by: Steve Longerbeam 
---
  include/uapi/linux/videodev2.h | 4 
  1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h
b/include/uapi/linux/videodev2.h
index 600877b..408ee96 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -126,6 +126,10 @@ enum v4l2_field {
 (field) == V4L2_FIELD_INTERLACED_BT ||\
 (field) == V4L2_FIELD_SEQ_TB ||\
 (field) == V4L2_FIELD_SEQ_BT)
+#define V4L2_FIELD_IS_SEQUENTIAL(field) \
+   ((field) == V4L2_FIELD_SEQ_TB ||\
+(field) == V4L2_FIELD_SEQ_BT ||\
+(field) == V4L2_FIELD_ALTERNATE)

No, alternate has no place here, in alternate mode each buffers have
only one field.


Then I misunderstand what is meant by "alternate". The name implies
to me that a source sends top or bottom field alternately, or top/bottom
fields exist in memory buffers alternately, but with no information about
which field came first. In other words, "alternate" is either seq-tb or 
seq-bt,

without any info about field order.

If it is just one field in a memory buffer, why isn't it called
V4L2_FIELD_TOP_OR_BOTTOM, e.g. we don't know which?

Steve



Re: [PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL

2018-05-25 Thread Nicolas Dufresne
(in text this time, sorry)

Le vendredi 25 mai 2018 à 16:53 -0700, Steve Longerbeam a écrit :
> Add a macro that returns true if the given field type is
> 'sequential',
> that is, the data is transmitted, or exists in memory, as all top
> field
> lines followed by all bottom field lines, or vice-versa.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  include/uapi/linux/videodev2.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/videodev2.h
> b/include/uapi/linux/videodev2.h
> index 600877b..408ee96 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -126,6 +126,10 @@ enum v4l2_field {
>(field) == V4L2_FIELD_INTERLACED_BT ||\
>(field) == V4L2_FIELD_SEQ_TB ||\
>(field) == V4L2_FIELD_SEQ_BT)
> +#define V4L2_FIELD_IS_SEQUENTIAL(field) \
> + ((field) == V4L2_FIELD_SEQ_TB ||\
> +  (field) == V4L2_FIELD_SEQ_BT ||\
> +  (field) == V4L2_FIELD_ALTERNATE)

No, alternate has no place here, in alternate mode each buffers have
only one field.

>  #define V4L2_FIELD_HAS_T_OR_B(field) \
>   ((field) == V4L2_FIELD_BOTTOM (||\
>(field) == V4L2_FIELD_TOP ||\


[PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL

2018-05-25 Thread Steve Longerbeam
Add a macro that returns true if the given field type is 'sequential',
that is, the data is transmitted, or exists in memory, as all top field
lines followed by all bottom field lines, or vice-versa.

Signed-off-by: Steve Longerbeam 
---
 include/uapi/linux/videodev2.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 600877b..408ee96 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -126,6 +126,10 @@ enum v4l2_field {
 (field) == V4L2_FIELD_INTERLACED_BT ||\
 (field) == V4L2_FIELD_SEQ_TB ||\
 (field) == V4L2_FIELD_SEQ_BT)
+#define V4L2_FIELD_IS_SEQUENTIAL(field) \
+   ((field) == V4L2_FIELD_SEQ_TB ||\
+(field) == V4L2_FIELD_SEQ_BT ||\
+(field) == V4L2_FIELD_ALTERNATE)
 #define V4L2_FIELD_HAS_T_OR_B(field)   \
((field) == V4L2_FIELD_BOTTOM ||\
 (field) == V4L2_FIELD_TOP ||\
-- 
2.7.4



[PATCH 6/6] media: staging/imx: interweave and odd-chroma-row skip are incompatible

2018-05-25 Thread Steve Longerbeam
If IDMAC interweaving is enabled in a write channel, the channel must
write the odd chroma rows for 4:2:0 formats. Skipping writing the odd
chroma rows produces corrupted captured 4:2:0 images when interweave
is enabled.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 18 +-
 drivers/staging/media/imx/imx-media-csi.c   | 18 --
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ae453fd..b63b3f4 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -353,6 +353,7 @@ static int prp_setup_channel(struct prp_priv *priv,
struct v4l2_mbus_framefmt *infmt;
unsigned int burst_size;
struct ipu_image image;
+   bool interweave;
int ret;
 
infmt = >format_mbus[PRPENCVF_SINK_PAD];
@@ -365,6 +366,10 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
 
+   interweave = (image.pix.field == V4L2_FIELD_NONE &&
+ V4L2_FIELD_HAS_BOTH(infmt->field) &&
+ channel == priv->out_ch);
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -377,12 +382,17 @@ static int prp_setup_channel(struct prp_priv *priv,
image.phys0 = addr0;
image.phys1 = addr1;
 
-   if (channel == priv->out_ch || channel == priv->rot_out_ch) {
+   /*
+* Skip writing U and V components to odd rows in the output
+* channels for planar 4:2:0 (but not when enabling IDMAC
+* interweaving, they are incompatible).
+*/
+   if (!interweave && (channel == priv->out_ch ||
+   channel == priv->rot_out_ch)) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
-   /* Skip writing U and V components to odd rows */
ipu_cpmem_skip_odd_chroma_rows(channel);
break;
}
@@ -405,9 +415,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
 
-   if (image.pix.field == V4L2_FIELD_NONE &&
-   V4L2_FIELD_HAS_BOTH(infmt->field) &&
-   channel == priv->out_ch)
+   if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
 
ret = ipu_ic_task_idma_init(priv->ic, channel,
diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 6829c08..ab2de71 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -368,10 +368,10 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 {
struct imx_media_video_dev *vdev = priv->vdev;
struct v4l2_mbus_framefmt *infmt;
+   bool passthrough, interweave;
struct ipu_image image;
u32 passthrough_bits;
dma_addr_t phys[2];
-   bool passthrough;
u32 burst_size;
int ret;
 
@@ -389,6 +389,10 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
image.phys0 = phys[0];
image.phys1 = phys[1];
 
+   interweave = (image.pix.field == V4L2_FIELD_NONE &&
+ (V4L2_FIELD_HAS_BOTH(infmt->field) ||
+  infmt->field == V4L2_FIELD_ALTERNATE));
+
/*
 * Check for conditions that require the IPU to handle the
 * data internally as generic data, aka passthrough mode:
@@ -422,8 +426,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
  ((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
passthrough = is_parallel_16bit_bus(>upstream_ep);
passthrough_bits = 16;
-   /* Skip writing U and V components to odd rows */
-   ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
+   /*
+* Skip writing U and V components to odd rows (but not
+* when enabling IDMAC interweaving, they are incompatible).
+*/
+   if (!interweave)
+   ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
break;
case V4L2_PIX_FMT_YUYV:
case V4L2_PIX_FMT_UYVY:
@@ -477,9 +485,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 
ipu_smfc_set_burstsize(priv->smfc, burst_size);
 
-   if (image.pix.field == V4L2_FIELD_NONE &&
-   (V4L2_FIELD_HAS_BOTH(infmt->field) ||
-infmt->field == V4L2_FIELD_ALTERNATE))
+   if (interweave)

[PATCH 5/6] media: imx-csi: Allow skipping odd chroma rows for YVU420

2018-05-25 Thread Steve Longerbeam
Skip writing U/V components to odd rows for YVU420 in addition to
YUV420 and NV12.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-media-csi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index eef3483..6829c08 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -415,6 +415,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
passthrough_bits = 16;
break;
case V4L2_PIX_FMT_YUV420:
+   case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
burst_size = (image.pix.width & 0x3f) ?
 ((image.pix.width & 0x1f) ?
-- 
2.7.4



[PATCH 2/6] gpu: ipu-csi: Check for field type alternate

2018-05-25 Thread Steve Longerbeam
When the CSI is receiving from a bt.656 bus, include a check for
field type 'alternate' when determining whether to set CSI clock
mode to CCIR656_INTERLACED or CCIR656_PROGRESSIVE.

Signed-off-by: Steve Longerbeam 
---
 drivers/gpu/ipu-v3/ipu-csi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index caa05b0..5450a2d 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -339,7 +339,8 @@ static void fill_csi_bus_cfg(struct ipu_csi_bus_config 
*csicfg,
break;
case V4L2_MBUS_BT656:
csicfg->ext_vsync = 0;
-   if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field))
+   if (V4L2_FIELD_HAS_BOTH(mbus_fmt->field) ||
+   mbus_fmt->field == V4L2_FIELD_ALTERNATE)
csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_INTERLACED;
else
csicfg->clk_mode = IPU_CSI_CLK_MODE_CCIR656_PROGRESSIVE;
-- 
2.7.4



[PATCH 0/6] imx-media: Fixes for bt.656 interlaced capture

2018-05-25 Thread Steve Longerbeam
A set of patches that fixes some bugs with capturing from a bt.656
interlaced source, and incompatibilites between IDMAC interlace
interweaving and 4:2:0 data write reduction.

Steve Longerbeam (6):
  media: imx-csi: Fix interlaced bt.656
  gpu: ipu-csi: Check for field type alternate
  media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL
  media: imx-csi: Enable interlaced scan for field type alternate
  media: imx-csi: Allow skipping odd chroma rows for YVU420
  media: staging/imx: interweave and odd-chroma-row skip are
incompatible

 drivers/gpu/ipu-v3/ipu-csi.c|  3 ++-
 drivers/staging/media/imx/imx-ic-prpencvf.c | 18 -
 drivers/staging/media/imx/imx-media-csi.c   | 31 ++---
 include/uapi/linux/videodev2.h  |  4 
 4 files changed, 34 insertions(+), 22 deletions(-)

-- 
2.7.4



[PATCH 4/6] media: imx-csi: Enable interlaced scan for field type alternate

2018-05-25 Thread Steve Longerbeam
Interlaced scan, a.k.a. interweave, should be enabled at the CSI IDMAC
output pad if the input field type is 'alternate' (in addition to field
types 'seq-tb' and 'seq-bt').

Which brings up whether V4L2_FIELD_HAS_BOTH() macro should be used
to determine enabling interlaced/interweave scan. That macro
includes the 'interlaced' field types, and in those cases the data
is already interweaved with top/bottom field lines. A heads-up for
now that this if statement may need to call V4L2_FIELD_IS_SEQUENTIAL()
instead, I have no sensor hardware that sends 'interlaced' data, so can't
test.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-media-csi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 9bc555c..eef3483 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -477,7 +477,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
ipu_smfc_set_burstsize(priv->smfc, burst_size);
 
if (image.pix.field == V4L2_FIELD_NONE &&
-   V4L2_FIELD_HAS_BOTH(infmt->field))
+   (V4L2_FIELD_HAS_BOTH(infmt->field) ||
+infmt->field == V4L2_FIELD_ALTERNATE))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
  image.pix.bytesperline);
 
-- 
2.7.4



[PATCH 1/6] media: imx-csi: Fix interlaced bt.656

2018-05-25 Thread Steve Longerbeam
The output pad's field type was being passed to ipu_csi_init_interface(),
in order to deal with field type 'alternate' at the sink pad, which
must be translated to either 'seq-tb' or 'seq-bt' to be understood by
ipu_csi_init_interface(). Doing so breaks the CSI interface setup if
the output pad field type is set to 'none' and the input bus type is
BT.656.

So remove that code and pass unmodified sink pad field type to
ipu_csi_init_interface(). The latter function will have to explicity
deal with field type 'alternate' when setting up the CSI interface
for BT.656 busses.

Signed-off-by: Steve Longerbeam 
---
 drivers/staging/media/imx/imx-media-csi.c | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 95d7805..9bc555c 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -629,12 +629,10 @@ static void csi_idmac_stop(struct csi_priv *priv)
 /* Update the CSI whole sensor and active windows */
 static int csi_setup(struct csi_priv *priv)
 {
-   struct v4l2_mbus_framefmt *infmt, *outfmt;
+   struct v4l2_mbus_framefmt *infmt;
struct v4l2_mbus_config mbus_cfg;
-   struct v4l2_mbus_framefmt if_fmt;
 
infmt = >format_mbus[CSI_SINK_PAD];
-   outfmt = >format_mbus[priv->active_output_pad];
 
/* compose mbus_config from the upstream endpoint */
mbus_cfg.type = priv->upstream_ep.bus_type;
@@ -642,20 +640,13 @@ static int csi_setup(struct csi_priv *priv)
priv->upstream_ep.bus.mipi_csi2.flags :
priv->upstream_ep.bus.parallel.flags;
 
-   /*
-* we need to pass input frame to CSI interface, but
-* with translated field type from output format
-*/
-   if_fmt = *infmt;
-   if_fmt.field = outfmt->field;
-
ipu_csi_set_window(priv->csi, >crop);
 
ipu_csi_set_downsize(priv->csi,
 priv->crop.width == 2 * priv->compose.width,
 priv->crop.height == 2 * priv->compose.height);
 
-   ipu_csi_init_interface(priv->csi, _cfg, _fmt);
+   ipu_csi_init_interface(priv->csi, _cfg, infmt);
 
ipu_csi_set_dest(priv->csi, priv->dest);
 
-- 
2.7.4



Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Steve Longerbeam

Hi Krzysztof, Philipp,


On 05/25/2018 12:18 AM, Krzysztof Hałasa wrote:

Philipp Zabel  writes:


Maybe scanline interlave and double write reduction can't be used at the
same time?

Well, if it works in non-interlaced modes - it may be the case.

Perhaps the data reduction is done before the field merge step.


Yeah, that might explain the incompatibility. The IDMAC top/bottom
line merging needs all the lines present. It won't have them if the
IDMAC has previously skipped the odd chroma lines. Or maybe I'm
over-simplifying.

In any case as I said they are proved to be incompatible. I am
preparing a patch-set with these fixes.

Krzysztof, in the meantime the patches are available in my
media-tree fork, for testing on the Ventana GW5300:

g...@github.com:slongerbeam/mediatree.git, branch 'fix-csi-interlaced'

Steve



Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation

2018-05-25 Thread Laurent Pinchart
Hi Mauro,

(CC'ing Kieran)

On Saturday, 26 May 2018 02:10:27 EEST Mauro Carvalho Chehab wrote:
> Em Sun, 20 May 2018 15:10:50 +0300 Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > The following changes since commit
> > 8ed8bba70b4355b1ba029b151ade84475dd12991:
> >   media: imx274: remove non-indexed pointers from mode_table (2018-05-17
> > 06:22:08 -0400)
> > 
> > are available in the Git repository at:
> >   git://linuxtv.org/pinchartl/media.git v4l2/vsp1/next
> > 
> > for you to fetch changes up to 429f256501652c90a4ed82f2416618f82a77d37c:
> >   media: vsp1: Move video configuration to a cached dlb (2018-05-20
> >   09:46:51 +0300)
> > 
> > The branch passes the VSP and DU test suites, both on its own and when
> > merged with the drm-next branch.
> 
> This series added a new warning:
> 
> drivers/media/platform/vsp1/vsp1_dl.c:69: warning: Function parameter or
> member 'refcnt' not described in 'vsp1_dl_body'

We'll fix that. Kieran, as you authored the code, would you like to give it a 
go ?

> To the already existing one:
> 
> drivers/media/platform/vsp1/vsp1_drm.c:336 vsp1_du_pipeline_setup_brx()
> error: we previously assumed 'pipe->brx' could be null (see line 244)

That's still on my todo list. I tried to give it a go but received plenty of 
SQL errors. How do you run smatch ?

> (there's also a Spectre warning too, but I'll looking into those
> in separate).
> 
> For now, I'll apply it, but I reserve the right of not pulling any
> new patchsets that would add more warnings.
> 
> > 
> > 
> > Geert Uytterhoeven (1):
> >   media: vsp1: Drop OF dependency of VIDEO_RENESAS_VSP1
> > 
> > Kieran Bingham (10):
> >   media: vsp1: Release buffers for each video node
> >   media: vsp1: Move video suspend resume handling to video object
> >   media: vsp1: Reword uses of 'fragment' as 'body'
> >   media: vsp1: Protect bodies against overflow
> >   media: vsp1: Provide a body pool
> >   media: vsp1: Convert display lists to use new body pool
> >   media: vsp1: Use reference counting for bodies
> >   media: vsp1: Refactor display list configure operations
> >   media: vsp1: Adapt entities to configure into a body
> >   media: vsp1: Move video configuration to a cached dlb
> >  
> >  drivers/media/platform/Kconfig|   2 +-
> >  drivers/media/platform/vsp1/vsp1_brx.c|  32 ++--
> >  drivers/media/platform/vsp1/vsp1_clu.c| 113 ++-
> >  drivers/media/platform/vsp1/vsp1_clu.h|   1 +
> >  drivers/media/platform/vsp1/vsp1_dl.c | 388 ++---
> >  drivers/media/platform/vsp1/vsp1_dl.h |  21 ++-
> >  drivers/media/platform/vsp1/vsp1_drm.c|  18 +-
> >  drivers/media/platform/vsp1/vsp1_drv.c|   4 +-
> >  drivers/media/platform/vsp1/vsp1_entity.c |  34 +++-
> >  drivers/media/platform/vsp1/vsp1_entity.h |  45 +++--
> >  drivers/media/platform/vsp1/vsp1_hgo.c|  26 ++-
> >  drivers/media/platform/vsp1/vsp1_hgt.c|  28 ++-
> >  drivers/media/platform/vsp1/vsp1_hsit.c   |  20 +-
> >  drivers/media/platform/vsp1/vsp1_lif.c|  25 ++-
> >  drivers/media/platform/vsp1/vsp1_lut.c|  80 +---
> >  drivers/media/platform/vsp1/vsp1_lut.h|   1 +
> >  drivers/media/platform/vsp1/vsp1_pipe.c   |  74 +---
> >  drivers/media/platform/vsp1/vsp1_pipe.h   |  12 +-
> >  drivers/media/platform/vsp1/vsp1_rpf.c| 189 ++-
> >  drivers/media/platform/vsp1/vsp1_sru.c|  24 +--
> >  drivers/media/platform/vsp1/vsp1_uds.c|  73 +++
> >  drivers/media/platform/vsp1/vsp1_uds.h|   2 +-
> >  drivers/media/platform/vsp1/vsp1_uif.c|  35 ++--
> >  drivers/media/platform/vsp1/vsp1_video.c  | 177 -
> >  drivers/media/platform/vsp1/vsp1_video.h  |   3 +
> >  drivers/media/platform/vsp1/vsp1_wpf.c| 326 ++---
> >  26 files changed, 967 insertions(+), 786 deletions(-)

-- 
Regards,

Laurent Pinchart





Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Steve Longerbeam

Hi Philipp,


On 05/24/2018 11:32 PM, Philipp Zabel wrote:

On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
[...]

The following is required as well. Now the question is why we can't skip
writing those odd UV rows. Anyway, with these 2 changes, I get a stable
NTSC (and probably PAL) interlaced video stream.

The manual says: Reduce Double Read or Writes (RDRW):
This bit is relevant for YUV4:2:0 formats. For write channels:
U and V components are not written to odd rows.

How could it be so? With YUV420, are they normally written?

Well, given that this bit exists, and assuming I understand it correctly
(1),
I guess the U and V components for odd rows normally are placed on the
AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
the U and V components are the same for odd and even rows.

In other words for writing 4:2:0 to memory, this bit should _always_ be set.

(1) OTOH I don't really understand what this bit is trying to say.
Whether this bit is set or not, the data in memory is correct
for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
(decimated by 2 in width and height), followed by Y plane of 1/4
size.

So I assume it is saying that the IPU normally places U/V components
on the AXI bus for odd rows, that are identical to the even row values.

Whether they are identical depends on the input format.


Right, this is the part I was missing, thanks for clarifying. The
even and odd chroma rows coming into the IDMAC from the
CSI (or IC) may not be identical if the CSI has captured 4:4:4
(or 4:2:2 yeah? 4:2:2 is only decimated in width not height).

But still, when the IDMAC has finished pixel packing/unpacking and
is writing 4:2:0 to memory, it should always skip overwriting the even
rows with the odd rows, whether or not it has received identical chroma
even/odd lines from the CSI.

Unless interweave is enabled :) See below.


The IDMAC always gets fed AYUV32 from the CSI or IC.
If the CSI captures YUV 4:2:x, odd and even lines will have the same
chroma values. But if the CSI captures YUV 4:4:4 (or RGB, fed through
the IC), we can have AYUV32 input with different chroma values on even
and odd lines.
In that case the IPU just writes the even chroma line and then
overwrites it with the odd line, unless the double write reduction bit
is set.


IOW somehow those identical odd rows are dropped before writing to
the U/V planes in memory.

potentially identical.


Right.




Philipp please chime in if you have something to add here.

I suppose the bit could be used to choose to write the chroma values of
odd instead of even lines for 4:4:4 inputs, at the cost of increased
memory bandwidth usage.


Steve


OTOH it seems that not only UV is broken with this bit set.
Y is broken as well.

Maybe scanline interlave and double write reduction can't be used at the
same time?


Yes, I just verified that. I went back to the SabreLite with the
progressive output OV5640, and double-write-reduction for
4:2:0 capture works fine, the images are correct.

Steve



Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation

2018-05-25 Thread Mauro Carvalho Chehab
Em Sun, 20 May 2018 15:10:50 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> The following changes since commit 8ed8bba70b4355b1ba029b151ade84475dd12991:
> 
>   media: imx274: remove non-indexed pointers from mode_table (2018-05-17 
> 06:22:08 -0400)
> 
> are available in the Git repository at:
> 
>   git://linuxtv.org/pinchartl/media.git v4l2/vsp1/next
> 
> for you to fetch changes up to 429f256501652c90a4ed82f2416618f82a77d37c:
> 
>   media: vsp1: Move video configuration to a cached dlb (2018-05-20 09:46:51 
> +0300)
> 
> The branch passes the VSP and DU test suites, both on its own and when merged 
> with the drm-next branch.

This series added a new warning:

drivers/media/platform/vsp1/vsp1_dl.c:69: warning: Function parameter or member 
'refcnt' not described in 'vsp1_dl_body'

To the already existing one:

drivers/media/platform/vsp1/vsp1_drm.c:336 vsp1_du_pipeline_setup_brx() error: 
we previously assumed 'pipe->brx' could be null (see line 244)

(there's also a Spectre warning too, but I'll looking into those
in separate).

For now, I'll apply it, but I reserve the right of not pulling any
new patchsets that would add more warnings.



> 
> 
> Geert Uytterhoeven (1):
>   media: vsp1: Drop OF dependency of VIDEO_RENESAS_VSP1
> 
> Kieran Bingham (10):
>   media: vsp1: Release buffers for each video node
>   media: vsp1: Move video suspend resume handling to video object
>   media: vsp1: Reword uses of 'fragment' as 'body'
>   media: vsp1: Protect bodies against overflow
>   media: vsp1: Provide a body pool
>   media: vsp1: Convert display lists to use new body pool
>   media: vsp1: Use reference counting for bodies
>   media: vsp1: Refactor display list configure operations
>   media: vsp1: Adapt entities to configure into a body
>   media: vsp1: Move video configuration to a cached dlb
> 
>  drivers/media/platform/Kconfig|   2 +-
>  drivers/media/platform/vsp1/vsp1_brx.c|  32 ++--
>  drivers/media/platform/vsp1/vsp1_clu.c| 113 ++-
>  drivers/media/platform/vsp1/vsp1_clu.h|   1 +
>  drivers/media/platform/vsp1/vsp1_dl.c | 388 -
>  drivers/media/platform/vsp1/vsp1_dl.h |  21 ++-
>  drivers/media/platform/vsp1/vsp1_drm.c|  18 +-
>  drivers/media/platform/vsp1/vsp1_drv.c|   4 +-
>  drivers/media/platform/vsp1/vsp1_entity.c |  34 +++-
>  drivers/media/platform/vsp1/vsp1_entity.h |  45 +++--
>  drivers/media/platform/vsp1/vsp1_hgo.c|  26 ++-
>  drivers/media/platform/vsp1/vsp1_hgt.c|  28 ++-
>  drivers/media/platform/vsp1/vsp1_hsit.c   |  20 +-
>  drivers/media/platform/vsp1/vsp1_lif.c|  25 ++-
>  drivers/media/platform/vsp1/vsp1_lut.c|  80 +---
>  drivers/media/platform/vsp1/vsp1_lut.h|   1 +
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  74 +---
>  drivers/media/platform/vsp1/vsp1_pipe.h   |  12 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c| 189 ++-
>  drivers/media/platform/vsp1/vsp1_sru.c|  24 +--
>  drivers/media/platform/vsp1/vsp1_uds.c|  73 +++
>  drivers/media/platform/vsp1/vsp1_uds.h|   2 +-
>  drivers/media/platform/vsp1/vsp1_uif.c|  35 ++--
>  drivers/media/platform/vsp1/vsp1_video.c  | 177 -
>  drivers/media/platform/vsp1/vsp1_video.h  |   3 +
>  drivers/media/platform/vsp1/vsp1_wpf.c| 326 ++-
>  26 files changed, 967 insertions(+), 786 deletions(-)
> 



Thanks,
Mauro


[PATCH] media: tc358743: release device_node in tc358743_probe_of()

2018-05-25 Thread Alexey Khoroshilov
of_graph_get_next_endpoint() returns device_node with refcnt increased,
but these is no of_node_put() for it.

The patch adds one on error and normal paths.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/media/i2c/tc358743.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 393baad7..44c41933415a 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1918,7 +1918,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
endpoint = v4l2_fwnode_endpoint_alloc_parse(of_fwnode_handle(ep));
if (IS_ERR(endpoint)) {
dev_err(dev, "failed to parse endpoint\n");
-   return PTR_ERR(endpoint);
+   ret = PTR_ERR(endpoint);
+   goto put_node;
}
 
if (endpoint->bus_type != V4L2_MBUS_CSI2 ||
@@ -2013,6 +2014,8 @@ static int tc358743_probe_of(struct tc358743_state *state)
clk_disable_unprepare(refclk);
 free_endpoint:
v4l2_fwnode_endpoint_free(endpoint);
+put_node:
+   of_node_put(ep);
return ret;
 }
 #else
-- 
2.7.4



Re: [PATCH v4 2/3] media: rc: introduce BPF_PROG_LIRC_MODE2

2018-05-25 Thread Alexei Starovoitov
On Fri, May 18, 2018 at 03:07:29PM +0100, Sean Young wrote:
> Add support for BPF_PROG_LIRC_MODE2. This type of BPF program can call
> rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> that the last key should be repeated.
> 
> The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> the target_fd must be the /dev/lircN device.
> 
> Signed-off-by: Sean Young 
...
>  enum bpf_attach_type {
> @@ -158,6 +159,7 @@ enum bpf_attach_type {
>   BPF_CGROUP_INET6_CONNECT,
>   BPF_CGROUP_INET4_POST_BIND,
>   BPF_CGROUP_INET6_POST_BIND,
> + BPF_LIRC_MODE2,
>   __MAX_BPF_ATTACH_TYPE
>  };
>  
> @@ -1902,6 +1904,53 @@ union bpf_attr {
>   *   egress otherwise). This is the only flag supported for now.
>   *   Return
>   *   **SK_PASS** on success, or **SK_DROP** on error.
> + *
> + * int bpf_rc_keydown(void *ctx, u32 protocol, u64 scancode, u32 toggle)
> + *   Description
> + *   This helper is used in programs implementing IR decoding, to
> + *   report a successfully decoded key press with *scancode*,
> + *   *toggle* value in the given *protocol*. The scancode will be
> + *   translated to a keycode using the rc keymap, and reported as
> + *   an input key down event. After a period a key up event is
> + *   generated. This period can be extended by calling either
> + *   **bpf_rc_keydown** () with the same values, or calling
> + *   **bpf_rc_repeat** ().
> + *
> + *   Some protocols include a toggle bit, in case the button
> + *   was released and pressed again between consecutive scancodes
> + *
> + *   The *ctx* should point to the lirc sample as passed into
> + *   the program.
> + *
> + *   The *protocol* is the decoded protocol number (see
> + *   **enum rc_proto** for some predefined values).
> + *
> + *   This helper is only available is the kernel was compiled with
> + *   the **CONFIG_BPF_LIRC_MODE2** configuration option set to
> + *   "**y**".
> + *
> + *   Return
> + *   0
> + *
> + * int bpf_rc_repeat(void *ctx)
> + *   Description
> + *   This helper is used in programs implementing IR decoding, to
> + *   report a successfully decoded repeat key message. This delays
> + *   the generation of a key up event for previously generated
> + *   key down event.
> + *
> + *   Some IR protocols like NEC have a special IR message for
> + *   repeating last button, for when a button is held down.
> + *
> + *   The *ctx* should point to the lirc sample as passed into
> + *   the program.
> + *
> + *   This helper is only available is the kernel was compiled with
> + *   the **CONFIG_BPF_LIRC_MODE2** configuration option set to
> + *   "**y**".

Hi Sean,

thank you for working on this. The patch set looks good to me.
I'd only ask to change above two helper names to something more specific.
Since BPF_PROG_TYPE_LIRC_MODE2 is the name of new prog type and kconfig.
May be bpf_lirc2_keydown() and bpf_lirc2_repeat() ?

> @@ -1576,6 +1577,8 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>   case BPF_SK_SKB_STREAM_PARSER:
>   case BPF_SK_SKB_STREAM_VERDICT:
>   return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, true);
> + case BPF_LIRC_MODE2:
> + return rc_dev_prog_attach(attr);
...
> + case BPF_LIRC_MODE2:
> + return rc_dev_prog_detach(attr);

and similar rename for internal function names that go into bpf core.

Please add accumulated acks when you respin.

Thanks



Re: [PATCH 3/3] media: rc: nuvoton: Keep device enabled during reg init

2018-05-25 Thread Sean Young
On Fri, May 25, 2018 at 04:42:02PM +0200, Michał Winiarski wrote:
> On Fri, May 25, 2018 at 02:59:41PM +0100, Sean Young wrote:
> > On Fri, May 25, 2018 at 03:35:23PM +0200, Michał Winiarski wrote:
> > > On Thu, May 24, 2018 at 12:31:40PM +0100, Sean Young wrote:
> > > > On Mon, May 21, 2018 at 04:38:03PM +0200, Michał Winiarski wrote:
> > > > > Doing writes when the device is disabled seems to be a NOOP.
> > > > > Let's enable the device, write the values, and then disable it on 
> > > > > init.
> > > > > This changes the behavior for wake device, which is now being disabled
> > > > > after init.
> > > > 
> > > > I don't have the datasheet so I might be misunderstanding this. We want
> > > > the IR wakeup to work fine even after kernel crash/power loss, right?
> > > 
> > > [snip]
> > > 
> > > Right, that makes sense. I completely ignored this scenario.
> > >  
> > > > > - /* enable the CIR WAKE logical device */
> > > > > - nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
> > > > > + nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
> > > > 
> > > > The way I read this is that the CIR, not CIR_WAKE, is being disabled,
> > > > which seems contrary to what the commit message says.
> > > > 
> > > 
> > > That's a typo. And by accident it makes the wake_device work correctly :)
> > > I think that registers init logic was still broken though, operating 
> > > under the
> > > assumption that the device is enabled on module load...
> > > 
> > > I guess we should just remove disable(LOGICAL_DEV_CIR) from 
> > > wake_regs_init.
> > > 
> > > Have you already included this in any non-rebasing tree?
> > 
> > Nothing has been applied yet.
> > 
> > > Should I send a v2 or fixup on top?
> > 
> > I don't have the hardware to test this, a v2 would be appreciated.
> > 
> > We're late in the release cycle and I'm wondering if this patch would also
> > solve the nuvoton probe problem:
> > 
> > https://patchwork.linuxtv.org/patch/49874/
> 
> It causes us to go back to previous behavior (we're refcounting open/close,
> with your patch initial open on my system is coming from kbd_connect(), so
> userspace close() doesn't propagate to nuvoton-cir).
> 
> It passes my test of "load the module with debug=1, see if I'm getting
> interrupts".
> 
> If there's any scenario in which->close() would be called, it's still going to
> be broken.

Great, thank you very much for testing that. I've created a pull request
for the v2 version.


Sean


Re: [PATCH v10 13/16] vb2: add out-fence support to QBUF

2018-05-25 Thread Brian Starkey

Hi Ezequiel,

Not sure if this patch series is still relevant, but I spotted a
couple more things below.

On Mon, May 21, 2018 at 01:59:43PM -0300, Ezequiel Garcia wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace in the fence_fd field as a
return arg for the QBUF call.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

v12: - Pass the fence_fd in vb2_qbuf for clarity.
- Increase fence seqno if fence context if shared
- Check get_unused_fd return

v11: - Return fence_fd to userpace only in the QBUF ioctl.
- Rework implementation to avoid storing the sync_file
  as state, which is not really needed.

v10: - use -EIO for fence error (Hans Verkuil)
- add comment around fence context creation (Hans Verkuil)

v9: - remove in-fences changes from this patch (Alex Courbot)
   - improve fence context creation (Hans Verkuil)
   - clean up out fences if vb2_core_qbuf() fails (Hans Verkuil)

v8: - return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
   - fix crash when checking not using fences in vb2_buffer_done()

v7: - merge patch that add the infrastructure to out-fences into
 this one (Alex Courbot)
   - Do not install the fd if there is no fence. (Alex Courbot)
   - do not report error on requeueing, just WARN_ON_ONCE() (Hans)

v6: - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
 ordering in vb2 for queueing in the driver, so the event is not
 necessary anymore and the out_fence_fd is sent back to userspace
 on QBUF call return arg
   - do not allow requeueing with out-fences, instead mark the buffer
 with an error and wake up to userspace.
   - send the out_fence_fd back to userspace on the fence_fd field

v5: - delay fd_install to DQ_EVENT (Hans)
   - if queue is fully ordered send OUT_FENCE event right away
 (Brian)
   - rename 'q->ordered' to 'q->ordered_in_driver'
   - merge change to implement OUT_FENCE event here

v4: - return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
   - set the OUT_FENCE flag if there is a fence pending (Hans)
   - call fd_install() after vb2_core_qbuf() (Hans)
   - clean up fence if vb2_core_qbuf() fails (Hans)
   - add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
Signed-off-by: Ezequiel Garcia 
---
drivers/media/common/videobuf2/videobuf2-core.c | 113 +++-
drivers/media/common/videobuf2/videobuf2-v4l2.c |  10 ++-
drivers/media/dvb-core/dvb_vb2.c|   2 +-
include/media/videobuf2-core.h  |  20 -
4 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 86b5ebe25263..edc2fdaf56de 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -25,6 +25,7 @@
#include 
#include 
#include 
+#include 

#include 
#include 
@@ -380,6 +381,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;

/* Allocate video buffer memory for the MMAP type */
@@ -976,10 +978,22 @@ static void vb2_process_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /* Requeuing with explicit synchronization, spit warning */
+   WARN_ON_ONCE(vb->out_fence);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (vb->out_fence) {
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EIO);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+   }
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1406,6 +1420,76 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
}
EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);

+static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence)
+{
+   return 

[GIT PULL FOR v4.18] RC fixes

2018-05-25 Thread Sean Young
Hi Mauro,

Fixes for a regression in v4.16, and broken open/close handling in the
nuvoton driver.

Thanks

Sean

The following changes since commit 8ed8bba70b4355b1ba029b151ade84475dd12991:

  media: imx274: remove non-indexed pointers from mode_table (2018-05-17 
06:22:08 -0400)

are available in the Git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.18d

for you to fetch changes up to 3b65073f8d650f2749bdac48153a126c568352b9:

  media: rc: ensure input/lirc device can be opened after register (2018-05-25 
16:32:15 +0100)


Michał Winiarski (3):
  media: rc: nuvoton: Tweak the interrupt enabling dance
  media: rc: nuvoton: Keep track of users on CIR enable/disable
  media: rc: nuvoton: Keep device enabled during reg init

Sean Young (1):
  media: rc: ensure input/lirc device can be opened after register

 drivers/media/rc/nuvoton-cir.c | 89 +++---
 drivers/media/rc/rc-main.c |  4 +-
 2 files changed, 43 insertions(+), 50 deletions(-)


[PATCH 0/8] xen: dma-buf support for grant device

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

This work is in response to my previous attempt to introduce Xen/DRM
zero-copy driver [1] to enable Linux dma-buf API [2] for Xen based
frontends/backends. There is also an existing hyper_dmabuf approach
available [3] which, if reworked to utilize the proposed solution,
can greatly benefit as well.

RFC for this series was published and discussed [9], comments addressed.

The original rationale behind this work was to enable zero-copying
use-cases while working with Xen para-virtual display driver [4]:
when using Xen PV DRM frontend driver then on backend side one will
need to do copying of display buffers' contents (filled by the
frontend's user-space) into buffers allocated at the backend side.
Taking into account the size of display buffers and frames per
second it may result in unneeded huge data bus occupation and
performance loss.

The helper driver [4] allows implementing zero-copying use-cases
when using Xen para-virtualized frontend display driver by implementing
a DRM/KMS helper driver running on backend's side.
It utilizes PRIME buffers API (implemented on top of Linux dma-buf)
to share frontend's buffers with physical device drivers on
backend's side:

 - a dumb buffer created on backend's side can be shared
   with the Xen PV frontend driver, so it directly writes
   into backend's domain memory (into the buffer exported from
   DRM/KMS driver of a physical display device)
 - a dumb buffer allocated by the frontend can be imported
   into physical device DRM/KMS driver, thus allowing to
   achieve no copying as well

Finally, it was discussed and decided ([1], [5]) that it is worth
implementing such use-cases via extension of the existing Xen gntdev
driver instead of introducing new DRM specific driver.
Please note, that the support of dma-buf is Linux only,
as dma-buf is a Linux only thing.

Now to the proposed solution. The changes  to the existing Xen drivers
in the Linux kernel fall into 2 categories:
1. DMA-able memory buffer allocation and increasing/decreasing memory
   reservation of the pages of such a buffer.
   This is required if we are about to share dma-buf with the hardware
   that does require those to be allocated with dma_alloc_xxx API.
   (It is still possible to allocate a dma-buf from any system memory,
   e.g. system pages).
2. Extension of the gntdev driver to enable it to import/export dma-buf’s.

The first four patches are in preparation for Xen dma-buf support,
but I consider those usable regardless of the dma-buf use-case,
e.g. other frontend/backend kernel modules may also benefit from these
for better code reuse:
   0001-xen-grant-table-Make-set-clear-page-private-code-sha.patch
   0002-xen-balloon-Move-common-memory-reservation-routines-.patch
   0003-xen-grant-table-Allow-allocating-buffers-suitable-fo.patch
   0004-xen-gntdev-Allow-mappings-for-DMA-buffers.patch

The next three patches are Xen implementation of dma-buf as part of
the grant device:
   0005-xen-gntdev-Add-initial-support-for-dma-buf-UAPI.patch
   0006-xen-gntdev-Implement-dma-buf-export-functionality.patch
   0007-xen-gntdev-Implement-dma-buf-import-functionality.patch

The last patch makes it possible for in-kernel use of Xen dma-buf API:
  0008-xen-gntdev-Expose-gntdev-s-dma-buf-API-for-in-kernel.patch

The corresponding libxengnttab changes are available at [6].

All the above was tested with display backend [7] and its accompanying
helper library [8] on Renesas ARM64 based board.

*To all the communities*: I would like to ask you to review the proposed
solution and give feedback on it, so I can improve and send final
patches for review (this is still work in progress, but enough to start
discussing the implementation).


Thank you in advance,
Oleksandr Andrushchenko

[1] https://lists.freedesktop.org/archives/dri-devel/2018-April/173163.html
[2] 
https://elixir.bootlin.com/linux/v4.17-rc5/source/Documentation/driver-api/dma-buf.rst
[3] https://lists.xenproject.org/archives/html/xen-devel/2018-02/msg01202.html
[4] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/xen
[5] https://patchwork.kernel.org/patch/10279681/
[6] https://github.com/andr2000/xen/tree/xen_dma_buf_v1
[7] https://github.com/andr2000/displ_be/tree/xen_dma_buf_v1
[8] https://github.com/andr2000/libxenbe/tree/xen_dma_buf_v1
[9] https://lkml.org/lkml/2018/5/17/215

Oleksandr Andrushchenko (8):
  xen/grant-table: Make set/clear page private code shared
  xen/balloon: Move common memory reservation routines to a module
  xen/grant-table: Allow allocating buffers suitable for DMA
  xen/gntdev: Allow mappings for DMA buffers
  xen/gntdev: Add initial support for dma-buf UAPI
  xen/gntdev: Implement dma-buf export functionality
  xen/gntdev: Implement dma-buf import functionality
  xen/gntdev: Expose gntdev's dma-buf API for in-kernel use

 drivers/xen/Kconfig   |   23 +
 drivers/xen/Makefile  |1 +
 drivers/xen/balloon.c |   71 

[PATCH 3/8] xen/grant-table: Allow allocating buffers suitable for DMA

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Extend grant table module API to allow allocating buffers that can
be used for DMA operations and mapping foreign grant references
on top of those.
The resulting buffer is similar to the one allocated by the balloon
driver in terms that proper memory reservation is made
({increase|decrease}_reservation and VA mappings updated if needed).
This is useful for sharing foreign buffers with HW drivers which
cannot work with scattered buffers provided by the balloon driver,
but require DMAable memory instead.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/Kconfig   |  13 
 drivers/xen/grant-table.c | 124 ++
 include/xen/grant_table.h |  25 
 3 files changed, 162 insertions(+)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index e5d0c28372ea..3431fe210624 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -161,6 +161,19 @@ config XEN_GRANT_DEV_ALLOC
  to other domains. This can be used to implement frontend drivers
  or as part of an inter-domain shared memory channel.
 
+config XEN_GRANT_DMA_ALLOC
+   bool "Allow allocating DMA capable buffers with grant reference module"
+   depends on XEN
+   help
+ Extends grant table module API to allow allocating DMA capable
+ buffers and mapping foreign grant references on top of it.
+ The resulting buffer is similar to one allocated by the balloon
+ driver in terms that proper memory reservation is made
+ ({increase|decrease}_reservation and VA mappings updated if needed).
+ This is useful for sharing foreign buffers with HW drivers which
+ cannot work with scattered buffers provided by the balloon driver,
+ but require DMAable memory instead.
+
 config SWIOTLB_XEN
def_bool y
select SWIOTLB
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index d7488226e1f2..06fe6e7f639c 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -45,6 +45,9 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+#include 
+#endif
 
 #include 
 #include 
@@ -57,6 +60,7 @@
 #ifdef CONFIG_X86
 #include 
 #endif
+#include 
 #include 
 #include 
 
@@ -811,6 +815,82 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 }
 EXPORT_SYMBOL(gnttab_alloc_pages);
 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+/**
+ * gnttab_dma_alloc_pages - alloc DMAable pages suitable for grant mapping into
+ * @args: arguments to the function
+ */
+int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
+{
+   unsigned long pfn, start_pfn;
+   xen_pfn_t *frames;
+   size_t size;
+   int i, ret;
+
+   frames = kcalloc(args->nr_pages, sizeof(*frames), GFP_KERNEL);
+   if (!frames)
+   return -ENOMEM;
+
+   size = args->nr_pages << PAGE_SHIFT;
+   if (args->coherent)
+   args->vaddr = dma_alloc_coherent(args->dev, size,
+>dev_bus_addr,
+GFP_KERNEL | __GFP_NOWARN);
+   else
+   args->vaddr = dma_alloc_wc(args->dev, size,
+  >dev_bus_addr,
+  GFP_KERNEL | __GFP_NOWARN);
+   if (!args->vaddr) {
+   pr_err("Failed to allocate DMA buffer of size %zu\n", size);
+   ret = -ENOMEM;
+   goto fail_free_frames;
+   }
+
+   start_pfn = __phys_to_pfn(args->dev_bus_addr);
+   for (pfn = start_pfn, i = 0; pfn < start_pfn + args->nr_pages;
+   pfn++, i++) {
+   struct page *page = pfn_to_page(pfn);
+
+   args->pages[i] = page;
+   frames[i] = xen_page_to_gfn(page);
+   xenmem_reservation_scrub_page(page);
+   }
+
+   xenmem_reservation_va_mapping_reset(args->nr_pages, args->pages);
+
+   ret = xenmem_reservation_decrease(args->nr_pages, frames);
+   if (ret != args->nr_pages) {
+   pr_err("Failed to decrease reservation for DMA buffer\n");
+   xenmem_reservation_increase(ret, frames);
+   ret = -EFAULT;
+   goto fail_free_dma;
+   }
+
+   ret = gnttab_pages_set_private(args->nr_pages, args->pages);
+   if (ret < 0)
+   goto fail_clear_private;
+
+   kfree(frames);
+   return 0;
+
+fail_clear_private:
+   gnttab_pages_clear_private(args->nr_pages, args->pages);
+fail_free_dma:
+   xenmem_reservation_va_mapping_update(args->nr_pages, args->pages,
+frames);
+   if (args->coherent)
+   dma_free_coherent(args->dev, size,
+ args->vaddr, args->dev_bus_addr);
+   else
+   dma_free_wc(args->dev, size,
+   

[PATCH 1/8] xen/grant-table: Make set/clear page private code shared

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Make set/clear page private code shared and accessible to
other kernel modules which can re-use these instead of open-coding.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/grant-table.c | 54 +--
 include/xen/grant_table.h |  3 +++
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 27be107d6480..d7488226e1f2 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void)
 }
 EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);
 
-/**
- * gnttab_alloc_pages - alloc pages suitable for grant mapping into
- * @nr_pages: number of pages to alloc
- * @pages: returns the pages
- */
-int gnttab_alloc_pages(int nr_pages, struct page **pages)
+int gnttab_pages_set_private(int nr_pages, struct page **pages)
 {
int i;
-   int ret;
-
-   ret = alloc_xenballooned_pages(nr_pages, pages);
-   if (ret < 0)
-   return ret;
 
for (i = 0; i < nr_pages; i++) {
 #if BITS_PER_LONG < 64
struct xen_page_foreign *foreign;
 
foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);
-   if (!foreign) {
-   gnttab_free_pages(nr_pages, pages);
+   if (!foreign)
return -ENOMEM;
-   }
+
set_page_private(pages[i], (unsigned long)foreign);
 #endif
SetPagePrivate(pages[i]);
@@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
 
return 0;
 }
-EXPORT_SYMBOL(gnttab_alloc_pages);
+EXPORT_SYMBOL(gnttab_pages_set_private);
 
 /**
- * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
- * @nr_pages; number of pages to free
- * @pages: the pages
+ * gnttab_alloc_pages - alloc pages suitable for grant mapping into
+ * @nr_pages: number of pages to alloc
+ * @pages: returns the pages
  */
-void gnttab_free_pages(int nr_pages, struct page **pages)
+int gnttab_alloc_pages(int nr_pages, struct page **pages)
+{
+   int ret;
+
+   ret = alloc_xenballooned_pages(nr_pages, pages);
+   if (ret < 0)
+   return ret;
+
+   ret = gnttab_pages_set_private(nr_pages, pages);
+   if (ret < 0)
+   gnttab_free_pages(nr_pages, pages);
+
+   return ret;
+}
+EXPORT_SYMBOL(gnttab_alloc_pages);
+
+void gnttab_pages_clear_private(int nr_pages, struct page **pages)
 {
int i;
 
@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages)
ClearPagePrivate(pages[i]);
}
}
+}
+EXPORT_SYMBOL(gnttab_pages_clear_private);
+
+/**
+ * gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
+ * @nr_pages; number of pages to free
+ * @pages: the pages
+ */
+void gnttab_free_pages(int nr_pages, struct page **pages)
+{
+   gnttab_pages_clear_private(nr_pages, pages);
free_xenballooned_pages(nr_pages, pages);
 }
 EXPORT_SYMBOL(gnttab_free_pages);
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 2e37741f6b8d..de03f2542bb7 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void);
 int gnttab_alloc_pages(int nr_pages, struct page **pages);
 void gnttab_free_pages(int nr_pages, struct page **pages);
 
+int gnttab_pages_set_private(int nr_pages, struct page **pages);
+void gnttab_pages_clear_private(int nr_pages, struct page **pages);
+
 int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
struct gnttab_map_grant_ref *kmap_ops,
struct page **pages, unsigned int count);
-- 
2.17.0



[PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Memory {increase|decrease}_reservation and VA mappings update/reset
code used in balloon driver can be made common, so other drivers can
also re-use the same functionality without open-coding.
Create a dedicated module for the shared code and export corresponding
symbols for other kernel modules.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/Makefile  |   1 +
 drivers/xen/balloon.c |  71 ++
 drivers/xen/mem-reservation.c | 134 ++
 include/xen/mem_reservation.h |  29 
 4 files changed, 170 insertions(+), 65 deletions(-)
 create mode 100644 drivers/xen/mem-reservation.c
 create mode 100644 include/xen/mem_reservation.h

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 451e833f5931..3c87b0c3aca6 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_HOTPLUG_CPU)  += cpu_hotplug.o
 obj-$(CONFIG_X86)  += fallback.o
 obj-y  += grant-table.o features.o balloon.o manage.o preempt.o time.o
+obj-y  += mem-reservation.o
 obj-y  += events/
 obj-y  += xenbus/
 
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 065f0b607373..57b482d67a3a 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static int xen_hotplug_unpopulated;
 
@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, 
balloon_process);
 #define GFP_BALLOON \
(GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
 
-static void scrub_page(struct page *page)
-{
-#ifdef CONFIG_XEN_SCRUB_PAGES
-   clear_highpage(page);
-#endif
-}
-
 /* balloon_append: add the given page to the balloon. */
 static void __balloon_append(struct page *page)
 {
@@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long 
nr_pages)
int rc;
unsigned long i;
struct page   *page;
-   struct xen_memory_reservation reservation = {
-   .address_bits = 0,
-   .extent_order = EXTENT_ORDER,
-   .domid= DOMID_SELF
-   };
 
if (nr_pages > ARRAY_SIZE(frame_list))
nr_pages = ARRAY_SIZE(frame_list);
@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long 
nr_pages)
page = balloon_next_page(page);
}
 
-   set_xen_guest_handle(reservation.extent_start, frame_list);
-   reservation.nr_extents = nr_pages;
-   rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, );
+   rc = xenmem_reservation_increase(nr_pages, frame_list);
if (rc <= 0)
return BP_EAGAIN;
 
@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long 
nr_pages)
page = balloon_retrieve(false);
BUG_ON(page == NULL);
 
-#ifdef CONFIG_XEN_HAVE_PVMMU
-   /*
-* We don't support PV MMU when Linux and Xen is using
-* different page granularity.
-*/
-   BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
-
-   if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-   unsigned long pfn = page_to_pfn(page);
-
-   set_phys_to_machine(pfn, frame_list[i]);
-
-   /* Link back into the page tables if not highmem. */
-   if (!PageHighMem(page)) {
-   int ret;
-   ret = HYPERVISOR_update_va_mapping(
-   (unsigned long)__va(pfn << 
PAGE_SHIFT),
-   mfn_pte(frame_list[i], 
PAGE_KERNEL),
-   0);
-   BUG_ON(ret);
-   }
-   }
-#endif
+   xenmem_reservation_va_mapping_update(1, , _list[i]);
 
/* Relinquish the page back to the allocator. */
free_reserved_page(page);
@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long 
nr_pages, gfp_t gfp)
unsigned long i;
struct page *page, *tmp;
int ret;
-   struct xen_memory_reservation reservation = {
-   .address_bits = 0,
-   .extent_order = EXTENT_ORDER,
-   .domid= DOMID_SELF
-   };
LIST_HEAD(pages);
 
if (nr_pages > ARRAY_SIZE(frame_list))
@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long 
nr_pages, gfp_t gfp)
break;
}
adjust_managed_page_count(page, -1);
-   scrub_page(page);
+   xenmem_reservation_scrub_page(page);
list_add(>lru, );
}
 
@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long 

[PATCH 4/8] xen/gntdev: Allow mappings for DMA buffers

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Allow mappings for DMA backed  buffers if grant table module
supports such: this extends grant device to not only map buffers
made of balloon pages, but also from buffers allocated with
dma_alloc_xxx.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c  | 100 +-
 include/uapi/xen/gntdev.h |  15 ++
 2 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index bd56653b9bbc..640a579f42ea 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -37,6 +37,9 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+#include 
+#endif
 
 #include 
 #include 
@@ -72,6 +75,11 @@ struct gntdev_priv {
struct mutex lock;
struct mm_struct *mm;
struct mmu_notifier mn;
+
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   /* Device for which DMA memory is allocated. */
+   struct device *dma_dev;
+#endif
 };
 
 struct unmap_notify {
@@ -96,10 +104,28 @@ struct grant_map {
struct gnttab_unmap_grant_ref *kunmap_ops;
struct page **pages;
unsigned long pages_vm_start;
+
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   /*
+* If dmabuf_vaddr is not NULL then this mapping is backed by DMA
+* capable memory.
+*/
+
+   /* Device for which DMA memory is allocated. */
+   struct device *dma_dev;
+   /* Flags used to create this DMA buffer: GNTDEV_DMABUF_FLAG_XXX. */
+   bool dma_flags;
+   /* Virtual/CPU address of the DMA buffer. */
+   void *dma_vaddr;
+   /* Bus address of the DMA buffer. */
+   dma_addr_t dma_bus_addr;
+#endif
 };
 
 static int unmap_grant_pages(struct grant_map *map, int offset, int pages);
 
+static struct miscdevice gntdev_miscdev;
+
 /* -- */
 
 static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -121,8 +147,26 @@ static void gntdev_free_map(struct grant_map *map)
if (map == NULL)
return;
 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   if (map->dma_vaddr) {
+   struct gnttab_dma_alloc_args args;
+
+   args.dev = map->dma_dev;
+   args.coherent = map->dma_flags & GNTDEV_DMA_FLAG_COHERENT;
+   args.nr_pages = map->count;
+   args.pages = map->pages;
+   args.vaddr = map->dma_vaddr;
+   args.dev_bus_addr = map->dma_bus_addr;
+
+   gnttab_dma_free_pages();
+   } else if (map->pages) {
+   gnttab_free_pages(map->count, map->pages);
+   }
+#else
if (map->pages)
gnttab_free_pages(map->count, map->pages);
+#endif
+
kfree(map->pages);
kfree(map->grants);
kfree(map->map_ops);
@@ -132,7 +176,8 @@ static void gntdev_free_map(struct grant_map *map)
kfree(map);
 }
 
-static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count)
+static struct grant_map *gntdev_alloc_map(struct gntdev_priv *priv, int count,
+ int dma_flags)
 {
struct grant_map *add;
int i;
@@ -155,8 +200,37 @@ static struct grant_map *gntdev_alloc_map(struct 
gntdev_priv *priv, int count)
NULL == add->pages)
goto err;
 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   add->dma_flags = dma_flags;
+
+   /*
+* Check if this mapping is requested to be backed
+* by a DMA buffer.
+*/
+   if (dma_flags & (GNTDEV_DMA_FLAG_WC | GNTDEV_DMA_FLAG_COHERENT)) {
+   struct gnttab_dma_alloc_args args;
+
+   /* Remember the device, so we can free DMA memory. */
+   add->dma_dev = priv->dma_dev;
+
+   args.dev = priv->dma_dev;
+   args.coherent = dma_flags & GNTDEV_DMA_FLAG_COHERENT;
+   args.nr_pages = count;
+   args.pages = add->pages;
+
+   if (gnttab_dma_alloc_pages())
+   goto err;
+
+   add->dma_vaddr = args.vaddr;
+   add->dma_bus_addr = args.dev_bus_addr;
+   } else {
+   if (gnttab_alloc_pages(count, add->pages))
+   goto err;
+   }
+#else
if (gnttab_alloc_pages(count, add->pages))
goto err;
+#endif
 
for (i = 0; i < count; i++) {
add->map_ops[i].handle = -1;
@@ -323,8 +397,19 @@ static int map_grant_pages(struct grant_map *map)
}
 
map->unmap_ops[i].handle = map->map_ops[i].handle;
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   if (use_ptemod) {
+   map->kunmap_ops[i].handle = map->kmap_ops[i].handle;
+   } else if (map->dma_vaddr) {
+   unsigned long mfn;
+
+   mfn = __pfn_to_mfn(page_to_pfn(map->pages[i]));
+   

[PATCH 6/8] xen/gntdev: Implement dma-buf export functionality

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

1. Create a dma-buf from grant references provided by the foreign
   domain. By default dma-buf is backed by system memory pages, but
   by providing GNTDEV_DMA_FLAG_XXX flags it can also be created
   as a DMA write-combine/coherent buffer, e.g. allocated with
   corresponding dma_alloc_xxx API.
   Export the resulting buffer as a new dma-buf.

2. Implement waiting for the dma-buf to be released: block until the
   dma-buf with the file descriptor provided is released.
   If within the time-out provided the buffer is not released then
   -ETIMEDOUT error is returned. If the buffer with the file descriptor
   does not exist or has already been released, then -ENOENT is returned.
   For valid file descriptors this must not be treated as error.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c | 478 ++-
 1 file changed, 476 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 9e450622af1a..52abc6cd5846 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -4,6 +4,8 @@
  * Device for accessing (in user-space) pages that have been granted by other
  * domains.
  *
+ * DMA buffer implementation is based on drivers/gpu/drm/drm_prime.c.
+ *
  * Copyright (c) 2006-2007, D G Murray.
  *   (c) 2009 Gerd Hoffmann 
  *   (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc.
@@ -41,6 +43,9 @@
 #ifdef CONFIG_XEN_GRANT_DMA_ALLOC
 #include 
 #endif
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+#include 
+#endif
 
 #include 
 #include 
@@ -81,6 +86,17 @@ struct gntdev_priv {
/* Device for which DMA memory is allocated. */
struct device *dma_dev;
 #endif
+
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+   /* Private data of the hyper DMA buffers. */
+
+   /* List of exported DMA buffers. */
+   struct list_head dmabuf_exp_list;
+   /* List of wait objects. */
+   struct list_head dmabuf_exp_wait_list;
+   /* This is the lock which protects dma_buf_xxx lists. */
+   struct mutex dmabuf_lock;
+#endif
 };
 
 struct unmap_notify {
@@ -125,12 +141,38 @@ struct grant_map {
 
 #ifdef CONFIG_XEN_GNTDEV_DMABUF
 struct xen_dmabuf {
+   struct gntdev_priv *priv;
+   struct dma_buf *dmabuf;
+   struct list_head next;
+   int fd;
+
union {
+   struct {
+   /* Exported buffers are reference counted. */
+   struct kref refcount;
+   struct grant_map *map;
+   } exp;
struct {
/* Granted references of the imported buffer. */
grant_ref_t *refs;
} imp;
} u;
+
+   /* Number of pages this buffer has. */
+   int nr_pages;
+   /* Pages of this buffer. */
+   struct page **pages;
+};
+
+struct xen_dmabuf_wait_obj {
+   struct list_head next;
+   struct xen_dmabuf *xen_dmabuf;
+   struct completion completion;
+};
+
+struct xen_dmabuf_attachment {
+   struct sg_table *sgt;
+   enum dma_data_direction dir;
 };
 #endif
 
@@ -320,6 +362,16 @@ static void gntdev_put_map(struct gntdev_priv *priv, 
struct grant_map *map)
gntdev_free_map(map);
 }
 
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+static void gntdev_remove_map(struct gntdev_priv *priv, struct grant_map *map)
+{
+   mutex_lock(>lock);
+   list_del(>next);
+   gntdev_put_map(NULL /* already removed */, map);
+   mutex_unlock(>lock);
+}
+#endif
+
 /* -- */
 
 static int find_grant_ptes(pte_t *pte, pgtable_t token,
@@ -628,6 +680,12 @@ static int gntdev_open(struct inode *inode, struct file 
*flip)
INIT_LIST_HEAD(>freeable_maps);
mutex_init(>lock);
 
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+   mutex_init(>dmabuf_lock);
+   INIT_LIST_HEAD(>dmabuf_exp_list);
+   INIT_LIST_HEAD(>dmabuf_exp_wait_list);
+#endif
+
if (use_ptemod) {
priv->mm = get_task_mm(current);
if (!priv->mm) {
@@ -1053,17 +,433 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv 
*priv, void __user *u)
 /* DMA buffer export support. */
 /* -- */
 
+/* -- */
+/* Implementation of wait for exported DMA buffer to be released. */
+/* -- */
+
+static void dmabuf_exp_release(struct kref *kref);
+
+static struct xen_dmabuf_wait_obj *
+dmabuf_exp_wait_obj_new(struct gntdev_priv *priv,
+   struct xen_dmabuf *xen_dmabuf)
+{
+   struct xen_dmabuf_wait_obj *obj;
+
+   obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+   if (!obj)
+   return 

[PATCH 5/8] xen/gntdev: Add initial support for dma-buf UAPI

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Add UAPI and IOCTLs for dma-buf grant device driver extension:
the extension allows userspace processes and kernel modules to
use Xen backed dma-buf implementation. With this extension grant
references to the pages of an imported dma-buf can be exported
for other domain use and grant references coming from a foreign
domain can be converted into a local dma-buf for local export.
Implement basic initialization and stubs for Xen DMA buffers'
support.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/Kconfig   |  10 +++
 drivers/xen/gntdev.c  | 148 ++
 include/uapi/xen/gntdev.h |  91 +++
 3 files changed, 249 insertions(+)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 3431fe210624..eaf63a2c7ae6 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -152,6 +152,16 @@ config XEN_GNTDEV
help
  Allows userspace processes to use grants.
 
+config XEN_GNTDEV_DMABUF
+   bool "Add support for dma-buf grant access device driver extension"
+   depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC
+   help
+ Allows userspace processes and kernel modules to use Xen backed
+ dma-buf implementation. With this extension grant references to
+ the pages of an imported dma-buf can be exported for other domain
+ use and grant references coming from a foreign domain can be
+ converted into a local dma-buf for local export.
+
 config XEN_GRANT_DEV_ALLOC
tristate "User-space grant reference allocator driver"
depends on XEN
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 640a579f42ea..9e450622af1a 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -6,6 +6,7 @@
  *
  * Copyright (c) 2006-2007, D G Murray.
  *   (c) 2009 Gerd Hoffmann 
+ *   (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -122,6 +123,17 @@ struct grant_map {
 #endif
 };
 
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+struct xen_dmabuf {
+   union {
+   struct {
+   /* Granted references of the imported buffer. */
+   grant_ref_t *refs;
+   } imp;
+   } u;
+};
+#endif
+
 static int unmap_grant_pages(struct grant_map *map, int offset, int pages);
 
 static struct miscdevice gntdev_miscdev;
@@ -1036,6 +1048,128 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv 
*priv, void __user *u)
return ret;
 }
 
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+/* -- */
+/* DMA buffer export support. */
+/* -- */
+
+static int dmabuf_exp_wait_released(struct gntdev_priv *priv, int fd,
+   int wait_to_ms)
+{
+   return -ETIMEDOUT;
+}
+
+static int dmabuf_exp_from_refs(struct gntdev_priv *priv, int flags,
+   int count, u32 domid, u32 *refs, u32 *fd)
+{
+   *fd = -1;
+   return -EINVAL;
+}
+
+/* -- */
+/* DMA buffer import support. */
+/* -- */
+
+static int dmabuf_imp_release(struct gntdev_priv *priv, u32 fd)
+{
+   return 0;
+}
+
+static struct xen_dmabuf *
+dmabuf_imp_to_refs(struct gntdev_priv *priv, int fd, int count, int domid)
+{
+   return ERR_PTR(-ENOMEM);
+}
+
+/* -- */
+/* DMA buffer IOCTL support.  */
+/* -- */
+
+static long
+gntdev_ioctl_dmabuf_exp_from_refs(struct gntdev_priv *priv,
+ struct ioctl_gntdev_dmabuf_exp_from_refs 
__user *u)
+{
+   struct ioctl_gntdev_dmabuf_exp_from_refs op;
+   u32 *refs;
+   long ret;
+
+   if (copy_from_user(, u, sizeof(op)) != 0)
+   return -EFAULT;
+
+   refs = kcalloc(op.count, sizeof(*refs), GFP_KERNEL);
+   if (!refs)
+   return -ENOMEM;
+
+   if (copy_from_user(refs, u->refs, sizeof(*refs) * op.count) != 0) {
+   ret = -EFAULT;
+   goto out;
+   }
+
+   ret = dmabuf_exp_from_refs(priv, op.flags, op.count,
+  op.domid, refs, );
+   if (ret)
+   goto out;
+
+   if (copy_to_user(u, , sizeof(op)) != 0)
+   ret = -EFAULT;
+
+out:
+   kfree(refs);
+   return ret;
+}
+
+static long
+gntdev_ioctl_dmabuf_exp_wait_released(struct gntdev_priv *priv,
+  

[PATCH 7/8] xen/gntdev: Implement dma-buf import functionality

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

1. Import a dma-buf with the file descriptor provided and export
   granted references to the pages of that dma-buf into the array
   of grant references.

2. Add API to close all references to an imported buffer, so it can be
   released by the owner. This is only valid for buffers created with
   IOCTL_GNTDEV_DMABUF_IMP_TO_REFS.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c | 237 ++-
 1 file changed, 234 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 52abc6cd5846..d8b6168f2cd9 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -71,6 +71,17 @@ static atomic_t pages_mapped = ATOMIC_INIT(0);
 static int use_ptemod;
 #define populate_freeable_maps use_ptemod
 
+#ifdef CONFIG_XEN_GNTDEV_DMABUF
+#ifndef GRANT_INVALID_REF
+/*
+ * Note on usage of grant reference 0 as invalid grant reference:
+ * grant reference 0 is valid, but never exposed to a driver,
+ * because of the fact it is already in use/reserved by the PV console.
+ */
+#define GRANT_INVALID_REF  0
+#endif
+#endif
+
 struct gntdev_priv {
/* maps with visible offsets in the file descriptor */
struct list_head maps;
@@ -94,6 +105,8 @@ struct gntdev_priv {
struct list_head dmabuf_exp_list;
/* List of wait objects. */
struct list_head dmabuf_exp_wait_list;
+   /* List of imported DMA buffers. */
+   struct list_head dmabuf_imp_list;
/* This is the lock which protects dma_buf_xxx lists. */
struct mutex dmabuf_lock;
 #endif
@@ -155,6 +168,10 @@ struct xen_dmabuf {
struct {
/* Granted references of the imported buffer. */
grant_ref_t *refs;
+   /* Scatter-gather table of the imported buffer. */
+   struct sg_table *sgt;
+   /* dma-buf attachment of the imported buffer. */
+   struct dma_buf_attachment *attach;
} imp;
} u;
 
@@ -684,6 +701,7 @@ static int gntdev_open(struct inode *inode, struct file 
*flip)
mutex_init(>dmabuf_lock);
INIT_LIST_HEAD(>dmabuf_exp_list);
INIT_LIST_HEAD(>dmabuf_exp_wait_list);
+   INIT_LIST_HEAD(>dmabuf_imp_list);
 #endif
 
if (use_ptemod) {
@@ -1544,15 +1562,228 @@ static int dmabuf_exp_from_refs(struct gntdev_priv 
*priv, int flags,
 /* DMA buffer import support. */
 /* -- */
 
-static int dmabuf_imp_release(struct gntdev_priv *priv, u32 fd)
+static int
+dmabuf_imp_grant_foreign_access(struct page **pages, u32 *refs,
+   int count, int domid)
 {
-   return 0;
+   grant_ref_t priv_gref_head;
+   int i, ret;
+
+   ret = gnttab_alloc_grant_references(count, _gref_head);
+   if (ret < 0) {
+   pr_err("Cannot allocate grant references, ret %d\n", ret);
+   return ret;
+   }
+
+   for (i = 0; i < count; i++) {
+   int cur_ref;
+
+   cur_ref = gnttab_claim_grant_reference(_gref_head);
+   if (cur_ref < 0) {
+   ret = cur_ref;
+   pr_err("Cannot claim grant reference, ret %d\n", ret);
+   goto out;
+   }
+
+   gnttab_grant_foreign_access_ref(cur_ref, domid,
+   xen_page_to_gfn(pages[i]), 0);
+   refs[i] = cur_ref;
+   }
+
+   ret = 0;
+
+out:
+   gnttab_free_grant_references(priv_gref_head);
+   return ret;
+}
+
+static void dmabuf_imp_end_foreign_access(u32 *refs, int count)
+{
+   int i;
+
+   for (i = 0; i < count; i++)
+   if (refs[i] != GRANT_INVALID_REF)
+   gnttab_end_foreign_access(refs[i], 0, 0UL);
+}
+
+static void dmabuf_imp_free_storage(struct xen_dmabuf *xen_dmabuf)
+{
+   kfree(xen_dmabuf->pages);
+   kfree(xen_dmabuf->u.imp.refs);
+   kfree(xen_dmabuf);
+}
+
+static struct xen_dmabuf *dmabuf_imp_alloc_storage(int count)
+{
+   struct xen_dmabuf *xen_dmabuf;
+   int i;
+
+   xen_dmabuf = kzalloc(sizeof(*xen_dmabuf), GFP_KERNEL);
+   if (!xen_dmabuf)
+   goto fail;
+
+   xen_dmabuf->u.imp.refs = kcalloc(count,
+sizeof(xen_dmabuf->u.imp.refs[0]),
+GFP_KERNEL);
+   if (!xen_dmabuf->u.imp.refs)
+   goto fail;
+
+   xen_dmabuf->pages = kcalloc(count,
+   sizeof(xen_dmabuf->pages[0]),
+   GFP_KERNEL);
+   if (!xen_dmabuf->pages)
+   goto fail;
+
+   xen_dmabuf->nr_pages = count;
+
+   for (i = 0; i < count; 

[PATCH 8/8] xen/gntdev: Expose gntdev's dma-buf API for in-kernel use

2018-05-25 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

Allow creating grant device context for use by kernel modules which
require functionality, provided by gntdev. Export symbols for dma-buf
API provided by the module.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c| 116 ++--
 include/xen/grant_dev.h |  37 +
 2 files changed, 113 insertions(+), 40 deletions(-)
 create mode 100644 include/xen/grant_dev.h

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index d8b6168f2cd9..912056f3e909 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -684,14 +684,33 @@ static const struct mmu_notifier_ops gntdev_mmu_ops = {
 
 /* -- */
 
-static int gntdev_open(struct inode *inode, struct file *flip)
+void gntdev_free_context(struct gntdev_priv *priv)
+{
+   struct grant_map *map;
+
+   pr_debug("priv %p\n", priv);
+
+   mutex_lock(>lock);
+   while (!list_empty(>maps)) {
+   map = list_entry(priv->maps.next, struct grant_map, next);
+   list_del(>next);
+   gntdev_put_map(NULL /* already removed */, map);
+   }
+   WARN_ON(!list_empty(>freeable_maps));
+
+   mutex_unlock(>lock);
+
+   kfree(priv);
+}
+EXPORT_SYMBOL(gntdev_free_context);
+
+struct gntdev_priv *gntdev_alloc_context(struct device *dev)
 {
struct gntdev_priv *priv;
-   int ret = 0;
 
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);
 
INIT_LIST_HEAD(>maps);
INIT_LIST_HEAD(>freeable_maps);
@@ -704,6 +723,32 @@ static int gntdev_open(struct inode *inode, struct file 
*flip)
INIT_LIST_HEAD(>dmabuf_imp_list);
 #endif
 
+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
+   priv->dma_dev = dev;
+
+   /*
+* The device is not spawn from a device tree, so arch_setup_dma_ops
+* is not called, thus leaving the device with dummy DMA ops.
+* Fix this call of_dma_configure() with a NULL node to set
+* default DMA ops.
+*/
+   of_dma_configure(priv->dma_dev, NULL);
+#endif
+   pr_debug("priv %p\n", priv);
+
+   return priv;
+}
+EXPORT_SYMBOL(gntdev_alloc_context);
+
+static int gntdev_open(struct inode *inode, struct file *flip)
+{
+   struct gntdev_priv *priv;
+   int ret = 0;
+
+   priv = gntdev_alloc_context(gntdev_miscdev.this_device);
+   if (IS_ERR(priv))
+   return PTR_ERR(priv);
+
if (use_ptemod) {
priv->mm = get_task_mm(current);
if (!priv->mm) {
@@ -716,23 +761,11 @@ static int gntdev_open(struct inode *inode, struct file 
*flip)
}
 
if (ret) {
-   kfree(priv);
+   gntdev_free_context(priv);
return ret;
}
 
flip->private_data = priv;
-#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
-   priv->dma_dev = gntdev_miscdev.this_device;
-
-   /*
-* The device is not spawn from a device tree, so arch_setup_dma_ops
-* is not called, thus leaving the device with dummy DMA ops.
-* Fix this call of_dma_configure() with a NULL node to set
-* default DMA ops.
-*/
-   of_dma_configure(priv->dma_dev, NULL);
-#endif
-   pr_debug("priv %p\n", priv);
 
return 0;
 }
@@ -740,22 +773,11 @@ static int gntdev_open(struct inode *inode, struct file 
*flip)
 static int gntdev_release(struct inode *inode, struct file *flip)
 {
struct gntdev_priv *priv = flip->private_data;
-   struct grant_map *map;
-
-   pr_debug("priv %p\n", priv);
-
-   mutex_lock(>lock);
-   while (!list_empty(>maps)) {
-   map = list_entry(priv->maps.next, struct grant_map, next);
-   list_del(>next);
-   gntdev_put_map(NULL /* already removed */, map);
-   }
-   WARN_ON(!list_empty(>freeable_maps));
-   mutex_unlock(>lock);
 
if (use_ptemod)
mmu_notifier_unregister(>mn, priv->mm);
-   kfree(priv);
+
+   gntdev_free_context(priv);
return 0;
 }
 
@@ -1210,7 +1232,7 @@ dmabuf_exp_wait_obj_get_by_fd(struct gntdev_priv *priv, 
int fd)
return ret;
 }
 
-static int dmabuf_exp_wait_released(struct gntdev_priv *priv, int fd,
+int gntdev_dmabuf_exp_wait_released(struct gntdev_priv *priv, int fd,
int wait_to_ms)
 {
struct xen_dmabuf *xen_dmabuf;
@@ -1242,6 +1264,7 @@ static int dmabuf_exp_wait_released(struct gntdev_priv 
*priv, int fd,
dmabuf_exp_wait_obj_free(priv, obj);
return ret;
 }
+EXPORT_SYMBOL(gntdev_dmabuf_exp_wait_released);
 
 /* -- */
 /* DMA buffer export support. */
@@ -1511,7 +1534,7 @@ 

[PATCH 5/5] media: omap2: fix compile-testing with FB_OMAP2=m

2018-05-25 Thread Arnd Bergmann
Compile-testing with FB_OMAP2=m results in a link error:

drivers/media/platform/omap/omap_vout.o: In function `vidioc_streamoff':
omap_vout.c:(.text+0x1028): undefined reference to `omap_dispc_unregister_isr'
drivers/media/platform/omap/omap_vout.o: In function `omap_vout_release':
omap_vout.c:(.text+0x1330): undefined reference to `omap_dispc_unregister_isr'
drivers/media/platform/omap/omap_vout.o: In function `vidioc_streamon':
omap_vout.c:(.text+0x2dd4): undefined reference to `omap_dispc_register_isr'
drivers/media/platform/omap/omap_vout.o: In function `omap_vout_remove':

In order to enable compile-testing but still keep the correct dependency,
this changes the Kconfig logic so we only allow CONFIG_COMPILE_TEST
building when FB_OMAP is completely disabled, or have use the old
dependency on FB_OMAP to ensure VIDEO_OMAP2_VOUT is also a loadable
module when FB_OMAP2 is.

Fixes: d8555fd2f452 ("media: omap2: allow building it with COMPILE_TEST && 
DRM_OMAP")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/omap/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap/Kconfig 
b/drivers/media/platform/omap/Kconfig
index a414bcbb9b08..d827b6c285a6 100644
--- a/drivers/media/platform/omap/Kconfig
+++ b/drivers/media/platform/omap/Kconfig
@@ -6,7 +6,7 @@ config VIDEO_OMAP2_VOUT_VRFB
 config VIDEO_OMAP2_VOUT
tristate "OMAP2/OMAP3 V4L2-Display driver"
depends on MMU
-   depends on FB_OMAP2 || COMPILE_TEST
+   depends on FB_OMAP2 || (COMPILE_TEST && FB_OMAP2=n)
depends on ARCH_OMAP2 || ARCH_OMAP3 || COMPILE_TEST
select VIDEOBUF_GEN
select VIDEOBUF_DMA_CONTIG
-- 
2.9.0



[PATCH 4/5] media: marvel-ccic: allow ccic and mmp drivers to coexist

2018-05-25 Thread Arnd Bergmann
Randconfig builds fail when one of the two is a built-in driver and
the other one is a loadable module:

drivers/media/platform/marvell-ccic/mcam-core.o: In function `mccic_register':
mcam-core.c:(.text+0x2594): undefined reference to `__this_module'
drivers/media/platform/marvell-ccic/mcam-core.o:(.rodata+0x50): undefined 
reference to `__this_module'

The problem is that mcam-core.c can not be built both ways at the smae
time. However, we can make kbuild take care of that by making the core
driver a separate module, which can be either built-in or loadable
as needed.
Making it a separate module requires exporting a few symbols and
adding the module license from the header.

Fixes: 0a9c643c8faa ("media: marvel-ccic: re-enable mmp-driver build")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/marvell-ccic/Makefile| 9 -
 drivers/media/platform/marvell-ccic/mcam-core.c | 9 -
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/Makefile 
b/drivers/media/platform/marvell-ccic/Makefile
index 05a792c579a2..b3a4d0cdccb8 100644
--- a/drivers/media/platform/marvell-ccic/Makefile
+++ b/drivers/media/platform/marvell-ccic/Makefile
@@ -1,6 +1,5 @@
-obj-$(CONFIG_VIDEO_CAFE_CCIC) += cafe_ccic.o
-cafe_ccic-y := cafe-driver.o mcam-core.o
-
-obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera.o
-mmp_camera-y := mmp-driver.o mcam-core.o
+obj-$(CONFIG_VIDEO_CAFE_CCIC) += cafe_ccic.o mcam-core.o
+cafe_ccic-y := cafe-driver.o
 
+obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera.o mcam-core.o
+mmp_camera-y := mmp-driver.o
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c 
b/drivers/media/platform/marvell-ccic/mcam-core.c
index 80670142..dfdbd4354b74 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1720,6 +1720,7 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs)
}
return handled;
 }
+EXPORT_SYMBOL_GPL(mccic_irq);
 
 /* -- */
 /*
@@ -1830,7 +1831,7 @@ int mccic_register(struct mcam_camera *cam)
v4l2_device_unregister(>v4l2_dev);
return ret;
 }
-
+EXPORT_SYMBOL_GPL(mccic_register);
 
 void mccic_shutdown(struct mcam_camera *cam)
 {
@@ -1850,6 +1851,7 @@ void mccic_shutdown(struct mcam_camera *cam)
v4l2_ctrl_handler_free(>ctrl_handler);
v4l2_device_unregister(>v4l2_dev);
 }
+EXPORT_SYMBOL_GPL(mccic_shutdown);
 
 /*
  * Power management
@@ -1868,6 +1870,7 @@ void mccic_suspend(struct mcam_camera *cam)
}
mutex_unlock(>s_mutex);
 }
+EXPORT_SYMBOL_GPL(mccic_suspend);
 
 int mccic_resume(struct mcam_camera *cam)
 {
@@ -1898,4 +1901,8 @@ int mccic_resume(struct mcam_camera *cam)
}
return ret;
 }
+EXPORT_SYMBOL_GPL(mccic_resume);
 #endif /* CONFIG_PM */
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jonathan Corbet ");
-- 
2.9.0



Re: RFC: Request API and memory-to-memory devices

2018-05-25 Thread Hans Verkuil
On 25/05/18 16:16, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, May 24, 2018 at 10:44:13AM +0200, Hans Verkuil wrote:
>> Memory-to-memory devices have one video node, one internal control handler
>> but two vb2_queues (DMA engines). While often there is one buffer produced
>> for every buffer consumed, but this is by no means standard. E.g. 
>> deinterlacers
>> will produce on buffer for every two buffers consumed. Codecs that receive
>> a bit stream and can parse it to discover the framing may have no relation
>> between the number of buffers consumed and the number of buffers produced.
> 
> Do you have examples of such devices? I presume they're not supported in
> the current m2m API either, are they?
> 
>>
>> This poses a few problems for the Request API. Requiring that a request
>> contains the buffers for both output and capture queue will be difficult
>> to implement, especially in the latter case where there is no relationship
>> between the number of consumed and produced buffers.
>>
>> In addition, userspace can make two requests: one for the capture queue,
>> one for the output queue, each with associated controls. But since the
>> controls are shared between capture and output there is an issue of
>> what to do when the same control is set in both requests.
> 
> As I commented on v13, the two requests need to be handled separately in
> this case. Mem-to-mem devices are rather special in this respect; there's
> an established practice of matching buffers in the order they arrive from
> the queues, but that's not how the request API is intended to work: the
> buffers are associated to the request, and a request is processed
> independently of other requests.
> 
> While that approach might work for mem-to-mem devices at least in some use
> cases, it is not a feasible approach for other devices. As a consequence,
> will have different API semantics between mem2mem devices and the rest. I'd
> like to avoid that if possible: this will be similarly visible in the user
> applications as well.
> 
>>
>> I propose to restrict the usage of requests for m2m drivers to the output
>> queue only. This keeps things simple for both kernel and userspace and
>> avoids complex solutions.
> 
> If there's a convincing reason to use different API semantics, such as the
> relationship between different buffers being unknown to the user, then
> there very likely is a need to associate non-request data with
> request-bound data in the driver. But it'd be better to limit it to where
> it's really needed.
> 
>>
>> Requests only make sense if there is actually configuration you can apply
>> for each buffer, and while that's true for the output queue, on the capture
>> queue you just capture the result of whatever the device delivers. I don't
>> believe there is much if anything you can or want to control per-buffer.
> 
> May there be controls associated with the capture queue buffers?
> 

In theory that could happen for m2m devices, but those controls would be 
different
from controls associated with the output queue.

The core problem is that if there is no clear relationship between capture
and output buffers, then you cannot add a capture and an output buffer to
the same request. That simply wouldn't work.

How to signal this to the user? For m2m devices we could just specify that
in the spec and check this in the core. As Tomasz said, m2m devices are
already sufficiently special that I don't think this is a problem. But in
the more generic case (complex pipelines) I cannot off-hand think of something
elegant.

I guess I would have to sleep on this a bit.

Regards,

Hans


[PATCH 3/5] media: cx231xx: fix RC_CORE dependency

2018-05-25 Thread Arnd Bergmann
With CONFIG_RC_CORE=m and VIDEO_CX231XX=y, we get a link failure:

drivers/media/usb/cx231xx/cx231xx-input.o: In function `cx231xx_ir_init':
cx231xx-input.c:(.text+0xd4): undefined reference to `rc_allocate_device'

This narrows down the dependency so that only valid configurations
are allowed.

Fixes: 84545d2a1436 ("media: cx231xx: Remove RC_CORE dependency")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/usb/cx231xx/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/cx231xx/Kconfig 
b/drivers/media/usb/cx231xx/Kconfig
index 0f13192634c7..9e5b3e7c3ef5 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -15,7 +15,7 @@ config VIDEO_CX231XX
 
 config VIDEO_CX231XX_RC
bool "Conexant cx231xx Remote Controller additional support"
-   depends on RC_CORE
+   depends on RC_CORE=y || RC_CORE=VIDEO_CX231XX
depends on VIDEO_CX231XX
default y
---help---
-- 
2.9.0



[PATCH 2/5] media: v4l: cadence: include linux/slab.h

2018-05-25 Thread Arnd Bergmann
I ran into a randconfig build error with the new driver:

drivers/media/platform/cadence/cdns-csi2tx.c: In function 'csi2tx_probe':
drivers/media/platform/cadence/cdns-csi2tx.c:477:11: error: implicit 
declaration of function 'kzalloc'; did you mean 'd_alloc'? 
[-Werror=implicit-function-declaration]

kzalloc() is declared in linux/slab.h, so let's include this to make it
build in all configurations.

Fixes: 84b477e6d4bc ("media: v4l: cadence: Add Cadence MIPI-CSI2 TX driver")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 1 +
 drivers/media/platform/cadence/cdns-csi2tx.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c 
b/drivers/media/platform/cadence/cdns-csi2rx.c
index a0f02916006b..43e43c7b3e98 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c 
b/drivers/media/platform/cadence/cdns-csi2tx.c
index dfa1d88d955b..40d0de690ff4 100644
--- a/drivers/media/platform/cadence/cdns-csi2tx.c
+++ b/drivers/media/platform/cadence/cdns-csi2tx.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
-- 
2.9.0



[PATCH 1/5] media: omap3isp: fix warning for !CONFIG_PM

2018-05-25 Thread Arnd Bergmann
The final version of the COMPILE_TEST patch for this driver missed
one warning about suspend/resume functions that can now appear
on platforms that don't always set CONFIG_PM:

drivers/media/platform/omap3isp/isp.c:1008:13: error: 'isp_resume_modules' 
defined but not used [-Werror=unused-function]
 static void isp_resume_modules(struct isp_device *isp)
 ^~
drivers/media/platform/omap3isp/isp.c:974:12: error: 'isp_suspend_modules' 
defined but not used [-Werror=unused-function]
 static int isp_suspend_modules(struct isp_device *isp)

This marks the respective functions as __maybe_unused as an easy
workaround.

Fixes: 243131134be4 ("media: omap3isp: Allow it to build with COMPILE_TEST")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/omap3isp/isp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index f22cf351e3ee..a658c12eead1 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -971,7 +971,7 @@ static void isp_resume_module_pipeline(struct media_entity 
*me)
  * Returns 0 if suspend left in idle state all the submodules properly,
  * or returns 1 if a general Reset is required to suspend the submodules.
  */
-static int isp_suspend_modules(struct isp_device *isp)
+static int __maybe_unused isp_suspend_modules(struct isp_device *isp)
 {
unsigned long timeout;
 
@@ -1005,7 +1005,7 @@ static int isp_suspend_modules(struct isp_device *isp)
  * isp_resume_modules - Resume ISP submodules.
  * @isp: OMAP3 ISP device
  */
-static void isp_resume_modules(struct isp_device *isp)
+static void __maybe_unused isp_resume_modules(struct isp_device *isp)
 {
omap3isp_stat_resume(>isp_aewb);
omap3isp_stat_resume(>isp_af);
-- 
2.9.0



Re: [PATCH 3/3] media: rc: nuvoton: Keep device enabled during reg init

2018-05-25 Thread Michał Winiarski
On Fri, May 25, 2018 at 02:59:41PM +0100, Sean Young wrote:
> On Fri, May 25, 2018 at 03:35:23PM +0200, Michał Winiarski wrote:
> > On Thu, May 24, 2018 at 12:31:40PM +0100, Sean Young wrote:
> > > On Mon, May 21, 2018 at 04:38:03PM +0200, Michał Winiarski wrote:
> > > > Doing writes when the device is disabled seems to be a NOOP.
> > > > Let's enable the device, write the values, and then disable it on init.
> > > > This changes the behavior for wake device, which is now being disabled
> > > > after init.
> > > 
> > > I don't have the datasheet so I might be misunderstanding this. We want
> > > the IR wakeup to work fine even after kernel crash/power loss, right?
> > 
> > [snip]
> > 
> > Right, that makes sense. I completely ignored this scenario.
> >  
> > > > -   /* enable the CIR WAKE logical device */
> > > > -   nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
> > > > +   nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
> > > 
> > > The way I read this is that the CIR, not CIR_WAKE, is being disabled,
> > > which seems contrary to what the commit message says.
> > > 
> > 
> > That's a typo. And by accident it makes the wake_device work correctly :)
> > I think that registers init logic was still broken though, operating under 
> > the
> > assumption that the device is enabled on module load...
> > 
> > I guess we should just remove disable(LOGICAL_DEV_CIR) from wake_regs_init.
> > 
> > Have you already included this in any non-rebasing tree?
> 
> Nothing has been applied yet.
> 
> > Should I send a v2 or fixup on top?
> 
> I don't have the hardware to test this, a v2 would be appreciated.
> 
> We're late in the release cycle and I'm wondering if this patch would also
> solve the nuvoton probe problem:
> 
> https://patchwork.linuxtv.org/patch/49874/

It causes us to go back to previous behavior (we're refcounting open/close,
with your patch initial open on my system is coming from kbd_connect(), so
userspace close() doesn't propagate to nuvoton-cir).

It passes my test of "load the module with debug=1, see if I'm getting
interrupts".

If there's any scenario in which->close() would be called, it's still going to
be broken.

-Michał

> 
> Thanks,
> 
> Sean


Re: RFC: Request API and memory-to-memory devices

2018-05-25 Thread Tomasz Figa
On Fri, May 25, 2018 at 11:17 PM Sakari Ailus 
wrote:

> Hi Hans,

> On Thu, May 24, 2018 at 10:44:13AM +0200, Hans Verkuil wrote:
> > Memory-to-memory devices have one video node, one internal control
handler
> > but two vb2_queues (DMA engines). While often there is one buffer
produced
> > for every buffer consumed, but this is by no means standard. E.g.
deinterlacers
> > will produce on buffer for every two buffers consumed. Codecs that
receive
> > a bit stream and can parse it to discover the framing may have no
relation
> > between the number of buffers consumed and the number of buffers
produced.

> Do you have examples of such devices? I presume they're not supported in
> the current m2m API either, are they?

All stateful video decoders work like this and this means all video
decoders currently in mainline (e.g. s5p-mfc, coda, mtk-vcodec), since we
don't support stateless decoders yet. This is due to the nature of encoded
bitstreams, such as H264, which can have frame reordering, long reference
frame range and other tricks.

I wouldn't say there is something like m2m API, unless you mean the m2m
helpers, but that's not mandatory. Stateful decoders work according to V4L2
Codec API, which is a further specification of V4L2 (or something like
application notes?).


> >
> > This poses a few problems for the Request API. Requiring that a request
> > contains the buffers for both output and capture queue will be difficult
> > to implement, especially in the latter case where there is no
relationship
> > between the number of consumed and produced buffers.
> >
> > In addition, userspace can make two requests: one for the capture queue,
> > one for the output queue, each with associated controls. But since the
> > controls are shared between capture and output there is an issue of
> > what to do when the same control is set in both requests.

> As I commented on v13, the two requests need to be handled separately in
> this case. Mem-to-mem devices are rather special in this respect; there's
> an established practice of matching buffers in the order they arrive from
> the queues, but that's not how the request API is intended to work: the
> buffers are associated to the request, and a request is processed
> independently of other requests.

> While that approach might work for mem-to-mem devices at least in some use
> cases, it is not a feasible approach for other devices. As a consequence,
> will have different API semantics between mem2mem devices and the rest.
I'd
> like to avoid that if possible: this will be similarly visible in the user
> applications as well.


I'd say that the semantics for m2m devices are already significantly
different from non-m2m devices even without Request API, e.g. we have codec
API for codecs. Everyone is aware of this special treatment and it doesn't
seem to be a problem for anyone.

> >
> > I propose to restrict the usage of requests for m2m drivers to the
output
> > queue only. This keeps things simple for both kernel and userspace and
> > avoids complex solutions.

> If there's a convincing reason to use different API semantics, such as the
> relationship between different buffers being unknown to the user, then
> there very likely is a need to associate non-request data with
> request-bound data in the driver. But it'd be better to limit it to where
> it's really needed.

> >
> > Requests only make sense if there is actually configuration you can
apply
> > for each buffer, and while that's true for the output queue, on the
capture
> > queue you just capture the result of whatever the device delivers. I
don't
> > believe there is much if anything you can or want to control per-buffer.

> May there be controls associated with the capture queue buffers?

What would be the value in having such association? Don't we want to
associate thing with particular hardware job (which is represented by a
request) rather than buffer?

Best regards,
Tomasz


[PATCH] udmabuf: fix odd_ptr_err.cocci warnings

2018-05-25 Thread Julia Lawall
From: kbuild test robot 

drivers/dma-buf/udmabuf.c:167:6-12: inconsistent IS_ERR and PTR_ERR on line 168.

 PTR_ERR should access the value just tested by IS_ERR

Semantic patch information:
 There can be false positives in the patch case, where it is the call to
 IS_ERR that is wrong.

Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci

Fixes: cc2d0e91bc15 ("udmabuf: driver update")
Signed-off-by: kbuild test robot 
Signed-off-by: linux-ker...@vger.kernel.org
---

tree:   git://git.kraxel.org/linux udmabuf
head:   cc2d0e91bc15849baff695d175bfb8fba35f1465
commit: cc2d0e91bc15849baff695d175bfb8fba35f1465 [6/6] udmabuf: driver
update

 udmabuf.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -165,7 +165,7 @@ static long udmabuf_ioctl_create(struct
page = shmem_read_mapping_page(
file_inode(ubuf->filp)->i_mapping, pgoff + pgidx);
if (IS_ERR(page)) {
-   ret = PTR_ERR(buf);
+   ret = PTR_ERR(page);
goto err_put_pages;
}
ubuf->pages[pgidx] = page;


[PATCH v2 3/3] media: rc: nuvoton: Keep device enabled during reg init

2018-05-25 Thread Michał Winiarski
Doing writes when the device is disabled seems to be a NOOP.
For CIR device, we should enable it, intialize it, and then disable it
until it's opened. CIR_WAKE should always be enabled.

Signed-off-by: Michał Winiarski 
Cc: Jarod Wilson 
Cc: Sean Young 
---
Changes since v1:
 - Don't mix CIR_WAKE with CIR device
 - Correct the commit message
---
 drivers/media/rc/nuvoton-cir.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/nuvoton-cir.c b/drivers/media/rc/nuvoton-cir.c
index eebd6fef5602..b8299c9a9744 100644
--- a/drivers/media/rc/nuvoton-cir.c
+++ b/drivers/media/rc/nuvoton-cir.c
@@ -535,6 +535,8 @@ static void nvt_set_cir_iren(struct nvt_dev *nvt)
 
 static void nvt_cir_regs_init(struct nvt_dev *nvt)
 {
+   nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR);
+
/* set sample limit count (PE interrupt raised when reached) */
nvt_cir_reg_write(nvt, CIR_RX_LIMIT_COUNT >> 8, CIR_SLCH);
nvt_cir_reg_write(nvt, CIR_RX_LIMIT_COUNT & 0xff, CIR_SLCL);
@@ -546,10 +548,14 @@ static void nvt_cir_regs_init(struct nvt_dev *nvt)
/* clear hardware rx and tx fifos */
nvt_clear_cir_fifo(nvt);
nvt_clear_tx_fifo(nvt);
+
+   nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
 }
 
 static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
 {
+   nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
+
/*
 * Disable RX, set specific carrier on = low, off = high,
 * and sample period (currently 50us)
@@ -561,9 +567,6 @@ static void nvt_cir_wake_regs_init(struct nvt_dev *nvt)
 
/* clear any and all stray interrupts */
nvt_cir_wake_reg_write(nvt, 0xff, CIR_WAKE_IRSTS);
-
-   /* enable the CIR WAKE logical device */
-   nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
 }
 
 static void nvt_enable_wake(struct nvt_dev *nvt)
-- 
2.17.0



Re: [PATCH v2.2 2/2] smiapp: Support the "rotation" property

2018-05-25 Thread Sakari Ailus
On Fri, May 25, 2018 at 04:09:55PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, May 25, 2018 at 04:52:35PM +0300, Sakari Ailus wrote:
> > Use the "rotation" property to tell that the sensor is mounted upside
> > down. This reverses the behaviour of the VFLIP and HFLIP controls as well
> > as the pixel order.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> 
> Reviewed-by: Sebastian Reichel 

Danke schön!

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


Re: RFC: Request API and memory-to-memory devices

2018-05-25 Thread Sakari Ailus
Hi Hans,

On Thu, May 24, 2018 at 10:44:13AM +0200, Hans Verkuil wrote:
> Memory-to-memory devices have one video node, one internal control handler
> but two vb2_queues (DMA engines). While often there is one buffer produced
> for every buffer consumed, but this is by no means standard. E.g. 
> deinterlacers
> will produce on buffer for every two buffers consumed. Codecs that receive
> a bit stream and can parse it to discover the framing may have no relation
> between the number of buffers consumed and the number of buffers produced.

Do you have examples of such devices? I presume they're not supported in
the current m2m API either, are they?

> 
> This poses a few problems for the Request API. Requiring that a request
> contains the buffers for both output and capture queue will be difficult
> to implement, especially in the latter case where there is no relationship
> between the number of consumed and produced buffers.
> 
> In addition, userspace can make two requests: one for the capture queue,
> one for the output queue, each with associated controls. But since the
> controls are shared between capture and output there is an issue of
> what to do when the same control is set in both requests.

As I commented on v13, the two requests need to be handled separately in
this case. Mem-to-mem devices are rather special in this respect; there's
an established practice of matching buffers in the order they arrive from
the queues, but that's not how the request API is intended to work: the
buffers are associated to the request, and a request is processed
independently of other requests.

While that approach might work for mem-to-mem devices at least in some use
cases, it is not a feasible approach for other devices. As a consequence,
will have different API semantics between mem2mem devices and the rest. I'd
like to avoid that if possible: this will be similarly visible in the user
applications as well.

> 
> I propose to restrict the usage of requests for m2m drivers to the output
> queue only. This keeps things simple for both kernel and userspace and
> avoids complex solutions.

If there's a convincing reason to use different API semantics, such as the
relationship between different buffers being unknown to the user, then
there very likely is a need to associate non-request data with
request-bound data in the driver. But it'd be better to limit it to where
it's really needed.

> 
> Requests only make sense if there is actually configuration you can apply
> for each buffer, and while that's true for the output queue, on the capture
> queue you just capture the result of whatever the device delivers. I don't
> believe there is much if anything you can or want to control per-buffer.

May there be controls associated with the capture queue buffers?

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


Re: [PATCH v2.2 2/2] smiapp: Support the "rotation" property

2018-05-25 Thread Sebastian Reichel
Hi,

On Fri, May 25, 2018 at 04:52:35PM +0300, Sakari Ailus wrote:
> Use the "rotation" property to tell that the sensor is mounted upside
> down. This reverses the behaviour of the VFLIP and HFLIP controls as well
> as the pixel order.
> 
> Signed-off-by: Sakari Ailus 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

> since v2.2:
> 
> - Fix property name in code.
> 
>  .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
>  drivers/media/i2c/smiapp/smiapp-core.c   | 16 
> 
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
> b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> index 33f10a94c381..6f509657470e 100644
> --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> @@ -29,6 +29,8 @@ Optional properties
>  - reset-gpios: XSHUTDOWN GPIO
>  - flash-leds: See ../video-interfaces.txt
>  - lens-focus: See ../video-interfaces.txt
> +- rotation: Integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).
>  
>  
>  Endpoint node mandatory properties
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index e1f8208581aa..e9e0f21efc2a 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig 
> *smiapp_get_hwconfig(struct device *dev)
>   struct v4l2_fwnode_endpoint *bus_cfg;
>   struct fwnode_handle *ep;
>   struct fwnode_handle *fwnode = dev_fwnode(dev);
> + u32 rotation;
>   int i;
>   int rval;
>  
> @@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
> *smiapp_get_hwconfig(struct device *dev)
>  
>   dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
>  
> + rval = fwnode_property_read_u32(fwnode, "rotation", );
> + if (!rval) {
> + switch (rotation) {
> + case 180:
> + hwcfg->module_board_orient =
> + SMIAPP_MODULE_BOARD_ORIENT_180;
> + /* Fall through */
> + case 0:
> + break;
> + default:
> + dev_err(dev, "invalid rotation %u\n", rotation);
> + goto out_err;
> + }
> + }
> +
>   /* NVM size is not mandatory */
>   fwnode_property_read_u32(fwnode, "nokia,nvm-size", >nvm_size);
>  
> -- 
> 2.11.0
> 


signature.asc
Description: PGP signature


[PATCH v3] Add udmabuf misc device

2018-05-25 Thread Gerd Hoffmann
A driver to let userspace turn memfd regions into dma-bufs.

Use case:  Allows qemu create dmabufs for the vga framebuffer or
virtio-gpu ressources.  Then they can be passed around to display
those guest things on the host.  To spice client for classic full
framebuffer display, and hopefully some day to wayland server for
seamless guest window display.

Note: Initial revision which supports a single region only so it
  can't handle virtio-gpu ressources yet.

qemu test branch:
  https://git.kraxel.org/cgit/qemu/log/?h=sirius/udmabuf

Cc: David Airlie 
Cc: Tomeu Vizoso 
Cc: Daniel Vetter 
Signed-off-by: Gerd Hoffmann 
---
 include/uapi/linux/udmabuf.h  |  19 ++
 drivers/dma-buf/udmabuf.c | 240 ++
 tools/testing/selftests/drivers/dma-buf/udmabuf.c |  95 +
 drivers/dma-buf/Kconfig   |   7 +
 drivers/dma-buf/Makefile  |   1 +
 tools/testing/selftests/drivers/dma-buf/Makefile  |   5 +
 6 files changed, 367 insertions(+)
 create mode 100644 include/uapi/linux/udmabuf.h
 create mode 100644 drivers/dma-buf/udmabuf.c
 create mode 100644 tools/testing/selftests/drivers/dma-buf/udmabuf.c
 create mode 100644 tools/testing/selftests/drivers/dma-buf/Makefile

diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h
new file mode 100644
index 00..2fbe69cf05
--- /dev/null
+++ b/include/uapi/linux/udmabuf.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_UDMABUF_H
+#define _UAPI_LINUX_UDMABUF_H
+
+#include 
+#include 
+
+#define UDMABUF_FLAGS_CLOEXEC  0x01
+
+struct udmabuf_create {
+   __u32 memfd;
+   __u32 flags;
+   __u64 offset;
+   __u64 size;
+};
+
+#define UDMABUF_CREATE _IOW(0x42, 0x23, struct udmabuf_create)
+
+#endif /* _UAPI_LINUX_UDMABUF_H */
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
new file mode 100644
index 00..f9600dc985
--- /dev/null
+++ b/drivers/dma-buf/udmabuf.c
@@ -0,0 +1,240 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+struct udmabuf {
+   struct file *filp;
+   u32 pagecount;
+   struct page **pages;
+};
+
+static int udmabuf_vm_fault(struct vm_fault *vmf)
+{
+   struct vm_area_struct *vma = vmf->vma;
+   struct udmabuf *ubuf = vma->vm_private_data;
+
+   if (WARN_ON(vmf->pgoff >= ubuf->pagecount))
+   return VM_FAULT_SIGBUS;
+
+   vmf->page = ubuf->pages[vmf->pgoff];
+   get_page(vmf->page);
+   return 0;
+}
+
+static const struct vm_operations_struct udmabuf_vm_ops = {
+   .fault = udmabuf_vm_fault,
+};
+
+static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma)
+{
+   struct udmabuf *ubuf = buf->priv;
+
+   if ((vma->vm_flags & VM_SHARED) == 0)
+   return -EINVAL;
+
+   vma->vm_ops = _vm_ops;
+   vma->vm_private_data = ubuf;
+   return 0;
+}
+
+static struct sg_table *map_udmabuf(struct dma_buf_attachment *at,
+   enum dma_data_direction direction)
+{
+   struct udmabuf *ubuf = at->dmabuf->priv;
+   struct sg_table *sg;
+
+   sg = kzalloc(sizeof(*sg), GFP_KERNEL);
+   if (!sg)
+   goto err1;
+   if (sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount,
+ 0, ubuf->pagecount << PAGE_SHIFT,
+ GFP_KERNEL) < 0)
+   goto err2;
+   if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction))
+   goto err3;
+
+   return sg;
+
+err3:
+   sg_free_table(sg);
+err2:
+   kfree(sg);
+err1:
+   return ERR_PTR(-ENOMEM);
+}
+
+static void unmap_udmabuf(struct dma_buf_attachment *at,
+ struct sg_table *sg,
+ enum dma_data_direction direction)
+{
+   sg_free_table(sg);
+   kfree(sg);
+}
+
+static void release_udmabuf(struct dma_buf *buf)
+{
+   struct udmabuf *ubuf = buf->priv;
+   pgoff_t pg;
+
+   for (pg = 0; pg < ubuf->pagecount; pg++)
+   put_page(ubuf->pages[pg]);
+   fput(ubuf->filp);
+   kfree(ubuf->pages);
+   kfree(ubuf);
+}
+
+static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
+{
+   struct udmabuf *ubuf = buf->priv;
+   struct page *page = ubuf->pages[page_num];
+
+   return kmap_atomic(page);
+}
+
+static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
+{
+   struct udmabuf *ubuf = buf->priv;
+   struct page *page = ubuf->pages[page_num];
+
+   return kmap(page);

Re: [PATCH v2 1/2] dt-bindings: media: Define "rotation" property for sensors

2018-05-25 Thread Sebastian Reichel
Hi,

On Fri, May 25, 2018 at 03:27:25PM +0300, Sakari Ailus wrote:
> Sensors are occasionally mounted upside down to systems such as mobile
> phones or tablets. In order to use such a sensor without having to turn
> every image upside down, most camera sensors support reversing the readout
> order by setting both horizontal and vertical flipping.
> 
> This patch documents the "rotation" property for camera sensors, mirroring
> what is defined for displays in
> Documentation/devicetree/bindings/display/panel/panel.txt .
> 
> Signed-off-by: Sakari Ailus 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  Documentation/devicetree/bindings/media/video-interfaces.txt | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 258b8dfddf48..52b7c7b57842 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -85,6 +85,10 @@ Optional properties
>  
>  - lens-focus: A phandle to the node of the focus lens controller.
>  
> +- rotation: The device, typically an image sensor, is not mounted upright,
> +  but a number of degrees counter clockwise. Typical values are 0 and 180
> +  (upside down).
> +
>  
>  Optional endpoint properties
>  
> -- 
> 2.11.0
> 


signature.asc
Description: PGP signature


Re: [PATCH 3/3] media: rc: nuvoton: Keep device enabled during reg init

2018-05-25 Thread Sean Young
On Fri, May 25, 2018 at 03:35:23PM +0200, Michał Winiarski wrote:
> On Thu, May 24, 2018 at 12:31:40PM +0100, Sean Young wrote:
> > On Mon, May 21, 2018 at 04:38:03PM +0200, Michał Winiarski wrote:
> > > Doing writes when the device is disabled seems to be a NOOP.
> > > Let's enable the device, write the values, and then disable it on init.
> > > This changes the behavior for wake device, which is now being disabled
> > > after init.
> > 
> > I don't have the datasheet so I might be misunderstanding this. We want
> > the IR wakeup to work fine even after kernel crash/power loss, right?
> 
> [snip]
> 
> Right, that makes sense. I completely ignored this scenario.
>  
> > > - /* enable the CIR WAKE logical device */
> > > - nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
> > > + nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
> > 
> > The way I read this is that the CIR, not CIR_WAKE, is being disabled,
> > which seems contrary to what the commit message says.
> > 
> 
> That's a typo. And by accident it makes the wake_device work correctly :)
> I think that registers init logic was still broken though, operating under the
> assumption that the device is enabled on module load...
> 
> I guess we should just remove disable(LOGICAL_DEV_CIR) from wake_regs_init.
> 
> Have you already included this in any non-rebasing tree?

Nothing has been applied yet.

> Should I send a v2 or fixup on top?

I don't have the hardware to test this, a v2 would be appreciated.

We're late in the release cycle and I'm wondering if this patch would also
solve the nuvoton probe problem:

https://patchwork.linuxtv.org/patch/49874/

Thanks,

Sean


[PATCH v2.2 2/2] smiapp: Support the "rotation" property

2018-05-25 Thread Sakari Ailus
Use the "rotation" property to tell that the sensor is mounted upside
down. This reverses the behaviour of the VFLIP and HFLIP controls as well
as the pixel order.

Signed-off-by: Sakari Ailus 
---
since v2.2:

- Fix property name in code.

 .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
 drivers/media/i2c/smiapp/smiapp-core.c   | 16 
 2 files changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
index 33f10a94c381..6f509657470e 100644
--- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
+++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
@@ -29,6 +29,8 @@ Optional properties
 - reset-gpios: XSHUTDOWN GPIO
 - flash-leds: See ../video-interfaces.txt
 - lens-focus: See ../video-interfaces.txt
+- rotation: Integer property; valid values are 0 (sensor mounted upright)
+   and 180 (sensor mounted upside down).
 
 
 Endpoint node mandatory properties
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index e1f8208581aa..e9e0f21efc2a 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct 
device *dev)
struct v4l2_fwnode_endpoint *bus_cfg;
struct fwnode_handle *ep;
struct fwnode_handle *fwnode = dev_fwnode(dev);
+   u32 rotation;
int i;
int rval;
 
@@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
*smiapp_get_hwconfig(struct device *dev)
 
dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
+   rval = fwnode_property_read_u32(fwnode, "rotation", );
+   if (!rval) {
+   switch (rotation) {
+   case 180:
+   hwcfg->module_board_orient =
+   SMIAPP_MODULE_BOARD_ORIENT_180;
+   /* Fall through */
+   case 0:
+   break;
+   default:
+   dev_err(dev, "invalid rotation %u\n", rotation);
+   goto out_err;
+   }
+   }
+
/* NVM size is not mandatory */
fwnode_property_read_u32(fwnode, "nokia,nvm-size", >nvm_size);
 
-- 
2.11.0



Re: [PATCH v2 2/2] smiapp: Support the "upside-down" property

2018-05-25 Thread Sakari Ailus
On Fri, May 25, 2018 at 03:41:59PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, May 25, 2018 at 03:27:26PM +0300, Sakari Ailus wrote:
> > Use the "upside-down" property to tell that the sensor is mounted upside
> > down. This reverses the behaviour of the VFLIP and HFLIP controls as well
> > as the pixel order.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> 
> Patch subject and description should be s/"upside-down"/"rotation"/g ?
> 
> >  .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
> >  drivers/media/i2c/smiapp/smiapp-core.c   | 16 
> > 
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
> > b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > index 33f10a94c381..6f509657470e 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > @@ -29,6 +29,8 @@ Optional properties
> >  - reset-gpios: XSHUTDOWN GPIO
> >  - flash-leds: See ../video-interfaces.txt
> >  - lens-focus: See ../video-interfaces.txt
> > +- rotation: Integer property; valid values are 0 (sensor mounted upright)
> > +   and 180 (sensor mounted upside down).
> >  
> >  
> >  Endpoint node mandatory properties
> > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> > b/drivers/media/i2c/smiapp/smiapp-core.c
> > index e1f8208581aa..32286df6ab43 100644
> > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > @@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig 
> > *smiapp_get_hwconfig(struct device *dev)
> > struct v4l2_fwnode_endpoint *bus_cfg;
> > struct fwnode_handle *ep;
> > struct fwnode_handle *fwnode = dev_fwnode(dev);
> > +   u32 rotation;
> > int i;
> > int rval;
> >  
> > @@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
> > *smiapp_get_hwconfig(struct device *dev)
> >  
> > dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
> >  
> > +   rval = fwnode_property_read_u32(fwnode, "upside-down", );
> 
> "rotation"

Thanks. Both fixed in v2.2.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi


Re: [PATCH v2 2/2] smiapp: Support the "upside-down" property

2018-05-25 Thread Sebastian Reichel
Hi,

On Fri, May 25, 2018 at 03:27:26PM +0300, Sakari Ailus wrote:
> Use the "upside-down" property to tell that the sensor is mounted upside
> down. This reverses the behaviour of the VFLIP and HFLIP controls as well
> as the pixel order.
> 
> Signed-off-by: Sakari Ailus 
> ---

Patch subject and description should be s/"upside-down"/"rotation"/g ?

>  .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
>  drivers/media/i2c/smiapp/smiapp-core.c   | 16 
> 
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
> b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> index 33f10a94c381..6f509657470e 100644
> --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> @@ -29,6 +29,8 @@ Optional properties
>  - reset-gpios: XSHUTDOWN GPIO
>  - flash-leds: See ../video-interfaces.txt
>  - lens-focus: See ../video-interfaces.txt
> +- rotation: Integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).
>  
>  
>  Endpoint node mandatory properties
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index e1f8208581aa..32286df6ab43 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig 
> *smiapp_get_hwconfig(struct device *dev)
>   struct v4l2_fwnode_endpoint *bus_cfg;
>   struct fwnode_handle *ep;
>   struct fwnode_handle *fwnode = dev_fwnode(dev);
> + u32 rotation;
>   int i;
>   int rval;
>  
> @@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
> *smiapp_get_hwconfig(struct device *dev)
>  
>   dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
>  
> + rval = fwnode_property_read_u32(fwnode, "upside-down", );

"rotation"

> + if (!rval) {
> + switch (rotation) {
> + case 180:
> + hwcfg->module_board_orient =
> + SMIAPP_MODULE_BOARD_ORIENT_180;
> + /* Fall through */
> + case 0:
> + break;
> + default:
> + dev_err(dev, "invalid rotation %u\n", rotation);
> + goto out_err;
> + }
> + }
> +
>   /* NVM size is not mandatory */
>   fwnode_property_read_u32(fwnode, "nokia,nvm-size", >nvm_size);

-- Sebastian


signature.asc
Description: PGP signature


[PATCH v2.1 2/2] smiapp: Support the "rotation" property

2018-05-25 Thread Sakari Ailus
Use the "rotation" property to tell that the sensor is mounted upside
down. This reverses the behaviour of the VFLIP and HFLIP controls as well
as the pixel order.

Signed-off-by: Sakari Ailus 
---
since v2:

- Fix the property name in the commit message

 .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
 drivers/media/i2c/smiapp/smiapp-core.c   | 16 
 2 files changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
index 33f10a94c381..6f509657470e 100644
--- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
+++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
@@ -29,6 +29,8 @@ Optional properties
 - reset-gpios: XSHUTDOWN GPIO
 - flash-leds: See ../video-interfaces.txt
 - lens-focus: See ../video-interfaces.txt
+- rotation: Integer property; valid values are 0 (sensor mounted upright)
+   and 180 (sensor mounted upside down).
 
 
 Endpoint node mandatory properties
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index e1f8208581aa..32286df6ab43 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct 
device *dev)
struct v4l2_fwnode_endpoint *bus_cfg;
struct fwnode_handle *ep;
struct fwnode_handle *fwnode = dev_fwnode(dev);
+   u32 rotation;
int i;
int rval;
 
@@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
*smiapp_get_hwconfig(struct device *dev)
 
dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
+   rval = fwnode_property_read_u32(fwnode, "upside-down", );
+   if (!rval) {
+   switch (rotation) {
+   case 180:
+   hwcfg->module_board_orient =
+   SMIAPP_MODULE_BOARD_ORIENT_180;
+   /* Fall through */
+   case 0:
+   break;
+   default:
+   dev_err(dev, "invalid rotation %u\n", rotation);
+   goto out_err;
+   }
+   }
+
/* NVM size is not mandatory */
fwnode_property_read_u32(fwnode, "nokia,nvm-size", >nvm_size);
 
-- 
2.11.0



Re: [PATCH 3/3] media: rc: nuvoton: Keep device enabled during reg init

2018-05-25 Thread Michał Winiarski
On Thu, May 24, 2018 at 12:31:40PM +0100, Sean Young wrote:
> On Mon, May 21, 2018 at 04:38:03PM +0200, Michał Winiarski wrote:
> > Doing writes when the device is disabled seems to be a NOOP.
> > Let's enable the device, write the values, and then disable it on init.
> > This changes the behavior for wake device, which is now being disabled
> > after init.
> 
> I don't have the datasheet so I might be misunderstanding this. We want
> the IR wakeup to work fine even after kernel crash/power loss, right?

[snip]

Right, that makes sense. I completely ignored this scenario.
 
> > -   /* enable the CIR WAKE logical device */
> > -   nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
> > +   nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
> 
> The way I read this is that the CIR, not CIR_WAKE, is being disabled,
> which seems contrary to what the commit message says.
> 

That's a typo. And by accident it makes the wake_device work correctly :)
I think that registers init logic was still broken though, operating under the
assumption that the device is enabled on module load...

I guess we should just remove disable(LOGICAL_DEV_CIR) from wake_regs_init.

Have you already included this in any non-rebasing tree?
Should I send a v2 or fixup on top?

-Michał

> 
> Sean
> 
> >  }
> >  
> >  static void nvt_enable_wake(struct nvt_dev *nvt)
> > -- 
> > 2.17.0


Re: [PATCH v2] media: davinci vpbe: array underflow in vpbe_enum_outputs()

2018-05-25 Thread Dan Carpenter
On Fri, May 25, 2018 at 03:16:21PM +0200, Hans Verkuil wrote:
> On 25/05/18 15:12, Dan Carpenter wrote:
> > In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
> > the problem is that temp_index can be negative.  I've made
> > cgf->num_outputs unsigned to fix this issue.
> 
> Shouldn't temp_index also be made unsigned? It certainly would make a lot of
> sense to do that.

Yeah, sure.  It doesn't make any difference in terms of runtime but it's
probably cleaner.

regards,
dan carpenter




Re: [PATCH v2] media: davinci vpbe: array underflow in vpbe_enum_outputs()

2018-05-25 Thread Hans Verkuil
On 25/05/18 15:12, Dan Carpenter wrote:
> In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
> the problem is that temp_index can be negative.  I've made
> cgf->num_outputs unsigned to fix this issue.

Shouldn't temp_index also be made unsigned? It certainly would make a lot of
sense to do that.

Regards,

Hans

> 
> Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
> Signed-off-by: Dan Carpenter 
> ---
> v2: fix it a different way
> 
> diff --git a/include/media/davinci/vpbe.h b/include/media/davinci/vpbe.h
> index 79a566d7defd..180a05e91497 100644
> --- a/include/media/davinci/vpbe.h
> +++ b/include/media/davinci/vpbe.h
> @@ -92,7 +92,7 @@ struct vpbe_config {
>   struct encoder_config_info *ext_encoders;
>   /* amplifier information goes here */
>   struct amp_config_info *amp;
> - int num_outputs;
> + unsigned int num_outputs;
>   /* Order is venc outputs followed by LCD and then external encoders */
>   struct vpbe_output *outputs;
>  };
> 



[PATCH v2] media: davinci vpbe: array underflow in vpbe_enum_outputs()

2018-05-25 Thread Dan Carpenter
In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
the problem is that temp_index can be negative.  I've made
cgf->num_outputs unsigned to fix this issue.

Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
Signed-off-by: Dan Carpenter 
---
v2: fix it a different way

diff --git a/include/media/davinci/vpbe.h b/include/media/davinci/vpbe.h
index 79a566d7defd..180a05e91497 100644
--- a/include/media/davinci/vpbe.h
+++ b/include/media/davinci/vpbe.h
@@ -92,7 +92,7 @@ struct vpbe_config {
struct encoder_config_info *ext_encoders;
/* amplifier information goes here */
struct amp_config_info *amp;
-   int num_outputs;
+   unsigned int num_outputs;
/* Order is venc outputs followed by LCD and then external encoders */
struct vpbe_output *outputs;
 };


[PATCH v2 2/2] smiapp: Support the "upside-down" property

2018-05-25 Thread Sakari Ailus
Use the "upside-down" property to tell that the sensor is mounted upside
down. This reverses the behaviour of the VFLIP and HFLIP controls as well
as the pixel order.

Signed-off-by: Sakari Ailus 
---
 .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
 drivers/media/i2c/smiapp/smiapp-core.c   | 16 
 2 files changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
index 33f10a94c381..6f509657470e 100644
--- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
+++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
@@ -29,6 +29,8 @@ Optional properties
 - reset-gpios: XSHUTDOWN GPIO
 - flash-leds: See ../video-interfaces.txt
 - lens-focus: See ../video-interfaces.txt
+- rotation: Integer property; valid values are 0 (sensor mounted upright)
+   and 180 (sensor mounted upside down).
 
 
 Endpoint node mandatory properties
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index e1f8208581aa..32286df6ab43 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2764,6 +2764,7 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct 
device *dev)
struct v4l2_fwnode_endpoint *bus_cfg;
struct fwnode_handle *ep;
struct fwnode_handle *fwnode = dev_fwnode(dev);
+   u32 rotation;
int i;
int rval;
 
@@ -2800,6 +2801,21 @@ static struct smiapp_hwconfig 
*smiapp_get_hwconfig(struct device *dev)
 
dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
+   rval = fwnode_property_read_u32(fwnode, "upside-down", );
+   if (!rval) {
+   switch (rotation) {
+   case 180:
+   hwcfg->module_board_orient =
+   SMIAPP_MODULE_BOARD_ORIENT_180;
+   /* Fall through */
+   case 0:
+   break;
+   default:
+   dev_err(dev, "invalid rotation %u\n", rotation);
+   goto out_err;
+   }
+   }
+
/* NVM size is not mandatory */
fwnode_property_read_u32(fwnode, "nokia,nvm-size", >nvm_size);
 
-- 
2.11.0



[PATCH v2 1/2] dt-bindings: media: Define "rotation" property for sensors

2018-05-25 Thread Sakari Ailus
Sensors are occasionally mounted upside down to systems such as mobile
phones or tablets. In order to use such a sensor without having to turn
every image upside down, most camera sensors support reversing the readout
order by setting both horizontal and vertical flipping.

This patch documents the "rotation" property for camera sensors, mirroring
what is defined for displays in
Documentation/devicetree/bindings/display/panel/panel.txt .

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 258b8dfddf48..52b7c7b57842 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -85,6 +85,10 @@ Optional properties
 
 - lens-focus: A phandle to the node of the focus lens controller.
 
+- rotation: The device, typically an image sensor, is not mounted upright,
+  but a number of degrees counter clockwise. Typical values are 0 and 180
+  (upside down).
+
 
 Optional endpoint properties
 
-- 
2.11.0



[PATCH v2 0/2] Define rotation property for camera sensors

2018-05-25 Thread Sakari Ailus
Hi,

Here's an update on my previous patchset adding an "upside-down" property.
Instead of dropping the first patch entirely as I first thought, I decided
to add documentation for the rotation property for sensors as well. The
updates to the patches are related to that.

Sakari Ailus (2):
  dt-bindings: media: Define "rotation" property for sensors
  smiapp: Support the "upside-down" property

 .../devicetree/bindings/media/i2c/nokia,smia.txt |  2 ++
 .../devicetree/bindings/media/video-interfaces.txt   |  4 
 drivers/media/i2c/smiapp/smiapp-core.c   | 16 
 3 files changed, 22 insertions(+)

-- 
2.11.0



Re: qcom: add firmware file for Venus on SDM845

2018-05-25 Thread Josh Boyer
On Fri, May 25, 2018 at 7:03 AM Vikash Garodia 
wrote:

> This pull request adds firmware files for Venus h/w codec found on the
Qualcomm SDM845 chipset.

> The following changes since commit
2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:

>Merge branch 'for-upstreaming-v1.7.2' of
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 -0400)

> are available in the git repository at:


>https://github.com/vgarodia/linux-firmware master

> for you to fetch changes up to d6088b9c9d7f49d3c6c43681190889eca0abdcce:

>qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)

> 
> Vikash Garodia (1):
>qcom: add venus firmware files for v5.2

>   WHENCE   |   9 +
>   qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
>   qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
>   qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
>   qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
>   qcom/venus-5.2/venus.b04 |   1 +
>   qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
>   qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
>   8 files changed, 10 insertions(+)
>   create mode 100644 qcom/venus-5.2/venus.b00
>   create mode 100644 qcom/venus-5.2/venus.b01
>   create mode 100644 qcom/venus-5.2/venus.b02
>   create mode 100644 qcom/venus-5.2/venus.b03
>   create mode 100644 qcom/venus-5.2/venus.b04
>   create mode 100644 qcom/venus-5.2/venus.mbn
>   create mode 100644 qcom/venus-5.2/venus.mdt

The venus.mbn file isn't mentioned in WHENCE:

[jwboyer@vader linux-firmware]$ ./check_whence.py
E: qcom/venus-5.2/venus.mbn not listed in WHENCE
[jwboyer@vader linux-firmware]$

Can you fix that up and let me know when to re-pull?

josh


Re: [PATCH v4 5/9] media: rcar-vin: Parse parallel input on Gen3

2018-05-25 Thread jacopo mondi
Hi Niklas,
   I might have another question before sending v5.

On Fri, May 25, 2018 at 12:29:44AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> I really like what you did with this patch in v4.
>
> On 2018-05-25 00:02:15 +0200, Jacopo Mondi wrote:
> > The rcar-vin driver so far had a mutually exclusive code path for
> > handling parallel and CSI-2 video input subdevices, with only the CSI-2
> > use case supporting media-controller. As we add support for parallel
> > inputs to Gen3 media-controller compliant code path now parse both port@0
> > and port@1, handling the media-controller use case in the parallel
> > bound/unbind notifier operations.
> >
> > Signed-off-by: Jacopo Mondi 
> >
> > ---
> > v3 -> v4:
> > - Change the mc/parallel initialization order. Initialize mc first, then
> >   parallel
> > - As a consequence no need to delay parallel notifiers registration, the
> >   media controller is set up already when parallel input got parsed,
> >   this greatly simplify the group notifier complete callback.
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 56 
> > ++---
> >  1 file changed, 35 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index a799684..29619c2 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct 
> > rvin_dev *vin,
> > ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> > vin->parallel->sink_pad = ret < 0 ? 0 : ret;
> >
> > +   if (vin->info->use_mc) {
> > +   vin->parallel->subdev = subdev;
> > +   return 0;
> > +   }
> > +
> > /* Find compatible subdevices mbus format */
> > vin->mbus_code = 0;
> > code.index = 0;
> > @@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct 
> > rvin_dev *vin,
> >  static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
> >  {
> > rvin_v4l2_unregister(vin);
> > -   v4l2_ctrl_handler_free(>ctrl_handler);
> > -
> > -   vin->vdev.ctrl_handler = NULL;
> > vin->parallel->subdev = NULL;
> > +
> > +   if (!vin->info->use_mc) {
> > +   v4l2_ctrl_handler_free(>ctrl_handler);
> > +   vin->vdev.ctrl_handler = NULL;
> > +   }
> >  }
> >
> >  static int rvin_parallel_notify_complete(struct v4l2_async_notifier 
> > *notifier)
> > @@ -552,18 +559,18 @@ static int rvin_parallel_parse_v4l2(struct device 
> > *dev,
> > return 0;
> >  }
> >
> > -static int rvin_parallel_graph_init(struct rvin_dev *vin)
> > +static int rvin_parallel_init(struct rvin_dev *vin)
> >  {
> > int ret;
> >
> > -   ret = v4l2_async_notifier_parse_fwnode_endpoints(
> > -   vin->dev, >notifier,
> > -   sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
> > +   ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> > +   vin->dev, >notifier, sizeof(struct rvin_parallel_entity),
> > +   0, rvin_parallel_parse_v4l2);
> > if (ret)
> > return ret;
> >
> > if (!vin->parallel)
> > -   return -ENODEV;
> > +   return -ENOTCONN;
>
> I think you still should return -ENODEV here if !vin->info->use_mc to
> preserve Gen2 which runs without media controller behavior. How about:
>
> return vin->info->use_mc ? -ENOTCONN : -ENODEV;
>
> >
> > vin_dbg(vin, "Found parallel subdevice %pOF\n",
> > to_of_node(vin->parallel->asd.match.fwnode));
> > @@ -784,14 +791,8 @@ static int rvin_mc_init(struct rvin_dev *vin)
> >  {
> > int ret;
> >
> > -   vin->pad.flags = MEDIA_PAD_FL_SINK;
> > -   ret = media_entity_pads_init(>vdev.entity, 1, >pad);
> > -   if (ret)
> > -   return ret;
> > -
> > -   ret = rvin_group_get(vin);
> > -   if (ret)
> > -   return ret;
> > +   if (!vin->info->use_mc)
> > +   return 0;
> >
> > ret = rvin_mc_parse_of_graph(vin);
> > if (ret)
> > @@ -1074,11 +1075,24 @@ static int rcar_vin_probe(struct platform_device 
> > *pdev)
> > return ret;
> >
> > platform_set_drvdata(pdev, vin);
> > -   if (vin->info->use_mc)
> > -   ret = rvin_mc_init(vin);
> > -   else
> > -   ret = rvin_parallel_graph_init(vin);
> > -   if (ret < 0)
> > +
> > +   if (vin->info->use_mc) {
> > +   vin->pad.flags = MEDIA_PAD_FL_SINK;
> > +   ret = media_entity_pads_init(>vdev.entity, 1, >pad);
> > +   if (ret)
> > +   return ret;
> > +
> > +   ret = rvin_group_get(vin);
> > +   if (ret)
> > +   return ret;
> > +   }
>
> I don't see why you need to move the media pad creation out of
> rvin_mc_init(). With the reorder of the rvin_mc_init()
> rvin_parallel_init() I would keep this in rvin_mc_init().
>
> > +
> > +   ret = rvin_mc_init(vin);
> > +   if (ret)
> > +   return ret;

qcom: add firmware file for Venus on SDM845

2018-05-25 Thread Vikash Garodia
This pull request adds firmware files for Venus h/w codec found on the Qualcomm 
SDM845 chipset.

The following changes since commit 2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:

  Merge branch 'for-upstreaming-v1.7.2' of 
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 -0400)

are available in the git repository at:


  https://github.com/vgarodia/linux-firmware master

for you to fetch changes up to d6088b9c9d7f49d3c6c43681190889eca0abdcce:

  qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)


Vikash Garodia (1):
  qcom: add venus firmware files for v5.2

 WHENCE   |   9 +
 qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
 qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
 qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
 qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
 qcom/venus-5.2/venus.b04 |   1 +
 qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
 qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
 8 files changed, 10 insertions(+)
 create mode 100644 qcom/venus-5.2/venus.b00
 create mode 100644 qcom/venus-5.2/venus.b01
 create mode 100644 qcom/venus-5.2/venus.b02
 create mode 100644 qcom/venus-5.2/venus.b03
 create mode 100644 qcom/venus-5.2/venus.b04
 create mode 100644 qcom/venus-5.2/venus.mbn
 create mode 100644 qcom/venus-5.2/venus.mdt


qcom: add firmware file for Venus on SDM845

2018-05-25 Thread Vikash Garodia
hi,

This pull request adds firmware files for Venus h/w codec found on the Qualcomm 
SDM845 chipset.

The following changes since commit 2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:

  Merge branch 'for-upstreaming-v1.7.2' of 
https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22 -0400)

are available in the git repository at:


  https://github.com/vgarodia/linux-firmware master

for you to fetch changes up to d6088b9c9d7f49d3c6c43681190889eca0abdcce:

  qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)


Vikash Garodia (1):
  qcom: add venus firmware files for v5.2

 WHENCE   |   9 +
 qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
 qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
 qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
 qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
 qcom/venus-5.2/venus.b04 |   1 +
 qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
 qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
 8 files changed, 10 insertions(+)
 create mode 100644 qcom/venus-5.2/venus.b00
 create mode 100644 qcom/venus-5.2/venus.b01
 create mode 100644 qcom/venus-5.2/venus.b02
 create mode 100644 qcom/venus-5.2/venus.b03
 create mode 100644 qcom/venus-5.2/venus.b04
 create mode 100644 qcom/venus-5.2/venus.mbn
 create mode 100644 qcom/venus-5.2/venus.mdt


Re: [PATCH v2 11/13] dmaengine: pxa: make the filter function internal

2018-05-25 Thread Vinod
On 24-05-18, 09:07, Robert Jarzmik wrote:
> As the pxa architecture and all its related drivers do not rely anymore
> on the filter function, thanks to the slave map conversion, make
> pxad_filter_fn() static, and remove it from the global namespace.

Acked-by: Vinod Koul 

-- 
~Vinod


Re: [PATCH v2 10/13] dmaengine: pxa: document pxad_param

2018-05-25 Thread Vinod
On 24-05-18, 09:07, Robert Jarzmik wrote:
> Add some documentation for the pxad_param structure, and describe the
> contract behind the minimal required priority of a DMA channel.

Acked-by: Vinod Koul 

-- 
~Vinod


Re: [PATCH v3] media: imx319: Add imx319 camera sensor driver

2018-05-25 Thread jacopo mondi
Hi Tomasz,

On Fri, May 25, 2018 at 04:31:07PM +0900, Tomasz Figa wrote:
> On Fri, May 25, 2018 at 4:12 PM jacopo mondi  wrote:
>
> > Hi Tomasz,
>

[snip]

> > > the controls set before powering on will actually be programmed into the
> > > hardware registers.
>
> > Thanks, I had missed that part.
>
> > I quickly tried searching for 's_ctrl' calls in the v4l2-core/ code
> > and I've found nothing that invokes that in response to a streaming
> > start operation. Just if you happen to have any reference handy, could
> > you please point me to that part, just for my better understanding?
>
> The driver does it itself by calling __v4l2_ctrl_handler_setup() from its
> .start_streaming() callback.

Thanks, I'm not sure how I've missed that part. Thanks and sorry for
the fuss!



signature.asc
Description: PGP signature


[GIT PULL FOR v4.18] Various fixes

2018-05-25 Thread Hans Verkuil
Hi Mauro,

This is the usual collection of random fixes/improvements.

A note on one patch "v4l2-core: push taking ioctl mutex down to ioctl handler.":

I would like to get this in for 4.18 since it will help the fence and request 
API
implementation. But it is also OK if you decide to push this to 4.19. In 
addition,
this patch will almost certainly conflict with the "v4l2-ioctl: delete unused
v4l2_disable_ioctl_locking" patch from the gspca pull request. It should be
trivial to resolve, though.

Regards,

Hans


The following changes since commit 8ed8bba70b4355b1ba029b151ade84475dd12991:

  media: imx274: remove non-indexed pointers from mode_table (2018-05-17 
06:22:08 -0400)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.18e

for you to fetch changes up to a0cd70c2ce3d48e5007632d2d58d3c776b87a6b2:

  gspca_zc3xx: Enable short exposure times for OV7648 (2018-05-25 11:37:04 
+0200)


Akinobu Mita (1):
  media: pxa_camera: avoid duplicate s_power calls

Colin Ian King (1):
  hdpvr: fix spelling mistake: "Hauppage" -> "Hauppauge"

Dan Carpenter (1):
  media: vivid: potential integer overflow in vidioc_g_edid()

Dmitry Osipenko (1):
  media: staging: tegra-vde: Reset memory client

Ezequiel Garcia (4):
  stk1160: Fix typo s/therwise/Otherwise
  stk1160: Add missing calls to mutex_destroy
  m2m-deinterlace: Remove DMA_ENGINE dependency
  tw686x: Fix incorrect vb2_mem_ops GFP flags

Geert Uytterhoeven (1):
  media: Remove depends on HAS_DMA in case of platform dependency

Gustavo A. R. Silva (1):
  au8522: remove duplicate code

Hans Verkuil (4):
  cec: fix wrong tx/rx_status values when canceling a msg
  adv7511: fix incorrect clear of CEC receive interrupt
  pvrusb2: replace pvr2_v4l2_ioctl by video_ioctl2
  v4l2-core: push taking ioctl mutex down to ioctl handler.

Mauro Carvalho Chehab (1):
  media: cec-pin-error-inj: avoid a false-positive Spectre detection

Ondrej Zary (3):
  gspca_zc3xx: Implement proper autogain and exposure control for OV7648
  gspca_zc3xx: Fix power line frequency settings for OV7648
  gspca_zc3xx: Enable short exposure times for OV7648

 drivers/media/cec/cec-adap.c| 19 +
 drivers/media/cec/cec-pin-error-inj.c   | 23 
 drivers/media/common/videobuf2/Kconfig  |  2 --
 drivers/media/dvb-frontends/au8522_decoder.c| 14 --
 drivers/media/i2c/adv7511.c |  4 +--
 drivers/media/pci/dt3155/Kconfig|  1 -
 drivers/media/pci/intel/ipu3/Kconfig|  1 -
 drivers/media/pci/solo6x10/Kconfig  |  1 -
 drivers/media/pci/sta2x11/Kconfig   |  1 -
 drivers/media/pci/tw5864/Kconfig|  1 -
 drivers/media/pci/tw686x/Kconfig|  1 -
 drivers/media/pci/tw686x/tw686x-video.c |  3 +-
 drivers/media/platform/Kconfig  | 45 
++
 drivers/media/platform/am437x/Kconfig   |  2 +-
 drivers/media/platform/atmel/Kconfig|  4 +--
 drivers/media/platform/davinci/Kconfig  |  6 
 drivers/media/platform/marvell-ccic/Kconfig |  2 --
 drivers/media/platform/pxa_camera.c | 17 
 drivers/media/platform/rcar-vin/Kconfig |  2 +-
 drivers/media/platform/soc_camera/Kconfig   |  3 +-
 drivers/media/platform/sti/c8sectpfe/Kconfig|  2 +-
 drivers/media/platform/vivid/vivid-vid-common.c |  2 +-
 drivers/media/usb/gspca/zc3xx.c | 58 
+++
 drivers/media/usb/hdpvr/hdpvr-i2c.c |  2 +-
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c| 83 
+---
 drivers/media/usb/stk1160/stk1160-core.c|  4 ++-
 drivers/media/v4l2-core/Kconfig |  2 --
 drivers/media/v4l2-core/v4l2-dev.c  |  6 
 drivers/media/v4l2-core/v4l2-ioctl.c| 20 --
 drivers/media/v4l2-core/v4l2-subdev.c   | 17 +++-
 drivers/staging/media/davinci_vpfe/Kconfig  |  1 -
 drivers/staging/media/omap4iss/Kconfig  |  1 -
 drivers/staging/media/tegra-vde/tegra-vde.c | 42 
+---
 include/media/v4l2-dev.h|  9 --
 include/media/v4l2-ioctl.h  | 12 
 35 files changed, 215 insertions(+), 198 deletions(-)


[RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-05-25 Thread bingbu . cao
From: Bingbu Cao 

Add a V4L2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the I2C bus to control the
focus position.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
 MAINTAINERS|   8 ++
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ak7375.c | 278 +
 4 files changed, 297 insertions(+)
 create mode 100644 drivers/media/i2c/ak7375.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea362219c4aa..20379a7baca0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git
 S: Maintained
 F: drivers/media/usb/airspy/
 
+AKM AK7375 LENS VOICE COIL DRIVER
+M: Tianshu Qiu 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ak7375.c
+F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
+
 ALACRITECH GIGABIT ETHERNET DRIVER
 M: Lino Sanfilippo 
 S: Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..ff3cb5afb0e1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,16 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_AK7375
+   tristate "AK7375 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   help
+ This is a driver for the AK7375 camera lens voice coil.
+ AK7375 is a 12 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..05b97e319ea9 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index ..012e673e9ced
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define AK7375_MAX_FOCUS_POS   4095
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define AK7375_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define AK7375_CTRL_STEPS  64
+#define AK7375_CTRL_DELAY_US   1000
+
+#define AK7375_REG_POSITION0x0
+#define AK7375_REG_CONT0x2
+#define AK7375_MODE_ACTIVE 0x0
+#define AK7375_MODE_STANDBY0x40
+
+/* ak7375 device structure */
+struct ak7375_device {
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   struct v4l2_ctrl *focus;
+};
+
+static inline struct ak7375_device *to_ak7375_vcm(struct v4l2_ctrl *ctrl)
+{
+   return container_of(ctrl->handler, struct ak7375_device, ctrls_vcm);
+}
+
+static inline struct ak7375_device *sd_to_ak7375_vcm(struct v4l2_subdev 
*subdev)
+{
+   return container_of(subdev, struct ak7375_device, sd);
+}
+
+static int ak7375_i2c_write(struct ak7375_device *ak7375,
+   u8 addr, u16 data, int size)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>sd);
+   int ret;
+   u8 buf[3];
+
+   if (size != 1 && size != 2)
+   return -EINVAL;
+   buf[0] = addr;
+   buf[2] = data & 0xff;
+   if (size == 2)
+   buf[1] = data >> 8;
+   ret = i2c_master_send(client, (const char *)buf, size + 1);
+   if (ret < 0)
+   return ret;
+   if (ret != size + 1)
+   return -EIO;
+   return 0;
+}
+
+static int ak7375_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct ak7375_device *dev_vcm = to_ak7375_vcm(ctrl);
+
+   if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+   return ak7375_i2c_write(dev_vcm, AK7375_REG_POSITION,
+   ctrl->val << 4, 2);
+
+   

[RESEND PATCH V2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

2018-05-25 Thread bingbu . cao
From: Bingbu Cao 

Add device tree bindings for AKM ak7375 voice coil lens
driver. This chip is used to drive a lens in a camera module.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
Changes since v1:
 - remove the vendor prefix change
---
 Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt 
b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
new file mode 100644
index ..aa3e24b41241
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
@@ -0,0 +1,8 @@
+Asahi Kasei Microdevices AK7375 voice coil lens driver
+
+AK7375 is a camera voice coil lens.
+
+Mandatory properties:
+
+- compatible: "asahi-kasei,ak7375"
+- reg: I2C slave address
-- 
1.9.1



[PATCH v2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

2018-05-25 Thread bingbu . cao
From: Bingbu Cao 

Add device tree bindings for AKM ak7375 voice coil lens
driver. This chip is used to drive a lens in a camera module.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
Changes since v1:
 - remove the vendor prefix change
 
Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt 
b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
new file mode 100644
index ..aa3e24b41241
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
@@ -0,0 +1,8 @@
+Asahi Kasei Microdevices AK7375 voice coil lens driver
+
+AK7375 is a camera voice coil lens.
+
+Mandatory properties:
+
+- compatible: "asahi-kasei,ak7375"
+- reg: I2C slave address
-- 
1.9.1



[PATCH v2 2/2] media: ak7375: Add ak7375 lens voice coil driver

2018-05-25 Thread bingbu . cao
From: Bingbu Cao 

Add a V4L2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the I2C bus to control the
focus position.

Signed-off-by: Tianshu Qiu 
Signed-off-by: Bingbu Cao 
---
 MAINTAINERS|   8 ++
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ak7375.c | 278 +
 4 files changed, 297 insertions(+)
 create mode 100644 drivers/media/i2c/ak7375.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea362219c4aa..20379a7baca0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git
 S: Maintained
 F: drivers/media/usb/airspy/
 
+AKM AK7375 LENS VOICE COIL DRIVER
+M: Tianshu Qiu 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ak7375.c
+F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
+
 ALACRITECH GIGABIT ETHERNET DRIVER
 M: Lino Sanfilippo 
 S: Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..ff3cb5afb0e1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,16 @@ config VIDEO_AD5820
  This is a driver for the AD5820 camera lens voice coil.
  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_AK7375
+   tristate "AK7375 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   help
+ This is a driver for the AK7375 camera lens voice coil.
+ AK7375 is a 12 bit DAC with 120mA output current sink
+ capability. This is designed for linear control of
+ voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9714
tristate "DW9714 lens voice coil support"
depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..05b97e319ea9 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index ..012e673e9ced
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define AK7375_MAX_FOCUS_POS   4095
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define AK7375_FOCUS_STEPS 1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define AK7375_CTRL_STEPS  64
+#define AK7375_CTRL_DELAY_US   1000
+
+#define AK7375_REG_POSITION0x0
+#define AK7375_REG_CONT0x2
+#define AK7375_MODE_ACTIVE 0x0
+#define AK7375_MODE_STANDBY0x40
+
+/* ak7375 device structure */
+struct ak7375_device {
+   struct v4l2_ctrl_handler ctrls_vcm;
+   struct v4l2_subdev sd;
+   struct v4l2_ctrl *focus;
+};
+
+static inline struct ak7375_device *to_ak7375_vcm(struct v4l2_ctrl *ctrl)
+{
+   return container_of(ctrl->handler, struct ak7375_device, ctrls_vcm);
+}
+
+static inline struct ak7375_device *sd_to_ak7375_vcm(struct v4l2_subdev 
*subdev)
+{
+   return container_of(subdev, struct ak7375_device, sd);
+}
+
+static int ak7375_i2c_write(struct ak7375_device *ak7375,
+   u8 addr, u16 data, int size)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>sd);
+   int ret;
+   u8 buf[3];
+
+   if (size != 1 && size != 2)
+   return -EINVAL;
+   buf[0] = addr;
+   buf[2] = data & 0xff;
+   if (size == 2)
+   buf[1] = data >> 8;
+   ret = i2c_master_send(client, (const char *)buf, size + 1);
+   if (ret < 0)
+   return ret;
+   if (ret != size + 1)
+   return -EIO;
+   return 0;
+}
+
+static int ak7375_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct ak7375_device *dev_vcm = to_ak7375_vcm(ctrl);
+
+   if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+   return ak7375_i2c_write(dev_vcm, AK7375_REG_POSITION,
+   ctrl->val << 4, 2);
+
+   

[PATCH 3/3 v2] gspca_zc3xx: Enable short exposure times for OV7648

2018-05-25 Thread Ondrej Zary
The 50Hz and 60Hz power line frequency settings disable short (1/120s
and 1/100s) exposure times for banding filter (causing overexposed
image near lamps). No flicker setting enables them (when banding
filter is disabled and they're not used).

Seems that the logic is just the wrong way around.
(This bug came from the Windows driver.)

Signed-off-by: Ondrej Zary 
---
 drivers/media/usb/gspca/zc3xx.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c
index c72f2d9167d9..cf21991e3d99 100644
--- a/drivers/media/usb/gspca/zc3xx.c
+++ b/drivers/media/usb/gspca/zc3xx.c
@@ -3186,7 +3186,8 @@ static const struct usb_action ov7620_InitialScale[] = {  
/* 320x240 */
 static const struct usb_action ov7620_50HZ[] = {
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x0096},   /* 00,2b,96,aa */
-   {0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */
+   /* enable 1/120s & 1/100s exposures for banding filter */
+   {0xaa, 0x75, 0x008e},
{0xaa, 0x2d, 0x0005},   /* 00,2d,05,aa */
{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH}, /* 01,90,00,cc */
{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID},  /* 01,91,04,cc */
@@ -3202,7 +3203,8 @@ static const struct usb_action ov7620_50HZ[] = {
 static const struct usb_action ov7620_60HZ[] = {
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
-   {0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */
+   /* enable 1/120s & 1/100s exposures for banding filter */
+   {0xaa, 0x75, 0x008e},
{0xaa, 0x2d, 0x0005},   /* 00,2d,05,aa */
{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH}, /* 01,90,00,cc */
{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID}, /* 01,91,04,cc */
@@ -3221,7 +3223,8 @@ static const struct usb_action ov7620_60HZ[] = {
 static const struct usb_action ov7620_NoFliker[] = {
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
-   {0xaa, 0x75, 0x008e},   /* 00,75,8e,aa */
+   /* disable 1/120s & 1/100s exposures for banding filter */
+   {0xaa, 0x75, 0x008a},
{0xaa, 0x2d, 0x0001},   /* 00,2d,01,aa */
{0xa0, 0x00, ZC3XX_R190_EXPOSURELIMITHIGH}, /* 01,90,00,cc */
{0xa0, 0x04, ZC3XX_R191_EXPOSURELIMITMID}, /* 01,91,04,cc */
-- 
Ondrej Zary



[PATCH 2/3 v2] gspca_zc3xx: Fix power line frequency settings for OV7648

2018-05-25 Thread Ondrej Zary
Power line frequency settings for OV7648 sensor contain autogain
and exposure commands, affecting unrelated controls. Remove them.

Signed-off-by: Ondrej Zary 
---
 drivers/media/usb/gspca/zc3xx.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c
index 992918b3ad0c..c72f2d9167d9 100644
--- a/drivers/media/usb/gspca/zc3xx.c
+++ b/drivers/media/usb/gspca/zc3xx.c
@@ -3184,7 +3184,6 @@ static const struct usb_action ov7620_InitialScale[] = {  
/* 320x240 */
{}
 };
 static const struct usb_action ov7620_50HZ[] = {
-   {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x0096},   /* 00,2b,96,aa */
{0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */
@@ -3195,15 +3194,12 @@ static const struct usb_action ov7620_50HZ[] = {
{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH},   /* 01,95,00,cc */
{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID},/* 01,96,00,cc */
{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW},/* 01,97,83,cc */
-   {0xaa, 0x10, 0x0082},   /* 00,10,82,aa */
{0xaa, 0x76, 0x0003},   /* 00,76,03,aa */
 /* {0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},* 00,02,40,cc
 * if mode0 (640x480) */
{}
 };
 static const struct usb_action ov7620_60HZ[] = {
-   {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
-   /* (bug in zs211.inf) */
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
{0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */
@@ -3214,7 +3210,6 @@ static const struct usb_action ov7620_60HZ[] = {
{0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH}, /* 01,95,00,cc */
{0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID}, /* 01,96,00,cc */
{0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW}, /* 01,97,83,cc */
-   {0xaa, 0x10, 0x0020},   /* 00,10,20,aa */
{0xaa, 0x76, 0x0003},   /* 00,76,03,aa */
 /* {0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},* 00,02,40,cc
 * if mode0 (640x480) */
@@ -3224,8 +3219,6 @@ static const struct usb_action ov7620_60HZ[] = {
{}
 };
 static const struct usb_action ov7620_NoFliker[] = {
-   {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
-   /* (bug in zs211.inf) */
{0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
{0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
{0xaa, 0x75, 0x008e},   /* 00,75,8e,aa */
-- 
Ondrej Zary



[PATCH 1/3] gspca_zc3xx: Implement proper autogain and exposure control for OV7648

2018-05-25 Thread Ondrej Zary
The ZS0211 internal autogain causes pumping and flickering with OV7648
sensor on 0ac8:307b webcam.
Implement OV7648 autogain and exposure control and use that instead.

Signed-off-by: Ondrej Zary 
---
 drivers/media/usb/gspca/zc3xx.c | 42 +
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c
index 25b4dbe8e049..992918b3ad0c 100644
--- a/drivers/media/usb/gspca/zc3xx.c
+++ b/drivers/media/usb/gspca/zc3xx.c
@@ -5778,16 +5778,34 @@ static void setcontrast(struct gspca_dev *gspca_dev,
 
 static s32 getexposure(struct gspca_dev *gspca_dev)
 {
-   return (i2c_read(gspca_dev, 0x25) << 9)
-   | (i2c_read(gspca_dev, 0x26) << 1)
-   | (i2c_read(gspca_dev, 0x27) >> 7);
+   struct sd *sd = (struct sd *) gspca_dev;
+
+   switch (sd->sensor) {
+   case SENSOR_HV7131R:
+   return (i2c_read(gspca_dev, 0x25) << 9)
+   | (i2c_read(gspca_dev, 0x26) << 1)
+   | (i2c_read(gspca_dev, 0x27) >> 7);
+   case SENSOR_OV7620:
+   return i2c_read(gspca_dev, 0x10);
+   default:
+   return -1;
+   }
 }
 
 static void setexposure(struct gspca_dev *gspca_dev, s32 val)
 {
-   i2c_write(gspca_dev, 0x25, val >> 9, 0x00);
-   i2c_write(gspca_dev, 0x26, val >> 1, 0x00);
-   i2c_write(gspca_dev, 0x27, val << 7, 0x00);
+   struct sd *sd = (struct sd *) gspca_dev;
+
+   switch (sd->sensor) {
+   case SENSOR_HV7131R:
+   i2c_write(gspca_dev, 0x25, val >> 9, 0x00);
+   i2c_write(gspca_dev, 0x26, val >> 1, 0x00);
+   i2c_write(gspca_dev, 0x27, val << 7, 0x00);
+   break;
+   case SENSOR_OV7620:
+   i2c_write(gspca_dev, 0x10, val, 0x00);
+   break;
+   }
 }
 
 static void setquality(struct gspca_dev *gspca_dev)
@@ -5918,7 +5936,12 @@ static void setlightfreq(struct gspca_dev *gspca_dev, 
s32 val)
 
 static void setautogain(struct gspca_dev *gspca_dev, s32 val)
 {
-   reg_w(gspca_dev, val ? 0x42 : 0x02, 0x0180);
+   struct sd *sd = (struct sd *) gspca_dev;
+
+   if (sd->sensor == SENSOR_OV7620)
+   i2c_write(gspca_dev, 0x13, val ? 0xa3 : 0x80, 0x00);
+   else
+   reg_w(gspca_dev, val ? 0x42 : 0x02, 0x0180);
 }
 
 /*
@@ -6439,6 +6462,9 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
if (sd->sensor == SENSOR_HV7131R)
sd->exposure = v4l2_ctrl_new_std(hdl, _ctrl_ops,
V4L2_CID_EXPOSURE, 0x30d, 0x493e, 1, 0x927);
+   else if (sd->sensor == SENSOR_OV7620)
+   sd->exposure = v4l2_ctrl_new_std(hdl, _ctrl_ops,
+   V4L2_CID_EXPOSURE, 0, 255, 1, 0x41);
sd->autogain = v4l2_ctrl_new_std(hdl, _ctrl_ops,
V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
if (sd->sensor != SENSOR_OV7630C)
@@ -6458,7 +6484,7 @@ static int sd_init_controls(struct gspca_dev *gspca_dev)
return hdl->error;
}
v4l2_ctrl_cluster(3, >gamma);
-   if (sd->sensor == SENSOR_HV7131R)
+   if (sd->sensor == SENSOR_HV7131R || sd->sensor == SENSOR_OV7620)
v4l2_ctrl_auto_cluster(2, >autogain, 0, true);
return 0;
 }
-- 
Ondrej Zary



Re: [PATCH] media: v4l2-ctrl: Add control for VP9 profile

2018-05-25 Thread Hans Verkuil
On 17/05/18 11:53, Keiichi Watanabe wrote:
> Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired
> profile for VP9 encoder and querying for supported profiles by VP9 encoder
> or decoder.
> 
> An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be
> used for querying since it is not a menu control but an integer
> control, which cannot return an arbitrary set of supported profiles.
> 
> The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as
> with controls for other codec profiles. (e.g. H264)

I don't mind adding this control (although I would like to have an Ack from
Sylwester), but we also need this to be used in an actual kernel driver.

Otherwise we're adding a control that nobody uses.

Regards,

Hans

> 
> Signed-off-by: Keiichi Watanabe 
> ---
> 
>  .../media/uapi/v4l/extended-controls.rst  | 26 +++
>  drivers/media/v4l2-core/v4l2-ctrls.c  | 12 +
>  include/uapi/linux/v4l2-controls.h|  8 ++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index 03931f9b1285..4f7f128a4998 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1959,6 +1959,32 @@ enum v4l2_vp8_golden_frame_sel -
>  Select the desired profile for VPx encoder. Acceptable values are 0,
>  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>  
> +.. _v4l2-mpeg-video-vp9-profile:
> +
> +``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> +(enum)
> +
> +enum v4l2_mpeg_video_vp9_profile -
> +This control allows to select the profile for VP9 encoder.
> +This is also used to enumerate supported profiles by VP9 encoder or 
> decoder.
> +Possible values are:
> +
> +
> +
> +.. flat-table::
> +:header-rows:  0
> +:stub-columns: 0
> +
> +* - ``V4L2_MPEG_VIDEO_VP9_PROFILE_0``
> +  - Profile 0
> +* - ``V4L2_MPEG_VIDEO_VP9_PROFILE_1``
> +  - Profile 1
> +* - ``V4L2_MPEG_VIDEO_VP9_PROFILE_2``
> +  - Profile 2
> +* - ``V4L2_MPEG_VIDEO_VP9_PROFILE_3``
> +  - Profile 3
> +
> +
>  
>  High Efficiency Video Coding (HEVC/H.265) Control Reference
>  ---
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d29e45516eb7..401ce21c2e63 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -431,6 +431,13 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   "Use Previous Specific Frame",
>   NULL,
>   };
> + static const char * const vp9_profile[] = {
> + "0",
> + "1",
> + "2",
> + "3",
> + NULL,
> + };
>  
>   static const char * const flash_led_mode[] = {
>   "Off",
> @@ -614,6 +621,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   return mpeg4_profile;
>   case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
>   return vpx_golden_frame_sel;
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
> + return vp9_profile;
>   case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>   return jpeg_chroma_subsampling;
>   case V4L2_CID_DV_TX_MODE:
> @@ -841,6 +850,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> P-Frame QP Value";
>   case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> Profile";
>  
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:   return "VP9 
> Profile";
> +
>   /* HEVC controls */
>   case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:   return "HEVC 
> I-Frame QP Value";
>   case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP:   return "HEVC 
> P-Frame QP Value";
> @@ -1180,6 +1191,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   case V4L2_CID_DEINTERLACING_MODE:
>   case V4L2_CID_TUNE_DEEMPHASIS:
>   case V4L2_CID_MPEG_VIDEO_VPX_GOLDEN_FRAME_SEL:
> + case V4L2_CID_MPEG_VIDEO_VP9_PROFILE:
>   case V4L2_CID_DETECT_MD_MODE:
>   case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
>   case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index 8d473c979b61..56203b7b715c 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -589,6 +589,14 @@ enum v4l2_vp8_golden_frame_sel {
>  #define V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP   (V4L2_CID_MPEG_BASE+510)
>  #define V4L2_CID_MPEG_VIDEO_VPX_PROFILE  
> (V4L2_CID_MPEG_BASE+511)
>  
> +#define V4L2_CID_MPEG_VIDEO_VP9_PROFILE  
> (V4L2_CID_MPEG_BASE+512)
> +enum v4l2_mpeg_video_vp9_profile {
> + 

Re: [PATCH] media: v4l2-ioctl: prevent underflow in v4l_enumoutput()

2018-05-25 Thread Hans Verkuil
On 17/05/18 11:05, Dan Carpenter wrote:
> My Smatch allmodconfig build only detects one function implementing
> vpbe_device_ops->enum_outputs and that's vpbe_enum_outputs().  The
> problem really happens in that function when we do:
> 
>   int temp_index = output->index;
> 
>   if (temp_index >= cfg->num_outputs)
>   return -EINVAL;
> 
> Unfortunately, both temp_index and cfg->num_outputs are type int so we
> have a potential read before the start of the array if "temp_index" is
> negative.

Why not fix it in this driver? Make num_outputs unsigned, as it should be.

I really don't like having a random index check in the core. If we ever
want to do such things in the core, then it needs to be implemented
consistently for all ioctls that do something similar.

Regards,

Hans

> 
> I could have fixed the bug in that function but it's more secure and
> future proof to block that bug earlier in a central place.  There is no
> one who need p->index to be more than INT_MAX.
> 
> Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index a40dbec271f1..115757ab8bc0 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1099,6 +1099,9 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops 
> *ops,
>   if (is_valid_ioctl(vfd, VIDIOC_S_STD))
>   p->capabilities |= V4L2_OUT_CAP_STD;
>  
> + if (p->index > INT_MAX)
> + return -EINVAL;
> +
>   return ops->vidioc_enum_output(file, fh, p);
>  }
>  
> 



Re: [PATCH v2 08/13] ASoC: pxa: remove the dmaengine compat need

2018-05-25 Thread Daniel Mack

On Thursday, May 24, 2018 09:06 AM, Robert Jarzmik wrote:

As the pxa architecture switched towards the dmaengine slave map, the
old compatibility mechanism to acquire the dma requestor line number and
priority are not needed anymore.

This patch simplifies the dma resource acquisition, using the more
generic function dma_request_slave_channel().

Signed-off-by: Robert Jarzmik 


Reviewed-by: Daniel Mack 


---
  sound/arm/pxa2xx-ac97.c | 14 ++
  sound/arm/pxa2xx-pcm-lib.c  |  6 +++---
  sound/soc/pxa/pxa2xx-ac97.c | 32 +---
  sound/soc/pxa/pxa2xx-i2s.c  |  6 ++
  4 files changed, 12 insertions(+), 46 deletions(-)

diff --git a/sound/arm/pxa2xx-ac97.c b/sound/arm/pxa2xx-ac97.c
index 4bc244c40f80..236a63cdaf9f 100644
--- a/sound/arm/pxa2xx-ac97.c
+++ b/sound/arm/pxa2xx-ac97.c
@@ -63,28 +63,18 @@ static struct snd_ac97_bus_ops pxa2xx_ac97_ops = {
.reset  = pxa2xx_ac97_legacy_reset,
  };
  
-static struct pxad_param pxa2xx_ac97_pcm_out_req = {

-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 12,
-};
-
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_out = {
.addr   = __PREG(PCDR),
.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+   .chan_name  = "pcm_pcm_stereo_out",
.maxburst   = 32,
-   .filter_data= _ac97_pcm_out_req,
-};
-
-static struct pxad_param pxa2xx_ac97_pcm_in_req = {
-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 11,
  };
  
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_in = {

.addr   = __PREG(PCDR),
.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+   .chan_name  = "pcm_pcm_stereo_in",
.maxburst   = 32,
-   .filter_data= _ac97_pcm_in_req,
  };
  
  static struct snd_pcm *pxa2xx_ac97_pcm;

diff --git a/sound/arm/pxa2xx-pcm-lib.c b/sound/arm/pxa2xx-pcm-lib.c
index e8da3b8ee721..dcbe7ecc1835 100644
--- a/sound/arm/pxa2xx-pcm-lib.c
+++ b/sound/arm/pxa2xx-pcm-lib.c
@@ -125,9 +125,9 @@ int __pxa2xx_pcm_open(struct snd_pcm_substream *substream)
if (ret < 0)
return ret;
  
-	return snd_dmaengine_pcm_open_request_chan(substream,

-   pxad_filter_fn,
-   dma_params->filter_data);
+   return snd_dmaengine_pcm_open(
+   substream, dma_request_slave_channel(rtd->cpu_dai->dev,
+dma_params->chan_name));
  }
  EXPORT_SYMBOL(__pxa2xx_pcm_open);
  
diff --git a/sound/soc/pxa/pxa2xx-ac97.c b/sound/soc/pxa/pxa2xx-ac97.c

index 803818aabee9..1b41c0f2a8fb 100644
--- a/sound/soc/pxa/pxa2xx-ac97.c
+++ b/sound/soc/pxa/pxa2xx-ac97.c
@@ -68,61 +68,39 @@ static struct snd_ac97_bus_ops pxa2xx_ac97_ops = {
.reset  = pxa2xx_ac97_cold_reset,
  };
  
-static struct pxad_param pxa2xx_ac97_pcm_stereo_in_req = {

-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 11,
-};
-
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_stereo_in = {
.addr   = __PREG(PCDR),
.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+   .chan_name  = "pcm_pcm_stereo_in",
.maxburst   = 32,
-   .filter_data= _ac97_pcm_stereo_in_req,
-};
-
-static struct pxad_param pxa2xx_ac97_pcm_stereo_out_req = {
-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 12,
  };
  
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_stereo_out = {

.addr   = __PREG(PCDR),
.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES,
+   .chan_name  = "pcm_pcm_stereo_out",
.maxburst   = 32,
-   .filter_data= _ac97_pcm_stereo_out_req,
  };
  
-static struct pxad_param pxa2xx_ac97_pcm_aux_mono_out_req = {

-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 10,
-};
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_aux_mono_out = {
.addr   = __PREG(MODR),
.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES,
+   .chan_name  = "pcm_aux_mono_out",
.maxburst   = 16,
-   .filter_data= _ac97_pcm_aux_mono_out_req,
  };
  
-static struct pxad_param pxa2xx_ac97_pcm_aux_mono_in_req = {

-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 9,
-};
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_aux_mono_in = {
.addr   = __PREG(MODR),
.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES,
+   .chan_name  = "pcm_aux_mono_in",
.maxburst   = 16,
-   .filter_data= _ac97_pcm_aux_mono_in_req,
  };
  
-static struct pxad_param pxa2xx_ac97_pcm_aux_mic_mono_req = {

-   .prio = PXAD_PRIO_LOWEST,
-   .drcmr = 8,
-};
  static struct snd_dmaengine_dai_dma_data pxa2xx_ac97_pcm_mic_mono_in = {
.addr   = __PREG(MCDR),
.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES,
+   .chan_name  = "pcm_aux_mic_mono",
.maxburst   = 16,
-   .filter_data= 

[GIT PULL FOR v4.18] gspca: convert to vb2

2018-05-25 Thread Hans Verkuil
This patch series converts gspca to vb2. It also fixes a vb2 bug found
while testing this, and it zeroes some fields for g/s_parm (they were
never tested in v4l2-compliance, so nobody noticed before).

Finally v4l2_disable_ioctl_locking() can now be removed since gspca no
longer needs it.

Tested with my (very large, thanks to Hans de Goede!) collection of gspca
webcams.

Regards,

Hans

The following changes since commit 8ed8bba70b4355b1ba029b151ade84475dd12991:

  media: imx274: remove non-indexed pointers from mode_table (2018-05-17 
06:22:08 -0400)

are available in the Git repository at:

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

for you to fetch changes up to 0946167e1f90a78928612c95cafad1cdf0868a15:

  gspca: Kill all URBs before releasing any of them (2018-05-25 10:10:43 +0200)


Ezequiel Garcia (1):
  gspca: Kill all URBs before releasing any of them

Hans Verkuil (5):
  videobuf2-core: don't call memop 'finish' when queueing
  gspca: convert to vb2
  v4l2-ioctl: clear fields in s_parm
  v4l2-ioctl: delete unused v4l2_disable_ioctl_locking
  gspca: fix g/s_parm handling

 drivers/media/common/videobuf2/videobuf2-core.c |   9 +-
 drivers/media/usb/gspca/Kconfig |   1 +
 drivers/media/usb/gspca/gspca.c | 946 
+++
 drivers/media/usb/gspca/gspca.h |  38 +--
 drivers/media/usb/gspca/m5602/m5602_core.c  |   4 +-
 drivers/media/usb/gspca/ov534.c |   1 -
 drivers/media/usb/gspca/topro.c |   1 -
 drivers/media/usb/gspca/vc032x.c|   2 +-
 drivers/media/v4l2-core/v4l2-ioctl.c|  19 +-
 include/media/v4l2-dev.h|  15 -
 10 files changed, 229 insertions(+), 807 deletions(-)


Re: [PATCH 2/3] gspca_zc3xx: Fix power line frequency settings for OV7648

2018-05-25 Thread Hans Verkuil
On 24/05/18 17:09, Ondrej Zary wrote:
> Power line frequency settings for OV7648 sensor contain autogain
> and exposure commands, affecting unrelated controls. Remove them.
> 
> Signed-off-by: Ondrej Zary 
> ---
>  drivers/media/usb/gspca/zc3xx.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/usb/gspca/zc3xx.c b/drivers/media/usb/gspca/zc3xx.c
> index 992918b3ad0c..9a78420e8ad8 100644
> --- a/drivers/media/usb/gspca/zc3xx.c
> +++ b/drivers/media/usb/gspca/zc3xx.c
> @@ -3184,7 +3184,8 @@ static const struct usb_action ov7620_InitialScale[] = 
> {/* 320x240 */
>   {}
>  };
>  static const struct usb_action ov7620_50HZ[] = {
> - {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
> +/*   {0xaa, 0x13, 0x00a3},* 00,13,a3,aa
> +  * don't change autoexposure */
>   {0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
>   {0xaa, 0x2b, 0x0096},   /* 00,2b,96,aa */
>   {0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */

Just remove these lines altogether. There are still present in the git history
if they are ever needed again. Same for the next patch.

Regards,

Hans

> @@ -3195,15 +3196,16 @@ static const struct usb_action ov7620_50HZ[] = {
>   {0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH},   /* 01,95,00,cc */
>   {0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID},/* 01,96,00,cc */
>   {0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW},/* 01,97,83,cc */
> - {0xaa, 0x10, 0x0082},   /* 00,10,82,aa */
> +/*   {0xaa, 0x10, 0x0082},* 00,10,82,aa
> +  * don't change exposure */
>   {0xaa, 0x76, 0x0003},   /* 00,76,03,aa */
>  /*   {0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},* 00,02,40,cc
>* if mode0 (640x480) */
>   {}
>  };
>  static const struct usb_action ov7620_60HZ[] = {
> - {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
> - /* (bug in zs211.inf) */
> +/*   {0xaa, 0x13, 0x00a3},* 00,13,a3,aa
> +  * don't change autoexposure */
>   {0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
>   {0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
>   {0xaa, 0x75, 0x008a},   /* 00,75,8a,aa */
> @@ -3214,7 +3216,8 @@ static const struct usb_action ov7620_60HZ[] = {
>   {0xa0, 0x00, ZC3XX_R195_ANTIFLICKERHIGH}, /* 01,95,00,cc */
>   {0xa0, 0x00, ZC3XX_R196_ANTIFLICKERMID}, /* 01,96,00,cc */
>   {0xa0, 0x83, ZC3XX_R197_ANTIFLICKERLOW}, /* 01,97,83,cc */
> - {0xaa, 0x10, 0x0020},   /* 00,10,20,aa */
> +/*   {0xaa, 0x10, 0x0020},* 00,10,20,aa
> +  * don't change exposure */
>   {0xaa, 0x76, 0x0003},   /* 00,76,03,aa */
>  /*   {0xa0, 0x40, ZC3XX_R002_CLOCKSELECT},* 00,02,40,cc
>* if mode0 (640x480) */
> @@ -3224,8 +3227,8 @@ static const struct usb_action ov7620_60HZ[] = {
>   {}
>  };
>  static const struct usb_action ov7620_NoFliker[] = {
> - {0xaa, 0x13, 0x00a3},   /* 00,13,a3,aa */
> - /* (bug in zs211.inf) */
> +/*   {0xaa, 0x13, 0x00a3},* 00,13,a3,aa
> +  * don't change autoexposure */
>   {0xdd, 0x00, 0x0100},   /* 00,01,00,dd */
>   {0xaa, 0x2b, 0x},   /* 00,2b,00,aa */
>   {0xaa, 0x75, 0x008e},   /* 00,75,8e,aa */
> 



Re: [PATCH v2 13/13] ARM: pxa: change SSP DMA channels allocation

2018-05-25 Thread Daniel Mack

On Thursday, May 24, 2018 09:07 AM, Robert Jarzmik wrote:

Now the dma_slave_map is available for PXA architecture, switch the SSP
device to it.

This specifically means that :
- for platform data based machines, the DMA requestor channels are
   extracted from the slave map, where pxa-ssp-dai. is a 1-1 match to
   ssp., and the channels are either "rx" or "tx".

- for device tree platforms, the dma node should be hooked into the
   pxa2xx-ac97 or pxa-ssp-dai node.

Signed-off-by: Robert Jarzmik 


Acked-by: Daniel Mack 


We should, however, merge what's left of this management glue code into 
the users of it, so the dma related properties can be put in the right 
devicetree node.


I'll prepare a patch for that for 4.18. This is a good preparation for 
this round though.



Thanks,
Daniel



---
Since v1: Removed channel names from platform_data
---
  arch/arm/plat-pxa/ssp.c| 47 --
  include/linux/pxa2xx_ssp.h |  2 --
  sound/soc/pxa/pxa-ssp.c|  5 ++---
  3 files changed, 2 insertions(+), 52 deletions(-)

diff --git a/arch/arm/plat-pxa/ssp.c b/arch/arm/plat-pxa/ssp.c
index ba13f793fbce..ed36dcab80f1 100644
--- a/arch/arm/plat-pxa/ssp.c
+++ b/arch/arm/plat-pxa/ssp.c
@@ -127,53 +127,6 @@ static int pxa_ssp_probe(struct platform_device *pdev)
if (IS_ERR(ssp->clk))
return PTR_ERR(ssp->clk);
  
-	if (dev->of_node) {

-   struct of_phandle_args dma_spec;
-   struct device_node *np = dev->of_node;
-   int ret;
-
-   /*
-* FIXME: we should allocate the DMA channel from this
-* context and pass the channel down to the ssp users.
-* For now, we lookup the rx and tx indices manually
-*/
-
-   /* rx */
-   ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells",
-0, _spec);
-
-   if (ret) {
-   dev_err(dev, "Can't parse dmas property\n");
-   return -ENODEV;
-   }
-   ssp->drcmr_rx = dma_spec.args[0];
-   of_node_put(dma_spec.np);
-
-   /* tx */
-   ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells",
-1, _spec);
-   if (ret) {
-   dev_err(dev, "Can't parse dmas property\n");
-   return -ENODEV;
-   }
-   ssp->drcmr_tx = dma_spec.args[0];
-   of_node_put(dma_spec.np);
-   } else {
-   res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-   if (res == NULL) {
-   dev_err(dev, "no SSP RX DRCMR defined\n");
-   return -ENODEV;
-   }
-   ssp->drcmr_rx = res->start;
-
-   res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-   if (res == NULL) {
-   dev_err(dev, "no SSP TX DRCMR defined\n");
-   return -ENODEV;
-   }
-   ssp->drcmr_tx = res->start;
-   }
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res == NULL) {
dev_err(dev, "no memory resource defined\n");
diff --git a/include/linux/pxa2xx_ssp.h b/include/linux/pxa2xx_ssp.h
index 8461b18e4608..03a7ca46735b 100644
--- a/include/linux/pxa2xx_ssp.h
+++ b/include/linux/pxa2xx_ssp.h
@@ -212,8 +212,6 @@ struct ssp_device {
int type;
int use_count;
int irq;
-   int drcmr_rx;
-   int drcmr_tx;
  
  	struct device_node	*of_node;

  };
diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
index 0291c7cb64eb..e09368d89bbc 100644
--- a/sound/soc/pxa/pxa-ssp.c
+++ b/sound/soc/pxa/pxa-ssp.c
@@ -104,9 +104,8 @@ static int pxa_ssp_startup(struct snd_pcm_substream 
*substream,
dma = kzalloc(sizeof(struct snd_dmaengine_dai_dma_data), GFP_KERNEL);
if (!dma)
return -ENOMEM;
-
-   dma->filter_data = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
-   >drcmr_tx : >drcmr_rx;
+   dma->chan_name = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
+   "tx" : "rx";
  
  	snd_soc_dai_set_dma_data(cpu_dai, substream, dma);
  





Re: [PATCH v3] media: imx319: Add imx319 camera sensor driver

2018-05-25 Thread Tomasz Figa
On Fri, May 25, 2018 at 4:12 PM jacopo mondi  wrote:

> Hi Tomasz,

> On Fri, May 25, 2018 at 03:18:38PM +0900, Tomasz Figa wrote:
> > On Fri, May 25, 2018 at 5:47 AM Sakari Ailus <
sakari.ai...@linux.intel.com>
> > wrote:
> >
> > > Hi Jacopo,
> >
> > > On Thu, May 24, 2018 at 10:07:38PM +0200, jacopo mondi wrote:
> > > ...
> > > > > > about that, but I wonder why setting controls should be enabled
only
> > > > > > when streaming. I would have expected runtime_pm_get/put in
> > subdevices
> > > > > > node open/close functions not only when streaming. Am I missing
> > something?
> > > > >
> > > > > You can do it either way. If powering on the sensor takes a long
> > time, then
> > > > > doing that in the open callback may be helpful as the user space
has
> > a way
> > > > > to keep the device powered.
> > > >
> > > > Ok, so I assume my comment could be ignored, assuming is fine not
> > > > being able to set control if the sensor is not streaming. Is it?
> >
> > > I'd say so. From the user's point of view, the sensor doesn't really
do
> > > anything when it's in software standby mode.
> >
> > Just to make sure we're on the same page, the code actually does nothing
> > when the sensor is not in streaming mode (i.e. powered off). When
stream is
> > being started, the V4L2 control framework will call s_ctrl for all the
> > controls in the handler to synchronize hardware state and this is when
all
> > the controls set before powering on will actually be programmed into the
> > hardware registers.

> Thanks, I had missed that part.

> I quickly tried searching for 's_ctrl' calls in the v4l2-core/ code
> and I've found nothing that invokes that in response to a streaming
> start operation. Just if you happen to have any reference handy, could
> you please point me to that part, just for my better understanding?

The driver does it itself by calling __v4l2_ctrl_handler_setup() from its
.start_streaming() callback.

Best regards,
Tomasz


Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Krzysztof Hałasa
Philipp Zabel  writes:

> Maybe scanline interlave and double write reduction can't be used at the
> same time?

Well, if it works in non-interlaced modes - it may be the case.

Perhaps the data reduction is done before the field merge step. This
would make it incompatible: in interlaced mode we need all color data
from a field (we could potentially remove all color info from the other
field, or use some average).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland


Re: [PATCH v4 5/9] media: rcar-vin: Parse parallel input on Gen3

2018-05-25 Thread jacopo mondi
Hi Niklas,

On Fri, May 25, 2018 at 12:29:44AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> I really like what you did with this patch in v4.

Thanks for review and suggestions, what's there comes mostly from your
comments and guidance.

>
> On 2018-05-25 00:02:15 +0200, Jacopo Mondi wrote:
> > The rcar-vin driver so far had a mutually exclusive code path for
> > handling parallel and CSI-2 video input subdevices, with only the CSI-2
> > use case supporting media-controller. As we add support for parallel
> > inputs to Gen3 media-controller compliant code path now parse both port@0
> > and port@1, handling the media-controller use case in the parallel
> > bound/unbind notifier operations.
> >
> > Signed-off-by: Jacopo Mondi 
> >
> > ---
> > v3 -> v4:
> > - Change the mc/parallel initialization order. Initialize mc first, then
> >   parallel
> > - As a consequence no need to delay parallel notifiers registration, the
> >   media controller is set up already when parallel input got parsed,
> >   this greatly simplify the group notifier complete callback.
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 56 
> > ++---
> >  1 file changed, 35 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index a799684..29619c2 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct 
> > rvin_dev *vin,
> > ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> > vin->parallel->sink_pad = ret < 0 ? 0 : ret;
> >
> > +   if (vin->info->use_mc) {
> > +   vin->parallel->subdev = subdev;
> > +   return 0;
> > +   }
> > +
> > /* Find compatible subdevices mbus format */
> > vin->mbus_code = 0;
> > code.index = 0;
> > @@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct 
> > rvin_dev *vin,
> >  static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
> >  {
> > rvin_v4l2_unregister(vin);
> > -   v4l2_ctrl_handler_free(>ctrl_handler);
> > -
> > -   vin->vdev.ctrl_handler = NULL;
> > vin->parallel->subdev = NULL;
> > +
> > +   if (!vin->info->use_mc) {
> > +   v4l2_ctrl_handler_free(>ctrl_handler);
> > +   vin->vdev.ctrl_handler = NULL;
> > +   }
> >  }
> >
> >  static int rvin_parallel_notify_complete(struct v4l2_async_notifier 
> > *notifier)
> > @@ -552,18 +559,18 @@ static int rvin_parallel_parse_v4l2(struct device 
> > *dev,
> > return 0;
> >  }
> >
> > -static int rvin_parallel_graph_init(struct rvin_dev *vin)
> > +static int rvin_parallel_init(struct rvin_dev *vin)
> >  {
> > int ret;
> >
> > -   ret = v4l2_async_notifier_parse_fwnode_endpoints(
> > -   vin->dev, >notifier,
> > -   sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
> > +   ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> > +   vin->dev, >notifier, sizeof(struct rvin_parallel_entity),
> > +   0, rvin_parallel_parse_v4l2);
> > if (ret)
> > return ret;
> >
> > if (!vin->parallel)
> > -   return -ENODEV;
> > +   return -ENOTCONN;
>
> I think you still should return -ENODEV here if !vin->info->use_mc to
> preserve Gen2 which runs without media controller behavior. How about:
>
> return vin->info->use_mc ? -ENOTCONN : -ENODEV;

Right, I wish I had some gen2 board to test. I wonder if that's not
better handled in probe though... I'll see how it looks like.
>
> >
> > vin_dbg(vin, "Found parallel subdevice %pOF\n",
> > to_of_node(vin->parallel->asd.match.fwnode));
> > @@ -784,14 +791,8 @@ static int rvin_mc_init(struct rvin_dev *vin)
> >  {
> > int ret;
> >
> > -   vin->pad.flags = MEDIA_PAD_FL_SINK;
> > -   ret = media_entity_pads_init(>vdev.entity, 1, >pad);
> > -   if (ret)
> > -   return ret;
> > -
> > -   ret = rvin_group_get(vin);
> > -   if (ret)
> > -   return ret;
> > +   if (!vin->info->use_mc)
> > +   return 0;
> >
> > ret = rvin_mc_parse_of_graph(vin);
> > if (ret)
> > @@ -1074,11 +1075,24 @@ static int rcar_vin_probe(struct platform_device 
> > *pdev)
> > return ret;
> >
> > platform_set_drvdata(pdev, vin);
> > -   if (vin->info->use_mc)
> > -   ret = rvin_mc_init(vin);
> > -   else
> > -   ret = rvin_parallel_graph_init(vin);
> > -   if (ret < 0)
> > +
> > +   if (vin->info->use_mc) {
> > +   vin->pad.flags = MEDIA_PAD_FL_SINK;
> > +   ret = media_entity_pads_init(>vdev.entity, 1, >pad);
> > +   if (ret)
> > +   return ret;
> > +
> > +   ret = rvin_group_get(vin);
> > +   if (ret)
> > +   return ret;
> > +   }
>
> I don't see why you need to move the media pad creation out of
> rvin_mc_init(). With the 

Re: [PATCH v3] media: imx319: Add imx319 camera sensor driver

2018-05-25 Thread jacopo mondi
Hi Tomasz,

On Fri, May 25, 2018 at 03:18:38PM +0900, Tomasz Figa wrote:
> On Fri, May 25, 2018 at 5:47 AM Sakari Ailus 
> wrote:
>
> > Hi Jacopo,
>
> > On Thu, May 24, 2018 at 10:07:38PM +0200, jacopo mondi wrote:
> > ...
> > > > > about that, but I wonder why setting controls should be enabled only
> > > > > when streaming. I would have expected runtime_pm_get/put in
> subdevices
> > > > > node open/close functions not only when streaming. Am I missing
> something?
> > > >
> > > > You can do it either way. If powering on the sensor takes a long
> time, then
> > > > doing that in the open callback may be helpful as the user space has
> a way
> > > > to keep the device powered.
> > >
> > > Ok, so I assume my comment could be ignored, assuming is fine not
> > > being able to set control if the sensor is not streaming. Is it?
>
> > I'd say so. From the user's point of view, the sensor doesn't really do
> > anything when it's in software standby mode.
>
> Just to make sure we're on the same page, the code actually does nothing
> when the sensor is not in streaming mode (i.e. powered off). When stream is
> being started, the V4L2 control framework will call s_ctrl for all the
> controls in the handler to synchronize hardware state and this is when all
> the controls set before powering on will actually be programmed into the
> hardware registers.

Thanks, I had missed that part.

I quickly tried searching for 's_ctrl' calls in the v4l2-core/ code
and I've found nothing that invokes that in response to a streaming
start operation. Just if you happen to have any reference handy, could
you please point me to that part, just for my better understanding?

Thanks
   j

>
> Best regards,
> Tomasz


signature.asc
Description: PGP signature


Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Krzysztof Hałasa
Steve Longerbeam  writes:

>> The manual says: Reduce Double Read or Writes (RDRW):
>> This bit is relevant for YUV4:2:0 formats. For write channels:
>> U and V components are not written to odd rows.
>>
>> How could it be so? With YUV420, are they normally written?
>
> Well, given that this bit exists, and assuming I understand it
> correctly (1),
> I guess the U and V components for odd rows normally are placed on the
> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> the U and V components are the same for odd and even rows.

Right. Now, the AXI bus is just a "memory bus", it's a newer version of
the AHB. One can't simply "place data" on AXI, it must be a write to
a specific address, and the data will end up in RAM (assuming the
configuration is sane). How can we have two possible data formats (with
and without the RDRW bit) with fixed image format (420-type) is beyond
me.

> Commits
>
> 14330d7f08 ("media: imx: csi: enable double write reduction")
> b54a5c2dc8 ("media: imx: prpencvf: enable double write reduction")
>
> should be reverted for now, until the behavior of this bit is better
> understood.

I agree.

I have dumped a raw frame (720 x 480 NV12 frame size 518400 from
interlaced NTSC camera), with the RDRW bit set.

The Y plane contains, well, valid Y data (720 x 480 bytes).

The color plane (360 pixels x 240 line pairs * 2 colors) has every other
line pair zeroed. I.e., there is a 720-byte line pair filled with valid UV
data, then there are 720 zeros (360 zeroed UV pairs). Then there is valid
UV data and so on.

Not sure what could it be for. Some weird sort of YUV 4:1:0? I guess we
don't want it ATM.

WRT ADV7180 field format:

> This might be a good time to bring up the fact that the ADV7180 driver
> is wrong
> to set output to "interlaced". The ADV7180 does not transmit top lines
> interlaced
> with bottom lines. It transmits all top lines followed by all bottom
> lines (or
> vice-versa), i.e. it should be either V4L2_FIELD_SEQ_TB or
> V4L2_FIELD_SEQ_BT.
> It can also be set to V4L2_FIELD_ALTERNATE, and then it is left up to
> downstream
> elements to determine field order (TB or BT).

Right. ADV7180, AFAIK, doesn't have the hardware (frame buffer) to get
two interlaced fields and merge them to form a complete frame.
It simply transforms the incoming analog signal into binary data stream.
This issue should be fixed.

Thanks for your work,
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland


Re: [PATCH v10 01/16] videobuf2: Make struct vb2_buffer refcounted

2018-05-25 Thread sathyam panda
Hello,

On 5/21/18, Ezequiel Garcia  wrote:
> The in-fence implementation involves having a per-buffer fence callback,
> that triggers on the fence signal. The fence callback is called
> asynchronously
> and needs a valid reference to the associated ideobuf2 buffer.
>
> Allow this by making the vb2_buffer refcounted, so it can be passed
> to other contexts.
>

-Is it really required, because when a queued buffer with an in_fence
is deallocated, firstly queue is cancelled.
-And __vb2_dqbuf is called which calls dma_fence_remove_callback.
-So if fence callback has been called -__vb2_dqbuf will wait to
acquire fence lock.
-So during execution of fence callback, buffers and queue are still valid.
-And if __vb2_dqbuf remove callback first ,then dma_fence_signal will
wait for lock
- so there won't be any fence callback to call for that buffer when
dma_fence_signal resumes.

> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 27
> ++---
>  include/media/videobuf2-core.h  |  7 +--
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index d3f7bb33a54d..f1feb45c1e37 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -190,6 +190,26 @@ module_param(debug, int, 0644);
>  static void __vb2_queue_cancel(struct vb2_queue *q);
>  static void __enqueue_in_driver(struct vb2_buffer *vb);
>
> +static void __vb2_buffer_free(struct kref *kref)
> +{
> + struct vb2_buffer *vb =
> + container_of(kref, struct vb2_buffer, refcount);
> + kfree(vb);
> +}
> +
> +static void __vb2_buffer_put(struct vb2_buffer *vb)
> +{
> + if (vb)
> + kref_put(>refcount, __vb2_buffer_free);
> +}
> +
> +static struct vb2_buffer *__vb2_buffer_get(struct vb2_buffer *vb)
> +{
> + if (vb)
> + kref_get(>refcount);
> + return vb;
> +}
> +
>  /*
>   * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
>   */
> @@ -346,6 +366,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
> vb2_memory memory,
>   break;
>   }
>
> + kref_init(>refcount);
>   vb->state = VB2_BUF_STATE_DEQUEUED;
>   vb->vb2_queue = q;
>   vb->num_planes = num_planes;
> @@ -365,7 +386,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
> vb2_memory memory,
>   dprintk(1, "failed allocating memory for buffer 
> %d\n",
>   buffer);
>   q->bufs[vb->index] = NULL;
> - kfree(vb);
> + __vb2_buffer_put(vb);
>   break;
>   }
>   __setup_offsets(vb);
> @@ -380,7 +401,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
> vb2_memory memory,
>   buffer, vb);
>   __vb2_buf_mem_free(vb);
>   q->bufs[vb->index] = NULL;
> - kfree(vb);
> + __vb2_buffer_put(vb);
>   break;
>   }
>   }
> @@ -520,7 +541,7 @@ static int __vb2_queue_free(struct vb2_queue *q,
> unsigned int buffers)
>   /* Free videobuf buffers */
>   for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
>++buffer) {
> - kfree(q->bufs[buffer]);
> + __vb2_buffer_put(q->bufs[buffer]);
>   q->bufs[buffer] = NULL;
>   }
>
> diff --git a/include/media/videobuf2-core.h
> b/include/media/videobuf2-core.h
> index f6818f732f34..baa4632c7e59 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -12,11 +12,12 @@
>  #ifndef _MEDIA_VIDEOBUF2_CORE_H
>  #define _MEDIA_VIDEOBUF2_CORE_H
>
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
>
>  #define VB2_MAX_FRAME(32)
>  #define VB2_MAX_PLANES   (8)
> @@ -249,6 +250,7 @@ struct vb2_buffer {
>
>   /* private: internal use only
>*
> +  * refcount:refcount for this buffer
>* state:   current buffer state; do not change
>* queued_entry:entry on the queued buffers list, which holds
>*  all buffers queued from userspace
> @@ -256,6 +258,7 @@ struct vb2_buffer {
>*  to be dequeued to userspace
>* vb2_plane:   per-plane information; do not change
>*/
> + struct kref refcount;
>   enum vb2_buffer_state   state;
>
>   struct vb2_planeplanes[VB2_MAX_PLANES];
>

Regards,
 Sathyam


Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Philipp Zabel
Hi Steve,

On Thu, 2018-05-24 at 14:33 -0700, Steve Longerbeam wrote:
> Hi Krzysztof, Philipp,
> 
> And I can confirm that capturing planar 4:2:0 (YU12, YV12, or NV12),
> is broken because of the call to ipu_cpmem_skip_odd_chroma_rows().
> YU12 or NV12 images look correct again when commenting out that
> call. Commits
> 
> 14330d7f08 ("media: imx: csi: enable double write reduction")
> b54a5c2dc8 ("media: imx: prpencvf: enable double write reduction")
> 
> should be reverted for now, until the behavior of this bit is better 
> understood.

I think that is a bit radical. I am not aware of any problems with non-
interlaced formats. Could we just disable them when the interlaced_scan
bit is set?

regards
Philipp


Re: i.MX6 IPU CSI analog video input on Ventana

2018-05-25 Thread Philipp Zabel
On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
[...]
> > The following is required as well. Now the question is why we can't skip
> > writing those odd UV rows. Anyway, with these 2 changes, I get a stable
> > NTSC (and probably PAL) interlaced video stream.
> > 
> > The manual says: Reduce Double Read or Writes (RDRW):
> > This bit is relevant for YUV4:2:0 formats. For write channels:
> > U and V components are not written to odd rows.
> > 
> > How could it be so? With YUV420, are they normally written?
> 
> Well, given that this bit exists, and assuming I understand it correctly 
> (1),
> I guess the U and V components for odd rows normally are placed on the
> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> the U and V components are the same for odd and even rows.
> 
> In other words for writing 4:2:0 to memory, this bit should _always_ be set.
> 
> (1) OTOH I don't really understand what this bit is trying to say.
> Whether this bit is set or not, the data in memory is correct
> for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
> (decimated by 2 in width and height), followed by Y plane of 1/4
> size.
> 
> So I assume it is saying that the IPU normally places U/V components
> on the AXI bus for odd rows, that are identical to the even row values.

Whether they are identical depends on the input format.

The IDMAC always gets fed AYUV32 from the CSI or IC.
If the CSI captures YUV 4:2:x, odd and even lines will have the same
chroma values. But if the CSI captures YUV 4:4:4 (or RGB, fed through
the IC), we can have AYUV32 input with different chroma values on even
and odd lines.
In that case the IPU just writes the even chroma line and then
overwrites it with the odd line, unless the double write reduction bit
is set.

> IOW somehow those identical odd rows are dropped before writing to
> the U/V planes in memory.

potentially identical.

> Philipp please chime in if you have something to add here.

I suppose the bit could be used to choose to write the chroma values of
odd instead of even lines for 4:4:4 inputs, at the cost of increased
memory bandwidth usage.

> Steve
> 
> > OTOH it seems that not only UV is broken with this bit set.
> > Y is broken as well.

Maybe scanline interlave and double write reduction can't be used at the
same time?

regards
Philipp


Re: [PATCH v3] media: imx319: Add imx319 camera sensor driver

2018-05-25 Thread Tomasz Figa
On Fri, May 25, 2018 at 5:47 AM Sakari Ailus 
wrote:

> Hi Jacopo,

> On Thu, May 24, 2018 at 10:07:38PM +0200, jacopo mondi wrote:
> ...
> > > > about that, but I wonder why setting controls should be enabled only
> > > > when streaming. I would have expected runtime_pm_get/put in
subdevices
> > > > node open/close functions not only when streaming. Am I missing
something?
> > >
> > > You can do it either way. If powering on the sensor takes a long
time, then
> > > doing that in the open callback may be helpful as the user space has
a way
> > > to keep the device powered.
> >
> > Ok, so I assume my comment could be ignored, assuming is fine not
> > being able to set control if the sensor is not streaming. Is it?

> I'd say so. From the user's point of view, the sensor doesn't really do
> anything when it's in software standby mode.

Just to make sure we're on the same page, the code actually does nothing
when the sensor is not in streaming mode (i.e. powered off). When stream is
being started, the V4L2 control framework will call s_ctrl for all the
controls in the handler to synchronize hardware state and this is when all
the controls set before powering on will actually be programmed into the
hardware registers.

Best regards,
Tomasz


Re: [PATCH v10 12/16] vb2: add in-fence support to QBUF

2018-05-25 Thread sathyam panda
Hello,

On 5/21/18, Ezequiel Garcia  wrote:
> From: Gustavo Padovan 
>
> Receive in-fence from userspace and add support for waiting on them
> before queueing the buffer to the driver. Buffers can't be queued to the
> driver before its fences signal. And a buffer can't be queued to the driver
> out of the order they were queued from userspace. That means that even if
> its fence signals it must wait for all other buffers, ahead of it in the
> queue,
> to signal first.
>
> If the fence for some buffer fails we do not queue it to the driver,
> instead we mark it as error and wait until the previous buffer is done
> to notify userspace of the error. We wait here to deliver the buffers back
> to userspace in order.
>
> v13: - cleanup implementation.
>  - remove wrong Kconfig changes.
>  - print noisy warning on unexpected enqueue conditioin
>  - schedule a vb2_start_streaming work from the fence callback
>
> v12: fixed dvb_vb2.c usage of vb2_core_qbuf.
>
> v11: - minor doc/comments fixes (Hans Verkuil)
>  - reviewed the in-fence path at __fill_v4l2_buffer()
>
> v10: - rename fence to in_fence in many places
>  - handle fences signalling with error better (Hans Verkuil)
>
> v9: - improve comments and docs (Hans Verkuil)
> - fix unlocking of vb->fence_cb_lock on vb2_core_qbuf (Hans Verkuil)
> - move in-fences code that was in the out-fences patch here (Alex)
>
> v8: - improve comments about fences with errors
>
> v7: - get rid of the fence array stuff for ordering and just use
>   get_num_buffers_ready() (Hans)
> - fix issue of queuing the buffer twice (Hans)
> - avoid the dma_fence_wait() in core_qbuf() (Alex)
> - merge preparation commit in
>
> v6: - With fences always keep the order userspace queues the buffers.
> - Protect in_fence manipulation with a lock (Brian Starkey)
> - check if fences have the same context before adding a fence array
> - Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
> - Clean up fence if __set_in_fence() fails (Brian Starkey)
> - treat -EINVAL from dma_fence_add_callback() (Brian Starkey)
>
> v5: - use fence_array to keep buffers ordered in vb2 core when
>   needed (Brian Starkey)
> - keep backward compat on the reserved2 field (Brian Starkey)
> - protect fence callback removal with lock (Brian Starkey)
>
> v4: - Add a comment about dma_fence_add_callback() not returning a
>   error (Hans)
> - Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
> - select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
> - Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
> - Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
> - Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
>   vb2_start_streaming() (Hans)
> - set IN_FENCE flags on __fill_v4l2_buffer (Hans)
> - Queue buffers to the driver as soon as they are ready (Hans)
> - call fill_user_buffer() after queuing the buffer (Hans)
> - add err: label to clean up fence
> - add dma_fence_wait() before calling vb2_start_streaming()
>
> v3: - document fence parameter
> - remove ternary if at vb2_qbuf() return (Mauro)
> - do not change if conditions behaviour (Mauro)
>
> v2: - fix vb2_queue_or_prepare_buf() ret check
> - remove check for VB2_MEMORY_DMABUF only (Javier)
> - check num of ready buffers to start streaming
> - when queueing, start from the first ready buffer
> - handle queue cancel
>
> Signed-off-by: Gustavo Padovan 
> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/media/common/videobuf2/Kconfig  |   1 +
>  drivers/media/common/videobuf2/videobuf2-core.c | 224
> 
>  drivers/media/common/videobuf2/videobuf2-v4l2.c |  37 +++-
>  drivers/media/dvb-core/dvb_vb2.c|   2 +-
>  include/media/videobuf2-core.h  |  19 +-
>  5 files changed, 242 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/Kconfig
> b/drivers/media/common/videobuf2/Kconfig
> index 17c32ea58395..27ad9e8a268b 100644
> --- a/drivers/media/common/videobuf2/Kconfig
> +++ b/drivers/media/common/videobuf2/Kconfig
> @@ -1,6 +1,7 @@
>  # Used by drivers that need Videobuf2 modules
>  config VIDEOBUF2_CORE
>   select DMA_SHARED_BUFFER
> + select SYNC_FILE
>   tristate
>
>  config VIDEOBUF2_V4L2
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index a9a0a9d1decb..86b5ebe25263 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -27,6 +27,7 @@
>  #include 
>
>  #include 
> +#include 
>  #include 
>
>  #include 
> @@ -189,6 +190,7 @@ module_param(debug, int, 0644);
>
>  static void __vb2_queue_cancel(struct vb2_queue *q);
>  static void