cron job: media_tree daily build: WARNINGS

[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:   Tue Sep 20 04:00:17 CEST 2016
git branch: test
git hash:   142a0e11b52c18a82c4fe55132b762005dda05c0
gcc version:i686-linux-gcc (GCC) 5.4.0
sparse version: v0.5.0-56-g7647c77
smatch version: v0.5.0-3428-gdfe27cf
host hardware:  x86_64
host os:4.6.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: Null pointer dereference in ngene-core.c

[Devin Heitmueller]

On Mon, Sep 19, 2016 at 8:51 PM, Alexandre-Xavier Labonté-Lamoureux
 wrote:
> Hi people,
>
> In the file "/linux/drivers/media/pci/ngene/ngene-core.c", there is a
> null pointer dereference at line 1480.
>
> Code in the function "static int init_channel(struct ngene_channel *chan)"
> ==
> if (io & NGENE_IO_TSIN) {
> chan->fe = NULL;  // Set to NULL
> if (ni->demod_attach[nr]) { // First condition
>ret = ni->demod_attach[nr](chan);
> if (ret < 0)   // Another condition
> goto err; // Goto that avoids
> the problem
> }
> if (chan->fe && ni->tuner_attach[nr]) { // Condition that
> tests the null pointer
> ret = ni->tuner_attach[nr](chan);
> if (ret < 0)
> goto err;
> }
> }
> =
>
> "chan->fe" is set to NULL, then it tests for something (I have no idea
> what it's doing, I know nothing about this driver), if the results of
> the first two if conditions fail to reach the goto, then it will test
> the condition with the null pointer, which will cause a crash. I don't
> know if the kernel can recover from null pointers, I think not.

I would have to actually look at the code, but my guess is because the
call to ni-ni->demod_attach[nr](chan) will actually set chan->fe if
successful.

The code path your describing is actually the primary use case.  The
cases where you see "goto err" will only be followed if there was some
sort of error condition, which means the driver essentially will
*always* hit the if() statement you are referring to during normal
operation (assuming nothing was broken in the hardware).

In short, the code makes sure chan->fe is NULL, then it calls the
demod_attach, then it checks both for the demod_attach returning an
error *and* making sure demod_attach set chan->fe to some non-NULL
value.  If both are the case, then it calls tuner_attach().

This is a pretty standard workflow -- you should see it in many other
drivers, although it's not uncommon in other drivers for something
like chan->fe to actually be the value returned by the demod_attach(),
and the demod attach routine would return NULL if there was some
failure condition.  The problem with that approach is that you can
only report one type of failure to the caller (all the caller knows is
a failure occured, it has no visibility as to the nature of the
failure), whereas with this approach you can return different values
for different error conditions.

Devin

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

Re: Null pointer dereference in ngene-core.c

[Devin Heitmueller]

On Mon, Sep 19, 2016 at 8:51 PM, Alexandre-Xavier Labonté-Lamoureux
 wrote:
> Hi people,
>
> In the file "/linux/drivers/media/pci/ngene/ngene-core.c", there is a
> null pointer dereference at line 1480.
>
> Code in the function "static int init_channel(struct ngene_channel *chan)"
> ==
> if (io & NGENE_IO_TSIN) {
> chan->fe = NULL;  // Set to NULL
> if (ni->demod_attach[nr]) { // First condition
>ret = ni->demod_attach[nr](chan);
> if (ret < 0)   // Another condition
> goto err; // Goto that avoids
> the problem
> }
> if (chan->fe && ni->tuner_attach[nr]) { // Condition that
> tests the null pointer
> ret = ni->tuner_attach[nr](chan);
> if (ret < 0)
> goto err;
> }
> }
> =
>
> "chan->fe" is set to NULL, then it tests for something (I have no idea
> what it's doing, I know nothing about this driver), if the results of
> the first two if conditions fail to reach the goto, then it will test
> the condition with the null pointer, which will cause a crash. I don't
> know if the kernel can recover from null pointers, I think not.

This looks fine to me.  It's a simple test to see if chan->fe got set
to null (presumably in the above block of code).  A null pointer
dereference would be if the first block set *chan* to NULL (as opposed
to chan->fe) and then the if() statement then attempted to inspect
chan->fe.

LGTM.

Devin

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

Summary of the discussion about Rockchip VPU in Gstreamer

[Randy Li]

Hello all media staff
Dear Mr.Verkuil
Dear Mr.Osciak
 I talked with Nicolas and Mr.ceyusa in the yesterday and early morning 
of today.
  I think I have made them get the situation of state-less Video 
Acceleration Unit(VPU) and Rockchip for VA-API driver. We both agree 
that creating a new C API bindings to V4L2 is making wheel again. 
Mr.Ceyusa suggest that there could be a middle library to parse those 
codec settings to V4L2 extra controls array, and push back to Gstreamer, 
leaving the V4L2 related job to Gstreamer.
  I agree with him. I think the Gstreamer then could get rid of 
hardware detail, even not need to know internal data structure in kernel 
driver of codec parameters.
  Later, the ad-n770 joined us. He gave me some idea about the 
relationship with VA-API and DXVA2. I found we do need some extra data 
beyond those data used by VA-API to reconstruct a frame, it is a 
limitation in HW. We better regard this kind of HW to a Acceleration 
Unit rather than Full decoder/Encoder. Also ad-n770 pointer out that it 
seems that Rockchip HW could do the reodering job, which is not need 
actually as it is done by Gstreamer, but I am not sure whether the 
Hardware does and I could disable this logic.
  I am sorry I can't attend the conference in Berlin. But I hope we 
could keep in discussion in this topic, and offering more information to 
you before the meetings.
  Currently, I would still work on VA-API framework and I am learning 
something about codec through a book, I hope that it make me explaining 
the situation easily.


--
Randy Li
The third produce department
===
This email message, including any attachments, is for the sole
use of the intended recipient(s) and may contain confidential and
privileged information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message. [Fuzhou Rockchip Electronics, INC. China mainland]
===

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

Null pointer dereference in ngene-core.c

[Alexandre-Xavier Labonté-Lamoureux]

Hi people,

In the file "/linux/drivers/media/pci/ngene/ngene-core.c", there is a
null pointer dereference at line 1480.

Code in the function "static int init_channel(struct ngene_channel *chan)"
==
if (io & NGENE_IO_TSIN) {
chan->fe = NULL;  // Set to NULL
if (ni->demod_attach[nr]) { // First condition
   ret = ni->demod_attach[nr](chan);
if (ret < 0)   // Another condition
goto err; // Goto that avoids
the problem
}
if (chan->fe && ni->tuner_attach[nr]) { // Condition that
tests the null pointer
ret = ni->tuner_attach[nr](chan);
if (ret < 0)
goto err;
}
}
=

"chan->fe" is set to NULL, then it tests for something (I have no idea
what it's doing, I know nothing about this driver), if the results of
the first two if conditions fail to reach the goto, then it will test
the condition with the null pointer, which will cause a crash. I don't
know if the kernel can recover from null pointers, I think not.

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

Re: [PATCH v3 00/18] More smiapp cleanups, fixes

[Sebastian Reichel]

Hi,

On Tue, Sep 20, 2016 at 01:02:33AM +0300, Sakari Ailus wrote:
> This set further cleans up the smiapp driver and prepares for
> later changes.
> 
> since v2:
> 
> - Fix badly formatted debug message on wrong frame format model type
> 
> - Add a debug message on faulty frame descriptor (image data lines are
>   among embedded data lines)
> 
> - Fix error handling in registered() callback, add  unregistered()
>   callback
> 
> - smiapp_create_subdev() will return immediately if its ssd argument is
>   NULL. No need for caller to check this.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 2/5] smiapp: Set device for pixel array and binner

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:29:18PM +0300, Sakari Ailus wrote:
> The dev field of the v4l2_subdev was left NULL for the pixel array and
> binner sub-devices. Fix this.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 3/5] smiapp: Set use suspend and resume ops for other functions

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:29:19PM +0300, Sakari Ailus wrote:
> Use the suspend and resume ops for freeze, thaw, poweroff and restore
> callbacks as well.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v1.1 4/5] smiapp: Use runtime PM

[Sebastian Reichel]

Hi,

On Fri, Sep 16, 2016 at 01:53:29AM +0300, Sakari Ailus wrote:
> [...]
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c 
> b/drivers/media/i2c/smiapp/smiapp-regs.c
> index 1e501c0..a9c7baf 100644
> --- a/drivers/media/i2c/smiapp/smiapp-regs.c
> +++ b/drivers/media/i2c/smiapp/smiapp-regs.c
> @@ -18,6 +18,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "smiapp.h"
>  #include "smiapp-regs.h"
> @@ -288,8 +289,12 @@ int smiapp_write_no_quirk(struct smiapp_sensor *sensor, 
> u32 reg, u32 val)
>   */
>  int smiapp_write(struct smiapp_sensor *sensor, u32 reg, u32 val)
>  {
> + struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
>   int rval;
>  
> + if (pm_runtime_suspended(>dev))
> + return 0;
> +

This looks racy. What if idle countdown runs out immediately after
this check? If you can't call get_sync in this function you can
call pm_runtime_get() before the suspend check and pm_runtime_put
before returning from the function, so that the device keeps being
enabled.

Also I would expect some error code instead of success for early
return due to device being suspended?

>   rval = smiapp_call_quirk(sensor, reg_access, true, , );
>   if (rval == -ENOIOCTLCMD)
>   return 0;
>
> [...]

-- Sebastian


signature.asc
Description: PGP signature

[PATCH] v4l-utils: ir-ctl: give proper error message if transmitter does not exist

[Sean Young]

If a transmitter does not exist when setting using -e, you get the error:

warning: /dev/lirc0: failed to set send transmitters: Success

Signed-off-by: Sean Young 
---
 utils/ir-ctl/ir-ctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index 05b46a3..229330e 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -516,7 +516,9 @@ static int lirc_options(struct arguments *args, int fd, 
unsigned features)
if (args->emitters) {
if (features & LIRC_CAN_SET_TRANSMITTER_MASK) {
rc = ioctl(fd, LIRC_SET_TRANSMITTER_MASK, 
>emitters);
-   if (rc)
+   if (rc > 0)
+   fprintf(stderr, _("warning: %s: failed to set 
send transmitters: only %d available\n"), dev, rc);
+   else if (rc < 0)
fprintf(stderr, _("warning: %s: failed to set 
send transmitters: %m\n"), dev);
} else
fprintf(stderr, _("warning: %s: does not support 
setting send transmitters\n"), dev);
-- 
2.7.4

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

[PATCH] [media] rc: rc6 decoder should report protocol correctly

[Sean Young]

When reporting decoded protocol use the enum rather than the bitmap.

Signed-off-by: Sean Young 
---
 drivers/media/rc/ir-rc6-decoder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/ir-rc6-decoder.c 
b/drivers/media/rc/ir-rc6-decoder.c
index e0e2ede..5cc54c9 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -248,7 +248,7 @@ again:
toggle = 0;
break;
case 24:
-   protocol = RC_BIT_RC6_6A_24;
+   protocol = RC_TYPE_RC6_6A_24;
toggle = 0;
break;
case 32:
@@ -257,7 +257,7 @@ again:
toggle = !!(scancode & 
RC6_6A_MCE_TOGGLE_MASK);
scancode &= ~RC6_6A_MCE_TOGGLE_MASK;
} else {
-   protocol = RC_BIT_RC6_6A_32;
+   protocol = RC_TYPE_RC6_6A_32;
toggle = 0;
}
break;
-- 
2.7.4

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

[PATCH] [media] rc: Hauppauge z8f0811 can decode RC6

[Sean Young]

The hardware does not decode the 16, 20 or 24 bit variety.

Signed-off-by: Sean Young 
---
 drivers/media/i2c/ir-kbd-i2c.c   | 90 ++--
 drivers/media/pci/cx18/cx18-i2c.c|  3 +-
 drivers/media/pci/cx88/cx88-input.c  |  3 +-
 drivers/media/pci/ivtv/ivtv-i2c.c|  3 +-
 drivers/media/usb/hdpvr/hdpvr-i2c.c  |  2 +-
 drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c |  3 +-
 6 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index bf82726..f95a6bc 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -35,6 +35,7 @@
  *
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -63,51 +64,80 @@ module_param(debug, int, 0644);/* debug level (0,1,2) */
 /* --- */
 
 static int get_key_haup_common(struct IR_i2c *ir, enum rc_type *protocol,
-  u32 *scancode, u8 *ptoggle, int size, int offset)
+   u32 *scancode, u8 *ptoggle, int size)
 {
unsigned char buf[6];
-   int start, range, toggle, dev, code, ircode;
+   int start, range, toggle, dev, code, ircode, vendor;
 
/* poll IR chip */
if (size != i2c_master_recv(ir->c, buf, size))
return -EIO;
 
-   /* split rc5 data block ... */
-   start  = (buf[offset] >> 7) &1;
-   range  = (buf[offset] >> 6) &1;
-   toggle = (buf[offset] >> 5) &1;
-   dev=  buf[offset]   & 0x1f;
-   code   = (buf[offset+1] >> 2) & 0x3f;
+   if (buf[0] & 0x80) {
+   int offset = (size == 6) ? 3 : 0;
 
-   /* rc5 has two start bits
-* the first bit must be one
-* the second bit defines the command range (1 = 0-63, 0 = 64 - 127)
-*/
-   if (!start)
-   /* no key pressed */
-   return 0;
+   /* split rc5 data block ... */
+   start  = (buf[offset] >> 7) &1;
+   range  = (buf[offset] >> 6) &1;
+   toggle = (buf[offset] >> 5) &1;
+   dev=  buf[offset]   & 0x1f;
+   code   = (buf[offset+1] >> 2) & 0x3f;
 
-   /* filter out invalid key presses */
-   ircode = (start << 12) | (toggle << 11) | (dev << 6) | code;
-   if ((ircode & 0x1fff) == 0x1fff)
-   return 0;
+   /* rc5 has two start bits
+* the first bit must be one
+* the second bit defines the command range:
+* 1 = 0-63, 0 = 64 - 127
+*/
+   if (!start)
+   /* no key pressed */
+   return 0;
 
-   if (!range)
-   code += 64;
+   /* filter out invalid key presses */
+   ircode = (start << 12) | (toggle << 11) | (dev << 6) | code;
+   if ((ircode & 0x1fff) == 0x1fff)
+   return 0;
 
-   dprintk(1,"ir hauppauge (rc5): s%d r%d t%d dev=%d code=%d\n",
-   start, range, toggle, dev, code);
+   if (!range)
+   code += 64;
 
-   *protocol = RC_TYPE_RC5;
-   *scancode = RC_SCANCODE_RC5(dev, code);
-   *ptoggle = toggle;
-   return 1;
+   dprintk(1, "ir hauppauge (rc5): s%d r%d t%d dev=%d code=%d\n",
+   start, range, toggle, dev, code);
+
+   *protocol = RC_TYPE_RC5;
+   *scancode = RC_SCANCODE_RC5(dev, code);
+   *ptoggle = toggle;
+
+   return 1;
+   } else if (size == 6 && (buf[0] & 0x40)) {
+   code = buf[4];
+   dev = buf[3];
+   vendor = get_unaligned_be16(buf + 1);
+
+   if (vendor == 0x800f) {
+   *ptoggle = (dev & 0x80) != 0;
+   *protocol = RC_TYPE_RC6_MCE;
+   dev &= 0x7f;
+   dprintk(1, "ir hauppauge (rc6-mce): t%d vendor=%d 
dev=%d code=%d\n",
+   toggle, vendor, dev, code);
+   } else {
+   *ptoggle = 0;
+   *protocol = RC_TYPE_RC6_6A_32;
+   dprintk(1, "ir hauppauge (rc6-6a-32): vendor=%d dev=%d 
code=%d\n",
+   vendor, dev, code);
+   }
+
+   *scancode = RC_SCANCODE_RC6_6A(vendor, dev, code);
+
+   return 1;
+   }
+
+   return 0;
 }
 
 static int get_key_haup(struct IR_i2c *ir, enum rc_type *protocol,
u32 *scancode, u8 *toggle)
 {
-   return get_key_haup_common (ir, protocol, scancode, toggle, 3, 0);
+   return get_key_haup_common(ir, protocol, scancode, toggle, 3);
 }
 
 static int get_key_haup_xvr(struct IR_i2c *ir, enum rc_type *protocol,
@@ 

[PATCH] v4l-utils: ir-ctl: deal with consecutive pulses or spaces correctly

[Sean Young]

When sending a pulse-space file with consecutive spaces or pulses, add them
together correctly. For example:

pulse 100
space 150
space 100
pulse 150
pulse 200

Would send pulse 100, space 250, and pulse 350.

Signed-off-by: Sean Young 
---
 utils/ir-ctl/ir-ctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index 229330e..2f85e6d 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -211,7 +211,7 @@ static struct file *read_file(const char *fname)
fprintf(stderr, _("warning: %s:%d: 
leading space ignored\n"),
fname, lineno);
} else {
-   f->buf[len] += arg;
+   f->buf[len-1] += arg;
}
} else {
f->buf[len++] = arg;
@@ -220,7 +220,7 @@ static struct file *read_file(const char *fname)
expect_pulse = true;
} else if (strcmp(keyword, "pulse") == 0) {
if (!expect_pulse)
-   f->buf[len] += arg;
+   f->buf[len-1] += arg;
else
f->buf[len++] = arg;
expect_pulse = false;
-- 
2.7.4

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

[PATCH v3 07/18] smiapp: Always initialise the sensor in probe

[Sakari Ailus]

Initialise the sensor in probe. The reason why it wasn't previously done
in case of platform data was that the probe() of the driver that provided
the clock through the set_xclk() callback would need to finish before the
probe() function of the smiapp driver. The set_xclk() callback no longer
exists.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 30 +-
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 9873b3d..8a58c64 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2530,8 +2530,9 @@ static int smiapp_register_subdev(struct smiapp_sensor 
*sensor,
return 0;
 }
 
-static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
+static int smiapp_registered(struct v4l2_subdev *subdev)
 {
+   struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
int rval;
 
if (sensor->scaler) {
@@ -2819,25 +2820,6 @@ out_power_off:
return rval;
 }
 
-static int smiapp_registered(struct v4l2_subdev *subdev)
-{
-   struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
-   struct i2c_client *client = v4l2_get_subdevdata(subdev);
-   int rval;
-
-   if (!client->dev.of_node) {
-   rval = smiapp_init(sensor);
-   if (rval)
-   return rval;
-   }
-
-   rval = smiapp_register_subdevs(sensor);
-   if (rval)
-   smiapp_cleanup(sensor);
-
-   return rval;
-}
-
 static int smiapp_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 {
struct smiapp_subdev *ssd = to_smiapp_subdev(sd);
@@ -3079,11 +3061,9 @@ static int smiapp_probe(struct i2c_client *client,
sensor->src->sensor = sensor;
sensor->src->pads[0].flags = MEDIA_PAD_FL_SOURCE;
 
-   if (client->dev.of_node) {
-   rval = smiapp_init(sensor);
-   if (rval)
-   goto out_media_entity_cleanup;
-   }
+   rval = smiapp_init(sensor);
+   if (rval)
+   goto out_media_entity_cleanup;
 
rval = media_entity_pads_init(>src->sd.entity, 2,
 sensor->src->pads);
-- 
2.1.4

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

[PATCH v3 03/18] smiapp: Initialise media entity after sensor init

[Sakari Ailus]

This allows determining the number of pads in the entity based on the
sensor.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 2090b7f..4f97503 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -3058,12 +3058,7 @@ static int smiapp_probe(struct i2c_client *client,
sensor->src->sd.internal_ops = _internal_src_ops;
sensor->src->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->src->sensor = sensor;
-
sensor->src->pads[0].flags = MEDIA_PAD_FL_SOURCE;
-   rval = media_entity_pads_init(>src->sd.entity, 2,
-sensor->src->pads);
-   if (rval < 0)
-   return rval;
 
if (client->dev.of_node) {
rval = smiapp_init(sensor);
@@ -3071,6 +3066,11 @@ static int smiapp_probe(struct i2c_client *client,
goto out_media_entity_cleanup;
}
 
+   rval = media_entity_pads_init(>src->sd.entity, 2,
+sensor->src->pads);
+   if (rval < 0)
+   goto out_media_entity_cleanup;
+
rval = v4l2_async_register_subdev(>src->sd);
if (rval < 0)
goto out_media_entity_cleanup;
-- 
2.1.4

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

[PATCH v3 11/18] smiapp: Unify setting up sub-devices

[Sakari Ailus]

The initialisation of the source sub-device is somewhat different as it's
not created by the smiapp driver itself. Remove redundancy in initialising
the two kind of sub-devices.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 6ec17ea..7ac0d4e0 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2591,6 +2591,7 @@ static void smiapp_create_subdev(struct smiapp_sensor 
*sensor,
if (ssd != sensor->src)
v4l2_subdev_init(>sd, _ops);
 
+   ssd->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
ssd->sensor = sensor;
 
ssd->npads = num_pads;
@@ -2616,7 +2617,6 @@ static void smiapp_create_subdev(struct smiapp_sensor 
*sensor,
if (ssd == sensor->src)
return;
 
-   ssd->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
ssd->sd.internal_ops = _internal_ops;
ssd->sd.owner = THIS_MODULE;
v4l2_set_subdevdata(>sd, client);
@@ -2861,9 +2861,6 @@ static int smiapp_probe(struct i2c_client *client,
 
v4l2_i2c_subdev_init(>src->sd, client, _ops);
sensor->src->sd.internal_ops = _internal_src_ops;
-   sensor->src->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   sensor->src->sensor = sensor;
-   sensor->src->pads[0].flags = MEDIA_PAD_FL_SOURCE;
 
sensor->vana = devm_regulator_get(>dev, "vana");
if (IS_ERR(sensor->vana)) {
-- 
2.1.4

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

[PATCH v3 10/18] smiapp: Read frame format earlier

[Sakari Ailus]

The information gathered during frame format reading will be required
earlier in the initialisation when it was available. Also return an error
if frame format cannot be obtained.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 384a13b..6ec17ea 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2908,6 +2908,12 @@ static int smiapp_probe(struct i2c_client *client,
goto out_power_off;
}
 
+   rval = smiapp_read_frame_fmt(sensor);
+   if (rval) {
+   rval = -ENODEV;
+   goto out_power_off;
+   }
+
/*
 * Handle Sensor Module orientation on the board.
 *
@@ -3030,8 +3036,6 @@ static int smiapp_probe(struct i2c_client *client,
 
sensor->pixel_array->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 
-   /* final steps */
-   smiapp_read_frame_fmt(sensor);
rval = smiapp_init_controls(sensor);
if (rval < 0)
goto out_cleanup;
-- 
2.1.4

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

[PATCH v3 04/18] smiapp: Split off sub-device registration into two

[Sakari Ailus]

Remove the loop in sub-device registration and create each sub-device
explicitly instead.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 82 +++---
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 4f97503..86c0d7c 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2475,54 +2475,62 @@ static const struct v4l2_subdev_ops smiapp_ops;
 static const struct v4l2_subdev_internal_ops smiapp_internal_ops;
 static const struct media_entity_operations smiapp_entity_ops;
 
-static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
+static int smiapp_register_subdev(struct smiapp_sensor *sensor,
+ struct smiapp_subdev *ssd,
+ struct smiapp_subdev *sink_ssd,
+ u16 source_pad, u16 sink_pad, u32 link_flags)
 {
struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
-   struct smiapp_subdev *ssds[] = {
-   sensor->scaler,
-   sensor->binner,
-   sensor->pixel_array,
-   };
-   unsigned int i;
int rval;
 
-   for (i = 0; i < SMIAPP_SUBDEVS - 1; i++) {
-   struct smiapp_subdev *this = ssds[i + 1];
-   struct smiapp_subdev *last = ssds[i];
+   if (!sink_ssd)
+   return 0;
 
-   if (!last)
-   continue;
+   rval = media_entity_pads_init(>sd.entity,
+ ssd->npads, ssd->pads);
+   if (rval) {
+   dev_err(>dev,
+   "media_entity_pads_init failed\n");
+   return rval;
+   }
 
-   rval = media_entity_pads_init(>sd.entity,
-this->npads, this->pads);
-   if (rval) {
-   dev_err(>dev,
-   "media_entity_pads_init failed\n");
-   return rval;
-   }
+   rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
+  >sd);
+   if (rval) {
+   dev_err(>dev,
+   "v4l2_device_register_subdev failed\n");
+   return rval;
+   }
 
-   rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
-  >sd);
-   if (rval) {
-   dev_err(>dev,
-   "v4l2_device_register_subdev failed\n");
-   return rval;
-   }
+   rval = media_create_pad_link(>sd.entity, source_pad,
+_ssd->sd.entity, sink_pad,
+link_flags);
+   if (rval) {
+   dev_err(>dev,
+   "media_create_pad_link failed\n");
+   return rval;
+   }
 
-   rval = media_create_pad_link(>sd.entity,
-this->source_pad,
->sd.entity,
-last->sink_pad,
-MEDIA_LNK_FL_ENABLED |
-MEDIA_LNK_FL_IMMUTABLE);
-   if (rval) {
-   dev_err(>dev,
-   "media_create_pad_link failed\n");
+   return 0;
+}
+
+static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
+{
+   int rval;
+
+   if (sensor->scaler) {
+   rval = smiapp_register_subdev(
+   sensor, sensor->binner, sensor->scaler,
+   SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
+   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
+   if (rval < 0)
return rval;
-   }
}
 
-   return 0;
+   return smiapp_register_subdev(
+   sensor, sensor->pixel_array, sensor->binner,
+   SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
+   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
 }
 
 static void smiapp_cleanup(struct smiapp_sensor *sensor)
-- 
2.1.4

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

[PATCH v3 08/18] smiapp: Fix resource management in registration failure

[Sakari Ailus]

If the registered() callback failed, resources were left unaccounted for.
Fix this, as well as add unregistering the sub-devices in driver
unregistered() callback.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 8a58c64..9d7af8b 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2524,12 +2524,22 @@ static int smiapp_register_subdev(struct smiapp_sensor 
*sensor,
if (rval) {
dev_err(>dev,
"media_create_pad_link failed\n");
+   v4l2_device_unregister_subdev(>sd);
return rval;
}
 
return 0;
 }
 
+static void smiapp_unregistered(struct v4l2_subdev *subdev)
+{
+   struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
+   unsigned int i;
+
+   for (i = 1; i < sensor->ssds_used; i++)
+   v4l2_device_unregister_subdev(>ssds[i].sd);
+}
+
 static int smiapp_registered(struct v4l2_subdev *subdev)
 {
struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
@@ -2544,10 +2554,19 @@ static int smiapp_registered(struct v4l2_subdev *subdev)
return rval;
}
 
-   return smiapp_register_subdev(
+   rval = smiapp_register_subdev(
sensor, sensor->pixel_array, sensor->binner,
SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
+   if (rval)
+   goto out_err;
+
+   return 0;
+
+out_err:
+   smiapp_unregistered(subdev);
+
+   return rval;
 }
 
 static void smiapp_cleanup(struct smiapp_sensor *sensor)
@@ -2894,6 +2913,7 @@ static const struct media_entity_operations 
smiapp_entity_ops = {
 
 static const struct v4l2_subdev_internal_ops smiapp_internal_src_ops = {
.registered = smiapp_registered,
+   .unregistered = smiapp_unregistered,
.open = smiapp_open,
.close = smiapp_close,
 };
-- 
2.1.4

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

[PATCH v3 05/18] smiapp: Provide a common function to obtain native pixel array size

[Sakari Ailus]

The same pixel array size is required for the active format of each
sub-device sink pad and try format of each sink pad of each opened file
handle as well as for the native size rectangle.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 39 +-
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 86c0d7c..e5d4584 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2161,6 +2161,15 @@ static int smiapp_set_crop(struct v4l2_subdev *subdev,
return 0;
 }
 
+static void smiapp_get_native_size(struct smiapp_subdev *ssd,
+   struct v4l2_rect *r)
+{
+   r->top = 0;
+   r->left = 0;
+   r->width = ssd->sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
+   r->height = ssd->sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
+}
+
 static int __smiapp_get_selection(struct v4l2_subdev *subdev,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_selection *sel)
@@ -2192,17 +2201,12 @@ static int __smiapp_get_selection(struct v4l2_subdev 
*subdev,
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
case V4L2_SEL_TGT_NATIVE_SIZE:
-   if (ssd == sensor->pixel_array) {
-   sel->r.left = sel->r.top = 0;
-   sel->r.width =
-   sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
-   sel->r.height =
-   sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
-   } else if (sel->pad == ssd->sink_pad) {
+   if (ssd == sensor->pixel_array)
+   smiapp_get_native_size(ssd, >r);
+   else if (sel->pad == ssd->sink_pad)
sel->r = sink_fmt;
-   } else {
+   else
sel->r = *comp;
-   }
break;
case V4L2_SEL_TGT_CROP:
case V4L2_SEL_TGT_COMPOSE_BOUNDS:
@@ -2564,10 +2568,8 @@ static void smiapp_create_subdev(struct smiapp_sensor 
*sensor,
 sizeof(ssd->sd.name), "%s %s %d-%4.4x", sensor->minfo.name,
 name, i2c_adapter_id(client->adapter), client->addr);
 
-   ssd->sink_fmt.width =
-   sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
-   ssd->sink_fmt.height =
-   sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
+   smiapp_get_native_size(ssd, >sink_fmt);
+
ssd->compose.width = ssd->sink_fmt.width;
ssd->compose.height = ssd->sink_fmt.height;
ssd->crop[ssd->source_pad] = ssd->compose;
@@ -2840,16 +2842,13 @@ static int smiapp_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
struct v4l2_rect *try_crop = v4l2_subdev_get_try_crop(sd, 
fh->pad, i);
struct v4l2_rect *try_comp;
 
-   try_fmt->width = sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
-   try_fmt->height = sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
+   smiapp_get_native_size(ssd, try_crop);
+
+   try_fmt->width = try_crop->width;
+   try_fmt->height = try_crop->height;
try_fmt->code = mbus_code;
try_fmt->field = V4L2_FIELD_NONE;
 
-   try_crop->top = 0;
-   try_crop->left = 0;
-   try_crop->width = try_fmt->width;
-   try_crop->height = try_fmt->height;
-
if (ssd != sensor->pixel_array)
continue;
 
-- 
2.1.4

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

[PATCH v3 18/18] smiapp-pll: Don't complain aloud about failing PLL calculation

[Sakari Ailus]

Don't complain about a failure to compute the pre_pll divisor. The
function is used to determine whether a particular combination of bits per
sample value and a link frequency can be used, in which case there are
lots of unnecessary driver messages. During normal operation the failure
generally does not happen. Use dev_dbg() instead.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp-pll.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index e3348db..771db56 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -479,7 +479,8 @@ int smiapp_pll_calculate(struct device *dev,
return 0;
}
 
-   dev_info(dev, "unable to compute pre_pll divisor\n");
+   dev_dbg(dev, "unable to compute pre_pll divisor\n");
+
return rval;
 }
 EXPORT_SYMBOL_GPL(smiapp_pll_calculate);
-- 
2.1.4

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

[PATCH v3 17/18] smiapp: Drop a debug print on frame size and bit depth

[Sakari Ailus]

The first time the sensor is powered on, the information is not yet
available.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 5f4680d..8f9690e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -926,12 +926,6 @@ static int smiapp_update_mode(struct smiapp_sensor *sensor)
unsigned int binning_mode;
int rval;
 
-   dev_dbg(>dev, "frame size: %dx%d\n",
-   sensor->src->crop[SMIAPP_PAD_SRC].width,
-   sensor->src->crop[SMIAPP_PAD_SRC].height);
-   dev_dbg(>dev, "csi format width: %d\n",
-   sensor->csi_format->width);
-
/* Binning has to be set up here; it affects limits */
if (sensor->binning_horizontal == 1 &&
sensor->binning_vertical == 1) {
-- 
2.1.4

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

[PATCH v3 14/18] smiapp: Improve debug messages from frame layout reading

[Sakari Ailus]

Provide more debugging information on reading the frame layout.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index a7afcea..1337b22 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -100,12 +100,11 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
*sensor)
u32 pixels;
char *which;
char *what;
+   u32 reg;
 
if (fmt_model_type == SMIAPP_FRAME_FORMAT_MODEL_TYPE_2BYTE) {
-   rval = smiapp_read(
-   sensor,
-   SMIAPP_REG_U16_FRAME_FORMAT_DESCRIPTOR_2(i),
-   );
+   reg = SMIAPP_REG_U16_FRAME_FORMAT_DESCRIPTOR_2(i);
+   rval = smiapp_read(sensor, reg, );
if (rval)
return rval;
 
@@ -116,10 +115,8 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
*sensor)
pixels = desc & SMIAPP_FRAME_FORMAT_DESC_2_PIXELS_MASK;
} else if (fmt_model_type
   == SMIAPP_FRAME_FORMAT_MODEL_TYPE_4BYTE) {
-   rval = smiapp_read(
-   sensor,
-   SMIAPP_REG_U32_FRAME_FORMAT_DESCRIPTOR_4(i),
-   );
+   reg = SMIAPP_REG_U32_FRAME_FORMAT_DESCRIPTOR_4(i);
+   rval = smiapp_read(sensor, reg, );
if (rval)
return rval;
 
@@ -158,12 +155,12 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
*sensor)
break;
default:
what = "invalid";
-   dev_dbg(>dev, "pixelcode %d\n", pixelcode);
break;
}
 
-   dev_dbg(>dev, "%s pixels: %d %s\n",
-   what, pixels, which);
+   dev_dbg(>dev,
+   "0x%8.8x %s pixels: %d %s (pixelcode %u)\n", reg,
+   what, pixels, which, pixelcode);
 
if (i < ncol_desc) {
if (pixelcode ==
-- 
2.1.4

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

[PATCH v3 12/18] smiapp: Use SMIAPP_PADS when referring to number of pads

[Sakari Ailus]

Replace plain value 2 with SMIAPP_PADS when referring to the number of
pads.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp.h 
b/drivers/media/i2c/smiapp/smiapp.h
index e71271e..f9febe0 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -157,9 +157,9 @@ struct smiapp_binning_subtype {
 
 struct smiapp_subdev {
struct v4l2_subdev sd;
-   struct media_pad pads[2];
+   struct media_pad pads[SMIAPP_PADS];
struct v4l2_rect sink_fmt;
-   struct v4l2_rect crop[2];
+   struct v4l2_rect crop[SMIAPP_PADS];
struct v4l2_rect compose; /* compose on sink */
unsigned short sink_pad;
unsigned short source_pad;
-- 
2.1.4

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

[PATCH v3 06/18] smiapp: Remove unnecessary BUG_ON()'s

[Sakari Ailus]

Instead, calculate how much is needed and then allocate the memory
dynamically.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 24 ++--
 drivers/media/i2c/smiapp/smiapp.h  |  8 ++--
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index e5d4584..9873b3d 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -621,7 +621,7 @@ static int smiapp_init_controls(struct smiapp_sensor 
*sensor)
 static int smiapp_init_late_controls(struct smiapp_sensor *sensor)
 {
unsigned long *valid_link_freqs = >valid_link_freqs[
-   sensor->csi_format->compressed - SMIAPP_COMPRESSED_BASE];
+   sensor->csi_format->compressed - sensor->compressed_min_bpp];
unsigned int max, i;
 
for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) {
@@ -754,6 +754,7 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor 
*sensor)
 {
struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
struct smiapp_pll *pll = >pll;
+   u8 compressed_max_bpp = 0;
unsigned int type, n;
unsigned int i, pixel_order;
int rval;
@@ -826,16 +827,27 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor 
*sensor)
pll->scale_m = sensor->scale_m;
 
for (i = 0; i < ARRAY_SIZE(smiapp_csi_data_formats); i++) {
+   sensor->compressed_min_bpp =
+   min(smiapp_csi_data_formats[i].compressed,
+   sensor->compressed_min_bpp);
+   compressed_max_bpp =
+   max(smiapp_csi_data_formats[i].compressed,
+   compressed_max_bpp);
+   }
+
+   sensor->valid_link_freqs = devm_kcalloc(
+   >dev,
+   compressed_max_bpp - sensor->compressed_min_bpp + 1,
+   sizeof(*sensor->valid_link_freqs), GFP_KERNEL);
+
+   for (i = 0; i < ARRAY_SIZE(smiapp_csi_data_formats); i++) {
const struct smiapp_csi_data_format *f =
_csi_data_formats[i];
unsigned long *valid_link_freqs =
>valid_link_freqs[
-   f->compressed - SMIAPP_COMPRESSED_BASE];
+   f->compressed - sensor->compressed_min_bpp];
unsigned int j;
 
-   BUG_ON(f->compressed < SMIAPP_COMPRESSED_BASE);
-   BUG_ON(f->compressed > SMIAPP_COMPRESSED_MAX);
-
if (!(sensor->default_mbus_frame_fmts & 1 << i))
continue;
 
@@ -1769,7 +1781,7 @@ static int smiapp_set_format_source(struct v4l2_subdev 
*subdev,
 
valid_link_freqs = 
>valid_link_freqs[sensor->csi_format->compressed
- - SMIAPP_COMPRESSED_BASE];
+ - sensor->compressed_min_bpp];
 
__v4l2_ctrl_modify_range(
sensor->link_freq, 0,
diff --git a/drivers/media/i2c/smiapp/smiapp.h 
b/drivers/media/i2c/smiapp/smiapp.h
index aae72bc..e71271e 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -150,11 +150,6 @@ struct smiapp_csi_data_format {
 #define SMIAPP_PAD_SRC 1
 #define SMIAPP_PADS2
 
-#define SMIAPP_COMPRESSED_BASE 8
-#define SMIAPP_COMPRESSED_MAX  16
-#define SMIAPP_NR_OF_COMPRESSED(SMIAPP_COMPRESSED_MAX - \
-SMIAPP_COMPRESSED_BASE + 1)
-
 struct smiapp_binning_subtype {
u8 horizontal:4;
u8 vertical:4;
@@ -224,6 +219,7 @@ struct smiapp_sensor {
 
bool streaming;
bool dev_init_done;
+   u8 compressed_min_bpp;
 
u8 *nvm;/* nvm memory buffer */
unsigned int nvm_size;  /* bytes */
@@ -233,7 +229,7 @@ struct smiapp_sensor {
struct smiapp_pll pll;
 
/* Is a default format supported for a given BPP? */
-   unsigned long valid_link_freqs[SMIAPP_NR_OF_COMPRESSED];
+   unsigned long *valid_link_freqs;
 
/* Pixel array controls */
struct v4l2_ctrl *analog_gain;
-- 
2.1.4

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

[PATCH v3 15/18] smiapp: Remove useless newlines and other small cleanups

[Sakari Ailus]

The code probably has been unindented at some point but rewrapping has not
been done. Do it now.

Also remove a useless memory allocation failure message.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 1337b22..2e8b7bf 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -446,8 +446,7 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
orient |= SMIAPP_IMAGE_ORIENTATION_VFLIP;
 
orient ^= sensor->hvflip_inv_mask;
-   rval = smiapp_write(sensor,
-   SMIAPP_REG_U8_IMAGE_ORIENTATION,
+   rval = smiapp_write(sensor, SMIAPP_REG_U8_IMAGE_ORIENTATION,
orient);
if (rval < 0)
return rval;
@@ -462,10 +461,8 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
__smiapp_update_exposure_limits(sensor);
 
if (exposure > sensor->exposure->maximum) {
-   sensor->exposure->val =
-   sensor->exposure->maximum;
-   rval = smiapp_set_ctrl(
-   sensor->exposure);
+   sensor->exposure->val = sensor->exposure->maximum;
+   rval = smiapp_set_ctrl(sensor->exposure);
if (rval < 0)
return rval;
}
@@ -1322,8 +1319,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
if (!sensor->pixel_array)
return 0;
 
-   rval = v4l2_ctrl_handler_setup(
-   >pixel_array->ctrl_handler);
+   rval = v4l2_ctrl_handler_setup(>pixel_array->ctrl_handler);
if (rval)
goto out_cci_addr_fail;
 
@@ -1629,7 +1625,8 @@ static int __smiapp_get_format(struct v4l2_subdev *subdev,
struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
 
if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-   fmt->format = *v4l2_subdev_get_try_format(subdev, cfg, 
fmt->pad);
+   fmt->format = *v4l2_subdev_get_try_format(subdev, cfg,
+ fmt->pad);
} else {
struct v4l2_rect *r;
 
@@ -1729,7 +1726,6 @@ static void smiapp_propagate(struct v4l2_subdev *subdev,
 static const struct smiapp_csi_data_format
 *smiapp_validate_csi_data_format(struct smiapp_sensor *sensor, u32 code)
 {
-   const struct smiapp_csi_data_format *csi_format = sensor->csi_format;
unsigned int i;
 
for (i = 0; i < ARRAY_SIZE(smiapp_csi_data_formats); i++) {
@@ -1738,7 +1734,7 @@ static const struct smiapp_csi_data_format
return _csi_data_formats[i];
}
 
-   return csi_format;
+   return sensor->csi_format;
 }
 
 static int smiapp_set_format_source(struct v4l2_subdev *subdev,
@@ -2072,8 +2068,7 @@ static int smiapp_set_compose(struct v4l2_subdev *subdev,
smiapp_set_compose_scaler(subdev, cfg, sel, crops, comp);
 
*comp = sel->r;
-   smiapp_propagate(subdev, cfg, sel->which,
-V4L2_SEL_TGT_COMPOSE);
+   smiapp_propagate(subdev, cfg, sel->which, V4L2_SEL_TGT_COMPOSE);
 
if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return smiapp_update_mode(sensor);
@@ -2150,9 +2145,8 @@ static int smiapp_set_crop(struct v4l2_subdev *subdev,
->height;
src_size = &_r;
} else {
-   src_size =
-   v4l2_subdev_get_try_compose(
-   subdev, cfg, ssd->sink_pad);
+   src_size = v4l2_subdev_get_try_compose(
+   subdev, cfg, ssd->sink_pad);
}
}
 
@@ -2638,7 +2632,8 @@ static int smiapp_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
for (i = 0; i < ssd->npads; i++) {
struct v4l2_mbus_framefmt *try_fmt =
v4l2_subdev_get_try_format(sd, fh->pad, i);
-   struct v4l2_rect *try_crop = v4l2_subdev_get_try_crop(sd, 
fh->pad, i);
+   struct v4l2_rect *try_crop =
+   v4l2_subdev_get_try_crop(sd, fh->pad, i);
struct v4l2_rect *try_comp;
 
smiapp_get_native_size(ssd, try_crop);
@@ -2878,8 +2873,7 @@ static int smiapp_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   rval = clk_set_rate(sensor->ext_clk,
-   sensor->hwcfg->ext_clk);
+   rval = clk_set_rate(sensor->ext_clk, sensor->hwcfg->ext_clk);
if (rval < 0) {
   

[PATCH v3 16/18] smiapp: Obtain correct media bus code for try format

[Sakari Ailus]

The media bus code obtained for try format may have been a code that the
sensor did not even support. Use a supported code with the current pixel
order.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 2e8b7bf..5f4680d 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2623,8 +2623,6 @@ static int smiapp_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
 {
struct smiapp_subdev *ssd = to_smiapp_subdev(sd);
struct smiapp_sensor *sensor = ssd->sensor;
-   u32 mbus_code =
-   smiapp_csi_data_formats[smiapp_pixel_order(sensor)].code;
unsigned int i;
 
mutex_lock(>mutex);
@@ -2640,7 +2638,7 @@ static int smiapp_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
 
try_fmt->width = try_crop->width;
try_fmt->height = try_crop->height;
-   try_fmt->code = mbus_code;
+   try_fmt->code = sensor->internal_csi_format->code;
try_fmt->field = V4L2_FIELD_NONE;
 
if (ssd != sensor->pixel_array)
-- 
2.1.4

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

[PATCH v3 13/18] smiapp: Obtain frame layout from the frame descriptor

[Sakari Ailus]

Besides the image data, SMIA++ compliant sensors also provide embedded
data in form of registers used to capture the image. Store this
information for later use in frame descriptor and routing.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 46 +++---
 drivers/media/i2c/smiapp/smiapp.h  |  5 +++-
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 7ac0d4e0..a7afcea 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -68,10 +68,9 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
*sensor)
struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
u32 fmt_model_type, fmt_model_subtype, ncol_desc, nrow_desc;
unsigned int i;
-   int rval;
+   int pixel_count = 0;
int line_count = 0;
-   int embedded_start = -1, embedded_end = -1;
-   int image_start = 0;
+   int rval;
 
rval = smiapp_read(sensor, SMIAPP_REG_U8_FRAME_FORMAT_MODEL_TYPE,
   _model_type);
@@ -166,33 +165,40 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
*sensor)
dev_dbg(>dev, "%s pixels: %d %s\n",
what, pixels, which);
 
-   if (i < ncol_desc)
+   if (i < ncol_desc) {
+   if (pixelcode ==
+   SMIAPP_FRAME_FORMAT_DESC_PIXELCODE_VISIBLE)
+   sensor->visible_pixel_start = pixel_count;
+   pixel_count += pixels;
continue;
+   }
 
/* Handle row descriptors */
-   if (pixelcode
-   == SMIAPP_FRAME_FORMAT_DESC_PIXELCODE_EMBEDDED) {
-   embedded_start = line_count;
-   } else {
-   if (pixelcode == 
SMIAPP_FRAME_FORMAT_DESC_PIXELCODE_VISIBLE
-   || pixels >= 
sensor->limits[SMIAPP_LIMIT_MIN_FRAME_LENGTH_LINES] / 2)
-   image_start = line_count;
-   if (embedded_start != -1 && embedded_end == -1)
-   embedded_end = line_count;
+   switch (pixelcode) {
+   case SMIAPP_FRAME_FORMAT_DESC_PIXELCODE_EMBEDDED:
+   if (sensor->embedded_end)
+   break;
+   sensor->embedded_start = line_count;
+   sensor->embedded_end = line_count + pixels;
+   break;
+   case SMIAPP_FRAME_FORMAT_DESC_PIXELCODE_VISIBLE:
+   sensor->image_start = line_count;
+   break;
}
line_count += pixels;
}
 
-   if (embedded_start == -1 || embedded_end == -1) {
-   embedded_start = 0;
-   embedded_end = 0;
+   if (sensor->embedded_end > sensor->image_start) {
+   dev_dbg(>dev,
+   "adjusting image start line to %u (was %u)\n",
+   sensor->embedded_end, sensor->image_start);
+   sensor->image_start = sensor->embedded_end;
}
 
-   sensor->image_start = image_start;
-
dev_dbg(>dev, "embedded data from lines %d to %d\n",
-   embedded_start, embedded_end);
-   dev_dbg(>dev, "image data starts at line %d\n", image_start);
+   sensor->embedded_start, sensor->embedded_end);
+   dev_dbg(>dev, "image data starts at line %d\n",
+   sensor->image_start);
 
return 0;
 }
diff --git a/drivers/media/i2c/smiapp/smiapp.h 
b/drivers/media/i2c/smiapp/smiapp.h
index f9febe0..d7b52a6 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -213,7 +213,10 @@ struct smiapp_sensor {
 
u8 hvflip_inv_mask; /* H/VFLIP inversion due to sensor orientation */
u8 frame_skip;
-   u16 image_start;/* Offset to first line after metadata lines */
+   u16 embedded_start; /* embedded data start line */
+   u16 embedded_end;
+   u16 image_start; /* image data start line */
+   u16 visible_pixel_start; /* start pixel of the visible image */
 
int power_count;
 
-- 
2.1.4

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

[PATCH v3 02/18] smiapp: Explicitly define number of pads in initialisation

[Sakari Ailus]

Define the number of pads explicitly in initialising the sub-devices.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 957e37e..2090b7f 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2536,7 +2536,8 @@ static void smiapp_cleanup(struct smiapp_sensor *sensor)
 }
 
 static void smiapp_create_subdev(struct smiapp_sensor *sensor,
-struct smiapp_subdev *ssd, const char *name)
+struct smiapp_subdev *ssd, const char *name,
+unsigned short num_pads)
 {
struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
 
@@ -2548,12 +2549,8 @@ static void smiapp_create_subdev(struct smiapp_sensor 
*sensor,
 
ssd->sensor = sensor;
 
-   if (ssd == sensor->pixel_array) {
-   ssd->npads = 1;
-   } else {
-   ssd->npads = 2;
-   ssd->source_pad = 1;
-   }
+   ssd->npads = num_pads;
+   ssd->source_pad = num_pads - 1;
 
snprintf(ssd->sd.name,
 sizeof(ssd->sd.name), "%s %s %d-%4.4x", sensor->minfo.name,
@@ -2747,9 +2744,9 @@ static int smiapp_init(struct smiapp_sensor *sensor)
if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
pll->flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
 
-   smiapp_create_subdev(sensor, sensor->scaler, "scaler");
-   smiapp_create_subdev(sensor, sensor->binner, "binner");
-   smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array");
+   smiapp_create_subdev(sensor, sensor->scaler, "scaler", 2);
+   smiapp_create_subdev(sensor, sensor->binner, "binner", 2);
+   smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array", 1);
 
dev_dbg(>dev, "profile %d\n", sensor->minfo.smiapp_profile);
 
-- 
2.1.4

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

[PATCH v3 09/18] smiapp: Merge smiapp_init() with smiapp_probe()

[Sakari Ailus]

The smiapp_probe() is the sole caller of smiapp_init(). Unify the two.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 423 -
 1 file changed, 204 insertions(+), 219 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 9d7af8b..384a13b 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2622,223 +2622,6 @@ static void smiapp_create_subdev(struct smiapp_sensor 
*sensor,
v4l2_set_subdevdata(>sd, client);
 }
 
-static int smiapp_init(struct smiapp_sensor *sensor)
-{
-   struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
-   struct smiapp_pll *pll = >pll;
-   unsigned int i;
-   int rval;
-
-   sensor->vana = devm_regulator_get(>dev, "vana");
-   if (IS_ERR(sensor->vana)) {
-   dev_err(>dev, "could not get regulator for vana\n");
-   return PTR_ERR(sensor->vana);
-   }
-
-   sensor->ext_clk = devm_clk_get(>dev, NULL);
-   if (IS_ERR(sensor->ext_clk)) {
-   dev_err(>dev, "could not get clock (%ld)\n",
-   PTR_ERR(sensor->ext_clk));
-   return -EPROBE_DEFER;
-   }
-
-   rval = clk_set_rate(sensor->ext_clk,
-   sensor->hwcfg->ext_clk);
-   if (rval < 0) {
-   dev_err(>dev,
-   "unable to set clock freq to %u\n",
-   sensor->hwcfg->ext_clk);
-   return rval;
-   }
-
-   sensor->xshutdown = devm_gpiod_get_optional(>dev, "xshutdown",
-   GPIOD_OUT_LOW);
-   if (IS_ERR(sensor->xshutdown))
-   return PTR_ERR(sensor->xshutdown);
-
-   rval = smiapp_power_on(sensor);
-   if (rval)
-   return -ENODEV;
-
-   rval = smiapp_identify_module(sensor);
-   if (rval) {
-   rval = -ENODEV;
-   goto out_power_off;
-   }
-
-   rval = smiapp_get_all_limits(sensor);
-   if (rval) {
-   rval = -ENODEV;
-   goto out_power_off;
-   }
-
-   /*
-* Handle Sensor Module orientation on the board.
-*
-* The application of H-FLIP and V-FLIP on the sensor is modified by
-* the sensor orientation on the board.
-*
-* For SMIAPP_BOARD_SENSOR_ORIENT_180 the default behaviour is to set
-* both H-FLIP and V-FLIP for normal operation which also implies
-* that a set/unset operation for user space HFLIP and VFLIP v4l2
-* controls will need to be internally inverted.
-*
-* Rotation also changes the bayer pattern.
-*/
-   if (sensor->hwcfg->module_board_orient ==
-   SMIAPP_MODULE_BOARD_ORIENT_180)
-   sensor->hvflip_inv_mask = SMIAPP_IMAGE_ORIENTATION_HFLIP |
- SMIAPP_IMAGE_ORIENTATION_VFLIP;
-
-   rval = smiapp_call_quirk(sensor, limits);
-   if (rval) {
-   dev_err(>dev, "limits quirks failed\n");
-   goto out_power_off;
-   }
-
-   if (sensor->limits[SMIAPP_LIMIT_BINNING_CAPABILITY]) {
-   u32 val;
-
-   rval = smiapp_read(sensor,
-  SMIAPP_REG_U8_BINNING_SUBTYPES, );
-   if (rval < 0) {
-   rval = -ENODEV;
-   goto out_power_off;
-   }
-   sensor->nbinning_subtypes = min_t(u8, val,
- SMIAPP_BINNING_SUBTYPES);
-
-   for (i = 0; i < sensor->nbinning_subtypes; i++) {
-   rval = smiapp_read(
-   sensor, SMIAPP_REG_U8_BINNING_TYPE_n(i), );
-   if (rval < 0) {
-   rval = -ENODEV;
-   goto out_power_off;
-   }
-   sensor->binning_subtypes[i] =
-   *(struct smiapp_binning_subtype *)
-
-   dev_dbg(>dev, "binning %xx%x\n",
-   sensor->binning_subtypes[i].horizontal,
-   sensor->binning_subtypes[i].vertical);
-   }
-   }
-   sensor->binning_horizontal = 1;
-   sensor->binning_vertical = 1;
-
-   if (device_create_file(>dev, _attr_ident) != 0) {
-   dev_err(>dev, "sysfs ident entry creation failed\n");
-   rval = -ENOENT;
-   goto out_power_off;
-   }
-   /* SMIA++ NVM initialization - it will be read from the sensor
-* when it is first requested by userspace.
-*/
-   if (sensor->minfo.smiapp_version && sensor->hwcfg->nvm_size) {
-   sensor->nvm = devm_kzalloc(>dev,
-   sensor->hwcfg->nvm_size, GFP_KERNEL);

[PATCH v3 00/18] More smiapp cleanups, fixes

[Sakari Ailus]

Hi,

This set further cleans up the smiapp driver and prepares for later
changes.

since v2:

- Fix badly formatted debug message on wrong frame format model type

- Add a debug message on faulty frame descriptor (image data lines are
  among embedded data lines)

- Fix error handling in registered() callback, add  unregistered()
  callback

- smiapp_create_subdev() will return immediately if its ssd argument is
  NULL. No need for caller to check this.

-- 
Kind regards,
Sakari

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

[PATCH v3 01/18] smiapp: Move sub-device initialisation into a separate function

[Sakari Ailus]

Simplify smiapp_init() by moving the initialisation of individual
sub-devices to a separate function.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/smiapp/smiapp-core.c | 110 +++--
 1 file changed, 51 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 44f8c7e..957e37e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2535,11 +2535,58 @@ static void smiapp_cleanup(struct smiapp_sensor *sensor)
smiapp_free_controls(sensor);
 }
 
+static void smiapp_create_subdev(struct smiapp_sensor *sensor,
+struct smiapp_subdev *ssd, const char *name)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
+
+   if (!ssd)
+   return;
+
+   if (ssd != sensor->src)
+   v4l2_subdev_init(>sd, _ops);
+
+   ssd->sensor = sensor;
+
+   if (ssd == sensor->pixel_array) {
+   ssd->npads = 1;
+   } else {
+   ssd->npads = 2;
+   ssd->source_pad = 1;
+   }
+
+   snprintf(ssd->sd.name,
+sizeof(ssd->sd.name), "%s %s %d-%4.4x", sensor->minfo.name,
+name, i2c_adapter_id(client->adapter), client->addr);
+
+   ssd->sink_fmt.width =
+   sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
+   ssd->sink_fmt.height =
+   sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
+   ssd->compose.width = ssd->sink_fmt.width;
+   ssd->compose.height = ssd->sink_fmt.height;
+   ssd->crop[ssd->source_pad] = ssd->compose;
+   ssd->pads[ssd->source_pad].flags = MEDIA_PAD_FL_SOURCE;
+   if (ssd != sensor->pixel_array) {
+   ssd->crop[ssd->sink_pad] = ssd->compose;
+   ssd->pads[ssd->sink_pad].flags = MEDIA_PAD_FL_SINK;
+   }
+
+   ssd->sd.entity.ops = _entity_ops;
+
+   if (ssd == sensor->src)
+   return;
+
+   ssd->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+   ssd->sd.internal_ops = _internal_ops;
+   ssd->sd.owner = THIS_MODULE;
+   v4l2_set_subdevdata(>sd, client);
+}
+
 static int smiapp_init(struct smiapp_sensor *sensor)
 {
struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
struct smiapp_pll *pll = >pll;
-   struct smiapp_subdev *last = NULL;
unsigned int i;
int rval;
 
@@ -2700,64 +2747,9 @@ static int smiapp_init(struct smiapp_sensor *sensor)
if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
pll->flags |= SMIAPP_PLL_FLAG_NO_OP_CLOCKS;
 
-   for (i = 0; i < SMIAPP_SUBDEVS; i++) {
-   struct {
-   struct smiapp_subdev *ssd;
-   char *name;
-   } const __this[] = {
-   { sensor->scaler, "scaler", },
-   { sensor->binner, "binner", },
-   { sensor->pixel_array, "pixel array", },
-   }, *_this = &__this[i];
-   struct smiapp_subdev *this = _this->ssd;
-
-   if (!this)
-   continue;
-
-   if (this != sensor->src)
-   v4l2_subdev_init(>sd, _ops);
-
-   this->sensor = sensor;
-
-   if (this == sensor->pixel_array) {
-   this->npads = 1;
-   } else {
-   this->npads = 2;
-   this->source_pad = 1;
-   }
-
-   snprintf(this->sd.name,
-sizeof(this->sd.name), "%s %s %d-%4.4x",
-sensor->minfo.name, _this->name,
-i2c_adapter_id(client->adapter), client->addr);
-
-   this->sink_fmt.width =
-   sensor->limits[SMIAPP_LIMIT_X_ADDR_MAX] + 1;
-   this->sink_fmt.height =
-   sensor->limits[SMIAPP_LIMIT_Y_ADDR_MAX] + 1;
-   this->compose.width = this->sink_fmt.width;
-   this->compose.height = this->sink_fmt.height;
-   this->crop[this->source_pad] = this->compose;
-   this->pads[this->source_pad].flags = MEDIA_PAD_FL_SOURCE;
-   if (this != sensor->pixel_array) {
-   this->crop[this->sink_pad] = this->compose;
-   this->pads[this->sink_pad].flags = MEDIA_PAD_FL_SINK;
-   }
-
-   this->sd.entity.ops = _entity_ops;
-
-   if (last == NULL) {
-   last = this;
-   continue;
-   }
-
-   this->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   this->sd.internal_ops = _internal_ops;
-   this->sd.owner = THIS_MODULE;
-   v4l2_set_subdevdata(>sd, client);
-
-   last = this;
-   }
+   smiapp_create_subdev(sensor, sensor->scaler, "scaler");
+   

Re: [PATCH v2 03/17] smiapp: Initialise media entity after sensor init

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:17PM +0300, Sakari Ailus wrote:
> This allows determining the number of pads in the entity based on the
> sensor.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index be74ba3..0a03f30 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -3056,12 +3056,7 @@ static int smiapp_probe(struct i2c_client *client,
>   sensor->src->sd.internal_ops = _internal_src_ops;
>   sensor->src->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>   sensor->src->sensor = sensor;
> -
>   sensor->src->pads[0].flags = MEDIA_PAD_FL_SOURCE;
> - rval = media_entity_pads_init(>src->sd.entity, 2,
> -  sensor->src->pads);
> - if (rval < 0)
> - return rval;
>  
>   if (client->dev.of_node) {
>   rval = smiapp_init(sensor);
> @@ -3069,6 +3064,11 @@ static int smiapp_probe(struct i2c_client *client,
>   goto out_media_entity_cleanup;
>   }
>  
> + rval = media_entity_pads_init(>src->sd.entity, 2,
> +  sensor->src->pads);
> + if (rval < 0)
> + goto out_media_entity_cleanup;
> +
>   rval = v4l2_async_register_subdev(>src->sd);
>   if (rval < 0)
>   goto out_media_entity_cleanup;

As far as I can see this is not strictly needed, but:

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 17/17] smiapp-pll: Don't complain aloud about failing PLL calculation

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:31PM +0300, Sakari Ailus wrote:
> Don't complain about a failure to compute the pre_pll divisor. The
> function is used to determine whether a particular combination of bits per
> sample value and a link frequency can be used, in which case there are
> lots of unnecessary driver messages. During normal operation the failure
> generally does not happen. Use dev_dbg() instead.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 16/17] smiapp: Drop a debug print on frame size and bit depth

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:30PM +0300, Sakari Ailus wrote:
> The first time the sensor is powered on, the information is not yet
> available.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 15/17] smiapp: Obtain correct media bus code for try format

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:29PM +0300, Sakari Ailus wrote:
> The media bus code obtained for try format may have been a code that the
> sensor did not even support. Use a supported code with the current pixel
> order.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 14/17] smiapp: Remove useless newlines and other small cleanups

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:28PM +0300, Sakari Ailus wrote:
> The code probably has been unindented at some point but rewrapping has not
> been done. Do it now.
> 
> Also remove a useless memory allocation failure message.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 13/17] smiapp: Improve debug messages from frame layout reading

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:27PM +0300, Sakari Ailus wrote:
> Provide more debugging information on reading the frame layout.
> 
> [...]
>
> @@ -130,7 +127,7 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor 
> *sensor)
>   pixels = desc & SMIAPP_FRAME_FORMAT_DESC_4_PIXELS_MASK;
>   } else {
>   dev_dbg(>dev,
> - "invalid frame format model type %d\n",
> + "0x8.8x invalid frame format model type %d\n",

0x8.8x doesn't look very interesting ;)

Apart from the '%' the actual argument is also missing.

> [...]

Once that is fixed:

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 09/17] smiapp: Read frame format earlier

[Sebastian Reichel]

Hi,

On Tue, Sep 20, 2016 at 12:19:54AM +0300, Sakari Ailus wrote:
> Hi Sebastian,
> 
> Sebastian Reichel wrote:
> > Hi,
> > 
> > On Thu, Sep 15, 2016 at 02:22:23PM +0300, Sakari Ailus wrote:
> > > The information gathered during frame format reading will be required
> > > earlier in the initialisation when it was available. Also return an error
> > > if frame format cannot be obtained.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > ---
> > >   drivers/media/i2c/smiapp/smiapp-core.c | 8 ++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> > > b/drivers/media/i2c/smiapp/smiapp-core.c
> > > index 0b5671c..c9aee83 100644
> > > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > > @@ -2890,6 +2890,12 @@ static int smiapp_probe(struct i2c_client *client,
> > >   goto out_power_off;
> > >   }
> > > 
> > > + rval = smiapp_read_frame_fmt(sensor);
> > > + if (rval) {
> > > + rval = -ENODEV;
> > > + goto out_power_off;
> > > + }
> > > +
> > >   /*
> > >* Handle Sensor Module orientation on the board.
> > >*
> > > @@ -3013,8 +3019,6 @@ static int smiapp_probe(struct i2c_client *client,
> > > 
> > >   sensor->pixel_array->sd.entity.function = 
> > > MEDIA_ENT_F_CAM_SENSOR;
> > > 
> > > - /* final steps */
> > > - smiapp_read_frame_fmt(sensor);
> > >   rval = smiapp_init_controls(sensor);
> > >   if (rval < 0)
> > >   goto out_cleanup;
> > 
> > Is this missing a Fixes tag, or will it only be required earlier for
> > future patches?
> 
> It's primarily for future patches. Reading the frame format will require
> limits but hardly any other information. On the other hand, the frame format
> will very likely be needed elsewhere, hence the move.
> 
> The missing return value check is just a bug which I believe has been there
> since around 2011.

ok, so the move is for future patches. Then

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 12/17] smiapp: Obtain frame layout from the frame descriptor

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:26PM +0300, Sakari Ailus wrote:
> Besides the image data, SMIA++ compliant sensors also provide embedded
> data in form of registers used to capture the image. Store this
> information for later use in frame descriptor and routing.

Reviewed-By: Sebastian Reichel 

> [...]
>
> + if (sensor->embedded_end > sensor->image_start)
> + sensor->image_start = sensor->embedded_end;

Maybe add a dev_dbg about sensor format information being broken?

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 09/17] smiapp: Read frame format earlier

[Sakari Ailus]

Hi Sebastian,

Sebastian Reichel wrote:

Hi,

On Thu, Sep 15, 2016 at 02:22:23PM +0300, Sakari Ailus wrote:

The information gathered during frame format reading will be required
earlier in the initialisation when it was available. Also return an error
if frame format cannot be obtained.

Signed-off-by: Sakari Ailus 
---
  drivers/media/i2c/smiapp/smiapp-core.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 0b5671c..c9aee83 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2890,6 +2890,12 @@ static int smiapp_probe(struct i2c_client *client,
goto out_power_off;
}

+   rval = smiapp_read_frame_fmt(sensor);
+   if (rval) {
+   rval = -ENODEV;
+   goto out_power_off;
+   }
+
/*
 * Handle Sensor Module orientation on the board.
 *
@@ -3013,8 +3019,6 @@ static int smiapp_probe(struct i2c_client *client,

sensor->pixel_array->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;

-   /* final steps */
-   smiapp_read_frame_fmt(sensor);
rval = smiapp_init_controls(sensor);
if (rval < 0)
goto out_cleanup;


Is this missing a Fixes tag, or will it only be required earlier for
future patches?


It's primarily for future patches. Reading the frame format will require 
limits but hardly any other information. On the other hand, the frame 
format will very likely be needed elsewhere, hence the move.


The missing return value check is just a bug which I believe has been 
there since around 2011.


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

Re: [PATCH v2 11/17] smiapp: Use SMIAPP_PADS when referring to number of pads

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:25PM +0300, Sakari Ailus wrote:
> Replace plain value 2 with SMIAPP_PADS when referring to the number of
> pads.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 10/17] smiapp: Unify setting up sub-devices

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:24PM +0300, Sakari Ailus wrote:
> The initialisation of the source sub-device is somewhat different as it's
> not created by the smiapp driver itself. Remove redundancy in initialising
> the two kind of sub-devices.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 09/17] smiapp: Read frame format earlier

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:23PM +0300, Sakari Ailus wrote:
> The information gathered during frame format reading will be required
> earlier in the initialisation when it was available. Also return an error
> if frame format cannot be obtained.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index 0b5671c..c9aee83 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2890,6 +2890,12 @@ static int smiapp_probe(struct i2c_client *client,
>   goto out_power_off;
>   }
>  
> + rval = smiapp_read_frame_fmt(sensor);
> + if (rval) {
> + rval = -ENODEV;
> + goto out_power_off;
> + }
> +
>   /*
>* Handle Sensor Module orientation on the board.
>*
> @@ -3013,8 +3019,6 @@ static int smiapp_probe(struct i2c_client *client,
>  
>   sensor->pixel_array->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>  
> - /* final steps */
> - smiapp_read_frame_fmt(sensor);
>   rval = smiapp_init_controls(sensor);
>   if (rval < 0)
>   goto out_cleanup;

Is this missing a Fixes tag, or will it only be required earlier for
future patches?

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 08/17] smiapp: Merge smiapp_init() with smiapp_probe()

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:22PM +0300, Sakari Ailus wrote:
> The smiapp_probe() is the sole caller of smiapp_init(). Unify the two.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 07/17] smiapp: Always initialise the sensor in probe

[Sakari Ailus]

Sebastian Reichel wrote:

Hi,

On Thu, Sep 15, 2016 at 02:22:21PM +0300, Sakari Ailus wrote:

Initialise the sensor in probe. The reason why it wasn't previously done
in case of platform data was that the probe() of the driver that provided
the clock through the set_xclk() callback would need to finish before the
probe() function of the smiapp driver. The set_xclk() callback no longer
exists.

Signed-off-by: Sakari Ailus 
---
  drivers/media/i2c/smiapp/smiapp-core.c | 53 --
  1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 5d251b4..13322f3 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2530,8 +2530,19 @@ static int smiapp_register_subdev(struct smiapp_sensor 
*sensor,
return 0;
  }

-static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
+static void smiapp_cleanup(struct smiapp_sensor *sensor)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
+
+   device_remove_file(>dev, _attr_nvm);
+   device_remove_file(>dev, _attr_ident);
+
+   smiapp_free_controls(sensor);
+}
+
+static int smiapp_registered(struct v4l2_subdev *subdev)
  {
+   struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
int rval;

if (sensor->scaler) {
@@ -2540,23 +2551,18 @@ static int smiapp_register_subdevs(struct smiapp_sensor 
*sensor)
SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
if (rval < 0)
-   return rval;
+   goto out_err;
}

return smiapp_register_subdev(
sensor, sensor->pixel_array, sensor->binner,
SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);


I guess you should also handle errors from the second
smiapp_register_subdev call?


Um, yes. Perhaps it'd be better just fix it here now that we still 
remember the problem. :-) I'll fix that for v2.





-}

-static void smiapp_cleanup(struct smiapp_sensor *sensor)
-{
-   struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
-
-   device_remove_file(>dev, _attr_nvm);
-   device_remove_file(>dev, _attr_ident);
+out_err:
+   smiapp_cleanup(sensor);

-   smiapp_free_controls(sensor);
+   return rval;
  }


-- Sebastian




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

Re: [PATCH v2 04/17] smiapp: Split off sub-device registration into two

[Sebastian Reichel]

Hi,

On Mon, Sep 19, 2016 at 11:50:02PM +0300, Sakari Ailus wrote:
> Hi Sebastian,
> 
> Sebastian Reichel wrote:
> > Hi,
> > 
> > On Thu, Sep 15, 2016 at 02:22:18PM +0300, Sakari Ailus wrote:
> > > Remove the loop in sub-device registration and create each sub-device
> > > explicitly instead.
> > 
> > Reviewed-By: Sebastian Reichel 
> 
> Thanks!
> 
> > 
> > > +static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
> > > +{
> > > + int rval;
> > > +
> > > + if (sensor->scaler) {
> > > + rval = smiapp_register_subdev(
> > > + sensor, sensor->binner, sensor->scaler,
> > > + SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
> > > + MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> > > + if (rval < 0)
> > >   return rval;
> > > - }
> > >   }
> > > 
> > > - return 0;
> > > + return smiapp_register_subdev(
> > > + sensor, sensor->pixel_array, sensor->binner,
> > > + SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
> > > + MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> > >   }
> > 
> > I haven't looked at the remaining code, but is sensor->scaler
> > stuff being cleaned up properly if the binner part fails?
> 
> That's a very good question. I don't think it is. But that's how
> the code has always been 

Yes, it's not a regression introduced by this patch, that's why I
gave Reviewed-By ;)

> --- there are issues left to be resolved if registered() fails for
> a reason or another. For instance, removing and reloading the
> omap3-isp module will cause a failure in the smiapp driver unless
> it's unloaded as well.
>
> I think I prefer to fix that in a different patch(set) as this one
> is quite large already.

ok.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 07/17] smiapp: Always initialise the sensor in probe

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:21PM +0300, Sakari Ailus wrote:
> Initialise the sensor in probe. The reason why it wasn't previously done
> in case of platform data was that the probe() of the driver that provided
> the clock through the set_xclk() callback would need to finish before the
> probe() function of the smiapp driver. The set_xclk() callback no longer
> exists.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 53 
> --
>  1 file changed, 19 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index 5d251b4..13322f3 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2530,8 +2530,19 @@ static int smiapp_register_subdev(struct smiapp_sensor 
> *sensor,
>   return 0;
>  }
>  
> -static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
> +static void smiapp_cleanup(struct smiapp_sensor *sensor)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
> +
> + device_remove_file(>dev, _attr_nvm);
> + device_remove_file(>dev, _attr_ident);
> +
> + smiapp_free_controls(sensor);
> +}
> +
> +static int smiapp_registered(struct v4l2_subdev *subdev)
>  {
> + struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
>   int rval;
>  
>   if (sensor->scaler) {
> @@ -2540,23 +2551,18 @@ static int smiapp_register_subdevs(struct 
> smiapp_sensor *sensor)
>   SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
>   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
>   if (rval < 0)
> - return rval;
> + goto out_err;
>   }
>  
>   return smiapp_register_subdev(
>   sensor, sensor->pixel_array, sensor->binner,
>   SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
>   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);

I guess you should also handle errors from the second
smiapp_register_subdev call?

> -}
>  
> -static void smiapp_cleanup(struct smiapp_sensor *sensor)
> -{
> - struct i2c_client *client = v4l2_get_subdevdata(>src->sd);
> -
> - device_remove_file(>dev, _attr_nvm);
> - device_remove_file(>dev, _attr_ident);
> +out_err:
> + smiapp_cleanup(sensor);
>  
> - smiapp_free_controls(sensor);
> + return rval;
>  }

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 01/17] smiapp: Move sub-device initialisation into a separate function

[Sakari Ailus]

Hi Sebastian,

Thank you for the review!

Sebastian Reichel wrote:

Hi,

On Thu, Sep 15, 2016 at 02:22:15PM +0300, Sakari Ailus wrote:

Simplify smiapp_init() by moving the initialisation of individual
sub-devices to a separate function.


Reviewed-By: Sebastian Reichel 


Signed-off-by: Sakari Ailus 
---
  drivers/media/i2c/smiapp/smiapp-core.c | 108 +++--
  1 file changed, 49 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
b/drivers/media/i2c/smiapp/smiapp-core.c
index 44f8c7e..862017e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c

[...]

+   if (sensor->scaler)
+   smiapp_create_subdev(sensor, sensor->scaler, "scaler");


I would add the NULL check to smiapp_create_subdev.


I first thought I'd say it makes it cleaner what's optional and what's 
not. The same is however visible some ten--twenty lines above this code, 
so not really an argument for that. Will fix.





+   smiapp_create_subdev(sensor, sensor->binner, "binner");
+   smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array");

dev_dbg(>dev, "profile %d\n", sensor->minfo.smiapp_profile);



-- Sebastian




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

Re: [PATCH v2 04/17] smiapp: Split off sub-device registration into two

[Sakari Ailus]

Hi Sebastian,

Sebastian Reichel wrote:

Hi,

On Thu, Sep 15, 2016 at 02:22:18PM +0300, Sakari Ailus wrote:

Remove the loop in sub-device registration and create each sub-device
explicitly instead.


Reviewed-By: Sebastian Reichel 


Thanks!




+static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
+{
+   int rval;
+
+   if (sensor->scaler) {
+   rval = smiapp_register_subdev(
+   sensor, sensor->binner, sensor->scaler,
+   SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
+   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
+   if (rval < 0)
return rval;
-   }
}

-   return 0;
+   return smiapp_register_subdev(
+   sensor, sensor->pixel_array, sensor->binner,
+   SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
+   MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
  }


I haven't looked at the remaining code, but is sensor->scaler
stuff being cleaned up properly if the binner part fails?


That's a very good question. I don't think it is. But that's how the 
code has always been --- there are issues left to be resolved if 
registered() fails for a reason or another. For instance, removing and 
reloading the omap3-isp module will cause a failure in the smiapp driver 
unless it's unloaded as well.


I think I prefer to fix that in a different patch(set) as this one is 
quite large already.


--
Kind regards,

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

Re: [PATCH 1/6] arcmsr: use pci_alloc_irq_vectors

[Martin K. Petersen]

> "Christoph" == Christoph Hellwig  writes:

Christoph> Switch the arcmsr driver to use pci_alloc_irq_vectors.  We
Christoph> need to two calls to pci_alloc_irq_vectors as arcmsr only
Christoph> supports multiple MSI-X vectors, but not multiple MSI
Christoph> vectors.

Christoph> Otherwise this cleans up a lot of cruft and allows to use a
Christoph> common request_irq loop for irq types, which happens to only
Christoph> iterate over a single line in the non MSI-X case.

Ching: Please test and review. Thanks!

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

Re: [PATCH 2/6] ipr: use pci_irq_allocate_vectors

[Martin K. Petersen]

> "Christoph" == Christoph Hellwig  writes:

Christoph> Switch the ipr driver to use pci_alloc_irq_vectors.  We need
Christoph> to two calls to pci_alloc_irq_vectors as ipr only supports
Christoph> multiple MSI-X vectors, but not multiple MSI vectors.

Christoph> Otherwise this cleans up a lot of cruft and allows to use a
Christoph> common request_irq loop for irq types, which happens to only
Christoph> iterate over a single line in the non MSI-X case.

Brian, Gabriel: Please test and review. Thanks!

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

Re: [PATCH v2 06/17] smiapp: Remove unnecessary BUG_ON()'s

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:20PM +0300, Sakari Ailus wrote:
> Instead, calculate how much is needed and then allocate the memory
> dynamically.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 05/17] smiapp: Provide a common function to obtain native pixel array size

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:19PM +0300, Sakari Ailus wrote:
> The same pixel array size is required for the active format of each
> sub-device sink pad and try format of each sink pad of each opened file
> handle as well as for the native size rectangle.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 04/17] smiapp: Split off sub-device registration into two

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:18PM +0300, Sakari Ailus wrote:
> Remove the loop in sub-device registration and create each sub-device
> explicitly instead.

Reviewed-By: Sebastian Reichel 

> +static int smiapp_register_subdevs(struct smiapp_sensor *sensor)
> +{
> + int rval;
> +
> + if (sensor->scaler) {
> + rval = smiapp_register_subdev(
> + sensor, sensor->binner, sensor->scaler,
> + SMIAPP_PAD_SRC, SMIAPP_PAD_SINK,
> + MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
> + if (rval < 0)
>   return rval;
> - }
>   }
>  
> - return 0;
> + return smiapp_register_subdev(
> + sensor, sensor->pixel_array, sensor->binner,
> + SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK,
> + MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
>  }

I haven't looked at the remaining code, but is sensor->scaler
stuff being cleaned up properly if the binner part fails?

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 02/17] smiapp: Explicitly define number of pads in initialisation

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:16PM +0300, Sakari Ailus wrote:
> Define the number of pads explicitly in initialising the sub-devices.

Reviewed-By: Sebastian Reichel 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH v2 01/17] smiapp: Move sub-device initialisation into a separate function

[Sebastian Reichel]

Hi,

On Thu, Sep 15, 2016 at 02:22:15PM +0300, Sakari Ailus wrote:
> Simplify smiapp_init() by moving the initialisation of individual
> sub-devices to a separate function.

Reviewed-By: Sebastian Reichel 

> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 108 
> +++--
>  1 file changed, 49 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
> b/drivers/media/i2c/smiapp/smiapp-core.c
> index 44f8c7e..862017e 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
>
> [...]
>
> + if (sensor->scaler)
> + smiapp_create_subdev(sensor, sensor->scaler, "scaler");

I would add the NULL check to smiapp_create_subdev.

> + smiapp_create_subdev(sensor, sensor->binner, "binner");
> + smiapp_create_subdev(sensor, sensor->pixel_array, "pixel_array");
>  
>   dev_dbg(>dev, "profile %d\n", sensor->minfo.smiapp_profile);
> 

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH for v4.8] cx23885/saa7134: assign q->dev to the PCI device

[Marton Balint]

On Sun, 18 Sep 2016, Hans Verkuil wrote:


Fix a regression caused by commit 2bc46b3a (media/pci: convert drivers to use 
the
new vb2_queue dev field). Three places where q->dev should be set were missed, 
causing
a WARN.

Signed-off-by: Hans Verkuil 
Reported-by: Marton Balint 
---


Tested-by: Marton Balint 

Thanks, the patch indeed fixes the WARN, and dvbstream is now working 
properly.


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

Re: [PATCH] [media] vsp1: fix CodingStyle violations on multi-line comments

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 21:35:36 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> 
> On Monday 19 Sep 2016 15:26:19 Mauro Carvalho Chehab wrote:
> > Several multi-line comments added at the vsp1 patch series
> > violate the Kernel CodingStyle. Fix them.
> >
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> I prefer the current style but that seems to be a hopeless battle :-) I have 
> a 
> small comment, please see below.
> 
> > ---
> >  drivers/media/platform/vsp1/vsp1_bru.c|  3 ++-
> >  drivers/media/platform/vsp1/vsp1_clu.c|  3 ++-
> >  drivers/media/platform/vsp1/vsp1_dl.c | 21 ++---
> >  drivers/media/platform/vsp1/vsp1_drm.c|  3 ++-
> >  drivers/media/platform/vsp1/vsp1_entity.h |  2 +-
> >  drivers/media/platform/vsp1/vsp1_pipe.c   |  2 +-
> >  drivers/media/platform/vsp1/vsp1_rpf.c|  9 ++---
> >  drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 --
> >  drivers/media/platform/vsp1/vsp1_video.c  | 20 +---
> >  drivers/media/platform/vsp1/vsp1_wpf.c|  9 ++---
> >  10 files changed, 51 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_bru.c
> > b/drivers/media/platform/vsp1/vsp1_bru.c index 2f5788c1a5be..ee8355c28f94
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_bru.c
> > +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> > @@ -242,7 +242,8 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> > goto done;
> > }
> > 
> > -   /* The compose rectangle top left corner must be inside the output
> > +   /*
> > +* The compose rectangle top left corner must be inside the output
> >  * frame.
> >  */
> > format = vsp1_entity_get_pad_format(>entity, config,
> > diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> > b/drivers/media/platform/vsp1/vsp1_clu.c index f052abd05166..f2fb26e5ab4e
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_clu.c
> > +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> > @@ -224,7 +224,8 @@ static void clu_configure(struct vsp1_entity *entity,
> > 
> > switch (params) {
> > case VSP1_ENTITY_PARAMS_INIT: {
> > -   /* The format can't be changed during streaming, only verify   
> it
> > +   /*
> > +* The format can't be changed during streaming, only verify   
> it
> >  * at setup time and store the information internally for   
> future
> >  * runtime configuration calls.
> >  */
> > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> > b/drivers/media/platform/vsp1/vsp1_dl.c index 0af3e8fdc714..ad545aff4e35
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_dl.c
> > +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> > @@ -296,7 +296,8 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct
> > vsp1_dl_manager *dlm) dl = list_first_entry(>free, struct
> > vsp1_dl_list, list);
> > list_del(>list);
> > 
> > -   /* The display list chain must be initialised to ensure every
> > +   /*
> > +* The display list chain must be initialised to ensure every
> >  * display list can assert list_empty() if it is not in a   
> chain.
> >  */
> > INIT_LIST_HEAD(>chain);
> > @@ -315,7 +316,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
> > if (!dl)
> > return;
> > 
> > -   /* Release any linked display-lists which were chained for a single
> > +   /*
> > +* Release any linked display-lists which were chained for a single
> >  * hardware operation.
> >  */
> > if (dl->has_chain) {
> > @@ -325,7 +327,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
> > 
> > dl->has_chain = false;
> > 
> > -   /* We can't free fragments here as DMA memory can only be freed in
> > +   /*
> > +* We can't free fragments here as DMA memory can only be freed in
> >  * interruptible context. Move all fragments to the display list
> >  * manager's list of fragments to be freed, they will be
> >  * garbage-collected by the work queue.
> > @@ -437,7 +440,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list
> > *dl, bool is_last) struct vsp1_dl_body *dlb;
> > unsigned int num_lists = 0;
> > 
> > -   /* Fill the header with the display list bodies addresses and sizes.   
> The
> > +   /*
> > +* Fill the header with the display list bodies addresses and sizes.   
> The
> >  * address of the first body has already been filled when the display
> >  * list was allocated.
> >  */
> > @@ -456,7 +460,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list
> > *dl, bool is_last)
> > 
> > dl->header->num_lists = num_lists;
> > 
> > -   /* If this display list's chain is not empty, we are on a list, where
> > +   /*
> > +* If this display list's chain is not empty, we are on a list, where
> >  * the next item in the list is the display list entity which should   

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 21:33:13 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> > Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:  
> > > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:  
> > >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:  
> > >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> > >>> 0-255. The same result can be obtained by cropping on the RPF without
> > >>> any such restriction, this feature isn't useful. Disable it.
> > >>> 
> > >>> Signed-off-by: Laurent Pinchart
> > >>> 
> > >>> ---
> > >>> 
> > >>>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> > >>>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
> > >>>  2 files changed, 26 insertions(+), 29 deletions(-)
> > >>> 
> > >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > >>> 8cb87e96b78b..a3ace8df7f4d 100644
> > >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c  
> 
> [snip]
> 
> > >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> > >>> v4l2_subdev *subdev,
> > >>> struct v4l2_mbus_framefmt *format;
> > >>> int ret = 0;
> > >>> 
> > >>> -   /* Cropping is implemented on the sink pad. */
> > >>> -   if (sel->pad != RWPF_PAD_SINK)
> > >>> +   /* Cropping is only supported on the RPF and is implemented on
> > >>> the sink
> > >>> +* pad.
> > >>> +*/  
> > >> 
> > >> Please read CodingStyle and run checkpatch before sending stuff
> > >> upstream.
> > >> 
> > >> This violates the CodingStyle: it should be, instead:
> > >>  /*
> > >>   * foo
> > >>   * bar
> > >>   */  
> > > 
> > > But it's consistent with the coding style of this driver. I'm OK fixing
> > > it, but it should be done globally in that case.  
> > 
> > There are inconsistencies inside the driver too on multi-line
> > comments even without fixing the ones introduced on this series,
> > as, on several places, multi-line comments are correct:
> > 
> > drivers/media/platform/vsp1/vsp1_bru.c:/*
> > drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> > conversion, all sink and source formats must be
> > drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> > the first sink pad (pad 0) and propagate it
> > drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> > drivers/media/platform/vsp1/vsp1_bru.c- */
> > 
> > drivers/media/platform/vsp1/vsp1_dl.c:/*
> > drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> > object and allocate DMA memory for the body
> > drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> > is expected to have been initialized to
> > drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> > drivers/media/platform/vsp1/vsp1_dl.c- */
> > 
> > ...
> > 
> > I'll address the ones only the CodingStyle violation introduced by this
> > series. I'll leave for the vsp1 maintainers/developers.  
> 
> OK, I'll address that.
> 
> > Btw, there are several kernel-doc tags that use just:
> > /*
> >  ...
> >  */
> > 
> > instead of:
> > 
> > /**
> >  ...
> >  */
> > 
> > I suggest you to add the files/headers with kernel-doc markups on
> > a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
> > 
> > This way, you can validate that such documentation is correct,
> > and produce an auto-generated documentation for this driver.  
> 
> I've thought about it, but I don't think those comments should become part of 
> the kernel documentation. They're really about driver internals, and meant 
> for 
> the driver developers. In particular only a subset of the driver is 
> documented 
> that way, when I've considered that the code or structures were complex 
> enough 
> to need proper documentation. A generated doc would then be quite incomplete 
> and not very useful, the comments are meant to be read while working on the 
> code.

The v4l-drivers book is meant to have driver internals documentation,
and not the subsystem kAPI or uAPI.

I don't see any problems if you want to document just the more complex
functions/structs over the v4l-drivers/ book. Yet, as you already took
the time to write documentation for those functions, providing that the
kernel-doc markups are ok, a v4l-drivers/vsp1.rst file for it could be as
simple as (untested):


VSP1 driver
^^^

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_dl.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_drm.h

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.c

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_entity.h

.. kernel-doc:: drivers/media/platform/vsp1/vsp1_pipe.c

.. kernel-doc:: 

Re: [PATCH] [media] vsp1: fix CodingStyle violations on multi-line comments

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.


On Monday 19 Sep 2016 15:26:19 Mauro Carvalho Chehab wrote:
> Several multi-line comments added at the vsp1 patch series
> violate the Kernel CodingStyle. Fix them.
>
> Signed-off-by: Mauro Carvalho Chehab 

I prefer the current style but that seems to be a hopeless battle :-) I have a 
small comment, please see below.

> ---
>  drivers/media/platform/vsp1/vsp1_bru.c|  3 ++-
>  drivers/media/platform/vsp1/vsp1_clu.c|  3 ++-
>  drivers/media/platform/vsp1/vsp1_dl.c | 21 ++---
>  drivers/media/platform/vsp1/vsp1_drm.c|  3 ++-
>  drivers/media/platform/vsp1/vsp1_entity.h |  2 +-
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  2 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c|  9 ++---
>  drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 --
>  drivers/media/platform/vsp1/vsp1_video.c  | 20 +---
>  drivers/media/platform/vsp1/vsp1_wpf.c|  9 ++---
>  10 files changed, 51 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_bru.c
> b/drivers/media/platform/vsp1/vsp1_bru.c index 2f5788c1a5be..ee8355c28f94
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_bru.c
> +++ b/drivers/media/platform/vsp1/vsp1_bru.c
> @@ -242,7 +242,8 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
> goto done;
>   }
> 
> - /* The compose rectangle top left corner must be inside the output
> + /*
> +  * The compose rectangle top left corner must be inside the output
>* frame.
>*/
>   format = vsp1_entity_get_pad_format(>entity, config,
> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> b/drivers/media/platform/vsp1/vsp1_clu.c index f052abd05166..f2fb26e5ab4e
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -224,7 +224,8 @@ static void clu_configure(struct vsp1_entity *entity,
> 
>   switch (params) {
>   case VSP1_ENTITY_PARAMS_INIT: {
> - /* The format can't be changed during streaming, only verify 
it
> + /*
> +  * The format can't be changed during streaming, only verify 
it
>* at setup time and store the information internally for 
future
>* runtime configuration calls.
>*/
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 0af3e8fdc714..ad545aff4e35
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -296,7 +296,8 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct
> vsp1_dl_manager *dlm) dl = list_first_entry(>free, struct
> vsp1_dl_list, list);
>   list_del(>list);
> 
> - /* The display list chain must be initialised to ensure every
> + /*
> +  * The display list chain must be initialised to ensure every
>* display list can assert list_empty() if it is not in a 
chain.
>*/
>   INIT_LIST_HEAD(>chain);
> @@ -315,7 +316,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
>   if (!dl)
>   return;
> 
> - /* Release any linked display-lists which were chained for a single
> + /*
> +  * Release any linked display-lists which were chained for a single
>* hardware operation.
>*/
>   if (dl->has_chain) {
> @@ -325,7 +327,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
> 
>   dl->has_chain = false;
> 
> - /* We can't free fragments here as DMA memory can only be freed in
> + /*
> +  * We can't free fragments here as DMA memory can only be freed in
>* interruptible context. Move all fragments to the display list
>* manager's list of fragments to be freed, they will be
>* garbage-collected by the work queue.
> @@ -437,7 +440,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list
> *dl, bool is_last) struct vsp1_dl_body *dlb;
>   unsigned int num_lists = 0;
> 
> - /* Fill the header with the display list bodies addresses and sizes. 
The
> + /*
> +  * Fill the header with the display list bodies addresses and sizes. 
The
>* address of the first body has already been filled when the display
>* list was allocated.
>*/
> @@ -456,7 +460,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list
> *dl, bool is_last)
> 
>   dl->header->num_lists = num_lists;
> 
> - /* If this display list's chain is not empty, we are on a list, where
> + /*
> +  * If this display list's chain is not empty, we are on a list, where
>* the next item in the list is the display list entity which should 
be
>* automatically queued by the hardware.
>*/
> @@ -482,7 +487,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
>   if (dl->dlm->mode == VSP1_DL_MODE_HEADER) {
>   struct vsp1_dl_list *dl_child;
> 
> - 

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Laurent Pinchart]

Hi Mauro,

On Monday 19 Sep 2016 15:26:15 Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 20:59:56 +0300 Laurent Pinchart escreveu:
> > On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> >> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> >>> Cropping on the WPF sink pad restricts the left and top coordinates to
> >>> 0-255. The same result can be obtained by cropping on the RPF without
> >>> any such restriction, this feature isn't useful. Disable it.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> 
> >>> ---
> >>> 
> >>>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> >>>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
> >>>  2 files changed, 26 insertions(+), 29 deletions(-)
> >>> 
> >>> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> >>> 8cb87e96b78b..a3ace8df7f4d 100644
> >>> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> >>> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c

[snip]

> >>> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct
> >>> v4l2_subdev *subdev,
> >>>   struct v4l2_mbus_framefmt *format;
> >>>   int ret = 0;
> >>> 
> >>> - /* Cropping is implemented on the sink pad. */
> >>> - if (sel->pad != RWPF_PAD_SINK)
> >>> + /* Cropping is only supported on the RPF and is implemented on
> >>> the sink
> >>> +  * pad.
> >>> +  */
> >> 
> >> Please read CodingStyle and run checkpatch before sending stuff
> >> upstream.
> >> 
> >> This violates the CodingStyle: it should be, instead:
> >>/*
> >> * foo
> >> * bar
> >> */
> > 
> > But it's consistent with the coding style of this driver. I'm OK fixing
> > it, but it should be done globally in that case.
> 
> There are inconsistencies inside the driver too on multi-line
> comments even without fixing the ones introduced on this series,
> as, on several places, multi-line comments are correct:
> 
> drivers/media/platform/vsp1/vsp1_bru.c:/*
> drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format
> conversion, all sink and source formats must be
> drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on
> the first sink pad (pad 0) and propagate it
> drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
> drivers/media/platform/vsp1/vsp1_bru.c- */
> 
> drivers/media/platform/vsp1/vsp1_dl.c:/*
> drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body
> object and allocate DMA memory for the body
> drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object
> is expected to have been initialized to
> drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
> drivers/media/platform/vsp1/vsp1_dl.c- */
> 
> ...
> 
> I'll address the ones only the CodingStyle violation introduced by this
> series. I'll leave for the vsp1 maintainers/developers.

OK, I'll address that.

> Btw, there are several kernel-doc tags that use just:
>   /*
>...
>*/
> 
> instead of:
> 
>   /**
>...
>*/
> 
> I suggest you to add the files/headers with kernel-doc markups on
> a Documentation/media/v4l-drivers/vsp1.rst file, to be created.
> 
> This way, you can validate that such documentation is correct,
> and produce an auto-generated documentation for this driver.

I've thought about it, but I don't think those comments should become part of 
the kernel documentation. They're really about driver internals, and meant for 
the driver developers. In particular only a subset of the driver is documented 
that way, when I've considered that the code or structures were complex enough 
to need proper documentation. A generated doc would then be quite incomplete 
and not very useful, the comments are meant to be read while working on the 
code.

-- 
Regards,

Laurent Pinchart

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

[PATCH] [media] vsp1: fix CodingStyle violations on multi-line comments

[Mauro Carvalho Chehab]

Several multi-line comments added at the vsp1 patch series
violate the Kernel CodingStyle. Fix them.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/vsp1/vsp1_bru.c|  3 ++-
 drivers/media/platform/vsp1/vsp1_clu.c|  3 ++-
 drivers/media/platform/vsp1/vsp1_dl.c | 21 ++---
 drivers/media/platform/vsp1/vsp1_drm.c|  3 ++-
 drivers/media/platform/vsp1/vsp1_entity.h |  2 +-
 drivers/media/platform/vsp1/vsp1_pipe.c   |  2 +-
 drivers/media/platform/vsp1/vsp1_rpf.c|  9 ++---
 drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 --
 drivers/media/platform/vsp1/vsp1_video.c  | 20 +---
 drivers/media/platform/vsp1/vsp1_wpf.c|  9 ++---
 10 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index 2f5788c1a5be..ee8355c28f94 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -242,7 +242,8 @@ static int bru_set_selection(struct v4l2_subdev *subdev,
goto done;
}
 
-   /* The compose rectangle top left corner must be inside the output
+   /*
+* The compose rectangle top left corner must be inside the output
 * frame.
 */
format = vsp1_entity_get_pad_format(>entity, config,
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index f052abd05166..f2fb26e5ab4e 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -224,7 +224,8 @@ static void clu_configure(struct vsp1_entity *entity,
 
switch (params) {
case VSP1_ENTITY_PARAMS_INIT: {
-   /* The format can't be changed during streaming, only verify it
+   /*
+* The format can't be changed during streaming, only verify it
 * at setup time and store the information internally for future
 * runtime configuration calls.
 */
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 0af3e8fdc714..ad545aff4e35 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -296,7 +296,8 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct 
vsp1_dl_manager *dlm)
dl = list_first_entry(>free, struct vsp1_dl_list, list);
list_del(>list);
 
-   /* The display list chain must be initialised to ensure every
+   /*
+* The display list chain must be initialised to ensure every
 * display list can assert list_empty() if it is not in a chain.
 */
INIT_LIST_HEAD(>chain);
@@ -315,7 +316,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
if (!dl)
return;
 
-   /* Release any linked display-lists which were chained for a single
+   /*
+* Release any linked display-lists which were chained for a single
 * hardware operation.
 */
if (dl->has_chain) {
@@ -325,7 +327,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
 
dl->has_chain = false;
 
-   /* We can't free fragments here as DMA memory can only be freed in
+   /*
+* We can't free fragments here as DMA memory can only be freed in
 * interruptible context. Move all fragments to the display list
 * manager's list of fragments to be freed, they will be
 * garbage-collected by the work queue.
@@ -437,7 +440,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list 
*dl, bool is_last)
struct vsp1_dl_body *dlb;
unsigned int num_lists = 0;
 
-   /* Fill the header with the display list bodies addresses and sizes. The
+   /*
+* Fill the header with the display list bodies addresses and sizes. The
 * address of the first body has already been filled when the display
 * list was allocated.
 */
@@ -456,7 +460,8 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list 
*dl, bool is_last)
 
dl->header->num_lists = num_lists;
 
-   /* If this display list's chain is not empty, we are on a list, where
+   /*
+* If this display list's chain is not empty, we are on a list, where
 * the next item in the list is the display list entity which should be
 * automatically queued by the hardware.
 */
@@ -482,7 +487,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
if (dl->dlm->mode == VSP1_DL_MODE_HEADER) {
struct vsp1_dl_list *dl_child;
 
-   /* In header mode the caller guarantees that the hardware is
+   /*
+* In header mode the caller guarantees that the hardware is
 * idle at this point.
 */
 
@@ -495,7 +501,8 @@ void vsp1_dl_list_commit(struct 

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 20:59:56 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> > Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:  
> > > Cropping on the WPF sink pad restricts the left and top coordinates to
> > > 0-255. The same result can be obtained by cropping on the RPF without
> > > any such restriction, this feature isn't useful. Disable it.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > 
> > > ---
> > > 
> > >  drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> > >  + drivers/media/platform/vsp1/vsp1_wpf.c
> > >   | 18 +++-
> > >  2 files changed, 26 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > > 8cb87e96b78b..a3ace8df7f4d 100644
> > > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev  
> > > *subdev,>   
> > >   struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > >   struct v4l2_subdev_pad_config *config;
> > >   struct v4l2_mbus_framefmt *format;
> > > 
> > > - struct v4l2_rect *crop;
> > > 
> > >   int ret = 0;
> > >   
> > >   mutex_lock(>entity.lock);
> > > 
> > > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev  
> > > *subdev,>   
> > >   fmt->format = *format;
> > > 
> > > - /* Update the sink crop rectangle. */
> > > - crop = vsp1_rwpf_get_crop(rwpf, config);
> > > - crop->left = 0;
> > > - crop->top = 0;
> > > - crop->width = fmt->format.width;
> > > - crop->height = fmt->format.height;
> > > + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > > + struct v4l2_rect *crop;
> > > +
> > > + /* Update the sink crop rectangle. */
> > > + crop = vsp1_rwpf_get_crop(rwpf, config);
> > > + crop->left = 0;
> > > + crop->top = 0;
> > > + crop->width = fmt->format.width;
> > > + crop->height = fmt->format.height;
> > > + }
> > > 
> > >   /* Propagate the format to the source pad. */
> > >   format = vsp1_entity_get_pad_format(>entity, config,
> > > 
> > > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct 
> > > v4l2_subdev  
> > > *subdev,>   
> > >   struct v4l2_mbus_framefmt *format;
> > >   int ret = 0;
> > > 
> > > - /* Cropping is implemented on the sink pad. */
> > > - if (sel->pad != RWPF_PAD_SINK)
> > > + /* Cropping is only supported on the RPF and is implemented on the   
> sink
> > > +  * pad.
> > > +  */  
> > 
> > Please read CodingStyle and run checkpatch before sending stuff upstream.
> > 
> > This violates the CodingStyle: it should be, instead:
> > /*
> >  * foo
> >  * bar
> >  */  
> 
> But it's consistent with the coding style of this driver. I'm OK fixing it, 
> but it should be done globally in that case.

There are inconsistencies inside the driver too on multi-line
comments even without fixing the ones introduced on this series,
as, on several places, multi-line comments are correct:

drivers/media/platform/vsp1/vsp1_bru.c:/*
drivers/media/platform/vsp1/vsp1_bru.c- * The BRU can't perform format 
conversion, all sink and source formats must be
drivers/media/platform/vsp1/vsp1_bru.c- * identical. We pick the format on the 
first sink pad (pad 0) and propagate it
drivers/media/platform/vsp1/vsp1_bru.c- * to all other pads.
drivers/media/platform/vsp1/vsp1_bru.c- */

drivers/media/platform/vsp1/vsp1_dl.c:/*
drivers/media/platform/vsp1/vsp1_dl.c- * Initialize a display list body object 
and allocate DMA memory for the body
drivers/media/platform/vsp1/vsp1_dl.c- * data. The display list body object is 
expected to have been initialized to
drivers/media/platform/vsp1/vsp1_dl.c- * 0 when allocated.
drivers/media/platform/vsp1/vsp1_dl.c- */

...

I'll address the ones only the CodingStyle violation introduced by this
series. I'll leave for the vsp1 maintainers/developers.

Btw, there are several kernel-doc tags that use just:
/*
 ...
 */

instead of:

/**
 ...
 */

I suggest you to add the files/headers with kernel-doc markups on
a Documentation/media/v4l-drivers/vsp1.rst file, to be created.

This way, you can validate that such documentation is correct,
and produce an auto-generated documentation for this driver.

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

Re: TW2866 i2c driver and solo6x10

[Andrey Utkin]

I'm going to have quite constrained time for participation in this
driver development, but still I think this is perspective project which
is in line with trend of exposing internal details of complex media
hardware for configuration by V4L2 framework. Also tw286x are used in
both tw5864 and solo6x10 boards, and in both cases it could be
controlled better from userspace.
I think first thing to do is expose tw286x chips as i2c- (or more
precisely SMBus-) controllable devices. I have accomplished that in some
way for tw5864, and hopefully I'll manage that for solo6x10.
But beyond that, I currently don't know.
A senior mentor would be very appreciated :)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Laurent Pinchart]

Hi Mauro,

On Monday 19 Sep 2016 14:55:43 Mauro Carvalho Chehab wrote:
> Em Wed, 14 Sep 2016 02:16:59 +0300 Laurent Pinchart escreveu:
> > Cropping on the WPF sink pad restricts the left and top coordinates to
> > 0-255. The same result can be obtained by cropping on the RPF without
> > any such restriction, this feature isn't useful. Disable it.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_rwpf.c | 37
> >  + drivers/media/platform/vsp1/vsp1_wpf.c
> >   | 18 +++-
> >  2 files changed, 26 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > b/drivers/media/platform/vsp1/vsp1_rwpf.c index
> > 8cb87e96b78b..a3ace8df7f4d 100644
> > --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> > +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> > @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,> 
> > struct vsp1_rwpf *rwpf = to_rwpf(subdev);
> > struct v4l2_subdev_pad_config *config;
> > struct v4l2_mbus_framefmt *format;
> > 
> > -   struct v4l2_rect *crop;
> > 
> > int ret = 0;
> > 
> > mutex_lock(>entity.lock);
> > 
> > @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev
> > *subdev,> 
> > fmt->format = *format;
> > 
> > -   /* Update the sink crop rectangle. */
> > -   crop = vsp1_rwpf_get_crop(rwpf, config);
> > -   crop->left = 0;
> > -   crop->top = 0;
> > -   crop->width = fmt->format.width;
> > -   crop->height = fmt->format.height;
> > +   if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> > +   struct v4l2_rect *crop;
> > +
> > +   /* Update the sink crop rectangle. */
> > +   crop = vsp1_rwpf_get_crop(rwpf, config);
> > +   crop->left = 0;
> > +   crop->top = 0;
> > +   crop->width = fmt->format.width;
> > +   crop->height = fmt->format.height;
> > +   }
> > 
> > /* Propagate the format to the source pad. */
> > format = vsp1_entity_get_pad_format(>entity, config,
> > 
> > @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev
> > *subdev,> 
> > struct v4l2_mbus_framefmt *format;
> > int ret = 0;
> > 
> > -   /* Cropping is implemented on the sink pad. */
> > -   if (sel->pad != RWPF_PAD_SINK)
> > +   /* Cropping is only supported on the RPF and is implemented on the 
sink
> > +* pad.
> > +*/
> 
> Please read CodingStyle and run checkpatch before sending stuff upstream.
> 
> This violates the CodingStyle: it should be, instead:
>   /*
>* foo
>* bar
>*/

But it's consistent with the coding style of this driver. I'm OK fixing it, 
but it should be done globally in that case.

> This time, I'll fix it, but next time I might not have enough time, and
> need to reject the patch series.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 06/13] v4l: vsp1: Disable cropping on WPF sink pad

[Mauro Carvalho Chehab]

Em Wed, 14 Sep 2016 02:16:59 +0300
Laurent Pinchart  escreveu:

> Cropping on the WPF sink pad restricts the left and top coordinates to
> 0-255. The same result can be obtained by cropping on the RPF without
> any such restriction, this feature isn't useful. Disable it.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/media/platform/vsp1/vsp1_rwpf.c | 37 
> +
>  drivers/media/platform/vsp1/vsp1_wpf.c  | 18 +++-
>  2 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c 
> b/drivers/media/platform/vsp1/vsp1_rwpf.c
> index 8cb87e96b78b..a3ace8df7f4d 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -66,7 +66,6 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
>   struct vsp1_rwpf *rwpf = to_rwpf(subdev);
>   struct v4l2_subdev_pad_config *config;
>   struct v4l2_mbus_framefmt *format;
> - struct v4l2_rect *crop;
>   int ret = 0;
>  
>   mutex_lock(>entity.lock);
> @@ -103,12 +102,16 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev 
> *subdev,
>  
>   fmt->format = *format;
>  
> - /* Update the sink crop rectangle. */
> - crop = vsp1_rwpf_get_crop(rwpf, config);
> - crop->left = 0;
> - crop->top = 0;
> - crop->width = fmt->format.width;
> - crop->height = fmt->format.height;
> + if (rwpf->entity.type == VSP1_ENTITY_RPF) {
> + struct v4l2_rect *crop;
> +
> + /* Update the sink crop rectangle. */
> + crop = vsp1_rwpf_get_crop(rwpf, config);
> + crop->left = 0;
> + crop->top = 0;
> + crop->width = fmt->format.width;
> + crop->height = fmt->format.height;
> + }
>  
>   /* Propagate the format to the source pad. */
>   format = vsp1_entity_get_pad_format(>entity, config,
> @@ -129,8 +132,10 @@ static int vsp1_rwpf_get_selection(struct v4l2_subdev 
> *subdev,
>   struct v4l2_mbus_framefmt *format;
>   int ret = 0;
>  
> - /* Cropping is implemented on the sink pad. */
> - if (sel->pad != RWPF_PAD_SINK)
> + /* Cropping is only supported on the RPF and is implemented on the sink
> +  * pad.
> +  */

Please read CodingStyle and run checkpatch before sending stuff upstream.

This violates the CodingStyle: it should be, instead:
/*
 * foo
 * bar
 */

This time, I'll fix it, but next time I might not have enough time, and
need to reject the patch series.

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

Re: [PATCH] [media] videodev2.h.rst.exceptions: fix warnings

[Hans Verkuil]

On 09/19/2016 07:32 PM, Mauro Carvalho Chehab wrote:
> Changeset ab6343956f9c ("[media] V4L2: Add documentation for SDI timings
> and related flags") added documentation for new V4L2 defines, but
> it forgot to update videodev2.h.rst.exceptions to point to where
> the documentation for those new values will be inside the book,
> causing those warnings:
> 
> Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
> v4l2-dv-bt-std-sdi (if the link has no caption the label must precede a 
> section header)
> Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
> v4l2-dv-fl-first-field-extra-line (if the link has no caption the label must 
> precede a section header)
> Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
> v4l2-in-st-no-v-lock (if the link has no caption the label must precede a 
> section header)
> Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
> v4l2-in-st-no-std-lock (if the link has no caption the label must precede a 
> section header)
> 
> Fixes: ab6343956f9c ("[media] V4L2: Add documentation for SDI timings and 
> related flags")
> 
> Cc: Charles-Antoine Couret 
> Cc: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Hans Verkuil 

I *know* I made the same change, but it looks like I may have forgotten to push 
to
my git branch :-(

Anyway, this is obviously correct.

Regards,

Hans

> ---
>  Documentation/media/videodev2.h.rst.exceptions | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/media/videodev2.h.rst.exceptions 
> b/Documentation/media/videodev2.h.rst.exceptions
> index 3828a2983acb..1d3f27d922b2 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -268,12 +268,14 @@ replace define V4L2_DV_BT_STD_CEA861 dv-bt-standards
>  replace define V4L2_DV_BT_STD_DMT dv-bt-standards
>  replace define V4L2_DV_BT_STD_CVT dv-bt-standards
>  replace define V4L2_DV_BT_STD_GTF dv-bt-standards
> +replace define V4L2_DV_BT_STD_SDI dv-bt-standards
>  
>  replace define V4L2_DV_FL_REDUCED_BLANKING dv-bt-standards
>  replace define V4L2_DV_FL_CAN_REDUCE_FPS dv-bt-standards
>  replace define V4L2_DV_FL_REDUCED_FPS dv-bt-standards
>  replace define V4L2_DV_FL_HALF_LINE dv-bt-standards
>  replace define V4L2_DV_FL_IS_CE_VIDEO dv-bt-standards
> +replace define V4L2_DV_FL_FIRST_FIELD_EXTRA_LINE dv-bt-standards
>  
>  replace define V4L2_DV_BT_656_1120 dv-timing-types
>  
> @@ -301,6 +303,8 @@ replace define V4L2_IN_ST_NO_CARRIER input-status
>  replace define V4L2_IN_ST_MACROVISION input-status
>  replace define V4L2_IN_ST_NO_ACCESS input-status
>  replace define V4L2_IN_ST_VTR input-status
> +replace define V4L2_IN_ST_NO_V_LOCK input-status
> +replace define V4L2_IN_ST_NO_STD_LOCK input-status
>  
>  replace define V4L2_IN_CAP_DV_TIMINGS input-capabilities
>  replace define V4L2_IN_CAP_STD input-capabilities
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] gs1662: make checkpatch happy

[Mauro Carvalho Chehab]

WARNING: line over 80 characters
+GS_HEIGHT_MAX, GS_PIXELCLOCK_MIN, 
GS_PIXELCLOCK_MAX,

WARNING: line over 80 characters
+   if (v4l2_match_dv_timings(timings, _fmt[i].format, 0, 
false))

WARNING: Block comments use a trailing */ on a separate line
+* which looks like a video signal activity.*/

WARNING: else is not generally useful after a break or return
+   return gs_write_register(gs->pdev, REG_FORCE_FMT, reg_value);
+   } else {

WARNING: Block comments use a trailing */ on a separate line
+* which looks like a video signal activity.*/

ERROR: spaces required around that '=' (ctx:VxW)
+   .enum_dv_timings= gs_enum_dv_timings,
^

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/spi/gs1662.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/media/spi/gs1662.c b/drivers/media/spi/gs1662.c
index f74342339e5a..d76f36233f43 100644
--- a/drivers/media/spi/gs1662.c
+++ b/drivers/media/spi/gs1662.c
@@ -134,7 +134,8 @@ static const struct v4l2_dv_timings_cap gs_timings_cap = {
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
V4L2_INIT_BT_TIMINGS(GS_WIDTH_MIN, GS_WIDTH_MAX, GS_HEIGHT_MIN,
-GS_HEIGHT_MAX, GS_PIXELCLOCK_MIN, 
GS_PIXELCLOCK_MAX,
+GS_HEIGHT_MAX, GS_PIXELCLOCK_MIN,
+GS_PIXELCLOCK_MAX,
 V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_SDI,
 V4L2_DV_BT_CAP_PROGRESSIVE
 | V4L2_DV_BT_CAP_INTERLACED)
@@ -237,7 +238,8 @@ static u16 get_register_timings(struct v4l2_dv_timings 
*timings)
int i;
 
for (i = 0; i < ARRAY_SIZE(reg_fmt); i++) {
-   if (v4l2_match_dv_timings(timings, _fmt[i].format, 0, 
false))
+   if (v4l2_match_dv_timings(timings, _fmt[i].format, 0,
+ false))
return reg_fmt[i].reg_value | MASK_FORCE_STD;
}
 
@@ -283,8 +285,10 @@ static int gs_query_dv_timings(struct v4l2_subdev *sd,
if (gs->enabled)
return -EBUSY;
 
-   /* Check if the component detect a line, a frame or something else
-* which looks like a video signal activity.*/
+   /*
+* Check if the component detect a line, a frame or something else
+* which looks like a video signal activity.
+*/
for (i = 0; i < 4; i++) {
gs_read_register(gs->pdev, REG_LINES_PER_FRAME + i, _value);
if (reg_value)
@@ -337,10 +341,10 @@ static int gs_s_stream(struct v4l2_subdev *sd, int enable)
/* To force the specific format */
reg_value = get_register_timings(>current_timings);
return gs_write_register(gs->pdev, REG_FORCE_FMT, reg_value);
-   } else {
-   /* To renable auto-detection mode */
-   return gs_write_register(gs->pdev, REG_FORCE_FMT, 0x0);
}
+
+   /* To renable auto-detection mode */
+   return gs_write_register(gs->pdev, REG_FORCE_FMT, 0x0);
 }
 
 static int gs_g_input_status(struct v4l2_subdev *sd, u32 *status)
@@ -349,8 +353,10 @@ static int gs_g_input_status(struct v4l2_subdev *sd, u32 
*status)
u16 reg_value, i;
int ret;
 
-   /* Check if the component detect a line, a frame or something else
-* which looks like a video signal activity.*/
+   /*
+* Check if the component detect a line, a frame or something else
+* which looks like a video signal activity.
+*/
for (i = 0; i < 4; i++) {
ret = gs_read_register(gs->pdev,
   REG_LINES_PER_FRAME + i, _value);
@@ -404,7 +410,7 @@ static const struct v4l2_subdev_video_ops gs_video_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops gs_pad_ops = {
-   .enum_dv_timings= gs_enum_dv_timings,
+   .enum_dv_timings = gs_enum_dv_timings,
.dv_timings_cap = gs_dv_timings_cap,
 };
 
-- 
2.7.4

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

[PATCH] [media] videodev2.h.rst.exceptions: fix warnings

[Mauro Carvalho Chehab]

Changeset ab6343956f9c ("[media] V4L2: Add documentation for SDI timings
and related flags") added documentation for new V4L2 defines, but
it forgot to update videodev2.h.rst.exceptions to point to where
the documentation for those new values will be inside the book,
causing those warnings:

Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
v4l2-dv-bt-std-sdi (if the link has no caption the label must precede a section 
header)
Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
v4l2-dv-fl-first-field-extra-line (if the link has no caption the label must 
precede a section header)
Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
v4l2-in-st-no-v-lock (if the link has no caption the label must precede a 
section header)
Documentation/output/videodev2.h.rst:6: WARNING: undefined label: 
v4l2-in-st-no-std-lock (if the link has no caption the label must precede a 
section header)

Fixes: ab6343956f9c ("[media] V4L2: Add documentation for SDI timings and 
related flags")

Cc: Charles-Antoine Couret 
Cc: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/videodev2.h.rst.exceptions | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/media/videodev2.h.rst.exceptions 
b/Documentation/media/videodev2.h.rst.exceptions
index 3828a2983acb..1d3f27d922b2 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -268,12 +268,14 @@ replace define V4L2_DV_BT_STD_CEA861 dv-bt-standards
 replace define V4L2_DV_BT_STD_DMT dv-bt-standards
 replace define V4L2_DV_BT_STD_CVT dv-bt-standards
 replace define V4L2_DV_BT_STD_GTF dv-bt-standards
+replace define V4L2_DV_BT_STD_SDI dv-bt-standards
 
 replace define V4L2_DV_FL_REDUCED_BLANKING dv-bt-standards
 replace define V4L2_DV_FL_CAN_REDUCE_FPS dv-bt-standards
 replace define V4L2_DV_FL_REDUCED_FPS dv-bt-standards
 replace define V4L2_DV_FL_HALF_LINE dv-bt-standards
 replace define V4L2_DV_FL_IS_CE_VIDEO dv-bt-standards
+replace define V4L2_DV_FL_FIRST_FIELD_EXTRA_LINE dv-bt-standards
 
 replace define V4L2_DV_BT_656_1120 dv-timing-types
 
@@ -301,6 +303,8 @@ replace define V4L2_IN_ST_NO_CARRIER input-status
 replace define V4L2_IN_ST_MACROVISION input-status
 replace define V4L2_IN_ST_NO_ACCESS input-status
 replace define V4L2_IN_ST_VTR input-status
+replace define V4L2_IN_ST_NO_V_LOCK input-status
+replace define V4L2_IN_ST_NO_STD_LOCK input-status
 
 replace define V4L2_IN_CAP_DV_TIMINGS input-capabilities
 replace define V4L2_IN_CAP_STD input-capabilities
-- 
2.7.4

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

Re: [PATCH v4 0/8] adv7180 subdev fixes, v4

[Hans Verkuil]

On 09/19/2016 04:19 PM, Jack Mitchell wrote:
> 
> 
> On 03/08/16 19:03, Steve Longerbeam wrote:
>> Steve Longerbeam (8):
>>   media: adv7180: fix field type
>>   media: adv7180: define more registers
>>   media: adv7180: add support for NEWAVMODE
>>   media: adv7180: add power pin control
>>   media: adv7180: implement g_parm
>>   media: adv7180: change mbus format to UYVY
>>   v4l: Add signal lock status to source change events
>>   media: adv7180: enable lock/unlock interrupts
>>
>>  .../devicetree/bindings/media/i2c/adv7180.txt  |   8 +
>>  Documentation/media/uapi/v4l/vidioc-dqevent.rst|   9 +
>>  Documentation/media/videodev2.h.rst.exceptions |   1 +
>>  drivers/media/i2c/Kconfig  |   2 +-
>>  drivers/media/i2c/adv7180.c| 200 
>> +
>>  include/uapi/linux/videodev2.h |   1 +
>>  6 files changed, 183 insertions(+), 38 deletions(-)
>>
> 
> Did anything come of this patchset, I see a few select patches from the 
> original (full imx6) series have been merged in but only seems partial?

I cherry-picked a few patches, but most are still in my TODO list.

I need time to carefully look at the interlaced/NEWAVMODE support. So those
patches won't make 4.9 but will be postponed for 4.10.

Regards,

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

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 13:36:55 +0200
Markus Heiser  escreveu:

> Hi Mauro, 
> 
> sorry for my late reply (so much work to do) ..
> 
> Am 09.09.2016 um 14:25 schrieb Markus Heiser :
> 
> >> Using either this approach or my kernel-doc patch, I'm now getting
> >> only two warnings:
> >> 
> >> 1) at media-entity.h, even without nitpick mode:
> >> 
> >> ./include/media/media-entity.h:1053: warning: No description found for 
> >> parameter '...'  
> 
> FYI: This message comes from the kernel-doc parser.
> 
> >> This is caused by this kernel-doc tag and the corresponding macro:
> >> 
> >>/**
> >> * media_entity_call - Calls a struct media_entity_operations operation 
> >> on
> >> *  an entity
> >> *
> >> * @entity: entity where the @operation will be called
> >> * @operation: type of the operation. Should be the name of a member of
> >> *  struct _entity_operations.
> >> *
> >> * This helper function will check if @operation is not %NULL. On such 
> >> case,
> >> * it will issue a call to @operation\(@entity, @args\).
> >> */
> >> 
> >>#define media_entity_call(entity, operation, args...)   
> >> \
> >>(((entity)->ops && (entity)->ops->operation) ?  
> >> \
> >> (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
> >> 
> >> 
> >> Basically, the Sphinx C domain seems to be expecting a description for
> >> "...". I didn't find any way to get rid of that.  
> 
> This is a bug in the kernel-doc parser.   The parser generates:
> 
>   .. c:function:: media_entity_call ( entity,  operation,  ...)
> 
> correct is:
> 
>   .. c:function::  media_entity_call( entity,  operation,  args...)
> 
> So both, the message and the wrong parse result comes from kernel-doc.

Ok, I'll try to address it by fixing kernel-doc script.

> 
> >> 
> >> 2) a nitpick warning at v4l2-mem2mem.h:
> >> 
> >> ./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not 
> >> found: queue_init  
> 
> FYI: this message comes from sphinx c-domain.
> 
> >>/**
> >> * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
> >> *
> >> * @m2m_dev: opaque pointer to the internal data to handle M2M context
> >> * @drv_priv: driver's instance private data
> >> * @queue_init: a callback for queue type-specific initialization 
> >> function
> >> *  to be used for initializing videobuf_queues
> >> *
> >> * Usually called from driver's ``open()`` function.
> >> */
> >>struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
> >>void *drv_priv,
> >>int (*queue_init)(void *priv, struct vb2_queue *src_vq, 
> >> struct vb2_queue *dst_vq));
> >> 
> >> I checked the output of kernel-doc, and it looked ok. Yet, it expects
> >> "queue_init" to be defined somehow. I suspect that this is an error at
> >> Sphinx C domain parser.  
> 
> Hmm, as far as I see, the output is not correct ... The output of
> functions with a function pointer argument are missing the 
> leading parenthesis in the function definition:
> 
>   .. c:function:: struct v4l2_m2m_ctx * v4l2_m2m_ctx_init (struct 
> v4l2_m2m_dev * m2m_dev, void * drv_priv, int (*queue_init) (void *priv, 
> struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> 
> The missing parenthesis cause the error message. 


Ah, OK! I'll kernel-doc and see what's happening here.

> 
> The output of the parameter description is:
> 
>   ``int (*)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) 
> queue_init``
> a callback for queue type-specific initialization function
> to be used for initializing videobuf_queues
> 
> Correct (and IMO better to read) is:
> 
>   .. c:function:: struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev 
> *m2m_dev, void *drv_priv, int (*queue_init)(void *priv, struct vb2_queue 
> *src_vq, struct vb2_queue *dst_vq))
> 
> and the parameter description should be something like ...
> 
>:param int (\*queue_init)(void \*priv, struct vb2_queue \*src_vq, struct 
> vb2_queue \*dst_vq):
> a callback for queue type-specific initialization function
> to be used for initializing videobuf_queues

I guess the better would be to strip the parameter type and output
it as:
queue_init
a callback for queue type-specific initialization function
to be used for initializing videobuf_queues

As I pointed before, the point is that such argument can easily have
more than 80 columns, with would cause troubles with LaTeX output,
as LaTeX doesn't break long verbatim text on multiple lines.

I submitted one patch fixing it. Not sure if it got merged by Jon
or not.

> 
> I tested this with my linuxdoc tools (parser) with I get no
> error messages from the sphinx c-domain.
> 
> BTW: 
> 
> The parser of my linuxdoc project is more strict and spit out some 
> 

Re: [PATCH v4 0/8] adv7180 subdev fixes, v4

[Jack Mitchell]

On 03/08/16 19:03, Steve Longerbeam wrote:

Steve Longerbeam (8):
  media: adv7180: fix field type
  media: adv7180: define more registers
  media: adv7180: add support for NEWAVMODE
  media: adv7180: add power pin control
  media: adv7180: implement g_parm
  media: adv7180: change mbus format to UYVY
  v4l: Add signal lock status to source change events
  media: adv7180: enable lock/unlock interrupts

 .../devicetree/bindings/media/i2c/adv7180.txt  |   8 +
 Documentation/media/uapi/v4l/vidioc-dqevent.rst|   9 +
 Documentation/media/videodev2.h.rst.exceptions |   1 +
 drivers/media/i2c/Kconfig  |   2 +-
 drivers/media/i2c/adv7180.c| 200 +
 include/uapi/linux/videodev2.h |   1 +
 6 files changed, 183 insertions(+), 38 deletions(-)



Did anything come of this patchset, I see a few select patches from the 
original (full imx6) series have been merged in but only seems partial?


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

Re: [v4l-utils PATCH 2/2] Add --with-static-binaries option to link binaries statically

[Sakari Ailus]

Hi Mauro,

On Mon, Sep 19, 2016 at 11:21:50AM -0300, Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 16:22:30 +0300
> Sakari Ailus  escreveu:
> 
> > Add a new variable STATIC_LDFLAGS to add the linker flags required for
> > static linking for each binary built.
> > 
> > Static and dynamic libraries are built by default but the binaries are
> > otherwise linked dynamically. --with-static-binaries requires that static
> > libraries are built.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  configure.ac  |  5 +
> >  contrib/gconv/Makefile.am |  4 ++--
> >  contrib/test/Makefile.am  |  8 
> >  lib/libv4l1/Makefile.am   |  2 +-
> >  lib/libv4l2/Makefile.am   |  2 +-
> >  utils/cec-compliance/Makefile.am  |  2 +-
> >  utils/cec-ctl/Makefile.am |  1 +
> >  utils/cec-follower/Makefile.am|  2 +-
> >  utils/cx18-ctl/Makefile.am|  1 +
> >  utils/decode_tm6000/Makefile.am   |  2 +-
> >  utils/dvb/Makefile.am | 10 +-
> >  utils/ir-ctl/Makefile.am  |  2 +-
> >  utils/ivtv-ctl/Makefile.am|  2 +-
> >  utils/keytable/Makefile.am|  2 +-
> >  utils/media-ctl/Makefile.am   |  1 +
> >  utils/qv4l2/Makefile.am   |  6 +++---
> >  utils/rds-ctl/Makefile.am |  2 +-
> >  utils/v4l2-compliance/Makefile.am |  1 +
> >  utils/v4l2-ctl/Makefile.am|  1 +
> >  utils/v4l2-sysfs-path/Makefile.am |  2 +-
> >  20 files changed, 34 insertions(+), 24 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 0d416b0..91597a4 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -427,6 +427,11 @@ AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_LIBV4L], [test 
> > x${enable_v4l2_compliance_li
> >  # append -static to libtool compile and link command to enforce static libs
> >  AS_IF([test x$enable_libdvbv5 = xno], [AC_SUBST([ENFORCE_LIBDVBV5_STATIC], 
> > ["-static"])])
> >  AS_IF([test x$enable_libv4l = xno],   [AC_SUBST([ENFORCE_LIBV4L_STATIC],   
> > ["-static"])])
> > +AC_ARG_WITH([static-binaries], AS_HELP_STRING([--with-static-binaries], 
> > [link binaries statically, requires static libraries to be built]))
> > +AS_IF([test x$with_static_binaries = xyes],
> > +  [AS_IF([test x$enable_static = xno],
> > +[AC_MSG_ERROR([--with-static-binaries requires --enable-static])])]
> > +  [AC_SUBST([STATIC_LDFLAGS], ["--static -static"])])
> >  
> >  # misc
> >  
> > diff --git a/contrib/gconv/Makefile.am b/contrib/gconv/Makefile.am
> > index 0e89f5b..2a39e5e 100644
> > --- a/contrib/gconv/Makefile.am
> > +++ b/contrib/gconv/Makefile.am
> > @@ -9,9 +9,9 @@ gconv_base_sources = iconv/skeleton.c iconv/loop.c
> >  arib-std-b24.c, en300-468-tab00.c: $(gconv_base_sources)
> >  
> >  ARIB_STD_B24_la_SOURCES = arib-std-b24.c jis0201.h jis0208.h jisx0213.h
> > -ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R 
> > @gconvsysdir@ -lJIS -lJISX0213
> > +ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R 
> > @gconvsysdir@ -lJIS -lJISX0213 $(STATIC_LDFLAGS)
> 
> Instead of adding STATIC_LDFLAGS to all LDFLAGS, wouldn't be better to
> add the flags to LDFLAGS on configure.ac?

That would affect libraries as well, which would break the build for shared
libraries, say, if you just pass --with-static-binaries, you'll get both
static and shared libraries as well as static binaries.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v4l-utils PATCH 2/2] Add --with-static-binaries option to link binaries statically

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 16:22:30 +0300
Sakari Ailus  escreveu:

> Add a new variable STATIC_LDFLAGS to add the linker flags required for
> static linking for each binary built.
> 
> Static and dynamic libraries are built by default but the binaries are
> otherwise linked dynamically. --with-static-binaries requires that static
> libraries are built.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  configure.ac  |  5 +
>  contrib/gconv/Makefile.am |  4 ++--
>  contrib/test/Makefile.am  |  8 
>  lib/libv4l1/Makefile.am   |  2 +-
>  lib/libv4l2/Makefile.am   |  2 +-
>  utils/cec-compliance/Makefile.am  |  2 +-
>  utils/cec-ctl/Makefile.am |  1 +
>  utils/cec-follower/Makefile.am|  2 +-
>  utils/cx18-ctl/Makefile.am|  1 +
>  utils/decode_tm6000/Makefile.am   |  2 +-
>  utils/dvb/Makefile.am | 10 +-
>  utils/ir-ctl/Makefile.am  |  2 +-
>  utils/ivtv-ctl/Makefile.am|  2 +-
>  utils/keytable/Makefile.am|  2 +-
>  utils/media-ctl/Makefile.am   |  1 +
>  utils/qv4l2/Makefile.am   |  6 +++---
>  utils/rds-ctl/Makefile.am |  2 +-
>  utils/v4l2-compliance/Makefile.am |  1 +
>  utils/v4l2-ctl/Makefile.am|  1 +
>  utils/v4l2-sysfs-path/Makefile.am |  2 +-
>  20 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 0d416b0..91597a4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -427,6 +427,11 @@ AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_LIBV4L], [test 
> x${enable_v4l2_compliance_li
>  # append -static to libtool compile and link command to enforce static libs
>  AS_IF([test x$enable_libdvbv5 = xno], [AC_SUBST([ENFORCE_LIBDVBV5_STATIC], 
> ["-static"])])
>  AS_IF([test x$enable_libv4l = xno],   [AC_SUBST([ENFORCE_LIBV4L_STATIC],   
> ["-static"])])
> +AC_ARG_WITH([static-binaries], AS_HELP_STRING([--with-static-binaries], 
> [link binaries statically, requires static libraries to be built]))
> +AS_IF([test x$with_static_binaries = xyes],
> +  [AS_IF([test x$enable_static = xno],
> +  [AC_MSG_ERROR([--with-static-binaries requires --enable-static])])]
> +  [AC_SUBST([STATIC_LDFLAGS], ["--static -static"])])
>  
>  # misc
>  
> diff --git a/contrib/gconv/Makefile.am b/contrib/gconv/Makefile.am
> index 0e89f5b..2a39e5e 100644
> --- a/contrib/gconv/Makefile.am
> +++ b/contrib/gconv/Makefile.am
> @@ -9,9 +9,9 @@ gconv_base_sources = iconv/skeleton.c iconv/loop.c
>  arib-std-b24.c, en300-468-tab00.c: $(gconv_base_sources)
>  
>  ARIB_STD_B24_la_SOURCES = arib-std-b24.c jis0201.h jis0208.h jisx0213.h
> -ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R @gconvsysdir@ 
> -lJIS -lJISX0213
> +ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R @gconvsysdir@ 
> -lJIS -lJISX0213 $(STATIC_LDFLAGS)

Instead of adding STATIC_LDFLAGS to all LDFLAGS, wouldn't be better to
add the flags to LDFLAGS on configure.ac?

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

Re: [v4l-utils PATCH 1/1] Fix static linking of v4l2-compliance and v4l2-ctl

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 16:21:30 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On 09/19/16 14:22, Mauro Carvalho Chehab wrote:
> > Em Mon, 19 Sep 2016 13:50:25 +0300
> > Sakari Ailus  escreveu:
> >   
> >> v4l2-compliance and v4l2-ctl depend on librt and libpthread. The symbols
> >> are found by the linker only if these libraries are specified after the
> >> objects that depend on them.
> >>
> >> As LDFLAGS variable end up expanded on libtool command line before LDADD,
> >> move the libraries to LDADD after local objects. -lpthread is added as on
> >> some systems librt depends on libpthread. This is the case on Ubuntu 16.04
> >> for instance.
> >>
> >> After this patch, creating a static build using the command
> >>
> >> LDFLAGS="--static -static" ./configure --disable-shared --enable-static  
> > 
> > It sounds weird to use LDFLAGS="--static -static" here, as the
> > configure options are already asking for static.
> > 
> > IMHO, the right way would be to change configure.ac to add those LDFLAGS
> > when --disable-shared is used.  
> 
> That's one option, but then shared libraries won't be built at all.

Well, my understanding is that  --disable-shared is meant to disable
building the shared library build :)

> I'm
> not sure what would be the use cases for that, though: static linking
> isn't very commonly needed except when you need to run the binaries
> elsewhere (for whatever reason) where you don't have the libraries you
> linked against available.

Yeah, that's the common usage. It is also interesting if someone
wants to build 2 versions of the same utility, each using a
different library, for testing purposes.

The usecase I can't see is to use --disable-shared but keeping
using the dynamic library for the exec files.

> That's still a separate issue from what this patch fixes.
> 
> Ideally it should be possible to link the binaries statically while
> still building shared libraries: both are built by default right now,
> yet shared libraries are always used for linking unless you disable
> shared libraries.

Well, the point is: if dynamic library build is disabled, it should
be doing static links that are produced by the build, instead of using
an already existing set of dynamic libraries present on the system
(that may not contain the symbols needed by the tool, or miss some
patches that were on -git).

> Most of the time this makes sense but not always.
> 
> I'm sending a separate patch to fix that by adding
> --with-static-binaries option.
> 



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

[v4l-utils PATCH 2/2] Add --with-static-binaries option to link binaries statically

[Sakari Ailus]

Add a new variable STATIC_LDFLAGS to add the linker flags required for
static linking for each binary built.

Static and dynamic libraries are built by default but the binaries are
otherwise linked dynamically. --with-static-binaries requires that static
libraries are built.

Signed-off-by: Sakari Ailus 
---
 configure.ac  |  5 +
 contrib/gconv/Makefile.am |  4 ++--
 contrib/test/Makefile.am  |  8 
 lib/libv4l1/Makefile.am   |  2 +-
 lib/libv4l2/Makefile.am   |  2 +-
 utils/cec-compliance/Makefile.am  |  2 +-
 utils/cec-ctl/Makefile.am |  1 +
 utils/cec-follower/Makefile.am|  2 +-
 utils/cx18-ctl/Makefile.am|  1 +
 utils/decode_tm6000/Makefile.am   |  2 +-
 utils/dvb/Makefile.am | 10 +-
 utils/ir-ctl/Makefile.am  |  2 +-
 utils/ivtv-ctl/Makefile.am|  2 +-
 utils/keytable/Makefile.am|  2 +-
 utils/media-ctl/Makefile.am   |  1 +
 utils/qv4l2/Makefile.am   |  6 +++---
 utils/rds-ctl/Makefile.am |  2 +-
 utils/v4l2-compliance/Makefile.am |  1 +
 utils/v4l2-ctl/Makefile.am|  1 +
 utils/v4l2-sysfs-path/Makefile.am |  2 +-
 20 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 0d416b0..91597a4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -427,6 +427,11 @@ AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_LIBV4L], [test 
x${enable_v4l2_compliance_li
 # append -static to libtool compile and link command to enforce static libs
 AS_IF([test x$enable_libdvbv5 = xno], [AC_SUBST([ENFORCE_LIBDVBV5_STATIC], 
["-static"])])
 AS_IF([test x$enable_libv4l = xno],   [AC_SUBST([ENFORCE_LIBV4L_STATIC],   
["-static"])])
+AC_ARG_WITH([static-binaries], AS_HELP_STRING([--with-static-binaries], [link 
binaries statically, requires static libraries to be built]))
+AS_IF([test x$with_static_binaries = xyes],
+  [AS_IF([test x$enable_static = xno],
+[AC_MSG_ERROR([--with-static-binaries requires --enable-static])])]
+  [AC_SUBST([STATIC_LDFLAGS], ["--static -static"])])
 
 # misc
 
diff --git a/contrib/gconv/Makefile.am b/contrib/gconv/Makefile.am
index 0e89f5b..2a39e5e 100644
--- a/contrib/gconv/Makefile.am
+++ b/contrib/gconv/Makefile.am
@@ -9,9 +9,9 @@ gconv_base_sources = iconv/skeleton.c iconv/loop.c
 arib-std-b24.c, en300-468-tab00.c: $(gconv_base_sources)
 
 ARIB_STD_B24_la_SOURCES = arib-std-b24.c jis0201.h jis0208.h jisx0213.h
-ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R @gconvsysdir@ 
-lJIS -lJISX0213
+ARIB_STD_B24_la_LDFLAGS = $(gconv_ldflags) -L@gconvsysdir@ -R @gconvsysdir@ 
-lJIS -lJISX0213 $(STATIC_LDFLAGS)
 
 EN300_468_TAB00_la_SOURCES = en300-468-tab00.c
-EN300_468_TAB00_la_LDFLAGS = $(gconv_ldflags)
+EN300_468_TAB00_la_LDFLAGS = $(gconv_ldflags) $(STATIC_LDFLAGS)
 
 EXTRA_DIST = $(gconv_base_sources) $(gconv_DATA) gconv.map
diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am
index 4641e21..7914f72 100644
--- a/contrib/test/Makefile.am
+++ b/contrib/test/Makefile.am
@@ -21,18 +21,18 @@ driver_test_LDADD = ../../utils/libv4l2util/libv4l2util.la
 
 pixfmt_test_SOURCES = pixfmt-test.c
 pixfmt_test_CFLAGS = $(X11_CFLAGS)
-pixfmt_test_LDFLAGS = $(X11_LIBS)
+pixfmt_test_LDFLAGS = $(X11_LIBS) $(STATIC_LDFLAGS)
 
 v4l2grab_SOURCES = v4l2grab.c
-v4l2grab_LDFLAGS = $(ARGP_LIBS)
+v4l2grab_LDFLAGS = $(ARGP_LIBS) $(STATIC_LDFLAGS)
 v4l2grab_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la -lpthread
 
 v4l2gl_SOURCES = v4l2gl.c
-v4l2gl_LDFLAGS = $(X11_LIBS) $(GL_LIBS) $(GLU_LIBS) $(ARGP_LIBS)
+v4l2gl_LDFLAGS = $(X11_LIBS) $(GL_LIBS) $(GLU_LIBS) $(ARGP_LIBS) 
$(STATIC_LDFLAGS)
 v4l2gl_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la
 
 mc_nextgen_test_CFLAGS = $(LIBUDEV_CFLAGS)
-mc_nextgen_test_LDFLAGS = $(LIBUDEV_LIBS)
+mc_nextgen_test_LDFLAGS = $(LIBUDEV_LIBS) $(STATIC_LDFLAGS)
 
 
 ioctl_test_SOURCES = ioctl-test.c ioctl-test.h ioctl_32.h ioctl_64.h
diff --git a/lib/libv4l1/Makefile.am b/lib/libv4l1/Makefile.am
index f768eaa..ea1fdf0 100644
--- a/lib/libv4l1/Makefile.am
+++ b/lib/libv4l1/Makefile.am
@@ -23,7 +23,7 @@ libv4l1_la_LIBADD = ../libv4l2/libv4l2.la
 v4l1compat_la_SOURCES = v4l1compat.c
 
 v4l1compat_la_LIBADD = libv4l1.la
-v4l1compat_la_LDFLAGS = -avoid-version -module -shared -export-dynamic
+v4l1compat_la_LDFLAGS = -avoid-version -module -shared -export-dynamic 
$(STATIC_LDFLAGS)
 v4l1compat_la_LIBTOOLFLAGS = --tag=disable-static
 
 EXTRA_DIST = libv4l1-kernelcode-license.txt
diff --git a/lib/libv4l2/Makefile.am b/lib/libv4l2/Makefile.am
index 1314a99..316d2e0 100644
--- a/lib/libv4l2/Makefile.am
+++ b/lib/libv4l2/Makefile.am
@@ -22,7 +22,7 @@ libv4l2_la_LIBADD = ../libv4lconvert/libv4lconvert.la
 
 v4l2convert_la_SOURCES = v4l2convert.c
 v4l2convert_la_LIBADD = libv4l2.la
-v4l2convert_la_LDFLAGS = -avoid-version -module -shared -export-dynamic
+v4l2convert_la_LDFLAGS = -avoid-version 

Re: [v4l-utils PATCH 1/1] Fix static linking of v4l2-compliance and v4l2-ctl

[Sakari Ailus]

Hi Mauro,

On 09/19/16 14:22, Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 13:50:25 +0300
> Sakari Ailus  escreveu:
> 
>> v4l2-compliance and v4l2-ctl depend on librt and libpthread. The symbols
>> are found by the linker only if these libraries are specified after the
>> objects that depend on them.
>>
>> As LDFLAGS variable end up expanded on libtool command line before LDADD,
>> move the libraries to LDADD after local objects. -lpthread is added as on
>> some systems librt depends on libpthread. This is the case on Ubuntu 16.04
>> for instance.
>>
>> After this patch, creating a static build using the command
>>
>> LDFLAGS="--static -static" ./configure --disable-shared --enable-static
> 
> It sounds weird to use LDFLAGS="--static -static" here, as the
> configure options are already asking for static.
> 
> IMHO, the right way would be to change configure.ac to add those LDFLAGS
> when --disable-shared is used.

That's one option, but then shared libraries won't be built at all. I'm
not sure what would be the use cases for that, though: static linking
isn't very commonly needed except when you need to run the binaries
elsewhere (for whatever reason) where you don't have the libraries you
linked against available.

That's still a separate issue from what this patch fixes.

Ideally it should be possible to link the binaries statically while
still building shared libraries: both are built by default right now,
yet shared libraries are always used for linking unless you disable
shared libraries. Most of the time this makes sense but not always.

I'm sending a separate patch to fix that by adding
--with-static-binaries option.

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

[ANN] Codec & Request API Brainstorm meeting Oct 10 & 11

[Hans Verkuil]

On October 10 and 11 we will have a Codec & Request API brainstorm meeting.

It will be held in room 'Salon 13 Paris' located on level 1 of the Maritim 
Berlin Hotel
from 9am to 5pm each day.

The main reason for doing this is that we have at least two codec drivers that 
are
blocked due to the fact that the Request API has not been mainlined yet. Since 
all the
core developers that are involved in this work will be in Berlin for the ELCE 
we decided
that this is a good opportunity to brainstorm about what is needed to get the 
missing
functionality mainlined.

Please note that this is a brainstorm meeting and not a regular media summit 
meeting.

It is also an invite-only meeting, and I want to cap the number of attendees to 
10.

The current list of attendees is:

Sakari Ailus
Kieran Bingham
Lars-Peter Clausen (Monday only)
Nicolas Dufresne (on and off?)
Pawel Osciak
Laurent Pinchart
Maxime Ripard
Hans Verkuil

If you believe you should be invited and are not in this list, then send me an 
email.

If you are in this list, but are not able to join, or if you know you won't be 
able
to attend on certain days or times, then let me know as well.

I do not have an agenda, other than that we'll start with figuring out the 
current
status of the codec and request APIs, and we'll go from there.

Hopefully the end result of this meeting will be a roadmap of how and when to 
get
this mainlined, and who will do the various action items.

Regards,

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

[PATCH] [media] platform: pxa_camera: add VIDEO_V4L2 dependency

[Arnd Bergmann]

Moving the pxa_camera driver from soc_camera lots the implied
VIDEO_V4L2 Kconfig dependency, and building the driver without
V4L2 results in a kernel that cannot link:

drivers/media/platform/pxa_camera.o: In function `pxa_camera_remove':
pxa_camera.c:(.text.pxa_camera_remove+0x10): undefined reference to 
`v4l2_clk_unregister'
pxa_camera.c:(.text.pxa_camera_remove+0x18): undefined reference to 
`v4l2_device_unregister'
drivers/media/platform/pxa_camera.o: In function `pxa_camera_probe':
pxa_camera.c:(.text.pxa_camera_probe+0x458): undefined reference to 
`v4l2_of_parse_endpoint'
drivers/media/v4l2-core/videobuf2-core.o: In function `__enqueue_in_driver':
drivers/media/v4l2-core/videobuf2-core.o: In function `vb2_core_streamon':
videobuf2-core.c:(.text.vb2_core_streamon+0x1b4): undefined reference to 
`v4l_vb2q_enable_media_source'
drivers/media/v4l2-core/videobuf2-v4l2.o: In function `vb2_ioctl_reqbufs':
videobuf2-v4l2.c:(.text.vb2_ioctl_reqbufs+0xc): undefined reference to 
`video_devdata'

This adds back an explicit dependency.

Fixes: 3050b9985024 ("[media] media: platform: pxa_camera: move pxa_camera out 
of soc_camera")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/platform/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ce4a96fccc43..5ff803efdc03 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -93,7 +93,7 @@ config VIDEO_OMAP3_DEBUG
 
 config VIDEO_PXA27x
tristate "PXA27x Quick Capture Interface driver"
-   depends on VIDEO_DEV && HAS_DMA
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
depends on PXA27x || COMPILE_TEST
select VIDEOBUF2_DMA_SG
select SG_SPLIT
-- 
2.9.0

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

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Hi Mauro, 

sorry for my late reply (so much work to do) ..

Am 09.09.2016 um 14:25 schrieb Markus Heiser :

>> Using either this approach or my kernel-doc patch, I'm now getting
>> only two warnings:
>> 
>> 1) at media-entity.h, even without nitpick mode:
>> 
>> ./include/media/media-entity.h:1053: warning: No description found for 
>> parameter '...'

FYI: This message comes from the kernel-doc parser.

>> This is caused by this kernel-doc tag and the corresponding macro:
>> 
>>  /**
>>   * media_entity_call - Calls a struct media_entity_operations operation 
>> on
>>   *  an entity
>>   *
>>   * @entity: entity where the @operation will be called
>>   * @operation: type of the operation. Should be the name of a member of
>>   *  struct _entity_operations.
>>   *
>>   * This helper function will check if @operation is not %NULL. On such 
>> case,
>>   * it will issue a call to @operation\(@entity, @args\).
>>   */
>> 
>>  #define media_entity_call(entity, operation, args...)   
>> \
>>  (((entity)->ops && (entity)->ops->operation) ?  
>> \
>>   (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
>> 
>> 
>> Basically, the Sphinx C domain seems to be expecting a description for
>> "...". I didn't find any way to get rid of that.

This is a bug in the kernel-doc parser. The parser generates:

  .. c:function:: media_entity_call ( entity,  operation,  ...)

correct is:

  .. c:function::  media_entity_call( entity,  operation,  args...)

So both, the message and the wrong parse result comes from kernel-doc.

>> 
>> 2) a nitpick warning at v4l2-mem2mem.h:
>> 
>> ./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not 
>> found: queue_init

FYI: this message comes from sphinx c-domain.

>>  /**
>>   * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
>>   *
>>   * @m2m_dev: opaque pointer to the internal data to handle M2M context
>>   * @drv_priv: driver's instance private data
>>   * @queue_init: a callback for queue type-specific initialization 
>> function
>>   *  to be used for initializing videobuf_queues
>>   *
>>   * Usually called from driver's ``open()`` function.
>>   */
>>  struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>>  void *drv_priv,
>>  int (*queue_init)(void *priv, struct vb2_queue *src_vq, 
>> struct vb2_queue *dst_vq));
>> 
>> I checked the output of kernel-doc, and it looked ok. Yet, it expects
>> "queue_init" to be defined somehow. I suspect that this is an error at
>> Sphinx C domain parser.

Hmm, as far as I see, the output is not correct ... The output of
functions with a function pointer argument are missing the 
leading parenthesis in the function definition:

  .. c:function:: struct v4l2_m2m_ctx * v4l2_m2m_ctx_init (struct v4l2_m2m_dev 
* m2m_dev, void * drv_priv, int (*queue_init) (void *priv, struct vb2_queue 
*src_vq, struct vb2_queue *dst_vq)

The missing parenthesis cause the error message. 

The output of the parameter description is:

  ``int (*)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) 
queue_init``
a callback for queue type-specific initialization function
to be used for initializing videobuf_queues

Correct (and IMO better to read) is:

  .. c:function:: struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev 
*m2m_dev, void *drv_priv, int (*queue_init)(void *priv, struct vb2_queue 
*src_vq, struct vb2_queue *dst_vq))

and the parameter description should be something like ...

   :param int (\*queue_init)(void \*priv, struct vb2_queue \*src_vq, struct 
vb2_queue \*dst_vq):
a callback for queue type-specific initialization function
to be used for initializing videobuf_queues

I tested this with my linuxdoc tools (parser) with I get no
error messages from the sphinx c-domain.

BTW: 

The parser of my linuxdoc project is more strict and spit out some 
warnings, which are not detected by the kernel-doc parser from the
kernel source tree.

For your rework on kernel-doc comments, it might be helpful to see
those messages, so I recommend to install the linuxdoc package and
do some lint.

install: https://return42.github.io/linuxdoc/install.html
lint:https://return42.github.io/linuxdoc/cmd-line.html#kernel-lintdoc

E.g. if you want to lint your whole include/media tree type:

  kernel-lintdoc [--sloppy] include/media


-- Markus --

>> 
>> Markus,
>> 
>> Could you please take a look on those?
> 
> Yes, I will give it a try, but I don't know if I find the time
> today.
> 
> On wich branch could I test this?
> 
> -- Markus --
> 
>> 
>> Thanks,
>> Mauro

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

Re: [v4l-utils PATCH 1/1] Fix static linking of v4l2-compliance and v4l2-ctl

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 13:50:25 +0300
Sakari Ailus  escreveu:

> v4l2-compliance and v4l2-ctl depend on librt and libpthread. The symbols
> are found by the linker only if these libraries are specified after the
> objects that depend on them.
> 
> As LDFLAGS variable end up expanded on libtool command line before LDADD,
> move the libraries to LDADD after local objects. -lpthread is added as on
> some systems librt depends on libpthread. This is the case on Ubuntu 16.04
> for instance.
> 
> After this patch, creating a static build using the command
> 
> LDFLAGS="--static -static" ./configure --disable-shared --enable-static

It sounds weird to use LDFLAGS="--static -static" here, as the
configure options are already asking for static.

IMHO, the right way would be to change configure.ac to add those LDFLAGS
when --disable-shared is used.

Regards,
Mauro

> 
> works again.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi,
> 
> This patch fixes the static build; the error is:
> 
>   CXXLDv4l2-compliance
> ../../lib/libv4l2/.libs/libv4l2.a(libv4l2_la-v4l2-plugin.o): In function 
> `v4l2_plugin_init':
> /home/sailus/a/v4l-utils/lib/libv4l2/v4l2-plugin.c:75: warning: Using 
> 'dlopen' in statically linked applications requires at runtime the shared 
> libraries from the glibc version used for linking
> /home/sailus/a/v4l-utils/lib/libv4lconvert/.libs/libv4lconvert.a(libv4lconvert_la-libv4lcontrol.o):
>  In function `v4lcontrol_create':
> /home/sailus/a/v4l-utils/lib/libv4lconvert/control/libv4lcontrol.c:693: 
> warning: Using 'getpwuid_r' in statically linked applications requires at 
> runtime the shared libraries from the glibc version used for linking
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(shm_open.o):
>  In function `shm_open':
> (.text+0x1f): undefined reference to `__shm_directory'
> collect2: error: ld returned 1 exit status
> distcc[21660] ERROR: compile (null) on localhost failed
> Makefile:559: recipe for target 'v4l2-compliance' failed
> 
> Kind regards,
> Sakari
> 
>  utils/v4l2-compliance/Makefile.am | 4 ++--
>  utils/v4l2-ctl/Makefile.am| 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/Makefile.am 
> b/utils/v4l2-compliance/Makefile.am
> index a895e8e..c2b5919 100644
> --- a/utils/v4l2-compliance/Makefile.am
> +++ b/utils/v4l2-compliance/Makefile.am
> @@ -5,12 +5,12 @@ DEFS :=
>  v4l2_compliance_SOURCES = v4l2-compliance.cpp v4l2-test-debug.cpp 
> v4l2-test-input-output.cpp \
>   v4l2-test-controls.cpp v4l2-test-io-config.cpp v4l2-test-formats.cpp 
> v4l2-test-buffers.cpp \
>   v4l2-test-codecs.cpp v4l2-test-colors.cpp v4l2-compliance.h
> -v4l2_compliance_LDFLAGS = -lrt
>  v4l2_compliance_CPPFLAGS = -I../common
>  
>  if WITH_V4L2_COMPLIANCE_LIBV4L
> -v4l2_compliance_LDADD = ../../lib/libv4l2/libv4l2.la 
> ../../lib/libv4lconvert/libv4lconvert.la
> +v4l2_compliance_LDADD = ../../lib/libv4l2/libv4l2.la 
> ../../lib/libv4lconvert/libv4lconvert.la -lrt -lpthread
>  else
> +v4l2_compliance_LDADD = -lrt -lpthread
>  DEFS += -DNO_LIBV4L2
>  endif
>  
> diff --git a/utils/v4l2-ctl/Makefile.am b/utils/v4l2-ctl/Makefile.am
> index 56943cd..2a05644 100644
> --- a/utils/v4l2-ctl/Makefile.am
> +++ b/utils/v4l2-ctl/Makefile.am
> @@ -7,11 +7,10 @@ v4l2_ctl_SOURCES = v4l2-ctl.cpp v4l2-ctl.h 
> v4l2-ctl-common.cpp v4l2-ctl-tuner.cp
>   v4l2-ctl-overlay.cpp v4l2-ctl-vbi.cpp v4l2-ctl-selection.cpp 
> v4l2-ctl-misc.cpp \
>   v4l2-ctl-streaming.cpp v4l2-ctl-sdr.cpp v4l2-ctl-edid.cpp 
> v4l2-ctl-modes.cpp \
>   v4l2-tpg-colors.c v4l2-tpg-core.c v4l-stream.c
> -v4l2_ctl_LDFLAGS = -lrt
>  v4l2_ctl_CPPFLAGS = -I../common
>  
>  if WITH_V4L2_CTL_LIBV4L
> -v4l2_ctl_LDADD = ../../lib/libv4l2/libv4l2.la 
> ../../lib/libv4lconvert/libv4lconvert.la
> +v4l2_ctl_LDADD = ../../lib/libv4l2/libv4l2.la 
> ../../lib/libv4lconvert/libv4lconvert.la -lrt -lpthread
>  else
>  DEFS += -DNO_LIBV4L2
>  endif



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

[v4l-utils PATCH 1/1] Fix static linking of v4l2-compliance and v4l2-ctl

[Sakari Ailus]

v4l2-compliance and v4l2-ctl depend on librt and libpthread. The symbols
are found by the linker only if these libraries are specified after the
objects that depend on them.

As LDFLAGS variable end up expanded on libtool command line before LDADD,
move the libraries to LDADD after local objects. -lpthread is added as on
some systems librt depends on libpthread. This is the case on Ubuntu 16.04
for instance.

After this patch, creating a static build using the command

LDFLAGS="--static -static" ./configure --disable-shared --enable-static

works again.

Signed-off-by: Sakari Ailus 
---
Hi,

This patch fixes the static build; the error is:

  CXXLDv4l2-compliance
../../lib/libv4l2/.libs/libv4l2.a(libv4l2_la-v4l2-plugin.o): In function 
`v4l2_plugin_init':
/home/sailus/a/v4l-utils/lib/libv4l2/v4l2-plugin.c:75: warning: Using 'dlopen' 
in statically linked applications requires at runtime the shared libraries from 
the glibc version used for linking
/home/sailus/a/v4l-utils/lib/libv4lconvert/.libs/libv4lconvert.a(libv4lconvert_la-libv4lcontrol.o):
 In function `v4lcontrol_create':
/home/sailus/a/v4l-utils/lib/libv4lconvert/control/libv4lcontrol.c:693: 
warning: Using 'getpwuid_r' in statically linked applications requires at 
runtime the shared libraries from the glibc version used for linking
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/librt.a(shm_open.o): 
In function `shm_open':
(.text+0x1f): undefined reference to `__shm_directory'
collect2: error: ld returned 1 exit status
distcc[21660] ERROR: compile (null) on localhost failed
Makefile:559: recipe for target 'v4l2-compliance' failed

Kind regards,
Sakari

 utils/v4l2-compliance/Makefile.am | 4 ++--
 utils/v4l2-ctl/Makefile.am| 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/utils/v4l2-compliance/Makefile.am 
b/utils/v4l2-compliance/Makefile.am
index a895e8e..c2b5919 100644
--- a/utils/v4l2-compliance/Makefile.am
+++ b/utils/v4l2-compliance/Makefile.am
@@ -5,12 +5,12 @@ DEFS :=
 v4l2_compliance_SOURCES = v4l2-compliance.cpp v4l2-test-debug.cpp 
v4l2-test-input-output.cpp \
v4l2-test-controls.cpp v4l2-test-io-config.cpp v4l2-test-formats.cpp 
v4l2-test-buffers.cpp \
v4l2-test-codecs.cpp v4l2-test-colors.cpp v4l2-compliance.h
-v4l2_compliance_LDFLAGS = -lrt
 v4l2_compliance_CPPFLAGS = -I../common
 
 if WITH_V4L2_COMPLIANCE_LIBV4L
-v4l2_compliance_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la
+v4l2_compliance_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la -lrt -lpthread
 else
+v4l2_compliance_LDADD = -lrt -lpthread
 DEFS += -DNO_LIBV4L2
 endif
 
diff --git a/utils/v4l2-ctl/Makefile.am b/utils/v4l2-ctl/Makefile.am
index 56943cd..2a05644 100644
--- a/utils/v4l2-ctl/Makefile.am
+++ b/utils/v4l2-ctl/Makefile.am
@@ -7,11 +7,10 @@ v4l2_ctl_SOURCES = v4l2-ctl.cpp v4l2-ctl.h 
v4l2-ctl-common.cpp v4l2-ctl-tuner.cp
v4l2-ctl-overlay.cpp v4l2-ctl-vbi.cpp v4l2-ctl-selection.cpp 
v4l2-ctl-misc.cpp \
v4l2-ctl-streaming.cpp v4l2-ctl-sdr.cpp v4l2-ctl-edid.cpp 
v4l2-ctl-modes.cpp \
v4l2-tpg-colors.c v4l2-tpg-core.c v4l-stream.c
-v4l2_ctl_LDFLAGS = -lrt
 v4l2_ctl_CPPFLAGS = -I../common
 
 if WITH_V4L2_CTL_LIBV4L
-v4l2_ctl_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la
+v4l2_ctl_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la -lrt -lpthread
 else
 DEFS += -DNO_LIBV4L2
 endif
-- 
2.7.4

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

Re: [PATCH v2 0/8] Qualcomm video decoder/encoder driver

[Hans Verkuil]

Hi Stanimir,

I've finished my review of this patch series.

I'll be traveling for the next three weeks, so you can take your time with
making a v3 since it is very unlikely I'll be able to review it before I'm
back mid-October.

Thanks for working on this!

Regards,

Hans

On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
> Changes since v1:
>   - s/ENOTSUPP/EINVAL in vidc_set_color_format()
>   - use video_device_alloc
>   - fill struct device pointer in vb2_queue_init instead of .setup_queue
>   - fill device_caps in struct video_device instead of .vidioc_querycap
>   - fill colorspace, ycbcr_enc, quantization and xfer_func only when type
> is V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
>   - delete rproc_boot|shutdown wrappers in core.c
>   - delete unused list_head in struct vidc_core
>   - delete mem.c|h and inline the function where they were used
>   - delete int_bufs.c|h files and move the functions in helpers.c, also
> cleanup the code by removing buffer reuse mechanism
>   - inline INIT_VIDC_LIST macro
>   - rename queue to other_queue in .start_streaming for encoder and
> decoder
>   - renamed vidc_buf_descs -> vidc_get_bufreq
>   - optimize instance checks in vidc_open
>   - moved resources structure in core.c and delete resources.c|h
>   - delete streamon and streamoff flags
> 
> The previous v1 can be found at [1].
> 
> [1] https://lkml.org/lkml/2016/8/22/308
> 
> The output of v4l2-compliance for decoder and encoder are:
> 
> root@dragonboard-410c:/home/linaro# ./v4l2-compliance -d /dev/video0
> v4l2-compliance SHA   : abc1453dfe89f244dccd3460d8e1a2e3091cbadb
> 
> Driver Info:
> Driver name   : vidc
> Card type : video decoder
> Bus info  : platform:vidc
> Driver version: 4.8.0
> Capabilities  : 0x84204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps   : 0x04204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> 
> Compliance test for device /dev/video0 (not using libv4l2):
> 
> Required ioctls:
> test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
> test second video open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
> test for unlimited opens: OK
> 
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> test VIDIOC_LOG_STATUS: OK (Not Supported)
> 
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 0 Audio Inputs: 0 Tuners: 0
> 
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
> 
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 7 Private Controls: 0
> 
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK
> 
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
> 
> Test input 0:
> 
> 
> Total: 43, Succeeded: 43, Failed: 0, Warnings: 0
> 
> 
> 

Re: [PATCH v2 7/8] media: vidc: add Makefiles and Kconfig files

[Hans Verkuil]

On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
> Makefile and Kconfig files to build the video codec driver.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/Kconfig   |  8 
>  drivers/media/platform/qcom/Makefile  |  6 ++
>  drivers/media/platform/qcom/vidc/Makefile | 15 +++
>  3 files changed, 29 insertions(+)
>  create mode 100644 drivers/media/platform/qcom/Kconfig
>  create mode 100644 drivers/media/platform/qcom/Makefile
>  create mode 100644 drivers/media/platform/qcom/vidc/Makefile
> 
> diff --git a/drivers/media/platform/qcom/Kconfig 
> b/drivers/media/platform/qcom/Kconfig
> new file mode 100644
> index ..4bad5c0f68e4
> --- /dev/null
> +++ b/drivers/media/platform/qcom/Kconfig
> @@ -0,0 +1,8 @@
> +comment "Qualcomm V4L2 drivers"
> +
> +menuconfig QCOM_VIDC
> +tristate "Qualcomm V4L2 encoder/decoder driver"
> +depends on ARCH_QCOM && VIDEO_V4L2
> +depends on IOMMU_DMA
> +depends on QCOM_VENUS_PIL
> +select VIDEOBUF2_DMA_SG

If at all possible, please depend on COMPILE_TEST as well!

Also missing: a patch adding an entry to the MAINTAINERS file.

Regards,

Hans

> diff --git a/drivers/media/platform/qcom/Makefile 
> b/drivers/media/platform/qcom/Makefile
> new file mode 100644
> index ..150892f6533b
> --- /dev/null
> +++ b/drivers/media/platform/qcom/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for the QCOM spcific video device drivers
> +# based on V4L2.
> +#
> +
> +obj-$(CONFIG_QCOM_VIDC) += vidc/
> diff --git a/drivers/media/platform/qcom/vidc/Makefile 
> b/drivers/media/platform/qcom/vidc/Makefile
> new file mode 100644
> index ..f8b5f9a438ee
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/Makefile
> @@ -0,0 +1,15 @@
> +# Makefile for Qualcomm vidc driver
> +
> +vidc-objs += \
> + core.o \
> + helpers.o \
> + vdec.o \
> + vdec_ctrls.o \
> + venc.o \
> + venc_ctrls.o \
> + hfi_venus.o \
> + hfi_msgs.o \
> + hfi_cmds.o \
> + hfi.o \
> +
> +obj-$(CONFIG_QCOM_VIDC) += vidc.o

I recommend renaming the module to qcom-vidc. 'vidc' is too generic.

Regards,

Hans

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

Re: [PATCH v2 4/8] media: vidc: encoder: add video encoder files

[Hans Verkuil]

Many of my review comments for the decoder apply to the encoder as well,
so I won't repeat those.

On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
> This adds encoder part of the driver plus encoder controls.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/vidc/venc.c   | 1252 
> +
>  drivers/media/platform/qcom/vidc/venc.h   |   29 +
>  drivers/media/platform/qcom/vidc/venc_ctrls.c |  396 
>  drivers/media/platform/qcom/vidc/venc_ctrls.h |   23 +
>  4 files changed, 1700 insertions(+)
>  create mode 100644 drivers/media/platform/qcom/vidc/venc.c
>  create mode 100644 drivers/media/platform/qcom/vidc/venc.h
>  create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.c
>  create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.h
> 
> diff --git a/drivers/media/platform/qcom/vidc/venc.c 
> b/drivers/media/platform/qcom/vidc/venc.c
> new file mode 100644
> index ..3b65f851a807
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/venc.c
> @@ -0,0 +1,1252 @@



> +static int venc_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + return -EINVAL;
> +}

Huh? Either remove this, or implement this correctly.



> +static int venc_subscribe_event(struct v4l2_fh *fh,
> + const struct  v4l2_event_subscription *sub)
> +{
> + switch (sub->type) {
> + case V4L2_EVENT_EOS:
> + return v4l2_event_subscribe(fh, sub, 2, NULL);
> + case V4L2_EVENT_SOURCE_CHANGE:
> + return v4l2_src_change_event_subscribe(fh, sub);

These two events aren't used in this driver AFAICT, so this can be dropped.

Since that leaves just V4L2_EVENT_CTRL this function can be replaced by
v4l2_ctrl_subscribe_event().

Regards,

Hans


> + case V4L2_EVENT_CTRL:
> + return v4l2_ctrl_subscribe_event(fh, sub);
> + default:
> + return -EINVAL;
> + }
> +}

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

Re: [PATCH v2 3/8] media: vidc: decoder: add video decoder files

[Hans Verkuil]

On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
> This consists of video decoder implementation plus decoder
> controls.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/vidc/vdec.c   | 1091 
> +
>  drivers/media/platform/qcom/vidc/vdec.h   |   29 +
>  drivers/media/platform/qcom/vidc/vdec_ctrls.c |  200 +
>  drivers/media/platform/qcom/vidc/vdec_ctrls.h |   21 +
>  4 files changed, 1341 insertions(+)
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
> 



> +static int vdec_event_notify(struct hfi_inst *hfi_inst, u32 event,
> +  struct hfi_event_data *data)
> +{
> + struct vidc_inst *inst = hfi_inst->ops_priv;
> + struct device *dev = inst->core->dev;
> + const struct v4l2_event ev = { .type = V4L2_EVENT_SOURCE_CHANGE };

1) this can be static
2) set the u.src_change.changes as well.


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

Re: [PATCH v2 3/8] media: vidc: decoder: add video decoder files

[Hans Verkuil]

On 09/07/2016 01:37 PM, Stanimir Varbanov wrote:
> This consists of video decoder implementation plus decoder
> controls.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/vidc/vdec.c   | 1091 
> +
>  drivers/media/platform/qcom/vidc/vdec.h   |   29 +
>  drivers/media/platform/qcom/vidc/vdec_ctrls.c |  200 +
>  drivers/media/platform/qcom/vidc/vdec_ctrls.h |   21 +
>  4 files changed, 1341 insertions(+)
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
>  create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
> 
> diff --git a/drivers/media/platform/qcom/vidc/vdec.c 
> b/drivers/media/platform/qcom/vidc/vdec.c
> new file mode 100644
> index ..433fe4f697d1
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/vdec.c
> @@ -0,0 +1,1091 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "core.h"
> +#include "helpers.h"
> +#include "vdec_ctrls.h"
> +
> +#define MACROBLOCKS_PER_PIXEL(16 * 16)
> +
> +static u32 get_framesize_nv12(int plane, u32 height, u32 width)
> +{
> + u32 y_stride, uv_stride, y_plane;
> + u32 y_sclines, uv_sclines, uv_plane;
> + u32 size;
> +
> + y_stride = ALIGN(width, 128);
> + uv_stride = ALIGN(width, 128);
> + y_sclines = ALIGN(height, 32);
> + uv_sclines = ALIGN(((height + 1) >> 1), 16);
> +
> + y_plane = y_stride * y_sclines;
> + uv_plane = uv_stride * uv_sclines + SZ_4K;
> + size = y_plane + uv_plane + SZ_8K;
> +
> + return ALIGN(size, SZ_4K);
> +}
> +
> +static u32 get_framesize_compressed(u32 mbs_per_frame)
> +{
> + return ((mbs_per_frame * MACROBLOCKS_PER_PIXEL * 3 / 2) / 2) + 128;
> +}
> +
> +static const struct vidc_format vdec_formats[] = {
> + {
> + .pixfmt = V4L2_PIX_FMT_NV12,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_MPEG4,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_MPEG2,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_H263,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_H264,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VP8,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_XVID,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + },
> +};
> +
> +static const struct vidc_format *find_format(u32 pixfmt, u32 type)
> +{
> + const struct vidc_format *fmt = vdec_formats;
> + unsigned int size = ARRAY_SIZE(vdec_formats);
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + if (fmt[i].pixfmt == pixfmt)
> + break;
> + }
> +
> + if (i == size || fmt[i].type != type)
> + return NULL;
> +
> + return [i];
> +}
> +
> +static const struct vidc_format *find_format_by_index(int index, u32 type)
> +{
> + const struct vidc_format *fmt = vdec_formats;
> + unsigned int size = ARRAY_SIZE(vdec_formats);
> + int i, k = 0;
> +
> + if (index < 0 || index > size)
> + return NULL;
> +
> + for (i = 0; i < size; i++) {
> + if (fmt[i].type != type)
> + continue;
> + if (k == index)
> + break;
> + k++;
> + }
> +
> + if (i == size)
> + return NULL;
> +
> + return [i];
> +}
> +
> +static int 

Re: [PATCH] pixfmt-reserved.rst: Improve MT21C documentation

[Tiffany Lin]

Hi Hans,

On Mon, 2016-09-19 at 09:22 +0200, Hans Verkuil wrote:
> Improve the MT21C documentation, making it clearer that this format requires 
> the MDP
> for further processing.
> 
> Also fix the fourcc (it was a fivecc :-) )
> 
reviewed-by: Tiffany Lin 

Thanks. I did not notice it become fivecc.

best regards,
Tiffany

> Signed-off-by: Hans Verkuil 
> ---
> diff --git a/Documentation/media/uapi/v4l/pixfmt-reserved.rst 
> b/Documentation/media/uapi/v4l/pixfmt-reserved.rst
> index 0989e99..a019f15 100644
> --- a/Documentation/media/uapi/v4l/pixfmt-reserved.rst
> +++ b/Documentation/media/uapi/v4l/pixfmt-reserved.rst
> @@ -343,13 +343,13 @@ please make a proposal on the linux-media mailing list.
> 
> -  ``V4L2_PIX_FMT_MT21C``
> 
> -   -  'MT21C'
> +   -  'MT21'
> 
> -  Compressed two-planar YVU420 format used by Mediatek MT8173.
>The compression is lossless.
> -  It is an opaque intermediate format, and MDP HW could convert
> -  V4L2_PIX_FMT_MT21C to V4L2_PIX_FMT_NV12M,
> -  V4L2_PIX_FMT_YUV420M and V4L2_PIX_FMT_YVU420.
> +  It is an opaque intermediate format and the MDP hardware must be
> +   used to convert ``V4L2_PIX_FMT_MT21C`` to ``V4L2_PIX_FMT_NV12M``,
> +  ``V4L2_PIX_FMT_YUV420M`` or ``V4L2_PIX_FMT_YVU420``.
> 
>  .. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> 


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

Re: [PATCH] mtk-mdp: fix double mutex_unlock

[Minghsiu Tsai]

On Mon, 2016-09-19 at 10:00 +0200, Hans Verkuil wrote:
> Fix smatch error:
> 
> media-git/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:100 
> mtk_mdp_vpu_send_msg() error: double unlock 'mutex:>mdp_dev->vpulock'
> 
> Signed-off-by: Hans Verkuil 


Reviewed-by: Minghsiu Tsai 

> ---
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c 
> b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> index 39188e5..4893825 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
> @@ -92,11 +92,9 @@ static int mtk_mdp_vpu_send_msg(void *msg, int len, struct 
> mtk_mdp_vpu *vpu,
> 
>   mutex_lock(>mdp_dev->vpulock);
>   err = vpu_ipi_send(vpu->pdev, (enum ipi_id)id, msg, len);
> - if (err) {
> - mutex_unlock(>mdp_dev->vpulock);
> + if (err)
>   dev_err(>mdp_dev->pdev->dev,
>   "vpu_ipi_send fail status %d\n", err);
> - }
>   mutex_unlock(>mdp_dev->vpulock);
> 
>   return err;
> 


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

[PATCH] mtk-mdp: fix double mutex_unlock

[Hans Verkuil]

Fix smatch error:

media-git/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c:100 
mtk_mdp_vpu_send_msg() error: double unlock 'mutex:>mdp_dev->vpulock'

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
index 39188e5..4893825 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
@@ -92,11 +92,9 @@ static int mtk_mdp_vpu_send_msg(void *msg, int len, struct 
mtk_mdp_vpu *vpu,

mutex_lock(>mdp_dev->vpulock);
err = vpu_ipi_send(vpu->pdev, (enum ipi_id)id, msg, len);
-   if (err) {
-   mutex_unlock(>mdp_dev->vpulock);
+   if (err)
dev_err(>mdp_dev->pdev->dev,
"vpu_ipi_send fail status %d\n", err);
-   }
mutex_unlock(>mdp_dev->vpulock);

return err;

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

[PATCH] pixfmt-reserved.rst: Improve MT21C documentation

[Hans Verkuil]

Improve the MT21C documentation, making it clearer that this format requires 
the MDP
for further processing.

Also fix the fourcc (it was a fivecc :-) )

Signed-off-by: Hans Verkuil 
---
diff --git a/Documentation/media/uapi/v4l/pixfmt-reserved.rst 
b/Documentation/media/uapi/v4l/pixfmt-reserved.rst
index 0989e99..a019f15 100644
--- a/Documentation/media/uapi/v4l/pixfmt-reserved.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-reserved.rst
@@ -343,13 +343,13 @@ please make a proposal on the linux-media mailing list.

-  ``V4L2_PIX_FMT_MT21C``

-   -  'MT21C'
+   -  'MT21'

-  Compressed two-planar YVU420 format used by Mediatek MT8173.
   The compression is lossless.
-  It is an opaque intermediate format, and MDP HW could convert
-  V4L2_PIX_FMT_MT21C to V4L2_PIX_FMT_NV12M,
-  V4L2_PIX_FMT_YUV420M and V4L2_PIX_FMT_YVU420.
+  It is an opaque intermediate format and the MDP hardware must be
+ used to convert ``V4L2_PIX_FMT_MT21C`` to ``V4L2_PIX_FMT_NV12M``,
+  ``V4L2_PIX_FMT_YUV420M`` or ``V4L2_PIX_FMT_YVU420``.

 .. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|

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

Re: [PATCH 1/3] ARM: dts: r8a7793: Enable VIN0, VIN1

[Geert Uytterhoeven]

Hi Ulrich,

On Fri, Sep 16, 2016 at 3:09 PM, Ulrich Hecht
 wrote:
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

> --- a/arch/arm/boot/dts/r8a7793.dtsi
> +++ b/arch/arm/boot/dts/r8a7793.dtsi
> @@ -30,6 +30,8 @@
> i2c7 = 
> i2c8 = 
> spi0 = 
> +   vin0 = 
> +   vin1 = 
> };
>
> cpus {
> @@ -852,6 +854,24 @@
> status = "disabled";
> };
>
> +   vin0: video@e6ef {

> +   vin1: video@e6ef1000 {

Any specific reason you didn't add vin2?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL v2 FOR v4.10] Media IOCTL handling rework

[Sakari Ailus]

Hi Mauro,

These four patches rework Media controller IOCTL handling for cleanups and  
preparation for variable sized IOCTL arguments. 

What's changed since the previous set is that the compat handling is kept
as-is, i.e. it simply calls the regular handler if the IOCTL is something
else than MEDIA_IOC_ENUM_LINKS32.

Please pull.


The following changes since commit c3b809834db8b1a8891c7ff873a216eac119628d:

  [media] pulse8-cec: fix compiler warning (2016-09-12 06:42:44 -0300)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git media-ioctl-rework

for you to fetch changes up to 267abb2f81a2457d0552eb1b3d988407f3f5eabc:

  media: Add flags to tell whether to take graph mutex for an IOCTL (2016-09-16 
12:36:08 +0300)


Sakari Ailus (4):
  media: Determine early whether an IOCTL is supported
  media: Unify IOCTL handler calling
  media: Refactor copying IOCTL arguments from and to user space
  media: Add flags to tell whether to take graph mutex for an IOCTL

 drivers/media/media-device.c | 224 +--
 1 file changed, 111 insertions(+), 113 deletions(-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] media: mtk-mdp: fix build error

[Minghsiu Tsai]

This patch fix build error without CONFIG_PM_RUNTIME
and CONFIG_PM_SLEEP

Signed-off-by: Minghsiu Tsai 
---
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index b0c421e..f4424064 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -233,7 +233,7 @@ static int mtk_mdp_remove(struct platform_device *pdev)
return 0;
 }
 
-#if defined(CONFIG_PM_RUNTIME) || defined(CONFIG_PM_SLEEP)
+#ifdef CONFIG_PM
 static int mtk_mdp_pm_suspend(struct device *dev)
 {
struct mtk_mdp_dev *mdp = dev_get_drvdata(dev);
@@ -251,7 +251,7 @@ static int mtk_mdp_pm_resume(struct device *dev)
 
return 0;
 }
-#endif /* CONFIG_PM_RUNTIME || CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
 
 #ifdef CONFIG_PM_SLEEP
 static int mtk_mdp_suspend(struct device *dev)
-- 
1.7.9.5

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

[PATCH 1/2] media: mtk-mdp: fix build warning in arch x86

[Minghsiu Tsai]

This patch fix build warning in arch x86

Signed-off-by: Minghsiu Tsai 
---
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c |1 +
 drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c |   10 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index a90972e..9a747e7 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
index fb07bf3..39188e5 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c
@@ -25,7 +25,8 @@ static inline struct mtk_mdp_ctx *vpu_to_ctx(struct 
mtk_mdp_vpu *vpu)
 
 static void mtk_mdp_vpu_handle_init_ack(struct mdp_ipi_comm_ack *msg)
 {
-   struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
+   struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)
+   (unsigned long)msg->ap_inst;
 
/* mapping VPU address to kernel virtual address */
vpu->vsi = (struct mdp_process_vsi *)
@@ -37,7 +38,8 @@ static void mtk_mdp_vpu_ipi_handler(void *data, unsigned int 
len, void *priv)
 {
unsigned int msg_id = *(unsigned int *)data;
struct mdp_ipi_comm_ack *msg = (struct mdp_ipi_comm_ack *)data;
-   struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)msg->ap_inst;
+   struct mtk_mdp_vpu *vpu = (struct mtk_mdp_vpu *)
+   (unsigned long)msg->ap_inst;
struct mtk_mdp_ctx *ctx;
 
vpu->failure = msg->status;
@@ -108,7 +110,7 @@ static int mtk_mdp_vpu_send_ap_ipi(struct mtk_mdp_vpu *vpu, 
uint32_t msg_id)
msg.msg_id = msg_id;
msg.ipi_id = IPI_MDP;
msg.vpu_inst_addr = vpu->inst_addr;
-   msg.ap_inst = (uint64_t)vpu;
+   msg.ap_inst = (unsigned long)vpu;
err = mtk_mdp_vpu_send_msg((void *), sizeof(msg), vpu, IPI_MDP);
if (!err && vpu->failure)
err = -EINVAL;
@@ -126,7 +128,7 @@ int mtk_mdp_vpu_init(struct mtk_mdp_vpu *vpu)
 
msg.msg_id = AP_MDP_INIT;
msg.ipi_id = IPI_MDP;
-   msg.ap_inst = (uint64_t)vpu;
+   msg.ap_inst = (unsigned long)vpu;
err = mtk_mdp_vpu_send_msg((void *), sizeof(msg), vpu, IPI_MDP);
if (!err && vpu->failure)
err = -EINVAL;
-- 
1.7.9.5

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

[PATCH 0/2] Fix build warning and error in Mediatek MDP driver

[Minghsiu Tsai]

- Fix build warning in arch x86 
- Fix build warning in kzalloc() and kfree() in arch x86 
- Fix build error without CONFIG_PM_RUNTIME and CONFIG_PM_SLEEP

v4l2-compliance test output:
v4l2-compliance SHA   : abc1453dfe89f244dccd3460d8e1a2e3091cbadb

Driver Info:
Driver name   : mtk-mdp
Card type : soc:mdp
Bus info  : platform:mt8173
Driver version: 4.8.0
Capabilities  : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format

Compliance test for device /dev/image-proc0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

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

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

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

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

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

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

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

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

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Test input 0:


Total: 43, Succeeded: 43, Failed: 0, Warnings: 0

Minghsiu Tsai (2):
  media: mtk-mdp: fix build warning in arch x86
  media: mtk-mdp: fix build error

 drivers/media/platform/mtk-mdp/mtk_mdp_core.c |4 ++--
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  |1 +
 drivers/media/platform/mtk-mdp/mtk_mdp_vpu.c  |   10 ++
 3 files changed, 9 insertions(+), 6 deletions(-)

-- 
1.7.9.5

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

Re: [PATCH] Potential fix for "[BUG] process stuck when closing saa7146 [dvb_ttpci]"

[Philipp Hahn]

Hello Andrey,

Am 16.09.2016 um 12:00 schrieb Andrey Utkin:
> Please try this patch. It is purely speculative as I don't have the hardware,
> but I hope my approach is right.

Thanks you for the patch; I've built a new kernel but didn't have the
time to test it yet; I'll mail you again as soon as I have tested it.

Thanks you for looking into that issues.

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

[PATCH] [media] st-hva: fix a copy-and-paste variable name error

[Colin King]

From: Colin Ian King 

The second check for an error on hva->lmi_err_reg appears
to be a copy-and-paste error, it should be hva->emi_err_reg
instead.

Signed-off-by: Colin Ian King 
---
 drivers/media/platform/sti/hva/hva-hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/hva/hva-hw.c 
b/drivers/media/platform/sti/hva/hva-hw.c
index d341d49..dcf362c 100644
--- a/drivers/media/platform/sti/hva/hva-hw.c
+++ b/drivers/media/platform/sti/hva/hva-hw.c
@@ -245,7 +245,7 @@ static irqreturn_t hva_hw_err_irq_thread(int irq, void *arg)
ctx->hw_err = true;
}
 
-   if (hva->lmi_err_reg) {
+   if (hva->emi_err_reg) {
dev_err(dev, "%s external memory interface error: 0x%08x\n",
ctx->name, hva->emi_err_reg);
ctx->hw_err = true;
-- 
2.9.3

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