* [PATCH v2 0/3] media: i2c: Introduce driver for the TW9900 decoder
@ 2020-11-13 13:44 Maxime Chevallier
2020-11-13 13:44 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix Maxime Chevallier
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Maxime Chevallier @ 2020-11-13 13:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Heiko Stuebner,
Hans Verkuil, Ezequiel Garcia
Cc: Maxime Chevallier, linux-media, devicetree, linux-kernel,
Thomas Petazzoni, Miquel Raynal, Paul Kocialkowski
Hello everyone,
This is the second version of the series adding support for the Techwell
TW9900 multi standard decoder. It's a pretty simple decoder compared to
the TW9910, since it doesn't have a built-in scaler/crop engine.
Since V2, I've fixed a few issues with the binding, and changed the
media entity type from sensor to decoder following Hans' review.
Any feedback is appreciated,
Thanks,
Maxime
Maxime Chevallier (3):
dt-bindings: vendor-prefixes: Add techwell vendor prefix
media: dt-bindings: media: i2c: Add bindings for TW9900
media: i2c: Introduce a driver for the Techwell TW9900 decoder
.../devicetree/bindings/media/i2c/tw9900.yaml | 60 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 6 +
drivers/media/i2c/Kconfig | 9 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/tw9900.c | 661 ++++++++++++++++++
6 files changed, 739 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml
create mode 100644 drivers/media/i2c/tw9900.c
--
2.25.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix 2020-11-13 13:44 [PATCH v2 0/3] media: i2c: Introduce driver for the TW9900 decoder Maxime Chevallier @ 2020-11-13 13:44 ` Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder Maxime Chevallier 2 siblings, 0 replies; 6+ messages in thread From: Maxime Chevallier @ 2020-11-13 13:44 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Heiko Stuebner, Hans Verkuil, Ezequiel Garcia Cc: Maxime Chevallier, linux-media, devicetree, linux-kernel, Thomas Petazzoni, Miquel Raynal, Paul Kocialkowski, Rob Herring Add prefix for Techwell, Inc. Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 2735be1a8470..ff65b8bff3c5 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1057,6 +1057,8 @@ patternProperties: description: TechNexion "^technologic,.*": description: Technologic Systems + "^techwell,.*": + description: Techwell, Inc. "^tempo,.*": description: Tempo Semiconductor "^techstar,.*": -- 2.25.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 2020-11-13 13:44 [PATCH v2 0/3] media: i2c: Introduce driver for the TW9900 decoder Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix Maxime Chevallier @ 2020-11-13 13:44 ` Maxime Chevallier 2020-11-16 14:58 ` Rob Herring 2020-11-13 13:44 ` [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder Maxime Chevallier 2 siblings, 1 reply; 6+ messages in thread From: Maxime Chevallier @ 2020-11-13 13:44 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Heiko Stuebner, Hans Verkuil, Ezequiel Garcia Cc: Maxime Chevallier, linux-media, devicetree, linux-kernel, Thomas Petazzoni, Miquel Raynal, Paul Kocialkowski The Techwell TW9900 is a video decoder supporting multiple input standards, such as PAL, NTSC and SECAM, and outputs a BT.656 video signal. It's designed to be low-power, posesses some features such as a programmable comb-filter, and automatic input standard detection. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- v1->v2: Fix the example not compiling .../devicetree/bindings/media/i2c/tw9900.yaml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml diff --git a/Documentation/devicetree/bindings/media/i2c/tw9900.yaml b/Documentation/devicetree/bindings/media/i2c/tw9900.yaml new file mode 100644 index 000000000000..fa7e237ca7a6 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/tw9900.yaml @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/tw9900.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Techwell TW9900 NTSC/PAL/SECAM video decoder + +maintainers: + - Maxime Chevallier <maxime.chevallier@bootlin.com> + +description: + The tw9900 is a multi-standard video decoder, supporting NTSC, PAL and SECAM + standards with auto-detection features. + +properties: + compatible: + items: + - enum: + - techwell,tw9900 + + reg: + maxItems: 1 + + vdd-supply: + description: VDD power supply + + port: + type: object + description: + A node containing a single endpoint as doucmented in + Documentation/devicetree/bindings/media/video-interfaces.txt + +additionalProperties: false + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + tw9900: tw9900@44 { + compatible = "techwell,tw9900"; + reg = <0x44>; + + vdd-supply = <&tw9900_supply>; + reset-gpio = <&gpio2 5 GPIO_ACTIVE_LOW>; + + port { + tw9900_out: endpoint { + remote-endpoint = <&vip_in>; + }; + }; + }; + }; -- 2.25.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 2020-11-13 13:44 ` [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 Maxime Chevallier @ 2020-11-16 14:58 ` Rob Herring 0 siblings, 0 replies; 6+ messages in thread From: Rob Herring @ 2020-11-16 14:58 UTC (permalink / raw) To: Maxime Chevallier Cc: devicetree, Ezequiel Garcia, Heiko Stuebner, Thomas Petazzoni, Hans Verkuil, Paul Kocialkowski, linux-media, Mark Rutland, Rob Herring, linux-kernel, Miquel Raynal, Mauro Carvalho Chehab On Fri, 13 Nov 2020 14:44:16 +0100, Maxime Chevallier wrote: > The Techwell TW9900 is a video decoder supporting multiple input > standards, such as PAL, NTSC and SECAM, and outputs a BT.656 video > signal. > > It's designed to be low-power, posesses some features such as a > programmable comb-filter, and automatic input standard detection. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > v1->v2: Fix the example not compiling > > .../devicetree/bindings/media/i2c/tw9900.yaml | 60 +++++++++++++++++++ > 1 file changed, 60 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/tw9900.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/tw9900.example.dt.yaml: tw9900@44: 'reset-gpio' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/tw9900.yaml See https://patchwork.ozlabs.org/patch/1399805 The base for the patch is generally the last rc1. Any dependencies should be noted. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder 2020-11-13 13:44 [PATCH v2 0/3] media: i2c: Introduce driver for the TW9900 decoder Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 Maxime Chevallier @ 2020-11-13 13:44 ` Maxime Chevallier 2020-11-13 15:00 ` Paul Kocialkowski 2 siblings, 1 reply; 6+ messages in thread From: Maxime Chevallier @ 2020-11-13 13:44 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Heiko Stuebner, Hans Verkuil, Ezequiel Garcia Cc: Maxime Chevallier, linux-media, devicetree, linux-kernel, Thomas Petazzoni, Miquel Raynal, Paul Kocialkowski The Techwell video decoder supports PAL, NTSC and SECAM input formats, and outputs a BT.656 signal. This commit adds support for this device, with basic support for NTSC and PAL, along with brightness and contrast controls. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- v1 -> v2: Set the media entity type to decoder, and implement the s_std/g_std ops MAINTAINERS | 6 + drivers/media/i2c/Kconfig | 9 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/tw9900.c | 661 +++++++++++++++++++++++++++++++++++++ 4 files changed, 677 insertions(+) create mode 100644 drivers/media/i2c/tw9900.c diff --git a/MAINTAINERS b/MAINTAINERS index 3da6d8c154e4..2890862a8285 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17160,6 +17160,12 @@ L: linux-media@vger.kernel.org S: Maintained F: drivers/media/rc/ttusbir.c +TECHWELL TW9900 VIDEO DECODER +M: Maxime Chevallier <maxime.chevallier@bootlin.com> +L: linux-media@vger.kernel.org +S: Maintained +F: drivers/media/i2c/tw9900.c + TECHWELL TW9910 VIDEO DECODER L: linux-media@vger.kernel.org S: Orphan diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 878f66ef2719..ea55aea4d949 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -426,6 +426,15 @@ config VIDEO_TW2804 To compile this driver as a module, choose M here: the module will be called tw2804. +config VIDEO_TW9900 + tristate "Techwell TW9900 video decoder" + depends on VIDEO_V4L2 && I2C + help + Support for the Techwell tw9900 multi-standard video decoder. + + To compile this driver as a module, choose M here: the + module will be called tw9900. + config VIDEO_TW9903 tristate "Techwell TW9903 video decoder" depends on VIDEO_V4L2 && I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index f0a77473979d..cbc1d9aedd38 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_VIDEO_TVP5150) += tvp5150.o obj-$(CONFIG_VIDEO_TVP514X) += tvp514x.o obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o obj-$(CONFIG_VIDEO_TW2804) += tw2804.o +obj-$(CONFIG_VIDEO_TW9900) += tw9900.o obj-$(CONFIG_VIDEO_TW9903) += tw9903.o obj-$(CONFIG_VIDEO_TW9906) += tw9906.o obj-$(CONFIG_VIDEO_TW9910) += tw9910.o diff --git a/drivers/media/i2c/tw9900.c b/drivers/media/i2c/tw9900.c new file mode 100644 index 000000000000..2075c3c120af --- /dev/null +++ b/drivers/media/i2c/tw9900.c @@ -0,0 +1,661 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Techwell TW9900 multi-standard video decoder. + * + * Copyright (C) 2018 Fuzhou Rockchip Electronics Co., Ltd. + * Copyright (C) 2020 Maxime Chevallier <maxime.chevallier@bootlin.com> + */ + +#include <linux/clk.h> +#include <linux/device.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> +#include <linux/sysfs.h> +#include <linux/timer.h> +#include <linux/delay.h> +#include <media/media-entity.h> +#include <media/v4l2-async.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-event.h> +#include <media/v4l2-subdev.h> + +#define TW9900_REG_CHIP_ID 0x00 +#define TW9900_REG_CHIP_STATUS 0x01 +#define TW9900_REG_CHIP_STATUS_VLOCK 0x08 +#define TW9900_REG_CHIP_STATUS_VDLOSS 0x80 +#define TW9900_REG_OUT_FMT_CTL 0x03 +#define TW9900_REG_CKHY_HSDLY 0x04 +#define TW9900_REG_OUT_CTRL_I 0x05 +#define TW9900_REG_ANALOG_CTL 0x06 +#define TW9900_REG_CROP_HI 0x07 +#define TW9900_REG_VDELAY_LO 0x08 +#define TW9900_REG_VACTIVE_LO 0x09 +#define TW9900_REG_HACTIVE_LO 0x0B +#define TW9900_REG_CNTRL1 0x0C +#define TW9900_REG_BRIGHT_CTL 0x10 +#define TW9900_REG_CONTRAST_CTL 0x11 +#define TW9900_REG_VBI_CNTL 0x19 +#define TW9900_REG_ANAL_CTL_II 0x1A +#define TW9900_REG_OUT_CTRL_II 0x1B +#define TW9900_REG_STD_SEL 0x1C +#define TW9900_REG_MISSCNT 0x26 +#define TW9900_REG_MISC_CTL_II 0x2F +#define TW9900_REG_VVBI 0x55 + +#define TW9900_CHIP_ID 0x00 + +#define REG_SC_CTRL_MODE TW9900_REG_OUT_FMT_CTL +#define SC_CTRL_MODE_STANDBY 0xA7 +#define SC_CTRL_MODE_STREAMING 0xA0 + +#define REG_NULL 0xFF + +#define VSYNC_POLL_INTERVAL_MS 20 +#define VSYNC_WAIT_MAX_POLLS 50 + +static const char * const tw9900_supply_names[] = { + "vdd", +}; + +#define TW9900_NUM_SUPPLIES ARRAY_SIZE(tw9900_supply_names) + +struct regval { + u8 addr; + u8 val; +}; + +struct tw9900_mode { + u32 width; + u32 height; + u32 skip_top; + u32 std; + u32 field; + const struct regval *reg_list; +}; + +struct tw9900 { + struct i2c_client *client; + struct gpio_desc *reset_gpio; + struct regulator_bulk_data supplies[TW9900_NUM_SUPPLIES]; + + bool streaming; + + struct v4l2_subdev subdev; + struct v4l2_ctrl_handler hdl; + struct media_pad pad; + + struct timer_list timer; + struct work_struct work_i2c_poll; + + const struct tw9900_mode *cur_mode; +}; + +#define to_tw9900(sd) container_of(sd, struct tw9900, subdev) + +static const struct regval tw9900_init_regs[] = { + {TW9900_REG_MISC_CTL_II, 0xE6}, + {TW9900_REG_MISSCNT, 0x24}, + {TW9900_REG_OUT_FMT_CTL, 0xA7}, + {TW9900_REG_ANAL_CTL_II, 0x0A}, + {TW9900_REG_VDELAY_LO, 0x19}, + {TW9900_REG_STD_SEL, 0x00}, + {TW9900_REG_VACTIVE_LO, 0xF0}, + {TW9900_REG_STD_SEL, 0x07}, + {TW9900_REG_CKHY_HSDLY, 0x40}, + {TW9900_REG_ANALOG_CTL, 0x80}, + {TW9900_REG_CNTRL1, 0xdc}, + {TW9900_REG_OUT_CTRL_I, 0x98}, + {REG_NULL, 0x0}, +}; + +static const struct regval tw9900_pal_regs[] = { + {TW9900_REG_STD_SEL, 0x01}, + {REG_NULL, 0x0}, +}; + +static const struct regval tw9900_ntsc_regs[] = { + {TW9900_REG_OUT_FMT_CTL, 0xA4}, + {TW9900_REG_VDELAY_LO, 0x12}, + {TW9900_REG_VACTIVE_LO, 0xf0}, + {TW9900_REG_CROP_HI, 0x02}, + {TW9900_REG_HACTIVE_LO, 0xD0}, + {TW9900_REG_VBI_CNTL, 0x01}, + {TW9900_REG_STD_SEL, 0x00}, + {REG_NULL, 0x0}, +}; + +static const struct tw9900_mode supported_modes[] = { + { + .width = 720, + .height = 576, + .skip_top = 0, + .std = V4L2_STD_PAL, + .field = V4L2_FIELD_NONE, + .reg_list = tw9900_pal_regs, + }, + { + .width = 720, + .height = 480, + .skip_top = 0, + .std = V4L2_STD_NTSC, + .field = V4L2_FIELD_NONE, + .reg_list = tw9900_ntsc_regs, + }, +}; + +static int tw9900_write_reg(struct i2c_client *client, u8 reg, u8 val) +{ + int ret; + + ret = i2c_smbus_write_byte_data(client, reg, val); + + if (ret < 0) + dev_err(&client->dev, "write reg error: %d\n", ret); + + return ret; +} + +static int tw9900_write_array(struct i2c_client *client, + const struct regval *regs) +{ + int i, ret = 0; + + for (i = 0; ret == 0 && regs[i].addr != REG_NULL; i++) + ret = tw9900_write_reg(client, regs[i].addr, regs[i].val); + + return ret; +} + +static inline u8 tw9900_read_reg(struct i2c_client *client, u8 reg) +{ + return i2c_smbus_read_byte_data(client, reg); +} + +static void tw9900_fill_fmt(const struct tw9900_mode *mode, + struct v4l2_mbus_framefmt *fmt) +{ + fmt->code = MEDIA_BUS_FMT_UYVY8_2X8; + fmt->width = mode->width; + fmt->height = mode->height; + fmt->field = mode->field; + fmt->colorspace = V4L2_COLORSPACE_SMPTE170M; +} + +static int tw9900_set_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *fmt) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format; + + tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt); + + mbus_fmt->width = tw9900->cur_mode->width; + mbus_fmt->height = tw9900->cur_mode->height; + + return 0; +} + +static int tw9900_get_fmt(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_format *fmt) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format; + + tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt); + + return 0; +} + +static int tw9900_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_mbus_code_enum *code) +{ + if (code->index >= ARRAY_SIZE(supported_modes)) + return -EINVAL; + + code->code = MEDIA_BUS_FMT_UYVY8_2X8; + + return 0; +} + +static int tw9900_enum_frame_sizes(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_frame_size_enum *fse) +{ + u32 index = fse->index; + + if (index >= ARRAY_SIZE(supported_modes)) + return -EINVAL; + + fse->code = MEDIA_BUS_FMT_UYVY8_2X8; + + fse->min_width = supported_modes[index].width; + fse->max_width = supported_modes[index].width; + fse->max_height = supported_modes[index].height; + fse->min_height = supported_modes[index].height; + + return 0; +} + +static int __tw9900_power_on(struct tw9900 *tw9900) +{ + int ret; + struct device *dev = &tw9900->client->dev; + + if (tw9900->reset_gpio) + gpiod_set_value_cansleep(tw9900->reset_gpio, 1); + + ret = regulator_bulk_enable(TW9900_NUM_SUPPLIES, tw9900->supplies); + if (ret < 0) + goto error; + + usleep_range(50000, 52000); + + if (tw9900->reset_gpio) + gpiod_set_value_cansleep(tw9900->reset_gpio, 0); + + usleep_range(1000, 2000); + + ret = tw9900_write_array(tw9900->client, tw9900_init_regs); + if (ret) { + dev_err(dev, "Failed to init tw9900\n"); + goto error; + } + + return 0; + +error: + + return ret; +} + +static void __tw9900_power_off(struct tw9900 *tw9900) +{ + if (tw9900->reset_gpio) + gpiod_set_value_cansleep(tw9900->reset_gpio, 1); + + regulator_bulk_disable(TW9900_NUM_SUPPLIES, tw9900->supplies); +} + +static int tw9900_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct tw9900 *tw9900 = container_of(ctrl->handler, struct tw9900, hdl); + + if (pm_runtime_suspended(&tw9900->client->dev)) + return 0; + + switch (ctrl->id) { + case V4L2_CID_BRIGHTNESS: + tw9900_write_reg(tw9900->client, 0x10, (u8)ctrl->val); + break; + case V4L2_CID_CONTRAST: + tw9900_write_reg(tw9900->client, 0x11, (u8)ctrl->val); + break; + default: + return -EINVAL; + } + return 0; +} + +static int tw9900_s_stream(struct v4l2_subdev *sd, int on) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + struct i2c_client *client = tw9900->client; + int i, ret = 0; + + on = !!on; + if (on == tw9900->streaming) + goto unlock_and_return; + + if (on) { + ret = pm_runtime_get_sync(&tw9900->client->dev); + if (ret < 0) { + pm_runtime_put_noidle(&client->dev); + goto unlock_and_return; + } + + ret = v4l2_ctrl_handler_setup(sd->ctrl_handler); + if (ret) { + pm_runtime_put(&client->dev); + goto unlock_and_return; + } + + ret = tw9900_write_array(tw9900->client, + tw9900->cur_mode->reg_list); + if (ret) { + pm_runtime_put(&client->dev); + goto unlock_and_return; + } + + /* Wait for VSync lock */ + for (i = 0; i < VSYNC_WAIT_MAX_POLLS; i++) { + u8 status = tw9900_read_reg(tw9900->client, + TW9900_REG_CHIP_STATUS); + if (!(status & TW9900_REG_CHIP_STATUS_VDLOSS) && + (status & TW9900_REG_CHIP_STATUS_VLOCK)) + break; + + msleep(VSYNC_POLL_INTERVAL_MS); + } + + ret = tw9900_write_reg(client, REG_SC_CTRL_MODE, + SC_CTRL_MODE_STREAMING); + if (ret) { + pm_runtime_put(&client->dev); + goto unlock_and_return; + } + + } else { + tw9900_write_reg(client, REG_SC_CTRL_MODE, + SC_CTRL_MODE_STANDBY); + pm_runtime_put(&client->dev); + } + + tw9900->streaming = on; + +unlock_and_return: + + return ret; +} + +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API +static int tw9900_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + struct v4l2_mbus_framefmt *try_fmt; + + try_fmt = v4l2_subdev_get_try_format(sd, fh->pad, 0); + + /* Initialize try_fmt */ + tw9900_fill_fmt(tw9900->cur_mode, try_fmt); + + return 0; +} +#endif + +static int tw9900_runtime_resume(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct tw9900 *tw9900 = to_tw9900(sd); + + return __tw9900_power_on(tw9900); +} + +static int tw9900_runtime_suspend(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct tw9900 *tw9900 = to_tw9900(sd); + + __tw9900_power_off(tw9900); + + return 0; +} + +static int tw9900_subscribe_event(struct v4l2_subdev *sd, + struct v4l2_fh *fh, + struct v4l2_event_subscription *sub) +{ + switch (sub->type) { + case V4L2_EVENT_SOURCE_CHANGE: + return v4l2_src_change_event_subdev_subscribe(sd, fh, sub); + case V4L2_EVENT_CTRL: + return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub); + default: + return -EINVAL; + } +} + +static const struct tw9900_mode *tw9900_get_mode_from_std(v4l2_std_id std) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(supported_modes); i++) { + if (supported_modes[i].std == std) + return &supported_modes[i]; + } + + return NULL; +} + +static int tw9900_s_std(struct v4l2_subdev *sd, v4l2_std_id norm) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + const struct tw9900_mode *mode; + int ret; + + if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL))) + return -EINVAL; + + mode = tw9900_get_mode_from_std(norm); + if (!mode) + return -EINVAL; + + ret = tw9900_write_array(tw9900->client, mode->reg_list); + + if (ret) + return ret; + + tw9900->cur_mode = mode; + + return 0; +} + +static int tw9900_g_std(struct v4l2_subdev *sd, v4l2_std_id *norm) +{ + struct tw9900 *tw9900 = to_tw9900(sd); + + *norm = tw9900->cur_mode->std; + + return 0; +} + +static const struct dev_pm_ops tw9900_pm_ops = { + SET_RUNTIME_PM_OPS(tw9900_runtime_suspend, + tw9900_runtime_resume, NULL) +}; + +static const struct v4l2_subdev_core_ops tw9900_core_ops = { + .subscribe_event = tw9900_subscribe_event, + .unsubscribe_event = v4l2_event_subdev_unsubscribe, +}; + +static const struct v4l2_subdev_video_ops tw9900_video_ops = { + .s_std = tw9900_s_std, + .g_std = tw9900_g_std, + .s_stream = tw9900_s_stream, +}; + +static const struct v4l2_subdev_pad_ops tw9900_pad_ops = { + .enum_mbus_code = tw9900_enum_mbus_code, + .enum_frame_size = tw9900_enum_frame_sizes, + .get_fmt = tw9900_get_fmt, + .set_fmt = tw9900_set_fmt, +}; + +static const struct v4l2_subdev_ops tw9900_subdev_ops = { + .core = &tw9900_core_ops, + .video = &tw9900_video_ops, + .pad = &tw9900_pad_ops, +}; + +static const struct v4l2_ctrl_ops tw9900_ctrl_ops = { + .s_ctrl = tw9900_s_ctrl, +}; + +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API +static const struct v4l2_subdev_internal_ops tw9900_internal_ops = { + .open = tw9900_open, +}; +#endif + +static int tw9900_check_id(struct tw9900 *tw9900, + struct i2c_client *client) +{ + struct device *dev = &tw9900->client->dev; + u8 id; + + id = tw9900_read_reg(client, TW9900_CHIP_ID); + + if (id != TW9900_CHIP_ID) { + dev_err(dev, "Unexpected decoder id(%04x)\n", id); + return -EINVAL; + } + + dev_info(dev, "Detected TW9900 (%04x) decoder\n", TW9900_CHIP_ID); + + return 0; +} + +static int tw9900_configure_regulators(struct tw9900 *tw9900) +{ + u32 i; + + for (i = 0; i < TW9900_NUM_SUPPLIES; i++) + tw9900->supplies[i].supply = tw9900_supply_names[i]; + + return devm_regulator_bulk_get(&tw9900->client->dev, + TW9900_NUM_SUPPLIES, + tw9900->supplies); +} + +static int tw9900_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct device *dev = &client->dev; + struct v4l2_ctrl_handler *hdl; + struct tw9900 *tw9900; + int ret; + + tw9900 = devm_kzalloc(dev, sizeof(*tw9900), GFP_KERNEL); + if (!tw9900) + return -ENOMEM; + + tw9900->client = client; + tw9900->cur_mode = &supported_modes[0]; + + tw9900->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(tw9900->reset_gpio)) + tw9900->reset_gpio = NULL; + + ret = tw9900_configure_regulators(tw9900); + if (ret) { + dev_err(dev, "Failed to get power regulators\n"); + return ret; + } + + v4l2_i2c_subdev_init(&tw9900->subdev, client, &tw9900_subdev_ops); + tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS; + + hdl = &tw9900->hdl; + + v4l2_ctrl_handler_init(hdl, 2); + + v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_BRIGHTNESS, + -128, 127, 1, 0); + v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_CONTRAST, + 0, 255, 1, 0x60); + + tw9900->subdev.ctrl_handler = hdl; + if (hdl->error) { + int err = hdl->error; + + v4l2_ctrl_handler_free(hdl); + return err; + } + + ret = __tw9900_power_on(tw9900); + if (ret) + return ret; + + ret = tw9900_check_id(tw9900, client); + if (ret) + goto err_power_off; + +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API + tw9900->subdev.internal_ops = &tw9900_internal_ops; + tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; +#endif +#if defined(CONFIG_MEDIA_CONTROLLER) + tw9900->pad.flags = MEDIA_PAD_FL_SOURCE; + tw9900->subdev.entity.function = MEDIA_ENT_F_DV_DECODER; + + ret = media_entity_pads_init(&tw9900->subdev.entity, 1, &tw9900->pad); + if (ret < 0) + goto err_power_off; +#endif + + ret = v4l2_async_register_subdev(&tw9900->subdev); + if (ret) { + dev_err(dev, "v4l2 async register subdev failed\n"); + goto err_clean_entity; + } + + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_runtime_idle(dev); + + return 0; + +err_clean_entity: +#if defined(CONFIG_MEDIA_CONTROLLER) + media_entity_cleanup(&tw9900->subdev.entity); +#endif +err_power_off: + __tw9900_power_off(tw9900); + + return ret; +} + +static int tw9900_remove(struct i2c_client *client) +{ + struct v4l2_subdev *sd = i2c_get_clientdata(client); + struct tw9900 *tw9900 = to_tw9900(sd); + + v4l2_async_unregister_subdev(sd); +#if defined(CONFIG_MEDIA_CONTROLLER) + media_entity_cleanup(&sd->entity); +#endif + + pm_runtime_disable(&client->dev); + if (!pm_runtime_status_suspended(&client->dev)) + __tw9900_power_off(tw9900); + pm_runtime_set_suspended(&client->dev); + + return 0; +} + +static const struct i2c_device_id tw9900_id[] = { + {"tw9900", 0}, + {}, +}; + +#if IS_ENABLED(CONFIG_OF) +static const struct of_device_id tw9900_of_match[] = { + { .compatible = "techwell,tw9900" }, + {}, +}; +MODULE_DEVICE_TABLE(of, tw9900_of_match); +#endif + +static struct i2c_driver tw9900_i2c_driver = { + .driver = { + .name = "tw9900", + .pm = &tw9900_pm_ops, + .of_match_table = tw9900_of_match + }, + .probe = tw9900_probe, + .remove = tw9900_remove, + .id_table = tw9900_id, +}; + +module_i2c_driver(tw9900_i2c_driver); + +MODULE_DESCRIPTION("tw9900 decoder driver"); +MODULE_LICENSE("GPL v2"); -- 2.25.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder 2020-11-13 13:44 ` [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder Maxime Chevallier @ 2020-11-13 15:00 ` Paul Kocialkowski 0 siblings, 0 replies; 6+ messages in thread From: Paul Kocialkowski @ 2020-11-13 15:00 UTC (permalink / raw) To: Maxime Chevallier Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Heiko Stuebner, Hans Verkuil, Ezequiel Garcia, linux-media, devicetree, linux-kernel, Thomas Petazzoni, Miquel Raynal [-- Attachment #1: Type: text/plain, Size: 22396 bytes --] Hi, On Fri 13 Nov 20, 14:44, Maxime Chevallier wrote: > The Techwell video decoder supports PAL, NTSC and SECAM input formats, > and outputs a BT.656 signal. > > This commit adds support for this device, with basic support for NTSC > and PAL, along with brightness and contrast controls. > > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > v1 -> v2: Set the media entity type to decoder, and implement the > s_std/g_std ops > > MAINTAINERS | 6 + > drivers/media/i2c/Kconfig | 9 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/tw9900.c | 661 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 677 insertions(+) > create mode 100644 drivers/media/i2c/tw9900.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3da6d8c154e4..2890862a8285 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17160,6 +17160,12 @@ L: linux-media@vger.kernel.org > S: Maintained > F: drivers/media/rc/ttusbir.c > > +TECHWELL TW9900 VIDEO DECODER > +M: Maxime Chevallier <maxime.chevallier@bootlin.com> > +L: linux-media@vger.kernel.org > +S: Maintained > +F: drivers/media/i2c/tw9900.c > + > TECHWELL TW9910 VIDEO DECODER > L: linux-media@vger.kernel.org > S: Orphan > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 878f66ef2719..ea55aea4d949 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -426,6 +426,15 @@ config VIDEO_TW2804 > To compile this driver as a module, choose M here: the > module will be called tw2804. > > +config VIDEO_TW9900 > + tristate "Techwell TW9900 video decoder" > + depends on VIDEO_V4L2 && I2C > + help > + Support for the Techwell tw9900 multi-standard video decoder. > + > + To compile this driver as a module, choose M here: the > + module will be called tw9900. > + > config VIDEO_TW9903 > tristate "Techwell TW9903 video decoder" > depends on VIDEO_V4L2 && I2C > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index f0a77473979d..cbc1d9aedd38 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -49,6 +49,7 @@ obj-$(CONFIG_VIDEO_TVP5150) += tvp5150.o > obj-$(CONFIG_VIDEO_TVP514X) += tvp514x.o > obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o > obj-$(CONFIG_VIDEO_TW2804) += tw2804.o > +obj-$(CONFIG_VIDEO_TW9900) += tw9900.o > obj-$(CONFIG_VIDEO_TW9903) += tw9903.o > obj-$(CONFIG_VIDEO_TW9906) += tw9906.o > obj-$(CONFIG_VIDEO_TW9910) += tw9910.o > diff --git a/drivers/media/i2c/tw9900.c b/drivers/media/i2c/tw9900.c > new file mode 100644 > index 000000000000..2075c3c120af > --- /dev/null > +++ b/drivers/media/i2c/tw9900.c > @@ -0,0 +1,661 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for the Techwell TW9900 multi-standard video decoder. > + * > + * Copyright (C) 2018 Fuzhou Rockchip Electronics Co., Ltd. > + * Copyright (C) 2020 Maxime Chevallier <maxime.chevallier@bootlin.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/device.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > +#include <linux/sysfs.h> > +#include <linux/timer.h> > +#include <linux/delay.h> This header is not sorted. > +#include <media/media-entity.h> > +#include <media/v4l2-async.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-subdev.h> > + > +#define TW9900_REG_CHIP_ID 0x00 > +#define TW9900_REG_CHIP_STATUS 0x01 You have whitespaces here for alignment, not tabs. Same goes for the various other values below. > +#define TW9900_REG_CHIP_STATUS_VLOCK 0x08 > +#define TW9900_REG_CHIP_STATUS_VDLOSS 0x80 Looks like you have two tabs here instead of a whitespace. > +#define TW9900_REG_OUT_FMT_CTL 0x03 > +#define TW9900_REG_CKHY_HSDLY 0x04 > +#define TW9900_REG_OUT_CTRL_I 0x05 > +#define TW9900_REG_ANALOG_CTL 0x06 > +#define TW9900_REG_CROP_HI 0x07 > +#define TW9900_REG_VDELAY_LO 0x08 > +#define TW9900_REG_VACTIVE_LO 0x09 > +#define TW9900_REG_HACTIVE_LO 0x0B > +#define TW9900_REG_CNTRL1 0x0C > +#define TW9900_REG_BRIGHT_CTL 0x10 > +#define TW9900_REG_CONTRAST_CTL 0x11 > +#define TW9900_REG_VBI_CNTL 0x19 > +#define TW9900_REG_ANAL_CTL_II 0x1A > +#define TW9900_REG_OUT_CTRL_II 0x1B > +#define TW9900_REG_STD_SEL 0x1C > +#define TW9900_REG_MISSCNT 0x26 > +#define TW9900_REG_MISC_CTL_II 0x2F > +#define TW9900_REG_VVBI 0x55 > + > +#define TW9900_CHIP_ID 0x00 > + > +#define REG_SC_CTRL_MODE TW9900_REG_OUT_FMT_CTL Why redeefine this here? > +#define SC_CTRL_MODE_STANDBY 0xA7 > +#define SC_CTRL_MODE_STREAMING 0xA0 YOu have a bunch of whitespaces after define here. > + > +#define REG_NULL 0xFF Maybe an explicit name like END or TERMINATOR would be more useful. You could also avoid using a terminator by passing explicit array sizes, which I find more elegant, but it's a personal taste. > + > +#define VSYNC_POLL_INTERVAL_MS 20 > +#define VSYNC_WAIT_MAX_POLLS 50 > + > +static const char * const tw9900_supply_names[] = { > + "vdd", > +}; > + > +#define TW9900_NUM_SUPPLIES ARRAY_SIZE(tw9900_supply_names) Why use the regulator bulk API here if there's only one regulator? Also, it could be nice to have static declarations after struct definitions. > + > +struct regval { > + u8 addr; > + u8 val; > +}; > + > +struct tw9900_mode { > + u32 width; > + u32 height; > + u32 skip_top; > + u32 std; > + u32 field; > + const struct regval *reg_list; > +}; > + > +struct tw9900 { > + struct i2c_client *client; > + struct gpio_desc *reset_gpio; > + struct regulator_bulk_data supplies[TW9900_NUM_SUPPLIES]; > + > + bool streaming; > + > + struct v4l2_subdev subdev; > + struct v4l2_ctrl_handler hdl; > + struct media_pad pad; > + > + struct timer_list timer; > + struct work_struct work_i2c_poll; > + > + const struct tw9900_mode *cur_mode; > +}; Alignment in the structure above is inconsistent. > + > +#define to_tw9900(sd) container_of(sd, struct tw9900, subdev) > + > +static const struct regval tw9900_init_regs[] = { > + {TW9900_REG_MISC_CTL_II, 0xE6}, > + {TW9900_REG_MISSCNT, 0x24}, > + {TW9900_REG_OUT_FMT_CTL, 0xA7}, > + {TW9900_REG_ANAL_CTL_II, 0x0A}, > + {TW9900_REG_VDELAY_LO, 0x19}, > + {TW9900_REG_STD_SEL, 0x00}, > + {TW9900_REG_VACTIVE_LO, 0xF0}, > + {TW9900_REG_STD_SEL, 0x07}, > + {TW9900_REG_CKHY_HSDLY, 0x40}, > + {TW9900_REG_ANALOG_CTL, 0x80}, > + {TW9900_REG_CNTRL1, 0xdc}, > + {TW9900_REG_OUT_CTRL_I, 0x98}, > + {REG_NULL, 0x0}, Maybe add whitespaces after { and before } and pad the final entry too. > +}; > + > +static const struct regval tw9900_pal_regs[] = { > + {TW9900_REG_STD_SEL, 0x01}, > + {REG_NULL, 0x0}, > +}; > + > +static const struct regval tw9900_ntsc_regs[] = { > + {TW9900_REG_OUT_FMT_CTL, 0xA4}, > + {TW9900_REG_VDELAY_LO, 0x12}, > + {TW9900_REG_VACTIVE_LO, 0xf0}, > + {TW9900_REG_CROP_HI, 0x02}, > + {TW9900_REG_HACTIVE_LO, 0xD0}, > + {TW9900_REG_VBI_CNTL, 0x01}, > + {TW9900_REG_STD_SEL, 0x00}, > + {REG_NULL, 0x0}, > +}; And ditto for the above. > + > +static const struct tw9900_mode supported_modes[] = { > + { > + .width = 720, > + .height = 576, > + .skip_top = 0, > + .std = V4L2_STD_PAL, > + .field = V4L2_FIELD_NONE, > + .reg_list = tw9900_pal_regs, > + }, > + { > + .width = 720, > + .height = 480, > + .skip_top = 0, > + .std = V4L2_STD_NTSC, > + .field = V4L2_FIELD_NONE, > + .reg_list = tw9900_ntsc_regs, > + }, > +}; > + > +static int tw9900_write_reg(struct i2c_client *client, u8 reg, u8 val) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte_data(client, reg, val); > + > + if (ret < 0) > + dev_err(&client->dev, "write reg error: %d\n", ret); > + > + return ret; > +} > + > +static int tw9900_write_array(struct i2c_client *client, > + const struct regval *regs) > +{ > + int i, ret = 0; > + > + for (i = 0; ret == 0 && regs[i].addr != REG_NULL; i++) > + ret = tw9900_write_reg(client, regs[i].addr, regs[i].val); > + > + return ret; > +} > + > +static inline u8 tw9900_read_reg(struct i2c_client *client, u8 reg) > +{ > + return i2c_smbus_read_byte_data(client, reg); > +} > + > +static void tw9900_fill_fmt(const struct tw9900_mode *mode, > + struct v4l2_mbus_framefmt *fmt) > +{ > + fmt->code = MEDIA_BUS_FMT_UYVY8_2X8; > + fmt->width = mode->width; > + fmt->height = mode->height; > + fmt->field = mode->field; > + fmt->colorspace = V4L2_COLORSPACE_SMPTE170M; > +} > + > +static int tw9900_set_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format; > + > + tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt); > + > + mbus_fmt->width = tw9900->cur_mode->width; > + mbus_fmt->height = tw9900->cur_mode->height; > + > + return 0; > +} > + > +static int tw9900_get_fmt(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_format *fmt) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format; > + > + tw9900_fill_fmt(tw9900->cur_mode, mbus_fmt); > + > + return 0; > +} > + > +static int tw9900_enum_mbus_code(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_mbus_code_enum *code) > +{ > + if (code->index >= ARRAY_SIZE(supported_modes)) > + return -EINVAL; This check looks unrelated to the mbus code. If only one code is supported, the check should be index >= 1. > + > + code->code = MEDIA_BUS_FMT_UYVY8_2X8; > + > + return 0; > +} > + > +static int tw9900_enum_frame_sizes(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + struct v4l2_subdev_frame_size_enum *fse) > +{ > + u32 index = fse->index; > + > + if (index >= ARRAY_SIZE(supported_modes)) > + return -EINVAL; > + > + fse->code = MEDIA_BUS_FMT_UYVY8_2X8; > + > + fse->min_width = supported_modes[index].width; > + fse->max_width = supported_modes[index].width; > + fse->max_height = supported_modes[index].height; > + fse->min_height = supported_modes[index].height; > + > + return 0; > +} > + > +static int __tw9900_power_on(struct tw9900 *tw9900) Why the heading __ ? > +{ > + int ret; > + struct device *dev = &tw9900->client->dev; > + > + if (tw9900->reset_gpio) > + gpiod_set_value_cansleep(tw9900->reset_gpio, 1); > + > + ret = regulator_bulk_enable(TW9900_NUM_SUPPLIES, tw9900->supplies); > + if (ret < 0) > + goto error; Same comment about using the regulator bulk API in this case. > + > + usleep_range(50000, 52000); > + > + if (tw9900->reset_gpio) > + gpiod_set_value_cansleep(tw9900->reset_gpio, 0); > + > + usleep_range(1000, 2000); > + > + ret = tw9900_write_array(tw9900->client, tw9900_init_regs); > + if (ret) { > + dev_err(dev, "Failed to init tw9900\n"); > + goto error; > + } > + > + return 0; > + > +error: > + > + return ret; > +} > + > +static void __tw9900_power_off(struct tw9900 *tw9900) Ditto about __. > +{ > + if (tw9900->reset_gpio) > + gpiod_set_value_cansleep(tw9900->reset_gpio, 1); > + > + regulator_bulk_disable(TW9900_NUM_SUPPLIES, tw9900->supplies); > +} > + > +static int tw9900_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct tw9900 *tw9900 = container_of(ctrl->handler, struct tw9900, hdl); > + > + if (pm_runtime_suspended(&tw9900->client->dev)) > + return 0; > + > + switch (ctrl->id) { > + case V4L2_CID_BRIGHTNESS: > + tw9900_write_reg(tw9900->client, 0x10, (u8)ctrl->val); > + break; > + case V4L2_CID_CONTRAST: > + tw9900_write_reg(tw9900->client, 0x11, (u8)ctrl->val); > + break; > + default: > + return -EINVAL; > + } > + return 0; > +} > + > +static int tw9900_s_stream(struct v4l2_subdev *sd, int on) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + struct i2c_client *client = tw9900->client; > + int i, ret = 0; > + > + on = !!on; > + if (on == tw9900->streaming) > + goto unlock_and_return; > + > + if (on) { > + ret = pm_runtime_get_sync(&tw9900->client->dev); > + if (ret < 0) { > + pm_runtime_put_noidle(&client->dev); > + goto unlock_and_return; > + } > + > + ret = v4l2_ctrl_handler_setup(sd->ctrl_handler); > + if (ret) { > + pm_runtime_put(&client->dev); Maybe add a label at the end with pm_runtime_put instead of duplicating it everywhere? > + goto unlock_and_return; > + } > + > + ret = tw9900_write_array(tw9900->client, > + tw9900->cur_mode->reg_list); > + if (ret) { > + pm_runtime_put(&client->dev); > + goto unlock_and_return; > + } > + > + /* Wait for VSync lock */ > + for (i = 0; i < VSYNC_WAIT_MAX_POLLS; i++) { > + u8 status = tw9900_read_reg(tw9900->client, > + TW9900_REG_CHIP_STATUS); > + if (!(status & TW9900_REG_CHIP_STATUS_VDLOSS) && > + (status & TW9900_REG_CHIP_STATUS_VLOCK)) > + break; > + > + msleep(VSYNC_POLL_INTERVAL_MS); > + } > + > + ret = tw9900_write_reg(client, REG_SC_CTRL_MODE, > + SC_CTRL_MODE_STREAMING); > + if (ret) { > + pm_runtime_put(&client->dev); > + goto unlock_and_return; > + } > + > + } else { > + tw9900_write_reg(client, REG_SC_CTRL_MODE, > + SC_CTRL_MODE_STANDBY); > + pm_runtime_put(&client->dev); > + } > + > + tw9900->streaming = on; > + > +unlock_and_return: > + > + return ret; > +} > + > +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API You might want to make this a requirement of the driver and avoid the ifdefs for now. > +static int tw9900_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + struct v4l2_mbus_framefmt *try_fmt; > + > + try_fmt = v4l2_subdev_get_try_format(sd, fh->pad, 0); > + > + /* Initialize try_fmt */ > + tw9900_fill_fmt(tw9900->cur_mode, try_fmt); > + > + return 0; > +} > +#endif > + > +static int tw9900_runtime_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct tw9900 *tw9900 = to_tw9900(sd); > + > + return __tw9900_power_on(tw9900); > +} > + > +static int tw9900_runtime_suspend(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct tw9900 *tw9900 = to_tw9900(sd); > + > + __tw9900_power_off(tw9900); > + > + return 0; > +} > + > +static int tw9900_subscribe_event(struct v4l2_subdev *sd, > + struct v4l2_fh *fh, > + struct v4l2_event_subscription *sub) > +{ > + switch (sub->type) { > + case V4L2_EVENT_SOURCE_CHANGE: > + return v4l2_src_change_event_subdev_subscribe(sd, fh, sub); > + case V4L2_EVENT_CTRL: > + return v4l2_ctrl_subdev_subscribe_event(sd, fh, sub); > + default: > + return -EINVAL; > + } > +} > + > +static const struct tw9900_mode *tw9900_get_mode_from_std(v4l2_std_id std) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(supported_modes); i++) { > + if (supported_modes[i].std == std) > + return &supported_modes[i]; > + } You can avoid the { } if you want. > + > + return NULL; > +} > + > +static int tw9900_s_std(struct v4l2_subdev *sd, v4l2_std_id norm) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + const struct tw9900_mode *mode; > + int ret; > + > + if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL))) > + return -EINVAL; > + > + mode = tw9900_get_mode_from_std(norm); > + if (!mode) > + return -EINVAL; > + > + ret = tw9900_write_array(tw9900->client, mode->reg_list); > + > + if (ret) > + return ret; > + > + tw9900->cur_mode = mode; > + > + return 0; > +} > + > +static int tw9900_g_std(struct v4l2_subdev *sd, v4l2_std_id *norm) > +{ > + struct tw9900 *tw9900 = to_tw9900(sd); > + > + *norm = tw9900->cur_mode->std; > + > + return 0; > +} > + > +static const struct dev_pm_ops tw9900_pm_ops = { > + SET_RUNTIME_PM_OPS(tw9900_runtime_suspend, > + tw9900_runtime_resume, NULL) > +}; > + > +static const struct v4l2_subdev_core_ops tw9900_core_ops = { > + .subscribe_event = tw9900_subscribe_event, > + .unsubscribe_event = v4l2_event_subdev_unsubscribe, > +}; This struct would look nicer with tab-padding before the =. > + > +static const struct v4l2_subdev_video_ops tw9900_video_ops = { > + .s_std = tw9900_s_std, > + .g_std = tw9900_g_std, > + .s_stream = tw9900_s_stream, > +}; > + > +static const struct v4l2_subdev_pad_ops tw9900_pad_ops = { > + .enum_mbus_code = tw9900_enum_mbus_code, > + .enum_frame_size = tw9900_enum_frame_sizes, > + .get_fmt = tw9900_get_fmt, > + .set_fmt = tw9900_set_fmt, > +}; Same here, > + > +static const struct v4l2_subdev_ops tw9900_subdev_ops = { > + .core = &tw9900_core_ops, > + .video = &tw9900_video_ops, > + .pad = &tw9900_pad_ops, > +}; > + > +static const struct v4l2_ctrl_ops tw9900_ctrl_ops = { > + .s_ctrl = tw9900_s_ctrl, > +}; here, > + > +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > +static const struct v4l2_subdev_internal_ops tw9900_internal_ops = { > + .open = tw9900_open, and here. > +}; > +#endif > + > +static int tw9900_check_id(struct tw9900 *tw9900, > + struct i2c_client *client) > +{ > + struct device *dev = &tw9900->client->dev; > + u8 id; > + > + id = tw9900_read_reg(client, TW9900_CHIP_ID); > + > + if (id != TW9900_CHIP_ID) { > + dev_err(dev, "Unexpected decoder id(%04x)\n", id); > + return -EINVAL; > + } > + > + dev_info(dev, "Detected TW9900 (%04x) decoder\n", TW9900_CHIP_ID); > + > + return 0; > +} > + > +static int tw9900_configure_regulators(struct tw9900 *tw9900) > +{ > + u32 i; > + > + for (i = 0; i < TW9900_NUM_SUPPLIES; i++) > + tw9900->supplies[i].supply = tw9900_supply_names[i]; > + > + return devm_regulator_bulk_get(&tw9900->client->dev, > + TW9900_NUM_SUPPLIES, > + tw9900->supplies); > +} > + > +static int tw9900_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct v4l2_ctrl_handler *hdl; > + struct tw9900 *tw9900; > + int ret; > + > + tw9900 = devm_kzalloc(dev, sizeof(*tw9900), GFP_KERNEL); > + if (!tw9900) > + return -ENOMEM; > + > + tw9900->client = client; > + tw9900->cur_mode = &supported_modes[0]; > + > + tw9900->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(tw9900->reset_gpio)) > + tw9900->reset_gpio = NULL; > + > + ret = tw9900_configure_regulators(tw9900); > + if (ret) { > + dev_err(dev, "Failed to get power regulators\n"); > + return ret; > + } > + > + v4l2_i2c_subdev_init(&tw9900->subdev, client, &tw9900_subdev_ops); > + tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS; Break this like. > + > + hdl = &tw9900->hdl; > + > + v4l2_ctrl_handler_init(hdl, 2); > + > + v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_BRIGHTNESS, > + -128, 127, 1, 0); > + v4l2_ctrl_new_std(hdl, &tw9900_ctrl_ops, V4L2_CID_CONTRAST, > + 0, 255, 1, 0x60); > + > + tw9900->subdev.ctrl_handler = hdl; > + if (hdl->error) { > + int err = hdl->error; > + > + v4l2_ctrl_handler_free(hdl); > + return err; > + } > + > + ret = __tw9900_power_on(tw9900); > + if (ret) > + return ret; > + > + ret = tw9900_check_id(tw9900, client); > + if (ret) > + goto err_power_off; > + > +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > + tw9900->subdev.internal_ops = &tw9900_internal_ops; > + tw9900->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > +#endif > +#if defined(CONFIG_MEDIA_CONTROLLER) If you're going to keep this conditional instead of a requirement of the driver, it would look nicer with ifdef instead of if defined() like the block just before. > + tw9900->pad.flags = MEDIA_PAD_FL_SOURCE; > + tw9900->subdev.entity.function = MEDIA_ENT_F_DV_DECODER; > + > + ret = media_entity_pads_init(&tw9900->subdev.entity, 1, &tw9900->pad); > + if (ret < 0) > + goto err_power_off; > +#endif > + > + ret = v4l2_async_register_subdev(&tw9900->subdev); > + if (ret) { > + dev_err(dev, "v4l2 async register subdev failed\n"); > + goto err_clean_entity; > + } > + > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + pm_runtime_idle(dev); > + > + return 0; > + > +err_clean_entity: > +#if defined(CONFIG_MEDIA_CONTROLLER) > + media_entity_cleanup(&tw9900->subdev.entity); > +#endif > +err_power_off: > + __tw9900_power_off(tw9900); > + > + return ret; > +} > + > +static int tw9900_remove(struct i2c_client *client) > +{ > + struct v4l2_subdev *sd = i2c_get_clientdata(client); > + struct tw9900 *tw9900 = to_tw9900(sd); > + > + v4l2_async_unregister_subdev(sd); > +#if defined(CONFIG_MEDIA_CONTROLLER) > + media_entity_cleanup(&sd->entity); > +#endif > + > + pm_runtime_disable(&client->dev); > + if (!pm_runtime_status_suspended(&client->dev)) > + __tw9900_power_off(tw9900); > + pm_runtime_set_suspended(&client->dev); > + > + return 0; > +} > + > +static const struct i2c_device_id tw9900_id[] = { > + {"tw9900", 0}, > + {}, > +}; I was told recently that using i2c_device_id tables is deprecated unless there's a realy need for it. > + > +#if IS_ENABLED(CONFIG_OF) > +static const struct of_device_id tw9900_of_match[] = { > + { .compatible = "techwell,tw9900" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, tw9900_of_match); > +#endif > + > +static struct i2c_driver tw9900_i2c_driver = { > + .driver = { > + .name = "tw9900", > + .pm = &tw9900_pm_ops, > + .of_match_table = tw9900_of_match Tab-padding before = would look nicer here. > + }, > + .probe = tw9900_probe, > + .remove = tw9900_remove, > + .id_table = tw9900_id, > +}; > + > +module_i2c_driver(tw9900_i2c_driver); > + > +MODULE_DESCRIPTION("tw9900 decoder driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.25.4 Cheers, Paul -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-16 15:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-13 13:44 [PATCH v2 0/3] media: i2c: Introduce driver for the TW9900 decoder Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add techwell vendor prefix Maxime Chevallier 2020-11-13 13:44 ` [PATCH v2 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900 Maxime Chevallier 2020-11-16 14:58 ` Rob Herring 2020-11-13 13:44 ` [PATCH v2 3/3] media: i2c: Introduce a driver for the Techwell TW9900 decoder Maxime Chevallier 2020-11-13 15:00 ` Paul Kocialkowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).