Re: [PATCH] media: dvb: get rid of VIDEO_SET_SPU_PALETTE
On Mon, May 28, 2018 at 11:32:41AM -0300, Mauro Carvalho Chehab wrote: > No upstream drivers use it. It doesn't make any sense to have > a compat32 code for something that nobody uses upstream. > > Reported-by: Alexander Viro > Signed-off-by: Mauro Carvalho Chehab Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH v2] media: staging: tegra-vde: Reset memory client
Hi Dmitry,
On 05/26/2018 04:27 PM, Dmitry Osipenko wrote:
> DMA requests must be blocked before resetting VDE HW, otherwise it is
> possible to get a memory corruption or a machine hang. Use the reset
> control provided by the Memory Controller to block DMA before resetting
> the VDE HW.
>
> Signed-off-by: Dmitry Osipenko
> ---
>
> Changelog:
>
> v2:
> - Reset HW even if Memory Client resetting fails.
Please note that v1 has already been merged, so if you can make a v3 rebased
on top of the latest media_tree master branch, then I'll queue that up for
4.18.
Regards,
Hans
>
> drivers/staging/media/tegra-vde/tegra-vde.c | 35 +++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c
> b/drivers/staging/media/tegra-vde/tegra-vde.c
> index 90177a59b97c..6f06061a40d9 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -73,6 +73,7 @@ struct tegra_vde {
> struct mutex lock;
> struct miscdevice miscdev;
> struct reset_control *rst;
> + struct reset_control *rst_mc;
> struct gen_pool *iram_pool;
> struct completion decode_completion;
> struct clk *clk;
> @@ -850,9 +851,23 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
> *vde,
>* We rely on the VDE registers reset value, otherwise VDE
>* causes bus lockup.
>*/
> + ret = reset_control_assert(vde->rst_mc);
> + if (ret) {
> + dev_err(dev, "DEC start: Failed to assert MC reset: %d\n",
> + ret);
> + goto put_runtime_pm;
> + }
> +
> ret = reset_control_reset(vde->rst);
> if (ret) {
> - dev_err(dev, "Failed to reset HW: %d\n", ret);
> + dev_err(dev, "DEC start: Failed to reset HW: %d\n", ret);
> + goto put_runtime_pm;
> + }
> +
> + ret = reset_control_deassert(vde->rst_mc);
> + if (ret) {
> + dev_err(dev, "DEC start: Failed to deassert MC reset: %d\n",
> + ret);
> goto put_runtime_pm;
> }
>
> @@ -880,9 +895,18 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde
> *vde,
> ret = timeout;
> }
>
> + /*
> + * At first reset memory client to avoid resetting VDE HW in the
> + * middle of DMA which could result into memory corruption or hang
> + * the whole system.
> + */
> + err = reset_control_assert(vde->rst_mc);
> + if (err)
> + dev_err(dev, "DEC end: Failed to assert MC reset: %d\n", err);
> +
> err = reset_control_assert(vde->rst);
> if (err)
> - dev_err(dev, "Failed to assert HW reset: %d\n", err);
> + dev_err(dev, "DEC end: Failed to assert HW reset: %d\n", err);
>
> put_runtime_pm:
> pm_runtime_mark_last_busy(dev);
> @@ -1074,6 +1098,13 @@ static int tegra_vde_probe(struct platform_device
> *pdev)
> return err;
> }
>
> + vde->rst_mc = devm_reset_control_get_optional(dev, "mc");
> + if (IS_ERR(vde->rst_mc)) {
> + err = PTR_ERR(vde->rst_mc);
> + dev_err(dev, "Could not get MC reset %d\n", err);
> + return err;
> + }
> +
> irq = platform_get_irq_byname(pdev, "sync-token");
> if (irq < 0)
> return irq;
>
Re: [PATCH v2] media: pxa_camera: avoid duplicate s_power calls
Hi Akinobu,
On 05/27/2018 05:30 PM, Akinobu Mita wrote:
> The open() operation for the pxa_camera driver always calls s_power()
> operation to put its subdevice sensor in normal operation mode, and the
> release() operation always call s_power() operation to put the subdevice
> in power saving mode.
>
> This requires the subdevice sensor driver to keep track of its power
> state in order to avoid putting the subdevice in power saving mode while
> the device is still opened by some users.
>
> Many subdevice drivers handle it by the boilerplate code that increments
> and decrements an internal counter in s_power() like below:
>
> /*
>* If the power count is modified from 0 to != 0 or from != 0 to 0,
>* update the power state.
>*/
> if (sensor->power_count == !on) {
> ret = ov5640_set_power(sensor, !!on);
> if (ret)
> goto out;
> }
>
> /* Update the power count. */
> sensor->power_count += on ? 1 : -1;
>
> However, some subdevice drivers don't handle it and may cause a problem
> with the pxa_camera driver if the video device is opened by more than
> two users at the same time.
>
> Instead of propagating the boilerplate code for each subdevice driver
> that implement s_power, this introduces an trick that many V4L2 drivers
> are using with v4l2_fh_is_singular_file().
>
> Cc: Sakari Ailus
> Cc: Mauro Carvalho Chehab
> Signed-off-by: Akinobu Mita
> ---
> * v2
> - Print warning message when s_power() is failed. (not printing warning
> when _vb2_fop_release() is failed as it always returns zero for now)
Please note that v1 has already been merged, so if you can make a v3 rebased
on top of the latest media_tree master branch, then I'll queue that up for
4.18.
Regards,
Hans
>
> drivers/media/platform/pxa_camera.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/pxa_camera.c
> b/drivers/media/platform/pxa_camera.c
> index c71a007..a35f461 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2040,6 +2040,9 @@ static int pxac_fops_camera_open(struct file *filp)
> if (ret < 0)
> goto out;
>
> + if (!v4l2_fh_is_singular_file(filp))
> + goto out;
> +
> ret = sensor_call(pcdev, core, s_power, 1);
> if (ret)
> v4l2_fh_release(filp);
> @@ -2052,13 +2055,23 @@ static int pxac_fops_camera_release(struct file *filp)
> {
> struct pxa_camera_dev *pcdev = video_drvdata(filp);
> int ret;
> -
> - ret = vb2_fop_release(filp);
> - if (ret < 0)
> - return ret;
> + bool fh_singular;
>
> mutex_lock(&pcdev->mlock);
> - ret = sensor_call(pcdev, core, s_power, 0);
> +
> + fh_singular = v4l2_fh_is_singular_file(filp);
> +
> + ret = _vb2_fop_release(filp, NULL);
> +
> + if (fh_singular) {
> + ret = sensor_call(pcdev, core, s_power, 0);
> + if (ret) {
> + dev_warn(pcdev_to_dev(pcdev),
> + "Failed to put subdevice in power saving mode:
> %d\n",
> + ret);
> + }
> + }
> +
> mutex_unlock(&pcdev->mlock);
>
> return ret;
>
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: Tue May 29 05:00:15 CEST 2018 media-tree git hash:a00031c159748f322f771f3c1d5ed944cba4bd30 media_build git hash: b2f4db1adbe0cb2e42e875c16c009f1fa95d3325 v4l-utils git hash: 2a12796b5c22cd1a549eb8fa25db873ced811ca5 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-i686: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.101-i686: ERRORS linux-3.0.101-x86_64: ERRORS linux-3.1.10-i686: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.101-i686: ERRORS linux-3.2.101-x86_64: ERRORS linux-3.3.8-i686: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.113-i686: ERRORS linux-3.4.113-x86_64: ERRORS linux-3.5.7-i686: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-i686: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.10-i686: ERRORS linux-3.7.10-x86_64: ERRORS linux-3.8.13-i686: ERRORS linux-3.8.13-x86_64: ERRORS linux-3.9.11-i686: ERRORS linux-3.9.11-x86_64: ERRORS linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.56-i686: ERRORS linux-3.16.56-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.102-i686: ERRORS linux-3.18.102-x86_64: ERRORS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.51-i686: ERRORS linux-4.1.51-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.109-i686: ERRORS linux-4.4.109-x86_64: ERRORS linux-4.5.7-i686: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-i686: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.10-i686: ERRORS linux-4.7.10-x86_64: ERRORS linux-4.8.17-i686: ERRORS linux-4.8.17-x86_64: ERRORS linux-4.9.91-i686: ERRORS linux-4.9.91-x86_64: ERRORS linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.42-i686: OK linux-4.14.42-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16.8-i686: OK linux-4.16.8-x86_64: OK linux-4.17-rc4-i686: OK linux-4.17-rc4-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
[RESEND PATCH v2] media: dvb-frontends: add Socionext SC1501A ISDB-S/T demodulator driver
This patch adds a frontend driver for the Socionext SC1501A series and Socionext MN88443x ISDB-S/T demodulators. Signed-off-by: Katsuhiro Suzuki --- Changes since v1: - Fix sparse warning about type of constant - Use div_s64() instead of divide operator --- drivers/media/dvb-frontends/Kconfig | 10 + drivers/media/dvb-frontends/Makefile | 1 + drivers/media/dvb-frontends/sc1501a.c | 802 ++ drivers/media/dvb-frontends/sc1501a.h | 27 + 4 files changed, 840 insertions(+) create mode 100644 drivers/media/dvb-frontends/sc1501a.c create mode 100644 drivers/media/dvb-frontends/sc1501a.h diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig index 55e36a4f5215..e9d2c94b290e 100644 --- a/drivers/media/dvb-frontends/Kconfig +++ b/drivers/media/dvb-frontends/Kconfig @@ -739,6 +739,16 @@ config DVB_TC90522 Toshiba TC90522 2xISDB-S 8PSK + 2xISDB-T OFDM demodulator. Say Y when you want to support this frontend. +config DVB_SC1501A + tristate "Socionext SC1501A" + depends on DVB_CORE && I2C + select REGMAP_I2C + default m if !MEDIA_SUBDRV_AUTOSELECT + help + A driver for Socionext SC1501A and Panasonic MN88443x + ISDB-S + ISDB-T demodulator. + Say Y when you want to support this frontend. + comment "Digital terrestrial only tuners/PLL" depends on DVB_CORE diff --git a/drivers/media/dvb-frontends/Makefile b/drivers/media/dvb-frontends/Makefile index 67a783fd5ed0..e204502347ed 100644 --- a/drivers/media/dvb-frontends/Makefile +++ b/drivers/media/dvb-frontends/Makefile @@ -125,6 +125,7 @@ obj-$(CONFIG_DVB_AF9033) += af9033.o obj-$(CONFIG_DVB_AS102_FE) += as102_fe.o obj-$(CONFIG_DVB_GP8PSK_FE) += gp8psk-fe.o obj-$(CONFIG_DVB_TC90522) += tc90522.o +obj-$(CONFIG_DVB_SC1501A) += sc1501a.o obj-$(CONFIG_DVB_HORUS3A) += horus3a.o obj-$(CONFIG_DVB_ASCOT2E) += ascot2e.o obj-$(CONFIG_DVB_HELENE) += helene.o diff --git a/drivers/media/dvb-frontends/sc1501a.c b/drivers/media/dvb-frontends/sc1501a.c new file mode 100644 index ..c3d0369a9448 --- /dev/null +++ b/drivers/media/dvb-frontends/sc1501a.c @@ -0,0 +1,802 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Socionext SC1501A series demodulator driver for ISDB-S/ISDB-T. +// +// Copyright (c) 2018 Socionext Inc. + +#include +#include +#include +#include +#include +#include +#include + +#include "sc1501a.h" + +/* ISDB-S registers */ +#define ATSIDU_S0x2f +#define ATSIDL_S0x30 +#define TSSET_S 0x31 +#define AGCREAD_S 0x5a +#define CPMON1_S0x5e +#define CPMON1_S_FSYNC BIT(5) +#define CPMON1_S_ERRMON BIT(4) +#define CPMON1_S_SIGOFF BIT(3) +#define CPMON1_S_W2LOCK BIT(2) +#define CPMON1_S_W1LOCK BIT(1) +#define CPMON1_S_DW1LOCKBIT(0) +#define TRMON_S 0x60 +#define BERCNFLG_S 0x68 +#define BERCNFLG_S_BERVRDY BIT(5) +#define BERCNFLG_S_BERVCHK BIT(4) +#define BERCNFLG_S_BERDRDY BIT(3) +#define BERCNFLG_S_BERDCHK BIT(2) +#define CNRDXU_S0x69 +#define CNRDXL_S0x6a +#define CNRDYU_S0x6b +#define CNRDYL_S0x6c +#define BERVRDU_S 0x71 +#define BERVRDL_S 0x72 +#define DOSET1_S0x73 + +/* Primary ISDB-T */ +#define PLLASET10x00 +#define PLLASET20x01 +#define PLLBSET10x02 +#define PLLBSET20x03 +#define PLLSET 0x04 +#define OUTCSET 0x08 +#define OUTCSET_CHDRV_8MA 0xff +#define OUTCSET_CHDRV_4MA 0x00 +#define PLDWSET 0x09 +#define PLDWSET_NORMAL 0x00 +#define PLDWSET_PULLDOWN 0xff +#define HIZSET1 0x0a +#define HIZSET2 0x0b + +/* Secondary ISDB-T (for MN884434 only) */ +#define RCVSET 0x00 +#define TSSET1_M0x01 +#define TSSET2_M0x02 +#define TSSET3_M0x03 +#define INTACSET
[RESEND PATCH] media: helene: fix xtal frequency setting at power on
This patch fixes crystal frequency setting when power on this device. Signed-off-by: Katsuhiro Suzuki Acked-by: Abylay Ospan --- Changes from before: - Add Abylay's Ack --- drivers/media/dvb-frontends/helene.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c index 0a4f312c4368..8fcf7a00782a 100644 --- a/drivers/media/dvb-frontends/helene.c +++ b/drivers/media/dvb-frontends/helene.c @@ -924,7 +924,10 @@ static int helene_x_pon(struct helene_priv *priv) helene_write_regs(priv, 0x99, cdata, sizeof(cdata)); /* 0x81 - 0x94 */ - data[0] = 0x18; /* xtal 24 MHz */ + if (priv->xtal == SONY_HELENE_XTAL_16000) + data[0] = 0x10; /* xtal 16 MHz */ + else + data[0] = 0x18; /* xtal 24 MHz */ data[1] = (uint8_t)(0x80 | (0x04 & 0x1F)); /* 4 x 25 = 100uA */ data[2] = (uint8_t)(0x80 | (0x26 & 0x7F)); /* 38 x 0.25 = 9.5pF */ data[3] = 0x80; /* REFOUT signal output 500mVpp */ -- 2.17.0
[RESEND PATCH] media: helene: fix tuning frequency of satellite
This patch fixes tuning frequency of satellite to kHz. That as same as terrestrial one. Signed-off-by: Katsuhiro Suzuki --- drivers/media/dvb-frontends/helene.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c index 04033f0c278b..0a4f312c4368 100644 --- a/drivers/media/dvb-frontends/helene.c +++ b/drivers/media/dvb-frontends/helene.c @@ -523,7 +523,7 @@ static int helene_set_params_s(struct dvb_frontend *fe) enum helene_tv_system_t tv_system; struct dtv_frontend_properties *p = &fe->dtv_property_cache; struct helene_priv *priv = fe->tuner_priv; - int frequencykHz = p->frequency; + int frequencykHz = p->frequency / 1000; uint32_t frequency4kHz = 0; u32 symbol_rate = p->symbol_rate/1000; -- 2.17.0
Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties
On 29.05.2018 02:48, Dmitry Osipenko wrote: > inversion=true" if mask has form of 0x11000111, though this could be not For clarity: I meant s/0x11000111/0xFF000FFF/.
[PATCH v2] media: rcar-vin: enable support for r8a77965
Add the SoC specific information for Renesas r8a77965.
Signed-off-by: Niklas Söderlund
Reviewed-by: Laurent Pinchart
---
* Changes since v1
- Fixed typo in variable name,
s/_rcar_info_r8a77965_routes/rcar_info_r8a77965_routes/.
- Collected Laurents tag.
---
drivers/media/platform/rcar-vin/rcar-core.c | 48 +
1 file changed, 48 insertions(+)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index d3072e166a1ca24f..e6a010ee4ba1f671 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -974,6 +974,50 @@ static const struct rvin_info rcar_info_r8a7796 = {
.routes = rcar_info_r8a7796_routes,
};
+static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 0, .mask = BIT(0) | BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 0, .mask = BIT(1) | BIT(4) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 0, .mask = BIT(2) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 1, .mask = BIT(0) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 1, .mask = BIT(1) | BIT(3) },
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 1, .mask = BIT(2) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 1, .mask = BIT(4) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 2, .mask = BIT(0) },
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 2, .mask = BIT(1) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 2, .mask = BIT(2) },
+ { .csi = RVIN_CSI40, .channel = 2, .vin = 2, .mask = BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 2, .vin = 2, .mask = BIT(4) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 3, .mask = BIT(0) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 3, .mask = BIT(1) | BIT(2) },
+ { .csi = RVIN_CSI40, .channel = 3, .vin = 3, .mask = BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 3, .vin = 3, .mask = BIT(4) },
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 4, .mask = BIT(1) | BIT(4) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 5, .mask = BIT(0) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 5, .mask = BIT(4) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 6, .mask = BIT(0) },
+ { .csi = RVIN_CSI40, .channel = 0, .vin = 6, .mask = BIT(1) },
+ { .csi = RVIN_CSI20, .channel = 0, .vin = 6, .mask = BIT(2) },
+ { .csi = RVIN_CSI40, .channel = 2, .vin = 6, .mask = BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 2, .vin = 6, .mask = BIT(4) },
+ { .csi = RVIN_CSI40, .channel = 1, .vin = 7, .mask = BIT(0) },
+ { .csi = RVIN_CSI20, .channel = 1, .vin = 7, .mask = BIT(1) | BIT(2) },
+ { .csi = RVIN_CSI40, .channel = 3, .vin = 7, .mask = BIT(3) },
+ { .csi = RVIN_CSI20, .channel = 3, .vin = 7, .mask = BIT(4) },
+ { /* Sentinel */ }
+};
+
+static const struct rvin_info rcar_info_r8a77965 = {
+ .model = RCAR_GEN3,
+ .use_mc = true,
+ .max_width = 4096,
+ .max_height = 4096,
+ .routes = rcar_info_r8a77965_routes,
+};
+
static const struct rvin_group_route _rcar_info_r8a77970_routes[] = {
{ .csi = RVIN_CSI40, .channel = 0, .vin = 0, .mask = BIT(0) | BIT(3) },
{ .csi = RVIN_CSI40, .channel = 0, .vin = 1, .mask = BIT(2) },
@@ -1030,6 +1074,10 @@ static const struct of_device_id rvin_of_id_table[] = {
.compatible = "renesas,vin-r8a7796",
.data = &rcar_info_r8a7796,
},
+ {
+ .compatible = "renesas,vin-r8a77965",
+ .data = &rcar_info_r8a77965,
+ },
{
.compatible = "renesas,vin-r8a77970",
.data = &rcar_info_r8a77970,
--
2.17.0
Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties
On 28.05.2018 16:15, Ville Syrjälä wrote:
> On Sat, May 26, 2018 at 06:56:22PM +0300, Dmitry Osipenko wrote:
>> Color keying is the action of replacing pixels matching a given color
>> (or range of colors) with transparent pixels in an overlay when
>> performing blitting. Depending on the hardware capabilities, the
>> matching pixel can either become fully transparent or gain adjustment
>> of the pixels component values.
>>
>> Color keying is found in a large number of devices whose capabilities
>> often differ, but they still have enough common features in range to
>> standardize color key properties. This commit adds nine generic DRM plane
>> properties related to the color keying to cover various HW capabilities.
>>
>> This patch is based on the initial work done by Laurent Pinchart, most of
>> credits for this patch goes to him.
>>
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/gpu/drm/drm_atomic.c | 36 ++
>> drivers/gpu/drm/drm_blend.c | 229 +++
>> include/drm/drm_blend.h | 3 +
>> include/drm/drm_plane.h | 77
>> 4 files changed, 345 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 895741e9cd7d..5b808cb68654 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -799,6 +799,24 @@ static int drm_atomic_plane_set_property(struct
>> drm_plane *plane,
>> state->rotation = val;
>> } else if (property == plane->zpos_property) {
>> state->zpos = val;
>> +} else if (property == plane->colorkey.mode_property) {
>> +state->colorkey.mode = val;
>> +} else if (property == plane->colorkey.min_property) {
>> +state->colorkey.min = val;
>> +} else if (property == plane->colorkey.max_property) {
>> +state->colorkey.max = val;
>> +} else if (property == plane->colorkey.format_property) {
>> +state->colorkey.format = val;
>> +} else if (property == plane->colorkey.mask_property) {
>> +state->colorkey.mask = val;
>> +} else if (property == plane->colorkey.inverted_match_property) {
>> +state->colorkey.inverted_match = val;
>> +} else if (property == plane->colorkey.replacement_mask_property) {
>> +state->colorkey.replacement_mask = val;
>> +} else if (property == plane->colorkey.replacement_value_property) {
>> +state->colorkey.replacement_value = val;
>> +} else if (property == plane->colorkey.replacement_format_property) {
>> +state->colorkey.replacement_format = val;
>> } else if (property == plane->color_encoding_property) {
>> state->color_encoding = val;
>> } else if (property == plane->color_range_property) {
>> @@ -864,6 +882,24 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>> *val = state->rotation;
>> } else if (property == plane->zpos_property) {
>> *val = state->zpos;
>> +} else if (property == plane->colorkey.mode_property) {
>> +*val = state->colorkey.mode;
>> +} else if (property == plane->colorkey.min_property) {
>> +*val = state->colorkey.min;
>> +} else if (property == plane->colorkey.max_property) {
>> +*val = state->colorkey.max;
>> +} else if (property == plane->colorkey.format_property) {
>> +*val = state->colorkey.format;
>> +} else if (property == plane->colorkey.mask_property) {
>> +*val = state->colorkey.mask;
>> +} else if (property == plane->colorkey.inverted_match_property) {
>> +*val = state->colorkey.inverted_match;
>> +} else if (property == plane->colorkey.replacement_mask_property) {
>> +*val = state->colorkey.replacement_mask;
>> +} else if (property == plane->colorkey.replacement_value_property) {
>> +*val = state->colorkey.replacement_value;
>> +} else if (property == plane->colorkey.replacement_format_property) {
>> +*val = state->colorkey.replacement_format;
>> } else if (property == plane->color_encoding_property) {
>> *val = state->color_encoding;
>> } else if (property == plane->color_range_property) {
>> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>> index a16a74d7e15e..05e5632ce375 100644
>> --- a/drivers/gpu/drm/drm_blend.c
>> +++ b/drivers/gpu/drm/drm_blend.c
>> @@ -107,6 +107,11 @@
>> * planes. Without this property the primary plane is always below the
>> cursor
>> * plane, and ordering between all other planes is undefined.
>> *
>> + * colorkey:
>> + * Color keying is set up with drm_plane_create_colorkey_properties().
>> + * It adds support for replacing a range of colors with a transparent
>> + * color in the plane.
>> + *
>> * Note that all the property extensions described here apply either to the
>> * plane or the CRTC (e.g. for the background color, which currently
[PATCH v2] rcar-vin: sync which hardware buffer to start capture from
When starting the VIN capture procedure we are not guaranteed that the
first buffer written to is VnMB1 to which we assigned the first buffer
queued. This is problematic for two reasons. Buffers might not be
dequeued in the same order they where queued for capture. Future
features planed for the VIN driver is support for outputting frames in
SEQ_TB/BT format and to do that it's important that capture starts from
the first buffer slot, VnMB1.
We are guaranteed that capturing always happens in sequence (VnMB1 ->
VnMB2 -> VnMB3 -> VnMB1). So drop up to two frames when starting
capturing so that the driver always returns buffers in the same order
they are queued and prepare for SEQ_TB/BT output.
Signed-off-by: Niklas Söderlund
---
* Changes since v1
- Fix spelling in commit message pointed out by Sergei.
s/writing/written/ and s/outputing/outputting/.
---
drivers/media/platform/rcar-vin/rcar-dma.c | 16 +++-
drivers/media/platform/rcar-vin/rcar-vin.h | 2 ++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99e3516a620..cfe5d7a9d44ee0e1 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -856,7 +856,7 @@ static int rvin_capture_start(struct rvin_dev *vin)
/* Continuous Frame Capture Mode */
rvin_write(vin, VNFC_C_FRAME, VNFC_REG);
- vin->state = RUNNING;
+ vin->state = STARTING;
return 0;
}
@@ -910,6 +910,20 @@ static irqreturn_t rvin_irq(int irq, void *data)
vnms = rvin_read(vin, VNMS_REG);
slot = (vnms & VNMS_FBS_MASK) >> VNMS_FBS_SHIFT;
+ /*
+* To hand buffers back in a known order to userspace start
+* to capture first from slot 0.
+*/
+ if (vin->state == STARTING) {
+ if (slot != 0) {
+ vin_dbg(vin, "Starting sync slot: %d\n", slot);
+ goto done;
+ }
+
+ vin_dbg(vin, "Capture start synced!\n");
+ vin->state = RUNNING;
+ }
+
/* Capture frame */
if (vin->queue_buf[slot]) {
vin->queue_buf[slot]->field = vin->format.field;
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
b/drivers/media/platform/rcar-vin/rcar-vin.h
index c2aef789b491ab31..ff747e22d8cfb643 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -53,11 +53,13 @@ enum rvin_csi_id {
/**
* STOPPED - No operation in progress
+ * STARTING - Capture starting up
* RUNNING - Operation in progress have buffers
* STOPPING - Stopping operation
*/
enum rvin_dma_state {
STOPPED = 0,
+ STARTING,
RUNNING,
STOPPING,
};
--
2.17.0
[PATCH v2] dt-bindings: media: rcar_vin: fix style for ports and endpoints
The style for referring to ports and endpoint are wrong. Refer to them using lowercase and a unit address, port@x and endpoint@x. Signed-off-by: Niklas Söderlund Reported-by: Geert Uytterhoeven Reviewed-by: Rob Herring --- * Changes since v1. - Fixed spelling reported by Sergei, s/then/than/. - Collected Rob's tag. --- .../devicetree/bindings/media/rcar_vin.txt| 20 +-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt index c2c57dcf73f4851b..f8b50f8a3a0bd5d3 100644 --- a/Documentation/devicetree/bindings/media/rcar_vin.txt +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt @@ -45,23 +45,23 @@ The per-board settings Gen2 platforms: The per-board settings Gen3 platforms: Gen3 platforms can support both a single connected parallel input source -from external SoC pins (port0) and/or multiple parallel input sources -from local SoC CSI-2 receivers (port1) depending on SoC. +from external SoC pins (port@0) and/or multiple parallel input sources +from local SoC CSI-2 receivers (port@1) depending on SoC. - renesas,id - ID number of the VIN, VINx in the documentation. - ports -- port 0 - sub-node describing a single endpoint connected to the VIN +- port@0 - sub-node describing a single endpoint connected to the VIN from external SoC pins described in video-interfaces.txt[1]. - Describing more then one endpoint in port 0 is invalid. Only VIN - instances that are connected to external pins should have port 0. -- port 1 - sub-nodes describing one or more endpoints connected to + Describing more than one endpoint in port@0 is invalid. Only VIN + instances that are connected to external pins should have port@0. +- port@1 - sub-nodes describing one or more endpoints connected to the VIN from local SoC CSI-2 receivers. The endpoint numbers must use the following schema. -- Endpoint 0 - sub-node describing the endpoint connected to CSI20 -- Endpoint 1 - sub-node describing the endpoint connected to CSI21 -- Endpoint 2 - sub-node describing the endpoint connected to CSI40 -- Endpoint 3 - sub-node describing the endpoint connected to CSI41 +- endpoint@0 - sub-node describing the endpoint connected to CSI20 +- endpoint@1 - sub-node describing the endpoint connected to CSI21 +- endpoint@2 - sub-node describing the endpoint connected to CSI40 +- endpoint@3 - sub-node describing the endpoint connected to CSI41 Device node example for Gen2 platforms -- -- 2.17.0
[ragnatech:media-tree] BUILD SUCCESS a00031c159748f322f771f3c1d5ed944cba4bd30
tree/branch: git://git.ragnatech.se/linux media-tree branch HEAD: a00031c159748f322f771f3c1d5ed944cba4bd30 media: ddbridge: conditionally enable fast TS for stv0910-equipped bridges elapsed time: 31m configs tested: 107 The following configs have been built successfully. More configs may be tested in the coming days. alpha defconfig pariscallnoconfig parisc b180_defconfig pariscc3000_defconfig parisc defconfig i386 tinyconfig i386 randconfig-x012-201821 i386 randconfig-x017-201821 i386 randconfig-x014-201821 i386 randconfig-x016-201821 i386 randconfig-x018-201821 i386 randconfig-x013-201821 i386 randconfig-x011-201821 i386 randconfig-x015-201821 i386 randconfig-x019-201821 i386 randconfig-x010-201821 x86_64 randconfig-x003-201821 x86_64 randconfig-x007-201821 x86_64 randconfig-x000-201821 x86_64 randconfig-x005-201821 x86_64 randconfig-x004-201821 x86_64 randconfig-x001-201821 x86_64 randconfig-x008-201821 x86_64 randconfig-x002-201821 x86_64 randconfig-x006-201821 x86_64 randconfig-x009-201821 i386 randconfig-i0-201821 i386 randconfig-i1-201821 i386 allmodconfig x86_64 randconfig-x011-201821 x86_64 randconfig-x015-201821 x86_64 randconfig-x018-201821 x86_64 randconfig-x010-201821 x86_64 randconfig-x019-201821 x86_64 randconfig-x014-201821 x86_64 randconfig-x013-201821 x86_64 randconfig-x016-201821 x86_64 randconfig-x017-201821 x86_64 randconfig-x012-201821 ia64 alldefconfig ia64 allnoconfig ia64defconfig i386 randconfig-a0-201821 i386 randconfig-a1-201821 armmini2440_defconfig m68k bvme6000_defconfig powerpc g5_defconfig powerpc mpc832x_mds_defconfig m68k m5475evb_defconfig m68k multi_defconfig m68k sun3_defconfig i386 randconfig-s0-201821 i386 randconfig-s1-201821 openriscor1ksim_defconfig um i386_defconfig um x86_64_defconfig x86_64 randconfig-i0-201821 sparc defconfig sparc64 allnoconfig sparc64 defconfig x86_64 randconfig-u0-05290433 i386 randconfig-x078-201821 i386 randconfig-x079-201821 i386 randconfig-x070-201821 i386 randconfig-x075-201821 i386 randconfig-x076-201821 i386 randconfig-x074-201821 i386 randconfig-x071-201821 i386 randconfig-x073-201821 i386 randconfig-x077-201821 i386 randconfig-x072-201821 i386 randconfig-x008-201821 i386 randconfig-x009-201821 i386 randconfig-x005-201821 i386 randconfig-x000-201821 i386 randconfig-x003-201821 i386 randconfig-x001-201821 i386 randconfig-x004-201821 i386 randconfig-x006-201821 i386 randconfig-x002-201821 i386 randconfig-x007-201821 mips 32r2_defconfig mips 64r6el_defconfig mips allnoconfig mips fuloong2e_defconfig mips jz4740 mips malta_kvm_defconfig mips txx9 powerpc allnoconfig powerpc defconfig powerpc ppc64_defconfig s390default_defconfig microblaze mmu_defconfig microblazenommu_defconfig arm allnoconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm
Re: [PATCH v2] media: pxa_camera: avoid duplicate s_power calls
On Mon, May 28, 2018 at 12:30:11AM +0900, Akinobu Mita wrote:
> The open() operation for the pxa_camera driver always calls s_power()
> operation to put its subdevice sensor in normal operation mode, and the
> release() operation always call s_power() operation to put the subdevice
> in power saving mode.
>
> This requires the subdevice sensor driver to keep track of its power
> state in order to avoid putting the subdevice in power saving mode while
> the device is still opened by some users.
>
> Many subdevice drivers handle it by the boilerplate code that increments
> and decrements an internal counter in s_power() like below:
>
> /*
>* If the power count is modified from 0 to != 0 or from != 0 to 0,
>* update the power state.
>*/
> if (sensor->power_count == !on) {
> ret = ov5640_set_power(sensor, !!on);
> if (ret)
> goto out;
> }
>
> /* Update the power count. */
> sensor->power_count += on ? 1 : -1;
>
> However, some subdevice drivers don't handle it and may cause a problem
> with the pxa_camera driver if the video device is opened by more than
> two users at the same time.
>
> Instead of propagating the boilerplate code for each subdevice driver
> that implement s_power, this introduces an trick that many V4L2 drivers
> are using with v4l2_fh_is_singular_file().
>
> Cc: Sakari Ailus
> Cc: Mauro Carvalho Chehab
> Signed-off-by: Akinobu Mita
Acked-by: Sakari Ailus
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH] media: imx258: get rid of an unused var
On Mon, May 28, 2018 at 01:57:11PM -0400, Mauro Carvalho Chehab wrote: > drivers/media/i2c/imx258.c: In function 'imx258_init_controls': > drivers/media/i2c/imx258.c:1117:6: warning: variable 'exposure_max' set but > not used [-Wunused-but-set-variable] > s64 exposure_max; > ^~~~ > > Signed-off-by: Mauro Carvalho Chehab Acked-by: Sakari Ailus -- Sakari Ailus sakari.ai...@linux.intel.com
[PATCH] media: imx258: get rid of an unused var
drivers/media/i2c/imx258.c: In function 'imx258_init_controls':
drivers/media/i2c/imx258.c:1117:6: warning: variable 'exposure_max' set but not
used [-Wunused-but-set-variable]
s64 exposure_max;
^~~~
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/i2c/imx258.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index fad3012f4fe5..f3b124723aa0 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -1114,7 +1114,6 @@ static int imx258_init_controls(struct imx258 *imx258)
{
struct i2c_client *client = v4l2_get_subdevdata(&imx258->sd);
struct v4l2_ctrl_handler *ctrl_hdlr;
- s64 exposure_max;
s64 vblank_def;
s64 vblank_min;
s64 pixel_rate_min;
@@ -1168,7 +1167,6 @@ static int imx258_init_controls(struct imx258 *imx258)
if (imx258->hblank)
imx258->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
- exposure_max = imx258->cur_mode->vts_def - 8;
imx258->exposure = v4l2_ctrl_new_std(
ctrl_hdlr, &imx258_ctrl_ops,
V4L2_CID_EXPOSURE, IMX258_EXPOSURE_MIN,
--
2.17.0
[PATCH 0/5] Remove sh_mobile_ceu_camera from arch/sh
Hello, this series removes dependencies on the soc_camera based sh_mobile_ceu_camera driver from 3 board files in arch/sh and from one sensor driver used by one of those boards. Hans, this means there are no more user of the soc_camera framework that I know of in Linux, and I guess we can now plan of to remove that framework. A note on the sensor driver: I haven't been able to find any documentation for the SHARP RJ54N1CB0C sensor and so I inferred as much as possible from the existing code. It seems to me that the sensor needs to power-up/enable gpios (both active high) and I have registered them from the kfr2r09 board file, assuming this was not a platform-specific design, but something the sensor requires. As per the previous drivers ported away from soc_camera, I'm in favour of moving them to staging if they do not match the quality expected from a modern V4L2 sensor driver. Ie. framerate control is missing, and I know this has been a blocker for other drivers in the past. What I've done to three board files closely resembles what I done already for Migo-R and Ecovec, and it listed in the single commit messages. The only tough one is probably ap325rxa, which had an additional sensor registered but controlled from i2c transaction of magic blobs from the board file. I dare to remove that sensor registration completely as it seems to me there is no diver for that at all in mainline. Last, this patch is based on the media tree master branch, with Akinobu Mita's patches on ov772x driver on top, which I strangely see only partially applied to the media master tree [1] Hans, as this is mostly media-related work, but mostly on SH architecture, I would like to ask if these should go through you or SH tree. All of that has only been compile tested. It's pretty hard to find this platforms around and testing would be awesome if somebody happens to have one of these. Thanks j [1] https://patchwork.linuxtv.org/patch/49318/ https://patchwork.linuxtv.org/patch/49317/ https://patchwork.linuxtv.org/patch/49312/ Jacopo Mondi (5): media: i2c: Copy rj54n1cb0c soc_camera sensor driver media: i2c: rj54n1: Remove soc_camera dependencies arch: sh: kfr2r09: Use new renesas-ceu camera driver arch: sh: ms7724se: Use new renesas-ceu camera driver arch: sh: ap325rxa: Use new renesas-ceu camera driver arch/sh/boards/mach-ap325rxa/setup.c | 282 ++- arch/sh/boards/mach-kfr2r09/setup.c| 216 +++-- arch/sh/boards/mach-se/7724/setup.c| 120 ++- arch/sh/kernel/cpu/sh4a/clock-sh7723.c |2 +- drivers/media/i2c/Kconfig | 11 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/rj54n1cb0c.c | 1437 7 files changed, 1710 insertions(+), 359 deletions(-) create mode 100644 drivers/media/i2c/rj54n1cb0c.c -- 2.7.4
[PATCH 3/5] arch: sh: kfr2r09: Use new renesas-ceu camera driver
Use the new renesas-ceu camera driver in kfr2r09 board file instead of
the soc_camera based sh_mobile_ceu_camera driver.
Get rid of soc_camera specific components, and move clk and gpio handling
away from board file, registering the clock source and the enable gpios
for driver consumption.
Memory for the CEU video buffers is now reserved with membocks APIs,
and need to be declared as dma_coherent during machine initialization to
remove that architecture specific part from CEU driver.
While at there update license to SPDX header and sort headers alphabetically.
No need to udapte the clock source names, as
commit c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera
driver")
already updated it to the new ceu driver name for all SH7724 boards (possibly
breaking kfr2r09 before this commit).
Compile tested only.
Signed-off-by: Jacopo Mondi
---
arch/sh/boards/mach-kfr2r09/setup.c | 217 +---
1 file changed, 103 insertions(+), 114 deletions(-)
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c
b/arch/sh/boards/mach-kfr2r09/setup.c
index 6af..e59c577 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -1,41 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* KFR2R09 board support code
*
* Copyright (C) 2009 Magnus Damm
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file "COPYING" in the main directory of this archive
- * for more details.
*/
-#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
+#include
+#include
+#include
#include
+#include
#include
#include
+#include
#include
#include
-#include
+
+#include
+
+#include
#include
-#include
-#include
+
#include
-#include
-#include
-#include
-#include
-#include
-#include
+
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu_dma_membase;
+
+/* set VIO_CKO clock to 25MHz */
+#define CEU_MCLK_FREQ 2500
+#define DRVCRB 0xA405018C
static struct mtd_partition kfr2r09_nor_flash_partitions[] =
{
@@ -230,8 +242,17 @@ static struct platform_device kfr2r09_usb0_gadget_device =
{
.resource = kfr2r09_usb0_gadget_resources,
};
-static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
- .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu_pdata = {
+ .num_subdevs= 1,
+ .subdevs = {
+ { /* [0] = rj54n1cb0c */
+ .flags = 0,
+ .bus_width = 8,
+ .bus_shift = 0,
+ .i2c_adapter_id = 1,
+ .i2c_address= 0x50,
+ },
+ },
};
static struct resource kfr2r09_ceu_resources[] = {
@@ -246,109 +267,35 @@ static struct resource kfr2r09_ceu_resources[] = {
.end= evt2irq(0x880),
.flags = IORESOURCE_IRQ,
},
- [2] = {
- /* place holder for contiguous memory */
- },
};
static struct platform_device kfr2r09_ceu_device = {
- .name = "sh_mobile_ceu",
+ .name = "renesas-ceu",
.id = 0, /* "ceu0" clock */
.num_resources = ARRAY_SIZE(kfr2r09_ceu_resources),
.resource = kfr2r09_ceu_resources,
.dev= {
- .platform_data = &sh_mobile_ceu_info,
+ .platform_data = &ceu_pdata,
},
};
-static struct i2c_board_info kfr2r09_i2c_camera = {
- I2C_BOARD_INFO("rj54n1cb0c", 0x50),
-};
-
-static struct clk *camera_clk;
-
-/* set VIO_CKO clock to 25MHz */
-#define CEU_MCLK_FREQ 2500
-
-#define DRVCRB 0xA405018C
-static int camera_power(struct device *dev, int mode)
-{
- int ret;
-
- if (mode) {
- long rate;
-
- camera_clk = clk_get(NULL, "video_clk");
- if (IS_ERR(camera_clk))
- return PTR_ERR(camera_clk);
-
- rate = clk_round_rate(camera_clk, CEU_MCLK_FREQ);
- ret = clk_set_rate(camera_clk, rate);
- if (ret < 0)
- goto eclkrate;
-
- /* set DRVCRB
-*
-* use 1.8 V for VccQ_VIO
-* use 2.85V for VccQ_SR
-*/
- __raw_writew((__raw_readw(DRVCRB) & ~0x0003) | 0x0001, DRVCRB);
-
- /* reset clear */
- ret = gpio_request(GPIO_PTB4, NULL);
- if (ret < 0)
- goto eptb4;
- ret = gpio_request(GPIO_PTB7, NULL);
- if (ret < 0)
- goto eptb7;
-
- ret
[PATCH 4/5] arch: sh: ms7724se: Use new renesas-ceu camera driver
Use the new renesas-ceu camera driver is ms7724se board file instead of
the soc_camera based sh_mobile_ceu_camera driver.
Get rid of soc_camera specific components, and register CEU0 and CEU1 with
no active video subdevices.
Memory for the CEU video buffers is now reserved with membocks APIs
and need to be declared as dma_coherent during machine initialization to
remove that architecture specific part from CEU driver.
While at there update license to SPDX header and sort headers
alphabetically.
No need to udapte the clock source names, as
commit c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera
driver")
already updated it to the new ceu driver name for all SH7724 boards
(possibly breaking ms7724se before this commit).
Compile tested only.
Signed-off-by: Jacopo Mondi
---
arch/sh/boards/mach-se/7724/setup.c | 120
1 file changed, 79 insertions(+), 41 deletions(-)
diff --git a/arch/sh/boards/mach-se/7724/setup.c
b/arch/sh/boards/mach-se/7724/setup.c
index 2559525..fdbec22a 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -1,43 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* linux/arch/sh/boards/se/7724/setup.c
*
* Copyright (C) 2009 Renesas Solutions Corp.
*
* Kuninori Morimoto
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file "COPYING" in the main directory of this archive
- * for more details.
*/
+#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
-#include
-#include
#include
#include
+#include
+#include
#include
-#include
-#include
+
+#include
+#include
+
#include
#include
-#include
-#include
-#include
-#include
-#include
-#include
+
+#include
+
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu0_dma_membase;
+static phys_addr_t ceu1_dma_membase;
/*
* SWx1234 5678
@@ -216,8 +222,8 @@ static struct platform_device lcdc_device = {
};
/* CEU0 */
-static struct sh_mobile_ceu_info sh_mobile_ceu0_info = {
- .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu0_pdata = {
+ .num_subdevs = 0,
};
static struct resource ceu0_resources[] = {
@@ -231,24 +237,21 @@ static struct resource ceu0_resources[] = {
.start = evt2irq(0x880),
.flags = IORESOURCE_IRQ,
},
- [2] = {
- /* place holder for contiguous memory */
- },
};
static struct platform_device ceu0_device = {
- .name = "sh_mobile_ceu",
- .id = 0, /* "ceu0" clock */
+ .name = "renesas-ceu",
+ .id = 0, /* "ceu.0" clock */
.num_resources = ARRAY_SIZE(ceu0_resources),
.resource = ceu0_resources,
.dev= {
- .platform_data = &sh_mobile_ceu0_info,
+ .platform_data = &ceu0_pdata,
},
};
/* CEU1 */
-static struct sh_mobile_ceu_info sh_mobile_ceu1_info = {
- .flags = SH_CEU_FLAG_USE_8BIT_BUS,
+static struct ceu_platform_data ceu1_pdata = {
+ .num_subdevs = 0,
};
static struct resource ceu1_resources[] = {
@@ -262,18 +265,15 @@ static struct resource ceu1_resources[] = {
.start = evt2irq(0x9e0),
.flags = IORESOURCE_IRQ,
},
- [2] = {
- /* place holder for contiguous memory */
- },
};
static struct platform_device ceu1_device = {
- .name = "sh_mobile_ceu",
- .id = 1, /* "ceu1" clock */
+ .name = "renesas-ceu",
+ .id = 1, /* "ceu.1" clock */
.num_resources = ARRAY_SIZE(ceu1_resources),
.resource = ceu1_resources,
.dev= {
- .platform_data = &sh_mobile_ceu1_info,
+ .platform_data = &ceu1_pdata,
},
};
@@ -574,13 +574,16 @@ static struct platform_device vou_device = {
},
};
+static struct platform_device *ms7724se_ceu_devices[] __initdata = {
+ &ceu0_device,
+ &ceu1_device,
+};
+
static struct platform_device *ms7724se_devices[] __initdata = {
&heartbeat_device,
&smc91x_eth_device,
&lcdc_device,
&nor_flash_device,
- &ceu0_device,
- &ceu1_device,
&keysc_device,
&sh_eth_device,
&sh7724_usb0_host_device,
@@ -797,7 +800,6 @@ static int __init devices_setup(void)
gpio_request(GPIO_FN_VIO0_CLK, NULL);
gpio_request(GPIO_FN_VIO0_FLD, NULL);
gpio_request(GPIO_FN_VIO0_HD, NULL);
- platform_resource_setup_memory(&ceu0_device, "ceu0", 4 << 20);
/* enable CEU1 */
gpio_request(GPIO_FN_VIO1_D7, NULL);
@@ -81
[PATCH 5/5] arch: sh: ap325rxa: Use new renesas-ceu camera driver
Use the new renesas-ceu camera driver in ap325rxa board file instead of
the soc_camera based sh_mobile_ceu_camera driver.
Get rid of soc_camera specific components, and register CEU0 with a single
video sensor (ov7725).
Memory for the CEU video buffers is now reserved with membocks APIs
and need to be declared as dma_coherent during machine initialization to
remove that architecture specific part from CEU driver.
The ap325rxa board file registers another camera (ncm03j) for which I found no
driver in mainline kernel version, and that was configured/probed by sending
i2c messages (of 'magic blobs) from the board file itself. I removed the
sensor registration from this new version as it used soc_camera components
that will be later removed.
While at there update license to SPDX header and sort headers alphabetically.
Compile tested only.
Signed-off-by: Jacopo Mondi
---
arch/sh/boards/mach-ap325rxa/setup.c | 282 +
arch/sh/kernel/cpu/sh4a/clock-sh7723.c | 2 +-
2 files changed, 80 insertions(+), 204 deletions(-)
diff --git a/arch/sh/boards/mach-ap325rxa/setup.c
b/arch/sh/boards/mach-ap325rxa/setup.c
index de8393c..8f234d04 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -1,40 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Renesas - AP-325RXA
* (Compatible with Algo System ., LTD. - AP-320A)
*
* Copyright (C) 2008 Renesas Solutions Corp.
* Author : Yusuke Goda
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file "COPYING" in the main directory of this archive
- * for more details.
*/
-#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
#include
-#include
#include
-#include
+
+#include
#include
-#include
-#include
-#include
+
#include
-#include
-#include
-#include
-#include
+
+#define CEU_BUFFER_MEMORY_SIZE (4 << 20)
+static phys_addr_t ceu_dma_membase;
/* Dummy supplies, where voltage doesn't matter */
static struct regulator_consumer_supply dummy_supplies[] = {
@@ -253,150 +258,25 @@ static struct platform_device lcdc_device = {
},
};
-static void camera_power(int val)
-{
- gpio_set_value(GPIO_PTZ5, val); /* RST_CAM/RSTB */
- mdelay(10);
-}
-
-#ifdef CONFIG_I2C
-/* support for the old ncm03j camera */
-static unsigned char camera_ncm03j_magic[] =
-{
- 0x87, 0x00, 0x88, 0x08, 0x89, 0x01, 0x8A, 0xE8,
- 0x1D, 0x00, 0x1E, 0x8A, 0x21, 0x00, 0x33, 0x36,
- 0x36, 0x60, 0x37, 0x08, 0x3B, 0x31, 0x44, 0x0F,
- 0x46, 0xF0, 0x4B, 0x28, 0x4C, 0x21, 0x4D, 0x55,
- 0x4E, 0x1B, 0x4F, 0xC7, 0x50, 0xFC, 0x51, 0x12,
- 0x58, 0x02, 0x66, 0xC0, 0x67, 0x46, 0x6B, 0xA0,
- 0x6C, 0x34, 0x7E, 0x25, 0x7F, 0x25, 0x8D, 0x0F,
- 0x92, 0x40, 0x93, 0x04, 0x94, 0x26, 0x95, 0x0A,
- 0x99, 0x03, 0x9A, 0xF0, 0x9B, 0x14, 0x9D, 0x7A,
- 0xC5, 0x02, 0xD6, 0x07, 0x59, 0x00, 0x5A, 0x1A,
- 0x5B, 0x2A, 0x5C, 0x37, 0x5D, 0x42, 0x5E, 0x56,
- 0xC8, 0x00, 0xC9, 0x1A, 0xCA, 0x2A, 0xCB, 0x37,
- 0xCC, 0x42, 0xCD, 0x56, 0xCE, 0x00, 0xCF, 0x1A,
- 0xD0, 0x2A, 0xD1, 0x37, 0xD2, 0x42, 0xD3, 0x56,
- 0x5F, 0x68, 0x60, 0x87, 0x61, 0xA3, 0x62, 0xBC,
- 0x63, 0xD4, 0x64, 0xEA, 0xD6, 0x0F,
-};
-
-static int camera_probe(void)
-{
- struct i2c_adapter *a = i2c_get_adapter(0);
- struct i2c_msg msg;
- int ret;
-
- if (!a)
- return -ENODEV;
-
- camera_power(1);
- msg.addr = 0x6e;
- msg.buf = camera_ncm03j_magic;
- msg.len = 2;
- msg.flags = 0;
- ret = i2c_transfer(a, &msg, 1);
- camera_power(0);
-
- return ret;
-}
-
-static int camera_set_capture(struct soc_camera_platform_info *info,
- int enable)
-{
- struct i2c_adapter *a = i2c_get_adapter(0);
- struct i2c_msg msg;
- int ret = 0;
- int i;
-
- camera_power(0);
- if (!enable)
- return 0; /* no disable for now */
-
- camera_power(1);
- for (i = 0; i < ARRAY_SIZE(camera_ncm03j_magic); i += 2) {
- u_int8_t buf[8];
-
- msg.addr = 0x6e;
- msg.buf = buf;
- msg.len = 2;
- msg.flags = 0;
-
- buf[0] = camera_ncm03j_magic[i];
- buf[1] = camera_ncm03j_magic[i + 1];
-
- ret = (ret < 0) ? ret : i2c_transfer(a, &msg, 1);
- }
-
- return ret;
-}
-
-static int ap325rxa_camera_add(struct soc_camera_device *icd);
-static void ap325rxa_camera_del(struct soc_camera_device *icd);
-
-static struct soc_camera_platform_info camera_info = {
- .format_name = "UYVY",
- .format_depth = 16,
- .format =
Re: [PATCH 4/6] media: imx-csi: Enable interlaced scan for field type alternate
On 05/28/2018 12:59 AM, Ian Arkver wrote: On 28/05/18 08:00, Philipp Zabel wrote: On Fri, 2018-05-25 at 16:53 -0700, Steve Longerbeam wrote: Interlaced scan, a.k.a. interweave, should be enabled at the CSI IDMAC output pad if the input field type is 'alternate' (in addition to field types 'seq-tb' and 'seq-bt'). Which brings up whether V4L2_FIELD_HAS_BOTH() macro should be used to determine enabling interlaced/interweave scan. That macro includes the 'interlaced' field types, and in those cases the data is already interweaved with top/bottom field lines. A heads-up for now that this if statement may need to call V4L2_FIELD_IS_SEQUENTIAL() instead, I have no sensor hardware that sends 'interlaced' data, so can't test. I agree, the check here should be IS_SEQUENTIAL || ALTERNATE, and interlaced_scan should also be enabled if image.pix.field is INTERLACED_TB/BT. Yep, in fact I have implemented that in v2. Interlace_scan will be enabled only if input field type is SEQUENTIAL || ALTERNATE, and output field type is INTERLACED_TB/BT or non-qualified INTERLACED. And for INTERLACED_TB/BT input, the logic should be inverted: then we'd have to enable interlaced_scan whenever image.pix.field is SEQ_BT/TB. Hi Philipp, If your intent here is to de-interweave the two fields back to two sequential fields, I don't believe the IDMAC operation would achieve that. It's basically line stride doubling and a line offset for the lines in the 2nd half of the incoming frame, right? I agree, I'm not sure interlaced_scan will perform the inverse (turn an interlaced buffer into a sequential buffer). If the implementation involves a scan of two lines, second line with a memory offset equal to field size, and doubling the line stride to reach the next set of two lines, then running that operation on an interlaced buffer would not produce a sequential buffer. Steve Signed-off-by: Steve Longerbeam --- drivers/staging/media/imx/imx-media-csi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 9bc555c..eef3483 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -477,7 +477,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) ipu_smfc_set_burstsize(priv->smfc, burst_size); if (image.pix.field == V4L2_FIELD_NONE && - V4L2_FIELD_HAS_BOTH(infmt->field)) + (V4L2_FIELD_HAS_BOTH(infmt->field) || + infmt->field == V4L2_FIELD_ALTERNATE)) ipu_cpmem_interlaced_scan(priv->idmac_ch, image.pix.bytesperline);
[PATCH 2/5] media: i2c: rj54n1: Remove soc_camera dependencies
Remove soc_camera framework dependencies from rj54n1 sensor driver.
- Handle clock
- Handle GPIOs (named 'powerup' and 'enable')
- Register the async subdevice
- Remove g/s_mbus_config as they're deprecated.
- Adjust build system
- List the driver as maintained for 'Odd Fixes' as I don't have HW to test.
This commits does not remove the original soc_camera based driver.
Compiled tested only.
Signed-off-by: Jacopo Mondi
---
MAINTAINERS| 8 +++
drivers/media/i2c/Kconfig | 11 +++
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/rj54n1cb0c.c | 153 +++--
4 files changed, 107 insertions(+), 66 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index cbcd5ab..0dd7532 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12680,6 +12680,14 @@ W:
http://www.ibm.com/developerworks/linux/linux390/
S: Supported
F: net/smc/
+SHARP RJ54N1CB0C SENSOR DRIVER
+M: Jacopo Mondi
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Odd fixes
+F: drivers/media/i2c/rj54n1cb0c.c
+F: include/media/i2c/rj54n1cb0c.h
+
SH_VEU V4L2 MEM2MEM DRIVER
L: linux-media@vger.kernel.org
S: Orphan
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index b95b447..7b5a224 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -846,6 +846,17 @@ config VIDEO_NOON010PC30
source "drivers/media/i2c/m5mols/Kconfig"
+config VIDEO_RJ54N1
+ tristate "Sharp RJ54N1CB0C sensor support"
+ depends on I2C && VIDEO_V4L2
+ depends on MEDIA_CAMERA_SUPPORT
+ help
+ This is a V4L2 sensor-level driver for Sharp RJ54N1CB0C CMOS image
+ sensor.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rj54n1.
+
config VIDEO_S5K6AA
tristate "Samsung S5K6AAFX sensor support"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ff6e291..3f9c1f7 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_VIDEO_MT9V011) += mt9v011.o
obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
+obj-$(CONFIG_VIDEO_RJ54N1) += rj54n1cb0c.o
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o
obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o
diff --git a/drivers/media/i2c/rj54n1cb0c.c b/drivers/media/i2c/rj54n1cb0c.c
index 02398d0..6ad998a 100644
--- a/drivers/media/i2c/rj54n1cb0c.c
+++ b/drivers/media/i2c/rj54n1cb0c.c
@@ -1,25 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Driver for RJ54N1CB0C CMOS Image Sensor from Sharp
*
- * Copyright (C) 2009, Guennadi Liakhovetski
+ * Copyright (C) 2018, Jacopo Mondi
*
- * 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.
+ * Copyright (C) 2009, Guennadi Liakhovetski
*/
+#include
#include
+#include
#include
+#include
#include
#include
#include
-#include
#include
-#include
-#include
-#include
+#include
#include
+#include
#define RJ54N1_DEV_CODE0x0400
#define RJ54N1_DEV_CODE2 0x0401
@@ -151,7 +151,9 @@ struct rj54n1_clock_div {
struct rj54n1 {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
- struct v4l2_clk *clk;
+ struct clk *clk;
+ struct gpio_desc *pwup_gpio;
+ struct gpio_desc *enable_gpio;
struct rj54n1_clock_div clk_div;
const struct rj54n1_datafmt *fmt;
struct v4l2_rect rect; /* Sensor window */
@@ -545,8 +547,7 @@ static int rj54n1_set_selection(struct v4l2_subdev *sd,
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct rj54n1 *rj54n1 = to_rj54n1(client);
const struct v4l2_rect *rect = &sel->r;
- int dummy = 0, output_w, output_h,
- input_w = rect->width, input_h = rect->height;
+ int output_w, output_h, input_w = rect->width, input_h = rect->height;
int ret;
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE ||
@@ -554,11 +555,8 @@ static int rj54n1_set_selection(struct v4l2_subdev *sd,
return -EINVAL;
/* arbitrary minimum width and height, edges unimportant */
- soc_camera_limit_side(&dummy, &input_w,
-RJ54N1_COLUMN_SKIP, 8, RJ54N1_MAX_WIDTH);
-
- soc_camera_limit_side(&dummy, &input_h,
-RJ54N1_ROW_SKIP, 8, RJ54N1_MAX_HEIGHT);
+ v4l_bound_align_image(&input_w, 8, RJ54N1_MAX_WIDTH, 0,
+ &input_h, 8, RJ54N1_MAX_HEIGHT, 0, 0);
output_w = (input_w * 1024 + rj54n1->resize / 2) / rj54n1->resize;
output_h = (input_h * 1024 + rj54n1->resize / 2) / rj54n1->
[PATCH 1/5] media: i2c: Copy rj54n1cb0c soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory. This commit just copies the original file without modifying it. No modification to KConfig and Makefile as soc_camera framework dependencies need to be removed first in next commit. Signed-off-by: Jacopo Mondi --- drivers/media/i2c/rj54n1cb0c.c | 1416 1 file changed, 1416 insertions(+) create mode 100644 drivers/media/i2c/rj54n1cb0c.c diff --git a/drivers/media/i2c/rj54n1cb0c.c b/drivers/media/i2c/rj54n1cb0c.c new file mode 100644 index 000..02398d0 --- /dev/null +++ b/drivers/media/i2c/rj54n1cb0c.c @@ -0,0 +1,1416 @@ +/* + * Driver for RJ54N1CB0C CMOS Image Sensor from Sharp + * + * Copyright (C) 2009, Guennadi Liakhovetski + * + * 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. + */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#define RJ54N1_DEV_CODE0x0400 +#define RJ54N1_DEV_CODE2 0x0401 +#define RJ54N1_OUT_SEL 0x0403 +#define RJ54N1_XY_OUTPUT_SIZE_S_H 0x0404 +#define RJ54N1_X_OUTPUT_SIZE_S_L 0x0405 +#define RJ54N1_Y_OUTPUT_SIZE_S_L 0x0406 +#define RJ54N1_XY_OUTPUT_SIZE_P_H 0x0407 +#define RJ54N1_X_OUTPUT_SIZE_P_L 0x0408 +#define RJ54N1_Y_OUTPUT_SIZE_P_L 0x0409 +#define RJ54N1_LINE_LENGTH_PCK_S_H 0x040a +#define RJ54N1_LINE_LENGTH_PCK_S_L 0x040b +#define RJ54N1_LINE_LENGTH_PCK_P_H 0x040c +#define RJ54N1_LINE_LENGTH_PCK_P_L 0x040d +#define RJ54N1_RESIZE_N0x040e +#define RJ54N1_RESIZE_N_STEP 0x040f +#define RJ54N1_RESIZE_STEP 0x0410 +#define RJ54N1_RESIZE_HOLD_H 0x0411 +#define RJ54N1_RESIZE_HOLD_L 0x0412 +#define RJ54N1_H_OBEN_OFS 0x0413 +#define RJ54N1_V_OBEN_OFS 0x0414 +#define RJ54N1_RESIZE_CONTROL 0x0415 +#define RJ54N1_STILL_CONTROL 0x0417 +#define RJ54N1_INC_USE_SEL_H 0x0425 +#define RJ54N1_INC_USE_SEL_L 0x0426 +#define RJ54N1_MIRROR_STILL_MODE 0x0427 +#define RJ54N1_INIT_START 0x0428 +#define RJ54N1_SCALE_1_2_LEV 0x0429 +#define RJ54N1_SCALE_4_LEV 0x042a +#define RJ54N1_Y_GAIN 0x04d8 +#define RJ54N1_APT_GAIN_UP 0x04fa +#define RJ54N1_RA_SEL_UL 0x0530 +#define RJ54N1_BYTE_SWAP 0x0531 +#define RJ54N1_OUT_SIGPO 0x053b +#define RJ54N1_WB_SEL_WEIGHT_I 0x054e +#define RJ54N1_BIT8_WB 0x0569 +#define RJ54N1_HCAPS_WB0x056a +#define RJ54N1_VCAPS_WB0x056b +#define RJ54N1_HCAPE_WB0x056c +#define RJ54N1_VCAPE_WB0x056d +#define RJ54N1_EXPOSURE_CONTROL0x058c +#define RJ54N1_FRAME_LENGTH_S_H0x0595 +#define RJ54N1_FRAME_LENGTH_S_L0x0596 +#define RJ54N1_FRAME_LENGTH_P_H0x0597 +#define RJ54N1_FRAME_LENGTH_P_L0x0598 +#define RJ54N1_PEAK_H 0x05b7 +#define RJ54N1_PEAK_50 0x05b8 +#define RJ54N1_PEAK_60 0x05b9 +#define RJ54N1_PEAK_DIFF 0x05ba +#define RJ54N1_IOC 0x05ef +#define RJ54N1_TG_BYPASS 0x0700 +#define RJ54N1_PLL_L 0x0701 +#define RJ54N1_PLL_N 0x0702 +#define RJ54N1_PLL_EN 0x0704 +#define RJ54N1_RATIO_TG0x0706 +#define RJ54N1_RATIO_T 0x0707 +#define RJ54N1_RATIO_R 0x0708 +#define RJ54N1_RAMP_TGCLK_EN 0x0709 +#define RJ54N1_OCLK_DSP0x0710 +#define RJ54N1_RATIO_OP0x0711 +#define RJ54N1_RATIO_O 0x0712 +#define RJ54N1_OCLK_SEL_EN 0x0713 +#define RJ54N1_CLK_RST 0x0717 +#define RJ54N1_RESET_STANDBY 0x0718 +#define RJ54N1_FWFLG 0x07fe + +#define E_EXCLK(1 << 7) +#define SOFT_STDBY (1 << 4) +#define SEN_RSTX (1 << 2) +#define TG_RSTX(1 << 1) +#define DSP_RSTX (1 << 0) + +#define RESIZE_HOLD_SEL(1 << 2) +#define RESIZE_GO (1 << 1) + +/* + * When cropping, the camera automatically centers the cropped region, there + * doesn't seem to be a way to specify an explicit location of the rectangle. + */ +#define RJ54N1_COLUMN_SKIP 0 +#define RJ54N1_ROW_SKIP0 +#define RJ54N1_MAX_WIDTH 1600 +#define RJ54N1_MAX_HEIGHT 1200 + +#define PLL_L 2 +#define PLL_N
[PATCH] media: marvel-ccic: mmp: select VIDEOBUF2_VMALLOC/DMA_CONTIG
Testing randconfig builds after the return of the mmp ccic driver shows
a link error in some configurations:
drivers/media/platform/marvell-ccic/mcam-core.o: In function `mccic_register':
mcam-core.c:(.text+0x2e48): undefined reference to `vb2_dma_contig_memops'
A closer look at the mcam-core.c file reveals that we need to select
both VIDEOBUF2_DMA_CONTIG and VIDEOBUF2_VMALLOC, as already do for
VIDEO_CAFE_CCIC.
Fixes: 0a9c643c8faa ("media: marvel-ccic: re-enable mmp-driver build")
Signed-off-by: Arnd Bergmann
---
drivers/media/platform/marvell-ccic/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/marvell-ccic/Kconfig
b/drivers/media/platform/marvell-ccic/Kconfig
index 21dacef7c2fc..13cc6f2159d3 100644
--- a/drivers/media/platform/marvell-ccic/Kconfig
+++ b/drivers/media/platform/marvell-ccic/Kconfig
@@ -18,6 +18,8 @@ config VIDEO_MMP_CAMERA
depends on ARCH_MMP || COMPILE_TEST
select VIDEO_OV7670
select I2C_GPIO
+ select VIDEOBUF2_VMALLOC
+ select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_DMA_SG
---help---
This is a Video4Linux2 driver for the integrated camera
--
2.9.0
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Sat, May 26, 2018 at 08:28:18AM -0300, Mauro Carvalho Chehab wrote:
> Em Sat, 26 May 2018 03:24:00 +0300
> Laurent Pinchart escreveu:
>
> > Hi Mauro,
> >
> > On Saturday, 26 May 2018 02:39:16 EEST Laurent Pinchart wrote:
> > > On Saturday, 26 May 2018 02:10:27 EEST Mauro Carvalho Chehab wrote:
> > > > Em Sun, 20 May 2018 15:10:50 +0300 Laurent Pinchart escreveu:
> > > >> Hi Mauro,
> > > >>
> > > >> The following changes since commit
> > > >>
> > > >> 8ed8bba70b4355b1ba029b151ade84475dd12991:
> > > >> media: imx274: remove non-indexed pointers from mode_table
> > > >> (2018-05-17
> > > >>
> > > >> 06:22:08 -0400)
> > > >>
> > > >> are available in the Git repository at:
> > > >> git://linuxtv.org/pinchartl/media.git v4l2/vsp1/next
> > > >>
> > > >> for you to fetch changes up to
> > > >> 429f256501652c90a4ed82f2416618f82a77d37c:
> > > >> media: vsp1: Move video configuration to a cached dlb (2018-05-20
> > > >> 09:46:51 +0300)
> > > >>
> > > >> The branch passes the VSP and DU test suites, both on its own and when
> > > >> merged with the drm-next branch.
> > > >
> > > > This series added a new warning:
> > > >
> > > > drivers/media/platform/vsp1/vsp1_dl.c:69: warning: Function parameter or
> > > > member 'refcnt' not described in 'vsp1_dl_body'
> > >
> > > We'll fix that. Kieran, as you authored the code, would you like to give
> > > it
> > > a go ?
> > >
> > > > To the already existing one:
> > > >
> > > > drivers/media/platform/vsp1/vsp1_drm.c:336 vsp1_du_pipeline_setup_brx()
> > > > error: we previously assumed 'pipe->brx' could be null (see line 244)
> > >
> > > That's still on my todo list. I tried to give it a go but received plenty
> > > of
> > > SQL errors. How do you run smatch ?
> >
> > Nevermind, I found out what was wrong (had to specify the data directory
> > manually).
> >
> > I've reproduced the issue and created a minimal test case.
> >
> > 1. struct vsp1_pipeline;
> > 2.
> > 3. struct vsp1_entity {
> > 4. struct vsp1_pipeline *pipe;
> > 5. struct vsp1_entity *sink;
> > 6. unsigned int source_pad;
> > 7. };
> > 8.
> > 9. struct vsp1_pipeline {
> > 10. struct vsp1_entity *brx;
> > 11. };
> > 12.
> > 13. struct vsp1_brx {
> > 14. struct vsp1_entity entity;
> > 15. };
> > 16.
> > 17. struct vsp1_device {
> > 18. struct vsp1_brx *bru;
> > 19. struct vsp1_brx *brs;
> > 20. };
> > 21.
> > 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline *pipe)
> > 23. {
> > 24. struct vsp1_entity *brx;
> > 25.
> > 26. if (pipe->brx)
> > 27. brx = pipe->brx;
> > 28. else if (!vsp1->bru->entity.pipe)
> > 29. brx = &vsp1->bru->entity;
> > 30. else
> > 31. brx = &vsp1->brs->entity;
> > 32.
> > 33. if (brx != pipe->brx)
> > 34. pipe->brx = brx;
> > 35.
> > 36. return pipe->brx->source_pad;
> > 37. }
> >
> > The reason why smatch complains is that it has no guarantee that vsp1->brs
> > is
> > not NULL. It's quite tricky:
> >
> > - On line 26, smatch assumes that pipe->brx can be NULL
> > - On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL
> > due
> > to line 26)
> > - On line 28, smatch assumes that vsp1->bru is not NULL
> > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > due
> > to line 28)
> > - On line 31, brx is assigned a possibly NULL value (as there's no
> > information
> > regarding vsp1->brs)
> > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > - On line 36 pipe->brx is dereferenced
> >
> > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > NULL.
> > Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the warning
> > disappear.
> >
I will respond to the other emails in this thread. You guys are
basically spot on. All this analysis is 100% correct. Btw, if you want
to see Smatch's internal state you can do:
#include "/home/whatever/smatch/check_debug.h"
else if (!vsp1->bru->entity.pipe) {
__smatch_implied(&vsp1->bru->entity);
And it tells you what Smatch thinks it is at that point. The
__smatch_about() output can also be useful.
regards,
dan carpenter
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Em Mon, 28 May 2018 11:31:01 +0300
Laurent Pinchart escreveu:
> Hi Mauro and Dan,
>
> Dan, there's a question for you below.
>
> On Monday, 28 May 2018 11:28:41 EEST Laurent Pinchart wrote:
> > On Saturday, 26 May 2018 14:28:18 EEST Mauro Carvalho Chehab wrote:
> > > Em Sat, 26 May 2018 03:24:00 +0300 Laurent Pinchart escreveu:
> > [snip]
> >
> > > > I've reproduced the issue and created a minimal test case.
> > > >
> > > > 1. struct vsp1_pipeline;
> > > > 2.
> > > > 3. struct vsp1_entity {
> > > > 4. struct vsp1_pipeline *pipe;
> > > > 5. struct vsp1_entity *sink;
> > > > 6. unsigned int source_pad;
> > > > 7. };
> > > > 8.
> > > > 9. struct vsp1_pipeline {
> > > > 10. struct vsp1_entity *brx;
> > > > 11. };
> > > > 12.
> > > > 13. struct vsp1_brx {
> > > > 14. struct vsp1_entity entity;
> > > > 15. };
> > > > 16.
> > > > 17. struct vsp1_device {
> > > > 18. struct vsp1_brx *bru;
> > > > 19. struct vsp1_brx *brs;
> > > > 20. };
> > > > 21.
> > > > 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline
> > > > *pipe)
> > > > 23. {
> > > > 24. struct vsp1_entity *brx;
> > > > 25.
> > > > 26. if (pipe->brx)
> > > > 27. brx = pipe->brx;
> > > > 28. else if (!vsp1->bru->entity.pipe)
> > > > 29. brx = &vsp1->bru->entity;
> > > > 30. else
> > > > 31. brx = &vsp1->brs->entity;
> > > > 32.
> > > > 33. if (brx != pipe->brx)
> > > > 34. pipe->brx = brx;
> > > > 35.
> > > > 36. return pipe->brx->source_pad;
> > > > 37. }
> > > >
> > > > The reason why smatch complains is that it has no guarantee that
> > > > vsp1->brs is not NULL. It's quite tricky:
> > > >
> > > > - On line 26, smatch assumes that pipe->brx can be NULL
> > > > - On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL
> > > > due to line 26)
> > > > - On line 28, smatch assumes that vsp1->bru is not NULL
> > > > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > > > due to line 28)
> > > > - On line 31, brx is assigned a possibly NULL value (as there's no
> > > > information regarding vsp1->brs)
> > > > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > > > - On line 36 pipe->brx is dereferenced
> > > >
> > > > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > > > NULL. Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the
> > > > warning disappear.
> > > >
> > > > So how do we know that vsp1->brs isn't NULL in the original code ?
> > > >
> > > > if (pipe->num_inputs > 2)
> > > > brx = &vsp1->bru->entity;
> > > > else if (pipe->brx && !drm_pipe->force_brx_release)
> > > > brx = pipe->brx;
> > > > else if (!vsp1->bru->entity.pipe)
> > > > brx = &vsp1->bru->entity;
> > > > else
> > > > brx = &vsp1->brs->entity;
> > > >
> > > > A VSP1 instance can have no brs, so in general vsp1->brs can be NULL.
> > > > However, when that's the case, the following conditions are fulfilled.
> > > >
> > > > - drm_pipe->force_brx_release will be false
> > > > - either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be
> > > > NULL
> > > >
> > > > The fourth branch should thus never be taken.
> > >
> > > I don't think that adding a forth branch there would solve.
> > >
> > > The thing is that Smatch knows that pipe->brx can be NULL, as the function
> > > explicly checks if pipe->brx != NULL.
> > >
> > > When Smatch handles this if:
> > > if (brx != pipe->brx) {
> > >
> > > It wrongly assumes that this could be false if pipe->brx is NULL.
> > > I don't know why, as Smatch should know that brx can't be NULL.
> >
> > brx can be NULL here if an only if vsp1->brs is NULL (as the entity field is
> > first in the vsp1->brs structure, so &vsp1->brs->entity has the same
> > address as vsp1->brs).
> >
> > vsp1->brs can be NULL on some devices, but in that case we have the
> > following guarantees:
> >
> > - drm_pipe->force_brx_release will always be FALSE
> > - either pipe->brx will be non-NULL or vsp1->bru->entity.pipe will be NULL
> >
> > So the fourth branch is never taken.
> >
> > The above conditions come from outside this function, and smatch can't know
> > about them. However, I don't know whether the problems comes from smatch
> > assuming that vsp1->brs can be NULL, or from somewhere else.
> >
> > > On such case, the next code to be executed would be:
> > > format.pad = pipe->brx->source_pad;
> > >
> > > With would be trying to de-ref a NULL pointer.
> > >
> > > There are two ways to fix it:
> > >
> > > 1) with my patch.
> > >
> > > It is based to the fact that, if pipe->brx is null, then brx won't be
> > > NULL. So, the logic that "Switch BRx if needed." will always be called:
> > >
> > > diff --git a/drivers/media/platform/vsp1/vsp
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 11:28:41AM +0300, Laurent Pinchart wrote:
> and I still get the same warning. I had to write the following (which is
> obviously not correct) to silence the warning.
>
> if (pipe->num_inputs > 2)
> brx = &vsp1->bru->entity;
> else if (pipe->brx)
> brx = pipe->brx;
> else if (!vsp1->bru->entity.pipe)
> brx = &vsp1->bru->entity;
> else {
> (void)vsp1->brs->entity;
> brx = &vsp1->brs->entity;
> }
>
> Both the (void)vsp1->brs->entity and the removal of the !drm_pipe-
> >force_brx_release were needed, any of those on its own didn't fix the
> problem.
The problem in this case is the first "brx = &vsp1->bru->entity;".
Smatch assumes &vsp1->bru->entity can be == to pipe->brx and NULL.
Adding a "(void)vsp1->bru->entity;" on that path will silence the
warning (hopefully).
regards,
dan carpenter
[PATCH 1/1] imx258: Check the rotation property has a value of 180
The driver only supports streaming images flipped horizontally and vertically. In order to ensure that all current users will be fine if or when support for upright streaming is added, require the presence of the "rotation" control now. Signed-off-by: Sakari Ailus Tested-by: "Lai, Jim" --- drivers/media/i2c/imx258.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index fad3012f4fe5..0383394f5676 100644 --- a/drivers/media/i2c/imx258.c +++ b/drivers/media/i2c/imx258.c @@ -1223,6 +1223,14 @@ static int imx258_probe(struct i2c_client *client) if (val != 1920) return -EINVAL; + /* +* Check that the device is mounted upside down. The driver only +* supports a single pixel order right now. +*/ + ret = device_property_read_u32(&client->dev, "rotation", &val); + if (ret || val != 180) + return -EINVAL; + imx258 = devm_kzalloc(&client->dev, sizeof(*imx258), GFP_KERNEL); if (!imx258) return -ENOMEM; -- 2.11.0
[PATCH] media: vsp1: Document vsp1_dl_body refcnt
In commit 2d9445db0ee9 ("media: vsp1: Use reference counting for
bodies"), a new field was introduced to the vsp1_dl_body structure to
account for usage tracking of the body.
Document the newly added field in the kerneldoc.
Signed-off-by: Kieran Bingham
---
drivers/media/platform/vsp1/vsp1_dl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
b/drivers/media/platform/vsp1/vsp1_dl.c
index d9b9cdd8fbe2..10a24bde2299 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -43,6 +43,7 @@ struct vsp1_dl_entry {
* struct vsp1_dl_body - Display list body
* @list: entry in the display list list of bodies
* @free: entry in the pool free body list
+ * @refcnt: reference tracking for the body
* @pool: pool to which this body belongs
* @vsp1: the VSP1 device
* @entries: array of entries
--
2.17.0
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Mauro, Laurent, On 26/05/18 00:39, Laurent Pinchart wrote: > Hi Mauro, > > (CC'ing Kieran) > > On Saturday, 26 May 2018 02:10:27 EEST Mauro Carvalho Chehab wrote: >> Em Sun, 20 May 2018 15:10:50 +0300 Laurent Pinchart escreveu: >>> Hi Mauro, >>> >>> The following changes since commit >>> 8ed8bba70b4355b1ba029b151ade84475dd12991: >>> media: imx274: remove non-indexed pointers from mode_table (2018-05-17 >>> 06:22:08 -0400) >>> >>> are available in the Git repository at: >>> git://linuxtv.org/pinchartl/media.git v4l2/vsp1/next >>> >>> for you to fetch changes up to 429f256501652c90a4ed82f2416618f82a77d37c: >>> media: vsp1: Move video configuration to a cached dlb (2018-05-20 >>> 09:46:51 +0300) >>> >>> The branch passes the VSP and DU test suites, both on its own and when >>> merged with the drm-next branch. >> >> This series added a new warning: >> >> drivers/media/platform/vsp1/vsp1_dl.c:69: warning: Function parameter or >> member 'refcnt' not described in 'vsp1_dl_body' > > We'll fix that. Kieran, as you authored the code, would you like to give it a > go ? Sent! Thanks for the catch. -- Kieran >> To the already existing one: >> >> drivers/media/platform/vsp1/vsp1_drm.c:336 vsp1_du_pipeline_setup_brx() >> error: we previously assumed 'pipe->brx' could be null (see line 244) > > That's still on my todo list. I tried to give it a go but received plenty of > SQL errors. How do you run smatch ? > >> (there's also a Spectre warning too, but I'll looking into those >> in separate). >> >> For now, I'll apply it, but I reserve the right of not pulling any >> new patchsets that would add more warnings. >> >>> >>> >>> Geert Uytterhoeven (1): >>> media: vsp1: Drop OF dependency of VIDEO_RENESAS_VSP1 >>> >>> Kieran Bingham (10): >>> media: vsp1: Release buffers for each video node >>> media: vsp1: Move video suspend resume handling to video object >>> media: vsp1: Reword uses of 'fragment' as 'body' >>> media: vsp1: Protect bodies against overflow >>> media: vsp1: Provide a body pool >>> media: vsp1: Convert display lists to use new body pool >>> media: vsp1: Use reference counting for bodies >>> media: vsp1: Refactor display list configure operations >>> media: vsp1: Adapt entities to configure into a body >>> media: vsp1: Move video configuration to a cached dlb >>> >>> drivers/media/platform/Kconfig| 2 +- >>> drivers/media/platform/vsp1/vsp1_brx.c| 32 ++-- >>> drivers/media/platform/vsp1/vsp1_clu.c| 113 ++- >>> drivers/media/platform/vsp1/vsp1_clu.h| 1 + >>> drivers/media/platform/vsp1/vsp1_dl.c | 388 ++--- >>> drivers/media/platform/vsp1/vsp1_dl.h | 21 ++- >>> drivers/media/platform/vsp1/vsp1_drm.c| 18 +- >>> drivers/media/platform/vsp1/vsp1_drv.c| 4 +- >>> drivers/media/platform/vsp1/vsp1_entity.c | 34 +++- >>> drivers/media/platform/vsp1/vsp1_entity.h | 45 +++-- >>> drivers/media/platform/vsp1/vsp1_hgo.c| 26 ++- >>> drivers/media/platform/vsp1/vsp1_hgt.c| 28 ++- >>> drivers/media/platform/vsp1/vsp1_hsit.c | 20 +- >>> drivers/media/platform/vsp1/vsp1_lif.c| 25 ++- >>> drivers/media/platform/vsp1/vsp1_lut.c| 80 +--- >>> drivers/media/platform/vsp1/vsp1_lut.h| 1 + >>> drivers/media/platform/vsp1/vsp1_pipe.c | 74 +--- >>> drivers/media/platform/vsp1/vsp1_pipe.h | 12 +- >>> drivers/media/platform/vsp1/vsp1_rpf.c| 189 ++- >>> drivers/media/platform/vsp1/vsp1_sru.c| 24 +-- >>> drivers/media/platform/vsp1/vsp1_uds.c| 73 +++ >>> drivers/media/platform/vsp1/vsp1_uds.h| 2 +- >>> drivers/media/platform/vsp1/vsp1_uif.c| 35 ++-- >>> drivers/media/platform/vsp1/vsp1_video.c | 177 - >>> drivers/media/platform/vsp1/vsp1_video.h | 3 + >>> drivers/media/platform/vsp1/vsp1_wpf.c| 326 ++--- >>> 26 files changed, 967 insertions(+), 786 deletions(-) >
[PATCH] media: dvb: get rid of VIDEO_SET_SPU_PALETTE
No upstream drivers use it. It doesn't make any sense to have
a compat32 code for something that nobody uses upstream.
Reported-by: Alexander Viro
Signed-off-by: Mauro Carvalho Chehab
---
.../media/uapi/dvb/video-set-spu-palette.rst | 82 ---
.../media/uapi/dvb/video_function_calls.rst | 1 -
Documentation/media/uapi/dvb/video_types.rst | 18
Documentation/media/video.h.rst.exceptions| 1 -
fs/compat_ioctl.c | 2 -
include/uapi/linux/dvb/video.h| 7 --
6 files changed, 111 deletions(-)
delete mode 100644 Documentation/media/uapi/dvb/video-set-spu-palette.rst
diff --git a/Documentation/media/uapi/dvb/video-set-spu-palette.rst
b/Documentation/media/uapi/dvb/video-set-spu-palette.rst
deleted file mode 100644
index 51a1913d21d2..
--- a/Documentation/media/uapi/dvb/video-set-spu-palette.rst
+++ /dev/null
@@ -1,82 +0,0 @@
-.. -*- coding: utf-8; mode: rst -*-
-
-.. _VIDEO_SET_SPU_PALETTE:
-
-=
-VIDEO_SET_SPU_PALETTE
-=
-
-Name
-
-
-VIDEO_SET_SPU_PALETTE
-
-.. attention:: This ioctl is deprecated.
-
-Synopsis
-
-
-.. c:function:: int ioctl(fd, VIDEO_SET_SPU_PALETTE, struct video_spu_palette
*palette )
-:name: VIDEO_SET_SPU_PALETTE
-
-
-Arguments
--
-
-.. flat-table::
-:header-rows: 0
-:stub-columns: 0
-
-
-- .. row 1
-
- - int fd
-
- - File descriptor returned by a previous call to open().
-
-- .. row 2
-
- - int request
-
- - Equals VIDEO_SET_SPU_PALETTE for this command.
-
-- .. row 3
-
- - video_spu_palette_t \*palette
-
- - SPU palette according to section ??.
-
-
-Description
-
-This ioctl sets the SPU color palette.
-
-.. c:type:: video_spu_palette
-
-.. code-block::c
-
- typedef struct video_spu_palette { /* SPU Palette information */
- int length;
- __u8 __user *palette;
- } video_spu_palette_t;
-
-Return Value
-
-
-On success 0 is returned, on error -1 and the ``errno`` variable is set
-appropriately. The generic error codes are described at the
-:ref:`Generic Error Codes ` chapter.
-
-
-
-.. flat-table::
-:header-rows: 0
-:stub-columns: 0
-
-
-- .. row 1
-
- - ``EINVAL``
-
- - input is not a valid palette or driver doesn’t handle SPU.
diff --git a/Documentation/media/uapi/dvb/video_function_calls.rst
b/Documentation/media/uapi/dvb/video_function_calls.rst
index 68588ac7fecb..8d8383ffaeba 100644
--- a/Documentation/media/uapi/dvb/video_function_calls.rst
+++ b/Documentation/media/uapi/dvb/video_function_calls.rst
@@ -38,6 +38,5 @@ Video Function Calls
video-set-system
video-set-highlight
video-set-spu
-video-set-spu-palette
video-get-navi
video-set-attributes
diff --git a/Documentation/media/uapi/dvb/video_types.rst
b/Documentation/media/uapi/dvb/video_types.rst
index 640a21de6b8a..4cfa00e5c934 100644
--- a/Documentation/media/uapi/dvb/video_types.rst
+++ b/Documentation/media/uapi/dvb/video_types.rst
@@ -320,24 +320,6 @@ to the following format:
} video_spu_t;
-.. c:type:: video_spu_palette
-
-struct video_spu_palette
-
-
-The following structure is used to set the SPU palette by calling
-VIDEO_SPU_PALETTE:
-
-
-.. code-block:: c
-
- typedef
- struct video_spu_palette {
-int length;
-uint8_t *palette;
- } video_spu_palette_t;
-
-
.. c:type:: video_navi_pack
struct video_navi_pack
diff --git a/Documentation/media/video.h.rst.exceptions
b/Documentation/media/video.h.rst.exceptions
index a91aa884ce0e..89d7c3ef2da7 100644
--- a/Documentation/media/video.h.rst.exceptions
+++ b/Documentation/media/video.h.rst.exceptions
@@ -36,5 +36,4 @@ replace typedef video_stream_source_t
:c:type:`video_stream_source`
replace typedef video_play_state_t :c:type:`video_play_state`
replace typedef video_highlight_t :c:type:`video_highlight`
replace typedef video_spu_t :c:type:`video_spu`
-replace typedef video_spu_palette_t :c:type:`video_spu_palette`
replace typedef video_navi_pack_t :c:type:`video_navi_pack`
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index ef80085ed564..e78ecda283d2 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1349,8 +1349,6 @@ static long do_ioctl_trans(unsigned int cmd,
return do_video_get_event(file, cmd, argp);
case VIDEO_STILLPICTURE:
return do_video_stillpicture(file, cmd, argp);
- case VIDEO_SET_SPU_PALETTE:
- return do_video_set_spu_palette(file, cmd, argp);
}
/*
diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
index df3d7028c807..6a0c9757b7ba 100644
--- a/include/uapi/linux/dvb/video.h
+++ b/include/uapi/linux/dvb/video.h
@@ -186,12 +186,6 @@ typedef struct video_spu {
} video_spu_t;
-typedef struct video_spu_palette { /* SPU Palette informatio
Re: [ANN] Meeting to discuss improvements to support MC-based cameras on generic apps
Hi Mauro, On 2018-05-28 10:43:51 -0300, Mauro Carvalho Chehab wrote: > Em Thu, 17 May 2018 16:07:08 -0300 > Mauro Carvalho Chehab escreveu: > > > Hi all, > > > > The goal of this e-mail is to schedule a meeting in order to discuss > > improvements at the media subsystem in order to support complex camera > > hardware by usual apps. > > > > The main focus here is to allow supporting devices with MC-based > > hardware connected to a camera. > > > > In short, my proposal is to meet with the interested parties on solving > > this issue during the Open Source Summit in Japan, e. g. between > > June, 19-22, in Tokyo. > > Let's schedule the meeting to happen in Tokyo, Japan at June, 19. I will be in Tokyo at that time and would be happy to join. > > Location yet to be defined, but it will either be together with > OSS Japan or at Google. I'll confirm the address tomorrow. > > Regards, > Mauro > > > > Thanks, > Mauro -- Regards, Niklas Söderlund
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 11:31:01AM +0300, Laurent Pinchart wrote: > And that being said, I just tried > > if (pipe->num_inputs > 2) > brx = &vsp1->bru->entity; > else if (pipe->brx && !drm_pipe->force_brx_release) > brx = pipe->brx; > else if (!vsp1->bru->entity.pipe) > brx = &vsp1->bru->entity; > else > brx = &vsp1->brs->entity; > > if (!brx) > return -EINVAL; > > and that didn't help either... Dan, would you have some light to shed on this > problem ? This is a problem in Smatch. We should be able to go backwards and say that "If we know 'brx' is non-NULL then let's mark &vsp1->brs->entity, vsp1->brs, &vsp1->bru->entity and vsp1->bru all as non-NULL as well". But Smatch doesn't go backwards like that. The information is mostly there to do it, but my instinct is that it's really hard to implement. The other potential problem here is that Smatch stores comparisons and values separately. In other words smatch_comparison.c has all the information about brx == &vsp1->bru->entity and smatch_extra.c has the information about if brx is NULL or non-NULL. They don't really share information very well. regards, dan carpenter
Re: [PATCH v4 5/9] media: rcar-vin: Parse parallel input on Gen3
Hi Jacopo,
On 2018-05-25 13:50:08 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>I might have another question before sending v5.
>
> On Fri, May 25, 2018 at 12:29:44AM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > I really like what you did with this patch in v4.
> >
> > On 2018-05-25 00:02:15 +0200, Jacopo Mondi wrote:
> > > The rcar-vin driver so far had a mutually exclusive code path for
> > > handling parallel and CSI-2 video input subdevices, with only the CSI-2
> > > use case supporting media-controller. As we add support for parallel
> > > inputs to Gen3 media-controller compliant code path now parse both port@0
> > > and port@1, handling the media-controller use case in the parallel
> > > bound/unbind notifier operations.
> > >
> > > Signed-off-by: Jacopo Mondi
> > >
> > > ---
> > > v3 -> v4:
> > > - Change the mc/parallel initialization order. Initialize mc first, then
> > > parallel
> > > - As a consequence no need to delay parallel notifiers registration, the
> > > media controller is set up already when parallel input got parsed,
> > > this greatly simplify the group notifier complete callback.
> > > ---
> > > drivers/media/platform/rcar-vin/rcar-core.c | 56
> > > ++---
> > > 1 file changed, 35 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
> > > b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index a799684..29619c2 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct
> > > rvin_dev *vin,
> > > ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> > > vin->parallel->sink_pad = ret < 0 ? 0 : ret;
> > >
> > > + if (vin->info->use_mc) {
> > > + vin->parallel->subdev = subdev;
> > > + return 0;
> > > + }
> > > +
> > > /* Find compatible subdevices mbus format */
> > > vin->mbus_code = 0;
> > > code.index = 0;
> > > @@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct
> > > rvin_dev *vin,
> > > static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
> > > {
> > > rvin_v4l2_unregister(vin);
> > > - v4l2_ctrl_handler_free(&vin->ctrl_handler);
> > > -
> > > - vin->vdev.ctrl_handler = NULL;
> > > vin->parallel->subdev = NULL;
> > > +
> > > + if (!vin->info->use_mc) {
> > > + v4l2_ctrl_handler_free(&vin->ctrl_handler);
> > > + vin->vdev.ctrl_handler = NULL;
> > > + }
> > > }
> > >
> > > static int rvin_parallel_notify_complete(struct v4l2_async_notifier
> > > *notifier)
> > > @@ -552,18 +559,18 @@ static int rvin_parallel_parse_v4l2(struct device
> > > *dev,
> > > return 0;
> > > }
> > >
> > > -static int rvin_parallel_graph_init(struct rvin_dev *vin)
> > > +static int rvin_parallel_init(struct rvin_dev *vin)
> > > {
> > > int ret;
> > >
> > > - ret = v4l2_async_notifier_parse_fwnode_endpoints(
> > > - vin->dev, &vin->notifier,
> > > - sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
> > > + ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> > > + vin->dev, &vin->notifier, sizeof(struct rvin_parallel_entity),
> > > + 0, rvin_parallel_parse_v4l2);
> > > if (ret)
> > > return ret;
> > >
> > > if (!vin->parallel)
> > > - return -ENODEV;
> > > + return -ENOTCONN;
> >
> > I think you still should return -ENODEV here if !vin->info->use_mc to
> > preserve Gen2 which runs without media controller behavior. How about:
> >
> > return vin->info->use_mc ? -ENOTCONN : -ENODEV;
> >
> > >
> > > vin_dbg(vin, "Found parallel subdevice %pOF\n",
> > > to_of_node(vin->parallel->asd.match.fwnode));
> > > @@ -784,14 +791,8 @@ static int rvin_mc_init(struct rvin_dev *vin)
> > > {
> > > int ret;
> > >
> > > - vin->pad.flags = MEDIA_PAD_FL_SINK;
> > > - ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = rvin_group_get(vin);
> > > - if (ret)
> > > - return ret;
> > > + if (!vin->info->use_mc)
> > > + return 0;
> > >
> > > ret = rvin_mc_parse_of_graph(vin);
> > > if (ret)
> > > @@ -1074,11 +1075,24 @@ static int rcar_vin_probe(struct platform_device
> > > *pdev)
> > > return ret;
> > >
> > > platform_set_drvdata(pdev, vin);
> > > - if (vin->info->use_mc)
> > > - ret = rvin_mc_init(vin);
> > > - else
> > > - ret = rvin_parallel_graph_init(vin);
> > > - if (ret < 0)
> > > +
> > > + if (vin->info->use_mc) {
> > > + vin->pad.flags = MEDIA_PAD_FL_SINK;
> > > + ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = rvin_group_get(vin);
> > > + if (ret)
> > > + return ret;
> > > + }
> >
> > I don't see why you need to move
Re: [ANN] Meeting to discuss improvements to support MC-based cameras on generic apps
Em Thu, 17 May 2018 16:07:08 -0300 Mauro Carvalho Chehab escreveu: > Hi all, > > The goal of this e-mail is to schedule a meeting in order to discuss > improvements at the media subsystem in order to support complex camera > hardware by usual apps. > > The main focus here is to allow supporting devices with MC-based > hardware connected to a camera. > > In short, my proposal is to meet with the interested parties on solving > this issue during the Open Source Summit in Japan, e. g. between > June, 19-22, in Tokyo. Let's schedule the meeting to happen in Tokyo, Japan at June, 19. Location yet to be defined, but it will either be together with OSS Japan or at Google. I'll confirm the address tomorrow. Regards, Mauro Thanks, Mauro
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Mauro,
On Monday, 28 May 2018 13:03:05 EEST Mauro Carvalho Chehab wrote:
> Em Mon, 28 May 2018 11:28:41 +0300 Laurent Pinchart escreveu:
> > On Saturday, 26 May 2018 14:28:18 EEST Mauro Carvalho Chehab wrote:
> >> Em Sat, 26 May 2018 03:24:00 +0300 Laurent Pinchart escreveu:
> > [snip]
> >
> >>> I've reproduced the issue and created a minimal test case.
> >>>
> >>> 1. struct vsp1_pipeline;
> >>> 2.
> >>> 3. struct vsp1_entity {
> >>> 4. struct vsp1_pipeline *pipe;
> >>> 5. struct vsp1_entity *sink;
> >>> 6. unsigned int source_pad;
> >>> 7. };
> >>> 8.
> >>> 9. struct vsp1_pipeline {
> >>> 10. struct vsp1_entity *brx;
> >>> 11. };
> >>> 12.
> >>> 13. struct vsp1_brx {
> >>> 14. struct vsp1_entity entity;
> >>> 15. };
> >>> 16.
> >>> 17. struct vsp1_device {
> >>> 18. struct vsp1_brx *bru;
> >>> 19. struct vsp1_brx *brs;
> >>> 20. };
> >>> 21.
> >>> 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline
> >>> *pipe)
> >>> 23. {
> >>> 24. struct vsp1_entity *brx;
> >>> 25.
> >>> 26. if (pipe->brx)
> >>> 27. brx = pipe->brx;
> >>> 28. else if (!vsp1->bru->entity.pipe)
> >>> 29. brx = &vsp1->bru->entity;
> >>> 30. else
> >>> 31. brx = &vsp1->brs->entity;
> >>> 32.
> >>> 33. if (brx != pipe->brx)
> >>> 34. pipe->brx = brx;
> >>> 35.
> >>> 36. return pipe->brx->source_pad;
> >>> 37. }
> >>>
> >>> The reason why smatch complains is that it has no guarantee that
> >>> vsp1->brs is not NULL. It's quite tricky:
> >>>
> >>> - On line 26, smatch assumes that pipe->brx can be NULL
> >>> - On line 27, brx is assigned a non-NULL value (as pipe->brx is not
> >>> NULL due to line 26)
> >>> - On line 28, smatch assumes that vsp1->bru is not NULL
> >>> - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not
> >>> NULL due to line 28)
> >>> - On line 31, brx is assigned a possibly NULL value (as there's no
> >>> information regarding vsp1->brs)
> >>> - On line 34, pipe->brx is not assigned a non-NULL value if brx is
> >>> NULL
> >>> - On line 36 pipe->brx is dereferenced
> >>>
> >>> The problem comes from the fact that smatch assumes that vsp1->brs
> >>> isn't NULL. Adding a "(void)vsp1->brs->entity;" statement on line 25
> >>> makes the warning disappear.
> >>>
> >>> So how do we know that vsp1->brs isn't NULL in the original code ?
> >>>
> >>> if (pipe->num_inputs > 2)
> >>> brx = &vsp1->bru->entity;
> >>> else if (pipe->brx && !drm_pipe->force_brx_release)
> >>> brx = pipe->brx;
> >>> else if (!vsp1->bru->entity.pipe)
> >>> brx = &vsp1->bru->entity;
> >>> else
> >>> brx = &vsp1->brs->entity;
> >>>
> >>> A VSP1 instance can have no brs, so in general vsp1->brs can be NULL.
> >>> However, when that's the case, the following conditions are fulfilled.
> >>>
> >>> - drm_pipe->force_brx_release will be false
> >>> - either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be
> >>> NULL
> >>>
> >>> The fourth branch should thus never be taken.
> >>
> >> I don't think that adding a forth branch there would solve.
> >>
> >> The thing is that Smatch knows that pipe->brx can be NULL, as the
> >> function explicly checks if pipe->brx != NULL.
> >>
> >> When Smatch handles this if:
> >>if (brx != pipe->brx) {
> >>
> >> It wrongly assumes that this could be false if pipe->brx is NULL.
> >> I don't know why, as Smatch should know that brx can't be NULL.
> >
> > brx can be NULL here if an only if vsp1->brs is NULL (as the entity field
> > is first in the vsp1->brs structure, so &vsp1->brs->entity has the same
> > address as vsp1->brs).
>
> I can't see how brx can be NULL. At the sequence of ifs:
>
> if (pipe->num_inputs > 2)
> brx = &vsp1->bru->entity;
> else if (pipe->brx && !drm_pipe->force_brx_release)
> brx = pipe->brx;
> else if (!vsp1->bru->entity.pipe)
> brx = &vsp1->bru->entity;
> else
> brx = &vsp1->brs->entity;
>
>
> The usage of brx = &(something) will always return a non NULL value[1].
I don't agree. When vsp1->brs is NULL, &vsp1->brs->entity will evaluate to
NULL as well.
> The only clause where it doesn't use this pattern is:
>
> ...
> if (pipe->brx && !drm_pipe->force_brx_release)
> brx = pipe->brx;
> ...
>
> This one explicitly checks if pipe->brx is NULL, so it will only
> hit on a non-NULL value. If it doesn't hit, brx will be initialized
> by a pointer too either bru or brs entity.
>
> So, brx will always be non-NULL, even if pipe->brx is NULL.
>
> [1] It might be doing a NULL deref - with seems to be your concern
> when you're talking about the case where vsp1->brs is NULL - but
> that's not what Smatch is complaining here.
&vsp1->brs->entity will n
Re: [RFC PATCH v2 1/2] drm: Add generic colorkey properties
On Sat, May 26, 2018 at 06:56:22PM +0300, Dmitry Osipenko wrote:
> Color keying is the action of replacing pixels matching a given color
> (or range of colors) with transparent pixels in an overlay when
> performing blitting. Depending on the hardware capabilities, the
> matching pixel can either become fully transparent or gain adjustment
> of the pixels component values.
>
> Color keying is found in a large number of devices whose capabilities
> often differ, but they still have enough common features in range to
> standardize color key properties. This commit adds nine generic DRM plane
> properties related to the color keying to cover various HW capabilities.
>
> This patch is based on the initial work done by Laurent Pinchart, most of
> credits for this patch goes to him.
>
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/gpu/drm/drm_atomic.c | 36 ++
> drivers/gpu/drm/drm_blend.c | 229 +++
> include/drm/drm_blend.h | 3 +
> include/drm/drm_plane.h | 77
> 4 files changed, 345 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 895741e9cd7d..5b808cb68654 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -799,6 +799,24 @@ static int drm_atomic_plane_set_property(struct
> drm_plane *plane,
> state->rotation = val;
> } else if (property == plane->zpos_property) {
> state->zpos = val;
> + } else if (property == plane->colorkey.mode_property) {
> + state->colorkey.mode = val;
> + } else if (property == plane->colorkey.min_property) {
> + state->colorkey.min = val;
> + } else if (property == plane->colorkey.max_property) {
> + state->colorkey.max = val;
> + } else if (property == plane->colorkey.format_property) {
> + state->colorkey.format = val;
> + } else if (property == plane->colorkey.mask_property) {
> + state->colorkey.mask = val;
> + } else if (property == plane->colorkey.inverted_match_property) {
> + state->colorkey.inverted_match = val;
> + } else if (property == plane->colorkey.replacement_mask_property) {
> + state->colorkey.replacement_mask = val;
> + } else if (property == plane->colorkey.replacement_value_property) {
> + state->colorkey.replacement_value = val;
> + } else if (property == plane->colorkey.replacement_format_property) {
> + state->colorkey.replacement_format = val;
> } else if (property == plane->color_encoding_property) {
> state->color_encoding = val;
> } else if (property == plane->color_range_property) {
> @@ -864,6 +882,24 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> *val = state->rotation;
> } else if (property == plane->zpos_property) {
> *val = state->zpos;
> + } else if (property == plane->colorkey.mode_property) {
> + *val = state->colorkey.mode;
> + } else if (property == plane->colorkey.min_property) {
> + *val = state->colorkey.min;
> + } else if (property == plane->colorkey.max_property) {
> + *val = state->colorkey.max;
> + } else if (property == plane->colorkey.format_property) {
> + *val = state->colorkey.format;
> + } else if (property == plane->colorkey.mask_property) {
> + *val = state->colorkey.mask;
> + } else if (property == plane->colorkey.inverted_match_property) {
> + *val = state->colorkey.inverted_match;
> + } else if (property == plane->colorkey.replacement_mask_property) {
> + *val = state->colorkey.replacement_mask;
> + } else if (property == plane->colorkey.replacement_value_property) {
> + *val = state->colorkey.replacement_value;
> + } else if (property == plane->colorkey.replacement_format_property) {
> + *val = state->colorkey.replacement_format;
> } else if (property == plane->color_encoding_property) {
> *val = state->color_encoding;
> } else if (property == plane->color_range_property) {
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index a16a74d7e15e..05e5632ce375 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -107,6 +107,11 @@
> * planes. Without this property the primary plane is always below the
> cursor
> * plane, and ordering between all other planes is undefined.
> *
> + * colorkey:
> + * Color keying is set up with drm_plane_create_colorkey_properties().
> + * It adds support for replacing a range of colors with a transparent
> + * color in the plane.
> + *
> * Note that all the property extensions described here apply either to the
> * plane or the CRTC (e.g. for the background color, which currently is not
> * exposed and assumed to be black).
> @@ -448,3 +453,227 @@ int drm_
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Dan,
Thank you for your quick reply.
On Monday, 28 May 2018 13:20:49 EEST Dan Carpenter wrote:
> On Mon, May 28, 2018 at 11:28:41AM +0300, Laurent Pinchart wrote:
> > and I still get the same warning. I had to write the following (which is
> > obviously not correct) to silence the warning.
> >
> > if (pipe->num_inputs > 2)
> > brx = &vsp1->bru->entity;
> > else if (pipe->brx)
> > brx = pipe->brx;
> > else if (!vsp1->bru->entity.pipe)
> > brx = &vsp1->bru->entity;
> > else {
> > (void)vsp1->brs->entity;
> > brx = &vsp1->brs->entity;
> > }
> >
> > Both the (void)vsp1->brs->entity and the removal of the !drm_pipe->
> > force_brx_release were needed, any of those on its own didn't fix the
> > problem.
>
> The problem in this case is the first "brx = &vsp1->bru->entity;".
> Smatch assumes &vsp1->bru->entity can be == to pipe->brx and NULL.
Why does smatch assume that &vsp1->bru->entity can be NULL, when the previous
line dereferences vsp1->bru ?
> Adding a "(void)vsp1->bru->entity;" on that path will silence the
> warning (hopefully).
--
Regards,
Laurent Pinchart
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 07:03:05AM -0300, Mauro Carvalho Chehab wrote: > I can't see how brx can be NULL. At the sequence of ifs: > > if (pipe->num_inputs > 2) > brx = &vsp1->bru->entity; > else if (pipe->brx && !drm_pipe->force_brx_release) > brx = pipe->brx; > else if (!vsp1->bru->entity.pipe) > brx = &vsp1->bru->entity; > else > brx = &vsp1->brs->entity; > > > The usage of brx = &(something) will always return a non NULL > value[1]. That's not right. It can be NULL if it's &foo->first_struct_member and ->entity is the first struct member. If it weren't the first struct member then Smatch would say that brx was non-NULL. > [1] It might be doing a NULL deref - with seems to be your concern > when you're talking about the case where vsp1->brs is NULL - but > that's not what Smatch is complaining here. If vsp1->bru were NULL, it wouldn't be a NULL dereference because it's not dereferencing it; it's just taking the address. On the path where we do: else if (!vsp1->bru->entity.pipe) brx = &vsp1->bru->entity; Then Smatch sees that vsp1->bru is dereferenced and marks "brx" as non-NULL. regards, dan carpenter
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Dan,
On Monday, 28 May 2018 13:36:08 EEST Dan Carpenter wrote:
> On Mon, May 28, 2018 at 11:31:01AM +0300, Laurent Pinchart wrote:
> > And that being said, I just tried
> >
> > if (pipe->num_inputs > 2)
> > brx = &vsp1->bru->entity;
> > else if (pipe->brx && !drm_pipe->force_brx_release)
> > brx = pipe->brx;
> > else if (!vsp1->bru->entity.pipe)
> > brx = &vsp1->bru->entity;
> > else
> > brx = &vsp1->brs->entity;
> >
> > if (!brx)
> > return -EINVAL;
> >
> > and that didn't help either... Dan, would you have some light to shed on
> > this problem ?
>
> This is a problem in Smatch.
>
> We should be able to go backwards and say that "If we know 'brx' is
> non-NULL then let's mark &vsp1->brs->entity, vsp1->brs,
> &vsp1->bru->entity and vsp1->bru all as non-NULL as well". But Smatch
> doesn't go backwards like that. The information is mostly there to do
> it, but my instinct is that it's really hard to implement.
>
> The other potential problem here is that Smatch stores comparisons and
> values separately. In other words smatch_comparison.c has all the
> information about brx == &vsp1->bru->entity and smatch_extra.c has the
> information about if brx is NULL or non-NULL. They don't really share
> information very well.
It would indeed be useful to implement, but I share your concern that this
would be pretty difficult.
However, there's still something that puzzles me. Let's add a bit more
context.
if (pipe->num_inputs > 2)
brx = &vsp1->bru->entity;
else if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
else if (!vsp1->bru->entity.pipe)
brx = &vsp1->bru->entity;
else
brx = &vsp1->brs->entity;
1. if (!brx)
return -EINVAL;
2. if (brx != pipe->brx) {
...
3. pipe->brx = brx;
...
}
4. format.pad = pipe->brx->source_pad
(1) ensures that brx can't be NULL. (2) is thus always true if pipe->brx is
NULL. (3) then assigns a non-NULL value to pipe->brx. Smatch should thus never
complain about (4), even if it can't backtrack.
--
Regards,
Laurent Pinchart
Re: [PATCH] media: v4l2-ctrl: Add control for VP9 profile
Hi Hans, On Fri, May 25, 2018 at 6:09 PM Hans Verkuil wrote: > On 17/05/18 11:53, Keiichi Watanabe wrote: > > Add a new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE for selecting desired > > profile for VP9 encoder and querying for supported profiles by VP9 encoder > > or decoder. > > > > An existing control V4L2_CID_MPEG_VIDEO_VPX_PROFILE cannot be > > used for querying since it is not a menu control but an integer > > control, which cannot return an arbitrary set of supported profiles. > > > > The new control V4L2_CID_MPEG_VIDEO_VP9_PROFILE is a menu control as > > with controls for other codec profiles. (e.g. H264) > I don't mind adding this control (although I would like to have an Ack from > Sylwester), but we also need this to be used in an actual kernel driver. > Otherwise we're adding a control that nobody uses. Indeed. We were supposed to also include a patch for mtk-vcodec driver, but somehow it went MIA. Keiichi will add it in next revision. Thanks for giving this a look. Best regards, Tomasz
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 01:58:41PM +0300, Laurent Pinchart wrote:
> It would indeed be useful to implement, but I share your concern that this
> would be pretty difficult.
>
> However, there's still something that puzzles me. Let's add a bit more
> context.
>
> if (pipe->num_inputs > 2)
> brx = &vsp1->bru->entity;
> else if (pipe->brx && !drm_pipe->force_brx_release)
> brx = pipe->brx;
> else if (!vsp1->bru->entity.pipe)
> brx = &vsp1->bru->entity;
> else
> brx = &vsp1->brs->entity;
>
> 1. if (!brx)
> return -EINVAL;
>
> 2. if (brx != pipe->brx) {
> ...
> 3. pipe->brx = brx;
> ...
> }
>
> 4. format.pad = pipe->brx->source_pad
>
>
> (1) ensures that brx can't be NULL. (2) is thus always true if pipe->brx is
> NULL. (3) then assigns a non-NULL value to pipe->brx. Smatch should thus
> never
> complain about (4), even if it can't backtrack.
Oh wow... That's a very basic and ancient bug. I've written a fix and
will push it out tomorrow if it passes my testing.
regards,
dan carpenter
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 01:54:18PM +0300, Laurent Pinchart wrote:
> Hi Dan,
>
> Thank you for your quick reply.
>
> On Monday, 28 May 2018 13:20:49 EEST Dan Carpenter wrote:
> > On Mon, May 28, 2018 at 11:28:41AM +0300, Laurent Pinchart wrote:
> > > and I still get the same warning. I had to write the following (which is
> > > obviously not correct) to silence the warning.
> > >
> > > if (pipe->num_inputs > 2)
> > > brx = &vsp1->bru->entity;
> > > else if (pipe->brx)
> > > brx = pipe->brx;
> > > else if (!vsp1->bru->entity.pipe)
> > > brx = &vsp1->bru->entity;
> > > else {
> > > (void)vsp1->brs->entity;
> > > brx = &vsp1->brs->entity;
> > > }
> > >
> > > Both the (void)vsp1->brs->entity and the removal of the !drm_pipe->
> > > force_brx_release were needed, any of those on its own didn't fix the
> > > problem.
> >
> > The problem in this case is the first "brx = &vsp1->bru->entity;".
> > Smatch assumes &vsp1->bru->entity can be == to pipe->brx and NULL.
>
> Why does smatch assume that &vsp1->bru->entity can be NULL, when the previous
> line dereferences vsp1->bru ?
I'm talking about when pipe->num_inputs > 2. For the second
"brx = &vsp1->bru->entity;" assignment, then Smatch parses it correctly
as you say.
regards,
dan carpenter
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
On Mon, May 28, 2018 at 07:17:54AM -0300, Mauro Carvalho Chehab wrote:
> This (obviously wrong patch) shut up the warning:
>
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -248,6 +248,9 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device
> *vsp1,
> else
> brx = &vsp1->brs->entity;
>
> + if (pipe->brx == brx)
> + pipe->brx = &vsp1->brs->entity;
> +
> /* Switch BRx if needed. */
> if (brx != pipe->brx) {
> struct vsp1_entity *released_brx = NULL;
>
Just to be clear. After this patch, Smatch does *NOT* think that
"pipe->brx" is necessarily non-NULL. What this patch does it that
Smatch says "pipe->brx has been modified on every code path since we
checked for NULL, so forget about the earlier check".
regards,
dan carpenter
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Em Mon, 28 May 2018 11:28:41 +0300
Laurent Pinchart escreveu:
> Hi Mauro,
>
> On Saturday, 26 May 2018 14:28:18 EEST Mauro Carvalho Chehab wrote:
> > Em Sat, 26 May 2018 03:24:00 +0300 Laurent Pinchart escreveu:
>
> [snip]
>
> > > I've reproduced the issue and created a minimal test case.
> > >
> > > 1. struct vsp1_pipeline;
> > > 2.
> > > 3. struct vsp1_entity {
> > > 4. struct vsp1_pipeline *pipe;
> > > 5. struct vsp1_entity *sink;
> > > 6. unsigned int source_pad;
> > > 7. };
> > > 8.
> > > 9. struct vsp1_pipeline {
> > > 10. struct vsp1_entity *brx;
> > > 11. };
> > > 12.
> > > 13. struct vsp1_brx {
> > > 14. struct vsp1_entity entity;
> > > 15. };
> > > 16.
> > > 17. struct vsp1_device {
> > > 18. struct vsp1_brx *bru;
> > > 19. struct vsp1_brx *brs;
> > > 20. };
> > > 21.
> > > 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline
> > > *pipe)
> > > 23. {
> > > 24. struct vsp1_entity *brx;
> > > 25.
> > > 26. if (pipe->brx)
> > > 27. brx = pipe->brx;
> > > 28. else if (!vsp1->bru->entity.pipe)
> > > 29. brx = &vsp1->bru->entity;
> > > 30. else
> > > 31. brx = &vsp1->brs->entity;
> > > 32.
> > > 33. if (brx != pipe->brx)
> > > 34. pipe->brx = brx;
> > > 35.
> > > 36. return pipe->brx->source_pad;
> > > 37. }
> > >
> > > The reason why smatch complains is that it has no guarantee that vsp1->brs
> > > is not NULL. It's quite tricky:
> > >
> > > - On line 26, smatch assumes that pipe->brx can be NULL
> > > - On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL
> > > due to line 26)
> > > - On line 28, smatch assumes that vsp1->bru is not NULL
> > > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > > due to line 28)
> > > - On line 31, brx is assigned a possibly NULL value (as there's no
> > > information regarding vsp1->brs)
> > > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > > - On line 36 pipe->brx is dereferenced
> > >
> > > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > > NULL. Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the
> > > warning disappear.
> > >
> > > So how do we know that vsp1->brs isn't NULL in the original code ?
> > >
> > > if (pipe->num_inputs > 2)
> > > brx = &vsp1->bru->entity;
> > > else if (pipe->brx && !drm_pipe->force_brx_release)
> > > brx = pipe->brx;
> > > else if (!vsp1->bru->entity.pipe)
> > > brx = &vsp1->bru->entity;
> > > else
> > > brx = &vsp1->brs->entity;
> > >
> > > A VSP1 instance can have no brs, so in general vsp1->brs can be NULL.
> > > However, when that's the case, the following conditions are fulfilled.
> > >
> > > - drm_pipe->force_brx_release will be false
> > > - either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be
> > > NULL
> > >
> > > The fourth branch should thus never be taken.
> >
> > I don't think that adding a forth branch there would solve.
> >
> > The thing is that Smatch knows that pipe->brx can be NULL, as the function
> > explicly checks if pipe->brx != NULL.
> >
> > When Smatch handles this if:
> >
> > if (brx != pipe->brx) {
> >
> > It wrongly assumes that this could be false if pipe->brx is NULL.
> > I don't know why, as Smatch should know that brx can't be NULL.
>
> brx can be NULL here if an only if vsp1->brs is NULL (as the entity field is
> first in the vsp1->brs structure, so &vsp1->brs->entity has the same address
> as vsp1->brs).
I can't see how brx can be NULL. At the sequence of ifs:
if (pipe->num_inputs > 2)
brx = &vsp1->bru->entity;
else if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
else if (!vsp1->bru->entity.pipe)
brx = &vsp1->bru->entity;
else
brx = &vsp1->brs->entity;
The usage of brx = &(something) will always return a non NULL
value[1]. The only clause where it doesn't use this pattern is:
...
if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
...
This one explicitly checks if pipe->brx is NULL, so it will only
hit on a non-NULL value. If it doesn't hit, brx will be initialized
by a pointer too either bru or brs entity.
So, brx will always be non-NULL, even if pipe->brx is NULL.
[1] It might be doing a NULL deref - with seems to be your concern
when you're talking about the case where vsp1->brs is NULL - but
that's not what Smatch is complaining here.
>
> vsp1->brs can be NULL on some devices, but in that case we have the following
> guarantees:
>
> - drm_pipe->force_brx_release will always be FALSE
> - either pipe->brx will be non-NULL or vsp1->bru->entity.pipe will be NULL
>
> So the fo
Re: [PATCH v2 11/29] venus: venc,vdec: adds clocks needed for venus 4xx
Hi Tomasz,
On 05/24/2018 09:11 AM, Tomasz Figa wrote:
> Hi Stanimir,
>
> On Tue, May 15, 2018 at 5:10 PM Stanimir Varbanov <
> stanimir.varba...@linaro.org> wrote:
>
>> This extends the clocks number to support suspend and resume
>> on Venus version 4xx.
>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/platform/qcom/venus/core.h | 4 +--
>> drivers/media/platform/qcom/venus/vdec.c | 42
> ++--
>> drivers/media/platform/qcom/venus/venc.c | 42
> ++--
>> 3 files changed, 72 insertions(+), 16 deletions(-)
>
>> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
>> index 8d3e150800c9..b5b9a84e9155 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -92,8 +92,8 @@ struct venus_core {
>> void __iomem *base;
>> int irq;
>> struct clk *clks[VIDC_CLKS_NUM_MAX];
>> - struct clk *core0_clk;
>> - struct clk *core1_clk;
>> + struct clk *core0_clk, *core0_bus_clk;
>> + struct clk *core1_clk, *core1_bus_clk;
>> struct video_device *vdev_dec;
>> struct video_device *vdev_enc;
>> struct v4l2_device v4l2_dev;
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
> b/drivers/media/platform/qcom/venus/vdec.c
>> index 261a51adeef2..c45452634e7e 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -1081,12 +1081,18 @@ static int vdec_probe(struct platform_device
> *pdev)
>> if (!core)
>> return -EPROBE_DEFER;
>
>> - if (core->res->hfi_version == HFI_VERSION_3XX) {
>> + if (IS_V3(core) || IS_V4(core)) {
>> core->core0_clk = devm_clk_get(dev, "core");
>> if (IS_ERR(core->core0_clk))
>> return PTR_ERR(core->core0_clk);
>> }
>
>> + if (IS_V4(core)) {
>> + core->core0_bus_clk = devm_clk_get(dev, "bus");
>> + if (IS_ERR(core->core0_bus_clk))
>> + return PTR_ERR(core->core0_bus_clk);
>> + }
>> +
>
> Rather than doing this conditional dance, wouldn't it make more sense to
> just list all the clocks in variant data struct and use clk_bulk_get()?
Do you mean the same as it is done for venus/core.c ?
>
>> platform_set_drvdata(pdev, core);
>
>> vdev = video_device_alloc();
>> @@ -1132,12 +1138,23 @@ static __maybe_unused int
> vdec_runtime_suspend(struct device *dev)
>> {
>> struct venus_core *core = dev_get_drvdata(dev);
>
>> - if (core->res->hfi_version == HFI_VERSION_1XX)
>> + if (IS_V1(core))
>> return 0;
>
>> - writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + if (IS_V3(core))
>> + writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + else if (IS_V4(core))
>> + writel(0, core->base +
> WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
>> +
>> + if (IS_V4(core))
>> + clk_disable_unprepare(core->core0_bus_clk);
>> +
>> clk_disable_unprepare(core->core0_clk);
>> - writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> +
>> + if (IS_V3(core))
>> + writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + else if (IS_V4(core))
>> + writel(1, core->base +
> WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
>
> Almost every step here differs between version. I'd suggest splitting this
> into separate functions for both versions.
I think it will be better to squash this patch with 13/29.
--
regards,
Stan
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Mauro and Dan,
Dan, there's a question for you below.
On Monday, 28 May 2018 11:28:41 EEST Laurent Pinchart wrote:
> On Saturday, 26 May 2018 14:28:18 EEST Mauro Carvalho Chehab wrote:
> > Em Sat, 26 May 2018 03:24:00 +0300 Laurent Pinchart escreveu:
> [snip]
>
> > > I've reproduced the issue and created a minimal test case.
> > >
> > > 1. struct vsp1_pipeline;
> > > 2.
> > > 3. struct vsp1_entity {
> > > 4. struct vsp1_pipeline *pipe;
> > > 5. struct vsp1_entity *sink;
> > > 6. unsigned int source_pad;
> > > 7. };
> > > 8.
> > > 9. struct vsp1_pipeline {
> > > 10. struct vsp1_entity *brx;
> > > 11. };
> > > 12.
> > > 13. struct vsp1_brx {
> > > 14. struct vsp1_entity entity;
> > > 15. };
> > > 16.
> > > 17. struct vsp1_device {
> > > 18. struct vsp1_brx *bru;
> > > 19. struct vsp1_brx *brs;
> > > 20. };
> > > 21.
> > > 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline
> > > *pipe)
> > > 23. {
> > > 24. struct vsp1_entity *brx;
> > > 25.
> > > 26. if (pipe->brx)
> > > 27. brx = pipe->brx;
> > > 28. else if (!vsp1->bru->entity.pipe)
> > > 29. brx = &vsp1->bru->entity;
> > > 30. else
> > > 31. brx = &vsp1->brs->entity;
> > > 32.
> > > 33. if (brx != pipe->brx)
> > > 34. pipe->brx = brx;
> > > 35.
> > > 36. return pipe->brx->source_pad;
> > > 37. }
> > >
> > > The reason why smatch complains is that it has no guarantee that
> > > vsp1->brs is not NULL. It's quite tricky:
> > >
> > > - On line 26, smatch assumes that pipe->brx can be NULL
> > > - On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL
> > > due to line 26)
> > > - On line 28, smatch assumes that vsp1->bru is not NULL
> > > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > > due to line 28)
> > > - On line 31, brx is assigned a possibly NULL value (as there's no
> > > information regarding vsp1->brs)
> > > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > > - On line 36 pipe->brx is dereferenced
> > >
> > > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > > NULL. Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the
> > > warning disappear.
> > >
> > > So how do we know that vsp1->brs isn't NULL in the original code ?
> > >
> > > if (pipe->num_inputs > 2)
> > > brx = &vsp1->bru->entity;
> > > else if (pipe->brx && !drm_pipe->force_brx_release)
> > > brx = pipe->brx;
> > > else if (!vsp1->bru->entity.pipe)
> > > brx = &vsp1->bru->entity;
> > > else
> > > brx = &vsp1->brs->entity;
> > >
> > > A VSP1 instance can have no brs, so in general vsp1->brs can be NULL.
> > > However, when that's the case, the following conditions are fulfilled.
> > >
> > > - drm_pipe->force_brx_release will be false
> > > - either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be
> > > NULL
> > >
> > > The fourth branch should thus never be taken.
> >
> > I don't think that adding a forth branch there would solve.
> >
> > The thing is that Smatch knows that pipe->brx can be NULL, as the function
> > explicly checks if pipe->brx != NULL.
> >
> > When Smatch handles this if:
> > if (brx != pipe->brx) {
> >
> > It wrongly assumes that this could be false if pipe->brx is NULL.
> > I don't know why, as Smatch should know that brx can't be NULL.
>
> brx can be NULL here if an only if vsp1->brs is NULL (as the entity field is
> first in the vsp1->brs structure, so &vsp1->brs->entity has the same
> address as vsp1->brs).
>
> vsp1->brs can be NULL on some devices, but in that case we have the
> following guarantees:
>
> - drm_pipe->force_brx_release will always be FALSE
> - either pipe->brx will be non-NULL or vsp1->bru->entity.pipe will be NULL
>
> So the fourth branch is never taken.
>
> The above conditions come from outside this function, and smatch can't know
> about them. However, I don't know whether the problems comes from smatch
> assuming that vsp1->brs can be NULL, or from somewhere else.
>
> > On such case, the next code to be executed would be:
> > format.pad = pipe->brx->source_pad;
> >
> > With would be trying to de-ref a NULL pointer.
> >
> > There are two ways to fix it:
> >
> > 1) with my patch.
> >
> > It is based to the fact that, if pipe->brx is null, then brx won't be
> > NULL. So, the logic that "Switch BRx if needed." will always be called:
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index 095dc48aa25a..cb6b60843400
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -185,7 +185,7 @@ static int vsp1_du_pipeline_setup_brx(struct
> > vsp1_device *vsp1,
> > brx = &vsp1->brs->entity;
> >
>
Re: [GIT PULL FOR v4.18] R-Car VSP1 TLB optimisation
Hi Mauro,
On Saturday, 26 May 2018 14:28:18 EEST Mauro Carvalho Chehab wrote:
> Em Sat, 26 May 2018 03:24:00 +0300 Laurent Pinchart escreveu:
[snip]
> > I've reproduced the issue and created a minimal test case.
> >
> > 1. struct vsp1_pipeline;
> > 2.
> > 3. struct vsp1_entity {
> > 4. struct vsp1_pipeline *pipe;
> > 5. struct vsp1_entity *sink;
> > 6. unsigned int source_pad;
> > 7. };
> > 8.
> > 9. struct vsp1_pipeline {
> > 10. struct vsp1_entity *brx;
> > 11. };
> > 12.
> > 13. struct vsp1_brx {
> > 14. struct vsp1_entity entity;
> > 15. };
> > 16.
> > 17. struct vsp1_device {
> > 18. struct vsp1_brx *bru;
> > 19. struct vsp1_brx *brs;
> > 20. };
> > 21.
> > 22. unsigned int frob(struct vsp1_device *vsp1, struct vsp1_pipeline
> > *pipe)
> > 23. {
> > 24. struct vsp1_entity *brx;
> > 25.
> > 26. if (pipe->brx)
> > 27. brx = pipe->brx;
> > 28. else if (!vsp1->bru->entity.pipe)
> > 29. brx = &vsp1->bru->entity;
> > 30. else
> > 31. brx = &vsp1->brs->entity;
> > 32.
> > 33. if (brx != pipe->brx)
> > 34. pipe->brx = brx;
> > 35.
> > 36. return pipe->brx->source_pad;
> > 37. }
> >
> > The reason why smatch complains is that it has no guarantee that vsp1->brs
> > is not NULL. It's quite tricky:
> >
> > - On line 26, smatch assumes that pipe->brx can be NULL
> > - On line 27, brx is assigned a non-NULL value (as pipe->brx is not NULL
> > due to line 26)
> > - On line 28, smatch assumes that vsp1->bru is not NULL
> > - On line 29, brx is assigned a non-NULL value (as vsp1->bru is not NULL
> > due to line 28)
> > - On line 31, brx is assigned a possibly NULL value (as there's no
> > information regarding vsp1->brs)
> > - On line 34, pipe->brx is not assigned a non-NULL value if brx is NULL
> > - On line 36 pipe->brx is dereferenced
> >
> > The problem comes from the fact that smatch assumes that vsp1->brs isn't
> > NULL. Adding a "(void)vsp1->brs->entity;" statement on line 25 makes the
> > warning disappear.
> >
> > So how do we know that vsp1->brs isn't NULL in the original code ?
> >
> > if (pipe->num_inputs > 2)
> > brx = &vsp1->bru->entity;
> > else if (pipe->brx && !drm_pipe->force_brx_release)
> > brx = pipe->brx;
> > else if (!vsp1->bru->entity.pipe)
> > brx = &vsp1->bru->entity;
> > else
> > brx = &vsp1->brs->entity;
> >
> > A VSP1 instance can have no brs, so in general vsp1->brs can be NULL.
> > However, when that's the case, the following conditions are fulfilled.
> >
> > - drm_pipe->force_brx_release will be false
> > - either pipe->brx will be non-NULL, or vsp1->bru->entity.pipe will be
> > NULL
> >
> > The fourth branch should thus never be taken.
>
> I don't think that adding a forth branch there would solve.
>
> The thing is that Smatch knows that pipe->brx can be NULL, as the function
> explicly checks if pipe->brx != NULL.
>
> When Smatch handles this if:
>
> if (brx != pipe->brx) {
>
> It wrongly assumes that this could be false if pipe->brx is NULL.
> I don't know why, as Smatch should know that brx can't be NULL.
brx can be NULL here if an only if vsp1->brs is NULL (as the entity field is
first in the vsp1->brs structure, so &vsp1->brs->entity has the same address
as vsp1->brs).
vsp1->brs can be NULL on some devices, but in that case we have the following
guarantees:
- drm_pipe->force_brx_release will always be FALSE
- either pipe->brx will be non-NULL or vsp1->bru->entity.pipe will be NULL
So the fourth branch is never taken.
The above conditions come from outside this function, and smatch can't know
about them. However, I don't know whether the problems comes from smatch
assuming that vsp1->brs can be NULL, or from somewhere else.
> On such case, the next code to be executed would be:
>
> format.pad = pipe->brx->source_pad;
>
> With would be trying to de-ref a NULL pointer.
>
> There are two ways to fix it:
>
> 1) with my patch.
>
> It is based to the fact that, if pipe->brx is null, then brx won't be
> NULL. So, the logic that "Switch BRx if needed." will always be called:
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index 095dc48aa25a..cb6b60843400
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -185,7 +185,7 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device
> *vsp1, brx = &vsp1->brs->entity;
>
> /* Switch BRx if needed. */
> - if (brx != pipe->brx) {
> + if (brx != pipe->brx || !pipe->brx) {
> struct vsp1_entity *released_brx = NULL;
>
> /* Release our BRx if we have one. */
>
> The code with switches BRx ensures that pipe->brx won't be null, as
> in the end, it sets:
>
> pipe->brx = brx;
>
> And br
Re: [PATCH 4/6] media: imx-csi: Enable interlaced scan for field type alternate
On 28/05/18 08:00, Philipp Zabel wrote: On Fri, 2018-05-25 at 16:53 -0700, Steve Longerbeam wrote: Interlaced scan, a.k.a. interweave, should be enabled at the CSI IDMAC output pad if the input field type is 'alternate' (in addition to field types 'seq-tb' and 'seq-bt'). Which brings up whether V4L2_FIELD_HAS_BOTH() macro should be used to determine enabling interlaced/interweave scan. That macro includes the 'interlaced' field types, and in those cases the data is already interweaved with top/bottom field lines. A heads-up for now that this if statement may need to call V4L2_FIELD_IS_SEQUENTIAL() instead, I have no sensor hardware that sends 'interlaced' data, so can't test. I agree, the check here should be IS_SEQUENTIAL || ALTERNATE, and interlaced_scan should also be enabled if image.pix.field is INTERLACED_TB/BT. And for INTERLACED_TB/BT input, the logic should be inverted: then we'd have to enable interlaced_scan whenever image.pix.field is SEQ_BT/TB. Hi Philipp, If your intent here is to de-interweave the two fields back to two sequential fields, I don't believe the IDMAC operation would achieve that. It's basically line stride doubling and a line offset for the lines in the 2nd half of the incoming frame, right? Regards, Ian regards Philipp Signed-off-by: Steve Longerbeam --- drivers/staging/media/imx/imx-media-csi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 9bc555c..eef3483 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -477,7 +477,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) ipu_smfc_set_burstsize(priv->smfc, burst_size); if (image.pix.field == V4L2_FIELD_NONE && - V4L2_FIELD_HAS_BOTH(infmt->field)) + (V4L2_FIELD_HAS_BOTH(infmt->field) || +infmt->field == V4L2_FIELD_ALTERNATE)) ipu_cpmem_interlaced_scan(priv->idmac_ch, image.pix.bytesperline);
Re: [PATCH 4/6] media: imx-csi: Enable interlaced scan for field type alternate
On Fri, 2018-05-25 at 16:53 -0700, Steve Longerbeam wrote: > Interlaced scan, a.k.a. interweave, should be enabled at the CSI IDMAC > output pad if the input field type is 'alternate' (in addition to field > types 'seq-tb' and 'seq-bt'). > > Which brings up whether V4L2_FIELD_HAS_BOTH() macro should be used > to determine enabling interlaced/interweave scan. That macro > includes the 'interlaced' field types, and in those cases the data > is already interweaved with top/bottom field lines. A heads-up for > now that this if statement may need to call V4L2_FIELD_IS_SEQUENTIAL() > instead, I have no sensor hardware that sends 'interlaced' data, so can't > test. I agree, the check here should be IS_SEQUENTIAL || ALTERNATE, and interlaced_scan should also be enabled if image.pix.field is INTERLACED_TB/BT. And for INTERLACED_TB/BT input, the logic should be inverted: then we'd have to enable interlaced_scan whenever image.pix.field is SEQ_BT/TB. regards Philipp > Signed-off-by: Steve Longerbeam > --- > drivers/staging/media/imx/imx-media-csi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/imx/imx-media-csi.c > b/drivers/staging/media/imx/imx-media-csi.c > index 9bc555c..eef3483 100644 > --- a/drivers/staging/media/imx/imx-media-csi.c > +++ b/drivers/staging/media/imx/imx-media-csi.c > @@ -477,7 +477,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) > ipu_smfc_set_burstsize(priv->smfc, burst_size); > > if (image.pix.field == V4L2_FIELD_NONE && > - V4L2_FIELD_HAS_BOTH(infmt->field)) > + (V4L2_FIELD_HAS_BOTH(infmt->field) || > + infmt->field == V4L2_FIELD_ALTERNATE)) > ipu_cpmem_interlaced_scan(priv->idmac_ch, > image.pix.bytesperline); >
