Skip to content

Commit

Permalink
media: atomisp: Adjust for v4l2_subdev_state handling changes in 6.8
Browse files Browse the repository at this point in the history 
The atomisp driver emulates a standard v4l2 device, which also works
for non media-controller aware applications.

Part of this requires making try_fmt calls on the sensor when
a normal v4l2 app is making try_fmt calls on the /dev/video# mode.

With the recent v4l2_subdev_state handling changes in 6.8 this no longer
works, fixing this requires 2 changes:

1. The atomisp code was using its own internal v4l2_subdev_pad_config
   for this. Replace the internal v4l2_subdev_pad_config with allocating
   a full v4l2_subdev_state for storing the full try_fmt state.

2. The paths actually setting the fmt or crop selection now need to be
   passed the v4l2_subdev's active state, so that sensor drivers which
   are using the v4l2_subdev's active state to store their state keep
   working.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
  • Loading branch information
@jwrdegoede @mchehab
jwrdegoede authored and mchehab committed Feb 1, 2024
1 parent ffb635b commit f66556c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 62 deletions.
58 changes: 34 additions & 24 deletions drivers/staging/media/atomisp/pci/atomisp_cmd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3723,12 +3723,10 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,

static int atomisp_set_crop(struct atomisp_device *isp,
const struct v4l2_mbus_framefmt *format,
struct v4l2_subdev_state *sd_state,
int which)
{
struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr];
struct v4l2_subdev_state pad_state = {
.pads = &input->pad_cfg,
};
struct v4l2_subdev_selection sel = {
.which = which,
.target = V4L2_SEL_TGT_CROP,
Expand All @@ -3754,7 +3752,7 @@ static int atomisp_set_crop(struct atomisp_device *isp,
sel.r.left = ((input->native_rect.width - sel.r.width) / 2) & ~1;
sel.r.top = ((input->native_rect.height - sel.r.height) / 2) & ~1;

ret = v4l2_subdev_call(input->camera, pad, set_selection, &pad_state, &sel);
ret = v4l2_subdev_call(input->camera, pad, set_selection, sd_state, &sel);
if (ret)
dev_err(isp->dev, "Error setting crop to %ux%u @%ux%u: %d\n",
sel.r.width, sel.r.height, sel.r.left, sel.r.top, ret);
Expand All @@ -3770,9 +3768,6 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
const struct atomisp_format_bridge *fmt, *snr_fmt;
struct atomisp_sub_device *asd = &isp->asd;
struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
struct v4l2_subdev_state pad_state = {
.pads = &input->pad_cfg,
};
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
Expand Down Expand Up @@ -3809,11 +3804,16 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
dev_dbg(isp->dev, "try_mbus_fmt: asking for %ux%u\n",
format.format.width, format.format.height);

ret = atomisp_set_crop(isp, &format.format, V4L2_SUBDEV_FORMAT_TRY);
if (ret)
return ret;
v4l2_subdev_lock_state(input->try_sd_state);

ret = atomisp_set_crop(isp, &format.format, input->try_sd_state,
V4L2_SUBDEV_FORMAT_TRY);
if (ret == 0)
ret = v4l2_subdev_call(input->camera, pad, set_fmt,
input->try_sd_state, &format);

v4l2_subdev_unlock_state(input->try_sd_state);

ret = v4l2_subdev_call(input->camera, pad, set_fmt, &pad_state, &format);
if (ret)
return ret;

Expand Down Expand Up @@ -4238,9 +4238,7 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
struct atomisp_device *isp = asd->isp;
struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
const struct atomisp_format_bridge *format;
struct v4l2_subdev_state pad_state = {
.pads = &input->pad_cfg,
};
struct v4l2_subdev_state *act_sd_state;
struct v4l2_subdev_format vformat = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
Expand Down Expand Up @@ -4268,12 +4266,18 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p

/* Disable dvs if resolution can't be supported by sensor */
if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
ret = atomisp_set_crop(isp, &vformat.format, V4L2_SUBDEV_FORMAT_TRY);
if (ret)
return ret;
v4l2_subdev_lock_state(input->try_sd_state);

ret = atomisp_set_crop(isp, &vformat.format, input->try_sd_state,
V4L2_SUBDEV_FORMAT_TRY);
if (ret == 0) {
vformat.which = V4L2_SUBDEV_FORMAT_TRY;
ret = v4l2_subdev_call(input->camera, pad, set_fmt,
input->try_sd_state, &vformat);
}

v4l2_subdev_unlock_state(input->try_sd_state);

vformat.which = V4L2_SUBDEV_FORMAT_TRY;
ret = v4l2_subdev_call(input->camera, pad, set_fmt, &pad_state, &vformat);
if (ret)
return ret;

Expand All @@ -4291,12 +4295,18 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
}
}

ret = atomisp_set_crop(isp, &vformat.format, V4L2_SUBDEV_FORMAT_ACTIVE);
if (ret)
return ret;
act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);

ret = atomisp_set_crop(isp, &vformat.format, act_sd_state,
V4L2_SUBDEV_FORMAT_ACTIVE);
if (ret == 0) {
vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(input->camera, pad, set_fmt, act_sd_state, &vformat);
}

if (act_sd_state)
v4l2_subdev_unlock_state(act_sd_state);

vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(input->camera, pad, set_fmt, NULL, &vformat);
if (ret)
return ret;

Expand Down
4 changes: 2 additions & 2 deletions drivers/staging/media/atomisp/pci/atomisp_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ struct atomisp_input_subdev {
/* Sensor rects for sensors which support crop */
struct v4l2_rect native_rect;
struct v4l2_rect active_rect;
/* Sensor pad_cfg for which == V4L2_SUBDEV_FORMAT_TRY calls */
struct v4l2_subdev_pad_config pad_cfg;
/* Sensor state for which == V4L2_SUBDEV_FORMAT_TRY calls */
struct v4l2_subdev_state *try_sd_state;

struct v4l2_subdev *motor;

Expand Down
52 changes: 31 additions & 21 deletions drivers/staging/media/atomisp/pci/atomisp_ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -781,12 +781,20 @@ static int atomisp_enum_framesizes(struct file *file, void *priv,
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
.code = input->code,
};
struct v4l2_subdev_state *act_sd_state;
int ret;

if (!input->camera)
return -EINVAL;

if (input->crop_support)
return atomisp_enum_framesizes_crop(isp, fsize);

ret = v4l2_subdev_call(input->camera, pad, enum_frame_size, NULL, &fse);
act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);
ret = v4l2_subdev_call(input->camera, pad, enum_frame_size,
act_sd_state, &fse);
if (act_sd_state)
v4l2_subdev_unlock_state(act_sd_state);
if (ret)
return ret;

Expand All @@ -803,18 +811,25 @@ static int atomisp_enum_frameintervals(struct file *file, void *priv,
struct video_device *vdev = video_devdata(file);
struct atomisp_device *isp = video_get_drvdata(vdev);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
struct v4l2_subdev_frame_interval_enum fie = {
.code = atomisp_in_fmt_conv[0].code,
.code = atomisp_in_fmt_conv[0].code,
.index = fival->index,
.width = fival->width,
.height = fival->height,
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
struct v4l2_subdev_state *act_sd_state;
int ret;

ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
pad, enum_frame_interval, NULL,
&fie);
if (!input->camera)
return -EINVAL;

act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);
ret = v4l2_subdev_call(input->camera, pad, enum_frame_interval,
act_sd_state, &fie);
if (act_sd_state)
v4l2_subdev_unlock_state(act_sd_state);
if (ret)
return ret;

Expand All @@ -830,30 +845,25 @@ static int atomisp_enum_fmt_cap(struct file *file, void *fh,
struct video_device *vdev = video_devdata(file);
struct atomisp_device *isp = video_get_drvdata(vdev);
struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
};
const struct atomisp_format_bridge *format;
struct v4l2_subdev *camera;
struct v4l2_subdev_state *act_sd_state;
unsigned int i, fi = 0;
int rval;
int ret;

camera = isp->inputs[asd->input_curr].camera;
if(!camera) {
dev_err(isp->dev, "%s(): camera is NULL, device is %s\n",
__func__, vdev->name);
if (!input->camera)
return -EINVAL;
}

rval = v4l2_subdev_call(camera, pad, enum_mbus_code, NULL, &code);
if (rval == -ENOIOCTLCMD) {
dev_warn(isp->dev,
"enum_mbus_code pad op not supported by %s. Please fix your sensor driver!\n",
camera->name);
}

if (rval)
return rval;
act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);
ret = v4l2_subdev_call(input->camera, pad, enum_mbus_code,
act_sd_state, &code);
if (act_sd_state)
v4l2_subdev_unlock_state(act_sd_state);
if (ret)
return ret;

for (i = 0; i < ARRAY_SIZE(atomisp_output_fmts); i++) {
format = &atomisp_output_fmts[i];
Expand Down
59 changes: 44 additions & 15 deletions drivers/staging/media/atomisp/pci/atomisp_v4l2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,9 @@ static void atomisp_unregister_entities(struct atomisp_device *isp)
v4l2_device_unregister(&isp->v4l2_dev);
media_device_unregister(&isp->media_dev);
media_device_cleanup(&isp->media_dev);

for (i = 0; i < isp->input_cnt; i++)
__v4l2_subdev_state_free(isp->inputs[i].try_sd_state);
}

static int atomisp_register_entities(struct atomisp_device *isp)
Expand Down Expand Up @@ -933,32 +936,49 @@ static int atomisp_register_entities(struct atomisp_device *isp)

static void atomisp_init_sensor(struct atomisp_input_subdev *input)
{
static struct lock_class_key try_sd_state_key;
struct v4l2_subdev_mbus_code_enum mbus_code_enum = { };
struct v4l2_subdev_frame_size_enum fse = { };
struct v4l2_subdev_state sd_state = {
.pads = &input->pad_cfg,
};
struct v4l2_subdev_selection sel = { };
struct v4l2_subdev_state *try_sd_state, *act_sd_state;
int i, err;

/*
* FIXME: Drivers are not supposed to use __v4l2_subdev_state_alloc()
* but atomisp needs this for try_fmt on its /dev/video# node since
* it emulates a normal v4l2 device there, passing through try_fmt /
* set_fmt to the sensor.
*/
try_sd_state = __v4l2_subdev_state_alloc(input->camera,
"atomisp:try_sd_state->lock", &try_sd_state_key);
if (IS_ERR(try_sd_state))
return;

input->try_sd_state = try_sd_state;

act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);

mbus_code_enum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
err = v4l2_subdev_call(input->camera, pad, enum_mbus_code, NULL, &mbus_code_enum);
err = v4l2_subdev_call(input->camera, pad, enum_mbus_code,
act_sd_state, &mbus_code_enum);
if (!err)
input->code = mbus_code_enum.code;

sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
sel.target = V4L2_SEL_TGT_NATIVE_SIZE;
err = v4l2_subdev_call(input->camera, pad, get_selection, NULL, &sel);
err = v4l2_subdev_call(input->camera, pad, get_selection,
act_sd_state, &sel);
if (err)
return;
goto unlock_act_sd_state;

input->native_rect = sel.r;

sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
sel.target = V4L2_SEL_TGT_CROP_DEFAULT;
err = v4l2_subdev_call(input->camera, pad, get_selection, NULL, &sel);
err = v4l2_subdev_call(input->camera, pad, get_selection,
act_sd_state, &sel);
if (err)
return;
goto unlock_act_sd_state;

input->active_rect = sel.r;

Expand All @@ -973,7 +993,8 @@ static void atomisp_init_sensor(struct atomisp_input_subdev *input)
fse.code = input->code;
fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;

err = v4l2_subdev_call(input->camera, pad, enum_frame_size, NULL, &fse);
err = v4l2_subdev_call(input->camera, pad, enum_frame_size,
act_sd_state, &fse);
if (err)
break;

Expand All @@ -989,29 +1010,37 @@ static void atomisp_init_sensor(struct atomisp_input_subdev *input)
* for padding, set the crop rect to cover the entire sensor instead
* of only the default active area.
*
* Do this for both try and active formats since the try_crop rect in
* pad_cfg may influence (clamp) future try_fmt calls with which == try.
* Do this for both try and active formats since the crop rect in
* try_sd_state may influence (clamp size) in calls with which == try.
*/
sel.which = V4L2_SUBDEV_FORMAT_TRY;
sel.target = V4L2_SEL_TGT_CROP;
sel.r = input->native_rect;
err = v4l2_subdev_call(input->camera, pad, set_selection, &sd_state, &sel);
v4l2_subdev_lock_state(input->try_sd_state);
err = v4l2_subdev_call(input->camera, pad, set_selection,
input->try_sd_state, &sel);
v4l2_subdev_unlock_state(input->try_sd_state);
if (err)
return;
goto unlock_act_sd_state;

sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
sel.target = V4L2_SEL_TGT_CROP;
sel.r = input->native_rect;
err = v4l2_subdev_call(input->camera, pad, set_selection, NULL, &sel);
err = v4l2_subdev_call(input->camera, pad, set_selection,
act_sd_state, &sel);
if (err)
return;
goto unlock_act_sd_state;

dev_info(input->camera->dev, "Supports crop native %dx%d active %dx%d binning %d\n",
input->native_rect.width, input->native_rect.height,
input->active_rect.width, input->active_rect.height,
input->binning_support);

input->crop_support = true;

unlock_act_sd_state:
if (act_sd_state)
v4l2_subdev_unlock_state(act_sd_state);
}

int atomisp_register_device_nodes(struct atomisp_device *isp)
Expand Down

0 comments on commit f66556c

Please sign in to comment.