Re: [PATCH v8 04/28] rcar-vin: move subdevice handling to async callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/29/2017 08:43 PM, 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.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

Reviewed-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>

Regards,

	Hans

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 105 ++++++++++++++++++----------
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  35 ----------
>  2 files changed, 67 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 6d99542ec74b49a7..6ab51acd676641ec 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -46,47 +46,11 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int direction)
>  	return -EINVAL;
>  }
>  
> -static bool rvin_mbus_supported(struct rvin_graph_entity *entity)
> -{
> -	struct v4l2_subdev *sd = entity->subdev;
> -	struct v4l2_subdev_mbus_code_enum code = {
> -		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> -	};
> -
> -	code.index = 0;
> -	code.pad = entity->source_pad;
> -	while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
> -		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;
> -		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)) {
> -		vin_err(vin, "Unsupported media bus format for %s\n",
> -			vin->digital->subdev->name);
> -		return -EINVAL;
> -	}
> -
> -	vin_dbg(vin, "Found media bus format for %s: %d\n",
> -		vin->digital->subdev->name, vin->digital->code);
> -
>  	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
>  	if (ret < 0) {
>  		vin_err(vin, "Failed to register subdev nodes\n");
> @@ -103,8 +67,16 @@ static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
>  
>  	vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
> +
> +	mutex_lock(&vin->lock);
> +
>  	rvin_v4l2_unregister(vin);
> +	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +
> +	vin->vdev.ctrl_handler = NULL;
>  	vin->digital->subdev = NULL;
> +
> +	mutex_unlock(&vin->lock);
>  }
>  
>  static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
> @@ -112,12 +84,14 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
>  				     struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
> +	struct v4l2_subdev_mbus_code_enum code = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
>  	int ret;
>  
>  	v4l2_set_subdev_hostdata(subdev, vin);
>  
>  	/* Find source and sink pad of remote subdevice */
> -
>  	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
>  	if (ret < 0)
>  		return ret;
> @@ -126,21 +100,74 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
>  	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 = vin->digital->source_pad;
> +	while (!vin->digital->code &&
> +	       !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
> +		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:
> +			vin->digital->code = code.code;
> +			vin_dbg(vin, "Found media bus format for %s: %d\n",
> +				subdev->name, vin->digital->code);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	if (!vin->digital->code) {
> +		vin_err(vin, "Unsupported media bus format for %s\n",
> +			subdev->name);
> +		return -EINVAL;
> +	}
> +
> +	/* Read tvnorms */
> +	ret = v4l2_subdev_call(subdev, video, g_tvnorms, &vin->vdev.tvnorms);
> +	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> +		return ret;
> +
> +	mutex_lock(&vin->lock);
> +
> +	/* Add the controls */
> +	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16);
> +	if (ret < 0)
> +		goto err;
> +
> +	ret = v4l2_ctrl_add_handler(&vin->ctrl_handler, subdev->ctrl_handler,
> +				    NULL);
> +	if (ret < 0)
> +		goto err_ctrl;
> +
> +	vin->vdev.ctrl_handler = &vin->ctrl_handler;
> +
>  	vin->digital->subdev = subdev;
>  
> +	mutex_unlock(&vin->lock);
> +
>  	vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
>  		subdev->name, vin->digital->source_pad,
>  		vin->digital->sink_pad);
>  
>  	return 0;
> +err_ctrl:
> +	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +err:
> +	mutex_unlock(&vin->lock);
> +	return ret;
>  }
> +
>  static const struct v4l2_async_notifier_operations rvin_digital_notify_ops = {
>  	.bound = rvin_digital_notify_bound,
>  	.unbind = rvin_digital_notify_unbind,
>  	.complete = rvin_digital_notify_complete,
>  };
>  
> -
>  static int rvin_digital_parse_v4l2(struct device *dev,
>  				   struct v4l2_fwnode_endpoint *vep,
>  				   struct v4l2_async_subdev *asd)
> @@ -277,6 +304,8 @@ static int rcar_vin_remove(struct platform_device *pdev)
>  	v4l2_async_notifier_unregister(&vin->notifier);
>  	v4l2_async_notifier_cleanup(&vin->notifier);
>  
> +	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +
>  	rvin_dma_unregister(vin);
>  
>  	return 0;
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 32a658214f48fa49..4a0610a6b4503501 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -847,9 +847,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
>  	v4l2_info(&vin->v4l2_dev, "Removing %s\n",
>  		  video_device_node_name(&vin->vdev));
>  
> -	/* Checks internaly if handlers have been init or not */
> -	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> -
>  	/* Checks internaly if vdev have been init or not */
>  	video_unregister_device(&vin->vdev);
>  }
> @@ -872,41 +869,10 @@ static void rvin_notify(struct v4l2_subdev *sd,
>  int rvin_v4l2_register(struct rvin_dev *vin)
>  {
>  	struct video_device *vdev = &vin->vdev;
> -	struct v4l2_subdev *sd = vin_to_source(vin);
>  	int ret;
>  
> -	v4l2_set_subdev_hostdata(sd, vin);
> -
>  	vin->v4l2_dev.notify = rvin_notify;
>  
> -	ret = v4l2_subdev_call(sd, video, g_tvnorms, &vin->vdev.tvnorms);
> -	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> -		return ret;
> -
> -	if (vin->vdev.tvnorms == 0) {
> -		/* Disable the STD API if there are no tvnorms defined */
> -		v4l2_disable_ioctl(&vin->vdev, VIDIOC_G_STD);
> -		v4l2_disable_ioctl(&vin->vdev, VIDIOC_S_STD);
> -		v4l2_disable_ioctl(&vin->vdev, VIDIOC_QUERYSTD);
> -		v4l2_disable_ioctl(&vin->vdev, VIDIOC_ENUMSTD);
> -	}
> -
> -	/* Add the controls */
> -	/*
> -	 * Currently the subdev with the largest number of controls (13) is
> -	 * ov6550. So let's pick 16 as a hint for the control handler. Note
> -	 * that this is a hint only: too large and you waste some memory, too
> -	 * small and there is a (very) small performance hit when looking up
> -	 * controls in the internal hash.
> -	 */
> -	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = v4l2_ctrl_add_handler(&vin->ctrl_handler, sd->ctrl_handler, NULL);
> -	if (ret < 0)
> -		return ret;
> -
>  	/* video node */
>  	vdev->fops = &rvin_fops;
>  	vdev->v4l2_dev = &vin->v4l2_dev;
> @@ -915,7 +881,6 @@ int rvin_v4l2_register(struct rvin_dev *vin)
>  	vdev->release = video_device_release_empty;
>  	vdev->ioctl_ops = &rvin_ioctl_ops;
>  	vdev->lock = &vin->lock;
> -	vdev->ctrl_handler = &vin->ctrl_handler;
>  	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
>  		V4L2_CAP_READWRITE;
>  
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux