Re: [PATCH v10 04/30] rcar-vin: move subdevice handling to async callbacks
Hi Niklas, Thank you for the patch. On Monday, 29 January 2018 18:34:09 EET Niklas Söderlund wrote: > In preparation for Gen3 support move the subdevice initialization and > clean up from rvin_v4l2_{register,unregister}() directly to the async > callbacks. This simplifies the addition of Gen3 support as the > rvin_v4l2_register() can be shared for both Gen2 and Gen3 while direct > subdevice control are only used on Gen2. > > While moving this code drop a large comment which is copied from the > framework documentation and fold rvin_mbus_supported() into its only > caller. Also move the initialization and cleanup code to separate > functions to increase readability. > > Signed-off-by: Niklas SöderlundReviewed-by: Laurent Pinchart > --- > drivers/media/platform/rcar-vin/rcar-core.c | 108 ++-- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 35 - > 2 files changed, 74 insertions(+), 69 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c > b/drivers/media/platform/rcar-vin/rcar-core.c index > 47f06acde2e698f2..663309ca9c04f208 100644 > --- a/drivers/media/platform/rcar-vin/rcar-core.c > +++ b/drivers/media/platform/rcar-vin/rcar-core.c > @@ -46,46 +46,88 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int > direction) return -EINVAL; > } > > -static bool rvin_mbus_supported(struct rvin_graph_entity *entity) > +/* The vin lock shuld be held when calling the subdevice attach and detach > */ +static int rvin_digital_subdevice_attach(struct rvin_dev *vin, > + struct v4l2_subdev *subdev) > { > - struct v4l2_subdev *sd = entity->subdev; > struct v4l2_subdev_mbus_code_enum code = { > .which = V4L2_SUBDEV_FORMAT_ACTIVE, > }; > + int ret; > > + /* Find source and sink pad of remote subdevice */ > + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE); > + if (ret < 0) > + return ret; > + vin->digital->source_pad = ret; > + > + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK); > + vin->digital->sink_pad = ret < 0 ? 0 : ret; > + > + /* Find compatible subdevices mbus format */ > + vin->digital->code = 0; > code.index = 0; > - code.pad = entity->source_pad; > - while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, )) { > + code.pad = vin->digital->source_pad; > + while (!vin->digital->code && > +!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) { > code.index++; > switch (code.code) { > case MEDIA_BUS_FMT_YUYV8_1X16: > case MEDIA_BUS_FMT_UYVY8_2X8: > case MEDIA_BUS_FMT_UYVY10_2X10: > case MEDIA_BUS_FMT_RGB888_1X24: > - entity->code = code.code; > - return true; > + vin->digital->code = code.code; > + vin_dbg(vin, "Found media bus format for %s: %d\n", > + subdev->name, vin->digital->code); > + break; > default: > break; > } > } > > - return false; > -} > - > -static int rvin_digital_notify_complete(struct v4l2_async_notifier > *notifier) -{ > - struct rvin_dev *vin = notifier_to_vin(notifier); > - int ret; > - > - /* Verify subdevices mbus format */ > - if (!rvin_mbus_supported(vin->digital)) { > + if (!vin->digital->code) { > vin_err(vin, "Unsupported media bus format for %s\n", > - vin->digital->subdev->name); > + subdev->name); > return -EINVAL; > } > > - vin_dbg(vin, "Found media bus format for %s: %d\n", > - vin->digital->subdev->name, vin->digital->code); > + /* Read tvnorms */ > + ret = v4l2_subdev_call(subdev, video, g_tvnorms, >vdev.tvnorms); > + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) > + return ret; > + > + /* Add the controls */ > + ret = v4l2_ctrl_handler_init(>ctrl_handler, 16); > + if (ret < 0) > + return ret; > + > + ret = v4l2_ctrl_add_handler(>ctrl_handler, subdev->ctrl_handler, > + NULL); > + if (ret < 0) { > + v4l2_ctrl_handler_free(>ctrl_handler); > + return ret; > + } > + > + vin->vdev.ctrl_handler = >ctrl_handler; > + > + vin->digital->subdev = subdev; > + > + return 0; > +} > + > +static void rvin_digital_subdevice_detach(struct rvin_dev *vin) > +{ > + rvin_v4l2_unregister(vin); > + v4l2_ctrl_handler_free(>ctrl_handler); > + > + vin->vdev.ctrl_handler = NULL; > + vin->digital->subdev = NULL; > +} > + > +static int rvin_digital_notify_complete(struct v4l2_async_notifier > *notifier) +{ > + struct rvin_dev *vin = notifier_to_vin(notifier); > + int ret; > > ret =
[PATCH v10 04/30] rcar-vin: move subdevice handling to async callbacks
In preparation for Gen3 support move the subdevice initialization and clean up from rvin_v4l2_{register,unregister}() directly to the async callbacks. This simplifies the addition of Gen3 support as the rvin_v4l2_register() can be shared for both Gen2 and Gen3 while direct subdevice control are only used on Gen2. While moving this code drop a large comment which is copied from the framework documentation and fold rvin_mbus_supported() into its only caller. Also move the initialization and cleanup code to separate functions to increase readability. Signed-off-by: Niklas Söderlund--- drivers/media/platform/rcar-vin/rcar-core.c | 108 +++- drivers/media/platform/rcar-vin/rcar-v4l2.c | 35 - 2 files changed, 74 insertions(+), 69 deletions(-) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index 47f06acde2e698f2..663309ca9c04f208 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -46,46 +46,88 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int direction) return -EINVAL; } -static bool rvin_mbus_supported(struct rvin_graph_entity *entity) +/* The vin lock shuld be held when calling the subdevice attach and detach */ +static int rvin_digital_subdevice_attach(struct rvin_dev *vin, +struct v4l2_subdev *subdev) { - struct v4l2_subdev *sd = entity->subdev; struct v4l2_subdev_mbus_code_enum code = { .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; + int ret; + /* Find source and sink pad of remote subdevice */ + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE); + if (ret < 0) + return ret; + vin->digital->source_pad = ret; + + ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK); + vin->digital->sink_pad = ret < 0 ? 0 : ret; + + /* Find compatible subdevices mbus format */ + vin->digital->code = 0; code.index = 0; - code.pad = entity->source_pad; - while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, )) { + code.pad = vin->digital->source_pad; + while (!vin->digital->code && + !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) { code.index++; switch (code.code) { case MEDIA_BUS_FMT_YUYV8_1X16: case MEDIA_BUS_FMT_UYVY8_2X8: case MEDIA_BUS_FMT_UYVY10_2X10: case MEDIA_BUS_FMT_RGB888_1X24: - entity->code = code.code; - return true; + vin->digital->code = code.code; + vin_dbg(vin, "Found media bus format for %s: %d\n", + subdev->name, vin->digital->code); + break; default: break; } } - return false; -} - -static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) -{ - struct rvin_dev *vin = notifier_to_vin(notifier); - int ret; - - /* Verify subdevices mbus format */ - if (!rvin_mbus_supported(vin->digital)) { + if (!vin->digital->code) { vin_err(vin, "Unsupported media bus format for %s\n", - vin->digital->subdev->name); + subdev->name); return -EINVAL; } - vin_dbg(vin, "Found media bus format for %s: %d\n", - vin->digital->subdev->name, vin->digital->code); + /* Read tvnorms */ + ret = v4l2_subdev_call(subdev, video, g_tvnorms, >vdev.tvnorms); + if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) + return ret; + + /* Add the controls */ + ret = v4l2_ctrl_handler_init(>ctrl_handler, 16); + if (ret < 0) + return ret; + + ret = v4l2_ctrl_add_handler(>ctrl_handler, subdev->ctrl_handler, + NULL); + if (ret < 0) { + v4l2_ctrl_handler_free(>ctrl_handler); + return ret; + } + + vin->vdev.ctrl_handler = >ctrl_handler; + + vin->digital->subdev = subdev; + + return 0; +} + +static void rvin_digital_subdevice_detach(struct rvin_dev *vin) +{ + rvin_v4l2_unregister(vin); + v4l2_ctrl_handler_free(>ctrl_handler); + + vin->vdev.ctrl_handler = NULL; + vin->digital->subdev = NULL; +} + +static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier) +{ + struct rvin_dev *vin = notifier_to_vin(notifier); + int ret; ret = v4l2_device_register_subdev_nodes(>v4l2_dev); if (ret < 0) { @@ -103,8 +145,10 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier, struct rvin_dev *vin = notifier_to_vin(notifier); vin_dbg(vin, "unbind