* [PATCH v2] media: vimc: Implement debayer control for mean window size
@ 2019-09-17 22:53 Arthur Moraes do Lago
2019-09-17 23:12 ` Shuah Khan
0 siblings, 1 reply; 3+ messages in thread
From: Arthur Moraes do Lago @ 2019-09-17 22:53 UTC (permalink / raw)
To: mchehab, helen.koike, skhan, hverkuil-cisco, andrealmeid,
laispc19, arthurmoraeslago, linux-media, linux-kernel
Add mean window size parameter for debayer filter as a control in
vimc-debayer.
vimc-debayer was patched to allow changing mean windows parameter
of the filter without needing to reload the driver. The parameter
can now be set using a v4l2-ctl control(mean_window_size).
Co-developed-by: Laís Pessine do Carmo <laispc19@gmail.com>
Signed-off-by: Laís Pessine do Carmo <laispc19@gmail.com>
Signed-off-by: Arthur Moraes do Lago <arthurmoraeslago@gmail.com>
Small fixes to vimc debayer mean window size patch
---
Changes in V2:
- Updated documentation
- Added v4l2_subev_core_ops to solve errors in v4l2-ctl compliance test
- Changed control naming to follow English capitalization rules
- Rebased to Shuah Khan's newest patch series 171283
("Collapse vimc single monolithic driver")
- Change maximum value for mean window size
We did run streaming tests, the value of 1 for mean window size has a
nice side effect of not applying any filter, so we can see the original
pattern. The value of 99 for mean window size worked, but was very slow.
We wanted to find a way to show that higher values for mean window size
yielded very similar results to just blurring the image, but we could
not find any way to show this, so we just made the maximum value 25,
which runs faster, but is still probably a little high.
---
Documentation/media/v4l-drivers/vimc.rst | 10 +--
drivers/media/platform/vimc/vimc-common.h | 1 +
drivers/media/platform/vimc/vimc-debayer.c | 89 +++++++++++++++++++---
3 files changed, 79 insertions(+), 21 deletions(-)
diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
index a582af0509ee..91c909904b87 100644
--- a/Documentation/media/v4l-drivers/vimc.rst
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -80,9 +80,7 @@ vimc-capture:
Module options
---------------
-Vimc has a few module parameters to configure the driver.
-
- param=value
+Vimc has a module parameters to configure the driver.
* ``sca_mult=<unsigned int>``
@@ -91,12 +89,6 @@ Vimc has a few module parameters to configure the driver.
original one. Currently, only supports scaling up (the default value
is 3).
-* ``deb_mean_win_size=<unsigned int>``
-
- Window size to calculate the mean. Note: the window size needs to be an
- odd number, as the main pixel stays in the center of the window,
- otherwise the next odd number is considered (the default value is 3).
-
Source code documentation
-------------------------
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 236412ad7548..3a5102ddf794 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -19,6 +19,7 @@
#define VIMC_CID_VIMC_BASE (0x00f00000 | 0xf000)
#define VIMC_CID_VIMC_CLASS (0x00f00000 | 1)
#define VIMC_CID_TEST_PATTERN (VIMC_CID_VIMC_BASE + 0)
+#define VIMC_CID_MEAN_WIN_SIZE (VIMC_CID_VIMC_BASE + 1)
#define VIMC_FRAME_MAX_WIDTH 4096
#define VIMC_FRAME_MAX_HEIGHT 2160
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 37f3767db469..76df2ac110c0 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -11,17 +11,12 @@
#include <linux/platform_device.h>
#include <linux/vmalloc.h>
#include <linux/v4l2-mediabus.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
#include <media/v4l2-subdev.h>
#include "vimc-common.h"
-static unsigned int deb_mean_win_size = 3;
-module_param(deb_mean_win_size, uint, 0000);
-MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
- "NOTE: the window size needs to be an odd number, as the main pixel "
- "stays in the center of the window, otherwise the next odd number "
- "is considered");
-
enum vimc_deb_rgb_colors {
VIMC_DEB_RED = 0,
VIMC_DEB_GREEN = 1,
@@ -46,6 +41,8 @@ struct vimc_deb_device {
u8 *src_frame;
const struct vimc_deb_pix_map *sink_pix_map;
unsigned int sink_bpp;
+ unsigned int mean_win_size;
+ struct v4l2_ctrl_handler hdl;
};
static const struct v4l2_mbus_framefmt sink_fmt_default = {
@@ -346,11 +343,18 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
+static const struct v4l2_subdev_core_ops vimc_deb_core_ops = {
+ .log_status = v4l2_ctrl_subdev_log_status,
+ .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
+ .unsubscribe_event = v4l2_event_subdev_unsubscribe,
+};
+
static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
.s_stream = vimc_deb_s_stream,
};
static const struct v4l2_subdev_ops vimc_deb_ops = {
+ .core = &vimc_deb_core_ops,
.pad = &vimc_deb_pad_ops,
.video = &vimc_deb_video_ops,
};
@@ -384,7 +388,7 @@ static void vimc_deb_calc_rgb_sink(struct vimc_deb_device *vdeb,
* the top left corner of the mean window (considering the current
* pixel as the center)
*/
- seek = deb_mean_win_size / 2;
+ seek = vdeb->mean_win_size / 2;
/* Sum the values of the colors in the mean window */
@@ -474,6 +478,33 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
}
+static inline void vimc_deb_s_mean_win_size(struct vimc_deb_device *vdeb,
+ unsigned int mean_win_size)
+{
+ if (vdeb->mean_win_size == mean_win_size)
+ return;
+ vdeb->mean_win_size = mean_win_size;
+}
+
+static int vimc_deb_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct vimc_deb_device *vdeb =
+ container_of(ctrl->handler, struct vimc_deb_device, hdl);
+
+ switch (ctrl->id) {
+ case VIMC_CID_MEAN_WIN_SIZE:
+ vimc_deb_s_mean_win_size(vdeb, ctrl->val);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops vimc_deb_ctrl_ops = {
+ .s_ctrl = vimc_deb_s_ctrl,
+};
+
static void vimc_deb_release(struct v4l2_subdev *sd)
{
struct vimc_deb_device *vdeb =
@@ -495,6 +526,24 @@ void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
vimc_ent_sd_unregister(ved, &vdeb->sd);
}
+static const struct v4l2_ctrl_config vimc_deb_ctrl_class = {
+ .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY,
+ .id = VIMC_CID_VIMC_CLASS,
+ .name = "VIMC Controls",
+ .type = V4L2_CTRL_TYPE_CTRL_CLASS,
+};
+
+static const struct v4l2_ctrl_config vimc_deb_ctrl_mean_win_size = {
+ .ops = &vimc_deb_ctrl_ops,
+ .id = VIMC_CID_MEAN_WIN_SIZE,
+ .name = "Mean Window Size",
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .min = 1,
+ .max = 25,
+ .step = 2,
+ .def = 3,
+};
+
struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
const char *vcfg_name)
{
@@ -507,6 +556,16 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
if (!vdeb)
return NULL;
+ /* Create controls: */
+ v4l2_ctrl_handler_init(&vdeb->hdl, 2);
+ v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_class, NULL);
+ v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_mean_win_size, NULL);
+ vdeb->sd.ctrl_handler = &vdeb->hdl;
+ if (vdeb->hdl.error) {
+ ret = vdeb->hdl.error;
+ goto err_free_vdeb;
+ }
+
/* Initialize ved and sd */
ret = vimc_ent_sd_register(&vdeb->ved, &vdeb->sd, v4l2_dev,
vcfg_name,
@@ -514,13 +573,12 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
(const unsigned long[2]) {MEDIA_PAD_FL_SINK,
MEDIA_PAD_FL_SOURCE},
&vimc_deb_int_ops, &vimc_deb_ops);
- if (ret) {
- kfree(vdeb);
- return NULL;
- }
+ if (ret)
+ goto err_free_hdl;
vdeb->ved.process_frame = vimc_deb_process_frame;
vdeb->dev = &vimc->pdev.dev;
+ vdeb->mean_win_size = vimc_deb_ctrl_mean_win_size.def;
/* Initialize the frame format */
vdeb->sink_fmt = sink_fmt_default;
@@ -534,4 +592,11 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
vdeb->set_rgb_src = vimc_deb_set_rgb_mbus_fmt_rgb888_1x24;
return &vdeb->ved;
+
+err_free_hdl:
+ v4l2_ctrl_handler_free(&vdeb->hdl);
+err_free_vdeb:
+ kfree(vdeb);
+
+ return NULL;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] media: vimc: Implement debayer control for mean window size
2019-09-17 22:53 [PATCH v2] media: vimc: Implement debayer control for mean window size Arthur Moraes do Lago
@ 2019-09-17 23:12 ` Shuah Khan
2019-09-19 8:20 ` Hans Verkuil
0 siblings, 1 reply; 3+ messages in thread
From: Shuah Khan @ 2019-09-17 23:12 UTC (permalink / raw)
To: Arthur Moraes do Lago, mchehab, helen.koike, hverkuil-cisco,
andrealmeid, laispc19, linux-media, linux-kernel, Shuah Khan
On 9/17/19 4:53 PM, Arthur Moraes do Lago wrote:
> Add mean window size parameter for debayer filter as a control in
> vimc-debayer.
>
> vimc-debayer was patched to allow changing mean windows parameter
> of the filter without needing to reload the driver. The parameter
> can now be set using a v4l2-ctl control(mean_window_size).
>
> Co-developed-by: Laís Pessine do Carmo <laispc19@gmail.com>
> Signed-off-by: Laís Pessine do Carmo <laispc19@gmail.com>
> Signed-off-by: Arthur Moraes do Lago <arthurmoraeslago@gmail.com>
>
> Small fixes to vimc debayer mean window size patch
> ---
> Changes in V2:
> - Updated documentation
> - Added v4l2_subev_core_ops to solve errors in v4l2-ctl compliance test
> - Changed control naming to follow English capitalization rules
> - Rebased to Shuah Khan's newest patch series 171283
> ("Collapse vimc single monolithic driver")
> - Change maximum value for mean window size
>
> We did run streaming tests, the value of 1 for mean window size has a
> nice side effect of not applying any filter, so we can see the original
> pattern. The value of 99 for mean window size worked, but was very slow.
> We wanted to find a way to show that higher values for mean window size
> yielded very similar results to just blurring the image, but we could
> not find any way to show this, so we just made the maximum value 25,
> which runs faster, but is still probably a little high.
> ---
> Documentation/media/v4l-drivers/vimc.rst | 10 +--
> drivers/media/platform/vimc/vimc-common.h | 1 +
> drivers/media/platform/vimc/vimc-debayer.c | 89 +++++++++++++++++++---
> 3 files changed, 79 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
> index a582af0509ee..91c909904b87 100644
> --- a/Documentation/media/v4l-drivers/vimc.rst
> +++ b/Documentation/media/v4l-drivers/vimc.rst
> @@ -80,9 +80,7 @@ vimc-capture:
> Module options
> ---------------
>
> -Vimc has a few module parameters to configure the driver.
> -
> - param=value
> +Vimc has a module parameters to configure the driver.
parameter
>
> * ``sca_mult=<unsigned int>``
>
> @@ -91,12 +89,6 @@ Vimc has a few module parameters to configure the driver.
> original one. Currently, only supports scaling up (the default value
> is 3).
>
> -* ``deb_mean_win_size=<unsigned int>``
> -
> - Window size to calculate the mean. Note: the window size needs to be an
> - odd number, as the main pixel stays in the center of the window,
> - otherwise the next odd number is considered (the default value is 3).
> -
> Source code documentation
> -------------------------
>
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 236412ad7548..3a5102ddf794 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -19,6 +19,7 @@
> #define VIMC_CID_VIMC_BASE (0x00f00000 | 0xf000)
> #define VIMC_CID_VIMC_CLASS (0x00f00000 | 1)
> #define VIMC_CID_TEST_PATTERN (VIMC_CID_VIMC_BASE + 0)
> +#define VIMC_CID_MEAN_WIN_SIZE (VIMC_CID_VIMC_BASE + 1)
>
> #define VIMC_FRAME_MAX_WIDTH 4096
> #define VIMC_FRAME_MAX_HEIGHT 2160
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 37f3767db469..76df2ac110c0 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -11,17 +11,12 @@
> #include <linux/platform_device.h>
> #include <linux/vmalloc.h>
> #include <linux/v4l2-mediabus.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> #include <media/v4l2-subdev.h>
>
> #include "vimc-common.h"
>
> -static unsigned int deb_mean_win_size = 3;
> -module_param(deb_mean_win_size, uint, 0000);
> -MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
> - "NOTE: the window size needs to be an odd number, as the main pixel "
> - "stays in the center of the window, otherwise the next odd number "
> - "is considered");
> -
> enum vimc_deb_rgb_colors {
> VIMC_DEB_RED = 0,
> VIMC_DEB_GREEN = 1,
> @@ -46,6 +41,8 @@ struct vimc_deb_device {
> u8 *src_frame;
> const struct vimc_deb_pix_map *sink_pix_map;
> unsigned int sink_bpp;
> + unsigned int mean_win_size;
> + struct v4l2_ctrl_handler hdl;
> };
>
> static const struct v4l2_mbus_framefmt sink_fmt_default = {
> @@ -346,11 +343,18 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> return 0;
> }
>
> +static const struct v4l2_subdev_core_ops vimc_deb_core_ops = {
> + .log_status = v4l2_ctrl_subdev_log_status,
> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> +};
> +
> static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
> .s_stream = vimc_deb_s_stream,
> };
>
> static const struct v4l2_subdev_ops vimc_deb_ops = {
> + .core = &vimc_deb_core_ops,
> .pad = &vimc_deb_pad_ops,
> .video = &vimc_deb_video_ops,
> };
> @@ -384,7 +388,7 @@ static void vimc_deb_calc_rgb_sink(struct vimc_deb_device *vdeb,
> * the top left corner of the mean window (considering the current
> * pixel as the center)
> */
> - seek = deb_mean_win_size / 2;
> + seek = vdeb->mean_win_size / 2;
>
> /* Sum the values of the colors in the mean window */
>
> @@ -474,6 +478,33 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
>
> }
>
> +static inline void vimc_deb_s_mean_win_size(struct vimc_deb_device *vdeb,
> + unsigned int mean_win_size)
> +{
> + if (vdeb->mean_win_size == mean_win_size)
> + return;
> + vdeb->mean_win_size = mean_win_size;
> +}
> +
> +static int vimc_deb_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct vimc_deb_device *vdeb =
> + container_of(ctrl->handler, struct vimc_deb_device, hdl);
> +
> + switch (ctrl->id) {
> + case VIMC_CID_MEAN_WIN_SIZE:
> + vimc_deb_s_mean_win_size(vdeb, ctrl->val);
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops vimc_deb_ctrl_ops = {
> + .s_ctrl = vimc_deb_s_ctrl,
> +};
> +
> static void vimc_deb_release(struct v4l2_subdev *sd)
> {
> struct vimc_deb_device *vdeb =
> @@ -495,6 +526,24 @@ void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
> vimc_ent_sd_unregister(ved, &vdeb->sd);
> }
>
> +static const struct v4l2_ctrl_config vimc_deb_ctrl_class = {
> + .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY,
> + .id = VIMC_CID_VIMC_CLASS,
> + .name = "VIMC Controls",
Make this VIMC Debayer Controls?
> + .type = V4L2_CTRL_TYPE_CTRL_CLASS,
> +};
> +
> +static const struct v4l2_ctrl_config vimc_deb_ctrl_mean_win_size = {
> + .ops = &vimc_deb_ctrl_ops,
> + .id = VIMC_CID_MEAN_WIN_SIZE,
> + .name = "Mean Window Size",
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .min = 1,
> + .max = 25,
> + .step = 2,
> + .def = 3,
> +};
> +
> struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> const char *vcfg_name)
> {
> @@ -507,6 +556,16 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> if (!vdeb)
> return NULL;
>
> + /* Create controls: */
> + v4l2_ctrl_handler_init(&vdeb->hdl, 2);
> + v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_class, NULL);
> + v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_mean_win_size, NULL);
> + vdeb->sd.ctrl_handler = &vdeb->hdl;
> + if (vdeb->hdl.error) {
> + ret = vdeb->hdl.error;
> + goto err_free_vdeb;
> + }
> +
> /* Initialize ved and sd */
> ret = vimc_ent_sd_register(&vdeb->ved, &vdeb->sd, v4l2_dev,
> vcfg_name,
> @@ -514,13 +573,12 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
> MEDIA_PAD_FL_SOURCE},
> &vimc_deb_int_ops, &vimc_deb_ops);
> - if (ret) {
> - kfree(vdeb);
> - return NULL;
> - }
> + if (ret)
> + goto err_free_hdl;
>
> vdeb->ved.process_frame = vimc_deb_process_frame;
> vdeb->dev = &vimc->pdev.dev;
> + vdeb->mean_win_size = vimc_deb_ctrl_mean_win_size.def;
>
> /* Initialize the frame format */
> vdeb->sink_fmt = sink_fmt_default;
> @@ -534,4 +592,11 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> vdeb->set_rgb_src = vimc_deb_set_rgb_mbus_fmt_rgb888_1x24;
>
> return &vdeb->ved;
> +
> +err_free_hdl:
> + v4l2_ctrl_handler_free(&vdeb->hdl);
Free the handle from vimc_deb_release()
> +err_free_vdeb:
> + kfree(vdeb);
> +
> + return NULL;
> }
>
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] media: vimc: Implement debayer control for mean window size
2019-09-17 23:12 ` Shuah Khan
@ 2019-09-19 8:20 ` Hans Verkuil
0 siblings, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2019-09-19 8:20 UTC (permalink / raw)
To: Shuah Khan, Arthur Moraes do Lago, mchehab, helen.koike,
andrealmeid, laispc19, linux-media, linux-kernel
On 9/18/19 1:12 AM, Shuah Khan wrote:
> On 9/17/19 4:53 PM, Arthur Moraes do Lago wrote:
>> Add mean window size parameter for debayer filter as a control in
>> vimc-debayer.
>>
>> vimc-debayer was patched to allow changing mean windows parameter
>> of the filter without needing to reload the driver. The parameter
>> can now be set using a v4l2-ctl control(mean_window_size).
>>
>> Co-developed-by: Laís Pessine do Carmo <laispc19@gmail.com>
>> Signed-off-by: Laís Pessine do Carmo <laispc19@gmail.com>
>> Signed-off-by: Arthur Moraes do Lago <arthurmoraeslago@gmail.com>
>>
>> Small fixes to vimc debayer mean window size patch
>> ---
>> Changes in V2:
>> - Updated documentation
>> - Added v4l2_subev_core_ops to solve errors in v4l2-ctl compliance test
>> - Changed control naming to follow English capitalization rules
>> - Rebased to Shuah Khan's newest patch series 171283
>> ("Collapse vimc single monolithic driver")
>> - Change maximum value for mean window size
>>
>> We did run streaming tests, the value of 1 for mean window size has a
>> nice side effect of not applying any filter, so we can see the original
>> pattern. The value of 99 for mean window size worked, but was very slow.
>> We wanted to find a way to show that higher values for mean window size
>> yielded very similar results to just blurring the image, but we could
>> not find any way to show this, so we just made the maximum value 25,
>> which runs faster, but is still probably a little high.
>> ---
>> Documentation/media/v4l-drivers/vimc.rst | 10 +--
>> drivers/media/platform/vimc/vimc-common.h | 1 +
>> drivers/media/platform/vimc/vimc-debayer.c | 89 +++++++++++++++++++---
>> 3 files changed, 79 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
>> index a582af0509ee..91c909904b87 100644
>> --- a/Documentation/media/v4l-drivers/vimc.rst
>> +++ b/Documentation/media/v4l-drivers/vimc.rst
>> @@ -80,9 +80,7 @@ vimc-capture:
>> Module options
>> ---------------
>> -Vimc has a few module parameters to configure the driver.
>> -
>> - param=value
>> +Vimc has a module parameters to configure the driver.
>
> parameter
>
>> * ``sca_mult=<unsigned int>``
>> @@ -91,12 +89,6 @@ Vimc has a few module parameters to configure the driver.
>> original one. Currently, only supports scaling up (the default value
>> is 3).
>> -* ``deb_mean_win_size=<unsigned int>``
>> -
>> - Window size to calculate the mean. Note: the window size needs to be an
>> - odd number, as the main pixel stays in the center of the window,
>> - otherwise the next odd number is considered (the default value is 3).
>> -
>> Source code documentation
>> -------------------------
>> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
>> index 236412ad7548..3a5102ddf794 100644
>> --- a/drivers/media/platform/vimc/vimc-common.h
>> +++ b/drivers/media/platform/vimc/vimc-common.h
>> @@ -19,6 +19,7 @@
>> #define VIMC_CID_VIMC_BASE (0x00f00000 | 0xf000)
>> #define VIMC_CID_VIMC_CLASS (0x00f00000 | 1)
>> #define VIMC_CID_TEST_PATTERN (VIMC_CID_VIMC_BASE + 0)
>> +#define VIMC_CID_MEAN_WIN_SIZE (VIMC_CID_VIMC_BASE + 1)
>> #define VIMC_FRAME_MAX_WIDTH 4096
>> #define VIMC_FRAME_MAX_HEIGHT 2160
>> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
>> index 37f3767db469..76df2ac110c0 100644
>> --- a/drivers/media/platform/vimc/vimc-debayer.c
>> +++ b/drivers/media/platform/vimc/vimc-debayer.c
>> @@ -11,17 +11,12 @@
>> #include <linux/platform_device.h>
>> #include <linux/vmalloc.h>
>> #include <linux/v4l2-mediabus.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-event.h>
>> #include <media/v4l2-subdev.h>
>> #include "vimc-common.h"
>> -static unsigned int deb_mean_win_size = 3;
>> -module_param(deb_mean_win_size, uint, 0000);
>> -MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n"
>> - "NOTE: the window size needs to be an odd number, as the main pixel "
>> - "stays in the center of the window, otherwise the next odd number "
>> - "is considered");
>> -
>> enum vimc_deb_rgb_colors {
>> VIMC_DEB_RED = 0,
>> VIMC_DEB_GREEN = 1,
>> @@ -46,6 +41,8 @@ struct vimc_deb_device {
>> u8 *src_frame;
>> const struct vimc_deb_pix_map *sink_pix_map;
>> unsigned int sink_bpp;
>> + unsigned int mean_win_size;
>> + struct v4l2_ctrl_handler hdl;
>> };
>> static const struct v4l2_mbus_framefmt sink_fmt_default = {
>> @@ -346,11 +343,18 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
>> return 0;
>> }
>> +static const struct v4l2_subdev_core_ops vimc_deb_core_ops = {
>> + .log_status = v4l2_ctrl_subdev_log_status,
>> + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
>> + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
>> +};
>> +
>> static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
>> .s_stream = vimc_deb_s_stream,
>> };
>> static const struct v4l2_subdev_ops vimc_deb_ops = {
>> + .core = &vimc_deb_core_ops,
>> .pad = &vimc_deb_pad_ops,
>> .video = &vimc_deb_video_ops,
>> };
>> @@ -384,7 +388,7 @@ static void vimc_deb_calc_rgb_sink(struct vimc_deb_device *vdeb,
>> * the top left corner of the mean window (considering the current
>> * pixel as the center)
>> */
>> - seek = deb_mean_win_size / 2;
>> + seek = vdeb->mean_win_size / 2;
>> /* Sum the values of the colors in the mean window */
>> @@ -474,6 +478,33 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
>> }
>> +static inline void vimc_deb_s_mean_win_size(struct vimc_deb_device *vdeb,
>> + unsigned int mean_win_size)
>> +{
>> + if (vdeb->mean_win_size == mean_win_size)
>> + return;
>> + vdeb->mean_win_size = mean_win_size;
>> +}
>> +
>> +static int vimc_deb_s_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> + struct vimc_deb_device *vdeb =
>> + container_of(ctrl->handler, struct vimc_deb_device, hdl);
>> +
>> + switch (ctrl->id) {
>> + case VIMC_CID_MEAN_WIN_SIZE:
>> + vimc_deb_s_mean_win_size(vdeb, ctrl->val);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_ctrl_ops vimc_deb_ctrl_ops = {
>> + .s_ctrl = vimc_deb_s_ctrl,
>> +};
>> +
>> static void vimc_deb_release(struct v4l2_subdev *sd)
>> {
>> struct vimc_deb_device *vdeb =
>> @@ -495,6 +526,24 @@ void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
>> vimc_ent_sd_unregister(ved, &vdeb->sd);
>> }
>> +static const struct v4l2_ctrl_config vimc_deb_ctrl_class = {
>> + .flags = V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY,
>> + .id = VIMC_CID_VIMC_CLASS,
>> + .name = "VIMC Controls",
>
> Make this VIMC Debayer Controls?
Actually, no. This control class is similar to the "Vivid Controls" in the vivid
driver: it collects all VIMC specific controls. It's only Debayer for now, but others
will be added here as well.
>
>> + .type = V4L2_CTRL_TYPE_CTRL_CLASS,
>> +};
>> +
>> +static const struct v4l2_ctrl_config vimc_deb_ctrl_mean_win_size = {
>> + .ops = &vimc_deb_ctrl_ops,
>> + .id = VIMC_CID_MEAN_WIN_SIZE,
>> + .name = "Mean Window Size",
But it would be useful to change this to:
"Debayer Mean Window Size"
Regards,
Hans
>> + .type = V4L2_CTRL_TYPE_INTEGER,
>> + .min = 1,
>> + .max = 25,
>> + .step = 2,
>> + .def = 3,
>> +};
>> +
>> struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
>> const char *vcfg_name)
>> {
>> @@ -507,6 +556,16 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
>> if (!vdeb)
>> return NULL;
>> + /* Create controls: */
>> + v4l2_ctrl_handler_init(&vdeb->hdl, 2);
>> + v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_class, NULL);
>> + v4l2_ctrl_new_custom(&vdeb->hdl, &vimc_deb_ctrl_mean_win_size, NULL);
>> + vdeb->sd.ctrl_handler = &vdeb->hdl;
>> + if (vdeb->hdl.error) {
>> + ret = vdeb->hdl.error;
>> + goto err_free_vdeb;
>> + }
>> +
>> /* Initialize ved and sd */
>> ret = vimc_ent_sd_register(&vdeb->ved, &vdeb->sd, v4l2_dev,
>> vcfg_name,
>> @@ -514,13 +573,12 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
>> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
>> MEDIA_PAD_FL_SOURCE},
>> &vimc_deb_int_ops, &vimc_deb_ops);
>> - if (ret) {
>> - kfree(vdeb);
>> - return NULL;
>> - }
>> + if (ret)
>> + goto err_free_hdl;
>> vdeb->ved.process_frame = vimc_deb_process_frame;
>> vdeb->dev = &vimc->pdev.dev;
>> + vdeb->mean_win_size = vimc_deb_ctrl_mean_win_size.def;
>> /* Initialize the frame format */
>> vdeb->sink_fmt = sink_fmt_default;
>> @@ -534,4 +592,11 @@ struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
>> vdeb->set_rgb_src = vimc_deb_set_rgb_mbus_fmt_rgb888_1x24;
>> return &vdeb->ved;
>> +
>> +err_free_hdl:
>> + v4l2_ctrl_handler_free(&vdeb->hdl);
>
> Free the handle from vimc_deb_release()
>
>> +err_free_vdeb:
>> + kfree(vdeb);
>> +
>> + return NULL;
>> }
>>
>
> thanks,
> -- Shuah
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-19 8:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-17 22:53 [PATCH v2] media: vimc: Implement debayer control for mean window size Arthur Moraes do Lago
2019-09-17 23:12 ` Shuah Khan
2019-09-19 8:20 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).