Re: [PATCH v2 2/3] s5p-g2d: Remove unrequired wait in .job_abort
On 18/06/18 06:38, Ezequiel Garcia wrote:
> As per the documentation, job_abort is not required
> to wait until the current job finishes. It is redundant
> to do so, as the core will perform the wait operation.
>
> Remove the wait infrastructure completely.
Sylwester, can you review this?
Thanks!
Hans
>
> Cc: Kyungmin Park
> Cc: Kamil Debski
> Cc: Andrzej Hajda
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/media/platform/s5p-g2d/g2d.c | 11 ---
> drivers/media/platform/s5p-g2d/g2d.h | 1 -
> 2 files changed, 12 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c
> b/drivers/media/platform/s5p-g2d/g2d.c
> index 66aa8cf1d048..e98708883413 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -483,15 +483,6 @@ static int vidioc_s_crop(struct file *file, void *prv,
> const struct v4l2_crop *c
>
> static void job_abort(void *prv)
> {
> - struct g2d_ctx *ctx = prv;
> - struct g2d_dev *dev = ctx->dev;
> -
> - if (dev->curr == NULL) /* No job currently running */
> - return;
> -
> - wait_event_timeout(dev->irq_queue,
> -dev->curr == NULL,
> -msecs_to_jiffies(G2D_TIMEOUT));
> }
>
> static void device_run(void *prv)
> @@ -563,7 +554,6 @@ static irqreturn_t g2d_isr(int irq, void *prv)
> v4l2_m2m_job_finish(dev->m2m_dev, ctx->fh.m2m_ctx);
>
> dev->curr = NULL;
> - wake_up(&dev->irq_queue);
> return IRQ_HANDLED;
> }
>
> @@ -633,7 +623,6 @@ static int g2d_probe(struct platform_device *pdev)
> spin_lock_init(&dev->ctrl_lock);
> mutex_init(&dev->mutex);
> atomic_set(&dev->num_inst, 0);
> - init_waitqueue_head(&dev->irq_queue);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> diff --git a/drivers/media/platform/s5p-g2d/g2d.h
> b/drivers/media/platform/s5p-g2d/g2d.h
> index dd812b557e87..9ffb458a1b93 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.h
> +++ b/drivers/media/platform/s5p-g2d/g2d.h
> @@ -31,7 +31,6 @@ struct g2d_dev {
> struct g2d_ctx *curr;
> struct g2d_variant *variant;
> int irq;
> - wait_queue_head_t irq_queue;
> };
>
> struct g2d_frame {
>
Re: [PATCH v7 1/2] media: ov2680: dt: Add bindings for OV2680
Hi Rui,
sorry, I'm a bit late, you're already at v7 and I don't want to
slow down inclusion with a few minor comments.
Please bear with me and see below...
On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva wrote:
> Add device tree binding documentation for the OV2680 camera sensor.
>
> CC: devicet...@vger.kernel.org
> Signed-off-by: Rui Miguel Silva
> ---
> .../devicetree/bindings/media/i2c/ov2680.txt | 46 +++
> 1 file changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> new file mode 100644
> index ..11e925ed9dad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
> @@ -0,0 +1,46 @@
> +* Omnivision OV2680 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: should be "ovti,ov2680".
> +- clocks: reference to the xvclk input clock.
> +- clock-names: should be "xvclk".
Having a single clock source I think you can omit 'clock-names' (or at
least not marking it as required)
> +- DOVDD-supply: Digital I/O voltage supply.
> +- DVDD-supply: Digital core voltage supply.
> +- AVDD-supply: Analog voltage supply.
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
> + if any. This is an active low signal to the OV2680.
> +
> +The device node must contain one 'port' child node for its digital output
> +video port, and this port must have a single endpoint in accordance with
> + the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Endpoint node required properties for CSI-2 connection are:
> +- remote-endpoint: a phandle to the bus receiver's endpoint node.
> +- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
> +- data-lanes: should be set to <1> (one CSI-2 lane supported).
What is the value of marking as required two properties which can only have
default values (the sensor does not support clock on different lanes,
nor it supports more than 1 data lane) ?
Thanks
j
> +
> +Example:
> +
> +&i2c2 {
> + ov2680: camera-sensor@36 {
> + compatible = "ovti,ov2680";
> + reg = <0x36>;
> + clocks = <&osc>;
> + clock-names = "xvclk";
> + reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
> + DOVDD-supply = <&sw2_reg>;
> + DVDD-supply = <&sw2_reg>;
> + AVDD-supply = <®_peri_3p15v>;
> +
> + port {
> + ov2680_to_mipi: endpoint {
> + remote-endpoint = <&mipi_from_sensor>;
> + clock-lanes = <0>;
> + data-lanes = <1>;
> + };
> + };
> + };
> +};
> --
> 2.18.0
>
signature.asc
Description: PGP signature
[PATCH] media: dvb_ca_en50221: off by one in dvb_ca_en50221_io_do_ioctl()
The > should be >= so we don't read one element beyond the end of the
ca->slot_info[] array. The array is allocated in dvb_ca_en50221_init().
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 1310526b0d49..4d371cea0d5d 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1391,7 +1391,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct file *file,
struct dvb_ca_slot *sl;
slot = info->num;
- if ((slot > ca->slot_count) || (slot < 0)) {
+ if ((slot >= ca->slot_count) || (slot < 0)) {
err = -EINVAL;
goto out_unlock;
}
[GIT PULL FOR v4.19] Various fixes
About half of this pull request is Jacopo's R-Car R8A77995 series, the other half are miscellaneous patches. Regards, Hans The following changes since commit 3c4a737267e89aafa6308c6c456d2ebea3fcd085: media: ov5640: fix frame interval enumeration (2018-06-28 09:24:38 -0400) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.19h for you to fetch changes up to f7080d547e040e061a6dd7aebcde74c14f844392: tuner-simple: allow setting mono radio mode (2018-07-04 11:35:49 +0200) Jacopo Mondi (11): media: rcar-vin: Rename 'digital' to 'parallel' media: rcar-vin: Remove two empty lines media: rcar-vin: Create a group notifier media: rcar-vin: Cleanup notifier in error path media: rcar-vin: Cache the mbus configuration flags media: rcar-vin: Parse parallel input on Gen3 media: rcar-vin: Link parallel input media entities media: rcar-vin: Handle parallel subdev in link_notify media: rcar-vin: Rename _rcar_info to rcar_info media: rcar-vin: Add support for R-Car R8A77995 SoC dt-bindings: media: rcar-vin: Add R8A77995 support Jan Luebbe (2): media: imx: capture: refactor enum_/try_fmt media: imx: add support for RGB565_2X8 on parallel bus Keiichi Watanabe (3): media: v4l2-ctrl: Change control for VP8 profile to menu control media: v4l2-ctrl: Add control for VP9 profile media: mtk-vcodec: Support VP9 profile in decoder Maciej S. Szmigiero (3): ivtv: zero-initialize cx25840 platform data cx25840: add kernel-doc description of struct cx25840_state tuner-simple: allow setting mono radio mode Peter Seiderer (2): media: staging/imx: fill vb2_v4l2_buffer field entry media: staging/imx: fill vb2_v4l2_buffer sequence entry Documentation/devicetree/bindings/media/rcar_vin.txt | 1 + Documentation/media/uapi/v4l/extended-controls.rst | 48 - drivers/media/i2c/cx25840/cx25840-core.h | 33 ++- drivers/media/pci/ivtv/ivtv-i2c.c| 1 + drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 5 + drivers/media/platform/qcom/venus/vdec_ctrls.c | 10 +- drivers/media/platform/qcom/venus/venc.c | 4 +- drivers/media/platform/qcom/venus/venc_ctrls.c | 10 +- drivers/media/platform/rcar-vin/rcar-core.c | 265 +- drivers/media/platform/rcar-vin/rcar-dma.c | 36 --- drivers/media/platform/rcar-vin/rcar-v4l2.c | 12 +-- drivers/media/platform/rcar-vin/rcar-vin.h | 29 -- drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 15 ++- drivers/media/tuners/tuner-simple.c | 5 +- drivers/media/v4l2-core/v4l2-ctrls.c | 23 - drivers/staging/media/imx/imx-ic-prpencvf.c | 5 + drivers/staging/media/imx/imx-media-capture.c| 38 drivers/staging/media/imx/imx-media-csi.c| 106 +--- drivers/staging/media/imx/imx-media-utils.c | 1 + drivers/staging/media/imx/imx-media.h| 2 + include/uapi/linux/v4l2-controls.h | 18 +++- 21 files changed, 468 insertions(+), 199 deletions(-)
Re: [PATCH] vivid: fix gain when autogain is on
Em Fri, 29 Jun 2018 11:40:41 +0200
Hans Verkuil escreveu:
> In the vivid driver you want gain to continuous change while autogain
> is on. However, dev->jiffies_vid_cap doesn't actually change. It probably
> did in the past, but changes in the code caused this to be a fixed value
> that is only set when you start streaming.
>
> Replace it by jiffies, which is always changing.
>
> Signed-off-by: Hans Verkuil
> ---
> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c
> b/drivers/media/platform/vivid/vivid-ctrls.c
> index 6b0bfa091592..6eb8ad7fb12c 100644
> --- a/drivers/media/platform/vivid/vivid-ctrls.c
> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
> @@ -295,7 +295,7 @@ static int vivid_user_vid_g_volatile_ctrl(struct
> v4l2_ctrl *ctrl)
>
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> - dev->gain->val = dev->jiffies_vid_cap & 0xff;
> + dev->gain->val = (jiffies / HZ) & 0xff;
Manipulating jiffies directly like the above is not a good idea.
Better to use, instead:
dev->gain->val = (jiffies_to_msecs(jiffies) / 1000) & 0xff;
I'll change the code when applying the patch.
Thanks,
Mauro
Re: [PATCH] vivid: fix gain when autogain is on
On 04/07/18 14:16, Mauro Carvalho Chehab wrote:
> Em Fri, 29 Jun 2018 11:40:41 +0200
> Hans Verkuil escreveu:
>
>> In the vivid driver you want gain to continuous change while autogain
>> is on. However, dev->jiffies_vid_cap doesn't actually change. It probably
>> did in the past, but changes in the code caused this to be a fixed value
>> that is only set when you start streaming.
>>
>> Replace it by jiffies, which is always changing.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c
>> b/drivers/media/platform/vivid/vivid-ctrls.c
>> index 6b0bfa091592..6eb8ad7fb12c 100644
>> --- a/drivers/media/platform/vivid/vivid-ctrls.c
>> +++ b/drivers/media/platform/vivid/vivid-ctrls.c
>> @@ -295,7 +295,7 @@ static int vivid_user_vid_g_volatile_ctrl(struct
>> v4l2_ctrl *ctrl)
>>
>> switch (ctrl->id) {
>> case V4L2_CID_AUTOGAIN:
>> -dev->gain->val = dev->jiffies_vid_cap & 0xff;
>> +dev->gain->val = (jiffies / HZ) & 0xff;
>
> Manipulating jiffies directly like the above is not a good idea.
Why not? There are HZ jiffies in one second, so this changes the
gain value once a second.
Regards,
Hans
>
> Better to use, instead:
>
> dev->gain->val = (jiffies_to_msecs(jiffies) / 1000) & 0xff;
>
> I'll change the code when applying the patch.
>
>
> Thanks,
> Mauro
>
[PATCH 1/5] media: ov5640: fix exposure regression
fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at
start time").
Symptom was black image when capturing HD or 5Mp picture
due to manual exposure set to 1 while it was intended to
set autoexposure to "manual", fix this.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..4b9da8b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -938,6 +938,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
return ret;
}
+static int ov5640_set_autoexposure(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(0), on ? 0 : BIT(0));
+}
+
/* read exposure, in number of line periods */
static int ov5640_get_exposure(struct ov5640_dev *sensor)
{
@@ -1593,7 +1599,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
const struct ov5640_mode_info *mode,
- s32 exposure)
+ bool auto_exp)
{
int ret;
@@ -1610,7 +1616,8 @@ static int ov5640_set_mode_direct(struct ov5640_dev
*sensor,
if (ret)
return ret;
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure);
+ return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
+ V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1618,7 +1625,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
- s32 exposure;
+ bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
dn_mode = mode->dn_mode;
@@ -1629,8 +1636,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret)
return ret;
- exposure = sensor->ctrls.auto_exp->val;
- ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
+ ret = ov5640_set_autoexposure(sensor, false);
if (ret)
return ret;
@@ -1646,7 +1652,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, exposure);
+ ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
}
if (ret < 0)
--
1.9.1
[PATCH 2/5] media: ov5640: fix auto gain & exposure when changing mode
Ensure that auto gain and auto exposure are well restored
when changing mode.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 96 ++
1 file changed, 54 insertions(+), 42 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 4b9da8b..7c569de 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1000,6 +1000,18 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
return gain & 0x3ff;
}
+static int ov5640_set_gain(struct ov5640_dev *sensor, int gain)
+{
+ return ov5640_write_reg16(sensor, OV5640_REG_AEC_PK_REAL_GAIN,
+ (u16)gain & 0x3ff);
+}
+
+static int ov5640_set_autogain(struct ov5640_dev *sensor, bool on)
+{
+ return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+ BIT(1), on ? 0 : BIT(1));
+}
+
static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
{
int ret;
@@ -1577,7 +1589,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
}
/* set capture gain */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.gain, cap_gain16);
+ ret = ov5640_set_gain(sensor, cap_gain16);
if (ret)
return ret;
@@ -1590,7 +1602,7 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
}
/* set exposure */
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.exposure, cap_shutter);
+ return ov5640_set_exposure(sensor, cap_shutter);
}
/*
@@ -1598,26 +1610,13 @@ static int ov5640_set_mode_exposure_calc(struct
ov5640_dev *sensor,
* change mode directly
*/
static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
- const struct ov5640_mode_info *mode,
- bool auto_exp)
+ const struct ov5640_mode_info *mode)
{
- int ret;
-
if (!mode->reg_data)
return -EINVAL;
/* Write capture setting */
- ret = ov5640_load_regs(sensor, mode);
- if (ret < 0)
- return ret;
-
- /* turn auto gain/exposure back on for direct mode */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
- if (ret)
- return ret;
-
- return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
- V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
+ return ov5640_load_regs(sensor, mode);
}
static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1625,6 +1624,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
{
const struct ov5640_mode_info *mode = sensor->current_mode;
enum ov5640_downsize_mode dn_mode, orig_dn_mode;
+ bool auto_gain = sensor->ctrls.auto_gain->val == 1;
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
@@ -1632,19 +1632,23 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
orig_dn_mode = orig_mode->dn_mode;
/* auto gain and exposure must be turned off when changing modes */
- ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0);
- if (ret)
- return ret;
+ if (auto_gain) {
+ ret = ov5640_set_autogain(sensor, false);
+ if (ret)
+ return ret;
+ }
- ret = ov5640_set_autoexposure(sensor, false);
- if (ret)
- return ret;
+ if (auto_exp) {
+ ret = ov5640_set_autoexposure(sensor, false);
+ if (ret)
+ goto restore_auto_gain;
+ }
if ((dn_mode == SUBSAMPLING && orig_dn_mode == SCALING) ||
(dn_mode == SCALING && orig_dn_mode == SUBSAMPLING)) {
/*
* change between subsampling and scaling
-* go through exposure calucation
+* go through exposure calculation
*/
ret = ov5640_set_mode_exposure_calc(sensor, mode);
} else {
@@ -1652,11 +1656,16 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
* change inside subsampling or scaling
* download firmware directly
*/
- ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
+ ret = ov5640_set_mode_direct(sensor, mode);
}
-
if (ret < 0)
- return ret;
+ goto restore_auto_exp_gain;
+
+ /* restore auto gain and exposure */
+ if (auto_gain)
+ ov5640_set_autogain(sensor, true);
+ if (auto_exp)
+ ov5640_set_autoexposure(sensor, true);
ret = ov5640_set_timings(sensor, mode);
if (ret < 0)
@@ -1681,6 +1690,15 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
sensor->pending_mode_change = false;
return 0;
+
+restore_auto_exp_gain:
+ if (auto_exp)
+
[PATCH 5/5] media: ov5640: fix restore of last mode set
Mode setting depends on last mode set, in particular
because of exposure calculation when downscale mode
change between subsampling and scaling.
At stream on the last mode was wrongly set to current mode,
so no change was detected and exposure calculation
was not made, fix this.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index a307f1e..f3fb140 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -225,6 +225,7 @@ struct ov5640_dev {
struct v4l2_mbus_framefmt fmt;
const struct ov5640_mode_info *current_mode;
+ const struct ov5640_mode_info *last_mode;
enum ov5640_frame_rate current_fr;
struct v4l2_fract frame_interval;
@@ -1628,6 +1629,9 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
int ret;
+ if (!orig_mode)
+ orig_mode = mode;
+
dn_mode = mode->dn_mode;
orig_dn_mode = orig_mode->dn_mode;
@@ -1688,6 +1692,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
return ret;
sensor->pending_mode_change = false;
+ sensor->last_mode = mode;
return 0;
@@ -2551,7 +2556,8 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int
enable)
if (sensor->streaming == !enable) {
if (enable && sensor->pending_mode_change) {
- ret = ov5640_set_mode(sensor, sensor->current_mode);
+ ret = ov5640_set_mode(sensor, sensor->last_mode);
+
if (ret)
goto out;
--
1.9.1
[PATCH 4/5] media: ov5640: fix auto controls values when switching to manual mode
When switching from auto to manual mode, V4L2 core is calling
g_volatile_ctrl() in manual mode in order to get the manual initial value.
Remove the manual mode check/return to not break this behaviour.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f9b256e..a307f1e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2277,16 +2277,12 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl
*ctrl)
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
- if (!ctrl->val)
- return 0;
val = ov5640_get_gain(sensor);
if (val < 0)
return val;
sensor->ctrls.gain->val = val;
break;
case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->val == V4L2_EXPOSURE_MANUAL)
- return 0;
val = ov5640_get_exposure(sensor);
if (val < 0)
return val;
--
1.9.1
[PATCH 3/5] media: ov5640: fix wrong binning value in exposure calculation
ov5640_set_mode_exposure_calc() is checking binning value but binning value read is buggy and binning value set is done after calling ov5640_set_mode_exposure_calc(), fix all of this. Signed-off-by: Hugues Fruchet --- drivers/media/i2c/ov5640.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 7c569de..f9b256e 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev *sensor) ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, &temp); if (ret) return ret; - temp &= 0xfe; - return temp ? 1 : 0; + + return temp & BIT(0); } static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable) -- 1.9.1
[PATCH 0/5] Fix OV5640 exposure & gain
This patch serie fixes some problems around exposure & gain in OV5640 driver. The 4th patch about autocontrols requires also a fix in v4l2-ctrls.c: https://www.mail-archive.com/linux-media@vger.kernel.org/msg133164.html Here is the test procedure used for exposure & gain controls check: 1) Preview in background $> gst-launch-1.0 v4l2src ! "video/x-raw, width=640, Height=480" ! queue ! waylandsink -e & 2) Check gain & exposure values $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=19 flags=inactive, volatile 3) Put finger in front of camera and check that gain/exposure values are changing: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=660 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=37 flags=inactive, volatile 4) switch to manual mode, image exposition must not change $> v4l2-ctl --set-ctrl=gain_automatic=0 $> v4l2-ctl --set-ctrl=auto_exposure=1 Note the "1" for manual exposure. 5) Check current gain/exposure values: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 gain (int): min=0 max=1023 step=1 default=0 value=20 6) Put finger behind camera and check that gain/exposure values are NOT changing: $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 gain (int): min=0 max=1023 step=1 default=0 value=20 7) Update exposure, check that it is well changed on display and that same value is returned: $> v4l2-ctl --set-ctrl=exposure=100 $> v4l2-ctl --get-ctrl=exposure exposure: 100 9) Update gain, check that it is well changed on display and that same value is returned: $> v4l2-ctl --set-ctrl=gain=10 $> v4l2-ctl --get-ctrl=gain gain: 10 10) Switch back to auto gain/exposure, verify that image is correct and values returned are correct: $> v4l2-ctl --set-ctrl=gain_automatic=1 $> v4l2-ctl --set-ctrl=auto_exposure=0 $> v4l2-ctl --all | grep -e exposure -e gain | grep "(int)" exposure (int): min=0 max=65535 step=1 default=0 value=330 flags=inactive, volatile gain (int): min=0 max=1023 step=1 default=0 value=22 flags=inactive, volatile Note the "0" for auto exposure. Hugues Fruchet (5): media: ov5640: fix exposure regression media: ov5640: fix auto gain & exposure when changing mode media: ov5640: fix wrong binning value in exposure calculation media: ov5640: fix auto controls values when switching to manual mode media: ov5640: fix restore of last mode set drivers/media/i2c/ov5640.c | 120 ++--- 1 file changed, 70 insertions(+), 50 deletions(-) -- 1.9.1
[PATCH] media: ov5640: do not change mode if format or frame interval is unchanged
Save load of mode registers array when V4L2 client sets a format or a
frame interval which selects the same mode than the current one.
Signed-off-by: Hugues Fruchet
---
drivers/media/i2c/ov5640.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..071f4bc 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1966,9 +1966,11 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
- sensor->current_mode = new_mode;
- sensor->fmt = *mbus_fmt;
- sensor->pending_mode_change = true;
+ if (new_mode != sensor->current_mode) {
+ sensor->current_mode = new_mode;
+ sensor->fmt = *mbus_fmt;
+ sensor->pending_mode_change = true;
+ }
out:
mutex_unlock(&sensor->lock);
return ret;
@@ -2508,8 +2510,10 @@ static int ov5640_s_frame_interval(struct v4l2_subdev
*sd,
goto out;
}
- sensor->current_mode = mode;
- sensor->pending_mode_change = true;
+ if (mode != sensor->current_mode) {
+ sensor->current_mode = mode;
+ sensor->pending_mode_change = true;
+ }
out:
mutex_unlock(&sensor->lock);
return ret;
--
1.9.1
[PATCH] media.h: add encoder/decoder functions for codecs
Add MEDIA_ENT_F_PROC_VIDEO_EN/DECODER to be used for the encoder
and decoder entities of codec hardware.
Signed-off-by: Hans Verkuil
---
Documentation/media/uapi/mediactl/media-types.rst | 10 ++
include/uapi/linux/media.h| 2 ++
2 files changed, 12 insertions(+)
diff --git a/Documentation/media/uapi/mediactl/media-types.rst
b/Documentation/media/uapi/mediactl/media-types.rst
index 96910cf2eaaa..825920c2f57f 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -188,6 +188,16 @@ Types and flags used to represent the media graph elements
received on its sink pad and outputs the statistics data on
its source pad.
+
+* - ``MEDIA_ENT_F_PROC_VIDEO_ENCODER``
+ - Video (MPEG, HEVC, VPx, etc.) encoder. An entity capable of
+ compressing video frames must have one sink pad and one source pad.
+
+* - ``MEDIA_ENT_F_PROC_VIDEO_DECODER``
+ - Video (MPEG, HEVC, VPx, etc.) decoder. An entity capable of
+ decompressing a compressed video stream into uncompressed video
+ frames must have one sink pad and one source pad.
+
* - ``MEDIA_ENT_F_VID_MUX``
- Video multiplexer. An entity capable of multiplexing must have at
least two sink pads and one source pad, and must pass the video
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 86c7dcc9cba3..9004d0c5560c 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -132,6 +132,8 @@ struct media_device_info {
#define MEDIA_ENT_F_PROC_VIDEO_LUT (MEDIA_ENT_F_BASE + 0x4004)
#define MEDIA_ENT_F_PROC_VIDEO_SCALER (MEDIA_ENT_F_BASE + 0x4005)
#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
+#define MEDIA_ENT_F_PROC_VIDEO_ENCODER (MEDIA_ENT_F_BASE + 0x4007)
+#define MEDIA_ENT_F_PROC_VIDEO_DECODER (MEDIA_ENT_F_BASE + 0x4008)
/*
* Switch and bridge entity functions
--
2.18.0
Re: [PATCH 3/5] media: ov5640: fix wrong binning value in exposure calculation
Hi Hugues,
On Wed, Jul 04, 2018 at 02:58:41PM +0200, Hugues Fruchet wrote:
> ov5640_set_mode_exposure_calc() is checking binning value but
> binning value read is buggy and binning value set is done
> after calling ov5640_set_mode_exposure_calc(), fix all of this.
The ov5640_binning_on() function was indeed wrong (side note: that
name is confusing, it should be 0v5640_get_binning() to comply with
others..) and always returned 0, but I don't see a fix here for the
second part of the issue. In facts, during the lenghty exposure
calculation process, binning is checked to decide if the preview
shutter time should be doubled or not
static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
const struct ov5640_mode_info *mode)
{
...
/* read preview shutter */
ret = ov5640_get_exposure(sensor);
if (ret < 0)
return ret;
prev_shutter = ret;
ret = ov5640_binning_on(sensor);
if (ret < 0)
return ret;
if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
mode->id != OV5640_MODE_1080P_1920_1080)
prev_shutter *= 2;
...
}
My understanding is that reading the value from the register returns
the binning settings for the previously configured mode, while the
binning value is later updated for the current mode in
ov5640_set_mode(), after 'ov5640_set_mode_exposure_calc()' has already
been called. Is this ok?
Also, I assume the code checks for mode->id to figure out if the mode
uses subsampling or scaling. Be aware that for 1280x720 mode, the
selected scaling mode depends on the FPS, not only on the mode id as
it is assumed here.
A final note, the 'ov5640_set_mode_exposure_calc()' also writes VTS to
update the shutter time to the newly calculated value.
/* write capture shutter */
if (cap_shutter > (cap_vts - 4)) {
cap_vts = cap_shutter + 4;
ret = ov5640_set_vts(sensor, cap_vts);
if (ret < 0)
return ret;
}
Be aware again that VTS is later restored to the mode->vtot value by
the 'ov5640_set_timings()' functions, which again, is called later
than 'ov5640_set_mode_exposure_calc()'.
Wouldn't it be better to postpone exposure calculation after timings
and binnings have been set?
Thanks
j
>
> Signed-off-by: Hugues Fruchet
> ---
> drivers/media/i2c/ov5640.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 7c569de..f9b256e 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
> ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, &temp);
> if (ret)
> return ret;
> - temp &= 0xfe;
> - return temp ? 1 : 0;
> +
> + return temp & BIT(0);
> }
>
> static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
> --
> 1.9.1
>
signature.asc
Description: PGP signature
[PATCH] media: bpf: ensure bpf program is freed on detach
Currently we are leaking bpf programs when they are detached from the lirc device; the refcount never reaches zero. Signed-off-by: Sean Young --- drivers/media/rc/bpf-lirc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c index fcfab6635f9c..81b150e5dfdb 100644 --- a/drivers/media/rc/bpf-lirc.c +++ b/drivers/media/rc/bpf-lirc.c @@ -174,6 +174,7 @@ static int lirc_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog) rcu_assign_pointer(raw->progs, new_array); bpf_prog_array_free(old_array); + bpf_prog_put(prog); unlock: mutex_unlock(&ir_raw_handler_lock); return ret; -- 2.17.1
Re: [PATCH 3/5] media: ov5640: fix wrong binning value in exposure calculation
Hi Jacopo,
Many thanks for you valuable comments, I hardly understand this exposure
code, and still some wrongly exposed images are observed switching from
subsampling to scaling modes.
Steve, do you have more insight to share with us on this code ?
On 07/04/2018 04:38 PM, jacopo mondi wrote:
> Hi Hugues,
>
> On Wed, Jul 04, 2018 at 02:58:41PM +0200, Hugues Fruchet wrote:
>> ov5640_set_mode_exposure_calc() is checking binning value but
>> binning value read is buggy and binning value set is done
>> after calling ov5640_set_mode_exposure_calc(), fix all of this.
>
> The ov5640_binning_on() function was indeed wrong (side note: that
> name is confusing, it should be 0v5640_get_binning() to comply with
> others..) and always returned 0, but I don't see a fix here for the
> second part of the issue.
Mistake from me here, I should have removed "and binning value set is
done after calling ov5640_set_mode_exposure_calc()" in commit message.
> In facts, during the lenghty exposure
> calculation process, binning is checked to decide if the preview
> shutter time should be doubled or not
>
> static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
>const struct ov5640_mode_info *mode)
> {
> ...
>
> /* read preview shutter */
> ret = ov5640_get_exposure(sensor);
> if (ret < 0)
> return ret;
> prev_shutter = ret;
> ret = ov5640_binning_on(sensor);
> if (ret < 0)
> return ret;
> if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
> mode->id != OV5640_MODE_1080P_1920_1080)
> prev_shutter *= 2;
> ...
> }
>
> My understanding is that reading the value from the register returns
> the binning settings for the previously configured mode, while the > binning
> value is later updated for the current mode in
> ov5640_set_mode(), after 'ov5640_set_mode_exposure_calc()' has already
> been called. Is this ok?
This is also my understanding.
>
> Also, I assume the code checks for mode->id to figure out if the mode
> uses subsampling or scaling. Be aware that for 1280x720 mode, the
> selected scaling mode depends on the FPS, not only on the mode id as
> it is assumed here.
This is not what I understand from this array:
static const struct ov5640_mode_info
ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
[15fps]
{OV5640_MODE_720P_1280_720, SUBSAMPLING,
1280, 1892, 720, 740,
ov5640_setting_15fps_720P_1280_720,
ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
[30fps]
{OV5640_MODE_720P_1280_720, SUBSAMPLING,
1280, 1892, 720, 740,
ov5640_setting_30fps_720P_1280_720,
ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
=> both modes uses subsampling here
>
> A final note, the 'ov5640_set_mode_exposure_calc()' also writes VTS to
> update the shutter time to the newly calculated value.
>
> /* write capture shutter */
> if (cap_shutter > (cap_vts - 4)) {
> cap_vts = cap_shutter + 4;
> ret = ov5640_set_vts(sensor, cap_vts);
> if (ret < 0)
> return ret;
> }
>
> Be aware again that VTS is later restored to the mode->vtot value by
> the 'ov5640_set_timings()' functions, which again, is called later
> than 'ov5640_set_mode_exposure_calc()'.
>
> Wouldn't it be better to postpone exposure calculation after timings
> and binnings have been set ?
As said, I'm new on all of this but I can give it a try.
>
> Thanks
> j
>
>>
>> Signed-off-by: Hugues Fruchet
>> ---
>> drivers/media/i2c/ov5640.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index 7c569de..f9b256e 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
>> ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, &temp);
>> if (ret)
>> return ret;
>> -temp &= 0xfe;
>> -return temp ? 1 : 0;
>> +
>> +return temp & BIT(0);
>> }
>>
>> static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
>> --
>> 1.9.1
>>
Best regards,
Hugues.
[PATCH] v4l2-ctrls: add v4l2_ctrl_request_hdl_find/put/ctrl_find functions
If a driver needs to find/inspect the controls set in a request then
it can use these functions.
E.g. to check if a required control is set in a request use this in the
req_validate() implementation:
int res = -EINVAL;
hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
if (hdl) {
if (v4l2_ctrl_request_hdl_ctrl_find(hdl, ctrl_id))
res = 0;
v4l2_ctrl_request_hdl_put(hdl);
}
return res;
Signed-off-by: Hans Verkuil
---
Paul, please test this. This should be what you need.
Regards,
Hans
---
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3ff17c87b3c8..03fd140736fd 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2976,6 +2976,31 @@ static const struct media_request_object_ops req_ops = {
.release = v4l2_ctrl_request_release,
};
+struct v4l2_ctrl_handler *v4l2_ctrl_request_hdl_find(struct media_request *req,
+ struct v4l2_ctrl_handler *parent)
+{
+ struct media_request_object *obj;
+
+ if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING &&
+ req->state != MEDIA_REQUEST_STATE_QUEUED))
+ return NULL;
+
+ obj = media_request_object_find(req, &req_ops, parent);
+ if (obj)
+ return container_of(obj, struct v4l2_ctrl_handler, req_obj);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_ctrl_request_hdl_find);
+
+struct v4l2_ctrl *
+v4l2_ctrl_request_hdl_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id)
+{
+ struct v4l2_ctrl_ref *ref = find_ref_lock(hdl, id);
+
+ return (ref && ref->req == ref) ? ref : NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_ctrl_request_hdl_ctrl_find);
+
static int v4l2_ctrl_request_bind(struct media_request *req,
struct v4l2_ctrl_handler *hdl,
struct v4l2_ctrl_handler *from)
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index de70cc3a1b80..34ee3167d7dd 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -,7 +,49 @@ void v4l2_ctrl_request_setup(struct media_request *req,
* request object.
*/
void v4l2_ctrl_request_complete(struct media_request *req,
- struct v4l2_ctrl_handler *hdl);
+ struct v4l2_ctrl_handler *parent);
+
+/**
+ * v4l2_ctrl_request_hdl_find - Find the control handler in the request
+ *
+ * @req: The request
+ * @parent: The parent control handler ('priv' in media_request_object_find())
+ *
+ * This function finds the control handler in the request. It may return
+ * NULL if not found. When done, you must call v4l2_ctrl_request_put_hdl()
+ * with the returned handler pointer.
+ *
+ * If the request is not in state VALIDATING or QUEUED, then this function
+ * will always return NULL.
+ */
+struct v4l2_ctrl_handler *v4l2_ctrl_request_hdl_find(struct media_request *req,
+ struct v4l2_ctrl_handler *parent);
+
+/**
+ * v4l2_ctrl_request_hdl_put - Put the control handler
+ *
+ * @hdl: Put this control handler
+ *
+ * This function released the control handler previously obtained from'
+ * v4l2_ctrl_request_hdl_find().
+ */
+static inline void v4l2_ctrl_request_hdl_put(struct v4l2_ctrl_handler *hdl)
+{
+ if (hdl)
+ media_request_object_put(&hdl->req_obj);
+}
+
+/**
+ * v4l2_ctrl_request_ctrl_find() - Find a control with the given ID.
+ *
+ * @hdl: The control handler from the request.
+ * @id: The ID of the control to find.
+ *
+ * This function returns a pointer to the control if this control is
+ * part of the request or NULL otherwise.
+ */
+struct v4l2_ctrl *
+v4l2_ctrl_request_hdl_ctrl_find(struct v4l2_ctrl_handler *hdl, u32 id);
/* Helpers for ioctl_ops */
Re: [PATCH v7 1/2] media: ov2680: dt: Add bindings for OV2680
Hi Jacopo,
Hope your fine.
Thanks for the review.
On Wed 04 Jul 2018 at 09:58, jacopo mondi wrote:
Hi Rui,
sorry, I'm a bit late, you're already at v7 and I don't want
to
slow down inclusion with a few minor comments.
Please bear with me and see below...
On Tue, Jul 03, 2018 at 03:08:02PM +0100, Rui Miguel Silva
wrote:
Add device tree binding documentation for the OV2680 camera
sensor.
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva
---
.../devicetree/bindings/media/i2c/ov2680.txt | 46
+++
1 file changed, 46 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/i2c/ov2680.txt
diff --git
a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..11e925ed9dad
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,46 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
Having a single clock source I think you can omit 'clock-names'
(or at
least not marking it as required)
yeah, I see you point, but really all other OV sensors share this
and
the bellow clock/data-lanes properties as required, I will let Rob
or
Sakari take a call in this one.
---
Cheers,
Rui
+- DOVDD-supply: Digital I/O voltage supply.
+- DVDD-supply: Digital core voltage supply.
+- AVDD-supply: Analog voltage supply.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the
powerdown/reset pin,
+ if any. This is an active low signal to the
OV2680.
+
+The device node must contain one 'port' child node for its
digital output
+video port, and this port must have a single endpoint in
accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint
node.
+- clock-lanes: should be set to <0> (clock lane on hardware
lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
What is the value of marking as required two properties which
can only have
default values (the sensor does not support clock on different
lanes,
nor it supports more than 1 data lane) ?
Thanks
j
+
+Example:
+
+&i2c2 {
+ ov2680: camera-sensor@36 {
+ compatible = "ovti,ov2680";
+ reg = <0x36>;
+ clocks = <&osc>;
+ clock-names = "xvclk";
+ reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
+ DOVDD-supply = <&sw2_reg>;
+ DVDD-supply = <&sw2_reg>;
+ AVDD-supply = <®_peri_3p15v>;
+
+ port {
+ ov2680_to_mipi: endpoint {
+remote-endpoint =
<&mipi_from_sensor>;
+ clock-lanes = <0>;
+ data-lanes = <1>;
+ };
+ };
+ };
+};
--
2.18.0
Re: [PATCH 3/5] media: ov5640: fix wrong binning value in exposure calculation
Hi Hugues,
On Wed, Jul 04, 2018 at 03:29:56PM +, Hugues FRUCHET wrote:
> Hi Jacopo,
>
> Many thanks for you valuable comments, I hardly understand this exposure
> code, and still some wrongly exposed images are observed switching from
> subsampling to scaling modes.
Thank you for the patches...
Just out of curiosity, have you ever been able to get images in 1280x720
and 1920x1080 mode? I assume so if you're able to switch between two
subsampling modes...
> Steve, do you have more insight to share with us on this code ?
>
> On 07/04/2018 04:38 PM, jacopo mondi wrote:
> > Hi Hugues,
> >
> > On Wed, Jul 04, 2018 at 02:58:41PM +0200, Hugues Fruchet wrote:
> >> ov5640_set_mode_exposure_calc() is checking binning value but
> >> binning value read is buggy and binning value set is done
> >> after calling ov5640_set_mode_exposure_calc(), fix all of this.
> >
> > The ov5640_binning_on() function was indeed wrong (side note: that
> > name is confusing, it should be 0v5640_get_binning() to comply with
> > others..) and always returned 0, but I don't see a fix here for the
> > second part of the issue.
> Mistake from me here, I should have removed "and binning value set is
> done after calling ov5640_set_mode_exposure_calc()" in commit message.
>
> > In facts, during the lenghty exposure
> > calculation process, binning is checked to decide if the preview
> > shutter time should be doubled or not
> >
> > static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
> > const struct ov5640_mode_info *mode)
> > {
> > ...
> >
> > /* read preview shutter */
> > ret = ov5640_get_exposure(sensor);
> > if (ret < 0)
> > return ret;
> > prev_shutter = ret;
> > ret = ov5640_binning_on(sensor);
> > if (ret < 0)
> > return ret;
> > if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
> > mode->id != OV5640_MODE_1080P_1920_1080)
> > prev_shutter *= 2;
> > ...
> > }
> >
> > My understanding is that reading the value from the register returns
> > the binning settings for the previously configured mode, while the >
> > binning value is later updated for the current mode in
> > ov5640_set_mode(), after 'ov5640_set_mode_exposure_calc()' has already
> > been called. Is this ok?
>
> This is also my understanding.
>
Thanks. This is probably worth fixing. Maybe your exposure issues
depend on this..
> >
> > Also, I assume the code checks for mode->id to figure out if the mode
> > uses subsampling or scaling. Be aware that for 1280x720 mode, the
> > selected scaling mode depends on the FPS, not only on the mode id as
> > it is assumed here.
>
> This is not what I understand from this array:
> static const struct ov5640_mode_info
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> [15fps]
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>1280, 1892, 720, 740,
>ov5640_setting_15fps_720P_1280_720,
>ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> [30fps]
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>1280, 1892, 720, 740,
>ov5640_setting_30fps_720P_1280_720,
>ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
>
> => both modes uses subsampling here
You are right, I counted the array entries and 30FPS has a -1
specified as downsizing mode in the last one, so I overlooked it, sorry!
So what is mode->id checked for, if 720p and 1080p modes use different
downsizing modes? Confused
>
> >
> > A final note, the 'ov5640_set_mode_exposure_calc()' also writes VTS to
> > update the shutter time to the newly calculated value.
> >
> > /* write capture shutter */
> > if (cap_shutter > (cap_vts - 4)) {
> > cap_vts = cap_shutter + 4;
> > ret = ov5640_set_vts(sensor, cap_vts);
> > if (ret < 0)
> > return ret;
> > }
> >
> > Be aware again that VTS is later restored to the mode->vtot value by
> > the 'ov5640_set_timings()' functions, which again, is called later
> > than 'ov5640_set_mode_exposure_calc()'.
> >
> > Wouldn't it be better to postpone exposure calculation after timings
> > and binnings have been set ?
>
> As said, I'm new on all of this but I can give it a try.
Thanks, I also see banding filter being calculated twice, and I'm sure
there are some other things I'm missing. That exposure calculation
procedure seems poorly integrated with the rest of the set_mode()
function :/
Thanks
j
>
> >
> > Thanks
> > j
> >
> >>
> >> Signed-off-by: Hugues Fruchet
> >> ---
> >> drivers/media/i2c/ov5640.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >> index 7c569de..f9b256e 100644
> >> --- a/drivers/media/i2c/ov5640.c
> >> +++ b/drivers/media/i2c/ov5640.c
> >> @@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct
Re: [PATCH v3 0/3] IOCTLs in ddbridge.
Hi Daniel, Em Sat, 12 May 2018 13:24:29 +0200 Daniel Scheller escreveu: > From: Daniel Scheller > > Third iteration of the IOCTL patches for ddbridge, split into multiple > patches: > > Patch 1 just adds the reservation/information of the used IOCTLs into > ioctl-numbers.txt in the Docs dir. Doc, s390 and LKML are Cc'ed on > this patch. Patch looks ok, although it would be great to get some acks there. I don't know who currently maintains Documentation/ioctl/ioctl-number.txt. Just in case, I would explicitly c/c LKML, Andrew Morton and Jonathan Corbet. Please c/c them on a next respin. > Patch 2 adds the header which defines the IOCTLs in include/uapi/ so > userspace applications can directly reuse the IOCTL definitions by > including this file. > > Patch 3 (re)implements the IOCTL handling into ddbridge. This is > basically code that was there since literally forever, but had to be > removed along with the initial ddbridge-0.9.x bump. Also looked ok. What I miss here is a forth patch to Documentation/media/dvb-drivers/, adding a documentation for ddbridge, in special explaining those new ioctls. > The whole functionality gets more important these days since ie. the > new MaxSX8 cards may require updating from time to time since these > cards implement the demod/tuner communication in their FPGA (which > normally I2C drivers exist for). Also, the CineS2v7 and derivatives > received some important updates and the possibility to receive higher > bitrate transponders these days, so users should be able to update > their cards. > > Changes since the last versions: > - Docs, headers and code split apart and sent out separately to > the subsystems. > - Only the two absolutely necessary IOCTLs (DDB_FLASHIO and DDB_ID) > are implemented for now. > > Daniel Scheller (3): > Documentation: ioctl-number: add ddbridge IOCTLs > [media] ddbridge: uAPI header for IOCTL definitions and related data > structs > [media] ddbridge: implement IOCTL handling > > Documentation/ioctl/ioctl-number.txt| 1 + > MAINTAINERS | 1 + > drivers/media/pci/ddbridge/Makefile | 3 +- > drivers/media/pci/ddbridge/ddbridge-core.c | 111 + > drivers/media/pci/ddbridge/ddbridge-ioctl.c | 179 > > drivers/media/pci/ddbridge/ddbridge-ioctl.h | 32 + > include/uapi/linux/ddbridge-ioctl.h | 61 ++ > 7 files changed, 278 insertions(+), 110 deletions(-) > create mode 100644 drivers/media/pci/ddbridge/ddbridge-ioctl.c > create mode 100644 drivers/media/pci/ddbridge/ddbridge-ioctl.h > create mode 100644 include/uapi/linux/ddbridge-ioctl.h Thanks, Mauro
Re: [PATCH v3 0/3] IOCTLs in ddbridge.
Am Wed, 4 Jul 2018 13:08:31 -0300 schrieb Mauro Carvalho Chehab : > Em Sat, 12 May 2018 13:24:29 +0200 > Daniel Scheller escreveu: > > > From: Daniel Scheller > > > > Third iteration of the IOCTL patches for ddbridge, split into multiple > > patches: > > > > Patch 1 just adds the reservation/information of the used IOCTLs into > > ioctl-numbers.txt in the Docs dir. Doc, s390 and LKML are Cc'ed on > > this patch. > > Patch looks ok, although it would be great to get some acks there. > I don't know who currently maintains Documentation/ioctl/ioctl-number.txt. > > Just in case, I would explicitly c/c LKML, Andrew Morton and Jonathan Corbet. > Please c/c them [...] I did in patch 1 explicitly asking esp. the s390 guys since they're on 0xDD too but at a lower range, no response in two months, from anyone. > on a next respin. [...] What I miss here is a forth patch to > Documentation/media/dvb-drivers/, > adding a documentation for ddbridge, in special explaining those new > ioctls. If you're fine with anything else in this series, please be so kind and pick this stuff up and I'll shove such doc up afterwards as a separate patch. This whole thing is being posted for over a year now, I waited another two months, and I'm not interested in waiting for another such long time (or even more) regarding this (remember I even asked for review for the 4.18 merge window). Best regards, Daniel Scheller -- https://github.com/herrnst
Please update linux-media.tar.bz2
Hello Mauro! I got a request from a user, that media_build does not compile for older Kernels. I checked that and found that the downloaded linux-media.tar.bz2 does not contain "include/linux/overflow.h". Please can you add this file, because media-build needs that file now. BR, Jasmin
Re: [PATCH v3 0/3] IOCTLs in ddbridge.
Hello Mauro! > Patch looks ok, although it would be great to get some acks there. I have no time to test this, but I know Daniel does all this tests and in fact he is currently adapting changes done by DD in their repo, modifying them to work in the current media-tree version and submits them to the list. So this is more or less work to keep the kernel driver in sync with the DD repo. Please remember this from Daniels patch submit message: Patch 3 (re)implements the IOCTL handling into ddbridge. This is basically code that was there since literally forever, but had to be removed along with the initial ddbridge-0.9.x bump. So he is adding back the original ioctl's, which are nowadays more used because of (also from the patch submit message): The whole functionality gets more important these days since ie. the new MaxSX8 cards may require updating from time to time since these cards implement the demod/tuner communication in their FPGA So if you need an ack: Acked-by: Jasmin Jessich > If you're fine with anything else in this series, please be so kind and > pick this stuff up and I'll shove such doc up afterwards as a separate > patch. This whole thing is being posted for over a year now, ... Please Mauro, merge this now and I am sure Daniel will add the missing documentation! Keep also in mind, that we all do this in our spare time and we do this because DD was and is not willing to submit their own drivers! Do your really believe that it is helpful when patches are rodding months for no reason? This is often demotivating and thus some maintainers may give up only because of that. I am extremely happy, that Daniel took over and made it happen, that we get the DD drivers into the Kernel and thus, we can use DD HW and also other HW in parallel. This would be not possible with the drivers from DD. BR, Jasmin
[GIT FIXES FOR v4.18] leaking bpf programs after detach
Hi Mauro, With the syscall bpf(BPF_PROG_ATTACH), a bpf program can be attached to a lirc device; that should increase the refcount so that the program is not freed. However, when we detach the bpf program, we don't decrease the refcount, so the bpf program will never be freed. Tested with kasan and ubsan. The list of bpf programs can be using the bpftool (in the kernel tree), command line "bpftool prog list". Thanks, Sean The following changes since commit 0ca54b29054151b7a52cbb8904732280afe5a302: media: rc: be less noisy when driver misbehaves (2018-06-27 10:03:45 -0400) are available in the Git repository at: git://linuxtv.org/syoung/media_tree.git for-v4.18f for you to fetch changes up to ff003645581b1ee4a0ac80fefdc262a5933b7007: media: bpf: ensure bpf program is freed on detach (2018-07-04 20:48:29 +0100) Sean Young (1): media: bpf: ensure bpf program is freed on detach drivers/media/rc/bpf-lirc.c | 1 + 1 file changed, 1 insertion(+)
[PATCH 1/2] media: dvb: convert tuner_info frequencies to Hz
Right now, satellite tuner drivers specify frequencies in kHz,
while terrestrial/cable ones specify in Hz. That's confusing
for developers.
However, the main problem is that universal tuners capable
of handling both satellite and non-satelite delivery systems
are appearing. We end by needing to hack the drivers in
order to support such hybrid tuners.
So, convert everything to specify tuner frequencies in Hz.
Plese notice that a similar patch is also needed for frontends.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/dvb-core/dvb_frontend.c | 25 ---
drivers/media/dvb-frontends/ascot2e.c | 6 ++---
drivers/media/dvb-frontends/cx24113.c | 8 +++---
drivers/media/dvb-frontends/dib0070.c | 8 +++---
drivers/media/dvb-frontends/dib0090.c | 12 -
drivers/media/dvb-frontends/dvb-pll.c | 16 ++--
drivers/media/dvb-frontends/helene.c | 12 -
drivers/media/dvb-frontends/horus3a.c | 6 ++---
drivers/media/dvb-frontends/itd1000.c | 8 +++---
drivers/media/dvb-frontends/ix2505v.c | 4 +--
drivers/media/dvb-frontends/stb6000.c | 4 +--
drivers/media/dvb-frontends/stb6100.c | 5 ++--
drivers/media/dvb-frontends/stv6110.c | 6 ++---
drivers/media/dvb-frontends/stv6110x.c| 7 +++---
drivers/media/dvb-frontends/stv6111.c | 5 ++--
drivers/media/dvb-frontends/tda18271c2dd.c| 6 ++---
drivers/media/dvb-frontends/tda665x.c | 6 ++---
drivers/media/dvb-frontends/tda8261.c | 9 +++
drivers/media/dvb-frontends/tda826x.c | 4 +--
drivers/media/dvb-frontends/ts2020.c | 4 +--
drivers/media/dvb-frontends/tua6100.c | 6 ++---
drivers/media/dvb-frontends/zl10036.c | 4 +--
drivers/media/tuners/e4000.c | 6 ++---
drivers/media/tuners/fc0011.c | 6 ++---
drivers/media/tuners/fc0012.c | 7 +++---
drivers/media/tuners/fc0013.c | 7 +++---
drivers/media/tuners/fc2580.c | 6 ++---
drivers/media/tuners/it913x.c | 6 ++---
drivers/media/tuners/m88rs6000t.c | 6 ++---
drivers/media/tuners/max2165.c| 8 +++---
drivers/media/tuners/mc44s803.c | 8 +++---
drivers/media/tuners/mt2060.c | 8 +++---
drivers/media/tuners/mt2063.c | 7 +++---
drivers/media/tuners/mt2131.c | 8 +++---
drivers/media/tuners/mt2266.c | 8 +++---
drivers/media/tuners/mxl301rf.c | 4 +--
drivers/media/tuners/mxl5005s.c | 8 +++---
drivers/media/tuners/qm1d1b0004.c | 4 +--
drivers/media/tuners/qm1d1c0042.c | 4 +--
drivers/media/tuners/qt1010.c | 8 +++---
drivers/media/tuners/qt1010_priv.h| 14 ++-
drivers/media/tuners/r820t.c | 6 ++---
drivers/media/tuners/si2157.c | 6 ++---
drivers/media/tuners/tda18212.c | 8 +++---
drivers/media/tuners/tda18218.c | 8 +++---
drivers/media/tuners/tda18250.c | 6 ++---
drivers/media/tuners/tda18271-fe.c| 6 ++---
drivers/media/tuners/tda827x.c| 12 -
drivers/media/tuners/tua9001.c| 6 ++---
drivers/media/tuners/tuner-xc2028.c | 6 ++---
drivers/media/tuners/xc4000.c | 12 -
drivers/media/tuners/xc5000.c | 12 -
drivers/media/usb/dvb-usb-v2/mxl111sf-tuner.c | 6 ++---
include/media/dvb_frontend.h | 19 +++---
54 files changed, 221 insertions(+), 196 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index ce25aef39008..75e95b56f8b3 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -896,14 +896,31 @@ static int dvb_frontend_start(struct dvb_frontend *fe)
static void dvb_frontend_get_frequency_limits(struct dvb_frontend *fe,
u32 *freq_min, u32 *freq_max)
{
- *freq_min = max(fe->ops.info.frequency_min,
fe->ops.tuner_ops.info.frequency_min);
+ struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+ __u32 tuner_min = fe->ops.tuner_ops.info.frequency_min_hz;
+ __u32 tuner_max = fe->ops.tuner_ops.info.frequency_max_hz;
+
+ /* If the standard is for satellite, convert frequencies to kHz */
+ switch (c->delivery_system) {
+ case SYS_DVBS:
+ case SYS_DVBS2:
+ case SYS_TURBO:
+ case SYS_ISDBS:
+ tuner_max /= kHz;
+ tuner_min /= kHz;
+ break;
+ default:
+ break;
+ }
+
+ *freq_min = max(fe->ops.info.frequency_min, tuner_min);
if (fe->ops.info.frequency_max == 0)
- *freq_max
[PATCH 0/2] DVB: represent frequencies at tuner/frontend .info in Hz
This is a long standing issue that affect newer devices whose tuner and frontend are capable of supporting both satellite and non-satellite delivery standards. Compile-tested only. I intend to run some tests later this week with some random hardware, but I'd appreciate any tests and feedback, as it is not hard to break something, but we can't cook an omelet without breaking some eggs. Mauro Carvalho Chehab (2): media: dvb: convert tuner_info frequencies to Hz media: dvb: represent min/max/step/tolerance freqs in Hz drivers/media/common/siano/smsdvb-main.c | 6 +- drivers/media/dvb-core/dvb_frontend.c | 63 +++ drivers/media/dvb-frontends/af9013.c | 7 +-- drivers/media/dvb-frontends/af9033.c | 7 +-- drivers/media/dvb-frontends/as102_fe.c| 6 +- drivers/media/dvb-frontends/ascot2e.c | 6 +- drivers/media/dvb-frontends/atbm8830.c| 6 +- drivers/media/dvb-frontends/au8522_dig.c | 6 +- drivers/media/dvb-frontends/bcm3510.c | 6 +- drivers/media/dvb-frontends/cx22700.c | 6 +- drivers/media/dvb-frontends/cx22702.c | 6 +- drivers/media/dvb-frontends/cx24110.c | 8 +-- drivers/media/dvb-frontends/cx24113.c | 8 +-- drivers/media/dvb-frontends/cx24116.c | 8 +-- drivers/media/dvb-frontends/cx24117.c | 8 +-- drivers/media/dvb-frontends/cx24120.c | 8 +-- drivers/media/dvb-frontends/cx24123.c | 8 +-- drivers/media/dvb-frontends/cxd2820r_t.c | 4 +- drivers/media/dvb-frontends/cxd2820r_t2.c | 4 +- drivers/media/dvb-frontends/cxd2841er.c | 9 ++- .../media/dvb-frontends/cxd2880/cxd2880_top.c | 6 +- drivers/media/dvb-frontends/dib0070.c | 8 +-- drivers/media/dvb-frontends/dib0090.c | 12 ++-- drivers/media/dvb-frontends/dib3000mb.c | 6 +- drivers/media/dvb-frontends/dib3000mc.c | 6 +- drivers/media/dvb-frontends/dib7000m.c| 6 +- drivers/media/dvb-frontends/dib7000p.c| 6 +- drivers/media/dvb-frontends/dib8000.c | 6 +- drivers/media/dvb-frontends/dib9000.c | 6 +- drivers/media/dvb-frontends/drx39xyj/drxj.c | 6 +- drivers/media/dvb-frontends/drxd_hard.c | 7 +-- drivers/media/dvb-frontends/drxk_hard.c | 8 +-- drivers/media/dvb-frontends/ds3000.c | 8 +-- drivers/media/dvb-frontends/dvb-pll.c | 16 - drivers/media/dvb-frontends/dvb_dummy_fe.c| 24 +++ drivers/media/dvb-frontends/gp8psk-fe.c | 6 +- drivers/media/dvb-frontends/helene.c | 12 ++-- drivers/media/dvb-frontends/horus3a.c | 6 +- drivers/media/dvb-frontends/itd1000.c | 8 +-- drivers/media/dvb-frontends/ix2505v.c | 8 +-- drivers/media/dvb-frontends/l64781.c | 7 +-- drivers/media/dvb-frontends/lg2160.c | 12 ++-- drivers/media/dvb-frontends/lgdt3305.c| 12 ++-- drivers/media/dvb-frontends/lgdt3306a.c | 6 +- drivers/media/dvb-frontends/lgdt330x.c| 12 ++-- drivers/media/dvb-frontends/lgs8gl5.c | 7 +-- drivers/media/dvb-frontends/lgs8gxx.c | 6 +- drivers/media/dvb-frontends/m88ds3103.c | 6 +- drivers/media/dvb-frontends/m88rs2000.c | 8 +-- drivers/media/dvb-frontends/mb86a16.c | 7 +-- drivers/media/dvb-frontends/mb86a20s.c| 6 +- drivers/media/dvb-frontends/mt312.c | 10 +-- drivers/media/dvb-frontends/mt352.c | 7 +-- drivers/media/dvb-frontends/mxl5xx.c | 6 +- drivers/media/dvb-frontends/nxt200x.c | 6 +- drivers/media/dvb-frontends/nxt6000.c | 6 +- drivers/media/dvb-frontends/or51132.c | 6 +- drivers/media/dvb-frontends/or51211.c | 8 +-- drivers/media/dvb-frontends/rtl2830.c | 4 +- drivers/media/dvb-frontends/rtl2832.c | 10 +-- drivers/media/dvb-frontends/s5h1409.c | 6 +- drivers/media/dvb-frontends/s5h1411.c | 6 +- drivers/media/dvb-frontends/s5h1420.c | 8 +-- drivers/media/dvb-frontends/s5h1432.c | 6 +- drivers/media/dvb-frontends/s921.c| 7 +-- drivers/media/dvb-frontends/si2165.c | 2 +- drivers/media/dvb-frontends/si21xx.c | 7 +-- drivers/media/dvb-frontends/sp8870.c | 6 +- drivers/media/dvb-frontends/sp887x.c | 6 +- drivers/media/dvb-frontends/stb0899_drv.c | 6 +- drivers/media/dvb-frontends/stb6000.c | 4 +- drivers/media/dvb-frontends/stb6100.c | 5 +- drivers/media/dvb-frontends/stv0288.c | 7 +-- drivers/media/dvb-frontends/stv0297.c | 6 +- drivers/media/dvb-frontends/stv0299.c | 7 +-- drivers/media/dvb-frontends/stv0367.c | 20 +++--- drivers/media/dvb-frontends/stv0900_core.c| 7 +-- drivers/media/dvb-frontends/stv090x.c | 6 +- drivers/media/dvb-frontends/stv0910.c | 6 +- drivers/media/dvb-frontends/stv6110.c | 6 +- d
[PATCH 2/2] media: dvb: represent min/max/step/tolerance freqs in Hz
Right now, satellite frontend drivers specify frequencies in kHz, while terrestrial/cable ones specify in Hz. That's confusing for developers. However, the main problem is that universal frontends capable of handling both satellite and non-satelite delivery systems are appearing. We end by needing to hack the drivers in order to support such hybrid frontends. So, convert everything to specify frontend frequencies in Hz. Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/siano/smsdvb-main.c | 6 +- drivers/media/dvb-core/dvb_frontend.c | 72 --- drivers/media/dvb-frontends/af9013.c | 7 +- drivers/media/dvb-frontends/af9033.c | 7 +- drivers/media/dvb-frontends/as102_fe.c| 6 +- drivers/media/dvb-frontends/atbm8830.c| 6 +- drivers/media/dvb-frontends/au8522_dig.c | 6 +- drivers/media/dvb-frontends/bcm3510.c | 6 +- drivers/media/dvb-frontends/cx22700.c | 6 +- drivers/media/dvb-frontends/cx22702.c | 6 +- drivers/media/dvb-frontends/cx24110.c | 8 +-- drivers/media/dvb-frontends/cx24116.c | 8 +-- drivers/media/dvb-frontends/cx24117.c | 8 +-- drivers/media/dvb-frontends/cx24120.c | 8 +-- drivers/media/dvb-frontends/cx24123.c | 8 +-- drivers/media/dvb-frontends/cxd2820r_t.c | 4 +- drivers/media/dvb-frontends/cxd2820r_t2.c | 4 +- drivers/media/dvb-frontends/cxd2841er.c | 9 ++- .../media/dvb-frontends/cxd2880/cxd2880_top.c | 6 +- drivers/media/dvb-frontends/dib3000mb.c | 6 +- drivers/media/dvb-frontends/dib3000mc.c | 6 +- drivers/media/dvb-frontends/dib7000m.c| 6 +- drivers/media/dvb-frontends/dib7000p.c| 6 +- drivers/media/dvb-frontends/dib8000.c | 6 +- drivers/media/dvb-frontends/dib9000.c | 6 +- drivers/media/dvb-frontends/drx39xyj/drxj.c | 6 +- drivers/media/dvb-frontends/drxd_hard.c | 7 +- drivers/media/dvb-frontends/drxk_hard.c | 8 +-- drivers/media/dvb-frontends/ds3000.c | 8 +-- drivers/media/dvb-frontends/dvb_dummy_fe.c| 24 +++ drivers/media/dvb-frontends/gp8psk-fe.c | 6 +- drivers/media/dvb-frontends/ix2505v.c | 4 +- drivers/media/dvb-frontends/l64781.c | 7 +- drivers/media/dvb-frontends/lg2160.c | 12 ++-- drivers/media/dvb-frontends/lgdt3305.c| 12 ++-- drivers/media/dvb-frontends/lgdt3306a.c | 6 +- drivers/media/dvb-frontends/lgdt330x.c| 12 ++-- drivers/media/dvb-frontends/lgs8gl5.c | 7 +- drivers/media/dvb-frontends/lgs8gxx.c | 6 +- drivers/media/dvb-frontends/m88ds3103.c | 6 +- drivers/media/dvb-frontends/m88rs2000.c | 8 +-- drivers/media/dvb-frontends/mb86a16.c | 7 +- drivers/media/dvb-frontends/mb86a20s.c| 6 +- drivers/media/dvb-frontends/mt312.c | 10 +-- drivers/media/dvb-frontends/mt352.c | 7 +- drivers/media/dvb-frontends/mxl5xx.c | 6 +- drivers/media/dvb-frontends/nxt200x.c | 6 +- drivers/media/dvb-frontends/nxt6000.c | 6 +- drivers/media/dvb-frontends/or51132.c | 6 +- drivers/media/dvb-frontends/or51211.c | 8 +-- drivers/media/dvb-frontends/rtl2830.c | 4 +- drivers/media/dvb-frontends/rtl2832.c | 10 +-- drivers/media/dvb-frontends/s5h1409.c | 6 +- drivers/media/dvb-frontends/s5h1411.c | 6 +- drivers/media/dvb-frontends/s5h1420.c | 8 +-- drivers/media/dvb-frontends/s5h1432.c | 6 +- drivers/media/dvb-frontends/s921.c| 7 +- drivers/media/dvb-frontends/si2165.c | 2 +- drivers/media/dvb-frontends/si21xx.c | 7 +- drivers/media/dvb-frontends/sp8870.c | 6 +- drivers/media/dvb-frontends/sp887x.c | 6 +- drivers/media/dvb-frontends/stb0899_drv.c | 6 +- drivers/media/dvb-frontends/stv0288.c | 7 +- drivers/media/dvb-frontends/stv0297.c | 6 +- drivers/media/dvb-frontends/stv0299.c | 7 +- drivers/media/dvb-frontends/stv0367.c | 20 +++--- drivers/media/dvb-frontends/stv0900_core.c| 7 +- drivers/media/dvb-frontends/stv090x.c | 6 +- drivers/media/dvb-frontends/stv0910.c | 6 +- drivers/media/dvb-frontends/tc90522.c | 10 +-- drivers/media/dvb-frontends/tda10021.c| 10 +-- drivers/media/dvb-frontends/tda10023.c| 6 +- drivers/media/dvb-frontends/tda10048.c| 6 +- drivers/media/dvb-frontends/tda1004x.c| 12 ++-- drivers/media/dvb-frontends/tda10071.c| 10 +-- drivers/media/dvb-frontends/tda10086.c| 6 +- drivers/media/dvb-frontends/tda8083.c | 7 +- drivers/media/dvb-frontends/ves1820.c | 6 +- drivers/media/dvb-frontends/ves1x93.c | 8 +-- drivers/media/dvb-frontends/zl10036.c | 4 +- drivers/media/dvb-frontends/zl10353.c | 7 +- drivers/media/firewire/
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: Thu Jul 5 05:00:11 CEST 2018 media-tree git hash:666e994aa2278e948e2492ee9d81b4df241e7222 media_build git hash: 90c56d1e6345747b6af929867f5b7c16017dcf02 v4l-utils git hash: f29433ed5a0c8679f69c2c9221ab83a8325e3e99 edid-decode git hash: ab18befbcacd6cd4dff63faa82e32700369d6f25 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-2-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.2-i686: OK linux-4.17.2-x86_64: OK linux-4.18-rc1-i686: OK linux-4.18-rc1-x86_64: OK apps: OK spec-git: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH 2/2] media: dvb: represent min/max/step/tolerance freqs in Hz
Hi Mauro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.18-rc3 next-20180704]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/DVB-represent-frequencies-at-tuner-frontend-info-in-Hz/20180705-105703
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x004-201826 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/media/dvb-core/dvb_frontend.c: In function
'dvb_frontend_handle_ioctl':
>> drivers/media/dvb-core/dvb_frontend.c:2396:25: warning: argument to 'sizeof'
>> in 'memset' call is the same expression as the destination; did you mean to
>> dereference it? [-Wsizeof-pointer-memaccess]
memset(info, 0, sizeof(info));
^
vim +2396 drivers/media/dvb-core/dvb_frontend.c
2295
2296 static int dvb_frontend_handle_ioctl(struct file *file,
2297 unsigned int cmd, void *parg)
2298 {
2299 struct dvb_device *dvbdev = file->private_data;
2300 struct dvb_frontend *fe = dvbdev->priv;
2301 struct dvb_frontend_private *fepriv = fe->frontend_priv;
2302 struct dtv_frontend_properties *c = &fe->dtv_property_cache;
2303 int i, err = -ENOTSUPP;
2304
2305 dev_dbg(fe->dvb->device, "%s:\n", __func__);
2306
2307 switch (cmd) {
2308 case FE_SET_PROPERTY: {
2309 struct dtv_properties *tvps = parg;
2310 struct dtv_property *tvp = NULL;
2311
2312 dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
2313 __func__, tvps->num);
2314 dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
2315 __func__, tvps->props);
2316
2317 /*
2318 * Put an arbitrary limit on the number of messages
that can
2319 * be sent at once
2320 */
2321 if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
2322 return -EINVAL;
2323
2324 tvp = memdup_user((void __user *)tvps->props, tvps->num
* sizeof(*tvp));
2325 if (IS_ERR(tvp))
2326 return PTR_ERR(tvp);
2327
2328 for (i = 0; i < tvps->num; i++) {
2329 err = dtv_property_process_set(fe, file,
2330 (tvp + i)->cmd,
2331 (tvp +
i)->u.data);
2332 if (err < 0) {
2333 kfree(tvp);
2334 return err;
2335 }
2336 }
2337 kfree(tvp);
2338 err = 0;
2339 break;
2340 }
2341 case FE_GET_PROPERTY: {
2342 struct dtv_properties *tvps = parg;
2343 struct dtv_property *tvp = NULL;
2344 struct dtv_frontend_properties getp =
fe->dtv_property_cache;
2345
2346 dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
2347 __func__, tvps->num);
2348 dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
2349 __func__, tvps->props);
2350
2351 /*
2352 * Put an arbitrary limit on the number of messages
that can
2353 * be sent at once
2354 */
2355 if (!tvps->num || (tvps->num > DTV_IOCTL_MAX_MSGS))
2356 return -EINVAL;
2357
2358 tvp = memdup_user((void __user *)tvps->props, tvps->num
* sizeof(*tvp));
2359 if (IS_ERR(tvp))
2360 return PTR_ERR(tvp);
2361
2362 /*
2363 * Let's use our own copy of property cache, in order to
2364 * avoid mangling with DTV zigzag logic, as drivers
might
2365 * return crap, if they don't check if the data is
available
2366 * before updating the properties cache.
2367 */
2368 if (fepriv->state != FESTATE_IDLE) {
2369
Re: [PATCH 2/2] media: dvb: represent min/max/step/tolerance freqs in Hz
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.18-rc3 next-20180704] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/DVB-represent-frequencies-at-tuner-frontend-info-in-Hz/20180705-105703 base: git://linuxtv.org/media_tree.git master config: i386-randconfig-x002-201826 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All warnings (new ones prefixed by >>): In file included from arch/x86/include/asm/string.h:3:0, from include/linux/string.h:20, from drivers/media/dvb-core/dvb_frontend.c:30: drivers/media/dvb-core/dvb_frontend.c: In function 'dvb_frontend_handle_ioctl': >> drivers/media/dvb-core/dvb_frontend.c:2396:25: warning: argument to 'sizeof' >> in '__builtin_memset' call is the same expression as the destination; did >> you mean to dereference it? [-Wsizeof-pointer-memaccess] memset(info, 0, sizeof(info)); ^ arch/x86/include/asm/string_32.h:325:52: note: in definition of macro 'memset' #define memset(s, c, count) __builtin_memset(s, c, count) ^ Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32 Cyclomatic Complexity 3 include/linux/log2.h:is_power_of_2 Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_set Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_set Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR Cyclomatic Complexity 1 include/linux/err.h:IS_ERR Cyclomatic Complexity 2 include/linux/thread_info.h:test_ti_thread_flag Cyclomatic Complexity 2 include/linux/thread_info.h:check_object_size Cyclomatic Complexity 2 include/linux/thread_info.h:copy_overflow Cyclomatic Complexity 4 include/linux/thread_info.h:check_copy_size Cyclomatic Complexity 70 include/linux/ktime.h:ktime_divns Cyclomatic Complexity 1 include/linux/ktime.h:ktime_to_us Cyclomatic Complexity 1 include/linux/ktime.h:ktime_us_delta Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us Cyclomatic Complexity 1 include/linux/timekeeping.h:ktime_get_boottime Cyclomatic Complexity 1 include/linux/refcount.h:refcount_set Cyclomatic Complexity 1 arch/x86/include/asm/refcount.h:refcount_inc Cyclomatic Complexity 1 arch/x86/include/asm/refcount.h:refcount_dec_and_test Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info Cyclomatic Complexity 1 include/linux/sched.h:test_tsk_thread_flag Cyclomatic Complexity 1 include/linux/sched/signal.h:signal_pending Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large Cyclomatic Complexity 5 include/linux/slab.h:kmalloc Cyclomatic Complexity 1 include/linux/slab.h:kzalloc Cyclomatic Complexity 1 include/linux/semaphore.h:sema_init Cyclomatic Complexity 2 include/linux/uaccess.h:copy_to_user Cyclomatic Complexity 4 include/linux/poll.h:poll_wait Cyclomatic Complexity 1 include/linux/kref.h:kref_init Cyclomatic Complexity 1 include/linux/kref.h:kref_get Cyclomatic Complexity 2 include/linux/kref.h:kref_put Cyclomatic Complexity 2 include/linux/freezer.h:freezing Cyclomatic Complexity 2 include/linux/freezer.h:try_to_freeze_unsafe Cyclomatic Complexity 2 include/linux/freezer.h:try_to_freeze Cyclomatic Complexity 1 drivers/media/dvb-core/dvb_frontend.c:dvb_frontend_get Cyclomatic Complexity 1 drivers/media/dvb-core/dvb_frontend.c:has_get_frontend Cyclomatic Complexity 5 drivers/media/dvb-core/dvb_frontend.c:dvbv3_type Cyclomatic Complexity 5 drivers/media/dvb-core/dvb_frontend.c:dvb_frontend_init Cyclomatic Complexity 2 drivers/media/dvb-core/dvb_frontend.c:dvb_frontend_swzigzag_update_delay Cyclomatic Complexity 20 drivers/media/dvb-core/dvb_fron
Re: [PATCH 2/2] media: dvb: represent min/max/step/tolerance freqs in Hz
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.18-rc3 next-20180704] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/DVB-represent-frequencies-at-tuner-frontend-info-in-Hz/20180705-105703 base: git://linuxtv.org/media_tree.git master coccinelle warnings: (new ones prefixed by >>) >> drivers/media/dvb-core/dvb_frontend.c:2396:18-24: ERROR: application of >> sizeof to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH] media: dvb: fix noderef.cocci warnings
From: kbuild test robot
drivers/media/dvb-core/dvb_frontend.c:2396:18-24: ERROR: application of sizeof
to pointer
sizeof when applied to a pointer typed expression gives the size of
the pointer
Generated by: scripts/coccinelle/misc/noderef.cocci
Fixes: 07acff734698 ("media: dvb: represent min/max/step/tolerance freqs in Hz")
CC: Mauro Carvalho Chehab
Signed-off-by: kbuild test robot
---
dvb_frontend.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2393,7 +2393,7 @@ static int dvb_frontend_handle_ioctl(str
case FE_GET_INFO: {
struct dvb_frontend_info *info = parg;
- memset(info, 0, sizeof(info));
+ memset(info, 0, sizeof(*info));
dvb_frontend_get_frequency_limits(fe, &info->frequency_min,
&info->frequency_max);
strcpy(info->name, fe->ops.info.name);
