[PATCH] media/platform/exynos4-is/fimc-is - Unmap region obtained by of_iomap
Free memory mapping, if fimc_is_probe is not successful.
Signed-off-by: Arvind Yadav
---
drivers/media/platform/exynos4-is/fimc-is.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c
b/drivers/media/platform/exynos4-is/fimc-is.c
index 32ca55f..10d98a5 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -818,12 +818,13 @@ static int fimc_is_probe(struct platform_device *pdev)
is->irq = irq_of_parse_and_map(dev->of_node, 0);
if (!is->irq) {
dev_err(dev, "no irq found\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto err_iounmap;
}
ret = fimc_is_get_clocks(is);
if (ret < 0)
- return ret;
+ goto err_iounmap;
platform_set_drvdata(pdev, is);
@@ -877,6 +878,8 @@ err_irq:
free_irq(is->irq, is);
err_clk:
fimc_is_put_clocks(is);
+err_iounmap:
+ iounmap(is->pmu_regs);
return ret;
}
@@ -932,6 +935,7 @@ static int fimc_is_remove(struct platform_device *pdev)
fimc_is_unregister_subdevs(is);
vb2_dma_contig_clear_max_seg_size(dev);
fimc_is_put_clocks(is);
+ iounmap(is->pmu_regs);
fimc_is_debugfs_remove(is);
release_firmware(is->fw.f_w);
fimc_is_free_cpu_memory(is);
--
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 v6 3/3] sound/usb: Use Media Controller API to share media resources
On Mon, 05 Dec 2016 23:44:30 +0100, Shuah Khan wrote: > > On 11/30/2016 03:01 PM, Shuah Khan wrote: > > Change ALSA driver to use Media Controller API to share media resources > > with DVB, and V4L2 drivers on a AU0828 media device. > > > > Media Controller specific initialization is done after sound card is > > registered. ALSA creates Media interface and entity function graph > > nodes for Control, Mixer, PCM Playback, and PCM Capture devices. > > > > snd_usb_hw_params() will call Media Controller enable source handler > > interface to request the media resource. If resource request is granted, > > it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is > > returned. > > > > Media specific cleanup is done in usb_audio_disconnect(). > > > > Signed-off-by: Shuah Khan > > Hi Takashi, > > If you are good with this patch, could you please Ack it, so Mauro > can pull it into media tree with the other two patches in this series, > when he is ready to do so. Sorry for the late review. The biggest concern right now is the way the driver is probed. As mentioned in the review reply, we need to consider the cases where the probe is called multiple times. Other things are minor. thanks, Takashi -- 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 v6 3/3] sound/usb: Use Media Controller API to share media resources
On Wed, 30 Nov 2016 23:01:16 +0100,
Shuah Khan wrote:
>
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
(snip)
> @@ -616,6 +617,11 @@ static int usb_audio_probe(struct usb_interface *intf,
> if (err < 0)
> goto __error;
>
> + if (quirk && quirk->media_device) {
> + /* don't want to fail when media_snd_device_create() fails */
> + media_snd_device_create(chip, intf);
Note that the usb-audio driver is probed for each usb interface, and
there are multiple interfaces per device. That is, usb_audio_probe()
may be called multiple times per device, and at the second or the
later calls, it appends the stuff onto the previously created card
object.
So, you'd have to make this call also conditional (e.g. check
chip->num_interfaces == 0, indicating the very first one), or allow to
be called multiple times.
In the former case, it'd be better to split media_snd_device_create()
and media_snd_mixer_init(). The *_device_create() needs to be called
only once, while *_mixer_init() still has to be called for each time
because the new mixer element may be added for each interface.
> + }
> +
> usb_chip[chip->index] = chip;
> chip->num_interfaces++;
> usb_set_intfdata(intf, chip);
> @@ -672,6 +678,14 @@ static void usb_audio_disconnect(struct usb_interface
> *intf)
> list_for_each(p, &chip->midi_list) {
> snd_usbmidi_disconnect(p);
> }
> + /*
> + * Nice to check quirk && quirk->media_device and
> + * then call media_snd_device_delete(). Don't have
> + * access to quirk here. media_snd_device_delete()
> + * acceses mixer_list
> + */
> + media_snd_device_delete(chip);
... meanwhile this is OK, as it's called only once.
(BTW, is it OK to call media_* stuff while the device is still in use?
The disconnect callback gets called for hot-unplug.)
> --- /dev/null
> +++ b/sound/usb/media.c
(snip)
> +void media_snd_stream_delete(struct snd_usb_substream *subs)
> +{
> + struct media_ctl *mctl = subs->media_ctl;
> +
> + if (mctl && mctl->media_dev) {
mctl->media_dev NULL check here is superfluous, as it's checked
mctl->below.
> + struct media_device *mdev;
> +
> + mdev = mctl->media_dev;
> + if (mdev && media_devnode_is_registered(mdev->devnode)) {
> + media_devnode_remove(mctl->intf_devnode);
> + media_device_unregister_entity(&mctl->media_entity);
> + media_entity_cleanup(&mctl->media_entity);
> + }
> + kfree(mctl);
> + subs->media_ctl = NULL;
> + }
> +}
> +
> +int media_snd_start_pipeline(struct snd_usb_substream *subs)
> +{
> + struct media_ctl *mctl = subs->media_ctl;
> +
> + if (mctl)
> + return media_snd_enable_source(mctl);
> + return 0;
> +}
It's merely a wrapper, and media_snd_enable_source() itself checks
NULL mctl. So we can replace media_snd_enable_source() with
media_snd_start_pipeline().
> +void media_snd_stop_pipeline(struct snd_usb_substream *subs)
> +{
> + struct media_ctl *mctl = subs->media_ctl;
> +
> + if (mctl)
> + media_snd_disable_source(mctl);
> +}
Ditto.
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 44d178e..0e4e0640 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
(snip)
> @@ -717,10 +718,14 @@ static int snd_usb_hw_params(struct snd_pcm_substream
> *substream,
> struct audioformat *fmt;
> int ret;
>
> + ret = media_snd_start_pipeline(subs);
> + if (ret)
> + return ret;
It's an open question at which timing we should call
media_snd_start_pipeline(). The hw_params is mostly OK, while the
real timing where the stream might be started is the prepare
callback. I guess we can keep as is for now.
Also, one more thing to be considered is whether
media_snd_start_pipeline() can be called multiple times. hw_params
and prepare callbacks may be called multiple times. I suppose it's
OK, but just to be sure.
> @@ -1234,7 +1246,12 @@ static int snd_usb_pcm_open(struct snd_pcm_substream
> *substream, int direction)
> subs->dsd_dop.channel = 0;
> subs->dsd_dop.marker = 1;
>
> - return setup_hw_info(runtime, subs);
> + ret = setup_hw_info(runtime, subs);
> + if (ret == 0)
> + ret = media_snd_stream_init(subs, as->pcm, direction);
> + if (ret)
> + snd_usb_autosuspend(subs->stream->chip);
> + return ret;
This leads to the PM refcount unbalance. The call of
snd_usb_autosuspend() must be in the former if block,
ret = setup_hw_info(runtime, subs);
if (ret == 0) {
ret = media_snd_stream_init(subs, as->pcm, direction);
if (ret)
snd_usb_autosuspend(subs->stream->chip);
}
return ret;
thanks,
Takashi
--
To unsubscribe
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Dec 6 05:00:14 CET 2016 media-tree git hash:365fe4e0ce218dc5ad10df17b150a366b6015499 media_build git hash: 1606032398b1d79149c1507be2029e1a00d8dff0 v4l-utils git hash: 063d1f5d5e60783002d781e8a23911acbda65e99 gcc version:i686-linux-gcc (GCC) 6.2.0 sparse version: v0.5.0-3553-g78b2ea6 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.8.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: 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.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-3.10.1-i686: WARNINGS linux-3.11.1-i686: OK linux-3.12.67-i686: OK linux-3.13.11-i686: WARNINGS linux-3.14.9-i686: WARNINGS linux-3.15.2-i686: WARNINGS linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: WARNINGS linux-3.18.7-i686: WARNINGS linux-3.19-i686: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.1.33-i686: WARNINGS linux-4.2.8-i686: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.7.5-i686: WARNINGS linux-4.8-i686: OK linux-4.9-rc5-i686: OK linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS linux-3.10.1-x86_64: WARNINGS linux-3.11.1-x86_64: OK linux-3.12.67-x86_64: OK linux-3.13.11-x86_64: WARNINGS linux-3.14.9-x86_64: WARNINGS linux-3.15.2-x86_64: WARNINGS linux-3.16.7-x86_64: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.7-x86_64: WARNINGS linux-3.19-x86_64: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.33-x86_64: WARNINGS linux-4.2.8-x86_64: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.22-x86_64: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.5-x86_64: WARNINGS linux-4.8-x86_64: OK linux-4.9-rc5-x86_64: OK apps: WARNINGS spec-git: OK smatch: ERRORS sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html -- 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 v6 3/3] sound/usb: Use Media Controller API to share media resources
On 12/05/2016 04:21 PM, Laurent Pinchart wrote: > Hi Shuah, > > On Monday 05 Dec 2016 15:44:30 Shuah Khan wrote: >> On 11/30/2016 03:01 PM, Shuah Khan wrote: >>> Change ALSA driver to use Media Controller API to share media resources >>> with DVB, and V4L2 drivers on a AU0828 media device. >>> >>> Media Controller specific initialization is done after sound card is >>> registered. ALSA creates Media interface and entity function graph >>> nodes for Control, Mixer, PCM Playback, and PCM Capture devices. >>> >>> snd_usb_hw_params() will call Media Controller enable source handler >>> interface to request the media resource. If resource request is granted, >>> it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is >>> returned. >>> >>> Media specific cleanup is done in usb_audio_disconnect(). >>> >>> Signed-off-by: Shuah Khan >> >> Hi Takashi, >> >> If you are good with this patch, could you please Ack it, so Mauro >> can pull it into media tree with the other two patches in this series, >> when he is ready to do so. > > I *really* want to address the concerns raised by Sakari before pulling more > code that makes fixing the race conditions more difficult. Please, let's all > work on fixing the core code to build a stable base on which we can build > additional features. V4L2 and MC need teamwork, it's time to give the > subsystem the love it deserves. > Hi Laurent, The issue Sakari brought up is specific to using devm for video_device in omap3 and vsp1. I tried reproducing the problem on two different drivers and couldn't on Linux 4.9-rc7. After sharing that with Sakari, I suggested to Sakari to pull up his patch that removes the devm usage and see if he still needs all the patches in his patch series. He didn't back to me on that. I also requested him to rebase on top of media dev allocator because the allocator routines he has don't address the shared media device need. He also didn't respond to my response regarding the reasons for choosing graph_mutex to protect enable_source and disable_source handlers. So I am not sure how to move forward at the moment without a concrete plan for Sakari's RFC series. Sakari's patch series is still RFC and doesn't address shared media_device and requires all drivers to change. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/9] doc: DT: venus: binding document for Qualcomm video driver
On Thu, Dec 01, 2016 at 11:03:14AM +0200, Stanimir Varbanov wrote:
> Add binding document for Venus video encoder/decoder driver
>
> Cc: Rob Herring
> Cc: Mark Rutland
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Stanimir Varbanov
> ---
> Rob, I have removed vmem clocks, interrupts and reg properties
> for vmem thing. Probably I will come with a separate platform
> driver fro that and pass the video memory DT node as phandle.
Looks good, a couple of minor things below.
>
> .../devicetree/bindings/media/qcom,venus.txt | 82
> ++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,venus.txt
> b/Documentation/devicetree/bindings/media/qcom,venus.txt
> new file mode 100644
> index ..a64b4ea1ebba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,venus.txt
> @@ -0,0 +1,82 @@
> +* Qualcomm Venus video encode/decode accelerator
> +
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: Value should contain one of:
> + - "qcom,msm8916-venus"
> + - "qcom,msm8996-venus"
> +- reg:
> + Usage: required
> + Value type:
> + Definition: Register ranges as listed in the reg-names property.
> +- reg-names:
> + Usage: required
> + Value type:
> + Definition: Should contain following entries:
> + - "base"Venus register base
-names is kind of pointless with only one.
> +- interrupts:
> + Usage: required
> + Value type:
> + Definition: Should contain interrupts as listed in the interrupt-names
> + property.
> +- interrupt-names:
> + Usage: required
> + Value type:
> + Definition: Should contain following entries:
> + - "venus" Venus interrupt line
ditto
> +- clocks:
> + Usage: required
> + Value type:
> + Definition: A List of phandle and clock specifier pairs as listed
> + in clock-names property.
> +- clock-names:
> + Usage: required
> + Value type:
> + Definition: Should contain the following entries:
> + - "core"Core video accelerator clock
> + - "iface" Video accelerator AHB clock
> + - "bus" Video accelerator AXI clock
> +- clock-names:
> + Usage: required for msm8996
It's not clear if this is in addition to the above list. I'd guess not
and you should make it clear the above applies to the 8916 only.
> + Value type:
> + Definition: Should contain the following entries:
> + - "subcore0"Subcore0 video accelerator clock
> + - "subcore1"Subcore1 video accelerator clock
> + - "mmssnoc_axi" Multimedia subsystem NOC AXI clock
> + - "mmss_mmagic_iface" Multimedia subsystem MMAGIC AHB clock
> + - "mmss_mmagic_mbus"Multimedia subsystem MMAGIC MAXI clock
> + - "mmagic_video_bus"MMAGIC video AXI clock
> + - "video_mbus" Video MAXI clock
> +- power-domains:
> + Usage: required
> + Value type:
> + Definition: A phandle and power domain specifier pairs to the
> + power domain which is responsible for collapsing
> + and restoring power to the peripheral.
> +- rproc:
> + Usage: required
> + Value type:
> + Definition: A phandle to remote processor responsible for
> + firmware loading and processor booting.
> +
> +- iommus:
> + Usage: required
> + Value type:
> + Definition: A list of phandle and IOMMU specifier pairs.
> +
> +* An Example
> + video-codec@1d0 {
> + compatible = "qcom,msm8916-venus";
> + reg = <0x01d0 0xff000>;
> + reg-names = "base";
> + interrupts = ;
> + interrupt-names = "venus";
> + clocks = <&gcc GCC_VENUS0_VCODEC0_CLK>,
> + <&gcc GCC_VENUS0_AHB_CLK>,
> + <&gcc GCC_VENUS0_AXI_CLK>;
> + clock-names = "core", "iface", "bus";
> + power-domains = <&gcc VENUS_GDSC>;
> + rproc = <&venus_rproc>;
> + iommus = <&apps_iommu 5>;
> + };
> --
> 2.7.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 v6 3/3] sound/usb: Use Media Controller API to share media resources
Hi Shuah, On Monday 05 Dec 2016 15:44:30 Shuah Khan wrote: > On 11/30/2016 03:01 PM, Shuah Khan wrote: > > Change ALSA driver to use Media Controller API to share media resources > > with DVB, and V4L2 drivers on a AU0828 media device. > > > > Media Controller specific initialization is done after sound card is > > registered. ALSA creates Media interface and entity function graph > > nodes for Control, Mixer, PCM Playback, and PCM Capture devices. > > > > snd_usb_hw_params() will call Media Controller enable source handler > > interface to request the media resource. If resource request is granted, > > it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is > > returned. > > > > Media specific cleanup is done in usb_audio_disconnect(). > > > > Signed-off-by: Shuah Khan > > Hi Takashi, > > If you are good with this patch, could you please Ack it, so Mauro > can pull it into media tree with the other two patches in this series, > when he is ready to do so. I *really* want to address the concerns raised by Sakari before pulling more code that makes fixing the race conditions more difficult. Please, let's all work on fixing the core code to build a stable base on which we can build additional features. V4L2 and MC need teamwork, it's time to give the subsystem the love it deserves. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
em28xx broken 4.9.0-rc6+
Hello Mauro I just noticed current em28xx driver seem to be broken. When I plug device first time it loads correctly, but when I re-plug it, it does not work anymore but yells a lot of noise to message log. Tested with PCTV 290e and 292e both same. Other USB DVB devices are working so it is very likely em28xx driver bug. Easy to reproduce: plug device unplug device plug device regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/3] sound/usb: Use Media Controller API to share media resources
On 11/30/2016 03:01 PM, Shuah Khan wrote:
> Change ALSA driver to use Media Controller API to share media resources
> with DVB, and V4L2 drivers on a AU0828 media device.
>
> Media Controller specific initialization is done after sound card is
> registered. ALSA creates Media interface and entity function graph
> nodes for Control, Mixer, PCM Playback, and PCM Capture devices.
>
> snd_usb_hw_params() will call Media Controller enable source handler
> interface to request the media resource. If resource request is granted,
> it will release it from snd_usb_hw_free(). If resource is busy, -EBUSY is
> returned.
>
> Media specific cleanup is done in usb_audio_disconnect().
>
> Signed-off-by: Shuah Khan
Hi Takashi,
If you are good with this patch, could you please Ack it, so Mauro
can pull it into media tree with the other two patches in this series,
when he is ready to do so.
thanks,
-- Shuah
> ---
> Changes to patch 0003:
> - Changed to hold graph_mutex to check and call enable_source and
> disable_source handlers - to match au0828 doing the same in:
> media: Protect enable_source and disable_source handler code paths
> https://lkml.org/lkml/2016/11/29/1001
>
> sound/usb/Kconfig| 4 +
> sound/usb/Makefile | 2 +
> sound/usb/card.c | 14 ++
> sound/usb/card.h | 3 +
> sound/usb/media.c| 326
> +++
> sound/usb/media.h| 73 +++
> sound/usb/mixer.h| 3 +
> sound/usb/pcm.c | 28 +++-
> sound/usb/quirks-table.h | 1 +
> sound/usb/stream.c | 2 +
> sound/usb/usbaudio.h | 6 +
> 11 files changed, 457 insertions(+), 5 deletions(-)
> create mode 100644 sound/usb/media.c
> create mode 100644 sound/usb/media.h
>
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index a452ad7..d14bf41 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
> select SND_RAWMIDI
> select SND_PCM
> select BITREVERSE
> + select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER &&
> (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)
> help
> Say Y here to include support for USB audio and USB MIDI
> devices.
> @@ -22,6 +23,9 @@ config SND_USB_AUDIO
> To compile this driver as a module, choose M here: the module
> will be called snd-usb-audio.
>
> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> + bool
> +
> config SND_USB_UA101
> tristate "Edirol UA-101/UA-1000 driver"
> select SND_PCM
> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
> index 2d2d122..8dca3c4 100644
> --- a/sound/usb/Makefile
> +++ b/sound/usb/Makefile
> @@ -15,6 +15,8 @@ snd-usb-audio-objs := card.o \
> quirks.o \
> stream.o
>
> +snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o
> +
> snd-usbmidi-lib-objs := midi.o
>
> # Toplevel Module Dependency
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 2ddc034..570ff00 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -66,6 +66,7 @@
> #include "format.h"
> #include "power.h"
> #include "stream.h"
> +#include "media.h"
>
> MODULE_AUTHOR("Takashi Iwai ");
> MODULE_DESCRIPTION("USB Audio");
> @@ -616,6 +617,11 @@ static int usb_audio_probe(struct usb_interface *intf,
> if (err < 0)
> goto __error;
>
> + if (quirk && quirk->media_device) {
> + /* don't want to fail when media_snd_device_create() fails */
> + media_snd_device_create(chip, intf);
> + }
> +
> usb_chip[chip->index] = chip;
> chip->num_interfaces++;
> usb_set_intfdata(intf, chip);
> @@ -672,6 +678,14 @@ static void usb_audio_disconnect(struct usb_interface
> *intf)
> list_for_each(p, &chip->midi_list) {
> snd_usbmidi_disconnect(p);
> }
> + /*
> + * Nice to check quirk && quirk->media_device and
> + * then call media_snd_device_delete(). Don't have
> + * access to quirk here. media_snd_device_delete()
> + * acceses mixer_list
> + */
> + media_snd_device_delete(chip);
> +
> /* release mixer resources */
> list_for_each_entry(mixer, &chip->mixer_list, list) {
> snd_usb_mixer_disconnect(mixer);
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index 111b0f0..6ef8e88 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -105,6 +105,8 @@ struct snd_usb_endpoint {
> struct list_head list;
> };
>
> +struct media_ctl;
> +
> struct snd_usb_substream {
> struct snd_usb_stream *stream;
> struct usb_device *dev;
> @@ -156,6 +158,7 @@ struct snd_usb_substream {
> } dsd_dop;
>
> bool trigger_tstamp_pending_update; /* trigger timestamp being updated
> from initial estimate */
> + struct media
Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.
On 5 December 2016 at 18:18, Marcel Hasler wrote: > 2016-12-05 22:06 GMT+01:00 Marcel Hasler : >> Hello >> >> 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia : >>> On 5 December 2016 at 09:12, Mauro Carvalho Chehab >>> wrote: Em Sun, 4 Dec 2016 15:25:25 -0300 Ezequiel Garcia escreveu: > On 4 December 2016 at 10:01, Marcel Hasler wrote: > > Hello > > > > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia > > : > >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab > >> wrote: > >>> Em Sun, 27 Nov 2016 12:11:48 +0100 > >>> Marcel Hasler escreveu: > >>> > Allow setting a custom record gain for the internal AC97 codec (if > available). This can be > a value between 0 and 15, 8 is the default and should be suitable > for most users. The Windows > driver also sets this to 8 without any possibility for changing it. > >>> > >>> The problem of removing the mixer is that you need this kind of > >>> crap to setup the volumes on a non-standard way. > >>> > >> > >> Right, that's a good point. > >> > >>> NACK. > >>> > >>> Instead, keep the alsa mixer. The way other drivers do (for example, > >>> em28xx) is that they configure the mixer when an input is selected, > >>> increasing the volume of the active audio channel to 100% and muting > >>> the other audio channels. Yet, as the alsa mixer is exported, users > >>> can change the mixer settings in runtime using some alsa (or pa) > >>> mixer application. > >>> > >> > >> Yeah, the AC97 mixer we are currently leveraging > >> exposes many controls that have no meaning in this device, > >> so removing that still looks like an improvement. > >> > >> I guess the proper way is creating our own mixer > >> (not using snd_ac97_mixer) exposing only the record > >> gain knob. > >> > >> Marcel, what do you think? > >> -- > >> Ezequiel García, VanguardiaSur > >> www.vanguardiasur.com.ar > > > > As I have written before, the recording gain isn't actually meant to > > be changed by the user. In the official Windows driver this value is > > hard-coded to 8 and cannot be changed in any way. And there really is > > no good reason why anyone should need to mess with it in the first > > place. The default value will give the best results in pretty much all > > cases and produces approximately the same volume as the internal 8-bit > > ADC whose gain cannot be changed at all, not even by a driver. > > > > I had considered writing a mixer but chose not to. If the gain setting > > is openly exposed to mixer applications, how do you tell the users > > that the value set by the driver already is the optimal and > > recommended value and that they shouldn't mess with the controls > > unless they really have to? By having a module parameter, this setting > > is practically hidden from the normal user but still is available to > > power-users if they think they really need it. In the end it's really > > just a compromise between hiding it completely and exposing it openly. > > Also, this way the driver guarantees reproducible results, since > > there's no need to remember the positions of any volume sliders. > > > > Hm, right. I've never changed the record gain, and it's true that it > doens't really improve the volume. So, I would be OK with having > a module parameter. > > On the other side, we are exposing it currently, through the "Capture" > mixer control: > > Simple mixer control 'Capture',0 > Capabilities: cvolume cswitch cswitch-joined > Capture channels: Front Left - Front Right > Limits: Capture 0 - 15 > Front Left: Capture 10 [67%] [15.00dB] [on] > Front Right: Capture 8 [53%] [12.00dB] [on] > > So, it would be user-friendly to keep the user interface and continue > to expose the same knob - even if the default is the optimal, etc. > > To be completely honest, I don't think any user is really relying > on any REC_GAIN / Capture setting, and I'm completely OK > with having a mixer control or a module parameter. It doesn't matter. If you're positive that *all* stk1160 use the ac97 mixer the same way, and that there's no sense on having a mixer for it, then it would be ok to remove it. >>> >>> Let's remove it then! >>> In such case, then why you need a modprobe parameter to allow setting the record level? If this mixer entry is not used, just set it to zero and be happy with that. >>> >>> Let's remove the module param too, then. >> >> I'm okay with that. >> >>> >>> Thanks, >>> -- >>> Ezequiel García, VanguardiaSur >>> www.vanguardiasur.com.ar >> >> I'm willing to prepare one final patchset, provided we can agree on >> and resolve all issues beforehand. >> >> So far the
Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.
On 5 December 2016 at 18:06, Marcel Hasler wrote: > Hello > > 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia : >> On 5 December 2016 at 09:12, Mauro Carvalho Chehab >> wrote: >>> Em Sun, 4 Dec 2016 15:25:25 -0300 >>> Ezequiel Garcia escreveu: >>> On 4 December 2016 at 10:01, Marcel Hasler wrote: > Hello > > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia > : >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab >> wrote: >>> Em Sun, 27 Nov 2016 12:11:48 +0100 >>> Marcel Hasler escreveu: >>> Allow setting a custom record gain for the internal AC97 codec (if available). This can be a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows driver also sets this to 8 without any possibility for changing it. >>> >>> The problem of removing the mixer is that you need this kind of >>> crap to setup the volumes on a non-standard way. >>> >> >> Right, that's a good point. >> >>> NACK. >>> >>> Instead, keep the alsa mixer. The way other drivers do (for example, >>> em28xx) is that they configure the mixer when an input is selected, >>> increasing the volume of the active audio channel to 100% and muting >>> the other audio channels. Yet, as the alsa mixer is exported, users >>> can change the mixer settings in runtime using some alsa (or pa) >>> mixer application. >>> >> >> Yeah, the AC97 mixer we are currently leveraging >> exposes many controls that have no meaning in this device, >> so removing that still looks like an improvement. >> >> I guess the proper way is creating our own mixer >> (not using snd_ac97_mixer) exposing only the record >> gain knob. >> >> Marcel, what do you think? >> -- >> Ezequiel García, VanguardiaSur >> www.vanguardiasur.com.ar > > As I have written before, the recording gain isn't actually meant to > be changed by the user. In the official Windows driver this value is > hard-coded to 8 and cannot be changed in any way. And there really is > no good reason why anyone should need to mess with it in the first > place. The default value will give the best results in pretty much all > cases and produces approximately the same volume as the internal 8-bit > ADC whose gain cannot be changed at all, not even by a driver. > > I had considered writing a mixer but chose not to. If the gain setting > is openly exposed to mixer applications, how do you tell the users > that the value set by the driver already is the optimal and > recommended value and that they shouldn't mess with the controls > unless they really have to? By having a module parameter, this setting > is practically hidden from the normal user but still is available to > power-users if they think they really need it. In the end it's really > just a compromise between hiding it completely and exposing it openly. > Also, this way the driver guarantees reproducible results, since > there's no need to remember the positions of any volume sliders. > Hm, right. I've never changed the record gain, and it's true that it doens't really improve the volume. So, I would be OK with having a module parameter. On the other side, we are exposing it currently, through the "Capture" mixer control: Simple mixer control 'Capture',0 Capabilities: cvolume cswitch cswitch-joined Capture channels: Front Left - Front Right Limits: Capture 0 - 15 Front Left: Capture 10 [67%] [15.00dB] [on] Front Right: Capture 8 [53%] [12.00dB] [on] So, it would be user-friendly to keep the user interface and continue to expose the same knob - even if the default is the optimal, etc. To be completely honest, I don't think any user is really relying on any REC_GAIN / Capture setting, and I'm completely OK with having a mixer control or a module parameter. It doesn't matter. >>> >>> If you're positive that *all* stk1160 use the ac97 mixer the >>> same way, and that there's no sense on having a mixer for it, >>> then it would be ok to remove it. >>> >> >> Let's remove it then! >> >>> In such case, then why you need a modprobe parameter to allow >>> setting the record level? If this mixer entry is not used, >>> just set it to zero and be happy with that. >>> >> >> Let's remove the module param too, then. > > I'm okay with that. > >> >> Thanks, >> -- >> Ezequiel García, VanguardiaSur >> www.vanguardiasur.com.ar > > I'm willing to prepare one final patchset, provided we can agree on > and resolve all issues beforehand. > > So far the changes would be to remove the module param and to poll > STK1160_AC97CTL_0 instead of using a fixed delay. It's probably better > to also poll it before writing, a
Re: [PATCH v2 3/3] uvcvideo: add a metadata device node
Hi Guennadi, On Monday 05 Dec 2016 23:13:53 Guennadi Liakhovetski wrote: > Just one question: > > On Tue, 6 Dec 2016, Laurent Pinchart wrote: > +/* > + * Register a metadata node. TODO: shall this only be enabled > for some > + * cameras? > + */ > +if (!(dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT)) > +uvc_meta_register(stream); > + > >>> > >>> I think so, only for the cameras that can produce metadata. > >> > >> Every UVC camera produces metadata, but most cameras only have standard > >> fields there. Whether we should stream standard header fields from the > >> metadata node will be discussed later. If we do decide to stream > >> standard header fields, then every USB camera gets metadata nodes. If we > >> decide not to include standard fields, how do we know whether the camera > >> has any private fields in headers without streaming from it? Do you want > >> a quirk for such cameras? > > > > Unless they can be detected in a standard way that's probably the best > > solution. Please remember that the UVC specification doesn't allow vendors > > to extend headers in a vendor-specific way. > > Does it not? Where is that specified? I didn't find that anywhere. It's the other way around, it document a standard way to extend the payload header. Any option you pick risks being incompatible with future revisions of the specification. For instance the payload header's bmHeaderInfo field can be extended through the use of the EOF bit, but a future version of the specification could also extend it, in a way that would make a vendor-specific extension be mistaken for the standard extension. Now, you could add data after the standard payload without touching the bmHeaderInfo field, but I'm still worried it could be broken by future versions of the specification. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] [media] dt-bindings: add TI VPIF documentation
On Tue, Nov 29, 2016 at 03:57:12PM -0800, Kevin Hilman wrote: > Signed-off-by: Kevin Hilman > --- > .../devicetree/bindings/media/ti,da850-vpif.txt| 67 > ++ > 1 file changed, 67 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt Acked-by: Rob Herring -- 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 3/3] uvcvideo: add a metadata device node
Just one question: On Tue, 6 Dec 2016, Laurent Pinchart wrote: > > >> +/* > > >> + * Register a metadata node. TODO: shall this only be enabled > > >> for some > > >> + * cameras? > > >> + */ > > >> +if (!(dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT)) > > >> +uvc_meta_register(stream); > > >> + > > > > > > I think so, only for the cameras that can produce metadata. > > > > Every UVC camera produces metadata, but most cameras only have standard > > fields there. Whether we should stream standard header fields from the > > metadata node will be discussed later. If we do decide to stream standard > > header fields, then every USB camera gets metadata nodes. If we decide not > > to include standard fields, how do we know whether the camera has any > > private fields in headers without streaming from it? Do you want a quirk > > for such cameras? > > Unless they can be detected in a standard way that's probably the best > solution. Please remember that the UVC specification doesn't allow vendors to > extend headers in a vendor-specific way. Does it not? Where is that specified? I didn't find that anywhere. Thanks Guennadi -- 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 3/3] uvcvideo: add a metadata device node
Hi Guennadi,
On Monday 05 Dec 2016 16:35:39 Guennadi Liakhovetski wrote:
> On Mon, 5 Dec 2016, Laurent Pinchart wrote:
> > On Friday 02 Dec 2016 11:53:23 Guennadi Liakhovetski wrote:
> >> Some UVC video cameras contain metadata in their payload headers. This
> >> patch extracts that data, skipping the standard part of the header, on
> >> both bulk and isochronous endpoints and makes it available to the user
> >> space on a separate video node, using the V4L2_CAP_META_CAPTURE
> >> capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> >> though different cameras will have different metadata formats, we use
> >> the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> >> parse data, based on the specific camera model information.
> >>
> >> Signed-off-by: Guennadi Liakhovetski
> >> ---
> >>
> >> v2:
> >> - updated to the current media/master
> >> - removed superfluous META capability from capture nodes
> >> - now the complete UVC payload header is copied to buffers, including
> >> standard fields
> >>
> >> Still open for discussion: is this really OK to always create an
> >> additional metadata node for each UVC camera or UVC video interface.
>
> [snip]
>
> >> + /*
> >> + * Register a metadata node. TODO: shall this only be enabled for some
> >> + * cameras?
> >> + */
> >> + if (!(dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT))
> >> + uvc_meta_register(stream);
> >> +
> >
> > I think so, only for the cameras that can produce metadata.
>
> Every UVC camera produces metadata, but most cameras only have standard
> fields there. Whether we should stream standard header fields from the
> metadata node will be discussed later. If we do decide to stream standard
> header fields, then every USB camera gets metadata nodes. If we decide not
> to include standard fields, how do we know whether the camera has any
> private fields in headers without streaming from it? Do you want a quirk
> for such cameras?
Unless they can be detected in a standard way that's probably the best
solution. Please remember that the UVC specification doesn't allow vendors to
extend headers in a vendor-specific way. This is an abuse of the
specification, so a quirk is probably the best option.
> [snip]
>
> > > +static struct vb2_ops uvc_meta_queue_ops = {
> > > + .queue_setup = meta_queue_setup,
> > > + .buf_prepare = meta_buffer_prepare,
> > > + .buf_queue = meta_buffer_queue,
> > > + .wait_prepare = vb2_ops_wait_prepare,
> > > + .wait_finish = vb2_ops_wait_finish,
> > > + .start_streaming = meta_start_streaming,
> > > + .stop_streaming = meta_stop_streaming,
> > > +};
> >
> > How about reusing the uvc_queue.c implementation, with a few new checks
> > for metadata buffers where needed, instead of duplicating code ? At first
> > sight the changes don't seem to be too intrusive (but I might have
> > overlooked something).
>
> I thought about that in the beginning and even started implementing it
> that way, but at some point it became too inconvenient, so, I switched
> over to a separate implementation. I'll think about it more and either
> explain, why I didn't want to do that, or unite them.
>
> [snip]
>
> > > +{
> > > + size_t nbytes;
> > > +
> > > + if (!meta_buf || !length)
> > > + return;
> > > +
> > > + nbytes = min_t(unsigned int, length, meta_buf->length);
> > > +
> > > + meta_buf->buf.sequence = buf->buf.sequence;
> > > + meta_buf->buf.field = buf->buf.field;
> > > + meta_buf->buf.vb2_buf.timestamp = buf->buf.vb2_buf.timestamp;
> > > +
> > > + memcpy(meta_buf->mem, mem, nbytes);
> >
> > Do you need the whole header ? Shouldn't you strip the part already
> > handled by the driver ?
>
> My original version did that, but we also need the timestamp from the
> header. The driver does parse it, but the implementation there has
> multiple times been reported as buggy and noone has been able to fix it so
> far :)
-ENOTIME I'm afraid, but feel free to give it a go :-) On the other hand
supplying the PTS and SCR values to userspace would be useful to implement a
high-accuracy clock translation algorithm that could make use of floating
point operations.
> So, I ended up pulling the complete header to the user-space. Unless time-
> stamp processing is fixed in the kernel, I don't think we can frop this.
SCR and PTS should be provided to userspace in a standard way.
> >> + meta_buf->bytesused = nbytes;
> >> + meta_buf->state = UVC_BUF_STATE_READY;
> >> +
> >> + uvc_queue_next_buffer(&stream->meta.queue, meta_buf);
> >
> > This essentially means that you'll need one buffer per isochronous packet.
> > Given the number of isochronous packets that make a complete image the
> > cost seem prohibitive to me. You should instead gather metadata from all
> > headers into a single buffer.
>
> Hm, I only worked with cameras, using bulk transfers, so, didn't consider
> ISOC implications. Will have to think about this.
--
Regards,
Laurent Pinchart
--
To unsubscribe from thi
Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.
2016-12-05 22:06 GMT+01:00 Marcel Hasler : > Hello > > 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia : >> On 5 December 2016 at 09:12, Mauro Carvalho Chehab >> wrote: >>> Em Sun, 4 Dec 2016 15:25:25 -0300 >>> Ezequiel Garcia escreveu: >>> On 4 December 2016 at 10:01, Marcel Hasler wrote: > Hello > > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia > : >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab >> wrote: >>> Em Sun, 27 Nov 2016 12:11:48 +0100 >>> Marcel Hasler escreveu: >>> Allow setting a custom record gain for the internal AC97 codec (if available). This can be a value between 0 and 15, 8 is the default and should be suitable for most users. The Windows driver also sets this to 8 without any possibility for changing it. >>> >>> The problem of removing the mixer is that you need this kind of >>> crap to setup the volumes on a non-standard way. >>> >> >> Right, that's a good point. >> >>> NACK. >>> >>> Instead, keep the alsa mixer. The way other drivers do (for example, >>> em28xx) is that they configure the mixer when an input is selected, >>> increasing the volume of the active audio channel to 100% and muting >>> the other audio channels. Yet, as the alsa mixer is exported, users >>> can change the mixer settings in runtime using some alsa (or pa) >>> mixer application. >>> >> >> Yeah, the AC97 mixer we are currently leveraging >> exposes many controls that have no meaning in this device, >> so removing that still looks like an improvement. >> >> I guess the proper way is creating our own mixer >> (not using snd_ac97_mixer) exposing only the record >> gain knob. >> >> Marcel, what do you think? >> -- >> Ezequiel García, VanguardiaSur >> www.vanguardiasur.com.ar > > As I have written before, the recording gain isn't actually meant to > be changed by the user. In the official Windows driver this value is > hard-coded to 8 and cannot be changed in any way. And there really is > no good reason why anyone should need to mess with it in the first > place. The default value will give the best results in pretty much all > cases and produces approximately the same volume as the internal 8-bit > ADC whose gain cannot be changed at all, not even by a driver. > > I had considered writing a mixer but chose not to. If the gain setting > is openly exposed to mixer applications, how do you tell the users > that the value set by the driver already is the optimal and > recommended value and that they shouldn't mess with the controls > unless they really have to? By having a module parameter, this setting > is practically hidden from the normal user but still is available to > power-users if they think they really need it. In the end it's really > just a compromise between hiding it completely and exposing it openly. > Also, this way the driver guarantees reproducible results, since > there's no need to remember the positions of any volume sliders. > Hm, right. I've never changed the record gain, and it's true that it doens't really improve the volume. So, I would be OK with having a module parameter. On the other side, we are exposing it currently, through the "Capture" mixer control: Simple mixer control 'Capture',0 Capabilities: cvolume cswitch cswitch-joined Capture channels: Front Left - Front Right Limits: Capture 0 - 15 Front Left: Capture 10 [67%] [15.00dB] [on] Front Right: Capture 8 [53%] [12.00dB] [on] So, it would be user-friendly to keep the user interface and continue to expose the same knob - even if the default is the optimal, etc. To be completely honest, I don't think any user is really relying on any REC_GAIN / Capture setting, and I'm completely OK with having a mixer control or a module parameter. It doesn't matter. >>> >>> If you're positive that *all* stk1160 use the ac97 mixer the >>> same way, and that there's no sense on having a mixer for it, >>> then it would be ok to remove it. >>> >> >> Let's remove it then! >> >>> In such case, then why you need a modprobe parameter to allow >>> setting the record level? If this mixer entry is not used, >>> just set it to zero and be happy with that. >>> >> >> Let's remove the module param too, then. > > I'm okay with that. > >> >> Thanks, >> -- >> Ezequiel García, VanguardiaSur >> www.vanguardiasur.com.ar > > I'm willing to prepare one final patchset, provided we can agree on > and resolve all issues beforehand. > > So far the changes would be to remove the module param and to poll > STK1160_AC97CTL_0 instead of using a fixed delay. It's probably better > to also poll it before writing, although
Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.
Hello 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia : > On 5 December 2016 at 09:12, Mauro Carvalho Chehab > wrote: >> Em Sun, 4 Dec 2016 15:25:25 -0300 >> Ezequiel Garcia escreveu: >> >>> On 4 December 2016 at 10:01, Marcel Hasler wrote: >>> > Hello >>> > >>> > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia >>> > : >>> >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab >>> >> wrote: >>> >>> Em Sun, 27 Nov 2016 12:11:48 +0100 >>> >>> Marcel Hasler escreveu: >>> >>> >>> Allow setting a custom record gain for the internal AC97 codec (if >>> available). This can be >>> a value between 0 and 15, 8 is the default and should be suitable for >>> most users. The Windows >>> driver also sets this to 8 without any possibility for changing it. >>> >>> >>> >>> The problem of removing the mixer is that you need this kind of >>> >>> crap to setup the volumes on a non-standard way. >>> >>> >>> >> >>> >> Right, that's a good point. >>> >> >>> >>> NACK. >>> >>> >>> >>> Instead, keep the alsa mixer. The way other drivers do (for example, >>> >>> em28xx) is that they configure the mixer when an input is selected, >>> >>> increasing the volume of the active audio channel to 100% and muting >>> >>> the other audio channels. Yet, as the alsa mixer is exported, users >>> >>> can change the mixer settings in runtime using some alsa (or pa) >>> >>> mixer application. >>> >>> >>> >> >>> >> Yeah, the AC97 mixer we are currently leveraging >>> >> exposes many controls that have no meaning in this device, >>> >> so removing that still looks like an improvement. >>> >> >>> >> I guess the proper way is creating our own mixer >>> >> (not using snd_ac97_mixer) exposing only the record >>> >> gain knob. >>> >> >>> >> Marcel, what do you think? >>> >> -- >>> >> Ezequiel García, VanguardiaSur >>> >> www.vanguardiasur.com.ar >>> > >>> > As I have written before, the recording gain isn't actually meant to >>> > be changed by the user. In the official Windows driver this value is >>> > hard-coded to 8 and cannot be changed in any way. And there really is >>> > no good reason why anyone should need to mess with it in the first >>> > place. The default value will give the best results in pretty much all >>> > cases and produces approximately the same volume as the internal 8-bit >>> > ADC whose gain cannot be changed at all, not even by a driver. >>> > >>> > I had considered writing a mixer but chose not to. If the gain setting >>> > is openly exposed to mixer applications, how do you tell the users >>> > that the value set by the driver already is the optimal and >>> > recommended value and that they shouldn't mess with the controls >>> > unless they really have to? By having a module parameter, this setting >>> > is practically hidden from the normal user but still is available to >>> > power-users if they think they really need it. In the end it's really >>> > just a compromise between hiding it completely and exposing it openly. >>> > Also, this way the driver guarantees reproducible results, since >>> > there's no need to remember the positions of any volume sliders. >>> > >>> >>> Hm, right. I've never changed the record gain, and it's true that it >>> doens't really improve the volume. So, I would be OK with having >>> a module parameter. >>> >>> On the other side, we are exposing it currently, through the "Capture" >>> mixer control: >>> >>> Simple mixer control 'Capture',0 >>> Capabilities: cvolume cswitch cswitch-joined >>> Capture channels: Front Left - Front Right >>> Limits: Capture 0 - 15 >>> Front Left: Capture 10 [67%] [15.00dB] [on] >>> Front Right: Capture 8 [53%] [12.00dB] [on] >>> >>> So, it would be user-friendly to keep the user interface and continue >>> to expose the same knob - even if the default is the optimal, etc. >>> >>> To be completely honest, I don't think any user is really relying >>> on any REC_GAIN / Capture setting, and I'm completely OK >>> with having a mixer control or a module parameter. It doesn't matter. >> >> If you're positive that *all* stk1160 use the ac97 mixer the >> same way, and that there's no sense on having a mixer for it, >> then it would be ok to remove it. >> > > Let's remove it then! > >> In such case, then why you need a modprobe parameter to allow >> setting the record level? If this mixer entry is not used, >> just set it to zero and be happy with that. >> > > Let's remove the module param too, then. I'm okay with that. > > Thanks, > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar I'm willing to prepare one final patchset, provided we can agree on and resolve all issues beforehand. So far the changes would be to remove the module param and to poll STK1160_AC97CTL_0 instead of using a fixed delay. It's probably better to also poll it before writing, although that never caused problems. I'll post some code for review before actually submitting patches. Mauro, is there anything else that you think should be changed? If so, please
Re: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 05, 2016 at 12:46:14PM -0700, Jason Gunthorpe wrote: > In any event the allocator still needs to track which regions are in > use and be able to hook 'free' from userspace. That does suggest it > should be integrated into the nvme driver and not a bolt on driver.. Two totally different use cases: - a card that exposes directly byte addressable storage as a PCI-e bar. Thin of it as a nvdimm on a PCI-e card. That's the iopmem case. - the NVMe CMB which exposes a byte addressable indirection buffer for I/O, but does not actually provide byte addressable persistent storage. This is something that needs to be added to the NVMe driver (and the block layer for the abstraction probably). -- 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: Enabling peer to peer device transactions for PCIe devices
On 05/12/16 12:46 PM, Jason Gunthorpe wrote: NVMe might have to deal with pci-e hot-unplug, which is a similar problem-class to the GPU case.. Sure, but if the NVMe device gets hot-unplugged it means that all the CMB mappings are useless and need to be torn down. This probably means killing any process that has mappings open. In any event the allocator still needs to track which regions are in use and be able to hook 'free' from userspace. That does suggest it should be integrated into the nvme driver and not a bolt on driver.. Yup, that's correct. And yes, I've never suggested this to be a bolt on driver -- I always expected for it to get integrated into the nvme driver. (iopmem was not meant for this.) Logan -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 05, 2016 at 12:27:20PM -0700, Logan Gunthorpe wrote: > > > On 05/12/16 12:14 PM, Jason Gunthorpe wrote: > >But CMB sounds much more like the GPU case where there is a > >specialized allocator handing out the BAR to consumers, so I'm not > >sure a general purpose chardev makes a lot of sense? > > I don't think it will ever need to be as complicated as the GPU case. There > will probably only ever be a relatively small amount of memory behind the > CMB and really the only users are those doing P2P work. Thus the specialized > allocator could be pretty simple and I expect it would be fine to just > return -ENOMEM if there is not enough memory. NVMe might have to deal with pci-e hot-unplug, which is a similar problem-class to the GPU case.. In any event the allocator still needs to track which regions are in use and be able to hook 'free' from userspace. That does suggest it should be integrated into the nvme driver and not a bolt on driver.. Jason -- 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: Enabling peer to peer device transactions for PCIe devices
On 05/12/16 12:14 PM, Jason Gunthorpe wrote: But CMB sounds much more like the GPU case where there is a specialized allocator handing out the BAR to consumers, so I'm not sure a general purpose chardev makes a lot of sense? I don't think it will ever need to be as complicated as the GPU case. There will probably only ever be a relatively small amount of memory behind the CMB and really the only users are those doing P2P work. Thus the specialized allocator could be pretty simple and I expect it would be fine to just return -ENOMEM if there is not enough memory. Also, if it was implemented this way, if there was a need to make the allocator more complicated it could easily be added later as the userspace interface is just mmap to obtain a buffer. Logan -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 05, 2016 at 10:48:58AM -0800, Dan Williams wrote: > On Mon, Dec 5, 2016 at 10:39 AM, Logan Gunthorpe wrote: > > On 05/12/16 11:08 AM, Dan Williams wrote: > >> > >> I've already recommended that iopmem not be a block device and instead > >> be a device-dax instance. I also don't think it should claim the PCI > >> ID, rather the driver that wants to map one of its bars this way can > >> register the memory region with the device-dax core. > >> > >> I'm not sure there are enough device drivers that want to do this to > >> have it be a generic /sys/.../resource_dmableX capability. It still > >> seems to be an exotic one-off type of configuration. > > > > > > Yes, this is essentially my thinking. Except I think the userspace interface > > should really depend on the device itself. Device dax is a good choice for > > many and I agree the block device approach wouldn't be ideal. > > > > Specifically for NVME CMB: I think it would make a lot of sense to just hand > > out these mappings with an mmap call on /dev/nvmeX. I expect CMB buffers > > would be volatile and thus you wouldn't need to keep track of where in the > > BAR the region came from. Thus, the mmap call would just be an allocator > > from BAR memory. If device-dax were used, userspace would need to lookup > > which device-dax instance corresponds to which nvme drive. > > I'm not opposed to mapping /dev/nvmeX. However, the lookup is trivial > to accomplish in sysfs through /sys/dev/char to find the sysfs path > of But CMB sounds much more like the GPU case where there is a specialized allocator handing out the BAR to consumers, so I'm not sure a general purpose chardev makes a lot of sense? Jason -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 5, 2016 at 10:39 AM, Logan Gunthorpe wrote: > On 05/12/16 11:08 AM, Dan Williams wrote: >> >> I've already recommended that iopmem not be a block device and instead >> be a device-dax instance. I also don't think it should claim the PCI >> ID, rather the driver that wants to map one of its bars this way can >> register the memory region with the device-dax core. >> >> I'm not sure there are enough device drivers that want to do this to >> have it be a generic /sys/.../resource_dmableX capability. It still >> seems to be an exotic one-off type of configuration. > > > Yes, this is essentially my thinking. Except I think the userspace interface > should really depend on the device itself. Device dax is a good choice for > many and I agree the block device approach wouldn't be ideal. > > Specifically for NVME CMB: I think it would make a lot of sense to just hand > out these mappings with an mmap call on /dev/nvmeX. I expect CMB buffers > would be volatile and thus you wouldn't need to keep track of where in the > BAR the region came from. Thus, the mmap call would just be an allocator > from BAR memory. If device-dax were used, userspace would need to lookup > which device-dax instance corresponds to which nvme drive. > I'm not opposed to mapping /dev/nvmeX. However, the lookup is trivial to accomplish in sysfs through /sys/dev/char to find the sysfs path of the device-dax instance under the nvme device, or if you already have the nvme sysfs path the dax instance(s) will appear under the "dax" sub-directory. -- 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: Enabling peer to peer device transactions for PCIe devices
On 05/12/16 11:08 AM, Dan Williams wrote: I've already recommended that iopmem not be a block device and instead be a device-dax instance. I also don't think it should claim the PCI ID, rather the driver that wants to map one of its bars this way can register the memory region with the device-dax core. I'm not sure there are enough device drivers that want to do this to have it be a generic /sys/.../resource_dmableX capability. It still seems to be an exotic one-off type of configuration. Yes, this is essentially my thinking. Except I think the userspace interface should really depend on the device itself. Device dax is a good choice for many and I agree the block device approach wouldn't be ideal. Specifically for NVME CMB: I think it would make a lot of sense to just hand out these mappings with an mmap call on /dev/nvmeX. I expect CMB buffers would be volatile and thus you wouldn't need to keep track of where in the BAR the region came from. Thus, the mmap call would just be an allocator from BAR memory. If device-dax were used, userspace would need to lookup which device-dax instance corresponds to which nvme drive. Logan -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 5, 2016 at 10:02 AM, Jason Gunthorpe wrote: > On Mon, Dec 05, 2016 at 09:40:38AM -0800, Dan Williams wrote: > >> > If it is kernel only with physical addresess we don't need a uAPI for >> > it, so I'm not sure #1 is at all related to iopmem. >> > >> > Most people who want #1 probably can just mmap >> > /sys/../pci/../resourceX to get a user handle to it, or pass around >> > __iomem pointers in the kernel. This has been asked for before with >> > RDMA. >> > >> > I'm still not really clear what iopmem is for, or why DAX should ever >> > be involved in this.. >> >> Right, by default remap_pfn_range() does not establish DMA capable >> mappings. You can think of iopmem as remap_pfn_range() converted to >> use devm_memremap_pages(). Given the extra constraints of >> devm_memremap_pages() it seems reasonable to have those DMA capable >> mappings be optionally established via a separate driver. > > Except the iopmem driver claims the PCI ID, and presents a block > interface which is really *NOT* what people who have asked for this in > the past have wanted. IIRC it was embedded stuff eg RDMA video > directly out of a capture card or a similar kind of thinking. > > It is a good point about devm_memremap_pages limitations, but maybe > that just says to create a /sys/.../resource_dmableX ? > > Or is there some reason why people want a filesystem on top of BAR > memory? That does not seem to have been covered yet.. > I've already recommended that iopmem not be a block device and instead be a device-dax instance. I also don't think it should claim the PCI ID, rather the driver that wants to map one of its bars this way can register the memory region with the device-dax core. I'm not sure there are enough device drivers that want to do this to have it be a generic /sys/.../resource_dmableX capability. It still seems to be an exotic one-off type of configuration. -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 05, 2016 at 09:40:38AM -0800, Dan Williams wrote: > > If it is kernel only with physical addresess we don't need a uAPI for > > it, so I'm not sure #1 is at all related to iopmem. > > > > Most people who want #1 probably can just mmap > > /sys/../pci/../resourceX to get a user handle to it, or pass around > > __iomem pointers in the kernel. This has been asked for before with > > RDMA. > > > > I'm still not really clear what iopmem is for, or why DAX should ever > > be involved in this.. > > Right, by default remap_pfn_range() does not establish DMA capable > mappings. You can think of iopmem as remap_pfn_range() converted to > use devm_memremap_pages(). Given the extra constraints of > devm_memremap_pages() it seems reasonable to have those DMA capable > mappings be optionally established via a separate driver. Except the iopmem driver claims the PCI ID, and presents a block interface which is really *NOT* what people who have asked for this in the past have wanted. IIRC it was embedded stuff eg RDMA video directly out of a capture card or a similar kind of thinking. It is a good point about devm_memremap_pages limitations, but maybe that just says to create a /sys/.../resource_dmableX ? Or is there some reason why people want a filesystem on top of BAR memory? That does not seem to have been covered yet.. Jason -- 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: Enabling peer to peer device transactions for PCIe devices
On Mon, Dec 5, 2016 at 9:18 AM, Jason Gunthorpe wrote: > On Sun, Dec 04, 2016 at 07:23:00AM -0600, Stephen Bates wrote: >> Hi All >> >> This has been a great thread (thanks to Alex for kicking it off) and I >> wanted to jump in and maybe try and put some summary around the >> discussion. I also wanted to propose we include this as a topic for LFS/MM >> because I think we need more discussion on the best way to add this >> functionality to the kernel. >> >> As far as I can tell the people looking for P2P support in the kernel fall >> into two main camps: >> >> 1. Those who simply want to expose static BARs on PCIe devices that can be >> used as the source/destination for DMAs from another PCIe device. This >> group has no need for memory invalidation and are happy to use >> physical/bus addresses and not virtual addresses. > > I didn't think there was much on this topic except for the CMB > thing.. Even that is really a mapped kernel address.. > >> I think something like the iopmem patches Logan and I submitted recently >> come close to addressing use case 1. There are some issues around >> routability but based on feedback to date that does not seem to be a >> show-stopper for an initial inclusion. > > If it is kernel only with physical addresess we don't need a uAPI for > it, so I'm not sure #1 is at all related to iopmem. > > Most people who want #1 probably can just mmap > /sys/../pci/../resourceX to get a user handle to it, or pass around > __iomem pointers in the kernel. This has been asked for before with > RDMA. > > I'm still not really clear what iopmem is for, or why DAX should ever > be involved in this.. Right, by default remap_pfn_range() does not establish DMA capable mappings. You can think of iopmem as remap_pfn_range() converted to use devm_memremap_pages(). Given the extra constraints of devm_memremap_pages() it seems reasonable to have those DMA capable mappings be optionally established via a separate driver. -- 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 v5 1/2] Add OV5647 device tree documentation
Add device tree documentation. Signed-off-by: Ramiro Oliveira --- .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt new file mode 100644 index 000..4c91b3b --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt @@ -0,0 +1,19 @@ +Omnivision OV5647 raw image sensor +- + +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces +and CCI (I2C compatible) control bus. + +Required properties: + +- compatible : "ovti,ov5647"; +- reg : I2C slave address of the sensor; + +The common video interfaces bindings (see video-interfaces.txt) should be +used to specify link to the image data receiver. The OV5647 device +node should contain one 'port' child node with an 'endpoint' subnode. + +Following properties are valid for the endpoint node: + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. The sensor supports only two data lanes. -- 2.10.2 -- 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 v5 2/2] Add support for OV5647 sensor
Add support for OV5647 sensor.
Modes supported:
- 640x480 RAW 8
Signed-off-by: Ramiro Oliveira
---
MAINTAINERS| 7 +
drivers/media/i2c/Kconfig | 12 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/ov5647.c | 866 +
4 files changed, 886 insertions(+)
create mode 100644 drivers/media/i2c/ov5647.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 52cc077..72e828a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8923,6 +8923,13 @@ M: Harald Welte
S: Maintained
F: drivers/char/pcmcia/cm4040_cs.*
+OMNIVISION OV5647 SENSOR DRIVER
+M: Ramiro Oliveira
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov5647.c
+
OMNIVISION OV7670 SENSOR DRIVER
M: Jonathan Corbet
L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index b31fa6f..c1b78e5 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -531,6 +531,18 @@ config VIDEO_OV2659
To compile this driver as a module, choose M here: the
module will be called ov2659.
+config VIDEO_OV5647
+ tristate "OmniVision OV5647 sensor support"
+ depends on OF
+ depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+ depends on MEDIA_CAMERA_SUPPORT
+ ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV5647 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov5647.
+
config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 92773b2..0d9014c 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
+obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
new file mode 100644
index 000..2aae806
--- /dev/null
+++ b/drivers/media/i2c/ov5647.c
@@ -0,0 +1,866 @@
+/*
+ * A V4L2 driver for OmniVision OV5647 cameras.
+ *
+ * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
+ * Copyright (C) 2011 Sylwester Nawrocki
+ *
+ * Based on Omnivision OV7670 Camera Driver
+ * Copyright (C) 2006-7 Jonathan Corbet
+ *
+ * Copyright (C) 2016, Synopsys, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SENSOR_NAME "ov5647"
+
+#define OV5647_SW_RESET0x1003
+#define OV5647_REG_CHIPID_H0x300A
+#define OV5647_REG_CHIPID_L0x300B
+
+#define REG_TERM 0xfffe
+#define VAL_TERM 0xfe
+#define REG_DLY 0x
+
+#define OV5647_ROW_START 0x01
+#define OV5647_ROW_START_MIN 0
+#define OV5647_ROW_START_MAX 2004
+#define OV5647_ROW_START_DEF 54
+
+#define OV5647_COLUMN_START0x02
+#define OV5647_COLUMN_START_MIN0
+#define OV5647_COLUMN_START_MAX2750
+#define OV5647_COLUMN_START_DEF16
+
+#define OV5647_WINDOW_HEIGHT 0x03
+#define OV5647_WINDOW_HEIGHT_MIN 2
+#define OV5647_WINDOW_HEIGHT_MAX 2006
+#define OV5647_WINDOW_HEIGHT_DEF 1944
+
+#define OV5647_WINDOW_WIDTH0x04
+#define OV5647_WINDOW_WIDTH_MIN2
+#define OV5647_WINDOW_WIDTH_MAX2752
+#define OV5647_WINDOW_WIDTH_DEF2592
+
+struct regval_list {
+ u16 addr;
+ u8 data;
+};
+
+struct cfg_array {
+ struct regval_list *regs;
+ int size;
+};
+
+struct sensor_win_size {
+ int width;
+ int height;
+ unsigned int hoffset;
+ unsigned int voffset;
+ unsigned int hts;
+ unsigned int vts;
+ unsigned int pclk;
+ unsigned int mipi_bps;
+ unsigned int fps_fixed;
+ unsigned int bin_factor;
+ unsigned int intg_min;
+ unsigned int intg_max;
+ void *regs;
+ int regs_size;
+ int (*set_size)(struct v4l2_subdev *sd);
+};
+
+
+struct ov5647 {
+ struct device *dev;
+ struct v4l2_subdev sd;
+ struct media_padpad;
+ struct mutexlock;
+
[PATCH v5 0/2] Add support for Omnivision OV5647
Hello, This patch adds support for the Omnivision OV5647 sensor. At the moment it only supports 640x480 in Raw 8. This is the fifth version of the OV5647 camera driver patchset. v5: - Refactor code - Change comments - Add missing error handling in some functions v4: - Add correct license - Revert debugging info to generic infrastructure - Turn defines into enums - Correct code style issues - Remove unused defines - Make sure all errors where being handled - Rename some functions to make code more readable - Add some debugging info v3: - No changes. Re-submitted due to lack of responses v2: - Corrections in DT documentation Ramiro Oliveira (2): Add OV5647 device tree documentation Add support for OV5647 sensor .../devicetree/bindings/media/i2c/ov5647.txt | 19 + MAINTAINERS| 7 + drivers/media/i2c/Kconfig | 12 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov5647.c | 866 + 5 files changed, 905 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt create mode 100644 drivers/media/i2c/ov5647.c -- 2.10.2 -- 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: Enabling peer to peer device transactions for PCIe devices
On Sun, Dec 04, 2016 at 07:23:00AM -0600, Stephen Bates wrote: > Hi All > > This has been a great thread (thanks to Alex for kicking it off) and I > wanted to jump in and maybe try and put some summary around the > discussion. I also wanted to propose we include this as a topic for LFS/MM > because I think we need more discussion on the best way to add this > functionality to the kernel. > > As far as I can tell the people looking for P2P support in the kernel fall > into two main camps: > > 1. Those who simply want to expose static BARs on PCIe devices that can be > used as the source/destination for DMAs from another PCIe device. This > group has no need for memory invalidation and are happy to use > physical/bus addresses and not virtual addresses. I didn't think there was much on this topic except for the CMB thing.. Even that is really a mapped kernel address.. > I think something like the iopmem patches Logan and I submitted recently > come close to addressing use case 1. There are some issues around > routability but based on feedback to date that does not seem to be a > show-stopper for an initial inclusion. If it is kernel only with physical addresess we don't need a uAPI for it, so I'm not sure #1 is at all related to iopmem. Most people who want #1 probably can just mmap /sys/../pci/../resourceX to get a user handle to it, or pass around __iomem pointers in the kernel. This has been asked for before with RDMA. I'm still not really clear what iopmem is for, or why DAX should ever be involved in this.. > For use-case 2 it looks like there are several options and some of them > (like HMM) have been around for quite some time without gaining > acceptance. I think there needs to be more discussion on this usecase and > it could be some time before we get something upstreamable. AFAIK, hmm makes parts easier, but isn't directly addressing this need.. I think you need to get ZONE_DEVICE accepted for non-cachable PCI BARs as the first step. >From there is pretty clear we the DMA API needs to be updated to support that use and work can be done to solve the various problems there on the basis of using ZONE_DEVICE pages to figure out to the PCI-E end points Jason -- 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 v4 06/10] [media] st-delta: add memory allocator helper functions
Helper functions used by decoder back-ends to allocate
physically contiguous memory required by hardware video
decoder.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/sti/delta/Makefile| 2 +-
drivers/media/platform/sti/delta/delta-mem.c | 51
drivers/media/platform/sti/delta/delta-mem.h | 14
drivers/media/platform/sti/delta/delta.h | 8 +
4 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/platform/sti/delta/delta-mem.c
create mode 100644 drivers/media/platform/sti/delta/delta-mem.h
diff --git a/drivers/media/platform/sti/delta/Makefile
b/drivers/media/platform/sti/delta/Makefile
index 07ba7ad..cbfb1b5 100644
--- a/drivers/media/platform/sti/delta/Makefile
+++ b/drivers/media/platform/sti/delta/Makefile
@@ -1,2 +1,2 @@
obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o
-st-delta-y := delta-v4l2.o
+st-delta-y := delta-v4l2.o delta-mem.o
diff --git a/drivers/media/platform/sti/delta/delta-mem.c
b/drivers/media/platform/sti/delta/delta-mem.c
new file mode 100644
index 000..d7b53d3
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-mem.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Author: Hugues Fruchet for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include "delta.h"
+#include "delta-mem.h"
+
+int hw_alloc(struct delta_ctx *ctx, u32 size, const char *name,
+struct delta_buf *buf)
+{
+ struct delta_dev *delta = ctx->dev;
+ dma_addr_t dma_addr;
+ void *addr;
+ unsigned long attrs = DMA_ATTR_WRITE_COMBINE;
+
+ addr = dma_alloc_attrs(delta->dev, size, &dma_addr,
+ GFP_KERNEL | __GFP_NOWARN, attrs);
+ if (!addr) {
+ dev_err(delta->dev,
+ "%s hw_alloc:dma_alloc_coherent failed for %s
(size=%d)\n",
+ ctx->name, name, size);
+ ctx->sys_errors++;
+ return -ENOMEM;
+ }
+
+ buf->size = size;
+ buf->paddr = dma_addr;
+ buf->vaddr = addr;
+ buf->name = name;
+ buf->attrs = attrs;
+
+ dev_dbg(delta->dev,
+ "%s allocate %d bytes of HW memory @(virt=0x%p, phy=0x%pad):
%s\n",
+ ctx->name, size, buf->vaddr, &buf->paddr, buf->name);
+
+ return 0;
+}
+
+void hw_free(struct delta_ctx *ctx, struct delta_buf *buf)
+{
+ struct delta_dev *delta = ctx->dev;
+
+ dev_dbg(delta->dev,
+ "%s free %d bytes of HW memory @(virt=0x%p, phy=0x%pad):
%s\n",
+ ctx->name, buf->size, buf->vaddr, &buf->paddr, buf->name);
+
+ dma_free_attrs(delta->dev, buf->size,
+ buf->vaddr, buf->paddr, buf->attrs);
+}
diff --git a/drivers/media/platform/sti/delta/delta-mem.h
b/drivers/media/platform/sti/delta/delta-mem.h
new file mode 100644
index 000..f8ca109
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-mem.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Author: Hugues Fruchet for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#ifndef DELTA_MEM_H
+#define DELTA_MEM_H
+
+int hw_alloc(struct delta_ctx *ctx, u32 size, const char *name,
+struct delta_buf *buf);
+void hw_free(struct delta_ctx *ctx, struct delta_buf *buf);
+
+#endif /* DELTA_MEM_H */
diff --git a/drivers/media/platform/sti/delta/delta.h
b/drivers/media/platform/sti/delta/delta.h
index 74a4240..9e26525 100644
--- a/drivers/media/platform/sti/delta/delta.h
+++ b/drivers/media/platform/sti/delta/delta.h
@@ -191,6 +191,14 @@ struct delta_dts {
u64 val;
};
+struct delta_buf {
+ u32 size;
+ void *vaddr;
+ dma_addr_t paddr;
+ const char *name;
+ unsigned long attrs;
+};
+
struct delta_ctx;
/*
--
1.9.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 v4 09/10] [media] st-delta: add mjpeg support
Adds support of DELTA MJPEG video decoder back-end,
implemented by calling JPEG_DECODER_HW0 firmware
using RPMSG IPC communication layer.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/Kconfig | 6 +
drivers/media/platform/sti/delta/Makefile | 4 +
drivers/media/platform/sti/delta/delta-cfg.h | 3 +
drivers/media/platform/sti/delta/delta-mjpeg-dec.c | 454 +
drivers/media/platform/sti/delta/delta-mjpeg-fw.h | 221 ++
drivers/media/platform/sti/delta/delta-mjpeg-hdr.c | 150 +++
drivers/media/platform/sti/delta/delta-mjpeg.h | 35 ++
drivers/media/platform/sti/delta/delta-v4l2.c | 3 +
8 files changed, 876 insertions(+)
create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-dec.c
create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-fw.h
create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg-hdr.c
create mode 100644 drivers/media/platform/sti/delta/delta-mjpeg.h
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 09e2797..b60de06 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -317,6 +317,12 @@ config VIDEO_STI_DELTA
if VIDEO_STI_DELTA
+config VIDEO_STI_DELTA_MJPEG
+ bool "STMicroelectronics DELTA MJPEG support"
+ default y
+ help
+ Enables DELTA MJPEG hardware support.
+
endif # VIDEO_STI_DELTA
config VIDEO_SH_VEU
diff --git a/drivers/media/platform/sti/delta/Makefile
b/drivers/media/platform/sti/delta/Makefile
index e47e0bd..663be70 100644
--- a/drivers/media/platform/sti/delta/Makefile
+++ b/drivers/media/platform/sti/delta/Makefile
@@ -1,2 +1,6 @@
obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o
st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o
+
+# MJPEG support
+st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-hdr.o
+st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-dec.o
diff --git a/drivers/media/platform/sti/delta/delta-cfg.h
b/drivers/media/platform/sti/delta/delta-cfg.h
index 0122a49..fa89064 100644
--- a/drivers/media/platform/sti/delta/delta-cfg.h
+++ b/drivers/media/platform/sti/delta/delta-cfg.h
@@ -56,5 +56,8 @@
#define DELTA_HW_AUTOSUSPEND_DELAY_MS 5
#define DELTA_MAX_DECODERS 10
+#ifdef CONFIG_VIDEO_STI_DELTA_MJPEG
+extern const struct delta_dec mjpegdec;
+#endif
#endif /* DELTA_CFG_H */
diff --git a/drivers/media/platform/sti/delta/delta-mjpeg-dec.c
b/drivers/media/platform/sti/delta/delta-mjpeg-dec.c
new file mode 100644
index 000..c6c421a
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-mjpeg-dec.c
@@ -0,0 +1,454 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2013
+ * Author: Hugues Fruchet for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+
+#include "delta.h"
+#include "delta-ipc.h"
+#include "delta-mjpeg.h"
+#include "delta-mjpeg-fw.h"
+
+#define DELTA_MJPEG_MAX_RESO DELTA_MAX_RESO
+
+struct delta_mjpeg_ctx {
+ /* jpeg header */
+ struct mjpeg_header header_struct;
+ struct mjpeg_header *header;
+
+ /* ipc */
+ void *ipc_hdl;
+ struct delta_buf *ipc_buf;
+
+ /* decoded output frame */
+ struct delta_frame *out_frame;
+
+ unsigned char str[3000];
+};
+
+#define to_ctx(ctx) ((struct delta_mjpeg_ctx *)(ctx)->priv)
+
+static char *ipc_open_param_str(struct jpeg_video_decode_init_params_t *p,
+ char *str, unsigned int len)
+{
+ char *b = str;
+
+ if (!p)
+ return "";
+
+ b += snprintf(b, len,
+ "jpeg_video_decode_init_params_t\n"
+ "circular_buffer_begin_addr_p 0x%x\n"
+ "circular_buffer_end_addr_p 0x%x\n",
+ p->circular_buffer_begin_addr_p,
+ p->circular_buffer_end_addr_p);
+
+ return str;
+}
+
+static char *ipc_decode_param_str(struct jpeg_decode_params_t *p,
+ char *str, unsigned int len)
+{
+ char *b = str;
+
+ if (!p)
+ return "";
+
+ b += snprintf(b, len,
+ "jpeg_decode_params_t\n"
+ "picture_start_addr_p 0x%x\n"
+ "picture_end_addr_p0x%x\n"
+ "decoding_mode%d\n"
+ "display_buffer_addr.display_decimated_luma_p 0x%x\n"
+ "display_buffer_addr.display_decimated_chroma_p 0x%x\n"
+ "main_aux_enable %d\n"
+ "additional_flags 0x%x\n"
+ "field_flag %x\n"
+ "is_jpeg_image%x\n",
+ p->picture_start_addr_p,
+ p->picture_end_addr_p,
+ p->decoding_mode,
+ p->dis
[PATCH v4 10/10] [media] st-delta: debug: trace stream/frame information & summary
Adds some trace points showing input compressed stream or
output decoded frame information.
Adds an unconditional trace point when streaming starts showing
the compressed stream and the decoded frame information.
Adds an unconditional trace point at instance closure summarizing
into a single line the decoding process (stream information, decoded
and output frames number, potential errors observed).
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/sti/delta/Makefile | 2 +-
drivers/media/platform/sti/delta/delta-debug.c | 72 ++
drivers/media/platform/sti/delta/delta-debug.h | 18 +++
drivers/media/platform/sti/delta/delta-v4l2.c | 29 +--
4 files changed, 116 insertions(+), 5 deletions(-)
create mode 100644 drivers/media/platform/sti/delta/delta-debug.c
create mode 100644 drivers/media/platform/sti/delta/delta-debug.h
diff --git a/drivers/media/platform/sti/delta/Makefile
b/drivers/media/platform/sti/delta/Makefile
index 663be70..f95580e 100644
--- a/drivers/media/platform/sti/delta/Makefile
+++ b/drivers/media/platform/sti/delta/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o
-st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o
+st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o delta-debug.o
# MJPEG support
st-delta-$(CONFIG_VIDEO_STI_DELTA_MJPEG) += delta-mjpeg-hdr.o
diff --git a/drivers/media/platform/sti/delta/delta-debug.c
b/drivers/media/platform/sti/delta/delta-debug.c
new file mode 100644
index 000..f1bc64e
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-debug.c
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Authors: Hugues Fruchet
+ * Fabrice Lecoultre
+ * for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include "delta.h"
+#include "delta-debug.h"
+
+char *delta_streaminfo_str(struct delta_streaminfo *s, char *str,
+ unsigned int len)
+{
+ if (!s)
+ return NULL;
+
+ snprintf(str, len,
+"%4.4s %dx%d %s %s dpb=%d %s %s %s%dx%d@(%d,%d) %s%d/%d",
+(char *)&s->streamformat, s->width, s->height,
+s->profile, s->level, s->dpb,
+(s->field == V4L2_FIELD_NONE) ? "progressive" : "interlaced",
+s->other,
+s->flags & DELTA_STREAMINFO_FLAG_CROP ? "crop=" : "",
+s->crop.width, s->crop.height,
+s->crop.left, s->crop.top,
+s->flags & DELTA_STREAMINFO_FLAG_PIXELASPECT ? "par=" : "",
+s->pixelaspect.numerator,
+s->pixelaspect.denominator);
+
+ return str;
+}
+
+char *delta_frameinfo_str(struct delta_frameinfo *f, char *str,
+ unsigned int len)
+{
+ if (!f)
+ return NULL;
+
+ snprintf(str, len,
+"%4.4s %dx%d aligned %dx%d %s %s%dx%d@(%d,%d) %s%d/%d",
+(char *)&f->pixelformat, f->width, f->height,
+f->aligned_width, f->aligned_height,
+(f->field == V4L2_FIELD_NONE) ? "progressive" : "interlaced",
+f->flags & DELTA_STREAMINFO_FLAG_CROP ? "crop=" : "",
+f->crop.width, f->crop.height,
+f->crop.left, f->crop.top,
+f->flags & DELTA_STREAMINFO_FLAG_PIXELASPECT ? "par=" : "",
+f->pixelaspect.numerator,
+f->pixelaspect.denominator);
+
+ return str;
+}
+
+void delta_trace_summary(struct delta_ctx *ctx)
+{
+ struct delta_dev *delta = ctx->dev;
+ struct delta_streaminfo *s = &ctx->streaminfo;
+ unsigned char str[100] = "";
+
+ if (!(ctx->flags & DELTA_FLAG_STREAMINFO))
+ return;
+
+ dev_info(delta->dev, "%s %s, %d frames decoded, %d frames output, %d
frames dropped, %d stream errors, %d decode errors",
+ctx->name,
+delta_streaminfo_str(s, str, sizeof(str)),
+ctx->decoded_frames,
+ctx->output_frames,
+ctx->dropped_frames,
+ctx->stream_errors,
+ctx->decode_errors);
+}
diff --git a/drivers/media/platform/sti/delta/delta-debug.h
b/drivers/media/platform/sti/delta/delta-debug.h
new file mode 100644
index 000..955c158
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-debug.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Authors: Hugues Fruchet
+ * Fabrice Lecoultre
+ * for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#ifndef DELTA_DEBUG_H
+#define DELTA_DEBUG_H
+
+char *delta_streaminfo_str(struct delta_streaminfo *s, char *str,
+ unsigned int len);
+char *delta_frameinfo_str(struct delta_frameinfo *f, char *str,
+ unsigned int len);
+void delta_trace_summary(struct delta_ctx *ctx);
+
+#endif /* DELTA_
[PATCH v4 07/10] [media] st-delta: rpmsg ipc support
IPC (Inter Process Communication) support for communication with
DELTA coprocessor firmware using rpmsg kernel framework.
Based on 4 services open/set_stream/decode/close and their associated
rpmsg messages.
The messages structures are duplicated on both host and firmware
side and are packed (use only of 32 bits size fields in messages
structures to ensure packing).
Each service is synchronous; service returns only when firmware
acknowledges the associated command message.
Due to significant parameters size exchanged from host to copro,
parameters are not inserted in rpmsg messages. Instead, parameters are
stored in physical memory shared between host and coprocessor.
Memory is non-cacheable, so no special operation is required
to ensure memory coherency on host and on coprocessor side.
Multi-instance support and re-entrance are ensured using host_hdl and
copro_hdl in message header exchanged between both host and coprocessor.
This avoids to manage tables on both sides to get back the running context
of each instance.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/Kconfig| 1 +
drivers/media/platform/sti/delta/Makefile | 2 +-
drivers/media/platform/sti/delta/delta-ipc.c | 591 ++
drivers/media/platform/sti/delta/delta-ipc.h | 76
drivers/media/platform/sti/delta/delta-v4l2.c | 11 +
drivers/media/platform/sti/delta/delta.h | 21 +
6 files changed, 701 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/platform/sti/delta/delta-ipc.c
create mode 100644 drivers/media/platform/sti/delta/delta-ipc.h
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f494f01..09e2797 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -305,6 +305,7 @@ config VIDEO_STI_DELTA
depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
+ select RPMSG
help
This V4L2 driver enables DELTA multi-format video decoder
of STMicroelectronics STiH4xx SoC series allowing hardware
diff --git a/drivers/media/platform/sti/delta/Makefile
b/drivers/media/platform/sti/delta/Makefile
index cbfb1b5..e47e0bd 100644
--- a/drivers/media/platform/sti/delta/Makefile
+++ b/drivers/media/platform/sti/delta/Makefile
@@ -1,2 +1,2 @@
obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o
-st-delta-y := delta-v4l2.o delta-mem.o
+st-delta-y := delta-v4l2.o delta-mem.o delta-ipc.o
diff --git a/drivers/media/platform/sti/delta/delta-ipc.c
b/drivers/media/platform/sti/delta/delta-ipc.c
new file mode 100644
index 000..a48fe9d
--- /dev/null
+++ b/drivers/media/platform/sti/delta/delta-ipc.c
@@ -0,0 +1,591 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Author: Hugues Fruchet for STMicroelectronics.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+
+#include "delta.h"
+#include "delta-ipc.h"
+#include "delta-mem.h"
+
+#define IPC_TIMEOUT 100
+#define IPC_SANITY_TAG 0xDEADBEEF
+
+enum delta_ipc_fw_command {
+ DELTA_IPC_OPEN,
+ DELTA_IPC_SET_STREAM,
+ DELTA_IPC_DECODE,
+ DELTA_IPC_CLOSE
+};
+
+#define to_rpmsg_driver(__drv) container_of(__drv, struct rpmsg_driver, drv)
+#define to_delta(__d) container_of(__d, struct delta_dev, rpmsg_driver)
+
+#define to_ctx(hdl) ((struct delta_ipc_ctx *)hdl)
+#define to_pctx(ctx) container_of(ctx, struct delta_ctx, ipc_ctx)
+
+struct delta_ipc_header_msg {
+ u32 tag;
+ void *host_hdl;
+ u32 copro_hdl;
+ u32 command;
+};
+
+#define to_host_hdl(ctx) ((void *)ctx)
+
+#define msg_to_ctx(msg) ((struct delta_ipc_ctx *)(msg)->header.host_hdl)
+#define msg_to_copro_hdl(msg) ((msg)->header.copro_hdl)
+
+static inline dma_addr_t to_paddr(struct delta_ipc_ctx *ctx, void *vaddr)
+{
+ return (ctx->ipc_buf->paddr + (vaddr - ctx->ipc_buf->vaddr));
+}
+
+static inline bool is_valid_data(struct delta_ipc_ctx *ctx,
+void *data, u32 size)
+{
+ return ((data >= ctx->ipc_buf->vaddr) &&
+ ((data + size) <= (ctx->ipc_buf->vaddr + ctx->ipc_buf->size)));
+}
+
+/*
+ * IPC shared memory (@ipc_buf_size, @ipc_buf_paddr) is sent to copro
+ * at each instance opening. This memory is allocated by IPC client
+ * and given through delta_ipc_open(). All messages parameters
+ * (open, set_stream, decode) will have their phy address within
+ * this IPC shared memory, avoiding de-facto recopies inside delta-ipc.
+ * All the below messages structures are used on both host and firmware
+ * side and are packed (use only of 32 bits size fields in messages
+ * structures to ensure packing):
+ * - struct delta_ipc_open_msg
+ * - struct delta_ipc_set_stream_msg
+ * - struct delta_ipc_decode_msg
+ * - struct delta_ipc_close_msg
+ * - struct delta_ipc_cb_msg
+ */
+struct delta_ipc_open_msg {
+ struct delta_ipc_header_msg header;
+ u32 ipc_buf_size;
+ dma_addr_t ipc_buf_paddr;
+ char
[PATCH v4 08/10] [media] st-delta: EOS (End Of Stream) support
EOS (End Of Stream) support allows user to get
all the potential decoded frames remaining in decoder
pipeline after having reached the end of video bitstream.
To do so, user calls VIDIOC_DECODER_CMD(V4L2_DEC_CMD_STOP)
which will drain the decoder and get the drained frames
that are then returned to user.
User is informed of EOS completion in two ways:
- dequeue of an empty frame flagged to V4L2_BUF_FLAG_LAST
- reception of a V4L2_EVENT_EOS event.
If, unfortunately, no buffer is available on CAPTURE queue
to return the empty frame, EOS is delayed till user queue
one CAPTURE buffer.
Signed-off-by: Hugues Fruchet
---
drivers/media/platform/sti/delta/delta-v4l2.c | 144 +-
drivers/media/platform/sti/delta/delta.h | 23
2 files changed, 166 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c
b/drivers/media/platform/sti/delta/delta-v4l2.c
index 7228565..9e2d2955 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -106,7 +106,8 @@ static void delta_frame_done(struct delta_ctx *ctx, struct
delta_frame *frame,
vbuf->sequence = ctx->frame_num++;
v4l2_m2m_buf_done(vbuf, err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
- ctx->output_frames++;
+ if (frame->info.size) /* ignore EOS */
+ ctx->output_frames++;
}
static void requeue_free_frames(struct delta_ctx *ctx)
@@ -757,6 +758,133 @@ static int delta_g_selection(struct file *file, void *fh,
return 0;
}
+static void delta_complete_eos(struct delta_ctx *ctx,
+ struct delta_frame *frame)
+{
+ struct delta_dev *delta = ctx->dev;
+ const struct v4l2_event ev = {.type = V4L2_EVENT_EOS};
+
+ /* Send EOS to user:
+* - by returning an empty frame flagged to V4L2_BUF_FLAG_LAST
+* - and then send EOS event
+*/
+
+ /* empty frame */
+ frame->info.size = 0;
+
+ /* set the last buffer flag */
+ frame->flags |= V4L2_BUF_FLAG_LAST;
+
+ /* release frame to user */
+ delta_frame_done(ctx, frame, 0);
+
+ /* send EOS event */
+ v4l2_event_queue_fh(&ctx->fh, &ev);
+
+ dev_dbg(delta->dev, "%s EOS completed\n", ctx->name);
+}
+
+static int delta_try_decoder_cmd(struct file *file, void *fh,
+struct v4l2_decoder_cmd *cmd)
+{
+ if (cmd->cmd != V4L2_DEC_CMD_STOP)
+ return -EINVAL;
+
+ if (cmd->flags & V4L2_DEC_CMD_STOP_TO_BLACK)
+ return -EINVAL;
+
+ if (!(cmd->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY) &&
+ (cmd->stop.pts != 0))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int delta_decoder_stop_cmd(struct delta_ctx *ctx, void *fh)
+{
+ const struct delta_dec *dec = ctx->dec;
+ struct delta_dev *delta = ctx->dev;
+ struct delta_frame *frame = NULL;
+ int ret = 0;
+
+ dev_dbg(delta->dev, "%s EOS received\n", ctx->name);
+
+ if (ctx->state != DELTA_STATE_READY)
+ return 0;
+
+ /* drain the decoder */
+ call_dec_op(dec, drain, ctx);
+
+ /* release to user drained frames */
+ while (1) {
+ frame = NULL;
+ ret = call_dec_op(dec, get_frame, ctx, &frame);
+ if (ret == -ENODATA) {
+ /* no more decoded frames */
+ break;
+ }
+ if (frame) {
+ dev_dbg(delta->dev, "%s drain frame[%d]\n",
+ ctx->name, frame->index);
+
+ /* pop timestamp and mark frame with it */
+ delta_pop_dts(ctx, &frame->dts);
+
+ /* release decoded frame to user */
+ delta_frame_done(ctx, frame, 0);
+ }
+ }
+
+ /* try to complete EOS */
+ ret = delta_get_free_frame(ctx, &frame);
+ if (ret)
+ goto delay_eos;
+
+ /* new frame available, EOS can now be completed */
+ delta_complete_eos(ctx, frame);
+
+ ctx->state = DELTA_STATE_EOS;
+
+ return 0;
+
+delay_eos:
+ /* EOS completion from driver is delayed because
+* we don't have a free empty frame available.
+* EOS completion is so delayed till next frame_queue() call
+* to be sure to have a free empty frame available.
+*/
+ ctx->state = DELTA_STATE_WF_EOS;
+ dev_dbg(delta->dev, "%s EOS delayed\n", ctx->name);
+
+ return 0;
+}
+
+static int delta_decoder_cmd(struct file *file, void *fh,
+struct v4l2_decoder_cmd *cmd)
+{
+ struct delta_ctx *ctx = to_ctx(fh);
+ int ret = 0;
+
+ ret = delta_try_decoder_cmd(file, fh, cmd);
+ if (ret)
+ return ret;
+
+ return delta_decoder_stop_cmd(ctx, fh);
+}
+
+static int delta_subscribe_event(struct v4l2_fh *fh,
+
[PATCH v4 04/10] [media] MAINTAINERS: add st-delta driver
Add entry for the STMicroelectronics DELTA driver. Signed-off-by: Hugues Fruchet --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7db3f7a..a96dd22 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2394,6 +2394,14 @@ W: https://linuxtv.org S: Supported F: drivers/media/platform/sti/bdisp +DELTA ST MEDIA DRIVER +M: Hugues Fruchet +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +W: https://linuxtv.org +S: Supported +F: drivers/media/platform/sti/delta + BEFS FILE SYSTEM M: Luis de Bethencourt M: Salah Triki -- 1.9.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 v4 03/10] ARM: multi_v7_defconfig: enable STMicroelectronics DELTA Support
Enables support of STMicroelectronics STiH4xx SoC series DELTA multi-format video decoder V4L2 driver. Signed-off-by: Hugues Fruchet --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 11f37ed..8500f75 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -562,6 +562,7 @@ CONFIG_V4L_MEM2MEM_DRIVERS=y CONFIG_VIDEO_SAMSUNG_S5P_JPEG=m CONFIG_VIDEO_SAMSUNG_S5P_MFC=m CONFIG_VIDEO_STI_BDISP=m +CONFIG_VIDEO_STI_DELTA=m CONFIG_VIDEO_RENESAS_JPU=m CONFIG_VIDEO_RENESAS_VSP1=m CONFIG_V4L_TEST_DRIVERS=y -- 1.9.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 v4 05/10] [media] st-delta: STiH4xx multi-format video decoder v4l2 driver
This V4L2 driver enables DELTA multi-format video decoder of STMicroelectronics STiH4xx SoC series. Signed-off-by: Hugues Fruchet --- drivers/media/platform/Kconfig| 20 + drivers/media/platform/Makefile |2 + drivers/media/platform/sti/delta/Makefile |2 + drivers/media/platform/sti/delta/delta-cfg.h | 60 + drivers/media/platform/sti/delta/delta-v4l2.c | 1800 + drivers/media/platform/sti/delta/delta.h | 514 +++ 6 files changed, 2398 insertions(+) create mode 100644 drivers/media/platform/sti/delta/Makefile create mode 100644 drivers/media/platform/sti/delta/delta-cfg.h create mode 100644 drivers/media/platform/sti/delta/delta-v4l2.c create mode 100644 drivers/media/platform/sti/delta/delta.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 3c5a0b6..f494f01 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -298,6 +298,26 @@ config VIDEO_STI_HVA To compile this driver as a module, choose M here: the module will be called st-hva. +config VIDEO_STI_DELTA + tristate "STMicroelectronics STiH4xx DELTA multi-format video decoder V4L2 driver" + depends on VIDEO_DEV && VIDEO_V4L2 + depends on ARCH_STI || COMPILE_TEST + depends on HAS_DMA + select VIDEOBUF2_DMA_CONTIG + select V4L2_MEM2MEM_DEV + help + This V4L2 driver enables DELTA multi-format video decoder + of STMicroelectronics STiH4xx SoC series allowing hardware + decoding of various compressed video bitstream format in + raw uncompressed format. + + To compile this driver as a module, choose M here: + the module will be called st-delta. + +if VIDEO_STI_DELTA + +endif # VIDEO_STI_DELTA + config VIDEO_SH_VEU tristate "SuperH VEU mem2mem video processing driver" depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 5b3cb27..349ddf6 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -39,6 +39,8 @@ obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/ obj-$(CONFIG_VIDEO_STI_HVA)+= sti/hva/ obj-$(CONFIG_DVB_C8SECTPFE)+= sti/c8sectpfe/ +obj-$(CONFIG_VIDEO_STI_DELTA) += sti/delta/ + obj-$(CONFIG_BLACKFIN) += blackfin/ obj-$(CONFIG_ARCH_DAVINCI) += davinci/ diff --git a/drivers/media/platform/sti/delta/Makefile b/drivers/media/platform/sti/delta/Makefile new file mode 100644 index 000..07ba7ad --- /dev/null +++ b/drivers/media/platform/sti/delta/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_VIDEO_STI_DELTA) := st-delta.o +st-delta-y := delta-v4l2.o diff --git a/drivers/media/platform/sti/delta/delta-cfg.h b/drivers/media/platform/sti/delta/delta-cfg.h new file mode 100644 index 000..0122a49 --- /dev/null +++ b/drivers/media/platform/sti/delta/delta-cfg.h @@ -0,0 +1,60 @@ +/* + * Copyright (C) STMicroelectronics SA 2015 + * Author: Hugues Fruchet for STMicroelectronics. + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef DELTA_CFG_H +#define DELTA_CFG_H + +#define DELTA_FW_VERSION "21.1-3" + +#define DELTA_MIN_WIDTH 32 +#define DELTA_MAX_WIDTH 4096 +#define DELTA_MIN_HEIGHT 32 +#define DELTA_MAX_HEIGHT 2400 + +/* DELTA requires a 32x32 pixels alignment for frames */ +#define DELTA_WIDTH_ALIGNMENT32 +#define DELTA_HEIGHT_ALIGNMENT 32 + +#define DELTA_DEFAULT_WIDTH DELTA_MIN_WIDTH +#define DELTA_DEFAULT_HEIGHT DELTA_MIN_HEIGHT +#define DELTA_DEFAULT_FRAMEFORMAT V4L2_PIX_FMT_NV12 +#define DELTA_DEFAULT_STREAMFORMAT V4L2_PIX_FMT_MJPEG + +#define DELTA_MAX_RESO (DELTA_MAX_WIDTH * DELTA_MAX_HEIGHT) + +/* guard value for number of access units */ +#define DELTA_MAX_AUS 10 + +/* IP perf dependent, can be tuned */ +#define DELTA_PEAK_FRAME_SMOOTHING 2 + +/* guard output frame count: + * - at least 1 frame needed for display + * - at worst 21 + * ( max h264 dpb (16) + + * decoding peak smoothing (2) + + * user display pipeline (3) ) + */ +#define DELTA_MIN_FRAME_USER1 +#define DELTA_MAX_DPB 16 +#define DELTA_MAX_FRAME_USER3 /* platform/use-case dependent */ +#define DELTA_MAX_FRAMES (DELTA_MAX_DPB + DELTA_PEAK_FRAME_SMOOTHING +\ + DELTA_MAX_FRAME_USER) + +#if DELTA_MAX_FRAMES > VIDEO_MAX_FRAME +#undef DELTA_MAX_FRAMES +#define DELTA_MAX_FRAMES (VIDEO_MAX_FRAME) +#endif + +/* extra space to be allocated to store codec specific data per frame */ +#define DELTA_MAX_FRAME_PRIV_SIZE 100 + +/* PM runtime auto power-off after 5ms of inactivity */ +#define DELTA_HW_AUTOSUSPEND_DELAY_MS 5 + +#define DELTA_MAX_DECODERS 10 + +#endif /* DELTA_CFG_H */ diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c b/drivers/media/platform/sti/delta/delta-v4l2.c new file mode 100644 index 000..cf35563
[PATCH v4 01/10] Documentation: DT: add bindings for ST DELTA
This patch adds DT binding documentation for STMicroelectronics
DELTA V4L2 video decoder.
Signed-off-by: Hugues Fruchet
---
Documentation/devicetree/bindings/media/st,st-delta.txt | 17 +
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/st,st-delta.txt
diff --git a/Documentation/devicetree/bindings/media/st,st-delta.txt
b/Documentation/devicetree/bindings/media/st,st-delta.txt
new file mode 100644
index 000..a538ab3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st,st-delta.txt
@@ -0,0 +1,17 @@
+* STMicroelectronics DELTA multi-format video decoder
+
+Required properties:
+- compatible: should be "st,st-delta".
+- clocks: from common clock binding: handle hardware IP needed clocks, the
+ number of clocks may depend on the SoC type.
+ See ../clock/clock-bindings.txt for details.
+- clock-names: names of the clocks listed in clocks property in the same order.
+
+Example:
+ delta0 {
+ compatible = "st,st-delta";
+ clock-names = "delta", "delta-st231", "delta-flash-promip";
+ clocks = <&clk_s_c0_flexgen CLK_VID_DMU>,
+<&clk_s_c0_flexgen CLK_ST231_DMU>,
+<&clk_s_c0_flexgen CLK_FLASH_PROMIP>;
+ };
--
1.9.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 v4 00/10] Add support for DELTA video decoder of STMicroelectronics STiH4xx SoC series
This patchset introduces a basic support for DELTA multi-format video decoder of STMicroelectronics STiH4xx SoC series. DELTA hardware IP is controlled by a remote firmware loaded in a ST231 coprocessor. Communication with firmware is done within an IPC layer using rpmsg kernel framework and a shared memory for messages handling. This driver is compatible with firmware version 21.1-3. While a single firmware is loaded in ST231 coprocessor, it is composed of several firmwares, one per video format family. This DELTA V4L2 driver is designed around files: - delta-v4l2.c : handles V4L2 APIs using M2M framework and calls decoder ops - delta-* : implements decoder calling its associated video firmware (for ex. MJPEG) using IPC layer - delta-ipc.c: IPC layer which handles communication with firmware using rpmsg This first basic support implements only MJPEG hardware acceleration but the driver structure is in place to support all the features of the DELTA video decoder hardware IP. This driver depends on: - ST remoteproc/rpmsg: patchset posted at https://lkml.org/lkml/2016/9/6/77 - ST DELTA firmware: its license is under review. When available, pull request will be done on linux-firmware. === = history = === version 4 - update after v3 review: - "select" RPMSG instead "depends on" - v4l2-compliance S_SELECTION is no more failed till sync on latest v4l2-compliance codebase - sparse warnings fixes version 3 - update after v2 review: - fixed m2m_buf_done missing on start_streaming error case - fixed q->dev missing in queue_init() - removed unsupported s_selection - refactored string namings in delta-debug.c - fixed space before comment - all commits have commit messages - reword memory allocator helper commit version 2 - update after v1 review: - simplified tracing - G_/S_SELECTION reworked to fit COMPOSE(CAPTURE) - fixed m2m_buf_done missing on start_streaming error case - fixed q->dev missing in queue_init() - switch to kernel-4.9 rpmsg API - DELTA support added in multi_v7_defconfig - minor typo fixes & code cleanup version 1: - Initial submission === = v4l2-compliance = === Below is the v4l2-compliance report for the version 4 of the DELTA video decoder driver. v4l2-compliance has been build from SHA1: 003f31e59f353b4aecc82e8fb1c7555964da7efa (v4l2-compliance: allow S_SELECTION to return ENOTTY) root@sti-4:~# v4l2-compliance -d /dev/video0 v4l2-compliance SHA : 003f31e59f353b4aecc82e8fb1c7555964da7efa Driver Info: Driver name : st-delta Card type : st-delta-21.1-3 Bus info : platform:soc:delta0 Driver version: 4.9.0 Capabilities : 0x84208000 Video Memory-to-Memory Streaming Extended Pix Format Device Capabilities Device Caps : 0x04208000 Video Memory-to-Memory Streaming Extended Pix Format Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported) test VIDIOC_QUERYCTRL: OK (Not Supported) test VIDIOC_G/S_CTRL: OK (Not Supported) test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported) test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported) test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 0 Private Controls: 0 Format ioctls: test VIDIOC_
[PATCH v4 02/10] ARM: dts: STiH410: add DELTA dt node
This patch adds DT node for STMicroelectronics
DELTA V4L2 video decoder
Signed-off-by: Hugues Fruchet
---
arch/arm/boot/dts/stih410.dtsi | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index a3ef734..fb03cb67 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -259,5 +259,15 @@
clocks = <&clk_sysin>;
interrupts = ;
};
+ delta0 {
+ compatible = "st,st-delta";
+ clock-names = "delta",
+ "delta-st231",
+ "delta-flash-promip";
+ clocks = <&clk_s_c0_flexgen CLK_VID_DMU>,
+<&clk_s_c0_flexgen CLK_ST231_DMU>,
+<&clk_s_c0_flexgen CLK_FLASH_PROMIP>;
+ };
+
};
};
--
1.9.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: [PATCH] [media] usbtv: add a new usbid
On Mon, 2016-12-05 at 23:47 +0800, Icenowy Zheng wrote: > 2016年12月5日 19:49于 Lubomir Rintel 写道： > > > > On Sun, 2016-12-04 at 22:59 +0800, Icenowy Zheng wrote: > > > > > > 04.12.2016, 22:00, "Icenowy Zheng" : > > > > A new usbid of UTV007 is found in a newly bought device. > > > > > > > > The usbid is 1f71:3301. > > > > > > > > The ID on the chip is: > > > > UTV007 > > > > A89029.1 > > > > 1520L18K1 > > > > > > > > > > Seems that my device come with more capabilities. > > > > > > I tested it under Windows, and I got wireless Analog TV > > > and FM radio functions. (An antenna is shipped with my device) > > > > > > Maybe a new radio function is be added, combined with the > > > new USB ID. > > > > > > But at least Composite AV function works well with current usbtv > > > driver and XawTV. > > > > Well, someone with the hardware would need to capture the traffic > > from > > the Windows driver (and ideally also extend the driver). Would you > > mind > > giving it a try? > > How to do it? > > Use wireshark? Yes, wireshark is okay. I've been using that one. Another good option I discovered recently is usb_capture from usbsniff package. > > Do you have a link to some further details about the device you > > got? > > Perhaps if it's available cheaply from dealextreme or somewhere I > > could > > take a look too. > > I bought directly from Taobao (I'm in China). Do you happen to have a link? Lubo -- 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 3/3] uvcvideo: add a metadata device node
Hi Laurent,
Thanks for the review! I'll work to address your comments. A couple of
clarifications:
On Mon, 5 Dec 2016, Laurent Pinchart wrote:
> Hi Guennadi,
>
> Thank you for the patch.
>
> On Friday 02 Dec 2016 11:53:23 Guennadi Liakhovetski wrote:
> > Some UVC video cameras contain metadata in their payload headers. This
> > patch extracts that data, skipping the standard part of the header, on
> > both bulk and isochronous endpoints and makes it available to the user
> > space on a separate video node, using the V4L2_CAP_META_CAPTURE
> > capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> > though different cameras will have different metadata formats, we use
> > the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> > parse data, based on the specific camera model information.
> >
> > Signed-off-by: Guennadi Liakhovetski
> > ---
> >
> > v2:
> > - updated to the current media/master
> > - removed superfluous META capability from capture nodes
> > - now the complete UVC payload header is copied to buffers, including
> > standard fields
> >
> > Still open for discussion: is this really OK to always create an
> > additional metadata node for each UVC camera or UVC video interface.
[snip]
> > + /*
> > +* Register a metadata node. TODO: shall this only be enabled for some
> > +* cameras?
> > +*/
> > + if (!(dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT))
> > + uvc_meta_register(stream);
> > +
>
> I think so, only for the cameras that can produce metadata.
Every UVC camera produces metadata, but most cameras only have standard
fields there. Whether we should stream standard header fields from the
metadata node will be discussed later. If we do decide to stream standard
header fields, then every USB camera gets metadata nodes. If we decide not
to include standard fields, how do we know whether the camera has any
private fields in headers without streaming from it? Do you want a quirk
for such cameras?
[snip]
> > +static struct vb2_ops uvc_meta_queue_ops = {
> > + .queue_setup = meta_queue_setup,
> > + .buf_prepare = meta_buffer_prepare,
> > + .buf_queue = meta_buffer_queue,
> > + .wait_prepare = vb2_ops_wait_prepare,
> > + .wait_finish = vb2_ops_wait_finish,
> > + .start_streaming = meta_start_streaming,
> > + .stop_streaming = meta_stop_streaming,
> > +};
>
> How about reusing the uvc_queue.c implementation, with a few new checks for
> metadata buffers where needed, instead of duplicating code ? At first sight
> the changes don't seem to be too intrusive (but I might have overlooked
> something).
I thought about that in the beginning and even started implementing it
that way, but at some point it became too inconvenient, so, I switched
over to a separate implementation. I'll think about it more and either
explain, why I didn't want to do that, or unite them.
[snip]
> > +{
> > + size_t nbytes;
> > +
> > + if (!meta_buf || !length)
> > + return;
> > +
> > + nbytes = min_t(unsigned int, length, meta_buf->length);
> > +
> > + meta_buf->buf.sequence = buf->buf.sequence;
> > + meta_buf->buf.field = buf->buf.field;
> > + meta_buf->buf.vb2_buf.timestamp = buf->buf.vb2_buf.timestamp;
> > +
> > + memcpy(meta_buf->mem, mem, nbytes);
>
> Do you need the whole header ? Shouldn't you strip the part already handled
> by
> the driver ?
My original version did that, but we also need the timestamp from the
header. The driver does parse it, but the implementation there has
multiple times been reported as buggy and noone has been able to fix it so
far :) So, I ended up pulling the complete header to the user-space.
Unless time-stamp processing is fixed in the kernel, I don't think we can
frop this.
> > + meta_buf->bytesused = nbytes;
> > + meta_buf->state = UVC_BUF_STATE_READY;
> > +
> > + uvc_queue_next_buffer(&stream->meta.queue, meta_buf);
>
> This essentially means that you'll need one buffer per isochronous packet.
> Given the number of isochronous packets that make a complete image the cost
> seem prohibitive to me. You should instead gather metadata from all headers
> into a single buffer.
Hm, I only worked with cameras, using bulk transfers, so, didn't consider
ISOC implications. Will have to think about this.
[snip]
Thanks
Guennadi
--
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 v3 3/4] stk1160: Add module param for setting the record gain.
On 5 December 2016 at 09:12, Mauro Carvalho Chehab wrote: > Em Sun, 4 Dec 2016 15:25:25 -0300 > Ezequiel Garcia escreveu: > >> On 4 December 2016 at 10:01, Marcel Hasler wrote: >> > Hello >> > >> > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia : >> >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab >> >> wrote: >> >>> Em Sun, 27 Nov 2016 12:11:48 +0100 >> >>> Marcel Hasler escreveu: >> >>> >> Allow setting a custom record gain for the internal AC97 codec (if >> available). This can be >> a value between 0 and 15, 8 is the default and should be suitable for >> most users. The Windows >> driver also sets this to 8 without any possibility for changing it. >> >>> >> >>> The problem of removing the mixer is that you need this kind of >> >>> crap to setup the volumes on a non-standard way. >> >>> >> >> >> >> Right, that's a good point. >> >> >> >>> NACK. >> >>> >> >>> Instead, keep the alsa mixer. The way other drivers do (for example, >> >>> em28xx) is that they configure the mixer when an input is selected, >> >>> increasing the volume of the active audio channel to 100% and muting >> >>> the other audio channels. Yet, as the alsa mixer is exported, users >> >>> can change the mixer settings in runtime using some alsa (or pa) >> >>> mixer application. >> >>> >> >> >> >> Yeah, the AC97 mixer we are currently leveraging >> >> exposes many controls that have no meaning in this device, >> >> so removing that still looks like an improvement. >> >> >> >> I guess the proper way is creating our own mixer >> >> (not using snd_ac97_mixer) exposing only the record >> >> gain knob. >> >> >> >> Marcel, what do you think? >> >> -- >> >> Ezequiel García, VanguardiaSur >> >> www.vanguardiasur.com.ar >> > >> > As I have written before, the recording gain isn't actually meant to >> > be changed by the user. In the official Windows driver this value is >> > hard-coded to 8 and cannot be changed in any way. And there really is >> > no good reason why anyone should need to mess with it in the first >> > place. The default value will give the best results in pretty much all >> > cases and produces approximately the same volume as the internal 8-bit >> > ADC whose gain cannot be changed at all, not even by a driver. >> > >> > I had considered writing a mixer but chose not to. If the gain setting >> > is openly exposed to mixer applications, how do you tell the users >> > that the value set by the driver already is the optimal and >> > recommended value and that they shouldn't mess with the controls >> > unless they really have to? By having a module parameter, this setting >> > is practically hidden from the normal user but still is available to >> > power-users if they think they really need it. In the end it's really >> > just a compromise between hiding it completely and exposing it openly. >> > Also, this way the driver guarantees reproducible results, since >> > there's no need to remember the positions of any volume sliders. >> > >> >> Hm, right. I've never changed the record gain, and it's true that it >> doens't really improve the volume. So, I would be OK with having >> a module parameter. >> >> On the other side, we are exposing it currently, through the "Capture" >> mixer control: >> >> Simple mixer control 'Capture',0 >> Capabilities: cvolume cswitch cswitch-joined >> Capture channels: Front Left - Front Right >> Limits: Capture 0 - 15 >> Front Left: Capture 10 [67%] [15.00dB] [on] >> Front Right: Capture 8 [53%] [12.00dB] [on] >> >> So, it would be user-friendly to keep the user interface and continue >> to expose the same knob - even if the default is the optimal, etc. >> >> To be completely honest, I don't think any user is really relying >> on any REC_GAIN / Capture setting, and I'm completely OK >> with having a mixer control or a module parameter. It doesn't matter. > > If you're positive that *all* stk1160 use the ac97 mixer the > same way, and that there's no sense on having a mixer for it, > then it would be ok to remove it. > Let's remove it then! > In such case, then why you need a modprobe parameter to allow > setting the record level? If this mixer entry is not used, > just set it to zero and be happy with that. > Let's remove the module param too, then. Thanks, -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- 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 08/10] media: camss: Add files which handle the video device nodes
Hi Hans,
On Monday 05 Dec 2016 16:02:55 Hans Verkuil wrote:
> On 12/05/2016 03:45 PM, Laurent Pinchart wrote:
> > On Monday 05 Dec 2016 14:44:55 Hans Verkuil wrote:
> >> On 11/25/2016 03:57 PM, Todor Tomov wrote:
> >>> These files handle the video device nodes of the camss driver.
> >>>
> >>> Signed-off-by: Todor Tomov
> >>> ---
> >>>
> >>> drivers/media/platform/qcom/camss-8x16/video.c | 597 +
> >>> drivers/media/platform/qcom/camss-8x16/video.h | 67 +++
> >>> 2 files changed, 664 insertions(+)
> >>> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
> >>> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h
> >
> > [snip]
> >
> >>> +int msm_video_register(struct camss_video *video, struct v4l2_device
> >>> *v4l2_dev,
> >>> +const char *name)
> >>> +{
> >>> + struct media_pad *pad = &video->pad;
> >>> + struct video_device *vdev;
> >>> + struct vb2_queue *q;
> >>> + int ret;
> >>> +
> >>> + vdev = video_device_alloc();
> >>> + if (vdev == NULL) {
> >>> + dev_err(v4l2_dev->dev, "Failed to allocate video
> >>> device\n");
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + video->vdev = vdev;
> >>> +
> >>> + q = &video->vb2_q;
> >>> + q->drv_priv = video;
> >>> + q->mem_ops = &vb2_dma_contig_memops;
> >>> + q->ops = &msm_video_vb2_q_ops;
> >>> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >>> + q->io_modes = VB2_MMAP;
> >>
> >> Add modes VB2_DMABUF and VB2_READ. These are for free, so why not?
> >> Especially DMABUF is of course very desirable to have.
> >
> > I certainly agree with VB2_DMABUF, but I wouldn't expose VB2_READ. read()
> > for this kind of device is inefficient and we should encourage userspace
> > application to move away from it (and certainly make it very clear that
> > new applications should not use read() with this device).
>
> VB2_READ is very nice when debugging (no need to program a stream I/O
> application first)
There's at least v4l2-ctl and yavta that can (and should) be used for
debugging ;-)
> and useful when you want to pipe the video.
Except that you lose frame boundaries. It's really a poor man's solution that
should never be used in any "real" application. I'd rather add an option to
v4l2-ctl to output the video stream to stdout (and/or stderr).
> Nobody uses read() in 'regular' applications since it is obviously
> inefficient, but I don't see that as a reason not to offer VB2_READ.
>
> I don't think this is something anyone will ever abuse,
Famous last words ;-)
> and it is a nice feature to have (and for free as well).
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] media: camss: Add files which handle the video device nodes
Hi Todor,
Thank you for the patch.
On Friday 25 Nov 2016 16:57:20 Todor Tomov wrote:
> These files handle the video device nodes of the camss driver.
camss is a quite generic, I'm a bit concerned about claiming that acronym in
the global kernel namespace. Would it be too long if we prefixed symbols with
msm_camss instead ?
> Signed-off-by: Todor Tomov
> ---
> drivers/media/platform/qcom/camss-8x16/video.c | 597 ++
> drivers/media/platform/qcom/camss-8x16/video.h | 67 +++
> 2 files changed, 664 insertions(+)
> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h
>
> diff --git a/drivers/media/platform/qcom/camss-8x16/video.c
> b/drivers/media/platform/qcom/camss-8x16/video.c new file mode 100644
> index 000..0bf8ea9
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss-8x16/video.c
> @@ -0,0 +1,597 @@
> +/*
> + * video.c
> + *
> + * Qualcomm MSM Camera Subsystem - V4L2 device node
> + *
> + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2015-2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "video.h"
> +#include "camss.h"
> +
> +/*
> + * struct format_info - ISP media bus format information
> + * @code: V4L2 media bus format code
> + * @pixelformat: V4L2 pixel format FCC identifier
> + * @bpp: Bits per pixel when stored in memory
> + */
> +static const struct format_info {
> + u32 code;
> + u32 pixelformat;
> + unsigned int bpp;
> +} formats[] = {
> + { MEDIA_BUS_FMT_UYVY8_2X8, V4L2_PIX_FMT_UYVY, 16 },
> + { MEDIA_BUS_FMT_VYUY8_2X8, V4L2_PIX_FMT_VYUY, 16 },
> + { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_PIX_FMT_YUYV, 16 },
> + { MEDIA_BUS_FMT_YVYU8_2X8, V4L2_PIX_FMT_YVYU, 16 },
> + { MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8, 8 },
> + { MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8, 8 },
> + { MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8, 8 },
> + { MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8, 8 },
> + { MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_PIX_FMT_SBGGR10P, 10 },
> + { MEDIA_BUS_FMT_SGBRG10_1X10, V4L2_PIX_FMT_SGBRG10P, 10 },
> + { MEDIA_BUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10P, 10 },
> + { MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_PIX_FMT_SRGGB10P, 10 },
> + { MEDIA_BUS_FMT_SBGGR12_1X12, V4L2_PIX_FMT_SRGGB12P, 12 },
> + { MEDIA_BUS_FMT_SGBRG12_1X12, V4L2_PIX_FMT_SGBRG12P, 12 },
> + { MEDIA_BUS_FMT_SGRBG12_1X12, V4L2_PIX_FMT_SGRBG12P, 12 },
> + { MEDIA_BUS_FMT_SRGGB12_1X12, V4L2_PIX_FMT_SRGGB12P, 12 }
> +};
> +
> +/*
> ---
> -- + * Helper functions
> + */
> +
> +/*
> + * video_mbus_to_pix - Convert v4l2_mbus_framefmt to v4l2_pix_format
> + * @mbus: v4l2_mbus_framefmt format (input)
> + * @pix: v4l2_pix_format format (output)
> + *
> + * Fill the output pix structure with information from the input mbus
> format.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static unsigned int video_mbus_to_pix(const struct v4l2_mbus_framefmt
> *mbus,
> + struct v4l2_pix_format *pix)
> +{
> + unsigned int i;
> +
> + memset(pix, 0, sizeof(*pix));
> + pix->width = mbus->width;
> + pix->height = mbus->height;
> +
> + for (i = 0; i < ARRAY_SIZE(formats); ++i) {
> + if (formats[i].code == mbus->code)
> + break;
> + }
> +
> + if (WARN_ON(i == ARRAY_SIZE(formats)))
> + return -EINVAL;
> +
> + pix->pixelformat = formats[i].pixelformat;
> + pix->bytesperline = pix->width * formats[i].bpp / 8;
> + pix->bytesperline = ALIGN(pix->bytesperline, 8);
Does the hardware support padding at the end of lines ? If so it would be
useful to use the maximum of the computed and requested by userspace values
(up to the maximum size of the padding supported by the hardware).
> + pix->sizeimage = pix->bytesperline * pix->height;
Similarly, should we support padding at the end of the image ?
> + pix->colorspace = mbus->colorspace;
> + pix->field = mbus->field;
> +
> + return 0;
> +}
> +
> +static struct v4l2_subdev *video_remote_subdev(struct camss_video *video,
> +u32 *pad)
> +{
> + struct media_pad *remote;
> +
> + remote = media_entity_remote_pad(&video->pad);
> +
> + if (!remo
Re: [PATCH 08/10] media: camss: Add files which handle the video device nodes
On 12/05/2016 03:45 PM, Laurent Pinchart wrote:
> Hello,
>
> On Monday 05 Dec 2016 14:44:55 Hans Verkuil wrote:
>> On 11/25/2016 03:57 PM, Todor Tomov wrote:
>>> These files handle the video device nodes of the camss driver.
>>>
>>> Signed-off-by: Todor Tomov
>>> ---
>>>
>>> drivers/media/platform/qcom/camss-8x16/video.c | 597
>>> drivers/media/platform/qcom/camss-8x16/video.h | 67 +++
>>> 2 files changed, 664 insertions(+)
>>> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
>>> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h
>
> [snip]
>
>>> +int msm_video_register(struct camss_video *video, struct v4l2_device
>>> *v4l2_dev,
>>> +const char *name)
>>> +{
>>> + struct media_pad *pad = &video->pad;
>>> + struct video_device *vdev;
>>> + struct vb2_queue *q;
>>> + int ret;
>>> +
>>> + vdev = video_device_alloc();
>>> + if (vdev == NULL) {
>>> + dev_err(v4l2_dev->dev, "Failed to allocate video device\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + video->vdev = vdev;
>>> +
>>> + q = &video->vb2_q;
>>> + q->drv_priv = video;
>>> + q->mem_ops = &vb2_dma_contig_memops;
>>> + q->ops = &msm_video_vb2_q_ops;
>>> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>> + q->io_modes = VB2_MMAP;
>>
>> Add modes VB2_DMABUF and VB2_READ. These are for free, so why not?
>> Especially DMABUF is of course very desirable to have.
>
> I certainly agree with VB2_DMABUF, but I wouldn't expose VB2_READ. read() for
> this kind of device is inefficient and we should encourage userspace
> application to move away from it (and certainly make it very clear that new
> applications should not use read() with this device).
VB2_READ is very nice when debugging (no need to program a stream I/O
application first)
and useful when you want to pipe the video.
Nobody uses read() in 'regular' applications since it is obviously inefficient,
but
I don't see that as a reason not to offer VB2_READ.
I don't think this is something anyone will ever abuse, and it is a nice feature
to have (and for free as well).
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] media: camss: Add files which handle the video device nodes
Hello,
On Monday 05 Dec 2016 14:44:55 Hans Verkuil wrote:
> On 11/25/2016 03:57 PM, Todor Tomov wrote:
> > These files handle the video device nodes of the camss driver.
> >
> > Signed-off-by: Todor Tomov
> > ---
> >
> > drivers/media/platform/qcom/camss-8x16/video.c | 597
> > drivers/media/platform/qcom/camss-8x16/video.h | 67 +++
> > 2 files changed, 664 insertions(+)
> > create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
> > create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h
[snip]
> > +int msm_video_register(struct camss_video *video, struct v4l2_device
> > *v4l2_dev,
> > +const char *name)
> > +{
> > + struct media_pad *pad = &video->pad;
> > + struct video_device *vdev;
> > + struct vb2_queue *q;
> > + int ret;
> > +
> > + vdev = video_device_alloc();
> > + if (vdev == NULL) {
> > + dev_err(v4l2_dev->dev, "Failed to allocate video device\n");
> > + return -ENOMEM;
> > + }
> > +
> > + video->vdev = vdev;
> > +
> > + q = &video->vb2_q;
> > + q->drv_priv = video;
> > + q->mem_ops = &vb2_dma_contig_memops;
> > + q->ops = &msm_video_vb2_q_ops;
> > + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > + q->io_modes = VB2_MMAP;
>
> Add modes VB2_DMABUF and VB2_READ. These are for free, so why not?
> Especially DMABUF is of course very desirable to have.
I certainly agree with VB2_DMABUF, but I wouldn't expose VB2_READ. read() for
this kind of device is inefficient and we should encourage userspace
application to move away from it (and certainly make it very clear that new
applications should not use read() with this device).
> > + q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > + q->buf_struct_size = sizeof(struct camss_buffer);
> > + q->dev = video->camss->dev;
> > + ret = vb2_queue_init(q);
> > + if (ret < 0) {
> > + dev_err(v4l2_dev->dev, "Failed to init vb2 queue\n");
> > + return ret;
> > + }
> > +
> > + pad->flags = MEDIA_PAD_FL_SINK;
> > + ret = media_entity_pads_init(&vdev->entity, 1, pad);
> > + if (ret < 0) {
> > + dev_err(v4l2_dev->dev, "Failed to init video entity\n");
> > + goto error_media_init;
> > + }
> > +
> > + vdev->fops = &msm_vid_fops;
> > + vdev->ioctl_ops = &msm_vid_ioctl_ops;
> > + vdev->release = video_device_release;
> > + vdev->v4l2_dev = v4l2_dev;
> > + vdev->vfl_dir = VFL_DIR_RX;
> > + vdev->queue = &video->vb2_q;
>
> As mentioned in querycap: set vdev->device_caps here.
>
> > + strlcpy(vdev->name, name, sizeof(vdev->name));
> > +
> > + ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> > + if (ret < 0) {
> > + dev_err(v4l2_dev->dev, "Failed to register video device\n");
> > + goto error_video_register;
> > + }
> > +
> > + video_set_drvdata(vdev, video);
> > +
> > + return 0;
> > +
> > +error_video_register:
> > + media_entity_cleanup(&vdev->entity);
> > +error_media_init:
> > + vb2_queue_release(&video->vb2_q);
> > +
> > + return ret;
> > +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] media: camss: Add files which handle the video device nodes
Hi Hans,
On Monday 05 Dec 2016 14:44:55 Hans Verkuil wrote:
> > +static int video_querycap(struct file *file, void *fh,
> > + struct v4l2_capability *cap)
> > +{
> > + strlcpy(cap->driver, "qcom-camss", sizeof(cap->driver));
> > + strlcpy(cap->card, "Qualcomm Camera Subsystem", sizeof(cap->card));
> > + strlcpy(cap->bus_info, "platform:qcom-camss",
> > sizeof(cap->bus_info));
> > + cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> > + V4L2_CAP_DEVICE_CAPS
> > ;
> > + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
>
> Don't set capabilities and device_caps here. Instead fill in the struct
> video_device device_caps field and the v4l2 core will take care of
> cap->capabilities and cap->device_caps.
Time to add this to checkpatch.pl ? :-)
> > +
> > + return 0;
> > +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format
Hi Evgeni,
On Monday 05 Dec 2016 15:24:59 evgeni.raik...@gmail.com wrote:
> From: Evgeni Raikhel
>
> Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
> format utilized by Intel SR300 Depth camera.
>
> This is a complimentary patch for:
> [PATCH] UVC: Add support for Intel SR300 depth camera
>
> Signed-off-by: Evgeni Raikhel
> ---
> Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 +
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
>
> diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> b/Documentation/media/uapi/v4l/pixfmt-inzi.rst new file mode 100644
> index ..cdfdeae4a664
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> @@ -0,0 +1,40 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-PIX-FMT-INZI:
> +
> +**
> +V4L2_PIX_FMT_INZI ('INZI')
> +**
> +
> +Infrared 10-bit linked with Depth 16-bit images
> +
> +
> +Description
> +===
> +
> +Custom multi-planar format used by Intel SR300 Depth cameras, comprise of
> Infrared image followed by Depth data. +The pixel definition is 32-bpp,
> with the Depth and Infrared Data split into separate continuous planes of
> identical dimensions. +
> +The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see
> :ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual
> data present in the 10 LSBs with values in range 0 to 1023. The six
> remaining MSBs are padded with zeros. +
> +The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16
> (:ref:`pixfmt-z16`) format. +
In addition to my previous comments, wouldn't it make more sense to create a
multiplanar format for this instead of bundling the two separate images into a
single plane ?
> +**Frame Structure.**
> +Each cell is a 16-bit word with the significant data byte is stored at
> lower memory address (little-endian). +
> ++-+-+-+-+--
> ---+-+ +| Ir\ :sub:`0`| Ir\ :sub:`1`|
> Ir\ :sub:`2`| ... |... | ... |
> ++-+-+-+-+-
> +-+ +| ... ... ...
> | +|
> Infrared Data
>| +|
> ... ... ... |
> ++-+-+-+-+-
> +-+ +| Ir\ :sub:`n-3` | Ir\ :sub:`n-2` |
> Ir\ :sub:`n-1` | Depth\ :sub:`0` | Depth\ :sub:`1` | Depth\ :sub:`2` |
> ++-+-+-+-+-
> +-+ +| ... ... ...
> | +|
> Depth Data
>| +|
> ... ... ... |
> ++-+-+-+-+-
> +-+ +| ... | ... |
> ... |Depth\ :sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
> ++-+-+-+-+-
> +-+
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format
Hi Evgeni,
Thank you for the patch.
On Monday 05 Dec 2016 15:24:59 evgeni.raik...@gmail.com wrote:
> From: Evgeni Raikhel
>
> Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
> format utilized by Intel SR300 Depth camera.
>
> This is a complimentary patch for:
> [PATCH] UVC: Add support for Intel SR300 depth camera
Once the patches will be committed this reference will be harder to use, as it
would require searching for the other commit through the whole git history.
Here you should instead group related changes in a single patch. The first
patch in this series should add the new INZI format to the V4L2 API, with the
new format definition in include/uapi/linux/videodev2.h (currently part of
patch 1/2) and this documentation. The second patch should then add support
for that format in the uvcvideo driver, with the changes to
drivers/media/usb/uvc/* from patch 1/2.
Could you please split the patches that way and resubmit ? And please see
below for additional comments.
> Signed-off-by: Evgeni Raikhel
> ---
> Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 +
> 1 file changed, 40 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
>
> diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> b/Documentation/media/uapi/v4l/pixfmt-inzi.rst new file mode 100644
> index ..cdfdeae4a664
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> @@ -0,0 +1,40 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-PIX-FMT-INZI:
> +
> +**
> +V4L2_PIX_FMT_INZI ('INZI')
> +**
> +
> +Infrared 10-bit linked with Depth 16-bit images
> +
> +
> +Description
> +===
> +
> +Custom multi-planar format used by Intel SR300 Depth cameras, comprise of
> Infrared image followed by Depth data. +The pixel definition is 32-bpp,
> with the Depth and Infrared Data split into separate continuous planes of
> identical dimensions. +
> +The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see
> :ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual
> data present in the 10 LSBs with values in range 0 to 1023. The six
> remaining MSBs are padded with zeros. +
> +The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16
> (:ref:`pixfmt-z16`) format. +
The documentation, like source code, should be limited to 80 characters per
column. Could you please reformat the file ?
> +**Frame Structure.**
> +Each cell is a 16-bit word with the significant data byte is stored at
> lower memory address (little-endian). +
> ++-+-+-+-+--
> ---+-+ +| Ir\ :sub:`0`| Ir\ :sub:`1`|
> Ir\ :sub:`2`| ... |... | ... |
> ++-+-+-+-+-
> +-+ +| ... ... ...
> | +|
> Infrared Data
>| +|
> ... ... ... |
> ++-+-+-+-+-
> +-+ +| Ir\ :sub:`n-3` | Ir\ :sub:`n-2` |
> Ir\ :sub:`n-1` | Depth\ :sub:`0` | Depth\ :sub:`1` | Depth\ :sub:`2` |
> ++-+-+-+-+-
> +-+ +| ... ... ...
> | +|
> Depth Data
>| +|
> ... ... ... |
> ++-+-+-+-+-
> +-+ +| ... | ... |
> ... |Depth\ :sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
> ++-+-+-+-+-
> +-+
This table is harder to wrap to 80 columns, but if you use the same format
description style as the rest of the media documentation the limit shouldn't
be an issue.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] media: camss: Add files which handle the video device nodes
A few comments below:
On 11/25/2016 03:57 PM, Todor Tomov wrote:
> These files handle the video device nodes of the camss driver.
>
> Signed-off-by: Todor Tomov
> ---
> drivers/media/platform/qcom/camss-8x16/video.c | 597
> +
> drivers/media/platform/qcom/camss-8x16/video.h | 67 +++
> 2 files changed, 664 insertions(+)
> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.c
> create mode 100644 drivers/media/platform/qcom/camss-8x16/video.h
>
> diff --git a/drivers/media/platform/qcom/camss-8x16/video.c
> b/drivers/media/platform/qcom/camss-8x16/video.c
> new file mode 100644
> index 000..0bf8ea9
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss-8x16/video.c
> @@ -0,0 +1,597 @@
> +static int video_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> + struct camss_video *video = vb2_get_drv_priv(q);
> + struct video_device *vdev = video->vdev;
> + struct media_entity *entity;
> + struct media_pad *pad;
> + struct v4l2_subdev *subdev;
> + int ret;
> +
> + ret = media_entity_pipeline_start(&vdev->entity, &video->pipe);
> + if (ret < 0)
> + return ret;
> +
> + ret = video_check_format(video);
> + if (ret < 0)
> + goto error;
> +
> + entity = &vdev->entity;
> + while (1) {
> + pad = &entity->pads[0];
> + if (!(pad->flags & MEDIA_PAD_FL_SINK))
> + break;
> +
> + pad = media_entity_remote_pad(pad);
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> + break;
> +
> + entity = pad->entity;
> + subdev = media_entity_to_v4l2_subdev(entity);
> +
> + ret = v4l2_subdev_call(subdev, video, s_stream, 1);
> + if (ret < 0 && ret != -ENOIOCTLCMD)
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + media_entity_pipeline_stop(&vdev->entity);
> +
On error all queued buffers must be returned with state VB2_STATE_QUEUED.
Basically the same as 'camss_video_call(video, flush_buffers);', just with
a different state.
> + return ret;
> +}
> +static int video_querycap(struct file *file, void *fh,
> + struct v4l2_capability *cap)
> +{
> + strlcpy(cap->driver, "qcom-camss", sizeof(cap->driver));
> + strlcpy(cap->card, "Qualcomm Camera Subsystem", sizeof(cap->card));
> + strlcpy(cap->bus_info, "platform:qcom-camss", sizeof(cap->bus_info));
> + cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> + V4L2_CAP_DEVICE_CAPS;
> + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
Don't set capabilities and device_caps here. Instead fill in the struct
video_device
device_caps field and the v4l2 core will take care of cap->capabilities and
cap->device_caps.
> +
> + return 0;
> +}
> +
> +static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc
> *f)
> +{
> + struct camss_video *video = video_drvdata(file);
> + struct v4l2_format format;
> + int ret;
> +
> + if (f->type != video->type)
> + return -EINVAL;
> +
> + if (f->index)
> + return -EINVAL;
> +
> + ret = video_get_subdev_format(video, &format);
> + if (ret < 0)
> + return ret;
> +
> + f->pixelformat = format.fmt.pix.pixelformat;
> +
> + return 0;
> +}
> +
> +static int video_g_fmt(struct file *file, void *fh, struct v4l2_format *f)
> +{
> + struct camss_video *video = video_drvdata(file);
> +
> + if (f->type != video->type)
> + return -EINVAL;
> +
> + *f = video->active_fmt;
> +
> + return 0;
> +}
> +
> +static int video_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
> +{
> + struct camss_video *video = video_drvdata(file);
> + int ret;
> +
> + if (f->type != video->type)
> + return -EINVAL;
> +
> + ret = video_get_subdev_format(video, f);
> + if (ret < 0)
> + return ret;
> +
> + video->active_fmt = *f;
> +
> + return 0;
> +}
> +
> +static int video_try_fmt(struct file *file, void *fh, struct v4l2_format *f)
> +{
> + struct camss_video *video = video_drvdata(file);
> +
> + if (f->type != video->type)
> + return -EINVAL;
> +
> + return video_get_subdev_format(video, f);
> +}
> +
> +static int video_enum_input(struct file *file, void *fh,
> + struct v4l2_input *input)
> +{
> + if (input->index > 0)
> + return -EINVAL;
> +
> + strlcpy(input->name, "camera", sizeof(input->name));
> + input->type = V4L2_INPUT_TYPE_CAMERA;
> +
> + return 0;
> +}
> +
> +static int video_g_input(struct file *file, void *fh, unsigned int *input)
> +{
> + *input = 0;
> +
> + return 0;
> +}
> +
> +static int video_s_input(struct file *file, void *fh, unsigned int input)
> +{
> + return input == 0
RE: [PATCH] UVC Module - Support Intel RealSense SR300 Depth Camera formats
Hi Laurent, Thanks for the feedback - I resubmitted the patches inline, as requested. Regards, Evgeni Raikhel -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Monday, December 05, 2016 13:02 To: Raikhel, Evgeni Cc: linux-media@vger.kernel.org Subject: Re: [PATCH] UVC Module - Support Intel RealSense SR300 Depth Camera formats Hi Evgeni, Thank you for the patch. On Monday 05 Dec 2016 10:06:55 Raikhel, Evgeni wrote: > Specify GUID and FourCC codes mapping for Depth-related pixel formats > advertised by Intel RealSense(tm) SR300 depth camera. Provide > documentation for the new INZI pixel format introduced. Could you please resend the patches inline instead of as attachments ? See Documentation/SubmittingPatches for more information. -- Regards, Laurent Pinchart - Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- 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 v3 07/10] [media] st-delta: rpmsg ipc support
Thanks Hans, I will change depends to select. BR, Hugues. On 12/05/2016 11:47 AM, Hans Verkuil wrote: > On 11/22/2016 04:53 PM, Hugues Fruchet wrote: >> IPC (Inter Process Communication) support for communication with >> DELTA coprocessor firmware using rpmsg kernel framework. >> Based on 4 services open/set_stream/decode/close and their associated >> rpmsg messages. >> The messages structures are duplicated on both host and firmware >> side and are packed (use only of 32 bits size fields in messages >> structures to ensure packing). >> Each service is synchronous; service returns only when firmware >> acknowledges the associated command message. >> Due to significant parameters size exchanged from host to copro, >> parameters are not inserted in rpmsg messages. Instead, parameters are >> stored in physical memory shared between host and coprocessor. >> Memory is non-cacheable, so no special operation is required >> to ensure memory coherency on host and on coprocessor side. >> Multi-instance support and re-entrance are ensured using host_hdl and >> copro_hdl in message header exchanged between both host and coprocessor. >> This avoids to manage tables on both sides to get back the running context >> of each instance. >> >> Signed-off-by: Hugues Fruchet >> --- >> drivers/media/platform/Kconfig| 1 + >> drivers/media/platform/sti/delta/Makefile | 2 +- >> drivers/media/platform/sti/delta/delta-ipc.c | 590 >> ++ >> drivers/media/platform/sti/delta/delta-ipc.h | 76 >> drivers/media/platform/sti/delta/delta-v4l2.c | 11 + >> drivers/media/platform/sti/delta/delta.h | 21 + >> 6 files changed, 700 insertions(+), 1 deletion(-) >> create mode 100644 drivers/media/platform/sti/delta/delta-ipc.c >> create mode 100644 drivers/media/platform/sti/delta/delta-ipc.h >> >> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig >> index f494f01..5519442 100644 >> --- a/drivers/media/platform/Kconfig >> +++ b/drivers/media/platform/Kconfig >> @@ -303,6 +303,7 @@ config VIDEO_STI_DELTA >> depends on VIDEO_DEV && VIDEO_V4L2 >> depends on ARCH_STI || COMPILE_TEST >> depends on HAS_DMA >> +depends on RPMSG > > This should be 'select', not 'depends on'. > >> select VIDEOBUF2_DMA_CONTIG >> select V4L2_MEM2MEM_DEV >> help > > Can you make a v3.1 of this patch correcting this? > > Regards, > > Hans >-- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] uvcvideo: *** Support Intel SR300 Depth camera formats ***
From: eraikhel *** Enable Intel RealSense™ SR300 Depth camera pixel formats to be recognized correctly by uvc module *** Aviv Greenberg (1): UVC: Add support for Intel SR300 depth camera Evgeni Raikhel (1): Document Intel SR300 Depth camera INZI format Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 drivers/media/usb/uvc/uvc_driver.c | 15 +++ drivers/media/usb/uvc/uvcvideo.h | 9 +++ include/uapi/linux/videodev2.h | 1 + 4 files changed, 65 insertions(+) create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst -- 2.7.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
[PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format
From: Evgeni Raikhel
Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
format utilized by Intel SR300 Depth camera.
This is a complimentary patch for:
[PATCH] UVC: Add support for Intel SR300 depth camera
Signed-off-by: Evgeni Raikhel
---
Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40
1 file changed, 40 insertions(+)
create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst
b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
new file mode 100644
index ..cdfdeae4a664
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
@@ -0,0 +1,40 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-INZI:
+
+**
+V4L2_PIX_FMT_INZI ('INZI')
+**
+
+Infrared 10-bit linked with Depth 16-bit images
+
+
+Description
+===
+
+Custom multi-planar format used by Intel SR300 Depth cameras, comprise of
Infrared image followed by Depth data.
+The pixel definition is 32-bpp, with the Depth and Infrared Data split into
separate continuous planes of identical dimensions.
+
+The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see
:ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual
data present in the 10 LSBs with values in range 0 to 1023. The six remaining
MSBs are padded with zeros.
+
+The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16
(:ref:`pixfmt-z16`) format.
+
+
+**Frame Structure.**
+Each cell is a 16-bit word with the significant data byte is stored at lower
memory address (little-endian).
+
++-+-+-+-+-+-+
+| Ir\ :sub:`0`| Ir\ :sub:`1`| Ir\ :sub:`2`| ... |
... | ... |
++-+-+-+-+-+-+
+| ... ... ...
|
+| Infrared Data
|
+| ... ... ...
|
++-+-+-+-+-+-+
+| Ir\ :sub:`n-3` | Ir\ :sub:`n-2` | Ir\ :sub:`n-1` | Depth\ :sub:`0` |
Depth\ :sub:`1` | Depth\ :sub:`2` |
++-+-+-+-+-+-+
+| ... ... ...
|
+| Depth Data
|
+| ... ... ...
|
++-+-+-+-+-+-+
+| ... | ... | ... |Depth\
:sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
++-+-+-+-+-+-+
--
2.7.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
[PATCH 1/2] uvcvideo: Add support for Intel SR300 depth camera
From: Aviv Greenberg
Add support for Intel SR300 depth camera in uvc driver.
This includes adding three uvc GUIDs for the required pixel formats,
adding a new V4L pixel format definition to user api headers,
and updating the uvc driver GUID-to-4cc tables with the new formats.
Signed-off-by: Aviv Greenberg
Signed-off-by: Evgeni Raikhel
---
drivers/media/usb/uvc/uvc_driver.c | 15 +++
drivers/media/usb/uvc/uvcvideo.h | 9 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 25 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index 11744f92097b..5b96a89f29ae 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -168,6 +168,21 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid = UVC_GUID_FORMAT_RW10,
.fcc= V4L2_PIX_FMT_SRGGB10P,
},
+ {
+ .name = "Depth data 16-bit (Z16)",
+ .guid = UVC_GUID_FORMAT_INVZ,
+ .fcc= V4L2_PIX_FMT_Z16,
+ },
+ {
+ .name = "IR:Depth 26-bit (INZI)",
+ .guid = UVC_GUID_FORMAT_INZI,
+ .fcc= V4L2_PIX_FMT_INZI,
+ },
+ {
+ .name = "Greyscale 10-bit (Y10 )",
+ .guid = UVC_GUID_FORMAT_INVI,
+ .fcc= V4L2_PIX_FMT_Y10,
+ },
};
/*
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 7e4d3eea371b..460b99ca99b7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -131,6 +131,15 @@
#define UVC_GUID_FORMAT_RW10 \
{ 'R', 'W', '1', '0', 0x00, 0x00, 0x10, 0x00, \
0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_INVZ \
+ { 'I', 'N', 'V', 'Z', 0x90, 0x2d, 0x58, 0x4a, \
+0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
+#define UVC_GUID_FORMAT_INZI \
+ { 'I', 'N', 'Z', 'I', 0x66, 0x1a, 0x42, 0xa2, \
+0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
+#define UVC_GUID_FORMAT_INVI \
+ { 'I', 'N', 'V', 'I', 0xdb, 0x57, 0x49, 0x5e, \
+0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
/*
* Driver specific constants.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index d3f613e2c54a..4ab995bbec5b 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -659,6 +659,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y12I v4l2_fourcc('Y', '1', '2', 'I') /* Greyscale
12-bit L/R interleaved */
#define V4L2_PIX_FMT_Z16 v4l2_fourcc('Z', '1', '6', ' ') /* Depth data
16-bit */
#define V4L2_PIX_FMT_MT21Cv4l2_fourcc('M', 'T', '2', '1') /* Mediatek
compressed block mode */
+#define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel
Infrared 10-bit linked with Depth 16-bit */
/* SDR formats - used only for Software Defined Radio devices */
#define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
--
2.7.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 v3 00/10] Add support for DELTA video decoder of STMicroelectronics STiH4xx SoC series
Thanks Hans, I will test with new compliance. BR, Hugues. On 12/05/2016 11:18 AM, Hans Verkuil wrote: > Hi Hugues, > > On 11/22/2016 04:53 PM, Hugues Fruchet wrote: >> This patchset introduces a basic support for DELTA multi-format video >> decoder of STMicroelectronics STiH4xx SoC series. >> >> DELTA hardware IP is controlled by a remote firmware loaded in a ST231 >> coprocessor. Communication with firmware is done within an IPC layer >> using rpmsg kernel framework and a shared memory for messages handling. >> This driver is compatible with firmware version 21.1-3. >> While a single firmware is loaded in ST231 coprocessor, it is composed >> of several firmwares, one per video format family. >> >> This DELTA V4L2 driver is designed around files: >> - delta-v4l2.c : handles V4L2 APIs using M2M framework and calls decoder >> ops >> - delta-* : implements decoder calling its associated >> video firmware (for ex. MJPEG) using IPC layer >> - delta-ipc.c: IPC layer which handles communication with firmware >> using rpmsg >> >> This first basic support implements only MJPEG hardware acceleration but >> the driver structure is in place to support all the features of the >> DELTA video decoder hardware IP. >> >> This driver depends on: >> - ST remoteproc/rpmsg: patchset posted at https://lkml.org/lkml/2016/9/6/77 >> - ST DELTA firmware: its license is under review. When available, >> pull request will be done on linux-firmware. >> >> === >> = history = >> === >> version 3 >> - update after v2 review: >> - fixed m2m_buf_done missing on start_streaming error case >> - fixed q->dev missing in queue_init() >> - removed unsupported s_selection >> - refactored string namings in delta-debug.c >> - fixed space before comment >> - all commits have commit messages >> - reword memory allocator helper commit >> >> version 2 >> - update after v1 review: >> - simplified tracing >> - G_/S_SELECTION reworked to fit COMPOSE(CAPTURE) >> - fixed m2m_buf_done missing on start_streaming error case >> - fixed q->dev missing in queue_init() >> - switch to kernel-4.9 rpmsg API >> - DELTA support added in multi_v7_defconfig >> - minor typo fixes & code cleanup >> >> version 1: >> - Initial submission >> >> === >> = v4l2-compliance = >> === >> Below is the v4l2-compliance report for the version 3 of the DELTA video >> decoder driver. v4l2-compliance has been build from SHA1: >> 600492351ddf40cc524aab73802153674d7d287b (libdvb5: Fix multiple definition >> of dvb_dev_remote_init linking error) > > Can you update v4l-utils and run this test again? The S_SELECTION compliance > test has been fixed to allow for ENOTTY as the error code. > > If the output looks good, then I'll merge this driver (will be for 4.11). > > Regards, > > Hans >-- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay
Hello Javi,
On 12/05/2016 07:09 AM, Javi Merino wrote:
> In asds configured with V4L2_ASYNC_MATCH_OF, the v4l2 subdev can be
> part of a devicetree overlay, for example:
>
> &media_bridge {
> ...
> my_port: port@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0>;
> ep: endpoint@0 {
> remote-endpoint = <&camera0>;
> };
> };
> };
>
> / {
> fragment@0 {
> target = <&i2c0>;
> __overlay__ {
> my_cam {
> compatible = "foo,bar";
> port {
> camera0: endpoint {
> remote-endpoint = <&my_port>;
> ...
> };
> };
> };
> };
> };
> };
>
> Each time the overlay is applied, its of_node pointer will be
> different. We are not interested in matching the pointer, what we
> want to match is that the path is the one we are expecting. Change to
> use of_node_cmp() so that we continue matching after the overlay has
> been removed and reapplied.
>
> Cc: Mauro Carvalho Chehab
> Cc: Javier Martinez Canillas
> Cc: Sakari Ailus
> Signed-off-by: Javi Merino
> ---
I already reviewed v1 but you didn't carry the tag. So again:
Reviewed-by: Javier Martinez Canillas
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/9] media: venus: vdec: add video decoder files
Hi Hans,
Thanks for the comments!
On 12/05/2016 01:32 PM, Hans Verkuil wrote:
> I have two comments (and the same two comments apply to the video encoder
> patch
> as well):
>
> On 12/01/2016 10:03 AM, Stanimir Varbanov wrote:
>> This consists of video decoder implementation plus decoder
>> controls.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/platform/qcom/venus/vdec.c | 976
>> +
>> drivers/media/platform/qcom/venus/vdec.h | 32 +
>> drivers/media/platform/qcom/venus/vdec_ctrls.c | 149
>> 3 files changed, 1157 insertions(+)
>> create mode 100644 drivers/media/platform/qcom/venus/vdec.c
>> create mode 100644 drivers/media/platform/qcom/venus/vdec.h
>> create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
>> b/drivers/media/platform/qcom/venus/vdec.c
>> new file mode 100644
>> index ..9f585a1e0ff1
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -0,0 +1,976 @@
>
>
>
>> +static int
>> +vdec_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
>> +{
>> +struct venus_inst *inst = to_inst(file);
>> +
>> +if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
>> +s->target != V4L2_SEL_TGT_COMPOSE)
>> +return -EINVAL;
>> +
>> +switch (s->target) {
>> +case V4L2_SEL_TGT_COMPOSE:
>> +s->r.width = inst->out_width;
>> +s->r.height = inst->out_height;
>> +break;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +s->r.top = 0;
>> +s->r.left = 0;
>> +
>> +return 0;
>> +}
>
> This doesn't actually set anything, so what's the point of this function?
>
> I've fixed the corresponding test in v4l2-compliance so you can now drop this
> op and v4l2-compliance won't complain anymore.
OK I will give a try.
>
>> +static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
>> +{
>> +struct venus_inst *inst = vb2_get_drv_priv(q);
>> +struct venus_core *core = inst->core;
>> +struct device *dev = core->dev;
>> +u32 ptype;
>> +int ret;
>> +
>> +mutex_lock(&inst->lock);
>> +
>> +if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>> +inst->streamon_out = 1;
>> +else
>> +inst->streamon_cap = 1;
>> +
>> +if (!(inst->streamon_out & inst->streamon_cap)) {
>> +mutex_unlock(&inst->lock);
>> +return 0;
>> +}
>> +
>> +inst->reconfig = false;
>> +inst->sequence = 0;
>> +inst->codec_cfg = false;
>> +
>> +ret = pm_runtime_get_sync(dev);
>> +if (ret < 0)
>> +return ret;
>
> This should be a goto so that 'helper_buffers_done(inst,
> VB2_BUF_STATE_QUEUED);'
> is called on error.
>
> It's wrong anyway since you don't unlock the mutex in this return path either.
yes, this return is wrong, will correct it in next version.
--
regards,
Stan
--
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 v3 3/3] ARM: multi_v7_defconfig: enable STMicroelectronics HVA debugfs
Please provide a commit message, it shouldn't be empty. But are you sure you want to enable it in the defconfig? I think in general DEBUGFS config options aren't enabled by default. Regards, Hans On 11/28/2016 11:30 AM, Jean-Christophe Trotin wrote: > Signed-off-by: Jean-Christophe Trotin > --- > arch/arm/configs/multi_v7_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/configs/multi_v7_defconfig > b/arch/arm/configs/multi_v7_defconfig > index eb14ab6..7a15107 100644 > --- a/arch/arm/configs/multi_v7_defconfig > +++ b/arch/arm/configs/multi_v7_defconfig > @@ -563,6 +563,7 @@ CONFIG_VIDEO_SAMSUNG_S5P_JPEG=m > CONFIG_VIDEO_SAMSUNG_S5P_MFC=m > CONFIG_VIDEO_STI_BDISP=m > CONFIG_VIDEO_STI_HVA=m > +CONFIG_VIDEO_STI_HVA_DEBUGFS=y > CONFIG_DYNAMIC_DEBUG=y > CONFIG_VIDEO_RENESAS_JPU=m > CONFIG_VIDEO_RENESAS_VSP1=m > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/9] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management
Hi Hans, On 12/05/2016 01:25 PM, Hans Verkuil wrote: > On 12/01/2016 10:03 AM, Stanimir Varbanov wrote: >> this add functions for: >> - remove buffers from src/dst queue by index >> - remove exact buffer from src/dst queue >> >> also extends m2m API to iterate over a list of src/dst buffers >> in safely and non-safely manner. >> >> Signed-off-by: Stanimir Varbanov >> --- >> drivers/media/v4l2-core/v4l2-mem2mem.c | 37 +++ >> include/media/v4l2-mem2mem.h | 83 >> ++ >> 2 files changed, 120 insertions(+) >> >> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c >> b/drivers/media/v4l2-core/v4l2-mem2mem.c >> index 6bc27e7b2a33..d689e7bb964f 100644 >> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c >> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c >> @@ -126,6 +126,43 @@ void *v4l2_m2m_buf_remove(struct v4l2_m2m_queue_ctx >> *q_ctx) >> } >> EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove); >> >> +void v4l2_m2m_buf_remove_exact(struct v4l2_m2m_queue_ctx *q_ctx, >> + struct vb2_v4l2_buffer *vbuf) > > I'd call this v4l2_m2m_buf_remove_by_buf to be consistent with _by_idx. Thanks, I will rename it. -- regards, Stan -- 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 v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
On 12/01/2016 10:16 AM, Laurent Pinchart wrote:
> Hello,
>
> On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
>> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
>>> Sakari Ailus writes:
On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
> Sakari Ailus writes:
>> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>>> Allow getting of subdevs from DT ports and endpoints.
>>>
>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>>> am437x-vpfe.c
>>>
>>> Signed-off-by: Kevin Hilman
>>> ---
>>>
>>> drivers/media/platform/davinci/vpif_capture.c | 130 +++-
>>> include/media/davinci/vpif_types.h
>>>| 9 +-
>>> 2 files changed, 133 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>>> b/drivers/media/platform/davinci/vpif_capture.c index
>>> 94ee6cf03f02..47a4699157e7 100644
>>> --- a/drivers/media/platform/davinci/vpif_capture.c
>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>>> @@ -26,6 +26,8 @@
>>> #include
>>>
>>> #include
>>> +#include
>>> +#include
>>
>> Do you need this header?
>
> Yes, based on discussion with Hans, since there is no DT binding for
> selecting the input pins of the TVP514x, I have to select it in the
> driver, so I need the defines from this header. More on this below...
>
> That's really ugly :-( The problem should be fixed properly instead of adding
> one more offender.
Do you have time for that, Laurent? I don't. Until that time we just need to
make do with this workaround.
>
>>> #include "vpif.h"
>>> #include "vpif_capture.h"
>>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>>
>>> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>>
>>> + if (!chan_cfg)
>>> + return -1;
>>> + if (input_index >= chan_cfg->input_count)
>>> + return -1;
>>> subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>> if (subdev_name == NULL)
>>> return -1;
>>> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
>>> /* loop through the sub device list to get the sub device info
>>> */
>>> for (i = 0; i < vpif_cfg->subdev_count; i++) {
>>> subdev_info = &vpif_cfg->subdev_info[i];
>>> - if (!strcmp(subdev_info->name, subdev_name))
>>> + if (subdev_info && !strcmp(subdev_info->name,
>>> subdev_name))
>>> return i;
>>> }
>>> return -1;
>>> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct
>>> v4l2_async_notifier *notifier,> >> >>
>>> {
>>> int i;
>>>
>>> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>>> + struct v4l2_async_subdev *_asd = vpif_obj.config
>>> ->asd[i];
>>> + const struct device_node *node = _asd->match.of.node;
>>> +
>>> + if (node == subdev->of_node) {
>>> + vpif_obj.sd[i] = subdev;
>>> + vpif_obj.config->chan_config
>>> ->inputs[i].subdev_name =
>>> + (char *)subdev->of_node->full_name;
>
> Can subdev_name be made const instead of blindly casting the full_name
> pointer
> ? If not this is probably unsafe, and if yes it should be done :-)
>
>>> + vpif_dbg(2, debug,
>>> +"%s: setting input %d subdev_name =
>>> %s\n",
>>> +__func__, i, subdev->of_node
>>> ->full_name);
>>> + return 0;
>>> + }
>>> + }
>>> +
>>> for (i = 0; i < vpif_obj.config->subdev_count; i++)
>>> if (!strcmp(vpif_obj.config->subdev_info[i].name,
>>> subdev->name)) {
>>> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct
>>> v4l2_async_notifier *notifier)
>>> return vpif_probe_complete();
>>> }
>>>
>>> +static struct vpif_capture_config *
>>> +vpif_capture_get_pdata(struct platform_device *pdev)
>>> +{
>>> + struct device_node *endpoint = NULL;
>>> + struct v4l2_of_endpoint bus_cfg;
>>> + struct vpif_capture_config *pdata;
>>> + struct vpif_subdev_info *sdinfo;
>>> + struct vpif_capture_chan_config *chan;
>>> + unsigned int i;
>>> +
>>> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
>>> +
>>> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>>> + return pdev->dev.platform_data;
>>> +
>>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>>
Re: [PATCH v4 8/9] media: venus: hfi: add Venus HFI files
Hi Hans,
On 12/05/2016 02:05 PM, Hans Verkuil wrote:
> On 12/01/2016 10:03 AM, Stanimir Varbanov wrote:
>> Here is the implementation of Venus video accelerator low-level
>> functionality. It contanins code which setup the registers and
>> startup uthe processor, allocate and manipulates with the shared
>> memory used for sending commands and receiving messages.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/platform/qcom/venus/hfi_venus.c| 1508
>> ++
>> drivers/media/platform/qcom/venus/hfi_venus.h| 23 +
>> drivers/media/platform/qcom/venus/hfi_venus_io.h | 98 ++
>> 3 files changed, 1629 insertions(+)
>> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.c
>> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.h
>> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus_io.h
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
>> b/drivers/media/platform/qcom/venus/hfi_venus.c
>> new file mode 100644
>> index ..f004a9a80d85
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
>> @@ -0,0 +1,1508 @@
>> +static int venus_tzbsp_set_video_state(enum tzbsp_video_state state)
>> +{
>> +return qcom_scm_video_set_state(state, 0);
>
> This functions doesn't seem to exist. Is there a prerequisite patch series
> that
> introduces this function?
yes, the patchset [1] is under review.
--
regards,
Stan
[1] https://lkml.org/lkml/2016/11/7/533
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] usbtv: add a new usbid
On Sun, 2016-12-04 at 21:59 +0800, Icenowy Zheng wrote:
> A new usbid of UTV007 is found in a newly bought device.
>
> The usbid is 1f71:3301.
>
> The ID on the chip is:
> UTV007
> A89029.1
> 1520L18K1
>
> Both video and audio is tested with the modified usbtv driver.
Thank you.
Acked-by: Lubomir Rintel
Also, it may make sense to add
Tested-by: Icenowy Zheng
>
> Signed-off-by: Icenowy Zheng
> ---
> drivers/media/usb/usbtv/usbtv-core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/usbtv/usbtv-core.c
> b/drivers/media/usb/usbtv/usbtv-core.c
> index dc76fd4..0324633 100644
> --- a/drivers/media/usb/usbtv/usbtv-core.c
> +++ b/drivers/media/usb/usbtv/usbtv-core.c
> @@ -141,6 +141,7 @@ static void usbtv_disconnect(struct usb_interface
> *intf)
>
> static struct usb_device_id usbtv_id_table[] = {
> { USB_DEVICE(0x1b71, 0x3002) },
> + { USB_DEVICE(0x1f71, 0x3301) },
> {}
> };
> MODULE_DEVICE_TABLE(usb, usbtv_id_table);
--
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 v3 3/4] stk1160: Add module param for setting the record gain.
Em Sun, 4 Dec 2016 15:25:25 -0300 Ezequiel Garcia escreveu: > On 4 December 2016 at 10:01, Marcel Hasler wrote: > > Hello > > > > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia : > > > >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab > >> wrote: > >>> Em Sun, 27 Nov 2016 12:11:48 +0100 > >>> Marcel Hasler escreveu: > >>> > Allow setting a custom record gain for the internal AC97 codec (if > available). This can be > a value between 0 and 15, 8 is the default and should be suitable for > most users. The Windows > driver also sets this to 8 without any possibility for changing it. > >>> > >>> The problem of removing the mixer is that you need this kind of > >>> crap to setup the volumes on a non-standard way. > >>> > >> > >> Right, that's a good point. > >> > >>> NACK. > >>> > >>> Instead, keep the alsa mixer. The way other drivers do (for example, > >>> em28xx) is that they configure the mixer when an input is selected, > >>> increasing the volume of the active audio channel to 100% and muting > >>> the other audio channels. Yet, as the alsa mixer is exported, users > >>> can change the mixer settings in runtime using some alsa (or pa) > >>> mixer application. > >>> > >> > >> Yeah, the AC97 mixer we are currently leveraging > >> exposes many controls that have no meaning in this device, > >> so removing that still looks like an improvement. > >> > >> I guess the proper way is creating our own mixer > >> (not using snd_ac97_mixer) exposing only the record > >> gain knob. > >> > >> Marcel, what do you think? > >> -- > >> Ezequiel García, VanguardiaSur > >> www.vanguardiasur.com.ar > > > > As I have written before, the recording gain isn't actually meant to > > be changed by the user. In the official Windows driver this value is > > hard-coded to 8 and cannot be changed in any way. And there really is > > no good reason why anyone should need to mess with it in the first > > place. The default value will give the best results in pretty much all > > cases and produces approximately the same volume as the internal 8-bit > > ADC whose gain cannot be changed at all, not even by a driver. > > > > I had considered writing a mixer but chose not to. If the gain setting > > is openly exposed to mixer applications, how do you tell the users > > that the value set by the driver already is the optimal and > > recommended value and that they shouldn't mess with the controls > > unless they really have to? By having a module parameter, this setting > > is practically hidden from the normal user but still is available to > > power-users if they think they really need it. In the end it's really > > just a compromise between hiding it completely and exposing it openly. > > Also, this way the driver guarantees reproducible results, since > > there's no need to remember the positions of any volume sliders. > > > > Hm, right. I've never changed the record gain, and it's true that it > doens't really improve the volume. So, I would be OK with having > a module parameter. > > On the other side, we are exposing it currently, through the "Capture" > mixer control: > > Simple mixer control 'Capture',0 > Capabilities: cvolume cswitch cswitch-joined > Capture channels: Front Left - Front Right > Limits: Capture 0 - 15 > Front Left: Capture 10 [67%] [15.00dB] [on] > Front Right: Capture 8 [53%] [12.00dB] [on] > > So, it would be user-friendly to keep the user interface and continue > to expose the same knob - even if the default is the optimal, etc. > > To be completely honest, I don't think any user is really relying > on any REC_GAIN / Capture setting, and I'm completely OK > with having a mixer control or a module parameter. It doesn't matter. If you're positive that *all* stk1160 use the ac97 mixer the same way, and that there's no sense on having a mixer for it, then it would be ok to remove it. In such case, then why you need a modprobe parameter to allow setting the record level? If this mixer entry is not used, just set it to zero and be happy with that. Regards, 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
Re: [PATCH] [media] usbtv: add a new usbid
On Sun, 2016-12-04 at 22:59 +0800, Icenowy Zheng wrote: > > 04.12.2016, 22:00, "Icenowy Zheng" : > > A new usbid of UTV007 is found in a newly bought device. > > > > The usbid is 1f71:3301. > > > > The ID on the chip is: > > UTV007 > > A89029.1 > > 1520L18K1 > > > > Seems that my device come with more capabilities. > > I tested it under Windows, and I got wireless Analog TV > and FM radio functions. (An antenna is shipped with my device) > > Maybe a new radio function is be added, combined with the > new USB ID. > > But at least Composite AV function works well with current usbtv > driver and XawTV. Well, someone with the hardware would need to capture the traffic from the Windows driver (and ideally also extend the driver). Would you mind giving it a try? Do you have a link to some further details about the device you got? Perhaps if it's available cheaply from dealextreme or somewhere I could take a look too. Lubo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 8/9] media: venus: hfi: add Venus HFI files
On 12/01/2016 10:03 AM, Stanimir Varbanov wrote:
> Here is the implementation of Venus video accelerator low-level
> functionality. It contanins code which setup the registers and
> startup uthe processor, allocate and manipulates with the shared
> memory used for sending commands and receiving messages.
>
> Signed-off-by: Stanimir Varbanov
> ---
> drivers/media/platform/qcom/venus/hfi_venus.c| 1508
> ++
> drivers/media/platform/qcom/venus/hfi_venus.h| 23 +
> drivers/media/platform/qcom/venus/hfi_venus_io.h | 98 ++
> 3 files changed, 1629 insertions(+)
> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.c
> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.h
> create mode 100644 drivers/media/platform/qcom/venus/hfi_venus_io.h
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
> b/drivers/media/platform/qcom/venus/hfi_venus.c
> new file mode 100644
> index ..f004a9a80d85
> --- /dev/null
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -0,0 +1,1508 @@
> +static int venus_tzbsp_set_video_state(enum tzbsp_video_state state)
> +{
> + return qcom_scm_video_set_state(state, 0);
This functions doesn't seem to exist. Is there a prerequisite patch series that
introduces this function?
> +}
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/9] media: venus: vdec: add video decoder files
I have two comments (and the same two comments apply to the video encoder patch
as well):
On 12/01/2016 10:03 AM, Stanimir Varbanov wrote:
> This consists of video decoder implementation plus decoder
> controls.
>
> Signed-off-by: Stanimir Varbanov
> ---
> drivers/media/platform/qcom/venus/vdec.c | 976
> +
> drivers/media/platform/qcom/venus/vdec.h | 32 +
> drivers/media/platform/qcom/venus/vdec_ctrls.c | 149
> 3 files changed, 1157 insertions(+)
> create mode 100644 drivers/media/platform/qcom/venus/vdec.c
> create mode 100644 drivers/media/platform/qcom/venus/vdec.h
> create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c
>
> diff --git a/drivers/media/platform/qcom/venus/vdec.c
> b/drivers/media/platform/qcom/venus/vdec.c
> new file mode 100644
> index ..9f585a1e0ff1
> --- /dev/null
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -0,0 +1,976 @@
> +static int
> +vdec_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
> +{
> + struct venus_inst *inst = to_inst(file);
> +
> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
> + s->target != V4L2_SEL_TGT_COMPOSE)
> + return -EINVAL;
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE:
> + s->r.width = inst->out_width;
> + s->r.height = inst->out_height;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + s->r.top = 0;
> + s->r.left = 0;
> +
> + return 0;
> +}
This doesn't actually set anything, so what's the point of this function?
I've fixed the corresponding test in v4l2-compliance so you can now drop this
op and v4l2-compliance won't complain anymore.
> +static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> + struct venus_inst *inst = vb2_get_drv_priv(q);
> + struct venus_core *core = inst->core;
> + struct device *dev = core->dev;
> + u32 ptype;
> + int ret;
> +
> + mutex_lock(&inst->lock);
> +
> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> + inst->streamon_out = 1;
> + else
> + inst->streamon_cap = 1;
> +
> + if (!(inst->streamon_out & inst->streamon_cap)) {
> + mutex_unlock(&inst->lock);
> + return 0;
> + }
> +
> + inst->reconfig = false;
> + inst->sequence = 0;
> + inst->codec_cfg = false;
> +
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0)
> + return ret;
This should be a goto so that 'helper_buffers_done(inst, VB2_BUF_STATE_QUEUED);'
is called on error.
It's wrong anyway since you don't unlock the mutex in this return path either.
> +
> + ret = vdec_init_session(inst);
> + if (ret)
> + goto put_sync;
> +
> + ret = vdec_set_properties(inst);
> + if (ret)
> + goto deinit_sess;
> +
> + if (core->res->hfi_version == HFI_VERSION_3XX) {
> + struct hfi_buffer_size_actual buf_sz;
> +
> + ptype = HFI_PROPERTY_PARAM_BUFFER_SIZE_ACTUAL;
> + buf_sz.type = HFI_BUFFER_OUTPUT;
> + buf_sz.size = inst->output_buf_size;
> +
> + ret = hfi_session_set_property(inst, ptype, &buf_sz);
> + if (ret)
> + goto deinit_sess;
> + }
> +
> + ret = vdec_verify_conf(inst);
> + if (ret)
> + goto deinit_sess;
> +
> + ret = helper_set_num_bufs(inst, inst->num_input_bufs,
> + inst->num_output_bufs);
> + if (ret)
> + goto deinit_sess;
> +
> + ret = helper_vb2_start_streaming(inst);
> + if (ret)
> + goto deinit_sess;
> +
> + mutex_unlock(&inst->lock);
> +
> + return 0;
> +
> +deinit_sess:
> + hfi_session_deinit(inst);
> +put_sync:
> + pm_runtime_put_sync(dev);
> + helper_buffers_done(inst, VB2_BUF_STATE_QUEUED);
> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> + inst->streamon_out = 0;
> + else
> + inst->streamon_cap = 0;
> + mutex_unlock(&inst->lock);
> + return ret;
> +}
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/9] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management
On 12/01/2016 10:03 AM, Stanimir Varbanov wrote:
> this add functions for:
> - remove buffers from src/dst queue by index
> - remove exact buffer from src/dst queue
>
> also extends m2m API to iterate over a list of src/dst buffers
> in safely and non-safely manner.
>
> Signed-off-by: Stanimir Varbanov
> ---
> drivers/media/v4l2-core/v4l2-mem2mem.c | 37 +++
> include/media/v4l2-mem2mem.h | 83
> ++
> 2 files changed, 120 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 6bc27e7b2a33..d689e7bb964f 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -126,6 +126,43 @@ void *v4l2_m2m_buf_remove(struct v4l2_m2m_queue_ctx
> *q_ctx)
> }
> EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove);
>
> +void v4l2_m2m_buf_remove_exact(struct v4l2_m2m_queue_ctx *q_ctx,
> +struct vb2_v4l2_buffer *vbuf)
I'd call this v4l2_m2m_buf_remove_by_buf to be consistent with _by_idx.
Other than that, this looks OK.
Regards,
Hans
> +{
> + struct v4l2_m2m_buffer *b;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
> + b = container_of(vbuf, struct v4l2_m2m_buffer, vb);
> + list_del(&b->list);
> + q_ctx->num_rdy--;
> + spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_exact);
> +
> +struct vb2_v4l2_buffer *
> +v4l2_m2m_buf_remove_by_idx(struct v4l2_m2m_queue_ctx *q_ctx, unsigned int
> idx)
> +
> +{
> + struct v4l2_m2m_buffer *b, *tmp;
> + struct vb2_v4l2_buffer *ret = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
> + list_for_each_entry_safe(b, tmp, &q_ctx->rdy_queue, list) {
> + if (b->vb.vb2_buf.index == idx) {
> + list_del(&b->list);
> + q_ctx->num_rdy--;
> + ret = &b->vb;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_by_idx);
> +
> /*
> * Scheduling handlers
> */
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 3ccd01bd245e..c8632c52d5e2 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -437,6 +437,41 @@ static inline void *v4l2_m2m_next_dst_buf(struct
> v4l2_m2m_ctx *m2m_ctx)
> }
>
> /**
> + * v4l2_m2m_for_each_dst_buf() - iterate over a list of destination ready
> + * buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> + */
> +#define v4l2_m2m_for_each_dst_buf(m2m_ctx, b)\
> + list_for_each_entry(b, &m2m_ctx->cap_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_src_buf() - iterate over a list of source ready buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> + */
> +#define v4l2_m2m_for_each_src_buf(m2m_ctx, b)\
> + list_for_each_entry(b, &m2m_ctx->out_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_dst_buf_safe() - iterate over a list of destination
> ready
> + * buffers safely
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> + */
> +#define v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, b, n)\
> + list_for_each_entry_safe(b, n, &m2m_ctx->cap_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_src_buf_safe() - iterate over a list of source ready
> + * buffers safely
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> + */
> +#define v4l2_m2m_for_each_src_buf_safe(m2m_ctx, b, n)\
> + list_for_each_entry_safe(b, n, &m2m_ctx->out_q_ctx.rdy_queue, list)
> +
> +/**
> * v4l2_m2m_get_src_vq() - return vb2_queue for source buffers
> *
> * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> @@ -488,6 +523,54 @@ static inline void *v4l2_m2m_dst_buf_remove(struct
> v4l2_m2m_ctx *m2m_ctx)
> return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
> }
>
> +/**
> + * v4l2_m2m_buf_remove_exact() - take off exact buffer from the list of ready
> + * buffers
> + *
> + * @q_ctx: pointer to struct @v4l2_m2m_queue_ctx
> + */
> +void v4l2_m2m_buf_remove_exact(struct v4l2_m2m_queue_ctx *q_ctx,
> +struct vb2_v4l2_buffer *vbuf);
> +
> +/**
> + * v4l2_m2m_src_buf_remove_exact() - take off exact source buffer from the
> list
> + * of ready buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct
> &v4l2_m2m_ctx
> + */
> +static inline void v4l2_m2m_src_buf_remove_exact(struct v4l2_m2m_ctx
> *m2m_ctx,
> + struct vb2_v4l2_buffer *vbuf)
> +{
> + v4l2_m2m_buf_remove_exact(&m2m_ctx->out_q_ctx,
Re: [PATCH] UVC Module - Support Intel RealSense SR300 Depth Camera formats
Hi Evgeni, Thank you for the patch. On Monday 05 Dec 2016 10:06:55 Raikhel, Evgeni wrote: > Specify GUID and FourCC codes mapping for Depth-related pixel formats > advertised by Intel RealSense(tm) SR300 depth camera. Provide documentation > for the new INZI pixel format introduced. Could you please resend the patches inline instead of as attachments ? See Documentation/SubmittingPatches for more information. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] uvcvideo: add a metadata device node
Hi Guennadi,
Thank you for the patch.
On Friday 02 Dec 2016 11:53:23 Guennadi Liakhovetski wrote:
> Some UVC video cameras contain metadata in their payload headers. This
> patch extracts that data, skipping the standard part of the header, on
> both bulk and isochronous endpoints and makes it available to the user
> space on a separate video node, using the V4L2_CAP_META_CAPTURE
> capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> though different cameras will have different metadata formats, we use
> the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> parse data, based on the specific camera model information.
>
> Signed-off-by: Guennadi Liakhovetski
> ---
>
> v2:
> - updated to the current media/master
> - removed superfluous META capability from capture nodes
> - now the complete UVC payload header is copied to buffers, including
> standard fields
>
> Still open for discussion: is this really OK to always create an
> additional metadata node for each UVC camera or UVC video interface.
>
> IIUC, Laurent's metadata node patch
> https://patchwork.linuxtv.org/patch/36810/ has been acked by Hans and the
> only thing, that's preventing it from being merged it the lack of
> documentation. While waiting for documentation, I'd appreciate some
> discussion of this patch to beat it into shape soon enough and have it
> ready for merge soon after Laurent's patches are pulled in.
>
> Thanks
> Guennadi
>
> drivers/media/usb/uvc/Makefile | 2 +-
> drivers/media/usb/uvc/uvc_driver.c | 10 ++
> drivers/media/usb/uvc/uvc_isight.c | 2 +-
> drivers/media/usb/uvc/uvc_metadata.c | 228 +++
> drivers/media/usb/uvc/uvc_video.c| 47 ++--
> drivers/media/usb/uvc/uvcvideo.h | 12 +-
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/uapi/linux/uvcvideo.h| 10 ++
> include/uapi/linux/videodev2.h | 3 +
> 9 files changed, 304 insertions(+), 11 deletions(-)
> create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 04bf350..edb67ac 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1866,6 +1866,9 @@ static void uvc_unregister_video(struct uvc_device
> *dev)
>
> video_unregister_device(&stream->vdev);
>
> + if (video_is_registered(&stream->meta.vdev))
> + video_unregister_device(&stream->meta.vdev);
video_unregister_device() contains a video_is_registered(), no need to
duplicate it.
> +
> uvc_debugfs_cleanup_stream(stream);
> }
>
> @@ -1926,6 +1929,13 @@ static int uvc_register_video(struct uvc_device *dev,
> return ret;
> }
>
> + /*
> + * Register a metadata node. TODO: shall this only be enabled for some
> + * cameras?
> + */
> + if (!(dev->quirks & UVC_QUIRK_BUILTIN_ISIGHT))
> + uvc_meta_register(stream);
> +
I think so, only for the cameras that can produce metadata.
> if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
> else
[snip]
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c
> b/drivers/media/usb/uvc/uvc_metadata.c new file mode 100644
> index 000..ddf77d9
> --- /dev/null
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -0,0 +1,228 @@
> +/*
> + * uvc_metadata.c -- USB Video Class driver - Metadata handling
> + *
> + * Copyright (C) 2016
> + * Guennadi Liakhovetski (guennadi.liakhovet...@intel.com)
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License as published
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +#include "uvcvideo.h"
> +
> +static inline struct uvc_buffer *to_uvc_buffer(struct vb2_v4l2_buffer
> *vbuf)
> +{
> + return container_of(vbuf, struct uvc_buffer, buf);
> +}
You can move this function to uvcvideo.h and use it in uvc_queue.c (a separate
patch would be best).
> +/*
> + * videobuf2 Queue Operations
> + */
> +
> +static int meta_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
> + unsigned int *nplanes, unsigned int sizes[],
> + struct device *alloc_devs[])
> +{
> + if (*nplanes) {
> + if (*nplanes != 1)
> + return -EINVAL;
> +
> + if (sizes[0] < UVC_PAYLOAD_HEADER_MAX_SIZE)
> + return -EINVAL;
> +
> + return 0;
> + }
> +
> + *nplanes = 1;
> + sizes[0] = UVC_PAYLOAD_HEADER_MAX_SIZE;
>
Re: [PATCH v3 07/10] [media] st-delta: rpmsg ipc support
On 11/22/2016 04:53 PM, Hugues Fruchet wrote: > IPC (Inter Process Communication) support for communication with > DELTA coprocessor firmware using rpmsg kernel framework. > Based on 4 services open/set_stream/decode/close and their associated > rpmsg messages. > The messages structures are duplicated on both host and firmware > side and are packed (use only of 32 bits size fields in messages > structures to ensure packing). > Each service is synchronous; service returns only when firmware > acknowledges the associated command message. > Due to significant parameters size exchanged from host to copro, > parameters are not inserted in rpmsg messages. Instead, parameters are > stored in physical memory shared between host and coprocessor. > Memory is non-cacheable, so no special operation is required > to ensure memory coherency on host and on coprocessor side. > Multi-instance support and re-entrance are ensured using host_hdl and > copro_hdl in message header exchanged between both host and coprocessor. > This avoids to manage tables on both sides to get back the running context > of each instance. > > Signed-off-by: Hugues Fruchet > --- > drivers/media/platform/Kconfig| 1 + > drivers/media/platform/sti/delta/Makefile | 2 +- > drivers/media/platform/sti/delta/delta-ipc.c | 590 > ++ > drivers/media/platform/sti/delta/delta-ipc.h | 76 > drivers/media/platform/sti/delta/delta-v4l2.c | 11 + > drivers/media/platform/sti/delta/delta.h | 21 + > 6 files changed, 700 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/platform/sti/delta/delta-ipc.c > create mode 100644 drivers/media/platform/sti/delta/delta-ipc.h > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index f494f01..5519442 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -303,6 +303,7 @@ config VIDEO_STI_DELTA > depends on VIDEO_DEV && VIDEO_V4L2 > depends on ARCH_STI || COMPILE_TEST > depends on HAS_DMA > + depends on RPMSG This should be 'select', not 'depends on'. > select VIDEOBUF2_DMA_CONTIG > select V4L2_MEM2MEM_DEV > help Can you make a v3.1 of this patch correcting this? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/10] Add support for DELTA video decoder of STMicroelectronics STiH4xx SoC series
Hi Hugues, On 11/22/2016 04:53 PM, Hugues Fruchet wrote: > This patchset introduces a basic support for DELTA multi-format video > decoder of STMicroelectronics STiH4xx SoC series. > > DELTA hardware IP is controlled by a remote firmware loaded in a ST231 > coprocessor. Communication with firmware is done within an IPC layer > using rpmsg kernel framework and a shared memory for messages handling. > This driver is compatible with firmware version 21.1-3. > While a single firmware is loaded in ST231 coprocessor, it is composed > of several firmwares, one per video format family. > > This DELTA V4L2 driver is designed around files: > - delta-v4l2.c : handles V4L2 APIs using M2M framework and calls decoder > ops > - delta-* : implements decoder calling its associated > video firmware (for ex. MJPEG) using IPC layer > - delta-ipc.c: IPC layer which handles communication with firmware > using rpmsg > > This first basic support implements only MJPEG hardware acceleration but > the driver structure is in place to support all the features of the > DELTA video decoder hardware IP. > > This driver depends on: > - ST remoteproc/rpmsg: patchset posted at https://lkml.org/lkml/2016/9/6/77 > - ST DELTA firmware: its license is under review. When available, > pull request will be done on linux-firmware. > > === > = history = > === > version 3 > - update after v2 review: > - fixed m2m_buf_done missing on start_streaming error case > - fixed q->dev missing in queue_init() > - removed unsupported s_selection > - refactored string namings in delta-debug.c > - fixed space before comment > - all commits have commit messages > - reword memory allocator helper commit > > version 2 > - update after v1 review: > - simplified tracing > - G_/S_SELECTION reworked to fit COMPOSE(CAPTURE) > - fixed m2m_buf_done missing on start_streaming error case > - fixed q->dev missing in queue_init() > - switch to kernel-4.9 rpmsg API > - DELTA support added in multi_v7_defconfig > - minor typo fixes & code cleanup > > version 1: > - Initial submission > > === > = v4l2-compliance = > === > Below is the v4l2-compliance report for the version 3 of the DELTA video > decoder driver. v4l2-compliance has been build from SHA1: > 600492351ddf40cc524aab73802153674d7d287b (libdvb5: Fix multiple definition of > dvb_dev_remote_init linking error) Can you update v4l-utils and run this test again? The S_SELECTION compliance test has been fixed to allow for ENOTTY as the error code. If the output looks good, then I'll merge this driver (will be for 4.11). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay
In asds configured with V4L2_ASYNC_MATCH_OF, the v4l2 subdev can be
part of a devicetree overlay, for example:
&media_bridge {
...
my_port: port@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
ep: endpoint@0 {
remote-endpoint = <&camera0>;
};
};
};
/ {
fragment@0 {
target = <&i2c0>;
__overlay__ {
my_cam {
compatible = "foo,bar";
port {
camera0: endpoint {
remote-endpoint = <&my_port>;
...
};
};
};
};
};
};
Each time the overlay is applied, its of_node pointer will be
different. We are not interested in matching the pointer, what we
want to match is that the path is the one we are expecting. Change to
use of_node_cmp() so that we continue matching after the overlay has
been removed and reapplied.
Cc: Mauro Carvalho Chehab
Cc: Javier Martinez Canillas
Cc: Sakari Ailus
Signed-off-by: Javi Merino
---
drivers/media/v4l2-core/v4l2-async.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c
b/drivers/media/v4l2-core/v4l2-async.c
index 5bada20..d33a17c 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
{
- return sd->of_node == asd->match.of.node;
+ return !of_node_cmp(of_node_full_name(sd->of_node),
+ of_node_full_name(asd->match.of.node));
}
static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
--
2.1.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
[PATCH] UVC Module - Support Intel RealSense SR300 Depth Camera formats
Specify GUID and FourCC codes mapping for Depth-related pixel formats
advertised by Intel RealSense(tm) SR300 depth camera.
Provide documentation for the new INZI pixel format introduced.
-
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
From e9693dd008bdaaafa2e2b57762cb75f84649de2e Mon Sep 17 00:00:00 2001
From: Aviv Greenberg
Date: Tue, 15 Nov 2016 12:10:09 +0200
Subject: [PATCH 1/2] UVC: Add support for Intel SR300 depth camera
Add support for Intel SR300 depth camera in uvc driver.
This includes adding three uvc GUIDs for the required pixel formats,
adding a new V4L pixel format definition to user api headers,
and updating the uvc driver GUID-to-4cc tables with the new formats.
Signed-off-by: Aviv Greenberg
Signed-off-by: Evgeni Raikhel
---
drivers/media/usb/uvc/uvc_driver.c | 15 +++
drivers/media/usb/uvc/uvcvideo.h | 9 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 25 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 11744f92097b..5b96a89f29ae 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -168,6 +168,21 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid = UVC_GUID_FORMAT_RW10,
.fcc = V4L2_PIX_FMT_SRGGB10P,
},
+ {
+ .name = "Depth data 16-bit (Z16)",
+ .guid = UVC_GUID_FORMAT_INVZ,
+ .fcc = V4L2_PIX_FMT_Z16,
+ },
+ {
+ .name = "IR:Depth 26-bit (INZI)",
+ .guid = UVC_GUID_FORMAT_INZI,
+ .fcc = V4L2_PIX_FMT_INZI,
+ },
+ {
+ .name = "Greyscale 10-bit (Y10 )",
+ .guid = UVC_GUID_FORMAT_INVI,
+ .fcc = V4L2_PIX_FMT_Y10,
+ },
};
/*
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 7e4d3eea371b..460b99ca99b7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -131,6 +131,15 @@
#define UVC_GUID_FORMAT_RW10 \
{ 'R', 'W', '1', '0', 0x00, 0x00, 0x10, 0x00, \
0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_INVZ \
+ { 'I', 'N', 'V', 'Z', 0x90, 0x2d, 0x58, 0x4a, \
+ 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
+#define UVC_GUID_FORMAT_INZI \
+ { 'I', 'N', 'Z', 'I', 0x66, 0x1a, 0x42, 0xa2, \
+ 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
+#define UVC_GUID_FORMAT_INVI \
+ { 'I', 'N', 'V', 'I', 0xdb, 0x57, 0x49, 0x5e, \
+ 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
/*
* Driver specific constants.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index d3f613e2c54a..4ab995bbec5b 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -659,6 +659,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Y12I v4l2_fourcc('Y', '1', '2', 'I') /* Greyscale 12-bit L/R interleaved */
#define V4L2_PIX_FMT_Z16 v4l2_fourcc('Z', '1', '6', ' ') /* Depth data 16-bit */
#define V4L2_PIX_FMT_MT21Cv4l2_fourcc('M', 'T', '2', '1') /* Mediatek compressed block mode */
+#define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Infrared 10-bit linked with Depth 16-bit */
/* SDR formats - used only for Software Defined Radio devices */
#define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
--
2.7.4
From 581f4c3e60d8e7895bc34f9e0e90476eed31fa8d Mon Sep 17 00:00:00 2001
From: Evgeni Raikhel
Date: Wed, 16 Nov 2016 11:53:49 +0200
Subject: [PATCH 2/2] Document Intel SR300 Depth camera INZI format
Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
format utilized by Intel SR300 Depth camera.
This is a complimentary patch for:
[PATCH] UVC: Add support for Intel SR300 depth camera
Signed-off-by: Evgeni Raikhel
---
Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40
1 file changed, 40 insertions(+)
create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
new file mode 100644
index ..cdfdeae4a664
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
@@ -0,0 +1,40 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-INZI:
+
+**
+V4L2_PIX_FMT_INZI ('INZI')
+**
+
+Infrared 10-bit linked with Depth 16-bit images
+
+
+Description
+===
+
+Custom multi-planar format used by Intel SR300 Depth cameras, comprise of Infrared image followed by Depth data.
+The pixel definition is 32-bpp, with the Depth and Infrared Data split into separate continuous planes
