[PATCH] [media] mtk-vcodec: fix build errors without DEBUG

[Minghsiu Tsai]

Fix build errors after removing DEBUG definition.

Signed-off-by: Minghsiu Tsai 
---
 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 9 -
 drivers/media/platform/mtk-vcodec/vdec_vpu_if.c| 5 ++---
 drivers/media/platform/mtk-vcodec/venc_vpu_if.c| 4 +---
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c 
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 0746592..6219c7d 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -1126,15 +1126,14 @@ static void vb2ops_vdec_buf_queue(struct vb2_buffer *vb)
 * if there is no SPS header or picture info
 * in bs
 */
-   int log_level = ret ? 0 : 1;
 
src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf),
VB2_BUF_STATE_DONE);
-   mtk_v4l2_debug(log_level,
-   "[%d] vdec_if_decode() src_buf=%d, size=%zu, 
fail=%d, res_chg=%d",
-   ctx->id, src_buf->index,
-   src_mem.size, ret, res_chg);
+   mtk_v4l2_debug(ret ? 0 : 1,
+  "[%d] vdec_if_decode() src_buf=%d, size=%zu, 
fail=%d, res_chg=%d",
+  ctx->id, src_buf->index,
+  src_mem.size, ret, res_chg);
return;
}
 
diff --git a/drivers/media/platform/mtk-vcodec/vdec_vpu_if.c 
b/drivers/media/platform/mtk-vcodec/vdec_vpu_if.c
index 5a24c51..1abd14e 100644
--- a/drivers/media/platform/mtk-vcodec/vdec_vpu_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec_vpu_if.c
@@ -70,9 +70,8 @@ void vpu_dec_ipi_handler(void *data, unsigned int len, void 
*priv)
 static int vcodec_vpu_send_msg(struct vdec_vpu_inst *vpu, void *msg, int len)
 {
int err;
-   uint32_t msg_id = *(uint32_t *)msg;
 
-   mtk_vcodec_debug(vpu, "id=%X", msg_id);
+   mtk_vcodec_debug(vpu, "id=%X", *(uint32_t *)msg);
 
vpu->failure = 0;
vpu->signaled = 0;
@@ -80,7 +79,7 @@ static int vcodec_vpu_send_msg(struct vdec_vpu_inst *vpu, 
void *msg, int len)
err = vpu_ipi_send(vpu->dev, vpu->id, msg, len);
if (err) {
mtk_vcodec_err(vpu, "send fail vpu_id=%d msg_id=%X status=%d",
-  vpu->id, msg_id, err);
+  vpu->id, *(uint32_t *)msg, err);
return err;
}
 
diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c 
b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
index a01c759..0d882ac 100644
--- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
@@ -79,10 +79,8 @@ static int vpu_enc_send_msg(struct venc_vpu_inst *vpu, void 
*msg,
 
status = vpu_ipi_send(vpu->dev, vpu->id, msg, len);
if (status) {
-   uint32_t msg_id = *(uint32_t *)msg;
-
mtk_vcodec_err(vpu, "vpu_ipi_send msg_id %x len %d fail %d",
-  msg_id, len, status);
+  *(uint32_t *)msg, len, status);
return -EINVAL;
}
if (vpu->failure)
-- 
1.9.1

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

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Feb  7 05:00:21 CET 2017
media-tree git hash:47b037a0512d9f8675ec2693bed46c8ea6a884ab
media_build git hash:   785cdf7f0798964681b33aad44fc2ff4d734733d
v4l-utils git hash: 99306f20cc7e76cf2161e3059de4da245aed2130
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.8.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9-i686: OK
linux-4.10-rc3-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: OK
linux-4.9-x86_64: OK
linux-4.10-rc3-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v3 16/24] media: Add i.MX media core driver

[Steve Longerbeam]

On 02/02/2017 02:50 PM, Russell King - ARM Linux wrote:

On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote:

+/* register an internal subdev as a platform device */
+static struct imx_media_subdev *
+add_internal_subdev(struct imx_media_dev *imxmd,
+   const struct internal_subdev *isd,
+   int ipu_id)
+{
+   struct imx_media_internal_sd_platformdata pdata;
+   struct platform_device_info pdevinfo = {0};
+   struct imx_media_subdev *imxsd;
+   struct platform_device *pdev;
+
+   switch (isd->id->grp_id) {
+   case IMX_MEDIA_GRP_ID_CAMIF0...IMX_MEDIA_GRP_ID_CAMIF1:
+   pdata.grp_id = isd->id->grp_id +
+   ((2 * ipu_id) << IMX_MEDIA_GRP_ID_CAMIF_BIT);
+   break;
+   default:
+   pdata.grp_id = isd->id->grp_id;
+   break;
+   }
+
+   /* the id of IPU this subdev will control */
+   pdata.ipu_id = ipu_id;
+
+   /* create subdev name */
+   imx_media_grp_id_to_sd_name(pdata.sd_name, sizeof(pdata.sd_name),
+   pdata.grp_id, ipu_id);
+
+   pdevinfo.name = isd->id->name;
+   pdevinfo.id = ipu_id * num_isd + isd->id->index;
+   pdevinfo.parent = imxmd->dev;
+   pdevinfo.data = 
+   pdevinfo.size_data = sizeof(pdata);
+   pdevinfo.dma_mask = DMA_BIT_MASK(32);
+
+   pdev = platform_device_register_full();
+   if (IS_ERR(pdev))
+   return ERR_CAST(pdev);
+
+   imxsd = imx_media_add_async_subdev(imxmd, NULL, dev_name(>dev));
+   if (IS_ERR(imxsd))
+   return imxsd;
+
+   imxsd->num_sink_pads = isd->num_sink_pads;
+   imxsd->num_src_pads = isd->num_src_pads;
+
+   return imxsd;
+}

You seem to create platform devices here, but I see nowhere that you
ever remove them - so if you get to the lucky point of being able to
rmmod imx-media and then try to re-insert it, you end up with a load
of kernel warnings, one for each device created this way, and
platform_device_register_full() fails:


Right, I never free the platform devices for the internal subdevs.
Fixed.

Steve

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

Re: [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver

[Steve Longerbeam]

On 02/02/2017 02:35 PM, Russell King - ARM Linux wrote:


However, "*vfd" contains a struct device, and you _correctly_ set the
release function for "*vfd" to video_device_release via camif_videodev.

However, if you try to rmmod imx-media, then you end up with a kernel
warning that you're freeing memory containing a held lock, and later
chaos ensues because kmalloc has been corrupted.

The root cause of this is embedding the device structure within the
video_device into the driver's private data.  *Any* structure what so
ever that contains a kref is reference counted, and that includes
struct device, and therefore also includes struct video_device.  What
that means is that its lifetime is _not_ under _your_ control, and
you may not free it except through its release function (which is
video_device_release().)  However, that also tries to kfree (with an
offset of 4) your private data, which results in the warning and the
corrupted kmalloc free lists.

The solution is simple, make "vfd" a pointer in your private data
structure and kmalloc() it separately, letting video_device_release()
kfree() that data when it needs to.


Thanks Russell for tracking this down. I remember doing this
when I was reviewing the code for opportunities to "optimize" :-/,
and carelessly caused this bug by not reviewing how video_device
is freed.

Fixed.

Steve

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

Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver

[Steve Longerbeam]

On 02/06/2017 02:33 PM, Laurent Pinchart wrote:

Hi Hans,

(CC'ing Sakari)

On Monday 06 Feb 2017 10:50:22 Hans Verkuil wrote:

On 02/05/2017 04:48 PM, Laurent Pinchart wrote:

On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:

On 01/24/2017 04:02 AM, Philipp Zabel wrote:

On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:

+
+int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct
v4l2_mbus_config
*cfg)
+{
+   struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
+   struct media_pad *pad;
+   int ret;
+
+   if (vidsw->active == -1) {
+   dev_err(sd->dev, "no configuration for inactive

mux\n");

+   return -EINVAL;
+   }
+
+   /*
+* Retrieve media bus configuration from the entity connected

to the

+* active input
+*/
+   pad = media_entity_remote_pad(>pads[vidsw->active]);
+   if (pad) {
+   sd = media_entity_to_v4l2_subdev(pad->entity);
+   ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
+   if (ret == -ENOIOCTLCMD)
+   pad = NULL;
+   else if (ret < 0) {
+   dev_err(sd->dev, "failed to get source
configuration\n");
+   return ret;
+   }
+   }
+   if (!pad) {
+   /* Mirror the input side on the output side */
+   cfg->type = vidsw->endpoint[vidsw->active].bus_type;
+   if (cfg->type == V4L2_MBUS_PARALLEL ||
+   cfg->type == V4L2_MBUS_BT656)
+   cfg->flags = vidsw->endpoint[vidsw-
active].bus.parallel.flags;
+   }
+
+   return 0;
+}

I am not certain this op is needed at all. In the current kernel this
op is only used by soc_camera, pxa_camera and omap3isp (somewhat
dubious). Normally this information should come from the device tree
and there should be no need for this op.

My (tentative) long-term plan was to get rid of this op.

If you don't need it, then I recommend it is removed.

Hi Hans, the imx-media driver was only calling g_mbus_config to the
camera sensor, and it was doing that to determine the sensor's bus type.
This info was already available from parsing a v4l2_of_endpoint from the
sensor node. So it was simple to remove the g_mbus_config calls, and
instead rely on the parsed sensor v4l2_of_endpoint.

That's not a good point.

(mea culpa, s/point/idea/)


The imx-media driver must not parse the sensor DT node as it is not aware
of what bindings the sensor is compatible with.


Hi Laurent,

I don't really understand this argument. The sensor node has been found
by parsing the OF graph, so it is known to be a camera sensor node at
that point.



  Information must instead
be queried from the sensor subdev at runtime, through the g_mbus_config()
operation.

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

Shouldn't this come from the imx-media DT node? BTW, why is omap3isp using
this?

It all depends on what type of information needs to be retrieved, and whether
it can change at runtime or is fixed. Adding properties to the imx-media DT
node is certainly fine as long as those properties describe the i.MX side.


In this case the info needed is the media bus type. That info is most easily
available by calling v4l2_of_parse_endpoint() on the sensor's endpoint node.

The media bus type is not something that can be added to the
imx-media node since it contains no endpoint nodes.



In the omap3isp case, we use the operation to query whether parallel data
contains embedded sync (BT.656) or uses separate h/v sync signals.


The reason I am suspicious about this op is that it came from soc-camera and
predates the DT. The contents of v4l2_mbus_config seems very much like a HW
description to me, i.e. something that belongs in the DT.

Part of it is possibly outdated, but for buses that support multiple modes of
operation (such as the parallel bus case described above) we need to make that
information discoverable at runtime. Maybe this should be considered as
related to Sakari's efforts to support VC/DT for CSI-2, and supported through
the API he is working on.


That sounds interesting, can you point me to some info on this effort?
I've been thinking the DT should contain virtual channel info for CSI-2
buses.

Steve





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

Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver

[Laurent Pinchart]

Hi Hans,

(CC'ing Sakari)

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

(mea culpa, s/point/idea/)

> > The imx-media driver must not parse the sensor DT node as it is not aware
> > of what bindings the sensor is compatible with. Information must instead
> > be queried from the sensor subdev at runtime, through the g_mbus_config()
> > operation.
> > 
> > Of course, if you can get the information from the imx-media DT node,
> > that's certainly an option. It's only information provided by the sensor
> > driver that you have no choice but query using a subdev operation.
> 
> Shouldn't this come from the imx-media DT node? BTW, why is omap3isp using
> this?

It all depends on what type of information needs to be retrieved, and whether 
it can change at runtime or is fixed. Adding properties to the imx-media DT 
node is certainly fine as long as those properties describe the i.MX side.

In the omap3isp case, we use the operation to query whether parallel data 
contains embedded sync (BT.656) or uses separate h/v sync signals.

> The reason I am suspicious about this op is that it came from soc-camera and
> predates the DT. The contents of v4l2_mbus_config seems very much like a HW
> description to me, i.e. something that belongs in the DT.

Part of it is possibly outdated, but for buses that support multiple modes of 
operation (such as the parallel bus case described above) we need to make that 
information discoverable at runtime. Maybe this should be considered as 
related to Sakari's efforts to support VC/DT for CSI-2, and supported through 
the API he is working on.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCHv2 08/16] atmel-isi: document device tree bindings

[Rob Herring]

On Mon, Feb 6, 2017 at 6:20 AM, Hans Verkuil  wrote:
> On 02/01/2017 05:50 PM, Rob Herring wrote:
>> On Mon, Jan 30, 2017 at 03:06:20PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil 
>>>
>>> Document the device tree bindings for this driver.

>>> +isi: isi@f0034000 {
>>> +compatible = "atmel,at91sam9g45-isi";
>>> +reg = <0xf0034000 0x4000>;
>>> +interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>>> +pinctrl-names = "default";
>>> +pinctrl-0 = <_isi_data_0_7>;
>>> +clocks = <_clk>;
>>> +clock-names = "isi_clk";
>>> +status = "ok";
>>> +port {
>>> +#address-cells = <1>;
>>> +#size-cells = <0>;
>>> +isi_0: endpoint {
>>> +reg = <0>;
>>
>> Drop reg.
>
> Is that because that is the default?

Essentially, yes. You only need reg if you have more than one of something.

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

V4L board mis-identified, Tuner is TDA18271HD not s921

[Matthew Hughes]

Plugged in a USB ATSC tuner, has an em28xx bridge along with both a
NXP TDA18271HDC2 and a Trident DRX3933J_B2... stenciled on the board
is EzTV306_1.2

It however registered as a DVB device with a sharp s921 tuner.

[  208.072748] usb 4-1.3: new high-speed USB device number 3 using ehci-pci
[  208.298601] media: Linux media interface: v0.10
[  208.312132] Linux video capture interface: v2.00
[  208.636160] em28xx: New device  USB 2875 Device @ 480 Mbps
(eb1a:2875, interface 0, class 0)
[  208.636162] em28xx: DVB interface 0 found: isoc
[  208.636222] em28xx: chip ID is em2874
[  208.715834] em2874 #0: EEPROM ID = 26 40 03 00, EEPROM hash = 0xe0a5bac9
[  208.715836] em2874 #0: EEPROM info:
[  208.715837] em2874 #0:  microcode start address = 0x4004, boot
configuration = 0x03
[  208.739463] em2874 #0:  I2S audio, 5 sample rates
[  208.739465] em2874 #0:  500mA max power
[  208.739467] em2874 #0:  Table at offset 0x24, strings=0x206a, 0x128a, 0x
[  208.741337] em2874 #0: No sensor detected
[  208.769463] em2874 #0: found i2c device @ 0xa0 on bus 0 [eeprom]
[  208.786096] em2874 #0: Your board has no unique USB ID.
[  208.786106] em2874 #0: A hint were successfully done, based on i2c
devicelist hash.
[  208.786110] em2874 #0: This method is not 100% failproof.
[  208.786114] em2874 #0: If the board were missdetected, please email
this log to:
[  208.786117] em2874 #0:  V4L Mailing List  
[  208.786120] em2874 #0: Board detected as EM2874 Leadership ISDBT
[  208.892731] em2874 #0: Identified as EM2874 Leadership ISDBT (card=77)
[  208.892734] em2874 #0: dvb set to isoc mode.
[  208.893013] usbcore: registered new interface driver em28xx
[  209.067077] em2874 #0: Binding DVB extension
[  209.124284] s921: s921_attach:
[  209.124291] DVB: registering new adapter (em2874 #0)
[  209.124299] usb 4-1.3: DVB: registering adapter 0 frontend 0 (Sharp S921)...
[  209.124819] em2874 #0: DVB extension successfully initialized
[  209.124823] em28xx: Registered (Em28xx dvb Extension) extension
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Staging: omap4iss: Fix coding style issues

[Avraham Shukron]

Fixes line-over-80-characters issues as well as multiline comments style.

Signed-off-by: Avraham Shukron 

---
 drivers/staging/media/omap4iss/iss_video.c | 41 --
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..42de513 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,
pix->width = mbus->width;
pix->height = mbus->height;
 
-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,
 
min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;
 
-   /* Clamp the requested bytes per line value. If the maximum bytes per
+   /*
+* Clamp the requested bytes per line value. If the maximum bytes per
 * line value is zero, the module doesn't support user configurable line
 * sizes. Override the requested value with the minimum in that case.
 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct 
v4l2_pix_format *pix,
mbus->width = pix->width;
mbus->height = pix->height;
 
-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -298,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct 
iss_video_fh *vfh)
 
 static int iss_video_queue_setup(struct vb2_queue *vq,
 unsigned int *count, unsigned int *num_planes,
-unsigned int sizes[], struct device 
*alloc_devs[])
+unsigned int sizes[],
+struct device *alloc_devs[])
 {
struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
struct iss_video *video = vfh->video;
@@ -360,7 +364,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)
 
spin_lock_irqsave(>qlock, flags);
 
-   /* Mark the buffer is faulty and give it back to the queue immediately
+   /*
+* Mark the buffer is faulty and give it back to the queue immediately
 * if the video node has registered an error. vb2 will perform the same
 * check when preparing the buffer, but that is inherently racy, so we
 * need to handle the race condition with an authoritative check here.
@@ -443,7 +448,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct 
iss_video *video)
 
buf->vb.vb2_buf.timestamp = ktime_get_ns();
 
-   /* Do frame number propagation only if this is the output video node.
+   /*
+* Do frame number propagation only if this is the output video node.
 * Frame number either comes from the CSI receivers or it gets
 * incremented here if H3A is not active.
 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +611,8 @@ iss_video_set_format(struct file *file, void *fh, struct 
v4l2_format *format)
 
mutex_lock(>mutex);
 
-   /* Fill the bytesperline and sizeimage fields by converting to media bus
+   /*
+* Fill the bytesperline and sizeimage fields by converting to media bus
 * format and back to pixel format.
 */
iss_video_pix_to_mbus(>fmt.pix, );
@@ -678,8 +685,9 @@ iss_video_get_selection(struct file *file, void *fh, struct 
v4l2_selection *sel)
if (subdev == NULL)
return -EINVAL;
 
-   /* Try the get selection operation first and fallback to get format if 
not
-* implemented.
+   /*
+* Try the get selection operation first and fallback to get format if
+* not implemented.
 */
sdsel.pad = pad;
ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, );
@@ -867,7 +875,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
 
mutex_lock(>stream_lock);
 
-   /* Start streaming on the pipeline. No link touching an entity in the
+   /*
+* Start streaming on the pipeline. No link touching an entity in the
 * pipeline can be activated or deactivated once streaming is started.
 */
pipe = entity->pipe
@@ -895,7 +904,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
while ((entity = media_entity_graph_walk_next()))
media_entity_enum_set(>ent_enum, entity);
 
-   /* Verify that the currently 

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

[Hans Verkuil]

On 02/06/2017 04:21 PM, Dave Stevenson wrote:
> Hi Hans.
> 
> On 06/02/17 12:58, Hans Verkuil wrote:
>> On 02/06/2017 12:37 PM, Dave Stevenson wrote:
>>> Hi Hans.
>>>
>>> On 06/02/17 09:08, Hans Verkuil wrote:
 Hi Eric,

 Great to see this driver appearing for upstream merging!

 See below for my review comments, focusing mostly on V4L2 specifics.

> 
> 
> 
> + f->fmt.pix.pixelformat = dev->capture.fmt->fourcc;
> + f->fmt.pix.bytesperline = dev->capture.stride;
> + f->fmt.pix.sizeimage = dev->capture.buffersize;
> +
> + if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_RGB24)
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> + else if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_JPEG)
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
> + else
> + f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;

 Colorspace has nothing to do with the pixel format. It should come from the
 sensor/video receiver.

 If this information is not available, then COLORSPACE_SRGB is generally a
 good fallback.
>>>
>>> I would if I could, but then I fail v4l2-compliance on V4L2_PIX_FMT_JPEG
>>> https://git.linuxtv.org/v4l-utils.git/tree/utils/v4l2-compliance/v4l2-test-formats.cpp#n329
>>> The special case for JPEG therefore has to remain.
>>
>> Correct. Sorry, my fault, I forgot about that.
>>
>>>
>>> It looks like I tripped over the subtlety between V4L2_COLORSPACE_,
>>> V4L2_XFER_FUNC_, V4L2_YCBCR_ENC_, and V4L2_QUANTIZATION_, and Y'CbCr
>>> encoding vs colourspace.
>>>
>>> The ISP coefficients are set up for BT601 limited range, and any
>>> conversion back to RGB is done based on that. That seemed to fit
>>> SMPTE170M rather than SRGB.
>>
>> Colorspace refers to the primary colors + whitepoint that are used to
>> create the colors (basically this answers the question to which colors
>> R, G and B exactly refer to). The SMPTE170M has different primaries
>> compared to sRGB (and a different default transfer function as well).
>>
>> RGB vs Y'CbCr is just an encoding and it doesn't change the underlying
>> colorspace. Unfortunately, the term 'colorspace' is often abused to just
>> refer to RGB vs Y'CbCr.
>>
>> If the colorspace is SRGB, then when the pixelformat is a Y'CbCr encoding,
>> then the BT601 limited range encoding is implied, unless overridden via
>> the ycbcr_enc and/or quantization fields in struct v4l2_pix_format.
>>
>> In other words, this does already the right thing.
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-007.html#colorspace-srgb-v4l2-colorspace-srgb
> "The default transfer function is V4L2_XFER_FUNC_SRGB. The default 
> Y’CbCr encoding is V4L2_YCBCR_ENC_601. The default Y’CbCr quantization 
> is full range."
> So full range or limited?

Ah, good catch. The default range for SRGB is full range, so the documentation
is correct. This is according to the sYCC standard.

This means that you need to set the quantization field to limited range in this 
driver.

Sorry for the confusion I caused.

Interesting, I should take a look at other drivers since I suspect that this is
signaled wrong elsewhere as well. It used to be limited range but I changed it
to full range (as per the sYCC spec). But in practice it is limited range in 
most
cases.

I'll take another look at this on Friday.

I recommend that you leave the code as is for now.

Regards,

Hans

>>> Test input 0:
>>>
>>> Control ioctls:
>>> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>>> test VIDIOC_QUERYCTRL: OK
>>> test VIDIOC_G/S_CTRL: OK
>>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
>>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>>> Standard Controls: 33 Private Controls: 0
>>>
>>> Format ioctls:
>>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>>> test VIDIOC_G/S_PARM: OK
>>> test VIDIOC_G_FBUF: OK
>>> test VIDIOC_G_FMT: OK
>>> test VIDIOC_TRY_FMT: OK
>>> test VIDIOC_S_FMT: OK
>>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>>> test Cropping: OK (Not Supported)
>>> test Composing: OK (Not Supported)
>>> test Scaling: OK
>>
>> Is scaling supported? Just checking.
> 
> The cropping/composing/scaling API is not currently supported.
> The hardware can do it, but I need to work out how it should be set up, 
> and what resolutions to quote via V4L2_SEL_TGT_CROP_BOUNDS and similar. 
> It just needs a bit of time.

OK. This needs some attention before it can be moved out of staging.

Regards,

Hans

> 
>>>
>>> Codec ioctls:
>>> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>>> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>>> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>>>
>>> Buffer ioctls:
>>> test 

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

[Dave Stevenson]

Hi Hans.

On 06/02/17 12:58, Hans Verkuil wrote:

On 02/06/2017 12:37 PM, Dave Stevenson wrote:

Hi Hans.

On 06/02/17 09:08, Hans Verkuil wrote:

Hi Eric,

Great to see this driver appearing for upstream merging!

See below for my review comments, focusing mostly on V4L2 specifics.






+   f->fmt.pix.pixelformat = dev->capture.fmt->fourcc;
+   f->fmt.pix.bytesperline = dev->capture.stride;
+   f->fmt.pix.sizeimage = dev->capture.buffersize;
+
+   if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_RGB24)
+   f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
+   else if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_JPEG)
+   f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
+   else
+   f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;


Colorspace has nothing to do with the pixel format. It should come from the
sensor/video receiver.

If this information is not available, then COLORSPACE_SRGB is generally a
good fallback.


I would if I could, but then I fail v4l2-compliance on V4L2_PIX_FMT_JPEG
https://git.linuxtv.org/v4l-utils.git/tree/utils/v4l2-compliance/v4l2-test-formats.cpp#n329
The special case for JPEG therefore has to remain.


Correct. Sorry, my fault, I forgot about that.



It looks like I tripped over the subtlety between V4L2_COLORSPACE_,
V4L2_XFER_FUNC_, V4L2_YCBCR_ENC_, and V4L2_QUANTIZATION_, and Y'CbCr
encoding vs colourspace.

The ISP coefficients are set up for BT601 limited range, and any
conversion back to RGB is done based on that. That seemed to fit
SMPTE170M rather than SRGB.


Colorspace refers to the primary colors + whitepoint that are used to
create the colors (basically this answers the question to which colors
R, G and B exactly refer to). The SMPTE170M has different primaries
compared to sRGB (and a different default transfer function as well).

RGB vs Y'CbCr is just an encoding and it doesn't change the underlying
colorspace. Unfortunately, the term 'colorspace' is often abused to just
refer to RGB vs Y'CbCr.

If the colorspace is SRGB, then when the pixelformat is a Y'CbCr encoding,
then the BT601 limited range encoding is implied, unless overridden via
the ycbcr_enc and/or quantization fields in struct v4l2_pix_format.

In other words, this does already the right thing.


https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-007.html#colorspace-srgb-v4l2-colorspace-srgb
"The default transfer function is V4L2_XFER_FUNC_SRGB. The default 
Y’CbCr encoding is V4L2_YCBCR_ENC_601. The default Y’CbCr quantization 
is full range."

So full range or limited?


The JPEG colorspace is a short-hand for V4L2_COLORSPACE_SRGB, V4L2_YCBCR_ENC_601
and V4L2_QUANTIZATION_FULL_RANGE. It's historical that this colorspace exists.

If I would redesign this this JPEG colorspace would be dropped.

For a lot more colorspace information see:

https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces.html



I do note that as there is now support for more RGB formats (BGR24 and
BGR32) the first "if" needs extending to cover those. Or I don't care
and only special case JPEG with all others just reporting SRGB.



Only special case JPEG.

But as I said, this information really needs to come from the sensor or
video receiver since this driver has no knowledge of this.


OK.






This is IMHO unnecessarily complex.

My recommendation is that controls are added with a set of v4l2_ctrl_new_std* 
calls
or if you really want to by walking a struct v4l2_ctrl_config array and adding 
controls
via v4l2_ctrl_new_custom.

The s_ctrl is a switch that calls the 'setter' function.

No need for arrays, callbacks, etc. Just keep it simple.


I can look into that, but I'm not sure I fully follow what you are
suggesting.

In the current implementation things like V4L2_CID_SATURATION,
V4L2_CID_SHARPNESS, V4L2_CID_CONTRAST, and V4L2_CID_BRIGHTNESS all use
the one common ctrl_set_rational setter function because the only thing
different in setting is the MMAL_PARAMETER_xxx value. I guess that could
move into the common setter based on V4L2_CID_xxx, but then the control
configuration is split between multiple places which feels less well
contained.


See e.g. samples/v4l/v4l2-pci-skeleton.c: in the probe function (or in a
function called from there if there are a lot of controls) you add the
controls, and in s_ctrl you handle them.

But this is just my preference.

So in s_ctrl you would see something like this:

switch (ctrl->id) {
case V4L2_CID_SATURATION:
ctrl_set_rational(ctrl->val, MMAL_PARAMETER_SAT);
break;
case V4L2_CID_BRIGHTNESS:
ctrl_set_rational(ctrl->val, MMAL_PARAMETER_BRIGHTNESS);
break;
...
}


OK, thanks for the clarification. That can be done.






Final question: did you run v4l2-compliance over this driver? Before this 
driver can
be moved out of staging it should pass the compliance tests. Note: always 
compile
this test from the main repository, don't rely on distros. That ensures you use 
the
latest code.

The 

[GIT PULL for v4.10] CEC fixes

[Mauro Carvalho Chehab]

Hi Linus,

Please pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.10-3

For a few documentation fixes at CEC (with got promoted from staging for 4.10),
and one fix on its core.

Thanks!
Mauro

-

The following changes since commit 0e0694ff1a7791274946b7f51bae692da0001a08:

  Merge branch 'patchwork' into v4l_for_linus (2016-12-26 14:09:28 -0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.10-3

for you to fetch changes up to f9f96fc10c09ca16e336854c08bc1563eed97985:

  [media] cec: fix wrong last_la determination (2017-01-30 11:42:31 -0200)


media fixes for v4.10


Hans Verkuil (3):
  [media] cec rst: remove "This API is not yet finalized" notice
  [media] cec-intro.rst: mention the v4l-utils package and CEC utilities
  [media] cec: fix wrong last_la determination

 Documentation/media/uapi/cec/cec-func-close.rst |  5 -
 Documentation/media/uapi/cec/cec-func-ioctl.rst |  5 -
 Documentation/media/uapi/cec/cec-func-open.rst  |  5 -
 Documentation/media/uapi/cec/cec-func-poll.rst  |  5 -
 Documentation/media/uapi/cec/cec-intro.rst  | 17 -
 Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst|  5 -
 .../media/uapi/cec/cec-ioc-adap-g-log-addrs.rst |  5 -
 .../media/uapi/cec/cec-ioc-adap-g-phys-addr.rst |  5 -
 Documentation/media/uapi/cec/cec-ioc-dqevent.rst|  5 -
 Documentation/media/uapi/cec/cec-ioc-g-mode.rst |  5 -
 Documentation/media/uapi/cec/cec-ioc-receive.rst|  5 -
 drivers/media/cec/cec-adap.c|  2 +-
 12 files changed, 13 insertions(+), 56 deletions(-)

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

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

[Dave Stevenson]

Hi Mauro.

Can I just say that I'm not attempting to upstream this, others are. 
I've just answered questions raised.


On 06/02/17 12:37, Mauro Carvalho Chehab wrote:

Em Sun, 5 Feb 2017 22:15:21 +
Dave Stevenson  escreveu:


If the goal was to protect some IP related to the sensors, I guess
this is not going to protect anything, as there are recent driver
submissions on linux-media for the ov5647 driver:

https://patchwork.kernel.org/patch/9472441/

There are also open source drivers for the Sony imx219 camera
floating around for android and chromeOS:


https://chromium.googlesource.com/chromiumos/third_party/kernel/+/factory-ryu-6486.14.B-chromeos-3.14/drivers/media/i2c/soc_camera/imx219.c

https://android.googlesource.com/kernel/bcm/+/android-bcm-tetra-3.10-lollipop-wear-release/drivers/media/video/imx219.c

Plus, there's a datasheet (with another driver) available at:
https://github.com/rellimmot/Sony-IMX219-Raspberry-Pi-V2-CMOS

So, you're not really protecting IP here.


None of the ISP processing, control loops, or tuning are open.
Yes there are kernel drivers kicking around for OV5647 and IMX219 now, 
but very little will consume raw Bayer.

That is also Omnivision or Sony IP, not Broadcom IP.


If the goal is to protect some proprietary algorithm meant to enhance
the camera captured streams, doing things like (auto-focus, auto-white
adjustments, scaling, etc), and/or implementing codec encoders, you should,
instead, restructure such codecs as mem2mem V4L2 drivers. There are a bunch
of such codecs already for other SoC where such functions are implemented
on GPU.

If you add DMABUF capabilities and media controller to the capture
driver and to the mem2mem drivers, userspace can use the V4L2 APIs
to use those modules using the arrangements they need, without
performance impacts.

So, by obfuscating the code, you're not protecting anything. Just making
harder for RPi customers to use, as you're providing a driver that is
limited.








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


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


I didn't analyze the VCHI driver. As I said before, if you rewrite
the driver in a way that the Kernel can actually see the sensors
via an I2C interface, you probably can get rid of the VCHI interface
for the capture part.

You could take a look on the other mem2mem drivers and see if are
there some way to provide an interface for the GPU encoders in a
similar way to what those drivers do.


I will be looking at a sub dev driver for just the CCP2/CSI1/CSI2 
receiver as Broadcom did manage to release (probably unintentionally) a 
bastardised soc-camera driver for it and therefore the info is in the 
public domain.


It would be possible to write a second sub dev driver that was similar 
to this in using VCHI/MMAL to create another mem2mem device to wrap the 
ISP, but that would just be an image processing pipe - control loops 
would all be elsewhere (userspace).
I haven't seen a sensible mechanism within V4L2 for exporting image 
statistics for AE, AWB, AF, or histograms to userspace so that 
appropriate algorithms can be run there. Have I missed them, or do they 
not exist?






That seems a terrible hack! let the user specify the resolution via
modprobe parameter... That should depend on the hardware capabilities
instead.


This is sitting on top of an OpenMaxIL style camera component (though
accessed via MMAL - long story, but basically MMAL removed a bundle of
the ugly/annoying parts of IL).
It has the extension above V1.1.2 that you have a preview port, video
capture port, and stills capture port. Stills captures have additional
processing stages to improve image quality, whilst video has to maintain
framerate.


I see.


If you're asked for YUV or RGB frame, how do you choose between video or
stills? That's what is being set with these parameters, not the sensor
resolution. Having independent stills and video processing options
doesn't appear to be something that is supported in V4L2, but I'm open
to suggestions.


At the capture stage:

Assuming that the user wants to use different resolutions for video and
stills (for example, you're seeing a real time video, then you "click"
to capture a still image), you can create two buffer groups, one for
low-res video and another one for high-res image. When the button is
clicked, it will stop the low-res stream, set the parameters for the
high-res image and capture it.

For post-processing stage:

Switch the media pipeline via the media controller adding the post
processing codecs that will enhance the image.

We're 

Re: [PATCH 10/11] [media] v4l2: Add v4l2 control IDs for HEVC encoder

[Andrzej Hajda]

Hi Smitha,

I have no big experience with HEVC, so it is hard to review it
appropriately but I will try do my best.
As these control names goes to user space you should be very careful
about it.
I guess it could be good to compare these controls with other HEVC
encoders including software ones (ffmpeg, intel, ...) to find some
similarities, common naming convention.


On 18.01.2017 11:02, Smitha T Murthy wrote:
> Add v4l2 controls for HEVC encoder
>
> CC: Hans Verkuil 
> CC: Wu-Cheng Li 
> CC: Kieran Bingham 
> CC: Vladimir Zapolskiy 
> CC: Laurent Pinchart 
> Signed-off-by: Smitha T Murthy 
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c |   51 
>  include/uapi/linux/v4l2-controls.h   |  109 
> ++
>  2 files changed, 160 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 47001e2..387439d 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -775,6 +775,57 @@ static bool is_new_manual(const struct v4l2_ctrl *master)
>   case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP:return "VPX 
> P-Frame QP Value";
>   case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:   return "VPX 
> Profile";
>  
> + /* HEVC controls */
> + case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP:   return "HEVC 
> Frame QP value";

Should be "HEVC I-Frame", it looks like the convention is to upper-case
first letter of all words,
and the convention is I-Frame, B-Frame, P-Frame, here and in the next
controls.
I would drop also the word "value", but it is already used in other
controls so I do not know :)

> + case V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP:   return "HEVC P 
> frame QP value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP:   return "HEVC B 
> frame QP value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP:   return "HEVC 
> Minimum QP value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP:   return "HEVC 
> Maximum QP value";
> + case V4L2_CID_MPEG_VIDEO_HEVC_ADAPTIVE_RC_DARK: return "HEVC 
> dark region adaptive";
> + case V4L2_CID_MPEG_VIDEO_HEVC_ADAPTIVE_RC_SMOOTH:   return "HEVC 
> smooth region adaptive";
> + case V4L2_CID_MPEG_VIDEO_HEVC_ADAPTIVE_RC_STATIC:   return "HEVC 
> static region adaptive";
> + case V4L2_CID_MPEG_VIDEO_HEVC_ADAPTIVE_RC_ACTIVITY: return "HEVC 
> activity adaptive";

Shouldn't it be "... Region Adaptive RC", or "... Region Adaptive Rate
Control" ?

> + case V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:  return "HEVC 
> Profile";
> + case V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:return "HEVC 
> level";
> + case V4L2_CID_MPEG_VIDEO_HEVC_TIER_FLAG:return "HEVC 
> tier_flag default is Main";

I guess 0 - means main tier, 1 means high tier, am I right? In such case
it should be named "HEVC high tier" or sth similar.

> + case V4L2_CID_MPEG_VIDEO_HEVC_RC_FRAME_RATE:return "HEVC 
> Frame rate";
> + case V4L2_CID_MPEG_VIDEO_HEVC_MAX_PARTITION_DEPTH:  return "HEVC 
> Maximum coding unit depth";
> + case V4L2_CID_MPEG_VIDEO_HEVC_REF_NUMBER_FOR_PFRAMES:   return "HEVC 
> Number of reference picture";

What is purpose of this control? Macro name suggest sth different than
string.

> + case V4L2_CID_MPEG_VIDEO_HEVC_REFRESH_TYPE: return "HEVC 
> refresh type";

Could you enumerate these refresh types, in patch 9 and documentation,
maybe it would be worth to make it menu.

> + case V4L2_CID_MPEG_VIDEO_HEVC_CONST_INTRA_PRED_ENABLE:  return "HEVC 
> constant intra prediction enabled";
> + case V4L2_CID_MPEG_VIDEO_HEVC_LOSSLESS_CU_ENABLE:   return "HEVC 
> lossless encoding select";
> + case V4L2_CID_MPEG_VIDEO_HEVC_WAVEFRONT_ENABLE: return "HEVC 
> Wavefront enable";

I see: enable, enabled, select. Let it be consistent.

> + case V4L2_CID_MPEG_VIDEO_HEVC_LF_DISABLE:   return "HEVC 
> Filter disable";

There is LF in macro name.

> + case V4L2_CID_MPEG_VIDEO_HEVC_LF_SLICE_BOUNDARY:return "across 
> or not slice boundary";

What does it mean?

> + case V4L2_CID_MPEG_VIDEO_HEVC_LTR_ENABLE:   return "long 
> term reference enable";
> + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_QP_ENABLE:   return "QP 
> values for temporal layer";
> + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_TYPE: return 
> "Hierarchical Coding Type";

Please enumerate types.

> + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER:return 
> "Hierarchical Coding Layer";

Please enumerate layers.

> + case V4L2_CID_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_LAYER_QP:return 
> "Hierarchical Coding Layer QP";
> + case 

Re: [PATCHv4 1/9] video: add hotplug detect notifier support

[Andrzej Hajda]

On 06.02.2017 11:29, Hans Verkuil wrote:
> From: Hans Verkuil 
>
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
>
> Based on an earlier version from Russell King:
>
> https://patchwork.kernel.org/patch/9277043/
>
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD 
> state
> of a video device.
>
> When a new notifier is registered the current state will be reported to
> that notifier at registration time.
>
> Signed-off-by: Hans Verkuil 
> Tested-by: Marek Szyprowski 

For patches 1-6:
Reviewed-by: Andrzej Hajda 
--
Regards
Andrzej


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

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

[Hans Verkuil]

On 01/27/2017 10:54 PM, Eric Anholt wrote:
> - Supports raw YUV capture, preview, JPEG and H264.
> - Uses videobuf2 for data transfer, using dma_buf.
> - Uses 3.6.10 timestamping
> - Camera power based on use
> - Uses immutable input mode on video encoder
> 
> This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
> a15ba877dab4e61ea3fc7b006e2a73828b083c52.
> 
> Signed-off-by: Eric Anholt 
> ---
>  .../media/platform/bcm2835/bcm2835-camera.c| 2016 
> 
>  .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
>  drivers/staging/media/platform/bcm2835/controls.c  | 1345 +
>  .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
>  .../media/platform/bcm2835/mmal-encodings.h|  127 ++
>  .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
>  .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
>  .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
>  drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
>  .../media/platform/bcm2835/mmal-parameters.h   |  689 +++
>  .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916 +++
>  .../staging/media/platform/bcm2835/mmal-vchiq.h|  178 ++
>  12 files changed, 7111 insertions(+)
>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/controls.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h
> 
> diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
> b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> new file mode 100644
> index ..4f03949aecf3
> --- /dev/null
> +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> @@ -0,0 +1,2016 @@



> +static int __init bm2835_mmal_init(void)
> +{
> + int ret;
> + struct bm2835_mmal_dev *dev;
> + struct vb2_queue *q;
> + int camera;
> + unsigned int num_cameras;
> + struct vchiq_mmal_instance *instance;
> + unsigned int resolutions[MAX_BCM2835_CAMERAS][2];
> +
> + ret = vchiq_mmal_init();
> + if (ret < 0)
> + return ret;
> +
> + num_cameras = get_num_cameras(instance,
> +   resolutions,
> +   MAX_BCM2835_CAMERAS);
> + if (num_cameras > MAX_BCM2835_CAMERAS)
> + num_cameras = MAX_BCM2835_CAMERAS;
> +
> + for (camera = 0; camera < num_cameras; camera++) {
> + dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL);
> + if (!dev)
> + return -ENOMEM;
> +
> + dev->camera_num = camera;
> + dev->max_width = resolutions[camera][0];
> + dev->max_height = resolutions[camera][1];
> +
> + /* setup device defaults */
> + dev->overlay.w.left = 150;
> + dev->overlay.w.top = 50;
> + dev->overlay.w.width = 1024;
> + dev->overlay.w.height = 768;
> + dev->overlay.clipcount = 0;
> + dev->overlay.field = V4L2_FIELD_NONE;
> + dev->overlay.global_alpha = 255;
> +
> + dev->capture.fmt = [3]; /* JPEG */
> +
> + /* v4l device registration */
> + snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name),
> +  "%s", BM2835_MMAL_MODULE_NAME);
> + ret = v4l2_device_register(NULL, >v4l2_dev);
> + if (ret)
> + goto free_dev;
> +
> + /* setup v4l controls */
> + ret = bm2835_mmal_init_controls(dev, >ctrl_handler);
> + if (ret < 0)
> + goto unreg_dev;
> + dev->v4l2_dev.ctrl_handler = >ctrl_handler;
> +
> + /* mmal init */
> + dev->instance = instance;
> + ret = mmal_init(dev);
> + if (ret < 0)
> + goto unreg_dev;
> +
> + /* initialize queue */
> + q = >capture.vb_vidq;
> + memset(q, 0, sizeof(*q));
> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> + q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;

I'm missing VB2_DMABUF here!

In fact, with dmabuf support I wonder if you still need overlay 

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

[Hans Verkuil]

On 02/06/2017 12:37 PM, Dave Stevenson wrote:
> Hi Hans.
> 
> On 06/02/17 09:08, Hans Verkuil wrote:
>> Hi Eric,
>>
>> Great to see this driver appearing for upstream merging!
>>
>> See below for my review comments, focusing mostly on V4L2 specifics.
>>
>> On 01/27/2017 10:54 PM, Eric Anholt wrote:
>>> - Supports raw YUV capture, preview, JPEG and H264.
>>> - Uses videobuf2 for data transfer, using dma_buf.
>>> - Uses 3.6.10 timestamping
>>> - Camera power based on use
>>> - Uses immutable input mode on video encoder
>>>
>>> This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
>>> a15ba877dab4e61ea3fc7b006e2a73828b083c52.
>>>
>>> Signed-off-by: Eric Anholt 
>>> ---
>>>  .../media/platform/bcm2835/bcm2835-camera.c| 2016 
>>> 
>>>  .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
>>>  drivers/staging/media/platform/bcm2835/controls.c  | 1345 +
>>>  .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
>>>  .../media/platform/bcm2835/mmal-encodings.h|  127 ++
>>>  .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
>>>  .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
>>>  .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
>>>  drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
>>>  .../media/platform/bcm2835/mmal-parameters.h   |  689 +++
>>>  .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916 
>>> +++
>>>  .../staging/media/platform/bcm2835/mmal-vchiq.h|  178 ++
>>>  12 files changed, 7111 insertions(+)
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/controls.c
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>>>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h
>>>
>>> diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
>>> b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
>>> new file mode 100644
>>> index ..4f03949aecf3
>>> --- /dev/null
>>> +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
>>> @@ -0,0 +1,2016 @@
>>
>> 
>>
>>> +static int start_streaming(struct vb2_queue *vq, unsigned int count)
>>> +{
>>> +   struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
>>> +   int ret;
>>> +   int parameter_size;
>>> +
>>> +   v4l2_dbg(1, bcm2835_v4l2_debug, >v4l2_dev, "%s: dev:%p\n",
>>> +__func__, dev);
>>> +
>>> +   /* ensure a format has actually been set */
>>> +   if (dev->capture.port == NULL)
>>> +   return -EINVAL;
>>
>> Standard mistake. If start_streaming returns an error, then it should call 
>> vb2_buffer_done
>> for all queued buffers with state VB2_BUF_STATE_QUEUED. Otherwise the buffer 
>> administration
>> gets unbalanced.
> 
> OK.
> This is an error path that I'm not convinced can ever be followed, just 
> defensive programming. It may be a candidate for just removing, but yes 
> otherwise it needs to be fixed to do the right thing.

It's not for this specific 'if', it is for all the paths where start_streaming 
or
stop_streaming can return a non-0 code.

Sorry if that was clear to you already, I just mention this to be unambiguous.



>>> +   f->pixelformat = fmt->fourcc;
>>> +   f->flags = fmt->flags;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
>>> +   struct v4l2_format *f)
>>> +{
>>> +   struct bm2835_mmal_dev *dev = video_drvdata(file);
>>> +
>>> +   f->fmt.pix.width = dev->capture.width;
>>> +   f->fmt.pix.height = dev->capture.height;
>>> +   f->fmt.pix.field = V4L2_FIELD_NONE;
>>> +   f->fmt.pix.pixelformat = dev->capture.fmt->fourcc;
>>> +   f->fmt.pix.bytesperline = dev->capture.stride;
>>> +   f->fmt.pix.sizeimage = dev->capture.buffersize;
>>> +
>>> +   if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_RGB24)
>>> +   f->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
>>> +   else if (dev->capture.fmt->fourcc == V4L2_PIX_FMT_JPEG)
>>> +   f->fmt.pix.colorspace = V4L2_COLORSPACE_JPEG;
>>> +   else
>>> +   f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
>>
>> Colorspace has nothing to do with the pixel format. It should come from the
>> sensor/video receiver.
>>
>> If this 

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

[Mauro Carvalho Chehab]

Em Sun, 5 Feb 2017 22:15:21 +
Dave Stevenson  escreveu:

> Hi Mauro.
> 
> I'm going to stick my head above the parapet as one of the original 
> authors back when I worked at Broadcom.
> As it happens I started working at Raspberry Pi last Monday, so that 
> puts me in a place where I can work on this again a bit more. (The last 
> two years have been just a spare time support role).
> Whilst I have done kernel development work in various roles, it's all 
> been downstream so I've not been that active on these lists before.
> 
> All formatting/checkpatch comments noted.
> Checkpatch was whinging when this was first written around December 2013 
> about long lines, so many got broken up to shut it up. Views on code 
> style and checkpatch seem to have changed a little since then.
> I thought we had made checkpatch happy before the driver was pushed, but 
> with some of the comments still having // style I guess some slipped 
> through the net.

Checkpatch now has some checks that are only enabled with --strict.

That's because some maintainers end by accepting patches using
different criteria, because of historic reasons.

Also, please notice that checkpatch is just a tool that gives you
a hint about what's wrong, but doesn't spare manual review, as,
on some cases, we violate what's indicated there, as the real goal
of the coding style is to make the code simpler and easier to review.

> Yes chunks of this could do with refactoring to reduce the levels of 
> indentation - always more to do.
> If I've removed any formatting/style type comments in my cuts it's not 
> because I'm ignoring them, just that they're not something that needs 
> discussion (just fixing). I've only taken out the really big lumps of 
> code with no comments on.

Ok.

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

Yes.

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

If the goal was to protect some IP related to the sensors, I guess
this is not going to protect anything, as there are recent driver 
submissions on linux-media for the ov5647 driver:

https://patchwork.kernel.org/patch/9472441/

There are also open source drivers for the Sony imx219 camera
floating around for android and chromeOS:


https://chromium.googlesource.com/chromiumos/third_party/kernel/+/factory-ryu-6486.14.B-chromeos-3.14/drivers/media/i2c/soc_camera/imx219.c

https://android.googlesource.com/kernel/bcm/+/android-bcm-tetra-3.10-lollipop-wear-release/drivers/media/video/imx219.c

Plus, there's a datasheet (with another driver) available at:
https://github.com/rellimmot/Sony-IMX219-Raspberry-Pi-V2-CMOS

So, you're not really protecting IP here.

If the goal is to protect some proprietary algorithm meant to enhance
the camera captured streams, doing things like (auto-focus, auto-white 
adjustments, scaling, etc), and/or implementing codec encoders, you should,
instead, restructure such codecs as mem2mem V4L2 drivers. There are a bunch
of such codecs already for other SoC where such functions are implemented
on GPU.

If you add DMABUF capabilities and media controller to the capture
driver and to the mem2mem drivers, userspace can use the V4L2 APIs
to use those modules using the arrangements they need, without
performance impacts.

So, by obfuscating the code, you're not protecting anything. Just making
harder for RPi customers to use, as you're providing a driver that is
limited.

> 
> Thanks
>Dave
> 
> 
> On 03/02/17 18:59, Mauro Carvalho Chehab wrote:
> > HI Eric,
> >
> > Em Fri, 27 Jan 2017 13:54:58 -0800
> > Eric Anholt  escreveu:
> >  
> >> - Supports raw YUV capture, preview, JPEG and H264.
> >> - Uses videobuf2 for data transfer, using dma_buf.
> >> - Uses 3.6.10 timestamping
> >> - Camera power based on use
> >> - Uses immutable input mode on video encoder
> >>
> >> This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
> >> a15ba877dab4e61ea3fc7b006e2a73828b083c52.  
> >
> > First of all, thanks for that! Having an upstream driver for the
> > RPi camera is something that has been long waited!
> >
> > Greg was kick on merging it on staging ;) Anyway, the real review
> > will happen when the driver becomes ready to be promoted out of
> > staging. When you address the existing 

Re: [PATCHv2 08/16] atmel-isi: document device tree bindings

[Hans Verkuil]

On 02/01/2017 05:50 PM, Rob Herring wrote:
> On Mon, Jan 30, 2017 at 03:06:20PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Document the device tree bindings for this driver.
> 
> Bindings document h/w not drivers.

Fixed.

> 
>>
>> Mostly copied from the atmel-isc bindings.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  .../devicetree/bindings/media/atmel-isi.txt| 91 
>> +-
>>  1 file changed, 56 insertions(+), 35 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> index 251f008..d1934b4 100644
>> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -1,51 +1,72 @@
>> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> ---
>> +Atmel Image Sensor Interface (ISI)
>> +--
>>  
>> -Required properties:
>> -- compatible: must be "atmel,at91sam9g45-isi"
>> -- reg: physical base address and length of the registers set for the device;
>> -- interrupts: should contain IRQ line for the ISI;
>> -- clocks: list of clock specifiers, corresponding to entries in
>> -  the clock-names property;
>> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> +Required properties for ISI:
>> +- compatible
>> +Must be "atmel,at91sam9g45-isi".
>> +- reg
>> +Physical base address and length of the registers set for the device.
>> +- interrupts
>> +Should contain IRQ line for the ISI.
>> +- clocks
>> +List of clock specifiers, corresponding to entries in
>> +the clock-names property;
>> +Please refer to clock-bindings.txt.
>> +- clock-names
>> +Required elements: "isi_clk".
>> +- #clock-cells
>> +Should be 0.
> 
> This reformatting is unrelated and the old form was more standard for 
> bindings (not that we have any real standard).

OK, I went back to the old format.

> 
>> +- pinctrl-names, pinctrl-0
>> +Please refer to pinctrl-bindings.txt.
>>  
>>  ISI supports a single port node with parallel bus. It should contain one
>>  'port' child node with child 'endpoint' node. Please refer to the bindings
>>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
>>  
>>  Example:
>> -isi: isi@f0034000 {
>> -compatible = "atmel,at91sam9g45-isi";
>> -reg = <0xf0034000 0x4000>;
>> -interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>>  
>> -clocks = <_clk>;
>> -clock-names = "isi_clk";
>> +isi: isi@f0034000 {
>> +compatible = "atmel,at91sam9g45-isi";
>> +reg = <0xf0034000 0x4000>;
>> +interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_isi_data_0_7>;
>> +clocks = <_clk>;
>> +clock-names = "isi_clk";
>> +status = "ok";
>> +port {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +isi_0: endpoint {
>> +reg = <0>;
> 
> Drop reg.

Is that because that is the default?

> 
>> +remote-endpoint = <_0>;
>> +bus-width = <8>;
>> +vsync-active = <1>;
>> +hsync-active = <1>;
> 
> Which side of the connect is supposed to define these?

The Atmel ISI side. In the v3 of this patch these properties are
explicitly documented.

> 
>> +};
>> +};
>> +};
>> +
>> +i2c1: i2c@f0018000 {
>> +status = "okay";
>>  
>> +ov2640: camera@0x30 {
> 
> Drop the '0x'.

Done.

> 
>> +compatible = "ovti,ov2640";
>> +reg = <0x30>;
>>  pinctrl-names = "default";
>> -pinctrl-0 = <_isi>;
>> +pinctrl-0 = <_pck0_as_isi_mck _sensor_power 
>> _sensor_reset>;
>> +resetb-gpios = < 11 GPIO_ACTIVE_LOW>;
>> +pwdn-gpios = < 13 GPIO_ACTIVE_HIGH>;
>> +clocks = <>;
>> +clock-names = "xvclk";
>> +assigned-clocks = <>;
>> +assigned-clock-rates = <2500>;
>>  
>>  port {
>> -#address-cells = <1>;
>> -#size-cells = <0>;
>> -
>> -isi_0: endpoint {
>> -remote-endpoint = <_0>;
>> +ov2640_0: endpoint {
>> +remote-endpoint = <_0>;
>>  bus-width = <8>;
> 
> It is pointless to define bus-width at both ends.

No, this is separate for each end. See Sakari's reply.

Thanks for the review!

Regards,

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

Re: [PATCHv2 10/16] ov2640: enable clock and fix power/reset

[Hans Verkuil]

On 01/31/2017 11:20 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> Thank you for the patchset!
> 
> On Mon, Jan 30, 2017 at 03:06:22PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Convert v4l2_clk to normal clk, enable the clock and fix the power/reset
>> handling.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/i2c/ov2640.c | 80 
>> +-
>>  1 file changed, 29 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
>> index 83f88ef..565742b 100644
>> --- a/drivers/media/i2c/ov2640.c
>> +++ b/drivers/media/i2c/ov2640.c
>> @@ -16,15 +16,14 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>>  #include 
>>  #include 
>>  
>> -#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -284,7 +283,7 @@ struct ov2640_priv {
>>  struct v4l2_subdev  subdev;
>>  struct v4l2_ctrl_handlerhdl;
>>  u32 cfmt_code;
>> -struct v4l2_clk *clk;
>> +struct clk  *clk;
> 
> Nice!
> 
>>  const struct ov2640_win_size*win;
>>  
>>  struct gpio_desc *resetb_gpio;
>> @@ -656,8 +655,9 @@ static int ov2640_mask_set(struct i2c_client *client,
>>  return i2c_smbus_write_byte_data(client, reg, val);
>>  }
>>  
>> -static int ov2640_reset(struct i2c_client *client)
>> +static int ov2640_reset(struct v4l2_subdev *sd, u32 val)
>>  {
>> +struct i2c_client *client = v4l2_get_subdevdata(sd);
>>  int ret;
>>  const struct regval_list reset_seq[] = {
>>  {BANK_SEL, BANK_SEL_SENS},
>> @@ -735,21 +735,6 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
>>  }
>>  #endif
>>  
>> -static int ov2640_s_power(struct v4l2_subdev *sd, int on)
>> -{
>> -struct i2c_client *client = v4l2_get_subdevdata(sd);
>> -struct ov2640_priv *priv = to_ov2640(client);
>> -
>> -gpiod_direction_output(priv->pwdn_gpio, !on);
>> -if (on && priv->resetb_gpio) {
>> -/* Active the resetb pin to perform a reset pulse */
>> -gpiod_direction_output(priv->resetb_gpio, 1);
>> -usleep_range(3000, 5000);
>> -gpiod_direction_output(priv->resetb_gpio, 0);
> 
> Do you still perform the reset sequence somewhere? This could be crucial for
> reliability.

All I can go with is what soc_camera did, and there the reset is performed only
once, when the sensor driver is bound to soc_camera. This is the equivalent of
that. The reset is now performed in ov2640_init_gpio although it doesn't do a
full reset there. See more about this below.

> 
>> -}
>> -return 0;
>> -}
>> -
>>  /* Select the nearest higher resolution for capture */
>>  static const struct ov2640_win_size *ov2640_select_win(u32 *width, u32 
>> *height)
>>  {
>> @@ -769,9 +754,10 @@ static const struct ov2640_win_size 
>> *ov2640_select_win(u32 *width, u32 *height)
>>  return _supported_win_sizes[default_size];
>>  }
>>  
>> -static int ov2640_set_params(struct i2c_client *client, u32 *width, u32 
>> *height,
>> +static int ov2640_set_params(struct v4l2_subdev *sd, u32 *width, u32 
>> *height,
>>   u32 code)
>>  {
>> +struct i2c_client *client = v4l2_get_subdevdata(sd);
>>  struct ov2640_priv   *priv = to_ov2640(client);
>>  const struct regval_list *selected_cfmt_regs;
>>  int ret;
>> @@ -802,7 +788,7 @@ static int ov2640_set_params(struct i2c_client *client, 
>> u32 *width, u32 *height,
>>  }
>>  
>>  /* reset hardware */
>> -ov2640_reset(client);
>> +ov2640_reset(sd, 0);
>>  
>>  /* initialize the sensor with default data */
>>  dev_dbg(>dev, "%s: Init default", __func__);
>> @@ -840,7 +826,7 @@ static int ov2640_set_params(struct i2c_client *client, 
>> u32 *width, u32 *height,
>>  
>>  err:
>>  dev_err(>dev, "%s: Error %d", __func__, ret);
>> -ov2640_reset(client);
>> +ov2640_reset(sd, 0);
>>  priv->win = NULL;
>>  
>>  return ret;
>> @@ -877,7 +863,6 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>>  struct v4l2_subdev_format *format)
>>  {
>>  struct v4l2_mbus_framefmt *mf = >format;
>> -struct i2c_client *client = v4l2_get_subdevdata(sd);
>>  
>>  if (format->pad)
>>  return -EINVAL;
>> @@ -902,7 +887,7 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>>  }
>>  
>>  if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> -return ov2640_set_params(client, >width,
>> +return ov2640_set_params(sd, >width,
>>   >height, mf->code);
>>  cfg->try_fmt = *mf;
>>  return 0;
>> @@ -947,10 +932,6 @@ static int ov2640_video_probe(struct i2c_client *client)
>>  const char *devname;
>>  int ret;
>>  
>> -ret = ov2640_s_power(>subdev, 1);
>> -if (ret < 0)
>> -return ret;
>> -
>> 

Re: [PATCHv2 08/16] atmel-isi: document device tree bindings

[Hans Verkuil]

On 01/31/2017 08:30 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Mon, Jan 30, 2017 at 03:06:20PM +0100, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Document the device tree bindings for this driver.
>>
>> Mostly copied from the atmel-isc bindings.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  .../devicetree/bindings/media/atmel-isi.txt| 91 
>> +-
>>  1 file changed, 56 insertions(+), 35 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt 
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> index 251f008..d1934b4 100644
>> --- a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -1,51 +1,72 @@
>> -Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> ---
>> +Atmel Image Sensor Interface (ISI)
>> +--
>>  
>> -Required properties:
>> -- compatible: must be "atmel,at91sam9g45-isi"
>> -- reg: physical base address and length of the registers set for the device;
>> -- interrupts: should contain IRQ line for the ISI;
>> -- clocks: list of clock specifiers, corresponding to entries in
>> -  the clock-names property;
>> -- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> +Required properties for ISI:
>> +- compatible
>> +Must be "atmel,at91sam9g45-isi".
>> +- reg
>> +Physical base address and length of the registers set for the device.
>> +- interrupts
>> +Should contain IRQ line for the ISI.
>> +- clocks
>> +List of clock specifiers, corresponding to entries in
>> +the clock-names property;
>> +Please refer to clock-bindings.txt.
>> +- clock-names
>> +Required elements: "isi_clk".
>> +- #clock-cells
>> +Should be 0.
> 
> #clock-cells can't be found in the example. Does the ISI block provide a
> #clock?

Oops, left-over from the atmel-isc.txt bindings. Removed.

> 
>> +- pinctrl-names, pinctrl-0
>> +Please refer to pinctrl-bindings.txt.
>>  
>>  ISI supports a single port node with parallel bus. It should contain one
>>  'port' child node with child 'endpoint' node. Please refer to the bindings
>>  defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
> 
> We haven't documented exactly which properties are relevant for parallel
> interfaces. I think we should, but until that's done we should explicitly
> document which endpoint properties are mandatory and which are optional.
> 
> Such as in Documentation/devicetree/bindings/media/i2c/nokia,smia.txt .

Done.

Thanks,

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

Re: [PATCH RESEND v7 2/2] Add support for OV5647 sensor.

[Ramiro Oliveira]

Hi Sakari,

Thank you for your feedback.

On 2/3/2017 8:17 PM, Sakari Ailus wrote:
> Hi Ramiro,
> 
> Thanks for the update!
> 
> Please see some comments below... some streaming and hardware control
> related matters I missed earlier.
> 
> On Fri, Feb 03, 2017 at 06:18:33PM +, Ramiro Oliveira wrote:
>> Modes supported:
>>  - 640x480 RAW 8
>>
>> Signed-off-by: Ramiro Oliveira 
>> Acked-by: Pavel Machek 
>> ---
>>  MAINTAINERS|   7 +
>>  drivers/media/i2c/Kconfig  |  12 +
>>  drivers/media/i2c/Makefile |   1 +
>>  drivers/media/i2c/ov5647.c | 718 
>> +
>>  4 files changed, 738 insertions(+)
>>  create mode 100644 drivers/media/i2c/ov5647.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 421adffbe376..56f392fd2c39 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9101,6 +9101,13 @@ M:Harald Welte 
>>  S:  Maintained
>>  F:  drivers/char/pcmcia/cm4040_cs.*
>>  
>> +OMNIVISION OV5647 SENSOR DRIVER
>> +M:  Ramiro Oliveira 
>> +L:  linux-media@vger.kernel.org
>> +T:  git git://linuxtv.org/media_tree.git
>> +S:  Maintained
>> +F:  drivers/media/i2c/ov5647.c
>> +
>>  OMNIVISION OV7670 SENSOR DRIVER
>>  M:  Jonathan Corbet 
>>  L:  linux-media@vger.kernel.org
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index cee1dae6e014..ac388be5f9a8 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -531,6 +531,18 @@ config VIDEO_OV2659
>>To compile this driver as a module, choose M here: the
>>module will be called ov2659.
>>  
>> +config VIDEO_OV5647
>> +tristate "OmniVision OV5647 sensor support"
>> +depends on OF
>> +depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> +depends on MEDIA_CAMERA_SUPPORT
>> +---help---
>> +  This is a Video4Linux2 sensor-level driver for the OmniVision
>> +  OV5647 camera.
>> +
>> +  To compile this driver as a module, choose M here: the
>> +  module will be called ov5647.
>> +
>>  config VIDEO_OV7640
>>  tristate "OmniVision OV7640 sensor support"
>>  depends on I2C && VIDEO_V4L2
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 5bc7bbeb5499..ef2ccf65f94c 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -83,3 +83,4 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>>  obj-$(CONFIG_VIDEO_ML86V7667)   += ml86v7667.o
>>  obj-$(CONFIG_VIDEO_OV2659)  += ov2659.o
>>  obj-$(CONFIG_VIDEO_TC358743)+= tc358743.o
>> +obj-$(CONFIG_VIDEO_OV5647)  += ov5647.o
>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
>> new file mode 100644
>> index ..c2828650d3a3
>> --- /dev/null
>> +++ b/drivers/media/i2c/ov5647.c
>> @@ -0,0 +1,718 @@
>> +/*
>> + * A V4L2 driver for OmniVision OV5647 cameras.
>> + *
>> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
>> + * Copyright (C) 2011 Sylwester Nawrocki 
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2006-7 Jonathan Corbet 
>> + *
>> + * Copyright (C) 2016, Synopsys, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define SENSOR_NAME "ov5647"
>> +
>> +#define OV5647_SW_RESET 0x1003
>> +#define OV5647_REG_CHIPID_H 0x300A
>> +#define OV5647_REG_CHIPID_L 0x300B
>> +
>> +#define REG_TERM 0xfffe
>> +#define VAL_TERM 0xfe
>> +#define REG_DLY  0x
>> +
>> +#define OV5647_ROW_START0x01
>> +#define OV5647_ROW_START_MIN0
>> +#define OV5647_ROW_START_MAX2004
>> +#define OV5647_ROW_START_DEF54
>> +
>> +#define OV5647_COLUMN_START 0x02
>> +#define OV5647_COLUMN_START_MIN 0
>> +#define OV5647_COLUMN_START_MAX 2750
>> +#define OV5647_COLUMN_START_DEF 16
>> +
>> +#define OV5647_WINDOW_HEIGHT0x03
>> +#define OV5647_WINDOW_HEIGHT_MIN2
>> +#define OV5647_WINDOW_HEIGHT_MAX2006
>> +#define OV5647_WINDOW_HEIGHT_DEF1944
>> +
>> +#define OV5647_WINDOW_WIDTH 0x04
>> +#define OV5647_WINDOW_WIDTH_MIN 2
>> +#define OV5647_WINDOW_WIDTH_MAX 2752
>> +#define OV5647_WINDOW_WIDTH_DEF 2592
>> +

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

[Dave Stevenson]

Hi Hans.

On 06/02/17 09:08, Hans Verkuil wrote:

Hi Eric,

Great to see this driver appearing for upstream merging!

See below for my review comments, focusing mostly on V4L2 specifics.

On 01/27/2017 10:54 PM, Eric Anholt wrote:

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

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

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

diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
new file mode 100644
index ..4f03949aecf3
--- /dev/null
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -0,0 +1,2016 @@





+static int start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+   struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+   int ret;
+   int parameter_size;
+
+   v4l2_dbg(1, bcm2835_v4l2_debug, >v4l2_dev, "%s: dev:%p\n",
+__func__, dev);
+
+   /* ensure a format has actually been set */
+   if (dev->capture.port == NULL)
+   return -EINVAL;


Standard mistake. If start_streaming returns an error, then it should call 
vb2_buffer_done
for all queued buffers with state VB2_BUF_STATE_QUEUED. Otherwise the buffer 
administration
gets unbalanced.


OK.
This is an error path that I'm not convinced can ever be followed, just 
defensive programming. It may be a candidate for just removing, but yes 
otherwise it needs to be fixed to do the right thing.



+
+   if (enable_camera(dev) < 0) {
+   v4l2_err(>v4l2_dev, "Failed to enable camera\n");
+   return -EINVAL;
+   }
+
+   /*init_completion(>capture.frame_cmplt); */
+
+   /* enable frame capture */
+   dev->capture.frame_count = 1;
+
+   /* if the preview is not already running, wait for a few frames for AGC
+* to settle down.
+*/
+   if (!dev->component[MMAL_COMPONENT_PREVIEW]->enabled)
+   msleep(300);
+
+   /* enable the connection from camera to encoder (if applicable) */
+   if (dev->capture.camera_port != dev->capture.port
+   && dev->capture.camera_port) {
+   ret = vchiq_mmal_port_enable(dev->instance,
+dev->capture.camera_port, NULL);
+   if (ret) {
+   v4l2_err(>v4l2_dev,
+"Failed to enable encode tunnel - error %d\n",
+ret);
+   return -1;


Use a proper error, not -1.


+   }
+   }
+
+   /* Get VC timestamp at this point in time */
+   parameter_size = sizeof(dev->capture.vc_start_timestamp);
+   if (vchiq_mmal_port_parameter_get(dev->instance,
+ dev->capture.camera_port,
+ MMAL_PARAMETER_SYSTEM_TIME,
+ >capture.vc_start_timestamp,
+ _size)) {
+   v4l2_err(>v4l2_dev,
+

[PATCHv4 9/9] arm: sti: update sti-cec for HPD notifier support

[Hans Verkuil]

From: Benjamin Gaignard 

To use HPD notifier sti CEC driver needs to get phandle
of the hdmi device.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
CC: devicet...@vger.kernel.org
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 
 arch/arm/boot/dts/stih410.dtsi| 13 +
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi 
b/arch/arm/boot/dts/stih407-family.dtsi
index c8b2944e304a..592d23538346 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -756,18 +756,6 @@
 <_s_c0_flexgen CLK_ETH_PHY>;
};
 
-   cec: sti-cec@094a087c {
-   compatible = "st,stih-cec";
-   reg = <0x94a087c 0x64>;
-   clocks = <_sysin>;
-   clock-names = "cec-clk";
-   interrupts = ;
-   interrupt-names = "cec-irq";
-   pinctrl-names = "default";
-   pinctrl-0 = <_cec0_default>;
-   resets = < STIH407_LPM_SOFTRESET>;
-   };
-
rng10: rng@08a89000 {
compatible  = "st,rng";
reg = <0x08a89000 0x1000>;
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 281a12424cf6..e8c01f77be80 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -259,5 +259,18 @@
clocks = <_sysin>;
interrupts = ;
};
+
+   sti-cec@094a087c {
+   compatible = "st,stih-cec";
+   reg = <0x94a087c 0x64>;
+   clocks = <_sysin>;
+   clock-names = "cec-clk";
+   interrupts = ;
+   interrupt-names = "cec-irq";
+   pinctrl-names = "default";
+   pinctrl-0 = <_cec0_default>;
+   resets = < STIH407_LPM_SOFTRESET>;
+   st,hdmi-handle = <_hdmi>;
+   };
};
 };
-- 
2.11.0

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

[PATCHv4 0/9] video/exynos/sti/cec: add HPD state notifier & use in drivers

[Hans Verkuil]

From: Hans Verkuil 

This patch series adds the hotplug detect notifier code, based on Russell's 
code:

https://patchwork.kernel.org/patch/9277043/

It adds support for it to the exynos_hdmi drm driver, adds support for
it to the CEC framework and finally adds support to the s5p-cec driver,
which now can be moved out of staging.

Also included is similar code for the STI platform, contributed by
Benjamin Gaignard.

Tested the exynos code with my Odroid U3 exynos4 devboard.

Comments are welcome. I'd like to get this in for the 4.12 kernel as
this is a missing piece needed to integrate CEC drivers.

Daniel, who should merge this? The HPD notifier code is in drivers/video
(correctly, I think), but it is initially only used by CEC drivers which
are in drivers/media (since CEC is part of the media subsystem).

It would be easiest to merge it all through the media subsystem, but in
that case I need Acks from you.

The alternative is to merge the drivers/video and drivers/gpu patches
via you and the others via media. It's a bit painful though due to the
dependencies.

Regards,

Hans

Changes since v3:
- Added the STI patches
- Split the exynos4 binding patches in one for documentation and one
  for the dts change itself, also use the correct subject and CC to
  the correct mailinglists (I hope :-) )

Changes since v2:
- Split off the dts changes of the s5p-cec patch into a separate patch
- Renamed HPD_NOTIFIERS to HPD_NOTIFIER to be consistent with the name
  of the source.

Changes since v1:

Renamed HDMI notifier to HPD (hotplug detect) notifier since this code is
not HDMI specific, but is interesting for any video source that has to
deal with hotplug detect and EDID/ELD (HDMI, DVI, VGA, DP, ).
Only the use with CEC adapters is HDMI specific, but the HPD notifier
is more generic.


Benjamin Gaignard (3):
  sti: hdmi: add HPD notifier support
  stih-cec: add HPD notifier support
  arm: sti: update sti-cec for HPD notifier support

Hans Verkuil (6):
  video: add hotplug detect notifier support
  exynos_hdmi: add HPD notifier support
  cec: integrate HPD notifier support
  ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi
  s5p-cec.txt: document the HDMI controller phandle
  s5p-cec: add hpd-notifier support, move out of staging

 .../devicetree/bindings/media/s5p-cec.txt  |   2 +
 .../devicetree/bindings/media/stih-cec.txt |   2 +
 arch/arm/boot/dts/exynos4.dtsi |   1 +
 arch/arm/boot/dts/stih407-family.dtsi  |  12 --
 arch/arm/boot/dts/stih410.dtsi |  13 ++
 drivers/gpu/drm/exynos/Kconfig |   1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c   |  23 +++-
 drivers/gpu/drm/sti/Kconfig|   1 +
 drivers/gpu/drm/sti/sti_hdmi.c |  14 +++
 drivers/gpu/drm/sti/sti_hdmi.h |   3 +
 drivers/media/cec/cec-core.c   |  50 
 drivers/media/platform/Kconfig |  28 +
 drivers/media/platform/Makefile|   2 +
 .../media => media/platform}/s5p-cec/Makefile  |   0
 .../platform}/s5p-cec/exynos_hdmi_cec.h|   0
 .../platform}/s5p-cec/exynos_hdmi_cecctrl.c|   0
 .../media => media/platform}/s5p-cec/regs-cec.h|   0
 .../media => media/platform}/s5p-cec/s5p_cec.c |  35 +-
 .../media => media/platform}/s5p-cec/s5p_cec.h |   3 +
 .../st-cec => media/platform/sti/cec}/Makefile |   0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   |  31 -
 drivers/staging/media/Kconfig  |   4 -
 drivers/staging/media/Makefile |   2 -
 drivers/staging/media/s5p-cec/Kconfig  |   9 --
 drivers/staging/media/s5p-cec/TODO |   7 --
 drivers/staging/media/st-cec/Kconfig   |   8 --
 drivers/staging/media/st-cec/TODO  |   7 --
 drivers/video/Kconfig  |   3 +
 drivers/video/Makefile |   1 +
 drivers/video/hpd-notifier.c   | 134 +
 include/linux/hpd-notifier.h   | 109 +
 include/media/cec.h|  15 +++
 32 files changed, 460 insertions(+), 60 deletions(-)
 rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h 
(100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c 
(100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c 
(93%)
 delete mode 100644 

[PATCHv4 6/9] s5p-cec: add hpd-notifier support, move out of staging

[Hans Verkuil]

From: Hans Verkuil 

By using the HPD notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.

Update the bindings documenting the new hdmi phandle and
update exynos4.dtsi accordingly.

Tested with my Odroid U3.

Signed-off-by: Hans Verkuil 
Tested-by: Marek Szyprowski 
CC: linux-samsung-...@vger.kernel.org
CC: Krzysztof Kozlowski 
---
 drivers/media/platform/Kconfig | 18 +++
 drivers/media/platform/Makefile|  1 +
 .../media => media/platform}/s5p-cec/Makefile  |  0
 .../platform}/s5p-cec/exynos_hdmi_cec.h|  0
 .../platform}/s5p-cec/exynos_hdmi_cecctrl.c|  0
 .../media => media/platform}/s5p-cec/regs-cec.h|  0
 .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++
 .../media => media/platform}/s5p-cec/s5p_cec.h |  3 ++
 drivers/staging/media/Kconfig  |  2 --
 drivers/staging/media/Makefile |  1 -
 drivers/staging/media/s5p-cec/Kconfig  |  9 --
 drivers/staging/media/s5p-cec/TODO |  7 -
 12 files changed, 52 insertions(+), 24 deletions(-)
 rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h 
(100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c 
(100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
 delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
 delete mode 100644 drivers/staging/media/s5p-cec/TODO

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 0245af0b76e0..9920726f14d2 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -406,6 +406,24 @@ config VIDEO_TI_SC
 config VIDEO_TI_CSC
tristate
 
+menuconfig V4L_CEC_DRIVERS
+   bool "Platform HDMI CEC drivers"
+   depends on MEDIA_CEC_SUPPORT
+
+if V4L_CEC_DRIVERS
+
+config VIDEO_SAMSUNG_S5P_CEC
+   tristate "Samsung S5P CEC driver"
+   depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS 
|| COMPILE_TEST)
+   select HPD_NOTIFIER
+   ---help---
+ This is a driver for Samsung S5P HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
+endif #V4L_CEC_DRIVERS
+
 menuconfig V4L_TEST_DRIVERS
bool "Media test drivers"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 5b3cb271d2b8..ad3bf22bfeae 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)  += s5p-jpeg/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/
 
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)+= s5p-g2d/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
 
 obj-$(CONFIG_VIDEO_STI_BDISP)  += sti/bdisp/
diff --git a/drivers/staging/media/s5p-cec/Makefile 
b/drivers/media/platform/s5p-cec/Makefile
similarity index 100%
rename from drivers/staging/media/s5p-cec/Makefile
rename to drivers/media/platform/s5p-cec/Makefile
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h 
b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c 
b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
diff --git a/drivers/staging/media/s5p-cec/regs-cec.h 
b/drivers/media/platform/s5p-cec/regs-cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/regs-cec.h
rename to drivers/media/platform/s5p-cec/regs-cec.h
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c 
b/drivers/media/platform/s5p-cec/s5p_cec.c
similarity index 89%
rename from drivers/staging/media/s5p-cec/s5p_cec.c
rename to drivers/media/platform/s5p-cec/s5p_cec.c
index 2a07968b5ac6..2014f792eceb 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.c
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -14,15 +14,18 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "exynos_hdmi_cec.h"

[PATCHv4 8/9] stih-cec: add HPD notifier support

[Hans Verkuil]

From: Benjamin Gaignard 

By using the HPD notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.

Update the bindings documentation the new hdmi phandle.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
CC: devicet...@vger.kernel.org
---
 .../devicetree/bindings/media/stih-cec.txt |  2 ++
 drivers/media/platform/Kconfig | 10 +++
 drivers/media/platform/Makefile|  1 +
 .../st-cec => media/platform/sti/cec}/Makefile |  0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   | 31 +++---
 drivers/staging/media/Kconfig  |  2 --
 drivers/staging/media/Makefile |  1 -
 drivers/staging/media/st-cec/Kconfig   |  8 --
 drivers/staging/media/st-cec/TODO  |  7 -
 9 files changed, 41 insertions(+), 21 deletions(-)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c 
(93%)
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/TODO

diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt 
b/Documentation/devicetree/bindings/media/stih-cec.txt
index 71c4b2f4bcef..7d82121d148a 100644
--- a/Documentation/devicetree/bindings/media/stih-cec.txt
+++ b/Documentation/devicetree/bindings/media/stih-cec.txt
@@ -9,6 +9,7 @@ Required properties:
  - pinctrl-names: Contains only one value - "default"
  - pinctrl-0: Specifies the pin control groups used for CEC hardware.
  - resets: Reference to a reset controller
+ - st,hdmi-handle: Phandle to the HMDI controller
 
 Example for STIH407:
 
@@ -22,4 +23,5 @@ sti-cec@094a087c {
pinctrl-names = "default";
pinctrl-0 = <_cec0_default>;
resets = < STIH407_LPM_SOFTRESET>;
+   st,hdmi-handle = <>;
 };
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9920726f14d2..46db8a32a931 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -422,6 +422,16 @@ config VIDEO_SAMSUNG_S5P_CEC
  CEC bus is present in the HDMI connector and enables communication
  between compatible devices.
 
+config VIDEO_STI_HDMI_CEC
+   tristate "STMicroelectronics STiH4xx HDMI CEC driver"
+   depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
+   select HPD_NOTIFIER
+   ---help---
+ This is a driver for STIH4xx HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
 endif #V4L_CEC_DRIVERS
 
 menuconfig V4L_TEST_DRIVERS
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index ad3bf22bfeae..01b689c10f69 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)+= exynos-gsc/
 obj-$(CONFIG_VIDEO_STI_BDISP)  += sti/bdisp/
 obj-$(CONFIG_VIDEO_STI_HVA)+= sti/hva/
 obj-$(CONFIG_DVB_C8SECTPFE)+= sti/c8sectpfe/
+obj-$(CONFIG_VIDEO_STI_HDMI_CEC)   += sti/cec/
 
 obj-$(CONFIG_BLACKFIN)  += blackfin/
 
diff --git a/drivers/staging/media/st-cec/Makefile 
b/drivers/media/platform/sti/cec/Makefile
similarity index 100%
rename from drivers/staging/media/st-cec/Makefile
rename to drivers/media/platform/sti/cec/Makefile
diff --git a/drivers/staging/media/st-cec/stih-cec.c 
b/drivers/media/platform/sti/cec/stih-cec.c
similarity index 93%
rename from drivers/staging/media/st-cec/stih-cec.c
rename to drivers/media/platform/sti/cec/stih-cec.c
index 3c25638a9610..e50e916837fd 100644
--- a/drivers/staging/media/st-cec/stih-cec.c
+++ b/drivers/media/platform/sti/cec/stih-cec.c
@@ -1,6 +1,4 @@
 /*
- * drivers/staging/media/st-cec/stih-cec.c
- *
  * STIH4xx CEC driver
  * Copyright (C) STMicroelectronic SA 2016
  *
@@ -10,11 +8,13 @@
  * (at your option) any later version.
  */
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -129,6 +129,7 @@ struct stih_cec {
void __iomem*regs;
int irq;
u32 irq_status;
+   struct hpd_notifier *notifier;
 };
 
 static int stih_cec_adap_enable(struct cec_adapter *adap, bool enable)
@@ -303,12 +304,29 @@ static int stih_cec_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct resource *res;
struct stih_cec *cec;
+   struct device_node *np;
+   struct platform_device *hdmi_dev;
int ret;
 
cec = devm_kzalloc(dev, 

[PATCHv4 4/9] ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi

[Hans Verkuil]

From: Hans Verkuil 

Add the new hdmi phandle to exynos4.dtsi. This phandle is needed by the
s5p-cec driver to initialize the HPD notifier framework.

Tested with my Odroid U3.

Signed-off-by: Hans Verkuil 
Tested-by: Marek Szyprowski 
CC: linux-samsung-...@vger.kernel.org
CC: devicet...@vger.kernel.org
CC: Krzysztof Kozlowski 
---
 arch/arm/boot/dts/exynos4.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737baa45e..51dfcbb51b6b 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -762,6 +762,7 @@
clocks = < CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <_system_controller>;
+   samsung,hdmi-phandle = <>;
pinctrl-names = "default";
pinctrl-0 = <_cec>;
status = "disabled";
-- 
2.11.0

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

[PATCHv4 2/9] exynos_hdmi: add HPD notifier support

[Hans Verkuil]

From: Hans Verkuil 

Implement the HPD notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.

Signed-off-by: Hans Verkuil 
Tested-by: Marek Szyprowski 
---
 drivers/gpu/drm/exynos/Kconfig   |  1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c | 23 ---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d706ca4e2f02..50309409d450 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
 config DRM_EXYNOS_HDMI
bool "HDMI"
depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
+   select HPD_NOTIFIER
help
  Choose this option if you want to use Exynos HDMI for DRM.
 
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1effe71..8d48a0a21565 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -118,6 +119,7 @@ struct hdmi_context {
booldvi_mode;
struct delayed_work hotplug_work;
struct drm_display_mode current_mode;
+   struct hpd_notifier *notifier;
const struct hdmi_driver_data   *drv_data;
 
void __iomem*regs;
@@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct 
drm_connector *connector,
 {
struct hdmi_context *hdata = connector_to_hdmi(connector);
 
-   if (gpiod_get_value(hdata->hpd_gpio))
+   if (gpiod_get_value(hdata->hpd_gpio)) {
+   hpd_event_connect(hdata->notifier);
return connector_status_connected;
+   }
 
+   hpd_event_disconnect(hdata->notifier);
return connector_status_disconnected;
 }
 
@@ -848,6 +853,8 @@ static int hdmi_get_modes(struct drm_connector *connector)
edid->width_cm, edid->height_cm);
 
drm_mode_connector_update_edid_property(connector, edid);
+   hpd_event_new_edid(hdata->notifier, edid,
+   EDID_LENGTH * (1 + edid->extensions));
 
ret = drm_add_edid_modes(connector, edid);
 
@@ -1483,6 +1490,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
if (funcs && funcs->disable)
(*funcs->disable)(crtc);
 
+   hpd_event_disconnect(hdata->notifier);
cancel_delayed_work(>hotplug_work);
 
hdmiphy_disable(hdata);
@@ -1832,15 +1840,22 @@ static int hdmi_probe(struct platform_device *pdev)
}
}
 
+   hdata->notifier = hpd_notifier_get(>dev);
+   if (hdata->notifier == NULL) {
+   ret = -ENOMEM;
+   goto err_hdmiphy;
+   }
+
pm_runtime_enable(dev);
 
ret = component_add(>dev, _component_ops);
if (ret)
-   goto err_disable_pm_runtime;
+   goto err_notifier_put;
 
return ret;
 
-err_disable_pm_runtime:
+err_notifier_put:
+   hpd_notifier_put(hdata->notifier);
pm_runtime_disable(dev);
 
 err_hdmiphy:
@@ -1859,9 +1874,11 @@ static int hdmi_remove(struct platform_device *pdev)
struct hdmi_context *hdata = platform_get_drvdata(pdev);
 
cancel_delayed_work_sync(>hotplug_work);
+   hpd_event_disconnect(hdata->notifier);
 
component_del(>dev, _component_ops);
 
+   hpd_notifier_put(hdata->notifier);
pm_runtime_disable(>dev);
 
if (!IS_ERR(hdata->reg_hdmi_en))
-- 
2.11.0

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

[PATCHv4 5/9] s5p-cec.txt: document the HDMI controller phandle

[Hans Verkuil]

From: Hans Verkuil 

Update the bindings documenting the new hdmi phandle.

Signed-off-by: Hans Verkuil 
CC: linux-samsung-...@vger.kernel.org
CC: devicet...@vger.kernel.org
CC: Krzysztof Kozlowski 
---
 Documentation/devicetree/bindings/media/s5p-cec.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt 
b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d72eaa..710fc005150c 100644
--- a/Documentation/devicetree/bindings/media/s5p-cec.txt
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -15,6 +15,7 @@ Required properties:
   - clock-names : from common clock binding: must contain "hdmicec",
  corresponding to entry in the clocks property.
   - samsung,syscon-phandle - phandle to the PMU system controller
+  - samsung,hdmi-phandle - phandle to the HDMI controller
 
 Example:
 
@@ -25,6 +26,7 @@ hdmicec: cec@100B {
clocks = < CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <_system_controller>;
+   samsung,hdmi-phandle = <>;
pinctrl-names = "default";
pinctrl-0 = <_cec>;
status = "okay";
-- 
2.11.0

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

[PATCHv4 3/9] cec: integrate HPD notifier support

[Hans Verkuil]

From: Hans Verkuil 

Support the HPD notifier framework, simplifying drivers that
depend on this.

Signed-off-by: Hans Verkuil 
Tested-by: Marek Szyprowski 
---
 drivers/media/cec/cec-core.c | 50 
 include/media/cec.h  | 15 +
 2 files changed, 65 insertions(+)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index 37217e205040..f055720a1c65 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,52 @@ static void cec_devnode_unregister(struct cec_devnode 
*devnode)
put_device(>dev);
 }
 
+#ifdef CONFIG_HPD_NOTIFIER
+static u16 parse_hdmi_addr(const struct edid *edid)
+{
+   if (!edid || edid->extensions == 0)
+   return CEC_PHYS_ADDR_INVALID;
+
+   return cec_get_edid_phys_addr((u8 *)edid,
+   EDID_LENGTH * (edid->extensions + 1), NULL);
+}
+
+static int cec_hpd_notify(struct notifier_block *nb, unsigned long event,
+  void *data)
+{
+   struct cec_adapter *adap = container_of(nb, struct cec_adapter, nb);
+   struct hpd_notifier *n = data;
+   unsigned int phys;
+
+   dprintk(1, "event %lu\n", event);
+
+   switch (event) {
+   case HPD_DISCONNECTED:
+   cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+   break;
+
+   case HPD_NEW_EDID:
+   phys = parse_hdmi_addr(n->edid);
+   cec_s_phys_addr(adap, phys, false);
+   break;
+   }
+
+   return NOTIFY_OK;
+}
+
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+   struct hpd_notifier *notifier)
+{
+   if (WARN_ON(!adap->devnode.registered))
+   return;
+
+   adap->nb.notifier_call = cec_hpd_notify;
+   adap->notifier = notifier;
+   hpd_notifier_register(adap->notifier, >nb);
+}
+EXPORT_SYMBOL_GPL(cec_register_hpd_notifier);
+#endif
+
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 void *priv, const char *name, u32 caps,
 u8 available_las)
@@ -343,6 +389,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
adap->rc = NULL;
 #endif
debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HPD_NOTIFIER
+   if (adap->notifier)
+   hpd_notifier_unregister(adap->notifier, >nb);
+#endif
cec_devnode_unregister(>devnode);
 }
 EXPORT_SYMBOL_GPL(cec_unregister_adapter);
diff --git a/include/media/cec.h b/include/media/cec.h
index 96a0aa770d61..f87a07ee36b3 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -28,6 +28,11 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_HPD_NOTIFIER
+#include 
+#include 
+#include 
+#endif
 #include 
 #include 
 
@@ -173,6 +178,11 @@ struct cec_adapter {
bool passthrough;
struct cec_log_addrs log_addrs;
 
+#ifdef CONFIG_HPD_NOTIFIER
+   struct hpd_notifier *notifier;
+   struct notifier_block   nb;
+#endif
+
struct dentry *cec_dir;
struct dentry *status_file;
 
@@ -213,6 +223,11 @@ void cec_transmit_done(struct cec_adapter *adap, u8 
status, u8 arb_lost_cnt,
   u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
 void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
 
+#ifdef CONFIG_HPD_NOTIFIER
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+   struct hpd_notifier *notifier);
+#endif
+
 #else
 
 static inline int cec_register_adapter(struct cec_adapter *adap,
-- 
2.11.0

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

[PATCHv4 1/9] video: add hotplug detect notifier support

[Hans Verkuil]

From: Hans Verkuil 

Add support for video hotplug detect and EDID/ELD notifiers, which is used
to convey information from video drivers to their CEC and audio counterparts.

Based on an earlier version from Russell King:

https://patchwork.kernel.org/patch/9277043/

The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
of a video device.

When a new notifier is registered the current state will be reported to
that notifier at registration time.

Signed-off-by: Hans Verkuil 
Tested-by: Marek Szyprowski 
---
 drivers/video/Kconfig|   3 +
 drivers/video/Makefile   |   1 +
 drivers/video/hpd-notifier.c | 134 +++
 include/linux/hpd-notifier.h | 109 +++
 4 files changed, 247 insertions(+)
 create mode 100644 drivers/video/hpd-notifier.c
 create mode 100644 include/linux/hpd-notifier.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af999893..a3a58d8481e9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
 config HDMI
bool
 
+config HPD_NOTIFIER
+   bool
+
 if VT
source "drivers/video/console/Kconfig"
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17d6456..2697ae5c4166 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_VGASTATE)+= vgastate.o
 obj-$(CONFIG_HDMI)+= hdmi.o
+obj-$(CONFIG_HPD_NOTIFIER)+= hpd-notifier.o
 
 obj-$(CONFIG_VT) += console/
 obj-$(CONFIG_LOGO)   += logo/
diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
new file mode 100644
index ..971e823ead00
--- /dev/null
+++ b/drivers/video/hpd-notifier.c
@@ -0,0 +1,134 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static LIST_HEAD(hpd_notifiers);
+static DEFINE_MUTEX(hpd_notifiers_lock);
+
+struct hpd_notifier *hpd_notifier_get(struct device *dev)
+{
+   struct hpd_notifier *n;
+
+   mutex_lock(_notifiers_lock);
+   list_for_each_entry(n, _notifiers, head) {
+   if (n->dev == dev) {
+   mutex_unlock(_notifiers_lock);
+   kref_get(>kref);
+   return n;
+   }
+   }
+   n = kzalloc(sizeof(*n), GFP_KERNEL);
+   if (!n)
+   goto unlock;
+   n->dev = dev;
+   mutex_init(>lock);
+   BLOCKING_INIT_NOTIFIER_HEAD(>notifiers);
+   kref_init(>kref);
+   list_add_tail(>head, _notifiers);
+unlock:
+   mutex_unlock(_notifiers_lock);
+   return n;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_get);
+
+static void hpd_notifier_release(struct kref *kref)
+{
+   struct hpd_notifier *n =
+   container_of(kref, struct hpd_notifier, kref);
+
+   list_del(>head);
+   kfree(n->edid);
+   kfree(n);
+}
+
+void hpd_notifier_put(struct hpd_notifier *n)
+{
+   mutex_lock(_notifiers_lock);
+   kref_put(>kref, hpd_notifier_release);
+   mutex_unlock(_notifiers_lock);
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_put);
+
+int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
+{
+   int ret = blocking_notifier_chain_register(>notifiers, nb);
+
+   if (ret)
+   return ret;
+   kref_get(>kref);
+   mutex_lock(>lock);
+   if (n->connected) {
+   blocking_notifier_call_chain(>notifiers, HPD_CONNECTED, n);
+   if (n->edid_size)
+   blocking_notifier_call_chain(>notifiers, 
HPD_NEW_EDID, n);
+   if (n->has_eld)
+   blocking_notifier_call_chain(>notifiers, 
HPD_NEW_ELD, n);
+   }
+   mutex_unlock(>lock);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_register);
+
+int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
+{
+   int ret = blocking_notifier_chain_unregister(>notifiers, nb);
+
+   if (ret == 0)
+   hpd_notifier_put(n);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
+
+void hpd_event_connect(struct hpd_notifier *n)
+{
+   mutex_lock(>lock);
+   n->connected = true;
+   blocking_notifier_call_chain(>notifiers, HPD_CONNECTED, n);
+   mutex_unlock(>lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_connect);
+
+void hpd_event_disconnect(struct hpd_notifier *n)
+{
+   mutex_lock(>lock);
+   n->connected = false;
+   n->has_eld = false;
+   n->edid_size = 0;
+   blocking_notifier_call_chain(>notifiers, HPD_DISCONNECTED, n);
+   mutex_unlock(>lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_disconnect);
+
+int hpd_event_new_edid(struct hpd_notifier *n, const void *edid, size_t size)
+{
+   mutex_lock(>lock);
+   if (n->edid_allocated_size < size) {
+   void *p = kmalloc(size, GFP_KERNEL);
+
+   if (p == 

[PATCHv4 7/9] sti: hdmi: add HPD notifier support

[Hans Verkuil]

From: Benjamin Gaignard 

Implement the HPD notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.

Signed-off-by: Benjamin Gaignard 
Signed-off-by: Hans Verkuil 
---
 drivers/gpu/drm/sti/Kconfig|  1 +
 drivers/gpu/drm/sti/sti_hdmi.c | 14 ++
 drivers/gpu/drm/sti/sti_hdmi.h |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index acd72865feac..f5c9572b4169 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -8,5 +8,6 @@ config DRM_STI
select DRM_PANEL
select FW_LOADER
select SND_SOC_HDMI_CODEC if SND_SOC
+   select HPD_NOTIFIER
help
  Choose this option to enable DRM on STM stiH4xx chipset
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 376b0763c874..d32a38362391 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -786,6 +786,8 @@ static void sti_hdmi_disable(struct drm_bridge *bridge)
clk_disable_unprepare(hdmi->clk_pix);
 
hdmi->enabled = false;
+
+   hpd_event_disconnect(hdmi->notifier);
 }
 
 static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
@@ -892,6 +894,9 @@ static int sti_hdmi_connector_get_modes(struct 
drm_connector *connector)
if (!edid)
goto fail;
 
+   hpd_event_new_edid(hdmi->notifier, edid,
+  EDID_LENGTH * (edid->extensions + 1));
+
count = drm_add_edid_modes(connector, edid);
drm_mode_connector_update_edid_property(connector, edid);
drm_edid_to_eld(connector, edid);
@@ -949,10 +954,12 @@ sti_hdmi_connector_detect(struct drm_connector 
*connector, bool force)
 
if (hdmi->hpd) {
DRM_DEBUG_DRIVER("hdmi cable connected\n");
+   hpd_event_connect(hdmi->notifier);
return connector_status_connected;
}
 
DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
+   hpd_event_disconnect(hdmi->notifier);
return connector_status_disconnected;
 }
 
@@ -1464,6 +1471,10 @@ static int sti_hdmi_probe(struct platform_device *pdev)
goto release_adapter;
}
 
+   hdmi->notifier = hpd_notifier_get(>dev);
+   if (!hdmi->notifier)
+   goto release_adapter;
+
hdmi->reset = devm_reset_control_get(dev, "hdmi");
/* Take hdmi out of reset */
if (!IS_ERR(hdmi->reset))
@@ -1483,11 +1494,14 @@ static int sti_hdmi_remove(struct platform_device *pdev)
 {
struct sti_hdmi *hdmi = dev_get_drvdata(>dev);
 
+   hpd_event_disconnect(hdmi->notifier);
+
i2c_put_adapter(hdmi->ddc_adapt);
if (hdmi->audio_pdev)
platform_device_unregister(hdmi->audio_pdev);
component_del(>dev, _hdmi_ops);
 
+   hpd_notifier_put(hdmi->notifier);
return 0;
 }
 
diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h
index 119bc3582ac7..2109c97eb933 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.h
+++ b/drivers/gpu/drm/sti/sti_hdmi.h
@@ -8,6 +8,7 @@
 #define _STI_HDMI_H_
 
 #include 
+#include 
 #include 
 
 #include 
@@ -77,6 +78,7 @@ static const struct drm_prop_enum_list 
colorspace_mode_names[] = {
  * @audio_pdev: ASoC hdmi-codec platform device
  * @audio: hdmi audio parameters.
  * @drm_connector: hdmi connector
+ * @notifier: hotplug detect notifier
  */
 struct sti_hdmi {
struct device dev;
@@ -102,6 +104,7 @@ struct sti_hdmi {
struct platform_device *audio_pdev;
struct hdmi_audio_params audio;
struct drm_connector *drm_connector;
+   struct hpd_notifier *notifier;
 };
 
 u32 hdmi_read(struct sti_hdmi *hdmi, int offset);
-- 
2.11.0

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

Re: [PATCH v3 12/24] add mux and video interface bridge entity functions

[Philipp Zabel]

On Sun, 2017-02-05 at 17:36 +0200, Laurent Pinchart wrote:
> Hi Steve,
> 
> Thank you for the patch
> 
> On Friday 06 Jan 2017 18:11:30 Steve Longerbeam wrote:
> > From: Philipp Zabel 
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  Documentation/media/uapi/mediactl/media-types.rst | 22 
> >  include/uapi/linux/media.h|  6 ++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/mediactl/media-types.rst
> > b/Documentation/media/uapi/mediactl/media-types.rst index 3e03dc2..023be29
> > 100644
> > --- a/Documentation/media/uapi/mediactl/media-types.rst
> > +++ b/Documentation/media/uapi/mediactl/media-types.rst
> > @@ -298,6 +298,28 @@ Types and flags used to represent the media graph
> > elements received on its sink pad and outputs the statistics data on
> >   its source pad.
> > 
> > +-  ..  row 29
> > +
> > +   ..  _MEDIA-ENT-F-MUX:
> > +
> > +   -  ``MEDIA_ENT_F_MUX``
> > +
> > +   - Video multiplexer. An entity capable of multiplexing must have at
> > + least two sink pads and one source pad, and must pass the video
> > + frame(s) received from the active sink pad to the source pad.
> > Video
> > + frame(s) from the inactive sink pads are discarded.
> 
> Apart from the comment made by Hans regarding the macro name, this looks good 
> to me.
> 
> > +-  ..  row 30
> > +
> > +   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
> > +
> > +   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
> > +
> > +   - Video interface bridge. A video interface bridge entity must have
> > at
> > + least one sink pad and one source pad. It receives video frame(s)
> > on
> > + its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
> > + converts them and outputs them on its source pad in another bus
> > format
> > + (eDP, MIPI CSI-2, parallel, ...).
> 
> 
> The first sentence mentions *at least* one sink pad and one source pad, and 
> the second one refers to "its sink pad" and "its source pad". This is a bit 
> confusing. An easy option would be to require a single sink and a single 
> source pad, but that would exclude bridges that combine a multiplexer.

Would it be enough to just switch to plural?

"It receives video frame(s) on its sink pads in one bus format and
converts them and outputs them on its source pads in another bus
format"?

I fear if this is made too specific to single-input single-output
devices, a whole lot of multi-input devices will have to be split up
unnecessarily. Although in cases of freely configurable multiplexers, it
might also make sense to describe them as multiple entities. Especially
if they can run in parallel with different configurations.

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

What media entity function would you suggest to use for the IPU CSI,
which basically is
 - a video interface bridge, because it converts media bus formats from
   an external (up to) 20-bit parallel bus to an internal 128-bit bus,
   with the option to either expand/compand/pack >8-bit per component
   pixel formats (so parts of a write pixel formatter)
 - also a video scaler, because it can crop and reduce width and/or
   height to 1/2 the original size

I wouldn't call it a transceiver, as it is completely contained inside
the SoC.

regards
Philipp

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

Re: [PATCHv2] dt: bindings: Add support for CSI1 bus

[Pavel Machek]

Hi!

> >  - bus-type: data bus type. Possible values are:
> > -  0 - MIPI CSI2
> > -  1 - parallel / Bt656
> > -  2 - MIPI CSI1
> > -  3 - CCP2
> > +  0 - autodetect based on other properties (MIPI CSI2, parallel, Bt656)
> 
> In the meantime, I also realised that we need to separate MIPI C-PHY and
> D-PHY from each other. So I think we'll need that property for CSI-2
> nevertheless. How about:
> 
> 0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or Bt656)
> 1 - MIPI CSI-2 C-PHY
> 2 - MIPI CSI1
> 3 - CCP2 
> 
> I wouldn't add a separate entry for the parallel case as there are plenty of
> existing DT binaries with parallel interface configuration without phy-type
> property. They will need to be continue to be supported anyway, so there's
> not too much value in adding a type for that purpose.
> 
> I do find this a bit annoying; we should have had something like phy-type
> from day one rather than try to guess which phy is being used...

Ok, v3 is in the mail. 

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


signature.asc
Description: Digital signature

Re: [PATCH v3 13/24] platform: add video-multiplexer subdevice driver

[Hans Verkuil]

On 02/05/2017 04:48 PM, Laurent Pinchart wrote:
> Hi Steve,
> 
> On Tuesday 24 Jan 2017 18:07:55 Steve Longerbeam wrote:
>> On 01/24/2017 04:02 AM, Philipp Zabel wrote:
>>> On Fri, 2017-01-20 at 15:03 +0100, Hans Verkuil wrote:
> +
> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config
> *cfg)
> +{
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> + struct media_pad *pad;
> + int ret;
> +
> + if (vidsw->active == -1) {
> + dev_err(sd->dev, "no configuration for inactive mux\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * Retrieve media bus configuration from the entity connected to the
> +  * active input
> +  */
> + pad = media_entity_remote_pad(>pads[vidsw->active]);
> + if (pad) {
> + sd = media_entity_to_v4l2_subdev(pad->entity);
> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
> + if (ret == -ENOIOCTLCMD)
> + pad = NULL;
> + else if (ret < 0) {
> + dev_err(sd->dev, "failed to get source 
> configuration\n");
> + return ret;
> + }
> + }
> + if (!pad) {
> + /* Mirror the input side on the output side */
> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
> + if (cfg->type == V4L2_MBUS_PARALLEL ||
> + cfg->type == V4L2_MBUS_BT656)
> + cfg->flags = vidsw->endpoint[vidsw-
>> active].bus.parallel.flags;
> + }
> +
> + return 0;
> +}

 I am not certain this op is needed at all. In the current kernel this op
 is only used by soc_camera, pxa_camera and omap3isp (somewhat dubious).
 Normally this information should come from the device tree and there
 should be no need for this op.

 My (tentative) long-term plan was to get rid of this op.

 If you don't need it, then I recommend it is removed.
>>
>> Hi Hans, the imx-media driver was only calling g_mbus_config to the camera
>> sensor, and it was doing that to determine the sensor's bus type. This info
>> was already available from parsing a v4l2_of_endpoint from the sensor node.
>> So it was simple to remove the g_mbus_config calls, and instead rely on the
>> parsed sensor v4l2_of_endpoint.
> 
> That's not a good point. The imx-media driver must not parse the sensor DT 
> node as it is not aware of what bindings the sensor is compatible with. 
> Information must instead be queried from the sensor subdev at runtime, 
> through 
> the g_mbus_config() operation.
> 
> Of course, if you can get the information from the imx-media DT node, that's 
> certainly an option. It's only information provided by the sensor driver that 
> you have no choice but query using a subdev operation.

Shouldn't this come from the imx-media DT node? BTW, why is omap3isp using this?

The reason I am suspicious about this op is that it came from soc-camera and
predates the DT. The contents of v4l2_mbus_config seems very much like a HW
description to me, i.e. something that belongs in the DT.

Regards,

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

Re: [PATCHv2] dt: bindings: Add support for CSI1 bus

[Pavel Machek]

From: Sakari Ailus 

In the vast majority of cases the bus type is known to the driver(s)
since a receiver or transmitter can only support a single one. There
are cases however where different options are possible.

The existing V4L2 OF support tries to figure out the bus type and
parse the bus parameters based on that. This does not scale too well
as there are multiple serial busses that share common properties.

Some hardware also supports multiple types of busses on the same
interfaces.

Document the CSI1/CCP2 property strobe. It signifies the clock or
strobe mode.
 
Signed-off-by: Sakari Ailus 
Signed-off-by: Ivaylo Dimitrov 
Signed-off-by: Pavel Machek 
Reviewed-By: Sebastian Reichel 

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 9cd2a36..6986fde 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -76,6 +76,12 @@ Optional endpoint properties
   mode horizontal and vertical synchronization signals are provided to the
   slave device (data source) by the master device (data sink). In the master
   mode the data source device is also the source of the synchronization 
signals.
+- bus-type: data bus type. Possible values are:
+  0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or 
Bt656)
+  1 - MIPI CSI-2 C-PHY
+  2 - MIPI CSI1
+  3 - CCP2
+  Autodetection is default, and bus-type property may be omitted in that case.
 - bus-width: number of data lines actively used, valid for the parallel busses.
 - data-shift: on the parallel data busses, if bus-width is used to specify the
   number of data lines, data-shift can be used to specify which data lines are
@@ -112,7 +118,8 @@ Optional endpoint properties
   should be the combined length of data-lanes and clock-lanes properties.
   If the lane-polarities property is omitted, the value must be interpreted
   as 0 (normal). This property is valid for serial busses only.
-
+- strobe: Whether the clock signal is used as clock or strobe. Used
+  with CCP2, for instance.
 
 Example
 ---

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


signature.asc
Description: Digital signature

[PATCH] media: add operation to get configuration of "the other side" of the link

[Pavel Machek]

Normally, link configuration can be determined at probe time... but
Nokia N900 has two cameras, and can switch between them at runtime, so
that mechanism is not suitable here.

Add a hook that tells us link configuration.

Signed-off-by: Pavel Machek 

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index cf778c5..74148b9 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* generic v4l2_device notify callback notification values */
 #define V4L2_SUBDEV_IR_RX_NOTIFY   _IOW('v', 0, u32)
@@ -383,6 +384,8 @@ struct v4l2_mbus_frame_desc {
  * @s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev
  * can adjust @size to a lower value and must not write more data to the
  * buffer starting at @data than the original value of @size.
+ *
+ * @g_endpoint_config: get link configuration required by this device.
  */
 struct v4l2_subdev_video_ops {
int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 
config);
@@ -415,6 +418,8 @@ struct v4l2_subdev_video_ops {
 const struct v4l2_mbus_config *cfg);
int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
   unsigned int *size);
+   int (*g_endpoint_config)(struct v4l2_subdev *sd,
+   struct v4l2_of_endpoint *cfg);
 };
 
 /**




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


signature.asc
Description: Digital signature

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

[Hans Verkuil]

Hi Eric,

Great to see this driver appearing for upstream merging!

See below for my review comments, focusing mostly on V4L2 specifics.

On 01/27/2017 10:54 PM, Eric Anholt wrote:
> - Supports raw YUV capture, preview, JPEG and H264.
> - Uses videobuf2 for data transfer, using dma_buf.
> - Uses 3.6.10 timestamping
> - Camera power based on use
> - Uses immutable input mode on video encoder
> 
> This code comes from the Raspberry Pi kernel tree (rpi-4.9.y) as of
> a15ba877dab4e61ea3fc7b006e2a73828b083c52.
> 
> Signed-off-by: Eric Anholt 
> ---
>  .../media/platform/bcm2835/bcm2835-camera.c| 2016 
> 
>  .../media/platform/bcm2835/bcm2835-camera.h|  145 ++
>  drivers/staging/media/platform/bcm2835/controls.c  | 1345 +
>  .../staging/media/platform/bcm2835/mmal-common.h   |   53 +
>  .../media/platform/bcm2835/mmal-encodings.h|  127 ++
>  .../media/platform/bcm2835/mmal-msg-common.h   |   50 +
>  .../media/platform/bcm2835/mmal-msg-format.h   |   81 +
>  .../staging/media/platform/bcm2835/mmal-msg-port.h |  107 ++
>  drivers/staging/media/platform/bcm2835/mmal-msg.h  |  404 
>  .../media/platform/bcm2835/mmal-parameters.h   |  689 +++
>  .../staging/media/platform/bcm2835/mmal-vchiq.c| 1916 +++
>  .../staging/media/platform/bcm2835/mmal-vchiq.h|  178 ++
>  12 files changed, 7111 insertions(+)
>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/controls.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>  create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h
> 
> diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c 
> b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> new file mode 100644
> index ..4f03949aecf3
> --- /dev/null
> +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> @@ -0,0 +1,2016 @@



> +static int start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> + struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
> + int ret;
> + int parameter_size;
> +
> + v4l2_dbg(1, bcm2835_v4l2_debug, >v4l2_dev, "%s: dev:%p\n",
> +  __func__, dev);
> +
> + /* ensure a format has actually been set */
> + if (dev->capture.port == NULL)
> + return -EINVAL;

Standard mistake. If start_streaming returns an error, then it should call 
vb2_buffer_done
for all queued buffers with state VB2_BUF_STATE_QUEUED. Otherwise the buffer 
administration
gets unbalanced.

> +
> + if (enable_camera(dev) < 0) {
> + v4l2_err(>v4l2_dev, "Failed to enable camera\n");
> + return -EINVAL;
> + }
> +
> + /*init_completion(>capture.frame_cmplt); */
> +
> + /* enable frame capture */
> + dev->capture.frame_count = 1;
> +
> + /* if the preview is not already running, wait for a few frames for AGC
> +  * to settle down.
> +  */
> + if (!dev->component[MMAL_COMPONENT_PREVIEW]->enabled)
> + msleep(300);
> +
> + /* enable the connection from camera to encoder (if applicable) */
> + if (dev->capture.camera_port != dev->capture.port
> + && dev->capture.camera_port) {
> + ret = vchiq_mmal_port_enable(dev->instance,
> +  dev->capture.camera_port, NULL);
> + if (ret) {
> + v4l2_err(>v4l2_dev,
> +  "Failed to enable encode tunnel - error %d\n",
> +  ret);
> + return -1;

Use a proper error, not -1.

> + }
> + }
> +
> + /* Get VC timestamp at this point in time */
> + parameter_size = sizeof(dev->capture.vc_start_timestamp);
> + if (vchiq_mmal_port_parameter_get(dev->instance,
> +   dev->capture.camera_port,
> +   MMAL_PARAMETER_SYSTEM_TIME,
> +   >capture.vc_start_timestamp,
> +   _size)) {
> + v4l2_err(>v4l2_dev,
> +  "Failed to get VC start time - update your VC f/w\n");
> +
> + /* Flag to indicate just to rely on kernel timestamps */
> +   

Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder

[Smitha T Murthy]

On Thu, 2017-02-02 at 09:20 +0100, Andrzej Hajda wrote: 
> On 02.02.2017 08:58, Andrzej Hajda wrote:
> > On 18.01.2017 11:02, Smitha T Murthy wrote:
> >> Add support for codec definition and corresponding buffer
> >> requirements for HEVC decoder.
> >>
> >> Signed-off-by: Smitha T Murthy 
> >> ---
> >>  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |3 +++
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |3 +++
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |1 +
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c|8 
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 --
> >>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |5 +
> >>  6 files changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> >> b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> index 153ee68..a57009a 100644
> >> --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> >> @@ -32,6 +32,9 @@
> >>  #define MFC_VERSION_V10   0xA0
> >>  #define MFC_NUM_PORTS_V10 1
> >>  
> >> +/* MFCv10 codec defines*/
> >> +#define S5P_FIMV_CODEC_HEVC_DEC   17
> >> +
> >>  /* Encoder buffer size for MFC v10.0 */
> >>  #define ENC_V100_H264_ME_SIZE(x, y)   \
> >>(((x + 3) * (y + 3) * 8)\
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c 
> >> b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> index b1b1491..76eca67 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> >> @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx 
> >> *ctx)
> >>case S5P_MFC_CODEC_VP8_DEC:
> >>codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
> >>break;
> >> +  case S5P_MFC_CODEC_HEVC_DEC:
> >> +  codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> >> +  break;
> >>case S5P_MFC_CODEC_H264_ENC:
> >>codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
> >>break;
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
> >> b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> index 998e24b..5c46060 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> >> @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, 
> >> void *b)
> >>  #define S5P_MFC_CODEC_H263_DEC5
> >>  #define S5P_MFC_CODEC_VC1RCV_DEC  6
> >>  #define S5P_MFC_CODEC_VP8_DEC 7
> >> +#define S5P_MFC_CODEC_HEVC_DEC17
> >>  
> >>  #define S5P_MFC_CODEC_H264_ENC20
> >>  #define S5P_MFC_CODEC_H264_MVC_ENC21
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> >> b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> index 784b28e..9f459b3 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> >> @@ -156,6 +156,14 @@
> >>.versions   = MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
> >>MFC_V10_BIT,
> >>},
> >> +  {
> >> +  .name   = "HEVC Encoded Stream",
> >> +  .fourcc = V4L2_PIX_FMT_HEVC,
> >> +  .codec_mode = S5P_FIMV_CODEC_HEVC_DEC,
> >> +  .type   = MFC_FMT_DEC,
> >> +  .num_planes = 1,
> >> +  .versions   = MFC_V10_BIT,
> >> +  },
> >>  };
> >>  
> >>  #define NUM_FORMATS ARRAY_SIZE(formats)
> >> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> >> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> >> index 369210a..b6cb280 100644
> >> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> >> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> >> @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> >> s5p_mfc_ctx *ctx)
> >>S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
> >>ctx->bank1.size = ctx->scratch_buf_size;
> >>break;
> >> +  case S5P_MFC_CODEC_HEVC_DEC:
> >> +  mfc_debug(2, "Use min scratch buffer size\n");
> >> +  ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);
> > Again alignment of something which should be already aligned, and magic
> > number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.
> >
> >> +  ctx->bank1.size =
> >> +  ctx->scratch_buf_size +
> >> +  (ctx->mv_count * ctx->mv_size);
> >> +  break;
> >>case S5P_MFC_CODEC_H264_ENC:
> >>if (IS_MFCV10(dev)) {
> >>mfc_debug(2, "Use min scratch buffer size\n");
> >> @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct 
> >> s5p_mfc_ctx *ctx)
> >>switch (ctx->codec_mode) {
> >>case S5P_MFC_CODEC_H264_DEC:
> >>case S5P_MFC_CODEC_H264_MVC_DEC:
> >> +  case S5P_MFC_CODEC_HEVC_DEC:

Re: [PATCH 02/11] [media] s5p-mfc: Adding initial support for MFC v10.10

[Smitha T Murthy]

On Sat, 2017-01-21 at 14:28 -0600, Rob Herring wrote:
> On Wed, Jan 18, 2017 at 03:32:00PM +0530, Smitha T Murthy wrote:
> > Adding the support for MFC v10.10, with new register file and
> > necessary hw control, decoder, encoder and structural changes.
> > 
> > CC: Rob Herring 
> > CC: devicet...@vger.kernel.org 
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  .../devicetree/bindings/media/s5p-mfc.txt  |1 +
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h  |   36 
> >  drivers/media/platform/s5p-mfc/s5p_mfc.c   |   30 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h|4 +-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c  |4 ++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c   |   44 
> > +++-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c   |   21 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c|9 +++-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h|2 +
> >  9 files changed, 118 insertions(+), 33 deletions(-)
> >  create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > 
> > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt 
> > b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > index 2c90128..b70c613 100644
> > --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> > @@ -13,6 +13,7 @@ Required properties:
> > (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC
> > (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC
> > (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC
> > +   (f) "samsung,mfc-v10" for MFC v10 present in a variant of Exynos7 SoC
> 
> You are up to v10 in how many SoCs? Please stop with versions and use 
> SoC numbers. It's one thing to use versions when you have many SoCs per 
> version, but that doesn't seem to be happening here.
> 
> Rob
MFCv10.10 is used in Exynos7880. There are other variants of MFCv10 used
in Exynos8890 and Exynos7870. I will mention in the next version of
patches the SoC name Exynos7880 using MFCv10 on which I have tested.
 
Thank you for the review.

Regards,
Smitha
> 
> 





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

Re: [PATCH 05/11] [media] s5p-mfc: Add support for HEVC decoder

[Smitha T Murthy]

On Thu, 2017-02-02 at 08:58 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Add support for codec definition and corresponding buffer
> > requirements for HEVC decoder.
> >
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |3 +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |3 +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c|8 
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   18 --
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |5 +
> >  6 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index 153ee68..a57009a 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -32,6 +32,9 @@
> >  #define MFC_VERSION_V100xA0
> >  #define MFC_NUM_PORTS_V10  1
> >  
> > +/* MFCv10 codec defines*/
> > +#define S5P_FIMV_CODEC_HEVC_DEC17
> > +
> >  /* Encoder buffer size for MFC v10.0 */
> >  #define ENC_V100_H264_ME_SIZE(x, y)\
> > (((x + 3) * (y + 3) * 8)\
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > index b1b1491..76eca67 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > @@ -101,6 +101,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx 
> > *ctx)
> > case S5P_MFC_CODEC_VP8_DEC:
> > codec_type = S5P_FIMV_CODEC_VP8_DEC_V6;
> > break;
> > +   case S5P_MFC_CODEC_HEVC_DEC:
> > +   codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> > +   break;
> > case S5P_MFC_CODEC_H264_ENC:
> > codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
> > break;
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > index 998e24b..5c46060 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > @@ -79,6 +79,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void 
> > *b)
> >  #define S5P_MFC_CODEC_H263_DEC 5
> >  #define S5P_MFC_CODEC_VC1RCV_DEC   6
> >  #define S5P_MFC_CODEC_VP8_DEC  7
> > +#define S5P_MFC_CODEC_HEVC_DEC 17
> >  
> >  #define S5P_MFC_CODEC_H264_ENC 20
> >  #define S5P_MFC_CODEC_H264_MVC_ENC 21
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > index 784b28e..9f459b3 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > @@ -156,6 +156,14 @@
> > .versions   = MFC_V6_BIT | MFC_V7_BIT | MFC_V8_BIT |
> > MFC_V10_BIT,
> > },
> > +   {
> > +   .name   = "HEVC Encoded Stream",
> > +   .fourcc = V4L2_PIX_FMT_HEVC,
> > +   .codec_mode = S5P_FIMV_CODEC_HEVC_DEC,
> > +   .type   = MFC_FMT_DEC,
> > +   .num_planes = 1,
> > +   .versions   = MFC_V10_BIT,
> > +   },
> >  };
> >  
> >  #define NUM_FORMATS ARRAY_SIZE(formats)
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > index 369210a..b6cb280 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -220,6 +220,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> > S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
> > ctx->bank1.size = ctx->scratch_buf_size;
> > break;
> > +   case S5P_MFC_CODEC_HEVC_DEC:
> > +   mfc_debug(2, "Use min scratch buffer size\n");
> > +   ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size, 256);
> 
> Again alignment of something which should be already aligned, and magic
> number instead of S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6.
> 
Yes if we are using the scratch buffer given by the firmware we need not
align. I will change it in the next version.

> > +   ctx->bank1.size =
> > +   ctx->scratch_buf_size +
> > +   (ctx->mv_count * ctx->mv_size);
> > +   break;
> > case S5P_MFC_CODEC_H264_ENC:
> > if (IS_MFCV10(dev)) {
> > mfc_debug(2, "Use min scratch buffer size\n");
> > @@ -322,6 +329,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct 
> > s5p_mfc_ctx *ctx)
> > switch (ctx->codec_mode) {
> > case S5P_MFC_CODEC_H264_DEC:
> > case S5P_MFC_CODEC_H264_MVC_DEC:
> > +   case 

Re: [PATCH 09/11] [media] s5p-mfc: Add support for HEVC encoder

[Smitha T Murthy]

On Thu, 2017-02-02 at 09:55 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Add HEVC encoder support and necessary registers, V4L2 CIDs,
> > and hevc encoder parameters
> >
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |   28 +-
> >  drivers/media/platform/s5p-mfc/s5p_mfc.c|1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |3 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |   55 ++-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c|  595 
> > +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr.h|8 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |  200 
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |7 +
> >  8 files changed, 895 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index 81a0a96..914ffec 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -20,13 +20,35 @@
> >  #define S5P_FIMV_MFC_STATE_V10 0x7124
> >  #define S5P_FIMV_D_STATIC_BUFFER_ADDR_V10  0xF570
> >  #define S5P_FIMV_D_STATIC_BUFFER_SIZE_V10  0xF574
> > +#define S5P_FIMV_E_NUM_T_LAYER_V10 0xFBAC
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER0_V10  0xFBB0
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER1_V10  0xFBB4
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER2_V10  0xFBB8
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER3_V10  0xFBBC
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER4_V10  0xFBC0
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER5_V10  0xFBC4
> > +#define S5P_FIMV_E_HIERARCHICAL_QP_LAYER6_V10  0xFBC8
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER0_V100xFD18
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER1_V100xFD1C
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER2_V100xFD20
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER3_V100xFD24
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER4_V100xFD28
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER5_V100xFD2C
> > +#define S5P_FIMV_E_HIERARCHICAL_BIT_RATE_LAYER6_V100xFD30
> > +#define S5P_FIMV_E_HEVC_OPTIONS_V100xFDD4
> > +#define S5P_FIMV_E_HEVC_REFRESH_PERIOD_V10 0xFDD8
> > +#define S5P_FIMV_E_HEVC_CHROMA_QP_OFFSET_V10   0xFDDC
> > +#define S5P_FIMV_E_HEVC_LF_BETA_OFFSET_DIV2_V100xFDE0
> > +#define S5P_FIMV_E_HEVC_LF_TC_OFFSET_DIV2_V10  0xFDE4
> > +#define S5P_FIMV_E_HEVC_NAL_CONTROL_V100xFDE8
> >  
> >  /* MFCv10 Context buffer sizes */
> >  #define MFC_CTX_BUF_SIZE_V10   (30 * SZ_1K)/* 30KB */
> >  #define MFC_H264_DEC_CTX_BUF_SIZE_V10  (2 * SZ_1M) /* 2MB */
> >  #define MFC_OTHER_DEC_CTX_BUF_SIZE_V10 (20 * SZ_1K)/* 20KB */
> >  #define MFC_H264_ENC_CTX_BUF_SIZE_V10  (100 * SZ_1K)   /* 100KB */
> > -#define MFC_OTHER_ENC_CTX_BUF_SIZE_V10 (15 * SZ_1K)/* 15KB */
> > +#define MFC_HEVC_ENC_CTX_BUF_SIZE_V10  (30 * SZ_1K)/* 30KB */
> > +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V10  (15 * SZ_1K)   /* 15KB */
> >  
> >  /* MFCv10 variant defines */
> >  #define MAX_FW_SIZE_V10(SZ_1M) /* 1MB */
> > @@ -37,6 +59,7 @@
> >  /* MFCv10 codec defines*/
> >  #define S5P_FIMV_CODEC_HEVC_DEC17
> >  #define S5P_FIMV_CODEC_VP9_DEC 18
> > +#define S5P_FIMV_CODEC_HEVC_ENC 26
> >  
> >  /* Decoder buffer size for MFC v10 */
> >  #define DEC_VP9_STATIC_BUFFER_SIZE 20480
> > @@ -53,6 +76,9 @@
> >  #define ENC_V100_VP8_ME_SIZE(x, y) \
> > (((x + 3) * (y + 3) * 8)\
> >  + (((y * 64) + 1280) * (x + 7) / 8))
> > +#define ENC_V100_HEVC_ME_SIZE(x, y)\
> > +   (((x + 3) * (y + 3) * 32)   \
> > ++ (((y * 128) + 1280) * (x + 3) / 4))
> 
> Use DIV_ROUND_UP.
> 
I will correct this in next version. 
> >  
> >  #endif /*_REGS_MFC_V10_H*/
> >  
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > index b014038..b01c556 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > @@ -1549,6 +1549,7 @@ static int s5p_mfc_resume(struct device *dev)
> > .h264_dec_ctx   = MFC_H264_DEC_CTX_BUF_SIZE_V10,
> > .other_dec_ctx  = MFC_OTHER_DEC_CTX_BUF_SIZE_V10,
> > .h264_enc_ctx   = MFC_H264_ENC_CTX_BUF_SIZE_V10,
> > +   .hevc_enc_ctx   = MFC_HEVC_ENC_CTX_BUF_SIZE_V10,
> > .other_enc_ctx  = MFC_OTHER_ENC_CTX_BUF_SIZE_V10,
> >  };
> >  
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > index 102b47e..7521fce 100644
> > --- 

Re: [PATCH 03/11] [media] s5p-mfc: Use min scratch buffer size

[Smitha T Murthy]

On Thu, 2017-02-02 at 08:16 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > After MFC v8.0, mfc f/w lets the driver know how much scratch buffer
> > size is required for decoder. If mfc f/w has the functionality,
> > E_MIN_SCRATCH_BUFFER_SIZE, driver can know how much scratch buffer size
> > is required for encoder too.
> 
> Subject says "Use min scratch buffer size" but it is already used.
> Maybe it should be changed to sth like:
> Use min scratch buffer size provided by F/W
> 
Yes it will be better if I change the commit message as "Use min scratch buffer 
size provided by F/W".
> >
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v8.h|2 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc.c|2 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c|7 ++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr.h|4 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   68 
> > +--
> >  6 files changed, 67 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v8.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
> > index 4d1c375..2cd396b 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v8.h
> > @@ -17,6 +17,7 @@
> >  
> >  /* Additional registers for v8 */
> >  #define S5P_FIMV_D_MVC_NUM_VIEWS_V80xf104
> > +#define S5P_FIMV_D_MIN_SCRATCH_BUFFER_SIZE_V8  0xf108
> >  #define S5P_FIMV_D_FIRST_PLANE_DPB_SIZE_V8 0xf144
> >  #define S5P_FIMV_D_SECOND_PLANE_DPB_SIZE_V80xf148
> >  #define S5P_FIMV_D_MV_BUFFER_SIZE_V8   0xf150
> > @@ -84,6 +85,7 @@
> >  
> >  #define S5P_FIMV_E_VBV_BUFFER_SIZE_V8  0xf78c
> >  #define S5P_FIMV_E_VBV_INIT_DELAY_V8   0xf790
> > +#define S5P_FIMV_E_MIN_SCRATCH_BUFFER_SIZE_V8   0xf894
> >  
> >  #define S5P_FIMV_E_ASPECT_RATIO_V8 0xfb4c
> >  #define S5P_FIMV_E_EXTENDED_SAR_V8 0xfb50
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > index a043cce..b014038 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> > @@ -520,6 +520,8 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx 
> > *ctx,
> > dev);
> > ctx->mv_count = s5p_mfc_hw_call(dev->mfc_ops, get_mv_count,
> > dev);
> > +   ctx->scratch_buf_size = s5p_mfc_hw_call(dev->mfc_ops,
> > +   get_min_scratch_buf_size, dev);
> > if (ctx->img_width == 0 || ctx->img_height == 0)
> > ctx->state = MFCINST_ERROR;
> > else
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > index 1941c63..998e24b 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > @@ -724,6 +724,7 @@ struct mfc_control {
> >  #define IS_MFCV7_PLUS(dev) (dev->variant->version >= 0x70 ? 1 : 0)
> >  #define IS_MFCV8_PLUS(dev) (dev->variant->version >= 0x80 ? 1 : 0)
> >  #define IS_MFCV10(dev) (dev->variant->version >= 0xA0 ? 1 : 0)
> > +#define FW_HAS_E_MIN_SCRATCH_BUF(dev) (IS_MFCV10(dev))
> >  
> >  #define MFC_V5_BIT BIT(0)
> >  #define MFC_V6_BIT BIT(1)
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> > index 9042378..ef15831 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> > @@ -818,6 +818,13 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx)
> > get_enc_dpb_count, dev);
> > if (ctx->pb_count < enc_pb_count)
> > ctx->pb_count = enc_pb_count;
> > +   if (FW_HAS_E_MIN_SCRATCH_BUF(dev)) {
> > +   ctx->scratch_buf_size = s5p_mfc_hw_call(dev->mfc_ops,
> > +   get_e_min_scratch_buf_size, dev);
> > +   ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size,
> > +   S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
> 
> Do we really need to align it here? Does firmware return unaligned value?
> Even then the alignment (if necessary) should be moved rather to
> get_e_min_scratch_buf_size.
> 
No we do not need alignment on the values returned by firmware, they
would be aligned as per the UM and returned to driver. I will change it
in the next version.

> > +   ctx->bank1.size += ctx->scratch_buf_size;
> > +   }
> > ctx->state = MFCINST_HEAD_PRODUCED;
> > }
> >  
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h 
> > 

Re: [PATCH 08/11] [media] s5p-mfc: Add VP9 decoder support

[Smitha T Murthy]

On Thu, 2017-02-02 at 09:39 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Add support for codec definition and corresponding buffer
> > requirements for VP9 decoder.
> >
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |6 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |3 ++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_common.h |1 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c|8 ++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr.h|2 +
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   28 
> > +++
> >  6 files changed, 48 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index a57009a..81a0a96 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -18,6 +18,8 @@
> >  /* MFCv10 register definitions*/
> >  #define S5P_FIMV_MFC_CLOCK_OFF_V10 0x7120
> >  #define S5P_FIMV_MFC_STATE_V10 0x7124
> > +#define S5P_FIMV_D_STATIC_BUFFER_ADDR_V10  0xF570
> > +#define S5P_FIMV_D_STATIC_BUFFER_SIZE_V10  0xF574
> >  
> >  /* MFCv10 Context buffer sizes */
> >  #define MFC_CTX_BUF_SIZE_V10   (30 * SZ_1K)/* 30KB */
> > @@ -34,6 +36,10 @@
> >  
> >  /* MFCv10 codec defines*/
> >  #define S5P_FIMV_CODEC_HEVC_DEC17
> > +#define S5P_FIMV_CODEC_VP9_DEC 18
> > +
> > +/* Decoder buffer size for MFC v10 */
> > +#define DEC_VP9_STATIC_BUFFER_SIZE 20480
> >  
> >  /* Encoder buffer size for MFC v10.0 */
> >  #define ENC_V100_H264_ME_SIZE(x, y)\
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > index 76eca67..102b47e 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> > @@ -104,6 +104,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx 
> > *ctx)
> > case S5P_MFC_CODEC_HEVC_DEC:
> > codec_type = S5P_FIMV_CODEC_HEVC_DEC;
> > break;
> > +   case S5P_MFC_CODEC_VP9_DEC:
> > +   codec_type = S5P_FIMV_CODEC_VP9_DEC;
> > +   break;
> > case S5P_MFC_CODEC_H264_ENC:
> > codec_type = S5P_FIMV_CODEC_H264_ENC_V6;
> > break;
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > index 5c46060..e720ce6 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> > @@ -80,6 +80,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void 
> > *b)
> >  #define S5P_MFC_CODEC_VC1RCV_DEC   6
> >  #define S5P_MFC_CODEC_VP8_DEC  7
> >  #define S5P_MFC_CODEC_HEVC_DEC 17
> > +#define S5P_MFC_CODEC_VP9_DEC  18
> >  
> >  #define S5P_MFC_CODEC_H264_ENC 20
> >  #define S5P_MFC_CODEC_H264_MVC_ENC 21
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > index 9f459b3..93626ed 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> > @@ -164,6 +164,14 @@
> > .num_planes = 1,
> > .versions   = MFC_V10_BIT,
> > },
> > +   {
> > +   .name   = "VP9 Encoded Stream",
> > +   .fourcc = V4L2_PIX_FMT_VP9,
> > +   .codec_mode = S5P_FIMV_CODEC_VP9_DEC,
> > +   .type   = MFC_FMT_DEC,
> > +   .num_planes = 1,
> > +   .versions   = MFC_V10_BIT,
> > +   },
> >  };
> >  
> >  #define NUM_FORMATS ARRAY_SIZE(formats)
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> > index 6478f70..565decf 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> > @@ -170,6 +170,8 @@ struct s5p_mfc_regs {
> > void __iomem *d_used_dpb_flag_upper;/* v7 and v8 */
> > void __iomem *d_used_dpb_flag_lower;/* v7 and v8 */
> > void __iomem *d_min_scratch_buffer_size; /* v10 */
> > +   void __iomem *d_static_buffer_addr; /* v10 */
> > +   void __iomem *d_static_buffer_size; /* v10 */
> >  
> > /* encoder registers */
> > void __iomem *e_frame_width;
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > index b6cb280..da4202f 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -227,6 +227,13 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> > ctx->scratch_buf_size +
> >   

Re: [PATCH 06/11] [media] videodev2.h: Add v4l2 definition for HEVC

[Smitha T Murthy]

On Thu, 2017-02-02 at 09:34 +0100, Andrzej Hajda wrote: 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Add V4L2 definition for HEVC compressed format
> >
> > Signed-off-by: Smitha T Murthy 
> Beside small nitpick.
> 
> Reviewed-by: Andrzej Hajda 
> 
> > ---
> >  include/uapi/linux/videodev2.h |1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 46e8a2e3..620e941 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -630,6 +630,7 @@ struct v4l2_pix_format {
> >  #define V4L2_PIX_FMT_VC1_ANNEX_L v4l2_fourcc('V', 'C', '1', 'L') /* SMPTE 
> > 421M Annex L compliant stream */
> >  #define V4L2_PIX_FMT_VP8  v4l2_fourcc('V', 'P', '8', '0') /* VP8 */
> >  #define V4L2_PIX_FMT_VP9  v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
> > +#define V4L2_PIX_FMT_HEVC v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC */
> 
> I am not sure if it shouldn't be sorted alphabetically in compressed
> formats stanza.
> 
> --
> Regards
> Andrzej

Actually the formats are not arranged alphabetically. For example
#define V4L2_PIX_FMT_XVID is added before the #define
V4L2_PIX_FMT_VC1_ANNEX_G. Hence I added the definition at the end.
If required, I will take this as a separate patch.

Thank you for the review.
Regards,
Smitha 
> 
> >  
> >  /*  Vendor-specific formats   */
> >  #define V4L2_PIX_FMT_CPIA1v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV 
> > */
> 
> 
> 





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

Re: [PATCH 04/11] [media] s5p-mfc: Support MFCv10.10 buffer requirements

[Smitha T Murthy]

On Thu, 2017-02-02 at 09:30 +0100, Andrzej Hajda wrote: 
> Hi Smitha,
> 
> Ups, I have missed this patch, I hope it wont influence the review :)
> 
> 
> On 18.01.2017 11:02, Smitha T Murthy wrote:
> > Aligning the luma_dpb_size, chroma_dpb_size, mv_size and me_buffer_size
> > for MFCv10.10.
> >
> > Signed-off-by: Smitha T Murthy 
> > ---
> >  drivers/media/platform/s5p-mfc/regs-mfc-v10.h   |   13 +++
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   97 
> > ++-
> >  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h |2 +
> >  3 files changed, 91 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h 
> > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > index bd671a5..153ee68 100644
> > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> > @@ -32,5 +32,18 @@
> >  #define MFC_VERSION_V100xA0
> >  #define MFC_NUM_PORTS_V10  1
> >  
> > +/* Encoder buffer size for MFC v10.0 */
> > +#define ENC_V100_H264_ME_SIZE(x, y)\
> > +   (((x + 3) * (y + 3) * 8)\
> > ++ x * y) + 63) / 64) * 32) \
> > ++ (((y * 64) + 1280) * (x + 7) / 8))
> > +#define ENC_V100_MPEG4_ME_SIZE(x, y)   \
> > +   (((x + 3) * (y + 3) * 8)\
> > ++ x * y) + 127) / 128) * 16)   \
> > ++ (((y * 64) + 1280) * (x + 7) / 8))
> > +#define ENC_V100_VP8_ME_SIZE(x, y) \
> > +   (((x + 3) * (y + 3) * 8)\
> > ++ (((y * 64) + 1280) * (x + 7) / 8))
> > +
> 
> Crazy, cryptic math here, I guess you can make it more readable by using
> DIV_ROUND_UP macro and abstracting out common parts, for example:
> 
> #define ENC_V100_BASE_SIZE(x, y) \
>   (((x + 3) * (y + 3) * 8) \
>   +  ((y * 64) + 1280) * DIV_ROUND_UP(x, 8))
> 
> #define ENC_V100_H264_ME_SIZE(x, y) \
>   (ENC_V100_BASE_SIZE(x, y)
>   + DIV_ROUND_UP(x * y, 64) * 32)
> 
> #define ENC_V100_MPEG4_ME_SIZE(x, y) \
>   (ENC_V100_BASE_SIZE(x, y)
>   + DIV_ROUND_UP(x * y, 128) * 16)
> 
> #define ENC_V100_VP8_ME_SIZE(x, y)\
>   ENC_V100_BASE_SIZE(x, y)
>  
> 
I put the equation as I had found in the User Manual, so that it will
help in quick reference check in future. But I will change it as per
your suggestion.

> >  #endif /*_REGS_MFC_V10_H*/
> >  
> > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c 
> > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > index faceee6..369210a 100644
> > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -64,6 +64,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> >  {
> > struct s5p_mfc_dev *dev = ctx->dev;
> > unsigned int mb_width, mb_height;
> > +   unsigned int lcu_width = 0, lcu_height = 0;
> > int ret;
> >  
> > mb_width = MB_WIDTH(ctx->img_width);
> > @@ -74,7 +75,9 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> >   ctx->luma_size, ctx->chroma_size, ctx->mv_size);
> > mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count);
> > } else if (ctx->type == MFCINST_ENCODER) {
> > -   if (IS_MFCV8_PLUS(dev))
> > +   if (IS_MFCV10(dev)) {
> > +   ctx->tmv_buffer_size = 0;
> > +   } else if (IS_MFCV8_PLUS(dev))
> > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, mb_height),
> > S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> > @@ -82,13 +85,36 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct 
> > s5p_mfc_ctx *ctx)
> > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, mb_height),
> > S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> > -
> > -   ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
> > -   S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
> > -   S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
> > -   ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
> > -   S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,
> > -   S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6);
> > +   if (IS_MFCV10(dev)) {
> > +   lcu_width = enc_lcu_width(ctx->img_width);
> > +   lcu_height = enc_lcu_height(ctx->img_height);
> > +   if (ctx->codec_mode != S5P_FIMV_CODEC_HEVC_ENC) {
> > +   ctx->luma_dpb_size =
> > +   ALIGNmb_width * 16) + 63) / 64)
> > +   * 64 * (((mb_height * 16) + 31)
> > +   / 32) * 32 + 64, 64);
> > +   ctx->chroma_dpb_size =
> > +   ALIGNmb_width * 16) + 63) / 64)
> > +  

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

[Greg Kroah-Hartman]

On Sun, Feb 05, 2017 at 10:15:21PM +, Dave Stevenson wrote:
> Newbie question: if this has already been merged to staging, where am I
> looking for the relevant tree to add patches on top of?
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git branch
> staging-next?

Yes, that is the correct place.

thanks,

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