public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* Re: CEU / Camera Driver Question
       [not found]   ` <Pine.LNX.4.64.1004080814370.4621@axis700.grange>
@ 2010-04-14  1:54     ` Charles D. Krebs
  2010-04-14  6:38       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 8+ messages in thread
From: Charles D. Krebs @ 2010-04-14  1:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

[-- Attachment #1: Type: text/plain, Size: 6850 bytes --]

Guennadi,

Thank you for the response and explanation.  It falls perfectly in line with 
what we had been suspecting on our end.

We ended up basing the driver off "mt9t112.c", which is an I2C camera.  The 
major issues have been figuring out how to remove the I2C components.

The driver (attached for reference) currently registers a device under 
"/sys/bus/platform/drivers/testcam".  However, udev does not populate a 
"video0" entry under "/dev", so it seems the V4L2 registration wasn't done 
correctly.

I'm fairly sure the problem falls under "testcam_probe" or 
"testcam_module_init".

Since we are not I2C, should we call "platform_driver_register" from 
"testcam_module_init"?

Do we need to fill out a link structure from the SOC Camera driver 
(soc_camera_link)?  I noticed that this is used in all the I2C cameras.

Unfortunately, I still need to figure out how to best integrate with the 
sh_mobile_ceu_camera driver since I am mid migration from 2.6.31-rc7 to 
2.6.33.  It appears that quite a lot has changed...  The Kernel change has 
spawned a plethora of issues, which has unfortunately delayed development on 
this driver until now.

Thanks for your input!

Charles Krebs, Embedded Solutions Developer
The Realtime Group

--------------------------------------------------
From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
Sent: Thursday, April 08, 2010 1:39 AM
To: "Magnus Damm" <magnus.damm@gmail.com>
Cc: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
Subject: Re: CEU / Camera Driver Question

> Hi Charles
>
> On Thu, 8 Apr 2010, Magnus Damm wrote:
>
>> Hi Charles,
>>
>> Thanks for your email. I am afraid I know too little about the current
>> status of the CEU driver and camera sensor integration. I do however know
>> one guy that can help you.
>>
>> Guennadi, can you please give us some recommendations? Charles is using
>> 2.6.33 on sh7724, see below.
>>
>> Thanks!
>>
>> / magnus
>>
>> On Apr 6, 2010 10:35 AM, "Charles D. Krebs" <ckrebs@therealtimegroup.com>
>> wrote:
>>
>>  Magnus,
>>
>> We have been working on integrating our camera into the 7724 platform.  I
>> think we are pretty close to having the camera up and working at this 
>> point,
>> but there are a few outstanding concerns.
>
> In the open-source community it is customary to discuss related topics and
> ask questions on respective mailing lists. So, I'll just give a very brief
> answer to this your mail, if you get more questions, please CC
>
> Linux Media Mailing List <linux-media@vger.kernel.org>
>
> in your naxt mail.
>
>> The basic objective is to interface a very dumb video camera that 
>> connects
>> directly to CEU driver in the SH7724 processor.  This camera needs no
>> control interface (the interface is actually RS232, which I plan to drive
>> completely from user space), but has 8 bit parallel video (Grayscale). 
>> The
>> camera driver was patterned after the the soc_camera driver, using the
>> platform interface.
>> Our camera driver is mostly dummy code because of the simplicity.
>
> The current Linux kernel mainline implementation of the video capture
> function on several embedded SoCs, including CEU-based SuperH platforms,
> is a V4L2 driver stack, consisting of
>
> 1. a host driver (in this case sh_mobile_ceu_camera.c), using the
>   soc-camera API to integrate itself into the V4L framework
> 2. the soc-camera core
> 3. client drivers, using the v4l2-subdev API for most V4L2 communication,
>   the mediabus API for pixel-format negotiation and a couple of
>   soc-camera API extensions.
>
> So, all you need is use the existing sh_mobile_ceu_camera.c driver, the
> soc-camera framework and add a new driver for your camera-sensor, which in
> your case would be very simple, as you say. Just use any platform,
> currently in the mainline (e.g., ecovec) as an example for your platform
> bindings, and any soc-camera client driver (e.g., mt9m001, or ov772x) as a
> template for your camera driver.
>
> There is one point, where you will have to be careful: your camera is not
> using I2C. soc-camera should support this too, but it hasn't been tested
> or used for a while, so, something might have bitrotted there.
>
> So, I would suggest - write a driver, test it and post to the mailing list
> (you can CC me too, if you like). If you have any questions in the
> meantime - don't hesitate to ask, but please cc the list. Regarding your
> intension to control the sensor from the user-space, however simple that
> controlling might be, I would seriously consider writing a line discipline
> for it, which would allow you then use any standard V4L(2) application
> with your system. The only addition you would have is a tiny app, that
> would open the serial port, set the required line discipline for it and
> keep it open for the whole time your video driver is going to be used.
>
> Thanks
> Guennadi
>
>>
>> Questions:
>>
>> 1. Is soc_camera a reasonable driver to use as a starting point, or is
>> there a better choice?
>>
>> 2. How is the CEU driver associated with the camera driver?
>>
>> 3. Is there a special bus type ID that needs to be claimed by the camera
>> driver?  Standard or custom?
>>
>> 4.  In /arch/sh/boards/mach-ecovec24/setup.c -
>>
>> I made quite a few modifications.  Pertaining to the new "testcam" 
>> device, I
>> have:
>>
>> static struct platform_device camera_devices[] = {
>>  {
>>   //.name = "soc-camera-pdrv",
>>   .name = "testcam",
>>   .id = 0,
>>   .dev = {
>>    .platform_data = &testcam_info2,
>>   },
>>  },
>> static struct testcam_camera_info testcam_info2 = {
>>  .flags = 0,
>>  .bus_param = 1,
>>  };
>> The connection from here to our camera driver appears to depend on the
>> "name" field of the platform_device structure:
>>
>> static struct platform_driver testcam_driver =
>> {
>>  .driver       = {
>>   .name = "testcam",
>>  },
>>  .probe        = testcam_probe,
>>  .remove       = testcam_remove,
>> };
>> In the "mt9t112" driver, it uses the "soc-camera-pdrv".  Should I have
>> emulated other functions from the SOC Camera driver such as the link 
>> field
>> to get the device to connect?  soc_camera_device_register in still called 
>> in
>> our driver's probe function, and in that way, the driver ends up being 
>> more
>> like "mx3_camera.c"
>>
>> Using the platform driver, the device registers
>> in "/sys/bus/platform/drivers/testcam".  However, udev does not populate 
>> a
>> "video0" entry under "/dev".  What is special about the "mt9t112" driver
>> that allows such a registration to take place?
>>
>> Any other insight regarding how the existing demo drivers were 
>> architected
>> would be extremely helpful.
>>
>> Thank you,
>>
>> Charles Krebs, Embedded Solutions Developer
>> The Realtime Group
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

[-- Attachment #2: testcam.c --]
[-- Type: text/plain, Size: 11769 bytes --]

/*
 * testcam Camera Driver
 *
 * Copyright (C) 2010
 * Based on the Generic Platform Camera Driver
 * Copyright (C) 2010
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 2 as
 * published by the Free Software Foundation.
 */

#include <linux/init.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/platform_device.h>
#include <linux/videodev2.h>
#include <media/v4l2-common.h>
#include <media/v4l2-chip-ident.h>
#include <media/soc_camera.h>
#include <media/testcam.h>

/*
 * max frame size
 */
#define MAX_WIDTH   320
#define MAX_HEIGHT  240

/* flags for testcam_priv :: flags */
#define INIT_DONE  (1<<0)


struct testcam_priv
{
	struct testcam_camera_info       *info;
	struct soc_camera_device               icd;
	int                                    model;
};


/* ------------------------------------------------------------------------
*/
/*
 * supported format list (only one supported entry, for 8 bit greyscale)
 */
/* ------------------------------------------------------------------------
*/
#define COL_FMT(_name, _depth, _fourcc, _colorspace)		\
	{ .name = _name, .depth = _depth, .fourcc = _fourcc,	\
			.colorspace = _colorspace }

#define RGB_FMT(_name, _depth, _fourcc)				\
	COL_FMT(_name, _depth, V4L2_PIX_FMT_ ## _fourcc, V4L2_COLORSPACE_SRGB)

static const struct soc_camera_data_format testcam_fmt_lists [] =
{
	RGB_FMT("Monochrome 8 bit", 8, NV12)
//	RGB_FMT("Monochrome 8 bit", 8, GREY)	// Changed to repurpose NV12
};

/* ------------------------------------------------------------------------
*/
/*
 * general functions
 */
/* ------------------------------------------------------------------------
*/
static inline struct testcam_camera_info *
soc_camera_platform_get_info(struct soc_camera_device *icd)
{
	struct testcam_priv *priv;
	priv = container_of(icd, struct testcam_priv, icd);
	return priv->info;
}

/* ------------------------------------------------------------------------
*/
static int testcam_check_frame(u16 width, u16 height)
{
	return ((height != MAX_HEIGHT)  ||  (width != MAX_WIDTH));
}

/* ------------------------------------------------------------------------
*/
static int testcam_get_chipID(struct soc_camera_device *icd)
{
	return 0x1234;
}

/* ------------------------------------------------------------------------
*/
static int testcam_reset(struct soc_camera_device *icd)
{
	return (0);
}

/* ------------------------------------------------------------------------
*/
/*
 * soc_camera_ops functions
 */
/* ------------------------------------------------------------------------
*/
static int testcam_init(struct soc_camera_device *icd)
{
	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
	int ret = 0;

//	if (info->power)
//		ret = info->power(1);

	return ret;
}

/* ------------------------------------------------------------------------
*/
static int testcam_release(struct soc_camera_device *icd)
{
	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);

//	if (info->power)
//		info->power(0);

	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_start_capture(struct soc_camera_device *icd)
{
	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);

	if (!(info->flags & INIT_DONE)) {
		dev_err(&icd->dev, "device init isn't done\n");
		return -EINVAL;
	}

	dev_info(&icd->dev, "size   : QVGA (%d x %d)\n", MAX_WIDTH, MAX_HEIGHT);

	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_stop_capture(struct soc_camera_device *icd)
{
//	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
// not supported; only single frame capture is supported
	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_bus_param(struct soc_camera_device *icd,
				      unsigned long		flags)
{
//	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
// nothing to do here
	return 0;
}

/* ------------------------------------------------------------------------
*/
static unsigned long testcam_query_bus_param(struct soc_camera_device *icd)
{
	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
	unsigned long flags = SOCAM_MASTER | SOCAM_VSYNC_ACTIVE_HIGH |
			      SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_DATA_ACTIVE_HIGH |
			      SOCAM_DATAWIDTH_8;

	info->flags = (info->flags & INIT_DONE) | flags;

	return info->bus_param;
}

/* ------------------------------------------------------------------------
*/
static int testcam_get_control(struct soc_camera_device *icd,
			      struct v4l2_control *ctrl)
{
//	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
// not supported
	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_control(struct soc_camera_device *icd,
			      struct v4l2_control *ctrl)
{
//	struct testcam_camera_info *info = soc_camera_platform_get_info(icd);
// not supported
	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_get_chip_id(struct soc_camera_device *icd,
			      struct v4l2_dbg_chip_ident   *id)
{
	struct testcam_priv *priv = container_of(icd, struct testcam_priv, icd);

	id->ident    = priv->model;
	id->revision = 0;

	return 0;
}

/* ------------------------------------------------------------------------
*/
#ifdef CONFIG_VIDEO_ADV_DEBUG
static int testcam_get_register(struct soc_camera_device *icd,
			       struct v4l2_dbg_register *reg)
{
// not supported
	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_register(struct soc_camera_device *icd,
			       struct v4l2_dbg_register *reg)
{
// not supported
	return testcam_reg_write(icd, reg->reg, reg->val);
}
#endif
/* ------------------------------------------------------------------------
*/
static int testcam_init_camera(struct soc_camera_device *icd)
{
	int ret = testcam_reset(icd);

	return ret;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_params(struct soc_camera_device *icd,
			      u32 width, u32 height, u32 pixfmt)
{
	struct testcam_priv *priv = container_of(icd, struct testcam_priv, icd);
	int ret;

	/*
	 * check frame size
	 */
	if (testcam_check_frame(width, height))
		return -EINVAL;

	/*
	 * testcam should init in 1st time.
	 */
	if (!(priv->info->flags & INIT_DONE)) {
		if (!(ret = testcam_init_camera(icd)))
			return ret;
		priv->info->flags |= INIT_DONE;
	}

	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_crop(struct soc_camera_device *icd,
				 struct v4l2_rect *rect)
{
// not supported
	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_set_fmt(struct soc_camera_device *icd,
			  struct v4l2_format *f)
{
	struct v4l2_pix_format *pix = &f->fmt.pix;

	return testcam_set_params(icd, pix->width, pix->height,
				 pix->pixelformat);
}

/* ------------------------------------------------------------------------
*/
static int testcam_try_fmt(struct soc_camera_device  *icd,
				struct v4l2_format        *f)
{
	struct v4l2_pix_format  *pix = &f->fmt.pix;

	/*
	 * check frame size
	 */
	if (testcam_check_frame(pix->width, pix->height))
		return -EINVAL;

	pix->field = V4L2_FIELD_NONE;

	return 0;
}

/* ------------------------------------------------------------------------
*/
static int testcam_camera_probe(struct soc_camera_device *icd)
{
	struct testcam_priv *priv = container_of(icd, struct testcam_priv, icd);
	const char               *devname;
	int                       chipid;

	/*
	 * check and show chip ID
	 */

	chipid		 = testcam_get_chipID(icd);
	devname		 = "testcam";
	priv->model	 = V4L2_IDENT_UNKNOWN;
	icd->formats     = testcam_fmt_lists;
	icd->num_formats = ARRAY_SIZE(testcam_fmt_lists);

	dev_info(&icd->dev,
		 "%s chip ID %04x\n", devname, chipid);

	return soc_camera_video_start(icd);
}

/* ------------------------------------------------------------------------
*/
static void testcam_camera_remove(struct soc_camera_device *icd)
{
	soc_camera_video_stop(icd);
}

/* ------------------------------------------------------------------------
*/
static struct soc_camera_ops testcam_ops =
{
	.owner			= THIS_MODULE,
	.probe			= testcam_camera_probe,
	.remove			= testcam_camera_remove,
	.init			= testcam_init,
	.release		= testcam_release,
	.start_capture		= testcam_start_capture,
	.stop_capture		= testcam_stop_capture,
	.set_crop		= testcam_set_crop,
	.set_fmt		= testcam_set_fmt,
	.try_fmt		= testcam_try_fmt,
	.set_bus_param		= testcam_set_bus_param,
	.query_bus_param	= testcam_query_bus_param,
	.get_control		= testcam_get_control,
	.set_control		= testcam_set_control,
	.get_chip_id		= testcam_get_chip_id,
#ifdef CONFIG_VIDEO_ADV_DEBUG
	.get_register		= testcam_get_register,
	.set_register		= testcam_set_register,
#endif
};

/* ------------------------------------------------------------------------
*/
/*
 * platform functions
 */
/* ------------------------------------------------------------------------
*/
static int testcam_probe(struct platform_device *pdev)
{
	struct testcam_priv        *priv;
	struct testcam_camera_info *info;
	struct soc_camera_device        *icd;
	int                              ret;

	info = pdev->dev.platform_data;
	if (!info)
		return -EINVAL;

	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return -ENOMEM;

	priv->info = info;
	platform_set_drvdata(pdev, priv);

	icd             = &priv->icd;
	icd->ops	= &testcam_ops;
	icd->control	= &pdev->dev;
	icd->width_min	= MAX_WIDTH;
	icd->width_max	= MAX_WIDTH;
	icd->height_min	= MAX_HEIGHT;
	icd->height_max	= MAX_HEIGHT;
	icd->y_skip_top	= 0;
	icd->iface	= priv->info->iface;

	ret = soc_camera_device_register(icd);
	if (ret)
		kfree(priv);

	return ret;
}

/* ------------------------------------------------------------------------
*/
static int testcam_remove(struct platform_device *pdev)
{
	struct testcam_priv *priv = platform_get_drvdata(pdev);

	soc_camera_device_unregister(&priv->icd);
	kfree(priv);
	return 0;
}

/* ------------------------------------------------------------------------
*/
static struct platform_driver testcam_driver =
{
	.driver       = {
		.name = "testcam",
	},
	.probe        = testcam_probe,
	.remove       = testcam_remove,
};
/* ------------------------------------------------------------------------
*/
/*
 * module functions
 */
/* ------------------------------------------------------------------------
*/
static int __init testcam_module_init(void)
{
	return platform_driver_register(&testcam_driver);
}

/* ------------------------------------------------------------------------
*/
static void __exit testcam_module_exit(void)
{
	platform_driver_unregister(&testcam_driver);
}

/* ------------------------------------------------------------------------
*/
module_init(testcam_module_init);
module_exit(testcam_module_exit);

MODULE_DESCRIPTION("SoC Camera driver for testcam");
MODULE_AUTHOR("CK");
MODULE_LICENSE("GPL v2");

[-- Attachment #3: testcam.h --]
[-- Type: text/plain, Size: 582 bytes --]

/*
 * TestCam Camera Driver Header
 *
 * Copyright (C) 2010
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 2 as
 * published by the Free Software Foundation.
 */

#ifndef __TESTCAM_H__
#define __TESTCAM_H__

#include <linux/videodev2.h>

/*
 * lanternshark camera info
 */
struct lanternshark_camera_info {
	u32                     flags;
	int                     iface;
	unsigned long           bus_param;
	struct soc_camera_link link;

#endif /* __TESTCAM_H__ */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-04-14  1:54     ` CEU / Camera Driver Question Charles D. Krebs
@ 2010-04-14  6:38       ` Guennadi Liakhovetski
  2010-05-04  2:26         ` Charles D. Krebs
  0 siblings, 1 reply; 8+ messages in thread
From: Guennadi Liakhovetski @ 2010-04-14  6:38 UTC (permalink / raw)
  To: Charles D. Krebs; +Cc: Linux Media Mailing List

Hi Charles

On Tue, 13 Apr 2010, Charles D. Krebs wrote:

> Guennadi,
> 
> Thank you for the response and explanation.  It falls perfectly in line with
> what we had been suspecting on our end.
> 
> We ended up basing the driver off "mt9t112.c", which is an I2C camera.  The
> major issues have been figuring out how to remove the I2C components.
> 
> The driver (attached for reference) currently registers a device under
> "/sys/bus/platform/drivers/testcam".  However, udev does not populate a
> "video0" entry under "/dev", so it seems the V4L2 registration wasn't done
> correctly.

All my comments will base on the current kernel, so, if you prefer to stay 
with an older one, they will not be (entirely) applicable.

> I'm fairly sure the problem falls under "testcam_probe" or
> "testcam_module_init".
> 
> Since we are not I2C, should we call "platform_driver_register" from
> "testcam_module_init"?
> 
> Do we need to fill out a link structure from the SOC Camera driver
> (soc_camera_link)?  I noticed that this is used in all the I2C cameras.

As I see, your driver is just a dummy, that only specifies camera's 
capabilities. That's also ok, you certainly can make a driver for a 
completely fixed-parameter camera, but then a few methods from your driver 
should either disappear, or return an "unsupported" error, or return the 
fixed configuration of the sensor (like s_fmt, try_fmt). Please, have a 
look at drivers/media/video/soc_camera_platform.c, that's an example of an 
soc-camera client driver, not using i2c. I'm not sure if it's working 
presently, it's use kind of discouraged, but you can certainly use it as 
an example. If you don't plan to mainline your driver, you can even 
actually use soc_camera_platform.c, then you'll just need to add some 
platform data for it (see arch/sh/boards/mach-ap325rxa/setup.c and struct 
soc_camera_link camera_link in it for an example). You might have to fix 
that driver slightly, but that shouldn't be too difficult.

Thanks
Guennadi

> 
> Unfortunately, I still need to figure out how to best integrate with the
> sh_mobile_ceu_camera driver since I am mid migration from 2.6.31-rc7 to
> 2.6.33.  It appears that quite a lot has changed...  The Kernel change has
> spawned a plethora of issues, which has unfortunately delayed development on
> this driver until now.
> 
> Thanks for your input!
> 
> Charles Krebs, Embedded Solutions Developer
> The Realtime Group
> 
> --------------------------------------------------
> From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
> Sent: Thursday, April 08, 2010 1:39 AM
> To: "Magnus Damm" <magnus.damm@gmail.com>
> Cc: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
> Subject: Re: CEU / Camera Driver Question
> 
> > Hi Charles
> > 
> > On Thu, 8 Apr 2010, Magnus Damm wrote:
> > 
> > > Hi Charles,
> > > 
> > > Thanks for your email. I am afraid I know too little about the current
> > > status of the CEU driver and camera sensor integration. I do however know
> > > one guy that can help you.
> > > 
> > > Guennadi, can you please give us some recommendations? Charles is using
> > > 2.6.33 on sh7724, see below.
> > > 
> > > Thanks!
> > > 
> > > / magnus
> > > 
> > > On Apr 6, 2010 10:35 AM, "Charles D. Krebs" <ckrebs@therealtimegroup.com>
> > > wrote:
> > > 
> > >  Magnus,
> > > 
> > > We have been working on integrating our camera into the 7724 platform.  I
> > > think we are pretty close to having the camera up and working at this
> > > point,
> > > but there are a few outstanding concerns.
> > 
> > In the open-source community it is customary to discuss related topics and
> > ask questions on respective mailing lists. So, I'll just give a very brief
> > answer to this your mail, if you get more questions, please CC
> > 
> > Linux Media Mailing List <linux-media@vger.kernel.org>
> > 
> > in your naxt mail.
> > 
> > > The basic objective is to interface a very dumb video camera that connects
> > > directly to CEU driver in the SH7724 processor.  This camera needs no
> > > control interface (the interface is actually RS232, which I plan to drive
> > > completely from user space), but has 8 bit parallel video (Grayscale). The
> > > camera driver was patterned after the the soc_camera driver, using the
> > > platform interface.
> > > Our camera driver is mostly dummy code because of the simplicity.
> > 
> > The current Linux kernel mainline implementation of the video capture
> > function on several embedded SoCs, including CEU-based SuperH platforms,
> > is a V4L2 driver stack, consisting of
> > 
> > 1. a host driver (in this case sh_mobile_ceu_camera.c), using the
> >   soc-camera API to integrate itself into the V4L framework
> > 2. the soc-camera core
> > 3. client drivers, using the v4l2-subdev API for most V4L2 communication,
> >   the mediabus API for pixel-format negotiation and a couple of
> >   soc-camera API extensions.
> > 
> > So, all you need is use the existing sh_mobile_ceu_camera.c driver, the
> > soc-camera framework and add a new driver for your camera-sensor, which in
> > your case would be very simple, as you say. Just use any platform,
> > currently in the mainline (e.g., ecovec) as an example for your platform
> > bindings, and any soc-camera client driver (e.g., mt9m001, or ov772x) as a
> > template for your camera driver.
> > 
> > There is one point, where you will have to be careful: your camera is not
> > using I2C. soc-camera should support this too, but it hasn't been tested
> > or used for a while, so, something might have bitrotted there.
> > 
> > So, I would suggest - write a driver, test it and post to the mailing list
> > (you can CC me too, if you like). If you have any questions in the
> > meantime - don't hesitate to ask, but please cc the list. Regarding your
> > intension to control the sensor from the user-space, however simple that
> > controlling might be, I would seriously consider writing a line discipline
> > for it, which would allow you then use any standard V4L(2) application
> > with your system. The only addition you would have is a tiny app, that
> > would open the serial port, set the required line discipline for it and
> > keep it open for the whole time your video driver is going to be used.
> > 
> > Thanks
> > Guennadi
> > 
> > > 
> > > Questions:
> > > 
> > > 1. Is soc_camera a reasonable driver to use as a starting point, or is
> > > there a better choice?
> > > 
> > > 2. How is the CEU driver associated with the camera driver?
> > > 
> > > 3. Is there a special bus type ID that needs to be claimed by the camera
> > > driver?  Standard or custom?
> > > 
> > > 4.  In /arch/sh/boards/mach-ecovec24/setup.c -
> > > 
> > > I made quite a few modifications.  Pertaining to the new "testcam" device,
> > > I
> > > have:
> > > 
> > > static struct platform_device camera_devices[] = {
> > >  {
> > >   //.name = "soc-camera-pdrv",
> > >   .name = "testcam",
> > >   .id = 0,
> > >   .dev = {
> > >    .platform_data = &testcam_info2,
> > >   },
> > >  },
> > > static struct testcam_camera_info testcam_info2 = {
> > >  .flags = 0,
> > >  .bus_param = 1,
> > >  };
> > > The connection from here to our camera driver appears to depend on the
> > > "name" field of the platform_device structure:
> > > 
> > > static struct platform_driver testcam_driver =
> > > {
> > >  .driver       = {
> > >   .name = "testcam",
> > >  },
> > >  .probe        = testcam_probe,
> > >  .remove       = testcam_remove,
> > > };
> > > In the "mt9t112" driver, it uses the "soc-camera-pdrv".  Should I have
> > > emulated other functions from the SOC Camera driver such as the link field
> > > to get the device to connect?  soc_camera_device_register in still called
> > > in
> > > our driver's probe function, and in that way, the driver ends up being
> > > more
> > > like "mx3_camera.c"
> > > 
> > > Using the platform driver, the device registers
> > > in "/sys/bus/platform/drivers/testcam".  However, udev does not populate a
> > > "video0" entry under "/dev".  What is special about the "mt9t112" driver
> > > that allows such a registration to take place?
> > > 
> > > Any other insight regarding how the existing demo drivers were architected
> > > would be extremely helpful.
> > > 
> > > Thank you,
> > > 
> > > Charles Krebs, Embedded Solutions Developer
> > > The Realtime Group
> > > 
> > 
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-04-14  6:38       ` Guennadi Liakhovetski
@ 2010-05-04  2:26         ` Charles D. Krebs
  2010-05-04  6:43           ` Guennadi Liakhovetski
  0 siblings, 1 reply; 8+ messages in thread
From: Charles D. Krebs @ 2010-05-04  2:26 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Linux Media Mailing List, Magnus Damm

Guennadi,

As per your recommendation I reviewed the "soc_camera_platform" driver, and 
have chosen to implement the new camera by simply modifying it.

Sure enough, I can boot up and properly register a device under /dev/video0.

The camera provides 8-bit Grayscale data corresponding to pixel format 
V4L2_PIX_FMT_GREY.  I can't seem to find any example of a device driver that 
uses this format, so I've been taking my best guess as how to setup 
"soc_camera_platform_info".  So far I have:

static struct soc_camera_platform_info mycam_camera_info = {
	.format_name = "GREY",
	.format_depth = 8,
	.format = {
		.code = V4L2_MBUS_FMT_YUYV8_2X8_BE,
		.colorspace = V4L2_COLORSPACE_JPEG,
		.field = V4L2_FIELD_NONE,
		.width = 320,
		.height = 240,
	},
	.bus_param = SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8 |
	SOCAM_DATA_ACTIVE_HIGH,
};

It looks like I'll need to modify "soc_camera_platform" it in a way to at 
least add a "fourcc" field that can be interpreted by the ceu driver for the 
"sh_mobile_ceu_set_bus_param" call to setup the hardware correctly.

But regardless of how I set this structure up, I don't see any direct 
support for a Grayscale mode data capture in the ceu driver.  For example, 
"sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in its list 
of fourcc formats.  Yet based on the 7724 hardware manual, and from the 
information I have received from Renesas, I'm not seeing any reason why this 
format should not be supported.

Is grayscale somehow supported under the current CEU driver?

Any suggestions on how I might go about implementing support?  I'm having 
trouble seeing the dataflow through the driver at the moment...

Thank you!

Charles Krebs, Embedded Solutions Developer
The Realtime Group

--------------------------------------------------
From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
Sent: Wednesday, April 14, 2010 1:38 AM
To: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>
Subject: Re: CEU / Camera Driver Question

> Hi Charles
>
> On Tue, 13 Apr 2010, Charles D. Krebs wrote:
>
>> Guennadi,
>>
>> Thank you for the response and explanation.  It falls perfectly in line 
>> with
>> what we had been suspecting on our end.
>>
>> We ended up basing the driver off "mt9t112.c", which is an I2C camera. 
>> The
>> major issues have been figuring out how to remove the I2C components.
>>
>> The driver (attached for reference) currently registers a device under
>> "/sys/bus/platform/drivers/testcam".  However, udev does not populate a
>> "video0" entry under "/dev", so it seems the V4L2 registration wasn't 
>> done
>> correctly.
>
> All my comments will base on the current kernel, so, if you prefer to stay
> with an older one, they will not be (entirely) applicable.
>
>> I'm fairly sure the problem falls under "testcam_probe" or
>> "testcam_module_init".
>>
>> Since we are not I2C, should we call "platform_driver_register" from
>> "testcam_module_init"?
>>
>> Do we need to fill out a link structure from the SOC Camera driver
>> (soc_camera_link)?  I noticed that this is used in all the I2C cameras.
>
> As I see, your driver is just a dummy, that only specifies camera's
> capabilities. That's also ok, you certainly can make a driver for a
> completely fixed-parameter camera, but then a few methods from your driver
> should either disappear, or return an "unsupported" error, or return the
> fixed configuration of the sensor (like s_fmt, try_fmt). Please, have a
> look at drivers/media/video/soc_camera_platform.c, that's an example of an
> soc-camera client driver, not using i2c. I'm not sure if it's working
> presently, it's use kind of discouraged, but you can certainly use it as
> an example. If you don't plan to mainline your driver, you can even
> actually use soc_camera_platform.c, then you'll just need to add some
> platform data for it (see arch/sh/boards/mach-ap325rxa/setup.c and struct
> soc_camera_link camera_link in it for an example). You might have to fix
> that driver slightly, but that shouldn't be too difficult.
>
> Thanks
> Guennadi
>
>>
>> Unfortunately, I still need to figure out how to best integrate with the
>> sh_mobile_ceu_camera driver since I am mid migration from 2.6.31-rc7 to
>> 2.6.33.  It appears that quite a lot has changed...  The Kernel change 
>> has
>> spawned a plethora of issues, which has unfortunately delayed development 
>> on
>> this driver until now.
>>
>> Thanks for your input!
>>
>> Charles Krebs, Embedded Solutions Developer
>> The Realtime Group
>>
>> --------------------------------------------------
>> From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
>> Sent: Thursday, April 08, 2010 1:39 AM
>> To: "Magnus Damm" <magnus.damm@gmail.com>
>> Cc: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
>> Subject: Re: CEU / Camera Driver Question
>>
>> > Hi Charles
>> >
>> > On Thu, 8 Apr 2010, Magnus Damm wrote:
>> >
>> > > Hi Charles,
>> > >
>> > > Thanks for your email. I am afraid I know too little about the 
>> > > current
>> > > status of the CEU driver and camera sensor integration. I do however 
>> > > know
>> > > one guy that can help you.
>> > >
>> > > Guennadi, can you please give us some recommendations? Charles is 
>> > > using
>> > > 2.6.33 on sh7724, see below.
>> > >
>> > > Thanks!
>> > >
>> > > / magnus
>> > >
>> > > On Apr 6, 2010 10:35 AM, "Charles D. Krebs" 
>> > > <ckrebs@therealtimegroup.com>
>> > > wrote:
>> > >
>> > >  Magnus,
>> > >
>> > > We have been working on integrating our camera into the 7724 
>> > > platform.  I
>> > > think we are pretty close to having the camera up and working at this
>> > > point,
>> > > but there are a few outstanding concerns.
>> >
>> > In the open-source community it is customary to discuss related topics 
>> > and
>> > ask questions on respective mailing lists. So, I'll just give a very 
>> > brief
>> > answer to this your mail, if you get more questions, please CC
>> >
>> > Linux Media Mailing List <linux-media@vger.kernel.org>
>> >
>> > in your naxt mail.
>> >
>> > > The basic objective is to interface a very dumb video camera that 
>> > > connects
>> > > directly to CEU driver in the SH7724 processor.  This camera needs no
>> > > control interface (the interface is actually RS232, which I plan to 
>> > > drive
>> > > completely from user space), but has 8 bit parallel video 
>> > > (Grayscale). The
>> > > camera driver was patterned after the the soc_camera driver, using 
>> > > the
>> > > platform interface.
>> > > Our camera driver is mostly dummy code because of the simplicity.
>> >
>> > The current Linux kernel mainline implementation of the video capture
>> > function on several embedded SoCs, including CEU-based SuperH 
>> > platforms,
>> > is a V4L2 driver stack, consisting of
>> >
>> > 1. a host driver (in this case sh_mobile_ceu_camera.c), using the
>> >   soc-camera API to integrate itself into the V4L framework
>> > 2. the soc-camera core
>> > 3. client drivers, using the v4l2-subdev API for most V4L2 
>> > communication,
>> >   the mediabus API for pixel-format negotiation and a couple of
>> >   soc-camera API extensions.
>> >
>> > So, all you need is use the existing sh_mobile_ceu_camera.c driver, the
>> > soc-camera framework and add a new driver for your camera-sensor, which 
>> > in
>> > your case would be very simple, as you say. Just use any platform,
>> > currently in the mainline (e.g., ecovec) as an example for your 
>> > platform
>> > bindings, and any soc-camera client driver (e.g., mt9m001, or ov772x) 
>> > as a
>> > template for your camera driver.
>> >
>> > There is one point, where you will have to be careful: your camera is 
>> > not
>> > using I2C. soc-camera should support this too, but it hasn't been 
>> > tested
>> > or used for a while, so, something might have bitrotted there.
>> >
>> > So, I would suggest - write a driver, test it and post to the mailing 
>> > list
>> > (you can CC me too, if you like). If you have any questions in the
>> > meantime - don't hesitate to ask, but please cc the list. Regarding 
>> > your
>> > intension to control the sensor from the user-space, however simple 
>> > that
>> > controlling might be, I would seriously consider writing a line 
>> > discipline
>> > for it, which would allow you then use any standard V4L(2) application
>> > with your system. The only addition you would have is a tiny app, that
>> > would open the serial port, set the required line discipline for it and
>> > keep it open for the whole time your video driver is going to be used.
>> >
>> > Thanks
>> > Guennadi
>> >
>> > >
>> > > Questions:
>> > >
>> > > 1. Is soc_camera a reasonable driver to use as a starting point, or 
>> > > is
>> > > there a better choice?
>> > >
>> > > 2. How is the CEU driver associated with the camera driver?
>> > >
>> > > 3. Is there a special bus type ID that needs to be claimed by the 
>> > > camera
>> > > driver?  Standard or custom?
>> > >
>> > > 4.  In /arch/sh/boards/mach-ecovec24/setup.c -
>> > >
>> > > I made quite a few modifications.  Pertaining to the new "testcam" 
>> > > device,
>> > > I
>> > > have:
>> > >
>> > > static struct platform_device camera_devices[] = {
>> > >  {
>> > >   //.name = "soc-camera-pdrv",
>> > >   .name = "testcam",
>> > >   .id = 0,
>> > >   .dev = {
>> > >    .platform_data = &testcam_info2,
>> > >   },
>> > >  },
>> > > static struct testcam_camera_info testcam_info2 = {
>> > >  .flags = 0,
>> > >  .bus_param = 1,
>> > >  };
>> > > The connection from here to our camera driver appears to depend on 
>> > > the
>> > > "name" field of the platform_device structure:
>> > >
>> > > static struct platform_driver testcam_driver =
>> > > {
>> > >  .driver       = {
>> > >   .name = "testcam",
>> > >  },
>> > >  .probe        = testcam_probe,
>> > >  .remove       = testcam_remove,
>> > > };
>> > > In the "mt9t112" driver, it uses the "soc-camera-pdrv".  Should I 
>> > > have
>> > > emulated other functions from the SOC Camera driver such as the link 
>> > > field
>> > > to get the device to connect?  soc_camera_device_register in still 
>> > > called
>> > > in
>> > > our driver's probe function, and in that way, the driver ends up 
>> > > being
>> > > more
>> > > like "mx3_camera.c"
>> > >
>> > > Using the platform driver, the device registers
>> > > in "/sys/bus/platform/drivers/testcam".  However, udev does not 
>> > > populate a
>> > > "video0" entry under "/dev".  What is special about the "mt9t112" 
>> > > driver
>> > > that allows such a registration to take place?
>> > >
>> > > Any other insight regarding how the existing demo drivers were 
>> > > architected
>> > > would be extremely helpful.
>> > >
>> > > Thank you,
>> > >
>> > > Charles Krebs, Embedded Solutions Developer
>> > > The Realtime Group
>> > >
>> >
>> > ---
>> > Guennadi Liakhovetski, Ph.D.
>> > Freelance Open-Source Software Developer
>> > http://www.open-technology.de/
>> >
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-05-04  2:26         ` Charles D. Krebs
@ 2010-05-04  6:43           ` Guennadi Liakhovetski
  2010-05-14 20:10             ` Charles D. Krebs
  0 siblings, 1 reply; 8+ messages in thread
From: Guennadi Liakhovetski @ 2010-05-04  6:43 UTC (permalink / raw)
  To: Charles D. Krebs; +Cc: Linux Media Mailing List, Magnus Damm

Hi Charles

On Mon, 3 May 2010, Charles D. Krebs wrote:

> Guennadi,
> 
> As per your recommendation I reviewed the "soc_camera_platform" driver, and
> have chosen to implement the new camera by simply modifying it.
> 
> Sure enough, I can boot up and properly register a device under /dev/video0.
> 
> The camera provides 8-bit Grayscale data corresponding to pixel format
> V4L2_PIX_FMT_GREY.  I can't seem to find any example of a device driver that
> uses this format, so I've been taking my best guess as how to setup
> "soc_camera_platform_info".  So far I have:
> 
> static struct soc_camera_platform_info mycam_camera_info = {
> 	.format_name = "GREY",
> 	.format_depth = 8,
> 	.format = {
> 		.code = V4L2_MBUS_FMT_YUYV8_2X8_BE,

No, you should be using V4L2_MBUS_FMT_GREY8_1X8 for grey.

> 		.colorspace = V4L2_COLORSPACE_JPEG,
> 		.field = V4L2_FIELD_NONE,
> 		.width = 320,
> 		.height = 240,
> 	},
> 	.bus_param = SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
> 	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8 |
> 	SOCAM_DATA_ACTIVE_HIGH,
> };
> 
> It looks like I'll need to modify "soc_camera_platform" it in a way to at
> least add a "fourcc" field that can be interpreted by the ceu driver for the
> "sh_mobile_ceu_set_bus_param" call to setup the hardware correctly.

No, subdevice drivers, using the mediabus API, know nothing about fourcc 
codes, that belongs to the user side of the pixel format handling API. The 
path, e.g., for the VIDIOC_S_FMT ioctl() is

soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)

the latter will try to call the .s_mbus_fmt() method from 
soc_camera_platform.c and will fail, because that got lost during the 
v4l2-subdev conversion, which is a bug and has to be fixed, patch welcome.

> But regardless of how I set this structure up, I don't see any direct support
> for a Grayscale mode data capture in the ceu driver.  For example,
> "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in its list
> of fourcc formats.  Yet based on the 7724 hardware manual, and from the
> information I have received from Renesas, I'm not seeing any reason why this
> format should not be supported.
> 
> Is grayscale somehow supported under the current CEU driver?

Sure, that's what the pass-through mode with a standard soc-mbus format 
conversion is for (see soc_mbus_get_fmtdesc()).

> Any suggestions on how I might go about implementing support?  I'm having
> trouble seeing the dataflow through the driver at the moment...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-05-04  6:43           ` Guennadi Liakhovetski
@ 2010-05-14 20:10             ` Charles D. Krebs
  2010-05-14 20:38               ` Guennadi Liakhovetski
  0 siblings, 1 reply; 8+ messages in thread
From: Charles D. Krebs @ 2010-05-14 20:10 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

Guennadi,

> No, subdevice drivers, using the mediabus API, know nothing about fourcc
> codes, that belongs to the user side of the pixel format handling API. The
> path, e.g., for the VIDIOC_S_FMT ioctl() is
>
> soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
> sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)
>
> the latter will try to call the .s_mbus_fmt() method from
> soc_camera_platform.c and will fail, because that got lost during the
> v4l2-subdev conversion, which is a bug and has to be fixed, patch welcome.
>

In trying to figure out how to best patch the lacking .s_mbus_fmt() option, 
I setup:

static struct v4l2_subdev_video_ops platform_subdev_video_ops = {
	.s_stream	= soc_camera_platform_s_stream,
	.try_mbus_fmt	= soc_camera_platform_try_fmt,
	.enum_mbus_fmt	= soc_camera_platform_enum_fmt,
	.g_mbus_fmt     = soc_camera_platform_try_fmt,
	.s_mbus_fmt     = soc_camera_platform_try_fmt,

};

Is there any reason I can't reuse a slightly modified "try_fmt" function to 
do the set?

Here's what I currently have:

static int soc_camera_platform_try_fmt(struct v4l2_subdev *sd,
				       struct v4l2_mbus_framefmt *mf)
{
	struct soc_camera_platform_info *p = v4l2_get_subdevdata(sd);

	mf->width	= p->format.width;
	mf->height	= p->format.height;
	mf->code	= p->format.code;
	mf->colorspace	= p->format.colorspace;
	mf->field	                = p->format.field;

	return 0;
}

>> But regardless of how I set this structure up, I don't see any direct 
>> support
>> for a Grayscale mode data capture in the ceu driver.  For example,
>> "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in its 
>> list
>> of fourcc formats.  Yet based on the 7724 hardware manual, and from the
>> information I have received from Renesas, I'm not seeing any reason why 
>> this
>> format should not be supported.
>>
>> Is grayscale somehow supported under the current CEU driver?
>
> Sure, that's what the pass-through mode with a standard soc-mbus format
> conversion is for (see soc_mbus_get_fmtdesc()).


After thoroughly reviewing the register documentation for the CEU, there are 
several changes I'm implementing.  This camera doesn't put out any color 
sampling, so the registers in the CEU must be configured to use "image data 
fetch" mode.

Where I'm currently struggling is in "sh_mobile_ceu_set_fmt" where the call 
is made to "soc_camera_xlate_by_fourcc(icd, pixfmt)".

I'm still trying to figure out where the data needs to be defined that 
eventually becomes "pixfmt".  Should this be setup as an additional 
parameter from inside "soc_camera_platform_try_fmt"?

What's the relationship between "v4l2_format" and "v4l2_mbus_framefmt"?

Thank you,

Charles Krebs, Embedded Solutions Developer
The Realtime Group

--------------------------------------------------
From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
Sent: Tuesday, May 04, 2010 1:43 AM
To: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>; "Magnus Damm" 
<magnus.damm@gmail.com>
Subject: Re: CEU / Camera Driver Question

> Hi Charles
>
> On Mon, 3 May 2010, Charles D. Krebs wrote:
>
>> Guennadi,
>>
>> As per your recommendation I reviewed the "soc_camera_platform" driver, 
>> and
>> have chosen to implement the new camera by simply modifying it.
>>
>> Sure enough, I can boot up and properly register a device under 
>> /dev/video0.
>>
>> The camera provides 8-bit Grayscale data corresponding to pixel format
>> V4L2_PIX_FMT_GREY.  I can't seem to find any example of a device driver 
>> that
>> uses this format, so I've been taking my best guess as how to setup
>> "soc_camera_platform_info".  So far I have:
>>
>> static struct soc_camera_platform_info mycam_camera_info = {
>> .format_name = "GREY",
>> .format_depth = 8,
>> .format = {
>> .code = V4L2_MBUS_FMT_YUYV8_2X8_BE,
>
> No, you should be using V4L2_MBUS_FMT_GREY8_1X8 for grey.
>
>> .colorspace = V4L2_COLORSPACE_JPEG,
>> .field = V4L2_FIELD_NONE,
>> .width = 320,
>> .height = 240,
>> },
>> .bus_param = SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
>> SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8 |
>> SOCAM_DATA_ACTIVE_HIGH,
>> };
>>
>> It looks like I'll need to modify "soc_camera_platform" it in a way to at
>> least add a "fourcc" field that can be interpreted by the ceu driver for 
>> the
>> "sh_mobile_ceu_set_bus_param" call to setup the hardware correctly.
>
> No, subdevice drivers, using the mediabus API, know nothing about fourcc
> codes, that belongs to the user side of the pixel format handling API. The
> path, e.g., for the VIDIOC_S_FMT ioctl() is
>
> soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
> sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)
>
> the latter will try to call the .s_mbus_fmt() method from
> soc_camera_platform.c and will fail, because that got lost during the
> v4l2-subdev conversion, which is a bug and has to be fixed, patch welcome.
>
>> But regardless of how I set this structure up, I don't see any direct 
>> support
>> for a Grayscale mode data capture in the ceu driver.  For example,
>> "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in its 
>> list
>> of fourcc formats.  Yet based on the 7724 hardware manual, and from the
>> information I have received from Renesas, I'm not seeing any reason why 
>> this
>> format should not be supported.
>>
>> Is grayscale somehow supported under the current CEU driver?
>
> Sure, that's what the pass-through mode with a standard soc-mbus format
> conversion is for (see soc_mbus_get_fmtdesc()).
>
>> Any suggestions on how I might go about implementing support?  I'm having
>> trouble seeing the dataflow through the driver at the moment...
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-05-14 20:10             ` Charles D. Krebs
@ 2010-05-14 20:38               ` Guennadi Liakhovetski
  2010-05-14 21:27                 ` Charles D. Krebs
  2010-05-15  3:10                 ` Charles D. Krebs
  0 siblings, 2 replies; 8+ messages in thread
From: Guennadi Liakhovetski @ 2010-05-14 20:38 UTC (permalink / raw)
  To: Charles D. Krebs; +Cc: Linux Media Mailing List

Hi Charles

On Fri, 14 May 2010, Charles D. Krebs wrote:

> Guennadi,
> 
> > No, subdevice drivers, using the mediabus API, know nothing about fourcc
> > codes, that belongs to the user side of the pixel format handling API. The
> > path, e.g., for the VIDIOC_S_FMT ioctl() is
> > 
> > soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
> > sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)
> > 
> > the latter will try to call the .s_mbus_fmt() method from
> > soc_camera_platform.c and will fail, because that got lost during the
> > v4l2-subdev conversion, which is a bug and has to be fixed, patch welcome.
> > 
> 
> In trying to figure out how to best patch the lacking .s_mbus_fmt() option, I
> setup:
> 
> static struct v4l2_subdev_video_ops platform_subdev_video_ops = {
> 	.s_stream	= soc_camera_platform_s_stream,
> 	.try_mbus_fmt	= soc_camera_platform_try_fmt,
> 	.enum_mbus_fmt	= soc_camera_platform_enum_fmt,
> 	.g_mbus_fmt     = soc_camera_platform_try_fmt,
> 	.s_mbus_fmt     = soc_camera_platform_try_fmt,
> 
> };
> 
> Is there any reason I can't reuse a slightly modified "try_fmt" function to do
> the set?
> 
> Here's what I currently have:
> 
> static int soc_camera_platform_try_fmt(struct v4l2_subdev *sd,
> 				       struct v4l2_mbus_framefmt *mf)
> {
> 	struct soc_camera_platform_info *p = v4l2_get_subdevdata(sd);
> 
> 	mf->width	= p->format.width;
> 	mf->height	= p->format.height;
> 	mf->code	= p->format.code;
> 	mf->colorspace	= p->format.colorspace;
> 	mf->field	                = p->format.field;
> 
> 	return 0;
> }

How is this different from this patch from Morimoto-san:

http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/19191

> > > But regardless of how I set this structure up, I don't see any direct
> > > support
> > > for a Grayscale mode data capture in the ceu driver.  For example,
> > > "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in its
> > > list
> > > of fourcc formats.  Yet based on the 7724 hardware manual, and from the
> > > information I have received from Renesas, I'm not seeing any reason why
> > > this
> > > format should not be supported.
> > > 
> > > Is grayscale somehow supported under the current CEU driver?
> > 
> > Sure, that's what the pass-through mode with a standard soc-mbus format
> > conversion is for (see soc_mbus_get_fmtdesc()).
> 
> 
> After thoroughly reviewing the register documentation for the CEU, there are
> several changes I'm implementing.  This camera doesn't put out any color
> sampling, so the registers in the CEU must be configured to use "image data
> fetch" mode.
> 
> Where I'm currently struggling is in "sh_mobile_ceu_set_fmt" where the call is
> made to "soc_camera_xlate_by_fourcc(icd, pixfmt)".
> 
> I'm still trying to figure out where the data needs to be defined that
> eventually becomes "pixfmt".  Should this be setup as an additional parameter
> from inside "soc_camera_platform_try_fmt"?
> 
> What's the relationship between "v4l2_format" and "v4l2_mbus_framefmt"?

Have you set up platform data similar to "struct soc_camera_link 
camera_link" in arch/sh/boards/mach-ap325rxa/setup.c. The 
soc_camera_platform.c driver should be getting pixel format information 
from the static struct soc_camera_platform_info, doesn't this work for 
you?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-05-14 20:38               ` Guennadi Liakhovetski
@ 2010-05-14 21:27                 ` Charles D. Krebs
  2010-05-15  3:10                 ` Charles D. Krebs
  1 sibling, 0 replies; 8+ messages in thread
From: Charles D. Krebs @ 2010-05-14 21:27 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: Linux Media Mailing List

--------------------------------------------------
From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
Sent: Friday, May 14, 2010 3:38 PM
To: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>
Subject: Re: CEU / Camera Driver Question

> Hi Charles
>
> On Fri, 14 May 2010, Charles D. Krebs wrote:
>
>> Guennadi,
>>
>> > No, subdevice drivers, using the mediabus API, know nothing about 
>> > fourcc
>> > codes, that belongs to the user side of the pixel format handling API. 
>> > The
>> > path, e.g., for the VIDIOC_S_FMT ioctl() is
>> >
>> > soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
>> > sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)
>> >
>> > the latter will try to call the .s_mbus_fmt() method from
>> > soc_camera_platform.c and will fail, because that got lost during the
>> > v4l2-subdev conversion, which is a bug and has to be fixed, patch 
>> > welcome.
>> >
>>
>> In trying to figure out how to best patch the lacking .s_mbus_fmt() 
>> option, I
>> setup:
>>
>> static struct v4l2_subdev_video_ops platform_subdev_video_ops = {
>> .s_stream = soc_camera_platform_s_stream,
>> .try_mbus_fmt = soc_camera_platform_try_fmt,
>> .enum_mbus_fmt = soc_camera_platform_enum_fmt,
>> .g_mbus_fmt     = soc_camera_platform_try_fmt,
>> .s_mbus_fmt     = soc_camera_platform_try_fmt,
>>
>> };
>>
>> Is there any reason I can't reuse a slightly modified "try_fmt" function 
>> to do
>> the set?
>>
>> Here's what I currently have:
>>
>> static int soc_camera_platform_try_fmt(struct v4l2_subdev *sd,
>>        struct v4l2_mbus_framefmt *mf)
>> {
>> struct soc_camera_platform_info *p = v4l2_get_subdevdata(sd);
>>
>> mf->width = p->format.width;
>> mf->height = p->format.height;
>> mf->code = p->format.code;
>> mf->colorspace = p->format.colorspace;
>> mf->field                 = p->format.field;
>>
>> return 0;
>> }
>
> How is this different from this patch from Morimoto-san:
>

You're right - It's not a whole lot different other than I didn't add the 
scaling functions to the sub commands.

> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/19191
>
>> > > But regardless of how I set this structure up, I don't see any direct
>> > > support
>> > > for a Grayscale mode data capture in the ceu driver.  For example,
>> > > "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in 
>> > > its
>> > > list
>> > > of fourcc formats.  Yet based on the 7724 hardware manual, and from 
>> > > the
>> > > information I have received from Renesas, I'm not seeing any reason 
>> > > why
>> > > this
>> > > format should not be supported.
>> > >
>> > > Is grayscale somehow supported under the current CEU driver?
>> >
>> > Sure, that's what the pass-through mode with a standard soc-mbus format
>> > conversion is for (see soc_mbus_get_fmtdesc()).
>>
>>
>> After thoroughly reviewing the register documentation for the CEU, there 
>> are
>> several changes I'm implementing.  This camera doesn't put out any color
>> sampling, so the registers in the CEU must be configured to use "image 
>> data
>> fetch" mode.
>>
>> Where I'm currently struggling is in "sh_mobile_ceu_set_fmt" where the 
>> call is
>> made to "soc_camera_xlate_by_fourcc(icd, pixfmt)".
>>
>> I'm still trying to figure out where the data needs to be defined that
>> eventually becomes "pixfmt".  Should this be setup as an additional 
>> parameter
>> from inside "soc_camera_platform_try_fmt"?
>>
>> What's the relationship between "v4l2_format" and "v4l2_mbus_framefmt"?
>
> Have you set up platform data similar to "struct soc_camera_link
> camera_link" in arch/sh/boards/mach-ap325rxa/setup.c. The
> soc_camera_platform.c driver should be getting pixel format information
> from the static struct soc_camera_platform_info, doesn't this work for
> you?
>

I have that structure setup as follows:

static struct soc_camera_platform_info mycamera_camera_info = {
	.format_name = "GREY",
	.format_depth = 8,
	.format = {
		.code = V4L2_MBUS_FMT_GREY8_1X8,
		.colorspace = V4L2_COLORSPACE_JPEG, //V4L2_COLORSPACE_SMPTE170M,
		.field = V4L2_FIELD_NONE,
		.width = 320,
		.height = 240,
	},
	.bus_param = SOCAM_PCLK_SAMPLE_RISING | SOCAM_HSYNC_ACTIVE_HIGH |
	SOCAM_VSYNC_ACTIVE_HIGH | SOCAM_MASTER | SOCAM_DATAWIDTH_8 |
	SOCAM_DATA_ACTIVE_HIGH,
	.set_capture = mycamera_camera_set_capture,
};

I would have thought that the pixel format information would have filtered 
back thorough the stack to the host driver, but...

In "sh_mobile_ceu_set_fmt" we have:

static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
				 struct v4l2_format *f)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
	struct sh_mobile_ceu_dev *pcdev = ici->priv;
	struct sh_mobile_ceu_cam *cam = icd->host_priv;
	struct v4l2_pix_format *pix = &f->fmt.pix;
	struct v4l2_mbus_framefmt mf;
	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
	struct device *dev = icd->dev.parent;
	__u32 pixfmt = pix->pixelformat;
	const struct soc_camera_format_xlate *xlate;
	struct v4l2_crop cam_crop;
	struct v4l2_rect *cam_rect = &cam_crop.c, cam_subrect, ceu_rect;
	unsigned int scale_cam_h, scale_cam_v;
	u16 scale_v, scale_h;
	int ret;
	bool image_mode;
	enum v4l2_field field;
...
...

pixfmt is a function of v4l2_format's embedded v4l2_pix_format.  I see that 
there is also a v4l2_mbus_framefmt function here as well.  The client 
driver's "soc_camera_platform_try_fmt" function only sets parameters from 
within the "v4l2_mbus_framefmt" structure.  Where in the stack should the 
v4l2_pix_format get filled out?  Currently it's always bogus data and causes 
" soc_camera_xlate_by_fourcc(icd, pixfmt)" to fail.

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: CEU / Camera Driver Question
  2010-05-14 20:38               ` Guennadi Liakhovetski
  2010-05-14 21:27                 ` Charles D. Krebs
@ 2010-05-15  3:10                 ` Charles D. Krebs
  1 sibling, 0 replies; 8+ messages in thread
From: Charles D. Krebs @ 2010-05-15  3:10 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media

Guennadi,

Thank you for making me aware of the patch that Morimoto-San composed. 
Implementing the remaining sub commands fixes the issues I was seeing.  It's 
rather ironic that he was working on that at the same time I was.

I'm now working through a minor userspace application issue and expect to 
have the overall demo application working shortly to stream video with this 
new camera.  Thanks for all of your help and patience.

If I need to make changes to either soc_camera_platform or the ceu driver, 
I'll be sure to submit patch proposals to that effect.

Best regards,

Charles Krebs, Embedded Solutions Developer
The Realtime Group

--------------------------------------------------
From: "Guennadi Liakhovetski" <g.liakhovetski@gmx.de>
Sent: Friday, May 14, 2010 3:38 PM
To: "Charles D. Krebs" <ckrebs@therealtimegroup.com>
Cc: "Linux Media Mailing List" <linux-media@vger.kernel.org>
Subject: Re: CEU / Camera Driver Question

> Hi Charles
>
> On Fri, 14 May 2010, Charles D. Krebs wrote:
>
>> Guennadi,
>>
>> > No, subdevice drivers, using the mediabus API, know nothing about 
>> > fourcc
>> > codes, that belongs to the user side of the pixel format handling API. 
>> > The
>> > path, e.g., for the VIDIOC_S_FMT ioctl() is
>> >
>> > soc_camera.c::soc_camera_s_fmt_vid_cap(V4L2_PIX_FMT_GREY)
>> > sh_mobile_ceu_camera.c::sh_mobile_ceu_set_fmt(V4L2_PIX_FMT_GREY)
>> >
>> > the latter will try to call the .s_mbus_fmt() method from
>> > soc_camera_platform.c and will fail, because that got lost during the
>> > v4l2-subdev conversion, which is a bug and has to be fixed, patch 
>> > welcome.
>> >
>>
>> In trying to figure out how to best patch the lacking .s_mbus_fmt() 
>> option, I
>> setup:
>>
>> static struct v4l2_subdev_video_ops platform_subdev_video_ops = {
>> .s_stream = soc_camera_platform_s_stream,
>> .try_mbus_fmt = soc_camera_platform_try_fmt,
>> .enum_mbus_fmt = soc_camera_platform_enum_fmt,
>> .g_mbus_fmt     = soc_camera_platform_try_fmt,
>> .s_mbus_fmt     = soc_camera_platform_try_fmt,
>>
>> };
>>
>> Is there any reason I can't reuse a slightly modified "try_fmt" function 
>> to do
>> the set?
>>
>> Here's what I currently have:
>>
>> static int soc_camera_platform_try_fmt(struct v4l2_subdev *sd,
>>        struct v4l2_mbus_framefmt *mf)
>> {
>> struct soc_camera_platform_info *p = v4l2_get_subdevdata(sd);
>>
>> mf->width = p->format.width;
>> mf->height = p->format.height;
>> mf->code = p->format.code;
>> mf->colorspace = p->format.colorspace;
>> mf->field                 = p->format.field;
>>
>> return 0;
>> }
>
> How is this different from this patch from Morimoto-san:
>
> http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/19191
>
>> > > But regardless of how I set this structure up, I don't see any direct
>> > > support
>> > > for a Grayscale mode data capture in the ceu driver.  For example,
>> > > "sh_mobile_ceu_set_bus_param" does not contain V4L2_PIX_FMT_GREY in 
>> > > its
>> > > list
>> > > of fourcc formats.  Yet based on the 7724 hardware manual, and from 
>> > > the
>> > > information I have received from Renesas, I'm not seeing any reason 
>> > > why
>> > > this
>> > > format should not be supported.
>> > >
>> > > Is grayscale somehow supported under the current CEU driver?
>> >
>> > Sure, that's what the pass-through mode with a standard soc-mbus format
>> > conversion is for (see soc_mbus_get_fmtdesc()).
>>
>>
>> After thoroughly reviewing the register documentation for the CEU, there 
>> are
>> several changes I'm implementing.  This camera doesn't put out any color
>> sampling, so the registers in the CEU must be configured to use "image 
>> data
>> fetch" mode.
>>
>> Where I'm currently struggling is in "sh_mobile_ceu_set_fmt" where the 
>> call is
>> made to "soc_camera_xlate_by_fourcc(icd, pixfmt)".
>>
>> I'm still trying to figure out where the data needs to be defined that
>> eventually becomes "pixfmt".  Should this be setup as an additional 
>> parameter
>> from inside "soc_camera_platform_try_fmt"?
>>
>> What's the relationship between "v4l2_format" and "v4l2_mbus_framefmt"?
>
> Have you set up platform data similar to "struct soc_camera_link
> camera_link" in arch/sh/boards/mach-ap325rxa/setup.c. The
> soc_camera_platform.c driver should be getting pixel format information
> from the static struct soc_camera_platform_info, doesn't this work for
> you?
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-05-15  3:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <C5F5A45C8EB6446BA837800AC37D53A2@RSI45>
     [not found] ` <h2laec7e5c31004071719m4a6551c7w8afdca6bdcf49eae@mail.gmail.com>
     [not found]   ` <Pine.LNX.4.64.1004080814370.4621@axis700.grange>
2010-04-14  1:54     ` CEU / Camera Driver Question Charles D. Krebs
2010-04-14  6:38       ` Guennadi Liakhovetski
2010-05-04  2:26         ` Charles D. Krebs
2010-05-04  6:43           ` Guennadi Liakhovetski
2010-05-14 20:10             ` Charles D. Krebs
2010-05-14 20:38               ` Guennadi Liakhovetski
2010-05-14 21:27                 ` Charles D. Krebs
2010-05-15  3:10                 ` Charles D. Krebs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox