Re: [RFC PATCH 11/18] s5p-tv: remove dv_preset support from mixer_video.

[Tomasz Stanislawski <t.stanislaws@samsung.com>]

Hi Hans,
Please refer to the comments below.

On 02/16/2013 10:28 AM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The dv_preset API is deprecated and is replaced by the much improved dv_timings
> API. Remove the dv_preset support from this driver as this will allow us to
> remove the dv_preset API altogether (s5p-tv being the last user of this code).
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/s5p-tv/mixer_video.c |   68 ++-------------------------
>  1 file changed, 3 insertions(+), 65 deletions(-)
> 

[snip]

>  static int mxr_enum_dv_timings(struct file *file, void *fh,
>  	struct v4l2_enum_dv_timings *timings)
>  {
> @@ -584,7 +526,7 @@ static int mxr_s_dv_timings(struct file *file, void *fh,
>  	/* lock protects from changing sd_out */
>  	mutex_lock(&mdev->mutex);
>  
> -	/* preset change cannot be done while there is an entity
> +	/* timings change cannot be done while there is an entity
>  	 * dependant on output configuration
>  	 */
>  	if (mdev->n_output > 0) {
> @@ -689,8 +631,8 @@ static int mxr_enum_output(struct file *file, void *fh, struct v4l2_output *a)
>  	/* try to obtain supported tv norms */
>  	v4l2_subdev_call(sd, video, g_tvnorms_output, &a->std);
>  	a->capabilities = 0;
> -	if (sd->ops->video && sd->ops->video->s_dv_preset)
> -		a->capabilities |= V4L2_OUT_CAP_PRESETS;

Could you move the lines below to the patch named
"[RFC PATCH 08/18] s5p-tv: add dv_timings support for mixer_video.".

> +	if (sd->ops->video && sd->ops->video->s_dv_timings)
> +		a->capabilities |= V4L2_OUT_CAP_DV_TIMINGS;

Don't you think that all "add" patches should go in reverse order?
I mean that dv_timings hdmiphy should be applied before hdmi. The hdmi before mixer.
This way all non-functional features would stay invisible from user-space until
they become functional.

>  	if (sd->ops->video && sd->ops->video->s_std_output)
>  		a->capabilities |= V4L2_OUT_CAP_STD;
>  	a->type = V4L2_OUTPUT_TYPE_ANALOG;
> @@ -811,10 +753,6 @@ static const struct v4l2_ioctl_ops mxr_ioctl_ops = {
>  	/* Streaming control */
>  	.vidioc_streamon = mxr_streamon,
>  	.vidioc_streamoff = mxr_streamoff,
> -	/* Preset functions */
> -	.vidioc_enum_dv_presets = mxr_enum_dv_presets,
> -	.vidioc_s_dv_preset = mxr_s_dv_preset,
> -	.vidioc_g_dv_preset = mxr_g_dv_preset,
>  	/* DV Timings functions */
>  	.vidioc_enum_dv_timings = mxr_enum_dv_timings,
>  	.vidioc_s_dv_timings = mxr_s_dv_timings,
>