Re: [PATCH v2 08/12] media: i2c: Add hblank control to ov8865

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Tue, Aug 10, 2021 at 11:07:22PM +0100, Daniel Scally wrote:
> On 10/08/2021 15:29, Sakari Ailus wrote:
> > On Mon, Aug 09, 2021 at 11:58:41PM +0100, Daniel Scally wrote:
> >> @@ -2542,6 +2544,13 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
> >>  				     0, 0, ov8865_test_pattern_menu);
> >>  
> >>  	/* Blanking */
> >> +	hblank = mode->hts < mode->output_size_x ? 0 : mode->hts - mode->output_size_x;
> >
> > Is the result in relation with the analogue crop size? Based on the above
> > it wouldn't seem like that.
> 
> This was a weird one actually. My understanding was that HTS should
> always be >= the horizontal crop plus hblank...but that doesn't hold
> true for some of this driver's modes and nor does it hold true when
> running the camera in Windows (I checked the registers whilst running
> it). So I went with setting hblank to 0 if the mode's HTS exceeded the
> horizontal crop as the only way I could see to reconcile that.

There's something very fishy here, HTS is, by definition, equal to the
analog crop width plus the horizontal blanking. I suspect that the
values in ov8865_modes are wrong.

> >> +	ctrls->hblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_HBLANK, hblank,
> >> +					  hblank, 1, hblank);
> >> +
> >> +	if (ctrls->hblank)
> >> +		ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> >> +
> >>  	vblank_max = OV8865_TIMING_MAX_VTS - mode->output_size_y;
> >>  	vblank_def = mode->vts - mode->output_size_y;
> >>  	ctrls->vblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_VBLANK,

-- 
Regards,

Laurent Pinchart