Re: [PATCH v2] media: venus: add support for selection rectangles
Hi Malathi,
On Fri, Nov 9, 2018 at 4:39 PM Malathi Gottam wrote:
>
> Handles target type crop by setting the new active rectangle
> to hardware. The new rectangle should be within YUV size.
>
> Signed-off-by: Malathi Gottam
> ---
> drivers/media/platform/qcom/venus/venc.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c
> b/drivers/media/platform/qcom/venus/venc.c
> index ce85962..d26c129 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh,
> struct v4l2_format *f)
> venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
> {
> struct venus_inst *inst = to_inst(file);
> + int ret;
> + u32 buftype;
>
> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> return -EINVAL;
>
> switch (s->target) {
> case V4L2_SEL_TGT_CROP:
> - if (s->r.width != inst->out_width ||
> - s->r.height != inst->out_height ||
> - s->r.top != 0 || s->r.left != 0)
> - return -EINVAL;
> + if (s->r.left != 0) {
> + s->r.width += s->r.left;
> + s->r.left = 0;
> + }
> +
> + if (s->r.top != 0) {
> + s->r.height += s->r.top;
> + s->r.top = 0;
> + }
> +
> + if (s->r.width > inst->width)
> + s->r.width = inst->width;
> + else
> + inst->width = s->r.width;
> +
> + if (s->r.height > inst->height)
> + s->r.height = inst->height;
> + else
> + inst->height = s->r.height;
> +
>From semantic point of view, it looks fine, but where is the rectangle
actually set to the hardware?
Best regards,
Tomasz
[PATCH] dt-bindings: media: i2c: Fix i2c address for OV5645 camera sensor
The i2c address for the Omnivision OV5645 camera sensor is 0x3c. It is
incorrectly mentioned as 0x78 in binding. Hence fix that.
Fixes: 09c716af36e6 [media] media: i2c/ov5645: add the device tree binding
document
Signed-off-by: Manivannan Sadhasivam
---
Documentation/devicetree/bindings/media/i2c/ov5645.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
index fd7aec9f8e24..1a68ca5eb9a3 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
@@ -26,9 +26,9 @@ Example:
&i2c1 {
...
- ov5645: ov5645@78 {
+ ov5645: ov5645@3c {
compatible = "ovti,ov5645";
- reg = <0x78>;
+ reg = <0x3c>;
enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
--
2.17.1
[PATCH v2] media: venus: add support for selection rectangles
Handles target type crop by setting the new active rectangle
to hardware. The new rectangle should be within YUV size.
Signed-off-by: Malathi Gottam
---
drivers/media/platform/qcom/venus/venc.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/venc.c
b/drivers/media/platform/qcom/venus/venc.c
index ce85962..d26c129 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct
v4l2_format *f)
venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
{
struct venus_inst *inst = to_inst(file);
+ int ret;
+ u32 buftype;
if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
return -EINVAL;
switch (s->target) {
case V4L2_SEL_TGT_CROP:
- if (s->r.width != inst->out_width ||
- s->r.height != inst->out_height ||
- s->r.top != 0 || s->r.left != 0)
- return -EINVAL;
+ if (s->r.left != 0) {
+ s->r.width += s->r.left;
+ s->r.left = 0;
+ }
+
+ if (s->r.top != 0) {
+ s->r.height += s->r.top;
+ s->r.top = 0;
+ }
+
+ if (s->r.width > inst->width)
+ s->r.width = inst->width;
+ else
+ inst->width = s->r.width;
+
+ if (s->r.height > inst->height)
+ s->r.height = inst->height;
+ else
+ inst->height = s->r.height;
+
break;
default:
return -EINVAL;
--
1.9.1
Re: [PATCH] media: venus: add support for selection rectangles
On 2018-11-02 03:16, Tomasz Figa wrote:
On Fri, Nov 2, 2018 at 12:02 AM Stanimir Varbanov
wrote:
Hi Malathi,
On 11/1/18 3:10 PM, mgot...@codeaurora.org wrote:
> On 2018-10-16 15:11, Stanimir Varbanov wrote:
>> Hi Malathi,
>>
>> On 10/09/2018 10:53 AM, Malathi Gottam wrote:
>>> Handles target type crop by setting the new active rectangle
>>> to hardware. The new rectangle should be within YUV size.
>>>
>>> Signed-off-by: Malathi Gottam
>>> ---
>>> drivers/media/platform/qcom/venus/venc.c | 19 +--
>>> 1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/venc.c
>>> b/drivers/media/platform/qcom/venus/venc.c
>>> index 3f50cd0..754c19a 100644
>>> --- a/drivers/media/platform/qcom/venus/venc.c
>>> +++ b/drivers/media/platform/qcom/venus/venc.c
>>> @@ -478,16 +478,31 @@ static int venc_g_fmt(struct file *file, void
>>> *fh, struct v4l2_format *f)
>>> venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
>>> {
>>> struct venus_inst *inst = to_inst(file);
>>> +int ret;
>>> +u32 buftype;
>>>
>>> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>>> return -EINVAL;
>>>
>>> switch (s->target) {
>>> case V4L2_SEL_TGT_CROP:
>>> -if (s->r.width != inst->out_width ||
>>> -s->r.height != inst->out_height ||
>>> +if (s->r.width > inst->out_width ||
>>> +s->r.height > inst->out_height ||
>>> s->r.top != 0 || s->r.left != 0)
>>> return -EINVAL;
Note that returning -EINVAL is not what VIDIOC_S_SELECTION should do
here.
As per the general V4L2 spec [1], VIDIOC_S_SELECTION is expected to
adjust the crop rectangle "as close as possible to the requested one".
In this case that would likely mean something like this:
if (s->r.left != 0) {
s->r.width += s->r.left;
s->r.left = 0;
}
if (s->r.top != 0) {
s->r.height += s->r.top;
s->r.top = 0;
}
if (s->r.width > inst->out_width)
s->r.width = inst->out_width;
if (s->r.height > inst->out_height)
s->r.height = inst->out_height;
[1]
https://www.kernel.org/doc/html/latest/media/uapi/v4l/vidioc-g-selection.html
Yes Tomasz, I overlooked the spec that we cannot return EINVAL in this
case.
I will make the necessary changes and update the new patch.
>>> +if (s->r.width != inst->width ||
>>> +s->r.height != inst->height) {
>>> +buftype = HFI_BUFFER_OUTPUT;
>>> +ret = venus_helper_set_output_resolution(inst,
>>> + s->r.width,
>>> + s->r.height,
>>> + buftype);
>>
>> I'm afraid that set_output_resolution cannot be called at any time. Do
>> you think we can set it after start_session?
>
> Yes Stan, we can set output_resolution after the session has been
> initialization.
> As per the spec, this call s_selection is an optional step under
> Initialization
> procedure of encoder even before we request buffers.
Setting this output resolution here I feel it as a repetition as we
have this width and height set to firmware as a part of
VIDIOC_REQBUFS().
So updating the selection rectangle would be sufficient in this case.
What spec you are referring to? The spec for the encoders [1] or
something else.
For our convenience, Hans was nice enough to host a compiled version
at:
https://hverkuil.home.xs4all.nl/request-api/uapi/v4l/dev-encoder.html
Best regards,
Tomasz
Re: [PATCH 3/3] media: imx: lift CSI width alignment restriction
On 11/5/18 7:20 AM, Philipp Zabel wrote:
The CSI subdevice shouldn't have to care about IDMAC line start
address alignment. With compose rectangle support in the capture
driver, it doesn't have to anymore.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-capture.c | 9 -
drivers/staging/media/imx/imx-media-csi.c | 2 +-
drivers/staging/media/imx/imx-media-utils.c | 15 ---
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index 2d49d9573056..f87d6e8019e5 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -204,10 +204,9 @@ static int capture_g_fmt_vid_cap(struct file *file, void
*fh,
}
static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
-struct v4l2_subev_format *fmt_src,
+struct v4l2_subdev_format *fmt_src,
struct v4l2_format *f)
{
- struct capture_priv *priv = video_drvdata(file);
const struct imx_media_pixfmt *cc, *cc_src;
cc_src = imx_media_find_ipu_format(fmt_src->format.code, CS_SEL_ANY);
@@ -250,7 +249,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void
*fh,
if (ret)
return ret;
- return __capture_try_fmt(priv, &fmt_src, f);
+ return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
}
static int capture_s_fmt_vid_cap(struct file *file, void *fh,
@@ -280,8 +279,8 @@ static int capture_s_fmt_vid_cap(struct file *file, void
*fh,
CS_SEL_ANY, true);
priv->vdev.compose.left = 0;
priv->vdev.compose.top = 0;
- priv->vdev.compose.width = fmt_src.width;
- priv->vdev.compose.height = fmt_src.height;
+ priv->vdev.compose.width = fmt_src.format.width;
+ priv->vdev.compose.height = fmt_src.format.height;
return 0;
}
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index c4523afe7b48..d39682192a67 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -41,7 +41,7 @@
#define MIN_H 144
#define MAX_W 4096
#define MAX_H 4096
-#define W_ALIGN4 /* multiple of 16 pixels */
+#define W_ALIGN1 /* multiple of 2 pixels */
This works for the IDMAC output pad because the channel's cpmem width
and stride can be rounded up, but width align at the CSI sink still
needs to be 8 pixels when directed to the IC via the CSI_SRC_PAD_DIRECT
pad, in order to support the 8x8 block rotator in the IC PRP, and
there's no way AFAIK to do the same trick of rounding up width and
stride for non-IDMAC direct paths through the IPU.
Also, the imx-ic-prpencvf.c W_ALIGN_SRC can be relaxed to 2 pixels as well.
Steve
#define H_ALIGN1 /* multiple of 2 lines */
#define S_ALIGN1 /* multiple of 2 */
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 0eaa353d5cb3..5f110d90a4ef 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -580,6 +580,7 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format
*pix,
struct v4l2_mbus_framefmt *mbus,
const struct imx_media_pixfmt *cc)
{
+ u32 width;
u32 stride;
if (!cc) {
@@ -602,9 +603,16 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format
*pix,
cc = imx_media_find_mbus_format(code, CS_SEL_YUV, false);
}
- stride = cc->planar ? mbus->width : (mbus->width * cc->bpp) >> 3;
+ /* Round up width for minimum burst size */
+ width = round_up(mbus->width, 8);
- pix->width = mbus->width;
+ /* Round up stride for IDMAC line start address alignment */
+ if (cc->planar)
+ stride = round_up(width, 16);
+ else
+ stride = round_up((width * cc->bpp) >> 3, 8);
+
+ pix->width = width;
pix->height = mbus->height;
pix->pixelformat = cc->fourcc;
pix->colorspace = mbus->colorspace;
@@ -613,7 +621,8 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format
*pix,
pix->quantization = mbus->quantization;
pix->field = mbus->field;
pix->bytesperline = stride;
- pix->sizeimage = (pix->width * pix->height * cc->bpp) >> 3;
+ pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
+stride * pix->height;
return 0;
}
Re: [PATCH 2/3] media: imx: set compose rectangle to mbus format
Hi Philipp,
On 11/5/18 7:20 AM, Philipp Zabel wrote:
Prepare for mbus format being smaller than the written rectangle
due to burst size.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-capture.c | 55 +--
1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index cace8a51aca8..2d49d9573056 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -203,21 +203,14 @@ static int capture_g_fmt_vid_cap(struct file *file, void
*fh,
return 0;
}
-static int capture_try_fmt_vid_cap(struct file *file, void *fh,
- struct v4l2_format *f)
+static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
+struct v4l2_subev_format *fmt_src,
typo: struct v4l2_subdev_format *fmt_src,
+struct v4l2_format *f)
{
struct capture_priv *priv = video_drvdata(file);
- struct v4l2_subdev_format fmt_src;
const struct imx_media_pixfmt *cc, *cc_src;
- int ret;
-
- fmt_src.pad = priv->src_sd_pad;
- fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
- ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
- if (ret)
- return ret;
- cc_src = imx_media_find_ipu_format(fmt_src.format.code, CS_SEL_ANY);
+ cc_src = imx_media_find_ipu_format(fmt_src->format.code, CS_SEL_ANY);
if (cc_src) {
u32 fourcc, cs_sel;
@@ -231,7 +224,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
cc = imx_media_find_format(fourcc, cs_sel, false);
}
} else {
- cc_src = imx_media_find_mbus_format(fmt_src.format.code,
+ cc_src = imx_media_find_mbus_format(fmt_src->format.code,
CS_SEL_ANY, true);
if (WARN_ON(!cc_src))
return -EINVAL;
@@ -239,15 +232,32 @@ static int capture_try_fmt_vid_cap(struct file *file,
void *fh,
cc = cc_src;
}
- imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src.format, cc);
+ imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
return 0;
}
+static int capture_try_fmt_vid_cap(struct file *file, void *fh,
+ struct v4l2_format *f)
+{
+ struct capture_priv *priv = video_drvdata(file);
+ struct v4l2_subdev_format fmt_src;
+ int ret;
+
+ fmt_src.pad = priv->src_sd_pad;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
+ if (ret)
+ return ret;
+
+ return __capture_try_fmt(priv, &fmt_src, f);
typo: return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
+}
+
static int capture_s_fmt_vid_cap(struct file *file, void *fh,
struct v4l2_format *f)
{
struct capture_priv *priv = video_drvdata(file);
+ struct v4l2_subdev_format fmt_src;
int ret;
if (vb2_is_busy(&priv->q)) {
@@ -255,7 +265,13 @@ static int capture_s_fmt_vid_cap(struct file *file, void
*fh,
return -EBUSY;
}
- ret = capture_try_fmt_vid_cap(file, priv, f);
+ fmt_src.pad = priv->src_sd_pad;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
+ if (ret)
+ return ret;
+
+ ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
if (ret)
return ret;
@@ -264,8 +280,8 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
CS_SEL_ANY, true);
priv->vdev.compose.left = 0;
priv->vdev.compose.top = 0;
- priv->vdev.compose.width = f->fmt.fmt.pix.width;
- priv->vdev.compose.height = f->fmt.fmt.pix.height;
+ priv->vdev.compose.width = fmt_src.width;
+ priv->vdev.compose.height = fmt_src.height;
return 0;
}
@@ -307,9 +323,14 @@ static int capture_g_selection(struct file *file, void *fh,
case V4L2_SEL_TGT_COMPOSE:
case V4L2_SEL_TGT_COMPOSE_DEFAULT:
case V4L2_SEL_TGT_COMPOSE_BOUNDS:
- case V4L2_SEL_TGT_COMPOSE_PADDED:
s->r = priv->vdev.compose;
break;
+ case V4L2_SEL_TGT_COMPOSE_PADDED:
+ s->r.left = 0;
+ s->r.top = 0;
+ s->r.width = priv->vdev.fmt.fmt.pix.width;
+ s->r.height = priv->vdev.fmt.fmt.pix.height;
+ break;
default:
return -EINVAL;
}
Re: [PATCH 1/3] media: imx: add capture compose rectangle
Hi Philipp,
On 11/5/18 7:20 AM, Philipp Zabel wrote:
Allowing to compose captured images into larger memory buffers
will let us lift alignment restrictions on CSI crop width.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 +-
drivers/staging/media/imx/imx-media-capture.c | 38 +++
drivers/staging/media/imx/imx-media-csi.c | 3 +-
drivers/staging/media/imx/imx-media-vdic.c| 4 +-
drivers/staging/media/imx/imx-media.h | 2 +
5 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..fe5a77baa592 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -366,8 +366,7 @@ static int prp_setup_channel(struct prp_priv *priv,
memset(&image, 0, sizeof(image));
image.pix = vdev->fmt.fmt.pix;
- image.rect.width = image.pix.width;
- image.rect.height = image.pix.height;
+ image.rect = vdev->compose;
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index b37e1186eb2f..cace8a51aca8 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -262,6 +262,10 @@ static int capture_s_fmt_vid_cap(struct file *file, void
*fh,
priv->vdev.fmt.fmt.pix = f->fmt.pix;
priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
CS_SEL_ANY, true);
+ priv->vdev.compose.left = 0;
+ priv->vdev.compose.top = 0;
+ priv->vdev.compose.width = f->fmt.fmt.pix.width;
+ priv->vdev.compose.height = f->fmt.fmt.pix.height;
this should be:
priv->vdev.compose.width = fmt_src.format.width;
priv->vdev.compose.height = fmt_src.format.height;
(corrected in the next patches but needs to be corrected here).
return 0;
}
@@ -290,6 +294,35 @@ static int capture_s_std(struct file *file, void *fh,
v4l2_std_id std)
return v4l2_subdev_call(priv->src_sd, video, s_std, std);
}
+static int capture_g_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ struct capture_priv *priv = video_drvdata(file);
+
+ switch (s->target) {
+ case V4L2_SEL_TGT_CROP:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ case V4L2_SEL_TGT_NATIVE_SIZE:
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ case V4L2_SEL_TGT_COMPOSE_PADDED:
+ s->r = priv->vdev.compose;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int capture_s_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ return capture_g_selection(file, fh, s);
+}
+
static int capture_g_parm(struct file *file, void *fh,
struct v4l2_streamparm *a)
{
@@ -350,6 +383,9 @@ static const struct v4l2_ioctl_ops capture_ioctl_ops = {
.vidioc_g_std = capture_g_std,
.vidioc_s_std = capture_s_std,
+ .vidioc_g_selection = capture_g_selection,
+ .vidioc_s_selection = capture_s_selection,
+
.vidioc_g_parm = capture_g_parm,
.vidioc_s_parm = capture_s_parm,
@@ -687,6 +723,8 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
&fmt_src.format, NULL);
+ vdev->compose.width = fmt_src.format.width;
+ vdev->compose.height = fmt_src.format.height;
vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
CS_SEL_ANY, false);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..c4523afe7b48 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -413,8 +413,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
memset(&image, 0, sizeof(image));
image.pix = vdev->fmt.fmt.pix;
- image.rect.width = image.pix.width;
- image.rect.height = image.pix.height;
+ image.rect = vdev->compose;
csi_idmac_setup_vb2_buf(priv, phys);
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 482250d47e7c..e08d296cf4eb 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -263,10 +263,10 @@ static int setup_vdi_channel(struct vdic_priv *priv,
mem
cron job: media_tree daily build: ERRORS
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: Fri Nov 9 05:00:10 CET 2018 media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708 media_build git hash: 0c8bb27f3aaa682b9548b656f77505c3d1f11e71 v4l-utils git hash: dd3ff81f58c4e1e6f33765dc61ad33c48ae6bb07 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.0-2-amd64 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: WARNINGS linux-git-arm-multi: WARNINGS linux-git-arm-pxa: WARNINGS linux-git-arm-stm32: WARNINGS linux-git-arm64: WARNINGS linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-sh: OK linux-git-x86_64: WARNINGS Check COMPILE_TEST: OK linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.57-i686: ERRORS linux-3.16.57-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.123-i686: ERRORS linux-3.18.123-x86_64: ERRORS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.52-i686: ERRORS linux-4.1.52-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.159-i686: ERRORS linux-4.4.159-x86_64: ERRORS linux-4.5.7-i686: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-i686: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.10-i686: ERRORS linux-4.7.10-x86_64: ERRORS linux-4.8.17-i686: ERRORS linux-4.8.17-x86_64: ERRORS linux-4.9.131-i686: ERRORS linux-4.9.131-x86_64: ERRORS linux-4.10.17-i686: ERRORS linux-4.10.17-x86_64: ERRORS linux-4.11.12-i686: ERRORS linux-4.11.12-x86_64: ERRORS linux-4.12.14-i686: ERRORS linux-4.12.14-x86_64: ERRORS linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19.1-i686: OK linux-4.19.1-x86_64: OK linux-4.20-rc1-i686: OK linux-4.20-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Logs weren't copied as they are too large (1668 kB) The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH 0/2] sony-cxd2880: add optional vcc regulator
Hi Neil, On 11/8/18 4:50 AM, Neil Armstrong wrote: > This patchset adds an optional VCC regulator to the bindings and driver to > make sure power is enabled to the module before starting attaching to > the device. > > Neil Armstrong (2): > media: cxd2880-spi: Add optional vcc regulator > media: sony-cxd2880: add optional vcc regulator to bindings > > .../devicetree/bindings/media/spi/sony-cxd2880.txt | 4 > drivers/media/spi/cxd2880-spi.c | 16 > > 2 files changed, 20 insertions(+) > Please see Documentation/devicetree/bindings/submitting-patches.txt for some helpful information about submitting a series that includes a bindings patch. You will want to add 'dt-bindings:' into the subject line, along with the current 'media:'. And getmaintainer will give you Rob's and Mark's emails. Thanks, Frank
RE: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi, Sakari, Thanks for your review and comments. Bingbu has replied to some of your questions, so I will continue with the rest. > -Original Message- > From: Bing Bu Cao [mailto:bingbu@linux.intel.com] > Sent: Tuesday, November 6, 2018 10:17 PM > To: Sakari Ailus ; Zhi, Yong > > Cc: linux-media@vger.kernel.org; tf...@chromium.org; > mche...@kernel.org; hans.verk...@cisco.com; > laurent.pinch...@ideasonboard.com; Mani, Rajmohan > ; Zheng, Jian Xu ; Hu, > Jerry W ; Toivonen, Tuukka > ; Qiu, Tian Shu ; Cao, > Bingbu > Subject: Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset > > > On 11/01/2018 08:03 PM, Sakari Ailus wrote: > > Hi Yong, > > > > Thanks for the update! > > > > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > >> Hi, > >> > >> This series adds support for the Intel IPU3 (Image Processing Unit) > >> ImgU which is essentially a modern memory-to-memory ISP. It > >> implements raw Bayer to YUV image format conversion as well as a > >> large number of other pixel processing algorithms for improving the image > quality. > >> > >> Meta data formats are defined for image statistics (3A, i.e. > >> automatic white balance, exposure and focus, histogram and local area > >> contrast > >> enhancement) as well as for the pixel processing algorithm parameters. > >> The documentation for these formats is currently not included in the > >> patchset but will be added in a future version of this set. > >> > >> The algorithm parameters need to be considered specific to a given > >> frame and typically a large number of these parameters change on > >> frame to frame basis. Additionally, the parameters are highly > >> structured (and not a flat space of independent configuration > >> primitives). They also reflect the data structures used by the > >> firmware and the hardware. On top of that, the algorithms require > >> highly specialized user space to make meaningful use of them. For > >> these reasons it has been chosen video buffers to pass the parameters to > the device. > >> > >> On individual patches: > >> > >> The heart of ImgU is the CSS, or Camera Subsystem, which contains the > >> image processors and HW accelerators. > >> > >> The 3A statistics and other firmware parameter computation related > >> functions are implemented in patch 11. > >> > >> All IPU3 pipeline default settings can be found in patch 10. > >> > >> To access DDR via ImgU's own memory space, IPU3 is also equipped with > >> its own MMU unit, the driver is implemented in patch 6. > >> > >> Patch 7 uses above driver for DMA mapping operation. > >> > >> The communication between IPU3 firmware and driver is implemented > >> with circular queues in patch 8. > >> > >> Patch 9 provide some utility functions and manage IPU3 fw download > >> and install. > >> > >> The firmware which is called ipu3-fw.bin can be downloaded from: > >> > >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware > >> .git (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) > >> > >> Firmware ABI is defined in patches 4 and 5. > >> > >> Patches 12 and 13 are of the same file, the former contains all h/w > >> programming related code, the latter implements interface functions > >> for access fw & hw capabilities. > >> > >> Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT > work: > >> > >> https://patchwork.kernel.org/patch/9976295/> > > I've pushed the latest set here: > > > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > > > > You can just say the entire set depends on those going forward; the > > documentation is needed, too. > > Ack. > >> Patch 15 represents the top level that glues all of the other > >> components together, passing arguments between the components. > >> > >> Patch 16 is a recent effort to extend v6 for advanced camera features > >> like Continuous View Finder (CVF) and Snapshot During Video(SDV) > support. > >> > >> Link to user space implementation: > >> > >> git clone > >> https://chromium.googlesource.com/chromiumos/platform/arc-camera > >> > >> ImgU media topology print: > >> > >> # media-ctl -d /dev/media0 -p > >> Media controller API version 4.19.0 > >> > >> Media device information > >> > >> driver ipu3-imgu > >> model ipu3-imgu > >> serial > >> bus infoPCI::00:05.0 > >> hw revision 0x80862015 > >> driver version 4.19.0 > >> > >> Device topology > >> - entity 1: ipu3-imgu 0 (5 pads, 5 links) > >> type V4L2 subdev subtype Unknown flags 0 > >> device node name /dev/v4l-subdev0 > >>pad0: Sink > >>[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > > This doesn't seem right. Which formats can be enumerated from the pad? Supposed to be raw formats for the colorspace field. > > > >> crop:(0,0)/1920x1080 > >> compose:(0,0)/1920x1080] > > Does the compose rectangle affect the scaling on all outputs? > Sakari, driver use crop and compose targets to
Re: [PATCH v4l-utils] Add missing linux/bpf_common.h
Hello Sean, On Wed, 7 Nov 2018 12:05:45 +, Sean Young wrote: > Hi Peter, > > On Tue, Nov 06, 2018 at 10:43:58PM +0100, Peter Seiderer wrote: > > On Tue, 6 Nov 2018 10:38:56 +, Sean Young wrote: > > > > > On Mon, Nov 05, 2018 at 09:30:47PM +0100, Peter Seiderer wrote: > > > > Copy from [1], needed by bpf.h. > > > > > > > > [1] > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/include/uapi/linux/bpf_common.h?h=v4.19 > > > > > > > > > > So bpf.h does include this file, but we don't use anything from it in > > > v4l-utils. > > > > > > > Maybe alternative fix is to remove the include (or not if your want > > the headers to be in sync with the kernel ones, but then they should > > be complete enough to be used for compile)? > > > > > This include file is for the original BPF, which has been around for a > > > long time. So why is this include file missing, i.e. what problem are you > > > trying to solve? > > > > A buildroot autobuild failure (see [1] for details) with older toolchains > > not providing this header... > > > > > > > > Lastely, the file should be included in the sync-with-kernel target so > > > it does not get out of sync -- should it really be necessary to add the > > > file. > > > > O.k, can do it on next patch iteration... > > > > Regards, > > Peter > > > > [1] http://lists.busybox.net/pipermail/buildroot/2018-November/234840.html > > So here libelf was not detected, hence ir-keytable should have been built > without BPF support, but it is still including bpf.h despite it not > being used. > > I've just sent a patch for better support for building without BPF, > see here: > https://patchwork.linuxtv.org/patch/52841/ > > > Would you mind seeing if that works for you? Thanks, works for the buildroot use case (disabling bpf support unconditionally)... The reason to provide copies of the linux kernel headers in v4l-utils is to be independent of old(-er) headers provided by toolchains? If so a copy of bpf_common.h is still needed (and the fallback, for out of linux kernel usage, define for __NR_bpf in bpf.h enhanced for all supported archs)? Regards, Peter > > > Thanks, > > Sean
Re: [PATCH v2 v4l-utils] configure: build without BPF support in ir-keytable
Hello Sean,
On Wed, 7 Nov 2018 12:02:05 +, Sean Young wrote:
> It currently does not build on mips and some platforms do not have
> BPF support yet (risc-v, for example).
>
Thanks for the patch, works for the buildroot use case (disabling
bpf support unconditionally), see [1]...
Regards,
Peter
[1] http://lists.busybox.net/pipermail/buildroot/2018-November/235268.html
> Signed-off-by: Sean Young
> ---
> configure.ac | 17 +
> utils/keytable/Makefile.am | 7 ---
> utils/keytable/keytable.c | 5 -
> 3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 387f8539..5cc34c24 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -173,16 +173,12 @@ AM_CONDITIONAL([HAVE_X11], [test x$x11_pkgconfig =
> xyes])
> PKG_CHECK_MODULES([LIBELF], [libelf], [libelf_pkgconfig=yes],
> [libelf_pkgconfig=no])
> AC_SUBST([LIBELF_CFLAGS])
> AC_SUBST([LIBELF_LIBS])
> -AM_CONDITIONAL([HAVE_LIBELF], [test x$libelf_pkgconfig = xyes])
> if test "x$libelf_pkgconfig" = "xyes"; then
>AC_CHECK_PROG([CLANG], clang, clang)
> - AC_DEFINE([HAVE_LIBELF], [1], [libelf library is present])
> else
>AC_MSG_WARN(libelf library not available)
> fi
>
> -AM_CONDITIONAL([BPF_PROTOCOLS], [test x$CLANG = xclang])
> -
> AS_IF([test "x$x11_pkgconfig" = xyes],
>[PKG_CHECK_MODULES(GL, [gl], [gl_pkgconfig=yes], [gl_pkgconfig=no])],
> [gl_pkgconfig=no])
> AC_SUBST([GL_CFLAGS])
> @@ -453,6 +449,14 @@ AC_ARG_ENABLE(gconv,
> esac]
> )
>
> +AC_ARG_ENABLE(bpf,
> + AS_HELP_STRING([--disable-bpf], [disable IR BPF decoders]),
> + [case "${enableval}" in
> +yes | no ) ;;
> +*) AC_MSG_ERROR(bad value ${enableval} for --enable-bpf) ;;
> + esac]
> +)
> +
> PKG_CHECK_MODULES([SDL2], [sdl2 SDL2_image], [sdl_pc=yes], [sdl_pc=no])
> AM_CONDITIONAL([HAVE_SDL], [test x$sdl_pc = xyes])
>
> @@ -475,6 +479,7 @@ AM_CONDITIONAL([WITH_GCONV],[test x$enable_gconv
> = xyes -a x$enable_shar
> AM_CONDITIONAL([WITH_V4L2_CTL_LIBV4L], [test x${enable_v4l2_ctl_libv4l} !=
> xno])
> AM_CONDITIONAL([WITH_V4L2_CTL_STREAM_TO], [test
> x${enable_v4l2_ctl_stream_to} != xno])
> AM_CONDITIONAL([WITH_V4L2_COMPLIANCE_LIBV4L], [test
> x${enable_v4l2_compliance_libv4l} != xno])
> +AM_CONDITIONAL([WITH_BPF], [test x$enable_bpf != xno -a
> x$libelf_pkgconfig = xyes -a x$CLANG = xclang])
>
> # append -static to libtool compile and link command to enforce static libs
> AS_IF([test x$enable_libdvbv5 = xno], [AC_SUBST([ENFORCE_LIBDVBV5_STATIC],
> ["-static"])])
> @@ -505,6 +510,9 @@ AM_COND_IF([WITH_V4L_WRAPPERS], [USE_V4L_WRAPPERS="yes"],
> [USE_V4L_WRAPPERS="no"
> AM_COND_IF([WITH_GCONV], [USE_GCONV="yes"], [USE_GCONV="no"])
> AM_COND_IF([WITH_V4L2_CTL_LIBV4L], [USE_V4L2_CTL_LIBV4L="yes"],
> [USE_V4L2_CTL_LIBV4L="no"])
> AM_COND_IF([WITH_V4L2_COMPLIANCE_LIBV4L],
> [USE_V4L2_COMPLIANCE_LIBV4L="yes"], [USE_V4L2_COMPLIANCE_LIBV4L="no"])
> +AM_COND_IF([WITH_BPF], [USE_BPF="yes"
> +AC_DEFINE([HAVE_BPF], [1], [BPF IR decoder
> support enabled])],
> + [USE_BPF="no"])
> AS_IF([test "x$alsa_pkgconfig" = "xtrue"], [USE_ALSA="yes"], [USE_ALSA="no"])
> bpf.h
> AC_OUTPUT
> @@ -549,4 +557,5 @@ compile time options summary
> qvidcap: $USE_QVIDCAP
> v4l2-ctl uses libv4l : $USE_V4L2_CTL_LIBV4L
> v4l2-compliance uses libv4l: $USE_V4L2_COMPLIANCE_LIBV4L
> +BPF IR Decoders: : $USE_BPF
> EOF
> diff --git a/utils/keytable/Makefile.am b/utils/keytable/Makefile.am
> index 90e4c8c8..ddbab0f7 100644
> --- a/utils/keytable/Makefile.am
> +++ b/utils/keytable/Makefile.am
> @@ -6,14 +6,15 @@ udevrules_DATA = 70-infrared.rules
>
> ir_keytable_SOURCES = keytable.c parse.h ir-encode.c ir-encode.h toml.c
> toml.h
>
> -if HAVE_LIBELF
> +if WITH_BPF
> ir_keytable_SOURCES += bpf.c bpf_load.c bpf.h bpf_load.h
> endif
>
> ir_keytable_LDADD = @LIBINTL@
> -ir_keytable_LDFLAGS = $(ARGP_LIBS) $(LIBELF_LIBS)
> +ir_keytable_LDFLAGS = $(ARGP_LIBS)
>
> -if BPF_PROTOCOLS
> +if WITH_BPF
> +ir_keytable_LDFLAGS += $(LIBELF_LIBS)
> SUBDIRS = bpf_protocols
> endif
>
> diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c
> index a7ed436b..6fc22358 100644
> --- a/utils/keytable/keytable.c
> +++ b/utils/keytable/keytable.c
> @@ -34,8 +34,11 @@
> #include "ir-encode.h"
> #include "parse.h"
> #include "toml.h"
> +
> +#ifdef HAVE_BPF
> #include "bpf.h"
> #include "bpf_load.h"
> +#endif
>
> #ifdef ENABLE_NLS
> # define _(string) gettext(string)
> @@ -1847,7 +1850,7 @@ static void device_info(int fd, char *prepend)
> perror ("EVIOCGID");
> }
>
> -#ifdef HAVE_LIBELF
> +#ifdef HAVE_BPF
> #define MAX_PROGS 64
> static void attach_bpf(const char *lirc_name, const char *bpf_prog, struct
> toml_table_t *toml)
> {
Re: [Outreachy kernel] [PATCH vicodec v4 0/3] Add support to more pixel formats in vicodec
On Thu, 8 Nov 2018 at 14:54, Sasha Levin wrote: > > On Thu, Nov 08, 2018 at 10:10:10AM +0200, Dafna Hirschfeld wrote: > >On Thu, Nov 8, 2018 at 2:51 AM Ezequiel Garcia < > >ezequ...@vanguardiasur.com.ar> wrote: > > > >> Hello Dafna, > >> > >> Thanks for the patches. > >> > >> Just out of curiosity. Why these patches havent't been submitted to > >> the media mailing list? > >> > >> Hi, > >I wasn't sure if I should send it to the media mailing list, since this > >part of outreachy application. > > In general, for any patch you send to any subsystem please Cc all the > relevant mailing lists and maintainers. For Outreachy application you > already did that (by Cc'ing Greg), you just need to keep doing the same > as you continue your work on other parts of the kernel. > Let's Cc the mailing list now, as these patches look good, and the test scripts look pretty decent too ;-) > >Also, how are you testing these changes? > >> > > > >Based on Helen's decoder: > >https://gitlab.collabora.com/koike/v4l2-codec > > > >I extended it to include encoders and decoders for the new supported > >formats. > > > >testing formats with alpha plane: > >https://github.com/kamomil/outreachy/blob/master/argb-and-abgr-full-example.sh > > > >testing greyscale: > >https://github.com/kamomil/outreachy/blob/master/greyscale-full-example.sh > > It's awesome seeing these testsuites, it gives reviewers confidence that > your patch is well tested and they can focus on other parts of the > review process rather than check for the basic correctness of the patch. > > Please include links such as these and indicate how you tested your code > in your future patches. > +1 Thanks! -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar
Re: [PATCH v4.9 1/1] v4l: event: Add subscription to list before calling "add" operation
On Thu, Nov 08, 2018 at 01:46:06PM +0200, Sakari Ailus wrote:
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions while
accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Cc: sta...@vger.kernel.org (for 4.14 and up)
Signed-off-by: Mauro Carvalho Chehab
Hi Sakari,
For the sake of completeness, can you sign off on the backport too and
indicate it was backported to 4.9 in the commit messge? Otherwise, this
commit message says it's for 4.14+ and will suddenly appear in the 4.9
tree, and if we have issues later it might cause confusion.
--
Thanks,
Sasha
Re: [PATCH v5 0/9] Asynchronous UVC
Hi Kieran,
On Wednesday, 7 November 2018 01:31:29 EET Laurent Pinchart wrote:
> On Tuesday, 6 November 2018 23:27:11 EET Kieran Bingham wrote:
> > From: Kieran Bingham
> >
> > The Linux UVC driver has long provided adequate performance capabilities
> > for web-cams and low data rate video devices in Linux while resolutions
> > were low.
> >
> > Modern USB cameras are now capable of high data rates thanks to USB3 with
> > 1080p, and even 4k capture resolutions supported.
> >
> > Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO
> > (isochronous transfers) can generate more data than an embedded ARM core
> > is able to process on a single core, resulting in frame loss.
> >
> > A large part of this performance impact is from the requirement to
> > ‘memcpy’ frames out from URB packets to destination frames. This
> > unfortunate requirement is due to the UVC protocol allowing a variable
> > length header, and thus it is not possible to provide the target frame
> > buffers directly.
> >
> > Extra throughput is possible by moving the actual memcpy actions to a work
> > queue, and moving the memcpy out of interrupt context thus allowing work
> > tasks to be scheduled across multiple cores.
> >
> > This series has been tested on both the ZED and BRIO cameras on arm64
> > platforms, and with thanks to Randy Dunlap, a Dynex 1.3MP Webcam, a Sonix
> > USB2 Camera, and a built in Toshiba Laptop camera, and with thanks to
> > Philipp Zabel for testing on a Lite-On internal Laptop Webcam, Logitech
> > C910 (USB2 isoc), Oculus Sensor (USB3 isoc), and Microsoft HoloLens
> > Sensors (USB3 bulk).
> >
> > As far as I am aware iSight devices, and devices which use UVC to encode
> > data (output device) have not yet been tested - but should find no ill
> > effect (at least not until they are tested of course :D )
>
> :-D
>
> I'm not sure whether anyone is still using those devices with Linux. I
> wouldn't be surprised if we realized down the road that they already don't
> work.
>
> > Tested-by: Randy Dunlap
> > Tested-by: Philipp Zabel
> >
> > v2:
> > - Fix race reported by Guennadi
> >
> > v3:
> > - Fix similar race reported by Laurent
> > - Only queue work if required (encode/isight do not queue work)
> > - Refactor/Rename variables for clarity
> >
> > v4:
> > - (Yet another) Rework of the uninitialise path.
> >
> >This time to hopefully clean up the shutdown races for good.
> >use usb_poison_urb() to halt all URBs, then flush the work queue
> >before freeing.
> >
> > - Rebase to latest linux-media/master
> >
> > v5:
> > - Provide lockdep validation
> > - rename uvc_queue_requeue -> uvc_queue_buffer_requeue()
> > - Fix comments and periods throughout
> > - Rebase to media/v4.20-2
> > - Use GFP_KERNEL allocation in uvc_video_copy_data_work()
> > - Fix function documentation for uvc_video_copy_data_work()
> > - Add periods to the end of sentences
> > - Rename 'decode' variable to 'op' in uvc_video_decode_data()
> > - Move uvc_urb->async_operations initialisation to before use
> > - Move async workqueue to match uvc_streaming lifetime instead of
> >
> >streamon/streamoff
> >
> > - bracket the for_each_uvc_urb() macro
> >
> > - New patches added to series:
> > media: uvcvideo: Split uvc_video_enable into two
> > media: uvcvideo: Rename uvc_{un,}init_video()
> > media: uvcvideo: Utilise for_each_uvc_urb iterator
> >
> > Kieran Bingham (9):
> > media: uvcvideo: Refactor URB descriptors
> > media: uvcvideo: Convert decode functions to use new context structure
> > media: uvcvideo: Protect queue internals with helper
> > media: uvcvideo: queue: Simplify spin-lock usage
> > media: uvcvideo: queue: Support asynchronous buffer handling
> > media: uvcvideo: Move decode processing to process context
> > media: uvcvideo: Split uvc_video_enable into two
>
> I've taken the above patches in my tree.
I'm afraid I'll have to drop them :-( If I disconnect the camera while in use,
I get the following oops:
[ 237.514625] usb 2-1.4: USB disconnect, device number 5
[ 237.516123] uvcvideo: Failed to resubmit video URB (-19).
[ 237.549470] BUG: unable to handle kernel paging request at 4bec0091
[ 237.549476] PGD 0 P4D 0
[ 237.549481] Oops: [#1] PREEMPT SMP PTI
[ 237.549485] CPU: 3 PID: 5332 Comm: luvcview Tainted: GW O
4.18.16-gentoo #1
[ 237.549487] Hardware name: Dell Inc. Latitude E6420/0K0DNP, BIOS A08
10/18/2011
[ 237.549493] RIP: 0010:flush_workqueue_prep_pwqs+0x49/0x120
[ 237.549494] Code: 47 48 85 c0 0f 85 e1 00 00 00 c7 45 48 01 00 00 00 48 8b
45 00 4c 8d 78 90 48 39 c5 0f 84 d0 00 00 00 c6 44 24 07 00 4d 63 f4 <4d> 8b 2f
4c 89 ef e8 5c b3 89 00 45 85 e4 78 1d 41 83 7f 14 ff 75
[ 237.549525] RSP: 0018:c9000154fc50 EFLAGS: 00010296
[ 237.549527] RAX: 4bec0101 RBX: 0002 RCX: 0002
[ 237.549529] RDX: 0002 RSI: 0001 RDI: 88030241
[PATCH] v4l2-tpg: array index could become negative
text[s] is a signed char, so using that as index into the font8x16 array
can result in negative indices. Cast it to u8 to be safe.
Signed-off-by: Hans Verkuil
Reported-by: syzbot+ccf0a61ed12f2a731...@syzkaller.appspotmail.com
---
diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
index fa483b95bc5a..d9a590ae7545 100644
--- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
+++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
@@ -1769,7 +1769,7 @@ typedef struct { u16 __; u8 _; } __packed x24;
unsigned s; \
\
for (s = 0; s < len; s++) { \
- u8 chr = font8x16[text[s] * 16 + line]; \
+ u8 chr = font8x16[(u8)text[s] * 16 + line]; \
\
if (hdiv == 2 && tpg->hflip) { \
pos[3] = (chr & (0x01 << 6) ? fg : bg); \
Re: [PATCH v4 2/2] media: platform: Add Aspeed Video Engine driver
On 10/19/2018 03:26 PM, Eddie James wrote:
On 10/12/2018 07:22 AM, Hans Verkuil wrote:
On 10/05/2018 09:57 PM, Eddie James wrote:
The Video Engine (VE) embedded in the Aspeed AST2400 and AST2500 SOCs
can capture and compress video data from digital or analog sources.
With
the Aspeed chip acting a service processor, the Video Engine can
capture
the host processor graphics output.
Add a V4L2 driver to capture video data and compress it to JPEG images.
Make the video frames available through the V4L2 streaming interface.
Signed-off-by: Eddie James
---
MAINTAINERS | 8 +
drivers/media/platform/Kconfig | 8 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/aspeed-video.c | 1674
+
4 files changed, 1691 insertions(+)
create mode 100644 drivers/media/platform/aspeed-video.c
+static int aspeed_video_enum_input(struct file *file, void *fh,
+ struct v4l2_input *inp)
+{
+ if (inp->index)
+ return -EINVAL;
+
+ strscpy(inp->name, "Host VGA capture", sizeof(inp->name));
+ inp->type = V4L2_INPUT_TYPE_CAMERA;
+ inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
+ inp->status = V4L2_IN_ST_NO_SIGNAL | V4L2_IN_ST_NO_SYNC;
This can't be right. If there is a valid signal, then status should
be 0.
And ideally you can tell the difference between no signal and no sync
as well.
+
+ return 0;
+}
+
+static int aspeed_video_get_input(struct file *file, void *fh,
unsigned int *i)
+{
+ *i = 0;
+
+ return 0;
+}
+
+static int aspeed_video_set_input(struct file *file, void *fh,
unsigned int i)
+{
+ if (i)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int aspeed_video_get_parm(struct file *file, void *fh,
+ struct v4l2_streamparm *a)
+{
+ struct aspeed_video *video = video_drvdata(file);
+
+ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+ a->parm.capture.readbuffers = 3;
+ a->parm.capture.timeperframe.numerator = 1;
+ if (!video->frame_rate)
+ a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
+ else
+ a->parm.capture.timeperframe.denominator = video->frame_rate;
+
+ return 0;
+}
+
+static int aspeed_video_set_parm(struct file *file, void *fh,
+ struct v4l2_streamparm *a)
+{
+ unsigned int frame_rate = 0;
+ struct aspeed_video *video = video_drvdata(file);
+
+ a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+ a->parm.capture.readbuffers = 3;
+
+ if (a->parm.capture.timeperframe.numerator)
+ frame_rate = a->parm.capture.timeperframe.denominator /
+ a->parm.capture.timeperframe.numerator;
+
+ if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
+ frame_rate = 0;
+
+ /*
+ * Set to max + 1 to differentiate between max and 0, which
+ * means "don't care".
But what does "don't care" mean in practice? It's still not clear to
me how this
is supposed to work.
+ */
+ a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE + 1;
And regardless of anything else this timeperframe is out of the range
that
aspeed_video_enum_frameintervals() returns.
+ a->parm.capture.timeperframe.numerator = 1;
+ }
+
+ if (video->frame_rate != frame_rate) {
+ video->frame_rate = frame_rate;
+ aspeed_video_update(video, VE_CTRL, VE_CTRL_FRC,
+ FIELD_PREP(VE_CTRL_FRC, frame_rate));
+ }
+
+ return 0;
+}
+
+static int aspeed_video_enum_framesizes(struct file *file, void *fh,
+ struct v4l2_frmsizeenum *fsize)
+{
+ struct aspeed_video *video = video_drvdata(file);
+
+ if (fsize->index)
+ return -EINVAL;
+
+ if (fsize->pixel_format != V4L2_PIX_FMT_JPEG)
+ return -EINVAL;
+
+ fsize->discrete.width = video->pix_fmt.width;
+ fsize->discrete.height = video->pix_fmt.height;
+ fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+
+ return 0;
+}
+
+static int aspeed_video_enum_frameintervals(struct file *file, void
*fh,
+ struct v4l2_frmivalenum *fival)
+{
+ struct aspeed_video *video = video_drvdata(file);
+
+ if (fival->index)
+ return -EINVAL;
+
+ if (fival->width != video->width || fival->height !=
video->height)
+ return -EINVAL;
+
+ if (fival->pixel_format != V4L2_PIX_FMT_JPEG)
+ return -EINVAL;
+
+ fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+
+ fival->stepwise.min.denominator = MAX_FRAME_RATE;
+ fival->stepwise.min.numerator = 1;
+ fival->stepwise.max.denominator = 1;
+ fival->stepwise.max.numerator = 1;
+ fival->stepwise.step = fival->stepwise.max;
+
+ return 0;
+}
+
+static int aspeed_video_set_dv_timings(struct file *file, void *fh,
+ struct v4l2_dv_timings *timings)
+{
+ struct aspeed_video *video = video_drvdata(file);
+
If vb2_is_busy() returns true, then return -EBUSY here. It is not
allowed to
set the timings while vb2 i
Re: [PATCH v7 08/16] intel-ipu3: css: Add dma buff pool utility functions
Hi Yong,
On Mon, Oct 29, 2018 at 03:23:02PM -0700, Yong Zhi wrote:
> The pools are used to store previous parameters set by
> user with the parameter queue. Due to pipelining,
> there needs to be multiple sets (up to four)
> of parameters which are queued in a host-to-sp queue.
>
> Signed-off-by: Yong Zhi
> ---
> drivers/media/pci/intel/ipu3/ipu3-css-pool.c | 136
> +++
> drivers/media/pci/intel/ipu3/ipu3-css-pool.h | 56 +++
> 2 files changed, 192 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.c
> create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.h
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.c
> b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c
> new file mode 100644
> index 000..eab41c3
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018 Intel Corporation
> +
> +#include
> +
> +#include "ipu3.h"
> +#include "ipu3-css-pool.h"
> +#include "ipu3-dmamap.h"
> +
> +int ipu3_css_dma_buffer_resize(struct imgu_device *imgu,
> +struct ipu3_css_map *map, size_t size)
> +{
> + if (map->size < size && map->vaddr) {
> + dev_warn(&imgu->pci_dev->dev, "dma buf resized from %zu to %zu",
> + map->size, size);
> +
> + ipu3_dmamap_free(imgu, map);
> + if (!ipu3_dmamap_alloc(imgu, map, size))
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +void ipu3_css_pool_cleanup(struct imgu_device *imgu, struct ipu3_css_pool
> *pool)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < IPU3_CSS_POOL_SIZE; i++)
> + ipu3_dmamap_free(imgu, &pool->entry[i].param);
> +}
> +
> +int ipu3_css_pool_init(struct imgu_device *imgu, struct ipu3_css_pool *pool,
> +size_t size)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < IPU3_CSS_POOL_SIZE; i++) {
> + /*
> + * entry[i].framenum is initialized to INT_MIN so that
> + * ipu3_css_pool_check() can treat it as usesable slot.
> + */
> + pool->entry[i].framenum = INT_MIN;
> +
> + if (size == 0) {
> + pool->entry[i].param.vaddr = NULL;
> + continue;
> + }
> +
> + if (!ipu3_dmamap_alloc(imgu, &pool->entry[i].param, size))
> + goto fail;
> + }
> +
> + pool->last = IPU3_CSS_POOL_SIZE;
> +
> + return 0;
> +
> +fail:
> + ipu3_css_pool_cleanup(imgu, pool);
> + return -ENOMEM;
> +}
> +
> +/*
> + * Check that the following call to pool_get succeeds.
> + * Return negative on error.
> + */
> +static int ipu3_css_pool_check(struct ipu3_css_pool *pool, long framenum)
> +{
> + /* Get the oldest entry */
> + int n = (pool->last + 1) % IPU3_CSS_POOL_SIZE;
> + long diff = framenum - pool->entry[n].framenum;
> +
> + /* if framenum wraps around and becomes smaller than entry n */
> + if (diff < 0)
> + diff += LONG_MAX;
Have you tested the wrap-around? As a result, the value of the diff is
between -1 and LONG_MAX - 1 (without considering more than just the two
lines above). Is that intended?
You seem to be using different types for the frame number; sometimes int,
sometimes long. Could you align that, preferrably to an unsigned type? u32
would probably be a sound choice.
The entry (index to pool->entry array) should be unsigned as well.
> +
> + /*
> + * pool->entry[n].framenum stores the frame number where that
> + * entry was allocated. If that was allocated more than POOL_SIZE
> + * frames back, it is old enough that we know it is no more in
> + * use by firmware.
> + */
> + if (diff > IPU3_CSS_POOL_SIZE)
> + return n;
> +
> + return -ENOSPC;
> +}
> +
> +/*
> + * Allocate a new parameter from pool at frame number `framenum'.
> + * Release the oldest entry in the pool to make space for the new entry.
> + * Return negative on error.
> + */
> +int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum)
> +{
> + int n = ipu3_css_pool_check(pool, framenum);
> +
> + if (n < 0)
> + return n;
> +
> + pool->entry[n].framenum = framenum;
> + pool->last = n;
> +
> + return n;
> +}
> +
> +/*
> + * Undo, for all practical purposes, the effect of pool_get().
> + */
> +void ipu3_css_pool_put(struct ipu3_css_pool *pool)
> +{
> + pool->entry[pool->last].framenum = INT_MIN;
> + pool->last = (pool->last + IPU3_CSS_POOL_SIZE - 1) % IPU3_CSS_POOL_SIZE;
> +}
> +
> +/**
> + * ipu3_css_pool_last - Retrieve the nth pool entry from last
> + *
> + * @pool: a pointer to &struct ipu3_css_pool.
> + * @n: the distance to the last index.
> + *
> + * Return: The nth entry from last or null map to indicate no frame stored.
> + */
> +const struct ipu3_css_map *
> +ipu3_css_pool_last(stru
KASAN: global-out-of-bounds Read in tpg_print_str_4
Hello, syzbot found the following crash on: HEAD commit:85758777c2a2 Merge tag 'hwmon-for-v4.20-rc2' of git://git... git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=157af45d40 kernel config: https://syzkaller.appspot.com/x/.config?x=8f559fee2fc3375a dashboard link: https://syzkaller.appspot.com/bug?extid=ccf0a61ed12f2a7313ee compiler: gcc (GCC) 8.0.1 20180413 (experimental) userspace arch: i386 Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+ccf0a61ed12f2a731...@syzkaller.appspotmail.com == BUG: KASAN: global-out-of-bounds in tpg_print_str_4+0xbc9/0xd70 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1820 Read of size 1 at addr 88631c50 by task vivid-000-vid-c/7282 CPU: 0 PID: 7282 Comm: vivid-000-vid-c Not tainted 4.20.0-rc1+ #228 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x244/0x39d lib/dump_stack.c:113 print_address_description.cold.7+0x58/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412 __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430 tpg_print_str_4+0xbc9/0xd70 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1820 tpg_gen_text+0x4ba/0x540 drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1874 vivid_fillbuff+0x3ff7/0x68e0 drivers/media/platform/vivid/vivid-kthread-cap.c:532 vivid_thread_vid_cap_tick drivers/media/platform/vivid/vivid-kthread-cap.c:709 [inline] vivid_thread_vid_cap+0xbc1/0x2650 drivers/media/platform/vivid/vivid-kthread-cap.c:813 kthread+0x35a/0x440 kernel/kthread.c:246 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352 The buggy address belongs to the variable: font_vga_8x16+0x50/0x60 Memory state around the buggy address: 88631b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88631b80: 00 00 00 00 fa fa fa fa 00 fa fa fa fa fa fa fa 88631c00: 00 00 00 00 00 fa fa fa fa fa fa fa 00 00 00 00 ^ 88631c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88631d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 == --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
Re: [PATCH 4/5] media: rcar-vin: Enable support for r8a774a1
On Mon, Sep 10, 2018 at 03:31:17PM +0100, Biju Das wrote: > Add the SoC specific information for RZ/G2M(r8a774a1) SoC. > The VIN module of RZ/G2M is similar to R-Car M3-W. > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman
RE: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support
Thank you Simon for getting back to me. Cheers, Fab > Subject: Re: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 > support > > On Thu, Nov 08, 2018 at 12:52:30PM +, Fabrizio Castro wrote: > > Dear All, > > > > Who is the best person to take this patch? > > I believe this is for Mauro. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Re: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support
On Mon, Sep 10, 2018 at 03:31:16PM +0100, Biju Das wrote: > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman
Re: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
On Mon, Sep 10, 2018 at 03:31:15PM +0100, Biju Das wrote: > Add the MIPI CSI-2 driver support for RZ/G2M(r8a774a1) SoC. > The CSI-2 module of RZ/G2M is similar to R-Car M3-W. > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman
RE: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support
Hello Mauro, Does this patch look ok to you? Thanks, Fab > From: linux-renesas-soc-ow...@vger.kernel.org > On Behalf Of Biju Das > Sent: 10 September 2018 15:31 > Subject: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support > > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > b/Documentation/devicetree/bindings/media/rcar_vin.txt > index 2f42005..8c81689 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -12,6 +12,7 @@ on Gen3 platforms to a CSI-2 receiver. > - compatible: Must be one or more of the following > - "renesas,vin-r8a7743" for the R8A7743 device > - "renesas,vin-r8a7745" for the R8A7745 device > + - "renesas,vin-r8a774a1" for the R8A774A1 device > - "renesas,vin-r8a7778" for the R8A7778 device > - "renesas,vin-r8a7779" for the R8A7779 device > - "renesas,vin-r8a7790" for the R8A7790 device > @@ -58,9 +59,9 @@ The per-board settings Gen2 platforms: > - data-enable-active: polarity of CLKENB signal, see [1] for >description. Default is active high. > > -The per-board settings Gen3 platforms: > +The per-board settings Gen3 and RZ/G2 platforms: > > -Gen3 platforms can support both a single connected parallel input source > +Gen3 and RZ/G2 platforms can support both a single connected parallel input > source > from external SoC pins (port@0) and/or multiple parallel input sources > from local SoC CSI-2 receivers (port@1) depending on SoC. > > -- > 2.7.4 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
RE: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
Thank you Simon for getting back to me. Cheers, Fab > Subject: Re: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1 > > On Thu, Nov 08, 2018 at 12:51:29PM +, Fabrizio Castro wrote: > > Dear All, > > > > Who is the best person to take this patch? > > I believe this is for Mauro. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
RE: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
Hello Mauro,
Does this patch look ok to you?
Thanks,
Fab
> From: linux-renesas-soc-ow...@vger.kernel.org
> On Behalf Of Biju Das
> Sent: 10 September 2018 15:31
> Subject: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
>
> Add the MIPI CSI-2 driver support for RZ/G2M(r8a774a1) SoC.
> The CSI-2 module of RZ/G2M is similar to R-Car M3-W.
>
> Signed-off-by: Biju Das
> Reviewed-by: Fabrizio Castro
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index daef72d..65c7efb 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -953,6 +953,10 @@ static const struct rcar_csi2_info
> rcar_csi2_info_r8a77970 = {
>
> static const struct of_device_id rcar_csi2_of_table[] = {
> {
> +.compatible = "renesas,r8a774a1-csi2",
> +.data = &rcar_csi2_info_r8a7796,
> +},
> +{
> .compatible = "renesas,r8a7795-csi2",
> .data = &rcar_csi2_info_r8a7795,
> },
> --
> 2.7.4
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End,
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered
No. 04586709.
Re: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support
On Thu, Nov 08, 2018 at 12:52:30PM +, Fabrizio Castro wrote: > Dear All, > > Who is the best person to take this patch? I believe this is for Mauro.
RE: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 support
Thank you Simon for getting back to me. Cheers, Fab > Subject: Re: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 > support > > On Thu, Nov 08, 2018 at 12:50:22PM +, Fabrizio Castro wrote: > > Dear All, > > > > Who is the best person to take this patch? > > I believe this patch is for Mauro. Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
RE: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 support
Hello Mauro, Does this patch look ok to you? Thanks, Fab > From: Biju Das > Sent: 10 September 2018 15:31 > Subject: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 > support > > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro > --- > Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > index 2d385b6..12fe685 100644 > --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > @@ -2,12 +2,13 @@ Renesas R-Car MIPI CSI-2 > > > The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the > -Renesas R-Car family of devices. It is used in conjunction with the > -R-Car VIN module, which provides the video capture capabilities. > +Renesas R-Car Gen3 and RZ/G2 family of devices. It is used in conjunction > +with the R-Car VIN module, which provides the video capture capabilities. > > Mandatory properties > > - compatible: Must be one or more of the following > + - "renesas,r8a774a1-csi2" for the R8A774A1 device. > - "renesas,r8a7795-csi2" for the R8A7795 device. > - "renesas,r8a7796-csi2" for the R8A7796 device. > - "renesas,r8a77965-csi2" for the R8A77965 device. > -- > 2.7.4 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Re: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
On Thu, Nov 08, 2018 at 12:51:29PM +, Fabrizio Castro wrote: > Dear All, > > Who is the best person to take this patch? I believe this is for Mauro.
Re: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 support
On Mon, Sep 10, 2018 at 03:31:14PM +0100, Biju Das wrote: > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro Reviewed-by: Simon Horman
Re: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 support
On Thu, Nov 08, 2018 at 12:50:22PM +, Fabrizio Castro wrote: > Dear All, > > Who is the best person to take this patch? I believe this patch is for Mauro.
Re: [PATCH] MAINTAINERS fixups
On 11/08/2018 02:23 PM, Hans Verkuil wrote: > Update file paths in MAINTAINERS. > > Signed-off-by: Hans Verkuil > Reported-by: Joe Perches Reviewed-by: Sylwester Nawrocki
[PATCH] MAINTAINERS fixups
Update file paths in MAINTAINERS. Signed-off-by: Hans Verkuil Reported-by: Joe Perches --- diff --git a/MAINTAINERS b/MAINTAINERS index a8588dedc683..4ee360fb33ad 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2062,7 +2062,6 @@ M:Andrzej Hajda L: linux-arm-ker...@lists.infradead.org L: linux-media@vger.kernel.org S: Maintained -F: arch/arm/plat-samsung/s5p-dev-mfc.c F: drivers/media/platform/s5p-mfc/ ARM/SHMOBILE ARM ARCHITECTURE @@ -3913,7 +3912,6 @@ T:git git://linuxtv.org/media_tree.git W: http://linuxtv.org S: Odd Fixes F: drivers/media/i2c/cs3308.c -F: drivers/media/i2c/cs3308.h CS5535 Audio ALSA driver M: Jaya Kumar @@ -3944,7 +3942,7 @@ T:git git://linuxtv.org/media_tree.git W: https://linuxtv.org S: Maintained F: drivers/media/common/cx2341x* -F: include/media/cx2341x* +F: include/media/drv-intf/cx2341x.h CX24120 MEDIA DRIVER M: Jemma Denson @@ -9670,14 +9668,14 @@ L: linux-media@vger.kernel.org S: Supported F: drivers/media/platform/atmel/atmel-isc.c F: drivers/media/platform/atmel/atmel-isc-regs.h -F: devicetree/bindings/media/atmel-isc.txt +F: Documentation/devicetree/bindings/media/atmel-isc.txt MICROCHIP ISI DRIVER M: Eugen Hristev L: linux-media@vger.kernel.org S: Supported F: drivers/media/platform/atmel/atmel-isi.c -F: include/media/atmel-isi.h +F: drivers/media/platform/atmel/atmel-isi.h MICROCHIP AT91 USART MFD DRIVER M: Radu Pirea @@ -12991,7 +12989,7 @@ T: git git://linuxtv.org/media_tree.git S: Maintained F: drivers/media/common/saa7146/ F: drivers/media/pci/saa7146/ -F: include/media/saa7146* +F: include/media/drv-intf/saa7146* SAMSUNG AUDIO (ASoC) DRIVERS M: Krzysztof Kozlowski
RE: [PATCH 4/5] media: rcar-vin: Enable support for r8a774a1
Dear All,
Who is the best person to take this patch?
Cheers,
Fab
> From: Biju Das
> Sent: 10 September 2018 15:31
> Subject: [PATCH 4/5] media: rcar-vin: Enable support for r8a774a1
>
> Add the SoC specific information for RZ/G2M(r8a774a1) SoC.
> The VIN module of RZ/G2M is similar to R-Car M3-W.
>
> Signed-off-by: Biju Das
> Reviewed-by: Fabrizio Castro
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..c0c84d1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -995,6 +995,10 @@ static const struct rvin_info rcar_info_r8a77970 = {
>
> static const struct of_device_id rvin_of_id_table[] = {
> {
> +.compatible = "renesas,vin-r8a774a1",
> +.data = &rcar_info_r8a7796,
> +},
> +{
> .compatible = "renesas,vin-r8a7778",
> .data = &rcar_info_m1,
> },
> --
> 2.7.4
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End,
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered
No. 04586709.
RE: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support
Dear All, Who is the best person to take this patch? Thanks, Fab > From: linux-renesas-soc-ow...@vger.kernel.org > On Behalf Of Biju Das > Sent: 10 September 2018 15:31 > Subject: [PATCH 3/5] media: dt-bindings: media: rcar_vin: Add r8a774a1 support > > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro > --- > Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt > b/Documentation/devicetree/bindings/media/rcar_vin.txt > index 2f42005..8c81689 100644 > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt > @@ -12,6 +12,7 @@ on Gen3 platforms to a CSI-2 receiver. > - compatible: Must be one or more of the following > - "renesas,vin-r8a7743" for the R8A7743 device > - "renesas,vin-r8a7745" for the R8A7745 device > + - "renesas,vin-r8a774a1" for the R8A774A1 device > - "renesas,vin-r8a7778" for the R8A7778 device > - "renesas,vin-r8a7779" for the R8A7779 device > - "renesas,vin-r8a7790" for the R8A7790 device > @@ -58,9 +59,9 @@ The per-board settings Gen2 platforms: > - data-enable-active: polarity of CLKENB signal, see [1] for >description. Default is active high. > > -The per-board settings Gen3 platforms: > +The per-board settings Gen3 and RZ/G2 platforms: > > -Gen3 platforms can support both a single connected parallel input source > +Gen3 and RZ/G2 platforms can support both a single connected parallel input > source > from external SoC pins (port@0) and/or multiple parallel input sources > from local SoC CSI-2 receivers (port@1) depending on SoC. > > -- > 2.7.4 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
RE: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
Dear All,
Who is the best person to take this patch?
Thanks,
Fab
> From: linux-renesas-soc-ow...@vger.kernel.org
> On Behalf Of Biju Das
> Sent: 10 September 2018 15:31
> Subject: [PATCH 2/5] media: rcar-csi2: Enable support for r8a774a1
>
> Add the MIPI CSI-2 driver support for RZ/G2M(r8a774a1) SoC.
> The CSI-2 module of RZ/G2M is similar to R-Car M3-W.
>
> Signed-off-by: Biju Das
> Reviewed-by: Fabrizio Castro
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index daef72d..65c7efb 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -953,6 +953,10 @@ static const struct rcar_csi2_info
> rcar_csi2_info_r8a77970 = {
>
> static const struct of_device_id rcar_csi2_of_table[] = {
> {
> +.compatible = "renesas,r8a774a1-csi2",
> +.data = &rcar_csi2_info_r8a7796,
> +},
> +{
> .compatible = "renesas,r8a7795-csi2",
> .data = &rcar_csi2_info_r8a7795,
> },
> --
> 2.7.4
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End,
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered
No. 04586709.
RE: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 support
Dear All, Who is the best person to take this patch? Thanks, Fab > From: Biju Das > Sent: 10 September 2018 15:31 > Subject: [PATCH 1/5] media: dt-bindings: media: rcar-csi2: Add r8a774a1 > support > > Document RZ/G2M (R8A774A1) SoC bindings. > > The RZ/G2M SoC is similar to R-Car M3-W (R8A7796). > > Signed-off-by: Biju Das > Reviewed-by: Fabrizio Castro > --- > Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > index 2d385b6..12fe685 100644 > --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt > @@ -2,12 +2,13 @@ Renesas R-Car MIPI CSI-2 > > > The R-Car CSI-2 receiver device provides MIPI CSI-2 capabilities for the > -Renesas R-Car family of devices. It is used in conjunction with the > -R-Car VIN module, which provides the video capture capabilities. > +Renesas R-Car Gen3 and RZ/G2 family of devices. It is used in conjunction > +with the R-Car VIN module, which provides the video capture capabilities. > > Mandatory properties > > - compatible: Must be one or more of the following > + - "renesas,r8a774a1-csi2" for the R8A774A1 device. > - "renesas,r8a7795-csi2" for the R8A7795 device. > - "renesas,r8a7796-csi2" for the R8A7796 device. > - "renesas,r8a77965-csi2" for the R8A77965 device. > -- > 2.7.4 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
[PATCH 1/2] media: cxd2880-spi: Add optional vcc regulator
This patchset adds an optional VCC regulator to the driver probe function to
make sure power is enabled to the module before starting attaching to
the device.
Signed-off-by: Neil Armstrong
---
drivers/media/spi/cxd2880-spi.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index c437309..d5c433e 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
#include
+#include
#include
#include
@@ -51,6 +52,7 @@ struct cxd2880_dvb_spi {
struct mutex spi_mutex; /* For SPI access exclusive control */
int feed_count;
int all_pid_feed_count;
+ struct regulator *vcc_supply;
u8 *ts_buf;
struct cxd2880_pid_filter_config filter_config;
};
@@ -518,6 +520,17 @@ cxd2880_spi_probe(struct spi_device *spi)
if (!dvb_spi)
return -ENOMEM;
+ dvb_spi->vcc_supply = devm_regulator_get_optional(&spi->dev, "vcc");
+ if (IS_ERR(dvb_spi->vcc_supply)) {
+ if (PTR_ERR(dvb_spi->vcc_supply) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dvb_spi->vcc_supply = NULL;
+ } else {
+ ret = regulator_enable(dvb_spi->vcc_supply);
+ if (ret)
+ return ret;
+ }
+
dvb_spi->spi = spi;
mutex_init(&dvb_spi->spi_mutex);
dev_set_drvdata(&spi->dev, dvb_spi);
@@ -631,6 +644,9 @@ cxd2880_spi_remove(struct spi_device *spi)
dvb_frontend_detach(&dvb_spi->dvb_fe);
dvb_unregister_adapter(&dvb_spi->adapter);
+ if (dvb_spi->vcc_supply)
+ regulator_disable(dvb_spi->vcc_supply);
+
kfree(dvb_spi);
pr_info("cxd2880_spi remove ok.\n");
--
2.7.4
[PATCH 2/2] media: sony-cxd2880: add optional vcc regulator to bindings
This patchset adds an optional VCC regulator to the bindings of the Sony
CXD2880 DVB-T2/T tuner + demodulator adapter.
Signed-off-by: Neil Armstrong
---
Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt | 4
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt
b/Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt
index fc5aa26..98a72c0 100644
--- a/Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt
+++ b/Documentation/devicetree/bindings/media/spi/sony-cxd2880.txt
@@ -5,6 +5,10 @@ Required properties:
- reg: SPI chip select number for the device.
- spi-max-frequency: Maximum bus speed, should be set to <5500> (55MHz).
+Optional properties:
+- vcc-supply: Optional phandle to the vcc regulator to power the adapter,
+ as described in the file ../regulator/regulator.txt
+
Example:
cxd2880@0 {
--
2.7.4
[PATCH 0/2] sony-cxd2880: add optional vcc regulator
This patchset adds an optional VCC regulator to the bindings and driver to make sure power is enabled to the module before starting attaching to the device. Neil Armstrong (2): media: cxd2880-spi: Add optional vcc regulator media: sony-cxd2880: add optional vcc regulator to bindings .../devicetree/bindings/media/spi/sony-cxd2880.txt | 4 drivers/media/spi/cxd2880-spi.c | 16 2 files changed, 20 insertions(+) -- 2.7.4
[PATCH] media: cxd2880-spi: fix probe when dvb_attach fails
When dvb_attach fails, probe returns 0, and remove crashes afterwards.
This patch sets the return value to -ENODEV when attach fails.
Fixes: bd24fcddf6b8 ("media: cxd2880-spi: Add support for CXD2880 SPI
interface")
Signed-off-by: Neil Armstrong
---
drivers/media/spi/cxd2880-spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index 11ce510..c437309 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -536,6 +536,7 @@ cxd2880_spi_probe(struct spi_device *spi)
if (!dvb_attach(cxd2880_attach, &dvb_spi->dvb_fe, &config)) {
pr_err("cxd2880_attach failed\n");
+ ret = -ENODEV;
goto fail_attach;
}
--
2.7.4
raw notes from the Media summit
In order to help writing a media summit report, I'm sending the raw notes that were at Etherpad after the end of the meeting. Thanks, Mauro Media summit 25-10-2018 Edinburgh https://www.spinics.net/lists/linux-media/msg141095.html Attendees Brad Love Ezequiel Garcia Gustavo Padovan Hans Verkuil Helen Koike Hidenori Yamaji Ivan Kalinin Jacopo Mondi Kieran Bingham Laurent Pinchart Mauro Chebab Maxime Ripard Michael Grzeschik Michael Ira Krufky Niklas Söderlund Patrick Lai Paul Elder Peter Griffin Ralph Clark Ricardo Ribalda Sakari Ailus Sean Young Seung-Woo Kim Stefan Klug Vinod Koul Topics CEC (Hans Verkuil) 1 pin bus, cec-gpio error injection support Tda998x (including BeagleBoard Bone support) ChromeOS CEC support DisplayPort CEC Tunneling over AUX for i915, nouveau, amdgpu Still lots of work happening around CEC utilities, the compliance test is in continuous use at cisco. URL to CEC status: https://hverkuil.home.xs4all.nl/cec-status.txt CISCO using CEC in their products, and pushing for development of CEC upstream. Automated test pipelines in place utilising cec-compliance 1.4 spec can be found quite easily online (http://d1.amobbs.com/bbs_upload782111/files_51/ourdev_716302E34B9Q.pdf) - Hans will tell us diff to 2.0 RC-Core progress report (Sean Young) Ported/removed all staging lirc drivers BPF IR decoding support. rc-core protocol decoders only support the most common protocols old lirc userspace rarely required Linux supports more IR hardware than any other operating system. Future/TODO Some driver work needed (mostly for older hardware) Build dvb without rc-core More BPF protocol decoders Support 2.4ghz remotes Needs automated testing Support with gpio and simple ir led is available Persistent storage of controls (Ricardo Ribalda - Qtec) Sensors usually come with some calibration data from the manufacturer about dead pixels etc. How to get this calibration data into the sensor. proposal: - new vid interface /dev/videoX /dev/v4l-subX - s_ext_ctrls - write array of ctrls - s_ctrl - replace specfific ctrl - dt - locate ctrl vals - (reuse v4l2 ioctls for save/restore/get value(s)) - implementation in two parts: main driver intf, storage why not set sensor vals from userspace? - user might impl wrong - if part of driver can make sure they work for specific sensor no get_min/max coz storage might not have space to save all ctrls NVMEM might be good for storage rw - Is it a filesystem (or is that more overhead) - controls validate the data in the same way as they are validated by the device sensor. Maintenance process tools - DIM (Laurent Pinchart - Ideas on Board) DRM have switched to co-maintainer model - Burden is shared - Increased commit rights to the mainline (multi-maintaner model) - no enforeced maintainer process (each their own) - multi-comitter means each has to do checks - need to standardize tools -> DIM - DIM wrapper for git kindaish - DRM inglorious maintenance tool : https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html - conflict resolution by commiter, not maintainer - DIM uses conflict resoultion system in git to pre-prepare the merge conflicts in a shared cache. - tool enforces quality checks drawbacks - pushing = testing = rebuilding :/ -> becomes a burden for the committers - Patches are integrated from a patchwork instance, and the commits then reference the history of the patch upstream - Adds an extra tool that must be used, and then an extra delay. - eg. takes time/difficult to ack easyish to get commit rights - Responsibility to perform correctly - spreads the review workload (creates review-economics) - trading rb tags -> lower quality :/ - "please submit patches" "also please merge conflict resolution and rebuild" - If automated testing can validate the process, it can simplify commit process for multi-commiters. - auto testing on test/build farm had this model in media for a few years - somebody merged 9k lines and lost a handful of devs - probably exception, model fine - many comitters but few maintainers - problem could be technically resolved too few ppl reviewing patches - wait time measured in months :) - multi-commiter model try to solve this problem incentive to get more maintainers - commit rights - responsilibity - get attached to code :) Automated testing (Ezequiel Garcia, Gustavo Padovan, Sakari Ailus) Testing only one config, one version, one patch application, one hardware (one test to rule them all). Sometimes testing is omitted altogether by a developer, or it has been done before making changes to a patch. A lot of problems will only be found in the additional validation steps Mauro now performs, or when the patches already have reached the m
[PATCH] vb2: check memory model for VIDIOC_CREATE_BUFS
vb2_core_create_bufs did not check if the memory model for newly added
buffers is the same as for already existing buffers. It should return an
error if they aren't the same.
Signed-off-by: Hans Verkuil
Reported-by: syzbot+e1fb118a2ebb88031...@syzkaller.appspotmail.com
Cc: # for v4.16 and up
---
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index 975ff5669f72..c49c67473408 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -812,6 +812,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
vb2_memory memory,
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
q->waiting_for_buffers = !q->is_output;
+ } else if (q->memory != memory) {
+ dprintk(1, "memory model mismatch\n");
+ return -EINVAL;
}
num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
[PATCH v3.16 0/2] V4L2 event subscription fixes
Hi Ben, The two patches fix a use-after-free issue in V4L2 event handling. The first patch that fixes that issue is already in the other stable trees (as well as Linus's tree) whereas the second that fixes a bug in the first one, is in the media tree only as of yet. https://git.linuxtv.org/media_tree.git/commit/?id=92539d3eda2c090b382699bbb896d4b54e9bdece> Sakari Ailus (2): v4l: event: Prevent freeing event subscriptions while accessed v4l: event: Add subscription to list before calling "add" operation drivers/media/v4l2-core/v4l2-event.c | 63 drivers/media/v4l2-core/v4l2-fh.c| 2 ++ include/media/v4l2-fh.h | 1 + 3 files changed, 38 insertions(+), 28 deletions(-) -- 2.11.0
[PATCH v3.16 1/2] v4l: event: Prevent freeing event subscriptions while accessed
[ upstream commit ad608fbcf166fec809e402d548761768f602702c ]
The event subscriptions are added to the subscribed event list while
holding a spinlock, but that lock is subsequently released while still
accessing the subscription object. This makes it possible to unsubscribe
the event --- and freeing the subscription object's memory --- while
the subscription object is simultaneously accessed.
Prevent this by adding a mutex to serialise the event subscription and
unsubscription. This also gives a guarantee to the callback ops that the
add op has returned before the del op is called.
This change also results in making the elems field less special:
subscriptions are only added to the event list once they are fully
initialised.
Signed-off-by: Sakari Ailus
Reviewed-by: Hans Verkuil
Reviewed-by: Laurent Pinchart
Cc: sta...@vger.kernel.org # for 4.14 and up
Fixes: c3b5b0241f62 ("V4L/DVB: V4L: Events: Add backend")
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-event.c | 38 +++-
drivers/media/v4l2-core/v4l2-fh.c| 2 ++
include/media/v4l2-fh.h | 1 +
3 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index 8761aab99de95..fcf65f131a2ac 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -119,14 +119,6 @@ static void __v4l2_event_queue_fh(struct v4l2_fh *fh,
const struct v4l2_event *e
if (sev == NULL)
return;
- /*
-* If the event has been added to the fh->subscribed list, but its
-* add op has not completed yet elems will be 0, treat this as
-* not being subscribed.
-*/
- if (!sev->elems)
- return;
-
/* Increase event sequence number on fh. */
fh->sequence++;
@@ -209,6 +201,7 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
struct v4l2_subscribed_event *sev, *found_ev;
unsigned long flags;
unsigned i;
+ int ret = 0;
if (sub->type == V4L2_EVENT_ALL)
return -EINVAL;
@@ -226,31 +219,36 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
sev->flags = sub->flags;
sev->fh = fh;
sev->ops = ops;
+ sev->elems = elems;
+
+ mutex_lock(&fh->subscribe_lock);
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (!found_ev)
- list_add(&sev->list, &fh->subscribed);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
if (found_ev) {
+ /* Already listening */
kfree(sev);
- return 0; /* Already listening */
+ goto out_unlock;
}
if (sev->ops && sev->ops->add) {
- int ret = sev->ops->add(sev, elems);
+ ret = sev->ops->add(sev, elems);
if (ret) {
- sev->ops = NULL;
- v4l2_event_unsubscribe(fh, sub);
- return ret;
+ kfree(sev);
+ goto out_unlock;
}
}
- /* Mark as ready for use */
- sev->elems = elems;
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ list_add(&sev->list, &fh->subscribed);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
- return 0;
+out_unlock:
+ mutex_unlock(&fh->subscribe_lock);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(v4l2_event_subscribe);
@@ -289,6 +287,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
return 0;
}
+ mutex_lock(&fh->subscribe_lock);
+
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
@@ -306,6 +306,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
if (sev && sev->ops && sev->ops->del)
sev->ops->del(sev);
+ mutex_unlock(&fh->subscribe_lock);
+
kfree(sev);
return 0;
diff --git a/drivers/media/v4l2-core/v4l2-fh.c
b/drivers/media/v4l2-core/v4l2-fh.c
index e57c002b41506..573ec2f192d44 100644
--- a/drivers/media/v4l2-core/v4l2-fh.c
+++ b/drivers/media/v4l2-core/v4l2-fh.c
@@ -42,6 +42,7 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device
*vdev)
INIT_LIST_HEAD(&fh->available);
INIT_LIST_HEAD(&fh->subscribed);
fh->sequence = -1;
+ mutex_init(&fh->subscribe_lock);
}
EXPORT_SYMBOL_GPL(v4l2_fh_init);
@@ -88,6 +89,7 @@ void v4l2_fh_exit(struct v4l2_fh *fh)
if (fh->vdev == NULL)
return;
v4l2_event_unsubscribe_all(fh);
+ mutex_destroy(&fh->subscribe_lock);
fh->vdev = NULL;
}
EXPORT_SYMBOL_GPL(v4l2_fh_exit);
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 803516775162d..a2777a324e083 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2
[PATCH v3.16 2/2] v4l: event: Add subscription to list before calling "add" operation
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Cc: sta...@vger.kernel.org (for 4.14 and up)
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index fcf65f131a2ac..35901f5b3971b 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -194,6 +194,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(&fh->subscribe_lock);
+ assert_spin_locked(&fh->vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(&sev->events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(&sev->list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -225,27 +241,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(&sev->list, &fh->subscribed);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
kfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(&fh->vdev->fh_lock, flags);
- list_add(&sev->list, &fh->subscribed);
- spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(&fh->subscribe_lock);
return ret;
@@ -280,7 +292,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -292,14 +303,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(&sev->events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(&sev->list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
--
2.11.0
[PATCH v4.4 1/1] v4l: event: Add subscription to list before calling "add" operation
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Cc: sta...@vger.kernel.org (for 4.14 and up)
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b47ac4e053d0e..f5c8a952f0aa3 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -197,6 +197,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(&fh->subscribe_lock);
+ assert_spin_locked(&fh->vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(&sev->events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(&sev->list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -228,27 +244,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(&sev->list, &fh->subscribed);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
kfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(&fh->vdev->fh_lock, flags);
- list_add(&sev->list, &fh->subscribed);
- spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(&fh->subscribe_lock);
return ret;
@@ -283,7 +295,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -295,14 +306,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(&sev->events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(&sev->list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
--
2.11.0
[PATCH v4.9 1/1] v4l: event: Add subscription to list before calling "add" operation
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Cc: sta...@vger.kernel.org (for 4.14 and up)
Signed-off-by: Mauro Carvalho Chehab
---
Hi Greg,
This is a backport of a fix in the media tree for the 4.9 stable series.
https://git.linuxtv.org/media_tree.git/commit/?id=92539d3eda2c090b382699bbb896d4b54e9bdece>
I'll send patches for 4.4 and 3.16 (to Ben H.) kernels shortly.
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index 567d86835f001..1fda2873375f6 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -197,6 +197,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(&fh->subscribe_lock);
+ assert_spin_locked(&fh->vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(&sev->events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(&sev->list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -228,27 +244,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(&sev->list, &fh->subscribed);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(&fh->vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
kfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(&fh->vdev->fh_lock, flags);
- list_add(&sev->list, &fh->subscribed);
- spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(&fh->subscribe_lock);
return ret;
@@ -283,7 +295,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -295,14 +306,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(&fh->vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(&sev->events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(&sev->list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
--
2.11.0
Re: [PATCH] media: staging: tegra-vde: print long unsigned using %lu format specifier
On 08.11.2018 14:02, Colin King wrote:
> From: Colin Ian King
>
> The frame.flags & FLAG_B_FRAME is promoted to a long unsigned because
> of the use of the BIT() macro when defining FLAG_B_FRAME and causing a
> build warning. Fix this by using the %lu format specifer.
>
> Cleans up warning:
> drivers/staging/media/tegra-vde/tegra-vde.c:267:5: warning: format
> specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
> b/drivers/staging/media/tegra-vde/tegra-vde.c
> index 6f06061a40d9..66cf14212c14 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -262,7 +262,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
> *vde,
> value |= frame->frame_num;
>
> dev_dbg(vde->miscdev.parent,
> - "\tFrame %d: frame_num = %d B_frame = %d\n",
> + "\tFrame %d: frame_num = %d B_frame = %lu\n",
> i + 1, frame->frame_num,
> (frame->flags & FLAG_B_FRAME));
> } else {
>
Thanks,
Acked-by: Dmitry Osipenko
Re: [PATCH v4 3/4] media: i2c: Add MAX9286 driver
Hi Jacopo, On 08/11/18 11:11, jacopo mondi wrote: > Hi Luca, Kieran, > sorry to jump up, but I feel this should be clarified. > > On Wed, Nov 07, 2018 at 06:24:18PM +0100, Luca Ceresoli wrote: >> Hi Kieran, >> >> thanks for the clarification. One additional note below. >> >> On 07/11/18 16:06, Kieran Bingham wrote: >>> Hi Luca >>> >>> >>> kbingham: hi, I'm looking at your GMSL v4 patches kbingham: jmondi helped me in understanding parts of it, but I still have a question kbingham: are the drivers waiting for the established link before the remote chips are accessed? where? >>> >>> I'm replying here rather than spam the IRC channel with a big paste. >>> It's also a useful description to the probe sequence, so I've kept it >>> with the driver posting. >>> >>> I hope the following helps illustrate the sequences which are involved: >>> >>> max9286_probe() >>> - max9286_i2c_mux_close() # Disable all links >>> - max9286_configure_i2c # Configure early communication settings >>> - max9286_init(): >>>- regulator_enable() # Power up all cameras >>>- max9286_setup() # Most link setup is done here. >>>... Set up v4l2/async/media-controller endpoints >>>- max9286_i2c_mux_init() # Start configuring cameras: >>> - i2c_mux_alloc() # Create our mux device >>> - for_each_source(dev, source) >>>i2c_mux_add_adapter() # This is where sensors get probed. >>> >>> So yes sensors are only communicated with once the link is brought up as >>> much as possible. >> >> For the records, an additional bit of explanation I got from Kieran via IRC. >> >> The fact that link is already up when the sensors are probed is due to >> the fact that the power regulator has a delay of *8 seconds*. This is >> intended, because there's an MCU on the camera modules that talks on the >> I2C bus during that time, and thus the drivers need to wait after it's done. > > The 8sec delay is due to the fact an integrated MCU on the remote > camera module programs the local sensor and the serializer intgrated > in the module in to some default configuration state. At power up, we > just want to let it finish, with all reverse channels closed > (camera module -> SoC direction) not to have the MCU transmitted > messages repeated to the local side (our remote serializer does repeat > messages not directed to it on it's remote side, as our local > deserialier does). > > The "link up" thing is fairly more complicated for GMSL than just > having a binary "on" or "off" mode. This technology defines two different > "channels", a 'configuration-channel' for transmitting control messages > on the serial link (i2c messages for the deserializer/serializer pair > this patches support) and a 'video-channel' for transmission of > high-speed data, such as, no surprise, video and images :) > > GMSL also defines two "link modes": a clock-less "configuration link" > and an high-speed "video link". The "configuration link" is available a > few msec after power up (roughly), while the "video link" needs a pixel > clock to be supplied to the serializer for it to enter this mode and > be able to lock the status between itself and the deserializer. Then it can > begin serializing video data. > > The 'control channel' is available both when the link is in > 'configuration' and 'video' mode, while the 'video' channel is > available only when the link is in 'video' mode (or, to put it more > simply: you can send i2c configuration messages while the link is > serializing video). > > Our implementation uses the link in 'configuration mode' during the > remote side programming phase, at 'max9286_i2c_mux_init()' time, with > the 'max9286_i2c_mux_select()' function enabling selectively the > 'configuration link' of each single remote end. It probes the remote device > by instantiating a new i2c_adapter connected to the mux, one for each > remote end, and performs the device configuration by initially using its > default power up i2c address (it is safe to do so, all other links are > closed), then changes the remote devices address to an unique one > (as our devices allows us to do so, otherwise you should use the > deserializer address translation feature to mask and translate the > remote addresses). > > Now all remote devices have an unique i2c address, and we can operate > with all 'configuration links' open with no risk of i2c addresses > collisions. > > At this point when we want to start the video stream, we send a > control message to the remote device, which enables the pixel clock > output from the image sensor, and activate the 'video channel' on the > remote serializer. The local deserializer makes sure all 'video links' > are locked (see 'max9286_check_video_links()') and at this point we > can begin serializing/deserializing video data. > > As you can see, the initial delay only plays a role in avoiding > collision before we properly configure the channels and the i2c > addresses. The link setu
Re: [PATCH] media: staging: tegra-vde: print long unsigned using %lu format specifier
On Thu, Nov 08, 2018 at 11:02:24AM +, Colin King wrote:
> From: Colin Ian King
>
> The frame.flags & FLAG_B_FRAME is promoted to a long unsigned because
> of the use of the BIT() macro when defining FLAG_B_FRAME and causing a
> build warning. Fix this by using the %lu format specifer.
>
> Cleans up warning:
> drivers/staging/media/tegra-vde/tegra-vde.c:267:5: warning: format
> specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
>
> Fixes: 42e764d05712 ("staging: tegravde: replace bit assignment with macro")
> Cc: Ioannis Valasakis
> Reported-by: Stephen Rothwell
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
> b/drivers/staging/media/tegra-vde/tegra-vde.c
> index 6f06061a40d9..66cf14212c14 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -262,7 +262,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
> *vde,
> value |= frame->frame_num;
>
> dev_dbg(vde->miscdev.parent,
> - "\tFrame %d: frame_num = %d B_frame = %d\n",
> + "\tFrame %d: frame_num = %d B_frame = %lu\n",
> i + 1, frame->frame_num,
> (frame->flags & FLAG_B_FRAME));
> } else {
> --
> 2.19.1
Thanks for this, you beat me too it :)
greg k-h
[PATCH] media: staging: tegra-vde: print long unsigned using %lu format specifier
From: Colin Ian King
The frame.flags & FLAG_B_FRAME is promoted to a long unsigned because
of the use of the BIT() macro when defining FLAG_B_FRAME and causing a
build warning. Fix this by using the %lu format specifer.
Cleans up warning:
drivers/staging/media/tegra-vde/tegra-vde.c:267:5: warning: format
specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
Signed-off-by: Colin Ian King
---
drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 6f06061a40d9..66cf14212c14 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -262,7 +262,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde
*vde,
value |= frame->frame_num;
dev_dbg(vde->miscdev.parent,
- "\tFrame %d: frame_num = %d B_frame = %d\n",
+ "\tFrame %d: frame_num = %d B_frame = %lu\n",
i + 1, frame->frame_num,
(frame->flags & FLAG_B_FRAME));
} else {
--
2.19.1
Re: [PATCH v1 5/5] media: venus: update number of bytes used field properly for EOS frames
Hi,
On 9/29/18 3:00 PM, Srinu Gorle wrote:
> - In video decoder session, update number of bytes used for
> yuv buffers appropriately for EOS buffers.
>
> Signed-off-by: Srinu Gorle
> ---
> drivers/media/platform/qcom/venus/vdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
NACK, that was already discussed see:
https://patchwork.kernel.org/patch/10630411/
>
> diff --git a/drivers/media/platform/qcom/venus/vdec.c
> b/drivers/media/platform/qcom/venus/vdec.c
> index 311f209..a48eed1 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -978,7 +978,7 @@ static void vdec_buf_done(struct venus_inst *inst,
> unsigned int buf_type,
>
> if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
> const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
> -
> + vb->planes[0].bytesused = bytesused;
> v4l2_event_queue_fh(&inst->fh, &ev);
> }
> } else {
>
--
regards,
Stan
Re: [PATCH v4 3/4] media: i2c: Add MAX9286 driver
Hi Luca, Kieran, sorry to jump up, but I feel this should be clarified. On Wed, Nov 07, 2018 at 06:24:18PM +0100, Luca Ceresoli wrote: > Hi Kieran, > > thanks for the clarification. One additional note below. > > On 07/11/18 16:06, Kieran Bingham wrote: > > Hi Luca > > > > > > > >> kbingham: hi, I'm looking at your GMSL v4 patches > >> kbingham: jmondi helped me in understanding parts of it, > >> but I still have a question > >> kbingham: are the drivers waiting for the established link > >> before the remote chips are accessed? where? > > > > I'm replying here rather than spam the IRC channel with a big paste. > > It's also a useful description to the probe sequence, so I've kept it > > with the driver posting. > > > > I hope the following helps illustrate the sequences which are involved: > > > > max9286_probe() > > - max9286_i2c_mux_close() # Disable all links > > - max9286_configure_i2c # Configure early communication settings > > - max9286_init(): > >- regulator_enable() # Power up all cameras > >- max9286_setup() # Most link setup is done here. > >... Set up v4l2/async/media-controller endpoints > >- max9286_i2c_mux_init() # Start configuring cameras: > > - i2c_mux_alloc() # Create our mux device > > - for_each_source(dev, source) > >i2c_mux_add_adapter() # This is where sensors get probed. > > > > So yes sensors are only communicated with once the link is brought up as > > much as possible. > > For the records, an additional bit of explanation I got from Kieran via IRC. > > The fact that link is already up when the sensors are probed is due to > the fact that the power regulator has a delay of *8 seconds*. This is > intended, because there's an MCU on the camera modules that talks on the > I2C bus during that time, and thus the drivers need to wait after it's done. The 8sec delay is due to the fact an integrated MCU on the remote camera module programs the local sensor and the serializer intgrated in the module in to some default configuration state. At power up, we just want to let it finish, with all reverse channels closed (camera module -> SoC direction) not to have the MCU transmitted messages repeated to the local side (our remote serializer does repeat messages not directed to it on it's remote side, as our local deserialier does). The "link up" thing is fairly more complicated for GMSL than just having a binary "on" or "off" mode. This technology defines two different "channels", a 'configuration-channel' for transmitting control messages on the serial link (i2c messages for the deserializer/serializer pair this patches support) and a 'video-channel' for transmission of high-speed data, such as, no surprise, video and images :) GMSL also defines two "link modes": a clock-less "configuration link" and an high-speed "video link". The "configuration link" is available a few msec after power up (roughly), while the "video link" needs a pixel clock to be supplied to the serializer for it to enter this mode and be able to lock the status between itself and the deserializer. Then it can begin serializing video data. The 'control channel' is available both when the link is in 'configuration' and 'video' mode, while the 'video' channel is available only when the link is in 'video' mode (or, to put it more simply: you can send i2c configuration messages while the link is serializing video). Our implementation uses the link in 'configuration mode' during the remote side programming phase, at 'max9286_i2c_mux_init()' time, with the 'max9286_i2c_mux_select()' function enabling selectively the 'configuration link' of each single remote end. It probes the remote device by instantiating a new i2c_adapter connected to the mux, one for each remote end, and performs the device configuration by initially using its default power up i2c address (it is safe to do so, all other links are closed), then changes the remote devices address to an unique one (as our devices allows us to do so, otherwise you should use the deserializer address translation feature to mask and translate the remote addresses). Now all remote devices have an unique i2c address, and we can operate with all 'configuration links' open with no risk of i2c addresses collisions. At this point when we want to start the video stream, we send a control message to the remote device, which enables the pixel clock output from the image sensor, and activate the 'video channel' on the remote serializer. The local deserializer makes sure all 'video links' are locked (see 'max9286_check_video_links()') and at this point we can begin serializing/deserializing video data. As you can see, the initial delay only plays a role in avoiding collision before we properly configure the channels and the i2c addresses. The link setup phase is instead an integral part of the system configuration, and there are no un-necessary delays used to work around it setup procedure. Does this help clarifying the system sta
[PATCH] adv*/tc358743/ths8200: fill in min width/height/pixelclock
The v4l2_dv_timings_cap struct is used to do sanity checks when setting and
enumerating DV timings, ensuring that only valid timings as per the HW
capabilities are allowed.
However, many drivers just filled in 0 for the minimum width, height or
pixelclock frequency. This can cause timings with e.g. 0 as width and height
to be accepted, which will in turn lead to a potential division by zero.
Fill in proper values are minimum boundaries. 640x350 was chosen since it is
the smallest resolution in v4l2-dv-timings.h. Same for 13 MHz as the lowest
pixelclock frequency (it's slightly below the minimum of 13.5 MHz in the
v4l2-dv-timings.h header).
Signed-off-by: Hans Verkuil
---
Note: there is another occurrence in vivid-vid-common.c, but that's fixed in
a separate patch that's already part of a pull request.
---
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 5b008b0002c0..aa8b04cfed0f 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -578,7 +578,7 @@ static const struct v4l2_dv_timings_cap ad9389b_timings_cap
= {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, 1920, 0, 1200, 2500, 17000,
+ V4L2_INIT_BT_TIMINGS(640, 1920, 350, 1200, 2500, 17000,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_REDUCED_BLANKING |
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index f3899cc84e27..88349b5053cc 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -130,7 +130,7 @@ static const struct v4l2_dv_timings_cap adv7511_timings_cap
= {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, ADV7511_MAX_WIDTH, 0, ADV7511_MAX_HEIGHT,
+ V4L2_INIT_BT_TIMINGS(640, ADV7511_MAX_WIDTH, 350, ADV7511_MAX_HEIGHT,
ADV7511_MIN_PIXELCLOCK, ADV7511_MAX_PIXELCLOCK,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 9eb7c70a7712..ff28f5692986 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -766,7 +766,7 @@ static const struct v4l2_dv_timings_cap
adv7604_timings_cap_analog = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, 1920, 0, 1200, 2500, 17000,
+ V4L2_INIT_BT_TIMINGS(640, 1920, 350, 1200, 2500, 17000,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_REDUCED_BLANKING |
@@ -777,7 +777,7 @@ static const struct v4l2_dv_timings_cap
adv76xx_timings_cap_digital = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, 1920, 0, 1200, 2500, 22500,
+ V4L2_INIT_BT_TIMINGS(640, 1920, 350, 1200, 2500, 22500,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_REDUCED_BLANKING |
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 4721d49dcf0f..5305c3ad80e6 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -663,7 +663,7 @@ static const struct v4l2_dv_timings_cap
adv7842_timings_cap_analog = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, 1920, 0, 1200, 2500, 17000,
+ V4L2_INIT_BT_TIMINGS(640, 1920, 350, 1200, 2500, 17000,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_REDUCED_BLANKING |
@@ -674,7 +674,7 @@ static const struct v4l2_dv_timings_cap
adv7842_timings_cap_digital = {
.type = V4L2_DV_BT_656_1120,
/* keep this initialization for compatibility with GCC < 4.4.6 */
.reserved = { 0 },
- V4L2_INIT_BT_TIMINGS(0, 1920, 0, 1200, 2500, 22500,
+ V4L2_INIT_BT_TIMINGS(640, 1920, 350, 1200, 2500, 22500,
V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
V4L2_DV_BT_STD_GTF | V4L2_DV_BT_STD_CVT,
V4L2_DV_BT_CAP_PROGRESSIVE | V4L2_DV_BT_CAP_REDUCED_BLANKING |
diff --git a/drivers/media/
Re: [PATCH v4 2/3] media: meson: add v4l2 m2m video decoder driver
On 11/06/2018 08:59 AM, Maxime Jourdan wrote:
> Amlogic SoCs feature a powerful video decoder unit able to
> decode many formats, with a performance of usually up to 4k60.
>
> This is a driver for this IP that is based around the v4l2 m2m framework.
>
> It features decoding for:
> - MPEG 1
> - MPEG 2
>
> Supported SoCs are: GXBB (S905), GXL (S905X/W/D), GXM (S912)
>
> There is also a hardware bitstream parser (ESPARSER) that is handled here.
>
> Signed-off-by: Maxime Jourdan
> ---
> drivers/media/platform/Kconfig| 10 +
> drivers/media/platform/meson/Makefile |1 +
> drivers/media/platform/meson/vdec/Makefile|8 +
> .../media/platform/meson/vdec/codec_mpeg12.c | 209
> .../media/platform/meson/vdec/codec_mpeg12.h | 14 +
> drivers/media/platform/meson/vdec/dos_regs.h | 98 ++
> drivers/media/platform/meson/vdec/esparser.c | 322 +
> drivers/media/platform/meson/vdec/esparser.h | 32 +
> drivers/media/platform/meson/vdec/vdec.c | 1034 +
> drivers/media/platform/meson/vdec/vdec.h | 251
> drivers/media/platform/meson/vdec/vdec_1.c| 231
> drivers/media/platform/meson/vdec/vdec_1.h| 14 +
> .../media/platform/meson/vdec/vdec_helpers.c | 412 +++
> .../media/platform/meson/vdec/vdec_helpers.h | 48 +
> .../media/platform/meson/vdec/vdec_platform.c | 101 ++
> .../media/platform/meson/vdec/vdec_platform.h | 30 +
> 16 files changed, 2815 insertions(+)
> create mode 100644 drivers/media/platform/meson/vdec/Makefile
> create mode 100644 drivers/media/platform/meson/vdec/codec_mpeg12.c
> create mode 100644 drivers/media/platform/meson/vdec/codec_mpeg12.h
> create mode 100644 drivers/media/platform/meson/vdec/dos_regs.h
> create mode 100644 drivers/media/platform/meson/vdec/esparser.c
> create mode 100644 drivers/media/platform/meson/vdec/esparser.h
> create mode 100644 drivers/media/platform/meson/vdec/vdec.c
> create mode 100644 drivers/media/platform/meson/vdec/vdec.h
> create mode 100644 drivers/media/platform/meson/vdec/vdec_1.c
> create mode 100644 drivers/media/platform/meson/vdec/vdec_1.h
> create mode 100644 drivers/media/platform/meson/vdec/vdec_helpers.c
> create mode 100644 drivers/media/platform/meson/vdec/vdec_helpers.h
> create mode 100644 drivers/media/platform/meson/vdec/vdec_platform.c
> create mode 100644 drivers/media/platform/meson/vdec/vdec_platform.h
>
> +static int vdec_queue_setup(struct vb2_queue *q,
> + unsigned int *num_buffers, unsigned int *num_planes,
> + unsigned int sizes[], struct device *alloc_devs[])
> +{
> + struct amvdec_session *sess = vb2_get_drv_priv(q);
> + const struct amvdec_format *fmt_out = sess->fmt_out;
> + u32 output_size = amvdec_get_output_size(sess);
> + u32 buffers_total;
> +
> + if (*num_planes) {
If you are not supporting create_bufs, then you can drop this part.
Without create_bufs you can assume that *num_planes == 0 and
q->num_buffers == 0.
You should add a comment here mentioning that create_bufs isn't
supported by this driver and explain why it isn't supported.
I understand it is due to gstreamer problems, but the explanation
in your cover letter didn't say why it is a problem with this driver
but not other drivers (apparently).
> + switch (q->type) {
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + if (*num_planes != 1 || sizes[0] < output_size)
> + return -EINVAL;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + switch (sess->pixfmt_cap) {
> + case V4L2_PIX_FMT_NV12M:
> + if (*num_planes != 2 ||
> + sizes[0] < output_size ||
> + sizes[1] < output_size / 2)
> + return -EINVAL;
> + break;
> + case V4L2_PIX_FMT_YUV420M:
> + if (*num_planes != 3 ||
> + sizes[0] < output_size ||
> + sizes[1] < output_size / 4 ||
> + sizes[2] < output_size / 4)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> + break;
> + }
> +
> + return 0;
> + }
> +
> + switch (q->type) {
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + sizes[0] = amvdec_get_output_size(sess);
> + *num_planes = 1;
> + break;
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + switch (sess->pixfmt_cap) {
> + case V4L2_PIX_FMT_NV12M:
> + sizes[0] = output_size;
> + size
