[RFC PATCH 0/4] Support for interlaced in cvt/gtf timings

[RFC PATCH 0/4] Support for interlaced in cvt/gtf timings

[Prashant Laddha]

Currently, detect_cvt/gtf() functions in v4l2-dv-timings do not calculate
timings for interlaced format. Besides the set-dv-bt-timings control in
vivid, drivers for adv7604 and adv7842 may also need cvt/gtf timings for
interlaced format. This patch series is a proposal for adding interlaced
support in cvt/gtf timing calculation. Previously, I had submitted a patch
for the same. https://patchwork.linuxtv.org/patch/29130/  Hans had reviewed
and asked to rework and resubmit the patches as RFC.

Patch 1/4 extends the detect_cvt/gtf() API to pass on an interlaced flag
(this flag, if true, indicates interlaced format type). This patch adds the
timings calculation for interlaced format. Since detect_cvt/gtf() API is
changed, all caller functions also modified in the same patch. To begin with,
interlaced flag is passed as false.

Subsequent patches 2/4 to 4/4, contain changes in caller functions to pass the
appropriate values for interlaced flag.

Please review this patch series and share your comments, suggestions.

Regards,
Prashant


Prashant Laddha (4):
  v4l2-dv-timings: Add interlace support in detect cvt/gtf
  vivid: Use interlaced info for cvt/gtf timing detection
  adv7604: Use interlaced info for cvt/gtf timing detection
  adv7842: Use interlaced info for cvt/gtf timing detection

 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(-)

-- 
1.9.1

[RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf

[Prashant Laddha]

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.verkuil@cisco.com>
Cc: Martin Bugge <marbugge@cisco.com>
Cc: Mats Randgaard <matrandg@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Prashant Laddha <prladdha@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 polarities, struct v4l2_dv_timings *fmt)
+		u32 polarities, bool interlaced, struct v4l2_dv_timings *fmt)
 {
 	int  v_fp, v_bp, h_fp, h_bp, hsync;
 	int  frame_width, image_height, image_width;
@@ -384,7 +385,11 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
 		if (v_bp < CVT_MIN_V_BPORCH)
 			v_bp = CVT_MIN_V_BPORCH;
 	}
-	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;
@@ -457,11 +462,20 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
 	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;
 	fmt->bt.pixelclock = pix_clk;
 	fmt->bt.standards = V4L2_DV_BT_STD_CVT;
 	if (reduced_blanking)
 		fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING;
+	if (interlaced) {
+		fmt->bt.interlaced = V4L2_DV_INTERLACED;
+		fmt->bt.il_vfrontporch = v_fp;
+		fmt->bt.il_vsync = vsync;
+		fmt->bt.il_vbackporch = v_bp + 1;
+		fmt->bt.flags |= V4L2_DV_FL_HALF_LINE;
+	} else {
+		fmt->bt.interlaced = V4L2_DV_PROGRESSIVE;
+	}
 	return true;
 }
 EXPORT_SYMBOL_GPL(v4l2_detect_cvt);
@@ -500,6 +514,7 @@ EXPORT_SYMBOL_GPL(v4l2_detect_cvt);
  * @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
  * @aspect - preferred aspect ratio. GTF has no method of determining the
  *		aspect ratio in order to derive the image width from the
  *		image height, so it has to be passed explicitly. Usually
@@ -515,6 +530,7 @@ bool v4l2_detect_gtf(unsigned frame_height,
 		unsigned hfreq,
 		unsigned vsync,
 		u32 polarities,
+		bool interlaced,
 		struct v4l2_fract aspect,
 		struct v4l2_dv_timings *fmt)
 {
@@ -539,9 +555,11 @@ bool v4l2_detect_gtf(unsigned frame_height,
 
 	/* Vertical */
 	v_fp = GTF_V_FP;
-
 	v_bp = (GTF_MIN_VSYNC_BP * hfreq + 500000) / 1000000 - 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;
 	fmt->bt.pixelclock = pix_clk;
 	fmt->bt.standards = V4L2_DV_BT_STD_GTF;
 	if (!default_gtf)
 		fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING;
+	if (interlaced) {
+		fmt->bt.interlaced = V4L2_DV_INTERLACED;
+		fmt->bt.il_vfrontporch = v_fp;
+		fmt->bt.il_vsync = vsync;
+		fmt->bt.il_vbackporch = v_bp + 1;
+		fmt->bt.flags |= V4L2_DV_FL_HALF_LINE;
+	} else {
+		fmt->bt.interlaced = V4L2_DV_PROGRESSIVE;
+	}
 	return true;
 }
 EXPORT_SYMBOL_GPL(v4l2_detect_gtf);
diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
index 4becc67..eecd310 100644
--- a/include/media/v4l2-dv-timings.h
+++ b/include/media/v4l2-dv-timings.h
@@ -117,6 +117,7 @@ void v4l2_print_dv_timings(const char *dev_prefix, const char *prefix,
  * @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
@@ -124,7 +125,7 @@ void v4l2_print_dv_timings(const char *dev_prefix, const char *prefix,
  * in with the found CVT timings.
  */
 bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
-		u32 polarities, struct v4l2_dv_timings *fmt);
+		u32 polarities, bool interlaced, struct v4l2_dv_timings *fmt);
 
 /** v4l2_detect_gtf - detect if the given timings follow the GTF standard
  * @frame_height - the total height of the frame (including blanking) in lines.
@@ -132,6 +133,7 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
  * @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
  * @aspect - preferred aspect ratio. GTF has no method of determining the
  *		aspect ratio in order to derive the image width from the
  *		image height, so it has to be passed explicitly. Usually
@@ -144,7 +146,7 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
  * in with the found GTF timings.
  */
 bool v4l2_detect_gtf(unsigned frame_height, unsigned hfreq, unsigned vsync,
-		u32 polarities, struct v4l2_fract aspect,
+		u32 polarities, bool interlaced, struct v4l2_fract aspect,
 		struct v4l2_dv_timings *fmt);
 
 /** v4l2_calc_aspect_ratio - calculate the aspect ratio based on bytes
-- 
1.9.1

[RFC PATCH 2/4] vivid: Use interlaced info for cvt/gtf timing detection

[Prashant Laddha]

The detect_cvt/gtf() now supports timing calculations for interlaced
format.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/platform/vivid/vivid-vid-cap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index a3b19dc..9b76463 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, false, timings))
+				    bt->polarities, bt->interlaced, timings))
 			return true;
 	}
 
@@ -1634,7 +1634,7 @@ 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, false,
+				    bt->polarities, bt->interlaced,
 				    aspect_ratio, timings))
 			return true;
 	}
-- 
1.9.1

[RFC PATCH 3/4] adv7604: Use interlaced info for cvt/gtf timing detection

[Prashant Laddha]

The interlaced information from stdi is passed to detect_cvt/gtf().
These functions now supports timing calculations for interlaced
format.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Martin Bugge <marbugge@cisco.com>
Cc: Mats Randgaard <matrandg@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/i2c/adv7604.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 74abfd4..cf9383b 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),
-			false, timings))
+			stdi->interlaced, 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),
-			false, state->aspect_ratio, timings))
+			stdi->interlaced, state->aspect_ratio, timings))
 		return 0;
 
 	v4l2_dbg(2, debug, sd,
-- 
1.9.1

[RFC PATCH 4/4] adv7842: Use interlaced info for cvt/gtf timing detection

[Prashant Laddha]

The interlaced information from stdi is passed to detect_cvt/gtf().

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Martin Bugge <marbugge@cisco.com>
Cc: Mats Randgaard <matrandg@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 drivers/media/i2c/adv7842.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index 90cbead..698fe38 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),
-			false, timings))
+			stdi->interlaced, 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),
-			false, state->aspect_ratio, timings))
+			stdi->interlaced, state->aspect_ratio, timings))
 		return 0;
 
 	v4l2_dbg(2, debug, sd,
-- 
1.9.1

Re: [RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf

[Hans Verkuil]

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.verkuil@cisco.com>
> Cc: Martin Bugge <marbugge@cisco.com>
> Cc: Mats Randgaard <matrandg@cisco.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Prashant Laddha <prladdha@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 polarities, struct v4l2_dv_timings *fmt)
> +		u32 polarities, bool interlaced, struct v4l2_dv_timings *fmt)
>  {
>  	int  v_fp, v_bp, h_fp, h_bp, hsync;
>  	int  frame_width, image_height, image_width;
> @@ -384,7 +385,11 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
>  		if (v_bp < CVT_MIN_V_BPORCH)
>  			v_bp = CVT_MIN_V_BPORCH;
>  	}
> -	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;
> @@ -457,11 +462,20 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
>  	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;
>  	fmt->bt.pixelclock = pix_clk;
>  	fmt->bt.standards = V4L2_DV_BT_STD_CVT;
>  	if (reduced_blanking)
>  		fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING;
> +	if (interlaced) {
> +		fmt->bt.interlaced = V4L2_DV_INTERLACED;
> +		fmt->bt.il_vfrontporch = v_fp;
> +		fmt->bt.il_vsync = vsync;
> +		fmt->bt.il_vbackporch = v_bp + 1;
> +		fmt->bt.flags |= V4L2_DV_FL_HALF_LINE;
> +	} else {
> +		fmt->bt.interlaced = V4L2_DV_PROGRESSIVE;
> +	}
>  	return true;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_detect_cvt);
> @@ -500,6 +514,7 @@ EXPORT_SYMBOL_GPL(v4l2_detect_cvt);
>   * @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
>   * @aspect - preferred aspect ratio. GTF has no method of determining the
>   *		aspect ratio in order to derive the image width from the
>   *		image height, so it has to be passed explicitly. Usually
> @@ -515,6 +530,7 @@ bool v4l2_detect_gtf(unsigned frame_height,
>  		unsigned hfreq,
>  		unsigned vsync,
>  		u32 polarities,
> +		bool interlaced,
>  		struct v4l2_fract aspect,
>  		struct v4l2_dv_timings *fmt)
>  {
> @@ -539,9 +555,11 @@ bool v4l2_detect_gtf(unsigned frame_height,
>  
>  	/* Vertical */
>  	v_fp = GTF_V_FP;
> -
>  	v_bp = (GTF_MIN_VSYNC_BP * hfreq + 500000) / 1000000 - 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.

Regards,

	Hans

>  	fmt->bt.pixelclock = pix_clk;
>  	fmt->bt.standards = V4L2_DV_BT_STD_GTF;
>  	if (!default_gtf)
>  		fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING;
> +	if (interlaced) {
> +		fmt->bt.interlaced = V4L2_DV_INTERLACED;
> +		fmt->bt.il_vfrontporch = v_fp;
> +		fmt->bt.il_vsync = vsync;
> +		fmt->bt.il_vbackporch = v_bp + 1;
> +		fmt->bt.flags |= V4L2_DV_FL_HALF_LINE;
> +	} else {
> +		fmt->bt.interlaced = V4L2_DV_PROGRESSIVE;
> +	}
>  	return true;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_detect_gtf);
> diff --git a/include/media/v4l2-dv-timings.h b/include/media/v4l2-dv-timings.h
> index 4becc67..eecd310 100644
> --- a/include/media/v4l2-dv-timings.h
> +++ b/include/media/v4l2-dv-timings.h
> @@ -117,6 +117,7 @@ void v4l2_print_dv_timings(const char *dev_prefix, const char *prefix,
>   * @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
> @@ -124,7 +125,7 @@ void v4l2_print_dv_timings(const char *dev_prefix, const char *prefix,
>   * in with the found CVT timings.
>   */
>  bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
> -		u32 polarities, struct v4l2_dv_timings *fmt);
> +		u32 polarities, bool interlaced, struct v4l2_dv_timings *fmt);
>  
>  /** v4l2_detect_gtf - detect if the given timings follow the GTF standard
>   * @frame_height - the total height of the frame (including blanking) in lines.
> @@ -132,6 +133,7 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
>   * @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
>   * @aspect - preferred aspect ratio. GTF has no method of determining the
>   *		aspect ratio in order to derive the image width from the
>   *		image height, so it has to be passed explicitly. Usually
> @@ -144,7 +146,7 @@ bool v4l2_detect_cvt(unsigned frame_height, unsigned hfreq, unsigned vsync,
>   * in with the found GTF timings.
>   */
>  bool v4l2_detect_gtf(unsigned frame_height, unsigned hfreq, unsigned vsync,
> -		u32 polarities, struct v4l2_fract aspect,
> +		u32 polarities, bool interlaced, struct v4l2_fract aspect,
>  		struct v4l2_dv_timings *fmt);
>  
>  /** v4l2_calc_aspect_ratio - calculate the aspect ratio based on bytes
> 

Re: [RFC PATCH 1/4] v4l2-dv-timings: Add interlace support in detect cvt/gtf

[Prashant Laddha (prladdha)]

Thanks for reviewing Hans. My answer below -

On 15/05/15 3:28 pm, "Hans Verkuil" <hverkuil@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 + 500000) / 1000000 - 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
 

>