History log of /linux-master/drivers/staging/media/atomisp/pci/atomisp_cmd.c
Revision Date Author Comments
# 26b53392 29-Dec-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Refactor sensor crop + fmt setting

There are 3 code-paths all of 3 which need to do:

1. Get try or active state
2. lock state
3. Call atomisp_set_crop()
4. Call v4l2_subdev_call(input->camera, pad, set_fmt, state, fmt)
5. unlock state

Change atomisp_set_crop into atomisp_set_crop_and_fmt() which does all of
this and change the 3 code-paths which need this to use the new
atomisp_set_crop_and_fmt() function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 592bb51d 28-Dec-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Adjust for v4l2_subdev_state handling changes in 6.8

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 <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f66556c1 28-Dec-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Adjust for v4l2_subdev_state handling changes in 6.8

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 <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 287fe160 13-Dec-2023 Laurent Pinchart <laurent.pinchart@ideasonboard.com>

media: v4l2-subdev: Turn .[gs]_frame_interval into pad operations

The subdev .[gs]_frame_interval are video operations, but they operate
on pads (and even on streams). Not only is this confusing, it causes
practical issues for drivers as the operations don't receive a subdev
state pointer, requiring manual state handling.

To improve the situation, turn the operations into pad operations, and
extend them to receive a state pointer like other pad operations.

While at it, rename the operations to .[gs]et_frame_interval at the same
time to match the naming scheme of other pad operations. This isn't
strictly necessary, but given that all drivers using those operations
need to be modified, handling the rename separately would generate more
churn for very little gain (if at all).

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> # for imx-media
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> # for tegra-video
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>


# 54566848 05-Oct-2023 Hans Verkuil <hverkuil-cisco@xs4all.nl>

staging: media: atomisp: improve unwinding

This fixes two smatch warnings:

drivers/staging/media/atomisp/pci/atomisp_cmd.c:2779 atomisp_cp_dvs_6axis_config() warn: missing unwind goto?
drivers/staging/media/atomisp/pci/atomisp_cmd.c:2878 atomisp_cp_morph_table() warn: missing unwind goto?

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
CC: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>


# b6da9e5a 18-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors

Remove the support to update a V4L2_CID_RUN_MODE run-mode control
on sensors when changing the atomisp run-mode or directly by calling
the custom ATOMISP_IOC_S_SENSOR_RUNMODE IOCTL.

No sensor drivers implement this and having custom controls / IOCTLs
is undesirable.

Even if there was such a control on sensors then userspace should directly
talk to the sensor v4l2-subdev, rather than relying on a custom IOCTLs
on the output /dev/video# node to pass this through to the senor.

Link: https://lore.kernel.org/r/20230619105212.303653-6-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# d86c33af 18-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Clamp width to max 1920 pixels when in ATOMISP_RUN_MODE_PREVIEW

The pipeline firmware-binaries used in previed mode have
ia_css_binary_xinfo.output.max_width set to 1920.

This causes ia_css_binary_find() to fail when trying to set a higher
resolution resulting in the dump_stack() call in ia_css_binary_find()
triggering and resulting in the try_fmt() or set_fmt() IOCTL failing.

Fix this by clamping the width to max 1920 when in preview mode.

Link: https://lore.kernel.org/r/20230619105212.303653-3-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# a2c77032 18-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove bogus asd == NULL checks

The asd is a sub-structure of the main driver data struct, so it is
never NULL. Drop the unnecessary NULL checks in a couple of places.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-media/533f6930-434a-45f3-afff-127003fa64c9@moroto.mountain/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>


# 930d333a 01-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Fix binning check in atomisp_set_crop()

The fmt / size passed to atomisp_set_crop() includes padding,
so the binning check should be against 1/2 of the native rect.
of the sensor, rather then 1/2 of the active rect.

This fixes binning not being used when using e.g. 800x600 on
a 1600x1200 sensor leading to a very small field of view.

Link: https://lore.kernel.org/r/20230601145858.59652-4-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 0f417dc1 01-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Make atomisp_enum_framesizes_crop() check resolution fits with padding

Now that atomisp_get_padding() takes minimum padding requirements
on BYT/ISP2400 into account, it is possible for a resolution which
fits in the active area of the sensor to not fit in the native area
once padding is added.

For example on the ov2680 which has a native resolution of 1616x1216
the max active resolution of 1600x1200 leaves 16x16 for padding which
meets the worst-case minimum padding requirement of 14x14 on BYT.

But after binning we are left with an native area of 808x608 and
an active area of 800x600. This leaves 8x8 for padding which is not
enough. So on BYT 800x600 is not a valid resolution (it could be
made by lots of cropping without binning but then the remaining
field of view is no good).

Modify atomisp_enum_framesizes_crop() to check the resolution +
padding fits in the native rectangle, removing 800x600 from
the list of valid modes on BYT.

Link: https://lore.kernel.org/r/20230601145858.59652-3-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 72f0ba79 01-Jun-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Take minimum padding requirement on BYT/ISP2400 into account

The main binary for the pipeline on BYT/ISP2400 has its
ia_css_binary_info.pipeline.left_cropping and .top_cropping fields
set to 12. So a minimum padding of 12 is required.

And for certain bayer-orders additional width / height padding is
necessary so that the ISP crop rectangle for the raw sensor data
can be used to change the bayer-order to the fixed bayer-order
supported by the debayer block.

Without the minmum required padding ia_css_ifmtr_configure() will fail
inside ifmtr_input_start_line() and/or ifmtr_start_column() because
it cannot set the ISP crop rectangle for the raw sensor data.

Fix this by making atomisp_get_padding() take the minimum padding
requirements into account on BYT/ISP2400 (CHT/ISP2401 does not seem to
need this).

Link: https://lore.kernel.org/r/20230601145858.59652-2-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 1e28b9e0 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Set crop before setting fmt

Some drivers which implement selections/crop only allow setting the format
with and height to either the crop rectangle width and height or to half
the crop rectangle width and height (binning). An example of such
a driver is the standard v4l2 ov5693 driver.

First set the crop rectangle to match the requested format
when trying or setting the sensor format, to match these drivers
expectations.

Link: https://lore.kernel.org/r/20230529103741.11904-20-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 3e399cf2 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Use selection API info to determine sensor padding

Using the selection / crop info to determine the padding values
to use with a specific resolution on specific sensor.

This allows e.g. automatically halving the padding when using
the max binned resolution and also ensures the right amount
of padding is used on models with 2 sensors with different
padding requirements.

Link: https://lore.kernel.org/r/20230529103741.11904-19-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 04eca831 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Add support for sensors which implement selection API / cropping

Sensor drivers which implement set_selection V4L2_SEL_TGT_CROP expect
v4l2_subdev_state.pads[pad].try_crop to have valid contents when calling
set_fmt with which == V4L2_SUBDEV_FORMAT_TRY since the crop-rectangle
may influence the available image size.

Just passing an uninitalized struct v4l2_subdev_pad_config from
the stack to set_fmt with which == V4L2_SUBDEV_FORMAT_TRY will result
in wrong results with such drivers.

Store a per sensor v4l2_subdev_pad_config and add a new
atomisp_init_sensor_crop() function to initialize this before
registering /dev/* nodes with userspace.

Sensor drivers which implement the selection API will allow
the atomisp to properly deal with the extra padding the ISP wants
on a per sensor basis instead of hardcoding this.
atomisp_init_sensor_crop() stores the native and active rects
of the sensor in preparation for using these for this.

Link: https://lore.kernel.org/r/20230529103741.11904-16-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 929eee2f 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Refactor atomisp_try_fmt() / atomisp_set_fmt()

There are a number of bugs in atomisp_try_fmt_cap() and atomisp_set_fmt():

1. atomisp_try_fmt_cap() uses atomisp_adjust_fmt() which adds the sensor
padding to the width passed to atomisp_adjust_fmt() to calculate
bytesperline. This is buggy for 2 reasons:

a) The width passed to atomisp_adjust_fmt() already contains
  the sensor padding.

b) The fmt returned by atomisp_try_fmt_cap() is the fmt outputted by
the ISP and the sensor padding applies to the input side of the ISP
not the output side. The output side of the ISP has its own padding /
pitch requirements which have nothing to do with the sensor.

Both these issues are fixed in this refactor by switching to
ia_css_frame_pad_width() to calculate the padding.

2. atomisp_set_fmt() takes the passed in bytesperline value without
doing any validation on it and then passes this unchecked value to
the configure_output() callback.

If bytesperline converted to pixels is > 1920 ia_css_binary_find()
will fail to find a valid binary for the preview pipeline triggering
a dump_stack_lvl() call inside ia_css_binary_find() and causing
atomisp_set_fmt() to fail.

This is fixed by making atomisp_set_fmt() call atomisp_try_fmt()
first which we override the userspace specified bytesperline with
the correct value.

Besides this bug there is also a bunch of weirdness and a lot of
duplication in the code:

1. atomisp_try_fmt_cap() adds the sensor padding itself but then
it gets substracted again in atomisp_adjust_fmt() not doing
the addition + substraction in the same place makes the code hard
to follow (weirdness).

2. atomisp_set_fmt() starts with basically an atomisp_try_fmt() call,
except that the only atomisp_try_fmt() caller: atomisp_try_fmt_cap()
adds the sensor padding itself rather than letting atomisp_try_fmt()
do this (duplication).

3. Both atomisp_try_fmt_cap() and atomisp_set_fmt() contain code to
lookup the bridge-format matching the requested pixelformat and
both will fallback to YUV420 if this is not set (duplication).

4. Both atomisp_try_fmt_cap() and atomisp_set_fmt() contain code to
fill in the passed in v4l2_pix_format struct (duplication).

Cleanup all of this (and fix the bugs mentioned above) by:

1. Adding a new atomisp_fill_pix_format() helper which properly uses
ia_css_frame_pad_width() to calculate bytesperline.

2. Move all sensor padding handling to atomisp_try_fmt() and
make atomisp_try_fmt() fill the passed in v4l2_pix_format struct.

3. This reduces atomisp_try_fmt_cap() to just a small wrapper around
atomisp_try_fmt().

4. Replace the DIY try_fmt code at the beginning of atomisp_set_fmt()
with atomisp_try_fmt(), this will also override/fix the bytersperline
passed by userspace.

5. Replace the DIY v4l2_pix_format filling at the end of atomisp_set_fmt()
with atomisp_fill_pix_format().

Link: https://lore.kernel.org/r/20230529103741.11904-15-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 9f221053 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Add input helper variable for isp->asd->inputs[asd->input_curr]

Passing 'isp->asd->inputs[asd->input_curr].foo' as argument to
various function calls is rather long.

Add a local input helper variable for this, so that the function
calls will fit on one line.

Link: https://lore.kernel.org/r/20230529103741.11904-13-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# e79fae3d 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove bogus fh use from atomisp_set_fmt*()

atomisp_set_fmt*() use a local v4l2_subdev_fh declared on the stack,
specifically they use fh.state which is never initialized so when
passing fh.state to atomisp_subdev_set_ffmt() / to
atomisp_subdev_set_selection() these functions are passing random
stack contents as a pointer.

The reason this works is because when the which parameter is
V4L2_SUBDEV_FORMAT_ACTIVE the passed in state is not used.

Remove the bogus fh usage and just pass NULL as state.

Link: https://lore.kernel.org/r/20230529103741.11904-12-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 7caa6570 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Simplify atomisp_subdev_set_selection() calls in atomisp_set_fmt()

With the atomisp_subdev_set_selection(sink-pad, V4L2_SEL_TGT_CROP, rect)
calls dropped. The first and last compount code blocks of the 3 code blocks
in the if (...) {} else if (...) {} else {} code setting the source-pad
V4L2_SEL_TGT_COMPOSE selection are the same.

The both set V4L2_SEL_TGT_COMPOSE to a rectangle with the same dimensions
as f->fmt.pix.height.

Remove the else {} block at the end, drop the second if and prepend
the first if condition with "!second-if-condition ||" to remove
the code duplication.

Link: https://lore.kernel.org/r/20230529103741.11904-10-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# af330c96 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove redundant atomisp_subdev_set_selection() calls from atomisp_set_fmt()

atomisp_subdev_set_selection(sink-pad, V4L2_SEL_TGT_CROP, rect)
ignores the passed in rect, using the width and height from the last
atomisp_subdev_set_ffmt(ATOMISP_SUBDEV_PAD_SINK, ffmt) call instead.

The atomisp_subdev_set_ffmt() call done by atomisp_set_fmt_to_snr()
already propagates the sink ffmt changes to V4L2_SEL_TGT_CROP
(this is what allows atomisp_set_fmt() to get the isp_sink_crop in
the first place).

Remove the redundant atomisp_subdev_set_selection(sink-pad, ...)
calls.

Note the removed aspect ratio correction in the last else block is
is already done by atomisp_subdev_set_selection() itself when
setting V4L2_SEL_TGT_COMPOSE on the source-pad.

Link: https://lore.kernel.org/r/20230529103741.11904-9-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 503bf309 29-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove a bunch of sensor related custom IOCTLs

Remove a bunch of sensor related custom IOCTLs because:

1. They are custom IOCTLs and all custom IOCTLs should be removed
2. Userspace should directly talk to the sensor v4l2-subdev, rather
then relying on ioctl-s on the output /dev/video# node to pass
through ioctl-s to the senor
3. Some of these rely on the atomisp specific camera_mipi_info struct
which is going away as we are switching to using standard v4l2
sensor drivers
4. In the case of ATOMISP_IOC_S_EXPOSURE_WINDOW this was using the
v4l2-subdev set_selection API in an undocumented atomisp custom way

Link: https://lore.kernel.org/r/20230529103741.11904-8-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 4d985809 16-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Allow camera_mipi_info to be NULL

camera_mipi_info is an atomisp / atomisp_gmin_platform specific struct,
allow mipi_info pointers to be NULL.

This is a preparation patch for making atomisp work with
standard v4l2 sensor drivers.

Link: https://lore.kernel.org/r/20230518153733.195306-9-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 64be2070 16-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Rename __get_mipi_port() to atomisp_port_to_mipi_port()

Rename __get_mipi_port() to atomisp_port_to_mipi_port(), this is not a
private (not static) function so its name should be properly prefixed.

While at is also cleanup the weird handling of ATOMISP_CAMERA_PORT_TERTIARY
this seems to be a left over from when the driver also supported CSI
receivers with only 2 ports, but those are not supported by the current
code base, so this can be cleaned up now.

Link: https://lore.kernel.org/r/20230518153733.195306-5-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# ae39964d 12-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove in_reset argument from atomisp_css_start()

The in_reset argument to atomisp_css_start() is only ever true
in atomisp_assert_recovery_work(), drop the argument and move
the special reset handlig to atomisp_assert_recovery_work().

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# e5fab487 12-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove unused css_pipe_id argument from atomisp_css_[start|stop]()

The css_pipe_id argument pass to atomisp_css_[start|stop]() is
not used, drop it.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# bcc90bb3 11-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Simplify atomisp_isr() and recovery_work()

Both atomisp_isr() and recovery_work() now have a combination of:

1. "if (!isp->asd.streaming) goto out;" code at the top
2. "if (sp->asd.streaming) {}" blocks in the body which are jumped over
by the goto out.

This means that the "if (sp->asd.streaming) {}" blocks are always
executed if they are not jumped over by the goto.

Remove the unnecessary "if (sp->asd.streaming)" checks and
re-indent the code.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 3fb3cd02 11-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_streaming_count()

atomisp_streaming_count() is just an alias for isp->asd.streaming now,
replace it with directly checking that and remove the helper.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# b19f8ef5 11-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove no longer used atomisp_css_flush()

Remove the no longer used atomisp_css_flush() function and merge
atomisp_assert_recovery_work() and __atomisp_css_recover() into
a single function.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 0f072c1e 11-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Turn asd->streaming state tracker into a bool

The ATOMISP_DEVICE_STREAMING_STOPPING pipe state comes from when we still
had continuous mode. This would be set when streaming from both capture +
preview devnodes when 1 of the 2 streams has been stopped and the driver
was waiting for the other stream to get stopped too.

With continuous mode gone the stopping state is no longer necessary and
asd->streaming can be changed to a bool.

Note that atomisp_assert_recovery_work() would still temporarily
set streaming to stopping, but it does so with the isp->mutex held and
changes streaming to either enabled or disabled before releasing
the mutex, so none of the consumers which care about the difference
ever see the stopping state.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 820a1338 11-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove isp_timeout flag

isp_timeout only ever gets set in __atomisp_css_recover() and then
immediately gets cleared again after calling atomisp_reset().

All this happens with isp->mutex held.

The only consumer of isp->isp_timeout is atomisp_stop_streaming(), which
also holds isp->mutex and which is *not* called by atomisp_reset().

Since both hold isp->mutex and since __atomisp_css_recover() clears
isp_timeout before releasing the mutex, atomisp_stop_streaming() can
never see isp_timeout being true, so just remove the flag.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 8fd3c9a4 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove 1 line atomisp_flush_bufs_and_wakeup() helper

After recent changes this now is just a wrapper around
atomisp_flush_video_pipe(). Make its single caller call
atomisp_flush_video_pipe() directly and drop the helper.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# e4454e06 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove source_pad parameter from functions and structs

Now that there is only 1 source-pad for an asd there is no need
to have a parameter for this in various places.

Remove the source_pad function parameter and
atomisp_sub_device.capture_pad data member.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# aca8a1df 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Rename video_out_preview to video_out

Now that we have only 1 /dev/video# node for output for all different
run-modes (with only 1 run-mode at a time) using video_out_preview for
the remaining atomisp_pipe does not properly reflect that this is
*the* output pipe. Fo the following renames to fix the naming:

s/video_out_preview/video_out/
s/ATOMISP_SUBDEV_PAD_SOURCE_PREVIEW/ATOMISP_SUBDEV_PAD_SOURCE/

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 26dcabc0 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Drop atomisp_is_vf_pipe()

Since there only is one /dev/video# node now (no more continuous mode),
there are now no longer separate main capture + view-finder pipes.

We are now always on the main pipe, so atomisp_is_vf_pipe() should
always return false now. Drop any checks using it, replacing them
with the code-path for a false return.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# d957824b 06-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Register only 1 /dev/video# node

Now that we no longer support continuous mode and thus no longer support
streaming from 2 /dev/video# nodes at the same time, there is no need
to have a separate /dev/video# node (+ matching v4l2-subdev pads)
for each run-mode.

Keep the video_out_preview /dev/video0 device and remove
the video_out_video_capture / video_out_vf / video_out_capture
video-devices (atomisp_pipe-s) and also remove the matching
ATOMISP_SUBDEV_PAD_SOURCE_VIDEO / ATOMISP_SUBDEV_PAD_SOURCE_VF /
ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE source-pads.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# ea360037 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Replace source-pad checks with run-mode checks

Currently atomisp behavior is determined by a mix of which /dev/video# node
(which isp-subdev source-pad) is opened + which run-mode is set.
With various combinations not being allowed and likely leading to crashes
due to lack of error checking.

Now that we no longer support continuous mode and thus no longer support
streaming from 2 /dev/video# nodes at the same time, there is no need
to have a separate /dev/video# node for each run-mode. Instead the plan is
to support the 3 different run-modes on a single /dev/video# node.
Since we are moving to a single isp-subdev source-pad, the behavior should
then be solely and consistently be defined by the run-mode.

Replace various source-pad checks with run-mode checks in preparation for
moving to a single source-pad. In some places the new run-mode checks
overlap with existing run-mode checks and the checks are folded together
into a single check.

This removes handling of the ATOMISP_SUBDEV_PAD_SOURCE_VF source-pad,
this source-pad was only useful for continuous mode, for which support has
been removed.

Note that currently the only run-mode which we actually have been able to
get to work is the video-capture with scaler aka preview mode and as such
that is also the only run-mode tested. This patch is intended to preserve
the current (known to not work 100%) behavior of the other run-modes, so
that those maybe can be enabled later.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 1b9a80a4 07-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove isp->need_gfx_throttle field

Remove the isp->need_gfx_throttle field it is only ever set and
never read.

Also the code setting it is broken, comparing run_mode->val to
ATOMISP_SUBDEV_PAD_SOURCE_VIDEO which are not of the same type /
not part of the same enum.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 649920af 06-May-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove res_overflow parameter from atomisp_try_fmt()

The only remaining caller of atomisp_try_fmt() always passes NULL
for the res_overflow parameter. Drop it and simplify atomisp_try_fmt().

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 2f2fce05 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove struct atomisp_sub_device index field

Remove the struct atomisp_sub_device.

This was used for 2 things:

1. In dev_dbg() messages
2. To set the name of the v4l2_subdev for each struct atomisp_sub_device

Now that only 1 subdev is used neither is useful anymore. Remove the _%d
postfix from the v4l2_subdev name and remove the logging of the asd index
from the dev_dbg() messages.

In case of the atomisp_s_input() check to see if an input/sensor has
already been assigned to another subdev the entire check no longer
makes sense, so instead of changing the dev_err() message there just
drop the entire check.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 401e0e41 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Drop support for streaming from 2 sensors at once

With support for depth mode gone there really is no need to support
streaming from 2 sensors at once. As discussed and acked on the list
(see Link tag) it is desirable to drop support for this since it
involves quite a lot of special handling / hacks in the code.

This initial commit limits itself to a minimum set of changes to switch
to 1 struct atomisp_sub_device / 1 stream. Further commits will actually
remove / cleanup much of the special handling.

Likewise this initial commit also deliberately skips the opportunity to
turn some multi-line statements into single-line statements, so as to
keep the diff small / easier to review.

Link: https://lore.kernel.org/linux-media/5309d845-063b-6dd9-529d-0f82654290f2@redhat.com/

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# df1697fa 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_try_fmt() call from atomisp_set_fmt()

atomisp_set_fmt() first calls:

v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
pad, set_fmt, ...);

with v4l2_subdev_format.which set to V4L2_SUBDEV_FORMAT_ACTIVE and then
reduces width + height by padding_w / padding_h followed by calling
atomisp_try_fmt() which then sets res_overflow if the requested resolution
is larger then the sensor's max resolution, but that will never happen
since we are requesting for the result of a set_fmt call minus padding,
so res_overflow will always be false.

And when called with a res_overflow parameter, combined with there
being no overflow then atomisp_try_fmt() does not change the passed
in width/height.

Except when using JPEG or CUSTOM_M10MO_RAW as pixelformat, then
atomisp_try_fmt() does change the width/height but in that case
atomisp_set_fmt actually restores the old widht/height which it
has saved before the atomisp_try_fmt() call. So this too is a no-op.

TL;DR: The atomisp_try_fmt() call in atomisp_set_fmt() is a no-op,
remove it.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 52172e9e 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove snr_mbus_fmt local var from atomisp_try_fmt()

snr_mbus_fmt is a local variable pointing to a substruct of another local
variable which really just makes the code harder to read / follow,
so drop it.

And likewise also drop the stream_info local variable.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# fa8730b0 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove custom V4L2_CID_FMT_AUTO control

The custom V4L2_CID_FMT_AUTO control, which defaults to true
controls whether the atomisp driver will automatically configure
the sensor and ISP settings to match.

This is necessary to make normal v4l2 apps to work. We do eventually
want to add libcamera support, but even then moving this to userspace
does not give any added value, while breaking classic v4l2 apps.

Moreover we really don't know all the exact pipeline constraints,
so moving this to userspace/libcamera will be tricky thus it is best
to keep the current in kernel code for this and make the kernel
unconditionally do this.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# b3713bca 05-Mar-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove continuous mode related code from atomisp_set_fmt()

Remove code to check / adjust code between video and preview /
capture and view-finder nodes now that we no longer support
continuous mode and this no longer support streaming from 2
/dev/video# nodes at the same time.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# edd076fe 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove online_process setting

online_process is always true, remove it.

Link: https://lore.kernel.org/r/20230221145906.8113-11-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 738ac75e 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove yuvpp_mode

yuvpp_mode is never set, so it always is false remove it.

Link: https://lore.kernel.org/r/20230221145906.8113-10-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 9c2f3b8f 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove ATOMISP_USE_YUVPP()

ATOMISP_USE_YUVPP() always returns false, so remove it and remove any
code-paths which only run when it would return true.

Link: https://lore.kernel.org/r/20230221145906.8113-9-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# bbfd899f 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove unused SOC_CAMERA, XENON_FLASH and FILE_INPUT subdev types

Nothing registers subdevs with the SOC_CAMERA, XENON_FLASH and FILE_INPUT
types, remove the code for these.

Link: https://lore.kernel.org/r/20230221145906.8113-8-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# c5449bab 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_get_metadata_type()

atomisp_get_metadata_type() always returns ATOMISP_MAIN_METADATA,
replace its uses with ATOMISP_MAIN_METADATA and remove it.

Link: https://lore.kernel.org/r/20230221145906.8113-7-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 7821a603 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove crop_needs_override from atomisp_set_fmt()

Remove the crop_needs_override local helper variable from
atomisp_set_fmt(), as it always is true now.

Link: https://lore.kernel.org/r/20230221145906.8113-5-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 9fd4b208 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove delayed_init related code

After the continues-mode removal the delayed-work never gets queues
remove all the related code.

Link: https://lore.kernel.org/r/20230221145906.8113-4-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 6048fc1c 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove continuous mode support

Continues mode is a special mode where 2 /dev/video devices can be active
at the same time. Either the video-preview + video nodes or the
viewfinder (for still capture) + capture nodes.

For the video-preview + video-recording case modern userspace will
use a single stream multiplexed by pipewire.

The still-capture case is extra special only starting the preview
stream and then relying on a custom ATOMISP_IOC_S_CONT_CAPTURE_CONFIG
ioctl to set things up followed by a second stream on to capture
the amount of configured still pictures. While running the sensor
at full resolution all the time. This case too is better handled
with dma-buf + GPU downscaling for the view-finder rather then all this
custom special code. Besises this the ioctl expects a bunch of special
non error checked conditions to be met otherwise things will crash/hang.

The continues mode also involves a special cases all over the code
getting in the way of further cleanups and simplifying the code to
using just 1 /dev/video# node. So lets remove it and the
related custom ATOMISP_IOC_S_CONT_CAPTURE_CONFIG ioctl.

Link: https://lore.kernel.org/linux-media/ea81b17b-7d1f-a5e1-11dd-04db310e1e50@redhat.com/
Link: https://lore.kernel.org/r/20230221145906.8113-3-hdegoede@redhat.com

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# d33b94c0 21-Feb-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove depth-mode support

Remove support for depth mode. This is a special mode where 2 streams
(from 2 different sensors) can be setup and then starting/stopping
1 will automatically also start/stop the other.

Like many of these special features I'm pretty sure that if the queue
setup is not done exactly right things will crash and there is no error
checking for this.

This seems to be for stereoscopic vision and the only known hw which
actually supports this is the Intel Aero board/SDK, all other 1000+
BYT/CHT models don't need this.

This false outside of the standard webcam use scenario which we are
trying to get working and this involves a bunch of hacks / special
exceptions all over the code, so lets remove this.

Link: https://lore.kernel.org/linux-media/ea81b17b-7d1f-a5e1-11dd-04db310e1e50@redhat.com/
Link: https://lore.kernel.org/r/20230221145906.8113-2-hdegoede@redhat.com

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# a1e25987 15-Feb-2023 Laurent Pinchart <laurent.pinchart@ideasonboard.com>

media: Fix indentation issues introduced by subdev-wide state struct

Commit 0d346d2a6f54 ("media: v4l2-subdev: add subdev-wide state struct")
applied a large media tree-wide change produced by coccinelle. It was so
large that a set of identical indentation issues went unnoticed during
review. Fix them.

While at it, and because it's easy to review both changes together, add
a trailing comma for the last (and only) struct member initialization of
the related structures, to avoid future changes should new fields need
to be initialized.

Fixes: 0d346d2a6f54 ("media: v4l2-subdev: add subdev-wide state struct")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>


# edcb14e5 22-Jan-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Stop overriding padding w/h to 12 on BYT

atomisp_set_fmt() first does:

v4l2_fill_mbus_format(&vformat.format, ...);
vformat.format.height += padding_h;
vformat.format.width += padding_w;

ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, pad,
set_fmt, NULL, &vformat);
if (ret)
return ret;

f->fmt.pix.width = vformat.format.width - padding_w;
f->fmt.pix.height = vformat.format.height - padding_h;

this happens with the original padding w/h = 16 values and then later
on it calls:

ret = atomisp_set_fmt_to_snr(vdev, &s_fmt,
f->fmt.pix.pixelformat, padding_w,
padding_h, dvs_env_w, dvs_env_h);

Which repeats the above structure. If at that point padding w/h are
changed to 12 then it will now request a different output-size of
the sensor driver.

The sensor drivers so far have actually been ignoring this since they use
v4l2_find_nearest_size() on a fixed resolution list and the nearest
resolution will be the one from the earlier calls where padding w/h
was 16.

But there really is no reason for sensor drivers to use a fixed
resolution list. They make lower resolutions using cropping so they
can make any resolution as long as width/height are even numbers.

Dropping the fixed-resolution list limit from sensors on BYT results
in trying to start streaming failing because the resolution set to
the sensor now no longer matches with the resolution used during
the initial part of the configuration done by atomisp_set_fmt().

Drop the BYT specific overriding of the padding_w/h to 12, so that
the padding in the first and second s_fmt calls made to the sensor
matches, to fix stream start failing when the fixed resolution list
is dropped.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f629e386 22-Jan-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Drop ffmt local var from atomisp_set_fmt()

ffmt is a local variable pointing to a substruct of another local
variable which really just makes the code harder to read / follow,
so drop it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 7f048750 12-Dec-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove custom ATOMISP_IOC_G_SENSOR_MODE_DATA ioctl

This ioctl returns a number of fixed sensor parameters +
a number of mode-specific parameters.

With libcamera these fixed parameters are instead stored in a table
with sensor-name to parameters mappings (camera_sensor_properties.cpp);
and the variable parameters can be derived from the set fmt.

So this custom ioctl is not necessary; and it currently has no users.

Remove the ioctl and all the sensor drivers xxxx_get_intg_factor()
helpers which return this info.

This is part of a patch-series which tries to remove atomisp specific /
custom code from the sensor drivers, with as end goal to make the atomisp
drivers regular camera sensor drivers.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 8b3332b2 11-Dec-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove custom ATOMISP_IOC_ISP_MAKERNOTE ioctl

This ioctl simply returns a couple of fixed sensor parameters.

With libcamera these fixed parameters are instead stored in a table
with sensor-name to parameters mappings (camera_sensor_properties.cpp),
so this custom ioctl is not necessary; and it currently has no users.

Remove the ioctl and also remove the custom v4l2-ctrls underpinning
the ioctl.

This is part of a patch-series which tries to remove atomisp specific /
custom code from the sensor drivers, with as end goal to make the atomisp
drivers regular camera sensor drivers.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 21b86873 21-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_sw_contex struct

Remove the atomisp_sw_contex struct, it has only 1 member: running_freq,
instead store running_freq directly.

While at it also change running_freq from an int to an unsigned int,
all values stored in it are unsigned and it is compared to the also
unsigned new_freq variable.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 2e18e118 21-Jan-2023 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Fix WARN() when the vb2 start_streaming callback fails

The videobuf2-core expects buffers to be put back in the queued state
when the vb2 start_streaming callback fails. But the atomisp
atomisp_flush_video_pipe() would unconditionally return them to the core
in an error state.

This triggers the following warning in the videobuf2-core:

drivers/media/common/videobuf2/videobuf2-core.c:1652:
/*
* If done_list is not empty, then start_streaming() didn't call
* vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED) but STATE_ERROR or
* STATE_DONE.
*/
WARN_ON(!list_empty(&q->done_list));

Fix this by adding a state argument to atomisp_flush_video_pipe() and use
VB2_BUF_STATE_QUEUED as state when atomisp_start_streaming() fails.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 2527b8d1 14-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_ospm_dphy_up()/_down() functions

atomisp_ospm_dphy_up() is an empty function now and
atomisp_ospm_dphy_down() contains a couple of checks + goto done
statements which don't matter since the function always ends up at
the done label regardless and then it does 1 pci-config write.

Move the single pci-config write directly to atomisp_power_off()
and remove the atomisp_ospm_dphy_up()/_down() functions.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 5317baa0 14-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Move calling of css_[un]init() to power_on()/_off()

atomisp_css_init() is always called after calling atomisp_power_on()
either directly or through getting a runtime-pm reference.

Likewise atomisp_css_uninit() is always called after calling
atomisp_power_off().

Move the call site of these 2 functions to inside atomisp_power_on() /
atomisp_power_off() to make this more explicit.

Note this makes atomisp_reset() also set isp_fatal_error on
atomisp_power_on() errors, where as before it only did this on
atomisp_css_init() errors. This behavior change is for the better,
since power-on failing is pretty fatal too.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 6736a68f 14-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove duplication between runtime-pm and normal-pm code

atomisp_suspend() contains a 1:1 copy of atomisp_runtime_suspend() and
the same goes for the resume() functions.

Rename the runtime functions to atomisp_power_on()/_off() and use these
as runtime-pm handlers as well as helper in other places to remove
the code duplication.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f1219d9e 14-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove sw_contex.power_state checks

Remove the unnecessary sw_contex.power_state checks:

1. atomisp_freq_scaling() and atomisp_stop_streaming() only run when the
ISP is powered up through runtime-pm, so the checks are not necessary

2. atomisp_mrfld_pre_power_down() and atomisp_runtime_resume() are only
called through the driver-core pm handling code which already
guarantees they are not called when already powered down / up.

3. atomisp_isr() also checks isp->css_initialized which only gets set
by atomisp_css_init() which runs *after* powering up the ISP and which
gets cleared by atomisp_css_uninit() *before* powering down the ISP.

So all the checks are unnecessary, remove them as well as the
sw_contex.power_state field itself.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 586ef0c6 14-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_css_suspend()/_resume()

atomisp_css_suspend() is a 1:1 copy of atomisp_css_uninit() and
atomisp_css_resume() is a 1:1 copy of atomisp_css_init().

Remove the 2 copies and have their one caller just call
atomisp_css_uninit() / atomisp_css_init() instead.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 24aba582 11-Nov-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove double atomisp_mrfld_power_down()/_up() calls from atomisp_reset()

atomisp_reset() calls atomisp_mrfld_power_down() after calling
atomisp_runtime_suspend(), which already calls
atomisp_mrfld_power_down() itself.

And the some goes for atomisp_runtime_resume() / atomisp_mrfld_power_up().

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# eb314d87 16-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove __atomisp_get_pipe() helper

Remove the complicated __atomisp_get_pipe() helper, atomisp_buf_done()
only needs the pipe pointer in cases where it has a frame, so we can
simply get the pipe from the frame using the vb_to_pipe() helper.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 4bdab809 15-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Make it possible to call atomisp_set_fmt() without a file handle

To fix atomisp_queue_setup() sometimes failing it needs to be able to call
atomisp_set_fmt(), but atomisp_queue_setup() (VIDIOC_REQBUFS) does not get
passed a file handle by the videobuf2 core.

Partly revert commit b3be98f984d4 ("media: atomisp: Remove a couple of not
useful function wrappers") so that atomisp_set_fmt() can be used
without a file handle.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# cb48ae89 08-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Convert to videobuf2

Convert atomisp to use videobuf2.

This fixes mmap not working and in general moving over to
the more modern videobuf2 is a good plan.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# b7465077 08-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Add an index helper variable to atomisp_buf_done()

With the videobuf2 conversion accessing the index of a frame is going
to become more involved and writing this out each time is undesired.

Add an 'i' helper variable for the index and assing this once as
preparation for the videobuf2 conversion.

This also makes use of the new rules wrt max line-lengths in the kernel
to avoid breaking up a bunch of lines. Not breaking these lines results
in better readable code (IMHO).

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 89f9829c 08-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Also track buffers in a list when submitted to the ISP

Instead of using an integer to keep count of how many buffers have
been handed over to the ISP (buffers_in_css) move buffers handed
over to the ISP to a new buffers_in_css list_head so that we can
easily loop over them.

This removes the need for atomisp_flush_video_pipe() to loop over
all buffers and then (ab)use the state to figure out if they
were handed over to the ISP.

Since the buffers are now always on a list when owned by the driver
this also allows the buffer_done path on flush vs normal completion
to be unified (both now need a list_del()) and this common code can
now be factored out into a new atomisp_buffer_done() helper.

This is a preparation patch for moving the driver over to
the videobuf2 framework.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# b895be29 07-Oct-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove unused atomisp_buffers_queued[_pipe] functions

The atomisp_buffers_queued[_pipe] functions are not used anywhere,
remove them.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 301fd0dd 24-Oct-2022 Hans Verkuil <hverkuil-cisco@xs4all.nl>

atomisp: fix potential NULL pointer dereferences

The asd pointer is used in lockdep_assert_held() before the check if
asd is NULL.

This fixes two smatch warnings:

drivers/staging/media/atomisp/pci/atomisp_cmd.c:3697 atomisp_handle_parameter_and_buffer() warn: variable dereferenced before check 'asd' (see line 3695)
drivers/staging/media/atomisp/pci/atomisp_cmd.c:3779 atomisp_set_parameters() warn: variable dereferenced before check 'asd' (see line 3777)

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>


# 07eb5659 13-Oct-2022 Colin Ian King <colin.i.king@gmail.com>

media: atomisp: Fix spelling mistake "mis-match" -> "mismatch"

There are a few spelling mistakes in dev_err messages. Fix them.

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>


# 1e32f6ea 09-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_source_pad_to_stream_id()

atomisp_source_pad_to_stream_id() returns ATOMISP_INPUT_STREAM_GENERAL
unconditionally now. Drop it and directly use ATOMISP_INPUT_STREAM_GENERAL
in its callers.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# cf2e0516 04-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove const/fixed camera_caps

The code checks a camera_caps struct in various places, but this always
points to the same const camera_caps struct.

Remove the checks, keeping the code paths which would be taken with
the fixed camera caps struct still in place and remove the camera_caps
struct itself.

Note this completely removes atomisp_pause_buffer_event() because that
only ever does something if camera_caps.sensors[0].is_slave is true and
that never is true.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# e208848c 04-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Make atomisp_set_raw_buffer_bitmap() static

atomisp_set_raw_buffer_bitmap() is only used in atomisp_cmd.c,
make it static. Unfortunately this still requires a forward
declaration (the function cannot be moved easily).

Still this will at least make it obvious to anyone reading the code
that the function is not used elsewhere.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# d7306735 04-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove a couple of not useful function wrappers

The __atomisp_reqbufs(), __atomisp_streamoff() are 1:1 wrappers for
the non __ prefixed functions now, drop these wrappers.

The atomisp_s_fmt_cap() wrapper is almost a 1:1 wrapper for
atomisp_set_fmt() adjust the latter to have the right function
prototype and drop the wrapper.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 0ecc5236 03-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Move atomisp_streaming_count() check into __atomisp_css_recover()

Both callers of __atomisp_css_recover() check atomisp_streaming_count()
first, move the check into __atomisp_css_recover().

And __atomisp_css_recover() already calls lockdep_assert_held(&isp->mutex),
so drop that from atomisp_css_flush().

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 93d3fb35 03-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove watchdog timer

The watchdog timer code to recover from the ISP getting stuck has several
major issues:

1. There is no way to do fault injection and normally the ISP does not
get stuck, so is it is impossible to test it.

2. It in essence just stops all streams, resets the ISP and then brings
everything back up. Userspace can easily do this itself by using a
timeout on dqbuf and then closing (which causes a poweroff) +
re-opening the device. Doing this in userspace (if it ever turns out
to be necessary) greatly simplifies the kernel code and in general
will be a more robust solution.

Even just a quick look at the code finds several more issues:

3. The need to sync-cancel the timers + work on streamoff requires
isp->mutex to be dropped halfway during the ioctl opening all sorts of
races.

4. The atomisp code supports setting up 2 pipelines, streaming from
two sensors at the same time. But there is only a single wdt_work
and stopping one of the 2 streams will cancel the timers + work,
stopping the wdt even though the other stream might still be running.

5. In case atomisp_css_flush() the sync cancel is done while keeping
isp->mutex locked, causing a deadlock when racing with wdt_work which
also takes isp->mutex.

6. Even though the watchdog is purely a software/driver thing which
just checkes that new frames keep coming in, there are 2 completely
different implementations for the ISP2400/ISP2401 which is not
necessary at all.

So all in all I believe that it is better to just remove the current
watchdog implementation. Fixing all the issues with the current
implementation will be so much work, that if it turns out that we do
need something like this then doing a clean re-implementation from
scratch will be better anyways.

wdt_work was also (ab)used to reset the ISP after the firmware signalled
an fw-assert error through the irq, add a new assert_recover_work to
replace this.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f315c1ac 03-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Add atomisp_pipe_check() helper

Several of the ioctl handlers all do the same checks
(isp->fatal_error and asd->streaming errors) add
an atomisp_pipe_check() helper for this.

Note this changes the vidioc_s_fmt_vid_cap and vidioc_s_input handlers
to now reject calls made while asd->streaming==STOPPING. This fixes
a possible race where one thread can make this ioctls while
vidioc_streamoff is running from another thread and it has
temporarily released isp->mutex to kill the watchdog timers / work.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 0d51573d 02-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove asd == NULL checks from ioctl handling

At probe time isp_subdev_init_entities() sets pipe->asd to a non NULL
value for all four (preview/vf/capture/capture_video) pipes by calling
atomisp_init_subdev_pipe() for all 4 pipes.

So it can never be NULL. Remove the redundant NULL checks.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 2468083f 02-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Fix locking around asd->streaming read/write

For reading / writing the asd->streaming enum the following rules
should be followed:

1. Writers of streaming must hold both isp->mutex and isp->lock.
2. Readers of streaming need to hold only one of the two locks.

Not all writers where properly taking both locks this fixes this.

In the case of the readers, many readers depend on their caller
to hold isp->mutex, add asserts for this

And in the case of atomisp_css_get_dis_stat() it is called with
isp->mutex held, so there is no need to take the spinlock just
for reading the streaming value.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f10fc179 01-Sep-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Use a normal mutex for the main lock

There is no reason for atomisp to use a rt_mutex instead of a normal
mutex, so switch over to a normal mutex.

All the changes in this patch are just s/rt_mutex/mutex/.

This is a preparation patch for switching the ioctl locking over
to using the video_dev.lock member so that the v4l2-core takes
care of the locking.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# af69562a 28-Aug-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove the ACC device node

The ACC /dev/video# device node uses a struct video_device embedded in
an atomisp_acc_pipe struct instead of in an atomisp_video_pipe struct.
Yet it uses the same file-ops and ioctl-ops even though it does not have
a videobuf queue, which makes e.g. the mmap fop nonsense.

Worse the only file-ops / ioctls which differentiate between
the 2 types and correctly do container_of on the right type
are the open/release fops and the vidioc_default handler.

The mmap and poll fops and *all* other ioctl handlers unconditionally
do container_of on the passed in struct video_device blindly assuming
they are dealing with the one embedded in the atomisp_video_pipe struct.

This makes it trivial for userspace to cause all sort of undefined behavior
by calling mmap, poll or the other ioctls on the ACC device node!

Presumably the use of the ACC device node was to allow making the special
ioctls to load custom programs while the other /dev/video# nodes were
already open, since the /dev/video# nodes can currently all be opened
only once (which needs to be fixed).

commit 4bbca788b6eb ("media: atomisp: remove private acceleration ioctls")
has removed the custom ATOMISP_ACC_* ioctls, so there no longer is any
reason to keep the ACC device node.

As explained above its presence can easily cause the kernel to crash,
so remove the ACC device node and the code for handling it.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 79adb947 27-Aug-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove never set file_input flag

After the file-injection support removal the file_input flag is
always false.

Remove the flag and replace any code checking it with the code-path
for when it is false.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 1ace82c7 27-Aug-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Remove atomisp_file_fops and atomisp_file_ioctl_ops

After the file-injection support removal, atomisp_video_pipe->type never
is V4L2_BUF_TYPE_VIDEO_OUTPUT anymore, so the V4L2_BUF_TYPE_VIDEO_OUTPUT
support path in atomisp_video_init() is never hit and this path is
the only user of atomisp_file_fops and atomisp_file_ioctl_ops.

Remove atomisp_file_fops and atomisp_file_ioctl_ops and all of
the functions which are only referenced by these ops structs.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 4d3aafb9 13-Aug-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: Fix VIDIOC_TRY_FMT

atomisp_try_fmt() calls the sensor's try_fmt handler but it does
not copy the result back to the passed in v4l2_pix_format under
some circumstances.

Potentially returning an unsupported resolution to userspace,
which VIDIOC_TRY_FMT is not supposed to do.

atomisp_set_fmt() also uses atomisp_try_fmt() and relies
on this wrong behavior. The VIDIOC_TRY_FMT call passes NULL for
the res_overflow argument where as the atomisp_set_fmt() call
passes non NULL.

Use the res_overflow argument to differentiate between the 2 callers
and always propagate the sensors result in the VIDIOC_TRY_FMT case.

This fixes the resolution list in camorama showing resolutions like e.g.
1584x1184 instead of 1600x1200.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# f064f018 03-Jul-2022 Li zeming <zeming@nfschina.com>

media: staging/media/atomisp/pci/atomisp: Fix typo in string

Remove the repeated ',' from string

Link: https://lore.kernel.org/linux-media/20220704022807.3215-1-zeming@nfschina.com
Signed-off-by: Li zeming <zeming@nfschina.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 09b204eb 13-Apr-2022 Xiaomeng Tong <xiam0nd.tong@gmail.com>

media: [PATCH] pci: atomisp_cmd: fix three missing checks on list iterator

The three bugs are here:
__func__, s3a_buf->s3a_data->exp_id);
__func__, md_buf->metadata->exp_id);
__func__, dis_buf->dis_data->exp_id);

The list iterator 's3a_buf/md_buf/dis_buf' will point to a bogus
position containing HEAD if the list is empty or no element is found.
This case must be checked before any use of the iterator, otherwise
it will lead to a invalid memory access.

To fix this bug, add an check. Use a new variable '*_iter' as the
list iterator, while use the old variable '*_buf' as a dedicated
pointer to point to the found element.

Link: https://lore.kernel.org/linux-media/20220414041415.3342-1-xiam0nd.tong@gmail.com
Cc: stable@vger.kernel.org
Fixes: ad85094b293e4 ("Revert "media: staging: atomisp: Remove driver"")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 54c15ddd 15-Jun-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: remove atomisp_is_acc_enabled()

atomisp_is_acc_enabled() always returns false now, remove it.

Link: https://lore.kernel.org/linux-media/20220615205037.16549-24-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# b717c517 15-Jun-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: asc.acc.pipeline is always NULL

With the removal of the ACC ioctls and atomisp_acc.c asc.acc.pipeline
never gets set, so it is always NULL.

Remove asc.acc.pipeline and drop checks for it being NULL / !NULL.

Link: https://lore.kernel.org/linux-media/20220615205037.16549-22-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# 868088a0 15-Jun-2022 Hans de Goede <hdegoede@redhat.com>

media: atomisp: remove atomisp_acc.c

With the ACC ioctls removed sd->acc.fw is always empty turning
the atomisp_acc.c code into no-ops, remove it.

Link: https://lore.kernel.org/linux-media/20220615205037.16549-20-hdegoede@redhat.com
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>


# ac56760a 01-Dec-2021 Tsuchiya Yuto <kitakar@gmail.com>

media: atomisp: fix "variable dereferenced before check 'asd'"

There are two occurrences where the variable 'asd' is dereferenced
before check. Fix this issue by using the variable after the check.

Link: https://lore.kernel.org/linux-media/20211122074122.GA6581@kili/

Link: https://lore.kernel.org/linux-media/20211201141904.47231-1-kitakar@gmail.com
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 3c82bf02 17-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: add YUVPP at __atomisp_get_pipe() logic

Aligns it which the Intel Aero firmware.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 13d72e69 17-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: atomisp_cmd: make it more compatible with firmware

Change some recovery logic at the driver, in order to make it
more compatible with ISP2401 Intel Aero firmware.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 83946783 15-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: simplify sh_css_defs.h

There are several unused macros. Simplify the logic there, making
it closer to the Intel Aero driver and the corresponding firmware,
as this is what we have widely available for this device.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# bcc3ba66 12-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of phys event abstractions

ia_css_dequeue_event() is just an alias to ia_css_dequeue_psys_event(),
and atomisp_css_dequeue_event() do nothing but calling the event
dequeue logic.

Get rid of both abstractions, calling the function directly.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# b467d97f 10-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of atomisp_get_frame_pgnr() abstraction

Simplify the code by getting rid of this function, as it ends
being just a single line of code.

Link: https://lore.kernel.org/linux-media/80ea920d14379124ba92aab2e6a6d12a92d79b2b.1636544620.git.mchehab+huawei@kernel.org
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 71665d81 09-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: check before deference asd variable

The asd->isp was referenced before checking if asd is not
NULL.

This fixes this warning:

../drivers/staging/media/atomisp/pci/atomisp_cmd.c:5548 atomisp_set_fmt_to_snr() warn: variable dereferenced before check 'asd' (see line 5540)

While here, avoid getting the pipe pointer twice.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 4a62b5cc 05-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of ISP2401_NEW_INPUT_SYSTEM

All ISP2401 devices use the new input system. So, get rid
of the remaining definitions, replacing them by runtime
checks for BYT/CHT when applicable.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# c9e9094c 04-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: fix try_fmt logic

The internal try_fmt logic is not meant to provide everything
that the V4L2 API should provide. Also, it doesn't decrement
the pads that are used only internally by the driver, but aren't
part of the device's output.

Fix it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# f5f3cedf 04-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: fix VIDIOC_S_FMT logic

There are several issues on S_FMT implementation:

- it doesn't properly handle pad_h/pad_w;
- it reports a wrong visible size to userspace;
- it allows setting the format to a raw mode, which
currently causes the pipeline to break.

Address such issues, for it to start working with generic
apps.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 9a542497 04-Nov-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: move a debug printf to a better place

The sensor width/height report is alread being printed after
its calculus. The only reason for an extra debug printk is
when dis is used. So, change its message to reflect and move
it to be inside the if checks.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 819d679b 31-Oct-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: comment-out JPEG format

This is already disabled on some parts of the code, and trying
to use it with current firmware causes an error:

[ 53.799946] atomisp-isp2 0000:00:03.0: can't create streams
[ 53.799962] atomisp-isp2 0000:00:03.0: __get_frame_info 1600x900 (padded to 0) returned -22

So, completely disable reporting it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 2b806251 29-Oct-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: report colorspace information

While different sensors could have different colorspaces, the
colorspace V4L2 support didn't exist by the time atomisp driver
was written. So, the sensors won't have any data about that.

So, let's report what Asus T101HA uses (ov2680 sensor).

It may require further changes, if other sensors used on this
driver have different colorspaces.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 6c84a35d 26-Oct-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: properly implement g_fmt

The current logic only initializes pipe->pix after setting up a
pipeline - e. g. after start streaming.

While it makes sense to get the format of the pipeline, when
it is set, this breaks support for generic applications, as they
rely on getting the current sensor format (which is usually the
highest resolution format).

So, implement a call to the sensor's get_fmt, when this is called
before setting up a pipeline.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 331adc2f 29-Oct-2021 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: better describe get_frame_info issues

When atomisp is used by a normal client, it fails to get
frame info. However, the information is confusing and misleading,
as there are several wrappers for such function, and the error
could be on different places.

So, improve the error log in order to allow narrowing down
where the error is actually occuring.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# c10bcb13 17-Oct-2021 Tsuchiya Yuto <kitakar@gmail.com>

media: atomisp: add NULL check for asd obtained from atomisp_video_pipe

This is almost a BUG report with RFC patch that just avoids kernel
oopses. Thus, prefixed with [BUG][RFC].

Here is the kernel log after running `v4l2-compliance -d /dev/video4`
with this patch applied:

kern :err : [25507.580392] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.592343] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.592995] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.593685] atomisp-isp2 0000:00:03.0: atomisp_g_input(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.593719] atomisp-isp2 0000:00:03.0: atomisp_g_parm(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.593727] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
[omitting 42 same messages]
kern :err : [25507.593976] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594191] atomisp-isp2 0000:00:03.0: atomisp_g_input(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594449] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
[omitting 43 same messages]
kern :err : [25507.594756] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594779] atomisp-isp2 0000:00:03.0: atomisp_g_ctrl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594787] atomisp-isp2 0000:00:03.0: atomisp_s_ctrl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594803] atomisp-isp2 0000:00:03.0: atomisp_camera_g_ext_ctrls(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594880] atomisp-isp2 0000:00:03.0: atomisp_enum_fmt_cap(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.594915] atomisp-isp2 0000:00:03.0: atomisp_g_parm(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.595058] atomisp-isp2 0000:00:03.0: atomisp_try_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.595089] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.595124] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.595221] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.595241] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.601571] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.607496] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.608604] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.611988] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.617420] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.618429] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.618811] atomisp-isp2 0000:00:03.0: atomisp_g_parm(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.622193] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.627355] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.628391] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.631143] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.635813] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.636489] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.636504] atomisp-isp2 0000:00:03.0: atomisp_s_input(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.636516] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.639111] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.646152] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.646831] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.646847] atomisp-isp2 0000:00:03.0: atomisp_s_input(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.650079] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.657476] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.658741] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.658759] atomisp-isp2 0000:00:03.0: atomisp_s_input(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.658771] atomisp-isp2 0000:00:03.0: atomisp_set_fmt(): asd is NULL, device is ATOMISP ISP ACC
kern :err : [25507.660959] atomisp-isp2 0000:00:03.0: can't change power state from D3cold to D0 (config space inaccessible)
kern :warn : [25507.666665] isys dma store at addr(0xcd408) val(0)
kern :err : [25507.667397] atomisp-isp2 0000:00:03.0: atomisp_queryctl(): asd is NULL, device is ATOMISP ISP ACC

[mchehab: fix coding style]
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 0d346d2a 10-Jun-2021 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

media: v4l2-subdev: add subdev-wide state struct

We have 'struct v4l2_subdev_pad_config' which contains configuration for
a single pad used for the TRY functionality, and an array of those
structs is passed to various v4l2_subdev_pad_ops.

I was working on subdev internal routing between pads, and realized that
there's no way to add TRY functionality for routes, which is not pad
specific configuration. Adding a separate struct for try-route config
wouldn't work either, as e.g. set-fmt needs to know the try-route
configuration to propagate the settings.

This patch adds a new struct, 'struct v4l2_subdev_state' (which at the
moment only contains the v4l2_subdev_pad_config array) and the new
struct is used in most of the places where v4l2_subdev_pad_config was
used. All v4l2_subdev_pad_ops functions taking v4l2_subdev_pad_config
are changed to instead take v4l2_subdev_state.

The changes to drivers/media/v4l2-core/v4l2-subdev.c and
include/media/v4l2-subdev.h were written by hand, and all the driver
changes were done with the semantic patch below. The spatch needs to be
applied to a select list of directories. I used the following shell
commands to apply the spatch:

dirs="drivers/media/i2c drivers/media/platform drivers/media/usb drivers/media/test-drivers/vimc drivers/media/pci drivers/staging/media"
for dir in $dirs; do spatch -j8 --dir --include-headers --no-show-diff --in-place --sp-file v4l2-subdev-state.cocci $dir; done

Note that Coccinelle chokes on a few drivers (gcc extensions?). With
minor changes we can make Coccinelle run fine, and these changes can be
reverted after spatch. The diff for these changes is:

For drivers/media/i2c/s5k5baf.c:

@@ -1481,7 +1481,7 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd,
&s5k5baf_cis_rect,
v4l2_subdev_get_try_crop(sd, cfg, PAD_CIS),
v4l2_subdev_get_try_compose(sd, cfg, PAD_CIS),
- v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT)
+ v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT),
};
s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
return 0;

For drivers/media/platform/s3c-camif/camif-capture.c:

@@ -1230,7 +1230,7 @@ static int s3c_camif_subdev_get_fmt(struct v4l2_subdev *sd,
*mf = camif->mbus_fmt;
break;

- case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
+ case CAMIF_SD_PAD_SOURCE_C:
/* crop rectangle at camera interface input */
mf->width = camif->camif_crop.width;
mf->height = camif->camif_crop.height;
@@ -1332,7 +1332,7 @@ static int s3c_camif_subdev_set_fmt(struct v4l2_subdev *sd,
}
break;

- case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
+ case CAMIF_SD_PAD_SOURCE_C:
/* Pixel format can be only changed on the sink pad. */
mf->code = camif->mbus_fmt.code;
mf->width = crop->width;

The semantic patch is:

// <smpl>

// Change function parameter

@@
identifier func;
identifier cfg;
@@

func(...,
- struct v4l2_subdev_pad_config *cfg
+ struct v4l2_subdev_state *sd_state
, ...)
{
<...
- cfg
+ sd_state
...>
}

// Change function declaration parameter

@@
identifier func;
identifier cfg;
type T;
@@
T func(...,
- struct v4l2_subdev_pad_config *cfg
+ struct v4l2_subdev_state *sd_state
, ...);

// Change function return value

@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
*func(...)
{
...
}

// Change function declaration return value

@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
*func(...);

// Some drivers pass a local pad_cfg for a single pad to a called function. Wrap it
// inside a pad_state.

@@
identifier func;
identifier pad_cfg;
@@
func(...)
{
...
struct v4l2_subdev_pad_config pad_cfg;
+ struct v4l2_subdev_state pad_state = { .pads = &pad_cfg };

<+...

(
v4l2_subdev_call
|
sensor_call
|
isi_try_fse
|
isc_try_fse
|
saa_call_all
)
(...,
- &pad_cfg
+ &pad_state
,...)

...+>
}

// If the function uses fields from pad_config, access via state->pads

@@
identifier func;
identifier state;
@@
func(...,
struct v4l2_subdev_state *state
, ...)
{
<...
(
- state->try_fmt
+ state->pads->try_fmt
|
- state->try_crop
+ state->pads->try_crop
|
- state->try_compose
+ state->pads->try_compose
)
...>
}

// If the function accesses the filehandle, use fh->state instead

@@
struct v4l2_subdev_fh *fh;
@@
- fh->pad
+ fh->state

@@
struct v4l2_subdev_fh fh;
@@
- fh.pad
+ fh.state

// Start of vsp1 specific

@@
@@
struct vsp1_entity {
...
- struct v4l2_subdev_pad_config *config;
+ struct v4l2_subdev_state *config;
...
};

@@
symbol entity;
@@
vsp1_entity_init(...)
{
...
entity->config =
- v4l2_subdev_alloc_pad_config
+ v4l2_subdev_alloc_state
(&entity->subdev);
...
}

@@
symbol entity;
@@
vsp1_entity_destroy(...)
{
...
- v4l2_subdev_free_pad_config
+ v4l2_subdev_free_state
(entity->config);
...
}

@exists@
identifier func =~ "(^vsp1.*)|(hsit_set_format)|(sru_enum_frame_size)|(sru_set_format)|(uif_get_selection)|(uif_set_selection)|(uds_enum_frame_size)|(uds_set_format)|(brx_set_format)|(brx_get_selection)|(histo_get_selection)|(histo_set_selection)|(brx_set_selection)";
symbol config;
@@
func(...) {
...
- struct v4l2_subdev_pad_config *config;
+ struct v4l2_subdev_state *config;
...
}

// End of vsp1 specific

// Start of rcar specific

@@
identifier sd;
identifier pad_cfg;
@@
rvin_try_format(...)
{
...
- struct v4l2_subdev_pad_config *pad_cfg;
+ struct v4l2_subdev_state *sd_state;
...
- pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+ sd_state = v4l2_subdev_alloc_state(sd);
<...
- pad_cfg
+ sd_state
...>
- v4l2_subdev_free_pad_config(pad_cfg);
+ v4l2_subdev_free_state(sd_state);
...
}

// End of rcar specific

// Start of rockchip specific

@@
identifier func =~ "(rkisp1_rsz_get_pad_fmt)|(rkisp1_rsz_get_pad_crop)|(rkisp1_rsz_register)";
symbol rsz;
symbol pad_cfg;
@@

func(...)
{
+ struct v4l2_subdev_state state = { .pads = rsz->pad_cfg };
...
- rsz->pad_cfg
+ &state
...
}

@@
identifier func =~ "(rkisp1_isp_get_pad_fmt)|(rkisp1_isp_get_pad_crop)";
symbol isp;
symbol pad_cfg;
@@

func(...)
{
+ struct v4l2_subdev_state state = { .pads = isp->pad_cfg };
...
- isp->pad_cfg
+ &state
...
}

@@
symbol rkisp1;
symbol isp;
symbol pad_cfg;
@@

rkisp1_isp_register(...)
{
+ struct v4l2_subdev_state state = { .pads = rkisp1->isp.pad_cfg };
...
- rkisp1->isp.pad_cfg
+ &state
...
}

// End of rockchip specific

// Start of tegra-video specific

@@
identifier sd;
identifier pad_cfg;
@@
__tegra_channel_try_format(...)
{
...
- struct v4l2_subdev_pad_config *pad_cfg;
+ struct v4l2_subdev_state *sd_state;
...
- pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+ sd_state = v4l2_subdev_alloc_state(sd);
<...
- pad_cfg
+ sd_state
...>
- v4l2_subdev_free_pad_config(pad_cfg);
+ v4l2_subdev_free_state(sd_state);
...
}

@@
identifier sd_state;
@@
__tegra_channel_try_format(...)
{
...
struct v4l2_subdev_state *sd_state;
<...
- sd_state->try_crop
+ sd_state->pads->try_crop
...>
}

// End of tegra-video specific

// </smpl>

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 0a016c35 15-Apr-2021 Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>

media: staging: media: atomisp: pci: Balance braces around conditional statements in file atomisp_cmd.c

Balance braces around conditional statements.
Issue detected by checkpatch.pl.
It happens in if-else statements where one of the commands
uses braces around a block of code and the other command
does not since it has just a single line of code.

Signed-off-by: Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 97b34809 26-Feb-2021 Arnd Bergmann <arnd@arndb.de>

media: staging: atomisp: reduce kernel stack usage

The atomisp_set_fmt() function has multiple copies of the large
v4l2_format structure on its stack, resulting in a warning about
excessive stack usage in some rare randconfig builds.

drivers/staging/media/atomisp/pci/atomisp_cmd.c:5600:5: error: stack frame size of 1084 bytes in function 'atomisp_set_fmt' [-Werror,-Wframe-larger-than=]

Of this structure, only three members in the 'fmt.pix' member are
used, so simplify it by using the smaller v4l2_pix_format structure
directly. This reduces the stack usage to 612 bytes, and it could
be reduced further by only storing the three members that are used.

Link: https://lore.kernel.org/linux-media/20210226140524.3478346-1-arnd@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# ac68fb7f 21-Jan-2021 Yang Li <abaci-bugfix@linux.alibaba.com>

media: atomisp: remove redundant NULL check

Fix below warnings reported by coccicheck:
./drivers/staging/media/atomisp/pci/atomisp_cmd.c:4269:2-8: WARNING:
NULL check before some freeing functions is not needed.
./drivers/staging/media/atomisp/pci/atomisp_cmd.c:4626:2-8: WARNING:
NULL check before some freeing functions is not needed.

Link: https://lore.kernel.org/linux-media/1611220312-89384-3-git-send-email-abaci-bugfix@linux.alibaba.com
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <abaci-bugfix@linux.alibaba.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 5d986a0e 14-Nov-2020 Kaixu Xia <kaixuxia@tencent.com>

media: atomisp: make atomisp_css_continuous_set_num_raw_frames() void

The return value of atomisp_css_continuous_set_num_raw_frames() function
is always 0, so there's no reason for a return value. In addition, the
__enable_continuous_mode() checked the return value for possible error
which is unnecessary. Convert atomisp_css_continuous_set_num_raw_frames()
to a void function.

Link: https://lore.kernel.org/linux-media/1605362150-11401-1-git-send-email-kaixuxia@tencent.com
Reported-by: Tosk Robot <tencent_os_robot@tencent.com>
Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 3aa059ae 17-Oct-2020 Alex Dewar <alex.dewar90@gmail.com>

media: staging: media: atomisp: Remove unnecessary if statement

The bodies of the if and else sections are the same, so just remove the
check.

Link: https://lore.kernel.org/linux-media/20201017142810.26967-2-alex.dewar90@gmail.com
Signed-off-by: Alex Dewar <alex.dewar90@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 2394984f 21-Sep-2020 Alex Dewar <alex.dewar90@gmail.com>

media: staging: media: atomisp: Fix bool-related style issues

Address the following issues:
* unnecessary comparison to true/false
* use of 0/1 instead of bool values
* unnecessary conversion to bool

These were fixed using the following Coccinelle scripts:
* scripts/coccinelle/misc/bool{init,conv,return}.cocci

Build-tested with allmodconfig.

Signed-off-by: Alex Dewar <alex.dewar90@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 852a53a0 03-Sep-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of unused vars

There are several vars declared and set but unused:

drivers/staging/media/atomisp//pci/sh_css_param_shading.c:239:4: warning: variable ‘padded_width’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:1366:24: warning: variable ‘stream’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:2702:22: warning: variable ‘capture_pipe’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:2831:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3534:15: warning: variable ‘num_output_pins’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:55: warning: variable ‘vf_pp_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:38: warning: variable ‘preview_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3936:26: warning: variable ‘me’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:5749:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:6013:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:6012:7: warning: variable ‘memory’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:7329:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:7459:26: warning: variable ‘num_vf_pp_stage’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:10420:22: warning: variable ‘pipe_id’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css_params.c:1099:36: warning: variable ‘row_padding’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css_mipi.c:410:27: warning: variable ‘mipi_intermediate_info’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/atomisp_cmd.c:2814:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/atomisp_cmd.c:2893:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c:237:31: warning: variable ‘isp_data_ptr’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:28: warning: variable ‘ver_num_isp’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:15: warning: variable ‘hor_num_isp’ set but not used [-Wunused-but-set-variable]

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 7e023a1c 23-Jul-2020 Aditya Jain <aditya.jainadityajain.jain@gmail.com>

media: atomisp: Fix braces placement coding style errors

Fix braces placement errors as reported by checkpatch.pl

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# f95cb3dd 29-Jul-2020 Ankit Baluni <b18007@students.iitmandi.ac.in>

media: atomisp: fixed a brace coding sytle issue

Removed braces for a 'if' condition as it contain only single line &
there is no need for braces for such case according to coding style
rules.

Signed-off-by: Ankit Baluni <b18007@students.iitmandi.ac.in>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# df561f66 23-Aug-2020 Gustavo A. R. Silva <gustavoars@kernel.org>

treewide: Use fallthrough pseudo-keyword

Replace the existing /* fall through */ comments and its variants with
the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
fall-through markings when it is the case.

[1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>


# 71aecd5d 26-Jun-2020 Andy Shevchenko <andriy.shevchenko@linux.intel.com>

media: atomisp: Replace direct access to MMIO with proper helpers

We have special helpers to access MMIO. Use them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 89027fea 26-Jun-2020 Andy Shevchenko <andriy.shevchenko@linux.intel.com>

media: atomisp: Unify pdev to be pointer to struct pci_device

Unify pdev to be pointer to struct pci_device.

While here, reindent some (touched) lines for better readability.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 1cfc8593 26-Jun-2020 Andy Shevchenko <andriy.shevchenko@linux.intel.com>

media: atomisp: Get rid of struct pci_dev in struct atomisp_device

struct atomisp device has struct device and struct pci_dev pointers
which are basically duplicates of each other. Drop the latter
in favour of the former.

While here, unify pdev to be pointer to struct pci_device and reindent
some (touched) lines for better readability.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# d6ad1c9b 21-Jun-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: fix call to g_frame_interval

The media core has now a check if fi->pad is bigger than zero
or bigger than sd->entity.num_pads, if the media controller
is defined.

This causes a call to g_frame_interval to return -EINVAL.

Fix it by first cleaning up the struct.

Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 69a03e36 01-Jun-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of an iomem abstraction layer

The hive_isp_css_custom_host_hrt.h code, together
with atomisp_helper.h, provides an abstraction layer for
some functions inside atomisp_compat_css20.c and atomisp_cmd.c.

There's no good reason for that. In a matter of fact, after
removing the abstraction, the code looked a lot cleaner
and easier to understand.

So, get rid of them.

While here, get rid also of the udelay(1) abstraction code.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# baf768cf 01-Jun-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: set DFS to MAX if sensor doesn't report fps

If the sensor doesn't implement support for g_frame_interval,
it won't return the expected fps rate.

Instead of keeping DFS on its minimal value (which will likely
not work), set it to the max.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 92b2bc49 31-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: use different dfs failed messages

There are several parts of the driver that could produce
a "dfs failed!" message. Change the texts, in order to help
identifying from where they're coming.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 469a7306 01-Jun-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: change the detection of ISP2401 at runtime

Instead of having a static var to detect it, let's use the
already-existing arch-specific bytes, as this is how other
parts of the code also checks when it needs to do something
different, depending on an specific chipset version.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# f2fb029b 31-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: improve IRQ handling debug messages

When an IRQ is not handled, it is nice to know what's the
reason.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 5b552b19 30-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: re-enable warnings again

For most warnings, the current code is OK. There are still
some issues with implicit-fallthough warnings.

Solve those and re-enable all warnings for this driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# fb1f6ae6 30-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: remove format duplication at mbus->fourcc table

This table used to be used also to translate between ia_css
abstraction and V4L2 fourcc codes.

This was removed on a past patch, but the table now contains
two fields with identical values.

Get rid of one of them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# f5fbb83f 29-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: add SPDX headers

This driver is licensed under GPL 2.0, as stated inside their
headers.

Add the proper tag there. We should probably latter cleanup
the reduntant licensing text, but this could be done later,
after we get rid of other abstraction layers.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# f8b39c65 04-Jun-2020 Sakari Ailus <sakari.ailus@linux.intel.com>

media: staging: atomisp: There's no struct atomisp_dvs2_coefficients

It's called struct atomisp_dis_coefficients.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 85b606e0 27-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of a bunch of other wrappers

There are too many wrapper functions at atomisp_compat_css20.c.

Get rid of another set of such wrappers.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# a86b4936 29-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of an error abstraction layer

There is an abstraction layer there meant to convert to
the Linux standard error codes. As the driver now use
such errors everywhere. we can get rid of this.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 41022d35 28-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of non-Linux error codes

The atomisp driver has its own error codes under the
ia_css_err.h file. On several places, those got already
replaced by standard error codes, but there are still a
lot more to be fixed.

Let's get rid of all of those, mapping them into
the already-existing set of Linux error codes.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# abbd669d 28-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: do another round of coding style cleanup

Run checkpatch --fix-inline again, in order to get rid
of some additional issues that got introduced (or that
checkpatch can now detect).

This should help preventing receiving random cleanups,
while keeping the code on a better shape.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 1a16d545 28-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: remove some trivial wrappers from compat css20

There are tons of code inside atomisp_compat_css20.c, but
several of them are just trivial wrappers to other functions.

Getting rid of all of them will take some time, but let's
start getting rid of some of the trivial ones.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 541f6813 27-May-2020 Nathan Chancellor <nathan@kernel.org>

media: atomisp: Remove unnecessary NULL check in atomisp_param

Clang warns:

drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: warning:
address of 'config->info' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!&config->info) {
~ ~~~~~~~~^~~~

config cannot be NULL because it comes from an ioctl, which ensures that
the user is not giving us an invalid pointer through copy_from_user. If
config is not NULL, info cannot be NULL. Remove this check.

Link: https://github.com/ClangBuiltLinux/linux/issues/1036

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 4fba2916 25-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of the hrt/hive_isp_css_mm_hrt abstraction layer

Simplify the code by removing this extra memory management
abstraction layer.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# c01d5546 20-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: partially get rid of one abstraction layer

The very same macros are defined as CSS_foo and IA_CSS_foo.

Remove this abstraction, as it just make things confusing,
for no good reason.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 6b3a422b 23-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of some old broken debug code

It sounds that someone once changed the debug level at compile
time for some testing, but forgot to remove the legacy code after
finishing debuging it.

Get rid of the dead code.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 171b7bd6 20-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: improve debug messages for set format

There are several error conditions that don't print anything,
making harder to identify bugs at the code there.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 370f6e5a 21-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: avoid a copy of v4l2_mbus_framefmt at stack

There's no reason to copy isp_sink_fmt, as the driver
uses it for read-only purposes.

Linux stack is a precious resource. Let's avoid wasting it.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# aaebb654 21-May-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: reduce debug printk rate when IRQs are received

Currently, when an EOF IRQ is received, it generates two messages:

[ 59.191893] atomisp-isp2 0000:00:03.0: irq:0x200000
[ 59.191913] atomisp-isp2 0000:00:03.0: atomisp_isr EOF exp_id 142, asd 0

Flooding the dmesg with lots of messages per second. The same
pattern happens for all other IRQs.

Change the logic for printing just one message per IRQ and
rate-limit those, as, for debugging purposes, it is usually
interesting to know that IRQs are being received, but not
displaying every single one.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 3c0538fb 30-Apr-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: get rid of most checks for ISP2401 version

There are lots of places inside this driver checking for
ISP2400/ISP2401 verison. Get rid of most of those, while
keep building for both.

Most of stuff in this patch is trivial to solve.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>


# 9d4fa1a1 30-Apr-2020 Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

media: atomisp: cleanup directory hierarchy

This driver has very long directories without a good
reason (IMHO). Let's drop two directories from such hierarchy,
in order to simplify things a little bit and make the dir
output a bit more readable.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>