Re: [RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf
Thanks for reviewing Hans. My answer below - On 15/05/15 3:28 pm, Hans Verkuil hverk...@xs4all.nl wrote: Hi Prashant, Sorry for the very late review, I finally have time today to go through my pending patches. I have one question, see below: @@ -539,9 +555,11 @@ bool v4l2_detect_gtf(unsigned frame_height, /* Vertical */ v_fp = GTF_V_FP; - v_bp = (GTF_MIN_VSYNC_BP * hfreq + 50) / 100 - vsync; -image_height = (frame_height - v_fp - vsync - v_bp + 1) ~0x1; +if (interlaced) +image_height = (frame_height - 2 * v_fp - 2 * vsync - 2 * v_bp) ~0x1; +else +image_height = (frame_height - v_fp - vsync - v_bp + 1) ~0x1; if (image_height 0) return false; @@ -586,11 +604,20 @@ bool v4l2_detect_gtf(unsigned frame_height, fmt-bt.hsync = hsync; fmt-bt.vsync = vsync; fmt-bt.hbackporch = frame_width - image_width - h_fp - hsync; -fmt-bt.vbackporch = frame_height - image_height - v_fp - vsync; +fmt-bt.vbackporch = v_bp; Is this change correct? My main concern comes from the earlier image_height calculation in the chunk above. The image_height value is rounded to an even value, but if the value is actually changed due to rounding, then one of the v_fp, vsync or v_bp values must change by one as well. After all, the frame_height is fixed and image_height must be even. And frame_height can be even or odd, both are possible. So that one line of rounding difference must go somewhere in the blanking timings. Thanks for spotting this. Yes, I agree that it does not look correct. If the image_height gets rounded to next even value, above calculation will result in one extra line. For interlaced case, I am wondering where to absorb the rounding difference ? I suppose, we cannot absorb this difference in vsync. Should it be v_bp of even field or odd field ? Not sure, but probably, the bottom (even) field ? Regards, Prashant -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf
Hi Prashant,
Sorry for the very late review, I finally have time today to go through
my pending patches.
I have one question, see below:
On 04/23/2015 10:59 AM, Prashant Laddha wrote:
Extended detect_cvt/gtf API to indicate the format type (interlaced
or progressive). In case of interlaced, the vertical front and back
porch and vsync values for both (odd,even) fields are considered to
derive image height. Populated vsync, verical front, back porch
values in bt timing structure for even and odd fields and updated
the flags appropriately.
Also modified the functions calling the detect_cvt/gtf(). As of now
these functions are calling detect_cvt/gtf() with interlaced flag
set to false.
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Martin Bugge marbu...@cisco.com
Cc: Mats Randgaard matra...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Prashant Laddha prlad...@cisco.com
---
drivers/media/i2c/adv7604.c | 4 +--
drivers/media/i2c/adv7842.c | 4 +--
drivers/media/platform/vivid/vivid-vid-cap.c | 5 ++--
drivers/media/v4l2-core/v4l2-dv-timings.c| 39
+++-
include/media/v4l2-dv-timings.h | 6 +++--
5 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 60ffcf0..74abfd4 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1304,12 +1304,12 @@ static int stdi2dv_timings(struct v4l2_subdev *sd,
if (v4l2_detect_cvt(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- timings))
+ false, timings))
return 0;
if (v4l2_detect_gtf(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- state-aspect_ratio, timings))
+ false, state-aspect_ratio, timings))
return 0;
v4l2_dbg(2, debug, sd,
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index b5a37fe..90cbead 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -1333,12 +1333,12 @@ static int stdi2dv_timings(struct v4l2_subdev *sd,
if (v4l2_detect_cvt(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- timings))
+ false, timings))
return 0;
if (v4l2_detect_gtf(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- state-aspect_ratio, timings))
+ false, state-aspect_ratio, timings))
return 0;
v4l2_dbg(2, debug, sd,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
b/drivers/media/platform/vivid/vivid-vid-cap.c
index be10b72..a3b19dc 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -1623,7 +1623,7 @@ static bool valid_cvt_gtf_timings(struct
v4l2_dv_timings *timings)
if (bt-standards == 0 || (bt-standards V4L2_DV_BT_STD_CVT)) {
if (v4l2_detect_cvt(total_v_lines, h_freq, bt-vsync,
- bt-polarities, timings))
+ bt-polarities, false, timings))
return true;
}
@@ -1634,7 +1634,8 @@ static bool valid_cvt_gtf_timings(struct
v4l2_dv_timings *timings)
aspect_ratio.numerator,
aspect_ratio.denominator);
if (v4l2_detect_gtf(total_v_lines, h_freq, bt-vsync,
- bt-polarities, aspect_ratio, timings))
+ bt-polarities, false,
+ aspect_ratio, timings))
return true;
}
return false;
diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c
b/drivers/media/v4l2-core/v4l2-dv-timings.c
index 37f0d6f..86e11d1 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -338,6 +338,7 @@ EXPORT_SYMBOL_GPL(v4l2_print_dv_timings);
* @vsync - the height of the vertical sync in lines.
* @polarities - the horizontal and vertical polarities (same as struct
* v4l2_bt_timings polarities).
+ * @interlaced - if this flag is true, it indicates interlaced format
* @fmt - the resulting timings.
*
* This function will attempt to detect if the given values
[RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf
Extended detect_cvt/gtf API to indicate the format type (interlaced
or progressive). In case of interlaced, the vertical front and back
porch and vsync values for both (odd,even) fields are considered to
derive image height. Populated vsync, verical front, back porch
values in bt timing structure for even and odd fields and updated
the flags appropriately.
Also modified the functions calling the detect_cvt/gtf(). As of now
these functions are calling detect_cvt/gtf() with interlaced flag
set to false.
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Martin Bugge marbu...@cisco.com
Cc: Mats Randgaard matra...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Signed-off-by: Prashant Laddha prlad...@cisco.com
---
drivers/media/i2c/adv7604.c | 4 +--
drivers/media/i2c/adv7842.c | 4 +--
drivers/media/platform/vivid/vivid-vid-cap.c | 5 ++--
drivers/media/v4l2-core/v4l2-dv-timings.c| 39 +++-
include/media/v4l2-dv-timings.h | 6 +++--
5 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 60ffcf0..74abfd4 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1304,12 +1304,12 @@ static int stdi2dv_timings(struct v4l2_subdev *sd,
if (v4l2_detect_cvt(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- timings))
+ false, timings))
return 0;
if (v4l2_detect_gtf(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- state-aspect_ratio, timings))
+ false, state-aspect_ratio, timings))
return 0;
v4l2_dbg(2, debug, sd,
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index b5a37fe..90cbead 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -1333,12 +1333,12 @@ static int stdi2dv_timings(struct v4l2_subdev *sd,
if (v4l2_detect_cvt(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- timings))
+ false, timings))
return 0;
if (v4l2_detect_gtf(stdi-lcf + 1, hfreq, stdi-lcvs,
(stdi-hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) |
(stdi-vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0),
- state-aspect_ratio, timings))
+ false, state-aspect_ratio, timings))
return 0;
v4l2_dbg(2, debug, sd,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c
b/drivers/media/platform/vivid/vivid-vid-cap.c
index be10b72..a3b19dc 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -1623,7 +1623,7 @@ static bool valid_cvt_gtf_timings(struct v4l2_dv_timings
*timings)
if (bt-standards == 0 || (bt-standards V4L2_DV_BT_STD_CVT)) {
if (v4l2_detect_cvt(total_v_lines, h_freq, bt-vsync,
- bt-polarities, timings))
+ bt-polarities, false, timings))
return true;
}
@@ -1634,7 +1634,8 @@ static bool valid_cvt_gtf_timings(struct v4l2_dv_timings
*timings)
aspect_ratio.numerator,
aspect_ratio.denominator);
if (v4l2_detect_gtf(total_v_lines, h_freq, bt-vsync,
- bt-polarities, aspect_ratio, timings))
+ bt-polarities, false,
+ aspect_ratio, timings))
return true;
}
return false;
diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c
b/drivers/media/v4l2-core/v4l2-dv-timings.c
index 37f0d6f..86e11d1 100644
--- a/drivers/media/v4l2-core/v4l2-dv-timings.c
+++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
@@ -338,6 +338,7 @@ EXPORT_SYMBOL_GPL(v4l2_print_dv_timings);
* @vsync - the height of the vertical sync in lines.
* @polarities - the horizontal and vertical polarities (same as struct
* v4l2_bt_timings polarities).
+ * @interlaced - if this flag is true, it indicates interlaced format
* @fmt - the resulting timings.
*
* This function will attempt to detect if the given values correspond to a
@@ -349,7 +350,7 @@ EXPORT_SYMBOL_GPL(v4l2_print_dv_timings);
* detection function.
*/
bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
- u32
