From f13d5f3619599d257b4bcb941245085f5d118af8 Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Tue, 17 Sep 2019 13:35:08 -0300 Subject: media: vimc: Collapse component structure into a single monolithic driver vimc uses Component API to split the driver into functional components. The real hardware resembles a monolith structure than component and component structure added a level of complexity making it hard to maintain without adding any real benefit. The sensor is one vimc component that would makes sense to be a separate module to closely align with the real hardware. It would be easier to collapse vimc into single monolithic driver first and then split the sensor off as a separate module. Collapse it into a single monolithic driver removing the Component API. This patch removes the component API and makes minimal changes to the code base preserving the functional division of the code structure. Preserving the functional structure allows us to split the sensor off as a separate module in the future. Major design elements in this change are: - Use existing struct vimc_ent_config and struct vimc_pipeline_config to drive the initialization of the functional components. - Make vimc_device and vimc_ent_config global by moving them to vimc-common.h - Add two new hooks add and rm to initialize and register, unregister and free subdevs. - All component API is now gone and bind and unbind hooks are modified to do "add" and "rm" with minimal changes to just add and rm subdevs. - vimc-core's bind and unbind are now register and unregister. - Add a new field to vimc_device structure for saving the pointers to struct vimc_ent_device(s) subdevs create in their "add" hooks. These get used to create links and removing the subdevs. vimc-core allocates this array which sized to number of entries in the topology defined in the vimc_pipeline_config structure. - vimc-core invokes "add" hooks from its vimc_register_devices(). The "add" hooks remain the same and register subdevs. They don't create platform devices of their own and use vimc's pdev.dev as their reference device. Each "add" hook returns pointer to its struct vimc_ent_device. This is saved in the vimc_device ent_devs array. - vimc-core invokes "rm" hooks from its unregister to unregister subdevs and cleanup. - vimc-core invokes "add" and "rm" hooks with pointer to struct vimc_device and the corresponding vimc_ent_device saved in the ent_devs. The following configure and stream test works on all devices. media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]' media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]' v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440 v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81 v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video1 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2 v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video3 Signed-off-by: Shuah Khan Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/vimc/Makefile | 7 +- drivers/media/platform/vimc/vimc-capture.c | 81 ++--------- drivers/media/platform/vimc/vimc-common.h | 54 +++++++ drivers/media/platform/vimc/vimc-core.c | 216 +++++++++++----------------- drivers/media/platform/vimc/vimc-debayer.c | 73 ++-------- drivers/media/platform/vimc/vimc-scaler.c | 75 ++-------- drivers/media/platform/vimc/vimc-sensor.c | 73 ++-------- drivers/media/platform/vimc/vimc-streamer.c | 1 - 8 files changed, 195 insertions(+), 385 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/vimc/Makefile b/drivers/media/platform/vimc/Makefile index 96d06f030c31..a53b2b532e9f 100644 --- a/drivers/media/platform/vimc/Makefile +++ b/drivers/media/platform/vimc/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 -vimc-y := vimc-core.o vimc-common.o vimc-streamer.o +vimc-y := vimc-core.o vimc-common.o vimc-streamer.o vimc-capture.o \ + vimc-debayer.o vimc-scaler.o vimc-sensor.o + +obj-$(CONFIG_VIDEO_VIMC) += vimc.o -obj-$(CONFIG_VIDEO_VIMC) += vimc.o vimc-capture.o vimc-debayer.o \ - vimc-scaler.o vimc-sensor.o diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c index 1d56b91830ba..602f80323031 100644 --- a/drivers/media/platform/vimc/vimc-capture.c +++ b/drivers/media/platform/vimc/vimc-capture.c @@ -5,10 +5,6 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include -#include -#include -#include #include #include #include @@ -16,8 +12,6 @@ #include "vimc-common.h" #include "vimc-streamer.h" -#define VIMC_CAP_DRV_NAME "vimc-capture" - struct vimc_cap_device { struct vimc_ent_device ved; struct video_device vdev; @@ -340,13 +334,11 @@ static void vimc_cap_release(struct video_device *vdev) kfree(vcap); } -static void vimc_cap_comp_unbind(struct device *comp, struct device *master, - void *master_data) +void vimc_cap_rm(struct vimc_device *vimc, struct vimc_ent_device *ved) { - struct vimc_ent_device *ved = dev_get_drvdata(comp); - struct vimc_cap_device *vcap = container_of(ved, struct vimc_cap_device, - ved); + struct vimc_cap_device *vcap; + vcap = container_of(ved, struct vimc_cap_device, ved); vb2_queue_release(&vcap->queue); media_entity_cleanup(ved->ent); video_unregister_device(&vcap->vdev); @@ -391,11 +383,10 @@ static void *vimc_cap_process_frame(struct vimc_ent_device *ved, return NULL; } -static int vimc_cap_comp_bind(struct device *comp, struct device *master, - void *master_data) +struct vimc_ent_device *vimc_cap_add(struct vimc_device *vimc, + const char *vcfg_name) { - struct v4l2_device *v4l2_dev = master_data; - struct vimc_platform_data *pdata = comp->platform_data; + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev; const struct vimc_pix_map *vpix; struct vimc_cap_device *vcap; struct video_device *vdev; @@ -405,7 +396,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master, /* Allocate the vimc_cap_device struct */ vcap = kzalloc(sizeof(*vcap), GFP_KERNEL); if (!vcap) - return -ENOMEM; + return NULL; /* Allocate the pads */ vcap->ved.pads = @@ -416,7 +407,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master, } /* Initialize the media entity */ - vcap->vdev.entity.name = pdata->entity_name; + vcap->vdev.entity.name = vcfg_name; vcap->vdev.entity.function = MEDIA_ENT_F_IO_V4L; ret = media_entity_pads_init(&vcap->vdev.entity, 1, vcap->ved.pads); @@ -440,8 +431,8 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master, ret = vb2_queue_init(q); if (ret) { - dev_err(comp, "%s: vb2 queue init failed (err=%d)\n", - pdata->entity_name, ret); + dev_err(&vimc->pdev.dev, "%s: vb2 queue init failed (err=%d)\n", + vcfg_name, ret); goto err_clean_m_ent; } @@ -460,8 +451,7 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master, vcap->ved.ent = &vcap->vdev.entity; vcap->ved.process_frame = vimc_cap_process_frame; vcap->ved.vdev_get_format = vimc_cap_get_format; - dev_set_drvdata(comp, &vcap->ved); - vcap->dev = comp; + vcap->dev = &vimc->pdev.dev; /* Initialize the video_device struct */ vdev = &vcap->vdev; @@ -474,18 +464,18 @@ static int vimc_cap_comp_bind(struct device *comp, struct device *master, vdev->queue = q; vdev->v4l2_dev = v4l2_dev; vdev->vfl_dir = VFL_DIR_RX; - strscpy(vdev->name, pdata->entity_name, sizeof(vdev->name)); + strscpy(vdev->name, vcfg_name, sizeof(vdev->name)); video_set_drvdata(vdev, &vcap->ved); /* Register the video_device with the v4l2 and the media framework */ ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1); if (ret) { - dev_err(comp, "%s: video register failed (err=%d)\n", + dev_err(&vimc->pdev.dev, "%s: video register failed (err=%d)\n", vcap->vdev.name, ret); goto err_release_queue; } - return 0; + return &vcap->ved; err_release_queue: vb2_queue_release(q); @@ -496,46 +486,5 @@ err_clean_pads: err_free_vcap: kfree(vcap); - return ret; -} - -static const struct component_ops vimc_cap_comp_ops = { - .bind = vimc_cap_comp_bind, - .unbind = vimc_cap_comp_unbind, -}; - -static int vimc_cap_probe(struct platform_device *pdev) -{ - return component_add(&pdev->dev, &vimc_cap_comp_ops); -} - -static int vimc_cap_remove(struct platform_device *pdev) -{ - component_del(&pdev->dev, &vimc_cap_comp_ops); - - return 0; + return NULL; } - -static const struct platform_device_id vimc_cap_driver_ids[] = { - { - .name = VIMC_CAP_DRV_NAME, - }, - { } -}; - -static struct platform_driver vimc_cap_pdrv = { - .probe = vimc_cap_probe, - .remove = vimc_cap_remove, - .id_table = vimc_cap_driver_ids, - .driver = { - .name = VIMC_CAP_DRV_NAME, - }, -}; - -module_platform_driver(vimc_cap_pdrv); - -MODULE_DEVICE_TABLE(platform, vimc_cap_driver_ids); - -MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Capture"); -MODULE_AUTHOR("Helen Mae Koike Fornazier "); -MODULE_LICENSE("GPL"); diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h index 9c2e0e216c6b..d7aaf31175bc 100644 --- a/drivers/media/platform/vimc/vimc-common.h +++ b/drivers/media/platform/vimc/vimc-common.h @@ -8,6 +8,7 @@ #ifndef _VIMC_COMMON_H_ #define _VIMC_COMMON_H_ +#include #include #include #include @@ -111,6 +112,59 @@ struct vimc_ent_device { struct v4l2_pix_format *fmt); }; +/** + * struct vimc_device - main device for vimc driver + * + * @pdev pointer to the platform device + * @pipe_cfg pointer to the vimc pipeline configuration structure + * @ent_devs array of vimc_ent_device pointers + * @mdev the associated media_device parent + * @v4l2_dev Internal v4l2 parent device + */ +struct vimc_device { + struct platform_device pdev; + const struct vimc_pipeline_config *pipe_cfg; + struct vimc_ent_device **ent_devs; + struct media_device mdev; + struct v4l2_device v4l2_dev; +}; + +/** + * struct vimc_ent_config Structure which describes individual + * configuration for each entity + * + * @name entity name + * @ved pointer to vimc_ent_device (a node in the + * topology) + * @add subdev add hook - initializes and registers + * subdev called from vimc-core + * @rm subdev rm hook - unregisters and frees + * subdev called from vimc-core + */ +struct vimc_ent_config { + const char *name; + struct vimc_ent_device *(*add)(struct vimc_device *vimc, + const char *vcfg_name); + void (*rm)(struct vimc_device *vimc, struct vimc_ent_device *ved); +}; + +/* prototypes for vimc_ent_config add and rm hooks */ +struct vimc_ent_device *vimc_cap_add(struct vimc_device *vimc, + const char *vcfg_name); +void vimc_cap_rm(struct vimc_device *vimc, struct vimc_ent_device *ved); + +struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc, + const char *vcfg_name); +void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved); + +struct vimc_ent_device *vimc_sca_add(struct vimc_device *vimc, + const char *vcfg_name); +void vimc_sca_rm(struct vimc_device *vimc, struct vimc_ent_device *ved); + +struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc, + const char *vcfg_name); +void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved); + /** * vimc_pads_init - initialize pads * diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c index 571c55aa0e16..6e3e5c91ae39 100644 --- a/drivers/media/platform/vimc/vimc-core.c +++ b/drivers/media/platform/vimc/vimc-core.c @@ -5,7 +5,6 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include #include #include #include @@ -24,29 +23,6 @@ .flags = link_flags, \ } -struct vimc_device { - /* The platform device */ - struct platform_device pdev; - - /* The pipeline configuration */ - const struct vimc_pipeline_config *pipe_cfg; - - /* The Associated media_device parent */ - struct media_device mdev; - - /* Internal v4l2 parent device*/ - struct v4l2_device v4l2_dev; - - /* Subdevices */ - struct platform_device **subdevs; -}; - -/* Structure which describes individual configuration for each entity */ -struct vimc_ent_config { - const char *name; - const char *drv; -}; - /* Structure which describes links between entities */ struct vimc_ent_link { unsigned int src_ent; @@ -68,43 +44,52 @@ struct vimc_pipeline_config { * Topology Configuration */ -static const struct vimc_ent_config ent_config[] = { +static struct vimc_ent_config ent_config[] = { { .name = "Sensor A", - .drv = "vimc-sensor", + .add = vimc_sen_add, + .rm = vimc_sen_rm, }, { .name = "Sensor B", - .drv = "vimc-sensor", + .add = vimc_sen_add, + .rm = vimc_sen_rm, }, { .name = "Debayer A", - .drv = "vimc-debayer", + .add = vimc_deb_add, + .rm = vimc_deb_rm, }, { .name = "Debayer B", - .drv = "vimc-debayer", + .add = vimc_deb_add, + .rm = vimc_deb_rm, }, { .name = "Raw Capture 0", - .drv = "vimc-capture", + .add = vimc_cap_add, + .rm = vimc_cap_rm, }, { .name = "Raw Capture 1", - .drv = "vimc-capture", + .add = vimc_cap_add, + .rm = vimc_cap_rm, }, { - .name = "RGB/YUV Input", /* TODO: change this to vimc-input when it is implemented */ - .drv = "vimc-sensor", + .name = "RGB/YUV Input", + .add = vimc_sen_add, + .rm = vimc_sen_rm, }, { .name = "Scaler", - .drv = "vimc-scaler", + .add = vimc_sca_add, + .rm = vimc_sca_rm, }, { .name = "RGB/YUV Capture", - .drv = "vimc-capture", + .add = vimc_cap_add, + .rm = vimc_cap_rm, }, }; @@ -127,7 +112,7 @@ static const struct vimc_ent_link ent_links[] = { VIMC_ENT_LINK(7, 1, 8, 0, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE), }; -static const struct vimc_pipeline_config pipe_cfg = { +static struct vimc_pipeline_config pipe_cfg = { .ents = ent_config, .num_ents = ARRAY_SIZE(ent_config), .links = ent_links, @@ -136,6 +121,14 @@ static const struct vimc_pipeline_config pipe_cfg = { /* -------------------------------------------------------------------------- */ +static void vimc_rm_links(struct vimc_device *vimc) +{ + unsigned int i; + + for (i = 0; i < vimc->pipe_cfg->num_ents; i++) + media_entity_remove_links(vimc->ent_devs[i]->ent); +} + static int vimc_create_links(struct vimc_device *vimc) { unsigned int i; @@ -144,32 +137,58 @@ static int vimc_create_links(struct vimc_device *vimc) /* Initialize the links between entities */ for (i = 0; i < vimc->pipe_cfg->num_links; i++) { const struct vimc_ent_link *link = &vimc->pipe_cfg->links[i]; - /* - * TODO: Check another way of retrieving ved struct without - * relying on platform_get_drvdata - */ + struct vimc_ent_device *ved_src = - platform_get_drvdata(vimc->subdevs[link->src_ent]); + vimc->ent_devs[link->src_ent]; struct vimc_ent_device *ved_sink = - platform_get_drvdata(vimc->subdevs[link->sink_ent]); + vimc->ent_devs[link->sink_ent]; ret = media_create_pad_link(ved_src->ent, link->src_pad, ved_sink->ent, link->sink_pad, link->flags); if (ret) - return ret; + goto err_rm_links; } return 0; + +err_rm_links: + vimc_rm_links(vimc); + return ret; } -static int vimc_comp_bind(struct device *master) +static int vimc_add_subdevs(struct vimc_device *vimc) { - struct vimc_device *vimc = container_of(to_platform_device(master), - struct vimc_device, pdev); - int ret; + unsigned int i; + struct vimc_ent_device *ved; + + for (i = 0; i < vimc->pipe_cfg->num_ents; i++) { + dev_dbg(&vimc->pdev.dev, "new entity for %s\n", + vimc->pipe_cfg->ents[i].name); + ved = vimc->pipe_cfg->ents[i].add(vimc, + vimc->pipe_cfg->ents[i].name); + if (!ved) { + dev_err(&vimc->pdev.dev, "add new entity for %s\n", + vimc->pipe_cfg->ents[i].name); + return -EINVAL; + } + vimc->ent_devs[i] = ved; + } + return 0; +} + +static void vimc_rm_subdevs(struct vimc_device *vimc) +{ + unsigned int i; - dev_dbg(master, "bind"); + for (i = 0; i < vimc->pipe_cfg->num_ents; i++) + if (vimc->ent_devs[i]) + vimc->pipe_cfg->ents[i].rm(vimc, vimc->ent_devs[i]); +} + +static int vimc_register_devices(struct vimc_device *vimc) +{ + int ret; /* Register the v4l2 struct */ ret = v4l2_device_register(vimc->mdev.dev, &vimc->v4l2_dev); @@ -179,22 +198,30 @@ static int vimc_comp_bind(struct device *master) return ret; } - /* Bind subdevices */ - ret = component_bind_all(master, &vimc->v4l2_dev); - if (ret) + /* allocate ent_devs */ + vimc->ent_devs = kmalloc_array(vimc->pipe_cfg->num_ents, + sizeof(*vimc->ent_devs), + GFP_KERNEL); + if (!vimc->ent_devs) goto err_v4l2_unregister; + /* Invoke entity config hooks to initialize and register subdevs */ + ret = vimc_add_subdevs(vimc); + if (ret) + /* remove sundevs that got added */ + goto err_rm_subdevs; + /* Initialize links */ ret = vimc_create_links(vimc); if (ret) - goto err_comp_unbind_all; + goto err_rm_subdevs; /* Register the media device */ ret = media_device_register(&vimc->mdev); if (ret) { dev_err(vimc->mdev.dev, "media device register failed (err=%d)\n", ret); - goto err_comp_unbind_all; + goto err_rm_subdevs; } /* Expose all subdev's nodes*/ @@ -211,98 +238,32 @@ static int vimc_comp_bind(struct device *master) err_mdev_unregister: media_device_unregister(&vimc->mdev); media_device_cleanup(&vimc->mdev); -err_comp_unbind_all: - component_unbind_all(master, NULL); +err_rm_subdevs: + vimc_rm_subdevs(vimc); + kfree(vimc->ent_devs); err_v4l2_unregister: v4l2_device_unregister(&vimc->v4l2_dev); return ret; } -static void vimc_comp_unbind(struct device *master) +static void vimc_unregister(struct vimc_device *vimc) { - struct vimc_device *vimc = container_of(to_platform_device(master), - struct vimc_device, pdev); - - dev_dbg(master, "unbind"); - media_device_unregister(&vimc->mdev); media_device_cleanup(&vimc->mdev); - component_unbind_all(master, NULL); v4l2_device_unregister(&vimc->v4l2_dev); + kfree(vimc->ent_devs); } -static int vimc_comp_compare(struct device *comp, void *data) -{ - return comp == data; -} - -static struct component_match *vimc_add_subdevs(struct vimc_device *vimc) -{ - struct component_match *match = NULL; - struct vimc_platform_data pdata; - int i; - - for (i = 0; i < vimc->pipe_cfg->num_ents; i++) { - dev_dbg(&vimc->pdev.dev, "new pdev for %s\n", - vimc->pipe_cfg->ents[i].drv); - - strscpy(pdata.entity_name, vimc->pipe_cfg->ents[i].name, - sizeof(pdata.entity_name)); - - vimc->subdevs[i] = platform_device_register_data(&vimc->pdev.dev, - vimc->pipe_cfg->ents[i].drv, - PLATFORM_DEVID_AUTO, - &pdata, - sizeof(pdata)); - if (IS_ERR(vimc->subdevs[i])) { - match = ERR_CAST(vimc->subdevs[i]); - while (--i >= 0) - platform_device_unregister(vimc->subdevs[i]); - - return match; - } - - component_match_add(&vimc->pdev.dev, &match, vimc_comp_compare, - &vimc->subdevs[i]->dev); - } - - return match; -} - -static void vimc_rm_subdevs(struct vimc_device *vimc) -{ - unsigned int i; - - for (i = 0; i < vimc->pipe_cfg->num_ents; i++) - platform_device_unregister(vimc->subdevs[i]); -} - -static const struct component_master_ops vimc_comp_ops = { - .bind = vimc_comp_bind, - .unbind = vimc_comp_unbind, -}; - static int vimc_probe(struct platform_device *pdev) { struct vimc_device *vimc = container_of(pdev, struct vimc_device, pdev); - struct component_match *match = NULL; int ret; dev_dbg(&pdev->dev, "probe"); memset(&vimc->mdev, 0, sizeof(vimc->mdev)); - /* Create platform_device for each entity in the topology*/ - vimc->subdevs = devm_kcalloc(&vimc->pdev.dev, vimc->pipe_cfg->num_ents, - sizeof(*vimc->subdevs), GFP_KERNEL); - if (!vimc->subdevs) - return -ENOMEM; - - match = vimc_add_subdevs(vimc); - if (IS_ERR(match)) - return PTR_ERR(match); - /* Link the media device within the v4l2_device */ vimc->v4l2_dev.mdev = &vimc->mdev; @@ -314,12 +275,9 @@ static int vimc_probe(struct platform_device *pdev) vimc->mdev.dev = &pdev->dev; media_device_init(&vimc->mdev); - /* Add self to the component system */ - ret = component_master_add_with_match(&pdev->dev, &vimc_comp_ops, - match); + ret = vimc_register_devices(vimc); if (ret) { media_device_cleanup(&vimc->mdev); - vimc_rm_subdevs(vimc); return ret; } @@ -332,8 +290,8 @@ static int vimc_remove(struct platform_device *pdev) dev_dbg(&pdev->dev, "remove"); - component_master_del(&pdev->dev, &vimc_comp_ops); vimc_rm_subdevs(vimc); + vimc_unregister(vimc); return 0; } diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index b72b8385067b..2c291447807e 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -5,9 +5,7 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include -#include -#include +#include #include #include #include @@ -15,8 +13,6 @@ #include "vimc-common.h" -#define VIMC_DEB_DRV_NAME "vimc-debayer" - 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" @@ -491,44 +487,40 @@ 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) +void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved) { - struct vimc_ent_device *ved = dev_get_drvdata(comp); - struct vimc_deb_device *vdeb = container_of(ved, struct vimc_deb_device, - ved); + struct vimc_deb_device *vdeb; + vdeb = container_of(ved, struct vimc_deb_device, ved); vimc_ent_sd_unregister(ved, &vdeb->sd); } -static int vimc_deb_comp_bind(struct device *comp, struct device *master, - void *master_data) +struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc, + const char *vcfg_name) { - struct v4l2_device *v4l2_dev = master_data; - struct vimc_platform_data *pdata = comp->platform_data; + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev; struct vimc_deb_device *vdeb; int ret; /* Allocate the vdeb struct */ vdeb = kzalloc(sizeof(*vdeb), GFP_KERNEL); if (!vdeb) - return -ENOMEM; + return NULL; /* Initialize ved and sd */ ret = vimc_ent_sd_register(&vdeb->ved, &vdeb->sd, v4l2_dev, - pdata->entity_name, + vcfg_name, MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2, (const unsigned long[2]) {MEDIA_PAD_FL_SINK, MEDIA_PAD_FL_SOURCE}, &vimc_deb_int_ops, &vimc_deb_ops); if (ret) { kfree(vdeb); - return ret; + return NULL; } vdeb->ved.process_frame = vimc_deb_process_frame; - dev_set_drvdata(comp, &vdeb->ved); - vdeb->dev = comp; + vdeb->dev = &vimc->pdev.dev; /* Initialize the frame format */ vdeb->sink_fmt = sink_fmt_default; @@ -541,46 +533,5 @@ static int vimc_deb_comp_bind(struct device *comp, struct device *master, vdeb->src_code = MEDIA_BUS_FMT_RGB888_1X24; vdeb->set_rgb_src = vimc_deb_set_rgb_mbus_fmt_rgb888_1x24; - return 0; + return &vdeb->ved; } - -static const struct component_ops vimc_deb_comp_ops = { - .bind = vimc_deb_comp_bind, - .unbind = vimc_deb_comp_unbind, -}; - -static int vimc_deb_probe(struct platform_device *pdev) -{ - return component_add(&pdev->dev, &vimc_deb_comp_ops); -} - -static int vimc_deb_remove(struct platform_device *pdev) -{ - component_del(&pdev->dev, &vimc_deb_comp_ops); - - return 0; -} - -static const struct platform_device_id vimc_deb_driver_ids[] = { - { - .name = VIMC_DEB_DRV_NAME, - }, - { } -}; - -static struct platform_driver vimc_deb_pdrv = { - .probe = vimc_deb_probe, - .remove = vimc_deb_remove, - .id_table = vimc_deb_driver_ids, - .driver = { - .name = VIMC_DEB_DRV_NAME, - }, -}; - -module_platform_driver(vimc_deb_pdrv); - -MODULE_DEVICE_TABLE(platform, vimc_deb_driver_ids); - -MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Debayer"); -MODULE_AUTHOR("Helen Mae Koike Fornazier "); -MODULE_LICENSE("GPL"); diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index 49ab8d9dd9c9..f72200de2535 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -5,18 +5,13 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include -#include -#include -#include +#include #include #include #include #include "vimc-common.h" -#define VIMC_SCA_DRV_NAME "vimc-scaler" - static unsigned int sca_mult = 3; module_param(sca_mult, uint, 0000); MODULE_PARM_DESC(sca_mult, " the image size multiplier"); @@ -350,89 +345,43 @@ 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) +void vimc_sca_rm(struct vimc_device *vimc, struct vimc_ent_device *ved) { - struct vimc_ent_device *ved = dev_get_drvdata(comp); - struct vimc_sca_device *vsca = container_of(ved, struct vimc_sca_device, - ved); + struct vimc_sca_device *vsca; + vsca = container_of(ved, struct vimc_sca_device, ved); vimc_ent_sd_unregister(ved, &vsca->sd); } - -static int vimc_sca_comp_bind(struct device *comp, struct device *master, - void *master_data) +struct vimc_ent_device *vimc_sca_add(struct vimc_device *vimc, + const char *vcfg_name) { - struct v4l2_device *v4l2_dev = master_data; - struct vimc_platform_data *pdata = comp->platform_data; + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev; struct vimc_sca_device *vsca; int ret; /* Allocate the vsca struct */ vsca = kzalloc(sizeof(*vsca), GFP_KERNEL); if (!vsca) - return -ENOMEM; + return NULL; /* Initialize ved and sd */ ret = vimc_ent_sd_register(&vsca->ved, &vsca->sd, v4l2_dev, - pdata->entity_name, + vcfg_name, MEDIA_ENT_F_PROC_VIDEO_SCALER, 2, (const unsigned long[2]) {MEDIA_PAD_FL_SINK, MEDIA_PAD_FL_SOURCE}, &vimc_sca_int_ops, &vimc_sca_ops); if (ret) { kfree(vsca); - return ret; + return NULL; } vsca->ved.process_frame = vimc_sca_process_frame; - dev_set_drvdata(comp, &vsca->ved); - vsca->dev = comp; + vsca->dev = &vimc->pdev.dev; /* Initialize the frame format */ vsca->sink_fmt = sink_fmt_default; - return 0; -} - -static const struct component_ops vimc_sca_comp_ops = { - .bind = vimc_sca_comp_bind, - .unbind = vimc_sca_comp_unbind, -}; - -static int vimc_sca_probe(struct platform_device *pdev) -{ - return component_add(&pdev->dev, &vimc_sca_comp_ops); + return &vsca->ved; } - -static int vimc_sca_remove(struct platform_device *pdev) -{ - component_del(&pdev->dev, &vimc_sca_comp_ops); - - return 0; -} - -static const struct platform_device_id vimc_sca_driver_ids[] = { - { - .name = VIMC_SCA_DRV_NAME, - }, - { } -}; - -static struct platform_driver vimc_sca_pdrv = { - .probe = vimc_sca_probe, - .remove = vimc_sca_remove, - .id_table = vimc_sca_driver_ids, - .driver = { - .name = VIMC_SCA_DRV_NAME, - }, -}; - -module_platform_driver(vimc_sca_pdrv); - -MODULE_DEVICE_TABLE(platform, vimc_sca_driver_ids); - -MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Scaler"); -MODULE_AUTHOR("Helen Mae Koike Fornazier "); -MODULE_LICENSE("GPL"); diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c index 6c53b9fc1617..1f15637ca8bb 100644 --- a/drivers/media/platform/vimc/vimc-sensor.c +++ b/drivers/media/platform/vimc/vimc-sensor.c @@ -5,10 +5,6 @@ * Copyright (C) 2015-2017 Helen Koike */ -#include -#include -#include -#include #include #include #include @@ -18,8 +14,6 @@ #include "vimc-common.h" -#define VIMC_SEN_DRV_NAME "vimc-sensor" - struct vimc_sen_device { struct vimc_ent_device ved; struct v4l2_subdev sd; @@ -304,13 +298,11 @@ 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) +void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved) { - struct vimc_ent_device *ved = dev_get_drvdata(comp); - struct vimc_sen_device *vsen = - container_of(ved, struct vimc_sen_device, ved); + struct vimc_sen_device *vsen; + vsen = container_of(ved, struct vimc_sen_device, ved); vimc_ent_sd_unregister(ved, &vsen->sd); } @@ -331,18 +323,17 @@ static const struct v4l2_ctrl_config vimc_sen_ctrl_test_pattern = { .qmenu = tpg_pattern_strings, }; -static int vimc_sen_comp_bind(struct device *comp, struct device *master, - void *master_data) +struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc, + const char *vcfg_name) { - struct v4l2_device *v4l2_dev = master_data; - struct vimc_platform_data *pdata = comp->platform_data; + struct v4l2_device *v4l2_dev = &vimc->v4l2_dev; struct vimc_sen_device *vsen; int ret; /* Allocate the vsen struct */ vsen = kzalloc(sizeof(*vsen), GFP_KERNEL); if (!vsen) - return -ENOMEM; + return NULL; v4l2_ctrl_handler_init(&vsen->hdl, 4); @@ -368,7 +359,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master, /* Initialize ved and sd */ ret = vimc_ent_sd_register(&vsen->ved, &vsen->sd, v4l2_dev, - pdata->entity_name, + vcfg_name, MEDIA_ENT_F_CAM_SENSOR, 1, (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE}, &vimc_sen_int_ops, &vimc_sen_ops); @@ -376,8 +367,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master, goto err_free_hdl; vsen->ved.process_frame = vimc_sen_process_frame; - dev_set_drvdata(comp, &vsen->ved); - vsen->dev = comp; + vsen->dev = &vimc->pdev.dev; /* Initialize the frame format */ vsen->mbus_format = fmt_default; @@ -389,7 +379,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct device *master, if (ret) goto err_unregister_ent_sd; - return 0; + return &vsen->ved; err_unregister_ent_sd: vimc_ent_sd_unregister(&vsen->ved, &vsen->sd); @@ -398,46 +388,5 @@ err_free_hdl: err_free_vsen: kfree(vsen); - return ret; + return NULL; } - -static const struct component_ops vimc_sen_comp_ops = { - .bind = vimc_sen_comp_bind, - .unbind = vimc_sen_comp_unbind, -}; - -static int vimc_sen_probe(struct platform_device *pdev) -{ - return component_add(&pdev->dev, &vimc_sen_comp_ops); -} - -static int vimc_sen_remove(struct platform_device *pdev) -{ - component_del(&pdev->dev, &vimc_sen_comp_ops); - - return 0; -} - -static const struct platform_device_id vimc_sen_driver_ids[] = { - { - .name = VIMC_SEN_DRV_NAME, - }, - { } -}; - -static struct platform_driver vimc_sen_pdrv = { - .probe = vimc_sen_probe, - .remove = vimc_sen_remove, - .id_table = vimc_sen_driver_ids, - .driver = { - .name = VIMC_SEN_DRV_NAME, - }, -}; - -module_platform_driver(vimc_sen_pdrv); - -MODULE_DEVICE_TABLE(platform, vimc_sen_driver_ids); - -MODULE_DESCRIPTION("Virtual Media Controller Driver (VIMC) Sensor"); -MODULE_AUTHOR("Helen Mae Koike Fornazier "); -MODULE_LICENSE("GPL"); diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c index 048d770e498b..faa2879c25df 100644 --- a/drivers/media/platform/vimc/vimc-streamer.c +++ b/drivers/media/platform/vimc/vimc-streamer.c @@ -7,7 +7,6 @@ */ #include -#include #include #include -- cgit v1.2.3 From d7fb5c361c2a2666d20e044206e1756bc8e87df2 Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Tue, 17 Sep 2019 13:35:09 -0300 Subject: media: vimc: Fix gpf in rmmod path when stream is active If vimc module is removed while streaming is in progress, sensor subdev unregister runs into general protection fault when it tries to unregister media entities. This is a common subdev problem related to releasing pads from v4l2_device_unregister_subdev() before calling unregister. Unregister references pads during unregistering subdev. The sd release handler is the right place for releasing all sd resources including pads. The release handlers currently release all resources except the pads. Fix v4l2_device_unregister_subdev() not release pads and release pads from the sd_int_op release handlers. kernel: [ 4136.715839] general protection fault: 0000 [#1] SMP PTI kernel: [ 4136.715847] CPU: 2 PID: 1972 Comm: bash Not tainted 5.3.0-rc2+ #4 kernel: [ 4136.715850] Hardware name: Dell Inc. OptiPlex 790/0HY9JP, BIOS A18 09/24/2013 kernel: [ 4136.715858] RIP: 0010:media_gobj_destroy.part.16+0x1f/0x60 kernel: [ 4136.715863] Code: ff 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 55 48 89 fe 48 89 e5 53 48 89 fb 48 c7 c7 00 7f cf b0 e8 24 fa ff ff 48 8b 03 <48> 83 80 a0 00 00 00 01 48 8b 43 18 48 8b 53 10 48 89 42 08 48 89 kernel: [ 4136.715866] RSP: 0018:ffff9b2248fe3cb0 EFLAGS: 00010246 kernel: [ 4136.715870] RAX: bcf2bfbfa0d63c2f RBX: ffff88c3eb37e9c0 RCX: 00000000802a0018 kernel: [ 4136.715873] RDX: ffff88c3e4f6a078 RSI: ffff88c3eb37e9c0 RDI: ffffffffb0cf7f00 kernel: [ 4136.715876] RBP: ffff9b2248fe3cb8 R08: 0000000001000002 R09: ffffffffb0492b00 kernel: [ 4136.715879] R10: ffff9b2248fe3c28 R11: 0000000000000001 R12: 0000000000000038 kernel: [ 4136.715881] R13: ffffffffc09a1628 R14: ffff88c3e4f6a028 R15: fffffffffffffff2 kernel: [ 4136.715885] FS: 00007f8389647740(0000) GS:ffff88c465500000(0000) knlGS:0000000000000000 kernel: [ 4136.715888] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 kernel: [ 4136.715891] CR2: 000055d008f80fd8 CR3: 00000001996ec005 CR4: 00000000000606e0 kernel: [ 4136.715894] Call Trace: kernel: [ 4136.715903] media_gobj_destroy+0x14/0x20 kernel: [ 4136.715908] __media_device_unregister_entity+0xb3/0xe0 kernel: [ 4136.715915] media_device_unregister_entity+0x30/0x40 kernel: [ 4136.715920] v4l2_device_unregister_subdev+0xa8/0xe0 kernel: [ 4136.715928] vimc_ent_sd_unregister+0x1e/0x30 [vimc] kernel: [ 4136.715933] vimc_sen_rm+0x16/0x20 [vimc] kernel: [ 4136.715938] vimc_remove+0x3e/0xa0 [vimc] kernel: [ 4136.715945] platform_drv_remove+0x25/0x50 kernel: [ 4136.715951] device_release_driver_internal+0xe0/0x1b0 kernel: [ 4136.715956] device_driver_detach+0x14/0x20 kernel: [ 4136.715960] unbind_store+0xd1/0x130 kernel: [ 4136.715965] drv_attr_store+0x27/0x40 kernel: [ 4136.715971] sysfs_kf_write+0x48/0x60 kernel: [ 4136.715976] kernfs_fop_write+0x128/0x1b0 kernel: [ 4136.715982] __vfs_write+0x1b/0x40 kernel: [ 4136.715987] vfs_write+0xc3/0x1d0 kernel: [ 4136.715993] ksys_write+0xaa/0xe0 kernel: [ 4136.715999] __x64_sys_write+0x1a/0x20 kernel: [ 4136.716005] do_syscall_64+0x5a/0x130 kernel: [ 4136.716010] entry_SYSCALL_64_after_hwframe+0x4 Signed-off-by: Shuah Khan Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/vimc/vimc-common.c | 3 +-- drivers/media/platform/vimc/vimc-debayer.c | 1 + drivers/media/platform/vimc/vimc-scaler.c | 1 + drivers/media/platform/vimc/vimc-sensor.c | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c index 7e1ae0b12f1e..a3120f4f7a90 100644 --- a/drivers/media/platform/vimc/vimc-common.c +++ b/drivers/media/platform/vimc/vimc-common.c @@ -375,7 +375,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved, { int ret; - /* Allocate the pads */ + /* Allocate the pads. Should be released from the sd_int_op release */ ved->pads = vimc_pads_init(num_pads, pads_flag); if (IS_ERR(ved->pads)) return PTR_ERR(ved->pads); @@ -424,7 +424,6 @@ EXPORT_SYMBOL_GPL(vimc_ent_sd_register); void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd) { media_entity_cleanup(ved->ent); - vimc_pads_cleanup(ved->pads); v4l2_device_unregister_subdev(sd); } EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister); diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index 2c291447807e..4125159e8f31 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -480,6 +480,7 @@ static void vimc_deb_release(struct v4l2_subdev *sd) struct vimc_deb_device *vdeb = container_of(sd, struct vimc_deb_device, sd); + vimc_pads_cleanup(vdeb->ved.pads); kfree(vdeb); } diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index f72200de2535..1a593d81ea7c 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -338,6 +338,7 @@ static void vimc_sca_release(struct v4l2_subdev *sd) struct vimc_sca_device *vsca = container_of(sd, struct vimc_sca_device, sd); + vimc_pads_cleanup(vsca->ved.pads); kfree(vsca); } diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c index 1f15637ca8bb..46dc6a535abe 100644 --- a/drivers/media/platform/vimc/vimc-sensor.c +++ b/drivers/media/platform/vimc/vimc-sensor.c @@ -291,6 +291,7 @@ static void vimc_sen_release(struct v4l2_subdev *sd) v4l2_ctrl_handler_free(&vsen->hdl); tpg_free(&vsen->tpg); + vimc_pads_cleanup(vsen->ved.pads); kfree(vsen); } -- cgit v1.2.3 From 3a9e69f1404f174ea8e0432d308aff20d04a9eeb Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Tue, 17 Sep 2019 13:35:10 -0300 Subject: media: vimc: move duplicated IS_SRC and IS_SINK to common header Move duplicated IS_SRC and IS_SINK dfines to common header. Rename them to VIMC_IS_SRC and VIM_IS_SINK. Signed-off-by: Shuah Khan Acked-by: Helen Koike Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/vimc/vimc-common.h | 4 ++++ drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- 3 files changed, 11 insertions(+), 12 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h index d7aaf31175bc..698db7c07645 100644 --- a/drivers/media/platform/vimc/vimc-common.h +++ b/drivers/media/platform/vimc/vimc-common.h @@ -27,6 +27,10 @@ #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) +/* Source and sink pad checks */ +#define VIMC_IS_SRC(pad) (pad) +#define VIMC_IS_SINK(pad) (!(pad)) + /** * struct vimc_colorimetry_clamp - Adjust colorimetry parameters * diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index 4125159e8f31..feac47d79449 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -20,9 +20,6 @@ MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n" "stays in the center of the window, otherwise the next odd number " "is considered"); -#define IS_SINK(pad) (!pad) -#define IS_SRC(pad) (pad) - enum vimc_deb_rgb_colors { VIMC_DEB_RED = 0, VIMC_DEB_GREEN = 1, @@ -155,7 +152,7 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_mbus_code_enum *code) { /* We only support one format for source pads */ - if (IS_SRC(code->pad)) { + if (VIMC_IS_SRC(code->pad)) { struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd); if (code->index) @@ -181,7 +178,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd, if (fse->index) return -EINVAL; - if (IS_SINK(fse->pad)) { + if (VIMC_IS_SINK(fse->pad)) { const struct vimc_deb_pix_map *vpix = vimc_deb_pix_map_by_code(fse->code); @@ -211,7 +208,7 @@ static int vimc_deb_get_fmt(struct v4l2_subdev *sd, vdeb->sink_fmt; /* Set the right code for the source pad */ - if (IS_SRC(fmt->pad)) + if (VIMC_IS_SRC(fmt->pad)) fmt->format.code = vdeb->src_code; return 0; @@ -258,7 +255,7 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd, * Do not change the format of the source pad, * it is propagated from the sink */ - if (IS_SRC(fmt->pad)) { + if (VIMC_IS_SRC(fmt->pad)) { fmt->format = *sink_fmt; /* TODO: Add support for other formats */ fmt->format.code = vdeb->src_code; diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index 1a593d81ea7c..a6a3cc5be872 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -16,8 +16,6 @@ static unsigned int sca_mult = 3; module_param(sca_mult, uint, 0000); MODULE_PARM_DESC(sca_mult, " the image size multiplier"); -#define IS_SINK(pad) (!pad) -#define IS_SRC(pad) (pad) #define MAX_ZOOM 8 struct vimc_sca_device { @@ -93,7 +91,7 @@ static int vimc_sca_enum_frame_size(struct v4l2_subdev *sd, fse->min_width = VIMC_FRAME_MIN_WIDTH; fse->min_height = VIMC_FRAME_MIN_HEIGHT; - if (IS_SINK(fse->pad)) { + if (VIMC_IS_SINK(fse->pad)) { fse->max_width = VIMC_FRAME_MAX_WIDTH; fse->max_height = VIMC_FRAME_MAX_HEIGHT; } else { @@ -116,7 +114,7 @@ static int vimc_sca_get_fmt(struct v4l2_subdev *sd, vsca->sink_fmt; /* Scale the frame size for the source pad */ - if (IS_SRC(format->pad)) { + if (VIMC_IS_SRC(format->pad)) { format->format.width = vsca->sink_fmt.width * sca_mult; format->format.height = vsca->sink_fmt.height * sca_mult; } @@ -165,7 +163,7 @@ static int vimc_sca_set_fmt(struct v4l2_subdev *sd, * Do not change the format of the source pad, * it is propagated from the sink */ - if (IS_SRC(fmt->pad)) { + if (VIMC_IS_SRC(fmt->pad)) { fmt->format = *sink_fmt; fmt->format.width = sink_fmt->width * sca_mult; fmt->format.height = sink_fmt->height * sca_mult; -- cgit v1.2.3 From b0e41bf23b590082c9109b691a1f949a0b8defae Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Fri, 20 Sep 2019 14:05:42 -0300 Subject: media: am437x-vpfe: Fix suspend path to always handle pinctrl config Currently if vpfe is not active then it returns immediately in the suspend and resume handlers. Change this so that it always performs the pinctrl config so that we can still get proper sleep state configuration on the pins even if we do not need to worry about fully saving and restoring context. Signed-off-by: Dave Gerlach Signed-off-by: Jyri Sarha Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 46 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index 2b42ba1f5949..a3d22f90e64c 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -2653,22 +2653,21 @@ static int vpfe_suspend(struct device *dev) struct vpfe_device *vpfe = dev_get_drvdata(dev); struct vpfe_ccdc *ccdc = &vpfe->ccdc; - /* if streaming has not started we don't care */ - if (!vb2_start_streaming_called(&vpfe->buffer_queue)) - return 0; - - pm_runtime_get_sync(dev); - vpfe_config_enable(ccdc, 1); + /* only do full suspend if streaming has started */ + if (vb2_start_streaming_called(&vpfe->buffer_queue)) { + pm_runtime_get_sync(dev); + vpfe_config_enable(ccdc, 1); - /* Save VPFE context */ - vpfe_save_context(ccdc); + /* Save VPFE context */ + vpfe_save_context(ccdc); - /* Disable CCDC */ - vpfe_pcr_enable(ccdc, 0); - vpfe_config_enable(ccdc, 0); + /* Disable CCDC */ + vpfe_pcr_enable(ccdc, 0); + vpfe_config_enable(ccdc, 0); - /* Disable both master and slave clock */ - pm_runtime_put_sync(dev); + /* Disable both master and slave clock */ + pm_runtime_put_sync(dev); + } /* Select sleep pin state */ pinctrl_pm_select_sleep_state(dev); @@ -2710,19 +2709,18 @@ static int vpfe_resume(struct device *dev) struct vpfe_device *vpfe = dev_get_drvdata(dev); struct vpfe_ccdc *ccdc = &vpfe->ccdc; - /* if streaming has not started we don't care */ - if (!vb2_start_streaming_called(&vpfe->buffer_queue)) - return 0; - - /* Enable both master and slave clock */ - pm_runtime_get_sync(dev); - vpfe_config_enable(ccdc, 1); + /* only do full resume if streaming has started */ + if (vb2_start_streaming_called(&vpfe->buffer_queue)) { + /* Enable both master and slave clock */ + pm_runtime_get_sync(dev); + vpfe_config_enable(ccdc, 1); - /* Restore VPFE context */ - vpfe_restore_context(ccdc); + /* Restore VPFE context */ + vpfe_restore_context(ccdc); - vpfe_config_enable(ccdc, 0); - pm_runtime_put_sync(dev); + vpfe_config_enable(ccdc, 0); + pm_runtime_put_sync(dev); + } /* Select default pin state */ pinctrl_pm_select_default_state(dev); -- cgit v1.2.3 From 47c7bcfdb387141126392ff4857a03910d95b4b6 Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:43 -0300 Subject: media: am437x-vpfe: Fix missing first line Previous generation of this driver were hard coded to handle encoder/decoder where the first line never contains any data and was therefore always skipped, however when dealing with actual camera sensors the first line is always present. Signed-off-by: Benoit Parrot Signed-off-by: Jyri Sarha Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index a3d22f90e64c..1521c072f3e3 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -345,13 +345,9 @@ static void vpfe_ccdc_setwin(struct vpfe_ccdc *ccdc, if (frm_fmt == CCDC_FRMFMT_INTERLACED) { vert_nr_lines = (image_win->height >> 1) - 1; vert_start >>= 1; - /* Since first line doesn't have any data */ - vert_start += 1; /* configure VDINT0 */ val = (vert_start << VPFE_VDINT_VDINT0_SHIFT); } else { - /* Since first line doesn't have any data */ - vert_start += 1; vert_nr_lines = image_win->height - 1; /* * configure VDINT0 and VDINT1. VDINT1 will be at half -- cgit v1.2.3 From e6784f9e4ebb6fac0c8741eeab4009a3a68a3831 Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:44 -0300 Subject: media: am437x-vpfe: Rework ISR routine for clarity Make the ISR code simpler to follow by removing goto and relocating/eliminating duplicate spinlock accesses. Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 128 ++++++++++++++-------------- 1 file changed, 66 insertions(+), 62 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index 1521c072f3e3..13bf4b32b40b 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -1233,22 +1233,29 @@ unlock: * This function will get next buffer from the dma queue and * set the buffer address in the vpfe register for capture. * the buffer is marked active - * - * Assumes caller is holding vpfe->dma_queue_lock already */ -static inline void vpfe_schedule_next_buffer(struct vpfe_device *vpfe) +static void vpfe_schedule_next_buffer(struct vpfe_device *vpfe) { + dma_addr_t addr; + + spin_lock(&vpfe->dma_queue_lock); + if (list_empty(&vpfe->dma_queue)) { + spin_unlock(&vpfe->dma_queue_lock); + return; + } + vpfe->next_frm = list_entry(vpfe->dma_queue.next, struct vpfe_cap_buffer, list); list_del(&vpfe->next_frm->list); + spin_unlock(&vpfe->dma_queue_lock); - vpfe_set_sdr_addr(&vpfe->ccdc, - vb2_dma_contig_plane_dma_addr(&vpfe->next_frm->vb.vb2_buf, 0)); + addr = vb2_dma_contig_plane_dma_addr(&vpfe->next_frm->vb.vb2_buf, 0); + vpfe_set_sdr_addr(&vpfe->ccdc, addr); } static inline void vpfe_schedule_bottom_field(struct vpfe_device *vpfe) { - unsigned long addr; + dma_addr_t addr; addr = vb2_dma_contig_plane_dma_addr(&vpfe->next_frm->vb.vb2_buf, 0) + vpfe->field_off; @@ -1273,6 +1280,55 @@ static inline void vpfe_process_buffer_complete(struct vpfe_device *vpfe) vpfe->cur_frm = vpfe->next_frm; } +static void vpfe_handle_interlaced_irq(struct vpfe_device *vpfe, + enum v4l2_field field) +{ + int fid; + + /* interlaced or TB capture check which field + * we are in hardware + */ + fid = vpfe_ccdc_getfid(&vpfe->ccdc); + + /* switch the software maintained field id */ + vpfe->field ^= 1; + if (fid == vpfe->field) { + /* we are in-sync here,continue */ + if (fid == 0) { + /* + * One frame is just being captured. If the + * next frame is available, release the + * current frame and move on + */ + if (vpfe->cur_frm != vpfe->next_frm) + vpfe_process_buffer_complete(vpfe); + + /* + * based on whether the two fields are stored + * interleave or separately in memory, + * reconfigure the CCDC memory address + */ + if (field == V4L2_FIELD_SEQ_TB) + vpfe_schedule_bottom_field(vpfe); + } else { + /* + * if one field is just being captured configure + * the next frame get the next frame from the empty + * queue if no frame is available hold on to the + * current buffer + */ + if (vpfe->cur_frm == vpfe->next_frm) + vpfe_schedule_next_buffer(vpfe); + } + } else if (fid == 0) { + /* + * out of sync. Recover from any hardware out-of-sync. + * May loose one frame + */ + vpfe->field = fid; + } +} + /* * vpfe_isr : ISR handler for vpfe capture (VINT0) * @irq: irq number @@ -1284,76 +1340,24 @@ static inline void vpfe_process_buffer_complete(struct vpfe_device *vpfe) static irqreturn_t vpfe_isr(int irq, void *dev) { struct vpfe_device *vpfe = (struct vpfe_device *)dev; - enum v4l2_field field; + enum v4l2_field field = vpfe->fmt.fmt.pix.field; int intr_status; - int fid; intr_status = vpfe_reg_read(&vpfe->ccdc, VPFE_IRQ_STS); if (intr_status & VPFE_VDINT0) { - field = vpfe->fmt.fmt.pix.field; - if (field == V4L2_FIELD_NONE) { - /* handle progressive frame capture */ if (vpfe->cur_frm != vpfe->next_frm) vpfe_process_buffer_complete(vpfe); - goto next_intr; - } - - /* interlaced or TB capture check which field - we are in hardware */ - fid = vpfe_ccdc_getfid(&vpfe->ccdc); - - /* switch the software maintained field id */ - vpfe->field ^= 1; - if (fid == vpfe->field) { - /* we are in-sync here,continue */ - if (fid == 0) { - /* - * One frame is just being captured. If the - * next frame is available, release the - * current frame and move on - */ - if (vpfe->cur_frm != vpfe->next_frm) - vpfe_process_buffer_complete(vpfe); - /* - * based on whether the two fields are stored - * interleave or separately in memory, - * reconfigure the CCDC memory address - */ - if (field == V4L2_FIELD_SEQ_TB) - vpfe_schedule_bottom_field(vpfe); - - goto next_intr; - } - /* - * if one field is just being captured configure - * the next frame get the next frame from the empty - * queue if no frame is available hold on to the - * current buffer - */ - spin_lock(&vpfe->dma_queue_lock); - if (!list_empty(&vpfe->dma_queue) && - vpfe->cur_frm == vpfe->next_frm) - vpfe_schedule_next_buffer(vpfe); - spin_unlock(&vpfe->dma_queue_lock); - } else if (fid == 0) { - /* - * out of sync. Recover from any hardware out-of-sync. - * May loose one frame - */ - vpfe->field = fid; + } else { + vpfe_handle_interlaced_irq(vpfe, field); } } -next_intr: if (intr_status & VPFE_VDINT1) { - spin_lock(&vpfe->dma_queue_lock); - if (vpfe->fmt.fmt.pix.field == V4L2_FIELD_NONE && - !list_empty(&vpfe->dma_queue) && + if (field == V4L2_FIELD_NONE && vpfe->cur_frm == vpfe->next_frm) vpfe_schedule_next_buffer(vpfe); - spin_unlock(&vpfe->dma_queue_lock); } vpfe_clear_intr(&vpfe->ccdc, intr_status); -- cgit v1.2.3 From b58e69e9a573e74946498077ae25e5244303e718 Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:45 -0300 Subject: media: am437x-vpfe: Wait for end of frame before tear-down We were originally attempting to stop all processing as soon as possible, but the in-progress DMA operation cannot be canceled. This led to the module being in a busy state and prevented proper power management functionality. The existing implementation would attempt to clean things up by waiting up to 50ms. However when receiving video frame at 15fps or lower, it meant an inter frame arrival rate of 66.6 ms or higher. In such cases upon tear down the following message could be seen: omap_hwmod: vpfe0: _wait_target_disable failed This patch fixes this issue by adding a stopping state where we would wait for the next Vsync before disabling the hardware. Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 52 ++++++++++++++--------------- drivers/media/platform/am437x/am437x-vpfe.h | 3 ++ 2 files changed, 29 insertions(+), 26 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index 13bf4b32b40b..66df87d310a2 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -441,40 +441,25 @@ static void vpfe_ccdc_restore_defaults(struct vpfe_ccdc *ccdc) static int vpfe_ccdc_close(struct vpfe_ccdc *ccdc, struct device *dev) { - int dma_cntl, i, pcr; + struct vpfe_device *vpfe = container_of(ccdc, struct vpfe_device, ccdc); + u32 dma_cntl, pcr; - /* If the CCDC module is still busy wait for it to be done */ - for (i = 0; i < 10; i++) { - usleep_range(5000, 6000); - pcr = vpfe_reg_read(ccdc, VPFE_PCR); - if (!pcr) - break; + pcr = vpfe_reg_read(ccdc, VPFE_PCR); + if (pcr) + vpfe_dbg(1, vpfe, "VPFE_PCR is still set (%x)", pcr); - /* make sure it it is disabled */ - vpfe_pcr_enable(ccdc, 0); - } + dma_cntl = vpfe_reg_read(ccdc, VPFE_DMA_CNTL); + if ((dma_cntl & VPFE_DMA_CNTL_OVERFLOW)) + vpfe_dbg(1, vpfe, "VPFE_DMA_CNTL_OVERFLOW is still set (%x)", + dma_cntl); /* Disable CCDC by resetting all register to default POR values */ vpfe_ccdc_restore_defaults(ccdc); - /* if DMA_CNTL overflow bit is set. Clear it - * It appears to take a while for this to become quiescent ~20ms - */ - for (i = 0; i < 10; i++) { - dma_cntl = vpfe_reg_read(ccdc, VPFE_DMA_CNTL); - if (!(dma_cntl & VPFE_DMA_CNTL_OVERFLOW)) - break; - - /* Clear the overflow bit */ - vpfe_reg_write(ccdc, dma_cntl, VPFE_DMA_CNTL); - usleep_range(5000, 6000); - } - /* Disabled the module at the CONFIG level */ vpfe_config_enable(ccdc, 0); pm_runtime_put_sync(dev); - return 0; } @@ -1303,6 +1288,9 @@ static void vpfe_handle_interlaced_irq(struct vpfe_device *vpfe, if (vpfe->cur_frm != vpfe->next_frm) vpfe_process_buffer_complete(vpfe); + if (vpfe->stopping) + return; + /* * based on whether the two fields are stored * interleave or separately in memory, @@ -1341,7 +1329,7 @@ static irqreturn_t vpfe_isr(int irq, void *dev) { struct vpfe_device *vpfe = (struct vpfe_device *)dev; enum v4l2_field field = vpfe->fmt.fmt.pix.field; - int intr_status; + int intr_status, stopping = vpfe->stopping; intr_status = vpfe_reg_read(&vpfe->ccdc, VPFE_IRQ_STS); @@ -1352,9 +1340,13 @@ static irqreturn_t vpfe_isr(int irq, void *dev) } else { vpfe_handle_interlaced_irq(vpfe, field); } + if (stopping) { + vpfe->stopping = false; + complete(&vpfe->capture_stop); + } } - if (intr_status & VPFE_VDINT1) { + if (intr_status & VPFE_VDINT1 && !stopping) { if (field == V4L2_FIELD_NONE && vpfe->cur_frm == vpfe->next_frm) vpfe_schedule_next_buffer(vpfe); @@ -1980,6 +1972,9 @@ static int vpfe_start_streaming(struct vb2_queue *vq, unsigned int count) vpfe_attach_irq(vpfe); + vpfe->stopping = false; + init_completion(&vpfe->capture_stop); + if (vpfe->ccdc.ccdc_cfg.if_type == VPFE_RAW_BAYER) vpfe_ccdc_config_raw(&vpfe->ccdc); else @@ -2032,6 +2027,11 @@ static void vpfe_stop_streaming(struct vb2_queue *vq) vpfe_pcr_enable(&vpfe->ccdc, 0); + /* Wait for the last frame to be captured */ + vpfe->stopping = true; + wait_for_completion_timeout(&vpfe->capture_stop, + msecs_to_jiffies(250)); + vpfe_detach_irq(vpfe); sdinfo = vpfe->current_subdev; diff --git a/drivers/media/platform/am437x/am437x-vpfe.h b/drivers/media/platform/am437x/am437x-vpfe.h index 4678285f34c6..2dde09780215 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.h +++ b/drivers/media/platform/am437x/am437x-vpfe.h @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -270,6 +271,8 @@ struct vpfe_device { */ u32 field_off; struct vpfe_ccdc ccdc; + int stopping; + struct completion capture_stop; }; #endif /* AM437X_VPFE_H */ -- cgit v1.2.3 From 158a1dddf2dbce5fdb3053213c1e9b36a54b4d4d Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:46 -0300 Subject: media: am437x-vpfe: fix start streaming error path When start_streaming fails the h/w module might be left enabled inadvertently. Make sure it is disabled in the error path. Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index 66df87d310a2..e0a4c8920df8 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -2008,6 +2008,7 @@ err: vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED); } + vpfe_pcr_enable(&vpfe->ccdc, 0); return ret; } -- cgit v1.2.3 From 73940235337ee91ba32eaf71d641d74d9b1c9b63 Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:47 -0300 Subject: media: am437x-vpfe: Streamlined vb2 buffer cleanup Returning queued vb2 buffers back to user space is a common task best handled by a helper function. Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/am437x/am437x-vpfe.c | 53 ++++++++++++++--------------- 1 file changed, 25 insertions(+), 28 deletions(-) (limited to 'drivers/media/platform') diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c index e0a4c8920df8..03415c179c85 100644 --- a/drivers/media/platform/am437x/am437x-vpfe.c +++ b/drivers/media/platform/am437x/am437x-vpfe.c @@ -1949,6 +1949,29 @@ static void vpfe_buffer_queue(struct vb2_buffer *vb) spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags); } +static void vpfe_return_all_buffers(struct vpfe_device *vpfe, + enum vb2_buffer_state state) +{ + struct vpfe_cap_buffer *buf, *node; + unsigned long flags; + + spin_lock_irqsave(&vpfe->dma_queue_lock, flags); + list_for_each_entry_safe(buf, node, &vpfe->dma_queue, list) { + vb2_buffer_done(&buf->vb.vb2_buf, state); + list_del(&buf->list); + } + + if (vpfe->cur_frm) + vb2_buffer_done(&vpfe->cur_frm->vb.vb2_buf, state); + + if (vpfe->next_frm && vpfe->next_frm != vpfe->cur_frm) + vb2_buffer_done(&vpfe->next_frm->vb.vb2_buf, state); + + vpfe->cur_frm = NULL; + vpfe->next_frm = NULL; + spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags); +} + /* * vpfe_start_streaming : Starts the DMA engine for streaming * @vb: ptr to vb2_buffer @@ -1957,7 +1980,6 @@ static void vpfe_buffer_queue(struct vb2_buffer *vb) static int vpfe_start_streaming(struct vb2_queue *vq, unsigned int count) { struct vpfe_device *vpfe = vb2_get_drv_priv(vq); - struct vpfe_cap_buffer *buf, *tmp; struct vpfe_subdev_info *sdinfo; unsigned long flags; unsigned long addr; @@ -2003,11 +2025,7 @@ static int vpfe_start_streaming(struct vb2_queue *vq, unsigned int count) return 0; err: - list_for_each_entry_safe(buf, tmp, &vpfe->dma_queue, list) { - list_del(&buf->list); - vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED); - } - + vpfe_return_all_buffers(vpfe, VB2_BUF_STATE_QUEUED); vpfe_pcr_enable(&vpfe->ccdc, 0); return ret; } @@ -2023,7 +2041,6 @@ static void vpfe_stop_streaming(struct vb2_queue *vq) { struct vpfe_device *vpfe = vb2_get_drv_priv(vq); struct vpfe_subdev_info *sdinfo; - unsigned long flags; int ret; vpfe_pcr_enable(&vpfe->ccdc, 0); @@ -2041,27 +2058,7 @@ static void vpfe_stop_streaming(struct vb2_queue *vq) vpfe_dbg(1, vpfe, "stream off failed in subdev\n"); /* release all active buffers */ - spin_lock_irqsave(&vpfe->dma_queue_lock, flags); - if (vpfe->cur_frm == vpfe->next_frm) { - vb2_buffer_done(&vpfe->cur_frm->vb.vb2_buf, - VB2_BUF_STATE_ERROR); - } else { - if (vpfe->cur_frm != NULL) - vb2_buffer_done(&vpfe->cur_frm->vb.vb2_buf, - VB2_BUF_STATE_ERROR); - if (vpfe->next_frm != NULL) - vb2_buffer_done(&vpfe->next_frm->vb.vb2_buf, - VB2_BUF_STATE_ERROR); - } - - while (!list_empty(&vpfe->dma_queue)) { - vpfe->next_frm = list_entry(vpfe->dma_queue.next, - struct vpfe_cap_buffer, list); - list_del(&vpfe->next_frm->list); - vb2_buffer_done(&vpfe->next_frm->vb.vb2_buf, - VB2_BUF_STATE_ERROR); - } - spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags); + vpfe_return_all_buffers(vpfe, VB2_BUF_STATE_ERROR); } static int vpfe_g_pixelaspect(struct file *file, void *priv, -- cgit v1.2.3 From 13aa21cfe92ce9ebb51824029d89f19c33f81419 Mon Sep 17 00:00:00 2001 From: Benoit Parrot Date: Fri, 20 Sep 2019 14:05:48 -0300 Subject: media: am437x-vpfe: Setting STD to current value is not an error VIDIOC_S_STD should not return an error if the value is identical to the current one. This error was highlighted by the v4l2-compliance test. Signed-off-by: Benoit Parrot Acked-by: Lad Prabhakar Si