* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-06 6:24 UTC (permalink / raw)
To: Sakari Ailus
Cc: linus.walleij, bgolaszewski, mchehab, andriy.shevchenko, robh+dt,
mark.rutland, drinkcat, tfiga, matthias.bgg, bingbu.cao,
srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang
In-Reply-To: <20200605121459.GS16711@paasikivi.fi.intel.com>
Hi Sakari,
Thanks for the timely review.
On Fri, 2020-06-05 at 15:14 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> Thank you for the update.
>
> On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> > Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> > control to set the desired focus via IIC serial interface.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/i2c/Kconfig | 13 ++
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/dw9768.c | 566 +++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 581 insertions(+)
> > create mode 100644 drivers/media/i2c/dw9768.c
> >
[snip]...
> > +
> > +static int dw9768_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct dw9768 *dw9768;
> > + u32 aac_mode_select;
> > + u32 aac_timing_select;
> > + u32 clock_presc_select;
> > + unsigned int i;
> > + int ret;
> > +
> > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > + if (!dw9768)
> > + return -ENOMEM;
> > +
> > + /* Initialize subdev */
> > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > +
> > + dw9768->aac_mode = DW9768_AAC_MODE_DEFAULT;
> > + dw9768->aac_timing = DW9768_AAC_TIME_DEFAULT;
> > + dw9768->clock_presc = DW9768_CLOCK_PRE_SCALE_DEFAULT;
> > +
> > + /* Optional indication of AAC mode select */
> > + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-mode",
> > + &aac_mode_select);
> > +
> > + if (!ret)
> > + dw9768->aac_mode = aac_mode_select;
> > +
> > + /* Optional indication of clock pre-scale select */
> > + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,clock-presc",
> > + &clock_presc_select);
> > +
> > + if (!ret)
> > + dw9768->clock_presc = clock_presc_select;
> > +
> > + /* Optional indication of AAC Timing */
> > + ret = fwnode_property_read_u32(dev_fwnode(dev), "dongwoon,aac-timing",
> > + &aac_timing_select);
> > +
> > + if (!ret)
> > + dw9768->aac_timing = aac_timing_select;
>
> You can assign the defaults to the dw9768 struct and use the fwnode
> property API to read the properties into the same fields. No return values
> need to be checked.
>
Good idea :-)
> > +
> > + for (i = 0; i < ARRAY_SIZE(dw9768_supply_names); i++)
> > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > +
> > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(dw9768_supply_names),
> > + dw9768->supplies);
> > + if (ret) {
> > + dev_err(dev, "failed to get regulators\n");
> > + return ret;
> > + }
> > +
> > + /* Initialize controls */
> > + ret = dw9768_init_controls(dw9768);
> > + if (ret)
> > + goto err_free_handler;
> > +
> > + /* Initialize subdev */
> > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > +
> > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > + if (ret < 0)
> > + goto err_free_handler;
> > +
> > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > +
> > + pm_runtime_enable(dev);
> > + if (!pm_runtime_enabled(dev)) {
> > + ret = dw9768_runtime_resume(dev);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to power on: %d\n", ret);
> > + goto err_clean_entity;
> > + }
> > + }
> > +
> > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > + goto error_async_register;
> > + }
> > +
> > + return 0;
> > +
> > +error_async_register:
> > + if (!pm_runtime_enabled(dev))
> > + dw9768_runtime_suspend(dev);
> > +err_clean_entity:
> > + media_entity_cleanup(&dw9768->sd.entity);
> > +err_free_handler:
> > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > +
> > + return ret;
> > +}
> > +
[snip]...
^ permalink raw reply
* Re: [V7, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-06 6:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linus.walleij, bgolaszewski, mchehab, robh+dt, mark.rutland,
sakari.ailus, drinkcat, tfiga, matthias.bgg, bingbu.cao,
srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu
In-Reply-To: <20200605124643.GG2428291@smile.fi.intel.com>
Hi Andy,
Thanks for the review. My replies are as below.
On Fri, 2020-06-05 at 15:46 +0300, Andy Shevchenko wrote:
> On Fri, Jun 05, 2020 at 06:54:12PM +0800, Dongchun Zhu wrote:
> > Add a V4L2 sub-device driver for DW9768 voice coil motor, providing
> > control to set the desired focus via IIC serial interface.
>
> ...
>
> > +config VIDEO_DW9768
> > + tristate "DW9768 lens voice coil support"
> > + depends on I2C && VIDEO_V4L2
>
> No compile test?
>
Sorry?
Kconfig here is based on the current media tree master branch.
It is also what the other similar drivers from Dongwoon do.
> > + depends on PM
>
> This is very strange dependency for ordinary driver.
>
Thanks for the reminder.
This would be removed in next release.
As dw9768_runtime_resume/suspend would be called if runtime PM is
disabled.
> > + select MEDIA_CONTROLLER
> > + select VIDEO_V4L2_SUBDEV_API
> > + select V4L2_FWNODE
>
> ...
>
> > +/*
> > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > + * or in the case of PD reset taking place.
> > + */
> > +#define DW9768_T_OPR_US 1000
> > +#define DW9768_Tvib_MS_BASE10 (64 - 1)
> > +#define DW9768_AAC_MODE_DEFAULT 2
>
> > +#define DW9768_AAC_TIME_DEFAULT 0x20
>
> Hex? Why not decimal?
>
There is one optional property 'dongwoon,aac-timing' defined in DT.
I don't know whether you have noticed that.
'DW9768_AAC_TIME_DEFAULT' is the value set to AACT[5:0] register.
I thought the Hex unit should be proper as it is directly written to the
Hex register.
> > +#define DW9768_CLOCK_PRE_SCALE_DEFAULT 1
>
> ...
>
> > +static int dw9768_mod_reg(struct dw9768 *dw9768, u8 reg, u8 mask, u8 val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret;
> > +
> > + ret = i2c_smbus_read_byte_data(client, reg);
> > + if (ret < 0)
> > + return ret;
> > +
>
> > + val = ((unsigned char)ret & ~mask) | (val & mask);
>
> This cast is weird.
>
Thanks for the review, but this cast is using classical pattern from
your suggestion on OV02A10 v5:
https://patchwork.linuxtv.org/patch/59788/
So I wonder whether it is still required to be refined currently.
Or what would it be supposed to do for the cast?
> > +
> > + return i2c_smbus_write_byte_data(client, reg, val);
> > +}
>
> ...
>
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
>
> One line?
>
Splitting the sentence into two lines or not should both be okay.
Of course, I could put them in one line in next release.
> ...
>
> > +static int dw9768_release(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + u32 move_delay_us = dw9768_cal_move_delay(dw9768->aac_mode,
> > + dw9768->clock_presc,
> > + dw9768->aac_timing) / 100;
> > + int ret, val;
> > +
> > + val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > + for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "I2C write fail: %d", ret);
> > + return ret;
> > + }
> > + usleep_range(move_delay_us, move_delay_us + 1000);
> > + }
>
>
> It will look more naturally in the multiplier kind of value.
>
> unsigned int steps = DIV_ROUND_UP(...);
>
> while (steps--) {
> ...(..., steps * ..._MOVE_STEPS);
> ...
> }
>
> but double check arithmetics.
>
The current coding style is actually updated with reference to your
previous comments on DW9768 v3:
https://patchwork.linuxtv.org/patch/61856/
> > + return 0;
> > +}
>
>
> Also it seems we need to have writex_poll_timeout() implementation (see
> iopoll.h).
>
^ permalink raw reply
* Re: [PATCH] media: cedrus: Add support for additional output formats
From: Ezequiel Garcia @ 2020-06-06 5:40 UTC (permalink / raw)
To: Jernej Skrabec
Cc: Maxime Ripard, Paul Kocialkowski, Mauro Carvalho Chehab, Greg KH,
Chen-Yu Tsai, Hans Verkuil, linux-media, devel, linux-arm-kernel,
Linux Kernel Mailing List
In-Reply-To: <20200520171457.11937-1-jernej.skrabec@siol.net>
Hello Jernej,
Thanks for the patch.
On Wed, 20 May 2020 at 14:12, Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> If VPU supports untiled output, it actually supports several different
> YUV 4:2:0 layouts, namely NV12, NV21, YUV420 and YVU420.
>
> Add support for all of them.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 18 +++++++++++++++++-
> .../staging/media/sunxi/cedrus/cedrus_video.c | 18 ++++++++++++++++++
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index daf5f244f93b..c119fd8c4b92 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -83,9 +83,25 @@ void cedrus_dst_format_set(struct cedrus_dev *dev,
>
> switch (fmt->pixelformat) {
> case V4L2_PIX_FMT_NV12:
> + case V4L2_PIX_FMT_NV21:
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
> chroma_size = ALIGN(width, 16) * ALIGN(height, 16) / 2;
>
> - reg = VE_PRIMARY_OUT_FMT_NV12;
> + switch (fmt->pixelformat) {
> + case V4L2_PIX_FMT_NV12:
> + reg = VE_PRIMARY_OUT_FMT_NV12;
> + break;
> + case V4L2_PIX_FMT_NV21:
> + reg = VE_PRIMARY_OUT_FMT_NV21;
> + break;
> + case V4L2_PIX_FMT_YUV420:
> + reg = VE_PRIMARY_OUT_FMT_YU12;
> + break;
> + case V4L2_PIX_FMT_YVU420:
> + reg = VE_PRIMARY_OUT_FMT_YV12;
> + break;
> + }
> cedrus_write(dev, VE_PRIMARY_OUT_FMT, reg);
>
I think it would result in a cleaner code if you extend
cedrus_format to include the hw_format.
Something along these lines (not a complete patch):
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 15cf1f10221b..618daaa65a82 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -48,10 +48,12 @@ static struct cedrus_format cedrus_formats[] = {
},
{
.pixelformat = V4L2_PIX_FMT_SUNXI_TILED_NV12,
+ .hw_format = VE_PRIMARY_OUT_FMT_TILED_32_NV12,
.directions = CEDRUS_DECODE_DST,
},
{
.pixelformat = V4L2_PIX_FMT_NV12,
+ .hw_format = VE_PRIMARY_OUT_FMT_NV12,
.directions = CEDRUS_DECODE_DST,
.capabilities = CEDRUS_CAPABILITY_UNTILED,
},
@@ -274,6 +276,7 @@ static int cedrus_s_fmt_vid_cap(struct file *file,
void *priv,
{
struct cedrus_ctx *ctx = cedrus_file2ctx(file);
struct cedrus_dev *dev = ctx->dev;
+ struct cedrus_format *fmt;
struct vb2_queue *vq;
int ret;
@@ -287,7 +290,10 @@ static int cedrus_s_fmt_vid_cap(struct file
*file, void *priv,
ctx->dst_fmt = f->fmt.pix;
- cedrus_dst_format_set(dev, &ctx->dst_fmt);
+ fmt = cedrus_find_format(ctx->dst_fmt.pixelformat,
+ CEDRUS_DECODE_DST,
+ dev->capabilities);
+ cedrus_dst_format_set(dev, fmt);
return 0;
}
So then in cedrus_dst_format_set() you can just
write VE_PRIMARY_OUT_FMT with fmt->hw_format.
> reg = chroma_size / 2;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 15cf1f10221b..016021d71df2 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -55,6 +55,21 @@ static struct cedrus_format cedrus_formats[] = {
> .directions = CEDRUS_DECODE_DST,
> .capabilities = CEDRUS_CAPABILITY_UNTILED,
> },
> + {
> + .pixelformat = V4L2_PIX_FMT_NV21,
> + .directions = CEDRUS_DECODE_DST,
> + .capabilities = CEDRUS_CAPABILITY_UNTILED,
> + },
> + {
> + .pixelformat = V4L2_PIX_FMT_YUV420,
> + .directions = CEDRUS_DECODE_DST,
> + .capabilities = CEDRUS_CAPABILITY_UNTILED,
> + },
> + {
> + .pixelformat = V4L2_PIX_FMT_YVU420,
> + .directions = CEDRUS_DECODE_DST,
> + .capabilities = CEDRUS_CAPABILITY_UNTILED,
> + },
> };
>
> #define CEDRUS_FORMATS_COUNT ARRAY_SIZE(cedrus_formats)
> @@ -130,6 +145,9 @@ void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt)
> break;
>
> case V4L2_PIX_FMT_NV12:
> + case V4L2_PIX_FMT_NV21:
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YVU420:
> /* 16-aligned stride. */
> bytesperline = ALIGN(width, 16);
>
> --
> 2.26.2
>
Thanks,
Ezequiel
^ permalink raw reply related
* cron job: media_tree daily build: WARNINGS
From: Hans Verkuil @ 2020-06-06 3:41 UTC (permalink / raw)
To: linux-media
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: Sat Jun 6 05:00:14 CEST 2020
media-tree git hash: 938b29db3aa9c293c7c1366b16e55e308f1a1ddd
media_build git hash: 337283131d6117aa9b0c0c62d32e323da54a9359
v4l-utils git hash: 74377da4f5f3b63203c599d5dd75db6af91fdbb9
edid-decode git hash: f20c85d7b4c537e0d458f85c4da9f45cd3c0fbd2
gcc version: i686-linux-gcc (GCC) 9.3.0
sparse repo: https://git.linuxtv.org/mchehab/sparse.git
sparse version: 0.6.1
smatch repo: https://git.linuxtv.org/mchehab/smatch.git
smatch version: 0.6.1-rc1
build-scripts repo: https://git.linuxtv.org/hverkuil/build-scripts.git
build-scripts git hash: a0e63d2e6cdc689a8af8c9ae6df1674d0fe38c74
host hardware: x86_64
host os: 5.6.0-1-amd64
linux-git-sh: OK
linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-stm32: OK
linux-git-arm-pxa: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-arm64: OK
linux-git-arm-multi: OK
linux-git-i686: WARNINGS
linux-git-x86_64: WARNINGS
Check COMPILE_TEST: OK
Check for strcpy/strncpy/strlcpy: WARNINGS: found 4 strcpy(), 4 strncpy(), 4 strlcpy()
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.81-i686: OK
linux-3.16.81-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.136-i686: OK
linux-3.18.136-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.52-i686: OK
linux-4.1.52-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.212-i686: OK
linux-4.4.212-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.212-i686: OK
linux-4.9.212-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.169-i686: OK
linux-4.14.169-x86_64: OK
linux-4.15.18-i686: OK
linux-4.15.18-x86_64: OK
linux-4.16.18-i686: OK
linux-4.16.18-x86_64: OK
linux-4.17.19-i686: OK
linux-4.17.19-x86_64: OK
linux-4.18.20-i686: OK
linux-4.18.20-x86_64: OK
linux-4.19.101-i686: OK
linux-4.19.101-x86_64: OK
linux-4.20.15-i686: OK
linux-4.20.15-x86_64: OK
linux-5.0.15-i686: OK
linux-5.0.15-x86_64: OK
linux-5.1.1-i686: OK
linux-5.1.1-x86_64: OK
linux-5.2.1-i686: OK
linux-5.2.1-x86_64: OK
linux-5.3.1-i686: OK
linux-5.3.1-x86_64: OK
linux-5.4.17-i686: OK
linux-5.4.17-x86_64: OK
linux-5.5.1-i686: OK
linux-5.5.1-x86_64: OK
linux-5.6.1-i686: OK
linux-5.6.1-x86_64: OK
linux-5.7-rc1-i686: OK
linux-5.7-rc1-x86_64: OK
apps: OK
spec-git: OK
virtme: WARNINGS: Final Summary: 2943, Succeeded: 2943, Failed: 0, Warnings: 2
virtme-32: WARNINGS: Final Summary: 2779, Succeeded: 2779, Failed: 0, Warnings: 1
sparse: OK
smatch: OK
Detailed results are available here:
http://www.xs4all.nl/~hverkuil/logs/Saturday.log
Detailed regression test results are available here:
http://www.xs4all.nl/~hverkuil/logs/Saturday-test-media.log
http://www.xs4all.nl/~hverkuil/logs/Saturday-test-media-32.log
http://www.xs4all.nl/~hverkuil/logs/Saturday-test-media-dmesg.log
Full logs are available here:
http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2
The Media Infrastructure API from this daily build is here:
http://www.xs4all.nl/~hverkuil/spec/index.html
^ permalink raw reply
* [PATCH v4 00/12] PCI: brcmstb: enable PCIe for STB chips
From: Jim Quinlan @ 2020-06-05 21:26 UTC (permalink / raw)
To: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, james.quinlan
Cc: Alan Stern, Andrew Morton, Andy Shevchenko, Arnd Bergmann,
Dan Williams, David Kershner, open list:STAGING SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE,
open list:DRM DRIVERS FOR ALLWINNER A10, Florian Fainelli,
Greg Kroah-Hartman, Hans de Goede, H. Peter Anvin,
open list:IOMMU DRIVERS, Jens Axboe, Julien Grall,
open list:ACPI FOR ARM64 (ACPI/arm64), moderated list:ARM PORT,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, open list:ALLWINNER A10 CSI DRIVER,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:SUPERH, open list:USB SUBSYSTEM, Mark Brown,
Oliver Neukum, Rafael J. Wysocki, Rob Herring, Robin Murphy,
Saravana Kannan, Stefano Stabellini, Suzuki K Poulose,
Ulf Hansson
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1] https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101024@gmail.com/
Jim Quinlan (12):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 register info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
device core: Introduce multiple dma pfn offsets
PCI: brcmstb: Set internal memory viewport sizes
PCI: brcmstb: Accommodate MSI for older chips
PCI: brcmstb: Set bus max burst size by chip type
PCI: brcmstb: Add bcm7211, bcm7216, bcm7445, bcm7278 to match list
.../bindings/pci/brcm,stb-pcie.yaml | 58 ++-
arch/arm/include/asm/dma-mapping.h | 9 +-
arch/arm/mach-keystone/keystone.c | 9 +-
arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
arch/sh/kernel/dma-coherent.c | 14 +-
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/ata/ahci_brcm.c | 14 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
drivers/iommu/io-pgtable-arm.c | 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +-
drivers/of/address.c | 72 +++-
drivers/of/device.c | 19 +-
drivers/of/of_private.h | 11 +-
drivers/of/unittest.c | 8 +-
drivers/pci/controller/Kconfig | 3 +-
drivers/pci/controller/pcie-brcmstb.c | 408 +++++++++++++++---
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c | 7 +-
drivers/usb/core/message.c | 4 +-
drivers/usb/core/usb.c | 2 +-
include/linux/device.h | 4 +-
include/linux/dma-direct.h | 16 +-
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +-
kernel/dma/mapping.c | 38 ++
27 files changed, 647 insertions(+), 131 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCHv3 1/5] media: docs-rst: Document memory-to-memory video encoder interface
From: Nicolas Dufresne @ 2020-06-05 21:24 UTC (permalink / raw)
To: Stanimir Varbanov, Hans Verkuil, linux-media; +Cc: Tomasz Figa, Michael Tretter
In-Reply-To: <728b621a-0df7-5c6f-9135-8b9794035b95@linaro.org>
Le vendredi 05 juin 2020 à 10:19 +0300, Stanimir Varbanov a écrit :
>
> On 5/26/20 1:09 PM, Hans Verkuil wrote:
> > From: Tomasz Figa <tfiga@chromium.org>
> >
> > Due to complexity of the video encoding process, the V4L2 drivers of
> > stateful encoder hardware require specific sequences of V4L2 API calls
> > to be followed. These include capability enumeration, initialization,
> > encoding, encode parameters change, drain and reset.
> >
> > Specifics of the above have been discussed during Media Workshops at
> > LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
> > Conference Europe 2014 in Düsseldorf. The de facto Codec API that
> > originated at those events was later implemented by the drivers we already
> > have merged in mainline, such as s5p-mfc or coda.
> >
> > The only thing missing was the real specification included as a part of
> > Linux Media documentation. Fix it now and document the encoder part of
> > the Codec API.
> >
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> > .../userspace-api/media/v4l/dev-encoder.rst | 728 ++++++++++++++++++
> > .../userspace-api/media/v4l/dev-mem2mem.rst | 1 +
> > .../userspace-api/media/v4l/pixfmt-v4l2.rst | 5 +
> > .../userspace-api/media/v4l/v4l2.rst | 2 +
> > .../media/v4l/vidioc-encoder-cmd.rst | 51 +-
> > 5 files changed, 767 insertions(+), 20 deletions(-)
> > create mode 100644 Documentation/userspace-api/media/v4l/dev-encoder.rst
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-encoder.rst b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > new file mode 100644
> > index 000000000000..aace7b812a9c
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/dev-encoder.rst
> > @@ -0,0 +1,728 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +.. _encoder:
> > +
> > +*************************************************
> > +Memory-to-Memory Stateful Video Encoder Interface
> > +*************************************************
> > +
> > +A stateful video encoder takes raw video frames in display order and encodes
> > +them into a bytestream. It generates complete chunks of the bytestream, including
> > +all metadata, headers, etc. The resulting bytestream does not require any
> > +further post-processing by the client.
> > +
> > +Performing software stream processing, header generation etc. in the driver
> > +in order to support this interface is strongly discouraged. In case such
> > +operations are needed, use of the Stateless Video Encoder Interface (in
> > +development) is strongly advised.
> > +
>
> <cut>
>
> > +Encoding Parameter Changes
> > +==========================
> > +
> > +The client is allowed to use :c:func:`VIDIOC_S_CTRL` to change encoder
> > +parameters at any time. The availability of parameters is encoder-specific
> > +and the client must query the encoder to find the set of available controls.
> > +
> > +The ability to change each parameter during encoding is encoder-specific, as
> > +per the standard semantics of the V4L2 control interface. The client may
> > +attempt to set a control during encoding and if the operation fails with the
> > +-EBUSY error code, the ``CAPTURE`` queue needs to be stopped for the
> > +configuration change to be allowed. To do this, it may follow the `Drain`
> > +sequence to avoid losing the already queued/encoded frames.
> > +
> > +The timing of parameter updates is encoder-specific, as per the standard
> > +semantics of the V4L2 control interface. If the client needs to apply the
> > +parameters exactly at specific frame, using the Request API
> > +(:ref:`media-request-api`) should be considered, if supported by the encoder.
>
> In request-api case does the control will return EBUSY immediately when
> S_CTRL is called from the client? I'm asking in the context of the
> controls which are not dynamic (cannot set during streaming and Reset
> sequence is needed).
This is all hypothetical, as nothing of that has been implemented. But
I suppose there should be instant validation to allow that, even if
that means more plumbing inside the kernel. It would be better then
just running the request ignoring silently that control. Ideally
userspace should not have to go into trial and errors, so controls that
are means for the should be marked.
>
> > +
> > +Drain
> > +=====
> > +
> > +To ensure that all the queued ``OUTPUT`` buffers have been processed and the
> > +related ``CAPTURE`` buffers are given to the client, the client must follow the
> > +drain sequence described below. After the drain sequence ends, the client has
> > +received all encoded frames for all ``OUTPUT`` buffers queued before the
> > +sequence was started.
> > +
> > +1. Begin the drain sequence by issuing :c:func:`VIDIOC_ENCODER_CMD`.
> > +
> > + * **Required fields:**
> > +
> > + ``cmd``
> > + set to ``V4L2_ENC_CMD_STOP``.
> > +
> > + ``flags``
> > + set to 0.
> > +
> > + ``pts``
> > + set to 0.
> > +
> > + .. warning::
> > +
> > + The sequence can be only initiated if both ``OUTPUT`` and ``CAPTURE``
> > + queues are streaming. For compatibility reasons, the call to
> > + :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is
> > + not streaming, but at the same time it will not initiate the `Drain`
> > + sequence and so the steps described below would not be applicable.
> > +
> > +2. Any ``OUTPUT`` buffers queued by the client before the
> > + :c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as
> > + normal. The client must continue to handle both queues independently,
> > + similarly to normal encode operation. This includes:
> > +
> > + * queuing and dequeuing ``CAPTURE`` buffers, until a buffer marked with the
> > + ``V4L2_BUF_FLAG_LAST`` flag is dequeued,
> > +
> > + .. warning::
> > +
> > + The last buffer may be empty (with :c:type:`v4l2_buffer`
> > + ``bytesused`` = 0) and in that case it must be ignored by the client,
> > + as it does not contain an encoded frame.
> > +
> > + .. note::
> > +
> > + Any attempt to dequeue more ``CAPTURE`` buffers beyond the buffer
> > + marked with ``V4L2_BUF_FLAG_LAST`` will result in a -EPIPE error from
> > + :c:func:`VIDIOC_DQBUF`.
> > +
> > + * dequeuing processed ``OUTPUT`` buffers, until all the buffers queued
> > + before the ``V4L2_ENC_CMD_STOP`` command are dequeued,
> > +
> > + * dequeuing the ``V4L2_EVENT_EOS`` event, if the client subscribes to it.
> > +
> > + .. note::
> > +
> > + For backwards compatibility, the encoder will signal a ``V4L2_EVENT_EOS``
> > + event when the last frame has been encoded and all frames are ready to be
> > + dequeued. It is deprecated behavior and the client must not rely on it.
> > + The ``V4L2_BUF_FLAG_LAST`` buffer flag should be used instead.
> > +
> > +3. Once all ``OUTPUT`` buffers queued before the ``V4L2_ENC_CMD_STOP`` call are
> > + dequeued and the last ``CAPTURE`` buffer is dequeued, the encoder is stopped
> > + and it will accept, but not process any newly queued ``OUTPUT`` buffers
> > + until the client issues any of the following operations:
> > +
> > + * ``V4L2_ENC_CMD_START`` - the encoder will not be reset and will resume
> > + operation normally, with all the state from before the drain,
> > +
> > + * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > + ``CAPTURE`` queue - the encoder will be reset (see the `Reset` sequence)
> > + and then resume encoding,
> > +
> > + * a pair of :c:func:`VIDIOC_STREAMOFF` and :c:func:`VIDIOC_STREAMON` on the
> > + ``OUTPUT`` queue - the encoder will resume operation normally, however any
> > + source frames queued to the ``OUTPUT`` queue between ``V4L2_ENC_CMD_STOP``
> > + and :c:func:`VIDIOC_STREAMOFF` will be discarded.
> > +
> > +.. note::
> > +
> > + Once the drain sequence is initiated, the client needs to drive it to
> > + completion, as described by the steps above, unless it aborts the process by
> > + issuing :c:func:`VIDIOC_STREAMOFF` on any of the ``OUTPUT`` or ``CAPTURE``
> > + queues. The client is not allowed to issue ``V4L2_ENC_CMD_START`` or
> > + ``V4L2_ENC_CMD_STOP`` again while the drain sequence is in progress and they
> > + will fail with -EBUSY error code if attempted.
> > +
> > + For reference, handling of various corner cases is described below:
> > +
> > + * In case of no buffer in the ``OUTPUT`` queue at the time the
> > + ``V4L2_ENC_CMD_STOP`` command was issued, the drain sequence completes
> > + immediately and the encoder returns an empty ``CAPTURE`` buffer with the
> > + ``V4L2_BUF_FLAG_LAST`` flag set.
> > +
> > + * In case of no buffer in the ``CAPTURE`` queue at the time the drain
> > + sequence completes, the next time the client queues a ``CAPTURE`` buffer
> > + it is returned at once as an empty buffer with the ``V4L2_BUF_FLAG_LAST``
> > + flag set.
> > +
> > + * If :c:func:`VIDIOC_STREAMOFF` is called on the ``CAPTURE`` queue in the
> > + middle of the drain sequence, the drain sequence is canceled and all
> > + ``CAPTURE`` buffers are implicitly returned to the client.
> > +
> > + * If :c:func:`VIDIOC_STREAMOFF` is called on the ``OUTPUT`` queue in the
> > + middle of the drain sequence, the drain sequence completes immediately and
> > + next ``CAPTURE`` buffer will be returned empty with the
> > + ``V4L2_BUF_FLAG_LAST`` flag set.
> > +
> > + Although not mandatory, the availability of encoder commands may be queried
> > + using :c:func:`VIDIOC_TRY_ENCODER_CMD`.
> > +
> > +Reset
> > +=====
> > +
> > +The client may want to request the encoder to reinitialize the encoding, so
> > +that the following stream data becomes independent from the stream data
> > +generated before. Depending on the coded format, that may imply that:
> > +
> > +* encoded frames produced after the restart must not reference any frames
> > + produced before the stop, e.g. no long term references for H.264/HEVC,
> > +
> > +* any headers that must be included in a standalone stream must be produced
> > + again, e.g. SPS and PPS for H.264/HEVC.
>
> It seems that clients needs to know SPS/PPS (for muxers?) and thus they
> will need to extract or parse the encoder output buffers to find them.
> Maybe the spec should consider adding some buffer flag to mark such
> buffers which contain SPS/PPS only.
>
> [1] - see at "type AvcCBox struct"
>
> Nicolas, how the gstreamer handle this?
>
> > +
> > +This can be achieved by performing the reset sequence.
> > +
> > +1. Perform the `Drain` sequence to ensure all the in-flight encoding finishes
> > + and respective buffers are dequeued.
> > +
> > +2. Stop streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMOFF`. This
> > + will return all currently queued ``CAPTURE`` buffers to the client, without
> > + valid frame data.
> > +
> > +3. Start streaming on the ``CAPTURE`` queue via :c:func:`VIDIOC_STREAMON` and
> > + continue with regular encoding sequence. The encoded frames produced into
> > + ``CAPTURE`` buffers from now on will contain a standalone stream that can be
> > + decoded without the need for frames encoded before the reset sequence,
> > + starting at the first ``OUTPUT`` buffer queued after issuing the
> > + `V4L2_ENC_CMD_STOP` of the `Drain` sequence.
> > +
> > +This sequence may be also used to change encoding parameters for encoders
> > +without the ability to change the parameters on the fly.
> > +
>
> <cut>
>
^ permalink raw reply
* Re: [PATCH] rcar-vin: rcar-csi2: Correct the selection of hsfreqrange
From: Michael Rodin @ 2020-06-05 18:44 UTC (permalink / raw)
To: Niklas Söderlund, Mauro Carvalho Chehab, linux-media,
linux-renesas-soc, linux-kernel
Cc: Michael Rodin, michael, efriedrich, erosca, Suresh Udipi
In-Reply-To: <1590596167-17403-1-git-send-email-mrodin@de.adit-jv.com>
Hello Niklas and Suresh,
Renesas confirmed that there is a typo in the Hardware Manual (Table 25.9):
The correct range for 220 Mbps is 197.125-244.125 and not 197.125 - 224.125
so the both patches are correct, we can do configuration based only on
the "default" bit rates. I would say that now we can correct the commit
message with respect to the exception value and use these patches. I would
additionally add a warning for bitrates below 80 Mbps. They seem to work
(for example Raspberry Pi camera), however they are not officially supported
so the user needs to be warned.
On Wed, May 27, 2020 at 06:16:07PM +0200, Michael Rodin wrote:
> From: Suresh Udipi <sudipi@jp.adit-jv.com>
>
> hsfreqrange should be chosen based on the calculated mbps which
> is closer to the default bit rate and within the range as per
> table[1]. But current calculation always selects first value which
> is greater than or equal to the calculated mbps which may lead
> to chosing a wrong range in some cases.
>
> For example for 360 mbps for H3/M3N
> Existing logic selects
> Calculated value 360Mbps : Default 400Mbps Range [368.125 -433.125 mbps]
>
> This hsfreqrange is out of range.
>
> The logic is changed to get the default value which is closest to the
> calculated value [1]
>
> Calculated value 360Mbps : Default 350Mbps Range [320.625 -380.625 mpbs]
>
> [1] specs r19uh0105ej0200-r-car-3rd-generation.pdf [Table 25.9]
>
> There is one exectpion value 227Mbps, which may cause out of
> range. This needs to be further handled if required.
>
> Fixes: ADIT v4.14 commit 9e568b895ee0 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver")
>
> Signed-off-by: Suresh Udipi <sudipi@jp.adit-jv.com>
> Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com>
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index c473a56..73d9830 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -199,6 +199,7 @@ static const struct rcsi2_mbps_reg phtw_mbps_v3m_e3[] = {
> /* PHY Frequency Control */
> #define PHYPLL_REG 0x68
> #define PHYPLL_HSFREQRANGE(n) ((n) << 16)
> +#define PHYPLL_HSFREQRANGE_MAX 1500
>
> static const struct rcsi2_mbps_reg hsfreqrange_h3_v3h_m3n[] = {
> { .mbps = 80, .reg = 0x00 },
> @@ -446,16 +447,23 @@ static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
> static int rcsi2_set_phypll(struct rcar_csi2 *priv, unsigned int mbps)
> {
> const struct rcsi2_mbps_reg *hsfreq;
> + const struct rcsi2_mbps_reg *hsfreq_prev = NULL;
>
> - for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++)
> - if (hsfreq->mbps >= mbps)
> - break;
> -
> - if (!hsfreq->mbps) {
> + if (mbps > PHYPLL_HSFREQRANGE_MAX) {
> dev_err(priv->dev, "Unsupported PHY speed (%u Mbps)", mbps);
> return -ERANGE;
> }
>
> + for (hsfreq = priv->info->hsfreqrange; hsfreq->mbps != 0; hsfreq++) {
> + if (hsfreq->mbps >= mbps)
> + break;
> + hsfreq_prev = hsfreq;
> + }
> +
> + if (hsfreq_prev &&
> + ((mbps - hsfreq_prev->mbps) <= (hsfreq->mbps - mbps)))
> + hsfreq = hsfreq_prev;
> +
> rcsi2_write(priv, PHYPLL_REG, PHYPLL_HSFREQRANGE(hsfreq->reg));
>
> return 0;
> --
> 2.7.4
>
--
Best Regards,
Michael
^ permalink raw reply
* Re: [RFC] METADATA design using V4l2 Request API
From: Nicolas Dufresne @ 2020-06-05 17:43 UTC (permalink / raw)
To: dikshita, Hans Verkuil
Cc: linux-media, stanimir.varbanov, linux-kernel, linux-arm-msm,
vgarodia, majja, jdas, linux-media-owner
In-Reply-To: <5356c146a340d951b8d492373f349199@codeaurora.org>
Le vendredi 05 juin 2020 à 12:32 +0530, dikshita@codeaurora.org a écrit :
> Hi Hans, Nicolas,
>
> On 2020-05-29 13:01, Hans Verkuil wrote:
> > On 29/05/2020 04:18, Nicolas Dufresne wrote:
> > > Le jeudi 28 mai 2020 à 16:18 +0530, dikshita@codeaurora.org a écrit :
> > > > > not allowed. So I need to know more about this.
> > > > > Regards,
> > > > > Hans
> > > >
> > > > we need this for use cases like HDR10+ where metadata info is part of
> > > > the bitstream.
> > > >
> > > > To handle such frame specific data, support for request api on
> > > > capture
> > > > plane would be needed.
> > >
> > > I have a bit of a mixed impression here. Considering how large the
> > > ioctl
> > > interface overhead is, relying on HW parser to extract this medata
> > > woud be the
> > > last thing I would consider.
> > >
> > > Instead, I'm quite convince we can achieve greater and likely
> > > zero-copy
> > > perfromance by locating this header in userspace. So everytime I see
> > > this kind
> > > of API, were the HW is *needed* to parse a trivial bit of bistream, I
> > > ended
> > > thinking that we simply craft this API to expose this because the HW
> > > can do it,
> > > but no further logical thinking or higher level design seems to have
> > > been
> > > applied.
> > >
> > > I'm sorry for this open critic, but are we designing this because the
> > > HW
> > > designer exposed that feature? This is so low complexity to extract in
> > > userspace, with the bonus that we are not forced into expanding the
> > > representation to another form immediatly, as maybe the display will
> > > be able to
> > > handle that form directly (where converting to a C structure and then
> > > back to
> > > some binary format to satisfy DRM property seems very sub-optimal).
> > >
> > > Nicolas
> > >
> >
> > Nicolas raises good questions: it would help if you can give more
> > detailed information
> > about the hardware. We had similar discussions with Xilinx about
> > HDR10+ (see this
> > thread: https://www.spinics.net/lists/linux-media/msg163297.html).
> >
> > There is no clear answer at the moment on how to handle dynamic HDR
> > metadata.
> > It will help if we have some more information how different SoCs handle
> > this
> > in hardware.
> >
> > Regards,
> >
> > Hans
>
> As per Widevine Level 1 requirement, it needs “Hardware Protected Video
> Path”.
> Hence, in case of secure bitstream decoding, we need decoder metadata
> delivered from HW.
> CPU cannot parse secure bitstream and extract them.
> Apart from this, there are other metadata like "histogram" which is not
> part of the bitstream
> and generated by hardware
(I'm ignoring the bit about camera data + histogram, this was about
CODEC, and it also does not make much sense to me)
We extract this data from the bitstream, before the decoder. The
bitstream is not subject to secure buffer, because it's encrypted. Be
aware that the implementation does not encrypt all NALs, PPS/SPS SEIs,
are in clear, and NAL headers are most of the time in clear.
Going with full bitstream encryption would have required rewriting a
lot more HW, since you would not be able to handle the
depayload/demuxing in userspace or you'd need all this multimedia stuff
to be placed on in your secure firmware. Multimedia software these
days, ffmpeg, gstreamer, chromium internal stack etc. needs a lot of
information from the bitstream that would be very hard to pass
properly.
regards,
Nicolas
^ permalink raw reply
* Re: [PATCH 1/3] media: uapi: h264: update reference lists
From: Jernej Škrabec @ 2020-06-05 17:26 UTC (permalink / raw)
To: paul.kocialkowski, mripard, Nicolas Dufresne
Cc: mchehab, wens, hverkuil-cisco, gregkh, jonas, linux-media,
linux-kernel, devel, linux-arm-kernel
In-Reply-To: <981458bfa639bbb9dbc7577256fde0a4c6259d53.camel@ndufresne.ca>
Dne petek, 05. junij 2020 ob 19:13:24 CEST je Nicolas Dufresne napisal(a):
> Sorry, missed one thing.
>
> Le vendredi 05 juin 2020 à 13:08 -0400, Nicolas Dufresne a écrit :
> > Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> > > When dealing with with interlaced frames, reference lists must tell if
> > > each particular reference is meant for top or bottom field. This info
> > > is currently not provided at all in the H264 related controls.
> > >
> > > Make reference lists hold a structure which will also hold flags along
> > > index into DPB array. Flags will tell if reference is meant for top or
> > > bottom field.
> > >
> > > Currently the only user of these lists is Cedrus which is just compile
> > > fixed here. Actual usage of newly introduced flags will come in
> > > following commit.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > This looks like the right approach to me and is extensible if anything
> > else is needed for MVC and SVC special referencing (at least will be
> > enough for what H.264 actually supports in this regard).
> >
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> > > ---
> > >
> > > .../media/v4l/ext-ctrls-codec.rst | 40 ++++++++++++++++++-
> > > .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> > > include/media/h264-ctrls.h | 12 +++++-
> > > 3 files changed, 51 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst index
> > > d0d506a444b1..6c36d298db20 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > @@ -1843,10 +1843,10 @@ enum
> > > v4l2_mpeg_video_h264_hierarchical_coding_type -> >
> > > * - __u32
> > >
> > > - ``slice_group_change_cycle``
> > > -
> > >
> > > - * - __u8
> > > + * - struct :c:type:`v4l2_h264_reference`
> > >
> > > - ``ref_pic_list0[32]``
> > > - Reference picture list after applying the per-slice
> > > modifications
> > >
> > > - * - __u8
> > > + * - struct :c:type:`v4l2_h264_reference`
> > >
> > > - ``ref_pic_list1[32]``
> > > - Reference picture list after applying the per-slice
> > > modifications
> > >
> > > * - __u32
> > >
> > > @@ -1926,6 +1926,42 @@ enum
> > > v4l2_mpeg_video_h264_hierarchical_coding_type -
> > >
> > > - ``chroma_offset[32][2]``
> > > -
> > >
> > > +``Picture Reference``
> > > +
> > > +.. c:type:: v4l2_h264_reference
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table:: struct v4l2_h264_reference
> > > + :header-rows: 0
> > > + :stub-columns: 0
> > > + :widths: 1 1 2
> > > +
> > > + * - __u16
> > > + - ``flags``
> > > + - See :ref:`Picture Reference Flags <h264_reference_flags>`
> > > + * - __u8
> > > + - ``index``
> > > + -
> > > +
> > > +.. _h264_reference_flags:
> > > +
> > > +``Picture Reference Flags``
> > > +
> > > +.. cssclass:: longtable
> > > +
> > > +.. flat-table::
> > > + :header-rows: 0
> > > + :stub-columns: 0
> > > + :widths: 1 1 2
> > > +
> > > + * - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> > > + - 0x00000001
> > > + -
> > > + * - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> > > + - 0x00000002
> > > + -
> > > +
> > >
> > > ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> > >
> > > Specifies the decode parameters (as extracted from the bitstream)
> > > for the associated H264 slice data. This includes the necessary
> > >
> > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index
> > > 54ee2aa423e2..cce527bbdf86 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > > @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct
> > > cedrus_ctx *ctx,> >
> > > static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > >
> > > struct cedrus_run *run,
> > >
> > > - const u8 *ref_list, u8
num_ref,
> > > - enum cedrus_h264_sram_off
sram)
> > > + const struct
v4l2_h264_reference *ref_list,
> > > + u8 num_ref, enum
cedrus_h264_sram_off sram)
> > >
> > > {
> > >
> > > const struct v4l2_ctrl_h264_decode_params *decode =
> > > run->h264.decode_params; struct vb2_queue *cap_q;
> > >
> > > @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> > > *ctx,> >
> > > int buf_idx;
> > > u8 dpb_idx;
> > >
> > > - dpb_idx = ref_list[i];
> > > + dpb_idx = ref_list[i].index;
> > >
> > > dpb = &decode->dpb[dpb_idx];
> > >
> > > if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> > >
> > > diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> > > index 080fd1293c42..9b1cbc9bc38e 100644
> > > --- a/include/media/h264-ctrls.h
> > > +++ b/include/media/h264-ctrls.h
> > > @@ -140,6 +140,14 @@ struct v4l2_h264_pred_weight_table {
> > >
> > > #define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04
> > > #define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
> > >
> > > +#define V4L2_H264_REFERENCE_FLAG_TOP_FIELD 0x01
> > > +#define V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD 0x02
> > > +
> > > +struct v4l2_h264_reference {
> > > + __u8 flags;
> > > + __u8 index;
> > > +};
> > > +
> > >
> > > struct v4l2_ctrl_h264_slice_params {
> > >
> > > /* Size in bytes, including header */
> > > __u32 size;
> > >
> > > @@ -182,8 +190,8 @@ struct v4l2_ctrl_h264_slice_params {
> > >
> > > * Entries on each list are indices into
> > > * v4l2_ctrl_h264_decode_params.dpb[].
> > > */
>
> This comment needs to be updated or moved inside the structure.
I'll move it in v2.
Best regards,
Jernej
>
> > > - __u8 ref_pic_list0[32];
> > > - __u8 ref_pic_list1[32];
> > > + struct v4l2_h264_reference ref_pic_list0[32];
> > > + struct v4l2_h264_reference ref_pic_list1[32];
> > >
> > > __u32 flags;
> > >
> > > };
^ permalink raw reply
* [RFC v4 5/8] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
The isp entity has a hardware support to force full range quantization
for YUV formats. Use the CSC API to allow userspace to set the
quantization for YUV formats on the isp, resizer and capture entities.
- The flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION is set on
YUV formats during enumeration of the resizer and isp formats.
- The flag V4L2_FMT_FLAG_CSC_QUANTIZATION is set on the YUV formats
during enumeration of the capture formats.
- The full quantization is set on YUV formats if userspace ask it
on the entities using the flag V4L2_MBUS_FRAMEFMT_SET_CSC
for subdevices and the flag V4L2_PIX_FMT_FLAG_SET_CSC on for
the capture entity.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 23 ++++++++++++-------
drivers/staging/media/rkisp1/rkisp1-isp.c | 22 ++++++++++++++----
drivers/staging/media/rkisp1/rkisp1-resizer.c | 22 ++++++++++++++++++
3 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index f69235f82c45..66856d5eb576 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -1085,8 +1085,6 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
const struct v4l2_format_info **fmt_info)
{
const struct rkisp1_capture_config *config = cap->config;
- struct rkisp1_capture *other_cap =
- &cap->rkisp1->capture_devs[cap->id ^ 1];
const struct rkisp1_capture_fmt_cfg *fmt;
const struct v4l2_format_info *info;
const unsigned int max_widths[] = { RKISP1_RSZ_MP_SRC_MAX_WIDTH,
@@ -1108,16 +1106,21 @@ static void rkisp1_try_fmt(const struct rkisp1_capture *cap,
pixm->field = V4L2_FIELD_NONE;
pixm->colorspace = V4L2_COLORSPACE_DEFAULT;
pixm->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ pixm->xfer_func = V4L2_XFER_FUNC_DEFAULT;
info = rkisp1_fill_pixfmt(pixm, cap->id);
- /* can not change quantization when stream-on */
- if (other_cap->is_streaming)
- pixm->quantization = other_cap->pix.fmt.quantization;
- /* output full range by default, take effect in params */
- else if (!pixm->quantization ||
- pixm->quantization > V4L2_QUANTIZATION_LIM_RANGE)
+ /*
+ * isp has a feature to select between limited and full range for YUV
+ * formats.
+ * So we should also support it in the capture using the CSC API
+ */
+ if (pixm->flags & V4L2_PIX_FMT_FLAG_SET_CSC &&
+ pixm->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+ v4l2_is_format_yuv(info))
pixm->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ else
+ pixm->quantization = V4L2_QUANTIZATION_DEFAULT;
if (fmt_cfg)
*fmt_cfg = fmt;
@@ -1152,12 +1155,16 @@ static int rkisp1_enum_fmt_vid_cap_mplane(struct file *file, void *priv,
{
struct rkisp1_capture *cap = video_drvdata(file);
const struct rkisp1_capture_fmt_cfg *fmt = NULL;
+ const struct v4l2_format_info *info;
if (f->index >= cap->config->fmt_size)
return -EINVAL;
fmt = &cap->config->fmts[f->index];
f->pixelformat = fmt->fourcc;
+ info = v4l2_format_info(f->pixelformat);
+ if (v4l2_is_format_yuv(info))
+ f->flags = V4L2_FMT_FLAG_CSC_QUANTIZATION;
return 0;
}
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index e66e87d6ea8b..28e0a3c594aa 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -591,6 +591,10 @@ static int rkisp1_isp_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index == pos - 1) {
code->code = fmt->mbus_code;
+ if (fmt->pixel_enc == V4L2_PIXEL_ENC_YUV &&
+ dir == RKISP1_DIR_SRC)
+ code->flags =
+ V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION;
return 0;
}
}
@@ -622,7 +626,6 @@ static int rkisp1_isp_init_config(struct v4l2_subdev *sd,
RKISP1_ISP_PAD_SOURCE_VIDEO);
*src_fmt = *sink_fmt;
src_fmt->code = RKISP1_DEF_SRC_PAD_FMT;
- src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
src_crop = v4l2_subdev_get_try_crop(sd, cfg,
RKISP1_ISP_PAD_SOURCE_VIDEO);
@@ -665,10 +668,21 @@ static void rkisp1_isp_set_src_fmt(struct rkisp1_isp *isp,
isp->src_fmt = mbus_info;
src_fmt->width = src_crop->width;
src_fmt->height = src_crop->height;
- src_fmt->quantization = format->quantization;
- /* full range by default */
- if (!src_fmt->quantization)
+
+ src_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
+ src_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ src_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+ /*
+ * The CSC API is used to allow userspace to force full
+ * quantization on YUV formats.
+ */
+ if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
+ format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+ mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
src_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ else
+ src_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
*format = *src_fmt;
}
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index fa28f4bd65c0..237cce9183f7 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -549,8 +549,30 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
rsz->pixel_enc = mbus_info->pixel_enc;
+ sink_fmt->colorspace = V4L2_COLORSPACE_DEFAULT;
+ sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+ sink_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+ /*
+ * isp has a feature to select between limited and full range for
+ * YUV formats.
+ * so we should support it in the resizer using the CSC API
+ */
+
+ if (format->flags & V4L2_MBUS_FRAMEFMT_SET_CSC &&
+ format->quantization == V4L2_QUANTIZATION_FULL_RANGE &&
+ mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV)
+ sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ else
+ sink_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+
/* Propagete to source pad */
src_fmt->code = sink_fmt->code;
+ src_fmt->field = sink_fmt->field;
+ src_fmt->colorspace = sink_fmt->colorspace;
+ src_fmt->ycbcr_enc = sink_fmt->ycbcr_enc;
+ src_fmt->xfer_func = sink_fmt->xfer_func;
+ src_fmt->quantization = sink_fmt->quantization;
sink_fmt->width = clamp_t(u32, format->width,
rsz->config->min_rsz_width,
--
2.17.1
^ permalink raw reply related
* [RFC v4 4/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel,
Hans Verkuil
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
From: Philipp Zabel <p.zabel@pengutronix.de>
For video capture it is the driver that reports the colorspace,
Y'CbCr/HSV encoding, quantization range and transfer function
used by the video, and there is no way to request something
different, even though many HDTV receivers have some sort of
colorspace conversion capabilities.
For output video this feature already exists since the application
specifies this information for the video format it will send out, and
the transmitter will enable any available CSC if a format conversion has
to be performed in order to match the capabilities of the sink.
For video capture we propose adding new v4l2_pix_format flag:
V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
as the requested colorspace information and will attempt to
do the conversion it supports.
Drivers set the flags
V4L2_FMT_FLAG_CSC_YCBCR_ENC,
V4L2_FMT_FLAG_CSC_HSV_ENC,
V4L2_FMT_FLAG_CSC_QUANTIZATION,
in the flags field of the struct v4l2_fmtdesc during enumeration to
indicate that they support colorspace conversion for the respective field.
Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
supported since there are no drivers that support it.
The same API is added for the subdevices. With the flag
V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
For subdevices, new 'flags' fields were added to the structs
v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
'reserved' field
Drivers do not have to actually look at the flagsr. If the flags are not
set, then the colorspace, ycbcr_enc and quantization fields are set to
the default values by the core, i.e. just pass on the received format
without conversion.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
.../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----
.../userspace-api/media/v4l/pixfmt-v4l2.rst | 46 ++++++++++++++--
.../media/v4l/subdev-formats.rst | 52 +++++++++++++++++--
.../media/v4l/vidioc-enum-fmt.rst | 22 +++++++-
.../v4l/vidioc-subdev-enum-mbus-code.rst | 28 ++++++++++
.../media/videodev2.h.rst.exceptions | 3 ++
include/uapi/linux/v4l2-mediabus.h | 5 +-
include/uapi/linux/v4l2-subdev.h | 5 +-
include/uapi/linux/videodev2.h | 4 ++
9 files changed, 160 insertions(+), 21 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
index 444b4082684c..66f3365d7b72 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst
@@ -105,29 +105,21 @@ describing all planes of that format.
* - __u8
- ``ycbcr_enc``
- Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
- This information supplements the ``colorspace`` and must be set by
- the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ See struct :c:type:`v4l2_pix_format`.
* - __u8
- ``hsv_enc``
- HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
- This information supplements the ``colorspace`` and must be set by
- the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ See struct :c:type:`v4l2_pix_format`.
* - }
-
* - __u8
- ``quantization``
- Quantization range, from enum :c:type:`v4l2_quantization`.
- This information supplements the ``colorspace`` and must be set by
- the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ See struct :c:type:`v4l2_pix_format`.
* - __u8
- ``xfer_func``
- Transfer function, from enum :c:type:`v4l2_xfer_func`.
- This information supplements the ``colorspace`` and must be set by
- the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ See struct :c:type:`v4l2_pix_format`.
* - __u8
- ``reserved[7]``
- Reserved for future extensions. Should be zeroed by drivers and
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
index ffa539592822..f23404efd90f 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
@@ -148,13 +148,29 @@ Single-planar format structure
- Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ streams, see :ref:`colorspaces`. If the application sets the
+ flag ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
+ this field for a capture stream to request a specific Y'CbCr encoding
+ for the captured image data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ This field is ignored for HSV pixelformats. The driver indicates that
+ ycbcr_enc conversion is supported by setting the flag
+ V4L2_FMT_FLAG_CSC_YCBCR_ENC in the corresponding struct
+ :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
* - __u32
- ``hsv_enc``
- HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ streams, see :ref:`colorspaces`. If the application sets the flag
+ ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set this
+ field for a capture stream to request a specific HSV encoding for the
+ captured image data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ This field is ignored for non-HSV pixelformats. The driver indicates
+ that hsv_enc conversion is supported by setting the flag
+ V4L2_FMT_FLAG_CSC_HSV_ENC in the corresponding struct
+ :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
* - }
-
* - __u32
@@ -162,7 +178,14 @@ Single-planar format structure
- Quantization range, from enum :c:type:`v4l2_quantization`.
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ streams, see :ref:`colorspaces`. If the application sets the flag
+ ``V4L2_PIX_FMT_FLAG_SET_CSC`` then the application can set
+ this field for a capture stream to request a specific quantization
+ range for the captured image data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ The driver indicates that quantization conversion is supported by setting
+ the flag V4L2_FMT_FLAG_CSC_QUANTIZATION in the corresponding struct
+ :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags`
* - __u32
- ``xfer_func``
- Transfer function, from enum :c:type:`v4l2_xfer_func`.
@@ -186,3 +209,20 @@ Single-planar format structure
by RGBA values (128, 192, 255, 128), the same pixel described with
premultiplied colors would be described by RGBA values (64, 96,
128, 128)
+ * .. _`v4l2-pix-fmt-flag-set-csc`:
+
+ - ``V4L2_PIX_FMT_FLAG_SET_CSC``
+ - 0x00000002
+ - Set by the application. It is only used for capture and is
+ ignored for output streams. If set, then request the device to do
+ colorspace conversion from the received colorspace to the requested
+ colorspace values. If colorimetry field (``ycncr_enc``, ``hsv_enc``
+ or ``quantization``) is set to 0, then that colorimetry setting will
+ remain unchanged from what was received. So to change the quantization
+ only the ``quantization`` field shall be set to non-zero values
+ (``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
+ and all other colorimetry fields shall be set to 0. The API does not
+ support the conversion of the fields ``colorspace`` and ``xfer_func``
+
+ To check which conversions are supported by the hardware for the current
+ pixel format, see :ref:`fmtdesc-flags`.
diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 9a4d61b0d76f..75eb7f8bb4c5 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -49,13 +49,32 @@ Media Bus Formats
- Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ streams, see :ref:`colorspaces`. If the application sets the
+ flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+ this field for a capture stream to request a specific Y'CbCr encoding
+ for the media bus data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ This field is ignored for HSV media bus formats. The driver indicates
+ that ycbcr_enc conversion is supported by setting the flag
+ V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
+ :c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
+ See :ref:`v4l2-subdev-mbus-code-flags`
+
* - __u16
- ``quantization``
- Quantization range, from enum :c:type:`v4l2_quantization`.
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
- streams, see :ref:`colorspaces`.
+ streams, see :ref:`colorspaces`. If the application sets the
+ flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+ this field for a capture stream to request a specific quantization
+ encoding for the media bus data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ The driver indicates that quantization conversion is supported by
+ setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION in the
+ corresponding struct :c:type:`v4l2_subdev_mbus_code_enum`
+ during enumeration. See :ref:`v4l2-subdev-mbus-code-flags`
+
* - __u16
- ``xfer_func``
- Transfer function, from enum :c:type:`v4l2_xfer_func`.
@@ -63,10 +82,37 @@ Media Bus Formats
the driver for capture streams and by the application for output
streams, see :ref:`colorspaces`.
* - __u16
- - ``reserved``\ [11]
+ - ``flags``
+ - flags See: :ref:v4l2-mbus-framefmt-flags
+ * - __u16
+ - ``reserved``\ [10]
- Reserved for future extensions. Applications and drivers must set
the array to zero.
+.. _v4l2-mbus-framefmt-flags:
+
+.. flat-table:: v4l2_mbus_framefmt Flags
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 3 1 4
+
+ * .. _`mbus-framefmt-set-csc`:
+
+ - ``V4L2_MBUS_FRAMEFMT_SET_CSC``
+ - 0x0001
+ - Set by the application. It is only used for capture and is
+ ignored for output streams. If set, then request the subdevice to do
+ colorspace conversion from the received colorspace to the requested
+ colorspace values. If colorimetry field (``ycbcr_enc`` or
+ ``quantization``) is set to 0, then that colorimetry setting will remain
+ unchanged from what was received. So to change the quantization, only the
+ ``quantization`` field shall be set to non-zero values
+ (``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``)
+ and all other colorimetry fields shall be set to 0. The API does not
+ support the conversion of the fields ``colorspace`` and ``xfer_func``.
+
+ To check which conversions are supported by the hardware for the current
+ media bus frame format, see :ref:`v4l2-mbus-framefmt-flags`.
.. _v4l2-mbus-pixelcode:
diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index a53dd3d7f7e2..11323755d41b 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -178,7 +178,27 @@ the ``mbus_code`` field is handled differently:
parameters are detected. This flag can only be used in combination
with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to
compressed formats only. It is also only applies to stateful codecs.
-
+ * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC``
+ - 0x0010
+ - The driver allows the application to try to change the default
+ Y'CbCr encoding. This flag is relevant only for capture devices.
+ The application can ask to configure the ycbcr_enc of the capture device
+ when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+ :ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+ * - ``V4L2_FMT_FLAG_CSC_HSV_ENC``
+ - 0x0010
+ - The driver allows the application to try to change the default
+ HSV encoding. This flag is relevant only for capture devices.
+ The application can ask to configure the hsv_enc of the capture device
+ when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+ :ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+ * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION``
+ - 0x0020
+ - The driver allows the application to try to change the default
+ quantization. This flag is relevant only for capture devices.
+ The application can ask to configure the quantization of the capture
+ device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
+ :ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
Return Value
============
diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
index 35b8607203a4..3d3430bdd71f 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
@@ -78,12 +78,40 @@ information about the try formats.
- ``which``
- Media bus format codes to be enumerated, from enum
:ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`.
+ * - __u32
+ - ``flags``
+ - See :ref:`v4l2-subdev-mbus-code-flags`
* - __u32
- ``reserved``\ [8]
- Reserved for future extensions. Applications and drivers must set
the array to zero.
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}|
+
+.. _v4l2-subdev-mbus-code-flags:
+
+.. flat-table:: Subdev Media Bus Code Enumerate Flags
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC
+ - 0x00000001
+ - The driver allows the application to try to change the default Y'CbCr
+ encoding. The application can ask to configure the ycbcr_enc of the
+ subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
+ ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
+ See :ref:`v4l2-mbus-format` on how to do this.
+ * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION
+ - 0x00000002
+ - The driver allows the application to try to change the default
+ quantization. The application can ask to configure the quantization of
+ the subdevice when calling the :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>`
+ ioctl with :ref:`V4L2_MBUS_FRAMEFMT_SET_CSC <mbus-framefmt-set-csc>` set.
+ See :ref:`v4l2-mbus-format` on how to do this.
+
Return Value
============
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 564a3bf5bc6d..f7be008cd479 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -187,6 +187,9 @@ replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
replace define V4L2_FMT_FLAG_EMULATED fmtdesc-flags
replace define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM fmtdesc-flags
replace define V4L2_FMT_FLAG_DYN_RESOLUTION fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
+replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
# V4L2 timecode types
replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
index 123a231001a8..0f916278137a 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -16,6 +16,8 @@
#include <linux/types.h>
#include <linux/videodev2.h>
+#define V4L2_MBUS_FRAMEFMT_SET_CSC 0x0001
+
/**
* struct v4l2_mbus_framefmt - frame format on the media bus
* @width: image width
@@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt {
__u16 ycbcr_enc;
__u16 quantization;
__u16 xfer_func;
- __u16 reserved[11];
+ __u16 flags;
+ __u16 reserved[10];
};
#ifndef __KERNEL__
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 5d2a1dab7911..972e64d8b54e 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -65,6 +65,8 @@ struct v4l2_subdev_crop {
__u32 reserved[8];
};
+#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001
+#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002
/**
* struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
* @pad: pad number, as reported by the media API
@@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum {
__u32 index;
__u32 code;
__u32 which;
- __u32 reserved[8];
+ __u32 flags;
+ __u32 reserved[7];
};
/**
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c3a1cf1c507f..15824316e0ca 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -774,6 +774,7 @@ struct v4l2_pix_format {
/* Flags */
#define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001
+#define V4L2_PIX_FMT_FLAG_SET_CSC 0x00000002
/*
* F O R M A T E N U M E R A T I O N
@@ -792,6 +793,9 @@ struct v4l2_fmtdesc {
#define V4L2_FMT_FLAG_EMULATED 0x0002
#define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004
#define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008
+#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010
+#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010
+#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020
/* Frame Size and frame rate enumeration */
/*
--
2.17.1
^ permalink raw reply related
* [RFC v4 2/8] media: staging: rkisp1: rsz: set default format if the given format is not RKISP1_DIR_SRC
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
When setting the sink format of the 'rkisp1_resizer'
the format should be supported by 'rkisp1_isp' on
the video source pad. This patch checks this condition
and set the format to default if the condition is false.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-common.h | 4 ++++
drivers/staging/media/rkisp1/rkisp1-isp.c | 4 ----
drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h
index 0c4fe503adc9..39d8e46d8d8a 100644
--- a/drivers/staging/media/rkisp1/rkisp1-common.h
+++ b/drivers/staging/media/rkisp1/rkisp1-common.h
@@ -22,6 +22,10 @@
#include "rkisp1-regs.h"
#include "uapi/rkisp1-config.h"
+#define RKISP1_DIR_SRC BIT(0)
+#define RKISP1_DIR_SINK BIT(1)
+#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)
+
#define RKISP1_ISP_MAX_WIDTH 4032
#define RKISP1_ISP_MAX_HEIGHT 3024
#define RKISP1_ISP_MIN_WIDTH 32
diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c
index dc2b59a0160a..e66e87d6ea8b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-isp.c
+++ b/drivers/staging/media/rkisp1/rkisp1-isp.c
@@ -23,10 +23,6 @@
#define RKISP1_ISP_DEV_NAME RKISP1_DRIVER_NAME "_isp"
-#define RKISP1_DIR_SRC BIT(0)
-#define RKISP1_DIR_SINK BIT(1)
-#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC)
-
/*
* NOTE: MIPI controller and input MUX are also configured in this file.
* This is because ISP Subdev describes not only ISP submodule (input size,
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index d64c064bdb1d..fa28f4bd65c0 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz,
which);
sink_fmt->code = format->code;
mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
- if (!mbus_info) {
+ if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) {
sink_fmt->code = RKISP1_DEF_FMT;
mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code);
}
--
2.17.1
^ permalink raw reply related
* [RFC v4 8/8] media: v4l2: add support for the CSC API for hsv_enc on mediabus
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
Add the flag V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC that drivers
can set when enumerating the supported mediabus formats
on subdevices to indicate that the userspace can ask to
set the 'hsv_enc'. The patch also replaces the 'ycbcr_enc'
field in 'struct v4l2_mbus_framefmt' with a union that
includes 'hsv_enc'
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
.../media/v4l/subdev-formats.rst | 22 +++++++++++++++++--
include/uapi/linux/v4l2-mediabus.h | 8 ++++++-
include/uapi/linux/v4l2-subdev.h | 1 +
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 75eb7f8bb4c5..d06d1ac95452 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -44,6 +44,8 @@ Media Bus Formats
- Image colorspace, from enum
:c:type:`v4l2_colorspace`. See
:ref:`colorspaces` for details.
+ * - union {
+ - (anonymous)
* - __u16
- ``ycbcr_enc``
- Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
@@ -59,7 +61,23 @@ Media Bus Formats
V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC in the corresponding struct
:c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
See :ref:`v4l2-subdev-mbus-code-flags`
-
+ * - __u16
+ - ``hsv_enc``
+ - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`.
+ This information supplements the ``colorspace`` and must be set by
+ the driver for capture streams and by the application for output
+ streams, see :ref:`colorspaces`. If the application sets the
+ flag ``V4L2_MBUS_FRAMEFMT_SET_CSC`` then the application can set
+ this field for a capture stream to request a specific HSV encoding
+ for the media bus data. If the driver cannot handle requested
+ conversion, it will return another supported encoding.
+ This field is ignored for Y'CbCr media bus formats. The driver indicates
+ that hsv_enc conversion is supported by setting the flag
+ V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC in the corresponding struct
+ :c:type:`v4l2_subdev_mbus_code_enum` during enumeration.
+ See :ref:`v4l2-subdev-mbus-code-flags`
+ * - }
+ -
* - __u16
- ``quantization``
- Quantization range, from enum :c:type:`v4l2_quantization`.
@@ -103,7 +121,7 @@ Media Bus Formats
- Set by the application. It is only used for capture and is
ignored for output streams. If set, then request the subdevice to do
colorspace conversion from the received colorspace to the requested
- colorspace values. If colorimetry field (``ycbcr_enc`` or
+ colorspace values. If colorimetry field (``ycbcr_enc``, ``hsv_enc`` or
``quantization``) is set to 0, then that colorimetry setting will remain
unchanged from what was received. So to change the quantization, only the
``quantization`` field shall be set to non-zero values
diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h
index 0f916278137a..9c4a10d21a49 100644
--- a/include/uapi/linux/v4l2-mediabus.h
+++ b/include/uapi/linux/v4l2-mediabus.h
@@ -26,6 +26,7 @@
* @field: used interlacing type (from enum v4l2_field)
* @colorspace: colorspace of the data (from enum v4l2_colorspace)
* @ycbcr_enc: YCbCr encoding of the data (from enum v4l2_ycbcr_encoding)
+ * @hsv_enc: HSV encoding of the data (from enum v4l2_hsv_encoding)
* @quantization: quantization of the data (from enum v4l2_quantization)
* @xfer_func: transfer function of the data (from enum v4l2_xfer_func)
*/
@@ -35,7 +36,12 @@ struct v4l2_mbus_framefmt {
__u32 code;
__u32 field;
__u32 colorspace;
- __u16 ycbcr_enc;
+ union {
+ /* enum v4l2_ycbcr_encoding */
+ __u16 ycbcr_enc;
+ /* enum v4l2_hsv_encoding */
+ __u16 hsv_enc;
+ };
__u16 quantization;
__u16 xfer_func;
__u16 flags;
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 972e64d8b54e..685f920de5c7 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -66,6 +66,7 @@ struct v4l2_subdev_crop {
};
#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001
+#define V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC 0x00000001
#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002
/**
* struct v4l2_subdev_mbus_code_enum - Media bus format enumeration
--
2.17.1
^ permalink raw reply related
* [RFC v4 3/8] media: Documentation: v4l: move table of v4l2_pix_format(_mplane) flags to pixfmt-v4l2.rst
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
The table of the flags of the structs
v4l2_pix_format(_mplane) is currently in pixfmt-reserved.rst
which is wrong, it should be in pixfmt-v4l2.rst
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
.../userspace-api/media/v4l/pixfmt-reserved.rst | 17 -----------------
.../userspace-api/media/v4l/pixfmt-v4l2.rst | 17 +++++++++++++++++
.../media/videodev2.h.rst.exceptions | 2 +-
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
index 59b9e7238f90..74ab6b5ce294 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
@@ -263,20 +263,3 @@ please make a proposal on the linux-media mailing list.
of tiles, resulting in 32-aligned resolutions for the luminance plane
and 16-aligned resolutions for the chrominance plane (with 2x2
subsampling).
-
-.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
-
-.. _format-flags:
-
-.. flat-table:: Format Flags
- :header-rows: 0
- :stub-columns: 0
- :widths: 3 1 4
-
- * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
- - 0x00000001
- - The color values are premultiplied by the alpha channel value. For
- example, if a light blue pixel with 50% transparency was described
- by RGBA values (128, 192, 255, 128), the same pixel described with
- premultiplied colors would be described by RGBA values (64, 96,
- 128, 128)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
index 759420a872d6..ffa539592822 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst
@@ -169,3 +169,20 @@ Single-planar format structure
This information supplements the ``colorspace`` and must be set by
the driver for capture streams and by the application for output
streams, see :ref:`colorspaces`.
+
+.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
+
+.. _format-flags:
+
+.. flat-table:: Format Flags
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 3 1 4
+
+ * - ``V4L2_PIX_FMT_FLAG_PREMUL_ALPHA``
+ - 0x00000001
+ - The color values are premultiplied by the alpha channel value. For
+ example, if a light blue pixel with 50% transparency was described
+ by RGBA values (128, 192, 255, 128), the same pixel described with
+ premultiplied colors would be described by RGBA values (64, 96,
+ 128, 128)
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index a625fb90e3a9..564a3bf5bc6d 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -180,7 +180,7 @@ replace define V4L2_CAP_IO_MC device-capabilities
# V4L2 pix flags
replace define V4L2_PIX_FMT_PRIV_MAGIC :c:type:`v4l2_pix_format`
-replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA reserved-formats
+replace define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA format-flags
# V4L2 format flags
replace define V4L2_FMT_FLAG_COMPRESSED fmtdesc-flags
--
2.17.1
^ permalink raw reply related
* [RFC v4 7/8] media: vivid: Add support to the CSC API
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
The CSC API (Colorspace conversion) allows userspace to try
to configure the ycbcr/hsv_enc function and the quantization
for capture devices. This patch adds support to the CSC API
in vivid.
Using the CSC API, userspace is allowed to do the following:
1. Set the ycbcr_enc function for YUV formats.
2. Set the hsv_enc function for HSV formats
3. Set the quantization for YUV and RGB formats.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
.../media/test-drivers/vivid/vivid-vid-cap.c | 49 +++++++++++++++++--
.../test-drivers/vivid/vivid-vid-common.c | 20 ++++++++
2 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index e94beef008c8..8a43d7ebe53f 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -549,6 +549,29 @@ int vivid_g_fmt_vid_cap(struct file *file, void *priv,
return 0;
}
+static bool vivid_is_hsv_enc_valid(__u8 hsv_enc)
+{
+ if (hsv_enc == V4L2_HSV_ENC_180 || hsv_enc == V4L2_HSV_ENC_256)
+ return true;
+ return false;
+}
+
+static bool vivid_is_ycbcr_enc_valid(__u8 ycbcr_enc)
+{
+ /* V4L2_YCBCR_ENC_SMPTE240M is the last ycbcr_enc enum */
+ if (ycbcr_enc && ycbcr_enc <= V4L2_YCBCR_ENC_SMPTE240M)
+ return true;
+ return false;
+}
+
+static bool vivid_is_quant_valid(__u8 quantization)
+{
+ if (quantization == V4L2_QUANTIZATION_FULL_RANGE ||
+ quantization == V4L2_QUANTIZATION_LIM_RANGE)
+ return true;
+ return false;
+}
+
int vivid_try_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
@@ -560,6 +583,7 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
unsigned factor = 1;
unsigned w, h;
unsigned p;
+ bool user_set_csc = !!(mp->flags & V4L2_PIX_FMT_FLAG_SET_CSC);
fmt = vivid_get_format(dev, mp->pixelformat);
if (!fmt) {
@@ -634,12 +658,23 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
(fmt->bit_depth[0] / fmt->vdownsampling[0]);
mp->colorspace = vivid_colorspace_cap(dev);
- if (fmt->color_enc == TGP_COLOR_ENC_HSV)
- mp->hsv_enc = vivid_hsv_enc_cap(dev);
- else
+ if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
+ if (!user_set_csc || !vivid_is_hsv_enc_valid(mp->hsv_enc))
+ mp->hsv_enc = vivid_hsv_enc_cap(dev);
+ } else if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
+ if (!user_set_csc || !vivid_is_ycbcr_enc_valid(mp->ycbcr_enc))
+ mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
+ } else {
mp->ycbcr_enc = vivid_ycbcr_enc_cap(dev);
+ }
mp->xfer_func = vivid_xfer_func_cap(dev);
- mp->quantization = vivid_quantization_cap(dev);
+ if (fmt->color_enc == TGP_COLOR_ENC_YCBCR ||
+ fmt->color_enc == TGP_COLOR_ENC_RGB) {
+ if (!user_set_csc || !vivid_is_quant_valid(mp->quantization))
+ mp->quantization = vivid_quantization_cap(dev);
+ } else {
+ mp->quantization = vivid_quantization_cap(dev);
+ }
memset(mp->reserved, 0, sizeof(mp->reserved));
return 0;
}
@@ -769,6 +804,12 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
if (vivid_is_sdtv_cap(dev))
dev->tv_field_cap = mp->field;
tpg_update_mv_step(&dev->tpg);
+ dev->tpg.quantization = mp->quantization;
+ if (dev->fmt_cap->color_enc == TGP_COLOR_ENC_YCBCR)
+ dev->tpg.ycbcr_enc = mp->ycbcr_enc;
+ else
+ dev->tpg.hsv_enc = mp->hsv_enc;
+
return 0;
}
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-common.c b/drivers/media/test-drivers/vivid/vivid-vid-common.c
index 76b0be670ebb..19aacb180e67 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-common.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-common.c
@@ -920,6 +920,26 @@ int vivid_enum_fmt_vid(struct file *file, void *priv,
fmt = &vivid_formats[f->index];
f->pixelformat = fmt->fourcc;
+
+ if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+ f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+ return 0;
+ /*
+ * For capture devices, we support the CSC API.
+ * We allow userspace to:
+ * 1. set the ycbcr_enc on yuv format
+ * 2. set the hsv_enc on hsv format
+ * 3. set the quantization on yuv and rgb formats
+ */
+ if (fmt->color_enc == TGP_COLOR_ENC_YCBCR) {
+ f->flags |= V4L2_FMT_FLAG_CSC_YCBCR_ENC;
+ f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
+ } else if (fmt->color_enc == TGP_COLOR_ENC_HSV) {
+ f->flags |= V4L2_FMT_FLAG_CSC_HSV_ENC;
+ } else if (fmt->color_enc == TGP_COLOR_ENC_RGB) {
+ f->flags |= V4L2_FMT_FLAG_CSC_QUANTIZATION;
+ }
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* [RFC v4 6/8] media: staging: rkisp1: validate quantization matching in link_validate callbacks
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
The quantization of the rkisp1 entities can be set by userspace
using the CSC API. Therefore we validate that the
quantization field matches on the links in the link_validate
callbacks.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-capture.c | 3 ++-
drivers/staging/media/rkisp1/rkisp1-resizer.c | 21 ++++++++++++++++++-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 66856d5eb576..3eb2ea1a9eb1 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -1263,7 +1263,8 @@ static int rkisp1_capture_link_validate(struct media_link *link)
return ret;
if (sd_fmt.format.height != cap->pix.fmt.height ||
- sd_fmt.format.width != cap->pix.fmt.width)
+ sd_fmt.format.width != cap->pix.fmt.width ||
+ sd_fmt.format.quantization != cap->pix.fmt.quantization)
return -EPIPE;
return 0;
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index 237cce9183f7..027396b00124 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -671,6 +671,25 @@ static int rkisp1_rsz_set_selection(struct v4l2_subdev *sd,
return 0;
}
+int rkisp1_rsz_link_validate(struct v4l2_subdev *sd, struct media_link *link,
+ struct v4l2_subdev_format *source_fmt,
+ struct v4l2_subdev_format *sink_fmt)
+{
+ int ret;
+
+ ret = v4l2_subdev_link_validate_default(sd, link, source_fmt, sink_fmt);
+ if (ret)
+ return ret;
+ if (source_fmt->format.quantization != sink_fmt->format.quantization) {
+ struct device *dev = link->graph_obj.mdev->dev;
+
+ dev_warn(dev, "isp->resizer link validation failed, ");
+ dev_warn(dev, "quantizations don't match\n");
+ return -EPIPE;
+ }
+ return 0;
+}
+
static const struct media_entity_operations rkisp1_rsz_media_ops = {
.link_validate = v4l2_subdev_link_validate,
};
@@ -682,7 +701,7 @@ static const struct v4l2_subdev_pad_ops rkisp1_rsz_pad_ops = {
.init_cfg = rkisp1_rsz_init_config,
.get_fmt = rkisp1_rsz_get_fmt,
.set_fmt = rkisp1_rsz_set_fmt,
- .link_validate = v4l2_subdev_link_validate_default,
+ .link_validate = rkisp1_rsz_link_validate,
};
/* ----------------------------------------------------------------------------
--
2.17.1
^ permalink raw reply related
* [RFC v4 1/8] media: staging: rkisp1: rsz: supported formats are the isp's src formats, not sink formats
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
In-Reply-To: <20200605172625.19777-1-dafna.hirschfeld@collabora.com>
The rkisp1_resizer's enum callback 'rkisp1_rsz_enum_mbus_code'
calls the enum callback of the 'rkisp1_isp' on it's video sink pad.
This is a bug, the resizer should support the same formats
supported by the 'rkisp1_isp' on the source pad (not the sink pad).
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-resizer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c
index d049374413dc..d64c064bdb1d 100644
--- a/drivers/staging/media/rkisp1/rkisp1-resizer.c
+++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c
@@ -437,8 +437,8 @@ static int rkisp1_rsz_enum_mbus_code(struct v4l2_subdev *sd,
u32 pad = code->pad;
int ret;
- /* supported mbus codes are the same in isp sink pad */
- code->pad = RKISP1_ISP_PAD_SINK_VIDEO;
+ /* supported mbus codes are the same in isp video src pad */
+ code->pad = RKISP1_ISP_PAD_SOURCE_VIDEO;
ret = v4l2_subdev_call(&rsz->rkisp1->isp.sd, pad, enum_mbus_code,
&dummy_cfg, code);
--
2.17.1
^ permalink raw reply related
* [RFC v4 0/8] v4l2: add support for colorspace conversion API (CSC) for video capture and subdevices
From: Dafna Hirschfeld @ 2020-06-05 17:26 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel, dafna3,
sakari.ailus, linux-rockchip, mchehab, tfiga, skhan, p.zabel
Quoted from patch "v4l2: add support for colorspace conversion API (CSC)
for video capture and subdevices":
========================================================================
For video capture it is the driver that reports the colorspace,
Y'CbCr/HSV encoding, quantization range and transfer function
used by the video, and there is no way to request something
different, even though many HDTV receivers have some sort of
colorspace conversion capabilities.
For output video this feature already exists since the application
specifies this information for the video format it will send out, and
the transmitter will enable any available CSC if a format conversion has
to be performed in order to match the capabilities of the sink.
For video capture we propose adding new v4l2_pix_format flag:
V4L2_PIX_FMT_FLAG_SET_CSC. The flag is set by the application,
the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields
as the requested colorspace information and will attempt to
do the conversion it supports.
Drivers set the flags
V4L2_FMT_FLAG_CSC_YCBCR_ENC,
V4L2_FMT_FLAG_CSC_HSV_ENC,
V4L2_FMT_FLAG_CSC_QUANTIZATION,
in the flags field of the struct v4l2_fmtdesc during enumeration to
indicate that they support colorspace conversion for the respective field.
Currently the conversion of the fields 'colorspace' and 'xfer_func' is not
supported since there are no drivers that support it.
The same API is added for the subdevices. With the flag
V4L2_MBUS_FRAMEFMT_SET_CSC set by the application in VIDIOC_SUBDEV_S_FMT
ioctl and the flags V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC,
V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION set by the driver in the
VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl.
For subdevices, new 'flags' fields were added to the structs
v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the
'reserved' field
Drivers do not have to actually look at the flagsr. If the flags are not
set, then the colorspace, ycbcr_enc and quantization fields are set to
the default values by the core, i.e. just pass on the received format
without conversion.
========================================================================
The patchset includes a patch in vivid that implements the API for video capture devices
and 2 patches in rkisp1 driver that implement the API for subdevices.
Patches Summary:
----------------
patches 1,2 are bug fixes in rkisp1 driver, they are not part of the patchset
but the patchset needs those fixes for the rkisp1 implementation of the API.
patch 3 - moves the description of the v4l2_pix_format flags in the Documentation to the right
place where it should be. This is just a fix
patch 4 - Adds the API - Adds the new macros and fields and their documentation
patches 5,6 - uses the API in rkisp1 to allow userspace to change the quantization for YUV
formats. The support is added to the entities 'isp', 'resizer' 'capture' userspace
should make sure that the quantization matches along the pipe, otherwise the validation fails
when trying to stream.
patch 7 - uses the API in the capture device of vivid. Using the API, userspace is allowed to:
1. Set the ycbcr_enc function for YUV formats.
2. Set the hsv_enc function for HSV formats
3. Set the quantization for YUV and RGB formats.
patch 8 - adds the field 'hsv_enc' as a union together with 'ycbcr_enc' in struct v4l2_mbus_framefmt
and adds the flag V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC to support the API for HSV media bus formats.
Changes from v3:
----------------
- patches 1,2,3,7,8 are new (rkisp1 bug fixes, move v4l2-pixfmt flags docs, vivid patch, support for HSV media bus)
- patch 4 (The API):
1) reformulate the Documentation according to comments from Verkuil and Tomasz Figa
2) remove the code in drivers/media/v4l2-core/v4l2-subdev.c and drivers/media/v4l2-core/v4l2-ioctl.c
so that except of the new macros and fields no new code is added to the core
- patches 5,6 - refactor the rkisp1 implementation so that the userspace should set the quantization separately
to each entity. In patch 6 the validation callbacks make sure the quantizations matche.
Dafna Hirschfeld (7):
media: staging: rkisp1: rsz: supported formats are the isp's src
formats, not sink formats
media: staging: rkisp1: rsz: set default format if the given format is
not RKISP1_DIR_SRC
media: Documentation: v4l: move table of v4l2_pix_format(_mplane)
flags to pixfmt-v4l2.rst
media: staging: rkisp1: allow quantization conversion from userspace
for isp source pad
media: staging: rkisp1: validate quantization matching in
link_validate callbacks
media: vivid: Add support to the CSC API
media: v4l2: add support for the CSC API for hsv_enc on mediabus
Philipp Zabel (1):
v4l2: add support for colorspace conversion API (CSC) for video
capture and subdevices
.../media/v4l/pixfmt-reserved.rst | 17 -----
.../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++---
.../userspace-api/media/v4l/pixfmt-v4l2.rst | 63 ++++++++++++++++-
.../media/v4l/subdev-formats.rst | 70 ++++++++++++++++++-
.../media/v4l/vidioc-enum-fmt.rst | 22 +++++-
.../v4l/vidioc-subdev-enum-mbus-code.rst | 28 ++++++++
.../media/videodev2.h.rst.exceptions | 5 +-
.../media/test-drivers/vivid/vivid-vid-cap.c | 49 +++++++++++--
.../test-drivers/vivid/vivid-vid-common.c | 20 ++++++
drivers/staging/media/rkisp1/rkisp1-capture.c | 26 ++++---
drivers/staging/media/rkisp1/rkisp1-common.h | 4 ++
drivers/staging/media/rkisp1/rkisp1-isp.c | 26 ++++---
drivers/staging/media/rkisp1/rkisp1-resizer.c | 49 +++++++++++--
include/uapi/linux/v4l2-mediabus.h | 13 +++-
include/uapi/linux/v4l2-subdev.h | 6 +-
include/uapi/linux/videodev2.h | 4 ++
16 files changed, 353 insertions(+), 65 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH 2/3] media: cedrus: h264: Properly configure reference field
From: Jernej Škrabec @ 2020-06-05 17:26 UTC (permalink / raw)
To: paul.kocialkowski, mripard, Nicolas Dufresne
Cc: mchehab, wens, hverkuil-cisco, gregkh, jonas, linux-media,
linux-kernel, devel, linux-arm-kernel
In-Reply-To: <7e74e15b7b3f9fc765182f1a43cfcf1e0e9602fc.camel@ndufresne.ca>
Dne petek, 05. junij 2020 ob 19:16:35 CEST je Nicolas Dufresne napisal(a):
> Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> > When interlaced H264 content is being decoded, references must indicate
> > which field is being referenced. Currently this was done by checking
> > capture buffer flags. However, that is not correct because capture
> > buffer may hold both fields.
> >
> > Fix this by checking newly introduced flags in reference lists.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> Perhaps an additional patch could cleanup the miss-leading comment in
> v4l2_h264_dpb_entry definition.
I missed that. I think this change actually belongs to patch 1. I'll fix it in
v2.
Best regards,
Jernej
>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index
> > cce527bbdf86..c87717d17ec5 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > @@ -183,7 +183,6 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> > *ctx,>
> > for (i = 0; i < num_ref; i++) {
> >
> > const struct v4l2_h264_dpb_entry *dpb;
> > const struct cedrus_buffer *cedrus_buf;
> >
> > - const struct vb2_v4l2_buffer *ref_buf;
> >
> > unsigned int position;
> > int buf_idx;
> > u8 dpb_idx;
> >
> > @@ -198,12 +197,11 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> > *ctx,>
> > if (buf_idx < 0)
> >
> > continue;
> >
> > - ref_buf = to_vb2_v4l2_buffer(cap_q->bufs[buf_idx]);
> > - cedrus_buf = vb2_v4l2_to_cedrus_buffer(ref_buf);
> > + cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
> >
> > position = cedrus_buf->codec.h264.position;
> >
> > sram_array[i] |= position << 1;
> >
> > - if (ref_buf->field == V4L2_FIELD_BOTTOM)
> > + if (ref_list[i].flags &
V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD)
> >
> > sram_array[i] |= BIT(0);
> >
> > }
^ permalink raw reply
* Re: [PATCH 2/3] media: cedrus: h264: Properly configure reference field
From: Nicolas Dufresne @ 2020-06-05 17:16 UTC (permalink / raw)
To: Jernej Skrabec, paul.kocialkowski, mripard
Cc: mchehab, wens, hverkuil-cisco, gregkh, jonas, linux-media,
linux-kernel, devel, linux-arm-kernel
In-Reply-To: <20200604185745.23568-3-jernej.skrabec@siol.net>
Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> When interlaced H264 content is being decoded, references must indicate
> which field is being referenced. Currently this was done by checking
> capture buffer flags. However, that is not correct because capture
> buffer may hold both fields.
>
> Fix this by checking newly introduced flags in reference lists.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Perhaps an additional patch could cleanup the miss-leading comment in
v4l2_h264_dpb_entry definition.
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index cce527bbdf86..c87717d17ec5 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -183,7 +183,6 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> for (i = 0; i < num_ref; i++) {
> const struct v4l2_h264_dpb_entry *dpb;
> const struct cedrus_buffer *cedrus_buf;
> - const struct vb2_v4l2_buffer *ref_buf;
> unsigned int position;
> int buf_idx;
> u8 dpb_idx;
> @@ -198,12 +197,11 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> if (buf_idx < 0)
> continue;
>
> - ref_buf = to_vb2_v4l2_buffer(cap_q->bufs[buf_idx]);
> - cedrus_buf = vb2_v4l2_to_cedrus_buffer(ref_buf);
> + cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
> position = cedrus_buf->codec.h264.position;
>
> sram_array[i] |= position << 1;
> - if (ref_buf->field == V4L2_FIELD_BOTTOM)
> + if (ref_list[i].flags & V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD)
> sram_array[i] |= BIT(0);
> }
>
^ permalink raw reply
* Re: [PATCH 1/3] media: uapi: h264: update reference lists
From: Nicolas Dufresne @ 2020-06-05 17:13 UTC (permalink / raw)
To: Jernej Skrabec, paul.kocialkowski, mripard
Cc: mchehab, wens, hverkuil-cisco, gregkh, jonas, linux-media,
linux-kernel, devel, linux-arm-kernel
In-Reply-To: <21efb826506f23d348fa58ca8b29eaca8c9dae55.camel@ndufresne.ca>
Sorry, missed one thing.
Le vendredi 05 juin 2020 à 13:08 -0400, Nicolas Dufresne a écrit :
> Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> > When dealing with with interlaced frames, reference lists must tell if
> > each particular reference is meant for top or bottom field. This info
> > is currently not provided at all in the H264 related controls.
> >
> > Make reference lists hold a structure which will also hold flags along
> > index into DPB array. Flags will tell if reference is meant for top or
> > bottom field.
> >
> > Currently the only user of these lists is Cedrus which is just compile
> > fixed here. Actual usage of newly introduced flags will come in
> > following commit.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> This looks like the right approach to me and is extensible if anything
> else is needed for MVC and SVC special referencing (at least will be
> enough for what H.264 actually supports in this regard).
>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> > ---
> > .../media/v4l/ext-ctrls-codec.rst | 40 ++++++++++++++++++-
> > .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> > include/media/h264-ctrls.h | 12 +++++-
> > 3 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index d0d506a444b1..6c36d298db20 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1843,10 +1843,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > * - __u32
> > - ``slice_group_change_cycle``
> > -
> > - * - __u8
> > + * - struct :c:type:`v4l2_h264_reference`
> > - ``ref_pic_list0[32]``
> > - Reference picture list after applying the per-slice modifications
> > - * - __u8
> > + * - struct :c:type:`v4l2_h264_reference`
> > - ``ref_pic_list1[32]``
> > - Reference picture list after applying the per-slice modifications
> > * - __u32
> > @@ -1926,6 +1926,42 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > - ``chroma_offset[32][2]``
> > -
> >
> > +``Picture Reference``
> > +
> > +.. c:type:: v4l2_h264_reference
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_h264_reference
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u16
> > + - ``flags``
> > + - See :ref:`Picture Reference Flags <h264_reference_flags>`
> > + * - __u8
> > + - ``index``
> > + -
> > +
> > +.. _h264_reference_flags:
> > +
> > +``Picture Reference Flags``
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table::
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> > + - 0x00000001
> > + -
> > + * - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> > + - 0x00000002
> > + -
> > +
> > ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> > Specifies the decode parameters (as extracted from the bitstream)
> > for the associated H264 slice data. This includes the necessary
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > index 54ee2aa423e2..cce527bbdf86 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
> >
> > static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > struct cedrus_run *run,
> > - const u8 *ref_list, u8 num_ref,
> > - enum cedrus_h264_sram_off sram)
> > + const struct v4l2_h264_reference *ref_list,
> > + u8 num_ref, enum cedrus_h264_sram_off sram)
> > {
> > const struct v4l2_ctrl_h264_decode_params *decode = run->h264.decode_params;
> > struct vb2_queue *cap_q;
> > @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > int buf_idx;
> > u8 dpb_idx;
> >
> > - dpb_idx = ref_list[i];
> > + dpb_idx = ref_list[i].index;
> > dpb = &decode->dpb[dpb_idx];
> >
> > if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> > diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> > index 080fd1293c42..9b1cbc9bc38e 100644
> > --- a/include/media/h264-ctrls.h
> > +++ b/include/media/h264-ctrls.h
> > @@ -140,6 +140,14 @@ struct v4l2_h264_pred_weight_table {
> > #define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04
> > #define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
> >
> > +#define V4L2_H264_REFERENCE_FLAG_TOP_FIELD 0x01
> > +#define V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD 0x02
> > +
> > +struct v4l2_h264_reference {
> > + __u8 flags;
> > + __u8 index;
> > +};
> > +
> > struct v4l2_ctrl_h264_slice_params {
> > /* Size in bytes, including header */
> > __u32 size;
> > @@ -182,8 +190,8 @@ struct v4l2_ctrl_h264_slice_params {
> > * Entries on each list are indices into
> > * v4l2_ctrl_h264_decode_params.dpb[].
> > */
This comment needs to be updated or moved inside the structure.
> > - __u8 ref_pic_list0[32];
> > - __u8 ref_pic_list1[32];
> > + struct v4l2_h264_reference ref_pic_list0[32];
> > + struct v4l2_h264_reference ref_pic_list1[32];
> >
> > __u32 flags;
> > };
^ permalink raw reply
* Re: [PATCH 1/3] media: uapi: h264: update reference lists
From: Nicolas Dufresne @ 2020-06-05 17:08 UTC (permalink / raw)
To: Jernej Skrabec, paul.kocialkowski, mripard
Cc: mchehab, wens, hverkuil-cisco, gregkh, jonas, linux-media,
linux-kernel, devel, linux-arm-kernel
In-Reply-To: <20200604185745.23568-2-jernej.skrabec@siol.net>
Le jeudi 04 juin 2020 à 20:57 +0200, Jernej Skrabec a écrit :
> When dealing with with interlaced frames, reference lists must tell if
> each particular reference is meant for top or bottom field. This info
> is currently not provided at all in the H264 related controls.
>
> Make reference lists hold a structure which will also hold flags along
> index into DPB array. Flags will tell if reference is meant for top or
> bottom field.
>
> Currently the only user of these lists is Cedrus which is just compile
> fixed here. Actual usage of newly introduced flags will come in
> following commit.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
This looks like the right approach to me and is extensible if anything
else is needed for MVC and SVC special referencing (at least will be
enough for what H.264 actually supports in this regard).
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> .../media/v4l/ext-ctrls-codec.rst | 40 ++++++++++++++++++-
> .../staging/media/sunxi/cedrus/cedrus_h264.c | 6 +--
> include/media/h264-ctrls.h | 12 +++++-
> 3 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index d0d506a444b1..6c36d298db20 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1843,10 +1843,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> * - __u32
> - ``slice_group_change_cycle``
> -
> - * - __u8
> + * - struct :c:type:`v4l2_h264_reference`
> - ``ref_pic_list0[32]``
> - Reference picture list after applying the per-slice modifications
> - * - __u8
> + * - struct :c:type:`v4l2_h264_reference`
> - ``ref_pic_list1[32]``
> - Reference picture list after applying the per-slice modifications
> * - __u32
> @@ -1926,6 +1926,42 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> - ``chroma_offset[32][2]``
> -
>
> +``Picture Reference``
> +
> +.. c:type:: v4l2_h264_reference
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_h264_reference
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u16
> + - ``flags``
> + - See :ref:`Picture Reference Flags <h264_reference_flags>`
> + * - __u8
> + - ``index``
> + -
> +
> +.. _h264_reference_flags:
> +
> +``Picture Reference Flags``
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - ``V4L2_H264_REFERENCE_FLAG_TOP_FIELD``
> + - 0x00000001
> + -
> + * - ``V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD``
> + - 0x00000002
> + -
> +
> ``V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS (struct)``
> Specifies the decode parameters (as extracted from the bitstream)
> for the associated H264 slice data. This includes the necessary
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index 54ee2aa423e2..cce527bbdf86 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -166,8 +166,8 @@ static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
>
> static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> struct cedrus_run *run,
> - const u8 *ref_list, u8 num_ref,
> - enum cedrus_h264_sram_off sram)
> + const struct v4l2_h264_reference *ref_list,
> + u8 num_ref, enum cedrus_h264_sram_off sram)
> {
> const struct v4l2_ctrl_h264_decode_params *decode = run->h264.decode_params;
> struct vb2_queue *cap_q;
> @@ -188,7 +188,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> int buf_idx;
> u8 dpb_idx;
>
> - dpb_idx = ref_list[i];
> + dpb_idx = ref_list[i].index;
> dpb = &decode->dpb[dpb_idx];
>
> if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
> index 080fd1293c42..9b1cbc9bc38e 100644
> --- a/include/media/h264-ctrls.h
> +++ b/include/media/h264-ctrls.h
> @@ -140,6 +140,14 @@ struct v4l2_h264_pred_weight_table {
> #define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED 0x04
> #define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
>
> +#define V4L2_H264_REFERENCE_FLAG_TOP_FIELD 0x01
> +#define V4L2_H264_REFERENCE_FLAG_BOTTOM_FIELD 0x02
> +
> +struct v4l2_h264_reference {
> + __u8 flags;
> + __u8 index;
> +};
> +
> struct v4l2_ctrl_h264_slice_params {
> /* Size in bytes, including header */
> __u32 size;
> @@ -182,8 +190,8 @@ struct v4l2_ctrl_h264_slice_params {
> * Entries on each list are indices into
> * v4l2_ctrl_h264_decode_params.dpb[].
> */
> - __u8 ref_pic_list0[32];
> - __u8 ref_pic_list1[32];
> + struct v4l2_h264_reference ref_pic_list0[32];
> + struct v4l2_h264_reference ref_pic_list1[32];
>
> __u32 flags;
> };
^ permalink raw reply
* [PATCH] mtd: rawnand: brcmnand: force raw OOB writes
From: Álvaro Fernández Rojas @ 2020-06-05 17:07 UTC (permalink / raw)
To: computersforpeace, kdasu.kdev, miquel.raynal, richard, vigneshr,
sumit.semwal, linux-mtd, bcm-kernel-feedback-list, linux-kernel,
linux-media, dri-devel, linaro-mm-sig
Cc: Álvaro Fernández Rojas
MTD_OPS_AUTO_OOB is writting OOB with ECC enabled, which changes all ECC bytes
from an erased page to 0x00 when JFFS2 cleanmarkers are added with mtd-utils.
| BBI | JFFS2 | ECC | JFFS2 | Spare |
00000800 ff ff 19 85 20 03 00 00 00 00 00 00 08 ff ff ff
However, if OOB is written with ECC disabled, the JFFS2 cleanmarkers are
correctly written without changing the ECC bytes:
| BBI | JFFS2 | ECC | JFFS2 | Spare |
00000800 ff ff 19 85 20 03 ff ff ff 00 00 00 08 ff ff ff
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 8f9ffb46a09f..566281770841 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2279,13 +2279,6 @@ static int brcmnand_write_page_raw(struct nand_chip *chip, const uint8_t *buf,
return nand_prog_page_end_op(chip);
}
-static int brcmnand_write_oob(struct nand_chip *chip, int page)
-{
- return brcmnand_write(nand_to_mtd(chip), chip,
- (u64)page << chip->page_shift, NULL,
- chip->oob_poi);
-}
-
static int brcmnand_write_oob_raw(struct nand_chip *chip, int page)
{
struct mtd_info *mtd = nand_to_mtd(chip);
@@ -2642,7 +2635,7 @@ static int brcmnand_init_cs(struct brcmnand_host *host, struct device_node *dn)
chip->ecc.write_oob_raw = brcmnand_write_oob_raw;
chip->ecc.read_oob_raw = brcmnand_read_oob_raw;
chip->ecc.read_oob = brcmnand_read_oob;
- chip->ecc.write_oob = brcmnand_write_oob;
+ chip->ecc.write_oob = brcmnand_write_oob_raw;
chip->controller = &ctrl->controller;
--
2.26.2
^ permalink raw reply related
* [PATCH stable 4.9 00/21] Unbreak 32-bit DVB applications on 64-bit kernels
From: Florian Fainelli @ 2020-06-05 16:24 UTC (permalink / raw)
To: linux-kernel
Cc: stable, Florian Fainelli, Mauro Carvalho Chehab, Michael Krufky,
Alexander Viro, Shuah Khan, Florian Fainelli, Jaedon Shin,
Colin Ian King, Katsuhiro Suzuki, Satendra Singh Thakur,
open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB),
open list:FILESYSTEMS (VFS and infrastructure)
Hi all,
This long patch series was motivated by backporting Jaedon's changes
which add a proper ioctl compatibility layer for 32-bit applications
running on 64-bit kernels. We have a number of Android TV-based products
currently running on the 4.9 kernel and this was broken for them.
Thanks to Robert McConnell for identifying and providing the patches in
their initial format.
In order for Jaedon's patches to apply cleanly a number of changes were
applied to support those changes. If you deem the patch series too big
please let me know.
Thanks
Colin Ian King (2):
media: dvb_frontend: ensure that inital front end status initialized
media: dvb_frontend: initialize variable s with FE_NONE instead of 0
Jaedon Shin (3):
media: dvb_frontend: Add unlocked_ioctl in dvb_frontend.c
media: dvb_frontend: Add compat_ioctl callback
media: dvb_frontend: Add commands implementation for compat ioct
Katsuhiro Suzuki (1):
media: dvb_frontend: fix wrong cast in compat_ioctl
Mauro Carvalho Chehab (14):
media: dvb/frontend.h: move out a private internal structure
media: dvb/frontend.h: document the uAPI file
media: dvb_frontend: get rid of get_property() callback
media: stv0288: get rid of set_property boilerplate
media: stv6110: get rid of a srate dead code
media: friio-fe: get rid of set_property()
media: dvb_frontend: get rid of set_property() callback
media: dvb_frontend: cleanup dvb_frontend_ioctl_properties()
media: dvb_frontend: cleanup ioctl handling logic
media: dvb_frontend: get rid of property cache's state
media: dvb_frontend: better document the -EPERM condition
media: dvb_frontend: fix return values for FE_SET_PROPERTY
media: dvb_frontend: be sure to init dvb_frontend_handle_ioctl()
return code
media: dvb_frontend: fix return error code
Satendra Singh Thakur (1):
media: dvb_frontend: dtv_property_process_set() cleanups
.../media/uapi/dvb/fe-get-property.rst | 7 +-
drivers/media/dvb-core/dvb_frontend.c | 571 +++++++++++------
drivers/media/dvb-core/dvb_frontend.h | 13 -
drivers/media/dvb-frontends/lg2160.c | 14 -
drivers/media/dvb-frontends/stv0288.c | 7 -
drivers/media/dvb-frontends/stv6110.c | 9 -
drivers/media/usb/dvb-usb/friio-fe.c | 24 -
fs/compat_ioctl.c | 17 -
include/uapi/linux/dvb/frontend.h | 592 +++++++++++++++---
9 files changed, 881 insertions(+), 373 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH stable 4.9 01/21] media: dvb_frontend: ensure that inital front end status initialized
From: Florian Fainelli @ 2020-06-05 16:24 UTC (permalink / raw)
To: linux-kernel
Cc: stable, Colin Ian King, Mauro Carvalho Chehab, Florian Fainelli,
Mauro Carvalho Chehab, Michael Krufky, Alexander Viro, Shuah Khan,
Jaedon Shin, Katsuhiro Suzuki, Satendra Singh Thakur,
open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB),
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20200605162518.28099-1-florian.fainelli@broadcom.com>
From: Colin Ian King <colin.king@canonical.com>
commit a9e4998073d49a762a154a6b48a332ec6cb8e6b1 upstream
The fe_status variable s is not initialized meaning it can have any
random garbage status. This could be problematic if fe->ops.tune is
false as s is not updated by the call to fe->ops.tune() and a
subsequent check on the change status will using a garbage value.
Fix this by adding FE_NONE to the enum fe_status and initializing
s to this.
Detected by CoverityScan, CID#112887 ("Uninitialized scalar variable")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/media/dvb-core/dvb_frontend.c | 2 +-
include/uapi/linux/dvb/frontend.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2f054db8807b..372057cabea4 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -629,7 +629,7 @@ static int dvb_frontend_thread(void *data)
struct dvb_frontend *fe = data;
struct dtv_frontend_properties *c = &fe->dtv_property_cache;
struct dvb_frontend_private *fepriv = fe->frontend_priv;
- enum fe_status s;
+ enum fe_status s = FE_NONE;
enum dvbfe_algo algo;
bool re_tune = false;
bool semheld = false;
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd21ee2..afc3972b0879 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -127,6 +127,7 @@ enum fe_sec_mini_cmd {
* to reset DiSEqC, tone and parameters
*/
enum fe_status {
+ FE_NONE = 0x00,
FE_HAS_SIGNAL = 0x01,
FE_HAS_CARRIER = 0x02,
FE_HAS_VITERBI = 0x04,
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox