Re: [PATCH v2 2/2] libv4lconvert: Support for RGB32 and BGR32 format
Hello Gregor Thanks for your comments. I have replied inline. On Sat, Aug 3, 2013 at 6:42 PM, Gregor Jasny gja...@googlemail.com wrote: On 8/3/13 12:42 AM, Ricardo Ribalda Delgado wrote: + case V4L2_PIX_FMT_RGB32: + switch (dest_pix_fmt) { + case V4L2_PIX_FMT_RGB24: + v4lconvert_rgb32_to_rgb24(src, dest, width, height, 0); + break; + case V4L2_PIX_FMT_BGR24: + v4lconvert_rgb32_to_rgb24(src, dest, width, height, 1); + break; + case V4L2_PIX_FMT_YUV420: + v4lconvert_rgb24_to_yuv420(src, dest, fmt, 0, 0, 4); + break; + case V4L2_PIX_FMT_YVU420: + v4lconvert_rgb24_to_yuv420(src, dest, fmt, 0, 1, 4); + break; + } + if (src_size (width * height * 4)) { + V4LCONVERT_ERR(short rgb32 data frame\n); + errno = EPIPE; + result = -1; + } + break; I have not looked at the whole function but shouldn't this sanity check happen before the actual work? Yes, but it is how it is done in the whole library with all the formats. Please grep for short on libv4lconvert.c Also aren't you applying the condition here also for rgb24_to_xxx which should have only three bpp? I have modified the function rgb24_to_yuv420 to support other bytes per pixel. + case V4L2_PIX_FMT_BGR32: + switch (dest_pix_fmt) { + case V4L2_PIX_FMT_RGB24: + v4lconvert_rgb32_to_rgb24(src, dest, width, height, 1); + break; + case V4L2_PIX_FMT_BGR24: + v4lconvert_rgb32_to_rgb24(src, dest, width, height, 0); + break; + case V4L2_PIX_FMT_YUV420: + v4lconvert_rgb24_to_yuv420(src, dest, fmt, 1, 0, 4); + break; + case V4L2_PIX_FMT_YVU420: + v4lconvert_rgb24_to_yuv420(src, dest, fmt, 1, 1, 4); + break; + } + if (src_size (width * height * 4)) { + V4LCONVERT_ERR(short bgr32 data frame\n); + errno = EPIPE; + result = -1; + } + break; Same here. And also in the other patch. Thanks again -- Ricardo Ribalda -- 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
mceusb Fintek ir transmitter only works when X is not running
Hi, I have a HP MCE ir transreceiver which is recognised as Fintek device. The receiver works fine, however the transmitter only works when there is no X session running. When X is stopped and the following command is issued from the virtual console (tty1), then the transmitter works: irsend SEND_ONCE mceusb KEY_1 However, as soon as X is started even though irsend goes through, the transmitter led's dont go through. Any idea why this may be happening? These are the system details: #uname -a Linux localhost 3.10.4-gentoo #7 SMP Sun Aug 4 12:07:08 BST 2013 x86_64 Intel(R) Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux # lsusb Bus 002 Device 008: ID 1934:5168 Feature Integration Technology Inc. (Fintek) F71610A or F71612A Consumer Infrared Receiver/Transceiver #cat /etc/conf.d/lircd LIRCD_OPTS=-d /dev/lirc0 Thanks Rajil -- 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 v3 10/13] [media] exynos5-fimc-is: Add the hardware interface module
Hi Arun, On 08/02/2013 05:02 PM, Arun Kumar K wrote: The hardware interface module finally sends the commands to the FIMC-IS firmware and runs the interrupt handler for getting the responses. Signed-off-by: Arun Kumar Karun...@samsung.com Signed-off-by: Kilyeon Imkilyeon...@samsung.com --- .../media/platform/exynos5-is/fimc-is-interface.c | 861 .../media/platform/exynos5-is/fimc-is-interface.h | 128 +++ 2 files changed, 989 insertions(+) create mode 100644 drivers/media/platform/exynos5-is/fimc-is-interface.c create mode 100644 drivers/media/platform/exynos5-is/fimc-is-interface.h diff --git a/drivers/media/platform/exynos5-is/fimc-is-interface.c b/drivers/media/platform/exynos5-is/fimc-is-interface.c new file mode 100644 index 000..12073be --- /dev/null +++ b/drivers/media/platform/exynos5-is/fimc-is-interface.c @@ -0,0 +1,861 @@ +/* + * Samsung EXYNOS5 FIMC-IS (Imaging Subsystem) driver +* + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Kil-yeon Limkilyeon...@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. + */ + +#includelinux/debugfs.h +#includelinux/seq_file.h +#include fimc-is.h +#include fimc-is-cmd.h +#include fimc-is-regs.h + +#define init_request_barrier(itf) mutex_init(itf-request_barrier) +#define enter_request_barrier(itf) mutex_lock(itf-request_barrier) +#define exit_request_barrier(itf) mutex_unlock(itf-request_barrier) + +static inline void itf_get_cmd(struct fimc_is_interface *itf, + struct fimc_is_msg *msg, unsigned int index) +{ + struct is_common_reg __iomem *com_regs = itf-com_regs; + + memset(msg, 0, sizeof(*msg)); + + switch (index) { + case INTR_GENERAL: + msg-command = com_regs-ihcmd; + msg-instance = com_regs-ihc_sensorid; nit: How about doing something like: memcpy(msg-param, com_regs-ihc_param, 4 * sizeof(mgs-iparam[0]); for such repeated assignments ? + msg-param[0] = com_regs-ihc_param[0]; + msg-param[1] = com_regs-ihc_param[1]; + msg-param[2] = com_regs-ihc_param[2]; + msg-param[3] = com_regs-ihc_param[3]; + break; + case INTR_SCC_FDONE: + msg-command = IHC_FRAME_DONE; + msg-instance = com_regs-scc_sensor_id; + msg-param[0] = com_regs-scc_param[0]; + msg-param[1] = com_regs-scc_param[1]; + msg-param[2] = com_regs-scc_param[2]; + break; + case INTR_SCP_FDONE: + msg-command = IHC_FRAME_DONE; + msg-instance = com_regs-scp_sensor_id; + msg-param[0] = com_regs-scp_param[0]; + msg-param[1] = com_regs-scp_param[1]; + msg-param[2] = com_regs-scp_param[2]; + break; + case INTR_META_DONE: + msg-command = IHC_FRAME_DONE; + msg-instance = com_regs-meta_sensor_id; + msg-param[0] = com_regs-meta_param1; + break; + case INTR_SHOT_DONE: + msg-command = IHC_FRAME_DONE; + msg-instance = com_regs-shot_sensor_id; + msg-param[0] = com_regs-shot_param[0]; + msg-param[1] = com_regs-shot_param[1]; + break; + default: + pr_err(unknown command getting\n); Would be nice to have at least function name in that log message. + break; + } +} + +static inline unsigned int itf_get_intr(struct fimc_is_interface *itf) +{ + unsigned int status; + struct is_common_reg __iomem *com_regs = itf-com_regs; + + status = readl(itf-regs + INTMSR1) | com_regs-ihcmd_iflag | + com_regs-scc_iflag | + com_regs-scp_iflag | + com_regs-meta_iflag | + com_regs-shot_iflag; + + return status; +} + +static void itf_set_state(struct fimc_is_interface *itf, + unsigned long state) +{ + unsigned long flags; + spin_lock_irqsave(itf-slock_state, flags); + __set_bit(state,itf-state); + spin_unlock_irqrestore(itf-slock_state, flags); +} + +static void itf_clr_state(struct fimc_is_interface *itf, + unsigned long state) +{ + unsigned long flags; + spin_lock_irqsave(itf-slock_state, flags); + __clear_bit(state,itf-state); + spin_unlock_irqrestore(itf-slock_state, flags); +} + +static int itf_get_state(struct fimc_is_interface *itf, + unsigned long state) +{ + int ret = 0; + unsigned long flags; + + spin_lock_irqsave(itf-slock_state, flags); + ret = test_bit(state,itf-state); Shouldn't it be __test_bit() ? + spin_unlock_irqrestore(itf-slock_state, flags); + return ret; +} + +static void
Re: How to express planar formats with mediabus format code?
Hi Su Jiaquan On Sun, 4 Aug 2013, Su Jiaquan wrote: Hi, I know the title looks crazy, but here is our problem: In our SoC based ISP, the hardware can be divide to several blocks. Some blocks can do color space conversion(raw to YUV interleave/planar), others can do the pixel re-order(interleave/planar/semi-planar conversion, UV planar switch). We use one subdev to describe each of them, then came the problem: How can we express the planar formats with mediabus format code? Could you please explain more exactly what you mean? How are those your blocks connected? How do they exchange data? If they exchange data over a serial bus, then I don't think planar formats make sense, right? Or do your blocks really output planes one after another, reordering data internally? That would be odd... If OTOH your blocks output data to RAM, and the next block takes data from there, then you use V4L2_PIX_FMT_* formats to describe them and any further processing block should be a mem2mem device. Wouldn't this work? Thanks Guennadi I understand at beginning, media-bus was designed to describe the data link between camera sensor and camera controller, where sensor is described in subdev. So interleave formats looks good enough at that time. But now as Media-controller is introduced, subdev can describe a much wider range of hardware, which is not limited to camera sensor. So now planar formats are possible to be passed between subdevs. I think the problem we meet can be very common for SoC based ISP solutions, what do you think about it? there are many possible solution for it: 1 change the definition of v4l2_subdev_format::format, use v4l2_format; 2 extend the mediabus format code, add planar format code; 3 use a extra bit to tell the meaning of v4l2_mbus_framefmt::code, is it in mediabus-format or in fourcc Do you have any suggestions? Thanks a lot! -- 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 --- 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 4/5] [media] winbond: wire up rc feedback led
On Wed, Jul 31, 2013 at 12:00:03AM +0100, Sean Young wrote: Note that with the rc-feedback trigger, the cir-rx trigger is now redundant. The cir-tx trigger is not used by default; if this functionality is desired then it should exist in rc-core, not in a driver. Also make sure that the led is suspended on suspend. Signed-off-by: Sean Young s...@mess.org Signed-off-by: David Härdeman da...@hardeman.nu --- drivers/media/rc/Kconfig | 1 - drivers/media/rc/winbond-cir.c | 38 ++ 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig index 5a79c33..7fa6b22 100644 --- a/drivers/media/rc/Kconfig +++ b/drivers/media/rc/Kconfig @@ -248,7 +248,6 @@ config IR_WINBOND_CIR depends on RC_CORE select NEW_LEDS select LEDS_CLASS - select LEDS_TRIGGERS select BITREVERSE ---help--- Say Y here if you want to use the IR remote functionality found diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c index 87af2d3..98bd496 100644 --- a/drivers/media/rc/winbond-cir.c +++ b/drivers/media/rc/winbond-cir.c @@ -213,13 +213,11 @@ struct wbcir_data { /* RX state */ enum wbcir_rxstate rxstate; - struct led_trigger *rxtrigger; int carrier_report_enabled; u32 pulse_duration; /* TX state */ enum wbcir_txstate txstate; - struct led_trigger *txtrigger; u32 txlen; u32 txoff; u32 *txbuf; @@ -366,14 +364,11 @@ wbcir_idle_rx(struct rc_dev *dev, bool idle) { struct wbcir_data *data = dev-priv; - if (!idle data-rxstate == WBCIR_RXSTATE_INACTIVE) { + if (!idle data-rxstate == WBCIR_RXSTATE_INACTIVE) data-rxstate = WBCIR_RXSTATE_ACTIVE; - led_trigger_event(data-rxtrigger, LED_FULL); - } if (idle data-rxstate != WBCIR_RXSTATE_INACTIVE) { data-rxstate = WBCIR_RXSTATE_INACTIVE; - led_trigger_event(data-rxtrigger, LED_OFF); if (data-carrier_report_enabled) wbcir_carrier_report(data); @@ -425,7 +420,6 @@ wbcir_irq_tx(struct wbcir_data *data) case WBCIR_TXSTATE_INACTIVE: /* TX FIFO empty */ space = 16; - led_trigger_event(data-txtrigger, LED_FULL); break; case WBCIR_TXSTATE_ACTIVE: /* TX FIFO low (3 bytes or less) */ @@ -464,7 +458,6 @@ wbcir_irq_tx(struct wbcir_data *data) /* Clear TX underrun bit */ outb(WBCIR_TX_UNDERRUN, data-sbase + WBCIR_REG_SP3_ASCR); wbcir_set_irqmask(data, WBCIR_IRQ_RX | WBCIR_IRQ_ERR); - led_trigger_event(data-txtrigger, LED_OFF); kfree(data-txbuf); data-txbuf = NULL; data-txstate = WBCIR_TXSTATE_INACTIVE; @@ -878,15 +871,13 @@ finish: */ wbcir_set_irqmask(data, WBCIR_IRQ_NONE); disable_irq(data-irq); - - /* Disable LED */ - led_trigger_event(data-rxtrigger, LED_OFF); - led_trigger_event(data-txtrigger, LED_OFF); } static int wbcir_suspend(struct pnp_dev *device, pm_message_t state) { + struct wbcir_data *data = pnp_get_drvdata(device); + led_classdev_suspend(data-led); wbcir_shutdown(device); return 0; } @@ -1015,6 +1006,7 @@ wbcir_resume(struct pnp_dev *device) wbcir_init_hw(data); enable_irq(data-irq); + led_classdev_resume(data-led); return 0; } @@ -1058,25 +1050,13 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id) (w: 0x%lX, e: 0x%lX, s: 0x%lX, i: %u)\n, data-wbase, data-ebase, data-sbase, data-irq); - led_trigger_register_simple(cir-tx, data-txtrigger); - if (!data-txtrigger) { - err = -ENOMEM; - goto exit_free_data; - } - - led_trigger_register_simple(cir-rx, data-rxtrigger); - if (!data-rxtrigger) { - err = -ENOMEM; - goto exit_unregister_txtrigger; - } - data-led.name = cir::activity; - data-led.default_trigger = cir-rx; + data-led.default_trigger = rc-feedback; data-led.brightness_set = wbcir_led_brightness_set; data-led.brightness_get = wbcir_led_brightness_get; err = led_classdev_register(device-dev, data-led); if (err) - goto exit_unregister_rxtrigger; + goto exit_free_data; data-dev = rc_allocate_device(); if (!data-dev) { @@ -1156,10 +1136,6 @@ exit_free_rc: rc_free_device(data-dev); exit_unregister_led: led_classdev_unregister(data-led); -exit_unregister_rxtrigger: - led_trigger_unregister_simple(data-rxtrigger); -exit_unregister_txtrigger: - led_trigger_unregister_simple(data-txtrigger); exit_free_data: kfree(data); pnp_set_drvdata(device, NULL); @@ -1187,8
[PATCH RFC] [media] gspca-ov534: don't call sd_start() from sd_init()
--- Hi Yaroslav, the patch below should fix the Oops caused by sd_start() called too early, but I am not sure about why sd_start() was called from sd_init() for Hercules webcams in the first place, maybe the snippet marked with: /* (from ms-win trace) */ in sd_start() must be moved to sd_init() too. Let me know if the change below alone is enough and the webcam keeps working, a test with suspend and resume would good to have too. Thanks, Antonio drivers/media/usb/gspca/ov534.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/usb/gspca/ov534.c b/drivers/media/usb/gspca/ov534.c index 2e28c81..03a33c4 100644 --- a/drivers/media/usb/gspca/ov534.c +++ b/drivers/media/usb/gspca/ov534.c @@ -1305,8 +1305,7 @@ static int sd_init(struct gspca_dev *gspca_dev) ov534_set_led(gspca_dev, 1); sccb_w_array(gspca_dev, sensor_init[sd-sensor].val, sensor_init[sd-sensor].len); - if (sd-sensor == SENSOR_OV767x) - sd_start(gspca_dev); + sd_stopN(gspca_dev); /* set_frame_rate(gspca_dev); */ -- 1.8.4.rc1 -- 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 v3 00/13] Exynos5 IS driver
Hi Sylwester, On Sun, Aug 4, 2013 at 3:10 AM, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Arun, On 08/02/2013 05:02 PM, Arun Kumar K wrote: The patch series add support for Exynos5 camera subsystem. It re-uses mipi-csis and fimc-lite from exynos4-is and adds a new media device and fimc-is device drivers for exynos5. The media device supports asynchronos subdev registration for the fimc-is sensors and is based on the patch series from Sylwester for exynos4-is [1]. [1]http://www.mail-archive.com/linux-media@vger.kernel.org/msg64653.html Changes from v2 --- - Added exynos5 media device driver from Shaik to this series - Added ISP pipeline support in media device driver - Based on Sylwester's latest exynos4-is development - Asynchronos registration of sensor subdevs - Made independent IS-sensor support - Add s5k4e5 sensor driver - Addressed review comments from Sylwester, Hans, Andrzej, Sachin This is starting to look pretty good to me, I hope we can merge this patch set for v3.12. Let use coming two weeks for one or two review/ corrections round. Sure. I will address the review comments quickly and send v4 version. In the meantime I've done numerous fixes to the patch series [1], especially the clock provider code was pretty buggy on the clean up paths. Let's go through the patches and see what can be improved yet. Ok. Is the updated version available in your git repository git://linuxtv.org/snawrocki/samsung.git? Regards Arun -- 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 2/3] [media] exynos4-is: Annotate unused functions
Hi Sylwester, On 2 August 2013 12:02, Sachin Kamat sachin.ka...@linaro.org wrote: __is_set_init_isp_aa and fimc_is_hw_set_tune currently do not have any callers. However these functions may be used in the future. Hence instead of deleting them, staticize and annotate them with __maybe_unused flag to avoid compiler warnings. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Thanks for applying the other 2 patches in this series. What is your opinion about this one? Does this look good or do you prefer to delete the code altogether? --- drivers/media/platform/exynos4-is/fimc-is-param.c |2 +- drivers/media/platform/exynos4-is/fimc-is-regs.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-param.c b/drivers/media/platform/exynos4-is/fimc-is-param.c index a353be0..9bf3ddd 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-param.c +++ b/drivers/media/platform/exynos4-is/fimc-is-param.c @@ -287,7 +287,7 @@ void __is_set_sensor(struct fimc_is *is, int fps) fimc_is_set_param_bit(is, PARAM_ISP_OTF_INPUT); } -void __is_set_init_isp_aa(struct fimc_is *is) +static void __maybe_unused __is_set_init_isp_aa(struct fimc_is *is) { struct isp_param *isp; diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c b/drivers/media/platform/exynos4-is/fimc-is-regs.c index 63c68ec..cf2e13a 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-regs.c +++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c @@ -96,7 +96,7 @@ int fimc_is_hw_set_param(struct fimc_is *is) return 0; } -int fimc_is_hw_set_tune(struct fimc_is *is) +static int __maybe_unused fimc_is_hw_set_tune(struct fimc_is *is) { fimc_is_hw_wait_intmsr0_intmsd0(is); -- 1.7.9.5 -- With warm regards, Sachin -- 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 v3 01/13] [media] exynos5-is: Adding media device driver for exynos5
On 2 August 2013 20:32, Arun Kumar K arun...@samsung.com wrote: From: Shaik Ameer Basha shaik.am...@samsung.com This patch adds support for media device for EXYNOS5 SoCs. The current media device supports the following ips to connect through the media controller framework. * MIPI-CSIS Support interconnection(subdev interface) between devices * FIMC-LITE Support capture interface from device(Sensor, MIPI-CSIS) to memory Support interconnection(subdev interface) between devices * FIMC-IS Camera post-processing IP having multiple sub-nodes. G-Scaler will be added later to the current media device. The media device creates two kinds of pipelines for connecting the above mentioned IPs. The pipeline0 is uses Sensor, MIPI-CSIS and FIMC-LITE which captures image data and dumps to memory. Pipeline1 uses FIMC-IS components for doing post-processing operations on the captured image and give scaled YUV output. Pipeline0 ++ +---+ +---+ ++ | Sensor | -- | MIPI-CSIS | -- | FIMC-LITE | -- | Memory | ++ +---+ +---+ ++ Pipeline1 ++ ++ +---+ +---+ | Memory | -- | ISP | -- |SCC| -- |SCP| ++ ++ +---+ +---+ Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com Signed-off-by: Arun Kumar K arun...@samsung.com [snip] + +Common 'camera' node + + +Required properties: + +- compatible : must be samsung,exynos5-fimc, simple-bus I am not sure if this point was discusssed during the previous versions. samsung,exynos5-fimc seems a bit generic. The compatible string should generally point to a specific SoC (the first one to have this IP), something like samsung,exynos5250-fimc. +- clocks : list of clock specifiers, corresponding to entries in + the clock-names property; +- clock-names : must contain sclk_cam0, sclk_cam1 entries, + matching entries in the clocks property. + [snip] +Example: + + aliases { + fimc-lite0 = fimc_lite_0 + }; + + /* Parallel bus IF sensor */ + i2c_0: i2c@1386 { + s5k6aa: sensor@3c { + compatible = samsung,s5k6aafx; + reg = 0x3c; + vddio-supply = ...; + + clock-frequency = 2400; + clocks = ...; + clock-names = mclk; + + port { + s5k6aa_ep: endpoint { + remote-endpoint = fimc0_ep; + bus-width = 8; + hsync-active = 0; + vsync-active = 1; + pclk-sample = 1; + }; + }; + }; + }; + + /* MIPI CSI-2 bus IF sensor */ + s5c73m3: sensor@0x1a { 0x not needed. -- With warm regards, Sachin -- 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