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

["Prashant Laddha (prladdha)" <prladdha@cisco.com>]

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
 

>