Re: [PATCH 3/6] [media] s5p-mfc: add support for VIDIOC_{G,S}_CROP to encoder
Hi John,
Thanks for the patches! It's nice to see fixes/improvements like this being
upstreamed.
Unfortunately I have to NACK this patch, but fortunately it is not difficult to
fix.
The main problem is that you use the wrong API: you need to use G/S_SELECTION
instead
of G/S_CROP. S_CROP on an output video node doesn't crop, it composes. And if
your
reaction is 'Huh?', then you're not alone. Which is why the selection API was
added.
The selection API can crop and compose for both capture and output nodes, and it
does what you expect.
You need to implement TGT_CROP, TGT_CROP_DEFAULT and TGT_CROP_BOUNDS. The latter
two are read-only and just return the current format's width and height.
Applications that today use S_CROP to set the crop rectangle for this driver's
output
need to be adapted to using the S_SELECTION API since S_CROP really doesn't do
what
they think it does.
Regards,
Hans
On 10/10/2013 01:49 AM, John Sheu wrote:
Allow userspace to set the crop rect of the input image buffer to
encode.
Signed-off-by: John Sheu s...@google.com
---
drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 6 ++-
drivers/media/platform/s5p-mfc/s5p_mfc_dec.c| 7 ++--
drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 54
-
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 16 +---
4 files changed, 70 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 6920b54..48f706f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -428,8 +428,10 @@ struct s5p_mfc_vp8_enc_params {
* struct s5p_mfc_enc_params - general encoding parameters
*/
struct s5p_mfc_enc_params {
- u16 width;
- u16 height;
+ u16 crop_left_offset;
+ u16 crop_right_offset;
+ u16 crop_top_offset;
+ u16 crop_bottom_offset;
u16 gop_size;
enum v4l2_mpeg_video_multi_slice_mode slice_mode;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 8faf969..e99bcb8 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -334,10 +334,9 @@ static int vidioc_g_fmt(struct file *file, void *priv,
struct v4l2_format *f)
ctx-state = MFCINST_HEAD_PARSED
ctx-state MFCINST_ABORT) {
/* This is run on CAPTURE (decode output) */
- /* Width and height are set to the dimensions
-of the movie, the buffer is bigger and
-further processing stages should crop to this
-rectangle. */
+ /* Width and height are set to the dimensions of the buffer,
+The movie's dimensions may be smaller; the cropping rectangle
+required should be queried with VIDIOC_G_CROP. */
pix_mp-width = ctx-buf_width;
pix_mp-height = ctx-buf_height;
pix_mp-field = V4L2_FIELD_NONE;
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 8b24829..4ad9349 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -1599,7 +1599,57 @@ static int vidioc_g_parm(struct file *file, void *priv,
a-parm.output.timeperframe.numerator =
ctx-enc_params.rc_framerate_denom;
} else {
- mfc_err(Setting FPS is only possible for the output queue\n);
+ mfc_err(Getting FPS is only possible for the output queue\n);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int vidioc_g_crop(struct file *file, void *priv, struct v4l2_crop *a)
+{
+ struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+ struct s5p_mfc_enc_params *p = ctx-enc_params;
+
+ if (a-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ a-c.left = p-crop_left_offset;
+ a-c.top = p-crop_top_offset;
+ a-c.width = ctx-img_width -
+ (p-crop_left_offset + p-crop_right_offset);
+ a-c.height = ctx-img_height -
+ (p-crop_top_offset + p-crop_bottom_offset);
+ } else {
+ mfc_err(Getting crop is only possible for the output queue\n);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int vidioc_s_crop(struct file *file, void *priv,
+ const struct v4l2_crop *a)
+{
+ struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+ struct s5p_mfc_enc_params *p = ctx-enc_params;
+
+ if (a-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+ int left, right, top, bottom;
+ left = round_down(a-c.left, 16);
+ right = ctx-img_width - (left + a-c.width);
+ top = round_down(a-c.top, 16);
+
[PATCH] s5p-jpeg: fix uninitialized use in hdr parse
For hdr parse error, it can return false without any assignments
which cause build warning.
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
---
drivers/media/platform/s5p-jpeg/jpeg-core.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 15d2396..7db374e 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -442,14 +442,14 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data
*result,
while (notfound) {
c = get_byte(jpeg_buffer);
if (c == -1)
- break;
+ return false;
if (c != 0xff)
continue;
do
c = get_byte(jpeg_buffer);
while (c == 0xff);
if (c == -1)
- break;
+ return false;
if (c == 0)
continue;
length = 0;
--
1.7.4.1
--
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] s5p-jpeg: fix encoder and decoder video dev names
It is hard to distinguish between decoder and encoder video device because their names are same. So this patch fixes the names. Signed-off-by: Seung-Woo Kim sw0312@samsung.com --- drivers/media/platform/s5p-jpeg/jpeg-core.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 15d2396..d5b4a0d 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1387,8 +1387,8 @@ static int s5p_jpeg_probe(struct platform_device *pdev) ret = -ENOMEM; goto vb2_allocator_rollback; } - strlcpy(jpeg-vfd_encoder-name, S5P_JPEG_M2M_NAME, - sizeof(jpeg-vfd_encoder-name)); + snprintf(jpeg-vfd_encoder-name, sizeof(jpeg-vfd_encoder-name), + %s-enc, S5P_JPEG_M2M_NAME); jpeg-vfd_encoder-fops = s5p_jpeg_fops; jpeg-vfd_encoder-ioctl_ops= s5p_jpeg_ioctl_ops; jpeg-vfd_encoder-minor= -1; @@ -1415,8 +1415,8 @@ static int s5p_jpeg_probe(struct platform_device *pdev) ret = -ENOMEM; goto enc_vdev_register_rollback; } - strlcpy(jpeg-vfd_decoder-name, S5P_JPEG_M2M_NAME, - sizeof(jpeg-vfd_decoder-name)); + snprintf(jpeg-vfd_decoder-name, sizeof(jpeg-vfd_decoder-name), + %s-dec, S5P_JPEG_M2M_NAME); jpeg-vfd_decoder-fops = s5p_jpeg_fops; jpeg-vfd_decoder-ioctl_ops= s5p_jpeg_ioctl_ops; jpeg-vfd_decoder-minor= -1; -- 1.7.4.1 -- 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: [RFC PATCH 1/4] mipi-dsi-bus: add MIPI DSI bus support
On 10/07/2013 12:47 PM, Bert Kenward wrote:
On Tuesday September 24 2013 at 15:23, Andrzej Hajda wrote:
MIPI DSI is a high-speed serial interface to transmit
data from/to host to display module.
Signed-off-by: Andrzej Hajda a.ha...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/video/display/Kconfig| 4 +
drivers/video/display/Makefile | 1 +
drivers/video/display/mipi-dsi-bus.c | 332
+++
include/video/display.h | 3 +
include/video/mipi-dsi-bus.h | 144 +++
5 files changed, 484 insertions(+)
snipped as far as mipi-dsi-bus.h
diff --git a/include/video/mipi-dsi-bus.h b/include/video/mipi-dsi-bus.h
new file mode 100644
index 000..a78792d
--- /dev/null
+++ b/include/video/mipi-dsi-bus.h
@@ -0,0 +1,144 @@
+/*
+ * MIPI DSI Bus
+ *
+ * Copyright (C) 2013, Samsung Electronics, Co., Ltd.
+ * Andrzej Hajda a.ha...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __MIPI_DSI_BUS_H__
+#define __MIPI_DSI_BUS_H__
+
+#include linux/device.h
+#include video/videomode.h
+
+struct mipi_dsi_bus;
+struct mipi_dsi_device;
+
+struct mipi_dsi_bus_ops {
+int (*set_power)(struct mipi_dsi_bus *bus, struct mipi_dsi_device
*dev,
+ bool on);
+int (*set_stream)(struct mipi_dsi_bus *bus, struct mipi_dsi_device
*dev,
+ bool on);
+int (*transfer)(struct mipi_dsi_bus *bus, struct mipi_dsi_device
*dev,
+u8 type, const u8 *tx_buf, size_t tx_len, u8 *rx_buf,
+size_t rx_len);
+};
+
+#define DSI_MODE_VIDEO (1 0)
+#define DSI_MODE_VIDEO_BURST(1 1)
+#define DSI_MODE_VIDEO_SYNC_PULSE (1 2)
+#define DSI_MODE_VIDEO_AUTO_VERT(1 3)
+#define DSI_MODE_VIDEO_HSE (1 4)
+#define DSI_MODE_VIDEO_HFP (1 5)
+#define DSI_MODE_VIDEO_HBP (1 6)
+#define DSI_MODE_VIDEO_HSA (1 7)
+#define DSI_MODE_VSYNC_FLUSH(1 8)
+#define DSI_MODE_EOT_PACKET (1 9)
+
+enum mipi_dsi_pixel_format {
+DSI_FMT_RGB888,
+DSI_FMT_RGB666,
+DSI_FMT_RGB666_PACKED,
+DSI_FMT_RGB565,
+};
+
+struct mipi_dsi_interface_params {
+enum mipi_dsi_pixel_format format;
+unsigned long mode;
+unsigned long hs_clk_freq;
+unsigned long esc_clk_freq;
+unsigned char data_lanes;
+unsigned char cmd_allow;
+};
+
+struct mipi_dsi_bus {
+struct device *dev;
+const struct mipi_dsi_bus_ops *ops;
+};
+
+#define MIPI_DSI_MODULE_PREFIX mipi-dsi:
+#define MIPI_DSI_NAME_SIZE 32
+
+struct mipi_dsi_device_id {
+char name[MIPI_DSI_NAME_SIZE];
+__kernel_ulong_t driver_data/* Data private to the driver */
+__aligned(sizeof(__kernel_ulong_t));
+};
+
+struct mipi_dsi_device {
+char name[MIPI_DSI_NAME_SIZE];
+int id;
+struct device dev;
+
+const struct mipi_dsi_device_id *id_entry;
+struct mipi_dsi_bus *bus;
+struct videomode vm;
+struct mipi_dsi_interface_params params;
+};
+
+#define to_mipi_dsi_device(d) container_of(d, struct
mipi_dsi_device, dev)
+
+int mipi_dsi_device_register(struct mipi_dsi_device *dev,
+ struct mipi_dsi_bus *bus);
+void mipi_dsi_device_unregister(struct mipi_dsi_device *dev);
+
+struct mipi_dsi_driver {
+int(*probe)(struct mipi_dsi_device *);
+int(*remove)(struct mipi_dsi_device *);
+struct device_driver driver;
+const struct mipi_dsi_device_id *id_table;
+};
+
+#define to_mipi_dsi_driver(d) container_of(d, struct
mipi_dsi_driver, driver)
+
+int mipi_dsi_driver_register(struct mipi_dsi_driver *drv);
+void mipi_dsi_driver_unregister(struct mipi_dsi_driver *drv);
+
+static inline void *mipi_dsi_get_drvdata(const struct mipi_dsi_device
*dev)
+{
+return dev_get_drvdata(dev-dev);
+}
+
+static inline void mipi_dsi_set_drvdata(struct mipi_dsi_device *dev,
+void *data)
+{
+dev_set_drvdata(dev-dev, data);
+}
+
+int of_mipi_dsi_register_devices(struct mipi_dsi_bus *bus);
+void mipi_dsi_unregister_devices(struct mipi_dsi_bus *bus);
+
+/* module_mipi_dsi_driver() - Helper macro for drivers that don't do
+ * anything special in module init/exit. This eliminates a lot of
+ * boilerplate. Each module may only use this macro once, and
+ * calling it replaces module_init() and module_exit()
+ */
+#define module_mipi_dsi_driver(__mipi_dsi_driver) \
+module_driver(__mipi_dsi_driver, mipi_dsi_driver_register, \
+mipi_dsi_driver_unregister)
+
+int mipi_dsi_set_power(struct mipi_dsi_device *dev, bool on);
+int mipi_dsi_set_stream(struct
Re: em28xx + ov2640 and v4l2-clk
Am 08.10.2013 18:38, schrieb Guennadi Liakhovetski: Hi Frank, On Tue, 8 Oct 2013, Frank SchÀfer wrote: Am 18.08.2013 17:20, schrieb Mauro Carvalho Chehab: Em Sun, 18 Aug 2013 13:40:25 +0200 Frank SchÀfer fschaefer@googlemail.com escreveu: Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski: Hi Frank, As I mentioned on the list, I'm currently on a holiday, so, replying briefly. Sorry, I missed that (can't read all mails on the list). Since em28xx is a USB device, I conclude, that it's supplying clock to its components including the ov2640 sensor. So, yes, I think the driver should export a V4L2 clock. Ok, so it's mandatory on purpose ? I'll take a deeper into the v4l2-clk code and the em28xx/ov2640/soc-camera interaction this week. Have a nice holiday ! commit 9aea470b399d797e88be08985c489855759c6c60 Author: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Fri Dec 21 13:01:55 2012 -0300 [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk Instead of centrally enabling and disabling subdevice master clocks in soc-camera core, let subdevice drivers do that themselves, using the V4L2 clock API and soc-camera convenience wrappers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com (c/c the ones that acked with this broken changeset) We need to fix it ASAP or to revert the ov2640 changes, as some em28xx cameras are currently broken on 3.10. I'll also reject other ports to the async API if the drivers are used outside an embedded driver, as no PC driver currently defines any clock source. The same applies to regulators. Guennadi, Next time, please check if the i2c drivers are used outside soc_camera and apply the fixes where needed, as no regressions are allowed. Regards, Mauro FYI: 8 weeks have passed by now and this regression has still not been fixed. Does anybody care about it ? WONTFIX ? You replied to my patch em28xx: balance subdevice power-off calls with a few non-essential IMHO comments but you didn't test it. Non-essential comments ? Maybe you disagree or don't care about them, but that's something different. Could you test, please? Yes, this patch will make the warnings disappear and works at least for my em28xx+ov2640 device. What about Mauros an my concerns with regards to all other em28xx devices ? And what about the em28xx v4l2-clk patches ? It's pretty simple: someone (usually the maintainer ;) ) needs to decide which way to go. Either accept and apply the existing patches or request new ones with changes. But IMHO doing nothing for 2 months isn't the right way to handle regressions. Regards, Frank In the meantime I'm still waiting for more comments to my [RFD] use-counting V4L2 clocks mail, so far only Sylwester has replied. Without all these we don't seem to progress very well. Thanks Guennadi -Original Message- From: Frank SchÀfer fschaefer@googlemail.com To: Guennadi Liakhovetski g.liakhovet...@gmx.de, Linux Media Mailing List linux-media@vger.kernel.org Sent: Fr., 16 Aug 2013 21:03 Subject: em28xx + ov2640 and v4l2-clk Hi Guennadi, since commit 9aea470b399d797e88be08985c489855759c6c60 soc-camera: switch I2C subdevice drivers to use v4l2-clk, the em28xx driver fails to register the ov2640 subdevice (if needed). The reason is that v4l2_clk_get() fails in ov2640_probe(). Does the em28xx driver have to register a (pseudo ?) clock first ? Regards, Frank --- 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 -- 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] Add support for KWorld UB435-Q V2
Am 09.10.2013 15:48, schrieb Jean-Francois Thibert: On Tue, Oct 8, 2013 at 12:24 PM, Frank Schäfer fschaefer@googlemail.com wrote: This adds support for the UB435-Q V2. It seems that you might need to use the device once with the official driver to reprogram the device descriptors. Thanks to Jarod Wilson for the initial attempt at adding support for this device. Could you please elaborate on this ? What's the official driver and what changes after using it ? Are these changes permanent ? From what I understand the Windows driver provided by KWorld will reprogram the eeprom so that the device descriptors are properly describing an Isochronous endpoint instead of Bulk. This only needs to be done once since it is permanent. I don't know if this is required since I don't have a device in the other state. What do you mean with properly describing an Isoc endpoint ? Did the Windows driver reprogram the device to provide an isoc instead of a bulk video endpoint ? Or did the device have no video endpoint and the bulk endpoint was used for something different (likely device programming) ? Any chance to get lsusb outputs ? If you don't have a device in the other state and didn't test the bulk endpoint's, how did you notice this issue ? Regards, Frank The commit message should be included in the patch and not be sent as a separate message. Can you fix the patch and resend it ? Sure, I will resend the patch with the message included. Regards Jean-Francois -- 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] v4l: tuner-core: fix typo
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/media/v4l2-core/tuner-core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/tuner-core.c
b/drivers/media/v4l2-core/tuner-core.c
index ddc9379..4b8a9a3 100644
--- a/drivers/media/v4l2-core/tuner-core.c
+++ b/drivers/media/v4l2-core/tuner-core.c
@@ -247,7 +247,7 @@ static const struct analog_demod_ops tuner_analog_ops = {
/**
* set_type - Sets the tuner type for a given device
*
- * @c: i2c_client descriptoy
+ * @c: i2c_client descriptor
* @type: type of the tuner (e. g. tuner number)
* @new_mode_mask: Indicates if tuner supports TV and/or Radio
* @new_config:an optional parameter used by a few tuners to
adjust
--
1.7.9.5
--
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 v2] Add support for KWorld UB435-Q V2
Am 09.10.2013 16:18, schrieb Jean-Francois Thibert:
This patch adds support for the UB435-Q V2. You might need to
use the device once with the Windows driver provided by KWorld
in order to permanently reprogram the device descriptors. Thanks
to Jarod Wilson for the initial attempt at adding support for this
device.
Signed-off-by: Jean-Francois Thibert jfthib...@google.com
---
drivers/media/usb/em28xx/em28xx-cards.c | 14 +-
drivers/media/usb/em28xx/em28xx-dvb.c | 27 +++
drivers/media/usb/em28xx/em28xx.h |1 +
3 files changed, 41 insertions(+), 1 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index dc65742..a512909 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -174,7 +174,7 @@ static struct em28xx_reg_seq evga_indtube_digital[] = {
};
/*
- * KWorld PlusTV 340U and UB435-Q (ATSC) GPIOs map:
+ * KWorld PlusTV 340U, UB435-Q and UB435-Q V2 (ATSC) GPIOs map:
* EM_GPIO_0 - currently unknown
* EM_GPIO_1 - LED disable/enable (1 = off, 0 = on)
* EM_GPIO_2 - currently unknown
@@ -2030,6 +2030,16 @@ struct em28xx_board em28xx_boards[] = {
.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
+ /* 1b80:e346 KWorld USB ATSC TV Stick UB435-Q V2
+ * Empia EM2874B + LG DT3305 + NXP TDA18271HDC2 */
+ [EM2874_BOARD_KWORLD_UB435Q_V2] = {
+ .name = KWorld USB ATSC TV Stick UB435-Q V2,
+ .tuner_type = TUNER_ABSENT,
+ .has_dvb= 1,
+ .dvb_gpio = kworld_a340_digital,
+ .tuner_gpio = default_tuner_gpio,
+ .def_i2c_bus = 1,
+ },
};
const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
@@ -2173,6 +2183,8 @@ struct usb_device_id em28xx_id_table[] = {
.driver_info = EM2860_BOARD_GADMEI_UTV330 },
{ USB_DEVICE(0x1b80, 0xa340),
.driver_info = EM2870_BOARD_KWORLD_A340 },
+ { USB_DEVICE(0x1b80, 0xe346),
+ .driver_info = EM2874_BOARD_KWORLD_UB435Q_V2 },
{ USB_DEVICE(0x2013, 0x024f),
.driver_info = EM28174_BOARD_PCTV_290E },
{ USB_DEVICE(0x2013, 0x024c),
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
b/drivers/media/usb/em28xx/em28xx-dvb.c
index bb1e8dc..547eea6 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -298,6 +298,18 @@ static struct lgdt3305_config em2870_lgdt3304_dev = {
.qam_if_khz = 4000,
};
+static struct lgdt3305_config em2874_lgdt3305_dev = {
+ .i2c_addr = 0x0e,
+ .demod_chip = LGDT3305,
+ .spectral_inversion = 1,
+ .deny_i2c_rptr = 0,
+ .mpeg_mode = LGDT3305_MPEG_SERIAL,
+ .tpclk_edge = LGDT3305_TPCLK_FALLING_EDGE,
+ .tpvalid_polarity = LGDT3305_TP_VALID_HIGH,
+ .vsb_if_khz = 3250,
+ .qam_if_khz = 4000,
+};
+
static struct s921_config sharp_isdbt = {
.demod_address = 0x30 1
};
@@ -329,6 +341,12 @@ static struct tda18271_config kworld_a340_config = {
.std_map = kworld_a340_std_map,
};
+static struct tda18271_config kworld_ub435q_v2_config = {
+ .std_map = kworld_a340_std_map,
+ .gate = TDA18271_GATE_DIGITAL,
TDA18271_GATE_AUTO doesn't work ? Then you could use kworld_a340_config.
Or the other way around: wouldn't TDA18271_GATE_DIGITAL also work for
the A340 ?
Apart from that:
Reviewed-by: Frank Schäfer fschaefer@googlemail.com
Regards,
Frank
+};
+
+
static struct zl10353_config em28xx_zl10353_xc3028_no_i2c_gate = {
.demod_address = (0x1e 1),
.no_tuner = 1,
@@ -1297,6 +1315,15 @@ static int em28xx_dvb_init(struct em28xx *dev)
goto out_free;
}
break;
+ case EM2874_BOARD_KWORLD_UB435Q_V2:
+ dvb-fe[0] = dvb_attach(lgdt3305_attach,
+ em2874_lgdt3305_dev,
+ dev-i2c_adap[dev-def_i2c_bus]);
+ if (dvb-fe[0] != NULL)
+ dvb_attach(tda18271_attach, dvb-fe[0], 0x60,
+ dev-i2c_adap[dev-def_i2c_bus], kworld_ub435q_v2_config);
+
+ break;
default:
em28xx_errdev(/2: The frontend of your DVB/ATSC card
isn't supported yet\n);
diff --git a/drivers/media/usb/em28xx/em28xx.h
b/drivers/media/usb/em28xx/em28xx.h
index 205e903..6d988ad 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -131,6 +131,7 @@
#define EM2884_BOARD_TERRATEC_HTC_USB_XS 87
#define EM2884_BOARD_C3TECH_DIGITAL_DUO 88
#define EM2874_BOARD_DELOCK_61959 89
+#define EM2874_BOARD_KWORLD_UB435Q_V2 90
/* Limits minimum and default number of buffers */
#define EM28XX_MIN_BUF 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
Re: em28xx + ov2640 and v4l2-clk
Hi Frank, On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 08.10.2013 18:38, schrieb Guennadi Liakhovetski: Hi Frank, On Tue, 8 Oct 2013, Frank SchÀfer wrote: Am 18.08.2013 17:20, schrieb Mauro Carvalho Chehab: Em Sun, 18 Aug 2013 13:40:25 +0200 Frank SchÀfer fschaefer@googlemail.com escreveu: Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski: Hi Frank, As I mentioned on the list, I'm currently on a holiday, so, replying briefly. Sorry, I missed that (can't read all mails on the list). Since em28xx is a USB device, I conclude, that it's supplying clock to its components including the ov2640 sensor. So, yes, I think the driver should export a V4L2 clock. Ok, so it's mandatory on purpose ? I'll take a deeper into the v4l2-clk code and the em28xx/ov2640/soc-camera interaction this week. Have a nice holiday ! commit 9aea470b399d797e88be08985c489855759c6c60 Author: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Fri Dec 21 13:01:55 2012 -0300 [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk Instead of centrally enabling and disabling subdevice master clocks in soc-camera core, let subdevice drivers do that themselves, using the V4L2 clock API and soc-camera convenience wrappers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com (c/c the ones that acked with this broken changeset) We need to fix it ASAP or to revert the ov2640 changes, as some em28xx cameras are currently broken on 3.10. I'll also reject other ports to the async API if the drivers are used outside an embedded driver, as no PC driver currently defines any clock source. The same applies to regulators. Guennadi, Next time, please check if the i2c drivers are used outside soc_camera and apply the fixes where needed, as no regressions are allowed. Regards, Mauro FYI: 8 weeks have passed by now and this regression has still not been fixed. Does anybody care about it ? WONTFIX ? You replied to my patch em28xx: balance subdevice power-off calls with a few non-essential IMHO comments but you didn't test it. Non-essential comments ? Maybe you disagree or don't care about them, but that's something different. Firstly, I did say IMHO, didn't I? Secondly, sure, let's have a look at them: I wonder if we should make the (s_power, 1) call part of em28xx_wake_i2c(). Is this an essential comment? Is it essential where to put an operation after a function or after it? em28xx_set_mode() calls em28xx_gpio_set(dev, INPUT(dev-ctl_input)-gpio) and I'm not sure if this could disable subdevice power again... You aren't sure about that. Me neither, so, there's no evidence whatsoever. This is just a guess. And I would consider switching subdevice power in a *_set_mode() function by explicitly toggling a GPIO in presence of proper APIs... not the best design perhaps. I consider this comment non-essential too then. Hmm... your patch didn't change this, but: Why do we call these functions only in case of V4L2_BUF_TYPE_VIDEO_CAPTURE ? Isn't it needed for VBI capturing, too ? em28xx_wake_i2c() is probably also needed for radio mode... Right, my patch doesn't change this, so, this is unrelated. Have I missed anything? Could you test, please? Yes, this patch will make the warnings disappear and works at least for my em28xx+ov2640 device. Good, thanks for testing! What about Mauros an my concerns with regards to all other em28xx devices ? This is still under discussion: http://www.mail-archive.com/linux-media@vger.kernel.org/msg66566.html And what about the em28xx v4l2-clk patches ? Their acceptance is related to the above. Thanks Guennadi It's pretty simple: someone (usually the maintainer ;) ) needs to decide which way to go. Either accept and apply the existing patches or request new ones with changes. But IMHO doing nothing for 2 months isn't the right way to handle regressions. Regards, Frank In the meantime I'm still waiting for more comments to my [RFD] use-counting V4L2 clocks mail, so far only Sylwester has replied. Without all these we don't seem to progress very well. Thanks Guennadi -Original Message- From: Frank SchÀfer fschaefer@googlemail.com To: Guennadi Liakhovetski g.liakhovet...@gmx.de, Linux Media Mailing List linux-media@vger.kernel.org Sent: Fr., 16 Aug 2013 21:03 Subject: em28xx + ov2640 and v4l2-clk Hi Guennadi, since commit 9aea470b399d797e88be08985c489855759c6c60 soc-camera: switch I2C subdevice drivers to use v4l2-clk, the em28xx driver fails to register the ov2640 subdevice (if needed). The reason is that v4l2_clk_get() fails in ov2640_probe(). Does the em28xx driver have to register a (pseudo
Re: [Trivial PATCH] media: Remove unnecessary semicolons
On Tue, Oct 08, 2013 at 04:29:08PM -0700, Joe Perches wrote: These aren't necessary after switch and while statements. Signed-off-by: Joe Perches j...@perches.com Reviewed-by: Sakari Ailus sakari.ai...@iki.fi -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: 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
Fw: dvb-apps/util/scan: zap compatible with w_scan (option)
Hi.
The feature is useful if you have file generated by w_scan -X and
your card is just tuned and works. I could use scan -c -o zap -a 0 to
update the file but it wan't full compatible with w_scan. Now it can be
with -p option like for VDR output format, ie. scan -c -o zap -a 0
-p. I would be nice to have the feature in mainline.
best regards
Janusz Uzycki
ELPROMA
From: Janusz Uzycki j.uzy...@elproma.com.pl
Date: Wed Oct 9 14:51:39 2013 +0200
Subject: scan: zap output compatible with w_scan
w_scan -X selects tzap/czap/xine output format.
It starts zap output as service_name(provider_name):, not service_name:.
Thanks to the patch scan -o zap -p generates compatible output.
Signed-off-by: Janusz Uzycki j.uzy...@elproma.com.pl
---
diff -r 3ee111da5b3a util/scan/dump-zap.c
--- a/util/scan/dump-zap.c Mon May 13 15:49:02 2013 +0530
+++ b/util/scan/dump-zap.c Wed Oct 09 17:49:35 2013 +0200
@@ -110,14 +110,19 @@
void zap_dump_service_parameter_set (FILE *f,
const char *service_name,
+ const char *provider_name,
fe_type_t type,
struct dvb_frontend_parameters *p,
char polarity,
int sat_number,
uint16_t video_pid,
uint16_t *audio_pid,
- uint16_t service_id)
+ uint16_t service_id,
+ int dump_provider)
{
+ if (dump_provider == 1)
+ fprintf (f, %s(%s):, service_name, provider_name); /* w_scan
tzap/czap/xine output format */
+ else
fprintf (f, %s:, service_name);
zap_dump_dvb_parameters (f, type, p, polarity, sat_number);
fprintf (f, :%i:%i:%i, video_pid, audio_pid[0], service_id);
diff -r 3ee111da5b3a util/scan/dump-zap.h
--- a/util/scan/dump-zap.h Mon May 13 15:49:02 2013 +0530
+++ b/util/scan/dump-zap.h Wed Oct 09 17:49:35 2013 +0200
@@ -9,11 +9,13 @@
extern void zap_dump_service_parameter_set (FILE *f,
const char *service_name,
+ const char *provider_name,
fe_type_t type,
struct dvb_frontend_parameters *t,
char polarity, int sat,
uint16_t video_pid,
uint16_t *audio_pid,
- uint16_t service_id);
+ uint16_t service_id,
+ int dump_provider);
#endif
diff -r 3ee111da5b3a util/scan/scan.c
--- a/util/scan/scan.c Mon May 13 15:49:02 2013 +0530
+++ b/util/scan/scan.c Wed Oct 09 17:49:35 2013 +0200
@@ -2471,13 +2471,15 @@
case OUTPUT_ZAP:
zap_dump_service_parameter_set (stdout,
s-service_name,
+ s-provider_name,
t-type,
t-param,
sat_polarisation(t),
sat_number(t),
s-video_pid,
s-audio_pid,
- s-service_id);
+ s-service_id,
+ vdr_dump_provider);
default:
break;
}
@@ -2539,7 +2541,7 @@
N=xxx sets ca field in vdr output to :xxx:\n
-t N Service select, Combined bitfield parameter.\n
1 = TV, 2 = Radio, 4 = Other, (default 7)\n
- -p for vdr output format: dump provider name\n
+ -p for vdr / zap output format: dump provider name\n
-e N VDR version, default 3 for VDR-1.3.x and newer\n
value 2 sets NIT and TID to zero\n
Vdr version 1.3.x and up implies -p.\n
zap_provider.patch
Description: Binary data
Re: em28xx + ov2640 and v4l2-clk
Am 10.10.2013 15:50, schrieb Guennadi Liakhovetski: Hi Frank, On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 08.10.2013 18:38, schrieb Guennadi Liakhovetski: Hi Frank, On Tue, 8 Oct 2013, Frank SchÀfer wrote: Am 18.08.2013 17:20, schrieb Mauro Carvalho Chehab: Em Sun, 18 Aug 2013 13:40:25 +0200 Frank SchÀfer fschaefer@googlemail.com escreveu: Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski: Hi Frank, As I mentioned on the list, I'm currently on a holiday, so, replying briefly. Sorry, I missed that (can't read all mails on the list). Since em28xx is a USB device, I conclude, that it's supplying clock to its components including the ov2640 sensor. So, yes, I think the driver should export a V4L2 clock. Ok, so it's mandatory on purpose ? I'll take a deeper into the v4l2-clk code and the em28xx/ov2640/soc-camera interaction this week. Have a nice holiday ! commit 9aea470b399d797e88be08985c489855759c6c60 Author: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Fri Dec 21 13:01:55 2012 -0300 [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk Instead of centrally enabling and disabling subdevice master clocks in soc-camera core, let subdevice drivers do that themselves, using the V4L2 clock API and soc-camera convenience wrappers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com (c/c the ones that acked with this broken changeset) We need to fix it ASAP or to revert the ov2640 changes, as some em28xx cameras are currently broken on 3.10. I'll also reject other ports to the async API if the drivers are used outside an embedded driver, as no PC driver currently defines any clock source. The same applies to regulators. Guennadi, Next time, please check if the i2c drivers are used outside soc_camera and apply the fixes where needed, as no regressions are allowed. Regards, Mauro FYI: 8 weeks have passed by now and this regression has still not been fixed. Does anybody care about it ? WONTFIX ? You replied to my patch em28xx: balance subdevice power-off calls with a few non-essential IMHO comments but you didn't test it. Non-essential comments ? Maybe you disagree or don't care about them, but that's something different. Firstly, I did say IMHO, didn't I? Secondly, sure, let's have a look at them: I wonder if we should make the (s_power, 1) call part of em28xx_wake_i2c(). Is this an essential comment? Is it essential where to put an operation after a function or after it? It's a proposal. em28xx_set_mode() calls em28xx_gpio_set(dev, INPUT(dev-ctl_input)-gpio) and I'm not sure if this could disable subdevice power again... You aren't sure about that. Me neither, so, there's no evidence whatsoever. This is just a guess. And I would consider switching subdevice power in a *_set_mode() function by explicitly toggling a GPIO in presence of proper APIs... not the best design perhaps. I consider this comment non-essential too then. It's a warning/indicator that the whole s_power thing is very dangerous because of the various GPIO-sequences used everywhere in the driver. And it substantiates what Mauro tries to explain you. Hmm... your patch didn't change this, but: Why do we call these functions only in case of V4L2_BUF_TYPE_VIDEO_CAPTURE ? Isn't it needed for VBI capturing, too ? em28xx_wake_i2c() is probably also needed for radio mode... Right, my patch doesn't change this, so, this is unrelated. Ok, I have to admit that I wasn't clear enough in this case: IMHO these are bugs that should be fixed, but I'm not 100% sure. In that case, there is no need to split the if-caluse containing the V4L2_BUF_TYPE_VIDEO_CAPTURE check, just remove this check while you're at it. Have I missed anything? It seems we have a different understanding about the meaning of the word (non)-essential. I wouldn't use it in the context of review feedbacks. I'm not in the position to force you to consider my remarks, so from your point of view they are certainly optional (and that's no problem for me). Maybe that's what you mean with non-essential ? ;) Could you test, please? Yes, this patch will make the warnings disappear and works at least for my em28xx+ov2640 device. Good, thanks for testing! What about Mauros an my concerns with regards to all other em28xx devices ? This is still under discussion: How long are you going to discuss this ? We are not talking about a new feature or improvement/extension. This is about fixing a regression, which should always have highest priority. 8 weeks IMHO should be more than enough. http://www.mail-archive.com/linux-media@vger.kernel.org/msg66566.html And what about the em28xx v4l2-clk patches ? Their acceptance is related to the above. Why ? The 3 patches
[PATCH 1/2] v4l2-ctrls: fix typo in header file media/v4l2-ctrls.h
Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- include/media/v4l2-ctrls.h |2 +- 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 47ada23..16f7f26 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -571,7 +571,7 @@ static inline void v4l2_ctrl_lock(struct v4l2_ctrl *ctrl) mutex_lock(ctrl-handler-lock); } -/** v4l2_ctrl_lock() - Helper function to unlock the handler +/** v4l2_ctrl_unlock() - Helper function to unlock the handler * associated with the control. * @ctrl: The control to unlock. */ -- 1.7.10.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
Re: [PATCH 1/2] v4l2-ctrls: fix typo in header file media/v4l2-ctrls.h
Sorry, there is no PATCH 2/2, this one is the only one. :) Frank Am 10.10.2013 19:21, schrieb Frank Schäfer: Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- include/media/v4l2-ctrls.h |2 +- 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 47ada23..16f7f26 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -571,7 +571,7 @@ static inline void v4l2_ctrl_lock(struct v4l2_ctrl *ctrl) mutex_lock(ctrl-handler-lock); } -/** v4l2_ctrl_lock() - Helper function to unlock the handler +/** v4l2_ctrl_unlock() - Helper function to unlock the handler * associated with the control. * @ctrl: The control to unlock. */ -- 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] em28xx: fix and unify the coding style of the GPIO register write sequences
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/usb/em28xx/em28xx-cards.c | 102 +++
drivers/media/usb/em28xx/em28xx-dvb.c | 16 ++---
2 Dateien geändert, 59 Zeilen hinzugefügt(+), 59 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index dc65742..3765670 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -95,8 +95,8 @@ static struct em28xx_reg_seq default_digital[] = {
/* Board Hauppauge WinTV HVR 900 analog */
static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x2d, ~EM_GPIO_4, 10},
- {0x05, 0xff, 0x10, 10},
- { -1, -1, -1, -1},
+ { 0x05, 0xff, 0x10, 10},
+ { -1, -1, -1, -1},
};
/* Board Hauppauge WinTV HVR 900 digital */
@@ -104,20 +104,20 @@ static struct em28xx_reg_seq
hauppauge_wintv_hvr_900_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x2e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x04, 0x0f, 10},
{EM2880_R04_GPO,0x0c, 0x0f, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
/* Board Hauppauge WinTV HVR 900 (R2) digital */
static struct em28xx_reg_seq hauppauge_wintv_hvr_900R2_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x2e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x0c, 0x0f, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
/* Boards - EM2880 MSI DIGIVOX AD and EM2880_BOARD_MSI_DIGIVOX_AD_II */
static struct em28xx_reg_seq em2880_msi_digivox_ad_analog[] = {
- {EM2820_R08_GPIO_CTRL, 0x69, ~EM_GPIO_4, 10},
- { -1, -1, -1, -1},
+ {EM2820_R08_GPIO_CTRL, 0x69, ~EM_GPIO_4, 10},
+ { -1, -1, -1, -1},
};
/* Boards - EM2880 MSI DIGIVOX AD and EM2880_BOARD_MSI_DIGIVOX_AD_II */
@@ -132,7 +132,7 @@ static struct em28xx_reg_seq em2882_kworld_315u_digital[] =
{
{EM2880_R04_GPO,0x04, 0xff, 10},
{EM2880_R04_GPO,0x0c, 0xff, 10},
{EM2820_R08_GPIO_CTRL, 0x7e, 0xff, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
static struct em28xx_reg_seq em2882_kworld_315u_tuner_gpio[] = {
@@ -140,19 +140,19 @@ static struct em28xx_reg_seq
em2882_kworld_315u_tuner_gpio[] = {
{EM2880_R04_GPO,0x0c, 0xff, 10},
{EM2880_R04_GPO,0x08, 0xff, 10},
{EM2880_R04_GPO,0x0c, 0xff, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
static struct em28xx_reg_seq kworld_330u_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x00, 0xff, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
static struct em28xx_reg_seq kworld_330u_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x6e, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x08, 0xff, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
/* Evga inDtube
@@ -170,7 +170,7 @@ static struct em28xx_reg_seq evga_indtube_digital[] = {
{EM2820_R08_GPIO_CTRL, 0x7a, 0xff,1},
{EM2880_R04_GPO,0x04, 0xff, 10},
{EM2880_R04_GPO,0x0c, 0xff,1},
- { -1, -1, -1, -1},
+ { -1, -1, -1, -1},
};
/*
@@ -185,8 +185,8 @@ static struct em28xx_reg_seq evga_indtube_digital[] = {
* EM_GPIO_7 - currently unknown
*/
static struct em28xx_reg_seq kworld_a340_digital[] = {
- {EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
- { -1, -1, -1, -1},
+ {EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
+ { -1, -1, -1, -1},
};
/* Pinnacle Hybrid Pro eb1a:2881 */
@@ -205,13 +205,13 @@ static struct em28xx_reg_seq
pinnacle_hybrid_pro_digital[] = {
static struct em28xx_reg_seq terratec_cinergy_USB_XS_FR_analog[] = {
{EM2820_R08_GPIO_CTRL, 0x6d, ~EM_GPIO_4, 10},
{EM2880_R04_GPO,0x00, 0xff, 10},
- { -1, -1, -1, -1},
+ { -1, -1, -1,
Re: em28xx + ov2640 and v4l2-clk
On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 10.10.2013 15:50, schrieb Guennadi Liakhovetski: Hi Frank, On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 08.10.2013 18:38, schrieb Guennadi Liakhovetski: Hi Frank, On Tue, 8 Oct 2013, Frank SchÀfer wrote: Am 18.08.2013 17:20, schrieb Mauro Carvalho Chehab: Em Sun, 18 Aug 2013 13:40:25 +0200 Frank SchÀfer fschaefer@googlemail.com escreveu: Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski: Hi Frank, As I mentioned on the list, I'm currently on a holiday, so, replying briefly. Sorry, I missed that (can't read all mails on the list). Since em28xx is a USB device, I conclude, that it's supplying clock to its components including the ov2640 sensor. So, yes, I think the driver should export a V4L2 clock. Ok, so it's mandatory on purpose ? I'll take a deeper into the v4l2-clk code and the em28xx/ov2640/soc-camera interaction this week. Have a nice holiday ! commit 9aea470b399d797e88be08985c489855759c6c60 Author: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Fri Dec 21 13:01:55 2012 -0300 [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk Instead of centrally enabling and disabling subdevice master clocks in soc-camera core, let subdevice drivers do that themselves, using the V4L2 clock API and soc-camera convenience wrappers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com (c/c the ones that acked with this broken changeset) We need to fix it ASAP or to revert the ov2640 changes, as some em28xx cameras are currently broken on 3.10. I'll also reject other ports to the async API if the drivers are used outside an embedded driver, as no PC driver currently defines any clock source. The same applies to regulators. Guennadi, Next time, please check if the i2c drivers are used outside soc_camera and apply the fixes where needed, as no regressions are allowed. Regards, Mauro FYI: 8 weeks have passed by now and this regression has still not been fixed. Does anybody care about it ? WONTFIX ? You replied to my patch em28xx: balance subdevice power-off calls with a few non-essential IMHO comments but you didn't test it. Non-essential comments ? Maybe you disagree or don't care about them, but that's something different. Firstly, I did say IMHO, didn't I? Secondly, sure, let's have a look at them: I wonder if we should make the (s_power, 1) call part of em28xx_wake_i2c(). Is this an essential comment? Is it essential where to put an operation after a function or after it? It should've been after a function or inside it of course. It's a proposal. em28xx_set_mode() calls em28xx_gpio_set(dev, INPUT(dev-ctl_input)-gpio) and I'm not sure if this could disable subdevice power again... You aren't sure about that. Me neither, so, there's no evidence whatsoever. This is just a guess. And I would consider switching subdevice power in a *_set_mode() function by explicitly toggling a GPIO in presence of proper APIs... not the best design perhaps. I consider this comment non-essential too then. It's a warning/indicator that the whole s_power thing is very dangerous because of the various GPIO-sequences used everywhere in the driver. It is important, sure. But let's try to understand again what my patch is doing. It is adding 2 calls to .s_power(1) without changing anything else, or at least that's what it should be doing. is *_set_mode() was problematic, I'm not changing anything about it. In a different thread I offered to review .s_power() methods of all em28xx subdevice drivers to see, what harm turning power on could do. A first quick glance didn't reveal any dangerous locations. Nothing else matters. Again: the only change should be adding .s_power(1) and it's only those functions that we should care about. And it substantiates what Mauro tries to explain you. I think I mostly understand what Mauro is explaining to me. Hmm... your patch didn't change this, but: Why do we call these functions only in case of V4L2_BUF_TYPE_VIDEO_CAPTURE ? Isn't it needed for VBI capturing, too ? em28xx_wake_i2c() is probably also needed for radio mode... Right, my patch doesn't change this, so, this is unrelated. Ok, I have to admit that I wasn't clear enough in this case: IMHO these are bugs that should be fixed, but I'm not 100% sure. In that case, there is no need to split the if-caluse containing the V4L2_BUF_TYPE_VIDEO_CAPTURE check, just remove this check while you're at it. No! It shouldn't be changed while at it. If it should be changed, it _certainly_ has to be a separate patch! And it is unrelated. Have I missed anything? It
[PATCH] em28xx: fix error path in em28xx_start_analog_streaming()
Increase the streaming_users count only if streaming start succeeds.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
Cc: sta...@kernel.org
---
drivers/media/usb/em28xx/em28xx-video.c |7 ---
1 Datei geändert, 4 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-video.c
b/drivers/media/usb/em28xx/em28xx-video.c
index 9d10334..fc5d60e 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -638,7 +638,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq,
unsigned int count)
if (rc)
return rc;
- if (dev-streaming_users++ == 0) {
+ if (dev-streaming_users == 0) {
/* First active streaming user, so allocate all the URBs */
/* Allocate the USB bandwidth */
@@ -657,7 +657,7 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq,
unsigned int count)
dev-packet_multiplier,
em28xx_urb_data_copy);
if (rc 0)
- goto fail;
+ return rc;
/*
* djh: it's not clear whether this code is still needed. I'm
@@ -675,7 +675,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq,
unsigned int count)
v4l2_device_call_all(dev-v4l2_dev, 0, tuner, s_frequency, f);
}
-fail:
+ dev-streaming_users++;
+
return rc;
}
--
1.7.10.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
Re: em28xx + ov2640 and v4l2-clk
Am 10.10.2013 19:50, schrieb Guennadi Liakhovetski: On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 10.10.2013 15:50, schrieb Guennadi Liakhovetski: Hi Frank, On Thu, 10 Oct 2013, Frank Schäfer wrote: Am 08.10.2013 18:38, schrieb Guennadi Liakhovetski: Hi Frank, On Tue, 8 Oct 2013, Frank SchÀfer wrote: Am 18.08.2013 17:20, schrieb Mauro Carvalho Chehab: Em Sun, 18 Aug 2013 13:40:25 +0200 Frank SchÀfer fschaefer@googlemail.com escreveu: Am 17.08.2013 12:51, schrieb Guennadi Liakhovetski: Hi Frank, As I mentioned on the list, I'm currently on a holiday, so, replying briefly. Sorry, I missed that (can't read all mails on the list). Since em28xx is a USB device, I conclude, that it's supplying clock to its components including the ov2640 sensor. So, yes, I think the driver should export a V4L2 clock. Ok, so it's mandatory on purpose ? I'll take a deeper into the v4l2-clk code and the em28xx/ov2640/soc-camera interaction this week. Have a nice holiday ! commit 9aea470b399d797e88be08985c489855759c6c60 Author: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Fri Dec 21 13:01:55 2012 -0300 [media] soc-camera: switch I2C subdevice drivers to use v4l2-clk Instead of centrally enabling and disabling subdevice master clocks in soc-camera core, let subdevice drivers do that themselves, using the V4L2 clock API and soc-camera convenience wrappers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com (c/c the ones that acked with this broken changeset) We need to fix it ASAP or to revert the ov2640 changes, as some em28xx cameras are currently broken on 3.10. I'll also reject other ports to the async API if the drivers are used outside an embedded driver, as no PC driver currently defines any clock source. The same applies to regulators. Guennadi, Next time, please check if the i2c drivers are used outside soc_camera and apply the fixes where needed, as no regressions are allowed. Regards, Mauro FYI: 8 weeks have passed by now and this regression has still not been fixed. Does anybody care about it ? WONTFIX ? You replied to my patch em28xx: balance subdevice power-off calls with a few non-essential IMHO comments but you didn't test it. Non-essential comments ? Maybe you disagree or don't care about them, but that's something different. Firstly, I did say IMHO, didn't I? Secondly, sure, let's have a look at them: I wonder if we should make the (s_power, 1) call part of em28xx_wake_i2c(). Is this an essential comment? Is it essential where to put an operation after a function or after it? It should've been after a function or inside it of course. It's a proposal. em28xx_set_mode() calls em28xx_gpio_set(dev, INPUT(dev-ctl_input)-gpio) and I'm not sure if this could disable subdevice power again... You aren't sure about that. Me neither, so, there's no evidence whatsoever. This is just a guess. And I would consider switching subdevice power in a *_set_mode() function by explicitly toggling a GPIO in presence of proper APIs... not the best design perhaps. I consider this comment non-essential too then. It's a warning/indicator that the whole s_power thing is very dangerous because of the various GPIO-sequences used everywhere in the driver. It is important, sure. But let's try to understand again what my patch is doing. It is adding 2 calls to .s_power(1) without changing anything else, or at least that's what it should be doing. is *_set_mode() was problematic, I'm not changing anything about it. In a different thread I offered to review .s_power() methods of all em28xx subdevice drivers to see, what harm turning power on could do. A first quick glance didn't reveal any dangerous locations. Nothing else matters. Again: the only change should be adding .s_power(1) and it's only those functions that we should care about. And it substantiates what Mauro tries to explain you. I think I mostly understand what Mauro is explaining to me. Hmm... your patch didn't change this, but: Why do we call these functions only in case of V4L2_BUF_TYPE_VIDEO_CAPTURE ? Isn't it needed for VBI capturing, too ? em28xx_wake_i2c() is probably also needed for radio mode... Right, my patch doesn't change this, so, this is unrelated. Ok, I have to admit that I wasn't clear enough in this case: IMHO these are bugs that should be fixed, but I'm not 100% sure. In that case, there is no need to split the if-caluse containing the V4L2_BUF_TYPE_VIDEO_CAPTURE check, just remove this check while you're at it. No! It shouldn't be changed while at it. If it should be changed, it _certainly_ has to be a separate patch! And it is unrelated. If you want the fix as a separate patch, then it would make sense
Re: em28xx + ov2640 and v4l2-clk
Am 10.10.2013 20:38, schrieb Frank Schäfer: [...] Hmm... your patch didn't change this, but: Why do we call these functions only in case of V4L2_BUF_TYPE_VIDEO_CAPTURE ? Isn't it needed for VBI capturing, too ? em28xx_wake_i2c() is probably also needed for radio mode... Right, my patch doesn't change this, so, this is unrelated. Ok, I have to admit that I wasn't clear enough in this case: IMHO these are bugs that should be fixed, but I'm not 100% sure. In that case, there is no need to split the if-caluse containing the V4L2_BUF_TYPE_VIDEO_CAPTURE check, just remove this check while you're at it. No! It shouldn't be changed while at it. If it should be changed, it _certainly_ has to be a separate patch! And it is unrelated. If you want the fix as a separate patch, then it would make sense to do this before the s_power change. IMHO it doesn't make sense to complicate the code just to keep a bug which can be fixed easily. Looking into the code again, I think there are even more things which need to be fixed. :( Will try to send a patch tomorrow. Regards, Frank -- 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 PATCH] em28xx: fix device initialization in em28xx_v4l2_open() for radio and VBI mode
- bail out on unsupported VFL_TYPE
- em28xx_set_mode() needs to be called for VBI and radio mode, too
- em28xx_wake_i2c() needs to be called for VBI and radio mode, too
- em28xx_resolution_set() also needs to be called for VBI
Compilation tested only and should be reviewed thoroughly !
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/usb/em28xx/em28xx-video.c | 17 +++--
1 Datei geändert, 11 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-video.c
b/drivers/media/usb/em28xx/em28xx-video.c
index fc5d60e..962f4b2 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1570,13 +1570,16 @@ static int em28xx_v4l2_open(struct file *filp)
case VFL_TYPE_VBI:
fh_type = V4L2_BUF_TYPE_VBI_CAPTURE;
break;
+ case VFL_TYPE_RADIO:
+ break;
+ default:
+ return -EINVAL;
}
em28xx_videodbg(open dev=%s type=%s users=%d\n,
video_device_node_name(vdev), v4l2_type_names[fh_type],
dev-users);
-
if (mutex_lock_interruptible(dev-lock))
return -ERESTARTSYS;
fh = kzalloc(sizeof(struct em28xx_fh), GFP_KERNEL);
@@ -1590,15 +1593,17 @@ static int em28xx_v4l2_open(struct file *filp)
fh-type = fh_type;
filp-private_data = fh;
- if (fh-type == V4L2_BUF_TYPE_VIDEO_CAPTURE dev-users == 0) {
+ if (dev-users == 0) {
em28xx_set_mode(dev, EM28XX_ANALOG_MODE);
- em28xx_resolution_set(dev);
- /* Needed, since GPIO might have disabled power of
- some i2c device
+ if (vdev-vfl_type != VFL_TYPE_RADIO)
+ em28xx_resolution_set(dev);
+
+ /*
+* Needed, since GPIO might have disabled power of
+* some i2c devices
*/
em28xx_wake_i2c(dev);
-
}
if (vdev-vfl_type == VFL_TYPE_RADIO) {
--
1.7.10.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
Re: ivtv 1.4.2/1.4.3 broken in recent kernels?
On 10 October 2013 03:20, Andy Walls awa...@md.metrocast.net wrote: On Wed, 2013-09-18 at 02:19 +0530, Rajil Saraswat wrote: Hi, I have a couple of PVR-500's which have additional tuners connected to them (using daughter cards). The PVR-500's don't have daughter cards with additional tuners AFAIK. There is this however: http://www.hauppauge.com/site/webstore2/webstore_avcable-pci.asp Make sure you have any jumpers set properly and the cable connectors seated properly. Also make sure the cable is routed aways from any electrically noisy cards and high speed data busses: disk controller cards, graphics cards, etc. The audio is not usable on either 1.4.2 or 1.4.3 ivtv drivers. The issue is described at http://ivtvdriver.org/pipermail/ivtv-users/2013-September/010462.html With your previous working kernel and with the non-working kernel, what is the output of $ v4l2-ctl -d /dev/videoX --log-status after you have set up the inputs properly and have a known good signal going into the input in question? I'm speculating this is a problem with the cx25840 driver or the wm8775 driver, since they change more often than the ivtv driver. Yes, thats right it is a set of extra inputs and not a separate tuner card. I played a video stream fro both kernels. Here are the logs Working kernel 2.6.35 v4l2-ctl -d /dev/video1 --log-status Status Log: [50885.487963] ivtv1: = START STATUS CARD #1 = [50885.487967] ivtv1: Version: 1.4.1 Card: WinTV PVR 500 (unit #2) [50885.541679] tveeprom 2-0050: Hauppauge model 23559, rev D591, serial# 8228753 [50885.541681] tveeprom 2-0050: tuner model is Philips FQ1216AME MK4 (idx 91, type 56) [50885.541684] tveeprom 2-0050: TV standards PAL(B/G) PAL(I) SECAM(L/L') PAL(D/D1/K) (eeprom 0x74) [50885.541686] tveeprom 2-0050: second tuner model is Philips TEA5768HL FM Radio (idx 101, type 62) [50885.541688] tveeprom 2-0050: audio processor is CX25843 (idx 37) [50885.541690] tveeprom 2-0050: decoder processor is CX25843 (idx 30) [50885.541692] tveeprom 2-0050: has radio [50885.541698] ivtv1: GPIO status: DIR=0xdf01 OUT=0x26f3 IN=0x17e7 [50885.545429] cx25840 2-0044: Video signal: present [50885.545431] cx25840 2-0044: Detected format: PAL-BDGHI [50885.545433] cx25840 2-0044: Specified standard:PAL-BDGHI [50885.545435] cx25840 2-0044: Specified video input: Composite 4 [50885.545437] cx25840 2-0044: Specified audioclock freq: 48000 Hz [50885.553051] cx25840 2-0044: Detected audio mode: forced mode [50885.553053] cx25840 2-0044: Detected audio standard: no detected audio standard [50885.553055] cx25840 2-0044: Audio muted: no [50885.553057] cx25840 2-0044: Audio microcontroller: stopped [50885.553059] cx25840 2-0044: Configured audio standard: automatic detection [50885.553061] cx25840 2-0044: Configured audio system: automatic standard and mode detection [50885.553063] cx25840 2-0044: Specified audio input: External [50885.553064] cx25840 2-0044: Preferred audio mode: stereo [50885.553066] cx25840 2-0044: Selected 65 MHz format:system DK [50885.553068] cx25840 2-0044: Selected 45 MHz format:chroma [50885.553070] tda9887 2-0043: Data bytes: b=0x94 c=0x6e e=0x49 [50885.553073] tuner 2-0061: Tuner mode: analog TV [50885.553075] tuner 2-0061: Frequency: 400.00 MHz [50885.553077] tuner 2-0061: Standard:0x0007 [50885.553079] wm8775 2-001b: Input: 4 [50885.553081] ivtv1: Video Input: Composite 2 [50885.553082] ivtv1: Audio Input: Line In 2 [50885.553084] ivtv1: Tuner: TV [50885.553086] ivtv1: Stream: MPEG-2 Program Stream [50885.553088] ivtv1: VBI Format: No VBI [50885.553089] ivtv1: Video: 720x576, 25 fps [50885.553092] ivtv1: Video: MPEG-2, 4x3, Variable Bitrate, 600, Peak 800 [50885.553094] ivtv1: Video: GOP Size 12, 2 B-Frames, GOP Closure [50885.553097] ivtv1: Audio: 48 kHz, MPEG-1/2 Layer II, 224 kbps, Stereo, No Emphasis, No CRC [50885.553100] ivtv1: Spatial Filter: Manual, Luma 1D Horizontal, Chroma 1D Horizontal, 0 [50885.553103] ivtv1: Temporal Filter: Manual, 8 [50885.553105] ivtv1: Median Filter: Off, Luma [0, 255], Chroma [0, 255] [50885.553106] ivtv1: Status flags:0x0020 [50885.553109] ivtv1: Stream encoder MPG: status 0x0118, 15% of 16384 KiB (512 buffers) in use [50885.553112] ivtv1: Stream encoder YUV: status 0x, 0% of 20480 KiB (640 buffers) in use [50885.553114] ivtv1: Stream encoder VBI: status 0x, 0% of 1049 KiB (41 buffers) in use [50885.553117] ivtv1: Stream encoder PCM: status 0x, 0% of 643 KiB (143 buffers) in use [50885.553119] ivtv1: Read MPG/VBI: 56262656/0 bytes [50885.553121] ivtv1: == END STATUS CARD #1 == For the non-working kernel 2.6.37 [ 212.730996] ivtv1: = START
Re: [media-workshop] V2: Agenda for the Edinburgh mini-summit
On Mon, Oct 7, 2013 at 3:24 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sakari, On Tuesday 08 October 2013 00:06:23 Sakari Ailus wrote: On Tue, Sep 24, 2013 at 11:20:53AM +0200, Thierry Reding wrote: On Mon, Sep 23, 2013 at 10:27:06PM +0200, Sylwester Nawrocki wrote: On 09/23/2013 06:37 PM, Oliver Schinagl wrote: On 09/23/13 16:45, Sylwester Nawrocki wrote: Hi, I would like to have a short discussion on LED flash devices support in the kernel. Currently there are two APIs: the V4L2 and LED class API exposed by the kernel, which I believe is not good from user space POV. Generic applications will need to implement both APIs. I think we should decide whether to extend the led class API to add support for more advanced LED controllers there or continue to use the both APIs with overlapping functionality. There has been some discussion about this on the ML, but without any consensus reached [1]. What about the linux-pwm framework and its support for the backlight via dts? Or am I talking way to uninformed here. Copying backlight to flashlight with some minor modification sounds sensible in a way... I'd assume we don't need yet another user interface for the LEDs ;) AFAICS the PWM subsystem exposes pretty much raw interface in sysfs. The PWM LED controllers are already handled in the leds-class API, there is the leds_pwm driver (drivers/leds/leds-pwm.c). I'm adding linux-pwm and linux-leds maintainers at Cc so someone may correct me if I got anything wrong. The PWM subsystem is most definitely not a good fit for this. The only thing it provides is a way for other drivers to access a PWM device and use it for some specific purpose (pwm-backlight, leds-pwm). The sysfs support is a convenience for people that needs to use a PWM in a way for which no driver framework exists, or for which it doesn't make sense to write a driver. Or for testing. Presumably, what we need is a few enhancements to support in a standard way devices like MAX77693, LM3560 or MAX8997. There is already a led class driver for the MAX8997 LED controller (drivers/leds/leds- max8997.c), but it uses some device-specific sysfs attributes. Thus similar devices are currently being handled by different subsystems. The split between the V4L2 Flash and the leds class API WRT to Flash LED controller drivers is included in RFC [1], it seems still up to date. [1] http://www.spinics.net/lists/linux-leds/msg00899.html Perhaps it would make sense for V4L2 to be able to use a LED as exposed by the LED subsystem and wrap it so that it can be integrated with V4L2? If functionality is missing from the LED subsystem I suppose that could be added. The V4L2 flash API supports also xenon flashes, not only LED ones. That said, I agree there's a common subset of functionality most LED flash controllers implement. If I understand correctly, the V4L2 subsystem uses LEDs as flashes for camera devices. I can easily imagine that there are devices out there which provide functionality beyond what a regular LED will provide. So perhaps for things such as mobile phones, which typically use a plain LED to illuminate the surroundings, an LED wrapped into something that emulates the flash functionality could work. But I doubt that the LED subsystem is a good fit for anything beyond that. I originally thought one way to do this could be to make it as easy as possible to support both APIs in driver which some aregued, to which I agree, is rather poor desing. Does the LED API have a user space interface library like libv4l2? If yes, one option oculd be to implement the wrapper between the V4L2 and LED APIs there so that the applications using the LED API could also access those devices that implement the V4L2 flash API. Torch mode functionality is common between the two right now AFAIU, The V4L2 flash API also provides a way to strobe the flash using an external trigger which typically connected to the sensor (and the user can choose between that and software strobe). I guess that and Xenon flashes aren't currently covered by the LED API. The issue is that we have a LED API targetted at controlling LEDs, a V4L2 flash API targetted at controlling flashes, and hardware devices somewhere in the middle that can be used to provide LED or flash function. Merging the two APIs on the kernel side, with a compatibility layer for both kernel space and user space APIs, might be an idea worth investigating. I'm so sorry for jumping in the discussion so late. Some how the emails from linux-media was archived in my Gmail and I haven't checkout this for several weeks. I agree right now LED API doesn't quite fit for the usage of V4L2 Flash API. But I'd also like to see a unified API. Currently, LED API are exported to user space as sysfs interface, while V4L2 Flash APIs are like IOCTL and user space library. We
Re: [RFC 0/2] V4L2 API for exposing flash subdevs as LED class device
On Tue, May 21, 2013 at 3:54 AM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Andrzej, On Tue, May 21, 2013 at 10:34:53AM +0200, Andrzej Hajda wrote: On 12.05.2013 23:12, Sakari Ailus wrote: On Wed, May 08, 2013 at 09:32:17AM +0200, Andrzej Hajda wrote: On 07.05.2013 17:07, Laurent Pinchart wrote: On Tuesday 07 May 2013 02:11:27 Kim, Milo wrote: On Monday, May 06, 2013 6:34 PM Andrzej Hajda wrote: This RFC proposes generic API for exposing flash subdevices via LED framework. Rationale Currently there are two frameworks which are used for exposing LED flash to user space: - V4L2 flash controls, - LED framework(with custom sysfs attributes). The list below shows flash drivers in mainline kernel with initial commit date and typical chip application (according to producer): LED API: lm3642: 2012-09-12, Cameras lm355x: 2012-09-05, Cameras max8997: 2011-12-14, Cameras (?) lp3944: 2009-06-19, Cameras, Lights, Indicators, Toys pca955x: 2008-07-16, Cameras, Indicators (?) V4L2 API: as3645a: 2011-05-05, Cameras adp1653: 2011-05-05, Cameras V4L2 provides richest functionality, but there is often demand from application developers to provide already established LED API. We would like to have an unified user interface for flash devices. Some of devices already have the LED API driver exposing limited set of a Flash IC functionality. In order to support all required features the LED API would have to be extended or the V4L2 API would need to be used. However when switching from a LED to a V4L2 Flash driver existing LED API interface would need to be retained. Proposed solution This patch adds V4L2 helper functions to register existing V4L2 flash subdev as LED class device. After registration via v4l2_leddev_register appropriate entry in /sys/class/leds/ is created. During registration all V4L2 flash controls are enumerated and corresponding attributes are added. I have attached also patch with new max77693-led driver using v4l2_leddev. This patch requires presence of the patch max77693: added device tree support: https://patchwork.kernel.org/patch/2414351/ . Additional features - simple API to access all V4L2 flash controls via sysfs, - V4L2 subdevice should not be registered by V4L2 device to use it, - LED triggers API can be used to control the device, - LED device is optional - it will be created only if V4L2_LEDDEV configuration option is enabled and the subdev driver calls v4l2_leddev_register. Doubts This RFC is a result of a uncertainty which API developers should expose by their flash drivers. It is a try to gluing together both APIs. I am not sure if it is the best solution, but I hope there will be some discussion and hopefully some decisions will be taken which way we should follow. The LED subsystem provides similar APIs for the Camera driver. With LED trigger event, flash and torch are enabled/disabled. I'm not sure this is applicable for you. Could you take a look at LED camera trigger feature? For the camera LED trigger, https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/ ?h=f or-nextid=48a1d032c954b9b06c3adbf35ef4735dd70ab757 Example of camera flash driver, https://git.kernel.org/cgit/linux/kernel/git/cooloney/linux-leds.git/commit/ ?h=f or-nextid=313bf0b1a0eaeaac17ea8c4b748f16e28fce8b7a I think we should decide on one API. Implementing two APIs for a single device is usually messy, and will result in different feature sets (and different bugs) being implemented through each API, depending on the driver. Interactions between the APIs are also a pain point on the kernel side to properly synchronize calls. I don't like having two APIs either. Especially we shouldn't have multiple drivers implementing different APIs for the same device. That said, I wonder if it's possible to support camera-related use cases using the LED API: it's originally designed for quite different devices. Even if you could handle flash strobing using the LED API, the functionality provided by the Media controller and subdev APIs will always be missing: device enumeration and association with the right camera. Is there a generic way to associate flash and camera subdevs in current V4L2 API? The only ways I see now are: - both belongs to the same media controller, but this is not enough if there is more than one camera subdev in that controller, Yes, there is. That's the group_id field in struct media_entity_desc. The lens subdev is associated to the rest of the devices the same way. - using media links/pads - at first sight it seems to be overkill/abuse... No. Links describe the flow of data, not relations between entities. ... The LED API is too limited for torch and flash usage, but I'm definitely open to moving flash devices to the LED API is we
Re: [media-workshop] Kernel Summit Media Mini-summit attendees on Oct 23 in Edinburgh
I'm sorry everyone, I unfortunately won't be able to make it after all due to schedule conflicts. Best regards, Pawel On Fri, Sep 20, 2013 at 7:50 PM, Mauro Carvalho Chehab mche...@infradead.org wrote: Em Fri, 20 Sep 2013 09:59:28 +0200 Hans de Goede hdego...@redhat.com escreveu: Hi, Sorry for replying in the midst of the thread I deleted the beginning before realizing I should respond. On 09/17/13 19:08, Mauro Carvalho Chehab wrote: Hi, I'm trying to consolidate the list of interested people on participating at this year's the media mini-summit. From what I got from the discussions, we have, so far: Benjamin Gaignard benjamin.gaign...@linaro.org Guennadi Liakhovetski g.liakhovet...@gmx.de Hans Verkuil hverk...@xs4all.nl Hugues FRUCHET hugues.fruc...@st.com Laurent Pinchart laurent.pinch...@ideasonboard.com Mauro Carvalho Chehab m.che...@samsung.com Michael Krufky mkru...@kernellabs.com Oliver Schinagl oliver+l...@schinagl.nl Pawel Osciak posc...@chromium.org Peter Senna Tschudin peter.se...@gmail.com Ricardo Ribalda Delgado ricardo.riba...@gmail.com Sakari Ailus sakari.ai...@iki.fi Please let me know if I'm missing someone, or if one of the above won't be able to go to the meeting, as my plan is to send the invitations tomorrow. I'll be in Edinburgh for kvm-forum and I would like to attend the meeting, at least the parts about the userspace libraries. Ok. I'll add your name to the list of atendees. Regards, Mauro Regards, Hans (de Goede) ___ media-workshop mailing list media-works...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/media-workshop Cheers, Mauro -- 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
cron job: media_tree daily build: WARNINGS
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: Fri Oct 11 04:00:36 CEST 2013 git branch: test git hash: d10e8280c4c2513d3e7350c27d8e6f0fa03a5f71 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.11-4.slh.2-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-rc1-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API 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
