Hello Guennadi,
Guennadi Liakhovetski wrote:
First more details about what I do with the camera: I open the device, issue
the S_CROP / S_FMT calls and read images, the behaviour is fine, then close
the device.
Then if I reopen the device, reissue the S_CROP / S_FMT calls with the same
params, but the images is not the sames because of different xskip and yskip.
From what I have debugged in the driver at the second S_CROP /S_FMT, xskip and
yskip are computed by mt9t031_skip (and have the same value that the one
stored in the mt9t031 struct) and thus with the current code are not
rewritten.
However, if I read the register values containing bin and skip values on the
camera chip they have been reset (does a open/close do some reset to the cam
?) and thus different than the ones that should be written.
Yes, if hooks are provided by the platform, on last close we power down
cameras, on first open we power on and reset them.
I hope this clarifies the problem that I am experiencing. I don't think that
the API wants you to get two different images when you open the device and
issue the same parameters twice.
Yes, sorry, this is a different issue. The issue is - too crude power
management in soc-camera. What we should do is implement proper (runtime)
power-management in soc-camera (ideally, in v4l2-subdev API) and call
suspend() to save registers before powering down, and resume() after
powering on and resetting.
I gave it a shot and just posted a patch
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/15686
(hm, would have been smart to cc you, sorry), doing just that. Below is an
example dummy implementation for mt9v022. Please, use it as example to
implement suspend / resume for mt9t031, for now, I think, it would suffice
if you just restore xskip and yskip in restore and skip suspend. If more
is needed in the future, we can always extend it.
OK, I agree with your analysis and I have tried to write the part for
runtime suspend and resume to solve this issue. I however find myself in
a deadend because of my limited knowledge of the kernel device model.
Here is my attempt with my questions in it (it compiles, but I have not
tested it yet because I am pretty sure something is wrong):
/*
* Power Management:
* This function does nothing for now but must be present for pm to work
*/
static int mt9t031_runtime_suspend(struct device *dev)
{
return 0;
}
First, can you confirm me that this function is needed even if it does
nothing ? I have read in the code that if the function is not present,
__pm_runtime_suspend is going to return -ENOSYS and will set
runtime_status to RPM_ACTIVE, which is not what we want. That's why I
left it.
/*
* Power Management:
* COLUMN_ADDRESS_MODE and ROW_ADDRESS_MODE are not rewritten if unchanged
* they are however changed at reset if the platform hook is present
* thus we rewrite them with the values stored by the driver
*/
static int mt9t031_runtime_resume(struct device *dev)
{
struct video_device *vdev = to_video_device(dev);
struct soc_camera_device *icd = container_of(vdev->parent,
struct soc_camera_device, dev);
struct device *i2c_dev = container_of(icd, struct device,
platform_data);
struct i2c_client *client = to_i2c_client(i2c_dev);
struct mt9t031 *mt9t031 = to_mt9t031(client);
Here I have a problem ... I am pretty sure that the third assignation
has a problem, because platform_data is a pointer and not a normal
member of struct device, and I thus cannot use container_of with it.
What would then be the way to go from the soc_camera_device to the
i2c_client (I'm a little bit confused with all the different structs and
layers involved with i2c, soc-camera, v4l2_device and v4l2_subdevice) ?
Just as a remark, this is the exact reverse path of this that is present
in your patch to add runtime pm support to soc-camera:
/* This is only temporary here - until v4l2-subdev begins to link to
video_device */
#include <linux/i2c.h>
static inline struct video_device *soc_camera_i2c_to_vdev(struct
i2c_client *client)
{
struct soc_camera_device *icd = client->dev.platform_data;
return icd->vdev;
}
int ret;
u16 xbin, ybin;
xbin = min(mt9t031->xskip, (u16)3);
ybin = min(mt9t031->yskip, (u16)3);
ret = reg_write(client, MT9T031_COLUMN_ADDRESS_MODE,
((xbin-1)<<4) | (mt9t031->xskip-1));
if (ret < 0)
return ret;
ret = reg_write(client, MT9T031_ROW_ADDRESS_MODE,
((ybin-1)<<4) | (mt9t031->yskip-1));
if (ret < 0)
return ret;
return 0;
}
static struct dev_pm_ops mt9t031_dev_pm_ops = {
.runtime_suspend = mt9t031_runtime_suspend,
.runtime_resume = mt9t031_runtime_resume,
};
static struct device_type mt9t031_dev_type = {
.name = "MT9T031",
.pm = &mt9t031_dev_pm_ops,
};
Thank you in advance for your help.
Val
--
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp@xxxxxxx, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEB3494, Station 9, CH-1015 Lausanne
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
