| Message ID | 20240217112438.15240-10-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: atomisp: Changes for libcamera support | expand |
On Sat, Feb 17, 2024 at 1:25 PM Hans de Goede <hdegoede@redhat.com> wrote: > > The atomisp driver's Android heritage makes it weird in that > even though it uses MC + subdev-s it is designed to primarily > be controlled through its /dev/video# node. > > It implements s_input on /dev/video# to select which sensor to use, > while ignoring setup_link calls to enable a link to another sensor. > > Add support for selecting the active sensor the MC way by adding > setup_link support. ->link_setup() > The implementation is a bit convoluted due to the atomisp driver's > heritage. ... > #include "atomisp_common.h" > #include "atomisp_compat.h" > #include "atomisp_fops.h" > +#include "atomisp_ioctl.h" > #include "atomisp_internal.h" Hmm... Perhaps keep it ordered? ... > + mutex_lock(&isp->mutex); Side note: Are you planning to use cleanup.h at some point? > + ret = atomisp_pipe_check(&asd->video_out, true); > + if (ret == 0) > + asd->input_curr = i; > + mutex_unlock(&isp->mutex); > + > + return ret;
Hi, On 2/17/24 19:52, Andy Shevchenko wrote: > On Sat, Feb 17, 2024 at 1:25 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> The atomisp driver's Android heritage makes it weird in that >> even though it uses MC + subdev-s it is designed to primarily >> be controlled through its /dev/video# node. >> >> It implements s_input on /dev/video# to select which sensor to use, >> while ignoring setup_link calls to enable a link to another sensor. >> >> Add support for selecting the active sensor the MC way by adding >> setup_link support. > > ->link_setup() > >> The implementation is a bit convoluted due to the atomisp driver's >> heritage. > > ... > >> #include "atomisp_common.h" >> #include "atomisp_compat.h" >> #include "atomisp_fops.h" >> +#include "atomisp_ioctl.h" >> #include "atomisp_internal.h" > > Hmm... Perhaps keep it ordered? Yeah my bad, I intended to keep it ordered but I somehow got it wrong. this and the commit msg remark are both fixed in my personal tree now. > > ... > >> + mutex_lock(&isp->mutex); > > Side note: Are you planning to use cleanup.h at some point? Maybe. I have no objections against. For functions needing e.g. heap memory only locally it certainly makes sense. Regards, Hans > >> + ret = atomisp_pipe_check(&asd->video_out, true); >> + if (ret == 0) >> + asd->input_curr = i; >> + mutex_unlock(&isp->mutex); >> + >> + return ret; > >
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index 822fe7d129e2..17819d7586dd 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -30,6 +30,7 @@ #include "atomisp_common.h" #include "atomisp_compat.h" #include "atomisp_fops.h" +#include "atomisp_ioctl.h" #include "atomisp_internal.h" const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = { @@ -634,8 +635,59 @@ static void isp_subdev_init_params(struct atomisp_sub_device *asd) } /* media operations */ +static int atomisp_link_setup(struct media_entity *entity, + const struct media_pad *local, + const struct media_pad *remote, u32 flags) +{ + struct v4l2_subdev *sd = container_of(entity, struct v4l2_subdev, + entity); + struct atomisp_sub_device *asd = v4l2_get_subdevdata(sd); + struct atomisp_device *isp = asd->isp; + int i, csi_idx, ret; + + /* ISP's source is immutable */ + if (local != &asd->pads[ATOMISP_SUBDEV_PAD_SINK]) { + v4l2_err(sd, "Error pad %d does not support changing flags\n", + local->index); + return -EINVAL; + } + + for (csi_idx = 0; csi_idx < ATOMISP_CAMERA_NR_PORTS; csi_idx++) { + if (&isp->csi2_port[csi_idx].pads[CSI2_PAD_SOURCE] == remote) + break; + } + + if (csi_idx == ATOMISP_CAMERA_NR_PORTS) { + v4l2_err(sd, "Error cannot find CSI receiver for remote pad\n"); + return -EINVAL; + } + + /* Ignore disables, input_curr should only be updated on enables */ + if (!(flags & MEDIA_LNK_FL_ENABLED)) + return 0; + + for (i = 0; i < isp->input_cnt; i++) { + if (isp->inputs[i].camera == isp->sensor_subdevs[csi_idx]) + break; + } + + if (i == isp->input_cnt) { + v4l2_err(sd, "Error no sensor for CSI receiver %d\n", csi_idx); + return -EINVAL; + } + + mutex_lock(&isp->mutex); + ret = atomisp_pipe_check(&asd->video_out, true); + if (ret == 0) + asd->input_curr = i; + mutex_unlock(&isp->mutex); + + return ret; +} + static const struct media_entity_operations isp_subdev_media_ops = { .link_validate = v4l2_subdev_link_validate, + .link_setup = atomisp_link_setup, /* .set_power = v4l2_subdev_set_power, */ };
The atomisp driver's Android heritage makes it weird in that even though it uses MC + subdev-s it is designed to primarily be controlled through its /dev/video# node. It implements s_input on /dev/video# to select which sensor to use, while ignoring setup_link calls to enable a link to another sensor. Add support for selecting the active sensor the MC way by adding setup_link support. The implementation is a bit convoluted due to the atomisp driver's heritage. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../media/atomisp/pci/atomisp_subdev.c | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+)