On Wed, Oct 21, 2020 at 02:37:47PM +0200, Sakari Ailus wrote: > I guess this indeed might be more a question of what is the purpose of a > PLL calculator. The SMIA PLL calculator was first merged with the SMIAPP > driver back in 2012, and the Aptina PLL calculator soon followed. I think > it is somewhat based on the SMIAPP PLL calculator. I don't have an answer to that question. I originally started hard coding the PLL parameters and then noticed that there is a calculator. So I attempted using it and figured that "it didn't work" (for my use case). That's how I ended up here. > The SMIA PLL configuration depends also on e.g. binning and format > configurations on the sensor, so there's also the runtime aspect that needs > to be taken into account. There are other factors such as the number of > lanes as well. It's not a static configuration. If the PLL calculator needs to run at another time than probing, your concerns about performance (in the earlier thread) make a lot more sense to me. I had previously assumed that it was only ever used during probe. > Then again, CCS PLL allows for a lot more variability than SMIA. This > includes, but is _not limited to_, to number of physical lanes, lane vs. > system speed model, sensor's internal lanes in OP and VT domains, whether > OP domain SYS and PIX clocks are DDR, whether certain configurations can > have only legacy values or if extended values are supported and which PHY > is in use. Feel free to look at it here, it's not yet merged: > > <URL:https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/ccs-pll.c?h=ccs> > > There, the configuration heavily depends on sensor's properties as well. You have succeeded at demonstrating the complexity. > The computational complexity of searching a "closest matching" frequency > there would be vastly higher than aiming for a specific frequency. The > former shouldn't be done at runtime IMHO. > > A PLL calculator that comes up with a closest frequency to something, given > some input parameters, could be certainly useful when deciding what to put > to DT source (while taking EMI considerations into account), but I think > that's different from what a driver would use. A consequence of that would be not using the PLL calculator for this driver and instead hard coding pre-computed PLL values in the device tree. Do you agree with that? I actually implemented my PLL approximator in Python first and converted it to kernel C afterwards. What would be a good place to put a PLL approximator to be used by DT authors? > Does aptina-pll come up with a valid configuration if you specify the > precise link frequency? Given that my PLL approximator has the same type signature as aptina_pll_calculate, this was easy to check. It does not find a valid configuration and both of us should have noticed that without having to run the code. The resulting frequency of the PLL approximator is not exactly 74242424. That's a rounded value. A more precise representation would be 74242424.24242424 and it is not a whole number. aptina_pll_calculate only deals with frequencies that are whole numbers though. It cannot do the job. The actual error message is "pll: no valid combined N*P1 divisor." and that's quite expected given the above. In retrospect, it was a good decision to defer the discussion on my PLL approximator until there is a driver that uses it. We've now quickly discovered the mismatch in assumptions. > ISO sensitivity control is a bit higher level control than the analogue > gain but it mostly does the same thing. I wonder what others think. This is > probably more user friendly but I guess it doesn't cover all values the > hardware is capable of, or does it? All values supported by the hardware are precisely representable in the ISO sensitivity menu. That applies to both imagers even though their analogue gain handling is completely different. > This leads to an interesting question regarding runtime PM --- how does the > driver determine the sensor needs to be powered on if it gets no s_stream > command? One option could be to add a control for external streaming > trigger. I have no clue. This seems to be a show-stopper for runtime PM as is. > How do you stop streaming? Is it level triggered, or how? We permanently put the imager into externally triggered mode. You could also say that we start streaming on probe and never stop. We suppress the trigger signal during reconfiguration. > Which receiver driver are you using this btw.? I cannot give any details on this. Maybe the closest description would be "custom hardware". Getting legal to sign off on this driver was hard enough. Helmut
