cron job: media_tree daily build: ERRORS

[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:   Sun Nov 12 05:00:15 CET 2017
media-tree git hash:eb0c19942288569e0ae492476534d5a485fb8ab4
media_build git hash:   106c5880df126bdd61f8d1c1f59570524a95d77c
v4l-utils git hash: 7b2d48ff594dcc2c9b395463441e5abb4f5e9439
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-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: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-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: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v2] media: vsp1: Prevent suspending and resuming DRM pipelines

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wednesday, 20 September 2017 12:16:54 EET Kieran Bingham wrote:
> When used as part of a display pipeline, the VSP is stopped and
> restarted explicitly by the DU from its suspend and resume handlers.
> There is thus no need to stop or restart pipelines in the VSP suspend
> and resume handlers, and doing so would cause the hardware to be
> left in a misconfigured state.
> 
> Ensure that the VSP suspend and resume handlers do not affect DRM
> based pipelines.

s/DRM-base/DRM-based/

> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/platform/vsp1/vsp1_drv.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 962e4c304076..ed25ba9d551b
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device
> *dev) {
>   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> 
> - vsp1_pipelines_suspend(vsp1);
> + /*
> +  * When used as part of a display pipeline, the VSP is stopped and
> +  * restarted explicitly by the DU

s/DU/DU./

> +  */
> + if (!vsp1->drm)
> + vsp1_pipelines_suspend(vsp1);
> +
>   pm_runtime_force_suspend(vsp1->dev);
> 
>   return 0;
> @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device
> *dev) struct vsp1_device *vsp1 = dev_get_drvdata(dev);
> 
>   pm_runtime_force_resume(vsp1->dev);
> - vsp1_pipelines_resume(vsp1);
> +
> + /*
> +  * When used as part of a display pipeline, the VSP is stopped and
> +  * restarted explicitly by the DU

s/DU/DU./

Apart from that,

Reviewed-by: Laurent Pinchart 

> +  */
> + if (!vsp1->drm)
> + vsp1_pipelines_resume(vsp1);
> 
>   return 0;
>  }

-- 
Regards,

Laurent Pinchart

[no subject]

[Scott Tisdale]

Salutations Linux



http://bit.ly/2meKvbU



Scott

Re: [PATCH v10 2/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Sakari Ailus]

Hi Laurent,

On Sat, Nov 11, 2017 at 08:17:59AM +0200, Laurent Pinchart wrote:
> > > +static int rcar_csi2_s_power(struct v4l2_subdev *sd, int on)
> > > +{
> > > + struct rcar_csi2 *priv = sd_to_csi2(sd);
> > > +
> > > + if (on)
> > > + pm_runtime_get_sync(priv->dev);
> > > + else
> > > + pm_runtime_put(priv->dev);
> > 
> > The pipeline you have is already rather complex. Would it be an option to
> > power the hardware on when streaming is started? The smiapp driver does
> > this, without even implementing the s_power callback. (You'd still need to
> > call it on the image source, as long as we have drivers that need it.)
> 
> We've briefly discussed this before, and I agree that pipeline power 
> management needs to be redesigned, but we still haven't agreed on a proper 
> architecture for that. Feel free to propose an RFC :-) In the meantime I 

Well, perhaps we should look at the use cases and see if there are gaps.
Runtime PM is essentially used everywhere else in the kernel.

> wouldn't try to enforce one specific model.

What I just wanted to point out that by switching to runtime PM you avoid
adding a new driver which is dependent on the s_power callback which I
don't think we'll want to keep in the long run. In this case there's no
benefit from s_power in any quantifiable way that I can see.

Actually the master driver manages calling the s_power callback so there's
hardly a need to do so here. It's just that the master drivers still need
that as long as there are sub-device drivers that depend on it.

-- 
Regards,

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

Re: [PATCH v10 2/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

[Sakari Ailus]

Hejssan, Niklas!

On Sat, Nov 11, 2017 at 01:11:13AM +0100, Niklas Söderlund wrote:
> Hej Sakari,
> 
> On 2017-11-11 00:32:27 +0200, Sakari Ailus wrote:
> > Hej Niklas,
> > 
> > Tack för uppdaterade lappar! Jag har några kommentar nedan... det ser bra
> > ut överallt.
> 
> Tack för din feedback!

Var så god!

...

> > > +static int rcar_csi2_calc_phypll(struct rcar_csi2 *priv,
> > > +  struct v4l2_subdev *source,
> > > +  struct v4l2_mbus_framefmt *mf,
> > > +  u32 *phypll)
> > > +{
> > > + const struct phypll_hsfreqrange *hsfreq;
> > > + const struct rcar_csi2_format *format;
> > > + struct v4l2_ctrl *ctrl;
> > > + u64 mbps;
> > > +
> > > + ctrl = v4l2_ctrl_find(source->ctrl_handler, V4L2_CID_PIXEL_RATE);
> > 
> > How about LINK_FREQ instead? It'd be more straightforward to calculate
> > this. Up to you.
> 
> I need to use PIXEL_RATE as my test setup uses the adv748x driver which 
> only implement that control. In the short term I would like to support 
> both, but I need a setup to test that before I can add support for it.  
> In the long term, maybe we should deprecate one of the controls?

Hmm. At least we musn't give the responsibility to calculate the pixel rate
to the end user: this depends on the bus and that is not visible to the
user space.

The link frequency is usually chosen from a few alternatives (or there's
just a single choice). And the pixel rate depends on the pixel code chosen
--- so you'd have a menu with menu entries changing based on format. That'd
be odd.

Perhaps just leave it as-is, as suggested by Laurent?

...

> > > +
> > > + dev_dbg(priv->dev, "Using source %s pad: %u\n", source->name,
> > > + source_pad->index);
> > > +
> > > + fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > > + fmt.pad = source_pad->index;
> > > + ret = v4l2_subdev_call(source, pad, get_fmt, NULL, );
> > > + if (ret)
> > > + return ret;
> > 
> > The link format has been validated already by this point this isn't
> > supposed to fail or produce different results than in link validation.
> > 
> > You could get the same format from the local pad, too.
> 
> Another good catch, I will use the format stored locally to make the 
> code simpler. As you state the formats are already validated so it's 
> safe to do so. I still need to get the remote subdevice to be able to 
> read the PIXEL_RATE control, but I can move that to 
> rcar_csi2_calc_phypll() where it's actually used.

Sounds good to me.
 
...

> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int rcar_csi2_set_pad_format(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_pad_config *cfg,
> > > + struct v4l2_subdev_format *format)
> > > +{
> > > + struct rcar_csi2 *priv = sd_to_csi2(sd);
> > > + struct v4l2_mbus_framefmt *framefmt;
> > > +
> > 
> > Is everything possible? For instance, are there limits regarding width or
> > height? Or the pixel code? They should be checked here.
> 
> Yes it do care about pixel code which was only validated at s_stream() 
> time, will move the check here thanks!
> 
> > 
> > Also the format on the source pad is, presumably, dependent on the format
> > on the sink pad.
> 
> Yes, but since this driver can't change the format in any way is there 
> any harm in mirror whatever is set on one pad on the other(s)? This will 
> change once multiple stream support is added in a later series.

If the hardware does not have functionality that may affect the size or the
pixel code, then the format on the source pad shouldn't be settable.

Yes, this will change when support for multiple streams is added. You'll
get more pads, and the sink pad no longer will have a format. But... it's a
good question how to prepare for this --- is it possible without changing
the user space API? I'm sure we'll have other such cases, such as the
smiapp driver with sensor embedded data.

...

> > > +static int rcar_csi2_parse_dt(struct rcar_csi2 *priv)
> > > +{
> > > + struct device_node *ep;
> > > + struct v4l2_fwnode_endpoint v4l2_ep;
> > > + int ret;
> > > +
> > > + ep = of_graph_get_endpoint_by_regs(priv->dev->of_node, 0, 0);
> > > + if (!ep) {
> > > + dev_err(priv->dev, "Not connected to subdevice\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep), _ep);
> > > + if (ret) {
> > > + dev_err(priv->dev, "Could not parse v4l2 endpoint\n");
> > > + of_node_put(ep);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = rcar_csi2_parse_v4l2(priv, _ep);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + priv->remote.match.fwnode.fwnode =
> > > + fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> > > + priv->remote.match_type = V4L2_ASYNC_MATCH_FWNODE;
> > > +
> > > + of_node_put(ep);
> > > +
> > > + priv->notifier.subdevs = devm_kzalloc(priv->dev,
> > > +   

[PATCH] build: VIDEO_IMX274 driver requires kernel 3.18.0 to compile

[Jasmin J.]

From: Jasmin Jessich 

Signed-off-by: Jasmin Jessich 
---
 v4l/versions.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index dab91c7..4ee0840 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -69,6 +69,7 @@ VIDEO_OV2640
 VIDEO_OV5645
 VIDEO_OV7670
 VIDEO_OV5640
+VIDEO_IMX274
 IR_GPIO_TX
 IR_GPIO_CIR
 # needs component_match_add
-- 
2.7.4

Re: 'LITE-ON USB2.0 DVB-T Tune' driver crash with kernel 4.13 / ubuntu 17.10

[Sean Young]

Hi Laurant,

On Sat, Nov 11, 2017 at 08:06:38PM +0100, Laurent Caumont wrote:
> Hi Sean,
> 
> I hope this one will be okay.

There is a memory leak in there, and there is no reason to have to kmallocs
for this function. Please would you mind testing this version?

Please note that there were other issues like whitespace which I've fixed
up.

Thanks,
Sean

>From 8362bc3e95016944b173c3866c103fcbc2587b6d Mon Sep 17 00:00:00 2001
From: Laurent Caumont 
Date: Sat, 11 Nov 2017 18:44:46 +0100
Subject: [PATCH] media: dvb: i2c transfers over usb cannot be done from stack

Cc: sta...@vger.kernel.org
Signed-off-by: Laurent Caumont 
Signed-off-by: Sean Young 
---
 drivers/media/usb/dvb-usb/dibusb-common.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c 
b/drivers/media/usb/dvb-usb/dibusb-common.c
index 8207e6900656..bcacb0f22028 100644
--- a/drivers/media/usb/dvb-usb/dibusb-common.c
+++ b/drivers/media/usb/dvb-usb/dibusb-common.c
@@ -223,8 +223,20 @@ EXPORT_SYMBOL(dibusb_i2c_algo);
 
 int dibusb_read_eeprom_byte(struct dvb_usb_device *d, u8 offs, u8 *val)
 {
-   u8 wbuf[1] = { offs };
-   return dibusb_i2c_msg(d, 0x50, wbuf, 1, val, 1);
+   u8 *buf;
+   int rc;
+
+   buf = kmalloc(2, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+
+   buf[0] = offs;
+
+   rc = dibusb_i2c_msg(d, 0x50, [0], 1, [1], 1);
+   *val = buf[1];
+   kfree(buf);
+
+   return rc;
 }
 EXPORT_SYMBOL(dibusb_read_eeprom_byte);
 
-- 
2.13.6

Re: 'LITE-ON USB2.0 DVB-T Tune' driver crash with kernel 4.13 / ubuntu 17.10

[Laurent Caumont]

Hi Sean,

I hope this one will be okay.


2017-11-11 19:01 GMT+01:00 Sean Young :
> Hi Laurent,
>
> On Sat, Nov 11, 2017 at 06:53:54PM +0100, Laurent Caumont wrote:
>> Hi Sean,
>>
>> I just realized that files in media_build/linux/driver are not
>> associate with a git repository. They are retrieved by the build
>> command.
>> So, I cloned the linux-stable repository to generate the patch.
>
> Great, thank you.
>
> We need a Signed-off-by: line to accept your patch, see part 11 of
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> Thanks,
>
> Sean
From bf48cb8988a0335038e8df1a40f1d1b2cf4225d5 Mon Sep 17 00:00:00 2001
From: Laurent Caumont 
Date: Sat, 11 Nov 2017 18:44:46 +0100
Subject: [PATCH] media: dvb: i2c transfers over usb - use kmalloc instead
 stack Signed-off-by: Laurent Caumont 

---
 drivers/media/usb/dvb-usb/dibusb-common.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c
index 8207e690..e1c31381 100644
--- a/drivers/media/usb/dvb-usb/dibusb-common.c
+++ b/drivers/media/usb/dvb-usb/dibusb-common.c
@@ -223,8 +223,26 @@ EXPORT_SYMBOL(dibusb_i2c_algo);
 
 int dibusb_read_eeprom_byte(struct dvb_usb_device *d, u8 offs, u8 *val)
 {
-	u8 wbuf[1] = { offs };
-	return dibusb_i2c_msg(d, 0x50, wbuf, 1, val, 1);
+	  u8 *wbuf;
+	  u8 *rbuf;
+	  int rc;
+	  
+	  rbuf = kmalloc(1, GFP_KERNEL);
+	  if (!rbuf)
+	return -ENOMEM;
+	 
+	  wbuf = kmalloc(1, GFP_KERNEL);
+	  if (!wbuf)
+	return -ENOMEM;
+	  
+ *wbuf = offs;
+
+	 rc = dibusb_i2c_msg(d, 0x50, wbuf, 1, rbuf, 1);
+ kfree(wbuf);
+	 *val = *rbuf;
+	 kfree(rbuf);
+	  
+	return rc;
 }
 EXPORT_SYMBOL(dibusb_read_eeprom_byte);
 
-- 
2.14.1

Re: 'LITE-ON USB2.0 DVB-T Tune' driver crash with kernel 4.13 / ubuntu 17.10

[Sean Young]

Hi Laurent,

On Sat, Nov 11, 2017 at 06:53:54PM +0100, Laurent Caumont wrote:
> Hi Sean,
> 
> I just realized that files in media_build/linux/driver are not
> associate with a git repository. They are retrieved by the build
> command.
> So, I cloned the linux-stable repository to generate the patch.

Great, thank you.

We need a Signed-off-by: line to accept your patch, see part 11 of

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Thanks,

Sean

Re: 'LITE-ON USB2.0 DVB-T Tune' driver crash with kernel 4.13 / ubuntu 17.10

[Laurent Caumont]

Hi Sean,

I just realized that files in media_build/linux/driver are not
associate with a git repository. They are retrieved by the build
command.
So, I cloned the linux-stable repository to generate the patch.

Regards,
Laurent

2017-11-11 11:56 GMT+01:00 Sean Young :
> Hi Laurent,
>
> On Fri, Nov 10, 2017 at 09:33:58PM +0100, Laurent Caumont wrote:
>> Hi Mauro,
>>
>> I could send you a patch but my git doesn't see the modification I
>> made, so it's unable to generate a patch.
>> I don't know if it's a git issue on ubunu 17.10 or if it's the way the
>> repository was created.
>
> The difference might be in the git index (or not in it). If you send a
> Signed-off-by: Laurent etc line then I'm happy to handle the patch
> generation.
>
> Thanks
>
> Sean
From 03cebd478b80677252a0f48d71d0de05e6c82740 Mon Sep 17 00:00:00 2001
From: Laurent Caumont 
Date: Sat, 11 Nov 2017 18:44:46 +0100
Subject: [PATCH] media: dvb: i2c transfers over usb - use kmalloc instead
 stack

---
 drivers/media/usb/dvb-usb/dibusb-common.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c
index 8207e690..e1c31381 100644
--- a/drivers/media/usb/dvb-usb/dibusb-common.c
+++ b/drivers/media/usb/dvb-usb/dibusb-common.c
@@ -223,8 +223,26 @@ EXPORT_SYMBOL(dibusb_i2c_algo);
 
 int dibusb_read_eeprom_byte(struct dvb_usb_device *d, u8 offs, u8 *val)
 {
-	u8 wbuf[1] = { offs };
-	return dibusb_i2c_msg(d, 0x50, wbuf, 1, val, 1);
+	  u8 *wbuf;
+	  u8 *rbuf;
+	  int rc;
+	  
+	  rbuf = kmalloc(1, GFP_KERNEL);
+	  if (!rbuf)
+	return -ENOMEM;
+	 
+	  wbuf = kmalloc(1, GFP_KERNEL);
+	  if (!wbuf)
+	return -ENOMEM;
+	  
+ *wbuf = offs;
+
+	 rc = dibusb_i2c_msg(d, 0x50, wbuf, 1, rbuf, 1);
+ kfree(wbuf);
+	 *val = *rbuf;
+	 kfree(rbuf);
+	  
+	return rc;
 }
 EXPORT_SYMBOL(dibusb_read_eeprom_byte);
 
-- 
2.14.1

Re: [PATCH v2] media: vsp1: Prevent suspending and resuming DRM pipelines

[Kieran Bingham]

Ping ...

This patch appears to have got lost in the post.

Could someone pick it up please?

--
Regards

Kieran

On 20/09/17 10:16, Kieran Bingham wrote:
> When used as part of a display pipeline, the VSP is stopped and
> restarted explicitly by the DU from its suspend and resume handlers.
> There is thus no need to stop or restart pipelines in the VSP suspend
> and resume handlers, and doing so would cause the hardware to be
> left in a misconfigured state.
> 
> Ensure that the VSP suspend and resume handlers do not affect DRM
> based pipelines.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  drivers/media/platform/vsp1/vsp1_drv.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
> b/drivers/media/platform/vsp1/vsp1_drv.c
> index 962e4c304076..ed25ba9d551b 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -571,7 +571,13 @@ static int __maybe_unused vsp1_pm_suspend(struct device 
> *dev)
>  {
>   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
>  
> - vsp1_pipelines_suspend(vsp1);
> + /*
> +  * When used as part of a display pipeline, the VSP is stopped and
> +  * restarted explicitly by the DU
> +  */
> + if (!vsp1->drm)
> + vsp1_pipelines_suspend(vsp1);
> +
>   pm_runtime_force_suspend(vsp1->dev);
>  
>   return 0;
> @@ -582,7 +588,13 @@ static int __maybe_unused vsp1_pm_resume(struct device 
> *dev)
>   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
>  
>   pm_runtime_force_resume(vsp1->dev);
> - vsp1_pipelines_resume(vsp1);
> +
> + /*
> +  * When used as part of a display pipeline, the VSP is stopped and
> +  * restarted explicitly by the DU
> +  */
> + if (!vsp1->drm)
> + vsp1_pipelines_resume(vsp1);
>  
>   return 0;
>  }
> 

Re: [PATCH v4 2/5] media: dt: bindings: Add binding for NVIDIA Tegra Video Decoder Engine

[Vladimir Zapolskiy]

Hi Dmitry,

On 10/20/2017 12:34 AM, Dmitry Osipenko wrote:
> Add binding documentation for the Video Decoder Engine which is found
> on NVIDIA Tegra20/30/114/124/132 SoC's.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  .../devicetree/bindings/media/nvidia,tegra-vde.txt | 55 
> ++
>  1 file changed, 55 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt 
> b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
> new file mode 100644
> index ..470237ed6fe5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
> @@ -0,0 +1,55 @@
> +NVIDIA Tegra Video Decoder Engine
> +
> +Required properties:
> +- compatible : Must contain one of the following values:
> +   - "nvidia,tegra20-vde"
> +   - "nvidia,tegra30-vde"
> +   - "nvidia,tegra114-vde"
> +   - "nvidia,tegra124-vde"
> +   - "nvidia,tegra132-vde"
> +- reg : Must contain an entry for each entry in reg-names.
> +- reg-names : Must include the following entries:
> +  - sxe
> +  - bsev
> +  - mbe
> +  - ppe
> +  - mce
> +  - tfe
> +  - ppb
> +  - vdma
> +  - frameid

I've already mentioned it in my review of the driver code, but the
version from v3 with a single region is more preferable.

Also it implies that "reg-names" property will be removed.

> +- iram : Must contain phandle to the mmio-sram device node that represents
> + IRAM region used by VDE.
> +- interrupts : Must contain an entry for each entry in interrupt-names.
> +- interrupt-names : Must include the following entries:
> +  - sync-token
> +  - bsev
> +  - sxe
> +- clocks : Must include the following entries:
> +  - vde
> +- resets : Must include the following entries:
> +  - vde
> +
> +Example:
> +
> +video-codec@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x1000 /* Syntax Engine */
> +0x6001b000 0x1000 /* Video Bitstream Engine */
> +0x6001c000  0x100 /* Macroblock Engine */
> +0x6001c200  0x100 /* Post-processing Engine */
> +0x6001c400  0x100 /* Motion Compensation Engine */
> +0x6001c600  0x100 /* Transform Engine */
> +0x6001c800  0x100 /* Pixel prediction block */
> +0x6001ca00  0x100 /* Video DMA */
> +0x6001d800  0x300 /* Video frame controls */>;
> + reg-names = "sxe", "bsev", "mbe", "ppe", "mce",
> + "tfe", "ppb", "vdma", "frameid";
> + iram = <_pool>; /* IRAM region */
> + interrupts = , /* Sync token interrupt 
> */
> +  , /* BSE-V interrupt */
> +  ; /* SXE interrupt */
> + interrupt-names = "sync-token", "bsev", "sxe";
> + clocks = <_car TEGRA20_CLK_VDE>;
> + resets = <_car 61>;
> +};
> 

--
With best wishes,
Vladimir

Re: [PATCH v4 1/5] ARM: tegra: Add device tree node to describe IRAM

[Vladimir Zapolskiy]

Hi Dmitry,

On 10/20/2017 12:34 AM, Dmitry Osipenko wrote:
> From: Vladimir Zapolskiy 
> 
> All Tegra SoCs contain 256KiB IRAM, which is used to store CPU resume code
> and by hardware engines like a video decoder.
> 
> Signed-off-by: Vladimir Zapolskiy 

Please add also your own closing "Signed-off-by" tag, please reference
to "Developer's Certificate of Origin 1.1", point (c), it is found in
Documentation/process/submitting-patches.rst

> ---
>  arch/arm/boot/dts/tegra114.dtsi | 8 
>  arch/arm/boot/dts/tegra124.dtsi | 8 
>  arch/arm/boot/dts/tegra20.dtsi  | 8 
>  arch/arm/boot/dts/tegra30.dtsi  | 8 

My assumption is that Thierry would prefer to get 4 separate patches,
one for each platform, please split the patch.

Also thanks for your time and your efforts applied to push my occasional
change, please feel free to take your own authorship for 3 out of 4 patches.

>  4 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
> index 8932ea3afd5f..13f6087790c8 100644
> --- a/arch/arm/boot/dts/tegra114.dtsi
> +++ b/arch/arm/boot/dts/tegra114.dtsi
> @@ -10,6 +10,14 @@
>   compatible = "nvidia,tegra114";
>   interrupt-parent = <>;
>  
> + iram@4000 {
> + compatible = "mmio-sram";

Unfortunately Thierry hasn't yet replied, but my assumption is that
the list of compatibles should be extended with one more SoC specific
value like

compatible = "nvidia,tegra114-sysram", "mmio-sram";

I'm not sure, if Tegra maintainers want to see a new compatible
described in Documentation/devicetree/bindings.

> + reg = <0x4000 0x4>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x4000 0x4>;
> + };
> +
>   host1x@5000 {
>   compatible = "nvidia,tegra114-host1x", "simple-bus";
>   reg = <0x5000 0x00028000>;
> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
> index 8baf00b89efb..a3585ed82646 100644
> --- a/arch/arm/boot/dts/tegra124.dtsi
> +++ b/arch/arm/boot/dts/tegra124.dtsi

The considerations from above are applicable to the rest of
the touched platforms.

--
With best wishes,
Vladimir

Re: [PATCH v4 3/5] staging: Introduce NVIDIA Tegra video decoder driver

[Vladimir Zapolskiy]

Hi Dmitry,

I'll add just a couple of minor comments, in general the code looks
very good.

On 10/20/2017 12:34 AM, Dmitry Osipenko wrote:
> NVIDIA Tegra20/30/114/124/132 SoC's have video decoder engine that
> supports standard set of video formats like H.264 / MPEG-4 / WMV / VC1.
> Currently implemented decoding of CAVLC H.264 on Tegra20 only.
> 
> Signed-off-by: Dmitry Osipenko 

[snip]

> +++ b/drivers/staging/tegra-vde/uapi.h
> @@ -0,0 +1,101 @@
> +/*
> + * Copyright (C) 2016-2017 Dmitry Osipenko 
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.

>From the specified MODULE_LICENSE("GPL") I'd rather expect to see a reference
to GPLv2+ license in the header, and here the text resembles MIT license only.

I understand that it is a UAPI header file and it may happen that different
rules are applied to this kind of sources, hopefully Greg can give the right
directions.

In general you may avoid the headache with the custom UAPI, if you reuse
V4L2 interfaces, if I remember correctly drivers/media/platform/coda does it.
Also from my point of view the custom UAPI is the only reason why the driver
is pushed to the staging folder.

[snip]

> +struct tegra_vde {
> + void __iomem *sxe;
> + void __iomem *bsev;
> + void __iomem *mbe;
> + void __iomem *ppe;
> + void __iomem *mce;
> + void __iomem *tfe;
> + void __iomem *ppb;
> + void __iomem *vdma;
> + void __iomem *frameid;

Please find a comment in tegra_vde_probe() function regarding
devm_ioremap_resource() calls.

> + struct mutex lock;
> + struct miscdevice miscdev;
> + struct reset_control *rst;
> + struct gen_pool *iram_pool;
> + struct completion decode_completion;
> + struct clk *clk;
> + dma_addr_t iram_lists_addr;
> + u32 *iram;
> +};

[snip]

> +static int tegra_vde_wait_bsev(struct tegra_vde *vde, bool wait_dma)
> +{
> + struct device *dev = vde->miscdev.parent;
> + u32 value;
> + int err;
> +
> + err = readl_relaxed_poll_timeout(vde->bsev + INTR_STATUS, value,
> +  !(value & BIT(2)), 1, 100);
> + if (err) {
> + dev_err(dev, "BSEV unknown bit timeout\n");
> + return err;
> + }
> +
> + err = readl_relaxed_poll_timeout(vde->bsev + INTR_STATUS, value,
> +  (value & BSE_ICMDQUE_EMPTY), 1, 100);
> + if (err) {
> + dev_err(dev, "BSEV ICMDQUE flush timeout\n");
> + return err;
> + }
> +
> + if (!wait_dma)
> + return 0;
> +
> + err = readl_relaxed_poll_timeout(vde->bsev + INTR_STATUS, value,
> +  !(value & BSE_DMA_BUSY), 1, 100);
> + if (err) {
> + dev_err(dev, "BSEV DMA timeout\n");
> + return err;
> + }
> +
> + return 0;

if (err)
dev_err(dev, "BSEV DMA timeout\n");

return err;

is two lines shorter.

> +}
> +

[snip]

> +static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
> + struct video_frame *frame,
> + struct tegra_vde_h264_frame *source,
> + enum dma_data_direction dma_dir,
> + bool baseline_profile,
> + size_t csize)
> +{
> + int err;
> +
> + err = tegra_vde_attach_dmabuf(dev, source->y_fd,
> +   source->y_offset, csize * 4,
> +   >y_dmabuf_attachment,
> +   >y_addr,
> +   >y_sgt,
> +   NULL, dma_dir);
> + if (err)
> + return err;
> +
> + err = tegra_vde_attach_dmabuf(dev, 

Re: [PATCH 2/2] sdlcam: ignore binary

[Rafaël Carré]

On 11/10/2017 05:05 PM, Rafaël Carré wrote:
> ---
>  contrib/test/.gitignore | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/test/.gitignore b/contrib/test/.gitignore
> index ad64325b..5bd81d01 100644
> --- a/contrib/test/.gitignore
> +++ b/contrib/test/.gitignore
> @@ -8,3 +8,4 @@ stress-buffer
>  v4l2gl
>  v4l2grab
>  mc_nextgen_test
> +sdlcam
> 

Signed-off-by: Rafaël Carré 

Re: [PATCH 1/2] sdlcam: fix linking

[Rafaël Carré]

On 11/10/2017 05:05 PM, Rafaël Carré wrote:
> ---
>  contrib/test/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am
> index 6a4303d7..0188fe21 100644
> --- a/contrib/test/Makefile.am
> +++ b/contrib/test/Makefile.am
> @@ -37,7 +37,7 @@ v4l2gl_LDADD = ../../lib/libv4l2/libv4l2.la 
> ../../lib/libv4lconvert/libv4lconver
>  
>  sdlcam_LDFLAGS = $(JPEG_LIBS) $(SDL2_LIBS) -lm -ldl -lrt
>  sdlcam_CFLAGS = -I../.. $(SDL2_CFLAGS)
> -sdlcam_LDADD = ../../lib/libv4l2/.libs/libv4l2.a  
> ../../lib/libv4lconvert/.libs/libv4lconvert.a
> +sdlcam_LDADD = ../../lib/libv4l2/libv4l2.la  
> ../../lib/libv4lconvert/libv4lconvert.la
>  
>  mc_nextgen_test_CFLAGS = $(LIBUDEV_CFLAGS)
>  mc_nextgen_test_LDFLAGS = $(LIBUDEV_LIBS)
> 

Signed-off-by: Rafaël Carré 

Re: 'LITE-ON USB2.0 DVB-T Tune' driver crash with kernel 4.13 / ubuntu 17.10

[Sean Young]

Hi Laurent,

On Fri, Nov 10, 2017 at 09:33:58PM +0100, Laurent Caumont wrote:
> Hi Mauro,
> 
> I could send you a patch but my git doesn't see the modification I
> made, so it's unable to generate a patch.
> I don't know if it's a git issue on ubunu 17.10 or if it's the way the
> repository was created.

The difference might be in the git index (or not in it). If you send a 
Signed-off-by: Laurent etc line then I'm happy to handle the patch
generation.

Thanks

Sean

Re: DVB-S2 and S2X API extensions

[Mauro Carvalho Chehab]

Em Thu, 9 Nov 2017 16:28:18 +0100
Ralph Metzler  escreveu:

> Hi,
> 
> I have a few RFCs regarding new needed enums and
> properties for DVB-S2 and DVB-S2X:
> 
> - DVB-S2/X physical layer scrambling
> 
> I currently use the inofficial DTV_PLS for setting the scrambling
> sequence index (cf. ETSI EN 300 468 table 41) of
> the physical layer scrambling in DVB-S2 and DVB-S2X.
> Some drivers already misuse bits 8-27 of the DTV_STREAM_ID
> for setting this. They also differentiate between gold, root
> and combo codes.
> The gold code is the actual scrambling sequence index.
> The root code is just an intermediate calculation
> accepted by some chips, but derived from the gold code.
> It can be easily mapped one-to-one to the gold code.
> (see https://github.com/DigitalDevices/dddvb/blob/master/apps/pls.c,
> A more optimized version of this could be added to dvb-math.c)
> The combo code does not really exist but is a by-product
> of the buggy usage of a gold to root code conversion in ST chips.
> 
> So, I would propose to officially introduce a property
> for the scrambling sequence index (=gold code).
> Should it be called DTV_PLS (which I already used in the dddvb package)
> or rather DTV_SSI?
> I realized PLS can be confused with physical layer signalling which
> uses the acronym PLS in the DVB-S2 specs.

Yes, it makes sense to have a DTV property for the PLS gold code.

I would prefer to use a clearer name, like DTV_PLS_GOLD_CODE,
to make easier to identify what it means.

At documentation, we should point to EN 302 307 section 5.5.4 and
to EN 300 468 table 41, with a good enough description to make
clear that it is the gold code, and not the root code (or
a chipset-specific "combo" code).

> DVB-S2X also defines 7 preferred scrambling code sequences
> (EN 302 307-2 5.5.4) which should be checked during tuning.
> If the demod does not do this, should the DVB kernel layer or
> application do this?

IMHO, it should be up to the kernel to check if the received
gold code is one of those 7 codes from EM 302 307 part 2 spec,
if the delivery system is DVB_S2X (btw, we likely need to add it
to the list of delivery systems). Not sure what would be the
best way to implement it. Perhaps via some ancillary routine
that the demods would be using.

> - slices
> 
> DVB-S2 and DVB-C2, additionally to ISI/PLP, also can have
> slicing. For DVB-C2 I currently use bits 8-15 of DTV_STREAM_ID as slice id.

Better to use a separate property for that. On the documentation
patches I wrote, I made clear that, for DVB-S2, only 8 bits of
DTV_STREAM_ID are valid.

We need to add DVB-C2 delivery system and update documentation
accordingly. I made an effort to document, per DTV property,
what delivery systems accept them, and what are the valid
values, per standard.

> For DVB-S2/X the misuse of bits 8-27 by some drivers for selecting
> the scrambling sequence index code could cause problems.
> Should there be a new property for slice selection?

Yes.

> It is recommended that slice id and ISI/PLP id are identical but they
> can be different.

The new property should reserve a value (0 or (unsigned)-1) to mean "AUTO",
in the sense that slice ID will be identical to ISI/PLP, being the default.

> - new DVB-S2X features
> 
> DVB-S2X needs some more roll-offs, FECs and modulations. I guess adding those
> should be no problem?

Yes, just adding those are OK. We should just document what values are 
valid for DVB-S2X at the spec.

Ok, this is actually already at the specs, but it helps application
developers to ensure that their apps will only send valid values to the
Kernel, if we keep such information at the uAPI documentation.

> Do we need FE_CAN_SLICES, FE_CAN_3G_MODULATION, etc?

That is a good question. On my opinion, yes, we should add new
capabilities there, but we're out of space at the u32 caps that we
use for capabilities (there are other missing caps there for other
new standards).

We could start using a DTV property for capabilities, or define
a variant of FE_GET_INFO that would use an u64 value for
the caps field.

> Or would a new delivery system type for S2X make sense?

IMHO, it makes sense to have a new delivery system type for S2X.
A FE_CAN_3G_MODULATION (and, in the future, CAN_4G, CAN_5G, ...)
could work too, but not sure how this would scale in the future,
as support for older variants could be removed from some devices,
e. g. a given demod could, for instance, support 3G, 4G and 5G
but won't be able to work with 1G or 2G.

My guess is that multiple delivery systems would scale better.

> -DVB-S2 base band frame support
> 
> There are some older patches which allowed to switch the demod
> to a raw BB frame mode (if it and the bridge support this) and
> have those parsed in the DVB layer.
> 
> See
> https://patchwork.linuxtv.org/patch/10402/
> or
> https://linuxtv.org/pipermail/linux-dvb/2007-December/022217.html
> 
> Chris Lee seems to have a tree based on those:
>