USB: add Sensoray 2255 v4l driver

From: Dean Anderson <[email protected]>

drivers/media/video/s2255drv.c new file +123 -0

+static int norm_maxw(struct video_device *vdev)
+{
+	return (vdev->current_norm != V4L2_STD_PAL_B) ?
+	    LINE_SZ_4CIFS_NTSC : LINE_SZ_4CIFS_PAL;
+}
+
+static int norm_maxh(struct video_device *vdev)
+{
+	return (vdev->current_norm != V4L2_STD_PAL_B) ?
+	    (NUM_LINES_1CIFS_NTSC * 2) : (NUM_LINES_1CIFS_PAL * 2);
+}
+
+static int norm_minw(struct video_device *vdev)
+{
+	return (vdev->current_norm != V4L2_STD_PAL_B) ?
+	    LINE_SZ_1CIFS_NTSC : LINE_SZ_1CIFS_PAL;
+}
+
+static int norm_minh(struct video_device *vdev)
+{
+	return (vdev->current_norm != V4L2_STD_PAL_B) ?
+	    (NUM_LINES_1CIFS_NTSC) : (NUM_LINES_1CIFS_PAL);
+}
+
+/*
+ * convert from YUV(YCrCb) to RGB
+ * 65536 R = 76533(Y-16) + 104936 * (Cr-128)
+ * 65536 G = 76533(Y-16) - 53451(Cr-128) - 25703(Cb -128)
+ * 65536 B = 76533(Y-16) + 132677(Cb-128)
+ */
+static void YCrCb2RGB(int Y, int Cr, int Cb, unsigned char *pR,
+		      unsigned char *pG, unsigned char *pB)
+{
+	int R, G, B;
+
+	Y = Y - 16;
+	Cr = Cr - 128;
+	Cb = Cb - 128;
+
+	R = (76533 * Y + 104936 * Cr) >> 16;
+	G = ((76533 * Y) - (53451 * Cr) - (25703 * Cb)) >> 16;
+	B = ((76533 * Y) + (132677 * Cb)) >> 16;
+	/* even with proper conversion, some values still need clipping. */
+	if (R > 255)
+		R = 255;
+	if (G > 255)
+		G = 255;
+	if (B > 255)
+		B = 255;
+	if (R < 0)
+		R = 0;
+	if (G < 0)
+		G = 0;
+	if (B < 0)
+		B = 0;
+	*pR = R;
+	*pG = G;
+	*pB = B;
+	return;
+}
+
+/* converts 2255 planar format to yuyv */
+static void planar422p_to_yuy2(const unsigned char *in, unsigned char *out,
+			       int width, int height)
+{
+	unsigned char *pY;
+	unsigned char *pCb;
+	unsigned char *pCr;
+	unsigned long size = height * width;
+	unsigned int i;
+	pY = (unsigned char *)in;
+	pCr = (unsigned char *)in + height * width;
+	pCb = (unsigned char *)in + height * width + (height * width / 2);
+	for (i = 0; i < size * 2; i += 4) {
+		out[i] = *pY++;
+		out[i + 1] = *pCr++;
+		out[i + 2] = *pY++;
+		out[i + 3] = *pCb++;
+	}
+	return;
+}
+
+/*
+ * basic 422 planar to RGB24 or BGR24 software conversion.
+ * This is best done with MMX. Update to kernel function
+ * when image conversion functions added to kernel.
+ */
+static void planar422p_to_rgb24(const unsigned char *in,
+				unsigned char *out, int width,
+				int height, int rev_order)
+{
+	unsigned char *pY;
+	unsigned char *pYEND;
+	unsigned char *pCb;
+	unsigned char *pCr;
+	unsigned char Cr, Cb, Y, r, g, b;
+	unsigned long k = 0;
+	pY = (unsigned char *)in;
+	pCb = (unsigned char *)in + (height * width);
+	pCr = (unsigned char *)in + (height * width) + (height * width / 2);
+	pYEND = pCb;
+	while (pY < pYEND) {
+		Y = *pY++;
+		Cr = *pCr;
+		Cb = *pCb;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		out[k++] = !rev_order ? b : r;
+		out[k++] = g;
+		out[k++] = !rev_order ? r : b;
+		if (pY >= pYEND)
+			break;
+		Y = *pY++;
+		Cr = *pCr++;
+		Cb = *pCb++;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		out[k++] = !rev_order ? b : r;
+		out[k++] = g;
+		out[k++] = !rev_order ? r : b;
+	}
+	return;
+}
+
+static void planar422p_to_rgb32(const unsigned char *in, unsigned char

*out,

drivers/media/video/s2255drv.c new file +37 -0

+				int width, int height, int rev_order)
+{
+	unsigned char *pY;
+	unsigned char *pYEND;
+	unsigned char *pCb;
+	unsigned char *pCr;
+	unsigned char Cr, Cb, Y, r, g, b;
+	unsigned long k = 0;
+	pY = (unsigned char *)in;
+	pCb = (unsigned char *)in + (height * width);
+	pCr = (unsigned char *)in + (height * width) + (height * width / 2);
+	pYEND = pCb;
+	while (pY < pYEND) {
+		Y = *pY++;
+		Cr = *pCr;
+		Cb = *pCb;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		out[k++] = rev_order ? b : r;
+		out[k++] = g;
+		out[k++] = rev_order ? r : b;
+		out[k++] = 0;
+		if (pY >= pYEND)
+			break;
+		Y = *pY++;
+		Cr = *pCr++;
+		Cb = *pCb++;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		out[k++] = rev_order ? b : r;
+		out[k++] = g;
+		out[k++] = rev_order ? r : b;
+		out[k++] = 0;
+	}
+
+	return;
+}
+
+static void planar422p_to_rgb565(unsigned char const *in, unsigned char

*out,

drivers/media/video/s2255drv.c new file +34 -0

+				 int width, int height, int rev_order)
+{
+	unsigned char *pY;
+	unsigned char *pYEND;
+	unsigned char *pCb;
+	unsigned char *pCr;
+	unsigned char Cr, Cb, Y, r, g, b;
+	unsigned long k = 0;
+	unsigned short rgbbytes;
+	pY = (unsigned char *)in;
+	pCb = (unsigned char *)in + (height * width);
+	pCr = (unsigned char *)in + (height * width) + (height * width / 2);
+	pYEND = pCb;
+	while (pY < pYEND) {
+		Y = *pY++;
+		Cr = *pCr;
+		Cb = *pCb;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		r = r >> 3;
+		g = g >> 2;
+		b = b >> 3;
+		if (rev_order)
+			rgbbytes = b + (g << 5) + (r << (5 + 6));
+		else
+			rgbbytes = r + (g << 5) + (b << (5 + 6));
+		out[k++] = rgbbytes & 0xff;
+		out[k++] = (rgbbytes >> 8) & 0xff;
+		Y = *pY++;
+		Cr = *pCr++;
+		Cb = *pCb++;
+		YCrCb2RGB(Y, Cr, Cb, &r, &g, &b);
+		r = r >> 3;
+		g = g >> 2;
+		b = b >> 3;

Hi Dean, Greg,

Quoted 2 lines

From: Dean Anderson <[email protected]>

Why do you do those conversions in kernelspace? ffmpeg/libswscale has optimized code for colourspace conversions. I know a few drivers do that in kernelspace but it's way more flexible in userspace and depending on the optimization requires less CPU power.

Quoted 1 line · Markus Rechberger

Hi Dean, Greg,

Adding dean to the cc: line... :)

Quoted 4 lines · Greg KH

Quoted 2 lines

From: Dean Anderson <[email protected]>

<big patch snipped>

Quoted 5 lines

Why do you do those conversions in kernelspace? ffmpeg/libswscale has optimized code for colourspace conversions. I know a few drivers do that in kernelspace but it's way more flexible in userspace and depending on the optimization requires less CPU power.

I thought they were there as needed by some V4L1 applications, but that code was recently removed by Dean I think. If they don't need to be there, and userspace apps can properly handle the different colorspace, then I'll be glad to remove them.

Quoted 20 lines · Greg KH

Quoted 2 lines · Markus Rechberger

Hi Dean, Greg,

Adding dean to the cc: line... :)

Quoted 5 lines · Greg KH

Quoted 4 lines

From: Dean Anderson <[email protected]>

<big patch snipped>

Quoted 6 lines

Why do you do those conversions in kernelspace? ffmpeg/libswscale has optimized code for colourspace conversions. I know a few drivers do that in kernelspace but it's way more flexible in userspace and depending on the optimization requires less CPU power.

Quoted 5 lines

I thought they were there as needed by some V4L1 applications, but that code was recently removed by Dean I think. If they don't need to be there, and userspace apps can properly handle the different colorspace, then I'll be glad to remove them.

Virtually all apps (V4L1 & 2) can handle YUV and RGB colorspaces. Certainly all the major ones do and all the major libraries as well.

The problem is when the device only supports some vendor specific or otherwise very uncommon format. In that case not doing the conversion in the kernel means the device won't work with any existing software without patches. In this case, while it's not "the right way", drivers often end up including an in kernel conversion for pragmatic reasons.

This was a problem with the bayer format, but now userspace support for that format is more common.

Quoted 4 lines

Virtually all apps (V4L1 & 2) can handle YUV and RGB colorspaces. Certainly all the major ones do and all the major libraries as well.

Quoted 9 lines

The problem is when the device only supports some vendor specific or otherwise very uncommon format. In that case not doing the conversion in the kernel means the device won't work with any existing software without patches. In this case, while it's not "the right way", drivers often end up including an in kernel conversion for pragmatic reasons.

This was a problem with the bayer format, but now userspace support for that format is more common.

I agree the conversions don't belong in a driver. For the record, the following are done in the 2255 hardware: V4L2_PIX_FMT_GREY and V4L2_PIX_FMT_YUV422P.

Since planar YUV formats such as V4L2_PIX_FMT_YUV422P are still not that well supported, is it possible to keep at least one packed YUV format(V4L2_PIX_FMT_YUYV) in the driver? If not, let me know. I will strongly suggest that the hardware Engineers add YUY2 or YUYV on board in the DSP firmware. Thanks, Dean

Since planar YUV formats such as V4L2_PIX_FMT_YUV422P are still not that well supported, is it possible to keep at least one packed YUV format(V4L2_PIX_FMT_YUYV) in the driver? If not, let me know. I will strongly suggest that the hardware Engineers add YUY2 or YUYV on board in the DSP firmware. Thanks, Dean

3. The firmware stuff. That's an interesting solution. However:

Am Donnerstag 15 Mai 2008 13:38:37 schrieb Oliver Neukum:

Quoted 1 line

3. The firmware stuff. That's an interesting solution. However:

Actually, on second thought, I take that back. It's a bad solution. If you don't want to do it in probe(), the only other sensible place is in open(). That way you can avoid the whole trouble if nobody opens the device. And you need to handle the case of unloaded firmware anyway, so you can trigger firmware load there.

Quoted 8 lines

Am Donnerstag 15 Mai 2008 13:38:37 schrieb Oliver Neukum:

Quoted 1 line

3. The firmware stuff. That's an interesting solution. However:

Actually, on second thought, I take that back. It's a bad solution. If you don't want to do it in probe(), the only other sensible place is in open(). That way you can avoid the whole trouble if nobody opens the device. And you need to handle the case of unloaded firmware anyway, so you can trigger firmware load there.

No, we want to do firmware load on probe, I'll change it to be async so

Am Donnerstag 15 Mai 2008 20:44:24 schrieb Greg KH:

Quoted 10 lines · Oliver Neukum

Quoted 8 lines

Am Donnerstag 15 Mai 2008 13:38:37 schrieb Oliver Neukum:

Quoted 1 line

3. The firmware stuff. That's an interesting solution. However:

Actually, on second thought, I take that back. It's a bad solution. If you don't want to do it in probe(), the only other sensible place is in open(). That way you can avoid the whole trouble if nobody opens the device. And you need to handle the case of unloaded firmware anyway, so you can trigger firmware load there.

Quoted 1 line

No, we want to do firmware load on probe, I'll change it to be async so

Could you state your reasons for that preference?

Quoted 11 lines

Am Donnerstag 15 Mai 2008 20:44:24 schrieb Greg KH:

Quoted 10 lines · Oliver Neukum

Quoted 8 lines

Am Donnerstag 15 Mai 2008 13:38:37 schrieb Oliver Neukum:

Quoted 1 line

3. The firmware stuff. That's an interesting solution. However:

Actually, on second thought, I take that back. It's a bad solution. If you don't want to do it in probe(), the only other sensible place is in open(). That way you can avoid the whole trouble if nobody opens the device. And you need to handle the case of unloaded firmware anyway, so you can trigger firmware load there.

Quoted 3 lines

Quoted 1 line

No, we want to do firmware load on probe, I'll change it to be async so

Could you state your reasons for that preference?

I don't want to create the device nodes, and have userspace think the device really is working, only to have everything fall down and die if open() is called and the firmware isn't present. I'd rather not create the video devices if we know this isn't going to work at all.

+#define S2255_NORMS (V4L2_STD_PAL_B | V4L2_STD_NTSC_M)

+static DEFINE_MUTEX(usb_s2255_open_mutex);

+static int restart_video_queue(struct s2255_dmaqueue *dma_q)

+#define EP_NUM_CONFIG 2

Quoted 1 line

+static DEFINE_MUTEX(usb_s2255_open_mutex);

Hmmm... what happens if the user plugs two or more of such devices?

Am Freitag 16 Mai 2008 04:51:02 schrieb Mauro Carvalho Chehab:

Quoted 4 lines

Quoted 2 lines

+static DEFINE_MUTEX(usb_s2255_open_mutex);

Hmmm... what happens if the user plugs two or more of such devices?

Concurrent calls to open() will be delayed. Not a problem, if the firmware issue is fixed as it needs to be.

Regards
Oliver

Hi Oliver,

Just to be clear. You mean fixing the loading issues(ugly msleep,

etc...) that Mauro mentioned, not the YUV formats(which seem unrelated)?

Would it be ok to have just 422P and greyscale in the driver for now if we can't get a firmware update in a reasonable timeframe? It's certainly not ideal, but we could direct our customers to the libraries, at least until we get a proper HW fix.

Hi Oliver,

Just to be clear. You mean fixing the loading issues(ugly msleep, etc...) that Mauro mentioned, not the YUV formats(which seem unrelated)?

Would it be ok to have just 422P and greyscale in the driver for now if we can't get a firmware update in a reasonable timeframe? It's certainly not ideal, but we could direct our customers to the libraries, at least until we get a proper HW fix.

From my side, I'm ok on having this color conversion for a short timeframe (one

Btw, I noticed the lack of Dean's SOB. Is this intentional?

Quoted 1 line

Btw, I noticed the lack of Dean's SOB. Is this intentional?

It's not intentional, I can sign off on it.

I have a few other questions. First, is Video for Linux version 1 going to be obsoleted soon?

Do the V4L1 compatibility routines still work in
the latest driver?

I had problems running the VIVI (virtual video
driver) driver with VideoLan/VLC 0.8.6a-f, but it worked with VLC 9.0 with the new V4L2 interface.

"videodev: "s2255v" has no release callback. Please fix your driver for proper sysfs support, see http://lwn.net/Articles/36850/"

Should we get rid of the warning message above? It's also been present in VIVI for quite a few kernel releases.