Re: [PATCH v4] s5p-fimc: Add runtime PM support in the mem-to-mem driver
Hi Sylwester,
On Tue, Aug 30, 2011 at 05:00:39PM +0200, Sylwester Nawrocki wrote:
Add runtime PM and system sleep support in the memory-to-memory driver.
This is required to enable the device operation on Exynos4 SoCs. This patch
prevents system boot failure when the driver is compiled in, as it now
tries to access its I/O memory without first enabling the corresponding
power domain.
The camera capture device suspend/resume is not fully covered,
the capture device is just powered on/off during the video node
open/close. However this enables it's normal operation on Exynos4 SoCs.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
I'm resending this patch after few minor changes since v3:
- added the driver dependency on PM_RUNTIME
- corrected the error path in probe()
- s/*_in_use/_busy
- edited the commit message
---
drivers/media/video/Kconfig |2 +-
drivers/media/video/s5p-fimc/fimc-capture.c | 18 ++
drivers/media/video/s5p-fimc/fimc-core.c| 279
---
drivers/media/video/s5p-fimc/fimc-core.h| 16 +-
drivers/media/video/s5p-fimc/fimc-reg.c |2 +-
5 files changed, 237 insertions(+), 80 deletions(-)
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index f574dc0..14326d7 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -950,7 +950,7 @@ config VIDEO_MX2
config VIDEO_SAMSUNG_S5P_FIMC
tristate Samsung S5P and EXYNOS4 camera host interface driver
- depends on VIDEO_DEV VIDEO_V4L2 PLAT_S5P
+ depends on VIDEO_V4L2 PLAT_S5P PM_RUNTIME
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
---help---
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 0d730e5..ea74e4b 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -17,6 +17,7 @@
#include linux/interrupt.h
#include linux/device.h
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include linux/list.h
#include linux/slab.h
#include linux/clk.h
@@ -196,6 +197,16 @@ static int fimc_stop_capture(struct fimc_dev *fimc)
return 0;
}
+int fimc_capture_suspend(struct fimc_dev *fimc)
+{
+ return -EBUSY;
+}
+
+int fimc_capture_resume(struct fimc_dev *fimc)
+{
+ return 0;
+}
+
static int start_streaming(struct vb2_queue *q)
{
struct fimc_ctx *ctx = q-drv_priv;
@@ -381,9 +392,14 @@ static int fimc_capture_open(struct file *file)
if (fimc_m2m_active(fimc))
return -EBUSY;
+ ret = pm_runtime_get_sync(fimc-pdev-dev);
+ if (ret)
+ return ret;
+
if (++fimc-vid_cap.refcnt == 1) {
ret = fimc_isp_subdev_init(fimc, 0);
if (ret) {
+ pm_runtime_put_sync(fimc-pdev-dev);
fimc-vid_cap.refcnt--;
return -EIO;
}
@@ -411,6 +427,8 @@ static int fimc_capture_close(struct file *file)
fimc_subdev_unregister(fimc);
}
+ pm_runtime_put_sync(fimc-pdev-dev);
+
return 0;
}
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index aa55066..7ca8091 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -18,6 +18,7 @@
#include linux/interrupt.h
#include linux/device.h
#include linux/platform_device.h
+#include linux/pm_runtime.h
#include linux/list.h
#include linux/io.h
#include linux/slab.h
@@ -301,7 +302,6 @@ int fimc_set_scaler_info(struct fimc_ctx *ctx)
static void fimc_m2m_job_finish(struct fimc_ctx *ctx, int vb_state)
{
struct vb2_buffer *src_vb, *dst_vb;
- struct fimc_dev *fimc = ctx-fimc_dev;
if (!ctx || !ctx-m2m_ctx)
return;
@@ -312,39 +312,48 @@ static void fimc_m2m_job_finish(struct fimc_ctx *ctx,
int vb_state)
if (src_vb dst_vb) {
v4l2_m2m_buf_done(src_vb, vb_state);
v4l2_m2m_buf_done(dst_vb, vb_state);
- v4l2_m2m_job_finish(fimc-m2m.m2m_dev, ctx-m2m_ctx);
+ v4l2_m2m_job_finish(ctx-fimc_dev-m2m.m2m_dev,
+ ctx-m2m_ctx);
}
}
/* Complete the transaction which has been scheduled for execution. */
-static void fimc_m2m_shutdown(struct fimc_ctx *ctx)
+static int fimc_m2m_shutdown(struct fimc_ctx *ctx)
{
struct fimc_dev *fimc = ctx-fimc_dev;
int ret;
if (!fimc_m2m_pending(fimc))
- return;
+ return 0;
fimc_ctx_state_lock_set(FIMC_CTX_SHUT, ctx);
ret = wait_event_timeout(fimc-irq_queue,
!fimc_ctx_state_is_set(FIMC_CTX_SHUT, ctx),
Re: spca1528 device (Device 015: ID 04fc:1528 Sunplus Technology)..libv4l2: error turning on stream: Timer expired issue
On Sun, 04 Sep 2011 15:39:30 -0400 Mauricio Henriquez buhochil...@gmail.com wrote: Recently I'm trying to make work a Sunplus crappy mini HD USB camera, lsusb list this info related to the device: Picture Transfer Protocol (PIMA 15470) Bus 001 Device 015: ID 04fc:1528 Sunplus Technology Co., Ltd idVendor 0x04fc Sunplus Technology Co., Ltd idProduct 0x1528 bcdDevice1.00 iManufacturer 1 Sunplus Co Ltd iProduct2 General Image Devic iSerial 0 ... Using the gspca-2.13.6 on my Fed12 (2.6.31.6-166.fc12.i686.PAE kernel), the device is listed as /dev/video1 and no error doing a dmesg...but trying to make it work, let say with xawtv, I get: [snip] Hi Mauricio, The problem seems tied to the alternate setting. It must be the #3 while the lastest versions of gspca compute a best one. May you apply the following patch to gspca-2.13.6? --8-- --- build/spca1528.c.orig 2011-09-05 08:41:54.0 +0200 +++ build/spca1528.c2011-09-05 08:53:51.0 +0200 @@ -307,8 +307,6 @@ sd-color = COLOR_DEF; sd-sharpness = SHARPNESS_DEF; - gspca_dev-nbalt = 4; /* use alternate setting 3 */ - return 0; } @@ -349,6 +347,9 @@ reg_r(gspca_dev, 0x25, 0x0004, 1); reg_wb(gspca_dev, 0x27, 0x, 0x, 0x06); reg_r(gspca_dev, 0x27, 0x, 1); + + gspca_dev-alt = 4; /* use alternate setting 3 */ + return gspca_dev-usb_err; } --8-- (Theodore, this webcam may work in mass storage mode with ID 04fc:0171. In webcam mode with ID 04fc:1528, it offers 3 interfaces: interface 0 contains only an interrupt endpoint, interface 1 is the webcam with only isochronous endpoints and interface 2 contains bulk in, bulk out and interrupt in endpoints - I don't know how to use the interfaces 0 and 2, but sure the interface 2 could be used to access the camera images) -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: DiBxxxx: fixes for 3.1/3.0
Hello Mauro,
I agree with you but when I wrote this patch, my concern was that the read
register function (dib0070_read_reg) returns a u16 and so I could not propagate
the error. That's why I decided to return 0 and not change the API. But if you
have a better idea, I will have no problem to implement it.
For the write register function (dib0070_write_reg), in case of problem with
the mutex lock, an error code is returned.
The same idea is true for the whole patch.
regards,
Olivier
From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org] On
Behalf Of Mauro Carvalho Chehab [mche...@infradead.org]
Sent: Sunday, September 04, 2011 2:45 AM
To: Patrick Boettcher
Cc: Linux Media Mailing List
Subject: Re: DiB: fixes for 3.1/3.0
Em 03-08-2011 12:33, Patrick Boettcher escreveu:
Hi Mauro,
Thanks for the patches!
Would you please pull from
git://linuxtv.org/pb/media_tree.git for_v3.0
for the following to changesets:
[media] dib0700: protect the dib0700 buffer access
-static uint16_t dib0070_read_reg(struct dib0070_state *state, u8 reg)
+static u16 dib0070_read_reg(struct dib0070_state *state, u8 reg)
{
+ u16 ret;
+
+ if (mutex_lock_interruptible(state-i2c_buffer_lock) 0) {
+ dprintk(could not acquire lock);
+ return 0;
Returning 0 doesn't seem right for me. IMO, it should be return -EAGAIN
or -EINTR (which is, incidentally, what mutex_lock_interruptible() will
return).
The same applies to the similar parts of the code, at the read and write
routines.
[media] DiBcom: protect the I2C bufer access
?
Those two changesets are fixing the remaining problems regarding the
dma-on-stack-buffer-fix applied for the first time in 2.6.39, IIRC.
They should go to stable 3.0 (as they are in my tree) and they can be
cherry-picked to 3.1.
We are preparing the same thing for 2.6.39 as the patches don't apply cleanly.
Thanks to Javier Marcet for his help during the debug-phase.
thanks and best regards,
--
Patrick Boettcher - Kernel Labs
http://www.kernellabs.com/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
CONFIDENTIAL NOTICE: The contents of this message, including any attachments,
are confidential and are intended solely for the use of the person or entity to
whom the message was addressed. If you are not the intended recipient of this
message, please be advised that any dissemination, distribution, or use of the
contents of this message is strictly prohibited. If you received this message
in error, please notify the sender. Please also permanently delete all copies
of the original message and any attached documentation. Thank you.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] at91: add Atmel ISI and ov2640 support on m10/g45 board.
On 09/03/2011 2:22 AM Jean-Christophe PLAGNIOL-VILLARD wrote:
#include asm/setup.h
#include asm/mach-types.h
@@ -194,6 +197,95 @@ static void __init ek_add_device_nand(void)
at91_add_device_nand(ek_nand_data);
}
+/*
+ * ISI
+ */
+#if defined(CONFIG_VIDEO_ATMEL_ISI) ||
defined(CONFIG_VIDEO_ATMEL_ISI_MODULE)
+static struct isi_platform_data __initdata isi_data = {
+.frate = ISI_CFG1_FRATE_CAPTURE_ALL,
+.has_emb_sync = 0,
+.emb_crc_sync = 0,
+.hsync_act_low = 0,
+.vsync_act_low = 0,
+.pclk_act_falling = 0,
+/* to use codec and preview path simultaneously */
+.isi_full_mode = 1,
+.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10,
+};
+
+static void __init isi_set_clk(void)
+{
+struct clk *pck1;
+struct clk *plla;
+
+pck1 = clk_get(NULL, pck1);
+plla = clk_get(NULL, plla);
+
+clk_set_parent(pck1, plla);
+clk_set_rate(pck1, 2500);
+clk_enable(pck1);
you must not enable the clock always
you must enable it just when you need it
and manage the clock at the board level really so so
I see, I will move such clock code to atmel_isi.c driver and add clock name,
clock frequence to isi_platform_data structure in next version.
Thanks.
Best Regards,
Josh Wu
Best Regards,
J.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hello Laurent,
2011/8/31 Laurent Pinchart laurent.pinch...@ideasonboard.com:
Hi Bastian,
Guennadi pointed out that should can sound a bit harsh, so please read my
reviews as if
#define you should I think you should
I think that you think I should do the right thing. I removed
out_sizes and repost v3 in a moment :)
was prepended to all of them :-)
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard 1280x720
only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client, ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current format
without performing any change or writing anything to the device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
- return 0;
+ return ret;
}
[snip]
@@ -827,15 +874,42 @@ static int ov5642_g_chip_ident(struct v4l2_subdev
[snip]
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height = OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
+ *rect = priv-crop_rect;
return 0;
}
--
Regards,
Laurent Pinchart
best,
Bastian
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 v3] media: Add support for arbitrary resolution for the ov5642 camera driver
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard 1280x720
only.
---
diff --git a/drivers/media/video/ov5642.c b/drivers/media/video/ov5642.c
index 6410bda..54c822c 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
@@ -14,8 +14,10 @@
* published by the Free Software Foundation.
*/
+#include linux/bitops.h
#include linux/delay.h
#include linux/i2c.h
+#include linux/kernel.h
#include linux/slab.h
#include linux/videodev2.h
@@ -34,7 +36,7 @@
#define REG_WINDOW_START_Y_LOW 0x3803
#define REG_WINDOW_WIDTH_HIGH 0x3804
#define REG_WINDOW_WIDTH_LOW 0x3805
-#define REG_WINDOW_HEIGHT_HIGH 0x3806
+#define REG_WINDOW_HEIGHT_HIGH 0x3806
#define REG_WINDOW_HEIGHT_LOW 0x3807
#define REG_OUT_WIDTH_HIGH 0x3808
#define REG_OUT_WIDTH_LOW 0x3809
@@ -44,19 +46,44 @@
#define REG_OUT_TOTAL_WIDTH_LOW0x380d
#define REG_OUT_TOTAL_HEIGHT_HIGH 0x380e
#define REG_OUT_TOTAL_HEIGHT_LOW 0x380f
+#define REG_OUTPUT_FORMAT 0x4300
+#define REG_ISP_CTRL_010x5001
+#define REG_AVG_WINDOW_END_X_HIGH 0x5682
+#define REG_AVG_WINDOW_END_X_LOW 0x5683
+#define REG_AVG_WINDOW_END_Y_HIGH 0x5686
+#define REG_AVG_WINDOW_END_Y_LOW 0x5687
+
+/* active pixel array size */
+#define OV5642_SENSOR_SIZE_X 2592
+#define OV5642_SENSOR_SIZE_Y 1944
/*
- * define standard resolution.
- * Works currently only for up to 720 lines
- * eg. 320x240, 640x480, 800x600, 1280x720, 2048x720
+ * About OV5642 resolution, cropping and binning:
+ * This sensor supports it all, at least in the feature description.
+ * Unfortunately, no combination of appropriate registers settings could make
+ * the chip work the intended way. As it works with predefined register lists,
+ * some undocumented registers are presumably changed there to achieve their
+ * goals.
+ * This driver currently only works for resolutions up to 720 lines with a
+ * 1:1 scale. Hopefully these restrictions will be removed in the future.
*/
+#define OV5642_MAX_WIDTH OV5642_SENSOR_SIZE_X
+#define OV5642_MAX_HEIGHT 720
-#define OV5642_WIDTH 1280
-#define OV5642_HEIGHT 720
-#define OV5642_TOTAL_WIDTH 3200
-#define OV5642_TOTAL_HEIGHT2000
-#define OV5642_SENSOR_SIZE_X 2592
-#define OV5642_SENSOR_SIZE_Y 1944
+/* default sizes */
+#define OV5642_DEFAULT_WIDTH 1280
+#define OV5642_DEFAULT_HEIGHT OV5642_MAX_HEIGHT
+
+/* minimum extra blanking */
+#define BLANKING_EXTRA_WIDTH 500
+#define BLANKING_EXTRA_HEIGHT 20
+
+/*
+ * the sensor's autoexposure is buggy when setting total_height low.
+ * It tries to expose longer than 1 frame period without taking care of it
+ * and this leads to weird output. So we set 1000 lines as minimum.
+ */
+#define BLANKING_MIN_HEIGHT1000
struct regval_list {
u16 reg_num;
@@ -581,6 +608,11 @@ struct ov5642_datafmt {
struct ov5642 {
struct v4l2_subdev subdev;
const struct ov5642_datafmt *fmt;
+ struct v4l2_rectcrop_rect;
+
+ /* blanking information */
+ int total_width;
+ int total_height;
};
static const struct ov5642_datafmt ov5642_colour_fmts[] = {
@@ -641,6 +673,21 @@ static int reg_write(struct i2c_client *client, u16 reg,
u8 val)
return 0;
}
+
+/*
+ * convenience function to write 16 bit register values that are split up
+ * into two consecutive high and low parts
+ */
+static int reg_write16(struct i2c_client *client, u16 reg, u16 val16)
+{
+ int ret;
+
+ ret = reg_write(client, reg, val16 8);
+ if (ret)
+ return ret;
+ return reg_write(client, reg + 1, val16 0x00ff);
+}
+
#ifdef CONFIG_VIDEO_ADV_DEBUG
static int ov5642_get_register(struct v4l2_subdev *sd, struct
v4l2_dbg_register *reg)
{
@@ -684,107 +731,101 @@ static int ov5642_write_array(struct i2c_client *client,
return 0;
}
-static int ov5642_set_resolution(struct i2c_client *client)
+static int ov5642_set_resolution(struct v4l2_subdev *sd)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
+ int width = priv-crop_rect.width;
+ int height = priv-crop_rect.height;
+ int total_width = priv-total_width;
+ int total_height = priv-total_height;
+ int start_x = (OV5642_SENSOR_SIZE_X - width) / 2;
+ int start_y = (OV5642_SENSOR_SIZE_Y - height) / 2;
int ret;
- u8 start_x_high = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) 8;
- u8 start_x_low = ((OV5642_SENSOR_SIZE_X - OV5642_WIDTH) / 2) 0xff;
- u8 start_y_high = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) 8;
- u8 start_y_low = ((OV5642_SENSOR_SIZE_Y - OV5642_HEIGHT) / 2) 0xff;
-
- u8 width_high =
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hi Bastian,
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
Hi Bastian,
Guennadi pointed out that should can sound a bit harsh, so please read
my reviews as if
#define you should I think you should
I think that you think I should do the right thing. I removed out_sizes and
repost v3 in a moment :)
Thanks :-)
was prepended to all of them :-)
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard
1280x720 only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c
b/drivers/media/video/ov5642.c index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct
i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client,
ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current
format without performing any change or writing anything to the device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
Sure, but calling s_fmt isn't mandatory for hosts/bridges. What about moving
sensor initialization to s_stream() ?
- return 0;
+ return ret;
}
[snip]
@@ -827,15 +874,42 @@ static int ov5642_g_chip_ident(struct
v4l2_subdev
[snip]
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height= OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an EINVAL
error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't. However I
believe the above should apply to VIDIOC_G_CROP as well. There is no explicit
documentation about error codes for subdev operations, but I think it makes
sense to follow what the V4L2 ioctls do.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
2011/9/1 Sakari Ailus sakari.ai...@iki.fi: On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote: Hi Sakari, On 09/01/2011 10:47 AM, Sakari Ailus wrote: On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski wrote: On Thu, 1 Sep 2011, Sakari Ailus wrote: On Wed, Aug 31, 2011 at 03:27:49PM +, Bastian Hecht wrote: 2011/8/28 Laurent Pinchart laurent.pinch...@ideasonboard.com: [clip] If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated. I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled V4L2_CID_PRIVATE_BASE deprecated and read Documentation/feature-removal-schedule.txt. I couldn't find anything. Hmm. Did you happen to check when that has been written? :) Please use this one instead: URL:http://hverkuil.home.xs4all.nl/spec/media.html Drivers can also implement their own custom controls using V4L2_CID_PRIVATE_BASE and higher values. Which specific location describes V4L2_CID_PRIVATE_BASE differently there? That was a general comment, not related to the private base. There's no use for a three-year-old spec as a reference! The control framework does not support private controls, for example. The controls should be put to their own class in videodev2.h nowadays, that's my understanding. Cc Hans. Is this really the case that we close the door for private controls in the mainline kernel ? Or am I misunderstanding something ? How about v4l2_ctrl_new_custom() ? What if there are controls applicable to single driver only ? Do we really want to have plenty of such in videodev2.h ? We have some of those already in videodev2.h. I'm not certain if I'm happy with this myself, considering e.g. that we could get a few truckloads of only camera lens hardware specific controls in the near future. So in my case (as these are controls that might be used by others too) I should add something like #define V4L2_CID_BLUE_SATURATION(V4L2_CID_CAMERA_CLASS_BASE+19) #define V4L2_CID_RED_SATURATION (V4L2_CID_CAMERA_CLASS_BASE+20) #define V4L2_CID_GRAY_SCALE_IMAGE (V4L2_CID_CAMERA_CLASS_BASE+21) #define V4L2_CID_SOLARIZE_EFFECT(V4L2_CID_CAMERA_CLASS_BASE+22) to videodev2.h? -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
2011/9/5 Laurent Pinchart laurent.pinch...@ideasonboard.com:
Hi Bastian,
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
Hi Bastian,
Guennadi pointed out that should can sound a bit harsh, so please read
my reviews as if
#define you should I think you should
I think that you think I should do the right thing. I removed out_sizes and
repost v3 in a moment :)
Thanks :-)
was prepended to all of them :-)
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard
1280x720 only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c
b/drivers/media/video/ov5642.c index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct
i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client,
ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current
format without performing any change or writing anything to the device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
Sure, but calling s_fmt isn't mandatory for hosts/bridges. What about moving
sensor initialization to s_stream() ?
- return 0;
+ return ret;
}
[snip]
@@ -827,15 +874,42 @@ static int ov5642_g_chip_ident(struct
v4l2_subdev
[snip]
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height = OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an EINVAL
error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't. However I
believe the above should apply to VIDIOC_G_CROP as well. There is no explicit
documentation about error codes for subdev operations, but I think it makes
sense to follow what the V4L2 ioctls do.
And these ioctl calls go straight through to my driver? Or is there
some intermediate work by the subdev architecture? I'm asking because
I don't check the buffer type in g_fmt as well. If so, I have to
change that too.
best,
Bastian
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hi Bastian,
On Monday 05 September 2011 11:41:28 Bastian Hecht wrote:
2011/9/5 Laurent Pinchart:
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop
*a) {
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height= OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an EINVAL
error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't.
However I believe the above should apply to VIDIOC_G_CROP as well. There
is no explicit documentation about error codes for subdev operations,
but I think it makes sense to follow what the V4L2 ioctls do.
And these ioctl calls go straight through to my driver? Or is there
some intermediate work by the subdev architecture? I'm asking because
I don't check the buffer type in g_fmt as well. If so, I have to
change that too.
The ioctls go to the host/bridge driver, which then decides when and how to
call g/s_fmt and g/s_crop. I would add the same check to g_fmt.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hello Laurent,
2011/9/5 Laurent Pinchart laurent.pinch...@ideasonboard.com:
Hi Bastian,
On Monday 05 September 2011 11:41:28 Bastian Hecht wrote:
2011/9/5 Laurent Pinchart:
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop
*a) {
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height = OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an EINVAL
error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't.
However I believe the above should apply to VIDIOC_G_CROP as well. There
is no explicit documentation about error codes for subdev operations,
but I think it makes sense to follow what the V4L2 ioctls do.
And these ioctl calls go straight through to my driver? Or is there
some intermediate work by the subdev architecture? I'm asking because
I don't check the buffer type in g_fmt as well. If so, I have to
change that too.
The ioctls go to the host/bridge driver, which then decides when and how to
call g/s_fmt and g/s_crop. I would add the same check to g_fmt.
Next time I will work in the media section of the kernel, I must study
the docs a bit better I guess :/
To the g_fmt() thing... is there the buffer type given at all? The
argument of type struct v4l2_mbus_framefmt doesn't contain it.
best,
Bastian
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
On Mon, 5 Sep 2011, Laurent Pinchart wrote:
Hi Bastian,
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
Hi Bastian,
Guennadi pointed out that should can sound a bit harsh, so please read
my reviews as if
#define you should I think you should
I think that you think I should do the right thing. I removed out_sizes and
repost v3 in a moment :)
Thanks :-)
was prepended to all of them :-)
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a width
up to 2592 and a height up to 720 pixels instead of the standard
1280x720 only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c
b/drivers/media/video/ov5642.c index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct
i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client,
ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current
format without performing any change or writing anything to the device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
Sure, but calling s_fmt isn't mandatory for hosts/bridges. What about moving
sensor initialization to s_stream() ?
Throughout the development of this driver, I was opposing the delayed
configuration approach. I.e., the approach, in which all the ioctl()s,
like S_FMT, S_CROP, etc. only store user values internally, and the actual
hardware configuration is only performed at STREAMON time. There are
several reasons to this: the spec says the driver may program the
hardware, allocate resources and generally prepare for data exchange
(yes, may != must), most drivers seem to do the same, the possibility
to check and return any hardware errors, returned by this operation, I
probably have forgotten something. But if we ignore all these reasons as
insufficiently important, then yes, doing the actualy hardware
configuration in .s_stream() brings a couple of advantages with it,
especially for drivers / devices like this one.
So, if there are no strong objections, maybe indeed move this back to
.s_stream() would be the better solution here.
Thanks
Guennadi
- return 0;
+ return ret;
}
[snip]
@@ -827,15 +874,42 @@ static int ov5642_g_chip_ident(struct
v4l2_subdev
[snip]
static int ov5642_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height= OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an EINVAL
error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't. However I
believe the above should apply to VIDIOC_G_CROP as well. There is no explicit
documentation about error codes for subdev operations, but I think it makes
sense to follow what the V4L2 ioctls do.
--
Regards,
Laurent Pinchart
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
Hi Hans,
On Friday 02 September 2011 09:29:08 Sitsofe Wheeler wrote:
On Fri, Sep 02, 2011 at 01:35:49PM +0800, Dave Young wrote:
On Fri, Sep 2, 2011 at 12:59 PM, Dave Young wrote:
On Fri, Sep 2, 2011 at 3:10 AM, Sitsofe Wheeler wrote:
On Thu, Sep 01, 2011 at 05:02:51PM +0800, Dave Young wrote:
On Tue, Aug 30, 2011 at 4:48 AM, Sitsofe Wheeler wrote:
I managed to produce an oops in 3.1.0-rc3-00270-g7a54f5e by
unplugging a
USB webcam. See below:
Could you try the attached patch?
This patch fixed the oops but extending the sequence (enable camera,
start cheese, disable camera, watch cheese pause, enable camera, quit
cheese, start cheese) causes the following poison overwritten
warning
to appear:
It seems another bug, I can reproduce this as well.
uvc_device is freed in uvc_delete,
struct v4l2_device vdev is the member of struct uvc_device, so vdev is
also freed. Later v4l2_device operations on vdev will overwrite the
poison memory area.
Please try attached patch on top of previous one, in this patch I
move v4l2_device_put after vdev-release in function
v4l2_device_release
Not sure if this is a right fix, comments?
(inlining the patch's contents)
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-
dev.c
index 98cee19..541dba3 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -172,13 +172,14 @@ static void v4l2_device_release(struct device *cd)
media_device_unregister_entity(vdev-entity);
#endif
+ /* Decrease v4l2_device refcount */
+ if (vdev-v4l2_dev)
+ v4l2_device_put(vdev-v4l2_dev);
+
/* Release video_device and perform other
cleanups as needed. */
vdev-release(vdev);
- /* Decrease v4l2_device refcount */
- if (vdev-v4l2_dev)
- v4l2_device_put(vdev-v4l2_dev);
}
static struct class video_class = {
v4l2_device_put() got introduced in commit
bedf8bcf6b4f90a6e31add3721a2e71877289381 (v4l2-device: add kref and a release
function). If I understand its purpose correctly, drivers that use a
v4l2_device instance should use the v4l2_device release callback to release
device structures instead of counting video_device release callbacks manually.
In that case I think the v4l2-dev.c code is correct, and all drivers that use
v4l2_device should be fixed.
The above patch fixes the problem in a central location, but seems to defeat
the original purpose of v4l2_device_get/put().
Hans, could you please comment on that ?
[ 191.240695] uvcvideo: Found UVC 1.00 device CNF7129 (04f2:b071)
[ 191.277965] input: CNF7129 as
/devices/pci:00/:00:1d.7/usb1/1-8/1-8:1.0/input/input9 [
220.287366]
=
[ 220.287379] BUG kmalloc-512: Poison overwritten
[ 220.287384]
-
[ 220.287387]
[ 220.287394] INFO: 0xec90f150-0xec90f150. First byte 0x6a instead of
0x6b [ 220.287410] INFO: Allocated in uvc_probe+0x54/0xd50
age=210617 cpu=0 pid=16 [ 220.287421] T.974+0x29d/0x5e0
[ 220.287427] kmem_cache_alloc+0x167/0x180
[ 220.287433] uvc_probe+0x54/0xd50
[ 220.287441] usb_probe_interface+0xd5/0x1d0
[ 220.287448] driver_probe_device+0x80/0x1a0
[ 220.287455] __device_attach+0x41/0x50
[ 220.287460] bus_for_each_drv+0x53/0x80
[ 220.287466] device_attach+0x89/0xa0
[ 220.287472] bus_probe_device+0x25/0x40
[ 220.287478] device_add+0x5a9/0x660
[ 220.287484] usb_set_configuration+0x562/0x670
[ 220.287491] generic_probe+0x36/0x90
[ 220.287497] usb_probe_device+0x24/0x50
[ 220.287503] driver_probe_device+0x80/0x1a0
[ 220.287509] __device_attach+0x41/0x50
[ 220.287515] bus_for_each_drv+0x53/0x80
[ 220.287522] INFO: Freed in uvc_delete+0xfe/0x110 age=22 cpu=0
pid=1645 [ 220.287530] __slab_free+0x1f8/0x300
[ 220.287536] kfree+0x100/0x140
[ 220.287541] uvc_delete+0xfe/0x110
[ 220.287547] uvc_release+0x25/0x30
[ 220.287555] v4l2_device_release+0x9d/0xc0
[ 220.287560] device_release+0x19/0x90
[ 220.287567] kobject_release+0x3c/0x90
[ 220.287573] kref_put+0x2c/0x60
[ 220.287578] kobject_put+0x1d/0x50
[ 220.287587] put_device+0xf/0x20
[ 220.287593] v4l2_release+0x56/0x60
[ 220.287599] fput+0xcc/0x220
[ 220.287605] filp_close+0x44/0x70
[ 220.287613] put_files_struct+0x158/0x180
[ 220.287619] exit_files+0x40/0x50
[snip]
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
On Mon, 5 Sep 2011, Bastian Hecht wrote: [snip] And these ioctl calls go straight through to my driver? Or is there some intermediate work by the subdev architecture? I'm asking because I don't check the buffer type in g_fmt as well. If so, I have to change that too. With soc-camera all ioctl()s first enter in soc_camera.c (check soc_camera_ioctl_ops), then they are typically dispatched to the camera host driver, which then calls respective subdev methods. Check respective ioctl() implementations for details. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hi Bastian,
On Monday 05 September 2011 11:51:52 Bastian Hecht wrote:
2011/9/5 Laurent Pinchart:
On Monday 05 September 2011 11:41:28 Bastian Hecht wrote:
2011/9/5 Laurent Pinchart:
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
static int ov5642_g_crop(struct v4l2_subdev *sd, struct
v4l2_crop *a) {
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
struct v4l2_rect *rect = a-c;
- a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- rect-top = 0;
- rect-left = 0;
- rect-width = OV5642_WIDTH;
- rect-height= OV5642_HEIGHT;
+ a-type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
Shouldn't you return an error instead when a-type is not
V4L2_BUF_TYPE_VIDEO_CAPTURE ?
No idea, but if you say so, I'll change it.
VIDIOC_G_FMT documentation states that
When the requested buffer type is not supported drivers return an
EINVAL error code.
I thought VIDIOC_G_CROP documentation did as well, but it doesn't.
However I believe the above should apply to VIDIOC_G_CROP as well.
There is no explicit documentation about error codes for subdev
operations, but I think it makes sense to follow what the V4L2 ioctls
do.
And these ioctl calls go straight through to my driver? Or is there
some intermediate work by the subdev architecture? I'm asking because
I don't check the buffer type in g_fmt as well. If so, I have to
change that too.
The ioctls go to the host/bridge driver, which then decides when and how
to call g/s_fmt and g/s_crop. I would add the same check to g_fmt.
Next time I will work in the media section of the kernel, I must study
the docs a bit better I guess :/
Studying the docs isn't very difficult, the real problem is understanding what
is not written in the documentation :-)
To the g_fmt() thing... is there the buffer type given at all? The
argument of type struct v4l2_mbus_framefmt doesn't contain it.
Oops, my bad. You're right, there's no buffer type argument to the mbus
g/s_fmt calls. Sorry for the mistake.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
Hi Guennadi,
On Monday 05 September 2011 11:51:57 Guennadi Liakhovetski wrote:
On Mon, 5 Sep 2011, Laurent Pinchart wrote:
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a
width up to 2592 and a height up to 720 pixels instead of the
standard 1280x720 only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c
b/drivers/media/video/ov5642.c index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct
i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client,
ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current
format without performing any change or writing anything to the
device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
Sure, but calling s_fmt isn't mandatory for hosts/bridges. What about
moving sensor initialization to s_stream() ?
Throughout the development of this driver, I was opposing the delayed
configuration approach. I.e., the approach, in which all the ioctl()s,
like S_FMT, S_CROP, etc. only store user values internally, and the actual
hardware configuration is only performed at STREAMON time. There are
several reasons to this: the spec says the driver may program the
hardware, allocate resources and generally prepare for data exchange
(yes, may != must), most drivers seem to do the same, the possibility
to check and return any hardware errors, returned by this operation, I
probably have forgotten something. But if we ignore all these reasons as
insufficiently important, then yes, doing the actualy hardware
configuration in .s_stream() brings a couple of advantages with it,
especially for drivers / devices like this one.
So, if there are no strong objections, maybe indeed move this back to
.s_stream() would be the better solution here.
I have no strong opinion here. Your points are certainly valid. I'm fine with
performing direct hardware setup in .s_crop(), but doing it in .s_fmt() looks
weird to me as .s_fmt() doesn't perform any operation now that the driver
moved to using .s_crop(). Without delayed initialization I believe the device
should be initialized when powered up, and have its crop rectangle altered in
.s_crop().
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
On Monday, September 05, 2011 11:59:03 Laurent Pinchart wrote:
Hi Hans,
On Friday 02 September 2011 09:29:08 Sitsofe Wheeler wrote:
On Fri, Sep 02, 2011 at 01:35:49PM +0800, Dave Young wrote:
On Fri, Sep 2, 2011 at 12:59 PM, Dave Young wrote:
On Fri, Sep 2, 2011 at 3:10 AM, Sitsofe Wheeler wrote:
On Thu, Sep 01, 2011 at 05:02:51PM +0800, Dave Young wrote:
On Tue, Aug 30, 2011 at 4:48 AM, Sitsofe Wheeler wrote:
I managed to produce an oops in 3.1.0-rc3-00270-g7a54f5e by
unplugging a
USB webcam. See below:
Could you try the attached patch?
This patch fixed the oops but extending the sequence (enable camera,
start cheese, disable camera, watch cheese pause, enable camera, quit
cheese, start cheese) causes the following poison overwritten
warning
to appear:
It seems another bug, I can reproduce this as well.
uvc_device is freed in uvc_delete,
struct v4l2_device vdev is the member of struct uvc_device, so vdev is
also freed. Later v4l2_device operations on vdev will overwrite the
poison memory area.
Please try attached patch on top of previous one, in this patch I
move v4l2_device_put after vdev-release in function
v4l2_device_release
Not sure if this is a right fix, comments?
(inlining the patch's contents)
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-
dev.c
index 98cee19..541dba3 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -172,13 +172,14 @@ static void v4l2_device_release(struct device *cd)
media_device_unregister_entity(vdev-entity);
#endif
+ /* Decrease v4l2_device refcount */
+ if (vdev-v4l2_dev)
+ v4l2_device_put(vdev-v4l2_dev);
+
/* Release video_device and perform other
cleanups as needed. */
vdev-release(vdev);
- /* Decrease v4l2_device refcount */
- if (vdev-v4l2_dev)
- v4l2_device_put(vdev-v4l2_dev);
}
static struct class video_class = {
v4l2_device_put() got introduced in commit
bedf8bcf6b4f90a6e31add3721a2e71877289381 (v4l2-device: add kref and a
release
function). If I understand its purpose correctly, drivers that use a
v4l2_device instance should use the v4l2_device release callback to release
device structures instead of counting video_device release callbacks
manually.
In that case I think the v4l2-dev.c code is correct, and all drivers that use
v4l2_device should be fixed.
The above patch fixes the problem in a central location, but seems to defeat
the original purpose of v4l2_device_get/put().
Hans, could you please comment on that ?
The original order is correct, but what I missed is that for drivers that
release
(free) everything in the videodev release callback the v4l2_device struct is
also freed and v4l2_device_put will fail.
To fix this, add this code just before the vdev-release call:
/* Do not call v4l2_device_put if there is no release callback set. */
if (v4l2_dev-release == NULL)
v4l2_dev = NULL;
If there is no release callback, then the refcounting is pointless anyway.
This should work.
Regards,
Hans
[ 191.240695] uvcvideo: Found UVC 1.00 device CNF7129 (04f2:b071)
[ 191.277965] input: CNF7129 as
/devices/pci:00/:00:1d.7/usb1/1-8/1-8:1.0/input/input9 [
220.287366]
=
[ 220.287379] BUG kmalloc-512: Poison overwritten
[ 220.287384]
-
[ 220.287387]
[ 220.287394] INFO: 0xec90f150-0xec90f150. First byte 0x6a instead of
0x6b [ 220.287410] INFO: Allocated in uvc_probe+0x54/0xd50
age=210617 cpu=0 pid=16 [ 220.287421] T.974+0x29d/0x5e0
[ 220.287427] kmem_cache_alloc+0x167/0x180
[ 220.287433] uvc_probe+0x54/0xd50
[ 220.287441] usb_probe_interface+0xd5/0x1d0
[ 220.287448] driver_probe_device+0x80/0x1a0
[ 220.287455] __device_attach+0x41/0x50
[ 220.287460] bus_for_each_drv+0x53/0x80
[ 220.287466] device_attach+0x89/0xa0
[ 220.287472] bus_probe_device+0x25/0x40
[ 220.287478] device_add+0x5a9/0x660
[ 220.287484] usb_set_configuration+0x562/0x670
[ 220.287491] generic_probe+0x36/0x90
[ 220.287497] usb_probe_device+0x24/0x50
[ 220.287503] driver_probe_device+0x80/0x1a0
[ 220.287509] __device_attach+0x41/0x50
[ 220.287515] bus_for_each_drv+0x53/0x80
[ 220.287522] INFO: Freed in uvc_delete+0xfe/0x110 age=22 cpu=0
pid=1645 [ 220.287530] __slab_free+0x1f8/0x300
[ 220.287536] kfree+0x100/0x140
[ 220.287541] uvc_delete+0xfe/0x110
[ 220.287547] uvc_release+0x25/0x30
[ 220.287555] v4l2_device_release+0x9d/0xc0
[ 220.287560] device_release+0x19/0x90
[ 220.287567]
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
On Monday, September 05, 2011 12:13:26 Hans Verkuil wrote:
On Monday, September 05, 2011 11:59:03 Laurent Pinchart wrote:
Hi Hans,
On Friday 02 September 2011 09:29:08 Sitsofe Wheeler wrote:
On Fri, Sep 02, 2011 at 01:35:49PM +0800, Dave Young wrote:
On Fri, Sep 2, 2011 at 12:59 PM, Dave Young wrote:
On Fri, Sep 2, 2011 at 3:10 AM, Sitsofe Wheeler wrote:
On Thu, Sep 01, 2011 at 05:02:51PM +0800, Dave Young wrote:
On Tue, Aug 30, 2011 at 4:48 AM, Sitsofe Wheeler wrote:
I managed to produce an oops in 3.1.0-rc3-00270-g7a54f5e by
unplugging a
USB webcam. See below:
Could you try the attached patch?
This patch fixed the oops but extending the sequence (enable camera,
start cheese, disable camera, watch cheese pause, enable camera, quit
cheese, start cheese) causes the following poison overwritten
warning
to appear:
It seems another bug, I can reproduce this as well.
uvc_device is freed in uvc_delete,
struct v4l2_device vdev is the member of struct uvc_device, so vdev is
also freed. Later v4l2_device operations on vdev will overwrite the
poison memory area.
Please try attached patch on top of previous one, in this patch I
move v4l2_device_put after vdev-release in function
v4l2_device_release
Not sure if this is a right fix, comments?
(inlining the patch's contents)
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-
dev.c
index 98cee19..541dba3 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -172,13 +172,14 @@ static void v4l2_device_release(struct device *cd)
media_device_unregister_entity(vdev-entity);
#endif
+ /* Decrease v4l2_device refcount */
+ if (vdev-v4l2_dev)
+ v4l2_device_put(vdev-v4l2_dev);
+
/* Release video_device and perform other
cleanups as needed. */
vdev-release(vdev);
- /* Decrease v4l2_device refcount */
- if (vdev-v4l2_dev)
- v4l2_device_put(vdev-v4l2_dev);
}
static struct class video_class = {
v4l2_device_put() got introduced in commit
bedf8bcf6b4f90a6e31add3721a2e71877289381 (v4l2-device: add kref and a
release
function). If I understand its purpose correctly, drivers that use a
v4l2_device instance should use the v4l2_device release callback to release
device structures instead of counting video_device release callbacks
manually.
In that case I think the v4l2-dev.c code is correct, and all drivers that
use
v4l2_device should be fixed.
The above patch fixes the problem in a central location, but seems to
defeat
the original purpose of v4l2_device_get/put().
Hans, could you please comment on that ?
The original order is correct, but what I missed is that for drivers that
release
(free) everything in the videodev release callback the v4l2_device struct is
also freed and v4l2_device_put will fail.
To fix this, add this code just before the vdev-release call:
/* Do not call v4l2_device_put if there is no release callback set. */
if (v4l2_dev-release == NULL)
v4l2_dev = NULL;
If there is no release callback, then the refcounting is pointless anyway.
This should work.
Note that in the long run using the v4l2_device release callback instead of the
videodev release is better. But it's a lot of work to convert everything so
that's
long term. I'm quite surprised BTW that this bug wasn't found much earlier.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] v4l: add support for selection api
Hi Tomasz,
Thanks for the patch!
On Wed, Aug 31, 2011 at 02:28:20PM +0200, Tomasz Stanislawski wrote:
This patch introduces new api for a precise control of cropping and composing
features for video devices. The new ioctls are VIDIOC_S_SELECTION and
VIDIOC_G_SELECTION.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |2 +
drivers/media/video/v4l2-ioctl.c | 28 +
include/linux/videodev2.h | 46
+
include/media/v4l2-ioctl.h|4 ++
4 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c
index 61979b7..f3b9d15 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -927,6 +927,8 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int
cmd, unsigned long arg)
case VIDIOC_CROPCAP:
case VIDIOC_G_CROP:
case VIDIOC_S_CROP:
+ case VIDIOC_G_SELECTION:
+ case VIDIOC_S_SELECTION:
case VIDIOC_G_JPEGCOMP:
case VIDIOC_S_JPEGCOMP:
case VIDIOC_QUERYSTD:
diff --git a/drivers/media/video/v4l2-ioctl.c
b/drivers/media/video/v4l2-ioctl.c
index 002ce13..6e02b45 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -225,6 +225,8 @@ static const char *v4l2_ioctls[] = {
[_IOC_NR(VIDIOC_CROPCAP)] = VIDIOC_CROPCAP,
[_IOC_NR(VIDIOC_G_CROP)] = VIDIOC_G_CROP,
[_IOC_NR(VIDIOC_S_CROP)] = VIDIOC_S_CROP,
+ [_IOC_NR(VIDIOC_G_SELECTION)] = VIDIOC_G_SELECTION,
+ [_IOC_NR(VIDIOC_S_SELECTION)] = VIDIOC_S_SELECTION,
[_IOC_NR(VIDIOC_G_JPEGCOMP)] = VIDIOC_G_JPEGCOMP,
[_IOC_NR(VIDIOC_S_JPEGCOMP)] = VIDIOC_S_JPEGCOMP,
[_IOC_NR(VIDIOC_QUERYSTD)] = VIDIOC_QUERYSTD,
@@ -1714,6 +1716,32 @@ static long __video_do_ioctl(struct file *file,
ret = ops-vidioc_s_crop(file, fh, p);
break;
}
+ case VIDIOC_G_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_g_selection)
+ break;
+
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+
+ ret = ops-vidioc_g_selection(file, fh, p);
+ if (!ret)
+ dbgrect(vfd, , p-r);
+ break;
+ }
+ case VIDIOC_S_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_s_selection)
+ break;
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+ dbgrect(vfd, , p-r);
+
+ ret = ops-vidioc_s_selection(file, fh, p);
+ break;
+ }
case VIDIOC_CROPCAP:
{
struct v4l2_cropcap *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..b7471fe 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -738,6 +738,48 @@ struct v4l2_crop {
struct v4l2_rectc;
};
+/* Hints for adjustments of selection rectangle */
+#define V4L2_SEL_SIZE_GE 0x0001
+#define V4L2_SEL_SIZE_LE 0x0002
+
+/* Selection targets */
+
+/* current cropping area */
+#define V4L2_SEL_CROP_ACTIVE 0
+/* default cropping area */
+#define V4L2_SEL_CROP_DEFAULT1
+/* cropping bounds */
+#define V4L2_SEL_CROP_BOUNDS 2
+/* current composing area */
+#define V4L2_SEL_COMPOSE_ACTIVE 256
+/* default composing area */
+#define V4L2_SEL_COMPOSE_DEFAULT 257
+/* composing bounds */
+#define V4L2_SEL_COMPOSE_BOUNDS 258
+/* current composing area plus all padding pixels */
+#define V4L2_SEL_COMPOSE_PADDED 259
+
+/**
+ * struct v4l2_selection - selection info
+ * @type:buffer type (do not use *_MPLANE types)
+ * @target: selection target, used to choose one of possible rectangles
+ * @flags: constraints flags
+ * @r: coordinates of selection window
+ * @reserved:for future use, rounds structure size to 64 bytes, set
to zero
+ *
+ * Hardware may use multiple helper window to process a video stream.
+ * The structure is used to exchange this selection areas between
+ * an application and a driver.
+ */
+struct v4l2_selection {
+ __u32 type;
+ __u32 target;
+ __u32 flags;
+ struct v4l2_rectr;
+ __u32 reserved[9];
+};
The v4l2_selection doesn't have which field such as v4l2_subdev_crop and
v4l2_subdev_format. This field is used to differentiate between try and
active format / crop. Shouldn't we use the same approach in selection?
+
[PATCH] [media] at91: add code to initialize and manage the ISI_MCK for Atmel ISI driver.
This patch enable the configuration for ISI_MCK, which is provided by
programmable clock.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/video/atmel-isi.c | 56 ++-
include/media/atmel-isi.h |4 +++
2 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 7b89f00..8bb59a8 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -90,7 +90,10 @@ struct atmel_isi {
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
struct completion complete;
+ /* ISI peripherial clock */
struct clk *pclk;
+ /* ISI_MCK, provided by PCK */
+ struct clk *mck;
unsigned intirq;
struct isi_platform_data*pdata;
@@ -763,6 +766,10 @@ static int isi_camera_add_device(struct soc_camera_device
*icd)
if (ret)
return ret;
+ ret = clk_enable(isi-mck);
+ if (ret)
+ return ret;
+
isi-icd = icd;
dev_dbg(icd-parent, Atmel ISI Camera driver attached to camera %d\n,
icd-devnum);
@@ -776,6 +783,7 @@ static void isi_camera_remove_device(struct
soc_camera_device *icd)
BUG_ON(icd != isi-icd);
+ clk_disable(isi-mck);
clk_disable(isi-pclk);
isi-icd = NULL;
@@ -882,6 +890,46 @@ static struct soc_camera_host_ops isi_soc_camera_host_ops
= {
};
/* ---*/
+static int initialize_mck(struct atmel_isi *isi,
+ struct isi_platform_data *pdata)
+{
+ int ret;
+ struct clk *pck_parent;
+
+ if (!strlen(pdata-pck_name) || !strlen(pdata-pck_parent_name))
+ return -EINVAL;
+
+ /* ISI_MCK is provided by PCK clock */
+ isi-mck = clk_get(NULL, pdata-pck_name);
+ if (IS_ERR(isi-mck)) {
+ printk(KERN_ERR Failed to get PCK: %s\n, pdata-pck_name);
+ return PTR_ERR(isi-mck);
+ }
+
+ pck_parent = clk_get(NULL, pdata-pck_parent_name);
+ if (IS_ERR(pck_parent)) {
+ ret = PTR_ERR(pck_parent);
+ printk(KERN_ERR Failed to get PCK parent: %s\n,
+ pdata-pck_parent_name);
+ goto err_init_mck;
+ }
+
+ ret = clk_set_parent(isi-mck, pck_parent);
+ clk_put(pck_parent);
+ if (ret)
+ goto err_init_mck;
+
+ ret = clk_set_rate(isi-mck, pdata-isi_mck_hz);
+ if (ret 0)
+ goto err_init_mck;
+
+ return 0;
+
+err_init_mck:
+ clk_put(isi-mck);
+ return ret;
+}
+
static int __devexit atmel_isi_remove(struct platform_device *pdev)
{
struct soc_camera_host *soc_host = to_soc_camera_host(pdev-dev);
@@ -897,6 +945,7 @@ static int __devexit atmel_isi_remove(struct
platform_device *pdev)
isi-fb_descriptors_phys);
iounmap(isi-regs);
+ clk_put(isi-mck);
clk_put(isi-pclk);
kfree(isi);
@@ -915,7 +964,8 @@ static int __devinit atmel_isi_probe(struct platform_device
*pdev)
struct isi_platform_data *pdata;
pdata = dev-platform_data;
- if (!pdata || !pdata-data_width_flags) {
+ if (!pdata || !pdata-data_width_flags || !pdata-isi_mck_hz
+ || !pdata-pck_name || !pdata-pck_parent_name) {
dev_err(pdev-dev,
No config available for Atmel ISI\n);
return -EINVAL;
@@ -944,6 +994,10 @@ static int __devinit atmel_isi_probe(struct
platform_device *pdev)
INIT_LIST_HEAD(isi-video_buffer_list);
INIT_LIST_HEAD(isi-dma_desc_head);
+ ret = initialize_mck(isi, pdata);
+ if (ret)
+ goto err_alloc_descriptors;
+
isi-p_fb_descriptors = dma_alloc_coherent(pdev-dev,
sizeof(struct fbd) * MAX_BUFFER_NUM,
isi-fb_descriptors_phys,
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 26cece5..dcbb822 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -114,6 +114,10 @@ struct isi_platform_data {
u32 data_width_flags;
/* Using for ISI_CFG1 */
u32 frate;
+ /* Using for ISI_MCK, provided by PCK */
+ u32 isi_mck_hz;
+ const char *pck_name;
+ const char *pck_parent_name;
};
#endif /* __ATMEL_ISI_H__ */
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media-ctl][PATCHv4 3/3] libmediactl: get the device name via udev
Hi Andy,
On Friday 02 September 2011 15:09:28 Andy Shevchenko wrote:
If configured with --with-libudev, the libmediactl is built with libudev
support. It allows to get the device name in right way in the modern linux
systems.
Thanks for the patch. We're nearly there :-)
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
configure.in| 22
src/Makefile.am |2 +
src/media.c | 73
+++ src/media.h |
1 +
4 files changed, 98 insertions(+), 0 deletions(-)
diff --git a/configure.in b/configure.in
index fd4c70c..983023e 100644
--- a/configure.in
+++ b/configure.in
@@ -13,6 +13,28 @@ AC_PROG_LIBTOOL
# Checks for libraries.
+AC_ARG_WITH([libudev],
+AS_HELP_STRING([--with-libudev],
+[Enable libudev to detect a device name]))
+
+AS_IF([test x$with_libudev = xyes],
+[PKG_CHECK_MODULES(libudev, libudev, have_libudev=yes,
have_libudev=no)], +[have_libudev=no])
+
+AS_IF([test x$have_libudev = xyes],
+[
+AC_DEFINE([HAVE_LIBUDEV], [], [Use libudev])
+LIBUDEV_CFLAGS=$libudev_CFLAGS
+LIBUDEV_LIBS=$libudev_LIBS
+AC_SUBST(LIBUDEV_CFLAGS)
+AC_SUBST(LIBUDEV_LIBS)
+],
+[AS_IF([test x$with_libudev = xyes],
+[AC_MSG_ERROR([libudev requested but not found])
+])
+])
+
+
# Kernel headers path.
AC_ARG_WITH(kernel-headers,
[AC_HELP_STRING([--with-kernel-headers=DIR],
diff --git a/src/Makefile.am b/src/Makefile.am
index 267ea83..52628d2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -5,6 +5,8 @@ mediactl_includedir=$(includedir)/mediactl
mediactl_include_HEADERS = media.h subdev.h
bin_PROGRAMS = media-ctl
+media_ctl_CFLAGS = $(LIBUDEV_CFLAGS)
+media_ctl_LDFLAGS = $(LIBUDEV_LIBS)
media_ctl_SOURCES = main.c options.c options.h tools.h
media_ctl_LDADD = libmediactl.la libv4l2subdev.la
diff --git a/src/media.c b/src/media.c
index 5d3ff7c..dae649a 100644
--- a/src/media.c
+++ b/src/media.c
@@ -17,6 +17,8 @@
* with this program; if not, write to the Free Software Foundation, Inc.,
*/
+#include config.h
+
#include sys/ioctl.h
#include sys/stat.h
#include sys/types.h
@@ -245,6 +247,64 @@ static int media_enum_links(struct media_device
*media) return ret;
}
+#ifdef HAVE_LIBUDEV
+
+#include libudev.h
+
+static inline int media_udev_open(struct media_device *media)
+{
+ media-priv = udev_new();
+ if (media-priv == NULL)
+ return -ENOMEM;
+ return 0;
+}
+
+static inline void media_udev_close(struct media_device *media)
+{
+ udev_unref(media-priv);
+}
+
+static int media_get_devname_udev(struct media_device *media,
+ struct media_entity *entity)
+{
+ int ret = -ENODEV;
+ struct udev *udev = media-priv;
+ dev_t devnum;
+ struct udev_device *device;
+ const char *p;
+
+ if (udev == NULL)
+ return -EINVAL;
+
+ devnum = makedev(entity-info.v4l.major, entity-info.v4l.minor);
+ printf(looking up device: %u:%u\n, major(devnum), minor(devnum));
+ device = udev_device_new_from_devnum(udev, 'c', devnum);
+ if (device) {
+ p = udev_device_get_devnode(device);
+ if (p)
+ snprintf(entity-devname, sizeof(entity-devname),
%s, p);
+ ret = 0;
+ }
+
+ udev_device_unref(device);
+
+ return ret;
+}
+
+#else/* HAVE_LIBUDEV */
+
+static inline int media_udev_open(struct media_device *media) { return 0;
} +
+static inline void media_udev_close(struct media_device *media) { }
+
+static inline int media_get_devname_udev(struct media_device *media,
+ struct media_entity *entity)
+{
+ return -ENOTSUP;
+}
+
+#endif /* HAVE_LIBUDEV */
+
static int media_get_devname_sysfs(struct media_entity *entity)
{
struct stat devstat;
@@ -324,6 +384,11 @@ static int media_enum_entities(struct media_device
*media) media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV)
continue;
+ /* Try to get the device name via udev */
+ if (!media_get_devname_udev(media, entity))
+ continue;
+
+ /* Fall back to get the device name via sysfs */
media_get_devname_sysfs(entity);
}
@@ -351,6 +416,13 @@ struct media_device *media_open(const char *name, int
verbose) return NULL;
}
+ ret = media_udev_open(media);
+ if (ret 0) {
+ printf(%s: Can't get udev context\n, __func__);
+ media_close(media);
+ return NULL;
+ }
+
if (verbose)
printf(Enumerating entities\n);
@@ -395,6 +467,7 @@ void media_close(struct media_device *media)
}
free(media-entities);
+ media_udev_close(media);
free(media);
}
diff --git a/src/media.h
Re: [PATCH] [media] at91: add code to initialize and manage the ISI_MCK for Atmel ISI driver.
On Mon, Sep 05, 2011 at 06:29:53PM +0800, Josh Wu wrote:
+static int initialize_mck(struct atmel_isi *isi,
+ struct isi_platform_data *pdata)
+{
+ int ret;
+ struct clk *pck_parent;
+
+ if (!strlen(pdata-pck_name) || !strlen(pdata-pck_parent_name))
+ return -EINVAL;
+
+ /* ISI_MCK is provided by PCK clock */
+ isi-mck = clk_get(NULL, pdata-pck_name);
No, this is not how you use the clk API. You do not pass clock names via
platform data.
You pass clk_get() the struct device. You then pass clk_get() a
_connection id_ on that _device_ if you have more than one struct clk
associated with the _device_. You then use clkdev to associate the
struct device plus the connection id with the appropriate struct clk.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media: Add camera controls for the ov5642 driver
2011/8/31 Guennadi Liakhovetski g.liakhovet...@gmx.de:
On Wed, 31 Aug 2011, Bastian Hecht wrote:
2011/8/28 Laurent Pinchart laurent.pinch...@ideasonboard.com:
Hi Bastian,
Thanks for the patch.
On Wednesday 17 August 2011 18:02:07 Bastian Hecht wrote:
The driver now supports automatic/manual gain, automatic/manual white
balance, automatic/manual exposure control, vertical flip, brightness
control, contrast control and saturation control. Additionally the
following effects are available now: rotating the hue in the colorspace,
gray scale image and solarize effect.
Any chance to port soc-camera to the control framework ? :-)
I redirect that to Guennadi :-)
Hans is prepaing an update of his port, then we'll integrate it... This
all will take time, so, it's better to do this driver now the old
soc-camera way, and port it later.
[snip]
+static int ov5642_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control
*ctrl) +{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ struct ov5642 *priv = to_ov5642(client);
+ int ret = 0;
+ u8 val8;
+ u16 val16;
+ u32 val32;
+ int trig;
+ struct v4l2_control aux_ctrl;
+
+ switch (ctrl-id) {
+ case V4L2_CID_AUTOGAIN:
+ if (!ctrl-value) {
+ aux_ctrl.id = V4L2_CID_GAIN;
+ ret = ov5642_g_ctrl(sd, aux_ctrl);
+ if (ret)
+ break;
+ priv-gain = aux_ctrl.value;
+ }
+
+ ret = reg_read(client, REG_EXP_GAIN_CTRL, val8);
+ if (ret)
+ break;
+ val8 = ctrl-value ? val8 ~BIT(1) : val8 | BIT(1);
+ ret = reg_write(client, REG_EXP_GAIN_CTRL, val8);
+ if (!ret)
+ priv-agc = ctrl-value;
What about caching the content of this register (and of other registers
below)
instead of reading it back ? If you can't do that, a reg_clr_set() function
would make the code simpler.
Ok I will do the caching.
Wasn't the reason for reading those registers from the hardware, that the
sensor changes them in auto* modes (autogain in this case)?
There are different cases. E.g. this register sets the autogain and
autoexposure on/off not the gain value itself. So indeed one should
cache it (sometimes I registered side-effects on the ov5642 - setting
1 register changed others. I hope I won't run into trouble with
these).
best,
Bastian
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] media: Add support for arbitrary resolution for the ov5642 camera driver
2011/9/5 Laurent Pinchart laurent.pinch...@ideasonboard.com:
Hi Guennadi,
On Monday 05 September 2011 11:51:57 Guennadi Liakhovetski wrote:
On Mon, 5 Sep 2011, Laurent Pinchart wrote:
On Monday 05 September 2011 11:10:48 Bastian Hecht wrote:
2011/8/31 Laurent Pinchart:
On Wednesday 31 August 2011 19:06:25 Laurent Pinchart wrote:
On Wednesday 31 August 2011 17:05:52 Bastian Hecht wrote:
This patch adds the ability to get arbitrary resolutions with a
width up to 2592 and a height up to 720 pixels instead of the
standard 1280x720 only.
Signed-off-by: Bastian Hecht hec...@gmail.com
---
diff --git a/drivers/media/video/ov5642.c
b/drivers/media/video/ov5642.c index 6410bda..87b432e 100644
--- a/drivers/media/video/ov5642.c
+++ b/drivers/media/video/ov5642.c
[snip]
@@ -684,107 +737,101 @@ static int ov5642_write_array(struct
i2c_client
[snip]
-static int ov5642_s_fmt(struct v4l2_subdev *sd,
- struct v4l2_mbus_framefmt *mf)
+static int ov5642_s_fmt(struct v4l2_subdev *sd, struct
v4l2_mbus_framefmt *mf) {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov5642 *priv = to_ov5642(client);
-
- dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
+ int ret;
/* MIPI CSI could have changed the format, double-check */
if (!ov5642_find_datafmt(mf-code))
return -EINVAL;
ov5642_try_fmt(sd, mf);
-
priv-fmt = ov5642_find_datafmt(mf-code);
- ov5642_write_array(client, ov5642_default_regs_init);
- ov5642_set_resolution(client);
- ov5642_write_array(client, ov5642_default_regs_finalise);
+ ret = ov5642_write_array(client, ov5642_default_regs_init);
+ if (!ret)
+ ret = ov5642_set_resolution(sd);
+ if (!ret)
+ ret = ov5642_write_array(client,
ov5642_default_regs_finalise);
You shouldn't write anything to the sensor here. As only .s_crop can
currently change the format, .s_fmt should just return the current
format without performing any change or writing anything to the
device.
We talked about it in the ov5642 controls thread. I need to initialize
the sensor at some point and it doesn't work to divide the calls
between different locations.
Sure, but calling s_fmt isn't mandatory for hosts/bridges. What about
moving sensor initialization to s_stream() ?
Throughout the development of this driver, I was opposing the delayed
configuration approach. I.e., the approach, in which all the ioctl()s,
like S_FMT, S_CROP, etc. only store user values internally, and the actual
hardware configuration is only performed at STREAMON time. There are
several reasons to this: the spec says the driver may program the
hardware, allocate resources and generally prepare for data exchange
(yes, may != must), most drivers seem to do the same, the possibility
to check and return any hardware errors, returned by this operation, I
probably have forgotten something. But if we ignore all these reasons as
insufficiently important, then yes, doing the actualy hardware
configuration in .s_stream() brings a couple of advantages with it,
especially for drivers / devices like this one.
So, if there are no strong objections, maybe indeed move this back to
.s_stream() would be the better solution here.
I have no strong opinion here. Your points are certainly valid. I'm fine with
performing direct hardware setup in .s_crop(), but doing it in .s_fmt() looks
weird to me as .s_fmt() doesn't perform any operation now that the driver
moved to using .s_crop(). Without delayed initialization I believe the device
should be initialized when powered up, and have its crop rectangle altered in
.s_crop().
Ok, it is moved to s_power and s_crop now. This approach sounds clean indeed.
best,
Bastian
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] misc: remove CONFIG_MISC_DEVICES
Hi Arnd, On Fri, 2 Sep 2011 16:43:14 +0200, Arnd Bergmann wrote: Since misc devices have nothing in common besides fitting in no other category, there is no need to group them in one Kconfig symbol. Simply removing the symbol gets rid of all sorts of Kconfig warnings about missing dependencies when another driver selects a misc driver without also selecting MISC_DEVICES. Signed-off-by: Arnd Bergmann a...@arndb.de --- arch/arm/mach-davinci/Kconfig |6 -- arch/unicore32/Kconfig|1 - drivers/misc/Kconfig | 26 -- drivers/mmc/host/Kconfig |1 - 4 files changed, 8 insertions(+), 26 deletions(-) (...) --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -2,23 +2,7 @@ # Misc strange devices # -# This one has to live outside of the MISC_DEVICES conditional, -# because it may be selected by drivers/platform/x86/hp_accel. -config SENSORS_LIS3LV02D - tristate - depends on INPUT - select INPUT_POLLDEV - default n - -menuconfig MISC_DEVICES - bool Misc devices - ---help--- - Say Y here to get to see options for device drivers from various - different categories. This option alone does not add any kernel code. - - If you say N, all options in this submenu will be skipped and disabled. - -if MISC_DEVICES +menu Misc devices As said before, I'm not sure. Yes, it makes it easier to select misc device drivers from Kconfig files. But it also makes it impossible to deselect all misc device drivers at once. I think that what we really need is the implementation in the Kconfig system of smart selects, i.e. whenever an entry is selected, everything it depends on gets selected as well. I don't know how feasible this is, but if it can be done then I'd prefer this to your proposal. Meanwhile, I am not in favor of applying your patch. The benefit is relatively small IMHO (misc device drivers are rarely selected) and there is one significant drawback. That being said, I'm not the one to decide, so if you can convince someone with more power (aka Andrew Morton)... config AD525X_DPOT tristate Analog Devices Digital Potentiometers @@ -344,6 +328,12 @@ config ISL29020 This driver can also be built as a module. If so, the module will be called isl29020. +config SENSORS_LIS3LV02D + tristate + depends on INPUT + select INPUT_POLLDEV + default n + If you patch gets applied, then this one would better be moved to drivers/misc/lis3lv02d/Kconfig. -- Jean Delvare -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] ISP:BUILD:FIX: Move media_entity_init() and
Hi Mauro, On Sunday 04 September 2011 15:32:28 Mauro Carvalho Chehab wrote: Em 04-09-2011 06:01, Laurent Pinchart escreveu: On Sunday 04 September 2011 00:21:38 Mauro Carvalho Chehab wrote: Em 24-08-2011 10:25, Laurent Pinchart escreveu: On Wednesday 24 August 2011 14:19:01 Hiremath, Vaibhav wrote: On Wednesday, August 24, 2011 5:00 PM Laurent Pinchart wrote: On Wednesday 24 August 2011 13:21:27 Ravi, Deepthy wrote: On Wed, Aug 24, 2011 at 4:47 PM, Laurent Pinchart wrote: On Friday 19 August 2011 15:48:45 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com Fix the build break caused when CONFIG_MEDIA_CONTROLLER option is disabled and if any sensor driver has to be used between MC and non MC framework compatible devices. For example,if tvp514x video decoder driver migrated to MC framework is being built without CONFIG_MEDIA_CONTROLLER option enabled, the following error messages will result. drivers/built-in.o: In function `tvp514x_remove': drivers/media/video/tvp514x.c:1285: undefined reference to `media_entity_cleanup' drivers/built-in.o: In function `tvp514x_probe': drivers/media/video/tvp514x.c:1237: undefined reference to `media_entity_init' If the tvp514x is migrated to the MC framework, its Kconfig option should depend on MEDIA_CONTROLLER. The same TVP514x driver is being used for both MC and non MC compatible devices, for example OMAP3 and AM35x. So if it is made dependent on MEDIA CONTROLLER, we cannot enable the driver for MC independent devices. Then you should use conditional compilation in the tvp514x driver itself. Or No. I am not in favor of conditional compilation in driver code. Actually, thinking some more about this, you should make the tvp514x driver depend on CONFIG_MEDIA_CONTROLLER unconditionally. This doesn't mean that the driver will become unusable by applications that are not MC-aware. Hosts/bridges don't have to export subdev nodes, they can just call subdev pad-level operations internally and let applications control the whole device through a single V4L2 video node. better, port the AM35x driver to the MC API. Why should we use MC if I have very simple device (like AM35x) which only supports single path? I can very well use simple V4L2 sub-dev based approach (master - slave), isn't it? The AM35x driver should use the in-kernel MC and V4L2 subdev APIs, but it doesn't have to expose them to userspace. I don't agree. If AM35x doesn't expose the MC API to userspace, CONFIG_MEDIA_CONTROLLER should not be required at all. Also, according with the Linux best practices, when #if tests for config symbols are required, developers should put it into the header files, and not inside the code, as it helps to improve code readability. From Documentation/SubmittingPatches: 2) #ifdefs are ugly Code cluttered with ifdefs is difficult to read and maintain. Don't do it. Instead, put your ifdefs in a header, and conditionally define 'static inline' functions, or macros, which are used in the code. Let the compiler optimize away the no-op case. So, this patch is perfectly fine on my eyes. I'm sorry, but I don't agree. Regarding the V4L2 subdev pad-level API, the goal is to convert all host and subdev drivers to it, so that's definitely the way to go. This does *not* mean that subdevs must expose a subdev device node. That's entirely optional. What I'm talking about is switching from video::*_mbus_fmt operations to pad::*_fmt operations. The pad-level format operations are very similar to video-level format operations, and more generic. Drivers shouldn't implement both. I agree that implementing two ways for doing the same thing is a bad idea, but especially since your idea is to convert all subdevs to it, this type of conversion should not require enabling CONFIG_MEDIA_CONTROLLER, as this feature is used to enable the MC userspace API. Regarding the MC API, drivers are not required to register a media_device instance. I have no issue with that. However, drivers should initialized the subdev's embedded media_entity, as that's required by subdev pad-level operations to get the number of pads for a subdev. There are two solutions: 1) add some fallback method at the core to use the video::*_mbus_fmt way, when MC is disabled; 2) split the config options into two: one configurable by the user to enable the userspace MC API, and another, used internally that would select the MC internal API when drivers need it. As your plan is to convert all drivers to the new way, (2) doesn't make much sense in long term, as, at the end, all drivers will be selecting it. But (1) makes even less sense :-) The issue here is that MC-enabled drivers will use pad-level subdev operations, so those operations need to be implemented in subdev drivers used by MC-enabled hosts/bridges. I don't
Re: [PATCH 1/4] v4l: add support for selection api
Hi Sakari,
On Monday 05 September 2011 12:25:08 Sakari Ailus wrote:
On Wed, Aug 31, 2011 at 02:28:20PM +0200, Tomasz Stanislawski wrote:
This patch introduces new api for a precise control of cropping and
composing features for video devices. The new ioctls are
VIDIOC_S_SELECTION and VIDIOC_G_SELECTION.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |2 +
drivers/media/video/v4l2-ioctl.c | 28 +
include/linux/videodev2.h | 46
+ include/media/v4l2-ioctl.h
|4 ++
4 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c index 61979b7..f3b9d15
100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -927,6 +927,8 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
int cmd, unsigned long arg)
case VIDIOC_CROPCAP:
case VIDIOC_G_CROP:
case VIDIOC_S_CROP:
+ case VIDIOC_G_SELECTION:
+ case VIDIOC_S_SELECTION:
case VIDIOC_G_JPEGCOMP:
case VIDIOC_S_JPEGCOMP:
case VIDIOC_QUERYSTD:
diff --git a/drivers/media/video/v4l2-ioctl.c
b/drivers/media/video/v4l2-ioctl.c index 002ce13..6e02b45 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -225,6 +225,8 @@ static const char *v4l2_ioctls[] = {
[_IOC_NR(VIDIOC_CROPCAP)] = VIDIOC_CROPCAP,
[_IOC_NR(VIDIOC_G_CROP)] = VIDIOC_G_CROP,
[_IOC_NR(VIDIOC_S_CROP)] = VIDIOC_S_CROP,
+ [_IOC_NR(VIDIOC_G_SELECTION)] = VIDIOC_G_SELECTION,
+ [_IOC_NR(VIDIOC_S_SELECTION)] = VIDIOC_S_SELECTION,
[_IOC_NR(VIDIOC_G_JPEGCOMP)] = VIDIOC_G_JPEGCOMP,
[_IOC_NR(VIDIOC_S_JPEGCOMP)] = VIDIOC_S_JPEGCOMP,
[_IOC_NR(VIDIOC_QUERYSTD)] = VIDIOC_QUERYSTD,
@@ -1714,6 +1716,32 @@ static long __video_do_ioctl(struct file *file,
ret = ops-vidioc_s_crop(file, fh, p);
break;
}
+ case VIDIOC_G_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_g_selection)
+ break;
+
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+
+ ret = ops-vidioc_g_selection(file, fh, p);
+ if (!ret)
+ dbgrect(vfd, , p-r);
+ break;
+ }
+ case VIDIOC_S_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_s_selection)
+ break;
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+ dbgrect(vfd, , p-r);
+
+ ret = ops-vidioc_s_selection(file, fh, p);
+ break;
+ }
case VIDIOC_CROPCAP:
{
struct v4l2_cropcap *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..b7471fe 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -738,6 +738,48 @@ struct v4l2_crop {
struct v4l2_rectc;
};
+/* Hints for adjustments of selection rectangle */
+#define V4L2_SEL_SIZE_GE 0x0001
+#define V4L2_SEL_SIZE_LE 0x0002
+
+/* Selection targets */
+
+/* current cropping area */
+#define V4L2_SEL_CROP_ACTIVE 0
+/* default cropping area */
+#define V4L2_SEL_CROP_DEFAULT 1
+/* cropping bounds */
+#define V4L2_SEL_CROP_BOUNDS 2
+/* current composing area */
+#define V4L2_SEL_COMPOSE_ACTIVE256
+/* default composing area */
+#define V4L2_SEL_COMPOSE_DEFAULT 257
+/* composing bounds */
+#define V4L2_SEL_COMPOSE_BOUNDS258
+/* current composing area plus all padding pixels */
+#define V4L2_SEL_COMPOSE_PADDED259
+
+/**
+ * struct v4l2_selection - selection info
+ * @type: buffer type (do not use *_MPLANE types)
+ * @target:selection target, used to choose one of possible
rectangles
+ * @flags: constraints flags
+ * @r: coordinates of selection window
+ * @reserved: for future use, rounds structure size to 64 bytes, set
to
zero + *
+ * Hardware may use multiple helper window to process a video stream.
+ * The structure is used to exchange this selection areas between
+ * an application and a driver.
+ */
+struct v4l2_selection {
+ __u32 type;
+ __u32 target;
+ __u32 flags;
+ struct v4l2_rectr;
+ __u32 reserved[9];
+};
The v4l2_selection doesn't have which field such as v4l2_subdev_crop and
v4l2_subdev_format. This
Re: [PATCH 1/4] v4l: add support for selection api
On Mon, Sep 05, 2011 at 02:52:03PM +0200, Laurent Pinchart wrote:
Hi Sakari,
On Monday 05 September 2011 12:25:08 Sakari Ailus wrote:
On Wed, Aug 31, 2011 at 02:28:20PM +0200, Tomasz Stanislawski wrote:
This patch introduces new api for a precise control of cropping and
composing features for video devices. The new ioctls are
VIDIOC_S_SELECTION and VIDIOC_G_SELECTION.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/video/v4l2-compat-ioctl32.c |2 +
drivers/media/video/v4l2-ioctl.c | 28 +
include/linux/videodev2.h | 46
+ include/media/v4l2-ioctl.h
|4 ++
4 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
b/drivers/media/video/v4l2-compat-ioctl32.c index 61979b7..f3b9d15
100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -927,6 +927,8 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
int cmd, unsigned long arg)
case VIDIOC_CROPCAP:
case VIDIOC_G_CROP:
case VIDIOC_S_CROP:
+ case VIDIOC_G_SELECTION:
+ case VIDIOC_S_SELECTION:
case VIDIOC_G_JPEGCOMP:
case VIDIOC_S_JPEGCOMP:
case VIDIOC_QUERYSTD:
diff --git a/drivers/media/video/v4l2-ioctl.c
b/drivers/media/video/v4l2-ioctl.c index 002ce13..6e02b45 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -225,6 +225,8 @@ static const char *v4l2_ioctls[] = {
[_IOC_NR(VIDIOC_CROPCAP)] = VIDIOC_CROPCAP,
[_IOC_NR(VIDIOC_G_CROP)] = VIDIOC_G_CROP,
[_IOC_NR(VIDIOC_S_CROP)] = VIDIOC_S_CROP,
+ [_IOC_NR(VIDIOC_G_SELECTION)] = VIDIOC_G_SELECTION,
+ [_IOC_NR(VIDIOC_S_SELECTION)] = VIDIOC_S_SELECTION,
[_IOC_NR(VIDIOC_G_JPEGCOMP)] = VIDIOC_G_JPEGCOMP,
[_IOC_NR(VIDIOC_S_JPEGCOMP)] = VIDIOC_S_JPEGCOMP,
[_IOC_NR(VIDIOC_QUERYSTD)] = VIDIOC_QUERYSTD,
@@ -1714,6 +1716,32 @@ static long __video_do_ioctl(struct file *file,
ret = ops-vidioc_s_crop(file, fh, p);
break;
}
+ case VIDIOC_G_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_g_selection)
+ break;
+
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+
+ ret = ops-vidioc_g_selection(file, fh, p);
+ if (!ret)
+ dbgrect(vfd, , p-r);
+ break;
+ }
+ case VIDIOC_S_SELECTION:
+ {
+ struct v4l2_selection *p = arg;
+
+ if (!ops-vidioc_s_selection)
+ break;
+ dbgarg(cmd, type=%s\n, prt_names(p-type, v4l2_type_names));
+ dbgrect(vfd, , p-r);
+
+ ret = ops-vidioc_s_selection(file, fh, p);
+ break;
+ }
case VIDIOC_CROPCAP:
{
struct v4l2_cropcap *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..b7471fe 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -738,6 +738,48 @@ struct v4l2_crop {
struct v4l2_rectc;
};
+/* Hints for adjustments of selection rectangle */
+#define V4L2_SEL_SIZE_GE 0x0001
+#define V4L2_SEL_SIZE_LE 0x0002
+
+/* Selection targets */
+
+/* current cropping area */
+#define V4L2_SEL_CROP_ACTIVE 0
+/* default cropping area */
+#define V4L2_SEL_CROP_DEFAULT1
+/* cropping bounds */
+#define V4L2_SEL_CROP_BOUNDS 2
+/* current composing area */
+#define V4L2_SEL_COMPOSE_ACTIVE 256
+/* default composing area */
+#define V4L2_SEL_COMPOSE_DEFAULT 257
+/* composing bounds */
+#define V4L2_SEL_COMPOSE_BOUNDS 258
+/* current composing area plus all padding pixels */
+#define V4L2_SEL_COMPOSE_PADDED 259
+
+/**
+ * struct v4l2_selection - selection info
+ * @type:buffer type (do not use *_MPLANE types)
+ * @target: selection target, used to choose one of possible
rectangles
+ * @flags: constraints flags
+ * @r: coordinates of selection window
+ * @reserved:for future use, rounds structure size to 64 bytes, set
to
zero + *
+ * Hardware may use multiple helper window to process a video stream.
+ * The structure is used to exchange this selection areas between
+ * an application and a driver.
+ */
+struct v4l2_selection {
+ __u32 type;
+ __u32 target;
+ __u32 flags;
+ struct v4l2_rectr;
+ __u32 reserved[9];
+};
The
Re: [PATCH 2/2] mfd: remove CONFIG_MFD_SUPPORT
On Fri, 2 Sep 2011 16:43:36 +0200, Arnd Bergmann wrote: We currently have two symbols to control compilation the MFD subsystem, MFD_SUPPORT and MFD_CORE. The MFD_SUPPORT is actually not required at all, it only hides the submenu when not set, with the effect that Kconfig warns about missing dependencies when another driver selects an MFD driver while MFD_SUPPORT is disabled. Turning the MFD submenu back from menuconfig into a plain menu simplifies the Kconfig syntax for those kinds of users and avoids the surprise when the menu suddenly appears because another driver was enabled that selects this symbol. Signed-off-by: Arnd Bergmann a...@arndb.de --- arch/arm/mach-omap2/Kconfig |1 - drivers/gpio/Kconfig|3 +-- drivers/i2c/busses/Kconfig |1 - drivers/media/radio/Kconfig |1 - drivers/mfd/Kconfig | 22 -- 5 files changed, 5 insertions(+), 23 deletions(-) (...) --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -2,23 +2,8 @@ # Multifunction miscellaneous devices # -menuconfig MFD_SUPPORT - bool Multifunction device drivers - depends on HAS_IOMEM - default y - help - Multifunction devices embed several functions (e.g. GPIOs, - touchscreens, keyboards, current regulators, power management chips, - etc...) in one single integrated circuit. They usually talk to the - main CPU through one or more IRQ lines and low speed data busses (SPI, - I2C, etc..). They appear as one single device to the main system - through the data bus and the MFD framework allows for sub devices - (a.k.a. functions) to appear as discrete platform devices. - MFDs are typically found on embedded platforms. - - This option alone does not add any kernel code. - -if MFD_SUPPORT +if HAS_IOMEM +menu Multifunction device drivers config MFD_CORE tristate I think I prefer Luciano's proposal, for the same reasons given for the misc device drivers patch. But here again I'm not the one making the decision, so it's up to Samuel to decide which patch he wants to apply. -- Jean Delvare -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DiBxxxx: fixes for 3.1/3.0
Em 05-09-2011 05:11, Olivier Grenie escreveu:
Hello Mauro,
I agree with you but when I wrote this patch, my concern was that the read
register function (dib0070_read_reg)
returns a u16 and so I could not propagate the error. That's why I decided to
return 0 and not change the API.
But if you have a better idea, I will have no problem to implement it.
Ok, I'll pull from it for 3.0/3.1. For 3.2, the better is to fix it.
What other drivers do when they need to read a 16 bit register is to declare
the function as
returning an 'int'. As you know, on Linux, int has 32 bits, so it returns an
u16 properly.
It will also return properly the errors.
So, all you need to do is to convert it to something like:
static int dib0070_read_reg(struct dib0070_state *state, u8 reg)
{
int ret;
ret = mutex_lock_interruptible(...);
if (ret 0)
return ret;
...
ret = i2c_transfer(state-i2c, state-msg, 2);
if (ret 0)
goto error;
if (ret != 2) {
ret = -EIO;
goto error;
}
ret = (state-i2c_read_buffer[0] 8)
| state-i2c_read_buffer[1];
error:
mutex_unlock(...);
return ret;
}
You'll need to add a check on all places that calls dib0070_read_reg() (and
dib070_write_reg) to do
the right thing when a negative number is returned, like:
static int dib0070_set_bandwidth(struct dvb_frontend *fe, struct
dvb_frontend_parameters *ch)
{
struct dib0070_state *state = fe-tuner_priv;
int tmp = dib0070_read_reg(state, 0x02);
if (tmp 0)
return tmp;
tmp | = 0x3fff;
...
}
For the write register function (dib0070_write_reg), in case of problem with
the mutex lock, an error code is returned.
Userspace applications in general handle EAGAIN on a different way, especially
if the application
is opening the device on non-blocking mode, as POSIX require that applications
should re-try
the ioctl, if EAGAIN is returned, on non-blocking mode. They might also handle
EINTR case as well.
So, using it instead of EINVAL is better.
The same idea is true for the whole patch.
regards,
Olivier
From: linux-media-ow...@vger.kernel.org [linux-media-ow...@vger.kernel.org]
On Behalf Of Mauro Carvalho Chehab [mche...@infradead.org]
Sent: Sunday, September 04, 2011 2:45 AM
To: Patrick Boettcher
Cc: Linux Media Mailing List
Subject: Re: DiB: fixes for 3.1/3.0
Em 03-08-2011 12:33, Patrick Boettcher escreveu:
Hi Mauro,
Thanks for the patches!
Would you please pull from
git://linuxtv.org/pb/media_tree.git for_v3.0
for the following to changesets:
[media] dib0700: protect the dib0700 buffer access
-static uint16_t dib0070_read_reg(struct dib0070_state *state, u8 reg)
+static u16 dib0070_read_reg(struct dib0070_state *state, u8 reg)
{
+ u16 ret;
+
+ if (mutex_lock_interruptible(state-i2c_buffer_lock) 0) {
+ dprintk(could not acquire lock);
+ return 0;
Returning 0 doesn't seem right for me. IMO, it should be return -EAGAIN
or -EINTR (which is, incidentally, what mutex_lock_interruptible() will
return).
The same applies to the similar parts of the code, at the read and write
routines.
[media] DiBcom: protect the I2C bufer access
?
Those two changesets are fixing the remaining problems regarding the
dma-on-stack-buffer-fix applied for the first time in 2.6.39, IIRC.
They should go to stable 3.0 (as they are in my tree) and they can be
cherry-picked to 3.1.
We are preparing the same thing for 2.6.39 as the patches don't apply
cleanly.
Thanks to Javier Marcet for his help during the debug-phase.
thanks and best regards,
--
Patrick Boettcher - Kernel Labs
http://www.kernellabs.com/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
CONFIDENTIAL NOTICE: The contents of this message, including any attachments,
are confidential and are intended solely for the use of the person or entity
to whom the message was addressed. If you are not the intended recipient of
this message, please be advised that any dissemination, distribution, or use
of the contents of this message is strictly prohibited. If you received this
message in error, please notify the sender. Please also permanently delete
all copies of the original message and any attached documentation. Thank you.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: DiBxxxx: fixes for 3.1/3.0
On Mon, 5 Sep 2011, Mauro Carvalho Chehab wrote:
Em 05-09-2011 05:11, Olivier Grenie escreveu:
Hello Mauro,
I agree with you but when I wrote this patch, my concern was that the read
register function (dib0070_read_reg)
returns a u16 and so I could not propagate the error. That's why I decided to
return 0 and not change the API.
But if you have a better idea, I will have no problem to implement it.
Ok, I'll pull from it for 3.0/3.1. For 3.2, the better is to fix it.
What other drivers do when they need to read a 16 bit register is to declare
the function as
returning an 'int'. As you know, on Linux, int has 32 bits, so it returns an
u16 properly.
It will also return properly the errors.
So, all you need to do is to convert it to something like:
static int dib0070_read_reg(struct dib0070_state *state, u8 reg)
{
int ret;
ret = mutex_lock_interruptible(...);
if (ret 0)
return ret;
...
ret = i2c_transfer(state-i2c, state-msg, 2);
if (ret 0)
goto error;
if (ret != 2) {
ret = -EIO;
goto error;
}
ret = (state-i2c_read_buffer[0] 8)
| state-i2c_read_buffer[1];
error:
mutex_unlock(...);
return ret;
}
You'll need to add a check on all places that calls dib0070_read_reg() (and
dib070_write_reg) to do
the right thing when a negative number is returned, like:
static int dib0070_set_bandwidth(struct dvb_frontend *fe, struct
dvb_frontend_parameters *ch)
{
struct dib0070_state *state = fe-tuner_priv;
int tmp = dib0070_read_reg(state, 0x02);
if (tmp 0)
return tmp;
tmp | = 0x3fff;
...
}
For the write register function (dib0070_write_reg), in case of problem with
the mutex lock, an error code is returned.
Userspace applications in general handle EAGAIN on a different way, especially
if the application
is opening the device on non-blocking mode, as POSIX require that applications
should re-try
the ioctl, if EAGAIN is returned, on non-blocking mode. They might also handle
EINTR case as well.
So, using it instead of EINVAL is better.
While I agree with you in principle I think the time we would need and the
risk we would take to do what you're asking here is too high.
I agree the drivers are quite huge and ugly but now adding hundreds of
if's and returns won't make them better.
Right now if a read fails it returns 0 which in some cases might be even
correct.
Fixing the error-handling in the drivers will most likely break things
unless it is not done automagically - IOW not by a human being.
I quickly checked some other sources in dvb/frontends/ and the Dibbies are
not the only ones where the error-path would need to be fixed.
I'd appreciate if we could restrict this requirement to new drivers which
certainly will arrive. Of course, if there is a volunteer I'm ready to
have a look.
What do you think?
regards,
--
Patrick
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
- Original message -
On Monday, September 05, 2011 11:59:03 Laurent Pinchart wrote:
Hi Hans,
On Friday 02 September 2011 09:29:08 Sitsofe Wheeler wrote:
On Fri, Sep 02, 2011 at 01:35:49PM +0800, Dave Young wrote:
On Fri, Sep 2, 2011 at 12:59 PM, Dave Young wrote:
On Fri, Sep 2, 2011 at 3:10 AM, Sitsofe Wheeler wrote:
On Thu, Sep 01, 2011 at 05:02:51PM +0800, Dave Young wrote:
On Tue, Aug 30, 2011 at 4:48 AM, Sitsofe Wheeler wrote:
I managed to produce an oops in 3.1.0-rc3-00270-g7a54f5e by
unplugging a
USB webcam. See below:
Could you try the attached patch?
This patch fixed the oops but extending the sequence (enable
camera, start cheese, disable camera, watch cheese pause,
enable camera, quit cheese, start cheese) causes the following
poison overwritten warning
to appear:
It seems another bug, I can reproduce this as well.
uvc_device is freed in uvc_delete,
struct v4l2_device vdev is the member of struct uvc_device, so
vdev is also freed. Later v4l2_device operations on vdev will
overwrite the poison memory area.
Please try attached patch on top of previous one, in this patch I
move v4l2_device_put after vdev-release in function
v4l2_device_release
Not sure if this is a right fix, comments?
(inlining the patch's contents)
diff --git a/drivers/media/video/v4l2-dev.c
b/drivers/media/video/v4l2- dev.c
index 98cee19..541dba3 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -172,13 +172,14 @@ static void v4l2_device_release(struct
device *cd) media_device_unregister_entity(vdev-entity);
#endif
+ /* Decrease v4l2_device refcount */
+ if (vdev-v4l2_dev)
+ v4l2_device_put(vdev-v4l2_dev);
+
/* Release video_device and perform other
cleanups as needed. */
vdev-release(vdev);
- /* Decrease v4l2_device refcount */
- if (vdev-v4l2_dev)
- v4l2_device_put(vdev-v4l2_dev);
}
static struct class video_class = {
v4l2_device_put() got introduced in commit
bedf8bcf6b4f90a6e31add3721a2e71877289381 (v4l2-device: add kref and a
release function). If I understand its purpose correctly, drivers
that use a v4l2_device instance should use the v4l2_device release
callback to release device structures instead of counting
video_device release callbacks manually. In that case I think the
v4l2-dev.c code is correct, and all drivers that use v4l2_device
should be fixed.
The above patch fixes the problem in a central location, but seems to
defeat the original purpose of v4l2_device_get/put().
Hans, could you please comment on that ?
The original order is correct, but what I missed is that for drivers
that release (free) everything in the videodev release callback the
v4l2_device struct is also freed and v4l2_device_put will fail.
To fix this, add this code just before the vdev-release call:
/* Do not call v4l2_device_put if there is no release callback set. */
if (v4l2_dev-release == NULL)
v4l2_dev = NULL;
If there is no release callback, then the refcounting is pointless
anyway.
This should work.
thanks, will test tommorrow
Regards,
Hans
[ 191.240695] uvcvideo: Found UVC 1.00 device CNF7129
(04f2:b071) [ 191.277965] input: CNF7129 as
/devices/pci:00/:00:1d.7/usb1/1-8/1-8:1.0/input/input9
[ 220.287366]
=
[ 220.287379] BUG kmalloc-512: Poison overwritten
[ 220.287384]
-
[ 220.287387]
[ 220.287394] INFO: 0xec90f150-0xec90f150. First byte 0x6a
instead of 0x6b [ 220.287410] INFO: Allocated in
uvc_probe+0x54/0xd50 age=210617 cpu=0 pid=16 [ 220.287421]
T.974+0x29d/0x5e0 [ 220.287427] kmem_cache_alloc+0x167/0x180
[ 220.287433] uvc_probe+0x54/0xd50
[ 220.287441] usb_probe_interface+0xd5/0x1d0
[ 220.287448] driver_probe_device+0x80/0x1a0
[ 220.287455] __device_attach+0x41/0x50
[ 220.287460] bus_for_each_drv+0x53/0x80
[ 220.287466] device_attach+0x89/0xa0
[ 220.287472] bus_probe_device+0x25/0x40
[ 220.287478] device_add+0x5a9/0x660
[ 220.287484] usb_set_configuration+0x562/0x670
[ 220.287491] generic_probe+0x36/0x90
[ 220.287497] usb_probe_device+0x24/0x50
[ 220.287503] driver_probe_device+0x80/0x1a0
[ 220.287509] __device_attach+0x41/0x50
[ 220.287515] bus_for_each_drv+0x53/0x80
[ 220.287522] INFO: Freed in uvc_delete+0xfe/0x110 age=22
cpu=0 pid=1645 [ 220.287530]
Re: ERROR: em28xx_add_into_devlist [drivers/media/video/em28xx/em28xx.ko] undefined!
Em 04-09-2011 21:34, Chris Rankin escreveu: On 05/09/11 01:24, Antti Palosaari wrote: If you select em28xx-cards.c blob link you give you can see it is there still for some reason. It's a merge issue. This lingering reference must have been added after I posted my original patch. Fortunately, it's easily fixed: the list_add_tail(dev-devlist, em28xx_devlist); operation is now done by ex28xx_init_extension() instead, meaning we only take the devlist mutex once. So that last em28xx_add_into_devlist() reference is obsolete. Could you please provide me a patch for it? I'll merge with your original one when submitting it upstream. Cheers, Chris -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: spca1528 device (Device 015: ID 04fc:1528 Sunplus Technology)..libv4l2: error turning on stream: Timer expired issue
On 09/04/2011 10:07 PM, Theodore Kilgore wrote: On Sun, 4 Sep 2011, Mauricio Henriquez wrote: Hi guys, Not sure if is the right place to ask since this device use a gspca driver and not sure if that driver is related to uvc or not, if not please point me to the right place... Looks right to me, and I hope that someone has more direct knowledge about your camera, which I do not. I do have a couple of questions, however, and a comment. Recently I'm trying to make work a Sunplus crappy mini HD USB camera, lsusb list this info related to the device: Picture Transfer Protocol (PIMA 15470) Bus 001 Device 015: ID 04fc:1528 Sunplus Technology Co., Ltd idVendor 0x04fc Sunplus Technology Co., Ltd idProduct 0x1528 bcdDevice1.00 iManufacturer 1 Sunplus Co Ltd iProduct2 General Image Devic iSerial 0 ... Using the gspca-2.13.6 on my Fed12 (2.6.31.6-166.fc12.i686.PAE kernel), the device is listed as /dev/video1 and no error doing a dmesg...but trying to make it work, let say with xawtv, I get: This is xawtv-3.95, running on Linux/i686 (2.6.31.6-166.fc12.i686.PAE) xinerama 0: 1600x900+0+0 WARNING: No DGA direct video mode for this display. /dev/video1 [v4l2]: no overlay support v4l-conf had some trouble, trying to continue anyway Warning: Missing charsets in String to FontSet conversion Warning: Missing charsets in String to FontSet conversion libv4l2: error turning on stream: Timer expired ioctl: VIDIOC_STREAMON(int=1): Timer expired ioctl: VIDIOC_S_STD(std=0x0 []): Invalid argument v4l2: oops: select timeout ..or doing: xawtv -c /dev/video1 -nodga -novm -norandr -noxv -noxv-video I get: ioctl: VIDIOC_STREAMON(int=1): Timer expired ioctl: VIDIOC_S_STD(std=0x0 []): Invalid argument v4l2: oops: select timeout libv4l2: error turning on stream: Timer expired libv4l2: error reading: Invalid argument vlc, cheese, etc give me similar Timeout related messages... The comment: Perhaps a good thing to try would be the nice, simple, basic program svv, which you can get from the website of Jean-Francois Moine. Some of these other things do not always work. Especially I have had trouble with xawtv, though the xawtv people may have fixed a lot of problems while I was not watching them. yeap, I try it, same kind of mesages... The question: Is this a dual-mode camera which is also supposed to have still camera capabilities? If so, you might be interested in contacting the Gphoto project. I just searched for it there, and it does not seem to be listed. yeap a dual-mode camera... I assume that the specialists on the spca cameras will step forward. I am not one of them, as I said. Good luck. Theodore Kilgore Thanks! Mauricio -- Mauricio R. Henriquez Schott Escuela de Ingeniería en Computación Universidad Austral de Chile - Sede Puerto Montt Los Pinos S/N, Balneario de Pelluco, Puerto Montt - Chile Tel: 65-487440 Fax: 65-277156 mail: mauriciohenriq...@uach.cl attachment: buhochileno.vcf
Re: [PATCH 1/2] misc: remove CONFIG_MISC_DEVICES
On Monday 05 September 2011, Jean Delvare wrote: As said before, I'm not sure. Yes, it makes it easier to select misc device drivers from Kconfig files. But it also makes it impossible to deselect all misc device drivers at once. I think that what we really need is the implementation in the Kconfig system of smart selects, i.e. whenever an entry is selected, everything it depends on gets selected as well. I don't know how feasible this is, but if it can be done then I'd prefer this to your proposal. Meanwhile, I am not in favor of applying your patch. The benefit is relatively small IMHO (misc device drivers are rarely selected) and there is one significant drawback. Before I made this patch, I started a different one that added about a dozen 'select MISC_DEVICES' statements sprinkled all over the kernel in order to silence the Kconfig warnings. The problem is that whenever you select that option, the misc directory suddenly becomes visible when it was disabled before, and things like 'oldconfig' will start asking about all other misc drivers as well. I think it would simply be more consistent to have it enabled all the time. Well, even better would be to move the bulk of the misc drivers to a proper location sorted by their subsystems. A lot of them should never have been merged in their current state IMHO. That being said, I'm not the one to decide, so if you can convince someone with more power (aka Andrew Morton)... I think I should finally do what has been talked about a few times and formally become the maintainer of drivers/char and drivers/misc ;-) The problem is that I'm not actually a good maintainer, but maybe it's better to just have someone instead of falling back to Andrew or some random subsystem maintainer to send any patches for drivers/misc. config AD525X_DPOT tristate Analog Devices Digital Potentiometers @@ -344,6 +328,12 @@ config ISL29020 This driver can also be built as a module. If so, the module will be called isl29020. +config SENSORS_LIS3LV02D + tristate + depends on INPUT + select INPUT_POLLDEV + default n + If you patch gets applied, then this one would better be moved to drivers/misc/lis3lv02d/Kconfig. Ah, that's true. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ERROR: em28xx_add_into_devlist [drivers/media/video/em28xx/em28xx.ko] undefined!
On 05/09/11 14:57, Mauro Carvalho Chehab wrote: Could you please provide me a patch for it? I'll merge with your original one when submitting it upstream. Here you go. Did you also pick up the other merge fix and the (separate) memory leak fix, please? Cheers, Chris Signed-off-by: Chris Rankin ranki...@yahoo.com --- linux/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-09-05 15:21:38.0 +0100 +++ linux/drivers/media/video/em28xx/em28xx-cards.c 2011-09-05 15:21:55.0 +0100 @@ -2885,7 +2885,6 @@ retval = em28xx_audio_setup(dev); if (retval) return -ENODEV; - em28xx_add_into_devlist(dev); em28xx_init_extension(dev); return 0;
Re: [PATCH 1/2] misc: remove CONFIG_MISC_DEVICES
On Mon, 5 Sep 2011 16:19:35 +0200, Arnd Bergmann wrote: On Monday 05 September 2011, Jean Delvare wrote: As said before, I'm not sure. Yes, it makes it easier to select misc device drivers from Kconfig files. But it also makes it impossible to deselect all misc device drivers at once. I think that what we really need is the implementation in the Kconfig system of smart selects, i.e. whenever an entry is selected, everything it depends on gets selected as well. I don't know how feasible this is, but if it can be done then I'd prefer this to your proposal. Meanwhile, I am not in favor of applying your patch. The benefit is relatively small IMHO (misc device drivers are rarely selected) and there is one significant drawback. Before I made this patch, I started a different one that added about a dozen 'select MISC_DEVICES' statements sprinkled all over the kernel in order to silence the Kconfig warnings. Ah, OK. This certainly shifts the scales towards your side. The problem is that whenever you select that option, the misc directory suddenly becomes visible when it was disabled before, and things like 'oldconfig' will start asking about all other misc drivers as well. Another good point. Maybe I'm convinced now. I think it would simply be more consistent to have it enabled all the time. Well, even better would be to move the bulk of the misc drivers to a proper location sorted by their subsystems. A lot of them should never have been merged in their current state IMHO. As one of the offenders, I won't dare to comment on this ;) That being said, I'm not the one to decide, so if you can convince someone with more power (aka Andrew Morton)... I think I should finally do what has been talked about a few times and formally become the maintainer of drivers/char and drivers/misc ;-) The problem is that I'm not actually a good maintainer, but maybe it's better to just have someone instead of falling back to Andrew or some random subsystem maintainer to send any patches for drivers/misc. Certainly. And having a maintainer for these (non-)subsystems would certainly help keep their size low, while the current trend is in the other direction. -- Jean Delvare -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: spca1528 device (Device 015: ID 04fc:1528 Sunplus Technology)..libv4l2: error turning on stream: Timer expired issue
On 09/05/2011 03:19 AM, Jean-Francois Moine wrote: On Sun, 04 Sep 2011 15:39:30 -0400 Mauricio Henriquezbuhochil...@gmail.com wrote: Recently I'm trying to make work a Sunplus crappy mini HD USB camera, lsusb list this info related to the device: Picture Transfer Protocol (PIMA 15470) Bus 001 Device 015: ID 04fc:1528 Sunplus Technology Co., Ltd idVendor 0x04fc Sunplus Technology Co., Ltd idProduct 0x1528 bcdDevice1.00 iManufacturer 1 Sunplus Co Ltd iProduct2 General Image Devic iSerial 0 ... Using the gspca-2.13.6 on my Fed12 (2.6.31.6-166.fc12.i686.PAE kernel), the device is listed as /dev/video1 and no error doing a dmesg...but trying to make it work, let say with xawtv, I get: [snip] Hi Mauricio, The problem seems tied to the alternate setting. It must be the #3 while the lastest versions of gspca compute a best one. May you apply the following patch to gspca-2.13.6? Thanks Jean, yeap I apply the patch, but still the same kind of messages about timeout sing xawtv or svv: xawtv: ioctl: VIDIOC_S_STD(std=0x0 []): Invalid argument libv4l2: error turning on stream: Timer expired libv4l2: error reading: Invalid argument v4l2: read: Invalid argument libv4l2: error turning on stream: Timer expired ioctl: VIDIOC_STREAMON(int=1): Timer expired v4l2: oops: select timeout ioctl: VIDIOC_REQBUFS(count=0;type=VIDEO_CAPTURE;memory=MMAP): Device or resource busy libv4l2: error reading: Invalid argument v4l2: read: Invalid argument svv: raw pixfmt: JPEG 640x480 pixfmt: RGB3 640x480 mmap method VIDIOC_STREAMON error 62, Timer expired this camera in mass storage mode works ok... Cheers Mauricio --8-- --- build/spca1528.c.orig 2011-09-05 08:41:54.0 +0200 +++ build/spca1528.c2011-09-05 08:53:51.0 +0200 @@ -307,8 +307,6 @@ sd-color = COLOR_DEF; sd-sharpness = SHARPNESS_DEF; - gspca_dev-nbalt = 4;/* use alternate setting 3 */ - return 0; } @@ -349,6 +347,9 @@ reg_r(gspca_dev, 0x25, 0x0004, 1); reg_wb(gspca_dev, 0x27, 0x, 0x, 0x06); reg_r(gspca_dev, 0x27, 0x, 1); + + gspca_dev-alt = 4; /* use alternate setting 3 */ + return gspca_dev-usb_err; } --8-- (Theodore, this webcam may work in mass storage mode with ID 04fc:0171. In webcam mode with ID 04fc:1528, it offers 3 interfaces: interface 0 contains only an interrupt endpoint, interface 1 is the webcam with only isochronous endpoints and interface 2 contains bulk in, bulk out and interrupt in endpoints - I don't know how to use the interfaces 0 and 2, but sure the interface 2 could be used to access the camera images) -- Mauricio R. Henriquez Schott Escuela de Ingeniería en Computación Universidad Austral de Chile - Sede Puerto Montt Los Pinos S/N, Balneario de Pelluco, Puerto Montt - Chile Tel: 65-487440 Fax: 65-277156 mail: mauriciohenriq...@uach.cl attachment: buhochileno.vcf
Re: [media-ctl][PATCHv4 3/3] libmediactl: get the device name via udev
On Mon, 2011-09-05 at 12:31 +0200, Laurent Pinchart wrote: Hi Andy, On Friday 02 September 2011 15:09:28 Andy Shevchenko wrote: If configured with --with-libudev, the libmediactl is built with libudev support. It allows to get the device name in right way in the modern linux systems. Thanks for the patch. We're nearly there :-) I see. This will break binary compatibility if an application creates a struct media_device instances itself. On the other hand applications are not supposed to do that. As the struct udev pointer is only used internally, what about passing it around between functions explicitly instead ? That we will break the API in media_close(). Might be I am a blind, but I can't see the way how to do both 1) don't provide static global variable and 2) don't break the API/ABI. So, I'll send 5th version of the patchset with API breakage. Hope that what you could accept as a final version. -- Andy Shevchenko andriy.shevche...@linux.intel.com Intel Finland Oy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [media-ctl][PATCHv4 3/3] libmediactl: get the device name via udev
Hi Andy, On Monday 05 September 2011 16:48:34 Andy Shevchenko wrote: On Mon, 2011-09-05 at 12:31 +0200, Laurent Pinchart wrote: Hi Andy, On Friday 02 September 2011 15:09:28 Andy Shevchenko wrote: If configured with --with-libudev, the libmediactl is built with libudev support. It allows to get the device name in right way in the modern linux systems. Thanks for the patch. We're nearly there :-) I see. This will break binary compatibility if an application creates a struct media_device instances itself. On the other hand applications are not supposed to do that. As the struct udev pointer is only used internally, what about passing it around between functions explicitly instead ? That we will break the API in media_close(). Might be I am a blind, but I can't see the way how to do both 1) don't provide static global variable and 2) don't break the API/ABI. What about passing the udev pointer explictly to media_enum_entities() (which is static), and calling media_udev_close() in media_open() after the media_enum_entities() call ? So, I'll send 5th version of the patchset with API breakage. Hope that what you could accept as a final version. Sorry for making it so difficult :-/ -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[media-ctl][PATCHv5 1/5] libmediactl: restruct error path
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
src/media.c | 16 +---
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/media.c b/src/media.c
index e3cab86..050289e 100644
--- a/src/media.c
+++ b/src/media.c
@@ -255,7 +255,7 @@ static int media_enum_entities(struct media_device *media)
char target[1024];
char *p;
__u32 id;
- int ret;
+ int ret = 0;
for (id = 0; ; id = entity-info.id) {
size = (media-entities_count + 1) * sizeof(*media-entities);
@@ -268,9 +268,9 @@ static int media_enum_entities(struct media_device *media)
ret = ioctl(media-fd, MEDIA_IOC_ENUM_ENTITIES, entity-info);
if (ret 0) {
- if (errno == EINVAL)
- break;
- return -errno;
+ if (errno != EINVAL)
+ ret = -errno;
+ break;
}
/* Number of links (for outbound links) plus number of pads (for
@@ -281,8 +281,10 @@ static int media_enum_entities(struct media_device *media)
entity-pads = malloc(entity-info.pads *
sizeof(*entity-pads));
entity-links = malloc(entity-max_links *
sizeof(*entity-links));
- if (entity-pads == NULL || entity-links == NULL)
- return -ENOMEM;
+ if (entity-pads == NULL || entity-links == NULL) {
+ ret = -ENOMEM;
+ break;
+ }
media-entities_count++;
@@ -316,7 +318,7 @@ static int media_enum_entities(struct media_device *media)
strcpy(entity-devname, devname);
}
- return 0;
+ return ret;
}
struct media_device *media_open(const char *name, int verbose)
--
1.7.5.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[media-ctl][PATCHv5 2/5] libmediactl: split media_get_devname_sysfs from media_enum_entities
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
src/media.c | 61 +-
1 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/src/media.c b/src/media.c
index 050289e..5d3ff7c 100644
--- a/src/media.c
+++ b/src/media.c
@@ -245,15 +245,46 @@ static int media_enum_links(struct media_device *media)
return ret;
}
-static int media_enum_entities(struct media_device *media)
+static int media_get_devname_sysfs(struct media_entity *entity)
{
- struct media_entity *entity;
struct stat devstat;
- unsigned int size;
char devname[32];
char sysname[32];
char target[1024];
char *p;
+ int ret;
+
+ sprintf(sysname, /sys/dev/char/%u:%u, entity-info.v4l.major,
+ entity-info.v4l.minor);
+ ret = readlink(sysname, target, sizeof(target));
+ if (ret 0)
+ return -errno;
+
+ target[ret] = '\0';
+ p = strrchr(target, '/');
+ if (p == NULL)
+ return -EINVAL;
+
+ sprintf(devname, /dev/%s, p + 1);
+ ret = stat(devname, devstat);
+ if (ret 0)
+ return -errno;
+
+ /* Sanity check: udev might have reordered the device nodes.
+* Make sure the major/minor match. We should really use
+* libudev.
+*/
+ if (major(devstat.st_rdev) == entity-info.v4l.major
+ minor(devstat.st_rdev) == entity-info.v4l.minor)
+ strcpy(entity-devname, devname);
+
+ return 0;
+}
+
+static int media_enum_entities(struct media_device *media)
+{
+ struct media_entity *entity;
+ unsigned int size;
__u32 id;
int ret = 0;
@@ -293,29 +324,7 @@ static int media_enum_entities(struct media_device *media)
media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV)
continue;
- sprintf(sysname, /sys/dev/char/%u:%u, entity-info.v4l.major,
- entity-info.v4l.minor);
- ret = readlink(sysname, target, sizeof(target));
- if (ret 0)
- continue;
-
- target[ret] = '\0';
- p = strrchr(target, '/');
- if (p == NULL)
- continue;
-
- sprintf(devname, /dev/%s, p + 1);
- ret = stat(devname, devstat);
- if (ret 0)
- continue;
-
- /* Sanity check: udev might have reordered the device nodes.
-* Make sure the major/minor match. We should really use
-* libudev.
-*/
- if (major(devstat.st_rdev) == entity-info.v4l.major
- minor(devstat.st_rdev) == entity-info.v4l.minor)
- strcpy(entity-devname, devname);
+ media_get_devname_sysfs(entity);
}
return ret;
--
1.7.5.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[media-ctl][PATCHv5 3/5] libmediactl: get the device name via udev
If configured with --with-libudev, the libmediactl is built with libudev
support. It allows to get the device name in right way in the modern linux
systems.
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
configure.in| 22 +++
src/Makefile.am |2 +
src/media.c | 105 +-
3 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/configure.in b/configure.in
index fd4c70c..983023e 100644
--- a/configure.in
+++ b/configure.in
@@ -13,6 +13,28 @@ AC_PROG_LIBTOOL
# Checks for libraries.
+AC_ARG_WITH([libudev],
+AS_HELP_STRING([--with-libudev],
+[Enable libudev to detect a device name]))
+
+AS_IF([test x$with_libudev = xyes],
+[PKG_CHECK_MODULES(libudev, libudev, have_libudev=yes, have_libudev=no)],
+[have_libudev=no])
+
+AS_IF([test x$have_libudev = xyes],
+[
+AC_DEFINE([HAVE_LIBUDEV], [], [Use libudev])
+LIBUDEV_CFLAGS=$libudev_CFLAGS
+LIBUDEV_LIBS=$libudev_LIBS
+AC_SUBST(LIBUDEV_CFLAGS)
+AC_SUBST(LIBUDEV_LIBS)
+],
+[AS_IF([test x$with_libudev = xyes],
+[AC_MSG_ERROR([libudev requested but not found])
+])
+])
+
+
# Kernel headers path.
AC_ARG_WITH(kernel-headers,
[AC_HELP_STRING([--with-kernel-headers=DIR],
diff --git a/src/Makefile.am b/src/Makefile.am
index 267ea83..52628d2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -5,6 +5,8 @@ mediactl_includedir=$(includedir)/mediactl
mediactl_include_HEADERS = media.h subdev.h
bin_PROGRAMS = media-ctl
+media_ctl_CFLAGS = $(LIBUDEV_CFLAGS)
+media_ctl_LDFLAGS = $(LIBUDEV_LIBS)
media_ctl_SOURCES = main.c options.c options.h tools.h
media_ctl_LDADD = libmediactl.la libv4l2subdev.la
diff --git a/src/media.c b/src/media.c
index 5d3ff7c..657b6c4 100644
--- a/src/media.c
+++ b/src/media.c
@@ -17,6 +17,8 @@
* with this program; if not, write to the Free Software Foundation, Inc.,
*/
+#include config.h
+
#include sys/ioctl.h
#include sys/stat.h
#include sys/types.h
@@ -245,6 +247,71 @@ static int media_enum_links(struct media_device *media)
return ret;
}
+struct media_private {
+ struct media_device *media;
+ int verbose;
+ void *priv;
+};
+
+#ifdef HAVE_LIBUDEV
+
+#include libudev.h
+
+static inline int media_udev_open(struct media_private *priv)
+{
+ priv-priv = udev_new();
+ if (priv-priv == NULL)
+ return -ENOMEM;
+ return 0;
+}
+
+static inline void media_udev_close(struct media_private *priv)
+{
+ udev_unref(priv-priv);
+}
+
+static int media_get_devname_udev(struct media_private *priv,
+ struct media_entity *entity)
+{
+ struct udev *udev = priv-priv;
+ dev_t devnum;
+ struct udev_device *device;
+ const char *p;
+ int ret = -ENODEV;
+
+ if (udev == NULL)
+ return -EINVAL;
+
+ devnum = makedev(entity-info.v4l.major, entity-info.v4l.minor);
+ if (priv-verbose)
+ printf(looking up device: %u:%u\n, major(devnum),
minor(devnum));
+ device = udev_device_new_from_devnum(udev, 'c', devnum);
+ if (device) {
+ p = udev_device_get_devnode(device);
+ if (p)
+ snprintf(entity-devname, sizeof(entity-devname),
%s, p);
+ ret = 0;
+ }
+
+ udev_device_unref(device);
+
+ return ret;
+}
+
+#else /* HAVE_LIBUDEV */
+
+static inline int media_udev_open(struct media_private *priv) { return 0; }
+
+static inline void media_udev_close(struct media_private *priv) { }
+
+static inline int media_get_devname_udev(struct media_private *priv,
+ struct media_entity *entity)
+{
+ return -ENOTSUP;
+}
+
+#endif /* HAVE_LIBUDEV */
+
static int media_get_devname_sysfs(struct media_entity *entity)
{
struct stat devstat;
@@ -281,8 +348,9 @@ static int media_get_devname_sysfs(struct media_entity
*entity)
return 0;
}
-static int media_enum_entities(struct media_device *media)
+static int media_enum_entities(struct media_private *priv)
{
+ struct media_device *media = priv-media;
struct media_entity *entity;
unsigned int size;
__u32 id;
@@ -324,6 +392,11 @@ static int media_enum_entities(struct media_device *media)
media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV)
continue;
+ /* Try to get the device name via udev */
+ if (!media_get_devname_udev(priv, entity))
+ continue;
+
+ /* Fall back to get the device name via sysfs */
media_get_devname_sysfs(entity);
}
@@ -333,6 +406,7 @@ static int media_enum_entities(struct media_device *media)
struct media_device *media_open(const char *name, int verbose)
{
struct media_device *media;
+ struct media_private *priv;
int ret;
media = malloc(sizeof(*media));
@@
[media-ctl][PATCHv5 4/5] libmediactl: simplify code by introducing close_and_free inliner
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
src/media.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/media.c b/src/media.c
index 657b6c4..6c03369 100644
--- a/src/media.c
+++ b/src/media.c
@@ -403,6 +403,12 @@ static int media_enum_entities(struct media_private *priv)
return ret;
}
+static inline void close_and_free(struct media_private *priv)
+{
+ free(priv);
+ media_close(priv-media);
+}
+
struct media_device *media_open(const char *name, int verbose)
{
struct media_device *media;
@@ -440,8 +446,7 @@ struct media_device *media_open(const char *name, int
verbose)
ret = media_udev_open(priv);
if (ret 0) {
printf(%s: Can't get udev context\n, __func__);
- free(priv);
- media_close(media);
+ close_and_free(priv);
return NULL;
}
@@ -457,8 +462,7 @@ struct media_device *media_open(const char *name, int
verbose)
if (ret 0) {
printf(%s: Unable to enumerate entities for device %s (%s)\n,
__func__, name, strerror(-ret));
- free(priv);
- media_close(media);
+ close_and_free(priv);
return NULL;
}
@@ -471,8 +475,7 @@ struct media_device *media_open(const char *name, int
verbose)
if (ret 0) {
printf(%s: Unable to enumerate pads and linksfor device %s\n,
__func__, name);
- free(priv);
- media_close(media);
+ close_and_free(priv);
return NULL;
}
--
1.7.5.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[media-ctl][PATCHv5 5/5] libmediactl: get rid of memset via using calloc
The code snippet
x = malloc(sizeof(*x));
memset(x, 0, sizeof(*x));
could be easily changed to
x = calloc(1, sizeof(*x));
Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
src/media.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/media.c b/src/media.c
index 6c03369..38ebaac 100644
--- a/src/media.c
+++ b/src/media.c
@@ -415,12 +415,11 @@ struct media_device *media_open(const char *name, int
verbose)
struct media_private *priv;
int ret;
- media = malloc(sizeof(*media));
+ media = calloc(1, sizeof(*media));
if (media == NULL) {
printf(%s: unable to allocate memory\n, __func__);
return NULL;
}
- memset(media, 0, sizeof(*media));
if (verbose)
printf(Opening media device %s\n, name);
@@ -431,13 +430,12 @@ struct media_device *media_open(const char *name, int
verbose)
return NULL;
}
- priv = malloc(sizeof(*priv));
+ priv = calloc(1, sizeof(*priv));
if (priv == NULL) {
printf(%s: unable to allocate memory\n, __func__);
media_close(media);
return NULL;
}
- memset(priv, 0, sizeof(*priv));
/* Fill the private structure */
priv-media = media;
--
1.7.5.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] Reserved fields in v4l2_mbus_framefmt, v4l2_subdev_format alignment
Hi all,
I recently came across a few issues in the definitions of v4l2_subdev_format
and v4l2_mbus_framefmt when I was working on sensor control that I wanted to
bring up here. The appropriate structure right now look like this:
include/linux/v4l2-subdev.h:
---8---
/**
* struct v4l2_subdev_format - Pad-level media bus format
* @which: format type (from enum v4l2_subdev_format_whence)
* @pad: pad number, as reported by the media API
* @format: media bus format (format code and frame size)
*/
struct v4l2_subdev_format {
__u32 which;
__u32 pad;
struct v4l2_mbus_framefmt format;
__u32 reserved[8];
};
---8---
include/linux/v4l2-mediabus.h:
---8---
/**
* struct v4l2_mbus_framefmt - frame format on the media bus
* @width: frame width
* @height: frame height
* @code: data format code (from enum v4l2_mbus_pixelcode)
* @field: used interlacing type (from enum v4l2_field)
* @colorspace: colorspace of the data (from enum v4l2_colorspace)
*/
struct v4l2_mbus_framefmt {
__u32 width;
__u32 height;
__u32 code;
__u32 field;
__u32 colorspace;
__u32 reserved[7];
};
---8---
Offering a lower level interface for sensors which allows better control of
them from the user space involves providing the link frequency to the user
space. While the link frequency will be a control, together with the bus
type and number of lanes (on serial links), this will define the pixel
clock.
URL:http://www.spinics.net/lists/linux-media/msg36492.html
After adding pixel clock to v4l2_mbus_framefmt there will be six reserved
fields left, one of which will be further possibly consumed by maximum image
size:
URL:http://www.spinics.net/lists/linux-media/msg35949.html
Frame blanking (horizontal and vertical) and number of lanes might be needed
in the struct as well in the future, bringing the reserved count down to
two. I find this alarmingly low for a relatively new structure definition
which will potentially have a few different uses in the future.
The another issue is that the size of the v4l2_subdev_format struct is not
aligned to a power of two. Instead of the intended 32 u32's, the size is
actually 22 u32's.
The interface is present in the 3.0 and marked experimental. My proposal is
to add reserved fields to v4l2_mbus_framefmt to extend its size up to 32
u32's. I understand there are already few which use the interface right now
and thus this change must be done now or left as-is forever.
Kind regards,
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Getting started with OMAP3 ISP
On Fri, Sep 2, 2011 at 1:27 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Friday 02 September 2011 11:02:23 Enrico wrote: Right now my problem is that i can't get the isp to generate interrupts, i think there is some isp configuration error. If your device generates interlaced images that's not surprising, as the CCDC will only receive half the number of lines it expects. Yes that was the first thing i tried, anyway now i have it finally working. Well at least yavta doesn't hang, do you know some application to see raw yuv images? Now the problem is that the fix is weird...as you suggested you must use half height values for VD0 and VD1 (2/3) interrupts, problem is that it only works if you DISABLE vd1 interrupt. If it is enabled the vd1_isr is run (once) and nothing else happens. Enrico -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: spca1528 device (Device 015: ID 04fc:1528 Sunplus Technology)..libv4l2: error turning on stream: Timer expired issue
On Mon, 05 Sep 2011 11:33:49 -0400 Mauricio Henriquez buhochil...@gmail.com wrote: Thanks Jean, yeap I apply the patch, but still the same kind of messages about timeout sing xawtv or svv: OK Mauricio. So, I need more information. May you set the gspca debug level to 0x0f echo 0x0f /sys/module/gspca_main/parameters/debug run 'svv' and send me the kernel messages starting from the last gspca open? Thanks. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cx18: Fix videobuf capture
When we moved to 3.0, we found that the cx18 driver was oopsing on close with:
NULL pointer deref at:
EIP: [c05ef145] __list_add+0x3e/0x81 SS:ESP 0068:f461be7c
[ 2290.461009] Call Trace:
[ 2290.461009] [c046007b] ? pm_qos_add_request+0xc/0x6e
[ 2290.461009] [c082631c] __mutex_lock_common+0x87/0x125
[ 2290.461009] [f8970e92] ? cx18_queue_flush+0x31/0x87 [cx18]
[ 2290.461009] [c0436b85] ? __might_sleep+0x29/0xe4
[ 2290.461009] [c0826515] __mutex_lock_slowpath+0x25/0x27
[ 2290.461009] [c08264b2] ? mutex_lock+0x2e/0x3b
[ 2290.461009] [c08264b2] mutex_lock+0x2e/0x3b
[ 2290.461009] [f88d3137] videobuf_queue_lock+0x13/0x15 [videobuf_core]
[ 2290.461009] [f88d3f86] __videobuf_free+0xfc/0x112 [videobuf_core]
[ 2290.461009] [f89741e6] cx18_v4l2_close+0x158/0x172 [cx18]
[ 2290.461009] [c0507522] ? cpumask_next+0x1a/0x1d
[ 2290.461009] [f88a319d] v4l2_release+0x35/0x52 [videodev]
[ 2290.461009] [c04f5717] fput+0x100/0x1a5
[ 2290.461009] [c04f2e09] filp_close+0x5c/0x64
[ 2290.461009] [c04f2e70] sys_close+0x5f/0x93
[ 2290.461009] [c082cd5f] sysenter_do_call+0x12/0x28
Some digging showed that a merge at some previous point partially
added broken mmap() support, causing this trace. Remove the broken
code completely.
On top of that, the calculation in place for buffer full depended on
UYUV instead of HM12, while our GStreamer code was picking HM12 in
some circumstances.
Finally, the V4L2_CAP_STREAMING capability was never exposed. Patch it
into the YUV encoder node only.
Signed-off-by: Simon Farnsworth simon.farnswo...@onelan.co.uk
---
This patch, against linuxtv/staging/for_v3.2, makes the upstream code
for videobuf-based YUV capture work for me. While it has a sign-off,
I'm sending it more in the spirit of not keeping bugfixes secret
rather than because I think it's in good state to apply.
drivers/media/video/cx18/cx18-driver.h |5 +
drivers/media/video/cx18/cx18-fileops.c |2 --
drivers/media/video/cx18/cx18-ioctl.c | 18 +++---
drivers/media/video/cx18/cx18-mailbox.c |2 +-
drivers/media/video/cx18/cx18-streams.c | 13 +
5 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/media/video/cx18/cx18-driver.h
b/drivers/media/video/cx18/cx18-driver.h
index 1834207..b9a94fc 100644
--- a/drivers/media/video/cx18/cx18-driver.h
+++ b/drivers/media/video/cx18/cx18-driver.h
@@ -409,6 +409,7 @@ struct cx18_stream {
/* Videobuf for YUV video */
u32 pixelformat;
+ u32 vb_bytes_per_frame;
struct list_head vb_capture;/* video capture queue */
spinlock_t vb_lock;
struct timer_list vb_timeout;
@@ -430,10 +431,6 @@ struct cx18_open_id {
u32 open_id;
int type;
struct cx18 *cx;
-
- struct videobuf_queue vbuf_q;
- spinlock_t s_lock; /* Protect vbuf_q */
- enum v4l2_buf_type vb_type;
};
static inline struct cx18_open_id *fh2id(struct v4l2_fh *fh)
diff --git a/drivers/media/video/cx18/cx18-fileops.c
b/drivers/media/video/cx18/cx18-fileops.c
index 07411f3..14cb961 100644
--- a/drivers/media/video/cx18/cx18-fileops.c
+++ b/drivers/media/video/cx18/cx18-fileops.c
@@ -784,8 +784,6 @@ int cx18_v4l2_close(struct file *filp)
cx18_release_stream(s);
} else {
cx18_stop_capture(id, 0);
- if (id-type == CX18_ENC_STREAM_TYPE_YUV)
- videobuf_mmap_free(id-vbuf_q);
}
kfree(id);
mutex_unlock(cx-serialize_lock);
diff --git a/drivers/media/video/cx18/cx18-ioctl.c
b/drivers/media/video/cx18/cx18-ioctl.c
index afe0a29..66b1c15 100644
--- a/drivers/media/video/cx18/cx18-ioctl.c
+++ b/drivers/media/video/cx18/cx18-ioctl.c
@@ -160,12 +160,7 @@ static int cx18_g_fmt_vid_cap(struct file *file, void *fh,
pixfmt-priv = 0;
if (id-type == CX18_ENC_STREAM_TYPE_YUV) {
pixfmt-pixelformat = s-pixelformat;
- /* HM12 YUV size is (Y=(h*720) + UV=(h*(720/2)))
- UYUV YUV size is (Y=(h*720) + UV=(h*(720))) */
- if (s-pixelformat == V4L2_PIX_FMT_HM12)
- pixfmt-sizeimage = pixfmt-height * 720 * 3 / 2;
- else
- pixfmt-sizeimage = pixfmt-height * 720 * 2;
+ pixfmt-sizeimage = s-vb_bytes_per_frame;
pixfmt-bytesperline = 720;
} else {
pixfmt-pixelformat = V4L2_PIX_FMT_MPEG;
@@ -296,6 +291,12 @@ static int cx18_s_fmt_vid_cap(struct file *file, void *fh,
return -EBUSY;
s-pixelformat = fmt-fmt.pix.pixelformat;
+ /* HM12 YUV size is (Y=(h*720) + UV=(h*(720/2)))
+ UYUV YUV size is (Y=(h*720) + UV=(h*(720))) */
+ if (s-pixelformat == V4L2_PIX_FMT_HM12)
+ s-vb_bytes_per_frame = h * 720 * 3 / 2;
+ else
+ s-vb_bytes_per_frame = h * 720 * 2;
mbus_fmt.width = cx-cxhdl.width = w;
mbus_fmt.height = cx-cxhdl.height = h;
@@ -463,13
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:Mon Sep 5 19:00:29 CEST 2011 git hash:1b19e42952963ae2a09a655f487de15b7c81c5b7 gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] s5p-fimc: Add runtime PM support in the mem-to-mem driver
Hi Sakari,
thanks for the comments.
On 09/05/2011 08:06 AM, Sakari Ailus wrote:
Hi Sylwester,
On Tue, Aug 30, 2011 at 05:00:39PM +0200, Sylwester Nawrocki wrote:
Add runtime PM and system sleep support in the memory-to-memory driver.
This is required to enable the device operation on Exynos4 SoCs. This patch
prevents system boot failure when the driver is compiled in, as it now
tries to access its I/O memory without first enabling the corresponding
power domain.
The camera capture device suspend/resume is not fully covered,
the capture device is just powered on/off during the video node
open/close. However this enables it's normal operation on Exynos4 SoCs.
Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com
Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com
---
I'm resending this patch after few minor changes since v3:
- added the driver dependency on PM_RUNTIME
- corrected the error path in probe()
- s/*_in_use/_busy
- edited the commit message
---
drivers/media/video/Kconfig |2 +-
drivers/media/video/s5p-fimc/fimc-capture.c | 18 ++
drivers/media/video/s5p-fimc/fimc-core.c| 279
---
drivers/media/video/s5p-fimc/fimc-core.h| 16 +-
drivers/media/video/s5p-fimc/fimc-reg.c |2 +-
5 files changed, 237 insertions(+), 80 deletions(-)
...
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 0d730e5..ea74e4b 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -17,6 +17,7 @@
#includelinux/interrupt.h
#includelinux/device.h
#includelinux/platform_device.h
+#includelinux/pm_runtime.h
#includelinux/list.h
#includelinux/slab.h
#includelinux/clk.h
@@ -196,6 +197,16 @@ static int fimc_stop_capture(struct fimc_dev *fimc)
return 0;
}
+int fimc_capture_suspend(struct fimc_dev *fimc)
+{
+return -EBUSY;
+}
+
+int fimc_capture_resume(struct fimc_dev *fimc)
+{
+return 0;
+}
+
static int start_streaming(struct vb2_queue *q)
{
struct fimc_ctx *ctx = q-drv_priv;
@@ -381,9 +392,14 @@ static int fimc_capture_open(struct file *file)
if (fimc_m2m_active(fimc))
return -EBUSY;
+ret = pm_runtime_get_sync(fimc-pdev-dev);
+if (ret)
+return ret;
+
if (++fimc-vid_cap.refcnt == 1) {
ret = fimc_isp_subdev_init(fimc, 0);
if (ret) {
+pm_runtime_put_sync(fimc-pdev-dev);
fimc-vid_cap.refcnt--;
return -EIO;
}
@@ -411,6 +427,8 @@ static int fimc_capture_close(struct file *file)
fimc_subdev_unregister(fimc);
}
+pm_runtime_put_sync(fimc-pdev-dev);
+
return 0;
}
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index aa55066..7ca8091 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -18,6 +18,7 @@
#includelinux/interrupt.h
#includelinux/device.h
#includelinux/platform_device.h
+#includelinux/pm_runtime.h
#includelinux/list.h
#includelinux/io.h
#includelinux/slab.h
@@ -301,7 +302,6 @@ int fimc_set_scaler_info(struct fimc_ctx *ctx)
static void fimc_m2m_job_finish(struct fimc_ctx *ctx, int vb_state)
{
struct vb2_buffer *src_vb, *dst_vb;
-struct fimc_dev *fimc = ctx-fimc_dev;
if (!ctx || !ctx-m2m_ctx)
return;
@@ -312,39 +312,48 @@ static void fimc_m2m_job_finish(struct fimc_ctx *ctx,
int vb_state)
if (src_vb dst_vb) {
v4l2_m2m_buf_done(src_vb, vb_state);
v4l2_m2m_buf_done(dst_vb, vb_state);
-v4l2_m2m_job_finish(fimc-m2m.m2m_dev, ctx-m2m_ctx);
+v4l2_m2m_job_finish(ctx-fimc_dev-m2m.m2m_dev,
+ctx-m2m_ctx);
}
}
/* Complete the transaction which has been scheduled for execution. */
-static void fimc_m2m_shutdown(struct fimc_ctx *ctx)
+static int fimc_m2m_shutdown(struct fimc_ctx *ctx)
{
struct fimc_dev *fimc = ctx-fimc_dev;
int ret;
if (!fimc_m2m_pending(fimc))
-return;
+return 0;
fimc_ctx_state_lock_set(FIMC_CTX_SHUT, ctx);
ret = wait_event_timeout(fimc-irq_queue,
!fimc_ctx_state_is_set(FIMC_CTX_SHUT, ctx),
FIMC_SHUTDOWN_TIMEOUT);
-/*
- * In case of a timeout the buffers are not released in the interrupt
- * handler so return them here with the error flag set, if there are
- * any on the queue.
- */
-if (ret == 0)
-fimc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
+
+return ret == 0 ? -ETIMEDOUT : ret;
+}
+
+static int start_streaming(struct vb2_queue *q)
+{
+struct fimc_ctx *ctx = q-drv_priv;
+int ret;
+
+ret =
Re: spca1528 device (Device 015: ID 04fc:1528 Sunplus Technology)..libv4l2: error turning on stream: Timer expired issue
On 09/05/2011 01:15 PM, Jean-Francois Moine wrote: On Mon, 05 Sep 2011 11:33:49 -0400 Mauricio Henriquezbuhochil...@gmail.com wrote: Thanks Jean, yeap I apply the patch, but still the same kind of messages about timeout sing xawtv or svv: OK Mauricio. So, I need more information. May you set the gspca debug level to 0x0f echo 0x0f /sys/module/gspca_main/parameters/debug run 'svv' and send me the kernel messages starting from the last gspca open? Ok, I set the gspca debug thing and this is the dmesg messages after running svv: gspca-2.13.6: [svv] open gspca-2.13.6: try fmt cap JPEG 640x480 gspca-2.13.6: try fmt cap JPEG 640x480 gspca-2.13.6: frame alloc frsz: 115790 gspca-2.13.6: reqbufs st:0 c:4 gspca-2.13.6: mmap start:b7739000 size:118784 gspca-2.13.6: mmap start:b7621000 size:118784 gspca-2.13.6: mmap start:b7604000 size:118784 gspca-2.13.6: mmap start:b75e7000 size:118784 gspca-2.13.6: init transfer alt 3 gspca-2.13.6: isoc 128 pkts size 512 = bsize:65536 spca1528-2.13.6: wait_status_0 timeout gspca-2.13.6: kill transfer gspca-2.13.6: [svv] close gspca-2.13.6: frame free gspca-2.13.6: close done gspca-2.13.6: [svv] open gspca-2.13.6: try fmt cap JPEG 640x480 gspca-2.13.6: try fmt cap JPEG 640x480 gspca-2.13.6: frame alloc frsz: 115790 gspca-2.13.6: reqbufs st:0 c:4 gspca-2.13.6: mmap start:b7732000 size:118784 gspca-2.13.6: mmap start:b761a000 size:118784 gspca-2.13.6: mmap start:b75fd000 size:118784 gspca-2.13.6: mmap start:b75e size:118784 gspca-2.13.6: init transfer alt 3 gspca-2.13.6: isoc 128 pkts size 512 = bsize:65536 spca1528-2.13.6: wait_status_0 timeout gspca-2.13.6: kill transfer gspca-2.13.6: [svv] close gspca-2.13.6: frame free gspca-2.13.6: close done ...hope you know what it mean :-) ..let me know if I can help in anything else, here I'm also playing with the code :-) Mauricio Thanks. -- Mauricio R. Henriquez Schott Escuela de Ingeniería en Computación Universidad Austral de Chile - Sede Puerto Montt Los Pinos S/N, Balneario de Pelluco, Puerto Montt - Chile Tel: 65-487440 Fax: 65-277156 mail: mauriciohenriq...@uach.cl attachment: buhochileno.vcf
Re: [PATCH v4] s5p-fimc: Add runtime PM support in the mem-to-mem driver
On Mon, Sep 05, 2011 at 09:01:35PM +0200, Sylwester Nawrocki wrote:
Hi Sakari,
thanks for the comments.
You're welcome!
On 09/05/2011 08:06 AM, Sakari Ailus wrote:
Hi Sylwester,
On Tue, Aug 30, 2011 at 05:00:39PM +0200, Sylwester Nawrocki wrote:
Add runtime PM and system sleep support in the memory-to-memory driver.
This is required to enable the device operation on Exynos4 SoCs. This patch
prevents system boot failure when the driver is compiled in, as it now
tries to access its I/O memory without first enabling the corresponding
power domain.
The camera capture device suspend/resume is not fully covered,
the capture device is just powered on/off during the video node
open/close. However this enables it's normal operation on Exynos4 SoCs.
Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com
Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com
---
I'm resending this patch after few minor changes since v3:
- added the driver dependency on PM_RUNTIME
- corrected the error path in probe()
- s/*_in_use/_busy
- edited the commit message
---
drivers/media/video/Kconfig |2 +-
drivers/media/video/s5p-fimc/fimc-capture.c | 18 ++
drivers/media/video/s5p-fimc/fimc-core.c| 279
---
drivers/media/video/s5p-fimc/fimc-core.h| 16 +-
drivers/media/video/s5p-fimc/fimc-reg.c |2 +-
5 files changed, 237 insertions(+), 80 deletions(-)
...
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c
b/drivers/media/video/s5p-fimc/fimc-capture.c
index 0d730e5..ea74e4b 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -17,6 +17,7 @@
#includelinux/interrupt.h
#includelinux/device.h
#includelinux/platform_device.h
+#includelinux/pm_runtime.h
#includelinux/list.h
#includelinux/slab.h
#includelinux/clk.h
@@ -196,6 +197,16 @@ static int fimc_stop_capture(struct fimc_dev *fimc)
return 0;
}
+int fimc_capture_suspend(struct fimc_dev *fimc)
+{
+ return -EBUSY;
+}
+
+int fimc_capture_resume(struct fimc_dev *fimc)
+{
+ return 0;
+}
+
static int start_streaming(struct vb2_queue *q)
{
struct fimc_ctx *ctx = q-drv_priv;
@@ -381,9 +392,14 @@ static int fimc_capture_open(struct file *file)
if (fimc_m2m_active(fimc))
return -EBUSY;
+ ret = pm_runtime_get_sync(fimc-pdev-dev);
+ if (ret)
+ return ret;
+
if (++fimc-vid_cap.refcnt == 1) {
ret = fimc_isp_subdev_init(fimc, 0);
if (ret) {
+ pm_runtime_put_sync(fimc-pdev-dev);
fimc-vid_cap.refcnt--;
return -EIO;
}
@@ -411,6 +427,8 @@ static int fimc_capture_close(struct file *file)
fimc_subdev_unregister(fimc);
}
+ pm_runtime_put_sync(fimc-pdev-dev);
+
return 0;
}
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c
b/drivers/media/video/s5p-fimc/fimc-core.c
index aa55066..7ca8091 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -18,6 +18,7 @@
#includelinux/interrupt.h
#includelinux/device.h
#includelinux/platform_device.h
+#includelinux/pm_runtime.h
#includelinux/list.h
#includelinux/io.h
#includelinux/slab.h
@@ -301,7 +302,6 @@ int fimc_set_scaler_info(struct fimc_ctx *ctx)
static void fimc_m2m_job_finish(struct fimc_ctx *ctx, int vb_state)
{
struct vb2_buffer *src_vb, *dst_vb;
- struct fimc_dev *fimc = ctx-fimc_dev;
if (!ctx || !ctx-m2m_ctx)
return;
@@ -312,39 +312,48 @@ static void fimc_m2m_job_finish(struct fimc_ctx
*ctx, int vb_state)
if (src_vb dst_vb) {
v4l2_m2m_buf_done(src_vb, vb_state);
v4l2_m2m_buf_done(dst_vb, vb_state);
- v4l2_m2m_job_finish(fimc-m2m.m2m_dev, ctx-m2m_ctx);
+ v4l2_m2m_job_finish(ctx-fimc_dev-m2m.m2m_dev,
+ ctx-m2m_ctx);
}
}
/* Complete the transaction which has been scheduled for execution. */
-static void fimc_m2m_shutdown(struct fimc_ctx *ctx)
+static int fimc_m2m_shutdown(struct fimc_ctx *ctx)
{
struct fimc_dev *fimc = ctx-fimc_dev;
int ret;
if (!fimc_m2m_pending(fimc))
- return;
+ return 0;
fimc_ctx_state_lock_set(FIMC_CTX_SHUT, ctx);
ret = wait_event_timeout(fimc-irq_queue,
!fimc_ctx_state_is_set(FIMC_CTX_SHUT, ctx),
FIMC_SHUTDOWN_TIMEOUT);
- /*
- * In case of a timeout the buffers are not released in the interrupt
- * handler so return them here with the error flag set, if there are
- * any on the queue.
- */
- if (ret == 0)
- fimc_m2m_job_finish(ctx, VB2_BUF_STATE_ERROR);
+
+ return ret == 0 ? -ETIMEDOUT : ret;
Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)
On Mon, Sep 05, 2011 at 12:16:42PM +0200, Hans Verkuil wrote:
On Monday, September 05, 2011 12:13:26 Hans Verkuil wrote:
The original order is correct, but what I missed is that for drivers
that release (free) everything in the videodev release callback the
v4l2_device struct is also freed and v4l2_device_put will fail.
To fix this, add this code just before the vdev-release call:
/* Do not call v4l2_device_put if there is no release callback set. */
if (v4l2_dev-release == NULL)
v4l2_dev = NULL;
If there is no release callback, then the refcounting is pointless
anyway.
This should work.
Note that in the long run using the v4l2_device release callback
instead of the videodev release is better. But it's a lot of work to
convert everything so that's long term. I'm quite surprised BTW that
this bug wasn't found much earlier.
This inline patch fixes the second poison overwritten problem so:
Tested-by: Sitsofe Wheeler sits...@yahoo.com
However, it does not prevent the original oops that was reported in the
original message. Yang Ruirui's patch in
https://lkml.org/lkml/2011/9/1/74 seems to be required to resolve
that initial problem - can it be ACK'd? Yang's patch is reproduced
inline below:
For uvc device, dev-vdev.dev is the intf-dev,
uvc_delete code is as below:
usb_put_intf(dev-intf);
usb_put_dev(dev-udev);
uvc_status_cleanup(dev);
uvc_ctrl_cleanup_device(dev);
## the intf dev is released above, so below code will oops.
if (dev-vdev.dev)
v4l2_device_unregister(dev-vdev);
Fix it by get_device in v4l2_device_register and put_device in
v4l2_device_disconnect
---
drivers/media/video/v4l2-device.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/video/v4l2-device.c
b/drivers/media/video/v4l2-device.c
index c72856c..e6a2c3b 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -38,6 +38,7 @@ int v4l2_device_register(struct device *dev, struct
v4l2_device *v4l2_dev)
mutex_init(v4l2_dev-ioctl_lock);
v4l2_prio_init(v4l2_dev-prio);
kref_init(v4l2_dev-ref);
+ get_device(dev);
v4l2_dev-dev = dev;
if (dev == NULL) {
/* If dev == NULL, then name must be filled in by the caller */
@@ -93,6 +94,7 @@ void v4l2_device_disconnect(struct v4l2_device *v4l2_dev)
if (dev_get_drvdata(v4l2_dev-dev) == v4l2_dev)
dev_set_drvdata(v4l2_dev-dev, NULL);
+ put_device(v4l2_dev-dev);
v4l2_dev-dev = NULL;
}
EXPORT_SYMBOL_GPL(v4l2_device_disconnect);
--
Sitsofe | http://sucs.org/~sits/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
checkpatch.pl WARNING: Do not use whitespace before Signed-off-by:
I am almost sure this have been working earlier, but now it seems like nothing is acceptable for checkpatch.pl! I did surely about 20 --amend and tried to remove everything, without luck. Could someone point out whats new acceptable logging format for checkpatch.pl ? [crope@localhost linux]$ git show 1b19e42952963ae2a09a655f487de15b7c81c5b7 |./scripts/checkpatch.pl - WARNING: Do not use whitespace before Signed-off-by: #10: Signed-off-by: Joe Perches j...@perches.com WARNING: Do not use whitespace before Signed-off-by: #11: Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com total: 0 errors, 2 warnings, 48 lines checked Your patch has style problems, please review. If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [media] at91: add code to initialize and manage theISI_MCK for Atmel ISI driver.
Hi, Russell
On 09/05/2011 6:34 PM Russell King wrote:
On Mon, Sep 05, 2011 at 06:29:53PM +0800, Josh Wu wrote:
+static int initialize_mck(struct atmel_isi *isi,
+struct isi_platform_data *pdata)
+{
+int ret;
+struct clk *pck_parent;
+
+if (!strlen(pdata-pck_name) || !strlen(pdata-pck_parent_name))
+return -EINVAL;
+
+/* ISI_MCK is provided by PCK clock */
+isi-mck = clk_get(NULL, pdata-pck_name);
No, this is not how you use the clk API. You do not pass clock names via
platform data.
You pass clk_get() the struct device. You then pass clk_get() a
_connection id_ on that _device_ if you have more than one struct clk
associated with the _device_. You then use clkdev to associate the
struct device plus the connection id with the appropriate struct clk.
I think I missed the struct device when called clk_get(). I will fix it in v2
version.
Best Regards,
Josh Wu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] [media] at91: add code to initialize and manage the ISI_MCK for Atmel ISI driver.
This patch enable the configuration for ISI_MCK, which is provided by
programmable clock.
Signed-off-by: Josh Wu josh...@atmel.com
---
drivers/media/video/atmel-isi.c | 60 ++-
include/media/atmel-isi.h |4 ++
2 files changed, 63 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 7b89f00..768bf59 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -90,7 +90,10 @@ struct atmel_isi {
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
struct completion complete;
+ /* ISI peripherial clock */
struct clk *pclk;
+ /* ISI_MCK, provided by PCK */
+ struct clk *mck;
unsigned intirq;
struct isi_platform_data*pdata;
@@ -763,6 +766,10 @@ static int isi_camera_add_device(struct soc_camera_device
*icd)
if (ret)
return ret;
+ ret = clk_enable(isi-mck);
+ if (ret)
+ return ret;
+
isi-icd = icd;
dev_dbg(icd-parent, Atmel ISI Camera driver attached to camera %d\n,
icd-devnum);
@@ -776,6 +783,7 @@ static void isi_camera_remove_device(struct
soc_camera_device *icd)
BUG_ON(icd != isi-icd);
+ clk_disable(isi-mck);
clk_disable(isi-pclk);
isi-icd = NULL;
@@ -882,6 +890,49 @@ static struct soc_camera_host_ops isi_soc_camera_host_ops
= {
};
/* ---*/
+/* Initialize ISI_MCK clock, called by atmel_isi_probe() function */
+static int initialize_mck(struct platform_device *pdev,
+ struct atmel_isi *isi)
+{
+ struct device *dev = pdev-dev;
+ struct isi_platform_data *pdata = dev-platform_data;
+ struct clk *pck_parent;
+ int ret;
+
+ if (!strlen(pdata-pck_name) || !strlen(pdata-pck_parent_name))
+ return -EINVAL;
+
+ /* ISI_MCK is provided by PCK clock */
+ isi-mck = clk_get(dev, pdata-pck_name);
+ if (IS_ERR(isi-mck)) {
+ dev_err(dev, Failed to get PCK: %s\n, pdata-pck_name);
+ return PTR_ERR(isi-mck);
+ }
+
+ pck_parent = clk_get(dev, pdata-pck_parent_name);
+ if (IS_ERR(pck_parent)) {
+ ret = PTR_ERR(pck_parent);
+ dev_err(dev, Failed to get PCK parent: %s\n,
+ pdata-pck_parent_name);
+ goto err_init_mck;
+ }
+
+ ret = clk_set_parent(isi-mck, pck_parent);
+ clk_put(pck_parent);
+ if (ret)
+ goto err_init_mck;
+
+ ret = clk_set_rate(isi-mck, pdata-isi_mck_hz);
+ if (ret 0)
+ goto err_init_mck;
+
+ return 0;
+
+err_init_mck:
+ clk_put(isi-mck);
+ return ret;
+}
+
static int __devexit atmel_isi_remove(struct platform_device *pdev)
{
struct soc_camera_host *soc_host = to_soc_camera_host(pdev-dev);
@@ -897,6 +948,7 @@ static int __devexit atmel_isi_remove(struct
platform_device *pdev)
isi-fb_descriptors_phys);
iounmap(isi-regs);
+ clk_put(isi-mck);
clk_put(isi-pclk);
kfree(isi);
@@ -915,7 +967,8 @@ static int __devinit atmel_isi_probe(struct platform_device
*pdev)
struct isi_platform_data *pdata;
pdata = dev-platform_data;
- if (!pdata || !pdata-data_width_flags) {
+ if (!pdata || !pdata-data_width_flags || !pdata-isi_mck_hz
+ || !pdata-pck_name || !pdata-pck_parent_name) {
dev_err(pdev-dev,
No config available for Atmel ISI\n);
return -EINVAL;
@@ -944,6 +997,11 @@ static int __devinit atmel_isi_probe(struct
platform_device *pdev)
INIT_LIST_HEAD(isi-video_buffer_list);
INIT_LIST_HEAD(isi-dma_desc_head);
+ /* Initialize ISI_MCK clock */
+ ret = initialize_mck(pdev, isi);
+ if (ret)
+ goto err_alloc_descriptors;
+
isi-p_fb_descriptors = dma_alloc_coherent(pdev-dev,
sizeof(struct fbd) * MAX_BUFFER_NUM,
isi-fb_descriptors_phys,
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 26cece5..dcbb822 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -114,6 +114,10 @@ struct isi_platform_data {
u32 data_width_flags;
/* Using for ISI_CFG1 */
u32 frate;
+ /* Using for ISI_MCK, provided by PCK */
+ u32 isi_mck_hz;
+ const char *pck_name;
+ const char *pck_parent_name;
};
#endif /* __ATMEL_ISI_H__ */
--
1.6.3.3
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at
