Hello Jerry,
Maybe I got two more bugs on NVidia code on file channel.c:
look at this function of channel.c:
static void tegra_channel_fmts_bitmap_init(struct tegra_channel *chan)
{
int ret, pixel_format_index = 0, init_code = 0;
struct v4l2_subdev *subdev = chan->subdev_on_csi;
struct v4l2_subdev_format fmt = {};
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
bitmap_zero(chan->fmts_bitmap, MAX_FORMAT_NUM);
/*
* Initialize all the formats available from
* the sub-device and extract the corresponding
* index from the pre-defined video formats and initialize
* the channel default format with the active code
* Index zero as the only sub-device is sensor
*/
while (1) {
ret = v4l2_subdev_call(subdev, pad, enum_mbus_code,
NULL, &code);
if (ret < 0)
/* no more formats */
break;
pixel_format_index =
tegra_core_get_idx_by_code(chan, code.code, 0);
while (pixel_format_index >= 0) {
bitmap_set(chan->fmts_bitmap, pixel_format_index, 1);
/* Set init_code to the first matched format */
if (!init_code)
init_code = code.code;
/* Look for other formats with the same mbus code */
pixel_format_index = tegra_core_get_idx_by_code(chan,
code.code, pixel_format_index + 1);
}
code.index++;
}
if (!init_code) {
pixel_format_index =
tegra_core_get_idx_by_code(chan, TEGRA_VF_DEF, 0);
if (pixel_format_index >= 0) {
bitmap_set(chan->fmts_bitmap, pixel_format_index, 1);
init_code = TEGRA_VF_DEF;
}
}
/* Get the format based on active code of the sub-device */
ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
if (ret)
return;
/* Initiate the channel format to the first matched format */
chan->fmtinfo =
tegra_core_get_format_by_code(chan, fmt.format.code, 0);
v4l2_fill_pix_format(&chan->format, &fmt.format);
tegra_channel_update_format(chan, chan->format.width,
chan->format.height,
chan->fmtinfo->fourcc,
&chan->fmtinfo->bpp, 0);
if (chan->total_ports > 1)
update_gang_mode(chan);
}
At line 50 it will call the function linked on struct v4l2_subdev_pad_ops by member get_fmt.
This struct is defined on file media/v4l2-subdev.h of linux kernel code:
struct v4l2_subdev_pad_ops {
//suppressed code...
int (*get_fmt)(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format);
int (*set_fmt)(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format);
//suppressed code.
};
The second argument is of kind v4l2_subdev_pad_config, and at line 50 nvidia code send a NULL to this argument.
But on the file media/v4l2-subdev.h we have the definition and a comment about this type:
/**
* struct v4l2_subdev_pad_config - Used for storing subdev pad information.
*
* @try_fmt: &struct v4l2_mbus_framefmt
* @try_crop: &struct v4l2_rect to be used for crop
* @try_compose: &struct v4l2_rect to be used for compose
*
* This structure only needs to be passed to the pad op if the 'which' field
* of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
* %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
*/
struct v4l2_subdev_pad_config {
struct v4l2_mbus_framefmt try_fmt;
struct v4l2_rect try_crop;
struct v4l2_rect try_compose;
};
The problem here is the initialization of the third argument (on line 5) because V4L2 library define
V4L2_SUBDEV_FORMAT_TRY = 0, so the variable fmt.which is V4L2_SUBDEV_FORMAT_TRY when get_fmt is called with cfg = NULL and this is not allowed by V4L2 library “This structure only needs to be passed to the pad op if the ‘which’ field of the main argument is set to V4L2_SUBDEV_FORMAT_TRY. For V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass NULL.”
Because of this the driver adv7180.c (and I supose many others) on Linux upstream code fail on initialization if used with NVidia L4T.
I changed the line 5 to:
struct v4l2_subdev_format fmt = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
And it doesn’t fail at driver initialization anymore.
Second bug
Now the problem is on function tegra_channel_set_format:
static int
tegra_channel_set_format(struct file *file, void *fh,
struct v4l2_format *format)
{
struct v4l2_fh *vfh = file->private_data;
struct tegra_channel *chan = to_tegra_channel(vfh->vdev);
int ret = 0;
/* get the suppod format by try_fmt */
ret = __tegra_channel_try_format(chan, &format->fmt.pix);
if (ret)
return ret;
if (vb2_is_busy(&chan->queue))
return -EBUSY;
return __tegra_channel_set_format(chan, &format->fmt.pix);
}
This function try to set the format by a “format try” on line 10, and if it don’t successful set
it by format active on line 17.
But on line 10 it calls the subfunction __tegra_channel_try_format
static int
__tegra_channel_try_format(struct tegra_channel *chan,
struct v4l2_pix_format *pix)
{
const struct tegra_video_format *vfmt;
struct v4l2_subdev_format fmt;
struct v4l2_subdev *sd = chan->subdev_on_csi;
int ret = 0;
/* Use the channel format if pixformat is not supported */
vfmt = tegra_core_get_format_by_fourcc(chan, pix->pixelformat);
if (!vfmt) {
pix->pixelformat = chan->format.pixelformat;
vfmt = tegra_core_get_format_by_fourcc(chan, pix->pixelformat);
}
fmt.which = V4L2_SUBDEV_FORMAT_TRY;
fmt.pad = 0;
v4l2_fill_mbus_format(&fmt.format, pix, vfmt->code);
ret = v4l2_subdev_call(sd, pad, set_fmt, NULL, &fmt);
if (ret == -ENOIOCTLCMD)
return -ENOTTY;
v4l2_fill_pix_format(pix, &fmt.format);
tegra_channel_set_bytesperline(chan, vfmt, pix);
tegra_channel_fmt_align(chan, vfmt,
&pix->width, &pix->height, &pix->bytesperline);
pix->sizeimage = get_aligned_buffer_size(chan,
pix->bytesperline, pix->height);
if (chan->fmtinfo->fourcc == V4L2_PIX_FMT_NV16)
pix->sizeimage *= 2;
return ret;
}
This function on line 21 calls the function “set_fmt” of driver and send a cfg=NULL like the previus bug.
I don’t have a solution to this second bug, so I did a workaround on the drivers source, and on “set_fmt” function if the cfg=NULL I return a ENOIOCTLCMD error, and the tegra source will call the driver function again with fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE
I looked at linux kernel drivers media/i2c/ and found others drivers that will fail with this tegra code. So maybe a good ideia to fix it.
's