Re: [PATCH 2/4] media: platform: dwc: Support for DW CSI-2 Host

[Sakari Ailus]

Hi Hans,

On Wed, May 17, 2017 at 09:00:59AM +0200, Hans Verkuil wrote:
> Hi Sakari,
> 
> Can you comment on this? You are much more a CSI sensor expert than I am.

Sure. Thanks for the ping.

> 
> On 16/05/17 20:18, Ramiro Oliveira wrote:
> > Hi Hans,
> > 
> > Thank you very much for your feedback.
> > 
> > On 5/8/2017 11:38 AM, Hans Verkuil wrote:
> >> Hi Ramiro,
> >>
> >> My sincere apologies for the long delay in reviewing this. The good news is that
> >> I should have more time for reviews going forward, so I hope I'll be a lot quicker
> >> in the future.
> >>
> >> On 03/07/2017 03:37 PM, Ramiro Oliveira wrote:
> 
> <snip>
> 
> >>> +		if (mf->width == bt->width && mf->height == bt->width) {
> >>
> >> This is way too generic. There are many preset timings that have the same
> >> width and height but different blanking periods.
> >>
> >> I am really not sure how this is supposed to work. If you want to support
> >> HDMI here, then I would expect to see support for the s_dv_timings op and friends
> >> in this driver. And I don't see support for that in the host driver either.
> >>
> >> Is this a generic csi driver, or specific for hdmi? Or supposed to handle both?
> > 
> > This is a generic CSI driver.
> > 
> >>
> >> Can you give some background and clarification of this?
> > 
> > This piece of code might seem strange but I'm just using it fill our controller
> > timing configuration.
> > 
> > I don't have any specific requirements, but they should match, more or less, the
> > sensor configurations, so I decided to re-use the HDMI blanking values, since,
> > usually, they match with the sensor configurations
> > 
> > So, my intention is to check if there is any HDMI preset that matches the
> > required width and height, and then use the blanking values to configure our
> > controller. I know this might not be very common, and I'm open to use different
> > approaches, but from my perspective it seems to work fine.

What kind of timing information do you need to configure the receiver?

If you pick a random HDMI configuration it can't be expected to match an
unrelated configuration of a sensor. Generally CSI-2 bus frequency, number
of lanes and horizontal and vertical blanking are enough to calculate what
the hardware needs regarding timing. The received image size is necessary
for other purposes.

Do you see that you'd need something else in addition to that?

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx