[PATCH v1] media: ov13858: Limit vblank to permissible range

[Chiranjeevi Rapolu]

Previously, vblank range given to user was too big, falling outside
of permissible range for a given resolution. Sometimes, too low vblank
resulted in errors.

Now, limit vblank to only permissible range for a given resolution.
This change limits lower-bounds of vblank, doesn't affect upper bounds.

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

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 45c0e96..af7af0d 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -57,7 +57,6 @@
 #define OV13858_VTS_30FPS  0x0c8e /* 30 fps */
 #define OV13858_VTS_60FPS  0x0648 /* 60 fps */
 #define OV13858_VTS_MAX0x7fff
-#define OV13858_VBLANK_MIN 56
 
 /* HBLANK control - read only */
 #define OV13858_PPL_270MHZ 2244
@@ -120,7 +119,8 @@ struct ov13858_mode {
u32 height;
 
/* V-timing */
-   u32 vts;
+   u32 vts_def;
+   u32 vts_min;
 
/* Index of Link frequency config to be used */
u32 link_freq_index;
@@ -982,7 +982,8 @@ struct ov13858_mode {
{
.width = 4224,
.height = 3136,
-   .vts = OV13858_VTS_30FPS,
+   .vts_def = OV13858_VTS_30FPS,
+   .vts_min = OV13858_VTS_30FPS,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_4224x3136_regs),
.regs = mode_4224x3136_regs,
@@ -992,7 +993,8 @@ struct ov13858_mode {
{
.width = 2112,
.height = 1568,
-   .vts = OV13858_VTS_30FPS,
+   .vts_def = OV13858_VTS_30FPS,
+   .vts_min = 1608,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2112x1568_regs),
.regs = mode_2112x1568_regs,
@@ -1002,7 +1004,8 @@ struct ov13858_mode {
{
.width = 2112,
.height = 1188,
-   .vts = OV13858_VTS_30FPS,
+   .vts_def = OV13858_VTS_30FPS,
+   .vts_min = 1608,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2112x1188_regs),
.regs = mode_2112x1188_regs,
@@ -1012,7 +1015,8 @@ struct ov13858_mode {
{
.width = 1056,
.height = 784,
-   .vts = OV13858_VTS_30FPS,
+   .vts_def = OV13858_VTS_30FPS,
+   .vts_min = 804,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1056x784_regs),
.regs = mode_1056x784_regs,
@@ -1379,6 +1383,7 @@ static int ov13858_get_pad_format(struct v4l2_subdev *sd,
const struct ov13858_mode *mode;
struct v4l2_mbus_framefmt *framefmt;
s32 vblank_def;
+   s32 vblank_min;
s64 h_blank;
 
mutex_lock(>mutex);
@@ -1399,9 +1404,12 @@ static int ov13858_get_pad_format(struct v4l2_subdev *sd,
ov13858->pixel_rate,
link_freq_configs[mode->link_freq_index].pixel_rate);
/* Update limits and set FPS to default */
-   vblank_def = ov13858->cur_mode->vts - ov13858->cur_mode->height;
+   vblank_def = ov13858->cur_mode->vts_def -
+ov13858->cur_mode->height;
+   vblank_min = ov13858->cur_mode->vts_min -
+ov13858->cur_mode->height;
__v4l2_ctrl_modify_range(
-   ov13858->vblank, OV13858_VBLANK_MIN,
+   ov13858->vblank, vblank_min,
OV13858_VTS_MAX - ov13858->cur_mode->height, 1,
vblank_def);
__v4l2_ctrl_s_ctrl(ov13858->vblank, vblank_def);
@@ -1607,6 +1615,8 @@ static int ov13858_init_controls(struct ov13858 *ov13858)
struct i2c_client *client = v4l2_get_subdevdata(>sd);
struct v4l2_ctrl_handler *ctrl_hdlr;
s64 exposure_max;
+   s64 vblank_def;
+   s64 vblank_min;
int ret;
 
ctrl_hdlr = >ctrl_handler;
@@ -1630,12 +1640,13 @@ static int ov13858_init_controls(struct ov13858 
*ov13858)
link_freq_configs[0].pixel_rate, 1,
link_freq_configs[0].pixel_rate);
 
+   vblank_def = ov13858->cur_mode->vts_def - ov13858->cur_mode->height;
+   vblank_min = ov13858->cur_mode->vts_min - ov13858->cur_mode->height;
ov13858->vblank = v4l2_ctrl_new_std(
ctrl_hdlr, _ctrl_ops, V4L2_CID_VBLANK,
-   OV13858_VBLANK_MIN,
+   vblank_min,
OV13858_VTS_MAX - ov13858->cur_mode->height, 1,
-   

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Fri Aug 18 05:00:17 CEST 2017
media-tree git hash:ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0
media_build git hash:   a03e89634b47f570039c7d6931cd751777d4bba1
v4l-utils git hash: 5a67da05fded64b5f678033c16196799e134c62c
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.11.0-164

linux-git-arm-at91: 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: WARNINGS
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: WARNINGS
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: 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: ERRORS
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: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: WARNINGS
linux-4.9.26-i686: WARNINGS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: 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
apps: WARNINGS
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH] dib0090: fix duplicated code for different branches

[Gustavo A. R. Silva]

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1226795
Signed-off-by: Gustavo A. R. Silva 
---
This code was tested by compilation only.

 drivers/media/dvb-frontends/dib0090.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib0090.c 
b/drivers/media/dvb-frontends/dib0090.c
index ae53a19..d9d730d 100644
--- a/drivers/media/dvb-frontends/dib0090.c
+++ b/drivers/media/dvb-frontends/dib0090.c
@@ -2435,14 +2435,7 @@ static int dib0090_tune(struct dvb_frontend *fe)
Den = 1;
 
if (Rest > 0) {
-   if (state->config->analog_output)
-   lo6 |= (1 << 2) | 2;
-   else {
-   if (state->identity.in_soc)
-   lo6 |= (1 << 2) | 2;
-   else
-   lo6 |= (1 << 2) | 2;
-   }
+   lo6 |= (1 << 2) | 2;
Den = 255;
}
dib0090_write_reg(state, 0x15, (u16) FBDiv);
-- 
2.5.0

Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"

[Laurent Pinchart]

Hi Javier,

(Resent to your new e-mail address)

Thank you for the patch.

On Tuesday 13 Dec 2016 12:39:19 Javier Martinez Canillas wrote:
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> 
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
> 
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Javier Martinez Canillas 

Acked-by: Laurent Pinchart 

> ---
> 
> Hello Laurent,
> 
> I've tested this patch on top of media/master on my IGEPv2 + tvp5150
> with the following:
> 
> $ media-ctl -r -l '"tvp5150 1-005c":1->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP
> CCDC":1->"OMAP3 ISP CCDC output":0[1]' $ media-ctl -v --set-format '"OMAP3
> ISP CCDC":0 [UYVY2X8 720x240 field:alternate]' $ media-ctl -v --set-format
> '"OMAP3 ISP CCDC":1 [UYVY2X8 720x240 field:interlaced-tb]' $ yavta -f UYVY
> -s 720x480 -n 1 --field interlaced-tb --capture=1 -F /dev/video2 $
> raw2rgbpnm -f UYVY -s 720x480 frame-00.bin frame.pnm
> 
> I've also tested the other composite input with the following change:
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 5fe5faefe212..973be68ff78c 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1371,7 +1371,7 @@ static int tvp5150_probe(struct i2c_client *c,
> return res;
> 
> core->norm = V4L2_STD_ALL;  /* Default is autodetect */
> -   core->input = TVP5150_COMPOSITE1;
> +   core->input = TVP5150_COMPOSITE0;
> core->enable = true;
> 
> v4l2_ctrl_handler_init(>hdl, 5);
> 
> But as mentioned, it also worked for me without the revert so please let
> me know if the driver works with your omap3 board.
> 
> Best regards,
> Javier
> 
>  drivers/media/i2c/tvp5150.c | 142
>  1 file changed, 142
> deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 48646a7f3fb0..5fe5faefe212 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -42,8 +42,6 @@ struct tvp5150 {
>   struct v4l2_subdev sd;
>  #ifdef CONFIG_MEDIA_CONTROLLER
>   struct media_pad pads[DEMOD_NUM_PADS];
> - struct media_entity input_ent[TVP5150_INPUT_NUM];
> - struct media_pad input_pad[TVP5150_INPUT_NUM];
>  #endif
>   struct v4l2_ctrl_handler hdl;
>   struct v4l2_rect rect;
> @@ -1018,40 +1016,6 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev
> *sd, }
> 
>  /**
> ** -  Media entity ops
> -
> ***
> */ -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -static int tvp5150_link_setup(struct media_entity *entity,
> -   const struct media_pad *local,
> -   const struct media_pad *remote, u32 flags)
> -{
> - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> - struct tvp5150 *decoder = to_tvp5150(sd);
> - int i;
> -
> - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> - if (remote->entity == >input_ent[i])
> - break;
> - }
> -
> - /* Do nothing for entities that are not input connectors */
> - if (i == TVP5150_INPUT_NUM)
> - return 0;
> -
> - decoder->input = i;
> -
> - tvp5150_selmux(sd);
> -
> - return 0;
> -}
> -
> -static const struct media_entity_operations tvp5150_sd_media_ops = {
> - .link_setup = tvp5150_link_setup,
> -};
> -#endif
> -
> -/**
> ** I2C Command
>  
> ***
> */
> 
> @@ -1188,42 +1152,6 @@ static int tvp5150_g_tuner(struct v4l2_subdev *sd,
> struct v4l2_tuner *vt) return 0;
>  }
> 
> -static int tvp5150_registered(struct v4l2_subdev *sd)
> -{
> -#ifdef CONFIG_MEDIA_CONTROLLER
> - struct tvp5150 *decoder = to_tvp5150(sd);
> - int ret = 0;
> - int i;
> -
> - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> - struct media_entity *input = >input_ent[i];
> - struct media_pad *pad = >input_pad[i];
> -
> - if (!input->name)
> - continue;
> -
> - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE;

Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Tuesday 13 Dec 2016 12:39:19 Javier Martinez Canillas wrote:
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> 
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
> 
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
> 
> Suggested-by: Laurent Pinchart 
> Signed-off-by: Javier Martinez Canillas 

Acked-by: Laurent Pinchart 

> ---
> 
> Hello Laurent,
> 
> I've tested this patch on top of media/master on my IGEPv2 + tvp5150
> with the following:
> 
> $ media-ctl -r -l '"tvp5150 1-005c":1->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP
> CCDC":1->"OMAP3 ISP CCDC output":0[1]' $ media-ctl -v --set-format '"OMAP3
> ISP CCDC":0 [UYVY2X8 720x240 field:alternate]' $ media-ctl -v --set-format
> '"OMAP3 ISP CCDC":1 [UYVY2X8 720x240 field:interlaced-tb]' $ yavta -f UYVY
> -s 720x480 -n 1 --field interlaced-tb --capture=1 -F /dev/video2 $
> raw2rgbpnm -f UYVY -s 720x480 frame-00.bin frame.pnm
> 
> I've also tested the other composite input with the following change:
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 5fe5faefe212..973be68ff78c 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1371,7 +1371,7 @@ static int tvp5150_probe(struct i2c_client *c,
> return res;
> 
> core->norm = V4L2_STD_ALL;  /* Default is autodetect */
> -   core->input = TVP5150_COMPOSITE1;
> +   core->input = TVP5150_COMPOSITE0;
> core->enable = true;
> 
> v4l2_ctrl_handler_init(>hdl, 5);
> 
> But as mentioned, it also worked for me without the revert so please let
> me know if the driver works with your omap3 board.
> 
> Best regards,
> Javier
> 
>  drivers/media/i2c/tvp5150.c | 142
>  1 file changed, 142
> deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 48646a7f3fb0..5fe5faefe212 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -42,8 +42,6 @@ struct tvp5150 {
>   struct v4l2_subdev sd;
>  #ifdef CONFIG_MEDIA_CONTROLLER
>   struct media_pad pads[DEMOD_NUM_PADS];
> - struct media_entity input_ent[TVP5150_INPUT_NUM];
> - struct media_pad input_pad[TVP5150_INPUT_NUM];
>  #endif
>   struct v4l2_ctrl_handler hdl;
>   struct v4l2_rect rect;
> @@ -1018,40 +1016,6 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev
> *sd, }
> 
>  /**
> ** -  Media entity ops
> -
> ***
> */ -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -static int tvp5150_link_setup(struct media_entity *entity,
> -   const struct media_pad *local,
> -   const struct media_pad *remote, u32 flags)
> -{
> - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> - struct tvp5150 *decoder = to_tvp5150(sd);
> - int i;
> -
> - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> - if (remote->entity == >input_ent[i])
> - break;
> - }
> -
> - /* Do nothing for entities that are not input connectors */
> - if (i == TVP5150_INPUT_NUM)
> - return 0;
> -
> - decoder->input = i;
> -
> - tvp5150_selmux(sd);
> -
> - return 0;
> -}
> -
> -static const struct media_entity_operations tvp5150_sd_media_ops = {
> - .link_setup = tvp5150_link_setup,
> -};
> -#endif
> -
> -/**
> ** I2C Command
>  
> ***
> */
> 
> @@ -1188,42 +1152,6 @@ static int tvp5150_g_tuner(struct v4l2_subdev *sd,
> struct v4l2_tuner *vt) return 0;
>  }
> 
> -static int tvp5150_registered(struct v4l2_subdev *sd)
> -{
> -#ifdef CONFIG_MEDIA_CONTROLLER
> - struct tvp5150 *decoder = to_tvp5150(sd);
> - int ret = 0;
> - int i;
> -
> - for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> - struct media_entity *input = >input_ent[i];
> - struct media_pad *pad = >input_pad[i];
> -
> - if (!input->name)
> - continue;
> -
> - decoder->input_pad[i].flags = MEDIA_PAD_FL_SOURCE;
> -
> - ret = 

[PATCH] media: venus: fix duplicated code for different branches

[Gustavo A. R. Silva]

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1415317
Signed-off-by: Gustavo A. R. Silva 
---
This code was reported by Coverity and it was tested by compilation only.
Please, verify if this is an actual bug.

 drivers/media/platform/qcom/venus/helpers.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c 
b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..8a5c467 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
-   if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-   else
-   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-
+   v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }
 
-- 
2.5.0

[PATCH v1] media: ov5670: Limit vblank to permissible range

[Chiranjeevi Rapolu]

Previously, vblank range given to user was too big, falling outside
of permissible range for a given resolution. Sometimes, too low vblank
resulted in errors.

Now, limit vblank to only permissible range for a given resolution.
This change limits lower-bounds of vblank, doesn't affect upper bounds.

Signed-off-by: Chiranjeevi Rapolu 
---
 drivers/media/i2c/ov5670.c | 40 ++--
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 8d8e16c..ddb7009 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -33,7 +33,6 @@
 #define OV5670_REG_VTS 0x380e
 #define OV5670_VTS_30FPS   0x0808 /* default for 30 fps */
 #define OV5670_VTS_MAX 0x
-#define OV5670_VBLANK_MIN  56
 
 /* horizontal-timings from sensor */
 #define OV5670_REG_HTS 0x380c
@@ -94,8 +93,11 @@ struct ov5670_mode {
/* Frame height in pixels */
u32 height;
 
-   /* Vertical timining size */
-   u32 vts;
+   /* Default vertical timining size */
+   u32 vts_def;
+
+   /* Min vertical timining size */
+   u32 vts_min;
 
/* Link frequency needed for this resolution */
u32 link_freq_index;
@@ -1731,7 +1733,8 @@ struct ov5670_mode {
{
.width = 2592,
.height = 1944,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = OV5670_VTS_30FPS,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
.regs = mode_2592x1944_regs,
@@ -1741,7 +1744,8 @@ struct ov5670_mode {
{
.width = 1296,
.height = 972,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = 996,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1296x972_regs),
.regs = mode_1296x972_regs,
@@ -1751,7 +1755,8 @@ struct ov5670_mode {
{
.width = 648,
.height = 486,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = 516,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_648x486_regs),
.regs = mode_648x486_regs,
@@ -1761,7 +1766,8 @@ struct ov5670_mode {
{
.width = 2560,
.height = 1440,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = OV5670_VTS_30FPS,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2560x1440_regs),
.regs = mode_2560x1440_regs,
@@ -1771,7 +1777,8 @@ struct ov5670_mode {
{
.width = 1280,
.height = 720,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = 1020,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
.regs = mode_1280x720_regs,
@@ -1781,7 +1788,8 @@ struct ov5670_mode {
{
.width = 640,
.height = 360,
-   .vts = OV5670_VTS_30FPS,
+   .vts_def = OV5670_VTS_30FPS,
+   .vts_min = 510,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_640x360_regs),
.regs = mode_640x360_regs,
@@ -2047,6 +2055,7 @@ static int ov5670_init_controls(struct ov5670 *ov5670)
struct v4l2_ctrl_handler *ctrl_hdlr;
s64 vblank_max;
s64 vblank_def;
+   s64 vblank_min;
s64 exposure_max;
int ret;
 
@@ -2071,9 +2080,10 @@ static int ov5670_init_controls(struct ov5670 *ov5670)
   link_freq_configs[0].pixel_rate);
 
vblank_max = OV5670_VTS_MAX - ov5670->cur_mode->height;
-   vblank_def = ov5670->cur_mode->vts - ov5670->cur_mode->height;
+   vblank_def = ov5670->cur_mode->vts_def - ov5670->cur_mode->height;
+   vblank_min = ov5670->cur_mode->vts_min - ov5670->cur_mode->height;
ov5670->vblank = v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops,
-  V4L2_CID_VBLANK, OV5670_VBLANK_MIN,
+  V4L2_CID_VBLANK, vblank_min,
   vblank_max, 1, vblank_def);
 
ov5670->hblank = v4l2_ctrl_new_std(
@@ -2093,7 +2103,7 @@ static int ov5670_init_controls(struct ov5670 *ov5670)
  OV5670_DGTL_GAIN_STEP, OV5670_DGTL_GAIN_DEFAULT);
 
/* Get min, max, step, default from sensor */
-   exposure_max = ov5670->cur_mode->vts - 8;
+   exposure_max = 

Re: [PATCH 1/1] et8ek8: Decrease stack usage

[Pavel Machek]

On Wed 2017-08-16 10:33:45, Sakari Ailus wrote:
> The et8ek8 driver combines I²C register writes to a single array that it
> passes to i2c_transfer(). The maximum number of writes is 48 at once,
> decrease it to 8 and make more transfers if needed, thus avoiding a
> warning on stack usage.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Pavel: this is just compile tested. Could you test it on N900, please?

(More than 1 et8ek8 device makes static bad idea).

Acked-by: Pavel Machek 
Tested-by: Pavel Machek 

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


signature.asc
Description: Digital signature

Re: [PATCH v7] media: platform: Renesas IMR driver

[Sergei Shtylyov]

Hello!

On 08/17/2017 10:59 AM, Hans Verkuil wrote:


A quick review. I'm concentrating on the mesh ioctl, since that's what sets this
driver apart.


   OK, waiting for the detailed review...


Index: media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
===
--- /dev/null
+++ media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
@@ -0,0 +1,372 @@
+Renesas R-Car Image Renderer (IMR) Driver
+=
+
+This file documents some driver-specific aspects of the IMR driver, such as
+the driver-specific ioctl.


Just drop the part ', such as...'.

Can you add a high-level description of the functionality here? Similar to what
you did in the bindings document.


   Sure, I can.


+
+The ioctl reference
+~~~
+
+See ``include/uapi/linux/rcar_imr.h`` for the data structures used.
+
+VIDIOC_IMR_MESH - Set mapping data
+^^
+
+Argument: ``struct imr_map_desc``
+
+**Description**:
+
+This ioctl sets up the mesh through which the input frames will be transformed
+into the output frames. The mesh can be strictly rectangular (when
+``IMR_MAP_MESH`` bit is set in ``imr_map_desc::type``) or arbitrary (when that
+bit is not set).


Wouldn't something like 'IMR_MAP_RECTANGULAR' be much more descriptive than 
_MESH?
There is nothing in the name _MESH to indicate that this switches the data to
rectangles.


   Well, in my Russian mind, mesh consists of the rectangles. :-)


+
+A rectangular mesh consists of the imr_mesh structure followed by M*N vertex
+objects (where M is ``imr_mesh::rows`` and N is ``imr_mesh::columns``).
+In case either ``IMR_MAP_AUTOSG`` or ``IMR_MAP_AUTODG`` (not both) bits are
+set in ``imr_map_desc::type``, ``imr_mesh::{x|y}0`` specify the coordinates
+of the top left corner of the auto-generated mesh and ``imr_mesh::d{x|y}``
+specify the mesh's X/Y steps.


So if the auto bits are set, then there are no vertex objects?


   No, there are no source or destination (not both) coordinate structures 
present in the vertex object; at least one of them must always be there.



Since it's auto generated by the hardware?


   Yes.


I believe we discussed in the past whether 'type' should be split in a 'type'
and 'flags' field.


   In this version, I still tried Cogent's original userland working, so the 
structures were left intact (aside of renaming).



+
+An arbitrary mesh consists of the imr_vbo structure followed by N triangle
+objects (where N is ``imr_vbo::num``), consisting of 3 vertex objects each.
+Setting ``IMR_MAP_AUTODG`` and ``IMR_MAP_AUTOSG`` bits in
+``imr_map_desc::type``) isn't allowed for this type of mesh.
+
+The vertex object has a complex structure depending on some of the bits in
+``imr_map_desc::type``:
+
+    ==  ==  
===
+IMR_MAP_CLCE  IMR_MAP_LUCE  IMR_MAP_AUTODG  IMR_MAP_AUTOSG  Vertex structure 
variant


You should explain the meaning of these bits in this section. I.e., what does
CLCE or AUTODG stand for?


   I think I have explained the IRM_MAPO_AUTO* bits above.


+    ==  ==  
===
+\   ``imr_full_coord``
+\   X   ``imr_dst_coord``
+\   X   ``imr_src_coord``
+\ X 
``imr_full_coord_any_correct``
+\ X X   
``imr_auto_coord_any_correct``
+\ X X   
``imr_auto_coord_any_crrect``


crrect -> correct


+X   
``imr_full_coord_any_correct``
+X   X   
``imr_auto_coord_any_correct``
+X   X   
``imr_auto_coord_any_correct``
+X X 
``imr_full_coord_both_correct``
+X X X   
``imr_auto_coord_both_correct``
+X X X   
``imr_auto_coord_both_correct``
+    ==  ==  
===
+
+The luma correction is calculated according to the following formula (where
+``Y`` is the luma value after texture mapping, ``Y'`` is the luma value after
+luma correction, ``lscal`` and ``lofst`` are the luma correction scale and
+offset taken from ``struct imr_luma_correct``, ``YLDPO`` is a luma correction
+scale decimal point position specified by ``IMR_MAP_YLDPO(n)``): ::
+
+   Y' = ((Y * lscal) >> YLDPO) + lofst
+
+The chroma correction is calculated according to the following formula (where
+``U/V`` are the chroma values after 

[PATCH] [media] mx2_emmaprp: Check for platform_get_irq() error

[Fabio Estevam]

From: Fabio Estevam 

platform_get_irq() may fail, so we should better check its return
value and propagate it in the case of error.

Signed-off-by: Fabio Estevam 
---
 drivers/media/platform/mx2_emmaprp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/mx2_emmaprp.c 
b/drivers/media/platform/mx2_emmaprp.c
index 03e47e0..f90eaa0 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -942,6 +942,8 @@ static int emmaprp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pcdev);
 
irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
ret = devm_request_irq(>dev, irq, emmaprp_irq, 0,
   dev_name(>dev), pcdev);
if (ret)
-- 
2.7.4

[GIT PULL v2 for 4.14] Omap3isp CCP2 support

[Sakari Ailus]

Hi Mauro,

These patches add functional CCP2 support for the omap3isp, as needed for
the Nokia N900.

since v1:

- Take further review comments into account, in particular store the entity
  associated to a given PHY struct (omap3isp) and add a patch cleaning up
  storing information on external sub-devices' bus configuration
  (omap3isp).

Please pull.


The following changes since commit ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0:

  media: ddbridge: split code into multiple files (2017-08-09 12:17:01 -0400)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git ccp2

for you to fetch changes up to d5107162567b192a58efc9ad930dd65cb70c4530:

  omap3isp: Quit using struct v4l2_subdev.host_priv field (2017-08-17 22:18:22 
+0300)


Pavel Machek (2):
  omap3isp: Parse CSI1 configuration from the device tree
  omap3isp: Correctly set IO_OUT_SEL and VP_CLK_POL for CCP2 mode

Sakari Ailus (3):
  omap3isp: Always initialise isp and mutex for csiphy1
  omap3isp: csiphy: Don't assume the CSI receiver is a CSI2 module
  omap3isp: Quit using struct v4l2_subdev.host_priv field

 drivers/media/platform/omap3isp/isp.c   | 134 ++--
 drivers/media/platform/omap3isp/isp.h   |   4 +-
 drivers/media/platform/omap3isp/ispccdc.c   |  16 ++--
 drivers/media/platform/omap3isp/ispccp2.c   |  12 ++-
 drivers/media/platform/omap3isp/ispcsi2.c   |   6 +-
 drivers/media/platform/omap3isp/ispcsiphy.c |  50 +--
 drivers/media/platform/omap3isp/ispcsiphy.h |   6 +-
 drivers/media/platform/omap3isp/ispreg.h|   4 +
 drivers/media/platform/omap3isp/omap3isp.h  |   1 +
 9 files changed, 139 insertions(+), 94 deletions(-)

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

Re: [PATCH v2] media: isl6421: add checks for current overflow

[Jemma Denson]

On 16/08/17 10:42, Mauro Carvalho Chehab wrote:


I've just tested both your v2 patch and changes I'm suggesting above; both work
fine on my setup. Do you want me to send a v3?

Yeah, sure! I'm currently in travel, returning only on Friday, and I don't
have the hardware to test. So, if you can send it, I'd appreciate :-)

Cheers,
Mauro


Ok, just sent. The if statements ended up being a bit complicated, but I added 
checking
if the DCL bit was being overridden (it is by several cards under cx88), only 
pausing
for a second if DCL was in use as the datasheet suggested that's only done in 
that mode,
and also skipped checking overflow if the device was set to off.

The latter should cover overflow somehow being picked up during attach and 
causing the
attach to fail. Unlikely to happen but we shouldn't fail on what could be a 
transient
issue.

Jemma.

[PATCH v3] media: isl6421: add checks for current overflow

[Jemma Denson]

This Kaffeine's BZ:
https://bugs.kde.org/show_bug.cgi?id=374693

affects SkyStar S2 PCI DVB-S/S2 rev 3.3 device. It could be due to
a Kernel bug.

While checking the Isil 6421, comparing with its manual, available at:

http://www.intersil.com/content/dam/Intersil/documents/isl6/isl6421a.pdf

It was noticed that, if the output load is highly capacitive, a different 
approach
is recomended when energizing the LNBf.

Also, it is possible to detect if a current overload is happening, by checking 
an
special flag.

Add support for it.

Tested on Skystar S2. Changes respect override_or option so should still work 
fine
on cx88 based cards which disable dynamic current limit.

Signed-off-by: Mauro Carvalho Chehab 
Signed-off-by: Jemma Denson 
---
 drivers/media/dvb-frontends/isl6421.c | 77 +--
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/isl6421.c 
b/drivers/media/dvb-frontends/isl6421.c
index 838b42771a05..f58590fe71f5 100644
--- a/drivers/media/dvb-frontends/isl6421.c
+++ b/drivers/media/dvb-frontends/isl6421.c
@@ -38,35 +38,102 @@ struct isl6421 {
u8  override_and;
struct i2c_adapter  *i2c;
u8  i2c_addr;
+   boolis_off;
 };
 
 static int isl6421_set_voltage(struct dvb_frontend *fe,
   enum fe_sec_voltage voltage)
 {
+   int ret;
+   u8 buf;
+   bool is_off;
struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
-   struct i2c_msg msg = {  .addr = isl6421->i2c_addr, .flags = 0,
-   .buf = >config,
-   .len = sizeof(isl6421->config) };
+   struct i2c_msg msg[2] = {
+   {
+ .addr = isl6421->i2c_addr,
+ .flags = 0,
+ .buf = >config,
+ .len = 1,
+   }, {
+ .addr = isl6421->i2c_addr,
+ .flags = I2C_M_RD,
+ .buf = ,
+ .len = 1,
+   }
+
+   };
 
isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
 
switch(voltage) {
case SEC_VOLTAGE_OFF:
+   is_off = true;
break;
case SEC_VOLTAGE_13:
+   is_off = false;
isl6421->config |= ISL6421_EN1;
break;
case SEC_VOLTAGE_18:
+   is_off = false;
isl6421->config |= (ISL6421_EN1 | ISL6421_VSEL1);
break;
default:
return -EINVAL;
}
 
+   /*
+* If LNBf were not powered on, disable dynamic current limit, as,
+* according with datasheet, highly capacitive load on the output may
+* cause a difficult start-up.
+*/
+   if (isl6421->is_off && !is_off)
+   isl6421->config |= ISL6421_DCL;
+
isl6421->config |= isl6421->override_or;
isl6421->config &= isl6421->override_and;
 
-   return (i2c_transfer(isl6421->i2c, , 1) == 1) ? 0 : -EIO;
+   ret = i2c_transfer(isl6421->i2c, msg, 2);
+   if (ret < 0)
+   return ret;
+   if (ret != 2)
+   return -EIO;
+
+   /* Store off status now incase future commands fail */
+   isl6421->is_off = is_off;
+
+   /* On overflow, the device will try again after 900 ms (typically) */
+   if (!(isl6421->config & ISL6421_DCL) && (buf & ISL6421_OLF1))
+   msleep(1000);
+
+   /* Re-enable dynamic current limit after a certain amount of time */
+   if ((isl6421->config & ISL6421_DCL) &&
+   !(isl6421->override_or & ISL6421_DCL)) {
+   msleep(200);
+   isl6421->config &= ~ISL6421_DCL;
+
+   ret = i2c_transfer(isl6421->i2c, msg, 2);
+   if (ret < 0)
+   return ret;
+   if (ret != 2)
+   return -EIO;
+   }
+
+   /* Check if overload flag is active. If so, disable power */
+   if (!is_off && (buf & ISL6421_OLF1)) {
+   isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
+   ret = i2c_transfer(isl6421->i2c, msg, 1);
+   if (ret < 0)
+   return ret;
+   if (ret != 1)
+   return -EIO;
+   isl6421->is_off = true;
+
+   dev_warn(>i2c->dev,
+"Overload current detected. disabling LNBf power\n");
+   return -EINVAL;
+   }
+
+   return 0;
 }
 
 static int isl6421_enable_high_lnb_voltage(struct dvb_frontend *fe, long arg)
@@ -148,6 +215,8 @@ struct dvb_frontend *isl6421_attach(struct dvb_frontend 
*fe, struct i2c_adapter
return NULL;
}
 
+   isl6421->is_off = true;
+
/* install release callback */
fe->ops.release_sec = isl6421_release;
 
-- 

Re: [PATCH v2.5 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday 17 Aug 2017 16:27:38 Sakari Ailus wrote:
> struct v4l2_subdev.host_priv is intended to be used by another driver. This
> is hardly good design but back in the days of platform data was a quick
> hack to get things done.
> 
> As the sub-device specific bus information can be stored to the ISP driver
> specific struct allocated along with v4l2_async_subdev, keep the
> information there and only there.
> 
> Signed-off-by: Sakari Ailus 

Tested-by: Laurent Pinchart 
Reviewed-by: Laurent Pinchart 

> ---
> since v2.4:
> 
> - Use temporary variables to avoid somewhat lengthy statements.
> 
>  drivers/media/platform/omap3isp/isp.c   | 29 ++
>  drivers/media/platform/omap3isp/isp.h   |  4 +++-
>  drivers/media/platform/omap3isp/ispccdc.c   | 16 +---
>  drivers/media/platform/omap3isp/ispccp2.c   |  3 ++-
>  drivers/media/platform/omap3isp/ispcsi2.c   |  2 +-
>  drivers/media/platform/omap3isp/ispcsiphy.c |  8 +++-
>  6 files changed, 25 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6cb1f0495804..c3014c82d64d
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
>   return -EINVAL;
>  }
> 
> -static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> -  struct v4l2_subdev *subdev,
> -  struct v4l2_async_subdev *asd)
> -{
> - struct isp_async_subdev *isd =
> - container_of(asd, struct isp_async_subdev, asd);
> -
> - isd->sd = subdev;
> - isd->sd->host_priv = >bus;
> -
> - return 0;
> -}
> -
>  static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
>  {
>   struct isp_device *isp = container_of(async, struct isp_device,
> notifier);
>   struct v4l2_device *v4l2_dev = >v4l2_dev;
>   struct v4l2_subdev *sd;
> - struct isp_bus_cfg *bus;
>   int ret;
> 
>   ret = media_entity_enum_init(>crashed, >media_dev);
> @@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async) return ret;
> 
>   list_for_each_entry(sd, _dev->subdevs, list) {
> - /* Only try to link entities whose interface was set on bound 
*/
> - if (sd->host_priv) {
> - bus = (struct isp_bus_cfg *)sd->host_priv;
> - ret = isp_link_entity(isp, >entity, bus-
>interface);
> - if (ret < 0)
> - return ret;
> - }
> + if (!sd->asd)
> + continue;
> +
> + ret = isp_link_entity(isp, >entity,
> +   v4l2_subdev_to_bus_cfg(sd)->interface);
> + if (ret < 0)
> + return ret;
>   }
> 
>   ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
> @@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
>   if (ret < 0)
>   goto error_register_entities;
> 
> - isp->notifier.bound = isp_subdev_notifier_bound;
>   isp->notifier.complete = isp_subdev_notifier_complete;
> 
>   ret = v4l2_async_notifier_register(>v4l2_dev, >notifier);
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index 2f2ae609c548..e528df6efc09
> 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -226,11 +226,13 @@ struct isp_device {
>  };
> 
>  struct isp_async_subdev {
> - struct v4l2_subdev *sd;
>   struct isp_bus_cfg bus;
>   struct v4l2_async_subdev asd;
>  };
> 
> +#define v4l2_subdev_to_bus_cfg(sd) \
> + (_of((sd)->asd, struct isp_async_subdev, asd)->bus)
> +
>  #define v4l2_dev_to_isp_device(dev) \
>   container_of(dev, struct isp_device, v4l2_dev)
> 
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 4947876cfadf..b66276ab5765 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1139,8 +1139,10 @@ static void ccdc_configure(struct isp_ccdc_device
> *ccdc) pad = media_entity_remote_pad(>pads[CCDC_PAD_SINK]);
>   sensor = media_entity_to_v4l2_subdev(pad->entity);
>   if (ccdc->input == CCDC_INPUT_PARALLEL) {
> - parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
> - ->bus.parallel;
> + struct v4l2_subdev *sd =
> + to_isp_pipeline(>subdev.entity)->external;
> +
> + parcfg = _subdev_to_bus_cfg(sd)->bus.parallel;
>   ccdc->bt656 = parcfg->bt656;
>   }
> 
> @@ 

Re: [PATCH v3 1/3] staging: greybus: light: fix memory leak in v4l2 register

[Greg Kroah-Hartman]

On Thu, Aug 10, 2017 at 06:49:45PM +0300, Sakari Ailus wrote:
> From: Rui Miguel Silva 
> 
> We are allocating memory for the v4l2 flash configuration structure and
> leak it in the normal path. Just use the stack for this as we do not
> use it outside of this function.
> 
> Also use IS_ERR() instead of IS_ERR_OR_NULL() to check return value from
> v4l2_flash_init() for it never returns NULL.
> 
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Reported-by: Sakari Ailus 
> Signed-off-by: Rui Miguel Silva 
> Reviewed-by: Viresh Kumar 
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 
> ---
>  drivers/staging/greybus/light.c | 29 +
>  1 file changed, 9 insertions(+), 20 deletions(-)

Does not apply to my tree :(

Re: [PATCH v3.2 2/3] v4l2-flash-led-class: Create separate sub-devices for indicators

[Greg Kroah-Hartman]

On Tue, Aug 15, 2017 at 02:28:11PM +0300, Sakari Ailus wrote:
> The V4L2 flash interface allows controlling multiple LEDs through a single
> sub-devices if, and only if, these LEDs are of different types. This
> approach scales badly for flash controllers that drive multiple flash LEDs
> or for LED specific associations. Essentially, the original assumption of a
> LED driver chip that drives a single flash LED and an indicator LED is no
> longer valid.
> 
> Address the matter by registering one sub-device per LED.
> 
> Signed-off-by: Sakari Ailus 
> Reviewed-by: Jacek Anaszewski 
> Acked-by: Pavel Machek 
> Reviewed-by: Rui Miguel Silva  (for greybus/light)
> Acked-by: Hans Verkuil 
> ---
> since v3.2:

"3.2"?

Isn't that just "v5"?  Please don't be cute with version numbering, we
have a hard time telling what is going on, make it obvious...

thanks,

greg k-h

Re: [PATCH v2 5/8] v4l: vsp1: Refactor display list configure operations

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:28 Kieran Bingham wrote:
> 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|  43 +--
>  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, 359 insertions(+), 371 deletions(-)

[snip]

> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> b/drivers/media/platform/vsp1/vsp1_clu.c index 175717018e11..5f65ce3ad97f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c
> @@ -213,37 +213,37 @@ 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, only verify it
> +  * at setup time and store the information internally for future
> +  * runtime configuration calls.
> +  */

I know you're just moving the comment around, but let's fix it at the same 
time. There's no verification here (and no "setup time" either). I'd write it 
as

/*
 * 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;
> +}

[snip]

> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
> b/drivers/media/platform/vsp1/vsp1_entity.h index
> 408602ebeb97..2f33e343ccc6 100644
> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> +++ b/drivers/media/platform/vsp1/vsp1_entity.h

[snip]

> @@ -80,8 +68,10 @@ struct vsp1_route {
>  /**
>   * struct vsp1_entity_operations - Entity operations
>   * @destroy: Destroy the entity.
> - * @configure:   Setup the hardware based on the entity state
> (pipeline, formats,
> - *   selection rectangles, ...)
> + * @prepare: Setup the initial hardware parameters for the stream
> (pipeline,
> + *   formats)
> + * @configure:   Configure the runtime parameters for each partition
> (rectangles,
> + *   buffer addresses, ...)

Now moving to the bikeshedding territory, I'm not sure if prepare and 
configure are the best names for those operations. I'd like to also point out 
that we could go one step further by caching the partition-related parameters 
too, in which case we would need a third operation (or possibly passing the 
partition number to the prepare operation). While I won't mind if you 
implement this now, the issue could also be addressed later, but I'd like the 
operations to already support that use case to avoid yet another painful 
rename patch.

>   * @max_width:   Return the max supported width of data that the entity
> can
>   *   process in a single operation.
>   * @partition:   Process the partition construction based on this
> entity's

[snip]

The rest of the patch looks good to me.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 7/8] v4l: vsp1: Move video configuration to a cached dlb

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:30 Kieran Bingham wrote:
> 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 
> ---
> 
> 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->fragments shows our constant configuration and LUTs.
> 
>   These two are LUT/CLU:
>  * dl->fragments[x]->num_entries 256 / max 256
>  * dl->fragments[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->fragments[x]->num_entries 15 / max 128
>   dl->fragments[x]->num_entries 16 / max 128
>   dl->fragments[x]->num_entries 17 / max 128
>   dl->fragments[x]->num_entries 18 / max 128
>   dl->fragments[x]->num_entries 20 / max 128
>   dl->fragments[x]->num_entries 21 / max 128
>   dl->fragments[x]->num_entries 256 / max 256
>   dl->fragments[x]->num_entries 31 / max 128
>   dl->fragments[x]->num_entries 32 / max 128
>   dl->fragments[x]->num_entries 39 / max 128
>   dl->fragments[x]->num_entries 40 / max 128
>   dl->fragments[x]->num_entries 47 / max 128
>   dl->fragments[x]->num_entries 48 / max 128
>   dl->fragments[x]->num_entries 4914 / max 4914
>   dl->fragments[x]->num_entries 55 / max 128
>   dl->fragments[x]->num_entries 56 / max 128
>   dl->fragments[x]->num_entries 63 / max 128
>   dl->fragments[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;

It shouldn't make a difference in practice, but I think it would be more 
logical to set the configured field to false after the hardware has been 
reset. I'd move this to the resume handler and update the comment to "The 
hardware might have been reset during suspend and need a full 
reconfiguration". 

>   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;
>   

Re: [PATCH v2 6/8] v4l: vsp1: Adapt entities to configure into a body

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:29 Kieran Bingham wrote:
> Currently the entities store their configurations into a display list.
> Adapt this such that the code can be configured into a body fragment
> 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_body() is provided to access the internal body0
> from the display list.
> 
> Signed-off-by: Kieran Bingham 
> ---
>  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 |  2 +-
>  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, 168 insertions(+), 153 deletions(-)

This is quite intrusive, and it bothers me slightly that we need to pass both 
the DL and the DLB to the configure function in order to add fragments to the 
DL in the CLU and LUT modules. Wouldn't it be simpler to add a pointer to the 
current body in the DL structure, and modify vsp1_dl_list_write() to write to 
the current fragment ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 8/8] v4l: vsp1: Reduce display list body size

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:31 Kieran Bingham wrote:
> 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.

Didn't you mention in patch 7/8 that one of the fragments uses exactly 64 
entries ? Which one is it, and is there a risk it could use more ? 

> 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 176a258146ac..b3f5eb2f9a4f
> 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_ENTRIES  256
> +#define VSP1_DL_NUM_ENTRIES  64
> 
>  #define VSP1_DLH_INT_ENABLE  (1 << 1)
>  #define VSP1_DLH_AUTO_START  (1 << 0)

-- 
Regards,

Laurent Pinchart

Re: [PATCHv3 1/4] dt-bindings: document the tegra CEC bindings

[Rob Herring]

On Thu, Aug 10, 2017 at 10:31:22AM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This documents the binding for the Tegra CEC module.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  .../devicetree/bindings/media/tegra-cec.txt| 27 
> ++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/tegra-cec.txt

Acked-by: Rob Herring 

Re: [Patch v5 12/12] Documention: v4l: Documentation for HEVC CIDs

[Stanimir Varbanov]

Hi,

On 07/17/2017 02:18 PM, Smitha T Murthy wrote:
> On Fri, 2017-07-07 at 17:59 +0300, Stanimir Varbanov wrote:
>> Hi,
>>
>> On 06/19/2017 08:10 AM, Smitha T Murthy wrote:
>>> Added V4l2 controls for HEVC encoder
>>>
>>> Signed-off-by: Smitha T Murthy 
>>> ---
>>>  Documentation/media/uapi/v4l/extended-controls.rst | 364 
>>> +
>>>  1 file changed, 364 insertions(+)
>>>



>>
>>> +MFC 10.10 MPEG Controls
>>> +---
>>> +
>>> +The following MPEG class controls deal with MPEG decoding and encoding
>>> +settings that are specific to the Multi Format Codec 10.10 device present
>>> +in the S5P and Exynos family of SoCs by Samsung.
>>> +
>>> +
>>> +.. _mfc1010-control-id:
>>> +
>>> +MFC 10.10 Control IDs
>>> +^
>>> +
>>> +``V4L2_CID_MPEG_MFC10_VIDEO_HEVC_REF_NUMBER_FOR_PFRAMES (integer)``
>>> +Selects number of P reference pictures required for HEVC encoder.
>>> +P-Frame can use 1 or 2 frames for reference.
>>> +
>>> +``V4L2_CID_MPEG_MFC10_VIDEO_HEVC_PREPEND_SPSPPS_TO_IDR (integer)``
>>> +Indicates whether to generate SPS and PPS at every IDR. Setting it to 0
>>> +disables generating SPS and PPS at every IDR. Setting it to one enables
>>> +generating SPS and PPS at every IDR.
>>> +
>>
>> I'm not sure those two should be driver specific, have to check does
>> venus driver has similar controls.
>>
> Yes please check and let me know if you have similar controls, I will
> move it out.
The venus encoder also has such a control so you can move it out of MFC
specific controls.

Also I think this control should be valid for every codec which supports
IDR, i.e. H264, so I think you could drop _HEVC_from the control name.

Do you plan to resend the patchset soon so that it could be applied for
4.14? If you haven't time let me know I can help with the generic HEVC
part of the patchset.

-- 
regards,
Stan

(no subject)

[Barr. Richard Williams]

--
I'd like you to be in custody of my late client's fortune.
My client died along with his family including his next-of-kin
The funds shall be used for investment under your management

Do reply for details.
Regards
Richard Williams
Email:rich19willi...@gmail.com

[RFC PATCH v4 0/6] i2c: document DMA handling and add helpers for it

[Wolfram Sang]

So, after revisiting old mail threads, taking part in a similar discussion on
the USB list, and implementing a not-convincing solution before, here is what I
cooked up to document and ease DMA handling for I2C within Linux. Please have a
look at the documentation introduced in patch 3 for details.

While the previous versions tried to magically apply bounce buffers when
needed, it became clear that detecting DMA safe buffers is too fragile. This
approach is now opt-in, a DMA_SAFE flag needs to be set on an i2c_msg. The
outcome so far is very convincing IMO. The core additions are simple and easy
to understand (makes me even think of inlining them again?). The driver changes
for the Renesas IP cores became easier to understand, too. While only a tad for
the i2c-sh_mobile driver, the situation became a LOT better for the i2c-rcar
driver. No more DMA disabling for the whole transfer in case of unsafe buffers,
we are back to per-msg handling. And the code fix is now an easy to understand
one line change. Yay!

Of course, we must now whitelist DMA safe buffers. An example for I2C_RDWR case
is in this series. It makes the i2ctransfer utility have DMA_SAFE buffers,
which is nice for testing as i2cdump will (currently) not use DMA_SAFE buffers.
My plan is to add two new calls: i2c_master_{send|receive}_dma_safe which can
be used if DMA_SAFE buffers are provided. So, drivers can simply switch to
them. Also, the buffers used within i2c_smbus_xfer_emulated() need to be
converted to be DMA_SAFE which will cover a huge bunch of use cases. The rest
is then updating drivers which can be done when needed.

As these conversions are not done yet, this patch series has RFC status. But I
already would like to get opinions on this approach, so I'll cc mailing lists
of the heavier I2C users. Please let me know what you think.

All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W).

The branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
renesas/topic/i2c-core-dma-rfc-v4

And big kudos to Renesas Electronics for funding this work, thank you very much!

Regards,

   Wolfram

Changes since v3:
* completely redesigned

Wolfram Sang (6):
  i2c: add a message flag for DMA safe buffers
  i2c: add helpers to ease DMA handling
  i2c: add docs to clarify DMA handling
  i2c: sh_mobile: use helper to decide if DMA is useful
  i2c: rcar: skip DMA if buffer is not safe
  i2c: dev: mark RDWR buffers as DMA_SAFE

 Documentation/i2c/DMA-considerations | 50 
 drivers/i2c/busses/i2c-rcar.c|  2 +-
 drivers/i2c/busses/i2c-sh_mobile.c   |  8 --
 drivers/i2c/i2c-core-base.c  | 45 
 drivers/i2c/i2c-dev.c|  2 ++
 include/linux/i2c.h  |  3 +++
 include/uapi/linux/i2c.h |  3 +++
 7 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/i2c/DMA-considerations

-- 
2.11.0

[RFC PATCH v4 4/6] i2c: sh_mobile: use helper to decide if DMA is useful

[Wolfram Sang]

This ensures that we fall back to PIO if the message length is too small
for DMA being useful. Otherwise, we use DMA. A bounce buffer might be
applied by the helper if the original message buffer is not DMA safe.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-sh_mobile.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
b/drivers/i2c/busses/i2c-sh_mobile.c
index 2e097d97d258bc..5efdb7becd83d6 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -145,6 +145,7 @@ struct sh_mobile_i2c_data {
struct dma_chan *dma_rx;
struct scatterlist sg;
enum dma_data_direction dma_direction;
+   u8 *dma_buf;
 };
 
 struct sh_mobile_dt_config {
@@ -548,6 +549,8 @@ static void sh_mobile_i2c_dma_callback(void *data)
pd->pos = pd->msg->len;
pd->stop_after_dma = true;
 
+   i2c_release_dma_safe_msg_buf(pd->msg, pd->dma_buf);
+
iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
 }
 
@@ -608,7 +611,7 @@ static void sh_mobile_i2c_xfer_dma(struct 
sh_mobile_i2c_data *pd)
if (IS_ERR(chan))
return;
 
-   dma_addr = dma_map_single(chan->device->dev, pd->msg->buf, 
pd->msg->len, dir);
+   dma_addr = dma_map_single(chan->device->dev, pd->dma_buf, pd->msg->len, 
dir);
if (dma_mapping_error(chan->device->dev, dma_addr)) {
dev_dbg(pd->dev, "dma map failed, using PIO\n");
return;
@@ -665,7 +668,8 @@ static int start_ch(struct sh_mobile_i2c_data *pd, struct 
i2c_msg *usr_msg,
pd->pos = -1;
pd->sr = 0;
 
-   if (pd->msg->len > 8)
+   pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
+   if (pd->dma_buf)
sh_mobile_i2c_xfer_dma(pd);
 
/* Enable all interrupts to begin with */
-- 
2.11.0

[RFC PATCH v4 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE

[Wolfram Sang]

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 6f638bbc922db4..bbc7aadb4c899d 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client 
*client,
res = PTR_ERR(rdwr_pa[i].buf);
break;
}
+   /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
+   rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
 
/*
 * If the message length is received from the slave (similar
-- 
2.11.0

[RFC PATCH v4 5/6] i2c: rcar: skip DMA if buffer is not safe

[Wolfram Sang]

This HW is prone to races, so it needs to setup new messages in irq
context. That means we can't alloc bounce buffers if a message buffer is
not DMA safe. So, in that case, simply fall back to PIO.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/busses/i2c-rcar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 93c1a54981df08..5654a7142bffec 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -359,7 +359,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
int len;
 
/* Do not use DMA if it's not available or for messages < 8 bytes */
-   if (IS_ERR(chan) || msg->len < 8)
+   if (IS_ERR(chan) || msg->len < 8 || !(msg->flags & I2C_M_DMA_SAFE))
return;
 
if (read) {
-- 
2.11.0

[RFC PATCH v4 1/6] i2c: add a message flag for DMA safe buffers

[Wolfram Sang]

I2C has no requirement that the buffer of a message needs to be DMA
safe. In case it is, it can now be flagged, so drivers wishing to
do DMA can use the buffer directly.

Signed-off-by: Wolfram Sang 
---
 include/uapi/linux/i2c.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index 009e27bb9abe19..1c683cb319e4b7 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -71,6 +71,9 @@ struct i2c_msg {
 #define I2C_M_RD   0x0001  /* read data, from slave to master */
/* I2C_M_RD is guaranteed to be 0x0001! 
*/
 #define I2C_M_TEN  0x0010  /* this is a ten bit chip address */
+#define I2C_M_DMA_SAFE 0x0200  /* the buffer of this message is DMA 
safe */
+   /* makes only sense in kernelspace */
+   /* userspace buffers are copied anyway 
*/
 #define I2C_M_RECV_LEN 0x0400  /* length will be first received byte */
 #define I2C_M_NO_RD_ACK0x0800  /* if 
I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK   0x1000  /* if I2C_FUNC_PROTOCOL_MANGLING */
-- 
2.11.0

[RFC PATCH v4 2/6] i2c: add helpers to ease DMA handling

[Wolfram Sang]

One helper checks if DMA is suitable and optionally creates a bounce
buffer, if not. The other function returns the bounce buffer and makes
sure the data is properly copied back to the message.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-core-base.c | 45 +
 include/linux/i2c.h |  3 +++
 2 files changed, 48 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 12822a4b8f8f09..a104ebc2d05af8 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
 }
 EXPORT_SYMBOL(i2c_put_adapter);
 
+/**
+ * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
+ * @msg: the message to be checked
+ * @threshold: the amount of byte from which using DMA makes sense
+ *
+ * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
+ *
+ *Or a valid pointer to be used with DMA. Note that it can either be
+ *msg->buf or a bounce buffer. After use, release it by calling
+ *i2c_release_dma_safe_msg_buf().
+ *
+ * This function must only be called from process context!
+ */
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
+{
+   if (msg->len < threshold)
+   return NULL;
+
+   if (msg->flags & I2C_M_DMA_SAFE)
+   return msg->buf;
+
+   if (msg->flags & I2C_M_RD)
+   return kzalloc(msg->len, GFP_KERNEL);
+   else
+   return kmemdup(msg->buf, msg->len, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
+
+/**
+ * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
+ * @msg: the message to be synced with
+ * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
+ */
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
+{
+   if (!buf || buf == msg->buf)
+   return;
+
+   if (msg->flags & I2C_M_RD)
+   memcpy(msg->buf, buf, msg->len);
+
+   kfree(buf);
+}
+EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
+
 MODULE_AUTHOR("Simon G. Vogl ");
 MODULE_DESCRIPTION("I2C-Bus main module");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d501d3956f13f0..1e99342f180f45 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
i2c_msg *msg)
return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
 }
 
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
+
 int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
addr);
 /**
  * module_i2c_driver() - Helper macro for registering a modular I2C driver
-- 
2.11.0

[RFC PATCH v4 3/6] i2c: add docs to clarify DMA handling

[Wolfram Sang]

Signed-off-by: Wolfram Sang 
---
 Documentation/i2c/DMA-considerations | 50 
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/i2c/DMA-considerations

diff --git a/Documentation/i2c/DMA-considerations 
b/Documentation/i2c/DMA-considerations
new file mode 100644
index 00..a4b4a107102452
--- /dev/null
+++ b/Documentation/i2c/DMA-considerations
@@ -0,0 +1,50 @@
+Linux I2C and DMA
+-
+
+Given that I2C is a low-speed bus where largely small messages are transferred,
+it is not considered a prime user of DMA access. At this time of writing, only
+10% of I2C bus master drivers have DMA support implemented. And the vast
+majority of transactions are so small that setting up DMA for it will likely
+add more overhead than a plain PIO transfer.
+
+Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
+It does not seem reasonable to apply additional burdens when the feature is so
+rarely used. However, it is recommended to use a DMA-safe buffer if your
+message size is likely applicable for DMA. Most drivers have this threshold
+around 8 bytes (as of today, this is mostly an educated guess, however). For
+any message of 16 byte or larger, it is probably a really good idea.
+
+If you use such a buffer in a i2c_msg, set the I2C_M_DMA_SAFE flag with it.
+Then, the I2C core and drivers know they can safely operate DMA on it. Note
+that setting this flag makes only sense in kernel space. User space data is
+copied into kernel space anyhow. The I2C core makes sure the destination
+buffers in kernel space are always DMA capable.
+
+FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers for 
i2c_smbus_xfer_emulated.
+
+Drivers wishing to implement DMA can use helper functions from the I2C core.
+One gives you a DMA-safe buffer for a given i2c_msg as long as a certain
+threshold is met.
+
+   dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
+
+If a buffer is returned, it either msg->buf for the I2C_M_DMA_SAFE case or a
+bounce buffer. But you don't need to care about that detail. If NULL is
+returned, the threshold was not met or a bounce buffer could not be allocated.
+Fall back to PIO in that case.
+
+In any case, a buffer obtained from above needs to be released. It ensures data
+is copied back to the message and a potentially used bounce buffer is freed.
+
+   i2c_release_dma_safe_msg_buf(msg, dma_buf);
+
+The bounce buffer handling from the core is generic and simple. It will always
+allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
+reusing pre-allocated buffers), you are free to implement your own.
+
+Please also check the in-kernel documentation for details. The i2c-sh_mobile
+driver can be used as a reference example how to use the above helpers.
+
+Final note: If you plan to use DMA with I2C (or with anything else, actually)
+make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can help
+you find various issues which can be complex to debug otherwise.
-- 
2.11.0

Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"

[Laurent Pinchart]

Hello,

On Thursday 17 Aug 2017 15:49:51 Javier Martinez Canillas wrote:
> On Thu, Aug 17, 2017 at 3:05 PM, Philipp Zabel wrote:
> > On Tue, 2016-12-13 at 12:39 -0300, Javier Martinez Canillas wrote:
> >> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> >> added input signals support for the tvp5150, but the approach was found
> >> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> >> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> >> 
> >> This left the driver with an undocumented (and wrong) DT parsing logic,
> >> so lets get rid of this code as well until the input connectors support
> >> is implemented properly.
> >> 
> >> It's a partial revert due other patches added on top of mentioned commit
> >> not allowing the commit to be reverted cleanly anymore. But all the code
> >> related to the DT parsing logic and input entities creation are removed.
> >> 
> >> > Suggested-by: Laurent Pinchart 
> >> > Signed-off-by: Javier Martinez Canillas 
> > 
> > what is the state of this patch? Was it forgotten or was the revert
> > deemed unnecessary?
> 
> I think that was just forgotten. That code still needs to be reverted
> since the DT patch was also reverted.

Yes, I think it was just forgotten.

> Albeit the code is harmless since should be a no-op if a connectors DT
> node isn't found.

-- 
Regards,

Laurent Pinchart

Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"

[Javier Martinez Canillas]

Hello Philipp,

On Thu, Aug 17, 2017 at 3:05 PM, Philipp Zabel  wrote:
> Hi,
>
> On Tue, 2016-12-13 at 12:39 -0300, Javier Martinez Canillas wrote:
>> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
>> added input signals support for the tvp5150, but the approach was found
>> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
>> ("[media] tvp5150: document input connectors DT bindings") was reverted.
>>
>> This left the driver with an undocumented (and wrong) DT parsing logic,
>> so lets get rid of this code as well until the input connectors support
>> is implemented properly.
>>
>> It's a partial revert due other patches added on top of mentioned commit
>> not allowing the commit to be reverted cleanly anymore. But all the code
>> related to the DT parsing logic and input entities creation are removed.
>>
>> > Suggested-by: Laurent Pinchart 
>> > Signed-off-by: Javier Martinez Canillas 
>
> what is the state of this patch? Was it forgotten or was the revert
> deemed unnecessary?
>

I think that was just forgotten. That code still needs to be reverted
since the DT patch was also reverted.

Albeit the code is harmless since should be a no-op if a connectors DT
node isn't found.

> regards
> Philipp
>

Best regards,
Javier

[PATCH 2/3] dw9714: Add Devicetree support

[Sakari Ailus]

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/dw9714.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
index 6a607d7..bcf64ef 100644
--- a/drivers/media/i2c/dw9714.c
+++ b/drivers/media/i2c/dw9714.c
@@ -264,6 +264,12 @@ static const struct i2c_device_id dw9714_id_table[] = {
 
 MODULE_DEVICE_TABLE(i2c, dw9714_id_table);
 
+static const struct of_device_id dw9714_of_table[] = {
+   { .compatible = "dongwoon,dw9714" },
+   { { 0 } }
+};
+MODULE_DEVICE_TABLE(of, dw9714_of_table);
+
 static const struct dev_pm_ops dw9714_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume)
SET_RUNTIME_PM_OPS(dw9714_vcm_suspend, dw9714_vcm_resume, NULL)
@@ -274,6 +280,7 @@ static struct i2c_driver dw9714_i2c_driver = {
.name = DW9714_NAME,
.pm = _pm_ops,
.acpi_match_table = ACPI_PTR(dw9714_acpi_match),
+   .of_match_table = dw9714_of_table,
},
.probe = dw9714_probe,
.remove = dw9714_remove,
-- 
2.7.4

[PATCH 1/3] dt-bindings: Add bindings for Dongwoon DW9714 voice coil

[Sakari Ailus]

Dongwoon DW9714 is a voice coil lens driver.

Also add a vendor prefix for Dongwoon for one did not exist previously.

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt | 9 +
 Documentation/devicetree/bindings/vendor-prefixes.txt   | 1 +
 2 files changed, 10 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt 
b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
new file mode 100644
index 000..b88dcdd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
@@ -0,0 +1,9 @@
+Dongwoon Anatech DW9714 camera voice coil lens driver
+
+DW9174 is a 10-bit DAC with current sink capability. It is intended
+for driving voice coil lenses in camera modules.
+
+Mandatory properties:
+
+- compatible: "dongwoon,dw9714"
+- reg: I²C slave address
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index daf465be..6b6e683 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -88,6 +88,7 @@ dlg   Dialog Semiconductor
 dlink  D-Link Corporation
 dmoData Modul AG
 domintech  Domintech Co., Ltd.
+dongwoon   Dongwoon Anatech
 dptechnics DPTechnics
 draginoDragino Technology Co., Limited
 ea Embedded Artists AB
-- 
2.7.4

[PATCH 3/3] dw9714: Remove ACPI match tables, convert to use probe_new

[Sakari Ailus]

The ACPI match table is empty. Remove it.

Also convert the drive to use probe_new callback in struct i2c_driver.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/dw9714.c | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
index bcf64ef..95af4fc 100644
--- a/drivers/media/i2c/dw9714.c
+++ b/drivers/media/i2c/dw9714.c
@@ -11,7 +11,6 @@
  * GNU General Public License for more details.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -147,8 +146,7 @@ static int dw9714_init_controls(struct dw9714_device 
*dev_vcm)
return hdl->error;
 }
 
-static int dw9714_probe(struct i2c_client *client,
-   const struct i2c_device_id *devid)
+static int dw9714_probe(struct i2c_client *client)
 {
struct dw9714_device *dw9714_dev;
int rval;
@@ -250,18 +248,10 @@ static int  __maybe_unused dw9714_vcm_resume(struct 
device *dev)
return 0;
 }
 
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id dw9714_acpi_match[] = {
-   {},
-};
-MODULE_DEVICE_TABLE(acpi, dw9714_acpi_match);
-#endif
-
 static const struct i2c_device_id dw9714_id_table[] = {
-   {DW9714_NAME, 0},
-   {}
+   { DW9714_NAME, 0 },
+   { { 0 } }
 };
-
 MODULE_DEVICE_TABLE(i2c, dw9714_id_table);
 
 static const struct of_device_id dw9714_of_table[] = {
@@ -279,10 +269,9 @@ static struct i2c_driver dw9714_i2c_driver = {
.driver = {
.name = DW9714_NAME,
.pm = _pm_ops,
-   .acpi_match_table = ACPI_PTR(dw9714_acpi_match),
.of_match_table = dw9714_of_table,
},
-   .probe = dw9714_probe,
+   .probe_new = dw9714_probe,
.remove = dw9714_remove,
.id_table = dw9714_id_table,
 };
-- 
2.7.4

[PATCH 0/3] DW9714 DT support

[Sakari Ailus]

Hi all,

This patchset adds DT bindings as well as DT support for DW9714. The
unused ACPI match table is removed.

Sakari Ailus (3):
  dt-bindings: Add bindings for Dongwoon DW9714 voice coil
  dw9714: Add Devicetree support
  dw9714: Remove ACPI match tables, convert to use probe_new

 .../bindings/media/i2c/dongwoon,dw9714.txt |  9 
 .../devicetree/bindings/vendor-prefixes.txt|  1 +
 drivers/media/i2c/dw9714.c | 26 +-
 3 files changed, 21 insertions(+), 15 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt

-- 
2.7.4

[PATCH v2.5 5/5] omap3isp: Quit using struct v4l2_subdev.host_priv field

[Sakari Ailus]

struct v4l2_subdev.host_priv is intended to be used by another driver. This
is hardly good design but back in the days of platform data was a quick
hack to get things done.

As the sub-device specific bus information can be stored to the ISP driver
specific struct allocated along with v4l2_async_subdev, keep the
information there and only there.

Signed-off-by: Sakari Ailus 
---
since v2.4:

- Use temporary variables to avoid somewhat lengthy statements.

 drivers/media/platform/omap3isp/isp.c   | 29 +++--
 drivers/media/platform/omap3isp/isp.h   |  4 +++-
 drivers/media/platform/omap3isp/ispccdc.c   | 16 +---
 drivers/media/platform/omap3isp/ispccp2.c   |  3 ++-
 drivers/media/platform/omap3isp/ispcsi2.c   |  2 +-
 drivers/media/platform/omap3isp/ispcsiphy.c |  8 +++-
 6 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 6cb1f0495804..c3014c82d64d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2188,26 +2188,12 @@ static int isp_fwnodes_parse(struct device *dev,
return -EINVAL;
 }
 
-static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
-struct v4l2_subdev *subdev,
-struct v4l2_async_subdev *asd)
-{
-   struct isp_async_subdev *isd =
-   container_of(asd, struct isp_async_subdev, asd);
-
-   isd->sd = subdev;
-   isd->sd->host_priv = >bus;
-
-   return 0;
-}
-
 static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
 {
struct isp_device *isp = container_of(async, struct isp_device,
  notifier);
struct v4l2_device *v4l2_dev = >v4l2_dev;
struct v4l2_subdev *sd;
-   struct isp_bus_cfg *bus;
int ret;
 
ret = media_entity_enum_init(>crashed, >media_dev);
@@ -2215,13 +2201,13 @@ static int isp_subdev_notifier_complete(struct 
v4l2_async_notifier *async)
return ret;
 
list_for_each_entry(sd, _dev->subdevs, list) {
-   /* Only try to link entities whose interface was set on bound */
-   if (sd->host_priv) {
-   bus = (struct isp_bus_cfg *)sd->host_priv;
-   ret = isp_link_entity(isp, >entity, bus->interface);
-   if (ret < 0)
-   return ret;
-   }
+   if (!sd->asd)
+   continue;
+
+   ret = isp_link_entity(isp, >entity,
+ v4l2_subdev_to_bus_cfg(sd)->interface);
+   if (ret < 0)
+   return ret;
}
 
ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
@@ -2399,7 +2385,6 @@ static int isp_probe(struct platform_device *pdev)
if (ret < 0)
goto error_register_entities;
 
-   isp->notifier.bound = isp_subdev_notifier_bound;
isp->notifier.complete = isp_subdev_notifier_complete;
 
ret = v4l2_async_notifier_register(>v4l2_dev, >notifier);
diff --git a/drivers/media/platform/omap3isp/isp.h 
b/drivers/media/platform/omap3isp/isp.h
index 2f2ae609c548..e528df6efc09 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -226,11 +226,13 @@ struct isp_device {
 };
 
 struct isp_async_subdev {
-   struct v4l2_subdev *sd;
struct isp_bus_cfg bus;
struct v4l2_async_subdev asd;
 };
 
+#define v4l2_subdev_to_bus_cfg(sd) \
+   (_of((sd)->asd, struct isp_async_subdev, asd)->bus)
+
 #define v4l2_dev_to_isp_device(dev) \
container_of(dev, struct isp_device, v4l2_dev)
 
diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
b/drivers/media/platform/omap3isp/ispccdc.c
index 4947876cfadf..b66276ab5765 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1139,8 +1139,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
pad = media_entity_remote_pad(>pads[CCDC_PAD_SINK]);
sensor = media_entity_to_v4l2_subdev(pad->entity);
if (ccdc->input == CCDC_INPUT_PARALLEL) {
-   parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
-   ->bus.parallel;
+   struct v4l2_subdev *sd =
+   to_isp_pipeline(>subdev.entity)->external;
+
+   parcfg = _subdev_to_bus_cfg(sd)->bus.parallel;
ccdc->bt656 = parcfg->bt656;
}
 
@@ -2412,11 +2414,11 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
 
/* We've got a parallel sensor here. */
if (ccdc->input == CCDC_INPUT_PARALLEL) {
-   struct isp_parallel_cfg *parcfg =
-   &((struct isp_bus_cfg *)
- 

[PATCHv3 9/9] omapdrm: omapdss_hdmi_ops: add lost_hotplug op

[Hans Verkuil]

The CEC framework needs to know when the hotplug detect signal
disappears, since that means the CEC physical address has to be
invalidated (i.e. set to f.f.f.f).

Add a lost_hotplug op that is called when the HPD signal goes away.

Signed-off-by: Hans Verkuil 
---
Change since v2: check that the lost_hotplug callback is set before
calling it in encoder-tpd12s015.c.
---
 drivers/gpu/drm/omapdrm/displays/connector-hdmi.c| 8 ++--
 drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 6 +-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c  | 8 ++--
 drivers/gpu/drm/omapdrm/dss/omapdss.h| 1 +
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c 
b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
index d9d25df6fc1b..4600d3841c25 100644
--- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
+++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
@@ -165,11 +165,15 @@ static bool hdmic_detect(struct omap_dss_device *dssdev)
 {
struct panel_drv_data *ddata = to_panel_data(dssdev);
struct omap_dss_device *in = ddata->in;
+   bool connected;

if (gpio_is_valid(ddata->hpd_gpio))
-   return gpio_get_value_cansleep(ddata->hpd_gpio);
+   connected = gpio_get_value_cansleep(ddata->hpd_gpio);
else
-   return in->ops.hdmi->detect(in);
+   connected = in->ops.hdmi->detect(in);
+   if (!connected && in->ops.hdmi->lost_hotplug)
+   in->ops.hdmi->lost_hotplug(in);
+   return connected;
 }

 static int hdmic_register_hpd_cb(struct omap_dss_device *dssdev,
diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c 
b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
index 293b8fd07cfc..e3d98d78fc40 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
@@ -159,8 +159,12 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
 static bool tpd_detect(struct omap_dss_device *dssdev)
 {
struct panel_drv_data *ddata = to_panel_data(dssdev);
+   struct omap_dss_device *in = ddata->in;
+   bool connected = gpiod_get_value_cansleep(ddata->hpd_gpio);

-   return gpiod_get_value_cansleep(ddata->hpd_gpio);
+   if (!connected && in->ops.hdmi->lost_hotplug)
+   in->ops.hdmi->lost_hotplug(in);
+   return connected;
 }

 static int tpd_register_hpd_cb(struct omap_dss_device *dssdev,
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c 
b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index e535010218e6..0eeba0d1a2f5 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -402,8 +402,6 @@ static void hdmi_display_disable(struct omap_dss_device 
*dssdev)

DSSDBG("Enter hdmi_display_disable\n");

-   hdmi4_cec_set_phys_addr(, CEC_PHYS_ADDR_INVALID);
-
mutex_lock();

spin_lock_irqsave(_playing_lock, flags);
@@ -515,6 +513,11 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
return r;
 }

+static void hdmi_lost_hotplug(struct omap_dss_device *dssdev)
+{
+   hdmi4_cec_set_phys_addr(, CEC_PHYS_ADDR_INVALID);
+}
+
 static int hdmi_set_infoframe(struct omap_dss_device *dssdev,
const struct hdmi_avi_infoframe *avi)
 {
@@ -541,6 +544,7 @@ static const struct omapdss_hdmi_ops hdmi_ops = {
.get_timings= hdmi_display_get_timings,

.read_edid  = hdmi_read_edid,
+   .lost_hotplug   = hdmi_lost_hotplug,
.set_infoframe  = hdmi_set_infoframe,
.set_hdmi_mode  = hdmi_set_hdmi_mode,
 };
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index b9b0bb27069a..482a385894d7 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -402,6 +402,7 @@ struct omapdss_hdmi_ops {
struct videomode *vm);

int (*read_edid)(struct omap_dss_device *dssdev, u8 *buf, int len);
+   void (*lost_hotplug)(struct omap_dss_device *dssdev);
bool (*detect)(struct omap_dss_device *dssdev);

int (*register_hpd_cb)(struct omap_dss_device *dssdev,
-- 
2.14.1

Re: [PATCH 2/3] [media] davinci: constify platform_device_id

[Lad, Prabhakar]

On Tue, Aug 15, 2017 at 12:23 PM, Arvind Yadav
 wrote:
>
> platform_device_id are not supposed to change at runtime. All functions
> working with platform_device_id provided by 
> work with const platform_device_id. So mark the non-const structs as
> const.
>
> Signed-off-by: Arvind Yadav 

Acked-by: Lad, Prabhakar 

Cheers,
--Prabhakar Lad

Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue

[Jacob Chen]

Hi,

2017-08-17 20:11 GMT+08:00 Stanimir Varbanov :
> Hi Laurent,
>
> On 08/16/2017 03:28 PM, Laurent Pinchart wrote:
>> Hi Stan,
>>
>> On Wednesday 16 Aug 2017 14:46:50 Stanimir Varbanov wrote:
>>> On 08/15/2017 01:04 PM, Hans Verkuil wrote:
 On 08/14/17 10:41, Stanimir Varbanov wrote:
> Hi,
>
> This RFC patch is intended to give to the drivers a choice to change
> the default behavior of the v4l2-core DMA mapping direction from
> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
> to DMA_BIDIRECTIONAL during queue_init time.
>
> Initially the issue with DMA mapping direction has been found in
> Venus encoder driver where the firmware side of the driver adds few
> lines padding on bottom of the image buffer, and the consequence was
> triggering of IOMMU protection faults.
>
> Probably other drivers could also has a benefit of this feature (hint)
> in the future.
>

Just butt in.
some codec hardware also need it, which will use
capture buffers as reference to decode other buffers.

> Signed-off-by: Stanimir Varbanov 
> ---
>
>  drivers/media/v4l2-core/videobuf2-core.c |  3 +++
>  include/media/videobuf2-core.h   | 11 +++
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index
> 14f83cecfa92..17d07fda4cdc 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>
>int plane;
>int ret = -ENOMEM;
>
> +  if (q->bidirectional)
> +  dma_dir = DMA_BIDIRECTIONAL;
> +

 Does this only have to be used in mem_alloc? In the __prepare_*() it is
 still using DMA_TO/FROM_DEVICE.
>>>
>>> Yes, it looks like the DMA direction should be covered in the
>>> __prepare_* too. Thus the patch should look like below:
>>>
>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>>> b/drivers/media/v4l2-core/videobuf2-core.c
>>> index 14f83cecfa92..0089e7dac7dd 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>> @@ -188,14 +188,21 @@ module_param(debug, int, 0644);
>>>  static void __vb2_queue_cancel(struct vb2_queue *q);
>>>  static void __enqueue_in_driver(struct vb2_buffer *vb);
>>>
>>> +static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
>>> +{
>>> +if (q->bidirectional)
>>> +return DMA_BIDIRECTIONAL;
>>> +
>>> +return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>>
>> We could also compute the DMA direction once only and store it in the queue. 
>> I
>> have no big preference at the moment.
>
> Yes, I like the idea. I'll cook a regular patch where the DMA direction
> will be computed in vb2_core_queue_init().
>
> --
> regards,
> Stan

Re: [PATCHv2 0/9] omapdrm: hdmi4: add CEC support

[Hans Verkuil]

On 08/17/17 15:03, Tomi Valkeinen wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 11/08/17 13:57, Tomi Valkeinen wrote:
>> Hi Hans,
>>
>> On 02/08/17 11:53, Hans Verkuil wrote:
>>> From: Hans Verkuil 
>>>
>>> This patch series adds CEC support for the omap4. It is based on
>>> the 4.13-rc2 kernel with this patch series applied:
>>>
>>> http://www.spinics.net/lists/dri-devel/msg143440.html
>>>
>>> It is virtually identical to the first patch series posted in
>>> April:
>>>
>>> http://www.spinics.net/lists/dri-devel/msg138950.html
>>>
>>> The only two changes are in the Kconfig due to CEC Kconfig
>>> changes in 4.13 (it now selects CEC_CORE instead of depending on
>>> CEC_CORE) and a final patch was added adding a lost_hotplug op
>>> since for proper CEC support I have to know when the hotplug
>>> signal goes away.
>>>
>>> Tested with my Pandaboard.
>>
>> I'm doing some testing with this series on my panda. One issue I see is
>> that when I unload the display modules, I get:
>>
>> [   75.180206] platform 58006000.encoder: enabled after unload, idling
>> [   75.187896] platform 58001000.dispc: enabled after unload, idling
>> [   75.198242] platform 5800.dss: enabled after unload, idling
>>
>> So I think something is left enabled, most likely in the HDMI driver. I
>> haven't debugged this yet.
>>
>> The first time I loaded the modules I also got "operation stopped when
>> reading edid", but I haven't seen that since. Possibly not related to
>> this series.
> 
> Sorry that I have had very little time to debug this. I rebased the cec code
> on top of the latest omapdrm patches, and tested on AM5 EVM (which is more or
> less equivalent to OMAP5 on the HDMI front). I get the following crash when
> I turn on my monitor, which causes a HPD irq.

I see the issue: in drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c,
tpd_detect() this line:

if (!connected)

should be:

if (!connected && in->ops.hdmi->lost_hotplug)

I did that correctly in connector-hdmi.c, but not in encoder-tpd12s015.c.
And since omap5 doesn't fill in lost_hotplug this crashes.

I'll make a v3 of that patch.

Regards,

Hans

> I'll continue looking at this as soon as I again find time, but I thought I'll
> share what I have at the moment. I've pushed the branch to:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git 4.14/omapdrm-cec
> 
>  Tomi
> 
> [   34.640159] Unable to handle kernel NULL pointer dereference at virtual 
> address 
> [   34.648449] pgd = c0004000
> [   34.651249] [] *pgd=
> [   34.654921] Internal error: Oops: 8007 [#1] PREEMPT SMP ARM
> [   34.660879] Modules linked in: omapdrm drm_kms_helper drm connector_dvi 
> panel_dsi_cm panel_dpi connector_analog_tv connector_hdmi encode
> r_tpd12s015 encoder_tfp410 omapdss omapdss_base snd_soc_omap_hdmi_audio cec 
> cfbfillrect cfbimgblt cfbcopyarea [last unloaded: omapdss_base]
> [   34.685482] CPU: 0 PID: 264 Comm: irq/248-tpd12s0 Not tainted 
> 4.13.0-rc5-00626-gbf51300abae9 #99
> [   34.694314] Hardware name: Generic DRA74X (Flattened Device Tree)
> [   34.700442] task: ed108140 task.stack: ed19
> [   34.705002] PC is at 0x0
> [   34.707561] LR is at tpd_detect+0x3c/0x44 [encoder_tpd12s015]
> [   34.713340] pc : [<>]lr : []psr: 600c0013
> [   34.719642] sp : ed191ee8  ip : ed191e68  fp : ed191efc
> [   34.724897] r10: 0001  r9 : ee2e0200  r8 : c01b45c8
> [   34.730153] r7 : ed10b064  r6 :   r5 : bf1d716c  r4 : 
> [   34.736716] r3 :   r2 :   r1 :   r0 : bf1d716c
> [   34.743283] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment 
> none
> [   34.750458] Control: 10c5387d  Table: ad26406a  DAC: 0051
> [   34.756236] Process irq/248-tpd12s0 (pid: 264, stack limit = 0xed190218)
> [   34.762976] Stack: (0xed191ee8 to 0xed192000)
> [   34.767363] 1ee0:   ed27e610 ed27e6d4 ed191f14 ed191f00 
> bf200388 bf200310
> [   34.775587] 1f00: ed10b040 ee2e0200 ed191f34 ed191f18 c01b433c bf200354 
> ed19 0001
> [   34.783812] 1f20:  ed10b064 ed191f74 ed191f38 c01b4660 c01b4324 
> c01b4318 ed10b040
> [   34.792036] 1f40:  c01b4418 ed191f74 ed1ebc80  ed392500 
> ed19 ed10b040
> [   34.800261] 1f60: ed1ebcb8 ed24fb08 ed191fac ed191f78 c0163e60 c01b4504 
>  c01b44f8
> [   34.808486] 1f80: ed191fac ed392500 c0163d30    
>  
> [   34.816710] 1fa0:  ed191fb0 c0108af0 c0163d3c   
>  
> [   34.824934] 1fc0:       
>  
> [   34.833157] 1fe0:     0013  
>  
> [   34.841376] Backtrace: 
> [   34.843861] [] (tpd_detect [encoder_tpd12s015]) from 
> [] (tpd_hpd_isr+0x40/0x68 

Re: [RFT PATCH] [media] partial revert of "[media] tvp5150: add HW input connectors support"

[Philipp Zabel]

Hi,

On Tue, 2016-12-13 at 12:39 -0300, Javier Martinez Canillas wrote:
> Commit f7b4b54e6364 ("[media] tvp5150: add HW input connectors support")
> added input signals support for the tvp5150, but the approach was found
> to be incorrect so the corresponding DT binding commit 82c2ffeb217a
> ("[media] tvp5150: document input connectors DT bindings") was reverted.
> 
> This left the driver with an undocumented (and wrong) DT parsing logic,
> so lets get rid of this code as well until the input connectors support
> is implemented properly.
> 
> It's a partial revert due other patches added on top of mentioned commit
> not allowing the commit to be reverted cleanly anymore. But all the code
> related to the DT parsing logic and input entities creation are removed.
> 
> > Suggested-by: Laurent Pinchart 
> > Signed-off-by: Javier Martinez Canillas 

what is the state of this patch? Was it forgotten or was the revert
deemed unnecessary?

regards
Philipp

> ---
> 
> Hello Laurent,
> 
> I've tested this patch on top of media/master on my IGEPv2 + tvp5150
> with the following:
> 
> $ media-ctl -r -l '"tvp5150 1-005c":1->"OMAP3 ISP CCDC":0[1], "OMAP3 ISP 
> CCDC":1->"OMAP3 ISP CCDC output":0[1]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":0 [UYVY2X8 720x240 
> field:alternate]'
> $ media-ctl -v --set-format '"OMAP3 ISP CCDC":1 [UYVY2X8 720x240 
> field:interlaced-tb]'
> $ yavta -f UYVY -s 720x480 -n 1 --field interlaced-tb --capture=1 -F 
> /dev/video2
> $ raw2rgbpnm -f UYVY -s 720x480 frame-00.bin frame.pnm
> 
> I've also tested the other composite input with the following change:
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 5fe5faefe212..973be68ff78c 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1371,7 +1371,7 @@ static int tvp5150_probe(struct i2c_client *c,
> return res;
> 
> core->norm = V4L2_STD_ALL;  /* Default is autodetect */
> -   core->input = TVP5150_COMPOSITE1;
> +   core->input = TVP5150_COMPOSITE0;
> core->enable = true;
> 
> v4l2_ctrl_handler_init(>hdl, 5);
> 
> But as mentioned, it also worked for me without the revert so please let
> me know if the driver works with your omap3 board.
> 
> Best regards,
> Javier
> 
>  drivers/media/i2c/tvp5150.c | 142 
> 
>  1 file changed, 142 deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 48646a7f3fb0..5fe5faefe212 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -42,8 +42,6 @@ struct tvp5150 {
> >     struct v4l2_subdev sd;
>  #ifdef CONFIG_MEDIA_CONTROLLER
> >     struct media_pad pads[DEMOD_NUM_PADS];
> > -   struct media_entity input_ent[TVP5150_INPUT_NUM];
> > -   struct media_pad input_pad[TVP5150_INPUT_NUM];
>  #endif
> >     struct v4l2_ctrl_handler hdl;
> >     struct v4l2_rect rect;
> @@ -1018,40 +1016,6 @@ static int tvp5150_enum_frame_size(struct v4l2_subdev 
> *sd,
>  }
>  
>  /
> > -   Media entity ops
> - 
> /
> -
> -#ifdef CONFIG_MEDIA_CONTROLLER
> -static int tvp5150_link_setup(struct media_entity *entity,
> > -     const struct media_pad *local,
> > -     const struct media_pad *remote, u32 flags)
> -{
> > -   struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> > -   struct tvp5150 *decoder = to_tvp5150(sd);
> > -   int i;
> -
> > -   for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > -   if (remote->entity == >input_ent[i])
> > -   break;
> > -   }
> -
> > -   /* Do nothing for entities that are not input connectors */
> > -   if (i == TVP5150_INPUT_NUM)
> > -   return 0;
> -
> > -   decoder->input = i;
> -
> > -   tvp5150_selmux(sd);
> -
> > -   return 0;
> -}
> -
> -static const struct media_entity_operations tvp5150_sd_media_ops = {
> > -   .link_setup = tvp5150_link_setup,
> -};
> -#endif
> -
> -/
> >     I2C Command
>   
> /
>  
> @@ -1188,42 +1152,6 @@ static int tvp5150_g_tuner(struct v4l2_subdev *sd, 
> struct v4l2_tuner *vt)
> >     return 0;
>  }
>  
> -static int tvp5150_registered(struct v4l2_subdev *sd)
> -{
> -#ifdef CONFIG_MEDIA_CONTROLLER
> > -   struct tvp5150 *decoder = to_tvp5150(sd);
> > -   int ret = 0;
> > -   int i;
> -
> > -   for (i = 0; i < TVP5150_INPUT_NUM; i++) {
> > -   struct media_entity *input = >input_ent[i];
> > -   struct media_pad *pad = >input_pad[i];
> -
> > -   if (!input->name)
> > -   continue;
> -
> > -   

Re: [PATCHv2 0/9] omapdrm: hdmi4: add CEC support

[Tomi Valkeinen]


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 11/08/17 13:57, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 02/08/17 11:53, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This patch series adds CEC support for the omap4. It is based on
>> the 4.13-rc2 kernel with this patch series applied:
>>
>> http://www.spinics.net/lists/dri-devel/msg143440.html
>>
>> It is virtually identical to the first patch series posted in
>> April:
>>
>> http://www.spinics.net/lists/dri-devel/msg138950.html
>>
>> The only two changes are in the Kconfig due to CEC Kconfig
>> changes in 4.13 (it now selects CEC_CORE instead of depending on
>> CEC_CORE) and a final patch was added adding a lost_hotplug op
>> since for proper CEC support I have to know when the hotplug
>> signal goes away.
>>
>> Tested with my Pandaboard.
> 
> I'm doing some testing with this series on my panda. One issue I see is
> that when I unload the display modules, I get:
> 
> [   75.180206] platform 58006000.encoder: enabled after unload, idling
> [   75.187896] platform 58001000.dispc: enabled after unload, idling
> [   75.198242] platform 5800.dss: enabled after unload, idling
> 
> So I think something is left enabled, most likely in the HDMI driver. I
> haven't debugged this yet.
> 
> The first time I loaded the modules I also got "operation stopped when
> reading edid", but I haven't seen that since. Possibly not related to
> this series.

Sorry that I have had very little time to debug this. I rebased the cec code
on top of the latest omapdrm patches, and tested on AM5 EVM (which is more or
less equivalent to OMAP5 on the HDMI front). I get the following crash when
I turn on my monitor, which causes a HPD irq.

I'll continue looking at this as soon as I again find time, but I thought I'll
share what I have at the moment. I've pushed the branch to:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git 4.14/omapdrm-cec

 Tomi

[   34.640159] Unable to handle kernel NULL pointer dereference at virtual 
address 
[   34.648449] pgd = c0004000
[   34.651249] [] *pgd=
[   34.654921] Internal error: Oops: 8007 [#1] PREEMPT SMP ARM
[   34.660879] Modules linked in: omapdrm drm_kms_helper drm connector_dvi 
panel_dsi_cm panel_dpi connector_analog_tv connector_hdmi encode
r_tpd12s015 encoder_tfp410 omapdss omapdss_base snd_soc_omap_hdmi_audio cec 
cfbfillrect cfbimgblt cfbcopyarea [last unloaded: omapdss_base]
[   34.685482] CPU: 0 PID: 264 Comm: irq/248-tpd12s0 Not tainted 
4.13.0-rc5-00626-gbf51300abae9 #99
[   34.694314] Hardware name: Generic DRA74X (Flattened Device Tree)
[   34.700442] task: ed108140 task.stack: ed19
[   34.705002] PC is at 0x0
[   34.707561] LR is at tpd_detect+0x3c/0x44 [encoder_tpd12s015]
[   34.713340] pc : [<>]lr : []psr: 600c0013
[   34.719642] sp : ed191ee8  ip : ed191e68  fp : ed191efc
[   34.724897] r10: 0001  r9 : ee2e0200  r8 : c01b45c8
[   34.730153] r7 : ed10b064  r6 :   r5 : bf1d716c  r4 : 
[   34.736716] r3 :   r2 :   r1 :   r0 : bf1d716c
[   34.743283] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   34.750458] Control: 10c5387d  Table: ad26406a  DAC: 0051
[   34.756236] Process irq/248-tpd12s0 (pid: 264, stack limit = 0xed190218)
[   34.762976] Stack: (0xed191ee8 to 0xed192000)
[   34.767363] 1ee0:   ed27e610 ed27e6d4 ed191f14 ed191f00 
bf200388 bf200310
[   34.775587] 1f00: ed10b040 ee2e0200 ed191f34 ed191f18 c01b433c bf200354 
ed19 0001
[   34.783812] 1f20:  ed10b064 ed191f74 ed191f38 c01b4660 c01b4324 
c01b4318 ed10b040
[   34.792036] 1f40:  c01b4418 ed191f74 ed1ebc80  ed392500 
ed19 ed10b040
[   34.800261] 1f60: ed1ebcb8 ed24fb08 ed191fac ed191f78 c0163e60 c01b4504 
 c01b44f8
[   34.808486] 1f80: ed191fac ed392500 c0163d30    
 
[   34.816710] 1fa0:  ed191fb0 c0108af0 c0163d3c   
 
[   34.824934] 1fc0:       
 
[   34.833157] 1fe0:     0013  
 
[   34.841376] Backtrace: 
[   34.843861] [] (tpd_detect [encoder_tpd12s015]) from [] 
(tpd_hpd_isr+0x40/0x68 [encoder_tpd12s015])
[   34.854701]  r5:ed27e6d4 r4:ed27e610
[   34.858310] [] (tpd_hpd_isr [encoder_tpd12s015]) from [] 
(irq_thread_fn+0x24/0x5c)
[   34.867667]  r5:ee2e0200 r4:ed10b040
[   34.871270] [] (irq_thread_fn) from [] 
(irq_thread+0x168/0x254)
[   34.878970]  r7:ed10b064 r6: r5:0001 r4:ed19
[   34.884670] [] (irq_thread) from [] (kthread+0x130/0x174)
[   34.891847]  r10:ed24fb08 r9:ed1ebcb8 r8:ed10b040 r7:ed19 r6:ed392500 
r5:
[   34.899719]  r4:ed1ebc80
[   34.902280] [] (kthread) from [] 
(ret_from_fork+0x14/0x24)
[   34.909544]  

Re: [PATCH v2 4/8] v4l: vsp1: Use reference counting for fragments

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:27 Kieran Bingham wrote:
> 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 fragment update code, but it's not a
> straightforward squash as the refcounts will affect both:
>   v4l: vsp1: Provide a fragment pool
> and
>   v4l: vsp1: Convert display lists to use new fragment pool
> therefore, I have kept this separate to prevent breaking bisectability
> of the vsp-tests.

Sounds good to me.

> ---
>  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 52c523625e2f..175717018e11
> 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_fragment(dl, dlb);
> +
> + /* release our local reference */
> + vsp1_dl_fragment_put(dlb);
> + }
> +
>   break;
>   }
>  }
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 6ffdc3549283..37feda248946
> 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_fragment_pool *pool;
>   struct vsp1_device *vsp1;
> 
> @@ -230,6 +233,7 @@ struct vsp1_dl_body *vsp1_dl_fragment_get(struct
> vsp1_dl_fragment_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);
> @@ -244,6 +248,9 @@ void vsp1_dl_fragment_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);
> @@ -428,7 +435,11 @@ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32
> reg, u32 data)
>   * list, in the order in which fragments are added.
>   *
>   * Adding a fragment to a display list passes ownership of the fragment to
> the
> - * list. The caller must not touch the fragment after this call.
> + * list. The caller must not modify the fragment after this call, but can
> retain
> + * a reference to it for future use if necessary, to add to subsequent
> lists.

I think there's a bit of contradiction here, if the ownership passes to the 
list then the caller shouldn't touch it anymore. How about stating it as 
follows ?

 * The caller retains its reference to the fragment when adding it to a
 * display list, but is not allowed to add new entries to the fragment.
 * The reference must be explicitly released by a call to
 * vsp1_dl_fragment_put() when the fragment isn't needed anymore.

> the
> - * list. The caller must not touch the fragment after this call.
> + * list. The caller must not modify the fragment 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.
>   *
>   * Fragments are only usable for display lists in header mode. Attempt to
>   * add a fragment to a header-less display list will return an error.
> @@ -440,6 +451,8 @@ int vsp1_dl_list_add_fragment(struct vsp1_dl_list *dl,
>   if (dl->dlm->mode != VSP1_DL_MODE_HEADER)
>   return -EINVAL;
> 
> + refcount_inc(>refcnt);
> +
>   list_add_tail(>list, >fragments);
>   return 0;
>  }
> diff --git a/drivers/media/platform/vsp1/vsp1_lut.c
> b/drivers/media/platform/vsp1/vsp1_lut.c index 57482e057e54..388bd89ade0b
> 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;
>   

Re: [PATCH v2 3/8] v4l: vsp1: Convert display lists to use new fragment pool

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:26 Kieran Bingham wrote:
> Adapt the dl->body0 object to use an object from the fragment 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 two bodies,
> allowing a userspace update before the hardware has committed a previous
> set of tables.

I think you'll need three bodies, one for the DL queued to the hardware, one 
for the pending DL and one for the new DL needed when you update the LUT/CLU. 
Given that the VSP test suite hasn't caught this problem, we also need a new 
test :-)

> Fragments 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 
> 
> ---
> 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 |  22 ++-
>  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 |  23 ++-
>  drivers/media/platform/vsp1/vsp1_lut.h |   1 +-
>  6 files changed, 90 insertions(+), 183 deletions(-)

This is a nice diffstat, but only if you add kerneldoc for the new functions 
introduced in patch 2/8, otherwise the overall documentation diffstat looks 
bad :-)

> diff --git a/drivers/media/platform/vsp1/vsp1_clu.c
> b/drivers/media/platform/vsp1/vsp1_clu.c index f2fb26e5ab4e..52c523625e2f
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_clu.c
> +++ b/drivers/media/platform/vsp1/vsp1_clu.c

[snip]

> @@ -288,6 +298,12 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device
> *vsp1) if (ret < 0)
>   return ERR_PTR(ret);
> 
> + /* Allocate a fragment pool */

The comment would be more useful if you explained why you need to allocate a 
pool here. Same comment for the LUT.

> + clu->pool = vsp1_dl_fragment_pool_alloc(clu->entity.vsp1, 2,
> + 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);

[snip]

> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index aab9dd6ec0eb..6ffdc3549283
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c

[snip]


> @@ -379,41 +289,39 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct
> vsp1_dl_manager *dlm) INIT_LIST_HEAD(>fragments);
>   dl->dlm = dlm;
> 
> - /*
> -  * Initialize the display list body and allocate DMA memory for the 
body
> -  * and the optional header. Both are allocated together to avoid 
memory
> -  * fragmentation, with the header located right after the body in
> -  * memory.
> -  */
> - header_size = dlm->mode == VSP1_DL_MODE_HEADER
> - ? ALIGN(sizeof(struct vsp1_dl_header), 8)
> - : 0;
> -
> - ret = vsp1_dl_body_init(dlm->vsp1, >body0, VSP1_DL_NUM_ENTRIES,
> - header_size);
> - if (ret < 0) {
> - kfree(dl);
> + /* Retrieve a body from our DLM body pool */
> + dl->body0 = vsp1_dl_fragment_get(pool);
> + if (!dl->body0)
>   return NULL;
> - }
> -
>   if (dlm->mode == VSP1_DL_MODE_HEADER) {
> - size_t header_offset = VSP1_DL_NUM_ENTRIES
> -  * sizeof(*dl->body0.entries);
> + size_t header_offset = dl->body0->max_entries
> +  * sizeof(*dl->body0->entries);
> 
> - dl->header = ((void *)dl->body0.entries) + header_offset;
> - dl->dma = dl->body0.dma + header_offset;
> + dl->header = ((void *)dl->body0->entries) + header_offset;
> + dl->dma = dl->body0->dma + header_offset;
> 
>   memset(dl->header, 0, sizeof(*dl->header));
> - dl->header->lists[0].addr = dl->body0.dma;
> + dl->header->lists[0].addr = dl->body0->dma;
>   }
> 
>   return dl;
>  }
> 
> +static void vsp1_dl_list_fragments_free(struct vsp1_dl_list *dl)

This function doesn't free fragments put puts them back to the free list. I'd 
call it vsp1_dl_list_fragments_put().

> +{
> + struct vsp1_dl_body *dlb, *tmp;
> +
> + list_for_each_entry_safe(dlb, tmp, >fragments, list) {
> + list_del(>list);
> + 

Re: [PATCH v2 2/8] v4l: vsp1: Provide a fragment pool

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:25 Kieran Bingham wrote:
> 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 'fragment pool'. A pool can be allocated by the display list
> manager or entities which require their own body allocations.
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2:
>  - assign dlb->dma correctly
> ---
>  drivers/media/platform/vsp1/vsp1_dl.c | 129 +++-
>  drivers/media/platform/vsp1/vsp1_dl.h |   8 ++-
>  2 files changed, 137 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index cb4625ae13c2..aab9dd6ec0eb
> 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_fragment_pool *pool;
>   struct vsp1_device *vsp1;
> 
>   struct vsp1_dl_entry *entries;
> @@ -65,6 +70,30 @@ struct vsp1_dl_body {
>  };
> 
>  /**
> + * struct vsp1_dl_fragment_pool - display list body/fragment 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_fragment_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: fragment pool for the display list bodies
>   * @gc_work: fragments garbage collector work struct
>   * @gc_fragments: array of display list fragments 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_fragment_pool *pool;
> +
>   struct work_struct gc_work;
>   struct list_head gc_fragments;
>  };
> @@ -128,6 +160,103 @@ struct vsp1_dl_manager {
>   */
> 
>  /*
> + * Fragment pool's reduce the pressure on the iommu TLB by allocating a
> single
> + * large area of DMA memory and allocating it as a pool of fragment bodies
> + */

Could you document non-static function using kerneldoc ? Parameters to this 
function would benefit from some documentation. I'd also like to see the 
fragment get/put functions documented, as you remove existing kerneldoc for 
the alloc/free existing functions in patch 3/8.

> +struct vsp1_dl_fragment_pool *
> +vsp1_dl_fragment_pool_alloc(struct vsp1_device *vsp1, unsigned int qty,

I think I would name this function vsp1_dl_fragment_pool_create(), as it does 
more than just allocating memory. Similarly I'd call the free function 
vsp1_dl_fragment_pool_destroy().

qty is a bit vague, I'd rename it to num_fragments.

> + unsigned int num_entries, size_t extra_size)
> +{
> + struct vsp1_dl_fragment_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;

extra_size is only used by vsp1_dlm_create(), to allocate extra memory for the 
display list header. We need one header per display list, not per display list 
body.

> + pool->size = dlb_size * qty;
> +
> + pool->bodies = kcalloc(qty, sizeof(*pool->bodies), GFP_KERNEL);
> + if (!pool->bodies) {
> + kfree(pool);
> + return NULL;
> + }
> +
> + pool->mem = dma_alloc_wc(vsp1->bus_master, 

Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue

[Stanimir Varbanov]

Hi Laurent,

On 08/16/2017 03:28 PM, Laurent Pinchart wrote:
> Hi Stan,
> 
> On Wednesday 16 Aug 2017 14:46:50 Stanimir Varbanov wrote:
>> On 08/15/2017 01:04 PM, Hans Verkuil wrote:
>>> On 08/14/17 10:41, Stanimir Varbanov wrote:
 Hi,

 This RFC patch is intended to give to the drivers a choice to change
 the default behavior of the v4l2-core DMA mapping direction from
 DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
 to DMA_BIDIRECTIONAL during queue_init time.

 Initially the issue with DMA mapping direction has been found in
 Venus encoder driver where the firmware side of the driver adds few
 lines padding on bottom of the image buffer, and the consequence was
 triggering of IOMMU protection faults.

 Probably other drivers could also has a benefit of this feature (hint)
 in the future.

 Signed-off-by: Stanimir Varbanov 
 ---

  drivers/media/v4l2-core/videobuf2-core.c |  3 +++
  include/media/videobuf2-core.h   | 11 +++
  2 files changed, 14 insertions(+)

 diff --git a/drivers/media/v4l2-core/videobuf2-core.c
 b/drivers/media/v4l2-core/videobuf2-core.c index
 14f83cecfa92..17d07fda4cdc 100644
 --- a/drivers/media/v4l2-core/videobuf2-core.c
 +++ b/drivers/media/v4l2-core/videobuf2-core.c
 @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)

int plane;
int ret = -ENOMEM;

 +  if (q->bidirectional)
 +  dma_dir = DMA_BIDIRECTIONAL;
 +
>>>
>>> Does this only have to be used in mem_alloc? In the __prepare_*() it is
>>> still using DMA_TO/FROM_DEVICE.
>>
>> Yes, it looks like the DMA direction should be covered in the
>> __prepare_* too. Thus the patch should look like below:
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 14f83cecfa92..0089e7dac7dd 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -188,14 +188,21 @@ module_param(debug, int, 0644);
>>  static void __vb2_queue_cancel(struct vb2_queue *q);
>>  static void __enqueue_in_driver(struct vb2_buffer *vb);
>>
>> +static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
>> +{
>> +if (q->bidirectional)
>> +return DMA_BIDIRECTIONAL;
>> +
>> +return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> 
> We could also compute the DMA direction once only and store it in the queue. 
> I 
> have no big preference at the moment.

Yes, I like the idea. I'll cook a regular patch where the DMA direction
will be computed in vb2_core_queue_init().

-- 
regards,
Stan

[PATCH] [media] usb: rainshadow-cec: constify serio_device_id

[Arvind Yadav]

serio_device_id are not supposed to change at runtime. All functions
working with serio_device_id provided by  work with
const serio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/usb/rainshadow-cec/rainshadow-cec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c 
b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
index f203699..b82e37c 100644
--- a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
+++ b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
@@ -361,7 +361,7 @@ static int rain_connect(struct serio *serio, struct 
serio_driver *drv)
return err;
 }
 
-static struct serio_device_id rain_serio_ids[] = {
+static const struct serio_device_id rain_serio_ids[] = {
{
.type   = SERIO_RS232,
.proto  = SERIO_RAINSHADOW_CEC,
-- 
2.7.4

[PATCH] [media] usb: pulse8-cec: constify serio_device_id

[Arvind Yadav]

serio_device_id are not supposed to change at runtime. All functions
working with serio_device_id provided by  work with
const serio_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav 
---
 drivers/media/usb/pulse8-cec/pulse8-cec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c 
b/drivers/media/usb/pulse8-cec/pulse8-cec.c
index c843070..d10d803 100644
--- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
+++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
@@ -732,7 +732,7 @@ static void pulse8_ping_eeprom_work_handler(struct 
work_struct *work)
mutex_unlock(>config_lock);
 }
 
-static struct serio_device_id pulse8_serio_ids[] = {
+static const struct serio_device_id pulse8_serio_ids[] = {
{
.type   = SERIO_RS232,
.proto  = SERIO_PULSE8_CEC,
-- 
2.7.4

Re: [PATCH v7] media: platform: Renesas IMR driver

[Hans Verkuil]

On 08/17/17 09:59, Hans Verkuil wrote:
> Hi Sergei,
> 
> A quick review. I'm concentrating on the mesh ioctl, since that's what sets 
> this
> driver apart.
> 
> On 08/04/2017 08:03 PM, Sergei Shtylyov wrote:
> 
> 
> 
>> Index: media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
>> ===
>> --- /dev/null
>> +++ media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
>> @@ -0,0 +1,372 @@
>> +Renesas R-Car Image Renderer (IMR) Driver
>> +=
>> +
>> +This file documents some driver-specific aspects of the IMR driver, such as
>> +the driver-specific ioctl.
> 
> Just drop the part ', such as...'.
> 
> Can you add a high-level description of the functionality here? Similar to 
> what
> you did in the bindings document.
> 
>> +
>> +The ioctl reference
>> +~~~
>> +
>> +See ``include/uapi/linux/rcar_imr.h`` for the data structures used.
>> +
>> +VIDIOC_IMR_MESH - Set mapping data
>> +^^
>> +
>> +Argument: ``struct imr_map_desc``
>> +
>> +**Description**:
>> +
>> +This ioctl sets up the mesh through which the input frames will be 
>> transformed
>> +into the output frames. The mesh can be strictly rectangular (when
>> +``IMR_MAP_MESH`` bit is set in ``imr_map_desc::type``) or arbitrary (when 
>> that
>> +bit is not set).
> 
> Wouldn't something like 'IMR_MAP_RECTANGULAR' be much more descriptive than 
> _MESH?
> There is nothing in the name _MESH to indicate that this switches the data to
> rectangles.
> 
>> +
>> +A rectangular mesh consists of the imr_mesh structure followed by M*N vertex
>> +objects (where M is ``imr_mesh::rows`` and N is ``imr_mesh::columns``).
>> +In case either ``IMR_MAP_AUTOSG`` or ``IMR_MAP_AUTODG`` (not both) bits are
>> +set in ``imr_map_desc::type``, ``imr_mesh::{x|y}0`` specify the coordinates
>> +of the top left corner of the auto-generated mesh and ``imr_mesh::d{x|y}``
>> +specify the mesh's X/Y steps.
> 
> So if the auto bits are set, then there are no vertex objects? Since it's auto
> generated by the hardware?
> 
> I believe we discussed in the past whether 'type' should be split in a 'type'
> and 'flags' field.
> 
>> +
>> +An arbitrary mesh consists of the imr_vbo structure followed by N triangle
>> +objects (where N is ``imr_vbo::num``), consisting of 3 vertex objects each.
>> +Setting ``IMR_MAP_AUTODG`` and ``IMR_MAP_AUTOSG`` bits in
>> +``imr_map_desc::type``) isn't allowed for this type of mesh.
>> +
>> +The vertex object has a complex structure depending on some of the bits in
>> +``imr_map_desc::type``:
>> +
>> +    ==  ==  
>> ===
>> +IMR_MAP_CLCE  IMR_MAP_LUCE  IMR_MAP_AUTODG  IMR_MAP_AUTOSG  Vertex 
>> structure variant
> 
> You should explain the meaning of these bits in this section. I.e., what does
> CLCE or AUTODG stand for?
> 
>> +    ==  ==  
>> ===
>> +\   
>> ``imr_full_coord``
>> +\   X   
>> ``imr_dst_coord``
>> +\   X   
>> ``imr_src_coord``
>> +\ X 
>> ``imr_full_coord_any_correct``
>> +\ X X   
>> ``imr_auto_coord_any_correct``
>> +\ X X   
>> ``imr_auto_coord_any_crrect``
> 
> crrect -> correct
> 
>> +X   
>> ``imr_full_coord_any_correct``
>> +X   X   
>> ``imr_auto_coord_any_correct``
>> +X   X   
>> ``imr_auto_coord_any_correct``
>> +X X 
>> ``imr_full_coord_both_correct``
>> +X X X   
>> ``imr_auto_coord_both_correct``
>> +X X X   
>> ``imr_auto_coord_both_correct``
>> +    ==  ==  
>> ===
>> +
>> +The luma correction is calculated according to the following formula (where
>> +``Y`` is the luma value after texture mapping, ``Y'`` is the luma value 
>> after
>> +luma correction, ``lscal`` and ``lofst`` are the luma correction scale and
>> +offset taken from ``struct imr_luma_correct``, ``YLDPO`` is a luma 
>> correction
>> +scale decimal point position specified by ``IMR_MAP_YLDPO(n)``): ::
>> +
>> +Y' = ((Y * lscal) >> YLDPO) + lofst
>> +
>> +The chroma correction is calculated according to the following formula 
>> (where
>> +``U/V`` are the chroma values after texture mapping, ``U'/V'`` are the 
>> chroma
>> +values after chroma correction, ``ubscl/vrscl`` and 

Re: [PATCH v2 1/8] v4l: vsp1: Protect fragments against overflow

[Kieran Bingham]

Hi Laurent,

Thanks for your review,

On 16/08/17 22:53, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.

> How about
> 
>   if (WARN_ONCE(dlb->num_entries >= dlb->max_entries,
> "DLB size exceeded (max %u)", dlb->max_entries))
>   return;
> 
> (WARN_ONCE contains the unlikely() already)
> 
> I'm not fussed either way,

That does seem cleaner. Updated ready for any repost.

Thanks
--
Kieran

Re: [PATCH 3/4] arm: dts: renesas: add cec clock for Koelsch board

[Simon Horman]

On Mon, Aug 14, 2017 at 05:34:41PM +0200, Geert Uytterhoeven wrote:
> On Sun, Jul 30, 2017 at 3:07 PM, Hans Verkuil  wrote:
> > From: Hans Verkuil 
> 
> Probably the one-line summary should be
> 
> ARM: dts: koelsch: Add CEC clock  for HDMI transmitter
> 
> > The adv7511 on the Koelsch board has a 12 MHz fixed clock
> > for the CEC block. Specify this in the dts to enable CEC support.
> >
> > Signed-off-by: Hans Verkuil 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, I have applied this patch with the updated one-line summary.

Re: [PATCH v7] media: platform: Renesas IMR driver

[Hans Verkuil]

Hi Sergei,

A quick review. I'm concentrating on the mesh ioctl, since that's what sets this
driver apart.

On 08/04/2017 08:03 PM, Sergei Shtylyov wrote:



> Index: media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
> ===
> --- /dev/null
> +++ media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
> @@ -0,0 +1,372 @@
> +Renesas R-Car Image Renderer (IMR) Driver
> +=
> +
> +This file documents some driver-specific aspects of the IMR driver, such as
> +the driver-specific ioctl.

Just drop the part ', such as...'.

Can you add a high-level description of the functionality here? Similar to what
you did in the bindings document.

> +
> +The ioctl reference
> +~~~
> +
> +See ``include/uapi/linux/rcar_imr.h`` for the data structures used.
> +
> +VIDIOC_IMR_MESH - Set mapping data
> +^^
> +
> +Argument: ``struct imr_map_desc``
> +
> +**Description**:
> +
> +This ioctl sets up the mesh through which the input frames will be 
> transformed
> +into the output frames. The mesh can be strictly rectangular (when
> +``IMR_MAP_MESH`` bit is set in ``imr_map_desc::type``) or arbitrary (when 
> that
> +bit is not set).

Wouldn't something like 'IMR_MAP_RECTANGULAR' be much more descriptive than 
_MESH?
There is nothing in the name _MESH to indicate that this switches the data to
rectangles.

> +
> +A rectangular mesh consists of the imr_mesh structure followed by M*N vertex
> +objects (where M is ``imr_mesh::rows`` and N is ``imr_mesh::columns``).
> +In case either ``IMR_MAP_AUTOSG`` or ``IMR_MAP_AUTODG`` (not both) bits are
> +set in ``imr_map_desc::type``, ``imr_mesh::{x|y}0`` specify the coordinates
> +of the top left corner of the auto-generated mesh and ``imr_mesh::d{x|y}``
> +specify the mesh's X/Y steps.

So if the auto bits are set, then there are no vertex objects? Since it's auto
generated by the hardware?

I believe we discussed in the past whether 'type' should be split in a 'type'
and 'flags' field.

> +
> +An arbitrary mesh consists of the imr_vbo structure followed by N triangle
> +objects (where N is ``imr_vbo::num``), consisting of 3 vertex objects each.
> +Setting ``IMR_MAP_AUTODG`` and ``IMR_MAP_AUTOSG`` bits in
> +``imr_map_desc::type``) isn't allowed for this type of mesh.
> +
> +The vertex object has a complex structure depending on some of the bits in
> +``imr_map_desc::type``:
> +
> +    ==  ==  
> ===
> +IMR_MAP_CLCE  IMR_MAP_LUCE  IMR_MAP_AUTODG  IMR_MAP_AUTOSG  Vertex structure 
> variant

You should explain the meaning of these bits in this section. I.e., what does
CLCE or AUTODG stand for?

> +    ==  ==  
> ===
> +\   
> ``imr_full_coord``
> +\   X   ``imr_dst_coord``
> +\   X   ``imr_src_coord``
> +\ X 
> ``imr_full_coord_any_correct``
> +\ X X   
> ``imr_auto_coord_any_correct``
> +\ X X   
> ``imr_auto_coord_any_crrect``

crrect -> correct

> +X   
> ``imr_full_coord_any_correct``
> +X   X   
> ``imr_auto_coord_any_correct``
> +X   X   
> ``imr_auto_coord_any_correct``
> +X X 
> ``imr_full_coord_both_correct``
> +X X X   
> ``imr_auto_coord_both_correct``
> +X X X   
> ``imr_auto_coord_both_correct``
> +    ==  ==  
> ===
> +
> +The luma correction is calculated according to the following formula (where
> +``Y`` is the luma value after texture mapping, ``Y'`` is the luma value after
> +luma correction, ``lscal`` and ``lofst`` are the luma correction scale and
> +offset taken from ``struct imr_luma_correct``, ``YLDPO`` is a luma correction
> +scale decimal point position specified by ``IMR_MAP_YLDPO(n)``): ::
> +
> + Y' = ((Y * lscal) >> YLDPO) + lofst
> +
> +The chroma correction is calculated according to the following formula (where
> +``U/V`` are the chroma values after texture mapping, ``U'/V'`` are the chroma
> +values after chroma correction, ``ubscl/vrscl`` and ``ubofs/vrofs`` are the
> +U/V value chroma correction scales and offsets taken from
> +``struct imr_chroma_correct``, ``UBDPO/VRDPO`` are the chroma correction 
> scale
> +decimal point positions specified by 

[PATCH 3/3] media: atmel-isc: Add more format configurations

[Wenyou Yang]

Add the configuration of formats: GREY, ARGB444, ARGB555 and ARGB32.

Signed-off-by: Wenyou Yang 
---

 drivers/media/platform/atmel/atmel-isc.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d91f4e5f8a8d..4e18fe1104c8 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -184,7 +184,7 @@ struct isc_device {
 #define RAW_FMT_IND_START0
 #define RAW_FMT_IND_END  11
 #define ISC_FMT_IND_START12
-#define ISC_FMT_IND_END  14
+#define ISC_FMT_IND_END  18
 
 static struct isc_format isc_formats[] = {
/* 0 */
@@ -246,12 +246,30 @@ static struct isc_format isc_formats[] = {
{ V4L2_PIX_FMT_YUV422P, 0x0, 16,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
  ISC_DCFG_IMODE_YC422P, ISC_DCTRL_DVIEW_PLANAR, 0x3fb },
+
/* 14 */
+   { V4L2_PIX_FMT_GREY, MEDIA_BUS_FMT_Y8_1X8, 8,
+ ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DATY8,
+ ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x1fb },
+
+   /* 15 */
+   { V4L2_PIX_FMT_ARGB444, MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE, 16,
+ ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB444,
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
+   /* 16 */
+   { V4L2_PIX_FMT_ARGB555, MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE, 16,
+ ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB555,
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
+   /* 17 */
{ V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_RGB565_2X8_LE, 16,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_RGB565,
  ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x7b },
+   /* 18 */
+   { V4L2_PIX_FMT_ARGB32, MEDIA_BUS_FMT_ARGB_1X32, 32,
+ ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_ARGB32,
+ ISC_DCFG_IMODE_PACKED32, ISC_DCTRL_DVIEW_PACKED, 0x7b },
 
-   /* 15 */
+   /* 19 */
{ V4L2_PIX_FMT_YUYV, MEDIA_BUS_FMT_YUYV8_2X8, 16,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
  ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
-- 
2.13.0

[PATCH 2/3] media: atmel-isc: Remove the redundant assignment

[Wenyou Yang]

Remove the redundant assignment of members in the isc_formats array.

Signed-off-by: Wenyou Yang 
---

 drivers/media/platform/atmel/atmel-isc.c | 64 
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 535bb03783fe..d91f4e5f8a8d 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -187,74 +187,74 @@ struct isc_device {
 #define ISC_FMT_IND_END  14
 
 static struct isc_format isc_formats[] = {
+   /* 0 */
{ V4L2_PIX_FMT_SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8, 8,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 1 */
{ V4L2_PIX_FMT_SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8, 8,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 2 */
{ V4L2_PIX_FMT_SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8, 8,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 3 */
{ V4L2_PIX_FMT_SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8, 8,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0 },
 
+   /* 4 */
{ V4L2_PIX_FMT_SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10, 16,
  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 5 */
{ V4L2_PIX_FMT_SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10, 16,
  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 6 */
{ V4L2_PIX_FMT_SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10, 16,
  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 7 */
{ V4L2_PIX_FMT_SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10, 16,
  ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
 
+   /* 8 */
{ V4L2_PIX_FMT_SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12, 16,
  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT12,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 9 */
{ V4L2_PIX_FMT_SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12, 16,
  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT12,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 10 */
{ V4L2_PIX_FMT_SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12, 16,
  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT12,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
+   /* 11 */
{ V4L2_PIX_FMT_SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12, 16,
  ISC_PFG_CFG0_BPS_TWELVE, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT12,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
+ ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0 },
 
+   /* 12 */
{ V4L2_PIX_FMT_YUV420, 0x0, 12,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
- ISC_DCFG_IMODE_YC420P, ISC_DCTRL_DVIEW_PLANAR, 0x7fb,
- false, false },
+ ISC_DCFG_IMODE_YC420P, ISC_DCTRL_DVIEW_PLANAR, 0x7fb },
+   /* 13 */
{ V4L2_PIX_FMT_YUV422P, 0x0, 16,
  ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_YYCC,
- ISC_DCFG_IMODE_YC422P, ISC_DCTRL_DVIEW_PLANAR, 0x3fb,
- false, false },
+ ISC_DCFG_IMODE_YC422P, ISC_DCTRL_DVIEW_PLANAR, 0x3fb },
+   /* 14 */
{ V4L2_PIX_FMT_RGB565, MEDIA_BUS_FMT_RGB565_2X8_LE, 16,
  

[PATCH 1/3] media: atmel-isc: Not support RBG format from sensor.

[Wenyou Yang]

The 12-bit parallel interface supports the Raw Bayer, YCbCr,
Monochrome and JPEG Compressed pixel formats from the external
sensor, not support RBG pixel format.

Signed-off-by: Wenyou Yang 
---

 drivers/media/platform/atmel/atmel-isc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index d4df3d4ccd85..535bb03783fe 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1478,6 +1478,11 @@ static int isc_formats_init(struct isc_device *isc)
while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
   NULL, _code)) {
mbus_code.index++;
+
+   /* Not support the RGB pixel formats from sensor */
+   if ((mbus_code.code & 0xf000) == 0x1000)
+   continue;
+
fmt = find_format_by_code(mbus_code.code, );
if (!fmt)
continue;
-- 
2.13.0

[PATCH 0/3] media: atmel-isc: Supplement the configuration of formats

[Wenyou Yang]

The intention of the patch set is to add more configuration of
formats: GREY, ARGB444, ARGB555 and ARGB32, and add the checking
the format from the extern sensor which doesn't support RGB
formats from the external sensor.


Wenyou Yang (3):
  media: atmel-isc: Not support RBG format from sensor.
  media: atmel-isc: Remove the redundant assignment
  media: atmel-isc: Add more format configurations

 drivers/media/platform/atmel/atmel-isc.c | 89 
 1 file changed, 56 insertions(+), 33 deletions(-)

-- 
2.13.0

Re: [PATCH v7] media: platform: Renesas IMR driver

[Hans Verkuil]

Hi Sergei,

A few high level comments (I'll look at the patch itself later):

- There is no MAINTAINERS entry, please add one.
- Don't attach the patch, post it inline (ideally with 'git send-email')
- Split up the patch into 4 separate patches: bindings, doc changes,
  driver and MAINTAINERS patch. This will make it easier to review.
- Please give the v4l2-compliance output in the cover letter of the v8
  patch series. I can't merge this driver without being certain there
  are no compliance issues.
- You also have IMR-LSX3 and IMR-LX3 patches, why not add them to the
  patch series? I can review the set as a single unit. Up to you, though.

Regards,

Hans

[GIT PULL FOR v4.14] cec: rename uAPI defines, fixes

[Hans Verkuil]

Hi Mauro,

The second patch renames two CEC events in the public API. Since these were
introduced for 4.14, now is the time to do the rename before they become
part of the ABI. While working with and working on the cec-gpio driver to
debug CEC issues I realized that optionally being able to monitor the HDMI
HPD (hotplug detect) pin as well is very useful.

So besides monitoring the CEC pin it will also be possible to monitor the
HPD pin in the future. That means that the pin event has to tell with pin
has an event, CEC or HPD. Hence the rename while I still can.

The last patch is a fix for the irq handling in cec-pin.c which was broken.
This only affects the cec-gpio driver which isn't merged yet (expected for
4.15), but it is good to get this fixed now.

Regards,

Hans

The following changes since commit ec0c3ec497cabbf3bfa03a9eb5edcc252190a4e0:

  media: ddbridge: split code into multiple files (2017-08-09 12:17:01 -0400)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git cec-rename

for you to fetch changes up to 36fa912800ef8129b6c8c9caffb74a84ff5be36d:

  cec-pin: fix irq handling (2017-08-17 08:43:50 +0200)


Hans Verkuil (3):
  s5p-cec: use CEC_CAP_DEFAULTS
  cec: rename pin events/function
  cec-pin: fix irq handling

 Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst |  2 +-
 Documentation/media/uapi/cec/cec-ioc-dqevent.rst |  8 
 Documentation/media/uapi/cec/cec-ioc-g-mode.rst  |  2 +-
 drivers/media/cec/cec-adap.c |  7 ---
 drivers/media/cec/cec-api.c  |  4 ++--
 drivers/media/cec/cec-pin.c  | 39 
---
 drivers/media/platform/s5p-cec/s5p_cec.c |  7 ++-
 include/media/cec-pin.h  |  6 +-
 include/media/cec.h  |  9 +
 include/uapi/linux/cec.h |  4 ++--
 10 files changed, 50 insertions(+), 38 deletions(-)

Re: [PATCH 4/5] stih-cec: use CEC_CAP_DEFAULTS

[Hans Verkuil]

Benjamin,

Can you please review this patch and the stm32-cec patch?

Thanks!

Hans

On 08/04/2017 12:41 PM, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Use the new CEC_CAP_DEFAULTS define in this driver.
> This also adds the CEC_CAP_RC capability which was missing here
> (and this is also the reason for this new define, to avoid missing
> such capabilities).
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/platform/sti/cec/stih-cec.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/sti/cec/stih-cec.c 
> b/drivers/media/platform/sti/cec/stih-cec.c
> index ce7964c71b50..dc221527fd05 100644
> --- a/drivers/media/platform/sti/cec/stih-cec.c
> +++ b/drivers/media/platform/sti/cec/stih-cec.c
> @@ -351,9 +351,7 @@ static int stih_cec_probe(struct platform_device *pdev)
>   }
>  
>   cec->adap = cec_allocate_adapter(_cec_adap_ops, cec,
> - CEC_NAME,
> - CEC_CAP_LOG_ADDRS | CEC_CAP_PASSTHROUGH |
> - CEC_CAP_TRANSMIT, CEC_MAX_LOG_ADDRS);
> + CEC_NAME, CEC_CAP_DEFAULTS, CEC_MAX_LOG_ADDRS);
>   if (IS_ERR(cec->adap))
>   return PTR_ERR(cec->adap);
>  
>