linux media DKMS

[Jasmin J.]

Hello!

I googled some time now, but I couldn't find a suitable DKMS to encapsulate
the media-build.

Before I write my own one I would like to ask if it is already existing(for
the case I can't properly use Google).

If not I will use "media-build-experimental-dkms" as a start.

BR,
   Jasmin

[GIT PULL] SAA716x DVB driver

[Soeren Moch]

This is a driver for DVB cards based on the SAA7160/62 PCIe bridges from
Philips/NXP. The most important of these cards, at least for me, is the
TechnoTrend S2-6400, a high-definition full-featured DVB-S2 card, the
successor of the famous ttpci decoder cards. Other supported cards are:
Avermedia H788
Avermedia HC82 Express-54
KNC One Dual S2
KWorld DVB-T PE310
Technisat SkyStar 2 eXpress HD
Twinhan/Azurewave VP-1028
Twinhan/Azurewave VP-3071
Twinhan/Azurewave VP-6002
Twinhan/Azurewave VP-6090

The driver is taken from [1] with adaptations for current mainline,
converted to git and moved to drivers/staging/media. See TODO for
required cleanups (from my point of view, maybe more to come from
additional code review by other developers). I added myself as driver
maintainer to document my commitment to further development to get this
out of staging, help from others welcome.

This driver was licensed "GPL" from the beginning. S2-6400 firmware is
available at [2].

I want to preserve the development history of the driver, since this
is mainly work of Andreas Regel, Manu Abraham, and Oliver Endriss.
Unfortunately Andreas seems not to take care of this driver anymore, he
also did not answer my requests to integrate patches for newer kernel
versions. So I send this upstream.

With all the history this is a 280 part patch series, so I send this as
pull request. Of course I can also send this as 'git send-email' series
if desired.

Since this is intended for staging, I hope this can be pulled quickly and
fixed up in mainline. For all patches I added SOBs, the series should not
hurt kernel bisectibility since the driver is hooked up in Kconfig at the
end of this series.

Regards,
Soeren

[1] https://bitbucket.org/powARman/v4l-dvb-saa716x/
[2] http://www.aregel.de/downloads/



The following changes since commit 5771a8c08880cdca3bfb4a3fc6d309d6bba20877:

  Linux v4.13-rc1 (2017-07-15 15:22:10 -0700)

are available in the git repository at:

  https://github.com/s-moch/linux-saa716x for-media

for you to fetch changes up to 1a9e44df11d702b9f569605e60c7cf8e05923178:

  saa716x: Add MAINTAINERS entry (2017-07-16 15:37:00 +0200)


Andreas Regel (127):
  saa716x: Simplified the code for I2C transfers.
  saa716x: Transport stream ports can be configured now.
  saa716x_ff: Disable frontend support through STi7109 firmware.
  saa716x_ff: Initialize the frontend of the TT S2-6400.
  saa716x_ff: Add VIDEO_GET_PTS ioctl.
  saa716x_ff: Add audio device for TT S2-6400.
  saa716x_ff: Use separate interrupts for OSD commands.
  saa716x_ff: Use third TS input of the STi7109 for playback
  saa716x_ff: Set error code when boot of firmware failed.
  saa716x_ff: Added 10ms sleep after configuring the FPGA.
  saa716x: Reduce compiler warnings
  saa716x: fix kernel oops caused by missing initialisation
  saa716x: fixed double frontend detach
  saa716x: 1ms is enough when waiting for the PLL
  saa716x_ff: Adapted frontend init to latest driver changes.
  saa716x: Improve PHI performance by shorten timings a bit.
  saa716x_ff: use double buffering for block transfers.
  saa716x_ff: Reset block_done after receiving it.
  saa716x_ff: add interrupt source for firmware log messages
  saa716x_ff: read FPGA version register
  saa716x_ff: fixed PTS/STC signed/unsigned issue
  saa716x_ff: query and print loader and firmware versions
  saa716x: disable debug printk
  osd.h: Add const keyword to struct members
  saa716x_ff: implement ioctl VIDEO_GET_SIZE
  saa716x_ff: support production version of the S2-6400
  saa716x: disable OVERFLOW and AV interrupts in FGPI
  saa716x_ff: add some FPGA register definitions
  saa716x_ff: restart TS capturing in case of non-aligned TS error
  saa716x_ff: don't clear FGPI interrupts in IRQ routine.
  saa716x: comment out SPI functions that are not needed currently
  saa716x: add changes needed for kernel 2.6.36 and up.
  saa716x: support building with media_build_experimental
  saa716x_ff: use tasklet for FIFO transfer
  saa716x: fix not working MSI
  saa716x: Disable shared IRQ when using MSI mode.
  saa716x_ff: Do an explicit demodulator reset.
  saa716x: enable wait for PLL lock when setting clocks of all ports.
  saa716x_ff: add counting of interrupts
  saa716x: don't set i2c_adapter id field
  saa716x_ff: support non-aligned TS data
  saa716x_ff: Use a TS clock of 135Mhz instead of 90 Mhz.
  saa716x_ff: Ignore false remote events.
  saa716x_ff: Fix possible crash in IRQ handler.
  saa716x_ff: move firmware command interface code to separate file.
  saa716x_ff: Allow to have osd device opened twice.
  saa716x_ff: support correct TS mux setting for FPGA 1.10.
  saa716x_budget: Add missing GPIO initialization.
  saa716x_hybrid: Add missing GPIO initialization.

[PATCH v2 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors

[Philipp Zabel]

The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor chips
directly via I2C commands using extension unit controls. The processing and
camera unit controls do not function at all.

Signed-off-by: Philipp Zabel 
---
 drivers/media/usb/uvc/uvc_ctrl.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 1d60321a6777..738edb81bc0a 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2213,6 +2213,10 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
 {
struct uvc_entity *entity;
unsigned int i;
+   const struct usb_device_id xu_only[] = {
+   { USB_DEVICE(0x2833, 0x0201) },
+   { USB_DEVICE(0x2833, 0x0211) },
+   };
 
/* Walk the entities list and instantiate controls */
list_for_each_entry(entity, >entities, list) {
@@ -2220,6 +2224,16 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
unsigned int bControlSize = 0, ncontrols;
__u8 *bmControls = NULL;
 
+   /* Oculus Sensors only handle extension unit controls */
+   if (UVC_ENTITY_TYPE(entity) != UVC_VC_EXTENSION_UNIT) {
+   for (i = 0; i < ARRAY_SIZE(xu_only); i++) {
+   if (usb_match_one_id(dev->intf, _only[i]))
+   break;
+   }
+   if (i != ARRAY_SIZE(xu_only))
+   continue;
+   }
+
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT) {
bmControls = entity->extension.bmControls;
bControlSize = entity->extension.bControlSize;
-- 
2.13.2

[PATCH v2 1/3] [media] uvcvideo: variable size controls

[Philipp Zabel]

Some USB webcam controllers have extension unit controls that report
different lengths via GET_LEN, depending on internal state. Add a flag
to mark these controls as variable length and issue GET_LEN before
GET/SET_CUR transfers to verify the current length.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Split uvc_ctrl_fill_xu_info_size and uvc_ctrl_fill_xu_info_flags out of
   uvc_ctrl_fill_xu_info.
 - Add uvc_ctrl_update_xu_info_size and reuse uvc_ctrl_fill_xu_info_size.
 - Call uvc_ctrl_update_xu_info_size from uvc_xu_ctrl_query instead of open
   coding the size update, thereby fixing a double kfree.
---
 drivers/media/usb/uvc/uvc_ctrl.c | 98 ++--
 include/uapi/linux/uvcvideo.h|  2 +
 2 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c2ee6e39fd0c..43e8851cc381 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1597,7 +1597,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
struct usb_device_id id;
u8 entity;
u8 selector;
-   u8 flags;
+   u16 flags;
};
 
static const struct uvc_ctrl_fixup fixups[] = {
@@ -1629,10 +1629,7 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device 
*dev,
}
 }
 
-/*
- * Query control information (size and flags) for XU controls.
- */
-static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
+static int uvc_ctrl_fill_xu_info_size(struct uvc_device *dev,
const struct uvc_control *ctrl, struct uvc_control_info *info)
 {
u8 *data;
@@ -1642,11 +1639,6 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
if (data == NULL)
return -ENOMEM;
 
-   memcpy(info->entity, ctrl->entity->extension.guidExtensionCode,
-  sizeof(info->entity));
-   info->index = ctrl->index;
-   info->selector = ctrl->index + 1;
-
/* Query and verify the control length (GET_LEN) */
ret = uvc_query_ctrl(dev, UVC_GET_LEN, ctrl->entity->id, dev->intfnum,
 info->selector, data, 2);
@@ -1659,6 +1651,21 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
 
info->size = le16_to_cpup((__le16 *)data);
 
+done:
+   kfree(data);
+   return ret;
+}
+
+static int uvc_ctrl_fill_xu_info_flags(struct uvc_device *dev,
+   const struct uvc_control *ctrl, struct uvc_control_info *info)
+{
+   u8 *data;
+   int ret;
+
+   data = kmalloc(1, GFP_KERNEL);
+   if (data == NULL)
+   return -ENOMEM;
+
/* Query the control information (GET_INFO) */
ret = uvc_query_ctrl(dev, UVC_GET_INFO, ctrl->entity->id, dev->intfnum,
 info->selector, data, 1);
@@ -1678,6 +1685,32 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
| (data[0] & UVC_CONTROL_CAP_AUTOUPDATE ?
   UVC_CTRL_FLAG_AUTO_UPDATE : 0);
 
+done:
+   kfree(data);
+   return ret;
+}
+
+/*
+ * Query control information (size and flags) for XU controls.
+ */
+static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
+   const struct uvc_control *ctrl, struct uvc_control_info *info)
+{
+   int ret;
+
+   memcpy(info->entity, ctrl->entity->extension.guidExtensionCode,
+  sizeof(info->entity));
+   info->index = ctrl->index;
+   info->selector = ctrl->index + 1;
+
+   ret = uvc_ctrl_fill_xu_info_size(dev, ctrl, info);
+   if (ret < 0)
+   return ret;
+
+   ret = uvc_ctrl_fill_xu_info_flags(dev, ctrl, info);
+   if (ret < 0)
+   return ret;
+
uvc_ctrl_fixup_xu_info(dev, ctrl, info);
 
uvc_trace(UVC_TRACE_CONTROL, "XU control %pUl/%u queried: len %u, "
@@ -1687,9 +1720,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
  (info->flags & UVC_CTRL_FLAG_SET_CUR) ? 1 : 0,
  (info->flags & UVC_CTRL_FLAG_AUTO_UPDATE) ? 1 : 0);
 
-done:
-   kfree(data);
-   return ret;
+   return 0;
 }
 
 static int uvc_ctrl_add_info(struct uvc_device *dev, struct uvc_control *ctrl,
@@ -1717,6 +1748,40 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev,
return ret;
 }
 
+/*
+ * Update control size for variable length XU controls.
+ */
+static int uvc_ctrl_update_xu_info_size(struct uvc_device *dev,
+   struct uvc_control *ctrl)
+{
+   u16 size = ctrl->info.size;
+   int ret;
+
+   if (!(ctrl->info.flags & UVC_CTRL_FLAG_VARIABLE_LEN))
+   return 0;
+
+   /* Check if the control size has changed */
+   ret = uvc_ctrl_fill_xu_info_size(dev, ctrl, >info);
+   if (ret < 0)
+   return ret;
+
+   if (ctrl->info.size != size) {
+   uvc_trace(UVC_TRACE_CONTROL,
+ "XU control %pUl/%u queried: len %u -> %u\n",

[PATCH v2 2/3] [media] uvcvideo: flag variable length control on Oculus Rift CV1 Sensor

[Philipp Zabel]

The extension unit controls with selectors 11 and 12 are used to make the
eSP770u webcam controller issue SPI transfers to configure the nRF51288
radio or to read the flash storage. Depending on internal state controlled
by selector 11, selector 12 reports different lengths.

Signed-off-by: Philipp Zabel 
---
 drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 43e8851cc381..1d60321a6777 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1613,6 +1613,9 @@ static void uvc_ctrl_fixup_xu_info(struct uvc_device *dev,
UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX |
UVC_CTRL_FLAG_GET_DEF | UVC_CTRL_FLAG_SET_CUR |
UVC_CTRL_FLAG_AUTO_UPDATE },
+   { { USB_DEVICE(0x2833, 0x0211) }, 4, 12,
+   UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_SET_CUR |
+   UVC_CTRL_FLAG_VARIABLE_LEN },
};
 
unsigned int i;
-- 
2.13.2

Re: [PATCH v2 6/6] dt-bindings: Document the Rockchip RGA bindings

[Heiko Stuebner]

Hi Laurent,

Am Samstag, 15. Juli 2017, 12:23:12 CEST schrieb Laurent Pinchart:
> On Saturday 15 Jul 2017 14:58:40 Jacob Chen wrote:
> > Add DT bindings documentation for Rockchip RGA
> > 
> > Signed-off-by: Yakir Yang 
> > Signed-off-by: Jacob Chen 
> > ---
> >  .../devicetree/bindings/media/rockchip-rga.txt | 35 +++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt
> > b/Documentation/devicetree/bindings/media/rockchip-rga.txt new file mode
> > 100644
> > index 000..966eba0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
> > @@ -0,0 +1,35 @@
> > +device-tree bindings for rockchip 2D raster graphic acceleration controller
> > (RGA)
> > +
> > +RGA is a separate 2D raster graphic acceleration unit. It accelerates 2D
> 
> "Separate" from what ? Do you mean "standalone" ?
> 
> > +graphics operations, such as point/line drawing, image scaling, rotation,
> > +BitBLT, alpha blending and image blur/sharpness.
> > +
> > +Required properties:
> > +- compatible: value should be one of the following
> > +   "rockchip,rk3228-rga";
> > +   "rockchip,rk3288-rga";
> > +   "rockchip,rk3399-rga";
> 
> The driver in patch 2/6 has match entry for rk3328, which is missing from 
> this 
> list.
> 
> As the implementation of the driver doesn't seem to discriminate between the 
> four SoCs, wouldn't it make sense to create a generic compatible string on 
> which the driver would match ? You can have both the generic and SoC-specific 
> compatible strings in DT if there are differences between the IP core in 
> those 
> SoCs that might need to be handled later by the driver.

I think the block is named something like RGA2 in some kernel trees, but
am not sure if that is an actual name, or someone just added a number.
>From short glances at vendor rga-code in the past, it looks like there are
currently 2 basic versions of the in existence I.e. older Rockchip socs
(like the rk3188 or so) use something older.

I think everywhere else we do have only (or at least mostly) soc-specifc
compatibles, but I guess that is more a question what Rob prefers.


Heiko

> > +- interrupts: RGA interrupt number.
> 
> This is an "interrupt specifier", not just an "interrupt number" (as you can 
> see in the example below there are three numbers)
> 
> > +
> > +- clocks: phandle to RGA sclk/hclk/aclk clocks
> > +
> > +- clock-names: should be "aclk" "hclk" and "sclk"
> 
> Nitpicking, there should be a comma after "aclk".
> 
> > +
> > +- resets: Must contain an entry for each entry in reset-names.
> > +  See ../reset/reset.txt for details.
> > +- reset-names: should be "core" "axi" and "ahb"
> 
> And a comma after "core".
> 
> > +
> > +Example:
> > +SoC specific DT entry:
> 
> s/SoC specific/SoC-specific/
> 
> > +   rga: rga@ff68 {
> > +   compatible = "rockchip,rk3399-rga";
> > +   reg = <0xff68 0x1>;
> > +   interrupts = ;
> > +   interrupt-names = "rga";
> 
> The interrupt-names property is not described above. Do you really need it ?
> 
> > +   clocks = < ACLK_RGA>, < HCLK_RGA>, < 
> SCLK_RGA_CORE>;
> > +   clock-names = "aclk", "hclk", "sclk";
> > +
> > +   resets = < SRST_RGA_CORE>, < SRST_A_RGA>, < 
> SRST_H_RGA>;
> > +   reset-names = "core, "axi", "ahb";
> > +   };
> 
> 

Re: [PATCH] ddbridge: constify i2c_algorithm structure

[Gustavo A. R. Silva]

Hi Daniel,


On 07/10/2017 10:16 AM, Daniel Scheller wrote:

Am Sun, 9 Jul 2017 20:15:36 -0500
schrieb "Gustavo A. R. Silva" :


Check for i2c_algorithm structures that are only stored in
the algo field of an i2c_adapter structure. This field is
declared const, so i2c_algorithm structures that have this
property can be declared as const also.

This issue was identified using Coccinelle and the following
semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct i2c_algorithm i@p = { ... };

@ok@
identifier r.i;
struct i2c_adapter e;
position p;
@@
e.algo = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct i2c_algorithm i = { ... };

Signed-off-by: Gustavo A. R. Silva 
---
  drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c
b/drivers/media/pci/ddbridge/ddbridge-core.c index cd1723e..9663a4c
100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -200,7 +200,7 @@ static u32 ddb_i2c_functionality(struct
i2c_adapter *adap) return I2C_FUNC_SMBUS_EMUL;
  }
  
-static struct i2c_algorithm ddb_i2c_algo = {

+static const struct i2c_algorithm ddb_i2c_algo = {
.master_xfer   = ddb_i2c_master_xfer,
.functionality = ddb_i2c_functionality,
  };

Hi Gustavo,
Hi all,

please hold this single one patch from the constify patches back for
now, since we're in the process of bumping the whole driver to a newer
version which involves lots of code shuffling. With this, quite some
GIT rebasing work needs to be done, and adding this one liner at a
later time (thus rebasing it) is way easier.

To be sure this will not be forgotten afterwards, I've already posted a
patch applying the exact change at [1].

Thank you very much!

[1] https://patchwork.linuxtv.org/patch/42393/


Thank you!

--
Gustavo A. R. Silva

ir-keytable question [Ubuntu 17.04]

[Szabolcs Andrasi]

Hi,

I'm using Ubuntu 17.04 and I installed the ir-keytable tool. The
output of the ir-keytable command is as follows:



Found /sys/class/rc/rc0/ (/dev/input/event5) with:
Driver ite-cir, table rc-rc6-mce
Supported protocols: unknown other lirc rc-5 rc-5-sz jvc sony nec
sanyo mce_kbd rc-6 sharp xmp
Enabled protocols: lirc rc-6
Name: ITE8708 CIR transceiver
bus: 25, vendor/product: 1283:, version: 0x
Repeat delay = 500 ms, repeat period = 125 ms



I'm trying to enable the supported mce_kbd protocol in addition to the
lirc and rc-6 protocols with the

$ sudo ir-keytable -p lirc -p rc-6 -p mce_kbd

command which works as expected. If, however, I reboot my computer,
ir-keytable forgets this change and only the lirc and rc-6 protocols
are enabled. Is there a configuration file I can edit so that after
the boot my IR remote still works? Or is that so that the only way to
make it work is to write a start-up script that runs the above command
to enable the needed protocol?

Thank you in advance!

-- Szabolcs

[PATCH 1/2] [media] staging/atomisp: fixed trivial coding style warning

[Shy More]

Below was the trivial wanrning flagged by checkpatch.pl
WARNING: Block comments use * on subsequent lines

Signed-off-by: Shy More 
---
 .../css2400/runtime/isys/src/ibuf_ctrl_rmgr.c  | 24 +++---
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
index 76d9142..bb9f5cd 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
@@ -14,18 +14,18 @@
  */
 #else
 /**
-Support for Intel Camera Imaging ISP subsystem.
-Copyright (c) 2010 - 2015, Intel Corporation.
-
-This program is free software; you can redistribute it and/or modify it
-under the terms and conditions of the GNU General Public License,
-version 2, as published by the Free Software Foundation.
-
-This program is distributed in the hope it will be useful, but WITHOUT
-ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-more details.
-*/
+ * Support for Intel Camera Imaging ISP subsystem.
+ * Copyright (c) 2010 - 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
 #endif
 
 #include "system_global.h"
-- 
1.9.1

[PATCH 2/2] [media] staging/atomisp: fixed trivial coding style issue

[Shy More]

Below was the trival error flagged by checkpatch.pl:
ERROR: space prohibited after that open parenthesis '('

Signed-off-by: Shy More 
---
 .../atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
index bb9f5cd..99edb81 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
@@ -131,7 +131,7 @@ void ia_css_isys_ibuf_rmgr_release(
for (i = 0; i < ibuf_rsrc.num_allocated; i++) {
handle = getHandle(i);
if ((handle->start_addr == *start_addr)
-   && ( true == handle->active)) {
+   && (true == handle->active)) {
handle->active = false;
ibuf_rsrc.num_active--;
break;
-- 
1.9.1

Re: [PATCH 2/2] [media] staging/atomisp: fixed trivial coding style issue

[Joe Perches]

On Sun, 2017-07-16 at 16:38 -0700, Shy More wrote:
> Below was the trival error flagged by checkpatch.pl:
> ERROR: space prohibited after that open parenthesis '('
[]
> diff --git 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
>  
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
[]
> @@ -131,7 +131,7 @@ void ia_css_isys_ibuf_rmgr_release(
>   for (i = 0; i < ibuf_rsrc.num_allocated; i++) {
>   handle = getHandle(i);
>   if ((handle->start_addr == *start_addr)
> - && ( true == handle->active)) {
> + && (true == handle->active)) {
>   handle->active = false;
>   ibuf_rsrc.num_active--;
>   break;

Better would have been to remove the comparison to true

if (handle->start_addr == *start_addr && handle->active)

but this would probably read better and perhaps be
marginally faster on some processors if written like:

if (handle->active && handle->start_addr == *start_addr)

Re: [PATCH v2 6/6] dt-bindings: Document the Rockchip RGA bindings

[Laurent Pinchart]

Hi Heiko,

On Sunday 16 Jul 2017 18:07:58 Heiko Stuebner wrote:
> Am Samstag, 15. Juli 2017, 12:23:12 CEST schrieb Laurent Pinchart:
> > On Saturday 15 Jul 2017 14:58:40 Jacob Chen wrote:
> >> Add DT bindings documentation for Rockchip RGA
> >> 
> >> Signed-off-by: Yakir Yang 
> >> Signed-off-by: Jacob Chen 
> >> ---
> >> 
> >>  .../devicetree/bindings/media/rockchip-rga.txt | 35 +++
> >>  1 file changed, 35 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/media/rockchip-rga.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.txt
> >> b/Documentation/devicetree/bindings/media/rockchip-rga.txt new file mode
> >> 100644
> >> index 000..966eba0
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
> >> @@ -0,0 +1,35 @@
> >> +device-tree bindings for rockchip 2D raster graphic acceleration
> >> controller (RGA)
> >> +
> >> +RGA is a separate 2D raster graphic acceleration unit. It accelerates
> >> 2D
> > 
> > "Separate" from what ? Do you mean "standalone" ?
> > 
> >> +graphics operations, such as point/line drawing, image scaling,
> >> rotation,
> >> +BitBLT, alpha blending and image blur/sharpness.
> >> +
> >> +Required properties:
> >> +- compatible: value should be one of the following
> >> +  "rockchip,rk3228-rga";
> >> +  "rockchip,rk3288-rga";
> >> +  "rockchip,rk3399-rga";
> > 
> > The driver in patch 2/6 has match entry for rk3328, which is missing from
> > this list.
> > 
> > As the implementation of the driver doesn't seem to discriminate between
> > the four SoCs, wouldn't it make sense to create a generic compatible
> > string on which the driver would match ? You can have both the generic
> > and SoC-specific compatible strings in DT if there are differences
> > between the IP core in those SoCs that might need to be handled later by
> > the driver.
> 
> I think the block is named something like RGA2 in some kernel trees, but
> am not sure if that is an actual name, or someone just added a number.
> From short glances at vendor rga-code in the past, it looks like there are
> currently 2 basic versions of the in existence I.e. older Rockchip socs
> (like the rk3188 or so) use something older.
> 
> I think everywhere else we do have only (or at least mostly) soc-specifc
> compatibles, but I guess that is more a question what Rob prefers.

Many SoC vendors use SoC-specific compatible string, often in addition to more 
generic ones. When a more generic compatible string is available, drivers can 
match against it, which avoid having to update drivers every time a new SoC 
integrates the same version of the IP core. The SoC-specific compatible string 
is still specified in DT, "just in case" we later realize that the IP core 
integrated in the SoC is slightly different than the other ones and requires 
specific handling in the driver.

I'm not completely happy with that scheme, as it's really a workaround for 
defective communication with the hardware team (or possibly defective hardware 
development...), but that's the best compromise we have so far.

> >> +- interrupts: RGA interrupt number.
> > 
> > This is an "interrupt specifier", not just an "interrupt number" (as you
> > can see in the example below there are three numbers)
> > 
> >> +
> >> +- clocks: phandle to RGA sclk/hclk/aclk clocks
> >> +
> >> +- clock-names: should be "aclk" "hclk" and "sclk"
> > 
> > Nitpicking, there should be a comma after "aclk".
> > 
> >> +
> >> +- resets: Must contain an entry for each entry in reset-names.
> >> +  See ../reset/reset.txt for details.
> >> +- reset-names: should be "core" "axi" and "ahb"
> > 
> > And a comma after "core".
> > 
> >> +
> >> +Example:
> >> +SoC specific DT entry:
> >
> > s/SoC specific/SoC-specific/
> > 
> >> +  rga: rga@ff68 {
> >> +  compatible = "rockchip,rk3399-rga";
> >> +  reg = <0xff68 0x1>;
> >> +  interrupts = ;
> >> +  interrupt-names = "rga";
> > 
> > The interrupt-names property is not described above. Do you really need it
> > ?
> >
> >> +  clocks = < ACLK_RGA>, < HCLK_RGA>, <
> >> SCLK_RGA_CORE>;
> >> +  clock-names = "aclk", "hclk", "sclk";
> >> +
> >> +  resets = < SRST_RGA_CORE>, < SRST_A_RGA>, <
> >> SRST_H_RGA>;
> >> +  reset-names = "core, "axi", "ahb";
> >> +  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/6] ARM: dts: rockchip: enable RGA for rk3288 devices

[Jacob Chen]

Hi Laurent,

2017-07-17 10:28 GMT+08:00 Laurent Pinchart :
> Hi Jacob,
>
> On Sunday 16 Jul 2017 12:23:02 Jacob Chen wrote:
>> 2017-07-15 17:16 GMT+08:00 Laurent Pinchart:
>> > On Saturday 15 Jul 2017 14:58:39 Jacob Chen wrote:
>> >> Signed-off-by: Jacob Chen 
>> >> ---
>> >>
>> >>  arch/arm/boot/dts/rk3288-evb.dtsi | 4 
>> >>  arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 4 
>> >>  arch/arm/boot/dts/rk3288-firefly.dtsi | 4 
>> >>  arch/arm/boot/dts/rk3288-miqi.dts | 4 
>> >>  arch/arm/boot/dts/rk3288-popmetal.dts | 4 
>> >>  arch/arm/boot/dts/rk3288-tinker.dts   | 4 
>> >
>> > Some boards are missing from this list (Fennec, Phycore, ...) What
>> > criteria have you used to decide on which ones to enable the RGA ? That
>> > should be explained in the commit message.
>>
>> Ok.
>>
>> I just enable the boards i have tested, because i can't make sure it
>> won't break the other board because of clocks or power-domains.
>
> Given the clocks and power domains shouldn't be board-specific, would it make
> sense to try and get the change tested on the remaining boards ? You could

Not all drivers have handle power domains and clocks appropriately, It
may triggers bugs,
but since it's a V4l2 driver not DRM driver, i will enable it for all
rk3288 boards.
(DRM device will try to probe in very eraly stage and update
clocks/power-domains...)


> then enable the device in the SoC .dtsi file, which would be much simpler.
>

We have many different version RGA drivers in rockchip downstream kernel.
To keep consistent, i didn't enable it in .dtsi.


>> >>  6 files changed, 24 insertions(+)
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Jacob Chen]

Hi,

2017-07-17 10:43 GMT+08:00 Laurent Pinchart :
> Hi Jacob,
>
> On Sunday 16 Jul 2017 12:19:41 Jacob Chen wrote:
>> 2017-07-16 0:49 GMT+08:00 Personnel:
>> > Le samedi 15 juillet 2017 à 12:42 +0300, Laurent Pinchart a écrit :
>> >> On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
>> >> > Rockchip RGA is a separate 2D raster graphic acceleration unit. It
>> >> > accelerates 2D graphics operations, such as point/line drawing, image
>> >> > scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
>> >> >
>> >> > The drvier is mostly based on s5p-g2d v4l2 m2m driver.
>> >> > And supports various operations from the rendering pipeline.
>> >> >
>> >> >  - copy
>> >> >  - fast solid color fill
>> >> >  - rotation
>> >> >  - flip
>> >> >  - alpha blending
>> >>
>> >> I notice that you don't support the drawing operations. How do you plan
>> >> to support them later through the V4L2 M2M API ? I hate stating the
>> >> obvious, but wouldn't the DRM API be better fit for a graphic accelerator
>> >> ?
>> >
>> > It could fit, maybe, but it really lacks some framework. Also, DRM is
>> > not really meant for M2M operation, and it's also not great for multi-
>> > process. Until recently, there was competing drivers for Exynos, both
>> > implemented in V4L2 and DRM, for similar rational, all DRM ones are
>> > being deprecated/removed.
>> >
>> > I think 2D blitters in V4L2 are fine, but they terribly lack something
>> > to differentiate them from converters/scalers when looking up the HW
>> > list. Could be as simple as a capability flag, if I can suggest. For
>> > the reference, the 2D blitter on IMX6 has been used to implement a live
>> > video mixer in GStreamer.
>> >
>> > https://bugzilla.gnome.org/show_bug.cgi?id=772766
>>
>> We have write a drm RGA driver.
>> https://patchwork.kernel.org/patch/8630841/
>>
>> Here are the reasons that why i rewrite it to V4l2 M2M.
>> 1. V4l2 have a better buffer framework. If it use DRM-GEM to handle buffers,
>> there will be much redundant cache flush, and we have to add much hack code
>> to workaround.
>
> I'm glad to hear that you find buffer handling easy in V4L2 :-)
>
>> 2. This driver will be used in rockchip linux project. We mostly use it to
>> scale/colorconvert/rotate/mix video/camera stream.
>> A V4L2 M2M drvier can be directly used in gstreamer.
>>
>> The disadvantages of V4l2 M2M API is that it's not stateless.
>> It's inconvenient if user change size frequently, but it's OK,
>> we have not yet need this and I think it's possible to extend. ;)
>
> CC'ing Alexandre Courbot. Alex, how's the request API going ? :-)
>
>> >> Additionally, V4L2 M2M has one source and one destination. How do you
>> >> implement alpha blending in that case, which by definition requires at
>> >> least two sources ?
>> >
>> > This type of HW only do in-place blits. When using such a node, the
>> > buffer queued on the V4L2_CAPTURE contains the destination image, and
>> > the buffer queued on the V4L2_OUTPUT is the source image.
>>
>> Yep.
>
> So the device performs bi-directional DMA on the capture queue buffers ?
> Interesting, does videobuf2 support that properly ?
>

>From the code, I think V4L2_CAPTURE and V4L2_OUTPUT are handled
in the same way. In my test, it work properly.


>> >>> The code in rga-hw.c is used to configure regs accroding to operations.
>> >>>
>> >>> The code in rga-buf.c is used to create private mmu table for RGA.
>> >>> The tables is stored in a list, and be removed when buffer is cleanup.
>> >>
>> >> Looking at the implementation it seems to be a scatter-gather list, not
>> >> an MMU. Is that right ? Does the hardware documentation refer to it as an
>> >> MMU ?
>>
>> It's a 1-level MMU... We use it like a scatter-gather list,
>> It's also the reason why we don't use RGA with DRM API.
>
> You might want to explain this in the code, otherwise someone will ask you why
> you don't implement support for the MMU through the IOMMU API. Calling it
> scatter-gather would solve that problem, but if the hardware manual calls it
> an MMU, there's no reason not to use that name in the code.
>

ok, i will add comments

>> >>> Signed-off-by: Jacob Chen 
>> >>> ---
>> >>>
>> >>>  drivers/media/platform/Kconfig|  11 +
>> >>>  drivers/media/platform/Makefile   |   2 +
>> >>>  drivers/media/platform/rockchip-rga/Makefile  |   3 +
>> >>>  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
>> >>>  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
>> >>>  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
>> >>>  drivers/media/platform/rockchip-rga/rga.c | 958 ++
>> >>>  drivers/media/platform/rockchip-rga/rga.h | 111 +++
>> >>>  8 files changed, 2296 insertions(+)
>> >>>  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
>> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
>> >>>  create mode 

Re: [PATCH] stm32-dcmi: constify vb2_ops structure

[Gustavo A. R. Silva]

On 07/07/2017 09:33 AM, Hugues FRUCHET wrote:

Acked-by: Hugues Fruchet 


Thank you, Hugues.


On 07/06/2017 10:05 PM, Gustavo A. R. Silva wrote:

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 
---
   drivers/media/platform/stm32/stm32-dcmi.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 83d32a5..24ef888 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -662,7 +662,7 @@ static void dcmi_stop_streaming(struct vb2_queue *vq)
dcmi->errors_count, dcmi->buffers_count);
   }
   
-static struct vb2_ops dcmi_video_qops = {

+static const struct vb2_ops dcmi_video_qops = {
.queue_setup= dcmi_queue_setup,
.buf_init   = dcmi_buf_init,
.buf_prepare= dcmi_buf_prepare,


--
Gustavo A. R. Silva

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Mon Jul 17 05:00:21 CEST 2017
media-tree git hash:2748e76ddb2967c4030171342ebdd3faa6a5e8e8
media_build git hash:   bc1db0a204a87da86349ea5e64ae0d65e945609d
v4l-utils git hash: 8e68406dae2233e811032dc8e7714c09c818e893
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.11.0-164

linux-git-arm-at91: WARNINGS
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: WARNINGS
linux-git-arm-pxa: OK
linux-git-arm-stm32: 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: WARNINGS
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: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.12.67-i686: OK
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
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.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-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: ERRORS
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: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
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: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

[PATCH v2] [media] staging/atomisp: fixed trivial coding style issue

[Shy More]

Below was the trival error flagged by checkpatch.pl:
ERROR: space prohibited after that open parenthesis '('

Signed-off-by: Shy More 

---
changes in v2:
- made the suggested corrections
---
 .../atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
index bb9f5cd..faef976 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/isys/src/ibuf_ctrl_rmgr.c
@@ -130,8 +130,7 @@ void ia_css_isys_ibuf_rmgr_release(
 
for (i = 0; i < ibuf_rsrc.num_allocated; i++) {
handle = getHandle(i);
-   if ((handle->start_addr == *start_addr)
-   && ( true == handle->active)) {
+   if (handle->active && handle->start_addr == *start_addr) {
handle->active = false;
ibuf_rsrc.num_active--;
break;
-- 
1.9.1

Re: [PATCH] st-delta: constify vb2_ops structures

[Gustavo A. R. Silva]

Thank you, Hugues.


On 07/07/2017 09:33 AM, Hugues FRUCHET wrote:

Acked-by: Hugues Fruchet 

On 07/06/2017 10:14 PM, Gustavo A. R. Silva wrote:

Check for vb2_ops structures that are only stored in the ops field of a
vb2_queue structure. That field is declared const, so vb2_ops structures
that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct vb2_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 
---
   drivers/media/platform/sti/delta/delta-v4l2.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c 
b/drivers/media/platform/sti/delta/delta-v4l2.c
index c6f2e24..ff9850e 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1574,7 +1574,7 @@ static void delta_vb2_frame_stop_streaming(struct 
vb2_queue *q)
   }
   
   /* VB2 queue ops */

-static struct vb2_ops delta_vb2_au_ops = {
+static const struct vb2_ops delta_vb2_au_ops = {
.queue_setup = delta_vb2_au_queue_setup,
.buf_prepare = delta_vb2_au_prepare,
.buf_queue = delta_vb2_au_queue,
@@ -1584,7 +1584,7 @@ static struct vb2_ops delta_vb2_au_ops = {
.stop_streaming = delta_vb2_au_stop_streaming,
   };
   
-static struct vb2_ops delta_vb2_frame_ops = {

+static const struct vb2_ops delta_vb2_frame_ops = {
.queue_setup = delta_vb2_frame_queue_setup,
.buf_prepare = delta_vb2_frame_prepare,
.buf_finish = delta_vb2_frame_finish,


--
Gustavo A. R. Silva

Re: [PATCH 3/3] [media] uvcvideo: skip non-extension unit controls on Oculus Rift Sensors

[Laurent Pinchart]

Hi Philipp,

On Saturday 15 Jul 2017 15:13:45 Philipp Zabel wrote:
> Am Samstag, den 15.07.2017, 12:54 +0300 schrieb Laurent Pinchart:
> > On Friday 14 Jul 2017 22:14:24 Philipp Zabel wrote:
> >> The Oculus Rift Sensors (DK2 and CV1) allow to configure their sensor
> >> chips directly via I2C commands using extension unit controls. The
> >> processing and camera unit controls do not function at all.
> > 
> > Do the processing and camera units they report controls that don't work
> > when  exercised ? Who in a sane state of mind could have designed such a
> > terrible product ?
> 
> Yes. Without this patch I get a bunch of controls that have no effect
> whatsoever.
> 
> > If I understand you correctly, this device requires userspace code that
> > knows  how to program the sensor (and possibly other chips). If that's
> > the case, is there an open-source implementation of that code publicly
> > available ?
>
> Well, it's all still a bit in the experimentation phase. We have an
> implementation to set up the DK2 camera for synchronised exposure
> triggered by the Rift DK2 HMD and to read the calibration data from
> flash, here:
> 
> https://github.com/pH5/ouvrt/blob/master/src/esp570.c
> https://github.com/pH5/ouvrt/blob/master/src/mt9v034.c
> 
> And an even more rough version to set up the CV1 camera for
> synchronised exposure triggered by the Rift CV1 HMD here:
> 
> https://github.com/OpenHMD/OpenHMD-RiftPlayground/blob/master/src/main.c
> 
> The latter is using libusb, as it needs the variable length SPI data
> control.
> 
> Do you think adding a pseudo i2c driver for the eSP570/eSP770u webcam
> controllers and then exposing the sensor chips as V4L2 subdevices could
> be a good idea? We already have a sensor driver for the MT9V034 in the
> DK2 USB camera.

Yes, I think a device-specific driver would make sense, especially if we can 
implement support for the sensor as a standalone V4L2 subdev driver. The 
device only fakes UVC compatibility :-(

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/6] ARM: dts: rockchip: enable RGA for rk3288 devices

[Laurent Pinchart]

Hi Jacob,

On Sunday 16 Jul 2017 12:23:02 Jacob Chen wrote:
> 2017-07-15 17:16 GMT+08:00 Laurent Pinchart:
> > On Saturday 15 Jul 2017 14:58:39 Jacob Chen wrote:
> >> Signed-off-by: Jacob Chen 
> >> ---
> >> 
> >>  arch/arm/boot/dts/rk3288-evb.dtsi | 4 
> >>  arch/arm/boot/dts/rk3288-firefly-reload-core.dtsi | 4 
> >>  arch/arm/boot/dts/rk3288-firefly.dtsi | 4 
> >>  arch/arm/boot/dts/rk3288-miqi.dts | 4 
> >>  arch/arm/boot/dts/rk3288-popmetal.dts | 4 
> >>  arch/arm/boot/dts/rk3288-tinker.dts   | 4 
> > 
> > Some boards are missing from this list (Fennec, Phycore, ...) What
> > criteria have you used to decide on which ones to enable the RGA ? That
> > should be explained in the commit message.
> 
> Ok.
> 
> I just enable the boards i have tested, because i can't make sure it
> won't break the other board because of clocks or power-domains.

Given the clocks and power domains shouldn't be board-specific, would it make 
sense to try and get the change tested on the remaining boards ? You could 
then enable the device in the SoC .dtsi file, which would be much simpler.

> >>  6 files changed, 24 insertions(+)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Laurent Pinchart]

Hi Nicolas,

On Saturday 15 Jul 2017 12:49:13 Personnel wrote:

You might want to fix your mailer to use your name :-)

> Le samedi 15 juillet 2017 à 12:42 +0300, Laurent Pinchart a écrit :
> > On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
> >> Rockchip RGA is a separate 2D raster graphic acceleration unit. It
> >> accelerates 2D graphics operations, such as point/line drawing, image
> >> scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
> >> 
> >> The drvier is mostly based on s5p-g2d v4l2 m2m driver.
> >> And supports various operations from the rendering pipeline.
> >> 
> >>  - copy
> >>  - fast solid color fill
> >>  - rotation
> >>  - flip
> >>  - alpha blending
> >
> > I notice that you don't support the drawing operations. How do you plan to
> > support them later through the V4L2 M2M API ? I hate stating the obvious,
> > but wouldn't the DRM API be better fit for a graphic accelerator ?
> 
> It could fit, maybe, but it really lacks some framework. Also, DRM is
> not really meant for M2M operation, and it's also not great for multi-
> process.

GPUs on embedded devices are mem-to-mem, and they're definitely shared between 
multiple processes :-)

> Until recently, there was competing drivers for Exynos, both
> implemented in V4L2 and DRM, for similar rational, all DRM ones are
> being deprecated/removed.
> 
> I think 2D blitters in V4L2 are fine, but they terribly lack something
> to differentiate them from converters/scalers when looking up the HW
> list. Could be as simple as a capability flag, if I can suggest. For
> the reference, the 2D blitter on IMX6 has been used to implement a live
> video mixer in GStreamer.
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=772766

If we decide that 2D blitters should be supported by V4L2 (and I'm open to get 
convinced about that), we really need to define a proper API before merging a 
bunch of drivers that will implement things in slightly different ways, 
otherwise the future will be very painful.

Among the issues that need to be solved are

- stateful vs. stateless operation (as mentioned by Jacob in this mail 
thread), a.k.a. the request API

- exposing capabilities to userspace (a single capability flag would be enough 
only if all blitters expose the same API, which I'm not sure we can assume)

- single input (a.k.a. in-place blitters as you mentioned below) vs. multiple 
inputs

- API for 2D-accelerated operations other than blitting (filling, point and 
line drawing, ...)

> > Additionally, V4L2 M2M has one source and one destination. How do you
> > implement alpha blending in that case, which by definition requires at
> > least two sources ?
> 
> This type of HW only do in-place blits. When using such a node, the
> buffer queued on the V4L2_CAPTURE contains the destination image, and
> the buffer queued on the V4L2_OUTPUT is the source image.
> 
> >> The code in rga-hw.c is used to configure regs accroding to operations.
> >> 
> >> The code in rga-buf.c is used to create private mmu table for RGA.
> >> The tables is stored in a list, and be removed when buffer is cleanup.
> > 
> > Looking at the implementation it seems to be a scatter-gather list, not an
> > MMU. Is that right ? Does the hardware documentation refer to it as an MMU
> > ?
> >
> >> Signed-off-by: Jacob Chen 
> >> ---
> >> 
> >>  drivers/media/platform/Kconfig|  11 +
> >>  drivers/media/platform/Makefile   |   2 +
> >>  drivers/media/platform/rockchip-rga/Makefile  |   3 +
> >>  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
> >>  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
> >>  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
> >>  drivers/media/platform/rockchip-rga/rga.c | 958 +++
> >>  drivers/media/platform/rockchip-rga/rga.h | 111 +++
> >>  8 files changed, 2296 insertions(+)
> >>  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
> >>  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
> >>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
> >>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
> >>  create mode 100644 drivers/media/platform/rockchip-rga/rga.c
> >>  create mode 100644 drivers/media/platform/rockchip-rga/rga.h

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/6] [media] rockchip/rga: v4l2 m2m support

[Laurent Pinchart]

Hi Jacob,

On Sunday 16 Jul 2017 12:19:41 Jacob Chen wrote:
> 2017-07-16 0:49 GMT+08:00 Personnel:
> > Le samedi 15 juillet 2017 à 12:42 +0300, Laurent Pinchart a écrit :
> >> On Saturday 15 Jul 2017 14:58:36 Jacob Chen wrote:
> >> > Rockchip RGA is a separate 2D raster graphic acceleration unit. It
> >> > accelerates 2D graphics operations, such as point/line drawing, image
> >> > scaling, rotation, BitBLT, alpha blending and image blur/sharpness.
> >> > 
> >> > The drvier is mostly based on s5p-g2d v4l2 m2m driver.
> >> > And supports various operations from the rendering pipeline.
> >> > 
> >> >  - copy
> >> >  - fast solid color fill
> >> >  - rotation
> >> >  - flip
> >> >  - alpha blending
> >> 
> >> I notice that you don't support the drawing operations. How do you plan
> >> to support them later through the V4L2 M2M API ? I hate stating the
> >> obvious, but wouldn't the DRM API be better fit for a graphic accelerator
> >> ?
> > 
> > It could fit, maybe, but it really lacks some framework. Also, DRM is
> > not really meant for M2M operation, and it's also not great for multi-
> > process. Until recently, there was competing drivers for Exynos, both
> > implemented in V4L2 and DRM, for similar rational, all DRM ones are
> > being deprecated/removed.
> > 
> > I think 2D blitters in V4L2 are fine, but they terribly lack something
> > to differentiate them from converters/scalers when looking up the HW
> > list. Could be as simple as a capability flag, if I can suggest. For
> > the reference, the 2D blitter on IMX6 has been used to implement a live
> > video mixer in GStreamer.
> > 
> > https://bugzilla.gnome.org/show_bug.cgi?id=772766
> 
> We have write a drm RGA driver.
> https://patchwork.kernel.org/patch/8630841/
> 
> Here are the reasons that why i rewrite it to V4l2 M2M.
> 1. V4l2 have a better buffer framework. If it use DRM-GEM to handle buffers,
> there will be much redundant cache flush, and we have to add much hack code
> to workaround.

I'm glad to hear that you find buffer handling easy in V4L2 :-)

> 2. This driver will be used in rockchip linux project. We mostly use it to
> scale/colorconvert/rotate/mix video/camera stream.
> A V4L2 M2M drvier can be directly used in gstreamer.
> 
> The disadvantages of V4l2 M2M API is that it's not stateless.
> It's inconvenient if user change size frequently, but it's OK,
> we have not yet need this and I think it's possible to extend. ;)

CC'ing Alexandre Courbot. Alex, how's the request API going ? :-)

> >> Additionally, V4L2 M2M has one source and one destination. How do you
> >> implement alpha blending in that case, which by definition requires at
> >> least two sources ?
> > 
> > This type of HW only do in-place blits. When using such a node, the
> > buffer queued on the V4L2_CAPTURE contains the destination image, and
> > the buffer queued on the V4L2_OUTPUT is the source image.
> 
> Yep.

So the device performs bi-directional DMA on the capture queue buffers ? 
Interesting, does videobuf2 support that properly ?

> >>> The code in rga-hw.c is used to configure regs accroding to operations.
> >>> 
> >>> The code in rga-buf.c is used to create private mmu table for RGA.
> >>> The tables is stored in a list, and be removed when buffer is cleanup.
> >> 
> >> Looking at the implementation it seems to be a scatter-gather list, not
> >> an MMU. Is that right ? Does the hardware documentation refer to it as an
> >> MMU ?
> 
> It's a 1-level MMU... We use it like a scatter-gather list,
> It's also the reason why we don't use RGA with DRM API.

You might want to explain this in the code, otherwise someone will ask you why 
you don't implement support for the MMU through the IOMMU API. Calling it 
scatter-gather would solve that problem, but if the hardware manual calls it 
an MMU, there's no reason not to use that name in the code.

> >>> Signed-off-by: Jacob Chen 
> >>> ---
> >>> 
> >>>  drivers/media/platform/Kconfig|  11 +
> >>>  drivers/media/platform/Makefile   |   2 +
> >>>  drivers/media/platform/rockchip-rga/Makefile  |   3 +
> >>>  drivers/media/platform/rockchip-rga/rga-buf.c | 122 
> >>>  drivers/media/platform/rockchip-rga/rga-hw.c  | 652 ++
> >>>  drivers/media/platform/rockchip-rga/rga-hw.h  | 437 
> >>>  drivers/media/platform/rockchip-rga/rga.c | 958 ++
> >>>  drivers/media/platform/rockchip-rga/rga.h | 111 +++
> >>>  8 files changed, 2296 insertions(+)
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/Makefile
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga-buf.c
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.c
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga-hw.h
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga.c
> >>>  create mode 100644 drivers/media/platform/rockchip-rga/rga.h

-- 
Regards,

Laurent Pinchart

dvb-core/demux.h: fix kernel-doc warning

[Hans Verkuil]

Fix this kernel-doc warning:

WARNING: kernel-doc 'media-git/scripts/kernel-doc -rst -enable-lineno 
media-git/drivers/media/dvb-core/demux.h' processing failed with: 'ascii' codec 
can't decode byte 0xe2 in position 6368: ordinal
not in range(128)

Caused by using fancy quotes instead of regular quotes.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/dvb-core/demux.h b/drivers/media/dvb-core/demux.h
index f854309ba8a5..c4df6cee48e6 100644
--- a/drivers/media/dvb-core/demux.h
+++ b/drivers/media/dvb-core/demux.h
@@ -210,7 +210,7 @@ struct dmx_section_feed {
  * the start of the first undelivered TS packet within a circular buffer.
  * The @buffer2 buffer parameter is normally NULL, except when the received
  * TS packets have crossed the last address of the circular buffer and
- * ”wrapped” to the beginning of the buffer. In the latter case the @buffer1
+ * "wrapped" to the beginning of the buffer. In the latter case the @buffer1
  * parameter would contain an address within the circular buffer, while the
  * @buffer2 parameter would contain the first address of the circular buffer.
  * The number of bytes delivered with this function (i.e. @buffer1_length +

[PATCH] pulse8-cec.rst: add documentation for the pulse8-cec driver

[Hans Verkuil]

Document the persistent_config module option.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/cec-drivers/index.rst  | 32 ++
 Documentation/media/cec-drivers/pulse8-cec.rst | 11 +
 Documentation/media/index.rst  |  1 +
 MAINTAINERS|  1 +
 4 files changed, 45 insertions(+)
 create mode 100644 Documentation/media/cec-drivers/index.rst
 create mode 100644 Documentation/media/cec-drivers/pulse8-cec.rst

diff --git a/Documentation/media/cec-drivers/index.rst 
b/Documentation/media/cec-drivers/index.rst
new file mode 100644
index ..1c817aa10bb6
--- /dev/null
+++ b/Documentation/media/cec-drivers/index.rst
@@ -0,0 +1,32 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. include:: 
+
+.. _cec-drivers:
+
+#
+CEC driver-specific documentation
+#
+
+**Copyright** |copy| 2017 : LinuxTV Developers
+
+This documentation 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 of the License.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+more details.
+
+For more details see the file COPYING in the source distribution of Linux.
+
+.. class:: toc-title
+
+Table of Contents
+
+.. toctree::
+   :maxdepth: 5
+   :numbered:
+
+   pulse8-cec
diff --git a/Documentation/media/cec-drivers/pulse8-cec.rst 
b/Documentation/media/cec-drivers/pulse8-cec.rst
new file mode 100644
index ..99551c6a9bc5
--- /dev/null
+++ b/Documentation/media/cec-drivers/pulse8-cec.rst
@@ -0,0 +1,11 @@
+Pulse-Eight CEC Adapter driver
+==
+
+The pulse8-cec driver implements the following module option:
+
+``persistent_config``
+-
+
+By default this is off, but when set to 1 the driver will store the current
+settings to the device's internal eeprom and restore it the next time the
+device is connected to the USB port.
diff --git a/Documentation/media/index.rst b/Documentation/media/index.rst
index 7f8f0af620ce..7d2907d4f8d7 100644
--- a/Documentation/media/index.rst
+++ b/Documentation/media/index.rst
@@ -10,6 +10,7 @@ Contents:
media_kapi
dvb-drivers/index
v4l-drivers/index
+   cec-drivers/index

 .. only::  subproject

diff --git a/MAINTAINERS b/MAINTAINERS
index c4be6d4af7d2..dd50b91aa81c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10416,6 +10416,7 @@ L:  linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 S: Maintained
 F: drivers/media/usb/pulse8-cec/*
+F: Documentation/media/cec-drivers/pulse8-cec.rst

 PVRUSB2 VIDEO4LINUX DRIVER
 M: Mike Isely 
-- 
2.11.0

[PATCH] pulse8-cec: persistent_config should be off by default

[Hans Verkuil]

The persistent_config option is used to make the CEC settings persistent by 
using
the eeprom inside the device to store this information. This was on by default, 
which
caused confusion since this device now behaves differently from other CEC 
devices
which all come up unconfigured.

Another reason for doing this now is that I hope a more standard way of 
selecting
persistent configuration will be created in the future. And for that to work all
CEC drivers should behave the same and come up unconfigured by default.

None of the open source CEC applications are using this CEC framework at the 
moment
so change this behavior before it is too late.

Signed-off-by: Hans Verkuil 
Cc:   # for v4.10 and up
---
diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c 
b/drivers/media/usb/pulse8-cec/pulse8-cec.c
index c843070f24c1..f9ed9c950247 100644
--- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
+++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
@@ -51,7 +51,7 @@ MODULE_DESCRIPTION("Pulse Eight HDMI CEC driver");
 MODULE_LICENSE("GPL");

 static int debug;
-static int persistent_config = 1;
+static int persistent_config;
 module_param(debug, int, 0644);
 module_param(persistent_config, int, 0644);
 MODULE_PARM_DESC(debug, "debug level (0-1)");

[PATCHv2 2/3] drm/vc4: prepare for CEC support

[Hans Verkuil]

From: Hans Verkuil 

In order to support CEC the hsm clock needs to be enabled in
vc4_hdmi_bind(), not in vc4_hdmi_encoder_enable(). Otherwise you wouldn't
be able to support CEC when there is no hotplug detect signal, which is
required by some monitors that turn off the HPD when in standby, but keep
the CEC bus alive so they can be woken up.

The HDMI core also has to be enabled in vc4_hdmi_bind() for the same
reason.

Signed-off-by: Hans Verkuil 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 59 +-
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ed63d4e85762..e0104f96011e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -463,11 +463,6 @@ static void vc4_hdmi_encoder_disable(struct drm_encoder 
*encoder)
HD_WRITE(VC4_HD_VID_CTL,
 HD_READ(VC4_HD_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
 
-   HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
-   udelay(1);
-   HD_WRITE(VC4_HD_M_CTL, 0);
-
-   clk_disable_unprepare(hdmi->hsm_clock);
clk_disable_unprepare(hdmi->pixel_clock);
 
ret = pm_runtime_put(>pdev->dev);
@@ -509,16 +504,6 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder 
*encoder)
return;
}
 
-   /* This is the rate that is set by the firmware.  The number
-* needs to be a bit higher than the pixel clock rate
-* (generally 148.5Mhz).
-*/
-   ret = clk_set_rate(hdmi->hsm_clock, 163682864);
-   if (ret) {
-   DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
-   return;
-   }
-
ret = clk_set_rate(hdmi->pixel_clock,
   mode->clock * 1000 *
   ((mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1));
@@ -533,20 +518,6 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder 
*encoder)
return;
}
 
-   ret = clk_prepare_enable(hdmi->hsm_clock);
-   if (ret) {
-   DRM_ERROR("Failed to turn on HDMI state machine clock: %d\n",
- ret);
-   clk_disable_unprepare(hdmi->pixel_clock);
-   return;
-   }
-
-   HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
-   udelay(1);
-   HD_WRITE(VC4_HD_M_CTL, 0);
-
-   HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_ENABLE);
-
HDMI_WRITE(VC4_HDMI_SW_RESET_CONTROL,
   VC4_HDMI_SW_RESET_HDMI |
   VC4_HDMI_SW_RESET_FORMAT_DETECT);
@@ -1205,6 +1176,23 @@ static int vc4_hdmi_bind(struct device *dev, struct 
device *master, void *data)
return -EPROBE_DEFER;
}
 
+   /* This is the rate that is set by the firmware.  The number
+* needs to be a bit higher than the pixel clock rate
+* (generally 148.5Mhz).
+*/
+   ret = clk_set_rate(hdmi->hsm_clock, 163682864);
+   if (ret) {
+   DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
+   goto err_put_i2c;
+   }
+
+   ret = clk_prepare_enable(hdmi->hsm_clock);
+   if (ret) {
+   DRM_ERROR("Failed to turn on HDMI state machine clock: %d\n",
+ ret);
+   goto err_put_i2c;
+   }
+
/* Only use the GPIO HPD pin if present in the DT, otherwise
 * we'll use the HDMI core's register.
 */
@@ -1216,7 +1204,7 @@ static int vc4_hdmi_bind(struct device *dev, struct 
device *master, void *data)
 _gpio_flags);
if (hdmi->hpd_gpio < 0) {
ret = hdmi->hpd_gpio;
-   goto err_put_i2c;
+   goto err_unprepare_hsm;
}
 
hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
@@ -1224,6 +1212,14 @@ static int vc4_hdmi_bind(struct device *dev, struct 
device *master, void *data)
 
vc4->hdmi = hdmi;
 
+   /* HDMI core must be enabled. */
+   if (!(HD_READ(VC4_HD_M_CTL) & VC4_HD_M_ENABLE)) {
+   HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
+   udelay(1);
+   HD_WRITE(VC4_HD_M_CTL, 0);
+
+   HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_ENABLE);
+   }
pm_runtime_enable(dev);
 
drm_encoder_init(drm, hdmi->encoder, _hdmi_encoder_funcs,
@@ -1244,6 +1240,8 @@ static int vc4_hdmi_bind(struct device *dev, struct 
device *master, void *data)
 
 err_destroy_encoder:
vc4_hdmi_encoder_destroy(hdmi->encoder);
+err_unprepare_hsm:
+   clk_disable_unprepare(hdmi->hsm_clock);
pm_runtime_disable(dev);
 err_put_i2c:
put_device(>ddc->dev);
@@ -1263,6 +1261,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct 
device *master,
vc4_hdmi_connector_destroy(hdmi->connector);
vc4_hdmi_encoder_destroy(hdmi->encoder);
 
+   

[PATCHv2 3/3] drm/vc4: add HDMI CEC support

[Hans Verkuil]

From: Hans Verkuil 

This patch adds support to VC4 for CEC.

Thanks to Eric Anholt for providing me with the CEC register information.

To prevent the firmware from eating the CEC interrupts you need to add this to
your config.txt:

mask_gpu_interrupt1=0x100

Signed-off-by: Hans Verkuil 
Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/vc4/Kconfig|   8 ++
 drivers/gpu/drm/vc4/vc4_hdmi.c | 227 +++--
 drivers/gpu/drm/vc4/vc4_regs.h | 113 
 3 files changed, 342 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 4361bdcfd28a..fdae18aeab4f 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -19,3 +19,11 @@ config DRM_VC4
  This driver requires that "avoid_warnings=2" be present in
  the config.txt for the firmware, to keep it from smashing
  our display setup.
+
+config DRM_VC4_HDMI_CEC
+   bool "Broadcom VC4 HDMI CEC Support"
+   depends on DRM_VC4
+   select CEC_CORE
+   help
+ Choose this option if you have a Broadcom VC4 GPU
+ and want to use CEC.
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index e0104f96011e..761b95f5deed 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -57,9 +57,14 @@
 #include 
 #include 
 #include 
+#include "media/cec.h"
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
+#define HSM_CLOCK_FREQ 163682864
+#define CEC_CLOCK_FREQ 4
+#define CEC_CLOCK_DIV  (HSM_CLOCK_FREQ / CEC_CLOCK_FREQ)
+
 /* HDMI audio information */
 struct vc4_hdmi_audio {
struct snd_soc_card card;
@@ -85,6 +90,11 @@ struct vc4_hdmi {
int hpd_gpio;
bool hpd_active_low;
 
+   struct cec_adapter *cec_adap;
+   struct cec_msg cec_rx_msg;
+   bool cec_tx_ok;
+   bool cec_irq_was_rx;
+
struct clk *pixel_clock;
struct clk *hsm_clock;
 };
@@ -149,6 +159,23 @@ static const struct {
HDMI_REG(VC4_HDMI_VERTB1),
HDMI_REG(VC4_HDMI_TX_PHY_RESET_CTL),
HDMI_REG(VC4_HDMI_TX_PHY_CTL0),
+
+   HDMI_REG(VC4_HDMI_CEC_CNTRL_1),
+   HDMI_REG(VC4_HDMI_CEC_CNTRL_2),
+   HDMI_REG(VC4_HDMI_CEC_CNTRL_3),
+   HDMI_REG(VC4_HDMI_CEC_CNTRL_4),
+   HDMI_REG(VC4_HDMI_CEC_CNTRL_5),
+   HDMI_REG(VC4_HDMI_CPU_STATUS),
+   HDMI_REG(VC4_HDMI_CPU_MASK_STATUS),
+
+   HDMI_REG(VC4_HDMI_CEC_RX_DATA_1),
+   HDMI_REG(VC4_HDMI_CEC_RX_DATA_2),
+   HDMI_REG(VC4_HDMI_CEC_RX_DATA_3),
+   HDMI_REG(VC4_HDMI_CEC_RX_DATA_4),
+   HDMI_REG(VC4_HDMI_CEC_TX_DATA_1),
+   HDMI_REG(VC4_HDMI_CEC_TX_DATA_2),
+   HDMI_REG(VC4_HDMI_CEC_TX_DATA_3),
+   HDMI_REG(VC4_HDMI_CEC_TX_DATA_4),
 };
 
 static const struct {
@@ -216,8 +243,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
bool force)
if (gpio_get_value_cansleep(vc4->hdmi->hpd_gpio) ^
vc4->hdmi->hpd_active_low)
return connector_status_connected;
-   else
-   return connector_status_disconnected;
+   cec_phys_addr_invalidate(vc4->hdmi->cec_adap);
+   return connector_status_disconnected;
}
 
if (drm_probe_ddc(vc4->hdmi->ddc))
@@ -225,8 +252,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, 
bool force)
 
if (HDMI_READ(VC4_HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
return connector_status_connected;
-   else
-   return connector_status_disconnected;
+   cec_phys_addr_invalidate(vc4->hdmi->cec_adap);
+   return connector_status_disconnected;
 }
 
 static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
@@ -247,6 +274,7 @@ static int vc4_hdmi_connector_get_modes(struct 
drm_connector *connector)
struct edid *edid;
 
edid = drm_get_edid(connector, vc4->hdmi->ddc);
+   cec_s_phys_addr_from_edid(vc4->hdmi->cec_adap, edid);
if (!edid)
return -ENODEV;
 
@@ -1121,6 +1149,159 @@ static void vc4_hdmi_audio_cleanup(struct vc4_hdmi 
*hdmi)
snd_soc_unregister_codec(dev);
 }
 
+#ifdef CONFIG_DRM_VC4_HDMI_CEC
+static irqreturn_t vc4_cec_irq_handler_thread(int irq, void *priv)
+{
+   struct vc4_dev *vc4 = priv;
+   struct vc4_hdmi *hdmi = vc4->hdmi;
+
+   if (hdmi->cec_irq_was_rx) {
+   if (hdmi->cec_rx_msg.len)
+   cec_received_msg(hdmi->cec_adap, >cec_rx_msg);
+   } else if (hdmi->cec_tx_ok) {
+   cec_transmit_done(hdmi->cec_adap, CEC_TX_STATUS_OK,
+ 0, 0, 0, 0);
+   } else {
+   /*
+* This CEC implementation makes 1 retry, so if we
+* get a NACK, then that means it made 2 attempts.
+*/
+   cec_transmit_done(hdmi->cec_adap, 

[PATCHv2 0/3] drm/vc4: add HDMI CEC support

[Hans Verkuil]

From: Hans Verkuil 

This patch series adds support for HDMI CEC to the vc4 drm driver.
This series is based on 4.13-rc1.

Note: the first cec patch is independent of the vc4 patches and will be
merged via the media subsystem for 4.14. Without that patch everything
will still work, but it will attempt to retry messages twice as many
times as it should.

This has been tested with the Raspberry Pi 2B and 3B. I don't have older
rpi's, so I can't test those.

Many thanks to Eric Anholt for his help with this driver!

There is one open issue: when booting the rpi without an HDMI cable
connected, then CEC won't work. But neither apparently does HDMI since
reconnecting it will not bring back any display.

If you boot with the HDMI cable connected, then all is well and
disconnecting and reconnecting the cable will do the right thing.

I don't understand what is going on here, but it does not appear to
be CEC related and the same problem occurs without this patch series.

You also need to update your config.txt with this line to prevent the
firmware from eating the CEC interrupts:

mask_gpu_interrupt1=0x100

Eric, I've experimented with setting hdmi_ignore_cec=1 but that simply
doesn't work. Instead that disables CEC completely. With this
mask_gpu_interrupt1 setting everything works perfectly. This also
prevents the firmware from sending the initial Active Source CEC
message so the CPU has full control over the CEC bus, as it should.

My main concern is that this is rather magical, but it is not
something I have any control over.

Changes since v1:

- Merged patch 3 and 4 together as suggested by Eric.
- The clock divider is now set explicitly instead of relying
  on the default value.

Regards,

Hans

Hans Verkuil (3):
  cec: be smarter about detecting the number of attempts made
  drm/vc4: prepare for CEC support
  drm/vc4: add HDMI CEC support

 drivers/gpu/drm/vc4/Kconfig|   8 ++
 drivers/gpu/drm/vc4/vc4_hdmi.c | 284 -
 drivers/gpu/drm/vc4/vc4_regs.h | 113 
 drivers/media/cec/cec-adap.c   |   9 +-
 4 files changed, 377 insertions(+), 37 deletions(-)

-- 
2.13.2

[PATCHv2 1/3] cec: be smarter about detecting the number of attempts made

[Hans Verkuil]

From: Hans Verkuil 

Some hardware does more than one attempt. So when it calls
cec_transmit_done when an error occurred it will e.g. use an error count
of 2 instead of 1.

The framework always assumed a single attempt, but now it is smarter
and will sum the counters to detect how many attempts were made.

Signed-off-by: Hans Verkuil 
---
 drivers/media/cec/cec-adap.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index bf45977b2823..e9284dbdc880 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -472,9 +472,14 @@ void cec_transmit_done(struct cec_adapter *adap, u8 
status, u8 arb_lost_cnt,
 {
struct cec_data *data;
struct cec_msg *msg;
+   unsigned int attempts_made = arb_lost_cnt + nack_cnt +
+low_drive_cnt + error_cnt;
u64 ts = ktime_get_ns();
 
dprintk(2, "%s: status %02x\n", __func__, status);
+   if (attempts_made < 1)
+   attempts_made = 1;
+
mutex_lock(>lock);
data = adap->transmitting;
if (!data) {
@@ -507,10 +512,10 @@ void cec_transmit_done(struct cec_adapter *adap, u8 
status, u8 arb_lost_cnt,
 * the hardware didn't signal that it retried itself (by setting
 * CEC_TX_STATUS_MAX_RETRIES), then we will retry ourselves.
 */
-   if (data->attempts > 1 &&
+   if (data->attempts > attempts_made &&
!(status & (CEC_TX_STATUS_MAX_RETRIES | CEC_TX_STATUS_OK))) {
/* Retry this message */
-   data->attempts--;
+   data->attempts -= attempts_made;
if (msg->timeout)
dprintk(2, "retransmit: %*ph (attempts: %d, wait for 
0x%02x)\n",
msg->len, msg->msg, data->attempts, msg->reply);
-- 
2.13.2

Re: [PATCH 4/4] drm/vc4: add HDMI CEC support

[Hans Verkuil]

On 12/07/17 21:43, Hans Verkuil wrote:
> On 12/07/17 21:02, Eric Anholt wrote:
>>> +static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 
>>> attempts,
>>> + u32 signal_free_time, struct cec_msg *msg)
>>> +{
>>> +   struct vc4_dev *vc4 = cec_get_drvdata(adap);
>>> +   u32 val;
>>> +   unsigned int i;
>>> +
>>> +   for (i = 0; i < msg->len; i += 4)
>>> +   HDMI_WRITE(VC4_HDMI_CEC_TX_DATA_1 + i,
>>> +  (msg->msg[i]) |
>>> +  (msg->msg[i + 1] << 8) |
>>> +  (msg->msg[i + 2] << 16) |
>>> +  (msg->msg[i + 3] << 24));
>>> +
>>> +   val = HDMI_READ(VC4_HDMI_CEC_CNTRL_1);
>>> +   val &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
>>> +   HDMI_WRITE(VC4_HDMI_CEC_CNTRL_1, val);
>>> +   val &= ~VC4_HDMI_CEC_MESSAGE_LENGTH_MASK;
>>> +   val |= (msg->len - 1) << VC4_HDMI_CEC_MESSAGE_LENGTH_SHIFT;
>>> +   val |= VC4_HDMI_CEC_START_XMIT_BEGIN;
>>
>> It doesn't look to me like len should have 1 subtracted from it.  The
>> field has 4 bits for our up-to-16-byte length, and the firmware seems to
>> be setting it to the same value as a memcpy for the message data uses.
> 
> You need to subtract by one. The CEC protocol supports messages of 1-16
> bytes in length. Since the message length mask is only 4 bits you need to
> encode this in the value 0-15. Hence the '-1', otherwise you would never
> be able to send 16 byte messages.
> 
> I actually found this when debugging the messages it was transmitting: they
> were one too long.
> 
> This suggests that the firmware does this wrong. I don't have time tomorrow,
> but I'll see if I can do a quick test on Friday to verify that.

I double-checked this and both the driver and the firmware do the right thing.
Just to be certain I also tried sending a message that uses the full 16 byte
payload and that too went well. So the code is definitely correct.

Regards,

Hans