---------- Forwarded message ----------
From: Muralidharan Karicheri <mkaricheri@xxxxxxxxx>
Date: Sun, Oct 24, 2010 at 10:24 AM
Subject: Re: [RFC/PATCH 3/5] vpif_cap/disp: Added support for DV presets
To: mats.randgaard@xxxxxxxxxxxx
On Fri, Oct 22, 2010 at 3:00 AM, <mats.randgaard@xxxxxxxxxxxx> wrote:
>
> From: Mats Randgaard <mats.randgaard@xxxxxxxxxxxx>
>
> Added functions to set/get/query/enum DV presets.
>
> Signed-off-by: Mats Randgaard <mats.randgaard@xxxxxxxxxxxx>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxxxxx>
> ---
> drivers/media/video/davinci/vpif_capture.c | 143 +++++++++++++++++++++++++++-
> drivers/media/video/davinci/vpif_capture.h | 1 +
> drivers/media/video/davinci/vpif_display.c | 119 ++++++++++++++++++++++-
> drivers/media/video/davinci/vpif_display.h | 1 +
> 4 files changed, 255 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
> index 778af7e..bf1adea 100644
> --- a/drivers/media/video/davinci/vpif_capture.c
> +++ b/drivers/media/video/davinci/vpif_capture.c
> @@ -432,9 +432,18 @@ static int vpif_update_std_info(struct channel_obj *ch)
>
> for (index = 0; index < vpif_ch_params_count; index++) {
> config = &ch_params[index];
> - if (config->stdid & vid_ch->stdid) {
> - memcpy(std_info, config, sizeof(*config));
> - break;
> + if (config->hd_sd == 0) {
> + vpif_dbg(2, debug, "SD format\n");
> + if (config->stdid & vid_ch->stdid) {
> + memcpy(std_info, config, sizeof(*config));
> + break;
> + }
> + } else {
> + vpif_dbg(2, debug, "HD format\n");
> + if (config->dv_preset == vid_ch->dv_preset) {
> + memcpy(std_info, config, sizeof(*config));
> + break;
> + }
> }
> }
>
> @@ -1442,6 +1451,7 @@ static int vpif_s_std(struct file *file, void *priv, v4l2_std_id *std_id)
> return -ERESTARTSYS;
>
> ch->video.stdid = *std_id;
> + ch->video.dv_preset = V4L2_DV_INVALID;
>
> /* Get the information about the standard */
> if (vpif_update_std_info(ch)) {
> @@ -1794,6 +1804,129 @@ static int vpif_cropcap(struct file *file, void *priv,
> return 0;
> }
>
> +/**
> + * vpif_enum_dv_presets() - ENUM_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_enum_dv_presets(struct file *file, void *priv,
> + struct v4l2_dv_enum_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> +
> + if (!vpif_obj.sd) {
> + vpif_dbg(2, debug, "No sub devices registered\n");
> +
> + if (preset->index >= vpif_ch_params_count)
> + return -EINVAL;
> +
> + /* dv-presets only */
> + if (ch_params[preset->index].hd_sd == 0)
> + return -EINVAL;
> +
> + return v4l_fill_dv_preset_info(
> + ch_params[preset->index].dv_preset, preset);
> + }
if (!vpif_obj.sd) check is not needed since it is already checked in
probe() as mentioned in other patch.
> +
> + return v4l2_subdev_call(vpif_obj.sd[ch->curr_sd_index],
> + video, enum_dv_presets, preset);
> +}
> +
> +/**
> + * vpif_query_dv_presets() - QUERY_DV_PRESET handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_query_dv_preset(struct file *file, void *priv,
> + struct v4l2_dv_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> +
> + if (!vpif_obj.sd) {
> + vpif_dbg(2, debug, "No sub devices registered\n");
> + return -EINVAL;
> + }
> +
Ditto.
>
> + return v4l2_subdev_call(vpif_obj.sd[ch->curr_sd_index],
> + video, query_dv_preset, preset);
> +}
> +/**
> + * vpif_s_dv_presets() - S_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_s_dv_preset(struct file *file, void *priv,
> + struct v4l2_dv_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> + struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
> + int ret = 0;
> +
> + if (common->started) {
> + vpif_err("streaming in progress\n");
> + return -EBUSY;
> + }
Please replace vpif_err() with vpif_dbg() since we don't want to flood
the console with unwanted print messages. Also if possible please
update the vpif_capture and vpif_display for similar instances.
>
> +
> + if ((VPIF_CHANNEL0_VIDEO == ch->channel_id) ||
> + (VPIF_CHANNEL1_VIDEO == ch->channel_id)) {
> + if (!fh->initialized) {
> + vpif_dbg(1, debug, "Channel Busy\n");
> + return -EBUSY;
> + }
> + }
> +
> + ret = v4l2_prio_check(&ch->prio, fh->prio);
> + if (ret)
> + return ret;
> +
> + fh->initialized = 1;
> +
> + /* Call encoder subdevice function to set the standard */
> + if (mutex_lock_interruptible(&common->lock))
> + return -ERESTARTSYS;
> +
> + ch->video.dv_preset = preset->preset;
> + ch->video.stdid = V4L2_STD_UNKNOWN;
> +
Shouldn't we move the above two lines to the else part below ?
>
> + /* Get the information about the standard */
> + if (vpif_update_std_info(ch)) {
> + ret = -EINVAL;
> + vpif_err("Error getting the standard info\n");
> + } else {
> + /* Configure the default format information */
> + vpif_config_format(ch);
> +
> + ret = v4l2_subdev_call(vpif_obj.sd[ch->curr_sd_index],
> + video, s_dv_preset, preset);
> + }
> +
> + mutex_unlock(&common->lock);
> +
> + return ret;
> +}
> +/**
> + * vpif_g_dv_presets() - G_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_g_dv_preset(struct file *file, void *priv,
> + struct v4l2_dv_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> +
> + preset->preset = ch->video.dv_preset;
> +
> + return 0;
> +}
> +
> /*
> * vpif_g_chip_ident() - Identify the chip
> * @file: file ptr
> @@ -1896,6 +2029,10 @@ static const struct v4l2_ioctl_ops vpif_ioctl_ops = {
> .vidioc_streamon = vpif_streamon,
> .vidioc_streamoff = vpif_streamoff,
> .vidioc_cropcap = vpif_cropcap,
> + .vidioc_enum_dv_presets = vpif_enum_dv_presets,
> + .vidioc_s_dv_preset = vpif_s_dv_preset,
> + .vidioc_g_dv_preset = vpif_g_dv_preset,
> + .vidioc_query_dv_preset = vpif_query_dv_preset,
> .vidioc_g_chip_ident = vpif_g_chip_ident,
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> .vidioc_g_register = vpif_dbg_g_register,
> diff --git a/drivers/media/video/davinci/vpif_capture.h b/drivers/media/video/davinci/vpif_capture.h
> index 4e12ec8..3452a8a 100644
> --- a/drivers/media/video/davinci/vpif_capture.h
> +++ b/drivers/media/video/davinci/vpif_capture.h
> @@ -59,6 +59,7 @@ struct video_obj {
> enum v4l2_field buf_field;
> /* Currently selected or default standard */
> v4l2_std_id stdid;
> + u32 dv_preset;
> /* This is to track the last input that is passed to application */
> u32 input_idx;
> };
> diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
> index edfc095..4554971 100644
> --- a/drivers/media/video/davinci/vpif_display.c
> +++ b/drivers/media/video/davinci/vpif_display.c
> @@ -373,15 +373,23 @@ static int vpif_get_std_info(struct channel_obj *ch)
>
> int index;
>
> - std_info->stdid = vid_ch->stdid;
> - if (!std_info->stdid)
> - return -1;
> + if (!vid_ch->stdid && !vid_ch->dv_preset)
> + return -EINVAL;
>
> for (index = 0; index < vpif_ch_params_count; index++) {
> config = &ch_params[index];
> - if (config->stdid & std_info->stdid) {
> - memcpy(std_info, config, sizeof(*config));
> - break;
> + if (config->hd_sd == 0) {
> + vpif_dbg(2, debug, "SD format\n");
> + if (config->stdid & vid_ch->stdid) {
> + memcpy(std_info, config, sizeof(*config));
> + break;
> + }
> + } else {
> + vpif_dbg(2, debug, "HD format\n");
> + if (config->dv_preset == vid_ch->dv_preset) {
> + memcpy(std_info, config, sizeof(*config));
> + break;
> + }
> }
> }
>
> @@ -1305,6 +1313,102 @@ static int vpif_s_priority(struct file *file, void *priv, enum v4l2_priority p)
> return v4l2_prio_change(&ch->prio, &fh->prio, p);
> }
>
> +/**
> + * vpif_enum_dv_presets() - ENUM_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_enum_dv_presets(struct file *file, void *priv,
> + struct v4l2_dv_enum_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> + struct video_obj *vid_ch = &ch->video;
> +
> + if (!vpif_obj.sd) {
> + vpif_dbg(2, debug, "No sub devices registered\n");
> +
If check not needed as mentioned before.
>
> + if (preset->index >= vpif_ch_params_count)
> + return -EINVAL;
> +
> + /* dv-presets only */
> + if (ch_params[preset->index].hd_sd == 0)
> + return -EINVAL;
> +
> + return v4l_fill_dv_preset_info(
> + ch_params[preset->index].dv_preset, preset);
> + }
> +
> + return v4l2_subdev_call(vpif_obj.sd[vid_ch->output_id],
> + video, enum_dv_presets, preset);
> +}
> +
> +/**
> + * vpif_s_dv_presets() - S_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_s_dv_preset(struct file *file, void *priv,
> + struct v4l2_dv_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> + struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
> + struct video_obj *vid_ch = &ch->video;
> + int ret = 0;
> +
> + if (common->started) {
> + vpif_err("streaming in progress\n");
Replace vpif_err() with vpif_dbg()
>
> + return -EBUSY;
> + }
> +
> + ret = v4l2_prio_check(&ch->prio, fh->prio);
> + if (ret != 0)
> + return ret;
> +
> + fh->initialized = 1;
> +
> + /* Call encoder subdevice function to set the standard */
> + if (mutex_lock_interruptible(&common->lock))
> + return -ERESTARTSYS;
> +
> + ch->video.dv_preset = preset->preset;
> + ch->video.stdid = V4L2_STD_UNKNOWN;
> +
Move this to else part below.
>
> + /* Get the information about the standard */
> + if (vpif_get_std_info(ch)) {
> + ret = -EINVAL;
> + vpif_err("Error getting the standard info\n");
> + } else {
> + /* Configure the default format information */
> + vpif_config_format(ch);
> +
> + ret = v4l2_subdev_call(vpif_obj.sd[vid_ch->output_id],
> + video, s_dv_preset, preset);
> + }
> +
> + mutex_unlock(&common->lock);
> +
> + return ret;
> +}
> +/**
> + * vpif_g_dv_presets() - G_DV_PRESETS handler
> + * @file: file ptr
> + * @priv: file handle
> + * @preset: input preset
> + */
> +static int vpif_g_dv_preset(struct file *file, void *priv,
> + struct v4l2_dv_preset *preset)
> +{
> + struct vpif_fh *fh = priv;
> + struct channel_obj *ch = fh->channel;
> +
> + preset->preset = ch->video.dv_preset;
> +
> + return 0;
> +}
>
> /*
> * vpif_g_chip_ident() - Identify the chip
> @@ -1410,6 +1514,9 @@ static const struct v4l2_ioctl_ops vpif_ioctl_ops = {
> .vidioc_s_output = vpif_s_output,
> .vidioc_g_output = vpif_g_output,
> .vidioc_cropcap = vpif_cropcap,
> + .vidioc_enum_dv_presets = vpif_enum_dv_presets,
> + .vidioc_s_dv_preset = vpif_s_dv_preset,
> + .vidioc_g_dv_preset = vpif_g_dv_preset,
> .vidioc_g_chip_ident = vpif_g_chip_ident,
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> .vidioc_g_register = vpif_dbg_g_register,
> diff --git a/drivers/media/video/davinci/vpif_display.h b/drivers/media/video/davinci/vpif_display.h
> index a2a7cd1..3d56b3e 100644
> --- a/drivers/media/video/davinci/vpif_display.h
> +++ b/drivers/media/video/davinci/vpif_display.h
> @@ -67,6 +67,7 @@ struct video_obj {
> * most recent displayed frame only */
> v4l2_std_id stdid; /* Currently selected or default
> * standard */
> + u32 dv_preset;
> u32 output_id; /* Current output id */
> };
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Murali Karicheri
mkaricheri@xxxxxxxxx
--
Murali Karicheri
mkaricheri@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
