cron job: media_tree daily build: OK
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 Sep 29 05:00:11 CEST 2018 media-tree git hash:4158757395b300b6eb308fc20b96d1d231484413 media_build git hash: 44385b9c61ecc27059a651885895c8ea09cd4179 v4l-utils git hash: 7bde5ef172bd4a09b9544788ba9c5dbb1aa9994a edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.10-marune linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK 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.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.123-i686: OK linux-3.18.123-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.158-i686: OK linux-4.4.158-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.129-i686: OK linux-4.9.129-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.72-i686: OK linux-4.14.72-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.10-i686: OK linux-4.18.10-x86_64: OK linux-4.19-rc5-i686: OK linux-4.19-rc5-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.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
Hello
I have a legal business proposal worth $30.5M for you if interested reply me for more details. Regards, Ms Margaret Kwan Wing Han
Re: [PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
Hi Hugues, On Thu, Sep 27, 2018 at 03:59:04PM +, Hugues FRUCHET wrote: > Hi Maxime & all OV5640 stakeholders, > > I've just pushed a new patchset also related to rate/pixel clock > handling [1], based on your V3 great work: > >media: ov5640: Adjust the clock based on the expected rate > >media: ov5640: Remove the clocks registers initialization > >media: ov5640: Remove redundant defines > >media: ov5640: Remove redundant register setup > >media: ov5640: Compute the clock rate at runtime > >media: ov5640: Remove pixel clock rates > >media: ov5640: Enhance FPS handling > > This is working perfectly fine on my parallel setup and allows me to > well support VGA@30fps (instead 27) and also support XGA(1024x768)@15fps > that I never seen working before. > So at least for the parallel setup, this serie is working fine for all > the discrete resolutions and framerate exposed by the driver for the moment: > * QCIF 176x144 15/30fps > * QVGA 320x240 15/30fps > * VGA 640x480 15/30fps > * 480p 720x480 15/30fps > * XGA 1024x768 15/30fps > * 720p 1280x720 15/30fps > * 1080p 1920x1080 15/30fps > * 5Mp 2592x1944 15fps I'm glad this is working for you as well. I guess I'll resubmit these patches, but this time making sure someone with a CSI setup tests before merging. I crtainly don't want to repeat the previous disaster. Do you have those patches rebased somewhere? I'm not quite sure how to fix the conflict with the v4l2_find_nearest_size addition. > Moreover I'm not clear on relationship between rate and pixel clock > frequency. > I've understood that to DVP_PCLK_DIVIDER (0x3824) register > and VFIFO_CTRL0C (0x460c) affects the effective pixel clock frequency. > All the resolutions up to 720x576 are forcing a manual value of 2 for > divider (0x460c=0x22), but including 720p and more, the divider value is > controlled by "auto-mode" (0x460c=0x20)... from what I measured and > understood, for those resolutions, the divider must be set to 1 in order > that your rate computation match the effective pixel clock on output, > see [2]. > > So I wonder if this PCLK divider register should be included > or not into your rate computation, what do you think ? Have you tried change the PCLK divider while in auto-mode? IIRC, I did that and it was affecting the PCLK rate on my scope, but I wouldn't be definitive about it. Can we always set the mode to auto and divider to 1, even for the lower resolutions? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
INVESTMENT!
I'm interested in funding your business project aimed at reaching an agreement.
Re: [PATCH 0/5] Add units to controls
On 09/28/2018 03:25 PM, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday, 25 September 2018 15:30:31 EEST Sakari Ailus wrote: >> On Tue, Sep 25, 2018 at 01:48:02PM +0200, Helmut Grohne wrote: >>> On Tue, Sep 25, 2018 at 12:14:29PM +0200, Sakari Ailus wrote: This set adds a few things to the current control framework in terms of what kind of information the user space may have on controls. It adds support for units and prefixes, exponential base as well as information on whether a control is linear or exponential, to struct v4l2_query_ext_ctrl. >>> >>> That's a great improvement. Being able to give meaning to controls is >>> great. However, I see two significant weaknesses in the approach being >>> taken: >>> >>> 1. There are a number of controls whose value is not easily described as >>>either linear or exponential. I'm faced with at least two controls >>>that actually are floating point numbers. One with two bits for the >>>exponent and one (strange) bit for the mantissa (no joke) and another >>>with three bits for the exponent and four bits for the mantissa. >>>Neither can suitably be represented. >>> >>>Since the value ranges are small for the cases I mentioned, I looked >>>into using an integer menu. The present approach does not allow for >>>replacing an integer with an integer menu though. Each control id has >>>a fixed type. I'm not sure how to solve this. >> >> The proposal does not address all potential situations, that's true. >> There's no way to try to represent everything out there (without >> enumerating the values) in an easily generalised way but something can be >> done. > > [1] > >> There are devices such as some flash LED controllers where the flash current >> if simply a value you can pick from the list. It's currently implemented as >> an integer control. AFAIR the driver is drivers/leds/leds-aat1290.c . >> >>> 2. The present implementation places the responsibility of assigning >>>units and scales into drivers. A number of controls (e.g. >>>V4L2_CID_EXPOSURE_ABSOLUTE, V4L2_CID_AUDIO_LIMITER_RELEASE_TIME, >>>V4L2_CID_PIXEL_RATE, ...) clearly state the scale and unit in the >>>documentation. Having each and every driver set units and scales in >>>the documented way will lead to duplication and buggy code. Having >>>each driver choose unit and scale will lead to inconsistency. It >>>would be very good to have a mechanism that puts the framework in >>>charge of maintaining units and scales for the standard control ids. >>> >>>What is a consumer supposed to do with a control that changes unit? >>>The algorithm expected e.g. a duration. It might be able to convert >>>to pixels, but what should it do if it gets back amperes? I argue >>>that the unit should be a property of the control id and be >>>documented (i.e. what is done now). If it is reported via an ioctl, >> >> The fact is that the unit is specific to hardware. The documentation >> documents something, and often suggests a unit, but if the hardware does >> something else, then that's what you get --- independently of what the >> documentation says. >> >> Hence the need to convey it via the API. >> >> Some controls could have the unit set by the framework if that makes sense. >> Most drivers shouldn't actually need to touch this if they're fine with >> defaults (whenever a control has a default). > > I have to agree with Helmut here. If we don't handle the 95% of the common > cases in the core, we'll never get it right. Drivers will be buggy in many > ways. Which leads me to a related comment, we need v4l2-compliance support > for > this, to verify that each control reports valid information. Indeed. > >>>the framework needs to be in charge of setting it. >>> >>>The story is much different for scales. Scaling the value up and down >>>is something consumers can reasonably be expected to do. It allows >>>shifting the value range such that the relevant values can be fully >>>represented. Allowing drivers to change scales is much more >>>reasonable. Still the framework should provide a default such that >>>most drivers should not need any update. >>> >>> I acknowledge that these expectations are high and see that they're >>> partially covered in your later remarks. >>> The smiapp driver gains support for the feature. In the near term, some controls could also be assigned the unit automatically. The pixel rate, for instance. Fewer driver changes would be needed this way. A driver could override the value if there's a need to. >>> >>> I believe that in the interest of keeping maintenance cost low, this >>> should happen rather sooner than later. Just even adding the support to >>> smiapp seems wrong when it would be possible to have the framework do >>> the work. >>> I think I'll merge the undefined and no unit cases. Same for the exponential bas
Re: [PATCH 5/5] smiapp: Set control units
On 09/25/2018 12:14 PM, Sakari Ailus wrote: > Assign units for the controls exposed by the smiapp driver. > > Signed-off-by: Sakari Ailus > --- > drivers/media/i2c/smiapp/smiapp-core.c | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c > b/drivers/media/i2c/smiapp/smiapp-core.c > index 99f3b295ae3c7..289313c232430 100644 > --- a/drivers/media/i2c/smiapp/smiapp-core.c > +++ b/drivers/media/i2c/smiapp/smiapp-core.c > @@ -562,17 +562,10 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) > sensor->vblank = v4l2_ctrl_new_std( > &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, > V4L2_CID_VBLANK, 0, 1, 1, 0); > - > - if (sensor->vblank) > - sensor->vblank->flags |= V4L2_CTRL_FLAG_UPDATE; > - > sensor->hblank = v4l2_ctrl_new_std( > &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, > V4L2_CID_HBLANK, 0, 1, 1, 0); > > - if (sensor->hblank) > - sensor->hblank->flags |= V4L2_CTRL_FLAG_UPDATE; > - > sensor->pixel_rate_parray = v4l2_ctrl_new_std( > &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, > V4L2_CID_PIXEL_RATE, 1, INT_MAX, 1, 1); > @@ -589,6 +582,13 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) > return sensor->pixel_array->ctrl_handler.error; > } > > + sensor->exposure->unit = V4L2_CTRL_UNIT_LINE; > + sensor->vblank->flags |= V4L2_CTRL_FLAG_UPDATE; > + sensor->vblank->unit = V4L2_CTRL_UNIT_LINE; You can fill in the unit for this control in v4l2_ctrl_fill(). That means that this function gets extra arguments, but that's OK. There are probably quite a few controls were you can be explicit about the unit. BTW, it might be easier to add a new v4l2_ctrl_fill_units() function, I'm not sure what is best. > + sensor->hblank->flags |= V4L2_CTRL_FLAG_UPDATE; > + sensor->hblank->unit = V4L2_CTRL_UNIT_PIXEL; > + sensor->pixel_rate_parray->unit = V4L2_CTRL_UNIT_PIXELS_PER_SEC; > + > sensor->pixel_array->sd.ctrl_handler = > &sensor->pixel_array->ctrl_handler; > > @@ -611,6 +611,8 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) > return sensor->src->ctrl_handler.error; > } > > + sensor->pixel_rate_csi->unit = V4L2_CTRL_UNIT_PIXELS_PER_SEC; > + > sensor->src->sd.ctrl_handler = &sensor->src->ctrl_handler; > > return 0; > Regards, Hans
Re: [PATCH 4/5] v4l: controls: QUERY_EXT_CTRL support for base, prefix and unit
On 09/25/2018 12:14 PM, Sakari Ailus wrote:
> Add support for conveying the information set by the driver to the user
> space.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 3 +++
> include/media/v4l2-ctrls.h | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index ee006d34c19f0..8d2931b0a4701 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2732,6 +2732,9 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
> struct v4l2_query_ext_ctr
> qc->minimum = ctrl->minimum;
> qc->maximum = ctrl->maximum;
> qc->default_value = ctrl->default_value;
> + qc->base = ctrl->base;
> + qc->prefix = ctrl->prefix;
> + qc->unit = ctrl->unit;
> if (ctrl->type == V4L2_CTRL_TYPE_MENU
> || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
> qc->step = 1;
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index f615ba1b29dd9..d6aaf45b70381 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -220,6 +220,8 @@ struct v4l2_ctrl {
> u32 elem_size;
> u32 dims[V4L2_CTRL_MAX_DIMS];
> u32 nr_of_dims;
> + u8 base, unit;
> + u16 prefix;
> union {
> u64 step;
> u64 menu_skip_mask;
>
You need to add base/unit/prefix to struct v4l2_ctrl_config as well.
Regards,
Hans
Re: [PATCH 3/5] Documentation: media: Document control exponential bases, units, prefixes
On 09/25/2018 12:14 PM, Sakari Ailus wrote:
> Document V4L2 control exponential bases, units and prefixes, as well as
> the control flag telling a control value is an exponent.
>
> Signed-off-by: Sakari Ailus
> ---
> Documentation/media/uapi/v4l/extended-controls.rst | 2 +
> Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 152
> -
> Documentation/media/videodev2.h.rst.exceptions | 22 +++
> 3 files changed, 175 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index 9f7312bf33651..8461fd92d1b9e 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -3460,6 +3460,8 @@ Image Process Control IDs
> by selecting the desired horizontal and vertical blanking. The unit
> of this control is Hz.
>
> +.. _v4l2_cid_pixel_rate:
> +
> ``V4L2_CID_PIXEL_RATE (64-bit integer)``
> Pixel rate in the source pads of the subdev. This control is
> read-only and its unit is pixels / second.
> diff --git a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> index ff2d131223b84..472378f5d7566 100644
> --- a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> @@ -269,8 +269,22 @@ See also the examples in :ref:`control`.
>- ``dims[V4L2_CTRL_MAX_DIMS]``
>- The size of each dimension. The first ``nr_of_dims`` elements of
> this array must be non-zero, all remaining elements must be zero.
> +* - __u8
> + - ``base``
> + - The exponential base of the control value. Valid only if the
> + :ref:`V4L2_CTRL_FLAG_EXPONENTIAL ` control flag is
> + set. This is an enumeration.
> +* - __u8
> + - ``prefix``
> + - Prefix of the unit. This is an enumeration.
> +* - __u16
> + - ``unit``
> + - Unit of the value. Together with the ``prefix`` as well as the
> ``base``
> + field (if :ref:`V4L2_CTRL_FLAG_EXPONENTIAL ` is set),
> + defines the relation between the control value and the property of the
> + hardware being controlled. This is an enumeration.
> * - __u32
> - - ``reserved``\ [32]
> + - ``reserved``\ [31]
>- Reserved for future extensions. Applications and drivers must set
> the array to zero.
>
> @@ -523,6 +537,142 @@ See also the examples in :ref:`control`.
> streaming is in progress since most drivers do not support changing
> the format in that case.
>
> +* .. _FLAG_EXPONENTIAL:
> +
> + - ``V4L2_CTRL_FLAG_EXPONENTIAL``
> + - 0x0800
> +
> + - The value of the control has an exponential relation to the feature
> + being controled instead of a linear relation. In other words, the value
controled -> controlled
> + of the control is an exponent of the base specified in the
> +base field in &struct v4l2_query_ext_ctrl.
It would be nice if you can add some math here. See colorspaces-details.rst
for examples of that.
> +
> +
> +.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> +
> +.. _control-bases:
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: Control Exponential Bases
> +:header-rows: 1
> +:stub-columns: 0
> +:widths: 3 1 4
> +
> +* - Base Name
> + - Value
> + - Description
> +
> +* - ``V4L2_CTRL_BASE_UNDEFINED``
> + - 0
> + - The control exponential base is not defined.
> +
> +* - ``V4L2_CTRL_BASE_LINEAR``
> + - 1
> + - The control is linear.
> +
> +* - ``V4L2_CTRL_BASE_2``
> + - 2
> + - The base of the control is 2.
> +
> +* - ``V4L2_CTRL_BASE_10``
> + - 10
> + - The base of the control is 10.
> +
> +.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> +
> +.. _control-prefixes:
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: Control Prefixes
> +:header-rows: 1
> +:stub-columns: 0
> +:widths: 3 1 4
> +
> +* - Prefix Name
> + - Value
> + - Description
> +
> +* - ``V4L2_CTRL_PREFIX_NANO``
> + - -9
> + - Nano
> +
> +* - ``V4L2_CTRL_PREFIX_MICRO``
> + - -6
> + - Micro
> +
> +* - ``V4L2_CTRL_PREFIX_MILLI``
> + - -3
> + - Milli
> +
> +* - ``V4L2_CTRL_PREFIX_1``
> + - 0
> + - \-
> +
> +* - ``V4L2_CTRL_PREFIX_KILO``
> + - 3
> + - Kilo
> +
> +* - ``V4L2_CTRL_PREFIX_MEGA``
> + - 6
> + - Mega
> +
> +* - ``V4L2_CTRL_PREFIX_GIGA``
> + - 9
> + - Giga
> +
> +.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> +
> +.. _control-units:
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: Control Units
> +:header-rows: 1
> +:stub-columns: 0
> +:widths: 3 1 4
> +
> +* - Unit Name
> + - Value
> + - Description
> +
> +* - ``V4L2_CTRL_UNIT_UNDEFINED``
> + - 0
> + - The unit of
Re: [PATCH 2/5] v4l: controls: Add support for exponential bases, prefixes and units
On 09/25/2018 12:14 PM, Sakari Ailus wrote:
> Add support for exponential bases, prefixes as well as units for V4L2
> controls. This makes it possible to convey information on the relation
> between the control value and the hardware feature being controlled.
>
> Signed-off-by: Sakari Ailus
> ---
> include/uapi/linux/videodev2.h | 32 +++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index ae083978988f1..23b02f2db85a1 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1652,6 +1652,32 @@ struct v4l2_queryctrl {
> __u32reserved[2];
> };
>
> +/* V4L2 control exponential bases */
> +#define V4L2_CTRL_BASE_UNDEFINED 0
> +#define V4L2_CTRL_BASE_LINEAR1
I'm not really sure you need BASE_LINEAR. That is effectively the same
as UNDEFINED since what else can you do? It's also weird to have this
as 'base' if the EXPONENTIAL flag is set.
I don't see why you need the EXPONENTIAL flag at all: if this is non-0,
then you know the exponential base.
> +#define V4L2_CTRL_BASE_2 2
> +#define V4L2_CTRL_BASE_1010
> +
> +/* V4L2 control unit prefixes */
> +#define V4L2_CTRL_PREFIX_NANO-9
> +#define V4L2_CTRL_PREFIX_MICRO -6
> +#define V4L2_CTRL_PREFIX_MILLI -3
> +#define V4L2_CTRL_PREFIX_1 0
I would prefer PREFIX_NONE, since there is no prefix in this case.
I assume this prefix is only valid if the unit is not UNDEFINED and not
NONE?
Is 'base' also dependent on a valid unit? (it doesn't appear to be)
> +#define V4L2_CTRL_PREFIX_KILO3
> +#define V4L2_CTRL_PREFIX_MEGA6
> +#define V4L2_CTRL_PREFIX_GIGA9
> +
> +/* V4L2 control units */
> +#define V4L2_CTRL_UNIT_UNDEFINED 0
> +#define V4L2_CTRL_UNIT_NONE 1
> +#define V4L2_CTRL_UNIT_SECOND2
> +#define V4L2_CTRL_UNIT_AMPERE3
> +#define V4L2_CTRL_UNIT_LINE 4
> +#define V4L2_CTRL_UNIT_PIXEL 5
> +#define V4L2_CTRL_UNIT_PIXELS_PER_SEC6
> +#define V4L2_CTRL_UNIT_HZ7
> +
> +
> /* Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls */
> struct v4l2_query_ext_ctrl {
> __u32id;
> @@ -1666,7 +1692,10 @@ struct v4l2_query_ext_ctrl {
> __u32elems;
> __u32nr_of_dims;
> __u32dims[V4L2_CTRL_MAX_DIMS];
> - __u32reserved[32];
> + __u8 base;
> + __s8 prefix;
> + __u16unit;
> + __u32reserved[31];
> };
>
> /* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */
> @@ -1692,6 +1721,7 @@ struct v4l2_querymenu {
> #define V4L2_CTRL_FLAG_HAS_PAYLOAD 0x0100
> #define V4L2_CTRL_FLAG_EXECUTE_ON_WRITE 0x0200
> #define V4L2_CTRL_FLAG_MODIFY_LAYOUT 0x0400
> +#define V4L2_CTRL_FLAG_EXPONENTIAL 0x0800
>
> /* Query flags, to be ORed with the control ID */
> #define V4L2_CTRL_FLAG_NEXT_CTRL 0x8000
>
Regards,
Hans
Re: [PATCH v5] media: imx208: Add imx208 camera sensor driver
Hi Helmut,
On Friday, 21 September 2018 10:23:37 EEST Helmut Grohne wrote:
> On Thu, Sep 20, 2018 at 11:00:26PM +0200, Laurent Pinchart wrote:
> > On Thursday, 20 September 2018 23:16:47 EEST Sylwester Nawrocki wrote:
> >> On 09/20/2018 06:49 PM, Grant Grundler wrote:
> >>> On Thu, Sep 20, 2018 at 1:52 AM Tomasz Figa wrote:
> We have a problem here. The sensor supports only a discrete range of
> values here - {1, 2, 4, 8, 16} (multiplied by 256, since the value is
> fixed point). This makes it possible for the userspace to set values
> that are not allowed by the sensor specification and also leaves no
> way to enumerate the supported values.
>
> I'm not sure I understand this correctly. Tomasz appears to imply here
> that the number of actually supported gain values is 5.
>
> >> I'm not sure if it's best approach but I once did something similar for
> >> the ov9650 sensor. The gain was fixed point 10-bits value with 4 bits
> >> for fractional part. The driver reports values multiplied by 16. See
> >> ov965x_set_gain() function in drivers/media/i2c/ov9650.c and "Table 4-1.
> >> Total Gain to Control Bit Correlation" in the OV9650 datasheet for
> >> details. The integer menu control just seemed not suitable for 2^10
> >> values.
>
> As long as values scale linearly (as is the case for fixed point
> numbers) that seems like a reasonable approach to me. That assumption
> appears to not hold for imx208 where the values scale exponentially.
And it's often neither. For instance the MT9P031 has three gain stages.
Quoting the driver:
/* Gain is controlled by 2 analog stages and a digital stage.
* Valid values for the 3 stages are
*
* StageMin Max Step
* --
* First analog stage x1 x2 1
* Second analog stage x1 x4 0.125
* Digital stagex1 x16 0.125
The resulting gain is the product of the three.
> > I've had a similar discussion on IRC recently with Helmut, who posted a
> > nice summary of the problem on the mailing list (see
> > https://www.mail-archive.com/
> > linux-media@vger.kernel.org/msg134502.html). This is a known issue, and
> > while I proposed the same approach, I understand that in some cases
> > userspace may need to know exactly what values are acceptable. In such a
> > case, however, I would expect userspace to have knowledge of the
> > particular sensor model, so the information may not need to come from the
> > kernel.
>
> A big reason to use V4L2 is to abstract hardware. Having to know what
> sensor model you use runs counter that goal. Indeed, gain (whether
> digital or analogue) can be abstracted in principle. I see little reason
> to not do that.
The purpose of V4L2 is to expose the features of the hardware device to
userspace, and we try to do so in an as much abstract as possible way. 100%
abstraction isn't feasible, there will always be small device-specific details
that will break whatever abstraction we design. We should thus strive for a
good balance, abstracting the most common types of features, while avoiding
over-engineering an API that would then become unusable (both because it would
be complex to use by applications, and implemented in subtly differently buggy
ways by drivers).
I don't think we'll reach an agreement here if we don't start talking about
real use cases. Would you have some to share ?
> >> Now the gain control has range 16...1984 out of which only 1024 values
> >> are valid. It might not be best approach for a GUI but at least the
> >> driver exposes mapping of all valid values, which could be enumerated
> >> with VIDIOC_TRY_EXT_CTRLS if required, without a need for a driver-
> >> specific user space code.
>
> If you have very many values, a reasonable compromise could be reducing
> the precision. If you try to represent a floating point number, values
> with higher exponent will have larger "gaps" when represented as
> integers for v4l2. A compromise could be increasing the step size and
> thus removing a few of the gains with lower exponent.
What if an application needs the precision ? Reducing it can cause issues in
the 3A algorithms, including closed-loops stability problems. I think we
should expose the device features with as much precision and coverage as
possible. Hardcoding use case assumptions in the kernel drives us into a wall
sooner or later.
> I can see two solutions here:
>
> 1) Define the control range from 0 to 4 and treat it as an exponent
> of 2, so that the value for the sensor becomes (1 << val) * 256.
> (Suggested by Sakari offline.)
>
> This approach has the problem of losing the original unit (and scale)
> of the value.
>
> This approach is the one where users will need to know which sensor they
> talk to. The one whe
Re: [PATCH 1/5] videodev2.h: Use 8 hexadecimals (32 bits) for control flags
On 09/25/2018 12:14 PM, Sakari Ailus wrote:
> The V4L2 control flags are a 32-bit bitmask. Use 32-bit hexadecimal
> numbers to specify the flags (was 16).
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
Regards,
Hans
> ---
> Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 22 +++---
> include/uapi/linux/videodev2.h| 22 +++---
> 2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> index 5bd26e8c9a1a0..ff2d131223b84 100644
> --- a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> @@ -439,37 +439,37 @@ See also the examples in :ref:`control`.
> :widths: 3 1 4
>
> * - ``V4L2_CTRL_FLAG_DISABLED``
> - - 0x0001
> + - 0x0001
>- This control is permanently disabled and should be ignored by the
> application. Any attempt to change the control will result in an
> ``EINVAL`` error code.
> * - ``V4L2_CTRL_FLAG_GRABBED``
> - - 0x0002
> + - 0x0002
>- This control is temporarily unchangeable, for example because
> another application took over control of the respective resource.
> Such controls may be displayed specially in a user interface.
> Attempts to change the control may result in an ``EBUSY`` error code.
> * - ``V4L2_CTRL_FLAG_READ_ONLY``
> - - 0x0004
> + - 0x0004
>- This control is permanently readable only. Any attempt to change
> the control will result in an ``EINVAL`` error code.
> * - ``V4L2_CTRL_FLAG_UPDATE``
> - - 0x0008
> + - 0x0008
>- A hint that changing this control may affect the value of other
> controls within the same control class. Applications should update
> their user interface accordingly.
> * - ``V4L2_CTRL_FLAG_INACTIVE``
> - - 0x0010
> + - 0x0010
>- This control is not applicable to the current configuration and
> should be displayed accordingly in a user interface. For example
> the flag may be set on a MPEG audio level 2 bitrate control when
> MPEG audio encoding level 1 was selected with another control.
> * - ``V4L2_CTRL_FLAG_SLIDER``
> - - 0x0020
> + - 0x0020
>- A hint that this control is best represented as a slider-like
> element in a user interface.
> * - ``V4L2_CTRL_FLAG_WRITE_ONLY``
> - - 0x0040
> + - 0x0040
>- This control is permanently writable only. Any attempt to read the
> control will result in an ``EACCES`` error code error code. This flag
> is typically present for relative controls or action controls
> @@ -477,7 +477,7 @@ See also the examples in :ref:`control`.
> action (e. g. motor control) but no meaningful value can be
> returned.
> * - ``V4L2_CTRL_FLAG_VOLATILE``
> - - 0x0080
> + - 0x0080
>- This control is volatile, which means that the value of the
> control changes continuously. A typical example would be the
> current gain value if the device is in auto-gain mode. In such a
> @@ -493,7 +493,7 @@ See also the examples in :ref:`control`.
> Setting a new value for a volatile control will *never* trigger a
> :ref:`V4L2_EVENT_CTRL_CH_VALUE ` event.
> * - ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
> - - 0x0100
> + - 0x0100
>- This control has a pointer type, so its value has to be accessed
> using one of the pointer fields of struct
> :c:type:`v4l2_ext_control`. This flag is set
> @@ -503,7 +503,7 @@ See also the examples in :ref:`control`.
> * .. _FLAG_EXECUTE_ON_WRITE:
>
>- ``V4L2_CTRL_FLAG_EXECUTE_ON_WRITE``
> - - 0x0200
> + - 0x0200
>- The value provided to the control will be propagated to the driver
> even if it remains constant. This is required when the control
> represents an action on the hardware. For example: clearing an
> @@ -512,7 +512,7 @@ See also the examples in :ref:`control`.
> * .. _FLAG_MODIFY_LAYOUT:
>
>- ``V4L2_CTRL_FLAG_MODIFY_LAYOUT``
> - - 0x0400
> + - 0x0400
>- Changing this control value may modify the layout of the
> buffer (for video devices) or the media bus format (for sub-devices).
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 184e4dbe8f9c0..ae083978988f1 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1681,17 +1681,17 @@ struct v4l2_querymenu {
> } __attribute__ ((packed));
>
> /* Control flags */
> -#define V4L2_CTRL_FLAG_DISABLED 0x0001
> -#define V4L2_CTRL_FLAG_GRABBED 0x0002
> -#define V4L2_CTRL_FLAG_READ_ONLY 0x0004
> -#define V4L2_CTRL_FLAG_UPDATE0x0008
> -#define
Re: [RFC,1/3] cpia2: move to staging in preparation for removal
Hi Andrea, On 09/28/2018 03:03 PM, Andrea Merello wrote: > I do often use this driver, and I'm interested in working on it for > preventing it from being removed. > > I can perform functional test with my HW (usb microscope) on a kernel from > current media tree (anyway currently it works on my box with a pretty recent > kernel). > > How much effort is expected to be required to port it to vb2? I'm currently > hacking on another (recent) v4l2 subdev driver, but my wknowledge of the > v4l2/media framework is far from good.. If someone give me some directions > then I can try to do that.. > cpia2 has its own streaming I/O implementation. This should be completely replaced by vb2. Easiest is to look at a fairly recent usb driver like usbtv to see how it is done there. The vb2 API is fairly clean (see include/media/videobuf2-core.h), but switching to vb2 is a big-bang action, you can't switch a little bit, it is all or nothing. So that makes this a big unreadable patch in the end. The v4l2-compliance utility is your friend when testing this. If you would be willing to work on this, then it's easiest if you use the #v4l channel on freenode irc to ask questions (which I am sure you'll have). It's a fair amount of work, I'm afraid. It would probably take me 1-2 days to convert depending on how nice the rest of the cpia2 driver is. Regards, Hans
Re: [PATCH 0/5] Add units to controls
Hi Sakari, On Tuesday, 25 September 2018 15:30:31 EEST Sakari Ailus wrote: > On Tue, Sep 25, 2018 at 01:48:02PM +0200, Helmut Grohne wrote: > > On Tue, Sep 25, 2018 at 12:14:29PM +0200, Sakari Ailus wrote: > >> This set adds a few things to the current control framework in terms of > >> what kind of information the user space may have on controls. It adds > >> support for units and prefixes, exponential base as well as information > >> on whether a control is linear or exponential, to struct > >> v4l2_query_ext_ctrl. > > > > That's a great improvement. Being able to give meaning to controls is > > great. However, I see two significant weaknesses in the approach being > > taken: > > > > 1. There are a number of controls whose value is not easily described as > >either linear or exponential. I'm faced with at least two controls > >that actually are floating point numbers. One with two bits for the > >exponent and one (strange) bit for the mantissa (no joke) and another > >with three bits for the exponent and four bits for the mantissa. > >Neither can suitably be represented. > > > >Since the value ranges are small for the cases I mentioned, I looked > >into using an integer menu. The present approach does not allow for > >replacing an integer with an integer menu though. Each control id has > >a fixed type. I'm not sure how to solve this. > > The proposal does not address all potential situations, that's true. > There's no way to try to represent everything out there (without > enumerating the values) in an easily generalised way but something can be > done. [1] > There are devices such as some flash LED controllers where the flash current > if simply a value you can pick from the list. It's currently implemented as > an integer control. AFAIR the driver is drivers/leds/leds-aat1290.c . > > > 2. The present implementation places the responsibility of assigning > >units and scales into drivers. A number of controls (e.g. > >V4L2_CID_EXPOSURE_ABSOLUTE, V4L2_CID_AUDIO_LIMITER_RELEASE_TIME, > >V4L2_CID_PIXEL_RATE, ...) clearly state the scale and unit in the > >documentation. Having each and every driver set units and scales in > >the documented way will lead to duplication and buggy code. Having > >each driver choose unit and scale will lead to inconsistency. It > >would be very good to have a mechanism that puts the framework in > >charge of maintaining units and scales for the standard control ids. > > > >What is a consumer supposed to do with a control that changes unit? > >The algorithm expected e.g. a duration. It might be able to convert > >to pixels, but what should it do if it gets back amperes? I argue > >that the unit should be a property of the control id and be > >documented (i.e. what is done now). If it is reported via an ioctl, > > The fact is that the unit is specific to hardware. The documentation > documents something, and often suggests a unit, but if the hardware does > something else, then that's what you get --- independently of what the > documentation says. > > Hence the need to convey it via the API. > > Some controls could have the unit set by the framework if that makes sense. > Most drivers shouldn't actually need to touch this if they're fine with > defaults (whenever a control has a default). I have to agree with Helmut here. If we don't handle the 95% of the common cases in the core, we'll never get it right. Drivers will be buggy in many ways. Which leads me to a related comment, we need v4l2-compliance support for this, to verify that each control reports valid information. > >the framework needs to be in charge of setting it. > > > >The story is much different for scales. Scaling the value up and down > >is something consumers can reasonably be expected to do. It allows > >shifting the value range such that the relevant values can be fully > >represented. Allowing drivers to change scales is much more > >reasonable. Still the framework should provide a default such that > >most drivers should not need any update. > > > > I acknowledge that these expectations are high and see that they're > > partially covered in your later remarks. > > > >> The smiapp driver gains support for the feature. In the near term, some > >> controls could also be assigned the unit automatically. The pixel rate, > >> for instance. Fewer driver changes would be needed this way. A driver > >> could override the value if there's a need to. > > > > I believe that in the interest of keeping maintenance cost low, this > > should happen rather sooner than later. Just even adding the support to > > smiapp seems wrong when it would be possible to have the framework do > > the work. > > > >> I think I'll merge the undefined and no unit cases. Same for the > >> exponential base actually --- the flag can be removed, too... > > > > I'm not sure I understand. It reads lik
Re: [PATCH 1/5] videodev2.h: Use 8 hexadecimals (32 bits) for control flags
Hi Sakari,
Thank you for the patch.
On Tuesday, 25 September 2018 13:14:30 EEST Sakari Ailus wrote:
> The V4L2 control flags are a 32-bit bitmask. Use 32-bit hexadecimal
> numbers to specify the flags (was 16).
>
> Signed-off-by: Sakari Ailus
This is the easy one,
Reviewed-by: Laurent Pinchart
> ---
> Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 22 ++--
> include/uapi/linux/videodev2.h| 22 ++--
> 2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst index
> 5bd26e8c9a1a0..ff2d131223b84 100644
> --- a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst
> @@ -439,37 +439,37 @@ See also the examples in :ref:`control`.
>
> :widths: 3 1 4
>
> * - ``V4L2_CTRL_FLAG_DISABLED``
> - - 0x0001
> + - 0x0001
>- This control is permanently disabled and should be ignored by the
> application. Any attempt to change the control will result in an
> ``EINVAL`` error code.
> * - ``V4L2_CTRL_FLAG_GRABBED``
> - - 0x0002
> + - 0x0002
>- This control is temporarily unchangeable, for example because
> another application took over control of the respective resource.
> Such controls may be displayed specially in a user interface.
> Attempts to change the control may result in an ``EBUSY`` error code.
> * - ``V4L2_CTRL_FLAG_READ_ONLY``
> - - 0x0004
> + - 0x0004
>- This control is permanently readable only. Any attempt to change
> the control will result in an ``EINVAL`` error code.
> * - ``V4L2_CTRL_FLAG_UPDATE``
> - - 0x0008
> + - 0x0008
>- A hint that changing this control may affect the value of other
> controls within the same control class. Applications should update
> their user interface accordingly.
> * - ``V4L2_CTRL_FLAG_INACTIVE``
> - - 0x0010
> + - 0x0010
>- This control is not applicable to the current configuration and
> should be displayed accordingly in a user interface. For example
> the flag may be set on a MPEG audio level 2 bitrate control when
> MPEG audio encoding level 1 was selected with another control.
> * - ``V4L2_CTRL_FLAG_SLIDER``
> - - 0x0020
> + - 0x0020
>- A hint that this control is best represented as a slider-like
> element in a user interface.
> * - ``V4L2_CTRL_FLAG_WRITE_ONLY``
> - - 0x0040
> + - 0x0040
>- This control is permanently writable only. Any attempt to read the
> control will result in an ``EACCES`` error code error code. This flag
> is typically present for relative controls or action controls
> @@ -477,7 +477,7 @@ See also the examples in :ref:`control`.
> action (e. g. motor control) but no meaningful value can be
> returned.
> * - ``V4L2_CTRL_FLAG_VOLATILE``
> - - 0x0080
> + - 0x0080
>- This control is volatile, which means that the value of the
> control changes continuously. A typical example would be the
> current gain value if the device is in auto-gain mode. In such a
> @@ -493,7 +493,7 @@ See also the examples in :ref:`control`.
> Setting a new value for a volatile control will *never* trigger a
>
> :ref:`V4L2_EVENT_CTRL_CH_VALUE ` event.
>
> * - ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
> - - 0x0100
> + - 0x0100
>- This control has a pointer type, so its value has to be accessed
> using one of the pointer fields of struct
>
> :c:type:`v4l2_ext_control`. This flag is set
>
> @@ -503,7 +503,7 @@ See also the examples in :ref:`control`.
> * .. _FLAG_EXECUTE_ON_WRITE:
>
>- ``V4L2_CTRL_FLAG_EXECUTE_ON_WRITE``
> - - 0x0200
> + - 0x0200
>- The value provided to the control will be propagated to the driver
> even if it remains constant. This is required when the control
> represents an action on the hardware. For example: clearing an
> @@ -512,7 +512,7 @@ See also the examples in :ref:`control`.
> * .. _FLAG_MODIFY_LAYOUT:
>
>- ``V4L2_CTRL_FLAG_MODIFY_LAYOUT``
> - - 0x0400
> + - 0x0400
>- Changing this control value may modify the layout of the
> buffer (for video devices) or the media bus format (for
> sub-devices).
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 184e4dbe8f9c0..ae083978988f1 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1681,17 +1681,17 @@ struct v4l2_querymenu {
> } __attribute__ ((packed));
>
> /* Control flags */
> -#define V4L2_CTRL_FLAG_DISABLED 0x0001
> -#define V4L2_CTRL_FLAG_GRABBED 0x0002
> -#define V4L2_CTRL_FLAG_READ_ONLY 0x
Re: [RFC,1/3] cpia2: move to staging in preparation for removal
I do often use this driver, and I'm interested in working on it for preventing it from being removed. I can perform functional test with my HW (usb microscope) on a kernel from current media tree (anyway currently it works on my box with a pretty recent kernel). How much effort is expected to be required to port it to vb2? I'm currently hacking on another (recent) v4l2 subdev driver, but my wknowledge of the v4l2/media framework is far from good.. If someone give me some directions then I can try to do that..
Re: [PATCH] [media] v4l: xilinx: fix typo in formats table
Hi Andrea,
Thank you for the patch.
On Friday, 28 September 2018 10:32:13 EEST Andrea Merello wrote:
> In formats table the entry for CFA pattern "rggb" has GRBG fourcc.
> This patch fixes it.
>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Mirco Di Salvo
> Signed-off-by: Andrea Merello
Reviewed-by: Laurent Pinchart
Michal, should I take the patch in my tree ?
> ---
> drivers/media/platform/xilinx/xilinx-vip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/xilinx/xilinx-vip.c
> b/drivers/media/platform/xilinx/xilinx-vip.c index
> 311259129504..e9567cdfb89b 100644
> --- a/drivers/media/platform/xilinx/xilinx-vip.c
> +++ b/drivers/media/platform/xilinx/xilinx-vip.c
> @@ -36,7 +36,7 @@ static const struct xvip_video_format xvip_video_formats[]
> = { { XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
> 1, V4L2_PIX_FMT_GREY, "Greyscale 8-bit" },
> { XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
> - 1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit RGGB" },
> + 1, V4L2_PIX_FMT_SRGGB8, "Bayer 8-bit RGGB" },
> { XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
> 1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit GRBG" },
> { XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
--
Regards,
Laurent Pinchart
Re: [PATCH v6 0/6] Add Rockchip VPU JPEG encoder
On 09/17/2018 07:30 PM, Ezequiel Garcia wrote:
> This series adds support for JPEG encoding via the VPU block
> present in Rockchip platforms. Currently, support for RK3288
> and RK3399 is included.
>
> Please, see the previous versions of this cover letter for
> more information.
>
> Compared to v5, the only change is in the V4L2_CID_JPEG_QUANTIZATION
> control. We've decided to support only baseline profile,
> and only add 8-bit luma and chroma tables.
>
> struct v4l2_ctrl_jpeg_quantization {
>__u8luma_quantization_matrix[64];
>__u8chroma_quantization_matrix[64];
> };
>
> By doing this, it's clear that we don't currently support anything
> but baseline.
>
> This series should apply cleanly on top of
>
> git://linuxtv.org/hverkuil/media_tree.git br-cedrus tag.
>
> If everyone is happy with this series, I'd like to route the devicetree
> changes through the rockchip tree, and the rest via the media subsystem.
OK, so I have what is no doubt an annoying question: do we really need
a JPEG_RAW format?
The JPEG header is really easy to parse for a decoder and really easy to
prepend to the compressed image for the encoder.
The only reason I can see for a JPEG_RAW is if the image must start at
some specific address alignment. Although even then you can just pad the
JPEG header that you will add according to the alignment requirements.
I know I am very late with this question, but I never looked all that
closely at what a JPEG header looks like. But this helped:
https://en.wikipedia.org/wiki/JPEG_File_Interchange_Format
and it doesn't seem difficult at all to parse or create the header.
I also think there are more drivers (solo6x10) that manipulate the JPEG
header.
Regards,
Hans
>
> Compliance
> ==
>
> (Same results as v3)
>
> v4l2-compliance SHA: d0f4ea7ddab6d0244c4fe1e960bb2aaeefb911b9, 64 bits
>
> Compliance test for device /dev/video0:
>
> Driver Info:
> Driver name : rockchip-vpu
> Card type: rockchip,rk3399-vpu-enc
> Bus info : platform: rockchip-vpu
> Driver version : 4.18.0
> Capabilities : 0x84204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps : 0x04204000
> Video Memory-to-Memory Multiplanar
> Streaming
> Extended Pix Format
> Media Driver Info:
> Driver name : rockchip-vpu
> Model: rockchip-vpu
> Serial :
> Bus info :
> Media version: 4.18.0
> Hardware revision: 0x (0)
> Driver version : 4.18.0
> Interface Info:
> ID : 0x030c
> Type : V4L Video
> Entity Info:
> ID : 0x0001 (1)
> Name : rockchip,rk3399-vpu-enc-source
> Function : V4L2 I/O
> Pad 0x0102 : Source
> Link 0x0208: to remote pad 0x105 of entity
> 'rockchip,rk3399-vpu-enc-proc': Data, Enabled, Immutable
>
> Required ioctls:
> test MC information (see 'Media Driver Info' above): OK
> test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
> test second /dev/video0 open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
> test for unlimited opens: OK
>
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> test VIDIOC_LOG_STATUS: OK (Not Supported)
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 0 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 2 Private Controls: 0
>
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK (Not Supp
cx23885 - regression between 4.17.x and 4.18.x
Hi,
First of all, sorry for not correctly replying to the thread. I just
subscribed to the list yesterday, so I do not have the message in my
inbox.
I am referring to this message:
https://www.spinics.net/lists/linux-media/msg140286.html
My system also suffered from the problem that, with Kernel 4.18 my DVB
card became unresponsive after a few hours. I am using tvheadend as a
video recorder, and with Kernel 4.18, tvheadend started missing
recordings, with log messages like the following:
linuxdvb: Silicon Labs Si2168 : DVB-C #1 - poll TIMEOUT
subscription: 0163: service instance is bad, reason: No input detected
subscription: 0162: service instance is bad, reason: No input detected
subscription: 0163: No input source available for subscription "DVR:
..." to channel "..."
In this state, the DVB card was not usable until unloading the cx23885
module and reloading it -- just to fail again few hours later.
Also, I noticed log messages like these, not directly correlatable to
the failures (not at the same time as the failed recordings, and
sometimes also working recordings after such a log message).
kernel: cx23885 :02:00.0: dma in progress detected 0x0001
0x0001, clearing
Nevertheless, the patch introducing the log message seems also to cause
the failure:
[PATCH 3/5] cx23885: Ryzen DMA related RiSC engine stall fixes
https://www.spinics.net/lists/linux-media/msg133899.html
I never suffered the problem the patch is trying to solve (card used to
work flawlessly for months). To see if that patch is not only the
messenger, but also the culprit, I built a custom kernel based on the
kernel-4.18.9-200.fc28.x86_64 source RPM from Fedora 28, only applying
the patch above in reverse -- and did not have a single failure for > 48
hours now.
Since the author tried to solve a problem, and maybe even did for his
system, I guess reversing the patch alone is not a proper solution for
everyone. And since the author did submit this patch, I guess he also
did not suffer from the problem I have right now... so a
fits-for-all-solution might be harder to find, also because the failure
is not immediate.
Unfortunately I don't know enough about the code in question (or kernel
development as whole), so I have no clue how to analyze (or even fix)
this any further. What I can offer is to test modified versions of the
patch above on my computer, as it does not seem to affect everyone.
Some information on my system:
- HP Microserver G7 N54L with AMD Turion II Neo CPU
- DVBSky T982 PCI-E card with dual DVB-T/C tuners, identifying as
02:00.0 Multimedia video controller: Conexant Systems, Inc. CX23885
PCI Video and Audio Decoder (rev 04)
Subsystem: DVBSky T982
Flags: bus master, fast devsel, latency 0, IRQ 16, NUMA node 0
Memory at fe60 (64-bit, non-prefetchable) [size=2M]
Capabilities:
Kernel driver in use: cx23885
Kernel modules: cx23885
- Fedora 28, currently with a custom kernel, usually with the most
recent "stock" fedora kernel.
- kernel-4.18.9-200.fixdvb.fc28.x86_64 ("fixdvb" being my local
extraversion, only difference to the original kernel: the reversed patch
above)
- tvheadend-4.2.6-1.fc28.x86_64
Regards,
Markus
Re: [PATCH v6 6/6] media: add Rockchip VPU JPEG encoder driver
On 09/17/2018 07:30 PM, Ezequiel Garcia wrote: > Add a mem2mem driver for the VPU available on Rockchip SoCs. > Currently only JPEG encoding is supported, for RK3399 and RK3288 > platforms. Please run this through checkpatch.pl --strict I get a lot of warnings/checks that all look trivial to fix. Regards, Hans > > Signed-off-by: Ezequiel Garcia > --- > MAINTAINERS | 7 + > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile| 1 + > drivers/staging/media/rockchip/vpu/Makefile | 9 + > drivers/staging/media/rockchip/vpu/TODO | 5 + > .../media/rockchip/vpu/rk3288_vpu_hw.c| 123 > .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 126 > .../media/rockchip/vpu/rk3288_vpu_regs.h | 442 + > .../media/rockchip/vpu/rk3399_vpu_hw.c| 123 > .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c | 154 + > .../media/rockchip/vpu/rk3399_vpu_regs.h | 601 + > .../staging/media/rockchip/vpu/rockchip_vpu.h | 292 + > .../media/rockchip/vpu/rockchip_vpu_common.h | 31 + > .../media/rockchip/vpu/rockchip_vpu_drv.c | 528 +++ > .../media/rockchip/vpu/rockchip_vpu_enc.c | 604 ++ > .../media/rockchip/vpu/rockchip_vpu_hw.h | 65 ++ > 16 files changed, 3113 insertions(+) > create mode 100644 drivers/staging/media/rockchip/vpu/Makefile > create mode 100644 drivers/staging/media/rockchip/vpu/TODO > create mode 100644 drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c > create mode 100644 > drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c > create mode 100644 drivers/staging/media/rockchip/vpu/rk3288_vpu_regs.h > create mode 100644 drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c > create mode 100644 > drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c > create mode 100644 drivers/staging/media/rockchip/vpu/rk3399_vpu_regs.h > create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu.h > create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h > create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c > create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c > create mode 100644 drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9812ae1a1a40..44a043019d0e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12462,6 +12462,13 @@ S: Maintained > F: drivers/media/platform/rockchip/rga/ > F: Documentation/devicetree/bindings/media/rockchip-rga.txt > > +ROCKCHIP VPU CODEC DRIVER > +M: Ezequiel Garcia > +L: linux-media@vger.kernel.org > +S: Maintained > +F: drivers/staging/media/platform/rockchip/vpu/ > +F: Documentation/devicetree/bindings/media/rockchip-vpu.txt > + > ROCKER DRIVER > M: Jiri Pirko > L: net...@vger.kernel.org > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index b3620a8f2d9f..c6f3404dea43 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -31,6 +31,8 @@ source "drivers/staging/media/mt9t031/Kconfig" > > source "drivers/staging/media/omap4iss/Kconfig" > > +source "drivers/staging/media/rockchip/vpu/Kconfig" > + > source "drivers/staging/media/sunxi/Kconfig" > > source "drivers/staging/media/tegra-vde/Kconfig" > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index 42948f805548..43c7bee1fc8c 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ > obj-$(CONFIG_VIDEO_SUNXI)+= sunxi/ > obj-$(CONFIG_TEGRA_VDE) += tegra-vde/ > obj-$(CONFIG_VIDEO_ZORAN)+= zoran/ > +obj-$(CONFIG_VIDEO_ROCKCHIP_VPU) += rockchip/vpu/ > diff --git a/drivers/staging/media/rockchip/vpu/Makefile > b/drivers/staging/media/rockchip/vpu/Makefile > new file mode 100644 > index ..2fb99adfbdb9 > --- /dev/null > +++ b/drivers/staging/media/rockchip/vpu/Makefile > @@ -0,0 +1,9 @@ > +obj-$(CONFIG_VIDEO_ROCKCHIP_VPU) += rockchip-vpu.o > + > +rockchip-vpu-y += \ > + rockchip_vpu_drv.o \ > + rockchip_vpu_enc.o \ > + rk3288_vpu_hw.o \ > + rk3288_vpu_hw_jpeg_enc.o \ > + rk3399_vpu_hw.o \ > + rk3399_vpu_hw_jpeg_enc.o > diff --git a/drivers/staging/media/rockchip/vpu/TODO > b/drivers/staging/media/rockchip/vpu/TODO > new file mode 100644 > index ..99bd1add7628 > --- /dev/null > +++ b/drivers/staging/media/rockchip/vpu/TODO > @@ -0,0 +1,5 @@ > +This driver is in staging until the V4L controls stabilize. > + > +Given the V4L controls are part of the uABI, it's better > +to have the driver in staging, so users are aware of this > +driver having an unstable interface. > diff --git a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c > b/drivers/sta
Re: [PATCH v6 1/6] dt-bindings: Document the Rockchip VPU bindings
On 09/17/2018 07:30 PM, Ezequiel Garcia wrote:
> Add devicetree binding documentation for Rockchip Video Processing
> Unit IP.
>
> Reviewed-by: Rob Herring
> Signed-off-by: Ezequiel Garcia
> ---
> .../bindings/media/rockchip-vpu.txt | 30 +++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip-vpu.txt
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip-vpu.txt
> b/Documentation/devicetree/bindings/media/rockchip-vpu.txt
> new file mode 100644
> index ..5e0d421301ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-vpu.txt
> @@ -0,0 +1,30 @@
> +device-tree bindings for rockchip VPU codec
> +
> +Rockchip (Video Processing Unit) present in various Rockchip platforms,
> +such as RK3288 and RK3399.
> +
> +Required properties:
> +- compatible: value should be one of the following
> + "rockchip,rk3288-vpu";
> + "rockchip,rk3399-vpu";
> +- interrupts: encoding and decoding interrupt specifiers
> +- interrupt-names: should be "vepu" and "vdpu"
> +- clocks: phandle to VPU aclk, hclk clocks
> +- clock-names: should be "aclk" and "hclk"
> +- power-domains: phandle to power domain node
> +- iommus: phandle to a iommu node
> +
> +Example:
> +SoC-specific DT entry:
> + vpu: video-codec@ff9a {
> + compatible = "rockchip,rk3288-vpu";
> + reg = <0x0 0xff9a 0x0 0x800>;
> + interrupts = ,
> + ;
> + interrupt-names = "vepu", "vdpu";
> + clocks = <&cru ACLK_VCODEC>, <&cru HCLK_VCODEC>;
> + clock-names = "aclk", "hclk";
> + power-domains = <&power RK3288_PD_VIDEO>;
> + iommus = <&vpu_mmu>;
> + };
> +
>
Please remove this empty last line to avoid this warning:
.git/rebase-apply/patch:41: new blank line at EOF.
Thanks!
Hans
Security Warning
Hello! I'm a member of an international hacker group. As you could probably have guessed, your account linux-media@vger.kernel.org was hacked, because I sent message you from it. Now I have access to you accounts! For example, your password for linux-media@vger.kernel.org is 123456 Within a period from July 7, 2018 to September 23, 2018, you were infected by the virus we've created, through an adult website you've visited. So far, we have access to your messages, social media accounts, and messengers. Moreover, we've gotten full damps of these data. We are aware of your little and big secrets...yeah, you do have them. We saw and recorded your doings on porn websites. Your tastes are so weird, you know.. But the key thing is that sometimes we recorded you with your webcam, syncing the recordings with what you watched! I think you are not interested show this video to your friends, relatives, and your intimate one... Transfer $700 to our Bitcoin wallet: 1Lughwk11SAsz54wZJ3bpGbNqGfVanMWzk If you don't know about Bitcoin please input in Google "buy BTC". It's really easy. I guarantee that after that, we'll erase all your "data" :D A timer will start once you read this message. You have 48 hours to pay the above-mentioned amount. Your data will be erased once the money are transferred. If they are not, all your messages and videos recorded will be automatically sent to all your contacts found on your devices at the moment of infection. You should always think about your security. We hope this case will teach you to keep secrets. Take care of yourself.
[RFCv3 PATCH 1/3] uapi/linux/media.h: add property support
From: Hans Verkuil
Add a new topology struct that includes properties.
Signed-off-by: Hans Verkuil
---
include/uapi/linux/media.h | 71 --
1 file changed, 68 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 36f76e777ef9..755e446f699e 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -144,6 +144,8 @@ struct media_device_info {
/* Entity flags */
#define MEDIA_ENT_FL_DEFAULT (1 << 0)
#define MEDIA_ENT_FL_CONNECTOR (1 << 1)
+#define MEDIA_ENT_FL_PAD_IDX (1 << 2)
+#define MEDIA_ENT_FL_PROP_IDX (1 << 3)
/* OR with the entity id value to find the next entity */
#define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
@@ -210,6 +212,8 @@ struct media_entity_desc {
#define MEDIA_PAD_FL_SINK (1 << 0)
#define MEDIA_PAD_FL_SOURCE(1 << 1)
#define MEDIA_PAD_FL_MUST_CONNECT (1 << 2)
+#define MEDIA_PAD_FL_LINK_IDX (1 << 3)
+#define MEDIA_PAD_FL_PROP_IDX (1 << 4)
struct media_pad_desc {
__u32 entity; /* entity ID */
@@ -296,7 +300,9 @@ struct media_v2_entity {
char name[64];
__u32 function; /* Main function of the entity */
__u32 flags;
- __u32 reserved[5];
+ __u16 pad_idx;
+ __u16 prop_idx;
+ __u32 reserved[4];
} __attribute__ ((packed));
/* Should match the specific fields at media_intf_devnode */
@@ -305,11 +311,14 @@ struct media_v2_intf_devnode {
__u32 minor;
} __attribute__ ((packed));
+#define MEDIA_INTF_FL_LINK_IDX (1 << 0)
+
struct media_v2_interface {
__u32 id;
__u32 intf_type;
__u32 flags;
- __u32 reserved[9];
+ __u16 link_idx;
+ __u16 reserved[17];
union {
struct media_v2_intf_devnode devnode;
@@ -331,7 +340,9 @@ struct media_v2_pad {
__u32 entity_id;
__u32 flags;
__u32 index;
- __u32 reserved[4];
+ __u16 link_idx;
+ __u16 prop_idx;
+ __u32 reserved[3];
} __attribute__ ((packed));
struct media_v2_link {
@@ -342,6 +353,54 @@ struct media_v2_link {
__u32 reserved[6];
} __attribute__ ((packed));
+#define MEDIA_PROP_TYPE_GROUP 1
+#define MEDIA_PROP_TYPE_U642
+#define MEDIA_PROP_TYPE_S643
+#define MEDIA_PROP_TYPE_STRING 4
+
+#define MEDIA_PROP_FL_PROP_IDX (1 << 0)
+
+/**
+ * struct media_v2_prop - A media property
+ *
+ * @id:The unique non-zero ID of this property
+ * @owner_id: The ID of the object this property belongs to
+ * @type: Property type
+ * @flags: Property flags
+ * @name: Property name
+ * @payload_size: Property payload size, 0 for U64/S64
+ * @payload_offset: Property payload starts at this offset from &prop.id.
+ * This is 0 for U64/S64.
+ * @prop_idx: Index to sub-properties, 0 means there are no sub-properties.
+ * @reserved: Property reserved field, will be zeroed.
+ */
+struct media_v2_prop {
+ __u32 id;
+ __u32 owner_id;
+ __u32 type;
+ __u32 flags;
+ char name[32];
+ __u32 payload_size;
+ __u32 payload_offset;
+ __u16 prop_idx;
+ __u16 reserved[35];
+} __attribute__ ((packed));
+
+static inline const char *media_prop2s(const struct media_v2_prop *prop)
+{
+ return (const char *)prop + prop->payload_offset;
+}
+
+static inline __u64 media_prop2u64(const struct media_v2_prop *prop)
+{
+ return *(const __u64 *)((const char *)prop + prop->payload_offset);
+}
+
+static inline __s64 media_prop2s64(const struct media_v2_prop *prop)
+{
+ return *(const __s64 *)((const char *)prop + prop->payload_offset);
+}
+
struct media_v2_topology {
__u64 topology_version;
@@ -360,6 +419,10 @@ struct media_v2_topology {
__u32 num_links;
__u32 reserved4;
__u64 ptr_links;
+
+ __u32 num_props;
+ __u32 props_payload_size;
+ __u64 ptr_props;
} __attribute__ ((packed));
/* ioctls */
@@ -368,6 +431,8 @@ struct media_v2_topology {
#define MEDIA_IOC_ENUM_ENTITIES_IOWR('|', 0x01, struct
media_entity_desc)
#define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
#define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
+/* Old MEDIA_IOC_G_TOPOLOGY ioctl without props support */
+#define MEDIA_IOC_G_TOPOLOGY_OLD 0xc0487c04
#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
#ifndef __KERNEL__
--
2.19.0
[RFCv3 PATCH 2/3] media controller: add properties support
From: Hans Verkuil
Add support for properties. In this initial implementation properties
can be added to entities and pads. In addition, properties can be
nested.
Signed-off-by: Hans Verkuil
---
drivers/media/media-device.c | 324 +++
drivers/media/media-entity.c | 107 +++-
include/media/media-device.h | 6 +
include/media/media-entity.h | 315 ++
4 files changed, 672 insertions(+), 80 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 4c7190db420e..0eb0f0b4b72e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -234,6 +234,21 @@ static long media_device_setup_link(struct media_device
*mdev, void *arg)
return __media_entity_setup_link(link, linkd->flags);
}
+static void walk_props(struct list_head *head, u32 *prop_idx, u32
*payload_size)
+{
+ struct media_prop *prop;
+
+ if (list_empty(head))
+ return;
+
+ list_for_each_entry(prop, head, list) {
+ prop->graph_obj.topology_idx = (*prop_idx)++;
+ *payload_size += prop->payload_size;
+ }
+ list_for_each_entry(prop, head, list)
+ walk_props(&prop->props, prop_idx, payload_size);
+}
+
static long media_device_get_topology(struct media_device *mdev, void *arg)
{
struct media_v2_topology *topo = arg;
@@ -241,27 +256,95 @@ static long media_device_get_topology(struct media_device
*mdev, void *arg)
struct media_interface *intf;
struct media_pad *pad;
struct media_link *link;
+ struct media_prop *prop, *subprop;
struct media_v2_entity kentity, __user *uentity;
struct media_v2_interface kintf, __user *uintf;
struct media_v2_pad kpad, __user *upad;
struct media_v2_link klink, __user *ulink;
+ struct media_v2_prop kprop, __user *uprop;
+ u32 payload_size = 0;
+ u32 payload_offset = 0;
+ u32 entity_idx = 0;
+ u32 interface_idx = 0;
+ u32 pad_idx = 0;
+ u32 link_idx = 0;
+ u32 prop_idx = 0;
unsigned int i;
int ret = 0;
topo->topology_version = mdev->topology_version;
+ /* Set entity/pad/link indices and number of entities */
+ media_device_for_each_entity(entity, mdev) {
+ entity->graph_obj.topology_idx = entity_idx++;
+ walk_props(&entity->props, &prop_idx, &payload_size);
+ for (i = 0; i < entity->num_pads; i++) {
+ pad = entity->pads + i;
+ pad->graph_obj.topology_idx = pad_idx++;
+ walk_props(&pad->props, &prop_idx, &payload_size);
+ }
+ /* Note: links are ordered by source pad index */
+ list_for_each_entry(link, &entity->links, list)
+ link->graph_obj.topology_idx = link_idx++;
+ }
+ if (topo->ptr_entities && entity_idx > topo->num_entities)
+ ret = -ENOSPC;
+ topo->num_entities = entity_idx;
+ topo->reserved1 = 0;
+
+ /* Set interface/link indices and number of interfaces */
+ media_device_for_each_intf(intf, mdev) {
+ intf->graph_obj.topology_idx = interface_idx++;
+ list_for_each_entry(link, &intf->links, list)
+ link->graph_obj.topology_idx = link_idx++;
+ }
+
+ if (topo->ptr_interfaces && interface_idx > topo->num_interfaces)
+ ret = -ENOSPC;
+ topo->num_interfaces = interface_idx;
+ topo->reserved2 = 0;
+
+ /* Set number of pads */
+ if (topo->ptr_pads && pad_idx > topo->num_pads)
+ ret = -ENOSPC;
+ topo->num_pads = pad_idx;
+ topo->reserved3 = 0;
+
+ /* Set number of links */
+ if (topo->ptr_links && link_idx > topo->num_links)
+ ret = -ENOSPC;
+ topo->num_links = link_idx;
+ topo->reserved4 = 0;
+
+ /* Set number of properties */
+ if (topo->ptr_props &&
+ (prop_idx > topo->num_props ||
+payload_size > topo->props_payload_size))
+ ret = -ENOSPC;
+ topo->num_props = prop_idx;
+ topo->props_payload_size = payload_size;
+
+ if (ret)
+ return ret;
+
+ /*
+* We use u16 for the graph object indices,
+* so check that it will fit in 16 bits.
+*/
+ if (WARN_ON(entity_idx >= 0x1 ||
+ interface_idx >= 0x1 ||
+ pad_idx >= 0x1 ||
+ link_idx >= 0x1 ||
+ prop_idx >= 0x1))
+ return -EINVAL;
+
/* Get entities and number of entities */
- i = 0;
uentity = media_get_uptr(topo->ptr_entities);
- media_device_for_each_entity(entity, mdev) {
- i++;
- if (ret || !uentity)
- continue;
+ if (!uentity)
[RFCv3 PATCH 0/3] Media Controller Properties
From: Hans Verkuil This RFC patch series implements properties for the media controller. The main changes since RFCv2 are: - Properties can be nested. - G_TOPOLOGY sets flags to indicate if there are pads/links/properties for the objects. And it adds index fields to provide a quick lookup. Effectively the topology arrays now represent a flattened tree. An updated v4l2-ctl and v4l2-compliance that can report properties is available here: https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props Currently I support u64, s64 and const char * property types. And also a 'group' type that groups sub-properties. But it can be extended to any type including binary data if needed. No array support (as we have for controls), but there are enough reserved fields in media_v2_prop to add this if needed. I added properties for entities and pads to vimc, so I could test this. Note that the changes to media_device_get_topology() are hard to read from the patch. It is easier to just look at the source code: https://git.linuxtv.org/hverkuil/media_tree.git/tree/drivers/media/media-device.c?h=mc-props I have some ideas to improve this some more: 1) Add the properties directly to media_gobj. This would simplify some of the code, but it would require a media_gobj_init function to initialize the property list. In general I am a bit unhappy about media_gobj_create: it doesn't really create the graph object, instead it just adds it to the media_device. It's confusing and it is something I would like to change. 2) The links between pads are stored in media_entity instead of in media_pad. This is a bit unexpected and makes it harder to traverse the data structures since to find the links for a pad you need to walk the entity links and find the links for that pad. Putting all links in the entity also mixes up pad and interface links, and it would be much cleaner if those are separated. 3) While it is easy to find the pads and links for an entity through the new pad and link index fields, the reverse is not true: i.e. media_v2_pad refers to the entity by entity ID, and that would require walking through all entities to find which one it is. I propose adding an entity_idx field as well (and similar to media_v2_links and media_v2_prop) to make it easy to look up the parent object. It's trivial to add in the kernel and makes life much easier for userspace. 4) I still think adding support for backlinks to G_TOPOLOGY is a good idea. Since the current data structure represents a flattened tree that is easy to navigate the only thing missing for userspace is backlink support. This is still something that userspace needs to figure out when the kernel has this readily available. I think that with this in place applications can just do all the lookups directly on the topology data structure. Regards, Hans Hans Verkuil (3): uapi/linux/media.h: add property support media controller: add properties support vimc: add property test code drivers/media/media-device.c | 324 +- drivers/media/media-entity.c | 107 ++- drivers/media/platform/vimc/vimc-common.c | 50 include/media/media-device.h | 6 + include/media/media-entity.h | 315 + include/uapi/linux/media.h| 71 - 6 files changed, 790 insertions(+), 83 deletions(-) -- 2.19.0
[RFCv3 PATCH 3/3] vimc: add property test code
From: Hans Verkuil
Add properties to entities and pads to be able to test the
properties API.
Signed-off-by: Hans Verkuil
---
drivers/media/platform/vimc/vimc-common.c | 50 +++
1 file changed, 50 insertions(+)
diff --git a/drivers/media/platform/vimc/vimc-common.c
b/drivers/media/platform/vimc/vimc-common.c
index dee1b9dfc4f6..2f70e4e64790 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -415,6 +415,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
const unsigned long *pads_flag,
const struct v4l2_subdev_ops *sd_ops)
{
+ struct media_prop *prop = NULL;
int ret;
/* Allocate the pads */
@@ -452,6 +453,55 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
goto err_clean_m_ent;
}
+ ret = media_entity_add_prop_u64(&sd->entity, "u64", ~1);
+ if (!ret)
+ ret = media_entity_add_prop_s64(&sd->entity, "s64", -5);
+ if (!ret)
+ ret = media_entity_add_prop_string(&sd->entity, "string",
+ sd->name);
+ if (!ret) {
+ prop = media_entity_add_prop_group(&sd->entity, "empty-group");
+ ret = PTR_ERR_OR_ZERO(prop);
+ }
+ if (!ret) {
+ prop = media_entity_add_prop_group(&sd->entity, "group");
+ ret = PTR_ERR_OR_ZERO(prop);
+ }
+ if (!ret)
+ ret = media_prop_add_prop_u64(prop, "u64", 42);
+ if (!ret)
+ ret = media_prop_add_prop_s64(prop, "s64", -42);
+ if (!ret)
+ ret = media_prop_add_prop_string(prop, "string", "42");
+ if (!ret)
+ ret = media_pad_add_prop_u64(&sd->entity.pads[num_pads - 1],
+"u64", ~1);
+ if (!ret)
+ ret = media_pad_add_prop_s64(&sd->entity.pads[num_pads - 1],
+"s64", -5);
+ if (!ret) {
+ prop = media_pad_add_prop_group(&sd->entity.pads[num_pads - 1],
+ "group");
+ ret = PTR_ERR_OR_ZERO(prop);
+ }
+ if (!ret)
+ ret = media_prop_add_prop_u64(prop, "u64", 24);
+ if (!ret)
+ ret = media_prop_add_prop_s64(prop, "s64", -24);
+ if (!ret)
+ ret = media_pad_add_prop_string(&sd->entity.pads[0],
+ "string", sd->name);
+ if (!ret)
+ ret = media_prop_add_prop_string(prop, "string", "24");
+ if (!ret) {
+ prop = media_prop_add_prop_group(prop, "subgroup");
+ ret = PTR_ERR_OR_ZERO(prop);
+ }
+ if (!ret)
+ ret = media_prop_add_prop_string(prop, "string", "substring");
+ if (ret)
+ goto err_clean_m_ent;
+
return 0;
err_clean_m_ent:
--
2.19.0
[PATCH] [media] v4l: xilinx: fix typo in formats table
In formats table the entry for CFA pattern "rggb" has GRBG fourcc.
This patch fixes it.
Cc: linux-media@vger.kernel.org
Signed-off-by: Mirco Di Salvo
Signed-off-by: Andrea Merello
---
drivers/media/platform/xilinx/xilinx-vip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/xilinx/xilinx-vip.c
b/drivers/media/platform/xilinx/xilinx-vip.c
index 311259129504..e9567cdfb89b 100644
--- a/drivers/media/platform/xilinx/xilinx-vip.c
+++ b/drivers/media/platform/xilinx/xilinx-vip.c
@@ -36,7 +36,7 @@ static const struct xvip_video_format xvip_video_formats[] = {
{ XVIP_VF_MONO_SENSOR, 8, "mono", MEDIA_BUS_FMT_Y8_1X8,
1, V4L2_PIX_FMT_GREY, "Greyscale 8-bit" },
{ XVIP_VF_MONO_SENSOR, 8, "rggb", MEDIA_BUS_FMT_SRGGB8_1X8,
- 1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit RGGB" },
+ 1, V4L2_PIX_FMT_SRGGB8, "Bayer 8-bit RGGB" },
{ XVIP_VF_MONO_SENSOR, 8, "grbg", MEDIA_BUS_FMT_SGRBG8_1X8,
1, V4L2_PIX_FMT_SGRBG8, "Bayer 8-bit GRBG" },
{ XVIP_VF_MONO_SENSOR, 8, "gbrg", MEDIA_BUS_FMT_SGBRG8_1X8,
--
2.17.1
Re: [PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property
Hi Hugues, On Thu, Sep 27, 2018 at 04:46:06PM +0200, Hugues Fruchet wrote: > This optional property aims to inform parallel video devices > of the maximum pixel clock frequency admissible by host video > interface. If bandwidth of data to be transferred requires a > pixel clock which is higher than this value, parallel video > device could then typically adapt framerate to reach > this constraint. > > Signed-off-by: Hugues Fruchet > --- > Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index baf9d97..fa4c112 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -147,6 +147,8 @@ Optional endpoint properties >as 0 (normal). This property is valid for serial busses only. > - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used >with CCP2, for instance. > +- pclk-max-frequency: maximum pixel clock frequency admissible by video > + host interface. Is there a limit on the pixel clock or the link frequency? We do have a property for the link frequency and a control for the pixel lock as well as for the link frequency. Could these be used for the purpose? The link frequency in general should be specified for the board, and that limits the pixel clock as well in the case the bus transfers a given number of pixels per clock. The OMAP3ISP driver also address this by reading back the pixel clock from the sensor before starting streaming. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
