From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: hverkuil-cisco@xs4all.nl
Cc: linux-media@vger.kernel.org, Helen Koike <helen.koike@collabora.com>
Subject: Re: [PATCHv2 9/9] vimc: use new release op
Date: Tue, 5 Mar 2019 21:53:47 +0200 [thread overview]
Message-ID: <20190305195347.GI14928@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20190305095847.21428-10-hverkuil-cisco@xs4all.nl>
Hi Hans,
Thank you for the patch.
On Tue, Mar 05, 2019 at 10:58:47AM +0100, hverkuil-cisco@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>
> Use the new v4l2_subdev_internal_ops release op to free the
> subdev memory only once the last user closed the file handle.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> drivers/media/platform/vimc/vimc-common.c | 2 ++
> drivers/media/platform/vimc/vimc-common.h | 2 ++
> drivers/media/platform/vimc/vimc-debayer.c | 15 +++++++++++++--
> drivers/media/platform/vimc/vimc-scaler.c | 15 +++++++++++++--
> drivers/media/platform/vimc/vimc-sensor.c | 19 +++++++++++++++----
> 5 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> index c1a74bb2df58..0adbfd8fd26d 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -380,6 +380,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
> u32 function,
> u16 num_pads,
> const unsigned long *pads_flag,
> + const struct v4l2_subdev_internal_ops *sd_int_ops,
> const struct v4l2_subdev_ops *sd_ops)
> {
> int ret;
> @@ -394,6 +395,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
>
> /* Initialize the subdev */
> v4l2_subdev_init(sd, sd_ops);
> + sd->internal_ops = sd_int_ops;
> sd->entity.function = function;
> sd->entity.ops = &vimc_ent_sd_mops;
> sd->owner = THIS_MODULE;
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 84539430b5e7..07987eab988f 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -187,6 +187,7 @@ const struct vimc_pix_map *vimc_pix_map_by_pixelformat(u32 pixelformat);
> * @function: media entity function defined by MEDIA_ENT_F_* macros
> * @num_pads: number of pads to initialize
> * @pads_flag: flags to use in each pad
> + * @sd_int_ops: pointer to &struct v4l2_subdev_internal_ops.
Nitpicking, most of the lines here don't have a period at the end.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> * @sd_ops: pointer to &struct v4l2_subdev_ops.
> *
> * Helper function initialize and register the struct vimc_ent_device and struct
> @@ -199,6 +200,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
> u32 function,
> u16 num_pads,
> const unsigned long *pads_flag,
> + const struct v4l2_subdev_internal_ops *sd_int_ops,
> const struct v4l2_subdev_ops *sd_ops);
>
> /**
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 7d77c63b99d2..eaed4233ad1b 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -489,6 +489,18 @@ static void *vimc_deb_process_frame(struct vimc_ent_device *ved,
>
> }
>
> +static void vimc_deb_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_deb_device *vdeb =
> + container_of(sd, struct vimc_deb_device, sd);
> +
> + kfree(vdeb);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_deb_int_ops = {
> + .release = vimc_deb_release,
> +};
> +
> static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -497,7 +509,6 @@ static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
> ved);
>
> vimc_ent_sd_unregister(ved, &vdeb->sd);
> - kfree(vdeb);
> }
>
> static int vimc_deb_comp_bind(struct device *comp, struct device *master,
> @@ -519,7 +530,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master,
> MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2,
> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
> MEDIA_PAD_FL_SOURCE},
> - &vimc_deb_ops);
> + &vimc_deb_int_ops, &vimc_deb_ops);
> if (ret) {
> kfree(vdeb);
> return ret;
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index 39b2a73dfcc1..2028afa4ef7a 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -348,6 +348,18 @@ static void *vimc_sca_process_frame(struct vimc_ent_device *ved,
> return vsca->src_frame;
> };
>
> +static void vimc_sca_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_sca_device *vsca =
> + container_of(sd, struct vimc_sca_device, sd);
> +
> + kfree(vsca);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_sca_int_ops = {
> + .release = vimc_sca_release,
> +};
> +
> static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -356,7 +368,6 @@ static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
> ved);
>
> vimc_ent_sd_unregister(ved, &vsca->sd);
> - kfree(vsca);
> }
>
>
> @@ -379,7 +390,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct device *master,
> MEDIA_ENT_F_PROC_VIDEO_SCALER, 2,
> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
> MEDIA_PAD_FL_SOURCE},
> - &vimc_sca_ops);
> + &vimc_sca_int_ops, &vimc_sca_ops);
> if (ret) {
> kfree(vsca);
> return ret;
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 59195f262623..d7891d3bbeaa 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -301,6 +301,20 @@ static const struct v4l2_ctrl_ops vimc_sen_ctrl_ops = {
> .s_ctrl = vimc_sen_s_ctrl,
> };
>
> +static void vimc_sen_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_sen_device *vsen =
> + container_of(sd, struct vimc_sen_device, sd);
> +
> + v4l2_ctrl_handler_free(&vsen->hdl);
> + tpg_free(&vsen->tpg);
> + kfree(vsen);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_sen_int_ops = {
> + .release = vimc_sen_release,
> +};
> +
> static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -309,9 +323,6 @@ static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
> container_of(ved, struct vimc_sen_device, ved);
>
> vimc_ent_sd_unregister(ved, &vsen->sd);
> - v4l2_ctrl_handler_free(&vsen->hdl);
> - tpg_free(&vsen->tpg);
> - kfree(vsen);
> }
>
> /* Image Processing Controls */
> @@ -371,7 +382,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master,
> pdata->entity_name,
> MEDIA_ENT_F_CAM_SENSOR, 1,
> (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
> - &vimc_sen_ops);
> + &vimc_sen_int_ops, &vimc_sen_ops);
> if (ret)
> goto err_free_hdl;
>
--
Regards,
Laurent Pinchart
prev parent reply other threads:[~2019-03-05 19:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-05 9:58 [PATCHv2 0/9] Various core and virtual driver fixes hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 1/9] cec: fill in cec chardev kobject to ease debugging hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 2/9] media-devnode: fill in media " hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 3/9] vivid: use vzalloc for dev->bitmap_out hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 4/9] media-entity: set ent_enum->bmap to NULL after freeing it hverkuil-cisco
2019-03-05 19:39 ` Laurent Pinchart
2019-03-07 9:23 ` Hans Verkuil
2019-03-08 11:26 ` Laurent Pinchart
2019-03-08 13:59 ` Hans Verkuil
2019-03-05 9:58 ` [PATCHv2 5/9] vim2m: replace devm_kzalloc by kzalloc hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 6/9] v4l2-subdev: add release() internal op hverkuil-cisco
2019-03-05 19:46 ` Laurent Pinchart
2019-03-07 8:54 ` Hans Verkuil
2019-03-07 9:05 ` Laurent Pinchart
2019-03-05 9:58 ` [PATCHv2 7/9] v4l2-subdev: handle module refcounting here hverkuil-cisco
2019-03-05 19:52 ` Laurent Pinchart
2019-03-07 8:57 ` Hans Verkuil
2019-03-07 16:06 ` Sakari Ailus
2019-03-05 9:58 ` [PATCHv2 8/9] vimc: free vimc_cap_device when the last user disappears hverkuil-cisco
2019-03-05 9:58 ` [PATCHv2 9/9] vimc: use new release op hverkuil-cisco
2019-03-05 19:53 ` Laurent Pinchart [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190305195347.GI14928@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=helen.koike@collabora.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox