MC preparation patches
Subsystems (23)
v4 cover MC preparation patches
Those are the initial patches from my previous series of MC changes.
The first patch removes an unused parameter when creating links.
The next 5 patches warrant that all object types (entities, pads and links) will have an unique ID, as agreed at the MC workshop.
They prepare for the addition of the media interfaces and interface links.
Mauro Carvalho Chehab (6):
media: get rid of unused "extra_links" param on media_entity_init()
media: create a macro to get entity ID
media: add a common struct to be embed on media graph objects
media: use media_graph_obj inside entities
media: use media_graph_obj inside pads
media: use media_graph_obj inside links
Documentation/media-framework.txt | 2 +- Documentation/video4linux/v4l2-framework.txt | 4 +- Documentation/zh_CN/video4linux/v4l2-framework.txt | 4 +- drivers/media/dvb-core/dvbdev.c | 2 +- drivers/media/i2c/ad9389b.c | 2 +- drivers/media/i2c/adp1653.c | 2 +- drivers/media/i2c/adv7180.c | 2 +- drivers/media/i2c/adv7511.c | 2 +- drivers/media/i2c/adv7604.c | 2 +- drivers/media/i2c/adv7842.c | 2 +- drivers/media/i2c/as3645a.c | 2 +- drivers/media/i2c/cx25840/cx25840-core.c | 2 +- drivers/media/i2c/lm3560.c | 2 +- drivers/media/i2c/lm3646.c | 2 +- drivers/media/i2c/m5mols/m5mols_core.c | 2 +- drivers/media/i2c/mt9m032.c | 2 +- drivers/media/i2c/mt9p031.c | 2 +- drivers/media/i2c/mt9t001.c | 2 +- drivers/media/i2c/mt9v032.c | 2 +- drivers/media/i2c/noon010pc30.c | 2 +- drivers/media/i2c/ov2659.c | 2 +- drivers/media/i2c/ov9650.c | 2 +- drivers/media/i2c/s5c73m3/s5c73m3-core.c | 4 +- drivers/media/i2c/s5k4ecgx.c | 2 +- drivers/media/i2c/s5k5baf.c | 4 +- drivers/media/i2c/s5k6a3.c | 2 +- drivers/media/i2c/s5k6aa.c | 2 +- drivers/media/i2c/smiapp/smiapp-core.c | 4 +- drivers/media/i2c/tc358743.c | 2 +- drivers/media/i2c/tvp514x.c | 2 +- drivers/media/i2c/tvp7002.c | 2 +- drivers/media/media-device.c | 29 +++++++---- drivers/media/media-entity.c | 58 +++++++++++++++++++--- drivers/media/platform/exynos4-is/fimc-capture.c | 4 +- drivers/media/platform/exynos4-is/fimc-isp-video.c | 2 +- drivers/media/platform/exynos4-is/fimc-isp.c | 2 +- drivers/media/platform/exynos4-is/fimc-lite.c | 4 +- drivers/media/platform/exynos4-is/fimc-m2m.c | 2 +- drivers/media/platform/exynos4-is/mipi-csis.c | 2 +- drivers/media/platform/omap3isp/ispccdc.c | 2 +- drivers/media/platform/omap3isp/ispccp2.c | 2 +- drivers/media/platform/omap3isp/ispcsi2.c | 2 +- drivers/media/platform/omap3isp/isppreview.c | 2 +- drivers/media/platform/omap3isp/ispresizer.c | 2 +- drivers/media/platform/omap3isp/ispstat.c | 2 +- drivers/media/platform/omap3isp/ispvideo.c | 2 +- drivers/media/platform/s3c-camif/camif-capture.c | 4 +- drivers/media/platform/vsp1/vsp1_entity.c | 2 +- drivers/media/platform/vsp1/vsp1_video.c | 6 +-- drivers/media/platform/xilinx/xilinx-dma.c | 2 +- drivers/media/platform/xilinx/xilinx-tpg.c | 2 +- drivers/media/usb/cx231xx/cx231xx-video.c | 4 +- drivers/media/usb/uvc/uvc_entity.c | 4 +- drivers/media/v4l2-core/tuner-core.c | 2 +- drivers/media/v4l2-core/v4l2-flash-led-class.c | 2 +- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 2 +- drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +- drivers/staging/media/davinci_vpfe/dm365_isif.c | 2 +- drivers/staging/media/davinci_vpfe/dm365_resizer.c | 6 +-- drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +- drivers/staging/media/omap4iss/iss_csi2.c | 2 +- drivers/staging/media/omap4iss/iss_ipipe.c | 2 +- drivers/staging/media/omap4iss/iss_ipipeif.c | 2 +- drivers/staging/media/omap4iss/iss_resizer.c | 2 +- drivers/staging/media/omap4iss/iss_video.c | 2 +- include/media/media-device.h | 8 ++- include/media/media-entity.h | 48 ++++++++++++++++-- 67 files changed, 200 insertions(+), 97 deletions(-)
Signature · 1 line
2.4.3
PATCH v4 1/6 media: get rid of unused "extra_links" param on media_entity_init()
Currently, media_entity_init() creates an array with the links, allocated at init time. It provides a parameter (extra_links) that would allocate more links than the current needs, but this is not used by any driver.
As we want to be able to do dynamic link allocation/removal, we'll need to change the implementation of the links. So, before doing that, let's first remove that extra unused parameter, in order to cleanup the interface first.
Documentation/media-framework.txt +1 -1
@@ -104,7 +104,7 @@ although drivers can allocate entities directly. Drivers initialize entities by calling media_entity_init(struct media_entity *entity, u16 num_pads, - struct media_pad *pads, u16 extra_links); + struct media_pad *pads); The media_entity name, type, flags, revision and group_id fields can be initialized before or after calling media_entity_init. Entities embedded in
Documentation/video4linux/v4l2-framework.txt +2 -2
@@ -300,7 +300,7 @@ calling media_entity_init(): struct media_pad *pads = &my_sd->pads; int err; - err = media_entity_init(&sd->entity, npads, pads, 0); + err = media_entity_init(&sd->entity, npads, pads); The pads array must have been previously initialized. There is no need to manually set the struct media_entity type and name fields, but the revision
@@ -700,7 +700,7 @@ calling media_entity_init(): struct media_pad *pad = &my_vdev->pad; int err; - err = media_entity_init(&vdev->entity, 1, pad, 0); + err = media_entity_init(&vdev->entity, 1, pad); The pads array must have been previously initialized. There is no need to manually set the struct media_entity type and name fields.
Documentation/zh_CN/video4linux/v4l2-framework.txt +2 -2
@@ -295,7 +295,7 @@ owner ??????????????? i2c ????????????????????????????????????????????? struct media_pad *pads = &my_sd->pads; int err; - err = media_entity_init(&sd->entity, npads, pads, 0); + err = media_entity_init(&sd->entity, npads, pads); pads ???????????????????????????????????????????????? media_entity ??? type ??? name ????????????????????????revision ?????????????????????
@@ -602,7 +602,7 @@ v4l2_file_operations ???????????? file_operations ??????????????????????????? struct media_pad *pad = &my_vdev->pad; int err; - err = media_entity_init(&vdev->entity, 1, pad, 0); + err = media_entity_init(&vdev->entity, 1, pad); pads ?????????????????????????????????????????????????????? media_entity ??? type ??? name ??????
drivers/media/dvb-core/dvbdev.c +1 -1
@@ -249,7 +249,7 @@ static void dvb_register_media_device(struct dvb_device *dvbdev, }if (npads) - ret =media_entity_init (dvbdev->entity, npads, dvbdev->pads,0 ); + ret =media_entity_init (dvbdev->entity, npads, dvbdev->pads);if (!ret) ret =media_device_register_entity (dvbdev->adapter->mdev, dvbdev->entity);
drivers/media/i2c/ad9389b.c +1 -1
@@ -1158,7 +1158,7 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id * state->rgb_quantization_range_ctrl->is_private = true; state->pad.flags =MEDIA_PAD_FL_SINK ; - err =media_entity_init (&sd->entity,1 , &state->pad,0 ); + err =media_entity_init (&sd->entity,1 , &state->pad);if (err) goto err_hdl;
drivers/media/i2c/adp1653.c +1 -1
@@ -512,7 +512,7 @@ static int adp1653_probe(struct i2c_client *client,if (ret) goto free_and_quit; - ret =media_entity_init (&flash->subdev.entity,0 ,NULL ,0 ); + ret =media_entity_init (&flash->subdev.entity,0 ,NULL );if (ret <0 ) goto free_and_quit;
drivers/media/i2c/adv7180.c +1 -1
@@ -1214,7 +1214,7 @@ static int adv7180_probe(struct i2c_client *client, state->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.flags |=MEDIA_ENT_T_V4L2_SUBDEV_DECODER ; - ret =media_entity_init (&sd->entity,1 , &state->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &state->pad);if (ret) goto err_free_ctrl;
drivers/media/i2c/adv7511.c +1 -1
@@ -1482,7 +1482,7 @@ static int adv7511_probe(struct i2c_client *client, const struct i2c_device_id * state->rgb_quantization_range_ctrl->is_private = true; state->pad.flags =MEDIA_PAD_FL_SINK ; - err =media_entity_init (&sd->entity,1 , &state->pad,0 ); + err =media_entity_init (&sd->entity,1 , &state->pad);if (err) goto err_hdl;
drivers/media/i2c/adv7604.c +1 -1
@@ -3209,7 +3209,7 @@ static int adv76xx_probe(struct i2c_client *client, state->pads[state->source_pad].flags =MEDIA_PAD_FL_SOURCE ; err =media_entity_init (&sd->entity, state->source_pad +1 , - state->pads,0 ); + state->pads);if (err) goto err_work_queues;
drivers/media/i2c/adv7842.c +1 -1
@@ -3309,7 +3309,7 @@ static int adv7842_probe(struct i2c_client *client, adv7842_delayed_work_enable_hotplug); state->pad.flags =MEDIA_PAD_FL_SOURCE ; - err =media_entity_init (&sd->entity,1 , &state->pad,0 ); + err =media_entity_init (&sd->entity,1 , &state->pad);if (err) goto err_work_queues;
drivers/media/i2c/as3645a.c +1 -1
@@ -827,7 +827,7 @@ static int as3645a_probe(struct i2c_client *client,if (ret <0 ) goto done; - ret =media_entity_init (&flash->subdev.entity,0 ,NULL ,0 ); + ret =media_entity_init (&flash->subdev.entity,0 ,NULL );if (ret <0 ) goto done;
drivers/media/i2c/cx25840/cx25840-core.c +1 -1
@@ -5211,7 +5211,7 @@ static int cx25840_probe(struct i2c_client *client, sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_DECODER ; ret =media_entity_init (&sd->entity,ARRAY_SIZE (state->pads), - state->pads,0 ); + state->pads);if (ret <0 ) {v4l_info (client,"failed to initialize media entity!\n" );return ret;
drivers/media/i2c/lm3560.c +1 -1
@@ -365,7 +365,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash, rval =lm3560_init_controls (flash, led_no);if (rval) goto err_out; - rval =media_entity_init (&flash->subdev_led[led_no].entity,0 ,NULL ,0 ); + rval =media_entity_init (&flash->subdev_led[led_no].entity,0 ,NULL );if (rval <0 ) goto err_out; flash->subdev_led[led_no].entity.type =MEDIA_ENT_T_V4L2_SUBDEV_FLASH ;
drivers/media/i2c/lm3646.c +1 -1
@@ -282,7 +282,7 @@ static int lm3646_subdev_init(struct lm3646_flash *flash) rval =lm3646_init_controls (flash);if (rval) goto err_out; - rval =media_entity_init (&flash->subdev_led.entity,0 ,NULL ,0 ); + rval =media_entity_init (&flash->subdev_led.entity,0 ,NULL );if (rval <0 ) goto err_out; flash->subdev_led.entity.type =MEDIA_ENT_T_V4L2_SUBDEV_FLASH ;
drivers/media/i2c/m5mols/m5mols_core.c +1 -1
@@ -975,7 +975,7 @@ static int m5mols_probe(struct i2c_client *client, sd->internal_ops = &m5mols_subdev_internal_ops; info->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&sd->entity,1 , &info->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &info->pad);if (ret <0 )return ret; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ;
drivers/media/i2c/mt9m032.c +1 -1
@@ -799,7 +799,7 @@ static int mt9m032_probe(struct i2c_client *client, sensor->subdev.ctrl_handler = &sensor->ctrls; sensor->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&sensor->subdev.entity,1 , &sensor->pad,0 ); + ret =media_entity_init (&sensor->subdev.entity,1 , &sensor->pad);if (ret <0 ) goto error_ctrl;
drivers/media/i2c/mt9p031.c +1 -1
@@ -1112,7 +1112,7 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->subdev.internal_ops = &mt9p031_subdev_internal_ops; mt9p031->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&mt9p031->subdev.entity,1 , &mt9p031->pad,0 ); + ret =media_entity_init (&mt9p031->subdev.entity,1 , &mt9p031->pad);if (ret <0 ) goto done;
drivers/media/i2c/mt9t001.c +1 -1
@@ -933,7 +933,7 @@ static int mt9t001_probe(struct i2c_client *client, mt9t001->subdev.flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ; mt9t001->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&mt9t001->subdev.entity,1 , &mt9t001->pad,0 ); + ret =media_entity_init (&mt9t001->subdev.entity,1 , &mt9t001->pad); done:if (ret <0 ) {
drivers/media/i2c/mt9v032.c +1 -1
@@ -1046,7 +1046,7 @@ static int mt9v032_probe(struct i2c_client *client, mt9v032->subdev.flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ; mt9v032->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&mt9v032->subdev.entity,1 , &mt9v032->pad,0 ); + ret =media_entity_init (&mt9v032->subdev.entity,1 , &mt9v032->pad);if (ret <0 ) goto err;
drivers/media/i2c/noon010pc30.c +1 -1
@@ -780,7 +780,7 @@ static int noon010_probe(struct i2c_client *client, info->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,1 , &info->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &info->pad);if (ret <0 ) goto np_err;
drivers/media/i2c/ov2659.c +1 -1
@@ -1450,7 +1450,7 @@ static int ov2659_probe(struct i2c_client *client,#if defined(CONFIG_MEDIA_CONTROLLER ) ov2659->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,1 , &ov2659->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &ov2659->pad);if (ret <0 ) {v4l2_ctrl_handler_free (&ov2659->ctrls);return ret;
drivers/media/i2c/ov9650.c +1 -1
@@ -1501,7 +1501,7 @@ static int ov965x_probe(struct i2c_client *client, ov965x->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,1 , &ov965x->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &ov965x->pad);if (ret <0 )return ret;
drivers/media/i2c/s5c73m3/s5c73m3-core.c +2 -2
@@ -1691,7 +1691,7 @@ static int s5c73m3_probe(struct i2c_client *client, sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV ; ret =media_entity_init (&sd->entity,S5C73M3_NUM_PADS , - state->sensor_pads,0 ); + state->sensor_pads);if (ret <0 )return ret;
@@ -1707,7 +1707,7 @@ static int s5c73m3_probe(struct i2c_client *client, oif_sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV ; ret =media_entity_init (&oif_sd->entity,OIF_NUM_PADS , - state->oif_pads,0 ); + state->oif_pads);if (ret <0 )return ret;
drivers/media/i2c/s5k4ecgx.c +1 -1
@@ -962,7 +962,7 @@ static int s5k4ecgx_probe(struct i2c_client *client, priv->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,1 , &priv->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &priv->pad);if (ret)return ret;
drivers/media/i2c/s5k5baf.c +2 -2
@@ -1905,7 +1905,7 @@ static int s5k5baf_configure_subdevs(struct s5k5baf *state, state->cis_pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,NUM_CIS_PADS , &state->cis_pad,0 ); + ret =media_entity_init (&sd->entity,NUM_CIS_PADS , &state->cis_pad);if (ret <0 ) goto err;
@@ -1920,7 +1920,7 @@ static int s5k5baf_configure_subdevs(struct s5k5baf *state, state->pads[PAD_CIS ].flags =MEDIA_PAD_FL_SINK ; state->pads[PAD_OUT ].flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV ; - ret =media_entity_init (&sd->entity,NUM_ISP_PADS , state->pads,0 ); + ret =media_entity_init (&sd->entity,NUM_ISP_PADS , state->pads);if (!ret)return 0 ;
drivers/media/i2c/s5k6a3.c +1 -1
@@ -333,7 +333,7 @@ static int s5k6a3_probe(struct i2c_client *client, sensor->format.height =S5K6A3_DEFAULT_HEIGHT ; sensor->pad.flags =MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&sd->entity,1 , &sensor->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &sensor->pad);if (ret <0 )return ret;
drivers/media/i2c/s5k6aa.c +1 -1
@@ -1578,7 +1578,7 @@ static int s5k6aa_probe(struct i2c_client *client, s5k6aa->pad.flags =MEDIA_PAD_FL_SOURCE ; sd->entity.type =MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ; - ret =media_entity_init (&sd->entity,1 , &s5k6aa->pad,0 ); + ret =media_entity_init (&sd->entity,1 , &s5k6aa->pad);if (ret)return ret;
drivers/media/i2c/smiapp/smiapp-core.c +2 -2
@@ -2488,7 +2488,7 @@ static int smiapp_register_subdevs(struct smiapp_sensor *sensor)continue ; rval =media_entity_init (&this->sd.entity, - this->npads, this->pads,0 ); + this->npads, this->pads);if (rval) {dev_err (&client->dev,"media_entity_init failed\n" );
@@ -3078,7 +3078,7 @@ static int smiapp_probe(struct i2c_client *client, sensor->src->pads[0 ].flags =MEDIA_PAD_FL_SOURCE ; rval =media_entity_init (&sensor->src->sd.entity,2 , - sensor->src->pads,0 ); + sensor->src->pads);if (rval <0 )return rval;
drivers/media/i2c/tc358743.c +1 -1
@@ -1877,7 +1877,7 @@ static int tc358743_probe(struct i2c_client *client, } state->pad.flags =MEDIA_PAD_FL_SOURCE ; - err =media_entity_init (&sd->entity,1 , &state->pad,0 ); + err =media_entity_init (&sd->entity,1 , &state->pad);if (err <0 ) goto err_hdl;
drivers/media/i2c/tvp514x.c +1 -1
@@ -1097,7 +1097,7 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id) decoder->sd.flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ; decoder->sd.entity.flags |=MEDIA_ENT_T_V4L2_SUBDEV_DECODER ; - ret =media_entity_init (&decoder->sd.entity,1 , &decoder->pad,0 ); + ret =media_entity_init (&decoder->sd.entity,1 , &decoder->pad);if (ret <0 ) {v4l2_err (sd,"%s decoder driver failed to register !!\n" , sd->name);
drivers/media/i2c/tvp7002.c +1 -1
@@ -1014,7 +1014,7 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id) device->sd.flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ; device->sd.entity.flags |=MEDIA_ENT_T_V4L2_SUBDEV_DECODER ; - error =media_entity_init (&device->sd.entity,1 , &device->pad,0 ); + error =media_entity_init (&device->sd.entity,1 , &device->pad);if (error <0 )return error;#endif
drivers/media/media-entity.c +2 -3
@@ -30,7 +30,6 @@* media_entity_init - Initialize a media entity * * @num_pads: Total number of sink and source pads. -* @extra_links: Initial estimate of the number of extra links. * @pads: Array of 'num_pads' pads. * * The total number of pads is an intrinsic property of entities known by the
@@ -52,10 +51,10 @@*/ int media_entity_init (struct media_entity *entity,u16 num_pads, -struct media_pad *pads,u16 extra_links) +struct media_pad *pads) {struct media_link *links; -unsigned int max_links = num_pads + extra_links; +unsigned int max_links = num_pads;unsigned int i; links =kzalloc (max_links *sizeof (links[0 ]),GFP_KERNEL );
drivers/media/platform/exynos4-is/fimc-capture.c +2 -2
@@ -1804,7 +1804,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, vid_cap->wb_fmt.code = fmt->mbus_code; vid_cap->vd_pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&vfd->entity,1 , &vid_cap->vd_pad,0 ); + ret =media_entity_init (&vfd->entity,1 , &vid_cap->vd_pad);if (ret) goto err_free_ctx;
@@ -1897,7 +1897,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) fimc->vid_cap.sd_pads[FIMC_SD_PAD_SINK_FIFO ].flags =MEDIA_PAD_FL_SINK ; fimc->vid_cap.sd_pads[FIMC_SD_PAD_SOURCE ].flags =MEDIA_PAD_FL_SOURCE ; ret =media_entity_init (&sd->entity,FIMC_SD_PADS_NUM , - fimc->vid_cap.sd_pads,0 ); + fimc->vid_cap.sd_pads);if (ret)return ret;
drivers/media/platform/exynos4-is/fimc-isp-video.c +1 -1
@@ -618,7 +618,7 @@ int fimc_isp_video_device_register(struct fimc_isp *isp, vdev->lock = &isp->video_lock; iv->pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&vdev->entity,1 , &iv->pad,0 ); + ret =media_entity_init (&vdev->entity,1 , &iv->pad);if (ret <0 )return ret;
drivers/media/platform/exynos4-is/fimc-isp.c +1 -1
@@ -709,7 +709,7 @@ int fimc_isp_subdev_create(struct fimc_isp *isp) isp->subdev_pads[FIMC_ISP_SD_PAD_SRC_FIFO ].flags =MEDIA_PAD_FL_SOURCE ; isp->subdev_pads[FIMC_ISP_SD_PAD_SRC_DMA ].flags =MEDIA_PAD_FL_SOURCE ; ret =media_entity_init (&sd->entity,FIMC_ISP_SD_PADS_NUM , - isp->subdev_pads,0 ); + isp->subdev_pads);if (ret)return ret;
drivers/media/platform/exynos4-is/fimc-lite.c +2 -2
@@ -1322,7 +1322,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)return ret; fimc->vd_pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&vfd->entity,1 , &fimc->vd_pad,0 ); + ret =media_entity_init (&vfd->entity,1 , &fimc->vd_pad);if (ret <0 )return ret;
@@ -1437,7 +1437,7 @@ static int fimc_lite_create_capture_subdev(struct fimc_lite *fimc) fimc->subdev_pads[FLITE_SD_PAD_SOURCE_DMA ].flags =MEDIA_PAD_FL_SOURCE ; fimc->subdev_pads[FLITE_SD_PAD_SOURCE_ISP ].flags =MEDIA_PAD_FL_SOURCE ; ret =media_entity_init (&sd->entity,FLITE_SD_PADS_NUM , - fimc->subdev_pads,0 ); + fimc->subdev_pads);if (ret)return ret;
drivers/media/platform/exynos4-is/fimc-m2m.c +1 -1
@@ -738,7 +738,7 @@ int fimc_register_m2m_device(struct fimc_dev *fimc,return PTR_ERR (fimc->m2m.m2m_dev); } - ret =media_entity_init (&vfd->entity,0 ,NULL ,0 ); + ret =media_entity_init (&vfd->entity,0 ,NULL );if (ret) goto err_me;
drivers/media/platform/exynos4-is/mipi-csis.c +1 -1
@@ -866,7 +866,7 @@ static int s5pcsis_probe(struct platform_device *pdev) state->pads[CSIS_PAD_SINK ].flags =MEDIA_PAD_FL_SINK ; state->pads[CSIS_PAD_SOURCE ].flags =MEDIA_PAD_FL_SOURCE ; ret =media_entity_init (&state->sd.entity, -CSIS_PADS_NUM , state->pads,0 ); +CSIS_PADS_NUM , state->pads);if (ret <0 ) goto e_clkdis;
drivers/media/platform/omap3isp/ispccdc.c +1 -1
@@ -2650,7 +2650,7 @@ static int ccdc_init_entities(struct isp_ccdc_device *ccdc) pads[CCDC_PAD_SOURCE_OF ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &ccdc_media_ops; - ret =media_entity_init (me,CCDC_PADS_NUM , pads,0 ); + ret =media_entity_init (me,CCDC_PADS_NUM , pads);if (ret <0 )return ret;
drivers/media/platform/omap3isp/ispccp2.c +1 -1
@@ -1071,7 +1071,7 @@ static int ccp2_init_entities(struct isp_ccp2_device *ccp2) pads[CCP2_PAD_SOURCE ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &ccp2_media_ops; - ret =media_entity_init (me,CCP2_PADS_NUM , pads,0 ); + ret =media_entity_init (me,CCP2_PADS_NUM , pads);if (ret <0 )return ret;
drivers/media/platform/omap3isp/ispcsi2.c +1 -1
@@ -1245,7 +1245,7 @@ static int csi2_init_entities(struct isp_csi2_device *csi2) | MEDIA_PAD_FL_MUST_CONNECT; me->ops = &csi2_media_ops; - ret =media_entity_init (me,CSI2_PADS_NUM , pads,0 ); + ret =media_entity_init (me,CSI2_PADS_NUM , pads);if (ret <0 )return ret;
drivers/media/platform/omap3isp/isppreview.c +1 -1
@@ -2282,7 +2282,7 @@ static int preview_init_entities(struct isp_prev_device *prev) pads[PREV_PAD_SOURCE ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &preview_media_ops; - ret =media_entity_init (me,PREV_PADS_NUM , pads,0 ); + ret =media_entity_init (me,PREV_PADS_NUM , pads);if (ret <0 )return ret;
drivers/media/platform/omap3isp/ispresizer.c +1 -1
@@ -1728,7 +1728,7 @@ static int resizer_init_entities(struct isp_res_device *res) pads[RESZ_PAD_SOURCE ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &resizer_media_ops; - ret =media_entity_init (me,RESZ_PADS_NUM , pads,0 ); + ret =media_entity_init (me,RESZ_PADS_NUM , pads);if (ret <0 )return ret;
drivers/media/platform/omap3isp/ispstat.c +1 -1
@@ -1029,7 +1029,7 @@ static int isp_stat_init_entities(struct ispstat *stat, const char *name, stat->pad.flags =MEDIA_PAD_FL_SINK |MEDIA_PAD_FL_MUST_CONNECT ; me->ops =NULL ; -return media_entity_init (me,1 , &stat->pad,0 ); +return media_entity_init (me,1 , &stat->pad); } int omap3isp_stat_init(struct ispstat *stat, const char *name,
drivers/media/platform/omap3isp/ispvideo.c +1 -1
@@ -1368,7 +1368,7 @@ int omap3isp_video_init(struct isp_video *video, const char *name)if (IS_ERR (video->alloc_ctx))return PTR_ERR (video->alloc_ctx); - ret =media_entity_init (&video->video.entity,1 , &video->pad,0 ); + ret =media_entity_init (&video->video.entity,1 , &video->pad);if (ret <0 ) {vb2_dma_contig_cleanup_ctx (video->alloc_ctx);return ret;
drivers/media/platform/s3c-camif/camif-capture.c +2 -2
@@ -1159,7 +1159,7 @@ int s3c_camif_register_video_node(struct camif_dev *camif, int idx) goto err_vd_rel; vp->pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&vfd->entity,1 , &vp->pad,0 ); + ret =media_entity_init (&vfd->entity,1 , &vp->pad);if (ret) goto err_vd_rel;
@@ -1575,7 +1575,7 @@ int s3c_camif_create_subdev(struct camif_dev *camif) camif->pads[CAMIF_SD_PAD_SOURCE_P ].flags =MEDIA_PAD_FL_SOURCE ; ret =media_entity_init (&sd->entity,CAMIF_SD_PADS_NUM , - camif->pads,0 ); + camif->pads);if (ret)return ret;
drivers/media/platform/vsp1/vsp1_entity.c +1 -1
@@ -220,7 +220,7 @@ int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,/* Initialize the media entity. */ return media_entity_init (&entity->subdev.entity, num_pads, - entity->pads,0 ); + entity->pads); }void vsp1_entity_destroy (struct vsp1_entity *entity)
drivers/media/platform/vsp1/vsp1_video.c +1 -1
@@ -1221,7 +1221,7 @@ int vsp1_video_init(struct vsp1_video *video, struct vsp1_entity *rwpf) video->pipe.state =VSP1_PIPELINE_STOPPED ;/* Initialize the media entity... */ - ret =media_entity_init (&video->video.entity,1 , &video->pad,0 ); + ret =media_entity_init (&video->video.entity,1 , &video->pad);if (ret <0 )return ret;
drivers/media/platform/xilinx/xilinx-dma.c +1 -1
@@ -676,7 +676,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, dma->pad.flags = type ==V4L2_BUF_TYPE_VIDEO_CAPTURE ?MEDIA_PAD_FL_SINK :MEDIA_PAD_FL_SOURCE ; - ret =media_entity_init (&dma->video.entity,1 , &dma->pad,0 ); + ret =media_entity_init (&dma->video.entity,1 , &dma->pad);if (ret <0 ) goto error;
drivers/media/platform/xilinx/xilinx-tpg.c +1 -1
@@ -836,7 +836,7 @@ static int xtpg_probe(struct platform_device *pdev) subdev->flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ; subdev->entity.ops = &xtpg_media_ops; - ret =media_entity_init (&subdev->entity, xtpg->npads, xtpg->pads,0 ); + ret =media_entity_init (&subdev->entity, xtpg->npads, xtpg->pads);if (ret <0 ) goto error;
drivers/media/usb/cx231xx/cx231xx-video.c +2 -2
@@ -2175,7 +2175,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)cx231xx_vdev_init (dev, &dev->vdev, &cx231xx_video_template,"video" );#if defined(CONFIG_MEDIA_CONTROLLER ) dev->video_pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&dev->vdev.entity,1 , &dev->video_pad,0 ); + ret =media_entity_init (&dev->vdev.entity,1 , &dev->video_pad);if (ret <0 )dev_err (dev->dev,"failed to initialize video media entity!\n" );#endif
@@ -2202,7 +2202,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)#if defined(CONFIG_MEDIA_CONTROLLER ) dev->vbi_pad.flags =MEDIA_PAD_FL_SINK ; - ret =media_entity_init (&dev->vbi_dev.entity,1 , &dev->vbi_pad,0 ); + ret =media_entity_init (&dev->vbi_dev.entity,1 , &dev->vbi_pad);if (ret <0 )dev_err (dev->dev,"failed to initialize vbi media entity!\n" );#endif
drivers/media/usb/uvc/uvc_entity.c +2 -2
@@ -89,10 +89,10 @@ static int uvc_mc_init_entity(struct uvc_entity *entity) sizeof(entity->subdev.name)); ret =media_entity_init (&entity->subdev.entity, - entity->num_pads, entity->pads,0 ); + entity->num_pads, entity->pads); }else if (entity->vdev !=NULL ) { ret =media_entity_init (&entity->vdev->entity, - entity->num_pads, entity->pads,0 ); + entity->num_pads, entity->pads);if (entity->flags &UVC_ENTITY_FLAG_DEFAULT ) entity->vdev->entity.flags |=MEDIA_ENT_FL_DEFAULT ; }else
drivers/media/v4l2-core/tuner-core.c +1 -1
@@ -699,7 +699,7 @@ register_client: t->sd.entity.type =MEDIA_ENT_T_V4L2_SUBDEV_TUNER ; t->sd.entity.name = t->name; - ret =media_entity_init (&t->sd.entity,1 , &t->pad,0 ); + ret =media_entity_init (&t->sd.entity,1 , &t->pad);if (ret <0 ) {tuner_err ("failed to initialize media entity!\n" );kfree (t);
drivers/media/v4l2-core/v4l2-flash-led-class.c +1 -1
@@ -651,7 +651,7 @@ struct v4l2_flash *v4l2_flash_init( sd->flags |=V4L2_SUBDEV_FL_HAS_DEVNODE ;strlcpy (sd->name, config->dev_name,sizeof (sd->name)); - ret =media_entity_init (&sd->entity,0 ,NULL ,0 ); + ret =media_entity_init (&sd->entity,0 ,NULL );if (ret <0 )return ERR_PTR (ret);
drivers/staging/media/davinci_vpfe/dm365_ipipe.c +1 -1
@@ -1839,7 +1839,7 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct platform_device *pdev)v4l2_ctrl_handler_setup (&ipipe->ctrls); sd->ctrl_handler = &ipipe->ctrls; -return media_entity_init (me,IPIPE_PADS_NUM , pads,0 ); +return media_entity_init (me,IPIPE_PADS_NUM , pads); }/*
drivers/staging/media/davinci_vpfe/dm365_ipipeif.c +1 -1
@@ -1026,7 +1026,7 @@ int vpfe_ipipeif_init(struct vpfe_ipipeif_device *ipipeif, ipipeif->output =IPIPEIF_OUTPUT_NONE ; me->ops = &ipipeif_media_ops; - ret =media_entity_init (me,IPIPEIF_NUM_PADS , pads,0 ); + ret =media_entity_init (me,IPIPEIF_NUM_PADS , pads);if (ret) goto fail;
drivers/staging/media/davinci_vpfe/dm365_isif.c +1 -1
@@ -2052,7 +2052,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, struct platform_device *pdev) isif->input =ISIF_INPUT_NONE ; isif->output =ISIF_OUTPUT_NONE ; me->ops = &isif_media_ops; - status =media_entity_init (me,ISIF_PADS_NUM , pads,0 ); + status =media_entity_init (me,ISIF_PADS_NUM , pads);if (status) goto isif_fail; isif->video_out.type =V4L2_BUF_TYPE_VIDEO_CAPTURE ;
drivers/staging/media/davinci_vpfe/dm365_resizer.c +3 -3
@@ -1915,7 +1915,7 @@ int vpfe_resizer_init(struct vpfe_resizer_device *vpfe_rsz, vpfe_rsz->crop_resizer.output2 =RESIZER_CROP_OUTPUT_NONE ; vpfe_rsz->crop_resizer.rsz_device = vpfe_rsz; me->ops = &resizer_media_ops; - ret =media_entity_init (me,RESIZER_CROP_PADS_NUM , pads,0 ); + ret =media_entity_init (me,RESIZER_CROP_PADS_NUM , pads);if (ret)return ret;
@@ -1937,7 +1937,7 @@ int vpfe_resizer_init(struct vpfe_resizer_device *vpfe_rsz, vpfe_rsz->resizer_a.output =RESIZER_OUTPUT_NONE ; vpfe_rsz->resizer_a.rsz_device = vpfe_rsz; me->ops = &resizer_media_ops; - ret =media_entity_init (me,RESIZER_PADS_NUM , pads,0 ); + ret =media_entity_init (me,RESIZER_PADS_NUM , pads);if (ret)return ret;
@@ -1959,7 +1959,7 @@ int vpfe_resizer_init(struct vpfe_resizer_device *vpfe_rsz, vpfe_rsz->resizer_b.output =RESIZER_OUTPUT_NONE ; vpfe_rsz->resizer_b.rsz_device = vpfe_rsz; me->ops = &resizer_media_ops; - ret =media_entity_init (me,RESIZER_PADS_NUM , pads,0 ); + ret =media_entity_init (me,RESIZER_PADS_NUM , pads);if (ret)return ret;
drivers/staging/media/davinci_vpfe/vpfe_video.c +1 -1
@@ -1594,7 +1594,7 @@ int vpfe_video_init(struct vpfe_video_device *video, const char *name)spin_lock_init (&video->dma_queue_lock);mutex_init (&video->lock); ret =media_entity_init (&video->video_dev.entity, -1 , &video->pad,0 ); +1 , &video->pad);if (ret <0 )return ret;
drivers/staging/media/omap4iss/iss_csi2.c +1 -1
@@ -1271,7 +1271,7 @@ static int csi2_init_entities(struct iss_csi2_device *csi2, const char *subname) pads[CSI2_PAD_SINK ].flags =MEDIA_PAD_FL_SINK ; me->ops = &csi2_media_ops; - ret =media_entity_init (me,CSI2_PADS_NUM , pads,0 ); + ret =media_entity_init (me,CSI2_PADS_NUM , pads);if (ret <0 )return ret;
drivers/staging/media/omap4iss/iss_ipipe.c +1 -1
@@ -513,7 +513,7 @@ static int ipipe_init_entities(struct iss_ipipe_device *ipipe) pads[IPIPE_PAD_SOURCE_VP ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &ipipe_media_ops; - ret =media_entity_init (me,IPIPE_PADS_NUM , pads,0 ); + ret =media_entity_init (me,IPIPE_PADS_NUM , pads);if (ret <0 )return ret;
drivers/staging/media/omap4iss/iss_ipipeif.c +1 -1
@@ -743,7 +743,7 @@ static int ipipeif_init_entities(struct iss_ipipeif_device *ipipeif) pads[IPIPEIF_PAD_SOURCE_VP ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &ipipeif_media_ops; - ret =media_entity_init (me,IPIPEIF_PADS_NUM , pads,0 ); + ret =media_entity_init (me,IPIPEIF_PADS_NUM , pads);if (ret <0 )return ret;
drivers/staging/media/omap4iss/iss_resizer.c +1 -1
@@ -787,7 +787,7 @@ static int resizer_init_entities(struct iss_resizer_device *resizer) pads[RESIZER_PAD_SOURCE_MEM ].flags =MEDIA_PAD_FL_SOURCE ; me->ops = &resizer_media_ops; - ret =media_entity_init (me,RESIZER_PADS_NUM , pads,0 ); + ret =media_entity_init (me,RESIZER_PADS_NUM , pads);if (ret <0 )return ret;
drivers/staging/media/omap4iss/iss_video.c +1 -1
@@ -1104,7 +1104,7 @@ int omap4iss_video_init(struct iss_video *video, const char *name)return -EINVAL ; } - ret =media_entity_init (&video->video.entity,1 , &video->pad,0 ); + ret =media_entity_init (&video->video.entity,1 , &video->pad);if (ret <0 )return ret;
include/media/media-entity.h +1 -1
@@ -127,7 +127,7 @@ struct media_entity_graph { };int media_entity_init (struct media_entity *entity,u16 num_pads, -struct media_pad *pads,u16 extra_links); +struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity); int media_entity_create_link(struct media_entity *source, u16 source_pad,
Signature · 1 line
2.4.3
PATCH v4 1/6 media: get rid of unused "extra_links" param on media_entity_init()
Quoted 5 lines · Mauro Carvalho Chehab
Currently,
media_entity_init()creates an array with the links, allocated at init time. It provides a parameter (extra_links) that would allocate more links than the current needs, but this is not used by any driver.
Quoted 7 lines
As we want to be able to do dynamic link allocation/removal, we'll need to change the implementation of the links. So, before doing that, let's first remove that extra unused parameter, in order to cleanup the interface first.
Signed-off-by Mauro Carvalho Chehab <[email protected]>Acked-by Sakari Ailus <[email protected]>
Thanks!
Hans
Laurent Pinchart
maintainerPATCH v4 1/6 media: get rid of unused "extra_links" param on media_entity_init()
In reply to Mauro Carvalho ChehabHi Mauro,
Thank you for the patch.
Quoted 18 lines · On Friday 14 August 2015 11:56:38 Mauro Carvalho Chehab wrote:
Currently,
media_entity_init()creates an array with the links, allocated at init time. It provides a parameter (extra_links) that would allocate more links than the current needs, but this is not used by any driver.As we want to be able to do dynamic link allocation/removal, we'll need to change the implementation of the links. So, before doing that, let's first remove that extra unused parameter, in order to cleanup the interface first.
Signed-off-by Mauro Carvalho Chehab <[email protected]>Acked-by Sakari Ailus <[email protected]>(quoted diff) +0
-0
b/Documentation/media-framework.txtindexf552a75c0e70..2cc6019f7147100644
Documentation/media-framework.txt although drivers can allocate entities directly. · line 110
Quoted 8 lines
Documentation/media-framework.txt +1
-1@@ -104,7 +104,7 @@ although drivers can allocate entities directly. Drivers initialize entities by calling media_entity_init(struct media_entity *entity, u16 num_pads, - struct media_pad *pads, u16 extra_links); + struct media_pad *pads); The media_entity name, type, flags, revision and group_id fields can be initialized before or after calling media_entity_init. Entities embedded in
The extra_links parameter is documented later on in this file. Could you replace the paragraph
"Unlike the number of pads, the total number of links isn't always known in advance by the entity driver. As an initial estimate, media_entity_init pre-allocates a number of links equal to the number of pads plus an optional number of extra links. The links array will be reallocated if it grows beyond the initial estimate."
with
"Unlike the number of pads, the total number of links isn't always known in advance by the entity driver. As an initial estimate, media_entity_init pre-allocates a number of links equal to the number of pads. The links array will be reallocated if it grows beyond the initial estimate."
?
[snip]
Quoted 12 lines
drivers/media/media-entity.c +0
-1@@ -30,7 +30,6 @@* media_entity_init - Initialize a media entity * * @num_pads: Total number of sink and source pads. -* @extra_links: Initial estimate of the number of extra links. * @pads: Array of 'num_pads' pads. * * The total number of pads is an intrinsic property of entities known by
Quoted 1 line
the
Similarly here, I would rewrite the documentation as
* The total number of pads is an intrinsic property of entities known by the
* entity driver, while the total number of links depends on hardware design
* and is an extrinsic property unknown to the entity driver. However, in most
* use cases the number of links can safely be assumed to be equal to or
* larger than the number of pads.
*
* For those reasons the links array can be preallocated based on the number
* of pads and will be reallocated later if extra links need to be created. *
* This function allocates a links array with enough space to hold at least
* 'num_pads' elements. The media_entity::max_links field will be set to the
* number of allocated elements.
With that fixed,
And feel free to merge this patch for v4.3 independently of the rest.
Quoted 13 lines
drivers/media/media-entity.c +2
-2@@ -52,10 +51,10 @@*/ int media_entity_init (struct media_entity *entity,u16 num_pads, -struct media_pad *pads,u16 extra_links) +struct media_pad *pads) {struct media_link *links; -unsigned int max_links = num_pads + extra_links; +unsigned int max_links = num_pads;unsigned int i; links =kzalloc (max_links *sizeof (links[0 ]),GFP_KERNEL );
Signature · 3 lines
Regards, Laurent Pinchart
PATCH v4 2/6 media: create a macro to get entity ID
In reply to Mauro Carvalho ChehabInstead of accessing direcly entity.id, let's create a macro, as this field will be moved into a common struct later on.
drivers/media/media-device.c +4 -4
@@ -75,8 +75,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 (((entity_id (entity) = id) && !next) || + ((entity_id (entity) > id) && next)) {spin_unlock (&mdev->lock);return entity; }
@@ -102,7 +102,7 @@ static long media_device_enum_entities(struct media_device *mdev,if (ent =NULL )return -EINVAL ; - u_ent.id = ent->id; + u_ent.id =entity_id (ent);if (ent->name)strlcpy (u_ent.name, ent->name,sizeof (u_ent.name)); u_ent.type = ent->type;
@@ -120,7 +120,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 =entity_id (kpad->entity); upad->index = kpad->index; upad->flags = kpad->flags; }
drivers/media/media-entity.c +4 -4
@@ -141,10 +141,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 (entity_id (entity) >=MEDIA_ENTITY_ENUM_MAX_ID ))return ; -__set_bit (entity->id, graph->entities); +__set_bit (entity_id (entity), graph->entities);stack_push (graph, entity); }EXPORT_SYMBOL_GPL (media_entity_graph_walk_start);
@@ -185,11 +185,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 (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 (entity_id (next), graph->entities)) {link_top (graph)++;continue ; }
drivers/media/platform/vsp1/vsp1_video.c +2 -2
@@ -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 <<entity_id (&entity->subdev.entity)))return -EPIPE ; - entities |=1 << entity->subdev.entity.id; + entities |=1 <<entity_id (&entity->subdev.entity);/* UDS can't be chained. */ if (entity->type =VSP1_ENTITY_UDS ) {
include/media/media-entity.h +2 -0
@@ -126,6 +126,8 @@ struct media_entity_graph {int top; }; +#define entity_id (entity) ((entity)->id) +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
Signature · 1 line
2.4.3
Re: [PATCH v4 2/6] media: create a macro to get entity ID
Quoted 1 line · Mauro Carvalho Chehab
Instead of accessing direcly entity.id, let's create a macro,
s/direcly/directly/
Quoted 3 lines
as this field will be moved into a common struct later on.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
Regards,
Hans
Quoted 96 lines
drivers/media/media-device.c +4
-4@@ -75,8 +75,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 (((entity_id (entity) = id) && !next) || + ((entity_id (entity) > id) && next)) {spin_unlock (&mdev->lock);return entity; }@@ -102,7 +102,7 @@ static long media_device_enum_entities(struct media_device *mdev,if (ent =NULL )return -EINVAL ; - u_ent.id = ent->id; + u_ent.id =entity_id (ent);if (ent->name)strlcpy (u_ent.name, ent->name,sizeof (u_ent.name)); u_ent.type = ent->type;@@ -120,7 +120,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 =entity_id (kpad->entity); upad->index = kpad->index; upad->flags = kpad->flags; }drivers/media/media-entity.c +4
-4@@ -141,10 +141,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 (entity_id (entity) >=MEDIA_ENTITY_ENUM_MAX_ID ))return ; -__set_bit (entity->id, graph->entities); +__set_bit (entity_id (entity), graph->entities);stack_push (graph, entity); }EXPORT_SYMBOL_GPL (media_entity_graph_walk_start);@@ -185,11 +185,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 (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 (entity_id (next), graph->entities)) {link_top (graph)++;continue ; }drivers/media/platform/vsp1/vsp1_video.c +2
-2@@ -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 <<entity_id (&entity->subdev.entity)))return -EPIPE ; - entities |=1 << entity->subdev.entity.id; + entities |=1 <<entity_id (&entity->subdev.entity);/* UDS can't be chained. */ if (entity->type =VSP1_ENTITY_UDS ) {include/media/media-entity.h +2
-0@@ -126,6 +126,8 @@ struct media_entity_graph {int top; }; +#define entity_id (entity) ((entity)->id) +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
Sakari Ailus
maintainerRe: [PATCH v4 2/6] media: create a macro to get entity ID
In reply to Mauro Carvalho ChehabHi Mauro,
On Fri, Aug 14, 2015 at 11:56:39AM -0300, Mauro Carvalho Chehab wrote:
...
include/media/media-entity.h struct media_entity_graph { · line 133
Quoted 13 lines
include/media/media-entity.h +2
-0@@ -126,6 +126,8 @@ struct media_entity_graph {int top; }; +#define entity_id (entity) ((entity)->id) +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
media-entity.h is a pretty widely included header file. Perhaps we should think about the naming a bit.
All the other names in the header begin with media (or __media); I'd very much prefer not changing that pattern.
Signature · 4 lines
Regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected]
Laurent Pinchart
maintainerRe: [PATCH v4 2/6] media: create a macro to get entity ID
Hello,
Quoted 16 lines · On Saturday 15 August 2015 00:08:55 Sakari Ailus wrote:
On Fri, Aug 14, 2015 at 11:56:39AM -0300, Mauro Carvalho Chehab wrote:
...Quoted 13 lines
include/media/media-entity.h +2
-0@@ -126,6 +126,8 @@ struct media_entity_graph {int top; }; +#define entity_id (entity) ((entity)->id) +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);
Quoted 7 lines
Quoted 1 line
void
media_entity_cleanup(struct media_entity *entity);
media-entity.his a pretty widely included header file. Perhaps we should think about the naming a bit.All the other names in the header begin with media (or
__media); I'd very much prefer not changing that pattern.
I'd prefer naming it media_entity_id() as well.
Slightly nitpicking, wouldn't it also be better to make it a static inline function instead of a macro to ensure type safety ? No strong preference though.
Signature · 3 lines
Regards, Laurent Pinchart
PATCH v4 3/6 media: add a common struct to be embed on media graph objects
In reply to Mauro Carvalho ChehabDue 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 latter on.
drivers/media/media-entity.c +33 -0
@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj 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 graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ; +} + +/** +* graph_obj_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 graph_obj_remove (struct media_graph_obj *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 +35 -0
@@ -28,6 +28,33 @@#include <linux/list.h> #include <linux/media.h> +/* Enums used internally at the media controller to represent graphs */ + +/** +* enum media_graph_type - type of a graph element +* +*/ +enum media_graph_type { +/* FIXME: add the types here, as we embeed media_graph_obj */ +MEDIA_GRAPH_NONE +}; + + +/* Structs to represent the objects that belong to a media graph */ + +/** +* struct media_graph_obj - Define a graph object. +* +* @id: Non-zero object ID identifier. The ID should be unique +* inside a media_device +* +* All elements on the media graph should have this struct embedded +*/ +struct media_graph_obj { +u32 id; +}; + +struct media_pipeline { };
@@ -128,6 +155,14 @@ struct media_entity_graph {#define entity_id (entity) ((entity)->id) +#define gobj_to_entity (gobj) \ + container_of(gobj, struct media_entity, graph_obj) + +void graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj); +void graph_obj_remove (struct media_graph_obj *gobj); +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
Signature · 1 line
2.4.3
Re: [PATCH v4 3/6] media: add a common struct to be embed on media graph objects
Quoted 6 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 latter on.
s/latter/later/
Quoted 53 lines
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-entity.c +33
-0@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj 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 graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ; +} + +/** +* graph_obj_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 graph_obj_remove (struct media_graph_obj *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 +0
-0@@ -28,6 +28,33 @@#include <linux/list.h>
include/media/media-entity.h line 38
Quoted 10 lines
include/media/media-entity.h +8
-0#include <linux/media.h> +/* Enums used internally at the media controller to represent graphs */ + +/** +* enum media_graph_type - type of a graph element +* +*/ +enum media_graph_type { +/* FIXME: add the types here, as we embeed media_graph_obj */
s/embeed/embed/
Quoted 28 lines
include/media/media-entity.h +20
-0+ MEDIA_GRAPH_NONE +}; + + +/* Structs to represent the objects that belong to a media graph */ + +/** +* struct media_graph_obj - Define a graph object. +* +* @id: Non-zero object ID identifier. The ID should be unique +* inside a media_device +* +* All elements on the media graph should have this struct embedded +*/ +struct media_graph_obj { +u32 id; +}; + +struct media_pipeline { };@@ -128,6 +155,14 @@ struct media_entity_graph {#define entity_id (entity) ((entity)->id) +#define gobj_to_entity (gobj) \
include/media/media-entity.h struct media_entity_graph { · line 168
Quoted 10 lines
include/media/media-entity.h +7
-0+ container_of(gobj, struct media_entity, graph_obj) + +void graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj); +void graph_obj_remove (struct media_graph_obj *gobj); +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
After fixing these small typos:
Regards,
Hans
Sakari Ailus
maintainerRe: [PATCH v4 3/6] media: add a common struct to be embed on media graph objects
In reply to Mauro Carvalho ChehabHi Mauro,
Quoted 25 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 latter on.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-entity.c +8
-0@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj inside a +* media graph object. It is called automatically if media_*_create()
drivers/media/media-entity.c line 47
Quoted 10 lines
drivers/media/media-entity.c +10
-0+* 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 graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ;
Ugh. This will mean the object IDs are going to be huge to begin with, ending up being a nuisance to work with as you often write them by hand. Do we win anything by doing so?
Quoted 61 lines
drivers/media/media-entity.c +15
-0+} + +/** +* graph_obj_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 graph_obj_remove (struct media_graph_obj *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 +28
-0@@ -28,6 +28,33 @@#include <linux/list.h> #include <linux/media.h> +/* Enums used internally at the media controller to represent graphs */ + +/** +* enum media_graph_type - type of a graph element +* +*/ +enum media_graph_type { +/* FIXME: add the types here, as we embeed media_graph_obj */ +MEDIA_GRAPH_NONE +}; + + +/* Structs to represent the objects that belong to a media graph */ + +/** +* struct media_graph_obj - Define a graph object. +* +* @id: Non-zero object ID identifier. The ID should be unique +* inside a media_device +* +* All elements on the media graph should have this struct embedded +*/ +struct media_graph_obj { +u32 id; +}; + +struct media_pipeline { };@@ -128,6 +155,14 @@ struct media_entity_graph {#define entity_id (entity) ((entity)->id) +#define gobj_to_entity (gobj) \
include/media/media-entity.h struct media_entity_graph { · line 168
Quoted 10 lines
include/media/media-entity.h +7
-0+ container_of(gobj, struct media_entity, graph_obj) + +void graph_obj_init (struct media_device *mdev, +enum media_graph_type type, +struct media_graph_obj *gobj); +void graph_obj_remove (struct media_graph_obj *gobj); +int media_entity_init (struct media_entity *entity,u16 num_pads,struct media_pad *pads);void media_entity_cleanup (struct media_entity *entity);
My previous comment on the naming applies on this patch as well. Please do maintain the common prefix.
Signature · 4 lines
Kind regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected]
Re: [PATCH v4 3/6] media: add a common struct to be embed on media graph objects
Em Sat, 15 Aug 2015 00:25:15 +0300
Sakari Ailus <[email protected]> escreveu:
Quoted 34 lines
Hi Mauro,
Quoted 31 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 latter on.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-entity.c +14
-0@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj 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 graph_obj_init(struct media_device *mdev, + enum media_graph_type type,
Quoted 8 lines
drivers/media/media-entity.c line 47
Quoted 4 lines
drivers/media/media-entity.c +4
-0+ struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ;Ugh. This will mean the object IDs are going to be huge to begin with, ending up being a nuisance to work with as you often write them by hand. Do we win anything by doing so?
There is a problem on the current implementation of the graph: it uses a bitmap in order to detect if the graph traversal entered inside a loop. Also, one of the drivers (vsp1, I think) assumes that the maximum ID for an entity is 31 (as it uses 1 << entity->id).
Due to that, we should have a separate range for entities starting from 0.
That should not affect neither debug printks or userspace, provided that the object type is known, as one could always do:
#define gobj_id(gobj) ( (gobj)->id & ( (1 << 25) - 1) )
dev_dbg(mdev->dev, "MC create: %s#%d\n",
gobj_type[media_gobj_type(gobj)],
gobj_id(gobj));in order to report the ID into a reasonable range.
I'm actually doing that on some debug patches I'm writing right now in order to allow me to test object creation/removal.
Regards,
Mauro
Laurent Pinchart
maintainerRe: [PATCH v4 3/6] media: add a common struct to be embed on media graph objects
Hi Mauro,
Quoted 46 lines · On Saturday 15 August 2015 11:56:18 Mauro Carvalho Chehab wrote:
Em Sat, 15 Aug 2015 00:25:15 +0300 Sakari Ailus escreveu:
Quoted 36 lines · Mauro Carvalho Chehab
Quoted 35 lines
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 latter on.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-entity.c +7
-0@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj 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.
+ */drivers/media/media-entity.c +2
-0+void graph_obj_init(struct media_device *mdev, + enum media_graph_type type,Quoted 8 lines
drivers/media/media-entity.c line 47
Quoted 4 lines
drivers/media/media-entity.c +4
-0+ struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ;Ugh. This will mean the object IDs are going to be huge to begin with, ending up being a nuisance to work with as you often write them by hand. Do we win anything by doing so?
Quoted 4 lines
There is a problem on the current implementation of the graph: it uses a bitmap in order to detect if the graph traversal entered inside a loop. Also, one of the drivers (vsp1, I think) assumes that the maximum ID for an entity is 31 (as it uses 1 <<
entity->id).
If core code or drivers do the wrong thing they should be fixed instead of working around the problem.
A fixed bitmap in the graph walk will just not scale when we'll add support for dynamically adding or removing entities. We thus need to change the algorithm anyway.
The OMAP3 ISP and VSP1 drivers could use fixed size bitmaps as they won't support dynamic addition or removal of entities, so the maximum ID will be known at init time.
For other drivers that have similar needs core helper functions would probably be helpful.
Quoted 17 lines
Due to that, we should have a separate range for entities starting from 0.
That should not affect neither debug printks or userspace, provided that the object type is known, as one could always do:
#define
gobj_id(gobj)( (gobj)->id & ( (1 << 25) - 1) )dev_dbg(mdev->dev, "MC create: %s#%d\n",
gobj_type[media_gobj_type(gobj)], gobj_id(gobj));in order to report the ID into a reasonable range.
I'm actually doing that on some debug patches I'm writing right now in order to allow me to test object creation/removal.
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v4 3/6] media: add a common struct to be embed on media graph objects
Em Sat, 15 Aug 2015 19:42:59 +0300
Laurent Pinchart <[email protected]> escreveu:
Quoted 54 lines
Hi Mauro,
Quoted 46 lines · On Saturday 15 August 2015 11:56:18 Mauro Carvalho Chehab wrote:
Em Sat, 15 Aug 2015 00:25:15 +0300 Sakari Ailus escreveu:
Quoted 36 lines · Mauro Carvalho Chehab
Quoted 35 lines
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 latter on.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-entity.c +7
-0@@ -27,6 +27,39 @@#include <media/media-device.h> /** +* graph_obj_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_graph_obj 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.
+ */drivers/media/media-entity.c +2
-0+void graph_obj_init(struct media_device *mdev, + enum media_graph_type type,Quoted 8 lines
drivers/media/media-entity.c line 47
Quoted 4 lines
drivers/media/media-entity.c +4
-0+ struct media_graph_obj *gobj) +{ +/* An unique object ID will be provided on next patches */ + gobj->id = type <<24 ;Ugh. This will mean the object IDs are going to be huge to begin with, ending up being a nuisance to work with as you often write them by hand. Do we win anything by doing so?
Quoted 4 lines
There is a problem on the current implementation of the graph: it uses a bitmap in order to detect if the graph traversal entered inside a loop. Also, one of the drivers (vsp1, I think) assumes that the maximum ID for an entity is 31 (as it uses 1 <<
entity->id).
Quoted 6 lines
If core code or drivers do the wrong thing they should be fixed instead of working around the problem.
A fixed bitmap in the graph walk will just not scale when we'll add support for dynamically adding or removing entities. We thus need to change the algorithm anyway.
Yes, but such change can happen latter, when we add support for dynamic entity add/removal.
Quoted 3 lines
The OMAP3 ISP and VSP1 drivers could use fixed size bitmaps as they won't support dynamic addition or removal of entities, so the maximum ID will be known at init time.
This is true only if we keep the current ID range for entities, as my patches are doing. If we use a global ID range for all element graphs, the risk of breaking OMAP3 and VSP1 are high.
So, at least for now, we need to preserve the current ID range for entities.
This could be changed on some future patch, but the one doing such change would need to have OMAP3 and VSP1 drivers in order to check if the changes won't break them. If I were to do such changes there, I would actually try to move those driver-specific graph traversal code to the core, as having those spread on drivers make harder to do some core changes.
Quoted 21 lines
For other drivers that have similar needs core helper functions would probably be helpful.
Quoted 17 lines
Due to that, we should have a separate range for entities starting from 0.
That should not affect neither debug printks or userspace, provided that the object type is known, as one could always do:
#define
gobj_id(gobj)( (gobj)->id & ( (1 << 25) - 1) )dev_dbg(mdev->dev, "MC create: %s#%d\n",
gobj_type[media_gobj_type(gobj)], gobj_id(gobj));in order to report the ID into a reasonable range.
I'm actually doing that on some debug patches I'm writing right now in order to allow me to test object creation/removal.
PATCH v4 4/6 media: use media_graph_obj inside entities
In reply to Mauro Carvalho ChehabAs entities are graph elements, let's embeed media_graph_obj 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 on some point, but for now, let's not do this, to avoid needing to review all drivers and the userspace apps.
drivers/media/media-device.c +4 -6
@@ -377,7 +377,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);
@@ -431,11 +430,9 @@ 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); -list_add_tail (&entity->list, &mdev->entities); +/* Initialize media_graph_obj embedded at the entity */ +graph_obj_init (mdev,MEDIA_GRAPH_ENTITY , &entity->graph_obj); +spin_unlock (&mdev->lock);return 0 ;
@@ -457,6 +454,7 @@ void media_device_unregister_entity(struct media_entity *entity)return ;spin_lock (&mdev->lock); +graph_obj_remove (&entity->graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;
drivers/media/media-entity.c +5 -1
@@ -43,8 +43,12 @@ void graph_obj_init(struct media_device *mdev,enum media_graph_type type, struct media_graph_obj *gobj) { -/* An unique object ID will be provided on next patches */ +/* Create a per-type unique object ID */ gobj->id = type << 24; +switch (type) { +case MEDIA_GRAPH_ENTITY : + gobj->id |= ++mdev->entity_id; + } }/**
include/media/media-device.h +3 -1
@@ -41,7 +41,7 @@ struct device;* @bus_info: Unique and stable device location identifier * @hw_revision: Hardware device revision * @driver_version: Device driver version -* @entity_id: ID of the next entity to be registered +* @entity_id: Unique ID used on the last entity registered * @entities: List of registered entities * @lock: Entities list lock * @graph_mutex: Entities graph operation lock
@@ -68,7 +68,9 @@ struct media_device {u32 hw_revision;u32 driver_version; +/* Unique object ID counter */ u32 entity_id; +struct list_head entities;/* Protects the entities list */
include/media/media-entity.h +4 -5
@@ -33,10 +33,10 @@/** * enum media_graph_type - type of a graph element * +* @MEDIA_GRAPH_ENTITY: Identify a media entity */ enum media_graph_type { -/* FIXME: add the types here, as we embeed media_graph_obj */ -MEDIA_GRAPH_NONE +MEDIA_GRAPH_ENTITY , };
@@ -88,10 +88,9 @@ struct media_entity_operations { };struct media_entity { +struct media_graph_obj graph_obj;struct list_head list;struct media_device *parent;/* Media device this entity belongs to*/ -u32 id;/* Entity ID, unique in the parent media -* device context */ const char *name;/* Entity name */ u32 type;/* Entity type (MEDIA_ENT_T_*) */ u32 revision;/* Entity revision, driver specific */
@@ -153,7 +152,7 @@ struct media_entity_graph {int top; }; -#define entity_id (entity) ((entity)->id) +#define entity_id (entity) ((entity)->graph_obj.id)#define gobj_to_entity (gobj) \ container_of(gobj, struct media_entity, graph_obj)
Signature · 1 line
2.4.3
Re: [PATCH v4 4/6] media: use media_graph_obj inside entities
Quoted 1 line · Mauro Carvalho Chehab
As entities are graph elements, let's embeed
media_graph_obj
s/embeed/embed/
Quoted 5 lines
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 on some point, but for now, let's
s/on/at/
Quoted 45 lines
not do this, to avoid needing to review all drivers and the userspace apps.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +4
-6@@ -377,7 +377,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);@@ -431,11 +430,9 @@ 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); -list_add_tail (&entity->list, &mdev->entities); +/* Initialize media_graph_obj embedded at the entity */ +graph_obj_init (mdev,MEDIA_GRAPH_ENTITY , &entity->graph_obj); +spin_unlock (&mdev->lock);return 0 ;@@ -457,6 +454,7 @@ void media_device_unregister_entity(struct media_entity *entity)return ;spin_lock (&mdev->lock); +graph_obj_remove (&entity->graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0
drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 50
Quoted 9 lines
drivers/media/media-entity.c +4
-1@@ -43,8 +43,12 @@ void graph_obj_init(struct media_device *mdev,enum media_graph_type type, struct media_graph_obj *gobj) { -/* An unique object ID will be provided on next patches */ +/* Create a per-type unique object ID */ gobj->id = type << 24; +switch (type) { +case MEDIA_GRAPH_ENTITY : + gobj->id |= ++mdev->entity_id;
Please add a break here.
Quoted 58 lines
drivers/media/media-entity.c +1
-0+ } }/** include/media/media-device.h +3
-1@@ -41,7 +41,7 @@ struct device;* @bus_info: Unique and stable device location identifier * @hw_revision: Hardware device revision * @driver_version: Device driver version -* @entity_id: ID of the next entity to be registered +* @entity_id: Unique ID used on the last entity registered * @entities: List of registered entities * @lock: Entities list lock * @graph_mutex: Entities graph operation lock @@ -68,7 +68,9 @@ struct media_device {u32 hw_revision;u32 driver_version; +/* Unique object ID counter */ u32 entity_id; +struct list_head entities;/* Protects the entities list */ include/media/media-entity.h +3
-4@@ -33,10 +33,10 @@/** * enum media_graph_type - type of a graph element * +* @MEDIA_GRAPH_ENTITY: Identify a media entity */ enum media_graph_type { -/* FIXME: add the types here, as we embeed media_graph_obj */ -MEDIA_GRAPH_NONE +MEDIA_GRAPH_ENTITY , };@@ -88,10 +88,9 @@ struct media_entity_operations { };struct media_entity { +struct media_graph_obj graph_obj;struct list_head list;struct media_device *parent;/* Media device this entity belongs to*/ -u32 id;/* Entity ID, unique in the parent media -* device context */ const char *name;/* Entity name */ u32 type;/* Entity type (MEDIA_ENT_T_*) */ u32 revision;/* Entity revision, driver specific */
include/media/media-entity.h struct media_entity_graph { · line 158
Quoted 8 lines
include/media/media-entity.h +1
-1@@ -153,7 +152,7 @@ struct media_entity_graph {int top; }; -#define entity_id (entity) ((entity)->id) +#define entity_id (entity) ((entity)->graph_obj.id)#define gobj_to_entity (gobj) \ container_of(gobj, struct media_entity, graph_obj)
With the small items above fixed:
Regards,
Hans
Sakari Ailus
maintainerRe: [PATCH v4 4/6] media: use media_graph_obj inside entities
In reply to Mauro Carvalho ChehabHi Mauro,
Quoted 78 lines · Mauro Carvalho Chehab
As entities are graph elements, let's embeed
media_graph_objon 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 on 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]>drivers/media/media-device.c +4
-6@@ -377,7 +377,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);@@ -431,11 +430,9 @@ 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); -list_add_tail (&entity->list, &mdev->entities); +/* Initialize media_graph_obj embedded at the entity */ +graph_obj_init (mdev,MEDIA_GRAPH_ENTITY , &entity->graph_obj); +spin_unlock (&mdev->lock);return 0 ;@@ -457,6 +454,7 @@ void media_device_unregister_entity(struct media_entity *entity)return ;spin_lock (&mdev->lock); +graph_obj_remove (&entity->graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +5
-1@@ -43,8 +43,12 @@ void graph_obj_init(struct media_device *mdev,enum media_graph_type type, struct media_graph_obj *gobj) { -/* An unique object ID will be provided on next patches */ +/* Create a per-type unique object ID */ gobj->id = type << 24; +switch (type) { +case MEDIA_GRAPH_ENTITY : + gobj->id |= ++mdev->entity_id; + } }/** include/media/media-device.h +1
-1@@ -41,7 +41,7 @@ struct device;* @bus_info: Unique and stable device location identifier * @hw_revision: Hardware device revision * @driver_version: Device driver version -* @entity_id: ID of the next entity to be registered +* @entity_id: Unique ID used on the last entity registered * @entities: List of registered entities * @lock: Entities list lock * @graph_mutex: Entities graph operation lock
include/media/media-device.h struct media_device { · line 71
Quoted 4 lines
include/media/media-device.h +1
-0@@ -68,7 +68,9 @@ struct media_device {u32 hw_revision;u32 driver_version; +/* Unique object ID counter */
I think the KernelDoc above should be enough.
Quoted 43 lines
include/media/media-device.h +1
-0u32 entity_id; +struct list_head entities;/* Protects the entities list */ include/media/media-entity.h +4
-5@@ -33,10 +33,10 @@/** * enum media_graph_type - type of a graph element * +* @MEDIA_GRAPH_ENTITY: Identify a media entity */ enum media_graph_type { -/* FIXME: add the types here, as we embeed media_graph_obj */ -MEDIA_GRAPH_NONE +MEDIA_GRAPH_ENTITY , };@@ -88,10 +88,9 @@ struct media_entity_operations { };struct media_entity { +struct media_graph_obj graph_obj;struct list_head list;struct media_device *parent;/* Media device this entity belongs to*/ -u32 id;/* Entity ID, unique in the parent media -* device context */ const char *name;/* Entity name */ u32 type;/* Entity type (MEDIA_ENT_T_*) */ u32 revision;/* Entity revision, driver specific */ @@ -153,7 +152,7 @@ struct media_entity_graph {int top; }; -#define entity_id (entity) ((entity)->id) +#define entity_id (entity) ((entity)->graph_obj.id)#define gobj_to_entity (gobj) \ container_of(gobj, struct media_entity, graph_obj)
Signature · 4 lines
Regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected]
PATCH v4 5/6 media: use media_graph_obj inside pads
In reply to Mauro Carvalho ChehabPADs also need unique object IDs that won't conflict with the entity object IDs.
The pad objects are currently created via media_entity_init() and, once created, never change.
While this will likely change in the future in order to suppory dynamic changes, for now we'll keep PADs as arrays and initialize the media_graph_obj embeeded structs when registering the entity.
drivers/media/media-device.c +11 -0
@@ -425,6 +425,8 @@ EXPORT_SYMBOL_GPL(media_device_unregister);int __must_check media_device_register_entity (struct media_device *mdev,struct media_entity *entity) { +int i; +/* Warn if we apparently re-register an entity */ WARN_ON (entity->parent !=NULL ); entity->parent = mdev;
@@ -433,6 +435,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,/* Initialize media_graph_obj embedded at the entity */ graph_obj_init (mdev,MEDIA_GRAPH_ENTITY , &entity->graph_obj); +/* Initialize objects at the pads */ +for (i =0 ; i < entity->num_pads; i++) +graph_obj_init (mdev,MEDIA_GRAPH_PAD , + &entity->pads[i].graph_obj); + +list_add_tail (&entity->list, &mdev->entities);spin_unlock (&mdev->lock);return 0 ;
@@ -448,6 +456,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;if (mdev ==NULL )
@@ -455,6 +464,8 @@ void media_device_unregister_entity(struct media_entity *entity)spin_lock (&mdev->lock);graph_obj_remove (&entity->graph_obj); +for (i =0 ; i < entity->num_pads; i++) +graph_obj_remove (&entity->pads[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;
drivers/media/media-entity.c +2 -0
@@ -48,6 +48,8 @@ void graph_obj_init(struct media_device *mdev,switch (type) {case MEDIA_GRAPH_ENTITY : gobj->id |= ++mdev->entity_id; +case MEDIA_GRAPH_PAD : + gobj->id |= ++mdev->pad_id; } }
include/media/media-device.h +2 -0
@@ -42,6 +42,7 @@ struct device;* @hw_revision: Hardware device revision * @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 * @entities: List of registered entities * @lock: Entities list lock * @graph_mutex: Entities graph operation lock
@@ -70,6 +71,7 @@ struct media_device {/* Unique object ID counter */ u32 entity_id; +u32 pad_id;struct list_head entities;
include/media/media-entity.h +3 -0
@@ -34,9 +34,11 @@* enum media_graph_type - type of a graph element * * @MEDIA_GRAPH_ENTITY: Identify a media entity +* @MEDIA_GRAPH_PAD: Identify a media pad */ enum media_graph_type {MEDIA_GRAPH_ENTITY , +MEDIA_GRAPH_PAD , };
@@ -66,6 +68,7 @@ struct media_link { };struct media_pad { +struct media_graph_obj graph_obj;struct media_entity *entity;/* Entity this pad belongs to */ u16 index;/* Pad index in the entity pads array */ unsigned long flags;/* Pad flags (MEDIA_PAD_FL_*) */
Signature · 1 line
2.4.3
Re: [PATCH v4 5/6] media: use media_graph_obj inside pads
Quoted 8 lines · Mauro Carvalho Chehab
PADs also need unique object IDs that won't conflict with the entity object IDs.
The pad objects are currently created via
media_entity_init()and, once created, never change.While this will likely change in the future in order to suppory dynamic changes, for now we'll keep PADs as arrays
s/suppory/support/
Quoted 1 line
and initialize the
media_graph_objembeeded structs when
s/embeeded/embedded/
Hmm, I sense a pattern here :-)
Quoted 52 lines
registering the entity.
Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +11
-0@@ -425,6 +425,8 @@ EXPORT_SYMBOL_GPL(media_device_unregister);int __must_check media_device_register_entity (struct media_device *mdev,struct media_entity *entity) { +int i; +/* Warn if we apparently re-register an entity */ WARN_ON (entity->parent !=NULL ); entity->parent = mdev;@@ -433,6 +435,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,/* Initialize media_graph_obj embedded at the entity */ graph_obj_init (mdev,MEDIA_GRAPH_ENTITY , &entity->graph_obj); +/* Initialize objects at the pads */ +for (i =0 ; i < entity->num_pads; i++) +graph_obj_init (mdev,MEDIA_GRAPH_PAD , + &entity->pads[i].graph_obj); + +list_add_tail (&entity->list, &mdev->entities);spin_unlock (&mdev->lock);return 0 ;@@ -448,6 +456,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;if (mdev ==NULL )@@ -455,6 +464,8 @@ void media_device_unregister_entity(struct media_entity *entity)spin_lock (&mdev->lock);graph_obj_remove (&entity->graph_obj); +for (i =0 ; i < entity->num_pads; i++) +graph_obj_remove (&entity->pads[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0
drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 50
Quoted 3 lines
drivers/media/media-entity.c +0
-0@@ -48,6 +48,8 @@ void graph_obj_init(struct media_device *mdev,switch (type) {case MEDIA_GRAPH_ENTITY : gobj->id |= ++mdev->entity_id;
And that's why you should always add a break after a case: this now falls through into the next case, and that's not what you want.
drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 52
Quoted 2 lines
drivers/media/media-entity.c +2
-0+case MEDIA_GRAPH_PAD : + gobj->id |= ++mdev->pad_id;
Ditto: add a break.
Quoted 41 lines
drivers/media/media-entity.c +0
-0} }include/media/media-device.h +2
-0@@ -42,6 +42,7 @@ struct device;* @hw_revision: Hardware device revision * @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 * @entities: List of registered entities * @lock: Entities list lock * @graph_mutex: Entities graph operation lock @@ -70,6 +71,7 @@ struct media_device {/* Unique object ID counter */ u32 entity_id; +u32 pad_id;struct list_head entities;include/media/media-entity.h +2
-0@@ -34,9 +34,11 @@* enum media_graph_type - type of a graph element * * @MEDIA_GRAPH_ENTITY: Identify a media entity +* @MEDIA_GRAPH_PAD: Identify a media pad */ enum media_graph_type {MEDIA_GRAPH_ENTITY , +MEDIA_GRAPH_PAD , };
include/media/media-entity.h struct media_link { · line 74
Quoted 7 lines
include/media/media-entity.h +1
-0@@ -66,6 +68,7 @@ struct media_link { };struct media_pad { +struct media_graph_obj graph_obj;struct media_entity *entity;/* Entity this pad belongs to */ u16 index;/* Pad index in the entity pads array */ unsigned long flags;/* Pad flags (MEDIA_PAD_FL_*) */
After these issues are fixed you can add my:
Regards,
Hans
Sakari Ailus
maintainerRe: [PATCH v4 5/6] media: use media_graph_obj inside pads
In reply to Mauro Carvalho ChehabHi Mauro,
drivers/media/media-device.c EXPORT_SYMBOL_GPL(media_device_register_entity); · line 459
Quoted 5 lines · Mauro Carvalho Chehab
drivers/media/media-device.c +1
-0@@ -448,6 +456,7 @@ EXPORT_SYMBOL_GPL(media_device_register_entity);*/ void media_device_unregister_entity (struct media_entity *entity) { +int i;
I'd declare temporary variables as last.
Quoted 3 lines
drivers/media/media-device.c +0
-0struct media_device *mdev = entity->parent;if (mdev ==NULL )
Signature · 4 lines
Regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected]
PATCH v4 6/6 media: use media_graph_obj inside links
In reply to Mauro Carvalho ChehabJust like entities and pads, links also need to have unique Object IDs along a given media controller.
So, let's add a media_graph_obj inside it and initialize the object then a new link is created.
drivers/media/media-device.c +2 -0
@@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity)graph_obj_remove (&entity->graph_obj);for (i =0 ; i < entity->num_pads; i++)graph_obj_remove (&entity->pads[i].graph_obj); +for (i =0 ; entity->num_links; i++) +graph_obj_remove (&entity->links[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;
drivers/media/media-entity.c +6 -0
@@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev, gobj->id |= ++mdev->entity_id;case MEDIA_GRAPH_PAD : gobj->id |= ++mdev->pad_id; +case MEDIA_GRAPH_LINK : + gobj->id |= ++mdev->pad_id; } }
@@ -469,6 +471,10 @@ static struct media_link *media_entity_add_link(struct media_entity *entity) entity->links = links; } +/* Initialize graph object embedded at the new link */ +graph_obj_init (entity->parent,MEDIA_GRAPH_LINK , + &entity->links[entity->num_links].graph_obj); +return &entity->links[entity->num_links++]; }
include/media/media-device.h +2 -0
@@ -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
@@ -72,6 +73,7 @@ struct media_device {/* Unique object ID counter */ u32 entity_id;u32 pad_id; +u32 link_id;struct list_head entities;
include/media/media-entity.h +3 -0
@@ -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_graph_type {MEDIA_GRAPH_ENTITY ,MEDIA_GRAPH_PAD , +MEDIA_GRAPH_LINK , };
@@ -61,6 +63,7 @@ struct media_pipeline { };struct media_link { +struct media_graph_obj graph_obj;struct media_pad *source;/* Source pad */ struct media_pad *sink;/* Sink pad */ struct media_link *reverse;/* Link in the reverse direction */
Signature · 1 line
2.4.3
Re: [PATCH v4 6/6] media: use media_graph_obj inside links
Quoted 26 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_graph_objinside it and initialize the object then a new link is created.Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +2
-0@@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity)graph_obj_remove (&entity->graph_obj);for (i =0 ; i < entity->num_pads; i++)graph_obj_remove (&entity->pads[i].graph_obj); +for (i =0 ; entity->num_links; i++) +graph_obj_remove (&entity->links[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0
drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 54
Quoted 5 lines
drivers/media/media-entity.c +2
-0@@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev, gobj->id |= ++mdev->entity_id;case MEDIA_GRAPH_PAD : gobj->id |= ++mdev->pad_id; +case MEDIA_GRAPH_LINK : + gobj->id |= ++mdev->pad_id;
Same issue with missing breaks. Also for links you want to use link_id, not pad_id. Clearly a copy-and-paste mistake.
A bigger question is whether you actually need graph_obj for a link? Links are *between* graph objects, but are they really graph objects in their own right?
Currently a link is identified by the source/sink objects and I think that is all you need. I didn't realize that when reviewing the v3 patch series, but I'm now wondering about it.
If you *don't* make links a graph_obj, will anything break or become difficult to use? I don't think so, but let me know if I am overlooking something.
Quoted 53 lines
drivers/media/media-entity.c +4
-0} }@@ -469,6 +471,10 @@ static struct media_link *media_entity_add_link(struct media_entity *entity) entity->links = links; } +/* Initialize graph object embedded at the new link */ +graph_obj_init (entity->parent,MEDIA_GRAPH_LINK , + &entity->links[entity->num_links].graph_obj); +return &entity->links[entity->num_links++]; }include/media/media-device.h +2
-0@@ -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 @@ -72,6 +73,7 @@ struct media_device {/* Unique object ID counter */ u32 entity_id;u32 pad_id; +u32 link_id;struct list_head entities;include/media/media-entity.h +2
-0@@ -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_graph_type {MEDIA_GRAPH_ENTITY ,MEDIA_GRAPH_PAD , +MEDIA_GRAPH_LINK , };
include/media/media-entity.h struct media_pipeline { · line 69
Quoted 7 lines
include/media/media-entity.h +1
-0@@ -61,6 +63,7 @@ struct media_pipeline { };struct media_link { +struct media_graph_obj graph_obj;struct media_pad *source;/* Source pad */ struct media_pad *sink;/* Sink pad */ struct media_link *reverse;/* Link in the reverse direction */
Regards,
Hans
Re: [PATCH v4 6/6] media: use media_graph_obj inside links
Quoted 39 lines · Hans Verkuil
Quoted 26 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_graph_objinside it and initialize the object then a new link is created.Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +2
-0@@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity)graph_obj_remove (&entity->graph_obj);for (i =0 ; i < entity->num_pads; i++)graph_obj_remove (&entity->pads[i].graph_obj); +for (i =0 ; entity->num_links; i++) +graph_obj_remove (&entity->links[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 54
Quoted 5 lines
drivers/media/media-entity.c +2
-0@@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev, gobj->id |= ++mdev->entity_id;case MEDIA_GRAPH_PAD : gobj->id |= ++mdev->pad_id; +case MEDIA_GRAPH_LINK : + gobj->id |= ++mdev->pad_id;Same issue with missing breaks. Also for links you want to use
link_id, notpad_id. Clearly a copy-and-paste mistake.A bigger question is whether you actually need
graph_objfor a link? Links are *between* graph objects, but are they really graph objects in their own right?
Quoted 6 lines
Currently a link is identified by the source/sink objects and I think that is all you need. I didn't realize that when reviewing the v3 patch series, but I'm now wondering about it.
If you *don't* make links a
graph_obj, will anything break or become difficult to use? I don't think so, but let me know if I am overlooking something.
Hmm, you probably want the list part of a graph_obj since you need to link the links (so to speak) in a list. I'm not sure if you need unique IDs for a link object, but it doesn't hurt either. Just ignore my comments about using the graph_obj in media_link.
Regards,
Hans
Re: [PATCH v4 6/6] media: use media_graph_obj inside links
Em Fri, 14 Aug 2015 17:33:28 +0200
Hans Verkuil <[email protected]> escreveu:
Quoted 44 lines · Hans Verkuil
Quoted 43 lines · Mauro Carvalho Chehab
Quoted 26 lines
Just like entities and pads, links also need to have unique Object IDs along a given media controller.
So, let's add a
media_graph_objinside it and initialize the object then a new link is created.Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +2
-0@@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity)graph_obj_remove (&entity->graph_obj);for (i =0 ; i < entity->num_pads; i++)graph_obj_remove (&entity->pads[i].graph_obj); +for (i =0 ; entity->num_links; i++) +graph_obj_remove (&entity->links[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 54
Quoted 5 lines
drivers/media/media-entity.c +2
-0@@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev, gobj->id |= ++mdev->entity_id;case MEDIA_GRAPH_PAD : gobj->id |= ++mdev->pad_id; +case MEDIA_GRAPH_LINK : + gobj->id |= ++mdev->pad_id;Same issue with missing breaks. Also for links you want to use
link_id, notpad_id. Clearly a copy-and-paste mistake.A bigger question is whether you actually need
graph_objfor a link? Links are *between* graph objects, but are they really graph objects in their own right?Currently a link is identified by the source/sink objects and I think that is all you need. I didn't realize that when reviewing the v3 patch series, but I'm now wondering about it.
Quoted 7 lines
Quoted 2 lines
If you *don't* make links a
graph_obj, will anything break or become difficult to use? I don't think so, but let me know if I am overlooking something.Hmm, you probably want the list part of a
graph_objsince you need to link the links (so to speak) in a list. I'm not sure if you need unique IDs for a link object, but it doesn't hurt either. Just ignore my comments about using thegraph_objinmedia_link.
Yes. that's another reason.
As you said, it doesn't hurt to have an unique ID and we'll need it soon enough (Shuah will need it).
Regards,
Mauro
Re: [PATCH v4 6/6] media: use media_graph_obj inside links
In reply to Hans VerkuilEm Fri, 14 Aug 2015 17:18:28 +0200
Hans Verkuil <[email protected]> escreveu:
Quoted 27 lines · Mauro Carvalho Chehab
Quoted 26 lines
Just like entities and pads, links also need to have unique Object IDs along a given media controller.
So, let's add a
media_graph_objinside it and initialize the object then a new link is created.Signed-off-by Mauro Carvalho Chehab <[email protected]>drivers/media/media-device.c +2
-0@@ -466,6 +466,8 @@ void media_device_unregister_entity(struct media_entity *entity)graph_obj_remove (&entity->graph_obj);for (i =0 ; i < entity->num_pads; i++)graph_obj_remove (&entity->pads[i].graph_obj); +for (i =0 ; entity->num_links; i++) +graph_obj_remove (&entity->links[i].graph_obj);list_del (&entity->list);spin_unlock (&mdev->lock); entity->parent =NULL ;drivers/media/media-entity.c +0
-0@@ -50,6 +50,8 @@ void graph_obj_init(struct media_device *mdev,
Quoted 8 lines
drivers/media/media-entity.c void graph_obj_init(struct media_device *mdev, · line 54
Quoted 5 lines
drivers/media/media-entity.c +2
-0gobj->id |= ++mdev->entity_id;case MEDIA_GRAPH_PAD : gobj->id |= ++mdev->pad_id; +case MEDIA_GRAPH_LINK : + gobj->id |= ++mdev->pad_id;Same issue with missing breaks. Also for links you want to use
link_id, notpad_id. Clearly a copy-and-paste mistake.
Fixed.
Quoted 2 lines
A bigger question is whether you actually need
graph_objfor a link? Links are *between* graph objects, but are they really graph objects in their own right?
Yes, a link is a graph object. See any CAD/CAM program and you'll see that they're mapping as such on all I'm aware of. If we either need it here or not, see below.
Quoted 6 lines
Currently a link is identified by the source/sink objects and I think that is all you need. I didn't realize that when reviewing the v3 patch series, but I'm now wondering about it.
If you *don't* make links a
graph_obj, will anything break or become difficult to use? I don't think so, but let me know if I am overlooking something.
There are several reasons why we want links to have a common object and an unique ID.
By having an unique ID, whenever we need to pass the link to, we can use the ID (or, actually, a pointer to the common object).
Also, please remember that the type is now embedded with the ID. So, in order to be able to check what graph element are used, we need either the type or the ID.
A real case, Shuah mentioned via email is that, in order to properly lock between ALSA, V4L and DVB, the pertinent drivers need to be notified when some links, entities (and maybe interfaces) are created (or removed), as the graph creation will be handle by multiple, indepentent drivers.
If we use an unique ID for the links, a single notify function can be used to report if a new graph element is added.
For example, where both entities and link creation needs to be tracked, such function would do be like:
static void notify_topology_change(struct media_graph_obj *gobj)
{
enum media_graph_type type = gobj->id >> 24;
switch (type) {
case MEDIA_GRAPH_ENTITY:
{
struct media_entity *entity = gobj_to_entity(gobj);
/* something */
break;
}
case MEDIA_GRAPH_LINK:
{
struct media_link *link = gobj_to_link(gobj);
/* something else */
break;
}
default:
/* do nothing */
}
}That's just the initial usecase. I'm pretty sure we'll need it when we add support for dynamic creation/removal and, eventually, even at the userspace API, but let's go by parts.
Regards,
Mauro
Sakari Ailus
maintainerHi Mauro,
Quoted 9 lines · Mauro Carvalho Chehab
Those are the initial patches from my previous series of MC changes.
The first patch removes an unused parameter when creating links.
The next 5 patches warrant that all object types (entities, pads and links) will have an unique ID, as agreed at the MC workshop.
They prepare for the addition of the media interfaces and interface links.
Having looked the set through, I don't think the patches in the set are strictly necessary for adding media interfaces. Again, I need to stress I'd very much prefer to keep things simple in order to get support for media interfaces in soon, as I understand your intention is as well.
We could make things more dynamic later on, and represent associations using links --- if there's a use case for that.
I don't as such object the patchset, but my question is: where will this all lead to? I'd like to see that, or at least some more, before finally acking the patches. I sense these should be closely related to supporting the property API rather than media interfaces (or DVB), but unfortunately I won't have time to work on the property API for the following ~ three weeks.
struct media_interface could be pointed to from entities using a statically allocated array of pointers, a bit like links (except that they're not pointers). I think we'd get quite far with this already while making much fewer changes to the framework.
One thing that wasn't discussed at length in the meeting, but which I understood was generally agreed on, was DMA engines as entities (vs. having a pad for the sake of the interface in the video node entity, which is ugly). IMHO a sound foundation is important for the proposed changes.
Just my one euro cent --- got some left from Italy. :-)
Signature · 4 lines
Kind regards, Sakari Ailus e-mail: [email protected] XMPP: [email protected]
Em Sat, 15 Aug 2015 01:37:44 +0300
Sakari Ailus <[email protected]> escreveu:
Quoted 10 lines
Hi Mauro,
Quoted 7 lines · Mauro Carvalho Chehab
Those are the initial patches from my previous series of MC changes.
The first patch removes an unused parameter when creating links.
The next 5 patches warrant that all object types (entities, pads and links) will have an unique ID, as agreed at the MC workshop.
Quoted 7 lines
Quoted 2 lines
They prepare for the addition of the media interfaces and interface links.
Having looked the set through, I don't think the patches in the set are strictly necessary for adding media interfaces. Again, I need to stress I'd very much prefer to keep things simple in order to get support for media interfaces in soon, as I understand your intention is as well.
Well, the new API we've agreed requires unique IDs. So, those patches are a mandatory requirement. Also, the ID is a requirement for links (see patch 9/16 and latter patches on my RFC v3).
Quoted 8 lines
We could make things more dynamic later on, and represent associations using links --- if there's a use case for that.
I don't as such object the patchset, but my question is: where will this all lead to? I'd like to see that, or at least some more, before finally acking the patches. I sense these should be closely related to supporting the property API rather than media interfaces (or DVB), but unfortunately I won't have time to work on the property API for the following ~ three weeks.
Well my goals are different then yours and so my test environment.
Currently, I have only the hybrid PC-consumer TV devices handy for test and my goal is to address what's needed for them to be properly and fully supported. Nothing more, nothing less.
Yet, I'm aiming to future support TV sets and Set Top Boxes. Even the simpler of such hardware would be at least 4x or 5x more complex than a PC consumer devices. So, whenever I need to take some design decision, I'll have those complex hardware in mind.
With regards to properties, I don't intend to touch on them. Those are on your action items, so I'm counting that you'll be doing that ;)
I might need to add something to internally replace the entity->type on some future patch, but I'm not there yet, and, if I need to do, I'll try to do the minimal amount required for my patches to work.
Quoted 4 lines
struct
media_interfacecould be pointed to from entities using a statically allocated array of pointers, a bit like links (except that they're not pointers). I think we'd get quite far with this already while making much fewer changes to the framework.
No, I won't be coding links using arrays, nor using a different struct to represent the links. We need dynamic link addition/removal. Doing it using realloc would fragment the memory and cause lots of harm.
So, my plan is do it right, in a way that will allow us to share the same code and data model, and to future allow graph traversals though interface links too. Graph traversal using both pad/pad and interface/entity links is needed to properly address conflicts when multiple drivers and multiple interfaces may control the same piece of the hardware.
Quoted 4 lines
One thing that wasn't discussed at length in the meeting, but which I understood was generally agreed on, was DMA engines as entities (vs. having a pad for the sake of the interface in the video node entity, which is ugly). IMHO a sound foundation is important for the proposed changes.
Not sure if I understood. Anyway, the comments below are actually unrelated to this patch series, and looks more like a RFC ;)
---
At the V4L2 case, DMA engines will be mapped as entities (on non-USB hardware). We still need to agree how we'll name such entities.
It should be noticed that:
- On V4L2, the video/VBI stream output is not the DMA engine. The DMA engine is actually linked to the USB EHCI/UHCI/xHCI driver. the USB driver sends a block of data to the V4L2 driver, with decapsulate the data packages and copy into a transfer buffer. Such buffer is mapped to userspace or made available via I/O (read() sysctl).
- Some hardware may not provide a stream sink. This is the case of DVB devices that are compliant with the ETSI encryption standards. All userspace can do is to control the pipeline to direct the unencrypted streams to the GPU and to the ASoC hardware;
- On DVB, the DMA is internal to the Kernel, even on PCI/SoC hardware. The Kernel splits the Transport Stream data into several ring buffers. The ring buffers are sent to userspace - currently, only via I/O (read() sysctl. We're planning to improve it, but we need to keep supporting the old way, as several hardware can only work on that mode;
- On Radio, it may or may not have DMA. Some devices work via reading the audio samples on some register. This can even be done on some firmware inside the device. The radio output can be:
- a wire from the radio board into the Motherboard's audio card;
- an I/O read() operation at ALSA.
So, I won't be calling the hardware that may or may not be doing DMA as "DMA". I guess we can simply call it as:
For output:
- video stream output
- vbi stream output
- TS stream output
- audio stream playback
For input:
- video stream capture
- vbi stream capture
- TS stream capture
- audio stream capture
Or something equivalent.
Just my 2 euro cents ;)
Quoted 3 lines
Just my one euro cent --- got some left from Italy. :-)