Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Fri, Nov 17, 2023 at 8:07 PM Hans Verkuil wrote:
>
> Here is an RFC patch adding support for 'fraction_bits'. It's lacking
> documentation, but it can be used for testing.
>
> It was rather a pain logging fixed point number in a reasonable format,
> but I think it is OK.
>
> In userspace (where you can use floating point) it is a lot easier:
>
> printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));
>
> I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl.
> I could add it to struct v4l2_queryctrl, but I did not think that was
> necessary. Other opinions are welcome.
>
> In the meantime, let me know if this works for your patch series. If it
> does, then I can clean this up.
Thanks. It works for me. What I have done are:
1. drop FIXED_POINT
2. use v4l2_ctrl_new_custom
Best regards
Wang shengjiu
>
> Regards,
>
> Hans
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 +
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++
> include/media/v4l2-ctrls.h| 7 ++-
> include/uapi/linux/videodev2.h| 20 ++-
> 4 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> index 002ea6588edf..3132df315b17 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
> struct v4l2_query_ext_ctr
> qc->elems = ctrl->elems;
> qc->nr_of_dims = ctrl->nr_of_dims;
> memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));
> + qc->fraction_bits = ctrl->fraction_bits;
> qc->minimum = ctrl->minimum;
> qc->maximum = ctrl->maximum;
> qc->default_value = ctrl->default_value;
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index a662fb60f73f..0e08a371af5c 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl
> *ctrl, u32 from_idx,
> }
> EXPORT_SYMBOL(v4l2_ctrl_type_op_init);
>
> +static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits)
> +{
> + s64 i = v4l2_fp_integer(v, fraction_bits);
> + s64 f = v4l2_fp_fraction(v, fraction_bits);
> +
> + if (!f) {
> + pr_cont("%lld", i);
> + } else if (fraction_bits < 20) {
> + u64 div = 1ULL << fraction_bits;
> +
> + if (!i && f < 0)
> + pr_cont("-%lld/%llu", -f, div);
> + else if (!i)
> + pr_cont("%lld/%llu", f, div);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/%llu", -i, -f, div);
> + else
> + pr_cont("%lld+%llu/%llu", i, f, div);
> + } else {
> + if (!i && f < 0)
> + pr_cont("-%lld/(2^%u)", -f, fraction_bits);
> + else if (!i)
> + pr_cont("%lld/(2^%u)", f, fraction_bits);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
> + else
> + pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
> + }
> +}
> +
> void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> {
> union v4l2_ctrl_ptr ptr = ctrl->p_cur;
>
> if (ctrl->is_array) {
> - unsigned i;
> + unsigned int i;
>
> for (i = 0; i < ctrl->nr_of_dims; i++)
> pr_cont("[%u]", ctrl->dims[i]);
> @@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
>
> switch (ctrl->type) {
> case V4L2_CTRL_TYPE_INTEGER:
> - pr_cont("%d", *ptr.p_s32);
> + if (!ctrl->fraction_bits)
> + pr_cont("%d", *ptr.p_s32);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_BOOLEAN:
> pr_cont("%s", *ptr.p_s32 ? "true" : "false");
> @@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> pr_cont("0x%08x", *ptr.p_s32);
> break;
> case V4L2_CTRL_TYPE_INTEGER64:
> - pr_cont("%lld", *ptr.p_s64);
> + if (!ctrl->fraction_bits)
> + pr_cont("%lld", *ptr.p_s64);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_STRING:
> pr_cont("%s", ptr.p_char);
> break;
> case V4L2_CTRL_TYPE_U8:
> - pr_cont("%u", (unsigned)*ptr.p_u8);
> +
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 17/11/2023 14:07, Sakari Ailus wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Fri, Nov 17, 2023 at 01:07:44PM +0100, Hans Verkuil wrote:
>> Here is an RFC patch adding support for 'fraction_bits'. It's lacking
>> documentation, but it can be used for testing.
>>
>> It was rather a pain logging fixed point number in a reasonable format,
>> but I think it is OK.
>>
>> In userspace (where you can use floating point) it is a lot easier:
>>
>> printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));
>
> I wonder if we could add a printk() format specifier for this. Doesn't need
> to be done right now though, just an idea.
>
>>
>> I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl.
>> I could add it to struct v4l2_queryctrl, but I did not think that was
>> necessary. Other opinions are welcome.
>>
>> In the meantime, let me know if this works for your patch series. If it
>> does, then I can clean this up.
>>
>> Regards,
>>
>> Hans
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 +
>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++
>> include/media/v4l2-ctrls.h| 7 ++-
>> include/uapi/linux/videodev2.h| 20 ++-
>> 4 files changed, 85 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c
>> b/drivers/media/v4l2-core/v4l2-ctrls-api.c
>> index 002ea6588edf..3132df315b17 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
>> @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
>> struct v4l2_query_ext_ctr
>> qc->elems = ctrl->elems;
>> qc->nr_of_dims = ctrl->nr_of_dims;
>> memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));
>> +qc->fraction_bits = ctrl->fraction_bits;
>> qc->minimum = ctrl->minimum;
>> qc->maximum = ctrl->maximum;
>> qc->default_value = ctrl->default_value;
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c
>> b/drivers/media/v4l2-core/v4l2-ctrls-core.c
>> index a662fb60f73f..0e08a371af5c 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
>> @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl
>> *ctrl, u32 from_idx,
>> }
>> EXPORT_SYMBOL(v4l2_ctrl_type_op_init);
>>
>> +static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits)
>> +{
>> +s64 i = v4l2_fp_integer(v, fraction_bits);
>> +s64 f = v4l2_fp_fraction(v, fraction_bits);
>> +
>> +if (!f) {
>> +pr_cont("%lld", i);
>> +} else if (fraction_bits < 20) {
>> +u64 div = 1ULL << fraction_bits;
>> +
>> +if (!i && f < 0)
>> +pr_cont("-%lld/%llu", -f, div);
>> +else if (!i)
>> +pr_cont("%lld/%llu", f, div);
>> +else if (i < 0 || f < 0)
>> +pr_cont("-%lld-%llu/%llu", -i, -f, div);
>> +else
>> +pr_cont("%lld+%llu/%llu", i, f, div);
>> +} else {
>> +if (!i && f < 0)
>> +pr_cont("-%lld/(2^%u)", -f, fraction_bits);
>> +else if (!i)
>> +pr_cont("%lld/(2^%u)", f, fraction_bits);
>> +else if (i < 0 || f < 0)
>> +pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
>> +else
>> +pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
>> +}
>> +}
>> +
>> void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
>> {
>> union v4l2_ctrl_ptr ptr = ctrl->p_cur;
>>
>> if (ctrl->is_array) {
>> -unsigned i;
>> +unsigned int i;
>>
>> for (i = 0; i < ctrl->nr_of_dims; i++)
>> pr_cont("[%u]", ctrl->dims[i]);
>> @@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
>>
>> switch (ctrl->type) {
>> case V4L2_CTRL_TYPE_INTEGER:
>> -pr_cont("%d", *ptr.p_s32);
>> +if (!ctrl->fraction_bits)
>> +pr_cont("%d", *ptr.p_s32);
>> +else
>> +v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
>> break;
>> case V4L2_CTRL_TYPE_BOOLEAN:
>> pr_cont("%s", *ptr.p_s32 ? "true" : "false");
>> @@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl
>> *ctrl)
>> pr_cont("0x%08x", *ptr.p_s32);
>> break;
>> case V4L2_CTRL_TYPE_INTEGER64:
>> -pr_cont("%lld", *ptr.p_s64);
>> +if (!ctrl->fraction_bits)
>> +pr_cont("%lld", *ptr.p_s64);
>> +else
>> +v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
>> break;
>> case V4L2_CTRL_TYPE_STRING:
>> pr_cont("%s", ptr.p_char);
>> break;
>> case V4L2_CTRL_TYPE_U8:
>> -
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans,
Thank you for the patch.
On Fri, Nov 17, 2023 at 01:07:44PM +0100, Hans Verkuil wrote:
> Here is an RFC patch adding support for 'fraction_bits'. It's lacking
> documentation, but it can be used for testing.
>
> It was rather a pain logging fixed point number in a reasonable format,
> but I think it is OK.
>
> In userspace (where you can use floating point) it is a lot easier:
>
> printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));
I wonder if we could add a printk() format specifier for this. Doesn't need
to be done right now though, just an idea.
>
> I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl.
> I could add it to struct v4l2_queryctrl, but I did not think that was
> necessary. Other opinions are welcome.
>
> In the meantime, let me know if this works for your patch series. If it
> does, then I can clean this up.
>
> Regards,
>
> Hans
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 +
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++
> include/media/v4l2-ctrls.h| 7 ++-
> include/uapi/linux/videodev2.h| 20 ++-
> 4 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> index 002ea6588edf..3132df315b17 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
> struct v4l2_query_ext_ctr
> qc->elems = ctrl->elems;
> qc->nr_of_dims = ctrl->nr_of_dims;
> memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));
> + qc->fraction_bits = ctrl->fraction_bits;
> qc->minimum = ctrl->minimum;
> qc->maximum = ctrl->maximum;
> qc->default_value = ctrl->default_value;
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index a662fb60f73f..0e08a371af5c 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl
> *ctrl, u32 from_idx,
> }
> EXPORT_SYMBOL(v4l2_ctrl_type_op_init);
>
> +static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits)
> +{
> + s64 i = v4l2_fp_integer(v, fraction_bits);
> + s64 f = v4l2_fp_fraction(v, fraction_bits);
> +
> + if (!f) {
> + pr_cont("%lld", i);
> + } else if (fraction_bits < 20) {
> + u64 div = 1ULL << fraction_bits;
> +
> + if (!i && f < 0)
> + pr_cont("-%lld/%llu", -f, div);
> + else if (!i)
> + pr_cont("%lld/%llu", f, div);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/%llu", -i, -f, div);
> + else
> + pr_cont("%lld+%llu/%llu", i, f, div);
> + } else {
> + if (!i && f < 0)
> + pr_cont("-%lld/(2^%u)", -f, fraction_bits);
> + else if (!i)
> + pr_cont("%lld/(2^%u)", f, fraction_bits);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
> + else
> + pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
> + }
> +}
> +
> void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> {
> union v4l2_ctrl_ptr ptr = ctrl->p_cur;
>
> if (ctrl->is_array) {
> - unsigned i;
> + unsigned int i;
>
> for (i = 0; i < ctrl->nr_of_dims; i++)
> pr_cont("[%u]", ctrl->dims[i]);
> @@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
>
> switch (ctrl->type) {
> case V4L2_CTRL_TYPE_INTEGER:
> - pr_cont("%d", *ptr.p_s32);
> + if (!ctrl->fraction_bits)
> + pr_cont("%d", *ptr.p_s32);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_BOOLEAN:
> pr_cont("%s", *ptr.p_s32 ? "true" : "false");
> @@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> pr_cont("0x%08x", *ptr.p_s32);
> break;
> case V4L2_CTRL_TYPE_INTEGER64:
> - pr_cont("%lld", *ptr.p_s64);
> + if (!ctrl->fraction_bits)
> + pr_cont("%lld", *ptr.p_s64);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_STRING:
> pr_cont("%s", ptr.p_char);
> break;
> case V4L2_CTRL_TYPE_U8:
> - pr_cont("%u", (unsigned)*ptr.p_u8);
> + if (!ctrl->fraction_bits)
> +
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans,
Thank you for the patch.
On Fri, Nov 17, 2023 at 01:07:44PM +0100, Hans Verkuil wrote:
> Here is an RFC patch adding support for 'fraction_bits'. It's lacking
> documentation, but it can be used for testing.
>
> It was rather a pain logging fixed point number in a reasonable format,
> but I think it is OK.
>
> In userspace (where you can use floating point) it is a lot easier:
>
> printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));
>
> I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl.
> I could add it to struct v4l2_queryctrl, but I did not think that was
> necessary. Other opinions are welcome.
Agreed.
> In the meantime, let me know if this works for your patch series. If it
> does, then I can clean this up.
>
> Regards,
>
> Hans
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 +
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++
> include/media/v4l2-ctrls.h| 7 ++-
> include/uapi/linux/videodev2.h| 20 ++-
> 4 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> index 002ea6588edf..3132df315b17 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
> @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
> struct v4l2_query_ext_ctr
> qc->elems = ctrl->elems;
> qc->nr_of_dims = ctrl->nr_of_dims;
> memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));
> + qc->fraction_bits = ctrl->fraction_bits;
> qc->minimum = ctrl->minimum;
> qc->maximum = ctrl->maximum;
> qc->default_value = ctrl->default_value;
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index a662fb60f73f..0e08a371af5c 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl
> *ctrl, u32 from_idx,
> }
> EXPORT_SYMBOL(v4l2_ctrl_type_op_init);
>
> +static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits)
> +{
> + s64 i = v4l2_fp_integer(v, fraction_bits);
> + s64 f = v4l2_fp_fraction(v, fraction_bits);
> +
> + if (!f) {
> + pr_cont("%lld", i);
> + } else if (fraction_bits < 20) {
> + u64 div = 1ULL << fraction_bits;
> +
> + if (!i && f < 0)
> + pr_cont("-%lld/%llu", -f, div);
> + else if (!i)
> + pr_cont("%lld/%llu", f, div);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/%llu", -i, -f, div);
> + else
> + pr_cont("%lld+%llu/%llu", i, f, div);
> + } else {
> + if (!i && f < 0)
> + pr_cont("-%lld/(2^%u)", -f, fraction_bits);
> + else if (!i)
> + pr_cont("%lld/(2^%u)", f, fraction_bits);
> + else if (i < 0 || f < 0)
> + pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
> + else
> + pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
> + }
> +}
> +
> void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> {
> union v4l2_ctrl_ptr ptr = ctrl->p_cur;
>
> if (ctrl->is_array) {
> - unsigned i;
> + unsigned int i;
>
> for (i = 0; i < ctrl->nr_of_dims; i++)
> pr_cont("[%u]", ctrl->dims[i]);
> @@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
>
> switch (ctrl->type) {
> case V4L2_CTRL_TYPE_INTEGER:
> - pr_cont("%d", *ptr.p_s32);
> + if (!ctrl->fraction_bits)
> + pr_cont("%d", *ptr.p_s32);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_BOOLEAN:
> pr_cont("%s", *ptr.p_s32 ? "true" : "false");
> @@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
> pr_cont("0x%08x", *ptr.p_s32);
> break;
> case V4L2_CTRL_TYPE_INTEGER64:
> - pr_cont("%lld", *ptr.p_s64);
> + if (!ctrl->fraction_bits)
> + pr_cont("%lld", *ptr.p_s64);
> + else
> + v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
> break;
> case V4L2_CTRL_TYPE_STRING:
> pr_cont("%s", ptr.p_char);
> break;
> case V4L2_CTRL_TYPE_U8:
> - pr_cont("%u", (unsigned)*ptr.p_u8);
> + if (!ctrl->fraction_bits)
> + pr_cont("%u", (unsigned int)*ptr.p_u8);
> + else
> + v4l2_ctrl_log_fp((unsigned
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Tomasz, On Thu, Nov 16, 2023 at 04:31:41PM +0900, Tomasz Figa wrote: > On Wed, Nov 15, 2023 at 8:49 PM Laurent Pinchart wrote: > > On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > > > On 11/15/23 11:55, Laurent Pinchart wrote: > > > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > > > >> On 13/11/2023 13:44, Laurent Pinchart wrote: > > > >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > > > On 13/11/2023 12:43, Laurent Pinchart wrote: > > > > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > > > >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > > >>> On 13/11/2023 12:07, Laurent Pinchart wrote: > > > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > > > On 13/11/2023 11:42, Laurent Pinchart wrote: > > > >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > > >>> On 10/11/2023 06:48, Shengjiu Wang wrote: > > > Fixed point controls are used by the user to configure > > > a fixed point value in 64bits, which Q31.32 format. > > > > > > Signed-off-by: Shengjiu Wang > > > >>> > > > >>> This patch adds a new control type. This is something that > > > >>> also needs to be > > > >>> tested by v4l2-compliance, and for that we need to add > > > >>> support for this to > > > >>> one of the media test-drivers. The best place for that is the > > > >>> vivid driver, > > > >>> since that has already a bunch of test controls for other > > > >>> control types. > > > >>> > > > >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > > >>> > > > >>> Can you add a patch adding a fixed point test control to > > > >>> vivid? > > > >> > > > >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > > > >> seems to > > > >> relate more to units than control types. We have lots of > > > >> fixed-point > > > >> values in controls already, using the 32-bit and 64-bit > > > >> integer control > > > >> types. They use various locations for the decimal point, > > > >> depending on > > > >> the control. If we want to make this more explicit to users, > > > >> we should > > > >> work on adding unit support to the V4L2 controls. > > > > > > > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are > > > > units. > > > > > > It's not a unit, but I think it's related to units. My point is > > > that, > > > without units support, I don't see why we need a formal > > > definition of > > > fixed-point types, and why this series couldn't just use > > > VIVID_CID_INTEGER64. Drivers already interpret > > > VIVID_CID_INTEGER64 > > > values as they see fit. > > > >>> > > > >>> They do? That's new to me. A quick grep for > > > >>> V4L2_CTRL_TYPE_INTEGER64 > > > >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows > > > >>> that it > > > > > > > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > > > > > > >>> is always interpreted as a 64 bit integer and nothing else. As it > > > >>> should. > > > > > > > > The most common case for control handling in drivers is taking the > > > > integer value and converting it to a register value, using > > > > device-specific encoding of the register value. It can be a > > > > fixed-point > > > > format or something else, depending on the device. My point is that > > > > drivers routinely convert a "plain" integer to something else, and > > > > that > > > > has never been considered as a cause of concern. I don't see why it > > > > would be different in this series. > > > > > > > >>> And while we do not have support for units (other than the > > > >>> documentation), > > > >>> we do have type support in the form of V4L2_CTRL_TYPE_*. > > > >>> > > > > A quick "git grep -i "fixed point" > > > > Documentation/userspace-api/media/' > > > > only shows a single driver specific control (dw100.rst). > > > > > > > > I'm not aware of other controls in mainline that use fixed > > > > point. > > > > > > The analog gain control for sensors for instance. > > > >>> > > > >>> Not really. The documentation is super vague: > > > >>> > > > >>> V4L2_CID_ANALOGUE_GAIN (integer) > > > >>> > > > >>> Analogue gain is gain affecting all colour components in > > > >>> the pixel matrix. The > > > >>> gain operation is performed in the analogue domain before > > > >>> A/D conversion. > > > >>> > > > >>> And the integer is just
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Here is an RFC patch adding support for 'fraction_bits'. It's lacking
documentation, but it can be used for testing.
It was rather a pain logging fixed point number in a reasonable format,
but I think it is OK.
In userspace (where you can use floating point) it is a lot easier:
printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));
I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl.
I could add it to struct v4l2_queryctrl, but I did not think that was
necessary. Other opinions are welcome.
In the meantime, let me know if this works for your patch series. If it
does, then I can clean this up.
Regards,
Hans
Signed-off-by: Hans Verkuil
---
drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 +
drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++
include/media/v4l2-ctrls.h| 7 ++-
include/uapi/linux/videodev2.h| 20 ++-
4 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c
b/drivers/media/v4l2-core/v4l2-ctrls-api.c
index 002ea6588edf..3132df315b17 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
@@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl,
struct v4l2_query_ext_ctr
qc->elems = ctrl->elems;
qc->nr_of_dims = ctrl->nr_of_dims;
memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));
+ qc->fraction_bits = ctrl->fraction_bits;
qc->minimum = ctrl->minimum;
qc->maximum = ctrl->maximum;
qc->default_value = ctrl->default_value;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c
b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index a662fb60f73f..0e08a371af5c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl *ctrl,
u32 from_idx,
}
EXPORT_SYMBOL(v4l2_ctrl_type_op_init);
+static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits)
+{
+ s64 i = v4l2_fp_integer(v, fraction_bits);
+ s64 f = v4l2_fp_fraction(v, fraction_bits);
+
+ if (!f) {
+ pr_cont("%lld", i);
+ } else if (fraction_bits < 20) {
+ u64 div = 1ULL << fraction_bits;
+
+ if (!i && f < 0)
+ pr_cont("-%lld/%llu", -f, div);
+ else if (!i)
+ pr_cont("%lld/%llu", f, div);
+ else if (i < 0 || f < 0)
+ pr_cont("-%lld-%llu/%llu", -i, -f, div);
+ else
+ pr_cont("%lld+%llu/%llu", i, f, div);
+ } else {
+ if (!i && f < 0)
+ pr_cont("-%lld/(2^%u)", -f, fraction_bits);
+ else if (!i)
+ pr_cont("%lld/(2^%u)", f, fraction_bits);
+ else if (i < 0 || f < 0)
+ pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
+ else
+ pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
+ }
+}
+
void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
{
union v4l2_ctrl_ptr ptr = ctrl->p_cur;
if (ctrl->is_array) {
- unsigned i;
+ unsigned int i;
for (i = 0; i < ctrl->nr_of_dims; i++)
pr_cont("[%u]", ctrl->dims[i]);
@@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
- pr_cont("%d", *ptr.p_s32);
+ if (!ctrl->fraction_bits)
+ pr_cont("%d", *ptr.p_s32);
+ else
+ v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
break;
case V4L2_CTRL_TYPE_BOOLEAN:
pr_cont("%s", *ptr.p_s32 ? "true" : "false");
@@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
pr_cont("0x%08x", *ptr.p_s32);
break;
case V4L2_CTRL_TYPE_INTEGER64:
- pr_cont("%lld", *ptr.p_s64);
+ if (!ctrl->fraction_bits)
+ pr_cont("%lld", *ptr.p_s64);
+ else
+ v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
break;
case V4L2_CTRL_TYPE_STRING:
pr_cont("%s", ptr.p_char);
break;
case V4L2_CTRL_TYPE_U8:
- pr_cont("%u", (unsigned)*ptr.p_u8);
+ if (!ctrl->fraction_bits)
+ pr_cont("%u", (unsigned int)*ptr.p_u8);
+ else
+ v4l2_ctrl_log_fp((unsigned int)*ptr.p_u8,
ctrl->fraction_bits);
break;
case V4L2_CTRL_TYPE_U16:
- pr_cont("%u", (unsigned)*ptr.p_u16);
+ if (!ctrl->fraction_bits)
+ pr_cont("%u",
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Thu, Nov 16, 2023 at 4:01 PM Hans Verkuil wrote: > > Shengjiu, > > FYI: I started work on adding the fraction_bits field. I hope to have a > patch for that early next week. > Thanks. I will wait for your patch to be ready. best regards wang shengjiu > Regards, > > Hans > > On 16/11/2023 08:31, Tomasz Figa wrote: > > On Wed, Nov 15, 2023 at 8:49 PM Laurent Pinchart > > wrote: > >> > >> Hi Hans, > >> > >> On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > >>> On 11/15/23 11:55, Laurent Pinchart wrote: > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > > On 13/11/2023 13:44, Laurent Pinchart wrote: > >> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 12:43, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >> On 13/11/2023 12:07, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>> Fixed point controls are used by the user to configure > >>> a fixed point value in 64bits, which Q31.32 format. > >>> > >>> Signed-off-by: Shengjiu Wang > >> > >> This patch adds a new control type. This is something that > >> also needs to be > >> tested by v4l2-compliance, and for that we need to add support > >> for this to > >> one of the media test-drivers. The best place for that is the > >> vivid driver, > >> since that has already a bunch of test controls for other > >> control types. > >> > >> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >> > >> Can you add a patch adding a fixed point test control to vivid? > > > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > > seems to > > relate more to units than control types. We have lots of > > fixed-point > > values in controls already, using the 32-bit and 64-bit integer > > control > > types. They use various locations for the decimal point, > > depending on > > the control. If we want to make this more explicit to users, we > > should > > work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are > units. > >>> > >>> It's not a unit, but I think it's related to units. My point is > >>> that, > >>> without units support, I don't see why we need a formal > >>> definition of > >>> fixed-point types, and why this series couldn't just use > >>> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >>> values as they see fit. > >> > >> They do? That's new to me. A quick grep for > >> V4L2_CTRL_TYPE_INTEGER64 > >> (I assume you meant that rather than VIVID_CID_INTEGER64) shows > >> that it > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > >> is always interpreted as a 64 bit integer and nothing else. As it > >> should. > > The most common case for control handling in drivers is taking the > integer value and converting it to a register value, using > device-specific encoding of the register value. It can be a > fixed-point > format or something else, depending on the device. My point is that > drivers routinely convert a "plain" integer to something else, and > that > has never been considered as a cause of concern. I don't see why it > would be different in this series. > > >> And while we do not have support for units (other than the > >> documentation), > >> we do have type support in the form of V4L2_CTRL_TYPE_*. > >> > A quick "git grep -i "fixed point" > Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. > >>> > >>> The analog gain control for sensors for instance. > >> > >> Not really. The documentation is super vague: > >> > >> V4L2_CID_ANALOGUE_GAIN (integer) > >> > >> Analogue gain is gain affecting all colour components in the > >> pixel matrix. The > >>
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Shengjiu, FYI: I started work on adding the fraction_bits field. I hope to have a patch for that early next week. Regards, Hans On 16/11/2023 08:31, Tomasz Figa wrote: > On Wed, Nov 15, 2023 at 8:49 PM Laurent Pinchart > wrote: >> >> Hi Hans, >> >> On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: >>> On 11/15/23 11:55, Laurent Pinchart wrote: On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > On 13/11/2023 13:44, Laurent Pinchart wrote: >> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: >>> On 13/11/2023 12:43, Laurent Pinchart wrote: On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: >> On 13/11/2023 12:07, Laurent Pinchart wrote: >>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: On 13/11/2023 11:42, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: >> On 10/11/2023 06:48, Shengjiu Wang wrote: >>> Fixed point controls are used by the user to configure >>> a fixed point value in 64bits, which Q31.32 format. >>> >>> Signed-off-by: Shengjiu Wang >> >> This patch adds a new control type. This is something that also >> needs to be >> tested by v4l2-compliance, and for that we need to add support >> for this to >> one of the media test-drivers. The best place for that is the >> vivid driver, >> since that has already a bunch of test controls for other >> control types. >> >> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. >> >> Can you add a patch adding a fixed point test control to vivid? > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > seems to > relate more to units than control types. We have lots of > fixed-point > values in controls already, using the 32-bit and 64-bit integer > control > types. They use various locations for the decimal point, > depending on > the control. If we want to make this more explicit to users, we > should > work on adding unit support to the V4L2 controls. "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. >>> >>> It's not a unit, but I think it's related to units. My point is >>> that, >>> without units support, I don't see why we need a formal definition >>> of >>> fixed-point types, and why this series couldn't just use >>> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 >>> values as they see fit. >> >> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 >> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that >> it Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) >> is always interpreted as a 64 bit integer and nothing else. As it >> should. The most common case for control handling in drivers is taking the integer value and converting it to a register value, using device-specific encoding of the register value. It can be a fixed-point format or something else, depending on the device. My point is that drivers routinely convert a "plain" integer to something else, and that has never been considered as a cause of concern. I don't see why it would be different in this series. >> And while we do not have support for units (other than the >> documentation), >> we do have type support in the form of V4L2_CTRL_TYPE_*. >> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' only shows a single driver specific control (dw100.rst). I'm not aware of other controls in mainline that use fixed point. >>> >>> The analog gain control for sensors for instance. >> >> Not really. The documentation is super vague: >> >> V4L2_CID_ANALOGUE_GAIN (integer) >> >> Analogue gain is gain affecting all colour components in the >> pixel matrix. The >> gain operation is performed in the analogue domain before A/D >> conversion. >> >> And the integer is just a range. Internally it might map to some >> fixed >> point value, but userspace won't see that, it's hidden in the driver >> AFAICT. It's hidden so well that libcamera has a database of the sensor
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Wed, Nov 15, 2023 at 8:49 PM Laurent Pinchart wrote: > > Hi Hans, > > On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > > On 11/15/23 11:55, Laurent Pinchart wrote: > > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > > >> On 13/11/2023 13:44, Laurent Pinchart wrote: > > >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > > On 13/11/2023 12:43, Laurent Pinchart wrote: > > > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > > >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > >>> On 13/11/2023 12:07, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > > On 13/11/2023 11:42, Laurent Pinchart wrote: > > >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > >>> On 10/11/2023 06:48, Shengjiu Wang wrote: > > Fixed point controls are used by the user to configure > > a fixed point value in 64bits, which Q31.32 format. > > > > Signed-off-by: Shengjiu Wang > > >>> > > >>> This patch adds a new control type. This is something that also > > >>> needs to be > > >>> tested by v4l2-compliance, and for that we need to add support > > >>> for this to > > >>> one of the media test-drivers. The best place for that is the > > >>> vivid driver, > > >>> since that has already a bunch of test controls for other > > >>> control types. > > >>> > > >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > >>> > > >>> Can you add a patch adding a fixed point test control to vivid? > > >> > > >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > > >> seems to > > >> relate more to units than control types. We have lots of > > >> fixed-point > > >> values in controls already, using the 32-bit and 64-bit integer > > >> control > > >> types. They use various locations for the decimal point, > > >> depending on > > >> the control. If we want to make this more explicit to users, we > > >> should > > >> work on adding unit support to the V4L2 controls. > > > > > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are > > > units. > > > > It's not a unit, but I think it's related to units. My point is > > that, > > without units support, I don't see why we need a formal definition > > of > > fixed-point types, and why this series couldn't just use > > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > > values as they see fit. > > >>> > > >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > > >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows > > >>> that it > > > > > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > > > > >>> is always interpreted as a 64 bit integer and nothing else. As it > > >>> should. > > > > > > The most common case for control handling in drivers is taking the > > > integer value and converting it to a register value, using > > > device-specific encoding of the register value. It can be a > > > fixed-point > > > format or something else, depending on the device. My point is that > > > drivers routinely convert a "plain" integer to something else, and > > > that > > > has never been considered as a cause of concern. I don't see why it > > > would be different in this series. > > > > > >>> And while we do not have support for units (other than the > > >>> documentation), > > >>> we do have type support in the form of V4L2_CTRL_TYPE_*. > > >>> > > > A quick "git grep -i "fixed point" > > > Documentation/userspace-api/media/' > > > only shows a single driver specific control (dw100.rst). > > > > > > I'm not aware of other controls in mainline that use fixed point. > > > > The analog gain control for sensors for instance. > > >>> > > >>> Not really. The documentation is super vague: > > >>> > > >>> V4L2_CID_ANALOGUE_GAIN (integer) > > >>> > > >>> Analogue gain is gain affecting all colour components in the > > >>> pixel matrix. The > > >>> gain operation is performed in the analogue domain before A/D > > >>> conversion. > > >>> > > >>> And the integer is just a range. Internally it might map to some > > >>> fixed > > >>> point value, but userspace won't see that, it's hidden in the > > >>> driver AFAICT. > > > > > > It's hidden so well that libcamera has a database of the sensor it > > > supports, with formulas to map a real gain value to the >
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > On 11/15/23 11:55, Laurent Pinchart wrote: > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > >> On 13/11/2023 13:44, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > On 13/11/2023 12:43, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 12:07, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > On 13/11/2023 11:42, Laurent Pinchart wrote: > >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >>> On 10/11/2023 06:48, Shengjiu Wang wrote: > Fixed point controls are used by the user to configure > a fixed point value in 64bits, which Q31.32 format. > > Signed-off-by: Shengjiu Wang > >>> > >>> This patch adds a new control type. This is something that also > >>> needs to be > >>> tested by v4l2-compliance, and for that we need to add support > >>> for this to > >>> one of the media test-drivers. The best place for that is the > >>> vivid driver, > >>> since that has already a bunch of test controls for other control > >>> types. > >>> > >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >>> > >>> Can you add a patch adding a fixed point test control to vivid? > >> > >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > >> seems to > >> relate more to units than control types. We have lots of > >> fixed-point > >> values in controls already, using the 32-bit and 64-bit integer > >> control > >> types. They use various locations for the decimal point, depending > >> on > >> the control. If we want to make this more explicit to users, we > >> should > >> work on adding unit support to the V4L2 controls. > > > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > It's not a unit, but I think it's related to units. My point is that, > without units support, I don't see why we need a formal definition of > fixed-point types, and why this series couldn't just use > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > values as they see fit. > >>> > >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that > >>> it > > > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > > >>> is always interpreted as a 64 bit integer and nothing else. As it > >>> should. > > > > The most common case for control handling in drivers is taking the > > integer value and converting it to a register value, using > > device-specific encoding of the register value. It can be a fixed-point > > format or something else, depending on the device. My point is that > > drivers routinely convert a "plain" integer to something else, and that > > has never been considered as a cause of concern. I don't see why it > > would be different in this series. > > > >>> And while we do not have support for units (other than the > >>> documentation), > >>> we do have type support in the form of V4L2_CTRL_TYPE_*. > >>> > > A quick "git grep -i "fixed point" > > Documentation/userspace-api/media/' > > only shows a single driver specific control (dw100.rst). > > > > I'm not aware of other controls in mainline that use fixed point. > > The analog gain control for sensors for instance. > >>> > >>> Not really. The documentation is super vague: > >>> > >>> V4L2_CID_ANALOGUE_GAIN (integer) > >>> > >>> Analogue gain is gain affecting all colour components in the > >>> pixel matrix. The > >>> gain operation is performed in the analogue domain before A/D > >>> conversion. > >>> > >>> And the integer is just a range. Internally it might map to some fixed > >>> point value, but userspace won't see that, it's hidden in the driver > >>> AFAICT. > > > > It's hidden so well that libcamera has a database of the sensor it > > supports, with formulas to map a real gain value to the > > V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > > matter, and the kernel doesn't expose it. We may or may not consider > > that as a shortcoming of the V4L2 control API, but in any case it's the > > situation we have today. > > > >> I wonder if Laurent meant digital gain. >
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 11/15/23 11:55, Laurent Pinchart wrote: > Hi Hans, > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: >> On 13/11/2023 13:44, Laurent Pinchart wrote: >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: On 13/11/2023 12:43, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: >>> On 13/11/2023 12:07, Laurent Pinchart wrote: On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: >>> On 10/11/2023 06:48, Shengjiu Wang wrote: Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format. Signed-off-by: Shengjiu Wang >>> >>> This patch adds a new control type. This is something that also >>> needs to be >>> tested by v4l2-compliance, and for that we need to add support for >>> this to >>> one of the media test-drivers. The best place for that is the vivid >>> driver, >>> since that has already a bunch of test controls for other control >>> types. >>> >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. >>> >>> Can you add a patch adding a fixed point test control to vivid? >> >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems >> to >> relate more to units than control types. We have lots of fixed-point >> values in controls already, using the 32-bit and 64-bit integer >> control >> types. They use various locations for the decimal point, depending on >> the control. If we want to make this more explicit to users, we >> should >> work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. It's not a unit, but I think it's related to units. My point is that, without units support, I don't see why we need a formal definition of fixed-point types, and why this series couldn't just use VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 values as they see fit. >>> >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> is always interpreted as a 64 bit integer and nothing else. As it >>> should. > > The most common case for control handling in drivers is taking the > integer value and converting it to a register value, using > device-specific encoding of the register value. It can be a fixed-point > format or something else, depending on the device. My point is that > drivers routinely convert a "plain" integer to something else, and that > has never been considered as a cause of concern. I don't see why it > would be different in this series. > >>> And while we do not have support for units (other than the >>> documentation), >>> we do have type support in the form of V4L2_CTRL_TYPE_*. >>> > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. The analog gain control for sensors for instance. >>> >>> Not really. The documentation is super vague: >>> >>> V4L2_CID_ANALOGUE_GAIN (integer) >>> >>> Analogue gain is gain affecting all colour components in the >>> pixel matrix. The >>> gain operation is performed in the analogue domain before A/D >>> conversion. >>> >>> And the integer is just a range. Internally it might map to some fixed >>> point value, but userspace won't see that, it's hidden in the driver >>> AFAICT. > > It's hidden so well that libcamera has a database of the sensor it > supports, with formulas to map a real gain value to the > V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > matter, and the kernel doesn't expose it. We may or may not consider > that as a shortcoming of the V4L2 control API, but in any case it's the > situation we have today. > >> I wonder if Laurent meant digital gain. > > No, I meant analog. It applies to digital gain too though. > >> Those are often Q numbers. The practice there has been that the default >> value yields gain of 1. >> >> There are probably many other examples in controls where something being >> controlled isn't
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > On 13/11/2023 13:44, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > >> On 13/11/2023 12:43, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > On 13/11/2023 12:07, Laurent Pinchart wrote: > >> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 11:42, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > On 10/11/2023 06:48, Shengjiu Wang wrote: > >> Fixed point controls are used by the user to configure > >> a fixed point value in 64bits, which Q31.32 format. > >> > >> Signed-off-by: Shengjiu Wang > > > > This patch adds a new control type. This is something that also > > needs to be > > tested by v4l2-compliance, and for that we need to add support for > > this to > > one of the media test-drivers. The best place for that is the vivid > > driver, > > since that has already a bunch of test controls for other control > > types. > > > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > > > Can you add a patch adding a fixed point test control to vivid? > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems > to > relate more to units than control types. We have lots of fixed-point > values in controls already, using the 32-bit and 64-bit integer > control > types. They use various locations for the decimal point, depending on > the control. If we want to make this more explicit to users, we > should > work on adding unit support to the V4L2 controls. > >>> > >>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > >> > >> It's not a unit, but I think it's related to units. My point is that, > >> without units support, I don't see why we need a formal definition of > >> fixed-point types, and why this series couldn't just use > >> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >> values as they see fit. > > > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > >>> > >>> Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> > > is always interpreted as a 64 bit integer and nothing else. As it > > should. > >>> > >>> The most common case for control handling in drivers is taking the > >>> integer value and converting it to a register value, using > >>> device-specific encoding of the register value. It can be a fixed-point > >>> format or something else, depending on the device. My point is that > >>> drivers routinely convert a "plain" integer to something else, and that > >>> has never been considered as a cause of concern. I don't see why it > >>> would be different in this series. > >>> > > And while we do not have support for units (other than the > > documentation), > > we do have type support in the form of V4L2_CTRL_TYPE_*. > > > >>> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > >>> only shows a single driver specific control (dw100.rst). > >>> > >>> I'm not aware of other controls in mainline that use fixed point. > >> > >> The analog gain control for sensors for instance. > > > > Not really. The documentation is super vague: > > > > V4L2_CID_ANALOGUE_GAIN (integer) > > > > Analogue gain is gain affecting all colour components in the > > pixel matrix. The > > gain operation is performed in the analogue domain before A/D > > conversion. > > > > And the integer is just a range. Internally it might map to some fixed > > point value, but userspace won't see that, it's hidden in the driver > > AFAICT. > >>> > >>> It's hidden so well that libcamera has a database of the sensor it > >>> supports, with formulas to map a real gain value to the > >>> V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > >>> matter, and the kernel doesn't expose it. We may or may not consider > >>> that as a shortcoming of the V4L2 control API, but in any case it's the > >>> situation we have today. > >>> > I wonder if Laurent meant digital gain. > >>> > >>> No, I meant analog. It applies to digital gain too though. > >>> > Those are often Q numbers. The practice there has been that the default > value yields gain of 1. > > There are probably many other examples in controls where something being > controlled isn't actually an integer while integer controls are still >
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 15/11/2023 09:45, Tomasz Figa wrote: > On Wed, Nov 15, 2023 at 5:09 PM Hans Verkuil wrote: >> >> Hi Laurent, >> >> On 13/11/2023 13:44, Laurent Pinchart wrote: >>> Hi Hans, >>> >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: On 13/11/2023 12:43, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: >>> On 13/11/2023 12:07, Laurent Pinchart wrote: On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: >>> On 10/11/2023 06:48, Shengjiu Wang wrote: Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format. Signed-off-by: Shengjiu Wang >>> >>> This patch adds a new control type. This is something that also >>> needs to be >>> tested by v4l2-compliance, and for that we need to add support for >>> this to >>> one of the media test-drivers. The best place for that is the vivid >>> driver, >>> since that has already a bunch of test controls for other control >>> types. >>> >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. >>> >>> Can you add a patch adding a fixed point test control to vivid? >> >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems >> to >> relate more to units than control types. We have lots of fixed-point >> values in controls already, using the 32-bit and 64-bit integer >> control >> types. They use various locations for the decimal point, depending on >> the control. If we want to make this more explicit to users, we >> should >> work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. It's not a unit, but I think it's related to units. My point is that, without units support, I don't see why we need a formal definition of fixed-point types, and why this series couldn't just use VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 values as they see fit. >>> >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> is always interpreted as a 64 bit integer and nothing else. As it >>> should. > > The most common case for control handling in drivers is taking the > integer value and converting it to a register value, using > device-specific encoding of the register value. It can be a fixed-point > format or something else, depending on the device. My point is that > drivers routinely convert a "plain" integer to something else, and that > has never been considered as a cause of concern. I don't see why it > would be different in this series. > >>> And while we do not have support for units (other than the >>> documentation), >>> we do have type support in the form of V4L2_CTRL_TYPE_*. >>> > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. The analog gain control for sensors for instance. >>> >>> Not really. The documentation is super vague: >>> >>> V4L2_CID_ANALOGUE_GAIN (integer) >>> >>> Analogue gain is gain affecting all colour components in the pixel >>> matrix. The >>> gain operation is performed in the analogue domain before A/D >>> conversion. >>> >>> And the integer is just a range. Internally it might map to some fixed >>> point value, but userspace won't see that, it's hidden in the driver >>> AFAICT. > > It's hidden so well that libcamera has a database of the sensor it > supports, with formulas to map a real gain value to the > V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > matter, and the kernel doesn't expose it. We may or may not consider > that as a shortcoming of the V4L2 control API, but in any case it's the > situation we have today. > >> I wonder if Laurent meant digital gain. > > No, I meant analog. It applies to digital gain too though. > >> Those are often Q numbers. The practice there has been that the default >> value yields gain of 1. >> >> There are probably many other examples in controls where something being >> controlled isn't
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Wed, Nov 15, 2023 at 5:09 PM Hans Verkuil wrote: > > Hi Laurent, > > On 13/11/2023 13:44, Laurent Pinchart wrote: > > Hi Hans, > > > > On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > >> On 13/11/2023 12:43, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > On 13/11/2023 12:07, Laurent Pinchart wrote: > >> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 11:42, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > On 10/11/2023 06:48, Shengjiu Wang wrote: > >> Fixed point controls are used by the user to configure > >> a fixed point value in 64bits, which Q31.32 format. > >> > >> Signed-off-by: Shengjiu Wang > > > > This patch adds a new control type. This is something that also > > needs to be > > tested by v4l2-compliance, and for that we need to add support for > > this to > > one of the media test-drivers. The best place for that is the vivid > > driver, > > since that has already a bunch of test controls for other control > > types. > > > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > > > Can you add a patch adding a fixed point test control to vivid? > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems > to > relate more to units than control types. We have lots of fixed-point > values in controls already, using the 32-bit and 64-bit integer > control > types. They use various locations for the decimal point, depending on > the control. If we want to make this more explicit to users, we > should > work on adding unit support to the V4L2 controls. > >>> > >>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > >> > >> It's not a unit, but I think it's related to units. My point is that, > >> without units support, I don't see why we need a formal definition of > >> fixed-point types, and why this series couldn't just use > >> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >> values as they see fit. > > > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > >>> > >>> Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> > > is always interpreted as a 64 bit integer and nothing else. As it > > should. > >>> > >>> The most common case for control handling in drivers is taking the > >>> integer value and converting it to a register value, using > >>> device-specific encoding of the register value. It can be a fixed-point > >>> format or something else, depending on the device. My point is that > >>> drivers routinely convert a "plain" integer to something else, and that > >>> has never been considered as a cause of concern. I don't see why it > >>> would be different in this series. > >>> > > And while we do not have support for units (other than the > > documentation), > > we do have type support in the form of V4L2_CTRL_TYPE_*. > > > >>> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > >>> only shows a single driver specific control (dw100.rst). > >>> > >>> I'm not aware of other controls in mainline that use fixed point. > >> > >> The analog gain control for sensors for instance. > > > > Not really. The documentation is super vague: > > > > V4L2_CID_ANALOGUE_GAIN (integer) > > > > Analogue gain is gain affecting all colour components in the pixel > > matrix. The > > gain operation is performed in the analogue domain before A/D > > conversion. > > > > And the integer is just a range. Internally it might map to some fixed > > point value, but userspace won't see that, it's hidden in the driver > > AFAICT. > >>> > >>> It's hidden so well that libcamera has a database of the sensor it > >>> supports, with formulas to map a real gain value to the > >>> V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > >>> matter, and the kernel doesn't expose it. We may or may not consider > >>> that as a shortcoming of the V4L2 control API, but in any case it's the > >>> situation we have today. > >>> > I wonder if Laurent meant digital gain. > >>> > >>> No, I meant analog. It applies to digital gain too though. > >>> > Those are often Q numbers. The practice there has been that the default > value yields gain of 1. > > There are probably many other examples in controls where something being > controlled isn't actually an integer while integer controls are
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 10/11/2023 06:48, Shengjiu Wang wrote: > Fixed point controls are used by the user to configure > a fixed point value in 64bits, which Q31.32 format. > > Signed-off-by: Shengjiu Wang > --- > .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- > .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - > .../userspace-api/media/videodev2.h.rst.exceptions | 1 + > drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - > drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ > include/uapi/linux/videodev2.h | 1 + > 6 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > index e8475f9fd2cf..e7e5d78dc11e 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > @@ -162,13 +162,13 @@ still cause this situation. > * - __s32 >- ``value`` >- New value or current value. Valid if this control is not of type > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > - not set. > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > * - __s64 >- ``value64`` >- New value or current value. Valid if this control is of type > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > - not set. > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > * - char * >- ``string`` >- A pointer to a string. Valid if this control is of type > @@ -193,8 +193,9 @@ still cause this situation. > * - __s64 * >- ``p_s64`` >- A pointer to a matrix control of signed 64-bit values. Valid if > -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and > -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. > +this control is of type ``V4L2_CTRL_TYPE_INTEGER64``, > +``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` > +is set. > * - struct :c:type:`v4l2_area` * >- ``p_area`` >- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is > diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > index 4d38acafe8e1..f3995ec57044 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. >- ``default_value`` >- The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, > ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8`` > - or ``_U16`` control. Not valid for other types of controls. > + ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of > + controls. > > .. note:: > > @@ -549,6 +550,12 @@ See also the examples in :ref:`control`. >- n/a >- A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film > Grain > parameters for stateless video decoders. > +* - ``V4L2_CTRL_TYPE_FIXED_POINT`` > + - any > + - any > + - any > + - A 64-bit integer valued control, containing parameter which is > +Q31.32 format. > > .. raw:: latex > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > index e61152bb80d1..2faa5a2015eb 100644 > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE > :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` > +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type` > > # V4L2 capability defines > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c > b/drivers/media/v4l2-core/v4l2-ctrls-api.c > index 002ea6588edf..e6a0fb8d6791 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c > @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c, > return copy_to_user(c->string, ptr.p_char, len + 1) ? > -EFAULT : 0; > case V4L2_CTRL_TYPE_INTEGER64: > + case V4L2_CTRL_TYPE_FIXED_POINT:n > c->value64 = *ptr.p_s64; > break; > default: > @@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c,
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Laurent, On 13/11/2023 13:44, Laurent Pinchart wrote: > Hi Hans, > > On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: >> On 13/11/2023 12:43, Laurent Pinchart wrote: >>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > On 13/11/2023 12:07, Laurent Pinchart wrote: >> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: >>> On 13/11/2023 11:42, Laurent Pinchart wrote: On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > On 10/11/2023 06:48, Shengjiu Wang wrote: >> Fixed point controls are used by the user to configure >> a fixed point value in 64bits, which Q31.32 format. >> >> Signed-off-by: Shengjiu Wang > > This patch adds a new control type. This is something that also needs > to be > tested by v4l2-compliance, and for that we need to add support for > this to > one of the media test-drivers. The best place for that is the vivid > driver, > since that has already a bunch of test controls for other control > types. > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > Can you add a patch adding a fixed point test control to vivid? I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to relate more to units than control types. We have lots of fixed-point values in controls already, using the 32-bit and 64-bit integer control types. They use various locations for the decimal point, depending on the control. If we want to make this more explicit to users, we should work on adding unit support to the V4L2 controls. >>> >>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. >> >> It's not a unit, but I think it's related to units. My point is that, >> without units support, I don't see why we need a formal definition of >> fixed-point types, and why this series couldn't just use >> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 >> values as they see fit. > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it >>> >>> Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) >>> > is always interpreted as a 64 bit integer and nothing else. As it should. >>> >>> The most common case for control handling in drivers is taking the >>> integer value and converting it to a register value, using >>> device-specific encoding of the register value. It can be a fixed-point >>> format or something else, depending on the device. My point is that >>> drivers routinely convert a "plain" integer to something else, and that >>> has never been considered as a cause of concern. I don't see why it >>> would be different in this series. >>> > And while we do not have support for units (other than the documentation), > we do have type support in the form of V4L2_CTRL_TYPE_*. > >>> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' >>> only shows a single driver specific control (dw100.rst). >>> >>> I'm not aware of other controls in mainline that use fixed point. >> >> The analog gain control for sensors for instance. > > Not really. The documentation is super vague: > > V4L2_CID_ANALOGUE_GAIN (integer) > > Analogue gain is gain affecting all colour components in the pixel > matrix. The > gain operation is performed in the analogue domain before A/D > conversion. > > And the integer is just a range. Internally it might map to some fixed > point value, but userspace won't see that, it's hidden in the driver > AFAICT. >>> >>> It's hidden so well that libcamera has a database of the sensor it >>> supports, with formulas to map a real gain value to the >>> V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does >>> matter, and the kernel doesn't expose it. We may or may not consider >>> that as a shortcoming of the V4L2 control API, but in any case it's the >>> situation we have today. >>> I wonder if Laurent meant digital gain. >>> >>> No, I meant analog. It applies to digital gain too though. >>> Those are often Q numbers. The practice there has been that the default value yields gain of 1. There are probably many other examples in controls where something being controlled isn't actually an integer while integer controls are still being used for the purpose. >>> >>> A good summary of my opinion :-) >> >> And that works fine as long as userspace doesn't need to know what the value >> actually means. >> >> That's not the case here. The control is really a fractional Hz value: >> >> +``V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET (fixed
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > On 13/11/2023 12:43, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 12:07, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > On 13/11/2023 11:42, Laurent Pinchart wrote: > >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >>> On 10/11/2023 06:48, Shengjiu Wang wrote: > Fixed point controls are used by the user to configure > a fixed point value in 64bits, which Q31.32 format. > > Signed-off-by: Shengjiu Wang > >>> > >>> This patch adds a new control type. This is something that also needs > >>> to be > >>> tested by v4l2-compliance, and for that we need to add support for > >>> this to > >>> one of the media test-drivers. The best place for that is the vivid > >>> driver, > >>> since that has already a bunch of test controls for other control > >>> types. > >>> > >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >>> > >>> Can you add a patch adding a fixed point test control to vivid? > >> > >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > >> relate more to units than control types. We have lots of fixed-point > >> values in controls already, using the 32-bit and 64-bit integer control > >> types. They use various locations for the decimal point, depending on > >> the control. If we want to make this more explicit to users, we should > >> work on adding unit support to the V4L2 controls. > > > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > It's not a unit, but I think it's related to units. My point is that, > without units support, I don't see why we need a formal definition of > fixed-point types, and why this series couldn't just use > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > values as they see fit. > >>> > >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > > > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > > >>> is always interpreted as a 64 bit integer and nothing else. As it should. > > > > The most common case for control handling in drivers is taking the > > integer value and converting it to a register value, using > > device-specific encoding of the register value. It can be a fixed-point > > format or something else, depending on the device. My point is that > > drivers routinely convert a "plain" integer to something else, and that > > has never been considered as a cause of concern. I don't see why it > > would be different in this series. > > > >>> And while we do not have support for units (other than the documentation), > >>> we do have type support in the form of V4L2_CTRL_TYPE_*. > >>> > > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > > only shows a single driver specific control (dw100.rst). > > > > I'm not aware of other controls in mainline that use fixed point. > > The analog gain control for sensors for instance. > >>> > >>> Not really. The documentation is super vague: > >>> > >>> V4L2_CID_ANALOGUE_GAIN (integer) > >>> > >>> Analogue gain is gain affecting all colour components in the pixel > >>> matrix. The > >>> gain operation is performed in the analogue domain before A/D > >>> conversion. > >>> > >>> And the integer is just a range. Internally it might map to some fixed > >>> point value, but userspace won't see that, it's hidden in the driver > >>> AFAICT. > > > > It's hidden so well that libcamera has a database of the sensor it > > supports, with formulas to map a real gain value to the > > V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > > matter, and the kernel doesn't expose it. We may or may not consider > > that as a shortcoming of the V4L2 control API, but in any case it's the > > situation we have today. > > > >> I wonder if Laurent meant digital gain. > > > > No, I meant analog. It applies to digital gain too though. > > > >> Those are often Q numbers. The practice there has been that the default > >> value yields gain of 1. > >> > >> There are probably many other examples in controls where something being > >> controlled isn't actually an integer while integer controls are still being > >> used for the purpose. > > > > A good summary of my opinion :-) > > And that works fine as long as userspace doesn't need to know what the value > actually means. > > That's not the case here. The control is really a fractional Hz value: > > +``V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET (fixed point)`` > +Sets the offset from the audio source sample
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 13/11/2023 12:43, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: >> Hi Hans, >> >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: >>> On 13/11/2023 12:07, Laurent Pinchart wrote: On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: >> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: >>> Hi Shengjiu, >>> >>> On 10/11/2023 06:48, Shengjiu Wang wrote: Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format. Signed-off-by: Shengjiu Wang >>> >>> This patch adds a new control type. This is something that also needs >>> to be >>> tested by v4l2-compliance, and for that we need to add support for this >>> to >>> one of the media test-drivers. The best place for that is the vivid >>> driver, >>> since that has already a bunch of test controls for other control types. >>> >>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. >>> >>> Can you add a patch adding a fixed point test control to vivid? >> >> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to >> relate more to units than control types. We have lots of fixed-point >> values in controls already, using the 32-bit and 64-bit integer control >> types. They use various locations for the decimal point, depending on >> the control. If we want to make this more explicit to users, we should >> work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. It's not a unit, but I think it's related to units. My point is that, without units support, I don't see why we need a formal definition of fixed-point types, and why this series couldn't just use VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 values as they see fit. >>> >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> is always interpreted as a 64 bit integer and nothing else. As it should. > > The most common case for control handling in drivers is taking the > integer value and converting it to a register value, using > device-specific encoding of the register value. It can be a fixed-point > format or something else, depending on the device. My point is that > drivers routinely convert a "plain" integer to something else, and that > has never been considered as a cause of concern. I don't see why it > would be different in this series. > >>> And while we do not have support for units (other than the documentation), >>> we do have type support in the form of V4L2_CTRL_TYPE_*. >>> > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. The analog gain control for sensors for instance. >>> >>> Not really. The documentation is super vague: >>> >>> V4L2_CID_ANALOGUE_GAIN (integer) >>> >>> Analogue gain is gain affecting all colour components in the pixel >>> matrix. The >>> gain operation is performed in the analogue domain before A/D >>> conversion. >>> >>> And the integer is just a range. Internally it might map to some fixed >>> point value, but userspace won't see that, it's hidden in the driver AFAICT. > > It's hidden so well that libcamera has a database of the sensor it > supports, with formulas to map a real gain value to the > V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does > matter, and the kernel doesn't expose it. We may or may not consider > that as a shortcoming of the V4L2 control API, but in any case it's the > situation we have today. > >> I wonder if Laurent meant digital gain. > > No, I meant analog. It applies to digital gain too though. > >> Those are often Q numbers. The practice there has been that the default >> value yields gain of 1. >> >> There are probably many other examples in controls where something being >> controlled isn't actually an integer while integer controls are still being >> used for the purpose. > > A good summary of my opinion :-) And that works fine as long as userspace doesn't need to know what the value actually means. That's not the case here. The control is really a fractional Hz value: +``V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET (fixed point)`` +Sets the offset from the audio source sample rate, unit is Hz. +The offset compensates for any clock drift. The actual source audio sample +rate is the ideal source audio sample rate from +``V4L2_CID_M2M_AUDIO_SOURCE_RATE`` plus this fixed point offset. > >> Instead of this
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > Hi Hans, > > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > On 13/11/2023 12:07, Laurent Pinchart wrote: > > > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > >> On 13/11/2023 11:42, Laurent Pinchart wrote: > > >>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > Hi Shengjiu, > > > > On 10/11/2023 06:48, Shengjiu Wang wrote: > > > Fixed point controls are used by the user to configure > > > a fixed point value in 64bits, which Q31.32 format. > > > > > > Signed-off-by: Shengjiu Wang > > > > This patch adds a new control type. This is something that also needs > > to be > > tested by v4l2-compliance, and for that we need to add support for > > this to > > one of the media test-drivers. The best place for that is the vivid > > driver, > > since that has already a bunch of test controls for other control > > types. > > > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > > > Can you add a patch adding a fixed point test control to vivid? > > >>> > > >>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > > >>> relate more to units than control types. We have lots of fixed-point > > >>> values in controls already, using the 32-bit and 64-bit integer control > > >>> types. They use various locations for the decimal point, depending on > > >>> the control. If we want to make this more explicit to users, we should > > >>> work on adding unit support to the V4L2 controls. > > >> > > >> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > > > > > It's not a unit, but I think it's related to units. My point is that, > > > without units support, I don't see why we need a formal definition of > > > fixed-point types, and why this series couldn't just use > > > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > > > values as they see fit. > > > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > is always interpreted as a 64 bit integer and nothing else. As it should. The most common case for control handling in drivers is taking the integer value and converting it to a register value, using device-specific encoding of the register value. It can be a fixed-point format or something else, depending on the device. My point is that drivers routinely convert a "plain" integer to something else, and that has never been considered as a cause of concern. I don't see why it would be different in this series. > > And while we do not have support for units (other than the documentation), > > we do have type support in the form of V4L2_CTRL_TYPE_*. > > > > >> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > > >> only shows a single driver specific control (dw100.rst). > > >> > > >> I'm not aware of other controls in mainline that use fixed point. > > > > > > The analog gain control for sensors for instance. > > > > Not really. The documentation is super vague: > > > > V4L2_CID_ANALOGUE_GAIN (integer) > > > > Analogue gain is gain affecting all colour components in the pixel > > matrix. The > > gain operation is performed in the analogue domain before A/D > > conversion. > > > > And the integer is just a range. Internally it might map to some fixed > > point value, but userspace won't see that, it's hidden in the driver AFAICT. It's hidden so well that libcamera has a database of the sensor it supports, with formulas to map a real gain value to the V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does matter, and the kernel doesn't expose it. We may or may not consider that as a shortcoming of the V4L2 control API, but in any case it's the situation we have today. > I wonder if Laurent meant digital gain. No, I meant analog. It applies to digital gain too though. > Those are often Q numbers. The practice there has been that the default > value yields gain of 1. > > There are probably many other examples in controls where something being > controlled isn't actually an integer while integer controls are still being > used for the purpose. A good summary of my opinion :-) > Instead of this patch, I'd prefer to have a way to express the meaning of > the control value, be it a Q number or something else, and do that > independently of the type of the control. Agreed. > > In the case of this particular series the control type is really a fixed > > point > > value with a documented unit (Hz). It really is not something you want to > > use type INTEGER64 for. > > > > >> Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting > > >> min/max/step you can easily map that to just about any QN.M format where > > >> N <=
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > On 13/11/2023 12:07, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >> On 13/11/2023 11:42, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > Hi Shengjiu, > > On 10/11/2023 06:48, Shengjiu Wang wrote: > > Fixed point controls are used by the user to configure > > a fixed point value in 64bits, which Q31.32 format. > > > > Signed-off-by: Shengjiu Wang > > This patch adds a new control type. This is something that also needs to > be > tested by v4l2-compliance, and for that we need to add support for this > to > one of the media test-drivers. The best place for that is the vivid > driver, > since that has already a bunch of test controls for other control types. > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > Can you add a patch adding a fixed point test control to vivid? > >>> > >>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > >>> relate more to units than control types. We have lots of fixed-point > >>> values in controls already, using the 32-bit and 64-bit integer control > >>> types. They use various locations for the decimal point, depending on > >>> the control. If we want to make this more explicit to users, we should > >>> work on adding unit support to the V4L2 controls. > >> > >> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > > > It's not a unit, but I think it's related to units. My point is that, > > without units support, I don't see why we need a formal definition of > > fixed-point types, and why this series couldn't just use > > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > > values as they see fit. > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > is always interpreted as a 64 bit integer and nothing else. As it should. > > And while we do not have support for units (other than the documentation), > we do have type support in the form of V4L2_CTRL_TYPE_*. > > > > >> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > >> only shows a single driver specific control (dw100.rst). > >> > >> I'm not aware of other controls in mainline that use fixed point. > > > > The analog gain control for sensors for instance. > > Not really. The documentation is super vague: > > V4L2_CID_ANALOGUE_GAIN (integer) > > Analogue gain is gain affecting all colour components in the pixel > matrix. The > gain operation is performed in the analogue domain before A/D > conversion. > > And the integer is just a range. Internally it might map to some fixed > point value, but userspace won't see that, it's hidden in the driver AFAICT. I wonder if Laurent meant digital gain. Those are often Q numbers. The practice there has been that the default value yields gain of 1. There are probably many other examples in controls where something being controlled isn't actually an integer while integer controls are still being used for the purpose. Instead of this patch, I'd prefer to have a way to express the meaning of the control value, be it a Q number or something else, and do that independently of the type of the control. > > In the case of this particular series the control type is really a fixed point > value with a documented unit (Hz). It really is not something you want to > use type INTEGER64 for. > > > > >> Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting > >> min/max/step you can easily map that to just about any QN.M format where > >> N <= 31 and M <= 32. > >> > >> In the case of dw100 it is a bit different in that it is quite specialized > >> and it had to fit in 16 bits. -- Regards, Sakari Ailus
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 13/11/2023 12:07, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: >> On 13/11/2023 11:42, Laurent Pinchart wrote: >>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: Hi Shengjiu, On 10/11/2023 06:48, Shengjiu Wang wrote: > Fixed point controls are used by the user to configure > a fixed point value in 64bits, which Q31.32 format. > > Signed-off-by: Shengjiu Wang This patch adds a new control type. This is something that also needs to be tested by v4l2-compliance, and for that we need to add support for this to one of the media test-drivers. The best place for that is the vivid driver, since that has already a bunch of test controls for other control types. See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. Can you add a patch adding a fixed point test control to vivid? >>> >>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to >>> relate more to units than control types. We have lots of fixed-point >>> values in controls already, using the 32-bit and 64-bit integer control >>> types. They use various locations for the decimal point, depending on >>> the control. If we want to make this more explicit to users, we should >>> work on adding unit support to the V4L2 controls. >> >> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > It's not a unit, but I think it's related to units. My point is that, > without units support, I don't see why we need a formal definition of > fixed-point types, and why this series couldn't just use > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > values as they see fit. They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it is always interpreted as a 64 bit integer and nothing else. As it should. And while we do not have support for units (other than the documentation), we do have type support in the form of V4L2_CTRL_TYPE_*. > >> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' >> only shows a single driver specific control (dw100.rst). >> >> I'm not aware of other controls in mainline that use fixed point. > > The analog gain control for sensors for instance. Not really. The documentation is super vague: V4L2_CID_ANALOGUE_GAIN (integer) Analogue gain is gain affecting all colour components in the pixel matrix. The gain operation is performed in the analogue domain before A/D conversion. And the integer is just a range. Internally it might map to some fixed point value, but userspace won't see that, it's hidden in the driver AFAICT. In the case of this particular series the control type is really a fixed point value with a documented unit (Hz). It really is not something you want to use type INTEGER64 for. > >> Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting >> min/max/step you can easily map that to just about any QN.M format where >> N <= 31 and M <= 32. >> >> In the case of dw100 it is a bit different in that it is quite specialized >> and it had to fit in 16 bits. Regards, Hans
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >> Hi Shengjiu, > >> > >> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>> Fixed point controls are used by the user to configure > >>> a fixed point value in 64bits, which Q31.32 format. > >>> > >>> Signed-off-by: Shengjiu Wang > >> > >> This patch adds a new control type. This is something that also needs to be > >> tested by v4l2-compliance, and for that we need to add support for this to > >> one of the media test-drivers. The best place for that is the vivid driver, > >> since that has already a bunch of test controls for other control types. > >> > >> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >> > >> Can you add a patch adding a fixed point test control to vivid? > > > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > > relate more to units than control types. We have lots of fixed-point > > values in controls already, using the 32-bit and 64-bit integer control > > types. They use various locations for the decimal point, depending on > > the control. If we want to make this more explicit to users, we should > > work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. It's not a unit, but I think it's related to units. My point is that, without units support, I don't see why we need a formal definition of fixed-point types, and why this series couldn't just use VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 values as they see fit. > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. The analog gain control for sensors for instance. > Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting > min/max/step you can easily map that to just about any QN.M format where > N <= 31 and M <= 32. > > In the case of dw100 it is a bit different in that it is quite specialized > and it had to fit in 16 bits. > > >>> --- > >>> .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- > >>> .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - > >>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 + > >>> drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - > >>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ > >>> include/uapi/linux/videodev2.h | 1 + > >>> 6 files changed, 23 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> index e8475f9fd2cf..e7e5d78dc11e 100644 > >>> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> @@ -162,13 +162,13 @@ still cause this situation. > >>> * - __s32 > >>>- ``value`` > >>>- New value or current value. Valid if this control is not of type > >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > >>> - not set. > >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > >>> * - __s64 > >>>- ``value64`` > >>>- New value or current value. Valid if this control is of type > >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > >>> - not set. > >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > >>> * - char * > >>>- ``string`` > >>>- A pointer to a string. Valid if this control is of type > >>> @@ -193,8 +193,9 @@ still cause this situation. > >>> * - __s64 * > >>>- ``p_s64`` > >>>- A pointer to a matrix control of signed 64-bit values. Valid if > >>> -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and > >>> -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. > >>> +this control is of type ``V4L2_CTRL_TYPE_INTEGER64``, > >>> +``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` > >>> +is set. > >>> * - struct :c:type:`v4l2_area` * > >>>- ``p_area`` > >>>- A pointer to a struct :c:type:`v4l2_area`. Valid if this control > >>> is > >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > >>> b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > >>> index 4d38acafe8e1..f3995ec57044 100644 > >>> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > >>> @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. > >>>- ``default_value`` > >>>- The default value of
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On 13/11/2023 11:42, Laurent Pinchart wrote: > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: >> Hi Shengjiu, >> >> On 10/11/2023 06:48, Shengjiu Wang wrote: >>> Fixed point controls are used by the user to configure >>> a fixed point value in 64bits, which Q31.32 format. >>> >>> Signed-off-by: Shengjiu Wang >> >> This patch adds a new control type. This is something that also needs to be >> tested by v4l2-compliance, and for that we need to add support for this to >> one of the media test-drivers. The best place for that is the vivid driver, >> since that has already a bunch of test controls for other control types. >> >> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. >> >> Can you add a patch adding a fixed point test control to vivid? > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > relate more to units than control types. We have lots of fixed-point > values in controls already, using the 32-bit and 64-bit integer control > types. They use various locations for the decimal point, depending on > the control. If we want to make this more explicit to users, we should > work on adding unit support to the V4L2 controls. "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. A quick "git grep -i "fixed point" Documentation/userspace-api/media/' only shows a single driver specific control (dw100.rst). I'm not aware of other controls in mainline that use fixed point. Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting min/max/step you can easily map that to just about any QN.M format where N <= 31 and M <= 32. In the case of dw100 it is a bit different in that it is quite specialized and it had to fit in 16 bits. Regards, Hans > >>> --- >>> .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- >>> .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - >>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 + >>> drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - >>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ >>> include/uapi/linux/videodev2.h | 1 + >>> 6 files changed, 23 insertions(+), 8 deletions(-) >>> >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >>> b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >>> index e8475f9fd2cf..e7e5d78dc11e 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >>> @@ -162,13 +162,13 @@ still cause this situation. >>> * - __s32 >>>- ``value`` >>>- New value or current value. Valid if this control is not of type >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is >>> - not set. >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. >>> * - __s64 >>>- ``value64`` >>>- New value or current value. Valid if this control is of type >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is >>> - not set. >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. >>> * - char * >>>- ``string`` >>>- A pointer to a string. Valid if this control is of type >>> @@ -193,8 +193,9 @@ still cause this situation. >>> * - __s64 * >>>- ``p_s64`` >>>- A pointer to a matrix control of signed 64-bit values. Valid if >>> -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and >>> -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. >>> +this control is of type ``V4L2_CTRL_TYPE_INTEGER64``, >>> +``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` >>> +is set. >>> * - struct :c:type:`v4l2_area` * >>>- ``p_area`` >>>- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst >>> b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst >>> index 4d38acafe8e1..f3995ec57044 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst >>> @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. >>>- ``default_value`` >>>- The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, >>> ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8`` >>> - or ``_U16`` control. Not valid for other types of controls. >>> + ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of >>> + controls. >>> >>> .. note:: >>> >>> @@ -549,6 +550,12 @@ See also the examples in :ref:`control`. >>>- n/a >>>- A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film >>> Grain >>> parameters for stateless video decoders. >>> +
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > Hi Shengjiu, > > On 10/11/2023 06:48, Shengjiu Wang wrote: > > Fixed point controls are used by the user to configure > > a fixed point value in 64bits, which Q31.32 format. > > > > Signed-off-by: Shengjiu Wang > > This patch adds a new control type. This is something that also needs to be > tested by v4l2-compliance, and for that we need to add support for this to > one of the media test-drivers. The best place for that is the vivid driver, > since that has already a bunch of test controls for other control types. > > See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > Can you add a patch adding a fixed point test control to vivid? I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to relate more to units than control types. We have lots of fixed-point values in controls already, using the 32-bit and 64-bit integer control types. They use various locations for the decimal point, depending on the control. If we want to make this more explicit to users, we should work on adding unit support to the V4L2 controls. > > --- > > .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- > > .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - > > .../userspace-api/media/videodev2.h.rst.exceptions | 1 + > > drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - > > drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ > > include/uapi/linux/videodev2.h | 1 + > > 6 files changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > > b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > > index e8475f9fd2cf..e7e5d78dc11e 100644 > > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > > @@ -162,13 +162,13 @@ still cause this situation. > > * - __s32 > >- ``value`` > >- New value or current value. Valid if this control is not of type > > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > > - not set. > > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > > * - __s64 > >- ``value64`` > >- New value or current value. Valid if this control is of type > > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > > - not set. > > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > > * - char * > >- ``string`` > >- A pointer to a string. Valid if this control is of type > > @@ -193,8 +193,9 @@ still cause this situation. > > * - __s64 * > >- ``p_s64`` > >- A pointer to a matrix control of signed 64-bit values. Valid if > > -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and > > -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. > > +this control is of type ``V4L2_CTRL_TYPE_INTEGER64``, > > +``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` > > +is set. > > * - struct :c:type:`v4l2_area` * > >- ``p_area`` > >- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > > b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > > index 4d38acafe8e1..f3995ec57044 100644 > > --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > > +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > > @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. > >- ``default_value`` > >- The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, > > ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8`` > > - or ``_U16`` control. Not valid for other types of controls. > > + ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of > > + controls. > > > > .. note:: > > > > @@ -549,6 +550,12 @@ See also the examples in :ref:`control`. > >- n/a > >- A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film > > Grain > > parameters for stateless video decoders. > > +* - ``V4L2_CTRL_TYPE_FIXED_POINT`` > > + - any > > + - any > > + - any > > + - A 64-bit integer valued control, containing parameter which is > > +Q31.32 format. > > > > .. raw:: latex > > > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > index e61152bb80d1..2faa5a2015eb 100644 > > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE > >
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Shengjiu, On 10/11/2023 06:48, Shengjiu Wang wrote: > Fixed point controls are used by the user to configure > a fixed point value in 64bits, which Q31.32 format. > > Signed-off-by: Shengjiu Wang This patch adds a new control type. This is something that also needs to be tested by v4l2-compliance, and for that we need to add support for this to one of the media test-drivers. The best place for that is the vivid driver, since that has already a bunch of test controls for other control types. See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. Can you add a patch adding a fixed point test control to vivid? Thanks! Hans > --- > .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- > .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - > .../userspace-api/media/videodev2.h.rst.exceptions | 1 + > drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - > drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ > include/uapi/linux/videodev2.h | 1 + > 6 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > index e8475f9fd2cf..e7e5d78dc11e 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > @@ -162,13 +162,13 @@ still cause this situation. > * - __s32 >- ``value`` >- New value or current value. Valid if this control is not of type > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > - not set. > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > * - __s64 >- ``value64`` >- New value or current value. Valid if this control is of type > - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > - not set. > + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > * - char * >- ``string`` >- A pointer to a string. Valid if this control is of type > @@ -193,8 +193,9 @@ still cause this situation. > * - __s64 * >- ``p_s64`` >- A pointer to a matrix control of signed 64-bit values. Valid if > -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and > -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. > +this control is of type ``V4L2_CTRL_TYPE_INTEGER64``, > +``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` > +is set. > * - struct :c:type:`v4l2_area` * >- ``p_area`` >- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is > diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > index 4d38acafe8e1..f3995ec57044 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst > @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. >- ``default_value`` >- The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, > ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8`` > - or ``_U16`` control. Not valid for other types of controls. > + ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of > + controls. > > .. note:: > > @@ -549,6 +550,12 @@ See also the examples in :ref:`control`. >- n/a >- A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film > Grain > parameters for stateless video decoders. > +* - ``V4L2_CTRL_TYPE_FIXED_POINT`` > + - any > + - any > + - any > + - A 64-bit integer valued control, containing parameter which is > +Q31.32 format. > > .. raw:: latex > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > index e61152bb80d1..2faa5a2015eb 100644 > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE > :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` > replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` > +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type` > > # V4L2 capability defines > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c > b/drivers/media/v4l2-core/v4l2-ctrls-api.c > index 002ea6588edf..e6a0fb8d6791 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c > +++
