Re: [RFC 1/2] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
Hi Dave, Am 14.06.2017 um 17:15 schrieb Dave Stevenson: > Document the DT bindings for the CSI2/CCP2 receiver peripheral > (known as Unicam) on BCM283x SoCs. > > Signed-off-by: Dave Stevenson please add the devicetree guys in CC for the binding. > --- > .../devicetree/bindings/media/bcm2835-unicam.txt | 76 > ++ > 1 file changed, 76 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt > > diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt > b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt > new file mode 100644 > index 000..cc5a451 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt > @@ -0,0 +1,76 @@ > +Broadcom BCM283x Camera Interface (Unicam) > +-- > + > +The Unicam block on BCM283x SoCs is the receiver for either > +CSI-2 or CCP2 data from image sensors or similar devices. It would be nice to add some of your explanations to Hans in this document or into the driver. > + > +Required properties: > +=== > +- compatible : must be "brcm,bcm2835-unicam". > +- reg: physical base address and length of the register sets > for the > + device. > +- interrupts : should contain the IRQ line for this Unicam instance. > +- clocks : list of clock specifiers, corresponding to entries in > + clock-names property. > +- clock-names: must contain an "lp_clock" entry, matching entries > + in the clocks property. > + > +Optional properties > +=== > +- max-data-lanes: the hardware can support varying numbers of clock lanes. > + This value is the maximum number supported by this instance. > + Known values of 2 or 4. Default is 2. AFAIK, this isn't a common property yet. So possibly a vendor prefix must be added. > + > + > +Unicam supports a single port node. It should contain one 'port' child node > +with child 'endpoint' node. Please refer to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. > + > +Example: > + csi1: csi@7e801000 { > + compatible = "brcm,bcm2835-unicam"; > + reg = <0x7e801000 0x800>, > + <0x7e802004 0x4>; > + interrupts = <2 7>; > + clocks = <&clocks BCM2835_CLOCK_CAM1>; > + clock-names = "lp_clock"; > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + endpoint { > + remote-endpoint = <&tc358743_0>; > + > + }; > + }; > + }; > + > + i2c0: i2c@7e205000 { > + > + tc358743: tc358743@0f { Usually the node name should describe the function of the node for example: tc358743: csi-hdmi-bridge@0f Best regards Stefan > + compatible = "toshiba,tc358743"; > + reg = <0x0f>; > + status = "okay"; > + > + clocks = <&tc358743_clk>; > + clock-names = "refclk"; > + > + tc358743_clk: bridge-clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <2700>; > + }; > + > + port { > + tc358743_0: endpoint { > + remote-endpoint = <&csi1>; > + clock-lanes = <0>; > + data-lanes = <1 2 3 4>; > + clock-noncontinuous; > + link-frequencies = > + /bits/ 64 <29700>; > + }; > + }; > + }; > + };
Re: [PATCH 4/8] v4l2-flash: Use led_classdev instead of led_classdev_flash for indicator
Hi Sakari, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12-rc5 next-20170614] [cannot apply to robh/for-next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Support-registering-lens-flash-and-EEPROM-devices/20170615-084016 base: git://linuxtv.org/media_tree.git master config: tile-allmodconfig (attached as .config) compiler: tilegx-linux-gcc (GCC) 4.6.2 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=tile All warnings (new ones prefixed by >>): drivers/staging//greybus/light.c: In function 'gb_lights_light_v4l2_register': >> drivers/staging//greybus/light.c:574:10: warning: passing argument 4 of >> 'v4l2_flash_init' from incompatible pointer type [enabled by default] include/media/v4l2-flash-led-class.h:124:20: note: expected 'struct led_classdev *' but argument is of type 'struct led_classdev_flash *' vim +/v4l2_flash_init +574 drivers/staging//greybus/light.c 2870b52b Rui Miguel Silva 2015-08-14 558 2870b52b Rui Miguel Silva 2015-08-14 559 channel_flash = get_channel_from_mode(light, GB_CHANNEL_MODE_FLASH); 2870b52b Rui Miguel Silva 2015-08-14 560 WARN_ON(!channel_flash); 2870b52b Rui Miguel Silva 2015-08-14 561 2870b52b Rui Miguel Silva 2015-08-14 562 fled = &channel_flash->fled; 2870b52b Rui Miguel Silva 2015-08-14 563 2870b52b Rui Miguel Silva 2015-08-14 564 snprintf(sd_cfg->dev_name, sizeof(sd_cfg->dev_name), "%s", light->name); 2870b52b Rui Miguel Silva 2015-08-14 565 2870b52b Rui Miguel Silva 2015-08-14 566 /* Set the possible values to faults, in our case all faults */ 2870b52b Rui Miguel Silva 2015-08-14 567 sd_cfg->flash_faults = LED_FAULT_OVER_VOLTAGE | LED_FAULT_TIMEOUT | 2870b52b Rui Miguel Silva 2015-08-14 568 LED_FAULT_OVER_TEMPERATURE | LED_FAULT_SHORT_CIRCUIT | 2870b52b Rui Miguel Silva 2015-08-14 569 LED_FAULT_OVER_CURRENT | LED_FAULT_INDICATOR | 2870b52b Rui Miguel Silva 2015-08-14 570 LED_FAULT_UNDER_VOLTAGE | LED_FAULT_INPUT_VOLTAGE | 2870b52b Rui Miguel Silva 2015-08-14 571 LED_FAULT_LED_OVER_TEMPERATURE; 2870b52b Rui Miguel Silva 2015-08-14 572 2870b52b Rui Miguel Silva 2015-08-14 573 light->v4l2_flash = v4l2_flash_init(dev, NULL, fled, iled, 3f85c787 Rui Miguel Silva 2015-12-03 @574 &v4l2_flash_ops, sd_cfg); 2870b52b Rui Miguel Silva 2015-08-14 575 if (IS_ERR_OR_NULL(light->v4l2_flash)) { 2870b52b Rui Miguel Silva 2015-08-14 576 ret = PTR_ERR(light->v4l2_flash); 2870b52b Rui Miguel Silva 2015-08-14 577 goto out_free; 2870b52b Rui Miguel Silva 2015-08-14 578 } 2870b52b Rui Miguel Silva 2015-08-14 579 2870b52b Rui Miguel Silva 2015-08-14 580 return ret; 2870b52b Rui Miguel Silva 2015-08-14 581 2870b52b Rui Miguel Silva 2015-08-14 582 out_free: :: The code at line 574 was first introduced by commit :: 3f85c787b74c26f3816017e64288af907f291462 greybus: lights: add v4l2 flash operations :: TO: Rui Miguel Silva :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2] [media] mtk-mdp: Fix g_/s_selection capture/compose logic
Hi, Hans, Would you have time to review this patch v2? The patch v1 violates v4l2 spec. I have fixed it in v2. Sincerely, Ming Hsiu On Fri, 2017-05-12 at 10:42 +0800, Minghsiu Tsai wrote: > From: Daniel Kurtz > > Experiments show that the: > (1) mtk-mdp uses the _MPLANE form of CAPTURE/OUTPUT > (2) CAPTURE types use CROP targets, and OUTPUT types use COMPOSE targets > > Signed-off-by: Daniel Kurtz > Signed-off-by: Minghsiu Tsai > Signed-off-by: Houlong Wei > > --- > Changes in v2: > . Can not use *_MPLANE type in g_/s_selection > --- > drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > index 13afe48..e18ac626 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > @@ -838,10 +838,10 @@ static int mtk_mdp_m2m_g_selection(struct file *file, > void *fh, > bool valid = false; > > if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > - if (mtk_mdp_is_target_compose(s->target)) > + if (mtk_mdp_is_target_crop(s->target)) > valid = true; > } else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > - if (mtk_mdp_is_target_crop(s->target)) > + if (mtk_mdp_is_target_compose(s->target)) > valid = true; > } > if (!valid) { > @@ -908,10 +908,10 @@ static int mtk_mdp_m2m_s_selection(struct file *file, > void *fh, > bool valid = false; > > if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) { > - if (s->target == V4L2_SEL_TGT_COMPOSE) > + if (s->target == V4L2_SEL_TGT_CROP) > valid = true; > } else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { > - if (s->target == V4L2_SEL_TGT_CROP) > + if (s->target == V4L2_SEL_TGT_COMPOSE) > valid = true; > } > if (!valid) { > @@ -925,7 +925,7 @@ static int mtk_mdp_m2m_s_selection(struct file *file, > void *fh, > if (ret) > return ret; > > - if (mtk_mdp_is_target_crop(s->target)) > + if (mtk_mdp_is_target_compose(s->target)) > frame = &ctx->s_frame; > else > frame = &ctx->d_frame;
[PATCH 11/15] af9013: add configurable TS output pin
On serial TS mode output pin could be selected from D0 or D7. Add configuration option to for it. Rename TS mode config option prefix from AF9013_TS_ to AF9013_TS_MODE_. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 27 ++- drivers/media/dvb-frontends/af9013.h | 2 ++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 68091f2..6b86437 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -28,6 +28,7 @@ struct af9013_state { u8 tuner; u32 if_frequency; u8 ts_mode; + u8 ts_output_pin; bool spec_inv; u8 api_version[4]; u8 gpio[4]; @@ -955,17 +956,12 @@ static int af9013_init(struct dvb_frontend *fe) goto err; /* settings for mp2if */ - if (state->ts_mode == AF9013_TS_USB) { + if (state->ts_mode == AF9013_TS_MODE_USB) { /* AF9015 split PSB to 1.5k + 0.5k */ ret = regmap_update_bits(state->regmap, 0xd50b, 0x04, 0x04); if (ret) goto err; } else { - /* AF9013 change the output bit to data7 */ - ret = regmap_update_bits(state->regmap, 0xd500, 0x08, 0x08); - if (ret) - goto err; - /* AF9013 set mpeg to full speed */ ret = regmap_update_bits(state->regmap, 0xd502, 0x10, 0x10); if (ret) @@ -1046,9 +1042,12 @@ static int af9013_init(struct dvb_frontend *fe) goto err; } - /* TS mode */ - ret = regmap_update_bits(state->regmap, 0xd500, 0x06, -state->ts_mode << 1); + /* TS interface */ + if (state->ts_output_pin == 7) + utmp = 1 << 3 | state->ts_mode << 1; + else + utmp = 0 << 3 | state->ts_mode << 1; + ret = regmap_update_bits(state->regmap, 0xd500, 0x0e, utmp); if (ret) goto err; @@ -1147,7 +1146,7 @@ static int af9013_i2c_gate_ctrl(struct dvb_frontend *fe, int enable) if (state->i2c_gate_state == enable) return 0; - if (state->ts_mode == AF9013_TS_USB) + if (state->ts_mode == AF9013_TS_MODE_USB) ret = regmap_update_bits(state->regmap, 0xd417, 0x08, enable << 3); else @@ -1297,6 +1296,7 @@ struct dvb_frontend *af9013_attach(const struct af9013_config *config, pdata.tuner = config->tuner; pdata.if_frequency = config->if_frequency; pdata.ts_mode = config->ts_mode; + pdata.ts_output_pin = 7; pdata.spec_inv = config->spec_inv; memcpy(&pdata.api_version, config->api_version, sizeof(pdata.api_version)); memcpy(&pdata.gpio, config->gpio, sizeof(pdata.gpio)); @@ -1450,7 +1450,7 @@ static int af9013_regmap_write(void *context, const void *data, size_t count) u8 *val = &((u8 *)data)[2]; const unsigned int len = count - 2; - if (state->ts_mode == AF9013_TS_USB && (reg & 0xff00) != 0xae00) { + if (state->ts_mode == AF9013_TS_MODE_USB && (reg & 0xff00) != 0xae00) { cmd = 0 << 7|0 << 6|(len - 1) << 2|1 << 1|1 << 0; ret = af9013_wregs(client, cmd, reg, val, len); if (ret) @@ -1487,7 +1487,7 @@ static int af9013_regmap_read(void *context, const void *reg_buf, u8 *val = &((u8 *)val_buf)[0]; const unsigned int len = val_size; - if (state->ts_mode == AF9013_TS_USB && (reg & 0xff00) != 0xae00) { + if (state->ts_mode == AF9013_TS_MODE_USB && (reg & 0xff00) != 0xae00) { cmd = 0 << 7|0 << 6|(len - 1) << 2|1 << 1|0 << 0; ret = af9013_rregs(client, cmd, reg, val_buf, len); if (ret) @@ -1537,6 +1537,7 @@ static int af9013_probe(struct i2c_client *client, state->tuner = pdata->tuner; state->if_frequency = pdata->if_frequency; state->ts_mode = pdata->ts_mode; + state->ts_output_pin = pdata->ts_output_pin; state->spec_inv = pdata->spec_inv; memcpy(&state->api_version, pdata->api_version, sizeof(state->api_version)); memcpy(&state->gpio, pdata->gpio, sizeof(state->gpio)); @@ -1549,7 +1550,7 @@ static int af9013_probe(struct i2c_client *client, } /* Download firmware */ - if (state->ts_mode != AF9013_TS_USB) { + if (state->ts_mode != AF9013_TS_MODE_USB) { ret = af9013_download_firmware(state); if (ret) goto err_regmap_exit; diff --git a/drivers/media/dvb-frontends/af9013.h b/drivers/media/dvb-frontends/af9013.h index 3f18258..3532745 100644 --- a/drivers/media/dvb-frontends/af9013.h +++ b/drivers/media/dvb-frontends/af9013.h @@ -33,6 +33,7 @@ * @tuner: Used tuner model.
[PATCH 03/15] af9013: add i2c client bindings
Add kernel i2c driver bindings. That allows dev_* logging, regmap and more. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 321 ++- drivers/media/dvb-frontends/af9013.h | 84 + 2 files changed, 241 insertions(+), 164 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 7880a63..f644182 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -24,9 +24,8 @@ #define MAX_XFER_SIZE 64 struct af9013_state { - struct i2c_adapter *i2c; + struct i2c_client *client; struct dvb_frontend fe; - u8 i2c_addr; u32 clk; u8 tuner; u32 if_frequency; @@ -59,7 +58,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { - .addr = priv->i2c_addr, + .addr = priv->client->addr, .flags = 0, .len = 3 + len, .buf = buf, @@ -67,7 +66,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, }; if (3 + len > sizeof(buf)) { - dev_warn(&priv->i2c->dev, + dev_warn(&priv->client->dev, "%s: i2c wr reg=%04x: len=%d is too big!\n", KBUILD_MODNAME, reg, len); return -EINVAL; @@ -78,11 +77,11 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, buf[2] = mbox; memcpy(&buf[3], val, len); - ret = i2c_transfer(priv->i2c, msg, 1); + ret = i2c_transfer(priv->client->adapter, msg, 1); if (ret == 1) { ret = 0; } else { - dev_warn(&priv->i2c->dev, "%s: i2c wr failed=%d reg=%04x " \ + dev_warn(&priv->client->dev, "%s: i2c wr failed=%d reg=%04x " \ "len=%d\n", KBUILD_MODNAME, ret, reg, len); ret = -EREMOTEIO; } @@ -97,12 +96,12 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, u8 buf[3]; struct i2c_msg msg[2] = { { - .addr = priv->i2c_addr, + .addr = priv->client->addr, .flags = 0, .len = 3, .buf = buf, }, { - .addr = priv->i2c_addr, + .addr = priv->client->addr, .flags = I2C_M_RD, .len = len, .buf = val, @@ -113,11 +112,11 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, buf[1] = (reg >> 0) & 0xff; buf[2] = mbox; - ret = i2c_transfer(priv->i2c, msg, 2); + ret = i2c_transfer(priv->client->adapter, msg, 2); if (ret == 2) { ret = 0; } else { - dev_warn(&priv->i2c->dev, "%s: i2c rd failed=%d reg=%04x " \ + dev_warn(&priv->client->dev, "%s: i2c rd failed=%d reg=%04x " \ "len=%d\n", KBUILD_MODNAME, ret, reg, len); ret = -EREMOTEIO; } @@ -231,7 +230,7 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) u8 pos; u16 addr; - dev_dbg(&state->i2c->dev, "%s: gpio=%d gpioval=%02x\n", + dev_dbg(&state->client->dev, "%s: gpio=%d gpioval=%02x\n", __func__, gpio, gpioval); /* @@ -250,7 +249,7 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) break; default: - dev_err(&state->i2c->dev, "%s: invalid gpio=%d\n", + dev_err(&state->client->dev, "%s: invalid gpio=%d\n", KBUILD_MODNAME, gpio); ret = -EINVAL; goto err; @@ -274,7 +273,7 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) return ret; err: - dev_dbg(&state->i2c->dev, "%s: failed=%d\n", __func__, ret); + dev_dbg(&state->client->dev, "%s: failed=%d\n", __func__, ret); return ret; } @@ -282,7 +281,7 @@ static u32 af9013_div(struct af9013_state *state, u32 a, u32 b, u32 x) { u32 r = 0, c = 0, i; - dev_dbg(&state->i2c->dev, "%s: a=%d b=%d x=%d\n", __func__, a, b, x); + dev_dbg(&state->client->dev, "%s: a=%d b=%d x=%d\n", __func__, a, b, x); if (a > b) { c = a / b; @@ -299,7 +298,7 @@ static u32 af9013_div(struct af9013_state *state, u32 a, u32 b, u32 x) } r = (c << (u32)x) + r; - dev_dbg(&state->i2c->dev, "%s: a=%d b=%d x=%d r=%d r=%x\n", + dev_dbg(&state->client->dev, "%s: a=%d b=%d x=%d r=%d r=%x\n", __fu
[PATCH 10/15] af9015: enable 2nd TS flow control when dual mode
It needs to be enabled in order to get stream from slave af9013 demod. Signed-off-by: Antti Palosaari --- drivers/media/usb/dvb-usb-v2/af9015.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index 54c1d47..ee0e354 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -1131,10 +1131,21 @@ static int af9015_init_endpoint(struct dvb_usb_device *d) } /* enable / disable mp2if2 */ - if (state->dual_mode) + if (state->dual_mode) { ret = af9015_set_reg_bit(d, 0xd50b, 0); - else + if (ret) + goto error; + ret = af9015_set_reg_bit(d, 0xd520, 4); + if (ret) + goto error; + } else { ret = af9015_clear_reg_bit(d, 0xd50b, 0); + if (ret) + goto error; + ret = af9015_clear_reg_bit(d, 0xd520, 4); + if (ret) + goto error; + } error: if (ret) -- http://palosaari.fi/
[PATCH 07/15] af9013: fix error handling
Use typical (return 0/goto err/return err) error handling everywhere. Add missing error handling where missing. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 86 +--- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 70102c1..a6b88ae 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -94,7 +94,7 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) if (ret) goto err; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -147,7 +147,7 @@ static int af9013_power_ctrl(struct af9013_state *state, u8 onoff) goto err; } - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -166,7 +166,7 @@ static int af9013_statistics_ber_unc_start(struct dvb_frontend *fe) if (ret) goto err; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -199,7 +199,7 @@ static int af9013_statistics_ber_unc_result(struct dvb_frontend *fe) state->ber = (buf[2] << 16) | (buf[1] << 8) | buf[0]; state->ucblocks += (buf[4] << 8) | buf[3]; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -218,7 +218,7 @@ static int af9013_statistics_snr_start(struct dvb_frontend *fe) if (ret) goto err; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -283,7 +283,7 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe) } state->snr = utmp * 10; /* dB/10 */ - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -321,7 +321,7 @@ static int af9013_statistics_signal_strength(struct dvb_frontend *fe) state->signal_strength = signal_strength; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -398,8 +398,11 @@ static int af9013_set_frontend(struct dvb_frontend *fe) c->frequency, c->bandwidth_hz); /* program tuner */ - if (fe->ops.tuner_ops.set_params) - fe->ops.tuner_ops.set_params(fe); + if (fe->ops.tuner_ops.set_params) { + ret = fe->ops.tuner_ops.set_params(fe); + if (ret) + goto err; + } /* program CFOE coefficients */ if (c->bandwidth_hz != state->bandwidth_hz) { @@ -411,20 +414,28 @@ static int af9013_set_frontend(struct dvb_frontend *fe) } /* Return an error if can't find bandwidth or the right clock */ - if (i == ARRAY_SIZE(coeff_lut)) - return -EINVAL; + if (i == ARRAY_SIZE(coeff_lut)) { + ret = -EINVAL; + goto err; + } ret = regmap_bulk_write(state->regmap, 0xae00, coeff_lut[i].val, sizeof(coeff_lut[i].val)); + if (ret) + goto err; } /* program frequency control */ if (c->bandwidth_hz != state->bandwidth_hz || state->first_tune) { /* get used IF frequency */ - if (fe->ops.tuner_ops.get_if_frequency) - fe->ops.tuner_ops.get_if_frequency(fe, &if_frequency); - else + if (fe->ops.tuner_ops.get_if_frequency) { + ret = fe->ops.tuner_ops.get_if_frequency(fe, +&if_frequency); + if (ret) + goto err; + } else { if_frequency = state->if_frequency; + } dev_dbg(&client->dev, "if_frequency %u\n", if_frequency); @@ -659,7 +670,7 @@ static int af9013_set_frontend(struct dvb_frontend *fe) state->set_frontend_jiffies = jiffies; state->first_tune = false; - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -777,7 +788,7 @@ static int af9013_get_frontend(struct dvb_frontend *fe, break; } - return ret; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; @@ -828,7 +839,7 @@ static int af9013_read_status(struct dvb_frontend *fe, enum fe_status *status) state->fe_status = *status; state->read_status_jiffies = jiffies; - return ret; + return 0; err: dev_dbg(&client->dev
[PATCH 13/15] af9015: move 2nd demod power-up wait different location
We need to wait 2nd demod power-up before download firmware. Move that wait to more correct location. Signed-off-by: Antti Palosaari --- drivers/media/usb/dvb-usb-v2/af9015.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index ee0e354..53d478d 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -740,9 +740,6 @@ static int af9015_copy_firmware(struct dvb_usb_device *d) fw_params[2] = state->firmware_checksum >> 8; fw_params[3] = state->firmware_checksum & 0xff; - /* wait 2nd demodulator ready */ - msleep(100); - ret = af9015_read_reg_i2c(d, state->af9013_config[1].i2c_addr, 0x98be, &val); if (ret) @@ -830,6 +827,9 @@ static int af9015_af9013_frontend_attach(struct dvb_usb_adapter *adap) /* copy firmware to 2nd demodulator */ if (state->dual_mode) { + /* Wait 2nd demodulator ready */ + msleep(100); + ret = af9015_copy_firmware(adap_to_d(adap)); if (ret) { dev_err(&adap_to_d(adap)->udev->dev, -- http://palosaari.fi/
[PATCH 15/15] af9013: refactor power control
Move power-up and power-down functionality to init/sleep ops and get rid of old function. Fixes and simplifies power-up functionality slightly. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 93 ++-- 1 file changed, 36 insertions(+), 57 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 40fd2ea..128d915 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -101,59 +101,6 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) return ret; } -static int af9013_power_ctrl(struct af9013_state *state, u8 onoff) -{ - struct i2c_client *client = state->client; - int ret; - unsigned int utmp; - - dev_dbg(&client->dev, "onoff %d\n", onoff); - - /* enable reset */ - ret = regmap_update_bits(state->regmap, 0xd417, 0x10, 0x10); - if (ret) - goto err; - - /* start reset mechanism */ - ret = regmap_write(state->regmap, 0xaeff, 0x01); - if (ret) - goto err; - - /* wait reset performs */ - ret = regmap_read_poll_timeout(state->regmap, 0xd417, utmp, - (utmp >> 1) & 0x01, 5000, 100); - if (ret) - goto err; - - if (!((utmp >> 1) & 0x01)) - return -ETIMEDOUT; - - if (onoff) { - /* clear reset */ - ret = regmap_update_bits(state->regmap, 0xd417, 0x02, 0x00); - if (ret) - goto err; - /* disable reset */ - ret = regmap_update_bits(state->regmap, 0xd417, 0x10, 0x00); - if (ret) - goto err; - /* power on */ - ret = regmap_update_bits(state->regmap, 0xd73a, 0x08, 0x00); - if (ret) - goto err; - } else { - /* power off */ - ret = regmap_update_bits(state->regmap, 0xd73a, 0x08, 0x08); - if (ret) - goto err; - } - - return 0; -err: - dev_dbg(&client->dev, "failed %d\n", ret); - return ret; -} - static int af9013_statistics_ber_unc_start(struct dvb_frontend *fe) { struct af9013_state *state = fe->demodulator_priv; @@ -889,8 +836,18 @@ static int af9013_init(struct dvb_frontend *fe) dev_dbg(&client->dev, "\n"); - /* power on */ - ret = af9013_power_ctrl(state, 1); + /* ADC on */ + ret = regmap_update_bits(state->regmap, 0xd73a, 0x08, 0x00); + if (ret) + goto err; + + /* Clear reset */ + ret = regmap_update_bits(state->regmap, 0xd417, 0x02, 0x00); + if (ret) + goto err; + + /* Disable reset */ + ret = regmap_update_bits(state->regmap, 0xd417, 0x10, 0x00); if (ret) goto err; @@ -1070,6 +1027,7 @@ static int af9013_sleep(struct dvb_frontend *fe) struct af9013_state *state = fe->demodulator_priv; struct i2c_client *client = state->client; int ret; + unsigned int utmp; dev_dbg(&client->dev, "\n"); @@ -1081,8 +1039,29 @@ static int af9013_sleep(struct dvb_frontend *fe) if (ret) goto err; - /* power off */ - ret = af9013_power_ctrl(state, 0); + /* Enable reset */ + ret = regmap_update_bits(state->regmap, 0xd417, 0x10, 0x10); + if (ret) + goto err; + + /* Start reset execution */ + ret = regmap_write(state->regmap, 0xaeff, 0x01); + if (ret) + goto err; + + /* Wait reset performs */ + ret = regmap_read_poll_timeout(state->regmap, 0xd417, utmp, + (utmp >> 1) & 0x01, 5000, 100); + if (ret) + goto err; + + if (!((utmp >> 1) & 0x01)) { + ret = -ETIMEDOUT; + goto err; + } + + /* ADC off */ + ret = regmap_update_bits(state->regmap, 0xd73a, 0x08, 0x08); if (ret) goto err; -- http://palosaari.fi/
[PATCH 02/15] af9013: move config values directly under driver state
It shorten, as typed chars, access to config values as there is one pointer less. Also, when config/platform data is passed to driver there could be some values that are not relevant to store state as such or not needed to store at all. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 62 ++-- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index b978002..7880a63 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -26,7 +26,14 @@ struct af9013_state { struct i2c_adapter *i2c; struct dvb_frontend fe; - struct af9013_config config; + u8 i2c_addr; + u32 clk; + u8 tuner; + u32 if_frequency; + u8 ts_mode; + bool spec_inv; + u8 api_version[4]; + u8 gpio[4]; /* tuner/demod RF and IF AGC limits used for signal strength calc */ u8 signal_strength_en, rf_50, rf_80, if_50, if_80; @@ -52,7 +59,7 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { - .addr = priv->config.i2c_addr, + .addr = priv->i2c_addr, .flags = 0, .len = 3 + len, .buf = buf, @@ -90,12 +97,12 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, u8 buf[3]; struct i2c_msg msg[2] = { { - .addr = priv->config.i2c_addr, + .addr = priv->i2c_addr, .flags = 0, .len = 3, .buf = buf, }, { - .addr = priv->config.i2c_addr, + .addr = priv->i2c_addr, .flags = I2C_M_RD, .len = len, .buf = val, @@ -124,7 +131,7 @@ static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val, int ret, i; u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0); - if ((priv->config.ts_mode == AF9013_TS_USB) && + if ((priv->ts_mode == AF9013_TS_USB) && ((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) { mbox |= ((len - 1) << 2); ret = af9013_wr_regs_i2c(priv, mbox, reg, val, len); @@ -146,7 +153,7 @@ static int af9013_rd_regs(struct af9013_state *priv, u16 reg, u8 *val, int len) int ret, i; u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(0 << 0); - if ((priv->config.ts_mode == AF9013_TS_USB) && + if ((priv->ts_mode == AF9013_TS_USB) && ((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) { mbox |= ((len - 1) << 2); ret = af9013_rd_regs_i2c(priv, mbox, reg, val, len); @@ -595,7 +602,7 @@ static int af9013_set_frontend(struct dvb_frontend *fe) /* program CFOE coefficients */ if (c->bandwidth_hz != state->bandwidth_hz) { for (i = 0; i < ARRAY_SIZE(coeff_lut); i++) { - if (coeff_lut[i].clock == state->config.clock && + if (coeff_lut[i].clock == state->clk && coeff_lut[i].bandwidth_hz == c->bandwidth_hz) { break; } @@ -615,24 +622,24 @@ static int af9013_set_frontend(struct dvb_frontend *fe) if (fe->ops.tuner_ops.get_if_frequency) fe->ops.tuner_ops.get_if_frequency(fe, &if_frequency); else - if_frequency = state->config.if_frequency; + if_frequency = state->if_frequency; dev_dbg(&state->i2c->dev, "%s: if_frequency=%d\n", __func__, if_frequency); sampling_freq = if_frequency; - while (sampling_freq > (state->config.clock / 2)) - sampling_freq -= state->config.clock; + while (sampling_freq > (state->clk / 2)) + sampling_freq -= state->clk; if (sampling_freq < 0) { sampling_freq *= -1; - spec_inv = state->config.spec_inv; + spec_inv = state->spec_inv; } else { - spec_inv = !state->config.spec_inv; + spec_inv = !state->spec_inv; } - freq_cw = af9013_div(state, sampling_freq, state->config.clock, + freq_cw = af9013_div(state, sampling_freq, state->clk, 23); if (spec_inv) @@ -1078,12 +1085,12 @@ static int af9013_init(struct dvb_frontend *fe) goto err; /* write API versio
[PATCH 14/15] af9013: refactor firmware download routine
Refactor firmware download routine. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 65 +--- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 63c532a..40fd2ea 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -1136,64 +1136,59 @@ static const struct dvb_frontend_ops af9013_ops; static int af9013_download_firmware(struct af9013_state *state) { struct i2c_client *client = state->client; - int ret, i, len, remaining; + int ret, i, len, rem; unsigned int utmp; - const struct firmware *fw; + u8 buf[4]; u16 checksum = 0; - u8 fw_params[4]; - u8 *fw_file = AF9013_FIRMWARE; + const struct firmware *firmware; + const char *name = AF9013_FIRMWARE; - msleep(100); - /* check whether firmware is already running */ + dev_dbg(&client->dev, "\n"); + + /* Check whether firmware is already running */ ret = regmap_read(state->regmap, 0x98be, &utmp); if (ret) goto err; dev_dbg(&client->dev, "firmware status %02x\n", utmp); - if (utmp == 0x0c) /* fw is running, no need for download */ + if (utmp == 0x0c) return 0; dev_info(&client->dev, "found a '%s' in cold state, will try to load a firmware\n", af9013_ops.info.name); - /* request the firmware, this will block and timeout */ - ret = request_firmware(&fw, fw_file, &client->dev); + /* Request the firmware, will block and timeout */ + ret = request_firmware(&firmware, name, &client->dev); if (ret) { dev_info(&client->dev, "firmware file '%s' not found %d\n", -fw_file, ret); +name, ret); goto err; } dev_info(&client->dev, "downloading firmware from file '%s'\n", -fw_file); - - /* calc checksum */ - for (i = 0; i < fw->size; i++) - checksum += fw->data[i]; +name); - fw_params[0] = checksum >> 8; - fw_params[1] = checksum & 0xff; - fw_params[2] = fw->size >> 8; - fw_params[3] = fw->size & 0xff; - - /* write fw checksum & size */ - ret = regmap_bulk_write(state->regmap, 0x50fc, fw_params, - sizeof(fw_params)); + /* Write firmware checksum & size */ + for (i = 0; i < firmware->size; i++) + checksum += firmware->data[i]; + buf[0] = (checksum >> 8) & 0xff; + buf[1] = (checksum >> 0) & 0xff; + buf[2] = (firmware->size >> 8) & 0xff; + buf[3] = (firmware->size >> 0) & 0xff; + ret = regmap_bulk_write(state->regmap, 0x50fc, buf, 4); if (ret) goto err_release_firmware; - #define FW_ADDR 0x5100 /* firmware start address */ - #define LEN_MAX 16 /* max packet size */ - for (remaining = fw->size; remaining > 0; remaining -= LEN_MAX) { - len = remaining; - if (len > LEN_MAX) - len = LEN_MAX; - + /* Download firmware */ + #define LEN_MAX 16 + for (rem = firmware->size; rem > 0; rem -= LEN_MAX) { + len = min(LEN_MAX, rem); ret = regmap_bulk_write(state->regmap, - FW_ADDR + fw->size - remaining, - &fw->data[fw->size - remaining], len); + 0x5100 + firmware->size - rem, + &firmware->data[firmware->size - rem], + len); if (ret) { dev_err(&client->dev, "firmware download failed %d\n", ret); @@ -1201,9 +1196,9 @@ static int af9013_download_firmware(struct af9013_state *state) } } - release_firmware(fw); + release_firmware(firmware); - /* request boot firmware */ + /* Boot firmware */ ret = regmap_write(state->regmap, 0xe205, 0x01); if (ret) goto err; @@ -1232,7 +1227,7 @@ static int af9013_download_firmware(struct af9013_state *state) return 0; err_release_firmware: - release_firmware(fw); + release_firmware(firmware); err: dev_dbg(&client->dev, "failed %d\n", ret); return ret; -- http://palosaari.fi/
[PATCH 01/15] af9015: use correct 7-bit i2c addresses
Driver was using wrong "8-bit" i2c addresses for demods and tuners. Internal demod i2c address was not set at all. These are needed to be fixed before proper i2c client binding is used. Signed-off-by: Antti Palosaari --- drivers/media/usb/dvb-usb-v2/af9015.c | 24 +--- drivers/media/usb/dvb-usb-v2/af9015.h | 4 ++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index caa1e61..138416c 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -36,7 +36,7 @@ static int af9015_ctrl_msg(struct dvb_usb_device *d, struct req_t *req) state->buf[0] = req->cmd; state->buf[1] = state->seq++; - state->buf[2] = req->i2c_addr; + state->buf[2] = req->i2c_addr << 1; state->buf[3] = req->addr >> 8; state->buf[4] = req->addr & 0xff; state->buf[5] = req->mbox; @@ -471,6 +471,8 @@ static int af9015_read_config(struct dvb_usb_device *d) if (d->udev->speed == USB_SPEED_FULL) state->dual_mode = 0; + state->af9013_config[0].i2c_addr = AF9015_I2C_DEMOD; + if (state->dual_mode) { /* read 2nd demodulator I2C address */ req.addr = AF9015_EEPROM_DEMOD2_I2C; @@ -478,7 +480,7 @@ static int af9015_read_config(struct dvb_usb_device *d) if (ret) goto error; - state->af9013_config[1].i2c_addr = val; + state->af9013_config[1].i2c_addr = val >> 1; } for (i = 0; i < state->dual_mode + 1; i++) { @@ -870,12 +872,12 @@ static int af9015_af9013_frontend_attach(struct dvb_usb_adapter *adap) } static struct mt2060_config af9015_mt2060_config = { - .i2c_address = 0xc0, + .i2c_address = 0x60, .clock_out = 0, }; static struct qt1010_config af9015_qt1010_config = { - .i2c_address = 0xc4, + .i2c_address = 0x62, }; static struct tda18271_config af9015_tda18271_config = { @@ -884,7 +886,7 @@ static struct tda18271_config af9015_tda18271_config = { }; static struct mxl5005s_config af9015_mxl5003_config = { - .i2c_address = 0xc6, + .i2c_address = 0x63, .if_freq = IF_FREQ_457HZ, .xtal_freq = CRYSTAL_FREQ_1600HZ, .agc_mode= MXL_SINGLE_AGC, @@ -901,7 +903,7 @@ static struct mxl5005s_config af9015_mxl5003_config = { }; static struct mxl5005s_config af9015_mxl5005_config = { - .i2c_address = 0xc6, + .i2c_address = 0x63, .if_freq = IF_FREQ_457HZ, .xtal_freq = CRYSTAL_FREQ_1600HZ, .agc_mode= MXL_SINGLE_AGC, @@ -918,12 +920,12 @@ static struct mxl5005s_config af9015_mxl5005_config = { }; static struct mc44s803_config af9015_mc44s803_config = { - .i2c_address = 0xc0, + .i2c_address = 0x60, .dig_out = 1, }; static struct tda18218_config af9015_tda18218_config = { - .i2c_address = 0xc0, + .i2c_address = 0x60, .i2c_wr_max = 21, /* max wr bytes AF9015 I2C adap can handle at once */ }; @@ -954,7 +956,7 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap) &af9015_qt1010_config) == NULL ? -ENODEV : 0; break; case AF9013_TUNER_TDA18271: - ret = dvb_attach(tda18271_attach, adap->fe[0], 0xc0, + ret = dvb_attach(tda18271_attach, adap->fe[0], 0x60, &adap_to_d(adap)->i2c_adap, &af9015_tda18271_config) == NULL ? -ENODEV : 0; break; @@ -975,7 +977,7 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap) &af9015_mxl5005_config) == NULL ? -ENODEV : 0; break; case AF9013_TUNER_ENV77H11D5: - ret = dvb_attach(dvb_pll_attach, adap->fe[0], 0xc0, + ret = dvb_attach(dvb_pll_attach, adap->fe[0], 0x60, &adap_to_d(adap)->i2c_adap, DVB_PLL_TDA665X) == NULL ? -ENODEV : 0; break; @@ -987,7 +989,7 @@ static int af9015_tuner_attach(struct dvb_usb_adapter *adap) case AF9013_TUNER_MXL5007T: ret = dvb_attach(mxl5007t_attach, adap->fe[0], &adap_to_d(adap)->i2c_adap, - 0xc0, &af9015_mxl5007t_config) == NULL ? -ENODEV : 0; + 0x60, &af9015_mxl5007t_config) == NULL ? -ENODEV : 0; break; case AF9013_TUNER_UNKNOWN: default: diff --git a/drivers/media/usb/dvb-usb-v2/af9015.h b/drivers/media/usb/dvb-usb-v2/af9015.h index 2dd9231..3a9d981 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.h +++ b/drivers/media/usb/dvb-usb-v2/af9015.h @@ -47,8 +47,8 @@ #define TS_USB20_MAX_PACKET_SIZE 512 #define TS_USB11_MAX_PACKET_SIZE 64 -#define AF9015_I2C_EEPROM 0xa0 -#define AF9015_I
[PATCH 12/15] af9013: remove unneeded register writes
Removed register writes are already chip defaults, are not required, are set later or belong to AF9015 USB interface. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 42 1 file changed, 42 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 6b86437..63c532a 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -894,11 +894,6 @@ static int af9013_init(struct dvb_frontend *fe) if (ret) goto err; - /* enable ADC */ - ret = regmap_write(state->regmap, 0xd73a, 0xa4); - if (ret) - goto err; - /* write API version to firmware */ ret = regmap_bulk_write(state->regmap, 0x9bf2, state->api_version, 4); if (ret) @@ -935,43 +930,6 @@ static int af9013_init(struct dvb_frontend *fe) if (ret) goto err; - /* set I2C master clock */ - ret = regmap_write(state->regmap, 0xd416, 0x14); - if (ret) - goto err; - - /* set 16 embx */ - ret = regmap_update_bits(state->regmap, 0xd700, 0x02, 0x02); - if (ret) - goto err; - - /* set no trigger */ - ret = regmap_update_bits(state->regmap, 0xd700, 0x04, 0x00); - if (ret) - goto err; - - /* set read-update bit for constellation */ - ret = regmap_update_bits(state->regmap, 0xd371, 0x02, 0x02); - if (ret) - goto err; - - /* settings for mp2if */ - if (state->ts_mode == AF9013_TS_MODE_USB) { - /* AF9015 split PSB to 1.5k + 0.5k */ - ret = regmap_update_bits(state->regmap, 0xd50b, 0x04, 0x04); - if (ret) - goto err; - } else { - /* AF9013 set mpeg to full speed */ - ret = regmap_update_bits(state->regmap, 0xd502, 0x10, 0x10); - if (ret) - goto err; - } - - ret = regmap_update_bits(state->regmap, 0xd520, 0x10, 0x10); - if (ret) - goto err; - /* load OFSM settings */ dev_dbg(&client->dev, "load ofsm settings\n"); len = ARRAY_SIZE(ofsm_init); -- http://palosaari.fi/
[PATCH 08/15] af9013: add dvbv5 cnr
Add support for DVBv5 CNR. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index a6b88ae..68091f2 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -228,6 +228,7 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe) { struct af9013_state *state = fe->demodulator_priv; struct i2c_client *client = state->client; + struct dtv_frontend_properties *c = &fe->dtv_property_cache; int ret, i, len; unsigned int utmp; u8 buf[3]; @@ -283,6 +284,9 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe) } state->snr = utmp * 10; /* dB/10 */ + c->cnr.stat[0].svalue = 1000 * utmp; + c->cnr.stat[0].scale = FE_SCALE_DECIBEL; + return 0; err: dev_dbg(&client->dev, "failed %d\n", ret); @@ -1508,6 +1512,7 @@ static int af9013_probe(struct i2c_client *client, { struct af9013_state *state; struct af9013_platform_data *pdata = client->dev.platform_data; + struct dtv_frontend_properties *c; int ret, i; u8 firmware_version[4]; static const struct regmap_bus regmap_bus = { @@ -1572,6 +1577,10 @@ static int af9013_probe(struct i2c_client *client, /* Setup callbacks */ pdata->get_dvb_frontend = af9013_get_dvb_frontend; + /* Init stats to indicate which stats are supported */ + c = &state->fe.dtv_property_cache; + c->cnr.len = 1; + dev_info(&client->dev, "Afatech AF9013 successfully attached\n"); dev_info(&client->dev, "firmware version: %d.%d.%d.%d\n", firmware_version[0], firmware_version[1], -- http://palosaari.fi/
[PATCH 05/15] af9013: fix logging
We can simplify logging as we now have a proper i2c client to pass for kernel dev_* logging functions. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 202 +-- 1 file changed, 100 insertions(+), 102 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index dd7ac0a..781e958 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -51,14 +51,15 @@ struct af9013_state { }; /* write multiple registers */ -static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, +static int af9013_wr_regs_i2c(struct af9013_state *state, u8 mbox, u16 reg, const u8 *val, int len) { + struct i2c_client *client = state->client; int ret; u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { - .addr = priv->client->addr, + .addr = state->client->addr, .flags = 0, .len = 3 + len, .buf = buf, @@ -66,9 +67,8 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, }; if (3 + len > sizeof(buf)) { - dev_warn(&priv->client->dev, -"%s: i2c wr reg=%04x: len=%d is too big!\n", -KBUILD_MODNAME, reg, len); + dev_warn(&client->dev, "i2c wr reg %04x, len %d, is too big!\n", +reg, len); return -EINVAL; } @@ -77,31 +77,32 @@ static int af9013_wr_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, buf[2] = mbox; memcpy(&buf[3], val, len); - ret = i2c_transfer(priv->client->adapter, msg, 1); + ret = i2c_transfer(state->client->adapter, msg, 1); if (ret == 1) { ret = 0; } else { - dev_warn(&priv->client->dev, "%s: i2c wr failed=%d reg=%04x " \ - "len=%d\n", KBUILD_MODNAME, ret, reg, len); + dev_warn(&client->dev, "i2c wr failed %d, reg %04x, len %d\n", +ret, reg, len); ret = -EREMOTEIO; } return ret; } /* read multiple registers */ -static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, +static int af9013_rd_regs_i2c(struct af9013_state *state, u8 mbox, u16 reg, u8 *val, int len) { + struct i2c_client *client = state->client; int ret; u8 buf[3]; struct i2c_msg msg[2] = { { - .addr = priv->client->addr, + .addr = state->client->addr, .flags = 0, .len = 3, .buf = buf, }, { - .addr = priv->client->addr, + .addr = state->client->addr, .flags = I2C_M_RD, .len = len, .buf = val, @@ -112,31 +113,31 @@ static int af9013_rd_regs_i2c(struct af9013_state *priv, u8 mbox, u16 reg, buf[1] = (reg >> 0) & 0xff; buf[2] = mbox; - ret = i2c_transfer(priv->client->adapter, msg, 2); + ret = i2c_transfer(state->client->adapter, msg, 2); if (ret == 2) { ret = 0; } else { - dev_warn(&priv->client->dev, "%s: i2c rd failed=%d reg=%04x " \ - "len=%d\n", KBUILD_MODNAME, ret, reg, len); + dev_warn(&client->dev, "i2c rd failed %d, reg %04x, len %d\n", +ret, reg, len); ret = -EREMOTEIO; } return ret; } /* write multiple registers */ -static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val, +static int af9013_wr_regs(struct af9013_state *state, u16 reg, const u8 *val, int len) { int ret, i; u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0); - if ((priv->ts_mode == AF9013_TS_USB) && + if ((state->ts_mode == AF9013_TS_USB) && ((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) { mbox |= ((len - 1) << 2); - ret = af9013_wr_regs_i2c(priv, mbox, reg, val, len); + ret = af9013_wr_regs_i2c(state, mbox, reg, val, len); } else { for (i = 0; i < len; i++) { - ret = af9013_wr_regs_i2c(priv, mbox, reg+i, val+i, 1); + ret = af9013_wr_regs_i2c(state, mbox, reg+i, val+i, 1); if (ret) goto err; } @@ -147,18 +148,18 @@ static int af9013_wr_regs(struct af9013_state *priv, u16 reg, const u8 *val, } /* read multiple registers */ -static int af9013_rd_regs(struct af9013_state *priv, u16 reg, u8 *val, int len) +static int af9013_rd_regs(struct af9013_state *state, u16 reg
[PATCH 04/15] af9033: use kernel 64-bit division
Replace own binary division with 64-bit multiply and division. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/af9013.c | 34 +++ drivers/media/dvb-frontends/af9013_priv.h | 1 + 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index f644182..dd7ac0a 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -277,33 +277,6 @@ static int af9013_set_gpio(struct af9013_state *state, u8 gpio, u8 gpioval) return ret; } -static u32 af9013_div(struct af9013_state *state, u32 a, u32 b, u32 x) -{ - u32 r = 0, c = 0, i; - - dev_dbg(&state->client->dev, "%s: a=%d b=%d x=%d\n", __func__, a, b, x); - - if (a > b) { - c = a / b; - a = a - c * b; - } - - for (i = 0; i < x; i++) { - if (a >= b) { - r += 1; - a -= b; - } - a <<= 1; - r <<= 1; - } - r = (c << (u32)x) + r; - - dev_dbg(&state->client->dev, "%s: a=%d b=%d x=%d r=%d r=%x\n", - __func__, a, b, x, r, r); - - return r; -} - static int af9013_power_ctrl(struct af9013_state *state, u8 onoff) { int ret, i; @@ -638,8 +611,8 @@ static int af9013_set_frontend(struct dvb_frontend *fe) spec_inv = !state->spec_inv; } - freq_cw = af9013_div(state, sampling_freq, state->clk, - 23); + freq_cw = DIV_ROUND_CLOSEST_ULL((u64)sampling_freq * 0x80, + state->clk); if (spec_inv) freq_cw = 0x80 - freq_cw; @@ -1108,11 +1081,10 @@ static int af9013_init(struct dvb_frontend *fe) return -EINVAL; } - adc_cw = af9013_div(state, state->clk, 100ul, 19); + adc_cw = div_u64((u64)state->clk * 0x8, 100); buf[0] = (adc_cw >> 0) & 0xff; buf[1] = (adc_cw >> 8) & 0xff; buf[2] = (adc_cw >> 16) & 0xff; - ret = af9013_wr_regs(state, 0xd180, buf, 3); if (ret) goto err; diff --git a/drivers/media/dvb-frontends/af9013_priv.h b/drivers/media/dvb-frontends/af9013_priv.h index 31d6538..97b5b0c 100644 --- a/drivers/media/dvb-frontends/af9013_priv.h +++ b/drivers/media/dvb-frontends/af9013_priv.h @@ -24,6 +24,7 @@ #include "dvb_frontend.h" #include "af9013.h" #include +#include #define AF9013_FIRMWARE "dvb-fe-af9013.fw" -- http://palosaari.fi/
[PATCH 06/15] af9013: convert to regmap api
Use regmap for register access. Own low level i2c read and write routines for regmap is still needed because chip uses single command byte in addition to typical i2c register access. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/Kconfig | 1 + drivers/media/dvb-frontends/af9013.c | 598 +++--- drivers/media/dvb-frontends/af9013_priv.h | 1 + 3 files changed, 294 insertions(+), 306 deletions(-) diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig index e8c6554..3a260b8 100644 --- a/drivers/media/dvb-frontends/Kconfig +++ b/drivers/media/dvb-frontends/Kconfig @@ -436,6 +436,7 @@ config DVB_TDA10048 config DVB_AF9013 tristate "Afatech AF9013 demodulator" depends on DVB_CORE && I2C + select REGMAP default m if !MEDIA_SUBDRV_AUTOSELECT help Say Y when you want to support this frontend. diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c index 781e958..70102c1 100644 --- a/drivers/media/dvb-frontends/af9013.c +++ b/drivers/media/dvb-frontends/af9013.c @@ -20,11 +20,9 @@ #include "af9013_priv.h" -/* Max transfer size done by I2C transfer functions */ -#define MAX_XFER_SIZE 64 - struct af9013_state { struct i2c_client *client; + struct regmap *regmap; struct dvb_frontend fe; u32 clk; u8 tuner; @@ -50,181 +48,6 @@ struct af9013_state { struct delayed_work statistics_work; }; -/* write multiple registers */ -static int af9013_wr_regs_i2c(struct af9013_state *state, u8 mbox, u16 reg, - const u8 *val, int len) -{ - struct i2c_client *client = state->client; - int ret; - u8 buf[MAX_XFER_SIZE]; - struct i2c_msg msg[1] = { - { - .addr = state->client->addr, - .flags = 0, - .len = 3 + len, - .buf = buf, - } - }; - - if (3 + len > sizeof(buf)) { - dev_warn(&client->dev, "i2c wr reg %04x, len %d, is too big!\n", -reg, len); - return -EINVAL; - } - - buf[0] = (reg >> 8) & 0xff; - buf[1] = (reg >> 0) & 0xff; - buf[2] = mbox; - memcpy(&buf[3], val, len); - - ret = i2c_transfer(state->client->adapter, msg, 1); - if (ret == 1) { - ret = 0; - } else { - dev_warn(&client->dev, "i2c wr failed %d, reg %04x, len %d\n", -ret, reg, len); - ret = -EREMOTEIO; - } - return ret; -} - -/* read multiple registers */ -static int af9013_rd_regs_i2c(struct af9013_state *state, u8 mbox, u16 reg, - u8 *val, int len) -{ - struct i2c_client *client = state->client; - int ret; - u8 buf[3]; - struct i2c_msg msg[2] = { - { - .addr = state->client->addr, - .flags = 0, - .len = 3, - .buf = buf, - }, { - .addr = state->client->addr, - .flags = I2C_M_RD, - .len = len, - .buf = val, - } - }; - - buf[0] = (reg >> 8) & 0xff; - buf[1] = (reg >> 0) & 0xff; - buf[2] = mbox; - - ret = i2c_transfer(state->client->adapter, msg, 2); - if (ret == 2) { - ret = 0; - } else { - dev_warn(&client->dev, "i2c rd failed %d, reg %04x, len %d\n", -ret, reg, len); - ret = -EREMOTEIO; - } - return ret; -} - -/* write multiple registers */ -static int af9013_wr_regs(struct af9013_state *state, u16 reg, const u8 *val, - int len) -{ - int ret, i; - u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(1 << 0); - - if ((state->ts_mode == AF9013_TS_USB) && - ((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) { - mbox |= ((len - 1) << 2); - ret = af9013_wr_regs_i2c(state, mbox, reg, val, len); - } else { - for (i = 0; i < len; i++) { - ret = af9013_wr_regs_i2c(state, mbox, reg+i, val+i, 1); - if (ret) - goto err; - } - } - -err: - return 0; -} - -/* read multiple registers */ -static int af9013_rd_regs(struct af9013_state *state, u16 reg, u8 *val, int len) -{ - int ret, i; - u8 mbox = (0 << 7)|(0 << 6)|(1 << 1)|(0 << 0); - - if ((state->ts_mode == AF9013_TS_USB) && - ((reg & 0xff00) != 0xff00) && ((reg & 0xff00) != 0xae00)) { - mbox |= ((len - 1) << 2); - ret = af9013_rd_regs_i2c(state, mbox, reg, val, len); - } else { - for (i = 0; i < len; i++) { - ret = af9013_rd_regs_i2c(state, mbo
[PATCH 09/15] af9015: fix and refactor i2c adapter algo logic
* fix write+read when write has more than one byte * remove lock, not needed on that case * remove useless i2c msg send loop, as we support only write, read and write+read as one go and nothing more Signed-off-by: Antti Palosaari --- drivers/media/usb/dvb-usb-v2/af9015.c | 153 ++ 1 file changed, 79 insertions(+), 74 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/af9015.c b/drivers/media/usb/dvb-usb-v2/af9015.c index 138416c..54c1d47 100644 --- a/drivers/media/usb/dvb-usb-v2/af9015.c +++ b/drivers/media/usb/dvb-usb-v2/af9015.c @@ -205,9 +205,9 @@ static int af9015_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msg[], { struct dvb_usb_device *d = i2c_get_adapdata(adap); struct af9015_state *state = d_to_priv(d); - int ret = 0, i = 0; + int ret; u16 addr; - u8 uninitialized_var(mbox), addr_len; + u8 mbox, addr_len; struct req_t req; /* @@ -232,84 +232,89 @@ Due to that the only way to select correct tuner is use demodulator I2C-gate. | addr 0x3a | | addr 0xc6 | || || */ - if (mutex_lock_interruptible(&d->i2c_mutex) < 0) - return -EAGAIN; - while (i < num) { - if (msg[i].addr == state->af9013_config[0].i2c_addr || - msg[i].addr == state->af9013_config[1].i2c_addr) { - addr = msg[i].buf[0] << 8; - addr += msg[i].buf[1]; - mbox = msg[i].buf[2]; - addr_len = 3; - } else { - addr = msg[i].buf[0]; - addr_len = 1; - /* mbox is don't care in that case */ - } + if (msg[0].len == 0 || msg[0].flags & I2C_M_RD) { + addr = 0x; + mbox = 0; + addr_len = 0; + } else if (msg[0].len == 1) { + addr = msg[0].buf[0]; + mbox = 0; + addr_len = 1; + } else if (msg[0].len == 2) { + addr = msg[0].buf[0] << 8|msg[0].buf[1] << 0; + mbox = 0; + addr_len = 2; + } else { + addr = msg[0].buf[0] << 8|msg[0].buf[1] << 0; + mbox = msg[0].buf[2]; + addr_len = 3; + } - if (num > i + 1 && (msg[i+1].flags & I2C_M_RD)) { - if (msg[i].len > 3 || msg[i+1].len > 61) { - ret = -EOPNOTSUPP; - goto error; - } - if (msg[i].addr == state->af9013_config[0].i2c_addr) - req.cmd = READ_MEMORY; - else - req.cmd = READ_I2C; - req.i2c_addr = msg[i].addr; - req.addr = addr; - req.mbox = mbox; - req.addr_len = addr_len; - req.data_len = msg[i+1].len; - req.data = &msg[i+1].buf[0]; - ret = af9015_ctrl_msg(d, &req); - i += 2; - } else if (msg[i].flags & I2C_M_RD) { - if (msg[i].len > 61) { - ret = -EOPNOTSUPP; - goto error; - } - if (msg[i].addr == state->af9013_config[0].i2c_addr) { - ret = -EINVAL; - goto error; - } + if (num == 1 && !(msg[0].flags & I2C_M_RD)) { + /* i2c write */ + if (msg[0].len > 21) { + ret = -EOPNOTSUPP; + goto err; + } + if (msg[0].addr == state->af9013_config[0].i2c_addr) + req.cmd = WRITE_MEMORY; + else + req.cmd = WRITE_I2C; + req.i2c_addr = msg[0].addr; + req.addr = addr; + req.mbox = mbox; + req.addr_len = addr_len; + req.data_len = msg[0].len-addr_len; + req.data = &msg[0].buf[addr_len]; + ret = af9015_ctrl_msg(d, &req); + } else if (num == 2 && !(msg[0].flags & I2C_M_RD) && + (msg[1].flags & I2C_M_RD)) { + /* i2c write + read */ + if (msg[0].len > 3 || msg[1].len > 61) { + ret = -EOPNOTSUPP; + goto err; + } + if (msg[0].addr == state->af9013_config[0].i2c_addr) + req.cmd = READ_MEMORY; + else req.cmd = READ_I2C; - req.i2c_addr = msg[i].addr; - req.addr = addr; -
Re: [PATCH 7/8] smiapp: Add support for flash, lens and EEPROM devices
Hi Sakari, [auto build test ERROR on linuxtv-media/master] [also build test ERROR on next-20170614] [cannot apply to robh/for-next v4.12-rc5] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Sakari-Ailus/Support-registering-lens-flash-and-EEPROM-devices/20170615-084016 base: git://linuxtv.org/media_tree.git master config: x86_64-randconfig-x000-201724 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/media/i2c/smiapp/smiapp-core.c: In function 'smiapp_unregistered': >> drivers/media/i2c/smiapp/smiapp-core.c:2578:2: error: implicit declaration >> of function 'v4l2_async_subnotifier_unregister' >> [-Werror=implicit-function-declaration] v4l2_async_subnotifier_unregister(&sensor->notifier); ^ drivers/media/i2c/smiapp/smiapp-core.c: In function 'smiapp_registered': >> drivers/media/i2c/smiapp/smiapp-core.c:2607:9: error: implicit declaration >> of function 'v4l2_async_subnotifier_register' >> [-Werror=implicit-function-declaration] rval = v4l2_async_subnotifier_register(subdev, &sensor->notifier); ^~~ cc1: some warnings being treated as errors vim +/v4l2_async_subnotifier_unregister +2578 drivers/media/i2c/smiapp/smiapp-core.c 2572 struct smiapp_sensor *sensor = to_smiapp_sensor(subdev); 2573 unsigned int i; 2574 2575 for (i = 1; i < sensor->ssds_used; i++) 2576 v4l2_device_unregister_subdev(&sensor->ssds[i].sd); 2577 > 2578 v4l2_async_subnotifier_unregister(&sensor->notifier); 2579 } 2580 2581 static int smiapp_registered(struct v4l2_subdev *subdev) 2582 { 2583 struct smiapp_sensor *sensor = to_smiapp_sensor(subdev); 2584 int rval; 2585 2586 if (sensor->scaler) { 2587 rval = smiapp_register_subdev( 2588 sensor, sensor->binner, sensor->scaler, 2589 SMIAPP_PAD_SRC, SMIAPP_PAD_SINK, 2590 MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE); 2591 if (rval < 0) 2592 return rval; 2593 } 2594 2595 rval = smiapp_register_subdev( 2596 sensor, sensor->pixel_array, sensor->binner, 2597 SMIAPP_PA_PAD_SRC, SMIAPP_PAD_SINK, 2598 MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE); 2599 if (rval) 2600 goto out_err; 2601 2602 if (!sensor->notifier.num_subdevs) 2603 return 0; 2604 2605 sensor->notifier.bound = smiapp_subdev_notifier_bound; 2606 sensor->notifier.complete = smiapp_subdev_notifier_complete; > 2607 rval = v4l2_async_subnotifier_register(subdev, > &sensor->notifier); 2608 if (rval) 2609 goto out_err; 2610 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
RE: [PATCH 11/12] intel-ipu3: Add imgu v4l2 driver
Hi, Hans and Sakari, Thanks a lot for the code review, I attached the topology print FYI. Regards, Yong From: Sakari Ailus [sakari.ai...@iki.fi] Sent: Friday, June 09, 2017 2:20 AM To: Hans Verkuil Cc: Zhi, Yong; linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian Xu; tf...@chromium.org; Mani, Rajmohan; Toivonen, Tuukka Subject: Re: [PATCH 11/12] intel-ipu3: Add imgu v4l2 driver Hi Hans, On Tue, Jun 06, 2017 at 11:08:07AM +0200, Hans Verkuil wrote: > > + /* Initialize vdev */ > > + strlcpy(vdev->name, node->name, sizeof(vdev->name)); > > + vdev->release = video_device_release_empty; > > + vdev->fops = &m2m2->v4l2_file_ops; > > + vdev->ioctl_ops = &ipu3_v4l2_ioctl_ops; > > + vdev->lock = &node->lock; > > + vdev->v4l2_dev = &m2m2->v4l2_dev; > > + vdev->queue = &node->vbq; > > + vdev->vfl_dir = node->output ? VFL_DIR_TX : VFL_DIR_RX; > > Why have two video nodes (one tx, one rx) instead of a single m2m device > node? > > I'm not saying this is wrong, I just like to know the rationale for this > design. There are a bunch of outputs from the same input stream. Also the parameters are needed to process the frame, I think there are two OUTPUT devices and five CAPTURE devices. Yong: could you send the media graph of the device, please? -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk Media controller API version 0.1.0 Media device information driver ipu3-imgu model ipu3-imgu serial bus info:00:05.0 hw revision 0x0 driver version 4.12.0 Device topology - entity 1: ipu3-imgu:0 (8 pads, 8 links) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev0 pad0:Sink stream:0[fmt:UYVY2X8/352x288] link: <- "input":0 [ENABLED,IMMUTABLE] pad1:Sink stream:0[fmt:UYVY2X8/352x288] link: <- "parameters":0 [] pad2:Source stream:0[fmt:UYVY2X8/352x288] link: -> "output":0 [] pad3:Source stream:0[fmt:UYVY2X8/352x288] link: -> "viewfinder":0 [] pad4:Source stream:0[fmt:UYVY2X8/352x288] link: -> "postview":0 [] pad5:Source stream:0[fmt:UYVY2X8/352x288] link: -> "3a stat":0 [] pad6:Source stream:0[fmt:UYVY2X8/352x288] link: -> "dvs stat":0 [] pad7:Source stream:0[fmt:UYVY2X8/352x288] link: -> "lace stat":0 [] - entity 2: input (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video0 pad0:Source link: -> "ipu3-imgu:0":0 [ENABLED,IMMUTABLE] - entity 3: parameters (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video1 pad0:Source link: -> "ipu3-imgu:0":1 [] - entity 4: output (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video2 pad0:Sink link: <- "ipu3-imgu:0":2 [] - entity 5: viewfinder (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video3 pad0:Sink link: <- "ipu3-imgu:0":3 [] - entity 6: postview (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video4 pad0:Sink link: <- "ipu3-imgu:0":4 [] - entity 7: 3a stat (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video5 pad0:Sink link: <- "ipu3-imgu:0":5 [] - entity 8: dvs stat (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video6 pad0:Sink link: <- "ipu3-imgu:0":6 [] - entity 9: lace stat (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video7 pad0:Sink link: <- "ipu3-imgu:0":7 [] imgu-topology.dot Description: imgu-topology.dot
Re: [PATCH v3 1/1] i2c: Add Omnivision OV5670 5M sensor support
Hi Chiranjeevi, I presume the register lists in the driver assume a particular external clock frequency? That was the case for the ov13848 driver, too. Could you check as well that the "clock-frequency" property has the desired value, as the latest ov13848 driver does? A few comments below... On Tue, Jun 13, 2017 at 06:39:08PM -0700, chiranjeevi.rap...@intel.com wrote: > From: Chiranjeevi Rapolu > > Provides single source pad with up to 2592x1944 pixels at 10-bit raw > bayer format over MIPI CSI2 two lanes at 640Mbps/lane. > The driver supports following features: > - up to 30fps at 5M pixels > - manual exposure > - digital/analog gain > - V-blank/H-blank > - test pattern > - media controller > - runtime pm > > Signed-off-by: Chiranjeevi Rapolu > --- > drivers/media/i2c/Kconfig | 12 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/ov5670.c | 2591 > > 3 files changed, 2604 insertions(+) > create mode 100644 drivers/media/i2c/ov5670.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index c380e24..55e1877 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -573,6 +573,18 @@ config VIDEO_OV5647 > To compile this driver as a module, choose M here: the > module will be called ov5647. > > +config VIDEO_OV5670 > + tristate "OmniVision OV5670 sensor support" > + depends on I2C && VIDEO_V4L2 > + depends on MEDIA_CAMERA_SUPPORT > + select V4L2_FWNODE > + ---help--- > + This is a Video4Linux2 sensor-level driver for the OmniVision > + OV5670 camera. > + > + To compile this driver as a module, choose M here: the > + module will be called ov5670. > + > config VIDEO_OV7640 > tristate "OmniVision OV7640 sensor support" > depends on I2C && VIDEO_V4L2 > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index 62323ec..de95651 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -60,6 +60,7 @@ obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o > obj-$(CONFIG_VIDEO_OV2640) += ov2640.o > obj-$(CONFIG_VIDEO_OV5645) += ov5645.o > obj-$(CONFIG_VIDEO_OV5647) += ov5647.o > +obj-$(CONFIG_VIDEO_OV5670) += ov5670.o > obj-$(CONFIG_VIDEO_OV7640) += ov7640.o > obj-$(CONFIG_VIDEO_OV7670) += ov7670.o > obj-$(CONFIG_VIDEO_OV9650) += ov9650.o > diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c > new file mode 100644 > index 000..64bb858 > --- /dev/null > +++ b/drivers/media/i2c/ov5670.c > @@ -0,0 +1,2591 @@ > +/* > + * Copyright (c) 2017 Intel Corporation. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License version > + * 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define OV5670_REG_CHIP_ID 0x300a > +#define OV5670_CHIP_ID 0x005670 > + > +#define OV5670_REG_MODE_SELECT 0x0100 > +#define OV5670_MODE_STANDBY 0x00 > +#define OV5670_MODE_STREAMING0x01 > + > +#define OV5670_REG_SOFTWARE_RST 0x0103 > +#define OV5670_SOFTWARE_RST 0x01 > + > +/* vertical-timings from sensor */ > +#define OV5670_REG_VTS 0x380e > +#define OV5670_VTS_30FPS 0x0808 /* default for 30 fps */ > +#define OV5670_VTS_MAX 0x7fff > +#define OV5670_VBLANK_MIN56 > + > +/* horizontal-timings from sensor */ > +#define OV5670_MAX_PPL 3408/* Maximum pixels per > line */ > + > +/* Exposure controls from sensor */ > +#define OV5670_REG_EXPOSURE 0x3500 > +#define OV5670_EXPOSURE_MIN 0 > +#define OV5670_EXPOSURE_MAX 1048575 > +#define OV5670_EXPOSURE_STEP1 > +#define OV5670_EXPOSURE_DEFAULT 31872 > + > +/* Analog gain controls from sensor */ > +#define OV5670_REG_ANALOG_GAIN 0x3508 > +#define ANALOG_GAIN_MIN 0 > +#define ANALOG_GAIN_MAX 8191 > +#define ANALOG_GAIN_STEP1 > +#define ANALOG_GAIN_DEFAULT 128 > + > +/* Digital gain controls from sensor */ > +#define OV5670_REG_R_DGTL_GAIN 0x5032 > +#define OV5670_REG_G_DGTL_GAIN 0x5034 > +#define OV5670_REG_B_DGTL_GAIN 0x5036 > +#define OV5670_DGTL_GAIN_MIN 0 > +#define OV5670_DGTL_GAIN_MAX 4095 > +#define OV5670_DGTL_GAIN_STEP1 > +#define OV5670_DGTL_GAIN_DEFAULT 1024 > + > +/* Test Pattern Control */ >
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Ahoy! On Thu, Jun 15, 2017 at 12:28:33AM +0200, Pavel Machek wrote: > Hi! > > > Thanks for the review! > > You are welcome :-). > > > On Wed, Jun 14, 2017 at 11:39:41PM +0200, Pavel Machek wrote: > > > Hi! > > > > > > > From: Sakari Ailus > > > > > > That address no longer works, right? > > > > Why wouldn't it work? Or... do you know something I don't? :-) > > Aha. I thought I was removing it from source files because it was no > longer working, but maybe I'm misremembering? That was probably my @maxwell.research.nokia.com address. :-) There are no occurrences of that in the kernel source anymore. > > > > > +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool > > > > is_flash, > > > > + unsigned int ua) > > > > +{ > > > > + struct { > > > > + unsigned int min; > > > > + unsigned int max; > > > > + unsigned int step; > > > > + } __mms[] = { > > > > + { > > > > + AS_TORCH_INTENSITY_MIN, > > > > + flash->cfg.assist_max_ua, > > > > + AS_TORCH_INTENSITY_STEP > > > > + }, > > > > + { > > > > + AS_FLASH_INTENSITY_MIN, > > > > + flash->cfg.flash_max_ua, > > > > + AS_FLASH_INTENSITY_STEP > > > > + }, > > > > + }, *mms = &__mms[is_flash]; > > > > + > > > > + if (ua < mms->min) > > > > + ua = mms->min; > > > > > > That's some... seriously interesting code. And you are forcing gcc to > > > create quite interesting structure on stack. Would it be easier to do > > > normal if()... without this magic? > > > > > > > + struct v4l2_flash_config cfg = { > > > > + .torch_intensity = { > > > > + .min = AS_TORCH_INTENSITY_MIN, > > > > + .max = flash->cfg.assist_max_ua, > > > > + .step = AS_TORCH_INTENSITY_STEP, > > > > + .val = flash->cfg.assist_max_ua, > > > > + }, > > > > + .indicator_intensity = { > > > > + .min = AS_INDICATOR_INTENSITY_MIN, > > > > + .max = flash->cfg.indicator_max_ua, > > > > + .step = AS_INDICATOR_INTENSITY_STEP, > > > > + .val = flash->cfg.indicator_max_ua, > > > > + }, > > > > + }; > > > > > > Ugh. And here you have copy of the above struct, + .val. Can it be > > > somehow de-duplicated? > > > > The flash_brightness_set callback uses micro-Amps as the unit and the driver > > needs to convert that to its own specific units. Yeah, there would be > > probably an easier way, too. But that'd likely require changes to the LED > > flash class. > > Can as3645a_current_to_reg just access struct v4l2_flash_config so > that it does not have to recreate its look-alike on the fly? struct v4l2_flash_config is only needed as an argument for v4l2_flash_init(). I'll split that into two functions in this occasion, it'll be nicer. We now have more or less the same conversion implemented in three or so times, there have to be ways to make that easier for drivers. I think that could be done later, as well as adding support for checking the flash strobe status. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v3 1/3] videodev2.h, v4l2-ioctl: add IPU3 raw10 color format
On Wed, Jun 14, 2017 at 02:48:40PM +0100, Alan Cox wrote: > On Tue, 13 Jun 2017 15:17:14 -0500 > Yong Zhi wrote: > > > Add IPU3 specific formats: > > > > V4L2_PIX_FMT_IPU3_SBGGR10 > > V4L2_PIX_FMT_IPU3_SGBRG10 > > V4L2_PIX_FMT_IPU3_SGRBG10 > > V4L2_PIX_FMT_IPU3_SRGGB10 > > As I said before these are just more bitpacked bayer formats with no > reason to encode them as IPUv3 specific names. I must have missed that comment --- the format is pretty unusual still. Basically it rams as much pixels into a 256-bit DMA word as there's room and then leaves the rest of the DMA word empty (6 bits in this case). The newer IPUs do not use this format AFAIK (they do use other unusual formats though). I haven't seen such formats being used by other non-IPU hardware either. I think I'd keep this IPU specific unless there's an indication the same formats might be used elsewhere. The other packed formats that have been defined in V4L2 are hardware independent. Cc Hans, too. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Hi! > Thanks for the review! You are welcome :-). > On Wed, Jun 14, 2017 at 11:39:41PM +0200, Pavel Machek wrote: > > Hi! > > > > > From: Sakari Ailus > > > > That address no longer works, right? > > Why wouldn't it work? Or... do you know something I don't? :-) Aha. I thought I was removing it from source files because it was no longer working, but maybe I'm misremembering? > > > +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool > > > is_flash, > > > +unsigned int ua) > > > +{ > > > + struct { > > > + unsigned int min; > > > + unsigned int max; > > > + unsigned int step; > > > + } __mms[] = { > > > + { > > > + AS_TORCH_INTENSITY_MIN, > > > + flash->cfg.assist_max_ua, > > > + AS_TORCH_INTENSITY_STEP > > > + }, > > > + { > > > + AS_FLASH_INTENSITY_MIN, > > > + flash->cfg.flash_max_ua, > > > + AS_FLASH_INTENSITY_STEP > > > + }, > > > + }, *mms = &__mms[is_flash]; > > > + > > > + if (ua < mms->min) > > > + ua = mms->min; > > > > That's some... seriously interesting code. And you are forcing gcc to > > create quite interesting structure on stack. Would it be easier to do > > normal if()... without this magic? > > > > > + struct v4l2_flash_config cfg = { > > > + .torch_intensity = { > > > + .min = AS_TORCH_INTENSITY_MIN, > > > + .max = flash->cfg.assist_max_ua, > > > + .step = AS_TORCH_INTENSITY_STEP, > > > + .val = flash->cfg.assist_max_ua, > > > + }, > > > + .indicator_intensity = { > > > + .min = AS_INDICATOR_INTENSITY_MIN, > > > + .max = flash->cfg.indicator_max_ua, > > > + .step = AS_INDICATOR_INTENSITY_STEP, > > > + .val = flash->cfg.indicator_max_ua, > > > + }, > > > + }; > > > > Ugh. And here you have copy of the above struct, + .val. Can it be > > somehow de-duplicated? > > The flash_brightness_set callback uses micro-Amps as the unit and the driver > needs to convert that to its own specific units. Yeah, there would be > probably an easier way, too. But that'd likely require changes to the LED > flash class. Can as3645a_current_to_reg just access struct v4l2_flash_config so that it does not have to recreate its look-alike on the fly? Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH 00/12] Intel IPU3 ImgU patchset
Hi Alan, On Mon, Jun 05, 2017 at 09:46:59PM +0100, Alan Cox wrote: > > data structures used by the firmware and the hardware. On top of that, > > the algorithms require highly specialized user space to make meaningful > > use of them. For these reasons it has been chosen video buffers to pass > > the parameters to the device. > > You should provide a pointer to the relevant userspace here as well. > People need that to evaluate the interface. I know... there will be some user space software to use this interface but it's unfortunately not available yet. > > > 6 and 7 provide some utility functions and manage IPU3 fw download and > > install. > > and a pointer to the firmware (which ideally should go into the standard > Linux firmware git) Good question. Let me see what I can find. > > Otherwise this is so much nicer than the IPUv2 code! Thanks! -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Hi Pavel, Thanks for the review! On Wed, Jun 14, 2017 at 11:39:41PM +0200, Pavel Machek wrote: > Hi! > > > From: Sakari Ailus > > That address no longer works, right? Why wouldn't it work? Or... do you know something I don't? :-) > > > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > > is based on that. > > > > Signed-off-by: Sakari Ailus > > > + * Based on drivers/media/i2c/as3645a.c. > > + * > > + * Contact: Sakari Ailus > > So I believe it should not be here. > > > +/* > > + * as3645a_set_config - Set flash configuration registers > > + * @flash: The flash > > + * > > /** for linuxdoc? Fixed. > > > + struct as3645a *flash = fled_to_as3645a(fled); > > + int rval; > > + > > + /* NOTE: reading register clear fault status */ > > clears. Fixed. > > > +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool > > is_flash, > > + unsigned int ua) > > +{ > > + struct { > > + unsigned int min; > > + unsigned int max; > > + unsigned int step; > > + } __mms[] = { > > + { > > + AS_TORCH_INTENSITY_MIN, > > + flash->cfg.assist_max_ua, > > + AS_TORCH_INTENSITY_STEP > > + }, > > + { > > + AS_FLASH_INTENSITY_MIN, > > + flash->cfg.flash_max_ua, > > + AS_FLASH_INTENSITY_STEP > > + }, > > + }, *mms = &__mms[is_flash]; > > + > > + if (ua < mms->min) > > + ua = mms->min; > > That's some... seriously interesting code. And you are forcing gcc to > create quite interesting structure on stack. Would it be easier to do > normal if()... without this magic? > > > + struct v4l2_flash_config cfg = { > > + .torch_intensity = { > > + .min = AS_TORCH_INTENSITY_MIN, > > + .max = flash->cfg.assist_max_ua, > > + .step = AS_TORCH_INTENSITY_STEP, > > + .val = flash->cfg.assist_max_ua, > > + }, > > + .indicator_intensity = { > > + .min = AS_INDICATOR_INTENSITY_MIN, > > + .max = flash->cfg.indicator_max_ua, > > + .step = AS_INDICATOR_INTENSITY_STEP, > > + .val = flash->cfg.indicator_max_ua, > > + }, > > + }; > > Ugh. And here you have copy of the above struct, + .val. Can it be > somehow de-duplicated? The flash_brightness_set callback uses micro-Amps as the unit and the driver needs to convert that to its own specific units. Yeah, there would be probably an easier way, too. But that'd likely require changes to the LED flash class. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
[PATCH v2 09/12] intel-ipu3: css hardware setup
Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-css.c | 515 drivers/media/pci/intel/ipu3/ipu3-css.h | 5 + 2 files changed, 520 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.c diff --git a/drivers/media/pci/intel/ipu3/ipu3-css.c b/drivers/media/pci/intel/ipu3/ipu3-css.c new file mode 100644 index 000..675be91 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-css.c @@ -0,0 +1,515 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include "ipu3-css.h" +#include "ipu3-css-fw.h" +#include "ipu3-tables.h" + +/* IRQ configuration */ +#define IMGU_IRQCTRL_IRQ_MASK (IMGU_IRQCTRL_IRQ_SP1 | \ +IMGU_IRQCTRL_IRQ_SP2 | \ +IMGU_IRQCTRL_IRQ_SW_PIN(0) | \ +IMGU_IRQCTRL_IRQ_SW_PIN(1)) + +static void writes(void *mem, ssize_t len, void __iomem *reg) +{ + while (len >= 4) { + writel(*(u32 *)mem, reg); + mem += 4; + reg += 4; + len -= 4; + } +} + +/*** css hw ***/ + +/* Wait until register `reg', masked with `mask', becomes `cmp' */ +static int ipu3_css_hw_wait(struct ipu3_css *css, int reg, u32 mask, u32 cmp) +{ + static const unsigned int delay = 1000; + unsigned int maxloops = 100 / 10 / delay; + + do { + if ((readl(css->base + reg) & mask) == cmp) + return 0; + usleep_range(delay, 2 * delay); + } while (--maxloops); + + return -EIO; +} + +/* Initialize the IPU3 CSS hardware and associated h/w blocks */ + +int ipu3_css_set_powerup(struct ipu3_css *css) +{ + struct device *dev = css->dev; + void __iomem *const base = css->base; + u32 pm_ctrl, state; + + /* Clear the CSS busy signal */ + readl(base + IMGU_REG_GP_BUSY); + writel(0, base + IMGU_REG_GP_BUSY); + + /* Wait for idle signal */ + if (ipu3_css_hw_wait(css, IMGU_REG_STATE, IMGU_STATE_IDLE_STS, + IMGU_STATE_IDLE_STS)) + dev_warn(dev, "failed to set CSS idle\n"); + + /* Reset the css */ + writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_FORCE_RESET, + base + IMGU_REG_PM_CTRL); + + usleep_range(200, 300); + + /** Prepare CSS */ + + pm_ctrl = readl(base + IMGU_REG_PM_CTRL); + state = readl(base + IMGU_REG_STATE); + + dev_dbg(dev, "CSS pm_ctrl 0x%x state 0x%x (power %s)\n", + pm_ctrl, state, state & IMGU_STATE_POWER_DOWN ? "down" : "up"); + + /* Power up CSS using wrapper */ + if (state & IMGU_STATE_POWER_DOWN) { + writel(IMGU_PM_CTRL_RACE_TO_HALT | IMGU_PM_CTRL_START, + base + IMGU_REG_PM_CTRL); + if (ipu3_css_hw_wait(css, IMGU_REG_PM_CTRL, + IMGU_PM_CTRL_START, 0)) + dev_warn(dev, "failed to power up CSS\n"); + usleep_range(2000, 3000); + } else { + writel(IMGU_PM_CTRL_RACE_TO_HALT, base + IMGU_REG_PM_CTRL); + } + + /* Set the busy bit */ + writel(readl(base + IMGU_REG_GP_BUSY) | 1, base + IMGU_REG_GP_BUSY); + + return 0; +} + +int ipu3_css_set_powerdown(struct ipu3_css *css) +{ + struct device *dev = css->dev; + void __iomem *const base = css->base; + + /* Clear the CSS busy signal */ + readl(base + IMGU_REG_GP_BUSY); + writel(0, base + IMGU_REG_GP_BUSY); + + /* Wait for idle signal */ + if (ipu3_css_hw_wait(css, IMGU_REG_STATE, IMGU_STATE_IDLE_STS, + IMGU_STATE_IDLE_STS)) + dev_warn(dev, "failed to set CSS idle\n"); + + /* Reset the css */ + writel(readl(base + IMGU_REG_PM_CTRL) | IMGU_PM_CTRL_CSS_PWRDN, + base + IMGU_REG_PM_CTRL); + + return 0; +} + +static int ipu3_css_hw_init(struct ipu3_css *css) +{ + /* For checking that streaming monitor statuses are valid */ + static const struct { + u32 reg; + u32 mask; + const char *name; + } stream_monitors[] = { + { + IMGU_REG_GP_SP1_STRMON_STAT, + IMGU_GP_STRMON_STAT_ISP_PORT_SP12ISP, + "ISP0 to SP0" + }, { + IMGU_REG_GP_ISP_STRMON_STA
[PATCH v2 08/12] intel-ipu3: params: compute and program ccs
A collection of routines that are mainly responsible to calculate the acc parameters. Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-css-params.c | 3119 drivers/media/pci/intel/ipu3/ipu3-css-params.h | 105 + drivers/media/pci/intel/ipu3/ipu3-css.h| 92 + 3 files changed, 3316 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-params.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-params.h diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-params.c b/drivers/media/pci/intel/ipu3/ipu3-css-params.c new file mode 100644 index 000..0dcdfed --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-css-params.c @@ -0,0 +1,3119 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include + +#include "ipu3-abi.h" +#include "ipu3-css.h" +#include "ipu3-css-fw.h" +#include "ipu3-css-params.h" +#include "ipu3-tables.h" + +static unsigned int ipu3_css_scaler_get_exp(unsigned int counter, + unsigned int divider) +{ + unsigned int i = 0; + + while (counter <= divider / 2) { + divider /= 2; + i++; + } + + return i; +} + +/* Set up the CSS scaler look up table */ +static void ipu3_css_scaler_setup_lut(unsigned int taps, + unsigned int input_width, + unsigned int output_width, + int phase_step_correction, + const int *coeffs, + unsigned int coeffs_size, + s8 coeff_lut[IMGU_SCALER_PHASES * + IMGU_SCALER_FILTER_TAPS], + struct ipu3_css_scaler_info *info) +{ + int tap; + int phase; + int exponent = ipu3_css_scaler_get_exp(output_width, input_width); + int mantissa = (1 << exponent) * output_width; + unsigned int phase_step = 0; + int phase_sum_left = 0; + int phase_sum_right = 0; + + for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) { + for (tap = 0; tap < taps; tap++) { + /* flip table to for convolution reverse indexing */ + s64 coeff = coeffs[coeffs_size - + ((tap * (coeffs_size / taps)) + + phase) - 1]; + coeff *= mantissa; + coeff /= input_width; + + /* Add +"0.5" */ + coeff += 1 << (IMGU_SCALER_COEFF_BITS - 1); + coeff >>= IMGU_SCALER_COEFF_BITS; + + coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap] = + coeff; + } + } + + phase_step = IMGU_SCALER_PHASES * + (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF) + * output_width / input_width; + phase_step += phase_step_correction; + phase_sum_left = (taps / 2 * IMGU_SCALER_PHASES * + (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF)) + - (1 << (IMGU_SCALER_PHASE_COUNTER_PREC_REF - 1)); + phase_sum_right = (taps / 2 * IMGU_SCALER_PHASES * + (1 << IMGU_SCALER_PHASE_COUNTER_PREC_REF)) + + (1 << (IMGU_SCALER_PHASE_COUNTER_PREC_REF - 1)); + + info->exp_shift = IMGU_SCALER_MAX_EXPONENT_SHIFT - exponent; + info->pad_left = (phase_sum_left % phase_step == 0) ? + phase_sum_left / phase_step - 1 : phase_sum_left / phase_step; + info->pad_right = (phase_sum_right % phase_step == 0) ? + phase_sum_right / phase_step - 1 : phase_sum_right / phase_step; + info->phase_init = phase_sum_left - phase_step * info->pad_left; + info->phase_step = phase_step; + info->crop_left = taps - 1; + info->crop_top = taps - 1; +} + +/* + * Calculates the exact output image width/height, based on phase_step setting + * (must be perfectly aligned with hardware). + */ +static unsigned int ipu3_css_scaler_calc_scaled_output(unsigned int input, + struct ipu3_css_scaler_info *info) +{ + unsigned int arg1 = input * info->phase_step + + (1 - IMGU_SCALER_TAPS_Y / 2) * + IMGU_SCALER_FIR_PHASES - IMGU_SCALER_FIR_PHASES +
[PATCH v2 06/12] intel-ipu3: css: imgu dma buff pool
The pools are used to store previous parameters set by user with the parameter queue. Due to pipelining, there needs to be multiple sets (up to four) of parameters which are queued in a host-to-sp queue. Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-css-pool.c | 129 +++ drivers/media/pci/intel/ipu3/ipu3-css-pool.h | 53 +++ 2 files changed, 182 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-pool.h diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.c b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c new file mode 100644 index 000..436ecd8 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.c @@ -0,0 +1,129 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include + +#include "ipu3-css-pool.h" + +int ipu3_css_dma_alloc(struct device *dev, + struct ipu3_css_map *map, size_t size) +{ + if (size == 0) { + map->vaddr = NULL; + return 0; + } + + map->vaddr = dma_alloc_coherent(dev, size, &map->daddr, GFP_KERNEL); + if (!map->vaddr) + return -ENOMEM; + map->size = size; + + return 0; +} + +void ipu3_css_dma_free(struct device *dev, struct ipu3_css_map *map) +{ + if (map->vaddr) + dma_free_coherent(dev, map->size, map->vaddr, map->daddr); + map->vaddr = NULL; +} + +void ipu3_css_pool_cleanup(struct device *dev, struct ipu3_css_pool *pool) +{ + int i; + + for (i = 0; i < IPU3_CSS_POOL_SIZE; i++) + ipu3_css_dma_free(dev, &pool->entry[i].param); +} + +int ipu3_css_pool_init(struct device *dev, struct ipu3_css_pool *pool, int size) +{ + int i; + + for (i = 0; i < IPU3_CSS_POOL_SIZE; i++) { + pool->entry[i].framenum = INT_MIN; + if (ipu3_css_dma_alloc(dev, &pool->entry[i].param, size)) + goto fail; + } + + pool->last = IPU3_CSS_POOL_SIZE; + + return 0; + +fail: + ipu3_css_pool_cleanup(dev, pool); + return -ENOMEM; +} + +/* + * Check that the following call to pool_get succeeds. + * Return negative on error. + */ +static int ipu3_css_pool_check(struct ipu3_css_pool *pool, long framenum) +{ + /* Get the oldest entry */ + int n = (pool->last + 1) % IPU3_CSS_POOL_SIZE; + + /* +* pool->entry[n].framenum stores the frame number where that +* entry was allocated. If that was allocated more than POOL_SIZE +* frames back, it is old enough that we know it is no more in +* use by firmware. +*/ + if (pool->entry[n].framenum + IPU3_CSS_POOL_SIZE > framenum) + return -ENOSPC; + + return n; +} + +/* + * Allocate a new parameter from pool at frame number `framenum'. + * Release the oldest entry in the pool to make space for the new entry. + * Return negative on error. + */ +int ipu3_css_pool_get(struct ipu3_css_pool *pool, long framenum) +{ + int n = ipu3_css_pool_check(pool, framenum); + + if (n < 0) + return n; + + pool->entry[n].framenum = framenum; + pool->last = n; + + return n; +} + +/* + * Undo, for all practical purposes, the effect of pool_get(). + */ +void ipu3_css_pool_put(struct ipu3_css_pool *pool) +{ + pool->entry[pool->last].framenum = INT_MIN; + pool->last = (pool->last + IPU3_CSS_POOL_SIZE - 1) % IPU3_CSS_POOL_SIZE; +} + +const struct ipu3_css_map * +ipu3_css_pool_last(struct ipu3_css_pool *pool, unsigned int n) +{ + static const struct ipu3_css_map null_map = { 0 }; + int i = (pool->last + IPU3_CSS_POOL_SIZE - n) % IPU3_CSS_POOL_SIZE; + + WARN_ON(n >= IPU3_CSS_POOL_SIZE); + + if (pool->entry[i].framenum < 0) + return &null_map; + + return &pool->entry[i].param; +} diff --git a/drivers/media/pci/intel/ipu3/ipu3-css-pool.h b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h new file mode 100644 index 000..a2d2494 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-css-pool.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABIL
[PATCH v2 12/12] intel-ipu3: imgu top level pci device
Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/Kconfig | 15 + drivers/media/pci/intel/ipu3/Makefile | 6 + drivers/media/pci/intel/ipu3/ipu3.c | 740 ++ drivers/media/pci/intel/ipu3/ipu3.h | 183 + 4 files changed, 944 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3.h diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig index 2030be7..1ba64e6 100644 --- a/drivers/media/pci/intel/ipu3/Kconfig +++ b/drivers/media/pci/intel/ipu3/Kconfig @@ -32,3 +32,18 @@ config INTEL_IPU3_DMAMAP select IOMMU_IOVA ---help--- This is IPU3 IOMMU domain specific DMA driver. + +config VIDEO_IPU3_IMGU + tristate "Intel ipu3-imgu driver" + depends on PCI && VIDEO_V4L2 && IOMMU_SUPPORT + depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API + select IOMMU_API + select IOMMU_IOVA + select VIDEOBUF2_DMA_CONTIG + ---help--- + This is the video4linux2 driver for Intel IPU3 image processing unit, + found in Intel Skylake and Kaby Lake SoCs and used for processing + images and video. + + Say Y or M here if you have a Skylake/Kaby Lake SoC with a MIPI + camera. The module will be called ipu3-imgu. diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile index 2c2a035..e740877 100644 --- a/drivers/media/pci/intel/ipu3/Makefile +++ b/drivers/media/pci/intel/ipu3/Makefile @@ -1,3 +1,9 @@ obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o +ipu3-imgu-objs += \ + ipu3-tables.o ipu3-css-pool.o \ + ipu3-css-fw.o ipu3-css-params.o \ + ipu3-css.o ipu3-v4l2.o ipu3.o + +obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3-imgu.o diff --git a/drivers/media/pci/intel/ipu3/ipu3.c b/drivers/media/pci/intel/ipu3/ipu3.c new file mode 100644 index 000..1550fa9 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3.c @@ -0,0 +1,740 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Based on Intel IPU4 driver. + * + */ + +#include +#include +#include +#include +#include + +#include "ipu3.h" + +#define IMGU_NAME "ipu3-imgu" +#define IMGU_PCI_ID0x1919 +#define IMGU_PCI_BAR 0 +#define IMGU_DMA_MASK DMA_BIT_MASK(39) +#define IMGU_MAX_QUEUE_DEPTH (2 + 2) + +static struct imgu_node_mapping const imgu_node_map[IMGU_NODE_NUM] = { + [IMGU_NODE_IN] = {IPU3_CSS_QUEUE_IN, "input"}, + [IMGU_NODE_PARAMS] = {IPU3_CSS_QUEUE_PARAMS, "parameters"}, + [IMGU_NODE_OUT] = {IPU3_CSS_QUEUE_OUT, "output"}, + [IMGU_NODE_VF] = {IPU3_CSS_QUEUE_VF, "viewfinder"}, + [IMGU_NODE_PV] = {IPU3_CSS_QUEUE_VF, "postview"}, + [IMGU_NODE_STAT_3A] = {IPU3_CSS_QUEUE_STAT_3A, "3a stat"}, + [IMGU_NODE_STAT_DVS] = {IPU3_CSS_QUEUE_STAT_DVS, "dvs stat"}, + [IMGU_NODE_STAT_LACE] = {IPU3_CSS_QUEUE_STAT_LACE, "lace stat"}, +}; + +int imgu_node_to_queue(int node) +{ + return imgu_node_map[node].css_queue; +} + +int imgu_map_node(struct imgu_device *imgu, int css_queue) +{ + unsigned int i; + + if (css_queue == IPU3_CSS_QUEUE_VF) + return imgu->mem2mem2.nodes[IMGU_NODE_VF].enabled ? + IMGU_NODE_VF : IMGU_NODE_PV; + + for (i = 0; i < IMGU_NODE_NUM; i++) + if (imgu_node_map[i].css_queue == css_queue) + return i; + + return -EINVAL; +} +/ Dummy buffers / + +void imgu_dummybufs_cleanup(struct imgu_device *imgu) +{ + unsigned int i; + + for (i = 0; i < IPU3_CSS_QUEUES; i++) { + if (imgu->queues[i].dummybuf_vaddr) + dma_free_coherent(imgu->mmu.dev, + imgu->queues[i].dummybuf_size, + imgu->queues[i].dummybuf_vaddr, + imgu->queues[i].dummybuf_daddr); + imgu->queues[i].dummybuf_vaddr = NULL; + } +} + +int imgu_dummybufs_init(struct imgu_device *imgu) +{ + unsigned int i, j; + int node; + + /* Allocate a dummy buffer for each queue where buffer is optional */ + for (i = 0; i < IPU3_CSS_QUEUES; i++) { + node = imgu_map_node(imgu, i); + if (!img
[PATCH v2 10/12] intel-ipu3: css pipeline
Add css pipeline and v4l code Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-css.c | 1764 ++- drivers/media/pci/intel/ipu3/ipu3-css.h | 84 ++ 2 files changed, 1842 insertions(+), 6 deletions(-) diff --git a/drivers/media/pci/intel/ipu3/ipu3-css.c b/drivers/media/pci/intel/ipu3/ipu3-css.c index 675be91..9f76d10 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-css.c +++ b/drivers/media/pci/intel/ipu3/ipu3-css.c @@ -13,8 +13,15 @@ #include #include +#include +#include +#include +#include +#include + #include "ipu3-css.h" #include "ipu3-css-fw.h" +#include "ipu3-css-params.h" #include "ipu3-tables.h" /* IRQ configuration */ @@ -23,6 +30,187 @@ IMGU_IRQCTRL_IRQ_SW_PIN(0) | \ IMGU_IRQCTRL_IRQ_SW_PIN(1)) +#define IPU3_CSS_FORMAT_BPP_DEN50 /* Denominator */ + +/* Some sane limits for resolutions */ +#define IPU3_CSS_MIN_RES 32 +#define IPU3_CSS_MAX_RES 65535 + +/* Formats supported by IPU3 Camera Sub System */ +static const struct ipu3_css_format ipu3_css_formats[] = { + { + .pixelformat = V4L2_PIX_FMT_NV12, + .colorspace = V4L2_COLORSPACE_SRGB, + .frame_format = IMGU_ABI_FRAME_FORMAT_NV12, + .osys_format = IMGU_ABI_OSYS_FORMAT_NV12, + .osys_tiling = IMGU_ABI_OSYS_TILING_NONE, + .bytesperpixel_num = 1 * IPU3_CSS_FORMAT_BPP_DEN, + .chroma_decim = 4, + .width_align = IPU3_UAPI_ISP_VEC_ELEMS, + .flags = IPU3_CSS_FORMAT_FL_OUT | IPU3_CSS_FORMAT_FL_VF, + }, { + /* Each 32 bytes contains 25 10-bit pixels */ + .pixelformat = V4L2_PIX_FMT_IPU3_SBGGR10, + .colorspace = V4L2_COLORSPACE_RAW, + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED, + .bayer_order = IMGU_ABI_BAYER_ORDER_BGGR, + .bit_depth = 10, + .bytesperpixel_num = 64, + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS, + .flags = IPU3_CSS_FORMAT_FL_IN, + }, { + .pixelformat = V4L2_PIX_FMT_IPU3_SGBRG10, + .colorspace = V4L2_COLORSPACE_RAW, + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED, + .bayer_order = IMGU_ABI_BAYER_ORDER_GBRG, + .bit_depth = 10, + .bytesperpixel_num = 64, + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS, + .flags = IPU3_CSS_FORMAT_FL_IN, + }, { + .pixelformat = V4L2_PIX_FMT_IPU3_SGRBG10, + .colorspace = V4L2_COLORSPACE_RAW, + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED, + .bayer_order = IMGU_ABI_BAYER_ORDER_GRBG, + .bit_depth = 10, + .bytesperpixel_num = 64, + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS, + .flags = IPU3_CSS_FORMAT_FL_IN, + }, { + .pixelformat = V4L2_PIX_FMT_IPU3_SRGGB10, + .colorspace = V4L2_COLORSPACE_RAW, + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED, + .bayer_order = IMGU_ABI_BAYER_ORDER_RGGB, + .bit_depth = 10, + .bytesperpixel_num = 64, + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS, + .flags = IPU3_CSS_FORMAT_FL_IN, + }, {/* Parameter pseudo-format */ + + .pixelformat = V4L2_META_FMT_IPU3_PARAMS, + .colorspace = V4L2_COLORSPACE_DEFAULT, + .bytesperpixel_num = sizeof(struct ipu3_uapi_params) * + IPU3_CSS_FORMAT_BPP_DEN, + .width_align = 1, + .flags = IPU3_CSS_FORMAT_FL_PARAMS, + + }, {/* Statistics pseudo-formats */ + + .pixelformat = V4L2_META_FMT_IPU3_STAT_3A, + .colorspace = V4L2_COLORSPACE_DEFAULT, + .bytesperpixel_num = sizeof(struct ipu3_uapi_stats_3a) * + IPU3_CSS_FORMAT_BPP_DEN, + .width_align = 1, + .flags = IPU3_CSS_FORMAT_FL_STAT_3A, + }, { + .pixelformat = V4L2_META_FMT_IPU3_STAT_DVS, + .colorspace = V4L2_COLORSPACE_DEFAULT, + .bytesperpixel_num = sizeof(struct ipu3_uapi_stats_dvs) * + IPU3_CSS_FORMAT_BPP_DEN, + .width_align = 1, + .flags = IPU3_CSS_FORMAT_FL_STAT_DVS, + }, { + .pixelformat = V4L2_META_FMT_IPU3_STAT_LACE, + .colorspace = V4L2_COLORSPACE_DEFAULT, + .bytesperpixel_num = sizeof(struct ipu3_uapi_stats_lace) * + IPU3_CSS_FORMAT_BPP_DEN, + .width_align = 1, + .flags = IPU3_CSS_FORMAT_FL_STAT_LACE, + }, +}; + +static const struct { + enum imgu_abi_queue_id qid; +
[PATCH v2 07/12] intel-ipu3: css: firmware management
Functions to load and install imgu FW blobs Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-abi.h| 1573 drivers/media/pci/intel/ipu3/ipu3-css-fw.c | 272 + drivers/media/pci/intel/ipu3/ipu3-css-fw.h | 219 drivers/media/pci/intel/ipu3/ipu3-css.h| 54 + 4 files changed, 2118 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.h diff --git a/drivers/media/pci/intel/ipu3/ipu3-abi.h b/drivers/media/pci/intel/ipu3/ipu3-abi.h new file mode 100644 index 000..5107b35 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-abi.h @@ -0,0 +1,1573 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __IPU3_ABI_H +#define __IPU3_ABI_H + +#include + +/*** IMGU Hardware information ***/ + +#define IMGU_ISP_VMEM_ALIGN128 +#define IMGU_DVS_BLOCK_W 64 +#define IMGU_DVS_BLOCK_H 32 +#define IMGU_GDC_BUF_X (2 * IMGU_DVS_BLOCK_W) +#define IMGU_GDC_BUF_Y IMGU_DVS_BLOCK_H +/* n = 0..1 */ +#define IMGU_SP_PMEM_BASE(n) (0x2 + (n) * 0x4000) +#define IMGU_MAX_BQ_GRID_WIDTH 80 +#define IMGU_MAX_BQ_GRID_HEIGHT60 +#define IMGU_OBGRID_TILE_SIZE 16 +#define IMGU_PIXELS_PER_WORD 50 +#define IMGU_BYTES_PER_WORD64 +#define IMGU_STRIPE_FIXED_HALF_OVERLAP 2 +#define IMGU_SHD_SETS 3 +#define IMGU_BDS_MIN_CLIP_VAL 0 +#define IMGU_BDS_MAX_CLIP_VAL 2 +#define IPU3_ABI_AWB_MAX_CELLS_PER_SET 160 +#define IPU3_ABI_AF_MAX_CELLS_PER_SET 32 +#define IPU3_ABI_AWB_FR_MAX_CELLS_PER_SET 32 + +#define IMGU_ABI_ACC_OP_IDLE 0 +#define IMGU_ABI_ACC_OP_END_OF_ACK 1 +#define IMGU_ABI_ACC_OP_END_OF_OPS 2 +#define IMGU_ABI_ACC_OP_NO_OPS 3 + +#define IMGU_ABI_ACC_OPTYPE_PROCESS_LINES 0 +#define IMGU_ABI_ACC_OPTYPE_TRANSFER_DATA 1 + +#define IMGU_MMU_PADDR_SHIFT 12 + +/* Register definitions */ + +/* PM_CTRL_0_5_0_IMGHMMADR */ +#define IMGU_REG_PM_CTRL 0x0 +#define IMGU_PM_CTRL_START BIT(0) +#define IMGU_PM_CTRL_CFG_DONE BIT(1) +#define IMGU_PM_CTRL_RACE_TO_HALT BIT(2) +#define IMGU_PM_CTRL_NACK_ALL BIT(3) +#define IMGU_PM_CTRL_CSS_PWRDN BIT(4) +#define IMGU_PM_CTRL_RST_AT_EOFBIT(5) +#define IMGU_PM_CTRL_FORCE_HALTBIT(6) +#define IMGU_PM_CTRL_FORCE_UNHALT BIT(7) +#define IMGU_PM_CTRL_FORCE_PWRDN BIT(8) +#define IMGU_PM_CTRL_FORCE_RESET BIT(9) +#define IMGU_PM_CTRL_RETURN_LICENSE_AT_EOF BIT(10) +#define IMGU_PM_CTRL_POWER_DOWN_AT_EOF (IMGU_PM_CTRL_CSS_PWRDN | \ + IMGU_PM_CTRL_RACE_TO_HALT | \ + IMGU_PM_CTRL_RETURN_LICENSE_AT_EOF) +#define IMGU_PM_CTRL_RESET_AT_EOF (IMGU_PM_CTRL_RST_AT_EOF | \ + IMGU_PM_CTRL_RACE_TO_HALT | \ + IMGU_PM_CTRL_RETURN_LICENSE_AT_EOF) +/* SYSTEM_REQ_0_5_0_IMGHMMADR */ +#define IMGU_REG_SYSTEM_REQ0x18 +#define IMGU_SYSTEM_REQ_FREQ_MASK 0x3f +#define IMGU_SYSTEM_REQ_FREQ_DIVIDER 25 +#define IMGU_REG_INT_STATUS0x30 +#define IMGU_REG_INT_ENABLE0x34 +#define IMGU_REG_INT_CSS_IRQ (1 << 31) +/* STATE_0_5_0_IMGHMMADR */ +#define IMGU_REG_STATE 0x130 +#define IMGU_STATE_HALT_STSBIT(0) +#define IMGU_STATE_IDLE_STSBIT(1) +#define IMGU_STATE_POWER_UPBIT(2) +#define IMGU_STATE_POWER_DOWN BIT(3) +#define IMGU_STATE_CSS_BUSY_MASK 0xc0 +#define IMGU_STATE_PM_FSM_MASK 0x180 +#define IMGU_STATE_PWRDNM_FSM_MASK 0x1E0 +/* PM_STS_0_5_0_IMGHMMADR */ +#define IMGU_REG_PM_STS0x140 + +#define IMGU_REG_BASE 0x4000 + +#define IMGU_REG_ISP_CTRL
[PATCH v2 11/12] intel-ipu3: Add imgu v4l2 driver
ipu3 imgu video device based on v4l2, vb2 and media controller framework. Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 727 +++ 1 file changed, 727 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c new file mode 100644 index 000..c9493dc --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c @@ -0,0 +1,727 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include +#include + +#include "ipu3.h" + +/ v4l2_subdev_ops / + +static int ipu3_subdev_s_stream(struct v4l2_subdev *sd, int enable) +{ + struct ipu3_mem2mem2_device *m2m2 = + container_of(sd, struct ipu3_mem2mem2_device, subdev); + int r = 0; + + if (m2m2->ops && m2m2->ops->s_stream) + r = m2m2->ops->s_stream(m2m2, enable); + + if (!r) + m2m2->streaming = enable; + + return r; +} + +static int ipu3_subdev_get_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *fmt) +{ + struct ipu3_mem2mem2_device *m2m2 = + container_of(sd, struct ipu3_mem2mem2_device, subdev); + struct v4l2_mbus_framefmt *mf; + u32 pad = fmt->pad; + + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { + fmt->format = m2m2->nodes[pad].pad_fmt; + } else { + mf = v4l2_subdev_get_try_format(sd, cfg, pad); + fmt->format = *mf; + } + + return 0; +} + +static int ipu3_subdev_set_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *fmt) +{ + struct ipu3_mem2mem2_device *m2m2 = + container_of(sd, struct ipu3_mem2mem2_device, subdev); + struct v4l2_mbus_framefmt *mf; + u32 pad = fmt->pad; + + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) + mf = v4l2_subdev_get_try_format(sd, cfg, pad); + else + mf = &m2m2->nodes[pad].pad_fmt; + + /* Clamp the w and h based on the hardware capabilities */ + if (m2m2->subdev_pads[pad].flags & MEDIA_PAD_FL_SOURCE) { + + fmt->format.width = clamp(fmt->format.width, + IPU3_OUTPUT_MIN_WIDTH, + IPU3_OUTPUT_MAX_WIDTH); + fmt->format.height = clamp(fmt->format.height, + IPU3_OUTPUT_MIN_HEIGHT, + IPU3_OUTPUT_MAX_HEIGHT); + } else { + fmt->format.width = clamp(fmt->format.width, + IPU3_INPUT_MIN_WIDTH, + IPU3_INPUT_MAX_WIDTH); + fmt->format.height = clamp(fmt->format.height, + IPU3_INPUT_MIN_HEIGHT, + IPU3_INPUT_MAX_HEIGHT); + } + + *mf = fmt->format; + + return 0; +} + +/ media_entity_operations / + +static int ipu3_link_setup(struct media_entity *entity, +const struct media_pad *local, +const struct media_pad *remote, u32 flags) +{ + struct ipu3_mem2mem2_device *m2m2 = + container_of(entity, struct ipu3_mem2mem2_device, subdev.entity); + u32 pad = local->index; + + WARN_ON(pad >= m2m2->num_nodes); + + m2m2->nodes[pad].enabled = !!(flags & MEDIA_LNK_FL_ENABLED); + + return 0; +} + +/ vb2_ops / + +/* Transfer buffer ownership to me */ +static void ipu3_vb2_buf_queue(struct vb2_buffer *vb) +{ + struct ipu3_mem2mem2_device *m2m2 = vb2_get_drv_priv(vb->vb2_queue); + struct imgu_device *imgu = + container_of(m2m2, struct imgu_device, mem2mem2); + struct imgu_video_device *node = + container_of(vb->vb2_queue, struct imgu_video_device, vbq); + int queue; + + queue = imgu_node_to_queue(node - m2m2->nodes); + + if (queue < 0) { + dev_err(&imgu->pci_dev->dev, "Invalid imgu node.\n"); + return; + } + + if (queue == IPU3_CSS_QUEUE_PARAMS) { + unsigned int need_byte
[PATCH v2 03/12] intel-ipu3: Add DMA API implementation
IPU3 mmu based DMA mapping driver Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/Kconfig | 6 + drivers/media/pci/intel/ipu3/Makefile | 1 + drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 366 + drivers/media/pci/intel/ipu3/ipu3-dmamap.h | 20 ++ 4 files changed, 393 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig index ab2edcb..2030be7 100644 --- a/drivers/media/pci/intel/ipu3/Kconfig +++ b/drivers/media/pci/intel/ipu3/Kconfig @@ -26,3 +26,9 @@ config INTEL_IPU3_MMU Say Y here if you have Skylake/Kaby Lake SoC with IPU3. Say N if un-sure. + +config INTEL_IPU3_DMAMAP + bool "Intel ipu3 DMA mapping driver" + select IOMMU_IOVA + ---help--- + This is IPU3 IOMMU domain specific DMA driver. diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile index 2b669df..2c2a035 100644 --- a/drivers/media/pci/intel/ipu3/Makefile +++ b/drivers/media/pci/intel/ipu3/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o +obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c new file mode 100644 index 000..44cedc0 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c @@ -0,0 +1,366 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include +#include +#include +#include +#include "ipu3-mmu.h" + +/* Begin of things adapted from arch/arm/mm/dma-mapping.c */ +static void ipu3_dmamap_clear_buffer(struct page *page, size_t size, +unsigned long attrs) +{ + /* +* Ensure that the allocated pages are zeroed, and that any data +* lurking in the kernel direct-mapped region is invalidated. +*/ + if (PageHighMem(page)) { + while (size > 0) { + void *ptr = kmap_atomic(page); + + memset(ptr, 0, PAGE_SIZE); + if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0) + clflush_cache_range(ptr, PAGE_SIZE); + kunmap_atomic(ptr); + page++; + size -= PAGE_SIZE; + } + } else { + void *ptr = page_address(page); + + memset(ptr, 0, size); + if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0) + clflush_cache_range(ptr, size); + } +} + +/** + * ipu3_dmamap_alloc_buffer - allocate buffer based on attributes + * @dev: struct device pointer + * @size: size of buffer in bytes + * @gfp: specify the free page type + * @attrs: defined in linux/dma-attrs.h + * + * This is a helper function for physical page allocation + * + * Return array representing buffer from alloc_pages() on success + * or NULL on failure + * + * Must be freed with ipu3_dmamap_free_buffer. + */ +static struct page **ipu3_dmamap_alloc_buffer(struct device *dev, size_t size, + gfp_t gfp, unsigned long attrs) +{ + struct page **pages; + int count = size >> PAGE_SHIFT; + int array_size = count * sizeof(struct page *); + int i = 0; + + /* Allocate mem for array of page ptrs */ + if (array_size <= PAGE_SIZE) + pages = kzalloc(array_size, GFP_KERNEL); + else + pages = vzalloc(array_size); + if (!pages) + return NULL; + + gfp |= __GFP_NOWARN; + + while (count) { + int j, order = __fls(count); + + pages[i] = alloc_pages(gfp, order); + while (!pages[i] && order) + pages[i] = alloc_pages(gfp, --order); + if (!pages[i]) + goto error; + + if (order) { + split_page(pages[i], order); + j = 1 << order; + while (--j) + pages[i + j] = pages[i] + j; + } + /* Zero and invalidate */ + ipu3_dmamap_clear_buffer(pages[i], PAGE_SIZE << order, attrs); + i += 1 << order; + count -= 1 << order; + } + + return pages; + +error: + while (i--)
[PATCH v2 04/12] intel-ipu3: Add user space ABI definitions
Signed-off-by: Yong Zhi --- include/uapi/linux/intel-ipu3.h | 2182 +++ 1 file changed, 2182 insertions(+) create mode 100644 include/uapi/linux/intel-ipu3.h diff --git a/include/uapi/linux/intel-ipu3.h b/include/uapi/linux/intel-ipu3.h new file mode 100644 index 000..9e90aec --- /dev/null +++ b/include/uapi/linux/intel-ipu3.h @@ -0,0 +1,2182 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __IPU3_UAPI_H +#define __IPU3_UAPI_H + +#include + +#define IPU3_UAPI_ISP_VEC_ELEMS64 + +#define IMGU_ABI_PAD __aligned(IPU3_UAPI_ISP_WORD_BYTES) +#define IPU3_ALIGN __attribute__((aligned(IPU3_UAPI_ISP_WORD_BYTES))) + +#define IPU3_UAPI_ISP_WORD_BYTES 32 +#define IPU3_UAPI_MAX_STRIPES 2 + +/*** ipu3_uapi_stats_3a ***/ + +#define IPU3_UAPI_MAX_BUBBLE_SIZE 10 + +#define IPU3_UAPI_AE_COLORS4 +#define IPU3_UAPI_AE_BINS 256 + +#define IPU3_UAPI_AWB_MD_ITEM_SIZE 8 +#define IPU3_UAPI_AWB_MAX_SETS 60 +#define IPU3_UAPI_AWB_SET_SIZE 0x500 +#define IPU3_UAPI_AWB_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ +IPU3_UAPI_AWB_MD_ITEM_SIZE) +#define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \ + (IPU3_UAPI_AWB_MAX_SETS * \ +(IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) + +#define IPU3_UAPI_AF_MAX_SETS 24 +#define IPU3_UAPI_AF_MD_ITEM_SIZE 4 +#define IPU3_UAPI_AF_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ +IPU3_UAPI_AF_MD_ITEM_SIZE) +#define IPU3_UAPI_AF_Y_TABLE_SET_SIZE 0x80 +#define IPU3_UAPI_AF_Y_TABLE_MAX_SIZE \ + (IPU3_UAPI_AF_MAX_SETS * \ +(IPU3_UAPI_AF_Y_TABLE_SET_SIZE + IPU3_UAPI_AF_SPARE_FOR_BUBBLES) * \ +IPU3_UAPI_MAX_STRIPES) + +#define IPU3_UAPI_AWB_FR_MAX_SETS 24 +#define IPU3_UAPI_AWB_FR_MD_ITEM_SIZE 8 +#define IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE0x100 +#define IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES \ + (IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \ + IPU3_UAPI_AWB_FR_MD_ITEM_SIZE) +#define IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE \ + (IPU3_UAPI_AWB_FR_MAX_SETS * \ + (IPU3_UAPI_AWB_FR_BAYER_TBL_SIZE + \ +IPU3_UAPI_AWB_FR_SPARE_FOR_BUBBLES) * \ + IPU3_UAPI_MAX_STRIPES) + +struct ipu3_uapi_grid_config { + __u8 width; /* 6 or 7 (rgbs_grd_cfg) bits */ + __u8 height; + __u16 block_width_log2:3; + __u16 block_height_log2:3; + __u16 height_per_slice:8; /* default value 1 */ + __u16 x_start; /* 12 bits */ + __u16 y_start; +#define IPU3_UAPI_GRID_START_MASK ((1 << 12) - 1) +#define IPU3_UAPI_GRID_Y_START_EN (1 << 15) + __u16 x_end;/* 12 bits */ + __u16 y_end; +}; + +struct ipu3_uapi_awb_meta_data { + __u8 meta_data_buffer[IPU3_UAPI_AWB_MAX_BUFFER_SIZE]; +}; + +struct ipu3_uapi_awb_raw_buffer { + struct ipu3_uapi_awb_meta_data meta_data; +}; + +struct IPU3_ALIGN ipu3_uapi_awb_config_s { + __u16 rgbs_thr_gr; + __u16 rgbs_thr_r; + __u16 rgbs_thr_gb; + __u16 rgbs_thr_b; +/* controls generation of meta_data (like FF enable/disable) */ +#define IPU3_UAPI_AWB_RGBS_THR_B_EN(1 << 14) +#define IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT (1 << 15) + + struct ipu3_uapi_grid_config grid; +}; + +struct ipu3_uapi_ae_raw_buffer { + __u32 vals[IPU3_UAPI_AE_BINS * IPU3_UAPI_AE_COLORS]; +}; + +struct ipu3_uapi_ae_raw_buffer_aligned { + struct ipu3_uapi_ae_raw_buffer buff IPU3_ALIGN; +}; + +struct ipu3_uapi_ae_grid_config { + __u8 width; + __u8 height; + __u8 block_width_log2:4; + __u8 block_height_log2:4; + __u8 __reserved0:5; + __u8 ae_en:1; + __u8 rst_hist_array:1; + __u8 done_rst_hist_array:1; + __u16 x_start; /* 12 bits */ + __u16 y_start; + __u16 x_end; + __u16 y_end; +}; + +struct ipu3_uapi_af_filter_config { + struct { + __u8 a1; + __u8 a2; +
[PATCH v2 02/12] intel-ipu3: mmu: implement driver
From: Tuukka Toivonen This driver translates Intel IPU3 internal virtual address to physical address. Signed-off-by: Tuukka Toivonen Signed-off-by: Yong Zhi --- drivers/media/pci/intel/ipu3/Kconfig| 11 + drivers/media/pci/intel/ipu3/Makefile | 1 + drivers/media/pci/intel/ipu3/ipu3-mmu.c | 422 drivers/media/pci/intel/ipu3/ipu3-mmu.h | 73 ++ 4 files changed, 507 insertions(+) create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.c create mode 100644 drivers/media/pci/intel/ipu3/ipu3-mmu.h diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig index 2a895d6..ab2edcb 100644 --- a/drivers/media/pci/intel/ipu3/Kconfig +++ b/drivers/media/pci/intel/ipu3/Kconfig @@ -15,3 +15,14 @@ config VIDEO_IPU3_CIO2 Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2 connected camera. The module will be called ipu3-cio2. + +config INTEL_IPU3_MMU + tristate "Intel ipu3-mmu driver" + select IOMMU_API + select IOMMU_IOVA + ---help--- + For IPU3, this option enables its MMU driver to translate its internal + virtual address to 39 bits wide physical address for 64GBytes space access. + + Say Y here if you have Skylake/Kaby Lake SoC with IPU3. + Say N if un-sure. diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile index 20186e3..2b669df 100644 --- a/drivers/media/pci/intel/ipu3/Makefile +++ b/drivers/media/pci/intel/ipu3/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o +obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o diff --git a/drivers/media/pci/intel/ipu3/ipu3-mmu.c b/drivers/media/pci/intel/ipu3/ipu3-mmu.c new file mode 100644 index 000..26ef045 --- /dev/null +++ b/drivers/media/pci/intel/ipu3/ipu3-mmu.c @@ -0,0 +1,422 @@ +/* + * Copyright (c) 2017 Intel Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include + +#include "ipu3-mmu.h" + +/** + * ipu3_mmu_tlb_invalidate - invalidate translation look-aside buffer + * @addr: base address to access REG_TLB_INVALIDATE + * + * This function must be called when MMU has power + */ +static void ipu3_mmu_tlb_invalidate(const struct ipu3_mmu *mmu) +{ + writel(TLB_INVALIDATE, mmu->base + REG_TLB_INVALIDATE); +} + +static int ipu3_mmu_add_device(struct device *dev) +{ + struct ipu3_mmu *mmu = dev_get_drvdata(dev); + + /* mapping domain must be prepared */ + if (!mmu->domain) + return 0; + + return iommu_attach_device(mmu->domain, dev); +} + +/** + * ipu3_mmu_alloc_page_table - get page to fill entries with dummy defaults + * @u32: default value of page table entry + * + * Index of L1 page table points to L2 tbl + * + * Return: Pointer to allocated page table + * or NULL on failure. + */ +static u32 *ipu3_mmu_alloc_page_table(u32 default_entry) +{ + u32 *pt = (u32 *)__get_free_page(GFP_KERNEL); + int i; + + if (!pt) + return NULL; + + for (i = 0; i < IPU3_MMU_L1PT_PTES; i++) + pt[i] = default_entry; + + return pt; +} + +/** + * ipu3_mmu_domain_alloc - initialize and allocate pgt for domain + * @type: possible domain-types + * + * Allocate dummy page, L2 tbl and L1 tbl in that order + * + * Return: Pointer to allocated iommu_domain instance + * or NULL on failure. + */ +static struct iommu_domain *ipu3_mmu_domain_alloc(unsigned int type) +{ + struct ipu3_mmu_domain *mmu_dom; + void *ptr; + + if (type != IOMMU_DOMAIN_UNMANAGED) + return NULL; + + mmu_dom = kzalloc(sizeof(*mmu_dom), GFP_KERNEL); + if (!mmu_dom) + return NULL; + + mmu_dom->domain.geometry.aperture_start = 0; + mmu_dom->domain.geometry.aperture_end = + DMA_BIT_MASK(IPU3_MMU_ADDRESS_BITS); + mmu_dom->domain.geometry.force_aperture = true; + + ptr = (void *)__get_free_page(GFP_KERNEL); + if (!ptr) + goto fail_get_page; + mmu_dom->dummy_page = virt_to_phys(ptr) >> IPU3_MMU_PAGE_SHIFT; + ptr = ipu3_mmu_alloc_page_table(mmu_dom->dummy_page); + if (!ptr) + goto fail_page_table; + + /* +* We always map the L1 page table (a single page as well as +* the L2 page tables). +*/ + mmu_dom->dummy_l2_tbl = virt_to_phys(ptr) >> IPU3_MMU_PAGE_SHIFT; + mmu_dom->pgtbl = ipu3_mmu_alloc_page_table(mmu_dom->dummy_l2_tbl); + if (!mmu_dom->pgtbl) + goto fail_
[PATCH v2 00/12] Intel IPU3 ImgU patchset
This patchset adds support for the Intel IPU3 (Image Processing Unit) ImgU which is essentially a modern memory-to-memory ISP. It implements raw Bayer to YUV image format conversion as well as a large number of other pixel processing algorithms for improving the image quality. Meta data formats are defined for image statistics (3A, i.e. automatic white balance, exposure and focus, histogram and local area contrast enhancement) as well as for the pixel processing algorithm parameters. The documentation for these formats is currently not included in the patchset but will be added in a future version of this set. The algorithm parameters need to be considered specific to a given frame and typically a large number of these parameters change on frame to frame basis. Additionally, the parameters are highly structured (and not a flat space of independent configuration primitives). They also reflect the data structures used by the firmware and the hardware. On top of that, the algorithms require highly specialized user space to make meaningful use of them. For these reasons it has been chosen video buffers to pass the parameters to the device. On individual patches: The heart of ImgU is the CSS, or Camera Subsystem, which contains the image processors and HW accelerators. The 3A statistics and other firmware parameter computation related functions are implemented in patch 8. All h/w programming related code can be found in patch 9. To access DDR via ImgU's own memory space, IPU3 is also equipped with its own MMU unit, the driver is implemented in patch 2. Patch 3 uses above driver for DMA mapping operation. Patch 5-10 are basically IPU3 CSS specific implementations: 6 and 7 provide some utility functions and manage IPU3 fw download and install. Will add a link here when the fw (ie. ipu3_fw.bin) is submitted. Patch 9 and 10 are of the same file, the latter implements interface functions for access fw & hw capabilities defined in patch 8. Patch 12 uses Kconfig and Makefile created by IPU3 cio2 patch set, the code path, however is updated to drivers/media/pci/intel/ipu3. The path change will be reflected in next revision of the cio2 patch as well. Here is the device topology: localhost ~ # media-ctl -d /dev/media0 -p Media controller API version 0.1.0 Media device information driver ipu3-imgu model ipu3-imgu serial bus info:00:05.0 hw revision 0x0 driver version 4.12.0 Device topology - entity 1: ipu3-imgu:0 (8 pads, 8 links) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev0 pad0:Sink stream:0[fmt:UYVY2X8/352x288] link: <- "input":0 [ENABLED,IMMUTABLE] pad1:Sink stream:0[fmt:UYVY2X8/352x288] link: <- "parameters":0 [] pad2:Source stream:0[fmt:UYVY2X8/352x288] link: -> "output":0 [] pad3:Source stream:0[fmt:UYVY2X8/352x288] link: -> "viewfinder":0 [] pad4:Source stream:0[fmt:UYVY2X8/352x288] link: -> "postview":0 [] pad5:Source stream:0[fmt:UYVY2X8/352x288] link: -> "3a stat":0 [] pad6:Source stream:0[fmt:UYVY2X8/352x288] link: -> "dvs stat":0 [] pad7:Source stream:0[fmt:UYVY2X8/352x288] link: -> "lace stat":0 [] - entity 2: input (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video0 pad0:Source link: -> "ipu3-imgu:0":0 [ENABLED,IMMUTABLE] - entity 3: parameters (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video1 pad0:Source link: -> "ipu3-imgu:0":1 [] - entity 4: output (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video2 pad0:Sink link: <- "ipu3-imgu:0":2 [] - entity 5: viewfinder (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video3 pad0:Sink link: <- "ipu3-imgu:0":3 [] - entity 6: postview (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video4 pad0:Sink link: <- "ipu3-imgu:0":4 [] - entity 7: 3a stat (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video5 pad0:Sink link: <- "ipu3-imgu:0":5 [] - entity 8: dvs stat (1 pad, 1 link) type Node subtype V4L flags 0 device node name /dev/video6 p
[PATCH v2 01/12] videodev2.h, v4l2-ioctl: add IPU3 meta buffer format
Add the IPU3 specific processing parameter format V4L2_META_FMT_IPU3_PARAMS and metadata formats for 3A and other statistics: V4L2_META_FMT_IPU3_PARAMS V4L2_META_FMT_IPU3_STAT_3A V4L2_META_FMT_IPU3_STAT_DVS V4L2_META_FMT_IPU3_STAT_LACE Signed-off-by: Yong Zhi --- drivers/media/v4l2-core/v4l2-ioctl.c | 4 include/uapi/linux/videodev2.h | 6 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index fb1387f..919aa24 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1239,6 +1239,10 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) case V4L2_TCH_FMT_TU08: descr = "8-bit unsigned touch data"; break; case V4L2_META_FMT_VSP1_HGO:descr = "R-Car VSP1 1-D Histogram"; break; case V4L2_META_FMT_VSP1_HGT:descr = "R-Car VSP1 2-D Histogram"; break; + case V4L2_META_FMT_IPU3_PARAMS: descr = "IPU3 processing parameters"; break; + case V4L2_META_FMT_IPU3_STAT_3A:descr = "IPU3 3A statistics"; break; + case V4L2_META_FMT_IPU3_STAT_DVS: descr = "IPU3 DVS statistics"; break; + case V4L2_META_FMT_IPU3_STAT_LACE: descr = "IPU3 LACE statistics"; break; default: /* Compressed formats */ diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 7bfa6ad..fab8822 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -685,6 +685,12 @@ struct v4l2_pix_format { #define V4L2_META_FMT_VSP1_HGOv4l2_fourcc('V', 'S', 'P', 'H') /* R-Car VSP1 1-D Histogram */ #define V4L2_META_FMT_VSP1_HGTv4l2_fourcc('V', 'S', 'P', 'T') /* R-Car VSP1 2-D Histogram */ +/* Vendor specific - used for IPU3 camera sub-system */ +#define V4L2_META_FMT_IPU3_PARAMS v4l2_fourcc('i', 'p', '3', 'p') /* IPU3 params */ +#define V4L2_META_FMT_IPU3_STAT_3A v4l2_fourcc('i', 'p', '3', 's') /* IPU3 3A statistics */ +#define V4L2_META_FMT_IPU3_STAT_DVSv4l2_fourcc('i', 'p', '3', 'd') /* IPU3 DVS statistics */ +#define V4L2_META_FMT_IPU3_STAT_LACE v4l2_fourcc('i', 'p', '3', 'l') /* IPU3 LACE statistics */ + /* priv field value to indicates that subsequent fields are valid. */ #define V4L2_PIX_FMT_PRIV_MAGIC0xfeedcafe -- 2.7.4
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Hi Jacek, Thanks for the review! I have to say I found the v4l2-flash-led-class framework quite useful, now that I refactored a driver for using it. Now we have a user for the indicator, too. :-) On Wed, Jun 14, 2017 at 11:15:24PM +0200, Jacek Anaszewski wrote: > > +static __maybe_unused int as3645a_suspend(struct device *dev) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + struct as3645a *flash = i2c_get_clientdata(client); > > + int rval; > > + > > + rval = as3645a_set_control(flash, AS_MODE_EXT_TORCH, false); > > + dev_dbg(dev, "Suspend %s\n", rval < 0 ? "failed" : "ok"); > > + > > + return rval; > > +} > > + > > +static __maybe_unused int as3645a_resume(struct device *dev) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + struct as3645a *flash = i2c_get_clientdata(client); > > + int rval; > > + > > + rval = as3645a_setup(flash); > > + > > nitpicking: inconsistent coding style - there is no empty line before > dev_dbg() in the as3645a_suspend(). Added one for as3645a_suspend() --- it should have been there. > > > + dev_dbg(dev, "Resume %s\n", rval < 0 ? "fail" : "ok"); > > + > > + return rval; > > +} ... > > +static int as3645a_led_class_setup(struct as3645a *flash) > > +{ > > + struct led_classdev *fled_cdev = &flash->fled.led_cdev; > > + struct led_classdev *iled_cdev = &flash->iled_cdev; > > + struct led_flash_setting *cfg; > > + int rval; > > + > > + iled_cdev->name = "as3645a indicator"; > > + iled_cdev->brightness_set_blocking = as3645a_set_indicator_brightness; > > + iled_cdev->max_brightness = > > + flash->cfg.indicator_max_ua / AS_INDICATOR_INTENSITY_STEP; > > + > > + rval = led_classdev_register(&flash->client->dev, iled_cdev); > > + if (rval < 0) > > + return rval; > > + > > + cfg = &flash->fled.brightness; > > + cfg->min = AS_FLASH_INTENSITY_MIN; > > + cfg->max = flash->cfg.flash_max_ua; > > + cfg->step = AS_FLASH_INTENSITY_STEP; > > + cfg->val = flash->cfg.flash_max_ua; > > + > > + cfg = &flash->fled.timeout; > > + cfg->min = AS_FLASH_TIMEOUT_MIN; > > + cfg->max = flash->cfg.flash_timeout_us; > > + cfg->step = AS_FLASH_TIMEOUT_STEP; > > + cfg->val = flash->cfg.flash_timeout_us; > > + > > + flash->fled.ops = &as3645a_led_flash_ops; > > + > > + fled_cdev->name = "as3645a flash"; > > LED class device name should be taken from label DT property, > or DT node name if the former wasn't defined. > > Also LED device naming convention defines colon as a separator > between name segments. Right. I'll fix that. I just realised I'm missing DT binding documentation for this device; I'll add that, too. Is the preference to allow freely chosen node names for the LEDs? Now that there's the label, too, this appears to be somewhat duplicated information. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
RE: [PATCH 07/12] intel-ipu3: css: firmware management
> -Original Message- > From: linux-media-ow...@vger.kernel.org [mailto:linux-media- > ow...@vger.kernel.org] On Behalf Of Hans Verkuil > Sent: Tuesday, June 6, 2017 1:39 AM > To: Zhi, Yong ; linux-media@vger.kernel.org; > sakari.ai...@linux.intel.com > Cc: Zheng, Jian Xu ; tf...@chromium.org; Mani, > Rajmohan ; Toivonen, Tuukka > > Subject: Re: [PATCH 07/12] intel-ipu3: css: firmware management > > On 05/06/17 22:39, Yong Zhi wrote: > > Functions to load and install imgu FW blobs > > > > Signed-off-by: Yong Zhi > > --- > > drivers/media/pci/intel/ipu3/ipu3-abi.h| 1572 > > > drivers/media/pci/intel/ipu3/ipu3-css-fw.c | 272 + > > drivers/media/pci/intel/ipu3/ipu3-css-fw.h | 215 > > drivers/media/pci/intel/ipu3/ipu3-css.h| 54 + > > 4 files changed, 2113 insertions(+) > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-abi.h > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.c > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css-fw.h > > create mode 100644 drivers/media/pci/intel/ipu3/ipu3-css.h > > Has this been tested for both i686 and x86_64 modes? > > Regards, > > Hans Sorry for the late response, the above code has been tested for x86_64 mode only.
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Hi! > From: Sakari Ailus That address no longer works, right? > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > is based on that. > > Signed-off-by: Sakari Ailus > + * Based on drivers/media/i2c/as3645a.c. > + * > + * Contact: Sakari Ailus So I believe it should not be here. > +/* > + * as3645a_set_config - Set flash configuration registers > + * @flash: The flash > + * /** for linuxdoc? > + struct as3645a *flash = fled_to_as3645a(fled); > + int rval; > + > + /* NOTE: reading register clear fault status */ clears. > +static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool > is_flash, > +unsigned int ua) > +{ > + struct { > + unsigned int min; > + unsigned int max; > + unsigned int step; > + } __mms[] = { > + { > + AS_TORCH_INTENSITY_MIN, > + flash->cfg.assist_max_ua, > + AS_TORCH_INTENSITY_STEP > + }, > + { > + AS_FLASH_INTENSITY_MIN, > + flash->cfg.flash_max_ua, > + AS_FLASH_INTENSITY_STEP > + }, > + }, *mms = &__mms[is_flash]; > + > + if (ua < mms->min) > + ua = mms->min; That's some... seriously interesting code. And you are forcing gcc to create quite interesting structure on stack. Would it be easier to do normal if()... without this magic? > + struct v4l2_flash_config cfg = { > + .torch_intensity = { > + .min = AS_TORCH_INTENSITY_MIN, > + .max = flash->cfg.assist_max_ua, > + .step = AS_TORCH_INTENSITY_STEP, > + .val = flash->cfg.assist_max_ua, > + }, > + .indicator_intensity = { > + .min = AS_INDICATOR_INTENSITY_MIN, > + .max = flash->cfg.indicator_max_ua, > + .step = AS_INDICATOR_INTENSITY_STEP, > + .val = flash->cfg.indicator_max_ua, > + }, > + }; Ugh. And here you have copy of the above struct, + .val. Can it be somehow de-duplicated? Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH v2 07/11] r820t: mark register functions as noinline_if_stackbloat
With KASAN, we get an overly long stack frame due to inlining the register access function: drivers/media/tuners/r820t.c: In function 'generic_set_freq.isra.7': drivers/media/tuners/r820t.c:1334:1: error: the frame size of 2880 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] An earlier patch I tried used an open-coded r820t_write_reg() implementation that may have been more efficent, while this version simply adds the annotation, which has a lower risk for regressions. Signed-off-by: Arnd Bergmann --- drivers/media/tuners/r820t.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/tuners/r820t.c b/drivers/media/tuners/r820t.c index ba80376a3b86..a26d0eb64555 100644 --- a/drivers/media/tuners/r820t.c +++ b/drivers/media/tuners/r820t.c @@ -396,7 +396,7 @@ static int r820t_write(struct r820t_priv *priv, u8 reg, const u8 *val, return 0; } -static int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val) +static noinline_if_stackbloat int r820t_write_reg(struct r820t_priv *priv, u8 reg, u8 val) { return r820t_write(priv, reg, &val, 1); } @@ -411,7 +411,7 @@ static int r820t_read_cache_reg(struct r820t_priv *priv, int reg) return -EINVAL; } -static int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val, +static noinline_if_stackbloat int r820t_write_reg_mask(struct r820t_priv *priv, u8 reg, u8 val, u8 bit_mask) { int rc = r820t_read_cache_reg(priv, reg); -- 2.9.0
[PATCH v2 06/11] dvb-frontends: reduce stack size in i2c access
A typical code fragment was copied across many dvb-frontend drivers and causes large stack frames when built with -fsanitize-address-use-after-scope, e.g. drivers/media/dvb-frontends/cxd2841er.c:3225:1: error: the frame size of 3992 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] drivers/media/dvb-frontends/cxd2841er.c:3404:1: error: the frame size of 3136 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] drivers/media/dvb-frontends/stv0367.c:3143:1: error: the frame size of 4016 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] drivers/media/dvb-frontends/stv090x.c:3430:1: error: the frame size of 5312 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] drivers/media/dvb-frontends/stv090x.c:4248:1: error: the frame size of 4872 bytes is larger than 3072 bytes [-Werror=frame-larger-than=] By marking the register access functions as noinline_if_stackbloat, we can completely avoid this problem. Signed-off-by: Arnd Bergmann --- drivers/media/dvb-frontends/ascot2e.c | 3 ++- drivers/media/dvb-frontends/cxd2841er.c | 4 ++-- drivers/media/dvb-frontends/drx39xyj/drxj.c | 14 +++--- drivers/media/dvb-frontends/helene.c| 4 ++-- drivers/media/dvb-frontends/horus3a.c | 2 +- drivers/media/dvb-frontends/itd1000.c | 2 +- drivers/media/dvb-frontends/mt312.c | 2 +- drivers/media/dvb-frontends/si2165.c| 14 +++--- drivers/media/dvb-frontends/stb0899_drv.c | 2 +- drivers/media/dvb-frontends/stb6100.c | 2 +- drivers/media/dvb-frontends/stv0367.c | 2 +- drivers/media/dvb-frontends/stv090x.c | 2 +- drivers/media/dvb-frontends/stv6110.c | 2 +- drivers/media/dvb-frontends/stv6110x.c | 2 +- drivers/media/dvb-frontends/tda8083.c | 2 +- drivers/media/dvb-frontends/zl10039.c | 2 +- 16 files changed, 31 insertions(+), 30 deletions(-) diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c index 0ee0df53b91b..da1d1fc03c5e 100644 --- a/drivers/media/dvb-frontends/ascot2e.c +++ b/drivers/media/dvb-frontends/ascot2e.c @@ -153,7 +153,8 @@ static int ascot2e_write_regs(struct ascot2e_priv *priv, return 0; } -static int ascot2e_write_reg(struct ascot2e_priv *priv, u8 reg, u8 val) +static noinline_if_stackbloat int ascot2e_write_reg(struct ascot2e_priv *priv, + u8 reg, u8 val) { return ascot2e_write_regs(priv, reg, &val, 1); } diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c index ce37dc2e89c7..6b851a948ce0 100644 --- a/drivers/media/dvb-frontends/cxd2841er.c +++ b/drivers/media/dvb-frontends/cxd2841er.c @@ -258,7 +258,7 @@ static int cxd2841er_write_regs(struct cxd2841er_priv *priv, return 0; } -static int cxd2841er_write_reg(struct cxd2841er_priv *priv, +static noinline_if_stackbloat int cxd2841er_write_reg(struct cxd2841er_priv *priv, u8 addr, u8 reg, u8 val) { return cxd2841er_write_regs(priv, addr, reg, &val, 1); @@ -306,7 +306,7 @@ static int cxd2841er_read_regs(struct cxd2841er_priv *priv, return 0; } -static int cxd2841er_read_reg(struct cxd2841er_priv *priv, +static noinline_if_stackbloat int cxd2841er_read_reg(struct cxd2841er_priv *priv, u8 addr, u8 reg, u8 *val) { return cxd2841er_read_regs(priv, addr, reg, val, 1); diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c b/drivers/media/dvb-frontends/drx39xyj/drxj.c index 14040c915dbb..ec5b13ca630b 100644 --- a/drivers/media/dvb-frontends/drx39xyj/drxj.c +++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c @@ -1516,7 +1516,7 @@ static int drxdap_fasi_read_block(struct i2c_device_addr *dev_addr, * **/ -static int drxdap_fasi_read_reg16(struct i2c_device_addr *dev_addr, +static noinline_if_stackbloat int drxdap_fasi_read_reg16(struct i2c_device_addr *dev_addr, u32 addr, u16 *data, u32 flags) { @@ -1549,7 +1549,7 @@ static int drxdap_fasi_read_reg16(struct i2c_device_addr *dev_addr, * **/ -static int drxdap_fasi_read_reg32(struct i2c_device_addr *dev_addr, +static noinline_if_stackbloat int drxdap_fasi_read_reg32(struct i2c_device_addr *dev_addr, u32 addr, u32 *data, u32 flags) { @@ -1722,7 +1722,7 @@ static int drxdap_fasi_write_block(struct i2c_device_addr *dev_addr, * **/ -static int drxdap_fasi_write_reg16(struct i2c_device_addr *dev_addr, +static noinline_if_stackbloat int drxdap_fasi_write_reg16(struct i2c_device_addr *dev_addr, u32 addr, u16 data, u32 flags) { @@ -1795,7 +1795,7 @@ static int drxdap
Re: [PATCH 5/8] v4l2-flash: Flash ops aren't mandatory
Hi Jacek, On Wed, Jun 14, 2017 at 11:14:13PM +0200, Jacek Anaszewski wrote: > Hi Sakari, > > On 06/14/2017 11:47 AM, Sakari Ailus wrote: > > None of the flash operations are not mandatory and therefore there should > > be no need for the flash ops structure either. Accept NULL. > > > > Signed-off-by: Sakari Ailus > > --- > > drivers/media/v4l2-core/v4l2-flash-led-class.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c > > b/drivers/media/v4l2-core/v4l2-flash-led-class.c > > index 6d69119..fdb79da 100644 > > --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c > > +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c > > @@ -18,7 +18,7 @@ > > #include > > > > #define has_flash_op(v4l2_flash, op) \ > > - (v4l2_flash && v4l2_flash->ops->op) > > + (v4l2_flash && v4l2_flash->ops && v4l2_flash->ops->op) > > This change doesn't seem to be related to the patch subject. Yes, it is: if there's a chance that ops is NULL, then you have to test here you actually have the ops struct around. The test is no longer in v4l2_flash_init(). > > > #define call_flash_op(v4l2_flash, op, arg) \ > > (has_flash_op(v4l2_flash, op) ? \ > > @@ -618,7 +618,7 @@ struct v4l2_flash *v4l2_flash_init( > > struct v4l2_subdev *sd; > > int ret; > > > > - if (!fled_cdev || !ops || !config) > > + if (!fled_cdev || !config) > > return ERR_PTR(-EINVAL); > > > > led_cdev = &fled_cdev->led_cdev; > > -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH 6/8] leds: as3645a: Add LED flash class driver
Hi Sakari, I have two remarks in the code below. On 06/14/2017 11:47 AM, Sakari Ailus wrote: > From: Sakari Ailus > > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver > is based on that. > > Signed-off-by: Sakari Ailus > --- > MAINTAINERS | 6 + > drivers/leds/Kconfig| 8 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-as3645a.c | 744 > > 4 files changed, 759 insertions(+) > create mode 100644 drivers/leds/leds-as3645a.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 053c3bd..c7682af 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2067,6 +2067,12 @@ F: arch/arm64/ > F: Documentation/arm64/ > > AS3645A LED FLASH CONTROLLER DRIVER > +M: Sakari Ailus > +L: linux-l...@vger.kernel.org > +S: Maintained > +F: drivers/leds/leds-as3645a.c > + > +AS3645A LED FLASH CONTROLLER DRIVER > M: Laurent Pinchart > L: linux-media@vger.kernel.org > T: git git://linuxtv.org/media_tree.git > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 6c29998..9fb1d86 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -58,6 +58,14 @@ config LEDS_AAT1290 > help >This option enables support for the LEDs on the AAT1290. > > +config LEDS_AS3645A > + tristate "AS3645A LED flash controller support" > + depends on I2C && LEDS_CLASS_FLASH > + help > + Enable LED flash class support for AS3645A LED flash > + controller. V4L2 flash API is provided as well if > + CONFIG_V4L2_FLASH_API is enabled. > + > config LEDS_BCM6328 > tristate "LED Support for Broadcom BCM6328" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 45f1339..b4def76 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o > # LED Platform Drivers > obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o > obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o > +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o > obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o > obj-$(CONFIG_LEDS_BD2802)+= leds-bd2802.o > diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c > new file mode 100644 > index 000..862d1b5 > --- /dev/null > +++ b/drivers/leds/leds-as3645a.c > @@ -0,0 +1,744 @@ > +/* > + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver > + * > + * Copyright (C) 2008-2011 Nokia Corporation > + * Copyright (c) 2011, 2017 Intel Corporation. > + * > + * Based on drivers/media/i2c/as3645a.c. > + * > + * Contact: Sakari Ailus > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / > 50) > +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * > 1000) > + > +/* Register definitions */ > + > +/* Read-only Design info register: Reset state: 0001 */ > +#define AS_DESIGN_INFO_REG 0x00 > +#define AS_DESIGN_INFO_FACTORY(x)(((x) >> 4)) > +#define AS_DESIGN_INFO_MODEL(x) ((x) & 0x0f) > + > +/* Read-only Version control register: Reset state: > + * for first engineering samples > + */ > +#define AS_VERSION_CONTROL_REG 0x01 > +#define AS_VERSION_CONTROL_RFU(x)(((x) >> 4)) > +#define AS_VERSION_CONTROL_VERSION(x)((x) & 0x0f) > + > +/* Read / Write (Indicator and timer register): Reset state: > */ > +#define AS_INDICATOR_AND_TIMER_REG 0x02 > +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 > +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT4 > +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 > + > +/* Read / Write (Current set register): Reset state: 0110 1001 */ > +#define AS_CURRENT_SET_REG 0x03 > +#define AS_CURRENT_ASSIST_LIGHT_SHIFT0 > +#define AS_CURRENT_LED_DET_ON(1 << 3) > +#define AS_CURRENT_FLASH_CURRENT_SHIFT 4 > + > +/* Read / Write (Control register): Reset state: 1011 0100 */ > +#define AS_CONTROL_REG
Re: [PATCH 5/8] v4l2-flash: Flash ops aren't mandatory
Hi Sakari, On 06/14/2017 11:47 AM, Sakari Ailus wrote: > None of the flash operations are not mandatory and therefore there should > be no need for the flash ops structure either. Accept NULL. > > Signed-off-by: Sakari Ailus > --- > drivers/media/v4l2-core/v4l2-flash-led-class.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c > b/drivers/media/v4l2-core/v4l2-flash-led-class.c > index 6d69119..fdb79da 100644 > --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c > +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c > @@ -18,7 +18,7 @@ > #include > > #define has_flash_op(v4l2_flash, op) \ > - (v4l2_flash && v4l2_flash->ops->op) > + (v4l2_flash && v4l2_flash->ops && v4l2_flash->ops->op) This change doesn't seem to be related to the patch subject. > #define call_flash_op(v4l2_flash, op, arg) \ > (has_flash_op(v4l2_flash, op) ? \ > @@ -618,7 +618,7 @@ struct v4l2_flash *v4l2_flash_init( > struct v4l2_subdev *sd; > int ret; > > - if (!fled_cdev || !ops || !config) > + if (!fled_cdev || !config) > return ERR_PTR(-EINVAL); > > led_cdev = &fled_cdev->led_cdev; > -- Best regards, Jacek Anaszewski
Re: [PATCH 4/8] v4l2-flash: Use led_classdev instead of led_classdev_flash for indicator
Hi Sakari, On 06/14/2017 11:47 AM, Sakari Ailus wrote: > The V4L2 flash class initialisation expects struct led_classdev_flash that > describes an indicator but only uses struct led_classdev which is a field > iled_cdev in the struct. Use struct iled_cdev only. > > Signed-off-by: Sakari Ailus > --- > drivers/media/v4l2-core/v4l2-flash-led-class.c | 19 +++ > include/media/v4l2-flash-led-class.h | 6 +++--- > 2 files changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c > b/drivers/media/v4l2-core/v4l2-flash-led-class.c > index 7b82881..6d69119 100644 > --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c > +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c > @@ -110,7 +110,7 @@ static void v4l2_flash_set_led_brightness(struct > v4l2_flash *v4l2_flash, > led_set_brightness_sync(&v4l2_flash->fled_cdev->led_cdev, > brightness); > } else { > - led_set_brightness_sync(&v4l2_flash->iled_cdev->led_cdev, > + led_set_brightness_sync(v4l2_flash->iled_cdev, > brightness); > } > } > @@ -133,7 +133,7 @@ static int v4l2_flash_update_led_brightness(struct > v4l2_flash *v4l2_flash, > return 0; > led_cdev = &v4l2_flash->fled_cdev->led_cdev; > } else { > - led_cdev = &v4l2_flash->iled_cdev->led_cdev; > + led_cdev = v4l2_flash->iled_cdev; > } > > ret = led_update_brightness(led_cdev); > @@ -529,8 +529,7 @@ static int v4l2_flash_open(struct v4l2_subdev *sd, struct > v4l2_subdev_fh *fh) > struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd); > struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev; > struct led_classdev *led_cdev = &fled_cdev->led_cdev; > - struct led_classdev_flash *iled_cdev = v4l2_flash->iled_cdev; > - struct led_classdev *led_cdev_ind = NULL; > + struct led_classdev *led_cdev_ind = v4l2_flash->iled_cdev; > int ret = 0; > > if (!v4l2_fh_is_singular(&fh->vfh)) > @@ -543,9 +542,7 @@ static int v4l2_flash_open(struct v4l2_subdev *sd, struct > v4l2_subdev_fh *fh) > > mutex_unlock(&led_cdev->led_access); > > - if (iled_cdev) { > - led_cdev_ind = &iled_cdev->led_cdev; > - > + if (led_cdev_ind) { > mutex_lock(&led_cdev_ind->led_access); > > led_sysfs_disable(led_cdev_ind); > @@ -578,7 +575,7 @@ static int v4l2_flash_close(struct v4l2_subdev *sd, > struct v4l2_subdev_fh *fh) > struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd); > struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev; > struct led_classdev *led_cdev = &fled_cdev->led_cdev; > - struct led_classdev_flash *iled_cdev = v4l2_flash->iled_cdev; > + struct led_classdev *led_cdev_ind = v4l2_flash->iled_cdev; > int ret = 0; > > if (!v4l2_fh_is_singular(&fh->vfh)) > @@ -593,9 +590,7 @@ static int v4l2_flash_close(struct v4l2_subdev *sd, > struct v4l2_subdev_fh *fh) > > mutex_unlock(&led_cdev->led_access); > > - if (iled_cdev) { > - struct led_classdev *led_cdev_ind = &iled_cdev->led_cdev; > - > + if (led_cdev_ind) { > mutex_lock(&led_cdev_ind->led_access); > led_sysfs_enable(led_cdev_ind); > mutex_unlock(&led_cdev_ind->led_access); > @@ -614,7 +609,7 @@ static const struct v4l2_subdev_ops v4l2_flash_subdev_ops; > struct v4l2_flash *v4l2_flash_init( > struct device *dev, struct fwnode_handle *fwn, > struct led_classdev_flash *fled_cdev, > - struct led_classdev_flash *iled_cdev, > + struct led_classdev *iled_cdev, > const struct v4l2_flash_ops *ops, > struct v4l2_flash_config *config) > { > diff --git a/include/media/v4l2-flash-led-class.h > b/include/media/v4l2-flash-led-class.h > index f9dcd54..54e31a8 100644 > --- a/include/media/v4l2-flash-led-class.h > +++ b/include/media/v4l2-flash-led-class.h > @@ -85,7 +85,7 @@ struct v4l2_flash_config { > */ > struct v4l2_flash { > struct led_classdev_flash *fled_cdev; > - struct led_classdev_flash *iled_cdev; > + struct led_classdev *iled_cdev; > const struct v4l2_flash_ops *ops; > > struct v4l2_subdev sd; > @@ -124,7 +124,7 @@ static inline struct v4l2_flash > *v4l2_ctrl_to_v4l2_flash(struct v4l2_ctrl *c) > struct v4l2_flash *v4l2_flash_init( > struct device *dev, struct fwnode_handle *fwn, > struct led_classdev_flash *fled_cdev, > - struct led_classdev_flash *iled_cdev, > + struct led_classdev *iled_cdev, > const struct v4l2_flash_ops *ops, > struct v4l2_flash_config *config); > > @@ -140,7 +140,7 @@ void v4l2_flash_release(struct v4l2_flash *v4l2_flash); > static inline struct v4l2_flash *v4l2_flash_init( > struct device *dev, struct fwnode_handle *fwn, > struct l
Re: [RFC 0/2] BCM283x Camera Receiver driver
On 14 June 2017 at 18:38, Hans Verkuil wrote: > On 06/14/2017 06:29 PM, Dave Stevenson wrote: >> >> Hi Hans. >> >> On 14 June 2017 at 16:42, Hans Verkuil wrote: >>> >>> Hi Dave, >>> >>> How does this driver relate to this staging driver: >>> >>> drivers/staging/vc04_services/bcm2835-camera/ >>> >>> It's not obvious to me. >> >> >> drivers/staging/vc04_services/bcm2835-camera/ is using the VideoCore >> firmware to control Unicam, ISP, and all the tuner algorithms. The ARM >> gets delivered fully processed buffers from the VideoCore side. The >> firmware only has drivers for the Omnivision OV5647 and Sony IMX219 >> (and an unsupported one for the Toshiba TC358743). >> >> This driver is solely the Unicam block, reading the data in over >> CSI2/CCP2 from the sensor and writing it to memory. No ISP or control >> loops. >> Other than power management, this driver is running solely on the ARM >> with no involvement from the VideoCore firmware. >> The sensor driver is whatever suitable V4L2 subdevice driver you fancy >> attaching (as long as it supports CSI2, or eventually CCP2). > > > What is the interaction between these two drivers? Can they co-exist? > I would expect them to be mutually exclusive. Mutually exclusive for the same Unicam instance, yes. There are two Unicam instances on all BCM283x chips and both are brought out on the Compute Modules. You could run bcm2835-unicam on one and bcm2835-camera on the other if you so wished. The firmware checks whether the csi nodes in the device tree are active, and won't touch them if they are. >> >>> On 06/14/2017 05:15 PM, Dave Stevenson wrote: Hi All. This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera receiver peripheral on BCM283x, as used on Raspberry Pi. v4l2-compliance results depend on the sensor subdevice this is connected to. It passes the basic tests cleanly with TC358743, but objects with OV5647 fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0 Neither OV5647 nor Unicam support any controls. >>> >>> >>> >>> Are you compiling v4l2-compliance from the v4l-utils git repo? If not, >>> then please do so and run again. The version packaged by distros tends >>> to be seriously outdated. >> >> >> Yes, I'm building from v4l-utils.git. >> I updated within the last week, although you appear to have added 2 >> commits since (both CEC related). >> I'm on "ef074cf media-ctl: add colorimetry support" > > > But the line with that error is at line number 587 in my repo, not 574. > So I'm a bit suspicious. > > Anyway, can you give the output of 'v4l2-ctl -l'? Will do tomorrow. I'll update to the latest v4l2-conformance too to avoid confusion. >> I must admit to not having got OV5647 to stream with the current driver register settings. It works with a set of register settings for VGA RAW10. I also have a couple of patches pending for OV5647, but would like to understand the issues better before sending them out. Two queries I do have in V4L2-land: - When s_dv_timings or s_std is called, is the format meant to be updated automatically? >>> >>> >>> >>> Yes. Exception is if the new timings/std is exactly the same as the old >>> timings/std, in that case you can just return 0 and do nothing. >> >> >> OK, can do that. >> Even if we're already streaming? >>> >>> >>> That's not allowed. Return -EBUSY in that case. >> >> >> Also reasonable. >> So if the TC358743 flags a source change we have to stop streaming, >> set the new timings (which will update the format), and start up again >> with fresh buffers. That's what I was expecting, but wanted to >> confirm. > > > Correct. In theory there are ways around this provided the buffers are > large enough to accommodate the new format size, but nobody actually > supports that (might change in the not-to-distant future). > >> Some existing drivers seem to, but others don't. - With s_fmt, is sizeimage settable by the application in the same way as bytesperline? >>> >>> >>> >>> No, the driver will fill in this field, overwriting anything the >>> application put there. >>> >>> bytesperline IS settable, but most drivers will ignore what userspace >>> did and overwrite this as well. >>> >>> Normally the driver knows about HW requirements and will set sizeimage >>> to something that will work (e.g. make sure it is a multiple of 16 >>> lines). >> >> >> There are subtly different requirements in different hardware blocks :-( >> eg Unicam needs bytesperline to be a multiple of 16 bytes,whilst the >> ISP requires a multiple of 32. >> The vertical padding is generally where we're doing software >> processing on the VideoCore side as it's easier to just leave the the >> 16 way SIMD processor running all 16 ways, hence needing scratch space >> to avoid reading beyond buffers. >> >> The main consumer is likely to be the ISP and that doesn't need >> vertical context, s
Re: [patch, libv4l]: add sdlcam example for testing digital still camera functionality
Hi! > > Utilities like v4l2-ctl are tied closely to the kernel and are updated > > whenever > > new APIs appear. But yet another viewer? > > > > Mauro, I find that v4l-utils is a bit polluted with non-core utilities. > > IMHO it should only contain the core libv4l2, core utilities and > > driver-specific > > utilities. I wonder if we should make a media-utils-contrib for all the > > non-core > > stuff. > > > > What is your opinion? > > One of the purposes the v4l-utils repository has is that the distributions > get these programs included to their v4l-utils package as it's typically > called. It's debatable whether or how much it should contain device specific > or otherwise random projects, but having a common location for such programs > has clear benefits, too. > > Based on how this one looks it is definitely not an end user application (I > hope I'm not miscategorising it) and as Pavel mentioned, it has been useful > in testing automatic focus / gain control on N900. Well, I intend to take some photos with it. But yes, it is more useful for testing -- it is hard to test digital camera without taking photos -- and eventually I'd like to turn most of it into a library which "real" camera application would link to. But that's future. For now I'd like to have something I can test the kernel drivers with, and perhaps take some photos in the process. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH v5 1/2] media: i2c: adv748x: add adv748x driver
From: Kieran Bingham Provide support for the ADV7481 and ADV7482. The driver is modelled with 4 subdevices to allow simultaneous streaming from the AFE (Analog front end) and HDMI inputs though two CSI TX entities. The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked to the TXB CSI bus. The driver is based on a prototype by Koji Matsuoka in the Renesas BSP, and an earlier rework by Niklas Söderlund. Signed-off-by: Kieran Bingham --- v2: - Implement DT parsing - adv748x: Add CSI2 entity - adv748x: Rework pad allocations and fmts - Give AFE 8 input pads and move pad defines - Use the enums to ensure pads are referenced correctly. - adv748x: Rename AFE/HDMI entities Now they are 'just afe' and 'just hdmi' - Reorder the entity enum and structures - Added pad-format for the CSI2 entities - CSI2 s_stream pass through - CSI2 control pass through (with link following) v3: - dt: Extend DT documentation to specify interrupt mappings - simplify adv748x_parse_dt - core: Add banner to header file describing ADV748x variants - Use entity structure pointers rather than global state pointers where possible - afe: Reduce indent on afe_status - hdmi: Add error checking to the bt->pixelclock values. - Remove all unnecessary pad checks: handled by core - Fix all probe cleanup paths - adv748x_csi2_probe() now fails if it has no endpoint - csi2: Fix small oneliners for is_txa and get_remote_sd() - csi2: Fix checkpatch warnings - csi2: Fix up s_stream pass-through - csi2: Fix up Pixel Rate passthrough - csi2: simplify adv748x_csi2_get_pad_format() - Remove 'async notifiers' from AFE/HDMI Using async notifiers was overkill, when we have access to the subdevices internally and can register them directly. - Use state lock in control handlers and clean up s_ctrls - remove _interruptible mutex locks v4: - all: Convert hex 0xXX to lowercase - all: Use defines instead of hardcoded register values - all: Use regmap - afe, csi2, hdmi: _probe -> _init - afe, csi2, hdmi: _remove -> _cleanup - afe, hdmi: Convert pattern generator to a control - afe, hdmi: get/set-fmt refactor - afe, hdmi, csi2: Convert to internal calls for pixelrate - afe: Allow the AFE to configure the Input Select from DT - afe: Reduce indent on adv748x_afe_status switch - afe: Remove ununsed macro definitions AIN0-7 - afe: Remove extraneous control checks handled by core - afe: Comment fixups - afe: Rename error label - afe: Correct control names on the SDP - afe: Use AIN0-7 rather than AIN1-8 to match ports and MC pads - core: adv748x_parse_dt references to nodes, and catch multiple endpoints in a port. - core: adv748x_dt_cleanup to simplify releasing DT nodes - core: adv748x_print_info renamed to adv748x_identify_chip - core: reorganise ordering of probe sequence - core: No need for of_match_ptr - core: Fix up i2c read/writes (regmap still on todo list) - core: Set specific functions from the entities on subdev-init - core: Use kzalloc for state instead of devm - core: Improve probe error reporting - core: Track unknown BIT(6) in tx{a,b}_power - csi2: Improve adv748x_csi2_get_remote_sd as adv748x_csi2_get_source_sd - csi2: -EPIPE instead of -ENODEV - csi2: adv_dbg, instead of adv_info - csi2: adv748x_csi2_set_format fix - csi2: remove async notifier and sd member variables - csi2: register links from the CSI2 - csi2: create virtual channel helper function - dt: Remove numbering from endpoints - dt: describe ports and interrupts as optional - dt: Re-tab - hdmi: adv748x_hdmi_have_signal -> adv748x_hdmi_has_signal - hdmi: fix adv748x_hdmi_read_pixelclock return checks - hdmi: improve adv748x_hdmi_set_video_timings - hdmi: Fix tmp variable as polarity - hdmi: Improve adv748x_hdmi_s_stream - hdmi: Clean up adv748x_hdmi_s_ctrl register definitions and usage - hdmi: Fix up adv748x_hdmi_s_dv_timings with macro names for register - hdmi: Add locking to adv748x_hdmi_g_dv_timings writes and locking - hdmi: adv748x_hdmi_set_de_timings function added to clarify DE writes - hdmi: Use CP in control register naming to match component processor - hdmi: clean up adv748x_hdmi_query_dv_timings() - KConfig: Fix up dependency and capitalisation v5: - afe,hdmi: _set_pixelrate -> _propagate_pixelrate - hdmi: Fix arm32 compilation failure : Use DIV_ROUND_CLOSEST_ULL - core: remove unused link functions - csi2: Use immutable links for HDMI->TXA, AFE->TXB Documentation/devicetree/bindings/media/i2c/adv748x.txt | 96 +- MAINTAINERS | 6 +- drivers/media/i2c/Kconfig | 11 +- drivers/media/i2c/Makefile | 1 +- drivers/media/i2c/adv748x/Makefile | 7 +- drivers/media/i2c/adv748x/adv748x-afe.c | 570 ++- drivers/media/i2c/adv748x/adv748x-core.c| 831 +- drivers/media/i2c/adv748x/adv748x-csi2.c
Re: v4l2-fwnode: status, plans for merge, any branch to merge against?
Hi! > > > > Are there any news about the fwnode branch? > > > > > > > > I have quite usable camera, but it is still based on > > > > 982e8e40390d26430ef106fede41594139a4111c (that's v4.10). It would be > > > > good to see fwnode stuff upstream... are there any plans for that? > > > > > > > > Is there stable branch to which I could move the stuff? > > > > > > What's relevant for most V4L2 drivers is in linux-media right now. > > > > > > There are new features that will take some time to get in. The trouble has > > > been, and continue to be, that the patches need to go through various > > > trees > > > so it'll take some time for them to be merged. > > > > > > I expect to have most of them in during the next merge window. > > > > So git://linuxtv.org/media_tree.git branch master is the right one to > > work one? > > I also pushed the rebased ccp2 branch there: > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2> > > It's now right on the top of media-tree master. Thanks, that's what I was looking for. Unfortunately, it does not compile. CC drivers/media/platform/omap3isp/ispcsiphy.o drivers/media/platform/omap3isp/isp.c: In function 'isp_fwnode_parse': drivers/media/platform/omap3isp/isp.c:2029:35: error: 'fwn' undeclared (first use in this function) drivers/media/platform/omap3isp/isp.c:2029:35: note: each undeclared identifier is reported only once for each function it appears in drivers/media/platform/omap3isp/isp.c:2029:2: error: incompatible type for argument 2 of 'v4l2_fwnode_endpoint_parse' In file included from drivers/media/platform/omap3isp/isp.c:67:0: ./include/media/v4l2-fwnode.h:112:5: note: expected 'struct v4l2_fwnode_endpoint *' but argument is of type 'struct v4l2_fwnode_endpoint' scripts/Makefile.build:302: recipe for target 'drivers/media/platform/omap3isp/isp.o' failed make[4]: *** [drivers/media/platform/omap3isp/isp.o] Error 1 make[4]: *** Waiting for unfinished jobs scripts/Makefile.build:561: recipe for target 'drivers/media/platform/omap3isp' failed make[3]: *** [drivers/media/platform/omap3isp] Error 2 You can get my config if needed. Now let me try to fix it... It was not too bad, good. commit 364340e7aa037535a65d2ef2a1711c97d233fede Author: Pavel Date: Wed Jun 14 21:40:37 2017 +0200 Fix compilation of omap3isp/isp.c. Signed-off-by: Pavel Machek diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 4ca3fc9..b80debf 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2026,7 +2026,7 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode, isd->bus = buscfg; - ret = v4l2_fwnode_endpoint_parse(fwn, vep); + ret = v4l2_fwnode_endpoint_parse(fwnode, &vep); if (ret) return ret; Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
RE: [PATCH] ov5670: Add Omnivision OV5670 5M sensor support
Sakari, Thanks for the review. Version 3 uploaded. See the responses below. Got some syntax suggestions from automated scripts on v3, will submit v4 soon. Thanks, Chiran. -Original Message- >From: Sakari Ailus [mailto:sakari.ai...@iki.fi] >Sent: Friday, May 26, 2017 1:29 AM >To: Rapolu, Chiranjeevi >Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian Xu >; Mani, Rajmohan ; Yang, >Hyungwoo ; tf...@chromium.org >Subject: Re: [PATCH] ov5670: Add Omnivision OV5670 5M sensor support > >Hi Chiranjeevi, > >On Fri, May 26, 2017 at 01:19:59AM +, Rapolu, Chiranjeevi wrote: >> >> +/* Analog gain controls from sensor */ >> >> +#define ANALOG_GAIN_MIN 0 >> >> +#define ANALOG_GAIN_MAX 8191 >> >> +#define ANALOG_GAIN_STEP1 >> >> +#define ANALOG_GAIN_DEFAULT 128 >> >> + >> >> +/* Exposure controls from sensor */ >> >> +#define EXPOSURE_MIN0 >> >> +#define EXPOSURE_MAX1048575 >> >> +#define EXPOSURE_STEP 1 >> >> +#define EXPOSURE_DEFAULT47232 >> > >> >Are these values dependent on sensor configuration i.e. in the case >> >of this driver modes? >> > >> Default values for a given resolutions can be fine-tuned. I think it >> is up to the HAL/application as to what default exposure for a given >> resolution. Driver has the support to change per application. > >How about the minimum and maximum values? Are they dependent on other >configuration? Exposure is now dependent on VBLANK and mode, modifies exposure-ranges dynamically through __v4l2_ctrl_modify_range(). > >... > >> >> +/* Write a list of registers */ >> >> +static int ov5670_write_regs(struct ov5670 *ov5670, >> >> + const struct ov5670_reg *regs, >> >> + int len) >> > >> >How about using unsigned int for len? >> > >> Now u32 len. >> >> +{ >> >> + struct i2c_client *client = v4l2_get_subdevdata(&ov5670->sd); >> >> + int i; >> > >> >i as well. >> > >> Now u32 i > >An insigned int should do. Types with explicit widths should be only used when >the exact value ranges is known, e.g. a 32-bit or 16-bit register. Now, unsigned int. > >... > >> >> +static int ov5670_set_stream(struct v4l2_subdev *sd, int enable) { >> >> + struct ov5670 *ov5670 = to_ov5670(sd); >> >> + int ret = 0; >> >> + >> >> + mutex_lock(&ov5670->mutex); >> >> + if (ov5670->streaming == enable) { >> >> + mutex_unlock(&ov5670->mutex); >> >> + return 0; >> >> + } >> >> + mutex_unlock(&ov5670->mutex); >> >> + >> >> + if (enable) { >> >> + ret = ov5670_prepare_streaming(ov5670); >> > >> >ov5670_prepare_streaming() and ov5670_start_streaming() are always >> >called sequientally. Could you combine the two? >> > >> >About locking --- you would likely benefit from an unlocked variant >> >of v4l2_ctrl_handler_setup(). I uploaded it here, let me know if it >> >works for >> >you: >> > >> >https://git.linuxtv.org/sailus/media_tree.git/log/?h=ctrl-setup- >> >unlocked> >> > >> Tried unlocked __v4l2_ctrl_handler_setup(), working fine, used. >> Can you push this patch? > >Great! I sent it to the list a moment ago. :-) Thanks, Sakari. > >> >> +static struct i2c_driver ov5670_i2c_driver = { >> > >> >const? >> > >> Made const > >I later on figured out this was probably a bad suggestion. The driver >registration changes the i2c_driver struct, causing a compiler warning. Yes, no const. > >> >> + .driver = { >> >> + .name = "ov5670", >> >> + .owner = THIS_MODULE, >> >> + .pm = &ov5670_pm_ops, >> >> + .acpi_match_table = ACPI_PTR(ov5670_acpi_ids), >> >> + }, >> >> + .probe = ov5670_probe, >> >> + .remove = ov5670_remove, >> >> + .id_table = ov5670_id_table, >> >> +}; >> >> + >> >> +module_i2c_driver(ov5670_i2c_driver); >> >> + >> >> +MODULE_AUTHOR("Rapolu, Chiranjeevi >> >> +"); MODULE_AUTHOR("Yang, Hyungwoo >> >> +"); MODULE_AUTHOR("Pu, Yuning >> >> +"); MODULE_DESCRIPTION("Omnivision ov5670 >> >> +sensor driver"); MODULE_LICENSE("GPL"); > >-- >Kind regards, > >Sakari Ailus >e-mail: sakari.ai...@iki.fiXMPP: sai...@retiisi.org.uk
Re: [PATCH] uvcvideo: Hardcoded CTRL_QUERY GET_LEN for a lying device
On 06/04/2017 03:41 PM, Alexandre Macabies wrote: > Hello, I forgot to Cc: the full list of maintainers for this patch. This follow-up includes them. Sorry for the noise! My original email & patch is quoted below. Best, Alexandre > This thread comes after two others[1][2] about a similar issue. > > I own a USB video microscope[3] from Dino-Lite. Even if the constructor does > not advertise it as being supported on Linux, it is mostly a "good citizen" > camera: it registers as a standard USB video device and as such, it is > properly > recognized by uvcvideo. > > This device is equipped with an integrated illuminator/lamp -- a set of LEDs. > After some research (using a USB sniffer) I managed to identify the > non-standard XU control used to switch this lamp on and off: one shall send > either 80 01 f0 (off) or 80 01 f1 (on) to XU control unit 4 selector 3. > > So at first I tried to send a raw ctrl_set using: > > $ uvcdynctrl -S 4:3 8001f0 > [...] > query control size of : 1 > [...] > ERROR: Unable to set the control value: Invalid argument. (Code: 3) > > Indeed, the device reports this XU as being only 1 in length, but the payload > has to be 3 bytes. So I assume there is a bug (or deliberate inaccuracy) in > the > GET_LEN reply from the device firmware. To overcome this issue, I compiled > a patched version of uvcvideo in which uvc_query_ctrl[4] returns an hardcoded > size of 3 for this specific device & UX control. I was finally able to switch > the lamp on and off: > > $ uvcdynctrl -S 4:3 8001f0 > [39252.854261] uvcvideo: Fixing USB a168:0870 UX control 4/3 len: 1 -> 3 > [...] > query control size of : 3 > [...] > set value of : (LE)0x8001f0 (BE)0xf00180 > [lamp goes off] > > You can find the patch below. I abstracted it in the spirit of > uvc_ctrl_fixup_xu_info[5] so we can add more entries to the table in the > future. What do you think, would it be relevant to merge? AFAICT there is no > API in uvcvideo or v4l for controlling this kind of illuminator/lamp features, > so giving userland the ability to control the devices via XU by lying seems to > be the only solution. > > Best, > > Alexandre > > [1] "Dino-Lite uvc support", 2008, > https://sourceforge.net/p/linux-uvc/mailman/message/29831153/ > [2] "switching light on device Dino-Lite Premier", 2013, > https://sourceforge.net/p/linux-uvc/mailman/message/31219122/ > [3] https://www.dinolite.us/products/digital-microscopes/usb/basic/am4111t > [4] > http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_video.c#L72 > [5] > http://elixir.free-electrons.com/linux/v4.11/source/drivers/media/usb/uvc/uvc_ctrl.c#L1593 > > Signed-off-by: Alexandre Macabies > > --- > drivers/media/usb/uvc/uvc_video.c | 37 + > 1 file changed, 37 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_video.c > b/drivers/media/usb/uvc/uvc_video.c > index 07a6c833ef7b..839dc02b4f33 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -69,6 +69,40 @@ static const char *uvc_query_name(__u8 query) > } > } > > +static void uvc_fixup_query_ctrl_len(const struct uvc_device *dev, __u8 unit, > + __u8 cs, void *data) > +{ > + struct uvc_ctrl_fixup { > + struct usb_device_id id; > + u8 unit; > + u8 selector; > + u16 len; > + }; > + > + static const struct uvc_ctrl_fixup fixups[] = { > + // Dino-Lite Premier (AM4111T) > + { { USB_DEVICE(0xa168, 0x0870) }, 4, 3, 3 }, > + }; > + > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(fixups); ++i) { > + if (!usb_match_one_id(dev->intf, &fixups[i].id)) > + continue; > + > + if (!(fixups[i].unit == unit && fixups[i].selector == cs)) > + continue; > + > + uvc_trace(UVC_TRACE_CONTROL, > + "Fixing USB %04x:%04x %u/%u GET_LEN: %u -> %u", > + fixups[i].id.idVendor, fixups[i].id.idProduct, > + unit, cs, > + le16_to_cpup((__le16 *)data), fixups[i].len); > + *((__le16 *)data) = cpu_to_le16(fixups[i].len); > + break; > + } > +} > + > int uvc_query_ctrl(struct uvc_device *dev, __u8 query, __u8 unit, > __u8 intfnum, __u8 cs, void *data, __u16 size) > { > @@ -83,6 +117,9 @@ int uvc_query_ctrl(struct uvc_device *dev, __u8 query, > __u8 unit, > return -EIO; > } > > + if (query == UVC_GET_LEN && size == 2) > + uvc_fixup_query_ctrl_len(dev, unit, cs, data); > + > return 0; > } > > > -- > 2.13.0 >
Re: [RFC 0/2] BCM283x Camera Receiver driver
On 06/14/2017 06:29 PM, Dave Stevenson wrote: Hi Hans. On 14 June 2017 at 16:42, Hans Verkuil wrote: Hi Dave, How does this driver relate to this staging driver: drivers/staging/vc04_services/bcm2835-camera/ It's not obvious to me. drivers/staging/vc04_services/bcm2835-camera/ is using the VideoCore firmware to control Unicam, ISP, and all the tuner algorithms. The ARM gets delivered fully processed buffers from the VideoCore side. The firmware only has drivers for the Omnivision OV5647 and Sony IMX219 (and an unsupported one for the Toshiba TC358743). This driver is solely the Unicam block, reading the data in over CSI2/CCP2 from the sensor and writing it to memory. No ISP or control loops. Other than power management, this driver is running solely on the ARM with no involvement from the VideoCore firmware. The sensor driver is whatever suitable V4L2 subdevice driver you fancy attaching (as long as it supports CSI2, or eventually CCP2). What is the interaction between these two drivers? Can they co-exist? I would expect them to be mutually exclusive. On 06/14/2017 05:15 PM, Dave Stevenson wrote: Hi All. This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera receiver peripheral on BCM283x, as used on Raspberry Pi. v4l2-compliance results depend on the sensor subdevice this is connected to. It passes the basic tests cleanly with TC358743, but objects with OV5647 fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0 Neither OV5647 nor Unicam support any controls. Are you compiling v4l2-compliance from the v4l-utils git repo? If not, then please do so and run again. The version packaged by distros tends to be seriously outdated. Yes, I'm building from v4l-utils.git. I updated within the last week, although you appear to have added 2 commits since (both CEC related). I'm on "ef074cf media-ctl: add colorimetry support" But the line with that error is at line number 587 in my repo, not 574. So I'm a bit suspicious. Anyway, can you give the output of 'v4l2-ctl -l'? I must admit to not having got OV5647 to stream with the current driver register settings. It works with a set of register settings for VGA RAW10. I also have a couple of patches pending for OV5647, but would like to understand the issues better before sending them out. Two queries I do have in V4L2-land: - When s_dv_timings or s_std is called, is the format meant to be updated automatically? Yes. Exception is if the new timings/std is exactly the same as the old timings/std, in that case you can just return 0 and do nothing. OK, can do that. Even if we're already streaming? That's not allowed. Return -EBUSY in that case. Also reasonable. So if the TC358743 flags a source change we have to stop streaming, set the new timings (which will update the format), and start up again with fresh buffers. That's what I was expecting, but wanted to confirm. Correct. In theory there are ways around this provided the buffers are large enough to accommodate the new format size, but nobody actually supports that (might change in the not-to-distant future). Some existing drivers seem to, but others don't. - With s_fmt, is sizeimage settable by the application in the same way as bytesperline? No, the driver will fill in this field, overwriting anything the application put there. bytesperline IS settable, but most drivers will ignore what userspace did and overwrite this as well. Normally the driver knows about HW requirements and will set sizeimage to something that will work (e.g. make sure it is a multiple of 16 lines). There are subtly different requirements in different hardware blocks :-( eg Unicam needs bytesperline to be a multiple of 16 bytes,whilst the ISP requires a multiple of 32. The vertical padding is generally where we're doing software processing on the VideoCore side as it's easier to just leave the the 16 way SIMD processor running all 16 ways, hence needing scratch space to avoid reading beyond buffers. The main consumer is likely to be the ISP and that doesn't need vertical context, so I'll look at removing the requirement there rather than forcing it in this driver. As long as we can set bytesperline (which is already supported) then that requirement of the ISP is already handled. It's up to you, but given the large sizes of the buffers the extra bit of padding doesn't really matter all that much. And if it would make it easier to digest by other HW blocks, then I would just add the padding. Note that users can also call VIDIOC_CREATE_BUFS instead of VIDIOC_REQBUFS in order to request buffers of a larger-than-needed size. I'm not sure if you are aware of that. yavta allows you to specify it on the command line, whilst v4l2-ctl doesn't. Some of the other parts of the Pi firmware have a requirement that the buffer is a multiple of 16 lines high, which can be matched by V4L2 if we can over-allocate the buffers by the app specifyi
Re: [RFC 0/2] BCM283x Camera Receiver driver
Hi Hans. On 14 June 2017 at 16:42, Hans Verkuil wrote: > Hi Dave, > > How does this driver relate to this staging driver: > > drivers/staging/vc04_services/bcm2835-camera/ > > It's not obvious to me. drivers/staging/vc04_services/bcm2835-camera/ is using the VideoCore firmware to control Unicam, ISP, and all the tuner algorithms. The ARM gets delivered fully processed buffers from the VideoCore side. The firmware only has drivers for the Omnivision OV5647 and Sony IMX219 (and an unsupported one for the Toshiba TC358743). This driver is solely the Unicam block, reading the data in over CSI2/CCP2 from the sensor and writing it to memory. No ISP or control loops. Other than power management, this driver is running solely on the ARM with no involvement from the VideoCore firmware. The sensor driver is whatever suitable V4L2 subdevice driver you fancy attaching (as long as it supports CSI2, or eventually CCP2). > On 06/14/2017 05:15 PM, Dave Stevenson wrote: >> >> Hi All. >> >> This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera >> receiver peripheral on BCM283x, as used on Raspberry Pi. >> >> v4l2-compliance results depend on the sensor subdevice this is >> connected to. It passes the basic tests cleanly with TC358743, >> but objects with OV5647 >> fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0 >> Neither OV5647 nor Unicam support any controls. > > > Are you compiling v4l2-compliance from the v4l-utils git repo? If not, > then please do so and run again. The version packaged by distros tends > to be seriously outdated. Yes, I'm building from v4l-utils.git. I updated within the last week, although you appear to have added 2 commits since (both CEC related). I'm on "ef074cf media-ctl: add colorimetry support" >> I must admit to not having got OV5647 to stream with the current driver >> register settings. It works with a set of register settings for VGA RAW10. >> I also have a couple of patches pending for OV5647, but would like to >> understand the issues better before sending them out. >> >> Two queries I do have in V4L2-land: >> - When s_dv_timings or s_std is called, is the format meant to >>be updated automatically? > > > Yes. Exception is if the new timings/std is exactly the same as the old > timings/std, in that case you can just return 0 and do nothing. OK, can do that. >> Even if we're already streaming? > > That's not allowed. Return -EBUSY in that case. Also reasonable. So if the TC358743 flags a source change we have to stop streaming, set the new timings (which will update the format), and start up again with fresh buffers. That's what I was expecting, but wanted to confirm. >>Some existing drivers seem to, but others don't. >> - With s_fmt, is sizeimage settable by the application in the same >>way as bytesperline? > > > No, the driver will fill in this field, overwriting anything the > application put there. > > bytesperline IS settable, but most drivers will ignore what userspace > did and overwrite this as well. > > Normally the driver knows about HW requirements and will set sizeimage > to something that will work (e.g. make sure it is a multiple of 16 lines). There are subtly different requirements in different hardware blocks :-( eg Unicam needs bytesperline to be a multiple of 16 bytes,whilst the ISP requires a multiple of 32. The vertical padding is generally where we're doing software processing on the VideoCore side as it's easier to just leave the the 16 way SIMD processor running all 16 ways, hence needing scratch space to avoid reading beyond buffers. The main consumer is likely to be the ISP and that doesn't need vertical context, so I'll look at removing the requirement there rather than forcing it in this driver. As long as we can set bytesperline (which is already supported) then that requirement of the ISP is already handled. >> yavta allows you to specify it on the command >> >>line, whilst v4l2-ctl doesn't. Some of the other parts of the Pi >>firmware have a requirement that the buffer is a multiple of 16 lines >>high, which can be matched by V4L2 if we can over-allocate the >>buffers by the app specifying sizeimage. But if I allow that, >>then I get a v4l2-compliance failure as the size doesn't get >>reset when switching from RGB3 to UYVY as it takes the request as >>a request to over-allocate. >> >> Apologies if I've messed up in sending these patches - so many ways >> to do something. > > > It looks fine at a glance. > > I will probably review this on Friday or Monday. But I need some > clarification > of the difference between this and the staging driver first. Thanks. Hopefully I've given you that clarification above. I'll fix the s_dv_timings and s_std handling, and s_fmt of sizeimage, but will wait for other review comments before sending a v2. Dave
Re: [RFC 0/2] BCM283x Camera Receiver driver
Hi Dave, How does this driver relate to this staging driver: drivers/staging/vc04_services/bcm2835-camera/ It's not obvious to me. On 06/14/2017 05:15 PM, Dave Stevenson wrote: Hi All. This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera receiver peripheral on BCM283x, as used on Raspberry Pi. v4l2-compliance results depend on the sensor subdevice this is connected to. It passes the basic tests cleanly with TC358743, but objects with OV5647 fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0 Neither OV5647 nor Unicam support any controls. Are you compiling v4l2-compliance from the v4l-utils git repo? If not, then please do so and run again. The version packaged by distros tends to be seriously outdated. I must admit to not having got OV5647 to stream with the current driver register settings. It works with a set of register settings for VGA RAW10. I also have a couple of patches pending for OV5647, but would like to understand the issues better before sending them out. Two queries I do have in V4L2-land: - When s_dv_timings or s_std is called, is the format meant to be updated automatically? Yes. Exception is if the new timings/std is exactly the same as the old timings/std, in that case you can just return 0 and do nothing. > Even if we're already streaming? That's not allowed. Return -EBUSY in that case. Some existing drivers seem to, but others don't. - With s_fmt, is sizeimage settable by the application in the same way as bytesperline? No, the driver will fill in this field, overwriting anything the application put there. bytesperline IS settable, but most drivers will ignore what userspace did and overwrite this as well. Normally the driver knows about HW requirements and will set sizeimage to something that will work (e.g. make sure it is a multiple of 16 lines). > yavta allows you to specify it on the command line, whilst v4l2-ctl doesn't. Some of the other parts of the Pi firmware have a requirement that the buffer is a multiple of 16 lines high, which can be matched by V4L2 if we can over-allocate the buffers by the app specifying sizeimage. But if I allow that, then I get a v4l2-compliance failure as the size doesn't get reset when switching from RGB3 to UYVY as it takes the request as a request to over-allocate. Apologies if I've messed up in sending these patches - so many ways to do something. It looks fine at a glance. I will probably review this on Friday or Monday. But I need some clarification of the difference between this and the staging driver first. Thanks! Hans
[RFC 2/2] [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface
Add driver for the Unicam camera receiver block on BCM283x processors. Signed-off-by: Dave Stevenson --- drivers/media/platform/Kconfig |1 + drivers/media/platform/Makefile |2 + drivers/media/platform/bcm2835/Kconfig | 14 + drivers/media/platform/bcm2835/Makefile |3 + drivers/media/platform/bcm2835/bcm2835-unicam.c | 2100 ++ drivers/media/platform/bcm2835/vc4-regs-unicam.h | 257 +++ 6 files changed, 2377 insertions(+) create mode 100644 drivers/media/platform/bcm2835/Kconfig create mode 100644 drivers/media/platform/bcm2835/Makefile create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 8da521a..aa9 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -135,6 +135,7 @@ source "drivers/media/platform/am437x/Kconfig" source "drivers/media/platform/xilinx/Kconfig" source "drivers/media/platform/rcar-vin/Kconfig" source "drivers/media/platform/atmel/Kconfig" +source "drivers/media/platform/bcm2835/Kconfig" config VIDEO_TI_CAL tristate "TI CAL (Camera Adaptation Layer) driver" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 6bbdf94..9c5e412 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -81,3 +81,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec/ obj-$(CONFIG_VIDEO_MEDIATEK_MDP) += mtk-mdp/ obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk-jpeg/ + +obj-y += bcm2835/ diff --git a/drivers/media/platform/bcm2835/Kconfig b/drivers/media/platform/bcm2835/Kconfig new file mode 100644 index 000..9f9be9e --- /dev/null +++ b/drivers/media/platform/bcm2835/Kconfig @@ -0,0 +1,14 @@ +# Broadcom VideoCore4 V4L2 camera support + +config VIDEO_BCM2835_UNICAM + tristate "Broadcom BCM2835 Unicam video capture driver" + depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + depends on ARCH_BCM2708 || ARCH_BCM2709 || ARCH_BCM2835 || COMPILE_TEST + select VIDEOBUF2_DMA_CONTIG + select V4L2_FWNODE + ---help--- + Say Y here to enable V4L2 subdevice for CSI2 receiver. + This is a V4L2 subdevice that interfaces directly to the VC4 peripheral. + + To compile this driver as a module, choose M here. The module + will be called bcm2835-unicam. diff --git a/drivers/media/platform/bcm2835/Makefile b/drivers/media/platform/bcm2835/Makefile new file mode 100644 index 000..a98aba0 --- /dev/null +++ b/drivers/media/platform/bcm2835/Makefile @@ -0,0 +1,3 @@ +# Makefile for BCM2835 Unicam driver + +obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c b/drivers/media/platform/bcm2835/bcm2835-unicam.c new file mode 100644 index 000..26039da --- /dev/null +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c @@ -0,0 +1,2100 @@ +/* + * BCM2835 Unicam capture Driver + * + * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd. + * + * Dave Stevenson + * + * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and + * TI CAL camera interface driver by Benoit Parrot. + * + * This program is free software; you may redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "vc4-regs-unicam.h" + +#define UNICAM_MODULE_NAME "unicam" +#define UNICAM_VERSION "0.1.0" + +static int debug; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, "Debug level 0-3"); + +#define unicam_dbg(level, dev, fmt, arg...)\ + v4l2_dbg(level, debug, &(dev)->v4l2_dev, fmt, ##arg) +#define unicam_info(dev, fmt, arg...) \ + v4l2_info(&(dev)->v4l2_dev, fmt, ##arg) +#define unicam_err(dev, fmt, arg...) \ + v4l2_err(&(dev)->v4l2_dev, fmt, ##arg) + +/* + * Stride is a 16 bit register. Max width is therefore determined by + * that divided by the number of bits per pixe
RE: [PATCH v3 1/3] videodev2.h, v4l2-ioctl: add IPU3 raw10 color format
> -Original Message- > From: Alan Cox [mailto:gno...@lxorguk.ukuu.org.uk] > Sent: Wednesday, June 14, 2017 6:49 AM > To: Zhi, Yong > Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian > Xu ; tf...@chromium.org; Mani, Rajmohan > ; Toivonen, Tuukka > ; Yang, Hyungwoo > ; Mohandass, Divagar > > Subject: Re: [PATCH v3 1/3] videodev2.h, v4l2-ioctl: add IPU3 raw10 color > format > > On Tue, 13 Jun 2017 15:17:14 -0500 > Yong Zhi wrote: > > > Add IPU3 specific formats: > > > > V4L2_PIX_FMT_IPU3_SBGGR10 > > V4L2_PIX_FMT_IPU3_SGBRG10 > > V4L2_PIX_FMT_IPU3_SGRBG10 > > V4L2_PIX_FMT_IPU3_SRGGB10 > > As I said before these are just more bitpacked bayer formats with no reason > to encode them as IPUv3 specific names. > > Alan Ack, will update for next version.
Re: [PATCH 2/8] dt: bindings: Add lens-focus binding for image sensors
On Wed, Jun 14, 2017 at 12:47:13PM +0300, Sakari Ailus wrote: > The lens-focus property contains a phandle to the lens voice coil driver > that is associated to the sensor; typically both are contained in the same > camera module. > > Signed-off-by: Sakari Ailus > Acked-by: Pavel Machek > Reviewed-by: Sebastian Reichel > --- > Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ > 1 file changed, 2 insertions(+) Acked-by: Rob Herring
Re: [PATCH 1/8] dt: bindings: Add a binding for flash devices associated to a sensor
On Wed, Jun 14, 2017 at 12:47:12PM +0300, Sakari Ailus wrote: > Camera flash drivers (and LEDs) are separate from the sensor devices in > DT. In order to make an association between the two, provide the > association information to the software. > > Signed-off-by: Sakari Ailus > --- > Documentation/devicetree/bindings/media/video-interfaces.txt | 8 > 1 file changed, 8 insertions(+) Acked-by: Rob Herring
[RFC 0/2] BCM283x Camera Receiver driver
Hi All. This is adding a V4L2 subdevice driver for the CSI2/CCP2 camera receiver peripheral on BCM283x, as used on Raspberry Pi. v4l2-compliance results depend on the sensor subdevice this is connected to. It passes the basic tests cleanly with TC358743, but objects with OV5647 fail: v4l2-test-controls.cpp(574): g_ext_ctrls does not support count == 0 Neither OV5647 nor Unicam support any controls. I must admit to not having got OV5647 to stream with the current driver register settings. It works with a set of register settings for VGA RAW10. I also have a couple of patches pending for OV5647, but would like to understand the issues better before sending them out. Two queries I do have in V4L2-land: - When s_dv_timings or s_std is called, is the format meant to be updated automatically? Even if we're already streaming? Some existing drivers seem to, but others don't. - With s_fmt, is sizeimage settable by the application in the same way as bytesperline? yavta allows you to specify it on the command line, whilst v4l2-ctl doesn't. Some of the other parts of the Pi firmware have a requirement that the buffer is a multiple of 16 lines high, which can be matched by V4L2 if we can over-allocate the buffers by the app specifying sizeimage. But if I allow that, then I get a v4l2-compliance failure as the size doesn't get reset when switching from RGB3 to UYVY as it takes the request as a request to over-allocate. Apologies if I've messed up in sending these patches - so many ways to do something. Thanks in advance. Dave Dave Stevenson (2): [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface .../devicetree/bindings/media/bcm2835-unicam.txt | 76 + drivers/media/platform/Kconfig |1 + drivers/media/platform/Makefile|2 + drivers/media/platform/bcm2835/Kconfig | 14 + drivers/media/platform/bcm2835/Makefile|3 + drivers/media/platform/bcm2835/bcm2835-unicam.c| 2100 drivers/media/platform/bcm2835/vc4-regs-unicam.h | 257 +++ 7 files changed, 2453 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt create mode 100644 drivers/media/platform/bcm2835/Kconfig create mode 100644 drivers/media/platform/bcm2835/Makefile create mode 100644 drivers/media/platform/bcm2835/bcm2835-unicam.c create mode 100644 drivers/media/platform/bcm2835/vc4-regs-unicam.h -- 2.7.4
[RFC 1/2] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
Document the DT bindings for the CSI2/CCP2 receiver peripheral (known as Unicam) on BCM283x SoCs. Signed-off-by: Dave Stevenson --- .../devicetree/bindings/media/bcm2835-unicam.txt | 76 ++ 1 file changed, 76 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt new file mode 100644 index 000..cc5a451 --- /dev/null +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt @@ -0,0 +1,76 @@ +Broadcom BCM283x Camera Interface (Unicam) +-- + +The Unicam block on BCM283x SoCs is the receiver for either +CSI-2 or CCP2 data from image sensors or similar devices. + +Required properties: +=== +- compatible : must be "brcm,bcm2835-unicam". +- reg : physical base address and length of the register sets for the + device. +- interrupts : should contain the IRQ line for this Unicam instance. +- clocks : list of clock specifiers, corresponding to entries in + clock-names property. +- clock-names : must contain an "lp_clock" entry, matching entries + in the clocks property. + +Optional properties +=== +- max-data-lanes: the hardware can support varying numbers of clock lanes. + This value is the maximum number supported by this instance. + Known values of 2 or 4. Default is 2. + + +Unicam supports a single port node. It should contain one 'port' child node +with child 'endpoint' node. Please refer to the bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: + csi1: csi@7e801000 { + compatible = "brcm,bcm2835-unicam"; + reg = <0x7e801000 0x800>, + <0x7e802004 0x4>; + interrupts = <2 7>; + clocks = <&clocks BCM2835_CLOCK_CAM1>; + clock-names = "lp_clock"; + + port { + #address-cells = <1>; + #size-cells = <0>; + + endpoint { + remote-endpoint = <&tc358743_0>; + + }; + }; + }; + + i2c0: i2c@7e205000 { + + tc358743: tc358743@0f { + compatible = "toshiba,tc358743"; + reg = <0x0f>; + status = "okay"; + + clocks = <&tc358743_clk>; + clock-names = "refclk"; + + tc358743_clk: bridge-clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <2700>; + }; + + port { + tc358743_0: endpoint { + remote-endpoint = <&csi1>; + clock-lanes = <0>; + data-lanes = <1 2 3 4>; + clock-noncontinuous; + link-frequencies = + /bits/ 64 <29700>; + }; + }; + }; + }; -- 2.7.4
Re: [PATCH v3 1/3] videodev2.h, v4l2-ioctl: add IPU3 raw10 color format
On Tue, 13 Jun 2017 15:17:14 -0500 Yong Zhi wrote: > Add IPU3 specific formats: > > V4L2_PIX_FMT_IPU3_SBGGR10 > V4L2_PIX_FMT_IPU3_SGBRG10 > V4L2_PIX_FMT_IPU3_SGRBG10 > V4L2_PIX_FMT_IPU3_SRGGB10 As I said before these are just more bitpacked bayer formats with no reason to encode them as IPUv3 specific names. Alan
[PATCH] i2c: fix platform_no_drv_owner.cocci warnings
drivers/media/i2c/ov5670.c:2577:3-8: No need to set .owner here. The core will do it. Remove .owner field if calls are used which set it automatically Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci CC: Chiranjeevi Rapolu Signed-off-by: Fengguang Wu --- ov5670.c |1 - 1 file changed, 1 deletion(-) --- a/drivers/media/i2c/ov5670.c +++ b/drivers/media/i2c/ov5670.c @@ -2574,7 +2574,6 @@ MODULE_DEVICE_TABLE(acpi, ov5670_acpi_id static struct i2c_driver ov5670_i2c_driver = { .driver = { .name = "ov5670", - .owner = THIS_MODULE, .pm = &ov5670_pm_ops, .acpi_match_table = ACPI_PTR(ov5670_acpi_ids), },
[PATCH] i2c: fix semicolon.cocci warnings
drivers/media/i2c/ov5670.c:2001:2-3: Unneeded semicolon drivers/media/i2c/ov5670.c:2033:2-3: Unneeded semicolon Remove unneeded semicolon. Generated by: scripts/coccinelle/misc/semicolon.cocci CC: Chiranjeevi Rapolu Signed-off-by: Fengguang Wu --- ov5670.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/drivers/media/i2c/ov5670.c +++ b/drivers/media/i2c/ov5670.c @@ -1998,7 +1998,7 @@ static int ov5670_set_ctrl(struct v4l2_c __v4l2_ctrl_modify_range(ov5670->exposure, ov5670->exposure->minimum, max, ov5670->exposure->step, max); break; - }; + } /* V4L2 controls values will be applied only when power is already up */ if (pm_runtime_get_if_in_use(&client->dev) <= 0) @@ -2030,7 +2030,7 @@ static int ov5670_set_ctrl(struct v4l2_c dev_info(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n", __func__, ctrl->id, ctrl->val); break; - }; + } pm_runtime_put(&client->dev);
Re: [PATCH v3 1/1] i2c: Add Omnivision OV5670 5M sensor support
Hi Chiranjeevi, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.12-rc5 next-20170614] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/chiranjeevi-rapolu-intel-com/i2c-Add-Omnivision-OV5670-5M-sensor-support/20170614-195050 base: git://linuxtv.org/media_tree.git master coccinelle warnings: (new ones prefixed by >>) >> drivers/media/i2c/ov5670.c:2577:3-8: No need to set .owner here. The core >> will do it. -- >> drivers/media/i2c/ov5670.c:2001:2-3: Unneeded semicolon drivers/media/i2c/ov5670.c:2033:2-3: Unneeded semicolon Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH] dvb-usb-af9035: load HID table
- Original Message - > Hello > > Jaroslav Škarvada kirjoitti 2017-06-09 20:46: > > Automatically load sniffed HID table from Windows driver if > > USB_VID_ITETECH:USB_PID_ITETECH_IT9135_9006 device is present (e.g. > > Evolveo > > Mars) or if module parameter force_hid_tab_load is set. > > There is few issues I don't like this approach. Mostly that module > parameter to select HID table. There is existing solution to select > remote controller, it is ir-keytable and it should be used rather than > defining device specific module parameter. > > If you look that HID table you could see there is 4 bytes NEC code and 3 > bytes HID code. Remote controller seems to have 34 keys. Remote > controller address bytes are 0x02bd, grepping existing remote controller > keytables it could be Total Media In Hand remote controller > (rc-total-media-in-hand.c). If not, then defining new keytable is > needed. > > I did some research about issue and found 2 better solutions. > 1) Configure HID table dynamically. Remote controller keytable has some > needed information, but those KEY_* events needed to be translated to > HID codes somehow. > 2) Kill HID and then use CMD_IR_GET to get remote controller scancodes > by polling. > > Solution 1 sounds most correct. No need to poll and decode by sw as hw > does all the job. But it is most hardest to implement, I am not aware if > anyone has done it yet. > > regards > Antti > > > -- > http://palosaari.fi/ > Hi, thanks for info. General approach is usually better than device specific hacks, but it looks like longer run. Unfortunately I returned the device to the original owner, so I will be probably unable to help with it more. But the problem needs to be addressed, the state when the remote isn't working isn't good thanks & regards Jaroslav
Re: [PATCH v10 00/18] Qualcomm video decoder/encoder driver
Hi Mauro, One note, I have sent pull request for the firmware but forgot to update the firmware path in the driver for the new location. It is not a big deal, but I have to send one more patch which changes the firmware path. About COMPILE_TEST, the patch for qcom_scm driver probably will be taken in 4.13 merge window through arm-soc. So I'm wondering could you postpone 18/18 until this patch is merged to avoid build breakage. regards, Stan On 06/12/2017 07:27 PM, Stanimir Varbanov wrote: > Hello, > > The changes since patchset v9 are the following: > * patches from 1/18 to 9/18 are the same. > * patches from 10/18 to 16/18 are fixes for warns/errors found by >Mauro when building with its gcc7. > * patch 17/18 adding support for minimum buffers for capture >get control. This fixes an issue with gstreamer and it will >be good to have it in the inital version of the venus driver. > * patch 18/18 enable COMPILE_TEST Kconfig option for the driver, >and this patch depends on the other one for qcom_scm driver. >The submited patch for qcom_scm driver can be found at [1]. > > Mauro, I failed to build gcc7 on my own machine and fallback to > a pre-built version of the gcc-7 for may Ubuntu distro. The version > which I tried was: gcc version 7.1.0 (Ubuntu 7.1.0-5ubuntu2~16.04). > Unfortunately I cannot reproduce the warns/errors (except two > warnings) from your compiler (even that the version looks > the same 7.1.0). So I fixed the warns/errors as per your response > to v9, and hope that the errors will disappear. > > [1] https://patchwork.kernel.org/patch/9775803/ > > Stanimir Varbanov (18): > media: v4l2-mem2mem: extend m2m APIs for more accurate buffer > management > doc: DT: venus: binding document for Qualcomm video driver > MAINTAINERS: Add Qualcomm Venus video accelerator driver > media: venus: adding core part and helper functions > media: venus: vdec: add video decoder files > media: venus: venc: add video encoder files > media: venus: hfi: add Host Firmware Interface (HFI) > media: venus: hfi: add Venus HFI files > media: venus: enable building of Venus video driver > media: venus: hfi: fix mutex unlock > media: venus: hfi_cmds: fix variable dereferenced before check > media: venus: helpers: fix variable dereferenced before check > media: venus: hfi_venus: fix variable dereferenced before check > media: venus: hfi_msgs: fix set but not used variables > media: venus: vdec: fix compile error in vdec_close > media: venus: venc: fix compile error in venc_close > media: venus: vdec: add support for min buffers for capture > media: venus: enable building with COMPILE_TEST > > .../devicetree/bindings/media/qcom,venus.txt | 107 ++ > MAINTAINERS|8 + > drivers/media/platform/Kconfig | 13 + > drivers/media/platform/Makefile|2 + > drivers/media/platform/qcom/venus/Makefile | 11 + > drivers/media/platform/qcom/venus/core.c | 388 + > drivers/media/platform/qcom/venus/core.h | 323 > drivers/media/platform/qcom/venus/firmware.c | 109 ++ > drivers/media/platform/qcom/venus/firmware.h | 22 + > drivers/media/platform/qcom/venus/helpers.c| 725 + > drivers/media/platform/qcom/venus/helpers.h| 45 + > drivers/media/platform/qcom/venus/hfi.c| 522 +++ > drivers/media/platform/qcom/venus/hfi.h| 175 +++ > drivers/media/platform/qcom/venus/hfi_cmds.c | 1259 > drivers/media/platform/qcom/venus/hfi_cmds.h | 304 > drivers/media/platform/qcom/venus/hfi_helper.h | 1050 + > drivers/media/platform/qcom/venus/hfi_msgs.c | 1052 + > drivers/media/platform/qcom/venus/hfi_msgs.h | 283 > drivers/media/platform/qcom/venus/hfi_venus.c | 1572 > > drivers/media/platform/qcom/venus/hfi_venus.h | 23 + > drivers/media/platform/qcom/venus/hfi_venus_io.h | 113 ++ > drivers/media/platform/qcom/venus/vdec.c | 1162 +++ > drivers/media/platform/qcom/venus/vdec.h | 23 + > drivers/media/platform/qcom/venus/vdec_ctrls.c | 158 ++ > drivers/media/platform/qcom/venus/venc.c | 1283 > drivers/media/platform/qcom/venus/venc.h | 23 + > drivers/media/platform/qcom/venus/venc_ctrls.c | 270 > drivers/media/v4l2-core/v4l2-mem2mem.c | 37 + > include/media/v4l2-mem2mem.h | 92 ++ > 29 files changed, 11154 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt > create mode 100644 drivers/media/platform/qcom/venus/Makefile > create mode 100644 drivers/media/platform/qcom/venus/core.c > create mode 100644 drivers/media/platform/qcom/venus/core.h > create mode 100644 drivers/media/platform/qc
Re: [PATCH 1/9] [media] s5p-jpeg: Reset the Codec before doing a soft reset
Hi, W dniu 13.06.2017 o 20:46, Jacek Anaszewski pisze: Hi Thierry, On 06/07/2017 02:34 PM, Thierry Escande wrote: Hi Jacek, On 02/06/2017 21:50, Jacek Anaszewski wrote: Hi Thierry, On 06/02/2017 06:02 PM, Thierry Escande wrote: From: Abhilash Kesavan This patch resets the encoding and decoding register bits before doing a soft reset. Signed-off-by: Tony K Nadackal Signed-off-by: Thierry Escande --- drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c index a1d823a..9ad8f6d 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-hw-exynos4.c @@ -21,6 +21,10 @@ void exynos4_jpeg_sw_reset(void __iomem *base) unsigned int reg; reg = readl(base + EXYNOS4_JPEG_CNTL_REG); +writel(reg & ~(EXYNOS4_DEC_MODE | EXYNOS4_ENC_MODE), + base + EXYNOS4_JPEG_CNTL_REG); Why is it required? It would be nice if commit message explained that. Unfortunately the bug entry in the ChromeOS issue tracker does not mention more information about that and the patch author is no more reachable on that email address. So unless someone else knows the answer I won't be able to give more explanation in the commit message... Unfortunately I don't have longer access to the hardware and can't test these changes. Have you tested them, or just cherry-picked from the bug tracker? I do have access to the hardware and will look into the series, however results can be expected next week. Andrzej
Re: [PATCH v4 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
Hi Kieran, On Wed, Jun 14, 2017 at 11:43 AM, Kieran Bingham wrote: > On 14/06/17 10:39, Geert Uytterhoeven wrote: >> On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham wrote: >>> From: Kieran Bingham >>> >>> Provide ADV7482, and the needed connectors >>> >>> Signed-off-by: Kieran Bingham >> >> Thanks for your patch! >> >>> v4: >>> - dt: Rebase to dts/renesas/salvator-x.dtsi >>> - dt: Use AIN0-7 rather than AIN1-8 >>> >>> arch/arm64/boot/dts/renesas/salvator-x.dtsi | 123 +- >> >> I believe all of this applies to both Salvator-X and Salvator-XS? >> >> Hence it should be applied to salvator-common.dtsi instead of >> salvator-x.dtsi. > > Hrm ... I don't have a salator-common.dtsi ... I'll need a new rebase. It's always a good idea to base your Renesas DT patches on Simon's latest devel branch. > But it sounds logical :) Good :-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [patch, libv4l]: add sdlcam example for testing digital still camera functionality
Hi Hans, Pavel, Mauro, others, On Mon, May 29, 2017 at 10:02:26AM +0200, Hans Verkuil wrote: > On 05/29/2017 09:32 AM, Pavel Machek wrote: > >On Mon 2017-05-29 08:13:22, Hans Verkuil wrote: > >>Hi Pavel, > >> > >>On 05/21/2017 12:33 PM, Pavel Machek wrote: > >>>Add simple SDL-based application for capturing photos. Manual > >>>focus/gain/exposure can be set, flash can be controlled and > >>>autofocus/autogain can be selected if camera supports that. > >>> > >>>It is already useful for testing autofocus/autogain improvements to > >>>the libraries on Nokia N900. > >>> > >>>Signed-off-by: Pavel Machek > >> > >>I think this is more suitable as a github project. To be honest, I feel that > >>v4l-utils already contains too many random utilities, so I prefer not to add > >>to that. > > > >>On the other hand, there is nothing against sharing this as on github as it > >>certainly can be useful. > > > >Can I get you to reconsider that? > > > >Originally, I planed to keep the utility separate, but then I got > >comments from Mauro ( https://lkml.org/lkml/2017/4/24/457 ) explaining > >that hard sdl dependency is not acceptable etc, and how I should do > >automake. > > > >So I had a lot of fun with automake integration, and generally doing > >things right. > > > >So getting "we all ready have too many utilities" _now_ is quite an > >unwelcome surprise. > > Too many *random* utilities. > > Utilities like v4l2-ctl are tied closely to the kernel and are updated > whenever > new APIs appear. But yet another viewer? > > Mauro, I find that v4l-utils is a bit polluted with non-core utilities. > IMHO it should only contain the core libv4l2, core utilities and > driver-specific > utilities. I wonder if we should make a media-utils-contrib for all the > non-core > stuff. > > What is your opinion? One of the purposes the v4l-utils repository has is that the distributions get these programs included to their v4l-utils package as it's typically called. It's debatable whether or how much it should contain device specific or otherwise random projects, but having a common location for such programs has clear benefits, too. Based on how this one looks it is definitely not an end user application (I hope I'm not miscategorising it) and as Pavel mentioned, it has been useful in testing automatic focus / gain control on N900. Just my 5 euro cents... -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: v4l2-fwnode: status, plans for merge, any branch to merge against?
Hi, Pavel! On Tue, Jun 13, 2017 at 11:09:00PM +0200, Pavel Machek wrote: > Hi! > > > > Are there any news about the fwnode branch? > > > > > > I have quite usable camera, but it is still based on > > > 982e8e40390d26430ef106fede41594139a4111c (that's v4.10). It would be > > > good to see fwnode stuff upstream... are there any plans for that? > > > > > > Is there stable branch to which I could move the stuff? > > > > What's relevant for most V4L2 drivers is in linux-media right now. > > > > There are new features that will take some time to get in. The trouble has > > been, and continue to be, that the patches need to go through various trees > > so it'll take some time for them to be merged. > > > > I expect to have most of them in during the next merge window. > > So git://linuxtv.org/media_tree.git branch master is the right one to > work one? I also pushed the rebased ccp2 branch there: https://git.linuxtv.org/sailus/media_tree.git/log/?h=ccp2> It's now right on the top of media-tree master. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v1] [media] as3645a: Join string literals back
On Sun, Jun 04, 2017 at 09:29:18PM +0300, Andy Shevchenko wrote: > There is no need to split long string literals. > Join them back. > > No functional change intended. > > Signed-off-by: Andy Shevchenko Thanks, applied! -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH RESEND v11 1/1] [media] i2c: add support for OV13858 sensor
On Wed, Jun 14, 2017 at 01:12:49PM +0300, Sakari Ailus wrote: > Hi Hyungwoo, > > On Tue, Jun 13, 2017 at 03:06:16PM -0700, Hyungwoo Yang wrote: > > This patch adds driver for Omnivision's ov13858 > > sensor, the driver supports following features: > > > > - manual exposure/gain(analog and digital) control support > > - two link frequencies > > - VBLANK/HBLANK support > > - test pattern support > > - media controller support > > - runtime pm support > > - supported resolutions > > + 4224x3136 at 30FPS > > + 2112x1568 at 30FPS(default) and 60FPS > > + 2112x1188 at 30FPS(default) and 60FPS > > + 1056x784 at 30FPS(default) and 60FPS > > > > Signed-off-by: Hyungwoo Yang > > --- > > If you make changes, please detail them in this part of a single patch. In > this case, this would have included mentioning that the expected > clock-frequency property value has changed. And the subject should have > indicated v12. "RESEND" is only used if there are no changes to the patches > as such. > > I've applied this to my tree. And switched to V4L2_CID_DIGITAL_GAIN, too. -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v8] dw9714: Initial driver for dw9714 VCM
On Sat, Jun 03, 2017 at 11:40:32AM +0300, Sakari Ailus wrote: > Hi Rajmohan, > > On Sat, Jun 03, 2017 at 01:11:40AM -0700, Rajmohan Mani wrote: > > DW9714 is a 10 bit DAC, designed for linear > > control of voice coil motor. > > > > This driver creates a V4L2 subdevice and > > provides control to set the desired focus. > > > > Signed-off-by: Rajmohan Mani > > Acked-by: Sakari Ailus Applied to my tree. -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v2 2/2] v4l: controls: Improve documentation for V4L2_CID_GAIN
On 06/09/17 15:21, Sakari Ailus wrote: > Elaborate the differences between V4L2_CID_GAIN and gain-type specific > V4L2_CID_DIGITAL_GAIN and V4L2_CID_ANALOGUE_GAIN. > > Signed-off-by: Sakari Ailus Acked-by: Hans Verkuil Regards, Hans > --- > Documentation/media/uapi/v4l/control.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/control.rst > b/Documentation/media/uapi/v4l/control.rst > index 51112ba..c1e6adb 100644 > --- a/Documentation/media/uapi/v4l/control.rst > +++ b/Documentation/media/uapi/v4l/control.rst > @@ -137,6 +137,12 @@ Control IDs > ``V4L2_CID_GAIN`` ``(integer)`` > Gain control. > > +Primarily used to control gain on e.g. TV tuners but also on > +webcams. Most devices control only digital gain with this control > +but on some this could include analogue gain as well. Devices that > +recognise the difference between digital and analogue gain use > +controls ``V4L2_CID_DIGITAL_GAIN`` and ``V4L2_CID_ANALOGUE_GAIN``. > + > ``V4L2_CID_HFLIP`` ``(boolean)`` > Mirror the picture horizontally. > >
Re: [PATCH v2 1/2] v4l: ctrls: Add a control for digital gain
On 06/09/17 15:21, Sakari Ailus wrote: > Add V4L2_CID_DIGITAL_GAIN to control explicitly digital gain. > > We already have analogue gain control which the digital gain control > complements. Typically higher quality images are obtained using analogue > gain only as the digital gain does not add information to the image > (rather it may remove it). > > Signed-off-by: Sakari Ailus Acked-by: Hans Verkuil Regards, Hans > --- > Documentation/media/uapi/v4l/extended-controls.rst | 7 +++ > drivers/media/v4l2-core/v4l2-ctrls.c | 1 + > include/uapi/linux/v4l2-controls.h | 2 +- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst > b/Documentation/media/uapi/v4l/extended-controls.rst > index 76c5b1a..9acc9cad 100644 > --- a/Documentation/media/uapi/v4l/extended-controls.rst > +++ b/Documentation/media/uapi/v4l/extended-controls.rst > @@ -3021,6 +3021,13 @@ Image Process Control IDs > The video deinterlacing mode (such as Bob, Weave, ...). The menu items > are > driver specific and are documented in :ref:`v4l-drivers`. > > +``V4L2_CID_DIGITAL_GAIN (integer)`` > +Digital gain is the value by which all colour components > +are multiplied by. Typically the digital gain applied is the > +control value divided by e.g. 0x100, meaning that to get no > +digital gain the control value needs to be 0x100. The no-gain > +configuration is also typically the default. > + > > .. _dv-controls: > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c > index 5aed7bd..36eede3 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -886,6 +886,7 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_PIXEL_RATE: return "Pixel Rate"; > case V4L2_CID_TEST_PATTERN: return "Test Pattern"; > case V4L2_CID_DEINTERLACING_MODE: return "Deinterlacing Mode"; > + case V4L2_CID_DIGITAL_GAIN: return "Digital Gain"; > > /* DV controls */ > /* Keep the order of the 'case's the same as in v4l2-controls.h! */ > diff --git a/include/uapi/linux/v4l2-controls.h > b/include/uapi/linux/v4l2-controls.h > index 0d2e1e0..0cdb8eb 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -893,7 +893,7 @@ enum v4l2_jpeg_chroma_subsampling { > #define V4L2_CID_PIXEL_RATE (V4L2_CID_IMAGE_PROC_CLASS_BASE > + 2) > #define V4L2_CID_TEST_PATTERN > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3) > #define V4L2_CID_DEINTERLACING_MODE (V4L2_CID_IMAGE_PROC_CLASS_BASE > + 4) > - > +#define V4L2_CID_DIGITAL_GAIN > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 5) > > /* DV-class control IDs defined by V4L2 */ > #define V4L2_CID_DV_CLASS_BASE (V4L2_CTRL_CLASS_DV | > 0x900) >
Re: [PATCH v7 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
Hi Niklas, On Tue, Jun 13, 2017 at 06:50:14PM +0200, Niklas Söderlund wrote: > Hi Sakari, > > Thanks for your feedback. > > On 2017-05-29 14:16:25 +0300, Sakari Ailus wrote: > > Hi Niklas, > > > > On Wed, May 24, 2017 at 02:13:52AM +0200, Niklas Söderlund wrote: > > > From: Niklas Söderlund > > > > > > Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers > > > are located between the video sources (CSI-2 transmitters) and the video > > > grabbers (VIN) on Gen3 of Renesas R-Car SoC. > > > > > > Each CSI-2 device is connected to more then one VIN device which > > > simultaneously can receive video from the same CSI-2 device. Each VIN > > > device can also be connected to more then one CSI-2 device. The routing > > > of which link are used are controlled by the VIN devices. There are only > > > a few possible routes which are set by hardware limitations, which are > > > different for each SoC in the Gen3 family. > > > > > > To work with the limitations of routing possibilities it is necessary > > > for the DT bindings to describe which VIN device is connected to which > > > CSI-2 device. This is why port 1 needs to to assign reg numbers for each > > > VIN device that be connected to it. To setup and to know which links are > > > valid for each SoC is the responsibility of the VIN driver since the > > > register to configure it belongs to the VIN hardware. > > > > > > Signed-off-by: Niklas Söderlund > > > --- > > > .../devicetree/bindings/media/rcar-csi2.txt| 116 > > > + > > > 1 file changed, 116 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/rcar-csi2.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/rcar-csi2.txt > > > b/Documentation/devicetree/bindings/media/rcar-csi2.txt > > > new file mode 100644 > > > index ..f6e2027ee92b171a > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/rcar-csi2.txt > > > @@ -0,0 +1,116 @@ > > > +Renesas R-Car MIPI CSI-2 > > > + > > > + > > > +The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas > > > R-Car > > > +family of devices. It is to be used in conjunction with the R-Car VIN > > > module, > > > +which provides the video capture capabilities. > > > + > > > + - compatible: Must be one or more of the following > > > + - "renesas,r8a7795-csi2" for the R8A7795 device. > > > + - "renesas,r8a7796-csi2" for the R8A7796 device. > > > + - "renesas,rcar-gen3-csi2" for a generic R-Car Gen3 compatible device. > > > + > > > + When compatible with a generic version nodes must list the > > > + SoC-specific version corresponding to the platform first > > > + followed by the generic version. > > > + > > > + - reg: the register base and size for the device registers > > > + - interrupts: the interrupt for the device > > > + - clocks: Reference to the parent clock > > > + > > > +The device node should contain two 'port' child nodes according to the > > > +bindings defined in Documentation/devicetree/bindings/media/ > > > +video-interfaces.txt. Port 0 should connect the node that is the video > > > +source for to the CSI-2. Port 1 should connect all the R-Car VIN > > > +modules, which can make use of the CSI-2 module. > > > > Should or shall? > > > > I guess you could add that it is possible to leave them unconnected, too. > > Which ports/endpoints are you talking about? In my mind it's not allowed > to leave them unconnected. > > If there ever is a system with only 4 VIN instances (I'm not aware of > any such system) then yes the endpoints for those VIN not present in the > system in port 1 should be left unconnected but other then that they > should all be mandatory right? Or am I missing something? I think so, yes. Then "shall" is right, isn't it? > > > > > > + > > > +- Port 0 - Video source > > > + - Reg 0 - sub-node describing the endpoint that is the video source > > > > Which endpoint properties are mandatory for the receiver? Which ones are > > optional? (I.e. it shouldn't be necessary to read driver code to write board > > description.) > > I will add a note that all endpoints in port 0 are mandatory and that > all endpoints that represents a connection to a VIN instance in the > system is mandatory for next version. Thanks I did not think about this > possibility. Please list the mandatory and optional properties, too. Not just the endpoints. -- Hälsningar, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH RESEND v11 1/1] [media] i2c: add support for OV13858 sensor
Hi Hyungwoo, On Tue, Jun 13, 2017 at 03:06:16PM -0700, Hyungwoo Yang wrote: > This patch adds driver for Omnivision's ov13858 > sensor, the driver supports following features: > > - manual exposure/gain(analog and digital) control support > - two link frequencies > - VBLANK/HBLANK support > - test pattern support > - media controller support > - runtime pm support > - supported resolutions > + 4224x3136 at 30FPS > + 2112x1568 at 30FPS(default) and 60FPS > + 2112x1188 at 30FPS(default) and 60FPS > + 1056x784 at 30FPS(default) and 60FPS > > Signed-off-by: Hyungwoo Yang > --- If you make changes, please detail them in this part of a single patch. In this case, this would have included mentioning that the expected clock-frequency property value has changed. And the subject should have indicated v12. "RESEND" is only used if there are no changes to the patches as such. I've applied this to my tree. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v3 2/2] media: entity: Add media_entity_get_fwnode_pad() function
Hi Niklas, Thanks for the update! On Tue, Jun 13, 2017 at 04:31:26PM +0200, Niklas Söderlund wrote: > This is a wrapper around the media entity get_fwnode_pad operation. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/media-entity.c | 35 +++ > include/media/media-entity.h | 23 +++ > 2 files changed, 58 insertions(+) > > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c > index bc44193efa4798b4..35a15263793f71e1 100644 > --- a/drivers/media/media-entity.c > +++ b/drivers/media/media-entity.c > @@ -18,6 +18,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -386,6 +387,40 @@ struct media_entity *media_graph_walk_next(struct > media_graph *graph) > } > EXPORT_SYMBOL_GPL(media_graph_walk_next); > > +int media_entity_get_fwnode_pad(struct media_entity *entity, > + struct fwnode_handle *fwnode, > + unsigned int direction) media entity pad flags has unsigned long type. I think you should call this e.g. direction_flags or pad_direction_flags. It'd be clear that it's about pad flags and only the direction matters. > +{ > + struct fwnode_endpoint endpoint; > + int i, ret; media entity num_pads field is u16. I guess unsigned int is fine, but it should be unsigned at least. With these, Acked-by: Sakari Ailus > + > + if (!entity->ops || !entity->ops->get_fwnode_pad) { > + for (i = 0; i < entity->num_pads; i++) { > + if (entity->pads[i].flags & direction) > + return i; > + } > + > + return -ENXIO; > + } > + > + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); > + if (ret) > + return ret; > + > + ret = entity->ops->get_fwnode_pad(&endpoint); > + if (ret < 0) > + return ret; > + > + if (ret >= entity->num_pads) > + return -ENXIO; > + > + if (!(entity->pads[ret].flags & direction)) > + return -ENXIO; > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(media_entity_get_fwnode_pad); > + > /* > - > * Pipeline management > */ > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index 46eeb036aa330534..4114e06964824ec9 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -821,6 +821,29 @@ struct media_pad *media_entity_remote_pad(struct > media_pad *pad); > struct media_entity *media_entity_get(struct media_entity *entity); > > /** > + * media_entity_get_fwnode_pad - Get pad number from fwnode > + * > + * @entity: The entity > + * @fwnode: Pointer to the fwnode_handle which should be used to find the pad > + * @direction: Expected direction of the pad, as defined in > + * :ref:`include/uapi/linux/media.h ` > + * (seek for ``MEDIA_PAD_FL_*``) > + * > + * This function can be used to resolve the media pad number from > + * a fwnode. This is useful for devices which use more complex > + * mappings of media pads. > + * > + * If the entity dose not implement the get_fwnode_pad() operation > + * then this function searches the entity for the first pad that > + * matches the @direction. > + * > + * Return: returns the pad number on success or a negative error code. > + */ > +int media_entity_get_fwnode_pad(struct media_entity *entity, > + struct fwnode_handle *fwnode, > + unsigned int direction); > + > +/** > * media_graph_walk_init - Allocate resources used by graph walk. > * > * @graph: Media graph structure that will be used to walk the graph -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v3 1/2] media: entity: Add get_fwnode_pad entity operation
On Tue, Jun 13, 2017 at 04:31:25PM +0200, Niklas Söderlund wrote: > The optional operation can be used by entities to report how it maps its > fwnode endpoints to media pad numbers. This is useful for devices which > require advanced mappings of pads. > > Signed-off-by: Niklas Söderlund Thanks! Acked-by: Sakari Ailus -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH 0/8] Support registering lens, flash and EEPROM devices
On Wed, Jun 14, 2017 at 12:47:11PM +0300, Sakari Ailus wrote: > Hi folks, > > This set adds support for async registering of lens, flash and EEPROM > devices, as well as support for this in the smiapp driver and a LED driver > for the as3645a. > > The lens and flash devices are entities in the media graph whereas the > EEPROM is at least currently not. By providing the association information > it is possible to add the flash device to the media graph. I forgot to add that this set depends on another set Niklas recently posted: http://www.spinics.net/lists/linux-media/msg116906.html> -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
[PATCH 8/8] arm: dts: omap3: N9/N950: Add AS3645A camera flash
From: Sakari Ailus Add the as3645a flash controller to the DT source as well as the flash property with the as3645a device phandle to the sensor DT node. Signed-off-by: Sakari Ailus --- arch/arm/boot/dts/omap3-n9.dts | 1 + arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 ++ arch/arm/boot/dts/omap3-n950.dts | 1 + 3 files changed, 16 insertions(+) diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts index b9e58c5..f95e7b1 100644 --- a/arch/arm/boot/dts/omap3-n9.dts +++ b/arch/arm/boot/dts/omap3-n9.dts @@ -26,6 +26,7 @@ clocks = <&isp 0>; clock-frequency = <960>; nokia,nvm-size = <(16 * 64)>; + flash = <&as3645a>; port { smia_1_1: endpoint { link-frequencies = /bits/ 64 <19920 21000 49920>; diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi index df3366f..8bd6673 100644 --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi @@ -265,6 +265,20 @@ &i2c2 { clock-frequency = <40>; + + as3645a: flash@30 { + reg = <0x30>; + compatible = "ams,as3645a"; + flash { + flash-timeout-us = <15>; + flash-max-microamp = <32>; + led-max-microamp = <6>; + peak-current-limit = <175>; + }; + indicator { + led-max-microamp = <1>; + }; + }; }; &i2c3 { diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts index 646601a..8fca038 100644 --- a/arch/arm/boot/dts/omap3-n950.dts +++ b/arch/arm/boot/dts/omap3-n950.dts @@ -60,6 +60,7 @@ clocks = <&isp 0>; clock-frequency = <960>; nokia,nvm-size = <(16 * 64)>; + flash = <&as3645a>; port { smia_1_1: endpoint { link-frequencies = /bits/ 64 <21000 33360 39840>; -- 2.1.4
[PATCH 1/8] dt: bindings: Add a binding for flash devices associated to a sensor
Camera flash drivers (and LEDs) are separate from the sensor devices in DT. In order to make an association between the two, provide the association information to the software. Signed-off-by: Sakari Ailus --- Documentation/devicetree/bindings/media/video-interfaces.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index 9cd2a36..9723f7e 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -67,6 +67,14 @@ are required in a relevant parent node: identifier, should be 1. - #size-cells: should be zero. + +Optional properties +--- + +- flash: phandle referring to the flash driver chip. A flash driver may + have multiple flashes connected to it. + + Optional endpoint properties -- 2.1.4
[PATCH 5/8] v4l2-flash: Flash ops aren't mandatory
None of the flash operations are not mandatory and therefore there should be no need for the flash ops structure either. Accept NULL. Signed-off-by: Sakari Ailus --- drivers/media/v4l2-core/v4l2-flash-led-class.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c index 6d69119..fdb79da 100644 --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c @@ -18,7 +18,7 @@ #include #define has_flash_op(v4l2_flash, op) \ - (v4l2_flash && v4l2_flash->ops->op) + (v4l2_flash && v4l2_flash->ops && v4l2_flash->ops->op) #define call_flash_op(v4l2_flash, op, arg) \ (has_flash_op(v4l2_flash, op) ? \ @@ -618,7 +618,7 @@ struct v4l2_flash *v4l2_flash_init( struct v4l2_subdev *sd; int ret; - if (!fled_cdev || !ops || !config) + if (!fled_cdev || !config) return ERR_PTR(-EINVAL); led_cdev = &fled_cdev->led_cdev; -- 2.1.4
[PATCH 3/8] dt: bindings: Add a binding for referencing EEPROM from camera sensors
Many camera sensor devices contain EEPROM chips that describe the properties of a given unit --- the data is specific to a given unit can thus is not stored e.g. in user space or the driver. Some sensors embed the EEPROM chip and it can be accessed through the sensor's I2C interface. This property is to be used for devices where the EEPROM chip is accessed through a different I2C address than the sensor. The intent is to later provide this information to the user space. Signed-off-by: Sakari Ailus Acked-by: Pavel Machek Reviewed-by: Sebastian Reichel --- Documentation/devicetree/bindings/media/video-interfaces.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index a18d9b2..ae259924 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -76,6 +76,9 @@ Optional properties - lens-focus: A phandle to the node of the focus lens controller. +- eeprom: A phandle to the node of the EEPROM describing the camera sensor + (i.e. device specific calibration data), in case it differs from the + sensor node. Optional endpoint properties -- 2.1.4
[PATCH 4/8] v4l2-flash: Use led_classdev instead of led_classdev_flash for indicator
The V4L2 flash class initialisation expects struct led_classdev_flash that describes an indicator but only uses struct led_classdev which is a field iled_cdev in the struct. Use struct iled_cdev only. Signed-off-by: Sakari Ailus --- drivers/media/v4l2-core/v4l2-flash-led-class.c | 19 +++ include/media/v4l2-flash-led-class.h | 6 +++--- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c index 7b82881..6d69119 100644 --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c @@ -110,7 +110,7 @@ static void v4l2_flash_set_led_brightness(struct v4l2_flash *v4l2_flash, led_set_brightness_sync(&v4l2_flash->fled_cdev->led_cdev, brightness); } else { - led_set_brightness_sync(&v4l2_flash->iled_cdev->led_cdev, + led_set_brightness_sync(v4l2_flash->iled_cdev, brightness); } } @@ -133,7 +133,7 @@ static int v4l2_flash_update_led_brightness(struct v4l2_flash *v4l2_flash, return 0; led_cdev = &v4l2_flash->fled_cdev->led_cdev; } else { - led_cdev = &v4l2_flash->iled_cdev->led_cdev; + led_cdev = v4l2_flash->iled_cdev; } ret = led_update_brightness(led_cdev); @@ -529,8 +529,7 @@ static int v4l2_flash_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd); struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev; struct led_classdev *led_cdev = &fled_cdev->led_cdev; - struct led_classdev_flash *iled_cdev = v4l2_flash->iled_cdev; - struct led_classdev *led_cdev_ind = NULL; + struct led_classdev *led_cdev_ind = v4l2_flash->iled_cdev; int ret = 0; if (!v4l2_fh_is_singular(&fh->vfh)) @@ -543,9 +542,7 @@ static int v4l2_flash_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) mutex_unlock(&led_cdev->led_access); - if (iled_cdev) { - led_cdev_ind = &iled_cdev->led_cdev; - + if (led_cdev_ind) { mutex_lock(&led_cdev_ind->led_access); led_sysfs_disable(led_cdev_ind); @@ -578,7 +575,7 @@ static int v4l2_flash_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) struct v4l2_flash *v4l2_flash = v4l2_subdev_to_v4l2_flash(sd); struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev; struct led_classdev *led_cdev = &fled_cdev->led_cdev; - struct led_classdev_flash *iled_cdev = v4l2_flash->iled_cdev; + struct led_classdev *led_cdev_ind = v4l2_flash->iled_cdev; int ret = 0; if (!v4l2_fh_is_singular(&fh->vfh)) @@ -593,9 +590,7 @@ static int v4l2_flash_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) mutex_unlock(&led_cdev->led_access); - if (iled_cdev) { - struct led_classdev *led_cdev_ind = &iled_cdev->led_cdev; - + if (led_cdev_ind) { mutex_lock(&led_cdev_ind->led_access); led_sysfs_enable(led_cdev_ind); mutex_unlock(&led_cdev_ind->led_access); @@ -614,7 +609,7 @@ static const struct v4l2_subdev_ops v4l2_flash_subdev_ops; struct v4l2_flash *v4l2_flash_init( struct device *dev, struct fwnode_handle *fwn, struct led_classdev_flash *fled_cdev, - struct led_classdev_flash *iled_cdev, + struct led_classdev *iled_cdev, const struct v4l2_flash_ops *ops, struct v4l2_flash_config *config) { diff --git a/include/media/v4l2-flash-led-class.h b/include/media/v4l2-flash-led-class.h index f9dcd54..54e31a8 100644 --- a/include/media/v4l2-flash-led-class.h +++ b/include/media/v4l2-flash-led-class.h @@ -85,7 +85,7 @@ struct v4l2_flash_config { */ struct v4l2_flash { struct led_classdev_flash *fled_cdev; - struct led_classdev_flash *iled_cdev; + struct led_classdev *iled_cdev; const struct v4l2_flash_ops *ops; struct v4l2_subdev sd; @@ -124,7 +124,7 @@ static inline struct v4l2_flash *v4l2_ctrl_to_v4l2_flash(struct v4l2_ctrl *c) struct v4l2_flash *v4l2_flash_init( struct device *dev, struct fwnode_handle *fwn, struct led_classdev_flash *fled_cdev, - struct led_classdev_flash *iled_cdev, + struct led_classdev *iled_cdev, const struct v4l2_flash_ops *ops, struct v4l2_flash_config *config); @@ -140,7 +140,7 @@ void v4l2_flash_release(struct v4l2_flash *v4l2_flash); static inline struct v4l2_flash *v4l2_flash_init( struct device *dev, struct fwnode_handle *fwn, struct led_classdev_flash *fled_cdev, - struct led_classdev_flash *iled_cdev, + struct led_classdev *iled_cdev, const struct v4l2_flash_ops *ops, st
[PATCH 0/8] Support registering lens, flash and EEPROM devices
Hi folks, This set adds support for async registering of lens, flash and EEPROM devices, as well as support for this in the smiapp driver and a LED driver for the as3645a. The lens and flash devices are entities in the media graph whereas the EEPROM is at least currently not. By providing the association information it is possible to add the flash device to the media graph. The smiapp driver makes use of the newly added properties. changes since "Document bindings for camera modules and associated flash devices", https://www.spinics.net/lists/linux-media/msg115124.html>: - Mention flash is a phandle reference to the flash driver chip only. Do not reference to LEDs themselves since this would be somewhat problematic for drivers to handle: the V4L2 sub-devices may have a flash as well as an indicator LED. Alternatively, allowing to use both LED driver and LED references could cause complications in async matching: the flash driver (software) doesn't know which one is presend in the sensor OF node. Instead, I'll propose using numeric IDs for the LEDs, just as we have for clocks for instance. The current definition of a flash driver device reference remains extensible. Due to the changes I've dropped the acks I've received to the flash binding patch. Sakari Ailus (8): dt: bindings: Add a binding for flash devices associated to a sensor dt: bindings: Add lens-focus binding for image sensors dt: bindings: Add a binding for referencing EEPROM from camera sensors v4l2-flash: Use led_classdev instead of led_classdev_flash for indicator v4l2-flash: Flash ops aren't mandatory leds: as3645a: Add LED flash class driver smiapp: Add support for flash, lens and EEPROM devices arm: dts: omap3: N9/N950: Add AS3645A camera flash .../devicetree/bindings/media/video-interfaces.txt | 13 + MAINTAINERS| 6 + arch/arm/boot/dts/omap3-n9.dts | 1 + arch/arm/boot/dts/omap3-n950-n9.dtsi | 14 + arch/arm/boot/dts/omap3-n950.dts | 1 + drivers/leds/Kconfig | 8 + drivers/leds/Makefile | 1 + drivers/leds/leds-as3645a.c| 744 + drivers/media/i2c/smiapp/smiapp-core.c | 81 ++- drivers/media/i2c/smiapp/smiapp.h | 5 + drivers/media/v4l2-core/v4l2-flash-led-class.c | 23 +- include/media/v4l2-flash-led-class.h | 6 +- 12 files changed, 879 insertions(+), 24 deletions(-) create mode 100644 drivers/leds/leds-as3645a.c -- 2.1.4
[PATCH 2/8] dt: bindings: Add lens-focus binding for image sensors
The lens-focus property contains a phandle to the lens voice coil driver that is associated to the sensor; typically both are contained in the same camera module. Signed-off-by: Sakari Ailus Acked-by: Pavel Machek Reviewed-by: Sebastian Reichel --- Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt index 9723f7e..a18d9b2 100644 --- a/Documentation/devicetree/bindings/media/video-interfaces.txt +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt @@ -74,6 +74,8 @@ Optional properties - flash: phandle referring to the flash driver chip. A flash driver may have multiple flashes connected to it. +- lens-focus: A phandle to the node of the focus lens controller. + Optional endpoint properties -- 2.1.4
[PATCH 6/8] leds: as3645a: Add LED flash class driver
From: Sakari Ailus Add a LED flash class driver for the as3654a flash controller. A V4L2 flash driver for it already exists (drivers/media/i2c/as3645a.c), and this driver is based on that. Signed-off-by: Sakari Ailus --- MAINTAINERS | 6 + drivers/leds/Kconfig| 8 + drivers/leds/Makefile | 1 + drivers/leds/leds-as3645a.c | 744 4 files changed, 759 insertions(+) create mode 100644 drivers/leds/leds-as3645a.c diff --git a/MAINTAINERS b/MAINTAINERS index 053c3bd..c7682af 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2067,6 +2067,12 @@ F: arch/arm64/ F: Documentation/arm64/ AS3645A LED FLASH CONTROLLER DRIVER +M: Sakari Ailus +L: linux-l...@vger.kernel.org +S: Maintained +F: drivers/leds/leds-as3645a.c + +AS3645A LED FLASH CONTROLLER DRIVER M: Laurent Pinchart L: linux-media@vger.kernel.org T: git git://linuxtv.org/media_tree.git diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 6c29998..9fb1d86 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -58,6 +58,14 @@ config LEDS_AAT1290 help This option enables support for the LEDs on the AAT1290. +config LEDS_AS3645A + tristate "AS3645A LED flash controller support" + depends on I2C && LEDS_CLASS_FLASH + help + Enable LED flash class support for AS3645A LED flash + controller. V4L2 flash API is provided as well if + CONFIG_V4L2_FLASH_API is enabled. + config LEDS_BCM6328 tristate "LED Support for Broadcom BCM6328" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 45f1339..b4def76 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o # LED Platform Drivers obj-$(CONFIG_LEDS_88PM860X)+= leds-88pm860x.o obj-$(CONFIG_LEDS_AAT1290) += leds-aat1290.o +obj-$(CONFIG_LEDS_AS3645A) += leds-as3645a.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c new file mode 100644 index 000..862d1b5 --- /dev/null +++ b/drivers/leds/leds-as3645a.c @@ -0,0 +1,744 @@ +/* + * drivers/leds/leds-as3645a.c - AS3645A and LM3555 flash controllers driver + * + * Copyright (C) 2008-2011 Nokia Corporation + * Copyright (c) 2011, 2017 Intel Corporation. + * + * Based on drivers/media/i2c/as3645a.c. + * + * Contact: Sakari Ailus + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define AS_TIMER_US_TO_CODE(t) (((t) / 1000 - 100) / 50) +#define AS_TIMER_CODE_TO_US(c) ((50 * (c) + 100) * 1000) + +/* Register definitions */ + +/* Read-only Design info register: Reset state: 0001 */ +#define AS_DESIGN_INFO_REG 0x00 +#define AS_DESIGN_INFO_FACTORY(x) (((x) >> 4)) +#define AS_DESIGN_INFO_MODEL(x)((x) & 0x0f) + +/* Read-only Version control register: Reset state: + * for first engineering samples + */ +#define AS_VERSION_CONTROL_REG 0x01 +#define AS_VERSION_CONTROL_RFU(x) (((x) >> 4)) +#define AS_VERSION_CONTROL_VERSION(x) ((x) & 0x0f) + +/* Read / Write(Indicator and timer register): Reset state: */ +#define AS_INDICATOR_AND_TIMER_REG 0x02 +#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0 +#define AS_INDICATOR_AND_TIMER_VREF_SHIFT 4 +#define AS_INDICATOR_AND_TIMER_INDICATOR_SHIFT 6 + +/* Read / Write(Current set register): Reset state: 0110 1001 */ +#define AS_CURRENT_SET_REG 0x03 +#define AS_CURRENT_ASSIST_LIGHT_SHIFT 0 +#define AS_CURRENT_LED_DET_ON (1 << 3) +#define AS_CURRENT_FLASH_CURRENT_SHIFT 4 + +/* Read / Write(Control register): Reset state: 1011 0100 */ +#define AS_CONTROL_REG 0x04 +#define AS_CONTROL_MODE_SETTING_SHIFT 0 +#define AS_CONTROL_STROBE_ON (1 << 2) +#define AS_CONTROL_OUT_ON (1 << 3) +#define AS_CONTROL_EXT_TORCH_ON(1 << 4) +#define AS_CONTROL_STROBE_TYPE_EDGE(0 << 5) +#define AS_CONTROL_STROBE_TYPE_LEVEL (1 << 5) +#
[PATCH 7/8] smiapp: Add support for flash, lens and EEPROM devices
These types devices aren't directly related to the sensor, but are nevertheless handled by the smiapp driver due to the relationship of these component to the main part of the camera module --- the sensor. Additionally, for the async sub-device registration to work, the notifier containing matching fwnodes will need to be registered. This is natural to perform in a sensor driver as well. This does not yet address providing the user space with information on how to associate the sensor, lens or EEPROM devices but the kernel now has the necessary information to do that. Signed-off-by: Sakari Ailus --- drivers/media/i2c/smiapp/smiapp-core.c | 81 +++--- drivers/media/i2c/smiapp/smiapp.h | 5 +++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index e0b0c03..26f2873 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -2551,6 +2551,22 @@ static int smiapp_register_subdev(struct smiapp_sensor *sensor, return 0; } +static int smiapp_subdev_notifier_bound(struct v4l2_async_notifier *notifier, + struct v4l2_subdev *sd, + struct v4l2_async_subdev *asd) +{ + return 0; +} + +static int smiapp_subdev_notifier_complete( + struct v4l2_async_notifier *notifier) +{ + struct smiapp_sensor *sensor = + container_of(notifier, struct smiapp_sensor, notifier); + + return v4l2_device_register_subdev_nodes(sensor->src->sd.v4l2_dev); +} + static void smiapp_unregistered(struct v4l2_subdev *subdev) { struct smiapp_sensor *sensor = to_smiapp_sensor(subdev); @@ -2558,6 +2574,8 @@ static void smiapp_unregistered(struct v4l2_subdev *subdev) for (i = 1; i < sensor->ssds_used; i++) v4l2_device_unregister_subdev(&sensor->ssds[i].sd); + + v4l2_async_subnotifier_unregister(&sensor->notifier); } static int smiapp_registered(struct v4l2_subdev *subdev) @@ -2581,6 +2599,15 @@ static int smiapp_registered(struct v4l2_subdev *subdev) if (rval) goto out_err; + if (!sensor->notifier.num_subdevs) + return 0; + + sensor->notifier.bound = smiapp_subdev_notifier_bound; + sensor->notifier.complete = smiapp_subdev_notifier_complete; + rval = v4l2_async_subnotifier_register(subdev, &sensor->notifier); + if (rval) + goto out_err; + return 0; out_err: @@ -2782,13 +2809,15 @@ static int __maybe_unused smiapp_resume(struct device *dev) return rval; } -static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev) +static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev, + struct smiapp_sensor *sensor) { + static const char *props[] = { "flash", "lens", "eeprom" }; struct smiapp_hwconfig *hwcfg; struct v4l2_fwnode_endpoint *bus_cfg; struct fwnode_handle *ep; struct fwnode_handle *fwnode = dev_fwnode(dev); - int i; + unsigned int i; int rval; if (!fwnode) @@ -2849,6 +2878,45 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev) v4l2_fwnode_endpoint_free(bus_cfg); fwnode_handle_put(ep); + + sensor->notifier.subdevs = + devm_kcalloc(dev, SMIAPP_MAX_ASYNC_SUBDEVS, +sizeof(struct v4l2_async_subdev *), GFP_KERNEL); + if (!sensor->notifier.subdevs) + goto out_err; + + for (i = 0; i < ARRAY_SIZE(props); i++) { + struct device_node *node; + unsigned int j = 0; + + while ((node = of_parse_phandle(dev->of_node, props[i], j++))) { + struct v4l2_async_subdev **asd = +&sensor->notifier.subdevs[ +sensor->notifier.num_subdevs]; + + if (WARN_ON(sensor->notifier.num_subdevs >= + SMIAPP_MAX_ASYNC_SUBDEVS)) { + of_node_put(node); + goto out; + } + + *asd = devm_kzalloc( + dev, sizeof(struct v4l2_async_subdev), + GFP_KERNEL); + if (!*asd) { + of_node_put(node); + goto out_err; + } + + (*asd)->match.fwnode.fwnode = of_fwnode_handle(node); + (*asd)->match_type = V4L2_ASYNC_MATCH_FWNODE; + sensor->notifier.num_subdevs++; + + of_node_put(node); + } + } + +out: return hwcfg; out_err: @@ -2861,18 +2929,17 @@ static
Re: [PATCH v4 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
Hi Geert, On 14/06/17 10:39, Geert Uytterhoeven wrote: > Hi Kieran, > > On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham wrote: >> From: Kieran Bingham >> >> Provide ADV7482, and the needed connectors >> >> Signed-off-by: Kieran Bingham > > Thanks for your patch! > >> v4: >> - dt: Rebase to dts/renesas/salvator-x.dtsi >> - dt: Use AIN0-7 rather than AIN1-8 >> >> arch/arm64/boot/dts/renesas/salvator-x.dtsi | 123 +- > > I believe all of this applies to both Salvator-X and Salvator-XS? > > Hence it should be applied to salvator-common.dtsi instead of salvator-x.dtsi. Hrm ... I don't have a salator-common.dtsi ... I'll need a new rebase. But it sounds logical :) -- Thanks > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds >
Re: [PATCH v4 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
Hi Kieran, On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham wrote: > From: Kieran Bingham > > Provide ADV7482, and the needed connectors > > Signed-off-by: Kieran Bingham Thanks for your patch! > v4: > - dt: Rebase to dts/renesas/salvator-x.dtsi > - dt: Use AIN0-7 rather than AIN1-8 > > arch/arm64/boot/dts/renesas/salvator-x.dtsi | 123 +- I believe all of this applies to both Salvator-X and Salvator-XS? Hence it should be applied to salvator-common.dtsi instead of salvator-x.dtsi. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v4 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
On Tue, Jun 13, 2017 at 01:35:08AM +0100, Kieran Bingham wrote: > From: Kieran Bingham > > Provide ADV7482, and the needed connectors > > Signed-off-by: Kieran Bingham I am marking this as deferred pending acceptance of the bindings.
Re: [PATCH v4 2/2] arm64: dts: renesas: salvator-x: Add ADV7482 support
Hi Simon, On 14/06/17 10:04, Simon Horman wrote: > On Tue, Jun 13, 2017 at 01:35:08AM +0100, Kieran Bingham wrote: >> From: Kieran Bingham >> >> Provide ADV7482, and the needed connectors >> >> Signed-off-by: Kieran Bingham > > I am marking this as deferred pending acceptance of the bindings. Good point - I didn't include RobH / DT-relevant lists on the mailing :) If only there was a script called get_maintainer.pl to remind me who to include :D Thanks for the heads up. I'll aim to send out a v5 today and include the required lists. -- Kieran
Re: [PATCH v2] [media] v4l2-subdev: check colorimetry in link validate
Hi Helen and Mauro, On Thu, Jun 08, 2017 at 02:05:08PM -0300, Helen Koike wrote: > colorspace, ycbcr_enc, quantization and xfer_func must match > across the link. > Check if they match in v4l2_subdev_link_validate_default > unless they are set as _DEFAULT. I think you could ignore my earlier comments on this --- the check will take place only iff both are not defaults, i.e. non-zero. And these values definitely should be zero unless explicitly set otherwise -- by the driver. I missed this on the previous review round. So I think it'd be fine to return an error on these. How about using dev_dbg() instead if dev_warn()? Using dev_warn() gives an easy way to flood the logs to the user. A debug level message is still important as it's next to impossible for the user to figure out what actually went wrong. Getting a single numeric error code from starting the pipeline isn't telling much... > > Signed-off-by: Helen Koike > > --- > > Hi, > > As discussed previously, I added a warn message instead of returning > error to give drivers some time to adapt. > But the problem is that: as the format is set by userspace, it is > possible that userspace will set the wrong format at pads and see these > messages when there is no error in the driver's code at all (or maybe > this is not a problem, just noise in the log). > > Helen > --- > drivers/media/v4l2-core/v4l2-subdev.c | 38 > +++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c > b/drivers/media/v4l2-core/v4l2-subdev.c > index da78497..1a642c7 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -508,6 +508,44 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev > *sd, > || source_fmt->format.code != sink_fmt->format.code) > return -EPIPE; > > + /* > + * TODO: return -EPIPE instead of printing a warning in the following > + * checks. As colorimetry properties were added after most of the > + * drivers, only a warning was added to avoid potential regressions > + */ > + > + /* colorspace match. */ > + if (source_fmt->format.colorspace != sink_fmt->format.colorspace) > + dev_warn(sd->v4l2_dev->dev, > + "colorspace doesn't match in link > \"%s\":%d->\"%s\":%d\n", > + link->source->entity->name, link->source->index, > + link->sink->entity->name, link->sink->index); > + > + /* Colorimetry must match if they are not set to DEFAULT */ > + if (source_fmt->format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT > + && sink_fmt->format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT > + && source_fmt->format.ycbcr_enc != sink_fmt->format.ycbcr_enc) > + dev_warn(sd->v4l2_dev->dev, > + "YCbCr encoding doesn't match in link > \"%s\":%d->\"%s\":%d\n", > + link->source->entity->name, link->source->index, > + link->sink->entity->name, link->sink->index); > + > + if (source_fmt->format.quantization != V4L2_QUANTIZATION_DEFAULT > + && sink_fmt->format.quantization != V4L2_QUANTIZATION_DEFAULT > + && source_fmt->format.quantization != sink_fmt->format.quantization) > + dev_warn(sd->v4l2_dev->dev, > + "quantization doesn't match in link > \"%s\":%d->\"%s\":%d\n", > + link->source->entity->name, link->source->index, > + link->sink->entity->name, link->sink->index); > + > + if (source_fmt->format.xfer_func != V4L2_XFER_FUNC_DEFAULT > + && sink_fmt->format.xfer_func != V4L2_XFER_FUNC_DEFAULT > + && source_fmt->format.xfer_func != sink_fmt->format.xfer_func) > + dev_warn(sd->v4l2_dev->dev, > + "transfer function doesn't match in link > \"%s\":%d->\"%s\":%d\n", > + link->source->entity->name, link->source->index, > + link->sink->entity->name, link->sink->index); > + > /* The field order must match, or the sink field order must be NONE >* to support interlaced hardware connected to bridges that support >* progressive formats only. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
cron job: media_tree daily build: ERRORS
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: Wed Jun 14 05:00:18 CEST 2017 media-tree git hash:acec3630155763c170c7ae6508cf973355464508 media_build git hash: dbdc2495ec17a3e71d2ec56778eed10081bb718f v4l-utils git hash: ce237eefc1f6dafafc0e1fe3a5fd9f075d3fd066 gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0-3553-g78b2ea6 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.9.0-164 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: WARNINGS linux-git-arm-multi: WARNINGS linux-git-arm-pxa: OK linux-git-arm-stm32: ERRORS linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-3.10.1-i686: ERRORS linux-3.11.1-i686: ERRORS linux-3.12.67-i686: ERRORS linux-3.13.11-i686: ERRORS linux-3.14.9-i686: ERRORS linux-3.15.2-i686: ERRORS linux-3.16.7-i686: ERRORS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: ERRORS linux-4.0.9-i686: ERRORS linux-4.1.33-i686: ERRORS linux-4.2.8-i686: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.7.5-i686: WARNINGS linux-4.8-i686: OK linux-4.9.26-i686: OK linux-4.10.14-i686: OK linux-4.11-i686: OK linux-4.12-rc1-i686: OK linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS linux-3.10.1-x86_64: ERRORS linux-3.11.1-x86_64: ERRORS linux-3.12.67-x86_64: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.9-x86_64: ERRORS linux-3.15.2-x86_64: ERRORS linux-3.16.7-x86_64: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.7-x86_64: ERRORS linux-3.19-x86_64: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.33-x86_64: ERRORS 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-rc1-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup
On 14/06/17 08:18, Ramesh Shanmugasundaram wrote: Subject: Re: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup On 13/06/17 14:33, Ramesh Shanmugasundaram wrote: Hi All, The readx_poll_timeout & similar macros defines local variable that can cause name space collision with the caller. Fixed this issue by prefixing them with underscores. The compound statement has a local variable scope, so these won't collide with the caller I believe. But xxx_poll_timeout is a macro?? Usage regmap_read_poll_timeout(..., timeout) with variable name "timeout" in the caller results in include/linux/regmap.h:123:20: warning: 'timeout' is used uninitialized in this function [-Wuninitialized] ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \ Oh right, collide with a passed in variable, yes. Sorry. Also tidied couple of instances where the macro arguments are used in expressions without paranthesis. This patchset is based on top of today's linux-next repo. commit bc4c75f41a1c ("Add linux-next specific files for 20170613") Change history: v2: - iopoll.h: - Enclosed timeout_us & sleep_us arguments with paranthesis - regmap.h: - Enclosed timeout_us & sleep_us arguments with paranthesis - Renamed pollret to __ret Note: timeout_us cause spare check warning as identified here [1]. [1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg1513 8.html Thanks, Ramesh Ramesh Shanmugasundaram (2): iopoll: Avoid namespace collision within macros & tidyup regmap: Avoid namespace collision within macro & tidyup include/linux/iopoll.h | 12 +++- include/linux/regmap.h | 17 + 2 files changed, 16 insertions(+), 13 deletions(-)
RE: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup
> Subject: Re: [PATCH v2 0/2] Avoid namespace collision within macros & > tidyup > > On 13/06/17 14:33, Ramesh Shanmugasundaram wrote: > > Hi All, > > > > The readx_poll_timeout & similar macros defines local variable that > > can cause name space collision with the caller. Fixed this issue by > > prefixing them with underscores. > > The compound statement has a local variable scope, so these won't collide > with the caller I believe. But xxx_poll_timeout is a macro?? Usage regmap_read_poll_timeout(..., timeout) with variable name "timeout" in the caller results in include/linux/regmap.h:123:20: warning: 'timeout' is used uninitialized in this function [-Wuninitialized] ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \ > > > Also tidied couple of instances where the macro arguments are used in > > expressions without paranthesis. > > > > This patchset is based on top of today's linux-next repo. > > commit bc4c75f41a1c ("Add linux-next specific files for 20170613") > > > > Change history: > > > > v2: > > - iopoll.h: > > - Enclosed timeout_us & sleep_us arguments with paranthesis > > - regmap.h: > > - Enclosed timeout_us & sleep_us arguments with paranthesis > > - Renamed pollret to __ret > > > > Note: timeout_us cause spare check warning as identified here [1]. > > > > [1] > > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg1513 > > 8.html > > > > Thanks, > > Ramesh > > > > Ramesh Shanmugasundaram (2): > >iopoll: Avoid namespace collision within macros & tidyup > >regmap: Avoid namespace collision within macro & tidyup > > > > include/linux/iopoll.h | 12 +++- > > include/linux/regmap.h | 17 + > > 2 files changed, 16 insertions(+), 13 deletions(-) > >