[PATCH] [media] usbtv: add sharpness control

[Lubomir Rintel]

Signed-off-by: Lubomir Rintel 
---
 drivers/media/usb/usbtv/usbtv-video.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/usb/usbtv/usbtv-video.c 
b/drivers/media/usb/usbtv/usbtv-video.c
index d3b6d3d..8135614 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -757,6 +757,12 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
data[1] = -ctrl->val & 0xff;
}
break;
+   case V4L2_CID_SHARPNESS:
+   index = USBTV_BASE + 0x0239;
+   data[0] = 0;
+   data[1] = ctrl->val;
+   size = 2;
+   break;
default:
kfree(data);
return -EINVAL;
@@ -825,6 +831,8 @@ int usbtv_video_init(struct usbtv *usbtv)
V4L2_CID_SATURATION, 0, 0x3ff, 1, 0x200);
v4l2_ctrl_new_std(>ctrl, _ctrl_ops,
V4L2_CID_HUE, -0xdff, 0xdff, 1, 0x000);
+   v4l2_ctrl_new_std(>ctrl, _ctrl_ops,
+   V4L2_CID_SHARPNESS, 0x0, 0xff, 1, 0x60);
ret = usbtv->ctrl.error;
if (ret < 0) {
dev_warn(usbtv->dev, "Could not initialize controls\n");
-- 
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

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:   Mon Feb  6 05:00:15 CET 2017
media-tree git hash:47b037a0512d9f8675ec2693bed46c8ea6a884ab
media_build git hash:   52072e6ab5b3ac8e3fa3db23479f806d8528
v4l-utils git hash: 99306f20cc7e76cf2161e3059de4da245aed2130
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.8.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: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
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-i686: OK
linux-4.10-rc3-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
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: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
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: OK
linux-4.9-x86_64: OK
linux-4.10-rc3-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.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: [PATCH] devicetree: Add video bus switch

[Sebastian Reichel]

Hi,

On Sun, Feb 05, 2017 at 10:12:20PM +0100, Pavel Machek wrote:
> > > 9) Highly reconfigurable hardware - Julien Beraud
> > > 
> > > - 44 sub-devices connected with an interconnect.
> > > - As long as formats match, any sub-device could be connected to any
> > > - other sub-device through a link.
> > > - The result is 44 * 44 links at worst.
> > > - A switch sub-device proposed as the solution to model the
> > > - interconnect. The sub-devices are connected to the switch
> > > - sub-devices through the hardware links that connect to the
> > > - interconnect.
> > > - The switch would be controlled through new IOCTLs S_ROUTING and
> > > - G_ROUTING.
> > > - Patches available:
> > >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > > 
> > > but the patches are from 2005. So I guess I'll need some guidance here...
> > 
> > Yeah, that's where it began (2015?), but right now I can only suggest to
> > wait until there's more. My estimate is within next couple of weeks /
> > months. But it won't be years.
> 
> Ok, week or two would be ok, couple of months is not. And all I need
> is single hook in common structure.
> 
> So if g_endpoint_config hook looks sane to _you_, I suggest we simply
> proceed. Now, maybe Mauro Carvalho Chehab 
> or Laurent or Julien will want a different solution, but
> then... they'll have to suggest something doable now, not in couple of
> months.
> 
> Does that sound like a plan?
> 
> Mauro added to cc list, so we can get some input.

side note: It's an kernel-internal API only used by the media
subsystem. Code can be easily updated to use an improved API
at any time.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 1/6] staging: Import the BCM2835 MMAL-based V4L2 camera driver.

[Michael Zoran]

Dave: I'd personally like to see the current version of vchi stabbed,
poisoned, beaten, shot, chained and thrown in a river.:)

Would it be possible to find another transport for this driver to
remove the dependency on VCHI?  Given the process of improving drivers
in stagging, I don't expect VCHI to ever make it out of staging in it's
current form either. One possibility is mbox, but it has the limitation
of one one request at a time.

Also, you mention protecting IP and that V4L expects everything to be
exposed.  How does V4L2 work if I have a hybrid software/hardware video
capture device? 

In the case of other drivers in linux that upload a large firmware blob
into who knows where, how does the open source part of the driver hook
into the firmware blob?

On Sun, 2017-02-05 at 22:15 +, Dave Stevenson wrote:
> Hi Mauro.
> 
> I'm going to stick my head above the parapet as one of the original 
> authors back when I worked at Broadcom.
> As it happens I started working at Raspberry Pi last Monday, so that 
> puts me in a place where I can work on this again a bit more. (The
> last 
> two years have been just a spare time support role).
> Whilst I have done kernel development work in various roles, it's
> all 
> been downstream so I've not been that active on these lists before.
> 
> All formatting/checkpatch comments noted.
> Checkpatch was whinging when this was first written around December
> 2013 
> about long lines, so many got broken up to shut it up. Views on code 
> style and checkpatch seem to have changed a little since then.
> I thought we had made checkpatch happy before the driver was pushed,
> but 
> with some of the comments still having // style I guess some slipped 
> through the net.
> Yes chunks of this could do with refactoring to reduce the levels of 
> indentation - always more to do.
> If I've removed any formatting/style type comments in my cuts it's
> not 
> because I'm ignoring them, just that they're not something that
> needs 
> discussion (just fixing). I've only taken out the really big lumps
> of 
> code with no comments on.
> 
> Newbie question: if this has already been merged to staging, where am
> I 
> looking for the relevant tree to add patches on top of? 
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> branch 
> staging-next?
> 
> Responses to the rest inline.
> TL;DR answer is that you are seeing the top edge of a full ISP 
> processing pipe and optional encoders running on the GPU, mainly as 
> there are blocks that can't be exposed for IP reasons (Raspberry Pi
> only 
> being the customer not silicon vendor constrains what can and can't
> be 
> made public).
> That doesn't seem to fit very well into V4L2 which expects that it
> can 
> see all the detail, so there are a few nasty spots to shoe-horn it
> in. 
> If there are better ways to solve the problems, then I'm open to
> them.
> 
> Thanks
>    Dave
> 
> 
> On 03/02/17 18:59, Mauro Carvalho Chehab wrote:
> > HI Eric,
> > 
> > Em Fri, 27 Jan 2017 13:54:58 -0800
> > Eric Anholt  escreveu:
> > 
> > > - Supports raw YUV capture, preview, JPEG and H264.
> > > - Uses videobuf2 for data transfer, using dma_buf.
> > > - Uses 3.6.10 timestamping
> > > - Camera power based on use
> > > - Uses immutable input mode on video encoder
> > > 
> > > This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as
> > > of
> > > a15ba877dab4e61ea3fc7b006e2a73828b083c52.
> > 
> > First of all, thanks for that! Having an upstream driver for the
> > RPi camera is something that has been long waited!
> > 
> > Greg was kick on merging it on staging ;) Anyway, the real review
> > will happen when the driver becomes ready to be promoted out of
> > staging. When you address the existing issues and get it ready to
> > merge, please send the patch with such changes to linux-media ML.
> > I'll do a full review on it by then.
> 
> Is that even likely given the dependence on VCHI? I wasn't expecting 
> VCHI to leave staging, which would force this to remain too.
> 
> > Still, let me do a quick review on this driver, specially at the
> > non-MMAL code.
> > 
> > > 
> > > Signed-off-by: Eric Anholt 
> > > ---
> > >  .../media/platform/bcm2835/bcm2835-camera.c| 2016
> > > 
> > >  .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
> > >  drivers/staging/media/platform/bcm2835/controls.c  | 1345
> > > +
> > >  .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
> > >  .../media/platform/bcm2835/mmal-encodings.h|  127 ++
> > >  .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
> > >  .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
> > >  .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
> > >  drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
> > >  .../media/platform/bcm2835/mmal-parameters.h   |  689
> > > +++
> > >  .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916

Re: [PATCH] media: Add video bus switch

[Sakari Ailus]

Hi, Pavel!

On Sun, Feb 05, 2017 at 11:16:22PM +0100, Pavel Machek wrote:
> Hi!
> 
> I lost my original reply... so this will be slightly brief.

:-o

> 
> > > > + * 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.
> > > > + */
> > > > +
> > > > +#define DEBUG
> > 
> > Please remove.
> 
> Ok.
> 
> > > > +#include 
> > > > +#include 
> > 
> > Alphabetical order, please.
> 
> Ok. (But let me make unhappy noise, because these rules are
> inconsistent across kernel.)
> 
> > > > + * TODO:
> > > > + * isp_subdev_notifier_complete() calls 
> > > > v4l2_device_register_subdev_nodes()
> > > > + */
> > > > +
> > > > +#define CSI_SWITCH_SUBDEVS 2
> > > > +#define CSI_SWITCH_PORTS 3
> > 
> > This could go to the enum below.
> > 
> > I guess the CSI_SWITCH_SUBDEVS could be (CSI_SWITCH_PORTS - 1).
> > 
> > I'd just replace CSI_SWITCH by VBS. The bus could be called
> > differently.
> 
> Ok.
> 
> > > > +static int vbs_registered(struct v4l2_subdev *sd)
> > > > +{
> > > > +   struct v4l2_device *v4l2_dev = sd->v4l2_dev;
> > > > +   struct vbs_data *pdata;
> > > > +   int err;
> > > > +
> > > > +   dev_dbg(sd->dev, "registered, init notifier...\n");
> > 
> > Looks like a development time debug message. :-)
> 
> ex-development message ;-).
> 
> > > > +   gpiod_set_value(pdata->swgpio, local->index == 
> > > > CSI_SWITCH_PORT_2);
> > > > +   pdata->state = local->index;
> > > > +
> > > > +   sd = vbs_get_remote_subdev(sd);
> > > > +   if (IS_ERR(sd))
> > > > +   return PTR_ERR(sd);
> > > > +
> > > > +   pdata->subdev.ctrl_handler = sd->ctrl_handler;
> > 
> > This is ugly. You're exposing all the controls through another sub-device.
> > 
> > How does link validation work now?
> > 
> > I wonder if it'd be less so if you just pass through the LINK_FREQ and
> > PIXEL_RATE controls. It'll certainly be more code though.
> > 
> > I think the link frequency could be something that goes to the frame
> > descriptor as well. Then we wouldn't need to worry about the controls
> > separately, just passing the frame descriptor would be enough.
> > 
> > I apologise that I don't have patches quite ready for posting to the
> > list.
> 
> (Plus of course question is "what is link validation".)

The links along the pipeline are validated for matching width, height, media
bus code and possibly other matters. The aim is to make sure that the
hardware configuration is a valid one before streaming starts.

> 
> Ok, let me play with this one. Solution you are suggesting is to make
> a custom harndler that only passes certain data through, right?

That's an option. But supposing we'll add that to the frame desciptors [1],
there won't be need for a custom handler either.

> 
> > > > +   dev_dbg(pdata->subdev.dev, "create link: %s[%d] -> 
> > > > %s[%d])\n",
> > > > +   src->name, src_pad, sink->name, sink_pad);
> > > > +   }
> > > > +
> > > > +   return 
> > > > v4l2_device_register_subdev_nodes(pdata->subdev.v4l2_dev);
> > 
> > The ISP driver's complete() callback calls
> > v4l2_device_register_subdev_nodes() already. Currently it cannot handle
> > being called more than once --- that needs to be fixed.
> 
> I may have patches for that. Let me check.
> 
> > > > +}
> > > > +
> > > > +
> > 
> > I'd say that's an extra newline.
> 
> Not any more.
> 
> > > > +   v4l2_subdev_init(>subdev, _ops);
> > > > +   pdata->subdev.dev = >dev;
> > > > +   pdata->subdev.owner = pdev->dev.driver->owner;
> > > > +   strncpy(pdata->subdev.name, dev_name(>dev), 
> > > > V4L2_SUBDEV_NAME_SIZE);
> > 
> > How about sizeof(pdata->subdev.name) ?
> 
> Ok.
> 
> > I'm not sure I like V4L2_SUBDEV_NAME_SIZE in general. It could be even
> > removed. But not by this patch. :-)
> > 
> > > > +   v4l2_set_subdevdata(>subdev, pdata);
> > > > +   pdata->subdev.entity.function = MEDIA_ENT_F_SWITCH;
> > > > +   pdata->subdev.entity.flags |= MEDIA_ENT_F_SWITCH;
> > 
> > MEDIA_ENT_FL_*
> 
> Do we actually have a flag here? We already have .function, so this
> looks like a duplicate.

You can skip setting this. We only have flags for DEFAULT and CONNECTOR and
neither is relevant here.

> 
> 
> > > > +   if (err < 0) {
> > > > +   dev_err(>dev, "Failed to register v4l2 subdev: 
> > > > %d\n", err);
> > > > +   media_entity_cleanup(>subdev.entity);
> > > > +   return err;
> > > > +   }
> > > > +
> > > > +   dev_info(>dev, "video-bus-switch registered\n");
> > 
> > How about dev_dbg()?
> 
> Ok.
> 
> > > > +static int video_bus_switch_remove(struct platform_device *pdev)
> > > > +{
> > > > +   struct vbs_data *pdata = platform_get_drvdata(pdev);
> > > > +
> > > > +   v4l2_async_notifier_unregister(>notifier);

Re: [PATCH 1/6] staging: Import the BCM2835 MMAL-based V4L2 camera driver.

[Dave Stevenson]

Hi Mauro.

I'm going to stick my head above the parapet as one of the original 
authors back when I worked at Broadcom.
As it happens I started working at Raspberry Pi last Monday, so that 
puts me in a place where I can work on this again a bit more. (The last 
two years have been just a spare time support role).
Whilst I have done kernel development work in various roles, it's all 
been downstream so I've not been that active on these lists before.


All formatting/checkpatch comments noted.
Checkpatch was whinging when this was first written around December 2013 
about long lines, so many got broken up to shut it up. Views on code 
style and checkpatch seem to have changed a little since then.
I thought we had made checkpatch happy before the driver was pushed, but 
with some of the comments still having // style I guess some slipped 
through the net.
Yes chunks of this could do with refactoring to reduce the levels of 
indentation - always more to do.
If I've removed any formatting/style type comments in my cuts it's not 
because I'm ignoring them, just that they're not something that needs 
discussion (just fixing). I've only taken out the really big lumps of 
code with no comments on.


Newbie question: if this has already been merged to staging, where am I 
looking for the relevant tree to add patches on top of? 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git branch 
staging-next?


Responses to the rest inline.
TL;DR answer is that you are seeing the top edge of a full ISP 
processing pipe and optional encoders running on the GPU, mainly as 
there are blocks that can't be exposed for IP reasons (Raspberry Pi only 
being the customer not silicon vendor constrains what can and can't be 
made public).
That doesn't seem to fit very well into V4L2 which expects that it can 
see all the detail, so there are a few nasty spots to shoe-horn it in. 
If there are better ways to solve the problems, then I'm open to them.


Thanks
  Dave


On 03/02/17 18:59, Mauro Carvalho Chehab wrote:

HI Eric,

Em Fri, 27 Jan 2017 13:54:58 -0800
Eric Anholt  escreveu:


- Supports raw YUV capture, preview, JPEG and H264.
- Uses videobuf2 for data transfer, using dma_buf.
- Uses 3.6.10 timestamping
- Camera power based on use
- Uses immutable input mode on video encoder

This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
a15ba877dab4e61ea3fc7b006e2a73828b083c52.


First of all, thanks for that! Having an upstream driver for the
RPi camera is something that has been long waited!

Greg was kick on merging it on staging ;) Anyway, the real review
will happen when the driver becomes ready to be promoted out of
staging. When you address the existing issues and get it ready to
merge, please send the patch with such changes to linux-media ML.
I'll do a full review on it by then.


Is that even likely given the dependence on VCHI? I wasn't expecting 
VCHI to leave staging, which would force this to remain too.



Still, let me do a quick review on this driver, specially at the
non-MMAL code.



Signed-off-by: Eric Anholt 
---
 .../media/platform/bcm2835/bcm2835-camera.c| 2016 
 .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
 drivers/staging/media/platform/bcm2835/controls.c  | 1345 +
 .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
 .../media/platform/bcm2835/mmal-encodings.h|  127 ++
 .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
 .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
 .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
 drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
 .../media/platform/bcm2835/mmal-parameters.h   |  689 +++
 .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916 +++
 .../staging/media/platform/bcm2835/mmal-vchiq.h|  178 ++
 12 files changed, 7111 insertions(+)
 create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c
 create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h
 create mode 100644 drivers/staging/media/platform/bcm2835/controls.c
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c
 create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
new file mode 100644

Re: linux-next: build failure after merge of the v4l-dvb tree

[Mauro Carvalho Chehab]

Em Mon, 6 Feb 2017 09:19:14 +1100
Stephen Rothwell  escreveu:

> Hi Mauro,
> 
> On Thu, 2 Feb 2017 22:24:35 -0200 Mauro Carvalho Chehab 
>  wrote:
> >
> > So, if this is not a problem to you, maybe you can setup your
> > environment to pull (in this order) from:
> > 
> > git://linuxtv.org/media_tree.git fixes
> > git://linuxtv.org/media_tree.git master
> > git://linuxtv.org/mchehab/media-next.git master
> > 
> > Most of the time, the last pull won't get anything.  
> 
> OK, from today I have those three trees called v4l-dvb-fixes, v4l-dvb
> and v4l-dvb-next respectively.  We'll see how it goes.

OK!

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: Add video bus switch

[Pavel Machek]

Hi!

I lost my original reply... so this will be slightly brief.

> > > + * 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.
> > > + */
> > > +
> > > +#define DEBUG
> 
> Please remove.

Ok.

> > > +#include 
> > > +#include 
> 
> Alphabetical order, please.

Ok. (But let me make unhappy noise, because these rules are
inconsistent across kernel.)

> > > + * TODO:
> > > + * isp_subdev_notifier_complete() calls 
> > > v4l2_device_register_subdev_nodes()
> > > + */
> > > +
> > > +#define CSI_SWITCH_SUBDEVS 2
> > > +#define CSI_SWITCH_PORTS 3
> 
> This could go to the enum below.
> 
> I guess the CSI_SWITCH_SUBDEVS could be (CSI_SWITCH_PORTS - 1).
> 
> I'd just replace CSI_SWITCH by VBS. The bus could be called
> differently.

Ok.

> > > +static int vbs_registered(struct v4l2_subdev *sd)
> > > +{
> > > + struct v4l2_device *v4l2_dev = sd->v4l2_dev;
> > > + struct vbs_data *pdata;
> > > + int err;
> > > +
> > > + dev_dbg(sd->dev, "registered, init notifier...\n");
> 
> Looks like a development time debug message. :-)

ex-development message ;-).

> > > + gpiod_set_value(pdata->swgpio, local->index == CSI_SWITCH_PORT_2);
> > > + pdata->state = local->index;
> > > +
> > > + sd = vbs_get_remote_subdev(sd);
> > > + if (IS_ERR(sd))
> > > + return PTR_ERR(sd);
> > > +
> > > + pdata->subdev.ctrl_handler = sd->ctrl_handler;
> 
> This is ugly. You're exposing all the controls through another sub-device.
> 
> How does link validation work now?
> 
> I wonder if it'd be less so if you just pass through the LINK_FREQ and
> PIXEL_RATE controls. It'll certainly be more code though.
> 
> I think the link frequency could be something that goes to the frame
> descriptor as well. Then we wouldn't need to worry about the controls
> separately, just passing the frame descriptor would be enough.
> 
> I apologise that I don't have patches quite ready for posting to the
> list.

(Plus of course question is "what is link validation".)

Ok, let me play with this one. Solution you are suggesting is to make
a custom harndler that only passes certain data through, right?

> > > + dev_dbg(pdata->subdev.dev, "create link: %s[%d] -> %s[%d])\n",
> > > + src->name, src_pad, sink->name, sink_pad);
> > > + }
> > > +
> > > + return v4l2_device_register_subdev_nodes(pdata->subdev.v4l2_dev);
> 
> The ISP driver's complete() callback calls
> v4l2_device_register_subdev_nodes() already. Currently it cannot handle
> being called more than once --- that needs to be fixed.

I may have patches for that. Let me check.

> > > +}
> > > +
> > > +
> 
> I'd say that's an extra newline.

Not any more.

> > > + v4l2_subdev_init(>subdev, _ops);
> > > + pdata->subdev.dev = >dev;
> > > + pdata->subdev.owner = pdev->dev.driver->owner;
> > > + strncpy(pdata->subdev.name, dev_name(>dev), 
> > > V4L2_SUBDEV_NAME_SIZE);
> 
> How about sizeof(pdata->subdev.name) ?

Ok.

> I'm not sure I like V4L2_SUBDEV_NAME_SIZE in general. It could be even
> removed. But not by this patch. :-)
> 
> > > + v4l2_set_subdevdata(>subdev, pdata);
> > > + pdata->subdev.entity.function = MEDIA_ENT_F_SWITCH;
> > > + pdata->subdev.entity.flags |= MEDIA_ENT_F_SWITCH;
> 
> MEDIA_ENT_FL_*

Do we actually have a flag here? We already have .function, so this
looks like a duplicate.


> > > + if (err < 0) {
> > > + dev_err(>dev, "Failed to register v4l2 subdev: %d\n", 
> > > err);
> > > + media_entity_cleanup(>subdev.entity);
> > > + return err;
> > > + }
> > > +
> > > + dev_info(>dev, "video-bus-switch registered\n");
> 
> How about dev_dbg()?

Ok.

> > > +static int video_bus_switch_remove(struct platform_device *pdev)
> > > +{
> > > + struct vbs_data *pdata = platform_get_drvdata(pdev);
> > > +
> > > + v4l2_async_notifier_unregister(>notifier);
> 
> Shouldn't you unregister the notifier in the .unregister() callback?

Ok, I guess we can do that for symetry.

> > >  /* generic v4l2_device notify callback notification values */
> > >  #define V4L2_SUBDEV_IR_RX_NOTIFY _IOW('v', 0, u32)
> > > @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> > >const struct v4l2_mbus_config *cfg);
> > >   int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> > >  unsigned int *size);
> > > + int (*g_endpoint_config)(struct v4l2_subdev *sd,
> > > + struct v4l2_of_endpoint *cfg);
> 
> This should be in a separate patch --- assuming we'll add this one.

Hmm. I believe the rest of the driver is quite useful in understanding
this. Ok, lets get the discussion started.

> > > --- a/include/uapi/linux/media.h
> > > +++ b/include/uapi/linux/media.h
> > > @@ -147,6 +147,7 @@ struct media_device_info {
> > >   * MEDIA_ENT_F_IF_VID_DECODER and/or MEDIA_ENT_F_IF_AUD_DECODER.
> > >   */
> > > 

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

Hi!

> > 9) Highly reconfigurable hardware - Julien Beraud
> > 
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > 
> > but the patches are from 2005. So I guess I'll need some guidance here...
> 
> Yeah, that's where it began (2015?), but right now I can only suggest to
> wait until there's more. My estimate is within next couple of weeks /
> months. But it won't be years.

Ok, week or two would be ok, couple of months is not. And all I need
is single hook in common structure.

So if g_endpoint_config hook looks sane to _you_, I suggest we simply
proceed. Now, maybe Mauro Carvalho Chehab 
or Laurent or Julien will want a different solution, but
then... they'll have to suggest something doable now, not in couple of
months.

Does that sound like a plan?

Mauro added to cc list, so we can get some input.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v3 15/24] media: Add userspace header file for i.MX

[Laurent Pinchart]

Hi Steve,

On Friday 13 Jan 2017 15:13:33 Steve Longerbeam wrote:
> On 01/13/2017 04:05 AM, Philipp Zabel wrote:
> > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> >> This adds a header file for use by userspace programs wanting to interact
> >> with the i.MX media driver. It defines custom v4l2 controls and events
> >> generated by the i.MX v4l2 subdevices.
> >> 
> >> Signed-off-by: Steve Longerbeam 
> >> ---
> >> 
> >>   include/uapi/media/Kbuild |  1 +
> >>   include/uapi/media/imx.h  | 30 ++
> >>   2 files changed, 31 insertions(+)
> >>   create mode 100644 include/uapi/media/imx.h
> >> 
> >> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> >> index aafaa5a..fa78958 100644
> >> --- a/include/uapi/media/Kbuild
> >> +++ b/include/uapi/media/Kbuild
> >> @@ -1 +1,2 @@
> >> 
> >>   # UAPI Header export list
> >> 
> >> +header-y += imx.h
> >> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> >> new file mode 100644
> >> index 000..2421d9c
> >> --- /dev/null
> >> +++ b/include/uapi/media/imx.h
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> the + * Free Software Foundation; either version 2 of the
> >> + * License, or (at your option) any later version
> >> + */
> >> +
> >> +#ifndef __UAPI_MEDIA_IMX_H__
> >> +#define __UAPI_MEDIA_IMX_H__
> >> +
> >> +/*
> >> + * events from the subdevs
> >> + */
> >> +#define V4L2_EVENT_IMX_CLASS  V4L2_EVENT_PRIVATE_START
> >> +#define V4L2_EVENT_IMX_NFB4EOF(V4L2_EVENT_IMX_CLASS + 1)
> >> +#define V4L2_EVENT_IMX_EOF_TIMEOUT(V4L2_EVENT_IMX_CLASS + 2)
> >> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
> > 
> > Aren't these generic enough to warrant common events? I would think
> > there have to be other capture IP cores that can signal aborted frames
> > or frame timeouts.
> 
> Yes, agreed. A frame capture timeout, or frame interval error, are
> both generic concepts. At some point it would be great to make the
> Frame Interval Monitor generally available under v4l2-core. As for the
> EOF timeout event, I'll look into moving that into a generic V4L2 event.

I'd prefer generic events if possible, but regardless of whether that's 
possible, the events must be documented in the V4L2 specification.

-- 
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 v3 13/24] platform: add video-multiplexer subdevice driver

[Laurent Pinchart]

Hi Steve,

On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:
> On 01/24/2017 04:02 AM, Philipp Zabel wrote:
> > On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:
> >>> +
> >>> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
> >>> *cfg)
> >>> +{
> >>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> >>> + struct media_pad *pad;
> >>> + int ret;
> >>> +
> >>> + if (vidsw->active == -1) {
> >>> + dev_err(sd->dev, "no configuration for inactive mux\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + /*
> >>> +  * Retrieve media bus configuration from the entity connected to the
> >>> +  * active input
> >>> +  */
> >>> + pad = media_entity_remote_pad(>pads[vidsw->active]);
> >>> + if (pad) {
> >>> + sd = media_entity_to_v4l2_subdev(pad->entity);
> >>> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
> >>> + if (ret == -ENOIOCTLCMD)
> >>> + pad = NULL;
> >>> + else if (ret < 0) {
> >>> + dev_err(sd->dev, "failed to get source 
configuration\n");
> >>> + return ret;
> >>> + }
> >>> + }
> >>> + if (!pad) {
> >>> + /* Mirror the input side on the output side */
> >>> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
> >>> + if (cfg->type == V4L2_MBUS_PARALLEL ||
> >>> + cfg->type == V4L2_MBUS_BT656)
> >>> + cfg->flags = vidsw->endpoint[vidsw-
>active].bus.parallel.flags;
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >> 
> >> I am not certain this op is needed at all. In the current kernel this op
> >> is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
> >> Normally this information should come from the device tree and there
> >> should be no need for this op.
> >> 
> >> My (tentative) long-term plan was to get rid of this op.
> >> 
> >> If you don't need it, then I recommend it is removed.
> 
> Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
> sensor, and it was doing that to determine the sensor's bus type. This info
> was already available from parsing a v4l2_of_endpoint from the sensor node.
> So it was simple to remove the g_mbus_config calls, and instead rely on the
> parsed sensor v4l2_of_endpoint.

That's not a good point. The imx-media driver must not parse the sensor DT 
node as it is not aware of what bindings the sensor is compatible with. 
Information must instead be queried from the sensor subdev at runtime, through 
the g_mbus_config() operation.

Of course, if you can get the information from the imx-media DT node, that's 
certainly an option. It's only information provided by the sensor driver that 
you have no choice but query using a subdev operation.

> > We currently use this to make the CSI capture interface understand
> > whether its source from the MIPI CSI-2 or from the parallel bus. That is
> > probably something that should be fixed, but I'm not quite sure how.
> > 
> > The Synopsys DesignWare MIPI CSI-2 reciever turns the incoming MIPI
> > CSI-2 signal into a 32-bit parallel pixel bus plus some signals for the
> > MIPI specific metadata (virtual channel, data type).
> > 
> > Then the CSI2IPU gasket turns this input bus into four separate parallel
> > 16-bit pixel buses plus an 8-bit "mct_di" bus for each of them, that
> > carries the MIPI metadata. The incoming data is split into the four
> > outputs according to the MIPI virtual channel.
> > 
> > Two of these 16-bit + 8-bit parallel buses are routed through a
> > multiplexer before finally arriving at the CSI on the other side.
> > 
> > We need to configure the CSI to either use or ignore the data from the
> > 8-bit mct_di bus depending on whether the source of the mux is
> > configured to the MIPI CSI-2 receiver / CSI2IPU gasket, or to a parallel
> > input.
> 
> Philipp, from my experience, the CSI_MIPI_DI register (configured
> by ipu_csi_set_mipi_datatype()) can only be given a virtual channel 0,
> otherwise no data is received from the MIPI CSI-2 sensor, regardless
> of the virtual channel the sensor is transmitting over.
> 
> So it seems the info on the 8-bit mct_di buses generated by the CSI2IPU
> gasket are ignored by the CSI's, at least the virtual channel number is
> ignored.
> 
> For example, if the sensor is transmitting on vc 1, the gasket routes
> the sensor data to the parallel bus to CSI1. But if CSI_MIPI_DI on CSI1
> is written with vc 1, no data is received.
> 
> Steve
> 
> > Currently we let g_mbus_config pretend that even the internal 32-bit +
> > metadata and 16-bit + 8-bit metadata parallel buses are of type
> > V4L2_MBUS_CSI so that the CSI can ask the mux, which propagates to the
> > CSI-2 receiver, if connected.
> > 
> > Without g_mbus_config we'd need to get that information from somewhere
> > else. One possibility would be to extend MEDIA_BUS formats to describe
> > these "parallelized MIPI data" buses separately.

-- 
Regards,

Laurent Pinchart

--
To 

Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Russell King - ARM Linux]

On Sun, Feb 05, 2017 at 05:24:30PM +0200, Laurent Pinchart wrote:
> Hi Russell,
> 
> On Monday 30 Jan 2017 22:51:33 Russell King - ARM Linux wrote:
> > > + ov5640_to_mipi_csi: endpoint@1 {
> > > + reg = <1>;
> > > + remote-endpoint = 
> <_csi_from_mipi_sensor>;
> > > + data-lanes = <0 1>;
> > > + clock-lanes = <2>;
> > 
> > How do you envision a four-lane sensor being described?
> > 
> > data-lanes = <0 1 3 4>;
> > clock-lanes = <2>;
> > 
> > ?
> > 
> > The binding document for video-interfaces.txt says:
> > 
> > - clock-lanes: an array of physical clock lane indexes. Position of an entry
> > determines the logical lane number, while the value of an entry indicates
> > physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes =
> > <0>;", which places the clock lane on hardware lane 0. This property is
> > valid for serial busses only (e.g. MIPI CSI-2). Note that for the MIPI
> > CSI-2 bus this array contains only one entry.
> > 
> > So I think you need to have a good reason to make the clock lane non-zero.
> 
> The purpose of the data-lanes and clock-lanes properties is to describe lane 
> assignment for hardware that supports lane routing. As far as I know the 
> OV5640 doesn't support lane routing and has dedicated pins for the clock and 
> data lanes. The data-lanes and clock-lanes properties should probably not be 
> specified at all.

You need at least data-lanes so you know how many data lanes are wired
between the camera and the mipi csi2 receiver.  Just because a camera
has (eg) four lanes does not mean you need to wire all four, and in
some cases less than four will be wired.

If data-lanes does not describe that, then all existing users of the
binding are abusing it:

$ grep data_lanes drivers/media/i2c -r
drivers/media/i2c/s5k5baf.c:state->nlanes = 
ep.bus.mipi_csi2.num_data_lanes;
drivers/media/i2c/s5c73m3/s5c73m3-core.c:   if (ep.bus.mipi_csi2.num_data_lanes 
!= S5C73M3_MIPI_DATA_LANES)
drivers/media/i2c/tc358743.c:   endpoint->bus.mipi_csi2.num_data_lanes 
== 0 ||
drivers/media/i2c/smiapp/smiapp-core.c: hwcfg->lanes = 
bus_cfg->bus.mipi_csi2.num_data_lanes;

So all those drivers are using it for the _number_ of CSI2 lanes, and
are not touching the mapping in any way (not even checking that it is
an identity mapping.)  You could specify any mapping to these drivers,
as long as num_data_lanes came out right.

And... there's no point having a property in a binding if no one is
using it... and even more silly not to have a property that everyone
needs...

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 12/24] add mux and video interface bridge entity functions

[Laurent Pinchart]

Hi Steve,

Thank you for the patch

On Friday 06 Jan 2017 18:11:30 Steve Longerbeam wrote:
> From: Philipp Zabel 
> 
> Signed-off-by: Philipp Zabel 
> ---
>  Documentation/media/uapi/mediactl/media-types.rst | 22 
>  include/uapi/linux/media.h|  6 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/media/uapi/mediactl/media-types.rst
> b/Documentation/media/uapi/mediactl/media-types.rst index 3e03dc2..023be29
> 100644
> --- a/Documentation/media/uapi/mediactl/media-types.rst
> +++ b/Documentation/media/uapi/mediactl/media-types.rst
> @@ -298,6 +298,28 @@ Types and flags used to represent the media graph
> elements received on its sink pad and outputs the statistics data on
> its source pad.
> 
> +-  ..  row 29
> +
> +   ..  _MEDIA-ENT-F-MUX:
> +
> +   -  ``MEDIA_ENT_F_MUX``
> +
> +   - Video multiplexer. An entity capable of multiplexing must have at
> + least two sink pads and one source pad, and must pass the video
> + frame(s) received from the active sink pad to the source pad.
> Video
> + frame(s) from the inactive sink pads are discarded.

Apart from the comment made by Hans regarding the macro name, this looks good 
to me.

> +-  ..  row 30
> +
> +   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
> +
> +   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
> +
> +   - Video interface bridge. A video interface bridge entity must have
> at
> + least one sink pad and one source pad. It receives video frame(s)
> on
> + its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
> + converts them and outputs them on its source pad in another bus
> format
> + (eDP, MIPI CSI-2, parallel, ...).


The first sentence mentions *at least* one sink pad and one source pad, and 
the second one refers to "its sink pad" and "its source pad". This is a bit 
confusing. An easy option would be to require a single sink and a single 
source pad, but that would exclude bridges that combine a multiplexer.

I also wonder whether "bridge" is the appropriate word here. Transceiver might 
be a better choice, to insist on the fact that one of the two pads corresponds 
to a physical interface that has special electrical properties (such as HDMI, 
eDP, or CSI-2 that all require PHYs).

>  ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 4890787..08a8bfa 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -105,6 +105,12 @@ struct media_device_info {
>  #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS(MEDIA_ENT_F_BASE + 0x4006)
> 
>  /*
> + * Switch and bridge entitites
> + */
> +#define MEDIA_ENT_F_MUX  (MEDIA_ENT_F_BASE + 
0x5001)
> +#define MEDIA_ENT_F_VID_IF_BRIDGE(MEDIA_ENT_F_BASE + 0x5002)
> +
> +/*
>   * Connectors
>   */
>  /* It is a responsibility of the entity drivers to add connectors and links
> */

-- 
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 v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Laurent Pinchart]

Hi Russell,

On Monday 30 Jan 2017 22:51:33 Russell King - ARM Linux wrote:
> On Fri, Jan 06, 2017 at 06:11:24PM -0800, Steve Longerbeam wrote:
> > +   ov5640: camera@40 {
> > +   compatible = "ovti,ov5640";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_ov5640>;
> > +   clocks = <_xclk>;
> > +   clock-names = "xclk";
> > +   reg = <0x40>;
> > +   xclk = <2200>;
> > +   reset-gpios = < 5 GPIO_ACTIVE_LOW>; /* NANDF_D5 */
> > +   pwdn-gpios = < 9 GPIO_ACTIVE_HIGH>; /* NANDF_WP_B */
> > +
> > +   port {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   ov5640_to_mipi_csi: endpoint@1 {
> > +   reg = <1>;
> > +   remote-endpoint = 
<_csi_from_mipi_sensor>;
> > +   data-lanes = <0 1>;
> > +   clock-lanes = <2>;
> 
> How do you envision a four-lane sensor being described?
> 
>   data-lanes = <0 1 3 4>;
>   clock-lanes = <2>;
> 
> ?
> 
> The binding document for video-interfaces.txt says:
> 
> - clock-lanes: an array of physical clock lane indexes. Position of an entry
> determines the logical lane number, while the value of an entry indicates
> physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes =
> <0>;", which places the clock lane on hardware lane 0. This property is
> valid for serial busses only (e.g. MIPI CSI-2). Note that for the MIPI
> CSI-2 bus this array contains only one entry.
> 
> So I think you need to have a good reason to make the clock lane non-zero.

The purpose of the data-lanes and clock-lanes properties is to describe lane 
assignment for hardware that supports lane routing. As far as I know the 
OV5640 doesn't support lane routing and has dedicated pins for the clock and 
data lanes. The data-lanes and clock-lanes properties should probably not be 
specified at all.

-- 
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 v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors

[Laurent Pinchart]

On Monday 16 Jan 2017 13:55:23 Philipp Zabel wrote:
> On Fri, 2017-01-13 at 15:04 -0800, Steve Longerbeam wrote:
> > On 01/13/2017 04:03 AM, Philipp Zabel wrote:
> > > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> > >> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2
> > >> sensor.
> > >> Both hang off the same i2c2 bus, so they require different (and non-
> > >> default) i2c slave addresses.
> > >> 
> > >> The OV5642 connects to the parallel-bus mux input port on
> > >> ipu1_csi0_mux.
> > >> 
> > >> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> > >> mipi_csi. It is set to transmit over MIPI virtual channel 1.
> > >> 
> > >> Signed-off-by: Steve Longerbeam 
> > >> ---
> > >> 
> > >>   arch/arm/boot/dts/imx6dl-sabrelite.dts   |   5 ++
> > >>   arch/arm/boot/dts/imx6q-sabrelite.dts|   6 ++
> > >>   arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 118 ++
> > >>   3 files changed, 129 insertions(+)
> > >> 
> > >> diff --git a/arch/arm/boot/dts/imx6dl-sabrelite.dts
> > >> b/arch/arm/boot/dts/imx6dl-sabrelite.dts index 0f06ca5..fec2524 100644
> > >> --- a/arch/arm/boot/dts/imx6dl-sabrelite.dts
> > >> +++ b/arch/arm/boot/dts/imx6dl-sabrelite.dts
> 
> [...]
> 
> > >> @@ -299,6 +326,52 @@
> > >> 
> > >>  pinctrl-names = "default";
> > >>  pinctrl-0 = <_i2c2>;
> > >>  status = "okay";
> > >> 
> > >> +
> > >> +ov5640: camera@40 {
> > >> +compatible = "ovti,ov5640";
> > >> +pinctrl-names = "default";
> > >> +pinctrl-0 = <_ov5640>;
> > >> +clocks = <_xclk>;
> > >> +clock-names = "xclk";
> > >> +reg = <0x40>;
> > >> +xclk = <2200>;
> > > 
> > > This is superfluous, you can use clk_get_rate on mipi_xclk.
> > 
> > This property is actually there to tell the driver what to set the
> > rate to, with clk_set_rate(). So you are saying it would be better
> > to set the rate in the device tree and the driver should only
> > retrieve the rate?
> 
> Yes. Given that this is a reference clock input that is constant on a
> given board and never changes during runtime, I think this is the
> correct way. The clock will be fixed rate on most boards, I assume.

I think it's a bit worse than that. The ov5640 and ov5642 drivers should 
retrieve the clock rate and compute register values accordingly (PLL 
configuration parameters for instance, but most probably other values as 
well). Unfortunately, as usual with Omnivision, the lack of public and even 
non-public information results in drivers hardcoding large lists of 
register/value pairs that have been computed for a specific clock frequency. 
The drivers will thus not operate correctly if the clock is running at a 
different rate. Until that can be fixed, the best option is probably to assign 
the rate in the device tree as Philipp proposed, and to use clk_get_rate() in 
the driver to reject any rate other than 22MHz.

-- 
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 2/2] [media] dvb-usb-firmware: don't do DMA on stack

[Stefan Brüns]

The USB control messages require DMA to work. We cannot pass
a stack-allocated buffer, as it is not warranted that the
stack would be into a DMA enabled area.

Signed-off-by: Stefan Brüns 
---
 drivers/media/usb/dvb-usb/dvb-usb-firmware.c | 30 
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dvb-usb-firmware.c 
b/drivers/media/usb/dvb-usb/dvb-usb-firmware.c
index dd048a7c461c..189b6725edd0 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-firmware.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-firmware.c
@@ -35,41 +35,45 @@ static int usb_cypress_writemem(struct usb_device *udev,u16 
addr,u8 *data, u8 le
 
 int usb_cypress_load_firmware(struct usb_device *udev, const struct firmware 
*fw, int type)
 {
-   struct hexline hx;
-   u8 reset;
-   int ret,pos=0;
+   u8 *buf = kmalloc(sizeof(struct hexline), GFP_KERNEL);
+   struct hexline *hx = (struct hexline *)buf;
+   int ret, pos = 0;
+   u16 cpu_cs_register = cypress[type].cpu_cs_register;
 
/* stop the CPU */
-   reset = 1;
-   if ((ret = 
usb_cypress_writemem(udev,cypress[type].cpu_cs_register,,1)) != 1)
+   buf[0] = 1;
+   if (usb_cypress_writemem(udev, cpu_cs_register, buf, 1) != 1)
err("could not stop the USB controller CPU.");
 
-   while ((ret = dvb_usb_get_hexline(fw,,)) > 0) {
-   deb_fw("writing to address 0x%04x (buffer: 0x%02x 
%02x)\n",hx.addr,hx.len,hx.chk);
-   ret = usb_cypress_writemem(udev,hx.addr,hx.data,hx.len);
+   while ((ret = dvb_usb_get_hexline(fw, hx, )) > 0) {
+   deb_fw("writing to address 0x%04x (buffer: 0x%02x %02x)\n",
+  hx->addr, hx->len, hx->chk);
+   ret = usb_cypress_writemem(udev, hx->addr, hx->data, hx->len);
 
-   if (ret != hx.len) {
+   if (ret != hx->len) {
err("error while transferring firmware "
"(transferred size: %d, block size: %d)",
-   ret,hx.len);
+   ret, hx->len);
ret = -EINVAL;
break;
}
}
if (ret < 0) {
-   err("firmware download failed at %d with %d",pos,ret);
+   err("firmware download failed at %d with %d", pos, ret);
+   kfree(buf);
return ret;
}
 
if (ret == 0) {
/* restart the CPU */
-   reset = 0;
-   if (ret || 
usb_cypress_writemem(udev,cypress[type].cpu_cs_register,,1) != 1) {
+   buf[0] = 0;
+   if (usb_cypress_writemem(udev, cpu_cs_register, buf, 1) != 1) {
err("could not restart the USB controller CPU.");
ret = -EINVAL;
}
} else
ret = -EIO;
+   kfree(buf);
 
return ret;
 }
-- 
2.11.0

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

[PATCH 1/2] [media] cxusb: Use a dma capable buffer also for reading

[Stefan Brüns]

Commit 17ce039b4e54 ("[media] cxusb: don't do DMA on stack")
added a kmalloc'ed bounce buffer for writes, but missed to do the same
for reads. As the read only happens after the write is finished, we can
reuse the same buffer.

As dvb_usb_generic_rw handles a read length of 0 by itself, avoid calling
it using the dvb_usb_generic_read wrapper function.

Signed-off-by: Stefan Brüns 
---
 drivers/media/usb/dvb-usb/cxusb.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/cxusb.c 
b/drivers/media/usb/dvb-usb/cxusb.c
index 9b8c82d94b3f..8f28a63597bd 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -59,23 +59,24 @@ static int cxusb_ctrl_msg(struct dvb_usb_device *d,
  u8 cmd, u8 *wbuf, int wlen, u8 *rbuf, int rlen)
 {
struct cxusb_state *st = d->priv;
-   int ret, wo;
+   int ret;
 
if (1 + wlen > MAX_XFER_SIZE) {
warn("i2c wr: len=%d is too big!\n", wlen);
return -EOPNOTSUPP;
}
 
-   wo = (rbuf == NULL || rlen == 0); /* write-only */
+   if (rlen > MAX_XFER_SIZE) {
+   warn("i2c rd: len=%d is too big!\n", rlen);
+   return -EOPNOTSUPP;
+   }
 
mutex_lock(>data_mutex);
st->data[0] = cmd;
memcpy(>data[1], wbuf, wlen);
-   if (wo)
-   ret = dvb_usb_generic_write(d, st->data, 1 + wlen);
-   else
-   ret = dvb_usb_generic_rw(d, st->data, 1 + wlen,
-rbuf, rlen, 0);
+   ret = dvb_usb_generic_rw(d, st->data, 1 + wlen, st->data, rlen, 0);
+   if (!ret && rbuf && rlen)
+   memcpy(rbuf, st->data, rlen);
 
mutex_unlock(>data_mutex);
return ret;
-- 
2.11.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

media_build handles FRAME_VECTOR incorrect

[Matthias Schwarzott]

Hi!

Compiling the media drivers out of tree with media_build does not handle
FRAME_VECTOR correctly.

My kernel sources have FRAME_VECTOR but it is not enabled. So all
modules depending on it (e.g. VIDEOBUF2_MEMOPS) will fail to load
because the relevant functions cannot be found.

I see two options:
1. Modify the real kernel sources to allow FRAME_VECTOR be enabled via
config (either y or m).

I locally just modified my kernel's mm/KConfig like this:

 config FRAME_VECTOR
-bool
+tristate "enable frame_vector for external modules"

2. Change media_build to supply a replacement when it is not enabled in
the system kernel.

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