Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605536 > unrolled thread

[PATCH 0/3] Handling of reduced FPS in V4L2

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Handling of reduced FPS in V4L2 Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-21 12:50 +0100
    [PATCH 3/3] [media] cobalt: Use v4l2_calc_timeperframe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-21 13:00 +0100
    [PATCH 1/3] [media] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-21 13:00 +0100
    [PATCH 2/3] [media] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-21 13:00 +0100
    Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-24 12:20 +0100
      Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Hans Verkuil <hansverk@cisco.com> - 2017-03-24 12:30 +0100
        Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-24 13:00 +0100
          Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Hans Verkuil <hansverk@cisco.com> - 2017-03-24 13:20 +0100
            Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-24 13:30 +0100
              Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Hans Verkuil <hansverk@cisco.com> - 2017-03-24 13:30 +0100
                Re: [PATCH 0/3] Handling of reduced FPS in V4L2 Jose Abreu <Jose.Abreu@synopsys.com> - 2017-03-27 14:10 +0200

#1605536 — [PATCH 0/3] Handling of reduced FPS in V4L2

Hi all,

This is a follow up patch from this discussion [1]. It should be
seen more as a starting point to introduce better handling of
time per frame in v4l2. Quoting Hans Verkuil from [1]:

1) "Add a flag V4L2_DV_FL_CAN_DETECT_REDUCED_FPS. If set,
then the hw can detect the difference between regular fps
and 1000/1001 fps. Note: this is only valid for timings of
VIC codes with the V4L2_DV_FL_CAN_REDUCE_FPS flag set."

2) "Allow V4L2_DV_FL_REDUCED_FPS to be used for receivers
if V4L2_DV_FL_CAN_DETECT_REDUCED_FPS is set."

3) "For standard VIC codes the pixelclock returned by
query_dv_timings is that of the corresponding VIC timing,
not what is measured. This will ensure fixed fps values"

4) "g_parm should calculate the fps based on the v4l2_bt_timings
struct, looking at the REDUCES_FPS flags. For those receivers that
cannot detect the difference, the fps will be 24/30/60 Hz, for
those that can detect the difference g_parm can check if both
V4L2_DV_FL_CAN_DETECT_REDUCED_FPS and V4L2_DV_FL_REDUCED_FPS are
set and reduce the fps by 1000/1001."

-----------
In terms of implementation:
	- Point 1) is done in patch 1/3
	- Point 2) and 3) should be done by a HDMI Receiver driver
	(I think?).
	- Point 4) is done in patch 2/3.
	- The patch 3/3 is a simple implementation (which was not
	tested) in the cobalt driver
-----------
	
[1] https://patchwork.kernel.org/patch/9609441/

Best regards,
Jose Miguel Abreu

Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Jose Abreu (3):
  [media] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS
  [media] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper
  [media] cobalt: Use v4l2_calc_timeperframe helper

 drivers/media/pci/cobalt/cobalt-v4l2.c    |  9 +++++--
 drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++++++++++++++++++++++++++++++
 include/media/v4l2-dv-timings.h           | 11 +++++++++
 include/uapi/linux/videodev2.h            |  7 ++++++
 4 files changed, 64 insertions(+), 2 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1605541 — [PATCH 3/3] [media] cobalt: Use v4l2_calc_timeperframe helper

Currently, cobalt driver always returns 60fps in g_parm.
This patch uses the new v4l2_calc_timeperframe helper to
calculate the time per frame value.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/media/pci/cobalt/cobalt-v4l2.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index def4a3b..25532ae 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -1076,10 +1076,15 @@ static int cobalt_subscribe_event(struct v4l2_fh *fh,
 
 static int cobalt_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 {
+	struct cobalt_stream *s = video_drvdata(file);
+	struct v4l2_fract fps;
+
 	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	a->parm.capture.timeperframe.numerator = 1;
-	a->parm.capture.timeperframe.denominator = 60;
+
+	fps = v4l2_calc_timeperframe(&s->timings);
+	a->parm.capture.timeperframe.numerator = fps.numerator;
+	a->parm.capture.timeperframe.denominator = fps.denominator;
 	a->parm.capture.readbuffers = 3;
 	return 0;
 }
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1605542 — [PATCH 1/3] [media] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS

Add a new flag to UAPI for DV timings which, whenever set,
indicates that hardware can detect the difference between
regular FPS and 1000/1001 FPS.

This is specific to HDMI receivers. Also, it is only valid
when V4L2_DV_FL_CAN_REDUCE_FPS is set.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 include/uapi/linux/videodev2.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45184a2..dd7b426 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1371,6 +1371,13 @@ struct v4l2_bt_timings {
  * InfoFrame).
  */
 #define V4L2_DV_FL_HAS_HDMI_VIC			(1 << 8)
+/*
+ * CEA-861 specific: only valid for video receivers.
+ * If set, then HW can detect the difference between regular FPS and
+ * 1000/1001 FPS. Note: This flag is only valid for HDMI VIC codes with
+ * the V4L2_DV_FL_CAN_REDUCE_FPS flag set.
+ */
+#define V4L2_DV_FL_CAN_DETECT_REDUCED_FPS	(1 << 9)
 
 /* A few useful defines to calculate the total blanking and frame sizes */
 #define V4L2_DV_BT_BLANKING_WIDTH(bt) \
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1605544 — [PATCH 2/3] [media] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper

A new helper function was introduced to facilitate the calculation
of time per frame value whenever we have access to the full
v4l2_dv_timings structure.

This should be used only for receivers and only when there is
enough accuracy in the measured pixel clock value as well as in
the horizontal/vertical values.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Carlos Palminha <palminha@synopsys.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++++++++++++++++++++++++++++++
 include/media/v4l2-dv-timings.h           | 11 +++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c
index 5c8c49d..b7b39c2 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -814,3 +814,42 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 hor_landscape, u8 vert_portrait)
 	return aspect;
 }
 EXPORT_SYMBOL_GPL(v4l2_calc_aspect_ratio);
+
+/** v4l2_calc_timeperframe - helper function to calculate timeperframe based
+ *	v4l2_dv_timings fields.
+ * @t - Timings for the video mode.
+ *
+ * Calculates the expected timeperframe using the pixel clock value and
+ * horizontal/vertical measures. This means that v4l2_dv_timings structure
+ * must be correctly and fully filled.
+ */
+struct v4l2_fract v4l2_calc_timeperframe(const struct v4l2_dv_timings *t)
+{
+	const struct v4l2_bt_timings *bt = &t->bt;
+	struct v4l2_fract fps_fract = { 1, 1 };
+	unsigned long n, d;
+	u32 htot, vtot, fps;
+	u64 pclk;
+
+	if (t->type != V4L2_DV_BT_656_1120)
+		return fps_fract;
+
+	htot = V4L2_DV_BT_FRAME_WIDTH(bt);
+	vtot = V4L2_DV_BT_FRAME_HEIGHT(bt);
+	pclk = bt->pixelclock;
+
+	if ((bt->flags & V4L2_DV_FL_CAN_DETECT_REDUCED_FPS) &&
+	    (bt->flags & V4L2_DV_FL_REDUCED_FPS))
+		pclk = div_u64(pclk * 1000ULL, 1001);
+
+	fps = (htot * vtot) > 0 ? div_u64((100 * pclk), (htot * vtot)) : 0;
+	if (!fps)
+		return fps_fract;
+
+	rational_best_approximation(fps, 100, fps, 100, &n, &d);
+
+	fps_fract.numerator = d;
+	fps_fract.denominator = n;
+	return fps_fract;
+}
+EXPORT_SYMBOL_GPL(v4l2_calc_timeperframe);
diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
index 61a1889..c1eef08 100644
--- a/include/media/v4l2-dv-timings.h
+++ b/include/media/v4l2-dv-timings.h
@@ -203,6 +203,17 @@ bool v4l2_detect_gtf(unsigned frame_height, unsigned hfreq, unsigned vsync,
  */
 struct v4l2_fract v4l2_dv_timings_aspect_ratio(const struct v4l2_dv_timings *t);
 
+/**
+ * v4l2_calc_timeperframe - helper function to calculate timeperframe based
+ *	v4l2_dv_timings fields.
+ * @t - Timings for the video mode.
+ *
+ * Calculates the expected timeperframe using the pixel clock value and
+ * horizontal/vertical measures. This means that v4l2_dv_timings structure
+ * must be correctly and fully filled.
+ */
+struct v4l2_fract v4l2_calc_timeperframe(const struct v4l2_dv_timings *t);
+
 /*
  * reduce_fps - check if conditions for reduced fps are true.
  * bt - v4l2 timing structure
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1608335

Hi Hans,


On 21-03-2017 11:49, Jose Abreu wrote:
> Hi all,
>
> This is a follow up patch from this discussion [1]. It should be
> seen more as a starting point to introduce better handling of
> time per frame in v4l2. Quoting Hans Verkuil from [1]:
>
> 1) "Add a flag V4L2_DV_FL_CAN_DETECT_REDUCED_FPS. If set,
> then the hw can detect the difference between regular fps
> and 1000/1001 fps. Note: this is only valid for timings of
> VIC codes with the V4L2_DV_FL_CAN_REDUCE_FPS flag set."
>
> 2) "Allow V4L2_DV_FL_REDUCED_FPS to be used for receivers
> if V4L2_DV_FL_CAN_DETECT_REDUCED_FPS is set."
>
> 3) "For standard VIC codes the pixelclock returned by
> query_dv_timings is that of the corresponding VIC timing,
> not what is measured. This will ensure fixed fps values"
>
> 4) "g_parm should calculate the fps based on the v4l2_bt_timings
> struct, looking at the REDUCES_FPS flags. For those receivers that
> cannot detect the difference, the fps will be 24/30/60 Hz, for
> those that can detect the difference g_parm can check if both
> V4L2_DV_FL_CAN_DETECT_REDUCED_FPS and V4L2_DV_FL_REDUCED_FPS are
> set and reduce the fps by 1000/1001."
>
> -----------
> In terms of implementation:
> 	- Point 1) is done in patch 1/3
> 	- Point 2) and 3) should be done by a HDMI Receiver driver
> 	(I think?).
> 	- Point 4) is done in patch 2/3.
> 	- The patch 3/3 is a simple implementation (which was not
> 	tested) in the cobalt driver
> -----------
> 	
> [1] https://patchwork.kernel.org/patch/9609441/
>
> Best regards,
> Jose Miguel Abreu
>
> Cc: Carlos Palminha <palminha@synopsys.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> Jose Abreu (3):
>   [media] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS
>   [media] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper
>   [media] cobalt: Use v4l2_calc_timeperframe helper
>
>  drivers/media/pci/cobalt/cobalt-v4l2.c    |  9 +++++--
>  drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++++++++++++++++++++++++++++++
>  include/media/v4l2-dv-timings.h           | 11 +++++++++
>  include/uapi/linux/videodev2.h            |  7 ++++++
>  4 files changed, 64 insertions(+), 2 deletions(-)
>

Can you please review this series, when possible? And if you
could test it on cobalt it would be great :)

Best regards,
Jose Miguel Abreu

[toc] | [prev] | [next] | [standalone]

#1608339

On 03/24/17 12:03, Jose Abreu wrote:
> Hi Hans,
> 
> 
> On 21-03-2017 11:49, Jose Abreu wrote:
>> Hi all,
>>
>> This is a follow up patch from this discussion [1]. It should be
>> seen more as a starting point to introduce better handling of
>> time per frame in v4l2. Quoting Hans Verkuil from [1]:
>>
>> 1) "Add a flag V4L2_DV_FL_CAN_DETECT_REDUCED_FPS. If set,
>> then the hw can detect the difference between regular fps
>> and 1000/1001 fps. Note: this is only valid for timings of
>> VIC codes with the V4L2_DV_FL_CAN_REDUCE_FPS flag set."
>>
>> 2) "Allow V4L2_DV_FL_REDUCED_FPS to be used for receivers
>> if V4L2_DV_FL_CAN_DETECT_REDUCED_FPS is set."
>>
>> 3) "For standard VIC codes the pixelclock returned by
>> query_dv_timings is that of the corresponding VIC timing,
>> not what is measured. This will ensure fixed fps values"
>>
>> 4) "g_parm should calculate the fps based on the v4l2_bt_timings
>> struct, looking at the REDUCES_FPS flags. For those receivers that
>> cannot detect the difference, the fps will be 24/30/60 Hz, for
>> those that can detect the difference g_parm can check if both
>> V4L2_DV_FL_CAN_DETECT_REDUCED_FPS and V4L2_DV_FL_REDUCED_FPS are
>> set and reduce the fps by 1000/1001."
>>
>> -----------
>> In terms of implementation:
>> 	- Point 1) is done in patch 1/3
>> 	- Point 2) and 3) should be done by a HDMI Receiver driver
>> 	(I think?).
>> 	- Point 4) is done in patch 2/3.
>> 	- The patch 3/3 is a simple implementation (which was not
>> 	tested) in the cobalt driver
>> -----------
>> 	
>> [1] https://patchwork.kernel.org/patch/9609441/
>>
>> Best regards,
>> Jose Miguel Abreu
>>
>> Cc: Carlos Palminha <palminha@synopsys.com>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: linux-media@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Jose Abreu (3):
>>   [media] videodev2.h: Add new DV flag CAN_DETECT_REDUCED_FPS
>>   [media] v4l2-dv-timings: Introduce v4l2_calc_timeperframe helper
>>   [media] cobalt: Use v4l2_calc_timeperframe helper
>>
>>  drivers/media/pci/cobalt/cobalt-v4l2.c    |  9 +++++--
>>  drivers/media/v4l2-core/v4l2-dv-timings.c | 39 +++++++++++++++++++++++++++++++
>>  include/media/v4l2-dv-timings.h           | 11 +++++++++
>>  include/uapi/linux/videodev2.h            |  7 ++++++
>>  4 files changed, 64 insertions(+), 2 deletions(-)
>>
> 
> Can you please review this series, when possible? And if you
> could test it on cobalt it would be great :)

Hopefully next week. Did you have some real-world numbers w.r.t. measured
pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?

I do want to see that, since this patch series only makes sense if you can
actually make use of it to reliably detect the difference.

I will try to test that myself with cobalt, but almost certainly I won't
be able to tell the difference; if memory serves it can't detect the freq
with high enough precision.

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]

#1608357

Hi Hans,


>> Can you please review this series, when possible? And if you
>> could test it on cobalt it would be great :)
> Hopefully next week. 

Thanks :)

> Did you have some real-world numbers w.r.t. measured
> pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?

I did make some measurements but I'm afraid I didn't yet test
with many sources (I mostly tested with signal generators which
should have a higher precision clock than real sources). I have a
bunch of players here, I will test them as soon as I can.
Regarding precision: for our controller is theoretically and
effectively enough: The worst case is for 640x480, and even in
that case the difference between 60Hz and 59.94Hz is > 1 unit of
the measuring register. This still doesn't solve the problem of
having a bad source with a bad clock, but I don't know if we can
do much more about that.

>
> I do want to see that, since this patch series only makes sense if you can
> actually make use of it to reliably detect the difference.
>
> I will try to test that myself with cobalt, but almost certainly I won't
> be able to tell the difference; if memory serves it can't detect the freq
> with high enough precision.

Ok, thanks, this would be great because I didn't test the series
exactly "as is" because I'm using 4.10. I did look at vivid
driver but it already handles reduced frame rate, so it kind of
does what it is proposed in this series. If this helper is
integrated in the v4l2 core then I can send the patch to vivid.

Best regards,
Jose Miguel Abreu

>
> Regards,
>
> 	Hans

[toc] | [prev] | [next] | [standalone]

#1608369

On 03/24/17 12:52, Jose Abreu wrote:
> Hi Hans,
> 
> 
>>> Can you please review this series, when possible? And if you
>>> could test it on cobalt it would be great :)
>> Hopefully next week. 
> 
> Thanks :)
> 
>> Did you have some real-world numbers w.r.t. measured
>> pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?
> 
> I did make some measurements but I'm afraid I didn't yet test
> with many sources (I mostly tested with signal generators which
> should have a higher precision clock than real sources). I have a
> bunch of players here, I will test them as soon as I can.
> Regarding precision: for our controller is theoretically and
> effectively enough: The worst case is for 640x480, and even in
> that case the difference between 60Hz and 59.94Hz is > 1 unit of
> the measuring register. This still doesn't solve the problem of
> having a bad source with a bad clock, but I don't know if we can
> do much more about that.

I would really like to see a table with different sources sending
these different framerates and the value that your HW detects.

If there is an obvious and clear difference, then this feature makes
sense. If it is all over the place, then I need to think about this
some more.

To be honest, I expect that you will see 'an obvious and clear'
difference, but that is no more than a gut feeling at the moment and
I would like to see some proper test results.

> 
>>
>> I do want to see that, since this patch series only makes sense if you can
>> actually make use of it to reliably detect the difference.
>>
>> I will try to test that myself with cobalt, but almost certainly I won't
>> be able to tell the difference; if memory serves it can't detect the freq
>> with high enough precision.
> 
> Ok, thanks, this would be great because I didn't test the series
> exactly "as is" because I'm using 4.10. I did look at vivid
> driver but it already handles reduced frame rate, so it kind of
> does what it is proposed in this series. If this helper is
> integrated in the v4l2 core then I can send the patch to vivid.

That would be nice to have in vivid.

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]

#1608374

Hi Hans,


On 24-03-2017 12:12, Hans Verkuil wrote:
> On 03/24/17 12:52, Jose Abreu wrote:
>> Hi Hans,
>>
>>
>>>> Can you please review this series, when possible? And if you
>>>> could test it on cobalt it would be great :)
>>> Hopefully next week. 
>> Thanks :)
>>
>>> Did you have some real-world numbers w.r.t. measured
>>> pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?
>> I did make some measurements but I'm afraid I didn't yet test
>> with many sources (I mostly tested with signal generators which
>> should have a higher precision clock than real sources). I have a
>> bunch of players here, I will test them as soon as I can.
>> Regarding precision: for our controller is theoretically and
>> effectively enough: The worst case is for 640x480, and even in
>> that case the difference between 60Hz and 59.94Hz is > 1 unit of
>> the measuring register. This still doesn't solve the problem of
>> having a bad source with a bad clock, but I don't know if we can
>> do much more about that.
> I would really like to see a table with different sources sending
> these different framerates and the value that your HW detects.
>
> If there is an obvious and clear difference, then this feature makes
> sense. If it is all over the place, then I need to think about this
> some more.
>
> To be honest, I expect that you will see 'an obvious and clear'
> difference, but that is no more than a gut feeling at the moment and
> I would like to see some proper test results.

Ok, I will make a table. The test procedure will be like this:
    - Measure pixel clock value using certified HDMI analyzer
    - Measure pixel clock using our controller
    - Compare the values obtained from analyzer, controller and
the values that the source is telling to send (the value
displayed in source menu for example [though, some of them may
not discriminate the exact frame rate, thats why analyzer should
be used also]).

Seems ok? I will need some time, something like a week because my
setup was "borrowed".

Best regards,
Jose Miguel Abreu

>
>>> I do want to see that, since this patch series only makes sense if you can
>>> actually make use of it to reliably detect the difference.
>>>
>>> I will try to test that myself with cobalt, but almost certainly I won't
>>> be able to tell the difference; if memory serves it can't detect the freq
>>> with high enough precision.
>> Ok, thanks, this would be great because I didn't test the series
>> exactly "as is" because I'm using 4.10. I did look at vivid
>> driver but it already handles reduced frame rate, so it kind of
>> does what it is proposed in this series. If this helper is
>> integrated in the v4l2 core then I can send the patch to vivid.
> That would be nice to have in vivid.
>
> Regards,
>
> 	Hans
>

[toc] | [prev] | [next] | [standalone]

#1608376

On 03/24/17 13:21, Jose Abreu wrote:
> Hi Hans,
> 
> 
> On 24-03-2017 12:12, Hans Verkuil wrote:
>> On 03/24/17 12:52, Jose Abreu wrote:
>>> Hi Hans,
>>>
>>>
>>>>> Can you please review this series, when possible? And if you
>>>>> could test it on cobalt it would be great :)
>>>> Hopefully next week. 
>>> Thanks :)
>>>
>>>> Did you have some real-world numbers w.r.t. measured
>>>> pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?
>>> I did make some measurements but I'm afraid I didn't yet test
>>> with many sources (I mostly tested with signal generators which
>>> should have a higher precision clock than real sources). I have a
>>> bunch of players here, I will test them as soon as I can.
>>> Regarding precision: for our controller is theoretically and
>>> effectively enough: The worst case is for 640x480, and even in
>>> that case the difference between 60Hz and 59.94Hz is > 1 unit of
>>> the measuring register. This still doesn't solve the problem of
>>> having a bad source with a bad clock, but I don't know if we can
>>> do much more about that.
>> I would really like to see a table with different sources sending
>> these different framerates and the value that your HW detects.
>>
>> If there is an obvious and clear difference, then this feature makes
>> sense. If it is all over the place, then I need to think about this
>> some more.
>>
>> To be honest, I expect that you will see 'an obvious and clear'
>> difference, but that is no more than a gut feeling at the moment and
>> I would like to see some proper test results.
> 
> Ok, I will make a table. The test procedure will be like this:
>     - Measure pixel clock value using certified HDMI analyzer
>     - Measure pixel clock using our controller
>     - Compare the values obtained from analyzer, controller and
> the values that the source is telling to send (the value
> displayed in source menu for example [though, some of them may
> not discriminate the exact frame rate, thats why analyzer should
> be used also]).
> 
> Seems ok? I will need some time, something like a week because my
> setup was "borrowed".

That sounds good. Sorry for adding to your workload, but there is no
point to have a flag that in practice is meaningless.

I'm actually very curious about the results!

Regards,

	Hans

> 
> Best regards,
> Jose Miguel Abreu
> 
>>
>>>> I do want to see that, since this patch series only makes sense if you can
>>>> actually make use of it to reliably detect the difference.
>>>>
>>>> I will try to test that myself with cobalt, but almost certainly I won't
>>>> be able to tell the difference; if memory serves it can't detect the freq
>>>> with high enough precision.
>>> Ok, thanks, this would be great because I didn't test the series
>>> exactly "as is" because I'm using 4.10. I did look at vivid
>>> driver but it already handles reduced frame rate, so it kind of
>>> does what it is proposed in this series. If this helper is
>>> integrated in the v4l2 core then I can send the patch to vivid.
>> That would be nice to have in vivid.
>>
>> Regards,
>>
>> 	Hans
>>
> 

[toc] | [prev] | [next] | [standalone]

#1609760

Hi Hans,


On 24-03-2017 12:28, Hans Verkuil wrote:
> On 03/24/17 13:21, Jose Abreu wrote:
>> Hi Hans,
>>
>>
>> On 24-03-2017 12:12, Hans Verkuil wrote:
>>> On 03/24/17 12:52, Jose Abreu wrote:
>>>> Hi Hans,
>>>>
>>>>
>>>>>> Can you please review this series, when possible? And if you
>>>>>> could test it on cobalt it would be great :)
>>>>> Hopefully next week. 
>>>> Thanks :)
>>>>
>>>>> Did you have some real-world numbers w.r.t. measured
>>>>> pixelclock frequencies and 60 vs 59.94 Hz and 24 vs 23.976 Hz?
>>>> I did make some measurements but I'm afraid I didn't yet test
>>>> with many sources (I mostly tested with signal generators which
>>>> should have a higher precision clock than real sources). I have a
>>>> bunch of players here, I will test them as soon as I can.
>>>> Regarding precision: for our controller is theoretically and
>>>> effectively enough: The worst case is for 640x480, and even in
>>>> that case the difference between 60Hz and 59.94Hz is > 1 unit of
>>>> the measuring register. This still doesn't solve the problem of
>>>> having a bad source with a bad clock, but I don't know if we can
>>>> do much more about that.
>>> I would really like to see a table with different sources sending
>>> these different framerates and the value that your HW detects.
>>>
>>> If there is an obvious and clear difference, then this feature makes
>>> sense. If it is all over the place, then I need to think about this
>>> some more.
>>>
>>> To be honest, I expect that you will see 'an obvious and clear'
>>> difference, but that is no more than a gut feeling at the moment and
>>> I would like to see some proper test results.
>> Ok, I will make a table. The test procedure will be like this:
>>     - Measure pixel clock value using certified HDMI analyzer
>>     - Measure pixel clock using our controller
>>     - Compare the values obtained from analyzer, controller and
>> the values that the source is telling to send (the value
>> displayed in source menu for example [though, some of them may
>> not discriminate the exact frame rate, thats why analyzer should
>> be used also]).
>>
>> Seems ok? I will need some time, something like a week because my
>> setup was "borrowed".
> That sounds good. Sorry for adding to your workload, but there is no
> point to have a flag that in practice is meaningless.
>
> I'm actually very curious about the results!

I managed to do the tests but unfortunately I can't publish the
full results (at least until I get approval).

I can say that the results look good. As you expected we have
some sources with a bad clock but this is correctly detected by
the controller (and also by the HDMI analyzer).

Using the v4l2_calc_framerate function I managed to get this:

| Source       | Resolution                  | v4l2_calc_framerate()
------------------------------------------------------------------------------
| Analyzer 1 | 640x480@59.94         | 59.92
| Analyzer 1 | 640x480@60              | 60
| Analyzer 1 | 1920x1080@60          | 60
| Player 1     | 1920x1080@59.94     | 59.94
| Player 2     | 1920x1080@59.94     | 59.93
| Player 3     | 3840x2160@59.94     | 59.94
| Player 4     | 1920x1080@59.94     | 59.94
| Player 5     | 1920x1080@59.94     | 59.93
| Player 6     | 1280x720@50            | 50
| Player 7     | 1920x1080@59.94     | 59.93
| Player 8     | 1920x1080@60          | 60
| Analyzer 2 | 720x480@59.94         | 59.94
| Analyzer 2 | 720x480@60              | 60
| Analyzer 2 | 1920x1080@59.94     | 59.93
| Analyzer 2 | 1920x180@60            | 60
| Analyzer 2 | 3840x2160@23.98     | 23.97
| Analyzer 2 | 3840x2160@24          | 24
| Analyzer 2 | 3840x2160@29.97     | 29.96
| Analyzer 2 | 3840x2160@30          | 30
| Analyzer 2 | 3840x2160@59.94     | 59.93
| Analyzer 2 | 3840x2160@60          | 60
------------------------------------------------------------------------------

What do you think? Shall we continue integrating this new patch
or drop it?

Best regards,
Jose Miguel Abreu

>
> Regards,
>
> 	Hans
>
>> Best regards,
>> Jose Miguel Abreu
>>
>>>>> I do want to see that, since this patch series only makes sense if you can
>>>>> actually make use of it to reliably detect the difference.
>>>>>
>>>>> I will try to test that myself with cobalt, but almost certainly I won't
>>>>> be able to tell the difference; if memory serves it can't detect the freq
>>>>> with high enough precision.
>>>> Ok, thanks, this would be great because I didn't test the series
>>>> exactly "as is" because I'm using 4.10. I did look at vivid
>>>> driver but it already handles reduced frame rate, so it kind of
>>>> does what it is proposed in this series. If this helper is
>>>> integrated in the v4l2 core then I can send the patch to vivid.
>>> That would be nice to have in vivid.
>>>
>>> Regards,
>>>
>>> 	Hans
>>>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web