->subdev, &pdev->dev);
> + snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> + KBUILD_MODNAME, dev_name(&pdev->dev));
> + priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->subdev.entity.ops = &rcar_csi2_entity_ops;
> +
> + priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> + for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> + priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(&priv->subdev.entity, NR_OF_RCAR_CSI2_PAD,
> + priv->pads);
> + if (ret)
> + goto error;
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = v4l2_async_register_subdev(&priv->subdev);
> + if (ret < 0)
> + goto error;
> +
> + dev_info(priv->dev, "%d lanes found\n", priv->lanes);
priv->lanes is unsigned you should use %u.
> +
> + return 0;
> +
> +error:
> + v4l2_async_notifier_unregister(&priv->notifier);
> + v4l2_async_notifier_cleanup(&priv->notifier);
> +
> + return ret;
> +}
[snip]
With these small issues fixed and Kieran's and Maxime's comments addressed as
you see fit,
Reviewed-by: Laurent Pinchart
--
Regards,
Laurent Pinchart
ices. There are only
> a few possible routes which are set by hardware limitations, which are
> different for each SoC in the Gen3 family.
>
> Signed-off-by: Niklas Söderlund
> Acked-by: Rob Herring
> Acked-by: Sakari Ailus
Reviewed-by: Laurent Pinchart
> ---
> .../bin
t;
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 6 ++
> drivers/media/platform/rcar-vin/rcar-dma.c | 7 ++-
> drivers/media/platform/rcar-vin/rcar-v4l2.c |
> - pix->pixelformat = RVIN_DEFAULT_FORMAT;
> -
> - /* Limit to source capabilities */
> - ret = __rvin_try_format_source(vin, which, pix, source);
> - if (ret)
> - return ret;
> -
> - return rvin_format_align(vin, pix);
> + ret
+ * Driver dose not (yet) support outputting ALTERNATE to a
s/dose/does/
Apart from that, I'll overlook the fact that if rvin_reset_format() function
is called from S_STD or S_DV_TIMINGS and userspace calls G_FMT immediately
after that, the field will be set to V4L2_FIELD_ALT
> + case V4L2_FIELD_INTERLACED_BT:
> + case V4L2_FIELD_INTERLACED:
> + break;
> + case V4L2_FIELD_ALTERNATE:
> + /*
> + * Driver dose not (yet) support outputting ALTERNATE to a
s/dose/does/
Apart from that,
Reviewed-by: Laurent Pinchart
> + * usersp
this mode a different set
> of v4l2 operations needs to be used.
>
> Add a new set of v4l2 operations to support operation without directly
> interacting with the source subdevice.
>
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinc
the API. Until the API is updated to
> allow for userspace to set these Hans wants the fields to be set by the
> driver to fixed values.
>
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinchart
> ---
> drivers/media
he struct rvin_info.
>
> Signed-off-by: Niklas Söderlund
> Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinchart
> ---
>
> * Changes since v11
> - Fixed spelling.
> - Reorderd filed order in struct rvin_group_route.
> - Renamed chan to channel in struct rvin_group_ro
MLINK;
> + goto out;
> + }
> +
> + /* New valid CHSEL found, set the new value. */
> + ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
> +out:
> + mutex_unlock(&group->lock);
> +
> + return ret;
> +}
> +
> +static const struct media_device_ops rvin_media_ops = {
> + .link_notify = rvin_group_link_notify,
> +};
> +
> /*
> ---
> -- * Gen3 CSI2 Group Allocator
> */
> @@ -85,6 +231,7 @@ static int rvin_group_init(struct rvin_group *group,
> struct rvin_dev *vin) vin_dbg(vin, "found %u enabled VIN's in DT",
> group->count);
>
> mdev->dev = vin->dev;
> + mdev->ops = &rvin_media_ops;
>
> match = of_match_node(vin->dev->driver->of_match_table,
> vin->dev->of_node);
--
Regards,
Laurent Pinchart
s pointed out by Geert and Sergei,
> >
> > thanks!
> >
> > - Refresh patch 2/32 with an updated version, thanks Sakari for pointing
> >
> > this out.
> >
> > - Add Sakaris Ack to patch 1/32.
> > - Rebase on top of v4.9-rc1 instead of v4.9-rc3 to ease integration
> >
> > testing together with renesas-drivers tree.
> >
> > Fabrizio Castro (2):
> > dt-bindings: media: rcar_vin: Reverse SoC part number list
> > dt-bindings: media: rcar_vin: add device tree support for r8a774[35]
> >
> > Niklas Söderlund (31):
> > rcar-vin: add Gen3 devicetree bindings documentation
> > rcar-vin: rename poorly named initialize and cleanup functions
> > rcar-vin: unregister video device on driver removal
> > rcar-vin: move subdevice handling to async callbacks
> > rcar-vin: move model information to own struct
> > rcar-vin: move max width and height information to chip information
> > rcar-vin: move functions regarding scaling
> > rcar-vin: all Gen2 boards can scale simplify logic
> > rcar-vin: set a default field to fallback on
> > rcar-vin: fix handling of single field frames (top, bottom and
> >
> > alternate fields)
> >
> > rcar-vin: update bytesperline and sizeimage calculation
> > rcar-vin: align pixelformat check
> > rcar-vin: break out format alignment and checking
> > rcar-vin: simplify how formats are set and reset
> > rcar-vin: cache video standard
> > rcar-vin: move media bus configuration to struct rvin_dev
> > rcar-vin: enable Gen3 hardware configuration
> > rcar-vin: add function to manipulate Gen3 chsel value
> > rcar-vin: add flag to switch to media controller mode
> > rcar-vin: use different v4l2 operations in media controller mode
> > rcar-vin: force default colorspace for media centric mode
> > rcar-vin: prepare for media controller mode initialization
> > rcar-vin: add group allocator functions
> > rcar-vin: change name of video device
> > rcar-vin: add chsel information to rvin_info
> > rcar-vin: parse Gen3 OF and setup media graph
> > rcar-vin: add link notify for Gen3
> > rcar-vin: extend {start,stop}_streaming to work with media controller
> > rcar-vin: enable support for r8a7795
> > rcar-vin: enable support for r8a7796
> > rcar-vin: enable support for r8a77970
> >
> > .../devicetree/bindings/media/rcar_vin.txt | 137 ++-
> > drivers/media/platform/rcar-vin/Kconfig| 2 +-
> > drivers/media/platform/rcar-vin/rcar-core.c| 962
> > +++-- drivers/media/platform/rcar-vin/rcar-dma.c
> > | 785 ++--- drivers/media/platform/rcar-vin/rcar-v4l2.c
> >| 488 ++- drivers/media/platform/rcar-vin/rcar-vin.h |
> > 146 +++-
> > 6 files changed, 1913 insertions(+), 607 deletions(-)
--
Regards,
Laurent Pinchart
Hi Kieran,
On Thursday, 29 March 2018 14:37:07 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > When disabling a DRM plane, the corresponding RPF is only marked as
> > removed from the pipeline in the atomic update handler, with the actual
> > r
Hi Kieran,
On Thursday, 29 March 2018 14:49:12 EEST Kieran Bingham wrote:
> Hi Laurent,
>
> Thank you for another patch :D
You're welcome, there will be more ;-)
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > In order to make the vsp1_du_setup_lif() easier to read, an
Hi Sakari,
On Thursday, 29 March 2018 10:35:49 EEST Sakari Ailus wrote:
> On Thu, Mar 29, 2018 at 09:19:43AM +0300, Laurent Pinchart wrote:
> > On Wednesday, 28 March 2018 23:16:08 EEST Sakari Ailus wrote:
> > > On Wed, Mar 28, 2018 at 02:59:22PM -0300, Mauro Carvalho Chehab
Hi Kieran,
On Wednesday, 28 March 2018 17:43:13 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > The DRM pipeline setup code used at atomic commit time is similar to the
> > setup code used when enabling the pipeline. Move it to a separate
> > funct
Hi Kieran,
On Wednesday, 28 March 2018 17:10:10 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > The DRM pipeline handling code uses the entity's pipe list head to check
> > whether the entity is already included in a pipeline. This method is a
&g
Hi Kieran,
On Wednesday, 28 March 2018 22:04:49 EEST Kieran Bingham wrote:
> On 28/03/18 13:27, Kieran Bingham wrote:
> > On 26/02/18 21:45, Laurent Pinchart wrote:
> >> The entities in the pipeline are all started when the LIF is setup.
> >> Remove the outdated c
mpat-ioctl32.c
> > @@ -101,7 +101,7 @@ static int get_v4l2_window32(struct v4l2_window __user
> > *kp,
> > static int put_v4l2_window32(struct v4l2_window __user *kp,
> > struct v4l2_window32 __user *up)
> > {
> > - struct v4l2_clip __user *kclips = kp->clips;
> > + struct v4l2_clip __user *kclips;
> > struct v4l2_clip32 __user *uclips;
> > compat_caddr_t p;
> > u32 clipcount;
> > @@ -116,6 +116,8 @@ static int put_v4l2_window32(struct v4l2_window __user
> > *kp,
> > if (!clipcount)
> > return 0;
> >
> > + if (get_user(kclips, &kp->clips))
> > + return -EFAULT;
> > if (get_user(p, &up->clips))
> > return -EFAULT;
> > uclips = compat_ptr(p);
>
> Good find. I checked for similar problems elsewhere in the file but could
> not find any.
>
> Reviewed-by: Sakari Ailus
Would it be useful to rename kp to something that doesn't imply the memory is
kernel memory ? I was confused at first when reading this patch.
--
Regards,
Laurent Pinchart
Hi Mauro,
On Monday, 26 March 2018 23:28:55 EEST Mauro Carvalho Chehab wrote:
> Em Mon, 26 Mar 2018 21:47:33 +0300 Laurent Pinchart escreveu:
> > On Monday, 26 March 2018 13:08:16 EEST Mauro Carvalho Chehab wrote:
> > > Em Fri, 23 Mar 2018 23:53:56 +0200 Sakari Ailus escreveu:
&
memset(parg, 0, size);
> + memset(parg, 0, ioc_size);
> }
> }
>
> @@ -2932,7 +2932,7 @@ video_usercopy(struct file *file, unsigned int cmd,
> unsigned long arg, switch (_IOC_DIR(cmd)) {
> case _IOC_READ:
> case (_IOC_WRITE | _IOC_READ):
> - if (copy_to_user((void __user *)arg, parg, size))
> + if (copy_to_user((void __user *)arg, parg, ioc_size))
> err = -EFAULT;
> break;
> }
--
Regards,
Laurent Pinchart
t;v4l: vsp1: Add support for the BRS entity")
> Cc: sta...@vger.kernel.org
>
> Suggested-by: Mauro Carvalho Chehab
> Signed-off-by: Kieran Bingham
Looking at commit 5d0beeec59e303c76160ddd67fa73dcfc5d76de0 I think your patch
is correct.
Reviewed-by: Laurent Pinchart
and tak
Hi Guennadi,
Thank you for the patch.
On Friday, 23 March 2018 11:24:00 EET Laurent Pinchart wrote:
> From: Guennadi Liakhovetski
>
> UVC defines a method of handling asynchronous controls, which sends a
> USB packet over the interrupt pipe. This patch implements support for
>
ret = -EIO;
goto out;
}
@@ -290,7 +336,8 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
uvc_printk(KERN_ERR, "Failed to set UVC %s control : "
"%d (exp. %u).\n", probe ? "probe&q
n *chain, struct v4l2_ext_control
*xctrl);
-int uvc_ctrl_set(struct uvc_video_chain *chain, struct v4l2_ext_control
*xctrl);
+int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
struct uvc_xu_control_query *xqry);
diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
index 020714d2c5bd..f80f05b3c423 100644
--- a/include/uapi/linux/uvcvideo.h
+++ b/include/uapi/linux/uvcvideo.h
@@ -28,6 +28,8 @@
#define UVC_CTRL_FLAG_RESTORE (1 << 6)
/* Control can be updated by the camera. */
#define UVC_CTRL_FLAG_AUTO_UPDATE (1 << 7)
+/* Control supports asynchronous reporting */
+#define UVC_CTRL_FLAG_ASYNCHRONOUS (1 << 8)
#define UVC_CTRL_FLAG_GET_RANGE \
(UVC_CTRL_FLAG_GET_CUR | UVC_CTRL_FLAG_GET_MIN | \
--
Regards,
Laurent Pinchart
uvc_v4l2.c | 4 +-
drivers/media/usb/uvc/uvc_video.c | 59 +++--
drivers/media/usb/uvc/uvcvideo.h | 15 +++-
include/uapi/linux/uvcvideo.h | 2 +
6 files changed, 322 insertions(+), 35 deletions(-)
--
Regards,
Laurent Pinchart
_flags()
> will only set flags based on their reported capability from the GET_INFO
> call.
>
> Fixes: 859086ae3636 ("media: uvcvideo: Apply flags from device to actual
> properties")
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
And applied to
aches the mapping in
> either the probe failure paths or the driver remove path resulting
> in an unbalanced mapping refcount and a memory leak. Fix this properly.
>
> Reported-by: Pavel Machek
> Signed-off-by: Suman Anna
> Acked-by: Sakari Ailus
Reviewed-by: Laurent Pi
Hi Paul,
On Tuesday, 20 March 2018 18:46:24 EET Paul Menzel wrote:
> On 03/20/18 14:30, Laurent Pinchart wrote:
> > On Tuesday, 20 March 2018 14:20:14 EET Paul Menzel wrote:
> >> On the Dell XPS 13 9370, Linux 4.16-rc6 outputs the messages below.
> >>
> >>
Hi Nicolas,
On Wednesday, 21 March 2018 05:38:59 EET Nicolas Dufresne wrote:
> Le mardi 20 mars 2018 à 20:04 +0200, Laurent Pinchart a écrit :
> > On Tuesday, 20 March 2018 19:45:51 EET Nicolas Dufresne wrote:
> > > Le mardi 20 mars 2018 à 13:20 +0100, Paul Menzel a écrit :
Entity type for entity Processing 9 was
> > not initialized!
> > [2.343424] uvcvideo 1-5:1.2: Entity type for entity Camera 11 was
> > not initialized!
> > [2.343472] input: Integrated_Webcam_HD: Integrate as
> > /devices/pci:00/:00:14.0/usb1/1-5/1-5:1.2/inpu
Hi Kieran,
Thank you for the patch.
On Tuesday, 20 March 2018 17:43:08 EET Kieran Bingham wrote:
> From: Kieran Bingham
>
> Provide the missing 't' from straightforward.
>
> Signed-off-by: Kieran Bingham
Acked-by: Laurent Pinchart
and applied to my tree.
>
formats without bothering to send a patch
for the uvcvideo driver. It would be easy to do so, but it requires knowing
which format is meant by the GUID. Most format GUIDs are of the form
32595559--0010-8000-00aa00389b71 that starts with a 4CC, but that's not
the case here.
Could you send me the output of
lsusb -v -d 0bda:58f4
running as root if possible ?
--
Regards,
Laurent Pinchart
esn't depend on OF. With this patch, it will likely
> cause build failures with randconfigs.
ARCH_RENESAS implies OF, so replacing (ARCH_RENESAS && OF) with ARCH_RENESAS
doesn't change anything. The driver can be compiled with COMPILE_TEST and !OF
both before and after this patch.
> > depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
> > select VIDEOBUF2_DMA_CONTIG
> > select VIDEOBUF2_VMALLOC
--
Regards,
Laurent Pinchart
k_new) {
> + ret = -EMLINK;
> + goto out;
> + }
> +
> + /* New valid CHSEL found, set the new value. */
> + ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
> +out:
> + mutex_unlock(&group->lock);
> +
> + return ret;
> +}
> +
> +static const struct media_device_ops rvin_media_ops = {
> + .link_notify = rvin_group_link_notify,
> +};
> +
> /*
> * Gen3 CSI2 Group Allocator
> */
> @@ -85,6 +237,7 @@ static int rvin_group_init(struct rvin_group *group,
> struct rvin_dev *vin) vin_dbg(vin, "found %u enabled VIN's in DT",
> group->count);
>
> mdev->dev = vin->dev;
> + mdev->ops = &rvin_media_ops;
>
> match = of_match_node(vin->dev->driver->of_match_table,
> vin->dev->of_node);
--
Regards,
Laurent Pinchart
; + * @routes: list of possible routes from the CSI-2 recivers to
> + * all VINs. The list mush be NULL terminated.
> */
> struct rvin_info {
> enum model_id model;
> @@ -94,6 +135,7 @@ struct rvin_info {
>
> unsigned int max_width;
> unsigned int max_height;
> + const struct rvin_group_route *routes;
> };
>
> /**
--
Regards,
Laurent Pinchart
t; strlcpy(vdev->name, KBUILD_MODNAME, sizeof(vdev->name));
> vdev->release = video_device_release_empty;
> - vdev->ioctl_ops = &rvin_ioctl_ops;
> vdev->lock = &vin->lock;
> vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> V4L2_CAP_READWRITE;
>
> + /* Set a default format */
> vin->format.pixelformat = RVIN_DEFAULT_FORMAT;
> - rvin_reset_format(vin);
> + vin->format.width = RVIN_DEFAULT_WIDTH;
> + vin->format.height = RVIN_DEFAULT_HEIGHT;
> + vin->format.field = RVIN_DEFAULT_FIELD;
> + vin->format.colorspace = RVIN_DEFAULT_COLORSPACE;
> +
> + if (vin->info->use_mc) {
> + vdev->fops = &rvin_mc_fops;
> + vdev->ioctl_ops = &rvin_mc_ioctl_ops;
> + } else {
> + vdev->fops = &rvin_fops;
> + vdev->ioctl_ops = &rvin_ioctl_ops;
> + rvin_reset_format(vin);
> + }
> +
> + ret = rvin_format_align(vin, &vin->format);
> + if (ret)
> + return ret;
>
> ret = video_register_device(&vin->vdev, VFL_TYPE_GRABBER, -1);
> if (ret) {
--
Regards,
Laurent Pinchart
all your comments but one.
>
>
>
> On 2017-12-08 12:14:05 +0200, Laurent Pinchart wrote:
> >> @@ -628,7 +628,8 @@ static int rvin_setup(struct rvin_dev *vin)
> >>/* Default to TB */
> >>vnmc = VNMC_IM_FULL;
> >&
gister is manipulated,
> once the operation is complete it's safe to switch the master off and
> the new routing will still be in effect.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 38 ++
static int rvin_setup(struct rvin_dev *vin)
> dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
>
> /* Hsync Signal Polarity Select */
> - if (!(vin->digital->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> + if
vin->crop = s->r = r;
>
> vin_dbg(vin, "Cropped %dx%d@%d:%d of %dx%d\n",
> r.width, r.height, r.left, r.top,
> - vin->source.width, vin->source.height);
> + pix.width, pix.height
case V4L2_FIELD_INTERLACED_BT:
> case V4L2_FIELD_INTERLACED:
> break;
> + case V4L2_FIELD_ALTERNATE:
> + /*
> + * Driver do not (yet) support outputting ALTERNATE to a
While at it, s/do/does/
> + * userspace. It does support outputting INTERLACED so use
> + * the VIN hardware to combine the two fields.
> + */
> + pix->field = V4L2_FIELD_INTERLACED;
> + pix->height *= 2;
> + break;
> default:
> pix->field = RVIN_DEFAULT_FIELD;
> break;
--
Regards,
Laurent Pinchart
dated force colorspace to the
> + * driver default.
> + */
> + pix->colorspace = RVIN_DEFAULT_COLORSPACE;
Should you also set the xfer_func, quantization and ycbcr_enc ?
Apart from that,
Reviewed-by: Laurent Pinchart
> +
> + return rvin_fo
> - walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
> -
> - /* Limit to VIN capabilities */
> - v4l_bound_align_image(&pix->width, 2, vin->info->max_width, walign,
> - &pix->height, 4, vin->info->max_height, 2, 0);
> -
> - pix->bytesperline = rvin_format_bytesperline(pix);
> - pix->sizeimage = rvin_format_sizeimage(pix);
> -
> - vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> - pix->width, pix->height, pix->bytesperline, pix->sizeimage);
> -
> - return 0;
> + return rvin_format_align(vin, pix);
> }
>
> static int rvin_querycap(struct file *file, void *priv,
--
Regards,
Laurent Pinchart
rline and sizeimage from
> user-space as they are set to 0 before the max_t() operation.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
the same state regardless of the
> current state of the device.
>
> Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/dri
> Reviewed-by: Biju Das
> Reviewed-by: Simon Horman
> Acked-by: Rob Herring
> Reviewed-by: Geert Uytterhoeven
> Acked-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 5 -
> 1 file changed
Reviewed-by: Biju Das
> Reviewed-by: Simon Horman
> Acked-by: Rob Herring
> Reviewed-by: Geert Uytterhoeven
> Acked-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 14 +++---
> 1 file changed, 7 inser
t; >
> > Choose your colours wisely :)
> >
> > On 12/09/17 20:19, Laurent Pinchart wrote:
> >> On Tuesday, 12 September 2017 00:16:50 EEST Kieran Bingham wrote:
> >>> On 17/08/17 19:13, Laurent Pinchart wrote:
> >>>> On Monday 14 Aug 2017 16:13:28 Kier
x27;re talking about the "[PATCH 0/2 v6] uvcvideo: asynchronous
controls" series, is that correct ?
--
Regards,
Laurent Pinchart
t; >>ctrls->error_idx = 0;
> >>
> >>return uvc_ctrl_rollback(handle);
> >>
> >> +
> >> +set_index:
> >> + ctrls->error_idx = i;
> >> + return ret;
> >>
> >> }
> >
> > For uvcvideo I find this to hinder readability
>
> I got an other development view.
>
> > without adding much added value.
>
> There can be a small effect for such a function implementation.
>
> > Please drop the uvcvideo change from this patch.
>
> Would it be nice if this source code adjustment could be integrated also?
Just for the record, and to avoid merging this patch by mistake,
Nacked-by: Laurent Pinchart
at least until the requested changes are implemented.
--
Regards,
Laurent Pinchart
Hi Philipp,
On Tuesday, 27 February 2018 11:13:04 EET Philipp Zabel wrote:
> On Fri, 2018-02-23 at 14:47 +0200, Sakari Ailus wrote:
> > On Fri, Feb 23, 2018 at 12:16:17PM +0100, Philipp Zabel wrote:
> >> On Fri, 2018-02-23 at 12:05 +0200, Laurent Pinchart wrote:
> >>&g
Hi Geert,
Thank you for the patch.
On Monday, 26 February 2018 20:09:10 EET Geert Uytterhoeven wrote:
> VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> As ARCH_RENESAS implies OF, the latter can be dropped.
>
> Signed-off-by: Geert Uytterhoeven
Acked-by: Laurent Pincha
Hi Sergei,
On Tuesday, 27 February 2018 10:22:25 EET Sergei Shtylyov wrote:
> On 2/27/2018 12:45 AM, Laurent Pinchart wrote:
> > The entities in the pipeline are all started when the LIF is setup.
> > Remove the outdated comment that state otherwise.
>
> States?
You
, this requires reconfiguring the
other pipeline to free the BRU before processing, which can result in
frame drop on both pipelines.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 160 +++--
drivers/media/platform/vsp1/vsp1_drm.h | 9 ++
2
The vsp1_drm_pipeline enabled field is set but never used. Remove it.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 4
drivers/media/platform/vsp1/vsp1_drm.h | 2 --
2 files changed, 6 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
b
In order to make the vsp1_du_setup_lif() easier to read, and for
symmetry with the DRM pipeline input setup, move the pipeline output
setup code to a separate function.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 107 +++--
1 file
Move the duplicated DRM pipeline configuration code to a function and
call it from vsp1_du_setup_lif() and vsp1_du_atomic_flush().
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 95 +++---
1 file changed, 43 insertions(+), 52 deletions
done from the atomic update handler.
The current implementation is RPF-specific. Make it independent of the
entity type by using the entity's pipe pointer to mark removal from the
pipeline. This will allow using the mechanism to remove BRU instances.
Signed-off-by: Laurent Pinchart
---
dri
registers to stay close to the datasheet.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/Makefile | 2 +-
drivers/media/platform/vsp1/vsp1.h | 6 +-
.../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202 ++---
.../media/platform/v
Various types of objects subclassing vsp1_entity currently store a
pointer to the pipeline. Move the pointer to vsp1_entity to simplify the
code and avoid storing the pipeline in more entity subclasses later.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c| 20
The entities in the pipeline are all started when the LIF is setup.
Remove the outdated comment that state otherwise.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/platform/vsp1
To implement fully dynamic plane assignment to pipelines, we need to
reassign the BRU and BRS to the DRM pipelines in the atomic commit
handler. In preparation for this setup factor out the BRU source pad
code and call it both at LIF setup and atomic commit time.
Signed-off-by: Laurent Pinchart
vsp1_du_pipeline_setup_input() function will not setup any RPF, and will
thus not setup formats on the BRU sink pads. This isn't a problem as all
inputs are disabled, and the BRU sink pads will be reconfigured from the
atomic commit handler when inputs will be enabled.
Signed-off-by: Laurent Pinchart
---
drivers/
new BRx dynamic assignment
test available from
git://git.ideasonboard.com/renesas/kms-tests.git bru-brs
The VSP test suite also runs without any noticed regression.
Laurent Pinchart (15):
v4l: vsp1: Don't start/stop media pipeline for DRM
v4l: vsp1: Remove outdated comment
v4l:
APIs to support such a notification.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_dl.c| 27 +--
drivers/media/platform/vsp1/vsp1_dl.h| 4 ++--
drivers/media/platform/vsp1/vsp1_drm.c | 4 ++--
drivers/media/platform/vsp1/vsp1_pipe.c | 5
Dynamic assignment of the BRU and BRS to pipelines is prone to
regressions, add messages to make debugging easier. Keep it as a
separate commit to ease removal of those messages once the code will
deem to be completely stable.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1
The DRM pipeline setup code used at atomic commit time is similar to the
setup code used when enabling the pipeline. Move it to a separate
function in order to share it.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 347 +
1 file
rrent
implementation is not correct. Remove the incorrect and unneeded code.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
b/drivers/media/platform
inter.
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_drm.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
b/drivers/media/platform/vsp1/vsp1_drm.c
index a7ad85ab0b08..e210917fdc3f 100644
--- a/drivers/media/pla
correct branch.
>
> $ git status
> On branch media_tree/master
> Your branch is up-to-date with 'r_media_tree/master'.
>
> I probably did `./build` instead of `./build --main-git` the first time.
>
> On Mon, Feb 19, 2018 at 2:10 PM, Laurent Pinchart wrote:
up to 3ce28e6d5808d2f805018c7903366d306f483ee8:
v4l: vsp1: Fix video output on R8A77970 (2018-02-23 15:03:17 +0200)
Kieran Bingham (1):
v4l: vsp1: Fix header display list status check in continuous mode
Laurent Pinchart (2
drivers/media/pci/ttpci/av7110_av.c: error: too few arguments to function
> 'feed->cb.ts': => 817:3
>
I think this misses a Fixes: line. Apart from that it looks good to me.
With the Fixes: line,
Acked-by: Laurent Pinchart
> Signed-off-by: Mauro Carvalho Chehab
> ---
Hi Philipp,
On Friday, 23 February 2018 11:56:52 EET Philipp Zabel wrote:
> On Fri, 2018-02-23 at 11:29 +0200, Laurent Pinchart wrote:
> > On Thursday, 22 February 2018 03:39:37 EET Steve Longerbeam wrote:
> >> For some subdevices, a fwnode endpoint that has no connectio
gt;"driver could not parse port@%u/endpoint@%u (%d)\n",
>vep->base.port, vep->base.id, ret);
> v4l2_fwnode_endpoint_free(vep);
> - if (ret < 0)
> + if (ret < 0 || !asd->match.fwnode)
> goto out_err;
>
> notifier->subdevs[notifier->num_subdevs] = asd;
--
Regards,
Laurent Pinchart
s/media/v4l2-core/Kconfig
> @@ -7,6 +7,7 @@ config VIDEO_V4L2
> tristate
> depends on (I2C || I2C=n) && VIDEO_DEV
> select RATIONAL
> + select VIDEOBUF2_V4L2 if VIDEOBUF2_CORE
> default (I2C || I2C=n) && VIDEO_DEV
I thought that if VIDEO_V4L2=y and VIDEOBUF2_CORE=m, this would set
VIDEOBUF2_V4L2=y, but testing the patch proved me wrong.
Tested-by: Laurent Pinchart
Reviewed-by: Laurent Pinchart
>
> config VIDEO_ADV_DEBUG
--
Regards,
Laurent Pinchart
The DRM pipelines can use either the BRU or the BRS for blending. Make
sure the right name is used in debugging messages to avoid confusion.
Signed-off-by: Laurent Pinchart
---
Changes since v1:
- Create a macro to get the right entity name instead of duplicating the
same code all over the
Hi Sergei,
On Thursday, 22 February 2018 20:34:37 EET Sergei Shtylyov wrote:
> On 02/22/2018 07:32 PM, Laurent Pinchart wrote:
> > From: Sergei Shtylyov
> >
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances"
-DL and
VSP2-D instances")
Signed-off-by: Sergei Shtylyov
Reviewed-by: Laurent Pinchart
[Removed braces, added VI6_IP_VERSION_MASK to improve readabiliy]
Signed-off-by: Laurent Pinchart
---
drivers/media/platform/vsp1/vsp1_lif.c | 12
drivers/media/platform/vsp1/vsp1_regs.h
Hi Sergei,
On Thursday, 22 February 2018 18:26:20 EET Laurent Pinchart wrote:
> On Thursday, 18 January 2018 16:05:51 EET Sergei Shtylyov wrote:
> > Commit d455b45f8393 ("v4l: vsp1: Add support for new VSP2-BS, VSP2-DL,
> > and VSP2-D instances") added support for the
;
> + }
There's no need for braces or inner parentheses. To make this easier to read I
propose defining a new macro VI6_IP_VERSION_MASK that combines both the model
and SoC. Otherwise this looks good to me,
Reviewed-by: Laurent Pinchart
LPHA_RATIO_MASK(0xff << 0)
> #define VI6_RPF_MULT_ALPHA_RATIO_SHIFT 0
>
> /* ----
--
Regards,
Laurent Pinchart
misses UVC 1.5 support.
Paul has recently started working on the UVC gadget driver to revive it along
with the userspace helper application. Further down on his to-do list he told
me he would like to implement UVC 1.5 support on the host side, but that won't
be for the near future (no pressure Paul :-)).
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Thursday, 22 February 2018 14:36:00 EET jacopo mondi wrote:
> On Thu, Feb 22, 2018 at 02:14:53PM +0200, Laurent Pinchart wrote:
> > On Thursday, 22 February 2018 14:04:12 EET jacopo mondi wrote:
> >> On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote
Hi Hans,
On Thursday, 22 February 2018 09:38:46 EET Hans Verkuil wrote:
> On 02/21/2018 09:16 PM, Laurent Pinchart wrote:
> > On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote:
> >> On 02/19/2018 11:28 PM, Niklas Söderlund wrote:
> >>> Hi Hans,
> &g
Hi Hans,
On Thursday, 22 February 2018 10:01:13 EET Hans Verkuil wrote:
> On 02/21/2018 10:01 PM, Sakari Ailus wrote:
> > On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote:
> >> No, I'm sorry, for MC-based drivers this isn't correct. The media entity
Hi Jacopo,
On Thursday, 22 February 2018 14:04:12 EET jacopo mondi wrote:
> On Wed, Feb 21, 2018 at 10:28:06PM +0200, Laurent Pinchart wrote:
> > On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
> > > On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
ameInterval( 2)40
> dwFrameInterval( 3)67
> VideoStreaming Interface Descriptor:
> bLength 6
> bDescriptorType36
> bDescriptorSubtype 13 (COLORFORMAT)
> bColorPrimaries 0 (Unspecified)
> bTransferCharacteristics0 (Unspecified)
> bMatrixCoefficients 0 (Unspecified)
> Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x82 EP 2 IN
> bmAttributes2
> Transfer TypeBulk
> Synch Type None
> Usage Type Data
> wMaxPacketSize 0x0200 1x 512 bytes
> bInterval 1
> ** UNRECOGNIZED: 02 23
> ** UNRECOGNIZED: 20 ac 75 10 02 75 11 e8 02 1d 4b 12 20 5e 7d 40 7c
> 01 7f 04 12 28 33 7d f0 7c 00 7f 03 12 28 33 Device Qualifier (for other
> device speed):
> bLength10
> bDescriptorType 6
> bcdUSB 2.00
> bDeviceClass 239 Miscellaneous Device
> bDeviceSubClass 2 ?
> bDeviceProtocol 1 Interface Association
> bMaxPacketSize064
> bNumConfigurations 1
> Device Status: 0x
> (Bus Powered)
Thank you.
--
Regards,
Laurent Pinchart
since commit 3bc85817d798 ("media: uvcvideo: Add extensible device
information"), how about making use of it and adding a field to the
uvc_device_info structure to store the forced format ?
> /* Format flags */
> #define UVC_FMT_FLAG_COMPRESSED 0x0001
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote:
> On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote:
> > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote:
> >> The sensor driver sets mbus format colorspace information and s
value here, but then what do you
> use in v4l2_pix_format? That definitely can't be 0. I think I prefer to
> always have something explicit here.
>
> 2) Validation of the colorspace fields when setting the format on the input
> pad. After thinking about this some more I believe that the only reasonable
> thing you can do here is to indeed validate it based on the current range
> of known colorspaces (or more strict of course if the hardware has
> limitations).
>
> But rather than adding validate defines I would just add something like this
> to the colorspace enum:
>
> /* Update when a new colorspace is added */
> V4L2_COLORSPACE_LAST= V4L2_COLORSPACE_DCI_P3
>
> and do the same for the other colorspace-related enums.
>
> In v4l2-subdev.c you can then check and validate this. Anything out-of-range
> would map to COLORSPACE_SRGB/RAW or 0 (DEFAULT) for the other fields.
>
> I think SRGB is best here since it is the most widely used and understood
> colorspace.
>
> Another issue is pipeline validation. See the link to the RFC patch above.
> The biggest issue here is that filling in these fields has been hit-and-miss
> and you don't want things to suddenly fail.
>
> If the core validates/initializes these fields, then at least we always see
> something valid here (i.e. never 0 or > V4L2_COLORSPACE_LAST for the
> colorspace). I'd have to think about it some more.
>
> This whole mess once again shows the importance of good compliance tests,
> especially for complex APIs like this. It forces you to think about how
> this should be handled instead of doing some vague hand-waving.
--
Regards,
Laurent Pinchart
Hi Hans,
On Wednesday, 21 February 2018 15:11:36 EET Hans Verkuil wrote:
> On 02/21/18 13:37, Laurent Pinchart wrote:
> > On Wednesday, 21 February 2018 09:40:29 EET Hans Verkuil wrote:
> >> On 02/21/2018 01:02 AM, Laurent Pinchart wrote:
> >>> On Tuesday, 20 F
Hi Hans,
On Wednesday, 21 February 2018 09:40:29 EET Hans Verkuil wrote:
> On 02/21/2018 01:02 AM, Laurent Pinchart wrote:
> > On Tuesday, 20 February 2018 11:44:20 EET Hans Verkuil wrote:
> >> The v4l2_subdev core s_power op was used for two different things: power
> >&
(NV[12|21|16|61]).
> >
> > This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> >
> > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> > platform GR-Peach.
> >
> > Tested with ov7725 camera sensor on SH4
l it when a tuner ops should
> *operate on radio mode, before being able to handle it.
> @@ -268,6 +271,7 @@ struct v4l2_subdev_core_ops {
> * }
> */
> struct v4l2_subdev_tuner_ops {
> + int (*standby)(struct v4l2_subdev *sd);
> int (*s_radio)(struct v4l2_subdev *sd);
> int (*s_frequency)(struct v4l2_subdev *sd, const struct v4l2_frequency
> *freq);
> int (*g_frequency)(struct v4l2_subdev *sd, struct v4l2_frequency *freq);
--
Regards,
Laurent Pinchart
Hi Devin,
On Tuesday, 20 February 2018 20:18:16 EET Devin Heitmueller wrote:
> On Mon, Feb 19, 2018 at 11:19 AM, Laurent Pinchart wrote:
> > I've tested VLC (2.2.8) and haven't noticed any issue. If a program is
> > directed to the metadata video node and tries to
h,
I'd prefer the bridge drivers to be fixed to use s_power in a balanced way,
but I understand that it might be difficult to achieve in a timely fashion.
I'm thus not against this patch, but I don't think it makes too much sense to
merge it without a user, that is a patch series that works on removing
s_power.
--
Regards,
Laurent Pinchart
FER_FUNC_DEFAULT;
How about setting the values explicitly instead of relying on defaults ? That
would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and
V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if the
sensor outputs limited or full range ?
> info->format = *fmt;
>
> return 0;
--
Regards,
Laurent Pinchart
] ioctl(VIDIOC_G_INPUT):
> Inappropriate ioctl for device
> /dev/video1: Inappropriate ioctl for device
>
> Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
> it to work. In the case of /dev/video0, I can't tell what could be
> wrong from the error message.
--
Regards,
Laurent Pinchart
handle);
> - ctrls->error_idx = i;
> - return ret;
> + goto set_index;
> }
> }
>
> ctrls->error_idx = 0;
>
> return uvc_ctrl_rollback(handle);
> +
> +set_index:
> + ctrls->error_idx = i;
> + return ret;
> }
For uvcvideo I find this to hinder readability without adding much added
value. Please drop the uvcvideo change from this patch.
>
> static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
--
Regards,
Laurent Pinchart
d VLC (2.2.8) and haven't noticed any issue. If a program is
directed to the metadata video node and tries to capture video from it it will
obviously fail. That being said, software that work today should continue
working, otherwise it's a regression, and we'll have to handle that.
> > Are there any known issues that need looking at ?
> >
> >>> v4l2-ctl --list-devices
> >>
> >> Laptop_Integrated_Webcam_E4HD: (usb-:00:1a.0-1.5):
> >> /dev/video0
> >> /dev/video1
> >>>
> >>> ls /dev/video*
> >>
> >> /dev/video0 /dev/video1
--
Regards,
Laurent Pinchart
701 - 800 of 8133 matches
Mail list logo
