Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari

On Mon, Feb 26, 2024 at 11:03:47AM +0000, Sakari Ailus wrote:
> Hi Daniel,
>
> Thanks for the set.
>
> How do you determine which buffers go together on the video buffer queues?
> Or do you need a buffer on both for every frame if the nodes are set
> streaming?
>
> This should be also documented, in the documentation patch. At least I
> didn't find it there.
>
> On Wed, Feb 14, 2024 at 02:19:04PM +0000, Daniel Scally wrote:
> > Add a driver for Arm's Mali-C55 Image Signal Processor. The driver is
> > V4L2 and Media Controller compliant and creates subdevices to manage
> > the ISP itself, its internal test pattern generator as well as the
> > crop, scaler and output format functionality for each of its two
> > output devices.
> >
> > Acked-by: Nayden Kanchev <nayden.kanchev@xxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> > ---
> > Changes in v2:
> >
> > 	- Clock handling
> > 	- Fixed the warnings raised by the kernel test robot
> >
> >  drivers/media/platform/Kconfig                |    1 +
> >  drivers/media/platform/Makefile               |    1 +
> >  drivers/media/platform/arm/Kconfig            |    5 +
> >  drivers/media/platform/arm/Makefile           |    2 +
> >  drivers/media/platform/arm/mali-c55/Kconfig   |   18 +
> >  drivers/media/platform/arm/mali-c55/Makefile  |    9 +
> >  .../platform/arm/mali-c55/mali-c55-capture.c  | 1021 +++++++++++++++++
> >  .../platform/arm/mali-c55/mali-c55-common.h   |  271 +++++
> >  .../platform/arm/mali-c55/mali-c55-core.c     |  767 +++++++++++++
> >  .../platform/arm/mali-c55/mali-c55-isp.c      |  682 +++++++++++
> >  .../arm/mali-c55/mali-c55-registers.h         |  180 +++
> >  .../arm/mali-c55/mali-c55-resizer-coefs.h     |  382 ++++++
> >  .../platform/arm/mali-c55/mali-c55-resizer.c  |  678 +++++++++++
> >  .../platform/arm/mali-c55/mali-c55-tpg.c      |  425 +++++++
> >  14 files changed, 4442 insertions(+)
> >  create mode 100644 drivers/media/platform/arm/Kconfig
> >  create mode 100644 drivers/media/platform/arm/Makefile
> >  create mode 100644 drivers/media/platform/arm/mali-c55/Kconfig
> >  create mode 100644 drivers/media/platform/arm/mali-c55/Makefile
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-common.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-core.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-isp.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer-coefs.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-tpg.c
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index 91e54215de3a..cd92c024e039 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -65,6 +65,7 @@ config VIDEO_MUX
> >  source "drivers/media/platform/allegro-dvt/Kconfig"
> >  source "drivers/media/platform/amlogic/Kconfig"
> >  source "drivers/media/platform/amphion/Kconfig"
> > +source "drivers/media/platform/arm/Kconfig"
> >  source "drivers/media/platform/aspeed/Kconfig"
> >  source "drivers/media/platform/atmel/Kconfig"
> >  source "drivers/media/platform/cadence/Kconfig"
> > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > index 3296ec1ebe16..ea6624c62559 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -8,6 +8,7 @@
> >  obj-y += allegro-dvt/
> >  obj-y += amlogic/
> >  obj-y += amphion/
> > +obj-y += arm/
> >  obj-y += aspeed/
> >  obj-y += atmel/
> >  obj-y += cadence/
> > diff --git a/drivers/media/platform/arm/Kconfig b/drivers/media/platform/arm/Kconfig
> > new file mode 100644
> > index 000000000000..4f0764c329c7
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/Kconfig
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +comment "ARM media platform drivers"
> > +
> > +source "drivers/media/platform/arm/mali-c55/Kconfig"
> > diff --git a/drivers/media/platform/arm/Makefile b/drivers/media/platform/arm/Makefile
> > new file mode 100644
> > index 000000000000..8cc4918725ef
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +obj-y += mali-c55/
> > diff --git a/drivers/media/platform/arm/mali-c55/Kconfig b/drivers/media/platform/arm/mali-c55/Kconfig
> > new file mode 100644
> > index 000000000000..602085e28b01
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/Kconfig
> > @@ -0,0 +1,18 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +config VIDEO_MALI_C55
> > +	tristate "ARM Mali-C55 Image Signal Processor driver"
> > +	depends on V4L_PLATFORM_DRIVERS
> > +	depends on VIDEO_DEV && OF
> > +	depends on ARCH_VEXPRESS || COMPILE_TEST
> > +	select MEDIA_CONTROLLER
> > +	select VIDEO_V4L2_SUBDEV_API
> > +	select VIDEOBUF2_DMA_CONTIG
> > +	select VIDEOBUF2_VMALLOC
> > +	select V4L2_FWNODE
> > +	select GENERIC_PHY_MIPI_DPHY
> > +	default n
> > +	help
> > +	  Enable this to support Arm's Mali-C55 Image Signal Processor.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called mali-c55.
> > diff --git a/drivers/media/platform/arm/mali-c55/Makefile b/drivers/media/platform/arm/mali-c55/Makefile
> > new file mode 100644
> > index 000000000000..77dcb2fbf0f4
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/Makefile
> > @@ -0,0 +1,9 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +mali-c55-y := mali-c55-capture.o \
> > +	      mali-c55-core.o \
> > +	      mali-c55-isp.o \
> > +	      mali-c55-tpg.o \
> > +	      mali-c55-resizer.o
> > +
> > +obj-$(CONFIG_VIDEO_MALI_C55) += mali-c55.o
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> > new file mode 100644
> > index 000000000000..98020b7ecb1e
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> > @@ -0,0 +1,1021 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ARM Mali-C55 ISP Driver - Video capture devices
> > + *
> > + * Copyright (C) 2023 Ideas on Board Oy
>
> 2024
>
> > + */
> > +
> > +#include <linux/minmax.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/string.h>
> > +#include <linux/videodev2.h>
> > +
> > +#include <media/v4l2-dev.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/v4l2-subdev.h>
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +
> > +#include "mali-c55-common.h"
> > +#include "mali-c55-registers.h"
> > +
> > +/*
> > + * The Mali-C55 ISP has up to two output pipes; known as full resolution and
> > + * down scaled. The register space for these is laid out identically, but offset
> > + * by 372 bytes.
> > + */
> > +#define MALI_C55_CAP_DEV_FR_REG_OFFSET		0x0
> > +#define MALI_C55_CAP_DEV_DS_REG_OFFSET		0x174
> > +
> > +static const struct mali_c55_fmt mali_c55_fmts[] = {
> > +	/*
> > +	 * This table is missing some entries which need further work or
> > +	 * investigation:
> > +	 *
> > +	 * Base mode 1 is a backwards V4L2_PIX_FMT_XRGB32 with no V4L2 equivalent
> > +	 * Base mode 5 is "Generic Data"
> > +	 * Base mode 8 is a backwards V4L2_PIX_FMT_XYUV32 - no V4L2 equivalent
> > +	 * Base mode 9 seems to have no V4L2 equivalent
> > +	 * Base mode 17, 19 and 20 describe formats which seem to have no V4L2
> > +	 * equivalent
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_ARGB2101010,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_A2R10G10B10,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_RGB565,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RGB565,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_BGR24,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RGB24,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_YUYV,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_YUY2,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_UYVY,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_UYVY,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_Y210,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_Y210,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	/*
> > +	 * This is something of a hack, the ISP thinks it's running NV12M but
> > +	 * by setting uv_plane = 0 we simply discard that planes and only output
> > +	 * the Y-plane.
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_GREY,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_NV12M,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT1
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_NV21M,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT2
> > +		}
> > +	},
> > +	/*
> > +	 * RAW uncompressed formats are all packed in 16 bpp.
> > +	 * TODO: Expand this list to encompass all possible RAW formats.
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SRGGB12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SRGGB12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SBGGR12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SBGGR12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SGBRG12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SGBRG12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SGRBG12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SGRBG12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +};
> > +
> > +static bool mali_c55_mbus_code_can_produce_fmt(const struct mali_c55_fmt *fmt,
> > +					       u32 code)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(fmt->mbus_codes); i++) {
> > +		if (fmt->mbus_codes[i] == code)
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > +						 bool allow_raw, bool unique)
> > +{
> > +	if (!fmt)
> > +		fmt = &mali_c55_fmts[0];
> > +	else
> > +		++fmt;
>
> fmt++, please.
>

Can I ask why ? (here and in the next occurrences you have reported)




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux