Re: [PATCH] staging: erofs: fix potential double iput in erofs_read_super()

2019-01-24 Thread Chao Yu
On 2019/1/23 14:12, Chengguang Xu wrote:
> Some error cases like failing from d_make_root() will
> cause double iput because d_make_root() also does iput
> in its error path.
> 
> Signed-off-by: Chengguang Xu 

Reviewed-by: Chao Yu 

Thanks,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/2] media: vb2: Keep dma-buf buffers mapped until they are freed

2019-01-24 Thread Paul Kocialkowski
From: Pawel Osciak 

When using vb2 for video decoding, dequeued capture buffers may still
be accessed by the hardware: this is the case when they are used as
reference frames for decoding subsequent frames.

When the buffer is imported with dma-buf, it needs to be mapped before
access. Until now, it was mapped when queuing and unmapped when
dequeuing, which doesn't work for access as a reference frames.

One way to solve this would be to map the buffer again when it is
needed as a reference, but the mapping/unmapping operations can
seriously impact performance. As a result, map the buffer once (when it
is first needed when queued) and keep it mapped until it is freed.

Signed-off-by: Pawel Osciak 
Reviewed-on: https://chromium-review.googlesource.com/334103
Signed-off-by: Paul Kocialkowski 
[Paul: Updated for mainline and changed commit message]
---
 drivers/media/common/videobuf2/videobuf2-core.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 70e8c3366f9c..ce9294a635cc 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1196,6 +1196,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 * userspace knows sooner rather than later if the dma-buf map fails.
 */
for (plane = 0; plane < vb->num_planes; ++plane) {
+   if (vb->planes[plane].dbuf_mapped)
+   continue;
+
ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
if (ret) {
dprintk(1, "failed to map dmabuf for plane %d\n",
@@ -1758,14 +1761,6 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 
vb->state = VB2_BUF_STATE_DEQUEUED;
 
-   /* unmap DMABUF buffer */
-   if (q->memory == VB2_MEMORY_DMABUF)
-   for (i = 0; i < vb->num_planes; ++i) {
-   if (!vb->planes[i].dbuf_mapped)
-   continue;
-   call_void_memop(vb, unmap_dmabuf, 
vb->planes[i].mem_priv);
-   vb->planes[i].dbuf_mapped = 0;
-   }
call_void_bufop(q, init_buffer, vb);
 }
 
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] media: cedrus: Remove completed item from TODO list (dma-buf references)

2019-01-24 Thread Paul Kocialkowski
Access to reference frames that were imported from dma-buf was taken
care of and is no longer a pending item on the driver's TODO list.

Signed-off-by: Paul Kocialkowski 
---
 drivers/staging/media/sunxi/cedrus/TODO | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/TODO 
b/drivers/staging/media/sunxi/cedrus/TODO
index a951b3fd1ea1..ec277ece47af 100644
--- a/drivers/staging/media/sunxi/cedrus/TODO
+++ b/drivers/staging/media/sunxi/cedrus/TODO
@@ -5,8 +5,3 @@ Before this stateless decoder driver can leave the staging area:
 * Userspace support for the Request API needs to be reviewed;
 * Another stateless decoder driver should be submitted;
 * At least one stateless encoder driver should be submitted.
-* When queueing a request containing references to I frames, the
-  refcount of the memory for those I frames needs to be incremented
-  and decremented when the request is completed. This will likely
-  require some help from vb2. The driver should fail the request
-  if the memory/buffer is gone.
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Revert "media: cedrus: Allow using the current dst buffer as reference"

2019-01-24 Thread Paul Kocialkowski
This reverts commit cf20ae1535eb690a87c29b9cc7af51881384e967.

The vb2_find_timestamp helper was modified to allow finding buffers
regardless of their current state in the queue. This means that we
no longer have to take particular care of references to the current
capture buffer.
---
 drivers/staging/media/sunxi/cedrus/cedrus_dec.c   | 13 -
 drivers/staging/media/sunxi/cedrus/cedrus_dec.h   |  2 --
 drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 10 --
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index 2c295286766c..443fb037e1cf 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -22,19 +22,6 @@
 #include "cedrus_dec.h"
 #include "cedrus_hw.h"
 
-int cedrus_reference_index_find(struct vb2_queue *queue,
-   struct vb2_buffer *vb2_buf, u64 timestamp)
-{
-   /*
-* Allow using the current capture buffer as reference, which can occur
-* for field-coded pictures.
-*/
-   if (vb2_buf->timestamp == timestamp)
-   return vb2_buf->index;
-   else
-   return vb2_find_timestamp(queue, timestamp, 0);
-}
-
 void cedrus_device_run(void *priv)
 {
struct cedrus_ctx *ctx = priv;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.h 
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
index 8d0fc248220f..d1ae7903677b 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
@@ -16,8 +16,6 @@
 #ifndef _CEDRUS_DEC_H_
 #define _CEDRUS_DEC_H_
 
-int cedrus_reference_index_find(struct vb2_queue *queue,
-   struct vb2_buffer *vb2_buf, u64 timestamp);
 void cedrus_device_run(void *priv);
 
 #endif
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 81c66a8aa1ac..cb45fda9aaeb 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -10,7 +10,6 @@
 #include 
 
 #include "cedrus.h"
-#include "cedrus_dec.h"
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
@@ -160,8 +159,8 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
/* Forward and backward prediction reference buffers. */
-   forward_idx = cedrus_reference_index_find(cap_q, &run->dst->vb2_buf,
- slice_params->forward_ref_ts);
+   forward_idx = vb2_find_timestamp(cap_q,
+slice_params->forward_ref_ts, 0);
 
fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
@@ -169,9 +168,8 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
 
-   backward_idx = cedrus_reference_index_find(cap_q, &run->dst->vb2_buf,
-  
slice_params->backward_ref_ts);
-
+   backward_idx = vb2_find_timestamp(cap_q,
+ slice_params->backward_ref_ts, 0);
bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1);
 
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls

2019-01-24 Thread Paul Kocialkowski
Hi,

On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote:
> I forget a important thing, for the rkvdec and rk hevc decoder, it would 
> requests cabac table, scaling list, picture parameter set and reference 
> picture storing in one or various of DMA buffers. I am not talking about 
> the data been parsed, the decoder would requests a raw data.
> 
> For the pps and rps, it is possible to reuse the slice header, just let 
> the decoder know the offset from the bitstream bufer, I would suggest to 
> add three properties(with sps) for them. But I think we need a method to 
> mark a OUTPUT side buffer for those aux data.

I'm quite confused about the hardware implementation then. From what
you're saying, it seems that it takes the raw bitstream elements rather
than parsed elements. Is it really a stateless implementation?

The stateless implementation was designed with the idea that only the
raw slice data should be passed in bitstream form to the decoder. For
H.264, it seems that some decoders also need the slice header in raw
bitstream form (because they take the full slice NAL unit), see the
discussions in this thread:
media: docs-rst: Document m2m stateless video decoder interface

Can you detail exactly what the rockchip decoder absolutely needs in
raw bitstream format?

Cheers,

Paul

> On 1/8/19 6:00 PM, Ayaka wrote:
> > Sent from my iPad
> > 
> > > On Jan 8, 2019, at 4:38 PM, Paul Kocialkowski 
> > >  wrote:
> > > 
> > > Hi,
> > > 
> > > > On Tue, 2019-01-08 at 09:16 +0800, Ayaka wrote:
> > > > 
> > > > Sent from my iPad
> > > > 
> > > > > On Jan 7, 2019, at 5:57 PM, Paul Kocialkowski 
> > > > >  wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > > > On Mon, 2019-01-07 at 11:49 +0800, Randy Li wrote:
> > > > > > > On 12/12/18 8:51 PM, Paul Kocialkowski wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On Wed, 2018-12-05 at 21:59 +0100, Jernej Škrabec wrote:
> > > > > > > 
> > > > > > > > > +
> > > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE0x01
> > > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER0x02
> > > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR0x03
> > > > > > > > > +
> > > > > > > > > +#define V4L2_HEVC_DPB_ENTRIES_NUM_MAX16
> > > > > > > > > +
> > > > > > > > > +struct v4l2_hevc_dpb_entry {
> > > > > > > > > +__u32buffer_tag;
> > > > > > > > > +__u8rps;
> > > > > > > > > +__u8field_pic;
> > > > > > > > > +__u16pic_order_cnt[2];
> > > > > > > > > +};
> > > > > > Please add a property for reference index, if that rps is not used 
> > > > > > for
> > > > > > this, some device would request that(not the rockchip one). And
> > > > > > Rockchip's VDPU1 and VDPU2 for AVC would request a similar property.
> > > > > What exactly is that reference index? Is it a bitstream element or
> > > > > something deduced from the bitstream?
> > > > > 
> > > > picture order count(POC) for HEVC and frame_num in AVC. I think it is
> > > > the number used in list0(P slice and B slice) and list1(B slice).
> > > The picture order count is already the last field of the DPB entry
> > > structure. There is one for each field picture.
> > As we are not sure whether there is a field coded slice or CTU, I would 
> > hold this part and else about the field.
> > > > > > Adding another buffer_tag for referring the memory of the motion 
> > > > > > vectors
> > > > > > for each frames. Or a better method is add a meta data to echo 
> > > > > > picture
> > > > > > buffer,  since the picture output is just the same as the original,
> > > > > > display won't care whether the motion vectors are written the 
> > > > > > button of
> > > > > > picture or somewhere else.
> > > > > The motion vectors are passed as part of the raw bitstream data, in 
> > > > > the
> > > > > slices. Is there a case where the motion vectors are coded 
> > > > > differently?
> > > > No, it is an additional cache for decoder, even FFmpeg having such
> > > > data, I think allwinner must output it into somewhere.
> > > Ah yes I see what you mean! This is handled internally by our driver
> > > and not exposed to userspace. I don't think it would be a good idea to
> > > expose this cache or request that userspace allocates it like a video
> > > buffer.
> > > 
> > No, usually the driver should allocate, as the user space have no idea on 
> > size of each devices.
> > But for advantage user, application can fix a broken picture with a proper 
> > data or analysis a object motion from that.
> > So I would suggest attaching this information to a picture buffer as a meta 
> > data.
> > > > > > > > > +
> > > > > > > > > +struct v4l2_hevc_pred_weight_table {
> > > > > > > > > +__u8luma_log2_weight_denom;
> > > > > > > > > +__s8delta_chroma_log2_weight_denom;
> > > > > > > > > +
> > > > > > > > > +__s8
> > > > > > > > > delta_luma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > > > > > > +__s8luma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > >

Re: [PATCH] Revert "media: cedrus: Allow using the current dst buffer as reference"

2019-01-24 Thread Greg KH
On Thu, Jan 24, 2019 at 10:55:42AM +0100, Paul Kocialkowski wrote:
> This reverts commit cf20ae1535eb690a87c29b9cc7af51881384e967.
> 
> The vb2_find_timestamp helper was modified to allow finding buffers
> regardless of their current state in the queue. This means that we
> no longer have to take particular care of references to the current
> capture buffer.
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_dec.c   | 13 -
>  drivers/staging/media/sunxi/cedrus/cedrus_dec.h   |  2 --
>  drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 10 --
>  3 files changed, 4 insertions(+), 21 deletions(-)

No signed-off-by?  :(
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls

2019-01-24 Thread Paul Kocialkowski
Hi,

On Tue, 2019-01-08 at 18:00 +0800, Ayaka wrote:
> 
> Sent from my iPad
> 
> > On Jan 8, 2019, at 4:38 PM, Paul Kocialkowski 
> >  wrote:
> > 
> > Hi,
> > 
> > > On Tue, 2019-01-08 at 09:16 +0800, Ayaka wrote:
> > > 
> > > Sent from my iPad
> > > 
> > > > On Jan 7, 2019, at 5:57 PM, Paul Kocialkowski 
> > > >  wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > > > On Mon, 2019-01-07 at 11:49 +0800, Randy Li wrote:
> > > > > > On 12/12/18 8:51 PM, Paul Kocialkowski wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Wed, 2018-12-05 at 21:59 +0100, Jernej Škrabec wrote:
> > > > > > 
> > > > > > > > +
> > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE0x01
> > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER0x02
> > > > > > > > +#define V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR0x03
> > > > > > > > +
> > > > > > > > +#define V4L2_HEVC_DPB_ENTRIES_NUM_MAX16
> > > > > > > > +
> > > > > > > > +struct v4l2_hevc_dpb_entry {
> > > > > > > > +__u32buffer_tag;
> > > > > > > > +__u8rps;
> > > > > > > > +__u8field_pic;
> > > > > > > > +__u16pic_order_cnt[2];
> > > > > > > > +};
> > > > > 
> > > > > Please add a property for reference index, if that rps is not used 
> > > > > for 
> > > > > this, some device would request that(not the rockchip one). And 
> > > > > Rockchip's VDPU1 and VDPU2 for AVC would request a similar property.
> > > > 
> > > > What exactly is that reference index? Is it a bitstream element or
> > > > something deduced from the bitstream?
> > > > 
> > > picture order count(POC) for HEVC and frame_num in AVC. I think it is
> > > the number used in list0(P slice and B slice) and list1(B slice).
> > 
> > The picture order count is already the last field of the DPB entry
> > structure. There is one for each field picture.
> As we are not sure whether there is a field coded slice or CTU, I
> would hold this part and else about the field.

I'm not sure what you meant here, sorry.

> > > > > Adding another buffer_tag for referring the memory of the motion 
> > > > > vectors 
> > > > > for each frames. Or a better method is add a meta data to echo 
> > > > > picture 
> > > > > buffer,  since the picture output is just the same as the original, 
> > > > > display won't care whether the motion vectors are written the button 
> > > > > of 
> > > > > picture or somewhere else.
> > > > 
> > > > The motion vectors are passed as part of the raw bitstream data, in the
> > > > slices. Is there a case where the motion vectors are coded differently?
> > > No, it is an additional cache for decoder, even FFmpeg having such
> > > data, I think allwinner must output it into somewhere.
> > 
> > Ah yes I see what you mean! This is handled internally by our driver
> > and not exposed to userspace. I don't think it would be a good idea to
> > expose this cache or request that userspace allocates it like a video
> > buffer.
> > 
> No, usually the driver should allocate, as the user space have no
> idea on size of each devices.
> But for advantage user, application can fix a broken picture with a
> proper data or analysis a object motion from that.
> So I would suggest attaching this information to a picture buffer as
> a meta data. 

Right, the driver will allocate chunks of memory for the decoding
metadata used by the hardware decoder.

Well, I don't think V4L2 has any mechanism to expose this data for now
and since it's very specific to the hardware implementation, I guess
the interest in having that is generally pretty low.

That's maybe something that could be added later if someone wants to
work on it, but I think we are better off keeping this metadata hidden
by the driver for now.

> > > > > > > > +
> > > > > > > > +struct v4l2_hevc_pred_weight_table {
> > > > > > > > +__u8luma_log2_weight_denom;
> > > > > > > > +__s8delta_chroma_log2_weight_denom;
> > > > > > > > +
> > > > > > > > +__s8
> > > > > > > > delta_luma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > > > > > +__s8luma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > > > > > +__s8
> > > > > > > > delta_chroma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > > > > > > +__s8chroma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > > > > > > +
> > > > > > > > +__s8
> > > > > > > > delta_luma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > > > > > +__s8luma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> > > > > > > > +__s8
> > > > > > > > delta_chroma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > > > > > > +__s8chroma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> > > > > > > > +};
> > > > > > > > +
> > > > > Those properties I think are not necessary are applying for the 
> > > > > Rockchip's device, may not work for the others.
> > > > 
> > > > Yes, it's possible that some of the elements are not necessary for some
> > > > decoders. What we want is to cover all the elements that might be
> > > > required for a d

Re: [PATCH v10 01/13] media: staging/imx: refactor imx media device probe

2019-01-24 Thread Sakari Ailus
Hi Rui,

On Wed, Jan 23, 2019 at 10:52:10AM +, Rui Miguel Silva wrote:
> Refactor and move media device initialization code to a new common
> module, so it can be used by other devices, this will allow for example
> a near to introduce imx7 CSI driver, to use this media device.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  drivers/staging/media/imx/Makefile|   1 +
>  .../staging/media/imx/imx-media-dev-common.c  | 102 ++
>  drivers/staging/media/imx/imx-media-dev.c |  88 ---
>  drivers/staging/media/imx/imx-media-of.c  |   6 +-
>  drivers/staging/media/imx/imx-media.h |  15 +++
>  5 files changed, 141 insertions(+), 71 deletions(-)
>  create mode 100644 drivers/staging/media/imx/imx-media-dev-common.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 698a4210316e..a30b3033f9a3 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o
> +imx-media-objs += imx-media-dev-common.o
>  imx-media-common-objs := imx-media-utils.o imx-media-fim.o
>  imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o
>  
> diff --git a/drivers/staging/media/imx/imx-media-dev-common.c 
> b/drivers/staging/media/imx/imx-media-dev-common.c
> new file mode 100644
> index ..55fe94fb72f2
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-dev-common.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL
> +/*
> + * V4L2 Media Controller Driver for Freescale common i.MX5/6/7 SOC
> + *
> + * Copyright (c) 2018 Linaro Ltd
> + * Copyright (c) 2016 Mentor Graphics Inc.
> + */
> +
> +#include 
> +#include 
> +#include "imx-media.h"
> +
> +static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {
> + .bound = imx_media_subdev_bound,
> + .complete = imx_media_probe_complete,
> +};
> +
> +static const struct media_device_ops imx_media_md_ops = {
> + .link_notify = imx_media_link_notify,
> +};
> +
> +struct imx_media_dev *imx_media_dev_init(struct device *dev)
> +{
> + struct imx_media_dev *imxmd;
> + int ret;
> +
> + imxmd = devm_kzalloc(dev, sizeof(*imxmd), GFP_KERNEL);
> + if (!imxmd)
> + return ERR_PTR(-ENOMEM);
> +
> + dev_set_drvdata(dev, imxmd);
> +
> + strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model));
> + imxmd->md.ops = &imx_media_md_ops;
> + imxmd->md.dev = dev;
> +
> + mutex_init(&imxmd->mutex);
> +
> + imxmd->v4l2_dev.mdev = &imxmd->md;
> + strlcpy(imxmd->v4l2_dev.name, "imx-media",
> + sizeof(imxmd->v4l2_dev.name));
> +
> + media_device_init(&imxmd->md);
> +
> + ret = v4l2_device_register(dev, &imxmd->v4l2_dev);
> + if (ret < 0) {
> + v4l2_err(&imxmd->v4l2_dev,
> +  "Failed to register v4l2_device: %d\n", ret);
> + goto cleanup;
> + }
> +
> + dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
> +
> + INIT_LIST_HEAD(&imxmd->vdev_list);
> +
> + v4l2_async_notifier_init(&imxmd->notifier);
> +
> + return imxmd;
> +
> +cleanup:
> + media_device_cleanup(&imxmd->md);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(imx_media_dev_init);
> +
> +int imx_media_dev_notifier_register(struct imx_media_dev *imxmd)
> +{
> + int ret;
> +
> + /* no subdevs? just bail */
> + if (list_empty(&imxmd->notifier.asd_list)) {
> + v4l2_err(&imxmd->v4l2_dev, "no subdevs\n");
> + return -ENODEV;
> + }
> +
> + /* prepare the async subdev notifier and register it */
> + imxmd->notifier.ops = &imx_media_subdev_ops;
> + ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
> +&imxmd->notifier);
> + if (ret) {
> + v4l2_err(&imxmd->v4l2_dev,
> +  "v4l2_async_notifier_register failed with %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_dev_notifier_register);
> +
> +void imx_media_dev_cleanup(struct imx_media_dev *imxmd)
> +{
> + v4l2_device_unregister(&imxmd->v4l2_dev);
> + media_device_cleanup(&imxmd->md);
> +}
> +EXPORT_SYMBOL_GPL(imx_media_dev_cleanup);
> +
> +void imx_media_dev_notifier_unregister(struct imx_media_dev *imxmd)
> +{
> + v4l2_async_notifier_cleanup(&imxmd->notifier);

Is the naming of the function intentional? I'd say adding an oddly named
wrapper for v4l2_async_notifier_cleanup() hardly makes the code more
readable. I'd just call the function directly instead; same for
imx_media_dev_cleanup().

> +}
> +EXPORT_SYMBOL_GPL(imx_media_dev_notifier_unregister);
> diff --git a/drivers/staging/media/imx/imx-media-dev.c 
> b/drivers/staging/media/imx/imx-media-dev.c
> index 4b344a4a3706..21f65af5c738 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +

Re: [PATCH v10 12/13] media: video-mux: add bayer formats

2019-01-24 Thread Philipp Zabel
On Wed, 2019-01-23 at 10:52 +, Rui Miguel Silva wrote:
> Add non vendor bayer formats to the  allowed format array.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  drivers/media/platform/video-mux.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/platform/video-mux.c 
> b/drivers/media/platform/video-mux.c
> index c33900e3c23e..0ba30756e1e4 100644
> --- a/drivers/media/platform/video-mux.c
> +++ b/drivers/media/platform/video-mux.c
> @@ -263,6 +263,26 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
>   case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
>   case MEDIA_BUS_FMT_JPEG_1X8:
>   case MEDIA_BUS_FMT_AHSV_1X32:
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + case MEDIA_BUS_FMT_SGBRG8_1X8:
> + case MEDIA_BUS_FMT_SGRBG8_1X8:
> + case MEDIA_BUS_FMT_SRGGB8_1X8:
> + case MEDIA_BUS_FMT_SBGGR10_1X10:
> + case MEDIA_BUS_FMT_SGBRG10_1X10:
> + case MEDIA_BUS_FMT_SGRBG10_1X10:
> + case MEDIA_BUS_FMT_SRGGB10_1X10:
> + case MEDIA_BUS_FMT_SBGGR12_1X12:
> + case MEDIA_BUS_FMT_SGBRG12_1X12:
> + case MEDIA_BUS_FMT_SGRBG12_1X12:
> + case MEDIA_BUS_FMT_SRGGB12_1X12:
> + case MEDIA_BUS_FMT_SBGGR14_1X14:
> + case MEDIA_BUS_FMT_SGBRG14_1X14:
> + case MEDIA_BUS_FMT_SGRBG14_1X14:
> + case MEDIA_BUS_FMT_SRGGB14_1X14:
> + case MEDIA_BUS_FMT_SBGGR16_1X16:
> + case MEDIA_BUS_FMT_SGBRG16_1X16:
> + case MEDIA_BUS_FMT_SGRBG16_1X16:
> + case MEDIA_BUS_FMT_SRGGB16_1X16:
>   break;
>   default:
>   sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;

This could be merged independently from the other changes.

Reviewed-by: Philipp Zabel 

regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls

2019-01-24 Thread Ayaka


Sent from my iPad

> On Jan 24, 2019, at 6:36 PM, Paul Kocialkowski 
>  wrote:
> 
> Hi,
> 
>> On Tue, 2019-01-08 at 18:00 +0800, Ayaka wrote:
>> 
>> Sent from my iPad
>> 
>>> On Jan 8, 2019, at 4:38 PM, Paul Kocialkowski 
>>>  wrote:
>>> 
>>> Hi,
>>> 
 On Tue, 2019-01-08 at 09:16 +0800, Ayaka wrote:
 
 Sent from my iPad
 
> On Jan 7, 2019, at 5:57 PM, Paul Kocialkowski 
>  wrote:
> 
> Hi,
> 
>>> On Mon, 2019-01-07 at 11:49 +0800, Randy Li wrote:
>>> On 12/12/18 8:51 PM, Paul Kocialkowski wrote:
>>> Hi,
>>> 
>>> On Wed, 2018-12-05 at 21:59 +0100, Jernej Škrabec wrote:
>>> 
> +
> +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE0x01
> +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER0x02
> +#define V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR0x03
> +
> +#define V4L2_HEVC_DPB_ENTRIES_NUM_MAX16
> +
> +struct v4l2_hevc_dpb_entry {
> +__u32buffer_tag;
> +__u8rps;
> +__u8field_pic;
> +__u16pic_order_cnt[2];
> +};
>> 
>> Please add a property for reference index, if that rps is not used for 
>> this, some device would request that(not the rockchip one). And 
>> Rockchip's VDPU1 and VDPU2 for AVC would request a similar property.
> 
> What exactly is that reference index? Is it a bitstream element or
> something deduced from the bitstream?
> 
 picture order count(POC) for HEVC and frame_num in AVC. I think it is
 the number used in list0(P slice and B slice) and list1(B slice).
>>> 
>>> The picture order count is already the last field of the DPB entry
>>> structure. There is one for each field picture.
>> As we are not sure whether there is a field coded slice or CTU, I
>> would hold this part and else about the field.
> 
> I'm not sure what you meant here, sorry.
As we talked in IRC, I am not sure the field coded picture is supported in HEVC.
And I don’t why there would be two pic order cnt, a picture can only be used a 
short term or  a long term reference at one picture decoding
> 
>> Adding another buffer_tag for referring the memory of the motion vectors 
>> for each frames. Or a better method is add a meta data to echo picture 
>> buffer,  since the picture output is just the same as the original, 
>> display won't care whether the motion vectors are written the button of 
>> picture or somewhere else.
> 
> The motion vectors are passed as part of the raw bitstream data, in the
> slices. Is there a case where the motion vectors are coded differently?
 No, it is an additional cache for decoder, even FFmpeg having such
 data, I think allwinner must output it into somewhere.
>>> 
>>> Ah yes I see what you mean! This is handled internally by our driver
>>> and not exposed to userspace. I don't think it would be a good idea to
>>> expose this cache or request that userspace allocates it like a video
>>> buffer.
>>> 
>> No, usually the driver should allocate, as the user space have no
>> idea on size of each devices.
>> But for advantage user, application can fix a broken picture with a
>> proper data or analysis a object motion from that.
>> So I would suggest attaching this information to a picture buffer as
>> a meta data. 
> 
> Right, the driver will allocate chunks of memory for the decoding
> metadata used by the hardware decoder.
> 
> Well, I don't think V4L2 has any mechanism to expose this data for now
> and since it's very specific to the hardware implementation, I guess
> the interest in having that is generally pretty low.
> 
> That's maybe something that could be added later if someone wants to
> work on it, but I think we are better off keeping this metadata hidden
> by the driver for now.
I am writing a V4l2 driver for rockchip based on the previous vendor driver I 
sent to mail list. I think I would offer a better way to describe the meta 
after that. But it need both work in derives and userspace, it would cost some 
times.
> 
> +
> +struct v4l2_hevc_pred_weight_table {
> +__u8luma_log2_weight_denom;
> +__s8delta_chroma_log2_weight_denom;
> +
> +__s8delta_luma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> +__s8luma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> +__s8delta_chroma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> +__s8chroma_offset_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> +
> +__s8delta_luma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> +__s8luma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX];
> +__s8delta_chroma_weight_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> +__s8chroma_offset_l1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX][2];
> +};
> +
>> Those properties I think are not necessary are applying for the 
>> Rockchip's dev

Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls

2019-01-24 Thread Ayaka


Sent from my iPad

> On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski 
>  wrote:
> 
> Hi,
> 
>> On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote:
>> I forget a important thing, for the rkvdec and rk hevc decoder, it would 
>> requests cabac table, scaling list, picture parameter set and reference 
>> picture storing in one or various of DMA buffers. I am not talking about 
>> the data been parsed, the decoder would requests a raw data.
>> 
>> For the pps and rps, it is possible to reuse the slice header, just let 
>> the decoder know the offset from the bitstream bufer, I would suggest to 
>> add three properties(with sps) for them. But I think we need a method to 
>> mark a OUTPUT side buffer for those aux data.
> 
> I'm quite confused about the hardware implementation then. From what
> you're saying, it seems that it takes the raw bitstream elements rather
> than parsed elements. Is it really a stateless implementation?
> 
> The stateless implementation was designed with the idea that only the
> raw slice data should be passed in bitstream form to the decoder. For
> H.264, it seems that some decoders also need the slice header in raw
> bitstream form (because they take the full slice NAL unit), see the
> discussions in this thread:
> media: docs-rst: Document m2m stateless video decoder interface

Stateless just mean it won’t track the previous result, but I don’t think you 
can define what a date the hardware would need. Even you just build a dpb for 
the decoder, it is still stateless, but parsing less or more data from the 
bitstream doesn’t stop a decoder become a stateless decoder.
> 
> Can you detail exactly what the rockchip decoder absolutely needs in
> raw bitstream format?
> 
> Cheers,
> 
> Paul
> 
>>> On 1/8/19 6:00 PM, Ayaka wrote:
>>> Sent from my iPad
>>> 
 On Jan 8, 2019, at 4:38 PM, Paul Kocialkowski 
  wrote:
 
 Hi,
 
> On Tue, 2019-01-08 at 09:16 +0800, Ayaka wrote:
> 
> Sent from my iPad
> 
>> On Jan 7, 2019, at 5:57 PM, Paul Kocialkowski 
>>  wrote:
>> 
>> Hi,
>> 
 On Mon, 2019-01-07 at 11:49 +0800, Randy Li wrote:
 On 12/12/18 8:51 PM, Paul Kocialkowski wrote:
 Hi,
 
 On Wed, 2018-12-05 at 21:59 +0100, Jernej Škrabec wrote:
 
>> +
>> +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_BEFORE0x01
>> +#define V4L2_HEVC_DPB_ENTRY_RPS_ST_CURR_AFTER0x02
>> +#define V4L2_HEVC_DPB_ENTRY_RPS_LT_CURR0x03
>> +
>> +#define V4L2_HEVC_DPB_ENTRIES_NUM_MAX16
>> +
>> +struct v4l2_hevc_dpb_entry {
>> +__u32buffer_tag;
>> +__u8rps;
>> +__u8field_pic;
>> +__u16pic_order_cnt[2];
>> +};
>>> Please add a property for reference index, if that rps is not used for
>>> this, some device would request that(not the rockchip one). And
>>> Rockchip's VDPU1 and VDPU2 for AVC would request a similar property.
>> What exactly is that reference index? Is it a bitstream element or
>> something deduced from the bitstream?
>> 
> picture order count(POC) for HEVC and frame_num in AVC. I think it is
> the number used in list0(P slice and B slice) and list1(B slice).
 The picture order count is already the last field of the DPB entry
 structure. There is one for each field picture.
>>> As we are not sure whether there is a field coded slice or CTU, I would 
>>> hold this part and else about the field.
>>> Adding another buffer_tag for referring the memory of the motion vectors
>>> for each frames. Or a better method is add a meta data to echo picture
>>> buffer,  since the picture output is just the same as the original,
>>> display won't care whether the motion vectors are written the button of
>>> picture or somewhere else.
>> The motion vectors are passed as part of the raw bitstream data, in the
>> slices. Is there a case where the motion vectors are coded differently?
> No, it is an additional cache for decoder, even FFmpeg having such
> data, I think allwinner must output it into somewhere.
 Ah yes I see what you mean! This is handled internally by our driver
 and not exposed to userspace. I don't think it would be a good idea to
 expose this cache or request that userspace allocates it like a video
 buffer.
 
>>> No, usually the driver should allocate, as the user space have no idea on 
>>> size of each devices.
>>> But for advantage user, application can fix a broken picture with a proper 
>>> data or analysis a object motion from that.
>>> So I would suggest attaching this information to a picture buffer as a meta 
>>> data.
>> +
>> +struct v4l2_hevc_pred_weight_table {
>> +__u8luma_log2_weight_denom;
>> +__s8delta_chroma_log2_weight_denom;
>> +
>> +__s8delta_luma_weight_l0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX

Re: [PATCH 2/2] PCI: hv: Refactor hv_irq_unmask() to use cpumask_to_vpset()

2019-01-24 Thread Dan Carpenter
On Wed, Jan 23, 2019 at 01:02:12PM -0800, Maya Nakamura wrote:
> @@ -908,12 +906,12 @@ static void hv_irq_unmask(struct irq_data *data)
>   struct retarget_msi_interrupt *params;
>   struct hv_pcibus_device *hbus;
>   struct cpumask *dest;
> + cpumask_var_t tmp;
>   struct pci_bus *pbus;
>   struct pci_dev *pdev;
>   unsigned long flags;
>   u32 var_size = 0;
> - int cpu_vmbus;
> - int cpu;
> + int cpu, nr_bank = 0;
^
No need to initialize this to a bogus value.  It's misleading and it
turns off GCC's uninitialized variable warning so it can lead to bugs.

>   u64 res;
>  
>   dest = irq_data_get_effective_affinity_mask(data);
> @@ -953,29 +951,28 @@ static void hv_irq_unmask(struct irq_data *data)
>*/
>   params->int_target.flags |=
>   HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
> - params->int_target.vp_set.valid_bank_mask =
> - (1ull << HV_VP_SET_BANK_COUNT_MAX) - 1;
> +
> + if (!alloc_cpumask_var(&tmp, GFP_KERNEL)) {
> + dev_err(&hbus->hdev->device, "out of memory");


No need for this error message.  alloc_cpumask_var() already has better
debug messages built in.

> + return;

We can't return directly.  We need to unlock first.

> + }
> +
> + cpumask_and(tmp, dest, cpu_online_mask);
> + nr_bank = cpumask_to_vpset(¶ms->int_target.vp_set, tmp);
> + free_cpumask_var(tmp);
> +
> + if (!nr_bank) {
> + dev_err(&hbus->hdev->device, "too high CPU");

This error message is not useful.

> + res = 1;
> + goto exit_unlock;
> + }

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Revert "media: cedrus: Allow using the current dst buffer as reference"

2019-01-24 Thread Paul Kocialkowski
Hi,

On Thu, 2019-01-24 at 11:32 +0100, Greg KH wrote:
> On Thu, Jan 24, 2019 at 10:55:42AM +0100, Paul Kocialkowski wrote:
> > This reverts commit cf20ae1535eb690a87c29b9cc7af51881384e967.
> > 
> > The vb2_find_timestamp helper was modified to allow finding buffers
> > regardless of their current state in the queue. This means that we
> > no longer have to take particular care of references to the current
> > capture buffer.
> > ---
> >  drivers/staging/media/sunxi/cedrus/cedrus_dec.c   | 13 -
> >  drivers/staging/media/sunxi/cedrus/cedrus_dec.h   |  2 --
> >  drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 10 --
> >  3 files changed, 4 insertions(+), 21 deletions(-)
> 
> No signed-off-by?  :(

Woops, sorry about that. Will fix in v2!

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] Revert "media: cedrus: Allow using the current dst buffer as reference"

2019-01-24 Thread Paul Kocialkowski
This reverts commit cf20ae1535eb690a87c29b9cc7af51881384e967.

The vb2_find_timestamp helper was modified to allow finding buffers
regardless of their current state in the queue. This means that we
no longer have to take particular care of references to the current
capture buffer.

Signed-off-by: Paul Kocialkowski 
---
 drivers/staging/media/sunxi/cedrus/cedrus_dec.c   | 13 -
 drivers/staging/media/sunxi/cedrus/cedrus_dec.h   |  2 --
 drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c | 10 --
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index 2c295286766c..443fb037e1cf 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -22,19 +22,6 @@
 #include "cedrus_dec.h"
 #include "cedrus_hw.h"
 
-int cedrus_reference_index_find(struct vb2_queue *queue,
-   struct vb2_buffer *vb2_buf, u64 timestamp)
-{
-   /*
-* Allow using the current capture buffer as reference, which can occur
-* for field-coded pictures.
-*/
-   if (vb2_buf->timestamp == timestamp)
-   return vb2_buf->index;
-   else
-   return vb2_find_timestamp(queue, timestamp, 0);
-}
-
 void cedrus_device_run(void *priv)
 {
struct cedrus_ctx *ctx = priv;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.h 
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
index 8d0fc248220f..d1ae7903677b 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.h
@@ -16,8 +16,6 @@
 #ifndef _CEDRUS_DEC_H_
 #define _CEDRUS_DEC_H_
 
-int cedrus_reference_index_find(struct vb2_queue *queue,
-   struct vb2_buffer *vb2_buf, u64 timestamp);
 void cedrus_device_run(void *priv);
 
 #endif
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c 
b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 81c66a8aa1ac..cb45fda9aaeb 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -10,7 +10,6 @@
 #include 
 
 #include "cedrus.h"
-#include "cedrus_dec.h"
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
@@ -160,8 +159,8 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
/* Forward and backward prediction reference buffers. */
-   forward_idx = cedrus_reference_index_find(cap_q, &run->dst->vb2_buf,
- slice_params->forward_ref_ts);
+   forward_idx = vb2_find_timestamp(cap_q,
+slice_params->forward_ref_ts, 0);
 
fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
@@ -169,9 +168,8 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, 
struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
 
-   backward_idx = cedrus_reference_index_find(cap_q, &run->dst->vb2_buf,
-  
slice_params->backward_ref_ts);
-
+   backward_idx = vb2_find_timestamp(cap_q,
+ slice_params->backward_ref_ts, 0);
bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1);
 
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 2/2] media: cedrus: Add HEVC/H.265 decoding support

2019-01-24 Thread Paul Kocialkowski
Hi,

On Tue, 2018-11-27 at 09:21 +0100, Maxime Ripard wrote:
> Hi!
> 
> On Fri, Nov 23, 2018 at 02:02:09PM +0100, Paul Kocialkowski wrote:
> > This introduces support for HEVC/H.265 to the Cedrus VPU driver, with
> > both uni-directional and bi-directional prediction modes supported.
> > 
> > Field-coded (interlaced) pictures, custom quantization matrices and
> > 10-bit output are not supported at this point.
> > 
> > Signed-off-by: Paul Kocialkowski 
> 
> Output from checkpatch:
> total: 0 errors, 68 warnings, 14 checks, 999 lines checked

Looks like many of the "line over 80 chars" are due to macros. I don't
think it would be a good idea to break them down or to change the
macros names since they are directly inherited from the bitstream
elements.

What do you think?

> > +/*
> > + * Note: Neighbor info buffer size is apparently doubled for H6, which may 
> > be
> > + * related to 10 bit H265 support.
> > + */
> > +#define CEDRUS_H265_NEIGHBOR_INFO_BUF_SIZE (397 * SZ_1K)
> > +#define CEDRUS_H265_ENTRY_POINTS_BUF_SIZE  (4 * SZ_1K)
> > +#define CEDRUS_H265_MV_COL_BUF_UNIT_CTB_SIZE   160
> 
> Having some information on where this is coming from would be useful.

Yes, definitely.

> > +static void cedrus_h265_sram_write_data(struct cedrus_dev *dev, u32 *data,
> 
> Since the data pointer is pretty much an opaque structure, you should
> have a void pointer here, that would avoid the type casting you're
> doing when calling that function.

Sure, that would make more sense.

[...]

> > +   /* Output frame. */
> > +
> > +   output_pic_list_index = V4L2_HEVC_DPB_ENTRIES_NUM_MAX;
> > +   pic_order_cnt[0] = pic_order_cnt[1] = slice_params->slice_pic_order_cnt;
> > +   mv_col_buf_addr[0] = cedrus_h265_frame_info_mv_col_buf_addr(ctx,
> > +   run->dst->vb2_buf.index, 0) - PHYS_OFFSET;
> > +   mv_col_buf_addr[1] = cedrus_h265_frame_info_mv_col_buf_addr(ctx,
> > +   run->dst->vb2_buf.index, 1) - PHYS_OFFSET;
> > +   dst_luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 0) -
> > +   PHYS_OFFSET;
> > +   dst_chroma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 1) -
> > +   PHYS_OFFSET;
> > +
> > +   cedrus_h265_frame_info_write_single(dev, output_pic_list_index,
> > +   slice_params->pic_struct != 0,
> > +   pic_order_cnt, mv_col_buf_addr,
> > +   dst_luma_addr, dst_chroma_addr);
> 
> You can only pass the run and slice_params pointers to that function.

The point is to make it independent from the context, so that the same
function can be called with either the slice_params or the dpb info.
I don't think making two variants or even two wrappers would bring any
significant benefit.

> > +
> > +   cedrus_write(dev, VE_DEC_H265_OUTPUT_FRAME_IDX, output_pic_list_index);
> > +
> > +   /* Reference picture list 0 (for P/B frames). */
> > +   if (slice_params->slice_type != V4L2_HEVC_SLICE_TYPE_I) {
> > +   cedrus_h265_ref_pic_list_write(dev, slice_params->ref_idx_l0,
> > +   slice_params->num_ref_idx_l0_active_minus1 + 1,
> > +   slice_params->dpb, slice_params->num_active_dpb_entries,
> > +   VE_DEC_H265_SRAM_OFFSET_REF_PIC_LIST0);
> > +
> 
> slice_params is enough.

The rationale is similar to the one above: being able to use the same
helper with either L0 or L1, which implies passing the relevant
elements directly.

> > +   if (pps->weighted_pred_flag || pps->weighted_bipred_flag)
> > +   cedrus_h265_pred_weight_write(dev,
> > +   pred_weight_table->delta_luma_weight_l0,
> > +   pred_weight_table->luma_offset_l0,
> > +   pred_weight_table->delta_chroma_weight_l0,
> > +   pred_weight_table->chroma_offset_l0,
> > +   slice_params->num_ref_idx_l0_active_minus1 + 1,
> > +   VE_DEC_H265_SRAM_OFFSET_PRED_WEIGHT_LUMA_L0,
> > +   VE_DEC_H265_SRAM_OFFSET_PRED_WEIGHT_CHROMA_L0);
> 
> Ditto, that function should only take the pred_weight_table and
> slice_params pointers

And same rational as well.

> > +   }
> > +
> > +   /* Reference picture list 1 (for B frames). */
> > +   if (slice_params->slice_type == V4L2_HEVC_SLICE_TYPE_B) {
> > +   cedrus_h265_ref_pic_list_write(dev, slice_params->ref_idx_l1,
> > +   slice_params->num_ref_idx_l1_active_minus1 + 1,
> > +   slice_params->dpb,
> > +   slice_params->num_active_dpb_entries,
> > +   VE_DEC_H265_SRAM_OFFSET_REF_PIC_LIST1);
> > +
> > +   if (pps->weighted_bipred_flag)
> > +   cedrus_h265_pred_weight_write(dev,
> > +   pred_weight_table->delta_luma_weight_l1,
> > +   pred_weight_table->luma_offset_l1,
> > +  

Re: [PATCH 1/2] PCI: hv: Replace hv_vp_set with hv_vpset

2019-01-24 Thread Vitaly Kuznetsov
Maya Nakamura  writes:

> Remove a duplicate definition of VP set (hv_vp_set) and use the common
> definition (hv_vpset) that is used in other places.
>
> Signed-off-by: Maya Nakamura 
> ---
>  drivers/pci/controller/pci-hyperv.c | 25 -
>  1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c 
> b/drivers/pci/controller/pci-hyperv.c
> index 9ba4d12c179c..da8b58d8630d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -393,12 +393,6 @@ struct hv_interrupt_entry {
>  
>  #define HV_VP_SET_BANK_COUNT_MAX 5 /* current implementation limit */
>  
> -struct hv_vp_set {
> - u64 format; /* 0 (HvGenericSetSparse4k) */
> - u64 valid_banks;
> - u64 masks[HV_VP_SET_BANK_COUNT_MAX];
> -};
> -
>  /*
>   * flags for hv_device_interrupt_target.flags
>   */
> @@ -410,7 +404,7 @@ struct hv_device_interrupt_target {
>   u32 flags;
>   union {
>   u64  vp_mask;
> - struct hv_vp_set vp_set;
> + struct hv_vpset vp_set;
>   };
>  };
>  
> @@ -460,12 +454,16 @@ struct hv_pcibus_device {
>   struct msi_controller msi_chip;
>   struct irq_domain *irq_domain;
>  
> - /* hypercall arg, must not cross page boundary */
> - struct retarget_msi_interrupt retarget_msi_interrupt_params;
> -
>   spinlock_t retarget_msi_interrupt_lock;
>  
>   struct workqueue_struct *wq;
> +
> + /* hypercall arg, must not cross page boundary */
> + struct retarget_msi_interrupt retarget_msi_interrupt_params;
> +
> + /*
> +  * Don't put anything here: retarget_msi_interrupt_params must be last
> +  */


This change seems to be unrelated and not anyhow described in the commit
message - or did I miss something?

>  };
>  
>  /*
> @@ -955,12 +953,13 @@ static void hv_irq_unmask(struct irq_data *data)
>*/
>   params->int_target.flags |=
>   HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
> - params->int_target.vp_set.valid_banks =
> + params->int_target.vp_set.valid_bank_mask =
>   (1ull << HV_VP_SET_BANK_COUNT_MAX) - 1;
>  
>   /*
>* var-sized hypercall, var-size starts after vp_mask (thus
> -  * vp_set.format does not count, but vp_set.valid_banks does).
> +  * vp_set.format does not count, but vp_set.valid_bank_mask
> +  * does).
>*/
>   var_size = 1 + HV_VP_SET_BANK_COUNT_MAX;
>  
> @@ -974,7 +973,7 @@ static void hv_irq_unmask(struct irq_data *data)
>   goto exit_unlock;
>   }
>  
> - params->int_target.vp_set.masks[cpu_vmbus / 64] |=
> + params->int_target.vp_set.bank_contents[cpu_vmbus / 64] 
> |=
>   (1ULL << (cpu_vmbus & 63));
>   }
>   } else {

-- 
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: android: ion: Allocate from heap ID directly without mask

2019-01-24 Thread Brian Starkey
Hi Andrew,

On Wed, Jan 23, 2019 at 01:28:35PM -0600, Andrew F. Davis wrote:
> Previously the heap to allocate from was selected by a mask of allowed
> heap types. This may have been done as a primitive form of constraint
> solving, the first heap type that matched any set bit of the heap mask
> was allocated from, unless that heap was excluded by having its heap
> ID bit not set in the separate passed in heap ID mask.

I checked our gralloc and it's not using the current "keep trying
until one succeeds" functionality, but it might be worth explicitly
calling out in the commit message that this will also disappear with
the new API. Maybe someone cares about that.

> 
> The heap type does not really represent constraints that should be
> matched against to begin with. So the patch [0] removed the the heap type
> mask matching but unfortunately left the heap ID mask check (possibly by
> mistake or to preserve API). Therefor we now only have a mask of heap
> IDs, but heap IDs are unique identifiers and have nothing to do with the
> underling heap, so mask matching is not useful here. This also limits us

s/underling/underlying/

> to 32 heaps total in a system.
> 
> With the heap query API users can find the right heap based on type or
> name themselves then just supply the ID for that heap. Remove heap ID
> mask and allow users to specify heap ID directly by its number.
> 
> I know this is an ABI break, but we are in staging so lets get this over
> with now rather than limit ourselves later.

What do you think about renaming ion_allocation_data.unused to heap_id
and adding a flag instead? It's adding cruft to a staging API, but it
might soften the transition. The "old way" could get completely
removed just before destaging. Just a thought.

> 
> [0] 2bb9f5034ec7 ("gpu: ion: Remove heapmask from client")
> 
> Signed-off-by: Andrew F. Davis 
> ---
> 
> This also means we don't need to store the available heaps in a plist,
> we only operation we care about is lookup, so a better data structure
> should be chosen at some point, regular list or xarray maybe?
> 
> This is base on -next as to be on top of the other taken Ion patches.
> 
>  drivers/staging/android/ion/ion.c  | 21 ++---
>  drivers/staging/android/uapi/ion.h |  6 ++
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index 92c2914239e3..06dd5bb10ecb 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -387,7 +387,7 @@ static const struct dma_buf_ops dma_buf_ops = {
>   .unmap = ion_dma_buf_kunmap,
>  };
>  
> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int 
> flags)
> +static int ion_alloc(size_t len, unsigned int heap_id, unsigned int flags)
>  {
>   struct ion_device *dev = internal_dev;
>   struct ion_buffer *buffer = NULL;
> @@ -396,23 +396,22 @@ static int ion_alloc(size_t len, unsigned int 
> heap_id_mask, unsigned int flags)
>   int fd;
>   struct dma_buf *dmabuf;
>  
> - pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
> -  len, heap_id_mask, flags);
> - /*
> -  * traverse the list of heaps available in this system in priority
> -  * order.  If the heap type is supported by the client, and matches the
> -  * request of the caller allocate from it.  Repeat until allocate has
> -  * succeeded or all heaps have been tried
> -  */
> + pr_debug("%s: len %zu heap_id %u flags %x\n", __func__,
> +  len, heap_id, flags);
> +
>   len = PAGE_ALIGN(len);
>  
>   if (!len)
>   return -EINVAL;
>  
> + /*
> +  * Traverse the list of heaps available in this system.  If the
> +  * heap id matches the request of the caller allocate from it.
> +  */
>   down_read(&dev->lock);
>   plist_for_each_entry(heap, &dev->heaps, node) {
>   /* if the caller didn't specify this heap id */
> - if (!((1 << heap->id) & heap_id_mask))
> + if (heap->id != heap_id)
>   continue;
>   buffer = ion_buffer_create(heap, dev, len, flags);
>   if (!IS_ERR(buffer))

You can break unconditionally here now, though it might be neater to
refactor to:

 if (heap matches) {
buffer = alloc();
break;
 }

> @@ -541,7 +540,7 @@ static long ion_ioctl(struct file *filp, unsigned int 
> cmd, unsigned long arg)
>   int fd;
>  
>   fd = ion_alloc(data.allocation.len,
> -data.allocation.heap_id_mask,
> +data.allocation.heap_id,
>  data.allocation.flags);
>   if (fd < 0)
>   return fd;
> diff --git a/drivers/staging/android/uapi/ion.h 
> b/drivers/staging/android/uapi/ion.h
> index 5d7009884c13..6a78a1e23251 100644
> --- a/drivers/staging/android/uapi/ion.h
> ++

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

2019-01-24 Thread Andrew F. Davis
On 1/23/19 11:11 AM, Brian Starkey wrote:
> Hi Andrew,
> 
> On Wed, Jan 23, 2019 at 10:51:24AM -0600, Andrew F. Davis wrote:
>> On 1/22/19 11:33 AM, Sumit Semwal wrote:
>>> Hello everyone,
>>>
>>> Sincere apologies for chiming in a bit late here, but was off due to
>>> some health issues.
>>>
>>
>> Hope you are feeling better friend :)
>>
>> Looks like this email was a bit broken and you replied again, the
>> responses are a little different in each email, so I'd like to respond
>> to bits of both, I'll fix up the formatting.
>>
>>> Also, adding Daniel Vetter to the mix, since he has been one of the
>>> core guys who shaped up dma-buf as it is today.
>>>
>>> On Tue, 22 Jan 2019 at 02:51, Andrew F. Davis  wrote:

 On 1/21/19 5:22 AM, Brian Starkey wrote:
>>
>> [snip]
>>
>
> Actually I meant in the kernel, in exporters. I haven't seen anyone
> using the API as it was intended (defer allocation until first map,
> migrate between different attachments, etc.). Mostly, backing storage
> seems to get allocated at the point of export, and device mappings are
> often held persistently (e.g. the DRM prime code maps the buffer at
> import time, and keeps it mapped: drm_gem_prime_import_dev).
>

>>>
>>> So I suppose some clarification on the 'intended use' part of dma-buf
>>> about deferred allocation is due, so here it is: (Daniel, please feel
>>> free to chime in with your opinion here)
>>>
>>>  - dma-buf was of course designed as a framework to help intelligent
>>> exporters to defer allocation until first map, and be able to migrate
>>> backing storage if required etc. At the same time, it is not a
>>> _requirement_ from any exporter, so exporters so far have just used it
>>> as a convenient mechanism for zero-copy.
>>> - ION is one of the few dma-buf exporters in kernel, which satisfies a
>>> certain set of expectations from its users.
>>>
>>
>> The issue here is that Ion is blocking the ability to late allocate, it
>> expects its heaps to have the memory ready at allocation time. My point
>> being if the DMA-BUFs intended design was to allow this then Ion should
>> respect that and also allow the same from its heap exporters.
>>
 I haven't either, which is a shame as it allows for some really useful
 management strategies for shared memory resources. I'm working on one
 such case right now, maybe I'll get to be the first to upstream one :)

>>> That will be a really good thing! Though perhaps we ought to think if
>>> for what you're trying to do, is ION the right place, or should you
>>> have a device-specific exporter, available to users via dma-buf apis?
>>>
>>
>> I'm starting to question if Ion is the right place myself..
>>
>> At a conceptual level I don't believe userspace should be picking the
>> backing memory type. This is because the right type of backing memory
>> for a task will change from system to system. The kernel should abstract
>> away these hardware differences from userspace as much as it can to
>> allow portable code.
>>
>> For instance a device may need a contiguous buffer on one system but the
>> same device on another may have some IOMMU. So which type of memory do
>> we allocate? Same issue for cacheability and other properties.
>>
>> What we need is a central allocator with full system knowledge to do the
>> choosing for us. It seems many agree with the above and I take
>> inspiration from your cenalloc patchset. The thing I'm not sure about is
>> letting the device drivers set their constraints, because they also
>> don't have the full system integration details. For cases where devices
>> are behind an IOMMU it is easy enough for the device to know, but what
>> about when we have external MMUs out on the bus for anyone to use (I'm
>> guessing you remember TILER..).
>>
>> I would propose the central allocator keep per-system knowledge (or
>> fetch it from DT, or if this is considered policy then userspace) which
>> it can use to directly check the attached devices and pick the right memory.
>>
>> Anyway the central system allocator could handle 90% of cases I can
>> think of, and this is where Ion comes back in, the other cases would
>> still require the program to manually pick the right memory (maybe for
>> performance reasons, etc.).
>>
>> So my vision is to have Ion as the the main front-end for DMA-BUF
>> allocations, and expose the central allocator through it (maybe as a
>> default heap type that can be populated on a per-system basis), but also
>> have other individual heap types exported for the edge cases where
>> manual selection is needed like we do now.
>>
>> Hence why Ion should allow direct control of the dma_buf_ops from the
>> heaps, so we can build central allocators as Ion heaps.
>>
>> If I'm off into the weeds here and you have some other ideas I'm all ears.
>>
> 
> This is a topic I've gone around a few times. The crux of it is, as
> you know, a central allocator is Really Hard. I don't know what you've
> 

[PATCH v11 01/13] media: staging/imx: rearrange group id to take in account IPU

2019-01-24 Thread Rui Miguel Silva
Some imx system do not have IPU, so prepare the imx media drivers to
support this kind of devices. Rename the group ids to include an _IPU_
prefix, add a new group id to support systems with only a CSI without
IPU, and also rename the create internal links to make it clear that
only systems with IPU have internal subdevices.

Signed-off-by: Rui Miguel Silva 
---
 drivers/staging/media/imx/imx-ic-common.c |  6 ++---
 drivers/staging/media/imx/imx-ic-prp.c| 16 ++---
 drivers/staging/media/imx/imx-media-csi.c |  6 ++---
 drivers/staging/media/imx/imx-media-dev.c | 22 ++
 .../staging/media/imx/imx-media-internal-sd.c | 20 
 drivers/staging/media/imx/imx-media-utils.c   | 12 +-
 drivers/staging/media/imx/imx-media.h | 23 ++-
 7 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-common.c 
b/drivers/staging/media/imx/imx-ic-common.c
index cfdd4900a3be..765919487a73 100644
--- a/drivers/staging/media/imx/imx-ic-common.c
+++ b/drivers/staging/media/imx/imx-ic-common.c
@@ -41,13 +41,13 @@ static int imx_ic_probe(struct platform_device *pdev)
pdata = priv->dev->platform_data;
priv->ipu_id = pdata->ipu_id;
switch (pdata->grp_id) {
-   case IMX_MEDIA_GRP_ID_IC_PRP:
+   case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
priv->task_id = IC_TASK_PRP;
break;
-   case IMX_MEDIA_GRP_ID_IC_PRPENC:
+   case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
priv->task_id = IC_TASK_ENCODER;
break;
-   case IMX_MEDIA_GRP_ID_IC_PRPVF:
+   case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
priv->task_id = IC_TASK_VIEWFINDER;
break;
default:
diff --git a/drivers/staging/media/imx/imx-ic-prp.c 
b/drivers/staging/media/imx/imx-ic-prp.c
index 98923fc844ce..2702548f83cf 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -77,7 +77,7 @@ static int prp_start(struct prp_priv *priv)
priv->ipu = priv->md->ipu[ic_priv->ipu_id];
 
/* set IC to receive from CSI or VDI depending on source */
-   src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
+   src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC);
 
ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
 
@@ -237,8 +237,8 @@ static int prp_link_setup(struct media_entity *entity,
ret = -EBUSY;
goto out;
}
-   if (priv->sink_sd_prpenc && (remote_sd->grp_id &
-IMX_MEDIA_GRP_ID_VDIC)) {
+   if (priv->sink_sd_prpenc &&
+   (remote_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC)) {
ret = -EINVAL;
goto out;
}
@@ -259,7 +259,7 @@ static int prp_link_setup(struct media_entity *entity,
goto out;
}
if (priv->src_sd && (priv->src_sd->grp_id &
-IMX_MEDIA_GRP_ID_VDIC)) {
+IMX_MEDIA_GRP_ID_IPU_VDIC)) {
ret = -EINVAL;
goto out;
}
@@ -309,13 +309,13 @@ static int prp_link_validate(struct v4l2_subdev *sd,
return ret;
 
csi = imx_media_find_upstream_subdev(priv->md, &ic_priv->sd.entity,
-IMX_MEDIA_GRP_ID_CSI);
+IMX_MEDIA_GRP_ID_IPU_CSI);
if (IS_ERR(csi))
csi = NULL;
 
mutex_lock(&priv->lock);
 
-   if (priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC) {
+   if (priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC) {
/*
 * the ->PRPENC link cannot be enabled if the source
 * is the VDIC
@@ -334,10 +334,10 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 
if (csi) {
switch (csi->grp_id) {
-   case IMX_MEDIA_GRP_ID_CSI0:
+   case IMX_MEDIA_GRP_ID_IPU_CSI0:
priv->csi_id = 0;
break;
-   case IMX_MEDIA_GRP_ID_CSI1:
+   case IMX_MEDIA_GRP_ID_IPU_CSI1:
priv->csi_id = 1;
break;
default:
diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 4223f8d418ae..a12fa1dd989e 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1029,10 +1029,10 @@ static int csi_link_setup(struct media_entity *entity,
 
remote_sd = media_entity_to_v4l2_subdev(remote->entity);

[PATCH v11 02/13] media: dt-bindings: add bindings for i.MX7 media driver

2019-01-24 Thread Rui Miguel Silva
Add bindings documentation for i.MX7 media drivers.
The imx7 MIPI CSI2 and imx7 CMOS Sensor Interface.

Signed-off-by: Rui Miguel Silva 
Reviewed-by: Rob Herring 
Acked-by: Sakari Ailus 
---
 .../devicetree/bindings/media/imx7-csi.txt| 45 ++
 .../bindings/media/imx7-mipi-csi2.txt | 90 +++
 2 files changed, 135 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/imx7-csi.txt
 create mode 100644 Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt

diff --git a/Documentation/devicetree/bindings/media/imx7-csi.txt 
b/Documentation/devicetree/bindings/media/imx7-csi.txt
new file mode 100644
index ..3c07bc676bc3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7-csi.txt
@@ -0,0 +1,45 @@
+Freescale i.MX7 CMOS Sensor Interface
+=
+
+csi node
+
+
+This is device node for the CMOS Sensor Interface (CSI) which enables the chip
+to connect directly to external CMOS image sensors.
+
+Required properties:
+
+- compatible: "fsl,imx7-csi";
+- reg   : base address and length of the register set for the device;
+- interrupts: should contain CSI interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names   : must contain "axi", "mclk" and "dcic" entries, matching
+ entries in the clock property;
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in:
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+In the following example a remote endpoint is a video multiplexer.
+
+example:
+
+csi: csi@3071 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+compatible = "fsl,imx7-csi";
+reg = <0x3071 0x1>;
+interrupts = ;
+clocks = <&clks IMX7D_CLK_DUMMY>,
+<&clks IMX7D_CSI_MCLK_ROOT_CLK>,
+<&clks IMX7D_CLK_DUMMY>;
+clock-names = "axi", "mclk", "dcic";
+
+port {
+csi_from_csi_mux: endpoint {
+remote-endpoint = <&csi_mux_to_csi>;
+};
+};
+};
diff --git a/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt 
b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
new file mode 100644
index ..71fd74ed3ec8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
@@ -0,0 +1,90 @@
+Freescale i.MX7 Mipi CSI2
+=
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
+compatible with previous version of Samsung D-phy.
+
+Required properties:
+
+- compatible: "fsl,imx7-mipi-csi2";
+- reg   : base address and length of the register set for the device;
+- interrupts: should contain MIPI CSIS interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
+  entries in the clock property;
+- power-domains : a phandle to the power domain, see
+  Documentation/devicetree/bindings/power/power_domain.txt for details.
+- reset-names   : should include following entry "mrst";
+- resets: a list of phandle, should contain reset entry of
+  reset-names;
+- phy-supply: from the generic phy bindings, a phandle to a regulator that
+ provides power to MIPI CSIS core;
+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
+   value when this property is not specified is 166 MHz;
+- fsl,csis-hs-settle : differential receiver (HS-RX) settle time;
+
+The device node should contain two 'port' child nodes with one child 'endpoint'
+node, according to the bindings defined in:
+ Documentation/devicetree/bindings/ media/video-interfaces.txt.
+ The following are properties specific to those nodes.
+
+port node
+-
+
+- reg: (required) can take the values 0 or 1, where 0 shall be
+ related to the sink port and port 1 shall be the source
+ one;
+
+endpoint node
+-
+
+- data-lanes: (required) an array specifying active physical MIPI-CSI2
+   data input lanes and their mapping to logical lanes; this
+shall only be applied to port 0 (sink port), the array's
+content is unused only its length is meaningful,
+in this case the maximum le

[PATCH v11 07/13] ARM: dts: imx7s: add multiplexer controls

2019-01-24 Thread Rui Miguel Silva
The IOMUXC General Purpose Register has bitfield to control video bus
multiplexer to control the CSI input between the MIPI-CSI2 and parallel
interface. Add that register and mask.

Signed-off-by: Rui Miguel Silva 
Reviewed-by: Philipp Zabel 
---
 arch/arm/boot/dts/imx7s.dtsi | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 9a680d3d6424..792efcd2caa1 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -497,8 +497,15 @@
 
gpr: iomuxc-gpr@3034 {
compatible = "fsl,imx7d-iomuxc-gpr",
-   "fsl,imx6q-iomuxc-gpr", "syscon";
+   "fsl,imx6q-iomuxc-gpr", "syscon",
+   "simple-mfd";
reg = <0x3034 0x1>;
+
+   mux: mux-controller {
+   compatible = "mmio-mux";
+   #mux-control-cells = <0>;
+   mux-reg-masks = <0x14 0x0010>;
+   };
};
 
ocotp: ocotp-ctrl@3035 {
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 10/13] media: imx7.rst: add documentation for i.MX7 media driver

2019-01-24 Thread Rui Miguel Silva
Add rst document to describe the i.MX7 media driver and also a working
example from the Warp7 board usage with a OV2680 sensor.

Signed-off-by: Rui Miguel Silva 
---
 Documentation/media/v4l-drivers/imx7.rst  | 157 ++
 Documentation/media/v4l-drivers/index.rst |   1 +
 2 files changed, 158 insertions(+)
 create mode 100644 Documentation/media/v4l-drivers/imx7.rst

diff --git a/Documentation/media/v4l-drivers/imx7.rst 
b/Documentation/media/v4l-drivers/imx7.rst
new file mode 100644
index ..cd1195d391c5
--- /dev/null
+++ b/Documentation/media/v4l-drivers/imx7.rst
@@ -0,0 +1,157 @@
+i.MX7 Video Capture Driver
+==
+
+Introduction
+
+
+The i.MX7 contrary to the i.MX5/6 family does not contain an Image Processing
+Unit (IPU); because of that the capabilities to perform operations or
+manipulation of the capture frames are less feature rich.
+
+For image capture the i.MX7 has three units:
+- CMOS Sensor Interface (CSI)
+- Video Multiplexer
+- MIPI CSI-2 Receiver
+
+::
+   |\
+   MIPI Camera Input ---> MIPI CSI-2 --- > | \
+   |  \
+   | M |
+   | U | -->  CSI ---> Capture
+   | X |
+   |  /
+   Parallel Camera Input > | /
+   |/
+
+For additional information, please refer to the latest versions of the i.MX7
+reference manual [#f1]_.
+
+Entities
+
+
+imx7-mipi-csi2
+--
+
+This is the MIPI CSI-2 receiver entity. It has one sink pad to receive the 
pixel
+data from MIPI CSI-2 camera sensor. It has one source pad, corresponding to the
+virtual channel 0. This module is compliant to previous version of Samsung
+D-phy, and supports two D-PHY Rx Data lanes.
+
+csi_mux
+---
+
+This is the video multiplexer. It has two sink pads to select from either 
camera
+sensor with a parallel interface or from MIPI CSI-2 virtual channel 0.  It has
+a single source pad that routes to the CSI.
+
+csi
+---
+
+The CSI enables the chip to connect directly to external CMOS image sensor. CSI
+can interface directly with Parallel and MIPI CSI-2 buses. It has 256 x 64 FIFO
+to store received image pixel data and embedded DMA controllers to transfer 
data
+from the FIFO through AHB bus.
+
+This entity has one sink pad that receives from the csi_mux entity and a single
+source pad that routes video frames directly to memory buffers. This pad is
+routed to a capture device node.
+
+Usage Notes
+---
+
+To aid in configuration and for backward compatibility with V4L2 applications
+that access controls only from video device nodes, the capture device 
interfaces
+inherit controls from the active entities in the current pipeline, so controls
+can be accessed either directly from the subdev or from the active capture
+device interface. For example, the sensor controls are available either from 
the
+sensor subdevs or from the active capture device.
+
+Warp7 with OV2680
+-
+
+On this platform an OV2680 MIPI CSI-2 module is connected to the internal MIPI
+CSI-2 receiver. The following example configures a video capture pipeline with
+an output of 800x600, and BGGR 10 bit bayer format:
+
+.. code-block:: none
+   # Setup links
+   media-ctl -l "'ov2680 1-0036':0 -> 'imx7-mipi-csis.0':0[1]"
+   media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi_mux':1[1]"
+   media-ctl -l "'csi_mux':2 -> 'csi':0[1]"
+   media-ctl -l "'csi':1 -> 'csi capture':0[1]"
+
+   # Configure pads for pipeline
+   media-ctl -V "'ov2680 1-0036':0 [fmt:SBGGR10_1X10/800x600 field:none]"
+   media-ctl -V "'csi_mux':1 [fmt:SBGGR10_1X10/800x600 field:none]"
+   media-ctl -V "'csi_mux':2 [fmt:SBGGR10_1X10/800x600 field:none]"
+   media-ctl -V "'imx7-mipi-csis.0':0 [fmt:SBGGR10_1X10/800x600 field:none]"
+   media-ctl -V "'csi':0 [fmt:SBGGR10_1X10/800x600 field:none]"
+
+After this streaming can start. The v4l2-ctl tool can be used to select any of
+the resolutions supported by the sensor.
+
+.. code-block:: none
+root@imx7s-warp:~# media-ctl -p
+Media controller API version 4.17.0
+
+Media device information
+
+driver  imx-media
+model   imx-media
+serial
+bus info
+hw revision 0x0
+driver version  4.17.0
+
+Device topology
+- entity 1: csi (2 pads, 2 links)
+   type V4L2 subdev subtype Unknown flags 0
+   device node name /dev/v4l-subdev0
+   pad0: Sink
+   [fmt:SBGGR10_1X10/800x600 field:none]
+   <- "csi_mux":2 [ENABLED]
+   pad1: Source
+   [fmt:SBGGR10_1X10/800x600 field:none]
+   -> "csi capture":0 [ENABLED]
+
+- entity 4: csi capture (1 pad, 1 link)
+   type Node subtype V4

[PATCH v11 06/13] ARM: dts: imx7s: add mipi phy power domain

2019-01-24 Thread Rui Miguel Silva
Add power domain index 0 related with mipi-phy to imx7s.

While at it rename pcie power-domain node to remove pgc prefix.

Signed-off-by: Rui Miguel Silva 
---
 arch/arm/boot/dts/imx7s.dtsi | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index e88f53a4c7f4..9a680d3d6424 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -606,7 +606,13 @@
#address-cells = <1>;
#size-cells = <0>;
 
-   pgc_pcie_phy: pgc-power-domain@1 {
+   pgc_mipi_phy: power-domain@0 {
+   #power-domain-cells = <0>;
+   reg = <0>;
+   power-supply = <®_1p0d>;
+   };
+
+   pgc_pcie_phy: power-domain@1 {
#power-domain-cells = <0>;
reg = <1>;
power-supply = <®_1p0d>;
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 11/13] media: staging/imx: add i.MX7 entries to TODO file

2019-01-24 Thread Rui Miguel Silva
Add some i.MX7 related entries to TODO file.

Signed-off-by: Rui Miguel Silva 
---
 drivers/staging/media/imx/TODO | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
index aeeb15494a49..6f29b5ca5324 100644
--- a/drivers/staging/media/imx/TODO
+++ b/drivers/staging/media/imx/TODO
@@ -45,3 +45,12 @@
 
  Which means a port must not contain mixed-use endpoints, they
  must all refer to media links between V4L2 subdevices.
+
+- i.MX7: all of the above, since it uses the imx media core
+
+- i.MX7: use Frame Interval Monitor
+
+- i.MX7: runtime testing with parallel sensor, links setup and streaming
+
+- i.MX7: runtime testing with different formats, for the time only 10-bit bayer
+  is tested
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 09/13] ARM: dts: imx7s-warp: add ov2680 sensor node

2019-01-24 Thread Rui Miguel Silva
Warp7 comes with a Omnivision OV2680 sensor, add the node here to make
complete the camera data path for this system. Add the needed regulator
to the analog voltage supply, the port and endpoints in mipi_csi node
and the pinctrl for the reset gpio.

Signed-off-by: Rui Miguel Silva 
---
 arch/arm/boot/dts/imx7s-warp.dts | 44 
 1 file changed, 44 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s-warp.dts b/arch/arm/boot/dts/imx7s-warp.dts
index 358bcae7ebaf..58d1a89ee3e3 100644
--- a/arch/arm/boot/dts/imx7s-warp.dts
+++ b/arch/arm/boot/dts/imx7s-warp.dts
@@ -55,6 +55,14 @@
regulator-always-on;
};
 
+   reg_peri_3p15v: regulator-peri-3p15v {
+   compatible = "regulator-fixed";
+   regulator-name = "peri_3p15v_reg";
+   regulator-min-microvolt = <315>;
+   regulator-max-microvolt = <315>;
+   regulator-always-on;
+   };
+
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "imx7-sgtl5000";
@@ -178,6 +186,27 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c2>;
status = "okay";
+
+   ov2680: camera@36 {
+   compatible = "ovti,ov2680";
+   pinctrl-names = "default";
+   pinctrl-0 = <&pinctrl_ov2680>;
+   reg = <0x36>;
+   clocks = <&osc>;
+   clock-names = "xvclk";
+   reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
+   DOVDD-supply = <&sw2_reg>;
+   DVDD-supply = <&sw2_reg>;
+   AVDD-supply = <®_peri_3p15v>;
+
+   port {
+   ov2680_to_mipi: endpoint {
+   remote-endpoint = <&mipi_from_sensor>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
 };
 
 &i2c3 {
@@ -319,6 +348,15 @@
#size-cells = <0>;
fsl,csis-hs-settle = <3>;
 
+   port@0 {
+   reg = <0>;
+
+   mipi_from_sensor: endpoint {
+   remote-endpoint = <&ov2680_to_mipi>;
+   data-lanes = <1>;
+   };
+   };
+
port@1 {
reg = <1>;
 
@@ -382,6 +420,12 @@
>;
};
 
+   pinctrl_ov2680: ov2660grp {
+   fsl,pins = <
+   MX7D_PAD_LPSR_GPIO1_IO03__GPIO1_IO3 0x14
+   >;
+   };
+
pinctrl_sai1: sai1grp {
fsl,pins = <
MX7D_PAD_SAI1_RX_DATA__SAI1_RX_DATA00x1f
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 13/13] media: MAINTAINERS: add entry for Freescale i.MX7 media driver

2019-01-24 Thread Rui Miguel Silva
Add maintainer entry for the imx7 media csi, mipi csis driver,
dt-bindings and documentation.

Signed-off-by: Rui Miguel Silva 
---
 MAINTAINERS | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 51029a425dbe..ad267b3dd18b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9350,6 +9350,17 @@ T:   git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/platform/imx-pxp.[ch]
 
+MEDIA DRIVERS FOR FREESCALE IMX7
+M: Rui Miguel Silva 
+L: linux-me...@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: Documentation/devicetree/bindings/media/imx7-csi.txt
+F: Documentation/devicetree/bindings/media/imx7-mipi-csi2.txt
+F: Documentation/media/v4l-drivers/imx7.rst
+F: drivers/staging/media/imx/imx7-media-csi.c
+F: drivers/staging/media/imx/imx7-mipi-csis.c
+
 MEDIA DRIVERS FOR HELENE
 M: Abylay Ospan 
 L: linux-me...@vger.kernel.org
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 00/13] media: staging/imx7: add i.MX7 media driver

2019-01-24 Thread Rui Miguel Silva
Hi,
This series introduces the Media driver to work with the i.MX7 SoC. it uses the
already existing imx media core drivers but since the i.MX7, contrary to
i.MX5/6, do not have an IPU and because of that some changes in the imx media
core are made along this series to make it support that case.

This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along with several
configurations changes for this to work as a capture subsystem. Some bugs are
also fixed along the line. And necessary documentation.

For a more detailed view of the capture paths, pads links in the i.MX7 please
take a look at the documentation in PATCH 10.

The system used to test and develop this was the Warp7 board with an OV2680
sensor, which output format is 10-bit bayer. So, only MIPI interface was
tested, a scenario with an parallel input would nice to have.

Bellow goes an example of the output of the pads and links and the output of
v4l2-compliance testing.

The v4l-utils version used is:
v4l2-compliance SHA: 1a6c8fe9a65c26e78ba34bd4aa2df28ede7d00cb, 32 bits

The Media Driver fail some tests but this failures are coming from code out of
scope of this series (imx-capture), and some from the sensor OV2680
but that I think not related with the sensor driver but with the testing and
core.

The csi and mipi-csi entities pass all compliance tests.

Cheers,
Rui

v10->v11:
  Sakari:
- Remove cleanup functions in dev-common and do direct calls
- Fix notifier cleanup on error path

  Philipp Zabel:
- Add reviewed tag to video mux patch 12/13

v9->v10:
  Hans:
  - move dt-bindings patch up in the series to avoid checkpatch warnings
  - Fix SPDX tag

  Sakari:
  - use debugfs and drop driver parameters
  - use dev_*() macros all over the place, drop v4l2_*() ones
  - use clk_bulk
  - give control to power state to runtime PM
  - unsigned and const for some objects

v8->v9:
Hans Verkuil:
 - Fix issues detected by checkpatch strict, still some left:
 - bigger kconfig option description
 - some alignement parenthesis that were left as they are, to be more
 readable 
 - added new patch (PATCH13) for Maintainers update
 - SPDX in documentation rst file
Sakari Ailus:
 - remove pad check in csi, this is done by core already
 - destroy mutex in probe error path (add label)
 - swap order in driver release
 - initialize endpoint in stack
 - use clk_bulk
kbuild test robot:
 - add the missing imx-media-dev-common.c in patch 1/13
 - remove OWNER of module csis
Myself:
 - add MAINTAINERS entries - new patch

v7->v8:
Myself:
 - rebase to latest linux-next (s/V4L2_MBUS_CSI2/V4L2_MBUS_CSI2_DPHY/)
 - Rebuild and test with latest v4l2-compliance
 - add Sakari reviewed-by tag to dt-bindings

v6->v7:
Myself:
 - Clock patches removed from this version since they were already merged
 - Rebuild and test with the latest v4l2-compliance
 - Add patch to video-mux regarding bayer formats
 - remove reference to dependent patch serie (was already merged)

Sakari Ailus:
 - add port and endpoint explanantions
 - fix some wording should -> shall

v5->v6:
Rob Herring:
 - rename power-domain node name from: pgc-power-domain to power-domain
 - change mux-control-cells to 0
 - remove bus-width from mipi bindings and dts
 - remove err... regarding clock names line
 - remove clk-settle from example
 - split mipi-csi2 and csi bindings per file
 - add OF graph description to CSI

Philipp Zabel:
 - rework group IDs and rename them with an _IPU_ prefix, this allowed to remove
   the ipu_present flag need.

v4->v5:
Sakari Ailus:
 - fix remove of the capture entries in dts bindings in the right patch

Stephen Boyd:
 - Send all series to clk list

v3->v4:
Philipp Zabel:
 - refactor initialization code from media device probe to be possible to used
   from other modules
 - Remove index of csi from all accurrencs (dts, code, documentation)
 - Remove need for capture node for imx7
 - fix pinctrl for ov2680
 - add reviewed tag to add multiplexer controls patch

Fabio Estevam:
 - remove always on from new regulator

Randy Dunlap:
 - several text editing fixes in documentation

Myself:
 - rebase on top of v4 of Steve series
 - change CSI probe to initialize imx media device
 - remove csi mux parallel endpoint from mux to avoid warning message

v2->v3:
Philipp Zabel:
 - use of_match_device in imx-media-dev instead of of_device_match
 - fix number of data lanes from 4 to 2
 - change the clock definitions and use of mipi
 - move hs-settle from endpoint

Rob Herring:
 - fix phy-supply description
 - add vendor properties
 - fix examples indentations

Stephen Boyd: patch 3/14
 - fix double sign-off
 - add fixes tag

Dong Aisheng: patch 3/14
 - fix double sign-off
 - add Acked-by tag

Shawn Guo:
patch 4/14
 - remove line breakage in parent redifiniton
 - added Acked-by tag

 - dropped CMA area increase and add more verbose information in case of
   dma allocation failure
patch 9/14
 - remove extra line between cells and reg masks

Myself:
 - rework on frame end in 

[PATCH v11 04/13] media: staging/imx7: add imx7 CSI subdev driver

2019-01-24 Thread Rui Miguel Silva
This add the media entity subdevice and control driver for the i.MX7
CMOS Sensor Interface.

Signed-off-by: Rui Miguel Silva 
---
 drivers/staging/media/imx/Kconfig  | 9 -
 drivers/staging/media/imx/Makefile | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/Kconfig 
b/drivers/staging/media/imx/Kconfig
index bfc17de56b17..36b276ea2ecc 100644
--- a/drivers/staging/media/imx/Kconfig
+++ b/drivers/staging/media/imx/Kconfig
@@ -11,7 +11,7 @@ config VIDEO_IMX_MEDIA
  driver for the i.MX5/6 SOC.
 
 if VIDEO_IMX_MEDIA
-menu "i.MX5/6 Media Sub devices"
+menu "i.MX5/6/7 Media Sub devices"
 
 config VIDEO_IMX_CSI
tristate "i.MX5/6 Camera Sensor Interface driver"
@@ -20,5 +20,12 @@ config VIDEO_IMX_CSI
---help---
  A video4linux camera sensor interface driver for i.MX5/6.
 
+config VIDEO_IMX7_CSI
+   tristate "i.MX7 Camera Sensor Interface driver"
+   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
+   default y
+   help
+ Enable support for video4linux camera sensor interface driver for
+ i.MX7.
 endmenu
 endif
diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index a30b3033f9a3..074f016d3519 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -12,3 +12,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
 
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
+
+obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 08/13] ARM: dts: imx7: Add video mux, csi and mipi_csi and connections

2019-01-24 Thread Rui Miguel Silva
This patch adds the device tree nodes for csi, video multiplexer and
mipi-csi besides the graph connecting the necessary endpoints to make
the media capture entities to work in imx7 Warp board.

Signed-off-by: Rui Miguel Silva 
---
 arch/arm/boot/dts/imx7s-warp.dts | 51 
 arch/arm/boot/dts/imx7s.dtsi | 27 +
 2 files changed, 78 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s-warp.dts b/arch/arm/boot/dts/imx7s-warp.dts
index 23431faecaf4..358bcae7ebaf 100644
--- a/arch/arm/boot/dts/imx7s-warp.dts
+++ b/arch/arm/boot/dts/imx7s-warp.dts
@@ -277,6 +277,57 @@
status = "okay";
 };
 
+&gpr {
+   csi_mux {
+   compatible = "video-mux";
+   mux-controls = <&mux 0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   reg = <1>;
+
+   csi_mux_from_mipi_vc0: endpoint {
+   remote-endpoint = <&mipi_vc0_to_csi_mux>;
+   };
+   };
+
+   port@2 {
+   reg = <2>;
+
+   csi_mux_to_csi: endpoint {
+   remote-endpoint = <&csi_from_csi_mux>;
+   };
+   };
+   };
+};
+
+&csi {
+   status = "okay";
+
+   port {
+   csi_from_csi_mux: endpoint {
+   remote-endpoint = <&csi_mux_to_csi>;
+   };
+   };
+};
+
+&mipi_csi {
+   clock-frequency = <16600>;
+   status = "okay";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   fsl,csis-hs-settle = <3>;
+
+   port@1 {
+   reg = <1>;
+
+   mipi_vc0_to_csi_mux: endpoint {
+   remote-endpoint = <&csi_mux_from_mipi_vc0>;
+   };
+   };
+};
+
 &wdog1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_wdog>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 792efcd2caa1..01962f85cab6 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "imx7d-pinfunc.h"
 
 / {
@@ -709,6 +710,17 @@
status = "disabled";
};
 
+   csi: csi@3071 {
+   compatible = "fsl,imx7-csi";
+   reg = <0x3071 0x1>;
+   interrupts = ;
+   clocks = <&clks IMX7D_CLK_DUMMY>,
+   <&clks IMX7D_CSI_MCLK_ROOT_CLK>,
+   <&clks IMX7D_CLK_DUMMY>;
+   clock-names = "axi", "mclk", "dcic";
+   status = "disabled";
+   };
+
lcdif: lcdif@3073 {
compatible = "fsl,imx7d-lcdif", 
"fsl,imx28-lcdif";
reg = <0x3073 0x1>;
@@ -718,6 +730,21 @@
clock-names = "pix", "axi";
status = "disabled";
};
+
+   mipi_csi: mipi-csi@3075 {
+   compatible = "fsl,imx7-mipi-csi2";
+   reg = <0x3075 0x1>;
+   interrupts = ;
+   clocks = <&clks IMX7D_IPG_ROOT_CLK>,
+   <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
+   <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
+   clock-names = "pclk", "wrap", "phy";
+   power-domains = <&pgc_mipi_phy>;
+   phy-supply = <®_1p0d>;
+   resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
+   reset-names = "mrst";
+   status = "disabled";
+   };
};
 
aips3: aips-bus@3080 {
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 05/13] media: staging/imx7: add MIPI CSI-2 receiver subdev for i.MX7

2019-01-24 Thread Rui Miguel Silva
Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI
CSI-2 interface.

Signed-off-by: Rui Miguel Silva 
---
 drivers/staging/media/imx/Makefile |1 +
 drivers/staging/media/imx/imx7-mipi-csis.c | 1184 
 2 files changed, 1185 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c

diff --git a/drivers/staging/media/imx/Makefile 
b/drivers/staging/media/imx/Makefile
index 074f016d3519..d2d909a36239 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
 
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
+obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
b/drivers/staging/media/imx/imx7-mipi-csis.c
new file mode 100644
index ..2d54fc7b20a0
--- /dev/null
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -0,0 +1,1184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver
+ *
+ * Copyright (C) 2019 Linaro Ltd
+ * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "imx-media.h"
+
+#define CSIS_DRIVER_NAME   "imx7-mipi-csis"
+#define CSIS_SUBDEV_NAME   CSIS_DRIVER_NAME
+
+#define CSIS_PAD_SINK  0
+#define CSIS_PAD_SOURCE1
+#define CSIS_PADS_NUM  2
+
+#define MIPI_CSIS_DEF_PIX_WIDTH640
+#define MIPI_CSIS_DEF_PIX_HEIGHT   480
+
+/* Register map definition */
+
+/* CSIS common control */
+#define MIPI_CSIS_CMN_CTRL 0x04
+#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW   BIT(16)
+#define MIPI_CSIS_CMN_CTRL_INTER_MODE  BIT(10)
+#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL  BIT(2)
+#define MIPI_CSIS_CMN_CTRL_RESET   BIT(1)
+#define MIPI_CSIS_CMN_CTRL_ENABLE  BIT(0)
+
+#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET  8
+#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK(3 << 8)
+
+/* CSIS clock control */
+#define MIPI_CSIS_CLK_CTRL 0x08
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)((x) << 28)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)((x) << 24)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)((x) << 20)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)((x) << 16)
+#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK  (0xf << 4)
+#define MIPI_CSIS_CLK_CTRL_WCLK_SRCBIT(0)
+
+/* CSIS Interrupt mask */
+#define MIPI_CSIS_INTMSK   0x10
+#define MIPI_CSIS_INTMSK_EVEN_BEFORE   BIT(31)
+#define MIPI_CSIS_INTMSK_EVEN_AFTERBIT(30)
+#define MIPI_CSIS_INTMSK_ODD_BEFOREBIT(29)
+#define MIPI_CSIS_INTMSK_ODD_AFTER BIT(28)
+#define MIPI_CSIS_INTMSK_FRAME_START   BIT(24)
+#define MIPI_CSIS_INTMSK_FRAME_END BIT(20)
+#define MIPI_CSIS_INTMSK_ERR_SOT_HSBIT(16)
+#define MIPI_CSIS_INTMSK_ERR_LOST_FS   BIT(12)
+#define MIPI_CSIS_INTMSK_ERR_LOST_FE   BIT(8)
+#define MIPI_CSIS_INTMSK_ERR_OVER  BIT(4)
+#define MIPI_CSIS_INTMSK_ERR_WRONG_CFG BIT(3)
+#define MIPI_CSIS_INTMSK_ERR_ECC   BIT(2)
+#define MIPI_CSIS_INTMSK_ERR_CRC   BIT(1)
+#define MIPI_CSIS_INTMSK_ERR_UNKNOWN   BIT(0)
+
+/* CSIS Interrupt source */
+#define MIPI_CSIS_INTSRC   0x14
+#define MIPI_CSIS_INTSRC_EVEN_BEFORE   BIT(31)
+#define MIPI_CSIS_INTSRC_EVEN_AFTERBIT(30)
+#define MIPI_CSIS_INTSRC_EVEN  BIT(30)
+#define MIPI_CSIS_INTSRC_ODD_BEFOREBIT(29)
+#define MIPI_CSIS_INTSRC_ODD_AFTER BIT(28)
+#define MIPI_CSIS_INTSRC_ODD   (0x3 << 28)
+#define MIPI_CSIS_INTSRC_NON_IMAGE_DATA(0xf << 28)
+#define MIPI_CSIS_INTSRC_FRAME_START   BIT(24)
+#define MIPI_CSIS_INTSRC_FRAME_END BIT(20)
+#define MIPI_CSIS_INTSRC_ERR_SOT_HSBIT(16)
+#define MIPI_CSIS_INTSRC_ERR_LOST_FS   BIT(12)
+#define MIPI_CSIS_INTSRC_ERR_LOST_FE   BIT(8)
+#define MIPI_CSIS_INTSRC_ERR_OVER  BIT(4)
+#define MIPI_CSIS_INTSRC_ERR_WRONG_CFG BIT(3)
+#define MIPI_CSIS_INTSRC_ERR_ECC   BIT(2)
+#define MIPI_CSIS_INTSRC_ERR_CRC   BIT(1)
+#define MIPI_CSIS_INTSRC_ERR_UNKNOWN   BIT(0)
+#define MIPI_CSIS_INTSRC_ERRORS0xf
+
+/* D-PHY status control */
+#define MIPI_CSIS_DPHYSTATUS   0x20
+#define MIPI_CSIS_DPHYSTATUS_ULPS_DAT  BIT(8)
+#define MIPI_CSIS_DPHYSTATUS_STOPSTATE_DAT BIT(4)
+#define MIPI_CSIS_DPHYSTATUS_ULPS_CLK  BIT(1)
+#define MIPI_CSIS_DPHYSTATUS_STOPSTATE_CLK BIT(0)
+
+/* D-PHY common control */
+#define MIPI_CSIS_DPHYCTRL 0x24
+#define MIPI_CSIS_DPHYCTRL_HSS_MASK(0xff << 24)
+#define MIPI_CSIS_DPHYCTRL_HSS_OFFSET  

[PATCH v11 12/13] media: video-mux: add bayer formats

2019-01-24 Thread Rui Miguel Silva
Add non vendor bayer formats to the  allowed format array.

Signed-off-by: Rui Miguel Silva 
Reviewed-by: Philipp Zabel 
---
 drivers/media/platform/video-mux.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/media/platform/video-mux.c 
b/drivers/media/platform/video-mux.c
index c33900e3c23e..0ba30756e1e4 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -263,6 +263,26 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
case MEDIA_BUS_FMT_JPEG_1X8:
case MEDIA_BUS_FMT_AHSV_1X32:
+   case MEDIA_BUS_FMT_SBGGR8_1X8:
+   case MEDIA_BUS_FMT_SGBRG8_1X8:
+   case MEDIA_BUS_FMT_SGRBG8_1X8:
+   case MEDIA_BUS_FMT_SRGGB8_1X8:
+   case MEDIA_BUS_FMT_SBGGR10_1X10:
+   case MEDIA_BUS_FMT_SGBRG10_1X10:
+   case MEDIA_BUS_FMT_SGRBG10_1X10:
+   case MEDIA_BUS_FMT_SRGGB10_1X10:
+   case MEDIA_BUS_FMT_SBGGR12_1X12:
+   case MEDIA_BUS_FMT_SGBRG12_1X12:
+   case MEDIA_BUS_FMT_SGRBG12_1X12:
+   case MEDIA_BUS_FMT_SRGGB12_1X12:
+   case MEDIA_BUS_FMT_SBGGR14_1X14:
+   case MEDIA_BUS_FMT_SGBRG14_1X14:
+   case MEDIA_BUS_FMT_SGRBG14_1X14:
+   case MEDIA_BUS_FMT_SRGGB14_1X14:
+   case MEDIA_BUS_FMT_SBGGR16_1X16:
+   case MEDIA_BUS_FMT_SGBRG16_1X16:
+   case MEDIA_BUS_FMT_SGRBG16_1X16:
+   case MEDIA_BUS_FMT_SRGGB16_1X16:
break;
default:
sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v11 03/13] media: staging/imx7: add imx7 CSI subdev driver

2019-01-24 Thread Rui Miguel Silva
This add the media entity subdevice and control driver for the i.MX7
CMOS Sensor Interface.

Signed-off-by: Rui Miguel Silva 
---
 drivers/staging/media/imx/imx7-media-csi.c | 1360 
 1 file changed, 1360 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx7-media-csi.c

diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
b/drivers/staging/media/imx/imx7-media-csi.c
new file mode 100644
index ..44ba008fdc12
--- /dev/null
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -0,0 +1,1360 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
+ *
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include "imx-media.h"
+
+#define IMX7_CSI_PAD_SINK  0
+#define IMX7_CSI_PAD_SRC   1
+#define IMX7_CSI_PADS_NUM  2
+
+/* reset values */
+#define CSICR1_RESET_VAL   0x4800
+#define CSICR2_RESET_VAL   0x0
+#define CSICR3_RESET_VAL   0x0
+
+/* csi control reg 1 */
+#define BIT_SWAP16_EN  BIT(31)
+#define BIT_EXT_VSYNC  BIT(30)
+#define BIT_EOF_INT_EN BIT(29)
+#define BIT_PRP_IF_EN  BIT(28)
+#define BIT_CCIR_MODE  BIT(27)
+#define BIT_COF_INT_EN BIT(26)
+#define BIT_SF_OR_INTENBIT(25)
+#define BIT_RF_OR_INTENBIT(24)
+#define BIT_SFF_DMA_DONE_INTEN  BIT(22)
+#define BIT_STATFF_INTEN   BIT(21)
+#define BIT_FB2_DMA_DONE_INTEN  BIT(20)
+#define BIT_FB1_DMA_DONE_INTEN  BIT(19)
+#define BIT_RXFF_INTEN BIT(18)
+#define BIT_SOF_POLBIT(17)
+#define BIT_SOF_INTEN  BIT(16)
+#define BIT_MCLKDIV(0xF << 12)
+#define BIT_HSYNC_POL  BIT(11)
+#define BIT_CCIR_ENBIT(10)
+#define BIT_MCLKEN BIT(9)
+#define BIT_FCCBIT(8)
+#define BIT_PACK_DIR   BIT(7)
+#define BIT_CLR_STATFIFO   BIT(6)
+#define BIT_CLR_RXFIFO BIT(5)
+#define BIT_GCLK_MODE  BIT(4)
+#define BIT_INV_DATA   BIT(3)
+#define BIT_INV_PCLK   BIT(2)
+#define BIT_REDGE  BIT(1)
+#define BIT_PIXEL_BIT  BIT(0)
+
+#define SHIFT_MCLKDIV  12
+
+/* control reg 3 */
+#define BIT_FRMCNT (0x << 16)
+#define BIT_FRMCNT_RST BIT(15)
+#define BIT_DMA_REFLASH_RFFBIT(14)
+#define BIT_DMA_REFLASH_SFFBIT(13)
+#define BIT_DMA_REQ_EN_RFF BIT(12)
+#define BIT_DMA_REQ_EN_SFF BIT(11)
+#define BIT_STATFF_LEVEL   (0x7 << 8)
+#define BIT_HRESP_ERR_EN   BIT(7)
+#define BIT_RXFF_LEVEL (0x7 << 4)
+#define BIT_TWO_8BIT_SENSORBIT(3)
+#define BIT_ZERO_PACK_EN   BIT(2)
+#define BIT_ECC_INT_EN BIT(1)
+#define BIT_ECC_AUTO_ENBIT(0)
+
+#define SHIFT_FRMCNT   16
+#define SHIFT_RXFIFO_LEVEL 4
+
+/* csi status reg */
+#define BIT_ADDR_CH_ERR_INTBIT(28)
+#define BIT_FIELD0_INT BIT(27)
+#define BIT_FIELD1_INT BIT(26)
+#define BIT_SFF_OR_INT BIT(25)
+#define BIT_RFF_OR_INT BIT(24)
+#define BIT_DMA_TSF_DONE_SFF   BIT(22)
+#define BIT_STATFF_INT BIT(21)
+#define BIT_DMA_TSF_DONE_FB2   BIT(20)
+#define BIT_DMA_TSF_DONE_FB1   BIT(19)
+#define BIT_RXFF_INT   BIT(18)
+#define BIT_EOF_INTBIT(17)
+#define BIT_SOF_INTBIT(16)
+#define BIT_F2_INT BIT(15)
+#define BIT_F1_INT BIT(14)
+#define BIT_COF_INTBIT(13)
+#define BIT_HRESP_ERR_INT  BIT(7)
+#define BIT_ECC_INTBIT(1)
+#define BIT_DRDY   BIT(0)
+
+/* csi control reg 18 */
+#define BIT_CSI_HW_ENABLE  BIT(31)
+#define BIT_MIPI_DATA_FORMAT_RAW8  (0x2a << 25)
+#define BIT_MIPI_DATA_FORMAT_RAW10 (0x2b << 25)
+#define BIT_MIPI_DATA_FORMAT_RAW12 (0x2c << 25)
+#define BIT_MIPI_DATA_FORMAT_RAW14 (0x2d << 25)
+#define BIT_MIPI_DATA_FORMAT_YUV422_8B (0x1e << 25)
+#define BIT_MIPI_DATA_FORMAT_MASK  (0x3F << 25)
+#define BIT_MIPI_DATA_FORMAT_OFFSET25
+#define BIT_DATA_FROM_MIPI BIT(22)
+#define BIT_MIPI_YU_SWAP   BIT(21)
+#define BIT_MIPI_DOUBLE_CMPNT  BIT(20)
+#define BIT_BASEADDR_CHG_ERR_ENBIT(9)
+#define BIT_BASEADDR_SWITCH_SELBIT(5)
+#define BIT_BASEADDR_SWITCH_EN BIT(4)
+#define BIT_PARALLEL24_EN  BIT(3)
+#define BIT_DEINTERLACE_EN BIT(2)
+#define BIT_TVDECODER_IN_ENBIT(1)
+#define BIT_NTSC_ENBIT(0)
+
+#define CSI_MCLK_VF1
+#define CSI_MCLK_ENC   2
+#define CSI_MCLK_RAW   4
+#define CSI_MCLK_I2C   8
+
+#define CSI_CSICR1 0x0
+#define CSI_CSICR2 0x4
+#define CSI_CSICR3 0x8
+#define CSI_STATFIFO   0xC
+#define CSI_CSIRXFIFO  0x10
+#define CSI_CSI

Re: [PATCH] staging: android: ion: Allocate from heap ID directly without mask

2019-01-24 Thread Andrew F. Davis
On 1/24/19 9:24 AM, Brian Starkey wrote:
> Hi Andrew,
> 
> On Wed, Jan 23, 2019 at 01:28:35PM -0600, Andrew F. Davis wrote:
>> Previously the heap to allocate from was selected by a mask of allowed
>> heap types. This may have been done as a primitive form of constraint
>> solving, the first heap type that matched any set bit of the heap mask
>> was allocated from, unless that heap was excluded by having its heap
>> ID bit not set in the separate passed in heap ID mask.
> 
> I checked our gralloc and it's not using the current "keep trying
> until one succeeds" functionality, but it might be worth explicitly
> calling out in the commit message that this will also disappear with
> the new API. Maybe someone cares about that.
> 

I couldn't find any users either, I'll make a note anyway.

>>
>> The heap type does not really represent constraints that should be
>> matched against to begin with. So the patch [0] removed the the heap type
>> mask matching but unfortunately left the heap ID mask check (possibly by
>> mistake or to preserve API). Therefor we now only have a mask of heap
>> IDs, but heap IDs are unique identifiers and have nothing to do with the
>> underling heap, so mask matching is not useful here. This also limits us
> 
> s/underling/underlying/
> 

ACK

>> to 32 heaps total in a system.
>>
>> With the heap query API users can find the right heap based on type or
>> name themselves then just supply the ID for that heap. Remove heap ID
>> mask and allow users to specify heap ID directly by its number.
>>
>> I know this is an ABI break, but we are in staging so lets get this over
>> with now rather than limit ourselves later.
> 
> What do you think about renaming ion_allocation_data.unused to heap_id
> and adding a flag instead? It's adding cruft to a staging API, but it
> might soften the transition. The "old way" could get completely
> removed just before destaging. Just a thought.
> 

Sounds confusing, backwards compatibility in staging just doesn't seem
like the right thing to do.

Plus I have (probably false) hope that we *are* close destaging. So I
don't want to leave stuff for later.

>>
>> [0] 2bb9f5034ec7 ("gpu: ion: Remove heapmask from client")
>>
>> Signed-off-by: Andrew F. Davis 
>> ---
>>
>> This also means we don't need to store the available heaps in a plist,
>> we only operation we care about is lookup, so a better data structure
>> should be chosen at some point, regular list or xarray maybe?
>>
>> This is base on -next as to be on top of the other taken Ion patches.
>>
>>  drivers/staging/android/ion/ion.c  | 21 ++---
>>  drivers/staging/android/uapi/ion.h |  6 ++
>>  2 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c 
>> b/drivers/staging/android/ion/ion.c
>> index 92c2914239e3..06dd5bb10ecb 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -387,7 +387,7 @@ static const struct dma_buf_ops dma_buf_ops = {
>>  .unmap = ion_dma_buf_kunmap,
>>  };
>>  
>> -static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int 
>> flags)
>> +static int ion_alloc(size_t len, unsigned int heap_id, unsigned int flags)
>>  {
>>  struct ion_device *dev = internal_dev;
>>  struct ion_buffer *buffer = NULL;
>> @@ -396,23 +396,22 @@ static int ion_alloc(size_t len, unsigned int 
>> heap_id_mask, unsigned int flags)
>>  int fd;
>>  struct dma_buf *dmabuf;
>>  
>> -pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
>> - len, heap_id_mask, flags);
>> -/*
>> - * traverse the list of heaps available in this system in priority
>> - * order.  If the heap type is supported by the client, and matches the
>> - * request of the caller allocate from it.  Repeat until allocate has
>> - * succeeded or all heaps have been tried
>> - */
>> +pr_debug("%s: len %zu heap_id %u flags %x\n", __func__,
>> + len, heap_id, flags);
>> +
>>  len = PAGE_ALIGN(len);
>>  
>>  if (!len)
>>  return -EINVAL;
>>  
>> +/*
>> + * Traverse the list of heaps available in this system.  If the
>> + * heap id matches the request of the caller allocate from it.
>> + */
>>  down_read(&dev->lock);
>>  plist_for_each_entry(heap, &dev->heaps, node) {
>>  /* if the caller didn't specify this heap id */
>> -if (!((1 << heap->id) & heap_id_mask))
>> +if (heap->id != heap_id)
>>  continue;
>>  buffer = ion_buffer_create(heap, dev, len, flags);
>>  if (!IS_ERR(buffer))
> 
> You can break unconditionally here now, though it might be neater to
> refactor to:
> 
>  if (heap matches) {
>   buffer = alloc();
>   break;
>  }

I was attempting to have the smallest patch that makes this API change
then clean up all the logic later, but this is looks like something that
should just be done now. Will fix for v2.


Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

2019-01-24 Thread Brian Starkey
On Thu, Jan 24, 2019 at 10:04:46AM -0600, Andrew F. Davis wrote:
> On 1/23/19 11:11 AM, Brian Starkey wrote:

[snip]

> 
> I'm very new to all this, so any pointers to history in this area are
> appreciated.
> 

[snip]

> 
> > In case you didn't come across it already, the effort which seems to
> > have gained the most "air-time" recently is
> > https://github.com/cubanismo/allocator, which is still a userspace
> > module (perhaps some concepts from there could go into the kernel?),
> > but makes some attempts at generic constraint solving. It's also not
> > really moving anywhere at the moment.
> > 
> 
> Very interesting, I'm going to have to stare at this for a bit.

In which case, some reading material that might be of interest :-)

https://www.x.org/wiki/Events/XDC2016/Program/Unix_Device_Memory_Allocation.pdf
https://www.x.org/wiki/Events/XDC2017/jones_allocator.pdf
https://lists.freedesktop.org/archives/mesa-dev/2017-November/177632.html

-Brian

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: android: ion: Allocate from heap ID directly without mask

2019-01-24 Thread Brian Starkey
On Thu, Jan 24, 2019 at 10:12:10AM -0600, Andrew F. Davis wrote:
> On 1/24/19 9:24 AM, Brian Starkey wrote:

[snip]

> > 
> > What do you think about renaming ion_allocation_data.unused to heap_id
> > and adding a flag instead? It's adding cruft to a staging API, but it
> > might soften the transition. The "old way" could get completely
> > removed just before destaging. Just a thought.
> > 
> 
> Sounds confusing, backwards compatibility in staging just doesn't seem
> like the right thing to do.
> 

Well, fair enough. It's going to cause a bit of pain - libion could
probably isolate most Android users. Laura patched that in AOSP last
time around, but I don't know how that went (or how strongly Google
feel about kernel ABI changes).

Thanks,
-Brian
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED

2019-01-24 Thread Rodrigo Ribeiro
Remove the checkpatch.pl check:

CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'?

Signed-off-by: Rodrigo Ribeiro 
Signed-off-by: Rafael Tsuha 
---
This macro is not used anywhere. Should we just correct the
spelling or remove the macro?

 drivers/staging/iio/cdc/ad7152.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 25f51db..c9da6d4 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -35,7 +35,7 @@
 #define AD7152_REG_CH2_GAIN_HIGH   12
 #define AD7152_REG_CH2_SETUP   14
 #define AD7152_REG_CFG 15
-#define AD7152_REG_RESEVERD16
+#define AD7152_REG_RESERVED16
 #define AD7152_REG_CAPDAC_POS  17
 #define AD7152_REG_CAPDAC_NEG  18
 #define AD7152_REG_CFG226
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference

2019-01-24 Thread Stephen Boyd
Quoting Jonathan Cameron (2018-12-16 02:07:41)
> Rob, Clk experts, questions for you below.
> 
> Jonathan
> 
> 
> On Thu, 13 Dec 2018 17:39:22 -0800
> Stephen Boyd  wrote:
> 
> > Quoting Jonathan Cameron (2018-12-08 07:29:54)
> > > On Thu, 6 Dec 2018 11:10:51 +0200
> > > Mircea Caprioru  wrote:
> > >   
> > > > This patch adds a clock to the state structure of ad7192 for getting the
> > > > external clock frequency. This modifications is in accordance with clock
> > > > framework dt bindings documentation.
> > > > 
> > > > Signed-off-by: Mircea Caprioru   
> > > 
> > > +cc Rob and the clk list for advise on how to do the binding for this one.
> > > 
> > > It is basically 2 pins, you can put a clock in on one of them or connect
> > > a crystal across them.  The driver has to set a register to say which is
> > > the case.  
> > > 
> > > Current proposal is two optional clocks (fall back to internal oscillator)
> > > but that doesn't seem to be commonly done, so I'm wondering if there
> > > is a 'standard' way to handle this sort of thing.
> > >   
> > 
> > I'm not sure I fully understand, but I think perhaps
> > assigned-clock-parents would work? Or does that not work because either
> > way some parent is assigned, either the crystal or the optional clk that
> > isn't a crystal?
> > 
> My concern is they aren't really separate clock inputs.   They are just 
> different
> ways of providing the same fundamental clock.  So I think we may want to just
> provide a single clock and have another dt binding to say what it is.
> 
> So lots of ways we could do it, but I'm not sure what the right one to
> go with is!
> 

Sorry for getting back to this so late. Is the datasheet public for this
device? If so, any link to it?

If it's two pins, and sometimes one pin is connected and other times two
pins are connected but a register needs to be set if the pins are
connected in one configuration or the other I would say your plan for a
DT property indicating how the pins are configured sounds good. Usually
the hardware can figure these things out itself so I find this sort of
odd, but if this is how it is then there's not much that we can do.

It sounds like there aren't two different clk inputs to the device.
Given that information specifying two optional clks is incorrect because
there is only one 'slot' is the external clk source.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel