MC next generation patches

Instead of accessing directly entity.id, let's create a macro, as this field will be moved into a common struct later on.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +4 -4

index c55ab5029323..e429605ca2c3 100644
@@ -77,8 +77,8 @@ static struct media_entity *find_entity(struct media_device *mdev, u32 id)
        spin_lock(&mdev->lock);
 
        media_device_for_each_entity(entity, mdev) {
-               if ((entity->id == id && !next) ||
-                   (entity->id > id && next)) {
+               if (((media_entity_id(entity) == id) && !next) ||
+                   ((media_entity_id(entity) > id) && next)) {
                        spin_unlock(&mdev->lock);
                        return entity;
                }

@@ -104,7 +104,7 @@ static long media_device_enum_entities(struct media_device *mdev,
        if (ent == NULL)
                return -EINVAL;
 
-       u_ent.id = ent->id;
+       u_ent.id = media_entity_id(ent);
        if (ent->name)
                strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
        u_ent.type = ent->type;

@@ -122,7 +122,7 @@ static long media_device_enum_entities(struct media_device *mdev,
 static void media_device_kpad_to_upad(const struct media_pad *kpad,
                                      struct media_pad_desc *upad)
 {
-       upad->entity = kpad->entity->id;
+       upad->entity = media_entity_id(kpad->entity);
        upad->index = kpad->index;
        upad->flags = kpad->flags;
 }

drivers/media/media-entity.c +4 -4

index 949e5f92cbdc..cb0ac4e0dfa5 100644
@@ -140,10 +140,10 @@ void media_entity_graph_walk_start(struct media_entity_graph *graph,
        graph->stack[graph->top].entity = NULL;
        bitmap_zero(graph->entities, MEDIA_ENTITY_ENUM_MAX_ID);
 
-       if (WARN_ON(entity->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+       if (WARN_ON(media_entity_id(entity) >= MEDIA_ENTITY_ENUM_MAX_ID))
                return;
 
-       __set_bit(entity->id, graph->entities);
+       __set_bit(media_entity_id(entity), graph->entities);
        stack_push(graph, entity);
 }
 EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);

@@ -184,11 +184,11 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
 
                /* Get the entity in the other end of the link . */
                next = media_entity_other(entity, link);
-               if (WARN_ON(next->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+               if (WARN_ON(media_entity_id(next) >= MEDIA_ENTITY_ENUM_MAX_ID))
                        return NULL;
 
                /* Has the entity already been visited? */
-               if (__test_and_set_bit(next->id, graph->entities)) {
+               if (__test_and_set_bit(media_entity_id(next), graph->entities)) {
                        link_top(graph)++;
                        continue;
                }

drivers/media/platform/vsp1/vsp1_video.c +2 -2

index 17f08973f835..debe4e539df6 100644
@@ -352,10 +352,10 @@ static int vsp1_pipeline_validate_branch(struct vsp1_pipeline *pipe,
                        break;
 
                /* Ensure the branch has no loop. */
-               if (entities & (1 << entity->subdev.entity.id))
+               if (entities & (1 << media_entity_id(&entity->subdev.entity)))
                        return -EPIPE;
 
-               entities |= 1 << entity->subdev.entity.id;
+               entities |= 1 << media_entity_id(&entity->subdev.entity);
 
                /* UDS can't be chained. */
                if (entity->type == VSP1_ENTITY_UDS) {

include/media/media-entity.h +0 -0

index 8b21a4d920d9..0a66fc225559 100644
@@ -113,6 +113,11 @@ static inline u32 media_entity_subtype(struct media_entity *entity)
        return entity->type & MEDIA_ENT_SUBTYPE_MASK;

Quoted 94 lines · Mauro Carvalho Chehab

Instead of accessing directly entity.id, let's create a macro, as this field will be moved into a common struct later on.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +4 -4

index c55ab5029323..e429605ca2c3 100644
@@ -77,8 +77,8 @@ static struct media_entity *find_entity(struct media_device *mdev, u32 id)
        spin_lock(&mdev->lock);
 
        media_device_for_each_entity(entity, mdev) {
-               if ((entity->id = id && !next) ||
-                   (entity->id > id && next)) {
+               if (((media_entity_id(entity) = id) && !next) ||
+                   ((media_entity_id(entity) > id) && next)) {
                        spin_unlock(&mdev->lock);
                        return entity;
                }

@@ -104,7 +104,7 @@ static long media_device_enum_entities(struct media_device *mdev,
        if (ent = NULL)
                return -EINVAL;
 
-       u_ent.id = ent->id;
+       u_ent.id = media_entity_id(ent);
        if (ent->name)
                strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
        u_ent.type = ent->type;

@@ -122,7 +122,7 @@ static long media_device_enum_entities(struct media_device *mdev,
 static void media_device_kpad_to_upad(const struct media_pad *kpad,
                                      struct media_pad_desc *upad)
 {
-       upad->entity = kpad->entity->id;
+       upad->entity = media_entity_id(kpad->entity);
        upad->index = kpad->index;
        upad->flags = kpad->flags;
 }

drivers/media/media-entity.c +4 -4

index 949e5f92cbdc..cb0ac4e0dfa5 100644
@@ -140,10 +140,10 @@ void media_entity_graph_walk_start(struct media_entity_graph *graph,
        graph->stack[graph->top].entity = NULL;
        bitmap_zero(graph->entities, MEDIA_ENTITY_ENUM_MAX_ID);
 
-       if (WARN_ON(entity->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+       if (WARN_ON(media_entity_id(entity) >= MEDIA_ENTITY_ENUM_MAX_ID))
                return;
 
-       __set_bit(entity->id, graph->entities);
+       __set_bit(media_entity_id(entity), graph->entities);
        stack_push(graph, entity);
 }
 EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);

@@ -184,11 +184,11 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
 
                /* Get the entity in the other end of the link . */
                next = media_entity_other(entity, link);
-               if (WARN_ON(next->id >= MEDIA_ENTITY_ENUM_MAX_ID))
+               if (WARN_ON(media_entity_id(next) >= MEDIA_ENTITY_ENUM_MAX_ID))
                        return NULL;
 
                /* Has the entity already been visited? */
-               if (__test_and_set_bit(next->id, graph->entities)) {
+               if (__test_and_set_bit(media_entity_id(next), graph->entities)) {
                        link_top(graph)++;
                        continue;
                }

drivers/media/platform/vsp1/vsp1_video.c +2 -2

index 17f08973f835..debe4e539df6 100644
@@ -352,10 +352,10 @@ static int vsp1_pipeline_validate_branch(struct vsp1_pipeline *pipe,
                        break;
 
                /* Ensure the branch has no loop. */
-               if (entities & (1 << entity->subdev.entity.id))
+               if (entities & (1 << media_entity_id(&entity->subdev.entity)))
                        return -EPIPE;
 
-               entities |= 1 << entity->subdev.entity.id;
+               entities |= 1 << media_entity_id(&entity->subdev.entity);
 
                /* UDS can't be chained. */
                if (entity->type = VSP1_ENTITY_UDS) {

include/media/media-entity.h +0 -0

index 8b21a4d920d9..0a66fc225559 100644
@@ -113,6 +113,11 @@ static inline u32 media_entity_subtype(struct media_entity *entity)
        return entity->type & MEDIA_ENT_SUBTYPE_MASK;

include/media/media-entity.h static inline u32 media_entity_subtype(struct media_entity *entity) · line 123

Quoted 10 lines

include/media/media-entity.h +5 -0

 }
 
+static inline u32 media_entity_id(struct media_entity *entity)
+{
+       return entity->id;
+}
+
 #define MEDIA_ENTITY_ENUM_MAX_DEPTH    16
 #define MEDIA_ENTITY_ENUM_MAX_ID       64
 

Could you please enclose the code in this file in CONFIG_MEDIA_CONTROLLER ifdef similar to what is done in media-device.h based on your comments.

It will help avoid using CONFIG_MEDIA_CONTROLLER in places where this code is used. We could create a task around cleaning up all the places that enclose the entity calls in CONFIG_MEDIA_CONTROLLER ifdef as a cleanup.

From: Javier Martinez Canillas <[email protected]>

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]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/staging/media/omap4iss/iss.c +1 -1

index 9bfb725b9986..e54a7afd31de 100644
@@ -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;
                }
        }

drivers/staging/media/omap4iss/iss_video.c +0 -0

index bae67742706f..25e9e7a6b99d 100644

From: Javier Martinez Canillas <[email protected]>

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]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/staging/media/omap4iss/iss.c +1 -1

index 9bfb725b9986..e54a7afd31de 100644
@@ -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;
 		}
 	}

drivers/staging/media/omap4iss/iss_video.c +1 -1

index bae67742706f..25e9e7a6b99d 100644
@@ -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.

From: Javier Martinez Canillas <[email protected]>

X-Patchwork-Delegate: [email protected] 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]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/platform/omap3isp/isp.c +1 -0

index 56e683b19a73..e08183f9d0f7 100644
@@ -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

drivers/media/platform/omap3isp/isp.c +1 -0

+                       }
                        failure = -ETIMEDOUT;
                }
        }

drivers/media/platform/omap3isp/ispccdc.c +1 -1

index 3b10304b580b..d96e3be5e252 100644
@@ -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;
        }

drivers/media/platform/omap3isp/ispvideo.c +5 -4

index 3094572f8897..6c89dc40df85 100644
@@ -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

Quoted 25 lines · Mauro Carvalho Chehab

From: Javier Martinez Canillas <[email protected]>

X-Patchwork-Delegate: [email protected] 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]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/platform/omap3isp/isp.c +1 -0

index 56e683b19a73..e08183f9d0f7 100644
@@ -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

drivers/media/platform/omap3isp/isp.c static int isp_pipeline_disable(struct isp_pipeline *pipe) · line 1033

Quoted 8 lines

drivers/media/platform/omap3isp/isp.c +3 -2

@@ -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;

Is there a reason why you need id defined here, unlike the cases below. Can you do

isp->crashed |= 1U << media_entity_id(&subdev->entity);

thanks,
-- Shuah

From: Javier Martinez Canillas <[email protected]>

X-Patchwork-Delegate: [email protected] 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]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/platform/omap3isp/isp.c +5 -2

index 56e683b19a73..e08183f9d0f7 100644
@@ -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;
 		}
 	}

drivers/media/platform/omap3isp/ispccdc.c +1 -1

index 3b10304b580b..d96e3be5e252 100644
@@ -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;
 	}

drivers/media/platform/omap3isp/ispvideo.c +5 -3

index 3094572f8897..6c89dc40df85 100644
@@ -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

Declare the interface types that will be used by the new G_TOPOLOGY ioctl that will be defined latter on.

For now, we need those types, as they'll be used on the internal structs associated with the new media_interface graph object defined on the next patch.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

include/uapi/linux/media.h +0 -0

index 4e816be3de39..21c96cd7a6ae 100644

include/uapi/linux/media.h +24 -0

+
+/* Interface types */
+
+#define MEDIA_INTF_T_DVB_FE    	(MEDIA_INTF_T_DVB_BASE)
+#define MEDIA_INTF_T_DVB_DEMUX  (MEDIA_INTF_T_DVB_BASE + 1)
+#define MEDIA_INTF_T_DVB_DVR    (MEDIA_INTF_T_DVB_BASE + 2)
+#define MEDIA_INTF_T_DVB_CA     (MEDIA_INTF_T_DVB_BASE + 3)
+#define MEDIA_INTF_T_DVB_NET    (MEDIA_INTF_T_DVB_BASE + 4)
+
+#define MEDIA_INTF_T_V4L_VIDEO  (MEDIA_INTF_T_V4L_BASE)
+#define MEDIA_INTF_T_V4L_VBI    (MEDIA_INTF_T_V4L_BASE + 1)
+#define MEDIA_INTF_T_V4L_RADIO  (MEDIA_INTF_T_V4L_BASE + 2)
+#define MEDIA_INTF_T_V4L_SUBDEV (MEDIA_INTF_T_V4L_BASE + 3)
+#define MEDIA_INTF_T_V4L_SWRADIO (MEDIA_INTF_T_V4L_BASE + 4)
+
+#define MEDIA_INTF_T_ALSA_PCM_CAPTURE   (MEDIA_INTF_T_ALSA_BASE)
+#define MEDIA_INTF_T_ALSA_PCM_PLAYBACK  (MEDIA_INTF_T_ALSA_BASE + 1)
+#define MEDIA_INTF_T_ALSA_CONTROL       (MEDIA_INTF_T_ALSA_BASE + 2)
+#define MEDIA_INTF_T_ALSA_COMPRESS      (MEDIA_INTF_T_ALSA_BASE + 3)
+#define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
+#define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)
+
+/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
+
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)

Declare the interface types that will be used by the new G_TOPOLOGY ioctl that will be defined latter on.

For now, we need those types, as they'll be used on the internal structs associated with the new media_interface graph object defined on the next patch.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

include/uapi/linux/media.h +16 -0

index 4e816be3de39..21c96cd7a6ae 100644
@@ -167,6 +167,35 @@ struct media_links_enum {
        __u32 reserved[4];
 };
 
+/* Interface type ranges */
+
+#define MEDIA_INTF_T_DVB_BASE  0x00000000
+#define MEDIA_INTF_T_V4L_BASE  0x00000100
+#define MEDIA_INTF_T_ALSA_BASE 0x00000200
+
+/* Interface types */
+
+#define MEDIA_INTF_T_DVB_FE            (MEDIA_INTF_T_DVB_BASE)
+#define MEDIA_INTF_T_DVB_DEMUX  (MEDIA_INTF_T_DVB_BASE + 1)
+#define MEDIA_INTF_T_DVB_DVR    (MEDIA_INTF_T_DVB_BASE + 2)
+#define MEDIA_INTF_T_DVB_CA     (MEDIA_INTF_T_DVB_BASE + 3)
+#define MEDIA_INTF_T_DVB_NET    (MEDIA_INTF_T_DVB_BASE + 4)
+
+#define MEDIA_INTF_T_V4L_VIDEO  (MEDIA_INTF_T_V4L_BASE)
+#define MEDIA_INTF_T_V4L_VBI    (MEDIA_INTF_T_V4L_BASE + 1)

Quoted 39 lines · Mauro Carvalho Chehab

Declare the interface types that will be used by the new G_TOPOLOGY ioctl that will be defined latter on.

For now, we need those types, as they'll be used on the internal structs associated with the new media_interface graph object defined on the next patch.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

include/uapi/linux/media.h +22 -0

index 4e816be3de39..21c96cd7a6ae 100644
@@ -167,6 +167,35 @@ struct media_links_enum {
        __u32 reserved[4];
 };
 
+/* Interface type ranges */
+
+#define MEDIA_INTF_T_DVB_BASE  0x00000000
+#define MEDIA_INTF_T_V4L_BASE  0x00000100
+#define MEDIA_INTF_T_ALSA_BASE 0x00000200
+
+/* Interface types */
+
+#define MEDIA_INTF_T_DVB_FE            (MEDIA_INTF_T_DVB_BASE)
+#define MEDIA_INTF_T_DVB_DEMUX  (MEDIA_INTF_T_DVB_BASE + 1)
+#define MEDIA_INTF_T_DVB_DVR    (MEDIA_INTF_T_DVB_BASE + 2)
+#define MEDIA_INTF_T_DVB_CA     (MEDIA_INTF_T_DVB_BASE + 3)
+#define MEDIA_INTF_T_DVB_NET    (MEDIA_INTF_T_DVB_BASE + 4)
+
+#define MEDIA_INTF_T_V4L_VIDEO  (MEDIA_INTF_T_V4L_BASE)
+#define MEDIA_INTF_T_V4L_VBI    (MEDIA_INTF_T_V4L_BASE + 1)
+#define MEDIA_INTF_T_V4L_RADIO  (MEDIA_INTF_T_V4L_BASE + 2)
+#define MEDIA_INTF_T_V4L_SUBDEV (MEDIA_INTF_T_V4L_BASE + 3)
+#define MEDIA_INTF_T_V4L_SWRADIO (MEDIA_INTF_T_V4L_BASE + 4)
+
+#define MEDIA_INTF_T_ALSA_PCM_CAPTURE   (MEDIA_INTF_T_ALSA_BASE)
+#define MEDIA_INTF_T_ALSA_PCM_PLAYBACK  (MEDIA_INTF_T_ALSA_BASE + 1)

include/uapi/linux/media.h struct media_links_enum { · line 195

Quoted 4 lines

include/uapi/linux/media.h +4 -0

+#define MEDIA_INTF_T_ALSA_CONTROL       (MEDIA_INTF_T_ALSA_BASE + 2)
+#define MEDIA_INTF_T_ALSA_COMPRESS      (MEDIA_INTF_T_ALSA_BASE + 3)
+#define MEDIA_INTF_T_ALSA_RAWMIDI       (MEDIA_INTF_T_ALSA_BASE + 4)
+#define MEDIA_INTF_T_ALSA_HWDEP         (MEDIA_INTF_T_ALSA_BASE + 5)

Is it necessary to add ALSA types at this time without ALSA media controller work?
Can these be added later when ALSA work is done.

thanks,
-- Shuah

Now that interfaces got created, we need to fix the entity namespace.

So, let's create a consistent new namespace and add backward compatibility macros to keep the old namespace preserved.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/dvb-core/dvbdev.c +13 -13

index 5a2bd03f5dc0..acada5ba9442 100644
@@ -229,17 +229,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
 
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_CA:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;

@@ -438,7 +438,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
 void dvb_create_media_graph(struct dvb_adapter *adap)
 {
 	struct media_device *mdev = adap->mdev;
-	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *entity, *tuner = NULL, *demod = NULL;
 	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
 	struct media_interface *intf;
 

@@ -450,26 +450,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
 		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
 			tuner = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_FE:
-			fe = entity;
+		case MEDIA_ENT_T_DVB_DEMOD:
+			demod = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+		case MEDIA_ENT_T_DVB_DEMUX:
 			demux = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+		case MEDIA_ENT_T_DVB_TSOUT:
 			dvr = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+		case MEDIA_ENT_T_DVB_CA:
 			ca = entity;
 			break;
 		}
 	}
 
-	if (tuner && fe)
-		media_create_pad_link(tuner, 0, fe, 0, 0);
+	if (tuner && demod)
+		media_create_pad_link(tuner, 0, demod, 0, 0);
 
-	if (fe && demux)
-		media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+	if (demod && demux)
+		media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && dvr)
 		media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);

include/uapi/linux/media.h +11 -0

index 21c96cd7a6ae..7306aeaff807 100644
@@ -42,31 +42,67 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
+/*
+ * Base numbers for entity types
+ *
+ * Please notice that the huge gap of 16 bits for each base is overkill!
+ * 8 bits is more than enough to avoid starving entity types for each
+ * subsystem.
+ *
+ * However, It is kept this way just to avoid binary breakages with the
+ * namespace provided on legacy versions of this header.
+ */
+#define MEDIA_ENT_T_DVB_BASE		0x00000000

include/uapi/linux/media.h +24 -12

+
+/* V4L2 Sub-device entities */
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
+	/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
+	/* Tuner entity is actually both V4L2 and DVB subdev */
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
+
+/* DVB entities */
+#define MEDIA_ENT_T_DVB_DEMOD		(MEDIA_ENT_T_DVB_BASE + 4)
+#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 5)
+#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 6)
+#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 7)
+#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 8)
+
+/* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000
 #define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
 
-#define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
+#define MEDIA_ENT_T_DEVNODE		MEDIA_ENT_T_V4L2_BASE
+#define MEDIA_ENT_T_V4L2_SUBDEV		MEDIA_ENT_T_V4L2_SUBDEV_BASE
+
+#define MEDIA_ENT_T_DEVNODE_V4L		MEDIA_ENT_T_V4L2_VIDEO
+
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
-#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
-#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
-#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
-#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
 
-/* Legacy symbol. Use it to avoid userspace compilation breakages */
-#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
-
-#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)

Quoted 97 lines

Now that interfaces got created, we need to fix the entity namespace.

So, let's create a consistent new namespace and add backward compatibility macros to keep the old namespace preserved.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/dvb-core/dvbdev.c +13 -13

index 5a2bd03f5dc0..acada5ba9442 100644
@@ -229,17 +229,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
 
 	switch (type) {
 	case DVB_DEVICE_FRONTEND:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_DEMUX:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;
 	case DVB_DEVICE_CA:
-		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
+		dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
 		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
 		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
 		break;

@@ -438,7 +438,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
 void dvb_create_media_graph(struct dvb_adapter *adap)
 {
 	struct media_device *mdev = adap->mdev;
-	struct media_entity *entity, *tuner = NULL, *fe = NULL;
+	struct media_entity *entity, *tuner = NULL, *demod = NULL;
 	struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
 	struct media_interface *intf;
 

@@ -450,26 +450,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
 		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
 			tuner = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_FE:
-			fe = entity;
+		case MEDIA_ENT_T_DVB_DEMOD:
+			demod = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
+		case MEDIA_ENT_T_DVB_DEMUX:
 			demux = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
+		case MEDIA_ENT_T_DVB_TSOUT:
 			dvr = entity;
 			break;
-		case MEDIA_ENT_T_DEVNODE_DVB_CA:
+		case MEDIA_ENT_T_DVB_CA:
 			ca = entity;
 			break;
 		}
 	}
 
-	if (tuner && fe)
-		media_create_pad_link(tuner, 0, fe, 0, 0);
+	if (tuner && demod)
+		media_create_pad_link(tuner, 0, demod, 0, 0);
 
-	if (fe && demux)
-		media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
+	if (demod && demux)
+		media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
 
 	if (demux && dvr)
 		media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);

include/uapi/linux/media.h +11 -0

index 21c96cd7a6ae..7306aeaff807 100644
@@ -42,31 +42,67 @@ struct media_device_info {
 
 #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
 
+/*
+ * Base numbers for entity types
+ *
+ * Please notice that the huge gap of 16 bits for each base is overkill!
+ * 8 bits is more than enough to avoid starving entity types for each
+ * subsystem.
+ *
+ * However, It is kept this way just to avoid binary breakages with the
+ * namespace provided on legacy versions of this header.
+ */
+#define MEDIA_ENT_T_DVB_BASE		0x00000000

include/uapi/linux/media.h struct media_device_info { · line 65

Quoted 10 lines

include/uapi/linux/media.h +10 -0

+#define MEDIA_ENT_T_V4L2_BASE		0x00010000
+#define MEDIA_ENT_T_V4L2_SUBDEV_BASE	0x00020000
+
+/* V4L2 entities */
+#define MEDIA_ENT_T_V4L2_VIDEO		(MEDIA_ENT_T_V4L2_BASE + 1)
+	/*
+	 * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
+	 * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
+	 * to be declared for FB, ALSA and DVB entities.
+	 * As those values were never atually used in practice, we're just

s/atually/actually/

include/uapi/linux/media.h struct media_device_info { · line 66

Quoted 1 line

include/uapi/linux/media.h +1 -0

+	 * adding them as backward compatibily macros and keeping the

s/compatibily/compatibility/

include/uapi/linux/media.h struct media_device_info { · line 67

Quoted 1 line

include/uapi/linux/media.h +1 -0

+	 * numberspace cleaned here. This way, we avoid breaking compilation,

s/cleaned/clean/

include/uapi/linux/media.h struct media_device_info { · line 73

Quoted 6 lines

include/uapi/linux/media.h +6 -0

+	 * in the case of having some userspace application using the old
+	 * symbols.
+	 */
+#define MEDIA_ENT_T_V4L2_VBI		(MEDIA_ENT_T_V4L2_BASE + 5)
+#define MEDIA_ENT_T_V4L2_RADIO		(MEDIA_ENT_T_V4L2_BASE + 6)
+#define MEDIA_ENT_T_V4L2_SWRADIO	(MEDIA_ENT_T_V4L2_BASE + 7)

Why are these entities? Aren't these interface types?

Quoted 45 lines

include/uapi/linux/media.h +24 -12

+
+/* V4L2 Sub-device entities */
+#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
+#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
+#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
+	/* A converter of analogue video to its digital representation. */
+#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
+	/* Tuner entity is actually both V4L2 and DVB subdev */
+#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
+
+/* DVB entities */
+#define MEDIA_ENT_T_DVB_DEMOD		(MEDIA_ENT_T_DVB_BASE + 4)
+#define MEDIA_ENT_T_DVB_DEMUX		(MEDIA_ENT_T_DVB_BASE + 5)
+#define MEDIA_ENT_T_DVB_TSOUT		(MEDIA_ENT_T_DVB_BASE + 6)
+#define MEDIA_ENT_T_DVB_CA		(MEDIA_ENT_T_DVB_BASE + 7)
+#define MEDIA_ENT_T_DVB_NET_DECAP	(MEDIA_ENT_T_DVB_BASE + 8)
+
+/* Legacy symbols used to avoid userspace compilation breakages */
 #define MEDIA_ENT_TYPE_SHIFT		16
 #define MEDIA_ENT_TYPE_MASK		0x00ff0000
 #define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
 
-#define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
+#define MEDIA_ENT_T_DEVNODE		MEDIA_ENT_T_V4L2_BASE
+#define MEDIA_ENT_T_V4L2_SUBDEV		MEDIA_ENT_T_V4L2_SUBDEV_BASE
+
+#define MEDIA_ENT_T_DEVNODE_V4L		MEDIA_ENT_T_V4L2_VIDEO
+
 #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
 #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
-#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
-#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
-#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
-#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
-#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
+#define MEDIA_ENT_T_DEVNODE_DVB		(MEDIA_ENT_T_DEVNODE + 4)
 
-/* Legacy symbol. Use it to avoid userspace compilation breakages */
-#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
-
-#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
-#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)

include/uapi/linux/media.h struct media_device_info { · line 108

Quoted 10 lines

include/uapi/linux/media.h +1 -6

-#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV + 2)
-#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV + 3)
-/* A converter of analogue video to its digital representation. */
-#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
-
-#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
+/* Entity types */
 
 #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
 

Hmm, I'm postponing further review. It might become clearer after reviewing more of this patch series.

One reason why this is a bit difficult to review is that it is not immediately obvious which defines are here for backwards compat (and shouldn't be used in the kernel anymore) and which defines are new.

May I suggest that either in this or in a later patch the defines that shouldn't be used in the kernel should be placed under #ifndef __KERNEL__?

Due to the MC API proposed changes, we'll need to have an unique object ID for all graph objects, and have some shared fields that will be common on all media graph objects.

Right now, the only common object is the object ID, but other fields will be added later on.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-entity.c +9 -0

index cb0ac4e0dfa5..4834172bf6f8 100644
@@ -27,6 +27,38 @@
 #include <media/media-device.h>
 
 /**
+ *  media_gobj_init - Initialize a graph object
+ *
+ * @mdev:      Pointer to the media_device that contains the object
+ * @type:      Type of the object
+ * @gobj:      Pointer to the object
+ *
+ * This routine initializes the embedded struct media_gobj inside a
+ * media graph object. It is called automatically if media_*_create()
+ * calls are used. However, if the object (entity, link, pad, interface)

drivers/media/media-entity.c +13 -0

+/**
+ *  media_gobj_remove - Stop using a graph object on a media device
+ *
+ * @graph_obj: Pointer to the object
+ *
+ * This should be called at media_device_unregister_*() routines
+ */
+void media_gobj_remove(struct media_gobj *gobj)
+{
+       /* For now, nothing to do */
+}
+
+/**
  * media_entity_init - Initialize a media entity
  *
  * @num_pads: Total number of sink and source pads.

include/media/media-entity.h +61 -1

index 0a66fc225559..b1854239a476 100644
@@ -28,6 +28,39 @@
 #include <linux/list.h>
 #include <linux/media.h>
 
+/* Enums used internally at the media controller to represent graphs */
+
+/**
+ * enum media_gobj_type - type of a graph object
+ *
+ */
+enum media_gobj_type {
+        /* FIXME: add the types here, as we embed media_gobj */
+       MEDIA_GRAPH_NONE
+};
+
+#define MEDIA_BITS_PER_TYPE            8
+#define MEDIA_BITS_PER_LOCAL_ID                (32 - MEDIA_BITS_PER_TYPE)
+#define MEDIA_LOCAL_ID_MASK             GENMASK(MEDIA_BITS_PER_LOCAL_ID - 1, 0)
+
+/* Structs to represent the objects that belong to a media graph */
+
+/**
+ * struct media_gobj - Define a graph object.
+ *
+ * @id:                Non-zero object ID identifier. The ID should be unique
+ *             inside a media_device, as it is composed by
+ *             MEDIA_BITS_PER_TYPE to store the type plus
+ *             MEDIA_BITS_PER_LOCAL_ID to store a per-type ID
+ *             (called as "local ID").
+ *
+ * All objects on the media graph should have this struct embedded
+ */
+struct media_gobj {
+       u32                     id;
+};
+
+
 struct media_pipeline {
 };
 

@@ -118,6 +151,26 @@ static inline u32 media_entity_id(struct media_entity *entity)
        return entity->id;
 }
 
+static inline enum media_gobj_type media_type(struct media_gobj *gobj)
+{
+       return gobj->id >> MEDIA_BITS_PER_LOCAL_ID;
+}
+
+static inline u32 media_localid(struct media_gobj *gobj)
+{
+       return gobj->id & MEDIA_LOCAL_ID_MASK;
+}
+
+static inline u32 media_gobj_gen_id(enum media_gobj_type type, u32 local_id)
+{
+       u32 id;
+
+       id = type << MEDIA_BITS_PER_LOCAL_ID;
+       id |= local_id & MEDIA_LOCAL_ID_MASK;
+
+       return id;
+}
+
 #define MEDIA_ENTITY_ENUM_MAX_DEPTH    16
 #define MEDIA_ENTITY_ENUM_MAX_ID       64
 

@@ -131,6 +184,14 @@ struct media_entity_graph {
        int top;
 };
 
+#define gobj_to_entity(gobj) \
+               container_of(gobj, struct media_entity, graph_obj)
+
+void media_gobj_init(struct media_device *mdev,
+                   enum media_gobj_type type,
+                   struct media_gobj *gobj);
+void media_gobj_remove(struct media_gobj *gobj);
+
 int media_entity_init(struct media_entity *entity, u16 num_pads,
                struct media_pad *pads);
 void media_entity_cleanup(struct media_entity *entity);
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 33 lines · Mauro Carvalho Chehab

Due to the MC API proposed changes, we'll need to have an unique object ID for all graph objects, and have some shared fields that will be common on all media graph objects.

Right now, the only common object is the object ID, but other fields will be added later on.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-entity.c +14 -0

index cb0ac4e0dfa5..4834172bf6f8 100644
@@ -27,6 +27,38 @@
 #include <media/media-device.h>
 
 /**
+ *  media_gobj_init - Initialize a graph object
+ *
+ * @mdev:      Pointer to the media_device that contains the object
+ * @type:      Type of the object
+ * @gobj:      Pointer to the object
+ *
+ * This routine initializes the embedded struct media_gobj inside a
+ * media graph object. It is called automatically if media_*_create()
+ * calls are used. However, if the object (entity, link, pad, interface)
+ * is embedded on some other object, this function should be called before
+ * registering the object at the media controller.
+ */
+void media_gobj_init(struct media_device *mdev,
+                          enum media_gobj_type type,

drivers/media/media-entity.c line 48

Quoted 5 lines

drivers/media/media-entity.c +5 -0

+                          struct media_gobj *gobj)
+{
+       /* For now, nothing to do */
+}
+

This patch is mainly adding skeleton framework. Does this patch series implement the media_gobj_init() and media_gobj_remove()?

If so, would it make sense to combine the implementation and the stubs?

thanks,
-- Shuah

Quoted 105 lines

drivers/media/media-entity.c +13 -0

+/**
+ *  media_gobj_remove - Stop using a graph object on a media device
+ *
+ * @graph_obj: Pointer to the object
+ *
+ * This should be called at media_device_unregister_*() routines
+ */
+void media_gobj_remove(struct media_gobj *gobj)
+{
+       /* For now, nothing to do */
+}
+
+/**
  * media_entity_init - Initialize a media entity
  *
  * @num_pads: Total number of sink and source pads.

include/media/media-entity.h +61 -1

index 0a66fc225559..b1854239a476 100644
@@ -28,6 +28,39 @@
 #include <linux/list.h>
 #include <linux/media.h>
 
+/* Enums used internally at the media controller to represent graphs */
+
+/**
+ * enum media_gobj_type - type of a graph object
+ *
+ */
+enum media_gobj_type {
+        /* FIXME: add the types here, as we embed media_gobj */
+       MEDIA_GRAPH_NONE
+};
+
+#define MEDIA_BITS_PER_TYPE            8
+#define MEDIA_BITS_PER_LOCAL_ID                (32 - MEDIA_BITS_PER_TYPE)
+#define MEDIA_LOCAL_ID_MASK             GENMASK(MEDIA_BITS_PER_LOCAL_ID - 1, 0)
+
+/* Structs to represent the objects that belong to a media graph */
+
+/**
+ * struct media_gobj - Define a graph object.
+ *
+ * @id:                Non-zero object ID identifier. The ID should be unique
+ *             inside a media_device, as it is composed by
+ *             MEDIA_BITS_PER_TYPE to store the type plus
+ *             MEDIA_BITS_PER_LOCAL_ID to store a per-type ID
+ *             (called as "local ID").
+ *
+ * All objects on the media graph should have this struct embedded
+ */
+struct media_gobj {
+       u32                     id;
+};
+
+
 struct media_pipeline {
 };
 

@@ -118,6 +151,26 @@ static inline u32 media_entity_id(struct media_entity *entity)
        return entity->id;
 }
 
+static inline enum media_gobj_type media_type(struct media_gobj *gobj)
+{
+       return gobj->id >> MEDIA_BITS_PER_LOCAL_ID;
+}
+
+static inline u32 media_localid(struct media_gobj *gobj)
+{
+       return gobj->id & MEDIA_LOCAL_ID_MASK;
+}
+
+static inline u32 media_gobj_gen_id(enum media_gobj_type type, u32 local_id)
+{
+       u32 id;
+
+       id = type << MEDIA_BITS_PER_LOCAL_ID;
+       id |= local_id & MEDIA_LOCAL_ID_MASK;
+
+       return id;
+}
+
 #define MEDIA_ENTITY_ENUM_MAX_DEPTH    16
 #define MEDIA_ENTITY_ENUM_MAX_ID       64
 

@@ -131,6 +184,14 @@ struct media_entity_graph {
        int top;
 };
 
+#define gobj_to_entity(gobj) \
+               container_of(gobj, struct media_entity, graph_obj)
+
+void media_gobj_init(struct media_device *mdev,
+                   enum media_gobj_type type,
+                   struct media_gobj *gobj);
+void media_gobj_remove(struct media_gobj *gobj);
+
 int media_entity_init(struct media_entity *entity, u16 num_pads,
                struct media_pad *pads);
 void media_entity_cleanup(struct media_entity *entity);
--

2.4.3

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

As entities are graph objects, let's embed media_gobj on it. That ensures an unique ID for entities that can be global along the entire media controller.

For now, we'll keep the already existing entity ID. Such field need to be dropped at some point, but for now, let's not do this, to avoid needing to review all drivers and the userspace apps.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +3 -5

index e429605ca2c3..81d6a130efef 100644
@@ -379,7 +379,6 @@ int __must_check __media_device_register(struct media_device *mdev,
        if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0))
                return -EINVAL;
 
-       mdev->entity_id = 1;
        INIT_LIST_HEAD(&mdev->entities);
        spin_lock_init(&mdev->lock);
        mutex_init(&mdev->graph_mutex);

@@ -433,10 +432,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
        entity->parent = mdev;
 
        spin_lock(&mdev->lock);
-       if (entity->id == 0)
-               entity->id = mdev->entity_id++;
-       else
-               mdev->entity_id = max(entity->id + 1, mdev->entity_id);
+       /* Initialize media_gobj embedded at the entity */
+       media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
        list_add_tail(&entity->list, &mdev->entities);
        spin_unlock(&mdev->lock);
 

@@ -459,6 +456,7 @@ void media_device_unregister_entity(struct media_entity *entity)
                return;
 
        spin_lock(&mdev->lock);
+       media_gobj_remove(&entity->graph_obj);
        list_del(&entity->list);
        spin_unlock(&mdev->lock);
        entity->parent = NULL;

drivers/media/media-entity.c +0 -0

index 4834172bf6f8..888cb88e19bf 100644
@@ -43,7 +43,12 @@ void media_gobj_init(struct media_device *mdev,
                           enum media_gobj_type type,

Quoted 52 lines · Mauro Carvalho Chehab

As entities are graph objects, let's embed media_gobj on it. That ensures an unique ID for entities that can be global along the entire media controller.

For now, we'll keep the already existing entity ID. Such field need to be dropped at some point, but for now, let's not do this, to avoid needing to review all drivers and the userspace apps.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +3 -5

index e429605ca2c3..81d6a130efef 100644
@@ -379,7 +379,6 @@ int __must_check __media_device_register(struct media_device *mdev,
        if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0))
                return -EINVAL;
 
-       mdev->entity_id = 1;
        INIT_LIST_HEAD(&mdev->entities);
        spin_lock_init(&mdev->lock);
        mutex_init(&mdev->graph_mutex);

@@ -433,10 +432,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
        entity->parent = mdev;
 
        spin_lock(&mdev->lock);
-       if (entity->id == 0)
-               entity->id = mdev->entity_id++;
-       else
-               mdev->entity_id = max(entity->id + 1, mdev->entity_id);
+       /* Initialize media_gobj embedded at the entity */
+       media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
        list_add_tail(&entity->list, &mdev->entities);
        spin_unlock(&mdev->lock);
 

@@ -459,6 +456,7 @@ void media_device_unregister_entity(struct media_entity *entity)
                return;
 
        spin_lock(&mdev->lock);
+       media_gobj_remove(&entity->graph_obj);
        list_del(&entity->list);
        spin_unlock(&mdev->lock);
        entity->parent = NULL;

drivers/media/media-entity.c +0 -0

index 4834172bf6f8..888cb88e19bf 100644
@@ -43,7 +43,12 @@ void media_gobj_init(struct media_device *mdev,
                           enum media_gobj_type type,

drivers/media/media-entity.c void media_gobj_init(struct media_device *mdev, · line 52

Quoted 10 lines

drivers/media/media-entity.c +6 -1

                           struct media_gobj *gobj)
 {
-       /* For now, nothing to do */
+       /* Create a per-type unique object ID */
+       switch (type) {
+       case MEDIA_GRAPH_ENTITY:
+               gobj->id = media_gobj_gen_id(type, ++mdev->entity_id);
+               break;
+       }
 }

Unless there is a reason to split patches 4 and 5, it would make it lot easier to review if these two patches are combined. I had to go back to review media_gobj_gen_id(type, ++mdev->entity_id) for me to get a feel for what is done here.

If combined, there is no need for skeleton media_gobj_init() and then followed by a second patch that fills it in. It will be easier to follow the new interface and its use.

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Just like entities and pads, links also need to have unique Object IDs along a given media controller.

So, let's add a media_gobj inside it and initialize the object then a new link is created.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +9 -0

index 3bdda16584fe..065f6f08da37 100644
@@ -438,6 +438,13 @@ int __must_check media_device_register_entity(struct media_device *mdev,
        media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
        list_add_tail(&entity->list, &mdev->entities);
 
+       /*
+        * Initialize objects at the links
+        * in the case where links got created before entity register
+        */
+       for (i = 0; i < entity->num_links; i++)
+               media_gobj_init(mdev, MEDIA_GRAPH_LINK,
+                               &entity->links[i].graph_obj);
        /* Initialize objects at the pads */
        for (i = 0; i < entity->num_pads; i++)
                media_gobj_init(mdev, MEDIA_GRAPH_PAD,

@@ -465,6 +472,8 @@ void media_device_unregister_entity(struct media_entity *entity)
                return;
 
        spin_lock(&mdev->lock);
+       for (i = 0; i < entity->num_links; i++)
+               media_gobj_remove(&entity->links[i].graph_obj);
        for (i = 0; i < entity->num_pads; i++)
                media_gobj_remove(&entity->pads[i].graph_obj);
        media_gobj_remove(&entity->graph_obj);

drivers/media/media-entity.c +0 -0

index 377c6655c5d0..36d725ec5f3d 100644

drivers/media/media-entity.c +4 -0

+
        /* Create the backlink. Backlinks are used to help graph traversal and
         * are not reported to userspace.
         */

@@ -504,6 +510,9 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
        backlink->sink = &sink->pads[sink_pad];
        backlink->flags = flags;
 
+       /* Initialize graph object embedded at the new link */
+       media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
+
        link->reverse = backlink;
        backlink->reverse = link;
 

include/media/media-device.h +2 -0

index 9493721f630e..05414e351f8e 100644
@@ -43,6 +43,7 @@ struct device;
  * @driver_version: Device driver version
  * @entity_id: Unique ID used on the last entity registered
  * @pad_id:    Unique ID used on the last pad registered
+ * @link_id:   Unique ID used on the last link registered
  * @entities:  List of registered entities
  * @lock:      Entities list lock
  * @graph_mutex: Entities graph operation lock

@@ -71,6 +72,7 @@ struct media_device {
 
        u32 entity_id;
        u32 pad_id;
+       u32 link_id;
 
        struct list_head entities;
 

include/media/media-entity.h +3 -1

index ce4c654486d6..cd08a96bfbaa 100644
@@ -35,10 +35,12 @@
  *
  * @MEDIA_GRAPH_ENTITY:                Identify a media entity
  * @MEDIA_GRAPH_PAD:           Identify a media pad
+ * @MEDIA_GRAPH_LINK:          Identify a media link
  */
 enum media_gobj_type {
        MEDIA_GRAPH_ENTITY,
        MEDIA_GRAPH_PAD,
+       MEDIA_GRAPH_LINK,
 };
 
 #define MEDIA_BITS_PER_TYPE            8

@@ -67,6 +69,7 @@ struct media_pipeline {
 };
 
 struct media_link {
+       struct media_gobj graph_obj;
        struct media_pad *source;       /* Source pad */
        struct media_pad *sink;         /* Sink pad  */
        struct media_link *reverse;     /* Link in the reverse direction */
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 46 lines · Mauro Carvalho Chehab

Just like entities and pads, links also need to have unique Object IDs along a given media controller.

So, let's add a media_gobj inside it and initialize the object then a new link is created.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-device.c +9 -0

index 3bdda16584fe..065f6f08da37 100644
@@ -438,6 +438,13 @@ int __must_check media_device_register_entity(struct media_device *mdev,
        media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
        list_add_tail(&entity->list, &mdev->entities);
 
+       /*
+        * Initialize objects at the links
+        * in the case where links got created before entity register
+        */
+       for (i = 0; i < entity->num_links; i++)
+               media_gobj_init(mdev, MEDIA_GRAPH_LINK,
+                               &entity->links[i].graph_obj);
        /* Initialize objects at the pads */
        for (i = 0; i < entity->num_pads; i++)
                media_gobj_init(mdev, MEDIA_GRAPH_PAD,

@@ -465,6 +472,8 @@ void media_device_unregister_entity(struct media_entity *entity)
                return;
 
        spin_lock(&mdev->lock);
+       for (i = 0; i < entity->num_links; i++)
+               media_gobj_remove(&entity->links[i].graph_obj);
        for (i = 0; i < entity->num_pads; i++)
                media_gobj_remove(&entity->pads[i].graph_obj);
        media_gobj_remove(&entity->graph_obj);

drivers/media/media-entity.c +1 -0

index 377c6655c5d0..36d725ec5f3d 100644
@@ -51,6 +51,9 @@ void media_gobj_init(struct media_device *mdev,
        case MEDIA_GRAPH_PAD:
                gobj->id = media_gobj_gen_id(type, ++mdev->pad_id);
                break;
+       case MEDIA_GRAPH_LINK:

drivers/media/media-entity.c void media_gobj_init(struct media_device *mdev, · line 59

Quoted 5 lines

drivers/media/media-entity.c +2 -0

+               gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
+               break;
        }
 }
 

I think collapsing patches 4,5,6, and 7 will make this code so much easier to review.

Can we simplify media_gobj_init()? If it takes mdev->*_id as a inout/output value, there is no need to have this switch. You can simply range check type.

Quoted 6 lines

drivers/media/media-entity.c +0 -0

@@ -491,6 +494,9 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
        link->sink = &sink->pads[sink_pad];
        link->flags = flags;
 

drivers/media/media-entity.c media_entity_create_link(struct media_entity *source, u16 source_pad, · line 498

Quoted 2 lines

drivers/media/media-entity.c +2 -0

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

So far calls to media_gobj_init() are made holding mdev->lock. This is the first
case without mdev->lock hold. This could lead race conditions as this routine is changes fields in mdev.

thanks,
-- Shuah

Quoted 66 lines

drivers/media/media-entity.c +4 -0

+
        /* Create the backlink. Backlinks are used to help graph traversal and
         * are not reported to userspace.
         */

@@ -504,6 +510,9 @@ media_entity_create_link(struct media_entity *source, u16 source_pad,
        backlink->sink = &sink->pads[sink_pad];
        backlink->flags = flags;
 
+       /* Initialize graph object embedded at the new link */
+       media_gobj_init(sink->parent, MEDIA_GRAPH_LINK, &backlink->graph_obj);
+
        link->reverse = backlink;
        backlink->reverse = link;
 

include/media/media-device.h +2 -0

index 9493721f630e..05414e351f8e 100644
@@ -43,6 +43,7 @@ struct device;
  * @driver_version: Device driver version
  * @entity_id: Unique ID used on the last entity registered
  * @pad_id:    Unique ID used on the last pad registered
+ * @link_id:   Unique ID used on the last link registered
  * @entities:  List of registered entities
  * @lock:      Entities list lock
  * @graph_mutex: Entities graph operation lock

@@ -71,6 +72,7 @@ struct media_device {
 
        u32 entity_id;
        u32 pad_id;
+       u32 link_id;
 
        struct list_head entities;
 

include/media/media-entity.h +3 -1

index ce4c654486d6..cd08a96bfbaa 100644
@@ -35,10 +35,12 @@
  *
  * @MEDIA_GRAPH_ENTITY:                Identify a media entity
  * @MEDIA_GRAPH_PAD:           Identify a media pad
+ * @MEDIA_GRAPH_LINK:          Identify a media link
  */
 enum media_gobj_type {
        MEDIA_GRAPH_ENTITY,
        MEDIA_GRAPH_PAD,
+       MEDIA_GRAPH_LINK,
 };
 
 #define MEDIA_BITS_PER_TYPE            8

@@ -67,6 +69,7 @@ struct media_pipeline {
 };
 
 struct media_link {
+       struct media_gobj graph_obj;
        struct media_pad *source;       /* Source pad */
        struct media_pad *sink;         /* Sink pad  */
        struct media_link *reverse;     /* Link in the reverse direction */
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

It helps to check if the media controller is doing the right thing with the object creation and removal.

No extra code/data will be produced if DEBUG or
CONFIG_DYNAMIC_DEBUG is not enabled.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-entity.c +12 -0

index 36d725ec5f3d..6d515e149d7f 100644
@@ -27,6 +27,69 @@
 #include <media/media-device.h>
 
 /**
+ *  dev_dbg_obj - Prints in debug mode a change on some object
+ *
+ * @event_name:        Name of the event to report. Could be __func__
+ * @gobj:      Pointer to the object
+ *
+ * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
+ * won't produce any code.
+ */
+static inline const char *gobj_type(enum media_gobj_type type)
+{
+       switch (type) {
+       case MEDIA_GRAPH_ENTITY:

drivers/media/media-entity.c +45 -1

+static void dev_dbg_obj(const char *event_name,  struct media_gobj *gobj)
+{
+#if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG)
+       switch (media_type(gobj)) {
+       case MEDIA_GRAPH_ENTITY:
+               dev_dbg(gobj->mdev->dev,
+                       "%s: id 0x%08x entity#%d: '%s'\n",
+                       event_name, gobj->id, media_localid(gobj),
+                       gobj_to_entity(gobj)->name);
+               break;
+       case MEDIA_GRAPH_LINK:
+       {
+               struct media_link *link = gobj_to_link(gobj);
+
+               dev_dbg(gobj->mdev->dev,
+                       "%s: id 0x%08x link#%d: '%s' %s#%d ==> '%s' %s#%d\n",
+                       event_name, gobj->id, media_localid(gobj),
+
+                       link->source->entity->name,
+                       gobj_type(media_type(&link->source->graph_obj)),
+                       media_localid(&link->source->graph_obj),
+
+                       link->sink->entity->name,
+                       gobj_type(media_type(&link->sink->graph_obj)),
+                       media_localid(&link->sink->graph_obj));
+               break;
+       }
+       case MEDIA_GRAPH_PAD:
+       {
+               struct media_pad *pad = gobj_to_pad(gobj);
+
+               dev_dbg(gobj->mdev->dev,
+                       "%s: id 0x%08x pad#%d: '%s':%d\n",
+                       event_name, gobj->id, media_localid(gobj),
+                       pad->entity->name, pad->index);
+       }
+       }
+#endif
+}
+
+/**
  *  media_gobj_init - Initialize a graph object
  *
  * @mdev:      Pointer to the media_device that contains the object

@@ -43,6 +106,8 @@ void media_gobj_init(struct media_device *mdev,
                           enum media_gobj_type type,
                           struct media_gobj *gobj)
 {
+       gobj->mdev = mdev;
+
        /* Create a per-type unique object ID */
        switch (type) {
        case MEDIA_GRAPH_ENTITY:

@@ -55,6 +120,7 @@ void media_gobj_init(struct media_device *mdev,
                gobj->id = media_gobj_gen_id(type, ++mdev->link_id);
                break;
        }
+       dev_dbg_obj(__func__, gobj);
 }
 
 /**

@@ -66,7 +132,7 @@ void media_gobj_init(struct media_device *mdev,
  */
 void media_gobj_remove(struct media_gobj *gobj)
 {
-       /* For now, nothing to do */
+       dev_dbg_obj(__func__, gobj);
 }
 
 /**

include/media/media-entity.h +7 -1

index cd08a96bfbaa..af6646ddf6db 100644
@@ -61,6 +61,7 @@ enum media_gobj_type {
  * All objects on the media graph should have this struct embedded
  */
 struct media_gobj {
+       struct media_device     *mdev;
        u32                     id;
 };
 

@@ -192,6 +193,12 @@ struct media_entity_graph {
 #define gobj_to_entity(gobj) \
                container_of(gobj, struct media_entity, graph_obj)
 
+#define gobj_to_pad(gobj) \
+               container_of(gobj, struct media_pad, graph_obj)
+
+#define gobj_to_link(gobj) \
+               container_of(gobj, struct media_link, graph_obj)
+
 void media_gobj_init(struct media_device *mdev,
                    enum media_gobj_type type,
                    struct media_gobj *gobj);
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 37 lines · Mauro Carvalho Chehab

It helps to check if the media controller is doing the right thing with the object creation and removal.

No extra code/data will be produced if DEBUG or
CONFIG_DYNAMIC_DEBUG is not enabled.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/media-entity.c +19 -0

index 36d725ec5f3d..6d515e149d7f 100644
@@ -27,6 +27,69 @@
 #include <media/media-device.h>
 
 /**
+ *  dev_dbg_obj - Prints in debug mode a change on some object
+ *
+ * @event_name:        Name of the event to report. Could be __func__
+ * @gobj:      Pointer to the object
+ *
+ * Enabled only if DEBUG or CONFIG_DYNAMIC_DEBUG. Otherwise, it
+ * won't produce any code.
+ */
+static inline const char *gobj_type(enum media_gobj_type type)
+{
+       switch (type) {
+       case MEDIA_GRAPH_ENTITY:
+               return "entity";
+       case MEDIA_GRAPH_PAD:
+               return "pad";
+       case MEDIA_GRAPH_LINK:
+               return "link";
+       default:
+               return "unknown";

drivers/media/media-entity.c line 51

Quoted 3 lines

drivers/media/media-entity.c +3 -0

+       }
+}
+

Shouldn't the above gobj_type be defined in if defined(DEBUG) || defined (CONFIG_DYNAMIC_DEBUG) scope? Unless gobj_type() is used from other places, you will see defined, but not used warning when DEBUG and CONFIG_DYNAMIC_DEBUG are undefined.

Kind of related, maybe we should be looking into adding trace event support for media as opposed to dynamic and debug.