[PATCH v2 3/3] media: ov7670: Add the s_power operation

[Wenyou Yang]

Add the s_power operation which is responsible for manipulating the
power dowm mode through the PWDN pin and the reset operation through
the RESET pin.

Signed-off-by: Wenyou Yang 
---

Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

 drivers/media/i2c/ov7670.c | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index efc738112e2a..d1211ae48f63 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1530,6 +1530,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
 }
 #endif
 
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   if (info->pwdn_gpio)
+   gpiod_direction_output(info->pwdn_gpio, !on);
+   if (on && info->resetb_gpio) {
+   gpiod_set_value(info->resetb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(info->resetb_gpio, 0);
+   usleep_range(3000, 5000);
+   }
+
+   return 0;
+}
+
 /* --- */
 
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
@@ -1539,6 +1555,7 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops 
= {
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
 #endif
+   .s_power = ov7670_s_power,
 };
 
 static const struct v4l2_subdev_video_ops ov7670_video_ops = {
@@ -1645,23 +1662,25 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
return ret;
 
-   ret = ov7670_init_gpio(client, info);
-   if (ret)
-   goto clk_disable;
-
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
goto clk_disable;
}
 
+   ret = ov7670_init_gpio(client, info);
+   if (ret)
+   goto clk_disable;
+
+   ov7670_s_power(sd, 1);
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
v4l_dbg(1, debug, client,
"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
client->addr << 1, client->adapter->name);
-   goto clk_disable;
+   goto power_off;
}
v4l_info(client, "chip found @ 0x%02x (%s)\n",
client->addr << 1, client->adapter->name);
@@ -1734,6 +1753,8 @@ static int ov7670_probe(struct i2c_client *client,
media_entity_cleanup(>sd.entity);
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
+power_off:
+   ov7670_s_power(sd, 0);
 clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1749,6 +1770,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_ctrl_handler_free(>hdl);
clk_disable_unprepare(info->clk);
media_entity_cleanup(>sd.entity);
+   ov7670_s_power(sd, 0);
return 0;
 }
 
-- 
2.13.0

[PATCH v2 2/3] media: ov7670: Add the get_fmt callback

[Wenyou Yang]

Add the get_fmt callback, also enable V4L2_SUBDEV_FL_HAS_DEVNODE flag
to make this subdev has device node.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/i2c/ov7670.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 5c8460ee65c3..efc738112e2a 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -230,6 +230,7 @@ struct ov7670_info {
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *hue;
};
+   struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt;  /* Current format */
struct clk *clk;
struct gpio_desc *resetb_gpio;
@@ -973,6 +974,29 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
fmt->width = wsize->width;
fmt->height = wsize->height;
fmt->colorspace = ov7670_formats[index].colorspace;
+
+   info->format.code = fmt->code;
+   info->format.width = fmt->width;
+   info->format.height = fmt->height;
+   info->format.colorspace = fmt->colorspace;
+
+   return 0;
+}
+
+static int ov7670_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+   struct ov7670_info *info = to_state(sd);
+   struct v4l2_mbus_framefmt *fmt;
+
+   if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+   fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+   else
+   fmt = >format;
+
+   format->format = *fmt;
+
return 0;
 }
 
@@ -1526,6 +1550,7 @@ static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
.enum_frame_interval = ov7670_enum_frame_interval,
.enum_frame_size = ov7670_enum_frame_size,
.enum_mbus_code = ov7670_enum_mbus_code,
+   .get_fmt = ov7670_get_fmt,
.set_fmt = ov7670_set_fmt,
 };
 
@@ -1698,6 +1723,7 @@ static int ov7670_probe(struct i2c_client *client,
 
v4l2_ctrl_handler_setup(>hdl);
 
+   info->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
ret = v4l2_async_register_subdev(>sd);
if (ret < 0)
goto entity_cleanup;
-- 
2.13.0

[PATCH v2 1/3] media: ov7670: Add entity pads initialization

[Wenyou Yang]

Add the media entity pads initialization.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/i2c/ov7670.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..5c8460ee65c3 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,7 @@ struct ov7670_devtype {
 struct ov7670_format_struct;  /* coming later */
 struct ov7670_info {
struct v4l2_subdev sd;
+   struct media_pad pad;
struct v4l2_ctrl_handler hdl;
struct {
/* gain cluster */
@@ -1688,14 +1689,23 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_auto_cluster(2, >auto_exposure,
   V4L2_EXPOSURE_MANUAL, false);
v4l2_ctrl_cluster(2, >saturation);
+
+   info->pad.flags = MEDIA_PAD_FL_SOURCE;
+   info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   ret = media_entity_pads_init(>sd.entity, 1, >pad);
+   if (ret < 0)
+   goto hdl_free;
+
v4l2_ctrl_handler_setup(>hdl);
 
ret = v4l2_async_register_subdev(>sd);
if (ret < 0)
-   goto hdl_free;
+   goto entity_cleanup;
 
return 0;
 
+entity_cleanup:
+   media_entity_cleanup(>sd.entity);
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
 clk_disable:
@@ -1712,6 +1722,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
clk_disable_unprepare(info->clk);
+   media_entity_cleanup(>sd.entity);
return 0;
 }
 
-- 
2.13.0

[PATCH v2 0/3] media: ov7670: Add entity init and power operation

[Wenyou Yang]

This patch set is to add the media entity pads initialization,
the s_power operation and get_fmt callback support.

Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

Wenyou Yang (3):
  media: ov7670: Add entity pads initialization
  media: ov7670: Add the get_fmt callback
  media: ov7670: Add the s_power operation

 drivers/media/i2c/ov7670.c | 71 ++
 1 file changed, 65 insertions(+), 6 deletions(-)

-- 
2.13.0

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:   Thu Sep 14 05:00:16 CEST 2017
media-tree git hash:1efdf1776e2253b77413c997bed862410e4b6aaf
media_build git hash:   bbd9f669f0da6705fe44aff89281c0d6e7bfd73e
v4l-utils git hash: d7c41e2576c09f37b33fe8bf2e38615703086045
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.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
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: 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.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-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: 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
linux-4.13-x86_64: OK
apps: WARNINGS
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH v9 0/4] Add Rockchip RGA V4l2 support

[Jacob Chen]

This patch series add a v4l2 m2m drvier for rockchip RGA direct rendering based 
2d graphics acceleration module.

Recently I tried to add protduff support for gstreamer on rockchip platform, 
and i found that API 
were not very suitable for my purpose. 
It shouldn't go upstream until we can figure out what people need, 

change in V9:
- remove protduff things
- test with the latest v4l2-compliance

change in V8:
- remove protduff things

change in V6,V7:
- correct warning in checkpatch.pl

change in V5:
- v4l2-compliance: handle invalid pxielformat 
- v4l2-compliance: add subscribe_event
- add colorspace support

change in V4:
- document the controls.
- change according to Hans's comments

change in V3:
- rename the controls.
- add pm_runtime support.
- enable node by default.
- correct spelling in documents.

change in V2:
- generalize the controls.
- map buffers (10-50 us) in every cmd-run rather than in buffer-import to avoid 
get_free_pages failed on
actively used systems.
- remove status in dt-bindings examples.

Jacob Chen (4):
  rockchip/rga: v4l2 m2m support
  ARM: dts: rockchip: add RGA device node for RK3288
  arm64: dts: rockchip: add RGA device node for RK3399
  dt-bindings: Document the Rockchip RGA bindings

 .../devicetree/bindings/media/rockchip-rga.txt |   33 +
 arch/arm/boot/dts/rk3288.dtsi  |   11 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   11 +
 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  |  156 +++
 drivers/media/platform/rockchip-rga/rga-hw.c   |  435 +
 drivers/media/platform/rockchip-rga/rga-hw.h   |  437 +
 drivers/media/platform/rockchip-rga/rga.c  | 1030 
 drivers/media/platform/rockchip-rga/rga.h  |  110 +++
 11 files changed, 2239 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
 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

-- 
2.7.4

[PATCH v9 4/4] dt-bindings: Document the Rockchip RGA bindings

[Jacob Chen]

Add DT bindings documentation for Rockchip RGA

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/media/rockchip-rga.txt | 33 ++
 1 file changed, 33 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..fd5276a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.txt
@@ -0,0 +1,33 @@
+device-tree bindings for rockchip 2D raster graphic acceleration controller 
(RGA)
+
+RGA is a standalone 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.
+
+Required properties:
+- compatible: value should be one of the following
+   "rockchip,rk3288-rga";
+   "rockchip,rk3399-rga";
+
+- interrupts: RGA interrupt specifier.
+
+- clocks: phandle to RGA sclk/hclk/aclk clocks
+
+- clock-names: should be "aclk", "hclk" and "sclk"
+
+- 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"
+
+Example:
+SoC-specific DT entry:
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0xff68 0x1>;
+   interrupts = ;
+   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";
+   };
-- 
2.7.4

[PATCH v9 3/4] arm64: dts: rockchip: add RGA device node for RK3399

[Jacob Chen]

This patch add the RGA dt config of RK3399 SoC.

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 5b78ce1..fa15770 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1153,6 +1153,17 @@
status = "disabled";
};
 
+   rga: rga@ff68 {
+   compatible = "rockchip,rk3399-rga";
+   reg = <0x0 0xff68 0x0 0x1>;
+   interrupts = ;
+   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";
+   power-domains = < RK3399_PD_RGA>;
+   };
+
efuse0: efuse@ff69 {
compatible = "rockchip,rk3399-efuse";
reg = <0x0 0xff69 0x0 0x80>;
-- 
2.7.4

[PATCH v9 2/4] ARM: dts: rockchip: add RGA device node for RK3288

[Jacob Chen]

This patch add the RGA dt config of rk3288 SoC.

Signed-off-by: Jacob Chen 
Signed-off-by: Yakir Yang 
---
 arch/arm/boot/dts/rk3288.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 595d395..ca6c63a 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -953,6 +953,17 @@
status = "okay";
};
 
+   rga: rga@ff92 {
+   compatible = "rockchip,rk3288-rga";
+   reg = <0xff92 0x180>;
+   interrupts = ;
+   clocks = < ACLK_RGA>, < HCLK_RGA>, < SCLK_RGA>;
+   clock-names = "aclk", "hclk", "sclk";
+   power-domains = < RK3288_PD_VIO>;
+   resets = < SRST_RGA_CORE>, < SRST_RGA_AXI>, < 
SRST_RGA_AHB>;
+   reset-names = "core", "axi", "ahb";
+   };
+
vopb: vop@ff93 {
compatible = "rockchip,rk3288-vop";
reg = <0xff93 0x19c>;
-- 
2.7.4

Re: [PATCH v9 0/4] Add Rockchip RGA V4l2 support

[Jacob Chen]

Hi all,


2017-09-14 9:19 GMT+08:00 Jacob Chen :
> This patch series add a v4l2 m2m drvier for rockchip RGA direct rendering 
> based 2d graphics acceleration module.
>
> Recently I tried to add protduff support for gstreamer on rockchip platform, 
> and i found that API
> were not very suitable for my purpose.
> It shouldn't go upstream until we can figure out what people need,
>
> change in V9:
> - remove protduff things
> - test with the latest v4l2-compliance
>
> change in V8:
> - remove protduff things
>
> change in V6,V7:
> - correct warning in checkpatch.pl
>
> change in V5:
> - v4l2-compliance: handle invalid pxielformat
> - v4l2-compliance: add subscribe_event
> - add colorspace support
>
> change in V4:
> - document the controls.
> - change according to Hans's comments
>
> change in V3:
> - rename the controls.
> - add pm_runtime support.
> - enable node by default.
> - correct spelling in documents.
>
> change in V2:
> - generalize the controls.
> - map buffers (10-50 us) in every cmd-run rather than in buffer-import to 
> avoid get_free_pages failed on
> actively used systems.
> - remove status in dt-bindings examples.
>
> Jacob Chen (4):
>   rockchip/rga: v4l2 m2m support
>   ARM: dts: rockchip: add RGA device node for RK3288
>   arm64: dts: rockchip: add RGA device node for RK3399
>   dt-bindings: Document the Rockchip RGA bindings
>
>  .../devicetree/bindings/media/rockchip-rga.txt |   33 +
>  arch/arm/boot/dts/rk3288.dtsi  |   11 +
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   11 +
>  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  |  156 +++
>  drivers/media/platform/rockchip-rga/rga-hw.c   |  435 +
>  drivers/media/platform/rockchip-rga/rga-hw.h   |  437 +
>  drivers/media/platform/rockchip-rga/rga.c  | 1030 
> 
>  drivers/media/platform/rockchip-rga/rga.h  |  110 +++
>  11 files changed, 2239 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-rga.txt
>  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
>
> --
> 2.7.4
>


v4l2-compliance SHA   : d7c41e2576c09f37b33fe8bf2e38615703086045

Driver Info:
Driver name   : rockchip-rga
Card type : rockchip-rga
Bus info  : platform:rga
Driver version: 4.13.0
Capabilities  : 0x84208000
Video Memory-to-Memory
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x04208000
Video Memory-to-Memory
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

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: 5 Private Controls: 0

Format ioctls:
  

[PATCH v9 1/4] rockchip/rga: v4l2 m2m support

[Jacob Chen]

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

The code in rga-hw.c is used to configure regs according to operations
The code in rga-buf.c is used to create private mmu table for RGA.

changes in V7:
- fix some warning reported by "checkpatch --strict"

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 |  156 
 drivers/media/platform/rockchip-rga/rga-hw.c  |  435 +++
 drivers/media/platform/rockchip-rga/rga-hw.h  |  437 +++
 drivers/media/platform/rockchip-rga/rga.c | 1030 +
 drivers/media/platform/rockchip-rga/rga.h |  110 +++
 8 files changed, 2184 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

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 7e7cc49..9b79350 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -458,6 +458,17 @@ config VIDEO_RENESAS_VSP1
  To compile this driver as a module, choose M here: the module
  will be called vsp1.
 
+config VIDEO_ROCKCHIP_RGA
+   tristate "Rockchip Raster 2d Grapphic Acceleration Unit"
+   depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_ROCKCHIP || COMPILE_TEST
+   select VIDEOBUF2_DMA_SG
+   select V4L2_MEM2MEM_DEV
+   default n
+   ---help---
+ This is a v4l2 driver for Rockchip SOC RGA2
+ 2d graphics accelerator.
+
 config VIDEO_TI_VPE
tristate "TI VPE (Video Processing Engine) driver"
depends on VIDEO_DEV && VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index c1ef946..06039c3 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -62,6 +62,8 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1)  += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1/
 
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA)   += rockchip-rga/
+
 obj-y  += omap/
 
 obj-$(CONFIG_VIDEO_AM437X_VPFE)+= am437x/
diff --git a/drivers/media/platform/rockchip-rga/Makefile 
b/drivers/media/platform/rockchip-rga/Makefile
new file mode 100644
index 000..92fe254
--- /dev/null
+++ b/drivers/media/platform/rockchip-rga/Makefile
@@ -0,0 +1,3 @@
+rockchip-rga-objs := rga.o rga-hw.o rga-buf.o
+
+obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip-rga.o
diff --git a/drivers/media/platform/rockchip-rga/rga-buf.c 
b/drivers/media/platform/rockchip-rga/rga-buf.c
new file mode 100644
index 000..373c36f
--- /dev/null
+++ b/drivers/media/platform/rockchip-rga/rga-buf.c
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) 2017 Fuzhou Rockchip Electronics Co.Ltd
+ * Author: Jacob Chen 
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "rga-hw.h"
+#include "rga.h"
+
+static int
+rga_queue_setup(struct vb2_queue *vq,
+   unsigned int *nbuffers, unsigned int *nplanes,
+   unsigned int sizes[], struct device *alloc_devs[])
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(vq);
+   struct rga_frame *f = rga_get_frame(ctx, vq->type);
+
+   if (IS_ERR(f))
+   return PTR_ERR(f);
+
+   if (*nplanes)
+   return sizes[0] < f->size ? -EINVAL : 0;
+
+   sizes[0] = f->size;
+   *nplanes = 1;
+
+   return 0;
+}
+
+static int rga_buf_prepare(struct vb2_buffer *vb)
+{
+   struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+   struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
+
+   if (IS_ERR(f))
+   return PTR_ERR(f);
+
+   vb2_set_plane_payload(vb, 0, f->size);
+
+   

Re: [PATCH v4 4/4] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

On 09/13/2017 09:13 PM, Sylwester Nawrocki wrote:

On 09/13/2017 01:41 PM, Hoegeun Kwon wrote:
@@ -1004,11 +1088,33 @@ static irqreturn_t gsc_irq_handler(int irq, 
void *priv)

  .num_clocks = 1,
  };
  +static struct gsc_driverdata gsc_v_5250_drvdata = {
+.variant = {
+[0] = _v_5250_variant,
+[1] = _v_5250_variant,
+[2] = _v_5250_variant,
+[3] = _v_5250_variant,
+},
+.num_entities = 4,
+.clk_names = { "gscl" },
+.num_clocks = 1,
+};
+
+static struct gsc_driverdata gsc_v_5420_drvdata = {
+.variant = {
+[0] = _v_5420_variant,
+[1] = _v_5420_variant,
+},
+.num_entities = 4,


Shouldn't num_enities be 2 here? You don't need to resend, I can
amend that when applying.




Yes, num_enities is 2.
Sorry I made a mistake.

Thanks Sylwester.

Best regards,
Hoegeun


+.clk_names = { "gscl" },
+.num_clocks = 1,
+};
+


--
Regards,
Sylwester

Investi?ná príležitos?

[Yi Tan]

Ahoj,

Obsah tejto správy je ve?mi dôverný. moje meno je Yi Tan, pracujem s bankou tu 
v Hongkongu.

Rozhodla som sa, ?e vás budem kontaktova? za mo?nos? investova? do akéhoko?vek 
lukratívneho podnikania vo va?ej krajine, som ochotný ponúknu? 40% ako môj 
obchodný partner.

Ak máte záujem, odpovedzte mi na viac informácií na mojom súkromnom e-maile: 
yihsbcta...@gmail.com

S pozdravom: Yi Tan





This email was sent by the shareware version of Postman Professional.

[PATCH v1] media: ov13858: Fix 4224x3136 video flickering at some vblanks

[Chiranjeevi Rapolu]

Previously, the sensor was outputting blank every other frame at 4224x3136
video when vblank was in the range [79, 86]. This resulted in video
flickering.

Omni Vision recommends us to use their settings for crop start/end for
4224x3136.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/ov13858.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index af7af0d..f7c5771 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -238,11 +238,11 @@ struct ov13858_mode {
{0x3800, 0x00},
{0x3801, 0x00},
{0x3802, 0x00},
-   {0x3803, 0x00},
+   {0x3803, 0x08},
{0x3804, 0x10},
{0x3805, 0x9f},
{0x3806, 0x0c},
-   {0x3807, 0x5f},
+   {0x3807, 0x57},
{0x3808, 0x10},
{0x3809, 0x80},
{0x380a, 0x0c},
-- 
1.9.1

Re: [PATCH 2/3] dt: bindings: as3645a: Use LED number to refer to LEDs

[Rob Herring]

On Fri, Sep 08, 2017 at 03:42:12PM +0300, Sakari Ailus wrote:
> Use integers (reg property) to tell the number of the LED to the driver
> instead of the node name. While both of these approaches are currently
> used by the LED bindings, using integers will require less driver changes
> for ACPI support. Additionally, it will make possible LED naming using
> chip and LED node names, effectively making the label property most useful
> for human-readable names only.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  .../devicetree/bindings/leds/ams,as3645a.txt   | 28 
> ++
>  1 file changed, 18 insertions(+), 10 deletions(-)

Acked-by: Rob Herring 

[PATCH] [media] saa7146: make saa7146_use_ops const

[Bhumika Goyal]

Make these const as they are not modified in the file referencing them.
They are only used when their function pointer fields invokes a
function and therefore none of the structure fields are getting modified.
Also, add a const to the declaration in the header.

Signed-off-by: Bhumika Goyal 
---
 drivers/media/common/saa7146/saa7146_vbi.c   | 2 +-
 drivers/media/common/saa7146/saa7146_video.c | 2 +-
 include/media/drv-intf/saa7146_vv.h  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/saa7146/saa7146_vbi.c 
b/drivers/media/common/saa7146/saa7146_vbi.c
index d79e4d7..69525ca 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -488,7 +488,7 @@ static ssize_t vbi_read(struct file *file, char __user 
*data, size_t count, loff
return ret;
 }
 
-struct saa7146_use_ops saa7146_vbi_uops = {
+const struct saa7146_use_ops saa7146_vbi_uops = {
.init   = vbi_init,
.open   = vbi_open,
.release= vbi_close,
diff --git a/drivers/media/common/saa7146/saa7146_video.c 
b/drivers/media/common/saa7146/saa7146_video.c
index 37b4654..51eeed8 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1303,7 +1303,7 @@ static ssize_t video_read(struct file *file, char __user 
*data, size_t count, lo
return ret;
 }
 
-struct saa7146_use_ops saa7146_video_uops = {
+const struct saa7146_use_ops saa7146_video_uops = {
.init = video_init,
.open = video_open,
.release = video_close,
diff --git a/include/media/drv-intf/saa7146_vv.h 
b/include/media/drv-intf/saa7146_vv.h
index 0da6ccc..736f4f2 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -202,14 +202,14 @@ void saa7146_dma_free(struct saa7146_dev* dev,struct 
videobuf_queue *q,
 /* from saa7146_video.c */
 extern const struct v4l2_ioctl_ops saa7146_video_ioctl_ops;
 extern const struct v4l2_ioctl_ops saa7146_vbi_ioctl_ops;
-extern struct saa7146_use_ops saa7146_video_uops;
+extern const struct saa7146_use_ops saa7146_video_uops;
 int saa7146_start_preview(struct saa7146_fh *fh);
 int saa7146_stop_preview(struct saa7146_fh *fh);
 long saa7146_video_do_ioctl(struct file *file, unsigned int cmd, void *arg);
 int saa7146_s_ctrl(struct v4l2_ctrl *ctrl);
 
 /* from saa7146_vbi.c */
-extern struct saa7146_use_ops saa7146_vbi_uops;
+extern const struct saa7146_use_ops saa7146_vbi_uops;
 
 /* resource management functions */
 int saa7146_res_get(struct saa7146_fh *fh, unsigned int bit);
-- 
1.9.1

Re: as3645a flash userland interface

[Jacek Anaszewski]

On 09/12/2017 11:55 PM, Pavel Machek wrote:
> On Tue 2017-09-12 20:53:33, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 09/12/2017 12:36 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>> There were some changes to as3645a flash controller. Before we have
>>> stable interface we have to keep forever I want to ask:
>>
>> Note that we have already two LED flash class drivers - leds-max77693
>> and leds-aat1290. They have been present in mainline for over two years
>> now.
> 
> Well.. that's ok. No change there is neccessary.
> 
>>> What directory are the flash controls in?
>>>
>>> /sys/class/leds/led-controller:flash ?
>>>
>>> Could we arrange for something less generic, like
>>>
>>> /sys/class/leds/main-camera:flash ?
>>
>> I'd rather avoid overcomplicating this. LED class device name pattern
>> is well defined to devicename:colour:function
>> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
>>
>> In this case "flash" in place of the "function" segment makes the
>> things clear enough I suppose.
> 
> It does not.
> 
> Phones usually have two cameras, front and back, and these days both
> cameras have their flash.
> 
> And poor userspace flashlight application can not know if as3645
> drivers front LED or back LED. Thus, I'd set devicename to
> front-camera or main-camera -- because that's what it is associated
> with. Userspace does not care what hardware drives the LED, but needs
> to know if it is front or back camera.

The name of a LED flash class device isn't fixed and is derived
from DT label property. Name in the example of some DT bindings
will not force people to apply similar pattern for the other
drivers and even for the related one. No worry about having
to keep anything forever basing on that.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH] [media] s3c-camif: fix out-of-bounds array access

[Sylwester Nawrocki]

On 09/13/2017 04:03 PM, Arnd Bergmann wrote:

On Wed, Sep 13, 2017 at 11:25 AM, Sylwester Nawrocki
  wrote:

On 09/12/2017 10:09 PM, Arnd Bergmann wrote:

   {
   const struct s3c_camif_variant *variant = camif->variant;
   const struct vp_pix_limits *pix_lim;
- int i = ARRAY_SIZE(camif_mbus_formats);

   /* FIXME: constraints against codec or preview path ? */
   pix_lim = >vp_pix_limits[VP_CODEC];

- while (i-- >= 0)
- if (camif_mbus_formats[i] == mf->code)
- break;
-
- mf->code = camif_mbus_formats[i];


Interesting finding... the function needs to ensure mf->code is set
to one of supported values by the driver, so instead of removing
how about changing the above line to:

 if (i < 0)
 mf->code = camif_mbus_formats[0];

?

That would still have one of the two out-of-bounds accesses;-)


Ah, indeed :/


maybe this

for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
 if (camif_mbus_formats[i] == mf->code)
break;

if (i == ARRAY_SIZE(camif_mbus_formats))
mf->code = camif_mbus_formats[0];


Yes, it should work that way.

--
Thanks,
Sylwester

Re: [PATCH v2 0/4] BCM283x Camera Receiver driver

[Dave Stevenson]

(dropping to linux-media and linux-rpi-kernel mailing lists as the
device tree folk aren't going to be bothered by v4l2-compliance
results)

v4l2-compliance results:

TC358743 (having loaded an EDID config)

v4l2-compliance SHA   : f6ecbc90656815d91dc6ba90aac0ad8193a14b38

Driver Info:
Driver name   : unicam
Card type : unicam
Bus info  : platform:unicam 3f801000.csi1
Driver version: 4.13.0
Capabilities  : 0x8521
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0521
Video Capture
Read/Write
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK
test VIDIOC_DV_TIMINGS_CAP: OK
test VIDIOC_G/S_EDID: OK

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: 3 Private Controls: 2

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
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 (Not Supported)

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 VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Test input 0:

Stream using all formats:
test MMAP for Format RGB3, Frame Size 1920x1080:
Stride 5760, Field None: OK
Stride 5824, Field None: OK
test MMAP for Format UYVY, Frame Size 1920x1080:
Stride 3840, Field None: OK
Stride 3904, Field None: OK

Total: 47, Succeeded: 47, Failed: 0, Warnings: 0

--
ADV7282-M
Minor hack required to select the first valid input (in my case
CVBS_AIN1). The hardware default is DIFF_CVBS_AIN1_AIN2.

v4l2-compliance SHA   : f6ecbc90656815d91dc6ba90aac0ad8193a14b38

Driver Info:
Driver name   : unicam
Card type : unicam
Bus info  : platform:unicam 3f801000.csi1
Driver version: 4.13.0
Capabilities  : 0x8521
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0521
Video Capture
Read/Write
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test 

Re: IR driver support for tango platforms

[Mason]

On 13/09/2017 16:57, Sean Young wrote:


On Sep 13, 2017 at 16:03, Mason wrote:


Changes from v1 to v2:

o Rebase driver on top of linuxtv/master
o Use ir_nec_bytes_to_scancode() in tango_ir_handle_nec()
o Use devm_rc_allocate_device() in tango_ir_probe()
o Use Use devm_rc_register_device() in tango_ir_probe()
o Rename rc->input_name to rc->device_name (not sure what value to use here)
o List all NEC variants for rc->allowed_protocols
o Change type of clkrate to u64
o Fix tango_ir_probe and tango_ir_remove for devm
o Move around some init calls in tango_ir_probe() for devm
o Use relaxed variants of MMIO accessors

TODO: test RC-5 and RC-6 (I need to locate proper remote)


You could get a IR transmitter (e.g. raspberry pi + IR led + resistor) or
some of the mceusb devices, and then you can use the ir-ctl tool to
test all the different protocols, including extended rc5 and the other
rc6 variants.


Thanks for the suggestions.

I do have a box full of remote controls, and I'm hoping some of
them are RC-5 and RC-6. (Someone told me there is a Sony decoder
in the chip, but I have found no documentation whatsoever regarding
that feature!)

There is an IR transmitter on the board, but I have no driver for
it, only a custom test app. So that doesn't help me for ir-ctl...
I don't know how much time a driver would require.


But I don't think we need to block merging because these protocols haven't
been tested. It would be nice though.


I'll try my best to test the driver thoroughly. And then I'll
send a formal patch.

Regards.

[PATCH v2 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface

[Dave Stevenson]

Add driver for the Unicam camera receiver block on
BCM283x processors.

Signed-off-by: Dave Stevenson 
---
 drivers/media/platform/Kconfig   |1 +
 drivers/media/platform/Makefile  |1 +
 drivers/media/platform/bcm2835/Kconfig   |   14 +
 drivers/media/platform/bcm2835/Makefile  |3 +
 drivers/media/platform/bcm2835/bcm2835-unicam.c  | 2192 ++
 drivers/media/platform/bcm2835/vc4-regs-unicam.h |  264 +++
 6 files changed, 2475 insertions(+)
 create mode 100644 drivers/media/platform/bcm2835/Kconfig
 create mode 100644 drivers/media/platform/bcm2835/Makefile
 create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
 create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 7e7cc49..1e5f004 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -150,6 +150,7 @@ source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 source "drivers/media/platform/rcar-vin/Kconfig"
 source "drivers/media/platform/atmel/Kconfig"
+source "drivers/media/platform/bcm2835/Kconfig"
 
 config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index c1ef946..045de3f 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -90,3 +90,4 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
qcom/camss-8x16/
 obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
 
 obj-y  += meson/
+obj-y  += bcm2835/
diff --git a/drivers/media/platform/bcm2835/Kconfig 
b/drivers/media/platform/bcm2835/Kconfig
new file mode 100644
index 000..6a74842
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Kconfig
@@ -0,0 +1,14 @@
+# Broadcom VideoCore4 V4L2 camera support
+
+config VIDEO_BCM2835_UNICAM
+   tristate "Broadcom BCM2835 Unicam video capture driver"
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on ARCH_BCM2835 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_FWNODE
+   ---help---
+ Say Y here to enable V4L2 subdevice for CSI2 receiver.
+ This is a V4L2 subdevice that interfaces directly to the VC4 
peripheral.
+
+  To compile this driver as a module, choose M here. The module
+  will be called bcm2835-unicam.
diff --git a/drivers/media/platform/bcm2835/Makefile 
b/drivers/media/platform/bcm2835/Makefile
new file mode 100644
index 000..a98aba0
--- /dev/null
+++ b/drivers/media/platform/bcm2835/Makefile
@@ -0,0 +1,3 @@
+# Makefile for BCM2835 Unicam driver
+
+obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o
diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c 
b/drivers/media/platform/bcm2835/bcm2835-unicam.c
new file mode 100644
index 000..5b1adc3
--- /dev/null
+++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c
@@ -0,0 +1,2192 @@
+/*
+ * BCM2835 Unicam capture Driver
+ *
+ * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd.
+ *
+ * Dave Stevenson 
+ *
+ * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and
+ * TI CAL camera interface driver by Benoit Parrot.
+ *
+ *
+ * There are two camera drivers in the kernel for BCM283x - this one
+ * and bcm2835-camera (currently in staging).
+ *
+ * This driver is purely the kernel control the Unicam peripheral - there
+ * is no involvement with the VideoCore firmware. Unicam receives CSI-2
+ * or CCP2 data and writes it into SDRAM. The only processing options are
+ * to repack Bayer data into an alternate format, and applying windowing
+ * (currently not implemented).
+ * It should be possible to connect it to any sensor with a
+ * suitable output interface and V4L2 subdevice driver.
+ *
+ * bcm2835-camera uses with the VideoCore firmware to control the sensor,
+ * Unicam, ISP, and all tuner control loops. Fully processed frames are
+ * delivered to the driver by the firmware. It only has sensor drivers
+ * for Omnivision OV5647, and Sony IMX219 sensors.
+ *
+ * The two drivers are mutually exclusive for the same Unicam instance.
+ * The VideoCore firmware checks the device tree configuration during boot.
+ * If it finds device tree nodes called csi0 or csi1 it will block the
+ * firmware from accessing the peripheral, and bcm2835-camera will
+ * not be able to stream data.
+ *
+ *
+ * This program is free software; you may 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.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * 

[PATCH v2 4/4] MAINTAINERS: Add entry for BCM2835 camera driver

[Dave Stevenson]

Signed-off-by: Dave Stevenson 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eb930eb..b47ddaa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2713,6 +2713,13 @@ S:   Maintained
 N: bcm2835
 F: drivers/staging/vc04_services
 
+BROADCOM BCM2835 CAMERA DRIVER
+M: Dave Stevenson 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/bcm2835/
+F: Documentation/devicetree/bindings/media/bcm2835-unicam.txt
+
 BROADCOM BCM47XX MIPS ARCHITECTURE
 M: Hauke Mehrtens 
 M: Rafał Miłecki 
-- 
2.7.4

[PATCH v2 0/4] BCM283x Camera Receiver driver

[Dave Stevenson]

Hi All.

This is v2 for adding a V4L2 subdevice driver for the CSI2/CCP2 camera
receiver peripheral on BCM283x, as used on Raspberry Pi.
Sorry for the delay since v1 - other tasks assigned, got sucked
into investigating why some devices were misbehaving, and
picking up on new features that had been added to the tree (eg CCP2).

v4l2-compliance results depend on the sensor subdevice connected to.
I have results for TC358743, ADV7282M, and OV5647 that I'll
send them as a follow up email.

OV647 and ADV7282M are now working with this driver, as well as TC358743.
v1 of the driver only supported continuous clock mode which Unicam was
failing to lock on to correctly.
The driver now checks the clock mode and adjusts termination accordingly.
Something is still a little off for OV5647, but I'll investigate that
later.

As per the v1 discussion with Hans, I have added text describing the
differences between this driver and the one in staging/vc04_service.
Addressing some of the issues in the bcm2835-camera driver is on my to-do
list, and I'll add similar text there when I'm dealing with that.

For those wanting to see the driver in context,
https://github.com/6by9/upstream-linux/tree/unicam is the linux-media
tree with my mods on top. It also includes a couple of TC358743 and
OV5647 driver updates that I'll send to the list in the next few days.

Thanks in advance.
  Dave

Changes from v1 to v2:
- Broken out a new helper function v4l2_fourcc2s as requested by Hans.
- Documented difference between this driver and the bcm2835-camera driver
  in staging/vc04_services.
- Corrected handling of s_dv_timings and s_std to update the current format
  but only if not streaming. This refactored some of the s_fmt code to
  remove duplication.
- Updated handling of sizeimage to include vertical padding. (Not updated
  the bytesperline calcs as the app can override).
- Added support for continuous clock mode (requires changes to lane
  termination configuration).
- Add support for CCP2 as Sakari's patches to support it have now been merged.
  I don't have a suitable sensor to test it with at present, but all settings
  have been taken from a known working configuration. If people would prefer
  I remove this until it has been proved against hardware then I'm happy to
  do so.
- Updated DT bindings to use  on the Unicam node to set the
  maximum number of lanes present instead of a having a custom property.
  Documents the mandatory endpoint properties.
- Removed RAW16 from the list of input formats as it isn't defined in the
  CSI-2 spec. The peripheral can still unpack the other Bayer formats to
  a 16 bit/pixel packing though.
- Added a log-status handler to get the status from the sensor.
- Automatically switch away from any interlaced formats reported via g_fmt,
  or that are attempted to be set via try/s_fmt.
- Addressed other more minor code review comments from v1.

Dave Stevenson (4):
  [media] v4l2-common: Add helper function for fourcc to string
  [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
  [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface
  MAINTAINERS: Add entry for BCM2835 camera driver

 .../devicetree/bindings/media/bcm2835-unicam.txt   |  107 +
 MAINTAINERS|7 +
 drivers/media/platform/Kconfig |1 +
 drivers/media/platform/Makefile|1 +
 drivers/media/platform/bcm2835/Kconfig |   14 +
 drivers/media/platform/bcm2835/Makefile|3 +
 drivers/media/platform/bcm2835/bcm2835-unicam.c| 2192 
 drivers/media/platform/bcm2835/vc4-regs-unicam.h   |  264 +++
 drivers/media/v4l2-core/v4l2-common.c  |   18 +
 include/media/v4l2-common.h|3 +
 10 files changed, 2610 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt
 create mode 100644 drivers/media/platform/bcm2835/Kconfig
 create mode 100644 drivers/media/platform/bcm2835/Makefile
 create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c
 create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h

-- 
2.7.4

[PATCH v2 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver

[Dave Stevenson]

Document the DT bindings for the CSI2/CCP2 receiver peripheral
(known as Unicam) on BCM283x SoCs.

Signed-off-by: Dave Stevenson 
---
 .../devicetree/bindings/media/bcm2835-unicam.txt   | 107 +
 1 file changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt

diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt 
b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
new file mode 100644
index 000..2ee5af7
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
@@ -0,0 +1,107 @@
+Broadcom BCM283x Camera Interface (Unicam)
+--
+
+The Unicam block on BCM283x SoCs is the receiver for either
+CSI-2 or CCP2 data from image sensors or similar devices.
+
+There are two camera drivers in the kernel for BCM283x - this one
+and bcm2835-camera (currently in staging).
+
+This driver is purely the kernel controlling the Unicam peripheral - there
+is no involvement with the VideoCore firmware. Unicam receives CSI-2
+(or CCP2) data and writes it into SDRAM. There is no additional processing
+performed.
+It should be possible to connect it to any sensor with a
+suitable output interface and V4L2 subdevice driver.
+
+bcm2835-camera uses the VideoCore firmware to control the sensor,
+Unicam, ISP, and various tuner control loops. Fully processed frames are
+delivered to the driver by the firmware. It only has sensor drivers
+for Omnivision OV5647, and Sony IMX219 sensors, and is closed source.
+
+The two drivers are mutually exclusive for the same Unicam instance.
+The firmware checks the device tree configuration during boot. If
+it finds device tree nodes called csi0 or csi1 then it will block the
+firmware from accessing the peripheral, and bcm2835-camera will
+not be able to stream data.
+It should be possible to use bcm2835-camera on one camera interface
+and bcm2835-unicam on the other interface if there is a need to.
+
+Required properties:
+===
+- compatible   : must be "brcm,bcm2835-unicam".
+- reg  : physical base address and length of the register sets for the
+ device.
+- interrupts   : should contain the IRQ line for this Unicam instance.
+- clocks   : list of clock specifiers, corresponding to entries in
+ clock-names property.
+- clock-names  : must contain an "lp_clock" entry, matching entries
+ in the clocks property.
+
+Unicam supports a single port node. 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.
+
+Within the endpoint node, the following properties are mandatory:
+- remote-endpoint  : links to the source device endpoint.
+- data-lanes   : An array denoting how many data lanes are physically
+ present for this CSI-2 receiver instance. This can
+ be limited by either the SoC itself, or by the
+ breakout on the platform.
+ Lane reordering is not supported, so lanes must be
+ in order, starting at 1.
+
+Lane reordering is not supported on the clock lane, so the optional property
+"clock-lane" will implicitly be <0>.
+Similarly lane inversion is not supported, therefore "lane-polarities" will
+implicitly be <0 0 0 0 0>.
+Neither of these values will be checked.
+
+Example:
+   csi1: csi@7e801000 {
+   compatible = "brcm,bcm2835-unicam";
+   reg = <0x7e801000 0x800>,
+ <0x7e802004 0x4>;
+   interrupts = <2 7>;
+   clocks = < BCM2835_CLOCK_CAM1>;
+   clock-names = "lp_clock";
+
+   port {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   csi1_ep: endpoint {
+   remote-endpoint = <_0>;
+   data-lanes = <1 2>;
+   };
+   };
+   };
+
+   i2c0: i2c@7e205000 {
+
+   tc358743: csi-hdmi-bridge@0f {
+   compatible = "toshiba,tc358743";
+   reg = <0x0f>;
+   status = "okay";
+
+   clocks = <_clk>;
+   clock-names = "refclk";
+
+   tc358743_clk: bridge-clk {
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <2700>;
+   };
+
+   port {
+   tc358743_0: endpoint {
+   remote-endpoint = <_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1 

[PATCH v2 1/4] [media] v4l2-common: Add helper function for fourcc to string

[Dave Stevenson]

New helper function char *v4l2_fourcc2s(u32 fourcc, char *buf)
that converts a fourcc into a nice printable version.

Signed-off-by: Dave Stevenson 
---
 drivers/media/v4l2-core/v4l2-common.c | 18 ++
 include/media/v4l2-common.h   |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index a5ea1f5..0219895 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -405,3 +405,21 @@ void v4l2_get_timestamp(struct timeval *tv)
tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 }
 EXPORT_SYMBOL_GPL(v4l2_get_timestamp);
+
+char *v4l2_fourcc2s(u32 fourcc, char *buf)
+{
+   buf[0] = fourcc & 0x7f;
+   buf[1] = (fourcc >> 8) & 0x7f;
+   buf[2] = (fourcc >> 16) & 0x7f;
+   buf[3] = (fourcc >> 24) & 0x7f;
+   if (fourcc & (1 << 31)) {
+   buf[4] = '-';
+   buf[5] = 'B';
+   buf[6] = 'E';
+   buf[7] = '\0';
+   } else {
+   buf[4] = '\0';
+   }
+   return buf;
+}
+EXPORT_SYMBOL_GPL(v4l2_fourcc2s);
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index aac8b7b..5b0fff9 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -264,4 +264,7 @@ const struct v4l2_frmsize_discrete 
*v4l2_find_nearest_format(
 
 void v4l2_get_timestamp(struct timeval *tv);
 
+#define V4L2_FOURCC_MAX_SIZE 8
+char *v4l2_fourcc2s(u32 fourcc, char *buf);
+
 #endif /* V4L2_COMMON_H_ */
-- 
2.7.4

Re: IR driver support for tango platforms

[Sean Young]

Hi,

On Wed, Sep 13, 2017 at 04:03:43PM +0200, Mason wrote:
> On 12/09/2017 20:19, Sean Young wrote:
> 
> > It looks great, thanks! I have made some minor points below.
> 
> Thanks for having reviewed the driver! :-)
> 
> I have now fixed all the points you mentioned.
> 
> Changes from v1 to v2:
> 
> o Rebase driver on top of linuxtv/master
> o Use ir_nec_bytes_to_scancode() in tango_ir_handle_nec()
> o Use devm_rc_allocate_device() in tango_ir_probe()
> o Use Use devm_rc_register_device() in tango_ir_probe()
> o Rename rc->input_name to rc->device_name (not sure what value to use here)
> o List all NEC variants for rc->allowed_protocols
> o Change type of clkrate to u64
> o Fix tango_ir_probe and tango_ir_remove for devm
> o Move around some init calls in tango_ir_probe() for devm
> o Use relaxed variants of MMIO accessors
> 
> TODO: test RC-5 and RC-6 (I need to locate proper remote)

You could get a IR transmitter (e.g. raspberry pi + IR led + resistor) or
some of the mceusb devices, and then you can use the ir-ctl tool to
test all the different protocols, including extended rc5 and the other
rc6 variants.

But I don't think we need to block merging because these protocols haven't
been tested. It would be nice though.

> 
> 
> /*
>  * Copyright (C) 2015 Mans Rullgard 
>  *
>  * 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;  either version 2 of the  License, or (at your
>  * option) any later version.
>  */
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #define IR_NEC_CTRL   0x00
> #define IR_NEC_DATA   0x04
> #define IR_CTRL   0x08
> #define IR_RC5_CLK_DIV0x0c
> #define IR_RC5_DATA   0x10
> #define IR_INT0x14
> 
> #define NEC_TIME_BASE 560
> #define RC5_TIME_BASE 1778
> 
> #define RC6_CTRL  0x00
> #define RC6_CLKDIV0x04
> #define RC6_DATA0 0x08
> #define RC6_DATA1 0x0c
> #define RC6_DATA2 0x10
> #define RC6_DATA3 0x14
> #define RC6_DATA4 0x18
> 
> #define RC6_CARRIER   36000
> #define RC6_TIME_BASE 16
> 
> struct tango_ir {
>   void __iomem *rc5_base;
>   void __iomem *rc6_base;
>   struct rc_dev *rc;
>   struct clk *clk;
> };
> 
> static void tango_ir_handle_nec(struct tango_ir *ir)
> {
>   u32 v, code;
>   enum rc_proto proto;
> 
>   v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
>   if (!v) {
>   rc_repeat(ir->rc);
>   return;
>   }
> 
>   code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
>   rc_keydown(ir->rc, proto, code, 0);
> }
> 
> static void tango_ir_handle_rc5(struct tango_ir *ir)
> {
>   u32 data, field, toggle, addr, cmd, code;
> 
>   data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
>   if (data & BIT(31))
>   return;
> 
>   field = data >> 12 & 1;
>   toggle = data >> 11 & 1;
>   addr = data >> 6 & 0x1f;
>   cmd = (data & 0x3f) | (field ^ 1) << 6;
> 
>   code = RC_SCANCODE_RC5(addr, cmd);
>   rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
> }
> 
> static void tango_ir_handle_rc6(struct tango_ir *ir)
> {
>   u32 data0, data1, toggle, mode, addr, cmd, code;
> 
>   data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
>   data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);
> 
>   mode = data0 >> 1 & 7;
>   if (mode != 0)
>   return;
> 
>   toggle = data0 & 1;
>   addr = data0 >> 16;
>   cmd = data1;
> 
>   code = RC_SCANCODE_RC6_0(addr, cmd);
>   rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
> }
> 
> static irqreturn_t tango_ir_irq(int irq, void *dev_id)
> {
>   struct tango_ir *ir = dev_id;
>   unsigned int rc5_stat;
>   unsigned int rc6_stat;
> 
>   rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
>   writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);
> 
>   rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
>   writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);
> 
>   if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
>   return IRQ_NONE;
> 
>   if (rc5_stat & BIT(0))
>   tango_ir_handle_rc5(ir);
> 
>   if (rc5_stat & BIT(1))
>   tango_ir_handle_nec(ir);
> 
>   if (rc6_stat & BIT(31))
>   tango_ir_handle_rc6(ir);
> 
>   return IRQ_HANDLED;
> }
> 
> #define DISABLE_NEC   (BIT(4) | BIT(8))
> #define ENABLE_RC5(BIT(0) | BIT(9))
> #define ENABLE_RC6(BIT(0) | BIT(7))
> 
> static int tango_change_protocol(struct rc_dev *dev, u64 *rc_type)
> {
>   struct tango_ir *ir = dev->priv;
>   u32 rc5_ctrl = DISABLE_NEC;
>   u32 rc6_ctrl = 0;
> 
>   if (*rc_type & RC_PROTO_BIT_NEC)
>   rc5_ctrl = 0;
> 
>   if (*rc_type & RC_PROTO_BIT_RC5)
>   rc5_ctrl |= ENABLE_RC5;
> 
>   if (*rc_type & RC_PROTO_BIT_RC6_0)
>   

Re: IR driver support for tango platforms

[Mason]

On 12/09/2017 20:19, Sean Young wrote:


It looks great, thanks! I have made some minor points below.


Thanks for having reviewed the driver! :-)

I have now fixed all the points you mentioned.

Changes from v1 to v2:

o Rebase driver on top of linuxtv/master
o Use ir_nec_bytes_to_scancode() in tango_ir_handle_nec()
o Use devm_rc_allocate_device() in tango_ir_probe()
o Use Use devm_rc_register_device() in tango_ir_probe()
o Rename rc->input_name to rc->device_name (not sure what value to use here)
o List all NEC variants for rc->allowed_protocols
o Change type of clkrate to u64
o Fix tango_ir_probe and tango_ir_remove for devm
o Move around some init calls in tango_ir_probe() for devm
o Use relaxed variants of MMIO accessors

TODO: test RC-5 and RC-6 (I need to locate proper remote)


/*
 * Copyright (C) 2015 Mans Rullgard 
 *
 * 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;  either version 2 of the  License, or (at your
 * option) any later version.
 */

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#define IR_NEC_CTRL 0x00
#define IR_NEC_DATA 0x04
#define IR_CTRL 0x08
#define IR_RC5_CLK_DIV  0x0c
#define IR_RC5_DATA 0x10
#define IR_INT  0x14

#define NEC_TIME_BASE   560
#define RC5_TIME_BASE   1778

#define RC6_CTRL0x00
#define RC6_CLKDIV  0x04
#define RC6_DATA0   0x08
#define RC6_DATA1   0x0c
#define RC6_DATA2   0x10
#define RC6_DATA3   0x14
#define RC6_DATA4   0x18

#define RC6_CARRIER 36000
#define RC6_TIME_BASE   16

struct tango_ir {
void __iomem *rc5_base;
void __iomem *rc6_base;
struct rc_dev *rc;
struct clk *clk;
};

static void tango_ir_handle_nec(struct tango_ir *ir)
{
u32 v, code;
enum rc_proto proto;

v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
if (!v) {
rc_repeat(ir->rc);
return;
}

code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
rc_keydown(ir->rc, proto, code, 0);
}

static void tango_ir_handle_rc5(struct tango_ir *ir)
{
u32 data, field, toggle, addr, cmd, code;

data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
if (data & BIT(31))
return;

field = data >> 12 & 1;
toggle = data >> 11 & 1;
addr = data >> 6 & 0x1f;
cmd = (data & 0x3f) | (field ^ 1) << 6;

code = RC_SCANCODE_RC5(addr, cmd);
rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
}

static void tango_ir_handle_rc6(struct tango_ir *ir)
{
u32 data0, data1, toggle, mode, addr, cmd, code;

data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);

mode = data0 >> 1 & 7;
if (mode != 0)
return;

toggle = data0 & 1;
addr = data0 >> 16;
cmd = data1;

code = RC_SCANCODE_RC6_0(addr, cmd);
rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
}

static irqreturn_t tango_ir_irq(int irq, void *dev_id)
{
struct tango_ir *ir = dev_id;
unsigned int rc5_stat;
unsigned int rc6_stat;

rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);

rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);

if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
return IRQ_NONE;

if (rc5_stat & BIT(0))
tango_ir_handle_rc5(ir);

if (rc5_stat & BIT(1))
tango_ir_handle_nec(ir);

if (rc6_stat & BIT(31))
tango_ir_handle_rc6(ir);

return IRQ_HANDLED;
}

#define DISABLE_NEC (BIT(4) | BIT(8))
#define ENABLE_RC5  (BIT(0) | BIT(9))
#define ENABLE_RC6  (BIT(0) | BIT(7))

static int tango_change_protocol(struct rc_dev *dev, u64 *rc_type)
{
struct tango_ir *ir = dev->priv;
u32 rc5_ctrl = DISABLE_NEC;
u32 rc6_ctrl = 0;

if (*rc_type & RC_PROTO_BIT_NEC)
rc5_ctrl = 0;

if (*rc_type & RC_PROTO_BIT_RC5)
rc5_ctrl |= ENABLE_RC5;

if (*rc_type & RC_PROTO_BIT_RC6_0)
rc6_ctrl = ENABLE_RC6;

writel_relaxed_relaxed(rc5_ctrl, ir->rc5_base + IR_CTRL);
writel_relaxed_relaxed(rc6_ctrl, ir->rc6_base + RC6_CTRL);

return 0;
}

static int tango_ir_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
struct rc_dev *rc;
struct tango_ir *ir;
struct resource *rc5_res;
struct resource *rc6_res;
u64 clkrate, clkdiv;
int irq, err;

rc5_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!rc5_res)
return -EINVAL;

rc6_res = 

Re: [PATCH] [media] s3c-camif: fix out-of-bounds array access

[Arnd Bergmann]

On Wed, Sep 13, 2017 at 11:25 AM, Sylwester Nawrocki
 wrote:
> On 09/12/2017 10:09 PM, Arnd Bergmann wrote:

>>   {
>>   const struct s3c_camif_variant *variant = camif->variant;
>>   const struct vp_pix_limits *pix_lim;
>> - int i = ARRAY_SIZE(camif_mbus_formats);
>>
>>   /* FIXME: constraints against codec or preview path ? */
>>   pix_lim = >vp_pix_limits[VP_CODEC];
>>
>> - while (i-- >= 0)
>> - if (camif_mbus_formats[i] == mf->code)
>> - break;
>> -
>> - mf->code = camif_mbus_formats[i];
>
>
> Interesting finding... the function needs to ensure mf->code is set
> to one of supported values by the driver, so instead of removing
> how about changing the above line to:
>
> if (i < 0)
> mf->code = camif_mbus_formats[0];
>
> ?

That would still have one of the two out-of-bounds accesses ;-)

maybe this

for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
if (camif_mbus_formats[i] == mf->code)
   break;

if (i == ARRAY_SIZE(camif_mbus_formats))
   mf->code = camif_mbus_formats[0];

  Arnd

Re: [PATCH] s5p-cec: add NACK detection support

[Sylwester Nawrocki]

On 08/31/2017 06:56 PM, Hans Verkuil wrote:

The s5p-cec driver returned CEC_TX_STATUS_ERROR for the NACK condition.

Some digging into the datasheet uncovered the S5P_CEC_TX_STAT1 register where
bit 0 indicates if the transmit was nacked or not.

Use this to return the correct CEC_TX_STATUS_NACK status to userspace.

This was the only driver that couldn't tell a NACK from another error, and
that was very unusual. And a potential problem for applications as well.

Tested with my Odroid-U3.

Signed-off-by: Hans Verkuil 


Acked-by: Sylwester Nawrocki 

Re: [PATCH v4 0/4] Exynos-gsc: Support the hardware rotation limits

[Sylwester Nawrocki]

On 09/13/2017 01:41 PM, Hoegeun Kwon wrote:

Hoegeun Kwon (4):
   [media] exynos-gsc: Add compatible for Exynos 5250 and 5420 specific
 version
   ARM: dts: exynos: Add clean name of compatible.
   drm/exynos/gsc: Add hardware rotation limits
   [media] exynos-gsc: Add hardware rotation limits


Thanks Hoegeung, I have applied patches 1 nad 4 from this series.

--
Regards,
Sylwester

Re: [PATCH v4 4/4] [media] exynos-gsc: Add hardware rotation limits

[Sylwester Nawrocki]

On 09/13/2017 01:41 PM, Hoegeun Kwon wrote:

@@ -1004,11 +1088,33 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
.num_clocks = 1,
  };
  
+static struct gsc_driverdata gsc_v_5250_drvdata = {

+   .variant = {
+   [0] = _v_5250_variant,
+   [1] = _v_5250_variant,
+   [2] = _v_5250_variant,
+   [3] = _v_5250_variant,
+   },
+   .num_entities = 4,
+   .clk_names = { "gscl" },
+   .num_clocks = 1,
+};
+
+static struct gsc_driverdata gsc_v_5420_drvdata = {
+   .variant = {
+   [0] = _v_5420_variant,
+   [1] = _v_5420_variant,
+   },
+   .num_entities = 4,


Shouldn't num_enities be 2 here? You don't need to resend, I can
amend that when applying.


+   .clk_names = { "gscl" },
+   .num_clocks = 1,
+};
+


--
Regards,
Sylwester

[PATCH v4 0/4] Exynos-gsc: Support the hardware rotation limits

[Hoegeun Kwon]

Hello all,

Frist, thanks Krzysztof, Robin and Sylwester.

The gscaler has hardware rotation limits. So this patch set support
the rotate hardware limits of gsc.

Changes for V4:
- Fixed the most specific compatible come first in device tree.
- Kept compatible("samsung,exynos5-gsc") in ther driver.
- Added mark compatible("samsung,exynos5-gsc") as deprecated.
- Added print dmesg if your driver uses compatible("samsung, exynos5-gsc").
- Removed the patch 5, 6 of ver3.

Changes for V3:
- Fixed of_match_node() to of_device_get_match_data() in drm gsc driver.
- Added hardware rotation limits for gsc driver of v4l2.
- Added the remove unnecessary compatible for DT document and Exynos dts.

Changes for V2:
- Added the interface info in binding document.
- Added clean name of compatible in Exynos dts.
- Added maximum supported picture size hardcoded into driver.

Best regards,
Hoegeun

Hoegeun Kwon (4):
  [media] exynos-gsc: Add compatible for Exynos 5250 and 5420 specific
version
  ARM: dts: exynos: Add clean name of compatible.
  drm/exynos/gsc: Add hardware rotation limits
  [media] exynos-gsc: Add hardware rotation limits

 .../devicetree/bindings/media/exynos5-gsc.txt  |   9 +-
 arch/arm/boot/dts/exynos5250.dtsi  |   8 +-
 arch/arm/boot/dts/exynos5420.dtsi  |   4 +-
 drivers/gpu/drm/exynos/exynos_drm_gsc.c| 104 -
 drivers/media/platform/exynos-gsc/gsc-core.c   | 127 -
 include/uapi/drm/exynos_drm.h  |   2 +
 6 files changed, 211 insertions(+), 43 deletions(-)

-- 
1.9.1

[PATCH v4 4/4] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

The hardware rotation limits of gsc depends on SOC (Exynos
5250/5420/5433). Distinguish them and add them to the driver data.

Signed-off-by: Hoegeun Kwon 
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 127 +--
 1 file changed, 122 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 4380150..173a238 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -958,6 +958,51 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
.target_rot_en_h= 2016,
 };
 
+static struct gsc_pix_max gsc_v_5250_max = {
+   .org_scaler_bypass_w= 8192,
+   .org_scaler_bypass_h= 8192,
+   .org_scaler_input_w = 4800,
+   .org_scaler_input_h = 3344,
+   .real_rot_dis_w = 4800,
+   .real_rot_dis_h = 3344,
+   .real_rot_en_w  = 2016,
+   .real_rot_en_h  = 2016,
+   .target_rot_dis_w   = 4800,
+   .target_rot_dis_h   = 3344,
+   .target_rot_en_w= 2016,
+   .target_rot_en_h= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5420_max = {
+   .org_scaler_bypass_w= 8192,
+   .org_scaler_bypass_h= 8192,
+   .org_scaler_input_w = 4800,
+   .org_scaler_input_h = 3344,
+   .real_rot_dis_w = 4800,
+   .real_rot_dis_h = 3344,
+   .real_rot_en_w  = 2048,
+   .real_rot_en_h  = 2048,
+   .target_rot_dis_w   = 4800,
+   .target_rot_dis_h   = 3344,
+   .target_rot_en_w= 2016,
+   .target_rot_en_h= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5433_max = {
+   .org_scaler_bypass_w= 8192,
+   .org_scaler_bypass_h= 8192,
+   .org_scaler_input_w = 4800,
+   .org_scaler_input_h = 3344,
+   .real_rot_dis_w = 4800,
+   .real_rot_dis_h = 3344,
+   .real_rot_en_w  = 2047,
+   .real_rot_en_h  = 2047,
+   .target_rot_dis_w   = 4800,
+   .target_rot_dis_h   = 3344,
+   .target_rot_en_w= 2016,
+   .target_rot_en_h= 2016,
+};
+
 static struct gsc_pix_min gsc_v_100_min = {
.org_w  = 64,
.org_h  = 32,
@@ -992,6 +1037,45 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
.local_sc_down  = 2,
 };
 
+static struct gsc_variant gsc_v_5250_variant = {
+   .pix_max= _v_5250_max,
+   .pix_min= _v_100_min,
+   .pix_align  = _v_100_align,
+   .in_buf_cnt = 32,
+   .out_buf_cnt= 32,
+   .sc_up_max  = 8,
+   .sc_down_max= 16,
+   .poly_sc_down_max   = 4,
+   .pre_sc_down_max= 4,
+   .local_sc_down  = 2,
+};
+
+static struct gsc_variant gsc_v_5420_variant = {
+   .pix_max= _v_5420_max,
+   .pix_min= _v_100_min,
+   .pix_align  = _v_100_align,
+   .in_buf_cnt = 32,
+   .out_buf_cnt= 32,
+   .sc_up_max  = 8,
+   .sc_down_max= 16,
+   .poly_sc_down_max   = 4,
+   .pre_sc_down_max= 4,
+   .local_sc_down  = 2,
+};
+
+static struct gsc_variant gsc_v_5433_variant = {
+   .pix_max= _v_5433_max,
+   .pix_min= _v_100_min,
+   .pix_align  = _v_100_align,
+   .in_buf_cnt = 32,
+   .out_buf_cnt= 32,
+   .sc_up_max  = 8,
+   .sc_down_max= 16,
+   .poly_sc_down_max   = 4,
+   .pre_sc_down_max= 4,
+   .local_sc_down  = 2,
+};
+
 static struct gsc_driverdata gsc_v_100_drvdata = {
.variant = {
[0] = _v_100_variant,
@@ -1004,11 +1088,33 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
.num_clocks = 1,
 };
 
+static struct gsc_driverdata gsc_v_5250_drvdata = {
+   .variant = {
+   [0] = _v_5250_variant,
+   [1] = _v_5250_variant,
+   [2] = _v_5250_variant,
+   [3] = _v_5250_variant,
+   },
+   .num_entities = 4,
+   .clk_names = { "gscl" },
+   .num_clocks = 1,
+};
+
+static struct gsc_driverdata gsc_v_5420_drvdata = {
+   .variant = {
+   [0] = _v_5420_variant,
+   [1] = _v_5420_variant,
+   },
+   .num_entities = 4,
+   .clk_names = { "gscl" },
+   .num_clocks = 1,
+};
+
 static struct gsc_driverdata gsc_5433_drvdata = {
.variant = {
-   [0] = _v_100_variant,
-   [1] = _v_100_variant,
-   [2] = _v_100_variant,
+   [0] = _v_5433_variant,
+   [1] = _v_5433_variant,
+   [2] = 

[PATCH v4 3/4] drm/exynos/gsc: Add hardware rotation limits

[Hoegeun Kwon]

The gscaler has hardware rotation limits that need to be hardcoded
into driver. Distinguish them and add them to the property list.

The hardware rotation limits are related to the cropped source size.
When swap occurs, use rot_max size instead of crop_max size.

Also the scaling limits are related to pos size, use pos size to check
the limits.

Signed-off-by: Hoegeun Kwon 
---
 drivers/gpu/drm/exynos/exynos_drm_gsc.c | 104 +++-
 include/uapi/drm/exynos_drm.h   |   2 +
 2 files changed, 77 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c 
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 0506b2b..66a19d7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -150,6 +151,15 @@ struct gsc_context {
boolsuspended;
 };
 
+/*
+ * struct gsc_driverdata - per device type driver data for init time.
+ *
+ * @rot_max: rotation max resolution.
+ */
+struct gsc_driverdata {
+   struct drm_exynos_sz rot_max;
+};
+
 /* 8-tap Filter Coefficient */
 static const int h_coef_8t[GSC_COEF_RATIO][GSC_COEF_ATTR][GSC_COEF_H_8T] = {
{   /* Ratio <= 65536 (~8:8) */
@@ -1401,6 +1411,23 @@ static int gsc_ippdrv_check_property(struct device *dev,
bool swap;
int i;
 
+   config = >config[EXYNOS_DRM_OPS_DST];
+
+   /* check for degree */
+   switch (config->degree) {
+   case EXYNOS_DRM_DEGREE_90:
+   case EXYNOS_DRM_DEGREE_270:
+   swap = true;
+   break;
+   case EXYNOS_DRM_DEGREE_0:
+   case EXYNOS_DRM_DEGREE_180:
+   swap = false;
+   break;
+   default:
+   DRM_ERROR("invalid degree.\n");
+   goto err_property;
+   }
+
for_each_ipp_ops(i) {
if ((i == EXYNOS_DRM_OPS_SRC) &&
(property->cmd == IPP_CMD_WB))
@@ -1416,21 +1443,6 @@ static int gsc_ippdrv_check_property(struct device *dev,
goto err_property;
}
 
-   /* check for degree */
-   switch (config->degree) {
-   case EXYNOS_DRM_DEGREE_90:
-   case EXYNOS_DRM_DEGREE_270:
-   swap = true;
-   break;
-   case EXYNOS_DRM_DEGREE_0:
-   case EXYNOS_DRM_DEGREE_180:
-   swap = false;
-   break;
-   default:
-   DRM_ERROR("invalid degree.\n");
-   goto err_property;
-   }
-
/* check for buffer bound */
if ((pos->x + pos->w > sz->hsize) ||
(pos->y + pos->h > sz->vsize)) {
@@ -1438,21 +1450,27 @@ static int gsc_ippdrv_check_property(struct device *dev,
goto err_property;
}
 
+   /*
+* The rotation hardware limits are related to the cropped
+* source size. So use rot_max size to check the limits when
+* swap happens. And also the scaling limits are related to pos
+* size, use pos size to check the limits.
+*/
/* check for crop */
if ((i == EXYNOS_DRM_OPS_SRC) && (pp->crop)) {
if (swap) {
if ((pos->h < pp->crop_min.hsize) ||
-   (sz->vsize > pp->crop_max.hsize) ||
+   (pos->h > pp->rot_max.hsize) ||
(pos->w < pp->crop_min.vsize) ||
-   (sz->hsize > pp->crop_max.vsize)) {
+   (pos->w > pp->rot_max.vsize)) {
DRM_ERROR("out of crop size.\n");
goto err_property;
}
} else {
if ((pos->w < pp->crop_min.hsize) ||
-   (sz->hsize > pp->crop_max.hsize) ||
+   (pos->w > pp->crop_max.hsize) ||
(pos->h < pp->crop_min.vsize) ||
-   (sz->vsize > pp->crop_max.vsize)) {
+   (pos->h > pp->crop_max.vsize)) {
DRM_ERROR("out of crop size.\n");
goto err_property;
}
@@ -1463,17 +1481,17 @@ static int gsc_ippdrv_check_property(struct device *dev,
if ((i == EXYNOS_DRM_OPS_DST) && (pp->scale)) {
if (swap) {
if ((pos->h < pp->scale_min.hsize) ||
-

[PATCH v4 2/4] ARM: dts: exynos: Add clean name of compatible.

[Hoegeun Kwon]

Exynos 5250 and 5420 have different hardware rotation limits. However,
currently it uses only one compatible - "exynos5-gsc". Since we have
to distinguish between these two, we add different compatible.

Signed-off-by: Hoegeun Kwon 
---
 arch/arm/boot/dts/exynos5250.dtsi | 8 
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 8dbeb87..d0d0460 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -637,7 +637,7 @@
};
 
gsc_0:  gsc@13e0 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5250-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e0 0x1000>;
interrupts = ;
power-domains = <_gsc>;
@@ -647,7 +647,7 @@
};
 
gsc_1:  gsc@13e1 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5250-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e1 0x1000>;
interrupts = ;
power-domains = <_gsc>;
@@ -657,7 +657,7 @@
};
 
gsc_2:  gsc@13e2 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5250-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e2 0x1000>;
interrupts = ;
power-domains = <_gsc>;
@@ -667,7 +667,7 @@
};
 
gsc_3:  gsc@13e3 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5250-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e3 0x1000>;
interrupts = ;
power-domains = <_gsc>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
b/arch/arm/boot/dts/exynos5420.dtsi
index 02d2f89..6166730 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -658,7 +658,7 @@
};
 
gsc_0: video-scaler@13e0 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5420-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e0 0x1000>;
interrupts = ;
clocks = < CLK_GSCL0>;
@@ -668,7 +668,7 @@
};
 
gsc_1: video-scaler@13e1 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5420-gsc", 
"samsung,exynos5-gsc";
reg = <0x13e1 0x1000>;
interrupts = ;
clocks = < CLK_GSCL1>;
-- 
1.9.1

[PATCH v4 1/4] [media] exynos-gsc: Add compatible for Exynos 5250 and 5420 specific version

[Hoegeun Kwon]

Exynos 5250 and 5420 have different hardware rotation limits.
Since we have to distinguish between these two, we add different
compatible(samsung,exynos5250-gsc and samsung,exynos5420-gsc).

Signed-off-by: Hoegeun Kwon 
---
 Documentation/devicetree/bindings/media/exynos5-gsc.txt | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt 
b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
index 26ca25b..0d4fdae 100644
--- a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
+++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
@@ -3,8 +3,11 @@
 G-Scaler is used for scaling and color space conversion on EXYNOS5 SoCs.
 
 Required properties:
-- compatible: should be "samsung,exynos5-gsc" (for Exynos 5250, 5420 and
- 5422 SoCs) or "samsung,exynos5433-gsc" (Exynos 5433)
+- compatible: should be one of
+ "samsung,exynos5250-gsc"
+ "samsung,exynos5420-gsc"
+ "samsung,exynos5433-gsc"
+ "samsung,exynos5-gsc" (deprecated)
 - reg: should contain G-Scaler physical address location and length.
 - interrupts: should contain G-Scaler interrupt number
 
@@ -15,7 +18,7 @@ Optional properties:
 Example:
 
 gsc_0:  gsc@0x13e0 {
-   compatible = "samsung,exynos5-gsc";
+   compatible = "samsung,exynos5250-gsc";
reg = <0x13e0 0x1000>;
interrupts = <0 85 0>;
 };
-- 
1.9.1

[PATCH v3 7/9] v4l: vsp1: Adapt entities to configure into a body

[Kieran Bingham]

Currently the entities store their configurations into a display list.
Adapt this such that the code can be configured into a body directly,
allowing greater flexibility and control of the content.

All users of vsp1_dl_list_write() are removed in this process, thus it
too is removed.

A helper, vsp1_dl_list_get_body() is provided to access the internal body0
from the display list.

Signed-off-by: Kieran Bingham 

---
I don't like the fact this adds vsp1_dl_list_get_body() on top of
vsp1_dl_body_get(); which are a bit too similar.

Perhaps we should remove body0, or at least call vsp1_dl_list_get_body()
vsp1_dl_list_get_body0()...
---
 drivers/media/platform/vsp1/vsp1_bru.c| 22 ++--
 drivers/media/platform/vsp1/vsp1_clu.c| 22 ++--
 drivers/media/platform/vsp1/vsp1_dl.c | 12 ++-
 drivers/media/platform/vsp1/vsp1_dl.h |  4 +-
 drivers/media/platform/vsp1/vsp1_drm.c| 14 +---
 drivers/media/platform/vsp1/vsp1_entity.c | 16 -
 drivers/media/platform/vsp1/vsp1_entity.h | 12 ---
 drivers/media/platform/vsp1/vsp1_hgo.c| 16 -
 drivers/media/platform/vsp1/vsp1_hgt.c| 18 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   | 10 +++---
 drivers/media/platform/vsp1/vsp1_lif.c| 13 +++
 drivers/media/platform/vsp1/vsp1_lut.c| 21 ++--
 drivers/media/platform/vsp1/vsp1_pipe.c   |  4 +-
 drivers/media/platform/vsp1/vsp1_pipe.h   |  3 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 43 +++-
 drivers/media/platform/vsp1/vsp1_sru.c| 14 
 drivers/media/platform/vsp1/vsp1_uds.c| 24 +++--
 drivers/media/platform/vsp1/vsp1_uds.h|  2 +-
 drivers/media/platform/vsp1/vsp1_video.c  | 11 --
 drivers/media/platform/vsp1/vsp1_wpf.c| 42 ---
 20 files changed, 169 insertions(+), 154 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index b9ff96f76b3e..60d449d7b135 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -30,10 +30,10 @@
  * Device Access
  */
 
-static inline void vsp1_bru_write(struct vsp1_bru *bru, struct vsp1_dl_list 
*dl,
- u32 reg, u32 data)
+static inline void vsp1_bru_write(struct vsp1_bru *bru,
+ struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
-   vsp1_dl_list_write(dl, bru->base + reg, data);
+   vsp1_dl_body_write(dlb, bru->base + reg, data);
 }
 
 /* 
-
@@ -287,7 +287,7 @@ static const struct v4l2_subdev_ops bru_ops = {
 
 static void bru_prepare(struct vsp1_entity *entity,
struct vsp1_pipeline *pipe,
-   struct vsp1_dl_list *dl)
+   struct vsp1_dl_body *dlb)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
@@ -309,7 +309,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * format at the pipeline output is premultiplied.
 */
flags = pipe->output ? pipe->output->format.flags : 0;
-   vsp1_bru_write(bru, dl, VI6_BRU_INCTRL,
+   vsp1_bru_write(bru, dlb, VI6_BRU_INCTRL,
   flags & V4L2_PIX_FMT_FLAG_PREMUL_ALPHA ?
   0 : VI6_BRU_INCTRL_NRM);
 
@@ -317,12 +317,12 @@ static void bru_prepare(struct vsp1_entity *entity,
 * Set the background position to cover the whole output image and
 * configure its color.
 */
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_SIZE,
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_SIZE,
   (format->width << VI6_BRU_VIRRPF_SIZE_HSIZE_SHIFT) |
   (format->height << VI6_BRU_VIRRPF_SIZE_VSIZE_SHIFT));
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_LOC, 0);
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_LOC, 0);
 
-   vsp1_bru_write(bru, dl, VI6_BRU_VIRRPF_COL, bru->bgcolor |
+   vsp1_bru_write(bru, dlb, VI6_BRU_VIRRPF_COL, bru->bgcolor |
   (0xff << VI6_BRU_VIRRPF_COL_A_SHIFT));
 
/*
@@ -332,7 +332,7 @@ static void bru_prepare(struct vsp1_entity *entity,
 * unit.
 */
if (entity->type == VSP1_ENTITY_BRU)
-   vsp1_bru_write(bru, dl, VI6_BRU_ROP,
+   vsp1_bru_write(bru, dlb, VI6_BRU_ROP,
   VI6_BRU_ROP_DSTSEL_BRUIN(1) |
   VI6_BRU_ROP_CROP(VI6_ROP_NOP) |
   VI6_BRU_ROP_AROP(VI6_ROP_NOP));
@@ -374,7 +374,7 @@ static void bru_prepare(struct vsp1_entity *entity,
if (!(entity->type == VSP1_ENTITY_BRU && i == 1))
ctrl |= VI6_BRU_CTRL_SRCSEL_BRUIN(i);
 
-   vsp1_bru_write(bru, dl, VI6_BRU_CTRL(i), ctrl);
+   vsp1_bru_write(bru, dlb, VI6_BRU_CTRL(i), ctrl);
 
/*

[PATCH v3 5/9] v4l: vsp1: Use reference counting for bodies

[Kieran Bingham]

Extend the display list body with a reference count, allowing bodies to
be kept as long as a reference is maintained. This provides the ability
to keep a cached copy of bodies which will not change, so that they can
be re-applied to multiple display lists.

Signed-off-by: Kieran Bingham 

---
This could be squashed into the body update code, but it's not a
straightforward squash as the refcounts will affect both:
  v4l: vsp1: Provide a body pool
and
  v4l: vsp1: Convert display lists to use new body pool
therefore, I have kept this separate to prevent breaking bisectability
of the vsp-tests.

v3:
 - 's/fragment/body/'
---
 drivers/media/platform/vsp1/vsp1_clu.c |  7 ++-
 drivers/media/platform/vsp1/vsp1_dl.c  | 15 ++-
 drivers/media/platform/vsp1/vsp1_lut.c |  7 ++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 2018144470c5..b2a39a6ef7e4 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -257,8 +257,13 @@ static void clu_configure(struct vsp1_entity *entity,
clu->clu = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 671aef30d090..0ee12d3338dd 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -58,6 +59,8 @@ struct vsp1_dl_body {
struct list_head list;
struct list_head free;
 
+   refcount_t refcnt;
+
struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
@@ -252,6 +255,7 @@ struct vsp1_dl_body *vsp1_dl_body_get(struct 
vsp1_dl_body_pool *pool)
if (!list_empty(>free)) {
dlb = list_first_entry(>free, struct vsp1_dl_body, free);
list_del(>free);
+   refcount_set(>refcnt, 1);
}
 
spin_unlock_irqrestore(>lock, flags);
@@ -272,6 +276,9 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb)
if (!dlb)
return;
 
+   if (!refcount_dec_and_test(>refcnt))
+   return;
+
dlb->num_entries = 0;
 
spin_lock_irqsave(>pool->lock, flags);
@@ -458,7 +465,11 @@ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, 
u32 data)
  * in the order in which bodies are added.
  *
  * Adding a body to a display list passes ownership of the body to the list. 
The
- * caller must not touch the body after this call.
+ * caller must not modify the body after this call, but can retain a reference
+ * to it for future use if necessary, to add to subsequent lists.
+ *
+ * The reference count of the body is incremented by this attachment, and thus
+ * the caller should release it's reference if does not want to cache the body.
  *
  * Additional bodies are only usable for display lists in header mode.
  * Attempting to add a body to a header-less display list will return an error.
@@ -470,6 +481,8 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl,
if (dl->dlm->mode != VSP1_DL_MODE_HEADER)
return -EINVAL;
 
+   refcount_inc(>refcnt);
+
list_add_tail(>list, >bodies);
return 0;
 }
diff --git a/drivers/media/platform/vsp1/vsp1_lut.c 
b/drivers/media/platform/vsp1/vsp1_lut.c
index 262cb72139d6..77cf7137a0f2 100644
--- a/drivers/media/platform/vsp1/vsp1_lut.c
+++ b/drivers/media/platform/vsp1/vsp1_lut.c
@@ -213,8 +213,13 @@ static void lut_configure(struct vsp1_entity *entity,
lut->lut = NULL;
spin_unlock_irqrestore(>lock, flags);
 
-   if (dlb)
+   if (dlb) {
vsp1_dl_list_add_body(dl, dlb);
+
+   /* release our local reference */
+   vsp1_dl_body_put(dlb);
+   }
+
break;
}
 }
-- 
git-series 0.9.1

[PATCH v3 6/9] v4l: vsp1: Refactor display list configure operations

[Kieran Bingham]

The entities provide a single .configure operation which configures the
object into the target display list, based on the vsp1_entity_params
selection.

This restricts us to a single function prototype for both static
configuration (the pre-stream INIT stage) and the dynamic runtime stages
for both each frame - and each partition therein.

Split the configure function into two parts, '.prepare()' and
'.configure()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and
VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the
.configure(). The configuration for individual partitions is handled by
passing the partition number to the configure call, and processing any
runtime stage actions on the first partition only.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_bru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_clu.c|  42 +--
 drivers/media/platform/vsp1/vsp1_drm.c|  11 +-
 drivers/media/platform/vsp1/vsp1_entity.c |  15 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  27 +--
 drivers/media/platform/vsp1/vsp1_hgo.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hgt.c|  12 +-
 drivers/media/platform/vsp1/vsp1_hsit.c   |  12 +-
 drivers/media/platform/vsp1/vsp1_lif.c|  12 +-
 drivers/media/platform/vsp1/vsp1_lut.c|  24 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 162 ++---
 drivers/media/platform/vsp1/vsp1_sru.c|  12 +-
 drivers/media/platform/vsp1/vsp1_uds.c|  55 ++--
 drivers/media/platform/vsp1/vsp1_video.c  |  24 +--
 drivers/media/platform/vsp1/vsp1_wpf.c| 297 ---
 15 files changed, 358 insertions(+), 371 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_bru.c 
b/drivers/media/platform/vsp1/vsp1_bru.c
index e8fd2ae3b3eb..b9ff96f76b3e 100644
--- a/drivers/media/platform/vsp1/vsp1_bru.c
+++ b/drivers/media/platform/vsp1/vsp1_bru.c
@@ -285,19 +285,15 @@ static const struct v4l2_subdev_ops bru_ops = {
  * VSP1 Entity Operations
  */
 
-static void bru_configure(struct vsp1_entity *entity,
- struct vsp1_pipeline *pipe,
- struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+static void bru_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
 {
struct vsp1_bru *bru = to_bru(>subdev);
struct v4l2_mbus_framefmt *format;
unsigned int flags;
unsigned int i;
 
-   if (params != VSP1_ENTITY_PARAMS_INIT)
-   return;
-
format = vsp1_entity_get_pad_format(>entity, bru->entity.config,
bru->entity.source_pad);
 
@@ -404,7 +400,7 @@ static void bru_configure(struct vsp1_entity *entity,
 }
 
 static const struct vsp1_entity_operations bru_entity_ops = {
-   .configure = bru_configure,
+   .prepare = bru_prepare,
 };
 
 /* 
-
diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index b2a39a6ef7e4..2e4af93a053f 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -213,37 +213,36 @@ static const struct v4l2_subdev_ops clu_ops = {
 /* 
-
  * VSP1 Entity Operations
  */
+static void clu_prepare(struct vsp1_entity *entity,
+   struct vsp1_pipeline *pipe,
+   struct vsp1_dl_list *dl)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   /*
+* The format can't be changed during streaming. Cache it internally
+* for future runtime configuration calls.
+*/
+   struct v4l2_mbus_framefmt *format;
+
+   format = vsp1_entity_get_pad_format(>entity,
+   clu->entity.config,
+   CLU_PAD_SINK);
+   clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32;
+}
 
 static void clu_configure(struct vsp1_entity *entity,
  struct vsp1_pipeline *pipe,
  struct vsp1_dl_list *dl,
- enum vsp1_entity_params params)
+ unsigned int partition)
 {
struct vsp1_clu *clu = to_clu(>subdev);
struct vsp1_dl_body *dlb;
unsigned long flags;
u32 ctrl = VI6_CLU_CTRL_AAI | VI6_CLU_CTRL_MVS | VI6_CLU_CTRL_EN;
 
-   switch (params) {
-   case VSP1_ENTITY_PARAMS_INIT: {
-   /*
-* The format can't be changed during streaming, only verify it
-* at setup time and store the information internally for future
-* runtime configuration calls.
-*/
-   struct v4l2_mbus_framefmt *format;
-
-   format = vsp1_entity_get_pad_format(>entity,
-  

[PATCH v3 3/9] v4l: vsp1: Provide a body pool

[Kieran Bingham]

Each display list allocates a body to store register values in a dma
accessible buffer from a dma_alloc_wc() allocation. Each of these
results in an entry in the TLB, and a large number of display list
allocations adds pressure to this resource.

Reduce TLB pressure on the IPMMUs by allocating multiple display list
bodies in a single allocation, and providing these to the display list
through a 'body pool'. A pool can be allocated by the display list
manager or entities which require their own body allocations.

Signed-off-by: Kieran Bingham 

---
v3:
 - s/fragment/body/, s/fragments/bodies/
 - qty -> num_bodies
 - indentation fix
 - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/'
 - Add kerneldoc to non-static functions

v2:
 - assign dlb->dma correctly
---
 drivers/media/platform/vsp1/vsp1_dl.c | 157 +++-
 drivers/media/platform/vsp1/vsp1_dl.h |   8 +-
 2 files changed, 165 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index a45d35aa676e..e6f3e68367ff 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -45,6 +45,8 @@ struct vsp1_dl_entry {
 /**
  * struct vsp1_dl_body - Display list body
  * @list: entry in the display list list of bodies
+ * @free: entry in the pool free body list
+ * @pool: pool to which this body belongs
  * @vsp1: the VSP1 device
  * @entries: array of entries
  * @dma: DMA address of the entries
@@ -54,6 +56,9 @@ struct vsp1_dl_entry {
  */
 struct vsp1_dl_body {
struct list_head list;
+   struct list_head free;
+
+   struct vsp1_dl_body_pool *pool;
struct vsp1_device *vsp1;
 
struct vsp1_dl_entry *entries;
@@ -65,6 +70,30 @@ struct vsp1_dl_body {
 };
 
 /**
+ * struct vsp1_dl_body_pool - display list body pool
+ * @dma: DMA address of the entries
+ * @size: size of the full DMA memory pool in bytes
+ * @mem: CPU memory pointer for the pool
+ * @bodies: Array of DLB structures for the pool
+ * @free: List of free DLB entries
+ * @lock: Protects the pool and free list
+ * @vsp1: the VSP1 device
+ */
+struct vsp1_dl_body_pool {
+   /* DMA allocation */
+   dma_addr_t dma;
+   size_t size;
+   void *mem;
+
+   /* Body management */
+   struct vsp1_dl_body *bodies;
+   struct list_head free;
+   spinlock_t lock;
+
+   struct vsp1_device *vsp1;
+};
+
+/**
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
@@ -104,6 +133,7 @@ enum vsp1_dl_mode {
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
+ * @pool: body pool for the display list bodies
  * @gc_work: bodies garbage collector work struct
  * @gc_bodies: array of display list bodies waiting to be freed
  */
@@ -119,6 +149,8 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *queued;
struct vsp1_dl_list *pending;
 
+   struct vsp1_dl_body_pool *pool;
+
struct work_struct gc_work;
struct list_head gc_bodies;
 };
@@ -127,6 +159,131 @@ struct vsp1_dl_manager {
  * Display List Body Management
  */
 
+/**
+ * vsp1_dl_body_pool_create - Create a pool of bodies from a single allocation
+ * @vsp1: The VSP1 device
+ * @num_bodies: The quantity of bodies to allocate
+ * @num_entries: The maximum number of entries that the body can contain
+ * @extra_size: Extra allocation provided for the bodies
+ *
+ * Allocate a pool of display list bodies each with enough memory to contain 
the
+ * requested number of entries.
+ *
+ * Return a pointer to a pool on success or NULL if memory can't be allocated.
+ */
+struct vsp1_dl_body_pool *
+vsp1_dl_body_pool_create(struct vsp1_device *vsp1, unsigned int num_bodies,
+unsigned int num_entries, size_t extra_size)
+{
+   struct vsp1_dl_body_pool *pool;
+   size_t dlb_size;
+   unsigned int i;
+
+   pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+   if (!pool)
+   return NULL;
+
+   pool->vsp1 = vsp1;
+
+   dlb_size = num_entries * sizeof(struct vsp1_dl_entry) + extra_size;
+   pool->size = dlb_size * num_bodies;
+
+   pool->bodies = kcalloc(num_bodies, sizeof(*pool->bodies), GFP_KERNEL);
+   if (!pool->bodies) {
+   kfree(pool);
+   return NULL;
+   }
+
+   pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, >dma,
+GFP_KERNEL);
+   if (!pool->mem) {
+   kfree(pool->bodies);
+   kfree(pool);
+   return NULL;
+   }
+
+   spin_lock_init(>lock);
+   INIT_LIST_HEAD(>free);
+
+   for (i = 0; i < num_bodies; ++i) {
+   struct vsp1_dl_body *dlb = >bodies[i];
+
+   dlb->pool = pool;
+   

[PATCH v3 9/9] v4l: vsp1: Reduce display list body size

[Kieran Bingham]

The display list originally allocated a body of 256 entries to store all
of the register lists required for each frame.

This has now been separated into fragments for constant stream setup, and
runtime updates.

Empirical testing shows that the body0 now uses a maximum of 41
registers for each frame, for both DRM and Video API pipelines thus a
rounded 64 entries provides a suitable allocation.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_dl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index bca9bd45ac7f..5bb1e9103f84 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -21,7 +21,7 @@
 #include "vsp1.h"
 #include "vsp1_dl.h"
 
-#define VSP1_DL_NUM_ENTRIES256
+#define VSP1_DL_NUM_ENTRIES64
 
 #define VSP1_DLH_INT_ENABLE(1 << 1)
 #define VSP1_DLH_AUTO_START(1 << 0)
-- 
git-series 0.9.1

[PATCH v3 8/9] v4l: vsp1: Move video configuration to a cached dlb

[Kieran Bingham]

We are now able to configure a pipeline directly into a local display
list body. Take advantage of this fact, and create a cacheable body to
store the configuration of the pipeline in the video object.

vsp1_video_pipeline_run() is now the last user of the pipe->dl object.
Convert this function to use the cached video->config body and obtain a
local display list reference.

Attach the video->config body to the display list when needed before
committing to hardware.

The pipe object is marked as un-configured when entering a suspend. This
ensures that upon resume, where the hardware is reset - our cached
configuration will be re-attached to the next committed DL.

Signed-off-by: Kieran Bingham 
---

v3:
 - 's/fragment/body/', 's/fragments/bodies/'
 - video dlb cache allocation increased from 2 to 3 dlbs

Our video DL usage now looks like the below output:

dl->body0 contains our disposable runtime configuration. Max 41.
dl_child->body0 is our partition specific configuration. Max 12.
dl->bodies shows our constant configuration and LUTs.

  These two are LUT/CLU:
 * dl->bodies[x]->num_entries 256 / max 256
 * dl->bodies[x]->num_entries 4914 / max 4914

Which shows that our 'constant' configuration cache is currently
utilised to a maximum of 64 entries.

trace-cmd report | \
grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq;

  dl->body0->num_entries 13 / max 128
  dl->body0->num_entries 14 / max 128
  dl->body0->num_entries 16 / max 128
  dl->body0->num_entries 20 / max 128
  dl->body0->num_entries 27 / max 128
  dl->body0->num_entries 34 / max 128
  dl->body0->num_entries 41 / max 128
  dl_child->body0->num_entries 10 / max 128
  dl_child->body0->num_entries 12 / max 128
  dl->bodies[x]->num_entries 15 / max 128
  dl->bodies[x]->num_entries 16 / max 128
  dl->bodies[x]->num_entries 17 / max 128
  dl->bodies[x]->num_entries 18 / max 128
  dl->bodies[x]->num_entries 20 / max 128
  dl->bodies[x]->num_entries 21 / max 128
  dl->bodies[x]->num_entries 256 / max 256
  dl->bodies[x]->num_entries 31 / max 128
  dl->bodies[x]->num_entries 32 / max 128
  dl->bodies[x]->num_entries 39 / max 128
  dl->bodies[x]->num_entries 40 / max 128
  dl->bodies[x]->num_entries 47 / max 128
  dl->bodies[x]->num_entries 48 / max 128
  dl->bodies[x]->num_entries 4914 / max 4914
  dl->bodies[x]->num_entries 55 / max 128
  dl->bodies[x]->num_entries 56 / max 128
  dl->bodies[x]->num_entries 63 / max 128
  dl->bodies[x]->num_entries 64 / max 128
---
 drivers/media/platform/vsp1/vsp1_pipe.c  |  4 +-
 drivers/media/platform/vsp1/vsp1_pipe.h  |  4 +-
 drivers/media/platform/vsp1/vsp1_video.c | 67 -
 drivers/media/platform/vsp1/vsp1_video.h |  2 +-
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c 
b/drivers/media/platform/vsp1/vsp1_pipe.c
index 5012643583b6..7d1f7ba43060 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/vsp1/vsp1_pipe.c
@@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe)
vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index),
   VI6_CMD_STRCMD);
pipe->state = VSP1_PIPELINE_RUNNING;
+   pipe->configured = true;
}
 
pipe->buffers_ready = 0;
@@ -430,6 +431,9 @@ void vsp1_pipelines_suspend(struct vsp1_device *vsp1)
spin_lock_irqsave(>irqlock, flags);
if (pipe->state == VSP1_PIPELINE_RUNNING)
pipe->state = VSP1_PIPELINE_STOPPING;
+
+   /* After a suspend, the hardware will be reset */
+   pipe->configured = false;
spin_unlock_irqrestore(>irqlock, flags);
}
 
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h 
b/drivers/media/platform/vsp1/vsp1_pipe.h
index 90d29492b9b9..e7ad6211b4d0 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -90,6 +90,7 @@ struct vsp1_partition {
  * @irqlock: protects the pipeline state
  * @state: current state
  * @wq: wait queue to wait for state change completion
+ * @configured: flag determining if the hardware has run since reset
  * @frame_end: frame end interrupt handler
  * @lock: protects the pipeline use count and stream count
  * @kref: pipeline reference count
@@ -117,6 +118,7 @@ struct vsp1_pipeline {
spinlock_t irqlock;
enum vsp1_pipeline_state state;
wait_queue_head_t wq;
+   bool configured;
 
void (*frame_end)(struct vsp1_pipeline *pipe, bool completed);
 
@@ -143,8 +145,6 @@ struct vsp1_pipeline {
 */
struct list_head entities;
 
-   struct vsp1_dl_list *dl;
-
unsigned int partitions;
struct vsp1_partition *partition;
struct vsp1_partition *part_table;
diff --git a/drivers/media/platform/vsp1/vsp1_video.c 
b/drivers/media/platform/vsp1/vsp1_video.c
index 9366b41c1b43..54fef64c3457 

[PATCH v3 2/9] v4l: vsp1: Protect bodies against overflow

[Kieran Bingham]

The body write function relies on the code never asking it to write more
than the entries available in the list.

Currently with each list body containing 256 entries, this is fine, but
we can reduce this number greatly saving memory. In preparation of this
add a level of protection to catch any buffer overflows.

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 

---

v3:
 - adapt for new 'body' terminology
 - simplify WARN_ON macro usage
---
 drivers/media/platform/vsp1/vsp1_dl.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 643f7ea3af24..a45d35aa676e 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -50,6 +50,7 @@ struct vsp1_dl_entry {
  * @dma: DMA address of the entries
  * @size: size of the DMA memory in bytes
  * @num_entries: number of stored entries
+ * @max_entries: number of entries available
  */
 struct vsp1_dl_body {
struct list_head list;
@@ -60,6 +61,7 @@ struct vsp1_dl_body {
size_t size;
 
unsigned int num_entries;
+   unsigned int max_entries;
 };
 
 /**
@@ -138,6 +140,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
 
dlb->vsp1 = vsp1;
dlb->size = size;
+   dlb->max_entries = num_entries;
 
dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, >dma,
GFP_KERNEL);
@@ -219,6 +222,10 @@ void vsp1_dl_body_free(struct vsp1_dl_body *dlb)
  */
 void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
 {
+   if (WARN_ONCE(dlb->num_entries >= dlb->max_entries,
+ "DLB size exceeded (max %u)", dlb->max_entries))
+   return;
+
dlb->entries[dlb->num_entries].addr = reg;
dlb->entries[dlb->num_entries].data = data;
dlb->num_entries++;
-- 
git-series 0.9.1

[PATCH v3 1/9] v4l: vsp1: Reword uses of 'fragment' as 'body'

[Kieran Bingham]

Throughout the codebase, the term 'fragment' is used to represent a
display list body. This term duplicates the 'body' which is already in
use.

The datasheet references these objects as a body, therefore replace all
mentions of a fragment with a body, along with the corresponding
pluralised terms.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_clu.c |  10 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 107 --
 drivers/media/platform/vsp1/vsp1_dl.h  |  14 +--
 drivers/media/platform/vsp1/vsp1_lut.c |   8 +-
 4 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index f2fb26e5ab4e..9621afa3658c 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -47,19 +47,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_fragment_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
if (!dlb)
return -ENOMEM;
 
-   vsp1_dl_fragment_write(dlb, VI6_CLU_ADDR, 0);
+   vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
for (i = 0; i < 17 * 17 * 17; ++i)
-   vsp1_dl_fragment_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
+   vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_fragment_free(dlb);
+   vsp1_dl_body_free(dlb);
return 0;
 }
 
@@ -256,7 +256,7 @@ static void clu_configure(struct vsp1_entity *entity,
spin_unlock_irqrestore(>lock, flags);
 
if (dlb)
-   vsp1_dl_list_add_fragment(dl, dlb);
+   vsp1_dl_list_add_body(dl, dlb);
break;
}
 }
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 8b5cbb6b7a70..643f7ea3af24 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -69,7 +69,7 @@ struct vsp1_dl_body {
  * @header: display list header, NULL for headerless lists
  * @dma: DMA address for the header
  * @body0: first display list body
- * @fragments: list of extra display list bodies
+ * @bodies: list of extra display list bodies
  * @chain: entry in the display list partition chain
  */
 struct vsp1_dl_list {
@@ -80,7 +80,7 @@ struct vsp1_dl_list {
dma_addr_t dma;
 
struct vsp1_dl_body body0;
-   struct list_head fragments;
+   struct list_head bodies;
 
bool has_chain;
struct list_head chain;
@@ -97,13 +97,13 @@ enum vsp1_dl_mode {
  * @mode: display list operation mode (header or headerless)
  * @singleshot: execute the display list in single-shot mode
  * @vsp1: the VSP1 device
- * @lock: protects the free, active, queued, pending and gc_fragments lists
+ * @lock: protects the free, active, queued, pending and gc_bodies lists
  * @free: array of all free display lists
  * @active: list currently being processed (loaded) by hardware
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
- * @gc_work: fragments garbage collector work struct
- * @gc_fragments: array of display list fragments waiting to be freed
+ * @gc_work: bodies garbage collector work struct
+ * @gc_bodies: array of display list bodies waiting to be freed
  */
 struct vsp1_dl_manager {
unsigned int index;
@@ -118,7 +118,7 @@ struct vsp1_dl_manager {
struct vsp1_dl_list *pending;
 
struct work_struct gc_work;
-   struct list_head gc_fragments;
+   struct list_head gc_bodies;
 };
 
 /* 
-
@@ -156,17 +156,16 @@ static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 }
 
 /**
- * vsp1_dl_fragment_alloc - Allocate a display list fragment
+ * vsp1_dl_body_alloc - Allocate a display list body
  * @vsp1: The VSP1 device
- * @num_entries: The maximum number of entries that the fragment can contain
+ * @num_entries: The maximum number of entries that the body can contain
  *
- * Allocate a display list fragment with enough memory to contain the requested
+ * Allocate a display list body with enough memory to contain the requested
  * number of entries.
  *
- * Return a pointer to a fragment on success or NULL if memory can't be
- * allocated.
+ * Return a pointer to a body on success or NULL if memory can't be allocated.
  */
-struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1,
+struct vsp1_dl_body *vsp1_dl_body_alloc(struct vsp1_device *vsp1,
unsigned int num_entries)
 {
struct vsp1_dl_body *dlb;
@@ -186,20 +185,20 @@ 

[PATCH v3 4/9] v4l: vsp1: Convert display lists to use new body pool

[Kieran Bingham]

Adapt the dl->body0 object to use an object from the body pool. This
greatly reduces the pressure on the TLB for IPMMU use cases, as all of
the lists use a single allocation for the main body.

The CLU and LUT objects pre-allocate a pool containing three bodies,
allowing a userspace update before the hardware has committed a previous
set of tables.

Bodies are no longer 'freed' in interrupt context, but instead released
back to their respective pools. This allows us to remove the garbage
collector in the DLM.

Signed-off-by: Kieran Bingham 

---
v3:
 - 's/fragment/body', 's/fragments/bodies/'
 - CLU/LUT now allocate 3 bodies
 - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put

v2:
 - Use dl->body0->max_entries to determine header offset, instead of the
   global constant VSP1_DL_NUM_ENTRIES which is incorrect.
 - squash updates for LUT, CLU, and fragment cleanup into single patch.
   (Not fully bisectable when separated)
---
 drivers/media/platform/vsp1/vsp1_clu.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_clu.h |   1 +-
 drivers/media/platform/vsp1/vsp1_dl.c  | 223 ++
 drivers/media/platform/vsp1/vsp1_dl.h  |   3 +-
 drivers/media/platform/vsp1/vsp1_lut.c |  27 ++-
 drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
 6 files changed, 101 insertions(+), 181 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_clu.c 
b/drivers/media/platform/vsp1/vsp1_clu.c
index 9621afa3658c..2018144470c5 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.c
+++ b/drivers/media/platform/vsp1/vsp1_clu.c
@@ -23,6 +23,8 @@
 #define CLU_MIN_SIZE   4U
 #define CLU_MAX_SIZE   8190U
 
+#define CLU_SIZE   (17 * 17 * 17)
+
 /* 
-
  * Device Access
  */
@@ -47,19 +49,19 @@ static int clu_set_table(struct vsp1_clu *clu, struct 
v4l2_ctrl *ctrl)
struct vsp1_dl_body *dlb;
unsigned int i;
 
-   dlb = vsp1_dl_body_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17);
+   dlb = vsp1_dl_body_get(clu->pool);
if (!dlb)
return -ENOMEM;
 
vsp1_dl_body_write(dlb, VI6_CLU_ADDR, 0);
-   for (i = 0; i < 17 * 17 * 17; ++i)
+   for (i = 0; i < CLU_SIZE; ++i)
vsp1_dl_body_write(dlb, VI6_CLU_DATA, ctrl->p_new.p_u32[i]);
 
spin_lock_irq(>lock);
swap(clu->clu, dlb);
spin_unlock_irq(>lock);
 
-   vsp1_dl_body_free(dlb);
+   vsp1_dl_body_put(dlb);
return 0;
 }
 
@@ -261,8 +263,16 @@ static void clu_configure(struct vsp1_entity *entity,
}
 }
 
+static void clu_destroy(struct vsp1_entity *entity)
+{
+   struct vsp1_clu *clu = to_clu(>subdev);
+
+   vsp1_dl_body_pool_destroy(clu->pool);
+}
+
 static const struct vsp1_entity_operations clu_entity_ops = {
.configure = clu_configure,
+   .destroy = clu_destroy,
 };
 
 /* 
-
@@ -288,6 +298,17 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1)
if (ret < 0)
return ERR_PTR(ret);
 
+   /*
+* Pre-allocate a body pool, with 3 bodies allowing a userspace update
+* before the hardware has committed a previous set of tables, handling
+* both the queued and pending dl entries. One extra entry is added to
+* the CLU_SIZE to allow for the VI6_CLU_ADDR header.
+*/
+   clu->pool = vsp1_dl_body_pool_create(clu->entity.vsp1, 3, CLU_SIZE + 1,
+0);
+   if (!clu->pool)
+   return ERR_PTR(-ENOMEM);
+
/* Initialize the control handler. */
v4l2_ctrl_handler_init(>ctrls, 2);
v4l2_ctrl_new_custom(>ctrls, _table_control, NULL);
diff --git a/drivers/media/platform/vsp1/vsp1_clu.h 
b/drivers/media/platform/vsp1/vsp1_clu.h
index 036e0a2f1a42..fa3fe856725b 100644
--- a/drivers/media/platform/vsp1/vsp1_clu.h
+++ b/drivers/media/platform/vsp1/vsp1_clu.h
@@ -36,6 +36,7 @@ struct vsp1_clu {
spinlock_t lock;
unsigned int mode;
struct vsp1_dl_body *clu;
+   struct vsp1_dl_body_pool *pool;
 };
 
 static inline struct vsp1_clu *to_clu(struct v4l2_subdev *subdev)
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index e6f3e68367ff..671aef30d090 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -110,7 +110,7 @@ struct vsp1_dl_list {
struct vsp1_dl_header *header;
dma_addr_t dma;
 
-   struct vsp1_dl_body body0;
+   struct vsp1_dl_body *body0;
struct list_head bodies;
 
bool has_chain;
@@ -134,8 +134,6 @@ enum vsp1_dl_mode {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
- * 

[PATCH v3 0/9] vsp1: TLB optimisation and DL caching

[Kieran Bingham]

Each display list currently allocates an area of DMA memory to store register
settings for the VSP1 to process. Each of these allocations adds pressure to
the IPMMU TLB entries.

We can reduce the pressure by pre-allocating larger areas and dividing the area
across multiple bodies represented as a pool.

With this reconfiguration of bodies, we can adapt the configuration code to
separate out constant hardware configuration and cache it for re-use.

This posting is only really a status update following the previous review after
quite a bit of work has been done. However this series is not yet complete, and
I do not expect a full review - or integration of this series in its current
form.

In particular:
  Patch 1 : Reword uses of 'fragment' as 'body'
  - Is new and can be reviewed if desired.

Otherwise, most other issues have been addressed, and the series is rebased to
utilise the term 'bodies' instead of 'fragments'. A few 'larger' topics came up
in review, and will be considered in a follow up series. (v4+)

  Patch 3 : Provide a body pool
  - Allocates more memory than is required for 'extra_size'

  Patch 5 : Use reference counting for bodies
  - Minor comment to be fixed up. (I should have done this already)

  Patch 7 : Adapt entities to configure into a body
  - To be reworked, quite substantially.

  Patch 8 : Move video configuration to a cached dlb
  - Previous review comments still to be addressed

The patches provided in this series can be found at:
  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git  
tags/vsp1/tlb-optimise/v3

Kieran Bingham (9):
  v4l: vsp1: Reword uses of 'fragment' as 'body'
  v4l: vsp1: Protect bodies against overflow
  v4l: vsp1: Provide a body pool
  v4l: vsp1: Convert display lists to use new body pool
  v4l: vsp1: Use reference counting for bodies
  v4l: vsp1: Refactor display list configure operations
  v4l: vsp1: Adapt entities to configure into a body
  v4l: vsp1: Move video configuration to a cached dlb
  v4l: vsp1: Reduce display list body size

 drivers/media/platform/vsp1/vsp1_bru.c|  32 +--
 drivers/media/platform/vsp1/vsp1_clu.c|  94 +++---
 drivers/media/platform/vsp1/vsp1_clu.h|   1 +-
 drivers/media/platform/vsp1/vsp1_dl.c | 387 +--
 drivers/media/platform/vsp1/vsp1_dl.h |  21 +-
 drivers/media/platform/vsp1/vsp1_drm.c|  21 +-
 drivers/media/platform/vsp1/vsp1_entity.c |  23 +-
 drivers/media/platform/vsp1/vsp1_entity.h |  31 +--
 drivers/media/platform/vsp1/vsp1_hgo.c|  26 +--
 drivers/media/platform/vsp1/vsp1_hgt.c|  28 +--
 drivers/media/platform/vsp1/vsp1_hsit.c   |  20 +-
 drivers/media/platform/vsp1/vsp1_lif.c|  23 +-
 drivers/media/platform/vsp1/vsp1_lut.c|  71 ++--
 drivers/media/platform/vsp1/vsp1_lut.h|   1 +-
 drivers/media/platform/vsp1/vsp1_pipe.c   |   8 +-
 drivers/media/platform/vsp1/vsp1_pipe.h   |   7 +-
 drivers/media/platform/vsp1/vsp1_rpf.c| 179 +--
 drivers/media/platform/vsp1/vsp1_sru.c|  24 +-
 drivers/media/platform/vsp1/vsp1_uds.c|  73 ++--
 drivers/media/platform/vsp1/vsp1_uds.h|   2 +-
 drivers/media/platform/vsp1/vsp1_video.c  |  82 ++---
 drivers/media/platform/vsp1/vsp1_video.h  |   2 +-
 drivers/media/platform/vsp1/vsp1_wpf.c| 325 +--
 23 files changed, 811 insertions(+), 670 deletions(-)

base-commit: f44bd631453bf7dcbe57f79b924db3a6dd038bff
-- 
git-series 0.9.1

Re: [PATCHv3 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[Ville Syrjälä]

On Wed, Sep 13, 2017 at 10:51:02AM +0200, Hans Verkuil wrote:
> On 09/12/2017 07:39 PM, Ville Syrjälä wrote:
> > On Mon, Sep 11, 2017 at 01:25:45PM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> This adds support for the DisplayPort CEC-Tunneling-over-AUX
> >> feature that is part of the DisplayPort 1.3 standard.
> >>
> >> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
> >> chip that has this capability actually hook up the CEC pin, so
> >> even though a CEC device is created, it may not actually work.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> Tested-by: Carlos Santa 
> >> ---
> >>  drivers/gpu/drm/Kconfig  |  10 ++
> >>  drivers/gpu/drm/Makefile |   1 +
> >>  drivers/gpu/drm/drm_dp_cec.c | 301 
> >> +++
> >>  include/drm/drm_dp_helper.h  |  24 
> >>  4 files changed, 336 insertions(+)
> >>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
> >>
> >> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> >> index 83cb2a88c204..1f2708df5c4e 100644
> >> --- a/drivers/gpu/drm/Kconfig
> >> +++ b/drivers/gpu/drm/Kconfig
> >> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
> >>  default case is N. Details and instructions how to build your own
> >>  EDID data are given in Documentation/EDID/HOWTO.txt.
> >>  
> >> +config DRM_DP_CEC
> >> +  bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> >> +  select CEC_CORE
> >> +  help
> >> +Choose this option if you want to enable HDMI CEC support for
> >> +DisplayPort/USB-C to HDMI adapters.
> >> +
> >> +Note: not all adapters support this feature, and even for those
> >> +that do support this they often do not hook up the CEC pin.
> >> +
> >>  config DRM_TTM
> >>tristate
> >>depends on DRM && MMU
> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> >> index 24a066e1841c..c6552c62049e 100644
> >> --- a/drivers/gpu/drm/Makefile
> >> +++ b/drivers/gpu/drm/Makefile
> >> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += 
> >> drm_edid_load.o
> >>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> >>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> >>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> >> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
> >>  
> >>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> >>  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
> >> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> >> new file mode 100644
> >> index ..b831bb72c932
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/drm_dp_cec.c
> >> @@ -0,0 +1,301 @@
> >> +/*
> >> + * DisplayPort CEC-Tunneling-over-AUX support
> >> + *
> >> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights 
> >> reserved.
> >> + *
> >> + * This program is free software; you may 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.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> >> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> >> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> >> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> >> + * SOFTWARE.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +/*
> >> + * Unfortunately it turns out that we have a chicken-and-egg situation
> >> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
> >> + * have a converter chip that supports CEC-Tunneling-over-AUX (usually the
> >> + * Parade PS176), but they do not wire up the CEC pin, thus making CEC
> >> + * useless.
> >> + *
> >> + * Sadly there is no way for this driver to know this. What happens is
> >> + * that a /dev/cecX device is created that is isolated and unable to see
> >> + * any of the other CEC devices. Quite literally the CEC wire is cut
> >> + * (or in this case, never connected in the first place).
> >> + *
> >> + * I suspect that the reason so few adapters support this is that this
> >> + * tunneling protocol was never supported by any OS. So there was no
> >> + * easy way of testing it, and no incentive to correctly wire up the
> >> + * CEC pin.
> >> + *
> >> + * Hopefully by creating this driver it will be easier for vendors to
> >> + * finally fix their adapters and test the CEC functionality.
> >> + *
> >> + * I keep a list of known working adapters here:
> >> + *
> >> + * https://hverkuil.home.xs4all.nl/cec-status.txt
> >> + 

Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references

[Hans Verkuil]

On 09/13/17 12:07, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Sep 13, 2017 at 11:28:44AM +0200, Hans Verkuil wrote:
>> On 09/13/17 11:24, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> Thanks for the review!
>>>
>>> On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:
 On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> Add function v4l2_fwnode_reference_count() for counting external

  There is no function v4l2_fwnode_reference_count()?!
>>>
>>> It got removed during the revisions but the commit message was not changed
>>> accordingly, I do that now.
>>>

> references and v4l2_fwnode_reference_parse() for parsing them as async
> sub-devices.
>
> This can be done on e.g. flash or lens async sub-devices that are not part
> of but are associated with a sensor.
>
> struct v4l2_async_notifier.max_subdevs field is added to contain the
> maximum number of sub-devices in a notifier to reflect the memory
> allocated for the subdevs array.

 You forgot to remove this outdated paragraph.
>>>
>>> Oops. Removed it now.
>>>

>
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
> +++
>  1 file changed, 69 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 44ee35f6aad5..a32473f95be1 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -498,6 +498,75 @@ int 
> v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>  }
>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
>  
> +/*
> + * v4l2_fwnode_reference_parse - parse references for async sub-devices
> + * @dev: the device node the properties of which are parsed for 
> references
> + * @notifier: the async notifier where the async subdevs will be added
> + * @prop: the name of the property
> + *
> + * Return: 0 on success
> + *  -ENOENT if no entries were found
> + *  -ENOMEM if memory allocation failed
> + *  -EINVAL if property parsing failed
> + */
> +static int v4l2_fwnode_reference_parse(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + const char *prop)
> +{
> + struct fwnode_reference_args args;
> + unsigned int index;
> + int ret;
> +
> + for (index = 0;
> +  !(ret = fwnode_property_get_reference_args(
> +dev_fwnode(dev), prop, NULL, 0, index, ));
> +  index++)
> + fwnode_handle_put(args.fwnode);
> +
> + if (!index)
> + return -ENOENT;
> +
> + /*
> +  * To-do: handle -ENODATA when "device property: Align return
> +  * codes of acpi_fwnode_get_reference_with_args" is merged.
> +  */
> + if (ret != -ENOENT && ret != -ENODATA)

 So while that patch referenced in the To-do above is not merged yet,
 what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
 Or ENOENT now and ENODATA later? Or vice versa?

 I can't tell based on that information whether this code is correct or not.

 The comment needs to explain this a bit better.
>>>
>>> I'll add this:
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
>>> b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> index a32473f95be1..74fcc3ba9ebd 100644
>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(
>>> /*
>>>  * To-do: handle -ENODATA when "device property: Align return
>>>  * codes of acpi_fwnode_get_reference_with_args" is merged.
>>
>> So after this patch referred to in the To-do is applied it will only
>> return ENODATA?
>>
>> In that case, change 'handle' to 'handle only'.
> 
> That depends a bit in which form the patch will be eventually accepted. The
> underlying issue there is that different error codes are used to signal
> conditions for out-of-bounds access and missing entry. After aligning them
> the code here can be updated.

Ah. In that case I'd drop the 'To-do' sentence.

> 
>>
>>> +* Right now, both -ENODATA and -ENOENT signal the end of
>>> +* references where only a single error code should be used
>>> +* for the purpose.

And add something like: "This might change in the future, in which
case this code should be updated."

>>>  */
>>> if (ret != -ENOENT && ret != -ENODATA)
>>> return ret;
>>>
> 

Regards,

Hans

Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references

[Sakari Ailus]

Hi Hans,

On Wed, Sep 13, 2017 at 11:28:44AM +0200, Hans Verkuil wrote:
> On 09/13/17 11:24, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > Thanks for the review!
> > 
> > On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:
> >> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> >>> Add function v4l2_fwnode_reference_count() for counting external
> >>
> >>  There is no function v4l2_fwnode_reference_count()?!
> > 
> > It got removed during the revisions but the commit message was not changed
> > accordingly, I do that now.
> > 
> >>
> >>> references and v4l2_fwnode_reference_parse() for parsing them as async
> >>> sub-devices.
> >>>
> >>> This can be done on e.g. flash or lens async sub-devices that are not part
> >>> of but are associated with a sensor.
> >>>
> >>> struct v4l2_async_notifier.max_subdevs field is added to contain the
> >>> maximum number of sub-devices in a notifier to reflect the memory
> >>> allocated for the subdevs array.
> >>
> >> You forgot to remove this outdated paragraph.
> > 
> > Oops. Removed it now.
> > 
> >>
> >>>
> >>> Signed-off-by: Sakari Ailus 
> >>> ---
> >>>  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
> >>> +++
> >>>  1 file changed, 69 insertions(+)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> >>> b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> index 44ee35f6aad5..a32473f95be1 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> @@ -498,6 +498,75 @@ int 
> >>> v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
> >>>  
> >>> +/*
> >>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices
> >>> + * @dev: the device node the properties of which are parsed for 
> >>> references
> >>> + * @notifier: the async notifier where the async subdevs will be added
> >>> + * @prop: the name of the property
> >>> + *
> >>> + * Return: 0 on success
> >>> + *  -ENOENT if no entries were found
> >>> + *  -ENOMEM if memory allocation failed
> >>> + *  -EINVAL if property parsing failed
> >>> + */
> >>> +static int v4l2_fwnode_reference_parse(
> >>> + struct device *dev, struct v4l2_async_notifier *notifier,
> >>> + const char *prop)
> >>> +{
> >>> + struct fwnode_reference_args args;
> >>> + unsigned int index;
> >>> + int ret;
> >>> +
> >>> + for (index = 0;
> >>> +  !(ret = fwnode_property_get_reference_args(
> >>> +dev_fwnode(dev), prop, NULL, 0, index, ));
> >>> +  index++)
> >>> + fwnode_handle_put(args.fwnode);
> >>> +
> >>> + if (!index)
> >>> + return -ENOENT;
> >>> +
> >>> + /*
> >>> +  * To-do: handle -ENODATA when "device property: Align return
> >>> +  * codes of acpi_fwnode_get_reference_with_args" is merged.
> >>> +  */
> >>> + if (ret != -ENOENT && ret != -ENODATA)
> >>
> >> So while that patch referenced in the To-do above is not merged yet,
> >> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
> >> Or ENOENT now and ENODATA later? Or vice versa?
> >>
> >> I can't tell based on that information whether this code is correct or not.
> >>
> >> The comment needs to explain this a bit better.
> > 
> > I'll add this:
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index a32473f95be1..74fcc3ba9ebd 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(
> > /*
> >  * To-do: handle -ENODATA when "device property: Align return
> >  * codes of acpi_fwnode_get_reference_with_args" is merged.
> 
> So after this patch referred to in the To-do is applied it will only
> return ENODATA?
> 
> In that case, change 'handle' to 'handle only'.

That depends a bit in which form the patch will be eventually accepted. The
underlying issue there is that different error codes are used to signal
conditions for out-of-bounds access and missing entry. After aligning them
the code here can be updated.

> 
> > +* Right now, both -ENODATA and -ENOENT signal the end of
> > +* references where only a single error code should be used
> > +* for the purpose.
> >  */
> > if (ret != -ENOENT && ret != -ENODATA)
> > return ret;
> > 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v8 1/4] rockchip/rga: v4l2 m2m support

[kbuild test robot]

Hi Jacob,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.13 next-20170913]
[cannot apply to rockchip/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jacob-Chen/Add-Rockchip-RGA-V4l2-support/20170913-171106
base:   git://linuxtv.org/media_tree.git master
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   drivers/media/platform/rockchip-rga/rga.c: In function 'rga_setup_ctrls':
>> drivers/media/platform/rockchip-rga/rga.c:190:11: error: 
>> 'V4L2_CID_PORTER_DUFF_MODE' undeclared (first use in this function)
  V4L2_CID_PORTER_DUFF_MODE,
  ^
   drivers/media/platform/rockchip-rga/rga.c:190:11: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/media/platform/rockchip-rga/rga.c:191:11: error: 
>> 'V4L2_PORTER_DUFF_CLEAR' undeclared (first use in this function)
  V4L2_PORTER_DUFF_CLEAR, 0,
  ^~
>> drivers/media/platform/rockchip-rga/rga.c:192:11: error: 
>> 'V4L2_PORTER_DUFF_SRC' undeclared (first use in this function)
  V4L2_PORTER_DUFF_SRC);
  ^~~~

vim +/V4L2_CID_PORTER_DUFF_MODE +190 drivers/media/platform/rockchip-rga/rga.c

   182  
   183  static int rga_setup_ctrls(struct rga_ctx *ctx)
   184  {
   185  struct rockchip_rga *rga = ctx->rga;
   186  
   187  v4l2_ctrl_handler_init(>ctrl_handler, 5);
   188  
   189  v4l2_ctrl_new_std_menu(>ctrl_handler, _ctrl_ops,
 > 190 V4L2_CID_PORTER_DUFF_MODE,
 > 191 V4L2_PORTER_DUFF_CLEAR, 0,
 > 192 V4L2_PORTER_DUFF_SRC);
   193  
   194  v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
   195V4L2_CID_HFLIP, 0, 1, 1, 0);
   196  
   197  v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
   198V4L2_CID_VFLIP, 0, 1, 1, 0);
   199  
   200  v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
   201V4L2_CID_ROTATE, 0, 270, 90, 0);
   202  
   203  v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
   204V4L2_CID_BG_COLOR, 0, 0x, 1, 0);
   205  
   206  if (ctx->ctrl_handler.error) {
   207  int err = ctx->ctrl_handler.error;
   208  
   209  v4l2_err(>v4l2_dev, "%s failed\n", __func__);
   210  v4l2_ctrl_handler_free(>ctrl_handler);
   211  return err;
   212  }
   213  
   214  return 0;
   215  }
   216  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain device / interger references

[Sakari Ailus]

Hi Hans,

On Wed, Sep 13, 2017 at 09:57:52AM +0200, Hans Verkuil wrote:
> The subject still has the 'interger' typo.
> 
> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> > v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
> > the device's own fwnode, it will follow child fwnodes with the given
> > property -- value pair and return the resulting fwnode.
> 
> As suggested before: 'property-value' is easier to read than ' -- '.

Oops. I must have missed some of your comments while making changes.
Apologies for that.

> 
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 145 
> > ++
> >  1 file changed, 145 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index a32473f95be1..a07599a8f647 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -567,6 +567,151 @@ static int v4l2_fwnode_reference_parse(
> > return ret;
> >  }
> >  
> > +/*
> > + * v4l2_fwnode_reference_get_int_prop - parse a reference with integer
> > + * arguments
> > + * @dev: struct device pointer
> > + * @notifier: notifier for @dev
> > + * @prop: the name of the property
> 
> @index is not documented.

Fixed.

> 
> > + * @props: the array of integer property names
> > + * @nprops: the number of integer properties
> 
> properties -> property names in @props

Fixed.

> 
> > + *
> > + * Find fwnodes referred to by a property @prop, then under that 
> > iteratively
> > + * follow each child node which has a property the value matches the 
> > integer
> 
> "the value" -> "whose value" or "with a value that"
> 
> At least, I think that's what you mean here.
> 
> How is @props/@nprops used?
> 
> > + * argument at an index.
> 
> I assume this should be "the @index"?

Um, no. This is the index to the @props array. I'll clarify the
documentation for this function.

> 
> > + *
> > + * Return: 0 on success
> > + *-ENOENT if no entries (or the property itself) were found
> > + *-EINVAL if property parsing otherwisefailed
> 
> Missing space before "failed"

Fixed.

> 
> > + *-ENOMEM if memory allocation failed
> > + */
> > +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
> > +   struct fwnode_handle *fwnode, const char *prop, unsigned int index,
> > +   const char **props, unsigned int nprops)
> > +{
> > +   struct fwnode_reference_args fwnode_args;
> > +   unsigned int *args = fwnode_args.args;
> > +   struct fwnode_handle *child;
> > +   int ret;
> > +
> > +   /*
> > +* Obtain remote fwnode as well as the integer arguments.
> > +*
> > +* To-do: handle -ENODATA when "device property: Align return
> > +* codes of acpi_fwnode_get_reference_with_args" is merged.
> > +*/
> > +   ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
> > +index, _args);
> > +   if (ret)
> > +   return ERR_PTR(ret == -ENODATA ? -ENOENT : ret);
> > +
> > +   /*
> > +* Find a node in the tree under the referred fwnode corresponding the
> 
> the -> to the

Fixed.

> 
> > +* integer arguments.
> > +*/
> > +   fwnode = fwnode_args.fwnode;
> > +   while (nprops) {
> 
> This can be 'while (nprops--) {'.

Changed.

> 
> > +   u32 val;
> > +
> > +   /* Loop over all child nodes under fwnode. */
> > +   fwnode_for_each_child_node(fwnode, child) {
> > +   if (fwnode_property_read_u32(child, *props, ))
> > +   continue;
> > +
> > +   /* Found property, see if its value matches. */
> > +   if (val == *args)
> > +   break;
> > +   }
> > +
> > +   fwnode_handle_put(fwnode);
> > +
> > +   /* No property found; return an error here. */
> > +   if (!child) {
> > +   fwnode = ERR_PTR(-ENOENT);
> > +   break;
> > +   }
> > +
> > +   props++;
> > +   args++;
> > +   fwnode = child;
> > +   nprops--;
> > +   }
> > +
> > +   return fwnode;
> > +}
> 
> You really need to add an ACPI example as comment for this source code.

I can copy the relevant portions of the LED flash example, and remove them
once the example is merged to mainline. That example really doesn't belong
here though.

> 
> I still don't understand the code. I know you pointed me to an example,
> but I can't remember/find what it was. Either copy the example here or
> point to the file containing the example (copying is best IMHO).

:-)

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references

[Hans Verkuil]

On 09/13/17 11:24, Sakari Ailus wrote:
> Hi Hans,
> 
> Thanks for the review!
> 
> On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:
>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
>>> Add function v4l2_fwnode_reference_count() for counting external
>>
>>  There is no function v4l2_fwnode_reference_count()?!
> 
> It got removed during the revisions but the commit message was not changed
> accordingly, I do that now.
> 
>>
>>> references and v4l2_fwnode_reference_parse() for parsing them as async
>>> sub-devices.
>>>
>>> This can be done on e.g. flash or lens async sub-devices that are not part
>>> of but are associated with a sensor.
>>>
>>> struct v4l2_async_notifier.max_subdevs field is added to contain the
>>> maximum number of sub-devices in a notifier to reflect the memory
>>> allocated for the subdevs array.
>>
>> You forgot to remove this outdated paragraph.
> 
> Oops. Removed it now.
> 
>>
>>>
>>> Signed-off-by: Sakari Ailus 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
>>> +++
>>>  1 file changed, 69 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
>>> b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> index 44ee35f6aad5..a32473f95be1 100644
>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>>>  }
>>>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
>>>  
>>> +/*
>>> + * v4l2_fwnode_reference_parse - parse references for async sub-devices
>>> + * @dev: the device node the properties of which are parsed for references
>>> + * @notifier: the async notifier where the async subdevs will be added
>>> + * @prop: the name of the property
>>> + *
>>> + * Return: 0 on success
>>> + *-ENOENT if no entries were found
>>> + *-ENOMEM if memory allocation failed
>>> + *-EINVAL if property parsing failed
>>> + */
>>> +static int v4l2_fwnode_reference_parse(
>>> +   struct device *dev, struct v4l2_async_notifier *notifier,
>>> +   const char *prop)
>>> +{
>>> +   struct fwnode_reference_args args;
>>> +   unsigned int index;
>>> +   int ret;
>>> +
>>> +   for (index = 0;
>>> +!(ret = fwnode_property_get_reference_args(
>>> +  dev_fwnode(dev), prop, NULL, 0, index, ));
>>> +index++)
>>> +   fwnode_handle_put(args.fwnode);
>>> +
>>> +   if (!index)
>>> +   return -ENOENT;
>>> +
>>> +   /*
>>> +* To-do: handle -ENODATA when "device property: Align return
>>> +* codes of acpi_fwnode_get_reference_with_args" is merged.
>>> +*/
>>> +   if (ret != -ENOENT && ret != -ENODATA)
>>
>> So while that patch referenced in the To-do above is not merged yet,
>> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
>> Or ENOENT now and ENODATA later? Or vice versa?
>>
>> I can't tell based on that information whether this code is correct or not.
>>
>> The comment needs to explain this a bit better.
> 
> I'll add this:
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index a32473f95be1..74fcc3ba9ebd 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(
>   /*
>* To-do: handle -ENODATA when "device property: Align return
>* codes of acpi_fwnode_get_reference_with_args" is merged.

So after this patch referred to in the To-do is applied it will only
return ENODATA?

In that case, change 'handle' to 'handle only'.

> +  * Right now, both -ENODATA and -ENOENT signal the end of
> +  * references where only a single error code should be used
> +  * for the purpose.
>*/
>   if (ret != -ENOENT && ret != -ENODATA)
>   return ret;
> 

Regards,

Hans

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Hoegeun Kwon]

On 09/13/2017 06:11 PM, Sylwester Nawrocki wrote:

Hi Hoegeun,

On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:

@@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
void *priv)
  static const struct of_device_id exynos_gsc_match[] = {
{
-.compatible = "samsung,exynos5-gsc",
-.data = _v_100_drvdata,

Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
data, so that it can work with "samsung,exynos5-gsc" compatible in DT
on both exynos5250 and exynos5420 SoCs?


Thank you for your question.

Exynos 5250 and 5420 have different hardware rotation limits.
Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
2048'.

So my opinion they must have different compatible.

I think there is some misunderstanding, mu suggestion was to keep the
"samsung,exynos5-gsc" compatible entry in addition to the new introduced
ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
order to make your changes possibly backward compatible, in theory
the updated driver should still work without changes in dts.



Thank you again for your explanation.

Yes, I understood.
I will keep "samsung,exynos5-gsc" compatible,
and add Exynos 5250/5420/5433 compatible.

Best regards,
Hoegeun

Re: [PATCH] [media] s3c-camif: fix out-of-bounds array access

[Sylwester Nawrocki]

On 09/12/2017 10:09 PM, Arnd Bergmann wrote:
> While experimenting with older compiler versions, I ran
> into a warning that no longer shows up on gcc-4.8 or newer:
> 
> drivers/media/platform/s3c-camif/camif-capture.c: In function 
> '__camif_subdev_try_format':
> drivers/media/platform/s3c-camif/camif-capture.c:1265:25: error: array 
> subscript is below array bounds
> 
> This is an off-by-one bug, leading to an access before the start of the
> array, while newer compilers silently assume this undefined behavior
> cannot happen and leave the loop at index 0 if no other entry matches.
> 
> Since the code is not only wrong, but also has no effect besides the
> out-of-bounds access, this patch just removes it.
> 
> I found an existing gcc bug for it and added a reduced version
> of the function there.
> 
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69249#c3
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series 
> camera interface")
> Signed-off-by: Arnd Bergmann 
> ---
>   drivers/media/platform/s3c-camif/camif-capture.c | 7 ---
>   1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
> b/drivers/media/platform/s3c-camif/camif-capture.c
> index 25c7a7d42292..c6921f6a5a6a 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1256,17 +1256,10 @@ static void __camif_subdev_try_format(struct 
> camif_dev *camif,
>   {
>   const struct s3c_camif_variant *variant = camif->variant;
>   const struct vp_pix_limits *pix_lim;
> - int i = ARRAY_SIZE(camif_mbus_formats);
>   
>   /* FIXME: constraints against codec or preview path ? */
>   pix_lim = >vp_pix_limits[VP_CODEC];
>   
> - while (i-- >= 0)
> - if (camif_mbus_formats[i] == mf->code)
> - break;
> -
> - mf->code = camif_mbus_formats[i];


Interesting finding... the function needs to ensure mf->code is set 
to one of supported values by the driver, so instead of removing 
how about changing the above line to:

if (i < 0)
mf->code = camif_mbus_formats[0];

?

-- 
Thanks,
Sylwester

Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to sub-devices

[Hans Verkuil]

On 09/13/17 10:29, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Sep 13, 2017 at 09:17:08AM +0200, Hans Verkuil wrote:
>> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
>>> Registering a notifier has required the knowledge of struct v4l2_device
>>> for the reason that sub-devices generally are registered to the
>>> v4l2_device (as well as the media device, also available through
>>> v4l2_device).
>>>
>>> This information is not available for sub-device drivers at probe time.
>>>
>>> What this patch does is that it allows registering notifiers without
>>> having v4l2_device around. Instead the sub-device pointer is stored in the
>>> notifier. Once the sub-device of the driver that registered the notifier
>>> is registered, the notifier will gain the knowledge of the v4l2_device,
>>> and the binding of async sub-devices from the sub-device driver's notifier
>>> may proceed.
>>>
>>> The root notifier's complete callback is only called when all sub-device
>>> notifiers are completed.
>>>
>>> Signed-off-by: Sakari Ailus 
>>
>> Just two small comments (see below).
>>
>> After changing those (the first is up to you) you can add my:
>>
>> Acked-by: Hans Verkuil 
> 
> Thanks; please see my comments below.
> 
> ...
> 
>>> +/* Return true if all sub-device notifiers are complete, false otherwise. 
>>> */
>>> +static bool v4l2_async_subdev_notifiers_complete(
>>> +   struct v4l2_async_notifier *notifier)
>>> +{
>>> +   struct v4l2_subdev *sd;
>>> +
>>> +   if (!list_empty(>waiting))
>>> +   return false;
>>> +
>>> +   list_for_each_entry(sd, >done, async_list) {
>>> +   struct v4l2_async_notifier *subdev_notifier =
>>> +   v4l2_async_find_subdev_notifier(sd);
>>> +
>>> +   if (!subdev_notifier)
>>> +   continue;
>>> +
>>> +   if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
>>
>> I think it is better to change this to:
>>
>>  if (subdev_notifier &&
>>  !v4l2_async_subdev_notifiers_complete(subdev_notifier))
>>
>> and drop this if..continue above. That's a bit overkill in this simple case.
>>
>> It's up to you, though.
> 
> Yes, makes sense.
> 
> ...
> 
>>> +/* Try completing a notifier. */
>>> +static int v4l2_async_notifier_try_complete(
>>> +   struct v4l2_async_notifier *notifier)
>>> +{
>>> +   do {
>>> +   int ret;
>>> +
>>> +   /* Any local async sub-devices left? */
>>> +   if (!list_empty(>waiting))
>>> +   return 0;
>>> +
>>> +   /*
>>> +* Any sub-device notifiers waiting for async subdevs
>>> +* to be bound?
>>> +*/
>>> +   if (!v4l2_async_subdev_notifiers_complete(notifier))
>>> +   return 0;
>>> +
>>> +   /* Proceed completing the notifier */
>>> +   ret = v4l2_async_notifier_call_complete(notifier);
>>> +   if (ret < 0)
>>> +   return ret;
>>> +
>>> +   /*
>>> +* Obtain notifier's parent. If there is one, repeat
>>> +* the process, otherwise we're done here.
>>> +*/
>>> +   } while ((notifier = notifier->parent));
>>
>> I'd change this to:
>>
>>  notifier = notifier->parent;
>>  } while (notifier);
>>
>> Assignment in a condition is frowned upon, and there is no need to do that
>> here.
> 
> Wouldn't that equally apply to any statement with side effects? In other
> words, what you're suggesting for patch 19? :-)

I don't like it there either, but rewriting that would make the code quite a
bit longer and you enter a gray area between 'no side-effects' and 
'readability'.
In cases like that I tend to accept the preference of the author of the code.

In the case of this do...while the 'no side-effects' version is just as readable
if not more so than the 'side-effect' version.

At least, that's my reasoning.

Regards,

Hans

Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references

[Sakari Ailus]

Hi Hans,

Thanks for the review!

On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote:
> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> > Add function v4l2_fwnode_reference_count() for counting external
> 
>  There is no function v4l2_fwnode_reference_count()?!

It got removed during the revisions but the commit message was not changed
accordingly, I do that now.

> 
> > references and v4l2_fwnode_reference_parse() for parsing them as async
> > sub-devices.
> > 
> > This can be done on e.g. flash or lens async sub-devices that are not part
> > of but are associated with a sensor.
> > 
> > struct v4l2_async_notifier.max_subdevs field is added to contain the
> > maximum number of sub-devices in a notifier to reflect the memory
> > allocated for the subdevs array.
> 
> You forgot to remove this outdated paragraph.

Oops. Removed it now.

> 
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
> > +++
> >  1 file changed, 69 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 44ee35f6aad5..a32473f95be1 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
> >  
> > +/*
> > + * v4l2_fwnode_reference_parse - parse references for async sub-devices
> > + * @dev: the device node the properties of which are parsed for references
> > + * @notifier: the async notifier where the async subdevs will be added
> > + * @prop: the name of the property
> > + *
> > + * Return: 0 on success
> > + *-ENOENT if no entries were found
> > + *-ENOMEM if memory allocation failed
> > + *-EINVAL if property parsing failed
> > + */
> > +static int v4l2_fwnode_reference_parse(
> > +   struct device *dev, struct v4l2_async_notifier *notifier,
> > +   const char *prop)
> > +{
> > +   struct fwnode_reference_args args;
> > +   unsigned int index;
> > +   int ret;
> > +
> > +   for (index = 0;
> > +!(ret = fwnode_property_get_reference_args(
> > +  dev_fwnode(dev), prop, NULL, 0, index, ));
> > +index++)
> > +   fwnode_handle_put(args.fwnode);
> > +
> > +   if (!index)
> > +   return -ENOENT;
> > +
> > +   /*
> > +* To-do: handle -ENODATA when "device property: Align return
> > +* codes of acpi_fwnode_get_reference_with_args" is merged.
> > +*/
> > +   if (ret != -ENOENT && ret != -ENODATA)
> 
> So while that patch referenced in the To-do above is not merged yet,
> what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
> Or ENOENT now and ENODATA later? Or vice versa?
> 
> I can't tell based on that information whether this code is correct or not.
> 
> The comment needs to explain this a bit better.

I'll add this:

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
b/drivers/media/v4l2-core/v4l2-fwnode.c
index a32473f95be1..74fcc3ba9ebd 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse(
/*
 * To-do: handle -ENODATA when "device property: Align return
 * codes of acpi_fwnode_get_reference_with_args" is merged.
+* Right now, both -ENODATA and -ENOENT signal the end of
+* references where only a single error code should be used
+* for the purpose.
 */
if (ret != -ENOENT && ret != -ENODATA)
return ret;

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v3] drivers/staging:[media]atomisp:use ARRAY_SIZE() instead of open coding.

[Allen]

> bad:  [PATCH v3] drivers/staging:[media]atomisp:use ARRAY_SIZE() instead of 
> open coding.
> good: [PATCH v4] [media] atomisp: use ARRAY_SIZE() instead of open coding.

My bad. Fixed it in V4. Thanks.

- Allen

[PATCH v4] [media]atomisp:use ARRAY_SIZE() instead of open coding.

[Allen Pais]

The array_length() macro just duplicates ARRAY_SIZE(), so we can
delete it.

v4: Update the commit message.

Signed-off-by: Allen Pais 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
index e882b55..bee3043 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
@@ -451,8 +451,6 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
IA_CSS_FRAME_FORMAT_YUYV
 };
 
-#define array_length(array) (sizeof(array)/sizeof(array[0]))
-
 /* Verify whether the selected output format is can be produced
  * by the copy binary given the stream format.
  * */
@@ -468,7 +466,7 @@ verify_copy_out_frame_format(struct ia_css_pipe *pipe)
switch (pipe->stream->config.input_config.format) {
case IA_CSS_STREAM_FORMAT_YUV420_8_LEGACY:
case IA_CSS_STREAM_FORMAT_YUV420_8:
-   for (i=0; i

Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits

[Sylwester Nawrocki]

Hi Hoegeun,

On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:
>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
>>> void *priv)
>>>  static const struct of_device_id exynos_gsc_match[] = {
>>>{
>>> -.compatible = "samsung,exynos5-gsc",
>>> -.data = _v_100_drvdata,
>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
>> on both exynos5250 and exynos5420 SoCs?
>>
> Thank you for your question.
> 
> Exynos 5250 and 5420 have different hardware rotation limits.
> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
> 2048'.
> 
> So my opinion they must have different compatible.

I think there is some misunderstanding, mu suggestion was to keep the 
"samsung,exynos5-gsc" compatible entry in addition to the new introduced 
ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
order to make your changes possibly backward compatible, in theory 
the updated driver should still work without changes in dts.

-- 
Regards,
Sylwester

Re: [PATCH v3] drivers/staging:[media]atomisp:use ARRAY_SIZE() instead of open coding.

[Dan Carpenter]

On Wed, Sep 13, 2017 at 02:27:53PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 

Sorry, the patch is right, but the commit is still totally messed up.

bad:  [PATCH v3] drivers/staging:[media]atomisp:use ARRAY_SIZE() instead of 
open coding.
good: [PATCH v4] [media] atomisp: use ARRAY_SIZE() instead of open coding.

Please, copy the "[media] atomisp: " prefix exactly as I wrote it.  Then
the commit message can say something like:

The array_length() macro just duplicates ARRAY_SIZE() so we can delete
it.

> Signed-off-by: Allen Pais 
> ---
  ^^^

Then under the --- line put:
v4: Update the commit message.

regards,
dan carpenter

Re: [PATCH v2] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Allen]

>
> This is going through linux-media and maybe they won't insist on some
> kind of commit message, but I know Greg does.

 Okay. I sent out a V3.

-- 
   - Allen

[PATCH v3] drivers/staging:[media]atomisp:use ARRAY_SIZE() instead of open coding.

[Allen Pais]

Signed-off-by: Allen Pais 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
index e882b55..bee3043 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
@@ -451,8 +451,6 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
IA_CSS_FRAME_FORMAT_YUYV
 };
 
-#define array_length(array) (sizeof(array)/sizeof(array[0]))
-
 /* Verify whether the selected output format is can be produced
  * by the copy binary given the stream format.
  * */
@@ -468,7 +466,7 @@ verify_copy_out_frame_format(struct ia_css_pipe *pipe)
switch (pipe->stream->config.input_config.format) {
case IA_CSS_STREAM_FORMAT_YUV420_8_LEGACY:
case IA_CSS_STREAM_FORMAT_YUV420_8:
-   for (i=0; i

Re: [PATCHv3 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[Hans Verkuil]

On 09/12/2017 07:39 PM, Ville Syrjälä wrote:
> On Mon, Sep 11, 2017 at 01:25:45PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This adds support for the DisplayPort CEC-Tunneling-over-AUX
>> feature that is part of the DisplayPort 1.3 standard.
>>
>> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
>> chip that has this capability actually hook up the CEC pin, so
>> even though a CEC device is created, it may not actually work.
>>
>> Signed-off-by: Hans Verkuil 
>> Tested-by: Carlos Santa 
>> ---
>>  drivers/gpu/drm/Kconfig  |  10 ++
>>  drivers/gpu/drm/Makefile |   1 +
>>  drivers/gpu/drm/drm_dp_cec.c | 301 
>> +++
>>  include/drm/drm_dp_helper.h  |  24 
>>  4 files changed, 336 insertions(+)
>>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 83cb2a88c204..1f2708df5c4e 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
>>default case is N. Details and instructions how to build your own
>>EDID data are given in Documentation/EDID/HOWTO.txt.
>>  
>> +config DRM_DP_CEC
>> +bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
>> +select CEC_CORE
>> +help
>> +  Choose this option if you want to enable HDMI CEC support for
>> +  DisplayPort/USB-C to HDMI adapters.
>> +
>> +  Note: not all adapters support this feature, and even for those
>> +  that do support this they often do not hook up the CEC pin.
>> +
>>  config DRM_TTM
>>  tristate
>>  depends on DRM && MMU
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 24a066e1841c..c6552c62049e 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += 
>> drm_edid_load.o
>>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
>>  
>>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>>  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
>> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
>> new file mode 100644
>> index ..b831bb72c932
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_dp_cec.c
>> @@ -0,0 +1,301 @@
>> +/*
>> + * DisplayPort CEC-Tunneling-over-AUX support
>> + *
>> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights 
>> reserved.
>> + *
>> + * This program is free software; you may 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.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/*
>> + * Unfortunately it turns out that we have a chicken-and-egg situation
>> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
>> + * have a converter chip that supports CEC-Tunneling-over-AUX (usually the
>> + * Parade PS176), but they do not wire up the CEC pin, thus making CEC
>> + * useless.
>> + *
>> + * Sadly there is no way for this driver to know this. What happens is
>> + * that a /dev/cecX device is created that is isolated and unable to see
>> + * any of the other CEC devices. Quite literally the CEC wire is cut
>> + * (or in this case, never connected in the first place).
>> + *
>> + * I suspect that the reason so few adapters support this is that this
>> + * tunneling protocol was never supported by any OS. So there was no
>> + * easy way of testing it, and no incentive to correctly wire up the
>> + * CEC pin.
>> + *
>> + * Hopefully by creating this driver it will be easier for vendors to
>> + * finally fix their adapters and test the CEC functionality.
>> + *
>> + * I keep a list of known working adapters here:
>> + *
>> + * https://hverkuil.home.xs4all.nl/cec-status.txt
>> + *
>> + * Please mail me (hverk...@xs4all.nl) if you find an adapter that works
>> + * and is not yet listed there.
>> + */
>> +
>> +/**
>> + * DOC: dp cec helpers
>> + *
>> + * These functions take care of supporting the CEC-Tunneling-over-AUX
>> + * feature of 

Re: [git:media_tree/master] media: adv7180: add missing adv7180cp, adv7180st i2c device IDs

[Geert Uytterhoeven]

On Thu, Jul 20, 2017 at 12:54 PM, Mauro Carvalho Chehab
 wrote:
> This is an automatic generated email to let you know that the following patch 
> were queued:
>
> Subject: media: adv7180: add missing adv7180cp, adv7180st i2c device IDs
> Author:  Ulrich Hecht 
> Date:Mon Jul 3 04:43:33 2017 -0400
>
> Fixes a crash on Renesas R8A7793 Gose board that uses these "compatible"
> entries.
>
> Signed-off-by: Ulrich Hecht 
> Tested-by: Geert Uytterhoeven 
> Signed-off-by: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 

Fixes: ce1ec5c07e0671cc ("[media] media: adv7180: add adv7180cp,
adv7180st compatible strings")

This fix is now upstream, as commit 281ddc3cdc10413b ("media: adv7180: add
missing adv7180cp, adv7180st i2c device IDs").

Greg: Can you please backport it to v3.14.x?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Dan Carpenter]

On Wed, Sep 13, 2017 at 02:09:25PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 

This is going through linux-media and maybe they won't insist on some
kind of commit message, but I know Greg does.

regards,
dan carpenter

Re: [PATCH] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Dan Carpenter]

Also change the subject prefix to: [media] atomisp: so it's:

Subject: [PATCH v2] [media] atomisp: Use ARRAY_SIZE() instead of open coding it

regards,
dan carpenter

Re: [PATCH] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Allen]

>>
>> -#define array_length(array) (sizeof(array)/sizeof(array[0]))
>> +#define array_length(array) (ARRAY_SIZE(array))
>
> Just get rid of this array_length macro and use ARRAY_SIZE() directly.
>

 Sure.

   - Allen

[PATCH v2] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Allen Pais]

Signed-off-by: Allen Pais 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
index e882b55..bee3043 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
@@ -451,8 +451,6 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
IA_CSS_FRAME_FORMAT_YUYV
 };
 
-#define array_length(array) (sizeof(array)/sizeof(array[0]))
-
 /* Verify whether the selected output format is can be produced
  * by the copy binary given the stream format.
  * */
@@ -468,7 +466,7 @@ verify_copy_out_frame_format(struct ia_css_pipe *pipe)
switch (pipe->stream->config.input_config.format) {
case IA_CSS_STREAM_FORMAT_YUV420_8_LEGACY:
case IA_CSS_STREAM_FORMAT_YUV420_8:
-   for (i=0; i

Re: [PATCH v2] v4l-ioctl: Fix typo on v4l_print_frmsizeenum

[Sakari Ailus]

On Wed, Sep 13, 2017 at 09:35:52AM +0200, Ricardo Ribalda Delgado wrote:
> max_width and max_height are swap with step_width and step_height.
> 
> Signed-off-by: Ricardo Ribalda Delgado 

Acked-by: Sakari Ailus 

> ---
> 
> Since that this bug has been here for ever. I do not know if we should
> notify stable or not
> 
> I have also cut the lines to respect the 80 char limit
> 
> v2: Suggested by Laurent Pinchart 
> -Fix indentation
> 
>  drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index b60a6b0841d1..0292f327467d 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -730,9 +730,9 @@ static void v4l_print_frmsizeenum(const void *arg, bool 
> write_only)
>   break;
>   case V4L2_FRMSIZE_TYPE_STEPWISE:
>   pr_cont(", min=%ux%u, max=%ux%u, step=%ux%u\n",
> - p->stepwise.min_width,  p->stepwise.min_height,
> - p->stepwise.step_width, p->stepwise.step_height,
> - p->stepwise.max_width,  p->stepwise.max_height);
> +   p->stepwise.min_width, p->stepwise.min_height,
> +   p->stepwise.max_width, p->stepwise.max_height,
> +   p->stepwise.step_width, p->stepwise.step_height);
>   break;
>   case V4L2_FRMSIZE_TYPE_CONTINUOUS:
>   /* fall through */

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to sub-devices

[Sakari Ailus]

Hi Hans,

On Wed, Sep 13, 2017 at 09:17:08AM +0200, Hans Verkuil wrote:
> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> > Registering a notifier has required the knowledge of struct v4l2_device
> > for the reason that sub-devices generally are registered to the
> > v4l2_device (as well as the media device, also available through
> > v4l2_device).
> > 
> > This information is not available for sub-device drivers at probe time.
> > 
> > What this patch does is that it allows registering notifiers without
> > having v4l2_device around. Instead the sub-device pointer is stored in the
> > notifier. Once the sub-device of the driver that registered the notifier
> > is registered, the notifier will gain the knowledge of the v4l2_device,
> > and the binding of async sub-devices from the sub-device driver's notifier
> > may proceed.
> > 
> > The root notifier's complete callback is only called when all sub-device
> > notifiers are completed.
> > 
> > Signed-off-by: Sakari Ailus 
> 
> Just two small comments (see below).
> 
> After changing those (the first is up to you) you can add my:
> 
> Acked-by: Hans Verkuil 

Thanks; please see my comments below.

...

> > +/* Return true if all sub-device notifiers are complete, false otherwise. 
> > */
> > +static bool v4l2_async_subdev_notifiers_complete(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   struct v4l2_subdev *sd;
> > +
> > +   if (!list_empty(>waiting))
> > +   return false;
> > +
> > +   list_for_each_entry(sd, >done, async_list) {
> > +   struct v4l2_async_notifier *subdev_notifier =
> > +   v4l2_async_find_subdev_notifier(sd);
> > +
> > +   if (!subdev_notifier)
> > +   continue;
> > +
> > +   if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
> 
> I think it is better to change this to:
> 
>   if (subdev_notifier &&
>   !v4l2_async_subdev_notifiers_complete(subdev_notifier))
> 
> and drop this if..continue above. That's a bit overkill in this simple case.
> 
> It's up to you, though.

Yes, makes sense.

...

> > +/* Try completing a notifier. */
> > +static int v4l2_async_notifier_try_complete(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   do {
> > +   int ret;
> > +
> > +   /* Any local async sub-devices left? */
> > +   if (!list_empty(>waiting))
> > +   return 0;
> > +
> > +   /*
> > +* Any sub-device notifiers waiting for async subdevs
> > +* to be bound?
> > +*/
> > +   if (!v4l2_async_subdev_notifiers_complete(notifier))
> > +   return 0;
> > +
> > +   /* Proceed completing the notifier */
> > +   ret = v4l2_async_notifier_call_complete(notifier);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   /*
> > +* Obtain notifier's parent. If there is one, repeat
> > +* the process, otherwise we're done here.
> > +*/
> > +   } while ((notifier = notifier->parent));
> 
> I'd change this to:
> 
>   notifier = notifier->parent;
>   } while (notifier);
> 
> Assignment in a condition is frowned upon, and there is no need to do that
> here.

Wouldn't that equally apply to any statement with side effects? In other
words, what you're suggesting for patch 19? :-)

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Dan Carpenter]

You need a changelog.

On Wed, Sep 13, 2017 at 01:34:39PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 
> ---
>  drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> index e882b55..d822918 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> @@ -451,7 +451,7 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
>   IA_CSS_FRAME_FORMAT_YUYV
>  };
>  
> -#define array_length(array) (sizeof(array)/sizeof(array[0]))
> +#define array_length(array) (ARRAY_SIZE(array))

Just get rid of this array_length macro and use ARRAY_SIZE() directly.

regards,
dan carpenter

Re: [PATCHv4 3/5] dt-bindings: document the CEC GPIO bindings

[Hans Verkuil]

On 09/12/2017 04:43 PM, Rob Herring wrote:
> On Thu, Aug 31, 2017 at 01:01:54PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Document the bindings for the cec-gpio module for hardware where the
>> CEC line and optionally the HPD line are connected to GPIO lines.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  .../devicetree/bindings/media/cec-gpio.txt | 22 
>> ++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/cec-gpio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/cec-gpio.txt 
>> b/Documentation/devicetree/bindings/media/cec-gpio.txt
>> new file mode 100644
>> index ..db20a7452dbd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/cec-gpio.txt
>> @@ -0,0 +1,22 @@
>> +* HDMI CEC GPIO driver
>> +
>> +The HDMI CEC GPIO module supports CEC implementations where the CEC line
>> +is hooked up to a pull-up GPIO line and - optionally - the HPD line is
>> +hooked up to another GPIO line.
>> +
>> +Required properties:
>> +  - compatible: value must be "cec-gpio"
>> +  - cec-gpio: gpio that the CEC line is connected to
> 
> cec-gpios

Will change.

> 
>> +
>> +Optional property:
>> +  - hpd-gpio: gpio that the HPD line is connected to
> 
> hpd-gpios

Will change.

> 
> However, HPD is already part of the HDMI connector binding. Having it in 
> 2 places would be wrong.

No. This is not an HDMI receiver/transmitter. There are two use-cases for this
driver:

1) For HDMI receivers/transmitters that connect the CEC pin of an HDMI connector
   to a GPIO pin. In that case the HPD goes to the HDMI transmitter/receiver and
   not to this driver. As you say, that would not make any sense.

   But currently no such devices are in the kernel (I know they exist, though).
   Once such a driver would appear in the kernel then these bindings need to be
   extended with an hdmi-phandle.

2) This driver is used for debugging CEC like this:

https://hverkuil.home.xs4all.nl/rpi3-cec.jpg

   Here the CEC pin of an HDMI breakout connector is hooked up to a Raspberry Pi
   GPIO pin and the RPi monitors it. It's a cheap but very effective CEC 
analyzer.
   In this use-case it is very helpful to also monitor the HPD pin since some
   displays do weird things with the HPD and knowing the state of the HPD helps
   a lot when debugging CEC problems. It's optional and it only monitors the 
pin.

   Actually, there does not have to be an HDMI connector involved at all: you 
can
   make two cec-gpio instances and just connect the two GPIO pins together in
   order to emulate two CEC adapters and play with that.

> 
> I think we should have either:
> 
> hdmi-connector {
>   compatible = 'hdmi-connector-a";
>   hpd-gpios = <...>;
>   cec-gpios = <...>;
>   ports {
>   // port to HDMI controller
>   ...
>   };
> };
> 
> Or:
> 
> hdmi-connector {
> compatible = 'hdmi-connector-a";
> hpd-gpios = <...>;
> cec = <>;
> ... 
> };
> 
> cec: cec-gpio {
>   compatible = "cec-gpio";
>   cec-gpios = <...>;
> };
> 
> My preference is probably the former. The latter just helps create a 
> device to bind to a driver, but DT is not the only way to create 
> devices. Then again, if you have a phandle to real CEC controllers in 
> the HDMI connector node, it may make sense to do the same thing with 
> cec-gpio. 
> 
>> +
>> +Example for the Raspberry Pi 3 where the CEC line is connected to
>> +pin 26 aka BCM7 aka CE1 on the GPIO pin header and the HPD line is
>> +connected to pin 11 aka BCM17:
>> +
>> +cec-gpio@7 {
> 
> unit address is not valid. Build your dts's with W=2.

I'll do that.

> 
>> +   compatible = "cec-gpio";
>> +   cec-gpio = < 7 GPIO_OPEN_DRAIN>;
>> +   hpd-gpio = < 17 GPIO_ACTIVE_HIGH>;
>> +};
>> -- 
>> 2.14.1

Regards,

Hans

Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph endpoints, per port

[Sakari Ailus]

Hi Hans,

Thanks for the review.

On Wed, Sep 13, 2017 at 09:06:11AM +0200, Hans Verkuil wrote:
> On 09/13/2017 09:01 AM, Hans Verkuil wrote:
> > On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> >> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it
> >> only parses endpoints on a single port. The behaviour is useful on devices
> >> that have both sinks and sources, in other words only some of these should
> >> be parsed.
> 
> I forgot to mention that I think the log message can be improved: it is not
> clear why you would only parse some of the ports for devices that have both
> sinks and sources. That should be explained better. And probably also in the
> header documentation.

I'll add both.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

[PATCH] drivers:staging/media:Use ARRAY_SIZE() for the size calculation of the array

[Allen Pais]

Signed-off-by: Allen Pais 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
index e882b55..d822918 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
@@ -451,7 +451,7 @@ static enum ia_css_frame_format yuv422_copy_formats[] = {
IA_CSS_FRAME_FORMAT_YUYV
 };
 
-#define array_length(array) (sizeof(array)/sizeof(array[0]))
+#define array_length(array) (ARRAY_SIZE(array))
 
 /* Verify whether the selected output format is can be produced
  * by the copy binary given the stream format.
-- 
2.7.4

Re: [PATCH v12 23/26] et8ek8: Add support for flash and lens devices

[Hans Verkuil]

On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> From: Pavel Machek 
> 
> Parse async sub-devices by using
> v4l2_subdev_fwnode_reference_parse_sensor_common().
> 
> These types devices aren't directly related to the sensor, but are
> nevertheless handled by the et8ek8 driver due to the relationship of these
> component to the main part of the camera module --- the sensor.
> 
> [Sakari Ailus: Rename fwnode function, check for ret < 0 only.]
> Signed-off-by: Pavel Machek 
> Signed-off-by: Sakari Ailus 

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/i2c/et8ek8/et8ek8_driver.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
> b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index c14f0fd6ded3..0ef1b8025935 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -34,10 +34,12 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "et8ek8_reg.h"
>  
> @@ -46,6 +48,7 @@
>  #define ET8EK8_MAX_MSG   8
>  
>  struct et8ek8_sensor {
> + struct v4l2_async_notifier notifier;
>   struct v4l2_subdev subdev;
>   struct media_pad pad;
>   struct v4l2_mbus_framefmt format;
> @@ -1446,6 +1449,11 @@ static int et8ek8_probe(struct i2c_client *client,
>   sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>   sensor->subdev.internal_ops = _internal_ops;
>  
> + ret = v4l2_async_notifier_parse_fwnode_sensor_common(
> + >dev, >notifier);
> + if (ret < 0)
> + goto err_release;
> +
>   sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
>   ret = media_entity_pads_init(>subdev.entity, 1, >pad);
>   if (ret < 0) {
> @@ -1453,18 +1461,27 @@ static int et8ek8_probe(struct i2c_client *client,
>   goto err_mutex;
>   }
>  
> + ret = v4l2_async_subdev_notifier_register(>subdev,
> +   >notifier);
> + if (ret)
> + goto err_entity;
> +
>   ret = v4l2_async_register_subdev(>subdev);
>   if (ret < 0)
> - goto err_entity;
> + goto err_async;
>  
>   dev_dbg(dev, "initialized!\n");
>  
>   return 0;
>  
> +err_async:
> + v4l2_async_notifier_unregister(>notifier);
>  err_entity:
>   media_entity_cleanup(>subdev.entity);
>  err_mutex:
>   mutex_destroy(>power_lock);
> +err_release:
> + v4l2_async_notifier_release(>notifier);
>   return ret;
>  }
>  
> @@ -1480,6 +1497,8 @@ static int __exit et8ek8_remove(struct i2c_client 
> *client)
>   }
>  
>   v4l2_device_unregister_subdev(>subdev);
> + v4l2_async_notifier_unregister(>notifier);
> + v4l2_async_notifier_release(>notifier);
>   device_remove_file(>dev, _attr_priv_mem);
>   v4l2_ctrl_handler_free(>ctrl_handler);
>   v4l2_async_unregister_subdev(>subdev);
> 

Re: [PATCH v12 19/26] v4l: fwnode: Add a helper function to obtain device / interger references

[Hans Verkuil]

The subject still has the 'interger' typo.

On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> v4l2_fwnode_reference_parse_int_prop() will find an fwnode such that under
> the device's own fwnode, it will follow child fwnodes with the given
> property -- value pair and return the resulting fwnode.

As suggested before: 'property-value' is easier to read than ' -- '.

> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 145 
> ++
>  1 file changed, 145 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index a32473f95be1..a07599a8f647 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -567,6 +567,151 @@ static int v4l2_fwnode_reference_parse(
>   return ret;
>  }
>  
> +/*
> + * v4l2_fwnode_reference_get_int_prop - parse a reference with integer
> + *   arguments
> + * @dev: struct device pointer
> + * @notifier: notifier for @dev
> + * @prop: the name of the property

@index is not documented.

> + * @props: the array of integer property names
> + * @nprops: the number of integer properties

properties -> property names in @props

> + *
> + * Find fwnodes referred to by a property @prop, then under that iteratively
> + * follow each child node which has a property the value matches the integer

"the value" -> "whose value" or "with a value that"

At least, I think that's what you mean here.

How is @props/@nprops used?

> + * argument at an index.

I assume this should be "the @index"?

> + *
> + * Return: 0 on success
> + *  -ENOENT if no entries (or the property itself) were found
> + *  -EINVAL if property parsing otherwisefailed

Missing space before "failed"

> + *  -ENOMEM if memory allocation failed
> + */
> +static struct fwnode_handle *v4l2_fwnode_reference_get_int_prop(
> + struct fwnode_handle *fwnode, const char *prop, unsigned int index,
> + const char **props, unsigned int nprops)
> +{
> + struct fwnode_reference_args fwnode_args;
> + unsigned int *args = fwnode_args.args;
> + struct fwnode_handle *child;
> + int ret;
> +
> + /*
> +  * Obtain remote fwnode as well as the integer arguments.
> +  *
> +  * To-do: handle -ENODATA when "device property: Align return
> +  * codes of acpi_fwnode_get_reference_with_args" is merged.
> +  */
> + ret = fwnode_property_get_reference_args(fwnode, prop, NULL, nprops,
> +  index, _args);
> + if (ret)
> + return ERR_PTR(ret == -ENODATA ? -ENOENT : ret);
> +
> + /*
> +  * Find a node in the tree under the referred fwnode corresponding the

the -> to the

> +  * integer arguments.
> +  */
> + fwnode = fwnode_args.fwnode;
> + while (nprops) {

This can be 'while (nprops--) {'.

> + u32 val;
> +
> + /* Loop over all child nodes under fwnode. */
> + fwnode_for_each_child_node(fwnode, child) {
> + if (fwnode_property_read_u32(child, *props, ))
> + continue;
> +
> + /* Found property, see if its value matches. */
> + if (val == *args)
> + break;
> + }
> +
> + fwnode_handle_put(fwnode);
> +
> + /* No property found; return an error here. */
> + if (!child) {
> + fwnode = ERR_PTR(-ENOENT);
> + break;
> + }
> +
> + props++;
> + args++;
> + fwnode = child;
> + nprops--;
> + }
> +
> + return fwnode;
> +}

You really need to add an ACPI example as comment for this source code.

I still don't understand the code. I know you pointed me to an example,
but I can't remember/find what it was. Either copy the example here or
point to the file containing the example (copying is best IMHO).

Regards,

Hans

> +
> +/*
> + * v4l2_fwnode_reference_parse_int_props - parse references for async 
> sub-devices
> + * @dev: struct device pointer
> + * @notifier: notifier for @dev
> + * @prop: the name of the property
> + * @props: the array of integer property names
> + * @nprops: the number of integer properties
> + *
> + * Use v4l2_fwnode_reference_get_int_prop to find fwnodes through reference 
> in
> + * property @prop with integer arguments with child nodes matching in 
> properties
> + * @props. Then, set up V4L2 async sub-devices for those fwnodes in the 
> notifier
> + * accordingly.
> + *
> + * While it is technically possible to use this function on DT, it is only
> + * meaningful on ACPI. On Device tree you can refer to any node in the tree 
> but
> + * on ACPI the references are limited to devices.
> + *
> + * Return: 0 on success
> + *  -ENOENT if no entries (or the property 

[PATCH v2] v4l-ioctl: Fix typo on v4l_print_frmsizeenum

[Ricardo Ribalda Delgado]

max_width and max_height are swap with step_width and step_height.

Signed-off-by: Ricardo Ribalda Delgado 
---

Since that this bug has been here for ever. I do not know if we should
notify stable or not

I have also cut the lines to respect the 80 char limit

v2: Suggested by Laurent Pinchart 
-Fix indentation

 drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index b60a6b0841d1..0292f327467d 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -730,9 +730,9 @@ static void v4l_print_frmsizeenum(const void *arg, bool 
write_only)
break;
case V4L2_FRMSIZE_TYPE_STEPWISE:
pr_cont(", min=%ux%u, max=%ux%u, step=%ux%u\n",
-   p->stepwise.min_width,  p->stepwise.min_height,
-   p->stepwise.step_width, p->stepwise.step_height,
-   p->stepwise.max_width,  p->stepwise.max_height);
+ p->stepwise.min_width, p->stepwise.min_height,
+ p->stepwise.max_width, p->stepwise.max_height,
+ p->stepwise.step_width, p->stepwise.step_height);
break;
case V4L2_FRMSIZE_TYPE_CONTINUOUS:
/* fall through */
-- 
2.14.1

Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references

[Hans Verkuil]

On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> Add function v4l2_fwnode_reference_count() for counting external

 There is no function v4l2_fwnode_reference_count()?!

> references and v4l2_fwnode_reference_parse() for parsing them as async
> sub-devices.
> 
> This can be done on e.g. flash or lens async sub-devices that are not part
> of but are associated with a sensor.
> 
> struct v4l2_async_notifier.max_subdevs field is added to contain the
> maximum number of sub-devices in a notifier to reflect the memory
> allocated for the subdevs array.

You forgot to remove this outdated paragraph.

> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 69 
> +++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 44ee35f6aad5..a32473f95be1 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>  }
>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
>  
> +/*
> + * v4l2_fwnode_reference_parse - parse references for async sub-devices
> + * @dev: the device node the properties of which are parsed for references
> + * @notifier: the async notifier where the async subdevs will be added
> + * @prop: the name of the property
> + *
> + * Return: 0 on success
> + *  -ENOENT if no entries were found
> + *  -ENOMEM if memory allocation failed
> + *  -EINVAL if property parsing failed
> + */
> +static int v4l2_fwnode_reference_parse(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + const char *prop)
> +{
> + struct fwnode_reference_args args;
> + unsigned int index;
> + int ret;
> +
> + for (index = 0;
> +  !(ret = fwnode_property_get_reference_args(
> +dev_fwnode(dev), prop, NULL, 0, index, ));
> +  index++)
> + fwnode_handle_put(args.fwnode);
> +
> + if (!index)
> + return -ENOENT;
> +
> + /*
> +  * To-do: handle -ENODATA when "device property: Align return
> +  * codes of acpi_fwnode_get_reference_with_args" is merged.
> +  */
> + if (ret != -ENOENT && ret != -ENODATA)

So while that patch referenced in the To-do above is not merged yet,
what does fwnode_property_get_reference_args return? ENOENT or ENODATA?
Or ENOENT now and ENODATA later? Or vice versa?

I can't tell based on that information whether this code is correct or not.

The comment needs to explain this a bit better.

> + return ret;
> +
> + ret = v4l2_async_notifier_realloc(notifier,
> +   notifier->num_subdevs + index);
> + if (ret)
> + return ret;
> +
> + for (index = 0; !fwnode_property_get_reference_args(
> +  dev_fwnode(dev), prop, NULL, 0, index, );
> +  index++) {
> + struct v4l2_async_subdev *asd;
> +
> + if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + asd = kzalloc(sizeof(*asd), GFP_KERNEL);
> + if (!asd) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + notifier->subdevs[notifier->num_subdevs] = asd;
> + asd->match.fwnode.fwnode = args.fwnode;
> + asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> + notifier->num_subdevs++;
> + }
> +
> + return 0;
> +
> +error:
> + fwnode_handle_put(args.fwnode);
> + return ret;
> +}
> +
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Sakari Ailus ");
>  MODULE_AUTHOR("Sylwester Nawrocki ");
> 

Regards,

Hans

Re: [PATCH v12 15/26] v4l: async: Allow binding notifiers to sub-devices

[Hans Verkuil]

On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> Registering a notifier has required the knowledge of struct v4l2_device
> for the reason that sub-devices generally are registered to the
> v4l2_device (as well as the media device, also available through
> v4l2_device).
> 
> This information is not available for sub-device drivers at probe time.
> 
> What this patch does is that it allows registering notifiers without
> having v4l2_device around. Instead the sub-device pointer is stored in the
> notifier. Once the sub-device of the driver that registered the notifier
> is registered, the notifier will gain the knowledge of the v4l2_device,
> and the binding of async sub-devices from the sub-device driver's notifier
> may proceed.
> 
> The root notifier's complete callback is only called when all sub-device
> notifiers are completed.
> 
> Signed-off-by: Sakari Ailus 

Just two small comments (see below).

After changing those (the first is up to you) you can add my:

Acked-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/v4l2-core/v4l2-async.c | 219 
> ++-
>  include/media/v4l2-async.h   |  16 ++-
>  2 files changed, 204 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 4525b03d59c1..5082b01d2b96 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct 
> v4l2_async_notifier *n)
>   return n->ops->complete(n);
>  }
>  
> +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> +struct v4l2_subdev *sd,
> +struct v4l2_async_subdev *asd);
> +
>  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>  {
>  #if IS_ENABLED(CONFIG_I2C)
> @@ -124,14 +128,128 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
>   return NULL;
>  }
>  
> +/* Find the sub-device notifier registered by a sub-device driver. */
> +static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier(
> + struct v4l2_subdev *sd)
> +{
> + struct v4l2_async_notifier *n;
> +
> + list_for_each_entry(n, _list, list)
> + if (n->sd == sd)
> + return n;
> +
> + return NULL;
> +}
> +
> +/* Return true if all sub-device notifiers are complete, false otherwise. */
> +static bool v4l2_async_subdev_notifiers_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!list_empty(>waiting))
> + return false;
> +
> + list_for_each_entry(sd, >done, async_list) {
> + struct v4l2_async_notifier *subdev_notifier =
> + v4l2_async_find_subdev_notifier(sd);
> +
> + if (!subdev_notifier)
> + continue;
> +
> + if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))

I think it is better to change this to:

if (subdev_notifier &&
!v4l2_async_subdev_notifiers_complete(subdev_notifier))

and drop this if..continue above. That's a bit overkill in this simple case.

It's up to you, though.

> + return false;
> + }
> +
> + return true;
> +}
> +
> +/* Get v4l2_device related to the notifier if one can be found. */
> +static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(
> + struct v4l2_async_notifier *notifier)
> +{
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + return notifier->v4l2_dev;
> +}
> +
> +/* Test all async sub-devices in a notifier for a match. */
> +static int v4l2_async_notifier_try_all_subdevs(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!v4l2_async_notifier_find_v4l2_dev(notifier))
> + return 0;
> +
> +again:
> + list_for_each_entry(sd, _list, async_list) {
> + struct v4l2_async_subdev *asd;
> + int ret;
> +
> + asd = v4l2_async_find_match(notifier, sd);
> + if (!asd)
> + continue;
> +
> + ret = v4l2_async_match_notify(notifier, sd, asd);
> + if (ret < 0)
> + return ret;
> +
> + /*
> +  * v4l2_async_match_notify() may lead to registering a
> +  * new notifier and thus changing the async subdevs
> +  * list. In order to proceed safely from here, restart
> +  * parsing the list from the beginning.
> +  */
> + goto again;
> + }
> +
> + return 0;
> +}
> +
> +/* Try completing a notifier. */
> +static int v4l2_async_notifier_try_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + do {
> + int ret;
> +
> + /* Any local async 

Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph endpoints, per port

[Hans Verkuil]

On 09/13/2017 09:01 AM, Hans Verkuil wrote:
> On 09/12/2017 03:41 PM, Sakari Ailus wrote:
>> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it
>> only parses endpoints on a single port. The behaviour is useful on devices
>> that have both sinks and sources, in other words only some of these should
>> be parsed.

I forgot to mention that I think the log message can be improved: it is not
clear why you would only parse some of the ports for devices that have both
sinks and sources. That should be explained better. And probably also in the
header documentation.

> 
> I think it would be better to merge this patch with the preceding patch. It
> does not make a lot of sense to have this separate IMHO.
> 
> That said:
> 
> Acked-by: Hans Verkuil 

Sorry, I'm retracting this. It needs a bit more work w.r.t. commit log and
header comments.

Regards,

Hans

> 
> Regards,
> 
>   Hans
> 
>>
>> Signed-off-by: Sakari Ailus 
>> ---
>>  drivers/media/v4l2-core/v4l2-fwnode.c | 59 
>> ---
>>  include/media/v4l2-fwnode.h   | 59 
>> +++
>>  2 files changed, 113 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
>> b/drivers/media/v4l2-core/v4l2-fwnode.c
>> index d978f2d714ca..44ee35f6aad5 100644
>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>> @@ -398,9 +398,9 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
>>  return ret == -ENOTCONN ? 0 : ret;
>>  }
>>  
>> -int v4l2_async_notifier_parse_fwnode_endpoints(
>> +static int __v4l2_async_notifier_parse_fwnode_endpoints(
>>  struct device *dev, struct v4l2_async_notifier *notifier,
>> -size_t asd_struct_size,
>> +size_t asd_struct_size, unsigned int port, bool has_port,
>>  int (*parse_endpoint)(struct device *dev,
>>  struct v4l2_fwnode_endpoint *vep,
>>  struct v4l2_async_subdev *asd))
>> @@ -413,10 +413,25 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>>  return -EINVAL;
>>  
>>  for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
>> - dev_fwnode(dev), fwnode)); )
>> -if (fwnode_device_is_available(
>> + dev_fwnode(dev), fwnode)); ) {
>> +if (!fwnode_device_is_available(
>>  fwnode_graph_get_port_parent(fwnode)))
>> -max_subdevs++;
>> +continue;
>> +
>> +if (has_port) {
>> +struct fwnode_endpoint ep;
>> +
>> +ret = fwnode_graph_parse_endpoint(fwnode, );
>> +if (ret) {
>> +fwnode_handle_put(fwnode);
>> +return ret;
>> +}
>> +
>> +if (ep.port != port)
>> +continue;
>> +}
>> +max_subdevs++;
>> +}
>>  
>>  /* No subdevs to add? Return here. */
>>  if (max_subdevs == notifier->max_subdevs)
>> @@ -437,6 +452,17 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>>  break;
>>  }
>>  
>> +if (has_port) {
>> +struct fwnode_endpoint ep;
>> +
>> +ret = fwnode_graph_parse_endpoint(fwnode, );
>> +if (ret)
>> +break;
>> +
>> +if (ep.port != port)
>> +continue;
>> +}
>> +
>>  ret = v4l2_async_notifier_fwnode_parse_endpoint(
>>  dev, notifier, fwnode, asd_struct_size, parse_endpoint);
>>  if (ret < 0)
>> @@ -447,8 +473,31 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>>  
>>  return ret;
>>  }
>> +
>> +int v4l2_async_notifier_parse_fwnode_endpoints(
>> +struct device *dev, struct v4l2_async_notifier *notifier,
>> +size_t asd_struct_size,
>> +int (*parse_endpoint)(struct device *dev,
>> +struct v4l2_fwnode_endpoint *vep,
>> +struct v4l2_async_subdev *asd))
>> +{
>> +return __v4l2_async_notifier_parse_fwnode_endpoints(
>> +dev, notifier, asd_struct_size, 0, false, parse_endpoint);
>> +}
>>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
>>  
>> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>> +struct device *dev, struct v4l2_async_notifier *notifier,
>> +size_t asd_struct_size, unsigned int port,
>> +int (*parse_endpoint)(struct device *dev,
>> +struct v4l2_fwnode_endpoint *vep,
>> +struct v4l2_async_subdev *asd))
>> +{
>> +return __v4l2_async_notifier_parse_fwnode_endpoints(
>> +dev, notifier, asd_struct_size, port, true, parse_endpoint);
>> +}
>> 

Re: [PATCH v12 06/26] v4l: fwnode: Support generic parsing of graph endpoints, per port

[Hans Verkuil]

On 09/12/2017 03:41 PM, Sakari Ailus wrote:
> This is just like like v4l2_async_notifier_parse_fwnode_endpoints but it
> only parses endpoints on a single port. The behaviour is useful on devices
> that have both sinks and sources, in other words only some of these should
> be parsed.

I think it would be better to merge this patch with the preceding patch. It
does not make a lot of sense to have this separate IMHO.

That said:

Acked-by: Hans Verkuil 

Regards,

Hans

> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 59 
> ---
>  include/media/v4l2-fwnode.h   | 59 
> +++
>  2 files changed, 113 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index d978f2d714ca..44ee35f6aad5 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -398,9 +398,9 @@ static int v4l2_async_notifier_fwnode_parse_endpoint(
>   return ret == -ENOTCONN ? 0 : ret;
>  }
>  
> -int v4l2_async_notifier_parse_fwnode_endpoints(
> +static int __v4l2_async_notifier_parse_fwnode_endpoints(
>   struct device *dev, struct v4l2_async_notifier *notifier,
> - size_t asd_struct_size,
> + size_t asd_struct_size, unsigned int port, bool has_port,
>   int (*parse_endpoint)(struct device *dev,
>   struct v4l2_fwnode_endpoint *vep,
>   struct v4l2_async_subdev *asd))
> @@ -413,10 +413,25 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>   return -EINVAL;
>  
>   for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
> -  dev_fwnode(dev), fwnode)); )
> - if (fwnode_device_is_available(
> +  dev_fwnode(dev), fwnode)); ) {
> + if (!fwnode_device_is_available(
>   fwnode_graph_get_port_parent(fwnode)))
> - max_subdevs++;
> + continue;
> +
> + if (has_port) {
> + struct fwnode_endpoint ep;
> +
> + ret = fwnode_graph_parse_endpoint(fwnode, );
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + return ret;
> + }
> +
> + if (ep.port != port)
> + continue;
> + }
> + max_subdevs++;
> + }
>  
>   /* No subdevs to add? Return here. */
>   if (max_subdevs == notifier->max_subdevs)
> @@ -437,6 +452,17 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>   break;
>   }
>  
> + if (has_port) {
> + struct fwnode_endpoint ep;
> +
> + ret = fwnode_graph_parse_endpoint(fwnode, );
> + if (ret)
> + break;
> +
> + if (ep.port != port)
> + continue;
> + }
> +
>   ret = v4l2_async_notifier_fwnode_parse_endpoint(
>   dev, notifier, fwnode, asd_struct_size, parse_endpoint);
>   if (ret < 0)
> @@ -447,8 +473,31 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>  
>   return ret;
>  }
> +
> +int v4l2_async_notifier_parse_fwnode_endpoints(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + size_t asd_struct_size,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + return __v4l2_async_notifier_parse_fwnode_endpoints(
> + dev, notifier, asd_struct_size, 0, false, parse_endpoint);
> +}
>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
>  
> +int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + size_t asd_struct_size, unsigned int port,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + return __v4l2_async_notifier_parse_fwnode_endpoints(
> + dev, notifier, asd_struct_size, port, true, parse_endpoint);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Sakari Ailus ");
>  MODULE_AUTHOR("Sylwester Nawrocki ");
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index 31fb77e470fa..b2eed4f33e6a 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -257,4 +257,63 @@ int