diff mbox

[v5,4/4] rcar-vin: implement EDID control ioctls

Message ID 1467819576-17743-5-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Ulrich Hecht July 6, 2016, 3:39 p.m. UTC 
Adds G_EDID and S_EDID.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

kernel test robot July 6, 2016, 4:54 p.m. UTC | #1 
Hi,

[auto build test ERROR on next-20160706]
[cannot apply to linuxtv-media/master renesas/next v4.7-rc6 v4.7-rc5 v4.7-rc4 v4.7-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ulrich-Hecht/media-adv7604-automatic-default-input-selection/20160706-234332
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/media/platform/rcar-vin/rcar-v4l2.c: In function 'rvin_g_edid':
>> drivers/media/platform/rcar-vin/rcar-v4l2.c:561:9: error: implicit declaration of function 'rvin_subdev_call' [-Werror=implicit-function-declaration]
     return rvin_subdev_call(vin, pad, get_edid, edid);
            ^~~~~~~~~~~~~~~~
>> drivers/media/platform/rcar-vin/rcar-v4l2.c:561:31: error: 'pad' undeclared (first use in this function)
     return rvin_subdev_call(vin, pad, get_edid, edid);
                                  ^~~
   drivers/media/platform/rcar-vin/rcar-v4l2.c:561:31: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/media/platform/rcar-vin/rcar-v4l2.c:561:36: error: 'get_edid' undeclared (first use in this function)
     return rvin_subdev_call(vin, pad, get_edid, edid);
                                       ^~~~~~~~
   drivers/media/platform/rcar-vin/rcar-v4l2.c: In function 'rvin_s_edid':
   drivers/media/platform/rcar-vin/rcar-v4l2.c:568:31: error: 'pad' undeclared (first use in this function)
     return rvin_subdev_call(vin, pad, set_edid, edid);
                                  ^~~
>> drivers/media/platform/rcar-vin/rcar-v4l2.c:568:36: error: 'set_edid' undeclared (first use in this function)
     return rvin_subdev_call(vin, pad, set_edid, edid);
                                       ^~~~~~~~
>> drivers/media/platform/rcar-vin/rcar-v4l2.c:569:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^
   drivers/media/platform/rcar-vin/rcar-v4l2.c: In function 'rvin_g_edid':
   drivers/media/platform/rcar-vin/rcar-v4l2.c:562:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^
   cc1: some warnings being treated as errors

vim +/rvin_subdev_call +561 drivers/media/platform/rcar-vin/rcar-v4l2.c

   555	}
   556	
   557	static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
   558	{
   559		struct rvin_dev *vin = video_drvdata(file);
   560	
 > 561		return rvin_subdev_call(vin, pad, get_edid, edid);
   562	}
   563	
   564	static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
   565	{
   566		struct rvin_dev *vin = video_drvdata(file);
   567	
 > 568		return rvin_subdev_call(vin, pad, set_edid, edid);
 > 569	}
   570	
   571	static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
   572		.vidioc_querycap		= rvin_querycap,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Niklas Söderlund July 7, 2016, 12:16 a.m. UTC | #2 
Hi Ulrich,

Thanks for your patch.

On 2016-07-06 17:39:36 +0200, Ulrich Hecht wrote:
> Adds G_EDID and S_EDID.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 396eabc..bd8f14c 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -661,6 +661,20 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>  	return ret;
>  }
>  
> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> +	struct rvin_dev *vin = video_drvdata(file);
> +
> +	return rvin_subdev_call(vin, pad, get_edid, edid);

You need to add a translation from the rcar-vin drivers view of it's 
current input to the subdevices view of how it's pads are arranged. I 
think something like this would work:

    struct rvin_dev *vin = video_drvdata(file);
    unsigned int input;
    int ret;

    input = edid->pad;

    edid->pad = vin->inputs[input].sink_idx;

    ret = vin_subdev_call(vin, pad, get_edid, edid);

    edid->pad = input;

    return ret;

I know it's not obvious you need this and I can't figure out a better 
way to solve runtime switching of subdevices. Any ideas on how to 
improve the situation are more then welcome :-)

> +}
> +
> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> +	struct rvin_dev *vin = video_drvdata(file);
> +
> +	return rvin_subdev_call(vin, pad, set_edid, edid);

Same comment as above.

> +}
> +
>  static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_querycap		= rvin_querycap,
>  	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
> @@ -683,6 +697,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_s_dv_timings		= rvin_s_dv_timings,
>  	.vidioc_query_dv_timings	= rvin_query_dv_timings,
>  
> +	.vidioc_g_edid			= rvin_g_edid,
> +	.vidioc_s_edid			= rvin_s_edid,
> +
>  	.vidioc_querystd		= rvin_querystd,
>  	.vidioc_g_std			= rvin_g_std,
>  	.vidioc_s_std			= rvin_s_std,
> -- 
> 2.7.4
>
Hans Verkuil July 7, 2016, 6:39 a.m. UTC | #3 
On 07/07/2016 02:16 AM, Niklas Söderlund wrote:
> Hi Ulrich,
> 
> Thanks for your patch.
> 
> On 2016-07-06 17:39:36 +0200, Ulrich Hecht wrote:
>> Adds G_EDID and S_EDID.
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> index 396eabc..bd8f14c 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -661,6 +661,20 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>>  	return ret;
>>  }
>>  
>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> +	struct rvin_dev *vin = video_drvdata(file);
>> +
>> +	return rvin_subdev_call(vin, pad, get_edid, edid);
> 
> You need to add a translation from the rcar-vin drivers view of it's 
> current input to the subdevices view of how it's pads are arranged. I 
> think something like this would work:
> 
>     struct rvin_dev *vin = video_drvdata(file);
>     unsigned int input;
>     int ret;
> 
>     input = edid->pad;
> 
>     edid->pad = vin->inputs[input].sink_idx;
> 
>     ret = vin_subdev_call(vin, pad, get_edid, edid);
> 
>     edid->pad = input;
> 
>     return ret;
> 
> I know it's not obvious you need this and I can't figure out a better 
> way to solve runtime switching of subdevices. Any ideas on how to 
> improve the situation are more then welcome :-)

I agree it is ugly, but it isn't used often enough to warrant the extra
work. I am thinking that the pad should be an extra argument in the subdev
op instead of using edid->pad. That should simplify the code.

Regards,

	Hans
diff mbox

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 396eabc..bd8f14c 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -661,6 +661,20 @@  static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
 	return ret;
 }
 
+static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct rvin_dev *vin = video_drvdata(file);
+
+	return rvin_subdev_call(vin, pad, get_edid, edid);
+}
+
+static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+	struct rvin_dev *vin = video_drvdata(file);
+
+	return rvin_subdev_call(vin, pad, set_edid, edid);
+}
+
 static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_querycap		= rvin_querycap,
 	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
@@ -683,6 +697,9 @@  static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_s_dv_timings		= rvin_s_dv_timings,
 	.vidioc_query_dv_timings	= rvin_query_dv_timings,
 
+	.vidioc_g_edid			= rvin_g_edid,
+	.vidioc_s_edid			= rvin_s_edid,
+
 	.vidioc_querystd		= rvin_querystd,
 	.vidioc_g_std			= rvin_g_std,
 	.vidioc_s_std			= rvin_s_std,