[RFC PATCH 0/2] v4l2-utils: add support for RB v2 in cvt modeline

[RFC PATCH 0/2] v4l2-utils: add support for RB v2 in cvt modeline

[Prashant Laddha]

Hi,

Please find patches adding support for reduced blanking v2 (RB v2) in cvt modeline
calculations in v4l2-utils. Recently, RB v2 support was added to v4l2-dv-timings and
RB v2 support in v4l2-utils is a follow up on that work. Please review.

Regards,
Prashant

Prashant Laddha (2):
  v4l2-ctl-modes: add support for reduced blanking version 2
  v4l2-utils: extend set-dv-timing options for RB version

 utils/v4l2-ctl/v4l2-ctl-modes.cpp | 44 +++++++++++++++++++++++++++++++--------
 utils/v4l2-ctl/v4l2-ctl-stds.cpp  | 11 +++++++++-
 utils/v4l2-ctl/v4l2-ctl.h         |  3 ++-
 3 files changed, 47 insertions(+), 11 deletions(-)

-- 
1.9.1

[RFC PATCH 1/2] v4l2-ctl-modes: add support for reduced blanking version 2

[Prashant Laddha]

Added support for reduced blanking version 2 (RB V2) in cvt
modeline calculations. Recently, RB V2 support was added to
v4l2-dv-timings. This patch follows up on that work.

Extended calc_cvt_modeline() api to pass an additional flag to
indicate the version of reduced blanking. This flag takes effect
only when reduced blanking is set to true. By default, timings
are calculated for reduced blanking version 1.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 utils/v4l2-ctl/v4l2-ctl-modes.cpp | 44 +++++++++++++++++++++++++++++++--------
 utils/v4l2-ctl/v4l2-ctl-stds.cpp  |  2 +-
 utils/v4l2-ctl/v4l2-ctl.h         |  3 ++-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/utils/v4l2-ctl/v4l2-ctl-modes.cpp b/utils/v4l2-ctl/v4l2-ctl-modes.cpp
index 7768998..b008f06 100644
--- a/utils/v4l2-ctl/v4l2-ctl-modes.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-modes.cpp
@@ -52,6 +52,7 @@ static bool valid_params(int width, int height, int refresh_rate)
  */
 
 #define CVT_PXL_CLK_GRAN    (250000)  /* pixel clock granularity */
+#define CVT_PXL_CLK_GRAN_RB_V2 (1000)	/* granularity for reduced blanking v2*/
 
 /* Normal blanking */
 #define CVT_MIN_V_BPORCH     (7)  /* lines */
@@ -77,6 +78,12 @@ static bool valid_params(int width, int height, int refresh_rate)
 #define CVT_RB_H_BPORCH       (80)       /* pixels */
 #define CVT_RB_H_BLANK       (160)       /* pixels */
 
+/* Reduce blanking Version 2 */
+#define CVT_RB_V2_H_BLANK     80       /* pixels */
+#define CVT_RB_MIN_V_FPORCH    3       /* lines  */
+#define CVT_RB_V2_MIN_V_FPORCH 1       /* lines  */
+#define CVT_RB_V_BPORCH        6       /* lines  */
+
 static int v_sync_from_aspect_ratio(int width, int height)
 {
 	if (((height * 4 / 3) / CVT_CELL_GRAN) * CVT_CELL_GRAN == width)
@@ -113,6 +120,7 @@ static int v_sync_from_aspect_ratio(int width, int height)
  * @image_height
  * @refresh_rate
  * @reduced_blanking: whether to use reduced blanking
+ * @use_rb_v2: whether to use reduced blanking version 2
  * @interlaced: whether to compute an interlaced mode
  * @cvt: stores results of cvt timing calculation
  *
@@ -123,7 +131,8 @@ static int v_sync_from_aspect_ratio(int width, int height)
 
 bool calc_cvt_modeline(int image_width, int image_height,
 		       int refresh_rate, bool reduced_blanking,
-		       bool interlaced, struct v4l2_bt_timings *cvt)
+		       bool interlaced, bool use_rb_v2,
+		       struct v4l2_bt_timings *cvt)
 {
 	int h_sync;
 	int v_sync;
@@ -149,6 +158,12 @@ bool calc_cvt_modeline(int image_width, int image_height,
 	int interlace;
 	int v_refresh;
 	int pixel_clock;
+	int clk_gran;
+	bool rb_v2 = false;
+
+	rb_v2 = (reduced_blanking && use_rb_v2) ? true : false;
+
+	clk_gran = rb_v2 ? CVT_PXL_CLK_GRAN_RB_V2 : CVT_PXL_CLK_GRAN;
 
 	if (!valid_params(image_width, image_height, refresh_rate))
 		return false;
@@ -187,7 +202,7 @@ bool calc_cvt_modeline(int image_width, int image_height,
 	active_h_pixel = h_pixel_rnd;
 	active_v_lines = v_lines_rnd;
 
-	v_sync = v_sync_from_aspect_ratio(h_pixel, v_lines);
+	v_sync = rb_v2 ? 8 : v_sync_from_aspect_ratio(h_pixel, v_lines);
 
 	if (!reduced_blanking) {
 		int tmp1, tmp2;
@@ -233,12 +248,13 @@ bool calc_cvt_modeline(int image_width, int image_height,
 
 		pixel_clock =  ((long long)total_h_pixel * HV_FACTOR * 1000000)
 				/ h_period;
-		pixel_clock -= pixel_clock  % CVT_PXL_CLK_GRAN;
+		pixel_clock -= pixel_clock  % clk_gran;
 	} else {
 		/* Reduced blanking */
 
 		int vbi_lines;
 		int tmp1, tmp2;
+		int min_vbi_lines;
 
 		/* estimate horizontal period. */
 		tmp1 = HV_FACTOR * 1000000 -
@@ -249,10 +265,15 @@ bool calc_cvt_modeline(int image_width, int image_height,
 
 		vbi_lines = CVT_RB_MIN_V_BLANK * HV_FACTOR / h_period + 1;
 
-		if (vbi_lines < (CVT_RB_V_FPORCH + v_sync + CVT_MIN_V_BPORCH))
-			vbi_lines = CVT_RB_V_FPORCH + v_sync + CVT_MIN_V_BPORCH;
+		if (rb_v2)
+			min_vbi_lines = CVT_RB_V2_MIN_V_FPORCH + v_sync + CVT_RB_V_BPORCH;
+		else
+			min_vbi_lines = CVT_RB_V_FPORCH + v_sync + CVT_MIN_V_BPORCH;
+
+		if (vbi_lines < min_vbi_lines)
+			vbi_lines = min_vbi_lines;
 
-		h_blank = CVT_RB_H_BLANK;
+		h_blank = rb_v2 ? CVT_RB_V2_H_BLANK : CVT_RB_H_BLANK;
 		v_blank = vbi_lines;
 
 		total_h_pixel = active_h_pixel + h_blank;
@@ -263,12 +284,17 @@ bool calc_cvt_modeline(int image_width, int image_height,
 		h_bp = h_blank / 2;
 		h_fp = h_blank - h_bp - h_sync;
 
-		v_fp = CVT_RB_V_FPORCH;
-		v_bp = v_blank - v_fp - v_sync;
+		if (rb_v2) {
+			v_bp = CVT_RB_V_BPORCH;
+			v_fp = v_blank - v_bp - v_sync;
+		} else {
+			v_fp = CVT_RB_V_FPORCH;
+			v_bp = v_blank - v_fp - v_sync;
+		}
 
 		pixel_clock = v_refresh * total_h_pixel *
 			      (2 * total_v_lines + interlace) / 2;
-		pixel_clock -= pixel_clock  % CVT_PXL_CLK_GRAN;
+		pixel_clock -= pixel_clock  % clk_gran;
 	}
 
 	cvt->standards 	 = V4L2_DV_BT_STD_CVT;
diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
index e725fa8..c0e919b 100644
--- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
@@ -240,7 +240,7 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
 		timings_valid = calc_cvt_modeline(width, height, fps,
 			              r_blank == 1 ? true : false,
 			              interlaced == 1 ? true : false,
-			              bt);
+			              false, bt);
 	} else {
 		timings_valid = calc_gtf_modeline(width, height, fps,
 			              r_blank == 1 ? true : false,
diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
index efb5eb3..8bf1610 100644
--- a/utils/v4l2-ctl/v4l2-ctl.h
+++ b/utils/v4l2-ctl/v4l2-ctl.h
@@ -351,7 +351,8 @@ void edid_get(int fd);
 /* v4l2-ctl-modes.cpp */
 bool calc_cvt_modeline(int image_width, int image_height,
 		       int refresh_rate, bool reduced_blanking,
-		       bool interlaced, struct v4l2_bt_timings *cvt);
+		       bool interlaced, bool use_rb_v2,
+		       struct v4l2_bt_timings *cvt);
 
 bool calc_gtf_modeline(int image_width, int image_height,
 		       int refresh_rate, bool reduced_blanking,
-- 
1.9.1

[RFC PATCH 2/2] v4l2-utils: extend set-dv-timing options for RB version

[Prashant Laddha]

To support the timings calculations for reduced blanking version 2
(RB v2), extended the command line options to include flag indicating
whether to use RB V2 or not. Updated the command usage for the same.

Cc: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Prashant Laddha <prladdha@cisco.com>
---
 utils/v4l2-ctl/v4l2-ctl-stds.cpp | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
index c0e919b..9734c80 100644
--- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
@@ -41,7 +41,10 @@ void stds_usage(void)
 	       "                     index=<index>: use the index as provided by --list-dv-timings\n"
 	       "                     or specify timings using cvt/gtf options as follows:\n"
 	       "                     cvt/gtf,width=<width>,height=<height>,fps=<frames per sec>\n"
-	       "                     interlaced=<0/1>,reduced-blanking=<0/1>\n"
+	       "                     interlaced=<0/1>,reduced-blanking=<0/1>,use-rb-v2=<0/1>\n"
+	       "                     use-rb-v2 indicates whether to use reduced blanking version 2\n"
+	       "                     or not. This flag is relevant only for cvt timings and has\n"
+	       "                     effect only if reduced-blanking=1\n"
 	       "                     or give a fully specified timings:\n"
 	       "                     width=<width>,height=<height>,interlaced=<0/1>,\n"
 	       "                     polarities=<polarities mask>,pixelclock=<pixelclock Hz>,\n"
@@ -148,6 +151,7 @@ enum timing_opts {
 	GTF,
 	FPS,
 	REDUCED_BLANK,
+	USE_RB_V2,
 };
 
 static int parse_timing_subopt(char **subopt_str, int *value)
@@ -175,6 +179,7 @@ static int parse_timing_subopt(char **subopt_str, int *value)
 		"gtf",
 		"fps",
 		"reduced-blanking",
+		"use-rb-v2",
 		NULL
 	};
 
@@ -205,6 +210,7 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
 	int fps = 0;
 	int r_blank = 0;
 	int interlaced = 0;
+	int use_rb_v2 = 0;
 
 	bool timings_valid = false;
 
@@ -231,6 +237,8 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
 		case INTERLACED:
 			interlaced = opt_val;
 			break;
+		case USE_RB_V2:
+			use_rb_v2 = opt_val;
 		default:
 			break;
 		}
@@ -240,7 +248,8 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
 		timings_valid = calc_cvt_modeline(width, height, fps,
 			              r_blank == 1 ? true : false,
 			              interlaced == 1 ? true : false,
-			              false, bt);
+			              use_rb_v2 == 1 ? true : false,
+			              bt);
 	} else {
 		timings_valid = calc_gtf_modeline(width, height, fps,
 			              r_blank == 1 ? true : false,
-- 
1.9.1

Re: [RFC PATCH 2/2] v4l2-utils: extend set-dv-timing options for RB version

[Hans Verkuil]

On 06/16/2015 11:30 AM, Prashant Laddha wrote:
> To support the timings calculations for reduced blanking version 2
> (RB v2), extended the command line options to include flag indicating
> whether to use RB V2 or not. Updated the command usage for the same.
> 
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Signed-off-by: Prashant Laddha <prladdha@cisco.com>
> ---
>  utils/v4l2-ctl/v4l2-ctl-stds.cpp | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> index c0e919b..9734c80 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> @@ -41,7 +41,10 @@ void stds_usage(void)
>  	       "                     index=<index>: use the index as provided by --list-dv-timings\n"
>  	       "                     or specify timings using cvt/gtf options as follows:\n"
>  	       "                     cvt/gtf,width=<width>,height=<height>,fps=<frames per sec>\n"
> -	       "                     interlaced=<0/1>,reduced-blanking=<0/1>\n"
> +	       "                     interlaced=<0/1>,reduced-blanking=<0/1>,use-rb-v2=<0/1>\n"
> +	       "                     use-rb-v2 indicates whether to use reduced blanking version 2\n"
> +	       "                     or not. This flag is relevant only for cvt timings and has\n"
> +	       "                     effect only if reduced-blanking=1\n"

Why not just allow a value of 2 for the reduced-blanking argument instead
of introducing a new argument? For gtf 1 and 2 mean the same thing, for cvt 1
will use the standard RB and 2 RBv2.

Seems simpler to me. It also means that calc_cvt_modeline doesn't need a new
argument, just that bool reduced_blanking becomes int reduced_blanking.

Other than this it looks good to me.

Regards,

	Hans

>  	       "                     or give a fully specified timings:\n"
>  	       "                     width=<width>,height=<height>,interlaced=<0/1>,\n"
>  	       "                     polarities=<polarities mask>,pixelclock=<pixelclock Hz>,\n"
> @@ -148,6 +151,7 @@ enum timing_opts {
>  	GTF,
>  	FPS,
>  	REDUCED_BLANK,
> +	USE_RB_V2,
>  };
>  
>  static int parse_timing_subopt(char **subopt_str, int *value)
> @@ -175,6 +179,7 @@ static int parse_timing_subopt(char **subopt_str, int *value)
>  		"gtf",
>  		"fps",
>  		"reduced-blanking",
> +		"use-rb-v2",
>  		NULL
>  	};
>  
> @@ -205,6 +210,7 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
>  	int fps = 0;
>  	int r_blank = 0;
>  	int interlaced = 0;
> +	int use_rb_v2 = 0;
>  
>  	bool timings_valid = false;
>  
> @@ -231,6 +237,8 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
>  		case INTERLACED:
>  			interlaced = opt_val;
>  			break;
> +		case USE_RB_V2:
> +			use_rb_v2 = opt_val;
>  		default:
>  			break;
>  		}
> @@ -240,7 +248,8 @@ static void get_cvt_gtf_timings(char *subopt, int standard,
>  		timings_valid = calc_cvt_modeline(width, height, fps,
>  			              r_blank == 1 ? true : false,
>  			              interlaced == 1 ? true : false,
> -			              false, bt);
> +			              use_rb_v2 == 1 ? true : false,
> +			              bt);
>  	} else {
>  		timings_valid = calc_gtf_modeline(width, height, fps,
>  			              r_blank == 1 ? true : false,
> 

Re: [RFC PATCH 2/2] v4l2-utils: extend set-dv-timing options for RB version

[Prashant Laddha (prladdha)]

Thanks for your comments Hans.

On 19/06/15 11:37 am, "Hans Verkuil" <hverkuil@xs4all.nl> wrote:

>>  utils/v4l2-ctl/v4l2-ctl-stds.cpp | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
>>b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
>> index c0e919b..9734c80 100644
>> --- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
>> +++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
>> @@ -41,7 +41,10 @@ void stds_usage(void)
>>  	       "                     index=<index>: use the index as provided
>>by --list-dv-timings\n"
>>  	       "                     or specify timings using cvt/gtf options
>>as follows:\n"
>>  	       "      
>>cvt/gtf,width=<width>,height=<height>,fps=<frames per sec>\n"
>> -	       "      
>>interlaced=<0/1>,reduced-blanking=<0/1>\n"
>> +	       "      
>>interlaced=<0/1>,reduced-blanking=<0/1>,use-rb-v2=<0/1>\n"
>> +	       "                     use-rb-v2 indicates whether to use
>>reduced blanking version 2\n"
>> +	       "                     or not. This flag is relevant only for
>>cvt timings and has\n"
>> +	       "                     effect only if reduced-blanking=1\n"
>
>Why not just allow a value of 2 for the reduced-blanking argument instead
>of introducing a new argument? For gtf 1 and 2 mean the same thing, for
>cvt 1
>will use the standard RB and 2 RBv2.

The only reason for which I added another flag is to make control
parameters same as cvt spreadsheet. But, yes, we could accept
reduced-blanking = 2. I will post version 2 with this change.

>
>Seems simpler to me. It also means that calc_cvt_modeline doesn't need a
>new
>argument, just that bool reduced_blanking becomes int reduced_blanking.
>
>Other than this it looks good to me.
>
>Regards,
>
>	Hans
>
>>  	       "                     or give a fully specified timings:\n"
>>  	       "      
>>width=<width>,height=<height>,interlaced=<0/1>,\n"
>>  	       "                     polarities=<polarities
>>mask>,pixelclock=<pixelclock Hz>,\n"
>> @@ -148,6 +151,7 @@ enum timing_opts {
>>  	GTF,
>>  	FPS,
>>  	REDUCED_BLANK,
>> +	USE_RB_V2,
>>  };
>>  
>>  static int parse_timing_subopt(char **subopt_str, int *value)
>> @@ -175,6 +179,7 @@ static int parse_timing_subopt(char **subopt_str,
>>int *value)
>>  		"gtf",
>>  		"fps",
>>  		"reduced-blanking",
>> +		"use-rb-v2",
>>  		NULL
>>  	};
>>  
>> @@ -205,6 +210,7 @@ static void get_cvt_gtf_timings(char *subopt, int
>>standard,
>>  	int fps = 0;
>>  	int r_blank = 0;
>>  	int interlaced = 0;
>> +	int use_rb_v2 = 0;
>>  
>>  	bool timings_valid = false;
>>  
>> @@ -231,6 +237,8 @@ static void get_cvt_gtf_timings(char *subopt, int
>>standard,
>>  		case INTERLACED:
>>  			interlaced = opt_val;
>>  			break;
>> +		case USE_RB_V2:
>> +			use_rb_v2 = opt_val;
>>  		default:
>>  			break;
>>  		}
>> @@ -240,7 +248,8 @@ static void get_cvt_gtf_timings(char *subopt, int
>>standard,
>>  		timings_valid = calc_cvt_modeline(width, height, fps,
>>  			              r_blank == 1 ? true : false,
>>  			              interlaced == 1 ? true : false,
>> -			              false, bt);
>> +			              use_rb_v2 == 1 ? true : false,
>> +			              bt);
>>  	} else {
>>  		timings_valid = calc_gtf_modeline(width, height, fps,
>>  			              r_blank == 1 ? true : false,
>> 
>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in