From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Daniel Scally <dan.scally@ideasonboard.com>,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Cc: Anthony.McGivern@arm.com, jacopo.mondi@ideasonboard.com,
nayden.kanchev@arm.com, robh+dt@kernel.org, mchehab@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
jerome.forissier@linaro.org, kieran.bingham@ideasonboard.com,
laurent.pinchart@ideasonboard.com
Subject: Re: [PATCH v10 07/17] media: mali-c55: Add Mali-C55 ISP driver
Date: Sat, 28 Jun 2025 23:06:54 +0300 [thread overview]
Message-ID: <cee962ce-3719-4ae7-9849-548a95d98e99@linux.intel.com> (raw)
In-Reply-To: <20250624-c55-v10-7-54f3d4196990@ideasonboard.com>
Hi Daniel,
On 6/24/25 13:21, Daniel Scally wrote:
...
> +static irqreturn_t mali_c55_isr(int irq, void *context)
> +{
> + struct device *dev = context;
> + struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
> + u32 interrupt_status;
> + unsigned int i;
> +
> + interrupt_status = mali_c55_read(mali_c55,
> + MALI_C55_REG_INTERRUPT_STATUS_VECTOR);
> + if (!interrupt_status)
> + return IRQ_NONE;
> +
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR_VECTOR,
> + interrupt_status);
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR, 0);
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR, 1);
> +
> + for (i = 0; i < MALI_C55_NUM_IRQ_BITS; i++) {
> + if (!(interrupt_status & (1 << i)))
BIT(), please!
Although, use __ffs() and this becomes redundant.
...
> +static void __mali_c55_power_off(struct mali_c55 *mali_c55)
> +{
> + reset_control_bulk_assert(ARRAY_SIZE(mali_c55->resets), mali_c55->resets);
> + clk_bulk_disable_unprepare(ARRAY_SIZE(mali_c55->clks), mali_c55->clks);
> +}
> +
> +static int mali_c55_runtime_suspend(struct device *dev)
> +{
> + struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
> +
> + free_irq(mali_c55->irqnum, dev);
Do you really free the IRQ on device suspend? The law probably doesn't
forbid that though.
> + __mali_c55_power_off(mali_c55);
> +
> + return 0;
> +}
> +
> +static int __mali_c55_power_on(struct mali_c55 *mali_c55)
> +{
> + int ret;
> + u32 val;
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(mali_c55->clks),
> + mali_c55->clks);
> + if (ret) {
> + dev_err(mali_c55->dev, "failed to enable clocks\n");
> + return ret;
> + }
> +
> + ret = reset_control_bulk_deassert(ARRAY_SIZE(mali_c55->resets),
> + mali_c55->resets);
> + if (ret) {
> + dev_err(mali_c55->dev, "failed to deassert resets\n");
> + return ret;
> + }
> +
> + /* Use "software only" context management. */
> + mali_c55_update_bits(mali_c55, MALI_C55_REG_MCU_CONFIG,
> + MALI_C55_REG_MCU_CONFIG_OVERRIDE_MASK, 0x01);
> +
> + /*
> + * Mask the interrupts and clear any that were set, then unmask the ones
> + * that we actually want to handle.
> + */
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_MASK_VECTOR,
> + MALI_C55_INTERRUPT_MASK_ALL);
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR_VECTOR,
> + MALI_C55_INTERRUPT_MASK_ALL);
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR, 0x01);
> + mali_c55_write(mali_c55, MALI_C55_REG_INTERRUPT_CLEAR, 0x00);
> +
> + mali_c55_update_bits(mali_c55, MALI_C55_REG_INTERRUPT_MASK_VECTOR,
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_ISP_START) |
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_ISP_DONE) |
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_FR_Y_DONE) |
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_FR_UV_DONE) |
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_DS_Y_DONE) |
> + MALI_C55_INTERRUPT_BIT(MALI_C55_IRQ_DS_UV_DONE),
> + 0x00);
> +
> + /* Set safe stop to ensure we're in a non-streaming state */
> + mali_c55_write(mali_c55, MALI_C55_REG_INPUT_MODE_REQUEST,
> + MALI_C55_INPUT_SAFE_STOP);
> + readl_poll_timeout(mali_c55->base + MALI_C55_REG_MODE_STATUS,
> + val, !val, 10 * USEC_PER_MSEC, 250 * USEC_PER_MSEC);
> +
> + return 0;
> +}
> +
> +static int mali_c55_runtime_resume(struct device *dev)
> +{
> + struct mali_c55 *mali_c55 = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = __mali_c55_power_on(mali_c55);
> + if (ret)
> + return ret;
> +
> + /*
> + * The driver needs to transfer large amounts of register settings to
> + * the ISP each frame, using either a DMA transfer or memcpy. We use a
> + * threaded IRQ to avoid disabling interrupts the entire time that's
> + * happening.
> + */
> + ret = request_threaded_irq(mali_c55->irqnum, NULL, mali_c55_isr,
> + IRQF_ONESHOT, dev_driver_string(dev), dev);
> + if (ret) {
> + __mali_c55_power_off(mali_c55);
> + dev_err(dev, "failed to request irq\n");
> + }
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops mali_c55_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(mali_c55_runtime_suspend, mali_c55_runtime_resume,
> + NULL)
> +};
> +
> +static int mali_c55_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mali_c55 *mali_c55;
> + struct resource *res;
> + dma_cap_mask_t mask;
> + int ret;
> +
> + mali_c55 = devm_kzalloc(dev, sizeof(*mali_c55), GFP_KERNEL);
> + if (!mali_c55)
> + return -ENOMEM;
> +
> + mali_c55->dev = dev;
> + platform_set_drvdata(pdev, mali_c55);
> +
> + mali_c55->base = devm_platform_get_and_ioremap_resource(pdev, 0,
> + &res);
> + if (IS_ERR(mali_c55->base))
> + return dev_err_probe(dev, PTR_ERR(mali_c55->base),
> + "failed to map IO memory\n");
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(mali_c55_clk_names); i++)
> + mali_c55->clks[i].id = mali_c55_clk_names[i];
> +
> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(mali_c55->clks), mali_c55->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to acquire clocks\n");
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(mali_c55_reset_names); i++)
> + mali_c55->resets[i].id = mali_c55_reset_names[i];
> +
> + ret = devm_reset_control_bulk_get_optional_shared(
> + dev, ARRAY_SIZE(mali_c55_reset_names), mali_c55->resets);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to acquire resets\n");
> +
> + of_reserved_mem_device_init(dev);
> + vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_MEMCPY, mask);
> +
> + ret = __mali_c55_power_on(mali_c55);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to power on\n");
> +
> + ret = mali_c55_check_hwcfg(mali_c55);
> + if (ret)
> + goto err_power_off;
> +
> + /*
> + * No failure here because we will just fallback on memcpy if there is
> + * no usable DMA channel on the system.
> + */
> + mali_c55->channel = dma_request_channel(mask, NULL, NULL);
> + dev_dbg(mali_c55->dev,
> + "No DMA channel for config, falling back to memcpy\n");
> +
> + ret = mali_c55_init_context(mali_c55, res);
> + if (ret)
> + goto err_release_dma_channel;
> +
> + mali_c55->media_dev.dev = dev;
> +
> + mali_c55->inline_mode = device_property_read_bool(dev, "arm,inline_mode");
> +
> + ret = mali_c55_media_frameworks_init(mali_c55);
> + if (ret)
> + goto err_free_context_registers;
> +
> + __mali_c55_power_off(mali_c55);
> +
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 2000);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
Note that runtime PM resume fails before this so accessing UAPI would
fail. Please enable runtime PM before registering anything outside the
driver.
> +
> + mali_c55->irqnum = platform_get_irq(pdev, 0);
Wouldn't it make sense to read this earlier? For the same reason than
above, actually.
> + if (mali_c55->irqnum < 0) {
> + dev_err(dev, "failed to get interrupt\n");
> + goto err_pm_runtime_disable;
> + }
> +
> + return 0;
> +
> +err_pm_runtime_disable:
> + pm_runtime_disable(&pdev->dev);
> + mali_c55_media_frameworks_deinit(mali_c55);
> +err_free_context_registers:
> + kfree(mali_c55->context.registers);
> +err_release_dma_channel:
> + if (mali_c55->channel)
> + dma_release_channel(mali_c55->channel);
> +err_power_off:
> + __mali_c55_power_off(mali_c55);
> +
> + return ret;
> +}
> +
...
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-isp.c b/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..20d4d16c75fbf0d5519ecadb5ed1d080bdae05de
> --- /dev/null
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-isp.c
> @@ -0,0 +1,656 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ARM Mali-C55 ISP Driver - Image signal processor
> + *
> + * Copyright (C) 2024 Ideas on Board Oy
It's 2025 already.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +#include <linux/property.h>
> +#include <linux/string.h>
> +
> +#include <linux/media/arm/mali-c55-config.h>
If this is a UAPI header, please include uapi in the path, too.
Earlier such headers have been under include/uapi/linux, I don't object
putting new ones elsewhere in principle though. Just check with Hans and
Laurent, too... I don't have an opinion yet really.
> +/* NOT const because the default needs to be filled in at runtime */
> +static struct v4l2_ctrl_config mali_c55_isp_v4l2_custom_ctrls[] = {
> + {
> + .ops = &mali_c55_isp_ctrl_ops,
> + .id = V4L2_CID_MALI_C55_CAPABILITIES,
> + .name = "Mali-C55 ISP Capabilities",
> + .type = V4L2_CTRL_TYPE_BITMASK,
> + .min = 0,
> + .max = MALI_C55_GPS_PONG_FITTED |
> + MALI_C55_GPS_WDR_FITTED |
> + MALI_C55_GPS_COMPRESSION_FITTED |
> + MALI_C55_GPS_TEMPER_FITTED |
> + MALI_C55_GPS_SINTER_LITE_FITTED |
> + MALI_C55_GPS_SINTER_FITTED |
> + MALI_C55_GPS_IRIDIX_LTM_FITTED |
> + MALI_C55_GPS_IRIDIX_GTM_FITTED |
> + MALI_C55_GPS_CNR_FITTED |
> + MALI_C55_GPS_FRSCALER_FITTED |
> + MALI_C55_GPS_DS_PIPE_FITTED,
> + .def = 0,
> + },
> +};
> +
> +static int mali_c55_isp_init_controls(struct mali_c55 *mali_c55)
> +{
> + struct v4l2_ctrl_handler *handler = &mali_c55->isp.handler;
> + struct v4l2_ctrl *capabilities;
> + int ret;
> +
> + ret = v4l2_ctrl_handler_init(handler, 1);
> + if (ret)
> + return ret;
> +
> + mali_c55_isp_v4l2_custom_ctrls[0].def = mali_c55->capabilities;
The capabilities here are still specific to a device, not global, in
principle at least. Can you move it here, as a local variable?
> +
> + capabilities = v4l2_ctrl_new_custom(handler,
> + &mali_c55_isp_v4l2_custom_ctrls[0],
> + NULL);
> + if (capabilities)
> + capabilities->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> + if (handler->error) {
> + dev_err(mali_c55->dev, "failed to register capabilities control\n");
> + v4l2_ctrl_handler_free(handler);
> + return handler->error;
v4l2_ctrl_handler_free() will return the error soon, presumably sooner
than the above code makes it to upstream. Before that, this pattern
won't work as v4l2_ctrl_handler_free() also resets the handler's error
field. :-)
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..36a81be0191a15da91809dd2da5d279716f6d725
> --- /dev/null
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> @@ -0,0 +1,318 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * ARM Mali-C55 ISP Driver - Register definitions
> + *
> + * Copyright (C) 2024 Ideas on Board Oy
> + */
> +
> +#ifndef _MALI_C55_REGISTERS_H
> +#define _MALI_C55_REGISTERS_H
> +
> +#include <linux/bits.h>
> +
> +/* ISP Common 0x00000 - 0x000ff */
> +
> +#define MALI_C55_REG_API 0x00000
> +#define MALI_C55_REG_PRODUCT 0x00004
> +#define MALI_C55_REG_VERSION 0x00008
> +#define MALI_C55_REG_REVISION 0x0000c
> +#define MALI_C55_REG_PULSE_MODE 0x0003c
> +#define MALI_C55_REG_INPUT_MODE_REQUEST 0x0009c
> +#define MALI_C55_INPUT_SAFE_STOP 0x00
> +#define MALI_C55_INPUT_SAFE_START 0x01
> +#define MALI_C55_REG_MODE_STATUS 0x000a0
> +#define MALI_C55_REG_INTERRUPT_MASK_VECTOR 0x00030
> +#define MALI_C55_INTERRUPT_MASK_ALL GENMASK(31, 0)
> +
> +#define MALI_C55_REG_GLOBAL_MONITOR 0x00050
> +
> +#define MALI_C55_REG_GEN_VIDEO 0x00080
> +#define MALI_C55_REG_GEN_VIDEO_ON_MASK BIT(0)
> +#define MALI_C55_REG_GEN_VIDEO_MULTI_MASK BIT(1)
> +#define MALI_C55_REG_GEN_PREFETCH_MASK GENMASK(31, 16)
> +
> +#define MALI_C55_REG_MCU_CONFIG 0x00020
> +#define MALI_C55_REG_MCU_CONFIG_OVERRIDE_MASK BIT(0)
> +#define MALI_C55_REG_MCU_CONFIG_WRITE_MASK BIT(1)
> +#define MALI_C55_MCU_CONFIG_WRITE(x) ((x) << 1)
Is x unsigned?
> +#define MALI_C55_REG_MCU_CONFIG_WRITE_PING BIT(1)
> +#define MALI_C55_REG_MCU_CONFIG_WRITE_PONG 0x00
> +#define MALI_C55_REG_MULTI_CONTEXT_MODE_MASK BIT(8)
> +#define MALI_C55_REG_PING_PONG_READ 0x00024
> +#define MALI_C55_REG_PING_PONG_READ_MASK BIT(2)
> +#define MALI_C55_INTERRUPT_BIT(x) BIT(x)
> +
> +#define MALI_C55_REG_GLOBAL_PARAMETER_STATUS 0x00068
> +#define MALI_C55_GPS_PONG_FITTED BIT(0)
> +#define MALI_C55_GPS_WDR_FITTED BIT(1)
> +#define MALI_C55_GPS_COMPRESSION_FITTED BIT(2)
> +#define MALI_C55_GPS_TEMPER_FITTED BIT(3)
> +#define MALI_C55_GPS_SINTER_LITE_FITTED BIT(4)
> +#define MALI_C55_GPS_SINTER_FITTED BIT(5)
> +#define MALI_C55_GPS_IRIDIX_LTM_FITTED BIT(6)
> +#define MALI_C55_GPS_IRIDIX_GTM_FITTED BIT(7)
> +#define MALI_C55_GPS_CNR_FITTED BIT(8)
> +#define MALI_C55_GPS_FRSCALER_FITTED BIT(9)
> +#define MALI_C55_GPS_DS_PIPE_FITTED BIT(10)
> +
> +#define MALI_C55_REG_BLANKING 0x00084
> +#define MALI_C55_REG_HBLANK_MASK GENMASK(15, 0)
> +#define MALI_C55_REG_VBLANK_MASK GENMASK(31, 16)
> +#define MALI_C55_VBLANK(x) ((x) << 16)
Same question for the bit shifts left elsewhere in the header.
> +
> +#define MALI_C55_REG_HC_START 0x00088
> +#define MALI_C55_HC_START(h) (((h) & 0xffff) << 16)
> +#define MALI_C55_REG_HC_SIZE 0x0008c
> +#define MALI_C55_HC_SIZE(h) ((h) & 0xffff)
> +#define MALI_C55_REG_VC_START_SIZE 0x00094
> +#define MALI_C55_VC_START(v) ((v) & 0xffff)
> +#define MALI_C55_VC_SIZE(v) (((v) & 0xffff) << 16)
> +
> +/* Ping/Pong Configuration Space */
> +#define MALI_C55_REG_BASE_ADDR 0x18e88
> +#define MALI_C55_REG_BYPASS_0 0x18eac
> +#define MALI_C55_REG_BYPASS_0_VIDEO_TEST BIT(0)
> +#define MALI_C55_REG_BYPASS_0_INPUT_FMT BIT(1)
> +#define MALI_C55_REG_BYPASS_0_DECOMPANDER BIT(2)
> +#define MALI_C55_REG_BYPASS_0_SENSOR_OFFSET_WDR BIT(3)
> +#define MALI_C55_REG_BYPASS_0_GAIN_WDR BIT(4)
> +#define MALI_C55_REG_BYPASS_0_FRAME_STITCH BIT(5)
> +#define MALI_C55_REG_BYPASS_1 0x18eb0
> +#define MALI_C55_REG_BYPASS_1_DIGI_GAIN BIT(0)
> +#define MALI_C55_REG_BYPASS_1_FE_SENSOR_OFFS BIT(1)
> +#define MALI_C55_REG_BYPASS_1_FE_SQRT BIT(2)
> +#define MALI_C55_REG_BYPASS_1_RAW_FE BIT(3)
> +#define MALI_C55_REG_BYPASS_2 0x18eb8
> +#define MALI_C55_REG_BYPASS_2_SINTER BIT(0)
> +#define MALI_C55_REG_BYPASS_2_TEMPER BIT(1)
> +#define MALI_C55_REG_BYPASS_3 0x18ebc
> +#define MALI_C55_REG_BYPASS_3_SQUARE_BE BIT(0)
> +#define MALI_C55_REG_BYPASS_3_SENSOR_OFFSET_PRE_SH BIT(1)
> +#define MALI_C55_REG_BYPASS_3_MESH_SHADING BIT(3)
> +#define MALI_C55_REG_BYPASS_3_WHITE_BALANCE BIT(4)
> +#define MALI_C55_REG_BYPASS_3_IRIDIX BIT(5)
> +#define MALI_C55_REG_BYPASS_3_IRIDIX_GAIN BIT(6)
> +#define MALI_C55_REG_BYPASS_4 0x18ec0
> +#define MALI_C55_REG_BYPASS_4_DEMOSAIC_RGB BIT(1)
> +#define MALI_C55_REG_BYPASS_4_PF_CORRECTION BIT(3)
> +#define MALI_C55_REG_BYPASS_4_CCM BIT(4)
> +#define MALI_C55_REG_BYPASS_4_CNR BIT(5)
> +#define MALI_C55_REG_FR_BYPASS 0x18ec4
> +#define MALI_C55_REG_DS_BYPASS 0x18ec8
> +#define MALI_C55_BYPASS_CROP BIT(0)
> +#define MALI_C55_BYPASS_SCALER BIT(1)
> +#define MALI_C55_BYPASS_GAMMA_RGB BIT(2)
> +#define MALI_C55_BYPASS_SHARPEN BIT(3)
> +#define MALI_C55_BYPASS_CS_CONV BIT(4)
> +#define MALI_C55_REG_ISP_RAW_BYPASS 0x18ecc
> +#define MALI_C55_ISP_RAW_BYPASS_BYPASS_MASK BIT(0)
> +#define MALI_C55_ISP_RAW_BYPASS_FR_BYPASS_MASK GENMASK(9, 8)
> +#define MALI_C55_ISP_RAW_BYPASS_RAW_FR_BYPASS (2 << 8)
> +#define MALI_C55_ISP_RAW_BYPASS_RGB_FR_BYPASS (1 << 8)
BIT() or make these unsigned.
...
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-resizer.c b/drivers/media/platform/arm/mali-c55/mali-c55-resizer.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f0b079a2322125ad6313d6cf9651afaf2180b96c
> --- /dev/null
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-resizer.c
...
> +static unsigned int mali_c55_rsz_calculate_bank(struct mali_c55 *mali_c55,
> + unsigned int rsz_in,
> + unsigned int rsz_out)
> +{
> + unsigned int rsz_ratio = (rsz_out * 1000U) / rsz_in;
Can this overflow?
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(mali_c55_rsz_coef_banks_range_start); i++)
> + if (rsz_ratio >= mali_c55_rsz_coef_banks_range_start[i])
> + break;
> +
> + return i;
> +}
...
> +static int mali_c55_rsz_set_sink_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_format *format)
> +{
> + struct v4l2_mbus_framefmt *fmt = &format->format;
> + struct v4l2_mbus_framefmt *sink_fmt;
> + unsigned int active_sink;
> + struct v4l2_rect *rect;
> +
> + sink_fmt = v4l2_subdev_state_get_format(state, format->pad, 0);
> +
> + /*
> + * Clamp to min/max and then reset crop and compose rectangles to the
> + * newly applied size.
> + */
> + sink_fmt->width = clamp_t(unsigned int, fmt->width,
> + MALI_C55_MIN_WIDTH, MALI_C55_MAX_WIDTH);
> + sink_fmt->height = clamp_t(unsigned int, fmt->height,
> + MALI_C55_MIN_HEIGHT, MALI_C55_MAX_HEIGHT);
> +
> + /*
> + * Make sure the media bus code for the bypass pad is one of the
> + * supported ISP input media bus codes. Default it to SRGGB otherwise.
> + */
> + if (format->pad == MALI_C55_RSZ_SINK_BYPASS_PAD)
> + sink_fmt->code = mali_c55_isp_get_mbus_config_by_code(fmt->code) ?
> + fmt->code : MEDIA_BUS_FMT_SRGGB20_1X20;
> +
> + *fmt = *sink_fmt;
> +
> + if (format->pad == MALI_C55_RSZ_SINK_PAD) {
> + rect = v4l2_subdev_state_get_crop(state, format->pad);
> + rect->left = 0;
> + rect->top = 0;
> + rect->width = fmt->width;
> + rect->height = fmt->height;
> +
> + rect = v4l2_subdev_state_get_compose(state, format->pad);
> + rect->left = 0;
> + rect->top = 0;
> + rect->width = fmt->width;
> + rect->height = fmt->height;
If both of the rects are the same, you can simply assign the former to
the latter.
Overall, this seems like a nicely written driver. It's a very big one,
too...
--
Kind regards,
Sakari Ailus
next prev parent reply other threads:[~2025-06-28 20:13 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-24 10:21 [PATCH v10 00/17] Add Arm Mali-C55 Image Signal Processor Driver Daniel Scally
2025-06-24 10:21 ` [PATCH v10 01/17] media: uapi: Add MEDIA_BUS_FMT_RGB202020_1X60 format code Daniel Scally
2025-06-24 10:21 ` [PATCH v10 02/17] media: uapi: Add 20-bit bayer formats Daniel Scally
2025-06-24 10:21 ` [PATCH v10 03/17] media: v4l2-common: Add RAW16 format info Daniel Scally
2025-06-24 10:21 ` [PATCH v10 04/17] media: v4l2-common: Add RAW14 " Daniel Scally
2025-06-24 10:21 ` [PATCH v10 05/17] dt-bindings: media: Add bindings for ARM mali-c55 Daniel Scally
2025-06-25 3:27 ` Rob Herring
2025-06-25 9:05 ` Krzysztof Kozlowski
2025-06-25 9:08 ` Krzysztof Kozlowski
2025-06-25 9:46 ` Dan Scally
2025-07-10 15:19 ` Dan Scally
2025-06-24 10:21 ` [PATCH v10 06/17] media: uapi: Add controls for Mali-C55 ISP Daniel Scally
2025-06-28 19:29 ` Sakari Ailus
2025-06-24 10:21 ` [PATCH v10 07/17] media: mali-c55: Add Mali-C55 ISP driver Daniel Scally
2025-06-28 20:06 ` Sakari Ailus [this message]
2025-06-29 18:35 ` Laurent Pinchart
2025-06-30 7:37 ` Sakari Ailus
2025-06-30 8:35 ` Laurent Pinchart
2025-06-30 10:16 ` Dan Scally
2025-06-30 10:29 ` Sakari Ailus
2025-06-30 10:14 ` Dan Scally
2025-06-24 10:21 ` [PATCH v10 08/17] media: Documentation: Add Mali-C55 ISP Documentation Daniel Scally
2025-06-24 10:21 ` [PATCH v10 09/17] MAINTAINERS: Add entry for mali-c55 driver Daniel Scally
2025-06-24 10:21 ` [PATCH v10 10/17] media: Add MALI_C55_3A_STATS meta format Daniel Scally
2025-06-24 10:21 ` [PATCH v10 11/17] media: uapi: Add 3a stats buffer for mali-c55 Daniel Scally
2025-06-24 10:21 ` [PATCH v10 12/17] media: platform: Add mali-c55 3a stats devnode Daniel Scally
2025-06-24 10:21 ` [PATCH v10 13/17] Documentation: mali-c55: Add Statistics documentation Daniel Scally
2025-06-24 10:21 ` [PATCH v10 14/17] media: mali-c55: Add image formats for Mali-C55 parameters buffer Daniel Scally
2025-06-24 10:21 ` [PATCH v10 15/17] media: uapi: Add parameters structs to mali-c55-config.h Daniel Scally
2025-06-24 10:21 ` [PATCH v10 16/17] media: platform: Add mali-c55 parameters video node Daniel Scally
2025-06-29 11:27 ` Sakari Ailus
2025-06-30 10:40 ` Dan Scally
2025-06-30 13:59 ` Jacopo Mondi
2025-06-30 14:52 ` Sakari Ailus
2025-06-24 10:21 ` [PATCH v10 17/17] Documentation: mali-c55: Document the mali-c55 parameter setting Daniel Scally
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cee962ce-3719-4ae7-9849-548a95d98e99@linux.intel.com \
--to=sakari.ailus@linux.intel.com \
--cc=Anthony.McGivern@arm.com \
--cc=conor+dt@kernel.org \
--cc=dan.scally@ideasonboard.com \
--cc=devicetree@vger.kernel.org \
--cc=jacopo.mondi@ideasonboard.com \
--cc=jerome.forissier@linaro.org \
--cc=kieran.bingham@ideasonboard.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=nayden.kanchev@arm.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.