Hello all On 09/09/2021 23:36, Daniel Scally wrote: > Hi Paul > > On 14/08/2021 21:56, Laurent Pinchart wrote: >> Hi Daniel, >> >> On Fri, Aug 13, 2021 at 10:45:48AM +0100, Daniel Scally wrote: >>> On 13/08/2021 04:05, Laurent Pinchart wrote: >>>> 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. >>> I thought that initially too but confirming that the same thing happened >>> running windows switched me into "you're probably wrong" mode. If we're >>> confident that the HTS is likely wrong though I can add an extra patch >>> to bring those into lining with that definition. >> I think it's worth investigating this. The hblank computed here is >> clearly incorrect, and would thus be useless for all practical purposes. >> As usual with OmniVision, the datasheet is also quite useless. >> >> Paul, do you have any information about this ? > > A gentle ping on this...I played around setting HTS / VTS values whilst > the camera was running windows; and it behaves as you'd expect it to > (raising/lowering the frame rate), so as far as I can tell the sensor > itself isn't doing anything unusual... So, looking at this again. The mode in question has: .output_size_x = 3264 .hts = 1944 .output_size_y = 2448 .vts = 2470 .frame_interval = { 1, 30 } And the driver sets a link frequency of 360MHz. That makes the pixel rate, depending on whether we're looking at 2 or 4 data lanes, either 144MHz or 288MHz. I think the HTS there is calculated so that the 2 lane configuration can make 30 FPS. Perhaps it would be better to default in the mode to the "ideal" 4-lane 30fps setting (by upping the .hts to 3888), but rather than hardcoding the frame interval there, calculate it for .g_frame_interval() based on the number of data lanes found in the bus (accepting that if we only have 2 it's going to be 15fps rather than 30, which doesn't seem unreasonable for that resolution)