Re: [PATCH 2/3] mx3_camera: Support correctly the YUV222 and BAYERconfigurations of CSI

[Alberto Panizzo]

On Mon, 2011-01-03 at 20:37 +0100, Guennadi Liakhovetski wrote:
> On Mon, 3 Jan 2011, Alberto Panizzo wrote:
> 
> > On Thu, 2010-12-30 at 20:38 +0100, Guennadi Liakhovetski wrote:
> > > On Sat, 18 Dec 2010, Guennadi Liakhovetski wrote:
> > > 
> > > > Alberto
> > > > 
> > > > it would be slowly on the time to address my comments and submit updates. 
> > > > While at it, also, please update the subject - you probably meant "YUV422" 
> > > > or "YUV444" there, also below:
> > > 
> > > Ok, I'm dropping this patch
> > > 
> > > Alberto, I've applied and pushed your other 2 patches from this series, 
> > > but I've dropped this one. The reason is not (only), that you didn't reply 
> > > to my two last mails with update-requests. But because of that I took the 
> > > time today to look deeper into detail at this patch. And as a result, I 
> > > don't think it is correct.
> > > 
> > > Currently the mx3_camera driver transfers data from video clients (camera 
> > > sensors) only in one mode - as raw data, 1-to-1. This is extablished in 
> > > the way, how it creates format translation tables during the initial 
> > > negotiation with client drivers in mx3_camera_get_formats().
> > > 
> > > Your patch is trying to add support for specific modes on CSI, but is only 
> > > doing this in the transfer part of the driver, and not in the negotiation 
> > > part. So, if you really need native support for various pixel formats, 
> > > this is a wrong way to do this. If you only want to transfer data from 
> > > your sensor into RAM and the current driver is failing for you, then this 
> > > is a wrong way to do this, and the bug has to be found and fixed, while 
> > > maintaining the present pass-through only model.
> > > 
> > 
> > This patch shows that IPU and CSI manage parameters in different 
> > units. It shows that an unknown at the CSI pixel format, that require n
> > bytes per pixel, have to be considered generic on the IPU side and the
> > parameters of the DMA and CSI have to be set properly to support it.
> > In this way also 10-bit wide pixels formats can be managed in pass
> > through mode, setting properly the IPU and CSI parameters.
> > 
> > So, this patch shows that the CSI can manage successfully n-byte wide
> > pixel codes (tested with n = 1,2) without breaking the old behaviour
> > of providing 10-bit wide pixel formats with 8-bit wide ones.
> > 
> > The next step is to uniform also the pixel-code translations at this 
> > type of management. Being able to capture real 10-bit wide samples.
> > 
> > This patch also, make use of a native functionality of the CSI: capture 
> > a YUV422 format. 
> > In this case the CSI convert this pixel format to YUV444 sent to the 
> > BUS  and the IPU re-pack the YUV444 to YUV422 into the memory.
> > 
> > This shows how CSI and IPU manage formats different than the generic 
> > one and open the way to understand how to support the communication
> > between agents of the IPU encoding chain.
> > 
> > Maybe the last part is misleading you and can be dropped out from this 
> > patch as an enhancement: the YUV422 interleaved format can be 
> > successfully managed as CSI-BAYER/IPU-GENERIC one, the same as rgb565.
> > Supporting the CSI-YUV422 is a plus only to show how the CSI works.
> 
> Let's try slowly again:
> 
> 1. The current mainline driver doesn't work for you, right? What exactly 
> is failing and how? What fourcc format?

Yes, does not work for me for both YUV422 interleaved and rgb565.
What is captured is an image that have in the bottom half the violet 
color and in the upper half, half of the real image (divided 
vertically) with even rows on the left and odd rows on the right.


> 2. Do you think, it would be possible to fix the driver to also support 
> your use-case with the present generic / pass-through mode? Have you tried 
> this? Could you try? That would be a bug-fix.

Yes, this is the way I told about: dividing the geometry fixes from the 
special YUV422 support.

> 
> 3. After the first two questions are answered, then we can think about 
> extending the driver by adding native support forvarious specific formats.

Yes, sure. I'll try to explain better what the single patches are 
fixing, improving especially for these core functionality.


Best Regards,
Alberto!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/