* [PATCH v12 2/3] of: Factor arguments passed to of_map_id() into a struct
From: Vijayanand Jitta @ 2026-03-31 14:04 UTC (permalink / raw)
To: Nipun Gupta, Nikhil Agarwal, Joerg Roedel, Will Deacon,
Robin Murphy, Marc Zyngier, Lorenzo Pieralisi, Thomas Gleixner,
Saravana Kannan, Richard Zhu, Lucas Stach,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Dmitry Baryshkov, Konrad Dybcio, Bjorn Andersson, Rob Herring,
Conor Dooley, Krzysztof Kozlowski, Prakash Gupta, Vikash Garodia
Cc: linux-kernel, iommu, linux-arm-kernel, devicetree, linux-pci, imx,
xen-devel, linux-arm-msm, Vijayanand Jitta, Charan Teja Kalla
In-Reply-To: <20260331-parse_iommu_cells-v12-0-decfd305eea9@oss.qualcomm.com>
From: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
Change of_map_id() to take a pointer to struct of_phandle_args
instead of passing target device node and translated IDs separately.
Update all callers accordingly.
Add an explicit filter_np parameter to of_map_id() and of_map_msi_id()
to separate the filter input from the output. Previously, the target
parameter served dual purpose: as an input filter (if non-NULL, only
match entries targeting that node) and as an output (receiving the
matched node with a reference held). Now filter_np is the explicit
input filter and arg->np is the pure output.
Previously, of_map_id() would call of_node_put() on the matched node
when a filter was provided, making reference ownership inconsistent.
Remove this internal of_node_put() call so that of_map_id() now always
transfers ownership of the matched node reference to the caller via
arg->np. Callers are now consistently responsible for releasing this
reference with of_node_put(arg->np) when done.
Suggested-by: Rob Herring (Arm) <robh@kernel.org>
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
drivers/cdx/cdx_msi.c | 7 ++--
drivers/iommu/of_iommu.c | 4 +-
drivers/irqchip/irq-gic-its-msi-parent.c | 11 ++++--
drivers/of/base.c | 68 +++++++++++++++++---------------
drivers/of/irq.c | 10 ++++-
drivers/pci/controller/dwc/pci-imx6.c | 32 +++++++--------
drivers/pci/controller/pcie-apple.c | 5 ++-
drivers/xen/grant-dma-ops.c | 4 +-
include/linux/of.h | 14 ++++---
9 files changed, 89 insertions(+), 66 deletions(-)
diff --git a/drivers/cdx/cdx_msi.c b/drivers/cdx/cdx_msi.c
index 63b3544ec997..6924e07c7528 100644
--- a/drivers/cdx/cdx_msi.c
+++ b/drivers/cdx/cdx_msi.c
@@ -121,22 +121,23 @@ static int cdx_msi_prepare(struct irq_domain *msi_domain,
struct device *dev,
int nvec, msi_alloc_info_t *info)
{
+ struct of_phandle_args msi_spec = {};
struct cdx_device *cdx_dev = to_cdx_device(dev);
struct device *parent = cdx_dev->cdx->dev;
struct msi_domain_info *msi_info;
- u32 dev_id;
int ret;
/* Retrieve device ID from requestor ID using parent device */
- ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, NULL, &dev_id);
+ ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, NULL, &msi_spec);
if (ret) {
dev_err(dev, "of_map_id failed for MSI: %d\n", ret);
return ret;
}
+ of_node_put(msi_spec.np);
#ifdef GENERIC_MSI_DOMAIN_OPS
/* Set the device Id to be passed to the GIC-ITS */
- info->scratchpad[0].ul = dev_id;
+ info->scratchpad[0].ul = msi_spec.args[0];
#endif
msi_info = msi_get_domain_info(msi_domain->parent);
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a511ecf21fcd..a18bb60f6f3d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -45,10 +45,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
{
- struct of_phandle_args iommu_spec = { .args_count = 1 };
+ struct of_phandle_args iommu_spec = {};
int err;
- err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
+ err = of_map_iommu_id(master_np, *id, &iommu_spec);
if (err)
return err;
diff --git a/drivers/irqchip/irq-gic-its-msi-parent.c b/drivers/irqchip/irq-gic-its-msi-parent.c
index b63343a227a9..dd5f84b6470a 100644
--- a/drivers/irqchip/irq-gic-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-its-msi-parent.c
@@ -152,6 +152,8 @@ static int its_v5_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
static int of_pmsi_get_msi_info(struct irq_domain *domain, struct device *dev, u32 *dev_id,
phys_addr_t *pa)
{
+ struct device_node *msi_ctrl __free(device_node) = NULL;
+ struct of_phandle_args msi_spec = {};
struct of_phandle_iterator it;
int ret;
@@ -178,9 +180,12 @@ static int of_pmsi_get_msi_info(struct irq_domain *domain, struct device *dev, u
}
}
- struct device_node *msi_ctrl __free(device_node) = NULL;
-
- return of_map_msi_id(dev->of_node, dev->id, &msi_ctrl, dev_id);
+ ret = of_map_msi_id(dev->of_node, dev->id, NULL, &msi_spec);
+ if (!ret) {
+ msi_ctrl = msi_spec.np;
+ *dev_id = msi_spec.args[0];
+ }
+ return ret;
}
static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ae04487bd614..b3d002015192 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2102,36 +2102,37 @@ int of_find_last_cache_level(unsigned int cpu)
* @id: device ID to map.
* @map_name: property name of the map to use.
* @map_mask_name: optional property name of the mask to use.
- * @target: optional pointer to a target device node.
- * @id_out: optional pointer to receive the translated ID.
+ * @filter_np: optional device node to filter matches by, or NULL to match any.
+ * If non-NULL, only map entries targeting this node will be matched.
+ * @arg: pointer to a &struct of_phandle_args for the result. On success,
+ * @arg->args[0] will contain the translated ID. If a map entry was
+ * matched, @arg->np will be set to the target node with a reference
+ * held that the caller must release with of_node_put().
*
* Given a device ID, look up the appropriate implementation-defined
* platform ID and/or the target device which receives transactions on that
- * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
- * @id_out may be NULL if only the other is required. If @target points to
- * a non-NULL device node pointer, only entries targeting that node will be
- * matched; if it points to a NULL value, it will receive the device node of
- * the first matching target phandle, with a reference held.
+ * ID, as per the "iommu-map" and "msi-map" bindings.
*
* Return: 0 on success or a standard error code on failure.
*/
int of_map_id(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+ const struct device_node *filter_np, struct of_phandle_args *arg)
{
u32 map_mask, masked_id;
int map_len;
const __be32 *map = NULL;
- if (!np || !map_name || (!target && !id_out))
+ if (!np || !map_name || !arg)
return -EINVAL;
map = of_get_property(np, map_name, &map_len);
if (!map) {
- if (target)
+ if (filter_np)
return -ENODEV;
/* Otherwise, no map implies no translation */
- *id_out = id;
+ arg->args[0] = id;
+ arg->args_count = 1;
return 0;
}
@@ -2173,18 +2174,14 @@ int of_map_id(const struct device_node *np, u32 id,
if (!phandle_node)
return -ENODEV;
- if (target) {
- if (*target)
- of_node_put(phandle_node);
- else
- *target = phandle_node;
-
- if (*target != phandle_node)
- continue;
+ if (filter_np && filter_np != phandle_node) {
+ of_node_put(phandle_node);
+ continue;
}
- if (id_out)
- *id_out = masked_id - id_base + out_base;
+ arg->np = phandle_node;
+ arg->args[0] = masked_id - id_base + out_base;
+ arg->args_count = 1;
pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
np, map_name, map_mask, id_base, out_base,
@@ -2193,11 +2190,11 @@ int of_map_id(const struct device_node *np, u32 id,
}
pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
- id, target && *target ? *target : NULL);
+ id, filter_np);
/* Bypasses translation */
- if (id_out)
- *id_out = id;
+ arg->args[0] = id;
+ arg->args_count = 1;
return 0;
}
EXPORT_SYMBOL_GPL(of_map_id);
@@ -2207,17 +2204,19 @@ EXPORT_SYMBOL_GPL(of_map_id);
* @np: root complex device node.
* @id: Requester ID of the device (e.g. PCI RID/BDF or a platform
* stream/device ID) used as the lookup key in the iommu-map table.
- * @target: optional pointer to a target device node.
- * @id_out: optional pointer to receive the translated ID.
+ * @arg: pointer to a &struct of_phandle_args for the result. On success,
+ * @arg->args[0] contains the translated ID. If a map entry was matched,
+ * @arg->np holds a reference to the target node that the caller must
+ * release with of_node_put().
*
* Convenience wrapper around of_map_id() using "iommu-map" and "iommu-map-mask".
*
* Return: 0 on success or a standard error code on failure.
*/
int of_map_iommu_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ struct of_phandle_args *arg)
{
- return of_map_id(np, id, "iommu-map", "iommu-map-mask", target, id_out);
+ return of_map_id(np, id, "iommu-map", "iommu-map-mask", NULL, arg);
}
EXPORT_SYMBOL_GPL(of_map_iommu_id);
@@ -2226,16 +2225,21 @@ EXPORT_SYMBOL_GPL(of_map_iommu_id);
* @np: root complex device node.
* @id: Requester ID of the device (e.g. PCI RID/BDF or a platform
* stream/device ID) used as the lookup key in the msi-map table.
- * @target: optional pointer to a target device node.
- * @id_out: optional pointer to receive the translated ID.
+ * @filter_np: optional MSI controller node to filter matches by, or NULL
+ * to match any. If non-NULL, only map entries targeting this node will
+ * be matched.
+ * @arg: pointer to a &struct of_phandle_args for the result. On success,
+ * @arg->args[0] contains the translated ID. If a map entry was matched,
+ * @arg->np holds a reference to the target node that the caller must
+ * release with of_node_put().
*
* Convenience wrapper around of_map_id() using "msi-map" and "msi-map-mask".
*
* Return: 0 on success or a standard error code on failure.
*/
int of_map_msi_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ const struct device_node *filter_np, struct of_phandle_args *arg)
{
- return of_map_id(np, id, "msi-map", "msi-map-mask", target, id_out);
+ return of_map_id(np, id, "msi-map", "msi-map-mask", filter_np, arg);
}
EXPORT_SYMBOL_GPL(of_map_msi_id);
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index e37c1b3f8736..f86a56bd81fc 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -817,8 +817,16 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
* "msi-map" or an "msi-parent" property.
*/
for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
- if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &id_out))
+ struct of_phandle_args msi_spec = {};
+
+ if (!of_map_msi_id(parent_dev->of_node, id_in, *msi_np, &msi_spec)) {
+ id_out = msi_spec.args[0];
+ if (!*msi_np)
+ *msi_np = msi_spec.np;
+ else
+ of_node_put(msi_spec.np);
break;
+ }
if (!of_check_msi_parent(parent_dev->of_node, msi_np))
break;
}
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index bff8289f804a..c0544d9c0921 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1137,30 +1137,32 @@ static void imx_pcie_remove_lut(struct imx_pcie *imx_pcie, u16 rid)
static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
{
+ struct of_phandle_args iommu_spec = {};
+ struct of_phandle_args msi_spec = {};
struct device *dev = imx_pcie->pci->dev;
- struct device_node *target;
u32 sid_i, sid_m;
int err_i, err_m;
u32 sid = 0;
- target = NULL;
- err_i = of_map_iommu_id(dev->of_node, rid, &target, &sid_i);
- if (target) {
- of_node_put(target);
- } else {
+ err_i = of_map_iommu_id(dev->of_node, rid, &iommu_spec);
+ if (!err_i)
+ sid_i = iommu_spec.args[0];
+ of_node_put(iommu_spec.np);
+ if (!err_i && !iommu_spec.np) {
/*
- * "target == NULL && err_i == 0" means RID out of map range.
- * Use 1:1 map RID to streamID. Hardware can't support this
- * because the streamID is only 6 bits
+ * "iommu_spec.np == NULL && err_i == 0" means RID out of map
+ * range. Use 1:1 map RID to streamID. Hardware can't support
+ * this because the streamID is only 6 bits.
*/
err_i = -EINVAL;
}
- target = NULL;
- err_m = of_map_msi_id(dev->of_node, rid, &target, &sid_m);
-
+ err_m = of_map_msi_id(dev->of_node, rid, NULL, &msi_spec);
+ if (!err_m)
+ sid_m = msi_spec.args[0];
+ of_node_put(msi_spec.np);
/*
- * err_m target
+ * err_m msi_spec.np
* 0 NULL RID out of range. Use 1:1 map RID to
* streamID, Current hardware can't
* support it, so return -EINVAL.
@@ -1168,10 +1170,8 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
* 0 != NULL Get correct streamID from RID
* != 0 != NULL Invalid combination
*/
- if (!err_m && !target)
+ if (!err_m && !msi_spec.np)
return -EINVAL;
- else if (target)
- of_node_put(target); /* Find streamID map entry for RID in msi-map */
/*
* msi-map iommu-map
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index a0937b7b3c4d..c2cffc0659f4 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -755,6 +755,7 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
{
u32 sid, rid = pci_dev_id(pdev);
struct apple_pcie_port *port;
+ struct of_phandle_args iommu_spec = {};
int idx, err;
port = apple_pcie_get_port(pdev);
@@ -764,10 +765,12 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
dev_dbg(&pdev->dev, "added to bus %s, index %d\n",
pci_name(pdev->bus->self), port->idx);
- err = of_map_iommu_id(port->pcie->dev->of_node, rid, NULL, &sid);
+ err = of_map_iommu_id(port->pcie->dev->of_node, rid, &iommu_spec);
if (err)
return err;
+ of_node_put(iommu_spec.np);
+ sid = iommu_spec.args[0];
mutex_lock(&port->pcie->lock);
idx = bitmap_find_free_region(port->sid_map, port->sid_map_sz, 0);
diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index 1b7696b2d762..2aa1a772a0ff 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -319,13 +319,13 @@ static int xen_dt_grant_init_backend_domid(struct device *dev,
struct device_node *np,
domid_t *backend_domid)
{
- struct of_phandle_args iommu_spec = { .args_count = 1 };
+ struct of_phandle_args iommu_spec = {};
if (dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(dev);
u32 rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
- if (of_map_iommu_id(np, rid, &iommu_spec.np, iommu_spec.args)) {
+ if (of_map_iommu_id(np, rid, &iommu_spec)) {
dev_dbg(dev, "Cannot translate ID\n");
return -ESRCH;
}
diff --git a/include/linux/of.h b/include/linux/of.h
index fe841f3cc747..8548cd9eb4f1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -463,13 +463,13 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
int of_map_id(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out);
+ const struct device_node *filter_np, struct of_phandle_args *arg);
int of_map_iommu_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out);
+ struct of_phandle_args *arg);
int of_map_msi_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out);
+ const struct device_node *filter_np, struct of_phandle_args *arg);
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
@@ -935,19 +935,21 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
static inline int of_map_id(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+ const struct device_node *filter_np,
+ struct of_phandle_args *arg)
{
return -EINVAL;
}
static inline int of_map_iommu_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ struct of_phandle_args *arg)
{
return -EINVAL;
}
static inline int of_map_msi_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ const struct device_node *filter_np,
+ struct of_phandle_args *arg)
{
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v12 1/3] of: Add convenience wrappers for of_map_id()
From: Vijayanand Jitta @ 2026-03-31 14:04 UTC (permalink / raw)
To: Nipun Gupta, Nikhil Agarwal, Joerg Roedel, Will Deacon,
Robin Murphy, Marc Zyngier, Lorenzo Pieralisi, Thomas Gleixner,
Saravana Kannan, Richard Zhu, Lucas Stach,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Dmitry Baryshkov, Konrad Dybcio, Bjorn Andersson, Rob Herring,
Conor Dooley, Krzysztof Kozlowski, Prakash Gupta, Vikash Garodia
Cc: linux-kernel, iommu, linux-arm-kernel, devicetree, linux-pci, imx,
xen-devel, linux-arm-msm, Vijayanand Jitta
In-Reply-To: <20260331-parse_iommu_cells-v12-0-decfd305eea9@oss.qualcomm.com>
From: Robin Murphy <robin.murphy@arm.com>
Since we now have quite a few users parsing "iommu-map" and "msi-map"
properties, give them some wrappers to conveniently encapsulate the
appropriate sets of property names. This will also make it easier to
then change of_map_id() to correctly account for specifier cells.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Acked-by: Marc Zyngier <maz@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
drivers/cdx/cdx_msi.c | 3 +--
drivers/iommu/of_iommu.c | 4 +---
drivers/irqchip/irq-gic-its-msi-parent.c | 2 +-
drivers/of/base.c | 38 ++++++++++++++++++++++++++++++++
drivers/of/irq.c | 3 +--
drivers/pci/controller/dwc/pci-imx6.c | 6 ++---
drivers/pci/controller/pcie-apple.c | 3 +--
drivers/xen/grant-dma-ops.c | 3 +--
include/linux/of.h | 18 +++++++++++++++
9 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/cdx/cdx_msi.c b/drivers/cdx/cdx_msi.c
index 91b95422b263..63b3544ec997 100644
--- a/drivers/cdx/cdx_msi.c
+++ b/drivers/cdx/cdx_msi.c
@@ -128,8 +128,7 @@ static int cdx_msi_prepare(struct irq_domain *msi_domain,
int ret;
/* Retrieve device ID from requestor ID using parent device */
- ret = of_map_id(parent->of_node, cdx_dev->msi_dev_id, "msi-map", "msi-map-mask",
- NULL, &dev_id);
+ ret = of_map_msi_id(parent->of_node, cdx_dev->msi_dev_id, NULL, &dev_id);
if (ret) {
dev_err(dev, "of_map_id failed for MSI: %d\n", ret);
return ret;
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def2..a511ecf21fcd 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -48,9 +48,7 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
struct of_phandle_args iommu_spec = { .args_count = 1 };
int err;
- err = of_map_id(master_np, *id, "iommu-map",
- "iommu-map-mask", &iommu_spec.np,
- iommu_spec.args);
+ err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
if (err)
return err;
diff --git a/drivers/irqchip/irq-gic-its-msi-parent.c b/drivers/irqchip/irq-gic-its-msi-parent.c
index d36b278ae66c..b63343a227a9 100644
--- a/drivers/irqchip/irq-gic-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-its-msi-parent.c
@@ -180,7 +180,7 @@ static int of_pmsi_get_msi_info(struct irq_domain *domain, struct device *dev, u
struct device_node *msi_ctrl __free(device_node) = NULL;
- return of_map_id(dev->of_node, dev->id, "msi-map", "msi-map-mask", &msi_ctrl, dev_id);
+ return of_map_msi_id(dev->of_node, dev->id, &msi_ctrl, dev_id);
}
static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 57420806c1a2..ae04487bd614 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2201,3 +2201,41 @@ int of_map_id(const struct device_node *np, u32 id,
return 0;
}
EXPORT_SYMBOL_GPL(of_map_id);
+
+/**
+ * of_map_iommu_id - Translate an ID using "iommu-map" bindings.
+ * @np: root complex device node.
+ * @id: Requester ID of the device (e.g. PCI RID/BDF or a platform
+ * stream/device ID) used as the lookup key in the iommu-map table.
+ * @target: optional pointer to a target device node.
+ * @id_out: optional pointer to receive the translated ID.
+ *
+ * Convenience wrapper around of_map_id() using "iommu-map" and "iommu-map-mask".
+ *
+ * Return: 0 on success or a standard error code on failure.
+ */
+int of_map_iommu_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out)
+{
+ return of_map_id(np, id, "iommu-map", "iommu-map-mask", target, id_out);
+}
+EXPORT_SYMBOL_GPL(of_map_iommu_id);
+
+/**
+ * of_map_msi_id - Translate an ID using "msi-map" bindings.
+ * @np: root complex device node.
+ * @id: Requester ID of the device (e.g. PCI RID/BDF or a platform
+ * stream/device ID) used as the lookup key in the msi-map table.
+ * @target: optional pointer to a target device node.
+ * @id_out: optional pointer to receive the translated ID.
+ *
+ * Convenience wrapper around of_map_id() using "msi-map" and "msi-map-mask".
+ *
+ * Return: 0 on success or a standard error code on failure.
+ */
+int of_map_msi_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out)
+{
+ return of_map_id(np, id, "msi-map", "msi-map-mask", target, id_out);
+}
+EXPORT_SYMBOL_GPL(of_map_msi_id);
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 6367c67732d2..e37c1b3f8736 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -817,8 +817,7 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
* "msi-map" or an "msi-parent" property.
*/
for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
- if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
- "msi-map-mask", msi_np, &id_out))
+ if (!of_map_msi_id(parent_dev->of_node, id_in, msi_np, &id_out))
break;
if (!of_check_msi_parent(parent_dev->of_node, msi_np))
break;
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index a5b8d0b71677..bff8289f804a 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1144,8 +1144,7 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
u32 sid = 0;
target = NULL;
- err_i = of_map_id(dev->of_node, rid, "iommu-map", "iommu-map-mask",
- &target, &sid_i);
+ err_i = of_map_iommu_id(dev->of_node, rid, &target, &sid_i);
if (target) {
of_node_put(target);
} else {
@@ -1158,8 +1157,7 @@ static int imx_pcie_add_lut_by_rid(struct imx_pcie *imx_pcie, u32 rid)
}
target = NULL;
- err_m = of_map_id(dev->of_node, rid, "msi-map", "msi-map-mask",
- &target, &sid_m);
+ err_m = of_map_msi_id(dev->of_node, rid, &target, &sid_m);
/*
* err_m target
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 2d92fc79f6dd..a0937b7b3c4d 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -764,8 +764,7 @@ static int apple_pcie_enable_device(struct pci_host_bridge *bridge, struct pci_d
dev_dbg(&pdev->dev, "added to bus %s, index %d\n",
pci_name(pdev->bus->self), port->idx);
- err = of_map_id(port->pcie->dev->of_node, rid, "iommu-map",
- "iommu-map-mask", NULL, &sid);
+ err = of_map_iommu_id(port->pcie->dev->of_node, rid, NULL, &sid);
if (err)
return err;
diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
index c2603e700178..1b7696b2d762 100644
--- a/drivers/xen/grant-dma-ops.c
+++ b/drivers/xen/grant-dma-ops.c
@@ -325,8 +325,7 @@ static int xen_dt_grant_init_backend_domid(struct device *dev,
struct pci_dev *pdev = to_pci_dev(dev);
u32 rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
- if (of_map_id(np, rid, "iommu-map", "iommu-map-mask", &iommu_spec.np,
- iommu_spec.args)) {
+ if (of_map_iommu_id(np, rid, &iommu_spec.np, iommu_spec.args)) {
dev_dbg(dev, "Cannot translate ID\n");
return -ESRCH;
}
diff --git a/include/linux/of.h b/include/linux/of.h
index be6ec4916adf..fe841f3cc747 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -465,6 +465,12 @@ int of_map_id(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out);
+int of_map_iommu_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out);
+
+int of_map_msi_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out);
+
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
struct kimage;
@@ -934,6 +940,18 @@ static inline int of_map_id(const struct device_node *np, u32 id,
return -EINVAL;
}
+static inline int of_map_iommu_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out)
+{
+ return -EINVAL;
+}
+
+static inline int of_map_msi_id(const struct device_node *np, u32 id,
+ struct device_node **target, u32 *id_out)
+{
+ return -EINVAL;
+}
+
static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
{
return PHYS_ADDR_MAX;
--
2.34.1
^ permalink raw reply related
* [PATCH v12 0/3] of: parsing of multi #{iommu,msi}-cells in maps
From: Vijayanand Jitta @ 2026-03-31 14:04 UTC (permalink / raw)
To: Nipun Gupta, Nikhil Agarwal, Joerg Roedel, Will Deacon,
Robin Murphy, Marc Zyngier, Lorenzo Pieralisi, Thomas Gleixner,
Saravana Kannan, Richard Zhu, Lucas Stach,
Krzysztof Wilczyński, Manivannan Sadhasivam, Bjorn Helgaas,
Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Dmitry Baryshkov, Konrad Dybcio, Bjorn Andersson, Rob Herring,
Conor Dooley, Krzysztof Kozlowski, Prakash Gupta, Vikash Garodia
Cc: linux-kernel, iommu, linux-arm-kernel, devicetree, linux-pci, imx,
xen-devel, linux-arm-msm, Vijayanand Jitta, Charan Teja Kalla
So far our parsing of {iommu,msi}-map properties has always blindly
assumed that the output specifiers will always have exactly 1 cell.
This typically does happen to be the case, but is not actually enforced
(and the PCI msi-map binding even explicitly states support for 0 or 1
cells) - as a result we've now ended up with dodgy DTs out in the field
which depend on this behaviour to map a 1-cell specifier for a 2-cell
provider, despite that being bogus per the bindings themselves.
Since there is some potential use[1] in being able to map at least
single input IDs to multi-cell output specifiers (and properly support
0-cell outputs as well), add support for properly parsing and using the
target nodes' #cells values, albeit with the unfortunate complication of
still having to work around expectations of the old behaviour too.
-- Robin.
Unlike single #{}-cell, it is complex to establish a linear relation
between input 'id' and output specifier for multi-cell properties, thus
it is always expected that len never going to be > 1.
These changes have been tested on QEMU for the arm64 architecture.
Since, this would also need update in dt-schema, raised PR[2] for the
same.
[1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
[2] PR for iommu-map dtschema: https://github.com/devicetree-org/dt-schema/pull/184
Robin,
Could this series be pulled into an immutable branch/tag, if it doesn't make
it into the v7.1 merge window? There are client changes dependent on it,
So it would help to get them moving forward rather than waiting another
cycle.
Thanks,
Vijay
V12:
- Call of_node_put() unconditionally in imx_pcie_add_lut_by_rid()
thereby addressing comments from Bjorn Helgaas.
Link to v11:
https://lore.kernel.org/r/20260325-parse_iommu_cells-v11-0-1fefa5c0e82c@oss.qualcomm.com
V11:
- Added explicit filter_np parameter to of_map_id() and of_map_msi_id()
per Dmitry Baryshkov's review feedback, making the filter explicit
instead of overloading arg->np as both input filter and output parameter.
- Removed of_node_put() from inside of_map_id(), making the caller responsible
for reference management. Updated of_msi_xlate() to properly handle reference counting.
- Collected ACKed by tags, and fixed minor typos.
Link to v10:
https://lore.kernel.org/r/20260309-parse_iommu_cells-v10-0-c62fcaa5a1d8@oss.qualcomm.com
V10:
- Move of_map_iommu_id()/of_map_msi_id() from include/linux/of.h to
drivers/of/base.c as out-of-line helpers per feedback from Marc Zyngier
and Rob Herring.
- Add kernel-doc to document both helpers for discoverability and
usage clarity.
- Fix of_map_msi_id() wrapper and all its callers (cdx_msi.c,
irq-gic-its-msi-parent.c, drivers/of/irq.c) to correctly use the new
struct of_phandle_args-based API with proper of_node_put() handling
as per feeback from Dmitry.
Link to v9:
https://lore.kernel.org/r/20260301-parse_iommu_cells-v9-0-4d1bceecc5e1@oss.qualcomm.com
V9:
- Updated TO/CC list based on feedback to include all relevant
maintainers.
- No functional changes to the patches themselves.
Link to V8:
https://lore.kernel.org/all/20260226074245.3098486-1-vijayanand.jitta@oss.qualcomm.com/
V8:
- Removed mentions of of_map_args from commit message to match code.
Link to V7:
https://lore.kernel.org/all/20260210101157.2145113-1-vijayanand.jitta@oss.qualcomm.com/
V7:
- Removed of_map_id_args structure and replaced it with
of_phandle_args as suggested by Dmitry.
Link to V6:
https://lore.kernel.org/all/20260121055400.937856-1-vijayanand.jitta@oss.qualcomm.com/
V6:
- Fixed build error reported by kernel test bot.
Link to V5:
https://lore.kernel.org/all/20260118181125.1436036-1-vijayanand.jitta@oss.qualcomm.com/
V5:
- Fixed Build Warnings.
- Raised PR for iommu-map dtschema: https://github.com/devicetree-org/dt-schema/pull/184
Link to V4:
https://lore.kernel.org/all/20251231114257.2382820-1-vijayanand.jitta@oss.qualcomm.com/
V4:
- Added Reviewed-by tag.
- Resolved warnings reported by kernel test bot, minor code
reorganization.
Link to V3:
https://lore.kernel.org/all/20251221213602.2413124-1-vijayanand.jitta@oss.qualcomm.com/
V3:
- Added Reviewed-by tag.
- Updated of_map_id_args struct as a wrapper to of_phandle_args and
added comment description as suggested by Rob Herring.
Link to V2:
https://lore.kernel.org/all/20251204095530.8627-1-vijayanand.jitta@oss.qualcomm.com/
V2:
- Incorporated the patches from Robin that does the clean implementation.
- Dropped the patches the were adding multi-map support from this series
as suggested.
V1:
https://lore.kernel.org/all/cover.1762235099.git.charan.kalla@oss.qualcomm.com/
RFC:
https://lore.kernel.org/all/20250928171718.436440-1-charan.kalla@oss.qualcomm.com/#r
Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
---
Charan Teja Kalla (1):
of: Factor arguments passed to of_map_id() into a struct
Robin Murphy (2):
of: Add convenience wrappers for of_map_id()
of: Respect #{iommu,msi}-cells in maps
drivers/cdx/cdx_msi.c | 8 +-
drivers/iommu/of_iommu.c | 6 +-
drivers/irqchip/irq-gic-its-msi-parent.c | 11 +-
drivers/of/base.c | 213 ++++++++++++++++++++++++-------
drivers/of/irq.c | 11 +-
drivers/pci/controller/dwc/pci-imx6.c | 34 +++--
drivers/pci/controller/pcie-apple.c | 6 +-
drivers/xen/grant-dma-ops.c | 5 +-
include/linux/of.h | 30 ++++-
9 files changed, 240 insertions(+), 84 deletions(-)
---
base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31
change-id: 20260301-parse_iommu_cells-1c33768aebba
Best regards,
--
Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Pekka Paalanen @ 2026-03-31 13:52 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Maxime Ripard, Nicolas Frattaroli, Harry Wentland, Leo Li,
Rodrigo Siqueira, Alex Deucher, Christian König,
David Airlie, Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Sandy Huang, Heiko Stübner,
Andy Yan, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Dmitry Baryshkov, Sascha Hauer, Rob Herring,
Jonathan Corbet, Shuah Khan, kernel, amd-gfx, dri-devel,
linux-kernel, linux-arm-kernel, linux-rockchip, intel-gfx,
intel-xe, linux-doc, Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <accfKbYPbH7EgAMc@intel.com>
[-- Attachment #1: Type: text/plain, Size: 11077 bytes --]
On Sat, 28 Mar 2026 02:22:01 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Mar 26, 2026 at 07:58:25PM +0200, Ville Syrjälä wrote:
> > On Thu, Mar 26, 2026 at 06:02:47PM +0100, Maxime Ripard wrote:
> > > On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > > > > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > > > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > > > +enum drm_connector_color_format {
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > > > > + * helpers should pick a suitable color format. All implementations of a
> > > > > > > > > > > + * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > > > > + * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > > > > + * semantics.
> > > > > > > > > > > + *
> > > > > > > > > > > + * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > > > > + * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > > > > + * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > > > > + * YCbCr 4:2:0.
> > > > > > > > > > > + *
> > > > > > > > > > > + * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > > > > + * format selection picks the first chain of bridge formats that works,
> > > > > > > > > > > + * as has already been the case before the introduction of the "color
> > > > > > > > > > > + * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > > > > + * bus output formats by preference, or agree on a unified auto format
> > > > > > > > > > > + * selection logic that's implemented in a common state helper (like
> > > > > > > > > > > + * how HDMI does it).
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > > > > + * not subsampled)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > > > > + * with horizontal subsampling)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > > > > + * with horizontal and vertical subsampling)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > > > >
> > > > > > > > > > Seems like this should document what the quantization range
> > > > > > > > > > should be for each format.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > > > > enum is more abstract than that, and is there to communicate
> > > > > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > > > > by other properties.
> > > > > > > > >
> > > > > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > > > > be added; no digital display protocol standard supports it to my
> > > > > > > > > knowledge, and hopefully none ever will.
> > > > > > > >
> > > > > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > > > >
> > > > > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > > > > RGB can either be full or limited range depending on the
> > > > > > > > "Broadcast RGB" property and other related factors.
> > > > > > >
> > > > > > > So far the HDMI state has both the format and quantization range as
> > > > > > > different fields. I'm not sure we need to document the range in the
> > > > > > > format field, maybe only mention it's not part of the format but has a
> > > > > > > field of its own?
> > > > > >
> > > > > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > > > > I think the assumption is limited range everywhere.
> > > > > >
> > > > > > But I'm not really concerned about documenting struct members.
> > > > > > What I'm talking about is the *uapi* docs. Surely userspace
> > > > > > will want to know what the new property actually does so the
> > > > > > uapi needs to be documented properly. And down the line some
> > > > > > new driver might also implement the wrong behaviour if there
> > > > > > is no clear specification.
> > > > >
> > > > > Ack
> > > > >
> > > > > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > > > > - YCbCr limited range
> > > > > > - RGB full range if "Broadcast RGB" property is not present
> > > > >
> > > > > Isn't it much more complicated than that for HDMI though? My
> > > > > recollection was that any VIC but VIC1 would be limited range, and
> > > > > anything else full range?
> > > >
> > > > Do we have some driver that implements the CTA-861 CE vs. IT mode
> > > > logic but doesn't expose the "Broadcast RGB" property? I was hoping
> > > > those would always go hand in hand now.
> > >
> > > I'm not sure. i915 and the HDMI state helpers handle it properly (I
> > > think?) but it looks like only vc4 registers the Broadcast RGB property
> > > and uses the HDMI state helpers.
> > >
> > > And it looks like amdgpu registers Broadcast RGB but doesn't use
> > > drm_default_rgb_quant_range() which seems suspicious?
> >
> > If they want just manual full vs. limited then they should
> > limit the property to not expose the "auto" option at all.
> >
> > amdgpu also ties this in with the "colorspace" property, which
> > originally in i915 only controlled the infoframes/etc. But on
> > amdgpu it now controls various aspects of output color
> > transformation. The end result is that the property is a complete
> > mess with most of the values making no sense. And for whatever
> > reason everyone involved refused to remove/deprecate the
> > nonsensical values :/
> >
> > Looks like this series should make sure the documentation for
> > the "colorspace" property is in sync with the new property
> > as well. Currently now it's giving conflicting information.
>
> After pondering about this a bit more I guess we could actually
> use this to make all the values of the colorspace property make
> some sense.
>
> Since we won't have to worry about that RGB->YCbCr 4:2:0
> fallback when using and explicit color format, all we'd have
> to do is explicitly reject the nonsensical combinations:
>
> color_format_and_colorspace_ok()
> {
> switch (color_format) {
> case DRM_CONNECTOR_COLOR_FORMAT_YCBCR444:
> case DRM_CONNECTOR_COLOR_FORMAT_YCBCR422:
> case DRM_CONNECTOR_COLOR_FORMAT_YCBCR420:
> switch (colorspace) {
> case DRM_MODE_COLORIMETRY_NO_DATA:
> case DRM_MODE_COLORIMETRY_SMPTE_170M_YCC:
> case DRM_MODE_COLORIMETRY_BT601_YCC:
> case DRM_MODE_COLORIMETRY_BT709_YCC:
> case DRM_MODE_COLORIMETRY_XVYCC_601:
> case DRM_MODE_COLORIMETRY_XVYCC_709:
> case DRM_MODE_COLORIMETRY_SYCC_601:
> case DRM_MODE_COLORIMETRY_OPYCC_601:
> case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> case DRM_MODE_COLORIMETRY_BT2020_YCC:
> case DRM_MODE_COLORIMETRY_BT709_YCC:
> return true;
> default:
> return false;
> }
> break;
> case DRM_CONNECTOR_COLOR_FORMAT_RGB444:
> switch (colorspace) {
> case DRM_MODE_COLORIMETRY_NO_DATA:
> case DRM_MODE_COLORIMETRY_OPRGB:
> case DRM_MODE_COLORIMETRY_BT2020_RGB:
> case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
> case DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED:
> case DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT:
> case DRM_MODE_COLORIMETRY_OPRGB:
> case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> case DRM_MODE_COLORIMETRY_BT2020_RGB:
> return true;
> default
> return false;
> }
> break;
> case DRM_CONNECTOR_COLOR_FORMAT_AUTO:
> switch (colorspace) {
> case DRM_MODE_COLORIMETRY_NO_DATA:
> case DRM_MODE_COLORIMETRY_BT2020_RGB:
> case DRM_MODE_COLORIMETRY_BT2020_YCC:
> return true;
> default:
> return false;
> }
> default:
> bad;
> }
> }
>
> And then presumably the colorspace property is the thing that should
> dictate which conversion matrix to use. So something like this:
>
> csc_matrix()
> {
> switch (colorspace) {
> case DRM_MODE_COLORIMETRY_SMPTE_170M_YCC:
> case DRM_MODE_COLORIMETRY_BT601_YCC:
> case DRM_MODE_COLORIMETRY_XVYCC_601:
> case DRM_MODE_COLORIMETRY_SYCC_601:
> case DRM_MODE_COLORIMETRY_OPYCC_601:
> return 601;
> case DRM_MODE_COLORIMETRY_BT709_YCC:
> case DRM_MODE_COLORIMETRY_XVYCC_709:
> return 709;
> case DRM_MODE_COLORIMETRY_BT2020_YCC:
> case DRM_MODE_COLORIMETRY_BT2020_RGB:
> return 2020;
> case DRM_MODE_COLORIMETRY_BT2020_CYCC:
> return 2020_const;
> case DRM_MODE_COLORIMETRY_NO_DATA:
> return vdisplay >= 720 ? 709 : 601;
> default:
> bad;
> }
> }
>
Hi Ville,
I believe the RGB-to-YCbCr matrix choice belongs in a colorop in the
CRTC color pipeline. It would be really nice to let go of the
requirement that the KMS color pipeline can be only in full-range RGB.
I imagine being able to scan out a YCbCr limited range buffer as-is
would be really useful for professional broadcasting industry and
particularly PLUGE testing which relies on being able to send the
sub-black and super-white YCbCr values for monitor calibration.
However, if there is an "auto" value in the colorop (implies the
input to the colorop must be full-range RGB) or the colorop is missing,
then your proposal could be used there.
Thanks,
pq
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 01/10] dt-bindings: mfd: add support for the NXP SIUL2 module
From: Khristine Andreea Barbulescu @ 2026-03-31 13:43 UTC (permalink / raw)
To: Arnd Bergmann, Krzysztof Kozlowski, Ghennadi Procopciuc
Cc: Linus Walleij, Bartosz Golaszewski, Krzysztof Kozlowski,
Conor Dooley, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
Larisa Grigore, Lee Jones, Shawn Guo, Sascha Hauer, Fabio Estevam,
Aisheng Dong, Jacky Bai, Greg Kroah-Hartman, Rafael J . Wysocki,
Alberto Ruiz, Christophe Lizzi, devicetree, Enric Balletbo,
Eric Chanudet, imx, linux-arm-kernel, open list:GPIO SUBSYSTEM,
linux-kernel, NXP S32 Linux Team, Pengutronix Kernel Team,
Vincent Guittot, Rob Herring
In-Reply-To: <e1c341d6-e60d-4200-a094-48667e8ccd5c@app.fastmail.com>
On 3/31/2026 1:11 PM, Arnd Bergmann wrote:
> On Tue, Mar 31, 2026, at 09:48, Khristine Andreea Barbulescu wrote:
>>
>> With the current layout, the SIUL2 node itself now contains the two
>> MMIO ranges directly, while the remaining child node is only the
>> pinctrl/GPIO function.
>
> The thread started by saying this is an MFD "It can export information
> about the SoC, configure the pinmux&pinconf for pins and it is also
> a GPIO controller with interrupt capability." Having a combined
> pinctrl/gpio/irqchip driver is normal, but can you clarify what
> you plan to do with the "information about the SoC" part?
>
> Was this a "soc_device" driver, or something else? Have you
> concluded now that this is not going to be needed at all?
> In that case, I guess having a monolithic driver is
> indeed simpler than an MFD.
>
Hi Arnd,
Our initial intention had been to expose that SoC-information as
discussed in the earlier revisions of this series. However,
taking the review feedback into account, the current direction is
to stop handling those SoC information registers in the Linux driver
altogether and instead rely on a boot firmware to pass that
information forward, as you suggested.
With this approach, the SIUL2 driver would no longer be responsible
for any separate SoC-information functionality. In that case,
I understand your point that a monolithic pinctrl/GPIO/irqchip
driver is a better fit than keeping the MFD structure.
> What I wonder about then is whether the binding needs to be changed
> at all. With the current nxp,s32g2-siul2-pinctrl.yaml binding
> and pinctrl-s32g2.c implementation, you seem to have a monolithic
> device already, though missing the gpio functionality. Rather
> than completely replacing this, I assume the easiest way then
> would be to add the PGPD registers into this device node, right?
>
There is indeed the option of extending the current
nxp,s32g2-siul2-pinctrl.yaml binding and adding the GPIO related
register areas to that existing device node.
> It is still a bit weird to list the individual register areas
> inside of the larger device here, but that still seems better
> than an incompatible binding change.
>
> Arnd
However, as you mentioned, this is still weird because it means
listing individual register areas of the larger device inside.
For this reason, I was wondering whether it would still be
acceptable to move forward with the new binding introduced
in this series, but simplify it so that it describes a single
monolithic SIUL2 pinctrl/GPIO device instead of an MFD,
following the example node I included in my previous reply [1].
[1] https://lore.kernel.org/linux-gpio/20260120115923.3463866-4-khristineandreea.barbulescu@oss.nxp.com/T/#m778088251774a15bde7463350d6e75d5e9b9b57d
Best regards,
Khristine
^ permalink raw reply
* Re: [PATCH v1] media: rkisp1: Add support for CAC
From: Laurent Pinchart @ 2026-03-31 13:39 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Barnabás Pőcze, Dafna Hirschfeld, Mauro Carvalho Chehab,
Heiko Stuebner, linux-media, linux-rockchip, linux-arm-kernel,
linux-kernel
In-Reply-To: <acu-j4hm0g8pTeAF@zed>
On Tue, Mar 31, 2026 at 02:42:33PM +0200, Jacopo Mondi wrote:
> On Mon, Mar 30, 2026 at 04:40:18PM +0200, Barnabás Pőcze wrote:
> > 2026. 03. 25. 16:21 keltezéssel, Jacopo Mondi írta:
> > > On Mon, Mar 23, 2026 at 03:02:16PM +0100, Barnabás Pőcze wrote:
> > > > The CAC block implements chromatic aberration correction. Expose it to
> > > > userspace using the extensible parameters format. This was tested on the
> > > > i.MX8MP platform, but based on available documentation it is also present
> > > > in the RK3399 variant (V10). Thus presumably also in later versions,
> > > > so no feature flag is introduced.
> > > >
> > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > >
> > > Only minors..
> > >
> > > > ---
> > > > .../platform/rockchip/rkisp1/rkisp1-params.c | 69 ++++++++++++
> > > > .../platform/rockchip/rkisp1/rkisp1-regs.h | 21 +++-
> > > > include/uapi/linux/rkisp1-config.h | 106 +++++++++++++++++-
> > > > 3 files changed, 193 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > > index 6442436a5e428..b889af9dcee45 100644
> > > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > > @@ -64,6 +64,7 @@ union rkisp1_ext_params_config {
> > > > struct rkisp1_ext_params_compand_bls_config compand_bls;
> > > > struct rkisp1_ext_params_compand_curve_config compand_curve;
> > > > struct rkisp1_ext_params_wdr_config wdr;
> > > > + struct rkisp1_ext_params_cac_config cac;
> > > > };
> > > >
> > > > enum rkisp1_params_formats {
> > > > @@ -1413,6 +1414,48 @@ static void rkisp1_wdr_config(struct rkisp1_params *params,
> > > > RKISP1_CIF_ISP_WDR_TONE_CURVE_YM_MASK);
> > > > }
> > > >
> > > > +static void
> > > > +rkisp1_cac_config(struct rkisp1_params *params,
> > > > + const struct rkisp1_cif_isp_cac_config *arg)
> > >
> > > Fits in one line without going over 80 cols
> >
> > This is what the other functions looks like, so went this this.
>
> It seems to me not all of them are broken, but only the ones that go
> over 80 cols
>
> in example:
>
> static void rkisp1_dpf_config(struct rkisp1_params *params,
> const struct rkisp1_cif_isp_dpf_config *arg)
>
> A detail anyway, up to you
>
> > > > +{
> > > > + u32 regval;
All other functions in this file name similar variables "val", "value"
or "reg_val". Let's not introduce a fourth one. I have a small
preference for "val", but that's not mandatory.
> > > > +
> > > > + /*
> > > > + * The enable bit is in the same register (RKISP1_CIF_ISP_CAC_CTRL),
> > > > + * so only set the clipping mode, and do not modify the other bits.
> > > > + */
> > > > + regval = rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_CAC_CTRL);
> > > > + regval &= ~(RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE |
> > > > + RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE);
> > > > + regval |= FIELD_PREP(RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE, arg->h_clip_mode) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE, arg->v_clip_mode);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_CTRL, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_COUNT_START_H_MASK, arg->h_count_start) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_COUNT_START_V_MASK, arg->v_count_start);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_COUNT_START, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_A_RED_MASK, arg->red[0]) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_A_BLUE_MASK, arg->blue[0]);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_A, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_B_RED_MASK, arg->red[1]) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_B_BLUE_MASK, arg->blue[1]);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_B, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_C_RED_MASK, arg->red[2]) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_C_BLUE_MASK, arg->blue[2]);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_C, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_X_NORM_NF_MASK, arg->x_nf) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_X_NORM_NS_MASK, arg->x_ns);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_X_NORM, regval);
> > > > +
> > > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_Y_NORM_NF_MASK, arg->y_nf) |
> > > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_Y_NORM_NS_MASK, arg->y_ns);
> > > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_Y_NORM, regval);
> > > > +}
> > > > +
> > > > static void
> > > > rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> > > > const struct rkisp1_params_cfg *new_params)
> > > > @@ -2089,6 +2132,25 @@ static void rkisp1_ext_params_wdr(struct rkisp1_params *params,
> > > > RKISP1_CIF_ISP_WDR_CTRL_ENABLE);
> > > > }
> > > >
> > > > +static void rkisp1_ext_params_cac(struct rkisp1_params *params,
> > > > + const union rkisp1_ext_params_config *block)
> > > > +{
> > > > + const struct rkisp1_ext_params_cac_config *cac = &block->cac;
> > > > +
> > > > + if (cac->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE) {
> > > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > > + return;
> > > > + }
> > > > +
> > > > + rkisp1_cac_config(params, &cac->config);
> > > > +
> > > > + if ((cac->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE) &&
> > > > + !(params->enabled_blocks & BIT(cac->header.type)))
> > > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > > +}
> > > > +
> > > > typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
> > > > const union rkisp1_ext_params_config *config);
> > > >
> > > > @@ -2185,6 +2247,10 @@ static const struct rkisp1_ext_params_handler {
> > > > .handler = rkisp1_ext_params_wdr,
> > > > .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS,
> > > > },
> > > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC] = {
> > > > + .handler = rkisp1_ext_params_cac,
> > > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS,
> > > > + },
> > > > };
> > > >
> > > > #define RKISP1_PARAMS_BLOCK_INFO(block, data) \
> > > > @@ -2215,6 +2281,7 @@ rkisp1_ext_params_block_types_info[] = {
> > > > RKISP1_PARAMS_BLOCK_INFO(COMPAND_EXPAND, compand_curve),
> > > > RKISP1_PARAMS_BLOCK_INFO(COMPAND_COMPRESS, compand_curve),
> > > > RKISP1_PARAMS_BLOCK_INFO(WDR, wdr),
> > > > + RKISP1_PARAMS_BLOCK_INFO(CAC, cac),
> > > > };
> > > >
> > > > static_assert(ARRAY_SIZE(rkisp1_ext_params_handlers) ==
> > > > @@ -2474,6 +2541,8 @@ void rkisp1_params_disable(struct rkisp1_params *params)
> > > > rkisp1_ie_enable(params, false);
> > > > rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > > > RKISP1_CIF_ISP_DPF_MODE_EN);
> > > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > > }
> > > >
> > > > static const struct rkisp1_params_ops rkisp1_v10_params_ops = {
> > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > > index fbeb186cde0d5..8e25537459bbd 100644
> > > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > > @@ -724,6 +724,23 @@
> > > > #define RKISP1_CIF_ISP_WDR_DMIN_STRENGTH_MASK GENMASK(20, 16)
> > > > #define RKISP1_CIF_ISP_WDR_DMIN_STRENGTH_MAX 16U
> > > >
> > > > +/* CAC */
> > > > +#define RKISP1_CIF_ISP_CAC_CTRL_ENABLE BIT(0)
> > > > +#define RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE GENMASK(2, 1)
> > > > +#define RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE GENMASK(3, 3)
I'd go for BIT(3) as you use BIT(0) for the enable bit.
> > > > +#define RKISP1_CIF_ISP_CAC_COUNT_START_H_MASK GENMASK(12, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_COUNT_START_V_MASK GENMASK(28, 16)
> > > > +#define RKISP1_CIF_ISP_CAC_A_RED_MASK GENMASK(8, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_A_BLUE_MASK GENMASK(24, 16)
> > > > +#define RKISP1_CIF_ISP_CAC_B_RED_MASK GENMASK(8, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_B_BLUE_MASK GENMASK(24, 16)
> > > > +#define RKISP1_CIF_ISP_CAC_C_RED_MASK GENMASK(8, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_C_BLUE_MASK GENMASK(24, 16)
> > >
> > > All these masks for coefficients 0, 1 and 2 are identical. Maybe
> > > #define RKISP1_CIF_ISP_CAC_RED_MASK GENMASK(8, 0)
> > > #define RKISP1_CIF_ISP_CAC_BLUE_MASK GENMASK(24, 16)
> > >
> > > is enough
> >
> > Adjusted.
> >
> > > > +#define RKISP1_CIF_ISP_CAC_X_NORM_NF_MASK GENMASK(4, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_X_NORM_NS_MASK GENMASK(19, 16)
> > > > +#define RKISP1_CIF_ISP_CAC_Y_NORM_NF_MASK GENMASK(4, 0)
> > > > +#define RKISP1_CIF_ISP_CAC_Y_NORM_NS_MASK GENMASK(19, 16)
> >
> > Did the same with these as well.
>
> Ah thanks!
>
> > > > +
> > > > /* =================================================================== */
> > > > /* CIF Registers */
> > > > /* =================================================================== */
> > > > @@ -1196,8 +1213,8 @@
> > > > #define RKISP1_CIF_ISP_CAC_A (RKISP1_CIF_ISP_CAC_BASE + 0x00000008)
> > > > #define RKISP1_CIF_ISP_CAC_B (RKISP1_CIF_ISP_CAC_BASE + 0x0000000c)
> > > > #define RKISP1_CIF_ISP_CAC_C (RKISP1_CIF_ISP_CAC_BASE + 0x00000010)
> > > > -#define RKISP1_CIF_ISP_X_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000014)
> > > > -#define RKISP1_CIF_ISP_Y_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000018)
> > > > +#define RKISP1_CIF_ISP_CAC_X_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000014)
> > > > +#define RKISP1_CIF_ISP_CAC_Y_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000018)
> > > >
> > > > #define RKISP1_CIF_ISP_EXP_BASE 0x00002600
> > > > #define RKISP1_CIF_ISP_EXP_CTRL (RKISP1_CIF_ISP_EXP_BASE + 0x00000000)
> > > > diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> > > > index b2d2a71f7baff..d8acccaddd0e9 100644
> > > > --- a/include/uapi/linux/rkisp1-config.h
> > > > +++ b/include/uapi/linux/rkisp1-config.h
> > > > @@ -967,6 +967,92 @@ struct rkisp1_cif_isp_wdr_config {
> > > > __u8 use_iref;
> > > > };
> > > >
> > > > +/*
> > > > + * enum rkisp1_cif_isp_cac_h_clip_mode - horizontal clipping mode
> > > > + *
> > > > + * @RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4PX: +/- 4 pixels
> > > > + * @RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4_5PX: +/- 4/5 pixels depending on bayer position
> > > > + */
> > > > +enum rkisp1_cif_isp_cac_h_clip_mode {
> > > > + RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4PX = 0,
> > > > + RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4_5PX = 1,
> > > > +};
> > > > +
> > > > +/**
> > > > + * enum rkisp1_cif_isp_cac_v_clip_mode - vertical clipping mode
> > > > + *
> > > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_2PX: +/- 2 pixels
> > > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3PX: +/- 3 pixels
> > > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3_4PX: +/- 3/4 pixels depending on bayer position
> > > > + */
> > > > +enum rkisp1_cif_isp_cac_v_clip_mode {
> > > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_2PX = 0,
> > > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3PX = 1,
> > > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3_4PX = 2,
> > > > +};
> > > > +
> > > > +/**
> > > > + * struct rkisp1_cif_isp_cac_config - chromatic aberration correction configuration
> > > > + *
> > > > + * The correction is carried out by shifting the red and blue pixels relative
> > > > + * to the green ones, depending on the distance from the optical center:
> > >
> > > Yes, the distance to the center is one parameter, but the shifting
> > > amount depends on other things. I would drop the last part of the
> > > sentence and move the description of the two below fields after the
> > > text
> >
> > That's true, but within a specific image, the only varying quantity
> > is the distance, so I think it is important to emphasize that.
> >
> > And I also quite like this structure of
> > - description of step
> > - parameters of step
> > - description of step
> > ...
> >
> > so I would love to keep it like this, if that's ok?
>
> Ok!
Ack.
> > > > + *
> > > > + * @h_count_start: horizontal coordinate of the optical center (13-bit unsigned integer; [1,8191])
> > > > + * @v_count_start: vertical coordinate of the optical center (13-bit unsigned integer; [1,8191])
> > >
> > > so these could go just before @x_nf
> > >
> > > > + *
> > > > + * For each pixel, the x/y distances from the optical center are calculated and
> > >
> > > I forgot: did we establish that the correction is applied to the
> > > euclidean distance or to x and y separately ?
> >
> > Given that there are two sets of "normalization" parameters, the assumption is that
> > at least the x/y distances are transformed separately. I see two reasonable choices
> > after that: (a) use the two distances separately, (b) use the radial distance. The
> > documentation says (b). However, testing with sensor test patterns suggests that
> > it is not the case (a horizontal/vertical boundary between appropriately colored
> > regions should have a curvature after the transformation with appropriate parameters).
>
> I now recall that you have been able to shift just one plan when using
> the sensor's test pattern
I agree about the normalization, but it sounds really weird that the
hardware would then shift separately in the X and Y directions. Below
you document the radial correction formula, which looks correct to me.
> > > > + * then transformed into the [0,255] range based on the following formula:
> > >
> > > s/transformed/normalized ?
> >
> > To be honest I vastly prefer "transform" / "map" over "normalize" here.
>
> You're right here, the below formula doesn't normalize the
> distance in an interval but just re-scale it
>
> > > > + *
> > > > + * (((d << 4) >> s) * f) >> 5
> > > > + *
> > > > + * where `d` is the distance, `s` and `f` are the normalization parameters:
> > >
> > > Can you use 'ns' and 'nf' to match the below ?
> >
> > Adjusted.
>
> Thanks!
>
> > > > + *
> > > > + * @x_nf: horizontal normalization scale parameter (5-bit unsigned integer; [0,31])
> > > > + * @x_ns: horizontal normalization shift parameter (4-bit unsigned integer; [0,15])
> > > > + *
> > > > + * @y_nf: vertical normalization scale parameter (5-bit unsigned integer; [0,31])
> > > > + * @y_ns: vertical normalization shift parameter (4-bit unsigned integer; [0,15])
> > > > + *
> > > > + * These parameters should be chosen based on the image resolution, the position
> > > > + * of the optical center, and the shape of pixels: so that no normalized distance
> > >
> > > s/pixels:/pixels/
> >
> > Replaced `:` with `,`.
> >
> > > > + * is larger than 255. If the pixels have square shape, the two sets of parameters
> > > > + * should be equal.
I wonder if we could have anisotropic lenses (from the point of view of
chromatic aberrations) with square pixels. We can deal with it later.
> > > > + *
> > > > + * The actual amount of correction is calculated with a third degree polynomial:
> > > > + *
> > > > + * c[0] * r + c[1] * r^2 + c[2] * r^3
> > > > + *
> > > > + * where `c` is the set of coefficients for the given color, and `r` is distance:
> > > > + *
> > > > + * @red: red coefficients (5.4 two's complement; [-16,15.9375])
> > > > + * @blue: blue coefficients (5.4 two's complement; [-16,15.9375])
> > > > + *
> > > > + * Finally, the amount is clipped as requested:
> > > > + *
> > > > + * @h_clip_mode: maximum horizontal shift (from enum rkisp1_cif_isp_cac_h_clip_mode)
> > > > + * @v_clip_mode: maximum vertical shift (from enum rkisp1_cif_isp_cac_v_clip_mode)
> > > > + *
> > > > + * A positive result will shift away from the optical center, while a negative
> > > > + * one will shift towards the optical center. In the latter case, the pixel
> > > > + * values at the edges are duplicated.
> > > > + */
> > > > +struct rkisp1_cif_isp_cac_config {
> > > > + __u8 h_clip_mode;
> > > > + __u8 v_clip_mode;
> > > > +
> > > > + __u16 h_count_start;
> > > > + __u16 v_count_start;
> > > > +
> > > > + __u16 red[3];
> > > > + __u16 blue[3];
> > > > +
> > > > + __u8 x_nf;
> > > > + __u8 x_ns;
> > > > +
> > > > + __u8 y_nf;
> > > > + __u8 y_ns;
> > > > +};
> > > > +
> > > > /*---------- PART2: Measurement Statistics ------------*/
> > > >
> > > > /**
> > > > @@ -1161,6 +1247,7 @@ enum rkisp1_ext_params_block_type {
> > > > RKISP1_EXT_PARAMS_BLOCK_TYPE_COMPAND_EXPAND,
> > > > RKISP1_EXT_PARAMS_BLOCK_TYPE_COMPAND_COMPRESS,
> > > > RKISP1_EXT_PARAMS_BLOCK_TYPE_WDR,
> > > > + RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC,
> > > > };
> > > >
> > > > /* For backward compatibility */
> > > > @@ -1507,6 +1594,22 @@ struct rkisp1_ext_params_wdr_config {
> > > > struct rkisp1_cif_isp_wdr_config config;
> > > > } __attribute__((aligned(8)));
> > > >
> > > > +/**
> > > > + * struct rkisp1_ext_params_cac_config - RkISP1 extensible params CAC config
> > > > + *
> > > > + * RkISP1 extensible parameters CAC block.
> > > > + * Identified by :c:type:`RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC`.
> > > > + *
> > > > + * @header: The RkISP1 extensible parameters header, see
> > > > + * :c:type:`rkisp1_ext_params_block_header`
> > > > + * @config: CAC configuration, see
> > > > + * :c:type:`rkisp1_cif_isp_cac_config`
> > > > + */
> > > > +struct rkisp1_ext_params_cac_config {
> > > > + struct rkisp1_ext_params_block_header header;
> > > > + struct rkisp1_cif_isp_cac_config config;
> > > > +} __attribute__((aligned(8)));
> > > > +
> > > > /*
> > > > * The rkisp1_ext_params_compand_curve_config structure is counted twice as it
> > > > * is used for both the COMPAND_EXPAND and COMPAND_COMPRESS block types.
> > > > @@ -1532,7 +1635,8 @@ struct rkisp1_ext_params_wdr_config {
> > > > sizeof(struct rkisp1_ext_params_compand_bls_config) +\
> > > > sizeof(struct rkisp1_ext_params_compand_curve_config) +\
> > > > sizeof(struct rkisp1_ext_params_compand_curve_config) +\
> > > > - sizeof(struct rkisp1_ext_params_wdr_config))
> > > > + sizeof(struct rkisp1_ext_params_wdr_config) +\
> > > > + sizeof(struct rkisp1_ext_params_cac_config))
> > >
> > > All minors, please add
> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
and
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > /**
> > > > * enum rksip1_ext_param_buffer_version - RkISP1 extensible parameters version
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] stmmac: cleanup dead dependencies on STMMAC_PLATFORM and STMMAC_ETH in Kconfig
From: Russell King (Oracle) @ 2026-03-31 13:25 UTC (permalink / raw)
To: Julian Braha
Cc: manabian, davem, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20260331125817.117091-1-julianbraha@gmail.com>
On Tue, Mar 31, 2026 at 01:58:17PM +0100, Julian Braha wrote:
> There are already 'if STMMAC_ETH' and 'STMMAC_PLATFORM'
> conditions wrapping these config options, making the
> 'depends on' statements duplicate dependencies (dead code).
>
> I propose leaving the outer 'if STMMAC_PLATFORM...endif' and
> 'if STMMAC_ETH...endif' conditions, and removing the
> individual 'depends on' statements.
>
> This dead code was found by kconfirm, a static analysis tool for Kconfig.
>
> Signed-off-by: Julian Braha <julianbraha@gmail.com>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v4 0/8] Bluetooth: Add MediaTek MT7927 (MT6639) support
From: Luiz Augusto von Dentz @ 2026-03-31 13:21 UTC (permalink / raw)
To: Javier Tia
Cc: Marcel Holtmann, Matthias Brugger, AngeloGioacchino Del Regno,
linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek,
Ryan Gilbert, Jose Tiburcio Ribeiro Netto, Llewellyn Curran,
Chapuis Dario, Evgeny Kapusta, Nitin Gurram, Thibaut FRANCOIS,
Ivan Lubnin
In-Reply-To: <20260330-mt7927-bt-support-v4-0-cecc025e7062@jetm.me>
Hi Javier,
On Mon, Mar 30, 2026 at 4:39 PM Javier Tia <floss@jetm.me> wrote:
>
> combo WiFi 7 + BT 5.4 module. The BT subsystem uses hardware variant
> 0x6639 and connects via USB.
>
> The MT7927 is shipping in motherboards and PCIe add-in cards from ASUS,
> Gigabyte, Lenovo, MSI, and TP-Link since mid-2024. Without these patches,
> users see "Unsupported hardware variant (00006639)" or the BT subsystem
> hangs during firmware download.
>
> The series consists of eight patches:
>
> [1/8] Bluetooth: btmtk: Add MT6639 (MT7927) Bluetooth support
> [2/8] Bluetooth: btmtk: fix ISO interface setup for single alt setting
> [3/8] Bluetooth: btusb: Add MT7927 ID for ASUS ROG Crosshair X870E Hero
> [4/8] Bluetooth: btusb: Add MT7927 ID for Lenovo Legion Pro 7 16ARX9
> [5/8] Bluetooth: btusb: Add MT7927 ID for Gigabyte Z790 AORUS MASTER X
> [6/8] Bluetooth: btusb: Add MT7927 ID for MSI X870E Ace Max
> [7/8] Bluetooth: btusb: Add MT7927 ID for TP-Link Archer TBE550E
> [8/8] Bluetooth: btusb: Add MT7927 ID for ASUS X870E / ProArt X870E-Creator
>
> Three driver changes are needed for MT6639 (patch 1):
>
> 1. CHIPID workaround: On some boards the BT USB MMIO register reads
> 0x0000 for dev_id. Force dev_id to 0x6639 only when the USB VID/PID
> matches a known MT6639 device, avoiding misdetection if a future
> chip also reads zero. This follows the WiFi-side pattern.
>
> 2. Firmware naming: MT6639 uses firmware version prefix "2_1" instead of
> "1_1" used by MT7925 and other variants. The firmware path is
> mediatek/mt7927/BT_RAM_CODE_MT6639_2_1_hdr.bin, using the mt7927
> directory to match the WiFi firmware convention. The filename will
> likely change to use MT7927 once MediaTek submits a dedicated
> Linux firmware binary.
>
> 3. Section filtering: The firmware binary contains 9 sections, but only
> sections with (dlmodecrctype & 0xff) == 0x01 are Bluetooth-related.
> Sending WiFi/other sections causes an irreversible BT subsystem hang.
>
> Patch 2 fixes the ISO interface setup for devices that expose only a
> single alternate setting (alt 0) on the ISO endpoint. Without this fix,
> btmtk_usb_claim_iso_intf() fails with EINVAL, causing ~20 second
> initialization delays on 13d3:3588 devices.
Ok, but on 8/8 it seem to suggest the problem still exists which
trigger the following review:
https://sashiko.dev/#/patchset/20260330-mt7927-bt-support-v4-0-cecc025e7062%40jetm.me
Perhaps we should ajust the commit description if the 19 seconds
doesn't really occurs when 2/8 is applied.
> WBS (Wideband Speech) was verified on MT7927 hardware. The controller
> reports LMP_TRANSPARENT, LMP_ERR_DATA_REPORTING, and mSBC codec support.
> A btmon capture confirms eSCO links establish with Transparent air mode
> (mSBC) and 60-byte frames. BTUSB_WIDEBAND_SPEECH is correct for this
> controller family.
>
> Tested on:
> - ASUS ROG Crosshair X870E Hero (USB 0489:e13a)
> - ASUS ROG STRIX X870E-E (USB 13d3:3588)
> - ASUS ROG STRIX B850-E GAMING WIFI (USB 0489:e13a)
> - Gigabyte Z790 AORUS MASTER X (USB 0489:e10f)
> - Lenovo Legion Pro 7 16ARX9 (USB 0489:e0fa)
>
> Changes in v4:
> - Pass dev_id as parameter to btmtk_setup_firmware_79xx instead of
> using hci_get_priv, which is not available in btmtksdio
> (suggested by Luiz Augusto von Dentz)
> - Skip post-reset CHIPID validation for 0x6639 in
> btmtk_usb_subsys_reset since the register always returns 0x0000
> on affected boards
>
> Link to v3: https://lore.kernel.org/linux-bluetooth/20260326-mt7927-bt-support-v3-0-fa7ebd424323@jetm.me/
>
> Signed-off-by: Javier Tia <floss@jetm.me>
> ---
> Javier Tia (8):
> Bluetooth: btmtk: Add MT6639 (MT7927) Bluetooth support
> Bluetooth: btmtk: fix ISO interface setup for single alt setting
> Bluetooth: btusb: Add MT7927 ID for ASUS ROG Crosshair X870E Hero
> Bluetooth: btusb: Add MT7927 ID for Lenovo Legion Pro 7 16ARX9
> Bluetooth: btusb: Add MT7927 ID for Gigabyte Z790 AORUS MASTER X
> Bluetooth: btusb: Add MT7927 ID for MSI X870E Ace Max
> Bluetooth: btusb: Add MT7927 ID for TP-Link Archer TBE550E
> Bluetooth: btusb: Add MT7927 ID for ASUS X870E / ProArt X870E-Creator
>
> drivers/bluetooth/btmtk.c | 63 ++++++++++++++++++++++++++++++++++++++-----
> drivers/bluetooth/btmtk.h | 7 +++--
> drivers/bluetooth/btmtksdio.c | 2 +-
> drivers/bluetooth/btusb.c | 12 +++++++++
> 4 files changed, 75 insertions(+), 9 deletions(-)
> ---
> base-commit: 7f9446b23ac77f46d356b354ea8da49113e8000d
> change-id: 20260305-mt7927-bt-support-6589a50c961f
>
> Best regards,
> --
> Javier Tia <floss@jetm.me>
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH v2] raid6: arm64: add SVE optimized implementation for syndrome generation
From: Demian Shulhan @ 2026-03-31 13:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Ard Biesheuvel, Mark Rutland, Song Liu, Yu Kuai, Will Deacon,
Catalin Marinas, Mark Brown, linux-arm-kernel, robin.murphy,
Li Nan, linux-raid, linux-kernel
In-Reply-To: <20260331063659.GA2061@lst.de>
Hi all,
Ard, your questions regarding real-world I/O bottlenecks and SVE power
efficiency versus raw throughput are entirely valid. I agree that
introducing SVE support requires solid real-world data to justify the
added complexity.
Due to my current workload, I won't be able to run the necessary
hardware tests and prepare the benchmark code immediately. I will get
back to the list in about 1 week with the requested source code,
unmangled test results, and further analysis.
Thanks!
вт, 31 бер. 2026 р. о 09:37 Christoph Hellwig <hch@lst.de> пише:
>
> On Mon, Mar 30, 2026 at 06:39:49PM +0200, Ard Biesheuvel wrote:
> > I think the results are impressive, but I'd like to better understand
> > its implications on a real-world scenario. Is this code only a
> > bottleneck when rebuilding an array?
>
> The syndrome generation is run every time you write data to a RAID6
> array, and if you do partial stripe writes it (or rather the XOR
> variant) is run twice. So this is the most performance critical
> path for writing to RAID6.
>
> Rebuild usually runs totally different code, but can end up here as well
> when both parity disks are lost.
>
> > > Furthermore, as Christoph suggested, I tested scalability on wider
> > > arrays since the default kernel benchmark is hardcoded to 8 disks,
> > > which doesn't give the unrolled SVE loop enough data to shine. On a
> > > 16-disk array, svex4 hits 15.1 GB/s compared to 8.0 GB/s for neonx4.
> > > On a 24-disk array, while neonx4 chokes and drops to 7.8 GB/s, svex4
> > > maintains a stable 15.0 GB/s — effectively doubling the throughput.
> >
> > Does this mean the kernel benchmark is no longer fit for purpose? If
> > it cannot distinguish between implementations that differ in performance
> > by a factor of 2, I don't think we can rely on it to pick the optimal one.
>
> It is not good, and we should either fix it or run more than one.
> The current setup is not really representative of real-life array.
> It also leads to wrong selections on x86, but only at the which unroll
> level to pick level, and only for minor differences so far. I plan
> to add this to the next version of the raid6 lib patches.
>
^ permalink raw reply
* Re: [PATCH v2] i2c: rk3x: add support for SCL OE debounce and slave hold recovery
From: Anand Moon @ 2026-03-31 13:18 UTC (permalink / raw)
To: Andi Shyti
Cc: Heiko Stuebner, moderated list:ARM/Rockchip SoC support,
open list:ARM/Rockchip SoC support,
open list:I2C SUBSYSTEM HOST DRIVERS, open list, David Wu
In-Reply-To: <acaO5ouMmGA8-tqJ@zenone.zhora.eu>
Hi Andi,
Thanks for your review comments.
On Fri, 27 Mar 2026 at 19:43, Andi Shyti <andi.shyti@kernel.org> wrote:
>
> Hi Anand,
>
> ...
>
> > @@ -1125,6 +1141,17 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
> > }
> > }
> >
> > + /*
> > + * If a timeout occurred and the slave is holding SCL,
> > + * re-apply the timings/dividers to attempt recovery.
> > + */
> > + if (ret == -ETIMEDOUT && i2c->soc_data->has_scl_oe_debounce) {
> > + if (ipd & REG_INT_SLV_HDSCL) {
> > + dev_err(i2c->dev, "SCL hold by slave detected, resetting timings.\n");
> > + rk3x_i2c_adapt_div(i2c, clk_get_rate(i2c->clk));
>
> argh! this nests i2c->lock. rk3x_i2c_xfer_common() already holds
> it when calling rk3x_i2c_adapt_div().
I agree; the current implementation can lead to a deadlock.
To resolve this, I'll split the function into locked and unlocked variants.
Note that this deadlock only occurs if REG_INT_SLV_HDSCL is active in the
interrupt enable mask, as shown below:
static void rk3x_i2c_start(struct rk3x_i2c *i2c)
{
u32 val = i2c_readl(i2c, REG_CON) & REG_CON_TUNING_MASK;
+ u32 ien = REG_INT_START;
- i2c_writel(i2c, REG_INT_START, REG_IEN);
+ if(i2c->soc_data->has_scl_oe_debounce)
+ ien != REG_INT_SLV_HDSCL;
+
+ i2c_writel(i2c, ien, REG_IEN);
Thanks
-Anand
^ permalink raw reply
* Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver
From: Lee Jones @ 2026-03-31 13:08 UTC (permalink / raw)
To: Thomas Perrot (Schneider Electric)
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
Bartosz Golaszewski, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
Jérémie Dautheribes, Wim Van Sebroeck, Guenter Roeck,
devicetree, linux-kernel, linux-gpio, imx, linux-arm-kernel,
linux-watchdog, Thomas Petazzoni, Miquel Raynal
In-Reply-To: <20260324-dev-b4-aaeon-mcu-driver-v4-3-afb011df4794@bootlin.com>
On Tue, 24 Mar 2026, Thomas Perrot (Schneider Electric) wrote:
> Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P
> embedded controller. This driver provides the core I2C communication
> interface and registers child devices (GPIO and watchdog controllers).
>
> The driver implements a custom regmap bus over I2C to match the MCU's
> fixed 3-byte command format [opcode, arg, value]. Register addresses
> are encoded as 16-bit values (opcode << 8 | arg) using the
> AAEON_MCU_REG() macro defined in the shared header. The regmap
> instance is shared with child drivers via dev_get_regmap(). Concurrent
> I2C accesses from child drivers are serialized by regmap's built-in
> locking.
>
> Co-developed-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Thomas Perrot (Schneider Electric) <thomas.perrot@bootlin.com>
> ---
> MAINTAINERS | 2 +
> drivers/mfd/Kconfig | 10 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/aaeon-mcu.c | 155 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/aaeon-mcu.h | 20 ++++++
> 5 files changed, 188 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221f6c8e8a3652cd77 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -191,6 +191,8 @@ M: Thomas Perrot <thomas.perrot@bootlin.com>
> R: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> S: Maintained
> F: Documentation/devicetree/bindings/mfd/aaeon,srg-imx8p-mcu.yaml
> +F: drivers/mfd/aaeon-mcu.c
> +F: include/linux/mfd/aaeon-mcu.h
>
> AAEON UPBOARD FPGA MFD DRIVER
> M: Thomas Richard <thomas.richard@bootlin.com>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe7b1c91b72253c05 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1574,6 +1574,16 @@ config AB8500_CORE
> the irq_chip parts for handling the Mixed Signal chip events.
> This chip embeds various other multimedia functionalities as well.
>
> +config MFD_AAEON_MCU
> + tristate "Aaeon SRG-IMX8P MCU Driver"
> + depends on I2C || COMPILE_TEST
> + select MFD_CORE
> + help
> + Select this option to enable support for the Aaeon SRG-IMX8P
> + onboard microcontroller (MCU). This driver provides the core
> + functionality to communicate with the MCU over I2C. The MCU
> + provides GPIO and watchdog functionality.
> +
> config MFD_DB8500_PRCMU
> bool "ST-Ericsson DB8500 Power Reset Control Management Unit"
> depends on UX500_SOC_DB8500
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1eef12528a74baf8f5 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X) += 88pm860x.o
> obj-$(CONFIG_MFD_88PM800) += 88pm800.o 88pm80x.o
> obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
> obj-$(CONFIG_MFD_88PM886_PMIC) += 88pm886.o
> +obj-$(CONFIG_MFD_AAEON_MCU) += aaeon-mcu.o
> obj-$(CONFIG_MFD_ACT8945A) += act8945a.o
> obj-$(CONFIG_MFD_SM501) += sm501.o
> obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
> diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b4d4d89b1f8c563e
> --- /dev/null
> +++ b/drivers/mfd/aaeon-mcu.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Aaeon MCU driver
> + *
> + * Copyright (C) 2025 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> + */
Consider updating the Copyright date - we're pretty deep into 2026 at this point.
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +static const struct mfd_cell aaeon_mcu_devs[] = {
> + {
> + .name = "aaeon-mcu-wdt",
> + },
> + {
> + .name = "aaeon-mcu-gpio",
> + },
> +};
MFD_CELL_BASIC()
> +/*
> + * Custom regmap bus for the Aaeon MCU I2C protocol.
> + *
> + * The MCU uses a fixed 3-byte command format [opcode, arg, value] followed
> + * by a 1-byte response. It requires a STOP condition between the command
> + * write and the response read, so two separate i2c_transfer() calls are
> + * issued. The regmap lock serialises concurrent accesses from the GPIO
> + * and watchdog child drivers.
> + *
> + * Register addresses are encoded as a 16-bit big-endian value where the
> + * high byte is the opcode and the low byte is the argument, matching the
> + * wire layout produced by regmap for reg_bits=16.
> + */
> +
> +static int aaeon_mcu_regmap_write(void *context, const void *data, size_t count)
> +{
> + struct i2c_client *client = context;
> + /* data = [opcode, arg, value] as formatted by regmap */
> + struct i2c_msg write_msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .buf = (u8 *)data,
> + .len = count,
> + };
> + u8 rsp;
> + /* The MCU always sends a response byte after each command; discard it. */
> + struct i2c_msg rsp_msg = {
Assuming 'rsp' means response, let's just write that out in full.
Readability wins over brevity every time.
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .buf = &rsp,
> + .len = 1,
> + };
> + int ret;
Since some I2C host controllers might use DMA, should we ensure that the
'rsp' buffer is allocated in DMA-safe memory rather than on the stack to
prevent potential cache-line corruption?
Also allocation of structs during in declaration statements is rough!
And adding that u8 in the middle is just rubbing it in.
> + ret = i2c_transfer(client->adapter, &write_msg, 1);
> + if (ret < 0)
> + return ret;
> + if (ret != 1)
> + return -EIO;
> +
> + ret = i2c_transfer(client->adapter, &rsp_msg, 1);
> + if (ret < 0)
> + return ret;
> + if (ret != 1)
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static int aaeon_mcu_regmap_read(void *context, const void *reg_buf,
> + size_t reg_size, void *val_buf, size_t val_size)
> +{
> + struct i2c_client *client = context;
> + /*
> + * reg_buf holds the 2-byte big-endian register address [opcode, arg].
> + * Append a trailing 0x00 to form the full 3-byte MCU command.
> + */
> + u8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00 };
> + struct i2c_msg write_msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .buf = cmd,
> + .len = sizeof(cmd),
> + };
> + struct i2c_msg read_msg = {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .buf = val_buf,
> + .len = val_size,
> + };
> + int ret;
> +
> + ret = i2c_transfer(client->adapter, &write_msg, 1);
> + if (ret < 0)
> + return ret;
> + if (ret != 1)
> + return -EIO;
> +
> + ret = i2c_transfer(client->adapter, &read_msg, 1);
> + if (ret < 0)
> + return ret;
> + if (ret != 1)
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static const struct regmap_bus aaeon_mcu_regmap_bus = {
> + .write = aaeon_mcu_regmap_write,
> + .read = aaeon_mcu_regmap_read,
> +};
> +
> +static const struct regmap_config aaeon_mcu_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .cache_type = REGCACHE_NONE,
Are you sure? Why none?
> +};
> +
> +static int aaeon_mcu_probe(struct i2c_client *client)
> +{
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init(&client->dev, &aaeon_mcu_regmap_bus,
> + client, &aaeon_mcu_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
dev_err_probe()
> +
> + return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> + aaeon_mcu_devs, ARRAY_SIZE(aaeon_mcu_devs),
> + NULL, 0, NULL);
Why PLATFORM_DEVID_NONE over AUTO here?
> +}
> +
> +static const struct of_device_id aaeon_mcu_of_match[] = {
> + { .compatible = "aaeon,srg-imx8p-mcu" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);
> +
> +static struct i2c_driver aaeon_mcu_driver = {
> + .driver = {
> + .name = "aaeon_mcu",
> + .of_match_table = aaeon_mcu_of_match,
> + },
> + .probe = aaeon_mcu_probe,
> +};
> +module_i2c_driver(aaeon_mcu_driver);
> +
> +MODULE_DESCRIPTION("Aaeon MCU Driver");
> +MODULE_AUTHOR("Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/aaeon-mcu.h b/include/linux/mfd/aaeon-mcu.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..861003f6dfd20424c3785008bd2cf89aaa1715b9
> --- /dev/null
> +++ b/include/linux/mfd/aaeon-mcu.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Aaeon MCU driver definitions
> + *
> + * Copyright (C) 2025 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> + */
As above.
> +
> +#ifndef __LINUX_MFD_AAEON_MCU_H
> +#define __LINUX_MFD_AAEON_MCU_H
> +
> +/*
> + * MCU register address: the high byte is the command opcode, the low
> + * byte is the argument. This matches the 3-byte wire format
> + * [opcode, arg, value] used by the MCU I2C protocol.
> + */
> +#define AAEON_MCU_REG(op, arg) (((op) << 8) | (arg))
Where else is this used?
> +#endif /* __LINUX_MFD_AAEON_MCU_H */
>
> --
> 2.53.0
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* [PATCH] stmmac: cleanup dead dependencies on STMMAC_PLATFORM and STMMAC_ETH in Kconfig
From: Julian Braha @ 2026-03-31 12:58 UTC (permalink / raw)
To: manabian, davem, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32
Cc: netdev, linux-arm-kernel, linux-kernel, Julian Braha
There are already 'if STMMAC_ETH' and 'STMMAC_PLATFORM'
conditions wrapping these config options, making the
'depends on' statements duplicate dependencies (dead code).
I propose leaving the outer 'if STMMAC_PLATFORM...endif' and
'if STMMAC_ETH...endif' conditions, and removing the
individual 'depends on' statements.
This dead code was found by kconfirm, a static analysis tool for Kconfig.
Signed-off-by: Julian Braha <julianbraha@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index c2cb530fd0a2..ba5473b8dae0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -20,7 +20,6 @@ if STMMAC_ETH
config STMMAC_SELFTESTS
bool "Support for STMMAC Selftests"
depends on INET
- depends on STMMAC_ETH
default n
help
This adds support for STMMAC Selftests using ethtool. Enable this
@@ -29,7 +28,6 @@ config STMMAC_SELFTESTS
config STMMAC_PLATFORM
tristate "STMMAC Platform bus support"
- depends on STMMAC_ETH
select MFD_SYSCON
default y
help
@@ -53,7 +51,6 @@ config DWMAC_DWC_QOS_ETH
config DWMAC_GENERIC
tristate "Generic driver for DWMAC"
- default STMMAC_PLATFORM
help
Generic DWMAC driver for platforms that don't require any
platform specific code to function or is using platform
@@ -336,7 +333,6 @@ config DWMAC_IMX8
config DWMAC_INTEL_PLAT
tristate "Intel dwmac support"
depends on OF && COMMON_CLK
- depends on STMMAC_ETH
help
Support for ethernet controllers on Intel SoCs
@@ -371,7 +367,7 @@ config DWMAC_VISCONTI
help
Support for ethernet controller on Visconti SoCs.
-endif
+endif # STMMAC_PLATFORM
config STMMAC_LIBPCI
tristate
@@ -381,7 +377,7 @@ config STMMAC_LIBPCI
config DWMAC_INTEL
tristate "Intel GMAC support"
default X86
- depends on X86 && STMMAC_ETH && PCI
+ depends on X86 && PCI
depends on COMMON_CLK
depends on ACPI
help
@@ -420,4 +416,4 @@ config STMMAC_PCI
If you have a controller with this interface, say Y or M here.
If unsure, say N.
-endif
+endif # STMMAC_ETH
--
2.51.2
^ permalink raw reply related
* Re: [PATCH v2 2/2] ARM: dts: nxp: imx51-ts4800: Rename wdt node to watchdog
From: Daniel Baluta @ 2026-03-31 13:00 UTC (permalink / raw)
To: Eduard Bostina, daniel.baluta, simona.toaca, d-gole, m-chawdhry,
Wim Van Sebroeck, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Mark Brown, linux-watchdog, devicetree,
linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20260323175948.302441-3-egbostina@gmail.com>
On 3/23/26 19:59, Eduard Bostina wrote:
> The Technologic Systems TS-4800 watchdog node was previously named 'wdt',
> which violates the core watchdog.yaml schema expecting generic node names.
>
> Rename the node to 'watchdog' to fix the following dtbs_check warning:
> 'wdt' does not match '^(pmic|timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$'
>
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
^ permalink raw reply
* Re: [PATCH v7 1/5] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
From: Mark Brown @ 2026-03-31 12:49 UTC (permalink / raw)
To: Maíra Canal
Cc: Michael Turquette, Stephen Boyd, Nicolas Saenz Julienne,
Florian Fainelli, Stefan Wahren, Maxime Ripard, Melissa Wen,
Iago Toral Quiroga, Chema Casanova, Dave Stevenson, Philipp Zabel,
linux-clk, dri-devel, linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev
In-Reply-To: <20260312-v3d-power-management-v7-1-9f006a1d4c55@igalia.com>
[-- Attachment #1: Type: text/plain, Size: 15753 bytes --]
On Thu, Mar 12, 2026 at 06:34:23PM -0300, Maíra Canal wrote:
> On current firmware versions, RPI_FIRMWARE_SET_CLOCK_STATE doesn't
> actually power off the clock. To achieve meaningful power savings, the
> clock rate must be set to the minimum before disabling. This might be
> fixed in future firmware releases.
> Rather than pushing rate management to clock consumers, handle it
> directly in the clock framework's prepare/unprepare callbacks. In
> unprepare, set the rate to the minimum before disabling the clock.
> In prepare, for clocks marked with `maximize` (currently v3d),
> restore the rate to the maximum after enabling.
I'm seeing boot regressions in -next with NFS root on Raspberry Pi 3B+
which bisect to this commit. We get a likely unrelated oops from the
firmware interface and the boot grinds to a halt some time later since
the ethernet never comes up:
[ 21.898686] Firmware transaction 0x00030066 timeout
[ 21.898769] WARNING: drivers/firmware/raspberrypi.c:128 at rpi_firmware_property_list+0x200/0x280, CPU#2: (udev-worker)/115
...
[ 22.067074] Call trace:
[ 22.069538] rpi_firmware_property_list+0x200/0x280 (P)
[ 22.074824] rpi_firmware_property+0x70/0xb8
[ 22.079140] vc4_drm_bind+0x12c/0x354 [vc4]
[ 22.083368] try_to_bring_up_aggregate_device+0x16c/0x1e0
[ 22.088831] component_master_add_with_match+0xb0/0xec
[ 22.094027] vc4_platform_drm_probe+0xc0/0xfc [vc4]
[ 22.098961] platform_probe+0x5c/0xa4
[0;1;31mTimed out while waiting for udev queue to empty.[0m
Begin: Loading essential drivers ... done.
Begin: Running /scripts/init-premount ... done.
Begin: Mounting root file system ... Begin: Running /scripts/nfs-top ... done.
Begin: Running /scripts/nfs-premount ... Waiting up to 60 secs for any ethernet to become available
Full log:
https://lava.sirena.org.uk/scheduler/job/2617478#L1124
bisect log:
# bad: [cf7c3c02fdd0dfccf4d6611714273dcb538af2cb] Add linux-next specific files for 20260330
# good: [a010730e610019b6d010ec43ce737cb59a37809d] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [3398da5432899c09712f30c92a154f3bf760a3e7] Merge branch 'clk-renesas' into clk-next
# good: [777bc3284f0360c66ac75a44f35fc96053536ca4] Merge branch 'clk-cleanup' into clk-next
# good: [f520a492e07bc6718e26cfb7543ab4cadd8bb0e2] clk: xgene: Fix mapping leak in xgene_pllclk_init()
# good: [aeb078cebc40d421f61a8f07b0e7919aeb44d751] arm64: dts: broadcom: bcm2712-d-rpi-5-b: add fixes for pinctrl/pinctrl_aon
# good: [9be71d462c33b1a00acfa4ab8f0f5332ed592817] firmware: cs_dsp: Simplify suppressing log messages during KUnit testing
# good: [7b3f8db159f710d432c4edc024fcefa9e62e8b4b] ASoC: fsl_xcvr: add bitcount and timestamp controls
# good: [8fc5c7895185d1119ae76b509892a1d14e0bd483] ASoC: wm_adsp: Combine some similar code in firmware file search
# good: [981b080a79724738882b0af1c5bb7ade30d94f24] spi: fsl-qspi: Use reinit_completion() for repeated operations
# good: [ed0313223ce6514dbd39c049e25f702980d7e3cc] ASoC: codecs: wcd9335: Remove potential undefined behavior in wcd9335_slimbus_irq()
# good: [97af961568c8682c44506c9ad4b26c8a5455ec1d] ASoC: cs35l56: Put OTP register defines in correct address order
# good: [0a208adefecb287d22321054470d4619cb303839] ASoC: cs42l43: Add support for the B variant
# good: [a8075ada4a341ce58ebf8bef0188cefe6c2f6487] ASoC: ti: davinci-mcasp: improve aux_div selection for mid-range dividers
# good: [82169065ffb07577075a5088b313d78673ded331] memory: tegra: Add MC error logging support for Tegra264
# good: [aa3d0c93a333182e887426366a4f3e5f06ee0d83] regulator: max20411: show failure on register
# good: [ef0b4783afc211a4b120e72b5a57f3d0340a9981] ASoC: cs35l56: KUnit tests for reading speaker ID from host GPIOs
# good: [e7662bced2e98ffa2c572126677deb9cf55d43b3] regcache: Move HW readback after cache initialisation
# good: [b1ef855c62601ed4de2c4b0ff75a075877e3dac8] regmap: Simplify devres handling
# good: [96f06d055ca03d1dfb5830fd07ff6eadbd66264c] spi: dt-bindings: mpfs-spi: remove clock-names
# good: [2adac914c72b6cb5aba2612f49050c82aecd498e] ASoC: cs35l56-test: Add test cases without onchip pulls defined
# good: [f48e7a246a567e3764112e2463274c479d95cd96] ASoC: soc-core: Use guard()/scoped_guard() for mutex lock
# good: [9891b52ba12e9d5fed5901b6b5f6e0cdcd424390] regcache: Factor out regcache_hw_exit() helper
# good: [e84141846decb77d2826e553318a608b256804e5] regulator: pf9453: Allow shared IRQ
# good: [9ab637ac5d3826606947f4e861107da958eda324] regcache: Amend printf() specifiers when printing registers
# good: [34b4fc44e4f904fbb81335d53163ffdcb0180000] ASoC: soc_sdw_utils: remove index from sdca codec name
# good: [1696fad8b259a2d46e51cd6e17e4bcdbe02279fa] ASoC: sti: use managed regmap_field allocations
# good: [d3b693a13b39bce16e284e1c737874966b3a96de] spi: spi-mem: clean up kernel-doc in spi-mem.h
# good: [06dba254de95b16e7793224d29daa5195de2e581] ASoC: dt-bindings: nvidia,tegra-audio-max9808x: document additional board pins
# good: [17c6bf433742e0c1ff5ce175145877c0194e4a7a] ASoC: cs35l45: Hibernate wm_adsp on runtime suspend
# good: [da37bfe76b5b4ccc01ed8132215098e20d78e5f3] ASoC: cs42xx8: add error checks for constraints in TDM mode
# good: [501efdcb3b3ab099fc0ce2f6e668b1c4095dd476] ASoC: SDCA: Pull the Q7.8 volume helpers out of soc-ops
# good: [2974aa42e6696a1d95b727d677dc01a71af5b998] ASoC: remove snd_soc_pcm_subclass
# good: [d90c0f78379454d51a428e312ac6db573060185c] regulator: cpcap-regulator: add support for Mot regulators
# good: [5c74a008ffc62fc57a041602b4517519c8bf9436] firmware: cs_dsp: Mark KUnit test suites KUNIT_SPEED_SLOW
# good: [260c3fff1fefc570d8f23e87953e181d7d248861] ASoC: cs-amp-lib-test: Stop including platform_device.h
# good: [7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4] spi: tegra210-quad: Add runtime autosuspend support
# good: [37983fad7f3ef296fa0504c8e945987459dc5487] regmap: define cleanup helper for regmap_field
# good: [ada32396f90951e12465224c04742607ca56a982] ASoC: SDCA: Add CS47L47 to class driver
# good: [e02902dd493bf9c9b05353c761737ac514ad7a5c] spi: add devm_spi_new_ancillary_device()
# good: [507a071d9868cb60e4e76f8a06fc8eb014f59ae4] spi: pxa2xx: use min() instead of min_t()
# good: [fed6e5084894373d76270cad4a32eb6479ad8247] spi: atcspi200: Remove redundant assignment to .owner
# good: [5ebc20921b7fff9feb44de465448e17a382c9965] ASoC: tas2552: Allow audio enable GPIO to sleep
# good: [171b3663f33e1efdc97f5112f49be10b47b20fa8] ASoC: codecs: aw88261: Add firmware-name support
# good: [c2bcf62ca75c541ec4297e6ff02a68ddc2e02029] regcache: Split regcache_count_cacheable_registers() helper
# good: [0556bb42a84ee391a2145ddba86756f9747bc27f] regulator: pf0900: Make regu_irqs variable static const
# good: [d075cef4af6327a5de4bee7bf77591e3201e54f4] ASoC: simple-card-utils: add sysclk ordering support
# good: [78dfbd4ad0be9f51de7b9a19388809254aeccd26] ASoC: Add quirk for Lecoo Bellator N176
# good: [bf122191473e26a8f195308b1ba924c98424c8e1] ASoC: rt5677-spi: Add SPI device ID matching table
# good: [fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7] regulator: spacemit-p1: Update supply names
git bisect start 'cf7c3c02fdd0dfccf4d6611714273dcb538af2cb' 'a010730e610019b6d010ec43ce737cb59a37809d' '3398da5432899c09712f30c92a154f3bf760a3e7' '777bc3284f0360c66ac75a44f35fc96053536ca4' 'f520a492e07bc6718e26cfb7543ab4cadd8bb0e2' 'aeb078cebc40d421f61a8f07b0e7919aeb44d751' '9be71d462c33b1a00acfa4ab8f0f5332ed592817' '7b3f8db159f710d432c4edc024fcefa9e62e8b4b' '8fc5c7895185d1119ae76b509892a1d14e0bd483' '981b080a79724738882b0af1c5bb7ade30d94f24' 'ed0313223ce6514dbd39c049e25f702980d7e3cc' '97af961568c8682c44506c9ad4b26c8a5455ec1d' '0a208adefecb287d22321054470d4619cb303839' 'a8075ada4a341ce58ebf8bef0188cefe6c2f6487' '82169065ffb07577075a5088b313d78673ded331' 'aa3d0c93a333182e887426366a4f3e5f06ee0d83' 'ef0b4783afc211a4b120e72b5a57f3d0340a9981' 'e7662bced2e98ffa2c572126677deb9cf55d43b3' 'b1ef855c62601ed4de2c4b0ff75a075877e3dac8' '96f06d055ca03d1dfb5830fd07ff6eadbd66264c' '2adac914c72b6cb5aba2612f49050c82aecd498e' 'f48e7a246a567e3764112e2463274c479d95cd96' '9891b52ba12e9d5fed5901b6b5f6e0cdcd424390' 'e84141846decb77d2826e553318a608b256804e5' '9ab637ac5d3826606947f4e861107da958eda324' '34b4fc44e4f904fbb81335d53163ffdcb0180000' '1696fad8b259a2d46e51cd6e17e4bcdbe02279fa' 'd3b693a13b39bce16e284e1c737874966b3a96de' '06dba254de95b16e7793224d29daa5195de2e581' '17c6bf433742e0c1ff5ce175145877c0194e4a7a' 'da37bfe76b5b4ccc01ed8132215098e20d78e5f3' '501efdcb3b3ab099fc0ce2f6e668b1c4095dd476' '2974aa42e6696a1d95b727d677dc01a71af5b998' 'd90c0f78379454d51a428e312ac6db573060185c' '5c74a008ffc62fc57a041602b4517519c8bf9436' '260c3fff1fefc570d8f23e87953e181d7d248861' '7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4' '37983fad7f3ef296fa0504c8e945987459dc5487' 'ada32396f90951e12465224c04742607ca56a982' 'e02902dd493bf9c9b05353c761737ac514ad7a5c' '507a071d9868cb60e4e76f8a06fc8eb014f59ae4' 'fed6e5084894373d76270cad4a32eb6479ad8247' '5ebc20921b7fff9feb44de465448e17a382c9965' '171b3663f33e1efdc97f5112f49be10b47b20fa8' 'c2bcf62ca75c541ec4297e6ff02a68ddc2e02029' '0556bb42a84ee391a2145ddba86756f9747bc27f' 'd075cef4af6327a5de4bee7bf77591e3201e54f4' '78dfbd4ad0be9f51de7b9a19388809254aeccd26' 'bf122191473e26a8f195308b1ba924c98424c8e1' 'fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7'
# test job: [3398da5432899c09712f30c92a154f3bf760a3e7] https://lava.sirena.org.uk/scheduler/job/2608225
# test job: [777bc3284f0360c66ac75a44f35fc96053536ca4] https://lava.sirena.org.uk/scheduler/job/2608418
# test job: [f520a492e07bc6718e26cfb7543ab4cadd8bb0e2] https://lava.sirena.org.uk/scheduler/job/2608110
# test job: [aeb078cebc40d421f61a8f07b0e7919aeb44d751] https://lava.sirena.org.uk/scheduler/job/2579214
# test job: [9be71d462c33b1a00acfa4ab8f0f5332ed592817] https://lava.sirena.org.uk/scheduler/job/2548706
# test job: [7b3f8db159f710d432c4edc024fcefa9e62e8b4b] https://lava.sirena.org.uk/scheduler/job/2548248
# test job: [8fc5c7895185d1119ae76b509892a1d14e0bd483] https://lava.sirena.org.uk/scheduler/job/2548893
# test job: [981b080a79724738882b0af1c5bb7ade30d94f24] https://lava.sirena.org.uk/scheduler/job/2545092
# test job: [ed0313223ce6514dbd39c049e25f702980d7e3cc] https://lava.sirena.org.uk/scheduler/job/2544887
# test job: [97af961568c8682c44506c9ad4b26c8a5455ec1d] https://lava.sirena.org.uk/scheduler/job/2543911
# test job: [0a208adefecb287d22321054470d4619cb303839] https://lava.sirena.org.uk/scheduler/job/2542875
# test job: [a8075ada4a341ce58ebf8bef0188cefe6c2f6487] https://lava.sirena.org.uk/scheduler/job/2540933
# test job: [82169065ffb07577075a5088b313d78673ded331] https://lava.sirena.org.uk/scheduler/job/2582197
# test job: [aa3d0c93a333182e887426366a4f3e5f06ee0d83] https://lava.sirena.org.uk/scheduler/job/2531504
# test job: [ef0b4783afc211a4b120e72b5a57f3d0340a9981] https://lava.sirena.org.uk/scheduler/job/2530821
# test job: [e7662bced2e98ffa2c572126677deb9cf55d43b3] https://lava.sirena.org.uk/scheduler/job/2530804
# test job: [b1ef855c62601ed4de2c4b0ff75a075877e3dac8] https://lava.sirena.org.uk/scheduler/job/2531890
# test job: [96f06d055ca03d1dfb5830fd07ff6eadbd66264c] https://lava.sirena.org.uk/scheduler/job/2523404
# test job: [2adac914c72b6cb5aba2612f49050c82aecd498e] https://lava.sirena.org.uk/scheduler/job/2523920
# test job: [f48e7a246a567e3764112e2463274c479d95cd96] https://lava.sirena.org.uk/scheduler/job/2522185
# test job: [9891b52ba12e9d5fed5901b6b5f6e0cdcd424390] https://lava.sirena.org.uk/scheduler/job/2522240
# test job: [e84141846decb77d2826e553318a608b256804e5] https://lava.sirena.org.uk/scheduler/job/2516913
# test job: [9ab637ac5d3826606947f4e861107da958eda324] https://lava.sirena.org.uk/scheduler/job/2516426
# test job: [34b4fc44e4f904fbb81335d53163ffdcb0180000] https://lava.sirena.org.uk/scheduler/job/2513607
# test job: [1696fad8b259a2d46e51cd6e17e4bcdbe02279fa] https://lava.sirena.org.uk/scheduler/job/2514118
# test job: [d3b693a13b39bce16e284e1c737874966b3a96de] https://lava.sirena.org.uk/scheduler/job/2511841
# test job: [06dba254de95b16e7793224d29daa5195de2e581] https://lava.sirena.org.uk/scheduler/job/2513766
# test job: [17c6bf433742e0c1ff5ce175145877c0194e4a7a] https://lava.sirena.org.uk/scheduler/job/2513835
# test job: [da37bfe76b5b4ccc01ed8132215098e20d78e5f3] https://lava.sirena.org.uk/scheduler/job/2511899
# test job: [501efdcb3b3ab099fc0ce2f6e668b1c4095dd476] https://lava.sirena.org.uk/scheduler/job/2500586
# test job: [2974aa42e6696a1d95b727d677dc01a71af5b998] https://lava.sirena.org.uk/scheduler/job/2502174
# test job: [d90c0f78379454d51a428e312ac6db573060185c] https://lava.sirena.org.uk/scheduler/job/2500322
# test job: [5c74a008ffc62fc57a041602b4517519c8bf9436] https://lava.sirena.org.uk/scheduler/job/2496391
# test job: [260c3fff1fefc570d8f23e87953e181d7d248861] https://lava.sirena.org.uk/scheduler/job/2494171
# test job: [7c12f6ead4672cb08b74e6f6115eb04dca8ccfa4] https://lava.sirena.org.uk/scheduler/job/2488528
# test job: [37983fad7f3ef296fa0504c8e945987459dc5487] https://lava.sirena.org.uk/scheduler/job/2489154
# test job: [ada32396f90951e12465224c04742607ca56a982] https://lava.sirena.org.uk/scheduler/job/2489236
# test job: [e02902dd493bf9c9b05353c761737ac514ad7a5c] https://lava.sirena.org.uk/scheduler/job/2489718
# test job: [507a071d9868cb60e4e76f8a06fc8eb014f59ae4] https://lava.sirena.org.uk/scheduler/job/2486375
# test job: [fed6e5084894373d76270cad4a32eb6479ad8247] https://lava.sirena.org.uk/scheduler/job/2484717
# test job: [5ebc20921b7fff9feb44de465448e17a382c9965] https://lava.sirena.org.uk/scheduler/job/2485130
# test job: [171b3663f33e1efdc97f5112f49be10b47b20fa8] https://lava.sirena.org.uk/scheduler/job/2482581
# test job: [c2bcf62ca75c541ec4297e6ff02a68ddc2e02029] https://lava.sirena.org.uk/scheduler/job/2483282
# test job: [0556bb42a84ee391a2145ddba86756f9747bc27f] https://lava.sirena.org.uk/scheduler/job/2483207
# test job: [d075cef4af6327a5de4bee7bf77591e3201e54f4] https://lava.sirena.org.uk/scheduler/job/2483468
# test job: [78dfbd4ad0be9f51de7b9a19388809254aeccd26] https://lava.sirena.org.uk/scheduler/job/2483108
# test job: [bf122191473e26a8f195308b1ba924c98424c8e1] https://lava.sirena.org.uk/scheduler/job/2482758
# test job: [fbb4c52ccdcb4a612d2b7f800aa57090eeee16d7] https://lava.sirena.org.uk/scheduler/job/2482462
# test job: [cf7c3c02fdd0dfccf4d6611714273dcb538af2cb] https://lava.sirena.org.uk/scheduler/job/2617478
# bad: [cf7c3c02fdd0dfccf4d6611714273dcb538af2cb] Add linux-next specific files for 20260330
git bisect bad cf7c3c02fdd0dfccf4d6611714273dcb538af2cb
# test job: [c99ea8b71328bb73baf24b2fb1591e076f1617a1] https://lava.sirena.org.uk/scheduler/job/2608073
# bad: [c99ea8b71328bb73baf24b2fb1591e076f1617a1] Merge branch 'clk-rpi' into clk-next
git bisect bad c99ea8b71328bb73baf24b2fb1591e076f1617a1
# test job: [672299736af6c398e867782708b7400957e62c76] https://lava.sirena.org.uk/scheduler/job/2608599
# bad: [672299736af6c398e867782708b7400957e62c76] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
git bisect bad 672299736af6c398e867782708b7400957e62c76
# first bad commit: [672299736af6c398e867782708b7400957e62c76] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
# test job: [672299736af6c398e867782708b7400957e62c76] https://lava.sirena.org.uk/scheduler/job/2608599
# bad: [672299736af6c398e867782708b7400957e62c76] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
git bisect bad 672299736af6c398e867782708b7400957e62c76
# first bad commit: [672299736af6c398e867782708b7400957e62c76] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: reset: imx8mq: Add _N suffix to IMX8MQ_RESET_MIPI_CSI*_RESET
From: Philipp Zabel @ 2026-03-31 12:45 UTC (permalink / raw)
To: Robby Cai, robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam
Cc: devicetree, kernel, imx, linux-arm-kernel, linux-kernel,
aisheng.dong
In-Reply-To: <20260331101331.1405588-2-robby.cai@nxp.com>
On Di, 2026-03-31 at 18:13 +0800, Robby Cai wrote:
> The assert logic of the MIPI CSI reset signals is active-low on i.MX8MQ,
> but the existing names do not indicate this explicitly. To improve
> consistency and clarity, append the _N suffix to all
> IMX8MQ_RESET_MIPI_CSI*_RESET definitions. The deprecated
> IMX8MQ_RESET_MIPI_CSI*_RESET versions remain temporarily for DT ABI
> compatibility and will be removed at an appropriate time in the future.
The register description in the latest reference manual I can download,
IMX8MDQLQRM Rev. 3.1 (06/2021), still call these bits
MIPI_CSI1_CORE_RESET and so on (without _N). There is no mention of
polarity in the bitfield description. Is a documentation update
planned?
Right now I'd say this improves clarity, but reduces consistency with
existing documentation.
Are these bits self-clearing, or can the reset be asserted by writing
0? As it stands, the CSI driver using these resets, imx8mq-mipi-csi2.c,
only calls reset_control_assert() in imx8mq_mipi_csi_sw_reset():
/*
* these are most likely self-clearing reset bits. to make it
* more clear, the reset-imx7 driver should implement the
* .reset() operation.
*/
ret = reset_control_assert(state->rst);
This will probably have to be turned into a deassert together with the
reset driver change.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v1] media: rkisp1: Add support for CAC
From: Jacopo Mondi @ 2026-03-31 12:42 UTC (permalink / raw)
To: Barnabás Pőcze
Cc: Jacopo Mondi, Dafna Hirschfeld, Laurent Pinchart,
Mauro Carvalho Chehab, Heiko Stuebner, linux-media,
linux-rockchip, linux-arm-kernel, linux-kernel
In-Reply-To: <6c7ffd2d-94fa-4408-ac24-1e7adac95e5d@ideasonboard.com>
Hi Barnabás
On Mon, Mar 30, 2026 at 04:40:18PM +0200, Barnabás Pőcze wrote:
> Hi
>
> 2026. 03. 25. 16:21 keltezéssel, Jacopo Mondi írta:
> > Hi Barnabás
> >
> > On Mon, Mar 23, 2026 at 03:02:16PM +0100, Barnabás Pőcze wrote:
> > > The CAC block implements chromatic aberration correction. Expose it to
> > > userspace using the extensible parameters format. This was tested on the
> > > i.MX8MP platform, but based on available documentation it is also present
> > > in the RK3399 variant (V10). Thus presumably also in later versions,
> > > so no feature flag is introduced.
> > >
> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >
> > Only minors..
> >
> > > ---
> > > .../platform/rockchip/rkisp1/rkisp1-params.c | 69 ++++++++++++
> > > .../platform/rockchip/rkisp1/rkisp1-regs.h | 21 +++-
> > > include/uapi/linux/rkisp1-config.h | 106 +++++++++++++++++-
> > > 3 files changed, 193 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > index 6442436a5e428..b889af9dcee45 100644
> > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > @@ -64,6 +64,7 @@ union rkisp1_ext_params_config {
> > > struct rkisp1_ext_params_compand_bls_config compand_bls;
> > > struct rkisp1_ext_params_compand_curve_config compand_curve;
> > > struct rkisp1_ext_params_wdr_config wdr;
> > > + struct rkisp1_ext_params_cac_config cac;
> > > };
> > >
> > > enum rkisp1_params_formats {
> > > @@ -1413,6 +1414,48 @@ static void rkisp1_wdr_config(struct rkisp1_params *params,
> > > RKISP1_CIF_ISP_WDR_TONE_CURVE_YM_MASK);
> > > }
> > >
> > > +static void
> > > +rkisp1_cac_config(struct rkisp1_params *params,
> > > + const struct rkisp1_cif_isp_cac_config *arg)
> >
> > Fits in one line without going over 80 cols
>
> This is what the other functions looks like, so went this this.
It seems to me not all of them are broken, but only the ones that go
over 80 cols
in example:
static void rkisp1_dpf_config(struct rkisp1_params *params,
const struct rkisp1_cif_isp_dpf_config *arg)
A detail anyway, up to you
>
>
> >
> > > +{
> > > + u32 regval;
> > > +
> > > + /*
> > > + * The enable bit is in the same register (RKISP1_CIF_ISP_CAC_CTRL),
> > > + * so only set the clipping mode, and do not modify the other bits.
> > > + */
> > > + regval = rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_CAC_CTRL);
> > > + regval &= ~(RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE |
> > > + RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE);
> > > + regval |= FIELD_PREP(RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE, arg->h_clip_mode) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE, arg->v_clip_mode);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_CTRL, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_COUNT_START_H_MASK, arg->h_count_start) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_COUNT_START_V_MASK, arg->v_count_start);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_COUNT_START, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_A_RED_MASK, arg->red[0]) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_A_BLUE_MASK, arg->blue[0]);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_A, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_B_RED_MASK, arg->red[1]) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_B_BLUE_MASK, arg->blue[1]);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_B, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_C_RED_MASK, arg->red[2]) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_C_BLUE_MASK, arg->blue[2]);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_C, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_X_NORM_NF_MASK, arg->x_nf) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_X_NORM_NS_MASK, arg->x_ns);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_X_NORM, regval);
> > > +
> > > + regval = FIELD_PREP(RKISP1_CIF_ISP_CAC_Y_NORM_NF_MASK, arg->y_nf) |
> > > + FIELD_PREP(RKISP1_CIF_ISP_CAC_Y_NORM_NS_MASK, arg->y_ns);
> > > + rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_CAC_Y_NORM, regval);
> > > +}
> > > +
> > > static void
> > > rkisp1_isp_isr_other_config(struct rkisp1_params *params,
> > > const struct rkisp1_params_cfg *new_params)
> > > @@ -2089,6 +2132,25 @@ static void rkisp1_ext_params_wdr(struct rkisp1_params *params,
> > > RKISP1_CIF_ISP_WDR_CTRL_ENABLE);
> > > }
> > >
> > > +static void rkisp1_ext_params_cac(struct rkisp1_params *params,
> > > + const union rkisp1_ext_params_config *block)
> > > +{
> > > + const struct rkisp1_ext_params_cac_config *cac = &block->cac;
> > > +
> > > + if (cac->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_cac_config(params, &cac->config);
> > > +
> > > + if ((cac->header.flags & RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE) &&
> > > + !(params->enabled_blocks & BIT(cac->header.type)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > +}
> > > +
> > > typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
> > > const union rkisp1_ext_params_config *config);
> > >
> > > @@ -2185,6 +2247,10 @@ static const struct rkisp1_ext_params_handler {
> > > .handler = rkisp1_ext_params_wdr,
> > > .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS,
> > > },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC] = {
> > > + .handler = rkisp1_ext_params_cac,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS,
> > > + },
> > > };
> > >
> > > #define RKISP1_PARAMS_BLOCK_INFO(block, data) \
> > > @@ -2215,6 +2281,7 @@ rkisp1_ext_params_block_types_info[] = {
> > > RKISP1_PARAMS_BLOCK_INFO(COMPAND_EXPAND, compand_curve),
> > > RKISP1_PARAMS_BLOCK_INFO(COMPAND_COMPRESS, compand_curve),
> > > RKISP1_PARAMS_BLOCK_INFO(WDR, wdr),
> > > + RKISP1_PARAMS_BLOCK_INFO(CAC, cac),
> > > };
> > >
> > > static_assert(ARRAY_SIZE(rkisp1_ext_params_handlers) ==
> > > @@ -2474,6 +2541,8 @@ void rkisp1_params_disable(struct rkisp1_params *params)
> > > rkisp1_ie_enable(params, false);
> > > rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > > RKISP1_CIF_ISP_DPF_MODE_EN);
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CAC_CTRL,
> > > + RKISP1_CIF_ISP_CAC_CTRL_ENABLE);
> > > }
> > >
> > > static const struct rkisp1_params_ops rkisp1_v10_params_ops = {
> > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > index fbeb186cde0d5..8e25537459bbd 100644
> > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h
> > > @@ -724,6 +724,23 @@
> > > #define RKISP1_CIF_ISP_WDR_DMIN_STRENGTH_MASK GENMASK(20, 16)
> > > #define RKISP1_CIF_ISP_WDR_DMIN_STRENGTH_MAX 16U
> > >
> > > +/* CAC */
> > > +#define RKISP1_CIF_ISP_CAC_CTRL_ENABLE BIT(0)
> > > +#define RKISP1_CIF_ISP_CAC_CTRL_V_CLIP_MODE GENMASK(2, 1)
> > > +#define RKISP1_CIF_ISP_CAC_CTRL_H_CLIP_MODE GENMASK(3, 3)
> > > +#define RKISP1_CIF_ISP_CAC_COUNT_START_H_MASK GENMASK(12, 0)
> > > +#define RKISP1_CIF_ISP_CAC_COUNT_START_V_MASK GENMASK(28, 16)
> > > +#define RKISP1_CIF_ISP_CAC_A_RED_MASK GENMASK(8, 0)
> > > +#define RKISP1_CIF_ISP_CAC_A_BLUE_MASK GENMASK(24, 16)
> > > +#define RKISP1_CIF_ISP_CAC_B_RED_MASK GENMASK(8, 0)
> > > +#define RKISP1_CIF_ISP_CAC_B_BLUE_MASK GENMASK(24, 16)
> > > +#define RKISP1_CIF_ISP_CAC_C_RED_MASK GENMASK(8, 0)
> > > +#define RKISP1_CIF_ISP_CAC_C_BLUE_MASK GENMASK(24, 16)
> >
> > All these masks for coefficients 0, 1 and 2 are identical. Maybe
> > #define RKISP1_CIF_ISP_CAC_RED_MASK GENMASK(8, 0)
> > #define RKISP1_CIF_ISP_CAC_BLUE_MASK GENMASK(24, 16)
> >
> > is enough
>
> Adjusted.
>
>
> >
> > > +#define RKISP1_CIF_ISP_CAC_X_NORM_NF_MASK GENMASK(4, 0)
> > > +#define RKISP1_CIF_ISP_CAC_X_NORM_NS_MASK GENMASK(19, 16)
> > > +#define RKISP1_CIF_ISP_CAC_Y_NORM_NF_MASK GENMASK(4, 0)
> > > +#define RKISP1_CIF_ISP_CAC_Y_NORM_NS_MASK GENMASK(19, 16)
>
> Did the same with these as well.
>
Ah thanks!
>
> > > +
> > > /* =================================================================== */
> > > /* CIF Registers */
> > > /* =================================================================== */
> > > @@ -1196,8 +1213,8 @@
> > > #define RKISP1_CIF_ISP_CAC_A (RKISP1_CIF_ISP_CAC_BASE + 0x00000008)
> > > #define RKISP1_CIF_ISP_CAC_B (RKISP1_CIF_ISP_CAC_BASE + 0x0000000c)
> > > #define RKISP1_CIF_ISP_CAC_C (RKISP1_CIF_ISP_CAC_BASE + 0x00000010)
> > > -#define RKISP1_CIF_ISP_X_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000014)
> > > -#define RKISP1_CIF_ISP_Y_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000018)
> > > +#define RKISP1_CIF_ISP_CAC_X_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000014)
> > > +#define RKISP1_CIF_ISP_CAC_Y_NORM (RKISP1_CIF_ISP_CAC_BASE + 0x00000018)
> > >
> > > #define RKISP1_CIF_ISP_EXP_BASE 0x00002600
> > > #define RKISP1_CIF_ISP_EXP_CTRL (RKISP1_CIF_ISP_EXP_BASE + 0x00000000)
> > > diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> > > index b2d2a71f7baff..d8acccaddd0e9 100644
> > > --- a/include/uapi/linux/rkisp1-config.h
> > > +++ b/include/uapi/linux/rkisp1-config.h
> > > @@ -967,6 +967,92 @@ struct rkisp1_cif_isp_wdr_config {
> > > __u8 use_iref;
> > > };
> > >
> > > +/*
> > > + * enum rkisp1_cif_isp_cac_h_clip_mode - horizontal clipping mode
> > > + *
> > > + * @RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4PX: +/- 4 pixels
> > > + * @RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4_5PX: +/- 4/5 pixels depending on bayer position
> > > + */
> > > +enum rkisp1_cif_isp_cac_h_clip_mode {
> > > + RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4PX = 0,
> > > + RKISP1_CIF_ISP_CAC_H_CLIP_MODE_4_5PX = 1,
> > > +};
> > > +
> > > +/**
> > > + * enum rkisp1_cif_isp_cac_v_clip_mode - vertical clipping mode
> > > + *
> > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_2PX: +/- 2 pixels
> > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3PX: +/- 3 pixels
> > > + * @RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3_4PX: +/- 3/4 pixels depending on bayer position
> > > + */
> > > +enum rkisp1_cif_isp_cac_v_clip_mode {
> > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_2PX = 0,
> > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3PX = 1,
> > > + RKISP1_CIF_ISP_CAC_V_CLIP_MODE_3_4PX = 2,
> > > +};
> > > +
> > > +/**
> > > + * struct rkisp1_cif_isp_cac_config - chromatic aberration correction configuration
> > > + *
> > > + * The correction is carried out by shifting the red and blue pixels relative
> > > + * to the green ones, depending on the distance from the optical center:
> >
> > Yes, the distance to the center is one parameter, but the shifting
> > amount depends on other things. I would drop the last part of the
> > sentence and move the description of the two below fields after the
> > text
>
> That's true, but within a specific image, the only varying quantity
> is the distance, so I think it is important to emphasize that.
>
> And I also quite like this structure of
> - description of step
> - parameters of step
> - description of step
> ...
>
> so I would love to keep it like this, if that's ok?
>
Ok!
>
> >
> > > + *
> > > + * @h_count_start: horizontal coordinate of the optical center (13-bit unsigned integer; [1,8191])
> > > + * @v_count_start: vertical coordinate of the optical center (13-bit unsigned integer; [1,8191])
> >
> > so these could go just before @x_nf
> >
> > > + *
> > > + * For each pixel, the x/y distances from the optical center are calculated and
> >
> > I forgot: did we establish that the correction is applied to the
> > euclidean distance or to x and y separately ?
>
> Given that there are two sets of "normalization" parameters, the assumption is that
> at least the x/y distances are transformed separately. I see two reasonable choices
> after that: (a) use the two distances separately, (b) use the radial distance. The
> documentation says (b). However, testing with sensor test patterns suggests that
> it is not the case (a horizontal/vertical boundary between appropriately colored
> regions should have a curvature after the transformation with appropriate parameters).
I now recall that you have been able to shift just one plan when using
the sensor's test pattern
>
>
> >
> > > + * then transformed into the [0,255] range based on the following formula:
> >
> > s/transformed/normalized ?
>
> To be honest I vastly prefer "transform" / "map" over "normalize" here.
You're right here, the below formula doesn't normalize the
distance in an interval but just re-scale it
>
>
> >
> > > + *
> > > + * (((d << 4) >> s) * f) >> 5
> > > + *
> > > + * where `d` is the distance, `s` and `f` are the normalization parameters:
> >
> > Can you use 'ns' and 'nf' to match the below ?
>
> Adjusted.
>
Thanks!
>
> >
> > > + *
> > > + * @x_nf: horizontal normalization scale parameter (5-bit unsigned integer; [0,31])
> > > + * @x_ns: horizontal normalization shift parameter (4-bit unsigned integer; [0,15])
> > > + *
> > > + * @y_nf: vertical normalization scale parameter (5-bit unsigned integer; [0,31])
> > > + * @y_ns: vertical normalization shift parameter (4-bit unsigned integer; [0,15])
> > > + *
> > > + * These parameters should be chosen based on the image resolution, the position
> > > + * of the optical center, and the shape of pixels: so that no normalized distance
> >
> > s/pixels:/pixels/
>
> Replaced `:` with `,`.
>
>
> >
> > > + * is larger than 255. If the pixels have square shape, the two sets of parameters
> > > + * should be equal.
> > > + *
> > > + * The actual amount of correction is calculated with a third degree polynomial:
> > > + *
> > > + * c[0] * r + c[1] * r^2 + c[2] * r^3
> > > + *
> > > + * where `c` is the set of coefficients for the given color, and `r` is distance:
> > > + *
> > > + * @red: red coefficients (5.4 two's complement; [-16,15.9375])
> > > + * @blue: blue coefficients (5.4 two's complement; [-16,15.9375])
> > > + *
> > > + * Finally, the amount is clipped as requested:
> > > + *
> > > + * @h_clip_mode: maximum horizontal shift (from enum rkisp1_cif_isp_cac_h_clip_mode)
> > > + * @v_clip_mode: maximum vertical shift (from enum rkisp1_cif_isp_cac_v_clip_mode)
> > > + *
> > > + * A positive result will shift away from the optical center, while a negative
> > > + * one will shift towards the optical center. In the latter case, the pixel
> > > + * values at the edges are duplicated.
> > > + */
> > > +struct rkisp1_cif_isp_cac_config {
> > > + __u8 h_clip_mode;
> > > + __u8 v_clip_mode;
> > > +
> > > + __u16 h_count_start;
> > > + __u16 v_count_start;
> > > +
> > > + __u16 red[3];
> > > + __u16 blue[3];
> > > +
> > > + __u8 x_nf;
> > > + __u8 x_ns;
> > > +
> > > + __u8 y_nf;
> > > + __u8 y_ns;
> > > +};
> > > +
> > > /*---------- PART2: Measurement Statistics ------------*/
> > >
> > > /**
> > > @@ -1161,6 +1247,7 @@ enum rkisp1_ext_params_block_type {
> > > RKISP1_EXT_PARAMS_BLOCK_TYPE_COMPAND_EXPAND,
> > > RKISP1_EXT_PARAMS_BLOCK_TYPE_COMPAND_COMPRESS,
> > > RKISP1_EXT_PARAMS_BLOCK_TYPE_WDR,
> > > + RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC,
> > > };
> > >
> > > /* For backward compatibility */
> > > @@ -1507,6 +1594,22 @@ struct rkisp1_ext_params_wdr_config {
> > > struct rkisp1_cif_isp_wdr_config config;
> > > } __attribute__((aligned(8)));
> > >
> > > +/**
> > > + * struct rkisp1_ext_params_cac_config - RkISP1 extensible params CAC config
> > > + *
> > > + * RkISP1 extensible parameters CAC block.
> > > + * Identified by :c:type:`RKISP1_EXT_PARAMS_BLOCK_TYPE_CAC`.
> > > + *
> > > + * @header: The RkISP1 extensible parameters header, see
> > > + * :c:type:`rkisp1_ext_params_block_header`
> > > + * @config: CAC configuration, see
> > > + * :c:type:`rkisp1_cif_isp_cac_config`
> > > + */
> > > +struct rkisp1_ext_params_cac_config {
> > > + struct rkisp1_ext_params_block_header header;
> > > + struct rkisp1_cif_isp_cac_config config;
> > > +} __attribute__((aligned(8)));
> > > +
> > > /*
> > > * The rkisp1_ext_params_compand_curve_config structure is counted twice as it
> > > * is used for both the COMPAND_EXPAND and COMPAND_COMPRESS block types.
> > > @@ -1532,7 +1635,8 @@ struct rkisp1_ext_params_wdr_config {
> > > sizeof(struct rkisp1_ext_params_compand_bls_config) +\
> > > sizeof(struct rkisp1_ext_params_compand_curve_config) +\
> > > sizeof(struct rkisp1_ext_params_compand_curve_config) +\
> > > - sizeof(struct rkisp1_ext_params_wdr_config))
> > > + sizeof(struct rkisp1_ext_params_wdr_config) +\
> > > + sizeof(struct rkisp1_ext_params_cac_config))
> >
> > All minors, please add
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> > Thanks
> > j
> >
> > >
> > > /**
> > > * enum rksip1_ext_param_buffer_version - RkISP1 extensible parameters version
> > > --
> > > 2.53.0
> > >
> > >
>
^ permalink raw reply
* [PATCH v9 2/3] drm/v3d: Allocate all resources before enabling the clock
From: Maíra Canal @ 2026-03-31 12:35 UTC (permalink / raw)
To: Stefan Wahren, Maxime Ripard, Melissa Wen, Iago Toral Quiroga,
Dave Stevenson, Florian Fainelli
Cc: dri-devel, linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev,
Maíra Canal
In-Reply-To: <20260331-v3d-power-management-v9-0-f52ff87bfd36@igalia.com>
Move all resource allocation operations before actually enabling the
clock, as those operations don't require the GPU to be powered on.
This is a preparation for runtime PM support. The next commit will
move all code related to powering on and initiating the GPU into the
runtime PM resume callback and all resource allocation will happen
before resume().
Reviewed-by: Melissa Wen <mwen@igalia.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_drv.c | 93 ++++++++++++++++++++++---------------------
drivers/gpu/drm/v3d/v3d_drv.h | 1 +
drivers/gpu/drm/v3d/v3d_gem.c | 18 ++++-----
drivers/gpu/drm/v3d/v3d_irq.c | 15 +++----
4 files changed, 62 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index c5e7c778ec7a1a39d81155f7ed2a7ba811b5e3aa..e57b36f4d81a59d15a223afdc4078ae6456de9a9 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -363,14 +363,50 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
return ret;
}
+ if (v3d->ver < V3D_GEN_41) {
+ ret = map_regs(v3d, &v3d->gca_regs, "gca");
+ if (ret)
+ return ret;
+ }
+
+ v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+ if (IS_ERR(v3d->reset))
+ return dev_err_probe(dev, PTR_ERR(v3d->reset),
+ "Failed to get reset control\n");
+
+ if (!v3d->reset) {
+ ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
+ if (ret) {
+ dev_err(dev, "Failed to get bridge registers\n");
+ return ret;
+ }
+ }
+
v3d->clk = devm_clk_get_optional(dev, NULL);
if (IS_ERR(v3d->clk))
return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to get V3D clock\n");
+ ret = v3d_irq_init(v3d);
+ if (ret)
+ return ret;
+
+ v3d_perfmon_init(v3d);
+
+ v3d->mmu_scratch = dma_alloc_wc(dev, 4096, &v3d->mmu_scratch_paddr,
+ GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
+ if (!v3d->mmu_scratch) {
+ dev_err(dev, "Failed to allocate MMU scratch page\n");
+ return -ENOMEM;
+ }
+
+ ret = v3d_gem_init(drm);
+ if (ret)
+ goto dma_free;
+
ret = clk_prepare_enable(v3d->clk);
if (ret) {
dev_err(&pdev->dev, "Couldn't enable the V3D clock\n");
- return ret;
+ goto gem_destroy;
}
v3d_idle_sms(v3d);
@@ -399,44 +435,9 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
ident3 = V3D_READ(V3D_HUB_IDENT3);
v3d->rev = V3D_GET_FIELD(ident3, V3D_HUB_IDENT3_IPREV);
- v3d_perfmon_init(v3d);
-
- v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
- if (IS_ERR(v3d->reset)) {
- ret = dev_err_probe(dev, PTR_ERR(v3d->reset),
- "Failed to get reset control\n");
- goto clk_disable;
- }
-
- if (!v3d->reset) {
- ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
- if (ret) {
- dev_err(dev, "Failed to get bridge registers\n");
- goto clk_disable;
- }
- }
-
- if (v3d->ver < V3D_GEN_41) {
- ret = map_regs(v3d, &v3d->gca_regs, "gca");
- if (ret)
- goto clk_disable;
- }
-
- v3d->mmu_scratch = dma_alloc_wc(dev, 4096, &v3d->mmu_scratch_paddr,
- GFP_KERNEL | __GFP_NOWARN | __GFP_ZERO);
- if (!v3d->mmu_scratch) {
- dev_err(dev, "Failed to allocate MMU scratch page\n");
- ret = -ENOMEM;
- goto clk_disable;
- }
-
- ret = v3d_gem_init(drm);
- if (ret)
- goto dma_free;
-
- ret = v3d_irq_init(v3d);
- if (ret)
- goto gem_destroy;
+ v3d_init_hw_state(v3d);
+ v3d_mmu_set_page_table(v3d);
+ v3d_irq_enable(v3d);
ret = drm_dev_register(drm, 0);
if (ret)
@@ -452,12 +453,13 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
drm_dev_unregister(drm);
irq_disable:
v3d_irq_disable(v3d);
+clk_disable:
+ v3d_power_off_sms(v3d);
+ clk_disable_unprepare(v3d->clk);
gem_destroy:
v3d_gem_destroy(drm);
dma_free:
dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
-clk_disable:
- clk_disable_unprepare(v3d->clk);
return ret;
}
@@ -471,14 +473,13 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
drm_dev_unregister(drm);
- v3d_gem_destroy(drm);
-
- dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch,
- v3d->mmu_scratch_paddr);
-
v3d_power_off_sms(v3d);
clk_disable_unprepare(v3d->clk);
+
+ v3d_gem_destroy(drm);
+
+ dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
}
static struct platform_driver v3d_platform_driver = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 6a3cad933439812d78da5797749c020a9bf46402..ebf406b615bb52de9cb98ea8efe941a5787fb4bd 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -565,6 +565,7 @@ struct dma_fence *v3d_fence_create(struct v3d_dev *v3d, enum v3d_queue q);
/* v3d_gem.c */
extern bool super_pages;
+void v3d_init_hw_state(struct v3d_dev *v3d);
int v3d_gem_init(struct drm_device *dev);
void v3d_gem_destroy(struct drm_device *dev);
void v3d_reset_sms(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 75d9eccd796664e67277c1f83ad59063f164d1da..def6a9612b857a241f6b2e1f601509928e3f9f8b 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -36,13 +36,6 @@ v3d_init_core(struct v3d_dev *v3d, int core)
V3D_CORE_WRITE(core, V3D_CTL_L2TFLEND, ~0);
}
-/* Sets invariant state for the HW. */
-static void
-v3d_init_hw_state(struct v3d_dev *v3d)
-{
- v3d_init_core(v3d, 0);
-}
-
static void
v3d_idle_axi(struct v3d_dev *v3d, int core)
{
@@ -259,6 +252,14 @@ v3d_invalidate_caches(struct v3d_dev *v3d)
v3d_invalidate_slices(v3d, 0);
}
+/* Sets invariant state for the HW. */
+void
+v3d_init_hw_state(struct v3d_dev *v3d)
+{
+ v3d_init_core(v3d, 0);
+}
+
+
static void
v3d_huge_mnt_init(struct v3d_dev *v3d)
{
@@ -328,9 +329,6 @@ v3d_gem_init(struct drm_device *dev)
goto err_dma_alloc;
}
- v3d_init_hw_state(v3d);
- v3d_mmu_set_page_table(v3d);
-
v3d_huge_mnt_init(v3d);
ret = v3d_sched_init(v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index c28e74ab5442857031b48bcbd4e43eb48c1e0f07..86efaef2722c3602346b037ba536228b2f368a81 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -248,17 +248,10 @@ v3d_hub_irq(int irq, void *arg)
int
v3d_irq_init(struct v3d_dev *v3d)
{
- int irq, ret, core;
+ int irq, ret;
INIT_WORK(&v3d->overflow_mem_work, v3d_overflow_mem_work);
- /* Clear any pending interrupts someone might have left around
- * for us.
- */
- for (core = 0; core < v3d->cores; core++)
- V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
- V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));
-
irq = platform_get_irq_optional(v3d_to_pdev(v3d), 1);
if (irq == -EPROBE_DEFER)
return irq;
@@ -296,7 +289,6 @@ v3d_irq_init(struct v3d_dev *v3d)
goto fail;
}
- v3d_irq_enable(v3d);
return 0;
fail:
@@ -310,6 +302,11 @@ v3d_irq_enable(struct v3d_dev *v3d)
{
int core;
+ /* Clear any pending interrupts someone might have left around for us. */
+ for (core = 0; core < v3d->cores; core++)
+ V3D_CORE_WRITE(core, V3D_CTL_INT_CLR, V3D_CORE_IRQS(v3d->ver));
+ V3D_WRITE(V3D_HUB_INT_CLR, V3D_HUB_IRQS(v3d->ver));
+
/* Enable our set of interrupts, masking out any others. */
for (core = 0; core < v3d->cores; core++) {
V3D_CORE_WRITE(core, V3D_CTL_INT_MSK_SET, ~V3D_CORE_IRQS(v3d->ver));
--
2.53.0
^ permalink raw reply related
* [PATCH v9 3/3] drm/v3d: Introduce Runtime Power Management
From: Maíra Canal @ 2026-03-31 12:35 UTC (permalink / raw)
To: Stefan Wahren, Maxime Ripard, Melissa Wen, Iago Toral Quiroga,
Dave Stevenson, Florian Fainelli
Cc: dri-devel, linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev,
Maíra Canal
In-Reply-To: <20260331-v3d-power-management-v9-0-f52ff87bfd36@igalia.com>
Commit 90a64adb0876 ("drm/v3d: Get rid of pm code") removed the last
bits of power management code that V3D had, which were actually never
hooked. Therefore, currently, the GPU clock is enabled during probe
and only disabled when removing the driver.
Implement proper power management using the kernel's Runtime PM
framework.
Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/Makefile | 1 +
drivers/gpu/drm/v3d/v3d_debugfs.c | 23 ++++++++++-
drivers/gpu/drm/v3d/v3d_drv.c | 84 +++++++++++++++++--------------------
drivers/gpu/drm/v3d/v3d_drv.h | 17 ++++++++
drivers/gpu/drm/v3d/v3d_mmu.c | 10 ++++-
drivers/gpu/drm/v3d/v3d_perfmon.c | 18 ++++++--
drivers/gpu/drm/v3d/v3d_power.c | 87 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/v3d/v3d_submit.c | 19 +++++++--
8 files changed, 200 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/v3d/Makefile b/drivers/gpu/drm/v3d/Makefile
index b7d673f1153bef16db3800e50b2bfaf36bf8871b..601b834e377e8342c6668645112347cca4214024 100644
--- a/drivers/gpu/drm/v3d/Makefile
+++ b/drivers/gpu/drm/v3d/Makefile
@@ -10,6 +10,7 @@ v3d-y := \
v3d_irq.o \
v3d_mmu.o \
v3d_perfmon.o \
+ v3d_power.o \
v3d_trace_points.o \
v3d_sched.o \
v3d_sysfs.o \
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 89f24eec62a74ec49b28f0b22dbf626ba7a35206..634cc796ba2324dc497694c070f2cfffcc4424c9 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -97,7 +97,11 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
struct drm_debugfs_entry *entry = m->private;
struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
- int i, core;
+ int i, core, ret;
+
+ ret = v3d_pm_runtime_get(v3d);
+ if (ret)
+ return ret;
for (i = 0; i < ARRAY_SIZE(v3d_hub_reg_defs); i++) {
const struct v3d_reg_def *def = &v3d_hub_reg_defs[i];
@@ -139,6 +143,8 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
}
}
+ v3d_pm_runtime_put(v3d);
+
return 0;
}
@@ -148,7 +154,11 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
u32 ident0, ident1, ident2, ident3, cores;
- int core;
+ int core, ret;
+
+ ret = v3d_pm_runtime_get(v3d);
+ if (ret)
+ return ret;
ident0 = V3D_READ(V3D_HUB_IDENT0);
ident1 = V3D_READ(V3D_HUB_IDENT1);
@@ -207,6 +217,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
}
}
+ v3d_pm_runtime_put(v3d);
+
return 0;
}
@@ -234,6 +246,11 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
uint32_t cycles;
int core = 0;
int measure_ms = 1000;
+ int ret;
+
+ ret = v3d_pm_runtime_get(v3d);
+ if (ret)
+ return ret;
if (v3d->ver >= V3D_GEN_41) {
int cycle_count_reg = V3D_PCTR_CYCLE_COUNT(v3d->ver);
@@ -253,6 +270,8 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
msleep(measure_ms);
cycles = V3D_CORE_READ(core, V3D_PCTR_0_PCTR0);
+ v3d_pm_runtime_put(v3d);
+
seq_printf(m, "cycles: %d (%d.%d Mhz)\n",
cycles,
cycles / (measure_ms * 1000),
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index e57b36f4d81a59d15a223afdc4078ae6456de9a9..fc81dd1247e33e15cd838cf11ae46d79aaf03976 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -59,6 +59,7 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
[DRM_V3D_PARAM_V3D_CORE0_IDENT1] = V3D_CTL_IDENT1,
[DRM_V3D_PARAM_V3D_CORE0_IDENT2] = V3D_CTL_IDENT2,
};
+ int ret;
if (args->pad != 0)
return -EINVAL;
@@ -75,12 +76,19 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
if (args->value != 0)
return -EINVAL;
+ ret = v3d_pm_runtime_get(v3d);
+ if (ret)
+ return ret;
+
if (args->param >= DRM_V3D_PARAM_V3D_CORE0_IDENT0 &&
args->param <= DRM_V3D_PARAM_V3D_CORE0_IDENT2) {
args->value = V3D_CORE_READ(0, offset);
} else {
args->value = V3D_READ(offset);
}
+
+ v3d_pm_runtime_put(v3d);
+
return 0;
}
@@ -290,36 +298,6 @@ static const struct of_device_id v3d_of_match[] = {
};
MODULE_DEVICE_TABLE(of, v3d_of_match);
-static void
-v3d_idle_sms(struct v3d_dev *v3d)
-{
- if (v3d->ver < V3D_GEN_71)
- return;
-
- V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
-
- if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
- V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
- drm_err(&v3d->drm, "Failed to power up SMS\n");
- }
-
- v3d_reset_sms(v3d);
-}
-
-static void
-v3d_power_off_sms(struct v3d_dev *v3d)
-{
- if (v3d->ver < V3D_GEN_71)
- return;
-
- V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
-
- if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
- V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
- drm_err(&v3d->drm, "Failed to power off SMS\n");
- }
-}
-
static int
map_regs(struct v3d_dev *v3d, void __iomem **regs, const char *name)
{
@@ -403,19 +381,26 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
if (ret)
goto dma_free;
- ret = clk_prepare_enable(v3d->clk);
- if (ret) {
- dev_err(&pdev->dev, "Couldn't enable the V3D clock\n");
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
goto gem_destroy;
- }
- v3d_idle_sms(v3d);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ goto gem_destroy;
+
+ /* If PM is disabled, we need to call v3d_power_resume() manually. */
+ if (!IS_ENABLED(CONFIG_PM)) {
+ ret = v3d_power_resume(dev);
+ if (ret)
+ goto gem_destroy;
+ }
mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
mask = DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
ret = dma_set_mask_and_coherent(dev, mask);
if (ret)
- goto clk_disable;
+ goto runtime_pm_put;
dma_set_max_seg_size(&pdev->dev, UINT_MAX);
@@ -435,27 +420,26 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
ident3 = V3D_READ(V3D_HUB_IDENT3);
v3d->rev = V3D_GET_FIELD(ident3, V3D_HUB_IDENT3_IPREV);
- v3d_init_hw_state(v3d);
- v3d_mmu_set_page_table(v3d);
- v3d_irq_enable(v3d);
+ pm_runtime_set_autosuspend_delay(dev, 100);
+ pm_runtime_use_autosuspend(dev);
ret = drm_dev_register(drm, 0);
if (ret)
- goto irq_disable;
+ goto runtime_pm_put;
ret = v3d_sysfs_init(dev);
if (ret)
goto drm_unregister;
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
return 0;
drm_unregister:
drm_dev_unregister(drm);
-irq_disable:
- v3d_irq_disable(v3d);
-clk_disable:
- v3d_power_off_sms(v3d);
- clk_disable_unprepare(v3d->clk);
+runtime_pm_put:
+ pm_runtime_put_sync_suspend(dev);
gem_destroy:
v3d_gem_destroy(drm);
dma_free:
@@ -473,21 +457,27 @@ static void v3d_platform_drm_remove(struct platform_device *pdev)
drm_dev_unregister(drm);
- v3d_power_off_sms(v3d);
+ pm_runtime_suspend(dev);
- clk_disable_unprepare(v3d->clk);
+ /* If PM is disabled, we need to call v3d_power_suspend() manually. */
+ if (!IS_ENABLED(CONFIG_PM))
+ v3d_power_suspend(dev);
v3d_gem_destroy(drm);
dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
}
+static DEFINE_RUNTIME_DEV_PM_OPS(v3d_pm_ops, v3d_power_suspend,
+ v3d_power_resume, NULL);
+
static struct platform_driver v3d_platform_driver = {
.probe = v3d_platform_drm_probe,
.remove = v3d_platform_drm_remove,
.driver = {
.name = "v3d",
.of_match_table = v3d_of_match,
+ .pm = pm_ptr(&v3d_pm_ops),
},
};
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index ebf406b615bb52de9cb98ea8efe941a5787fb4bd..4ebe175a8c6b0016d087388ce02cd35a8ae65a13 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -3,6 +3,7 @@
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
#include <linux/spinlock_types.h>
#include <linux/workqueue.h>
@@ -324,6 +325,8 @@ struct v3d_job {
/* Callback for the freeing of the job on refcount going to 0. */
void (*free)(struct kref *ref);
+
+ bool has_pm_ref;
};
struct v3d_bin_job {
@@ -597,6 +600,20 @@ int v3d_mmu_set_page_table(struct v3d_dev *v3d);
void v3d_mmu_insert_ptes(struct v3d_bo *bo);
void v3d_mmu_remove_ptes(struct v3d_bo *bo);
+/* v3d_power.c */
+int v3d_power_suspend(struct device *dev);
+int v3d_power_resume(struct device *dev);
+
+static __always_inline int v3d_pm_runtime_get(struct v3d_dev *v3d)
+{
+ return pm_runtime_resume_and_get(v3d->drm.dev);
+}
+
+static __always_inline int v3d_pm_runtime_put(struct v3d_dev *v3d)
+{
+ return pm_runtime_put_autosuspend(v3d->drm.dev);
+}
+
/* v3d_sched.c */
void v3d_timestamp_query_info_free(struct v3d_timestamp_query_info *query_info,
unsigned int count);
diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/v3d_mmu.c
index c513a393c0313772650fd6d7236127b2dc4101d9..630c64e51d2f2ad30e59fa2b175487efe0bfba49 100644
--- a/drivers/gpu/drm/v3d/v3d_mmu.c
+++ b/drivers/gpu/drm/v3d/v3d_mmu.c
@@ -39,7 +39,11 @@ static bool v3d_mmu_is_aligned(u32 page, u32 page_address, size_t alignment)
int v3d_mmu_flush_all(struct v3d_dev *v3d)
{
- int ret;
+ int ret = 0;
+
+ /* Flush the PTs only if we're already awake */
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ return 0;
V3D_WRITE(V3D_MMUC_CONTROL, V3D_MMUC_CONTROL_FLUSH |
V3D_MMUC_CONTROL_ENABLE);
@@ -48,7 +52,7 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
V3D_MMUC_CONTROL_FLUSHING), 100);
if (ret) {
dev_err(v3d->drm.dev, "MMUC flush wait idle failed\n");
- return ret;
+ goto pm_put;
}
V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL) |
@@ -59,6 +63,8 @@ int v3d_mmu_flush_all(struct v3d_dev *v3d)
if (ret)
dev_err(v3d->drm.dev, "MMU TLB clear wait idle failed\n");
+pm_put:
+ v3d_pm_runtime_put(v3d);
return ret;
}
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 8e0249580bbacac507b2d7c0bcac37ef19c1a54e..02451fc09dbbf6d33640000249786e2836732647 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -232,6 +232,9 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
if (WARN_ON_ONCE(!perfmon || v3d->active_perfmon))
return;
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ return;
+
ncounters = perfmon->ncounters;
mask = GENMASK(ncounters - 1, 0);
@@ -257,6 +260,8 @@ void v3d_perfmon_start(struct v3d_dev *v3d, struct v3d_perfmon *perfmon)
V3D_CORE_WRITE(0, V3D_PCTR_0_OVERFLOW, mask);
v3d->active_perfmon = perfmon;
+
+ v3d_pm_runtime_put(v3d);
}
void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
@@ -268,10 +273,11 @@ void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
return;
mutex_lock(&perfmon->lock);
- if (perfmon != v3d->active_perfmon) {
- mutex_unlock(&perfmon->lock);
- return;
- }
+ if (perfmon != v3d->active_perfmon)
+ goto out;
+
+ if (!pm_runtime_get_if_active(v3d->drm.dev))
+ goto out_clear;
if (capture)
for (i = 0; i < perfmon->ncounters; i++)
@@ -279,7 +285,11 @@ void v3d_perfmon_stop(struct v3d_dev *v3d, struct v3d_perfmon *perfmon,
V3D_CORE_WRITE(0, V3D_V4_PCTR_0_EN, 0);
+ v3d_pm_runtime_put(v3d);
+
+out_clear:
v3d->active_perfmon = NULL;
+out:
mutex_unlock(&perfmon->lock);
}
diff --git a/drivers/gpu/drm/v3d/v3d_power.c b/drivers/gpu/drm/v3d/v3d_power.c
new file mode 100644
index 0000000000000000000000000000000000000000..769e90032b042ab3471259d3f0ddd7c8a3f3f7b2
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_power.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2026 Raspberry Pi */
+
+#include <linux/clk.h>
+
+#include <drm/drm_print.h>
+
+#include "v3d_drv.h"
+#include "v3d_regs.h"
+
+static int
+v3d_resume_sms(struct v3d_dev *v3d)
+{
+ if (v3d->ver < V3D_GEN_71)
+ return 0;
+
+ V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_CLEAR_POWER_OFF);
+
+ if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+ V3D_SMS_STATE) == V3D_SMS_IDLE), 100)) {
+ drm_err(&v3d->drm, "Failed to power up SMS\n");
+ return -ETIMEDOUT;
+ }
+
+ v3d_reset_sms(v3d);
+
+ return 0;
+}
+
+static int
+v3d_suspend_sms(struct v3d_dev *v3d)
+{
+ if (v3d->ver < V3D_GEN_71)
+ return 0;
+
+ V3D_SMS_WRITE(V3D_SMS_TEE_CS, V3D_SMS_POWER_OFF);
+
+ if (wait_for((V3D_GET_FIELD(V3D_SMS_READ(V3D_SMS_TEE_CS),
+ V3D_SMS_STATE) == V3D_SMS_POWER_OFF_STATE), 100)) {
+ drm_err(&v3d->drm, "Failed to power off SMS\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+int v3d_power_suspend(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+ struct v3d_dev *v3d = to_v3d_dev(drm);
+ int ret;
+
+ v3d_irq_disable(v3d);
+
+ ret = v3d_suspend_sms(v3d);
+ if (ret) {
+ v3d_irq_enable(v3d);
+ return ret;
+ }
+
+ clk_disable_unprepare(v3d->clk);
+
+ return 0;
+}
+
+int v3d_power_resume(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+ struct v3d_dev *v3d = to_v3d_dev(drm);
+ int ret;
+
+ ret = clk_prepare_enable(v3d->clk);
+ if (ret)
+ return ret;
+
+ ret = v3d_resume_sms(v3d);
+ if (ret) {
+ clk_disable_unprepare(v3d->clk);
+ return ret;
+ }
+
+ v3d_init_hw_state(v3d);
+ v3d_mmu_set_page_table(v3d);
+ v3d_irq_enable(v3d);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 8f061b6a05c6aa76ea5513407ebf3c0ce80b8256..f75da2e3533e236821818924f91ec602950af85a 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -106,6 +106,9 @@ v3d_job_free(struct kref *ref)
v3d_stats_put(job->client_stats);
v3d_stats_put(job->global_stats);
+ if (job->has_pm_ref)
+ v3d_pm_runtime_put(job->v3d);
+
kfree(job);
}
@@ -187,13 +190,13 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
if (copy_from_user(&in, handle++, sizeof(in))) {
ret = -EFAULT;
drm_dbg(&v3d->drm, "Failed to copy wait dep handle.\n");
- goto fail_deps;
+ goto fail_job_init;
}
ret = drm_sched_job_add_syncobj_dependency(&job->base, file_priv, in.handle, 0);
// TODO: Investigate why this was filtered out for the IOCTL.
if (ret && ret != -ENOENT)
- goto fail_deps;
+ goto fail_job_init;
}
}
} else {
@@ -201,7 +204,15 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
// TODO: Investigate why this was filtered out for the IOCTL.
if (ret && ret != -ENOENT)
- goto fail_deps;
+ goto fail_job_init;
+ }
+
+ /* CPU jobs don't require hardware resources */
+ if (queue != V3D_CPU) {
+ ret = v3d_pm_runtime_get(v3d);
+ if (ret)
+ goto fail_job_init;
+ job->has_pm_ref = true;
}
kref_init(&job->refcount);
@@ -211,7 +222,7 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
return 0;
-fail_deps:
+fail_job_init:
drm_sched_job_cleanup(&job->base);
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v9 1/3] drm/v3d: Use devm_reset_control_get_optional_exclusive()
From: Maíra Canal @ 2026-03-31 12:35 UTC (permalink / raw)
To: Stefan Wahren, Maxime Ripard, Melissa Wen, Iago Toral Quiroga,
Dave Stevenson, Florian Fainelli
Cc: dri-devel, linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev, Philipp Zabel,
Maíra Canal
In-Reply-To: <20260331-v3d-power-management-v9-0-f52ff87bfd36@igalia.com>
Simplify optional reset handling by using the function
devm_reset_control_get_optional_exclusive().
Reviewed-by: Melissa Wen <mwen@igalia.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_drv.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 4b441afcb602de08bd193d57649121e44ab31f2a..c5e7c778ec7a1a39d81155f7ed2a7ba811b5e3aa 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -401,18 +401,17 @@ static int v3d_platform_drm_probe(struct platform_device *pdev)
v3d_perfmon_init(v3d);
- v3d->reset = devm_reset_control_get_exclusive(dev, NULL);
+ v3d->reset = devm_reset_control_get_optional_exclusive(dev, NULL);
if (IS_ERR(v3d->reset)) {
- ret = PTR_ERR(v3d->reset);
+ ret = dev_err_probe(dev, PTR_ERR(v3d->reset),
+ "Failed to get reset control\n");
+ goto clk_disable;
+ }
- if (ret == -EPROBE_DEFER)
- goto clk_disable;
-
- v3d->reset = NULL;
+ if (!v3d->reset) {
ret = map_regs(v3d, &v3d->bridge_regs, "bridge");
if (ret) {
- dev_err(dev,
- "Failed to get reset control or bridge regs\n");
+ dev_err(dev, "Failed to get bridge registers\n");
goto clk_disable;
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH v9 0/3] Power Management for Raspberry Pi V3D GPU
From: Maíra Canal @ 2026-03-31 12:35 UTC (permalink / raw)
To: Stefan Wahren, Maxime Ripard, Melissa Wen, Iago Toral Quiroga,
Dave Stevenson, Florian Fainelli
Cc: dri-devel, linux-rpi-kernel, linux-arm-kernel,
Broadcom internal kernel review list, kernel-dev, Philipp Zabel,
Maíra Canal
This series introduces Runtime Power Management (PM) support for the
Raspberry Pi V3D GPU.
Currently, the V3D clock remains enabled for the entire system uptime,
even when the GPU is idle. With the introduction of Runtime PM, the
clock can now be disabled during idle periods. For example, with this
series applied to a Raspberry Pi 5, if we check `vcgencmd measure_clock
v3d`, we get:
(idle)
$ vcgencmd measure_clock v3d
frequency(0)=0
(running glmark2)
$ vcgencmd measure_clock v3d
frequency(0)=960016128
I left Melissa's R-b in the last patch even with the changes, as I
considered them uncontroversial. However, Melissa, please let me know if
you would like to have your tag removed or you would like to request more
changes to the last patch.
In the case of no comments, I plan to merge this series by the end of
next week.
Best regards,
- Maíra
v1 -> v2: https://lore.kernel.org/r/20250728-v3d-power-management-v1-0-780f922b1048@igalia.com
- [1/5] NEW PATCH: "clk: bcm: rpi: Add missing logs if firmware fails" (Stefan Wahren)
- [2/5] Remove the "Fixes:" tag (Stefan Wahren)
- [2/5] dev_err_ratelimited() instead of dev_err() (Stefan Wahren)
- [2/5] Instead of logging the clock ID, use clk_hw_get_name(hw) to log the name (Stefan Wahren)
- [2/5] Add a newline character at the end of the log message (Stefan Wahren)
- [2/5] Use CLK_IS_CRITICAL for all clocks that can't be disabled (Maxime Ripard)
- [3/5] NEW PATCH: "clk: bcm: rpi: Maximize V3D clock"
- [4/5] Use devm_reset_control_get_optional_exclusive() (Philipp Zabel)
- [4/5] Make sure that resources are cleaned in the inverse order of allocation (Philipp Zabel)
v2 -> v3: https://lore.kernel.org/r/20250731-v3d-power-management-v2-0-032d56b01964@igalia.com
- Rebased on top of drm-misc-next
- Patches "[PATCH v2 1/5] clk: bcm: rpi: Add missing logs if firmware
fails", "[PATCH v2 2/5] clk: bcm: rpi: Turn firmware clock on/off when
preparing/unpreparing", and "[PATCH v2 3/5] clk: bcm: rpi: Maximize
V3D clock" were applied to clk-next.
- [1/4] NEW PATCH: "clk: bcm: rpi: Let V3D consumers manage clock rate"
- [2/4] NEW PATCH: "clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED"
- [3/4] Add Philipp's R-b (Philipp Zabel)
- [4/4] s/DRM_ERROR/drm_err
- [4/4] Set the clock rate to 0 during suspend and to the maximum rate during resume
v3 -> v4: https://lore.kernel.org/r/20260116-v3d-power-management-v3-0-4e1874e81dd6@igalia.com
- Rebased on top of drm-misc-next
- [1/6, 3/6] Add Melissa's A-b (Melissa Wen)
- [2/6] NEW PATCH: "clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE
behavior" (Stefan Wahren)
- [4/6] NEW PATCH: "drm/v3d: Use devm_reset_control_get_optional_exclusive()" (Melissa Wen)
- [5/6] Include more context in the commit message (Melissa Wen)
- [5/6, 6/6] Instead of creating the function v3d_gem_allocate(), use v3d_gem_init()
and move HW initialization out of it (Melissa Wen)
v4 -> v5: https://lore.kernel.org/r/20260126-v3d-power-management-v4-0-caf2df16d4e2@igalia.com
- [2/7] Add Stefan's A-b (Stefan Wahren)
- [2/7, 5/7, 6/7] Add Melissa's R-b (Melissa Wen)
- [4/7] NEW PATCH: "pmdomain: bcm: bcm2835-power: Increase ASB control timeout"
- [7/7] Remove redundant pm_runtime_mark_last_busy() from v3d_pm_runtime_put()
- [7/7] Use pm_runtime_get_if_active() in v3d_mmu_flush_all() instead of
pm_runtime_get_noresume() + pm_runtime_active()
- [7/7] Add missing PM runtime calls to v3d_perfmon_start() and v3d_perfmon_stop()
v5 -> v6: https://lore.kernel.org/r/20260213-v3d-power-management-v5-0-7a8b381eb379@igalia.com
- [1/6] NEW PATCH: "clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks" (Maxime Ripard)
- Replaces "clk: bcm: rpi: Let V3D consumers manage clock rate" and
"clk: bcm: rpi: Add a comment about RPI_FIRMWARE_SET_CLOCK_STATE
behavior"
- [6/6] Stop setting min and max clock rates directly in v3d (Maxime Ripard)
v6 -> v7: https://lore.kernel.org/r/20260218-v3d-power-management-v6-0-40683fd39865@igalia.com
- Drop commit "[PATCH v6 2/6] clk: bcm: rpi: Mark PIXEL_CLK and HEVC_CLK as CLK_IGNORE_UNUSED"
- [1/5] Add comment about why is okay to set the clock's rate at prepare/unprepare (Maxime Ripard)
- [1/5] Use clk_hw_get_rate_range() (Maxime Ripard)
- [2/5] Add Stefan's R-b and stable tag (Stefan Wahren)
- [3/5] Add Philipp's R-b (Philipp Zabel)
- [5/5] Keep the alphabetical order in the Makefile (Stefan Wahren)
- [5/5] Propagate `reset_control_assert()` error (Stefan Wahren)
- [5/5] Add v3d_init_hw_state() before v3d_mmu_set_page_table()
- [5/5] Stop any active perfmon during suspend
v7 -> v8: https://lore.kernel.org/r/20260312-v3d-power-management-v7-0-9f006a1d4c55@igalia.com
- "[PATCH v7 2/5] pmdomain: bcm: bcm2835-power: Increase ASB control timeout"
was merged through -fixes (Thanks Stefan and Ulf!)
- "[PATCH v7 1/5] clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks"
was merged through clk-fixes (Thanks Maxime and Stephen!)
- Rebased on top of drm-misc-next.
- [3/3] The perfmon will never be active during a suspend call (Melissa Wen)
- [3/3] No need to call reset_control_(de)assert() as bcm2835-power doesn't
implement these hooks.
- [3/3] Increase the autosuspend delay to 100ms, as 50ms was too short for RPi 4.
- [3/3] Add Melissa's R-b (Melissa Wen)
v8 -> v9: https://lore.kernel.org/r/20260328-v3d-power-management-v8-0-94336830df5f@igalia.com
- [1/3, 2/3] Add Florian's R-b (Florian Fainelli)
- [3/3] Handle v3d_resume_sms() and v3d_suspend_sms() errors (Florian Fainelli)
---
Maíra Canal (3):
drm/v3d: Use devm_reset_control_get_optional_exclusive()
drm/v3d: Allocate all resources before enabling the clock
drm/v3d: Introduce Runtime Power Management
drivers/gpu/drm/v3d/Makefile | 1 +
drivers/gpu/drm/v3d/v3d_debugfs.c | 23 +++++-
drivers/gpu/drm/v3d/v3d_drv.c | 160 ++++++++++++++++++--------------------
drivers/gpu/drm/v3d/v3d_drv.h | 18 +++++
drivers/gpu/drm/v3d/v3d_gem.c | 18 ++---
drivers/gpu/drm/v3d/v3d_irq.c | 15 ++--
drivers/gpu/drm/v3d/v3d_mmu.c | 10 ++-
drivers/gpu/drm/v3d/v3d_perfmon.c | 18 ++++-
drivers/gpu/drm/v3d/v3d_power.c | 87 +++++++++++++++++++++
drivers/gpu/drm/v3d/v3d_submit.c | 19 ++++-
10 files changed, 253 insertions(+), 116 deletions(-)
---
base-commit: 86a28e154c3a754cf661c91b57eb96816e8485df
change-id: 20250728-v3d-power-management-eebb2024dc96
^ permalink raw reply
* Re: [PATCH v9 3/6] mfd: max77759: add register bitmasks and modify irq configs for charger
From: Lee Jones @ 2026-03-31 12:31 UTC (permalink / raw)
To: amitsd
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
André Draszik, Greg Kroah-Hartman, Badhri Jagan Sridharan,
Heikki Krogerus, Peter Griffin, Tudor Ambarus, Alim Akhtar,
Mark Brown, Matti Vaittinen, Andrew Morton, linux-kernel,
linux-pm, devicetree, linux-usb, linux-arm-kernel,
linux-samsung-soc, RD Babiera, Kyle Tso
In-Reply-To: <20260325-max77759-charger-v9-3-4486dd297adc@google.com>
On Wed, 25 Mar 2026, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add register bitmasks for charger function.
> In addition split the charger IRQs further such that each bit represents
> an IRQ downstream of charger regmap irq chip. In addition populate the
> ack_base to offload irq ack to the regmap irq chip framework.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: André Draszik <andre.draszik@linaro.org>
> ---
> drivers/mfd/max77759.c | 95 ++++++++++++++++++++++---
> include/linux/mfd/max77759.h | 166 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 222 insertions(+), 39 deletions(-)
>
[...]
> +/*
> + * enum max77759_chgr_chgin_dtls_status - Charger Input Status
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_UNDERVOLTAGE:
> + * Charger input voltage (Vchgin) < Under Voltage Threshold (Vuvlo)
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE: Vchgin > Vuvlo and
> + * Vchgin < (Battery Voltage (Vbatt) + system voltage (Vsys))
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_OVERVOLTAGE:
> + * Vchgin > Over Voltage threshold (Vovlo)
> + * @MAX77759_CHGR_CHGIN_DTLS_VBUS_VALID:
> + * Vchgin > Vuvlo, Vchgin < Vovlo and Vchgin > (Vsys + Vbatt)
> + */
This comment is masquerading as a kernel-doc header, but isn't actually
kernel-doc. Either change the formatting or adapt the formatting to use
/** and use W=1 to check it.
> +enum max77759_chgr_chgin_dtls_status {
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_UNDERVOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_OVERVOLTAGE,
> + MAX77759_CHGR_CHGIN_DTLS_VBUS_VALID,
> +};
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j721e-main: Update delay select values for MMC1/2 subsystems
From: Moteen Shah @ 2026-03-31 12:19 UTC (permalink / raw)
To: Romain Naour, devicetree, linux-arm-kernel, linux-omap
Cc: conor+dt, krzk+dt, robh, kristo, vigneshr, nm, stable
In-Reply-To: <20260218203823.1825554-1-romain.naour@smile.fr>
Hey Romain,
Thanks for the patch
On 19/02/26 02:08, Romain Naour wrote:
> The previous SPRSP36J datasheet recommends to set ti,otap-del-sel-sd-hs
> value to 0 for MMC1 and MMC2 interfaces. These values were updated in
> kernel 6.5. As a result we have some occasional regression with ultra
> high speed DDR50 SDXC cards while mounting the rootfs:
This error shouldn't be limited to just DDR50, were you seeing similar
behavior with other speed modes?
>
> mmc1: error -110 whilst initialising SD card
>
> A similar issue may occur with u-boot after a reboot while
> initialising the SD card:
>
> mmc_init: -110, time 67
>
> Update the delay values for legacy and high speed modes, based on
> the latest revised datasheet SPRSP36K released in April 2024 [1].
>
> (MMC1/2 - SD/SDIO Interface): Updated/Changed the
> "OTAPDLYENA, DELAY ENABLE" and "OTAPDLYSEL, DELAY VALUE" for the
> Default Speed and High Speed modes from "0x0" to "0x1"
>
> [1] Table 6-86. MMC1/2 DLL Delay Mapping for All Timing Modes, in
> https://www.ti.com/lit/ds/symlink/tda4vm.pdf,
> (SPRSP36K – SEPTEMBER 2021 – REVISED APRIL 2024)
>
> Cc: stable@vger.kernel.org # 6.5+
> Fixes: af398252d68e ("arm64: dts: ti: k3-j721e-main: Update delay select values for MMC subsystems")
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> index d5fd30a01032..418e6010ef1f 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> @@ -1643,8 +1643,8 @@ main_sdhci1: mmc@4fb0000 {
> clocks = <&k3_clks 92 5>, <&k3_clks 92 0>;
> assigned-clocks = <&k3_clks 92 0>;
> assigned-clock-parents = <&k3_clks 92 1>;
> - ti,otap-del-sel-legacy = <0x0>;
> - ti,otap-del-sel-sd-hs = <0x0>;
> + ti,otap-del-sel-legacy = <0x1>;
> + ti,otap-del-sel-sd-hs = <0x1>;
> ti,otap-del-sel-sdr12 = <0xf>;
> ti,otap-del-sel-sdr25 = <0xf>;
> ti,otap-del-sel-sdr50 = <0xc>;
> @@ -1671,8 +1671,8 @@ main_sdhci2: mmc@4f98000 {
> clocks = <&k3_clks 93 5>, <&k3_clks 93 0>;
> assigned-clocks = <&k3_clks 93 0>;
> assigned-clock-parents = <&k3_clks 93 1>;
> - ti,otap-del-sel-legacy = <0x0>;
> - ti,otap-del-sel-sd-hs = <0x0>;
> + ti,otap-del-sel-legacy = <0x1>;
> + ti,otap-del-sel-sd-hs = <0x1>;
> ti,otap-del-sel-sdr12 = <0xf>;
> ti,otap-del-sel-sdr25 = <0xf>;
> ti,otap-del-sel-sdr50 = <0xc>;
Reviewed-by: Moteen Shah <m-shah@ti.com>
Regards,
Moteen
^ permalink raw reply
* RE:(3) [PATCH 1/1] arm: get task_stack reference before dump_backtrace
From: Maninder Singh @ 2026-03-31 12:16 UTC (permalink / raw)
To: bigeasy@linutronix.de
Cc: Russell King (Oracle), peterz@infradead.org, kees@kernel.org,
ardb@kernel.org, keithpac@amazon.com, linusw@kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, jpoimboe@kernel.org
In-Reply-To: <20260331065230.9T3e_rt-@linutronix.de>
Hi,
> Hi,
>
> > >"otherwise if someone calls show_stack() for task" ... and the stack
> > >trace given stops at show_stack() and doesn't show the "someone".
> > >
> > >I'd like to know _how_ this happens, and why ARM64 and now 32-bit ARM
> > >are different from x86.
> >
> > I tried to simulate same thing on x86_64, it is also crashing.
> >
> > Just a dummy code to save task_struct to reproduce the race:
> >
> > + rcu_read_lock();
> > + for_each_process(p) {
> > + if (!strcmp(p->comm, "sleep")) {
> > + check_task = p;
> > + get_task_struct(p);
> > + pr_emerg("get done for %s %d\n", p->comm, p->pid);
> > + }
> > + }
> > + rcu_read_unlock();
> >
> > // in mean time here sleep binary will be exited.
> >
> > + show_stack(check_task, NULL, KERN_EMERG);
>
> The task's stack is released on its final schedule() invocation.
> Therefore holding task_struct does not hold the stack of the task if it
> is separated out of task_struct and can be gone if the tasks quits.
>
> Therefore holding a reference to the stack while accessing it, like
> during a backtrace, makes sense and is required if the task is not
> current.
>
> Let me add this to my list and tackle it later today for x86. Then we
> get probably Russell on board for ARM.
>
Thanks :)
I have checked more details about missing this handling from x86 and found the reason.
Originally show_stack() was calling show_stack_log_lvl()
which had proper call to try_get_task_stack().
It was replaced with show_trace_log_lvl() later. Which removed this check.
Below commit did that.
commit 0ee1dd9f5e7eae4e55f95935b72d4beecb03de9c (HEAD)
Author: Josh Poimboeuf <jpoimboe@redhat.com>
Date: Tue Oct 25 09:51:13 2016 -0500
x86/dumpstack: Remove raw stack dump
...
...
-void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
- unsigned long *sp, char *log_lvl)
-{
...
...
- if (!try_get_task_stack(task))
- return;
-
...
@@ -171,12 +170,12 @@ void show_stack(struct task_struct *task, unsigned long *sp)
if (!sp && task == current)
sp = get_stack_pointer(current, NULL);
- show_stack_log_lvl(task, NULL, sp, KERN_DEFAULT);
+ show_trace_log_lvl(task, NULL, sp, KERN_DEFAULT);
}
So I think x86 patch shall also be merged to kernel:
https://lkml.org/lkml/2026/3/11/317
Thanks and regrads
Maninder singh
^ permalink raw reply
* Re: [PATCH 3/3] gpio: realtek: Add driver for Realtek DHC RTD1625 SoC
From: Bartosz Golaszewski @ 2026-03-31 12:15 UTC (permalink / raw)
To: Yu-Chun Lin
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
linux-realtek-soc, cy.huang, stanley_chang, james.tai, linusw,
brgl, robh, krzk+dt, conor+dt, afaerber, tychang
In-Reply-To: <20260331113835.3510341-4-eleanor.lin@realtek.com>
On Tue, 31 Mar 2026 13:38:35 +0200, Yu-Chun Lin <eleanor.lin@realtek.com> said:
> From: Tzuyi Chang <tychang@realtek.com>
>
> Add support for the GPIO controller found on Realtek DHC RTD1625 SoCs.
>
> Unlike the existing Realtek GPIO driver (drivers/gpio/gpio-rtd.c),
> which manages pins via shared bank registers, the RTD1625 introduces
> a per-pin register architecture. Each GPIO line now has its own
> dedicated 32-bit control register to manage configuration independently,
> including direction, output value, input value, interrupt enable, and
> debounce. Therefore, this distinct hardware design requires a separate
> driver.
>
> Signed-off-by: Tzuyi Chang <tychang@realtek.com>
> Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> ---
> drivers/gpio/Kconfig | 12 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-rtd1625.c | 581 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 594 insertions(+)
> create mode 100644 drivers/gpio/gpio-rtd1625.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index b45fb799e36c..6ffc95e02cb9 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -639,6 +639,18 @@ config GPIO_RTD
> Say yes here to support GPIO functionality and GPIO interrupt on
> Realtek DHC SoCs.
>
> +config GPIO_RTD1625
> + tristate "Realtek DHC RTD1625 GPIO support"
> + depends on ARCH_REALTEK || COMPILE_TEST
> + default y
Don't default to y for COMPILE_TEST. If you need this for ARCH_REALTEK then
limit it to it. Though seeing as it's a module_initcall() anyway, maybe you
don't need it at all?
> + select GPIOLIB_IRQCHIP
> + help
> + This option enables support for the GPIO controller on Realtek
> + DHC (Digital Home Center) RTD1625 SoC.
> +
> + Say yes here to support both basic GPIO line functionality
> + and GPIO interrupt handling capabilities for this platform.
> +
Other than that looks really nice!
Bart
^ permalink raw reply
* [PATCH v2] ARM: dts: aspeed: yosemite5: Add MP5998 power monitor
From: Daniel Hsu @ 2026-03-31 12:07 UTC (permalink / raw)
To: andrew
Cc: robh, krzk+dt, conor+dt, joel, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel, Daniel Hsu
Add an MP5998 power monitor used to monitor the power consumption
of the Paddle_P12V_HSC rail on the Yosemite5 paddle board.
[v2]
- Describe the purpose of the power monitor
- Remove software/driver related description
Signed-off-by: Daniel Hsu <Daniel-Hsu@quantatw.com>
---
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
index 2486981f3d6b..6287f80d70d9 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
@@ -422,6 +422,11 @@ power-sensor@45 {
shunt-resistor = <1000>;
};
+ power-monitor@46 {
+ compatible = "mps,mp5998";
+ reg = <0x46>;
+ };
+
power-monitor@47 {
compatible = "ti,tps25990";
reg = <0x47>;
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox