2015-08-19 15:36:14

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 0/4] [media] Media entity cleanups and build fixes

Hello,

This series contains a couple of build fixes and cleanups for the
Media Controller framework. The goal of the series is to get rid of
the struct media_entity .parent member since now that a media_gobj is
embedded into entities, the media_gobj .mdev member can be used to
store a pointer to the parent struct media_device.

So the .parent field becomes redundant and can be removed after all
the users are converted to use entity .graph_obj.mdev instead.

Patches 1/4 and 2/4 are build fixes I found while build testing if no
regressions were introduced by the conversion. Patch 3/4 converts
all the drivers and the MC core to use .mdev instead of .parent and
finally patch 4/4 removes the .parent field since now is unused.

The series depend on Mauro's "[PATCH v6 0/8] MC preparation patches
series" [0].

The transformation were automated using a coccinelle semantic patch
and the drivers were build tested using allyesconfig and x-building
the ARM Exynos and OMAP defconfigs + the needed media config options.

Best regards,
Javier

[0]: http://www.mail-archive.com/[email protected]/msg91330.html


Javier Martinez Canillas (4):
[media] staging: omap4iss: get entity ID using media_entity_id()
[media] omap3isp: get entity ID using media_entity_id()
[media] media: use entity.graph_obj.mdev instead of .parent
[media] media: remove media entity .parent field

drivers/media/media-device.c | 8 ++---
drivers/media/media-entity.c | 34 ++++++++++++----------
drivers/media/platform/exynos4-is/fimc-isp-video.c | 6 ++--
drivers/media/platform/exynos4-is/fimc-lite.c | 8 ++---
drivers/media/platform/exynos4-is/media-dev.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.h | 8 ++---
drivers/media/platform/omap3isp/isp.c | 11 ++++---
drivers/media/platform/omap3isp/ispccdc.c | 2 +-
drivers/media/platform/omap3isp/ispvideo.c | 10 ++++---
drivers/media/platform/vsp1/vsp1_video.c | 2 +-
drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 ++--
drivers/staging/media/omap4iss/iss.c | 6 ++--
drivers/staging/media/omap4iss/iss_video.c | 4 +--
include/media/media-entity.h | 1 -
15 files changed, 58 insertions(+), 52 deletions(-)

--
2.4.3


2015-08-19 15:36:17

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

The struct media_entity does not have an .id field anymore since
now the entity ID is stored in the embedded struct media_gobj.

This caused the omap4iss driver fail to build. Fix by using the
media_entity_id() macro to obtain the entity ID.

Signed-off-by: Javier Martinez Canillas <[email protected]>
---

drivers/staging/media/omap4iss/iss.c | 2 +-
drivers/staging/media/omap4iss/iss_video.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index f32ab7b98ae2..7226553ceb2f 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -607,7 +607,7 @@ static int iss_pipeline_disable(struct iss_pipeline *pipe,
* crashed. Mark it as such, the ISS will be reset when
* applications will release it.
*/
- iss->crashed |= 1U << subdev->entity.id;
+ iss->crashed |= 1U << media_entity_id(&subdev->entity);
failure = -ETIMEDOUT;
}
}
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index bae67742706f..25e9e7a6b99d 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -784,7 +784,7 @@ iss_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
entity = &video->video.entity;
media_entity_graph_walk_start(&graph, entity);
while ((entity = media_entity_graph_walk_next(&graph)))
- pipe->entities |= 1 << entity->id;
+ pipe->entities |= 1 << media_entity_id(entity);

/* Verify that the currently configured format matches the output of
* the connected subdev.
--
2.4.3

2015-08-19 15:36:31

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 2/4] [media] omap3isp: get entity ID using media_entity_id()

The struct media_entity does not have an .id field anymore since
now the entity ID is stored in the embedded struct media_gobj.

This caused the omap3isp driver fail to build. Fix by using the
media_entity_id() macro to obtain the entity ID.

Signed-off-by: Javier Martinez Canillas <[email protected]>
---

drivers/media/platform/omap3isp/isp.c | 7 +++++--
drivers/media/platform/omap3isp/ispccdc.c | 2 +-
drivers/media/platform/omap3isp/ispvideo.c | 8 +++++---
3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index d6b2922ed55c..6351f35b0a65 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -975,6 +975,7 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
struct v4l2_subdev *subdev;
int failure = 0;
int ret;
+ u32 id;

/*
* We need to stop all the modules after CCDC first or they'll
@@ -1027,8 +1028,10 @@ static int isp_pipeline_disable(struct isp_pipeline *pipe)
if (ret) {
dev_info(isp->dev, "Unable to stop %s\n", subdev->name);
isp->stop_failure = true;
- if (subdev == &isp->isp_prev.subdev)
- isp->crashed |= 1U << subdev->entity.id;
+ if (subdev == &isp->isp_prev.subdev) {
+ id = media_entity_id(&subdev->entity);
+ isp->crashed |= 1U << id;
+ }
failure = -ETIMEDOUT;
}
}
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 3fe425f254bb..27555e4f4aa8 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1608,7 +1608,7 @@ static int ccdc_isr_buffer(struct isp_ccdc_device *ccdc)
/* Wait for the CCDC to become idle. */
if (ccdc_sbl_wait_idle(ccdc, 1000)) {
dev_info(isp->dev, "CCDC won't become idle!\n");
- isp->crashed |= 1U << ccdc->subdev.entity.id;
+ isp->crashed |= 1U << media_entity_id(&ccdc->subdev.entity);
omap3isp_pipeline_cancel_stream(pipe);
return 0;
}
diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 3094572f8897..6c89dc40df85 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -235,7 +235,7 @@ static int isp_video_get_graph_data(struct isp_video *video,
while ((entity = media_entity_graph_walk_next(&graph))) {
struct isp_video *__video;

- pipe->entities |= 1 << entity->id;
+ pipe->entities |= 1 << media_entity_id(entity);

if (far_end != NULL)
continue;
@@ -891,6 +891,7 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
struct v4l2_ext_control ctrl;
unsigned int i;
int ret;
+ u32 id;

/* Memory-to-memory pipelines have no external subdev. */
if (pipe->input != NULL)
@@ -898,7 +899,7 @@ static int isp_video_check_external_subdevs(struct isp_video *video,

for (i = 0; i < ARRAY_SIZE(ents); i++) {
/* Is the entity part of the pipeline? */
- if (!(pipe->entities & (1 << ents[i]->id)))
+ if (!(pipe->entities & (1 << media_entity_id(ents[i]))))
continue;

/* ISP entities have always sink pad == 0. Find source. */
@@ -950,7 +951,8 @@ static int isp_video_check_external_subdevs(struct isp_video *video,

pipe->external_rate = ctrl.value64;

- if (pipe->entities & (1 << isp->isp_ccdc.subdev.entity.id)) {
+ id = media_entity_id(&isp->isp_ccdc.subdev.entity);
+ if (pipe->entities & (1 << id)) {
unsigned int rate = UINT_MAX;
/*
* Check that maximum allowed CCDC pixel rate isn't
--
2.4.3

2015-08-19 15:37:05

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 3/4] [media] media: use entity.graph_obj.mdev instead of .parent

The struct media_entity has a .parent field that stores a pointer
to the parent struct media_device. But recently a media_gobj was
embedded into the entities and since struct media_gojb already has
a pointer to a struct media_device in the .mdev field, the .parent
field becomes redundant and can be removed.

This patch replaces all the usage of .parent by .graph_obj.mdev so
that field will become unused and can be removed on a later patch.

No functional changes.

The transformation was made using the following coccinelle spatch:

@@
struct media_entity *me;
@@

- me->parent
+ me->graph_obj.mdev

@@
struct media_entity *link;
@@

- link->source->entity->parent
+ link->source->entity->graph_obj.mdev

@@
struct exynos_video_entity *ve;
@@

- ve->vdev.entity.parent
+ ve->vdev.entity.graph_obj.mdev

Suggested-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---

drivers/media/media-device.c | 8 ++---
drivers/media/media-entity.c | 34 ++++++++++++----------
drivers/media/platform/exynos4-is/fimc-isp-video.c | 6 ++--
drivers/media/platform/exynos4-is/fimc-lite.c | 8 ++---
drivers/media/platform/exynos4-is/media-dev.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.h | 8 ++---
drivers/media/platform/omap3isp/isp.c | 4 +--
drivers/media/platform/omap3isp/ispvideo.c | 2 +-
drivers/media/platform/vsp1/vsp1_video.c | 2 +-
drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 ++--
drivers/staging/media/omap4iss/iss.c | 4 +--
drivers/staging/media/omap4iss/iss_video.c | 2 +-
13 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0f3844470147..138b18416460 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -435,8 +435,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
int i;

/* Warn if we apparently re-register an entity */
- WARN_ON(entity->parent != NULL);
- entity->parent = mdev;
+ WARN_ON(entity->graph_obj.mdev != NULL);
+ entity->graph_obj.mdev = mdev;

spin_lock(&mdev->lock);
/* Initialize media_gobj embedded at the entity */
@@ -471,7 +471,7 @@ EXPORT_SYMBOL_GPL(media_device_register_entity);
void media_device_unregister_entity(struct media_entity *entity)
{
int i;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;

if (mdev == NULL)
return;
@@ -484,7 +484,7 @@ void media_device_unregister_entity(struct media_entity *entity)
media_gobj_remove(&entity->graph_obj);
list_del(&entity->list);
spin_unlock(&mdev->lock);
- entity->parent = NULL;
+ entity->graph_obj.mdev = NULL;
}
EXPORT_SYMBOL_GPL(media_device_unregister_entity);

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 35e52cd1fc5a..a23c93369a04 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -332,7 +332,7 @@ EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
__must_check int media_entity_pipeline_start(struct media_entity *entity,
struct media_pipeline *pipe)
{
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
struct media_entity_graph graph;
struct media_entity *entity_err = entity;
int ret;
@@ -387,7 +387,7 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,

ret = entity->ops->link_validate(link);
if (ret < 0 && ret != -ENOIOCTLCMD) {
- dev_dbg(entity->parent->dev,
+ dev_dbg(entity->graph_obj.mdev->dev,
"link validation failed for \"%s\":%u -> \"%s\":%u, error %d\n",
link->source->entity->name,
link->source->index,
@@ -401,7 +401,7 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,

if (!bitmap_full(active, entity->num_pads)) {
ret = -EPIPE;
- dev_dbg(entity->parent->dev,
+ dev_dbg(entity->graph_obj.mdev->dev,
"\"%s\":%u must be connected by an enabled link\n",
entity->name,
(unsigned)find_first_zero_bit(
@@ -454,7 +454,7 @@ EXPORT_SYMBOL_GPL(media_entity_pipeline_start);
*/
void media_entity_pipeline_stop(struct media_entity *entity)
{
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
struct media_entity_graph graph;

mutex_lock(&mdev->graph_mutex);
@@ -490,8 +490,8 @@ struct media_entity *media_entity_get(struct media_entity *entity)
if (entity == NULL)
return NULL;

- if (entity->parent->dev &&
- !try_module_get(entity->parent->dev->driver->owner))
+ if (entity->graph_obj.mdev->dev &&
+ !try_module_get(entity->graph_obj.mdev->dev->driver->owner))
return NULL;

return entity;
@@ -511,8 +511,8 @@ void media_entity_put(struct media_entity *entity)
if (entity == NULL)
return;

- if (entity->parent->dev)
- module_put(entity->parent->dev->driver->owner);
+ if (entity->graph_obj.mdev->dev)
+ module_put(entity->graph_obj.mdev->dev->driver->owner);
}
EXPORT_SYMBOL_GPL(media_entity_put);

@@ -561,7 +561,8 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
link->flags = flags;

/* Initialize graph object embedded at the new link */
- media_gobj_init(source->parent, MEDIA_GRAPH_LINK, &link->graph_obj);
+ media_gobj_init(source->graph_obj.mdev, MEDIA_GRAPH_LINK,
+ &link->graph_obj);

/* Create the backlink. Backlinks are used to help graph traversal and
* are not reported to userspace.
@@ -577,7 +578,8 @@ media_create_pad_link(struct media_entity *source, u16 source_pad,
backlink->flags = flags;

/* Initialize graph object embedded at the new link */
- media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
+ media_gobj_init(sink->graph_obj.mdev, MEDIA_GRAPH_LINK,
+ &backlink->graph_obj);

link->reverse = backlink;
backlink->reverse = link;
@@ -629,12 +631,12 @@ EXPORT_SYMBOL_GPL(__media_entity_remove_links);
void media_entity_remove_links(struct media_entity *entity)
{
/* Do nothing if the entity is not registered. */
- if (entity->parent == NULL)
+ if (entity->graph_obj.mdev == NULL)
return;

- mutex_lock(&entity->parent->graph_mutex);
+ mutex_lock(&entity->graph_obj.mdev->graph_mutex);
__media_entity_remove_links(entity);
- mutex_unlock(&entity->parent->graph_mutex);
+ mutex_unlock(&entity->graph_obj.mdev->graph_mutex);
}
EXPORT_SYMBOL_GPL(media_entity_remove_links);

@@ -703,7 +705,7 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
(source->stream_count || sink->stream_count))
return -EBUSY;

- mdev = source->parent;
+ mdev = source->graph_obj.mdev;

if (mdev->link_notify) {
ret = mdev->link_notify(link, flags,
@@ -724,9 +726,9 @@ int media_entity_setup_link(struct media_link *link, u32 flags)
{
int ret;

- mutex_lock(&link->source->entity->parent->graph_mutex);
+ mutex_lock(&link->source->entity->graph_obj.mdev->graph_mutex);
ret = __media_entity_setup_link(link, flags);
- mutex_unlock(&link->source->entity->parent->graph_mutex);
+ mutex_unlock(&link->source->entity->graph_obj.mdev->graph_mutex);

return ret;
}
diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c b/drivers/media/platform/exynos4-is/fimc-isp-video.c
index b7dc5ac66e36..3d9ccbf5f10f 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
@@ -288,7 +288,7 @@ static int isp_video_open(struct file *file)
goto rel_fh;

if (v4l2_fh_is_singular_file(file)) {
- mutex_lock(&me->parent->graph_mutex);
+ mutex_lock(&me->graph_obj.mdev->graph_mutex);

ret = fimc_pipeline_call(ve, open, me, true);

@@ -296,7 +296,7 @@ static int isp_video_open(struct file *file)
if (ret == 0)
me->use_count++;

- mutex_unlock(&me->parent->graph_mutex);
+ mutex_unlock(&me->graph_obj.mdev->graph_mutex);
}
if (!ret)
goto unlock;
@@ -312,7 +312,7 @@ static int isp_video_release(struct file *file)
struct fimc_isp *isp = video_drvdata(file);
struct fimc_is_video *ivc = &isp->video_capture;
struct media_entity *entity = &ivc->ve.vdev.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;

mutex_lock(&isp->video_lock);

diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index e8f707d1729b..b2607da4ad14 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -500,7 +500,7 @@ static int fimc_lite_open(struct file *file)
atomic_read(&fimc->out_path) != FIMC_IO_DMA)
goto unlock;

- mutex_lock(&me->parent->graph_mutex);
+ mutex_lock(&me->graph_obj.mdev->graph_mutex);

ret = fimc_pipeline_call(&fimc->ve, open, me, true);

@@ -508,7 +508,7 @@ static int fimc_lite_open(struct file *file)
if (ret == 0)
me->use_count++;

- mutex_unlock(&me->parent->graph_mutex);
+ mutex_unlock(&me->graph_obj.mdev->graph_mutex);

if (!ret) {
fimc_lite_clear_event_counters(fimc);
@@ -541,9 +541,9 @@ static int fimc_lite_release(struct file *file)
fimc_pipeline_call(&fimc->ve, close);
clear_bit(ST_FLITE_IN_USE, &fimc->state);

- mutex_lock(&entity->parent->graph_mutex);
+ mutex_lock(&entity->graph_obj.mdev->graph_mutex);
entity->use_count--;
- mutex_unlock(&entity->parent->graph_mutex);
+ mutex_unlock(&entity->graph_obj.mdev->graph_mutex);
}

_vb2_fop_release(file, NULL);
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 3ba76940eef5..92dbade2fffc 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1046,7 +1046,7 @@ static int __fimc_md_modify_pipeline(struct media_entity *entity, bool enable)
return ret;
}

-/* Locking: called with entity->parent->graph_mutex mutex held. */
+/* Locking: called with entity->graph_obj.mdev->graph_mutex mutex held. */
static int __fimc_md_modify_pipelines(struct media_entity *entity, bool enable)
{
struct media_entity *entity_err = entity;
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 03214541f149..9a69913b31cb 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -164,8 +164,8 @@ struct fimc_sensor_info *source_to_sensor_info(struct fimc_source_info *si)

static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me)
{
- return me->parent == NULL ? NULL :
- container_of(me->parent, struct fimc_md, media_dev);
+ return me->graph_obj.mdev == NULL ? NULL :
+ container_of(me->graph_obj.mdev, struct fimc_md, media_dev);
}

static inline struct fimc_md *notifier_to_fimc_md(struct v4l2_async_notifier *n)
@@ -175,12 +175,12 @@ static inline struct fimc_md *notifier_to_fimc_md(struct v4l2_async_notifier *n)

static inline void fimc_md_graph_lock(struct exynos_video_entity *ve)
{
- mutex_lock(&ve->vdev.entity.parent->graph_mutex);
+ mutex_lock(&ve->vdev.entity.graph_obj.mdev->graph_mutex);
}

static inline void fimc_md_graph_unlock(struct exynos_video_entity *ve)
{
- mutex_unlock(&ve->vdev.entity.parent->graph_mutex);
+ mutex_unlock(&ve->vdev.entity.graph_obj.mdev->graph_mutex);
}

int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on);
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 6351f35b0a65..aa13b17d19a0 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -787,7 +787,7 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity, int use)
int change = use ? 1 : -1;
int ret;

- mutex_lock(&entity->parent->graph_mutex);
+ mutex_lock(&entity->graph_obj.mdev->graph_mutex);

/* Apply use count to node. */
entity->use_count += change;
@@ -798,7 +798,7 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity, int use)
if (ret < 0)
entity->use_count -= change;

- mutex_unlock(&entity->parent->graph_mutex);
+ mutex_unlock(&entity->graph_obj.mdev->graph_mutex);

return ret;
}
diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 6c89dc40df85..4c367352b1f7 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -226,7 +226,7 @@ static int isp_video_get_graph_data(struct isp_video *video,
{
struct media_entity_graph graph;
struct media_entity *entity = &video->video.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
struct isp_video *far_end = NULL;

mutex_lock(&mdev->graph_mutex);
diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c
index debe4e539df6..1f94c1a54e00 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -409,7 +409,7 @@ static int vsp1_pipeline_validate(struct vsp1_pipeline *pipe,
{
struct media_entity_graph graph;
struct media_entity *entity = &video->video.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
unsigned int i;
int ret;

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index f7f9aa353a55..92e8116dc28f 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -181,7 +181,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
{
struct media_entity_graph graph;
struct media_entity *entity = &start->video.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
unsigned int num_inputs = 0;
unsigned int num_outputs = 0;

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 61a8d5beff58..92573fa852a9 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -130,7 +130,7 @@ __vpfe_video_get_format(struct vpfe_video_device *video,
static void vpfe_prepare_pipeline(struct vpfe_video_device *video)
{
struct media_entity *entity = &video->video_dev.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
struct vpfe_pipeline *pipe = &video->pipe;
struct vpfe_video_device *far_end = NULL;
struct media_entity_graph graph;
@@ -288,7 +288,7 @@ static int vpfe_pipeline_enable(struct vpfe_pipeline *pipe)
else
entity = &pipe->inputs[0]->video_dev.entity;

- mdev = entity->parent;
+ mdev = entity->graph_obj.mdev;
mutex_lock(&mdev->graph_mutex);
media_entity_graph_walk_start(&graph, entity);
while ((entity = media_entity_graph_walk_next(&graph))) {
@@ -328,7 +328,7 @@ static int vpfe_pipeline_disable(struct vpfe_pipeline *pipe)
else
entity = &pipe->inputs[0]->video_dev.entity;

- mdev = entity->parent;
+ mdev = entity->graph_obj.mdev;
mutex_lock(&mdev->graph_mutex);
media_entity_graph_walk_start(&graph, entity);

diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c
index 7226553ceb2f..40591963b42b 100644
--- a/drivers/staging/media/omap4iss/iss.c
+++ b/drivers/staging/media/omap4iss/iss.c
@@ -494,7 +494,7 @@ int omap4iss_pipeline_pm_use(struct media_entity *entity, int use)
int change = use ? 1 : -1;
int ret;

- mutex_lock(&entity->parent->graph_mutex);
+ mutex_lock(&entity->graph_obj.mdev->graph_mutex);

/* Apply use count to node. */
entity->use_count += change;
@@ -505,7 +505,7 @@ int omap4iss_pipeline_pm_use(struct media_entity *entity, int use)
if (ret < 0)
entity->use_count -= change;

- mutex_unlock(&entity->parent->graph_mutex);
+ mutex_unlock(&entity->graph_obj.mdev->graph_mutex);

return ret;
}
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 25e9e7a6b99d..45a3f2d778fc 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -207,7 +207,7 @@ iss_video_far_end(struct iss_video *video)
{
struct media_entity_graph graph;
struct media_entity *entity = &video->video.entity;
- struct media_device *mdev = entity->parent;
+ struct media_device *mdev = entity->graph_obj.mdev;
struct iss_video *far_end = NULL;

mutex_lock(&mdev->graph_mutex);
--
2.4.3

2015-08-19 15:37:07

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 4/4] [media] media: remove media entity .parent field

Now that the struct media_entity .parent field is unused, it can be
safely removed. Since all the previous users were converted to use
the .mdev field from the embedded struct media_gobj instead.

Suggested-by: Mauro Carvalho Chehab <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>

---

include/media/media-entity.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 3b653e9321f2..d7e007f624a5 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -103,7 +103,6 @@ struct media_entity_operations {
struct media_entity {
struct media_gobj graph_obj;
struct list_head list;
- struct media_device *parent; /* Media device this entity belongs to*/
const char *name; /* Entity name */
u32 type; /* Entity type (MEDIA_ENT_T_*) */
u32 revision; /* Entity revision, driver specific */
--
2.4.3

2015-08-19 16:58:46

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 0/4] [media] Media entity cleanups and build fixes

Hi Javier,

Em Wed, 19 Aug 2015 17:35:18 +0200
Javier Martinez Canillas <[email protected]> escreveu:

> Hello,
>
> This series contains a couple of build fixes and cleanups for the
> Media Controller framework. The goal of the series is to get rid of
> the struct media_entity .parent member since now that a media_gobj is
> embedded into entities, the media_gobj .mdev member can be used to
> store a pointer to the parent struct media_device.
>
> So the .parent field becomes redundant and can be removed after all
> the users are converted to use entity .graph_obj.mdev instead.
>
> Patches 1/4 and 2/4 are build fixes I found while build testing if no
> regressions were introduced by the conversion.

Crap. Those omap drivers depend on some sub-arch specific DMA functions,
and can't use COMPILE_TEST. I hope some day someone would fix it,
in order to avoid troubles like that.

> Patch 3/4 converts
> all the drivers and the MC core to use .mdev instead of .parent and
> finally patch 4/4 removes the .parent field since now is unused.

Thanks for that!
All patches look good on my eyes.
>
> The series depend on Mauro's "[PATCH v6 0/8] MC preparation patches
> series" [0].

I'll add those patches on my experimental branch together with the
other patches that belong to the MC next gen rework.

>
> The transformation were automated using a coccinelle semantic patch
> and the drivers were build tested using allyesconfig and x-building
> the ARM Exynos and OMAP defconfigs + the needed media config options.
>
> Best regards,
> Javier
>
> [0]: http://www.mail-archive.com/[email protected]/msg91330.html
>
>
> Javier Martinez Canillas (4):
> [media] staging: omap4iss: get entity ID using media_entity_id()
> [media] omap3isp: get entity ID using media_entity_id()
> [media] media: use entity.graph_obj.mdev instead of .parent
> [media] media: remove media entity .parent field
>
> drivers/media/media-device.c | 8 ++---
> drivers/media/media-entity.c | 34 ++++++++++++----------
> drivers/media/platform/exynos4-is/fimc-isp-video.c | 6 ++--
> drivers/media/platform/exynos4-is/fimc-lite.c | 8 ++---
> drivers/media/platform/exynos4-is/media-dev.c | 2 +-
> drivers/media/platform/exynos4-is/media-dev.h | 8 ++---
> drivers/media/platform/omap3isp/isp.c | 11 ++++---
> drivers/media/platform/omap3isp/ispccdc.c | 2 +-
> drivers/media/platform/omap3isp/ispvideo.c | 10 ++++---
> drivers/media/platform/vsp1/vsp1_video.c | 2 +-
> drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
> drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 ++--
> drivers/staging/media/omap4iss/iss.c | 6 ++--
> drivers/staging/media/omap4iss/iss_video.c | 4 +--
> include/media/media-entity.h | 1 -
> 15 files changed, 58 insertions(+), 52 deletions(-)
>

Thanks and regards,
Mauro

2015-08-20 12:44:26

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 0/4] [media] Media entity cleanups and build fixes

On 08/19/15 17:35, Javier Martinez Canillas wrote:
> Hello,
>
> This series contains a couple of build fixes and cleanups for the
> Media Controller framework. The goal of the series is to get rid of
> the struct media_entity .parent member since now that a media_gobj is
> embedded into entities, the media_gobj .mdev member can be used to
> store a pointer to the parent struct media_device.
>
> So the .parent field becomes redundant and can be removed after all
> the users are converted to use entity .graph_obj.mdev instead.
>
> Patches 1/4 and 2/4 are build fixes I found while build testing if no
> regressions were introduced by the conversion. Patch 3/4 converts
> all the drivers and the MC core to use .mdev instead of .parent and
> finally patch 4/4 removes the .parent field since now is unused.

Regarding patches 1 and 2: these should of course be merged with Mauro's
patches that make this particular change (patch 3/8), otherwise it would
break git bisect.

Anyway,

Acked-by: Hans Verkuil <[email protected]> for the changes in patch
1 and 2, as long as they are added to Mauro's patch 3/8.

Regards,

Hans

>
> The series depend on Mauro's "[PATCH v6 0/8] MC preparation patches
> series" [0].
>
> The transformation were automated using a coccinelle semantic patch
> and the drivers were build tested using allyesconfig and x-building
> the ARM Exynos and OMAP defconfigs + the needed media config options.
>
> Best regards,
> Javier
>
> [0]: http://www.mail-archive.com/[email protected]/msg91330.html
>
>
> Javier Martinez Canillas (4):
> [media] staging: omap4iss: get entity ID using media_entity_id()
> [media] omap3isp: get entity ID using media_entity_id()
> [media] media: use entity.graph_obj.mdev instead of .parent
> [media] media: remove media entity .parent field
>
> drivers/media/media-device.c | 8 ++---
> drivers/media/media-entity.c | 34 ++++++++++++----------
> drivers/media/platform/exynos4-is/fimc-isp-video.c | 6 ++--
> drivers/media/platform/exynos4-is/fimc-lite.c | 8 ++---
> drivers/media/platform/exynos4-is/media-dev.c | 2 +-
> drivers/media/platform/exynos4-is/media-dev.h | 8 ++---
> drivers/media/platform/omap3isp/isp.c | 11 ++++---
> drivers/media/platform/omap3isp/ispccdc.c | 2 +-
> drivers/media/platform/omap3isp/ispvideo.c | 10 ++++---
> drivers/media/platform/vsp1/vsp1_video.c | 2 +-
> drivers/media/platform/xilinx/xilinx-dma.c | 2 +-
> drivers/staging/media/davinci_vpfe/vpfe_video.c | 6 ++--
> drivers/staging/media/omap4iss/iss.c | 6 ++--
> drivers/staging/media/omap4iss/iss_video.c | 4 +--
> include/media/media-entity.h | 1 -
> 15 files changed, 58 insertions(+), 52 deletions(-)
>

2015-08-20 12:45:50

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 3/4] [media] media: use entity.graph_obj.mdev instead of .parent

On 08/19/15 17:35, Javier Martinez Canillas wrote:
> The struct media_entity has a .parent field that stores a pointer
> to the parent struct media_device. But recently a media_gobj was
> embedded into the entities and since struct media_gojb already has
> a pointer to a struct media_device in the .mdev field, the .parent
> field becomes redundant and can be removed.
>
> This patch replaces all the usage of .parent by .graph_obj.mdev so
> that field will become unused and can be removed on a later patch.
>
> No functional changes.
>
> The transformation was made using the following coccinelle spatch:
>
> @@
> struct media_entity *me;
> @@
>
> - me->parent
> + me->graph_obj.mdev
>
> @@
> struct media_entity *link;
> @@
>
> - link->source->entity->parent
> + link->source->entity->graph_obj.mdev
>
> @@
> struct exynos_video_entity *ve;
> @@
>
> - ve->vdev.entity.parent
> + ve->vdev.entity.graph_obj.mdev
>
> Suggested-by: Mauro Carvalho Chehab <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>

Acked-by: Hans Verkuil <[email protected]>

Regards,

Hans

2015-08-20 12:46:09

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 4/4] [media] media: remove media entity .parent field

On 08/19/15 17:35, Javier Martinez Canillas wrote:
> Now that the struct media_entity .parent field is unused, it can be
> safely removed. Since all the previous users were converted to use
> the .mdev field from the embedded struct media_gobj instead.
>
> Suggested-by: Mauro Carvalho Chehab <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>

Acked-by: Hans Verkuil <[email protected]>

>
> ---
>
> include/media/media-entity.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 3b653e9321f2..d7e007f624a5 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -103,7 +103,6 @@ struct media_entity_operations {
> struct media_entity {
> struct media_gobj graph_obj;
> struct list_head list;
> - struct media_device *parent; /* Media device this entity belongs to*/
> const char *name; /* Entity name */
> u32 type; /* Entity type (MEDIA_ENT_T_*) */
> u32 revision; /* Entity revision, driver specific */
>

2015-08-20 12:51:04

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 0/4] [media] Media entity cleanups and build fixes

Hello Hans,

On 08/20/2015 02:41 PM, Hans Verkuil wrote:
> On 08/19/15 17:35, Javier Martinez Canillas wrote:
>> Hello,
>>
>> This series contains a couple of build fixes and cleanups for the
>> Media Controller framework. The goal of the series is to get rid of
>> the struct media_entity .parent member since now that a media_gobj is
>> embedded into entities, the media_gobj .mdev member can be used to
>> store a pointer to the parent struct media_device.
>>
>> So the .parent field becomes redundant and can be removed after all
>> the users are converted to use entity .graph_obj.mdev instead.
>>
>> Patches 1/4 and 2/4 are build fixes I found while build testing if no
>> regressions were introduced by the conversion. Patch 3/4 converts
>> all the drivers and the MC core to use .mdev instead of .parent and
>> finally patch 4/4 removes the .parent field since now is unused.
>
> Regarding patches 1 and 2: these should of course be merged with Mauro's
> patches that make this particular change (patch 3/8), otherwise it would
> break git bisect.
>
> Anyway,
>
> Acked-by: Hans Verkuil <[email protected]> for the changes in patch

Thanks a lot for the acks.

> 1 and 2, as long as they are added to Mauro's patch 3/8.
>

Indeed, I completely agree that these should be squashed with
Mauro's patch to maintain git bisect-ability.

> Regards,
>
> Hans
>

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

2015-08-20 18:37:45

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

Hi Javier,

Thank you for the patch.

On Wednesday 19 August 2015 17:35:19 Javier Martinez Canillas wrote:
> The struct media_entity does not have an .id field anymore since
> now the entity ID is stored in the embedded struct media_gobj.
>
> This caused the omap4iss driver fail to build. Fix by using the
> media_entity_id() macro to obtain the entity ID.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>

This looks fine to me. The patch needs to be moved between Mauro's 1/8 and 2/8
patches to avoid breaking bisection with patch 3/8. I'd squash this patch and
2/4 into a single "media: Use media_entity_id() in drivers" patch.

> ---
>
> drivers/staging/media/omap4iss/iss.c | 2 +-
> drivers/staging/media/omap4iss/iss_video.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c index f32ab7b98ae2..7226553ceb2f
> 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -607,7 +607,7 @@ static int iss_pipeline_disable(struct iss_pipeline
> *pipe, * crashed. Mark it as such, the ISS will be reset when
> * applications will release it.
> */
> - iss->crashed |= 1U << subdev->entity.id;
> + iss->crashed |= 1U << media_entity_id(&subdev->entity);
> failure = -ETIMEDOUT;
> }
> }
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> bae67742706f..25e9e7a6b99d 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -784,7 +784,7 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) entity = &video->video.entity;
> media_entity_graph_walk_start(&graph, entity);
> while ((entity = media_entity_graph_walk_next(&graph)))
> - pipe->entities |= 1 << entity->id;
> + pipe->entities |= 1 << media_entity_id(entity);
>
> /* Verify that the currently configured format matches the output of
> * the connected subdev.

--
Regards,

Laurent Pinchart

2015-08-21 00:14:14

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

Hello Laurent,

On 08/20/2015 08:37 PM, Laurent Pinchart wrote:
> Hi Javier,
>
> Thank you for the patch.
>
> On Wednesday 19 August 2015 17:35:19 Javier Martinez Canillas wrote:
>> The struct media_entity does not have an .id field anymore since
>> now the entity ID is stored in the embedded struct media_gobj.
>>
>> This caused the omap4iss driver fail to build. Fix by using the
>> media_entity_id() macro to obtain the entity ID.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
> This looks fine to me. The patch needs to be moved between Mauro's 1/8 and 2/8
> patches to avoid breaking bisection with patch 3/8. I'd squash this patch and
> 2/4 into a single "media: Use media_entity_id() in drivers" patch.
>

Yes, Hans and Mauro already mentioned it and I completely agree that
should be squashed with Mauro's patch to maintain git bisect-ability.

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

2015-08-21 00:15:32

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

Hi Javier,

On Friday 21 August 2015 02:14:05 Javier Martinez Canillas wrote:
> On 08/20/2015 08:37 PM, Laurent Pinchart wrote:
> > On Wednesday 19 August 2015 17:35:19 Javier Martinez Canillas wrote:
> >> The struct media_entity does not have an .id field anymore since
> >> now the entity ID is stored in the embedded struct media_gobj.
> >>
> >> This caused the omap4iss driver fail to build. Fix by using the
> >> media_entity_id() macro to obtain the entity ID.
> >>
> >> Signed-off-by: Javier Martinez Canillas <[email protected]>
> >
> > This looks fine to me. The patch needs to be moved between Mauro's 1/8 and
> > 2/8 patches to avoid breaking bisection with patch 3/8. I'd squash this
> > patch and 2/4 into a single "media: Use media_entity_id() in drivers"
> > patch.
>
> Yes, Hans and Mauro already mentioned it and I completely agree that
> should be squashed with Mauro's patch to maintain git bisect-ability.

I wouldn't squash patches 1/4 and 2/4 into Mauro's 3/8 patch as Hans proposed,
but instead squashing them together into a single patch and move the result as
1.5/8 in Mauro's series.

--
Regards,

Laurent Pinchart

2015-08-21 00:31:53

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

Hello Laurent,

On 08/21/2015 02:15 AM, Laurent Pinchart wrote:
> Hi Javier,
>
> On Friday 21 August 2015 02:14:05 Javier Martinez Canillas wrote:
>> On 08/20/2015 08:37 PM, Laurent Pinchart wrote:
>>> On Wednesday 19 August 2015 17:35:19 Javier Martinez Canillas wrote:
>>>> The struct media_entity does not have an .id field anymore since
>>>> now the entity ID is stored in the embedded struct media_gobj.
>>>>
>>>> This caused the omap4iss driver fail to build. Fix by using the
>>>> media_entity_id() macro to obtain the entity ID.
>>>>
>>>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>>>
>>> This looks fine to me. The patch needs to be moved between Mauro's 1/8 and
>>> 2/8 patches to avoid breaking bisection with patch 3/8. I'd squash this
>>> patch and 2/4 into a single "media: Use media_entity_id() in drivers"
>>> patch.
>>
>> Yes, Hans and Mauro already mentioned it and I completely agree that
>> should be squashed with Mauro's patch to maintain git bisect-ability.
>
> I wouldn't squash patches 1/4 and 2/4 into Mauro's 3/8 patch as Hans proposed,
> but instead squashing them together into a single patch and move the result as
> 1.5/8 in Mauro's series.
>

I see. I don't mind either option tbh, I'm OK with whatever works better.

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

2015-08-21 07:50:31

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 1/4] [media] staging: omap4iss: get entity ID using media_entity_id()

On 08/21/2015 02:15 AM, Laurent Pinchart wrote:
> Hi Javier,
>
> On Friday 21 August 2015 02:14:05 Javier Martinez Canillas wrote:
>> On 08/20/2015 08:37 PM, Laurent Pinchart wrote:
>>> On Wednesday 19 August 2015 17:35:19 Javier Martinez Canillas wrote:
>>>> The struct media_entity does not have an .id field anymore since
>>>> now the entity ID is stored in the embedded struct media_gobj.
>>>>
>>>> This caused the omap4iss driver fail to build. Fix by using the
>>>> media_entity_id() macro to obtain the entity ID.
>>>>
>>>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>>>
>>> This looks fine to me. The patch needs to be moved between Mauro's 1/8 and
>>> 2/8 patches to avoid breaking bisection with patch 3/8. I'd squash this
>>> patch and 2/4 into a single "media: Use media_entity_id() in drivers"
>>> patch.
>>
>> Yes, Hans and Mauro already mentioned it and I completely agree that
>> should be squashed with Mauro's patch to maintain git bisect-ability.
>
> I wouldn't squash patches 1/4 and 2/4 into Mauro's 3/8 patch as Hans proposed,
> but instead squashing them together into a single patch and move the result as
> 1.5/8 in Mauro's series.
>

I agree with Laurent, this is a better solution.

Regards,

Hans